Bugfix/preferred identifiers broke dependency indices

include/IdentifiableSchema.h

@@ -33,7 +33,9 @@ class IdentifiableSchema : public LinkedSchema {
       // Note: We add reserved identifiers to the identifiers to avoid conflicts
       // with the predefined schemas.
       std::set<std::string> identifiers{"True", "False"};
-      std::vector<IdentifiableSchema> identifiableSchemas;
+      std::vector<std::unique_ptr<IdentifiableSchema>> identifiableSchemas(
+          linkedSchemas.size());
+
       for (const auto& [uri, identifier] : preferredIdentifiers) {
         for (size_t i = 0; i < linkedSchemas.size(); i++) {
           const auto& linkedSchema = linkedSchemas[i];
@@ -43,13 +45,16 @@ class IdentifiableSchema : public LinkedSchema {
                 linkedSchema->json_, linkedSchema->baseUri_,
                 linkedSchema->pointer_, linkedSchema->draft_, identifier);
             schema.dependencies_ = linkedSchema->dependencies_;
-            identifiableSchemas.emplace_back(std::move(schema));
-            linkedSchemas.erase(linkedSchemas.begin() + i);
+            identifiableSchemas[i] =
+                std::make_unique<IdentifiableSchema>(std::move(schema));
             break;
           }
         }
       }
       for (size_t i = 0; i < linkedSchemas.size(); i++) {
+        if (identifiableSchemas[i] != nullptr) {
+          continue;
+        }
         const auto& linkedSchema = linkedSchemas[i];
         const std::string preferred_identifier =
             linkedSchema->getPreferredIdentifier();
@@ -69,9 +74,14 @@ class IdentifiableSchema : public LinkedSchema {
             linkedSchema->json_, linkedSchema->baseUri_, linkedSchema->pointer_,
             linkedSchema->draft_, identifier);
         schema.dependencies_ = linkedSchema->dependencies_;
-        identifiableSchemas.emplace_back(std::move(schema));
+        identifiableSchemas[i] =
+            std::make_unique<IdentifiableSchema>(std::move(schema));
+      }
+      std::vector<IdentifiableSchema> identifiableSchemaVec;
+      for (auto& schema : identifiableSchemas) {
+        identifiableSchemaVec.push_back(std::move(*schema));
       }
