Mercurial > hg > orthanc
changeset 5950:bfadfbcca13e default
tools/find: fix query by ModalitiesInStudy with pagination
| author | Alain Mazy <am@orthanc.team> |
|---|---|
| date | Wed, 08 Jan 2025 15:18:00 +0100 |
| parents | 81c78bbc9c97 |
| children | |
| files | NEWS OrthancServer/Sources/Database/MainDicomTagsRegistry.cpp OrthancServer/Sources/Database/MainDicomTagsRegistry.h OrthancServer/Sources/Database/PrepareDatabase.sql OrthancServer/Sources/OrthancFindRequestHandler.cpp OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp OrthancServer/Sources/OrthancWebDav.cpp OrthancServer/Sources/ResourceFinder.cpp OrthancServer/Sources/ResourceFinder.h OrthancServer/Sources/Search/ISqlLookupFormatter.cpp |
| diffstat | 10 files changed, 126 insertions(+), 64 deletions(-) [+] |
line wrap: on
line diff
--- a/NEWS Mon Jan 06 17:10:09 2025 +0100 +++ b/NEWS Wed Jan 08 15:18:00 2025 +0100 @@ -4,6 +4,9 @@ Maintenance ----------- +* Fix: With "ExtendedFind", using tools/find while querying against ModalitiesInStudy + was not compatible with pagination which prevented, amongst others, to use the + modality filter in OE2. * When the "HttpsCACertificates" configuration is empty. Orthanc will now use the operating system native CA store (if any). This is equivalent to the --ca-native curl option.
--- a/OrthancServer/Sources/Database/MainDicomTagsRegistry.cpp Mon Jan 06 17:10:09 2025 +0100 +++ b/OrthancServer/Sources/Database/MainDicomTagsRegistry.cpp Wed Jan 08 15:18:00 2025 +0100 @@ -98,11 +98,14 @@ } - bool MainDicomTagsRegistry::NormalizeLookup(DatabaseDicomTagConstraints& target, + bool MainDicomTagsRegistry::NormalizeLookup(bool& canBeFullyPerformedInDb, + DatabaseDicomTagConstraints& target, const DatabaseLookup& source, - ResourceType queryLevel) const + ResourceType queryLevel, + bool allowChildrenExistsQueries) const { bool isEquivalentLookup = true; + canBeFullyPerformedInDb = true; target.Clear(); @@ -110,8 +113,10 @@ { ResourceType level; DicomTagType type; + const DicomTag& tag = source.GetConstraint(i).GetTag(); - LookupTag(level, type, source.GetConstraint(i).GetTag()); + LookupTag(level, type, tag); + if (type == DicomTagType_Identifier || type == DicomTagType_Main) @@ -124,6 +129,13 @@ } bool isEquivalentConstraint; + + // DicomIdentifiers are stored UPPERCASE -> as soon as a case senstive search happens, it is currently not possible to perform it in DB only + if (type == DicomTagType_Identifier && source.GetConstraint(i).IsCaseSensitive()) + { + canBeFullyPerformedInDb = false; + } + target.AddConstraint(source.GetConstraint(i).ConvertToDatabaseConstraint(isEquivalentConstraint, level, type)); if (!isEquivalentConstraint) @@ -133,7 +145,26 @@ } else { - isEquivalentLookup = false; + if (allowChildrenExistsQueries && + tag == DICOM_TAG_MODALITIES_IN_STUDY && queryLevel == ResourceType_Study) + { + // transform the query into a children query + std::vector<std::string> values(source.GetConstraint(i).GetValues().begin(), source.GetConstraint(i).GetValues().end()); + + std::unique_ptr<DatabaseDicomTagConstraint> constraint(new DatabaseDicomTagConstraint(ResourceType_Series, + DICOM_TAG_MODALITY, + false, + ConstraintType_List, + values, + false, + true)); + target.AddConstraint(constraint.release()); + } + else + { + isEquivalentLookup = false; + canBeFullyPerformedInDb = false; + } } }
--- a/OrthancServer/Sources/Database/MainDicomTagsRegistry.h Mon Jan 06 17:10:09 2025 +0100 +++ b/OrthancServer/Sources/Database/MainDicomTagsRegistry.h Wed Jan 08 15:18:00 2025 +0100 @@ -82,8 +82,10 @@ * constraints are less strict than the original DatabaseLookup, * so more resources will match them. **/ - bool NormalizeLookup(DatabaseDicomTagConstraints& target, + bool NormalizeLookup(bool& canBeFullyPerformedInDb, + DatabaseDicomTagConstraints& target, const DatabaseLookup& source, - ResourceType queryLevel) const; + ResourceType queryLevel, + bool allowChildrenExistsQueries) const; }; }
--- a/OrthancServer/Sources/Database/PrepareDatabase.sql Mon Jan 06 17:10:09 2025 +0100 +++ b/OrthancServer/Sources/Database/PrepareDatabase.sql Wed Jan 08 15:18:00 2025 +0100 @@ -40,6 +40,9 @@ ); -- The following table was added in Orthanc 0.8.5 (database v5) +-- It contains only the DICOM Tags that are commonly used for searches. +-- All these tags are converted to UPPERCASE ! +-- These tags are also stored in the MainDicomTags table without casing modificiation. CREATE TABLE DicomIdentifiers( id INTEGER REFERENCES Resources(internalId) ON DELETE CASCADE, tagGroup INTEGER,
--- a/OrthancServer/Sources/OrthancFindRequestHandler.cpp Mon Jan 06 17:10:09 2025 +0100 +++ b/OrthancServer/Sources/OrthancFindRequestHandler.cpp Wed Jan 08 15:18:00 2025 +0100 @@ -444,7 +444,7 @@ * Run the query. **/ - ResourceFinder finder(level, ResponseContentFlags_ID, context_.GetFindStorageAccessMode()); + ResourceFinder finder(level, ResponseContentFlags_ID, context_.GetFindStorageAccessMode(), context_.GetIndex().HasFindSupport()); finder.SetDatabaseLookup(lookup); finder.AddRequestedTags(requestedTags);
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp Mon Jan 06 17:10:09 2025 +0100 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestResources.cpp Wed Jan 08 15:18:00 2025 +0100 @@ -154,7 +154,7 @@ responseContent = static_cast<ResponseContentFlags>(static_cast<uint32_t>(responseContent) | ResponseContentFlags_Metadata); } - ResourceFinder finder(level, responseContent, context.GetFindStorageAccessMode()); + ResourceFinder finder(level, responseContent, context.GetFindStorageAccessMode(), context.GetIndex().HasFindSupport()); finder.SetOrthancId(level, identifier); finder.SetRetrieveMetadata(retrieveMetadata); @@ -194,7 +194,10 @@ OrthancRestApi::GetRequestedTags(requestedTags, call); OrthancRestApi::GetResponseContentAndExpand(responseContent, call); - ResourceFinder finder(resourceType, responseContent, OrthancRestApi::GetContext(call).GetFindStorageAccessMode()); + ResourceFinder finder(resourceType, + responseContent, + OrthancRestApi::GetContext(call).GetFindStorageAccessMode(), + OrthancRestApi::GetContext(call).GetIndex().HasFindSupport()); finder.AddRequestedTags(requestedTags); if (call.HasArgument("limit") || @@ -252,7 +255,10 @@ const DicomToJsonFormat format = OrthancRestApi::GetDicomFormat(call, DicomToJsonFormat_Human); - ResourceFinder finder(resourceType, ResponseContentFlags_ExpandTrue, OrthancRestApi::GetContext(call).GetFindStorageAccessMode()); + ResourceFinder finder(resourceType, + ResponseContentFlags_ExpandTrue, + OrthancRestApi::GetContext(call).GetFindStorageAccessMode(), + OrthancRestApi::GetContext(call).GetIndex().HasFindSupport()); finder.AddRequestedTags(requestedTags); finder.SetOrthancId(resourceType, call.GetUriComponent("id", "")); @@ -3072,25 +3078,6 @@ FindType_Count }; - static bool CanBePerformedInDb(const DatabaseLookup& lookup) - { - std::set<DicomTag> nonMainDicomTags; - if (!lookup.HasOnlyMainDicomTags(nonMainDicomTags)) - { - // TODO: prepared work for https://github.com/orthanc-server/orthanc-explorer-2/issues/73 - - // // filtering on ModalitiesInStudy is actually performed in DB too although the tag is not stored in the MainDicomTags - // if (nonMainDicomTags.size() == 1 && nonMainDicomTags.find(DICOM_TAG_MODALITIES_IN_STUDY) != nonMainDicomTags.end()) - // { - // return true; - // } - - return false; - } - - return true; - } - template <enum FindType requestType> static void Find(RestApiPostCall& call) @@ -3292,7 +3279,7 @@ const ResourceType level = StringToResourceType(request[KEY_LEVEL].asCString()); - ResourceFinder finder(level, responseContent, context.GetFindStorageAccessMode()); + ResourceFinder finder(level, responseContent, context.GetFindStorageAccessMode(), context.GetIndex().HasFindSupport()); DatabaseLookup dicomTagLookup; @@ -3339,12 +3326,6 @@ } } - if (requestType == FindType_Count && !CanBePerformedInDb(dicomTagLookup)) - { - throw OrthancException(ErrorCode_BadRequest, - "Unable to count resources when querying tags that are not stored as MainDicomTags in the Database"); - } - finder.SetDatabaseLookup(dicomTagLookup); } @@ -3444,12 +3425,11 @@ finder.SetDatabaseLimits(context.GetDatabaseLimits(level)); - if (((request.isMember(KEY_LIMIT) && request[KEY_LIMIT].asInt64() != 0) || - (request.isMember(KEY_SINCE) && request[KEY_SINCE].asInt64() != 0)) && - !CanBePerformedInDb(dicomTagLookup)) + if ((request.isMember(KEY_SINCE) && request[KEY_SINCE].asInt64() != 0) && + !finder.CanBeFullyPerformedInDb()) { throw OrthancException(ErrorCode_BadRequest, - "Unable to use " + std::string(KEY_LIMIT) + " or " + std::string(KEY_SINCE) + " in tools/find when querying tags that are not stored as MainDicomTags in the Database"); + "Unable to use '" + std::string(KEY_SINCE) + "' in tools/find when querying tags that are not stored as MainDicomTags in the Database"); } if (request.isMember(KEY_LIMIT)) @@ -3625,7 +3605,10 @@ std::set<DicomTag> requestedTags; OrthancRestApi::GetRequestedTags(requestedTags, call); - ResourceFinder finder(end, (expand ? ResponseContentFlags_ExpandTrue : ResponseContentFlags_ID), OrthancRestApi::GetContext(call).GetFindStorageAccessMode()); + ResourceFinder finder(end, + (expand ? ResponseContentFlags_ExpandTrue : ResponseContentFlags_ID), + OrthancRestApi::GetContext(call).GetFindStorageAccessMode(), + OrthancRestApi::GetContext(call).GetIndex().HasFindSupport()); finder.SetOrthancId(start, call.GetUriComponent("id", "")); finder.AddRequestedTags(requestedTags);
--- a/OrthancServer/Sources/OrthancWebDav.cpp Mon Jan 06 17:10:09 2025 +0100 +++ b/OrthancServer/Sources/OrthancWebDav.cpp Wed Jan 08 15:18:00 2025 +0100 @@ -938,7 +938,7 @@ Visitor visitor(resources); - ResourceFinder finder(ResourceType_Study, ResponseContentFlags_ID, GetContext().GetFindStorageAccessMode()); + ResourceFinder finder(ResourceType_Study, ResponseContentFlags_ID, GetContext().GetFindStorageAccessMode(), GetContext().GetIndex().HasFindSupport()); finder.SetDatabaseLookup(query); finder.Execute(visitor, GetContext()); } @@ -1016,7 +1016,7 @@ Visitor visitor; - ResourceFinder finder(ResourceType_Study, ResponseContentFlags_ID, context_.GetFindStorageAccessMode()); + ResourceFinder finder(ResourceType_Study, ResponseContentFlags_ID, context_.GetFindStorageAccessMode(), context_.GetIndex().HasFindSupport()); finder.SetDatabaseLookup(query); finder.Execute(visitor, context_); @@ -1394,7 +1394,7 @@ return false; } - ResourceFinder finder(level, ResponseContentFlags_ID, context_.GetFindStorageAccessMode()); + ResourceFinder finder(level, ResponseContentFlags_ID, context_.GetFindStorageAccessMode(), context_.GetIndex().HasFindSupport()); finder.SetDatabaseLookup(query); finder.SetRetrieveMetadata(true); @@ -1445,7 +1445,7 @@ ResourceType level, const DatabaseLookup& query) { - ResourceFinder finder(level, ResponseContentFlags_ExpandTrue, context.GetFindStorageAccessMode()); + ResourceFinder finder(level, ResponseContentFlags_ExpandTrue, context.GetFindStorageAccessMode(), context.GetIndex().HasFindSupport()); finder.SetDatabaseLookup(query); Json::Value expanded; @@ -1515,7 +1515,7 @@ mime = MimeType_Dicom; - ResourceFinder finder(ResourceType_Instance, ResponseContentFlags_ID, context_.GetFindStorageAccessMode()); + ResourceFinder finder(ResourceType_Instance, ResponseContentFlags_ID, context_.GetFindStorageAccessMode(), context_.GetIndex().HasFindSupport()); finder.SetDatabaseLookup(query); finder.SetRetrieveMetadata(true); finder.SetRetrieveAttachments(true); @@ -1645,7 +1645,7 @@ DicomDeleteVisitor visitor(context_, level); - ResourceFinder finder(level, ResponseContentFlags_ID, context_.GetFindStorageAccessMode()); + ResourceFinder finder(level, ResponseContentFlags_ID, context_.GetFindStorageAccessMode(), context_.GetIndex().HasFindSupport()); finder.SetDatabaseLookup(query); finder.Execute(visitor, context_); return true;
--- a/OrthancServer/Sources/ResourceFinder.cpp Mon Jan 06 17:10:09 2025 +0100 +++ b/OrthancServer/Sources/ResourceFinder.cpp Wed Jan 08 15:18:00 2025 +0100 @@ -517,7 +517,7 @@ } if (lookup_.get() != NULL && - isSimpleLookup_ && + canBeFullyPerformedInDb_ && (hasLimitsSince_ || hasLimitsCount_)) { /** @@ -544,10 +544,12 @@ ResourceFinder::ResourceFinder(ResourceType level, ResponseContentFlags responseContent, - FindStorageAccessMode storageAccessMode) : + FindStorageAccessMode storageAccessMode, + bool supportsChildExistQueries) : request_(level), databaseLimits_(0), isSimpleLookup_(true), + canBeFullyPerformedInDb_(true), pagingMode_(PagingMode_FullManual), hasLimitsSince_(false), hasLimitsCount_(false), @@ -555,6 +557,7 @@ limitsCount_(0), responseContent_(responseContent), storageAccessMode_(storageAccessMode), + supportsChildExistQueries_(supportsChildExistQueries), isWarning002Enabled_(false), isWarning004Enabled_(false), isWarning005Enabled_(false) @@ -664,7 +667,7 @@ } } - isSimpleLookup_ = registry.NormalizeLookup(request_.GetDicomTagConstraints(), lookup, request_.GetLevel()); + isSimpleLookup_ = registry.NormalizeLookup(canBeFullyPerformedInDb_, request_.GetDicomTagConstraints(), lookup, request_.GetLevel(), supportsChildExistQueries_); // "request_.GetDicomTagConstraints()" only contains constraints on main DICOM tags @@ -681,12 +684,15 @@ } else { - LOG(WARNING) << "Executing a database lookup at level " << EnumerationToString(request_.GetLevel()) - << " on main DICOM tag " << constraint.GetTag().Format() << " from an inferior level (" - << EnumerationToString(constraint.GetLevel()) << "), this will return no result"; + if (!supportsChildExistQueries_ || (constraint.GetLevel() != ResourceType_Series && constraint.GetTag() != DICOM_TAG_MODALITIES_IN_STUDY)) + { + LOG(WARNING) << "Executing a database lookup at level " << EnumerationToString(request_.GetLevel()) + << " on main DICOM tag " << constraint.GetTag().Format() << " from an inferior level (" + << EnumerationToString(constraint.GetLevel()) << "), this will return no result"; + } } - if (IsComputedTag(constraint.GetTag())) + if (IsComputedTag(constraint.GetTag()) && constraint.GetTag() != DICOM_TAG_MODALITIES_IN_STUDY) { // Sanity check throw OrthancException(ErrorCode_InternalError); @@ -1028,6 +1034,12 @@ uint64_t ResourceFinder::Count(ServerContext& context) const { + if (!canBeFullyPerformedInDb_) + { + throw OrthancException(ErrorCode_BadRequest, + "Unable to count resources when querying tags that are not stored as MainDicomTags in the Database or when using case sensitive queries."); + } + uint64_t count = 0; context.GetIndex().ExecuteCount(count, request_); return count; @@ -1039,6 +1051,13 @@ { UpdateRequestLimits(context); + if ((request_.HasLimits() && request_.GetLimitsSince() > 0) && + !canBeFullyPerformedInDb_) + { + throw OrthancException(ErrorCode_BadRequest, + "nable to use 'Since' when finding resources when querying against Dicom Tags that are not in the MainDicomTags or when using CaseSenstive queries."); + } + bool isWarning002Enabled = false; bool isWarning004Enabled = false; bool isWarning006Enabled = false;
--- a/OrthancServer/Sources/ResourceFinder.h Mon Jan 06 17:10:09 2025 +0100 +++ b/OrthancServer/Sources/ResourceFinder.h Wed Jan 08 15:18:00 2025 +0100 @@ -60,6 +60,7 @@ uint64_t databaseLimits_; std::unique_ptr<DatabaseLookup> lookup_; bool isSimpleLookup_; + bool canBeFullyPerformedInDb_; PagingMode pagingMode_; bool hasLimitsSince_; bool hasLimitsCount_; @@ -67,6 +68,7 @@ uint64_t limitsCount_; ResponseContentFlags responseContent_; FindStorageAccessMode storageAccessMode_; + bool supportsChildExistQueries_; std::set<DicomTag> requestedTags_; std::set<DicomTag> requestedComputedTags_; @@ -106,7 +108,8 @@ public: ResourceFinder(ResourceType level, ResponseContentFlags responseContent, - FindStorageAccessMode storageAccessMode); + FindStorageAccessMode storageAccessMode, + bool supportsChildExistQueries); void SetDatabaseLimits(uint64_t limits); @@ -195,5 +198,10 @@ bool includeAllMetadata); uint64_t Count(ServerContext& context) const; + + bool CanBeFullyPerformedInDb() const + { + return canBeFullyPerformedInDb_; + } }; }
--- a/OrthancServer/Sources/Search/ISqlLookupFormatter.cpp Mon Jan 06 17:10:09 2025 +0100 +++ b/OrthancServer/Sources/Search/ISqlLookupFormatter.cpp Wed Jan 08 15:18:00 2025 +0100 @@ -856,13 +856,25 @@ { std::string join; FormatJoin(join, constraint, count); - joins += join; + + if (constraint.GetLevel() <= queryLevel) + { + joins += join; + } + else if (constraint.GetLevel() == queryLevel + 1 && !comparison.empty()) + { + // new in v 1.12.6, the constraints on child tags are actually looking for one child with this value + comparison = " EXISTS (SELECT 1 FROM Resources AS " + FormatLevel(static_cast<ResourceType>(queryLevel + 1)) + + join + + " WHERE " + comparison + " AND " + + FormatLevel(static_cast<ResourceType>(queryLevel + 1)) + ".parentId = " + FormatLevel(static_cast<ResourceType>(queryLevel)) + ".internalId) "; + } if (!comparison.empty()) { comparisons += " AND " + comparison; } - + count ++; } } @@ -894,13 +906,14 @@ 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"); - } + // disabled in v 1.12.6 now that the child levels are considered as "is there at least one child that meets this constraint" + // 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(strQueryLevel + ".resourceType = " +