changeset 5593:862b54b4cfe2 find-refactoring

implemented the default multi-stage find/expand
author Sebastien Jodogne <s.jodogne@gmail.com>
date Sat, 04 May 2024 11:35:34 +0200
parents 1e2631b8b9af
children a906dc19264c
files OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp OrthancServer/Plugins/Engine/OrthancPluginDatabaseV4.cpp OrthancServer/Plugins/Engine/OrthancPluginDatabaseV4.h OrthancServer/Resources/Orthanc.doxygen OrthancServer/Sources/Database/BaseDatabaseWrapper.cpp OrthancServer/Sources/Database/BaseDatabaseWrapper.h OrthancServer/Sources/Database/Compatibility/GenericFind.cpp OrthancServer/Sources/Database/Compatibility/GenericFind.h OrthancServer/Sources/Database/IDatabaseWrapper.h OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp OrthancServer/Sources/Database/StatelessDatabaseOperations.h
diffstat 13 files changed, 245 insertions(+), 98 deletions(-) [+]
line wrap: on
line diff
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp	Sat May 04 11:35:34 2024 +0200
@@ -30,7 +30,6 @@
 
 #include "../../../OrthancFramework/Sources/Logging.h"
 #include "../../../OrthancFramework/Sources/OrthancException.h"
-#include "../../Sources/Database/Compatibility/GenericFind.h"
 #include "../../Sources/Database/Compatibility/ICreateInstance.h"
 #include "../../Sources/Database/Compatibility/IGetChildrenMetadata.h"
 #include "../../Sources/Database/Compatibility/ILookupResourceAndParent.h"
@@ -1448,15 +1447,6 @@
     {
       throw OrthancException(ErrorCode_InternalError);  // Not supported
     }
-
-
-    virtual void ExecuteFind(FindResponse& response,
-                             const FindRequest& request, 
-                             const std::vector<DatabaseConstraint>& normalized) ORTHANC_OVERRIDE
-    {
-      Compatibility::GenericFind find(*this);
-      find.Execute(response, request);
-    }
   };
 
 
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV3.cpp	Sat May 04 11:35:34 2024 +0200
@@ -29,7 +29,6 @@
 
 #include "../../../OrthancFramework/Sources/Logging.h"
 #include "../../../OrthancFramework/Sources/OrthancException.h"
-#include "../../Sources/Database/Compatibility/GenericFind.h"
 #include "../../Sources/Database/ResourcesContent.h"
 #include "../../Sources/Database/VoidDatabaseListener.h"
 #include "PluginsEnumerations.h"
@@ -1061,15 +1060,6 @@
     {
       throw OrthancException(ErrorCode_InternalError);  // Not supported
     }
-
-
-    virtual void ExecuteFind(FindResponse& response,
-                             const FindRequest& request, 
-                             const std::vector<DatabaseConstraint>& normalized) ORTHANC_OVERRIDE
-    {
-      Compatibility::GenericFind find(*this);
-      find.Execute(response, request);
-    }
   };
 
   
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV4.cpp	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV4.cpp	Sat May 04 11:35:34 2024 +0200
@@ -1279,11 +1279,31 @@
 
 
     virtual void ExecuteFind(FindResponse& response,
-                             const FindRequest& request, 
+                             const FindRequest& request,
+                             const std::vector<DatabaseConstraint>& normalized) ORTHANC_OVERRIDE
+    {
+      // TODO-FIND
+      throw OrthancException(ErrorCode_NotImplemented);
+    }
+
+
+    virtual void ExecuteFind(std::list<std::string>& identifiers,
+                             const FindRequest& request,
                              const std::vector<DatabaseConstraint>& normalized) ORTHANC_OVERRIDE
     {
+      // TODO-FIND
       Compatibility::GenericFind find(*this);
-      find.Execute(response, request);
+      find.ExecuteFind(identifiers, request, normalized);
+    }
+
+
+    virtual void ExecuteExpand(FindResponse& response,
+                               const FindRequest& request,
+                               const std::string& identifier) ORTHANC_OVERRIDE
+    {
+      // TODO-FIND
+      Compatibility::GenericFind find(*this);
+      find.ExecuteExpand(response, request, identifier);
     }
   };
 
