From e1e380ba388fbe514b38c3023ca9eb02cc2c78ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Fri, 24 Sep 2021 14:42:44 +0200
Subject: Asset Catalogs: write catalogs sorted by path & UUID

When writing asset catalog definition files, order the catalogs by
(path, UUID). This ensures that every write produces the same file, playing
nice with versioning / synchronisation systems.
---
 source/blender/blenkernel/BKE_asset_catalog.hh     | 13 ++++++++
 source/blender/blenkernel/intern/asset_catalog.cc  |  9 ++++-
 .../blenkernel/intern/asset_catalog_test.cc        | 38 ++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/source/blender/blenkernel/BKE_asset_catalog.hh b/source/blender/blenkernel/BKE_asset_catalog.hh
index 5fd68e33f4a..51cfa8452c0 100644
--- a/source/blender/blenkernel/BKE_asset_catalog.hh
+++ b/source/blender/blenkernel/BKE_asset_catalog.hh
@@ -32,6 +32,7 @@
 
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 
 namespace blender::bke {
@@ -255,4 +256,16 @@ class AssetCatalog {
   static std::string sensible_simple_name_for_path(const CatalogPath &path);
 };
 
+struct AssetCatalogPathCmp {
+  bool operator()(const AssetCatalog *lhs, const AssetCatalog *rhs) const
+  {
+    if (lhs->path == rhs->path) {
+      return lhs->catalog_id < rhs->catalog_id;
+    }
+    return lhs->path < rhs->path;
+  }
+};
+
+using AssetCatalogOrderedSet = std::set<const AssetCatalog *, AssetCatalogPathCmp>;
+
 }  // namespace blender::bke
diff --git a/source/blender/blenkernel/intern/asset_catalog.cc b/source/blender/blenkernel/intern/asset_catalog.cc
index 20f2dc0a571..4f1de09e148 100644
--- a/source/blender/blenkernel/intern/asset_catalog.cc
+++ b/source/blender/blenkernel/intern/asset_catalog.cc
@@ -30,6 +30,7 @@
 #endif
 
 #include <fstream>
+#include <set>
 
 namespace blender::bke {
 
@@ -525,10 +526,16 @@ bool AssetCatalogDefinitionFile::write_to_disk_unsafe(const CatalogFilePath &des
 
   // Write the catalogs.
   // TODO(@sybren): order them by Catalog ID or Catalog Path.
-  for (const auto &catalog : catalogs_.values()) {
+
+  AssetCatalogOrderedSet catalogs_by_path;
+  for (const AssetCatalog *catalog : catalogs_.values()) {
     if (catalog->flags.is_deleted) {
       continue;
     }
+    catalogs_by_path.insert(catalog);
+  }
+
+  for (const AssetCatalog *catalog : catalogs_by_path) {
     output << catalog->catalog_id << ":" << catalog->path << ":" << catalog->simple_name
            << std::endl;
   }
diff --git a/source/blender/blenkernel/intern/asset_catalog_test.cc b/source/blender/blenkernel/intern/asset_catalog_test.cc
index 074ffdbcaac..b40aae5d64a 100644
--- a/source/blender/blenkernel/intern/asset_catalog_test.cc
+++ b/source/blender/blenkernel/intern/asset_catalog_test.cc
@@ -442,4 +442,42 @@ TEST_F(AssetCatalogTest, backups)
   EXPECT_NE(nullptr, loaded_service.find_catalog(UUID_POSES_RUZENA_FACE));
 }
 
+TEST_F(AssetCatalogTest, order_by_path)
+{
+  const bUUID cat2_uuid("22222222-b847-44d9-bdca-ff04db1c24f5");
+  const bUUID cat4_uuid("11111111-b847-44d9-bdca-ff04db1c24f5");  // Sorts earlier than above.
+  const AssetCatalog cat1(BLI_uuid_generate_random(), "simple/path/child", "");
+  const AssetCatalog cat2(cat2_uuid, "simple/path", "");
+  const AssetCatalog cat3(BLI_uuid_generate_random(), "complex/path/...or/is/it?", "");
+  const AssetCatalog cat4(cat4_uuid, "simple/path", "different ID, same path");  // should be kept
+  const AssetCatalog cat5(cat4_uuid, "simple/path", "same ID, same path");       // disappears
+
+  AssetCatalogOrderedSet by_path;
+  by_path.insert(&cat1);
+  by_path.insert(&cat2);
+  by_path.insert(&cat3);
+  by_path.insert(&cat4);
+  by_path.insert(&cat5);
+
+  AssetCatalogOrderedSet::const_iterator set_iter = by_path.begin();
+
+  EXPECT_EQ(1, by_path.count(&cat1));
+  EXPECT_EQ(1, by_path.count(&cat2));
+  EXPECT_EQ(1, by_path.count(&cat3));
+  EXPECT_EQ(1, by_path.count(&cat4));
+  ASSERT_EQ(4, by_path.size()) << "Expecting cat5 to not be stored in the set, as it duplicates "
+                                  "an already-existing path + UUID";
+
+  EXPECT_EQ(cat3.catalog_id, (*(set_iter++))->catalog_id);  // complex/path
+  EXPECT_EQ(cat4.catalog_id, (*(set_iter++))->catalog_id);  // simple/path with 111.. ID
+  EXPECT_EQ(cat2.catalog_id, (*(set_iter++))->catalog_id);  // simple/path with 222.. ID
+  EXPECT_EQ(cat1.catalog_id, (*(set_iter++))->catalog_id);  // simple/path/child
+
+  if (set_iter != by_path.end()) {
+    const AssetCatalog *next_cat = *set_iter;
+    FAIL() << "Did not expect more items in the set, had at least " << next_cat->catalog_id << ":"
+           << next_cat->path;
+  }
+}
+
 }  // namespace blender::bke::tests
-- 
cgit v1.2.3