From 9a8a03b37dea8c4ffe749c1ba5943b0c64bcc1da Mon Sep 17 00:00:00 2001
From: Sergey Yakubov <sergey.yakubov@desy.de>
Date: Fri, 4 Oct 2019 14:41:31 +0200
Subject: [PATCH] fix

---
 common/cpp/src/database/mongodb_client.cpp    | 39 +++++++----
 common/cpp/src/database/mongodb_client.h      |  2 +-
 tests/automatic/mongo_db/CMakeLists.txt       |  1 +
 .../mongo_db/insert_dataset/CMakeLists.txt    | 21 ++++++
 .../mongo_db/insert_dataset/cleanup_linux.sh  |  5 ++
 .../insert_dataset/cleanup_windows.bat        |  4 ++
 .../insert_dataset/insert_dataset_mongodb.cpp | 69 +++++++++++++++++++
 7 files changed, 128 insertions(+), 13 deletions(-)
 create mode 100644 tests/automatic/mongo_db/insert_dataset/CMakeLists.txt
 create mode 100644 tests/automatic/mongo_db/insert_dataset/cleanup_linux.sh
 create mode 100644 tests/automatic/mongo_db/insert_dataset/cleanup_windows.bat
 create mode 100644 tests/automatic/mongo_db/insert_dataset/insert_dataset_mongodb.cpp

diff --git a/common/cpp/src/database/mongodb_client.cpp b/common/cpp/src/database/mongodb_client.cpp
index 9d8b919d4..63f69cd3f 100644
--- a/common/cpp/src/database/mongodb_client.cpp
+++ b/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);
diff --git a/common/cpp/src/database/mongodb_client.h b/common/cpp/src/database/mongodb_client.h
index 82266de6e..6537151c6 100644
--- a/common/cpp/src/database/mongodb_client.h
+++ b/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;
 };
 
 }
diff --git a/tests/automatic/mongo_db/CMakeLists.txt b/tests/automatic/mongo_db/CMakeLists.txt
index 279e95393..1fcf5373d 100644
--- a/tests/automatic/mongo_db/CMakeLists.txt
+++ b/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)
 
diff --git a/tests/automatic/mongo_db/insert_dataset/CMakeLists.txt b/tests/automatic/mongo_db/insert_dataset/CMakeLists.txt
new file mode 100644
index 000000000..a7c1d9648
--- /dev/null
+++ b/tests/automatic/mongo_db/insert_dataset/CMakeLists.txt
@@ -0,0 +1,21 @@
+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")
+
+
diff --git a/tests/automatic/mongo_db/insert_dataset/cleanup_linux.sh b/tests/automatic/mongo_db/insert_dataset/cleanup_linux.sh
new file mode 100644
index 000000000..adf03ce95
--- /dev/null
+++ b/tests/automatic/mongo_db/insert_dataset/cleanup_linux.sh
@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+
+database_name=data
+
+echo "db.dropDatabase()" | mongo ${database_name}
diff --git a/tests/automatic/mongo_db/insert_dataset/cleanup_windows.bat b/tests/automatic/mongo_db/insert_dataset/cleanup_windows.bat
new file mode 100644
index 000000000..fd3321842
--- /dev/null
+++ b/tests/automatic/mongo_db/insert_dataset/cleanup_windows.bat
@@ -0,0 +1,4 @@
+SET database_name=data
+SET mongo_exe="c:\Program Files\MongoDB\Server\3.6\bin\mongo.exe"
+
+echo db.dropDatabase() | %mongo_exe% %database_name%
diff --git a/tests/automatic/mongo_db/insert_dataset/insert_dataset_mongodb.cpp b/tests/automatic/mongo_db/insert_dataset/insert_dataset_mongodb.cpp
new file mode 100644
index 000000000..2d595be84
--- /dev/null
+++ b/tests/automatic/mongo_db/insert_dataset/insert_dataset_mongodb.cpp
@@ -0,0 +1,69 @@
+#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;
+}
-- 
GitLab