From a1f5428c79054819dd78685b663d52db14b92830 Mon Sep 17 00:00:00 2001 From: Boubaker Khanfir Date: Wed, 9 Oct 2024 09:01:14 +0100 Subject: [PATCH] feat: Improve Spring To Spring Beans definition compatibility - MEED-7576 - Meeds-io/meeds#2469 This change will enhance Spring contexts Beans sharing by allowing to reference Bean Interface rather than implementation between Spring contexts and without having to define the Interface As Service with Primary annotation in Implementation. --- .../KernelContainerLifecyclePlugin.java | 186 +++++++++++++----- .../kernel/PortalApplicationContext.java | 36 ++-- .../PortalApplicationContextInitializer.java | 4 + .../test/SpringBeanFactoryInterceptor.java | 2 +- .../spring/module/service/TestService.java | 29 +-- .../module/service/TestServiceImpl.java | 51 +++++ 6 files changed, 217 insertions(+), 91 deletions(-) create mode 100644 component/common/src/test/java/io/meeds/spring/module/service/TestServiceImpl.java diff --git a/component/common/src/main/java/io/meeds/spring/kernel/KernelContainerLifecyclePlugin.java b/component/common/src/main/java/io/meeds/spring/kernel/KernelContainerLifecyclePlugin.java index 4f4bf182b3..c9d0a41d78 100644 --- a/component/common/src/main/java/io/meeds/spring/kernel/KernelContainerLifecyclePlugin.java +++ b/component/common/src/main/java/io/meeds/spring/kernel/KernelContainerLifecyclePlugin.java @@ -37,7 +37,9 @@ import org.picocontainer.Startable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.config.AutowireCapableBeanFactory; import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.RootBeanDefinition; @@ -46,10 +48,14 @@ import org.springframework.context.annotation.Configuration; import org.springframework.stereotype.Service; +import org.exoplatform.commons.api.persistence.GenericDAO; import org.exoplatform.container.BaseContainerLifecyclePlugin; import org.exoplatform.container.ExoContainer; +import org.exoplatform.container.ExoContainerContext; import org.exoplatform.container.PortalContainer; import org.exoplatform.container.PropertyConfigurator; +import org.exoplatform.container.RootContainer.PortalContainerPostInitTask; +import org.exoplatform.container.definition.PortalContainerConfig; import org.exoplatform.container.spi.ComponentAdapter; import org.exoplatform.services.rest.impl.StartableApplication; import org.exoplatform.services.rest.resource.ResourceContainer; @@ -57,20 +63,25 @@ import io.meeds.spring.kernel.annotation.Exclude; import io.meeds.spring.kernel.model.SpringBeanComponentAdapter; +import jakarta.servlet.ServletContext; + public class KernelContainerLifecyclePlugin extends BaseContainerLifecyclePlugin { - private static final Logger LOG = + private static final Logger LOG = LoggerFactory.getLogger(KernelContainerLifecyclePlugin.class); - private static Map springBeanRegistries = new ConcurrentHashMap<>(); + private static Map springBeanRegistries = new ConcurrentHashMap<>(); + + private static Map springContexts = new ConcurrentHashMap<>(); - private static Map springContexts = new ConcurrentHashMap<>(); + private static Map springContextsInitTasks = new ConcurrentHashMap<>(); - private static boolean kernelAlreadyBooted = false; + private static boolean kernelAlreadyBooted = false; public static void addSpringContext(String servletContextName, ApplicationContext applicationContext, - BeanDefinitionRegistry beanDefinitionRegistry) { + BeanDefinitionRegistry beanDefinitionRegistry, + Runnable springContextInitTask) { if (kernelAlreadyBooted) { LOG.warn("Adding Spring context '{}' happened too late in Server startup, spring beans will not be injected for this context", servletContextName); @@ -79,6 +90,9 @@ public static void addSpringContext(String servletContextName, LOG.info("Add Spring context '{}' to inject its Beans in Kernel as available components", servletContextName); springContexts.put(servletContextName, applicationContext); springBeanRegistries.put(servletContextName, beanDefinitionRegistry); + if (springContextInitTask != null) { + springContextsInitTasks.put(servletContextName, springContextInitTask); + } } @Override @@ -92,6 +106,16 @@ public void initContainer(ExoContainer container) throws Exception { StringUtils.join(springContexts.keySet(), ",")); } long start = System.currentTimeMillis(); + + // Delay Spring context finishing startup until the Kernel container is + // fully started + PortalContainer.addInitTask(portalContainer.getPortalContext(), new PortalContainerPostInitTask() { + @Override + public void execute(ServletContext context, PortalContainer portalContainer) { + finishSpringContextStartup(portalContainer); + } + }, "portal"); + Collection> containerComponentAdapters = portalContainer.getComponentAdapters(); Collection> parentComponentAdapters = portalContainer.getParent() != null ? portalContainer.getParent().getComponentAdapters() : Collections.emptyList(); @@ -110,35 +134,45 @@ public void initContainer(ExoContainer container) throws Exception { public static void addKernelToSpring(PortalContainer portalContainer, Collection> kernelComponentAdapters) { kernelComponentAdapters.forEach(adapter -> { Class componentKey = componentToBeanName(adapter); - if (!componentKey.isInterface() && isComponentDuplicated(portalContainer, componentKey)) { + if (isIgnoreKernelComponent(portalContainer, + componentKey, + adapter.getComponentImplementation())) { return; } + if (LOG.isDebugEnabled()) { + LOG.debug("Add Kernel Component '{}' in Spring contexts [{}]", + componentKey.getName(), + StringUtils.join(springContexts.keySet(), ", ")); + } springContexts.forEach((servletContextName, applicationContext) -> { BeanDefinitionRegistry beanRegistry = springBeanRegistries.get(servletContextName); String componentKeyName = componentKey.getName(); - if (beanRegistry.containsBeanDefinition(componentKeyName)) { - LOG.debug("Ignore Kernel Component '{}' as it's already injected in Spring context '{}'", - componentKeyName, - servletContextName); - } else { + if (!beanRegistry.containsBeanDefinition(componentKeyName)) { RootBeanDefinition beanDefinition = createBeanDefinition(componentKey, portalContainer); - LOG.debug("Add Kernel Component '{}' in Spring context '{}'", componentKeyName, servletContextName); beanRegistry.registerBeanDefinition(componentKeyName, beanDefinition); + } else if (LOG.isDebugEnabled()) { + LOG.debug("-- Ignore Adding duplicated Kernel Component '{}' in Spring contexts {}", + componentKey.getName(), + servletContextName); } }); }); } - public static void addSpringToKernel(PortalContainer portalContainer, Map> springBeansByContext) { + @SuppressWarnings("rawtypes") + public static void addSpringToKernel(PortalContainer portalContainer, + Map> springBeansByContext) { springContexts.forEach((servletContextName, applicationContext) -> { Map beansMap = springBeansByContext.get(servletContextName); beansMap.forEach((beanName, beanDefinition) -> { + Class componentKey = beanNameToComponentKey(beanDefinition.getBeanClassName(), beanName); ComponentAdapter componentAdapter = createComponentAdapter(servletContextName, portalContainer, - beanName, + componentKey.getName(), beanDefinition.getBeanClassName(), () -> getBeanInstance(applicationContext, - beanName)); + beanName, + portalContainer.getName())); if (componentAdapter != null) { LOG.debug("Add Spring Bean '{}' from context '{}' as Kernel component '{}'", beanName, @@ -160,25 +194,13 @@ public static void addSpringToEachOther(Map> springBeanRegistries.entrySet() .stream() .filter(e -> !StringUtils.equals(senderServletContextName, e.getKey())) - .forEach(entry -> { - String receiverServletContextName = entry.getKey(); - BeanDefinitionRegistry receiverBeanRegistry = entry.getValue(); - beansMap.forEach((beanName, senderBeanDefinition) -> { - Class beanClassName = getClass(senderBeanDefinition.getBeanClassName()); - if (beanClassName != null - && !receiverBeanRegistry.containsBeanDefinition(beanName) - && !receiverBeanRegistry.containsBeanDefinition(senderBeanDefinition.getBeanClassName())) { - RootBeanDefinition receiverBeanDefinition = createBeanDefinition(beanName, - beanClassName, - senderApplicationContext); - receiverBeanRegistry.registerBeanDefinition(beanName, receiverBeanDefinition); - LOG.debug("Add Spring Bean '{}' from Spring Context '{}' to Spring context '{}'", - beanName, - senderServletContextName, - receiverServletContextName); - } - }); - }); + .collect(Collectors.toMap(Entry::getKey, Entry::getValue)) + .forEach((receiverServletContextName, + receiverBeanRegistry) -> addSpringBeansToEachOtherContexts(senderServletContextName, + receiverServletContextName, + senderApplicationContext, + receiverBeanRegistry, + beansMap)); }); } @@ -189,12 +211,34 @@ public static Map> getBeansByServletContext( e -> getEligibleBeans(e.getValue()))); } + /** + * Finish booting Spring contexts switch the order or priority added in Kernel + * addons definition + * + * @param portalContainer + */ + public static void finishSpringContextStartup(PortalContainer portalContainer) { + PortalContainerConfig portalContainerConfig = portalContainer.getComponentInstanceOfType(PortalContainerConfig.class); + List springDependencies = portalContainerConfig.getDependencies(portalContainer.getName()) + .stream() + .filter(springContextsInitTasks::containsKey) + .filter(Objects::nonNull) + .toList(); + if (LOG.isInfoEnabled()) { + LOG.info("4. Finish booting spring based contexts switch Addons Priority: [{}]", + StringUtils.join(springDependencies, ", ")); + } + springDependencies.stream() + .map(springContextsInitTasks::get) + .forEach(Runnable::run); + } + @SuppressWarnings({ "rawtypes", "unchecked" }) private static ComponentAdapter createComponentAdapter(String servletContextName, - PortalContainer portalContainer, - String beanName, - String beanClassName, - Supplier getBeanFunction) { + PortalContainer portalContainer, + String beanName, + String beanClassName, + Supplier getBeanFunction) { Class componentKey = beanNameToComponentKey(beanClassName, beanName); if (componentKey != null && portalContainer.getComponentAdapter(componentKey) == null) { return new SpringBeanComponentAdapter(servletContextName, @@ -206,6 +250,47 @@ private static ComponentAdapter createComponentAdapter(String servletCon } } + @SuppressWarnings({ "rawtypes", "unchecked" }) + private static void addSpringBeansToEachOtherContexts(String senderServletContextName, + String receiverServletContextName, + ApplicationContext senderApplicationContext, + BeanDefinitionRegistry receiverBeanRegistry, + Map beansMap) { + beansMap.forEach((beanName, senderBeanDefinition) -> { + Class beanClassName = getClass(senderBeanDefinition.getBeanClassName()); + Class beanClassNameInterface = beanNameToComponentKey(senderBeanDefinition.getBeanClassName()); + if (beanClassName != null + && !receiverBeanRegistry.containsBeanDefinition(beanName) + && !receiverBeanRegistry.containsBeanDefinition(senderBeanDefinition.getBeanClassName()) + && !receiverBeanRegistry.containsBeanDefinition(beanClassNameInterface.getTypeName())) { + LOG.debug("Add Spring Bean '{}' with Class '{}' Interface '{}' from Spring Context '{}' to Spring context '{}'", + beanName, + beanClassName, + beanClassNameInterface, + senderServletContextName, + receiverServletContextName); + RootBeanDefinition receiverBeanDefinition = createBeanDefinition(beanName, + beanClassNameInterface, + senderApplicationContext, + receiverServletContextName); + receiverBeanRegistry.registerBeanDefinition(beanClassNameInterface.getName(), receiverBeanDefinition); + } + }); + } + + private static boolean isIgnoreKernelComponent(PortalContainer portalContainer, + Class componentKey, + Class componentImplementation) { + return (!componentKey.isInterface() && isComponentDuplicated(portalContainer, componentKey)) + || componentImplementation.getPackageName().startsWith("java") + || componentImplementation.getPackageName().startsWith("jakarta") + || componentImplementation.getPackageName().startsWith("sun.") + || ResourceContainer.class.isAssignableFrom(componentImplementation) + || PropertyConfigurator.class.isAssignableFrom(componentImplementation) + || ExoContainerContext.class.isAssignableFrom(componentImplementation) + || GenericDAO.class.isAssignableFrom(componentImplementation); + } + private static RootBeanDefinition createBeanDefinition(Class keyClass, PortalContainer portalContainer) { RootBeanDefinition beanDefinition = new RootBeanDefinition(keyClass, () -> getComponentInstance(portalContainer, keyClass)); @@ -215,13 +300,19 @@ private static RootBeanDefinition createBeanDefinition(Class keyClass, Po } private static RootBeanDefinition createBeanDefinition(String beanName, - Class beanClassName, - ApplicationContext senderApplicationContext) { + Class beanClassName, + ApplicationContext senderApplicationContext, + String receiverServletContextName) { RootBeanDefinition receiverBeanDefinition = new RootBeanDefinition(beanClassName, () -> getBeanInstance(senderApplicationContext, - beanName)); + beanName, + receiverServletContextName)); receiverBeanDefinition.setLazyInit(true); + receiverBeanDefinition.setAutowireCandidate(true); + receiverBeanDefinition.setAutowireMode(AutowireCapableBeanFactory.AUTOWIRE_BY_TYPE); + receiverBeanDefinition.setScope(ConfigurableBeanFactory.SCOPE_SINGLETON); receiverBeanDefinition.setDependencyCheck(AbstractBeanDefinition.DEPENDENCY_CHECK_NONE); + receiverBeanDefinition.setBeanClass(beanClassName); return receiverBeanDefinition; } @@ -229,8 +320,7 @@ private static Map getEligibleBeans(BeanDefinitionRegist String[] beanNames = beanRegistry.getBeanDefinitionNames(); return Stream.of(beanNames) .map(beanName -> { - BeanDefinition beanDefinition = - beanRegistry.getBeanDefinition(beanName); + BeanDefinition beanDefinition = beanRegistry.getBeanDefinition(beanName); if (isBeanEligible(beanName, beanDefinition.getBeanClassName())) { return new SimpleEntry<>(beanName, beanDefinition); } @@ -319,9 +409,11 @@ private static boolean isComponentClassValid(Class componentClass) { && !StringUtils.equals(componentClass.getName(), "org.exoplatform.commons.cluster.StartableClusterAware"); } - private static Object getBeanInstance(ApplicationContext applicationContext, String beanName) { - LOG.trace("Retrieve Bean with name '{}' from Spring to Kernel using Application context", - beanName); + private static Object getBeanInstance(ApplicationContext applicationContext, String beanName, String contextName) { + LOG.trace("Retrieve Bean with name '{}' from Spring to Kernel using Application context '{}' in detstination to '{}'", + beanName, + applicationContext.getApplicationName(), + contextName); return applicationContext.getBean(beanName); } @@ -345,7 +437,7 @@ private static T getComponentInstance(PortalContainer portalContainer, Class } private static Class getComponentKeyClass(Object key) { - if (key instanceof Class keyClass) { + if (key instanceof Class keyClass) { // NOSONAR return keyClass; } else if (key instanceof String keyString) { return getClass(keyString); diff --git a/component/common/src/main/java/io/meeds/spring/kernel/PortalApplicationContext.java b/component/common/src/main/java/io/meeds/spring/kernel/PortalApplicationContext.java index 3c61cb6e95..9427c134fc 100644 --- a/component/common/src/main/java/io/meeds/spring/kernel/PortalApplicationContext.java +++ b/component/common/src/main/java/io/meeds/spring/kernel/PortalApplicationContext.java @@ -17,6 +17,9 @@ */ package io.meeds.spring.kernel; +import static io.meeds.spring.kernel.KernelContainerLifecyclePlugin.addSpringContext; + +import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; @@ -25,7 +28,6 @@ import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext; import org.exoplatform.container.PortalContainer; -import org.exoplatform.container.RootContainer.PortalContainerPostInitTask; import jakarta.servlet.ServletContext; @@ -48,20 +50,13 @@ public PortalApplicationContext(ServletContext servletContext, DefaultListableBe @Override protected void finishBeanFactoryInitialization(ConfigurableListableBeanFactory beanFactory) { - BeanDefinitionRegistry beanRegistry = (BeanDefinitionRegistry) beanFactory; // Declare spring Context for integration on Kernel when it will start // ( Kernel container is always started after all Spring contexts are // started, see "PortalContainersCreator" in Meeds-io/meeds project ) - KernelContainerLifecyclePlugin.addSpringContext(servletContext.getServletContextName(), this, beanRegistry); - - // Delay Spring context finishing startup until the Kernel container is - // fully started - PortalContainer.addInitTask(servletContext, new PortalContainerPostInitTask() { - @Override - public void execute(ServletContext context, PortalContainer portalContainer) { - finishSpringContextStartup(beanFactory); - } - }, "portal"); + addSpringContext(servletContext.getServletContextName(), + this, + (BeanDefinitionRegistry) beanFactory, + () -> finishSpringContextStartup(beanFactory)); } @Override @@ -72,11 +67,18 @@ protected void finishRefresh() { private void finishSpringContextStartup(ConfigurableListableBeanFactory beanFactory) { long start = System.currentTimeMillis(); LOG.info("Continue Spring context '{}' initialization", servletContext.getServletContextName()); - PortalApplicationContext.super.finishBeanFactoryInitialization(beanFactory); - PortalApplicationContext.super.finishRefresh(); - LOG.info("Spring context '{}' initialized in {}ms", - servletContext.getServletContextName(), - System.currentTimeMillis() - start); + try { + PortalApplicationContext.super.finishBeanFactoryInitialization(beanFactory); + PortalApplicationContext.super.finishRefresh(); + LOG.info("Spring context '{}' initialized in {}ms", + servletContext.getServletContextName(), + System.currentTimeMillis() - start); + } catch (Exception e) { + throw new IllegalStateException(String.format("Error While finishing Beans Initialization in context '%s' with Bean names [%s] (May be related to issue Meeds-io/meeds#2469)", + servletContext.getServletContextName(), + StringUtils.join(beanFactory.getBeanDefinitionNames(), " , ")), + e); + } } } diff --git a/component/common/src/main/java/io/meeds/spring/kernel/PortalApplicationContextInitializer.java b/component/common/src/main/java/io/meeds/spring/kernel/PortalApplicationContextInitializer.java index 5f5daa7fc7..2da6a8f4a9 100644 --- a/component/common/src/main/java/io/meeds/spring/kernel/PortalApplicationContextInitializer.java +++ b/component/common/src/main/java/io/meeds/spring/kernel/PortalApplicationContextInitializer.java @@ -26,6 +26,10 @@ import jakarta.servlet.ServletContext; import jakarta.servlet.ServletException; +/** + * A super class used to define {@link PortalApplicationContext} as the Spring + * Application Builder to use + */ public abstract class PortalApplicationContextInitializer extends SpringBootServletInitializer { private DefaultListableBeanFactory beanFactory; diff --git a/component/common/src/test/java/io/meeds/spring/kernel/test/SpringBeanFactoryInterceptor.java b/component/common/src/test/java/io/meeds/spring/kernel/test/SpringBeanFactoryInterceptor.java index 7928f00604..ba63483f36 100644 --- a/component/common/src/test/java/io/meeds/spring/kernel/test/SpringBeanFactoryInterceptor.java +++ b/component/common/src/test/java/io/meeds/spring/kernel/test/SpringBeanFactoryInterceptor.java @@ -49,7 +49,7 @@ public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) LOG.info("Integrating Spring Context with Container. Application name = '{}' using Kernel configuration class '{}'", applicationContext.getApplicationName(), getTestClass()); - addSpringContext("test", applicationContext, (BeanDefinitionRegistry) beanFactory); + addSpringContext("test", applicationContext, (BeanDefinitionRegistry) beanFactory, null); bootContainer(getTestClass()); } diff --git a/component/common/src/test/java/io/meeds/spring/module/service/TestService.java b/component/common/src/test/java/io/meeds/spring/module/service/TestService.java index d809727452..8211306bee 100644 --- a/component/common/src/test/java/io/meeds/spring/module/service/TestService.java +++ b/component/common/src/test/java/io/meeds/spring/module/service/TestService.java @@ -1,7 +1,7 @@ /** * This file is part of the Meeds project (https://meeds.io/). * - * Copyright (C) 2020 - 2023 Meeds Association contact@meeds.io + * Copyright (C) 2020 - 2024 Meeds Association contact@meeds.io * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -18,33 +18,10 @@ */ package io.meeds.spring.module.service; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.stereotype.Service; - -import org.exoplatform.commons.api.settings.SettingService; - import io.meeds.spring.module.model.TestModel; -import io.meeds.spring.module.storage.TestStorage; - -@Service -public class TestService { - - // Fake injection from Kernel for Testing Purpose - @Autowired - private SettingService settingService; - @Autowired - private TestStorage storage; +public interface TestService { - public TestModel save(TestModel model) { - if (model == null) { - throw new IllegalArgumentException("TestModel is mandatory"); - } - if (settingService == null) { - // Fake exception - throw new IllegalStateException("SettingService is null"); - } - return storage.save(model); - } + TestModel save(TestModel testModel); } diff --git a/component/common/src/test/java/io/meeds/spring/module/service/TestServiceImpl.java b/component/common/src/test/java/io/meeds/spring/module/service/TestServiceImpl.java new file mode 100644 index 0000000000..a2297ebba5 --- /dev/null +++ b/component/common/src/test/java/io/meeds/spring/module/service/TestServiceImpl.java @@ -0,0 +1,51 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2020 - 2024 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package io.meeds.spring.module.service; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +import org.exoplatform.commons.api.settings.SettingService; + +import io.meeds.spring.module.model.TestModel; +import io.meeds.spring.module.storage.TestStorage; + +@Service +public class TestServiceImpl implements TestService { + + // Fake injection from Kernel for Testing Purpose + @Autowired + private SettingService settingService; + + @Autowired + private TestStorage storage; + + @Override + public TestModel save(TestModel model) { + if (model == null) { + throw new IllegalArgumentException("TestModel is mandatory"); + } + if (settingService == null) { + // Fake exception + throw new IllegalStateException("SettingService is null"); + } + return storage.save(model); + } + +}