@@ -1499,4 +1519,10 @@
       return dbCapabilities_;
     }
   }
+
+
+  bool OrthancPluginDatabaseV4::HasIntegratedFind() const
+  {
+    return false;  // TODO-FIND
+  }
 }
--- a/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV4.h	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Plugins/Engine/OrthancPluginDatabaseV4.h	Sat May 04 11:35:34 2024 +0200
@@ -92,6 +92,8 @@
     virtual uint64_t MeasureLatency() ORTHANC_OVERRIDE;
 
     virtual const Capabilities GetDatabaseCapabilities() const ORTHANC_OVERRIDE;
+
+    virtual bool HasIntegratedFind() const ORTHANC_OVERRIDE;
   };
 }
 
--- a/OrthancServer/Resources/Orthanc.doxygen	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Resources/Orthanc.doxygen	Sat May 04 11:35:34 2024 +0200
@@ -755,6 +755,7 @@
 # Note: If this tag is empty the current directory is searched.
 
 INPUT                  = @CMAKE_SOURCE_DIR@/../OrthancFramework/Sources \
+                         @CMAKE_SOURCE_DIR@/Plugins/Engine \
                          @CMAKE_SOURCE_DIR@/Sources
 
 # This tag can be used to specify the character encoding of the source files
--- a/OrthancServer/Sources/Database/BaseDatabaseWrapper.cpp	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Sources/Database/BaseDatabaseWrapper.cpp	Sat May 04 11:35:34 2024 +0200
@@ -23,6 +23,7 @@
 #include "BaseDatabaseWrapper.h"
 
 #include "../../../OrthancFramework/Sources/OrthancException.h"
+#include "Compatibility/GenericFind.h"
 
 namespace Orthanc
 {
@@ -45,6 +46,32 @@
   }
 
 
+  void BaseDatabaseWrapper::BaseTransaction::ExecuteFind(FindResponse& response,
+                                                         const FindRequest& request,
+                                                         const std::vector<DatabaseConstraint>& normalized)
+  {
+    throw OrthancException(ErrorCode_NotImplemented);  // Not supported
+  }
+
+
+  void BaseDatabaseWrapper::BaseTransaction::ExecuteFind(std::list<std::string>& identifiers,
+                                                         const FindRequest& request,
+                                                         const std::vector<DatabaseConstraint>& normalized)
+  {
+    Compatibility::GenericFind find(*this);
+    find.ExecuteFind(identifiers, request, normalized);
+  }
+
+
+  void BaseDatabaseWrapper::BaseTransaction::ExecuteExpand(FindResponse& response,
+                                                           const FindRequest& request,
+                                                           const std::string& identifier)
+  {
+    Compatibility::GenericFind find(*this);
+    find.ExecuteExpand(response, request, identifier);
+  }
+
+
   uint64_t BaseDatabaseWrapper::MeasureLatency()
   {
     throw OrthancException(ErrorCode_NotImplemented);  // only implemented in V4
--- a/OrthancServer/Sources/Database/BaseDatabaseWrapper.h	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Sources/Database/BaseDatabaseWrapper.h	Sat May 04 11:35:34 2024 +0200
@@ -46,8 +46,25 @@
                                           int64_t& instancesCount,
                                           int64_t& compressedSize,
                                           int64_t& uncompressedSize) ORTHANC_OVERRIDE;
+
+      virtual void ExecuteFind(FindResponse& response,
+                               const FindRequest& request,
+                               const std::vector<DatabaseConstraint>& normalized) ORTHANC_OVERRIDE;
+
+      virtual void ExecuteFind(std::list<std::string>& identifiers,
+                               const FindRequest& request,
+                               const std::vector<DatabaseConstraint>& normalized) ORTHANC_OVERRIDE;
+
+      virtual void ExecuteExpand(FindResponse& response,
+                                 const FindRequest& request,
+                                 const std::string& identifier) ORTHANC_OVERRIDE;
     };
 
     virtual uint64_t MeasureLatency() ORTHANC_OVERRIDE;
