Asset Catalogs: support reloading without losing local changes

Keep track of unsaved asset catalog changes, in a more granular way than
just one boolean per asset library. Individual catalogs can now be
marked with a flag `has_unsaved_changes`. This is taken into account
when reloading data from the catalog definition file (CDF):

- New catalog in CDF: gets loaded
- Already-known catalog in CDF:
    - local unsaved changes: on-disk catalog is ignored
    - otherwise: on-disk catalog replaces in-memory one
- Already-known catalog that does not exist in CDF:
    - local unsaved changes: catalog is kept around
    - otherwise: catalog is deleted.

Because this saving-is-also-loading behaviour, the "has unsaved changes"
flags are all stored in the undo buffer; undoing after saving will not
change the CDF, but at least it'll undo the loading from disk, and it'll
re-mark any changes as "not saved".

Reviewed By: Severin

Differential Revision: https://developer.blender.org/D12967

1 files changed, 107 insertions, 14 deletions

diff --git a/source/blender/blenkernel/intern/asset_catalog_test.cc b/source/blender/blenkernel/intern/asset_catalog_test.cc
index a6e372e4ae6..a2835d7b3b2 100644
--- a/source/blender/blenkernel/intern/asset_catalog_test.cc
+++ b/source/blender/blenkernel/intern/asset_catalog_test.cc
@@ -35,6 +35,7 @@ const bUUID UUID_ID_WITHOUT_PATH("e34dd2c5-5d2e-4668-9794-1db5de2a4f71");
 const bUUID UUID_POSES_ELLIE("df60e1f6-2259-475b-93d9-69a1b4a8db78");
 const bUUID UUID_POSES_ELLIE_WHITESPACE("b06132f6-5687-4751-a6dd-392740eb3c46");
 const bUUID UUID_POSES_ELLIE_TRAILING_SLASH("3376b94b-a28d-4d05-86c1-bf30b937130d");
+const bUUID UUID_POSES_ELLIE_BACKSLASHES("a51e17ae-34fc-47d5-ba0f-64c2c9b771f7");
 const bUUID UUID_POSES_RUZENA("79a4f887-ab60-4bd4-94da-d572e27d6aed");
 const bUUID UUID_POSES_RUZENA_HAND("81811c31-1a88-4bd7-bb34-c6fc2607a12e");
 const bUUID UUID_POSES_RUZENA_FACE("82162c1f-06cc-4d91-a9bf-4f72c104e348");
@@ -59,11 +60,21 @@ class TestableAssetCatalogService : public AssetCatalogService {
     return AssetCatalogService::get_catalog_definition_file();
   }
 
+  OwningAssetCatalogMap &get_deleted_catalogs()
+  {
+    return AssetCatalogService::get_deleted_catalogs();
+  }
+
   void create_missing_catalogs()
   {
     AssetCatalogService::create_missing_catalogs();
   }
 
