Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider a list input type to accept a list of inputs identified by their own IDs #105

Closed
jerstlouis opened this issue Nov 2, 2020 · 5 comments
Labels
1.0-draft.5 Draft version for after the public review change request

Comments

@jerstlouis
Copy link
Member

jerstlouis commented Nov 2, 2020

Introduce list as a type of input which itself is a list of multiple inputs. This enables associating multiple inputs with a single input ID, supporting multiplicity. Having a dedicated list type is closer to the natural JSON way to encode such information, unlike in XML where multiplicity is achieved by repeating tags.

The classic example is a RenderMap process and a layers input, where it is desirable to preserve or assign a custom ID to each, e.g. to apply styles.

It also allows a very natural JSON way to present the information and use the plural e.g. layers allowing to nicely regroup these inputs in a list.

@joanma747 suggested a need for this capability in the Modular OGC API Workflows projects.

@christophenoel
Copy link
Contributor

christophenoel commented Nov 2, 2020

Since the OGC Process API initially mapped XML bindings to JSON, there might have a lot of XML specificities that would benefit to be better suited to REST/JSON, not only the cardinality of inputs.

I think that dataDescriptionType , complexInputType and literalInputType definitions should be reworked in the future, in particular to define those types using JSON types and constraints.

During the Hackaton 2019, we tried to find how to map the whole process description to JSON schema (https://github.com/spacebel/testbed14/wiki/OGC-Hackaton-2019). I think we should partially apply those mapping for defining the inputs/outputs types.

The following JSON Schema properties shall be used to map the minOccurs and maxOccurs definitions:

  • 'Required' attribute in case the property is mandatory (minOccurs >= 1)
  • 'Array' type in case the property includes multiple values (minOccurs > 1)
  • For arrays, 'minItems' and 'maxItems' for providing any specific cardinality
  • 'maxItems' shall be null if the number of items is unbounded
input1:
  type: array
  minItems: 1
  maxItems: 10
  items:
    $ref: 'myItemType'

To be discussed...

@bpross-52n bpross-52n added 1.0-draft.5 Draft version for after the public review change request labels Nov 5, 2020
@bpross-52n
Copy link
Contributor

Since the OGC Process API initially mapped XML bindings to JSON, there might have a lot of XML specificities that would benefit to be better suited to REST/JSON, not only the cardinality of inputs.

I think that dataDescriptionType , complexInputType and literalInputType definitions should be reworked in the future, in particular to define those types using JSON types and constraints.

During the Hackaton 2019, we tried to find how to map the whole process description to JSON schema (https://github.com/spacebel/testbed14/wiki/OGC-Hackaton-2019). I think we should partially apply those mapping for defining the inputs/outputs types.

The following JSON Schema properties shall be used to map the minOccurs and maxOccurs definitions:

  • 'Required' attribute in case the property is mandatory (minOccurs >= 1)
  • 'Array' type in case the property includes multiple values (minOccurs > 1)
  • For arrays, 'minItems' and 'maxItems' for providing any specific cardinality
  • 'maxItems' shall be null if the number of items is unbounded
input1:
  type: array
  minItems: 1
  maxItems: 10
  items:
    $ref: 'myItemType'

To be discussed...

This should probably go in a new issue (or we re-open #45). As the standard allows different process descriptions, this could eventually lead to a new conformance class.

@christophenoel
Copy link
Contributor

christophenoel commented Nov 13, 2020

Personally, I discourage this choice of allowing multiple process descriptions formats. A standard allowing all alternatives is not a standard... At least, one of the possible format should be mandatory.

I would not re-open #45 because it was question of reformatting the whole process description to provide it as an OpenAPI operation (and my conclusion was that it would clearly complexifly a lot the implementation of clients). This might be confusing that the starting topic is not considered anymore.

I really believe that the input/ouptut definitions are way too rooted in the past XML paradigm, and a few minor changes relying on OpenAPI schema would benefit to the API (https://swagger.io/specification/#schema-object).

Cheers

@jerstlouis
Copy link
Member Author

jerstlouis commented Nov 13, 2020

@bpross-52n @spacebel having been working on our implementation of a process discoverer / workflow editor this week, and looking into the process description schemas in more detail, I agree very strongly with @spacebel that some of the process description carries too much XML baggage (and possibly new baggage coming from figuring out how to describe it in OpenAPI when there could actually be a way to also describe some things simpler -- see input discussion below), and this appears to overcomplicate things. It could probably easily be made much simpler and more natural with a few tweaks, and I feel it is worth every effort to do so.

I disagree that "one of the possible format should be mandatory." per the general OGC API philosophy, but the one conformance class appearing as the default that we hope everyone will implement should feel natural and be simple, so that no one would be tempted to create a better one because the proposed one is lacking.

Two things in particular which confused our developers and that I find out of place:

  • The input inside inputDescription -- couldn't its members be included directly in the inputDescription?
  • literalDataDomain can be completely different things: an array, an object (just for the purpose of saying it can be any value), or a string (to point a type URI if understand correctly?). That makes it quite tricky to parse in our approach to parsing JSON, and it also seems difficult to validate or present the user with valid values with all these different ways of specifying valid values for literals.
  • Also, since we are discussing in OpenAPI literalData only String, inlineValue/referenceValue cannot specify uom / format; Merge complexData and literalData? #95 merging ComplexData and LiteralData, we would also need to merge LiteralDataType and ComplexDataType to simplify things and keep consistency? (e.g. being able to specify the valid formats for an inline value)

@bpross-52n
Copy link
Contributor

The respective changes should be merged now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0-draft.5 Draft version for after the public review change request
Projects
None yet
Development

No branches or pull requests

3 participants