Browse Source

test/refactor: modify handling of oneOf to only log the matching when a success occurs (and therefore to unannotate the failures)

Sam Jaffe 2 weeks ago
parent
commit
6f5a4a44cf

+ 11 - 0
include/jvalidate/validation_result.h

@@ -301,6 +301,17 @@ private:
     results_[where][schema_path].annotations.emplace(name, std::move(message));
   }
 
+  /**
+   * @brief Remove annotations that we added without knowing if they were
+   * needed, now that we know that they are not required.
+   *
+   * @param where A path into the document being validated
+   * @param schema_path The schema path to remove
+   */
+  void unannotate(detail::Pointer const & where, detail::Pointer const & schema_path) {
+    results_[where].erase(schema_path);
+  }
+
   static void sanitize(std::string & message) {
     for (auto it = message.begin(); it != message.end(); ++it) {
       if (*it == '"' || *it == '\\') {

+ 13 - 3
include/jvalidate/validation_visitor.h

@@ -189,10 +189,20 @@ public:
       }
     }
 
-    if (matches.size() == 1) {
-      return result(Status::Accept, "validates subschema ", *matches.begin());
+    if (matches.empty()) {
+      return result(Status::Reject, "validates no subschemas");
     }
-    return result(Status::Reject, "validates multiple subschemas ", matches);
+    if (matches.size() > 1) {
+      return result(Status::Reject, "validates multiple subschemas ", matches);
+    }
+
+    size_t const match = *matches.begin();
+    for (size_t i = 0; result_ and i < cons.children.size(); ++i) {
+      if (i != match) {
+        result_->unannotate(where_, schema_path_ / i);
+      }
+    }
+    return result(Status::Accept, "validates subschema ", match);
   }
 
   Status visit(constraint::NotConstraint const & cons, Adapter auto const & document) const {

+ 55 - 17
tests/annotation_test.cxx

@@ -104,23 +104,6 @@ TEST(Annotation, PathFollowsSchemaNotConstraintModel) {
   EXPECT_THAT(result, ErrorAt("disallow", "string is in types [string]"));
 }
 
-TEST(Annotation, SomeConstraintsAnnotateBothValidAndInvalid) {
-  auto const schema = R"({
-    "$comment": "accepts any number <= 0 or >= 10",
-    "oneOf": [
-      { "minimum": 10 },
-      { "maximum": 0 }
-    ]
-  })"_json;
-
-  auto const instance = R"(-1)"_json;
-  jvalidate::ValidationResult result = validate(schema, instance);
-
-  EXPECT_THAT(result, Not(HasAnnotationAt(""_jptr, "/oneOf"_jptr)));
-  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/0"_jptr, "minimum", "-1 < 10"));
-  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/1"_jptr, "maximum", "-1 <= 0"));
-}
-
 TEST(Annotation, AttachesAlwaysFalseSensibly) {
   auto const schema = R"({
     "properties": {
@@ -204,6 +187,61 @@ TEST(Annotation, IfElseCanGiveABecauseReason) {
                               "4 is not a multiple of 5"));
 }
 
+TEST(Annotation, OneOfGivesAllReasonsOnTooMany) {
+  auto const schema = R"({
+    "oneOf": [
+      { "minimum": 5 },
+      { "multipleOf": 3 },
+      { "maximum": 2 }
+    ]
+  })"_json;
+
+  auto const instance = R"(6)"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, ErrorAt(""_jptr, ""_jptr, "oneOf", "validates multiple subschemas [ 0, 1 ]"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/0"_jptr, "minimum", "6 >= 5"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/1"_jptr, "multipleOf", "6 is a multiple of 3"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/2"_jptr, "maximum", "6 > 2"));
+}
+
+TEST(Annotation, OneOfGivesAllReasonsOnNoMatches) {
+  auto const schema = R"({
+    "oneOf": [
+      { "minimum": 5 },
+      { "multipleOf": 3 },
+      { "maximum": 2 }
+    ]
+  })"_json;
+
+  auto const instance = R"(4)"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, ErrorAt(""_jptr, ""_jptr, "oneOf", "validates no subschemas"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/0"_jptr, "minimum", "4 < 5"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/1"_jptr, "multipleOf", "4 is not a multiple of 3"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/2"_jptr, "maximum", "4 > 2"));
+}
+
+TEST(Annotation, OneOfGivesAllReasonsOnMatch) {
+  auto const schema = R"({
+    "oneOf": [
+      { "minimum": 5 },
+      { "multipleOf": 3 },
+      { "maximum": 2 }
+    ]
+  })"_json;
+
+  auto const instance = R"(3)"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, Not(HasAnnotationAt(""_jptr, ""_jptr)));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/1"_jptr, "multipleOf", "3 is a multiple of 3"));
+}
+
 int main(int argc, char ** argv) {
   testing::InitGoogleMock(&argc, argv);
   return RUN_ALL_TESTS();