Skip to content

Add a Make target to expand generics in schema.json #1763

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

Merged
merged 2 commits into from
Jun 24, 2022
Merged

Conversation

swallez
Copy link
Member

@swallez swallez commented Jun 15, 2022

This PR adds a new make transform-expand-generics target.

This target transforms schema.json to expand all generic types to concrete types and remove all generic parameters. These concrete types can have two forms:

  • for "internal generic types" that are instanciated by other specification types (e.g. MultiBucketAggregateBase<TBucket>), a new type is generated for every instance of this type, with the value of generic parameters replaced with the concrete value defined by the instanciating type.
  • for "external generic types", i.e. requests and response with a user-provided generic parameter (e.g. SearchRequest<TDocument>), the generic parameters are replaced by UserDefinedData.

Currently make transform-expand-generics creates a new output/schema/schema-no-generics.json file, but ultimately we should be able to specify the input and output paths as arguments.

This transformation is the first of its kind, and is meant to produce a schema that is easier to consume by some code generators without removing features from the specification that would be useful for some other languages. It's the result of a discussion with @Anaethelion about the Go client generator, where generics bring significant complexity, but should be kept in the spec for languages with generic support.

Other transformations may be of interest, such as removing inheritance by expanding fields from a super class into its child classes.

Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

I'm happy with this idea of transforming the schema.json, leaving @Anaethelion to review whether the implementation is effective for the Go client.

Copy link
Contributor

@Anaethelion Anaethelion left a comment

Choose a reason for hiding this comment

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

Works beautifully, the generated code seems to match the API as far as my tests went.
LGTM

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

Successfully merging this pull request may close these issues.

3 participants