Skip to content

Deprecate Parameterizable, introduce default CodecProvider.get(Class<T>, List<Type>, CodecRegistry) instead #1115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import org.bson.BsonWriter
import org.bson.codecs.Codec
import org.bson.codecs.DecoderContext
import org.bson.codecs.EncoderContext
import org.bson.codecs.Parameterizable
import org.bson.codecs.RepresentationConfigurable
import org.bson.codecs.configuration.CodecConfigurationException
import org.bson.codecs.configuration.CodecRegistry
Expand Down Expand Up @@ -136,31 +135,22 @@ internal data class DataClassCodec<T : Any>(
): Codec<R>? {
return if (!kClass.isData) {
null
} else if (kClass.typeParameters.isNotEmpty()) {
RawDataClassCodec(kClass)
} else {
createDataClassCodec(kClass, codecRegistry, types)
validateAnnotations(kClass)
val primaryConstructor =
kClass.primaryConstructor ?: throw CodecConfigurationException("No primary constructor for $kClass")
val typeMap = types.mapIndexed { i, k -> primaryConstructor.typeParameters[i].createType() to k }
.toMap()

val propertyModels =
primaryConstructor.parameters.map { kParameter ->
PropertyModel(
kParameter, computeFieldName(kParameter), getCodec(kParameter, typeMap, codecRegistry))
}
return DataClassCodec(kClass, primaryConstructor, propertyModels)
}
}

private fun <R : Any> createDataClassCodec(
kClass: KClass<R>,
codecRegistry: CodecRegistry,
types: List<Type> = emptyList()
): Codec<R> {
validateAnnotations(kClass)
val primaryConstructor =
kClass.primaryConstructor ?: throw CodecConfigurationException("No primary constructor for $kClass")
val typeMap = types.mapIndexed { i, k -> primaryConstructor.typeParameters[i].createType() to k }.toMap()

val propertyModels =
primaryConstructor.parameters.map { kParameter ->
PropertyModel(
kParameter, computeFieldName(kParameter), getCodec(kParameter, typeMap, codecRegistry))
}
return DataClassCodec(kClass, primaryConstructor, propertyModels)
}

