Mercurial > hg > orthanc
changeset 1286:b4acdb37e43b
refactoring
| author | Sebastien Jodogne <s.jodogne@gmail.com> |
|---|---|
| date | Tue, 03 Feb 2015 16:51:19 +0100 |
| parents | 5730f374e4e6 |
| children | 63a6428771f4 |
| files | NEWS OrthancServer/DatabaseWrapper.cpp OrthancServer/DatabaseWrapper.h OrthancServer/IDatabaseWrapper.h OrthancServer/ServerIndex.cpp OrthancServer/ServerIndex.h UnitTestsSources/ServerIndexTests.cpp |
| diffstat | 7 files changed, 157 insertions(+), 112 deletions(-) [+] |
line wrap: on
line diff
--- a/NEWS Tue Feb 03 15:00:42 2015 +0100 +++ b/NEWS Tue Feb 03 16:51:19 2015 +0100 @@ -5,17 +5,16 @@ ----- * URIs to get all the parents of a given resource in a single REST call -* Support of HTTP proxy * Instances without PatientID are now allowed +* Support of HTTP proxy to access Orthanc peers Minor ----- + * Support of Tudor DICOM in Query/Retrieve -* Fix issue 25 (AET with underscore not allowed) * More flexible "/modify" and "/anonymize" for single instance -* Code refactorings +* Access to called AET and remote AET from Lua scripts ("OnStoredInstance") * Option "DicomAssociationCloseDelay" to set delay before closing DICOM association -* Access to called AET and remote AET from Lua scripts ("OnStoredInstance") Plugins ------- @@ -27,6 +26,12 @@ * Plugins can do REST calls to other plugins (cf. "xxxAfterPlugins()") * Scan of folders for plugins +Fixes +----- + +* Code refactorings +* Fix issue 25 (AET with underscore not allowed) + Version 0.8.5 (2014/11/04) ==========================
--- a/OrthancServer/DatabaseWrapper.cpp Tue Feb 03 15:00:42 2015 +0100 +++ b/OrthancServer/DatabaseWrapper.cpp Tue Feb 03 16:51:19 2015 +0100 @@ -494,32 +494,30 @@ static void SetMainDicomTagsInternal(SQLite::Statement& s, int64_t id, - const DicomElement& element) + const DicomTag& tag, + const std::string& value) { s.BindInt64(0, id); - s.BindInt(1, element.GetTag().GetGroup()); - s.BindInt(2, element.GetTag().GetElement()); - s.BindString(3, element.GetValue().AsString()); + s.BindInt(1, tag.GetGroup()); + s.BindInt(2, tag.GetElement()); + s.BindString(3, value); s.Run(); } - void DatabaseWrapper::SetMainDicomTags(int64_t id, - const DicomMap& tags) + void DatabaseWrapper::SetMainDicomTag(int64_t id, + const DicomTag& tag, + const std::string& value) { - DicomArray flattened(tags); - for (size_t i = 0; i < flattened.GetSize(); i++) + if (tag.IsIdentifier()) { - if (flattened.GetElement(i).GetTag().IsIdentifier()) - { - SQLite::Statement s(db_, SQLITE_FROM_HERE, "INSERT INTO DicomIdentifiers VALUES(?, ?, ?, ?)"); - SetMainDicomTagsInternal(s, id, flattened.GetElement(i)); - } - else - { - SQLite::Statement s(db_, SQLITE_FROM_HERE, "INSERT INTO MainDicomTags VALUES(?, ?, ?, ?)"); - SetMainDicomTagsInternal(s, id, flattened.GetElement(i)); - } + SQLite::Statement s(db_, SQLITE_FROM_HERE, "INSERT INTO DicomIdentifiers VALUES(?, ?, ?, ?)"); + SetMainDicomTagsInternal(s, id, tag, value); + } + else + { + SQLite::Statement s(db_, SQLITE_FROM_HERE, "INSERT INTO MainDicomTags VALUES(?, ?, ?, ?)"); + SetMainDicomTagsInternal(s, id, tag, value); } }
--- a/OrthancServer/DatabaseWrapper.h Tue Feb 03 15:00:42 2015 +0100 +++ b/OrthancServer/DatabaseWrapper.h Tue Feb 03 16:51:19 2015 +0100 @@ -68,6 +68,8 @@ SQLite::Statement& s, unsigned int maxResults); + void ClearTable(const std::string& tableName); + public: DatabaseWrapper(const std::string& path); @@ -127,8 +129,9 @@ int64_t id, FileContentType contentType); - virtual void SetMainDicomTags(int64_t id, - const DicomMap& tags); + virtual void SetMainDicomTag(int64_t id, + const DicomTag& tag, + const std::string& value); virtual void GetMainDicomTags(DicomMap& map, int64_t id); @@ -187,7 +190,15 @@ db_.FlushToDisk(); } - virtual void ClearTable(const std::string& tableName); + virtual void ClearChanges() + { + ClearTable("Changes"); + } + + virtual void ClearExportedResources() + { + ClearTable("ExportedResources"); + } virtual bool IsExistingResource(int64_t internalId); @@ -213,13 +224,13 @@ return db_.GetErrorMessage(); } - virtual void GetChildren(std::list<std::string>& childrenPublicIds, - int64_t id); + void GetChildren(std::list<std::string>& childrenPublicIds, + int64_t id); - virtual int64_t GetTableRecordCount(const std::string& table); + int64_t GetTableRecordCount(const std::string& table); - virtual bool GetParentPublicId(std::string& result, - int64_t id); + bool GetParentPublicId(std::string& result, + int64_t id); }; }
--- a/OrthancServer/IDatabaseWrapper.h Tue Feb 03 15:00:42 2015 +0100 +++ b/OrthancServer/IDatabaseWrapper.h Tue Feb 03 16:51:19 2015 +0100 @@ -56,7 +56,9 @@ virtual void AttachChild(int64_t parent, int64_t child) = 0; - virtual void ClearTable(const std::string& tableName) = 0; + virtual void ClearChanges() = 0; + + virtual void ClearExportedResources() = 0; virtual int64_t CreateResource(const std::string& publicId, ResourceType type) = 0; @@ -159,8 +161,9 @@ virtual void SetGlobalProperty(GlobalProperty property, const std::string& value) = 0; - virtual void SetMainDicomTags(int64_t id, - const DicomMap& tags) = 0; + virtual void SetMainDicomTag(int64_t id, + const DicomTag& tag, + const std::string& value) = 0; virtual void SetMetadata(int64_t id, MetadataType type, @@ -172,18 +175,5 @@ virtual SQLite::ITransaction* StartTransaction() = 0; virtual void SetListener(IServerIndexListener& listener) = 0; - - - // For unit tests only! - virtual void GetChildren(std::list<std::string>& childrenPublicIds, - int64_t id) = 0; - - // For unit tests only! - virtual int64_t GetTableRecordCount(const std::string& table) = 0; - - // For unit tests only! - virtual bool GetParentPublicId(std::string& result, - int64_t id) = 0; - }; }
--- a/OrthancServer/ServerIndex.cpp Tue Feb 03 15:00:42 2015 +0100 +++ b/OrthancServer/ServerIndex.cpp Tue Feb 03 16:51:19 2015 +0100 @@ -471,6 +471,19 @@ + void ServerIndex::SetMainDicomTags(int64_t resource, + const DicomMap& tags) + { + DicomArray flattened(tags); + for (size_t i = 0; i < flattened.GetSize(); i++) + { + const DicomElement& element = flattened.GetElement(i); + db_.SetMainDicomTag(resource, element.GetTag(), element.GetValue().AsString()); + } + } + + + ServerIndex::ServerIndex(ServerContext& context, IDatabaseWrapper& db) : done_(false), @@ -551,7 +564,7 @@ DicomMap dicom; dicomSummary.ExtractInstanceInformation(dicom); - db_.SetMainDicomTags(instance, dicom); + SetMainDicomTags(instance, dicom); // Detect up to which level the patient/study/series/instance // hierarchy must be created @@ -604,7 +617,7 @@ { series = db_.CreateResource(hasher.HashSeries(), ResourceType_Series); dicomSummary.ExtractSeriesInformation(dicom); - db_.SetMainDicomTags(series, dicom); + SetMainDicomTags(series, dicom); } // Create the study if needed @@ -612,7 +625,7 @@ { study = db_.CreateResource(hasher.HashStudy(), ResourceType_Study); dicomSummary.ExtractStudyInformation(dicom); - db_.SetMainDicomTags(study, dicom); + SetMainDicomTags(study, dicom); } // Create the patient if needed @@ -620,7 +633,7 @@ { patient = db_.CreateResource(hasher.HashPatient(), ResourceType_Patient); dicomSummary.ExtractPatientInformation(dicom); - db_.SetMainDicomTags(patient, dicom); + SetMainDicomTags(patient, dicom); } // Create the parent-to-child links @@ -1561,13 +1574,13 @@ void ServerIndex::DeleteChanges() { boost::mutex::scoped_lock lock(mutex_); - db_.ClearTable("Changes"); + db_.ClearChanges(); } void ServerIndex::DeleteExportedResources() { boost::mutex::scoped_lock lock(mutex_); - db_.ClearTable("ExportedResources"); + db_.ClearExportedResources(); }
--- a/OrthancServer/ServerIndex.h Tue Feb 03 15:00:42 2015 +0100 +++ b/OrthancServer/ServerIndex.h Tue Feb 03 16:51:19 2015 +0100 @@ -118,6 +118,8 @@ uint64_t IncrementGlobalSequenceInternal(GlobalProperty property); + void SetMainDicomTags(int64_t resource, + const DicomMap& tags); public: ServerIndex(ServerContext& context,
--- a/UnitTestsSources/ServerIndexTests.cpp Tue Feb 03 15:00:42 2015 +0100 +++ b/UnitTestsSources/ServerIndexTests.cpp Tue Feb 03 16:51:19 2015 +0100 @@ -119,6 +119,16 @@ index_.reset(NULL); listener_.reset(NULL); } + + void CheckTableRecordCount(uint32_t expected, const char* table) + { + if (GetParam() == DatabaseWrapperClass_SQLite) + { + DatabaseWrapper* sqlite = dynamic_cast<DatabaseWrapper*>(index_.get()); + ASSERT_EQ(expected, sqlite->GetTableRecordCount(table)); + } + } + }; } @@ -130,6 +140,12 @@ TEST_P(DatabaseWrapperTest, Simple) { + DatabaseWrapper* sqlite_ = NULL; + if (GetParam() == DatabaseWrapperClass_SQLite) + { + sqlite_ = dynamic_cast<DatabaseWrapper*>(index_.get()); + } + int64_t a[] = { index_->CreateResource("a", ResourceType_Patient), // 0 index_->CreateResource("b", ResourceType_Study), // 1 @@ -192,14 +208,17 @@ ASSERT_FALSE(index_->LookupParent(parent, a[6])); std::string s; - - ASSERT_FALSE(index_->GetParentPublicId(s, a[0])); - ASSERT_FALSE(index_->GetParentPublicId(s, a[6])); - ASSERT_TRUE(index_->GetParentPublicId(s, a[1])); ASSERT_EQ("a", s); - ASSERT_TRUE(index_->GetParentPublicId(s, a[2])); ASSERT_EQ("b", s); - ASSERT_TRUE(index_->GetParentPublicId(s, a[3])); ASSERT_EQ("c", s); - ASSERT_TRUE(index_->GetParentPublicId(s, a[4])); ASSERT_EQ("c", s); - ASSERT_TRUE(index_->GetParentPublicId(s, a[5])); ASSERT_EQ("g", s); + + if (sqlite_ != NULL) + { + ASSERT_FALSE(sqlite_->GetParentPublicId(s, a[0])); + ASSERT_FALSE(sqlite_->GetParentPublicId(s, a[6])); + ASSERT_TRUE(sqlite_->GetParentPublicId(s, a[1])); ASSERT_EQ("a", s); + ASSERT_TRUE(sqlite_->GetParentPublicId(s, a[2])); ASSERT_EQ("b", s); + ASSERT_TRUE(sqlite_->GetParentPublicId(s, a[3])); ASSERT_EQ("c", s); + ASSERT_TRUE(sqlite_->GetParentPublicId(s, a[4])); ASSERT_EQ("c", s); + ASSERT_TRUE(sqlite_->GetParentPublicId(s, a[5])); ASSERT_EQ("g", s); + } std::list<std::string> l; index_->GetChildrenPublicId(l, a[0]); ASSERT_EQ(1u, l.size()); ASSERT_EQ("b", l.front()); @@ -256,9 +275,7 @@ ASSERT_EQ(21u + 42u + 44u, index_->GetTotalCompressedSize()); ASSERT_EQ(42u + 42u + 44u, index_->GetTotalUncompressedSize()); - DicomMap m; - m.SetValue(0x0010, 0x0010, "PatientName"); - index_->SetMainDicomTags(a[3], m); + index_->SetMainDicomTag(a[3], DicomTag(0x0010, 0x0010), "PatientName"); int64_t b; ResourceType t; @@ -298,10 +315,11 @@ ASSERT_EQ(0u, listener_->deletedFiles_.size()); ASSERT_EQ(0u, listener_->deletedResources_.size()); - ASSERT_EQ(7u, index_->GetTableRecordCount("Resources")); - ASSERT_EQ(3u, index_->GetTableRecordCount("AttachedFiles")); - ASSERT_EQ(1u, index_->GetTableRecordCount("Metadata")); - ASSERT_EQ(1u, index_->GetTableRecordCount("MainDicomTags")); + + CheckTableRecordCount(7, "Resources"); + CheckTableRecordCount(3, "AttachedFiles"); + CheckTableRecordCount(1, "Metadata"); + CheckTableRecordCount(1, "MainDicomTags"); index_->DeleteResource(a[0]); ASSERT_EQ(5u, listener_->deletedResources_.size()); @@ -313,15 +331,17 @@ listener_->deletedFiles_.end(), "my dicom file") == listener_->deletedFiles_.end()); - ASSERT_EQ(2u, index_->GetTableRecordCount("Resources")); - ASSERT_EQ(0u, index_->GetTableRecordCount("Metadata")); - ASSERT_EQ(1u, index_->GetTableRecordCount("AttachedFiles")); - ASSERT_EQ(0u, index_->GetTableRecordCount("MainDicomTags")); + CheckTableRecordCount(2, "Resources"); + CheckTableRecordCount(0, "Metadata"); + CheckTableRecordCount(1, "AttachedFiles"); + CheckTableRecordCount(0, "MainDicomTags"); + index_->DeleteResource(a[5]); ASSERT_EQ(7u, listener_->deletedResources_.size()); - ASSERT_EQ(0u, index_->GetTableRecordCount("Resources")); - ASSERT_EQ(0u, index_->GetTableRecordCount("AttachedFiles")); - ASSERT_EQ(2u, index_->GetTableRecordCount("GlobalProperties")); + + CheckTableRecordCount(0, "Resources"); + CheckTableRecordCount(0, "AttachedFiles"); + CheckTableRecordCount(2, "GlobalProperties"); ASSERT_EQ(3u, listener_->deletedFiles_.size()); ASSERT_FALSE(std::find(listener_->deletedFiles_.begin(), @@ -334,6 +354,12 @@ TEST_P(DatabaseWrapperTest, Upward) { + DatabaseWrapper* sqlite_ = NULL; + if (GetParam() == DatabaseWrapperClass_SQLite) + { + sqlite_ = dynamic_cast<DatabaseWrapper*>(index_.get()); + } + int64_t a[] = { index_->CreateResource("a", ResourceType_Patient), // 0 index_->CreateResource("b", ResourceType_Study), // 1 @@ -353,28 +379,29 @@ index_->AttachChild(a[0], a[5]); index_->AttachChild(a[5], a[7]); + if (sqlite_ != NULL) { std::list<std::string> j; - index_->GetChildren(j, a[0]); + sqlite_->GetChildren(j, a[0]); ASSERT_EQ(2u, j.size()); ASSERT_TRUE((j.front() == "b" && j.back() == "f") || (j.back() == "b" && j.front() == "f")); - index_->GetChildren(j, a[1]); + sqlite_->GetChildren(j, a[1]); ASSERT_EQ(2u, j.size()); ASSERT_TRUE((j.front() == "c" && j.back() == "g") || (j.back() == "c" && j.front() == "g")); - index_->GetChildren(j, a[2]); + sqlite_->GetChildren(j, a[2]); ASSERT_EQ(2u, j.size()); ASSERT_TRUE((j.front() == "d" && j.back() == "e") || (j.back() == "d" && j.front() == "e")); - index_->GetChildren(j, a[3]); ASSERT_EQ(0u, j.size()); - index_->GetChildren(j, a[4]); ASSERT_EQ(0u, j.size()); - index_->GetChildren(j, a[5]); ASSERT_EQ(1u, j.size()); ASSERT_EQ("h", j.front()); - index_->GetChildren(j, a[6]); ASSERT_EQ(0u, j.size()); - index_->GetChildren(j, a[7]); ASSERT_EQ(0u, j.size()); + sqlite_->GetChildren(j, a[3]); ASSERT_EQ(0u, j.size()); + sqlite_->GetChildren(j, a[4]); ASSERT_EQ(0u, j.size()); + sqlite_->GetChildren(j, a[5]); ASSERT_EQ(1u, j.size()); ASSERT_EQ("h", j.front()); + sqlite_->GetChildren(j, a[6]); ASSERT_EQ(0u, j.size()); + sqlite_->GetChildren(j, a[7]); ASSERT_EQ(0u, j.size()); } listener_->Reset(); @@ -410,8 +437,8 @@ ASSERT_FALSE(index_->IsProtectedPatient(patients[i])); } - ASSERT_EQ(10u, index_->GetTableRecordCount("Resources")); - ASSERT_EQ(10u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(10u, "Resources"); + CheckTableRecordCount(10u, "PatientRecyclingOrder"); listener_->Reset(); ASSERT_EQ(0u, listener_->deletedResources_.size()); @@ -419,8 +446,9 @@ index_->DeleteResource(patients[5]); index_->DeleteResource(patients[0]); ASSERT_EQ(2u, listener_->deletedResources_.size()); - ASSERT_EQ(8u, index_->GetTableRecordCount("Resources")); - ASSERT_EQ(8u, index_->GetTableRecordCount("PatientRecyclingOrder")); + + CheckTableRecordCount(8u, "Resources"); + CheckTableRecordCount(8u, "PatientRecyclingOrder"); ASSERT_EQ(2u, listener_->deletedFiles_.size()); ASSERT_EQ("Patient 5", listener_->deletedFiles_[0]); @@ -452,8 +480,9 @@ ASSERT_EQ(10u, listener_->deletedResources_.size()); ASSERT_EQ(10u, listener_->deletedFiles_.size()); - ASSERT_EQ(0u, index_->GetTableRecordCount("Resources")); - ASSERT_EQ(0u, index_->GetTableRecordCount("PatientRecyclingOrder")); + + CheckTableRecordCount(0, "Resources"); + CheckTableRecordCount(0, "PatientRecyclingOrder"); } @@ -469,38 +498,36 @@ ASSERT_FALSE(index_->IsProtectedPatient(patients[i])); } - ASSERT_EQ(5u, index_->GetTableRecordCount("Resources")); - ASSERT_EQ(5u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(5, "Resources"); + CheckTableRecordCount(5, "PatientRecyclingOrder"); ASSERT_FALSE(index_->IsProtectedPatient(patients[2])); index_->SetProtectedPatient(patients[2], true); ASSERT_TRUE(index_->IsProtectedPatient(patients[2])); - ASSERT_EQ(4u, index_->GetTableRecordCount("PatientRecyclingOrder")); - ASSERT_EQ(5u, index_->GetTableRecordCount("Resources")); + CheckTableRecordCount(5, "Resources"); + CheckTableRecordCount(4, "PatientRecyclingOrder"); index_->SetProtectedPatient(patients[2], true); ASSERT_TRUE(index_->IsProtectedPatient(patients[2])); - ASSERT_EQ(4u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(4, "PatientRecyclingOrder"); index_->SetProtectedPatient(patients[2], false); ASSERT_FALSE(index_->IsProtectedPatient(patients[2])); - ASSERT_EQ(5u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(5, "PatientRecyclingOrder"); index_->SetProtectedPatient(patients[2], false); ASSERT_FALSE(index_->IsProtectedPatient(patients[2])); - ASSERT_EQ(5u, index_->GetTableRecordCount("PatientRecyclingOrder")); - - ASSERT_EQ(5u, index_->GetTableRecordCount("Resources")); - ASSERT_EQ(5u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(5, "PatientRecyclingOrder"); + CheckTableRecordCount(5, "Resources"); index_->SetProtectedPatient(patients[2], true); ASSERT_TRUE(index_->IsProtectedPatient(patients[2])); - ASSERT_EQ(4u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(4, "PatientRecyclingOrder"); index_->SetProtectedPatient(patients[2], false); ASSERT_FALSE(index_->IsProtectedPatient(patients[2])); - ASSERT_EQ(5u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(5, "PatientRecyclingOrder"); index_->SetProtectedPatient(patients[3], true); ASSERT_TRUE(index_->IsProtectedPatient(patients[3])); - ASSERT_EQ(4u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(4, "PatientRecyclingOrder"); - ASSERT_EQ(5u, index_->GetTableRecordCount("Resources")); + CheckTableRecordCount(5, "Resources"); ASSERT_EQ(0u, listener_->deletedFiles_.size()); // Unprotecting a patient puts it at the last position in the recycling queue @@ -524,11 +551,11 @@ ASSERT_FALSE(index_->SelectPatientToRecycle(p)); ASSERT_EQ(4u, listener_->deletedFiles_.size()); - ASSERT_EQ(1u, index_->GetTableRecordCount("Resources")); - ASSERT_EQ(0u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(1, "Resources"); + CheckTableRecordCount(0, "PatientRecyclingOrder"); index_->SetProtectedPatient(patients[3], false); - ASSERT_EQ(1u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(1, "PatientRecyclingOrder"); ASSERT_FALSE(index_->SelectPatientToRecycle(p, patients[3])); ASSERT_TRUE(index_->SelectPatientToRecycle(p, patients[2])); ASSERT_TRUE(index_->SelectPatientToRecycle(p)); ASSERT_EQ(p, patients[3]); @@ -536,8 +563,8 @@ ASSERT_EQ(5u, listener_->deletedResources_.size()); ASSERT_EQ(5u, listener_->deletedFiles_.size()); - ASSERT_EQ(0u, index_->GetTableRecordCount("Resources")); - ASSERT_EQ(0u, index_->GetTableRecordCount("PatientRecyclingOrder")); + CheckTableRecordCount(0, "Resources"); + CheckTableRecordCount(0, "PatientRecyclingOrder"); } @@ -570,11 +597,10 @@ index_->CreateResource("d", ResourceType_Series) // 3 }; - DicomMap m; - m.Clear(); m.SetValue(DICOM_TAG_STUDY_INSTANCE_UID, "0"); index_->SetMainDicomTags(a[0], m); - m.Clear(); m.SetValue(DICOM_TAG_STUDY_INSTANCE_UID, "1"); index_->SetMainDicomTags(a[1], m); - m.Clear(); m.SetValue(DICOM_TAG_STUDY_INSTANCE_UID, "0"); index_->SetMainDicomTags(a[2], m); - m.Clear(); m.SetValue(DICOM_TAG_SERIES_INSTANCE_UID, "0"); index_->SetMainDicomTags(a[3], m); + index_->SetMainDicomTag(a[0], DICOM_TAG_STUDY_INSTANCE_UID, "0"); + index_->SetMainDicomTag(a[1], DICOM_TAG_STUDY_INSTANCE_UID, "1"); + index_->SetMainDicomTag(a[2], DICOM_TAG_STUDY_INSTANCE_UID, "0"); + index_->SetMainDicomTag(a[3], DICOM_TAG_SERIES_INSTANCE_UID, "0"); std::list<int64_t> s;