OAI / Arazzo-Specification

The Arazzo Specification - A Tapestry for Deterministic API Workflows
https://spec.openapis.org/arazzo/latest.html
Apache License 2.0
224 stars 44 forks source link

Clarify conditions when a Parameter object shall be defined mutually exclusive to a Reusable object #236

Closed jeremyfiel closed 3 months ago

jeremyfiel commented 3 months ago

Regarding this allOf definition related to workflowId, operationId and operationPath, the specification indicates there are conditions to be met. That's the reason I created these conditional statements.

https://github.com/OAI/Arazzo-Specification/blob/df64e14d36e4c963256cbdf1b4f3759d8ccd95c5/versions/1.0.0.md?plain=1#L391

The name location of the parameter. Possible values are "path", "query", "header", "cookie", or "body". When the step in context specifies a workflowId, then all parameters map to workflow inputs. In all other scenarios (e.g., a step specifies an operationId), the in field MUST be specified.


My interpretation of this section means if operationId or operationPath are the type defined, the reference can only be a Parameter object and the in attribute must be required.

If a workflowId is defined, either parameterObject or reusableObject may be defined.

Is that a correction interpretation?

_Originally posted by @jeremyfiel in https://github.com/OAI/Arazzo-Specification/pull/224#discussion_r1702377928_

frankkilcommins commented 3 months ago

@jeremyfiel no that's not the correct interpretation.

My interpretation of this section means if operationId or operationPath are the type defined, the reference can only be a Parameter object and the in attribute must be required.

It's perfectly valid to have a Reusable Object defined for a parameter within a step that specifies with an operationId or a operationPath.

Take the following example, which defines a Reusable Object which references a page parameter component and overrides the parameter value with a value of 1.

   reference: $components.parameters.page
   value: 1

If a workflowId is defined, either parameterObject or reusableObject may be defined.

This is correct. But to be clear regardless of operationId, operationPath, or workflowId, it is possible to have step parameters defined using either Parameter Object or Reusable Object

jeremyfiel commented 3 months ago

ok, so that means

if operationPath or operationId, >> reusableObject OR parameterObject with required: [in]

if workflowId >> reusableObject OR parameterObject and in is optional

allOf:
      - if:
          oneOf:
            - required:
                - operationPath
            - required:
                - operationId
        then:
          properties:
            parameters:
              items:
                oneOf:
                  - $ref: '#/$defs/reusable-object'
                  - allOf:
                      - $ref: '#/$defs/parameter-object'
                      - required:
                          - in
      - if:
          required:
            - workflowId
        then:
          properties:
            parameters:
              items:
                oneOf:
                  - $ref: '#/$defs/parameter-object'
                  - $ref: '#/$defs/reusable-object'

Can I suggest updating this text in the spec to clarify that? What are the other scenarios? operationId, operationPath, ???

In all other scenarios (e.g., a step specifies an operationId), the in field MUST be specified.

frankkilcommins commented 3 months ago

if workflowId >> reusableObject OR parameterObject and in is optional

When workflowId is specified then in within a Parameter Object has no meaning as all parameters map to workflow inputs.

jeremyfiel commented 3 months ago

if workflowId >> reusableObject OR parameterObject and in is optional

When workflowId is specified then in within a Parameter Object has no meaning as all parameters map to workflow inputs.

I'm not sure that mapping logic works. What is the behavior if you have parameters with name clashes? I know we have a header and a path parameter that share a name.

I think the in has to be defined for a parameter no matter what and the Parameter Object should be revisited.

OAS has a notion of request parameters which can have path, header, query, cookie, but they also have a separate headers map for response headers. That's already a mess, but allowing the behavior of a headers map that can be a request or a response would be sufficient and a nice upgrade.

unfortunately, there will be some use of non-http standardized headers out in the wild that don't use x-, where the use of x- would prevent a collision like this.

GET /workers/{aoid} HTTP/1.1
accept: application/json
aoid: "123333"

--- 

200 OK
content-type: application/json
aoid: "123333"
jeremyfiel commented 3 months ago

take this example... how does the reference know which parameter to modify?

arazzo: 1.0.0
info:
  title: test
  version: 0.0.1
sourceDescriptions:
  - name: arazzo
    type: arazzo
    url: https://example.com/arazzo.yaml
workflows:
  - workflowId: workflow1
    steps:
      - stepId: step1
        workflowId: workflow1
        parameters:
          - reference: '$components.parameters.aoid'
            value: 'john doe'
          - reference: '$components.parameters.aoid'
            value: '55555'
components:
  parameters:
    aoid:
      name: aoid
      value: '12345'
jeremyfiel commented 3 months ago

rereading #181

