Skip to content

Commit dc41f40

Browse files
simbo1905simbo1905
committed
wip
1 parent 219b7a6 commit dc41f40

File tree

4 files changed

+167
-58
lines changed

4 files changed

+167
-58
lines changed

‎README.md‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,6 @@ Per **RFC 8927 (JSON Typedef)**, the empty schema `{}` is the **empty form** and
308308
⚠️ Note: Some tools or in-house validators mistakenly interpret `{}` as "object with no
309309
properties allowed." **That is not JTD.** This implementation follows RFC 8927 strictly.
310310

311-
### Logging
312-
When a `{}` schema is compiled, the validator logs at **INFO** level:
313-
> `Empty schema {} encountered. Per RFC 8927 this means 'accept anything'. Some non-JTD validators interpret {} with object semantics; this implementation follows RFC 8927.`
314-
315311
```java
316312
import json.java21.jtd.Jtd;
317313
import jdk.sandbox.java.util.json.*;

‎json-java21-jtd/src/main/java/json/java21/jtd/Jtd.java‎

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -202,11 +202,22 @@ void pushChildFrames(Frame frame, java.util.Deque<Frame> stack) {
202202
}
203203
case JtdSchema.PropertiesSchema propsSchema -> {
204204
if (instance instanceof JsonObject obj) {
205-
// Push required properties that are present
205+
String discriminatorKey = frame.discriminatorKey();
206+
207+
// ================================= CHANGE 1: SKIP DISCRIMINATOR FIELD =================================
208+
// ADDED: Skip the discriminator field when pushing required property validation frames
209+
// Push required properties that are present (except discriminator field)
206210
for (var entry : propsSchema.properties().entrySet()) {
207211
String key = entry.getKey();
212+
213+
// Skip the discriminator field - it was already validated by discriminator logic
214+
if (discriminatorKey != null && key.equals(discriminatorKey)) {
215+
LOG.finer(() -> "Skipping discriminator field validation for: " + key);
216+
continue;
217+
}
218+
208219
JsonValue value = obj.members().get(key);
209-
220+
210221
if (value != null) {
211222
String childPtr = frame.ptr + "/" + key;
212223
Crumbs childCrumbs = frame.crumbs.withObjectField(key);
@@ -215,13 +226,21 @@ void pushChildFrames(Frame frame, java.util.Deque<Frame> stack) {
215226
LOG.finer(() -> "Pushed required property frame at " + childPtr);
216227
}
217228
}
218-
219-
// Push optional properties that are present
229+
230+
// ADDED: Skip the discriminator field when pushing optional property validation frames
231+
// Push optional properties that are present (except discriminator field)
220232
for (var entry : propsSchema.optionalProperties().entrySet()) {
221233
String key = entry.getKey();
234+
235+
// Skip the discriminator field - it was already validated by discriminator logic
236+
if (discriminatorKey != null && key.equals(discriminatorKey)) {
237+
LOG.finer(() -> "Skipping discriminator field validation for optional: " + key);
238+
continue;
239+
}
240+
222241
JtdSchema childSchema = entry.getValue();
223242
JsonValue value = obj.members().get(key);
224-
243+
225244
if (value != null) {
226245
String childPtr = frame.ptr + "/" + key;
227246
Crumbs childCrumbs = frame.crumbs.withObjectField(key);
@@ -230,6 +249,8 @@ void pushChildFrames(Frame frame, java.util.Deque<Frame> stack) {
230249
LOG.finer(() -> "Pushed optional property frame at " + childPtr);
231250
}
232251
}
252+
253+
// ============================= END CHANGE 1: SKIP DISCRIMINATOR FIELD =============================
233254
}
234255
}
235256
case JtdSchema.ValuesSchema valuesSchema -> {
@@ -252,15 +273,24 @@ void pushChildFrames(Frame frame, java.util.Deque<Frame> stack) {
252273
String discriminatorValueStr = discStr.value();
253274
JtdSchema variantSchema = discSchema.mapping().get(discriminatorValueStr);
254275
if (variantSchema != null) {
255-
// Special-case: skip pushing variant schema if object contains only discriminator key
256-
if (obj.members().size() == 1 && obj.members().containsKey(discSchema.discriminator())) {
257-
LOG.finer(() -> "Skipping variant schema push for discriminator-only object");
258-
} else {
259-
// Push variant schema for validation with discriminator key context
260-
Frame variantFrame = new Frame(variantSchema, instance, frame.ptr, frame.crumbs, discSchema.discriminator());
261-
stack.push(variantFrame);
262-
LOG.finer(() -> "Pushed discriminator variant frame for " + discriminatorValueStr + " with discriminator key: " + discSchema.discriminator());
263-
}
276+
277+
// ========================== CHANGE 2: REMOVE FAULTY OPTIMIZATION ==========================
278+
// REMOVED: Special-case optimization that skipped validation for discriminator-only objects
279+
// OLD CODE:
280+
// if (obj.members().size() == 1 && obj.members().containsKey(discSchema.discriminator())) {
281+
// LOG.finer(() -> "Skipping variant schema push for discriminator-only object");
282+
// } else {
283+
// Frame variantFrame = new Frame(variantSchema, instance, frame.ptr, frame.crumbs, discSchema.discriminator());
284+
// stack.push(variantFrame);
285+
// LOG.finer(() -> "Pushed discriminator variant frame for " + discriminatorValueStr + " with discriminator key: " + discSchema.discriminator());
286+
// }
287+
288+
// NEW CODE: Always push variant schema for validation with discriminator key context
289+
Frame variantFrame = new Frame(variantSchema, instance, frame.ptr, frame.crumbs, discSchema.discriminator());
290+
stack.push(variantFrame);
291+
LOG.finer(() -> "Pushed discriminator variant frame for " + discriminatorValueStr + " with discriminator key: " + discSchema.discriminator());
292+
// ======================== END CHANGE 2: REMOVE FAULTY OPTIMIZATION ========================
293+
264294
}
265295
}
266296
}
@@ -342,7 +372,7 @@ JtdSchema compileObjectSchema(JsonObject obj) {
342372

343373
// RFC 8927: {} is the empty form and accepts all instances
344374
if (forms.isEmpty() && obj.members().isEmpty()) {
345-
LOG.info(() -> "Empty schema {} encountered. Per RFC 8927 this means 'accept anything'. "
375+
LOG.finer(() -> "Empty schema {} encountered. Per RFC 8927 this means 'accept anything'. "
346376
+ "Some non-JTD validators interpret {} with object semantics; this implementation follows RFC 8927.");
347377
return new JtdSchema.EmptySchema();
348378
} else if (forms.isEmpty()) {
@@ -352,7 +382,7 @@ JtdSchema compileObjectSchema(JsonObject obj) {
352382

353383
if (!hasNonMetadataKeys) {
354384
// This is an empty schema (possibly with metadata)
355-
LOG.info(() -> "Empty schema encountered (with metadata: " + members.keySet() + "). "
385+
LOG.finer(() -> "Empty schema encountered (with metadata: " + members.keySet() + "). "
356386
+ "Per RFC 8927 this means 'accept anything'. "
357387
+ "Some non-JTD validators interpret {} with object semantics; this implementation follows RFC 8927.");
358388
return new JtdSchema.EmptySchema();

‎json-java21-jtd/src/test/java/json/java21/jtd/JtdExhaustiveTest.java‎

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -467,16 +467,69 @@ private static Arbitrary<JtdTestSchema> valuesSchemaArbitrary(int depth) {
467467
return jtdSchemaArbitrary(depth - 1)
468468
.map(ValuesSchema::new);
469469
}
470+
// ======================== NEW METHOD: SIMPLE PROPERTIES GENERATOR ========================
471+
/// Creates simple PropertiesSchema instances for discriminator mappings without recursion
472+
/// This prevents stack overflow while ensuring RFC 8927 compliance
473+
private static Arbitrary<JtdTestSchema> simplePropertiesSchemaArbitrary() {
474+
// Create primitive schemas that don't recurse
475+
final var primitiveSchemas = Arbitraries.of(
476+
new EmptySchema(),
477+
new TypeSchema("boolean"),
478+
new TypeSchema("string"),
479+
new TypeSchema("int32"),
480+
new EnumSchema(List.of("red", "green", "blue"))
481+
);
482+
483+
return Arbitraries.oneOf(
484+
// Empty properties schema
485+
Arbitraries.of(new PropertiesSchema(Map.of(), Map.of(), false)),
486+
487+
// Single required property with primitive schema
488+
Combinators.combine(
489+
Arbitraries.of(PROPERTY_NAMES),
490+
primitiveSchemas
491+
).as((name, schema) -> new PropertiesSchema(
492+
Map.of(name, schema),
493+
Map.of(),
494+
false
495+
)),
496+
497+
// Single optional property with primitive schema
498+
Combinators.combine(
499+
Arbitraries.of(PROPERTY_NAMES),
500+
primitiveSchemas
501+
).as((name, schema) -> new PropertiesSchema(
502+
Map.of(),
503+
Map.of(name, schema),
504+
false
505+
)),
470506

507+
// Required + optional property with primitive schemas
508+
Combinators.combine(
509+
Arbitraries.of(PROPERTY_PAIRS),
510+
primitiveSchemas,
511+
primitiveSchemas
512+
).as((names, requiredSchema, optionalSchema) -> new PropertiesSchema(
513+
Map.of(names.get(0), requiredSchema),
514+
Map.of(names.get(1), optionalSchema),
515+
false
516+
))
517+
);
518+
}
519+
// ====================== END NEW METHOD: SIMPLE PROPERTIES GENERATOR ======================
471520
private static Arbitrary<JtdTestSchema> discriminatorSchemaArbitrary(int depth) {
472521
final var childDepth = depth - 1;
473522

474523
return Combinators.combine(
475524
Arbitraries.of(PROPERTY_NAMES),
476525
Arbitraries.of(DISCRIMINATOR_VALUES),
477526
Arbitraries.of(DISCRIMINATOR_VALUES),
478-
jtdSchemaArbitrary(childDepth),
479-
jtdSchemaArbitrary(childDepth)
527+
// ======================== CHANGE: ONLY GENERATE PROPERTIES SCHEMAS ========================
528+
// RFC 8927 §2.4: discriminator mapping values must be properties form schemas
529+
// Generate only PropertiesSchema instead of arbitrary schemas
530+
simplePropertiesSchemaArbitrary(),
531+
simplePropertiesSchemaArbitrary()
532+
// ==================== END CHANGE: ONLY GENERATE PROPERTIES SCHEMAS ====================
480533
).as((discriminatorKey, value1, value2, schema1, schema2) -> {
481534
final var mapping = new LinkedHashMap<String, JtdTestSchema>();
482535
mapping.put(value1, schema1);
@@ -511,4 +564,4 @@ record PropertiesSchema(
511564
record ValuesSchema(JtdTestSchema values) implements JtdTestSchema {}
512565
record DiscriminatorSchema(String discriminator, Map<String, JtdTestSchema> mapping) implements JtdTestSchema {}
513566
record NullableSchema(JtdTestSchema schema) implements JtdTestSchema {}
514-
}
567+
}

‎json-java21-jtd/src/test/java/json/java21/jtd/TestRfc8927.java‎

Lines changed: 65 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -550,52 +550,82 @@ public void testAdditionalPropertiesDefaultsToFalse() throws Exception {
550550
.isNotEmpty();
551551
}
552552

553-
/// Test case from JtdExhaustiveTest property test failure
554-
/// Schema: {"elements":{"properties":{"alpha":{"discriminator":"alpha","mapping":{"type1":{"type":"boolean"}}}}}}
555-
/// Document: [{"alpha":{"alpha":"type1"}},{"alpha":{"alpha":"type1"}}]
556-
/// This should pass validation but currently fails with "expected boolean, got JsonObjectImpl"
553+
/// Test discriminator schema nested within elements schema (RFC 8927 compliant)
554+
/// Schema has array elements with discriminator properties that map to valid properties forms
557555
@Test
558556
public void testDiscriminatorInElementsSchema() throws Exception {
559557
JsonValue schema = Json.parse("""
560-
{
561-
"elements": {
562-
"properties": {
563-
"alpha": {
564-
"discriminator": "alpha",
565-
"mapping": {
566-
"type1": {"type": "boolean"}
558+
{
559+
"elements": {
560+
"properties": {
561+
"alpha": {
562+
"discriminator": "type",
563+
"mapping": {
564+
"config": {
565+
"properties": {
566+
"type": {},
567+
"value": {"type": "string"}
568+
},
569+
"additionalProperties": false
570+
},
571+
"flag": {
572+
"properties": {
573+
"type": {},
574+
"enabled": {"type": "boolean"}
575+
},
576+
"additionalProperties": false
567577
}
568578
}
569579
}
570-
}
580+
},
581+
"additionalProperties": false
571582
}
572-
""");
573-
JsonValue document = Json.parse("""
574-
[
575-
{"alpha": {"alpha": "type1"}},
576-
{"alpha": {"alpha": "type1"}}
577-
]
578-
""");
579-
580-
LOG.info(() -> "Testing discriminator in elements schema - property test failure case");
583+
}
584+
""");
585+
586+
JsonValue validDocument = Json.parse("""
587+
[
588+
{"alpha": {"type": "config", "value": "test"}},
589+
{"alpha": {"type": "flag", "enabled": true}}
590+
]
591+
""");
592+
593+
JsonValue invalidDocument = Json.parse("""
594+
[
595+
{"alpha": {"type": "config"}},
596+
{"alpha": {"type": "flag", "enabled": true}}
597+
]
598+
""");
599+
600+
LOG.info(() -> "Testing RFC 8927 compliant discriminator in elements schema");
581601
LOG.fine(() -> "Schema: " + schema);
582-
LOG.fine(() -> "Document: " + document);
583-
602+
LOG.fine(() -> "Valid document: " + validDocument);
603+
LOG.fine(() -> "Invalid document: " + invalidDocument);
604+
584605
Jtd validator = new Jtd();
585-
Jtd.Result result = validator.validate(schema, document);
586-
587-
LOG.fine(() -> "Validation result: " + (result.isValid() ? "VALID" : "INVALID"));
588-
if (!result.isValid()) {
589-
LOG.fine(() -> "Errors: " + result.errors());
606+
607+
// Valid case: all required properties present
608+
Jtd.Result validResult = validator.validate(schema, validDocument);
609+
LOG.fine(() -> "Valid validation result: " + (validResult.isValid() ? "VALID" : "INVALID"));
610+
if (!validResult.isValid()) {
611+
LOG.fine(() -> "Valid errors: " + validResult.errors());
590612
}
591-
592-
// This should be valid according to the property test expectation
593-
// but currently fails with "expected boolean, got JsonObjectImpl"
594-
assertThat(result.isValid())
595-
.as("Discriminator in elements schema should validate the property test case")
596-
.isTrue();
597-
}
598613

614+
assertThat(validResult.isValid())
615+
.as("RFC 8927 compliant discriminator in elements should validate correctly")
616+
.isTrue();
617+
618+
// Invalid case: missing required property in first element
619+
Jtd.Result invalidResult = validator.validate(schema, invalidDocument);
620+
LOG.fine(() -> "Invalid validation result: " + (invalidResult.isValid() ? "VALID" : "INVALID"));
621+
if (!invalidResult.isValid()) {
622+
LOG.fine(() -> "Invalid errors: " + invalidResult.errors());
623+
}
624+
625+
assertThat(invalidResult.isValid())
626+
.as("Should reject document with missing required properties")
627+
.isFalse();
628+
}
599629
/// Test case from JtdExhaustiveTest property test failure
600630
/// Nested elements containing properties schemas should reject additional properties
601631
/// Schema: {"elements":{"elements":{"properties":{}}}}

0 commit comments

Comments
 (0)