Mercurial > hg > orthanc-databases

changeset 307:8de3a1ecac11

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
MySQL: Added missing calls to OrthancPluginDatabaseSignalDeletedResource()
author Sebastien Jodogne <s.jodogne@gmail.com>
date Mon, 12 Jul 2021 16:53:28 +0200
parents 544e0c943b40
children 6a668f5d1069
files Framework/Common/DatabaseManager.h Framework/Plugins/IndexBackend.cpp Framework/Plugins/IndexUnitTests.h MySQL/NEWS MySQL/Plugins/MySQLIndex.cpp
diffstat 5 files changed, 204 insertions(+), 26 deletions(-) [+]
line wrap: on
line diff
--- a/Framework/Common/DatabaseManager.h	Mon Jul 12 12:03:33 2021 +0200
+++ b/Framework/Common/DatabaseManager.h	Mon Jul 12 16:53:28 2021 +0200
@@ -185,6 +185,11 @@
       int64_t ReadInteger64(size_t field) const;
 
       std::string ReadString(size_t field) const;
+
+      void PrintResult(std::ostream& stream)
+      {
+        IResult::Print(stream, GetResult());
+      }
     };
 
 
--- a/Framework/Plugins/IndexBackend.cpp	Mon Jul 12 12:03:33 2021 +0200
+++ b/Framework/Plugins/IndexBackend.cpp	Mon Jul 12 16:53:28 2021 +0200
@@ -230,7 +230,7 @@
   {
     DatabaseManager::CachedStatement statement(
       STATEMENT_FROM_HERE, manager,
-      "SELECT * FROM DeletedResources");
+      "SELECT resourceType, publicId FROM DeletedResources");
 
     statement.SetReadOnly(true);
     statement.Execute();
--- a/Framework/Plugins/IndexUnitTests.h	Mon Jul 12 12:03:33 2021 +0200
+++ b/Framework/Plugins/IndexUnitTests.h	Mon Jul 12 16:53:28 2021 +0200
@@ -68,6 +68,11 @@
 static std::list<OrthancPluginDicomTag>  expectedDicomTags;
 static std::unique_ptr<OrthancPluginExportedResource>  expectedExported;
 
+static std::map<std::string, OrthancPluginResourceType> deletedResources;
+static std::unique_ptr< std::pair<std::string, OrthancPluginResourceType> > remainingAncestor;
+static std::set<std::string> deletedAttachments;
+
+
 static void CheckAttachment(const OrthancPluginAttachment& attachment)
 {
   ASSERT_STREQ(expectedAttachment->uuid, attachment.uuid);
@@ -148,6 +153,19 @@
           break;
         }
 
+        case _OrthancPluginDatabaseAnswerType_DeletedResource:
+          deletedResources[answer.valueString] = static_cast<OrthancPluginResourceType>(answer.valueInt32);
+          break;
+
+        case _OrthancPluginDatabaseAnswerType_RemainingAncestor:
+          remainingAncestor.reset(new std::pair<std::string, OrthancPluginResourceType>());
+          *remainingAncestor = std::make_pair(answer.valueString, static_cast<OrthancPluginResourceType>(answer.valueInt32));
+          break;
+
+        case _OrthancPluginDatabaseAnswerType_DeletedAttachment:
+          deletedAttachments.insert(reinterpret_cast<const OrthancPluginAttachment*>(answer.valueGeneric)->uuid);
+          break;
+
         default:
           printf("Unhandled message: %d\n", answer.type);
           break;
@@ -488,11 +506,28 @@
     // A transaction is needed here for MySQL, as it was not possible
     // to implement recursive deletion of resources using pure SQL
     // statements
-    manager->StartTransaction(TransactionType_ReadWrite);    
+    manager->StartTransaction(TransactionType_ReadWrite);
+
+    deletedAttachments.clear();
+    deletedResources.clear();
+    remainingAncestor.reset();
+    
     db.DeleteResource(*output, *manager, c);
+
+    ASSERT_EQ(0u, deletedAttachments.size());
+    ASSERT_EQ(1u, deletedResources.size());
+    ASSERT_EQ(OrthancPluginResourceType_Series, deletedResources["series2"]);
+    ASSERT_TRUE(remainingAncestor.get() != NULL);
+    ASSERT_EQ("study", remainingAncestor->first);
+    ASSERT_EQ(OrthancPluginResourceType_Study, remainingAncestor->second);
+    
     manager->CommitTransaction();
   }
   
+  deletedAttachments.clear();
+  deletedResources.clear();
+  remainingAncestor.reset();
+
   ASSERT_FALSE(db.IsExistingResource(*manager, c));
   ASSERT_TRUE(db.IsExistingResource(*manager, a));
   ASSERT_TRUE(db.IsExistingResource(*manager, b));
@@ -503,6 +538,12 @@
   ASSERT_FALSE(db.IsExistingResource(*manager, b));
   ASSERT_FALSE(db.IsExistingResource(*manager, c));
 
+  ASSERT_EQ(0u, deletedAttachments.size());
+  ASSERT_EQ(2u, deletedResources.size());
+  ASSERT_EQ(OrthancPluginResourceType_Series, deletedResources["series"]);
+  ASSERT_EQ(OrthancPluginResourceType_Study, deletedResources["study"]);
+  ASSERT_FALSE(remainingAncestor.get() != NULL);
+  
   ASSERT_EQ(0u, db.GetAllResourcesCount(*manager));
   ASSERT_EQ(0u, db.GetUnprotectedPatientsCount(*manager));
   int64_t p1 = db.CreateResource(*manager, "patient1", OrthancPluginResourceType_Patient);
@@ -553,5 +594,112 @@
     ASSERT_EQ(longProperty, tmp);
   }
 
+  db.DeleteResource(*output, *manager, p1);
+  db.DeleteResource(*output, *manager, p3);
+
+  for (size_t level = 0; level < 4; level++)
+  {
+    // Test cascade up to the "patient" level
+    ASSERT_EQ(0u, db.GetAllResourcesCount(*manager));
+
+    std::vector<int64_t> resources;
+    resources.push_back(db.CreateResource(*manager, "patient", OrthancPluginResourceType_Patient));
+    resources.push_back(db.CreateResource(*manager, "study", OrthancPluginResourceType_Study));
+    resources.push_back(db.CreateResource(*manager, "series", OrthancPluginResourceType_Series));
+    resources.push_back(db.CreateResource(*manager, "instance", OrthancPluginResourceType_Instance));
+
+    OrthancPluginAttachment a;
+    a.uuid = "attachment";
+    a.contentType = Orthanc::FileContentType_DicomAsJson;
+    a.uncompressedSize = 4242;
+    a.uncompressedHash = "md5";
+    a.compressionType = Orthanc::CompressionType_None;
+    a.compressedSize = 4242;
+    a.compressedHash = "md5";
+    db.AddAttachment(*manager, resources[2], a, 42);
+    
+    db.AttachChild(*manager, resources[0], resources[1]);
+    db.AttachChild(*manager, resources[1], resources[2]);
+    db.AttachChild(*manager, resources[2], resources[3]);
+    ASSERT_EQ(4u, db.GetAllResourcesCount(*manager));
+
+    deletedAttachments.clear();
+    deletedResources.clear();
+    remainingAncestor.reset();
+    
+    db.DeleteResource(*output, *manager, resources[level]);
+    
+    ASSERT_EQ(1u, deletedAttachments.size());
+    ASSERT_EQ("attachment", *deletedAttachments.begin());
+    ASSERT_EQ(4u, deletedResources.size());
+    ASSERT_EQ(OrthancPluginResourceType_Patient, deletedResources["patient"]);
+    ASSERT_EQ(OrthancPluginResourceType_Study, deletedResources["study"]);
+    ASSERT_EQ(OrthancPluginResourceType_Series, deletedResources["series"]);
+    ASSERT_EQ(OrthancPluginResourceType_Instance, deletedResources["instance"]);
+    ASSERT_TRUE(remainingAncestor.get() == NULL);
+  }
+
+  for (size_t level = 1; level < 4; level++)
+  {
+    // Test remaining ancestor
+    ASSERT_EQ(0u, db.GetAllResourcesCount(*manager));
+
+    std::vector<int64_t> resources;
+    resources.push_back(db.CreateResource(*manager, "patient", OrthancPluginResourceType_Patient));
+    resources.push_back(db.CreateResource(*manager, "study", OrthancPluginResourceType_Study));
+    resources.push_back(db.CreateResource(*manager, "series", OrthancPluginResourceType_Series));
+    resources.push_back(db.CreateResource(*manager, "instance", OrthancPluginResourceType_Instance));
+
+    int64_t unrelated = db.CreateResource(*manager, "unrelated", OrthancPluginResourceType_Patient);
+    int64_t remaining = db.CreateResource(*manager, "remaining", static_cast<OrthancPluginResourceType>(level));
+
+    db.AttachChild(*manager, resources[0], resources[1]);
+    db.AttachChild(*manager, resources[1], resources[2]);
+    db.AttachChild(*manager, resources[2], resources[3]);
+    db.AttachChild(*manager, resources[level - 1], remaining);
+    ASSERT_EQ(6u, db.GetAllResourcesCount(*manager));
+    
+    deletedAttachments.clear();
+    deletedResources.clear();
+    remainingAncestor.reset();
+    
+    db.DeleteResource(*output, *manager, resources[3]);  // delete instance
+    
+    ASSERT_EQ(0u, deletedAttachments.size());
+    ASSERT_EQ(OrthancPluginResourceType_Instance, deletedResources["instance"]);
+    
+    ASSERT_TRUE(remainingAncestor.get() != NULL);
+    
+    switch (level)
+    {
+      case 1:
+        ASSERT_EQ(3u, deletedResources.size());
+        ASSERT_EQ(OrthancPluginResourceType_Study, deletedResources["study"]);
+        ASSERT_EQ(OrthancPluginResourceType_Series, deletedResources["series"]);
+        ASSERT_EQ("patient", remainingAncestor->first);
+        ASSERT_EQ(OrthancPluginResourceType_Patient, remainingAncestor->second);
+        break;
+
+      case 2:
+        ASSERT_EQ(2u, deletedResources.size());
+        ASSERT_EQ(OrthancPluginResourceType_Series, deletedResources["series"]);
+        ASSERT_EQ("study", remainingAncestor->first);
+        ASSERT_EQ(OrthancPluginResourceType_Study, remainingAncestor->second);
+        break;
+
+      case 3:
+        ASSERT_EQ(1u, deletedResources.size());
+        ASSERT_EQ("series", remainingAncestor->first);
+        ASSERT_EQ(OrthancPluginResourceType_Series, remainingAncestor->second);
+        break;
+
+      default:
+        throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError);
+    }
+    
+    db.DeleteResource(*output, *manager, resources[0]);
+    db.DeleteResource(*output, *manager, unrelated);
+  }
+
   manager->Close();
 }
--- a/MySQL/NEWS	Mon Jul 12 12:03:33 2021 +0200
+++ b/MySQL/NEWS	Mon Jul 12 16:53:28 2021 +0200
@@ -1,6 +1,8 @@
 Pending changes in the mainline
 ===============================
 
+* Added missing calls to OrthancPluginDatabaseSignalDeletedResource()
+
 
 Release 4.1 (2021-06-30)
 ========================
