Merge branch '357-dicom-insert-delete-reinsert'

* 357-dicom-insert-delete-reinsert:
  If given a new or newly modified file, update the Images database row
  WIP - don't commit - some debugging code
  STYLE: clarify that a variable is a query
  #357 Add test that fails with current CTK
  #357 Add test that fails with current CTK

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

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

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

@@ -0,0 +1,110 @@
+/*=========================================================================
+
+  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 ctkDICOMDatabaseTest5( 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;
+    }
+  std::cerr << "Database is in " << databaseDirectory.path().toStdString() << std::endl;
+
+
+  //
+  // First, copy the passed file to two temp directories
+  //
+  databaseDirectory.rmdir("firstInsert");
+  databaseDirectory.rmdir("secondInsert");
+  databaseDirectory.mkdir("firstInsert");
+  databaseDirectory.mkdir("secondInsert");
+  QString firstDestination = QDir::cleanPath(databaseDirectory.absolutePath() + QDir::separator() + "firstInsert" + QDir::separator() + "file.dcm");
+  QString secondDestination = QDir::cleanPath(databaseDirectory.absolutePath() + QDir::separator() + "secondInsert" + QDir::separator() + "file.dcm");
+  QFile::copy(dicomFilePath, firstDestination);
+  std::cerr << "copied to: " << firstDestination.toStdString() << std::endl;
+  QFile::copy(dicomFilePath, secondDestination);
+  std::cerr << "copied to: " << secondDestination.toStdString() << std::endl;
+
+  //
+  // Insert the first instance
+  //
+  database.insert(firstDestination, false, false);
+
+  //
+  // Now delete the first instance and try inserting the second
+  //
+  QFile::remove(firstDestination);
+  database.insert(secondDestination, false, false);
+
+  //
+  // At this point, the file associated with the instance UID should be
+  // the secondDestination - it's an error if not.
+  //
+  QString instanceUID("1.2.840.113619.2.135.3596.6358736.4843.1115808177.83");
+  QString filePathInDatabase = database.fileForInstance(instanceUID);
+
+  if (filePathInDatabase != secondDestination)
+    {
+    std::cerr << "ctkDICOMDatabase thinks instance is in " << filePathInDatabase.toStdString() << std::endl;
+    std::cerr << "But we just inserted it from " << secondDestination.toStdString() << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  //
+  // Close and clean up
+  //
+  database.closeDatabase();
+
+
+  return EXIT_SUCCESS;
+}

Libs/DICOM/Core/ctkDICOMDatabase.cpp

@@ -1113,21 +1113,21 @@ void ctkDICOMDatabasePrivate::insert( const ctkDICOMItem& ctkDataset, const QStr
   
   QString sopInstanceUID ( ctkDataset.GetElementAsString(DCM_SOPInstanceUID) );
 
-  QSqlQuery fileExists ( Database );
-  fileExists.prepare("SELECT InsertTimestamp,Filename FROM Images WHERE SOPInstanceUID == :sopInstanceUID");
-  fileExists.bindValue(":sopInstanceUID",sopInstanceUID);
+  QSqlQuery fileExistsQuery ( Database );
+  fileExistsQuery.prepare("SELECT InsertTimestamp,Filename FROM Images WHERE SOPInstanceUID == :sopInstanceUID");
+  fileExistsQuery.bindValue(":sopInstanceUID",sopInstanceUID);
   {
-  bool success = fileExists.exec();
+  bool success = fileExistsQuery.exec();
   if (!success)
     {
-      logger.error("SQLITE ERROR: " + fileExists.lastError().driverText());
+      logger.error("SQLITE ERROR: " + fileExistsQuery.lastError().driverText());
       return;
     }
-  }
+  fileExistsQuery.next();
 
-  QString databaseFilename(fileExists.value(1).toString());
+  QString databaseFilename(fileExistsQuery.value(1).toString());
   QDateTime fileLastModified(QFileInfo(databaseFilename).lastModified());
-  QDateTime databaseInsertTimestamp(QDateTime::fromString(fileExists.value(0).toString(),Qt::ISODate));
+  QDateTime databaseInsertTimestamp(QDateTime::fromString(fileExistsQuery.value(0).toString(),Qt::ISODate));
 
   qDebug() << "inserting filePath: " << filePath;
   if (databaseFilename == "")
@@ -1136,15 +1136,25 @@ void ctkDICOMDatabasePrivate::insert( const ctkDICOMItem& ctkDataset, const QStr
     }
   else
     {
-      qDebug() << "database filename for " << sopInstanceUID << " is: " << databaseFilename;
-      qDebug() << "modified date is: " << fileLastModified;
-      qDebug() << "db insert date is: " << databaseInsertTimestamp;
-      if ( fileExists.next() && fileLastModified < databaseInsertTimestamp )
+      if ( databaseFilename == filePath && fileLastModified < databaseInsertTimestamp )
         {
           logger.debug ( "File " + databaseFilename + " already added" );
           return;
         }
+      else
+        {
+        QSqlQuery deleteFile ( Database );
+        deleteFile.prepare("DELETE FROM Images WHERE SOPInstanceUID == :sopInstanceUID");
+        deleteFile.bindValue(":sopInstanceUID",sopInstanceUID);
+        bool success = deleteFile.exec();
+        if (!success)
+          {
+            logger.error("SQLITE ERROR deleting old image row: " + deleteFile.lastError().driverText());
+            return;
+          }
+        }
     }
+  }
 
   //If the following fields can not be evaluated, cancel evaluation of the DICOM file
   QString patientsName(ctkDataset.GetElementAsString(DCM_PatientName) );
@@ -1248,14 +1258,17 @@ void ctkDICOMDatabasePrivate::insert( const ctkDICOMItem& ctkDataset, const QStr
 
           // let users of this class track when things happen
           emit q->studyAdded(studyInstanceUID);
+          qDebug() << "Study Added";
         }
 
+
       if ( seriesInstanceUID != "" && seriesInstanceUID != LastSeriesInstanceUID )
         {
           insertSeries(ctkDataset, studyInstanceUID);
 
           // let users of this class track when things happen
           emit q->seriesAdded(seriesInstanceUID);
+          qDebug() << "Series Added";
         }
       // TODO: what to do with imported files
       //
@@ -1265,6 +1278,7 @@ void ctkDICOMDatabasePrivate::insert( const ctkDICOMItem& ctkDataset, const QStr
           checkImageExistsQuery.prepare ( "SELECT * FROM Images WHERE Filename = ?" );
           checkImageExistsQuery.bindValue ( 0, filename );
           checkImageExistsQuery.exec();
+          qDebug() << "Maybe add Instance";
           if(!checkImageExistsQuery.next())
             {
               QSqlQuery insertImageStatement ( Database );
@@ -1280,6 +1294,7 @@ void ctkDICOMDatabasePrivate::insert( const ctkDICOMItem& ctkDataset, const QStr
 
               // let users of this class track when things happen
               emit q->instanceAdded(sopInstanceUID);
+              qDebug() << "Instance Added";
             }
         }
 
@@ -1353,22 +1368,22 @@ bool ctkDICOMDatabase::removeSeries(const QString& seriesInstanceUID)
   Q_D(ctkDICOMDatabase);
 
   // get all images from series
-  QSqlQuery fileExists ( d->Database );
-  fileExists.prepare("SELECT Filename, SOPInstanceUID, StudyInstanceUID FROM Images,Series WHERE Series.SeriesInstanceUID = Images.SeriesInstanceUID AND Images.SeriesInstanceUID = :seriesID");
-  fileExists.bindValue(":seriesID",seriesInstanceUID);
-  bool success = fileExists.exec();
+  QSqlQuery fileExistsQuery ( d->Database );
+  fileExistsQuery.prepare("SELECT Filename, SOPInstanceUID, StudyInstanceUID FROM Images,Series WHERE Series.SeriesInstanceUID = Images.SeriesInstanceUID AND Images.SeriesInstanceUID = :seriesID");
+  fileExistsQuery.bindValue(":seriesID",seriesInstanceUID);
+  bool success = fileExistsQuery.exec();
   if (!success)
     {
-      logger.error("SQLITE ERROR: " + fileExists.lastError().driverText());
+      logger.error("SQLITE ERROR: " + fileExistsQuery.lastError().driverText());
       return false;
     }
 
   QList< QPair<QString,QString> > removeList;
-  while ( fileExists.next() )
+  while ( fileExistsQuery.next() )
     {
-      QString dbFilePath = fileExists.value(fileExists.record().indexOf("Filename")).toString();
-      QString sopInstanceUID = fileExists.value(fileExists.record().indexOf("SOPInstanceUID")).toString();
-      QString studyInstanceUID = fileExists.value(fileExists.record().indexOf("StudyInstanceUID")).toString();
+      QString dbFilePath = fileExistsQuery.value(fileExistsQuery.record().indexOf("Filename")).toString();
+      QString sopInstanceUID = fileExistsQuery.value(fileExistsQuery.record().indexOf("SOPInstanceUID")).toString();
+      QString studyInstanceUID = fileExistsQuery.value(fileExistsQuery.record().indexOf("StudyInstanceUID")).toString();
       QString internalFilePath = studyInstanceUID + "/" + seriesInstanceUID + "/" + sopInstanceUID;
       removeList << qMakePair(dbFilePath,internalFilePath);
     }