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

Generate better schema types #341

Merged
merged 2 commits into from
Dec 4, 2023
Merged

Generate better schema types #341

merged 2 commits into from
Dec 4, 2023

Conversation

dani-garcia
Copy link
Member

@dani-garcia dani-garcia commented Nov 22, 2023

Type of change

- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

The quicktype schema generation seems to have trouble with types in schema files being referenced from other types in separate schema files, which created duplicate classes, with a lot of strange names.

Instead of generating one individual file for each response, we create a big struct with all the options. This seems to solve the issue of references between schema files, and so no duplicate classes are created.

Note that the way that the file is loaded in quicktype is different. By default, quicktype generates JSON conversion helper functions only for the root type, which would be the big struct SchemaTypes. By importing the file with #/definitions/ appended, we only bring the sub schemas as root types, and ignore SchemaTypes, which we don't care about.

Examples of the schema changes, using C# as an example:

  • ProjectsResponse previously contained an array of DatumElement, which were identical to ProjectResponse. Now it properly contains an array of ProjectResponse. This is also applicable to SecretsResponse which contained an array of DatumClass
  • Previously it would generate multiple variants for Argon2ID, named: FluffyArgon2Id and PurpleArgon2Id, now there is only one Argon2Id. This is applicable to the two factor response types, which change in the same way

This is working correctly with the Java, Go, C++, PHP and Ruby bindings.

@bitwarden-bot
Copy link

bitwarden-bot commented Nov 22, 2023

Logo
Checkmarx One – Scan Summary & Details22f67793-e167-46df-8332-f6e62451b4fd

No New Or Fixed Issues Found

Make sure itertools is only imported when used

Format
@dani-garcia dani-garcia marked this pull request as ready for review December 1, 2023 16:22
Copy link
Member

@Hinton Hinton left a comment

Choose a reason for hiding this comment

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

Did you verify the TS types to?

@dani-garcia
Copy link
Member Author

I did now and they work correctly, I also created a PR with some of the changes to get the TS bindings up to par with the rest of them #398

@dani-garcia dani-garcia merged commit f684cc9 into master Dec 4, 2023
40 checks passed
@dani-garcia dani-garcia deleted the ps/schema-gen branch December 4, 2023 19:14
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.

3 participants