refs #6449 more unit tests for MDBoxSaveable

and small changes related to its enhancement.

Code/Mantid/Framework/API/test/BoxControllerTest.h

@@ -4,41 +4,16 @@
 #include "MantidKernel/DiskBuffer.h"
 #include "MantidAPI/BoxController.h"
 #include "MantidAPI/IBoxControllerIO.h"
+#include "MantidTestHelpers/BoxControllerDummyIO.h"
 #include <cxxtest/TestSuite.h>
 #include <map>
 #include <memory>
+#include <boost/make_shared.hpp>
 
 using namespace Mantid;
 using namespace Mantid::API;
 using namespace Mantid::Kernel;
 
-class FakeBoxControllerIO : public IBoxControllerIO
-{
-    std::string m_fileName;
-    bool m_isOpened;
-public:
-    FakeBoxControllerIO():m_isOpened(false){}
-    bool openFile(const std::string &fileName,const std::string &mode)
-    {
-        this->m_fileName=fileName;
-        m_isOpened=true;
-        return true;
-    }
-    virtual bool isOpened()const
-    { return this->m_isOpened ;}
-    const std::string &getFileName()const
-    {return m_fileName;}
-
-    virtual void saveBlock(void const * const /* Block */, const uint64_t /*blockPosition*/,const size_t /*blockSize*/){};
-    virtual void loadBlock(void  * const  /* Block */, const uint64_t /*blockPosition*/,const size_t /*blockSize*/){};
-    virtual void flushData(){};
-    virtual void closeFile()
-    {m_isOpened=false;}
-
-     ~FakeBoxControllerIO()
-     {this->closeFile();}
-
-};
 class BoxControllerTest :    public CxxTest::TestSuite
 {
 public:
@@ -227,23 +202,23 @@ class BoxControllerTest :    public CxxTest::TestSuite
 
   void test_CloneFileBased()
   {
-    BoxController a(2);
-    a.setMaxDepth(4);
-    a.setSplitInto(10);
-    a.setMaxDepth(10);
-    a.setMaxId(123456);
-    TS_ASSERT_THROWS_NOTHING(a.setFileBacked(new FakeBoxControllerIO(),"fakeFile"));
-    TS_ASSERT(a.isFileBacked());
-
-    BoxController_sptr b  = a.clone();
+    BoxController_sptr a = boost::shared_ptr<BoxController>(new BoxController(2));
+    a->setMaxDepth(4);
+    a->setSplitInto(10);
+    a->setMaxDepth(10);
+    a->setMaxId(123456);
+    TS_ASSERT_THROWS_NOTHING(a->setFileBacked(new MantidTestHelpers::BoxControllerDummyIO(a),"fakeFile"));
+    TS_ASSERT(a->isFileBacked());
+
+    BoxController_sptr b  = a->clone();
     // Check that settings are the same but BC are different
-    compareBoxControllers(a, *b);
+    compareBoxControllers(*a, *b);
 
     TS_ASSERT(!b->isFileBacked());
-    TS_ASSERT_THROWS_NOTHING(b->setFileBacked(new FakeBoxControllerIO(),"fakeFile2"));
+    TS_ASSERT_THROWS_NOTHING(b->setFileBacked(new MantidTestHelpers::BoxControllerDummyIO(b),"fakeFile2"));
 
     // Check that settings are the same but BC are different
-    compareBoxControllers(a, *b);
+    compareBoxControllers(*a, *b);
     TS_ASSERT(b->isFileBacked());
 
 
@@ -252,15 +227,14 @@ class BoxControllerTest :    public CxxTest::TestSuite
 
   void test_MRU_access()
   {
-    BoxController a(2);
-    DiskBuffer & dbuf = a.getDiskBuffer();
-    // Set the cache parameters
+    BoxController_sptr a = boost::shared_ptr<BoxController>(new BoxController(2));
 
-    // Can't have 0-sized events
-    TS_ASSERT_THROWS_ANYTHING( a.setCacheParameters(0, 4560) );
-    a.setCacheParameters(40, 123);
+    a->setFileBacked(new MantidTestHelpers::BoxControllerDummyIO(a),"existingFakeFile");
+    DiskBuffer * dbuf = a->getFileIO();
 
-    TS_ASSERT_EQUALS( dbuf.getWriteBufferSize(), 123);
+    // Set the cache parameters
+    TS_ASSERT_THROWS_NOTHING(dbuf->setWriteBufferSize(123));
+    TS_ASSERT_EQUALS( dbuf->getWriteBufferSize(), 123);
   }
 
   void test_construction_defaults()
@@ -270,7 +244,6 @@ class BoxControllerTest :    public CxxTest::TestSuite
     TS_ASSERT_EQUALS(2, box_controller.getNDims());
     TS_ASSERT_EQUALS(1, box_controller.getNumSplit());
     TS_ASSERT_EQUALS(0, box_controller.getMaxId());
-    //TS_ASSERT_EQUALS(true, box_controller.useWriteBuffer());
   }
 
 

Code/Mantid/Framework/API/test/CMakeLists.txt

@@ -4,7 +4,7 @@ if ( CXXTEST_FOUND )
   include_directories ( ../../TestHelpers/inc )
   # This variable is used within the cxxtest_add_test macro to build these helper classes into the test executable.
   # It will go out of scope at the end of this file so doesn't need un-setting
-  set ( TESTHELPER_SRCS ../../TestHelpers/src/ComponentCreationHelper.cpp )
+  set ( TESTHELPER_SRCS ../../TestHelpers/src/ComponentCreationHelper.cpp  ../../TestHelpers/src/BoxControllerDummyIO.cpp)
 
   if ( GMOCK_FOUND AND GTEST_FOUND )
     cxxtest_add_test ( APITest ${TEST_FILES} ${GMOCK_TEST_FILES})

Code/Mantid/Framework/Kernel/inc/MantidKernel/ISaveable.h

@@ -2,6 +2,7 @@
 #define MANTID_KERNEL_ISAVEABLE_H_
 
 #include "MantidKernel/System.h"
+#include "MantidKernel/MultiThreaded.h"
 #include <list>
 #include <vector>
 #include <algorithm>
@@ -46,7 +47,7 @@ namespace Kernel
   {
   public:
     ISaveable();
-    //ISaveable(const ISaveable &other); --> no pointers, standard CC
+    ISaveable(const ISaveable &other); 
     virtual ~ISaveable(){};
 
 
@@ -102,6 +103,8 @@ namespace Kernel
     uint64_t m_fileIndexStart;
     /// Number of events saved in the file, after the start index location
     uint64_t m_fileNumEvents;  
+
+    Kernel::Mutex m_setter; 
   private:
     // the iterator which describes the position of this object in the DiskBuffer. Undefined if not placed to buffer
     boost::optional< std::list<ISaveable * const >::iterator> m_BufPosition;

Code/Mantid/Framework/Kernel/inc/MantidKernel/Saveable.h

@@ -45,51 +45,27 @@ namespace Kernel
 
     /// @return true if it the data of the object is busy and so cannot be cleared; false if the data was released and can be cleared/written.
     bool isBusy() const
-    {
-      return m_Busy;
-    }
-    /// @ set the data busy to prevent from removing them from memory. The process which does that should clean the data when finished with them
-    void setBusy(bool On)
-    {
-        m_Busy=On;
-    }
+    {   return m_Busy;  }
+
     /** Returns the state of the parameter, which tells disk buffer to force writing data 
      * to disk despite the size of the object have not changed (so one have probably done something with object contents. */
-    bool isDataChanged()const{return m_dataChanged;}
-
-    /** Call this method from the method which changes the object but keeps the object size the same to tell DiskBuffer to write it back
-        the dataChanged ID is reset after save from the DataBuffer is emptied   */
-    void setDataChanged()
-    { 
-      if(this->wasSaved())m_dataChanged=true;
-    }
-    /** this method has to be called if the object has been discarded from memory and is not changed any more. 
-    It expected to be called from clearDataFromMemory. */
-    void clearDataChanged()
-    {
-      m_dataChanged=false;
-    }
-
-
-    /** Sets the location of the object on HDD 
-     *@param newPos -- the file position where the opbject should/was saved
-     *@param newSize -- the object size on file (in some object units)
-     *@param wasSaved -- if true, the object was indeed saved by some other means so can be loaded if necessary. if false, only place for it is reserved
-    */
-    void setFilePosition(uint64_t newPos,size_t newSize,bool wasSaved);
+    bool isDataChanged()const
+    {return m_dataChanged;}
 
     /** function returns true if the object have ever been saved on HDD and knows it place there*/
-    bool wasSaved()const
-    { // for speed it returns this boolean, but for relaibility this should be m_wasSaved&&(m_fileIndexStart!=max())
-      return m_wasSaved;  
-    }
+    bool wasSaved()const // for speed it returns this boolean, but for relaibility this should be m_wasSaved&&(m_fileIndexStart!=max())
+    {  return m_wasSaved;    }
 
-
     bool isLoaded()const
     { return m_isLoaded;}
 
-    void setLoaded(bool Yes)
-    { m_isLoaded = Yes;}
+
+    void setBusy(bool On);
+    void setDataChanged();
+    void clearDataChanged();
+    void setFilePosition(uint64_t newPos,size_t newSize,bool wasSaved);
+    void setLoaded(bool Yes);
+
   protected:
     //-------------- 
     /// a user needs to set this variable to true preventing from deleting data from buffer

Code/Mantid/Framework/Kernel/src/ISaveable.cpp

@@ -16,12 +16,10 @@ namespace Kernel
   {}
 
   //----------------------------------------------------------------------------------------------
-  /** Copy constructor --> needed? for std containers.
-   */
-  //ISaveable::ISaveable(const ISaveable & other):
-  ////m_FileId(other.m_FileId),
-  // m_fileIndexStart(other.m_fileIndexStart),m_fileNumEvents(other.m_fileNumEvents)
-  //{ }
+  /** Copy constructor --> needed for std containers and not to copy mutexes   */
+  ISaveable::ISaveable(const ISaveable & other):
+     m_fileIndexStart(other.m_fileIndexStart),m_fileNumEvents(other.m_fileNumEvents)
+ { }
 
   //ISaveable::ISaveable(const size_t fileId):
   // m_FileId(fileId),m_fileIndexStart(std::numeric_limits<uint64_t>::max() ),m_fileNumEvents(0),m_BufMemorySize(0)
@@ -34,25 +32,30 @@ namespace Kernel
   */
   size_t ISaveable::setBufferPosition(std::list<ISaveable *const>::iterator &bufPosition)
   {
+      m_setter.lock();
       m_BufPosition = boost::optional<std::list<ISaveable *const>::iterator >(bufPosition);
       m_BufMemorySize  = this->getDataMemorySize();
+      m_setter.unlock();
       return m_BufMemorySize ;
   }
 
   /** private function which used by the disk buffer to save the contents of the  */
   void ISaveable::saveAt(uint64_t newPos, uint64_t newSize)
   {
-
+      m_setter.lock();
       m_fileIndexStart= newPos;
       m_fileNumEvents = newSize;
       this->save();
       this->clearDataFromMemory();      
+      m_setter.unlock();
   }
   /// clears the state of the object, and indicate that it is not stored in buffer any more 
   void ISaveable::clearBufferState()
   {
+      m_setter.lock();
       m_BufMemorySize=0;
       m_BufPosition = boost::optional<std::list<ISaveable *const>::iterator>();
+      m_setter.unlock();
   }
 } // namespace Mantid
 } // namespace Kernel