+
+    virtual bool HasIntegratedFind() const ORTHANC_OVERRIDE
+    {
+      return false;
+    }
   };
 }
--- a/OrthancServer/Sources/Database/Compatibility/GenericFind.cpp	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Sources/Database/Compatibility/GenericFind.cpp	Sat May 04 11:35:34 2024 +0200
@@ -30,8 +30,9 @@
 {
   namespace Compatibility
   {
-    void GenericFind::Execute(FindResponse& response,
-                              const FindRequest& request)
+    void GenericFind::ExecuteFind(std::list<std::string>& identifiers,
+                                  const FindRequest& request,
+                                  const std::vector<DatabaseConstraint>& normalized)
     {
       if (!request.GetOrthancIdentifiers().HasPatientId() &&
           !request.GetOrthancIdentifiers().HasStudyId() &&
@@ -42,94 +43,89 @@
           request.GetOrdering().empty() &&
           request.GetLabels().empty())
       {
-        std::list<std::string> ids;
-
         if (request.HasLimits())
         {
-          transaction_.GetAllPublicIds(ids, request.GetLevel(), request.GetLimitsSince(), request.GetLimitsCount());
+          transaction_.GetAllPublicIds(identifiers, request.GetLevel(), request.GetLimitsSince(), request.GetLimitsCount());
         }
         else
         {
-          transaction_.GetAllPublicIds(ids, request.GetLevel());
-        }
-
-        for (std::list<std::string>::const_iterator it = ids.begin(); it != ids.end(); ++it)
-        {
-          int64_t internalId;
-          ResourceType t;
-          if (!transaction_.LookupResource(internalId, t, *it) ||
-              t != request.GetLevel())
-          {
-            throw OrthancException(ErrorCode_InternalError);
-          }
-
-          std::unique_ptr<FindResponse::Resource> resource(new FindResponse::Resource(request.GetLevel(), *it));
-
-          if (request.IsRetrieveMainDicomTags())
-          {
-            DicomMap m;
-            transaction_.GetMainDicomTags(m, internalId);
-
-            DicomArray a(m);
-            for (size_t i = 0; i < a.GetSize(); i++)
-            {
-              const DicomElement& element = a.GetElement(i);
-              if (element.GetValue().IsString())
-              {
-                resource->AddStringDicomTag(element.GetTag().GetGroup(), element.GetTag().GetElement(),
-                                            element.GetValue().GetContent());
-              }
-              else
-              {
-                throw OrthancException(ErrorCode_BadParameterType);
-              }
-            }
-          }
-
-          if (request.IsRetrieveParentIdentifier())
-          {
-            int64_t parentId;
-            if (transaction_.LookupParent(parentId, internalId))
-            {
-              resource->SetParentIdentifier(transaction_.GetPublicId(parentId));
-            }
-            else
-            {
-              throw OrthancException(ErrorCode_InternalError);
-            }
-          }
-
-          // TODO-FIND: Continue
-
-          response.Add(resource.release());
+          transaction_.GetAllPublicIds(identifiers, request.GetLevel());
         }
       }
       else
       {
-        throw OrthancException(ErrorCode_NotImplemented);
+        throw OrthancException(ErrorCode_NotImplemented);  // Not supported
+      }
+    }
+
+
+    void GenericFind::ExecuteExpand(FindResponse& response,
+                                    const FindRequest& request,
+                                    const std::string& identifier)
+    {
+      int64_t internalId;
+      ResourceType t;
+      if (!transaction_.LookupResource(internalId, t, identifier) ||
+          t != request.GetLevel())
+      {
+        throw OrthancException(ErrorCode_InternalError);
       }
 
+      std::unique_ptr<FindResponse::Resource> resource(new FindResponse::Resource(request.GetLevel(), identifier));
+
+      if (request.IsRetrieveMainDicomTags())
+      {
+        DicomMap m;
+        transaction_.GetMainDicomTags(m, internalId);
+
+        DicomArray a(m);
+        for (size_t i = 0; i < a.GetSize(); i++)
+        {
+          const DicomElement& element = a.GetElement(i);
+          if (element.GetValue().IsString())
+          {
+            resource->AddStringDicomTag(element.GetTag().GetGroup(), element.GetTag().GetElement(),
+                                        element.GetValue().GetContent());
+          }
+          else
+          {
+            throw OrthancException(ErrorCode_BadParameterType);
+          }
+        }
+      }
+
+      if (request.IsRetrieveParentIdentifier())
+      {
+        int64_t parentId;
+        if (transaction_.LookupParent(parentId, internalId))
+        {
+          resource->SetParentIdentifier(transaction_.GetPublicId(parentId));
+        }
+        else
+        {
+          throw OrthancException(ErrorCode_InternalError);
+        }
+      }
+
+      // TODO-FIND: Continue
+
 
       /**
        * Sanity checks
        **/
 
-      for (size_t i = 0; i < response.GetSize(); i++)
+      if (request.IsRetrieveMainDicomTags())
       {
-        const FindResponse::Resource& resource = response.GetResource(i);
-
-        if (request.IsRetrieveMainDicomTags())
+        DicomMap tmp;
+        resource->GetMainDicomTags(tmp);
+        if (tmp.GetSize() == 0)
         {
-          DicomMap tmp;
-          resource.GetMainDicomTags(tmp);
-          if (tmp.GetSize() == 0)
-          {
-            throw OrthancException(ErrorCode_InternalError);
-          }
+          throw OrthancException(ErrorCode_InternalError);
         }
+      }
 
-        // TODO: other sanity checks
-      }
+
+      response.Add(resource.release());
     }
   }
 }
