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

Fix: OpenAPI attributes are not getting picked up if they are in a referenced class library project #298 #301

Merged
merged 6 commits into from
Feb 9, 2022

Conversation

PanosKousidis
Copy link
Contributor

This solution introduces an optional parameter to load referenced assemblies' types as well when trying to find out which methods are eligible to be shown on the SwaggerUI

@ghost
Copy link

ghost commented Nov 1, 2021

CLA assistant check
All CLA requirements met.

@PanosKousidis
Copy link
Contributor Author

There are 8 references of this function and I wasn't sure which ones needed to get the referenced types as well. I just changed the one that was causing the problem for me, perhaps more changes should be made and if they are more, we could potentially change the default value to true

@justinyoo
Copy link
Contributor

@PanosKousidis Thanks for the PR! It could be really helpful. Do you mind providing a sample repo that causes the issue so that I can reproduce the same error on my end?

@PanosKousidis
Copy link
Contributor Author

PanosKousidis commented Nov 2, 2021

@justinyoo
Copy link
Contributor

Thanks for the sample repo! I was able to reproduce the error.

I'll take a look + I'll take a further look, if we can also apply this approach to the in-proc worker.

@PanosKousidis
Copy link
Contributor Author

PanosKousidis commented Nov 7, 2021

Hi @justinyoo , I had a deeper look and it turns out that changing it to always include referenced types causes errors for 2 reasons:

  1. The VisitorCollection.CreateInstance() method looks for types that end with "Visitor". If we load referenced assemblies this causes problems as other assemblies may have types that end with "Visitor" and are not compliant with the logic (e.g. they don't have parameterless constructors or any public constructors at all etc. Such is the case for a NewtonsoftJson package that is included in the referenced assemblies)
  2. Loading referenced assemblies causes problems with the .SingleOrDefault(...) calls that expect for example one implementation of IOpenApiConfigurationOptions. By loading the referenced assemblies it loads the potential client function's implementation along with the referenced DefaultOpenApiConfigurationOptions and OpenApiSettings of your project.

To resolve (1) I changed the code in VisitorCollection.CreateInstance() to look for IVisitor interface implementations instead of classes ending in "Visitor", which should be ok, as the linq query has in its select statement (IVisitor)Activator.CreateInstance... so if it's not IVisitor there would be a problem anyway.

To resolve (2) I just excluded all types that belong in a namespace that starts with Microsoft.Azure.WebJobs.Extensions.OpenApi. That excludes all of your project so it works fine.

Now the GetLoadableTypes(...) can always include referenced assemblies and it seems to be working for me.
See comments on the commit above

@PanosKousidis PanosKousidis changed the title #298 OpenAPI attributes are not getting picked up if they are in a referenced class library project Fixes #298 OpenAPI attributes are not getting picked up if they are in a referenced class library project Nov 7, 2021
@PanosKousidis PanosKousidis changed the title Fixes #298 OpenAPI attributes are not getting picked up if they are in a referenced class library project fixes #298 OpenAPI attributes are not getting picked up if they are in a referenced class library project Nov 7, 2021
@PanosKousidis PanosKousidis changed the title fixes #298 OpenAPI attributes are not getting picked up if they are in a referenced class library project Fixes: OpenAPI attributes are not getting picked up if they are in a referenced class library project #298 Nov 7, 2021
@PanosKousidis PanosKousidis changed the title Fixes: OpenAPI attributes are not getting picked up if they are in a referenced class library project #298 Fix: OpenAPI attributes are not getting picked up if they are in a referenced class library project #298 Nov 7, 2021
@justinyoo justinyoo added bug Something isn't working v1.1.0 labels Nov 8, 2021
Copy link
Contributor

@justinyoo justinyoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few comments. Would you please be able to take a look?

@PanosKousidis
Copy link
Contributor Author

Yep, that's done

Copy link
Contributor

@justinyoo justinyoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the long wait! It looks good to me.

@justinyoo justinyoo merged commit 982ad22 into Azure:main Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v1.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenAPI attributes are not getting picked up if they are in a referenced class library project
2 participants