Do runtime check to ensure update validator has the same parameters a…

…s the update it validates (#2323) Check update validator has a matching update method
temporalio · Nov 22, 2024 · c6f0b58 · c6f0b58
1 parent 1d86a57
commit c6f0b58
Show file tree

Hide file tree

Showing 2 changed files with 113 additions and 15 deletions.
diff --git a/temporal-sdk/src/main/java/io/temporal/common/metadata/POJOWorkflowInterfaceMetadata.java b/temporal-sdk/src/main/java/io/temporal/common/metadata/POJOWorkflowInterfaceMetadata.java
@@ -20,10 +20,7 @@
 
 package io.temporal.common.metadata;
 
-import io.temporal.workflow.QueryMethod;
-import io.temporal.workflow.SignalMethod;
-import io.temporal.workflow.WorkflowInterface;
-import io.temporal.workflow.WorkflowMethod;
+import io.temporal.workflow.*;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.*;
@@ -97,7 +94,7 @@ public static POJOWorkflowInterfaceMetadata newInstanceSkipWorkflowAnnotationChe
    * <ul>
    *   <li>{@code anInterface} to be annotated with {@link WorkflowInterface}
    *   <li>All methods of {@code anInterface} to be annotated with {@link WorkflowMethod}, {@link
-   *       QueryMethod} or {@link SignalMethod}
+   *       QueryMethod}, {@link UpdateMethod}, {@link UpdateValidatorMethod} or {@link SignalMethod}
    * </ul>
    *
    * @param anInterface interface to create metadata for
@@ -137,15 +134,17 @@ public static POJOWorkflowInterfaceMetadata newInstance(
    * #newInstance(Class, boolean)} is that the interfaces passing here can be not annotated with
    * {@link WorkflowInterface} at all and even not having {@link WorkflowInterface} as a parent. It
    * also can have all kinds of additional methods that are not annotated with {@link
-   * WorkflowMethod}, {@link QueryMethod} or {@link SignalMethod}. Such unannotated methods or
-   * methods that are not part of some {@link WorkflowInterface} will be ignored.
+   * WorkflowMethod}, {@link QueryMethod}, {@link UpdateMethod}, {@link UpdateValidatorMethod} or
+   * {@link SignalMethod}. Such unannotated methods or methods that are not part of some {@link
+   * WorkflowInterface} will be ignored.
    *
    * @param anInterface interface to create metadata for
    * @param forceProcessWorkflowMethods if true, methods of the {@code anInterface} that are
-   *     annotated with {@link WorkflowMethod}, {@link QueryMethod} or {@link SignalMethod} are
-   *     processed like {@code current} is a workflow interface even if it is not annotated with
-   *     {@link WorkflowInterface} itself. For example, this is useful when we have a query-only
-   *     interface to register as a listener or call as a stub.
+   *     annotated with {@link WorkflowMethod}, {@link QueryMethod}, {@link UpdateMethod}, {@link
+   *     UpdateValidatorMethod} or {@link SignalMethod} are processed like {@code current} is a
+   *     workflow interface even if it is not annotated with {@link WorkflowInterface} itself. For
+   *     example, this is useful when we have a query-only interface to register as a listener or
+   *     call as a stub.
    * @return metadata for the interface
    */
   static POJOWorkflowInterfaceMetadata newImplementationInstance(
@@ -161,10 +160,11 @@ static POJOWorkflowInterfaceMetadata newImplementationInstance(
    * @param validateWorkflowAnnotation check if the interface has a {@link WorkflowInterface}
    *     annotation with methods
    * @param forceProcessWorkflowMethods if true, methods of the {@code anInterface} that are
-   *     annotated with {@link WorkflowMethod}, {@link QueryMethod} or {@link SignalMethod} are
-   *     processed like {@code current} is a workflow interface even if it is not annotated with
-   *     {@link WorkflowInterface} itself. For example, this is useful when we have a query-only
-   *     interface to register as a listener or call as a stub.
+   *     annotated with {@link WorkflowMethod}, {@link QueryMethod}, {@link UpdateMethod}, {@link
+   *     UpdateValidatorMethod} or {@link SignalMethod} are processed like {@code current} is a
+   *     workflow interface even if it is not annotated with {@link WorkflowInterface} itself. For
+   *     example, this is useful when we have a query-only interface to register as a listener or
+   *     call as a stub.
    */
   private static POJOWorkflowInterfaceMetadata newInstanceInternal(
       Class<?> anInterface,
@@ -189,6 +189,39 @@ private static POJOWorkflowInterfaceMetadata newInstanceInternal(
             "Interface doesn't contain any methods: " + anInterface.getName());
       }
     }
+    // Validate that all @UpdateValidatorMethod methods have corresponding @UpdateMethod
+    Map<String, POJOWorkflowMethodMetadata> updateMethods = new HashMap<>();
+    for (POJOWorkflowMethodMetadata methodMetadata : result.methods.values()) {
+      if (methodMetadata.getType() == WorkflowMethodType.UPDATE) {
+        updateMethods.put(methodMetadata.getName(), methodMetadata);
+      }
+    }
+
+    for (POJOWorkflowMethodMetadata methodMetadata : result.methods.values()) {
+      if (methodMetadata.getType() == WorkflowMethodType.UPDATE_VALIDATOR) {
+        UpdateValidatorMethod validator =
+            methodMetadata.getWorkflowMethod().getAnnotation(UpdateValidatorMethod.class);
+        POJOWorkflowMethodMetadata update = updateMethods.get(validator.updateName());
+        if (update == null) {
+          throw new IllegalArgumentException(
+              "Missing @UpdateMethod with name \""
+                  + validator.updateName()
+                  + "\" for @UpdateValidatorMethod \""
+                  + methodMetadata.getWorkflowMethod().getName()
+                  + "\"");
+        }
+        if (!Arrays.equals(
+            update.getWorkflowMethod().getGenericParameterTypes(),
+            methodMetadata.getWorkflowMethod().getGenericParameterTypes())) {
+          throw new IllegalArgumentException(
+              "Update method \""
+                  + update.getWorkflowMethod().getName()
+                  + "\" and Validator method \""
+                  + methodMetadata.getWorkflowMethod().getName()
+                  + "\" do not have the same parameters");
+        }
+      }
+    }
     return result;
   }
 

