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

Add streamlined sealed class polymorphic de/serialization #3549

Open
wants to merge 15 commits into
base: 2.14
Choose a base branch
from
10 changes: 10 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@
<configuration>
<sources>
<source>src/test-jdk14/java</source>
<source>src/test-jdk17/java</source>
</sources>
</configuration>
</execution>
Expand All @@ -312,9 +313,18 @@
<release>17</release>
<compilerArgs>
<arg>-parameters</arg>
<arg>--add-opens=java.base/java.lang=ALL-UNNAMED</arg>
<arg>--add-opens=java.base/java.util=ALL-UNNAMED</arg>
</compilerArgs>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<argLine>--add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED</argLine>
</configuration>
</plugin>
</plugins>
</build>
</profile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ public VisibilityChecker<?> findAutoDetectVisibility(AnnotatedClass ac,
* This includes not only
* instantiating resolver builder, but also configuring it based on
* relevant annotations (not including ones checked with a call to
* {@link #findSubtypes}
* {@link #findSubtypesByAnnotations}
*
* @param config Configuration settings in effect (for serialization or deserialization)
* @param ac Annotated class to check for annotations
Expand All @@ -494,7 +494,7 @@ public TypeResolverBuilder<?> findTypeResolver(MapperConfig<?> config,
* This includes not only
* instantiating resolver builder, but also configuring it based on
* relevant annotations (not including ones checked with a call to
* {@link #findSubtypes}
* {@link #findSubtypesByAnnotations}
*
* @param config Configuration settings in effect (for serialization or deserialization)
* @param am Annotated member (field or method) to check for annotations
Expand All @@ -516,7 +516,7 @@ public TypeResolverBuilder<?> findPropertyTypeResolver(MapperConfig<?> config,
* This includes not only
* instantiating resolver builder, but also configuring it based on
* relevant annotations (not including ones checked with a call to
* {@link #findSubtypes}
* {@link #findSubtypesByAnnotations}
*
* @param config Configuration settings in effect (for serialization or deserialization)
* @param am Annotated member (field or method) to check for annotations
Expand All @@ -529,6 +529,12 @@ public TypeResolverBuilder<?> findPropertyContentTypeResolver(MapperConfig<?> co
AnnotatedMember am, JavaType containerType) {
return null;
}

/**
* @deprecated Use {@link #findSubtypesByAnnotations(Annotated)} instead.
*/
@Deprecated(since="2.14")
public List<NamedType> findSubtypes(Annotated a) { return findSubtypesByAnnotations(a); }
sigpwned marked this conversation as resolved.
Show resolved Hide resolved

/**
* Method for locating annotation-specified subtypes related to annotated
Expand All @@ -541,7 +547,20 @@ public TypeResolverBuilder<?> findPropertyContentTypeResolver(MapperConfig<?> co
*
* @return List of subtype definitions found if any; {@code null} if none
*/
public List<NamedType> findSubtypes(Annotated a) { return null; }
public List<NamedType> findSubtypesByAnnotations(Annotated a) { return null; }

/**
* Method for locating the permitted subclasses specified by a sealed class.
* Note that this is only guaranteed to be a list of direct subtypes, no
* recursive processing is guaranteed (i.e., caller has to do it if/as
* necessary). Note that invoking this method may implicitly load all Jackson
* Java 17 integrations and features.
*
* @param klass A Java content type {@link Class}
*
* @return List of subtype definitions found if any; {@code null} if none
*/
public List<NamedType> findSubtypesByPermittedSubclasses(Class<?> klass) { return null; }

/**
* Method for checking if specified type has explicit name.
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/com/fasterxml/jackson/databind/MapperFeature.java
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,15 @@ public enum MapperFeature implements ConfigFeature
*
* @since 2.13
*/
APPLY_DEFAULT_VALUES(true)
APPLY_DEFAULT_VALUES(true),

/**
* Feature that determines whether subtypes are discovered from sealed class permitted
* subclasses automatically.
*
* @since 2.14
*/
DISCOVER_SEALED_CLASS_PERMITTED_SUBCLASSES(true)
;

private final boolean _defaultState;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,27 @@ public TypeResolverBuilder<?> findPropertyContentTypeResolver(MapperConfig<?> co
@Override
public List<NamedType> findSubtypes(Annotated a)
{
List<NamedType> types1 = _primary.findSubtypes(a);
List<NamedType> types2 = _secondary.findSubtypes(a);
return findSubtypesByAnnotations(a);
}

@Override
public List<NamedType> findSubtypesByAnnotations(Annotated a)
{
List<NamedType> types1 = _primary.findSubtypesByAnnotations(a);
List<NamedType> types2 = _secondary.findSubtypesByAnnotations(a);
if (types1 == null || types1.isEmpty()) return types2;
if (types2 == null || types2.isEmpty()) return types1;
ArrayList<NamedType> result = new ArrayList<NamedType>(types1.size() + types2.size());
result.addAll(types1);
result.addAll(types2);
return result;
}

@Override
public List<NamedType> findSubtypesByPermittedSubclasses(Class<?> klass)
{
List<NamedType> types1 = _primary.findSubtypesByPermittedSubclasses(klass);
List<NamedType> types2 = _secondary.findSubtypesByPermittedSubclasses(klass);
if (types1 == null || types1.isEmpty()) return types2;
if (types2 == null || types2.isEmpty()) return types1;
ArrayList<NamedType> result = new ArrayList<NamedType>(types1.size() + types2.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import com.fasterxml.jackson.databind.cfg.HandlerInstantiator;
import com.fasterxml.jackson.databind.cfg.MapperConfig;
import com.fasterxml.jackson.databind.ext.Java7Support;
import com.fasterxml.jackson.databind.jdk17.JDK17Util;
import com.fasterxml.jackson.databind.jsontype.NamedType;
import com.fasterxml.jackson.databind.jsontype.TypeIdResolver;
import com.fasterxml.jackson.databind.jsontype.TypeResolverBuilder;
Expand Down Expand Up @@ -614,10 +615,9 @@ public TypeResolverBuilder<?> findPropertyContentTypeResolver(MapperConfig<?> co
}
return _findTypeResolver(config, am, containerType);
}

@Override
public List<NamedType> findSubtypes(Annotated a)
{
public List<NamedType> findSubtypesByAnnotations(Annotated a) {
JsonSubTypes t = _findAnnotation(a, JsonSubTypes.class);
if (t == null) return null;
JsonSubTypes.Type[] types = t.value();
Expand All @@ -632,6 +632,18 @@ public List<NamedType> findSubtypes(Annotated a)
return result;
}

@Override
public List<NamedType> findSubtypesByPermittedSubclasses(Class<?> klass) {
boolean sealed = Optional.ofNullable(JDK17Util.isSealed(klass)).orElse(false);
if (sealed) {
Class<?>[] permittedSubclasses = JDK17Util.getPermittedSubclasses(klass);
if (permittedSubclasses != null && permittedSubclasses.length > 0) {
return Arrays.stream(permittedSubclasses).map(NamedType::new).toList();
}
}
return null;
}

@Override
public String findTypeName(AnnotatedClass ac)
{
Expand Down
85 changes: 85 additions & 0 deletions src/main/java/com/fasterxml/jackson/databind/jdk17/JDK17Util.java
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) {
sigpwned marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Up @@ -2,9 +2,9 @@

import java.lang.reflect.Modifier;
import java.util.*;

import com.fasterxml.jackson.databind.AnnotationIntrospector;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.cfg.MapperConfig;
import com.fasterxml.jackson.databind.introspect.*;
import com.fasterxml.jackson.databind.jsontype.NamedType;
Expand Down Expand Up @@ -109,7 +109,9 @@ public Collection<NamedType> collectAndResolveSubtypesByClass(MapperConfig<?> co

// then annotated types for property itself
if (property != null) {
Collection<NamedType> st = ai.findSubtypes(property);
Collection<NamedType> st = ai.findSubtypesByAnnotations(property);
if(st == null && config.isEnabled(MapperFeature.DISCOVER_SEALED_CLASS_PERMITTED_SUBCLASSES))
st = ai.findSubtypesByPermittedSubclasses(rawBase);
if (st != null) {
for (NamedType nt : st) {
AnnotatedClass ac = AnnotatedClassResolver.resolveWithoutSuperTypes(config,
Expand Down Expand Up @@ -178,7 +180,9 @@ public Collection<NamedType> collectAndResolveSubtypesByTypeId(MapperConfig<?> c

// then with definitions from property
if (property != null) {
Collection<NamedType> st = ai.findSubtypes(property);
Collection<NamedType> st = ai.findSubtypesByAnnotations(property);
if(st == null && config.isEnabled(MapperFeature.DISCOVER_SEALED_CLASS_PERMITTED_SUBCLASSES))
st = ai.findSubtypesByPermittedSubclasses(rawBase);
if (st != null) {
for (NamedType nt : st) {
ac = AnnotatedClassResolver.resolveWithoutSuperTypes(config, nt.getType());
Expand Down Expand Up @@ -262,7 +266,9 @@ protected void _collectAndResolve(AnnotatedClass annotatedType, NamedType namedT
}
// if it wasn't, add and check subtypes recursively
collectedSubtypes.put(typeOnlyNamedType, namedType);
Collection<NamedType> st = ai.findSubtypes(annotatedType);
Collection<NamedType> st = ai.findSubtypesByAnnotations(annotatedType);
if(st == null && config.isEnabled(MapperFeature.DISCOVER_SEALED_CLASS_PERMITTED_SUBCLASSES))
st = ai.findSubtypesByPermittedSubclasses(annotatedType.getRawType());
if (st != null && !st.isEmpty()) {
for (NamedType subtype : st) {
AnnotatedClass subtypeClass = AnnotatedClassResolver.resolveWithoutSuperTypes(config,
Expand Down Expand Up @@ -293,7 +299,9 @@ protected void _collectAndResolveByTypeId(AnnotatedClass annotatedType, NamedTyp

// only check subtypes if this type hadn't yet been handled
if (typesHandled.add(namedType.getType())) {
Collection<NamedType> st = ai.findSubtypes(annotatedType);
Collection<NamedType> st = ai.findSubtypesByAnnotations(annotatedType);
if(st == null && config.isEnabled(MapperFeature.DISCOVER_SEALED_CLASS_PERMITTED_SUBCLASSES))
st = ai.findSubtypesByPermittedSubclasses(annotatedType.getRawType());
if (st != null && !st.isEmpty()) {
for (NamedType subtype : st) {
AnnotatedClass subtypeClass = AnnotatedClassResolver.resolveWithoutSuperTypes(config,
Expand Down
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
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Author

@sigpwned sigpwned Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and that was the first thing I tried. But the @Ignore only seems to have any effect if it's applied to the whole class. I admit that I don't understand why. I just tried it again and got the same result. Please check my math here because I agree that would be the better solution. I would be happy to be wrong!

Copy link
Member

@pjfanning pjfanning Jul 27, 2022

Choose a reason for hiding this comment

The 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 @Test).

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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
package com.fasterxml.jackson.databind.records;

import com.fasterxml.jackson.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;
Expand Down Expand Up @@ -142,14 +137,17 @@ public void testDeserializeJsonRename() throws Exception {
/**********************************************************************
*/

// TODO Ignore this test case
// [databind#2992]
public void testNamingStrategy() throws Exception
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you use @Ignore like the other tests you disabled?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move this single test (not full class) to failing package?

Copy link
Member

Choose a reason for hiding this comment

The 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);
// }

/*
/**********************************************************************
Expand Down