diff --git a/temporal-kotlin/src/main/kotlin/io/temporal/common/metadata/WorkflowMetadata.kt b/temporal-kotlin/src/main/kotlin/io/temporal/common/metadata/WorkflowMetadata.kt index 45b8a9242..b57aa00b1 100644 --- a/temporal-kotlin/src/main/kotlin/io/temporal/common/metadata/WorkflowMetadata.kt +++ b/temporal-kotlin/src/main/kotlin/io/temporal/common/metadata/WorkflowMetadata.kt @@ -31,7 +31,7 @@ import kotlin.reflect.jvm.javaMethod * ``` */ fun workflowName(workflowClass: Class<*>): String { - val workflowInterfaceMetadata = POJOWorkflowInterfaceMetadata.newInstance(workflowClass) + val workflowInterfaceMetadata = POJOWorkflowInterfaceMetadata.newStubInstance(workflowClass) return workflowInterfaceMetadata.workflowType.orElse(null) ?: throw IllegalArgumentException("$workflowClass does not define a workflow method") } @@ -72,7 +72,7 @@ fun workflowQueryType(method: KFunction<*>): String { private fun workflowMethodName(method: KFunction<*>, type: WorkflowMethodType): String { val javaMethod = method.javaMethod ?: throw IllegalArgumentException("Invalid method reference $method") - val interfaceMetadata = POJOWorkflowInterfaceMetadata.newInstance(javaMethod.declaringClass) + val interfaceMetadata = POJOWorkflowInterfaceMetadata.newStubInstance(javaMethod.declaringClass) val methodMetadata = interfaceMetadata.methodsMetadata.find { it.workflowMethod == javaMethod } ?: throw IllegalArgumentException("Not a workflow method reference $method") if (methodMetadata.type != type) { diff --git a/temporal-sdk/src/main/java/io/temporal/common/metadata/POJOActivityInterfaceMetadata.java b/temporal-sdk/src/main/java/io/temporal/common/metadata/POJOActivityInterfaceMetadata.java index ef0ce0e78..b1959a36d 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/metadata/POJOActivityInterfaceMetadata.java +++ b/temporal-sdk/src/main/java/io/temporal/common/metadata/POJOActivityInterfaceMetadata.java @@ -146,11 +146,14 @@ private static Set getActivityInterfaceMethods( } Method[] declaredMethods = current.getDeclaredMethods(); - result.addAll(Arrays.asList(declaredMethods)); + for (Method declaredMethod : declaredMethods) { + if (validateAndQualifiedForActivityMethod(declaredMethod)) { + result.add(declaredMethod); + } + } if (isCurrentAnActivityInterface) { result.stream() - .filter(POJOActivityInterfaceMetadata::validateAndQualifiedForActivityMethod) .map(method -> new POJOActivityMethodMetadata(method, current, annotation)) .forEach( methodMetadata -> @@ -217,11 +220,13 @@ private static void validateModifierAccess(Class activityInterface) { /** * @return true if the method should be used as an activity method, false if it shouldn't * @throws IllegalArgumentException if the method is incorrectly configured (for example, a - * combination of ActivityMethod and a static modifier) + * combination of {@link ActivityMethod} and a {@code static} modifier) */ private static boolean validateAndQualifiedForActivityMethod(Method method) { + boolean isAnnotatedActivityMethod = method.getAnnotation(ActivityMethod.class) != null; + if (Modifier.isStatic(method.getModifiers())) { - if (method.getAnnotation(ActivityMethod.class) != null) { + if (isAnnotatedActivityMethod) { throw new IllegalArgumentException( "Method with @ActivityMethod annotation can't be static: " + method); } else { @@ -229,16 +234,18 @@ private static boolean validateAndQualifiedForActivityMethod(Method method) { } } - if (method.getAnnotation(ActivityMethod.class) != null) { + if (isAnnotatedActivityMethod) { // all methods explicitly marked with ActivityMethod qualify return true; } + if (method.isSynthetic()) { // if method is synthetic and not explicitly marked as an ActivityMethod, // it's not qualified as an activity method. // https://github.com/temporalio/sdk-java/issues/977 return false; } + return true; } } diff --git a/temporal-sdk/src/main/java/io/temporal/common/metadata/POJOWorkflowImplMetadata.java b/temporal-sdk/src/main/java/io/temporal/common/metadata/POJOWorkflowImplMetadata.java index a17e7c875..08b2899a5 100644 --- a/temporal-sdk/src/main/java/io/temporal/common/metadata/POJOWorkflowImplMetadata.java +++ b/temporal-sdk/src/main/java/io/temporal/common/metadata/POJOWorkflowImplMetadata.java @@ -107,9 +107,9 @@ private POJOWorkflowImplMetadata(Class implClass, boolean listener) { POJOWorkflowInterfaceMetadata interfaceMetadata; if (listener) { interfaceMetadata = - POJOWorkflowInterfaceMetadata.newInstanceSkipWorkflowAnnotationCheck(anInterface); + POJOWorkflowInterfaceMetadata.newStubInstanceSkipWorkflowAnnotationCheck(anInterface); } else { - interfaceMetadata = POJOWorkflowInterfaceMetadata.newImplementationInterface(anInterface); + interfaceMetadata = POJOWorkflowInterfaceMetadata.newImplementationInstance(anInterface); } workflowInterfaces.add(interfaceMetadata); List methods = interfaceMetadata.getMethodsMetadata(); 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 1d3a868ba..7ca8d4168 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 @@ -21,6 +21,7 @@ package io.temporal.common.metadata; import io.temporal.workflow.WorkflowInterface; +import io.temporal.workflow.WorkflowMethod; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.*; @@ -46,7 +47,7 @@ public final class POJOWorkflowInterfaceMetadata { /** Used to override equals and hashCode of Method to ensure deduping by method name in a set. */ private static class EqualsByName { private final Method method; - private String nameFromAnnotation; + private final String nameFromAnnotation; EqualsByName(Method method, String nameFromAnnotation) { this.method = method; @@ -80,8 +81,8 @@ public int hashCode() { * Returns POJOWorkflowInterfaceMetadata for an interface annotated with {@link * WorkflowInterface}. */ - public static POJOWorkflowInterfaceMetadata newInstance(Class anInterface) { - return newInstance(anInterface, true); + public static POJOWorkflowInterfaceMetadata newStubInstance(Class anInterface) { + return newInstance(anInterface, true, true); } /** @@ -89,13 +90,35 @@ public static POJOWorkflowInterfaceMetadata newInstance(Class anInterface) { * WorkflowInterface}. This to support invoking workflow signal and query methods through a base * interface without such annotation. */ - public static POJOWorkflowInterfaceMetadata newInstanceSkipWorkflowAnnotationCheck( + public static POJOWorkflowInterfaceMetadata newStubInstanceSkipWorkflowAnnotationCheck( Class anInterface) { - return newInstance(anInterface, false); + return newInstance(anInterface, false, true); } + /** + * Returns POJOWorkflowInterfaceMetadata for an interface annotated with {@link WorkflowInterface} + * for workflow implementation. The core difference from stub methods that the interfaces passing + * here can be not {@link WorkflowInterface} at all. + */ + static POJOWorkflowInterfaceMetadata newImplementationInstance(Class anInterface) { + return newInstance(anInterface, false, false); + } + + /** + * Ensures that the interface that the stub is created with is a {@link WorkflowInterface} with + * the right structure. + * + * @param anInterface interface class + * @param checkWorkflowInterfaceAnnotation check if the interface has a {@link WorkflowInterface} + * annotation with methods + * @param forceProcessAsWorkflowInterface if true, the {@code current} is processed as a workflow + * interface even if it is not annotated with {@link WorkflowInterface}. true for stub + * interface if it's used to create a stub. + */ private static POJOWorkflowInterfaceMetadata newInstance( - Class anInterface, boolean checkWorkflowInterfaceAnnotation) { + Class anInterface, + boolean checkWorkflowInterfaceAnnotation, + boolean forceProcessAsWorkflowInterface) { if (!anInterface.isInterface()) { throw new IllegalArgumentException("Not an interface: " + anInterface); } @@ -105,9 +128,10 @@ private static POJOWorkflowInterfaceMetadata newInstance( throw new IllegalArgumentException( "Missing required @WorkflowInterface annotation: " + anInterface); } - validatePublicModifier(anInterface); + validateModifierAccess(anInterface); } - POJOWorkflowInterfaceMetadata result = new POJOWorkflowInterfaceMetadata(anInterface, false); + POJOWorkflowInterfaceMetadata result = + new POJOWorkflowInterfaceMetadata(anInterface, forceProcessAsWorkflowInterface); if (result.methods.isEmpty()) { if (checkWorkflowInterfaceAnnotation) { throw new IllegalArgumentException( @@ -117,24 +141,16 @@ private static POJOWorkflowInterfaceMetadata newInstance( return result; } - private static void validatePublicModifier(Class anInterface) { - if (!Modifier.isPublic(anInterface.getModifiers())) { - throw new IllegalArgumentException( - "Interface with @WorkflowInterface annotation must be public: " + anInterface); - } - } - - static POJOWorkflowInterfaceMetadata newImplementationInterface(Class anInterface) { - return new POJOWorkflowInterfaceMetadata(anInterface, true); - } - /** - * @param implementation if the metadata is for a workflow implementation class vs stub. + * @param forceProcessAsWorkflowInterface if true, the {@code current} is processed as a workflow + * interface even if it is not annotated with {@link WorkflowInterface}. true for stub + * interface if it's used to create a stub. */ - private POJOWorkflowInterfaceMetadata(Class anInterface, boolean implementation) { + private POJOWorkflowInterfaceMetadata( + Class anInterface, boolean forceProcessAsWorkflowInterface) { this.interfaceClass = anInterface; Map dedupeMap = new HashMap<>(); - getWorkflowInterfaceMethods(anInterface, !implementation, dedupeMap); + getWorkflowInterfaceMethods(anInterface, forceProcessAsWorkflowInterface, dedupeMap); } public Optional getWorkflowMethod() { @@ -175,14 +191,21 @@ public List getMethodsMetadata() { } /** + * @param forceProcessAsWorkflowInterface if true, the {@code current} is processed as a workflow + * interface even if it is not annotated with {@link WorkflowInterface}. true for stub + * interface if it's used to create a stub. * @return methods which are not part of an interface annotated with WorkflowInterface */ private Set getWorkflowInterfaceMethods( - Class current, boolean rootClass, Map dedupeMap) { + Class current, + boolean forceProcessAsWorkflowInterface, + Map dedupeMap) { WorkflowInterface annotation = current.getAnnotation(WorkflowInterface.class); - if (annotation != null) { - validatePublicModifier(current); + final boolean isCurrentAWorkflowInterface = annotation != null; + + if (isCurrentAWorkflowInterface) { + validateModifierAccess(current); } // Set to de-dupe the same method due to diamond inheritance @@ -191,68 +214,140 @@ private Set getWorkflowInterfaceMethods( for (Class anInterface : interfaces) { Set parentMethods = getWorkflowInterfaceMethods(anInterface, false, dedupeMap); - for (POJOWorkflowMethod parentMethod : parentMethods) { - if (parentMethod.getType() == WorkflowMethodType.NONE) { - Method method = parentMethod.getMethod(); - try { - current.getMethod(method.getName(), method.getParameterTypes()); - // Don't add to result as it is redefined by current. - // This allows overriding methods without annotation with annotated methods. - continue; - } catch (NoSuchMethodException e) { - if (annotation != null) { + addParentMethods(parentMethods, current, result); + } + + Method[] declaredMethods = current.getDeclaredMethods(); + for (Method declaredMethod : declaredMethods) { + POJOWorkflowMethod methodMetadata = new POJOWorkflowMethod(declaredMethod); + if (validateAndQualifiedForWorkflowMethod(methodMetadata)) { + result.add(methodMetadata); + } + } + + if (isCurrentAWorkflowInterface || forceProcessAsWorkflowInterface) { + for (POJOWorkflowMethod workflowMethod : result) { + if (workflowMethod.getType() == WorkflowMethodType.NONE && isCurrentAWorkflowInterface) { + // Workflow methods are different from activity methods. + // Method in a hierarchy of activity interfaces that is not annotated is an activity + // interface. + // Method in a hierarchy of workflow interfaces that is not annotated is a mistake. + throw new IllegalArgumentException( + "Missing @WorkflowMethod, @SignalMethod or @QueryMethod annotation on " + + workflowMethod.getMethod()); + } + + dedupeAndAdd(workflowMethod, dedupeMap); + + if (workflowMethod.getType() != WorkflowMethodType.NONE) { + POJOWorkflowMethodMetadata methodMetadata = + new POJOWorkflowMethodMetadata(workflowMethod, current); + methods.put(workflowMethod.getMethod(), methodMetadata); + + if (workflowMethod.getType() == WorkflowMethodType.WORKFLOW) { + if (this.workflowMethod != null) { throw new IllegalArgumentException( - "Missing @WorkflowMethod, @SignalMethod or @QueryMethod annotation on " + method); + "Duplicated @WorkflowMethod: " + + workflowMethod.getMethod() + + " and " + + this.workflowMethod.getWorkflowMethod()); } + this.workflowMethod = methodMetadata; } } - result.add(parentMethod); } + // the current interface is a WorkflowInterface, or forced to processed like one, + // so we process the collected methods and there is nothing to pass down + return Collections.emptySet(); + } else { + // Not annotated and forceProcessAsWorkflowInterface is false. + // Don't verify the integrity of the interface and don't flush the result into internal state. + // Just pass an accumulated result down + return result; } - Method[] declaredMethods = current.getDeclaredMethods(); - for (Method declaredMethod : declaredMethods) { - POJOWorkflowMethod methodMetadata = new POJOWorkflowMethod(declaredMethod); - result.add(methodMetadata); - } - if (annotation == null && !rootClass) { - return result; // Not annotated just pass all the methods to the parent + } + + private static void dedupeAndAdd( + POJOWorkflowMethod workflowMethod, Map dedupeMap) { + Method method = workflowMethod.getMethod(); + EqualsByName wrapped = + new EqualsByName(method, workflowMethod.getNameFromAnnotation().orElse(null)); + + Method registeredBefore = dedupeMap.put(wrapped, method); + if (registeredBefore != null && !registeredBefore.equals(method)) { + // TODO POJOActivityInterfaceMetadata is having an access to POJOActivityMethodMetadata + // on this stage and it able to provide more data about the conflict, this implementation + // should be updated in the same manner + throw new IllegalArgumentException( + "Duplicated methods (overloads are not allowed in workflow interfaces): \"" + + registeredBefore + + "\" and \"" + + method + + "\""); } - for (POJOWorkflowMethod workflowMethod : result) { - Method method = workflowMethod.getMethod(); - if (workflowMethod.getType() == WorkflowMethodType.NONE && annotation != null) { - throw new IllegalArgumentException( - "Missing @WorkflowMethod, @SignalMethod or @QueryMethod annotation on " + method); - } - EqualsByName wrapped = - new EqualsByName(method, workflowMethod.getNameFromAnnotation().orElse(null)); - Method registered = dedupeMap.put(wrapped, method); - if (registered != null && !registered.equals(method)) { - throw new IllegalArgumentException( - "Duplicated methods (overloads are not allowed): \"" - + registered - + "\" and \"" - + method - + "\""); - } + } - if (workflowMethod.getType() == WorkflowMethodType.NONE) { - continue; + private static void addParentMethods( + Set parentMethods, + Class currentInterface, + Set toSet) { + for (POJOWorkflowMethod parentMethod : parentMethods) { + if (parentMethod.getType() == WorkflowMethodType.NONE) { + Method method = parentMethod.getMethod(); + try { + currentInterface.getMethod(method.getName(), method.getParameterTypes()); + // Don't add to result as it is redefined by current. + // This allows overriding methods without annotation with annotated methods. + } catch (NoSuchMethodException e) { + // current interface doesn't have an override for this method - add it from the parent + toSet.add(parentMethod); + } + } else { + // parent interface method is explicitly annotated with one of workflow method annotations - + // adding to the + // result + toSet.add(parentMethod); } + } + } - POJOWorkflowMethodMetadata methodMetadata = - new POJOWorkflowMethodMetadata(workflowMethod, current); - if (workflowMethod.getType() == WorkflowMethodType.WORKFLOW) { - if (this.workflowMethod != null) { - throw new IllegalArgumentException( - "Duplicated @WorkflowMethod: " - + workflowMethod.getMethod() - + " and " - + this.workflowMethod.getWorkflowMethod()); - } - this.workflowMethod = methodMetadata; + private static void validateModifierAccess(Class workflowInterface) { + if (!Modifier.isPublic(workflowInterface.getModifiers())) { + throw new IllegalArgumentException( + "Interface with @WorkflowInterface annotation must be public: " + workflowInterface); + } + } + + /** + * @return true if the method may be used as a workflow method, false if it can't + * @throws IllegalArgumentException if the method is incorrectly configured (for example, a + * combination of {@link WorkflowMethod} and a {@code static} modifier) + */ + private static boolean validateAndQualifiedForWorkflowMethod(POJOWorkflowMethod workflowMethod) { + Method method = workflowMethod.getMethod(); + boolean isAnnotatedWorkflowMethod = !workflowMethod.getType().equals(WorkflowMethodType.NONE); + + if (Modifier.isStatic(method.getModifiers())) { + if (isAnnotatedWorkflowMethod) { + throw new IllegalArgumentException("Workflow Method can't be static: " + method); + } else { + return false; } - methods.put(method, methodMetadata); } - return Collections.emptySet(); + + if (isAnnotatedWorkflowMethod) { + // all methods explicitly marked with one of workflow method qualifiers + return true; + } + + if (method.isSynthetic()) { + // if method is synthetic and not explicitly marked as a workflow method, + // it's not qualified as a workflow method. + // https://github.com/temporalio/sdk-java/issues/977 + // https://github.com/temporalio/sdk-java/issues/1331 + return false; + } + + return true; } } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/sync/ChildWorkflowInvocationHandler.java b/temporal-sdk/src/main/java/io/temporal/internal/sync/ChildWorkflowInvocationHandler.java index a897670ab..dab571a69 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/sync/ChildWorkflowInvocationHandler.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/sync/ChildWorkflowInvocationHandler.java @@ -44,7 +44,7 @@ class ChildWorkflowInvocationHandler implements InvocationHandler { Class workflowInterface, ChildWorkflowOptions options, WorkflowOutboundCallsInterceptor outboundCallsInterceptor) { - workflowMetadata = POJOWorkflowInterfaceMetadata.newInstance(workflowInterface); + workflowMetadata = POJOWorkflowInterfaceMetadata.newStubInstance(workflowInterface); Optional workflowMethodMetadata = workflowMetadata.getWorkflowMethod(); if (!workflowMethodMetadata.isPresent()) { diff --git a/temporal-sdk/src/main/java/io/temporal/internal/sync/ContinueAsNewWorkflowInvocationHandler.java b/temporal-sdk/src/main/java/io/temporal/internal/sync/ContinueAsNewWorkflowInvocationHandler.java index 84dd00201..c95109532 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/sync/ContinueAsNewWorkflowInvocationHandler.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/sync/ContinueAsNewWorkflowInvocationHandler.java @@ -38,7 +38,7 @@ class ContinueAsNewWorkflowInvocationHandler implements InvocationHandler { Class interfaceClass, @Nullable ContinueAsNewOptions options, WorkflowOutboundCallsInterceptor outboundCallsInterceptor) { - workflowMetadata = POJOWorkflowInterfaceMetadata.newInstance(interfaceClass); + workflowMetadata = POJOWorkflowInterfaceMetadata.newStubInstance(interfaceClass); if (!workflowMetadata.getWorkflowMethod().isPresent()) { throw new IllegalArgumentException( "Missing method annotated with @WorkflowMethod: " + interfaceClass.getName()); diff --git a/temporal-sdk/src/main/java/io/temporal/internal/sync/ExternalWorkflowInvocationHandler.java b/temporal-sdk/src/main/java/io/temporal/internal/sync/ExternalWorkflowInvocationHandler.java index b01cc752d..02c12d061 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/sync/ExternalWorkflowInvocationHandler.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/sync/ExternalWorkflowInvocationHandler.java @@ -38,7 +38,7 @@ public ExternalWorkflowInvocationHandler( Class workflowInterface, WorkflowExecution execution, WorkflowOutboundCallsInterceptor workflowOutboundCallsInterceptor) { - workflowMetadata = POJOWorkflowInterfaceMetadata.newInstance(workflowInterface); + workflowMetadata = POJOWorkflowInterfaceMetadata.newStubInstance(workflowInterface); stub = new ExternalWorkflowStubImpl(execution, workflowOutboundCallsInterceptor); } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/sync/POJOWorkflowImplementationFactory.java b/temporal-sdk/src/main/java/io/temporal/internal/sync/POJOWorkflowImplementationFactory.java index ab586b9d0..5fa1c623c 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/sync/POJOWorkflowImplementationFactory.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/sync/POJOWorkflowImplementationFactory.java @@ -125,7 +125,7 @@ public void addWorkflowImplementationFactory( } workflowImplementationFactories.put(clazz, factory); POJOWorkflowInterfaceMetadata workflowMetadata = - POJOWorkflowInterfaceMetadata.newInstance(clazz); + POJOWorkflowInterfaceMetadata.newStubInstance(clazz); if (!workflowMetadata.getWorkflowMethod().isPresent()) { throw new IllegalArgumentException( "Workflow interface doesn't contain a method annotated with @WorkflowMethod: " + clazz); diff --git a/temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInternal.java b/temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInternal.java index be2b166b4..d4f02a6e6 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInternal.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInternal.java @@ -603,7 +603,7 @@ public static DataConverter getDataConverter() { */ public static String getWorkflowType(Class workflowInterfaceClass) { POJOWorkflowInterfaceMetadata metadata = - POJOWorkflowInterfaceMetadata.newInstance(workflowInterfaceClass); + POJOWorkflowInterfaceMetadata.newStubInstance(workflowInterfaceClass); return metadata.getWorkflowType().get(); } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInvocationHandler.java b/temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInvocationHandler.java index 2cddb1d09..ac9e52e72 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInvocationHandler.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/sync/WorkflowInvocationHandler.java @@ -116,7 +116,7 @@ static void closeAsyncInvocation() { WorkflowClientCallsInterceptor workflowClientCallsInvoker, WorkflowExecution execution) { workflowMetadata = - POJOWorkflowInterfaceMetadata.newInstanceSkipWorkflowAnnotationCheck(workflowInterface); + POJOWorkflowInterfaceMetadata.newStubInstanceSkipWorkflowAnnotationCheck(workflowInterface); Optional workflowType = workflowMetadata.getWorkflowType(); WorkflowStub stub = new WorkflowStubImpl(clientOptions, workflowClientCallsInvoker, workflowType, execution); @@ -133,7 +133,7 @@ static void closeAsyncInvocation() { WorkflowClientCallsInterceptor workflowClientCallsInvoker, WorkflowOptions options) { Objects.requireNonNull(options, "options"); - workflowMetadata = POJOWorkflowInterfaceMetadata.newInstance(workflowInterface); + workflowMetadata = POJOWorkflowInterfaceMetadata.newStubInstance(workflowInterface); Optional workflowMethodMetadata = workflowMetadata.getWorkflowMethod(); if (!workflowMethodMetadata.isPresent()) { diff --git a/temporal-sdk/src/test/java/io/temporal/internal/sync/POJOWorkflowMetadataTest.java b/temporal-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowMetadataTest.java similarity index 66% rename from temporal-sdk/src/test/java/io/temporal/internal/sync/POJOWorkflowMetadataTest.java rename to temporal-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowMetadataTest.java index 2c41ec437..208c82ada 100644 --- a/temporal-sdk/src/test/java/io/temporal/internal/sync/POJOWorkflowMetadataTest.java +++ b/temporal-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowMetadataTest.java @@ -18,26 +18,36 @@ * limitations under the License. */ -package io.temporal.internal.sync; +package io.temporal.common.metadata; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; -import io.temporal.common.metadata.POJOWorkflowImplMetadata; -import io.temporal.common.metadata.POJOWorkflowInterfaceMetadata; -import io.temporal.common.metadata.POJOWorkflowMethodMetadata; -import io.temporal.common.metadata.WorkflowMethodType; +import io.temporal.common.metadata.testclasses.WorkflowInterfaceWithOneWorkflowMethod; import io.temporal.worker.Worker; import io.temporal.workflow.QueryMethod; import io.temporal.workflow.SignalMethod; import io.temporal.workflow.WorkflowInterface; import io.temporal.workflow.WorkflowMethod; import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; import java.util.Set; +import junitparams.JUnitParamsRunner; +import junitparams.Parameters; +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.description.annotation.AnnotationDescription; +import net.bytebuddy.description.modifier.ModifierContributor; +import net.bytebuddy.description.modifier.Ownership; +import net.bytebuddy.description.modifier.SyntheticState; +import net.bytebuddy.description.modifier.Visibility; +import net.bytebuddy.dynamic.DynamicType; +import net.bytebuddy.implementation.FixedValue; import org.junit.Test; +import org.junit.function.ThrowingRunnable; +import org.junit.runner.RunWith; +@RunWith(JUnitParamsRunner.class) public class POJOWorkflowMetadataTest { public interface A { @@ -247,12 +257,12 @@ public void testNonPublicInterface() { @Test(expected = IllegalArgumentException.class) public void testNonInterface() { - POJOWorkflowInterfaceMetadata.newInstance(DImpl.class); + POJOWorkflowInterfaceMetadata.newStubInstance(DImpl.class); } @Test(expected = IllegalArgumentException.class) public void testEmptyInterface() { - POJOWorkflowInterfaceMetadata.newInstance(Empty.class); + POJOWorkflowInterfaceMetadata.newStubInstance(Empty.class); } @Test @@ -267,7 +277,8 @@ public void testWorkflowInterface() throws NoSuchMethodException { expected.add("E_a"); expected.add("E_b"); - POJOWorkflowInterfaceMetadata dMetadata = POJOWorkflowInterfaceMetadata.newInstance(D.class); + POJOWorkflowInterfaceMetadata dMetadata = + POJOWorkflowInterfaceMetadata.newStubInstance(D.class); Method c = C.class.getDeclaredMethod("c"); POJOWorkflowMethodMetadata cMethod = dMetadata.getMethodMetadata(c); assertEquals(c, cMethod.getWorkflowMethod()); @@ -278,4 +289,66 @@ public void testWorkflowInterface() throws NoSuchMethodException { public void testGetWorkflowType() { assertEquals("AM_C_bb", Worker.getWorkflowType(F.class)); } + + @Test + @Parameters({ + "false, true, false, false, false", + "true, false, false, false, false", + "false, true, true, false, true", + "true, false, true, true, false" + }) + public void testSyntheticAndStaticMethods( + boolean synthetic, + boolean statik, + boolean annotated, + boolean shouldBeConsideredAWorkflowMethod, + boolean shouldThrow) + throws Throwable { + Class interfaice = generateWorkflowInterfaceWithQueryMethod(synthetic, statik, annotated); + + ThrowingRunnable r = + () -> { + POJOWorkflowInterfaceMetadata metadata = + POJOWorkflowInterfaceMetadata.newStubInstance(interfaice); + assertEquals( + shouldBeConsideredAWorkflowMethod, + metadata.getMethodsMetadata().stream() + .anyMatch(m -> m.getWorkflowMethod().getName().equals("method"))); + }; + if (shouldThrow) { + assertThrows(IllegalArgumentException.class, r); + } else { + r.run(); + } + } + + private Class generateWorkflowInterfaceWithQueryMethod( + boolean synthetic, boolean statik, boolean annotated) { + DynamicType.Builder builder = + new ByteBuddy() + .makeInterface(WorkflowInterfaceWithOneWorkflowMethod.class) + .name("GeneratedWorkflowInterface") + .annotateType(AnnotationDescription.Builder.ofType(WorkflowInterface.class).build()); + Collection modifiers = new ArrayList<>(); + modifiers.add(Visibility.PUBLIC); + if (synthetic) { + modifiers.add(SyntheticState.SYNTHETIC); + } + if (statik) { + modifiers.add(Ownership.STATIC); + } + + DynamicType.Builder.MethodDefinition.ParameterDefinition.Initial methodInitial = + builder.defineMethod("method", String.class, modifiers); + DynamicType.Builder.MethodDefinition methodDefinition = + statik ? methodInitial.intercept(FixedValue.value("hi")) : methodInitial.withoutCode(); + + if (annotated) { + methodDefinition = + methodDefinition.annotateMethod( + AnnotationDescription.Builder.ofType(QueryMethod.class).build()); + } + + return methodDefinition.make().load(this.getClass().getClassLoader()).getLoaded(); + } } diff --git a/temporal-sdk/src/test/java/io/temporal/common/metadata/testclasses/WorkflowInterfaceWithOneWorkflowMethod.java b/temporal-sdk/src/test/java/io/temporal/common/metadata/testclasses/WorkflowInterfaceWithOneWorkflowMethod.java new file mode 100644 index 000000000..5972bfcf8 --- /dev/null +++ b/temporal-sdk/src/test/java/io/temporal/common/metadata/testclasses/WorkflowInterfaceWithOneWorkflowMethod.java @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2022 Temporal Technologies, Inc. All Rights Reserved. + * + * Copyright (C) 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 material except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License 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.common.metadata.testclasses; + +import io.temporal.workflow.WorkflowInterface; +import io.temporal.workflow.WorkflowMethod; + +@WorkflowInterface +public interface WorkflowInterfaceWithOneWorkflowMethod { + @WorkflowMethod + boolean workflowMethod(); +} diff --git a/temporal-testing/src/main/java/io/temporal/testing/TestWorkflowExtension.java b/temporal-testing/src/main/java/io/temporal/testing/TestWorkflowExtension.java index 85f3b9734..b35ab44af 100644 --- a/temporal-testing/src/main/java/io/temporal/testing/TestWorkflowExtension.java +++ b/temporal-testing/src/main/java/io/temporal/testing/TestWorkflowExtension.java @@ -169,7 +169,7 @@ public boolean supportsParameter( try { // If POJOWorkflowInterfaceMetadata can be instantiated then parameterType is a proper // workflow interface and can be injected - POJOWorkflowInterfaceMetadata.newInstance(parameterType); + POJOWorkflowInterfaceMetadata.newStubInstance(parameterType); return true; } catch (Exception e) { return false;