nsip / specgen_input_au

Capture of specgen input files
2 stars 0 forks source link

Move mandatory attribute of XML element from being mandatory element of object #40

Closed opoudjis closed 1 year ago

opoudjis commented 1 year ago

SIF_RefObject is defined as required field under SchoolInfo object, but as per the example that is part of "OtherLEA" object and not in root level for SchoolInfo. Specification seems to be wrong.

Me: This is indeed an error in the JSON Schema generation: @SIF_RefObject is mandatory within the object it is an attribute of, not within the host object.

This is the JSON Schema generation reading specgen naively, and the initial implementation won’t have paid attention to it as it was done for NZ and not using attributes anyway. This should have been fixed for the rendering of the JSON in payload.

opoudjis commented 1 year ago

@joerghuber Wait until I investigate this a bit more.

opoudjis commented 1 year ago

I actually think the real solution to this is either to change the specgen source, so that

IdRef + SIF_RefObject

becomes a new type (and that's the usual circumstance), or else some quite convoluted parsing of the items in a sequence, to generate new ad hoc types.

Note also that there is a lot of ad hoc enumerating of SIF_RefObject values. it is not tenable to transport this validation if the Element + Attribute combination turns into a new type. But restrictions of Object.Element.SIF_RefObject values for that full path are beyond JSON Schema, and should be.

I think this particular enum restriction is too hard to transfer to JSON Schema, for not enough payoff. Making IdRef + SIF_RefObject etc a new type is the best outcome...

4pins commented 1 year ago

@opoudjis This seems like a wonderful idea!

opoudjis commented 1 year ago

Actually, I've tweaked this, and I'm going to see if I can solve it with less effort.

I'm now going to try doing the following:

This will reduce the transition effort significantly. Going to try it out today.

opoudjis commented 1 year ago

If I can trust the Redocly error log as an indicator:

SIF_Refobject instances, that will be taken care of by TypedIdRefType automatically, fingers crossed:

Other attributes, that need to be moved into new types:

For those of you with long memories: this is a (half-way) Venetian Blinds refactoring of Element + Attribute pairs in the spec, that in retrospect should have been done along with all the other Venetian Blinds out of Russian Doll refactoring that was done on Specgen XML years ago. By leaving the IdRef + Type (and their idiosyncratic enums) alone in the XML, we're substantially removing the number of types we end up having to shuffle, and we're not losing the idiosyncratic Object-specific enum validations.

opoudjis commented 1 year ago

Will ignore attributes as potential required fields unless they are attributes of the root element being described. As an approximation of this: /Item[2][Attribute] is a root element attribute. /Item[3][Attribute] is if /Item[2][Attribute]. /Item[4][Attribute] is if /Item[2][Attribute] and /Item[3][Attribute].

opoudjis commented 1 year ago

Because the name of the attribute is part of the definition of an element type, only elements with attribute SIF_RefObject can be moved to TypedIdRefType . If another attribute name has been used, that will need to be factored out as a new type.

opoudjis commented 1 year ago

Needing to add ref="CommonTypes" to Generic-CommonTypes.xml, for their types to be processed correctly.

opoudjis commented 1 year ago

In fact all these additions of attributes to existing types need to be treated as extensions (complex="extension").

opoudjis commented 1 year ago

No, they can't, because this is a string turned into an object

opoudjis commented 1 year ago

Need to factor out in specgen:

/CommonElement consisting of List, Element, Attribute => /CommonElement consisting of List, /CommonElement consisting of Element, Attribute : e.g. IdentityAssertionsType