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 1ad2ee6ff..ed4775ab6 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 @@ -175,14 +175,27 @@ private POJOWorkflowImplMetadata( this.queryMethods = ImmutableList.copyOf(queryMethods.values()); this.updateMethods = ImmutableList.copyOf(updateMethods.values()); this.updateValidatorMethods = ImmutableList.copyOf(updateValidatorMethods.values()); - if (!listener && validateConstructor) { + if (!listener) { this.workflowInit = - ReflectionUtils.getConstructor( + ReflectionUtils.getWorkflowInitConstructor( implClass, this.workflowMethods.stream() .map(POJOWorkflowMethodMetadata::getWorkflowMethod) .collect(Collectors.toList())) .orElse(null); + if (validateConstructor) { + Constructor defaultConstructor = + ReflectionUtils.getPublicDefaultConstructor(implClass).orElse(null); + if (defaultConstructor == null && this.workflowInit == null) { + throw new IllegalArgumentException( + "No default constructor or constructor annotated with @WorkflowInit found: " + + implClass.getName()); + } else if (defaultConstructor != null && this.workflowInit != null) { + throw new IllegalArgumentException( + "Found both a default constructor and constructor annotated with @WorkflowInit: " + + implClass.getName()); + } + } } else { this.workflowInit = null; } diff --git a/temporal-sdk/src/main/java/io/temporal/internal/common/env/ReflectionUtils.java b/temporal-sdk/src/main/java/io/temporal/internal/common/env/ReflectionUtils.java index b292467f6..4265ce419 100644 --- a/temporal-sdk/src/main/java/io/temporal/internal/common/env/ReflectionUtils.java +++ b/temporal-sdk/src/main/java/io/temporal/internal/common/env/ReflectionUtils.java @@ -32,27 +32,16 @@ public final class ReflectionUtils { private ReflectionUtils() {} - public static Optional> getConstructor( + public static Optional> getWorkflowInitConstructor( Class clazz, List workflowMethod) { // We iterate through all constructors to find the one annotated with @WorkflowInit // and check if it has the same parameters as the workflow method. // We check all declared constructors to find any constructors that are annotated with - // @WorkflowInit, but not public, - // to give a more informative error message. + // @WorkflowInit, but are not public, to give a more informative error message. Optional> workflowInit = Optional.empty(); - Constructor defaultConstructors = null; for (Constructor ctor : clazz.getDeclaredConstructors()) { WorkflowInit wfInit = ctor.getAnnotation(WorkflowInit.class); if (wfInit == null) { - if (ctor.getParameterCount() == 0 && Modifier.isPublic(ctor.getModifiers())) { - if (workflowInit.isPresent() || defaultConstructors != null) { - throw new IllegalArgumentException( - "Multiple constructors annotated with @WorkflowInit or a default constructor found: " - + clazz.getName()); - } - defaultConstructors = ctor; - continue; - } continue; } if (workflowMethod.size() != 1) { @@ -60,9 +49,9 @@ public static Optional> getConstructor( "Multiple interfaces implemented while using @WorkflowInit annotation. Only one is allowed: " + clazz.getName()); } - if (workflowInit.isPresent() || defaultConstructors != null) { + if (workflowInit.isPresent()) { throw new IllegalArgumentException( - "Multiple constructors annotated with @WorkflowInit or a default constructor found: " + "Multiple constructors annotated with @WorkflowInit found. Only one is allowed: " + clazz.getName()); } if (!Modifier.isPublic(ctor.getModifiers())) { @@ -76,14 +65,25 @@ public static Optional> getConstructor( } workflowInit = Optional.of(ctor); } - if (!workflowInit.isPresent() && defaultConstructors == null) { - throw new IllegalArgumentException( - "No default constructor or constructor annotated with @WorkflowInit found: " - + clazz.getName()); - } return workflowInit; } + public static Optional> getPublicDefaultConstructor(Class clazz) { + Constructor defaultConstructors = null; + for (Constructor ctor : clazz.getDeclaredConstructors()) { + if (ctor.getParameterCount() != 0) { + continue; + } + if (!Modifier.isPublic(ctor.getModifiers())) { + throw new IllegalArgumentException( + "Default constructor must be public: " + clazz.getName()); + } + defaultConstructors = ctor; + break; + } + return Optional.ofNullable(defaultConstructors); + } + public static String getMethodNameForStackTraceCutoff( Class clazz, String methodName, Class... parameterTypes) throws RuntimeException { try { 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 47a9b7b13..40120d23a 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 @@ -191,7 +191,7 @@ private void registerWorkflowImplementationType( Method executeMethod = workflowImplementationClass.getMethod("execute", EncodedValues.class); Optional> ctor = - ReflectionUtils.getConstructor( + ReflectionUtils.getWorkflowInitConstructor( workflowImplementationClass, Collections.singletonList(executeMethod)); dynamicWorkflowImplementationFactory = (encodedValues) -> { diff --git a/temporal-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowImplMetadataTest.java b/temporal-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowImplMetadataTest.java index 5efd4bc61..23cddd030 100644 --- a/temporal-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowImplMetadataTest.java +++ b/temporal-sdk/src/test/java/io/temporal/common/metadata/POJOWorkflowImplMetadataTest.java @@ -196,7 +196,7 @@ public void testWorkflowWithMultipleInit() { assertTrue( e.getMessage() .contains( - "Multiple constructors annotated with @WorkflowInit or a default constructor found:")); + "Found both a default constructor and constructor annotated with @WorkflowInit")); } }