diff --git a/README.md b/README.md index ac5464f..49d5896 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ A Java port of the `SealedPolymorphismSupport` added to jackson-module-scala in using the same `@type` property and the same name-derivation rules, so a value written by one is readable by the other. -> **Status: early.** Covered by 70 tests, including ports of the Scala module's +> **Status: early.** Covered by 76 tests, including ports of the Scala module's > `SealedPolymorphismSpec` and `NestedPolymorphismSpec`, so the examples below are verified output. > Not published anywhere yet, and the API may still change. @@ -117,10 +117,11 @@ This is the main thing the Java version does better than the Scala one. scalac l `sealed` on the JVM, so jackson-module-scala has to rebuild candidate class names from where the base is declared and filter them by subtype relationship. -## Enum members +## Enums are left to Jackson -An enum in a sealed hierarchy is the closest Java has to a set of Scala `case object`s. Because the -enum class holds several values, each *constant* is named individually: +This module does not touch enums, even ones permitted by a hierarchy it handles. Jackson writes an +enum as a string, and it keeps doing so — in every position, including as a `Map` key, and including +an enum with a custom `@JsonValue` representation or with constant bodies. ```java public sealed interface Signal extends SealedPolymorphismSupport permits Data, Status {} @@ -128,12 +129,21 @@ public sealed interface Signal extends SealedPolymorphismSupport permits Data, S public record Data(int value) implements Signal {} public enum Status implements Signal { IDLE, BUSY } -// {"@type":"Data","value":1} -// {"@type":"Status$IDLE"} +// {"signal":{"@type":"Data","value":1}} the record is tagged +// {"signal":"IDLE"} the enum is not ``` -Only value serializers are replaced — an enum used as a `Map` key keeps Jackson's ordinary key -handling, since a tagged object cannot be a JSON property name. +An enum therefore has no `@type` name, which has one consequence worth knowing: **a value of an +enum member cannot be read back through the hierarchy's base type.** A string is not something the +base type can dispatch on, so reading `{"signal":"IDLE"}` as a `Signal` fails, and says why. Writing +works, and reading works wherever the property is declared as the enum type itself. + +If you need a hierarchy member that round-trips through the base type and carries no state, use a +record with no components — `record Unknown() implements Animal {}` writes as `{"@type":"Unknown"}` +and reads straight back. + +Putting the marker on an enum is not an error, it just has no effect: the enum is written as a +string either way. ## Concrete sealed roots @@ -165,9 +175,10 @@ that could not be read back: | `sealed class X implements SealedPolymorphismSupport` | supported — a value and a base | | `interface X extends SealedPolymorphismSupport` | error: not sealed | | `non-sealed class X implements Base` | error: reopens the hierarchy | -| `enum X implements SealedPolymorphismSupport` | error: mark the sealed interface it implements | +| `enum X implements SealedPolymorphismSupport` | ignored — enums are always Jackson's to write | -Records and enum constants are closed by construction and need no modifier of their own. +Records are closed by construction and need no modifier of their own. An enum permitted by the root +is skipped rather than checked, since this module does not handle it either way. ## Working alongside `@JsonTypeInfo` @@ -194,11 +205,12 @@ performance but not behaviour. ## Tests -70 tests, in `src/test/java/com/github/pjfanning/jackson/sealed/`: +76 tests, in `src/test/java/com/github/pjfanning/jackson/sealed/`: | Test | Covers | | --- | --- | -| `poly/SealedPolymorphismTest` | Ported from the Scala `SealedPolymorphismSpec`, plus enum members | +| `poly/SealedPolymorphismTest` | Ported from the Scala `SealedPolymorphismSpec` | +| `poly/EnumsUntouchedTest` | That enums serialize identically with and without this module | | `poly/NestedPolymorphismTest` | Ported from the Scala `NestedPolymorphismSpec` — a polymorphic value holding a polymorphic value | | `poly/InvalidHierarchyTest` | The four ways a hierarchy can fail to be closed, on both the read and the write path | | `SealedTypesTest` | The name derivation itself, and resolution | @@ -209,8 +221,9 @@ performance but not behaviour. endpoint, which are deliberately not wired up yet. - A Jackson 2.x build. All Jackson API contact is confined to the serializer and deserializer classes, so the reflection core would port unchanged. -- Polymorphic values as `Map` keys. An enum key keeps Jackson's ordinary key handling; a tagged - object cannot be a JSON property name, so a marked hierarchy is not usable as a key type. +- Polymorphic values as `Map` keys. A tagged object cannot be a JSON property name, so a handled + hierarchy is not usable as a key type. Enum keys are unaffected, being Jackson's to write. +- Reading an enum member back through the hierarchy's base type — see above. ## License diff --git a/src/main/java/com/github/pjfanning/jackson/sealed/SealedHierarchy.java b/src/main/java/com/github/pjfanning/jackson/sealed/SealedHierarchy.java index 431db08..0f54b0f 100644 --- a/src/main/java/com/github/pjfanning/jackson/sealed/SealedHierarchy.java +++ b/src/main/java/com/github/pjfanning/jackson/sealed/SealedHierarchy.java @@ -2,16 +2,20 @@ import java.lang.reflect.Modifier; import java.util.ArrayDeque; +import java.util.Arrays; import java.util.Deque; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import com.fasterxml.jackson.annotation.JsonTypeInfo; /** - * The implementations of one marked hierarchy, and the {@code @type} name each is written under. + * The implementations of one hierarchy, and the {@code @type} name each is written under. * *