--- a/OrthancServer/Sources/Database/Compatibility/GenericFind.h	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Sources/Database/Compatibility/GenericFind.h	Sat May 04 11:35:34 2024 +0200
@@ -28,7 +28,6 @@
 {
   namespace Compatibility
   {
-    // TODO-FIND: remove this class that only contains a temporary implementation
     class GenericFind : public boost::noncopyable
     {
     private:
@@ -40,8 +39,13 @@
       {
       }
 
-      void Execute(FindResponse& response,
-                   const FindRequest& request);
+      void ExecuteFind(std::list<std::string>& identifiers,
+                       const FindRequest& request,
+                       const std::vector<DatabaseConstraint>& normalized);
+
+      void ExecuteExpand(FindResponse& response,
+                         const FindRequest& request,
+                         const std::string& identifier);
     };
   }
 }
--- a/OrthancServer/Sources/Database/IDatabaseWrapper.h	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Sources/Database/IDatabaseWrapper.h	Sat May 04 11:35:34 2024 +0200
@@ -357,9 +357,28 @@
        * Primitives introduced in Orthanc 1.12.4
        **/
 
+      // This is only implemented if "HasIntegratedFind()" is "true"
       virtual void ExecuteFind(FindResponse& response,
                                const FindRequest& request,
                                const std::vector<DatabaseConstraint>& normalized) = 0;
+
+      // This is only implemented if "HasIntegratedFind()" is "false"
+      virtual void ExecuteFind(std::list<std::string>& identifiers,
+                               const FindRequest& request,
+                               const std::vector<DatabaseConstraint>& normalized) = 0;
+
+      /**
+       * This is only implemented if "HasIntegratedFind()" is
+       * "false". In this flavor, the resource of interest might have
+       * been deleted, as the expansion is not done in the same
+       * transaction as the "ExecuteFind()". In such cases, the
+       * wrapper should not throw an exception, but simply ignore the
+       * request to expand the resource (i.e., "response" must not be
+       * modified).
+       **/
+      virtual void ExecuteExpand(FindResponse& response,
+                                 const FindRequest& request,
+                                 const std::string& identifier) = 0;
     };
 
 
@@ -384,5 +403,9 @@
     virtual const Capabilities GetDatabaseCapabilities() const = 0;
 
     virtual uint64_t MeasureLatency() = 0;
+
+    // Returns "true" iff. the database engine supports the
+    // simultaneous find and expansion of resources.
+    virtual bool HasIntegratedFind() const = 0;
   };
 }
--- a/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp	Sat May 04 11:35:34 2024 +0200
@@ -1140,6 +1140,10 @@
     }
 
 
