ENH: Create rules instead of hard-coded DICOM display fields

There are DICOM display fields that do not concern only one instance, but a higher level such as series (number of images), study (number of series), or patient (last study date, number of studies). These fields were calculated using hard-coded functions. Now these have their own rule classes that do the necessary processing in their endUpdate functions.

Libs/DICOM/Core/CMakeLists.txt

@@ -40,6 +40,14 @@ set(KIT_SRCS
   ctkDICOMDisplayedFieldGeneratorDefaultRule.cpp
   ctkDICOMDisplayedFieldGeneratorRadiotherapySeriesDescriptionRule.h
   ctkDICOMDisplayedFieldGeneratorRadiotherapySeriesDescriptionRule.cpp
+  ctkDICOMDisplayedFieldGeneratorLastStudyDateRule.h
+  ctkDICOMDisplayedFieldGeneratorLastStudyDateRule.cpp
+  ctkDICOMDisplayedFieldGeneratorSeriesImageCount.h
+  ctkDICOMDisplayedFieldGeneratorSeriesImageCount.cpp
+  ctkDICOMDisplayedFieldGeneratorStudyNumberOfSeries.h
+  ctkDICOMDisplayedFieldGeneratorStudyNumberOfSeries.cpp
+  ctkDICOMDisplayedFieldGeneratorPatientNumberOfStudies.h
+  ctkDICOMDisplayedFieldGeneratorPatientNumberOfStudies.cpp
 )
 
 # Abstract class should not be wrapped !

Libs/DICOM/Core/ctkDICOMDatabase.cpp

@@ -315,12 +315,6 @@ QStringList ctkDICOMDatabasePrivate::filenames(QString table)
   return allFileNames;
 }
 
-//------------------------------------------------------------------------------
-QString ctkDICOMDatabasePrivate::compositePatientID(const QString& patientID, const QString& patientsName, const QString& patientsBirthDate)
-{
-  return QString("%1~%2~%3").arg(patientID).arg(patientsBirthDate).arg(patientsName);
-}
-
 //------------------------------------------------------------------------------
 bool ctkDICOMDatabasePrivate::insertPatient(const ctkDICOMItem& dataset, int& dbPatientID)
 {
@@ -342,7 +336,7 @@ bool ctkDICOMDatabasePrivate::insertPatient(const ctkDICOMItem& dataset, int& db
   checkPatientExistsQuery.bindValue(1, patientsName);
   loggedExec(checkPatientExistsQuery);
 
-  QString compositeID = this->compositePatientID(patientID, patientsName, patientsBirthDate);
+  QString compositeID = ctkDICOMDatabase::compositePatientID(patientID, patientsName, patientsBirthDate);
   if (checkPatientExistsQuery.next())
   {
     // we found him
@@ -747,7 +741,7 @@ bool ctkDICOMDatabasePrivate::insertPatientStudySeries(const ctkDICOMItem& datas
   // Insert new patient if needed
   // Generate composite patient ID
   QString patientsBirthDate(dataset.GetElementAsString(DCM_PatientBirthDate));
-  QString compositePatientId = this->compositePatientID(patientID, patientsName, patientsBirthDate);
+  QString compositePatientId = ctkDICOMDatabase::compositePatientID(patientID, patientsName, patientsBirthDate);
   // The dbPatientID  is a unique number within the database, generated by the sqlite autoincrement.
   // The patientID  is the (non-unique) DICOM patient id.
   QMap<QString, int>::iterator dbPatientIDit = this->InsertedPatientsCompositeIDCache.find(compositePatientId);
@@ -1148,7 +1142,7 @@ QString ctkDICOMDatabasePrivate::getDisplayPatientFieldsKey(const QString& patie
   const QString& patientsName, const QString& patientsBirthDate,
   QMap<QString, QMap<QString, QString> >& displayedFieldsMapPatient)
 {
-  QString compositeID = this->compositePatientID(patientID, patientsName, patientsBirthDate);
+  QString compositeID = ctkDICOMDatabase::compositePatientID(patientID, patientsName, patientsBirthDate);
 
   // Look for the patient in the displayed fields cache first
   if (displayedFieldsMapPatient.find(compositeID) != displayedFieldsMapPatient.end())
@@ -1277,7 +1271,7 @@ bool ctkDICOMDatabasePrivate::applyDisplayedFieldsChanges( QMap<QString, QMap<QS
 
   // Update patient fields
 
-  foreach(QString compositeID, displayedFieldsMapPatient.keys())
+  foreach (QString compositeID, displayedFieldsMapPatient.keys())
   {
     QMap<QString, QString> currentPatient = displayedFieldsMapPatient[compositeID];
     QSqlQuery displayPatientsQuery(this->Database);
@@ -1434,100 +1428,6 @@ bool ctkDICOMDatabasePrivate::applyDisplayedFieldsChanges( QMap<QString, QMap<QS
   return true;
 }
 
-//------------------------------------------------------------------------------
-void ctkDICOMDatabasePrivate::setCountToSeriesDisplayedFields(QMap<QString, QMap<QString, QString> > &displayedFieldsMapSeries)
-{
-  foreach (QString currentSeriesInstanceUid, displayedFieldsMapSeries.keys())
-  {
-    QSqlQuery countQuery(this->Database);
-    countQuery.prepare("SELECT COUNT(*) FROM Images WHERE SeriesInstanceUID = ? ;");
-    countQuery.addBindValue(currentSeriesInstanceUid);
-    if (!countQuery.exec())
-    {
-      logger.error("SQLITE ERROR: " + countQuery.lastError().driverText());
-      continue;
-    }
-
-    countQuery.first();
-    int currentCount = countQuery.value(0).toInt();
-
-    QMap<QString, QString> displayedFieldsForCurrentSeries = displayedFieldsMapSeries[currentSeriesInstanceUid];
-    displayedFieldsForCurrentSeries["DisplayedCount"] = QString::number(currentCount);
-    displayedFieldsMapSeries[currentSeriesInstanceUid] = displayedFieldsForCurrentSeries;
-  }
-}
-
-//------------------------------------------------------------------------------
-void ctkDICOMDatabasePrivate::setNumberOfSeriesToStudyDisplayedFields(QMap<QString, QMap<QString, QString> > &displayedFieldsMapStudy)
-{
-  foreach (QString currentStudyInstanceUid, displayedFieldsMapStudy.keys())
-  {
-    QSqlQuery numberOfSeriesQuery(this->Database);
-    numberOfSeriesQuery.prepare("SELECT COUNT(*) FROM Series WHERE StudyInstanceUID = ? ;");
-    numberOfSeriesQuery.addBindValue(currentStudyInstanceUid);
-    if (!numberOfSeriesQuery.exec())
-    {
-      logger.error("SQLITE ERROR: " + numberOfSeriesQuery.lastError().driverText());
-      continue;
-    }
-
-    numberOfSeriesQuery.first();
-    int currentNumberOfSeries = numberOfSeriesQuery.value(0).toInt();
-
-    QMap<QString, QString> displayedFieldsForCurrentStudy = displayedFieldsMapStudy[currentStudyInstanceUid];
-    displayedFieldsForCurrentStudy["DisplayedNumberOfSeries"] = QString::number(currentNumberOfSeries);
-    displayedFieldsMapStudy[currentStudyInstanceUid] = displayedFieldsForCurrentStudy;
-  }
-}
-
-//------------------------------------------------------------------------------
-void ctkDICOMDatabasePrivate::setNumberOfStudiesToPatientDisplayedFields(QMap<QString, QMap<QString, QString> >& displayedFieldsMapPatient)
-{
-  foreach(QString compositeID, displayedFieldsMapPatient.keys())
-  {
-    QMap<QString, QString> displayedFieldsForCurrentPatient = displayedFieldsMapPatient[compositeID];
-    int patientUID = displayedFieldsForCurrentPatient["UID"].toInt();
-    QSqlQuery numberOfStudiesQuery(this->Database);
-    numberOfStudiesQuery.prepare("SELECT COUNT(*) FROM Studies WHERE PatientsUID = ? ;");
-    numberOfStudiesQuery.addBindValue(patientUID);
-    if (!numberOfStudiesQuery.exec())
-    {
-      logger.error("SQLITE ERROR: " + numberOfStudiesQuery.lastError().driverText());
-      continue;
-    }
-
-    numberOfStudiesQuery.first();
-    int currentNumberOfStudies = numberOfStudiesQuery.value(0).toInt();
-
-    displayedFieldsForCurrentPatient["DisplayedNumberOfStudies"] = QString::number(currentNumberOfStudies);
-    displayedFieldsMapPatient[compositeID] = displayedFieldsForCurrentPatient;
-  }
-}
-
-//------------------------------------------------------------------------------
-void ctkDICOMDatabasePrivate::setLastStudyDateToPatientDisplayedFields(QMap<QString, QMap<QString, QString> >& displayedFieldsMapPatient)
-{
-  foreach(QString compositeID, displayedFieldsMapPatient.keys())
-  {
-    QMap<QString, QString> displayedFieldsForCurrentPatient = displayedFieldsMapPatient[compositeID];
-    int patientUID = displayedFieldsForCurrentPatient["UID"].toInt();
-    QSqlQuery numberOfStudiesQuery(this->Database);
-    numberOfStudiesQuery.prepare("SELECT MAX(StudyDate) FROM Studies WHERE PatientsUID = ? ;");
-    numberOfStudiesQuery.addBindValue(patientUID);
-    if (!numberOfStudiesQuery.exec())
-    {
-      logger.error("SQLITE ERROR: " + numberOfStudiesQuery.lastError().driverText());
-      continue;
-    }
-
-    numberOfStudiesQuery.first();
-    QDate lastStudyDate = numberOfStudiesQuery.value(0).toDate();
-
-    displayedFieldsForCurrentPatient["DisplayedLastStudyDate"] = lastStudyDate.toString();
-    displayedFieldsMapPatient[compositeID] = displayedFieldsForCurrentPatient;
-  }
-}
-
 //------------------------------------------------------------------------------
 QString ctkDICOMDatabasePrivate::absolutePathFromInternal(const QString& filename)
 {
@@ -3070,7 +2970,8 @@ void ctkDICOMDatabase::updateDisplayedFields()
   Q_D(ctkDICOMDatabase);
 
   // Get the files for which the displayed fields have not been created yet (DisplayedFieldsUpdatedTimestamp is NULL)
-  //TODO: handle cases when the values actually changed; now we only cover insertion and schema update
+  // Note: The per-instance update only covers insertion and schema update. If fields on the series/study/patient level need to be
+  // updated on the insertion of a new instance, then it can be handled using the startUpdate/endUpdate functions of the rules.
   QSqlQuery newFilesQuery(d->Database);
   d->loggedExec(newFilesQuery,QString("SELECT SOPInstanceUID, SeriesInstanceUID FROM Images WHERE DisplayedFieldsUpdatedTimestamp IS NULL;"));
 
@@ -3084,7 +2985,7 @@ void ctkDICOMDatabase::updateDisplayedFields()
   int progressValue = 0;
   emit displayedFieldsUpdateProgress(++progressValue);
 
-  // Initialize roles for starting the update
+  // Initialize rules for starting the update
   d->DisplayedFieldGenerator->startUpdate();
 
   // Get display names for newly added files and add them into the display tables
@@ -3134,7 +3035,7 @@ void ctkDICOMDatabase::updateDisplayedFields()
     }
     QMap<QString, QString> displayedFieldsForCurrentSeries = displayedFieldsMapSeries[ displayedFieldsKeyForCurrentSeries ];
 
-    // Do the update of the displayed fields using the roles
+    // Do the update of the displayed fields using the rules
     d->DisplayedFieldGenerator->updateDisplayedFieldsForInstance(sopInstanceUID, cachedTags,
       displayedFieldsForCurrentSeries, displayedFieldsForCurrentStudy, displayedFieldsForCurrentPatient);
 
@@ -3146,25 +3047,11 @@ void ctkDICOMDatabase::updateDisplayedFields()
 
   emit displayedFieldsUpdateProgress(++progressValue);
 
-  // Finalize update by giving the roles the chance to write the final results in the maps
+  // Finalize update by giving the rules the chance to write the final results in the maps
   d->DisplayedFieldGenerator->endUpdate(displayedFieldsMapSeries, displayedFieldsMapStudy, displayedFieldsMapPatient);
 
   emit displayedFieldsUpdateProgress(++progressValue);
 
-  // Calculate number of images in each updated series
-  d->setCountToSeriesDisplayedFields(displayedFieldsMapSeries);
-
-  emit displayedFieldsUpdateProgress(++progressValue);
-
-  // Calculate number of series in each updated study
-  d->setNumberOfSeriesToStudyDisplayedFields(displayedFieldsMapStudy);
-  // Calculate number of studies in each updated patient
-  d->setNumberOfStudiesToPatientDisplayedFields(displayedFieldsMapPatient);
-  // Determine lat study date in each updated patient
-  d->setLastStudyDateToPatientDisplayedFields(displayedFieldsMapPatient);
-
-  emit displayedFieldsUpdateProgress(++progressValue);
-
   // Update/insert the display values
   if (displayedFieldsMapSeries.count() > 0)
   {
@@ -3366,3 +3253,9 @@ void ctkDICOMDatabase::setFormatForField(QString table, QString field, QString f
 
   emit databaseChanged();
 }
+
+//------------------------------------------------------------------------------
+QString ctkDICOMDatabase::compositePatientID(const QString& patientID, const QString& patientsName, const QString& patientsBirthDate)
+{
+  return QString("%1~%2~%3").arg(patientID).arg(patientsBirthDate).arg(patientsName);
+}

Libs/DICOM/Core/ctkDICOMDatabase.h

@@ -388,6 +388,12 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDatabase : public QObject
   /// Set format of a given field
   Q_INVOKABLE void setFormatForField(QString table, QString field, QString format);
 
+  /// There is no patient UID in DICOM, so we need to use use this composite ID to uniquely identify a patient with a string.
+  /// Used when inserting a patient (InsertedPatientsCompositeIDCache) and in the display field update process.
+  /// Note: It is not a problem that is somewhat more strict than the criteria that is used to decide if a study should be
+  /// inserted under the same patient.
+  Q_INVOKABLE static QString compositePatientID(const QString& patientID, const QString& patientsName, const QString& patientsBirthDate);
+
 Q_SIGNALS:
 
   /// Things inserted to database.

Libs/DICOM/Core/ctkDICOMDatabase_p.h

@@ -88,7 +88,7 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDatabasePrivate
   /// Copy the complete list of files to an extra table
   QStringList allFilesInDatabase();
 
-  /// Update database tables from the displayed fields determined by the plugin roles
+  /// Update database tables from the displayed fields determined by the plugin rules
   /// \return Success flag
   bool applyDisplayedFieldsChanges( QMap<QString, QMap<QString, QString> > &displayedFieldsMapSeries,
                                     QMap<QString, QMap<QString, QString> > &displayedFieldsMapStudy,
@@ -114,20 +114,6 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDatabasePrivate
   /// Get all Filename values from table
   QStringList filenames(QString table);
 
-  QVector<QMap<QString /*DisplayField*/, QString /*Value*/> > displayedFieldsVectorPatient; // The index in the vector is the internal patient UID
-  /// Calculate count (number of objects in interest) for each series in the displayed fields container
-  /// \param displayedFieldsMapSeries (SeriesInstanceUID -> (DisplayField -> Value) )
-  void setCountToSeriesDisplayedFields(QMap<QString, QMap<QString, QString> > &displayedFieldsMapSeries);
-  /// Calculate number of series for each study in the displayed fields container
-  /// \param displayedFieldsMapStudy (StudyInstanceUID -> (DisplayField -> Value) )
-  void setNumberOfSeriesToStudyDisplayedFields(QMap<QString, QMap<QString, QString> > &displayedFieldsMapStudy);
-  /// Calculate number of studies for each patient in the displayed fields container
-  /// \param displayedFieldsVectorPatient (Internal_ID -> (DisplayField -> Value) )
-  void setNumberOfStudiesToPatientDisplayedFields(QMap<QString, QMap<QString, QString> >& displayedFieldsMapPatient);
-  /// Determine last study date for each patient in the displayed fields container
-  /// \param displayedFieldsVectorPatient (Internal_ID -> (DisplayField -> Value) )
-  void setLastStudyDateToPatientDisplayedFields(QMap<QString, QMap<QString, QString> >& displayedFieldsMapPatient);
-
   int rowCount(const QString& tableName);
 
   /// Convert an internal path (absolute or relative to database folder) to an absolute path.
@@ -163,11 +149,6 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDatabasePrivate
   QSet<QString> InsertedStudyUIDsCache;
   QSet<QString> InsertedSeriesUIDsCache;
 
-  /// There is no unique patient ID. We use this composite ID in InsertedPatientsCompositeIDCache.
-  /// It is not a problem that is somewhat more strict than the criteria that is used to decide if a study should be insert
-  /// under the same patient.
-  QString compositePatientID(const QString& patientID, const QString& patientsName, const QString& patientsBirthDate);
-
   /// resets the variables to new inserts won't be fooled by leftover values
   void resetLastInsertedValues();
 

Libs/DICOM/Core/ctkDICOMDisplayedFieldGenerator.cpp

@@ -29,6 +29,10 @@
 #include "ctkDICOMDatabase.h"
 #include "ctkDICOMDisplayedFieldGeneratorDefaultRule.h"
 #include "ctkDICOMDisplayedFieldGeneratorRadiotherapySeriesDescriptionRule.h"
+#include "ctkDICOMDisplayedFieldGeneratorLastStudyDateRule.h"
+#include "ctkDICOMDisplayedFieldGeneratorSeriesImageCount.h"
+#include "ctkDICOMDisplayedFieldGeneratorStudyNumberOfSeries.h"
+#include "ctkDICOMDisplayedFieldGeneratorPatientNumberOfStudies.h"
 
 //------------------------------------------------------------------------------
 static ctkLogger logger("org.commontk.dicom.DICOMDisplayedFieldGenerator" );
@@ -46,6 +50,10 @@ ctkDICOMDisplayedFieldGeneratorPrivate::ctkDICOMDisplayedFieldGeneratorPrivate(c
   // register commonly used rules
   this->AllRules.append(new ctkDICOMDisplayedFieldGeneratorDefaultRule);
   this->AllRules.append(new ctkDICOMDisplayedFieldGeneratorRadiotherapySeriesDescriptionRule);
+  this->AllRules.append(new ctkDICOMDisplayedFieldGeneratorLastStudyDateRule);
+  this->AllRules.append(new ctkDICOMDisplayedFieldGeneratorSeriesImageCount);
+  this->AllRules.append(new ctkDICOMDisplayedFieldGeneratorStudyNumberOfSeries);
+  this->AllRules.append(new ctkDICOMDisplayedFieldGeneratorPatientNumberOfStudies);
 
   foreach(ctkDICOMDisplayedFieldGeneratorAbstractRule* rule, this->AllRules)
   {

Libs/DICOM/Core/ctkDICOMDisplayedFieldGeneratorAbstractRule.h

@@ -48,7 +48,7 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDisplayedFieldGeneratorAbstractRule
   /// Each rule plugin has the chance to fill any field in the series, study, and patient fields.
   /// The way these generated fields will be used is defined by \sa mergeDisplayedFieldsForInstance
   virtual void getDisplayedFieldsForInstance(const QMap<QString, QString> &cachedTagsForInstance, QMap<QString, QString> &displayedFieldsForCurrentSeries,
-    QMap<QString, QString> &displayedFieldsForCurrentStudy, QMap<QString, QString> &displayedFieldsForCurrentPatient)=0;
+    QMap<QString, QString> &displayedFieldsForCurrentStudy, QMap<QString, QString> &displayedFieldsForCurrentPatient) { };
 
   /// Define rules of merging initial database values with new values generated by the rule plugins
   /// Currently available options:
@@ -59,7 +59,7 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDisplayedFieldGeneratorAbstractRule
     const QMap<QString, QString> &newFieldsSeries, const QMap<QString, QString> &newFieldsStudy, const QMap<QString, QString> &newFieldsPatient,
     QMap<QString, QString> &mergedFieldsSeries, QMap<QString, QString> &mergedFieldsStudy, QMap<QString, QString> &mergedFieldsPatient,
     const QMap<QString, QString> &emptyFieldsSeries, const QMap<QString, QString> &emptyFieldsStudy, const QMap<QString, QString> &emptyFieldsPatient
-    )=0;
+    ) { };
 
   /// Specify list of DICOM tags required by the rule. These tags will be included in the tag cache
   virtual QStringList getRequiredDICOMTags()=0;
@@ -84,7 +84,7 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDisplayedFieldGeneratorAbstractRule
   /// Used when merging the original database content with the displayed fields generated by the rules.
   /// Example: SeriesDescription -> Unnamed Series
   virtual void registerEmptyFieldNames(
-    QMap<QString, QString> emptyFieldsSeries, QMap<QString, QString> emptyFieldsStudies, QMap<QString, QString> emptyFieldsPatients)=0;
+    QMap<QString, QString> emptyFieldsSeries, QMap<QString, QString> emptyFieldsStudies, QMap<QString, QString> emptyFieldsPatients) { };
 
   /// Set DICOM database to the rule in case it needs to use it e.g. in \sa end().
   void setDatabase(ctkDICOMDatabase* database) { this->DICOMDatabase = database; }