Code/Mantid/Framework/Kernel/src/Saveable.cpp

@@ -4,58 +4,77 @@
 
 namespace Mantid
 {
-namespace Kernel
-{
+    namespace Kernel
+    {
+
+
+        //----------------------------------------------------------------------------------------------
+        /** Constructor
+        */
+        Saveable::Saveable()
+            :ISaveable(),
+            m_Busy(false),m_dataChanged(false),m_wasSaved(false),m_isLoaded(false)
+        {
+        }
+
+        //----------------------------------------------------------------------------------------------
+        /** Copy constructor --> big qusetions about the validity of such implementation
+        */
+        Saveable::Saveable(const Saveable & other)
+            :ISaveable(other),
+            m_Busy(other.m_Busy),m_dataChanged(other.m_dataChanged),m_wasSaved(other.m_wasSaved),m_isLoaded(false)
+        {
+        }
+
+        /// @ set the data busy to prevent from removing them from memory. The process which does that should clean the data when finished with them
+        void Saveable::setBusy(bool On)
+        {
+            m_Busy=On;
+        }
+        /** Call this method from the method which changes the object but keeps the object size the same to tell DiskBuffer to write it back
+        the dataChanged ID is reset after save from the DataBuffer is emptied   */
+        void Saveable::setDataChanged()
+        { 
+            if(this->wasSaved())m_dataChanged=true;
+        }
+        /** this method has to be called if the object has been discarded from memory and is not changed any more. 
+        It expected to be called from clearDataFromMemory. */
+        void Saveable::clearDataChanged()
+        {
+            m_dataChanged=false;
+        }
+
+
+        void Saveable::setLoaded(bool Yes)
+        { m_isLoaded = Yes;}
+
+
+        //----------------------------------------------------------------------------------------------
+        /** Destructor
+        */
+        Saveable::~Saveable()
+        {
+        }
+
+
+
+        /** Set the start/end point in the file where the events are located
+        * @param newPos :: start point,
+        * @param newSize :: number of events in the file   
+        * @param wasSaved :: flag to mark if the info was saved, by default it does
+        */
+        void Saveable::setFilePosition(uint64_t newPos, size_t newSize, bool wasSaved)
+        {  
+            this->m_setter.lock();
+            this->m_fileIndexStart=newPos;  
+            this->m_fileNumEvents =static_cast<uint64_t>(newSize);
+            m_wasSaved = wasSaved;
+            this->m_setter.unlock();
+        }
+
+
 
 
-//----------------------------------------------------------------------------------------------
-  /** Constructor
-   */
-  Saveable::Saveable()
-      :ISaveable(),
-       m_Busy(false),m_dataChanged(false),m_wasSaved(false),m_isLoaded(false)
-  {
-  }
-
-  //----------------------------------------------------------------------------------------------
-  /** Copy constructor --> big qusetions about the validity of such implementation
-   */
-  Saveable::Saveable(const Saveable & other)
-      :ISaveable(other),
-        m_Busy(other.m_Busy),m_dataChanged(other.m_dataChanged),m_wasSaved(other.m_wasSaved),m_isLoaded(false)
-  {
-  }
-
- /* Saveable::Saveable(const size_t id)
-      :ISaveable(id),
-       m_Busy(false),m_dataChanged(false),m_wasSaved(false),m_isLoaded(false)
-  {
-  }*/
-
-  //----------------------------------------------------------------------------------------------
-  /** Destructor
-   */
-  Saveable::~Saveable()
-  {
-  }
-
-
-
-  /** Set the start/end point in the file where the events are located
-     * @param newPos :: start point,
-     * @param newSize :: number of events in the file   
-     * @param wasSaved :: flag to mark if the info was saved, by default it does
-   */
-  void Saveable::setFilePosition(uint64_t newPos, size_t newSize, bool wasSaved)
-  {  
-      this->m_fileIndexStart=newPos;  
-      this->m_fileNumEvents =static_cast<uint64_t>(newSize);
-      m_wasSaved = wasSaved;
-  }
-
-
-
-
-} // namespace Mantid
+    } // namespace Mantid
 } // namespace Kernel