Merge pull request #5156 from feuerste/cloud_storage_version

Handle gcs cloud storage file extensions with versioning.

code/Common/BaseImporter.cpp

@@ -59,6 +59,31 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <memory>
 #include <sstream>
 
+namespace {
+// Checks whether the passed string is a gcs version.
+bool IsGcsVersion(const std::string &s) {
+    if (s.empty()) return false;
+    return std::all_of(s.cbegin(), s.cend(), [](const char c) {
+        // gcs only permits numeric characters.
+        return std::isdigit(static_cast<int>(c));
+    });
+}
+
+// Removes a possible version hash from a filename, as found for example in
+// gcs uris (e.g. `gs://bucket/model.glb#1234`), see also
+// https://github.com/GoogleCloudPlatform/gsutil/blob/c80f329bc3c4011236c78ce8910988773b2606cb/gslib/storage_url.py#L39.
+std::string StripVersionHash(const std::string &filename) {
+    const std::string::size_type pos = filename.find_last_of('#');
+    // Only strip if the hash is behind a possible file extension and the part
+    // behind the hash is a version string.
+    if (pos != std::string::npos && pos > filename.find_last_of('.') &&
+        IsGcsVersion(filename.substr(pos + 1))) {
+        return filename.substr(0, pos);
+    }
+    return filename;
+}
+}  // namespace
+
 using namespace Assimp;
 
 // ------------------------------------------------------------------------------------------------
@@ -241,6 +266,7 @@ void BaseImporter::GetExtensionList(std::set<std::string> &extensions) {
 // ------------------------------------------------------------------------------------------------
 // Check for file extension
 /*static*/ bool BaseImporter::HasExtension(const std::string &pFile, const std::set<std::string> &extensions) {
+    const std::string file = StripVersionHash(pFile);
     // CAUTION: Do not just search for the extension!
     // GetExtension() returns the part after the *last* dot, but some extensions
     // have dots inside them, e.g. ogre.mesh.xml. Compare the entire end of the
@@ -248,9 +274,9 @@ void BaseImporter::GetExtensionList(std::set<std::string> &extensions) {
     for (const std::string& ext : extensions) {
         // Yay for C++<20 not having std::string::ends_with()
         const std::string dotExt = "." + ext;
-        if (dotExt.length() > pFile.length()) continue;
+        if (dotExt.length() > file.length()) continue;
         // Possible optimization: Fetch the lowercase filename!
-        if (0 == ASSIMP_stricmp(pFile.c_str() + pFile.length() - dotExt.length(), dotExt.c_str())) {
+        if (0 == ASSIMP_stricmp(file.c_str() + file.length() - dotExt.length(), dotExt.c_str())) {
             return true;
         }
     }
@@ -259,7 +285,8 @@ void BaseImporter::GetExtensionList(std::set<std::string> &extensions) {
 
 // ------------------------------------------------------------------------------------------------
 // Get file extension from path
-std::string BaseImporter::GetExtension(const std::string &file) {
+std::string BaseImporter::GetExtension(const std::string &pFile) {
+    const std::string file = StripVersionHash(pFile);
     std::string::size_type pos = file.find_last_of('.');
 
     // no file extension at all

test/unit/utImporter.cpp

@@ -361,3 +361,37 @@ TEST_F(ImporterTest, unexpectedException) {
         EXPECT_TRUE(false);
     }
 }
+
+// ------------------------------------------------------------------------------------------------
+
+struct ExtensionTestCase {
+    std::string testName;
+    std::string filename;
+    std::string getExtensionResult;
+    std::string hasExtension;
+    bool hasExtensionResult;
+};
+
+using ExtensionTest = ::testing::TestWithParam<ExtensionTestCase>;
+
+TEST_P(ExtensionTest, testGetAndHasExtension) {
+    const ExtensionTestCase& testCase = GetParam();
+    EXPECT_EQ(testCase.getExtensionResult, BaseImporter::GetExtension(testCase.filename));
+    EXPECT_EQ(testCase.hasExtensionResult, BaseImporter::HasExtension(testCase.filename, {testCase.hasExtension}));
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    ExtensionTests, ExtensionTest,
+    ::testing::ValuesIn<ExtensionTestCase>({
+        {"NoExtension", "name", "", "glb", false},
+        {"NoExtensionAndEmptyVersion", "name#", "", "glb", false},
+        {"WithExtensionAndEmptyVersion", "name.glb#", "glb#", "glb", false},
+        {"WithExtensionAndVersion", "name.glb#1234", "glb", "glb", true},
+        {"WithExtensionAndHashInStem", "name#1234.glb", "glb", "glb", true},
+        {"WithExtensionAndInvalidVersion", "name.glb#_", "glb#_", "glb", false},
+        {"WithExtensionAndDotAndHashInStem", "name.glb#.abc", "abc", "glb", false},
+        {"WithTwoExtensions", "name.abc.def", "def", "abc.def", true},
+    }),
+    [](const ::testing::TestParamInfo<ExtensionTest::ParamType>& info) {
+        return info.param.testName;
+    });