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

GET requests that use complex objects #1038

Open
replaysMike opened this issue Mar 24, 2017 · 13 comments
Open

GET requests that use complex objects #1038

replaysMike opened this issue Mar 24, 2017 · 13 comments

Comments

@replaysMike
Copy link

replaysMike commented Mar 24, 2017

Hey all,

I can't seem to find a proper solution to dealing with GET requests that use a complex request object rather than individual method calls. Swashbuckle seems to do some flattening of the query parameters already but it prepends the variable name specified in the GET request. For some reason, it also generates a definition for the complex request object but doesn't use it - which leads to swagger warnings. I did find one other issue which seems to be referencing the same: #70 and domaindrivendev/Swashbuckle.AspNetCore#66 but I could not find anything in the 5.0 documentation that would lead me to believe this is already handled within Swashbuckle.

Example method:

When using:

[HttpGet]
public async Task<IHttpActionResult> GetThings(GetThingsRequest queryParameters) {}

instead of:

[HttpGet]
public async Task<IHttpActionResult> GetThings(int[] ids, string name, int pageNumber) {}

results in schema:

paths: {
    "/api/things": {
      "get": {
        "summary": "Get some things",
        "operationId": "getThings",
        "parameters": [
          {
            "name": "queryParameters.ids",
            "in": "query",
            "description": "The ids to reference",
          }
    }
}
...
definitions: {
    "GetThingsRequest": {
      "type": "object",
      "properties": {
        "ids": {
          "description": "The list of id's to filter by",
          "type": "array",
          "items": {
            "format": "int32",
            "type": "integer"
          }
        },
        "name": {
          "format": "date-time",
          "description": "The date to filter by",
          "type": "string"
        },
    }
}
...

But really instead of "queryParameters.ids" it should be defining just "ids" as the parameter. Is there a way to do this without invoking a custom OperationFilter?

@replaysMike
Copy link
Author

replaysMike commented Mar 24, 2017

domaindrivendev/Swashbuckle.AspNetCore#66 seems to reference that using the [FromUri] attribute on a property will work, but under .Net Web API 2 that attribute can only be applied to Class and Parameters as per https://msdn.microsoft.com/en-us/library/system.web.http.fromuriattribute(v=vs.118).aspx

Maybe this is specific to .Net core? (yes, yes it is).

@replaysMike
Copy link
Author

replaysMike commented Mar 24, 2017

For reference, the following IOperationFilter will perform this desired behavior - I'm hoping this isn't needed. Alternatively, it would be even better I suppose to point a $ref to the input request object being used (it's in the schema) but I didn't tackle this part.

public class ComplexRequestObjectOperationFilter : IOperationFilter
    {
        public void Apply(Operation operation, SchemaRegistry schemaRegistry, ApiDescription apiDescription)
        {
            var actionDescriptor = apiDescription.ActionDescriptor;
            var method = apiDescription.HttpMethod.Method.ToLower();
            var controllerName = actionDescriptor.ControllerDescriptor.ControllerName;
            var parameterBindings = actionDescriptor.ActionBinding.ParameterBindings;
            // we are only interested in modifying GET requests
            if (method == "get")
            {
                if (parameterBindings.Length > 0)
                {
                    var parameterDescriptor = actionDescriptor.ActionBinding.ParameterBindings.First().Descriptor;
                    var parameterName = parameterDescriptor.ParameterName;
                    // correct the name of the property to _not_ prepend it with the controller method's argument name
                    if (parameterDescriptor.ParameterType.BaseType == typeof(object))
                    {
                        foreach (var property in operation.parameters)
                            property.name = property.name.Replace(parameterName + ".", "");
                    }
                }
            }
        }
    }

@bobvandevijver
Copy link

I have the same issue: even though requests do work correctly with the extra queryParameters part, it should not be there according to the spec.

I'm now using the snippet above, which works like a charm.

@heldersepu
Copy link
Contributor

@bobvandevijver Sorry but that link is not a spec document by any means
https://docs.microsoft.com/en-us/aspnet/web-api/overview/formats-and-model-binding/parameter-binding-in-aspnet-web-api#using-fromuri

I actually do prefer the extra queryParameters that way if you have multiple object in the url you can clearly identify them, like this one:
http://swashbuckletest.azurewebsites.net/swagger/ui/index?filter=Fr#/FromUri/FromUri_GetWith0

@bobvandevijver
Copy link

The link contains the MS docs about what the code should do: so yes, it is not a spec in literal sense, but is does describe how it is supposed to work.

When you have multiple objects in your controller parameters, the queryParameters part is actually required. However, if you require multiple objects in your requests, you're probably doing something wrong as such objects need to be tailored to your API anyways. If I understand your example correctly (the v1 and v2 are used to indicate API versions), it is actually wrong: that distinction should be on URL level.

Conclusion: for single object arguments the queryParameters should be left out, for multiple objects it should be there (but I would not recommend using it).

@heldersepu
Copy link
Contributor

heldersepu commented Oct 11, 2017

@bobvandevijver
On my example v1& v2 are abbreviations for value1 and value2, nothing to do with API versioning.

I agree with you, for single object arguments the queryParameters should be left out.
The key is "should", It is not a requirement.

@daniel-gwilt-software
Copy link

daniel-gwilt-software commented Dec 5, 2017

This is indeed more than just a styling issue. Query string serializers don't expect the extra queryParameters text to be there. This prevents the usage of 3rd party tools, like NSwagStudio, for code generation. Spent a whole day figuring this out.

@heldersepu
Copy link
Contributor

@daniel-gwilt-software sounds like you should fill a bug report with NSwagStudio

@daniel-gwilt-software
Copy link

daniel-gwilt-software commented Dec 5, 2017

@heldersepu NSwagStudio does support creating C# from the JSON instead of the controllers, so this is what we'll use. It's really all the confusion created by not following the standard conventions that were upsetting. I believe that's what standards are for. To facilitate communication on intention. It also creates more work for developers to use tools that don't follow popular conventions. For example, I had to write a custom object to query string conversion function to hit the API instead of being able to use the built-in AngularJS and/or JQuery functions. (https://docs.angularjs.org/api/ng/service/$httpParamSerializer)

@heldersepu
Copy link
Contributor

@daniel-gwilt-software Who is not following the standard conventions ?

@daniel-gwilt-software
Copy link

daniel-gwilt-software commented Dec 6, 2017

@heldersepu It turns out support for Complex Objects in the query string are supported in the Swagger 3.0 spec, but Swashbuckle nor NSwag support 3.0 right now. Thanks for your help.

https://swagger.io/docs/specification/serialization/

@tihomir-kit
Copy link

Hi. I assume no updates on this perhaps?

@kiranakhale
Copy link

public void Apply(Operation operation, SchemaRegistry schemaRegistry, ApiDescription apiDescription)
{
if (operation.parameters != null)
{
foreach (Parameter param in operation.parameters.Where(p => p.name.Contains('.')))
{
param.name = param.name.SplitWithoutEmpty('.').Last();
}
}
}

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

6 participants