Fix some dependency issues with NonRootBeanPostProcessor (#2556)

Fix some dependency issues with NonRootBeanPostProcessor

Author: Quinn-With-Two-Ns

temporal-spring-boot-autoconfigure/src/main/java/io/temporal/spring/boot/autoconfigure/NamespacesPresentCondition.java

@@ -0,0 +1,34 @@
+package io.temporal.spring.boot.autoconfigure;
+
+import io.temporal.spring.boot.autoconfigure.properties.NonRootNamespaceProperties;
+import java.util.List;
+import org.springframework.boot.autoconfigure.condition.ConditionMessage;
+import org.springframework.boot.autoconfigure.condition.ConditionOutcome;
+import org.springframework.boot.autoconfigure.condition.SpringBootCondition;
+import org.springframework.boot.context.properties.bind.BindResult;
+import org.springframework.boot.context.properties.bind.Bindable;
+import org.springframework.boot.context.properties.bind.Binder;
+import org.springframework.context.annotation.ConditionContext;
+import org.springframework.core.type.AnnotatedTypeMetadata;
+
+/**
+ * Condition that checks if the "spring.temporal.namespaces" property is present in the application
+ * context.
+ */
+class NamespacesPresentCondition extends SpringBootCondition {
+  private static final Bindable<List<NonRootNamespaceProperties>> NAMESPACES_LIST =
+      Bindable.listOf(NonRootNamespaceProperties.class);
+  private static final String NAMESPACES_KEY = "spring.temporal.namespaces";
+
+  @Override
+  public ConditionOutcome getMatchOutcome(
+      ConditionContext context, AnnotatedTypeMetadata metadata) {
+    BindResult<?> namespacesProperty =
+        Binder.get(context.getEnvironment()).bind(NAMESPACES_KEY, NAMESPACES_LIST);
+    ConditionMessage.Builder messageBuilder = ConditionMessage.forCondition("Present namespaces");
+    if (namespacesProperty.isBound()) {
+      return ConditionOutcome.match(messageBuilder.found("property").items(NAMESPACES_KEY));
+    }
+    return ConditionOutcome.noMatch(messageBuilder.didNotFind("property").items(NAMESPACES_KEY));
+  }
+}

temporal-spring-boot-autoconfigure/src/main/java/io/temporal/spring/boot/autoconfigure/NonRootBeanPostProcessor.java

@@ -33,6 +33,7 @@
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
+import org.springframework.beans.factory.BeanNotOfRequiredTypeException;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.beans.factory.config.BeanPostProcessor;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
@@ -45,26 +46,32 @@ public class NonRootBeanPostProcessor implements BeanPostProcessor, BeanFactoryA
 
   private final @Nonnull TemporalProperties temporalProperties;
   private final @Nullable List<NonRootNamespaceProperties> namespaceProperties;
-  private final @Nullable Tracer tracer;
-  private final @Nullable TestWorkflowEnvironmentAdapter testWorkflowEnvironment;
-  private final @Nullable Scope metricsScope;
-
-  public NonRootBeanPostProcessor(
-      @Nonnull TemporalProperties temporalProperties,
-      @Nullable Tracer tracer,
-      @Nullable TestWorkflowEnvironmentAdapter testWorkflowEnvironment,
-      @Nullable Scope metricsScope) {
+  private @Nullable Tracer tracer;
+  private @Nullable TestWorkflowEnvironmentAdapter testWorkflowEnvironment;
+  private @Nullable Scope metricsScope;
+
+  public NonRootBeanPostProcessor(@Nonnull TemporalProperties temporalProperties) {
     this.temporalProperties = temporalProperties;
     this.namespaceProperties = temporalProperties.getNamespaces();
-    this.tracer = tracer;
-    this.testWorkflowEnvironment = testWorkflowEnvironment;
-    this.metricsScope = metricsScope;
   }
 
   @Override
-  public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {
+  public Object postProcessAfterInitialization(@Nonnull Object bean, @Nonnull String beanName)
+      throws BeansException {
     if (bean instanceof NamespaceTemplate && beanName.equals("temporalRootNamespaceTemplate")) {
       if (namespaceProperties != null) {
+        // If there are non-root namespaces, we need to inject beans for each of them. Look
+        // up the bean manually instead of using @Autowired to avoid circular dependencies or
+        // causing the dependency to
+        // get initialized to early and skip post-processing.
+        //
+        // Note: We don't use @Lazy here because these dependencies are optional and @Lazy doesn't
+        // interact well with
+        // optional dependencies.
+        metricsScope = findBean("temporalMetricsScope", Scope.class);
+        tracer = findBean(Tracer.class);
+        testWorkflowEnvironment =
+            findBean("temporalTestWorkflowEnvironment", TestWorkflowEnvironmentAdapter.class);
         namespaceProperties.forEach(this::injectBeanByNonRootNamespace);
       }
     }
@@ -163,6 +170,24 @@ private <T> T findBeanByNamespace(String beanPrefix, Class<T> clazz) {
     return null;
   }
 
+  private <T> @Nullable T findBean(Class<T> clazz) {
+    try {
+      return beanFactory.getBean(clazz);
+    } catch (NoSuchBeanDefinitionException | BeanNotOfRequiredTypeException ignore) {
+      // Ignore if the bean is not found or not of the required type
+    }
+    return null;
+  }
+
+  private <T> @Nullable T findBean(String beanName, Class<T> clazz) {
+    try {
+      return beanFactory.getBean(beanName, clazz);
+    } catch (NoSuchBeanDefinitionException | BeanNotOfRequiredTypeException ignore) {
+      // Ignore if the bean is not found or not of the required type
+    }
+    return null;
+  }
+
   private <T> TemporalOptionsCustomizer<T> findBeanByNameSpaceForTemporalCustomizer(
       String beanPrefix, Class<T> genericOptionsBuilderClass) {
     String beanName =

temporal-spring-boot-autoconfigure/src/main/java/io/temporal/spring/boot/autoconfigure/NonRootNamespaceAutoConfiguration.java

@@ -1,12 +1,9 @@
 package io.temporal.spring.boot.autoconfigure;
 
 import com.google.common.base.MoreObjects;
-import com.uber.m3.tally.Scope;
-import io.opentracing.Tracer;
 import io.temporal.spring.boot.autoconfigure.properties.NamespaceProperties;
 import io.temporal.spring.boot.autoconfigure.properties.NonRootNamespaceProperties;
 import io.temporal.spring.boot.autoconfigure.properties.TemporalProperties;
-import io.temporal.spring.boot.autoconfigure.template.TestWorkflowEnvironmentAdapter;
 import io.temporal.spring.boot.autoconfigure.template.WorkersTemplate;
 import java.util.List;
 import java.util.Optional;
@@ -15,8 +12,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.BeansException;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.boot.autoconfigure.AutoConfigureAfter;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
@@ -25,6 +20,7 @@
 import org.springframework.context.ApplicationContextAware;
 import org.springframework.context.ApplicationListener;
 import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Conditional;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.context.event.ApplicationContextEvent;
@@ -35,6 +31,7 @@
 @EnableConfigurationProperties(TemporalProperties.class)
 @AutoConfigureAfter({RootNamespaceAutoConfiguration.class, ServiceStubsAutoConfiguration.class})
 @ConditionalOnBean(ServiceStubsAutoConfiguration.class)
+@Conditional(NamespacesPresentCondition.class)
 @ConditionalOnExpression(
     "${spring.temporal.test-server.enabled:false} || '${spring.temporal.connection.target:}'.length() > 0")
 public class NonRootNamespaceAutoConfiguration {
@@ -43,20 +40,14 @@ public class NonRootNamespaceAutoConfiguration {
       LoggerFactory.getLogger(NonRootNamespaceAutoConfiguration.class);
 
   @Bean
-  public NonRootBeanPostProcessor nonRootBeanPostProcessor(
-      TemporalProperties properties,
-      @Autowired(required = false) @Nullable Tracer otTracer,
-      @Qualifier("temporalTestWorkflowEnvironmentAdapter") @Autowired(required = false) @Nullable
-          TestWorkflowEnvironmentAdapter testWorkflowEnvironment,
-      @Qualifier("temporalMetricsScope") @Autowired(required = false) @Nullable
-          Scope metricsScope) {
-    return new NonRootBeanPostProcessor(
-        properties, otTracer, testWorkflowEnvironment, metricsScope);
+  public static NonRootBeanPostProcessor nonRootBeanPostProcessor(
+      @Lazy TemporalProperties properties) {
+    return new NonRootBeanPostProcessor(properties);
   }
 
   @Bean
-  public NonRootNamespaceEventListener nonRootNamespaceEventListener(
-      TemporalProperties temporalProperties,
+  public static NonRootNamespaceEventListener nonRootNamespaceEventListener(
+      @Lazy TemporalProperties temporalProperties,
       @Nullable @Lazy List<WorkersTemplate> workersTemplates) {
     return new NonRootNamespaceEventListener(temporalProperties, workersTemplates);
   }

temporal-spring-boot-autoconfigure/src/test/java/io/temporal/spring/boot/autoconfigure/MultiNamespaceTest.java

@@ -36,6 +36,7 @@ void setUp() {
   @Test
   @Timeout(value = 10)
   public void shouldContainsNonRootRelatedBean() {
+    Assertions.assertTrue(applicationContext.containsBean("nonRootBeanPostProcessor"));
     Assertions.assertTrue(applicationContext.containsBean("ns1NamespaceTemplate"));
     Assertions.assertTrue(applicationContext.containsBean("namespace2NamespaceTemplate"));
     Assertions.assertTrue(applicationContext.containsBean("ns1ClientTemplate"));

temporal-spring-boot-autoconfigure/src/test/java/io/temporal/spring/boot/autoconfigure/StartWorkersTest.java

@@ -5,11 +5,13 @@
 import io.temporal.spring.boot.autoconfigure.properties.TemporalProperties;
 import io.temporal.testing.TestWorkflowEnvironment;
 import io.temporal.worker.Worker;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInstance;
 import org.junit.jupiter.api.Timeout;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.annotation.ComponentScan;
 import org.springframework.context.annotation.FilterType;
 import org.springframework.test.context.ActiveProfiles;
@@ -18,6 +20,7 @@
 @ActiveProfiles(profiles = "disable-start-workers")
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 public class StartWorkersTest {
+  @Autowired ConfigurableApplicationContext applicationContext;
 
   @Autowired TemporalProperties temporalProperties;
 
@@ -33,6 +36,7 @@ public void testStartWorkersConfigDisabled() {
   @Timeout(value = 10)
   public void testWorkersStarted() {
     Worker worker = testWorkflowEnvironment.getWorkerFactory().getWorker("UnitTest");
+    Assertions.assertFalse(applicationContext.containsBean("nonRootBeanPostProcessor"));
     assertNotNull(worker);
     assertTrue(worker.isSuspended());
   }