Fix Codacity warnings, test Exporter metadata

Pass std::string around instead as need to create one anyway.

Use CamelCase version to avoid caseSensiTivity issues
as will usually want the camelcase edition anyway.
Not UTF-8 but the standard XML tags are ASCII anyway

code/Collada/ColladaHelper.cpp

@@ -44,6 +44,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "ColladaHelper.h"
 
 #include <assimp/commonMetaData.h>
+#include <assimp/ParsingUtils.h>
 
 namespace Assimp {
 namespace Collada {
@@ -60,5 +61,45 @@ const MetaKeyPairVector &GetColladaAssimpMetaKeys() {
     return result;
 }
 
+const MetaKeyPairVector MakeColladaAssimpMetaKeysCamelCase() {
+    MetaKeyPairVector result = MakeColladaAssimpMetaKeys();
+    for (auto &val : result)
+    {
+        ToCamelCase(val.first);
+    }
+    return result;
+};
+
+const MetaKeyPairVector &GetColladaAssimpMetaKeysCamelCase()
+{
+    static const MetaKeyPairVector result = MakeColladaAssimpMetaKeysCamelCase();
+    return result;
+}
+
+// ------------------------------------------------------------------------------------------------
+// Convert underscore_separated to CamelCase: "authoring_tool" becomes "AuthoringTool"
+void ToCamelCase(std::string &text)
+{
+    if (text.empty())
+        return;
+    // Capitalise first character
+    text[0] = Assimp::ToUpper(text[0]);
+    for (auto it = text.begin(); it != text.end(); /*iterated below*/)
+    {
+        if ((*it) == '_')
+        {
+            it = text.erase(it);
+            if (it != text.end())
+                (*it) = ToUpper(*it);
+        }
+        else
+        {
+            // Make lower case
+            (*it) = ToLower(*it);
+            ++it;
+        }
+    }
+}
+
 } // namespace Collada
 } // namespace Assimp

code/Collada/ColladaHelper.h

@@ -105,10 +105,16 @@ enum MorphMethod
 };
 
 /** Common metadata keys as <Collada, Assimp> */
-typedef std::pair<const char*, const char*> MetaKeyPair;
+typedef std::pair<std::string, std::string> MetaKeyPair;
 typedef std::vector<MetaKeyPair> MetaKeyPairVector;
 
+// Collada as lower_case (native)
 const MetaKeyPairVector &GetColladaAssimpMetaKeys();
+// Collada as CamelCase (used by Assimp for consistency)
+const MetaKeyPairVector &GetColladaAssimpMetaKeysCamelCase();
+
+/** Convert underscore_separated to CamelCase "authoring_tool" becomes "AuthoringTool" */
+void ToCamelCase(std::string &text);
 
 /** Contains all data for one of the different transformation types */
 struct Transform

code/Collada/ColladaParser.cpp

@@ -411,7 +411,7 @@ void ColladaParser::ReadAssetInfo()
             }
             else
             {
-                ReadMetaDataItem(mAssetMetaData, GetColladaAssimpMetaKeys());
+                ReadMetaDataItem(mAssetMetaData);
             }
         }
         else if (mReader->getNodeType() == irr::io::EXN_ELEMENT_END)
@@ -435,7 +435,7 @@ void ColladaParser::ReadContributorInfo()
     {
         if (mReader->getNodeType() == irr::io::EXN_ELEMENT)
         {
-            ReadMetaDataItem(mAssetMetaData, GetColladaAssimpMetaKeys());
+            ReadMetaDataItem(mAssetMetaData);
         }
         else if (mReader->getNodeType() == irr::io::EXN_ELEMENT_END)
         {
@@ -446,25 +446,21 @@ void ColladaParser::ReadContributorInfo()
     }
 }
 
