Merge pull request #57 in ASAPO/asapo from bugfix_subset-can-be-added-twice to develop

* commit '9a8a03b3':
  fix

common/cpp/src/database/mongodb_client.cpp

@@ -198,6 +198,31 @@ Error MongoDBClient::Upsert(uint64_t id, const uint8_t* data, uint64_t size) con
     return UpdateBsonDocument(id, document, true);
 
 }
+
+
+Error MongoDBClient::AddBsonDocumentToArray(bson_t* query, bson_t* update, bool ignore_duplicates) const {
+    Error err;
+    bson_error_t mongo_err;
+// first update may fail due to multiple threads try to create document at once, the second one should succeed
+// https://jira.mongodb.org/browse/SERVER-14322
+    if (!mongoc_collection_update (collection_, MONGOC_UPDATE_UPSERT, query, update, NULL, &mongo_err)) {
+        if (mongo_err.code == MONGOC_ERROR_DUPLICATE_KEY) {
+            if (!mongoc_collection_update (collection_, MONGOC_UPDATE_UPSERT, query, update, NULL, &mongo_err)) {
+                if (mongo_err.code == MONGOC_ERROR_DUPLICATE_KEY) {
+                    err =  ignore_duplicates ? nullptr : DBErrorTemplates::kDuplicateID.Generate();
+                } else {
+                    err = DBErrorTemplates::kInsertError.Generate(mongo_err.message);
+                }
+            }
+        } else {
+            err = DBErrorTemplates::kInsertError.Generate(mongo_err.message);
+        }
+    }
+    return err;
+}
+
+
+
 Error MongoDBClient::InsertAsSubset(const FileInfo& file,
                                     uint64_t subset_id,
                                     uint64_t subset_size,
@@ -211,24 +236,14 @@ Error MongoDBClient::InsertAsSubset(const FileInfo& file,
     if (err) {
         return err;
     }
-    auto query = BCON_NEW ("_id", BCON_INT64(subset_id));
+    auto query = BCON_NEW ("$and","[","{","_id", BCON_INT64(subset_id),"}","{","images._id","{","$ne",BCON_INT64(file.id),"}","}","]");
     auto update = BCON_NEW ("$setOnInsert", "{",
                             "size", BCON_INT64 (subset_size),
                             "}",
                             "$addToSet", "{",
                             "images", BCON_DOCUMENT(document.get()), "}");
 
-
-    bson_error_t mongo_err;
-    if (!mongoc_collection_update (collection_, MONGOC_UPDATE_UPSERT, query, update, NULL, &mongo_err)) {
-        if (mongo_err.code == MONGOC_ERROR_DUPLICATE_KEY) {
-            if (!mongoc_collection_update (collection_, MONGOC_UPDATE_NONE, query, update, NULL, &mongo_err)) {
-                err = DBErrorTemplates::kInsertError.Generate(mongo_err.message);
-            }
-        } else {
-            err = DBErrorTemplates::kInsertError.Generate(mongo_err.message);
-        }
-    }
+    err = AddBsonDocumentToArray(query, update,ignore_duplicates);
 
     bson_destroy (query);
     bson_destroy (update);

common/cpp/src/database/mongodb_client.h

@@ -57,7 +57,7 @@ class MongoDBClient final : public Database {
     Error TryConnectDatabase();
     Error InsertBsonDocument(const bson_p& document, bool ignore_duplicates) const;
     Error UpdateBsonDocument(uint64_t id, const bson_p& document, bool upsert) const;
-
+    Error AddBsonDocumentToArray(bson_t* query, bson_t* update, bool ignore_duplicates) const;
 };
 
 }

tests/automatic/mongo_db/CMakeLists.txt

@@ -2,5 +2,6 @@ CMAKE_MINIMUM_REQUIRED(VERSION 3.7) # needed for fixtures
 
 add_subdirectory(connect)
 add_subdirectory(insert)
+add_subdirectory(insert_dataset)
 add_subdirectory(upsert)
 

tests/automatic/mongo_db/insert_dataset/CMakeLists.txt

+set(TARGET_NAME insert_dataset_mongodb)
+set(SOURCE_FILES insert_dataset_mongodb.cpp)
+
+
+################################
+# Executable and link
+################################
+add_executable(${TARGET_NAME} ${SOURCE_FILES})
+target_link_libraries(${TARGET_NAME} test_common database)
+target_include_directories(${TARGET_NAME} PUBLIC ${ASAPO_CXX_COMMON_INCLUDE_DIR})
+
+################################
+# Testing
+################################
+add_test_cleanup(${TARGET_NAME})
+add_integration_test(${TARGET_NAME} insertOK "OK 1" "OK 2")
+add_integration_test(${TARGET_NAME} insertFailsForDuplicateID
+        "DuplicateID 6"
+        "DuplicateID 5")
+
+

tests/automatic/mongo_db/insert_dataset/cleanup_linux.sh

+#!/usr/bin/env bash
+
+database_name=data
+
+echo "db.dropDatabase()" | mongo ${database_name}

tests/automatic/mongo_db/insert_dataset/cleanup_windows.bat

+SET database_name=data
+SET mongo_exe="c:\Program Files\MongoDB\Server\3.6\bin\mongo.exe"
+
+echo db.dropDatabase() | %mongo_exe% %database_name%

tests/automatic/mongo_db/insert_dataset/insert_dataset_mongodb.cpp

+#include <iostream>
+#include <chrono>
+
+#include "../../../common/cpp/src/database/mongodb_client.h"
+#include "testing.h"
+
+
+using asapo::M_AssertContains;
+using asapo::Error;
+
+
+void Assert(const Error& error, const std::string& expect) {
+    std::string result;
+    if (error == nullptr) {
+        result = "OK";
+    } else {
+        result = error->Explain();
+    }
+    M_AssertContains(result, expect);
+}
+
+struct Args {
+    std::string keyword;
+    int file_id;
+};
+
+Args GetArgs(int argc, char* argv[]) {
+    if (argc != 3) {
+        std::cout << "Wrong number of arguments" << std::endl;
+        exit(EXIT_FAILURE);
+    }
+    return Args{argv[1], atoi(argv[2])};
+}
+
+
+int main(int argc, char* argv[]) {
+    auto args = GetArgs(argc, argv);
+    asapo::MongoDBClient db;
+
+    asapo::FileInfo fi;
+    fi.size = 100;
+    fi.name = "relpath/1";
+    uint64_t subset_id = args.file_id;
+    fi.modify_date = std::chrono::system_clock::now();
+    fi.buf_id = 18446744073709551615ull;
+    fi.source = "host:1234";
+    fi.id = 1;
+
+    uint64_t subset_size=2;
+
+    if (args.keyword != "Notconnected") {
+        db.Connect("127.0.0.1", "data", "test");
+    }
+
+    auto err =  db.InsertAsSubset(fi, subset_id, subset_size, true);
+
+
+    if (args.keyword == "DuplicateID") {
+        Assert(err, "OK");
+        fi.id = 2;
+        err =  db.InsertAsSubset(fi, subset_id, subset_size, true);
+//        Assert(err, "OK");
+        err =  db.InsertAsSubset(fi, subset_id, subset_size, false);
+    }
+
+    Assert(err, args.keyword);
+
+    return 0;
+}