lukeautry / tsoa

Build OpenAPI-compliant REST APIs using TypeScript and Node
MIT License
3.56k stars 503 forks source link

Deeply nested wide mapped types on enums do not resolve (sometimes) #681

Closed E-gy closed 1 year ago

E-gy commented 4 years ago

Nesting mapped types eventually [deeply] does not correctly resolve the schema - not marking top-level props as required; sometimes causing a crash, depending on the breadth of nested types; and in oddly specific configurations, it's weird.

Sorting

Setup

We'll use the following mapped types setup:

enum venum { ab = "cd", ef = "gh", ij = "kl"}
type contained = { name: string; val: number; }
type K1 = { [P in venum]: contained }
type K2 = { [P in venum]: { [P in venum]: contained } }
type K3 = { [P in venum]: { [P in venum]: { [P in venum]: contained } } }
type K4 = { [P in venum]: { [P in venum]: { [P in venum]: { [P in venum]: contained } } } }
type J2 = { [P in venum]: K1 }
type J3 = { [P in venum]: J2 }
type J4 = { [P in venum]: J3 }
type J5 = { [P in venum]: J4 }

and the extended

enum venum1 { ab = "cd1", ef = "gh1", ij = "kl1", mn = "op1", qr = "st1", uv = "wx1"}
enum venum2 { ab = "cd2", ef = "gh2", ij = "kl2", mn = "op2", qr = "st2", uv = "wx2"}
enum venum3 { ab = "cd3", ef = "gh3", ij = "kl3", mn = "op3", qr = "st3", uv = "wx3"}
enum venum4 { ab = "cd4", ef = "gh4", ij = "kl4", mn = "op4", qr = "st4", uv = "wx4"}
type contained = { name: string; val: number; }
type K1 = { [P in venum1]: contained }
type K2 = { [P in venum2]: { [P in venum1]: contained } }
type K3 = { [P in venum3]: { [P in venum2]: { [P in venum1]: contained } } }
type K4 = { [P in venum4]: { [P in venum3]: { [P in venum2]: { [P in venum1]: contained } } } }
type J2 = { [P in venum2]: K1 }
type J3 = { [P in venum3]: J2 }
type J4 = { [P in venum4]: J3 }

Basically, K types are mapped directly (without referring to smaller K), J are mapped to lower J.

Using a union type instead of enum

type venum = "cd" | "gh" | "kl";

does not, and should not, change the behaviour.

Using different enums for every layer does not change the behaviour. Sometimes it does, see (far) below. Correction: it does not indeed.

To generate the schema, i'm putting all types in the same request body, but they can be put anywhere, we just need to use the types so they get included in the schema really,

@Post('stuff')
public async nestedStuffs(@Body() body: {
    contained: contained;
    k1: K1;
    k2: K2;
    k3: K3;
    // k4: K4; //← this crashes
    j2: J2;
    j3: J3;
    j4: J4;
    j5: J5;
}): Promise<any> {}

To demonstrate a particular problem with J nesting however, K1 and nested must not be referenced outside J types:

@Post('stuff')
public async nestedStuffs(@Body() body: {
    // contained: contained;
    // k1: K1;
    k2: K2;
    k3: K3;
    j2: J2;
    j3: J3;
    j4: J4;
    j5: J5;
}): Promise<any> {}

Expected Behavior

For the K types, deeper types should get nested the same way as shallower types (or perhaps they should rely on additional schemas, if OpenAPI has depth limits). And the properties should be marked as required. (repeated sections stripped to keep the issue somewhat readable)

K1:
  properties:
    cd:
      $ref: '#/components/schemas/contained'
    gh:
      $ref: '#/components/schemas/contained'
    kl:
      $ref: '#/components/schemas/contained'
  required:
    - kl
    - gh
    - cd
  type: object
K2:
  properties:
    cd:
      properties:
        kl:
          $ref: '#/components/schemas/contained'
        gh:
          $ref: '#/components/schemas/contained'
        cd:
          $ref: '#/components/schemas/contained'
      required:
        - kl
        - gh
        - cd
      type: object
    gh: ...
    kl: ...
  required:
    - kl
    - gh
    - cd
  type: object
