Fixed activity registration (#287)

Author: mfateev

temporal-sdk/src/main/java/io/temporal/internal/sync/POJOActivityTaskHandler.java

@@ -147,15 +147,13 @@ public Set<String> getRegisteredActivityTypes() {
     return activities.keySet();
   }
 
-  void setActivitiesImplementation(Object[] activitiesImplementation) {
-    activities.clear();
+  void registerActivityImplementations(Object[] activitiesImplementation) {
     for (Object activity : activitiesImplementation) {
       addActivityImplementation(activity, POJOActivityImplementation::new);
     }
   }
 
-  void setLocalActivitiesImplementation(Object[] activitiesImplementation) {
-    activities.clear();
+  void registerLocalActivityImplementations(Object[] activitiesImplementation) {
     for (Object activity : activitiesImplementation) {
       addActivityImplementation(activity, POJOLocalActivityImplementation::new);
     }

temporal-sdk/src/main/java/io/temporal/internal/sync/SyncActivityWorker.java

@@ -55,8 +55,8 @@ public SyncActivityWorker(
             service, namespace, taskQueue, taskQueueActivitiesPerSecond, options, taskHandler);
   }
 
-  public void setActivitiesImplementation(Object... activitiesImplementation) {
-    taskHandler.setActivitiesImplementation(activitiesImplementation);
+  public void registerActivityImplementations(Object... activitiesImplementation) {
+    taskHandler.registerActivityImplementations(activitiesImplementation);
   }
 
   @Override

temporal-sdk/src/main/java/io/temporal/internal/sync/SyncWorkflowWorker.java

@@ -121,8 +121,8 @@ public <R> void addWorkflowImplementationFactory(Class<R> clazz, Func<R> factory
     this.factory.addWorkflowImplementationFactory(clazz, factory);
   }
 
-  public void setLocalActivitiesImplementation(Object... activitiesImplementation) {
-    this.laTaskHandler.setLocalActivitiesImplementation(activitiesImplementation);
+  public void registerLocalActivityImplementations(Object... activitiesImplementation) {
+    this.laTaskHandler.registerLocalActivityImplementations(activitiesImplementation);
   }
 
   @Override

temporal-sdk/src/main/java/io/temporal/internal/sync/TestActivityEnvironmentInternal.java

@@ -169,7 +169,7 @@ public void recordActivityTaskHeartbeat(
    */
   @Override
   public void registerActivitiesImplementations(Object... activityImplementations) {
-    activityTaskHandler.setActivitiesImplementation(activityImplementations);
+    activityTaskHandler.registerActivityImplementations(activityImplementations);
   }
 
   /**

temporal-sdk/src/main/java/io/temporal/worker/Worker.java

@@ -317,9 +317,9 @@ public void registerActivitiesImplementations(Object... activityImplementations)
         "registerActivitiesImplementations is not allowed after worker has started");
 
     if (activityWorker != null) {
-      activityWorker.setActivitiesImplementation(activityImplementations);
+      activityWorker.registerActivityImplementations(activityImplementations);
     }
-    workflowWorker.setLocalActivitiesImplementation(activityImplementations);
+    workflowWorker.registerLocalActivityImplementations(activityImplementations);
   }
 
   void start() {

temporal-sdk/src/test/java/io/temporal/internal/testing/ActivityTestingTest.java

@@ -19,7 +19,9 @@
 
 package io.temporal.internal.testing;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import io.grpc.Status;
 import io.temporal.activity.Activity;
@@ -353,68 +355,107 @@ public void e() {
   }
 
   @Test
-  public void testInvokingActivityByBaseInterface() {
+  public void testInvokingActivityByBaseInterface1() {
     BImpl bImpl = new BImpl();
     DImpl dImpl = new DImpl();
-    EImpl eImpl = new EImpl();
-    {
-      testEnvironment.registerActivitiesImplementations(bImpl, dImpl);
-      try {
-        testEnvironment.newActivityStub(A.class);
-        fail("A doesn't implement activity");
-      } catch (IllegalArgumentException e) {
-        // expected as A doesn't implement any activity
-      }
-      B b = testEnvironment.newActivityStub(B.class);
-      b.a();
-      b.b();
-      A a = b;
-      a.a();
-      D d = testEnvironment.newActivityStub(D.class);
-      d.a();
-      d.d();
-      a = d;
-      a.a();
-      List<String> expectedB = new ArrayList<>();
-      expectedB.add("a");
-      expectedB.add("b");
-      expectedB.add("a");
-      assertEquals(expectedB, bImpl.invocations);
-
-      List<String> expectedD = new ArrayList<>();
-      expectedD.add("a");
-      expectedD.add("d");
-      expectedD.add("a");
-      assertEquals(expectedD, dImpl.invocations);
+    testEnvironment.registerActivitiesImplementations(bImpl, dImpl);
+    try {
+      testEnvironment.newActivityStub(A.class);
+      fail("A doesn't implement activity");
+    } catch (IllegalArgumentException e) {
+      // expected as A doesn't implement any activity
     }
-    {
-      testEnvironment.registerActivitiesImplementations(eImpl);
-      E e = testEnvironment.newActivityStub(E.class);
-      e.a();
-      e.d();
-      e.e();
-      D d = testEnvironment.newActivityStub(D.class);
-      d.a();
-      d.d();
-      List<String> expectedE = new ArrayList<>();
-      expectedE.add("a");
-      expectedE.add("d");
-      expectedE.add("e");
-      expectedE.add("a");
-      expectedE.add("d");
-      assertEquals(expectedE, eImpl.invocations);
+    B b = testEnvironment.newActivityStub(B.class);
+    b.a();
+    b.b();
+    A a = b;
+    a.a();
+    D d = testEnvironment.newActivityStub(D.class);
+    d.a();
+    d.d();
+    a = d;
+    a.a();
+    List<String> expectedB = new ArrayList<>();
+    expectedB.add("a");
+    expectedB.add("b");
+    expectedB.add("a");
+    assertEquals(expectedB, bImpl.invocations);
+
+    List<String> expectedD = new ArrayList<>();
+    expectedD.add("a");
+    expectedD.add("d");
+    expectedD.add("a");
+    assertEquals(expectedD, dImpl.invocations);
+  }
+
+  @Test
+  public void testInvokingActivityByBaseInterface1CallRegisterTwice() {
+    BImpl bImpl = new BImpl();
+    DImpl dImpl = new DImpl();
+    testEnvironment.registerActivitiesImplementations(bImpl);
+    testEnvironment.registerActivitiesImplementations(dImpl);
+    try {
+      testEnvironment.newActivityStub(A.class);
+      fail("A doesn't implement activity");
+    } catch (IllegalArgumentException e) {
+      // expected as A doesn't implement any activity
     }
+    B b = testEnvironment.newActivityStub(B.class);
+    b.a();
+    b.b();
+    A a = b;
+    a.a();
+    D d = testEnvironment.newActivityStub(D.class);
+    d.a();
+    d.d();
+    a = d;
+    a.a();
+    List<String> expectedB = new ArrayList<>();
+    expectedB.add("a");
+    expectedB.add("b");
+    expectedB.add("a");
+    assertEquals(expectedB, bImpl.invocations);
+
+    List<String> expectedD = new ArrayList<>();
+    expectedD.add("a");
+    expectedD.add("d");
+    expectedD.add("a");
+    assertEquals(expectedD, dImpl.invocations);
   }
 
   @Test
-  public void testDuplicates() {
+  public void testInvokingActivityByBaseInterface2() {
+    EImpl eImpl = new EImpl();
+    testEnvironment.registerActivitiesImplementations(eImpl);
+    E e = testEnvironment.newActivityStub(E.class);
+    e.a();
+    e.d();
+    e.e();
+    D d = testEnvironment.newActivityStub(D.class);
+    d.a();
+    d.d();
+    List<String> expectedE = new ArrayList<>();
+    expectedE.add("a");
+    expectedE.add("d");
+    expectedE.add("e");
+    expectedE.add("a");
+    expectedE.add("d");
+    assertEquals(expectedE, eImpl.invocations);
+  }
+
+  @Test
+  public void testDuplicates1() {
     try {
       CImpl cImpl = new CImpl();
       testEnvironment.registerActivitiesImplementations(cImpl);
     } catch (IllegalArgumentException e) {
       assertTrue(e.getMessage().contains("A.a()"));
       assertTrue(e.getMessage().contains("Duplicated"));
     }
+  }
+
+  @Test
+  public void testDuplicates2() {
     DImpl dImpl = new DImpl();
     EImpl eImpl = new EImpl();
     try {
@@ -424,4 +465,17 @@ public void testDuplicates() {
       assertTrue(e.getMessage().contains("already registered"));
     }
   }
+
+  @Test
+  public void testDuplicates2CallRegisterTwice() {
+    DImpl dImpl = new DImpl();
+    EImpl eImpl = new EImpl();
+    try {
+      testEnvironment.registerActivitiesImplementations(dImpl);
+      testEnvironment.registerActivitiesImplementations(eImpl);
+    } catch (IllegalArgumentException e) {
+      assertTrue(e.getMessage().contains("a"));
+      assertTrue(e.getMessage().contains("already registered"));
+    }
+  }
 }