-      return identifiableSchemas;
+      return identifiableSchemaVec;
     } catch (const std::exception& e) {
       std::cerr << "Error caught in transition from Linked to Identifiable:\n";
       throw e;
@@ -90,11 +100,11 @@ class IdentifiableSchema : public LinkedSchema {
       auto& ptrDump = uriDump[iSchema.pointer_.toFragment()];
       for (const auto& [uri, idx] : iSchema.dependencies_) {
         const auto& idxSchema = identifiableSchemas[idx];
-        ptrDump.push_back(nlohmann::json::array(
-            {uri.toString().value(),
-             idxSchema.baseUri_.withPointer(idxSchema.pointer_)
-                 .toString()
-                 .value()}));
+        ptrDump.push_back({{"uri", uri.toString().value()},
+                           {"dependent location",
+                            idxSchema.baseUri_.withPointer(idxSchema.pointer_)
+                                .toString()
+                                .value()}});
       }
     }
 

include/SchemaResource.h

@@ -121,14 +121,18 @@ class SchemaResource {
   static void dumpSchemas(SetMap<UriWrapper, SchemaResource>& schemaResources,
                           std::filesystem::path outputDirectory = ".") {
     nlohmann::json schemaResourceDump = nlohmann::json::object();
+    // iterate over all the schemas.
     for (const auto& [uris, schema] : schemaResources) {
       auto& baseUriList =
           schemaResourceDump[schema.get().baseUri_.toString().value()];
-      baseUriList[schema.get().pointer_.toFragment()] = nlohmann::json::array();
+      if (!baseUriList[schema.get().pointer_.toFragment()].is_array()) {
+        baseUriList[schema.get().pointer_.toFragment()] =
+            nlohmann::json::array();
+      }
       for (const auto& uri : uris.get()) {
         baseUriList[schema.get().pointer_.toFragment()].push_back(
-            nlohmann::json::array({uri.toFragmentlessString().value_or(""),
-                                   uri.getFragment().value_or("")}));
+            {{"uri", uri.toFragmentlessString().value_or("")},
+             {"frag", uri.getFragment().value_or("")}});
       }
     }
     std::ofstream resourceDump(outputDirectory / "resource.dump.json");

tests/test_IdentifiableSchema.cpp

@@ -73,15 +73,78 @@ TEST_CASE("IdentifiableSchema transition with preferred identifiers",
       std::move(linkedSchemas), preferredIdentifiers);
 
   REQUIRE(identifiableSchemas.size() == 2);
-  REQUIRE(identifiableSchemas[0].dependencies_.contains(
-      baseUri.withPointer(pointer / "properties" / "a")));
-  REQUIRE(identifiableSchemas[0].dependencies_.at(
-              baseUri.withPointer(pointer / "properties" / "a")) == 0);
+  REQUIRE((identifiableSchemas[0].dependencies_.contains(
+               baseUri.withPointer(pointer / "properties" / "a")) ||
+           identifiableSchemas[1].dependencies_.contains(
+               baseUri.withPointer(pointer / "properties" / "a"))));
+
+  if (identifiableSchemas[0].dependencies_.contains(
+          baseUri.withPointer(pointer / "properties" / "a"))) {
+    REQUIRE(identifiableSchemas[0].dependencies_.at(
+                baseUri.withPointer(pointer / "properties" / "a")) == 0);
+  } else if (identifiableSchemas[1].dependencies_.contains(
+                 baseUri.withPointer(pointer / "properties" / "a"))) {
+    REQUIRE(identifiableSchemas[1].dependencies_.at(
+                baseUri.withPointer(pointer / "properties" / "a")) == 0);
+  } else {
+    FAIL("Dependency not found");
+  }
   // Identifiers are unique
 
   REQUIRE(identifiableSchemas[1].identifier_ !=
           identifiableSchemas[0].identifier_);
 
-  REQUIRE(identifiableSchemas[0].identifier_ == "MySchema");
-  REQUIRE(identifiableSchemas[1].identifier_ == "MySchemaA");
+  REQUIRE((identifiableSchemas[0].identifier_ == "MySchema" ||
+           identifiableSchemas[1].identifier_ == "MySchema"));
+  REQUIRE((identifiableSchemas[1].identifier_ == "MySchemaA" ||
+           identifiableSchemas[0].identifier_ == "MySchemaA"));
+}
+
+/// @brief Preferred identifiers caused an incident where some schemas were
+/// handled before the rest of the schemas, causing swaps in the array, breaking
+/// dependencies. This test case ensures that the dependencies are not broken.
+TEST_CASE("IdentifiableSchema preferred identifiers don't break dependencies",
+          "[IdentifiableSchema]") {
+  const auto json = R"(
+{
+  "$id": "https://example.com/schema",
+  "title": "Schema",
+  "properties": {
+    "a": {
+      "type": "string"
+    },
+    "b": {
+      "$ref": "#/properties/a"
+    }
+  },
+  "additionalProperties": false,
+  "required": ["a"],
+  "type": "object"
+})"_json;
+  const UriWrapper baseUri("https://example.com/schema");
+  const JSONPointer pointer = JSONPointer();
+  const Draft draft = Draft::DRAFT_07;
+  std::vector<std::unique_ptr<LinkedSchema>> linkedSchemas;
+  linkedSchemas.emplace_back(std::make_unique<LinkedSchema>(
+      json["properties"]["a"], baseUri, pointer / "properties" / "a", draft,
+      std::map<UriWrapper, size_t>{}));
+  linkedSchemas.emplace_back(std::make_unique<LinkedSchema>(
+      json["properties"]["b"], baseUri, pointer / "properties" / "b", draft,
+      std::map<UriWrapper, size_t>{
+          {baseUri.withPointer(pointer / "properties" / "a"), 0}}));
+  linkedSchemas.emplace_back(std::make_unique<LinkedSchema>(
+      json, baseUri, pointer, draft,
+      std::map<UriWrapper, size_t>{
+          {baseUri.withPointer(pointer / "properties" / "a"), 0},
+          {baseUri.withPointer(pointer / "properties" / "b"), 1}}));
+  std::map<UriWrapper, std::string> preferredIdentifiers = {
+      {baseUri.withPointer(pointer / "properties" / "b"), "MySchemaB"}};
+
+  auto identifiableSchemas = IdentifiableSchema::transition(
+      std::move(linkedSchemas), preferredIdentifiers);
+
+  REQUIRE(identifiableSchemas.size() == 3);
+  REQUIRE(identifiableSchemas[0].pointer_ == pointer / "properties" / "a");
+  REQUIRE(identifiableSchemas[1].pointer_ == pointer / "properties" / "b");
+  REQUIRE(identifiableSchemas[2].pointer_ == pointer);
 }