From 0d181984d4b7dc884dcb0b0d3bd840400865cbfb Mon Sep 17 00:00:00 2001 From: Mariana Developer <87234445+mvorotniak@users.noreply.github.com> Date: Wed, 29 Nov 2023 08:00:10 +0200 Subject: [PATCH] [BLZT-47] Fixed inject non-config bean into config bean (#85) * [BLZT-47] Fixed inject non-config bean into config bean * [BLZT-47] Fixed inject non-config bean into config bean * [BLZT-47] Fixed unit tests --- .../resolver/AnnotationResolver.java | 6 ++ .../ComponentBeanNameAnnotationResolver.java | 5 +- ...nfigurationBeanNameAnnotationResolver.java | 2 +- .../InterfaceBeanNameAnnotationResolver.java | 2 +- .../ServiceBeanNameAnnotationResolver.java | 2 +- .../impl/AnnotationBringBeanRegistry.java | 4 +- .../bring/core/context/impl/BeanCreator.java | 81 +++++++++++++++++-- .../impl/ConfigurationBeanRegistry.java | 79 ------------------ .../impl/ConstructorBeanInjection.java | 32 ++------ ...BringApplicationContextHappyCasesTest.java | 3 + ...ngApplicationContextNegativeCasesTest.java | 2 +- ...pplicationContextCyclicDependencyTest.java | 2 +- .../configuration/TestConfiguration.java | 4 +- .../fullinjection/AppConfiguration.java | 8 ++ .../fullinjection/DefaultRestClient.java | 14 ++++ .../web/servlet/JsonExceptionHandler.java | 1 - 16 files changed, 124 insertions(+), 123 deletions(-) delete mode 100644 core/src/main/java/com/bobocode/bring/core/context/impl/ConfigurationBeanRegistry.java create mode 100644 core/src/test/java/testdata/di/positive/fullinjection/DefaultRestClient.java diff --git a/core/src/main/java/com/bobocode/bring/core/annotation/resolver/AnnotationResolver.java b/core/src/main/java/com/bobocode/bring/core/annotation/resolver/AnnotationResolver.java index 39a95987..d2b9b4e8 100644 --- a/core/src/main/java/com/bobocode/bring/core/annotation/resolver/AnnotationResolver.java +++ b/core/src/main/java/com/bobocode/bring/core/annotation/resolver/AnnotationResolver.java @@ -29,4 +29,10 @@ public interface AnnotationResolver { * @return a string representing resolved information from the annotations */ String resolve(Class clazz); + + default String getSimpleName(Class clazz) { + String simpleName = clazz.getSimpleName(); + + return simpleName.substring(0, 1).toLowerCase() + simpleName.substring(1); + } } diff --git a/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ComponentBeanNameAnnotationResolver.java b/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ComponentBeanNameAnnotationResolver.java index 786ee4c5..73deed28 100644 --- a/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ComponentBeanNameAnnotationResolver.java +++ b/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ComponentBeanNameAnnotationResolver.java @@ -44,9 +44,12 @@ public boolean isSupported(Class clazz) { @Override public String resolve(Class clazz) { String value = clazz.getAnnotation(Component.class).value(); + String componentName = value.isEmpty() ? getSimpleName(clazz) : value; + String qualifier = getQualifier(clazz); + // qualifier -> Component.value -> className - return qualifier != null ? qualifier : value.isEmpty() ? clazz.getSimpleName() : value; + return qualifier != null ? qualifier : componentName; } private String getQualifier (Class clazz) { diff --git a/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ConfigurationBeanNameAnnotationResolver.java b/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ConfigurationBeanNameAnnotationResolver.java index e10ed207..018152ec 100644 --- a/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ConfigurationBeanNameAnnotationResolver.java +++ b/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ConfigurationBeanNameAnnotationResolver.java @@ -33,7 +33,7 @@ public boolean isSupported(Class clazz) { */ @Override public String resolve(Class clazz) { - return clazz.getSimpleName(); + return getSimpleName(clazz); } } diff --git a/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/InterfaceBeanNameAnnotationResolver.java b/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/InterfaceBeanNameAnnotationResolver.java index bedc0847..460b3dda 100644 --- a/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/InterfaceBeanNameAnnotationResolver.java +++ b/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/InterfaceBeanNameAnnotationResolver.java @@ -38,6 +38,6 @@ public boolean isSupported(Class clazz) { */ @Override public String resolve(Class clazz) { - return clazz.getSimpleName(); + return getSimpleName(clazz); } } diff --git a/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ServiceBeanNameAnnotationResolver.java b/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ServiceBeanNameAnnotationResolver.java index a7b72840..8f95a5a3 100644 --- a/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ServiceBeanNameAnnotationResolver.java +++ b/core/src/main/java/com/bobocode/bring/core/annotation/resolver/impl/ServiceBeanNameAnnotationResolver.java @@ -42,7 +42,7 @@ public boolean isSupported(Class clazz) { @Override public String resolve(Class clazz) { String value = clazz.getAnnotation(Service.class).value(); - return value.isEmpty() ? clazz.getSimpleName() : value; + return value.isEmpty() ? getSimpleName(clazz) : value; } } diff --git a/core/src/main/java/com/bobocode/bring/core/context/impl/AnnotationBringBeanRegistry.java b/core/src/main/java/com/bobocode/bring/core/context/impl/AnnotationBringBeanRegistry.java index 3551fb1a..c497850a 100644 --- a/core/src/main/java/com/bobocode/bring/core/context/impl/AnnotationBringBeanRegistry.java +++ b/core/src/main/java/com/bobocode/bring/core/context/impl/AnnotationBringBeanRegistry.java @@ -19,7 +19,6 @@ public class AnnotationBringBeanRegistry extends DefaultBringBeanFactory impleme @Getter protected ClassPathScannerFactory classPathScannerFactory; - private final ConfigurationBeanRegistry configurationBeanRegistry; private final BeanCreator beanCreator; private final Set currentlyCreatingBeans = new HashSet<>(); @Getter @@ -28,7 +27,6 @@ public class AnnotationBringBeanRegistry extends DefaultBringBeanFactory impleme public AnnotationBringBeanRegistry(Reflections reflections) { this.reflections = reflections; this.classPathScannerFactory = new ClassPathScannerFactory(reflections); - this.configurationBeanRegistry = new ConfigurationBeanRegistry(this); this.beanCreator = new BeanCreator(this, classPathScannerFactory); } @@ -50,7 +48,7 @@ public void registerBean(String beanName, BeanDefinition beanDefinition) { currentlyCreatingBeans.add(beanName); if (beanDefinition.isConfigurationBean()) { - configurationBeanRegistry.registerConfigurationBean(beanName, beanDefinition); + beanCreator.registerConfigurationBean(beanName, beanDefinition); } else { beanCreator.create(clazz, beanName, beanDefinition); } diff --git a/core/src/main/java/com/bobocode/bring/core/context/impl/BeanCreator.java b/core/src/main/java/com/bobocode/bring/core/context/impl/BeanCreator.java index d4cd2603..31875325 100644 --- a/core/src/main/java/com/bobocode/bring/core/context/impl/BeanCreator.java +++ b/core/src/main/java/com/bobocode/bring/core/context/impl/BeanCreator.java @@ -2,8 +2,16 @@ import com.bobocode.bring.core.context.scaner.ClassPathScannerFactory; import com.bobocode.bring.core.domain.BeanDefinition; +import com.bobocode.bring.core.exception.NoSuchBeanException; +import com.bobocode.bring.core.utils.ReflectionUtils; import lombok.extern.slf4j.Slf4j; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Supplier; + /** * Responsible for creating beans and injecting dependencies into these beans based on provided definitions. * @@ -35,21 +43,78 @@ public BeanCreator(AnnotationBringBeanRegistry beanRegistry, ClassPathScannerFac * * @param clazz The class of the bean to be created. * @param beanName The name of the bean being created. - * @param beanDefinition The definition of the bean being created. + * @param beanDefinition The definition of the bean being created. + * @return The created bean object. */ - public void create(Class clazz, String beanName, BeanDefinition beanDefinition) { - createBeanUsingConstructor.create(clazz, beanName, beanDefinition); - injectDependencies(clazz, beanName); + public Object create(Class clazz, String beanName, BeanDefinition beanDefinition) { + Object bean = createBeanUsingConstructor.create(clazz, beanName, beanDefinition); + injectDependencies(clazz, bean); + + return bean; + } + + /** + * Registers a configuration bean in the context based on the provided bean name and definition. + * Resolves dependencies and instantiates the configuration bean within the context. + * + * @param beanName The name of the configuration bean to be registered. + * @param beanDefinition The definition of the configuration bean. + * @return The registered configuration bean. + */ + public Object registerConfigurationBean(String beanName, BeanDefinition beanDefinition) { + Object configObj = Optional.ofNullable(beanRegistry.getSingletonObjects().get(beanDefinition.getFactoryBeanName())) + .orElseThrow(() -> { + log.info("Unable to register Bean from Configuration class [{}]: " + + "Configuration class not annotated or is not of Singleton scope.", beanName); + return new NoSuchBeanException(beanDefinition.getBeanClass()); + }); + + List methodParamNames = ReflectionUtils.getParameterNames(beanDefinition.getMethod()); + + List methodObjs = new ArrayList<>(); + methodParamNames.forEach( + paramName -> { + Object object = beanRegistry.getBeanByName(paramName); + + if (Objects.nonNull(object)) { + methodObjs.add(object); + } else { + BeanDefinition bd = Optional.ofNullable(beanRegistry.getBeanDefinitionMap().get(paramName)) + .orElseThrow(() -> { + if (beanDefinition.getBeanClass().isInterface()) { + return new NoSuchBeanException(String.format("No such bean that implements this %s", + beanDefinition.getBeanClass())); + } + return new NoSuchBeanException(beanDefinition.getBeanClass()); + }); + + Object newObj = bd.isConfigurationBean() + ? registerConfigurationBean(beanName, bd) + : create(bd.getBeanClass(), beanName, bd); + methodObjs.add(newObj); + } + }); + + Supplier supplier = ReflectionUtils.invokeBeanMethod(beanDefinition.getMethod(), + configObj, methodObjs.toArray()); + Object bean = supplier.get(); + + if (beanDefinition.isPrototype()) { + beanRegistry.addPrototypeBean(beanName, supplier); + } else { + beanRegistry.addSingletonBean(beanName, bean); + } + + return bean; } /** - * Injects dependencies into the bean based on its class and name. + * Injects dependencies into the passed bean based on its class. * * @param clazz The class of the bean. - * @param beanName The name of the bean. + * @param bean The bean. */ - private void injectDependencies(Class clazz, String beanName) { - Object bean = beanRegistry.getSingletonObjects().get(beanName); + private void injectDependencies(Class clazz, Object bean) { fieldBeanInjection.injectViaFields(clazz, bean); setterBeanInjection.injectViaSetter(clazz, bean); } diff --git a/core/src/main/java/com/bobocode/bring/core/context/impl/ConfigurationBeanRegistry.java b/core/src/main/java/com/bobocode/bring/core/context/impl/ConfigurationBeanRegistry.java deleted file mode 100644 index 6fe557ed..00000000 --- a/core/src/main/java/com/bobocode/bring/core/context/impl/ConfigurationBeanRegistry.java +++ /dev/null @@ -1,79 +0,0 @@ -package com.bobocode.bring.core.context.impl; - -import com.bobocode.bring.core.domain.BeanDefinition; -import com.bobocode.bring.core.exception.NoSuchBeanException; -import com.bobocode.bring.core.utils.ReflectionUtils; -import lombok.AllArgsConstructor; -import lombok.extern.slf4j.Slf4j; - -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.function.Supplier; - -/** - * Responsible for registering configuration beans within the DefaultBringBeanFactory context. - * This class facilitates the instantiation and management of beans defined within configuration classes. - * - * @author Blyzhnytsia Team - * @since 1.0 - */ -@Slf4j -@AllArgsConstructor -public class ConfigurationBeanRegistry { - - private final DefaultBringBeanFactory beanRegistry; - - /** - * Registers a configuration bean in the context based on the provided bean name and definition. - * Resolves dependencies and instantiates the configuration bean within the context. - * - * @param beanName The name of the configuration bean to be registered. - * @param beanDefinition The definition of the configuration bean. - * @return The registered configuration bean. - */ - public Object registerConfigurationBean(String beanName, BeanDefinition beanDefinition) { - Object configObj = Optional.ofNullable(beanRegistry.getSingletonObjects().get(beanDefinition.getFactoryBeanName())) - .orElseThrow(() -> { - log.info("Unable to register Bean from Configuration class [{}]: " + - "Configuration class not annotated or is not of Singleton scope.", beanName); - return new NoSuchBeanException(beanDefinition.getBeanClass()); - }); - - List methodParamNames = ReflectionUtils.getParameterNames(beanDefinition.getMethod()); - - List methodObjs = new ArrayList<>(); - methodParamNames.forEach( - paramName -> { - Object object = beanRegistry.getBeanByName(paramName); - - if (Objects.nonNull(object)) { - methodObjs.add(object); - } else { - BeanDefinition bd = Optional.ofNullable(beanRegistry.getBeanDefinitionMap().get(paramName)) - .orElseThrow(() -> { - if (beanDefinition.getBeanClass().isInterface()) { - return new NoSuchBeanException(String.format("No such bean that implements this %s", - beanDefinition.getBeanClass())); - } - return new NoSuchBeanException(beanDefinition.getBeanClass()); - }); - Object newObj = registerConfigurationBean(beanName, bd); - methodObjs.add(newObj); - } - }); - - Supplier supplier = ReflectionUtils.invokeBeanMethod(beanDefinition.getMethod(), - configObj, methodObjs.toArray()); - Object bean = supplier.get(); - - if (beanDefinition.isPrototype()) { - beanRegistry.addPrototypeBean(beanName, supplier); - } else { - beanRegistry.addSingletonBean(beanName, bean); - } - - return bean; - } -} diff --git a/core/src/main/java/com/bobocode/bring/core/context/impl/ConstructorBeanInjection.java b/core/src/main/java/com/bobocode/bring/core/context/impl/ConstructorBeanInjection.java index bf513561..6e5a669a 100644 --- a/core/src/main/java/com/bobocode/bring/core/context/impl/ConstructorBeanInjection.java +++ b/core/src/main/java/com/bobocode/bring/core/context/impl/ConstructorBeanInjection.java @@ -40,12 +40,13 @@ public class ConstructorBeanInjection { * @param clazz The class for which the bean is to be created. * @param beanName The name of the bean. * @param beanDefinition The definition of the bean. + * @return The created bean object. * @throws NoConstructorWithAutowiredAnnotationBeanException If no constructor is annotated with @Autowired. * @throws NoSuchBeanException If a required dependency bean is not found. */ - public void create(Class clazz, String beanName, BeanDefinition beanDefinition) { + public Object create(Class clazz, String beanName, BeanDefinition beanDefinition) { var constructor = findAutowiredConstructor(clazz); - createBeanUsingConstructor(constructor, beanName, beanDefinition); + return createBeanUsingConstructor(constructor, beanName, beanDefinition); } /** @@ -77,9 +78,10 @@ private Constructor findAutowiredConstructor(Class clazz) { * @param constructor The constructor to use for bean instantiation. * @param beanName The name of the bean to be created. * @param beanDefinition The definition of the bean being created. + * @return The created bean object using constructor. * @throws NoSuchBeanException If a required dependency bean is not found. */ - private void createBeanUsingConstructor(Constructor constructor, String beanName, + private Object createBeanUsingConstructor(Constructor constructor, String beanName, BeanDefinition beanDefinition) { var createdBeanAnnotations = classPathScannerFactory.getCreatedBeanAnnotations(); Parameter[] parameters = constructor.getParameters(); @@ -113,6 +115,8 @@ private void createBeanUsingConstructor(Constructor constructor, String beanN } else { beanRegistry.addSingletonBean(beanName, bean); } + + return bean; } /** @@ -164,7 +168,6 @@ private String findBeanNameForArgumentInConstructor(Parameter parameter, List beanNames, String paramName, Parameter parameter) { Class parameterType = parameter.getType(); - String qualifier = getQualifier(parameter); var primaryNames = beanNames.stream() .filter(beanName -> beanRegistry.getBeanDefinitionByName(beanName).isPrimary()) .toList(); @@ -172,30 +175,11 @@ private String findPrimaryBeanNameOrByQualifierOrbBParameter(List beanNa return primaryNames.get(0); } else if (primaryNames.size() > 1) { throw new NoUniqueBeanException(parameterType); - } else if (qualifier != null) { - return beanNames.stream().filter(name -> name.equals(qualifier)) - .findFirst().orElseThrow(() -> { - if (parameterType.isInterface()) { - return new NoSuchBeanException(String.format("No such bean that implements this %s ", parameterType)); - } - return new NoSuchBeanException(parameterType); - }); - } - else { + } else { return beanNames.stream() .filter(name -> name.equalsIgnoreCase(paramName)) .findFirst() .orElseThrow(() -> new NoUniqueBeanException(parameterType, beanNames)); } } - - /** - * Retrieves the qualifier value from the parameter's annotation, if present. - * - * @param parameter The parameter for which the qualifier value is to be retrieved. - * @return The qualifier value if present, otherwise null. - */ - private String getQualifier(Parameter parameter) { - return parameter.isAnnotationPresent(Qualifier.class) ? parameter.getAnnotation(Qualifier.class).value() : null; - } } diff --git a/core/src/test/java/com/bobocode/bring/core/context/impl/BringApplicationContextHappyCasesTest.java b/core/src/test/java/com/bobocode/bring/core/context/impl/BringApplicationContextHappyCasesTest.java index 926c3dbf..cc159d5b 100644 --- a/core/src/test/java/com/bobocode/bring/core/context/impl/BringApplicationContextHappyCasesTest.java +++ b/core/src/test/java/com/bobocode/bring/core/context/impl/BringApplicationContextHappyCasesTest.java @@ -122,6 +122,7 @@ void shouldCreateAndInjectDifferentBeans() { // when var useCase = bringApplicationContext.getBean(GetInfoFromExternalServicesUseCase.class); + var restClient3 = bringApplicationContext.getBean(testdata.di.positive.fullinjection.RestClient.class, "restClient3"); // then assertThat(useCase).isNotNull(); @@ -136,6 +137,8 @@ void shouldCreateAndInjectDifferentBeans() { "restClient=RestClient{url='https://exterl.service', username='user100'}}, " + "externalService2=ExternalService2{" + "restClient2=RestClient{url='https://exterl.service2', username='user200'}}}"); + assertThat(restClient3.getUrl()).isEqualTo("url"); + assertThat(restClient3.getUsername()).isEqualTo("username"); } @DisplayName("Should inject implementations of Interface to Field") diff --git a/core/src/test/java/com/bobocode/bring/core/context/impl/BringApplicationContextNegativeCasesTest.java b/core/src/test/java/com/bobocode/bring/core/context/impl/BringApplicationContextNegativeCasesTest.java index e85e3efc..b49b8160 100644 --- a/core/src/test/java/com/bobocode/bring/core/context/impl/BringApplicationContextNegativeCasesTest.java +++ b/core/src/test/java/com/bobocode/bring/core/context/impl/BringApplicationContextNegativeCasesTest.java @@ -68,7 +68,7 @@ void shouldThrowExceptionWhenMethodNotAnnotated() { void shouldThrowExceptionWhenWeHaveTwoDependenciesForOneInterface() { //given var expectedMessage = "No qualifying bean of type 'interface testdata.di.negative.oneinterfacetwodependency.Drink'" + - " available: expected single matching bean but found 2: [Espresso,Latte]"; + " available: expected single matching bean but found 2: [espresso,latte]"; //when Executable executable = () -> { var bringApplicationContext = BringApplication.run(DEMO_PACKAGE + ".oneinterfacetwodependency"); diff --git a/core/src/test/java/com/bobocode/bring/core/injection/cyclic/BringApplicationContextCyclicDependencyTest.java b/core/src/test/java/com/bobocode/bring/core/injection/cyclic/BringApplicationContextCyclicDependencyTest.java index 30ea6810..27287d83 100644 --- a/core/src/test/java/com/bobocode/bring/core/injection/cyclic/BringApplicationContextCyclicDependencyTest.java +++ b/core/src/test/java/com/bobocode/bring/core/injection/cyclic/BringApplicationContextCyclicDependencyTest.java @@ -22,7 +22,7 @@ class BringApplicationContextCyclicDependencyTest { @Test void shouldThrowExceptionWhenWeHaveCyclicDependencyViaConstructor() { //given - var expectedMessage = "Looks like you have cyclic dependency between those beans [A -> B -> C -> A]"; + var expectedMessage = "Looks like you have cyclic dependency between those beans [a -> b -> c -> a]"; // when diff --git a/core/src/test/java/testdata/di/positive/configuration/configuration/TestConfiguration.java b/core/src/test/java/testdata/di/positive/configuration/configuration/TestConfiguration.java index 3fafeefd..aab3a5e5 100644 --- a/core/src/test/java/testdata/di/positive/configuration/configuration/TestConfiguration.java +++ b/core/src/test/java/testdata/di/positive/configuration/configuration/TestConfiguration.java @@ -48,8 +48,8 @@ public String url(String urlValue) { } @Bean - public BringService bringService3(@Qualifier("bringRestClient") RestClient сlient, String url) { - RestClient restClient = сlient.toBuilder().url(url).build(); + public BringService bringService3(@Qualifier("bringRestClient") RestClient client, String url) { + RestClient restClient = client.toBuilder().url(url).build(); return new BringService(restClient); } diff --git a/core/src/test/java/testdata/di/positive/fullinjection/AppConfiguration.java b/core/src/test/java/testdata/di/positive/fullinjection/AppConfiguration.java index c4b770af..11f0b239 100644 --- a/core/src/test/java/testdata/di/positive/fullinjection/AppConfiguration.java +++ b/core/src/test/java/testdata/di/positive/fullinjection/AppConfiguration.java @@ -21,5 +21,13 @@ public RestClient restClient2() { .username("user200") .build(); } + + @Bean + public RestClient restClient3(DefaultRestClient defaultRestClient) { + return RestClient.builder() + .url(defaultRestClient.getUrl()) + .username(defaultRestClient.getUsername()) + .build(); + } } diff --git a/core/src/test/java/testdata/di/positive/fullinjection/DefaultRestClient.java b/core/src/test/java/testdata/di/positive/fullinjection/DefaultRestClient.java new file mode 100644 index 00000000..7f244bc1 --- /dev/null +++ b/core/src/test/java/testdata/di/positive/fullinjection/DefaultRestClient.java @@ -0,0 +1,14 @@ +package testdata.di.positive.fullinjection; + +import com.bobocode.bring.core.annotation.Component; +import lombok.Data; + +@Data +@Component +public class DefaultRestClient { + + private final String url = "url"; + + private final String username = "username"; + +} diff --git a/web/src/main/java/com/bobocode/bring/web/servlet/JsonExceptionHandler.java b/web/src/main/java/com/bobocode/bring/web/servlet/JsonExceptionHandler.java index a987ae6c..81b288d5 100644 --- a/web/src/main/java/com/bobocode/bring/web/servlet/JsonExceptionHandler.java +++ b/web/src/main/java/com/bobocode/bring/web/servlet/JsonExceptionHandler.java @@ -22,7 +22,6 @@ * @author Blyzhnytsia Team * @since 1.0 */ -@Component @RequiredArgsConstructor public class JsonExceptionHandler extends ErrorReportValve {