Skip to content

Commit 8bc5bca

Browse files
sakarssakars
authored
Merge pull request #11 from sakars/bugfix/preferred-identifiers-broke-depoendency-indices
Bugfix/preferred identifiers broke dependency indices
2 parents 9f1cd01 + 1c5f5f1 commit 8bc5bca

File tree

3 files changed

+96
-19
lines changed

3 files changed

+96
-19
lines changed

‎include/IdentifiableSchema.h‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ class IdentifiableSchema : public LinkedSchema {
3333
// Note: We add reserved identifiers to the identifiers to avoid conflicts
3434
// with the predefined schemas.
3535
std::set<std::string> identifiers{"True", "False"};
36-
std::vector<IdentifiableSchema> identifiableSchemas;
36+
std::vector<std::unique_ptr<IdentifiableSchema>> identifiableSchemas(
37+
linkedSchemas.size());
38+
3739
for (const auto& [uri, identifier] : preferredIdentifiers) {
3840
for (size_t i = 0; i < linkedSchemas.size(); i++) {
3941
const auto& linkedSchema = linkedSchemas[i];
@@ -43,13 +45,16 @@ class IdentifiableSchema : public LinkedSchema {
4345
linkedSchema->json_, linkedSchema->baseUri_,
4446
linkedSchema->pointer_, linkedSchema->draft_, identifier);
4547
schema.dependencies_ = linkedSchema->dependencies_;
46-
identifiableSchemas.emplace_back(std::move(schema));
47-
linkedSchemas.erase(linkedSchemas.begin() + i);
48+
identifiableSchemas[i] =
49+
std::make_unique<IdentifiableSchema>(std::move(schema));
4850
break;
4951
}
5052
}
5153
}
5254
for (size_t i = 0; i < linkedSchemas.size(); i++) {
55+
if (identifiableSchemas[i] != nullptr) {
56+
continue;
57+
}
5358
const auto& linkedSchema = linkedSchemas[i];
5459
const std::string preferred_identifier =
5560
linkedSchema->getPreferredIdentifier();
@@ -69,9 +74,14 @@ class IdentifiableSchema : public LinkedSchema {
6974
linkedSchema->json_, linkedSchema->baseUri_, linkedSchema->pointer_,
7075
linkedSchema->draft_, identifier);
7176
schema.dependencies_ = linkedSchema->dependencies_;
72-
identifiableSchemas.emplace_back(std::move(schema));
77+
identifiableSchemas[i] =
78+
std::make_unique<IdentifiableSchema>(std::move(schema));
79+
}
80+
std::vector<IdentifiableSchema> identifiableSchemaVec;
81+
for (auto& schema : identifiableSchemas) {
82+
identifiableSchemaVec.push_back(std::move(*schema));
7383
}
74-
return identifiableSchemas;
84+
return identifiableSchemaVec;
7585
} catch (const std::exception& e) {
7686
std::cerr << "Error caught in transition from Linked to Identifiable:\n";
7787
throw e;
@@ -90,11 +100,11 @@ class IdentifiableSchema : public LinkedSchema {
90100
auto& ptrDump = uriDump[iSchema.pointer_.toFragment()];
91101
for (const auto& [uri, idx] : iSchema.dependencies_) {
92102
const auto& idxSchema = identifiableSchemas[idx];
93-
ptrDump.push_back(nlohmann::json::array(
94-
{uri.toString().value(),
95-
idxSchema.baseUri_.withPointer(idxSchema.pointer_)
96-
.toString()
97-
.value()}));
103+
ptrDump.push_back({{"uri", uri.toString().value()},
104+
{"dependent location",
105+
idxSchema.baseUri_.withPointer(idxSchema.pointer_)
106+
.toString()
107+
.value()}});
98108
}
99109
}
100110

‎include/SchemaResource.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,18 @@ class SchemaResource {
121121
static void dumpSchemas(SetMap<UriWrapper, SchemaResource>& schemaResources,
122122
std::filesystem::path outputDirectory = ".") {
123123
nlohmann::json schemaResourceDump = nlohmann::json::object();
124+
// iterate over all the schemas.
124125
for (const auto& [uris, schema] : schemaResources) {
125126
auto& baseUriList =
126127
schemaResourceDump[schema.get().baseUri_.toString().value()];
127-
baseUriList[schema.get().pointer_.toFragment()] = nlohmann::json::array();
128+
if (!baseUriList[schema.get().pointer_.toFragment()].is_array()) {
129+
baseUriList[schema.get().pointer_.toFragment()] =
130+
nlohmann::json::array();
131+
}
128132
for (const auto& uri : uris.get()) {
129133
baseUriList[schema.get().pointer_.toFragment()].push_back(
130-
nlohmann::json::array({uri.toFragmentlessString().value_or(""),
131-
uri.getFragment().value_or("")}));
134+
{{"uri", uri.toFragmentlessString().value_or("")},
135+
{"frag", uri.getFragment().value_or("")}});
132136
}
133137
}
134138
std::ofstream resourceDump(outputDirectory / "resource.dump.json");

‎tests/test_IdentifiableSchema.cpp‎

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,78 @@ TEST_CASE("IdentifiableSchema transition with preferred identifiers",
7373
std::move(linkedSchemas), preferredIdentifiers);
7474

7575
REQUIRE(identifiableSchemas.size() == 2);
76-
REQUIRE(identifiableSchemas[0].dependencies_.contains(
77-
baseUri.withPointer(pointer / "properties" / "a")));
78-
REQUIRE(identifiableSchemas[0].dependencies_.at(
79-
baseUri.withPointer(pointer / "properties" / "a")) == 0);
76+
REQUIRE((identifiableSchemas[0].dependencies_.contains(
77+
baseUri.withPointer(pointer / "properties" / "a")) ||
78+
identifiableSchemas[1].dependencies_.contains(
79+
baseUri.withPointer(pointer / "properties" / "a"))));
80+
81+
if (identifiableSchemas[0].dependencies_.contains(
82+
baseUri.withPointer(pointer / "properties" / "a"))) {
83+
REQUIRE(identifiableSchemas[0].dependencies_.at(
84+
baseUri.withPointer(pointer / "properties" / "a")) == 0);
85+
} else if (identifiableSchemas[1].dependencies_.contains(
86+
baseUri.withPointer(pointer / "properties" / "a"))) {
87+
REQUIRE(identifiableSchemas[1].dependencies_.at(
88+
baseUri.withPointer(pointer / "properties" / "a")) == 0);
89+
} else {
90+
FAIL("Dependency not found");
91+
}
8092
// Identifiers are unique
8193

8294
REQUIRE(identifiableSchemas[1].identifier_ !=
8395
identifiableSchemas[0].identifier_);
8496

85-
REQUIRE(identifiableSchemas[0].identifier_ == "MySchema");
86-
REQUIRE(identifiableSchemas[1].identifier_ == "MySchemaA");
97+
REQUIRE((identifiableSchemas[0].identifier_ == "MySchema" ||
98+
identifiableSchemas[1].identifier_ == "MySchema"));
99+
REQUIRE((identifiableSchemas[1].identifier_ == "MySchemaA" ||
100+
identifiableSchemas[0].identifier_ == "MySchemaA"));
101+
}
102+
103+
/// @brief Preferred identifiers caused an incident where some schemas were
104+
/// handled before the rest of the schemas, causing swaps in the array, breaking
105+
/// dependencies. This test case ensures that the dependencies are not broken.
106+
TEST_CASE("IdentifiableSchema preferred identifiers don't break dependencies",
107+
"[IdentifiableSchema]") {
108+
const auto json = R"(
109+
{
110+
"$id": "https://example.com/schema",
111+
"title": "Schema",
112+
"properties": {
113+
"a": {
114+
"type": "string"
115+
},
116+
"b": {
117+
"$ref": "#/properties/a"
118+
}
119+
},
120+
"additionalProperties": false,
121+
"required": ["a"],
122+
"type": "object"
123+
})"_json;
124+
const UriWrapper baseUri("https://example.com/schema");
125+
const JSONPointer pointer = JSONPointer();
126+
const Draft draft = Draft::DRAFT_07;
127+
std::vector<std::unique_ptr<LinkedSchema>> linkedSchemas;
128+
linkedSchemas.emplace_back(std::make_unique<LinkedSchema>(
129+
json["properties"]["a"], baseUri, pointer / "properties" / "a", draft,
130+
std::map<UriWrapper, size_t>{}));
131+
linkedSchemas.emplace_back(std::make_unique<LinkedSchema>(
132+
json["properties"]["b"], baseUri, pointer / "properties" / "b", draft,
133+
std::map<UriWrapper, size_t>{
134+
{baseUri.withPointer(pointer / "properties" / "a"), 0}}));
135+
linkedSchemas.emplace_back(std::make_unique<LinkedSchema>(
136+
json, baseUri, pointer, draft,
137+
std::map<UriWrapper, size_t>{
138+
{baseUri.withPointer(pointer / "properties" / "a"), 0},
139+
{baseUri.withPointer(pointer / "properties" / "b"), 1}}));
140+
std::map<UriWrapper, std::string> preferredIdentifiers = {
141+
{baseUri.withPointer(pointer / "properties" / "b"), "MySchemaB"}};
142+
143+
auto identifiableSchemas = IdentifiableSchema::transition(
144+
std::move(linkedSchemas), preferredIdentifiers);
145+
146+
REQUIRE(identifiableSchemas.size() == 3);
147+
REQUIRE(identifiableSchemas[0].pointer_ == pointer / "properties" / "a");
148+
REQUIRE(identifiableSchemas[1].pointer_ == pointer / "properties" / "b");
149+
REQUIRE(identifiableSchemas[2].pointer_ == pointer);
87150
}

0 commit comments

Comments
 (0)