diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 258d977..bbe7e23 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,7 +39,7 @@ jobs: for k in totals: totals[k]+=int(r.get(k,'0')) except Exception: pass - exp_tests=463 + exp_tests=464 exp_skipped=0 if totals['tests']!=exp_tests or totals['skipped']!=exp_skipped: print(f"Unexpected test totals: {totals} != expected tests={exp_tests}, skipped={exp_skipped}") diff --git a/json-java21-jtd/src/main/java/json/java21/jtd/Jtd.java b/json-java21-jtd/src/main/java/json/java21/jtd/Jtd.java index d56949a..e5faf09 100644 --- a/json-java21-jtd/src/main/java/json/java21/jtd/Jtd.java +++ b/json-java21-jtd/src/main/java/json/java21/jtd/Jtd.java @@ -442,8 +442,8 @@ JtdSchema compilePropertiesSchema(JsonObject obj) { } additionalProperties = bool.value(); } else if (properties.isEmpty() && optionalProperties.isEmpty()) { - // Empty schema with no properties defined allows additional properties by default - additionalProperties = true; + // Empty schema with no properties defined rejects additional properties by default + additionalProperties = false; } return new JtdSchema.PropertiesSchema(properties, optionalProperties, additionalProperties); diff --git a/json-java21-jtd/src/test/java/json/java21/jtd/JtdExhaustiveTest.java.backup b/json-java21-jtd/src/test/java/json/java21/jtd/JtdExhaustiveTest.java.backup new file mode 100644 index 0000000..bfc0b40 --- /dev/null +++ b/json-java21-jtd/src/test/java/json/java21/jtd/JtdExhaustiveTest.java.backup @@ -0,0 +1,477 @@ +package json.java21.jtd; + +import jdk.sandbox.java.util.json.*; +import net.jqwik.api.*; +import net.jqwik.api.providers.ArbitraryProvider; +import net.jqwik.api.providers.TypeUsage; + +import java.math.BigDecimal; +import java.util.*; +import java.util.logging.Logger; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; + +/// Exhaustive property-based testing for JTD validator +/// Generates comprehensive schema/document permutations to validate RFC 8927 compliance +class JtdExhaustiveTest extends JtdTestBase { + + private static final int MAX_DEPTH = 3; + private static final List PROPERTY_NAMES = List.of("alpha", "beta", "gamma", "delta", "epsilon"); + private static final List> PROPERTY_PAIRS = List.of( + List.of("alpha", "beta"), + List.of("alpha", "gamma"), + List.of("beta", "delta"), + List.of("gamma", "epsilon") + ); + private static final List DISCRIMINATOR_VALUES = List.of("type1", "type2", "type3"); + private static final List ENUM_VALUES = List.of("red", "green", "blue", "yellow"); + + @Provide + Arbitrary jtdSchemas() { + return jtdSchemaArbitrary(MAX_DEPTH); + } + + @Property(tries = 100) + void exhaustiveJtdValidation(@ForAll("jtdSchemas") JtdExhaustiveTest.JtdTestSchema schema) { + LOG.info(() -> "Executing exhaustiveJtdValidation property test"); + + final var schemaDescription = describeJtdSchema(schema); + + // Skip problematic schema combinations that create validation issues + if (schemaDescription.contains("elements[discriminator[") && schemaDescription.contains("type=")) { + LOG.fine(() -> "Skipping problematic schema combination: " + schemaDescription); + return; // Skip this test case + } + + LOG.fine(() -> "JTD schema descriptor: " + schemaDescription); + + final var schemaJson = jtdSchemaToJsonObject(schema); + LOG.fine(() -> "JTD schema JSON: " + schemaJson); + + final var validator = new Jtd(); + + final var compliantDocument = buildCompliantJtdDocument(schema); + LOG.fine(() -> "Compliant JTD document: " + compliantDocument); + + final var validationResult = validator.validate(schemaJson, compliantDocument); + + if (!validationResult.isValid()) { + LOG.severe(() -> String.format("ERROR: Compliant document failed validation!%nSchema: %s%nDocument: %s%nErrors: %s", + schemaJson, compliantDocument, validationResult.errors())); + } + + assertThat(validationResult.isValid()) + .as("Compliant JTD document should validate for schema %s", schemaDescription) + .isTrue(); + assertThat(validationResult.errors()) + .as("No validation errors expected for compliant JTD document") + .isEmpty(); + + final var failingDocuments = createFailingJtdDocuments(schema, compliantDocument); + + // Empty schema accepts everything, so no failing documents are expected + // Nullable schema also accepts null, so may have limited failing cases + if (!(schema instanceof EmptySchema) && !(schema instanceof NullableSchema)) { + assertThat(failingDocuments) + .as("Negative cases should be generated for JTD schema %s", schemaDescription) + .isNotEmpty(); + } + + final var failingDocumentStrings = failingDocuments.stream() + .map(Object::toString) + .toList(); + LOG.finest(() -> "Failing JTD documents: " + failingDocumentStrings); + + failingDocuments.forEach(failing -> { + final var failingResult = validator.validate(schemaJson, failing); + assertThat(failingResult.isValid()) + .as("Expected JTD validation failure for %s against schema %s", failing, schemaDescription) + .isFalse(); + assertThat(failingResult.errors()) + .as("Expected JTD validation errors for %s against schema %s", failing, schemaDescription) + .isNotEmpty(); + }); + } + + private static JsonValue buildCompliantJtdDocument(JtdTestSchema schema) { + return switch (schema) { + case EmptySchema() -> JsonString.of("any value works"); + case RefSchema(var ref) -> JsonString.of("ref-compliant-value"); + case TypeSchema(var type) -> buildCompliantTypeValue(type); + case EnumSchema(var values) -> JsonString.of(values.getFirst()); + case ElementsSchema(var elementSchema) -> JsonArray.of(List.of( + buildCompliantJtdDocument(elementSchema), + buildCompliantJtdDocument(elementSchema) + )); + case PropertiesSchema(var required, var optional, var additional) -> { + final var members = new LinkedHashMap(); + required.forEach((key, valueSchema) -> + members.put(key, buildCompliantJtdDocument(valueSchema)) + ); + optional.forEach((key, valueSchema) -> + members.put(key, buildCompliantJtdDocument(valueSchema)) + ); + yield JsonObject.of(members); + } + case ValuesSchema(var valueSchema) -> JsonObject.of(Map.of( + "key1", buildCompliantJtdDocument(valueSchema), + "key2", buildCompliantJtdDocument(valueSchema) + )); + case DiscriminatorSchema(var discriminator, var mapping) -> { + final var firstEntry = mapping.entrySet().iterator().next(); + final var discriminatorValue = firstEntry.getKey(); + final var variantSchema = firstEntry.getValue(); + + // Discriminator schemas always generate objects with the discriminator field + final var members = new LinkedHashMap(); + members.put(discriminator, JsonString.of(discriminatorValue)); + + // Add properties based on the variant schema type + if (variantSchema instanceof PropertiesSchema props) { + props.properties().forEach((key, valueSchema) -> + members.put(key, buildCompliantJtdDocument(valueSchema)) + ); + } + // For TypeSchema variants, the object with just the discriminator field should be valid + // For EnumSchema variants, same logic applies + + yield JsonObject.of(members); + } + case NullableSchema(var inner) -> JsonNull.of(); + }; + } + + private static JsonValue buildCompliantTypeValue(String type) { + return switch (type) { + case "boolean" -> JsonBoolean.of(true); + case "string" -> JsonString.of("compliant-string"); + case "timestamp" -> JsonString.of("2023-12-25T10:30:00Z"); + case "int8" -> JsonNumber.of(42); + case "uint8" -> JsonNumber.of(200); + case "int16" -> JsonNumber.of(30000); + case "uint16" -> JsonNumber.of(50000); + case "int32" -> JsonNumber.of(1000000); + case "uint32" -> JsonNumber.of(3000000000L); + case "float32" -> JsonNumber.of(new BigDecimal("3.14159")); + case "float64" -> JsonNumber.of(new BigDecimal("3.14159")); + default -> JsonString.of("unknown-type-value"); + }; + } + + private static List createFailingJtdDocuments(JtdTestSchema schema, JsonValue compliant) { + return switch (schema) { + case EmptySchema unused -> List.of(); // Empty schema accepts everything + case RefSchema unused -> List.of(JsonNull.of()); // Ref should fail on null + case TypeSchema(var type) -> createFailingTypeValues(type); + case EnumSchema(var values) -> List.of(JsonString.of("invalid-enum-value")); + case ElementsSchema(var elementSchema) -> { + if (compliant instanceof JsonArray arr && !arr.values().isEmpty()) { + final var invalidElement = createFailingJtdDocuments(elementSchema, arr.values().getFirst()); + if (!invalidElement.isEmpty()) { + final var mixedArray = JsonArray.of(List.of( + arr.values().getFirst(), + invalidElement.getFirst() + )); + yield List.of(mixedArray, JsonNull.of()); + } + } + yield List.of(JsonNull.of()); + } + case PropertiesSchema(var required, var optional, var additional) -> { + final var failures = new ArrayList(); + if (!required.isEmpty()) { + final var firstKey = required.keySet().iterator().next(); + failures.add(removeProperty((JsonObject) compliant, firstKey)); + } + if (!additional) { + failures.add(addExtraProperty((JsonObject) compliant, "extraProperty")); + } + failures.add(JsonNull.of()); + yield failures; + } + case ValuesSchema unused -> List.of(JsonNull.of(), JsonString.of("not-an-object")); + case DiscriminatorSchema(var discriminator, var mapping) -> { + final var failures = new ArrayList(); + failures.add(replaceDiscriminatorValue((JsonObject) compliant, "invalid-discriminator")); + failures.add(JsonNull.of()); + yield failures; + } + case NullableSchema unused -> List.of(); // Nullable accepts null + }; + } + + private static List createFailingTypeValues(String type) { + return switch (type) { + case "boolean" -> List.of(JsonString.of("not-boolean"), JsonNumber.of(1)); + case "string", "timestamp" -> List.of(JsonNumber.of(123), JsonBoolean.of(false)); + case "int8" -> List.of(JsonString.of("not-integer"), JsonNumber.of(new BigDecimal("3.14"))); + case "uint8" -> List.of(JsonString.of("not-integer"), JsonNumber.of(new BigDecimal("3.14"))); + case "int16" -> List.of(JsonString.of("not-integer"), JsonNumber.of(new BigDecimal("3.14"))); + case "uint16" -> List.of(JsonString.of("not-integer"), JsonNumber.of(new BigDecimal("3.14"))); + case "int32" -> List.of(JsonString.of("not-integer"), JsonNumber.of(new BigDecimal("3.14"))); + case "uint32" -> List.of(JsonString.of("not-integer"), JsonNumber.of(new BigDecimal("3.14"))); + case "float32" -> List.of(JsonString.of("not-float"), JsonBoolean.of(true)); + case "float64" -> List.of(JsonString.of("not-float"), JsonBoolean.of(true)); + default -> List.of(JsonNull.of()); + }; + } + + private static JsonObject removeProperty(JsonObject original, String missingProperty) { + final var filtered = original.members().entrySet().stream() + .filter(entry -> !Objects.equals(entry.getKey(), missingProperty)) + .collect(Collectors.toMap( + Map.Entry::getKey, + Map.Entry::getValue, + (left, right) -> left, + LinkedHashMap::new + )); + return JsonObject.of(filtered); + } + + private static JsonObject addExtraProperty(JsonObject original, String extraProperty) { + final var extended = new LinkedHashMap<>(original.members()); + extended.put(extraProperty, JsonString.of("extra-value")); + return JsonObject.of(extended); + } + + private static JsonValue replaceDiscriminatorValue(JsonObject original, String newValue) { + final var modified = new LinkedHashMap<>(original.members()); + // Find and replace discriminator field + for (var entry : modified.entrySet()) { + if (entry.getValue() instanceof JsonString) { + modified.put(entry.getKey(), JsonString.of(newValue)); + break; + } + } + return JsonObject.of(modified); + } + + private static JsonObject jtdSchemaToJsonObject(JtdTestSchema schema) { + return switch (schema) { + case EmptySchema() -> JsonObject.of(Map.of()); + case RefSchema(var ref) -> JsonObject.of(Map.of("ref", JsonString.of(ref))); + case TypeSchema(var type) -> JsonObject.of(Map.of("type", JsonString.of(type))); + case EnumSchema(var values) -> JsonObject.of(Map.of( + "enum", JsonArray.of(values.stream().map(JsonString::of).toList()) + )); + case ElementsSchema(var elementSchema) -> JsonObject.of(Map.of( + "elements", jtdSchemaToJsonObject(elementSchema) + )); + case PropertiesSchema(var required, var optional, var additional) -> { + final var schemaMap = new LinkedHashMap(); + if (!required.isEmpty()) { + schemaMap.put("properties", JsonObject.of( + required.entrySet().stream() + .collect(Collectors.toMap( + Map.Entry::getKey, + entry -> jtdSchemaToJsonObject(entry.getValue()) + )) + )); + } + if (!optional.isEmpty()) { + schemaMap.put("optionalProperties", JsonObject.of( + optional.entrySet().stream() + .collect(Collectors.toMap( + Map.Entry::getKey, + entry -> jtdSchemaToJsonObject(entry.getValue()) + )) + )); + } + if (additional) { + schemaMap.put("additionalProperties", JsonBoolean.of(true)); + } + yield JsonObject.of(schemaMap); + } + case ValuesSchema(var valueSchema) -> JsonObject.of(Map.of( + "values", jtdSchemaToJsonObject(valueSchema) + )); + case DiscriminatorSchema(var discriminator, var mapping) -> { + final var schemaMap = new LinkedHashMap(); + schemaMap.put("discriminator", JsonString.of(discriminator)); + schemaMap.put("mapping", JsonObject.of( + mapping.entrySet().stream() + .collect(Collectors.toMap( + Map.Entry::getKey, + entry -> jtdSchemaToJsonObject(entry.getValue()) + )) + )); + yield JsonObject.of(schemaMap); + } + case NullableSchema(var inner) -> { + final var innerSchema = jtdSchemaToJsonObject(inner); + final var nullableMap = new LinkedHashMap<>(innerSchema.members()); + nullableMap.put("nullable", JsonBoolean.of(true)); + yield JsonObject.of(nullableMap); + } + }; + } + + private static String describeJtdSchema(JtdTestSchema schema) { + return switch (schema) { + case EmptySchema() -> "empty"; + case RefSchema(var ref) -> "ref:" + ref; + case TypeSchema(var type) -> "type:" + type; + case EnumSchema(var values) -> "enum[" + String.join(",", values) + "]"; + case ElementsSchema(var elementSchema) -> "elements[" + describeJtdSchema(elementSchema) + "]"; + case PropertiesSchema(var required, var optional, var additional) -> { + final var parts = new ArrayList(); + if (!required.isEmpty()) { + parts.add("required{" + String.join(",", required.keySet()) + "}"); + } + if (!optional.isEmpty()) { + parts.add("optional{" + String.join(",", optional.keySet()) + "}"); + } + if (additional) { + parts.add("additional"); + } + yield "properties[" + String.join(",", parts) + "]"; + } + case ValuesSchema(var valueSchema) -> "values[" + describeJtdSchema(valueSchema) + "]"; + case DiscriminatorSchema(var discriminator, var mapping) -> + "discriminator[" + discriminator + "→{" + String.join(",", mapping.keySet()) + "}]"; + case NullableSchema(var inner) -> "nullable[" + describeJtdSchema(inner) + "]"; + }; + } + + /// Custom arbitrary provider for JTD test schemas + static final class JtdSchemaArbitraryProvider implements ArbitraryProvider { + @Override + public boolean canProvideFor(TypeUsage targetType) { + return targetType.isOfType(JtdExhaustiveTest.JtdTestSchema.class); + } + + @Override + public Set> provideFor(TypeUsage targetType, SubtypeProvider subtypeProvider) { + return Set.of(jtdSchemaArbitrary(MAX_DEPTH)); + } + } + + @SuppressWarnings("unchecked") + private static Arbitrary jtdSchemaArbitrary(int depth) { + final var primitives = Arbitraries.of( + new EmptySchema(), + new TypeSchema("boolean"), + new TypeSchema("string"), + new TypeSchema("int32"), + new TypeSchema("float64"), + new TypeSchema("timestamp") + ); + + if (depth == 0) { + return (Arbitrary) (Arbitrary) primitives; + } + + return (Arbitrary) (Arbitrary) Arbitraries.oneOf( + primitives, + enumSchemaArbitrary(), + elementsSchemaArbitrary(depth), + propertiesSchemaArbitrary(depth), + valuesSchemaArbitrary(depth), + discriminatorSchemaArbitrary(depth), + nullableSchemaArbitrary(depth) + ); + } + + private static Arbitrary enumSchemaArbitrary() { + return Arbitraries.of(ENUM_VALUES) + .list().ofMinSize(1).ofMaxSize(4) + .map(values -> new EnumSchema(new ArrayList<>(values))); + } + + private static Arbitrary elementsSchemaArbitrary(int depth) { + // Avoid generating ElementsSchema with DiscriminatorSchema that maps to simple types + // This creates validation issues as discriminator objects won't match simple type schemas + return jtdSchemaArbitrary(depth - 1) + .filter(schema -> { + // Filter out problematic combinations + if (schema instanceof DiscriminatorSchema disc) { + // Avoid discriminator mapping to simple types when used in elements + var firstVariant = disc.mapping().values().iterator().next(); + return !(firstVariant instanceof TypeSchema) && !(firstVariant instanceof EnumSchema); + } + return true; + }) + .map(ElementsSchema::new); + } + + private static Arbitrary propertiesSchemaArbitrary(int depth) { + final var childDepth = depth - 1; + + final var empty = Arbitraries.of(new PropertiesSchema(Map.of(), Map.of(), false)); + + final var singleRequired = Combinators.combine( + Arbitraries.of(PROPERTY_NAMES), + jtdSchemaArbitrary(childDepth) + ).as((name, schema) -> new PropertiesSchema( + Map.of(name, schema), + Map.of(), + false + )); + + final var mixed = Combinators.combine( + Arbitraries.of(PROPERTY_PAIRS), + jtdSchemaArbitrary(childDepth), + jtdSchemaArbitrary(childDepth) + ).as((names, requiredSchema, optionalSchema) -> new PropertiesSchema( + Map.of(names.getFirst(), requiredSchema), + Map.of(names.getLast(), optionalSchema), + false + )); + + final var withAdditional = mixed.map(props -> + new PropertiesSchema(props.properties(), props.optionalProperties(), true) + ); + + return Arbitraries.oneOf(empty, singleRequired, mixed, withAdditional); + } + + private static Arbitrary valuesSchemaArbitrary(int depth) { + return jtdSchemaArbitrary(depth - 1) + .map(ValuesSchema::new); + } + + private static Arbitrary discriminatorSchemaArbitrary(int depth) { + final var childDepth = depth - 1; + + return Combinators.combine( + Arbitraries.of(PROPERTY_NAMES), + Arbitraries.of(DISCRIMINATOR_VALUES), + Arbitraries.of(DISCRIMINATOR_VALUES), + jtdSchemaArbitrary(childDepth), + jtdSchemaArbitrary(childDepth) + ).as((discriminatorKey, value1, value2, schema1, schema2) -> { + final var mapping = new LinkedHashMap(); + mapping.put(value1, schema1); + if (!value1.equals(value2)) { + mapping.put(value2, schema2); + } + return new DiscriminatorSchema(discriminatorKey, mapping); + }); + } + + private static Arbitrary nullableSchemaArbitrary(int depth) { + return jtdSchemaArbitrary(depth - 1) + .map(NullableSchema::new); + } + + /// Sealed interface for JTD test schemas + sealed interface JtdTestSchema permits + EmptySchema, RefSchema, TypeSchema, EnumSchema, + ElementsSchema, PropertiesSchema, ValuesSchema, + DiscriminatorSchema, NullableSchema {} + + record EmptySchema() implements JtdTestSchema {} + record RefSchema(String ref) implements JtdTestSchema {} + record TypeSchema(String type) implements JtdTestSchema {} + record EnumSchema(List values) implements JtdTestSchema {} + record ElementsSchema(JtdTestSchema elements) implements JtdTestSchema {} + record PropertiesSchema( + Map properties, + Map optionalProperties, + boolean additionalProperties + ) implements JtdTestSchema {} + record ValuesSchema(JtdTestSchema values) implements JtdTestSchema {} + record DiscriminatorSchema(String discriminator, Map mapping) implements JtdTestSchema {} + record NullableSchema(JtdTestSchema schema) implements JtdTestSchema {} +} \ No newline at end of file diff --git a/json-java21-jtd/src/test/java/json/java21/jtd/TestRfc8927.java b/json-java21-jtd/src/test/java/json/java21/jtd/TestRfc8927.java index 3c9b186..450cf8f 100644 --- a/json-java21-jtd/src/test/java/json/java21/jtd/TestRfc8927.java +++ b/json-java21-jtd/src/test/java/json/java21/jtd/TestRfc8927.java @@ -530,4 +530,23 @@ public void testIntegerTypesRejectFractionalComponents() throws Exception { .isNotEmpty(); } } + + /// Test for Issue #91: additionalProperties should default to false when no properties defined + /// Empty properties schema should reject additional properties + @Test + public void testAdditionalPropertiesDefaultsToFalse() throws Exception { + JsonValue schema = Json.parse("{\"elements\": {\"properties\": {}}}"); + JsonValue invalidData = Json.parse("[{\"extraProperty\":\"extra-value\"}]"); + + Jtd validator = new Jtd(); + Jtd.Result result = validator.validate(schema, invalidData); + + // This should fail validation because additionalProperties defaults to false + assertThat(result.isValid()) + .as("Empty properties schema should reject additional properties by default") + .isFalse(); + assertThat(result.errors()) + .as("Should have validation error for additional property") + .isNotEmpty(); + } }