Don't fail Springboot if a bean is specified twice (#2558)

Don't fail Springboot if a bean is specified twice

Author: Quinn-With-Two-Ns

temporal-spring-boot-autoconfigure/src/main/java/io/temporal/spring/boot/autoconfigure/template/WorkersTemplate.java

@@ -409,6 +409,17 @@ private void configureActivityImplementationAutoDiscovery(
       String byWorkerName,
       Workers workers) {
     try {
+      if (registeredInfo.containsKey(worker.getTaskQueue())
+          && registeredInfo.get(worker.getTaskQueue()).isActivityRegistered(beanName)) {
+        if (log.isInfoEnabled()) {
+          log.debug(
+              "Activity bean {} is already registered on a worker {} with a task queue '{}'",
+              beanName,
+              byWorkerName != null ? "'" + byWorkerName + "' " : "",
+              worker.getTaskQueue());
+        }
+        return; // already registered
+      }
       worker.registerActivitiesImplementations(bean);
       POJOActivityImplMetadata activityImplMetadata =
           POJOActivityImplMetadata.newInstance(AopUtils.getTargetClass(bean));
@@ -446,6 +457,17 @@ private void configureNexusServiceImplementationAutoDiscovery(
       String byWorkerName,
       Workers workers) {
     try {
+      if (registeredInfo.containsKey(worker.getTaskQueue())
+          && registeredInfo.get(worker.getTaskQueue()).isNexusServiceRegistered(beanName)) {
+        if (log.isInfoEnabled()) {
+          log.debug(
+              "Nexus service bean {} is already registered on a worker {} with a task queue '{}'",
+              beanName,
+              byWorkerName != null ? "'" + byWorkerName + "' " : "",
+              worker.getTaskQueue());
+        }
+        return; // already registered
+      }
       worker.registerNexusServiceImplementation(bean);
       addRegisteredNexusServiceImpl(
           worker,
@@ -480,10 +502,21 @@ private void configureNexusServiceImplementationAutoDiscovery(
   private void configureWorkflowImplementationAutoDiscovery(
       Worker worker, Class<?> clazz, String byWorkerName, Workers workers) {
     try {
+      if (registeredInfo.containsKey(worker.getTaskQueue())
+          && registeredInfo.get(worker.getTaskQueue()).isWorkflowRegistered(clazz)) {
+        if (log.isInfoEnabled()) {
+          log.debug(
+              "Workflow class {} is already registered on a worker {} with a task queue '{}'",
+              clazz,
+              byWorkerName != null ? "'" + byWorkerName + "' " : "",
+              worker.getTaskQueue());
+        }
+        return; // already registered
+      }
       configureWorkflowImplementation(worker, clazz);
       if (log.isInfoEnabled()) {
         log.info(
-            "Registering auto-discovered workflow class {} on a worker {}with a task queue '{}'",
+            "Registering auto-discovered workflow class {} on a worker {} with a task queue '{}'",
             clazz,
             byWorkerName != null ? "'" + byWorkerName + "' " : "",
             worker.getTaskQueue());
@@ -494,7 +527,7 @@ private void configureWorkflowImplementationAutoDiscovery(
       }
       if (log.isInfoEnabled()) {
         log.info(
-            "Skip registering of auto-discovered workflow class {} on a worker {}with a task queue '{}' "
+            "Skipping registering of auto-discovered workflow class {} on a worker {} with a task queue '{}' "
                 + "as workflow type '{}' is already registered on the worker",
             clazz,
             byWorkerName != null ? "'" + byWorkerName + "' " : "",
@@ -605,7 +638,7 @@ private <T> void configureWorkflowImplementation(Worker worker, Class<?> clazz)
           },
           workflowImplementationOptions);
       addRegisteredWorkflowImpl(
-          worker, workflowMethod.getWorkflowInterface().getName(), workflowMetadata);
+          worker, clazz, workflowMethod.getWorkflowInterface().getName(), workflowMetadata);
     } else {
       for (POJOWorkflowMethodMetadata workflowMethod : workflowMetadata.getWorkflowMethods()) {
         if (deploymentOptions != null && deploymentOptions.isUsingVersioning()) {
@@ -623,7 +656,7 @@ private <T> void configureWorkflowImplementation(Worker worker, Class<?> clazz)
             () -> (T) beanFactory.createBean(clazz),
             workflowImplementationOptions);
         addRegisteredWorkflowImpl(
-            worker, workflowMethod.getWorkflowInterface().getName(), workflowMetadata);
+            worker, clazz, workflowMethod.getWorkflowInterface().getName(), workflowMetadata);
       }
     }
   }
@@ -658,97 +691,82 @@ private Worker createNewWorker(
   }
 
   private void addRegisteredWorkflowImpl(
-      Worker worker, String workflowClass, POJOWorkflowImplMetadata metadata) {
-    if (!registeredInfo.containsKey(worker.getTaskQueue())) {
-      registeredInfo.put(
-          worker.getTaskQueue(),
-          new RegisteredInfo()
-              .addWorkflowInfo(
-                  new RegisteredWorkflowInfo().addClassName(workflowClass).addMetadata(metadata)));
-    } else {
-      registeredInfo
-          .get(worker.getTaskQueue())
-          .getRegisteredWorkflowInfo()
-          .add(new RegisteredWorkflowInfo().addClassName(workflowClass).addMetadata(metadata));
-    }
+      Worker worker, Class<?> clazz, String workflowClass, POJOWorkflowImplMetadata metadata) {
+    registeredInfo
+        .computeIfAbsent(worker.getTaskQueue(), (k) -> new RegisteredInfo())
+        .addWorkflowInfo(
+            new RegisteredWorkflowInfo()
+                .addImplementationClass(clazz)
+                .addClassName(workflowClass)
+                .addMetadata(metadata));
   }
 
   private void addRegisteredActivityImpl(
       Worker worker, String beanName, String beanClass, POJOActivityImplMetadata metadata) {
-    if (!registeredInfo.containsKey(worker.getTaskQueue())) {
-      registeredInfo.put(
-          worker.getTaskQueue(),
-          new RegisteredInfo()
-              .addActivityInfo(
-                  new RegisteredActivityInfo()
-                      .addBeanName(beanName)
-                      .addClassName(beanClass)
-                      .addMetadata(metadata)));
-    } else {
-      registeredInfo
-          .get(worker.getTaskQueue())
-          .getRegisteredActivityInfo()
-          .add(
-              new RegisteredActivityInfo()
-                  .addBeanName(beanName)
-                  .addClassName(beanClass)
-                  .addMetadata(metadata));
-    }
+    registeredInfo
+        .computeIfAbsent(worker.getTaskQueue(), (k) -> new RegisteredInfo())
+        .addActivityInfo(
+            new RegisteredActivityInfo()
+                .addBeanName(beanName)
+                .addClassName(beanClass)
+                .addMetadata(metadata));
   }
 
   private void addRegisteredNexusServiceImpl(
       Worker worker, String beanName, String beanClass, ServiceDefinition serviceDefinition) {
-    if (!registeredInfo.containsKey(worker.getTaskQueue())) {
-      registeredInfo.put(
-          worker.getTaskQueue(),
-          new RegisteredInfo()
-              .addNexusServiceInfo(
-                  new RegisteredNexusServiceInfo()
-                      .addBeanName(beanName)
-                      .addClassName(beanClass)
-                      .addDefinition(serviceDefinition)));
-    } else {
-      registeredInfo
-          .get(worker.getTaskQueue())
-          .getRegisteredNexusServiceInfos()
-          .add(
-              new RegisteredNexusServiceInfo()
-                  .addBeanName(beanName)
-                  .addClassName(beanClass)
-                  .addDefinition(serviceDefinition));
-    }
+    registeredInfo
+        .computeIfAbsent(worker.getTaskQueue(), (k) -> new RegisteredInfo())
+        .addNexusServiceInfo(
+            new RegisteredNexusServiceInfo()
+                .addBeanName(beanName)
+                .addClassName(beanClass)
+                .addDefinition(serviceDefinition));
   }
 
   public static class RegisteredInfo {
-    private final List<RegisteredActivityInfo> registeredActivityInfo = new ArrayList<>();
-    private final List<RegisteredWorkflowInfo> registeredWorkflowInfo = new ArrayList<>();
-    private final List<RegisteredNexusServiceInfo> registeredNexusServiceInfos = new ArrayList<>();
+    private final HashMap<Class<?>, RegisteredWorkflowInfo> registeredWorkflowInfo =
+        new HashMap<>();
+    private final HashMap<String, RegisteredActivityInfo> registeredActivityInfo = new HashMap<>();
+    private final HashMap<String, RegisteredNexusServiceInfo> registeredNexusServiceInfos =
+        new HashMap<>();
 
     public RegisteredInfo addActivityInfo(RegisteredActivityInfo activityInfo) {
-      registeredActivityInfo.add(activityInfo);
+      registeredActivityInfo.put(activityInfo.getBeanName(), activityInfo);
       return this;
     }
 
     public RegisteredInfo addNexusServiceInfo(RegisteredNexusServiceInfo nexusServiceInfo) {
-      registeredNexusServiceInfos.add(nexusServiceInfo);
+      registeredNexusServiceInfos.put(nexusServiceInfo.getBeanName(), nexusServiceInfo);
       return this;
     }
 
     public RegisteredInfo addWorkflowInfo(RegisteredWorkflowInfo workflowInfo) {
-      registeredWorkflowInfo.add(workflowInfo);
+      registeredWorkflowInfo.put(workflowInfo.getImplementationClass(), workflowInfo);
       return this;
     }
 
+    public boolean isWorkflowRegistered(Class<?> workflowClass) {
+      return registeredWorkflowInfo.containsKey(workflowClass);
+    }
+
+    public boolean isActivityRegistered(String beanName) {
+      return registeredActivityInfo.containsKey(beanName);
+    }
+
+    public boolean isNexusServiceRegistered(String beanName) {
+      return registeredNexusServiceInfos.containsKey(beanName);
+    }
+
     public List<RegisteredActivityInfo> getRegisteredActivityInfo() {
-      return registeredActivityInfo;
+      return new ArrayList<>(registeredActivityInfo.values());
     }
 
     public List<RegisteredWorkflowInfo> getRegisteredWorkflowInfo() {
-      return registeredWorkflowInfo;
+      return new ArrayList<>(registeredWorkflowInfo.values());
     }
 
     public List<RegisteredNexusServiceInfo> getRegisteredNexusServiceInfos() {
-      return registeredNexusServiceInfos;
+      return new ArrayList<>(registeredNexusServiceInfos.values());
     }
   }
 
@@ -822,6 +840,7 @@ public ServiceDefinition getDefinition() {
 
   @Experimental
   public static class RegisteredWorkflowInfo {
+    private Class<?> implementationClass;
     private String className;
     private POJOWorkflowImplMetadata metadata;
 
@@ -830,6 +849,11 @@ public RegisteredWorkflowInfo addClassName(String className) {
       return this;
     }
 
+    public RegisteredWorkflowInfo addImplementationClass(Class<?> implementationClass) {
+      this.implementationClass = implementationClass;
+      return this;
+    }
+
     public RegisteredWorkflowInfo addMetadata(POJOWorkflowImplMetadata metadata) {
       this.metadata = metadata;
       return this;
@@ -842,6 +866,10 @@ public String getClassName() {
     public POJOWorkflowImplMetadata getMetadata() {
       return metadata;
     }
+
+    public Class<?> getImplementationClass() {
+      return implementationClass;
+    }
   }
 
   private static class Workers {

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

@@ -0,0 +1,50 @@
+package io.temporal.spring.boot.autoconfigure;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import io.temporal.client.WorkflowClient;
+import io.temporal.client.WorkflowOptions;
+import io.temporal.spring.boot.autoconfigure.bytaskqueue.TestWorkflow;
+import io.temporal.testing.TestWorkflowEnvironment;
+import org.junit.jupiter.api.BeforeEach;
+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;
+
+@SpringBootTest(classes = AutoDiscoveryAndExplicitWithDuplicatesTest.Configuration.class)
+@ActiveProfiles(profiles = "auto-discovery-and-explicit-with-duplicate")
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+public class AutoDiscoveryAndExplicitWithDuplicatesTest {
+  @Autowired ConfigurableApplicationContext applicationContext;
+
+  @Autowired TestWorkflowEnvironment testWorkflowEnvironment;
+
+  @Autowired WorkflowClient workflowClient;
+
+  @BeforeEach
+  void setUp() {
+    applicationContext.start();
+  }
+
+  @Test
+  @Timeout(value = 10)
+  public void testAutoDiscoveryAndExplicitWithDuplicate() {
+    TestWorkflow testWorkflow =
+        workflowClient.newWorkflowStub(
+            TestWorkflow.class, WorkflowOptions.newBuilder().setTaskQueue("UnitTest").build());
+    assertEquals("done", testWorkflow.execute("profile"));
+  }
+
+  @ComponentScan(
+      excludeFilters =
+          @ComponentScan.Filter(
+              pattern = "io\\.temporal\\.spring\\.boot\\.autoconfigure\\.bytaskqueue\\..*",
+              type = FilterType.REGEX))
+  public static class Configuration {}
+}

temporal-spring-boot-autoconfigure/src/test/resources/application.yml

@@ -72,6 +72,23 @@ spring:
       packages:
         - io.temporal.spring.boot.autoconfigure.byworkername
 
+---
+spring:
+  config:
+    activate:
+      on-profile: auto-discovery-and-explicit-with-duplicate
+  temporal:
+    workers:
+      - task-queue: UnitTest
+        name: mainWorker
+        workflow-classes:
+          - io.temporal.spring.boot.autoconfigure.byworkername.TestWorkflowImpl
+        activity-beans:
+          - TestActivityImpl
+    workers-auto-discovery:
+      packages:
+        - io.temporal.spring.boot.autoconfigure.byworkername
+
 ---
 spring:
   config: