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

chore: token api simplification #4600

Merged
merged 3 commits into from
Sep 4, 2023
Merged

chore: token api simplification #4600

merged 3 commits into from
Sep 4, 2023

Conversation

gastonfournier
Copy link
Contributor

@gastonfournier gastonfournier commented Sep 4, 2023

About the changes

We found a problem generating the Go SDK client for the tokens API that makes use of oneOf, combined with allOf. The generator doesn't know how to map this type and leaves it as an object. So we tested other generators such as Oval. In this case, Orval manages to create the type but it looks ugly and complex:

export type CreateApiTokenSchema =
| (CreateApiTokenSchemaOneOf & { expiresAt?: string; })
| (CreateApiTokenSchemaOneOfFour & { expiresAt?: string; })
| (CreateApiTokenSchemaOneOfSeven & { expiresAt?: string; })
| (CreateApiTokenSchemaOneOfOnezero & { expiresAt?: string; });

// below are all the one of options. All of is translated to & type combination
export type CreateApiTokenSchemaOneOf = CreateApiTokenSchemaOneOfAllOf & CreateApiTokenSchemaOneOfAllOfTwo;
export type CreateApiTokenSchemaOneOfFour = CreateApiTokenSchemaOneOfFourAllOf & CreateApiTokenSchemaOneOfFourAllOfTwo;
export type CreateApiTokenSchemaOneOfSeven = CreateApiTokenSchemaOneOfSevenAllOf & CreateApiTokenSchemaOneOfSevenAllOfTwo;
export type CreateApiTokenSchemaOneOfOnezero = CreateApiTokenSchemaOneOfOnezeroAllOf & CreateApiTokenSchemaOneOfOnezeroAllOfTwo;

// And then each one individually (note the generated code creates equal types with different names):
export type CreateApiTokenSchemaOneOfAllOf = { type: string; };
export type CreateApiTokenSchemaOneOfAllOfTwo = { tokenName: string; };
export type CreateApiTokenSchemaOneOfFourAllOf = { type: string; };
export type CreateApiTokenSchemaOneOfFourAllOfTwo = { username: string; };
export type CreateApiTokenSchemaOneOfSevenAllOf = { type: string; environment?: string; project?: string; projects?: string[]; };
export type CreateApiTokenSchemaOneOfSevenAllOfTwo = { tokenName: string; };
export type CreateApiTokenSchemaOneOfOnezeroAllOf = { type: string; environment?: string; project?: string; projects?: string[]; };
export type CreateApiTokenSchemaOneOfOnezeroAllOfTwo = { username: string; };

Instead of defining allOf we changed the definition and combined the schemas at runtime. With the change the schema looks much better and this unblocks the generator of Golang SDK:

export type CreateApiTokenSchema =
| (CreateApiTokenSchemaOneOf & { expiresAt?: string; })
| (CreateApiTokenSchemaOneOfTwo & { expiresAt?: string; })
| (CreateApiTokenSchemaOneOfThree & { expiresAt?: string; })
| (CreateApiTokenSchemaOneOfFour & { expiresAt?: string; });

export type CreateApiTokenSchemaOneOf = { type: string; tokenName: string; };
export type CreateApiTokenSchemaOneOfTwo = { type: string; username: string; };
export type CreateApiTokenSchemaOneOfThree = { type: string; environment?: string; project?: string; projects?: string[]; tokenName: string; };
export type CreateApiTokenSchemaOneOfFour = { type: string; environment?: string; project?: string; projects?: string[]; username: string; };

Iterating on this idea, we can include expiresAt as well, and we end up with this:

export type CreateApiTokenSchema =
| CreateApiTokenSchemaOneOf
| CreateApiTokenSchemaOneOfTwo
| CreateApiTokenSchemaOneOfThree
| CreateApiTokenSchemaOneOfFour;

export type CreateApiTokenSchemaOneOf = { expiresAt?: string; type: string; tokenName: string; };
export type CreateApiTokenSchemaOneOfTwo = { expiresAt?: string; type: string; username: string; };
export type CreateApiTokenSchemaOneOfThree = { expiresAt?: string; type: string; environment?: string; project?: string; projects?: string[]; tokenName: string; };
export type CreateApiTokenSchemaOneOfFour = { expiresAt?: string; type: string; environment?: string; project?: string; projects?: string[]; username: string; };

This is closer to what the interface should look like, while the other result is just a consequence of following DRY (avoid duplicated code) and not resolving those references at compile/runtime. Because of that, the references leak to the schema, resulting in additional complexity.

This is what the OpenAPI UI looks like after the change
image

@vercel
Copy link

vercel bot commented Sep 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Sep 4, 2023 1:46pm
unleash-monorepo-frontend ⬜️ Ignored (Inspect) Visit Preview Sep 4, 2023 1:46pm

Copy link
Member

@sighphyre sighphyre left a comment

Choose a reason for hiding this comment

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

❤️

@gastonfournier gastonfournier merged commit 19ec66a into main Sep 4, 2023
@gastonfournier gastonfournier deleted the simplify-token-schema branch September 4, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants