From 2556af1a5e9921676e975570d58ca036e610f3db Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 23 Aug 2026 20:09:56 +0100 Subject: [PATCH] Leave Java enums entirely to Jackson An enum permitted by a handled root was being written as a tagged object with a name per constant - {"@type":"Status$IDLE"} - which took over a representation Jackson already has. Enums are now skipped everywhere: Jackson writes them as strings and this module does not interfere. That removes the only thing a resolved name could be other than a class, so Subtype and the singleton it carried are gone, along with TypeTaggedEnumSerializer, TaggedEnumDeserializer and the enum modifier hooks. A name now resolves to a Class and nothing else. Putting the marker on an enum is no longer an error either - it simply has no effect, since an enum is written as a string however it is declared. The consequence is that an enum member has no @type name, so a value of one cannot be read back through the hierarchy's base type: a string is not something the base can dispatch on. Writing works, and reading works wherever the property is declared as the enum type itself. The failure explains this rather than just reporting a token mismatch, naming the enums the hierarchy permits and pointing at the alternatives. The enum tests are kept with corrected assertions, and EnumsUntouched proves non-interference by writing each value through a mapper with the module and a plain one and asserting the two agree - which catches differences that were not predicted. It covers enums declared at the base and at their own type, in collections, as map keys, with a custom @JsonValue, and with constant bodies, that last being implicitly sealed and abstract with each constant an anonymous subclass, so it reaches every sealed check the module makes. 76 tests. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012hxYY9KPDAMXADWzjBHNV4 --- README.md | 41 ++++--- .../jackson/sealed/SealedHierarchy.java | 116 ++++++++++-------- .../sealed/SealedPolymorphicDeserializer.java | 12 +- ...ealedPolymorphismDeserializerModifier.java | 13 -- .../SealedPolymorphismSerializerModifier.java | 19 --- .../sealed/SealedPolymorphismSupport.java | 12 +- .../pjfanning/jackson/sealed/SealedTypes.java | 3 +- .../pjfanning/jackson/sealed/Subtype.java | 19 --- .../sealed/TaggedBeanDeserializer.java | 9 +- .../sealed/TaggedEnumDeserializer.java | 38 ------ .../sealed/TypeTaggedEnumSerializer.java | 33 ----- .../jackson/sealed/SealedTypesTest.java | 32 ++--- .../sealed/poly/EnumsUntouchedTest.java | 96 +++++++++++++++ .../jackson/sealed/poly/Fixtures.java | 47 ++++++- .../sealed/poly/InvalidHierarchyTest.java | 13 +- .../sealed/poly/SealedPolymorphismTest.java | 28 +++-- 16 files changed, 298 insertions(+), 233 deletions(-) delete mode 100644 src/main/java/com/github/pjfanning/jackson/sealed/Subtype.java delete mode 100644 src/main/java/com/github/pjfanning/jackson/sealed/TaggedEnumDeserializer.java delete mode 100644 src/main/java/com/github/pjfanning/jackson/sealed/TypeTaggedEnumSerializer.java create mode 100644 src/test/java/com/github/pjfanning/jackson/sealed/poly/EnumsUntouchedTest.java 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