# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1539082280 -7200
# Node ID 6eebc2eb3168c59681d608640b925af52d999635
# Parent  716dd24974ef0d8b8952f6cba31fe58755921b4b
refactoring serialization of RemoteModalityParameters

diff -r 716dd24974ef -r 6eebc2eb3168 Core/DicomNetworking/RemoteModalityParameters.cpp
--- a/Core/DicomNetworking/RemoteModalityParameters.cpp	Mon Oct 08 17:36:54 2018 +0200
+++ b/Core/DicomNetworking/RemoteModalityParameters.cpp	Tue Oct 09 12:51:20 2018 +0200
@@ -41,6 +41,13 @@
 #include <boost/lexical_cast.hpp>
 #include <stdexcept>
 
+
+static const char* KEY_AET = "AET";
+static const char* KEY_MANUFACTURER = "Manufacturer";
+static const char* KEY_PORT = "Port";
+static const char* KEY_HOST = "Host";
+
+
 namespace Orthanc
 {
   RemoteModalityParameters::RemoteModalityParameters() :
@@ -63,89 +70,144 @@
   }
 
 
-  void RemoteModalityParameters::FromJson(const Json::Value& modality)
+  static uint16_t ReadPortNumber(const Json::Value& value)
   {
-    if (!modality.isArray() ||
-        (modality.size() != 3 && modality.size() != 4))
+    int tmp;
+
+    switch (value.type())
+    {
+      case Json::intValue:
+      case Json::uintValue:
+        tmp = value.asInt();
+        break;
+
+      case Json::stringValue:
+        try
+        {
+          tmp = boost::lexical_cast<int>(value.asString());
+        }
+        catch (boost::bad_lexical_cast&)
+        {
+          throw OrthancException(ErrorCode_BadFileFormat);
+        }
+        break;
+
+      default:
+        throw OrthancException(ErrorCode_BadFileFormat);
+    }
+
+    if (tmp <= 0 || tmp >= 65535)
+    {
+      LOG(ERROR) << "A TCP port number must be in range [1..65534], but found: " << tmp;
+      throw OrthancException(ErrorCode_ParameterOutOfRange);
+    }
+    else
+    {
+      return static_cast<uint16_t>(tmp);
+    }
+  }
+
+
+  void RemoteModalityParameters::UnserializeArray(const Json::Value& serialized)
+  {
+    assert(serialized.type() == Json::arrayValue);
+
+    if ((serialized.size() != 3 && 
+         serialized.size() != 4) ||
+        serialized[0].type() != Json::stringValue ||
+        serialized[1].type() != Json::stringValue ||
+        (serialized.size() == 4 &&
+         serialized[3].type() != Json::stringValue))
     {
       throw OrthancException(ErrorCode_BadFileFormat);
     }
 
-    SetApplicationEntityTitle(modality.get(0u, "").asString());
-    SetHost(modality.get(1u, "").asString());
-
-    const Json::Value& portValue = modality.get(2u, "");
-    try
-    {
-      int tmp = portValue.asInt();
+    aet_ = serialized[0].asString();
+    host_ = serialized[1].asString();
+    port_ = ReadPortNumber(serialized[2]);
 
-      if (tmp <= 0 || tmp >= 65535)
-      {
-        throw OrthancException(ErrorCode_ParameterOutOfRange);
-      }
-
-      SetPortNumber(static_cast<uint16_t>(tmp));
-    }
-    catch (std::runtime_error& /* error inside JsonCpp */)
+    if (serialized.size() == 4)
     {
-      try
-      {
-        SetPortNumber(boost::lexical_cast<uint16_t>(portValue.asString()));
-      }
-      catch (boost::bad_lexical_cast&)
-      {
-        throw OrthancException(ErrorCode_BadFileFormat);
-      }
-    }
-
-    if (modality.size() == 4)
-    {
-      const std::string& manufacturer = modality.get(3u, "").asString();
-
-      try
-      {
-        SetManufacturer(manufacturer);
-      }
-      catch (OrthancException&)
-      {
-        LOG(ERROR) << "Unknown modality manufacturer: \"" << manufacturer << "\"";
-        throw;
-      }
+      manufacturer_ = StringToModalityManufacturer(serialized[3].asString());
     }
     else
     {
-      SetManufacturer(ModalityManufacturer_Generic);
+      manufacturer_ = ModalityManufacturer_Generic;
     }
   }
 
   
