Ability of specifying base package for configuration beans #63

Stmated · 2022-05-06T09:36:40Z

Ability of specifying base package for configuration beans, so components that are registered inside @Configuration can be found. This is because some projects simply do not use the @Component annotation, instead manually wire the beans.

In a way I think this overcomplicates things, and it would almost be easier to give ANT path pattern for which classes to search, and the code would just search all the methods of all the classes that match that pattern, with no regard to any class-level annotations at all.

Probable issues with this PR:

It removes @Configuration from being matched in the regular base package scanning. So if a user has a @KafkaListener inside a @Configuration class, it would not be found.

@configuration

… so components that are registered inside @configuration can be found.

Stmated · 2022-05-06T09:41:15Z

It would probably also be better to change the docket to take a ComponentScanner instance instead of specifying the basePackage, so it is decoupled and you can give a custom ComponentScanner that works completely different depending on need. Like not wanting any scanning at all, and just giving your classes manually.

stavshamir · 2022-05-06T10:05:14Z

Hi @Stmated , thanks for the PR! This does sound like a relevant use case that should be supported.
I will look into it soon, in the meanwhile please merge from master as I just fixed a failing test.

…into master

stavshamir · 2022-05-07T11:16:47Z

springwolf-core/src/main/java/io/github/stavshamir/springwolf/configuration/AsyncApiDocket.java

