Merge branch '216-cache-on-import'

Closes #216
* 216-cache-on-import:
  Expose tags to precache in app widget for use with private database
  Avoid assert on ill-formed group/element strings
  Add tests for tag cache and precaching in dicom database
  Differentiate missing keys and previously seen keys in dicom tag cache
  Add a precache tags property

Libs/DICOM/Core/Testing/Cpp/CMakeLists.txt

@@ -5,6 +5,7 @@ create_test_sourcelist(Tests ${KIT}CppTests.cpp
   ctkDICOMDatabaseTest1.cpp
   ctkDICOMDatabaseTest2.cpp
   ctkDICOMDatabaseTest3.cpp
+  ctkDICOMDatabaseTest4.cpp
   ctkDICOMDatasetTest1.cpp
   ctkDICOMIndexerTest1.cpp
   ctkDICOMModelTest1.cpp
@@ -35,6 +36,7 @@ SIMPLE_TEST(ctkDICOMDatabaseTest2 ${CTKData_DIR}/Data/DICOM/MRHEAD/000055.IMA)
 SIMPLE_TEST(ctkDICOMDatabaseTest3
   ${CMAKE_CURRENT_SOURCE_DIR}/../../Resources/dicom-unversioned-schema.sql
   )
+SIMPLE_TEST(ctkDICOMDatabaseTest4 ${CTKData_DIR}/Data/DICOM/MRHEAD/000055.IMA)
 SIMPLE_TEST(ctkDICOMDatasetTest1)
 SIMPLE_TEST(ctkDICOMIndexerTest1 )
 

Libs/DICOM/Core/Testing/Cpp/ctkDICOMDatabaseTest4.cpp

@@ -0,0 +1,132 @@
+/*=========================================================================
+
+  Library:   CTK
+
+  Copyright (c) Kitware Inc.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0.txt
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+=========================================================================*/
+
+// Qt includes
+#include <QCoreApplication>
+#include <QDir>
+
+// ctkDICOMCore includes
+#include "ctkDICOMDatabase.h"
+
+// STD includes
+#include <iostream>
+#include <cstdlib>
+
+
+int ctkDICOMDatabaseTest4( int argc, char * argv [] )
+{
+  QCoreApplication app(argc, argv);
+
+  if (argc < 2)
+    {
+    std::cerr << "ctkDICOMDatabaseTest2: missing dicom filePath argument";
+    std::cerr << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  QString dicomFilePath(argv[1]);
+
+  ctkDICOMDatabase database;
+  QDir databaseDirectory = QDir::temp();
+  databaseDirectory.remove("ctkDICOMDatabase.sql");
+  databaseDirectory.remove("ctkDICOMTagCache.sql");
+
+  QFileInfo databaseFile(databaseDirectory, QString("database.test"));
+  database.openDatabase(databaseFile.absoluteFilePath());
+
+  bool res = database.initializeDatabase();
+
+  if (!res)
+    {
+    std::cerr << "ctkDICOMDatabase::initializeDatabase() failed." << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  //
+  // Basic test:
+  // - insert the file specified on the command line
+  // - ask for tag values and compare to known results
+  //
+  QString instanceUID("1.2.840.113619.2.135.3596.6358736.4843.1115808177.83");
+  QString tag("0008,103e");
+  QString badTag("9999,9999");
+
+  //
+  // Test the precache feature of the database
+  //
+
+  if (database.cachedTag(instanceUID, tag) != QString(""))
+    {
+    std::cerr << "ctkDICOMDatabase: tag cache should return empty string for unknown instance tag" << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  if (database.cachedTag(instanceUID, badTag) != QString(""))
+    {
+    std::cerr << "ctkDICOMDatabase: bad tag cache should return empty string for unknown instance tag" << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  QStringList tagsToPrecache;
+  tagsToPrecache << tag;
+  database.setTagsToPrecache(tagsToPrecache);
+
+  if (database.tagsToPrecache() != tagsToPrecache)
+    {
+    std::cerr << "ctkDICOMDatabase: tags to precache not correct" << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  database.insert(dicomFilePath, false, false);
+
+  QString knownSeriesDescription("3D Cor T1 FAST IR-prepped GRE");
+  
+  QString cachedTag = database.cachedTag(instanceUID, tag);
+
+  if (cachedTag != knownSeriesDescription)
+    {
+    std::cerr << "ctkDICOMDatabase: tag cache should return known value for instance" << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  if (database.instanceValue(instanceUID, tag) != knownSeriesDescription)
+    {
+    std::cerr << "ctkDICOMDatabase: database should return known value for instance" << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  if (database.instanceValue(instanceUID, badTag) != QString(""))
+    {
+    std::cerr << "ctkDICOMDatabase: bad tag should have empty value" << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  if (database.cachedTag(instanceUID, badTag) != QString("__TAG_NOT_IN_INSTANCE__"))
+    {
+    std::cerr << "ctkDICOMDatabase: bad tag should have sentinal value in cache" << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  database.closeDatabase();
+
+  std::cerr << "Database is in " << databaseDirectory.path().toStdString() << std::endl;
+
+  return EXIT_SUCCESS;
+}

Libs/DICOM/Core/ctkDICOMDatabase.cpp

@@ -62,6 +62,10 @@
 static ctkLogger logger("org.commontk.dicom.DICOMDatabase" );
 //------------------------------------------------------------------------------
 
+// Flag for tag cache to avoid repeated serarches for
+// tags that do no exist.
+static QString TagNotInInstance("__TAG_NOT_IN_INSTANCE__");
+
 //------------------------------------------------------------------------------
 class ctkDICOMDatabasePrivate
 {
@@ -131,6 +135,8 @@ public:
   /// reading while the tag cache is writing
   QSqlDatabase TagCacheDatabase;
   QString TagCacheDatabaseFilename;
+  QStringList TagsToPrecache;
+  void precacheTags( const QString sopInstanceUID );
 
   int insertPatient(const ctkDICOMDataset& ctkDataset);
   void insertStudy(const ctkDICOMDataset& ctkDataset, int dbPatientID);
@@ -674,6 +680,10 @@ QString ctkDICOMDatabase::headerValue (QString key)
 QString ctkDICOMDatabase::instanceValue(QString sopInstanceUID, QString tag)
 {
   QString value = this->cachedTag(sopInstanceUID, tag);
+  if (value == TagNotInInstance)
+    {
+    return "";
+    }
   if (value != "")
     {
     return value;
@@ -688,6 +698,10 @@ QString ctkDICOMDatabase::instanceValue(const QString sopInstanceUID, const unsi
 {
   QString tag = this->groupElementToTag(group,element);
   QString value = this->cachedTag(sopInstanceUID, tag);
+  if (value == TagNotInInstance)
+    {
+    return "";
+    }
   if (value != "")
     {
     return value;
@@ -712,6 +726,10 @@ QString ctkDICOMDatabase::fileValue(const QString fileName, QString tag)
   this->tagToGroupElement(tag, group, element);
   QString sopInstanceUID = this->instanceForFile(fileName);
   QString value = this->cachedTag(sopInstanceUID, tag);
+  if (value == TagNotInInstance)
+    {
+    return "";
+    }
   if (value != "")
     {
     return value;
@@ -738,6 +756,10 @@ QString ctkDICOMDatabase::fileValue(const QString fileName, const unsigned short
   QString tag = this->groupElementToTag(group, element);
   QString sopInstanceUID = this->instanceForFile(fileName);
   QString value = this->cachedTag(sopInstanceUID, tag);
+  if (value == TagNotInInstance)
+    {
+    return "";
+    }
   if (value != "")
     {
     return value;
@@ -758,6 +780,10 @@ bool ctkDICOMDatabase::tagToGroupElement(const QString tag, unsigned short& grou
 {
   QStringList groupElement = tag.split(",");
   bool groupOK, elementOK;
+  if (groupElement.length() != 2)
+    {
+    return false;
+    }
   group = groupElement[0].toUInt(&groupOK, 16);
   element = groupElement[1].toUInt(&elementOK, 16);
 
@@ -986,10 +1012,37 @@ void ctkDICOMDatabasePrivate::insertSeries(const ctkDICOMDataset& ctkDataset, QS
 }
 
 //------------------------------------------------------------------------------
+void ctkDICOMDatabase::setTagsToPrecache( const QStringList tags)
+{
+  Q_D(ctkDICOMDatabase);
+  d->TagsToPrecache = tags;
+}
+
+//------------------------------------------------------------------------------
+const QStringList ctkDICOMDatabase::tagsToPrecache()
+{
+  Q_D(ctkDICOMDatabase);
+  return d->TagsToPrecache;
+}
+
+//------------------------------------------------------------------------------
+void ctkDICOMDatabasePrivate::precacheTags( const QString sopInstanceUID )
+{
+  Q_Q(ctkDICOMDatabase);
+  foreach (const QString &tag, this->TagsToPrecache)
+    {
+    q->instanceValue(sopInstanceUID, tag);
+    }
+}
+
+//------------------------------------------------------------------------------
 void ctkDICOMDatabasePrivate::insert( const ctkDICOMDataset& ctkDataset, const QString& filePath, bool storeFile, bool generateThumbnail)
 {
   Q_Q(ctkDICOMDatabase);
 
+  // this is the method that all other insert signatures end up calling
+  // after they have pre-parsed their arguments
+
   QMutexLocker lock(&insertMutex);
 
   // Check to see if the file has already been loaded
@@ -1150,6 +1203,9 @@ void ctkDICOMDatabasePrivate::insert( const ctkDICOMDataset& ctkDataset, const Q
               insertImageStatement.bindValue ( 2, seriesInstanceUID );
               insertImageStatement.bindValue ( 3, QDateTime::currentDateTime() );
               insertImageStatement.exec();
+
+              // insert was needed, so cache any application-requested tags
+              this->precacheTags(sopInstanceUID);
             }
         }
 
@@ -1470,10 +1526,15 @@ bool ctkDICOMDatabase::cacheTag(const QString sopInstanceUID, const QString tag,
       return false;
       }
     }
+  QString valueToInsert(value);
+  if (valueToInsert == "")
+    {
+    valueToInsert = TagNotInInstance;
+    }
   QSqlQuery insertTag( d->TagCacheDatabase );
   insertTag.prepare( "INSERT OR REPLACE INTO TagCache VALUES(:sopInstanceUID, :tag, :value)" );
   insertTag.bindValue(":sopInstanceUID",sopInstanceUID);
   insertTag.bindValue(":tag",tag);
-  insertTag.bindValue(":value",value);
+  insertTag.bindValue(":value",valueToInsert);
   return d->loggedExec(insertTag);
 }

Libs/DICOM/Core/ctkDICOMDatabase.h

@@ -55,6 +55,7 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDatabase : public QObject
   Q_PROPERTY(bool isOpen READ isOpen)
   Q_PROPERTY(QString lastError READ lastError)
   Q_PROPERTY(QString databaseFilename READ databaseFilename)
+  Q_PROPERTY(QStringList tagsToPrecache READ tagsToPrecache WRITE setTagsToPrecache)
 
 public:
   explicit ctkDICOMDatabase(QObject *parent = 0);
@@ -146,6 +147,19 @@ public:
   Q_INVOKABLE QStringList headerKeys ();
   Q_INVOKABLE QString headerValue (const QString key);
 
+  ///
+  /// \brief application-defined tags of interest
+  /// This list of tags is added to the internal tag cache during import
+  /// operations.  The list should be prepared by the application as
+  /// a hint to the database that these tags are likely to be accessed
+  /// later.  Internally, the database will cache the values of these
+  /// tags so that subsequent calls to fileValue or instanceValue will
+  /// be able to use the cache rather than re-reading the file.
+  /// @param tags should be a list of ascii hex group/element tags
+  ///  like "0008,0008" as in the instanceValue and fileValue calls
+  void setTagsToPrecache(const QStringList tags);
+  const QStringList tagsToPrecache();
+
   /// Insert into the database if not already exsting.
   /// @param dataset The dataset to store into the database. Usually, this is
   ///                is a complete DICOM object, like a complete image. However
@@ -161,9 +175,14 @@ public:
   ///                  does only make sense if a full object is received.
   /// @param @generateThumbnail If true, a thumbnail is generated.
   ///
-  Q_INVOKABLE void insert( const ctkDICOMDataset& ctkDataset, bool storeFile, bool generateThumbnail);
-  void insert ( DcmDataset *dataset, bool storeFile = true, bool generateThumbnail = true);
-  Q_INVOKABLE void insert ( const QString& filePath, bool storeFile = true, bool generateThumbnail = true, bool createHierarchy = true, const QString& destinationDirectoryName = QString() );
+  Q_INVOKABLE void insert( const ctkDICOMDataset& ctkDataset, 
+                              bool storeFile, bool generateThumbnail);
+  void insert ( DcmDataset *dataset, 
+                              bool storeFile = true, bool generateThumbnail = true);
+  Q_INVOKABLE void insert ( const QString& filePath, 
+                            bool storeFile = true, bool generateThumbnail = true, 
+                            bool createHierarchy = true, 
+                            const QString& destinationDirectoryName = QString() );
 
   /// Check if file is already in database and up-to-date
   bool fileExistsAndUpToDate(const QString& filePath);

Libs/DICOM/Widgets/ctkDICOMAppWidget.cpp

@@ -296,6 +296,22 @@ bool ctkDICOMAppWidget::searchWidgetPopUpMode(){
   return d->IsSearchWidgetPopUpMode;
 }
 
+//------------------------------------------------------------------------------
+void ctkDICOMAppWidget::setTagsToPrecache( const QStringList tags)
+{
+  Q_D(ctkDICOMAppWidget);
+  d->DICOMDatabase->setTagsToPrecache(tags);
+}
+
+//------------------------------------------------------------------------------
+const QStringList ctkDICOMAppWidget::tagsToPrecache()
+{
+  Q_D(ctkDICOMAppWidget);
+  return d->DICOMDatabase->tagsToPrecache();
+}
+
+
+
 //----------------------------------------------------------------------------
 void ctkDICOMAppWidget::setSearchWidgetPopUpMode(bool flag){
   Q_D(ctkDICOMAppWidget);

Libs/DICOM/Widgets/ctkDICOMAppWidget.h

@@ -36,14 +36,23 @@ class CTK_DICOM_WIDGETS_EXPORT ctkDICOMAppWidget : public QWidget
   Q_OBJECT
   Q_PROPERTY(QString databaseDirectory READ databaseDirectory WRITE setDatabaseDirectory)
   Q_PROPERTY(bool searchWidgetPopUpMode READ searchWidgetPopUpMode WRITE setSearchWidgetPopUpMode)
+  Q_PROPERTY(QStringList tagsToPrecache READ tagsToPrecache WRITE setTagsToPrecache)
 
 public:
   typedef QWidget Superclass;
   explicit ctkDICOMAppWidget(QWidget* parent=0);
   virtual ~ctkDICOMAppWidget();
 
+  /// Directory being used to store the dicom database
   QString databaseDirectory() const;
 
+  /// See ctkDICOMDatabase for description - these accessors
+  /// delegate to the corresponding routines of the internal
+  /// instance of the database.
+  /// @see ctkDICOMDatabase
+  void setTagsToPrecache(const QStringList tags);
+  const QStringList tagsToPrecache();
+
   /// Updates schema of loaded database to match the one
   /// coded by the current version of ctkDICOMDatabase.
   /// Also provides a dialog box for progress