joelittlejohn / jsonschema2pojo

Generate Java types from JSON or JSON Schema and annotate those types for data-binding with Jackson, Gson, etc
http://www.jsonschema2pojo.org
Apache License 2.0
6.24k stars 1.66k forks source link

Use decorated JsonFactory backed by SchemaGenerator when resolving JSON/YAML source inputs #1464

Closed unkish closed 1 year ago

unkish commented 1 year ago

Closes #1427

unkish commented 1 year ago

Some notes:

Feedback is welcome

joelittlejohn commented 1 year ago

@unkish Thanks for submitting this.

I'm struggling to understand exactly what's going on in #1427 and why your changes fix the problem. Maybe you could give me a summary of the problem you found and what you had to do to fix it?

It looks like you are wanting to unify the way that inputs are read using the JsonFactory abstraction, so that the source type applies correct whenever any file is read, not just at the start. Is that right?

unkish commented 1 year ago

Hi

Should have provided additional justification/clarification in the first place as I've struggled to understand what is the cause myself :/

It's related to the way how schemas are getting stored/retrieved in/from SchemaStore and manifests itself if there are several files processed. Following assumes that we have 2 files: AResponse.json and BResponse.json which do not reference each other but have a object property with same name in both of them. When files are passed as yaml/json schema, SchemaMapper wraps paths to them into $ref after that they end up being added to SchemaStore through https://github.com/joelittlejohn/jsonschema2pojo/blob/fccc0161afd787ece9f1e25e9e6f6638d8630e8e/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/SchemaStore.java#L140 https://github.com/joelittlejohn/jsonschema2pojo/blob/fccc0161afd787ece9f1e25e9e6f6638d8630e8e/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/SchemaStore.java#L54 with id's having URI as prefix and path appended to them eg.:

That way they can be/are uniquely referenced/resolved.

However when files are passed as yaml/json examples content is first transformed via SchemaGenerator and then stored/resolved in SchemaStore through https://github.com/joelittlejohn/jsonschema2pojo/blob/fccc0161afd787ece9f1e25e9e6f6638d8630e8e/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/SchemaStore.java#L128-L138 with id's being:

As id's are no longer unique across files then when #/properties/common is being resolved/looked up for BResponse.json it ends up getting "cached" value "#/properties/common" -> {Schema@3384} of AResponse.json.

\ So when we're dealing with input from URI's, it seemed that if we won't treat schemas different from samples (logic processing schemas shouldn't care whether input was from sample or not) then they'd be processed in similar way. JsonFactoryProvider was supposed to provide JsonFactory to be used by ContentResolver/SchemaMapper such that it would look to latter as if they're working with json schema input (example transformation to schema would happen ad hoc). I've left some of the public methods in place in an attempt to maintain backwards compatibility. It should be noted though that this fix won't help if someone uses own JsonFactory that won't act as instance provided by JsonFactoryProvider whilst passing multiple example files with similar property names - issue would re-surface itself.

joelittlejohn commented 1 year ago

I'm going to release 1.1.3 before merging this, as it's quite substantial and I'd like to get a 'good' release out that resolves the 'path not present' problem before we make any further fundamental change. Nothing stopping us releasing 1.1.4 soon though.

unkish commented 1 year ago

Of course, no problem - there is no rush.

I won't mind if someone will find/propose another way of fixing issue and we'll drop this PR 🙂

unkish commented 1 year ago

Added also copyright headers to java files (how embarrassing that I didn't notice their absence previously 🤦‍♂️)

unkish commented 1 year ago

Superseded by #1484.