feature: discriminator

[Eywek]

Sorting

• I'm submitting a ...

  • [ ] bug report
  • [x] feature request
  • [ ] support request

• I confirm that I

  • [x] used the search to make sure that a similar issue hasn't already been submit

Description

I'm using mongoose discriminators for my models, so I have 1 principal model and 3 children. In my controllers, I have a PATCH method to update my models, in this PATCH method all properties are optionals and I can use this method to update any of my children models.

BUT, tsoa need to validate the right schema depending on which model I'm updating. It will be nice if we can have a solution to declare a pre-validation function to tell tsoa which schema to validate against.

Currently, I'm updating the handlebars template to call my custom method (see below) in the patch route instead of validating schema:

  public async validate (validate: (args: any, request: any, response: any) => any[], args: Args, req: express.Request, res: express.Response): Promise<any[]> {
    const dt = await req.models.Datasource.findOne({ _id: new ObjectId(req.params.id) }, 'mode').exec()
    if (!dt) {
      throw new httpErrors.DatasourceNotFound({ id: req.params.id })
    }
    switch (dt.mode) {
      case DatasourceMode.WORKER:
        args.body.ref = 'PatchWorkerDt'
        break
      case DatasourceMode.INGESTER:
        args.body.ref = 'PatchIngesterDt'
        break
      case DatasourceMode.PROXY:
        args.body.ref = 'PatchProxyDt'
        break
    }
    return validate(args, req, res)
  }

Possible Solution

Maybe a decorator to define a custom pre-validation function where we will need to return the type name we need to validate against?

[jeremyVignelles]

The discriminator is also needed in the case of inheritance : https://swagger.io/specification/#composition-and-inheritance-polymorphism

Related: We should be able to tell which "children classes" we need to include in the generated schema. NSwag does that through the [KnownType(typeof(ChildType))]. It's not perfect (as you can't annotate external types), but it's better than nothing. See https://github.com/RicoSuter/NSwag/blob/e2e920fca79d1fda848bcd339908aa2b3d423dd4/src/NSwag.CodeGeneration.TypeScript.Tests/TypeScriptDiscriminatorTests.cs#L15-L17

[tsimbalar]

Regarding discriminators etc, my understanding if that this would "Just Work ™️ " if you use union types as accepted models in your controller.

Imagine you have :

interface Type1{
  discriminator: 'type1';
  propertyOfType1: string;
}

interface Type2{
  discriminator: 'type2';
  propertyOfType2: number;
}


interface Type3{
  discriminator: 'type3';
  propertyOfType3: string[];
}

export type ExposedType = Type1 | Type2 | Type3;

then implicitly the discriminator property would be used to distinguish between each type, bothen in the OpenApi spec and in typescript.

I might be missing something totally though ...

[jeremyVignelles]

That would work with typescript, but if you're generating open api definitions to be consumed in any language, some generators might rely on the discriminator property to resolve the type really used.

[tsimbalar]

Oh right, I didn't take this into account 👍

[bytenik]

Didn't see this when I made my issue (#875). I'm happy to attempt to implement this, but I do need a bit of guidance as to where to start in the codebase.

[WoH]

Since TypeScript does this (discriminating based on properties) without explicitly specifying a discriminator, I'd find it helpful if you could work out a proposal as to how you would declare a discriminator.

[bytenik]

Since both interfaces and type declarations don't exist at runtime, decorators aren't allowed on them. As a result, I think the only option would be jsdoc on the union type that specifies the name of the discriminator.

[meyraa]

I have another case that this feature might solve. When I save a layer, it can have an existing and newly created attributes:

• When I define attributes: Array<ICreateAttribute | IAttribute> it only uses the ICreateAttribute and removes the ids.
• When I define attributes: Array<IAttribute | ICreateAttribute> it only uses the IAttribute and I get validation errors, because of missing ids.

@Post("/{id}")
public async save(@Path() id: number, @Body() body: ISaveLayer): Promise<ILayer> {
    // body.attributes are ICreateAttribute[] (but could also be IAttribute) 
    // and the id's are always undefined - not correct
}

@Post("")
public async create(@Body() body: ICreateLayer): Promise<ILayer> {
    // body.attributes are ICreateAttribute[] - correct
}

interface ICreateLayer {
    name: string;
    attributes: ICreateAttribute[];
}

