From c6f0b58c78c034a79413bba9cf6a5120a2529f57 Mon Sep 17 00:00:00 2001 From: Quinn Klassen Date: Fri, 22 Nov 2024 08:19:18 -0800 Subject: [PATCH] Do runtime check to ensure update validator has the same parameters as the update it validates (#2323) Check update validator has a matching update method --- .../POJOWorkflowInterfaceMetadata.java | 63 +++++++++++++----- .../POJOWorkflowInterfaceMetadataTest.java | 65 +++++++++++++++++++ 2 files changed, 113 insertions(+), 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 index 4ac5ea40b..b003ceabd 100644 --- 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 * * * @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 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/temporal-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowInterfaceMetadataTest.java b/temporal-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowInterfaceMetadataTest.java index 467aa684c..6dce87403 100644 --- a/temporal-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowInterfaceMetadataTest.java +++ b/temporal-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 input); } + @WorkflowInterface + public interface L { + @WorkflowMethod + void l(); + } + + @WorkflowInterface + public interface LUpdate extends L { + @UpdateMethod + void update(Map input); + + @UpdateValidatorMethod(updateName = "update") + void validate(Map input); + } + + @WorkflowInterface + public interface LUpdateBadValidator extends L { + @UpdateMethod + void update(Map input); + + @UpdateValidatorMethod(updateName = "update") + void validate(Map input); + } + + @WorkflowInterface + public interface LUpdateValidatorWithNoUpdate extends L { + @UpdateMethod + void update(Map input); + + @UpdateValidatorMethod(updateName = "wrongUpdate") + void validate(Map input); + } + public interface DE extends D, E {} @WorkflowInterface