--- a/MySQL/Plugins/MySQLIndex.cpp	Mon Jul 12 12:03:33 2021 +0200
+++ b/MySQL/Plugins/MySQLIndex.cpp	Mon Jul 12 16:53:28 2021 +0200
@@ -367,6 +367,9 @@
 
     while (!done)
     {
+      bool hasSibling = false;
+      int64_t parentId;
+      
       {
         DatabaseManager::CachedStatement lookupSiblings(
           STATEMENT_FROM_HERE, manager,
@@ -387,7 +390,7 @@
         }
         else
         {
-          int64_t parentId = lookupSiblings.ReadInteger64(0);
+          parentId = lookupSiblings.ReadInteger64(0);
           lookupSiblings.Next();
 
           if (lookupSiblings.IsDone())
@@ -400,39 +403,59 @@
           {
             // "id" has at least one sibling node: the parent node is the remaining ancestor
             done = true;
-
-            DatabaseManager::CachedStatement parent(
-              STATEMENT_FROM_HERE, manager,
-              "SELECT publicId, resourceType FROM Resources WHERE internalId=${id};");
-            
-            parent.SetParameterType("id", ValueType_Integer64);
-
-            Dictionary args2;
-            args2.SetIntegerValue("id", parentId);
-    
-            parent.Execute(args2);
-
-            output.SignalRemainingAncestor(
-              parent.ReadString(0),
-              static_cast<OrthancPluginResourceType>(parent.ReadInteger32(1)));
+            hasSibling = true;
           }
         }
       }
+
+      if (hasSibling)
+      {
+        // This cannot be executed in the same scope as another
+        // DatabaseManager::CachedStatement
+        
+        DatabaseManager::CachedStatement parent(
+          STATEMENT_FROM_HERE, manager,
+          "SELECT publicId, resourceType FROM Resources WHERE internalId=${id};");
+        
+        parent.SetParameterType("id", ValueType_Integer64);
+        
+        Dictionary args2;
+        args2.SetIntegerValue("id", parentId);
+        
+        parent.Execute(args2);
+        
+        output.SignalRemainingAncestor(
+          parent.ReadString(0),
+          static_cast<OrthancPluginResourceType>(parent.ReadInteger32(1)));
+      }
+    }
+
+    {
+      DatabaseManager::CachedStatement dropTemporaryTable(
+        STATEMENT_FROM_HERE, manager,
+        "DROP TEMPORARY TABLE IF EXISTS DeletedResources");
+      dropTemporaryTable.Execute();
+    }
+
+    {
+      DatabaseManager::CachedStatement lookupResourcesToDelete(
+        STATEMENT_FROM_HERE, manager,
+        "CREATE TEMPORARY TABLE DeletedResources SELECT * FROM (SELECT internalId, resourceType, publicId FROM Resources WHERE internalId=${id} OR parentId=${id} OR parentId IN (SELECT internalId FROM Resources WHERE parentId=${id}) OR parentId IN (SELECT internalId FROM Resources WHERE parentId IN (SELECT internalId FROM Resources WHERE parentId=${id}))) AS t");
+      lookupResourcesToDelete.SetParameterType("id", ValueType_Integer64);
+
+      Dictionary args;
+      args.SetIntegerValue("id", id);
+      lookupResourcesToDelete.Execute(args);
     }
 
     {
       DatabaseManager::CachedStatement deleteHierarchy(
         STATEMENT_FROM_HERE, manager,
-        "DELETE FROM Resources WHERE internalId IN (SELECT * FROM (SELECT internalId FROM Resources WHERE internalId=${id} OR parentId=${id} OR parentId IN (SELECT internalId FROM Resources WHERE parentId=${id}) OR parentId IN (SELECT internalId FROM Resources WHERE parentId IN (SELECT internalId FROM Resources WHERE parentId=${id}))) as t);");
-      
-      deleteHierarchy.SetParameterType("id", ValueType_Integer64);
-      
-      Dictionary args;
-      args.SetIntegerValue("id", id);
-    
-      deleteHierarchy.Execute(args);
+        "DELETE FROM Resources WHERE internalId IN (SELECT internalId FROM DeletedResources)");
+      deleteHierarchy.Execute();
     }
 
+    SignalDeletedResources(output, manager);
     SignalDeletedFiles(output, manager);
   }