Introduce ManagedTypes. #2635
Introduce ManagedTypes. #2635
Conversation
| * @since 3.0 | ||
| */ | ||
| @FunctionalInterface | ||
| public interface ManagedTypes { |
There was a problem hiding this comment.
How about implementing Streamable<Class<?>> here?
| Supplier<Set<String>> basePackages) { | ||
|
|
||
| String beanName = String.format("%s.managed-types", extension.getModulePrefix()); | ||
| if (!registry.isBeanNameInUse(beanName)) { |
There was a problem hiding this comment.
Using multiple times @Enable…Repositories can cause conflicts. Spring has somewhere a utility to generate unique bean names from a bean base name.
There was a problem hiding this comment.
resolved by not not scanning for types here. (see comment below).
src/main/java/org/springframework/data/repository/config/RepositoryConfigurationExtension.java
Show resolved
Hide resolved
| @@ -45,6 +49,10 @@ public class AnnotatedTypeScanner implements ResourceLoaderAware, EnvironmentAwa | |||
| private @Nullable ResourceLoader resourceLoader; | |||
| private @Nullable Environment environment; | |||
|
|
|||
| private Consumer<ClassNotFoundException> classNotFoundAction = ex -> { | |||
There was a problem hiding this comment.
What is the idea behind the ClassNotFoundException handling? Is this a response to a certain use case? If so, then we should make this concept more explicit with an ErrorHandler.
| * @return | ||
| * @since 3.0 | ||
| */ | ||
| default Collection<Class<? extends Annotation>> getIdentifyingAnnotations() { |
There was a problem hiding this comment.
It feels a bit weird handing out identifying annotations to the outside of the config extension as the managed type scanning vs. repository scanning are located at two different abstractions.
Repository type scanning happens inside the RepositoryConfigurationExtension. We still need to merge repository-managed types with the ones that we discover through the classpath scan.
How about returning a RepositoryConfigurations object from getRepositoryConfigurations that wraps Collection<RepositoryConfiguration<T>> and provides a ManagedTypes object?
Doing so would allow keeping all scanning-related operations within the extension.
There was a problem hiding this comment.
I think we should not be doing the ManagedType scanning in the RepositoryConfigurationDelegate and just consider the domain types exposed by the repository declaration.
ManagedTypes may be contributed by an external configuration source, such as Boot.
| * @return new instance of {@link TypeScanner}. | ||
| * @see #scanPackages(Collection) | ||
| */ | ||
| default TypeScanner scanPackages(String... packageNames) { |
There was a problem hiding this comment.
scanPackages should be renamed to a name that is less imperative and expresses rather that this is a config option packagesToScan or something like that, so that the imperative names (forEach, collectAsSet) remain as terminal methods.
There was a problem hiding this comment.
Taking this 1 step further, perhaps all the typeScanner(..) factory methods could be renamed to from(..) and the scanPackages(..) method could be renamed to inPackages(..).
There was a problem hiding this comment.
considered - TypeScanner is likely to be dropped from this PR and move to another one.
| * @return new instance of {@link TypeScanner}. | ||
| * @see #scanPackages(String...) | ||
| */ | ||
| TypeScanner scanPackages(Collection<String> packageNames); |
christophstrobl
force-pushed
the
issue/2634
branch
from
29231f9 to
be84bca
Compare
|
That's merged and polished now. |
See: #2634