Skip to content

Commit 821dab1

Browse files
authored
Fix Jackson3JsonPayloadConverter API mismatch in the multi-release jar (#3087)
1 parent 3fb41a5 commit 821dab1

5 files changed

Lines changed: 90 additions & 28 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ to docs, or any other relevant information.
7272
- `ActivityClient` now sends the `temporal-namespace` gRPC header like `WorkflowClient` and `ScheduleClient` do,
7373
so standalone activity calls are no longer rejected with `PERMISSION_DENIED` by namespace-scoped auth (e.g.
7474
Temporal Cloud API keys).
75+
- The Java 8 entry of `Jackson3JsonPayloadConverter` in the multi-release jar now declares the same public API as the
76+
Java 17 class, including the `JsonMapper` constructor and `newDefaultJsonMapper(boolean)`. Tools that compile against
77+
the base entry, such as IntelliJ's build system, no longer reject `new Jackson3JsonPayloadConverter(mapper)`.
7578
- Kotlin `Async` method references compiled with Kotlin 2.4 now retain their inline behavior,
7679
preventing `NonDeterministicException` when replaying workflows started with Kotlin 2.2. For
7780
workflows already started with Kotlin 2.4 and SDK 1.40 or earlier, set the

‎temporal-sdk/build.gradle‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ dependencies {
2424
exclude group: 'org.slf4j', module: 'slf4j-api'
2525
}
2626

27+
// The Java 8 stub of Jackson3JsonPayloadConverter must declare the same public API as the
28+
// Java 17 class (JEP 238), so the Jackson 3 types are needed at compile time only. The java17
29+
// source set sees them through the main compile classpath.
30+
compileOnly "tools.jackson.core:jackson-databind:$jackson3Version"
31+
2732
testImplementation project(':temporal-testing')
2833
// The optional envconfig-backed test harness is loaded only while SDK tests are running.
2934
testRuntimeOnly project(':temporal-envconfig')
@@ -62,7 +67,6 @@ dependencies {
6267
// project's JVM compatibility (e.g. 21+ on CI edge runners).
6368
java17Implementation files(sourceSets.main.output.classesDirs) { builtBy compileJava }
6469
java17Implementation files({ sourceSets.main.compileClasspath })
65-
java17CompileOnly "tools.jackson.core:jackson-databind:$jackson3Version"
6670

6771
java21Implementation files(sourceSets.main.output.classesDirs) { builtBy compileJava }
6872
}
@@ -202,9 +206,8 @@ task testCloud(type: Test) {
202206
}
203207

204208
// On Java 17+, prepend java17 classes to all test classpaths so that Class.forName finds
205-
// the real Jackson3JsonPayloadConverter instead of the Java 8 stub. This lets us test
206-
// the present-java17-but-absent-jackson3 behavior (NoClassDefFoundError) in the same
207-
// test that tests the Java 8 stub behavior (UnsupportedOperationException).
209+
// the real Jackson3JsonPayloadConverter instead of the Java 8 stub, matching what the
210+
// multi-release jar resolves to at runtime.
208211
tasks.withType(Test).configureEach {
209212
dependsOn compileJava17Java
210213
doFirst {
@@ -257,9 +260,8 @@ testing {
257260
jackson3Tests(JvmTestSuite) {
258261
dependencies {
259262
// java17 output must come before project() (added by configureEach) so that
260-
// the compiler and runtime see the real Jackson3JsonPayloadConverter — which
261-
// has a wider API than the Java 8 stub (newDefaultJsonMapper, JsonMapper
262-
// constructor) because the stub can't reference Jackson 3 types.
263+
// the compiler and runtime see the real Jackson3JsonPayloadConverter rather
264+
// than the Java 8 stub, which only throws.
263265
implementation files(sourceSets.java17.output.classesDirs) { builtBy compileJava17Java }
264266
implementation "tools.jackson.core:jackson-databind:$jackson3Version"
265267
}
@@ -271,6 +273,7 @@ testing {
271273
languageVersion = JavaLanguageVersion.of(project.property("testJavaVersion") as int)
272274
}
273275
}
276+
systemProperty 'temporal.sdk.mainClassesDirs', sourceSets.main.output.classesDirs.asPath
274277
shouldRunAfter(test)
275278
}
276279
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package io.temporal.common.converter;
2+
3+
import static org.junit.Assert.assertEquals;
4+
5+
import java.io.File;
6+
import java.io.IOException;
7+
import java.lang.reflect.Constructor;
8+
import java.lang.reflect.Method;
9+
import java.lang.reflect.Modifier;
10+
import java.nio.file.Files;
11+
import java.util.Set;
12+
import java.util.TreeSet;
13+
import org.junit.Test;
14+
15+
/**
16+
* Verifies that the Java 8 stub of {@link Jackson3JsonPayloadConverter} declares the same public
17+
* API as the Java 17 class, as required for versioned entries of a multi-release jar. Tools that
18+
* compile against the base entry otherwise cannot resolve members that exist only in the Java 17
19+
* class.
20+
*/
21+
public class Jackson3JsonPayloadConverterApiTest {
22+
23+
private static final String CLASS_FILE =
24+
Jackson3JsonPayloadConverter.class.getName().replace('.', '/') + ".class";
25+
26+
@Test
27+
public void testStubDeclaresTheSamePublicApi() throws Exception {
28+
byte[] stub = readStub();
29+
Class<?> stubClass =
30+
new ClassLoader(getClass().getClassLoader()) {
31+
Class<?> define() {
32+
return defineClass(null, stub, 0, stub.length);
33+
}
34+
}.define();
35+
36+
assertEquals(publicApi(Jackson3JsonPayloadConverter.class), publicApi(stubClass));
37+
}
38+
39+
private static byte[] readStub() throws IOException {
40+
String dirs = System.getProperty("temporal.sdk.mainClassesDirs");
41+
for (String dir : dirs.split(File.pathSeparator, -1)) {
42+
File classFile = new File(dir, CLASS_FILE);
43+
if (classFile.isFile()) {
44+
return Files.readAllBytes(classFile.toPath());
45+
}
46+
}
47+
throw new AssertionError("stub not found in " + dirs);
48+
}
49+
50+
private static Set<String> publicApi(Class<?> clazz) {
51+
Set<String> api = new TreeSet<>();
52+
for (Constructor<?> constructor : clazz.getDeclaredConstructors()) {
53+
if (Modifier.isPublic(constructor.getModifiers())) {
54+
api.add(constructor.toGenericString());
55+
}
56+
}
57+
for (Method method : clazz.getDeclaredMethods()) {
58+
if (Modifier.isPublic(method.getModifiers()) && !method.isSynthetic()) {
59+
api.add(method.toGenericString());
60+
}
61+
}
62+
return api;
63+
}
64+
}

‎temporal-sdk/src/main/java/io/temporal/common/converter/Jackson3JsonPayloadConverter.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import io.temporal.common.Experimental;
55
import java.lang.reflect.Type;
66
import java.util.Optional;
7+
import tools.jackson.databind.json.JsonMapper;
78

89
/**
910
* A {@link PayloadConverter} that uses Jackson 3.x for JSON serialization/deserialization. This
@@ -32,6 +33,14 @@ public Jackson3JsonPayloadConverter(boolean jackson2Compat) {
3233
throw new UnsupportedOperationException(UNSUPPORTED_MSG);
3334
}
3435

36+
public Jackson3JsonPayloadConverter(JsonMapper mapper) {
37+
throw new UnsupportedOperationException(UNSUPPORTED_MSG);
38+
}
39+
40+
public static JsonMapper newDefaultJsonMapper(boolean jackson2Compat) {
41+
throw new UnsupportedOperationException(UNSUPPORTED_MSG);
42+
}
43+
3544
@Override
3645
public String getEncodingType() {
3746
throw new UnsupportedOperationException(UNSUPPORTED_MSG);

‎temporal-sdk/src/test/java/io/temporal/common/converter/JacksonJsonPayloadConverterTest.java‎

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import static org.junit.Assert.fail;
66

77
import io.temporal.api.common.v1.Payloads;
8-
import java.lang.reflect.InvocationTargetException;
98
import java.time.Instant;
109
import java.util.Objects;
1110
import java.util.Optional;
@@ -25,27 +24,11 @@ public void testSetDefaultAsJackson3ThrowsWithoutJackson3() {
2524
JacksonJsonPayloadConverter.setDefaultAsJackson3(true, true);
2625
fail("Expected IllegalStateException");
2726
} catch (IllegalStateException e) {
28-
// On Java 8: Class.forName finds the stub, whose constructor throws
29-
// UnsupportedOperationException → wrapped in InvocationTargetException by reflection.
30-
// On Java 17+: Class.forName finds the real impl (java17 classes are on the classpath)
31-
// but Jackson 3 types are absent, so class loading throws NoClassDefFoundError directly.
27+
// Both the Java 8 stub and the Java 17 class reference Jackson 3 types, so reflective
28+
// constructor lookup fails while linking when Jackson 3 is absent.
3229
Throwable cause = e.getCause();
33-
String specVersion = System.getProperty("java.specification.version");
34-
int majorVersion =
35-
specVersion.startsWith("1.")
36-
? Integer.parseInt(specVersion.substring(2))
37-
: Integer.parseInt(specVersion);
38-
if (majorVersion >= 17) {
39-
assertTrue(
40-
"Expected NoClassDefFoundError, got: " + cause, cause instanceof NoClassDefFoundError);
41-
} else {
42-
assertTrue(
43-
"Expected InvocationTargetException, got: " + cause,
44-
cause instanceof InvocationTargetException);
45-
assertTrue(
46-
"Expected UnsupportedOperationException, got: " + cause.getCause(),
47-
cause.getCause() instanceof UnsupportedOperationException);
48-
}
30+
assertTrue(
31+
"Expected NoClassDefFoundError, got: " + cause, cause instanceof NoClassDefFoundError);
4932
}
5033
}
5134

0 commit comments

Comments
 (0)