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

feat(cloud-stream): search configuration classes for channels and operations #690

Conversation

timonback
Copy link
Member

No description provided.

@timonback timonback requested a review from stavshamir April 6, 2024 18:30
Copy link

netlify bot commented Apr 6, 2024

Deploy Preview for springwolf-ui canceled.

Name Link
🔨 Latest commit b7dc60c
🔍 Latest deploy log https://app.netlify.com/sites/springwolf-ui/deploys/661194b39ecb0d0008109516

Copy link
Member

@stavshamir stavshamir 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 improvements, nice to be working together again :)

@@ -42,7 +42,7 @@ public class CloudStreamFunctionChannelsScanner implements ChannelsScanner {

private final AsyncApiDocketService asyncApiDocketService;
private final BeanMethodsScanner beanMethodsScanner;
private final ComponentClassScanner componentClassScanner;
private final SpringwolfClassScanner classScanner;
Copy link
Member

Choose a reason for hiding this comment

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

The use of ComponentClassScanner and not SpringwolfClassScanner is intentional.
SpringwolfClassScanner will return classes annotated with @Component which is needed here, but also classes returned by bean methods. In the case of spring cloud stream functions, the bean method always has generics, which can't be retrieved (to my knowledge) from the Class object.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Therefore both scanners are needed.
My intention is to support Configuration annotations as well. And overall to align the cloudstream with the other plugins - if possible.

Will have a closer look, whether Configurations are Components already

if (Function.class.isAssignableFrom(type)) {
Class<?> inputType = getTypeGenerics(element).get(0);
Class<?> outputType = getTypeGenerics(element).get(1);
if (Function.class.isAssignableFrom(type) && typeGenerics.size() >= 2) {
Copy link
Member

Choose a reason for hiding this comment

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

Might be too defensive, Consumer and Supplier always have exactly one generic type and Function always has exactly 2, but that wouldn't hurt.

@@ -89,8 +90,11 @@ private static String getElementName(AnnotatedElement element) {

private List<Class<?>> getTypeGenerics(AnnotatedElement element) {
if (element instanceof Method m) {
ParameterizedType genericReturnType = (ParameterizedType) m.getGenericReturnType();
return getTypeGenerics(genericReturnType);
if (m.getGenericReturnType() instanceof ParameterizedType) {
Copy link
Member

Choose a reason for hiding this comment

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

I did not add the type check because we are always dealing here with methods that return Consumer, Supplier or Function for which getGenericReturnType is always ParameterizedType. But that's probably good practice.
Maybe use smart casting though?

if (m.getGenericReturnType() instanceof ParameterizedType parameterizedType) {
    return getTypeGenerics(parameterizedType);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I did run into an exception, but could have been an intermediate state.

@@ -39,15 +39,15 @@ public class CloudStreamFunctionOperationsScanner implements OperationsScanner {

private final AsyncApiDocketService asyncApiDocketService;
private final BeanMethodsScanner beanMethodsScanner;
private final ComponentClassScanner componentClassScanner;
Copy link
Member

Choose a reason for hiding this comment

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

Same as first comment, the use of ComponentClassScanner here is intentional.

@timonback
Copy link
Member Author

I just checked, @Configuration classes are also @Components. Therefore closing this PR.

@timonback timonback closed this Apr 12, 2024
@timonback timonback deleted the feat/find-channels-in-configuration-classes branch April 26, 2024 15:54
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.

2 participants