Browse Source

refactor: cleanup some todos

Sam Jaffe 1 year ago
parent
commit
ed54c83423

+ 0 - 1
include/jvalidate/detail/reference_cache.h

@@ -37,7 +37,6 @@ public:
     }
 
     to_anchor_.emplace(absolute, canonical);
-    // TODO: Validate uniqueness?
     to_absolute_.emplace(canonical, absolute);
 
     return Reference(canonical);

+ 26 - 16
include/jvalidate/detail/reference_manager.h

@@ -93,25 +93,12 @@ public:
       return base.parent() / uri;
     }();
 
-    // TODO(samjaffe): Clean up this block too...
     URI const dyn_uri = ref.uri().empty() ? ref.uri() : uri;
-
-    OnBlockExit scope;
-    if (not ref.uri().empty() && dynamic_reference &&
-        active_dynamic_anchors_.contains(ref.anchor())) {
-      scope = dynamic_scope(Reference(dyn_uri));
+    if (std::optional dynref = dynamic(dyn_uri, ref, dynamic_reference)) {
+      return *dynref;
     }
 
-    if (std::optional dynamic = active_dynamic_anchors_.lookup(dyn_uri, ref.anchor())) {
-      if (dynamic_reference) {
-        return *dynamic;
-      }
-      dynamic_reference = ref.uri().empty() && ref.pointer().empty();
-    }
-
-    if (active_dynamic_anchors_.empty()) {
-      dynamic_reference = true;
-    }
+    dynamic_reference = dynamic_reference || active_dynamic_anchors_.empty();
 
     // Relative URI, not in the HEREDOC (or we set an $id)
     if (ref.uri().empty() and ref.anchor().empty()) {
@@ -122,6 +109,29 @@ public:
   }
 
 private:
+  std::optional<Reference> dynamic(URI const & uri, Reference const & ref,
+                                   inout<bool> dynamic_reference) {
+    bool const anchor_is_dynamic = active_dynamic_anchors_.contains(ref.anchor());
+    if (not dynamic_reference) {
+      // A normal $ref to an $anchor that matches a $dynamicAnchor breaks the
+      // dynamic recursion pattern. This requires that we are not looking for a
+      // subschema of the anchor AND that we are not targetting an anchor in a
+      // different root document.
+      dynamic_reference = (anchor_is_dynamic && ref.uri().empty() && ref.pointer().empty());
+      return std::nullopt;
+    }
+
+    OnBlockExit scope;
+    if (not ref.uri().empty() && anchor_is_dynamic) {
+      // Register the scope of this (potential) $dynamicAnchor BEFORE we attempt
+      // to enter the reference, in case we end up pointing to an otherwise
+      // suppressed $dynamicAnchor in a higher scope.
+      scope = dynamic_scope(Reference(uri));
+    }
+
+    return active_dynamic_anchors_.lookup(uri, ref.anchor());
+  }
+
   void prime(Adapter auto const & json, Reference where, schema::Version version,
              Keywords const & keywords) {
     if (json.type() != adapter::Type::Object) {

+ 0 - 1
include/jvalidate/detail/simple_adapter.h

@@ -167,7 +167,6 @@ public:
   bool equals(Adapter const & rhs, bool strict = false) const final {
     Type const rhs_type = rhs.type();
 
-    // TODO(samjaffe): Needs type coercion
     switch (rhs_type) {
     case Type::Null:
       return maybe_null(strict);

+ 1 - 0
include/jvalidate/validation_visitor.h

@@ -232,6 +232,7 @@ public:
   }
 
   Status visit(constraint::FormatConstraint const & cons) const {
+    // https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-validation-01#name-defined-formats
     NOOP_UNLESS_TYPE(String);
 
     if (not cfg_.validate_format) {