K3:
  properties:
    cd:
      properties:
        kl:
          properties:
            kl:
              $ref: '#/components/schemas/contained'
            gh:
              $ref: '#/components/schemas/contained'
            cd:
              $ref: '#/components/schemas/contained'
          required:
            - kl
            - gh
            - cd
          type: object
        gh: ...
        cd: ...
      required:
        - kl
        - gh
        - cd
      type: object
    gh: ...
    kl: ...
  required:
    - kl
    - gh
    - cd
  type: object
K4: ...

For J types, deeper types should literally reference shallower types for nesting (and the properties be marked required on every depth level):

J2:
  properties:
    cd:
      $ref: '#/components/schemas/K1'
    gh:
      $ref: '#/components/schemas/K1'
    kl:
      $ref: '#/components/schemas/K1'
  required:
    - kl
    - gh
    - cd
  type: object
J3:
  properties:
    cd:
      $ref: '#/components/schemas/J2'
    gh:
      $ref: '#/components/schemas/J2'
    kl:
      $ref: '#/components/schemas/J2'
  required:
    - kl
    - gh
    - cd
  type: object
J4:
  properties:
    cd:
      $ref: '#/components/schemas/J3'
    gh:
      $ref: '#/components/schemas/J3'
    kl:
      $ref: '#/components/schemas/J3'
  required:
    - kl
    - gh
    - cd
  type: object
J5:
  properties:
    cd:
      $ref: '#/components/schemas/J4'
    gh:
      $ref: '#/components/schemas/J4'
    kl:
      $ref: '#/components/schemas/J4'
  required:
    - kl
    - gh
    - cd
  type: object

Current Behavior

Alright, so, for the shallow types (K1, K2) and J types* everything works mostly as expected - the properties at first level of the schema are not marked as required.

K1:
  properties:
    cd:
      $ref: '#/components/schemas/contained'
    gh:
      $ref: '#/components/schemas/contained'
    kl:
      $ref: '#/components/schemas/contained'
<  required:
<    - kl
<    - gh
<    - cd
  type: object
K2:
  properties:
    cd:
      properties:
        kl:
          $ref: '#/components/schemas/contained'
        gh:
          $ref: '#/components/schemas/contained'
        cd:
          $ref: '#/components/schemas/contained'
      required:
        - kl
        - gh
        - cd
      type: object
    gh: ...
    kl: ...
<  required:
<    - kl
<    - gh
<    - cd
  type: object
K3:
  properties:
    cd:
      properties:
        kl:
          properties:
            kl:
              $ref: '#/components/schemas/contained'
            gh:
              $ref: '#/components/schemas/contained'
            cd:
              $ref: '#/components/schemas/contained'
          required:
            - kl
            - gh
            - cd
          type: object
        gh: ...
        cd: ...
      required:
        - kl
        - gh
        - cd
      type: object
    gh: ...
    kl: ...
<  required:
<    - kl
<    - gh
<    - cd
  type: object
...
J2:
  properties:
    cd:
      $ref: '#/components/schemas/K1'
    gh:
      $ref: '#/components/schemas/K1'
    kl:
      $ref: '#/components/schemas/K1'
<  required:
<    - kl
<    - gh
<    - cd
  type: object
J3:
  properties:
    cd:
      $ref: '#/components/schemas/J2'
    gh:
      $ref: '#/components/schemas/J2'
    kl:
      $ref: '#/components/schemas/J2'
<  required:
<    - kl
<    - gh
<    - cd
  type: object
...

Deep K types - K3, K4 cause a crash [for both docs and routes generation], but not always. Which level crashes actually depends on the breadth - the primary setup generates K3 fine and crashes on K4, but the extended setup crashes on K3 (and K4). The crash is the same in both cases (just replace K4 for K3):

