diff --git a/CHANGELOG.md b/CHANGELOG.md index 8409dd25d..542dab2e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,9 @@ A join widens the query: from the join onward, every clause accepts paths from a - The metamodel processors converge on one contract. The Java annotation processor generates the `NullableMetamodel` chain variant KSP already generates, and a nullable field selects the nullable variant of its child metamodel, so `Owner_.address` reads as the same static type from Java and Kotlin. KSP sources metamodel components from the primary constructor — a body-declared or inherited property has no column, so it gets no metamodel field, and sealed interfaces contribute abstract properties only — and escapes keyword-named properties (`` `object` `` and friends) at every emission site, so a metamodel for such a data class compiles. - The Java annotation processor registers with Gradle as an aggregating incremental annotation processor, so attaching it no longer switches the whole source set to full recompilation on every change. A failure while generating reports the record it occurred on with the stack trace and stops processing, matching the KSP diagnostics. - The Kotlin modules keep their implementation to themselves. Every declaration under storm-kotlin's `st.orm.template.impl` and `st.orm.repository.impl` is `internal` — the `Flow.flatMapConcat` and `flattenConcat` operators that collided with their kotlinx.coroutines namesakes, the top-level predicate factories whose generic names polluted completion, and the `*Impl` classes — as are the kotlinx-serialization converter provider and the Kotlin starter's auto-configured repository post processor. All five Kotlin modules compile in explicit API mode, so a declaration missing an explicit visibility fails the build instead of shipping public. The coroutine-aware SQL log recording that the Ktor plugin shares is the one deliberate exception, published as `st.orm.template.recordSqlLog` behind the `@InternalStormApi` opt-in. +- The Jackson JSON converters key their mapper cache on the field's type: a custom `@JsonSerialize`/`@JsonDeserialize` class shared by fields of different types is registered per field type instead of serving only the first field, and a sealed interface reached through a container type — `@Json List` — has its permitted subtypes registered, so the discriminator resolves for container elements. The kotlinx converter keys its mapper cache on the `@Json` flags alone, bounding it at one mapper per flag combination. +- A record component's field metadata now includes annotations that Java propagates to the backing field, accessor or constructor parameter — an annotation only reaches the component itself when its targets include `RECORD_COMPONENT`, which third-party annotations rarely declare. `@JsonSerialize`/`@JsonDeserialize` on a `@Json` record component are therefore honored; they were silently ignored before. +- `@Json` fields serialize with the declared field type instead of the value's erased runtime type, so a polymorphic value writes the discriminator that reading the column expects, completing the round trip for fields like `@Json List`. ## [1.13.1] - 2026-08-07 diff --git a/storm-core/src/main/java/st/orm/core/repository/impl/DefaultORMReflectionImpl.java b/storm-core/src/main/java/st/orm/core/repository/impl/DefaultORMReflectionImpl.java index 5ab0dfaea..08af11730 100644 --- a/storm-core/src/main/java/st/orm/core/repository/impl/DefaultORMReflectionImpl.java +++ b/storm-core/src/main/java/st/orm/core/repository/impl/DefaultORMReflectionImpl.java @@ -16,19 +16,22 @@ package st.orm.core.repository.impl; import static java.util.Arrays.asList; -import static java.util.Arrays.stream; import static java.util.Objects.requireNonNull; import static java.util.Optional.empty; +import static java.util.stream.IntStream.range; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import java.lang.annotation.Annotation; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Parameter; import java.lang.reflect.RecordComponent; +import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; @@ -111,29 +114,50 @@ protected Optional computeValue(@Nonnull Class type) { if (!type.isRecord()) { return empty(); } + var components = requireNonNull(type.getRecordComponents(), "getRecordComponents should not return null"); return CONSTRUCTOR_CACHE.get(type) .map(constructor -> new RecordType( type, constructor, asList(type.getAnnotations()), - stream(requireNonNull(type.getRecordComponents(), "getRecordComponents should not return null")) - .map(component -> new RecordField( - component.getDeclaringRecord(), - component.getName(), - component.getType(), - component.getGenericType(), - !isNonnull(component), - false, - component.getAccessor(), - asList(component.getAnnotations()) - ) - ) + range(0, components.length) + .mapToObj(index -> { + var component = components[index]; + return new RecordField( + component.getDeclaringRecord(), + component.getName(), + component.getType(), + component.getGenericType(), + !isNonnull(component), + false, + component.getAccessor(), + getAnnotations(component, constructor.getParameters()[index]) + ); + }) .toList() ) ); } }; + /** + * An annotation on a record component reaches the component itself only when its targets include + * RECORD_COMPONENT; annotations from other libraries typically propagate to the backing field, accessor or + * constructor parameter instead, so all four sites are folded together. An instance propagated to several + * sites compares equal and collapses to one, keeping single-instance lookups unambiguous. + */ + private static List getAnnotations(@Nonnull RecordComponent component, @Nonnull Parameter parameter) { + var annotations = new LinkedHashSet<>(asList(component.getAnnotations())); + try { + annotations.addAll(asList(component.getDeclaringRecord().getDeclaredField(component.getName()).getAnnotations())); + } catch (NoSuchFieldException e) { + // A record component always has a backing field of the same name. + } + annotations.addAll(asList(component.getAccessor().getAnnotations())); + annotations.addAll(asList(parameter.getAnnotations())); + return List.copyOf(annotations); + } + @Override public Optional findRecordType(@Nonnull Class type) { return TYPE_CACHE.get(type); diff --git a/storm-jackson2/pom.xml b/storm-jackson2/pom.xml index c7bd721f7..49d7c7f13 100644 --- a/storm-jackson2/pom.xml +++ b/storm-jackson2/pom.xml @@ -47,6 +47,47 @@ + + + org.jetbrains.kotlin + kotlin-maven-plugin + ${kotlin.version} + + 21 + + + + test-compile + process-test-sources + test-compile + + + ${project.basedir}/src/test/java + ${project.basedir}/src/test/kotlin + + + + + + + org.codehaus.mojo + build-helper-maven-plugin + 3.6.0 + + + add-kotlin-test + generate-test-sources + add-test-source + + + src/test/kotlin + + + + + @@ -85,6 +126,21 @@ 2.17.0 test + + + st.orm + storm-kotlin + ${project.version} + test + + + + com.fasterxml.jackson.module + jackson-module-kotlin + 2.17.0 + test + org.springframework.boot spring-boot-starter-test diff --git a/storm-jackson2/src/main/java/st/orm/jackson/spi/JsonORMConverterImpl.java b/storm-jackson2/src/main/java/st/orm/jackson/spi/JsonORMConverterImpl.java index 2a09fc61d..017349322 100644 --- a/storm-jackson2/src/main/java/st/orm/jackson/spi/JsonORMConverterImpl.java +++ b/storm-jackson2/src/main/java/st/orm/jackson/spi/JsonORMConverterImpl.java @@ -27,16 +27,21 @@ import com.fasterxml.jackson.databind.JsonDeserializer; import com.fasterxml.jackson.databind.JsonSerializer; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectWriter; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.fasterxml.jackson.databind.jsontype.NamedType; import com.fasterxml.jackson.databind.module.SimpleModule; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import java.lang.reflect.GenericArrayType; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.lang.reflect.WildcardType; +import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import st.orm.Json; import st.orm.core.spi.JsonString; import st.orm.core.spi.Name; @@ -61,9 +66,11 @@ public final class JsonORMConverterImpl implements ORMConverter { private final RecordField field; private final TypeReference typeReference; private final ObjectMapper mapper; + private final ObjectWriter writer; record CacheKey(@Nonnull Json json, - @Nullable Class sealedType, + @Nonnull List> sealedTypes, + @Nullable Class targetType, @Nullable Class> serializer, @Nullable Class> deserializer) {} @@ -73,9 +80,7 @@ public JsonORMConverterImpl(@Nonnull RecordField field, @Nonnull Json json) { this.field = requireNonNull(field, "field"); this.typeReference = requireNonNull(typeReference, "typeReference"); - var type = getRawType(typeReference.getType()) - .filter(Class::isSealed) - .orElse(null); + var sealedTypes = getSealedTypes(typeReference.getType()); // Check for custom serializer/deserializer annotations. var serializeAnnotation = field.getAnnotation(JsonSerialize.class); var deserializeAnnotation = field.getAnnotation(JsonDeserialize.class); @@ -87,8 +92,14 @@ public JsonORMConverterImpl(@Nonnull RecordField field, deserializeAnnotation != null && deserializeAnnotation.using() != JsonDeserializer.None.class ? (Class>) deserializeAnnotation.using() : null; + // Custom serializers are registered against the raw field type, so that type is part of the cache key + // whenever one is present; without it, fields of different types sharing a serializer class would share + // a mapper that only serves the first field's type. + Class targetType = serializerClass != null || deserializerClass != null + ? getRawType(typeReference.getType()).orElse(Object.class) + : null; this.mapper = OBJECT_MAPPER.getOrCompute( - new CacheKey(requireNonNull(json, "json"), type, serializerClass, deserializerClass), + new CacheKey(requireNonNull(json, "json"), sealedTypes, targetType, serializerClass, deserializerClass), () -> { var mapper = new ObjectMapper(); mapper.findAndRegisterModules(); @@ -98,8 +109,8 @@ public JsonORMConverterImpl(@Nonnull RecordField field, if (!json.failOnMissing()) { mapper.disable(FAIL_ON_MISSING_CREATOR_PROPERTIES); } - if (type != null) { - mapper.registerSubtypes(getPermittedSubtypes(type)); + for (var sealedType : sealedTypes) { + mapper.registerSubtypes(getPermittedSubtypes(sealedType)); } // Register StormModule with supplier for dynamic RefFactory resolution. mapper.registerModule(new StormModule(REF_FACTORY::get)); @@ -108,18 +119,16 @@ public JsonORMConverterImpl(@Nonnull RecordField field, var customModule = new SimpleModule(); if (serializerClass != null) { try { - Class fieldType = getRawType(typeReference.getType()).orElse(Object.class); JsonSerializer serializerInstance = serializerClass.getDeclaredConstructor().newInstance(); - customModule.addSerializer(fieldType, serializerInstance); + customModule.addSerializer((Class) targetType, serializerInstance); } catch (Exception e) { throw new RuntimeException("Failed to instantiate custom serializer: " + serializerClass, e); } } if (deserializerClass != null) { try { - Class fieldType = getRawType(typeReference.getType()).orElse(Object.class); JsonDeserializer deserializerInstance = deserializerClass.getDeclaredConstructor().newInstance(); - customModule.addDeserializer(fieldType, deserializerInstance); + customModule.addDeserializer((Class) targetType, deserializerInstance); } catch (Exception e) { throw new RuntimeException("Failed to instantiate custom deserializer: " + deserializerClass, e); } @@ -128,6 +137,9 @@ public JsonORMConverterImpl(@Nonnull RecordField field, } return mapper; }); + // Serialization carries the declared field type rather than the erased runtime type, so polymorphic + // values write the discriminator that reading with the same declared type expects. + this.writer = mapper.writerFor(typeReference); } private static Optional> getRawType(@Nonnull Type type) { @@ -140,6 +152,35 @@ private static Optional> getRawType(@Nonnull Type type) { } } + /** + * Collects the sealed classes appearing anywhere in the field's generic type, so that permitted subtypes are + * registered for container-typed fields such as {@code List} as well as top-level sealed fields. + */ + private static List> getSealedTypes(@Nonnull Type type) { + var sealedTypes = new LinkedHashSet>(); + collectSealedTypes(type, sealedTypes); + return List.copyOf(sealedTypes); + } + + private static void collectSealedTypes(@Nonnull Type type, @Nonnull Set> sealedTypes) { + if (type instanceof Class clazz) { + if (clazz.isSealed()) { + sealedTypes.add(clazz); + } + } else if (type instanceof ParameterizedType parameterizedType) { + collectSealedTypes(parameterizedType.getRawType(), sealedTypes); + for (Type typeArgument : parameterizedType.getActualTypeArguments()) { + collectSealedTypes(typeArgument, sealedTypes); + } + } else if (type instanceof GenericArrayType arrayType) { + collectSealedTypes(arrayType.getGenericComponentType(), sealedTypes); + } else if (type instanceof WildcardType wildcardType) { + for (Type bound : wildcardType.getUpperBounds()) { + collectSealedTypes(bound, sealedTypes); + } + } + } + private static NamedType[] getPermittedSubtypes(@Nonnull Class sealedClass) { return REFLECTION.getPermittedSubclasses(sealedClass).stream() .map(subclass -> { @@ -169,7 +210,7 @@ public List getColumns(@Nonnull NameResolver nameResolver) throws SqlTempl public List toDatabase(@Nullable Object record) throws SqlTemplateException { try { Object o = record == null ? null : REFLECTION.invoke(field, record); - return singletonList(o == null ? null : new JsonString(mapper.writeValueAsString(o))); + return singletonList(o == null ? null : new JsonString(writer.writeValueAsString(o))); } catch (Throwable e) { throw new SqlTemplateException(e); } diff --git a/storm-jackson2/src/test/java/st/orm/jackson/JsonORMConverterImplTest.java b/storm-jackson2/src/test/java/st/orm/jackson/JsonORMConverterImplTest.java index 74fb484ce..d8a7ecfa8 100644 --- a/storm-jackson2/src/test/java/st/orm/jackson/JsonORMConverterImplTest.java +++ b/storm-jackson2/src/test/java/st/orm/jackson/JsonORMConverterImplTest.java @@ -20,6 +20,8 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import java.util.List; +import java.util.Map; import javax.sql.DataSource; import lombok.Builder; import org.junit.jupiter.api.Test; @@ -323,4 +325,128 @@ public void refDeserializedAfterNestedConversionShouldRemainAttached() { assertTrue(ownerRef.isFetchable()); assertEquals("Betty", ownerRef.fetch().firstName()); } + + // Two @Json fields of different types sharing one custom serializer class. + + public static class TypeNameMarkerSerializer extends JsonSerializer { + @Override + public void serialize(Object value, JsonGenerator gen, SerializerProvider serializers) + throws java.io.IOException { + gen.writeString("marker:" + value.getClass().getSimpleName()); + } + } + + public record Phone(String number) {} + + @Builder(toBuilder = true) + @DbTable("owner") + public record OwnerWithSharedSerializer( + @PK Integer id, + @Nonnull String firstName, + @Nonnull String lastName, + @Nonnull @Json @JsonSerialize(using = TypeNameMarkerSerializer.class) Address address, + @Nonnull @Json @JsonSerialize(using = TypeNameMarkerSerializer.class) Phone telephone + ) implements Entity {} + + public record RawOwnerRow(String address, String telephone) {} + + @Test + public void fieldsOfDifferentTypesSharingSerializerClassShouldEachUseTheSerializer() { + // Both fields use the same serializer class but have different types, so each field needs a mapper with + // the serializer registered for its own type. + var orm = of(dataSource); + var repository = orm.entity(OwnerWithSharedSerializer.class); + var owner = OwnerWithSharedSerializer.builder() + .firstName("Shared") + .lastName("Serializer") + .address(new Address("1 Way", "Town")) + .telephone(new Phone("555")) + .build(); + repository.insert(owner); + var row = orm.query("SELECT address, telephone FROM owner WHERE first_name = 'Shared'") + .getSingleResult(RawOwnerRow.class); + assertEquals("\"marker:Address\"", row.address()); + assertEquals("\"marker:Phone\"", row.telephone()); + } + + // Sealed element type inside a container. + + @Builder(toBuilder = true) + @DbTable("owner") + public record OwnerWithPolymorphicPersonList( + @PK Integer id, + @Nonnull @Json List address, + @Nullable String telephone + ) implements Entity {} + + @Test + public void polymorphicJsonListShouldResolveSubtypesViaDiscriminator() { + // The sealed interface appears as the List element type; its permitted subtypes are registered from the + // generic type, not just the raw List type. + var orm = of(dataSource); + var query = orm.query(""" + SELECT id, + '[{"@type":"PersonA","firstName":"Jane","lastName":"Doe"},{"@type":"PersonB","firstName":"John","lastName":"Doe"}]' AS address, + telephone + FROM owner WHERE id = 1"""); + var result = query.getSingleResult(OwnerWithPolymorphicPersonList.class); + assertEquals(2, result.address().size()); + assertTrue(result.address().get(0) instanceof PersonA); + assertTrue(result.address().get(1) instanceof PersonB); + } + + @Test + public void polymorphicJsonListShouldRoundTripThroughDatabase() { + var orm = of(dataSource); + var repository = orm.entity(OwnerWithPolymorphicPersonList.class); + var owner = OwnerWithPolymorphicPersonList.builder() + .address(List.of(new PersonA("Jane", "Doe"), new PersonB("John", "Doe"))) + .telephone("555") + .build(); + var inserted = repository.insertAndFetch(owner); + assertEquals(List.of(new PersonA("Jane", "Doe"), new PersonB("John", "Doe")), inserted.address()); + } + + @Builder(toBuilder = true) + @DbTable("owner") + public record OwnerWithPolymorphicAddress( + @PK Integer id, + @Nonnull @Json PolymorphicPerson address, + @Nullable String telephone + ) implements Entity {} + + @Test + public void polymorphicJsonFieldShouldRoundTripThroughDatabase() { + var orm = of(dataSource); + var repository = orm.entity(OwnerWithPolymorphicAddress.class); + var owner = OwnerWithPolymorphicAddress.builder() + .address(new PersonB("Jane", "Doe")) + .telephone("555") + .build(); + var inserted = repository.insertAndFetch(owner); + assertEquals(new PersonB("Jane", "Doe"), inserted.address()); + } + + @Builder(toBuilder = true) + @DbTable("owner") + public record OwnerWithPolymorphicPersonMap( + @PK Integer id, + @Nonnull @Json Map address, + @Nullable String telephone + ) implements Entity {} + + @Test + public void polymorphicJsonMapValuesShouldRoundTripThroughDatabase() { + var orm = of(dataSource); + var repository = orm.entity(OwnerWithPolymorphicPersonMap.class); + var persons = Map.of( + "primary", (PolymorphicPerson) new PersonA("Jane", "Doe"), + "secondary", new PersonB("John", "Doe")); + var owner = OwnerWithPolymorphicPersonMap.builder() + .address(persons) + .telephone("555") + .build(); + var inserted = repository.insertAndFetch(owner); + assertEquals(persons, inserted.address()); + } } diff --git a/storm-jackson2/src/test/kotlin/st/orm/jackson/JsonORMConverterKotlinInteropTest.kt b/storm-jackson2/src/test/kotlin/st/orm/jackson/JsonORMConverterKotlinInteropTest.kt new file mode 100644 index 000000000..62e8bee03 --- /dev/null +++ b/storm-jackson2/src/test/kotlin/st/orm/jackson/JsonORMConverterKotlinInteropTest.kt @@ -0,0 +1,204 @@ +/* + * Copyright 2024 - 2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package st.orm.jackson + +import com.fasterxml.jackson.annotation.JsonTypeInfo +import com.fasterxml.jackson.annotation.JsonTypeInfo.Id.NAME +import com.fasterxml.jackson.core.JsonGenerator +import com.fasterxml.jackson.core.JsonParser +import com.fasterxml.jackson.databind.DeserializationContext +import com.fasterxml.jackson.databind.JsonDeserializer +import com.fasterxml.jackson.databind.JsonSerializer +import com.fasterxml.jackson.databind.SerializerProvider +import com.fasterxml.jackson.databind.annotation.JsonDeserialize +import com.fasterxml.jackson.databind.annotation.JsonSerialize +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest +import org.springframework.test.annotation.DirtiesContext +import org.springframework.test.context.ContextConfiguration +import org.springframework.test.context.junit.jupiter.SpringExtension +import st.orm.DbTable +import st.orm.Entity +import st.orm.Json +import st.orm.PK +import st.orm.core.spi.Providers +import st.orm.core.template.ORMTemplate.of +import st.orm.core.template.impl.BindHint +import javax.sql.DataSource + +/** + * Runs the Jackson converter in the configuration every Kotlin application has: storm-kotlin on the class path, + * so its reflection provider builds the field metadata. Covers the language seams the Java suite cannot reach: + * annotations placed on Kotlin constructor properties, Kotlin sealed hierarchies in the sealed-type walk, and + * Java sealed hierarchies enumerated through Kotlin reflection. + */ +@ExtendWith(SpringExtension::class) +@ContextConfiguration(classes = [IntegrationConfig::class]) +@DataJpaTest(showSql = false) +@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) +open class JsonORMConverterKotlinInteropTest { + + @Autowired + private lateinit var dataSource: DataSource + + data class RawOwnerRow(val address: String?, val telephone: String?) + + @Test + fun `kotlin reflection provider is active`() { + // The interop premise: with storm-kotlin on the class path, its provider outranks the default one. + assertEquals("st.orm.spi.ORMReflectionImpl", Providers.getORMReflection().javaClass.name) + } + + // Custom serializer/deserializer annotations on a Kotlin constructor property. + + data class KotlinAddress(val address: String, val city: String) + + class PipeAddressSerializer : JsonSerializer() { + override fun serialize(value: KotlinAddress, gen: JsonGenerator, serializers: SerializerProvider) { + gen.writeString("${value.address} | ${value.city}") + } + } + + class PipeAddressDeserializer : JsonDeserializer() { + override fun deserialize(parser: JsonParser, context: DeserializationContext): KotlinAddress { + val parts = parser.text.split(" | ") + return KotlinAddress(parts[0], parts[1]) + } + } + + @DbTable("owner") + data class OwnerWithCustomSerde( + @PK val id: Int = 0, + @Json + @JsonSerialize(using = PipeAddressSerializer::class) + @JsonDeserialize(using = PipeAddressDeserializer::class) + val address: KotlinAddress, + val telephone: String? = null, + ) : Entity + + @Test + fun `custom serde on constructor property should be honored`() { + // The annotations land on the constructor parameter, the default Kotlin use-site target for them. + val orm = of(dataSource) + val repository = orm.entity(OwnerWithCustomSerde::class.java) + val inserted = repository.insertAndFetch(OwnerWithCustomSerde(address = KotlinAddress("1 Way", "Town"))) + assertEquals(KotlinAddress("1 Way", "Town"), inserted.address) + // The raw column value proves the serializer engaged; a bypassed serializer/deserializer pair would + // round trip plain JSON and pass the assertion above. + val row = orm.query("SELECT address, telephone FROM owner WHERE id = ${inserted.id}") + .getSingleResult(RawOwnerRow::class.java) + assertEquals("\"1 Way | Town\"", row.address) + } + + // One serializer class shared by two fields of different Kotlin types. + + data class KotlinPhone(val number: String) + + class TypeNameMarkerSerializer : JsonSerializer() { + override fun serialize(value: Any, gen: JsonGenerator, serializers: SerializerProvider) { + gen.writeString("marker:${value.javaClass.simpleName}") + } + } + + @DbTable("owner") + data class OwnerWithSharedSerializer( + @PK val id: Int = 0, + @Json @JsonSerialize(using = TypeNameMarkerSerializer::class) val address: KotlinAddress, + @Json @JsonSerialize(using = TypeNameMarkerSerializer::class) val telephone: KotlinPhone, + ) : Entity + + @Test + fun `fields of different types sharing serializer class should each use the serializer`() { + val orm = of(dataSource) + val repository = orm.entity(OwnerWithSharedSerializer::class.java) + repository.insert(OwnerWithSharedSerializer(address = KotlinAddress("1 Way", "Town"), telephone = KotlinPhone("555"))) + val row = orm.query("SELECT address, telephone FROM owner WHERE address LIKE '%marker%'") + .getSingleResult(RawOwnerRow::class.java) + assertEquals("\"marker:KotlinAddress\"", row.address) + assertEquals("\"marker:KotlinPhone\"", row.telephone) + } + + // Kotlin sealed hierarchies, top-level and inside containers. + + @JsonTypeInfo(use = NAME) + sealed interface KotlinPerson + + data class KotlinPersonA(val firstName: String, val lastName: String) : KotlinPerson + + data class KotlinPersonB(val firstName: String, val lastName: String) : KotlinPerson + + @DbTable("owner") + data class OwnerWithSealedAddress( + @PK val id: Int = 0, + @Json val address: KotlinPerson, + val telephone: String? = null, + ) : Entity + + @Test + fun `kotlin sealed field should round trip through database`() { + val orm = of(dataSource) + val repository = orm.entity(OwnerWithSealedAddress::class.java) + val inserted = repository.insertAndFetch(OwnerWithSealedAddress(address = KotlinPersonB("Jane", "Doe"))) + assertEquals(KotlinPersonB("Jane", "Doe"), inserted.address) + } + + @Test + fun `kotlin sealed field should resolve subtype via discriminator`() { + val orm = of(dataSource) + val result = orm.query( + """ + SELECT id, + '{"@type":"KotlinPersonA","firstName":"Jane","lastName":"Doe"}' AS address, + telephone + FROM owner WHERE id = 1 + """.trimIndent(), + ).getSingleResult(OwnerWithSealedAddress::class.java) + assertTrue(result.address is KotlinPersonA) + } + + @DbTable("owner") + data class OwnerWithSealedList( + @PK val id: Int = 0, + @Json val address: List, + val telephone: String? = null, + ) : Entity + + @Test + fun `kotlin sealed list field should round trip through database`() { + // The sealed interface appears as the List element type; the walk registers its subtypes from the + // generic type the Kotlin reflection provider reports. + val orm = of(dataSource) + val repository = orm.entity(OwnerWithSealedList::class.java) + val persons = listOf(KotlinPersonA("Jane", "Doe"), KotlinPersonB("John", "Doe")) + val inserted = repository.insertAndFetch(OwnerWithSealedList(address = persons)) + assertEquals(persons, inserted.address) + } + + // Java sealed hierarchies enumerated through Kotlin reflection. + + @Test + fun `java sealed types enumerate through kotlin reflection`() { + // BindHint is a Java sealed interface from storm-core; the Kotlin provider resolves its permitted + // subclasses through KClass.sealedSubclasses, which must match Java reflection. + val permittedSubclasses = Providers.getORMReflection().getPermittedSubclasses(BindHint::class.java) + assertEquals(BindHint::class.java.permittedSubclasses.toSet(), permittedSubclasses.toSet()) + assertTrue(permittedSubclasses.isNotEmpty()) + } +} diff --git a/storm-jackson3/src/main/java/st/orm/jackson/spi/JsonORMConverterImpl.java b/storm-jackson3/src/main/java/st/orm/jackson/spi/JsonORMConverterImpl.java index 78b61e79b..1e142716f 100644 --- a/storm-jackson3/src/main/java/st/orm/jackson/spi/JsonORMConverterImpl.java +++ b/storm-jackson3/src/main/java/st/orm/jackson/spi/JsonORMConverterImpl.java @@ -24,10 +24,14 @@ import com.fasterxml.jackson.annotation.JsonTypeName; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import java.lang.reflect.GenericArrayType; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.lang.reflect.WildcardType; +import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import st.orm.Json; import st.orm.core.spi.JsonString; import st.orm.core.spi.Name; @@ -41,6 +45,7 @@ import st.orm.mapping.RecordField; import tools.jackson.core.JacksonException; import tools.jackson.core.type.TypeReference; +import tools.jackson.databind.ObjectWriter; import tools.jackson.databind.ValueDeserializer; import tools.jackson.databind.ValueSerializer; import tools.jackson.databind.annotation.JsonDeserialize; @@ -61,9 +66,11 @@ public final class JsonORMConverterImpl implements ORMConverter { private final RecordField field; private final TypeReference typeReference; private final JsonMapper mapper; + private final ObjectWriter writer; record CacheKey(@Nonnull Json json, - @Nullable Class sealedType, + @Nonnull List> sealedTypes, + @Nullable Class targetType, @Nullable Class> serializer, @Nullable Class> deserializer) {} @@ -73,9 +80,7 @@ public JsonORMConverterImpl(@Nonnull RecordField field, @Nonnull Json json) { this.field = requireNonNull(field, "field"); this.typeReference = requireNonNull(typeReference, "typeReference"); - var type = getRawType(typeReference.getType()) - .filter(Class::isSealed) - .orElse(null); + var sealedTypes = getSealedTypes(typeReference.getType()); // Check for custom serializer/deserializer annotations. var serializeAnnotation = field.getAnnotation(JsonSerialize.class); var deserializeAnnotation = field.getAnnotation(JsonDeserialize.class); @@ -87,8 +92,14 @@ public JsonORMConverterImpl(@Nonnull RecordField field, deserializeAnnotation != null && deserializeAnnotation.using() != ValueDeserializer.None.class ? (Class>) deserializeAnnotation.using() : null; + // Custom serializers are registered against the raw field type, so that type is part of the cache key + // whenever one is present; without it, fields of different types sharing a serializer class would share + // a mapper that only serves the first field's type. + Class targetType = serializerClass != null || deserializerClass != null + ? getRawType(typeReference.getType()).orElse(Object.class) + : null; this.mapper = MAPPER_CACHE.getOrCompute( - new CacheKey(requireNonNull(json, "json"), type, serializerClass, deserializerClass), + new CacheKey(requireNonNull(json, "json"), sealedTypes, targetType, serializerClass, deserializerClass), () -> { var builder = JsonMapper.builder(); builder.findAndAddModules(); @@ -98,8 +109,8 @@ public JsonORMConverterImpl(@Nonnull RecordField field, if (!json.failOnMissing()) { builder.disable(FAIL_ON_MISSING_CREATOR_PROPERTIES); } - if (type != null) { - builder.registerSubtypes(getPermittedSubtypes(type)); + for (var sealedType : sealedTypes) { + builder.registerSubtypes(getPermittedSubtypes(sealedType)); } // Register StormModule with supplier for dynamic RefFactory resolution. builder.addModule(new StormModule(REF_FACTORY::get)); @@ -108,18 +119,16 @@ public JsonORMConverterImpl(@Nonnull RecordField field, var customModule = new SimpleModule(); if (serializerClass != null) { try { - Class fieldType = getRawType(typeReference.getType()).orElse(Object.class); ValueSerializer serializerInstance = serializerClass.getDeclaredConstructor().newInstance(); - customModule.addSerializer(fieldType, serializerInstance); + customModule.addSerializer((Class) targetType, serializerInstance); } catch (Exception e) { throw new RuntimeException("Failed to instantiate custom serializer: " + serializerClass, e); } } if (deserializerClass != null) { try { - Class fieldType = getRawType(typeReference.getType()).orElse(Object.class); ValueDeserializer deserializerInstance = deserializerClass.getDeclaredConstructor().newInstance(); - customModule.addDeserializer(fieldType, deserializerInstance); + customModule.addDeserializer((Class) targetType, deserializerInstance); } catch (Exception e) { throw new RuntimeException("Failed to instantiate custom deserializer: " + deserializerClass, e); } @@ -128,6 +137,9 @@ public JsonORMConverterImpl(@Nonnull RecordField field, } return builder.build(); }); + // Serialization carries the declared field type rather than the erased runtime type, so polymorphic + // values write the discriminator that reading with the same declared type expects. + this.writer = mapper.writerFor(typeReference); } private static Optional> getRawType(@Nonnull Type type) { @@ -140,6 +152,35 @@ private static Optional> getRawType(@Nonnull Type type) { } } + /** + * Collects the sealed classes appearing anywhere in the field's generic type, so that permitted subtypes are + * registered for container-typed fields such as {@code List} as well as top-level sealed fields. + */ + private static List> getSealedTypes(@Nonnull Type type) { + var sealedTypes = new LinkedHashSet>(); + collectSealedTypes(type, sealedTypes); + return List.copyOf(sealedTypes); + } + + private static void collectSealedTypes(@Nonnull Type type, @Nonnull Set> sealedTypes) { + if (type instanceof Class clazz) { + if (clazz.isSealed()) { + sealedTypes.add(clazz); + } + } else if (type instanceof ParameterizedType parameterizedType) { + collectSealedTypes(parameterizedType.getRawType(), sealedTypes); + for (Type typeArgument : parameterizedType.getActualTypeArguments()) { + collectSealedTypes(typeArgument, sealedTypes); + } + } else if (type instanceof GenericArrayType arrayType) { + collectSealedTypes(arrayType.getGenericComponentType(), sealedTypes); + } else if (type instanceof WildcardType wildcardType) { + for (Type bound : wildcardType.getUpperBounds()) { + collectSealedTypes(bound, sealedTypes); + } + } + } + private static NamedType[] getPermittedSubtypes(@Nonnull Class sealedClass) { return REFLECTION.getPermittedSubclasses(sealedClass).stream() .map(subclass -> { @@ -169,7 +210,7 @@ public List getColumns(@Nonnull NameResolver nameResolver) throws SqlTempl public List toDatabase(@Nullable Object record) throws SqlTemplateException { try { Object o = record == null ? null : REFLECTION.invoke(field, record); - return singletonList(o == null ? null : new JsonString(mapper.writeValueAsString(o))); + return singletonList(o == null ? null : new JsonString(writer.writeValueAsString(o))); } catch (Throwable e) { throw new SqlTemplateException(e); } diff --git a/storm-jackson3/src/test/java/st/orm/jackson/JsonORMConverterIntegrationTest.java b/storm-jackson3/src/test/java/st/orm/jackson/JsonORMConverterIntegrationTest.java index 66b9d2e5d..24caa882c 100644 --- a/storm-jackson3/src/test/java/st/orm/jackson/JsonORMConverterIntegrationTest.java +++ b/storm-jackson3/src/test/java/st/orm/jackson/JsonORMConverterIntegrationTest.java @@ -503,4 +503,137 @@ public void refDeserializedAfterNestedConversionShouldRemainAttached() { assertTrue(ownerRef.isFetchable()); assertEquals("Betty", ownerRef.fetch().firstName()); } + + // Two @Json fields of different types sharing one custom serializer class. + + public static class TypeNameMarkerSerializer extends tools.jackson.databind.ser.std.StdSerializer { + public TypeNameMarkerSerializer() { + super(Object.class); + } + + @Override + public void serialize(Object value, tools.jackson.core.JsonGenerator gen, + tools.jackson.databind.SerializationContext ctxt) + throws tools.jackson.core.JacksonException { + gen.writeString("marker:" + value.getClass().getSimpleName()); + } + } + + public record Phone(String number) {} + + @Builder(toBuilder = true) + @DbTable("owner") + public record OwnerWithSharedSerializer( + @PK Integer id, + @Nonnull String firstName, + @Nonnull String lastName, + @Nonnull @Json + @tools.jackson.databind.annotation.JsonSerialize(using = TypeNameMarkerSerializer.class) + Address address, + @Nonnull @Json + @tools.jackson.databind.annotation.JsonSerialize(using = TypeNameMarkerSerializer.class) + Phone telephone + ) implements Entity {} + + public record RawOwnerRow(String address, String telephone) {} + + @Test + public void fieldsOfDifferentTypesSharingSerializerClassShouldEachUseTheSerializer() { + // Both fields use the same serializer class but have different types, so each field needs a mapper with + // the serializer registered for its own type. + var orm = of(dataSource); + var repository = orm.entity(OwnerWithSharedSerializer.class); + var owner = OwnerWithSharedSerializer.builder() + .firstName("Shared") + .lastName("Serializer") + .address(new Address("1 Way", "Town")) + .telephone(new Phone("555")) + .build(); + repository.insert(owner); + var row = orm.query("SELECT address, telephone FROM owner WHERE first_name = 'Shared'") + .getSingleResult(RawOwnerRow.class); + assertEquals("\"marker:Address\"", row.address()); + assertEquals("\"marker:Phone\"", row.telephone()); + } + + // Sealed element type inside a container. + + @Builder(toBuilder = true) + @DbTable("owner") + public record OwnerWithPolymorphicPersonList( + @PK Integer id, + @Nonnull @Json List address, + @Nullable String telephone + ) implements Entity {} + + @Test + public void polymorphicJsonListShouldResolveSubtypesViaDiscriminator() { + // The sealed interface appears as the List element type; its permitted subtypes are registered from the + // generic type, not just the raw List type. + var orm = of(dataSource); + var query = orm.query(""" + SELECT id, + '[{"@type":"PersonA","firstName":"Jane","lastName":"Doe"},{"@type":"PersonB","firstName":"John","lastName":"Doe"}]' AS address, + telephone + FROM owner WHERE id = 1"""); + var result = query.getSingleResult(OwnerWithPolymorphicPersonList.class); + assertEquals(2, result.address().size()); + assertTrue(result.address().get(0) instanceof PersonA); + assertTrue(result.address().get(1) instanceof PersonB); + } + + @Test + public void polymorphicJsonListShouldRoundTripThroughDatabase() { + var orm = of(dataSource); + var repository = orm.entity(OwnerWithPolymorphicPersonList.class); + var owner = OwnerWithPolymorphicPersonList.builder() + .address(List.of(new PersonA("Jane", "Doe"), new PersonB("John", "Doe"))) + .telephone("555") + .build(); + var inserted = repository.insertAndFetch(owner); + assertEquals(List.of(new PersonA("Jane", "Doe"), new PersonB("John", "Doe")), inserted.address()); + } + + @Builder(toBuilder = true) + @DbTable("owner") + public record OwnerWithPolymorphicAddress( + @PK Integer id, + @Nonnull @Json PolymorphicPerson address, + @Nullable String telephone + ) implements Entity {} + + @Test + public void polymorphicJsonFieldShouldRoundTripThroughDatabase() { + var orm = of(dataSource); + var repository = orm.entity(OwnerWithPolymorphicAddress.class); + var owner = OwnerWithPolymorphicAddress.builder() + .address(new PersonB("Jane", "Doe")) + .telephone("555") + .build(); + var inserted = repository.insertAndFetch(owner); + assertEquals(new PersonB("Jane", "Doe"), inserted.address()); + } + + @Builder(toBuilder = true) + @DbTable("owner") + public record OwnerWithPolymorphicPersonMap( + @PK Integer id, + @Nonnull @Json Map address, + @Nullable String telephone + ) implements Entity {} + + @Test + public void polymorphicJsonMapValuesShouldRoundTripThroughDatabase() { + var orm = of(dataSource); + var repository = orm.entity(OwnerWithPolymorphicPersonMap.class); + var persons = Map.of( + "primary", (PolymorphicPerson) new PersonA("Jane", "Doe"), + "secondary", new PersonB("John", "Doe")); + var owner = OwnerWithPolymorphicPersonMap.builder() + .address(persons) + .telephone("555") + .build(); + var inserted = repository.insertAndFetch(owner); + assertEquals(persons, inserted.address()); + } } diff --git a/storm-kotlinx-serialization/src/main/kotlin/st/orm/serialization/spi/JsonORMConverterImpl.kt b/storm-kotlinx-serialization/src/main/kotlin/st/orm/serialization/spi/JsonORMConverterImpl.kt index c270ff22b..dceadd5a2 100644 --- a/storm-kotlinx-serialization/src/main/kotlin/st/orm/serialization/spi/JsonORMConverterImpl.kt +++ b/storm-kotlinx-serialization/src/main/kotlin/st/orm/serialization/spi/JsonORMConverterImpl.kt @@ -34,7 +34,6 @@ import java.util.concurrent.ConcurrentHashMap import kotlin.reflect.KClass import kotlin.reflect.KType import kotlin.reflect.full.createInstance -import kotlin.reflect.jvm.jvmErasure import kotlinx.serialization.json.Json as JsonMapper internal class JsonORMConverterImpl( @@ -45,13 +44,10 @@ internal class JsonORMConverterImpl( companion object { private val REFLECTION: ORMReflection = Providers.getORMReflection() - private val JSON_CACHE = ConcurrentHashMap() - private val REF_FACTORY = ThreadLocal() - private data class CacheKey( - val sealedBase: Class<*>?, - val json: Json, - ) + // The mapper depends only on the Json flags, so the cache holds at most one entry per flag combination. + private val JSON_CACHE = ConcurrentHashMap() + private val REF_FACTORY = ThreadLocal() private fun buildJson(json: Json): JsonMapper = JsonMapper { serializersModule = StormSerializersModule { REF_FACTORY.get() } @@ -156,10 +152,7 @@ internal class JsonORMConverterImpl( private val serializer: KSerializer init { - val sealedBase = kType.jvmErasure.java.takeIf { it.isSealed } - this.json = JSON_CACHE.computeIfAbsent(CacheKey(sealedBase, json)) { key -> - buildJson(key.json) - } + this.json = JSON_CACHE.computeIfAbsent(json) { buildJson(it) } this.serializer = try { createSerializer(this@JsonORMConverterImpl.json, field, kType) } catch (e: SerializationException) {