From cc0b063b1e428fb15fcc829ac47ebc2e8859da3e Mon Sep 17 00:00:00 2001
From: Timm Fitschen <t.fitschen@indiscale.com>
Date: Wed, 22 Sep 2021 00:25:25 +0200
Subject: [PATCH] BUG: add fix for caosdb-cpplib#24

---
 include/caosdb/protobuf_helper.h |  6 ------
 include/caosdb/value.h           |  2 ++
 src/caosdb/entity.cpp            | 18 ++++++++++++----
 test/test_list_properties.cpp    | 36 ++++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/include/caosdb/protobuf_helper.h b/include/caosdb/protobuf_helper.h
index 026887a..18e5400 100644
--- a/include/caosdb/protobuf_helper.h
+++ b/include/caosdb/protobuf_helper.h
@@ -45,12 +45,6 @@ auto get_arena() -> Arena *;
 template <typename P> class ProtoMessageWrapper {
 public:
   virtual ~ProtoMessageWrapper() = 0;
-  ProtoMessageWrapper(const ProtoMessageWrapper<P> &other) = default;
-  inline auto CopyFrom(const ProtoMessageWrapper<P> &other) noexcept -> StatusCode {
-    this->wrapped->CopyFrom(*other.wrapped);
-    return StatusCode::SUCCESS;
-  }
-
   /**
    * Return a json representation of this object.
    */
diff --git a/include/caosdb/value.h b/include/caosdb/value.h
index f4f8f3b..2937f18 100644
--- a/include/caosdb/value.h
+++ b/include/caosdb/value.h
@@ -394,6 +394,7 @@ public:
   inline auto operator=(const Value &other) -> Value & {
     if (&other != this) {
       this->wrapped->CopyFrom(*other.wrapped);
+      this->collection_values.reset();
     }
     return *this;
   }
@@ -404,6 +405,7 @@ public:
   inline auto operator=(Value &&other) -> Value & {
     if (&other != this) {
       this->wrapped = std::move(other.wrapped);
+      this->collection_values.reset();
     }
     return *this;
   }
diff --git a/src/caosdb/entity.cpp b/src/caosdb/entity.cpp
index 37d7b3c..050a9bd 100644
--- a/src/caosdb/entity.cpp
+++ b/src/caosdb/entity.cpp
@@ -90,7 +90,11 @@ auto Property::SetImportance(Importance importance) -> void {
   this->wrapped->set_importance(static_cast<ProtoImportance>(importance));
 }
 
-auto Property::SetValue(const Value &value) -> StatusCode { return this->value.CopyFrom(value); }
+auto Property::SetValue(const Value &value) -> StatusCode {
+  this->wrapped->mutable_value()->CopyFrom(*value.wrapped);
+  this->value = Value(this->wrapped->mutable_value());
+  return StatusCode::SUCCESS;
+}
 
 auto Property::SetValue(const AbstractValue &value) -> StatusCode { return SetValue(Value(value)); }
 
@@ -133,7 +137,9 @@ auto Property::SetValue(const bool value) -> StatusCode { return SetValue(Value(
 auto Property::SetUnit(const std::string &unit) -> void { this->wrapped->set_unit(unit); }
 
 auto Property::SetDataType(const DataType &new_data_type) -> StatusCode {
-  return this->data_type.CopyFrom(new_data_type);
+  this->wrapped->mutable_data_type()->CopyFrom(*new_data_type.wrapped);
+  this->data_type = DataType(this->wrapped->mutable_data_type());
+  return StatusCode::SUCCESS;
 }
 
 auto Property::SetDataType(const AtomicDataType new_data_type, bool list_type) -> StatusCode {
@@ -185,7 +191,9 @@ auto Entity::SetValue(const Value &value) -> StatusCode {
   if (GetRole() != Role::PROPERTY) {
     return StatusCode::ENTITY_CANNOT_HAVE_A_VALUE;
   }
-  return this->value.CopyFrom(value);
+  this->wrapped->mutable_value()->CopyFrom(*value.wrapped);
+  this->value = Value(this->wrapped->mutable_value());
+  return StatusCode::SUCCESS;
 }
 
 auto Entity::SetValue(const AbstractValue &value) -> StatusCode { return SetValue(Value(value)); }
@@ -232,7 +240,9 @@ auto Entity::SetDataType(const DataType &new_data_type) -> StatusCode {
   if (GetRole() != Role::PROPERTY) {
     return StatusCode::ENTITY_CANNOT_HAVE_A_DATA_TYPE;
   }
-  return this->data_type.CopyFrom(new_data_type);
+  this->wrapped->mutable_data_type()->CopyFrom(*new_data_type.wrapped);
+  this->data_type = DataType(this->wrapped->mutable_data_type());
+  return StatusCode::SUCCESS;
 }
 
 auto Entity::SetDataType(const AtomicDataType new_data_type, bool list_type) -> StatusCode {
diff --git a/test/test_list_properties.cpp b/test/test_list_properties.cpp
index 27d0aad..bc82896 100644
--- a/test/test_list_properties.cpp
+++ b/test/test_list_properties.cpp
@@ -24,6 +24,7 @@
 #include "caosdb/entity.h"                  // for Entity
 #include "caosdb/entity/v1alpha1/main.pb.h" // for AtomicDataType, DataType
 #include "caosdb/value.h"                   // for Value
+#include <cstdint>                          // for int64_t
 #include <gtest/gtest-message.h>            // for Message
 #include <gtest/gtest-test-part.h>          // for TestPartResult, SuiteApi...
 #include <gtest/gtest_pred_impl.h>          // for AssertionResult, Test
@@ -61,4 +62,39 @@ TEST(test_list_property, test_list_of_text) {
   EXPECT_EQ(value.GetAsVector().at(1).GetAsString(), "item2");
 }
 
+TEST(test_list_property, test_list_reassignment) {
+  Property list_property;
+  // assign int list
+  std::vector<int64_t> int_values{1, 2, 3};
+  list_property.SetValue(int_values);
+  const auto &value_ints = list_property.GetValue();
+  EXPECT_TRUE(value_ints.IsVector());
+  EXPECT_EQ(value_ints.GetAsVector().size(), 3);
+  for (int ii = 0; ii < 3; ii++) {
+    EXPECT_TRUE(value_ints.GetAsVector().at(ii).IsInt64());
+    EXPECT_EQ(value_ints.GetAsVector().at(ii).GetAsInt64(), int_values[ii]);
+  }
+
+  // Re-assign to double scalar
+  double double_value(1.23);
+  list_property.SetValue(double_value);
+  const auto &value_double = list_property.GetValue();
+  EXPECT_FALSE(value_double.IsVector());
+  EXPECT_TRUE(value_double.IsDouble());
+  EXPECT_FALSE(value_double.IsInt64());
+  EXPECT_EQ(value_double.GetAsDouble(), double_value);
+
+  // Re-assign to boolean list
+  std::vector<bool> bool_values{true, false, false, true};
+  list_property.SetValue(bool_values);
+  const auto &value_bools = list_property.GetValue();
+  EXPECT_TRUE(value_bools.IsVector());
+  EXPECT_EQ(value_bools.GetAsVector().size(), 4);
+  for (int jj = 0; jj < 4; jj++) {
+    EXPECT_TRUE(value_bools.GetAsVector().at(jj).IsBool());
+    EXPECT_FALSE(value_bools.GetAsVector().at(jj).IsInt64());
+    EXPECT_EQ(value_bools.GetAsVector().at(jj).GetAsBool(), bool_values[jj]);
+  }
+}
+
 } // namespace caosdb::entity
-- 
GitLab