Handle required query string and headers in Java

[piotrooo]

[!NOTE] First of all, many thanks for your work on the library. It could be very important for a lot of projects! :pray:

Please notice one huge disclaimer when I wrote this issue.

[!IMPORTANT] The purpose of generating API clients is so that you can provide them to completely different teams, and they should be as intuitive as possible for them to use - even if they don't know exactly how to use it. So in some situations - generated API clients - must 'think' for the API consumers.

Let's say we have the following OpenAPI definition:

we have one required header (X-Client) and one required query (token).

Right now I'm consuming this API in that way:

TemporaryFileResponse temporaryFileResponse = temporaryFilesServiceClient
        .api()
        .temporaryFiles()
        .get(getRequestConfiguration -> {
            getRequestConfiguration.headers.add("X-Client", "acme");

            getRequestConfiguration.queryParameters.token = "token-123";
        });

A couple of improvements could be done here (*):

(*) all examples are cleaned for unnecessary code for brevity

1. Required query shouldn't be @Nullable

Definition of the GetQueryParameters is:

public class GetQueryParameters implements QueryParameters {
    @jakarta.annotation.Nullable
    public String token;
}

That definition of the query parameter isn't valid because it is required.

---

2. Headers should be available like query parameters

Headers defined in OpenAPI Spec should be available as query parameters for auto-completion purposes and to help better understand how the API works. Using following example:

getRequestConfiguration.headers.xClient = "acme";

---

3. queryParameters shouldn't be @Nullable

Instead of this the empty QueryParameters object should be used.

public class GetRequestConfiguration extends BaseRequestConfiguration {
    @jakarta.annotation.Nullable
    public GetQueryParameters queryParameters = new GetQueryParameters();
}

This helps with following warnings:

---

4. Additional check for required query and header

When the query and the header are required, there should be an additional check before serialization to ensure that they aren't null.

---

I hope :pray: these ideas make sense for you. If something needs more clarification - let me know.

[baywet]

Hi @piotrooo Thanks for using kiota, the praises, the detailed issue, and your patience.

First off, around nullability, I can see that you've already submitted a PR, thanks! https://github.com/microsoft/kiota-java/pull/1149

Then, here are a couple of answers to your questions.

The headers are not being projected to their own type due to a mix of a size concern and a concern about the developer experience of an object that'd be mixed between properties and a map behaviour.

Both the examples of headers you've provided look like they are cross-cutting (required for all or a majority of operations on the API surface). For that, instead of specifying things on a per request base we suggest:

• using the API Key Authentication provider for the token header
• defining your own middleware handler for the client header

Besides avoiding duplication of code, this approach will allow that initialization to live where your application configuration/initialization happens, separate from where the requests are made.

Let us know if you have further questions.

[piotrooo]

I'm not sure if I understand you correctly, but creating manually a middleware per request is not what you want from a code generator. The variable nature of the OpenAPI spec disables the generation of reusable code.

Let's consider the following scenario:

GET /api/first-request, which has the following headers: HeaderOne, HeaderTwo POST /api/second-request, which has the following headers: HeaderTwo, YetAnotherHeader

And so on for other endpoints with different, sets of request-defined headers.

This could be described using the following OpenAPI Spec.

OpenAPI Spec

``` paths: /api/first-request: get: operationId: get parameters: - name: HeaderOne in: header required: true schema: type: string - name: HeaderTwo in: header required: true schema: type: string responses: '204': description: No Content /api/second-request: post: operationId: post parameters: - name: HeaderTwo in: header required: true schema: type: string - name: YetAnotherHeader in: header required: true schema: type: string responses: '204': description: No Content ```

Creating middleware is really inconvenient. If I have to manually create some parts of the OpenAPI Spec, why would I need a generator? I can do this manually.

[baywet]

in that scenario, yes providing headers at request time makes sense. Or you could choose to build two separate clients grouped by headers depending on how many endpoints are impacted by any given header. Let us know if you have further questions/comments.

[piotrooo]

Or you could choose to build two separate clients grouped by headers depending on how many endpoints are impacted by any given header.

Could you elaborate on it? Or show an example? I don't understand the purpose of having two clients; it seems to be very odd to me. I want to simplify things, not complicate them further with another client. This is a reason why I want to use a generator: to simplify things. Moreover, I have all the necessary information in the OpenAPI Spec.

If you consider the nature of the defined headers in the OpenAPI Spec, you can easily compare them to query parameters. Even though the section is labeled parameters in the OpenAPI Spec, the only difference lies in the in property: for headers, it's header, and for queries, it's query.

---

I play around a little bit with the desired (?) API for request configuration. Couple of ideas:

1. Like queryParameters

(which I personally don't like)

get(requestConfiguration -> {
    requestConfiguration.headers.headerOne = "first-header";
    requestConfiguration.headers.headerTwo = "second-header";

    requestConfiguration.queryParameters.token = "1234qwerty";
});

2. RequestConfiguration with all described parameters

[!NOTE] Here, we need also to implement nullable checks because some parameters must be set (as required fields).

The configuration class:

public class TemporaryFilesRequestConfiguration implements Consumer<RequestInformation> {
    private final List<Consumer<RequestInformation>> configurers = new ArrayList<>();

    public static TemporaryFilesRequestConfiguration temporaryFilesRequestConfiguration() {
        return new TemporaryFilesRequestConfiguration();
    }

    public TemporaryFilesRequestConfiguration headerOne(String value) {
        Consumer<RequestInformation> configurer = (requestInformation) -> {
            requestInformation.headers.tryAdd("HeaderOne", value);
        };
        configurers.add(configurer);
        return this;
    }

    public TemporaryFilesRequestConfiguration xClient(String value) {
        Consumer<RequestInformation> configurer = (requestInformation) -> {
            requestInformation.headers.tryAdd("X-Client", value);
        };
        configurers.add(configurer);
        return this;
    }

    public TemporaryFilesRequestConfiguration token(String value) {
        Consumer<RequestInformation> configurer = (requestInformation) -> {
            requestInformation.getQueryParameters().put("token", value);
        };
        configurers.add(configurer);
        return this;
    }

    public void accept(RequestInformation requestInformation) {
        configurers.forEach(consumer -> consumer.accept(requestInformation));
    }
}

Usage:

TemporaryFileResponse temporaryFileResponse = sampleClient
        .api()
        .temporaryFiles()
        .get(temporaryFilesRequestConfiguration()
                .headerOne("value1")
                .xClient("value2")
                .token("value3")
        );

It's handy. It covers all the query parameters and headers cases. Moreover, we can easily extend these configurations with cookie support.

[baywet]

Thank you for the suggestion. This approach presents a couple of challenges:

• what if token is both defined in headers and query parameters? (the symbols will collide)
• what if the user wants to provide a request header that's not documented in the description? (arguably this one is easier to fix with a fluent method "addHeader")

Could you elaborate on it?

Usually, cross cutting concerns like telemetry, authentication, etc... are better handled as an authentication provider and/or a middleware handler as I explained in a previous reply. The "multiple clients approach" comes handy when you have "functional partitions" of the API surface e.g.:

• This set of endpoints are for CRUD operations, they work with OAuth and require this random telemetry header1.
• This other set of endpoints are for migration scenarios, they work with API keys and require a different telemetry header2.

You could generate two different clients:

• the CRUD client, augment it with a middleware to add the header1 on every request, and provide it an OAuth authentication provider.
• the migration client, augment it with a middleware to add the header2 on every request, and provide it with an API Key authentication provider.

This way the auto-completion will ONLY suggest operations that are valid for the scenario, and you don't need to provide the headers on every request.

I hope that makes things clearer?

[piotrooo]

@baywet thanks for answering. I believe this conversation could produce something good :+1:. That's a constructive flame :wink:

• what if token is both defined in headers and query parameters? (the symbols will collide)

My example was just an idea. Of course, we can discuss the desired API. Perhaps we could include a discriminator header/query to methods, or maybe add another nesting level. Other users might have preferences, so we could ask them as well.

• what if the user wants to provide a request header that's not documented in the description? (arguably this one is easier to fix with a fluent method "addHeader")

Yes, maybe configurers should have a base class with that kind of "utility" methods. I like this idea.

This way the auto-completion will ONLY suggest operations that are valid for the scenario, and you don't need to provide the headers on every request.

Now I understand (I hope) what you mean by using middleware. Middleware for fixed values is ok (e.g., Authentication, Trace-Id, etc). The described case with the X-Client header provides the possibility to include headers in every request (which is my case). I can imagine that many users may tailor their API to utilize dynamically valued headers.

Moreover, if something is in the OpenAPI Spec (headers, cookies, query parameters), it should be reflected in the generated code.

[baywet]

The other challenge with this approach (not using a lambda/callback for the configuration) is it requires to add an additional import. We've already received the feedback this can be challenging to users with deeply nested APIs.

[piotrooo]

We've already received the feedback this can be challenging to users with deeply nested APIs.

It's too broad. I don't know how to respond to that.

Perhaps I should ask - @baywet, what other information do you need? I believe I've provided you with a specific case, benefits, and an example of how it could look. I don't have any further arguments.

[baywet]

I don't need additional information at this point. Thanks for the open discussion. I'll point out that as this is a breaking change, if we implement it, we're likely to do so only in the next major version (no timelines yet), and we might consider it across languages. If @sebastienlevert @maisarissi @andrueastman and @andreaTP want to provide additional input, they are welcome to do so.

[andreaTP]

+1 from me, this was slightly related: https://github.com/microsoft/kiota/issues/2428 but the proposed builder pattern, e.g.:

TemporaryFileResponse temporaryFileResponse = sampleClient
        .api()
        .temporaryFiles()
        .get(temporaryFilesRequestConfiguration()
                .headerOne("value1")
                .xClient("value2")
                .token("value3")
        );

Looks extremely appealing!

[maisarissi]

I think we should consider this for 2.0 and across languages!