aj-foster / open-api-generator

Open API code generator for Elixir
MIT License
97 stars 13 forks source link

response type mapping #32

Closed paulbalomiri closed 9 months ago

paulbalomiri commented 10 months ago

I just noticed that the client response option is passed as:

[{"application/json", [{OApi.Kratos.Identity.JsonPatch, :t}]}]

According to the openapi response specification this can be actually a map, if i understand correctly that the response client option maps only the paths/<path>/<method>/responses/content/<mime-type> part of any spec.

A Map would enforce that only one schema spec can exist & help keeping client code concise EDIT and aid in pattern matching.

This is either an enhancement request or nitpicking, so feel free to tag appropiately 😉

paulbalomiri commented 10 months ago

Just to clarify I'm lising an example of the patch method.

The proposal would be for the request argument to be

%{"application/json", [{OApi.Kratos.Identity.JsonPatch, :t}]}

instead of a List (not even Keyword because of the String.t keys), as seen here in the patch/2-3 method

 @doc """
  Patch an Identity

  Partially updates an [identity's](https://www.ory.sh/docs/kratos/concepts/identity-user-model) field using [JSON Patch](https://jsonpatch.com/).
  The fields `id`, `stateChangedAt` and `credentials` can not be updated using this method.
  """
  @spec patch_identity(String.t(), OApi.Kratos.Identity.JsonPatch.t(), keyword) ::
          {:ok, OApi.Kratos.Identity.t()} | {:error, OApi.Client.Error.t()}
  def patch_identity(id, body, opts \\ []) do
    client = opts[:client] || @default_client

    client.request(%{
      args: [id: id, body: body],
      call: {OApi.Kratos.Identity, :patch_identity},
      url: "/admin/identities/#{id}",
      body: body,
      method: :patch,
      request: [{"application/json", [{OApi.Kratos.Identity.JsonPatch, :t}]}],
      response: [
        {200, {OApi.Kratos.Identity, :t}},
        {400, {OApi.Kratos.Error.Generic, :t}},
        {404, {OApi.Kratos.Error.Generic, :t}},
        {409, {OApi.Kratos.Error.Generic, :t}},
        default: {OApi.Kratos.Error.Generic, :t}
      ],
      opts: opts
    })
  end

This is the Ory Kratos patch_identity call speced like so: [EDIT: corrected path ~/components/schema~ to /paths in schema paste below]

paths:
  /admin/identities/{id}:
     patch:
      description: |-
        Partially updates an [identity's](https://www.ory.sh/docs/kratos/concepts/identity-user-model) field using [JSON Patch](https://jsonpatch.com/).
        The fields `id`, `stateChangedAt` and `credentials` can not be updated using this method.
      operationId: patchIdentity
      parameters:
        - description: ID must be set to the ID of identity you want to update
          in: path
          name: id
          required: true
          schema:
            type: string
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/jsonPatchDocument'
        x-originalParamName: Body
      responses:
        '200':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/identity'
          description: identity
        '400':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/errorGeneric'
          description: errorGeneric
        '404':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/errorGeneric'
          description: errorGeneric
        '409':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/errorGeneric'
          description: errorGeneric
        default:
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/errorGeneric'
          description: errorGeneric
      security:
        - oryAccessToken: []
      summary: Patch an Identity
      tags:
        - identity
aj-foster commented 9 months ago

Hi there!

I completely agree, a map is a much nicer and more flexible way to express this information. For backwards compatibility, I'm keeping the list of tuples as the default behaviour, but you can now use the configuration output.operation_call.request with a value of :map to achieve what you described. It's available now on main.

Thanks! ❤️

paulbalomiri commented 9 months ago

Thanks, will use it for the oapi_kratos , oapo_hydra and parhaps more!