+#if 0
+    // TODO-FIND: Remove this implementation, as it should be done by
+    // the compatibility mode implemented by "GenericFind"
+    
     virtual void ExecuteFind(FindResponse& response,
                              const FindRequest& request, 
                              const std::vector<DatabaseConstraint>& normalized) ORTHANC_OVERRIDE
@@ -1301,6 +1305,8 @@
 
 #endif
     }
+#endif
+
   };
 
 
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp	Sat May 04 11:35:34 2024 +0200
@@ -3844,7 +3844,17 @@
   void StatelessDatabaseOperations::ExecuteFind(FindResponse& response,
                                                 const FindRequest& request)
   {
-    class Operations : public ReadOnlyOperationsT3<FindResponse&, const FindRequest&, const std::vector<DatabaseConstraint>&>
+    class IntegratedFind : public ReadOnlyOperationsT3<FindResponse&, const FindRequest&, const std::vector<DatabaseConstraint>&>
+    {
+    public:
+      virtual void ApplyTuple(ReadOnlyTransaction& transaction,
+                              const Tuple& tuple) ORTHANC_OVERRIDE
+      {
+        transaction.ExecuteFind(tuple.get<0>(), tuple.get<1>(), tuple.get<2>());
+      }
+    };
+
+    class FindStage : public ReadOnlyOperationsT3<std::list<std::string>&, const FindRequest&, const std::vector<DatabaseConstraint>&>
     {
     public:
       virtual void ApplyTuple(ReadOnlyTransaction& transaction,
@@ -3854,11 +3864,52 @@
       }
     };
 
+    class ExpandStage : public ReadOnlyOperationsT3<FindResponse&, const FindRequest&, const std::string&>
+    {
+    public:
+      virtual void ApplyTuple(ReadOnlyTransaction& transaction,
+                              const Tuple& tuple) ORTHANC_OVERRIDE
+      {
+        transaction.ExecuteExpand(tuple.get<0>(), tuple.get<1>(), tuple.get<2>());
+      }
+    };
+
     std::vector<DatabaseConstraint> normalized;
     NormalizeLookup(normalized, request);
 
-    Operations operations;
-    operations.Apply(*this, response, request, normalized);
+    if (db_.HasIntegratedFind())
+    {
+      /**
+       * In this flavor, the "find" and the "expand" phases are
+       * executed in one single transaction.
+       **/
+      IntegratedFind operations;
+      operations.Apply(*this, response, request, normalized);
+    }
+    else
+    {
+      /**
+       * In this flavor, the "find" and the "expand" phases for each
+       * found resource are executed in distinct transactions. This is
+       * the compatibility mode equivalent to Orthanc <= 1.12.3.
+       **/
+      std::list<std::string> identifiers;
+
+      FindStage find;
+      find.Apply(*this, identifiers, request, normalized);
+
+      ExpandStage expand;
+
+      for (std::list<std::string>::const_iterator it = identifiers.begin(); it != identifiers.end(); ++it)
+      {
+        /**
+         * Not that the resource might have been deleted (as we are in
+         * another transaction). The database engine must ignore such
+         * error cases.
+         **/
+        expand.Apply(*this, response, request, *it);
+      }
+    }
   }
 
   // TODO-FIND: we reuse the ExpandedResource class to reuse Serialization code from ExpandedResource
--- a/OrthancServer/Sources/Database/StatelessDatabaseOperations.h	Fri May 03 21:26:06 2024 +0200
+++ b/OrthancServer/Sources/Database/StatelessDatabaseOperations.h	Sat May 04 11:35:34 2024 +0200
@@ -397,6 +397,20 @@
       {
         transaction_.ExecuteFind(response, request, normalized);
       }
+
+      void ExecuteFind(std::list<std::string>& identifiers,
+                       const FindRequest& request,
+                       const std::vector<DatabaseConstraint>& normalized)
+      {
+        transaction_.ExecuteFind(identifiers, request, normalized);
+      }
+
+      void ExecuteExpand(FindResponse& response,
+                         const FindRequest& request,
+                         const std::string& identifier)
+      {
+        transaction_.ExecuteExpand(response, request, identifier);
+      }
     };