gs1 / EPCIS

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

Epcis query parameters in query string #434

Closed mgh128 closed 2 years ago

mgh128 commented 2 years ago

I've proposed an updated version of openapi.yaml (and regenerated openapi.json) in which I have included the fixed-name EPCIS query parameters (e.g. eventType, EQ_bizStep, GE_eventTime ) within #/components/parameters/ and have referenced these from the GET methods of most REST endpoints that end in /events

I did find that OpenAPI 3.1.0 already supports "style":"pipeDelimited" with "explode": false, which appears to be exactly the combination we need when expressing something like EQ_bizStep=shipping|receiving or EQ_disposition=active|in_transit in the URI query string. We had not been using this until now - but from the testing I did using the Swagger editor, this seems to be the correct approach and did not require us to describe some clumsy workaround.

Unfortunately, I can't find any support for variable parameter names within OpenAPI 3.1.0, which is clearly problematic since around half of the EPCIS query parameters shown in the table in section 8.2.7.1 do not have fixed names but instead have flexible names in which uom_, _type or fieldname_ etc. are not literal strings but might be substituted with something like _CEL, _possessing_party, etc., respectively. I did see a mention in https://spec.openapis.org/oas/v3.1.0#patterned-fields-2 of 'patterned fields which declare a regex pattern for the field name.' - but no practical examples of how to formulate these correctly, nor in the Swagger editor.

I noted also that the current query-schema.json appears to be treating such italicised uom_, _type, fieldname_ as though they were a literal part of a fixed parameter name (e.g. literally EQ_source_type whereas the actual parameters are EQ_source_possessing_party, EQ_source_owning_party or EQ_source_location), so I'm not convinced that query-schema.json as it's currently formulated will be able to validate such flexibly-named query parameter fields - but nor do I have a more elegant solution about how to do that in JSON Schema. I think there might be an opportunity to include descriptions and enumerations within query-schema.json to make that friendlier to any tools that might display such information.

While preparing this, I did notice a number of other glitches, both in openapi.yaml and in the table of section 8.2.7.1 of the EPCIS specification, so I've informed @CraigRe about those.

I hope you feel that this is helpful and aligns with your intention to make more of the EPCIS Query language discoverable via the OpenAPI interface, using tools such as the Swagger Editor. I used the Swagger Editor while developing these enhancements to openapi.yaml.

If you're aware of a way to group the set of references to the various fixed-name EPCIS Query parameters, please make that improvement, rather than having to list them all each time. However, when listing them all, I did take care to omit the one that could conflict with the value of a field expressed within the endpoint URI path information, e.g. within the GET method for endpoint /bizSteps/{bizStep}/events I have omitted the reference to #/components/parameters/EQ_bizStep.

Looking at EPCIS §12.7.3, I'm wondering whether we should have more explicit text about how an implementation should handle the situation where the same parameter (e.g. bizStep) could be expressed in both the URI path information and the URI query string. Should that throw a QueryParameterException or does the value expressed in the path information override the value(s) expressed in the query string? - or vice versa? - or should the implementation merge all such specified values into a list/array of alternative values, sourced from the URI path information and the URI query string?

jmcanterafonseca-iota commented 2 years ago

@mgh128 I am concerned about the maintainability of the spec with this approach.

For instance, why the bizSteps are duplicated instead of getting them from the JSON Schema and injected as we did in other cases? Same with dispositions ...

mgh128 commented 2 years ago

Hi @jmcanterafonseca-iota If you agree in principle with the output (openapi.json), we can then adjust your schema injector code to insert those enumerations from the JSON Schema, to improve maintainability, rather than hard-code them in openapi.yaml

jmcanterafonseca-iota commented 2 years ago

concerning the issue on duplicated parameters I think /bizSteps/shipping/events?EQ_bizStep=delivering should raise an error i.e. you cannot query events by bizStep when you are getting only those that match a particular bizStep through an explicit endpoint. That should be clarified in the spec and in the OpenAPI so that such parameter is not allowed in the corresponding endpoint.