interface ISaveLayer extends ICreateLayer {
    id: number;
    attributes: Array<ICreateAttribute | IAttribute>;
}

interface ILayer extends ISaveLayer {
    attributes: IAttribute[];
}

interface ICreateAttribute {
    name: string;
    dataType: DataTypes;
    geometryType?: GeometryType;
    isSearchable?: boolean;
    maxLength?: number;
    minLength?: number;
    position?: number;
}

interface IAttribute extends ICreateAttribute {
    id: number;
    saveDate: Date;

    geometryType: GeometryType;
    isSearchable: boolean;
    maxLength: number;
    minLength: number;
    position: number;
}

[devnev]

I tried hacking together a solution that detects some cases that can be converted to discriminators, diff below. In the specific cases it can handle, it seemed to generate the desired spec. For it to work, (1) the union must of object type reference, not object literals, as swagger discriminators mappings only allow schema references AFAICT (2) there must be a required field with non-empty enum of string values, as the discriminator mapping requires string keys (3) the enum values must be distinct between the various types so that we can work out which value maps to which type (4) there must be exactly one property that fulfils the above conditions.

In this case, from something like

interface ProcessingStatusProcessing {status: "processing", progressPercent: number};
interface ProcessingStatusCompleted {status: "completed", outputUrl: string};
interface ProcessingStatusFailed {status: "failed", failureCode: string};
type ProcessingStatus =
  | ProcessingStatusProcessing
  | ProcessingStatusCompleted
  | ProcessingStatusFailed

it generates

components:
  schemas:
    ProcessingStatus:
      oneOf:
        - $ref: "#/components/schemas/ProcessingStatusProcessing"
        - $ref: "#/components/schemas/ProcessingStatusCompleted"
        - $ref: "#/components/schemas/ProcessingStatusFailed"
      discriminator:
        propertyName: status
        mapping:
          processing: "#/components/schemas/ProcessingStatusProcessing"
          completed: "#/components/schemas/ProcessingStatusCompleted"
          failed: "#/components/schemas/ProcessingStatusFailed"
    ProcessingStatusProcessing:
      properties:
        status:
          type: string
          enum: [processing]
        progressPercent:
          type: number
      required:
        - status
        - progressPercent
    ProcessingStatusCompleted:
      properties:
        status:
          type: string
          enum: [completed]
        outputUrl:
          type: string
      required:
        - status
        - outputUrl
    ProcessingStatusFailed:
      properties:
        status:
          type: string
          enum: [failed]
        failureCode:
          type: string
      required:
        - status
        - failureCode

The code:

diff --git a/packages/cli/src/swagger/specGenerator3.ts b/packages/cli/src/swagger/specGenerator3.ts
index c80e84b..19068c7 100644
--- a/packages/cli/src/swagger/specGenerator3.ts
+++ b/packages/cli/src/swagger/specGenerator3.ts
@@ -593,6 +593,60 @@ export class SpecGenerator3 extends SpecGenerator {
     }
   }

+  protected getUnionDiscriminator(types: Tsoa.Type[]) {
+    const isRefObject = (t: Tsoa.Type): t is Tsoa.RefObjectType => t.dataType === 'refObject';
+    const isRefAlias = (t: Tsoa.Type): t is Tsoa.RefAliasType => t.dataType === 'refAlias';
+    const isNestedObjectLieral = (t: Tsoa.Type): t is Tsoa.NestedObjectLiteralType => t.dataType === 'nestedObjectLiteral';
+
+    // collect all required enum props with non-empty string-only values as
+    // candidates for discriminator properties
+    const enumProps: Record<string, string[][]> = {};
+    const refNames: string[] = [];
+    for (const type of types) {
+      refNames.push(type['refName']);
+      if (isRefObject(type)) {
+        for (const prop of type.properties) {
+          const isString = (x: unknown): x is string => typeof x === 'string';
+          if (prop.required && prop.type.dataType === 'enum' && prop.type.enums.length && prop.type.enums.every(isString)) {
+            (enumProps[prop.name] ||= []).push(prop.type.enums);
+          }
+        }
+      } else if (isRefAlias(type) && isNestedObjectLieral(type.type)) {
+        for (const prop of type.type.properties) {
+          const isString = (x: unknown): x is string => typeof x === 'string';
+          if (prop.required && prop.type.dataType === 'enum' && prop.type.enums.length && prop.type.enums.every(isString)) {
+            (enumProps[prop.name] ||= []).push(prop.type.enums);
+          }
+        }
+      }
+    }
+
+    // only keep candidates that are present on all types and have non-overlapping values
+    const candidates = Object.entries(enumProps).filter(([_, enums]) => {
+      if (enums.length !== types.length) return false;
+      const counts = new Map<string, number>();
+      enums.forEach(values => values.forEach(value => counts.set(value, (counts.get(value) ?? 0) + 1)));
+      if (Array.from(counts.values()).some(count => count !== 1)) return false;
+      return true;
+    });
+
+    // if we have unambiguously identified a viable discriminator property,
+    // generate a oneOf with disciminator and mapping using the enum values.
+    if (candidates.length === 1) {
+      const mapping = {};
+      candidates[0][1].forEach((values, idx) => values.forEach(value => (mapping[value] = refNames[idx])));
+      return {
+        oneOf: types.map(t => this.getSwaggerType(t)),
+        discriminator: {
+          propertyName: candidates[0][0],
+          mapping: mapping,
+        },
+      };
+    }
+
+    return null;
+  }
+
   protected getSwaggerTypeForUnionType(type: Tsoa.UnionType) {
     // Filter out nulls and undefineds
     const actualSwaggerTypes = this.removeDuplicateSwaggerTypes(
@@ -605,6 +659,9 @@ export class SpecGenerator3 extends SpecGenerator {
     );
     const nullable = type.types.some(x => this.isNull(x));

+    const discriminatorType = this.getUnionDiscriminator(type.types);
+    if (discriminatorType) return discriminatorType;
+
     if (nullable) {
       if (actualSwaggerTypes.length === 1) {
         const [swaggerType] = actualSwaggerTypes;
diff --git a/packages/runtime/src/swagger/swagger.ts b/packages/runtime/src/swagger/swagger.ts
index 6dfcf58..07bdffd 100644
--- a/packages/runtime/src/swagger/swagger.ts
+++ b/packages/runtime/src/swagger/swagger.ts
@@ -251,9 +251,14 @@ export namespace Swagger {
   export interface Schema3 extends Omit<Schema, 'type'> {
     type?: DataType;
     nullable?: boolean;
+    oneOf?: BaseSchema[];
     anyOf?: BaseSchema[];
     allOf?: BaseSchema[];
     deprecated?: boolean;
+    discriminator?: {
+      propertyName: string;
+      mapping?: { [value: string]: string };
+    };
   }

   export interface Schema2 extends Schema {
@@ -267,7 +272,6 @@ export namespace Swagger {
     format?: DataFormat;
     additionalProperties?: boolean | BaseSchema;
     properties?: { [propertyName: string]: Schema3 };
-    discriminator?: string;
     readOnly?: boolean;
     xml?: XML;
     externalDocs?: ExternalDocs;

[devnev]

In theory (based on the OpenAPI spec and its use of the JSON Reference spec and its use of the JSON pointer spec), to handle unions with inline object definitions, the mapping reference could use a path like #/components/schemas/ProcessingStatus/oneOf/0, as in

components:
  schemas:
    ProcessingStatus:
      discriminator:
        propertyName: status
        mapping:
          processing: "#/components/schemas/ProcessingStatus/oneOf/0"
          completed: "#/components/schemas/ProcessingStatus/oneOf/1"
          failed: "#/components/schemas/ProcessingStatus/oneOf/2"
      oneOf:
        - # inline schema for processing status
        - # inline schema for completed status
        - # inline schema for failed status

But the Go generator of openapi-generator I've been testing with doesn't handle it at all, producing code that won't even compile, as it attempts to reference a field that doesn't exist.

[devnev]

Looking at the OAS again, I notice the line regarding discriminators:

When using the discriminator, inline schemas will not be considered.

However, the spec is also explicit that the discriminator serves as a hint for serialisation and validation but isn't necessary for the proper use of oneOf when only one subschema is valid.

Based on that, TSOA could do two separate steps:

1. Check if there's a field that behaves like a discriminator, in which case use a oneOf instead of an anyOf
2. If additionally that field has a string type and all the subschemas are references, also output a discriminator with a mapping

[sureyeaah]

I really need this to be fixed as well. I like the proposal by @devnev, is there any consensus on this?