# HG changeset patch # User Sebastien Jodogne # Date 1626101608 -7200 # Node ID 8de3a1ecac11542a079c977ee00039c296e0d434 # Parent 544e0c943b40d2486a508f1c7cf01b5da0d9dea2 MySQL: Added missing calls to OrthancPluginDatabaseSignalDeletedResource() diff -r 544e0c943b40 -r 8de3a1ecac11 Framework/Common/DatabaseManager.h --- 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()); + } }; diff -r 544e0c943b40 -r 8de3a1ecac11 Framework/Plugins/IndexBackend.cpp --- 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(); diff -r 544e0c943b40 -r 8de3a1ecac11 Framework/Plugins/IndexUnitTests.h --- 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 expectedDicomTags; static std::unique_ptr expectedExported; +static std::map deletedResources; +static std::unique_ptr< std::pair > remainingAncestor; +static std::set 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(answer.valueInt32); + break; + + case _OrthancPluginDatabaseAnswerType_RemainingAncestor: + remainingAncestor.reset(new std::pair()); + *remainingAncestor = std::make_pair(answer.valueString, static_cast(answer.valueInt32)); + break; + + case _OrthancPluginDatabaseAnswerType_DeletedAttachment: + deletedAttachments.insert(reinterpret_cast(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 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 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(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(); } diff -r 544e0c943b40 -r 8de3a1ecac11 MySQL/NEWS --- 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) ======================== diff -r 544e0c943b40 -r 8de3a1ecac11 MySQL/Plugins/MySQLIndex.cpp --- 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(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(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); }