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

Spike: OpenAPI spec lists all fields as optional #4775

Closed
jhrozek opened this issue Oct 17, 2024 · 3 comments
Closed

Spike: OpenAPI spec lists all fields as optional #4775

jhrozek opened this issue Oct 17, 2024 · 3 comments
Assignees

Comments

@jhrozek
Copy link
Contributor

jhrozek commented Oct 17, 2024

In Protov3 all fields are optional by default, but that means that in our OpenAPI spec, all fields in requests and responses are optional as well. This is causing trouble for the TypeScript generated clients as they need to do custom validation.

It seems like we could add a custom option and annotate the required fields in the proto file, but I don't know if buf has the option to generate the required fields based on custom options. Needs more research.

@jhrozek jhrozek added the bug Something isn't working label Oct 24, 2024
@mesembria
Copy link
Contributor

This will be spike. A new ticket will be created to implement the solution.

@eleftherias eleftherias self-assigned this Oct 28, 2024
@eleftherias eleftherias changed the title OpenAPI spec lists all fields as optional Spike: OpenAPI spec lists all fields as optional Oct 28, 2024
@eleftherias eleftherias removed the bug Something isn't working label Nov 6, 2024
@eleftherias
Copy link
Contributor

After looking into this, my recommendation would be to use the field_behaviour flag.

By setting the behaviour to OUTPUT_ONLY, the typescipt generated field is readonly. For example:

message Provider {
    // ...
    string project = 2 [(google.api.field_behavior) = OUTPUT_ONLY];
}

becomes:

export type v1Provider = {
    readonly project?: string;
}

By setting the behaviour to REQUIRED, the typescipt generated field is non-optional. For example:

message Provider {
    // name is the name of the provider.
    string name = 1 [
        (google.api.field_behavior) = REQUIRED
    ];

produces the following typescipt type (note that it does not have a ? marking it as optional):

type v1Provider = {
    /**
     * name is the name of the provider.
     */
    name: string;
}

As a result, we can annotate with REQUIRED all of the Minder field that will never be empty in the response and will always be required in a request.

One limitation is that there is no annotation for fields that are required for requests, but not always populated in responses. If we wanted to work around that case, we would need to create different proto messages for those in the request and those in the response. For example, we could create InputProvider and OutputProvider, instead of using the same Provider for requests and responses.
I don't think this is necessary for the time being.

@eleftherias
Copy link
Contributor

Created #4952 and #4951 as outputs of this spike

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

No branches or pull requests

3 participants