Asset Catalogs: be more accepting of backslashes as separators

Asset Catalog Paths should only contain forward slashes as separators, but
now the UI is more resilient to people using blackslashes instead.

Manifest Task: T90553

4 files changed, 59 insertions, 6 deletions

diff --git a/source/blender/blenkernel/intern/asset_catalog_path.cc b/source/blender/blenkernel/intern/asset_catalog_path.cc
index fec2b76e7a1..20cac76b40b 100644
--- a/source/blender/blenkernel/intern/asset_catalog_path.cc
+++ b/source/blender/blenkernel/intern/asset_catalog_path.cc
@@ -197,6 +197,8 @@ void AssetCatalogPath::iterate_components(ComponentIteratorFn callback) const
   for (const char *path_component = this->path_.data(); path_component && path_component[0];
        /* Jump to one after the next slash if there is any. */
        path_component = next_slash_ptr ? next_slash_ptr + 1 : nullptr) {
+    /* Note that this also treats backslashes as component separators, which
+     * helps in cleaning up backslash-separated paths. */
     next_slash_ptr = BLI_path_slash_find(path_component);
 
     const bool is_last_component = next_slash_ptr == nullptr;
diff --git a/source/blender/blenkernel/intern/asset_catalog_path_test.cc b/source/blender/blenkernel/intern/asset_catalog_path_test.cc
index 997f57a161c..f248863ce77 100644
--- a/source/blender/blenkernel/intern/asset_catalog_path_test.cc
+++ b/source/blender/blenkernel/intern/asset_catalog_path_test.cc
@@ -208,6 +208,18 @@ TEST(AssetCatalogPathTest, cleanup)
     AssetCatalogPath with_colons("some/key:subkey=value/path");
     EXPECT_EQ(AssetCatalogPath("some/key-subkey=value/path"), with_colons.cleanup());
   }
+  {
+    const AssetCatalogPath with_backslashes("windows\\for\\life");
+    EXPECT_EQ(AssetCatalogPath("windows/for/life"), with_backslashes.cleanup());
+  }
+  {
+    const AssetCatalogPath with_mixed("windows\\for/life");
+    EXPECT_EQ(AssetCatalogPath("windows/for/life"), with_mixed.cleanup());
+  }
+  {
+    const AssetCatalogPath with_punctuation("is!/this?/¿valid?");
+    EXPECT_EQ(AssetCatalogPath("is!/this?/¿valid?"), with_punctuation.cleanup());
+  }
 }
 
 TEST(AssetCatalogPathTest, iterate_components)
diff --git a/source/blender/blenkernel/intern/asset_catalog_test.cc b/source/blender/blenkernel/intern/asset_catalog_test.cc
index abe6d384247..2cef34966f8 100644
--- a/source/blender/blenkernel/intern/asset_catalog_test.cc
+++ b/source/blender/blenkernel/intern/asset_catalog_test.cc
@@ -902,8 +902,6 @@ TEST_F(AssetCatalogTest, update_catalog_path)
   const AssetCatalog *renamed_cat = service.find_catalog(UUID_POSES_RUZENA);
   ASSERT_NE(nullptr, renamed_cat);
   ASSERT_EQ(orig_cat, renamed_cat) << "Changing the path should not reallocate the catalog.";
-  EXPECT_EQ(orig_cat->simple_name, renamed_cat->simple_name)
-      << "Changing the path should not change the simple name.";
   EXPECT_EQ(orig_cat->catalog_id, renamed_cat->catalog_id)
       << "Changing the path should not change the catalog ID.";
 
@@ -932,6 +930,47 @@ TEST_F(AssetCatalogTest, update_catalog_path_simple_name)
       << "Changing the path should update the simplename of children.";
 }
 
+TEST_F(AssetCatalogTest, update_catalog_path_add_slashes)
+{
+  AssetCatalogService service(asset_library_root_);
+  service.load_from_disk(asset_library_root_ + "/" +
+                         AssetCatalogService::DEFAULT_CATALOG_FILENAME);
+
+  const AssetCatalog *orig_cat = service.find_catalog(UUID_POSES_RUZENA);
+  const AssetCatalogPath orig_path = orig_cat->path;
+
+  /* Original path is `character/Ružena/poselib`.
+   * This rename will also create a new catalog for `character/Ružena/poses`. */
+  service.update_catalog_path(UUID_POSES_RUZENA, "character/Ružena/poses/general");
+
+  EXPECT_EQ(nullptr, service.find_catalog_by_path(orig_path))
+      << "The original (pre-rename) path should not be associated with a catalog any more.";
+
+  const AssetCatalog *renamed_cat = service.find_catalog(UUID_POSES_RUZENA);
+  ASSERT_NE(nullptr, renamed_cat);
+  EXPECT_EQ(orig_cat->catalog_id, renamed_cat->catalog_id)
+      << "Changing the path should not change the catalog ID.";
+
+  EXPECT_EQ("character/Ružena/poses/general", renamed_cat->path.str())
+      << "When creating a new catalog by renaming + adding a slash, the renamed catalog should be "
+         "assigned the path passed to update_catalog_path()";
+
+  /* Test the newly created catalog. */
+  const AssetCatalog *new_cat = service.find_catalog_by_path("character/Ružena/poses");
+  ASSERT_NE(nullptr, new_cat) << "Renaming to .../X/Y should cause .../X to exist as well.";
+  EXPECT_EQ("character/Ružena/poses", new_cat->path.str());
+  EXPECT_EQ("character-Ružena-poses", new_cat->simple_name);
+  EXPECT_TRUE(new_cat->flags.has_unsaved_changes);
+
+  /* Test the children. */
+  EXPECT_EQ("character/Ružena/poses/general/hand",
+            service.find_catalog(UUID_POSES_RUZENA_HAND)->path.str())
+      << "Changing the path should update children.";
+  EXPECT_EQ("character/Ružena/poses/general/face",
+            service.find_catalog(UUID_POSES_RUZENA_FACE)->path.str())
+      << "Changing the path should update children.";
+}
+
 TEST_F(AssetCatalogTest, merge_catalog_files)
 {
   const CatalogFilePath cdf_dir = create_temp_path();
diff --git a/source/blender/editors/asset/intern/asset_catalog.cc b/source/blender/editors/asset/intern/asset_catalog.cc
index dae960cbb0a..f3ba12a6324 100644
--- a/source/blender/editors/asset/intern/asset_catalog.cc
+++ b/source/blender/editors/asset/intern/asset_catalog.cc
@@ -107,17 +107,17 @@ void ED_asset_catalog_rename(::AssetLibrary *library,
 
   AssetCatalog *catalog = catalog_service->find_catalog(catalog_id);
 
-  AssetCatalogPath new_path = catalog->path.parent();
-  new_path = new_path / StringRef(new_name);
+  const AssetCatalogPath new_path = catalog->path.parent() / StringRef(new_name);
+  const AssetCatalogPath clean_new_path = new_path.cleanup();
 
-  if (new_path == catalog->path) {
+  if (new_path == catalog->path || clean_new_path == catalog->path) {
     /* Nothing changed, so don't bother renaming for nothing. */
     return;
   }
 
   catalog_service->undo_push();
   catalog_service->tag_has_unsaved_changes(catalog);
-  catalog_service->update_catalog_path(catalog_id, new_path);
+  catalog_service->update_catalog_path(catalog_id, clean_new_path);
   WM_main_add_notifier(NC_SPACE | ND_SPACE_ASSET_PARAMS, nullptr);
 }