Merge branch 'f-consolidation' of...

Merge branch 'f-consolidation' of gitlab.indiscale.com:caosdb/src/caosdb-cpplib into f-consolidation

CMakeLists.txt

@@ -38,6 +38,7 @@ set(CMAKE_C_EXTENSIONS OFF)
 set(CMAKE_C_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
 
 set(CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake" ${CMAKE_MODULE_PATH})
 
@@ -411,7 +412,7 @@ install(FILES ${PROJECT_SOURCE_DIR}/caosdbConfigVersion.cmake
 ### code formatting with clang-format
 #######################################################
 option(AUTOFORMATTING "call clang-format at configure time" ON)
-if(AUTOFORMATTING)
+if(AUTOFORMATTING AND NOT SKIP_LINTING)
     file(GLOB format_test_sources test/*.cpp test/*.h test/*.h.in)
     execute_process(COMMAND clang-format -i --verbose ${libcaosdb_INCL}
         ${libcaosdb_SRC} ${libcaosdb_TEST_SRC}

README_SETUP.md

@@ -77,6 +77,8 @@ For the tests there is a slightly different setup required (with option `-D CMAK
 1. `mkdir build && cd build/`
 2. `conan install .. -s "compiler.libcxx=libstdc++11"`
 3. `cmake -B . -D CMAKE_BUILD_TYPE=Debug ..`
+   * If your clang-format version is too old, formatting, linting etc. can be skipped:  
+     `cmake -B . -D CMAKE_BUILD_TYPE=Debug -D SKIP_LINTING=ON ..`
 4. `cmake --build .`
 
 ### Run
@@ -116,6 +118,7 @@ Please adhere to [Google's C++ naming conventions](https://google.github.io/styl
 You can use a json file for the configuration of the client. See
 `test/test_data/test_caosdb_client.json` for an example. You may use
 `caosdb-client-configuration-schema.json` to validate your schema.
+Typically, you will need to provide the path to your SSL certificate.
 
 The client will load the configuration file from the first existing
 file in the following locations (precedence from highest to lowest):

include/CMakeLists.txt

@@ -42,6 +42,8 @@ set(libcaosdb_INCL
     ${CMAKE_CURRENT_SOURCE_DIR}/caosdb/utility.h
     ${CMAKE_CURRENT_SOURCE_DIR}/caosdb/value.h
     ${CMAKE_CURRENT_SOURCE_DIR}/caosdb/file_transmission/register_file_upload_handler.h
+	# TODO this  file is still missing
+	# ${CMAKE_CURRENT_SOURCE_DIR}/caosdb/file_transmission/register_file_download_handler.h
     ${CMAKE_CURRENT_SOURCE_DIR}/caosdb/file_transmission/upload_request_handler.h
     ${CMAKE_CURRENT_SOURCE_DIR}/caosdb/file_transmission/download_request_handler.h
     ${CMAKE_CURRENT_SOURCE_DIR}/caosdb/file_transmission/file_writer.h

include/caosdb/data_type.h

@@ -19,6 +19,13 @@
  *
  */
 
+/**
+ * DataTypes have 2 dimensions: They may be atomic or reference typed, and they
+ * may be scalar or list valued.  If they are atomic, they have an
+ * AtomicDataType.  If they are reference typed, the reference name can be
+ * obtained with GetName().
+ */
+
 #ifndef CAOSDB_DATA_TYPE_H
 #define CAOSDB_DATA_TYPE_H
 #include "caosdb/protobuf_helper.h"         // for ProtoMessageWrapper

include/caosdb/entity.h

@@ -67,6 +67,7 @@ using ProtoImportance = caosdb::entity::v1alpha1::Importance;
 using caosdb::StatusCode;
 using caosdb::entity::v1alpha1::EntityResponse;
 using caosdb::entity::v1alpha1::FileTransmissionId;
+using ::google::protobuf::RepeatedPtrField;
 using google::protobuf::RepeatedPtrField;
 
 static const std::string logger_name = "caosdb::entity";
@@ -179,7 +180,7 @@ protected:
   /**
    * Remove the element at the given index.
    */
-  inline auto remove(int index) -> void {
+  inline auto Remove(int index) -> void {
     this->wrapped->DeleteSubrange(index, 1);
     if (cache.count(index) > 0) {
       cache.erase(index);
@@ -204,6 +205,7 @@ private:
   public:
     explicit iterator(const RepeatedPtrFieldWrapper<T, P> *instance,
                       int index = 0);
+    // TODO(henrik) add unit tests
     auto operator*() const -> T &;
     auto operator++() -> iterator &;
     auto operator++(int) -> iterator;
@@ -565,6 +567,16 @@ private:
  * Container for Properties of Entities.
  *
  * Should only be instantiated and write-accessed by the owning entity.
+ *
+ * Note that iterating over the Property contents only works via references,
+ * since the Property copy constructor is deliberately disabled:
+ *
+ * \code
+ * // Accessing single properties as reference
+ * auto &property = my_properties.at(0);
+ * // Iterating via reference
+ * for (auto &property : my_properties) {...}
+ * \endcode
  */
 class Properties
   : public RepeatedPtrFieldWrapper<Property,
@@ -651,6 +663,8 @@ public:
   };
 
   [[nodiscard]] auto GetParents() const -> const Parents &;
+  // TODO(henrik) const prevents properties from being changed
+  // what about an interface that operates on the list directly?
   [[nodiscard]] auto GetProperties() const -> const Properties &;
   [[nodiscard]] inline auto GetErrors() const -> const Messages & {
     return errors;

include/caosdb/file_transmission/download_request_handler.h

@@ -68,6 +68,9 @@ using caosdb::entity::v1alpha1::FileTransmissionService;
 using caosdb::transaction::HandlerInterface;
 using caosdb::transaction::HandlerTag;
 
+/*
+ * Handler for the download request of a single file
+ */
 class DownloadRequestHandler final : public HandlerInterface {
 public:
   DownloadRequestHandler(HandlerTag tag, FileTransmissionService::Stub *stub,

include/caosdb/handler_interface.h

@@ -56,7 +56,14 @@
 namespace caosdb::transaction {
 
 const static std::string logger_name = "caosdb::transaction";
-
+/*
+ * Baseclass for UnaryRpcHandler, DownloadRequestHandler and
+ * UploadRequestHandler
+ *
+ * It handles a request: Its status is contained in the transaction_status
+ * member variable and the functions Start, OnNext and Cancel need to be
+ * overwritten by child classes.
+ */
 class HandlerInterface {
 public:
   HandlerInterface() : transaction_status(TransactionStatus::READY()) {}
@@ -65,6 +72,12 @@ public:
 
   virtual void Start() = 0;
 
+  /*
+   * ok indicates whether the current request is in a good state or not. If
+   * ok is false, the request will be ended.
+   *
+   * returns false if the handler is done
+   */
   virtual bool OnNext(bool ok) = 0;
 
   virtual void Cancel() = 0;

include/caosdb/transaction.h

@@ -433,6 +433,8 @@ public:
 protected:
   /**
    * Await and process the current handler's results.
+   *
+   * This implies consecutive calls to the handler's OnNext function.
    */
   auto ProcessCalls() -> TransactionStatus;
 

src/caosdb/entity.cpp

@@ -190,7 +190,7 @@ auto Entity::AppendParent(const Parent &parent) -> void {
   this->parents.Append(parent);
 }
 
-auto Entity::RemoveParent(int index) -> void { this->parents.remove(index); }
+auto Entity::RemoveParent(int index) -> void { this->parents.Remove(index); }
 
 [[nodiscard]] auto Entity::GetProperties() const -> const Properties & {
   return properties;
@@ -201,7 +201,7 @@ auto Entity::AppendProperty(const Property &property) -> void {
 }
 
 auto Entity::RemoveProperty(int index) -> void {
-  this->properties.remove(index);
+  this->properties.Remove(index);
 }
 
 auto Entity::CreateProtoEntity() -> ProtoEntity * {

src/caosdb/file_transmission/file_reader.cpp

@@ -77,7 +77,8 @@ std::size_t FileReader::read(std::string &buffer) {
   if (!stream_.eof()) {
     auto bufferSize = buffer.size();
     if (bufferSize > 0) {
-      if (!stream_.read(&buffer[0], bufferSize)) {
+      // TODO(henrik): fix nolint
+      if (!stream_.read(&buffer[0], bufferSize)) { // NOLINT
         throw FileIOError("Can't read file: " + filename_.string());
       }
 

src/caosdb/file_transmission/file_writer.cpp

@@ -68,7 +68,8 @@ void FileWriter::openFile() {
 void FileWriter::write(const std::string &buffer) {
   auto bufferSize = buffer.size();
   if (bufferSize > 0) {
-    if (!stream_.write(buffer.data(), bufferSize)) {
+    // TODO(henrik): fix nolint
+    if (!stream_.write(buffer.data(), bufferSize)) { // NOLINT
       throw FileIOError("Can't write file: " + filename_.string());
     }
   }

test/test_data_type.cpp

@@ -48,6 +48,7 @@ TEST(test_data_type, test_atomic) {
   for (int i = 1; i < 6; i++) {
     Entity entity;
     entity.SetRole(Role::PROPERTY);
+    // the different AtomicDataType are associated with integers
     entity.SetDataType(static_cast<AtomicDataType>(i));
     EXPECT_TRUE(entity.GetDataType().IsAtomic());
     EXPECT_EQ(entity.GetDataType().AsAtomic(), static_cast<AtomicDataType>(i));
@@ -57,6 +58,7 @@ TEST(test_data_type, test_atomic) {
     entity.SetDataType(data_type);
 
     EXPECT_FALSE(data_type.IsReference());
+    EXPECT_EQ(data_type.AsReference().GetName(), std::basic_string<char>(""));
     EXPECT_FALSE(data_type.IsList());
     EXPECT_TRUE(data_type.IsAtomic());
     EXPECT_EQ(data_type.AsAtomic(), static_cast<AtomicDataType>(i));
@@ -95,6 +97,8 @@ TEST(test_data_type, test_list_of_atomic) {
     EXPECT_FALSE(data_type.IsAtomic());
     EXPECT_TRUE(data_type.IsList());
     const auto &list_data_type = data_type.AsList();
+    EXPECT_EQ(list_data_type.GetReferenceDataType().GetName(),
+              std::basic_string<char>(""));
     EXPECT_TRUE(list_data_type.IsListOfAtomic());
     EXPECT_FALSE(list_data_type.IsListOfReference());
     EXPECT_EQ(list_data_type.GetAtomicDataType(),

test/test_entity.cpp

@@ -30,12 +30,14 @@
 #include "caosdb/status_code.h"                  // for StatusCode, FILE_DO...
 #include "caosdb/transaction.h"                  // for Transaction
 #include "caosdb/value.h"                        // for Value
+#include <exception>
 #include <google/protobuf/arena.h> // for Arena
 #include <gtest/gtest-message.h>   // for Message
 #include <gtest/gtest-test-part.h> // for TestPartResult, Sui...
 #include <gtest/gtest_pred_impl.h> // for Test, EXPECT_EQ
 #include <iostream>
 #include <memory> // for allocator, shared_ptr
+#include <stdexcept>
 #include <string> // for operator+, string
 
 namespace caosdb::entity {
@@ -100,7 +102,10 @@ TEST(test_entity, test_append_property) {
   entity.AppendProperty(prop);
   EXPECT_EQ(entity.GetProperties().size(), 1);
 
+  // also test RepeatedPtrFieldWrapper.at()
   const auto &same_prop = entity.GetProperties().at(0);
+  EXPECT_THROW((void)entity.GetProperties().at(2), std::out_of_range);
+  EXPECT_THROW((void)entity.GetProperties().at(-1), std::out_of_range);
 
   EXPECT_EQ(prop.GetName(), same_prop.GetName());
   EXPECT_EQ(prop.GetId(), same_prop.GetId());
@@ -393,8 +398,11 @@ TEST(test_entity, test_property_iterator) {
   ASSERT_EQ(entity.GetProperties().size(), 5);
   int counter = 0;
   for (auto &property : entity.GetProperties()) {
+    // TODO(tf) Copy constructor was deleted
+    // auto nonref_property = entity.GetProperties().at(counter);
     auto name = "PROPERTY-" + std::to_string(counter);
     EXPECT_EQ(property.GetName(), name);
+    // EXPECT_EQ(nonref_property.GetName(), name);
     counter++;
   }
   EXPECT_EQ(counter, 5);