-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add streamlined sealed class polymorphic de/serialization #3549
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
Changes from 9 commits
2d6fbcc
d57f5a9
cfb8372
63a6250
f0af3bb
48c6118
4417418
02c36a1
07c9df0
bb3ce82
821a467
e8f808a
cd9acc7
f8139c4
f82de3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
package com.fasterxml.jackson.databind.jdk17; | ||
|
||
import java.lang.reflect.Method; | ||
import com.fasterxml.jackson.databind.util.ClassUtil; | ||
import com.fasterxml.jackson.databind.util.NativeImageUtil; | ||
|
||
/** | ||
* Helper class to support some of JDK 17 (and later) features without Jackson itself being run on | ||
* (or even built with) Java 17. In particular allows better support of sealed class types (see | ||
* <a href="https://openjdk.java.net/jeps/409">JEP 409</a>). | ||
* | ||
* @since 2.14 | ||
*/ | ||
public class JDK17Util { | ||
public static Boolean isSealed(Class<?> type) { | ||
return SealedClassAccessor.instance().isSealed(type); | ||
} | ||
|
||
public static Class<?>[] getPermittedSubclasses(Class<?> sealedType) { | ||
return SealedClassAccessor.instance().getPermittedSubclasses(sealedType); | ||
} | ||
|
||
static class SealedClassAccessor { | ||
private final Method SEALED_IS_SEALED; | ||
private final Method SEALED_GET_PERMITTED_SUBCLASSES; | ||
|
||
private final static SealedClassAccessor INSTANCE; | ||
private final static RuntimeException PROBLEM; | ||
|
||
static { | ||
RuntimeException prob = null; | ||
SealedClassAccessor inst = null; | ||
try { | ||
inst = new SealedClassAccessor(); | ||
} catch (RuntimeException e) { | ||
prob = e; | ||
} | ||
INSTANCE = inst; | ||
PROBLEM = prob; | ||
} | ||
|
||
private SealedClassAccessor() throws RuntimeException { | ||
try { | ||
SEALED_IS_SEALED = Class.class.getMethod("isSealed"); | ||
SEALED_GET_PERMITTED_SUBCLASSES = Class.class.getMethod("getPermittedSubclasses"); | ||
} catch (Exception e) { | ||
throw new RuntimeException( | ||
String.format("Failed to access Methods needed to support sealed classes: (%s) %s", | ||
e.getClass().getName(), e.getMessage()), | ||
e); | ||
} | ||
} | ||
|
||
public static SealedClassAccessor instance() { | ||
if (PROBLEM != null) { | ||
throw PROBLEM; | ||
} | ||
return INSTANCE; | ||
} | ||
|
||
public Boolean isSealed(Class<?> type) throws IllegalArgumentException { | ||
try { | ||
return (Boolean) SEALED_IS_SEALED.invoke(type); | ||
} catch (Exception e) { | ||
if (NativeImageUtil.isUnsupportedFeatureError(e)) { | ||
return null; | ||
} | ||
throw new IllegalArgumentException( | ||
"Failed to access sealedness of type " + ClassUtil.nameOf(type)); | ||
} | ||
} | ||
|
||
public Class<?>[] getPermittedSubclasses(Class<?> sealedType) throws IllegalArgumentException { | ||
try { | ||
return (Class<?>[]) SEALED_GET_PERMITTED_SUBCLASSES.invoke(sealedType); | ||
} catch (Exception e) { | ||
if (NativeImageUtil.isUnsupportedFeatureError(e)) { | ||
return null; | ||
} | ||
throw new IllegalArgumentException( | ||
"Failed to access permitted subclasses of type " + ClassUtil.nameOf(sealedType)); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
/** | ||
Contains helper class(es) needed to support some of JDK17+ | ||
features without requiring running or building using JDK 17. | ||
*/ | ||
|
||
package com.fasterxml.jackson.databind.jdk17; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,16 @@ | ||
package com.fasterxml.jackson.databind.failing; | ||
|
||
import org.junit.Ignore; | ||
import com.fasterxml.jackson.annotation.JsonCreator; | ||
|
||
import com.fasterxml.jackson.databind.BaseMapTest; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.databind.ObjectReader; | ||
import com.fasterxml.jackson.databind.PropertyNamingStrategies; | ||
import com.fasterxml.jackson.databind.annotation.JsonNaming; | ||
|
||
// [databind#3102]: fails on JDK 16 which finally blocks mutation | ||
// of Record fields. | ||
//[databind#3102]: fails on JDK 16 which finally blocks mutation | ||
//of Record fields. | ||
@Ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. only the first of these tests fails - can you just ignore the one that fails? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, and that was the first thing I tried. But the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, jackson still uses junit3 as opposed to junit4 (or even junit5) - so that might explain why the Ignore annotation doesn't work at the test level - it works well in junit4 style (where you annotate methods with Could you move the failing tests to the com.fasterxml.jackson.databind.failing package of test-jdk14? Just the failing tests, not the full test classes. So you can copy the full class to com.fasterxml.jackson.databind.failing but then remove the tests that work and go back to the original class and remove the broken test(s). The tests in the 'failing' package should be ignored when you run the build with mvn from command line.
sigpwned marked this conversation as resolved.
Show resolved
Hide resolved
|
||
public class RecordWithJsonNaming3102Test extends BaseMapTest | ||
{ | ||
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,14 +2,15 @@ | |
|
||
import java.util.List; | ||
import java.util.Map; | ||
|
||
import org.junit.Ignore; | ||
import com.fasterxml.jackson.annotation.JsonSetter; | ||
import com.fasterxml.jackson.annotation.Nulls; | ||
import com.fasterxml.jackson.databind.BaseMapTest; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.databind.ObjectReader; | ||
import com.fasterxml.jackson.databind.exc.InvalidNullException; | ||
|
||
@Ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again - can you ignore just the individual tests that fail? |
||
public class RecordWithJsonSetter2974Test extends BaseMapTest | ||
{ | ||
record RecordWithNonNullDefs(@JsonSetter(nulls=Nulls.AS_EMPTY) List<String> names, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,10 @@ | ||
package com.fasterxml.jackson.databind.records; | ||
|
||
import com.fasterxml.jackson.annotation.*; | ||
|
||
import com.fasterxml.jackson.databind.annotation.*; | ||
import com.fasterxml.jackson.databind.*; | ||
import com.fasterxml.jackson.databind.annotation.JsonNaming; | ||
import com.fasterxml.jackson.databind.json.JsonMapper; | ||
import com.fasterxml.jackson.databind.util.ClassUtil; | ||
|
||
import java.util.Collections; | ||
import java.util.LinkedHashMap; | ||
import java.util.Map; | ||
|
@@ -142,14 +140,17 @@ public void testDeserializeJsonRename() throws Exception { | |
/********************************************************************** | ||
*/ | ||
|
||
// TODO Ignore this test case | ||
// [databind#2992] | ||
public void testNamingStrategy() throws Exception | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you move this single test (not full class) to failing package? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and uncommented when it is moved |
||
{ | ||
SnakeRecord input = new SnakeRecord("123", "value"); | ||
String json = MAPPER.writeValueAsString(input); | ||
SnakeRecord output = MAPPER.readValue(json, SnakeRecord.class); | ||
assertEquals(input, output); | ||
} | ||
// [databind#3102]: fails on JDK 16 which finally blocks mutation | ||
// of Record fields. | ||
// public void testNamingStrategy() throws Exception | ||
// { | ||
// SnakeRecord input = new SnakeRecord("123", "value"); | ||
// String json = MAPPER.writeValueAsString(input); | ||
// SnakeRecord output = MAPPER.readValue(json, SnakeRecord.class); | ||
// assertEquals(input, output); | ||
// } | ||
|
||
/* | ||
/********************************************************************** | ||
|
Uh oh!
There was an error while loading. Please reload this page.