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

QueryStringParameter is Mispelled and also breaks the swagger definition #9

Closed
SpicySyntax opened this issue Jul 11, 2019 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@SpicySyntax
Copy link

First off I wanted to say how thankful I am you are working on this. I was able to get up and running quite easily and everything seems to be working well. However, I have noticed some issues.
First: 'QueryStringParamater' is mis-spelled and should be 'QueryStringParameter'
Second: When I use it it breaks my swagger definition.

To reproduce just add an annotation like '[QueryStringParamater("jobId", "Id of Job you are deleting.", Required = true)]' to a function then the azure logs give:
'[7/11/2019 8:42:58 PM] Executed 'Swagger' (Failed, Id=10b20162-2910-43b3-a9cb-4f99db00efe9)
[7/11/2019 8:42:58 PM] System.Private.CoreLib: Exception while executing function: Swagger. Swashbuckle.AspNetCore.SwaggerGen: Object reference not set to an instance of an
object.'

And the swagger UI endpoint yields: 'No API definition provided.'

Once this is fixed an available on Nuget I would happily start using this in my azure function projects. Thanks for you work!

@MrBMills
Copy link

MrBMills commented Jul 16, 2019

So if you drill down in the code. You will see that there is no check for null and no default specified for the Property DataType. So in the QueryStringParameterAttributeFilter.cs file on line 26, this will error out
Type = context.SchemaRegistry.GetOrRegister(attribute.DataType).Type . So for all intends and purpose DataType is required for the Attribute

@SpicySyntax
Copy link
Author

@MrBMills After adding this the query param definition works. Maybe for clarity sake, adding the DataType as a Constructor param would make it more explicit that it is required. Other than that, as long as the spelling is corrected on the next release this issue can be closed.

Best

@yuka1984 yuka1984 added the enhancement New feature or request label Jul 24, 2019
@nancymalhotra23
Copy link

Any plans for this issue fix?

yuka1984 pushed a commit that referenced this issue Dec 14, 2019
@yuka1984 yuka1984 added this to the ver 1.4.4 milestone Dec 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants