Skip to content

Commit f696b2c

Browse files
jviottijviotti
authored
Use WeakPointer for SchemaFrame::Location::pointer (#2181)
Signed-off-by: Juan Cruz Viotti <[email protected]>
1 parent 8fb7ba6 commit f696b2c

31 files changed

+456
-234
lines changed

‎src/core/jsonpointer/include/sourcemeta/core/jsonpointer.h‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,28 @@ auto get(const JSON &document, const WeakPointer &pointer) -> const JSON &;
114114
// constant reference.
115115
auto get(JSON &&document, const WeakPointer &pointer) -> const JSON & = delete;
116116

117+
/// @ingroup jsonpointer
118+
/// Get a value from a JSON document using a JSON WeakPointer (non-`const`
119+
/// overload). For example:
120+
///
121+
/// ```cpp
122+
/// #include <sourcemeta/core/json.h>
123+
/// #include <sourcemeta/core/jsonpointer.h>
124+
/// #include <cassert>
125+
/// #include <sstream>
126+
///
127+
/// std::istringstream stream{"[ { \"foo\": 1 }, { \"bar\": 2 } ]"};
128+
/// auto document{sourcemeta::core::parse_json(stream)};
129+
/// const sourcemeta::core::Pointer pointer{1, "bar"};
130+
/// sourcemeta::core::JSON &value{
131+
/// sourcemeta::core::get(document,
132+
/// sourcemeta::core::to_weak_pointer(pointer))};
133+
/// value = sourcemeta::core::JSON{3};
134+
/// assert(document.at(1).at("bar").to_integer() == 3);
135+
/// ```
136+
SOURCEMETA_CORE_JSONPOINTER_EXPORT
137+
auto get(JSON &document, const WeakPointer &pointer) -> JSON &;
138+
117139
/// @ingroup jsonpointer
118140
/// Get a value from a JSON document using a Pointer, returning an optional that
119141
/// is not set if the path does not exist in the document. For example:

‎src/core/jsonpointer/jsonpointer.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,14 @@ auto get(JSON &document, const Pointer &pointer) -> JSON & {
162162
return traverse_all<std::allocator, JSON>(document, pointer);
163163
}
164164

165+
auto get(JSON &document, const WeakPointer &pointer) -> JSON & {
166+
if (pointer.empty()) {
167+
return document;
168+
}
169+
170+
return traverse_all<std::allocator, JSON>(document, pointer);
171+
}
172+
165173
auto try_get(const JSON &document, const Pointer &pointer) -> const JSON * {
166174
return pointer.empty() ? &document : try_traverse(document, pointer);
167175
}

‎src/core/jsonschema/format.cc‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <limits> // std::numeric_limits
55
#include <string_view> // std::string_view
66
#include <unordered_map> // std::unordered_map
7+
#include <vector> // std::vector
78

89
namespace {
910

@@ -139,19 +140,28 @@ auto format(JSON &schema, const SchemaWalker &walker,
139140
const SchemaResolver &resolver, std::string_view default_dialect)
140141
-> void {
141142
assert(is_schema(schema));
142-
SchemaFrame frame{SchemaFrame::Mode::Locations};
143-
frame.analyse(schema, walker, resolver, default_dialect);
144-
145-
for (const auto &entry : frame.locations()) {
146-
if (entry.second.type != SchemaFrame::LocationType::Resource &&
147-
entry.second.type != SchemaFrame::LocationType::Subschema) {
148-
continue;
143+
std::vector<JSON *> objects_to_reorder;
144+
145+
{
146+
SchemaFrame frame{SchemaFrame::Mode::Locations};
147+
frame.analyse(schema, walker, resolver, default_dialect);
148+
149+
for (const auto &entry : frame.locations()) {
150+
if (entry.second.type != SchemaFrame::LocationType::Resource &&
151+
entry.second.type != SchemaFrame::LocationType::Subschema) {
152+
continue;
153+
}
154+
155+
auto &subschema{get(schema, entry.second.pointer)};
156+
if (subschema.is_object()) {
157+
objects_to_reorder.push_back(&subschema);
158+
}
149159
}
160+
}
150161

151-
auto &value{get(schema, entry.second.pointer)};
152-
if (value.is_object()) {
153-
value.reorder(keyword_compare);
154-
}
162+
// Now apply the reordering after the frame is destroyed
163+
for (auto *object : objects_to_reorder) {
164+
object->reorder(keyword_compare);
155165
}
156166
}
157167

‎src/core/jsonschema/frame.cc‎

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ auto store(sourcemeta::core::SchemaFrame::Locations &frame,
287287
{.parent = parent,
288288
.type = entry_type,
289289
.base = base,
290-
.pointer = to_pointer(pointer_from_root),
290+
.pointer = pointer_from_root,
291291
.relative_pointer = relative_pointer_offset,
292292
.dialect = dialect,
293293
.base_dialect = base_dialect}});
@@ -357,9 +357,9 @@ auto SchemaFrame::to_json(
357357
entry.assign_assume_new("pointer",
358358
sourcemeta::core::to_json(location.second.pointer));
359359
if (tracker.has_value()) {
360-
entry.assign_assume_new(
361-
"position", sourcemeta::core::to_json(
362-
tracker.value().get(location.second.pointer)));
360+
entry.assign_assume_new("position",
361+
sourcemeta::core::to_json(tracker.value().get(
362+
to_pointer(location.second.pointer))));
363363
} else {
364364
entry.assign_assume_new("position", sourcemeta::core::to_json(nullptr));
365365
}
@@ -573,7 +573,7 @@ auto SchemaFrame::analyse(const JSON &root, const SchemaWalker &walker,
573573
const auto maybe_match{
574574
this->locations_.find({SchemaReferenceType::Static, new_id})};
575575
if (maybe_match != this->locations_.cend() &&
576-
maybe_match->second.pointer != common_pointer) {
576+
maybe_match->second.pointer != common_pointer_weak) {
577577
throw_already_exists(new_id);
578578
}
579579

@@ -1017,8 +1017,8 @@ auto SchemaFrame::vocabularies(const Location &location,
10171017
auto SchemaFrame::uri(const Location &location,
10181018
const Pointer &relative_schema_location) const
10191019
-> JSON::String {
1020-
return to_uri(this->relative_instance_location(location).concat(
1021-
relative_schema_location),
1020+
return to_uri(to_pointer(this->relative_instance_location(location))
1021+
.concat(relative_schema_location),
10221022
location.base)
10231023
.recompose();
10241024
}
@@ -1057,7 +1057,7 @@ auto SchemaFrame::traverse(const std::string_view uri) const
10571057
return std::nullopt;
10581058
}
10591059

1060-
auto SchemaFrame::traverse(const Pointer &pointer) const
1060+
auto SchemaFrame::traverse(const WeakPointer &pointer) const
10611061
-> std::optional<std::reference_wrapper<const Location>> {
10621062
// TODO: This is slow. Consider adding a pointer-indexed secondary
10631063
// lookup structure to SchemaFrame
@@ -1070,7 +1070,7 @@ auto SchemaFrame::traverse(const Pointer &pointer) const
10701070
return std::nullopt;
10711071
}
10721072

1073-
auto SchemaFrame::uri(const Pointer &pointer) const
1073+
auto SchemaFrame::uri(const WeakPointer &pointer) const
10741074
-> std::optional<std::reference_wrapper<const JSON::String>> {
10751075
for (const auto &entry : this->locations_) {
10761076
if (entry.second.pointer == pointer) {
@@ -1086,7 +1086,7 @@ auto SchemaFrame::dereference(const Location &location,
10861086
-> std::pair<SchemaReferenceType,
10871087
std::optional<std::reference_wrapper<const Location>>> {
10881088
const auto effective_location{
1089-
location.pointer.concat({relative_schema_location})};
1089+
to_pointer(location.pointer).concat(relative_schema_location)};
10901090
const auto maybe_reference_entry{this->references_.find(
10911091
{SchemaReferenceType::Static, effective_location})};
10921092
if (maybe_reference_entry == this->references_.cend()) {
@@ -1127,7 +1127,7 @@ auto SchemaFrame::for_each_unresolved_reference(
11271127
}
11281128
}
11291129

1130-
auto SchemaFrame::has_references_to(const Pointer &pointer) const -> bool {
1130+
auto SchemaFrame::has_references_to(const WeakPointer &pointer) const -> bool {
11311131
for (const auto &reference : this->references_) {
11321132
assert(!reference.first.second.empty());
11331133
assert(reference.first.second.back().is_property());
@@ -1157,7 +1157,8 @@ auto SchemaFrame::has_references_to(const Pointer &pointer) const -> bool {
11571157
return false;
11581158
}
11591159

1160-
auto SchemaFrame::has_references_through(const Pointer &pointer) const -> bool {
1160+
auto SchemaFrame::has_references_through(const WeakPointer &pointer) const
1161+
-> bool {
11611162
for (const auto &reference : this->references_) {
11621163
assert(!reference.first.second.empty());
11631164
assert(reference.first.second.back().is_property());
@@ -1187,8 +1188,40 @@ auto SchemaFrame::has_references_through(const Pointer &pointer) const -> bool {
11871188
return false;
11881189
}
11891190

1191+
auto SchemaFrame::has_references_through(const WeakPointer &pointer,
1192+
const WeakPointer::Token &tail) const
1193+
-> bool {
1194+
for (const auto &reference : this->references_) {
1195+
assert(!reference.first.second.empty());
1196+
assert(reference.first.second.back().is_property());
1197+
1198+
if (reference.first.first == SchemaReferenceType::Static) {
1199+
const auto match{this->locations_.find(
1200+
{reference.first.first, reference.second.destination})};
1201+
if (match != this->locations_.cend() &&
1202+
match->second.pointer.starts_with(pointer, tail)) {
1203+
return true;
1204+
}
1205+
} else {
1206+
for (const auto &location : this->locations_) {
1207+
if (location.second.type == LocationType::Anchor &&
1208+
location.first.first == SchemaReferenceType::Dynamic &&
1209+
location.second.pointer.starts_with(pointer, tail)) {
1210+
if (!reference.second.fragment.has_value() ||
1211+
URI{location.first.second}.fragment().value_or("") ==
1212+
reference.second.fragment.value()) {
1213+
return true;
1214+
}
1215+
}
1216+
}
1217+
}
1218+
}
1219+
1220+
return false;
1221+
}
1222+
11901223
auto SchemaFrame::relative_instance_location(const Location &location) const
1191-
-> Pointer {
1224+
-> WeakPointer {
11921225
return location.pointer.slice(location.relative_pointer);
11931226
}
11941227

‎src/core/jsonschema/include/sourcemeta/core/jsonschema_frame.h‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,11 @@ class SOURCEMETA_CORE_JSONSCHEMA_EXPORT SchemaFrame {
8282
/// The reference type is part of the key as it is possible to
8383
/// have a static and a dynamic reference to the same location
8484
/// on the same schema object.
85-
using References =
86-
std::map<std::pair<SchemaReferenceType, Pointer>, ReferencesEntry>;
85+
using References = std::map<std::pair<SchemaReferenceType,
86+
// TODO: Turn this into a weak pointer
87+
// or reference to the location pointer?
88+
Pointer>,
89+
ReferencesEntry>;
8790

8891
#if defined(__GNUC__)
8992
#pragma GCC diagnostic push
@@ -109,8 +112,7 @@ class SOURCEMETA_CORE_JSONSCHEMA_EXPORT SchemaFrame {
109112
std::optional<WeakPointer> parent;
110113
LocationType type;
111114
std::string_view base;
112-
// TODO: Turn this into a weak pointer
113-
Pointer pointer;
115+
WeakPointer pointer;
114116
std::size_t relative_pointer;
115117
std::string_view dialect;
116118
SchemaBaseDialect base_dialect;
@@ -181,11 +183,11 @@ class SOURCEMETA_CORE_JSONSCHEMA_EXPORT SchemaFrame {
181183
-> std::optional<std::reference_wrapper<const Location>>;
182184

183185
/// Get the location associated with a given pointer
184-
[[nodiscard]] auto traverse(const Pointer &pointer) const
186+
[[nodiscard]] auto traverse(const WeakPointer &pointer) const
185187
-> std::optional<std::reference_wrapper<const Location>>;
186188

187189
/// Turn an absolute pointer into a location URI
188-
[[nodiscard]] auto uri(const Pointer &pointer) const
190+
[[nodiscard]] auto uri(const WeakPointer &pointer) const
189191
-> std::optional<std::reference_wrapper<const JSON::String>>;
190192

191193
/// Try to dereference a reference location into its destination location
@@ -206,15 +208,21 @@ class SOURCEMETA_CORE_JSONSCHEMA_EXPORT SchemaFrame {
206208
&callback) const -> void;
207209

208210
/// Check if there are any references to a given location pointer
209-
[[nodiscard]] auto has_references_to(const Pointer &pointer) const -> bool;
211+
[[nodiscard]] auto has_references_to(const WeakPointer &pointer) const
212+
-> bool;
210213

211214
/// Check if there are any references that go through a given location pointer
212-
[[nodiscard]] auto has_references_through(const Pointer &pointer) const
215+
[[nodiscard]] auto has_references_through(const WeakPointer &pointer) const
213216
-> bool;
217+
/// Check if there are any references that go through a given location pointer
218+
/// with a tail token
219+
[[nodiscard]] auto
220+
has_references_through(const WeakPointer &pointer,
221+
const WeakPointer::Token &tail) const -> bool;
214222

215223
/// Get the relative instance location pointer for a given location entry
216224
[[nodiscard]] auto relative_instance_location(const Location &location) const
217-
-> Pointer;
225+
-> WeakPointer;
218226

219227
/// Check if the frame has no analysed data
220228
[[nodiscard]] auto empty() const noexcept -> bool;

‎src/core/jsonschema/transformer.cc‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -107,21 +107,24 @@ auto SchemaTransformer::check(const JSON &schema, const SchemaWalker &walker,
107107

108108
// Framing may report resource twice or more given default identifiers and
109109
// nested resources, risking reporting the same errors twice
110-
if (!visited.insert(entry.second.pointer).second) {
110+
const auto [visited_iterator, inserted] =
111+
visited.insert(to_pointer(entry.second.pointer));
112+
if (!inserted) {
111113
continue;
112114
}
115+
const auto &entry_pointer{*visited_iterator};
113116

114117
subschema_count += 1;
115118

116-
const auto &current{get(schema, entry.second.pointer)};
119+
const auto &current{get(schema, entry_pointer)};
117120
const auto current_vocabularies{frame.vocabularies(entry.second, resolver)};
118121
bool subresult{true};
119122
for (const auto &rule : this->rules) {
120123
const auto outcome{rule->check(current, schema, current_vocabularies,
121124
walker, resolver, frame, entry.second)};
122125
if (outcome.applies) {
123126
subresult = false;
124-
callback(entry.second.pointer, rule->name(), rule->message(), outcome);
127+
callback(entry_pointer, rule->name(), rule->message(), outcome);
125128
}
126129
}
127130

@@ -180,13 +183,16 @@ auto SchemaTransformer::apply(JSON &schema, const SchemaWalker &walker,
180183

181184
// Framing may report resource twice or more given default identifiers and
182185
// nested resources, risking reporting the same errors twice
183-
if (!visited.insert(entry.second.pointer).second) {
186+
const auto [visited_iterator, inserted] =
187+
visited.insert(to_pointer(entry.second.pointer));
188+
if (!inserted) {
184189
continue;
185190
}
191+
const auto &entry_pointer{*visited_iterator};
186192

187193
subschema_count += 1;
188194

189-
auto &current{get(schema, entry.second.pointer)};
195+
auto &current{get(schema, entry_pointer)};
190196
const auto current_vocabularies{
191197
frame.vocabularies(entry.second, resolver)};
192198

@@ -199,10 +205,6 @@ auto SchemaTransformer::apply(JSON &schema, const SchemaWalker &walker,
199205
continue;
200206
}
201207

202-
// Store data we need before invalidating the frame
203-
const auto transformed_pointer{entry.second.pointer};
204-
const auto transformed_relative_pointer{entry.second.relative_pointer};
205-
206208
// Collect reference information BEFORE invalidating the frame.
207209
// We need to save this data because after the transform, the old
208210
// frame's views may point to invalid memory, and a new frame won't
@@ -219,7 +221,7 @@ auto SchemaTransformer::apply(JSON &schema, const SchemaWalker &walker,
219221
const auto &target{destination.value().get()};
220222
potentially_broken_references.push_back(
221223
{reference.first.second, JSON::String{reference.second.original},
222-
reference.second.destination, target.pointer,
224+
reference.second.destination, to_pointer(target.pointer),
223225
target.relative_pointer});
224226
}
225227

@@ -228,15 +230,15 @@ auto SchemaTransformer::apply(JSON &schema, const SchemaWalker &walker,
228230
} catch (const SchemaAbortError &) {
229231
result = false;
230232
subschema_failed = true;
231-
callback(transformed_pointer, rule->name(), rule->message(), outcome);
233+
callback(entry_pointer, rule->name(), rule->message(), outcome);
232234
continue;
233235
}
234236

235237
applied = true;
236238

237239
frame.analyse(schema, walker, resolver, default_dialect, default_id);
238240

239-
const auto new_location{frame.traverse(transformed_pointer)};
241+
const auto new_location{frame.traverse(to_weak_pointer(entry_pointer))};
240242
// The location should still exist after transform
241243
assert(new_location.has_value());
242244

@@ -271,7 +273,8 @@ auto SchemaTransformer::apply(JSON &schema, const SchemaWalker &walker,
271273
saved_reference.destination, saved_reference.origin,
272274
saved_reference.target_pointer.slice(
273275
saved_reference.target_relative_pointer),
274-
transformed_pointer.slice(transformed_relative_pointer))};
276+
entry_pointer.slice(
277+
new_location.value().get().relative_pointer))};
275278

276279
// Note we use the base from the original reference before any
277280
// canonicalisation takes place so that we don't overly change
@@ -290,7 +293,7 @@ auto SchemaTransformer::apply(JSON &schema, const SchemaWalker &walker,
290293
current.fast_hash()};
291294
if (processed_rules.contains(mark)) {
292295
throw SchemaTransformRuleProcessedTwiceError(rule->name(),
293-
transformed_pointer);
296+
entry_pointer);
294297
}
295298

296299
processed_rules.emplace(std::move(mark));

0 commit comments

Comments
 (0)