@@ -18,9 +18,13 @@ public class AsyncApiDocket {
    /**
     * The base package to scan for listeners.
     */
-    @NonNull
    private String basePackage;


I think we should make the comments more clear, as basePackage and configurationBasePackage are very similar and may be confusing.

I think this should change to:

/** * The base package to scan for listeners which are declared inside a class annotated with @Component or @Service. */

stavshamir · 2022-05-07T11:21:37Z

springwolf-core/src/main/java/io/github/stavshamir/springwolf/configuration/AsyncApiDocket.java

+    /**
+     * The base package to scan for configurations that contain beans that contain listeners.
+     */
+    private String configurationBasePackage;


Like they comment above, I think this comment should be more specific:

/** * The base package to scan for @Configuration classes that contain beans that contain listeners. * Set this property if your listeners are declared in a class that is not annotated with @Component or @Service, * and are declared in a class which is instantiated in a @Bean method in a @Configuration class. */

stavshamir

@Stmated, that's a great contribution, well thought out and useful. I also appreciate providing new unit tests :) I have a few comments that I would like you to check and after the required changes I would gladly merge this!

stavshamir · 2022-05-07T11:26:15Z

...ain/java/io/github/stavshamir/springwolf/asyncapi/scanners/components/ComponentsScanner.java

@@ -9,13 +9,16 @@ public interface ComponentsScanner {
     * @param basePackage The root package to scan.
     * @return A set of found classes.
     */
-    Set<Class<?>> scanForComponents(Package basePackage);
+    default Set<Class<?>> scanForComponents(String basePackage) {


Let's remove the argument completely form scanForComponents and leave it without arguments. Then we can also remove the AsyncAciDocket wirings from AbstractChannelScanner etc. and have DefaultComponentScanner ue the docket directly.

stavshamir · 2022-05-07T11:27:50Z

...a/io/github/stavshamir/springwolf/asyncapi/scanners/components/DefaultComponentsScanner.java

@@ -17,28 +24,79 @@
 public class DefaultComponentsScanner implements ComponentsScanner {



Wire the AsyncApiDocket here instead of in AbstrctChannelScanner, AbstractMethodLevelListener and ClassLevelKafkaListenerScanner.

stavshamir · 2022-05-07T11:33:04Z

...a/io/github/stavshamir/springwolf/asyncapi/scanners/components/DefaultComponentsScanner.java

+
+        Set<Class<?>> components;
+        if (StringUtils.isNotBlank(basePackage) && StringUtils.isNotBlank(configurationBasePackage)) {
+            log.debug("Scanning for component classes in configuration package {}, filtered by beans in {}", configurationBasePackage, basePackage);


I think the filtering part is going to be too confusing - if the user want to scan based on beans in a configuration class, let it scan all the beans. Let's remove the filtering logic.

stavshamir · 2022-05-07T11:35:24Z

...a/io/github/stavshamir/springwolf/asyncapi/scanners/components/DefaultComponentsScanner.java

        ClassPathScanningCandidateComponentProvider provider = new ClassPathScanningCandidateComponentProvider(false);
        provider.addIncludeFilter(new AnnotationTypeFilter(Component.class));
+        provider.addExcludeFilter(new AnnotationTypeFilter(Configuration.class));


Why should we exclude @Configuration?

stavshamir · 2022-05-07T11:36:05Z

...a/io/github/stavshamir/springwolf/asyncapi/scanners/components/DefaultComponentsScanner.java

+                    final Bean bean = AnnotationUtils.getAnnotation(method, Bean.class);
+                    if (bean != null) {
+                        final Class<?> returnType = method.getReturnType();
+                        final String beanPackage = returnType.getPackage().getName();


As mentioned above, I think this would be too confusing, let's remove the package filtering logic here.

stavshamir · 2022-05-07T11:40:36Z

@Stmated, that's a great contribution, well thought out and useful. I also appreciate providing new unit tests :) I have a few comments that I would like you to check and after the required changes I would gladly merge this!

…omponent scanners, Spring-aware Component Scanner

pontus-eliason · 2022-05-13T07:39:13Z

I did the changes noted in this PR, but also did a few extra things (which you might disagree with, who knows):

Changed so AsyncApiDocket takes a ComponentsScanner directly, but backwards compatible to previous basePackage way.
Added a ApplicationContextComponentsScanner which searches the bean registry. What's good about it: Can find beans given by external libraries and by a FactoryBean.
Separated the different ComponentsScanners into what they do, and created a default DefaultClassPathComponentsScanner that searches for @Component and @Configuration + @Bean annotations.

I did not make any changes to documentation, since it all works like it did previously, just with an option to swap the scanner if want to.

Stmated · 2022-05-13T22:04:13Z

And that ^ was me. Having multiple accounts is a hassle.

stavshamir · 2022-05-22T18:33:27Z

Sorry for the long time it took me to review!
Wow, that's a very impressive contribution. Thanks a lot. I will merge this now (the build is failing on some java doc stuff, I suspect it's related to lombok - I won't bother you with fixing that, I can do it myself).

Since this is a big amount of content added to the library, that would be great if we could also document it, so if you are looking for further contributions, I'm opening an issue in the project documentation site for this additions.

You are also welcome to the discord server that's starting to attract some action, and please add me on linkedin (https://www.linkedin.com/in/stavshamir/).

Cheers!

Added ability of specifying a base package for configuration classes,…

e0629ae

… so components that are registered inside @configuration can be found.

Stmated added 2 commits May 6, 2022 11:43

Wrong log

3382ab4

doc

51d0a7e

Merge branch 'master' of https://github.com/springwolf/springwolf-core …

31f9a2f

…into master

stavshamir reviewed May 7, 2022

View reviewed changes

stavshamir requested changes May 7, 2022

View reviewed changes

Moved ComponentsScanner to AsyncApiDocker, separated into different c…

4302436

…omponent scanners, Spring-aware Component Scanner

Stmated added 2 commits May 13, 2022 10:55

4-spacing

f4f3c39

4-spacing

b0d0dfc

stavshamir merged commit 14d0b07 into springwolf:master May 22, 2022

stavshamir mentioned this pull request May 22, 2022

Add examples for using the non-default new component scanner springwolf/springwolf.github.io#4

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ability of specifying base package for configuration beans #63

Ability of specifying base package for configuration beans #63

Stmated commented May 6, 2022 •

edited

Loading

Stmated commented May 6, 2022

stavshamir commented May 6, 2022

stavshamir May 7, 2022

stavshamir May 7, 2022

stavshamir left a comment

stavshamir May 7, 2022

stavshamir May 7, 2022

stavshamir May 7, 2022

stavshamir May 7, 2022

stavshamir May 7, 2022

stavshamir commented May 7, 2022

pontus-eliason commented May 13, 2022

Stmated commented May 13, 2022

stavshamir commented May 22, 2022

		@@ -17,28 +24,79 @@
		public class DefaultComponentsScanner implements ComponentsScanner {

Ability of specifying base package for configuration beans #63

Ability of specifying base package for configuration beans #63

Conversation

Stmated commented May 6, 2022 • edited Loading

Stmated commented May 6, 2022

stavshamir commented May 6, 2022

stavshamir May 7, 2022

Choose a reason for hiding this comment

stavshamir May 7, 2022

Choose a reason for hiding this comment

stavshamir left a comment

Choose a reason for hiding this comment

stavshamir May 7, 2022

Choose a reason for hiding this comment

stavshamir May 7, 2022

Choose a reason for hiding this comment

stavshamir May 7, 2022

Choose a reason for hiding this comment

stavshamir May 7, 2022

Choose a reason for hiding this comment

stavshamir May 7, 2022

Choose a reason for hiding this comment

stavshamir commented May 7, 2022

pontus-eliason commented May 13, 2022

Stmated commented May 13, 2022

stavshamir commented May 22, 2022

Stmated commented May 6, 2022 •

edited

Loading