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

Request body design update? #152

Closed
RomanHotsiy opened this issue Feb 22, 2024 · 9 comments · Fixed by #162
Closed

Request body design update? #152

RomanHotsiy opened this issue Feb 22, 2024 · 9 comments · Fixed by #162
Labels
implementor-draft In scope for first version

Comments

@RomanHotsiy
Copy link

RomanHotsiy commented Feb 22, 2024

We find the current design of request body not very flexible and not consistent with OpenAPI 3.0 definition terminology. (we hit issues trying to cover our real-life use cases)

Are there any reasons for this specific design?

We have an alternative design which we want to share here and get some feedback!

The main idea is to describe request body params in requestBody property (same as OpenAPI 3.0) instead of in: body (was used in OpenAPI 2.0).

We can work on the formal definition but would be faster to start from a few examples to gather feedback first.

Example from the spec:

- stepId: do-the-auth-flow
  requestBody:
    # contentType is optional, by default should use the one defined in OpenAPI spec or application/json
    # Object value will be serialized according to the contentType if possible
    contentType: application/x-www-form-urlencoded
    value:
      client_id: $inputs.clientId
      grant_type: $inputs.grantType
      redirect_uri: $inputs.redirectUri
      client_secret: $inputs.clientSecret
      code: $steps.browser-authorize.outputs.code
      scope: $inputs.scope

We can control the serialization format with OpenAPI encoding object, e.g.:

- stepId: do-the-auth-flow
  requestBody:
    contentType: application/x-www-form-urlencoded
    encoding:
      scope:
        style: form
        explode: true
    value:
      client_id: $inputs.clientId
      grant_type: $inputs.grantType
      redirect_uri: $inputs.redirectUri
      client_secret: $inputs.clientSecret
      code: $steps.browser-authorize.outputs.code
      scope: $inputs.scope

Application/json example:

- stepId: place-order
  requestBody:
    contentType: application/json
    value:
      pet_id: $steps.find-pet.outputs.my_pet_id
      coupon_code: $steps.find-coupons.outputs.my_coupon_code

Example with XML body:

- stepId: place-order-xml
  operationId: placeOrderXml
  requestBody:
    contentType: application/xml
    value: |
      <xml>
        <petId>{$inputs.pet_id}</petId>
        <couponCode>{$inputs.coupon_code}</couponCode>
        <quantity>{$inputs.quantity}</quantity>
        <status>placed</status>
        <complete>false</complete>
      </xml>

We also have an idea for file uploads using special $file:

- stepId: upload-receipt-with-status
  operationId: uploadReceipt
  requestBody:
    contentType: multipart/form-data
    encoding:
      file:
        contentType: application/pdf
    value: 
      file: $file('./receipt.pdf') # uses special $file to load file from the file system, if encoding is not defined, it will be encoded as application/octet-stream
      status: placed

What do you think about this approach? We like it more as it is more flexible (covers all the real use-cases we and our users have) and also because it reuses terminology from OpenAPI.

Waiting for your feedback!

@handrews
Copy link
Member

@RomanHotsiy should that x-requestBody in the last example just be requestBody?

On the general topic: While I haven't been very involved in the Workflows project, I am definitely in favor of anything that aligns it more with OAS 3.x than 2.0.

@RomanHotsiy
Copy link
Author

@RomanHotsiy should that x-requestBody in the last example just be requestBody?

Yes. thanks! I fixed it. (this is how we implemented it right now in our implementation)

@kevinduffey
Copy link
Collaborator

I am not sure I understand.. the operationId or operationRef points to the operation in the API description that has the details of the request body. Why would we want/need to add that as an option here? I apologize if I am missing the significance of the purpose, but I believe the operation contains the full request body details, hence the pointer to it via operationId or operationRef?
Tooling would need to load in the API description operation details from that source, when it reached this step using the operationId pointer details (or operationRef).

@RomanHotsiy
Copy link
Author

has the details of the request body. Why would we want/need to add that as an option here

Yes. That's why contentType and encoding should not be required. BUT the operation can have multiple request bodies defined so you can pick the one by providing the contentType.

Same can be asked about the current design.

https://github.com/OAI/sig-workflows/blob/9164da1e3e63df94737d84bac4b135e2479688a8/examples/1.0.0/oauth.workflow.yaml#L93-L104

Why do we need to specify style: form here? Shouldn't it be already defined in the OpenAPI description?

While it's okay to provide request bodies for form in this format it's very inconvenient to do same for JSON payloads with the current design. Also it's not consistent with OpenAPI. Parameters in OpenAPI don't have in: body (it used to have in Swagger).

@frankkilcommins
Copy link
Collaborator

Hi @RomanHotsiy, thanks for raising this.

FYI - there's no underlying intention here to align to OpenAPI 2.0 or OpenAPI 3.* from a structural perspective. This spec should be able to describe workflows on top of both underlying OAS major versions. That being said I agree that the currently structure is inflexible especially when wanting to template out the request body structure for JSON or XML, and it's something that was on my concerns list.

In general, the proposal laid out makes sense and I'll review in more detail and see what changes we can get into this version prior to launch. I would like to keep the ability to specifically target parts of a request using JSON Pointer as well as having the ability to provide a fully templated payload.

Rough examples:

  1. XML Templated Payload
- stepId: place-order-xml
  operationId: placeOrder
  requestBody:
    contentType: application/xml
    payload: |
      <petOrder>
        <petId>{$inputs.pet_id}</petId>
        <couponCode>{$inputs.coupon_code}</couponCode>
        <quantity>{$inputs.quantity}</quantity>
        <status>placed</status>
        <complete>false</complete>
      </petOrder>
  1. JSON Templated Payload
- stepId: place-order-json
  operationId: placeOrder
  requestBody:
    contentType: application/json
    payload: |
      {
        "petOrder": {
          "petId": "{$inputs.pet_id}",
          "couponCode": "{$inputs.coupon_code}",
          "quantity": "{$inputs.quantity}",
          "status": "placed",
          "complete": false
        }
      }
  1. JSON Targeted Values
- stepId: place-order-json
  operationId: placeOrder
  requestBody:
    contentType: application/json
    values:
      - name: pet_id
        target: /petId
        value: $inputs.pet_id
      - name: quantity
        target: /quantity
        value: 1
  1. Form Data Values
- stepId: do-the-auth-flow
  requestBody:
    contentType: application/x-www-form-urlencoded
    values:
      - name: client_id: 
        value: $inputs.clientId
      - name: grant_type
        value: $inputs.grantType
      - name: redirect_uri
        value: $inputs.redirectUri
      - name: client_secret
        value: $inputs.clientSecret
      - name: code
        value: $steps.browser-authorize.outputs.code
      - name: scope
        value: $inputs.scope

@RomanHotsiy
Copy link
Author

@frankkilcommins I like you examples!

A few questions:

  • Should the target be a JSON Pointer and start with #?
  • Do you have any concerns if payload accepts JSON object additionally to string? I like the simplicity of string but the usability of JSON object makes me want to support it (IDE syntax highlight/format validation)

@frankkilcommins
Copy link
Collaborator

frankkilcommins commented Feb 29, 2024

Hey @RomanHotsiy

  • Should the target be a JSON Pointer and start with #?

JSON Pointers start with '/'. The '#' general refers to URI Fragment Identifier Representation.

  • Do you have any concerns if payload accepts JSON object additionally to string? I like the simplicity of string but the usability of JSON object makes me want to support it (IDE syntax highlight/format validation)

How do you propose to represent a JSON Object? Similar to inline examples in OAS?

@RomanHotsiy
Copy link
Author

JSON Pointers start with '/'. The '#' general refers to URI Fragment Identifier Representation.

Yes, makes sense. Thanks!

How do you propose to represent a JSON Object? Similar to inline examples in OAS?

Yes. Like below:

- stepId: place-order-json
  operationId: placeOrder
  requestBody:
    contentType: application/json
    payload: 
      petOrder:
        petId: $inputs.pet_id
        couponCode: $inputs.coupon_code
        quantity: $inputs.quantity
        status: placed
        complete: false

@frankkilcommins
Copy link
Collaborator

@RomanHotsiy I've taken a pass at catering for this enhancement in #162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implementor-draft In scope for first version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants