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

OpenAPI: Added AutoTag and Response for Auto Security Requirement #19464

Merged
merged 1 commit into from
Aug 20, 2021
Merged

OpenAPI: Added AutoTag and Response for Auto Security Requirement #19464

merged 1 commit into from
Aug 20, 2021

Conversation

phillip-kruger
Copy link
Member

This PR adds a AutoTag filter that will tag an operation with the class name. (if it's not explicitly tagged in annotations)
It also refactors the Auto Roles Allowed filter into it's own filer (from the SecurityConfig Filter) so it can apply even if security in not added with config.

Signed-off-by:Phillip Kruger [email protected]

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 17, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building f16c03f

Status Name Step Test failures Logs Raw logs
MicroProfile TCKs Tests Verify Test failures Logs Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ MicroProfile TCKs Tests #

📦 tcks/microprofile-openapi

org.eclipse.microprofile.openapi.tck.AirlinesAppTest.testTagsInOperations[JSON](1) line 736 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
JSON path paths.'/bookings/{id}'.delete.tags doesn't match.
Expected: not a collection with size a value greater than <0>
  Actual: <[Booking Resource]>

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:72)
	at org.codehaus.groovy.reflection.CachedConstructor.doConstructorInvoke(CachedConstructor.java:59)
	at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrap.callConstructor(ConstructorSite.java:84)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstruc...

org.eclipse.microprofile.openapi.tck.AirlinesAppTest.testTagsInOperations[YAML](2) line 736 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
JSON path paths.'/bookings/{id}'.delete.tags doesn't match.
Expected: not a collection with size a value greater than <0>
  Actual: <[Booking Resource]>

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:72)
	at org.codehaus.groovy.reflection.CachedConstructor.doConstructorInvoke(CachedConstructor.java:59)
	at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrap.callConstructor(ConstructorSite.java:84)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(...

@phillip-kruger phillip-kruger requested a review from geoand August 18, 2021 09:34
@geoand
Copy link
Contributor

geoand commented Aug 18, 2021

It would probably be best to get a review from someone else (@michalszynkiewicz maybe?) because I won't be back for a few more days

@phillip-kruger
Copy link
Member Author

Thanks, apologies I did not know you are away.

@geoand
Copy link
Contributor

geoand commented Aug 18, 2021

NP!

@sberyozkin
Copy link
Member

@phillip-kruger Hi, I'm not very familiar with this code but the idea of the AutoRolesAllowed makes sense, +1.

What does AutoTag help with ?

thanks

@phillip-kruger
Copy link
Member Author

@sberyozkin at the moment if an operation is not tagged, it's all grouped under "default" (see swagger ui screen). This will group them automatically by class (using the ClassName but making it readable, so Class Name). A user can still use the tag annotation, and in that case we will not auto tag.

Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

LGTM

@phillip-kruger phillip-kruger merged commit 1a63a85 into quarkusio:main Aug 20, 2021
@quarkus-bot quarkus-bot bot added this to the 2.3 - main milestone Aug 20, 2021
@phillip-kruger phillip-kruger deleted the openapi-autotag branch August 26, 2021 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants