External operations trump conflicting root operations in the `operations` export

[zinckiwi]

Description

Our particular use case is a user spec and a superset admin spec, that at various points refers to models within the user spec. Many operation IDs are replicated between the two with minor differences (e.g. an admin version of a user operation supporting additional query params).

Because the external parsing comes after the primary file parsing, any operation which has an ID shared by an external file is overwritten by that external file's definition.

A minimal test case to demonstrate:

test-one.yaml

openapi: 3.0.1
paths:
  /api/v1/list:
    get:
      operationId: listObjects
      responses:
        "200":
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/TestObjOne"
  /api/v1/listOther:
    get:
      operationId: listOtherObjects
      responses:
        "200":
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/TestObjOne"
components:
  schemas:
    TestObjOne:
      type: object
      properties:
        prop1:
          type: number
        prop2:
          $ref: "./test-two.yaml#/components/schemas/TestObjTwo"

test-two.yaml

openapi: 3.0.1
paths:
  /api/v1/list:
    get:
      operationId: listObjects
      responses:
        "200":
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/TestObjTwo"
components:
  schemas:
    TestObjTwo:
      type: object
      properties:
        prop1:
          type: string

output (excerpt)

npx openapi-typescript test-one.yaml -o x.ts

=>

export interface operations {
  listObjects: {
    responses: {
      200: {
        content: {
          "application/json": external["test-two.yaml"]["components"]["schemas"]["TestObjTwo"];
        };
      };
    };
  };
  listOtherObjects: {
    responses: {
      200: {
        content: {
          "application/json": components["schemas"]["TestObjOne"];
        };
      };
    };
  };
}

I would have expected both operations to invoke the response type of components["schemas"]["TestObjOne"]. Instead, the definition from the external spec (which is only in the picture due to a schema ref) is used.

Proposal

When conflicting operations (i.e., by ID) exist in multiple schemas, the root schema's operation should "win" over an external schema's operation. Or, since this would be a breaking change, some mechanism to achieve it (a default to the current behaviour, an optional choice-making callback, or similar) would be welcome.

Checklist

• [x] I’m willing to open a PR for this (see CONTRIBUTING.md)

[zinckiwi]

Hi @drwpow, any thoughts on this? A quick and dirty swap of 2c and 2d in the index file (plus a tweak to ignore the root schema in 2d which hasn't been removed yet, as a result) is enough to achieve what I'm after, and could fork it accordingly if there's no interest from your end. But if there is, happy to PR something, uh, "slow and clean".

[ysulyma]

Ran into something similar, I would expect external operations to be generated under external[filename]["operations"] and not pollute the root operations export.

[drwpow]

In general, conflict problems was a huge reason why v7 of this library moved to pre-bundling with Redocly which fixes issues like this. But fortunately due to an implementation detail of how operations specifically get built, we could detect ID conflicts in ctx.operations and change the default behavior (i.e. handle conflicts better).

I’m not opposed to letting the root operation “win” as suggested. I don’t really see this as a true breaking change per se because I generally consider the foundation to be intentional behavior. And as-is, there’s no intentional, deterministic behavior for handling operationId conflicts (I would also suspect that if multiple external schemas conflicted, it might be a race condition present as well).

That leaves the question of: what to rename the conflicted operationId to? E.g. operations["conflict-[filename]"]? I personallly think they should get bubbled up into the root operations object, but beyond that don’t have any strong opinions.

Also worth pointing out: v7 is getting rid of the external object so the less you rely on that now the easier migrating will be when the time comes

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. If there is no activity in the next 7 days, the issue will be closed.

[github-actions[bot]]

This issue was closed because it has been inactive for 7 days since being marked as stale. Please open a new issue if you believe you are encountering a related problem.