jmcanterafonseca-iota commented 2 years ago

a last overall comment is,

I am still concerned about the maintainability of the approach of enumerating query parameters

probably future versions of the standard should consider just having a single query parameter, for instance q , and q can be specified to contain an expression according to certain grammar, for example

GET /events?q=bizStep=shipping;eventTime>=22-05-2022T12:00

jmcanterafonseca-iota commented 2 years ago

I've just checked and it is pretty easy to inject bizSteps in the Schema

for instance

BizStepCollection:
      type: object
      required:
        - '@context'
        - 'type'
        - member
      properties:
        '@context':
          $ref: '#/components/schemas/LDContext'
        'type':
          type: string
          enum: 
            - 'Collection'
        member:
          type: array
          items:
            $ref: 'https://gs1.github.io/EPCIS/EPCIS-JSON-Schema.json#/definitions/bizStep'
          uniqueItems: true
mgh128 commented 2 years ago

a last overall comment is,

I am still concerned about the maintainability of the approach of enumerating query parameters

probably future versions of the standard should consider just having a single query parameter, for instance q , and q can be specified to contain an expression according to certain grammar, for example

GET /events?q=bizStep=shipping;eventTime>=22-05-2022T12:00

I'm not sure that q=bizStep=shipping will even be valid in a URI query string. Would everything that processes a query string actually extract bizStep=shipping;eventTime>=22-05-2022T12:00 as the value of q ? It certainly diverges from the more usual approach of ?key1=value1&key2=value2... where key1, key2 etc. would be the query parameters and value1, value2 might be pipe-delimited, when expressing a list of alternative values.

Even if a future version were to use a single query parameter, I expect that the second = in your example would need to be percent-encoded, together with many of the other symbol characters such as >= when these are considered to be the value of q.

jmcanterafonseca-iota commented 2 years ago

thank you Mark. once you fix that I can make a final follow-up PR that fixes the issue of reusability of bizStep, disposition, types, etc.

On Tue, Jun 7, 2022 at 2:03 PM Mark Harrison @.***> wrote:

@.**** commented on this pull request.

In REST Bindings/openapi.yaml https://github.com/gs1/EPCIS/pull/434#discussion_r891131154:

  • minItems: 1
  • items:
  • type: string
  • EQ_correctiveEventID:
  • name: EQ_correctiveEventID
  • description: "If this parameter is specified, the result will only include events that (a) contain an ErrorDeclaration; and where (b) one of the elements of the correctiveEventIDs list is equal to one of the values specified in this parameter. If this parameter is omitted, events are returned regardless of whether they contain an ErrorDeclaration or the contents of the correctiveEventIDs list."
  • in: query
  • style: pipeDelimited
  • explode: false
  • required: false
  • schema:
  • type: array
  • minItems: 1
  • items:
  • type: string

Thanks - I'll check these. On the positive side, my misuse of JSON within YAML has nevertheless resulted in the intended result in openapi.json - though I agree that I should correct the openapi.yaml - and will also check the type at line 5235 - though right now, I'm preparing a pull request for some gaps and glitches I noticed in query-schema.json

— Reply to this email directly, view it on GitHub https://github.com/gs1/EPCIS/pull/434#discussion_r891131154, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQEZJLQ6ACFX4MYTF6F6NWTVN427HANCNFSM5X5JM6LQ . You are receiving this because you were mentioned.Message ID: @.***>

-- IOTA Foundation Pappelallee 78/79 10437 Berlin, Germany

Board of Directors: Dominik Schiener, Navin Ramachandran ID/Foundation No.: 3416/1234/2 (Foundation Register of Berlin)

mgh128 commented 2 years ago

thank you Mark. once you fix that I can make a final follow-up PR that fixes the issue of reusability of bizStep, disposition, types, etc.

Thanks in advance, @jmcanterafonseca-iota - I think I have now reformulated my JSON within YAML as YAML.