diff --git a/...oral-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowInterfaceMetadataTest.java b/...oral-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowInterfaceMetadataTest.java
@@ -211,6 +211,33 @@ private Class<?> generateWorkflowInterfaceWithQueryMethod(
     return methodDefinition.make().load(this.getClass().getClassLoader()).getLoaded();
   }
 
+  @Test
+  public void workflowInterfaceWithUpdateValidator() {
+    POJOWorkflowInterfaceMetadata metadata =
+        POJOWorkflowInterfaceMetadata.newInstance(LUpdate.class);
+  }
+
+  @Test
+  public void workflowInterfaceWithBadUpdateValidator() {
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> POJOWorkflowInterfaceMetadata.newInstance(LUpdateBadValidator.class));
+  }
+
+  @Test
+  public void workflowInterfaceValidatorWithNoUpdate() {
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> POJOWorkflowInterfaceMetadata.newInstance(LUpdateValidatorWithNoUpdate.class));
+  }
+
+  @Test
+  public void interfaceWithInvalidValidator() {
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> POJOWorkflowInterfaceMetadata.newImplementationInstance(J.class, true));
+  }
+
   public interface O {
     void someMethod();
   }
@@ -272,12 +299,50 @@ public interface I {
     void i();
   }
 
+  public interface J {
+    @UpdateValidatorMethod(updateName = "update")
+    void validate(String input);
+  }
+
   @WorkflowInterface
   public interface K {
     @WorkflowMethod
     void f(Map<String, EncodedValuesTest.Pair> input);
   }
 
+  @WorkflowInterface
+  public interface L {
+    @WorkflowMethod
+    void l();
+  }
+
+  @WorkflowInterface
+  public interface LUpdate extends L {
+    @UpdateMethod
+    void update(Map<String, Integer> input);
+
+    @UpdateValidatorMethod(updateName = "update")
+    void validate(Map<String, Integer> input);
+  }
+
+  @WorkflowInterface
+  public interface LUpdateBadValidator extends L {
+    @UpdateMethod
+    void update(Map<String, Integer> input);
+
+    @UpdateValidatorMethod(updateName = "update")
+    void validate(Map<String, String> input);
+  }
+
+  @WorkflowInterface
+  public interface LUpdateValidatorWithNoUpdate extends L {
+    @UpdateMethod
+    void update(Map<String, Integer> input);
+
+    @UpdateValidatorMethod(updateName = "wrongUpdate")
+    void validate(Map<String, Integer> input);
+  }
+
   public interface DE extends D, E {}
 
   @WorkflowInterface