Refactoring of Value, ScalarValue classes

include/caosdb/entity.h

@@ -515,6 +515,7 @@ public:
    * Set the value of this property.
    */
   auto SetValue(const Value &value) -> StatusCode;
+  // auto SetValue(const AbstractValue &value) -> StatusCode;
   auto SetValue(const std::string &value) -> StatusCode;
   auto SetValue(const char *value) -> StatusCode;
   auto SetValue(const double value) -> StatusCode;
@@ -687,6 +688,7 @@ public:
    */
   auto SetDescription(const std::string &description) -> void;
 
+  // auto SetValue(const AbstractValue &value) -> StatusCode;
   auto SetValue(const Value &value) -> StatusCode;
   auto SetValue(const std::string &value) -> StatusCode;
   auto SetValue(const char *value) -> StatusCode;

include/caosdb/value.h

@@ -22,13 +22,17 @@
 #ifndef CAOSDB_VALUE_H
 #define CAOSDB_VALUE_H
 #include "caosdb/protobuf_helper.h"         // for ProtoMessageWrapper
+#include "caosdb/logging.h"                 // IWYU pragma: keep
 #include "caosdb/entity/v1alpha1/main.pb.h" // for RepeatedPtrField, Message
-#include "caosdb/logging.h"
-#include <google/protobuf/util/json_util.h> // for MessageToJson...
-#include <map>
-#include <memory> // for unique_ptr
-#include <string> // for string
-#include <vector> // for vector
+
+#include <cstdint>                                  // for int64_t
+#include <google/protobuf/arena.h>                  // for Arena
+#include <google/protobuf/generated_message_util.h> // for Arena
+#include <google/protobuf/util/json_util.h>         // IWYU pragma: keep
+#include <memory>                                   // for unique_ptr
+#include <string>                                   // for string, operator==
+#include <utility>                                  // for move
+#include <vector>                                   // for vector
 
 #define LIST_VALUE_CONSTRUCTOR(TYPE, SETTER)                                                       \
   explicit inline Value(const std::vector<TYPE> &values) : ProtoMessageWrapper<ProtoValue>() {     \
@@ -47,8 +51,6 @@ using ProtoScalarValue = caosdb::entity::v1alpha1::ScalarValue;
 using ValueCase = caosdb::entity::v1alpha1::Value::ValueCase;
 using ScalarValueCase = caosdb::entity::v1alpha1::ScalarValue::ScalarValueCase;
 
-class Entity;
-class Property;
 class ScalarValue;
 class Value;
 
@@ -60,12 +62,15 @@ enum SpecialValue {
   EMPTY_STRING = ProtoSpecialValue::SPECIAL_VALUE_EMPTY_STRING,
 };
 
+/**
+ * Pure abstract base class for values.
+ */
 class AbstractValue {
 public:
   /**
-   * Virtual destructor.
+   * Pure virtual destructor.
    */
-  virtual ~AbstractValue(){};
+  virtual ~AbstractValue() = 0;
   /**
    * Return true iff the value is a NULL value (NULL in the CaosDB sense).
    */
@@ -163,6 +168,8 @@ protected:
   [[nodiscard]] virtual auto GetProtoValue() const noexcept -> const ProtoValue * = 0;
 };
 
+inline AbstractValue::~AbstractValue() {}
+
 class ScalarValue : public AbstractValue, public ProtoMessageWrapper<ProtoScalarValue> {
 public:
   /**
@@ -198,7 +205,8 @@ public:
     return *this;
   }
 
-  inline ScalarValue(ProtoScalarValue *wrapped) : ProtoMessageWrapper<ProtoScalarValue>(wrapped) {}
+  inline ScalarValue(ProtoScalarValue *wrapped)
+    : ProtoMessageWrapper<ProtoScalarValue>(wrapped), proto_value(nullptr) {}
 
   [[nodiscard]] inline auto IsNull() const noexcept -> bool {
     return (this->wrapped->scalar_value_case() == ScalarValueCase::kSpecialValue &&
@@ -252,7 +260,7 @@ protected:
     }
     return this->proto_value;
   };
-  inline ScalarValue() : ProtoMessageWrapper<ProtoScalarValue>() {}
+  inline ScalarValue() : ProtoMessageWrapper<ProtoScalarValue>(), proto_value(nullptr) {}
 
 private:
   mutable ProtoValue *proto_value;

src/caosdb/entity.cpp

@@ -109,6 +109,9 @@ auto Property::SetImportance(Importance importance) -> void {
 
 auto Property::SetValue(const Value &value) -> StatusCode { return this->value.CopyFrom(value); }
 
+// auto Property::SetValue(const AbstractValue &value) -> StatusCode { return
+// SetValue(Value(value)); }
+
 auto Property::SetValue(const std::string &value) -> StatusCode { return SetValue(Value(value)); }
 
 auto Property::SetValue(const char *value) -> StatusCode { return SetValue(Value(value)); }
@@ -203,6 +206,9 @@ auto Entity::SetValue(const Value &value) -> StatusCode {
   return this->value.CopyFrom(value);
 }
 
+// auto Entity::SetValue(const AbstractValue &value) -> StatusCode { return SetValue(Value(value));
+// }
+
 auto Entity::SetValue(const std::string &value) -> StatusCode { return SetValue(Value(value)); }
 
 auto Entity::SetValue(const char *value) -> StatusCode { return SetValue(Value(value)); }

test/CMakeLists.txt

@@ -61,6 +61,7 @@ foreach (i RANGE "${len_test_cases}")
     target_include_directories(${test_case_name}
       PUBLIC ${CMAKE_CURRENT_BINARY_DIR})
     if(_LINTING)
+        message(STATUS "linting for tests: ${_CMAKE_CXX_INCLUDE_WHAT_YOU_USE}")
         set_target_properties(${test_case_name}
             PROPERTIES
             CXX_CLANG_TIDY "${_CMAKE_CXX_CLANG_TIDY};${_CMAKE_CXX_CLANG_TIDY_TEST_CHECKS}"

test/test_entity.cpp

@@ -80,7 +80,7 @@ TEST(test_entity, test_property_setters) {
   EXPECT_EQ(prop.GetId(), "prop_id");
   EXPECT_EQ(prop.GetImportance(), Importance::OBLIGATORY);
   EXPECT_TRUE(prop.GetValue().IsString());
-  EXPECT_EQ(prop.GetValue().AsString(), "prop_value");
+  EXPECT_EQ(prop.GetValue().GetAsString(), "prop_value");
   EXPECT_EQ(prop.GetUnit(), "prop_unit");
   EXPECT_TRUE(prop.GetDataType().IsReference());
   EXPECT_EQ(prop.GetDataType().AsReference().GetName(), "prop_dtype");
@@ -204,7 +204,7 @@ TEST(test_entity, test_insert_with_role) {
   EXPECT_EQ(entity.GetName(), "Length");
   EXPECT_EQ(entity.GetUnit(), "m");
   EXPECT_TRUE(entity.GetValue().IsDouble());
-  EXPECT_DOUBLE_EQ(entity.GetValue().AsDouble(), 5.5);
+  EXPECT_DOUBLE_EQ(entity.GetValue().GetAsDouble(), 5.5);
 }
 
 TEST(test_entity, test_insert_with_parent) {

test/test_list_properties.cpp

@@ -55,10 +55,10 @@ TEST(test_list_property, test_list_of_text) {
   EXPECT_TRUE(data_type.AsList().IsListOfAtomic());
   EXPECT_EQ(data_type.AsList().GetAtomicDataType(), AtomicDataType::TEXT);
 
-  EXPECT_TRUE(value.IsList());
-  EXPECT_EQ(value.AsList().size(), 3);
-  EXPECT_TRUE(value.AsList().at(1).IsString());
-  EXPECT_EQ(value.AsList().at(1).AsString(), "item2");
+  EXPECT_TRUE(value.IsVector());
+  EXPECT_EQ(value.GetAsVector().size(), 3);
+  EXPECT_TRUE(value.GetAsVector().at(1).IsString());
+  EXPECT_EQ(value.GetAsVector().at(1).GetAsString(), "item2");
 }
 
 } // namespace caosdb::entity

test/test_value.cpp

@@ -42,9 +42,9 @@ TEST(test_value, test_null) {
   EXPECT_TRUE(value.IsNull());
   EXPECT_FALSE(value.IsDouble());
   EXPECT_FALSE(value.IsBool());
-  EXPECT_FALSE(value.IsInteger());
+  EXPECT_FALSE(value.IsInt64());
   EXPECT_FALSE(value.IsString());
-  EXPECT_FALSE(value.IsList());
+  EXPECT_FALSE(value.IsVector());
 }
 
 TEST(test_value, test_string) {
@@ -53,18 +53,18 @@ TEST(test_value, test_string) {
   EXPECT_TRUE(value.IsString());
   EXPECT_FALSE(value.IsDouble());
   EXPECT_FALSE(value.IsBool());
-  EXPECT_FALSE(value.IsInteger());
+  EXPECT_FALSE(value.IsInt64());
 
-  EXPECT_EQ(value.AsString(), "test");
+  EXPECT_EQ(value.GetAsString(), "test");
 
   Value empty_string(std::string(""));
   EXPECT_FALSE(empty_string.IsNull());
   EXPECT_TRUE(empty_string.IsString());
   EXPECT_FALSE(empty_string.IsDouble());
   EXPECT_FALSE(empty_string.IsBool());
-  EXPECT_FALSE(empty_string.IsInteger());
+  EXPECT_FALSE(empty_string.IsInt64());
 
-  EXPECT_EQ(empty_string.AsString(), "");
+  EXPECT_EQ(empty_string.GetAsString(), "");
 
   // Test inequality
   Value string1("1");
@@ -78,18 +78,18 @@ TEST(test_value, test_double) {
   EXPECT_FALSE(value.IsString());
   EXPECT_TRUE(value.IsDouble());
   EXPECT_FALSE(value.IsBool());
-  EXPECT_FALSE(value.IsInteger());
+  EXPECT_FALSE(value.IsInt64());
 
-  EXPECT_EQ(value.AsDouble(), 2.26);
+  EXPECT_EQ(value.GetAsDouble(), 2.26);
 
   Value nan(std::sqrt(-1.0));
   EXPECT_FALSE(nan.IsNull());
   EXPECT_FALSE(nan.IsString());
   EXPECT_TRUE(nan.IsDouble());
   EXPECT_FALSE(nan.IsBool());
-  EXPECT_FALSE(nan.IsInteger());
+  EXPECT_FALSE(nan.IsInt64());
 
-  EXPECT_TRUE(std::isnan(nan.AsDouble()));
+  EXPECT_TRUE(std::isnan(nan.GetAsDouble()));
 }
 
 TEST(test_value, test_integer) {
@@ -98,9 +98,9 @@ TEST(test_value, test_integer) {
   EXPECT_FALSE(value.IsString());
   EXPECT_FALSE(value.IsDouble());
   EXPECT_FALSE(value.IsBool());
-  EXPECT_TRUE(value.IsInteger());
+  EXPECT_TRUE(value.IsInt64());
 
-  EXPECT_EQ(value.AsInteger(), 1337);
+  EXPECT_EQ(value.GetAsInt64(), 1337);
 }
 
 TEST(test_value, test_boolean) {
@@ -109,9 +109,9 @@ TEST(test_value, test_boolean) {
   EXPECT_FALSE(value.IsString());
   EXPECT_FALSE(value.IsDouble());
   EXPECT_TRUE(value.IsBool());
-  EXPECT_FALSE(value.IsInteger());
+  EXPECT_FALSE(value.IsInt64());
 
-  EXPECT_EQ(value.AsBool(), true);
+  EXPECT_EQ(value.GetAsBool(), true);
 }
 
 TEST(test_value, test_list) {
@@ -122,17 +122,29 @@ TEST(test_value, test_list) {
   Value value(ids);
 
   EXPECT_FALSE(value.IsNull());
-  EXPECT_TRUE(value.IsList());
+  EXPECT_TRUE(value.IsVector());
   EXPECT_FALSE(value.IsString());
   EXPECT_FALSE(value.IsDouble());
   EXPECT_FALSE(value.IsBool());
-  EXPECT_FALSE(value.IsInteger());
+  EXPECT_FALSE(value.IsInt64());
 
-  auto list_value = value.AsList();
+  auto list_value = value.GetAsVector();
   int counter = 0;
-  for (auto item : list_value) {
+  for (const auto &item : list_value) {
     EXPECT_EQ(item.IsString(), true);
-    EXPECT_EQ(item.AsString(), "id" + std::to_string(counter++));
+    EXPECT_EQ(item.GetAsString(), "id" + std::to_string(counter++));
   }
 }
+
+TEST(test_value, test_scalar_value_to_value) {
+  ProtoScalarValue proto_scalar_value;
+  proto_scalar_value.set_integer_value(5);
+  ScalarValue scalar_value(&proto_scalar_value);
+  Value value(scalar_value);
+
+  EXPECT_TRUE(scalar_value.IsInt64());
+  EXPECT_TRUE(value.IsInt64());
+  EXPECT_EQ(scalar_value.GetAsInt64(), value.GetAsInt64());
+}
+
 } // namespace caosdb::entity