Remove specialization-related conditional logic from PojoCodecImpl by introducing LazyPropertyModelCodec.NeedsSpecializationCodec (#1136)

stIncMale · web-flow · commit 14cfcdca70e3 · 2023-06-09T08:57:13.000-06:00
This commit moves some of the conditional logic from `PojoCodecImpl` under the rug, thus allegedly simplifying `PojoCodecImpl`, but not the POJO codec implementation overall. JAVA-4954
diff --git a/bson/src/main/org/bson/codecs/pojo/LazyPropertyModelCodec.java b/bson/src/main/org/bson/codecs/pojo/LazyPropertyModelCodec.java
@@ -28,22 +28,21 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import static java.lang.String.format;
 import static org.bson.codecs.pojo.PojoSpecializationHelper.specializeTypeData;
 
 class LazyPropertyModelCodec<T> implements Codec<T> {
     private final PropertyModel<T> propertyModel;
     private final CodecRegistry registry;
     private final PropertyCodecRegistry propertyCodecRegistry;
-    private final DiscriminatorLookup discriminatorLookup;
 
     private volatile Codec<T> codec;
 
     LazyPropertyModelCodec(final PropertyModel<T> propertyModel, final CodecRegistry registry,
-                                  final PropertyCodecRegistry propertyCodecRegistry, final DiscriminatorLookup discriminatorLookup) {
+            final PropertyCodecRegistry propertyCodecRegistry) {
         this.propertyModel = propertyModel;
         this.registry = registry;
         this.propertyCodecRegistry = propertyCodecRegistry;
-        this.discriminatorLookup = discriminatorLookup;
     }
 
     @Override
@@ -77,7 +76,7 @@ private Codec<T> createCodec() {
         if (localCodec instanceof PojoCodec) {
             PojoCodec<T> pojoCodec = (PojoCodec<T>) localCodec;
             ClassModel<T> specialized = getSpecializedClassModel(pojoCodec.getClassModel(), propertyModel);
-            localCodec = new PojoCodecImpl<>(specialized, registry, propertyCodecRegistry, pojoCodec.getDiscriminatorLookup(), true);
+            localCodec = new PojoCodecImpl<>(specialized, registry, propertyCodecRegistry, pojoCodec.getDiscriminatorLookup());
         }
         return localCodec;
     }
@@ -150,4 +149,46 @@ private <V> PropertyModel<V> getSpecializedPropertyModel(final PropertyModel<V>
                 propertyModel.getPropertyAccessor(), propertyModel.getError(), propertyModel.getBsonRepresentation());
     }
 
+    /**
+     * Instances of this codec are supposed to be replaced with usable implementations by {@link LazyPropertyModelCodec#createCodec()}.
+     */
+    static final class NeedSpecializationCodec<T> extends PojoCodec<T> {
+        private final ClassModel<T> classModel;
+        private final DiscriminatorLookup discriminatorLookup;
+
+        NeedSpecializationCodec(final ClassModel<T> classModel, final DiscriminatorLookup discriminatorLookup) {
+            this.classModel = classModel;
+            this.discriminatorLookup = discriminatorLookup;
+        }
+
+        @Override
+        public T decode(final BsonReader reader, final DecoderContext decoderContext) {
+            throw exception();
+        }
+
+        @Override
+        public void encode(final BsonWriter writer, final T value, final EncoderContext encoderContext) {
+            throw exception();
+        }
+
+        @Override
+        public Class<T> getEncoderClass() {
+            return classModel.getType();
+        }
+
+        private CodecConfigurationException exception() {
+            return new CodecConfigurationException(format("%s contains generic types that have not been specialised.%n"
+                    + "Top level classes with generic types are not supported by the PojoCodec.", classModel.getName()));
+        }
+
+        @Override
+        ClassModel<T> getClassModel() {
+            return classModel;
+        }
+
+        @Override
+        DiscriminatorLookup getDiscriminatorLookup() {
+            return discriminatorLookup;
+        }
+    }
 }