There was a problem resolving type of 'K3'.
Generate swagger error.
 Error: Cannot read property 'kind' of undefined
 in 'Controller.nestedStuffs'
    at new GenerateMetadataError (...\node_modules\tsoa\dist\metadataGeneration\exceptions.js:21:28)
    at ...\node_modules\tsoa\dist\metadataGeneration\methodGenerator.js:80:23
    at Array.map (<anonymous>)
    at MethodGenerator.buildParameters (...\node_modules\tsoa\dist\metadataGeneration\methodGenerator.js:73:47)
    at MethodGenerator.Generate (...\node_modules\tsoa\dist\metadataGeneration\methodGenerator.js:62:30)
    at ...\node_modules\tsoa\dist\metadataGeneration\controllerGenerator.js:40:58
    at Array.map (<anonymous>)
    at ControllerGenerator.buildMethods (...\node_modules\tsoa\dist\metadataGeneration\controllerGenerator.js:40:14)
    at ControllerGenerator.Generate (...\node_modules\tsoa\dist\metadataGeneration\controllerGenerator.js:29:27)
    at ...\node_modules\tsoa\dist\metadataGeneration\metadataGenerator.js:98:58

Finally, there's another thing. For some oddly specific configurations, J nesting generates schemas that loop on itself, instead of, well, going to lower levels. I haven't yet been able to determine a consistent/minimal type structure that produces this behaviour, and i will update the issue once i do. Best way to put it is "the nested type right after last nested type that is not reffered to outside of shallower nested types gets recursed on itself". Here's what i mean: For the last referencing configuration

@Post('stuff')
public async nestedStuffs(@Body() body: {
    // contained: contained;
    // k1: K1;
    k2: K2;
    k3: K3;
    j2: J2;
    j3: J3;
    j4: J4;
    j5: J5;
}): Promise<any> {}

This is the generated spec:

K1:
  properties:
    cd:
      $ref: '#/components/schemas/K1'
    gh:
      $ref: '#/components/schemas/K1'
    kl:
      $ref: '#/components/schemas/K1'
  type: object
J2:
  properties:
    cd:
      $ref: '#/components/schemas/K1'
    gh:
      $ref: '#/components/schemas/K1'
    kl:
      $ref: '#/components/schemas/K1'
  type: object
J3:
  properties:
    cd:
      $ref: '#/components/schemas/J2'
    gh:
      $ref: '#/components/schemas/J2'
    kl:
      $ref: '#/components/schemas/J2'
  type: object
J4:
  properties:
    cd:
      $ref: '#/components/schemas/J3'
    gh:
      $ref: '#/components/schemas/J3'
    kl:
      $ref: '#/components/schemas/J3'
  type: object
J5:
  properties:
    cd:
      $ref: '#/components/schemas/J4'
    gh:
      $ref: '#/components/schemas/J4'
    kl:
      $ref: '#/components/schemas/J4'

Notice how K1 refers to itself instead of contained:

K1:
  properties:
    cd:
>      $ref: '#/components/schemas/K1'
<      $ref: '#/components/schemas/contained'
    gh:
>      $ref: '#/components/schemas/K1'
<      $ref: '#/components/schemas/contained'
    kl:
>      $ref: '#/components/schemas/K1'
<      $ref: '#/components/schemas/contained'
  type: object

Now, if we change remove J2, J3 from initial referencing:

@Post('stuff')
public async nestedStuffs(@Body() body: {
    // contained: contained;
    // k1: K1;
    k2: K2;
    k3: K3;
    //j2: J2;
    //j3: J3;
    j4: J4;
    j5: J5;
}): Promise<any> {}

The spec generated is following:

J3:
  properties:
    cd:
      $ref: '#/components/schemas/J3'
    gh:
      $ref: '#/components/schemas/J3'
    kl:
      $ref: '#/components/schemas/J3'
  type: object
J4:
  properties:
    cd:
      $ref: '#/components/schemas/J3'
    gh:
      $ref: '#/components/schemas/J3'
    kl:
      $ref: '#/components/schemas/J3'
  type: object