Built once per hierarchy root by walking the {@code PermittedSubclasses} attribute that * {@code javac} records for every {@code sealed} type. The result is an exact, closed table: a name @@ -22,32 +26,32 @@ * *

Building the table is also where the hierarchy is checked to be closed, so a type that could * not be read back is reported before anything is written. + * + *

Enum members are left out. Jackson writes an enum as a string, and this module does not take + * that over, so an enum permitted by the root carries no {@code @type} name of its own. */ final class SealedHierarchy { private final Class root; /** True when the root carries {@code @JsonTypeInfo}, so Jackson's own handling owns it. */ private final boolean jacksonOwned; - private final Map byName; + private final Map> byName; private final Map, String> namesByClass; + /** Permitted enums, kept only so that a value of one can be explained when it cannot be read. */ + private final Set> enumMembers; - private SealedHierarchy(Class root, boolean jacksonOwned, - Map byName, Map, String> namesByClass) { + private SealedHierarchy(Class root, boolean jacksonOwned, Map> byName, + Map, String> namesByClass, Set> enumMembers) { this.root = root; this.jacksonOwned = jacksonOwned; this.byName = byName; this.namesByClass = namesByClass; + this.enumMembers = enumMembers; } static SealedHierarchy of(Class root) { if (root.getAnnotation(JsonTypeInfo.class) != null) { - return new SealedHierarchy(root, true, Map.of(), Map.of()); - } - if (SealedTypes.enumClassOf(root) != null) { - throw new IllegalArgumentException(root.getName() + " is an enum marked with " - + SealedPolymorphismSupport.class.getSimpleName() + ". An enum is not a hierarchy of its own; " - + "mark the sealed interface it implements instead, and its constants are named " - + "individually within that hierarchy."); + return new SealedHierarchy(root, true, Map.of(), Map.of(), Set.of()); } if (!root.isSealed()) { throw new IllegalArgumentException(root.getName() + " is marked with " @@ -56,65 +60,59 @@ static SealedHierarchy of(Class root) { + "its permitted subtypes as `final` or `sealed` in turn."); } - Map byName = new HashMap<>(); + Map> byName = new HashMap<>(); Map, String> namesByClass = new HashMap<>(); - collect(root, root, byName, namesByClass, new HashSet<>()); - return new SealedHierarchy(root, false, Map.copyOf(byName), Map.copyOf(namesByClass)); + Set> enumMembers = new LinkedHashSet<>(); + collect(root, byName, namesByClass, enumMembers); + // Set.copyOf does not keep insertion order, and this set is rendered into an error message + return new SealedHierarchy(root, false, Map.copyOf(byName), Map.copyOf(namesByClass), + Collections.unmodifiableSet(enumMembers)); } - private static void collect(Class root, Class current, Map byName, - Map, String> namesByClass, Set> seen) { + private static void collect(Class root, Map> byName, Map, String> namesByClass, + Set> enumMembers) { + Set> seen = new HashSet<>(); Deque> queue = new ArrayDeque<>(); - queue.add(current); + queue.add(root); while (!queue.isEmpty()) { Class clazz = queue.poll(); if (!seen.add(clazz)) { continue; } + // an enum is Jackson's to write, as a string - it takes no name here, and its constant + // bodies are not part of this hierarchy either Class enumClass = SealedTypes.enumClassOf(clazz); if (enumClass != null) { - // an enum is closed by construction, and each constant is a value of the hierarchy in - // its own right - the Java counterpart of the Scala module's `case object` - String prefix = SealedTypes.typeNameFor(root, enumClass); - for (Object constant : enumClass.getEnumConstants()) { - Enum value = (Enum) constant; - register(root, byName, prefix + '$' + value.name(), Subtype.ofConstant(value), value.getClass()); - } + enumMembers.add(enumClass); continue; } - int modifiers = clazz.getModifiers(); boolean sealed = clazz.isSealed(); - if (!sealed && !Modifier.isFinal(modifiers)) { - throw new IllegalArgumentException(clazz.getName() + " belongs to the " - + SealedPolymorphismSupport.class.getSimpleName() + " hierarchy rooted at " + root.getName() - + ", but is neither sealed nor final. A `non-sealed` type reopens the hierarchy, so its " - + "subclasses could not be resolved back from a " + SealedTypes.TYPE_PROPERTY_NAME - + " name; declare " + clazz.getSimpleName() + " as `final` or `sealed`."); + if (!sealed && !Modifier.isFinal(clazz.getModifiers())) { + throw new IllegalArgumentException(clazz.getName() + " belongs to the sealed hierarchy rooted at " + + root.getName() + ", but is neither sealed nor final. A `non-sealed` type reopens the " + + "hierarchy, so its subclasses could not be resolved back from a " + + SealedTypes.TYPE_PROPERTY_NAME + " name; declare " + clazz.getSimpleName() + + " as `final` or `sealed`."); } // an interface or abstract class is only ever dispatched through, so carries no name of its own if (SealedTypes.isConcrete(clazz)) { String name = SealedTypes.typeNameFor(root, clazz); - register(root, byName, name, Subtype.ofClass(clazz), clazz); + Class existing = byName.putIfAbsent(name, clazz); + if (existing != null && existing != clazz) { + throw new IllegalArgumentException(clazz.getName() + " is written as " + + SealedTypes.TYPE_PROPERTY_NAME + " '" + name + "', but that name already belongs to " + + existing.getName() + " in the hierarchy rooted at " + root.getName() + + ". Rename one of them so that the two derive different names."); + } namesByClass.put(clazz, name); } if (sealed) { - queue.addAll(java.util.Arrays.asList(clazz.getPermittedSubclasses())); + queue.addAll(Arrays.asList(clazz.getPermittedSubclasses())); } } } - private static void register(Class root, Map byName, String name, Subtype subtype, - Class declaring) { - Subtype existing = byName.putIfAbsent(name, subtype); - if (existing != null && !existing.equals(subtype)) { - throw new IllegalArgumentException(declaring.getName() + " is written as " - + SealedTypes.TYPE_PROPERTY_NAME + " '" + name + "', but that name already belongs to " - + existing.type().getName() + " in the hierarchy rooted at " + root.getName() - + ". Rename one of them so that the two derive different names."); - } - } - Class root() { return root; } @@ -132,26 +130,36 @@ boolean isJacksonOwned() { * {@code baseClass} here, so a name from a sibling branch does not resolve into a property that * could not hold it. */ - Subtype resolve(Class baseClass, String typeName) { + Class resolve(Class baseClass, String typeName) { if (typeName == null) { return null; } - Subtype subtype = byName.get(typeName); - return (subtype != null && baseClass.isAssignableFrom(subtype.type())) ? subtype : null; + Class subtype = byName.get(typeName); + return (subtype != null && baseClass.isAssignableFrom(subtype)) ? subtype : null; + } + + /** + * Explains that a value read at this hierarchy's base was not an object, when the reason is that + * the hierarchy permits an enum - which Jackson writes as a string, and which therefore cannot + * be dispatched on. Returns {@code null} when that is not the explanation. + */ + String enumMemberHint() { + if (enumMembers.isEmpty()) { + return null; + } + return " The hierarchy permits " + enumMembers.stream().map(Class::getSimpleName) + .collect(Collectors.joining(", ")) + ", which Jackson writes as a string rather than a tagged " + + "object, so a value of that type cannot be read back through " + root.getSimpleName() + + ". Declare the property as the enum type itself, or replace the enum with final classes."; } /** The {@code @type} name for a concrete implementation. */ String nameOf(Class clazz) { String name = namesByClass.get(clazz); if (name == null) { - throw new IllegalArgumentException(clazz.getName() + " is not a permitted implementation of the " - + SealedPolymorphismSupport.class.getSimpleName() + " hierarchy rooted at " + root.getName() + "."); + throw new IllegalArgumentException(clazz.getName() + " is not a permitted implementation of the sealed " + + "hierarchy rooted at " + root.getName() + "."); } return name; } - - /** The {@code @type} name of an enum constant of this hierarchy. */ - String nameOfConstant(Enum constant) { - return SealedTypes.typeNameFor(root, constant.getDeclaringClass()) + '$' + constant.name(); - } } diff --git a/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphicDeserializer.java b/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphicDeserializer.java index c5d22ee..b593601 100644 --- a/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphicDeserializer.java +++ b/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphicDeserializer.java @@ -23,17 +23,15 @@ final class SealedPolymorphicDeserializer extends StdDeserializer { @Override public Object deserialize(JsonParser p, DeserializationContext ctxt) { if (p.currentToken() != JsonToken.START_OBJECT) { - return ctxt.reportInputMismatch(baseClass, "Expected a JSON object with a %s property to create %s", - SealedTypes.TYPE_PROPERTY_NAME, baseClass.getName()); + String hint = SealedTypes.hierarchyOf(baseClass).enumMemberHint(); + return ctxt.reportInputMismatch(baseClass, "Expected a JSON object with a %s property to create %s.%s", + SealedTypes.TYPE_PROPERTY_NAME, baseClass.getName(), hint == null ? "" : hint); } TaggedObject tagged = TaggedObject.split(p, ctxt); - Subtype subtype = SealedTypes.hierarchyOf(baseClass).resolve(baseClass, tagged.typeName()); + Class subtype = SealedTypes.hierarchyOf(baseClass).resolve(baseClass, tagged.typeName()); if (subtype == null) { return TaggedObject.unresolved(ctxt, baseClass, tagged.typeName()); } - if (subtype.singleton() != null) { - return subtype.singleton(); - } - return ctxt.readValue(tagged.parser(), subtype.type()); + return ctxt.readValue(tagged.parser(), subtype); } } diff --git a/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismDeserializerModifier.java b/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismDeserializerModifier.java index e9adfe8..d3e7789 100644 --- a/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismDeserializerModifier.java +++ b/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismDeserializerModifier.java @@ -2,7 +2,6 @@ import tools.jackson.databind.BeanDescription; import tools.jackson.databind.DeserializationConfig; -import tools.jackson.databind.JavaType; import tools.jackson.databind.ValueDeserializer; import tools.jackson.databind.deser.BeanDeserializerBuilder; import tools.jackson.databind.deser.ValueDeserializerModifier; @@ -42,16 +41,4 @@ public ValueDeserializer modifyDeserializer(DeserializationConfig config, Bea return new TaggedBeanDeserializer(rawClass, delegate); } - @Override - public ValueDeserializer modifyEnumDeserializer(DeserializationConfig config, JavaType valueType, - BeanDescription.Supplier beanDescRef, - ValueDeserializer deserializer) { - Class rawClass = valueType.getRawClass(); - if (!SealedTypes.isMarked(rawClass) || !SealedTypes.isSupported(rawClass)) { - return deserializer; - } - @SuppressWarnings("unchecked") - ValueDeserializer delegate = (ValueDeserializer) deserializer; - return new TaggedEnumDeserializer(rawClass, delegate); - } } diff --git a/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismSerializerModifier.java b/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismSerializerModifier.java index 3948a0d..4adaa4c 100644 --- a/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismSerializerModifier.java +++ b/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismSerializerModifier.java @@ -1,7 +1,6 @@ package com.github.pjfanning.jackson.sealed; import tools.jackson.databind.BeanDescription; -import tools.jackson.databind.JavaType; import tools.jackson.databind.SerializationConfig; import tools.jackson.databind.ValueSerializer; import tools.jackson.databind.ser.ValueSerializerModifier; @@ -26,27 +25,9 @@ public ValueSerializer modifySerializer(SerializationConfig config, BeanDescr if (!SealedTypes.isSupported(rawClass) || !SealedTypes.isConcrete(rawClass)) { return serializer; } - // an enum reaches this path as well as modifyEnumSerializer, and is named per constant - if (SealedTypes.enumClassOf(rawClass) != null) { - return new TypeTaggedEnumSerializer(SealedTypes.hierarchyOf(rawClass)); - } @SuppressWarnings("unchecked") ValueSerializer delegate = (ValueSerializer) serializer; return new TypeTaggedSerializer(SealedTypes.hierarchyOf(rawClass).nameOf(rawClass), delegate); } - @Override - public ValueSerializer modifyEnumSerializer(SerializationConfig config, JavaType valueType, - BeanDescription.Supplier beanDescRef, - ValueSerializer serializer) { - Class rawClass = valueType.getRawClass(); - if (!SealedTypes.isMarked(rawClass)) { - return serializer; - } - SealedTypes.checkNoConflictingJsonTypeInfo(rawClass); - if (!SealedTypes.isSupported(rawClass)) { - return serializer; - } - return new TypeTaggedEnumSerializer(SealedTypes.hierarchyOf(rawClass)); - } } diff --git a/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismSupport.java b/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismSupport.java index 9abc878..84f6581 100644 --- a/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismSupport.java +++ b/src/main/java/com/github/pjfanning/jackson/sealed/SealedPolymorphismSupport.java @@ -43,8 +43,16 @@ * is neither - a plain interface, an abstract class that is not sealed, or a {@code non-sealed} * member, which reopens the hierarchy to subclasses that could never be resolved back from a name - * is reported as a configuration error the first time Jackson meets it, rather than being written - * out as JSON that could not be read back. Records and enum constants are closed by construction - * and need no modifier of their own. + * out as JSON that could not be read back. Records are closed by construction and need no modifier + * of their own. + * + *

Enums are left to Jackson

+ * + *

An enum permitted by the root is not handled here. Jackson writes an enum as a string and keeps + * doing so, so an enum member carries no {@code @type} name - which means a value of one cannot be + * read back through the hierarchy's base type, though it reads normally where the property is + * declared as the enum type itself. For a stateless member that does round trip through the base, + * use a record with no components. Putting this marker on an enum has no effect. * *

Deferring to Jackson

* diff --git a/src/main/java/com/github/pjfanning/jackson/sealed/SealedTypes.java b/src/main/java/com/github/pjfanning/jackson/sealed/SealedTypes.java index cb3edbe..0a22178 100644 --- a/src/main/java/com/github/pjfanning/jackson/sealed/SealedTypes.java +++ b/src/main/java/com/github/pjfanning/jackson/sealed/SealedTypes.java @@ -38,7 +38,8 @@ static boolean isMarked(Class clazz) { * twice. */ static boolean isSupported(Class clazz) { - return isMarked(clazz) && !hierarchyOf(clazz).isJacksonOwned(); + // an enum is Jackson's to write, as a string; this module does not take that over + return isMarked(clazz) && enumClassOf(clazz) == null && !hierarchyOf(clazz).isJacksonOwned(); } /** True for a type that can hold a value of its own, so can carry a name of its own. */ diff --git a/src/main/java/com/github/pjfanning/jackson/sealed/Subtype.java b/src/main/java/com/github/pjfanning/jackson/sealed/Subtype.java deleted file mode 100644 index 111e5a9..0000000 --- a/src/main/java/com/github/pjfanning/jackson/sealed/Subtype.java +++ /dev/null @@ -1,19 +0,0 @@ -package com.github.pjfanning.jackson.sealed; - -/** - * One implementation of a marked hierarchy, as reached from its {@code @type} name. - * - * @param type the class the name resolves to - * @param singleton the one value of that name, for a type that has exactly one - an enum constant - - * or {@code null} for a type whose values have to be read from the JSON - */ -record Subtype(Class type, Object singleton) { - - static Subtype ofClass(Class type) { - return new Subtype(type, null); - } - - static Subtype ofConstant(Enum constant) { - return new Subtype(constant.getDeclaringClass(), constant); - } -} diff --git a/src/main/java/com/github/pjfanning/jackson/sealed/TaggedBeanDeserializer.java b/src/main/java/com/github/pjfanning/jackson/sealed/TaggedBeanDeserializer.java index 2b1f6d9..d14cfd9 100644 --- a/src/main/java/com/github/pjfanning/jackson/sealed/TaggedBeanDeserializer.java +++ b/src/main/java/com/github/pjfanning/jackson/sealed/TaggedBeanDeserializer.java @@ -49,16 +49,13 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt) { if (tagged.typeName() == null) { return delegate.deserialize(tagged.parser(), ctxt); } - Subtype subtype = SealedTypes.hierarchyOf(declaredClass).resolve(declaredClass, tagged.typeName()); + Class subtype = SealedTypes.hierarchyOf(declaredClass).resolve(declaredClass, tagged.typeName()); if (subtype == null) { return TaggedObject.unresolved(ctxt, declaredClass, tagged.typeName()); } - if (subtype.type() == declaredClass) { + if (subtype == declaredClass) { return delegate.deserialize(tagged.parser(), ctxt); } - if (subtype.singleton() != null) { - return subtype.singleton(); - } - return ctxt.readValue(tagged.parser(), subtype.type()); + return ctxt.readValue(tagged.parser(), subtype); } } diff --git a/src/main/java/com/github/pjfanning/jackson/sealed/TaggedEnumDeserializer.java b/src/main/java/com/github/pjfanning/jackson/sealed/TaggedEnumDeserializer.java deleted file mode 100644 index f96d0ac..0000000 --- a/src/main/java/com/github/pjfanning/jackson/sealed/TaggedEnumDeserializer.java +++ /dev/null @@ -1,38 +0,0 @@ -package com.github.pjfanning.jackson.sealed; - -import tools.jackson.core.JsonParser; -import tools.jackson.core.JsonToken; -import tools.jackson.databind.DeserializationContext; -import tools.jackson.databind.ValueDeserializer; - -/** - * Reads an enum constant of a marked hierarchy from the tagged object form its serializer writes. - * Needed where a property is declared as the enum type itself rather than as the hierarchy's base, - * so the value is read straight rather than dispatched. - * - *

Anything that is not an object - a plain string, as Jackson writes an untagged enum - is left - * to the enum's own deserializer, so enums read from JSON this module did not write still work. - */ -final class TaggedEnumDeserializer extends ValueDeserializer { - - private final Class enumClass; - private final ValueDeserializer delegate; - - TaggedEnumDeserializer(Class enumClass, ValueDeserializer delegate) { - this.enumClass = enumClass; - this.delegate = delegate; - } - - @Override - public Object deserialize(JsonParser p, DeserializationContext ctxt) { - if (p.currentToken() != JsonToken.START_OBJECT) { - return delegate.deserialize(p, ctxt); - } - TaggedObject tagged = TaggedObject.split(p, ctxt); - Subtype subtype = SealedTypes.hierarchyOf(enumClass).resolve(enumClass, tagged.typeName()); - if (subtype == null || subtype.singleton() == null) { - return TaggedObject.unresolved(ctxt, enumClass, tagged.typeName()); - } - return subtype.singleton(); - } -} diff --git a/src/main/java/com/github/pjfanning/jackson/sealed/TypeTaggedEnumSerializer.java b/src/main/java/com/github/pjfanning/jackson/sealed/TypeTaggedEnumSerializer.java deleted file mode 100644 index bb1113f..0000000 --- a/src/main/java/com/github/pjfanning/jackson/sealed/TypeTaggedEnumSerializer.java +++ /dev/null @@ -1,33 +0,0 @@ -package com.github.pjfanning.jackson.sealed; - -import tools.jackson.core.JsonGenerator; -import tools.jackson.databind.SerializationContext; -import tools.jackson.databind.ValueSerializer; - -/** - * Writes an enum constant of a marked hierarchy as a tagged, otherwise empty object - - * {@code {"@type":"Status$IDLE"}}. - * - *

An enum constant is the Java counterpart of the Scala module's {@code case object}: one value, - * carrying no state, that has to be told apart from its siblings by name alone. The enum class - * cannot be the unit of naming, because it holds several such values, so each constant is named - * within it. - * - *

Only value serializers are replaced. An enum used as a map key keeps Jackson's ordinary key - * handling, since a tagged object cannot be a JSON property name. - */ -final class TypeTaggedEnumSerializer extends ValueSerializer { - - private final SealedHierarchy hierarchy; - - TypeTaggedEnumSerializer(SealedHierarchy hierarchy) { - this.hierarchy = hierarchy; - } - - @Override - public void serialize(Object value, JsonGenerator gen, SerializationContext ctxt) { - gen.writeStartObject(value); - gen.writeStringProperty(SealedTypes.TYPE_PROPERTY_NAME, hierarchy.nameOfConstant((Enum) value)); - gen.writeEndObject(); - } -} diff --git a/src/test/java/com/github/pjfanning/jackson/sealed/SealedTypesTest.java b/src/test/java/com/github/pjfanning/jackson/sealed/SealedTypesTest.java index 57b789d..36e68a3 100644 --- a/src/test/java/com/github/pjfanning/jackson/sealed/SealedTypesTest.java +++ b/src/test/java/com/github/pjfanning/jackson/sealed/SealedTypesTest.java @@ -120,34 +120,36 @@ void findsTheEnumAConstantBelongsTo() { assertThat(SealedTypes.enumClassOf(Fixtures.Dog.class)).isNull(); } + /** An enum permitted by the root is Jackson's to write, so it takes no name in the table. */ @Test - void namesEnumConstantsWithinTheirEnum() { + void givesNoNameToAnEnumMember() { SealedHierarchy hierarchy = SealedTypes.hierarchyOf(Fixtures.Signal.class); - assertThat(hierarchy.nameOfConstant(Fixtures.Status.IDLE)).isEqualTo("Status$IDLE"); - assertThat(hierarchy.nameOfConstant(Fixtures.Status.BUSY)).isEqualTo("Status$BUSY"); + assertThat(hierarchy.resolve(Fixtures.Signal.class, "Status")).isNull(); + assertThat(hierarchy.resolve(Fixtures.Signal.class, "Status$IDLE")).isNull(); + assertThat(hierarchy.resolve(Fixtures.Signal.class, "IDLE")).isNull(); + // its sibling in the same hierarchy is named as usual + assertThat(hierarchy.resolve(Fixtures.Signal.class, "Data")).isEqualTo(Fixtures.Data.class); } @Test - void resolvesANameToItsImplementation() { - SealedHierarchy hierarchy = SealedTypes.hierarchyOf(Fixtures.Animal.class); - Subtype dog = hierarchy.resolve(Fixtures.Animal.class, "Dog"); - assertThat(dog).isNotNull(); - assertThat(dog.type()).isEqualTo(Fixtures.Dog.class); - assertThat(dog.singleton()).isNull(); + void doesNotHandleAnEnumMember() { + assertThat(SealedTypes.isMarked(Fixtures.Status.class)).isTrue(); + assertThat(SealedTypes.isSupported(Fixtures.Status.class)).isFalse(); + assertThat(SealedTypes.isBaseType(Fixtures.Status.class)).isFalse(); + assertThat(SealedTypes.needsSubtypeDispatch(Fixtures.Status.class)).isFalse(); } @Test - void resolvesAnEnumConstantToItsSingleton() { - Subtype idle = SealedTypes.hierarchyOf(Fixtures.Signal.class).resolve(Fixtures.Signal.class, "Status$IDLE"); - assertThat(idle).isNotNull(); - assertThat(idle.singleton()).isSameAs(Fixtures.Status.IDLE); + void resolvesANameToItsImplementation() { + SealedHierarchy hierarchy = SealedTypes.hierarchyOf(Fixtures.Animal.class); + assertThat(hierarchy.resolve(Fixtures.Animal.class, "Dog")).isEqualTo(Fixtures.Dog.class); } /** A name only resolves where the declared type could actually hold the result. */ @Test void refusesANameFromASiblingBranch() { SealedHierarchy hierarchy = SealedTypes.hierarchyOf(Fixtures.Animal.class); - assertThat(hierarchy.resolve(Fixtures.Animal.class, "Bird")).isNotNull(); + assertThat(hierarchy.resolve(Fixtures.Animal.class, "Bird")).isEqualTo(Fixtures.Bird.class); assertThat(hierarchy.resolve(Fixtures.Dog.class, "Bird")).isNull(); } @@ -173,6 +175,6 @@ void survivesTheCacheBeingCleared() { SealedPolymorphismModule.clearCache(); SealedHierarchy after = SealedTypes.hierarchyOf(Fixtures.Animal.class); assertThat(after).isNotSameAs(before); - assertThat(after.resolve(Fixtures.Animal.class, "Dog").type()).isEqualTo(Fixtures.Dog.class); + assertThat(after.resolve(Fixtures.Animal.class, "Dog")).isEqualTo(Fixtures.Dog.class); } } diff --git a/src/test/java/com/github/pjfanning/jackson/sealed/poly/EnumsUntouchedTest.java b/src/test/java/com/github/pjfanning/jackson/sealed/poly/EnumsUntouchedTest.java new file mode 100644 index 0000000..3fa68f5 --- /dev/null +++ b/src/test/java/com/github/pjfanning/jackson/sealed/poly/EnumsUntouchedTest.java @@ -0,0 +1,96 @@ +package com.github.pjfanning.jackson.sealed.poly; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; +import java.util.Map; + +import com.github.pjfanning.jackson.sealed.SealedPolymorphismModule; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.Gauge; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.GaugeHolder; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.Level; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.LevelHolder; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.Mode; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.ModeHolder; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.Reading; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.Signal; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.SignalHolder; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.Status; +import com.github.pjfanning.jackson.sealed.poly.Fixtures.StatusHolder; +import org.junit.jupiter.api.Test; +import tools.jackson.databind.ObjectMapper; +import tools.jackson.databind.json.JsonMapper; + +/** + * Enums belong to Jackson, not to this module, even inside a hierarchy it handles. + * + *

Each case writes the same value through a mapper carrying the module and through a plain one, + * and asserts the two agree - the strongest form of "we did not interfere", since it would catch a + * difference this module's author had not thought to predict. + */ +class EnumsUntouchedTest { + + private final ObjectMapper withModule = JsonMapper.builder() + .addModule(new SealedPolymorphismModule()) + .build(); + private final ObjectMapper vanilla = JsonMapper.builder().build(); + + private void agreesWithPlainJackson(Object value) { + assertThat(withModule.writeValueAsString(value)).isEqualTo(vanilla.writeValueAsString(value)); + } + + @Test + void writesAnEnumMemberOfAHierarchyAsPlainJacksonDoes() { + agreesWithPlainJackson(new SignalHolder(Status.IDLE)); + agreesWithPlainJackson(new StatusHolder(Status.BUSY)); + agreesWithPlainJackson(Status.IDLE); + assertThat(withModule.writeValueAsString(new SignalHolder(Status.IDLE))).isEqualTo("{\"signal\":\"IDLE\"}"); + } + + /** + * Constants with bodies compile to anonymous subclasses, and such an enum is implicitly sealed + * and abstract - so it reaches every check this module makes about sealed types. + */ + @Test + void writesAnEnumWithConstantBodiesAsPlainJacksonDoes() { + agreesWithPlainJackson(new GaugeHolder(Level.LOW)); + agreesWithPlainJackson(new LevelHolder(Level.HIGH)); + assertThat(withModule.writeValueAsString(new GaugeHolder(Level.LOW))).isEqualTo("{\"gauge\":\"LOW\"}"); + // the constant's own class is not the enum class, and neither is tagged + assertThat(Level.LOW.getClass()).isNotEqualTo(Level.class); + assertThat(Level.class.isSealed()).isTrue(); + } + + @Test + void keepsACustomEnumRepresentation() { + agreesWithPlainJackson(new ModeHolder(Mode.FAST)); + assertThat(withModule.writeValueAsString(new ModeHolder(Mode.FAST))).isEqualTo("{\"mode\":\"fast\"}"); + assertThat(withModule.writeValueAsString(new SignalHolder(Mode.SLOW))).isEqualTo("{\"signal\":\"slow\"}"); + } + + @Test + void writesEnumsInCollectionsAndAsMapKeysAsPlainJacksonDoes() { + agreesWithPlainJackson(Map.of("a", Status.IDLE)); + agreesWithPlainJackson(List.of(Status.IDLE, Status.BUSY)); + agreesWithPlainJackson(Map.of(Status.IDLE, "x")); + assertThat(withModule.writeValueAsString(Map.of(Status.IDLE, "x"))).isEqualTo("{\"IDLE\":\"x\"}"); + } + + @Test + void readsEnumsBackWhenDeclaredAsTheirOwnType() { + assertThat(withModule.readValue("{\"status\":\"BUSY\"}", StatusHolder.class).status()).isSameAs(Status.BUSY); + assertThat(withModule.readValue("{\"level\":\"HIGH\"}", LevelHolder.class).level()).isSameAs(Level.HIGH); + assertThat(withModule.readValue("{\"mode\":\"fast\"}", ModeHolder.class).mode()).isSameAs(Mode.FAST); + assertThat(withModule.readValue("\"IDLE\"", Status.class)).isSameAs(Status.IDLE); + } + + /** A non-enum sibling in the same hierarchy is still tagged and still round trips. */ + @Test + void stillTagsTheNonEnumMembersOfTheSameHierarchy() { + assertThat(withModule.writeValueAsString(new GaugeHolder(new Reading(4)))) + .isEqualTo("{\"gauge\":{\"@type\":\"Reading\",\"v\":4}}"); + Gauge gauge = withModule.readValue("{\"gauge\":{\"@type\":\"Reading\",\"v\":4}}", GaugeHolder.class).gauge(); + assertThat(gauge).isEqualTo(new Reading(4)); + assertThat(Signal.class).isNotNull(); + } +} diff --git a/src/test/java/com/github/pjfanning/jackson/sealed/poly/Fixtures.java b/src/test/java/com/github/pjfanning/jackson/sealed/poly/Fixtures.java index 3fe2b5e..318ee65 100644 --- a/src/test/java/com/github/pjfanning/jackson/sealed/poly/Fixtures.java +++ b/src/test/java/com/github/pjfanning/jackson/sealed/poly/Fixtures.java @@ -242,7 +242,7 @@ public record Limb(Branch branch) { // an enum member of a marked hierarchy - the closest Java has to a set of Scala case objects, // so each constant is named individually rather than the enum class as a whole - public sealed interface Signal extends SealedPolymorphismSupport permits Data, Status { + public sealed interface Signal extends SealedPolymorphismSupport permits Data, Status, Mode { } public record Data(int value) implements Signal { @@ -258,6 +258,51 @@ public record SignalHolder(Signal signal) { public record StatusHolder(Status status) { } + // an enum whose constants have bodies, so it is implicitly sealed and abstract, and each + // constant compiles to an anonymous subclass - the shape most likely to trip a module that + // walks permitted subclasses + public sealed interface Gauge extends SealedPolymorphismSupport permits Reading, Level { + } + + public record Reading(int v) implements Gauge { + } + + public enum Level implements Gauge { + LOW { + @Override + public int weight() { + return 1; + } + }, + HIGH { + @Override + public int weight() { + return 9; + } + }; + + public abstract int weight(); + } + + public record GaugeHolder(Gauge gauge) { + } + + public record LevelHolder(Level level) { + } + + // an enum with a custom JSON representation, to check the module does not override it + public enum Mode implements Signal { + FAST, SLOW; + + @com.fasterxml.jackson.annotation.JsonValue + public String json() { + return name().toLowerCase(java.util.Locale.ROOT); + } + } + + public record ModeHolder(Mode mode) { + } + // a hierarchy that is not marked - it must be untouched public sealed interface Plain permits PlainDog { } diff --git a/src/test/java/com/github/pjfanning/jackson/sealed/poly/InvalidHierarchyTest.java b/src/test/java/com/github/pjfanning/jackson/sealed/poly/InvalidHierarchyTest.java index 016a4dd..e84a5a7 100644 --- a/src/test/java/com/github/pjfanning/jackson/sealed/poly/InvalidHierarchyTest.java +++ b/src/test/java/com/github/pjfanning/jackson/sealed/poly/InvalidHierarchyTest.java @@ -80,11 +80,16 @@ void refusesToReadAHierarchyWithAClashingName() { .satisfies(messageContaining("already belongs to")); } + /** + * An enum carrying the marker is not an error, it is simply ignored - enums are Jackson's to + * write however they are declared, so the marker has nothing to add and nothing to complain + * about. + */ @Test - void refusesAnEnumMarkedDirectly() { - assertThatThrownBy(() -> mapper.writeValueAsString(new MarkedEnumHolder(MarkedEnum.ONE))) - .satisfies(messageContaining("MarkedEnum", "is an enum marked with", - "mark the sealed interface it implements")); + void ignoresAnEnumMarkedDirectly() { + assertThat(mapper.writeValueAsString(new MarkedEnumHolder(MarkedEnum.ONE))) + .isEqualTo("{\"e\":\"ONE\"}"); + assertThat(mapper.readValue("{\"e\":\"TWO\"}", MarkedEnumHolder.class).e()).isSameAs(MarkedEnum.TWO); } /** diff --git a/src/test/java/com/github/pjfanning/jackson/sealed/poly/SealedPolymorphismTest.java b/src/test/java/com/github/pjfanning/jackson/sealed/poly/SealedPolymorphismTest.java index 48fd29e..5ebb288 100644 --- a/src/test/java/com/github/pjfanning/jackson/sealed/poly/SealedPolymorphismTest.java +++ b/src/test/java/com/github/pjfanning/jackson/sealed/poly/SealedPolymorphismTest.java @@ -226,23 +226,37 @@ void readsAnUntaggedObjectAtAConcreteDeclaredType() { assertThat(branch.getLabel()).isEqualTo("u"); } + /** An enum is left to Jackson, which writes it as a string - it gets no tag of its own. */ @Test - void namesEachConstantOfAnEnumMember() { + void leavesAnEnumMemberToJackson() { assertThat(mapper.writeValueAsString(new SignalHolder(Status.IDLE))) - .isEqualTo("{\"signal\":{\"@type\":\"Status$IDLE\"}}"); + .isEqualTo("{\"signal\":\"IDLE\"}"); + // its sibling in the same hierarchy is tagged as usual assertThat(mapper.writeValueAsString(new SignalHolder(new Data(1)))) .isEqualTo("{\"signal\":{\"@type\":\"Data\",\"value\":1}}"); } @Test - void roundTripsAnEnumMemberToTheSameConstant() { - assertThat(roundTrip(new SignalHolder(Status.BUSY), SignalHolder.class).signal()).isSameAs(Status.BUSY); - assertThat(mapper.readValue("{\"@type\":\"Status$IDLE\"}", Signal.class)).isSameAs(Status.IDLE); + void roundTripsAnEnumDeclaredAsItsOwnType() { + assertThat(mapper.writeValueAsString(new StatusHolder(Status.BUSY))) + .isEqualTo("{\"status\":\"BUSY\"}"); + assertThat(roundTrip(new StatusHolder(Status.BUSY), StatusHolder.class).status()).isSameAs(Status.BUSY); } + /** + * The consequence of leaving enums alone: a string is not something the base type can dispatch + * on, so an enum value written at the base type cannot be read back there. The failure says so. + */ @Test - void roundTripsAnEnumDeclaredAsItsOwnType() { - assertThat(roundTrip(new StatusHolder(Status.BUSY), StatusHolder.class).status()).isSameAs(Status.BUSY); + void cannotReadAnEnumMemberBackThroughTheBaseType() { + String json = mapper.writeValueAsString(new SignalHolder(Status.IDLE)); + assertThatThrownBy(() -> mapper.readValue(json, SignalHolder.class)) + .satisfies(error -> assertThat(String.valueOf(rootCause(error).getMessage())) + .contains("Expected a JSON object") + .contains("permits Status, Mode") + .contains("writes as a string") + .contains("Declare the property as the enum type itself")); + assertThat(Signal.class).isNotNull(); } @Test