private fun <R : Any> validateAnnotations(kClass: KClass<R>) {
codecConfigurationRequires(kClass.findAnnotation<BsonDiscriminator>() == null) {
"""Annotation 'BsonDiscriminator' is not supported on kotlin data classes,
Expand Down Expand Up @@ -251,29 +241,5 @@ internal data class DataClassCodec<T : Any>(
throw CodecConfigurationException(lazyMessage.invoke())
}
}

/**
* A Raw unparameterized data class
*
* It cannot encode or decode it just can create parameterized DataClassCodecs
*/
internal data class RawDataClassCodec<T : Any>(private val kClass: KClass<T>) : Codec<T>, Parameterizable {

override fun getEncoderClass(): Class<T> = kClass.java

override fun parameterize(codecRegistry: CodecRegistry, types: List<Type>): Codec<*> {
return createDataClassCodec(kClass, codecRegistry, types)
}

override fun decode(reader: BsonReader?, decoderContext: DecoderContext?): T {
throw CodecConfigurationException(
"Can not decode to ${kClass.simpleName} as it has type parameters and has not been parameterized.")
}

override fun encode(writer: BsonWriter?, value: T, encoderContext: EncoderContext?) {
throw CodecConfigurationException(
"Can not encode to ${kClass.simpleName} as it has type parameters and has not been parameterized.")
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@ package org.bson.codecs.kotlin
import org.bson.codecs.Codec
import org.bson.codecs.configuration.CodecProvider
import org.bson.codecs.configuration.CodecRegistry
import java.lang.reflect.Type

/** A Kotlin reflection based Codec Provider for data classes */
public class DataClassCodecProvider : CodecProvider {
override fun <T : Any> get(clazz: Class<T>, registry: CodecRegistry): Codec<T>? =
DataClassCodec.create(clazz.kotlin, registry)
get(clazz, emptyList(), registry)

override fun <T : Any> get(clazz: Class<T>, typeArguments: List<Type>, registry: CodecRegistry): Codec<T>? =
DataClassCodec.create(clazz.kotlin, registry, typeArguments)
Comment on lines 24 to +29
Copy link
Member Author

Choose a reason for hiding this comment

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

If this provider is rolled back to the previous implementation, some tests in DataClassCodecTest (for example, testDataClassWithParameterizedDataClass), which this PR did not modify, start failing with CodecConfigurationException: Could not find codec for number with type N. I.e., they do test parameterization and it appear to work when the provider properly overrides the new get method.

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,16 @@
package org.bson.codecs.kotlin

import com.mongodb.MongoClientSettings
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue
import org.bson.BsonDocument
import org.bson.BsonDocumentReader
import org.bson.BsonDocumentWriter
import org.bson.codecs.DecoderContext
import org.bson.codecs.EncoderContext
import org.bson.codecs.configuration.CodecConfigurationException
import org.bson.codecs.kotlin.samples.DataClassEmbedded
import org.bson.codecs.kotlin.samples.DataClassParameterized
import org.bson.codecs.kotlin.samples.DataClassWithParameterizedDataClass
import org.bson.codecs.kotlin.samples.DataClassWithSimpleValues
import org.bson.conversions.Bson
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue

class DataClassCodecProviderTest {

Expand All @@ -52,27 +45,9 @@ class DataClassCodecProviderTest {
}

@Test
fun shouldReturnRawDataClassCodecForParameterizedDataClass() {
val provider = DataClassCodecProvider()
val codec = provider.get(DataClassParameterized::class.java, Bson.DEFAULT_CODEC_REGISTRY)

assertNotNull(codec)
assertTrue { codec is DataClassCodec.Companion.RawDataClassCodec }
assertEquals(DataClassParameterized::class.java, codec.encoderClass)

assertThrows<CodecConfigurationException> {
val writer = BsonDocumentWriter(BsonDocument())
val dataClass =
DataClassWithParameterizedDataClass(
"myId", DataClassParameterized(2.0, "myString", listOf(DataClassEmbedded("embedded1"))))
codec.encode(writer, dataClass.parameterizedDataClass, EncoderContext.builder().build())
}

fun shouldRequireTypeArgumentsForDataClassParameterized() {
assertThrows<CodecConfigurationException> {
val value =
BsonDocument.parse(
"""{"number": 2.0, "string": "myString", "parameterizedList": [{"name": "embedded1"}]}""")
codec.decode(BsonDocumentReader(value), DecoderContext.builder().build())
DataClassCodecProvider().get(DataClassParameterized::class.java, Bson.DEFAULT_CODEC_REGISTRY)
}
}

Expand Down
40 changes: 3 additions & 37 deletions bson-record-codec/src/main/org/bson/codecs/record/RecordCodec.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.bson.codecs.Codec;
import org.bson.codecs.DecoderContext;
import org.bson.codecs.EncoderContext;
import org.bson.codecs.Parameterizable;
import org.bson.codecs.RepresentationConfigurable;
import org.bson.codecs.configuration.CodecConfigurationException;
import org.bson.codecs.configuration.CodecRegistry;
Expand Down Expand Up @@ -50,10 +49,9 @@
import static java.lang.String.format;
import static org.bson.assertions.Assertions.notNull;

final class RecordCodec<T extends Record> implements Codec<T>, Parameterizable {
final class RecordCodec<T extends Record> implements Codec<T> {
private static final Logger LOGGER = Loggers.getLogger("RecordCodec");
private final Class<T> clazz;
private final boolean requiresParameterization;
private final Constructor<?> canonicalConstructor;
private final List<ComponentModel> componentModels;
private final ComponentModel componentModelForId;
Expand Down Expand Up @@ -251,49 +249,21 @@ private static <T extends Annotation> void validateAnnotationOnlyOnField(final R
}
}

RecordCodec(final Class<T> clazz, final CodecRegistry codecRegistry) {
this.clazz = notNull("class", clazz);
if (clazz.getTypeParameters().length > 0) {
requiresParameterization = true;
canonicalConstructor = null;
componentModels = null;
fieldNameToComponentModel = null;
componentModelForId = null;
} else {
requiresParameterization = false;
canonicalConstructor = notNull("canonicalConstructor", getCanonicalConstructor(clazz));
componentModels = getComponentModels(clazz, codecRegistry, List.of());
fieldNameToComponentModel = componentModels.stream()
.collect(Collectors.toMap(ComponentModel::getFieldName, Function.identity()));
componentModelForId = getComponentModelForId(clazz, componentModels);
}
}

RecordCodec(final Class<T> clazz, final CodecRegistry codecRegistry, final List<Type> types) {
if (types.size() != clazz.getTypeParameters().length || types.isEmpty()) {
RecordCodec(final Class<T> clazz, final List<Type> types, final CodecRegistry codecRegistry) {
if (types.size() != clazz.getTypeParameters().length) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is so much nicer

throw new CodecConfigurationException("Unexpected number of type parameters for record class " + clazz);
}
this.clazz = notNull("class", clazz);
requiresParameterization = false;
canonicalConstructor = notNull("canonicalConstructor", getCanonicalConstructor(clazz));
componentModels = getComponentModels(clazz, codecRegistry, types);
fieldNameToComponentModel = componentModels.stream()
.collect(Collectors.toMap(ComponentModel::getFieldName, Function.identity()));
componentModelForId = getComponentModelForId(clazz, componentModels);
}

@Override
public Codec<?> parameterize(final CodecRegistry codecRegistry, final List<Type> types) {
return new RecordCodec<>(clazz, codecRegistry, types);
}

@SuppressWarnings("unchecked")
@Override
public T decode(final BsonReader reader, final DecoderContext decoderContext) {
if (requiresParameterization) {
throw new CodecConfigurationException("Can not decode to a record with type parameters that has not been parameterized");
}

reader.readStartDocument();

Object[] constructorArguments = new Object[componentModels.size()];
Expand All @@ -320,10 +290,6 @@ public T decode(final BsonReader reader, final DecoderContext decoderContext) {

@Override
public void encode(final BsonWriter writer, final T record, final EncoderContext encoderContext) {
if (requiresParameterization) {
throw new CodecConfigurationException("Can not decode to a record with type parameters that has not been parameterized");
}

writer.writeStartDocument();
if (componentModelForId != null) {
writeComponent(writer, record, componentModelForId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,30 @@
import org.bson.codecs.configuration.CodecProvider;
import org.bson.codecs.configuration.CodecRegistry;

import java.lang.reflect.Type;
import java.util.List;

import static org.bson.assertions.Assertions.assertNotNull;

/**
* Provides Codec instances for Java records.
*
* @since 4.6
* @see Record
*/
public final class RecordCodecProvider implements CodecProvider {
@SuppressWarnings({"unchecked", "rawtypes"})
@Override
public <T> Codec<T> get(final Class<T> clazz, final CodecRegistry registry) {
if (!clazz.isRecord()) {
return get(clazz, List.of(), registry);
}

@Override
public <T> Codec<T> get(final Class<T> clazz, final List<Type> typeArguments, final CodecRegistry registry) {
if (!assertNotNull(clazz).isRecord()) {
return null;
}

return (Codec<T>) new RecordCodec(clazz, registry);
@SuppressWarnings({"unchecked", "rawtypes"})
Codec<T> result = new RecordCodec(clazz, assertNotNull(typeArguments), registry);
return result;
}
}
Loading