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

feat: Add support for composed types in Typescript #4602

Merged
merged 162 commits into from
Aug 29, 2024

Conversation

koros
Copy link
Contributor

@koros koros commented May 6, 2024

Fixes: #1812

Related Tickets

TODO

Generation Logic Design:

1. Composed Types Comprised of Primitives Without Discriminator Property

Sample yml
openapi: 3.0.1
info:
  title: Example of UnionTypes
  version: 1.0.0
paths:
  /primitives:
    get:
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/primitives'
components:
  schemas:
    primitives:
      oneOf:
        - type: string
        - type: number

For the union of primitive values shown above, the generation logic is as follows:
The factory method for union of primitives determines the return type from the parse node value. For example, if the node is number | string, the method should return the correct type based on the node's value, as shown below:

export type Primitives = number | string;

export function createPrimitivesFromDiscriminatorValue(parseNode: ParseNode | undefined) : number | string | undefined {
  return parseNode?.getNumberValue() ?? parseNode?.getStringValue();
}

There is no need for a deserialization method, so it is omitted during generation.

The serialization method will determine the type of the node and call the respective method on the abstraction library for that type. Using the example above, if the node is number | string, the serializer should call either writer.writeNumberValue or writer.writeStringValue based on the node's actual value, as shown below:

export function serializePrimitives(writer: SerializationWriter, key: string, primitives: Primitives | undefined) : void {
    if (primitives == undefined) return;
    switch (typeof primitives) {
        case "number":
            writer.writeNumberValue(key, primitives);
            break;
        case "string":
            writer.writeStringValue(key, primitives);
            break;
    }
}

2. Composed Types Comprised of Objects with a Discriminator Property Specified

Sample yml
openapi: 3.0.3
info:
  title: Pet API
  description: An API to return pet information.
  version: 1.0.0
servers:
  - url: http://localhost:8080
    description: Local server

paths:
  /pet:
    get:
      summary: Get pet information
      operationId: getPet
      responses:
        '200':
          description: Successful response
          content:
            application/json:
              schema:
                oneOf:
                  - $ref: '#/components/schemas/Cat'
                  - $ref: '#/components/schemas/Dog'
                discriminator:
                  propertyName: petType
                  mapping:
                    cat: '#/components/schemas/Cat'
                    dog: '#/components/schemas/Dog'
        '400':
          description: Bad Request
        '500':
          description: Internal Server Error

components:
  schemas:
    Pet:
      type: object
      required:
        - id
        - name
        - petType
      properties:
        id:
          type: integer
          example: 1
        name:
          type: string
          example: "Fido"
        petType:
          type: string
          description: "Type of the pet"
          example: "cat"

    Cat:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            favoriteToy:
              type: string
              example: "Mouse"

    Dog:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          properties:
            breed:
              type: string
              example: "Labrador"

For the example above, the factory method determines which pet to create, either Cat or Dog, based on the discriminator information, as shown below:

export type PetGetResponse = Cat | Dog;

export function createPetGetResponseFromDiscriminatorValue(parseNode: ParseNode | undefined) : ((instance?: Parsable) => Record<string, (node: ParseNode) => void>) {
    const mappingValueNode = parseNode.getChildNode("petType");
    if (mappingValueNode) {
        const mappingValue = mappingValueNode.getStringValue();
        if (mappingValue) {
            switch (mappingValue) {
                case "cat":
                    return deserializeIntoCat;
                case "dog":
                    return deserializeIntoDog;
            }
        }
    }
    // If there is no discriminator property specified merge the 2 objects e.g
    // {} as Cat + {dogName: "Rex"} as Dog = {dogName: "Rex"}
    return { ...deserializeIntoCat, ...deserializeIntoDog };
}

The deserializer is not necessary since the function has been delegated to the respective handlers as shown above. So instead of deserializePet, we have either deserializeCat if the response is a Cat, or deserializeDog if the response is a Dog.

The serializer will also delegate the writing functionality to the respective writer. In the example above, the generated output will be as follows:

export function serializePetGetResponse(writer: SerializationWriter, petGetResponse: Partial<PetGetResponse> | undefined = {}) : void {
    if (petGetResponse == undefined) return;
    switch (petGetResponse.petType) {
        case "cat":
            serializeCat(writer, petGetResponse);
            break;
        case "dog":
            serializeDog(writer, petGetResponse);
            break;
    }
}

When there is no discriminator property specified, the serialization logic should default to serializing all possible types. This approach ensures that the writer captures all available fields from each type. For example, in the following TypeScript code, the absence of a discriminator property leads to serializing both Cat and Dog fields in the PetGetResponse:

// No discriminator property
export function serializePetGetResponse(writer: SerializationWriter, petGetResponse: Partial<PetGetResponse> | undefined = {}) : void {
    serializeCat(writer, petGetResponse as Cat);
    serializeDog(writer, petGetResponse as Dog);
}

3. Intersection of Object Values

// Dummy intersection Objects to use in the tests

type Foo = { foo?: string; }
type Bar = { bar?: string; }

// CodeIntersectionType will use same union symbol however the serialization/deserialization methods will slightly differ 
// since CodeIntersectionType of Foo and Bar can contain both Foo & Bar while CodeUnion can only contain either Foo or Bar
export type FooBar = Foo | Bar; 


// Factory Method
export function createFooBarFromDiscriminatorValue(parseNode: ParseNode | undefined) : ((instance?: Parsable) => Record<string, (node: ParseNode) => void>) {
	return deserializeIntoFooBar;
}

// Deserialization methods
export function deserializeIntoFooBar(fooBar: Partial<FooBar> | undefined = {}) : Record<string, (node: ParseNode) => void> {
    return {
        ...deserializeIntoFoo(fooBar as Foo),
        ...deserializeIntoBar(fooBar as Bar),
    }
}

export function deserializeIntoFoo(foo: Partial<Foo> | undefined = {}) : Record<string, (node: ParseNode) => void> {
    return {
        "foo": n => { foo.foo = n.getStringValue(); },
    }
}

export function deserializeIntoBar(bar: Partial<Bar> | undefined = {}) : Record<string, (node: ParseNode) => void> {
    return {
        "bar": n => { bar.bar = n.getStringValue(); },
    }
}


// Serialization methods
export function serializeFoo(writer: SerializationWriter, foo: Partial<Foo> | undefined = {}) : void {
    writer.writeStringValue("foo", foo.foo);
}

export function serializeBar(writer: SerializationWriter, bar: Partial<Bar> | undefined = {}) : void {
    writer.writeStringValue("bar", bar.bar);
}

export function serializeFooBar(writer: SerializationWriter, fooBar: Partial<FooBar> | undefined = {}) : void {
  serializeFoo(writer, fooBar as Foo);
  serializeBar(writer, fooBar as Bar);
}

Edge Cases

Union between objects and primitive values with no discriminator property specified

export type TestUnionObject = Foo | Bar | string | number;

export interface TestParser {
  testUnionObject?: TestUnionObject | undefined;
}

export function deserializeTestParser(testParser: TestParser | undefined = {}): Record<string, (node: ParseNode) => void> {
  return {
    testUnionObject: (n) => {
	testParser.testUnionObject = n.getStringValue() ?? n.getNumberValue() ?? n.getObjectValue(createTestUnionObjectFromDiscriminatorValue);
    },
  };
}

// Factory Method
export function createTestUnionObjectFromDiscriminatorValue(parseNode: ParseNode | undefined): (instance?: Parsable) => Record<string, (node: ParseNode) => void> {
  return deserializeIntoTestUnionObject;
}

// Deserialization methods
export function deserializeIntoTestUnionObject(fooBar: Partial<TestUnionObject> | undefined = {}): Record<string, (node: ParseNode) => void> {
  return {
    ...deserializeIntoFoo(fooBar as Foo),
    ...deserializeIntoBar(fooBar as Bar),
  };
}

export function serializeTestUnionObject(writer: SerializationWriter, fooBar: Partial<TestUnionObject> | undefined = {}): void {
	serializeFoo(writer, fooBar as Foo);
	serializeBar(writer, fooBar as Bar);
}

export function serializeTestParser(writer: SerializationWriter, entity: TestParser | undefined = {}): void {
  if (typeof entity.testUnionObject === "string") {
    writer.writeStringValue("testUnionObject", entity.testUnionObject);
  }
  else if (typeof entity.testUnionObject === "number" ) {
    writer.writeNumberValue("testUnionObject", entity.testUnionObject);
  } else {
    writer.writeObjectValue("testUnionObject", entity.testUnionObject as any, serializeTestUnionObject);
  }
}

