BUG: fix DataType and Value ToString

include/caosdb/data_type.h

@@ -141,11 +141,12 @@ protected:
 class DataType : public ScalarProtoMessageWrapper<ProtoDataType> {
 public:
   DataType(ProtoDataType *wrapped) : ScalarProtoMessageWrapper<ProtoDataType>(wrapped) {}
-  DataType() : ScalarProtoMessageWrapper<ProtoDataType>() {}
+  DataType() : ScalarProtoMessageWrapper<ProtoDataType>(static_cast<ProtoDataType *>(nullptr)) {}
   /**
    * Create an AtomicDataType typed DataType.  For references, use the std::string constructor.
    */
-  DataType(AtomicDataType data_type, bool list_type = false) : DataType() {
+  DataType(AtomicDataType data_type, bool list_type = false)
+    : ScalarProtoMessageWrapper<ProtoDataType>() {
     if (list_type) {
       this->wrapped->mutable_list_data_type()->set_atomic_data_type(
         static_cast<ProtoAtomicDataType>(data_type));
@@ -156,7 +157,8 @@ public:
   /**
    * Create a reference typed DataType.
    */
-  DataType(const std::string &data_type, bool list_type = false) : DataType() {
+  DataType(const std::string &data_type, bool list_type = false)
+    : ScalarProtoMessageWrapper<ProtoDataType>() {
     if (list_type) {
       this->wrapped->mutable_list_data_type()->mutable_reference_data_type()->set_name(data_type);
     } else {
@@ -165,27 +167,28 @@ public:
   }
 
   inline static auto ListOf(const AtomicDataType &atomic_data_type) -> DataType {
-    DataType result;
-    result.wrapped->mutable_list_data_type()->set_atomic_data_type(
-      static_cast<ProtoAtomicDataType>(atomic_data_type));
-    return result;
+    return DataType(atomic_data_type, true);
   }
   inline static auto ListOf(const std::string reference_data_type) -> DataType {
-    DataType result;
-    result.wrapped->mutable_list_data_type()->mutable_reference_data_type()->set_name(
-      reference_data_type);
-    return result;
+    return DataType(reference_data_type, true);
   }
 
+  [[nodiscard]] inline auto IsUndefined() const noexcept -> bool {
+    return this->wrapped == nullptr;
+  }
   [[nodiscard]] inline auto IsAtomic() const noexcept -> bool {
-    return this->wrapped->data_type_case() == DataTypeCase::kAtomicDataType;
+    return !IsUndefined() && this->wrapped->data_type_case() == DataTypeCase::kAtomicDataType;
   }
   [[nodiscard]] inline auto GetAsAtomic() const noexcept -> AtomicDataType {
+    if (!IsAtomic()) {
+      static AtomicDataType undefined;
+      return undefined;
+    }
     return static_cast<AtomicDataType>(this->wrapped->atomic_data_type());
   }
 
   [[nodiscard]] inline auto IsReference() const noexcept -> bool {
-    return this->wrapped->data_type_case() == DataTypeCase::kReferenceDataType;
+    return !IsUndefined() && this->wrapped->data_type_case() == DataTypeCase::kReferenceDataType;
   }
   [[nodiscard]] inline auto GetAsReference() const noexcept -> const ReferenceDataType & {
     if (!IsReference()) {
@@ -198,7 +201,7 @@ public:
   }
 
   [[nodiscard]] inline auto IsList() const noexcept -> bool {
-    return this->wrapped->data_type_case() == DataTypeCase::kListDataType;
+    return !IsUndefined() && this->wrapped->data_type_case() == DataTypeCase::kListDataType;
   }
 
   [[nodiscard]] inline auto GetAsList() const noexcept -> const ListDataType & {
@@ -210,10 +213,19 @@ public:
     return *list_data_type;
   }
 
+  /**
+   * Return true if `other` is equal to this object.
+   *
+   * This compares the underlying wrapped ProtoMessages and return true if they
+   * are both nullptrs or if the serialization is equal.
+   */
   inline auto operator==(const DataType &other) const noexcept -> bool {
-    // TODO(tf) Is this safe?
+    if (this->wrapped != nullptr && other.wrapped != nullptr) {
       return this->wrapped->SerializeAsString() == other.wrapped->SerializeAsString();
     }
+    // both nullptr?
+    return this->wrapped == other.wrapped;
+  }
 
   friend class Entity;
   friend class Property;

include/caosdb/protobuf_helper.h

@@ -99,6 +99,9 @@ public:
    * Return a json representation of this object.
    */
   inline auto ToString() const noexcept -> const std::string override {
+    if (this->wrapped == nullptr) {
+      return "{}\n";
+    }
     CAOSDB_DEBUG_MESSAGE_STRING(*this->wrapped, out)
     return out;
   }

include/caosdb/value.h

@@ -208,34 +208,36 @@ public:
     : ScalarProtoMessageWrapper<ProtoScalarValue>(wrapped), proto_value(nullptr) {}
 
   [[nodiscard]] inline auto IsNull() const noexcept -> bool {
-    return (this->wrapped->scalar_value_case() == ScalarValueCase::kSpecialValue &&
+    return this->wrapped == nullptr ||
+           (this->wrapped->scalar_value_case() == ScalarValueCase::kSpecialValue &&
             this->wrapped->special_value() == ProtoSpecialValue::SPECIAL_VALUE_UNSPECIFIED);
   }
   [[nodiscard]] inline auto IsString() const noexcept -> bool {
-    return (this->wrapped->scalar_value_case() == ScalarValueCase::kStringValue) ||
+    return !IsNull() &&
+           ((this->wrapped->scalar_value_case() == ScalarValueCase::kStringValue) ||
             (this->wrapped->scalar_value_case() == ScalarValueCase::kSpecialValue &&
-            this->wrapped->special_value() == ProtoSpecialValue::SPECIAL_VALUE_EMPTY_STRING);
+             this->wrapped->special_value() == ProtoSpecialValue::SPECIAL_VALUE_EMPTY_STRING));
   }
   [[nodiscard]] inline auto GetAsString() const noexcept -> const std::string & {
     return this->wrapped->string_value();
   }
 
   [[nodiscard]] inline auto IsDouble() const noexcept -> bool {
-    return (this->wrapped->scalar_value_case() == ScalarValueCase::kDoubleValue);
+    return !IsNull() && (this->wrapped->scalar_value_case() == ScalarValueCase::kDoubleValue);
   }
   [[nodiscard]] inline auto GetAsDouble() const noexcept -> double {
     return this->wrapped->double_value();
   }
 
   [[nodiscard]] inline auto IsInt64() const noexcept -> bool {
-    return (this->wrapped->scalar_value_case() == ScalarValueCase::kIntegerValue);
+    return !IsNull() && (this->wrapped->scalar_value_case() == ScalarValueCase::kIntegerValue);
   }
   [[nodiscard]] inline auto GetAsInt64() const noexcept -> int64_t {
     return this->wrapped->integer_value();
   }
 
   [[nodiscard]] inline auto IsBool() const noexcept -> bool {
-    return (this->wrapped->scalar_value_case() == ScalarValueCase::kBooleanValue);
+    return !IsNull() && (this->wrapped->scalar_value_case() == ScalarValueCase::kBooleanValue);
   }
   [[nodiscard]] inline auto GetAsBool() const noexcept -> bool {
     return this->wrapped->boolean_value();
@@ -274,7 +276,9 @@ public:
   /**
    * Copy constructor.
    */
-  inline Value(const Value &original) : Value() { this->wrapped->CopyFrom(*original.wrapped); }
+  inline Value(const Value &original) : ScalarProtoMessageWrapper<ProtoValue>() {
+    this->wrapped->CopyFrom(*original.wrapped);
+  }
   /**
    * Move constructor.
    */
@@ -283,13 +287,13 @@ public:
    * Destructor.
    */
   inline ~Value() {}
-  inline Value() : ScalarProtoMessageWrapper<ProtoValue>() {
+  inline Value() : ScalarProtoMessageWrapper<ProtoValue>(static_cast<ProtoValue *>(nullptr)) {
     // has NULL_VALUE now
   }
-  explicit inline Value(const ScalarValue &value) : Value() {
+  explicit inline Value(const ScalarValue &value) : ScalarProtoMessageWrapper<ProtoValue>() {
     this->wrapped->mutable_scalar_value()->CopyFrom(*value.wrapped);
   }
-  explicit inline Value(const AbstractValue &value) : Value() {
+  explicit inline Value(const AbstractValue &value) : ScalarProtoMessageWrapper<ProtoValue>() {
     this->wrapped->CopyFrom(*value.GetProtoValue());
   }
   explicit inline Value(ProtoValue *wrapped) : ScalarProtoMessageWrapper<ProtoValue>(wrapped) {}
@@ -318,14 +322,15 @@ public:
   LIST_VALUE_CONSTRUCTOR(bool, set_boolean_value)
 
   [[nodiscard]] inline auto IsNull() const noexcept -> bool {
-    return this->wrapped->value_case() == ValueCase::VALUE_NOT_SET ||
+    return this->wrapped == nullptr ||
+           (this->wrapped->value_case() == ValueCase::VALUE_NOT_SET ||
             (this->wrapped->scalar_value().scalar_value_case() == ScalarValueCase::kSpecialValue &&
              this->wrapped->scalar_value().special_value() ==
-              ProtoSpecialValue::SPECIAL_VALUE_UNSPECIFIED);
+               ProtoSpecialValue::SPECIAL_VALUE_UNSPECIFIED));
   }
 
   [[nodiscard]] inline auto IsString() const noexcept -> bool {
-    if (this->wrapped->value_case() == ValueCase::kScalarValue) {
+    if (!IsNull() && this->wrapped->value_case() == ValueCase::kScalarValue) {
 
       return (this->wrapped->scalar_value().scalar_value_case() == ScalarValueCase::kStringValue) ||
              (this->wrapped->scalar_value().scalar_value_case() == ScalarValueCase::kSpecialValue &&
@@ -335,50 +340,60 @@ public:
     return false;
   }
   [[nodiscard]] inline auto GetAsString() const noexcept -> const std::string & {
+    if (!IsString()) {
+      static std::string empty_string;
+      return empty_string;
+    }
     return this->wrapped->scalar_value().string_value();
-    ;
   }
 
   [[nodiscard]] inline auto IsDouble() const noexcept -> bool {
-    if (this->wrapped->value_case() == ValueCase::kScalarValue) {
+    if (!IsNull() && this->wrapped->value_case() == ValueCase::kScalarValue) {
 
       return (this->wrapped->scalar_value().scalar_value_case() == ScalarValueCase::kDoubleValue);
     }
     return false;
   }
   [[nodiscard]] inline auto GetAsDouble() const noexcept -> double {
+    if (!IsDouble())
+      return 0.0;
     return this->wrapped->scalar_value().double_value();
   }
 
   [[nodiscard]] inline auto IsInt64() const noexcept -> bool {
-    if (this->wrapped->value_case() == ValueCase::kScalarValue) {
+    if (!IsNull() && this->wrapped->value_case() == ValueCase::kScalarValue) {
 
       return (this->wrapped->scalar_value().scalar_value_case() == ScalarValueCase::kIntegerValue);
     }
     return false;
   }
   [[nodiscard]] inline auto GetAsInt64() const noexcept -> int64_t {
+    if (!IsInt64())
+      return 0;
     return this->wrapped->scalar_value().integer_value();
   }
 
   [[nodiscard]] inline auto IsBool() const noexcept -> bool {
-    if (this->wrapped->value_case() == ValueCase::kScalarValue) {
+    if (!IsNull() && this->wrapped->value_case() == ValueCase::kScalarValue) {
 
       return (this->wrapped->scalar_value().scalar_value_case() == ScalarValueCase::kBooleanValue);
     }
     return false;
   }
   [[nodiscard]] inline auto GetAsBool() const noexcept -> bool {
+    if (!IsBool())
+      return false;
     return this->wrapped->scalar_value().boolean_value();
   }
 
   [[nodiscard]] inline auto IsVector() const noexcept -> bool {
-    return this->wrapped->value_case() == ValueCase::kListValues;
+    return !IsNull() && this->wrapped->value_case() == ValueCase::kListValues;
   }
   [[nodiscard]] inline auto GetAsVector() const noexcept -> const std::vector<ScalarValue> & {
     if (!IsVector()) {
       // create empty list
-      this->collection_values = std::make_unique<std::vector<ScalarValue>>();
+      static std::vector<ScalarValue> empty_values;
+      return empty_values;
     }
     if (this->collection_values == nullptr) {
       this->collection_values = std::make_unique<std::vector<ScalarValue>>();

src/caosdb/entity.cpp

@@ -91,8 +91,13 @@ auto Property::SetImportance(Importance importance) -> void {
 }
 
 auto Property::SetValue(const Value &value) -> StatusCode {
+  if (value.wrapped == nullptr) {
+    this->wrapped->clear_value();
+    this->value = Value();
+  } else {
     this->wrapped->mutable_value()->CopyFrom(*value.wrapped);
     this->value = Value(this->wrapped->mutable_value());
+  }
   return StatusCode::SUCCESS;
 }
 
@@ -137,8 +142,13 @@ 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 {
+  if (new_data_type.wrapped == nullptr) {
+    this->wrapped->clear_data_type();
+    this->data_type = DataType();
+  } else {
     this->wrapped->mutable_data_type()->CopyFrom(*new_data_type.wrapped);
     this->data_type = DataType(this->wrapped->mutable_data_type());
+  }
   return StatusCode::SUCCESS;
 }
 
@@ -187,8 +197,13 @@ auto Entity::SetValue(const Value &value) -> StatusCode {
   if (GetRole() != Role::PROPERTY) {
     return StatusCode::ENTITY_CANNOT_HAVE_A_VALUE;
   }
+  if (value.wrapped == nullptr) {
+    this->wrapped->clear_value();
+    this->value = Value();
+  } else {
     this->wrapped->mutable_value()->CopyFrom(*value.wrapped);
     this->value = Value(this->wrapped->mutable_value());
+  }
   return StatusCode::SUCCESS;
 }
 
@@ -236,8 +251,13 @@ auto Entity::SetDataType(const DataType &new_data_type) -> StatusCode {
   if (GetRole() != Role::PROPERTY) {
     return StatusCode::ENTITY_CANNOT_HAVE_A_DATA_TYPE;
   }
+  if (new_data_type.wrapped == nullptr) {
+    this->wrapped->clear_data_type();
+    this->data_type = DataType();
+  } else {
     this->wrapped->mutable_data_type()->CopyFrom(*new_data_type.wrapped);
     this->data_type = DataType(this->wrapped->mutable_data_type());
+  }
   return StatusCode::SUCCESS;
 }
 

test/test_entity.cpp

@@ -568,6 +568,15 @@ TEST(test_entity, test_add_file) {
 TEST(test_entity, test_entity_to_string) {
   Entity entity;
   EXPECT_EQ(entity.ToString(), "{}\n");
+
+  entity.SetRole(Role::PROPERTY);
+  EXPECT_EQ(entity.ToString(), "{\n \"role\": \"ENTITY_ROLE_PROPERTY\"\n}\n");
+
+  entity.SetValue(Value());
+  EXPECT_EQ(entity.ToString(), "{\n \"role\": \"ENTITY_ROLE_PROPERTY\"\n}\n");
+
+  entity.SetDataType(DataType());
+  EXPECT_EQ(entity.ToString(), "{\n \"role\": \"ENTITY_ROLE_PROPERTY\"\n}\n");
 }
 
 TEST(test_entity, test_properties_to_string) {