Mercurial > hg > orthanc-dicomweb
changeset 253:172ddccbbc1a
Fix issue #4 (QIDO-RS: wrong serialization of number values)
author | Sebastien Jodogne <s.jodogne@gmail.com> |
---|---|
date | Tue, 26 Feb 2019 17:33:07 +0100 |
parents | 13b8e4c3c5bd |
children | eb9f94754cb2 |
files | CMakeLists.txt NEWS Plugin/DicomResults.cpp Plugin/DicomResults.h Plugin/DicomWebFormatter.cpp Plugin/DicomWebFormatter.h Plugin/QidoRs.cpp Plugin/WadoRs.cpp |
diffstat | 8 files changed, 65 insertions(+), 537 deletions(-) [+] |
line wrap: on
line diff
--- a/CMakeLists.txt Tue Feb 26 17:05:30 2019 +0100 +++ b/CMakeLists.txt Tue Feb 26 17:33:07 2019 +0100 @@ -120,7 +120,6 @@ set(CORE_SOURCES Plugin/Configuration.cpp Plugin/Dicom.cpp - Plugin/DicomResults.cpp ${ORTHANC_ROOT}/Plugins/Samples/Common/OrthancPluginCppWrapper.cpp ${ORTHANC_CORE_SOURCES}
--- a/NEWS Tue Feb 26 17:05:30 2019 +0100 +++ b/NEWS Tue Feb 26 17:33:07 2019 +0100 @@ -6,6 +6,7 @@ * Sending "HttpHeaders" of the "DicomWeb.Servers" configuration to remote DICOMweb servers * Improved WADO-RS compatibility if Orthanc is acting as a DICOMweb client * More detailed information about errors is provided in the HTTP answers +* Fix issue #112/4 (QIDO-RS: wrong serialization of number values) * Upgrade to GDCM 2.8.8 for static builds
--- a/Plugin/DicomResults.cpp Tue Feb 26 17:05:30 2019 +0100 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,426 +0,0 @@ -/** - * Orthanc - A Lightweight, RESTful DICOM Store - * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics - * Department, University Hospital of Liege, Belgium - * Copyright (C) 2017-2019 Osimis S.A., Belgium - * - * This program is free software: you can redistribute it and/or - * modify it under the terms of the GNU Affero General Public License - * as published by the Free Software Foundation, either version 3 of - * the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see <http://www.gnu.org/licenses/>. - **/ - - -#include "DicomResults.h" - -#include "Dicom.h" - -#include <Core/Toolbox.h> -#include <Plugins/Samples/Common/OrthancPluginCppWrapper.h> - -#include <boost/lexical_cast.hpp> -#include <boost/noncopyable.hpp> - -namespace OrthancPlugins -{ - DicomResults::DicomResults(OrthancPluginRestOutput* output, - const std::string& wadoBase, - const gdcm::Dict& dictionary, - bool isXml, - bool isBulkAccessible) : - output_(output), - wadoBase_(wadoBase), - dictionary_(dictionary), - isFirst_(true), - isXml_(isXml), - isBulkAccessible_(isBulkAccessible) - { - if (isXml_ && - OrthancPluginStartMultipartAnswer(GetGlobalContext(), output_, - "related", "application/dicom+xml") != 0) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_NetworkProtocol, - "Unable to create a multipart stream of DICOM+XML answers"); - } - - jsonWriter_.AddChunk("[\n"); - } - - - void DicomResults::AddInternal(const std::string& item) - { - if (isXml_) - { - if (OrthancPluginSendMultipartItem(GetGlobalContext(), output_, item.c_str(), item.size()) != 0) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_NetworkProtocol, - "Unable to create a multipart stream of DICOM+XML answers"); - } - } - else - { - if (!isFirst_) - { - jsonWriter_.AddChunk(",\n"); - } - - jsonWriter_.AddChunk(item); - } - - isFirst_ = false; - } - - - void DicomResults::AddInternal(const gdcm::DataSet& dicom) - { - std::string item; - - if (isXml_) - { - GenerateSingleDicomAnswer(item, wadoBase_, dictionary_, dicom, true, isBulkAccessible_); - } - else - { - GenerateSingleDicomAnswer(item, wadoBase_, dictionary_, dicom, false, isBulkAccessible_); - } - - AddInternal(item); - - isFirst_ = false; - } - - - - namespace - { - class ITagVisitor : public boost::noncopyable - { - public: - virtual ~ITagVisitor() - { - } - - virtual void Visit(const gdcm::Tag& tag, - bool isSequence, - const std::string& vr, - const std::string& type, - const Json::Value& value) = 0; - - static void Apply(ITagVisitor& visitor, - const Json::Value& source, - const gdcm::Dict& dictionary) - { - if (source.type() != Json::objectValue) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); - } - - Json::Value::Members members = source.getMemberNames(); - for (size_t i = 0; i < members.size(); i++) - { - if (members[i].size() != 9 || - members[i][4] != ',' || - source[members[i]].type() != Json::objectValue || - !source[members[i]].isMember("Value") || - !source[members[i]].isMember("Type") || - source[members[i]]["Type"].type() != Json::stringValue) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); - } - - const Json::Value& value = source[members[i]]["Value"]; - const std::string type = source[members[i]]["Type"].asString(); - - gdcm::Tag tag(OrthancPlugins::ParseTag(dictionary, members[i])); - - bool isSequence = false; - std::string vr = GetVRName(isSequence, dictionary, tag); - - if (tag == DICOM_TAG_RETRIEVE_URL) - { - // The VR of this attribute has changed from UT to UR. - vr = "UR"; - } - else - { - vr = GetVRName(isSequence, dictionary, tag); - } - - visitor.Visit(tag, isSequence, vr, type, value); - } - } - }; - - - class TagVisitorBase : public ITagVisitor - { - protected: - const Json::Value& source_; - const gdcm::Dict& dictionary_; - const std::string& bulkUri_; - - public: - TagVisitorBase(const Json::Value& source, - const gdcm::Dict& dictionary, - const std::string& bulkUri) : - source_(source), - dictionary_(dictionary), - bulkUri_(bulkUri) - { - } - }; - - - class JsonVisitor : public TagVisitorBase - { - private: - Json::Value& target_; - - public: - JsonVisitor(Json::Value& target, - const Json::Value& source, - const gdcm::Dict& dictionary, - const std::string& bulkUri) : - TagVisitorBase(source, dictionary, bulkUri), - target_(target) - { - target_ = Json::objectValue; - } - - virtual void Visit(const gdcm::Tag& tag, - bool isSequence, - const std::string& vr, - const std::string& type, - const Json::Value& value) - { - const std::string formattedTag = OrthancPlugins::FormatTag(tag); - - Json::Value node = Json::objectValue; - node["vr"] = vr; - - bool ok = false; - if (isSequence) - { - // Deal with sequences - if (type != "Sequence" || - value.type() != Json::arrayValue) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); - } - - node["Value"] = Json::arrayValue; - - for (Json::Value::ArrayIndex i = 0; i < value.size(); i++) - { - if (value[i].type() != Json::objectValue) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); - } - - Json::Value child; - - std::string childUri; - if (!bulkUri_.empty()) - { - std::string number = boost::lexical_cast<std::string>(i); - childUri = bulkUri_ + formattedTag + "/" + number + "/"; - } - - JsonVisitor visitor(child, value[i], dictionary_, childUri); - JsonVisitor::Apply(visitor, value[i], dictionary_); - - node["Value"].append(child); - } - - ok = true; - } - else if (type == "String" && - value.type() == Json::stringValue) - { - // Deal with string representations - node["Value"] = Json::arrayValue; - node["Value"].append(value.asString()); - ok = true; - } - else - { - // Bulk data - if (!bulkUri_.empty()) - { - node["BulkDataURI"] = bulkUri_ + formattedTag; - ok = true; - } - } - - if (ok) - { - target_[formattedTag] = node; - } - } - }; - - - class XmlVisitor : public TagVisitorBase - { - private: - pugi::xml_node& target_; - - public: - XmlVisitor(pugi::xml_node& target, - const Json::Value& source, - const gdcm::Dict& dictionary, - const std::string& bulkUri) : - TagVisitorBase(source, dictionary, bulkUri), - target_(target) - { - } - - virtual void Visit(const gdcm::Tag& tag, - bool isSequence, - const std::string& vr, - const std::string& type, - const Json::Value& value) - { - const std::string formattedTag = OrthancPlugins::FormatTag(tag); - - pugi::xml_node node = target_.append_child("DicomAttribute"); - node.append_attribute("tag").set_value(formattedTag.c_str()); - node.append_attribute("vr").set_value(vr.c_str()); - - const char* keyword = GetKeyword(dictionary_, tag); - if (keyword != NULL) - { - node.append_attribute("keyword").set_value(keyword); - } - - if (isSequence) - { - // Deal with sequences - if (type != "Sequence" || - value.type() != Json::arrayValue) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); - } - - for (Json::Value::ArrayIndex i = 0; i < value.size(); i++) - { - if (value[i].type() != Json::objectValue) - { - throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError); - } - - pugi::xml_node child = node.append_child("Item"); - std::string number = boost::lexical_cast<std::string>(i + 1); - child.append_attribute("number").set_value(number.c_str()); - - std::string childUri; - if (!bulkUri_.empty()) - { - childUri = bulkUri_ + formattedTag + "/" + number + "/"; - } - - XmlVisitor visitor(child, value[i], dictionary_, childUri); - XmlVisitor::Apply(visitor, value[i], dictionary_); - } - } - else if (type == "String" && - value.type() == Json::stringValue) - { - // Deal with string representations - pugi::xml_node item = node.append_child("Value"); - item.append_attribute("number").set_value("1"); - item.append_child(pugi::node_pcdata).set_value(value.asCString()); - } - else - { - // Bulk data - if (!bulkUri_.empty()) - { - pugi::xml_node value = node.append_child("BulkData"); - std::string uri = bulkUri_ + formattedTag; - value.append_attribute("uri").set_value(uri.c_str()); - } - } - } - }; - } - - - static void OrthancToDicomWebXml(pugi::xml_document& target, - const Json::Value& source, - const gdcm::Dict& dictionary, - const std::string& bulkUriRoot) - { - pugi::xml_node root = target.append_child("NativeDicomModel"); - root.append_attribute("xmlns").set_value("http://dicom.nema.org/PS3.19/models/NativeDICOM"); - root.append_attribute("xsi:schemaLocation").set_value("http://dicom.nema.org/PS3.19/models/NativeDICOM"); - root.append_attribute("xmlns:xsi").set_value("http://www.w3.org/2001/XMLSchema-instance"); - - XmlVisitor visitor(root, source, dictionary, bulkUriRoot); - ITagVisitor::Apply(visitor, source, dictionary); - - pugi::xml_node decl = target.prepend_child(pugi::node_declaration); - decl.append_attribute("version").set_value("1.0"); - decl.append_attribute("encoding").set_value("utf-8"); - } - - - void DicomResults::AddFromOrthanc(const Json::Value& dicom, - const std::string& wadoUrl) - { - std::string bulkUriRoot; - if (isBulkAccessible_) - { - bulkUriRoot = wadoUrl + "bulk/"; - } - - if (isXml_) - { - pugi::xml_document doc; - OrthancToDicomWebXml(doc, dicom, dictionary_, bulkUriRoot); - - ChunkedBufferWriter writer; - doc.save(writer, " ", pugi::format_default, pugi::encoding_utf8); - - std::string item; - writer.Flatten(item); - - AddInternal(item); - } - else - { - Json::Value v; - JsonVisitor visitor(v, dicom, dictionary_, bulkUriRoot); - ITagVisitor::Apply(visitor, dicom, dictionary_); - - Json::FastWriter writer; - AddInternal(writer.write(v)); - } - } - - - void DicomResults::Answer() - { - if (isXml_) - { - // Nothing to do in this case - } - else - { - jsonWriter_.AddChunk("]\n"); - - std::string answer; - jsonWriter_.Flatten(answer); - OrthancPluginAnswerBuffer(GetGlobalContext(), output_, answer.c_str(), - answer.size(), "application/dicom+json"); - } - } -}
--- a/Plugin/DicomResults.h Tue Feb 26 17:05:30 2019 +0100 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,71 +0,0 @@ -/** - * Orthanc - A Lightweight, RESTful DICOM Store - * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics - * Department, University Hospital of Liege, Belgium - * Copyright (C) 2017-2019 Osimis S.A., Belgium - * - * This program is free software: you can redistribute it and/or - * modify it under the terms of the GNU Affero General Public License - * as published by the Free Software Foundation, either version 3 of - * the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see <http://www.gnu.org/licenses/>. - **/ - - -#pragma once - -#include "ChunkedBuffer.h" - -#include <orthanc/OrthancCPlugin.h> -#include <gdcmDataSet.h> -#include <gdcmDict.h> -#include <gdcmFile.h> -#include <json/value.h> - -namespace OrthancPlugins -{ - class DicomResults - { - private: - OrthancPluginRestOutput* output_; - std::string wadoBase_; - const gdcm::Dict& dictionary_; - Orthanc::ChunkedBuffer jsonWriter_; // Used for JSON output - bool isFirst_; - bool isXml_; - bool isBulkAccessible_; - - void AddInternal(const std::string& item); - - void AddInternal(const gdcm::DataSet& dicom); - - public: - DicomResults(OrthancPluginRestOutput* output, - const std::string& wadoBase, - const gdcm::Dict& dictionary, - bool isXml, - bool isBulkAccessible); - - void Add(const gdcm::File& file) - { - AddInternal(file.GetDataSet()); - } - - void Add(const gdcm::DataSet& subset) - { - AddInternal(subset); - } - - void AddFromOrthanc(const Json::Value& dicom, - const std::string& wadoUrl); - - void Answer(); - }; -}
--- a/Plugin/DicomWebFormatter.cpp Tue Feb 26 17:05:30 2019 +0100 +++ b/Plugin/DicomWebFormatter.cpp Tue Feb 26 17:33:07 2019 +0100 @@ -128,7 +128,7 @@ } - void DicomWebFormatter::HttpWriter::AddRawDicom(const void* dicom, + void DicomWebFormatter::HttpWriter::AddInternal(const void* dicom, size_t size, OrthancPluginDicomWebBinaryMode mode, const std::string& bulkRoot) @@ -159,6 +159,15 @@ } } + + void DicomWebFormatter::HttpWriter::AddJson(const Json::Value& value) + { + MemoryBuffer dicom; + dicom.CreateDicom(value, OrthancPluginCreateDicomFlags_None); + + AddInternal(dicom.GetData(), dicom.GetSize(), OrthancPluginDicomWebBinaryMode_Ignore, ""); + } + void DicomWebFormatter::HttpWriter::Send() {
--- a/Plugin/DicomWebFormatter.h Tue Feb 26 17:05:30 2019 +0100 +++ b/Plugin/DicomWebFormatter.h Tue Feb 26 17:33:07 2019 +0100 @@ -25,6 +25,8 @@ #include <orthanc/OrthancCPlugin.h> +#include <json/value.h> + #include <boost/noncopyable.hpp> #include <boost/thread/mutex.hpp> @@ -81,14 +83,23 @@ bool first_; Orthanc::ChunkedBuffer jsonBuffer_; + void AddInternal(const void* dicom, + size_t size, + OrthancPluginDicomWebBinaryMode mode, + const std::string& bulkRoot); + public: HttpWriter(OrthancPluginRestOutput* output, bool isXml); - void AddRawDicom(const void* dicom, - size_t size, - OrthancPluginDicomWebBinaryMode mode, - const std::string& bulkRoot); + void AddDicom(const void* dicom, + size_t size, + const std::string& bulkRoot) + { + AddInternal(dicom, size, OrthancPluginDicomWebBinaryMode_BulkDataUri, bulkRoot); + } + + void AddJson(const Json::Value& value); void Send(); };
--- a/Plugin/QidoRs.cpp Tue Feb 26 17:05:30 2019 +0100 +++ b/Plugin/QidoRs.cpp Tue Feb 26 17:33:07 2019 +0100 @@ -24,8 +24,8 @@ #include "Plugin.h" #include "StowRs.h" // For IsXmlExpected() #include "Dicom.h" -#include "DicomResults.h" #include "Configuration.h" +#include "DicomWebFormatter.h" #include <Core/DicomFormat/DicomTag.h> #include <Core/Toolbox.h> @@ -56,16 +56,29 @@ const Orthanc::DicomTag& tag, const std::string& defaultValue) { - std::string s = FormatOrthancTag(tag); - - if (source.isMember(s) && - source[s].type() == Json::objectValue && - source[s].isMember("Value") && - source[s].isMember("Type") && - source[s]["Type"] == "String" && - source[s]["Value"].type() == Json::stringValue) + const std::string s = tag.Format(); + + if (source.isMember(s)) { - return source[s]["Value"].asString(); + switch (source[s].type()) + { + case Json::stringValue: + return source[s].asString(); + + // The conversions below should *not* be necessary + + case Json::intValue: + return boost::lexical_cast<std::string>(source[s].asInt()); + + case Json::uintValue: + return boost::lexical_cast<std::string>(source[s].asUInt()); + + case Json::realValue: + return boost::lexical_cast<std::string>(source[s].asFloat()); + + default: + return defaultValue; + } } else { @@ -95,7 +108,7 @@ { case Orthanc::ResourceType_Study: // http://medical.nema.org/medical/dicom/current/output/html/part18.html#table_6.7.1-2 - result.push_back(Orthanc::DicomTag(0x0008, 0x0005)); // Specific Character Set + //result.push_back(Orthanc::DicomTag(0x0008, 0x0005)); // Specific Character Set => SPECIAL CASE result.push_back(Orthanc::DicomTag(0x0008, 0x0020)); // Study Date result.push_back(Orthanc::DicomTag(0x0008, 0x0030)); // Study Time result.push_back(Orthanc::DicomTag(0x0008, 0x0050)); // Accession Number @@ -116,7 +129,7 @@ case Orthanc::ResourceType_Series: // http://medical.nema.org/medical/dicom/current/output/html/part18.html#table_6.7.1-2a - result.push_back(Orthanc::DicomTag(0x0008, 0x0005)); // Specific Character Set + //result.push_back(Orthanc::DicomTag(0x0008, 0x0005)); // Specific Character Set => SPECIAL CASE result.push_back(Orthanc::DicomTag(0x0008, 0x0060)); // Modality result.push_back(Orthanc::DicomTag(0x0008, 0x0201)); // Timezone Offset From UTC result.push_back(Orthanc::DicomTag(0x0008, 0x103E)); // Series Description @@ -131,7 +144,7 @@ case Orthanc::ResourceType_Instance: // http://medical.nema.org/medical/dicom/current/output/html/part18.html#table_6.7.1-2b - result.push_back(Orthanc::DicomTag(0x0008, 0x0005)); // Specific Character Set + //result.push_back(Orthanc::DicomTag(0x0008, 0x0005)); // Specific Character Set => SPECIAL CASE result.push_back(Orthanc::DicomTag(0x0008, 0x0016)); // SOP Class UID result.push_back(Orthanc::DicomTag(0x0008, 0x0018)); // SOP Instance UID result.push_back(Orthanc::DicomTag(0x0008, 0x0056)); // Instance Availability @@ -496,13 +509,8 @@ url += "/instances/" + GetOrthancTag(source, Orthanc::DICOM_TAG_SOP_INSTANCE_UID, ""); } - Json::Value tmp = Json::objectValue; - tmp["Name"] = "RetrieveURL"; - tmp["Type"] = "String"; - tmp["Value"] = url; - static const Orthanc::DicomTag DICOM_TAG_RETRIEVE_URL(0x0008, 0x1190); - result[FormatOrthancTag(DICOM_TAG_RETRIEVE_URL)] = tmp; + result[DICOM_TAG_RETRIEVE_URL.Format()] = url; } }; } @@ -517,8 +525,12 @@ Json::Value find; matcher.ConvertToOrthanc(find, level); - Json::FastWriter writer; - std::string body = writer.write(find); + std::string body; + + { + Json::FastWriter writer; + body = writer.write(find); + } Json::Value resources; if (!OrthancPlugins::RestApiPost(resources, "/tools/find", body, false) || @@ -556,14 +568,14 @@ std::string wadoBase = OrthancPlugins::Configuration::GetBaseUrl(request); - OrthancPlugins::DicomResults results(output, wadoBase, *dictionary_, IsXmlExpected(request), true); + OrthancPlugins::DicomWebFormatter::HttpWriter writer(output, IsXmlExpected(request)); // Fix of issue #13 for (ResourcesAndInstances::const_iterator it = resourcesAndInstances.begin(); it != resourcesAndInstances.end(); ++it) { Json::Value tags; - if (OrthancPlugins::RestApiGet(tags, "/instances/" + it->second + "/tags", false)) + if (OrthancPlugins::RestApiGet(tags, "/instances/" + it->second + "/tags?short", false)) { std::string wadoUrl = OrthancPlugins::Configuration::GetWadoUrl( wadoBase, @@ -581,20 +593,14 @@ for (ModuleMatcher::Filters::const_iterator tag = derivedTags.begin(); tag != derivedTags.end(); ++tag) { - const gdcm::Tag t(tag->first.GetGroup(), tag->first.GetElement()); - - Json::Value tmp = Json::objectValue; - tmp["Name"] = OrthancPlugins::GetKeyword(*dictionary_, t); - tmp["Type"] = "String"; - tmp["Value"] = tag->second; - result[FormatOrthancTag(tag->first)] = tmp; + result[tag->first.Format()] = tag->second; } - results.AddFromOrthanc(result, wadoUrl); + writer.AddJson(result); } } - results.Answer(); + writer.Send(); }
--- a/Plugin/WadoRs.cpp Tue Feb 26 17:05:30 2019 +0100 +++ b/Plugin/WadoRs.cpp Tue Feb 26 17:33:07 2019 +0100 @@ -326,8 +326,7 @@ OrthancPlugins::MemoryBuffer dicom; if (dicom.RestApiGet("/instances/" + *it + "/file", false)) { - writer.AddRawDicom(dicom.GetData(), dicom.GetSize(), - OrthancPluginDicomWebBinaryMode_BulkDataUri, bulkRoot); + writer.AddDicom(dicom.GetData(), dicom.GetSize(), bulkRoot); } } }