From 34d626b31b198d7f664588515d9478e3643bbfca Mon Sep 17 00:00:00 2001
From: Matthew McGill <mhollismcgill@gmail.com>
Date: Sat, 6 Oct 2018 17:06:57 +0000
Subject: [PATCH] Minor changes

Former-commit-id: acd722f5b52b24b969c4e52c5c1e23f9c8878bc7
---
 libIRDB/include/util/IRDB_Objects.hpp |  8 ++-
 libIRDB/src/util/IRDB_Objects.cpp     | 88 ++++++++++++++++-----------
 2 files changed, 60 insertions(+), 36 deletions(-)

diff --git a/libIRDB/include/util/IRDB_Objects.hpp b/libIRDB/include/util/IRDB_Objects.hpp
index da2a6e8dc..a8df3297e 100644
--- a/libIRDB/include/util/IRDB_Objects.hpp
+++ b/libIRDB/include/util/IRDB_Objects.hpp
@@ -1,5 +1,5 @@
-#ifndef IRDBObjects_h
-#define IRDBObjects_h
+#ifndef IRDB_Objects_h
+#define IRDB_Objects_h
 
 #include <map>
 #include <utility>
@@ -63,6 +63,10 @@ class IRDBObjects_t
 		std::map<db_id_t, std::shared_ptr<VariantID_t>> variant_map;
                 // maps file id to (file, file ir)
 		std::map<db_id_t, std::pair<std::shared_ptr<File_t>, std::shared_ptr<FileIR_t>>> file_IR_map;
+                
+                // minor helpers (used to check assertions)
+                bool FilesAlreadyPresent(std::set<File_t*> the_files);
+                bool FilesBeingShared(std::shared_ptr<VariantID_t> the_variant);
 };
 
 #endif
diff --git a/libIRDB/src/util/IRDB_Objects.cpp b/libIRDB/src/util/IRDB_Objects.cpp
index fccb82458..9ac100435 100644
--- a/libIRDB/src/util/IRDB_Objects.cpp
+++ b/libIRDB/src/util/IRDB_Objects.cpp
@@ -53,11 +53,7 @@ int IRDBObjects_t::DeleteFileIR(db_id_t file_id, bool write_to_DB)
         map<db_id_t, pair<shared_ptr<File_t>, shared_ptr<FileIR_t>>>::iterator 
             it = file_IR_map.find(file_id);
         
-        if(it == file_IR_map.end())
-        {
-            ret_status = 1;
-        }
-        else
+        if(it != file_IR_map.end())
         {
             if(it->second.second != NULL)
             {
@@ -76,12 +72,12 @@ int IRDBObjects_t::DeleteFileIR(db_id_t file_id, bool write_to_DB)
                     catch (DatabaseError_t pnide)
                     {
                         cerr << "Unexpected database error: " << pnide << "file url: " << the_file->GetURL() << endl;
-                        ret_status = 2;
+                        ret_status = 1;
                     }
                     catch (...)
                     {
                         cerr << "Unexpected error file url: " << the_file->GetURL() << endl;
-                        ret_status = 2;
+                        ret_status = 1;
                     }
                 }
                 (it->second.second).reset();
@@ -92,10 +88,31 @@ int IRDBObjects_t::DeleteFileIR(db_id_t file_id, bool write_to_DB)
 }
 
 
+bool IRDBObjects_t::FilesAlreadyPresent(set<File_t*> the_files)
+{
+        for(set<File_t*>::iterator it=the_files.begin();
+            it!=the_files.end();
+            ++it
+           )
+        {       
+            if(GetFile((*it)->GetBaseID()) != NULL)
+            {
+                return true;
+            }
+        }
+        
+        return false;
+}
+
+
 int IRDBObjects_t::AddVariant(db_id_t variant_id)
 {
-        shared_ptr<VariantID_t> the_variant = make_shared<VariantID_t>(variant_id);        
+        shared_ptr<VariantID_t> the_variant = make_shared<VariantID_t>(variant_id);      
+        
         assert(the_variant->IsRegistered()==true);
+        // disallow variants that share shallow copies to be read in simultaneously
+        // to prevent desynchronization. 
+        assert(!FilesAlreadyPresent(the_variant->GetFiles()));
         
         pair<db_id_t, shared_ptr<VariantID_t>> var_pair = make_pair(variant_id, the_variant);
         variant_map.insert(var_pair);
@@ -119,32 +136,35 @@ int IRDBObjects_t::AddVariant(db_id_t variant_id)
 }
 
 
+bool IRDBObjects_t::FilesBeingShared(shared_ptr<VariantID_t> the_variant)
+{
+        for(set<File_t*>::iterator file_it=the_variant->GetFiles().begin();
+                file_it!=the_variant->GetFiles().end();
+                ++file_it
+               )
+        {
+            assert(file_IR_map.find((*file_it)->GetBaseID()) != file_IR_map.end());
+            pair<shared_ptr<File_t>, shared_ptr<FileIR_t>> file_IR_pair = file_IR_map.at((*file_it)->GetBaseID());
+            if(!file_IR_pair.first.unique() || !file_IR_pair.second.unique())
+            {
+                return true;
+            }
+        }
+        
+        return false;
+}
+
+
 int IRDBObjects_t::DeleteVariant(db_id_t variant_id, bool write_to_DB)
 {
         int ret_status = 0;
         map<db_id_t, shared_ptr<VariantID_t>>::iterator var_it = variant_map.find(variant_id);
         
-        if(var_it == variant_map.end())
+        if(var_it != variant_map.end())
         {
-            ret_status = 1;
-        }
-        else
-        {
-            bool files_being_shared = false;
-            for(set<File_t*>::iterator file_it=var_it->second->GetFiles().begin();
-                file_it!=var_it->second->GetFiles().end();
-                ++file_it
-               )
-            {
-                assert(file_IR_map.find((*file_it)->GetBaseID()) != file_IR_map.end());
-                pair<shared_ptr<File_t>, shared_ptr<FileIR_t>> file_IR_pair = file_IR_map.at((*file_it)->GetBaseID());
-                if(!file_IR_pair.first.unique() || !file_IR_pair.second.unique())
-                {
-                    files_being_shared = true;
-                }
-            }
-            
-            assert(!files_being_shared);
+            // To prevent reading in the same files again while they are being used
+            // somewhere else, which could lead to desynchronization
+            assert(!FilesBeingShared(var_it->second));
             assert(var_it->second.unique());
             
             if(write_to_DB)
@@ -157,21 +177,21 @@ int IRDBObjects_t::DeleteVariant(db_id_t variant_id, bool write_to_DB)
                 catch (DatabaseError_t pnide)
                 {
                     cerr << "Unexpected database error: " << pnide << "variant ID: " << variant_id << endl;
-                    ret_status = 2;
+                    ret_status = 1;
                 }
                 catch (...)
                 {
                     cerr << "Unexpected error variant ID: " << variant_id << endl;
-                    ret_status = 2;
+                    ret_status = 1;
                 }
             }
             // remove files and file IRs
-            for(set<File_t*>::iterator file_it2=var_it->second->GetFiles().begin();
-                file_it2!=var_it->second->GetFiles().end();
-                ++file_it2
+            for(set<File_t*>::iterator file_it=var_it->second->GetFiles().begin();
+                file_it!=var_it->second->GetFiles().end();
+                ++file_it
                )
             {
-                file_IR_map.erase((*file_it2)->GetBaseID());
+                file_IR_map.erase((*file_it)->GetBaseID());
             }
             
             // remove variant
-- 
GitLab