-
Notifications
You must be signed in to change notification settings - Fork 217
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
How to pass a free-format query string? #3800
Comments
Hi @bkoelman
Examples of the first category include sort, filter, page (size, offset, limit...). There should be described as first class query parameters in the OpenAPI description so everyone relying on OpenAPI with their tooling can benefit from it. For the second category, I guess it depends on how this description gets created. If there's a single description for all instances of JSON API, then it's going to be hard to add them without complex layering. If the JSON API instance can generate the description from the underlying data, it could also include those other query parameters in the description, and a client should be generated from this "augmented" description instead. Regardless, I believe parameters you added as part of your initial comment is wrong. I think the intent here was to describe "we can have any query parameter of type string". But this actually describes "there's one query parameter called query, which can have any property of type string". In addition to this wrong aspect of the description, Kiota does NOT support object query parameters as their serialization conventions are not standardized. Lastly, for the dynamic query parameter, you could use your own class even though it's kind of a hack // example from https://jsonapi.org/format/#fetching-sparse-fieldsets
public class CustomQueryParameters: BazGetQueryParameters { //alternatively, this could inherit from the default query parameters if no class is generated
[QueryParameter("fields[articles]")]
public string ArticleFields {get; set;}
}
var result = await client.Foo.Baz.GetAsync(x => {
x.QueyParameters = new CustomQueryParameters {
ArticleFields = "title"
}
}); |
Hi @baywet, thanks for your response.
Unfortunately, it's not that simple. When I first came in contact with JSON:API, I found it hard to fully understand. Since then, I've been a maintainer of JsonApiDotNetCore (the most popular server implementation in .NET, there are many in various languages) for several years, have contributed major parts of it and answered hundreds of questions from users. By now, I'm pretty confident I know it very well, so please allow me to help you understand the problem. JSON:API defines a format for REST APIs, conceptually similar to OData and GraphQL. At its core, an API consists of resource types and fields, where a field is either an attribute or a relationship. Resource types can be compared to database tables, attributes to their columns, and relationships to foreign keys (roughly). In EF Core terminology, a resource is an entity, an attribute is an entity property or scalar value, and a relationship is a navigation. JsonApiDotNetCore enables developers to expose an EF Core model through JSON:API by annotating their EF Core models with Consider the following EF Core entity, which has been annotated for JSON:API: public class Human : Identifiable<long>
{
[Attr]
public string Name { get; set; } = null!;
[HasOne]
public Human? Father { get; set; }
[HasMany]
public ISet<Human> Children { get; set; } = new HashSet<Human>();
} Based on the model above, JSON:API enables to side-load related resources, for example: GET /api/humans/1?include=children,father.father.children HTTP/1.1 which returns a compound document that contains (assuming data exists):
This request can be combined with filters at multiple depths (targeting relationship chains ending in a to-many relationship), for example: GET /api/humans/1
?include=children,father.father.children
&filter[children]=equals(name,'Joe')
&filter[father.father.children]=equals(name,'Jack') HTTP/1.1 The difference with the previous request is that this one filters on the sets from the 2nd and last bullet of the returned compound document. As you can see, the query string parameter name ( The OAS generation logic within JsonApiDotNetCore lives in a separate branch, built by a former team member using lots of Swashbuckle extensions, but the prototype was never completed. As you may have realized by now, we already tailor the produced OAS to the specific models being used (except for query strings). I'm firm on completing the OAS implementation, because OAS support is our number-one feature request. Though I'm a bit concerned that our existing integration tests using NSwag are leading to a dead end ultimately, because the primary maintainer of NSwag isn't active anymore. So I thought I'd give Kiota a try. Its code looks clean, is documented and the project is active. Using query strings was just the first issue I ran into, and I doubt it will be the only one. I'm curious if we can work together and find ways to address them. If all goes well, I'd be happy to drop support for NSwag in favor of Kiota, but it's too early for such a decision.
Yes, that's the intent. I'm still learning OAS (read your series, which is great). The OAS fragment in the first post originates from a stack overflow answer on how to do it with NSwag, which appears to work. I haven't found any other way to make it work with NSwag, which is what we use today. As you said, many things are not standardized in OAS. Would you consider special-casing this OAS structure in Kiota for the sake of compatibility? |
Hey @bkoelman,
Absolutely.
Probably not, here is the rationale for it: Kiota is built by the Microsoft Graph client experience team. Microsoft Graph itself is an OData-ish service. When we started kiota, one of our core principle was to make sure it had "nothing to do" with OData, so it could be used by the broader base of the OpenAPI ecosystem. All the OData conventions are pushed to OpenApi.net.OData. Maintaining that commitment through the years (which was hard at times) has two major benefits: kiota can be used against any REST API with an OpenAPI description, the OData to OpenAPI conversion story got much better. If we look at the value of code generation of clients for REST APIs, it is mostly to "guide" client application developers through auto-completion, compile-time validation, etc... I'm not sure how a blanket "this endpoint accepts any query parameter" helps anyone here. (which was the intent of the snippet you provided). Instead if we look at that request GET /api/humans/1
?include=children,father.father.children
&filter[children]=equals(name,'Joe')
&filter[father.father.children]=equals(name,'Jack') HTTP/1.1 Maybe we could stage things instead of trying to address it all at once, and drive things based off the community demands. I think there are now 3 cases:
Here my suggestion is to implement 1 and 2 (and get 3 for free). For those limitations, we could consider adding an "Arbitrary map of query parameters" in the base query parameters class but only after scenario 1 and 2 have been implemented and if there's strong customer demand for it. The reason why I'm reticent to add it by default is because I'd rather have people improve and fix their descriptions (or reach out to the API provider to do so) than having them use this as an escape hatch all the time and loose some of the value kiota can provide for them. Also, kiota makes use of RFC 6570, and if the query parameter is not described in the URI template, the passed value will be ignored during expansion, so we'd have to somehow "inject" those arbitrary parameters into the template, which could lead to reliability trade-offs. What do you think of this plan? |
Kiota depends on URI Templates (RFC 6570) for resolving URLs. The way to have dynamic query parameter names with URI templates is to use a parameter that is a map and "explode" it. e.g.
If the params value is a dictionary then the keys of the dictionary become query string parameter names and the values in the dictionary become query parameter values. I think we could squint an describe this in OpenAPI. We need it described in OAS before we can address the issue with Kiota. |
That makes sense. You've convinced me that's the better decision for Kiota. Is there a way to plug this into Kiota through an external NuGet package? I'm asking because both NSwag and SwaggerUI work with this syntax.
Yes, discovery and documentation are the primary reasons we started the OpenAPI effort. My initial thought was to apply the same principle for query string parameters. Until I started implementing it. The difficulty is not in traversing the relationship graph or detecting recursion. JsonApiDotNetCore already has a setting to constrain the maximum inclusion depth (amongst other constraints), which enables API owners to keep database pressure under control. But the default is unconstrained, for backward compatibility. Here's the syntax for the query string parameter names:
Consider the following model, which we use in our samples:
If we'd generate all the possible query string parameter names, assuming a maximum inclusion depth of 3, it would result in:
As can be seen above, it gets completely out of control, even in an unrealistically small sample API with only 3 resource types and 6 relationships. Already with this sample, SwaggerUI becomes completely useless (users need to scroll forever). So in my opinion, pointing users to the mechanism of how these parameters work (see the screenshot in json-api-dotnet/JsonApiDotNetCore#1378) is the better solution, from a usability perspective. I wonder how helpful an auto-completion list with thousands of entries would be, aside from the increased generation and compilation effort. We've only covered the parameter names. The parameter values for filter/sort take compound expressions. My plan is to release untyped query string parameters in the MVP. Then offer additional building blocks (in the form of JSON:API client packages) to compose these parameters and expressions in future releases. The primary goal is to enable OAS, gather feedback, and then improve the experience.
The first version of JsonApiDotNetCore had many undocumented limitations, such as "filtering only works one level deep" and "sorting does not work on nested endpoints, such as
In our case, API providers cannot fix their descriptions, because we auto-generate them. If developers decide to use this as an escape hatch, I'm going to assume they do that intentionally (like me). It's still better than not using Kiota or even OpenAPI at all. Your sentiment, though well intended, comes across to me as treating developers as lazy or stupid: "We know what's best for you, so we won't let you." I believe a great framework should empower developers, not try to hold them back. It's appropriate that it takes extra effort to do something that's not recommended, though. So I'm okay if this feature is disabled by default, and logs a warning when used (that can be turned off). I'd be interested in the OAS shape that Kiota needs to potentially enable free-format parameters, so I can verify if SwaggerUI understands it and see what NSwag makes of it. |
What I forgot to mention is that JsonApiDotNetCore has a setting to allow unknown query string parameters to be passed through. When enabled (false by default), API developers can implement a hook to handle their custom parameters. Some devs use it to expose functional-style filters, for example: |
Thank you for the detailed analysis of paths here. I agree with you that'd be verbose while still posing some limitations. From your example, it seems the only "reasonable" depth for code generation would be to support only 1 level of depth. Which would cause severe limitations if support arbitrary query parameters is not provided. Question about the hooks: are they registered somewhere? meaning do you have a way to enumerate them so they could also be projected to query parameters?
I'd like to address this comment head-on so there's no misunderstanding here. The intent is never to be pedantic and shouldn't be perceived as so. But rather to try and iterate on solution designs that lead to better practices across the ecosystem. The thinking process is never "I know better than the rest of you" but rather "let's design together solutions that are practical from an experience perspective while being efficient for the industry". Hopefully this clarifies the tone of some of my comments for future references, as async communications can be lossy at times. 🙂 @darrelmiller considering the RFC 6570 supports the explode scenario with maps, and considering that OAS already support describing the following snippet (the name of the parameter doesn't matter here) "parameters": [
{
"name": "query",
"in": "query",
"description": "...",
"schema": {
"type": "object",
"additionalProperties": {
"type": "string",
"nullable": true
},
"example": null
}
}
] Do you see any reason why we could project this as [QueryParameter("query")]
public Dictionary<string, string>? Query { get; set; } and
With usage looking like var result = await client.Foo.Bar.GetAsync(x => x.QueryParameters.Query = new() { {"filter[ownedTodoItems.owner.ownedTodoItems]" : "equals(priority, 'high')" }}); Which would result in GET https://localhost/foo/bar?filter[ownedTodoItems.owner.ownedTodoItems]=equals(priority, 'high') |
If someone creates a parameter that is style=form and explode=true and defines a schema that is an object, then yes, we definitely could project that as a dictionary<string,string>. We could do better than Dictionary<string,string> if we use the style "deepObject". OAS was never able to be explicit as to how to serialize a "deepObject" in a query string. However, we could have a default opinion that looks like what JSONAPI does and use an extension for alternative serializations if they show up. |
Thanks for the additional details @darrelmiller "parameters": [
{
"name": "query",
"style": "form", // what do we do with other styles in that scenario?
"explode": true, // what do we do if it's false?
"in": "query",
"description": "...",
"schema": {
"type": "object",
"additionalProperties": { // my assumption was that if additionalProperties is absent here, we project as a "regular property" (i.e. not a map)
"type": "string", // what do we do if this is anything else than scalar? should we implement scalar types only? as a first stop-gap?
"nullable": true
},
"example": null
}
}
] |
Unfortunately, no. It is a virtual method that users can override, which executes at request time. So the set may not be constant, but depend on the current endpoint, the logged-in user, etc.
Thanks, no worries.
Here's our code to produce that: operation.Parameters.Add(new OpenApiParameter
{
In = ParameterLocation.Query,
Name = "query",
Style = ParameterStyle.Form, // added for Kiota
Explode = true, // added for Kiota
Schema = new OpenApiSchema
{
Type = "object",
AdditionalProperties = new OpenApiSchema
{
Type = "string",
Nullable = true
},
Example = new OpenApiNull()
},
Description = "..."
}); which uses the object model from https://www.nuget.org/packages/Microsoft.OpenApi to render the OAS. For reasons unknown to me, the resulting OAS file differs from your proposal in that Other than that, I can confirm this structure works with both SwaggerUI and NSwag. |
Thanks for sharing your progress here. |
Created microsoft/OpenAPI.NET#1489. |
Due to the dynamic nature of JSON:API, which I'm trying to generate a C# client for, we need the query string parameters to be unconstrained. Is there a way to do that with Kiota?
The C# Web API produces the following OAS, which results in an
IDictionary<string, string?> query
method parameter using NSwag:When passing the following value to the NSwag-generated method:
it gets translated to:
Kiota generates the following instead:
It's unclear to me how to pass a query string to the generated code that produces:
?filter[node.parent.parent.parent...]=equals(name,'X')
because it always emits?query=....
The produced OAS is not set in stone, though it would be preferable if it works with both NSwag and Kiota.
The text was updated successfully, but these errors were encountered: