nsip / specgen_input_au

Capture of specgen input files
2 stars 0 forks source link

Add infrastructure schema to PESC stylesheet #43

Closed opoudjis closed 1 year ago

opoudjis commented 1 year ago

Currently the error response is having single error object, do we require an array of error object in the response ?

I assume the issue here is that the XML-to-PESC stylesheet, which generates the array wrapper, is only receiving the data model as input, and is therefore not recognising list elements in the infrastructure. This needs to be fixed.

When it is, I need to ensure @4pins is comfortable with how to run the XML-to-PESC stylesheet for his side.

If this is a top-level array, this may be the same issue as https://github.com/nsip/specgen_input_au/issues/38 : the stylesheets reading in the object definition but not the wrapper definition. But I suspect it isn't.

opoudjis commented 1 year ago

@joerghuber can I confirm that this is not the case, and error is an array?

In case of multiple validation errors in the request, are we expected to combine the error details and set it in message field?

joerghuber commented 1 year ago

The Infrastructure specification defines the error message in a response as a single object. It is not an array.

joerghuber commented 1 year ago

The infrastructure team is reviewing the way errors can be reported as part of the current infrastructure work. This will significantly enrich the way errors can be reported to a caller.

opoudjis commented 1 year ago

OK. Still need to ensure XMLtoPesc is aware of infrastructure.

opoudjis commented 1 year ago

In order for the payloads of Infrastructure examples to be converted to correct PESC, I need access to a Specgen XML version of the infrastructure spec, to run through https://github.com/nsip/sifxml2pescjson_source and generate an up to date sif2jsonspecgen.xslt

... I cannot find one other than https://github.com/nsip/DraftSIFInfrastructureSpec . Is a Specgen XML version of the infrastructure spec still being maintained, and in a public place, @4pins @joerghuber ?

opoudjis commented 1 year ago

@4pins Clearly https://github.com/nsip/sifxml2pescjson_source needs to deal with the NA specgen XML as well; I'm putting in a desultory version of it for now.

4pins commented 1 year ago

Nick:

Like I have said previously, I need to update both the Infrastructure and NA source repositories on GitHub (hopefully Monday). If you need anything ASAP it is sitting on my hard drive. I’ll try to send you the latest Infrastructure sif2jsonspecgen.xslt by another channel.

Thanks,

John Lovell Technology Director

Access 4 Learning Community http://www.A4L.orghttp://www.a4l.org/

From: Nick Nicholas @.> Reply-To: nsip/specgen_input_au @.> Date: Thursday, December 15, 2022 at 10:11 PM To: nsip/specgen_input_au @.> Cc: John Lovell @.>, Mention @.***> Subject: Re: [nsip/specgen_input_au] Add infrastructure schema to PESC stylesheet (Issue #43)

@4pinshttps://github.com/4pins Clearly https://github.com/nsip/sifxml2pescjson_source needs to deal with the NA specgen XML as well; I'm putting in a desultory version of it for now.

— Reply to this email directly, view it on GitHubhttps://github.com/nsip/specgen_input_au/issues/43#issuecomment-1354270217, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACTVSOZW6NKJ3TO26PJN2ADWNQB3PANCNFSM6AAAAAAS7KORWU. You are receiving this because you were mentioned.Message ID: @.***>

opoudjis commented 1 year ago

Another infuriating inconsistency.

PESC consistently encodes lists as

<XList><X>one</X><X>two</X></XList>

to

{ "XLIST: {  "X" : ["one", "two"] } }

And I convert the data model examples accordingly.

However, the infrastructure examples given follow the Goessner realisation of lists:

{ "XLIST": [ { "X": "one" }, { "X": "two" } ] }

Thus, for example,

      deleteMultiExamples:
        pesc:             
          summary: PESC - Batch Delete Request
          value:              
            deleteRequest:    
              deletes:    
                - delete:     
                    id: df789e1c-dfe7-4c18-8ef0-d907b81ea61e
                - delete:     
                    id: 41953aaa-2811-11e6-b67b-9e71128cae77
                - delete:     
                    id: ff789e1c-dfe7-4c18-8ef0-d907b81ea61e

But running the Infrastructure specgen XML through the existing XSLT, as is consistent with PESC, and as appears to have been done to generate jsonSchemaCreate_IN.yaml, is generating instead a PESC-like schema for the infrastructure:

  deleteRequestCollection:
    type: object
    properties: 
      deleteRequest:
        type: array
        items:
          $ref: '#/definitions/deleteRequest'
  deleteRequest:
    type: object
    properties:
      deletes:
        type: object
        $ref: '#/definitions/deleteIdCollection'
    description: >-
      <p>A "deleteRequest" element, which consists of multiple, "delete" object ids. Since REST conventions do not support payloads on HTTP DELETE messages, all multi-object Delete Requests are conveyed via an HTTP PUT message containing an additional HTTP Header Field value of methodOverride set to DELETE.</p>
  deleteIdCollection:
    type: object
    required:
    - "delete"
    properties:
      delete: 
        type: array
        items:
          $ref: '#/definitions/deleteId'
  deleteId:
    type: object
    properties:
      id:
        type: string
        xml:
          attribute: true 

Which rejects the example given to date as invalid, and instead expects to see:

          value:              
            deleteRequest:    
              deletes:    
                delete:     
                  -  id: df789e1c-dfe7-4c18-8ef0-d907b81ea61e
                  -  id: 41953aaa-2811-11e6-b67b-9e71128cae77
                  -  id: ff789e1c-dfe7-4c18-8ef0-d907b81ea61e

(Sidestepping the fact that deleteRequest itself is meant to be an array.)

We have a serious problem here @joerghuber. The infrastructure examples are assuming PESC and Goessner handle arrays identically in its examples, but they do not, and the XSLT stylesheets are only making sense of the payloads as PESC, not Goessner.

In the following, by Goessner-mode, I mean processing arrays with repeating keys, [ {X: Y}, {X: Y} ], rather than the PESC-mode { X: [Y, Y] }. The infrastructure examples currently have PESC handling of attribute names (id not @id) but Goessner-mode treatment of lists.

Alternatives are:

  1. No validation of infrastructure messages at all.
  2. Create a variant XSLT version, to deal with infrastructure messages in the Goessner way always. (So the infrastructure always handles lists in Goessner-mode, under both PESC and Goessner notation.
  3. Allow expressly that the PESC and Goessner versions of the infrastructure are going to be structurally different, as well as having different key naming conventions, and change all PESC infrastructure examples to actually conform to PESC, including PESC-mode lists.

(1) is a non-option. The entire point of OpenAPI is that is validates examples, and we cannot allow payloads in any aspect of OpenAPI that do not validate. (That is going to be a real problem for Goessner payloads, which currently have no associated JSON Schema at all in our set-up, which is PESC-only: https://github.com/nsip/specgen_input_au/issues/47 )

(2) may be attractive for infrastructure, but I believe it is a non-option as well. Already infrastructure has to switch between @id and id for PESC and Goessner. To switch the mode of array processing between infrastructure JSON consumers and data model JSON consumers is not, I believe, an acceptable expectation for our clients.

So I think (3) is the only realistic option. It will need changes in all PESC infrastructure payloads, but at least they are restricted, as far as I can tell to the standalone commondefs* files, and can be updated as a one-off task. All infrastructure files generated via the XSLT transforms, including the static file jsonSchemaCreate_IN.yaml (as far as I can tell!), are already in PESC-mode.

If we reject (3), and go with (2), we will have to (a) author a Goessner-mode XSLT processor (presumably with lots of choose statements and parameterisation), and (b) regenerate jsonSchemaCreate_IN.yaml, so that it is in Goessner-mode rather than PESC-mode. Right now, the payload examples for PESCS in commondefs* and the payload schemas in jsonSchemaCreate_IN.yaml are simply not consistent with each other.

The diagnostic of all this is the Redocly error must NOT have unevaluated properties {k} (meaning, as far as I can tell, that Redocly is finding an array under {k}, in Goessner-mode, where it expected to find an object, in PESC-mode.) The same error is being reported for https://github.com/nsip/specgen_input_au/issues/38, failure to insert [ ] around object list examples (e.g. {"StudentSectionEnrollments": {"StudentSectionEnrollment": ...

opoudjis commented 1 year ago

I am now going to attempt to generate OpenAPI from the latest infrastructure spec as well as to the pesc stylesheet, from https://github.com/Access4Learning/specgen_input_infra .

As determined in #39, OpenAPI 3.1 output, which we will continue to use for online documentation, will continue to reference Goessner and XML; OpenAPI 3.0 output shall not. There shall be OpenAPI 3.1 and OpenAPI 3.0 versions of the infrastructure content accordingly.

In order to generate these added outputs, am creating PR for specgen_input_infra

opoudjis commented 1 year ago

Infrastructure specgen integration is complete, and there are no more errors being reported in the OpenAPI output.

4pins commented 1 year ago

@opoudjis Good work!