Selaa lähdekoodia

fix: numerous bugfixes

Sam Jaffe 1 vuosi sitten
vanhempi
commit
9ddb887a3a

+ 1 - 4
include/jvalidate/adapters/jsoncpp.h

@@ -39,9 +39,6 @@ public:
 };
 
 template <typename JSON> class JsonCppAdapter final : public detail::SimpleAdapter<JSON> {
-private:
-  JSON * value_;
-
 public:
   using JsonCppAdapter::SimpleAdapter::SimpleAdapter;
 
@@ -70,7 +67,7 @@ public:
   double as_number() const { return const_value().asDouble(); }
   std::string as_string() const { return const_value().asString(); }
 
-  JsonCppObjectAdapter<JSON> as_object() const { return value_; }
+  JsonCppObjectAdapter<JSON> as_object() const { return value(); }
 
   static std::string key(auto it) { return it.key().asString(); }
 

+ 6 - 4
include/jvalidate/constraint.h

@@ -276,12 +276,14 @@ public:
   }
 
   static auto minItems(detail::ParserContext<A> const & context) {
-    EXPECT(context.schema.type() == adapter::Type::Integer);
+    EXPECT(context.schema.type() == adapter::Type::Integer ||
+           context.schema.type() == adapter::Type::Number);
     return std::make_unique<constraint::MinItemsConstraint>(context.schema.as_integer());
   }
 
   static auto maxItems(detail::ParserContext<A> const & context) {
-    EXPECT(context.schema.type() == adapter::Type::Integer);
+    EXPECT(context.schema.type() == adapter::Type::Integer ||
+           context.schema.type() == adapter::Type::Number);
     return std::make_unique<constraint::MaxItemsConstraint>(context.schema.as_integer());
   }
 
@@ -304,8 +306,8 @@ public:
 
     Object const & parent = *context.parent;
     size_t start_after = 0;
-    if (not prefix.empty() && parent.contains(prefix)) {
-      start_after = parent[prefix].as_integer();
+    if (parent.contains(prefix) && parent[prefix].type() == adapter::Type::Array) {
+      start_after = parent[prefix].as_array().size();
     }
 
     using C = constraint::AdditionalItemsConstraint;

+ 31 - 4
include/jvalidate/schema.h

@@ -110,13 +110,28 @@ private:
   };
 
 private:
-  schema::Node accept_{true};
-  schema::Node reject_{false};
+  schema::Node accept_{false};
+  schema::Node reject_{true};
 
+  // A map of (URI, Anchor) => (URI, Pointer), binding an anchor reference
+  // to it's fully resolved path.
   std::map<detail::Reference, detail::Reference> anchors_;
+
+  // A map of anchors to DynamicRef info - note that DynamicRef.reconstruct is
+  // an unsafe object, because it holds an object which may hold references to
+  // temporary objects.
+  // Nothing should be added to this object except through calling
+  // {@see Node::resolve_anchor}, which returns a scope(exit) construct that
+  // cleans up the element.
   std::map<detail::Anchor, DynamicRef> dynamic_anchors_;
+
+  // An owning cache of all created schemas. Avoids storing duplicates such as
+  // the "always-true" schema, "always-false" schema, and schemas whose only
+  // meaningful field is "$ref", "$recursiveRef", or "$dynamicRef".
   std::map<detail::Reference, schema::Node> cache_;
 
+  // A non-owning cache of all schemas, including duplcates where multiple
+  // References map to the same underlying schema.
   std::map<detail::Reference, schema::Node const *> alias_cache_;
 
 public:
@@ -148,8 +163,11 @@ public:
    */
   template <Adapter A>
   Schema(A const & json, schema::Version version, DocumentCache<A> & external,
-         ConstraintFactory<A> const & factory = {})
-      : schema::Node(detail::ParserContext<A>{*this, json, version, factory, external}) {}
+         ConstraintFactory<A> const & factory = {}) {
+    detail::ParserContext<A> root{*this, json, version, factory, external};
+    // Prevent unintialized data caches
+    schema::Node::operator=(root);
+  }
 
   /**
    * @param json An adapter to a json schema
@@ -202,6 +220,13 @@ public:
       : Schema(adapter::AdapterFor<JSON const>(json), std::forward<Args>(args)...) {}
 
 private:
+  /**
+   * @brief Associate an anchor with its absolute path
+   * @pre We should not already have an anchor associated with this anchor
+   *
+   * @param anchor A URI-Reference containing only a URI and Anchor
+   * @param from A URI-Reference representing the absolute path to this Anchor
+   */
   void anchor(detail::Reference const & anchor, detail::Reference const & from) {
     EXPECT_M(anchors_.try_emplace(anchor.root(), from).second,
              "more than one anchor found for uri " << anchor);
@@ -342,6 +367,8 @@ template <Adapter A> detail::OnBlockExit Node::resolve_anchor(detail::ParserCont
     context.root.dynamic_anchor(anchor, context);
     return [&context, anchor]() { context.root.remove_dynamic_anchor(anchor, context.where); };
   }
+
+  return nullptr;
 }
 
 template <Adapter A> bool Node::resolve_reference(detail::ParserContext<A> const & context) {

+ 19 - 7
include/jvalidate/status.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <compare>
+
 namespace jvalidate {
 class Status {
 public:
@@ -12,7 +14,7 @@ public:
   Status(bool state) : state_(state ? Accept : Reject) {}
   Status(Enum state) : state_(state) {}
 
-  operator bool() const { return state_ == Accept; }
+  explicit operator bool() const { return state_ != Reject; }
 
   friend Status operator!(Status val) {
     if (val.state_ == Noop) {
@@ -25,23 +27,33 @@ public:
     if (lhs.state_ == Noop && rhs.state_ == Noop) {
       return Noop;
     }
-    if (lhs.state_ == Reject || rhs.state_ == Reject) {
-      return Reject;
+    if (lhs.state_ == Accept || rhs.state_ == Accept) {
+      return Accept;
     }
-    return Accept;
+    return Reject;
   }
 
   friend Status operator&(Status lhs, Status rhs) {
     if (lhs.state_ == Noop && rhs.state_ == Noop) {
       return Noop;
     }
-    if (lhs.state_ == Accept && rhs.state_ == Accept) {
-      return Accept;
+    if (lhs.state_ == Reject || rhs.state_ == Reject) {
+      return Reject;
     }
-    return Reject;
+    return Accept;
   }
 
   Status & operator&=(Status rhs) { return *this = *this & rhs; }
   Status & operator|=(Status rhs) { return *this = *this | rhs; }
+
+  friend auto operator==(Status lhs, Status::Enum rhs) {
+    return static_cast<int>(lhs.state_) == static_cast<int>(rhs);
+  }
+  friend auto operator!=(Status lhs, Status::Enum rhs) {
+    return static_cast<int>(lhs.state_) != static_cast<int>(rhs);
+  }
+  friend auto operator<=>(Status lhs, Status rhs) {
+    return static_cast<int>(lhs.state_) <=> static_cast<int>(rhs.state_);
+  }
 };
 }

+ 7 - 7
include/jvalidate/uri.h

@@ -10,20 +10,20 @@ namespace jvalidate {
 class URI {
 private:
   std::string uri_;
-  std::string_view resource_;
-  std::string_view scheme_;
+  size_t scheme_{0};
+  size_t resource_{0};
 
 public:
   URI() = default;
 
-  explicit URI(std::string_view uri) : uri_(uri), resource_(uri_) {
+  explicit URI(std::string_view uri) : uri_(uri) {
     if (uri_.back() == '#') {
       uri_.pop_back();
     }
 
     if (size_t n = uri_.find("://"); n != std::string::npos) {
-      scheme_ = {uri_.c_str(), n};
-      resource_.remove_prefix(n + 3);
+      scheme_ = n;
+      resource_ = n + 3;
     }
   }
 
@@ -31,8 +31,8 @@ public:
 
   URI operator/(URI const & relative) const { return URI(uri_ + relative.uri_); }
 
-  std::string_view scheme() const { return scheme_; }
-  std::string_view resource() const { return resource_; }
+  std::string_view scheme() const { return std::string_view(uri_).substr(0, scheme_); }
+  std::string_view resource() const { return std::string_view(uri_).substr(resource_); }
 
   explicit operator std::string const &() const { return uri_; }
   bool empty() const { return uri_.empty(); }

+ 11 - 11
include/jvalidate/validation_visitor.h

@@ -155,7 +155,7 @@ public:
     Status rval = Status::Accept;
     for (size_t i = cons.applies_after_nth; i < array.size(); ++i) {
       rval &= validate_subschema_on(cons.subschema, array[i], i);
-      if (!rval && result_ == nullptr) {
+      if (rval == Status::Reject && result_ == nullptr) {
         break;
       }
     }
@@ -204,7 +204,7 @@ public:
     size_t const n = std::min(cons.items.size(), array.size());
     for (size_t i = 0; i < n; ++i) {
       rval &= validate_subschema_on(cons.items[i], array[i], i);
-      if (!rval && result_ == nullptr) {
+      if (rval == Status::Reject && result_ == nullptr) {
         break;
       }
     }
@@ -254,7 +254,7 @@ public:
       if (not cons.properties.contains(key) && not matches_any_pattern(key)) {
         rval &= validate_subschema_on(cons.subschema, elem, key);
       }
-      if (!rval && result_ == nullptr) {
+      if (rval == Status::Reject && result_ == nullptr) {
         break;
       }
     }
@@ -273,7 +273,7 @@ public:
       }
 
       rval &= validate_subschema(subschema);
-      if (!rval && result_ == nullptr) {
+      if (rval == Status::Reject && result_ == nullptr) {
         break;
       }
     }
@@ -288,7 +288,7 @@ public:
       }
 
       rval &= required.empty();
-      if (!rval && result_ == nullptr) {
+      if (rval == Status::Reject && result_ == nullptr) {
         break;
       }
     }
@@ -316,7 +316,7 @@ public:
         if (regex.search(key)) {
           rval &= validate_subschema_on(subschema, elem, key);
         }
-        if (!rval && result_ == nullptr) {
+        if (rval == Status::Reject && result_ == nullptr) {
           break;
         }
       }
@@ -344,7 +344,7 @@ public:
       if (auto it = cons.properties.find(key); it != cons.properties.end()) {
         rval &= validate_subschema_on(it->second, elem, key);
       }
-      if (!rval && result_ == nullptr) {
+      if (rval == Status::Reject && result_ == nullptr) {
         break;
       }
     }
@@ -384,7 +384,7 @@ public:
       if (not local_result_->visited_items.contains(i)) {
         rval &= validate_subschema_on(cons.subschema, array[i], i);
       }
-      if (!rval && result_ == nullptr) {
+      if (rval == Status::Reject && result_ == nullptr) {
         break;
       }
     }
@@ -399,7 +399,7 @@ public:
       if (not local_result_->visited_properties.contains(key)) {
         rval &= validate_subschema_on(cons.subschema, elem, key);
       }
-      if (!rval && result_ == nullptr) {
+      if (rval == Status::Reject && result_ == nullptr) {
         break;
       }
     }
@@ -422,13 +422,13 @@ public:
     }
 
     for (auto const & [key, p_constraint] : schema_.constraints()) {
-      if (rval || result_) {
+      if (rval != Status::Reject || result_) {
         rval &= p_constraint->accept(*this);
       }
     }
 
     for (auto const & [key, p_constraint] : schema_.post_constraints()) {
-      if (rval || result_) {
+      if (rval != Status::Reject || result_) {
         rval &= p_constraint->accept(*this);
       }
     }

+ 35 - 0
tests/printer_helper.h

@@ -0,0 +1,35 @@
+#pragma once
+
+#include <iostream>
+
+#include <jvalidate/enum.h>
+#include <jvalidate/status.h>
+
+namespace jvalidate::schema {
+inline std::ostream & operator<<(std::ostream & os, Version version) {
+  switch (version) {
+  case Version::Draft04:
+    return os << "draft4";
+  case Version::Draft06:
+    return os << "draft6";
+  case Version::Draft07:
+    return os << "draft7";
+  case Version::Draft2019_09:
+    return os << "draft2019-09";
+  case Version::Draft2020_12:
+    return os << "draft2020-12";
+  }
+}
+}
+
+namespace jvalidate {
+inline std::ostream & operator<<(std::ostream & os, Status st) {
+  if (st == Status::Accept) {
+    return os << "Accept";
+  }
+  if (st == Status::Reject) {
+    return os << "Reject";
+  }
+  return os << "Noop";
+}
+}

+ 1 - 3
tests/selfvalidate_test.cxx

@@ -1,7 +1,4 @@
 
-#include "gmock/gmock.h"
-#include "gtest/gtest-param-test.h"
-#include "gtest/gtest.h"
 #include <cstdio>
 #include <cstdlib>
 #include <filesystem>
@@ -21,6 +18,7 @@
 #include <json/writer.h>
 
 #include "./json_schema_test_suite.h"
+#include "./printer_helper.h"
 
 using jvalidate::schema::Version;