Skip to content

Commit e4943d0

Browse files
jviottijviotti
authored
Fix various documentation and canonicalizer bugs (#741)
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent 216d8d4 commit e4943d0

7 files changed

Lines changed: 760 additions & 101 deletions

File tree

‎src/alterschema/canonicalizer/single_branch_allof.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ class SingleBranchAllOf final : public SchemaTransformRule {
3232
ONLY_CONTINUE_IF(!frame.has_references_through(
3333
location.pointer, WeakPointer::Token{std::cref(KEYWORD)}));
3434
const auto &branch{schema.at(KEYWORD).at(0)};
35-
if (branch.is_object() && branch.size() == 1) {
36-
const auto &key{branch.as_object().cbegin()->first};
37-
ONLY_CONTINUE_IF(key != "$ref" && key != "$dynamicRef" &&
38-
key != "$recursiveRef");
35+
if (branch.is_object()) {
36+
ONLY_CONTINUE_IF(!branch.defines("$ref") &&
37+
!branch.defines("$dynamicRef") &&
38+
!branch.defines("$recursiveRef"));
3939
}
4040
return true;
4141
}

‎src/alterschema/canonicalizer/type_with_applicator_to_allof.h‎

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,10 @@ class TypeWithApplicatorToAllOf final : public SchemaTransformRule {
5656
const bool has_structural{has_type || has_enum || has_ref};
5757

5858
bool modern_ref_needs_wrapping{false};
59+
this->ref_annotations_only_ = false;
5960
if (this->has_modern_ref_ || this->has_dynamic_ref_ ||
6061
this->has_recursive_ref_) {
62+
this->ref_annotations_only_ = true;
6163
for (const auto &entry : schema.as_object()) {
6264
if (entry.first == "$ref" || entry.first == "$dynamicRef" ||
6365
entry.first == "$recursiveRef") {
@@ -68,7 +70,12 @@ class TypeWithApplicatorToAllOf final : public SchemaTransformRule {
6870
keyword_type != sourcemeta::core::SchemaKeywordType::Annotation &&
6971
keyword_type != sourcemeta::core::SchemaKeywordType::Comment) {
7072
modern_ref_needs_wrapping = true;
71-
break;
73+
if (keyword_type != sourcemeta::core::SchemaKeywordType::Reference &&
74+
keyword_type != sourcemeta::core::SchemaKeywordType::Other &&
75+
keyword_type !=
76+
sourcemeta::core::SchemaKeywordType::LocationMembers) {
77+
this->ref_annotations_only_ = false;
78+
}
7279
}
7380
}
7481
}
@@ -94,9 +101,10 @@ class TypeWithApplicatorToAllOf final : public SchemaTransformRule {
94101
}
95102
}
96103

97-
ONLY_CONTINUE_IF((has_structural && applicator_count >= 1) ||
98-
applicator_count >= 2 || modern_ref_needs_wrapping ||
99-
has_orphaned_typed_keywords);
104+
ONLY_CONTINUE_IF(
105+
(has_structural && applicator_count >= 1) || applicator_count >= 2 ||
106+
modern_ref_needs_wrapping ||
107+
(has_orphaned_typed_keywords && !this->ref_annotations_only_));
100108

101109
this->strategy_ = Strategy::FullRestructure;
102110
this->applicators_with_refs_ = 0;
@@ -222,8 +230,10 @@ class TypeWithApplicatorToAllOf final : public SchemaTransformRule {
222230
entry.first == "allOf" || entry.first == "oneOf" ||
223231
entry.first == "$schema" || entry.first == "id" ||
224232
entry.first == "$id" || entry.first == "definitions" ||
225-
entry.first == "$defs" || entry.first == "dependencies" ||
226-
entry.first == "dependentSchemas" ||
233+
entry.first == "$defs" || entry.first == "$anchor" ||
234+
entry.first == "$dynamicAnchor" ||
235+
entry.first == "$recursiveAnchor" || entry.first == "$vocabulary" ||
236+
entry.first == "dependencies" || entry.first == "dependentSchemas" ||
227237
(this->has_if_then_else_ &&
228238
(entry.first == "if" || entry.first == "then" ||
229239
entry.first == "else")) ||
@@ -253,6 +263,25 @@ class TypeWithApplicatorToAllOf final : public SchemaTransformRule {
253263
schema.assign("allOf", std::move(new_allof));
254264
}
255265

266+
if (this->has_modern_ref_ && schema.defines("$ref")) {
267+
auto branch{JSON::make_object()};
268+
branch.assign("$ref", schema.at("$ref"));
269+
schema.at("allOf").push_back(std::move(branch));
270+
schema.erase("$ref");
271+
}
272+
if (this->has_dynamic_ref_ && schema.defines("$dynamicRef")) {
273+
auto branch{JSON::make_object()};
274+
branch.assign("$dynamicRef", schema.at("$dynamicRef"));
275+
schema.at("allOf").push_back(std::move(branch));
276+
schema.erase("$dynamicRef");
277+
}
278+
if (this->has_recursive_ref_ && schema.defines("$recursiveRef")) {
279+
auto branch{JSON::make_object()};
280+
branch.assign("$recursiveRef", schema.at("$recursiveRef"));
281+
schema.at("allOf").push_back(std::move(branch));
282+
schema.erase("$recursiveRef");
283+
}
284+
256285
for (const auto &applicator : APPLICATORS_WITHOUT_ALLOF) {
257286
if (!schema.defines(applicator)) {
258287
continue;
@@ -291,16 +320,34 @@ class TypeWithApplicatorToAllOf final : public SchemaTransformRule {
291320
if (this->has_modern_ref_ && schema.defines("$ref")) {
292321
auto branch{JSON::make_object()};
293322
branch.assign("$ref", schema.at("$ref"));
323+
if (this->ref_annotations_only_ && !this->typed_keywords_.empty()) {
324+
for (const auto &entry : typed_branch.as_object()) {
325+
branch.assign(entry.first, entry.second);
326+
}
327+
this->typed_keywords_.clear();
328+
}
294329
new_allof.push_back(std::move(branch));
295330
}
296331
if (this->has_dynamic_ref_ && schema.defines("$dynamicRef")) {
297332
auto branch{JSON::make_object()};
298333
branch.assign("$dynamicRef", schema.at("$dynamicRef"));
334+
if (this->ref_annotations_only_ && !this->typed_keywords_.empty()) {
335+
for (const auto &entry : typed_branch.as_object()) {
336+
branch.assign(entry.first, entry.second);
337+
}
338+
this->typed_keywords_.clear();
339+
}
299340
new_allof.push_back(std::move(branch));
300341
}
301342
if (this->has_recursive_ref_ && schema.defines("$recursiveRef")) {
302343
auto branch{JSON::make_object()};
303344
branch.assign("$recursiveRef", schema.at("$recursiveRef"));
345+
if (this->ref_annotations_only_ && !this->typed_keywords_.empty()) {
346+
for (const auto &entry : typed_branch.as_object()) {
347+
branch.assign(entry.first, entry.second);
348+
}
349+
this->typed_keywords_.clear();
350+
}
304351
new_allof.push_back(std::move(branch));
305352
}
306353

@@ -342,6 +389,18 @@ class TypeWithApplicatorToAllOf final : public SchemaTransformRule {
342389
if (schema.defines("$defs")) {
343390
new_schema.assign("$defs", schema.at("$defs"));
344391
}
392+
if (schema.defines("$anchor")) {
393+
new_schema.assign("$anchor", schema.at("$anchor"));
394+
}
395+
if (schema.defines("$dynamicAnchor")) {
396+
new_schema.assign("$dynamicAnchor", schema.at("$dynamicAnchor"));
397+
}
398+
if (schema.defines("$recursiveAnchor")) {
399+
new_schema.assign("$recursiveAnchor", schema.at("$recursiveAnchor"));
400+
}
401+
if (schema.defines("$vocabulary")) {
402+
new_schema.assign("$vocabulary", schema.at("$vocabulary"));
403+
}
345404
if (schema.defines("dependencies")) {
346405
new_schema.assign("dependencies", schema.at("dependencies"));
347406
}
@@ -455,6 +514,7 @@ class TypeWithApplicatorToAllOf final : public SchemaTransformRule {
455514
mutable bool has_dynamic_ref_{false};
456515
mutable bool has_recursive_ref_{false};
457516
mutable bool has_unevaluated_{false};
517+
mutable bool ref_annotations_only_{false};
458518
mutable std::vector<std::string> typed_keywords_;
459519
mutable std::uint8_t applicator_indices_{0};
460520
mutable std::uint8_t applicators_with_refs_{0};

‎src/alterschema/transformer.cc‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -300,33 +300,40 @@ auto SchemaTransformer::apply(core::JSON &schema,
300300
frame.traverse(core::to_weak_pointer(entry_pointer))};
301301
assert(new_location.has_value());
302302

303-
const auto new_vocabularies{
304-
frame.vocabularies(new_location.value().get(), resolver)};
305-
306-
if (rule->check(current, schema, new_vocabularies, walker, resolver,
307-
frame, new_location.value().get(), exclude_keyword)
308-
.applies) {
309-
std::ostringstream error;
310-
error << "Rule condition holds after application: " << rule->name();
311-
throw std::runtime_error(error.str());
312-
}
313-
303+
// Fix broken references before re-checking the condition,
304+
// as the re-check may mutate rule state that rereference needs
314305
bool references_fixed{false};
306+
const auto resource_offset{new_location.value().get().relative_pointer};
307+
const auto current_slice{entry_pointer.slice(resource_offset)};
315308
for (const auto &saved_reference : potentially_broken_references) {
316309
if (core::try_get(schema, saved_reference.target_pointer)) {
317310
continue;
318311
}
319312

313+
// If the origin was also relocated, resolve its new location
314+
auto effective_origin{saved_reference.origin};
320315
if (!core::try_get(schema, saved_reference.origin.initial())) {
321-
continue;
316+
try {
317+
const auto new_origin{rule->rereference(
318+
saved_reference.destination, saved_reference.origin,
319+
saved_reference.origin.slice(resource_offset),
320+
current_slice)};
321+
effective_origin =
322+
saved_reference.origin.slice(0, resource_offset)
323+
.concat(new_origin);
324+
} catch (...) {
325+
continue;
326+
}
327+
if (!core::try_get(schema, effective_origin.initial())) {
328+
continue;
329+
}
322330
}
323331

324332
const auto new_relative{rule->rereference(
325333
saved_reference.destination, saved_reference.origin,
326334
saved_reference.target_pointer.slice(
327335
saved_reference.target_relative_pointer),
328-
entry_pointer.slice(
329-
new_location.value().get().relative_pointer))};
336+
current_slice)};
330337
const auto new_fragment{
331338
saved_reference.fragment ==
332339
core::to_string(saved_reference.target_pointer)
@@ -337,11 +344,21 @@ auto SchemaTransformer::apply(core::JSON &schema,
337344

338345
core::URI original{saved_reference.original};
339346
original.fragment(core::to_string(new_fragment));
340-
core::set(schema, saved_reference.origin,
341-
core::JSON{original.recompose()});
347+
core::set(schema, effective_origin, core::JSON{original.recompose()});
342348
references_fixed = true;
343349
}
344350

351+
const auto new_vocabularies{
352+
frame.vocabularies(new_location.value().get(), resolver)};
353+
354+
if (rule->check(current, schema, new_vocabularies, walker, resolver,
355+
frame, new_location.value().get(), exclude_keyword)
356+
.applies) {
357+
std::ostringstream error;
358+
error << "Rule condition holds after application: " << rule->name();
359+
throw std::runtime_error(error.str());
360+
}
361+
345362
std::tuple<core::Pointer, std::string_view, core::JSON> mark{
346363
entry_pointer, rule->name(), current};
347364
if (processed_rules.contains(mark)) {

0 commit comments

Comments
 (0)