J5:
  properties:
    cd:
      $ref: '#/components/schemas/J4'
    gh:
      $ref: '#/components/schemas/J4'
    kl:
      $ref: '#/components/schemas/J4'
  type: object

Once again, J3 loops on itself, but also schemas for J2 and K1 are not even generated

<K1:
<  properties:
<    cd:
<      $ref: '#/components/schemas/K1'
<    gh:
<      $ref: '#/components/schemas/K1'
<    kl:
<      $ref: '#/components/schemas/K1'
<  type: object
<J2:
<  properties:
<    cd:
<      $ref: '#/components/schemas/K1'
<    gh:
<      $ref: '#/components/schemas/K1'
<    kl:
<      $ref: '#/components/schemas/K1'
<  type: object
J3:
  properties:
    cd:
>      $ref: '#/components/schemas/J3'
<      $ref: '#/components/schemas/J2'
    gh:
>      $ref: '#/components/schemas/J3'
<      $ref: '#/components/schemas/J2'
    kl:
>      $ref: '#/components/schemas/J3'
<      $ref: '#/components/schemas/J2'
  type: object

By extending J types down to J7, and referencing selectively from the request, we can see the looping consistently "in action":

@Post('stuff')
public async nestedStuffs(@Body() body: {
    j3: J3;
    j5: J5;
    j7: J7;
}): Promise<any> {}

J2:
  properties:
    cd:
>      $ref: '#/components/schemas/J2'
<      $ref: '#/components/schemas/J1'
    gh:
>      $ref: '#/components/schemas/J2'
<      $ref: '#/components/schemas/J1'
    kl:
>      $ref: '#/components/schemas/J2'
<      $ref: '#/components/schemas/J1'
  type: object
J3:
  properties:
    cd:
      $ref: '#/components/schemas/J2'
    gh:
      $ref: '#/components/schemas/J2'
    kl:
      $ref: '#/components/schemas/J2'
  type: object
J4:
  properties:
    cd:
>      $ref: '#/components/schemas/J4'
<      $ref: '#/components/schemas/J3'
    gh:
>      $ref: '#/components/schemas/J4'
<      $ref: '#/components/schemas/J3'
    kl:
>      $ref: '#/components/schemas/J4'
<      $ref: '#/components/schemas/J3'
  type: object
J5:
  properties:
    cd:
      $ref: '#/components/schemas/J4'
    gh:
      $ref: '#/components/schemas/J4'
    kl:
      $ref: '#/components/schemas/J4'
  type: object
J6:
  properties:
    cd:
>      $ref: '#/components/schemas/J6'
<      $ref: '#/components/schemas/J5'
    gh:
>      $ref: '#/components/schemas/J6'
<      $ref: '#/components/schemas/J5'
    kl:
>      $ref: '#/components/schemas/J6'
<      $ref: '#/components/schemas/J5'
  type: object
J7:
  properties:
    cd:
      $ref: '#/components/schemas/J6'
    gh:
      $ref: '#/components/schemas/J6'
    kl:
      $ref: '#/components/schemas/J6'

Context (Environment)

Version of the library: 3.0.8 Version of NodeJS: 13.11.0

Breaking change?

I don't think so.

WoH commented 4 years ago

O boy. This one's really pushing it, thanks a lot for your detailed bug report. Properly determining which props should be required seems like a good issue we can fix first. I have no idea how the references are mixed up, I suspect this is the reason why some schemas are not even generated, but I'm curious to know how that would happen. Maybe our circular dependency resolver is giving up when it shouldn't?

E-gy commented 4 years ago

Current workaround:

  1. Export all [intermediate] types 1.1 (if any, expand ~K3+ types into J types)
  2. Create a dummy unused method (in a controller), declared before everything; in body or return, reference all the intermediate types
  3. Straight up throw an error inside it just in case
  4. mark as @Hidden to remove from docs

Thankfully @Hidden removes the method from docs, and does not intefere with types construction/exploration, so the docs, and validation get generated successfully.

github-actions[bot] commented 4 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days