Skip to content

Conversation

@simbo1905
Copy link
Owner

rename and tidy

- Added BigDecimal fractional part checking in validateInteger method
- Added test case testInt32RejectsDecimal() to verify the fix
- Ensures all integer types (int8, uint8, int16, uint16, int32, uint32) reject decimal values
- Maintains RFC 8927 compliance for integer type validation
- Fixed JTD validator to correctly default additionalProperties to false when no properties are defined
- Added test case testAdditionalPropertiesDefaultsToFalse() to verify the fix
- Updated CI test count from 463 to 464 to account for new test
- This ensures RFC 8927 compliance where empty properties schemas reject additional properties by default

The bug was in Jtd.java line 446 where additionalProperties was set to true instead of false
when both properties and optionalProperties were empty. This caused empty schemas to incorrectly
allow additional properties instead of rejecting them by default.

Closes #91
- Removed JtdExhaustiveTest.java from this PR to separate property test development from the bug fix
- Backed up the property test as JtdExhaustiveTest.java.backup for future development
- Updated CI test count from 464 back to 463 to reflect removal of property test
- This allows the additionalProperties bug fix (Issue #91) to be merged independently
- The property test can be restored and continued separately after merge
- Added special-case handling in pushChildFrames to skip pushing variant schema
  when discriminator object contains only the discriminator key
- This fixes validation failures when discriminator maps to simple types like boolean
- Preserves RFC 8927 semantics while handling property-test conventions
- Test case testDiscriminatorInElementsSchema now passes

The fix addresses the bug where discriminator objects like {alpha:type1}
were incorrectly validated against simple type schemas like {type:boolean},
causing "expected boolean, got JsonObjectImpl" errors.
- Updated CI test count from 464 to 465 to reflect new discriminator test
- Added instructions for creating PRs with GitHub CLI to AGENTS.md
- Includes guidance on avoiding special characters in titles and using body-file flag
- Added testNestedElementsPropertiesRejectsAdditionalProperties() to TestRfc8927
- Tests nested elements containing properties schemas with empty properties
- Verifies that additional properties are correctly rejected by default
- Reproduces the exact failing case found by JtdExhaustiveTest property testing
- Schema: {elements:{elements:{properties:{}}}}
- Document: [[{},{},[{},{extraProperty:extra-value}]]
@chatgpt-codex-connector
Copy link

💡 Codex Review

case JtdSchema.DiscriminatorSchema discSchema -> {
if (instance instanceof JsonObject obj) {
JsonValue discriminatorValue = obj.members().get(discSchema.discriminator());
if (discriminatorValue instanceof JsonString discStr) {
String discriminatorValueStr = discStr.value();
JtdSchema variantSchema = discSchema.mapping().get(discriminatorValueStr);
if (variantSchema != null) {
Frame variantFrame = new Frame(variantSchema, instance, frame.ptr(), frame.crumbs(), discSchema.discriminator());
stack.push(variantFrame);

[P1] Avoid validating discriminator variant when object only holds tag

The previous code skipped pushing a variant frame when a discriminator object contained only the discriminator field. That allowed schemas that map the discriminator to simple types (e.g. { "discriminator": "kind", "mapping": { "bool": { "type": "boolean" }}}) to treat { "kind": "bool" } as valid, matching the special‑case in DiscriminatorSchema.validate. The new implementation always pushes the variant frame, so the boolean TypeSchema will now validate against the whole object and report expected boolean, got JsonObject, turning formerly valid inputs into failures. Either restore the guard before enqueuing the variant frame or remove the special‑case in DiscriminatorSchema.validate so both paths agree.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

@simbo1905 simbo1905 merged commit 22a6396 into main Sep 28, 2025
4 checks passed
@simbo1905 simbo1905 deleted the feature/jqwik-property-testing-88 branch September 28, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants