diff --git a/temporal-sdk/src/main/java/io/temporal/internal/sync/POJOActivityTaskHandler.java b/temporal-sdk/src/main/java/io/temporal/internal/sync/POJOActivityTaskHandler.java index f0489a897e..a1ae391434 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/sync/POJOActivityTaskHandler.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/sync/POJOActivityTaskHandler.java @@ -147,15 +147,13 @@ public Set 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); } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/sync/SyncActivityWorker.java b/temporal-sdk/src/main/java/io/temporal/internal/sync/SyncActivityWorker.java index c1582df6f1..a125dbe543 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/sync/SyncActivityWorker.java +++ b/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 diff --git a/temporal-sdk/src/main/java/io/temporal/internal/sync/SyncWorkflowWorker.java b/temporal-sdk/src/main/java/io/temporal/internal/sync/SyncWorkflowWorker.java index 694a403805..e12707e852 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/sync/SyncWorkflowWorker.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/sync/SyncWorkflowWorker.java @@ -121,8 +121,8 @@ public void addWorkflowImplementationFactory(Class clazz, Func 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 diff --git a/temporal-sdk/src/main/java/io/temporal/internal/sync/TestActivityEnvironmentInternal.java b/temporal-sdk/src/main/java/io/temporal/internal/sync/TestActivityEnvironmentInternal.java index 918aa9fd18..1410b138b6 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/sync/TestActivityEnvironmentInternal.java +++ b/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); } /** diff --git a/temporal-sdk/src/main/java/io/temporal/worker/Worker.java b/temporal-sdk/src/main/java/io/temporal/worker/Worker.java index 387148f182..040aab8b2b 100644 --- a/temporal-sdk/src/main/java/io/temporal/worker/Worker.java +++ b/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() { diff --git a/temporal-sdk/src/test/java/io/temporal/internal/testing/ActivityTestingTest.java b/temporal-sdk/src/test/java/io/temporal/internal/testing/ActivityTestingTest.java index 4302bb7ac8..c5d3fb1ea0 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/testing/ActivityTestingTest.java +++ b/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,61 +355,96 @@ 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 expectedB = new ArrayList<>(); - expectedB.add("a"); - expectedB.add("b"); - expectedB.add("a"); - assertEquals(expectedB, bImpl.invocations); - - List 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 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 expectedB = new ArrayList<>(); + expectedB.add("a"); + expectedB.add("b"); + expectedB.add("a"); + assertEquals(expectedB, bImpl.invocations); + + List 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 expectedB = new ArrayList<>(); + expectedB.add("a"); + expectedB.add("b"); + expectedB.add("a"); + assertEquals(expectedB, bImpl.invocations); + + List 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 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); @@ -415,6 +452,10 @@ public void testDuplicates() { 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")); + } + } }