Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do runtime check to ensure update validator has the same parameters as the update it validates #2323

Merged
merged 3 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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
Expand Down
Loading