Skip to content

add additional OpenAPIExtension::decorateOperation overload#3917

Open
fabianvo wants to merge 5 commits intoswagger-api:masterfrom
fabianvo:master
Open

add additional OpenAPIExtension::decorateOperation overload#3917
fabianvo wants to merge 5 commits intoswagger-api:masterfrom
fabianvo:master

Conversation

@fabianvo
Copy link

fixes #3916

I added a second overload to avoid breaking existing OpenAPIExtension implementations.

Having access to a declaring class allows implementing some resource methods in abstract base classes without needing to override in each subclass just to decorate them with all required annotations.

I included a simple use case in DecoratorExtensionTest.java

Copy link
Contributor

@frantuma frantuma left a comment

Choose a reason for hiding this comment

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

@fabianvo Thanks for the contribution! I added a couple of comments which I'd rather have addressed before merging

extension.decorateOperation(cls, operation, method, chain2);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather use same iterator and invoke in sequence the 2 overloads, is there a reason why two iterations are necessary?

return openAPI;
}

protected void decorateOperation(Class<?> cls, Operation operation, Method method) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would possibly just add javadoc @since 2.1.9


/**
* Decorates operation with additional vendor based extensions.
*
Copy link
Contributor

Choose a reason for hiding this comment

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

would add some clarification in javadocs, mentioning that new overload is available @since 2.1.9 and that the two overloads are both invoked in a given order

@MartinWahnschaffe
Copy link

@fabianvo Would be great to have this PR merged. Any chance you have another look at this?

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

Successfully merging this pull request may close these issues.

OpenAPIExtension::decorateOperation is missing declaring class type information

3 participants