Repository navigation
Fix and test experimental vmf-jackson (de)serialization and schema generation - #81
Merged
Merged
Conversation
…neration Serialization/deserialization: - deserialization no longer silently drops fields: a builder call that fails now aborts with a clear error instead of printStackTrace()+continue, which previously caused hard-to-diagnose data loss. - guard a fragile Optional.get() in VMFTypeUtils.isParentOfPropContainer. JSON schema generation: - remove a leftover stdout debug block (printed on any "Condition"-named type). - emit a valid draft-07 polymorphic discriminator: the @vmf-type const now lives in an allOf branch instead of as a $ref sibling (draft-07 ignores keywords next to $ref, so the discriminator was being silently dropped). - resolve subtypes for @InterfaceOnly element types (e.g. Update[] -> StateUpdate) by scanning the model's full type list by name; interface-only types cannot reflect on themselves, which is why "extending types" did not work in schemas. - deterministic output (LinkedHashMap) and emitted propertyOrder preserving VMF declaration order. Cleanup: - delete dead code (two commented-out Main.java), the unused application-plugin config and the now-unused victools test dependency. Tests (8 classes / 18 tests, all green): - rewrite JsonSchemaModelTest, FormatModelTest and SchemaTest01 from print-only into real assertions covering the allOf discriminator, propertyOrder, defaults, numeric constraints, format/inject/uniqueItems, enum handling and interface-only polymorphism. - fix swallowed catch blocks in round-trip tests, convert bare assert to JUnit and verify map contents survive the round-trip, add a rename assertion, and drop SchemaTest01's working-directory file-write side effect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the experimental vmf-jackson module by making deserialization fail fast (avoiding silent data loss) and by fixing JSON Schema draft-07 generation for polymorphic models (notably discriminator placement and interface-only subtype resolution). It also replaces prior print-only schema tests with assertion-based coverage and removes unused/dead Gradle + playground code.
Changes:
- Make jackson deserialization propagate field-level failures instead of swallowing exceptions.
- Fix JSON Schema draft-07 polymorphism output (
oneOf+allOfdiscriminator), interface-only subtype discovery, and make schema output deterministic (LinkedHashMap,propertyOrder). - Convert multiple schema/round-trip tests from print-only to assertion-based checks; remove dead code and unused dependencies.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| jackson/src/test/java/eu/mihosoft/vmf/jackson/test/type_formats01/FormatModelTest.java | Replaces print-only schema generation with assertions for formats/defaults/injected fragments/enum handling. |
| jackson/src/test/java/eu/mihosoft/vmf/jackson/test/story_graph_01/json_spec_test_01/SchemaTest01.java | Adds assertions for interface-only polymorphism and recursion in generated schemas. |
| jackson/src/test/java/eu/mihosoft/vmf/jackson/test/simple/PolymorphicModelTest.java | Stops swallowing exceptions during re-serialization by failing the test. |
| jackson/src/test/java/eu/mihosoft/vmf/jackson/test/simple/JsonSchemaModelTest.java | Adds concrete draft-07 structure assertions, including oneOf+allOf discriminator shape and constraints/defaults. |
| jackson/src/test/java/eu/mihosoft/vmf/jackson/test/renameprops01/RenamePropsModelTest.java | Adds assertions that renamed properties serialize under the renamed field name. |
| jackson/src/test/java/eu/mihosoft/vmf/jackson/test/playground/Main.java | Deletes unused playground code. |
| jackson/src/test/java/eu/mihosoft/vmf/jackson/test/interfaceonly01/InterfaceOnly01ModelTest.java | Stops swallowing exceptions during re-serialization by failing the test. |
| jackson/src/test/java/eu/mihosoft/vmf/jackson/test/external_types02/ExternalTypeModel02Test.java | Replaces bare assert + swallowed exceptions with JUnit assertions and explicit map round-trip checks. |
| jackson/src/main/java/eu/mihosoft/vmf/jackson/VMFTypeUtils.java | Guards an Optional.get() to avoid failure when containment metadata is missing. |
| jackson/src/main/java/eu/mihosoft/vmf/jackson/VMFJsonSchemaGenerator.java | Fixes polymorphic schema generation (draft-07-safe discriminator), subtype resolution for @InterfaceOnly, and deterministic property ordering/output. |
| jackson/src/main/java/eu/mihosoft/vmf/jackson/VMFJacksonModule.java | Makes deserialization failures explicit instead of silently dropping fields. |
| jackson/src/main/java/eu/mihosoft/vmf/jackson/Main.java | Deletes commented-out dead code. |
| jackson/build.gradle | Removes unused application plugin configuration and drops an unused victools JSON schema generator test dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+37
to
+40
| JsonNode allOf = option.get("allOf"); | ||
| assertTrue(allOf != null && allOf.size() == 2, | ||
| "each polymorphic option must combine $ref + discriminator via allOf"); | ||
| assertTrue(allOf.get(0).has("$ref"), "first allOf branch must be the $ref"); |
Comment on lines
+406
to
+410
| // do not swallow: a field that fails to deserialize would otherwise be | ||
| // silently dropped, causing hard-to-diagnose data loss. Propagate so the | ||
| // enclosing handler reports it as an IOException. | ||
| throw new RuntimeException( | ||
| "Failed to deserialize field '" + fieldName + "'", e); |
Comment on lines
+29
to
+31
| // the property must be renamed via @Annotation("vmf:jackson:rename") | ||
| assertTrue(json.contains("\"comPort\""), "property must be serialized as 'comPort'"); | ||
| assertFalse(json.contains("cOMPort"), "the original property name must not leak into JSON"); |
Add the execute bit (100644 -> 100755) so the script can be run directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add VMFJsonSchemaGenerator.createObjectMapper(): a preconfigured, order- preserving ObjectMapper for reading/writing the schema Map returned by generateSchema(). It keeps the schema's insertion order (which encodes propertyOrder / declaration order) instead of sorting keys alphabetically. generateSchemaAsString() now uses it, and the schema tests dogfood it. Adds a focused test asserting the mapper preserves order and round-trips. Closes #74 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Follow-up in 7a78462: added Related issues this PR touches:
Test suite is now 8 classes / 19 tests, all green. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the experimental
vmf-jacksonmodule (de)serialization + JSON-schema generation actually work, and replaces print-only tests with real assertions. 8 test classes / 18 tests, all green on a clean build.Serialization / deserialization
printStackTrace()+ continue, which previously dropped the field silently.Optional.get()inVMFTypeUtils.isParentOfPropContainer.JSON schema generation
Condition-named type).@vmf-typeconst now lives in anallOfbranch instead of as a$refsibling. Draft-07 ignores keywords placed next to$ref, so the discriminator was being silently dropped.@InterfaceOnlyelement types (e.g.Update[]→StateUpdate) by scanning the model's full type list by name. Interface-only types can't reflect on themselves, which is why "extending types" did not work in generated schemas.LinkedHashMap) and emittedpropertyOrderpreserving VMF declaration order.Cleanup
Main.java), the unusedapplication-plugin config, and the now-unused victools test dependency.Tests
JsonSchemaModelTest,FormatModelTest,SchemaTest01from print-only into real assertions covering theallOfdiscriminator,propertyOrder, defaults, numeric constraints,format/inject/uniqueItems, enum handling and interface-only polymorphism.catchblocks in round-trip tests, convert a bareassertto JUnit and verify map contents survive the round-trip, add a rename assertion, and dropSchemaTest01's working-directory file-write side effect.Notes for reviewers
allOfschema-shape change is spec-correct andvmfedit's Java side only touchesdefinitions/$ref, but the change was not verified against thevmfeditJS json-editor (the JavaFX app couldn't be run in this environment) — worth a quick render check there.--init-scriptredirecting toolchains to 17 (bytecode stays Java 11 viaoptions.release). Unrelated to the code changes.🤖 Generated with Claude Code
Note
Medium Risk
Schema shape changes (
allOfdiscriminators, property order) may affect json-editor consumers; deserialize now fails fast where it previously silently omitted fields.Overview
Hardens the experimental vmf-jackson module so deserialization failures surface instead of dropping fields, and so generated JSON Schema is valid draft-07 for polymorphic VMF models.
Deserialization: Builder field failures now throw with a clear message instead of
printStackTrace()and continuing.VMFTypeUtils.isParentOfPropContaineronly reads containment annotations when present, avoiding a fragileOptional.get().JSON schema: Polymorphic
oneOfoptions put the@vmf-typediscriminator in anallOfbranch (draft-07 ignores siblings of$ref). Subtype lists for@InterfaceOnlydeclared types (e.g.Update[]→StateUpdate) come from the model’s full type list viasubTypesOf. Schemas useLinkedHashMap, emitpropertyOrder, andcreateObjectMapper()disables key sorting so serialized schema order stays stable. Debug stdout forCondition-named types is removed.Cleanup: Drops the Gradle
applicationplugin, unused victools test dependency, and deadMain/ playground sources.Tests:
JsonSchemaModelTest,FormatModelTest, andSchemaTest01assert schema shape (discriminator, defaults, constraints, formats, interface-only polymorphism). Round-trip tests use JUnit failures instead of swallowed exceptions; map and rename coverage is tightened.Reviewed by Cursor Bugbot for commit 7a78462. Bugbot is set up for automated code reviews on this repo. Configure here.