@koros koros requested a review from a team as a code owner May 6, 2024 08:48
@koros koros marked this pull request as draft May 6, 2024 08:48
@rkodev rkodev marked this pull request as ready for review August 27, 2024 15:46
@koros
Copy link
Contributor Author

koros commented Aug 27, 2024

@rkodev Please also remember to refactor the composed type which comprises of primitive values only,
The factory methods I used won't work for primitive only values. My proposal is remove the factory methods together with serializers and deserializers.

then if the composed type field is property inside another object the existing logic will handle it e.g:

Deserializing

primitiveComposedType: (n) => {
    response.primitiveComposedType= n.getNumberValue() ?? n.getStringValue();
},

Serializing

switch (typeof response.primitiveComposedType) {
    case "number":
        writer.writeNumberValue("data", response.primitiveComposedType);
	 break;
    case "string":
        writer.writeStringValue("data", response.primitiveComposedType);
        break;
}

However if the composed type is at the root of the payload then sendPrimitive should be modified since basically there is no corresponding factory

@rkodev
Copy link
Contributor

rkodev commented Aug 27, 2024

@koros there are a number of changes that have been made

For serializing this is the syntax to check for types when its a collection or single value of primitives

        switch (true) {
            case typeof bank_account.account === "string":
                writer.writeStringValue("account", bank_account.account as string);
            break;
            case Array.isArray(bank_account.account) && bank_account.account.every(item => typeof item === 'number'):
                writer.writeCollectionOfPrimitiveValues<string>("account", bank_account.account);
            break;
            default:
                writer.writeObjectValue<Account>("account", bank_account.account as Account | undefined | null, serializeBank_account_account);
            break;
        }

as for the deserializing, here is an example of a composed type that will serialize objects / collection of objects or primitives

petGetResponse.data = n.getObjectValue<Cat | Dog>(createpetResponseFromDiscriminatorValue) ?? n.getCollectionOfObjectValues<Dog>(createDogFromDiscriminatorValue) ?? n.getNumberValue() ?? n.getStringValue()

it/config.json Show resolved Hide resolved
src/Kiota.Builder/CodeDOM/CodeFile.cs Outdated Show resolved Hide resolved
src/Kiota.Builder/Refiners/TypeScriptRefiner.cs Outdated Show resolved Hide resolved
@baywet
Copy link
Member

baywet commented Aug 28, 2024

@rkodev can you also look at the defects reported here (ignore the complexity ones) and address the ones you can before I give it another review please? https://sonarcloud.io/project/issues?id=microsoft_kiota&pullRequest=4602&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true

@rkodev rkodev changed the title Add support for composed types in Typescript feat: Add support for composed types in Typescript Aug 28, 2024
baywet
baywet previously approved these changes Aug 29, 2024
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @rkodev @koros @andrueastman and others for the long haul here!!!!
Please make sure we squash merge this PR as there's a lot of convoluted history.
🚀 🚢 :shipit:

@baywet
Copy link
Member

baywet commented Aug 29, 2024

@andrueastman FYI you still have two comments that are unresolved in the history. I didn't resolve them so you get a chance at checking whether they still apply

@rkodev rkodev enabled auto-merge (squash) August 29, 2024 13:54
Copy link

sonarcloud bot commented Aug 29, 2024

@rkodev rkodev merged commit aef4574 into main Aug 29, 2024
207 checks passed
@rkodev rkodev deleted the feature/typescript/composed-types branch August 29, 2024 15:10
Copy link
Member

@andrueastman andrueastman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrueastman FYI you still have two comments that are unresolved in the history. I didn't resolve them so you get a chance at checking whether they still apply

Confirmed these are resolved. Thanks for pushing this through @koros @rkodev

@baywet
Copy link
Member

baywet commented Aug 29, 2024

@rkodev I just noticed we forgot the changelog entry. Can you stand a quick pr please?

@rkodev
Copy link
Contributor

rkodev commented Aug 29, 2024

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

composed types serialization support for TypeScript
7 participants