kiota icon indicating copy to clipboard operation
kiota copied to clipboard

Customizing the Structure of Generated Client Code in Kiota

Open sander1095 opened this issue 5 months ago • 10 comments

Issue

Currently, Kiota generates client code based on the structure of the provided OpenAPI document. I have an API that looks like this:

/
 └─api
    └─talks
       └─{id}

This results in the following API client code:

var result = await kiotaClient.Api.Talks.GetAsync();

I find this a bit ugly. I'd prefer it to be kiotaClient.Talks.GetAsync(); because the Api doesn't add anything.

Is there a way to do this? Lots of API's have api/ in their path, so I don't think this is an exceptional case :)

My code can be found here: https://github.com/sander1095/openapi-code-generation-talk/tree/main/api-client-generation-demo

sander1095 avatar Jan 13 '24 22:01 sander1095

Just add the /api part to the server URL (aka base uri). Then your open API docs won't even show it. That is how I always configure my docs anyway, and that is also what Microsoft itself does with their Graph API

svrooij avatar Jan 13 '24 22:01 svrooij

Hey @svrooij, thanks for the quick reply!

I believe that you simply remove the api/ bit from the OpenAPI document this way, this feels a bit like cheating ;-). While clever, I believe this wouldn't work on a server that serves an API on api/ and other content in /client, for example. Like websites that return both a server-rendered client or an API for 3rd party clients.

This isn't the case for me right now, but I definitely believe this isn't uncommon. I'd like to see an option to tell Kiota to handle Api as a part of its internal base address so I don't need to touch this .Api. bit :)

sander1095 avatar Jan 13 '24 22:01 sander1095

@sander1095 What @svrooij suggested definitely is not cheating. It is a core philosophy of Kiota that the projected language experience is as close a match to the HTTP API description as possible. While I understand that it is appealing to many people to want to tweak the API surface to "fix" it client libraries, we believe that introducing these differences lead to confusion in developer experiences.

Having said all of this.... Kiota is designed to allow generation of clients for just subsets of an API. It would be possible to allow a parameter that defines a new base URL for the client to use. e.g. Consider Twilio's API image

