Testworkflowrule Interceptor Fix (temporalio#421)

vkoby · web-flow · commit ba691a245bfb · 2021-04-02T15:48:43.000-07:00
* Fixed a bug where TestWorkflowRule was merging workerInterceptors from two sources incorrectly
* Added Unit tests to prevent this in the future
diff --git a/temporal-sdk/src/main/java/io/temporal/worker/WorkflowImplementationOptions.java b/temporal-sdk/src/main/java/io/temporal/worker/WorkflowImplementationOptions.java
@@ -23,6 +23,16 @@
 
 public final class WorkflowImplementationOptions {
 
+  private static final WorkflowImplementationOptions DEFAULT_INSTANCE;
+
+  static {
+    DEFAULT_INSTANCE = WorkflowImplementationOptions.newBuilder().build();
+  }
+
+  public static WorkflowImplementationOptions getDefaultInstance() {
+    return DEFAULT_INSTANCE;
+  }
+
   public static Builder newBuilder() {
     return new Builder();
   }
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/ContinueAsNewNoArgsTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/ContinueAsNewNoArgsTest.java
@@ -29,11 +29,7 @@ public class ContinueAsNewNoArgsTest {
 
   @Rule
   public SDKTestWorkflowRule testWorkflowRule =
-      SDKTestWorkflowRule.newBuilder()
-          .setWorkflowTypes(TestContinueAsNewNoArgsImpl.class)
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
-          .build();
+      SDKTestWorkflowRule.newBuilder().setWorkflowTypes(TestContinueAsNewNoArgsImpl.class).build();
 
   @Test
   public void testContinueAsNewNoArgs() {
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/ContinueAsNewTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/ContinueAsNewTest.java
@@ -33,11 +33,7 @@ public class ContinueAsNewTest {
 
   @Rule
   public SDKTestWorkflowRule testWorkflowRule =
-      SDKTestWorkflowRule.newBuilder()
-          .setWorkflowTypes(TestContinueAsNewImpl.class)
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
-          .build();
+      SDKTestWorkflowRule.newBuilder().setWorkflowTypes(TestContinueAsNewImpl.class).build();
 
   @Test
   public void testContinueAsNew() {
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/SagaTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/SagaTest.java
@@ -39,8 +39,6 @@ public class SagaTest {
               TestCompensationWorkflowImpl.class,
               TestMultiargsWorkflowsFuncImpl.class)
           .setActivityImplementations(new TestActivities.TestActivitiesImpl())
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
           .build();
 
   @Test
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/SideEffectTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/SideEffectTest.java
@@ -38,8 +38,6 @@ public class SideEffectTest {
       SDKTestWorkflowRule.newBuilder()
           .setWorkflowTypes(TestSideEffectWorkflowImpl.class)
           .setActivityImplementations(new TestActivities.TestActivitiesImpl())
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
           .build();
 
   @Test
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/SyncTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/SyncTest.java
@@ -47,8 +47,6 @@ public class SyncTest {
       SDKTestWorkflowRule.newBuilder()
           .setWorkflowTypes(TestSyncWorkflowImpl.class)
           .setActivityImplementations(activitiesImpl)
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
           .build();
 
   @Test
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/TimerTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/TimerTest.java
@@ -36,11 +36,7 @@ public class TimerTest {
 
   @Rule
   public SDKTestWorkflowRule testWorkflowRule =
-      SDKTestWorkflowRule.newBuilder()
-          .setWorkflowTypes(TestTimerWorkflowImpl.class)
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
-          .build();
+      SDKTestWorkflowRule.newBuilder().setWorkflowTypes(TestTimerWorkflowImpl.class).build();
 
   @Test
   public void testTimer() {
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/UUIDAndRandomTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/UUIDAndRandomTest.java
@@ -38,8 +38,6 @@ public class UUIDAndRandomTest {
       SDKTestWorkflowRule.newBuilder()
           .setWorkflowTypes(TestUUIDAndRandom.class)
           .setActivityImplementations(new TestActivities.TestActivitiesImpl())
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
           .build();
 
   @Test
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/UpsertSearchAttributesTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/UpsertSearchAttributesTest.java
@@ -46,8 +46,6 @@ public class UpsertSearchAttributesTest {
       SDKTestWorkflowRule.newBuilder()
           .setWorkflowTypes(TestUpsertSearchAttributesImpl.class)
           .setActivityImplementations(new TestActivities.TestActivitiesImpl())
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
           .build();
 
   @Test
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/activityTests/ActivityRetryWithMaxAttemptsTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/activityTests/ActivityRetryWithMaxAttemptsTest.java
@@ -45,8 +45,6 @@ public class ActivityRetryWithMaxAttemptsTest {
   public SDKTestWorkflowRule testWorkflowRule =
       SDKTestWorkflowRule.newBuilder()
           .setWorkflowTypes(TestActivityRetryWithMaxAttempts.class)
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
           .setActivityImplementations(activitiesImpl)
           .build();
 
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/activityTests/LocalActivityAndQueryTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/activityTests/LocalActivityAndQueryTest.java
@@ -22,11 +22,11 @@
 import io.temporal.client.WorkflowClient;
 import io.temporal.client.WorkflowOptions;
 import io.temporal.client.WorkflowStub;
-import io.temporal.testing.TestWorkflowRule;
 import io.temporal.workflow.QueryMethod;
 import io.temporal.workflow.Workflow;
 import io.temporal.workflow.WorkflowInterface;
 import io.temporal.workflow.WorkflowMethod;
+import io.temporal.workflow.shared.SDKTestWorkflowRule;
 import io.temporal.workflow.shared.TestActivities;
 import io.temporal.workflow.shared.TestOptions;
 import java.time.Duration;
@@ -45,8 +45,8 @@ public class LocalActivityAndQueryTest {
       new TestActivities.TestActivitiesImpl();
 
   @Rule
-  public TestWorkflowRule testWorkflowRule =
-      TestWorkflowRule.newBuilder()
+  public SDKTestWorkflowRule testWorkflowRule =
+      SDKTestWorkflowRule.newBuilder()
           .setWorkflowTypes(TestLocalActivityAndQueryWorkflow.class)
           .setActivityImplementations(activitiesImpl)
           .build();
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/activityTests/LocalActivityTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/activityTests/LocalActivityTest.java
@@ -45,8 +45,6 @@ public class LocalActivityTest {
       SDKTestWorkflowRule.newBuilder()
           .setWorkflowTypes(TestLocalActivityWorkflowImpl.class)
           .setActivityImplementations(activitiesImpl)
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
           .build();
 
   @Test
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/activityTests/ParallelLocalActivitiesTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/activityTests/ParallelLocalActivitiesTest.java
@@ -46,8 +46,6 @@ public class ParallelLocalActivitiesTest {
       SDKTestWorkflowRule.newBuilder()
           .setWorkflowTypes(TestParallelLocalActivitiesWorkflowImpl.class)
           .setActivityImplementations(activitiesImpl)
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
           .build();
 
   @Test
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/childWorkflowTests/ChildWorkflowExecutionPromiseHandlerTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/childWorkflowTests/ChildWorkflowExecutionPromiseHandlerTest.java
@@ -24,11 +24,11 @@
 import io.temporal.api.common.v1.WorkflowExecution;
 import io.temporal.client.WorkflowClient;
 import io.temporal.client.WorkflowOptions;
-import io.temporal.testing.TestWorkflowRule;
 import io.temporal.workflow.Async;
 import io.temporal.workflow.CompletablePromise;
 import io.temporal.workflow.Promise;
 import io.temporal.workflow.Workflow;
+import io.temporal.workflow.shared.SDKTestWorkflowRule;
 import io.temporal.workflow.shared.TestWorkflows;
 import java.time.Duration;
 import org.junit.Rule;
@@ -37,8 +37,8 @@
 public class ChildWorkflowExecutionPromiseHandlerTest {
 
   @Rule
-  public TestWorkflowRule testWorkflowRule =
-      TestWorkflowRule.newBuilder()
+  public SDKTestWorkflowRule testWorkflowRule =
+      SDKTestWorkflowRule.newBuilder()
           .setWorkflowTypes(TestNamedChild.class, TestChildWorkflowExecutionPromiseHandler.class)
           .build();
 
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/sdkTestWorkflowRuleTests/SDKWorkflowRuleInterceptorTest1.java b/temporal-sdk/src/test/java/io/temporal/workflow/sdkTestWorkflowRuleTests/SDKWorkflowRuleInterceptorTest1.java
@@ -0,0 +1,36 @@
+/*
+ *  Copyright (C) 2020 Temporal Technologies, Inc. All Rights Reserved.
+ *
+ *  Copyright 2012-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *  Modifications copyright (C) 2017 Uber Technologies, Inc.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License"). You may not
+ *  use this file except in compliance with the License. A copy of the License is
+ *  located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ *  or in the "license" file accompanying this file. This file is distributed on
+ *  an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *  express or implied. See the License for the specific language governing
+ *  permissions and limitations under the License.
+ */
+
+package io.temporal.workflow.sdkTestWorkflowRuleTests;
+
+import io.temporal.testing.TracingWorkerInterceptor;
+import io.temporal.workflow.shared.SDKTestWorkflowRule;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+
+public class SDKWorkflowRuleInterceptorTest1 {
+
+  @Rule public SDKTestWorkflowRule testWorkflowRule = SDKTestWorkflowRule.newBuilder().build();
+
+  @Test
+  public void testWorkerInterceptorWorkerFactoryOptionsNotSet() {
+    Assert.assertNotNull(testWorkflowRule.getInterceptor(TracingWorkerInterceptor.class));
+  }
+}
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/sdkTestWorkflowRuleTests/SDKWorkflowRuleInterceptorTest2.java b/temporal-sdk/src/test/java/io/temporal/workflow/sdkTestWorkflowRuleTests/SDKWorkflowRuleInterceptorTest2.java
@@ -0,0 +1,41 @@
+/*
+ *  Copyright (C) 2020 Temporal Technologies, Inc. All Rights Reserved.
+ *
+ *  Copyright 2012-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *  Modifications copyright (C) 2017 Uber Technologies, Inc.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License"). You may not
+ *  use this file except in compliance with the License. A copy of the License is
+ *  located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ *  or in the "license" file accompanying this file. This file is distributed on
+ *  an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *  express or implied. See the License for the specific language governing
+ *  permissions and limitations under the License.
+ */
+
+package io.temporal.workflow.sdkTestWorkflowRuleTests;
+
+import io.temporal.testing.TracingWorkerInterceptor;
+import io.temporal.worker.WorkerFactoryOptions;
+import io.temporal.workflow.shared.SDKTestWorkflowRule;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+
+public class SDKWorkflowRuleInterceptorTest2 {
+
+  @Rule
+  public SDKTestWorkflowRule testWorkflowRule =
+      SDKTestWorkflowRule.newBuilder()
+          .setWorkerFactoryOptions(WorkerFactoryOptions.getDefaultInstance())
+          .build();
+
+  @Test
+  public void testWorkerInterceptorWorkerFactoryOptionsSet() {
+    Assert.assertNotNull(testWorkflowRule.getInterceptor(TracingWorkerInterceptor.class));
+  }
+}
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/sdkTestWorkflowRuleTests/SDKWorkflowRuleInterceptorTest3.java b/temporal-sdk/src/test/java/io/temporal/workflow/sdkTestWorkflowRuleTests/SDKWorkflowRuleInterceptorTest3.java
@@ -0,0 +1,48 @@
+/*
+ *  Copyright (C) 2020 Temporal Technologies, Inc. All Rights Reserved.
+ *
+ *  Copyright 2012-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *  Modifications copyright (C) 2017 Uber Technologies, Inc.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License"). You may not
+ *  use this file except in compliance with the License. A copy of the License is
+ *  located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ *  or in the "license" file accompanying this file. This file is distributed on
+ *  an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *  express or implied. See the License for the specific language governing
+ *  permissions and limitations under the License.
+ */
+
+package io.temporal.workflow.sdkTestWorkflowRuleTests;
+
+import io.temporal.testing.TracingWorkerInterceptor;
+import io.temporal.worker.WorkerFactoryOptions;
+import io.temporal.workflow.shared.SDKTestWorkflowRule;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+
+public class SDKWorkflowRuleInterceptorTest3 {
+
+  TracingWorkerInterceptor tracingWorkerInterceptor =
+      new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace());
+
+  @Rule
+  public SDKTestWorkflowRule testWorkflowRule =
+      SDKTestWorkflowRule.newBuilder()
+          .setWorkerFactoryOptions(
+              WorkerFactoryOptions.newBuilder()
+                  .setWorkerInterceptors(tracingWorkerInterceptor)
+                  .build())
+          .build();
+
+  @Test
+  public void testWorkerInterceptorWorkerFactoryOptionsSetWithInterceptor() {
+    Assert.assertSame(
+        tracingWorkerInterceptor, testWorkflowRule.getInterceptor(TracingWorkerInterceptor.class));
+  }
+}
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/shared/SDKTestWorkflowRule.java b/temporal-sdk/src/test/java/io/temporal/workflow/shared/SDKTestWorkflowRule.java
@@ -39,6 +39,7 @@
 import io.temporal.serviceclient.WorkflowServiceStubs;
 import io.temporal.testing.TestWorkflowEnvironment;
 import io.temporal.testing.TestWorkflowRule;
+import io.temporal.testing.TracingWorkerInterceptor;
 import io.temporal.worker.WorkerFactoryOptions;
 import io.temporal.worker.WorkerOptions;
 import io.temporal.worker.WorkflowImplementationOptions;
@@ -48,7 +49,11 @@
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.*;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 import org.junit.rules.TestRule;
 import org.junit.runner.Description;
 import org.junit.runners.model.Statement;
@@ -64,7 +69,7 @@ public class SDKTestWorkflowRule implements TestRule {
   // Enable to regenerate JsonFiles used for replay testing.
   public static final boolean REGENERATE_JSON_FILES = false;
   // Only enable when USE_DOCKER_SERVICE is true
-  public static final Boolean useExternalService =
+  public static final boolean useExternalService =
       Boolean.parseBoolean(System.getenv("USE_DOCKER_SERVICE"));
   public static final String temporalServiceAddress = System.getenv("TEMPORAL_SERVICE_ADDRESS");
   private static final List<ScheduledFuture<?>> delayedCallbacks = new ArrayList<>();
@@ -88,6 +93,8 @@ public static Builder newBuilder() {
   }
 
   public static class Builder {
+
+    private boolean workerFactoryOptionsAreSet = false;
     TestWorkflowRule.Builder testWorkflowRuleBuilder;
 
     public Builder() {
@@ -100,7 +107,15 @@ public Builder setWorkerOptions(WorkerOptions options) {
     }
 
     public Builder setWorkerFactoryOptions(WorkerFactoryOptions options) {
+      options =
+          (options.getWorkerInterceptors() == null)
+              ? WorkerFactoryOptions.newBuilder(options)
+                  .setWorkerInterceptors(
+                      new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
+                  .build()
+              : options;
       testWorkflowRuleBuilder.setWorkerFactoryOptions(options);
+      workerFactoryOptionsAreSet = true;
       return this;
     }
 
@@ -114,11 +129,6 @@ public Builder setNamespace(String namespace) {
       return this;
     }
 
-    public Builder setWorkerInterceptors(WorkerInterceptor... workerInterceptors) {
-      testWorkflowRuleBuilder.setWorkerInterceptors(workerInterceptors);
-      return this;
-    }
-
     public Builder setWorkflowTypes(Class<?>... workflowTypes) {
       testWorkflowRuleBuilder.setWorkflowTypes(workflowTypes);
       return this;
@@ -156,6 +166,13 @@ public Builder setDoNotStart(boolean doNotStart) {
     }
 
     public SDKTestWorkflowRule build() {
+      if (!workerFactoryOptionsAreSet) {
+        testWorkflowRuleBuilder.setWorkerFactoryOptions(
+            WorkerFactoryOptions.newBuilder()
+                .setWorkerInterceptors(
+                    new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
+                .build());
+      }
       return new SDKTestWorkflowRule(testWorkflowRuleBuilder);
     }
   }
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/signalTests/SignalAndQueryListenerTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/signalTests/SignalAndQueryListenerTest.java
@@ -43,8 +43,6 @@ public class SignalAndQueryListenerTest {
   public SDKTestWorkflowRule testWorkflowRule =
       SDKTestWorkflowRule.newBuilder()
           .setWorkflowTypes(TestSignalAndQueryListenerWorkflowImpl.class)
-          .setWorkerInterceptors(
-              new TracingWorkerInterceptor(new TracingWorkerInterceptor.FilteredTrace()))
           .build();
 
   @Test
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/signalTests/SignalExternalWorkflowTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/signalTests/SignalExternalWorkflowTest.java
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/versionTests/GetVersionRemovedBeforeTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/versionTests/GetVersionRemovedBeforeTest.java
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/versionTests/GetVersionRemovedInReplayTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/versionTests/GetVersionRemovedInReplayTest.java
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/versionTests/GetVersionTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/versionTests/GetVersionTest.java
diff --git a/temporal-sdk/src/test/java/io/temporal/workflow/versionTests/GetVersionWorkflowRemoveTest.java b/temporal-sdk/src/test/java/io/temporal/workflow/versionTests/GetVersionWorkflowRemoveTest.java
diff --git a/temporal-testing-junit4/src/main/java/io/temporal/testing/TestWorkflowRule.java b/temporal-testing-junit4/src/main/java/io/temporal/testing/TestWorkflowRule.java
diff --git a/temporal-testing-junit4/src/main/java/io/temporal/testing/TracingWorkerInterceptor.java b/temporal-testing-junit4/src/main/java/io/temporal/testing/TracingWorkerInterceptor.java