-  void RemoteModalityParameters::ToJson(Json::Value& value) const
+  void RemoteModalityParameters::UnserializeObject(const Json::Value& serialized)
   {
-    value = Json::arrayValue;
-    value.append(GetApplicationEntityTitle());
-    value.append(GetHost());
-    value.append(GetPortNumber());
-    value.append(EnumerationToString(GetManufacturer()));
+    assert(serialized.type() == Json::objectValue);
+
+    aet_ = SerializationToolbox::ReadString(serialized, KEY_AET);
+    host_ = SerializationToolbox::ReadString(serialized, KEY_HOST);
+
+    if (serialized.isMember(KEY_PORT))
+    {
+      port_ = ReadPortNumber(serialized[KEY_PORT]);
+    }
+    else
+    {
+      throw OrthancException(ErrorCode_BadFileFormat);
+    }
+
+    if (serialized.isMember(KEY_MANUFACTURER))
+    {
+      manufacturer_ = StringToModalityManufacturer
+        (SerializationToolbox::ReadString(serialized, KEY_MANUFACTURER));
+    }   
+    else
+    {
+      manufacturer_ = ModalityManufacturer_Generic;
+    }
+  }
+
+
+  bool RemoteModalityParameters::IsAdvancedFormatNeeded() const
+  {
+    return false; // TODO
   }
 
   
-  void RemoteModalityParameters::Serialize(Json::Value& target) const
+  void RemoteModalityParameters::Serialize(Json::Value& target,
+                                           bool forceAdvancedFormat) const
   {
-    target = Json::objectValue;
-    target["AET"] = aet_;
-    target["Host"] = host_;
-    target["Port"] = port_;
-    target["Manufacturer"] = EnumerationToString(manufacturer_);
+    if (forceAdvancedFormat ||
+        IsAdvancedFormatNeeded())
+    {
+      target = Json::objectValue;
+      target[KEY_AET] = aet_;
+      target[KEY_HOST] = host_;
+      target[KEY_PORT] = port_;
+      target[KEY_MANUFACTURER] = EnumerationToString(manufacturer_);
+    }
+    else
+    {
+      target = Json::arrayValue;
+      target.append(GetApplicationEntityTitle());
+      target.append(GetHost());
+      target.append(GetPortNumber());
+      target.append(EnumerationToString(GetManufacturer()));
+    }
   }
 
   
-  RemoteModalityParameters::RemoteModalityParameters(const Json::Value& serialized)
+  void RemoteModalityParameters::Unserialize(const Json::Value& serialized)
   {
-    aet_ = SerializationToolbox::ReadString(serialized, "AET");
-    host_ = SerializationToolbox::ReadString(serialized, "Host");
-    port_ = static_cast<uint16_t>
-      (SerializationToolbox::ReadUnsignedInteger(serialized, "Port"));
-    manufacturer_ = StringToModalityManufacturer
-      (SerializationToolbox::ReadString(serialized, "Manufacturer"));
+    switch (serialized.type())
+    {
+      case Json::objectValue:
+        UnserializeObject(serialized);
+        break;
+
+      case Json::arrayValue:
+        UnserializeArray(serialized);
+        break;
+
+      default:
+        throw OrthancException(ErrorCode_BadFileFormat);
+    }
   }
 }
diff -r 716dd24974ef -r 6eebc2eb3168 Core/DicomNetworking/RemoteModalityParameters.h
--- a/Core/DicomNetworking/RemoteModalityParameters.h	Mon Oct 08 17:36:54 2018 +0200
+++ b/Core/DicomNetworking/RemoteModalityParameters.h	Tue Oct 09 12:51:20 2018 +0200
@@ -49,10 +49,17 @@
     uint16_t port_;
     ModalityManufacturer manufacturer_;
 
+    void UnserializeArray(const Json::Value& serialized);
+
+    void UnserializeObject(const Json::Value& serialized);
+
   public:
     RemoteModalityParameters();
 
-    RemoteModalityParameters(const Json::Value& serialized);
+    RemoteModalityParameters(const Json::Value& serialized)
+    {
+      Unserialize(serialized);
+    }
 
     RemoteModalityParameters(const std::string& aet,
                              const std::string& host,
@@ -104,10 +111,11 @@
       manufacturer_ = StringToModalityManufacturer(manufacturer);
     }
 
-    void FromJson(const Json::Value& modality);
+    void Unserialize(const Json::Value& modality);
 
-    void ToJson(Json::Value& value) const;
+    bool IsAdvancedFormatNeeded() const;
 
-    void Serialize(Json::Value& target) const;
+    void Serialize(Json::Value& target,
+                   bool forceAdvancedFormat) const;
   };
 }
diff -r 716dd24974ef -r 6eebc2eb3168 Core/Enumerations.cpp
--- a/Core/Enumerations.cpp	Mon Oct 08 17:36:54 2018 +0200
+++ b/Core/Enumerations.cpp	Tue Oct 09 12:51:20 2018 +0200
@@ -1434,6 +1434,7 @@
     }
     else
     {
+      LOG(ERROR) << "Unknown modality manufacturer: \"" << manufacturer << "\"";
       throw OrthancException(ErrorCode_ParameterOutOfRange);
     }
 
diff -r 716dd24974ef -r 6eebc2eb3168 OrthancServer/OrthancInitialization.cpp
--- a/OrthancServer/OrthancInitialization.cpp	Mon Oct 08 17:36:54 2018 +0200
+++ b/OrthancServer/OrthancInitialization.cpp	Tue Oct 09 12:51:20 2018 +0200
@@ -599,7 +599,7 @@
 
     try
     {
-      modality.FromJson(modalities[name]);
+      modality.Unserialize(modalities[name]);
     }
     catch (OrthancException&)
     {
@@ -917,7 +917,7 @@
       modalities.removeMember(symbolicName);
 
       Json::Value v;
-      modality.ToJson(v);
+      modality.Serialize(v, true /* force advanced format */);
       modalities[symbolicName] = v;
     }
 
diff -r 716dd24974ef -r 6eebc2eb3168 OrthancServer/OrthancRestApi/OrthancRestModalities.cpp
--- a/OrthancServer/OrthancRestApi/OrthancRestModalities.cpp	Mon Oct 08 17:36:54 2018 +0200
+++ b/OrthancServer/OrthancRestApi/OrthancRestModalities.cpp	Tue Oct 09 12:51:20 2018 +0200
@@ -829,7 +829,9 @@
         if (Configuration::GetOrthancPeer(peer, *it))
         {
           Json::Value jsonPeer = Json::objectValue;
-          // only return the minimum information to identify the destination, do not include "security" information like passwords
+          // only return the minimum information to identify the
+          // destination, do not include "security" information like
+          // passwords
           jsonPeer["Url"] = peer.GetUrl();
           if (!peer.GetUsername().empty())
           {
@@ -911,10 +913,11 @@
       for (OrthancRestApi::SetOfStrings::const_iterator
              it = modalities.begin(); it != modalities.end(); ++it)
       {
-        Json::Value modality;
-        Configuration::GetModalityUsingSymbolicName(*it).ToJson(modality);
-
-        result[*it] = modality;
+        const RemoteModalityParameters& remote = Configuration::GetModalityUsingSymbolicName(*it);
+        
+        Json::Value info;
+        remote.Serialize(info, true /* force advanced format */);
+        result[*it] = info;
       }
       call.GetOutput().AnswerJson(result);
     }
@@ -953,7 +956,7 @@
     if (reader.parse(call.GetBodyData(), call.GetBodyData() + call.GetBodySize(), json))
     {
       RemoteModalityParameters modality;
-      modality.FromJson(json);
+      modality.Unserialize(json);
       Configuration::UpdateModality(context, call.GetUriComponent("id", ""), modality);
       call.GetOutput().AnswerBuffer("", "text/plain");
     }
diff -r 716dd24974ef -r 6eebc2eb3168 OrthancServer/ServerJobs/DicomModalityStoreJob.cpp
--- a/OrthancServer/ServerJobs/DicomModalityStoreJob.cpp	Mon Oct 08 17:36:54 2018 +0200
+++ b/OrthancServer/ServerJobs/DicomModalityStoreJob.cpp	Tue Oct 09 12:51:20 2018 +0200
@@ -219,7 +219,7 @@
     else
     {
       target[LOCAL_AET] = localAet_;
-      remote_.Serialize(target[REMOTE]);
+      remote_.Serialize(target[REMOTE], true /* force advanced format */);
       target[MOVE_ORIGINATOR_AET] = moveOriginatorAet_;
       target[MOVE_ORIGINATOR_ID] = moveOriginatorId_;
       return true;
diff -r 716dd24974ef -r 6eebc2eb3168 OrthancServer/ServerJobs/DicomMoveScuJob.cpp
--- a/OrthancServer/ServerJobs/DicomMoveScuJob.cpp	Mon Oct 08 17:36:54 2018 +0200
+++ b/OrthancServer/ServerJobs/DicomMoveScuJob.cpp	Tue Oct 09 12:51:20 2018 +0200
@@ -192,7 +192,7 @@
     {
       target[LOCAL_AET] = localAet_;
       target[TARGET_AET] = targetAet_;
-      remote_.Serialize(target[REMOTE]);
+      remote_.Serialize(target[REMOTE], true /* force advanced format */);
       return true;
     }
   }
diff -r 716dd24974ef -r 6eebc2eb3168 OrthancServer/ServerJobs/Operations/StoreScuOperation.cpp
--- a/OrthancServer/ServerJobs/Operations/StoreScuOperation.cpp	Mon Oct 08 17:36:54 2018 +0200
+++ b/OrthancServer/ServerJobs/Operations/StoreScuOperation.cpp	Tue Oct 09 12:51:20 2018 +0200
@@ -87,7 +87,7 @@
     result = Json::objectValue;
     result["Type"] = "StoreScu";
     result["LocalAET"] = localAet_;
-    modality_.Serialize(result["Modality"]);
+    modality_.Serialize(result["Modality"], true /* force advanced format */);
   }
 
 
diff -r 716dd24974ef -r 6eebc2eb3168 UnitTestsSources/MultiThreadingTests.cpp
--- a/UnitTestsSources/MultiThreadingTests.cpp	Mon Oct 08 17:36:54 2018 +0200
+++ b/UnitTestsSources/MultiThreadingTests.cpp	Tue Oct 09 12:51:20 2018 +0200
@@ -1867,3 +1867,51 @@
     ASSERT_THROW(job.Step(), OrthancException);
   }
 }
+
+
+TEST(JobsSerialization, RemoteModalityParameters)
+{
+  Json::Value s;
+
+  {
+    RemoteModalityParameters modality;
+    modality.Serialize(s, false);
+    ASSERT_EQ(Json::arrayValue, s.type());
+  }
+
+  {
+    RemoteModalityParameters modality(s);
+    ASSERT_EQ("ORTHANC", modality.GetApplicationEntityTitle());
+    ASSERT_EQ("127.0.0.1", modality.GetHost());
+    ASSERT_EQ(104u, modality.GetPortNumber());
+    ASSERT_EQ(ModalityManufacturer_Generic, modality.GetManufacturer());
+  }
+
+  s = Json::nullValue;
+
+  {
+    RemoteModalityParameters modality;
+    ASSERT_FALSE(modality.IsAdvancedFormatNeeded());
+    modality.SetApplicationEntityTitle("HELLO");
+    modality.SetHost("world");
+    modality.SetPortNumber(45);
+    modality.SetManufacturer(ModalityManufacturer_Dcm4Chee);
+    modality.Serialize(s, true);
+    ASSERT_EQ(Json::objectValue, s.type());
+  }
+
+  {
+    RemoteModalityParameters modality(s);
+    ASSERT_EQ("HELLO", modality.GetApplicationEntityTitle());
+    ASSERT_EQ("world", modality.GetHost());
+    ASSERT_EQ(45u, modality.GetPortNumber());
+    ASSERT_EQ(ModalityManufacturer_Dcm4Chee, modality.GetManufacturer());
+  }
+
+  s["Port"] = "46";
+  
+  {
+    RemoteModalityParameters modality(s);
+    ASSERT_EQ(46u, modality.GetPortNumber());
+  }
+}