diff --git a/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java b/bson/src/main/org/bson/codecs/pojo/PojoCodecImpl.java
@@ -50,36 +50,28 @@ final class PojoCodecImpl<T> extends PojoCodec<T> {
     private final CodecRegistry registry;
     private final PropertyCodecRegistry propertyCodecRegistry;
     private final DiscriminatorLookup discriminatorLookup;
-    private final boolean specialized;
 
     PojoCodecImpl(final ClassModel<T> classModel, final CodecRegistry codecRegistry,
-                  final List<PropertyCodecProvider> propertyCodecProviders, final DiscriminatorLookup discriminatorLookup) {
+            final List<PropertyCodecProvider> propertyCodecProviders, final DiscriminatorLookup discriminatorLookup) {
         this.classModel = classModel;
         this.registry = codecRegistry;
         this.discriminatorLookup = discriminatorLookup;
         this.propertyCodecRegistry = new PropertyCodecRegistryImpl(this, registry, propertyCodecProviders);
-        this.specialized = shouldSpecialize(classModel);
         specialize();
     }
 
-    PojoCodecImpl(final ClassModel<T> classModel, final CodecRegistry codecRegistry, final PropertyCodecRegistry propertyCodecRegistry,
-                  final DiscriminatorLookup discriminatorLookup, final boolean specialized) {
+    PojoCodecImpl(final ClassModel<T> classModel, final CodecRegistry codecRegistry,
+            final PropertyCodecRegistry propertyCodecRegistry, final DiscriminatorLookup discriminatorLookup) {
         this.classModel = classModel;
         this.registry = codecRegistry;
         this.discriminatorLookup = discriminatorLookup;
         this.propertyCodecRegistry = propertyCodecRegistry;
-        this.specialized = specialized;
         specialize();
     }
 
     @SuppressWarnings("unchecked")
     @Override
     public void encode(final BsonWriter writer, final T value, final EncoderContext encoderContext) {
-        if (!specialized) {
-            throw new CodecConfigurationException(format("%s contains generic types that have not been specialised.%n"
-                            + "Top level classes with generic types are not supported by the PojoCodec.", classModel.getName()));
-        }
-
         if (areEquivalentTypes(value.getClass(), classModel.getType())) {
             writer.writeStartDocument();
 
@@ -104,10 +96,6 @@ public void encode(final BsonWriter writer, final T value, final EncoderContext
     @Override
     public T decode(final BsonReader reader, final DecoderContext decoderContext) {
         if (decoderContext.hasCheckedDiscriminator()) {
-            if (!specialized) {
-                throw new CodecConfigurationException(format("%s contains generic types that have not been specialised.%n"
-                        + "Top level classes with generic types are not supported by the PojoCodec.", classModel.getName()));
-            }
             InstanceCreator<T> instanceCreator = classModel.getInstanceCreator();
             decodeProperties(reader, decoderContext, instanceCreator);
             return instanceCreator.getInstance();
@@ -264,15 +252,13 @@ private <S> void setPropertyValueBsonExtraElements(final InstanceCreator<T> inst
     }
 
     private void specialize() {
-        if (specialized) {
-            classModel.getPropertyModels().forEach(this::cachePropertyModelCodec);
-        }
+        classModel.getPropertyModels().forEach(this::cachePropertyModelCodec);
     }
 
     private <S> void cachePropertyModelCodec(final PropertyModel<S> propertyModel) {
         if (propertyModel.getCachedCodec() == null) {
             Codec<S> codec = propertyModel.getCodec() != null ? propertyModel.getCodec()
-                    : new LazyPropertyModelCodec<>(propertyModel, registry, propertyCodecRegistry, discriminatorLookup);
+                    : new LazyPropertyModelCodec<>(propertyModel, registry, propertyCodecRegistry);
             propertyModel.cachedCodec(codec);
         }
     }
@@ -328,21 +314,6 @@ private PropertyModel<?> getPropertyModelByWriteName(final ClassModel<T> classMo
         return null;
     }
 
-    private static <T> boolean shouldSpecialize(final ClassModel<T> classModel) {
-        if (!classModel.hasTypeParameters()) {
-            return true;
-        }
-
-        for (Map.Entry<String, TypeParameterMap> entry : classModel.getPropertyNameToTypeParameterMap().entrySet()) {
-            TypeParameterMap typeParameterMap = entry.getValue();
-            PropertyModel<?> propertyModel = classModel.getPropertyModel(entry.getKey());
-            if (typeParameterMap.hasTypeParameters() && (propertyModel == null || propertyModel.getCodec() == null)) {
-                return false;
-            }
-        }
-        return true;
-    }
-
     @Override
     DiscriminatorLookup getDiscriminatorLookup() {
         return discriminatorLookup;
diff --git a/bson/src/main/org/bson/codecs/pojo/PojoCodecProvider.java b/bson/src/main/org/bson/codecs/pojo/PojoCodecProvider.java
@@ -69,20 +69,20 @@ public static Builder builder() {
 
     @Override
     public <T> Codec<T> get(final Class<T> clazz, final CodecRegistry registry) {
-        return getPojoCodec(clazz, registry);
+        return createCodec(clazz, registry);
     }
 
     @SuppressWarnings("unchecked")
-    private <T> PojoCodec<T> getPojoCodec(final Class<T> clazz, final CodecRegistry registry) {
+    private <T> PojoCodec<T> createCodec(final Class<T> clazz, final CodecRegistry registry) {
         ClassModel<T> classModel = (ClassModel<T>) classModels.get(clazz);
         if (classModel != null) {
-            return new PojoCodecImpl<>(classModel, registry, propertyCodecProviders, discriminatorLookup);
+            return createCodec(classModel, registry, propertyCodecProviders, discriminatorLookup);
         } else if (automatic || (clazz.getPackage() != null && packages.contains(clazz.getPackage().getName()))) {
             try {
                 classModel = createClassModel(clazz, conventions);
                 if (clazz.isInterface() || !classModel.getPropertyModels().isEmpty()) {
                     discriminatorLookup.addClassModel(classModel);
-                    return new AutomaticPojoCodec<>(new PojoCodecImpl<>(classModel, registry, propertyCodecProviders,
+                    return new AutomaticPojoCodec<>(createCodec(classModel, registry, propertyCodecProviders,
                             discriminatorLookup));
                 }
             } catch (Exception e) {
@@ -93,6 +93,13 @@ private <T> PojoCodec<T> getPojoCodec(final Class<T> clazz, final CodecRegistry
         return null;
     }
 
+    private static <T> PojoCodec<T> createCodec(final ClassModel<T> classModel, final CodecRegistry codecRegistry,
+            final List<PropertyCodecProvider> propertyCodecProviders, final DiscriminatorLookup discriminatorLookup) {
+        return shouldSpecialize(classModel)
+                ? new PojoCodecImpl<>(classModel, codecRegistry, propertyCodecProviders, discriminatorLookup)
+                : new LazyPropertyModelCodec.NeedSpecializationCodec<>(classModel, discriminatorLookup);
+    }
+
     /**
      * A Builder for the PojoCodecProvider
      */
@@ -218,4 +225,19 @@ private static <T> ClassModel<T> createClassModel(final Class<T> clazz, final Li
         }
         return builder.build();
     }
+
+    private static boolean shouldSpecialize(final ClassModel<?> classModel) {
+        if (!classModel.hasTypeParameters()) {
+            return true;
+        }
+
+        for (Map.Entry<String, TypeParameterMap> entry : classModel.getPropertyNameToTypeParameterMap().entrySet()) {
+            TypeParameterMap typeParameterMap = entry.getValue();
+            PropertyModel<?> propertyModel = classModel.getPropertyModel(entry.getKey());
+            if (typeParameterMap.hasTypeParameters() && (propertyModel == null || propertyModel.getCodec() == null)) {
+                return false;
+            }
+        }
+        return true;
+    }
 }
diff --git a/bson/src/test/unit/org/bson/codecs/pojo/PojoTestCase.java b/bson/src/test/unit/org/bson/codecs/pojo/PojoTestCase.java
@@ -176,11 +176,11 @@ static PojoCodecProvider.Builder getPojoCodecProviderBuilder(final Class<?>... c
         return builder;
     }
 
-    <T> PojoCodecImpl<T> getCodec(final PojoCodecProvider.Builder builder, final Class<T> clazz) {
-        return (PojoCodecImpl<T>) getCodecRegistry(builder).get(clazz);
+    <T> PojoCodec<T> getCodec(final PojoCodecProvider.Builder builder, final Class<T> clazz) {
+        return (PojoCodec<T>) getCodecRegistry(builder).get(clazz);
     }
 
-    <T> PojoCodecImpl<T> getCodec(final Class<T> clazz) {
+    <T> PojoCodec<T> getCodec(final Class<T> clazz) {
         return getCodec(getPojoCodecProviderBuilder(clazz), clazz);
     }
 

Original file line number	Diff line number	Diff line change
`@@ -176,11 +176,11 @@ static PojoCodecProvider.Builder getPojoCodecProviderBuilder(final Class<?>... c`
`176`	`176`	`return builder;`
`177`	`177`	`}`
`178`	`178`
`179`		`- <T> PojoCodecImpl<T> getCodec(final PojoCodecProvider.Builder builder, final Class<T> clazz) {`
`180`		`- return (PojoCodecImpl<T>) getCodecRegistry(builder).get(clazz);`
	`179`	`+ <T> PojoCodec<T> getCodec(final PojoCodecProvider.Builder builder, final Class<T> clazz) {`
	`180`	`+ return (PojoCodec<T>) getCodecRegistry(builder).get(clazz);`
`181`	`181`	`}`
`182`	`182`
`183`		`- <T> PojoCodecImpl<T> getCodec(final Class<T> clazz) {`
	`183`	`+ <T> PojoCodec<T> getCodec(final Class<T> clazz) {`
`184`	`184`	`return getCodec(getPojoCodecProviderBuilder(clazz), clazz);`
`185`	`185`	`}`
`186`	`186`