Remove specialization-related conditional logic from PojoCodecImpl by introducing LazyPropertyModelCodec.NeedsSpecializationCodec

[stIncMale]

This PR moves some of the conditional logic from PojoCodecImpl under the rug, thus allegedly simplifying PojoCodecImpl, but not the POJO codec implementation overall.

I still have no idea about the following:

• What "specialization" means and why it exists?
• Why there is two "unusable" types codecs; previously it was LazyPropertyModelCodec and some instances of PojoCodecImpl, now its LazyPropertyModelCodec and all instances of NeedsSpecializationCodec?
• Why is it so happens that NeedsSpecializationCodec is always (I judge based on the existing tests) replaced by LazyPropertyModelCodec with a usable codec instance?

JAVA-4954

[stIncMale]

@jyemin I think that this change makes the code a bit simpler. Unfortunately, it does not result in having any new step towards drastically simplifying the POJO codec implementation.

[rozza]

Great questions - I had to dive through the code to understand it more.

• What "specialization" means and why it exists?

Originally, the CodecRegistry could only handle classes eg: registry.get(MyPojo.class). "specialization" just means setting the concrete types parameters for a POJO's property.

public final class NestedSelfReferentialGenericHolderModel {
    private NestedSelfReferentialGenericModel<Boolean, Long, Double> nested;
}

public final class NestedSelfReferentialGenericModel<T, V, Z> {
    private SelfReferentialGenericModel<T, V> selfRef1;
    private SelfReferentialGenericModel<T, Z> selfRef2;
}

public final class SelfReferentialGenericModel<T, V> {
    private T t;
    private V v;
    private SelfReferentialGenericModel<V, T> child;
}

With the above POJOs, we have enough data to model all the POJOs for the properties. However, we have some cyclical considerations as SelfReferentialGenericModel is self referential.
Specialization is also used to handle the SelfReferentialGenericModel codec and its appropriate types when set per property.

• Why there is two "unusable" types codecs; previously it was LazyPropertyModelCodec and some instances of PojoCodecImpl, now its LazyPropertyModelCodec and all instances of NeedsSpecializationCodec?

• LazyPropertyModelCodec just defers the handling of a property's generic types until its actually used to decode/encode data. Hence breaking any cycles.
• NeedsSpecializationCodec means we have no type data at that point in time. This happens when a POJO containing type parameters is used with codecRegistry.get(clazz).

• Why is it so happens that NeedsSpecializationCodec is always (I judge based on the existing tests) replaced by LazyPropertyModelCodec with a usable codec instance?

Nearly always ;) There are two causes; direct use like in testCannotEncodeUnspecializedClasses Or via the FallbackPropertyCodecProvider.
(This would be the place to actually call the registry with the TypeArguments as they are effectively lost after this point - we should open a ticket for that).

As NeedsSpecializationCodec is a PojoCodec the LazyPropertyModelCodec both creates and replaces it when calling createCodec - which has a code smell.

I like the PR - just a couple of comments

[rozza]

Looks like the constructors can now be chained - ie. this one calls the below.

[stIncMale]

I wish ... : one of them (PojoCodecImpl(final ClassModel<T> classModel, final CodecRegistry codecRegistry, final List<PropertyCodecProvider> propertyCodecProviders, final DiscriminatorLookup discriminatorLookup)) has to give this to a new PropertyCodecRegistry, which means that this constructor can neither be called from the other one, nor the other one can call this one.

As far as I can see, only refactoring of the code around PojoCodecImpl may open up opportunities to either calling one of the constructors from the other, or getting rid of one of them. But I don't even know if such refactoring is possible, and if yes, what it will look like.

[rozza]

Can this specialize() logic now be moved into the constructor?

[stIncMale]

Unless we want to duplicate a bunch of code, this can only be done if one of the constructors reuses the other one, which does not seem possible.

[rozza]

LGTM