It would be possible to only include paths /2010-04-01/* and redefine the base path to include the 2010-04-01 so that it would not be a property of the client. That would be convenient. However, it means that if someone rebuilds the client with endpoints that have resources from a later version, it would be a breaking change to the client.

I do think this is worth having a conversation about for this particular conversation. It is particularly interesting for Azure APIs that have /subscriptions/{subscriptionId}/Microsoft.SomeProvider as a prefix to many of their API endpoints.

darrelmiller avatar Jan 13 '24 22:01 darrelmiller

Hi @darrelmiller, thanks for your reply.

While I understand that it is appealing to many people to want to tweak the API surface to "fix" it client libraries, we believe that introducing these differences lead to confusion in developer experiences.

Honestly, I find the current experience to be more "confusing". I see the api/ purely as a prefix to access the real API, which is talks in my case. :) Of course this is just my opinion ;).

FYI: Whilst I do not really want to do this, another workaround would be to create a wrapper around the client like so:

class ApiClientWrapper(KiotaApiClient apiClient)
{
    public ApiRequestBuilder Talks => apiClient.Api;
}

var kiotaApiClient = GetKiotaApiClient();
var apiClientWrapper = new(kiotaApiClient);

// Now we avoid the .Api. property in actual Kiota usage.
var talks = apiClientWrapper.Talks.GetAsync();

I'm not the biggest fan of this custom approach, mostly because I'm used to using NSwag for my API Client generation, and these issues do not exist there because it simply generates a class with the API calls (and api/ path) in it.

I'd love to have this conversation that you mention. From my point of view, I think it would be worth having a flag like --internalizePrefix "api/" (or something) so it would simply call api/ in the generated Kiota code. Clients would then choose for this behavior and would not be confused. The top-level objects (In my case, Talks) could even have some XML comments that explain that this is api/Talks under the hood because of client generation options, so things are clearer for devs.

This would be similar to NSwag's behavior, where the api/ is internalized: https://github.com/sander1095/openapi-code-generation-talk/blob/9a5ae8999d32b29a3a8c73a4171640d81fd2a95a/api-client-generation-demo/ConferenceApp/Clients/NSwag/GeneratedApiClient.cs#L108

sander1095 avatar Jan 13 '24 23:01 sander1095

Hi @sander1095,

If we added a list of prefixes to ignore when projecting the fluent API, this brings two new problems:

  • What should the list be (everyone has different opinions)? which leads to another command line parameter, which makes kiota more complex to use.
  • What happens when a sub path segment conflicts with a root path segment? (e.g. description has both api/talks and /talks for some reason)

As for the fluent API aspect (when comparing with NSwag), it's one of the core experience aspects of kiota, so kiota clients work great regardless of the shape of the API. Flattening clients works ok for APIs that don't have a lot of endpoints and/or a lot of nesting. But for the many cases of APIs that aren't designed this way, it becomes a nightmare (e.g. /users/id/conversations/id/members/id/profile/picture become a client.GetUsersIdConversationsIdMembersIdProfilePictureAsync if we don't want it to conflict with other APIs, which in my opinion is harder to parse than client.Users["id"].Conversations["id"].Members["id"].Profile.Picture.GetAsync() where you know a "dot" on the fluent API call roughly maps to a slash in the path segmentation.

baywet avatar Jan 15 '24 14:01 baywet

@baywet

What should the list be (everyone has different opinions)? which leads to another command line parameter, which makes kiota more complex to use.

The list would not be decided by kiota. A developer could set these up when using kiota generate like this: --internalizePrefix "api/".

What happens when a sub path segment conflicts with a root path segment? (e.g. description has both api/talks and /talks for some reason)

This can be found out during code generation, so an error can be thrown.

(e.g. /users/id/conversations/id/members/id/profile/picture become a client.GetUsersIdConversationsIdMembersIdProfilePictureAsync if we don't want it to conflict with other APIs

NSwag handles this in a smart way by allowing users to decide the method name in multiple ways, like path segments, tags, operationid, etc.. I understand that the method name would be vague, but that would also be the case for the server implementation of this method :)

sander1095 avatar Jan 23 '24 07:01 sander1095

The additional argument solution (with no defaults) could be a valid approach. My concern with the current "design" is people will want another one for the suffix as well, and soon after one to remove segments in the middle as well. Which is a slippery slope all together.

As for throwing an exception, even assuming we format it properly with an explicit error message, I'm not sure how that's going to improve the experience.

I'll let @sebastienlevert lean on this one as the PM, but from my perspective there's already a workaround with the base URL. Although I understand how painful it can be to maintain those change in sync if you're not the API producer as well.

baywet avatar Jan 23 '24 15:01 baywet

I share sentiment with @baywet on this scenario, but I really think this is an important one to support. Though, having yet another configuration shouldn't be how we move forward with this. This will become unmanageable for generating clients in the future and we should think broader. Where I think there is an opportunity is to leverage OpenAPI overlays to let developers simply override the pieces they need. There is a lot of things to think about with overlays but it's something we're already envisioning for the future of Kiota. If a developer was providing a super simple overlay, they could override the server url without the need to control the OpenAPI description and would be a good and standard experience.

overlay: 1.0.0
servers:
  - url: "http://localhost:4010/api"
    description: Local development server

Adding @darrelmiller to share thoughts, but I feel this is the way to go.

sebastienlevert avatar Jan 23 '24 17:01 sebastienlevert

Could this issue be reopened until a conclusion is reached?

sander1095 avatar Jan 30 '24 20:01 sander1095

I see the tag "author feedback needed" is added. i don't have experience with openapi overlays. I think it could be interesting, however I also think that lots of developers generate their openapi definition instead of designing it upfront, so i dont know how many generators will start utilizing these overlays.

But, anything that makes use of the spec is a plus point for me!

sander1095 avatar Jan 30 '24 20:01 sander1095