Fix copying of ExpectData objects (#337)

Classes derived form `ExpectData` can use compiler generated copy
constructors and they will behave correctly. The copy constructors call the
copy constructors of `CompositeData` and `ExpectData` respectively.
However, copying concrete ExpectData objects fails because the copy
constructor of `CompositeData` is never called. This patch fixes the
problem by manually calling `CompositeData::Copy` when it detects that
the copy constructor of `CompositeData` has not been called.

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

include/ignition/physics/detail/SpecifyData.hh

@@ -20,6 +20,7 @@
 
 #include <memory>
 #include <utility>
+#include <iostream>
 
 #include "ignition/physics/SpecifyData.hh"
 
@@ -41,10 +42,18 @@ namespace ignition
 
     /////////////////////////////////////////////////
     template <typename Expected>
-    ExpectData<Expected>::ExpectData(const ExpectData<Expected> &)
+    ExpectData<Expected>::ExpectData(const ExpectData<Expected> &_other)
         : ExpectData()
     {
-      // Do nothing
+      // Call CompositeData::Copy if it hasn't already been called. Note that
+      // when classed derived from ExpectData use compiler generated copy
+      // constructors, CompositeData's copy constructor is called before
+      // ExpectData's constructor. However, since ExpectData has a user defined
+      // copy constructor, copying objects of ExpectData type will not
+      // automatically call CompositeData's copy constructor.
+      if (this->EntryCount() != _other.EntryCount()){
+        this->Copy(_other);
+      }
     }
 
     /////////////////////////////////////////////////

src/SpecifyData_TEST.cc

@@ -685,6 +685,39 @@ TEST(SpecifyData, Copy)
   }
 }
 
+/////////////////////////////////////////////////
+TEST(SpecifyData, CopyExpectData)
+{
+  ignition::physics::ExpectData<StringData> data;
+  data.Get<StringData>().myString = "old_string";
+  EXPECT_EQ("old_string", data.Get<StringData>().myString);
+
+  ignition::physics::ExpectData<StringData>copyCtor(data);
+  EXPECT_EQ("old_string", copyCtor.Get<StringData>().myString);
+
+  // Modify the original and check that the copy is not affected
+  data.Get<StringData>().myString = "new_string";
+  EXPECT_EQ("old_string", copyCtor.Get<StringData>().myString);
+}
+
+class ExpectString : public virtual ignition::physics::ExpectData<StringData>
+{
+};
+
+TEST(SpecifyData, CopyExpectString)
+{
+  ExpectString data;
+  data.Get<StringData>().myString = "old_string";
+  EXPECT_EQ("old_string", data.Get<StringData>().myString);
+
+  ExpectString copyCtor(data);
+  EXPECT_EQ("old_string", copyCtor.Get<StringData>().myString);
+
+  // Modify the original and check that the copy is not affected
+  data.Get<StringData>().myString = "new_string";
+  EXPECT_EQ("old_string", copyCtor.Get<StringData>().myString);
+}
+
 /////////////////////////////////////////////////
 TEST(SpecifyData, Move)
 {