BatchImporter: make validation configurable and add unittest for class.

code/BaseImporter.cpp

@@ -481,7 +481,7 @@ namespace Assimp
         BatchLoader::PropertyMap map;
         unsigned int id;
 
-        bool operator== (const std::string& f) {
+        bool operator== (const std::string& f) const {
             return file == f;
         }
     };
@@ -489,13 +489,22 @@ namespace Assimp
 
 // ------------------------------------------------------------------------------------------------
 // BatchLoader::pimpl data structure
-struct Assimp::BatchData
+struct Assimp::BatchData {
-{
+    BatchData( IOSystem* pIO, bool validate )
-    BatchData()
+    : pIOSystem( pIO )
-        : pIOSystem()
+    , pImporter( nullptr )
-        , pImporter()
+    , next_id(0xffff)
-        , next_id(0xffff)
+    , validate( validate ) {
-    {}
+        ai_assert( NULL != pIO );
+        
+        pImporter = new Importer();
+        pImporter->SetIOHandler( pIO );
+    }
+
+    ~BatchData() {
+        pImporter->SetIOHandler( NULL ); /* get pointer back into our possession */
+        delete pImporter;
+    }
 
     // IO system to be used for all imports
     IOSystem* pIOSystem;
@@ -511,53 +520,59 @@ struct Assimp::BatchData
 
     // Id for next item
     unsigned int next_id;
+
+    // Validation enabled state
+    bool validate;
 };
 
+typedef std::list<LoadRequest>::iterator LoadReqIt;
+
 // ------------------------------------------------------------------------------------------------
-BatchLoader::BatchLoader(IOSystem* pIO)
+BatchLoader::BatchLoader(IOSystem* pIO, bool validate )
 {
     ai_assert(NULL != pIO);
 
-    data = new BatchData();
+    m_data = new BatchData( pIO, validate );
-    data->pIOSystem = pIO;
-
-    data->pImporter = new Importer();
-    data->pImporter->SetIOHandler(data->pIOSystem);
 }
 
 // ------------------------------------------------------------------------------------------------
 BatchLoader::~BatchLoader()
 {
-    // delete all scenes wthat have not been polled by the user
+    // delete all scenes what have not been polled by the user
-    for (std::list<LoadRequest>::iterator it = data->requests.begin();it != data->requests.end(); ++it) {
+    for ( LoadReqIt it = m_data->requests.begin();it != m_data->requests.end(); ++it) {
-
         delete (*it).scene;
     }
-    data->pImporter->SetIOHandler(NULL); /* get pointer back into our possession */
+    delete m_data;
-    delete data->pImporter;
-    delete data;
 }
 
+// ------------------------------------------------------------------------------------------------
+void BatchLoader::setValidation( bool enabled ) {
+    m_data->validate = enabled;
+}
 
 // ------------------------------------------------------------------------------------------------
-unsigned int BatchLoader::AddLoadRequest    (const std::string& file,
+bool BatchLoader::getValidation() const {
+    return m_data->validate;
+}
+
+// ------------------------------------------------------------------------------------------------
+unsigned int BatchLoader::AddLoadRequest(const std::string& file,
     unsigned int steps /*= 0*/, const PropertyMap* map /*= NULL*/)
 {
     ai_assert(!file.empty());
 
     // check whether we have this loading request already
-    std::list<LoadRequest>::iterator it;
+    for ( LoadReqIt it = m_data->requests.begin();it != m_data->requests.end(); ++it)  {
-    for (it = data->requests.begin();it != data->requests.end(); ++it)  {
-
         // Call IOSystem's path comparison function here
-        if (data->pIOSystem->ComparePaths((*it).file,file)) {
+        if ( m_data->pIOSystem->ComparePaths((*it).file,file)) {
-
             if (map) {
-                if (!((*it).map == *map))
+                if ( !( ( *it ).map == *map ) ) {
                     continue;
+                }
             }
-            else if (!(*it).map.empty())
+            else if ( !( *it ).map.empty() ) {
                 continue;
+            }
 
             (*it).refCnt++;
             return (*it).id;
@@ -565,20 +580,18 @@ unsigned int BatchLoader::AddLoadRequest    (const std::string& file,
     }
 
     // no, we don't have it. So add it to the queue ...
-    data->requests.push_back(LoadRequest(file,steps,map,data->next_id));
+    m_data->requests.push_back(LoadRequest(file,steps,map, m_data->next_id));
-    return data->next_id++;
+    return m_data->next_id++;
 }
 
 // ------------------------------------------------------------------------------------------------
-aiScene* BatchLoader::GetImport     (unsigned int which)
+aiScene* BatchLoader::GetImport( unsigned int which )
 {
-    for (std::list<LoadRequest>::iterator it = data->requests.begin();it != data->requests.end(); ++it) {
+    for ( LoadReqIt it = m_data->requests.begin();it != m_data->requests.end(); ++it) {
-
         if ((*it).id == which && (*it).loaded)  {
-
             aiScene* sc = (*it).scene;
             if (!(--(*it).refCnt))  {
-                data->requests.erase(it);
+                m_data->requests.erase(it);
             }
             return sc;
         }
@@ -590,14 +603,15 @@ aiScene* BatchLoader::GetImport     (unsigned int which)
 void BatchLoader::LoadAll()
 {
     // no threaded implementation for the moment
-    for (std::list<LoadRequest>::iterator it = data->requests.begin();it != data->requests.end(); ++it) {
+    for ( LoadReqIt it = m_data->requests.begin();it != m_data->requests.end(); ++it) {
         // force validation in debug builds
         unsigned int pp = (*it).flags;
-#ifdef ASSIMP_BUILD_DEBUG
+        if ( m_data->validate ) {
-        pp |= aiProcess_ValidateDataStructure;
+            pp |= aiProcess_ValidateDataStructure;
-#endif
+        }
+
         // setup config properties if necessary
-        ImporterPimpl* pimpl = data->pImporter->Pimpl();
+        ImporterPimpl* pimpl = m_data->pImporter->Pimpl();
         pimpl->mFloatProperties  = (*it).map.floats;
         pimpl->mIntProperties    = (*it).map.ints;
         pimpl->mStringProperties = (*it).map.strings;
@@ -608,8 +622,8 @@ void BatchLoader::LoadAll()
             DefaultLogger::get()->info("%%% BEGIN EXTERNAL FILE %%%");
             DefaultLogger::get()->info("File: " + (*it).file);
         }
-        data->pImporter->ReadFile((*it).file,pp);
+        m_data->pImporter->ReadFile((*it).file,pp);
-        (*it).scene = data->pImporter->GetOrphanedScene();
+        (*it).scene = m_data->pImporter->GetOrphanedScene();
         (*it).loaded = true;
 
         DefaultLogger::get()->info("%%% END EXTERNAL FILE %%%");

code/BaseImporter.h

@@ -347,7 +347,12 @@ public: // static utilities
     static void ConvertUTF8toISO8859_1(
         std::string& data);
 
-    enum TextFileMode { ALLOW_EMPTY, FORBID_EMPTY };
+    // -------------------------------------------------------------------
+    /// @brief  Enum to define, if empty files are ok or not.
+    enum TextFileMode { 
+        ALLOW_EMPTY,
+        FORBID_EMPTY 
+    };
 
     // -------------------------------------------------------------------
     /** Utility for text file loaders which copies the contents of the
@@ -382,14 +387,10 @@ public: // static utilities
         }
     }
 
-    
-
 protected:
-
+    /// Error description in case there was one.
-    /** Error description in case there was one. */
     std::string m_ErrorText;
-
+    /// Currently set progress handler.
-    /** Currently set progress handler */
     ProgressHandler* m_progress;
 };
 

code/Importer.h

@@ -39,6 +39,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */
 
 /** @file Importer.h mostly internal stuff for use by #Assimp::Importer */
+#pragma once
 #ifndef INCLUDED_AI_IMPORTER_H
 #define INCLUDED_AI_IMPORTER_H
 
@@ -133,12 +134,11 @@ struct BatchData;
  *  could, this has not yet been implemented at the moment).
  *
  *  @note The class may not be used by more than one thread*/
-class BatchLoader
+class ASSIMP_API BatchLoader
 {
     // friend of Importer
 
 public:
-
     //! @cond never
     // -------------------------------------------------------------------
     /** Wraps a full list of configuration properties for an importer.
@@ -162,15 +162,29 @@ public:
     //! @endcond
 
 public:
-
-
     // -------------------------------------------------------------------
     /** Construct a batch loader from a given IO system to be used
-     *  to access external files */
+     *  to access external files 
-    explicit BatchLoader(IOSystem* pIO);
+     */
+    explicit BatchLoader(IOSystem* pIO, bool validate = false );
+
+    // -------------------------------------------------------------------
+    /** The class destructor.
+     */
     ~BatchLoader();
 
-
+    // -------------------------------------------------------------------
+    /** Sets the validation step. True for enable validation during postprocess.
+     *  @param  enable  True for validation.
+     */
+    void setValidation( bool enabled );
+    
+    // -------------------------------------------------------------------
+    /** Returns the current validation step.
+     *  @return The current validation step.
+     */
+    bool getValidation() const;
+    
     // -------------------------------------------------------------------
     /** Add a new file to the list of files to be loaded.
      *  @param file File to be loaded
@@ -185,7 +199,6 @@ public:
         const PropertyMap* map = NULL
         );
 
-
     // -------------------------------------------------------------------
     /** Get an imported scene.
      *  This polls the import from the internal request list.
@@ -199,20 +212,16 @@ public:
         unsigned int which
         );
 
-
     // -------------------------------------------------------------------
     /** Waits until all scenes have been loaded. This returns
      *  immediately if no scenes are queued.*/
     void LoadAll();
 
 private:
-
     // No need to have that in the public API ...
-    BatchData* data;
+    BatchData *m_data;
 };
 
-}
+} // Namespace Assimp
 
-
+#endif // INCLUDED_AI_IMPORTER_H
-
-#endif

test/CMakeLists.txt

@@ -55,7 +55,9 @@ SOURCE_GROUP( unit FILES
 )
 
 SET( TEST_SRCS
+  unit/TestIOSystem.h
   unit/AssimpAPITest.cpp
+  unit/utBatchLoader.cpp
   unit/utBlenderIntermediate.cpp
   unit/utBlendImportAreaLight.cpp
   unit/utBlendImportMaterials.cpp

test/unit/TestIOSystem.h

@@ -0,0 +1,29 @@
+#pragma once
+
+#include "UnitTestPCH.h"
+
+#include <assimp/IOSystem.hpp>
+
+using namespace std;
+using namespace Assimp;
+
+static const string Sep = "/";
+class TestIOSystem : public IOSystem {
+public:
+    TestIOSystem() : IOSystem() {}
+    virtual ~TestIOSystem() {}
+    virtual bool Exists( const char* ) const {
+        return true;
+    }
+    virtual char getOsSeparator() const {
+        return Sep[ 0 ];
+    }
+
+    virtual IOStream* Open( const char* pFile, const char* pMode = "rb" ) {
+        return NULL;
+    }
+
+    virtual void Close( IOStream* pFile ) {
+        // empty
+    }
+};

test/unit/utBatchLoader.cpp

@@ -0,0 +1,78 @@
+/*
+---------------------------------------------------------------------------
+Open Asset Import Library (assimp)
+---------------------------------------------------------------------------
+
+Copyright (c) 2006-2016, assimp team
+
+All rights reserved.
+
+Redistribution and use of this software in source and binary forms,
+with or without modification, are permitted provided that the following
+conditions are met:
+
+* Redistributions of source code must retain the above
+copyright notice, this list of conditions and the
+following disclaimer.
+
+* Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the
+following disclaimer in the documentation and/or other
+materials provided with the distribution.
+
+* Neither the name of the assimp team, nor the names of its
+contributors may be used to endorse or promote products
+derived from this software without specific prior
+written permission of the assimp team.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+---------------------------------------------------------------------------
+*/
+#include "UnitTestPCH.h"
+#include "Importer.h"
+#include "TestIOSystem.h"
+
+using namespace ::Assimp;
+
+class BatchLoaderTest : public ::testing::Test {
+public:
+    virtual void SetUp() {
+        m_io = new TestIOSystem();
+    }
+
+    virtual void TearDown() {
+        delete m_io;
+    }
+
+protected:
+    TestIOSystem* m_io;
+};
+
+TEST_F( BatchLoaderTest, createTest ) {
+    bool ok( true );
+    try {
+        BatchLoader loader( m_io );
+    } catch ( ... ) {
+        ok = false;
+    }
+}
+
+TEST_F( BatchLoaderTest, validateAccessTest ) {
+    BatchLoader loader1( m_io );
+    EXPECT_FALSE( loader1.getValidation() );
+    loader1.setValidation( true );
+    EXPECT_TRUE( loader1.getValidation() );
+
+    BatchLoader loader2( m_io, true );
+    EXPECT_TRUE( loader2.getValidation() );
+}

test/unit/utIOSystem.cpp

@@ -39,37 +39,22 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 ---------------------------------------------------------------------------
 */
 #include "UnitTestPCH.h"
+#include "TestIOSystem.h"
 
 #include <assimp/IOSystem.hpp>
 
 using namespace std;
 using namespace Assimp;
 
-static const string Sep = "/";
-class TestIOSystem : public IOSystem {
-public:
-    TestIOSystem() : IOSystem() {}
-    virtual ~TestIOSystem() {}
-    virtual bool Exists( const char* ) const {
-        return true;
-    }
-    virtual char getOsSeparator() const {
-        return Sep[ 0 ];
-    }
-
-    virtual IOStream* Open(const char* pFile, const char* pMode = "rb") {
-        return NULL;
-    }
-
-    virtual void Close( IOStream* pFile) {
-        // empty
-    }
-};
-
 class IOSystemTest : public ::testing::Test {
 public:
-    virtual void SetUp() { pImp = new TestIOSystem(); }
+    virtual void SetUp() { 
-    virtual void TearDown() { delete pImp; }
+        pImp = new TestIOSystem(); 
+    }
+    
+    virtual void TearDown() { 
+        delete pImp; 
+    }
 
 protected:
     TestIOSystem* pImp;

test/unit/utImporter.cpp

@@ -270,3 +270,5 @@ TEST_F(ImporterTest, testMultipleReads)
     EXPECT_TRUE(pImp->ReadFile(ASSIMP_TEST_MODELS_DIR "/X/BCN_Epileptic.X",flags));
     //EXPECT_TRUE(pImp->ReadFile(ASSIMP_TEST_MODELS_DIR "/X/dwarf.x",flags)); # is in nonbsd
 }
+
+// ------------------------------------------------------------------------------------------------