-static bool FindCommonKey(const char *collada_key, const MetaKeyPairVector &key_renaming, size_t &found_index) {
+static bool FindCommonKey(const std::string &collada_key, const MetaKeyPairVector &key_renaming, size_t &found_index) {
-    found_index = 9999999u;
-    if ( nullptr == collada_key ) {
-        return false;
-    }
-    
     for (size_t i = 0; i < key_renaming.size(); ++i) {
-		if (strncmp(key_renaming[i].first, collada_key, strlen(collada_key)) == 0) {
+		if (key_renaming[i].first == collada_key) {
             found_index = i;
             return true;
 		}
 	}
-    
+    found_index = std::numeric_limits<size_t>::max();
     return false;
 }
 
 // ------------------------------------------------------------------------------------------------
 // Reads a single string metadata item
-void ColladaParser::ReadMetaDataItem(StringMetaData &metadata, const MetaKeyPairVector &key_renaming) {
+void ColladaParser::ReadMetaDataItem(StringMetaData &metadata) {
+    const Collada::MetaKeyPairVector &key_renaming = GetColladaAssimpMetaKeysCamelCase();
 	// Metadata such as created, keywords, subject etc
 	const char *key_char = mReader->getNodeName();
 	if (key_char != nullptr) {
@@ -474,12 +470,13 @@ void ColladaParser::ReadMetaDataItem(StringMetaData &metadata, const MetaKeyPair
             aiString aistr;
 			aistr.Set(value_char);
 
-			size_t found_index;
-			if (FindCommonKey(key_str.c_str(), key_renaming, found_index)) {
-                metadata.emplace(key_renaming[found_index].second, aistr);
-            } else {
             std::string camel_key_str(key_str);
 			ToCamelCase(camel_key_str);
+
+			size_t found_index;
+			if (FindCommonKey(camel_key_str, key_renaming, found_index)) {
+                metadata.emplace(key_renaming[found_index].second, aistr);
+            } else {
 				metadata.emplace(camel_key_str, aistr);
 			}
         }
@@ -489,27 +486,6 @@ void ColladaParser::ReadMetaDataItem(StringMetaData &metadata, const MetaKeyPair
         SkipElement();
 }
 
-// ------------------------------------------------------------------------------------------------
-// Convert underscore_seperated to CamelCase: "authoring_tool" becomes "AuthoringTool"
-void ColladaParser::ToCamelCase(std::string &text)
-{
-    if (text.empty())
-        return;
-    // Capitalise first character
-    text[0] = ToUpper(text[0]);
-    for (auto it = text.begin(); it != text.end(); /*iterated below*/)
-    {
-        if ((*it) == '_')
-        {
-            it = text.erase(it);
-            if (it != text.end())
-                (*it) = ToUpper(*it);
-        }
-        else
-            ++it;
-    }
-}
-
 // ------------------------------------------------------------------------------------------------
 // Reads the animation clips
 void ColladaParser::ReadAnimationClipLibrary()

code/Collada/ColladaParser.h

@@ -95,10 +95,7 @@ namespace Assimp
         void ReadContributorInfo();
 
         /** Reads generic metadata into provided map and renames keys for Assimp */
-        void ReadMetaDataItem(StringMetaData &metadata, const Collada::MetaKeyPairVector &key_renaming);
+        void ReadMetaDataItem(StringMetaData &metadata);
-
-        /** Convert underscore_seperated to CamelCase "authoring_tool" becomes "AuthoringTool" */
-        static void ToCamelCase(std::string &text);
 
         /** Reads the animation library */
         void ReadAnimationLibrary();

test/models/Collada/lights.dae

@@ -4,6 +4,7 @@
     <contributor>
       <author>Blender User</author>
       <authoring_tool>Blender 2.74.0 commit date:2015-03-31, commit time:13:39, hash:000dfc0</authoring_tool>
+      <copyright>BSD</copyright>
     </contributor>
     <created>2015-05-17T21:55:44</created>
     <modified>2015-05-17T21:55:44</modified>

test/unit/utColladaExportLight.cpp

@@ -43,6 +43,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "UnitTestPCH.h"
 
 #include <assimp/cexport.h>
+#include <assimp/commonMetaData.h>
 #include <assimp/Exporter.hpp>
 #include <assimp/Importer.hpp>
 #include <assimp/scene.h>
@@ -75,7 +76,7 @@ TEST_F(ColladaExportLight, testExportLight)
     const char* file = "lightsExp.dae";
 
     const aiScene* pTest = im->ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/lights.dae", aiProcess_ValidateDataStructure);
-    ASSERT_TRUE(pTest!=NULL);
+    ASSERT_NE(pTest, nullptr);
     ASSERT_TRUE(pTest->HasLights());
 
     const unsigned int origNumLights( pTest->mNumLights );
@@ -86,15 +87,63 @@ TEST_F(ColladaExportLight, testExportLight)
         origLights[ i ] = *(pTest->mLights[ i ]);
     }
 
-    EXPECT_EQ(AI_SUCCESS,ex->Export(pTest,"collada",file));
+    // Common metadata
+    // Confirm was loaded by the Collada importer
+    aiString origImporter;
+    EXPECT_TRUE(pTest->mMetaData->Get(AI_METADATA_SOURCE_FORMAT, origImporter)) << "No importer format metadata";
+	EXPECT_STREQ("Collada Importer", origImporter.C_Str());
+
+    aiString origGenerator;
+	EXPECT_TRUE(pTest->mMetaData->Get(AI_METADATA_SOURCE_GENERATOR, origGenerator)) << "No generator metadata";
+	EXPECT_EQ(strncmp(origGenerator.C_Str(), "Blender", 7), 0) << "AI_METADATA_SOURCE_GENERATOR was: " << origGenerator.C_Str();
+
+    aiString origCopyright;
+	EXPECT_TRUE(pTest->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, origCopyright)) << "No copyright metadata";
+    EXPECT_STREQ("BSD", origCopyright.C_Str());
+
+    aiString origCreated;
+	EXPECT_TRUE(pTest->mMetaData->Get("Created", origCreated)) << "No created metadata";
+    EXPECT_STREQ("2015-05-17T21:55:44", origCreated.C_Str());
+
+    aiString origModified;
+	EXPECT_TRUE(pTest->mMetaData->Get("Modified", origModified)) << "No modified metadata";
+    EXPECT_STREQ("2015-05-17T21:55:44", origModified.C_Str());
+
+    EXPECT_EQ(AI_SUCCESS, ex->Export(pTest, "collada", file));
+
+    // Drop the pointer as about to become invalid
+    pTest = nullptr;
 
     const aiScene* imported = im->ReadFile(file, aiProcess_ValidateDataStructure);
 
-    ASSERT_TRUE(imported!=NULL);
+    ASSERT_TRUE(imported != NULL);
 
+    // Check common metadata survived roundtrip
+    aiString readImporter;
+    EXPECT_TRUE(imported->mMetaData->Get(AI_METADATA_SOURCE_FORMAT, readImporter)) << "No importer format metadata after export";
+    EXPECT_STREQ(origImporter.C_Str(), readImporter.C_Str()) << "Assimp Importer Format changed";
+
+    aiString readGenerator;
+	EXPECT_TRUE(imported->mMetaData->Get(AI_METADATA_SOURCE_GENERATOR, readGenerator)) << "No generator metadata";
+	EXPECT_STREQ(origGenerator.C_Str(), readGenerator.C_Str()) << "Generator changed";
+
+    aiString readCopyright;
+	EXPECT_TRUE(imported->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, readCopyright)) << "No copyright metadata";
+    EXPECT_STREQ(origCopyright.C_Str(), readCopyright.C_Str()) << "Copyright changed";
+
+    aiString readCreated;
+	EXPECT_TRUE(imported->mMetaData->Get("Created", readCreated)) << "No created metadata";
+    EXPECT_STREQ(origCreated.C_Str(), readCreated.C_Str()) << "Created date changed";
+
+    aiString readModified;
+	EXPECT_TRUE(imported->mMetaData->Get("Modified", readModified)) << "No modified metadata";
+    EXPECT_STRNE(origModified.C_Str(), readModified.C_Str()) << "Modified date did not change";
+    EXPECT_GT(readModified.length, 18) << "Modified date too short";
+
+    // Lights
     EXPECT_TRUE(imported->HasLights());
-    EXPECT_EQ( origNumLights,imported->mNumLights );
+    EXPECT_EQ(origNumLights, imported->mNumLights);
-    for(size_t i=0; i< origNumLights; i++) {
+    for(size_t i=0; i < origNumLights; i++) {
         const aiLight *orig = &origLights[ i ];
         const aiLight *read = imported->mLights[i];
         EXPECT_EQ( 0,strncmp(origNames[ i ].c_str(),read->mName.C_Str(), origNames[ i ].size() ) );

test/unit/utColladaImportExport.cpp

@@ -77,7 +77,7 @@ public:
 		EXPECT_TRUE(scene->mMetaData->Get(AI_METADATA_SOURCE_GENERATOR, value)) << "No generator metadata";
 		EXPECT_EQ(strncmp(value.C_Str(), "Maya 8.0", 8), 0) << "AI_METADATA_SOURCE_GENERATOR was: " << value.C_Str();
 
-		EXPECT_TRUE(scene->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, value));
+		EXPECT_TRUE(scene->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, value)) << "No copyright metadata";
 		EXPECT_EQ(strncmp(value.C_Str(), "Copyright 2006", 14), 0) << "AI_METADATA_SOURCE_COPYRIGHT was: " << value.C_Str();
 
 		return true;