+  void delete_catalog_by_id_soft(CatalogID catalog_id)
+  {
+    AssetCatalogService::delete_catalog_by_id_soft(catalog_id);
+  }
+
   int64_t count_catalogs_with_path(const CatalogFilePath &path)
   {
     int64_t count = 0;
@@ -305,8 +316,7 @@ TEST_F(AssetCatalogTest, load_catalog_path_backslashes)
   AssetCatalogService service(asset_library_root_);
   service.load_from_disk(asset_library_root_ + "/" + "blender_assets.cats.txt");
 
-  const bUUID ELLIE_BACKSLASHES_UUID("a51e17ae-34fc-47d5-ba0f-64c2c9b771f7");
-  const AssetCatalog *found_by_id = service.find_catalog(ELLIE_BACKSLASHES_UUID);
+  const AssetCatalog *found_by_id = service.find_catalog(UUID_POSES_ELLIE_BACKSLASHES);
   ASSERT_NE(nullptr, found_by_id);
   EXPECT_EQ(AssetCatalogPath("character/Ellie/backslashes"), found_by_id->path)
       << "Backslashes should be normalised when loading from disk.";
@@ -799,11 +809,11 @@ TEST_F(AssetCatalogTest, delete_catalog_leaf)
 
 TEST_F(AssetCatalogTest, delete_catalog_parent_by_id)
 {
-  AssetCatalogService service(asset_library_root_);
+  TestableAssetCatalogService service(asset_library_root_);
   service.load_from_disk(asset_library_root_ + "/" + "blender_assets.cats.txt");
 
   /* Delete a parent catalog. */
-  service.delete_catalog_by_id(UUID_POSES_RUZENA);
+  service.delete_catalog_by_id_soft(UUID_POSES_RUZENA);
 
   /* The catalog should have been deleted, but its children should still be there. */
   EXPECT_EQ(nullptr, service.find_catalog(UUID_POSES_RUZENA));
@@ -857,7 +867,7 @@ TEST_F(AssetCatalogTest, delete_catalog_write_to_disk)
   service.load_from_disk(asset_library_root_ + "/" +
                          AssetCatalogService::DEFAULT_CATALOG_FILENAME);
 
-  service.delete_catalog_by_id(UUID_POSES_ELLIE);
+  service.delete_catalog_by_id_soft(UUID_POSES_ELLIE);
 
   const CatalogFilePath save_to_path = use_temp_path();
   AssetCatalogDefinitionFile *cdf = service.get_catalog_definition_file();
@@ -938,7 +948,9 @@ TEST_F(AssetCatalogTest, merge_catalog_files)
    * CDF after we loaded it. */
   ASSERT_EQ(0, BLI_copy(modified_cdf_file.c_str(), temp_cdf_file.c_str()));
 
-  /* Overwrite the modified file. This should merge the on-disk file with our catalogs. */
+  /* Overwrite the modified file. This should merge the on-disk file with our catalogs.
+   * No catalog was marked as "has unsaved changes", so effectively this should not
+   * save anything, and reload what's on disk. */
   service.write_to_disk(cdf_dir + "phony.blend");
 
   AssetCatalogService loaded_service(cdf_dir);
@@ -946,16 +958,90 @@ TEST_F(AssetCatalogTest, merge_catalog_files)
 
   /* Test that the expected catalogs are there. */
   EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_ELLIE));
-  EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_ELLIE_WHITESPACE));
-  EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH));
-  EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA));
-  EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA_HAND));
   EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA_FACE));
   EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_AGENT_47)); /* New in the modified file. */
 
-  /* When there are overlaps, the in-memory (i.e. last-saved) paths should win. */
+  /* Test that catalogs removed from modified CDF are gone. */
+  EXPECT_EQ(nullptr, loaded_service.find_catalog(UUID_POSES_ELLIE_WHITESPACE));
+  EXPECT_EQ(nullptr, loaded_service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH));
+  EXPECT_EQ(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA));
+  EXPECT_EQ(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA_HAND));
+
+  /* On-disk changed catalogs should have overridden in-memory not-changed ones. */
   const AssetCatalog *ruzena_face = loaded_service.find_catalog(UUID_POSES_RUZENA_FACE);
