Mercurial > hg > orthanc-databases

changeset 620:28fd73902e60

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
merge
author Alain Mazy <am@orthanc.team>
date Wed, 18 Dec 2024 12:29:54 +0100
parents a9a7dceeaad0 (diff) 5a325e5aee51 (current diff)
children f00417b34bbb
files
diffstat 5 files changed, 108 insertions(+), 21 deletions(-) [+]
line wrap: on
line diff
--- a/Framework/Plugins/IndexUnitTests.h	Wed Dec 18 10:09:48 2024 +0100
+++ b/Framework/Plugins/IndexUnitTests.h	Wed Dec 18 12:29:54 2024 +0100
@@ -567,7 +567,14 @@
   ASSERT_TRUE(db.IsExistingResource(*manager, a));
   ASSERT_TRUE(db.IsExistingResource(*manager, b));
   ASSERT_EQ(2u, db.GetAllResourcesCount(*manager));
-  db.DeleteResource(*output, *manager, a);
+
+  {
+    // An explicit transaction is needed here
+    manager->StartTransaction(TransactionType_ReadWrite);
+    db.DeleteResource(*output, *manager, a);
+    manager->CommitTransaction();
+  }
+
   ASSERT_EQ(0u, db.GetAllResourcesCount(*manager));
   ASSERT_FALSE(db.IsExistingResource(*manager, a));
   ASSERT_FALSE(db.IsExistingResource(*manager, b));
@@ -599,7 +606,14 @@
   ASSERT_FALSE(db.IsProtectedPatient(*manager, p1));
   ASSERT_TRUE(db.SelectPatientToRecycle(r, *manager));
   ASSERT_EQ(p2, r);
-  db.DeleteResource(*output, *manager, p2);
+
+  {
+    // An explicit transaction is needed here
+    manager->StartTransaction(TransactionType_ReadWrite);
+    db.DeleteResource(*output, *manager, p2);
+    manager->CommitTransaction();
+  }
+
   ASSERT_TRUE(db.SelectPatientToRecycle(r, *manager, p3));
   ASSERT_EQ(p1, r);
 
@@ -629,8 +643,13 @@
     ASSERT_EQ(longProperty, tmp);
   }
 
-  db.DeleteResource(*output, *manager, p1);
-  db.DeleteResource(*output, *manager, p3);
+  {
+    manager->StartTransaction(TransactionType_ReadWrite);
+    db.DeleteResource(*output, *manager, p1);
+    db.DeleteResource(*output, *manager, p3);
+    manager->CommitTransaction();
+  }
+
 
   for (size_t level = 0; level < 4; level++)
   {
@@ -664,7 +683,11 @@
       deletedResources.clear();
       remainingAncestor.reset();
     
-      db.DeleteResource(*output, *manager, resources[level]);
+      {
+        manager->StartTransaction(TransactionType_ReadWrite);
+        db.DeleteResource(*output, *manager, resources[level]);
+        manager->CommitTransaction();
+      }
     
       ASSERT_EQ(1u, deletedAttachments.size());
       ASSERT_EQ("attachment", *deletedAttachments.begin());
@@ -721,7 +744,11 @@
       deletedResources.clear();
       remainingAncestor.reset();
     
-      db.DeleteResource(*output, *manager, resources[3]);  // delete instance
+      {
+        manager->StartTransaction(TransactionType_ReadWrite);
+        db.DeleteResource(*output, *manager, resources[3]);  // delete instance
+        manager->CommitTransaction();
+      }
 
       if (attachmentLevel < level)
       {
@@ -764,8 +791,12 @@
           throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError);
       }
     
-      db.DeleteResource(*output, *manager, resources[0]);
-      db.DeleteResource(*output, *manager, unrelated);
+      {
+        manager->StartTransaction(TransactionType_ReadWrite);
+        db.DeleteResource(*output, *manager, resources[0]);
+        db.DeleteResource(*output, *manager, unrelated);
+        manager->CommitTransaction();
+      }
     }
   }
 