does it make sense to collapse the reusable object into the parameter object OR extend the parameter object with these additional properties to have a single reuse mechanism for Parameters.

couldn't we do something similar to the criterion object?

$comment: parameter-object
properties:
  name:
    type: string
  in:
    type: string
  value:
    type: string
  type:
    enum:
      - jsonpath
      - xpath
  context:
    type: string
required:
  - name
  - in
  - value
parameters:
  - name: my parameter
    in: path
    type: jsonpath
    context: $components.parameters.page
    value: 1

OR

update the resuable object naming to use context rather than reference. that naming seems more consistent with the rest of the specification too.

$comment: reusable-object
properties:
  context:
    type: string
  type:
    enum:
    - jsonpath
    - xpath
  value:
    type: string
required:
  - context
  - type
  - value

cc @DmitryAnansky @frankkilcommins @handrews

frankkilcommins commented 3 months ago

I'm not sure that mapping logic works. What is the behavior if you have parameters with name clashes? I know we have a header and a path parameter that share a name.

A unique parameter is defined by the combination of a name and in fields.

I think the in has to be defined for a parameter no matter what and the Parameter Object should be revisited.

in MUST be specified when defined a parameter which is to be mapped to an API endpoint (as referenced by an operationId or operationPath. This is covered in the spec by the following verbiage: "When the step in context specifies a workflowId, then all parameters map to workflow inputs. In all other scenarios (e.g., a step specifies an operationId), the in field MUST be specified.".

In situations where parameters are being mapped to workflow inputs (e.g. when workflowId is defined at the step), in is not required as all parameters are mapped to inputs within the referenced Workflow.

That's already a mess, but allowing the behavior of a headers map that can be a request or a response would be sufficient and a nice upgrade.

I don't see a need to cover response headers as parameters in Arazzo only target API request elements or workflow inputs. It's fine for response headers to be returned from a step. If they are of interest, the will be mapped to step outputs.

Looking at the example you provide:

arazzo: 1.0.0
info:
  title: test
  version: 0.0.1
sourceDescriptions:
  - name: arazzo
    type: arazzo
    url: https://example.com/arazzo.yaml
workflows:
  - workflowId: workflow1
    steps:
      - stepId: step1
        workflowId: workflow1
        parameters:
          - reference: '$components.parameters.aoid'
            value: 'john doe'
          - reference: '$components.parameters.aoid'
            value: '55555'
components:
  parameters:
    aoid:
      name: aoid
      value: '12345'

This is not a valid parameter construct as it contains duplicate aoid parameters.

jeremyfiel commented 3 months ago

in MUST be specified when defined a parameter which is to be mapped to an API endpoint (as referenced by an operationId or operationPath. This is covered in the spec by the following verbiage: "When the step in context specifies a workflowId, then all parameters map to workflow inputs. In all other scenarios (e.g., a step specifies an operationId), the in field MUST be specified.".

in is only required when a parameter object is used, but a reusable object targeting a parameter doesn't require the same constraint.

This is not a valid parameter construct as it contains duplicate aoid parameters.

it doesn't contain duplicate parameters, it targets duplicate named parameters, but in reality, those two parameter definitions are unique, one is in: path the other is in: header. The current spec doesn't define how to make this work. This is a legal construct per the specification, apart from the schema validation. But, this does indeed pass schema validation because even with a uniqueItems: true definition on the parameter array, the different value creates a unique entry.


Btw, just for reference, the OAS requires in for all parameters, Arazzo does not. Is that an oversight? only name, value are required.

frankkilcommins commented 3 months ago

in is only required when a parameter object is used, but a reusable object targeting a parameter doesn't require the same constraint.

True, but reusable objects are just providing a referencing mechanism with some override affordances. Ultimately, they need to be resolved and in this context to a Parameter Object.

it doesn't contain duplicate parameters, it targets duplicate named parameters, but in reality, those two parameter definitions are unique, one is in: path the other is in: header.

I don't see in specified in the "aoid" parameter that is referenced in the example, so I am not sure how you determine that the two definitions are unique with one using in: path and the other using in: header.

There can be situations that we may not be able to have an opinion on from a schema validation perspective, but that's not to say they should not be exercised by resolvers and/or parsers.

Btw, just for reference, the OAS requires in for all parameters, Arazzo does not. Is that an oversight? only name, value are required.

This was deliberate to prevent authoring verbosity when defining parameters where workflowId is set. In this situation, all parameters are to be mapped to inputs on the referenced Workflow. Forcing authors to specify in: workflow for every parameter was removed as an affordance.

jeremyfiel commented 3 months ago

thanks @frankkilcommins I think i'm clear on the expectations and usage of Parameter objects.