From 6b684b3a538cc0eac8e9ea8c5d29b16119fde10b Mon Sep 17 00:00:00 2001
From: Juan Cruz Viotti <jv@jviotti.com>
Date: Fri, 22 Nov 2024 14:57:47 -0400
Subject: [PATCH] Unroll closed `properties` if all are required (#224)

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
---
 src/compiler/default_compiler_draft4.h  |  7 ++++-
 test/evaluator/evaluator_draft4_test.cc | 41 +++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/compiler/default_compiler_draft4.h b/src/compiler/default_compiler_draft4.h
index 4d9b6f54..1a60e06f 100644
--- a/src/compiler/default_compiler_draft4.h
+++ b/src/compiler/default_compiler_draft4.h
@@ -640,7 +640,9 @@ auto properties_as_loop(const Context &context,
   if (!inside_disjunctor &&
       schema_context.schema.defines("additionalProperties") &&
       schema_context.schema.at("additionalProperties").is_boolean() &&
-      !schema_context.schema.at("additionalProperties").to_boolean()) {
+      !schema_context.schema.at("additionalProperties").to_boolean() &&
+      // If all properties are required, we should still unroll
+      required.size() < size) {
     return true;
   }
 
@@ -973,6 +975,9 @@ auto compiler_draft4_applicator_additionalproperties_with_options(
     if (properties_as_loop(context, schema_context,
                            schema_context.schema.at("properties"))) {
       return {};
+    } else if (!children.empty() &&
+               std::holds_alternative<AssertionFail>(children.front())) {
+      return {};
     } else {
       return {make<LoopPropertiesWhitelist>(
           context, schema_context, dynamic_context, std::move(filter_strings))};
diff --git a/test/evaluator/evaluator_draft4_test.cc b/test/evaluator/evaluator_draft4_test.cc
index 5ba61f36..af2b4237 100644
--- a/test/evaluator/evaluator_draft4_test.cc
+++ b/test/evaluator/evaluator_draft4_test.cc
@@ -2766,6 +2766,47 @@ TEST(Evaluator_draft4, additionalProperties_11) {
       "The object value was not expected to define additional properties");
 }
 
+TEST(Evaluator_draft4, additionalProperties_12) {
+  const sourcemeta::jsontoolkit::JSON schema{
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "required": [ "foo", "bar" ],
+    "additionalProperties": false,
+    "properties": {
+      "foo": { "type": "boolean" },
+      "bar": { "type": "boolean" }
+    }
+  })JSON")};
+
+  const sourcemeta::jsontoolkit::JSON instance{
+      sourcemeta::jsontoolkit::parse("{ \"foo\": true, \"bar\": false }")};
+
+  EVALUATE_WITH_TRACE_FAST_SUCCESS(schema, instance, 3);
+
+  EVALUATE_TRACE_PRE(0, AssertionDefinesAll, "/required", "#/required", "");
+  EVALUATE_TRACE_PRE(1, AssertionPropertyTypeStrict, "/properties/bar/type",
+                     "#/properties/bar/type", "/bar");
+  EVALUATE_TRACE_PRE(2, AssertionPropertyTypeStrict, "/properties/foo/type",
+                     "#/properties/foo/type", "/foo");
+
+  EVALUATE_TRACE_POST_SUCCESS(0, AssertionDefinesAll, "/required", "#/required",
+                              "");
+  EVALUATE_TRACE_POST_SUCCESS(1, AssertionPropertyTypeStrict,
+                              "/properties/bar/type", "#/properties/bar/type",
+                              "/bar");
+  EVALUATE_TRACE_POST_SUCCESS(2, AssertionPropertyTypeStrict,
+                              "/properties/foo/type", "#/properties/foo/type",
+                              "/foo");
+
+  EVALUATE_TRACE_POST_DESCRIBE(instance, 0,
+                               "The object value was expected to define "
+                               "properties \"foo\", and \"bar\"");
+  EVALUATE_TRACE_POST_DESCRIBE(instance, 1,
+                               "The value was expected to be of type boolean");
+  EVALUATE_TRACE_POST_DESCRIBE(instance, 2,
+                               "The value was expected to be of type boolean");
+}
+
 TEST(Evaluator_draft4, not_1) {
   const sourcemeta::jsontoolkit::JSON schema{
       sourcemeta::jsontoolkit::parse(R"JSON({