--- a/PostgreSQL/NEWS	Wed Dec 18 10:09:48 2024 +0100
+++ b/PostgreSQL/NEWS	Wed Dec 18 12:29:54 2024 +0100
@@ -22,7 +22,6 @@
   is populating this new table and might consume DB bandwitdh and CPU.
 
 
-* Fix updates from plugin version 3.3 to latest version
 * Added support for ExtendedChanges:
   - changes?type=...&to=...
 * Performance optimizations (to be summarized before release):
@@ -40,7 +39,8 @@
   DB plugins requiring it (currently only PostgreSQL).  E.g: This avoids very long update
   times in case you don't call /statistics for a long period.  The execution interval of
   this thread can be configured through "HousekeepingInterval".
-
+* Fix updates from plugin version 3.3 to latest version
+* Fix a bug introduced in 6.0 in the PatientRecyclingOrder when using patient protection.
 
 Release 6.2 (2024-03-25)
 ========================
--- a/PostgreSQL/Plugins/SQL/Downgrades/Rev3ToRev2.sql	Wed Dec 18 10:09:48 2024 +0100
+++ b/PostgreSQL/Plugins/SQL/Downgrades/Rev3ToRev2.sql	Wed Dec 18 12:29:54 2024 +0100
@@ -84,7 +84,51 @@
 DROP FUNCTION ComputeMissingChildCount;
 DROP FUNCTION UpdateChildCount;
 
+-- restore the previous PatientRecyclingOrder logic although it was buggy wrt protected patients
+DROP TRIGGER IF EXISTS PatientAdded ON Resources;
 
+CREATE OR REPLACE FUNCTION PatientAddedOrUpdated(
+    IN patient_id BIGINT,
+    IN is_update BIGINT
+    )
+RETURNS VOID AS $body$
+BEGIN
+    DECLARE
+        newSeq BIGINT;
+    BEGIN
+        UPDATE GlobalIntegers SET value = value + 1 WHERE key = 7 RETURNING value INTO newSeq;
+        IF is_update > 0 THEN
+            -- Note: Protected patients are not listed in this table !  So, they won't be updated
+            UPDATE PatientRecyclingOrder SET seq = newSeq WHERE PatientRecyclingOrder.patientId = patient_id;
+        ELSE
+            INSERT INTO PatientRecyclingOrder VALUES (newSeq, patient_id);
+        END IF;
+    END;
+END;
+$body$ LANGUAGE plpgsql;
+
+CREATE OR REPLACE FUNCTION PatientAddedFunc() 
+RETURNS TRIGGER AS $body$
+BEGIN
+  -- The "0" corresponds to "OrthancPluginResourceType_Patient"
+  IF new.resourceType = 0 THEN
+    PERFORM PatientAddedOrUpdated(new.internalId, 0);
+  END IF;
+  RETURN NULL;
+END;
+$body$ LANGUAGE plpgsql;
+
+DROP TRIGGER IF EXISTS PatientAdded on Resources;
+CREATE TRIGGER PatientAdded
+AFTER INSERT ON Resources
+FOR EACH ROW
+EXECUTE PROCEDURE PatientAddedFunc();
+
+INSERT INTO GlobalIntegers
+    SELECT 7, CAST(COALESCE(MAX(seq), 0) AS BIGINT) FROM PatientRecyclingOrder
+    ON CONFLICT DO NOTHING;
+
+-----------
 
 -- set the global properties that actually documents the DB version, revision and some of the capabilities
 -- modify only the ones that have changed
--- a/PostgreSQL/Plugins/SQL/PrepareIndex.sql	Wed Dec 18 10:09:48 2024 +0100
+++ b/PostgreSQL/Plugins/SQL/PrepareIndex.sql	Wed Dec 18 12:29:54 2024 +0100
@@ -99,7 +99,7 @@
 -- 4: SeriesCount
 -- 5: InstancesCount
 -- 6: ChangeSeq
--- 7: PatientRecyclingOrderSeq
+-- 7: PatientRecyclingOrderSeq  (removed in 7.0)
 
 CREATE TABLE IF NOT EXISTS ServerProperties(
         server VARCHAR(64) NOT NULL,
@@ -170,12 +170,18 @@
     DECLARE
         newSeq BIGINT;
     BEGIN
-        UPDATE GlobalIntegers SET value = value + 1 WHERE key = 7 RETURNING value INTO newSeq;
         IF is_update > 0 THEN
             -- Note: Protected patients are not listed in this table !  So, they won't be updated
-            UPDATE PatientRecyclingOrder SET seq = newSeq WHERE PatientRecyclingOrder.patientId = patient_id;
+            WITH deleted_rows AS (
+                DELETE FROM PatientRecyclingOrder
+                WHERE PatientRecyclingOrder.patientId = patient_id
+                RETURNING patientId
+            )
+            INSERT INTO PatientRecyclingOrder (patientId)
+            SELECT patientID FROM deleted_rows
+            WHERE EXISTS(SELECT 1 FROM deleted_rows);
         ELSE
-            INSERT INTO PatientRecyclingOrder VALUES (newSeq, patient_id);
+            INSERT INTO PatientRecyclingOrder VALUES (DEFAULT, patient_id);
         END IF;
     END;
 END;
@@ -198,10 +204,14 @@
 FOR EACH ROW
 EXECUTE PROCEDURE PatientAddedFunc();
 
--- initial population of PatientRecyclingOrderSeq
-INSERT INTO GlobalIntegers
-    SELECT 7, CAST(COALESCE(MAX(seq), 0) AS BIGINT) FROM PatientRecyclingOrder
-    ON CONFLICT DO NOTHING;
+-- initial population of 
+SELECT setval('patientrecyclingorder_seq_seq', MAX(seq)) FROM PatientRecyclingOrder;
+DELETE FROM GlobalIntegers WHERE key = 7;
+        -- UPDATE GlobalIntegers SET value = value + 1 WHERE key = 7 RETURNING value INTO newSeq;
+
+-- INSERT INTO GlobalIntegers
+--     SELECT 7, CAST(COALESCE(MAX(seq), 0) AS BIGINT) FROM PatientRecyclingOrder
+--     ON CONFLICT DO NOTHING;
 
 
 ------------------- ResourceDeleted trigger -------------------
--- a/PostgreSQL/UnitTests/PostgreSQLTests.cpp	Wed Dec 18 10:09:48 2024 +0100
+++ b/PostgreSQL/UnitTests/PostgreSQLTests.cpp	Wed Dec 18 12:29:54 2024 +0100
@@ -548,7 +548,7 @@
 
   std::string s;
   ASSERT_TRUE(db.LookupGlobalProperty(s, *manager, MISSING_SERVER_IDENTIFIER, Orthanc::GlobalProperty_DatabaseInternal1));
-  ASSERT_EQ("2", s);
+  ASSERT_EQ("3", s);
 
   OrthancPluginCreateInstanceResult r1, r2;
   
@@ -565,8 +565,10 @@
   ASSERT_EQ(r1.instanceId, r2.instanceId);
 
   // Breaking the hierarchy
-  memset(&r2, 0, sizeof(r2));
-  ASSERT_THROW(db.CreateInstance(r2, *manager, "a", "e", "c", "f"), Orthanc::OrthancException);
+  // This does not throw anymore since at least 6.0.  This would only happen in case of series hash collision
+  // which would actually be very damagefull at many places in Orthanc.
+  // memset(&r2, 0, sizeof(r2));
+  // ASSERT_THROW(db.CreateInstance(r2, *manager, "a", "e", "c", "f"), Orthanc::OrthancException);
 
   memset(&r2, 0, sizeof(r2));
   db.CreateInstance(r2, *manager, "a", "b", "c", "e");