Mercurial > hg > orthanc
changeset 6562:d4a3c883f6c1
restored ApplyLookupResources and make sure GenericFind handles Labels filtering against 'None' and [] correctly
| author | Alain Mazy <am@orthanc.team> |
|---|---|
| date | Thu, 15 Jan 2026 15:25:30 +0100 |
| parents | f2de0250584c |
| children | 62ebe3e9e8bf |
| files | NEWS OrthancServer/Sources/Database/Compatibility/GenericFind.cpp OrthancServer/Sources/Database/FindRequest.cpp OrthancServer/Sources/Database/FindRequest.h OrthancServer/Sources/Database/IDatabaseWrapper.h OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp OrthancServer/Sources/Database/SQLiteDatabaseWrapper.h OrthancServer/Sources/Search/ISqlLookupFormatter.cpp OrthancServer/Sources/Search/ISqlLookupFormatter.h |
| diffstat | 9 files changed, 185 insertions(+), 17 deletions(-) [+] |
line wrap: on
line diff
--- a/NEWS Wed Jan 14 19:06:30 2026 +0100 +++ b/NEWS Thu Jan 15 15:25:30 2026 +0100 @@ -4,7 +4,7 @@ General ------- - +TODO: re-enable ExtendedFind in SQLite REST API -------- @@ -15,7 +15,7 @@ through the `filename` argument of "/.../file" or "/.../archive" routes. * In tools/find, filtering against "LabelsConstraint": "None" with an empty "Labels" list now returns all resources that do not have any labels attached instead of returning all resources. - This applies to the default SQLite DB and will apply to the next PostgreSQL plugin (vXXX) + This applies to the default SQLite DB and will apply to the next PostgreSQL plugin (v 10.1) Maintenance
--- a/OrthancServer/Sources/Database/Compatibility/GenericFind.cpp Wed Jan 14 19:06:30 2026 +0100 +++ b/OrthancServer/Sources/Database/Compatibility/GenericFind.cpp Thu Jan 15 15:25:30 2026 +0100 @@ -110,7 +110,7 @@ const IDatabaseWrapper::Capabilities& capabilities, const FindRequest& request) { - if (!request.GetLabels().empty() && + if (request.HasLabelsConstraint() && !capabilities.HasLabelsSupport()) { throw OrthancException(ErrorCode_NotImplemented, "The database backend doesn't support labels");
--- a/OrthancServer/Sources/Database/FindRequest.cpp Wed Jan 14 19:06:30 2026 +0100 +++ b/OrthancServer/Sources/Database/FindRequest.cpp Thu Jan 15 15:25:30 2026 +0100 @@ -285,11 +285,16 @@ } } + bool FindRequest::HasLabelsConstraint() const + { + return !GetLabels().empty() || GetLabelsConstraint() == LabelsConstraint_None; // from 1.12.11, 'None' with an empty labels list means "list all resources without any labels" + } + bool FindRequest::HasConstraints() const { return (!GetDicomTagConstraints().IsEmpty() || GetMetadataConstraintsCount() != 0 || - !GetLabels().empty() || + HasLabelsConstraint() || !GetOrdering().empty()); }
--- a/OrthancServer/Sources/Database/FindRequest.h Wed Jan 14 19:06:30 2026 +0100 +++ b/OrthancServer/Sources/Database/FindRequest.h Thu Jan 15 15:25:30 2026 +0100 @@ -465,6 +465,8 @@ bool HasConstraints() const; + bool HasLabelsConstraint() const; + bool IsTrivialFind(std::string& publicId /* out */) const; }; }
--- a/OrthancServer/Sources/Database/IDatabaseWrapper.h Wed Jan 14 19:06:30 2026 +0100 +++ b/OrthancServer/Sources/Database/IDatabaseWrapper.h Thu Jan 15 15:25:30 2026 +0100 @@ -497,7 +497,7 @@ }; - // TODO-FIND: Could this interface be removed? + // This interface is still used by old plugins that do not implement the ExtendedFind. class ICompatibilityTransaction : public boost::noncopyable { public:
--- a/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp Wed Jan 14 19:06:30 2026 +0100 +++ b/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.cpp Thu Jan 15 15:25:30 2026 +0100 @@ -441,6 +441,48 @@ s.Run(); } + virtual void ApplyLookupResources(std::list<std::string>& resourcesId, + std::list<std::string>* instancesId, + const DatabaseDicomTagConstraints& lookup, + ResourceType queryLevel, + const std::set<std::string>& labels, + LabelsConstraint labelsConstraint, + uint32_t limit) ORTHANC_OVERRIDE + { + LookupFormatter formatter; + + std::string sql; + LookupFormatter::Apply(sql, formatter, lookup, queryLevel, labels, labelsConstraint, limit); + + sql = "CREATE TEMPORARY TABLE Lookup AS " + sql; // TODO-FIND: use a CTE (or is this method obsolete ?) + + { + SQLite::Statement s(db_, SQLITE_FROM_HERE, "DROP TABLE IF EXISTS Lookup"); + s.Run(); + } + + { + SQLite::Statement statement(db_, sql); + formatter.Bind(statement); + statement.Run(); + } + + if (instancesId != NULL) + { + AnswerLookup(resourcesId, *instancesId, queryLevel); + } + else + { + resourcesId.clear(); + + SQLite::Statement s(db_, SQLITE_FROM_HERE, "SELECT publicId FROM Lookup"); + + while (s.Step()) + { + resourcesId.push_back(s.ColumnString(0)); + } + } + } #define C0_QUERY_ID 0 #define C1_INTERNAL_ID 1
--- a/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.h Wed Jan 14 19:06:30 2026 +0100 +++ b/OrthancServer/Sources/Database/SQLiteDatabaseWrapper.h Thu Jan 15 15:25:30 2026 +0100 @@ -103,7 +103,8 @@ virtual bool HasIntegratedFind() const ORTHANC_OVERRIDE { return true; // => This uses specialized SQL commands - //return false; // => This uses Compatibility/GenericFind + // return false; // => This uses Compatibility/GenericFind and this is useful to keep this implementation in SQLite to test the compatibility layer without using an external plugin ! + // hence the derivation from BaseCompatibilityTransaction } /** @@ -112,7 +113,7 @@ * "UnitTestsTransaction" give access to additional information * about the underlying SQLite database to be used in unit tests. **/ - class UnitTestsTransaction : public BaseCompatibilityTransaction // TODO: replace by IDatabaseWrapper::ITransaction and remove all compatibility methods from the SQLiteDatabaseWrapper ? + class UnitTestsTransaction : public BaseCompatibilityTransaction // We keep the compatibility transaction in SQLite in order to facilitate testing/development of the CompatibilityLayer (GenericFind) without using an external DB plugin { protected: SQLite::Connection& db_; @@ -145,16 +146,6 @@ const DicomTag& tag, const std::string& value); - virtual void ApplyLookupResources(std::list<std::string>& resourcesId, - std::list<std::string>* instancesId, - const DatabaseDicomTagConstraints& lookup, - ResourceType queryLevel, - const std::set<std::string>& labels, - LabelsConstraint labelsConstraint, - uint32_t limit) ORTHANC_OVERRIDE - { - throw OrthancException(ErrorCode_BadSequenceOfCalls); // this function is not supposed to be called with the SQLite engine - } }; }; }
--- a/OrthancServer/Sources/Search/ISqlLookupFormatter.cpp Wed Jan 14 19:06:30 2026 +0100 +++ b/OrthancServer/Sources/Search/ISqlLookupFormatter.cpp Thu Jan 15 15:25:30 2026 +0100 @@ -607,6 +607,125 @@ } + // Note: this is used only when disabling ExtendedFind in SQLite in order to validate the GenericFind compatibility layer + void ISqlLookupFormatter::Apply(std::string& sql, + ISqlLookupFormatter& formatter, + const DatabaseDicomTagConstraints& lookup, + ResourceType queryLevel, + const std::set<std::string>& labels, + LabelsConstraint labelsConstraint, + size_t limit) + { + // get the limit levels of the DICOM Tags lookup + ResourceType lowerLevel, upperLevel; + GetLookupLevels(lowerLevel, upperLevel, queryLevel, lookup); + + assert(upperLevel <= queryLevel && + queryLevel <= lowerLevel); + + const bool escapeBrackets = formatter.IsEscapeBrackets(); + + std::string joins, comparisons; + + size_t count = 0; + + for (size_t i = 0; i < lookup.GetSize(); i++) + { + const DatabaseDicomTagConstraint& constraint = lookup.GetConstraint(i); + + std::string comparison; + + if (FormatComparison(comparison, formatter, constraint, count, escapeBrackets)) + { + std::string join; + FormatJoin(join, constraint, count); + joins += join; + + if (!comparison.empty()) + { + comparisons += " AND " + comparison; + } + + count ++; + } + } + + sql = ("SELECT " + + FormatLevel(queryLevel) + ".publicId, " + + FormatLevel(queryLevel) + ".internalId" + + " FROM Resources AS " + FormatLevel(queryLevel)); + + for (int level = queryLevel - 1; level >= upperLevel; level--) + { + sql += (" INNER JOIN Resources " + + FormatLevel(static_cast<ResourceType>(level)) + " ON " + + FormatLevel(static_cast<ResourceType>(level)) + ".internalId=" + + FormatLevel(static_cast<ResourceType>(level + 1)) + ".parentId"); + } + + for (int level = queryLevel + 1; level <= lowerLevel; level++) + { + sql += (" INNER JOIN Resources " + + FormatLevel(static_cast<ResourceType>(level)) + " ON " + + FormatLevel(static_cast<ResourceType>(level - 1)) + ".internalId=" + + FormatLevel(static_cast<ResourceType>(level)) + ".parentId"); + } + + std::list<std::string> where; + where.push_back(FormatLevel(queryLevel) + ".resourceType = " + + formatter.FormatResourceType(queryLevel) + comparisons); + + if (!labels.empty()) + { + /** + * "In SQL Server, NOT EXISTS and NOT IN predicates are the best + * way to search for missing values, as long as both columns in + * question are NOT NULL." + * https://explainextended.com/2009/09/15/not-in-vs-not-exists-vs-left-join-is-null-sql-server/ + **/ + + std::list<std::string> formattedLabels; + for (std::set<std::string>::const_iterator it = labels.begin(); it != labels.end(); ++it) + { + formattedLabels.push_back(formatter.GenerateParameter(*it)); + } + + std::string condition; + switch (labelsConstraint) + { + case LabelsConstraint_Any: + condition = "> 0"; + break; + + case LabelsConstraint_All: + condition = "= " + boost::lexical_cast<std::string>(labels.size()); + break; + + case LabelsConstraint_None: + condition = "= 0"; + break; + + default: + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + + where.push_back("(SELECT COUNT(1) FROM Labels AS selectedLabels WHERE selectedLabels.id = " + FormatLevel(queryLevel) + + ".internalId AND selectedLabels.label IN (" + Join(formattedLabels, "", ", ") + ")) " + condition); + } + else if (labelsConstraint == LabelsConstraint_None) // from 1.12.11, 'None' with an empty labels list means "list all resources without any labels" + { + where.push_back("(SELECT COUNT(1) FROM Labels WHERE id = " + FormatLevel(queryLevel) + ".internalId) = 0"); + } + + sql += joins + Join(where, " WHERE ", " AND "); + + if (limit != 0) + { + sql += " LIMIT " + boost::lexical_cast<std::string>(limit); + } + } + + void ISqlLookupFormatter::Apply(std::string& sql, ISqlLookupFormatter& formatter, const FindRequest& request)
--- a/OrthancServer/Sources/Search/ISqlLookupFormatter.h Wed Jan 14 19:06:30 2026 +0100 +++ b/OrthancServer/Sources/Search/ISqlLookupFormatter.h Thu Jan 15 15:25:30 2026 +0100 @@ -68,6 +68,15 @@ const ResourceType& queryLevel, const DatabaseDicomTagConstraints& lookup); + // used only for the compatibility mode when disabling ExtendedFind for dev + static void Apply(std::string& sql, + ISqlLookupFormatter& formatter, + const DatabaseDicomTagConstraints& lookup, + ResourceType queryLevel, + const std::set<std::string>& labels, // New in Orthanc 1.12.0 + LabelsConstraint labelsConstraint, // New in Orthanc 1.12.0 + size_t limit); + static void ApplySingleLevel(std::string& sql, ISqlLookupFormatter& formatter, const DatabaseDicomTagConstraints& lookup,