-  EXPECT_EQ("character/Ružena/poselib/face", ruzena_face->path.str());
+  EXPECT_EQ("character/Ružena/poselib/gezicht", ruzena_face->path.str());
+}
+
+TEST_F(AssetCatalogTest, refresh_catalogs_with_modification)
+{
+  const CatalogFilePath cdf_dir = create_temp_path();
+  const CatalogFilePath original_cdf_file = asset_library_root_ + "/blender_assets.cats.txt";
+  const CatalogFilePath modified_cdf_file = asset_library_root_ + "/catalog_reload_test.cats.txt";
+  const CatalogFilePath temp_cdf_file = cdf_dir + "blender_assets.cats.txt";
+  ASSERT_EQ(0, BLI_copy(original_cdf_file.c_str(), temp_cdf_file.c_str()));
+
+  /* Load the unmodified, original CDF. */
+  TestableAssetCatalogService service(asset_library_root_);
+  service.load_from_disk(cdf_dir);
+
+  /* === Perfom changes that should be handled gracefully by the reloading code: */
+
+  /* 1. Delete a subtree of catalogs. */
+  service.prune_catalogs_by_id(UUID_POSES_RUZENA);
+  /* 2. Rename a catalog. */
+  service.tag_has_unsaved_changes(service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH));
+  service.update_catalog_path(UUID_POSES_ELLIE_TRAILING_SLASH, "character/Ellie/test-value");
+
+  /* Copy a modified file, to mimic a situation where someone changed the
+   * CDF after we loaded it. */
+  ASSERT_EQ(0, BLI_copy(modified_cdf_file.c_str(), temp_cdf_file.c_str()));
+
+  AssetCatalog *const ellie_whitespace_before_reload = service.find_catalog(
+      UUID_POSES_ELLIE_WHITESPACE);
+
+  /* This should merge the on-disk file with our catalogs. */
+  service.reload_catalogs();
+
+  /* === Test that the expected catalogs are there. */
+  EXPECT_NE(nullptr, service.find_catalog(UUID_POSES_ELLIE));
+  EXPECT_NE(nullptr, service.find_catalog(UUID_POSES_ELLIE_WHITESPACE));
+  EXPECT_NE(nullptr, service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH));
+
+  /* === Test changes made to the CDF: */
+
+  /* Removed from the file. */
+  EXPECT_EQ(nullptr, service.find_catalog(UUID_POSES_ELLIE_BACKSLASHES));
+  /* Added to the file. */
+  EXPECT_NE(nullptr, service.find_catalog(UUID_AGENT_47));
+  /* Path modified in file. */
+  AssetCatalog *ellie_whitespace_after_reload = service.find_catalog(UUID_POSES_ELLIE_WHITESPACE);
+  EXPECT_EQ(AssetCatalogPath("whitespace from file"), ellie_whitespace_after_reload->path);
+  EXPECT_NE(ellie_whitespace_after_reload, ellie_whitespace_before_reload);
+  /* Simple name modified in file. */
+  EXPECT_EQ(std::string("Hah simple name after all"),
+            service.find_catalog(UUID_WITHOUT_SIMPLENAME)->simple_name);
+
+  /* === Test persistence of in-memory changes: */
+
+  /* This part of the tree we deleted, but still existed in the CDF. They should remain deleted
+   * after reloading: */
+  EXPECT_EQ(nullptr, service.find_catalog(UUID_POSES_RUZENA));
+  EXPECT_EQ(nullptr, service.find_catalog(UUID_POSES_RUZENA_HAND));
+  EXPECT_EQ(nullptr, service.find_catalog(UUID_POSES_RUZENA_FACE));
+
+  /* This catalog had its path changed in the test and in the CDF. The change from the test (i.e.
+   * the in-memory, yet-unsaved change) should persist. */
+  EXPECT_EQ(AssetCatalogPath("character/Ellie/test-value"),
+            service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH)->path);
+
+  /* Overwrite the modified file. This should merge the on-disk file with our catalogs, and clear
+   * the "has_unsaved_changes" flags. */
+  service.write_to_disk(cdf_dir + "phony.blend");
+
+  EXPECT_FALSE(service.find_catalog(UUID_POSES_ELLIE_TRAILING_SLASH)->flags.has_unsaved_changes)
+      << "The catalogs whose path we changed should now be saved";
+  EXPECT_TRUE(service.get_deleted_catalogs().is_empty())
+      << "Deleted catalogs should not be remembered after saving.";
 }
 
 TEST_F(AssetCatalogTest, backups)
@@ -966,9 +1052,9 @@ TEST_F(AssetCatalogTest, backups)
   ASSERT_EQ(0, BLI_copy(original_cdf_file.c_str(), writable_cdf_file.c_str()));
 
   /* Read a CDF, modify, and write it. */
-  AssetCatalogService service(cdf_dir);
+  TestableAssetCatalogService service(cdf_dir);
   service.load_from_disk();
-  service.delete_catalog_by_id(UUID_POSES_ELLIE);
+  service.delete_catalog_by_id_soft(UUID_POSES_ELLIE);
   service.write_to_disk(cdf_dir + "phony.blend");
 
   const CatalogFilePath backup_path = writable_cdf_file + "~";
@@ -1100,6 +1186,13 @@ TEST_F(AssetCatalogTest, create_missing_catalogs_after_loading)
   ASSERT_NE(nullptr, cat_ellie) << "Missing parents should be created immediately after loading.";
   ASSERT_NE(nullptr, cat_ruzena) << "Missing parents should be created immediately after loading.";
 
+  EXPECT_TRUE(cat_char->flags.has_unsaved_changes)
+      << "Missing parents should be marked as having changes.";
+  EXPECT_TRUE(cat_ellie->flags.has_unsaved_changes)
+      << "Missing parents should be marked as having changes.";
+  EXPECT_TRUE(cat_ruzena->flags.has_unsaved_changes)
+      << "Missing parents should be marked as having changes.";
+
   AssetCatalogDefinitionFile *cdf = loaded_service.get_catalog_definition_file();
   ASSERT_NE(nullptr, cdf);
   EXPECT_TRUE(cdf->contains(cat_char->catalog_id)) << "Missing parents should be saved to a CDF.";