search

gs1 / EPCIS

Draft files being shared for EPCIS 2.0 development
Other
20 stars 7 forks source link

Improve EPCIS query schema injecting EPCIS Schema definitions #437

Closed jmcanterafonseca-iota closed 2 years ago

jmcanterafonseca-iota commented 2 years ago

@mgh128 this is still a draft, but please can you start having a look?

mgh128 commented 2 years ago

Thanks - I've only taken a look through the diffs (which are tricky to read) - but I can see the logic in what you're doing - and in refactoring structures such as string-array-or-date-string-or-number so that they're defined once then referenced. Let me know if there's something in particular you'd like me to focus on - or when you're ready for me to merge the PR. Thanks!

jmcanterafonseca-iota commented 2 years ago

@mgh128 I have modified a little bit the algorithm to generate the Schema inlining so that only the needed terms are finally added (the previous assumption was that all definitions from a file would always be included which it is not the case for the query Schema).

I've regenerated with the new script the full EPCIS Schema. and despite there are differences and the file sizes are different the validation of the JSON content example takes place seamlessly. But before merging that it would be good that you could also double check ... Perhaps the file size difference is due to some redundant definition introduced but I would like to double check.

jmcanterafonseca-iota commented 2 years ago

Looks OK, though I might have been tempted to name { "type": "string", "format": "uri" } as "#/definitions/uri" and {"type": "number"} as "$ref": "#/definitions/double" (because we're not using xsd:decimal

Not sure that changing {"type": "number"} to "$ref": "#/definitions/decimal" or "$ref": "#/definitions/double" really adds much to the readability but I understand that such refactoring does make sense when using anyOf with multiple types - or when using type and format together.

we can certainly do that ... we need also to enable eventType to be an extended one as well

jmcanterafonseca-iota commented 2 years ago

@mgh128 please review in detail, on my side I consider the work in a good enough state to be landed. I have also briefed you by email on the differences between JSON Schemas and we are fine but need to perform a further tweak on the EPCIS Schema or injector so that we ensure that definitions only needed for the openapi.json are included as well

But that can be done as part of another PR

jmcanterafonseca-iota commented 2 years ago

@mgh128 sorry for the mess. The process of inlining the Schema was not working as expected and I have revoked. The query schema has been generated partially automatically and removing the unneeded definitions.

Sorry for the mess again. Now things are tidy up. I will try to work on the final and good solution in the coming hours but we can land this PR safely without breaking anything.

mgh128 commented 2 years ago

No problem. I'm on a TDS call this hour but will try to take a closer look in 2 hours from now. Thanks again for all your efforts on this.