Add a hook to intercept registration of all new serializers.

[chrisr3]

Create a mechanism for customising, configuring, or even replacing a serializer when it is registered.

See #943.

[theigl]

Hi @chrisr3. Do you need this change for your custom serializer registration to work, or is this just something that would be nice to have? Would overriding register be enough in your case?

[chrisr3]

Hi @chrisr3. Do you need this change for your custom serializer registration to work, or is this just something that would be nice to have? Would overriding register be enough in your case?

Something like this would simplify my integration. The problem with overriding Kryo.register(Registration) is that not everything uses it:

public Registration register (Class type, Serializer serializer) {
	Registration registration = classResolver.getRegistration(type);
	if (registration != null) {
		registration.setSerializer(serializer);
		return registration;
	}
	return classResolver.register(new Registration(type, serializer, getNextRegistrationId()));
}

And not forgetting

public Registration register (Class type)

which invokes register(Class, Serializer).

[theigl]

I mean something like this:

public class MyClassResolver extends DefaultClassResolver {

    @Override
    public Registration register(Registration registration) {
        return super.register(adapt(registration));
    }

    private Registration adapt(Registration registration) {
        // modify registration here
        return registration;
    }   
}

WDYT?

[chrisr3]

WDYT?

I think my customised Kryo class must now be paired with a customised ClassResolver, and Bad Things will happen if they are separated.

And you presumably mean?

protected Registration adapt(Registration registration)

[theigl]

Do you have full control over the Kryo instance? Or can it be changed by clients/users? If you have full control, then using a custom class resolver isn't really an issue. It's just like any other configurable part of Kryo.

[chrisr3]

We are already providing Kryo with a modified ClassResolver, for unrelated reasons and from a different code-base. Which is why I would prefer to keep all of my Kryo-related changes encapsulated within the Kryo class.

---

I've tried creating a

protected Registration adapt(Registration)

method, with the goal of adapting Kryo's invocations of ClassResolver.register(Registration), except this doesn't work for its invocation of ClassResolver.registerImplicit(Class) 🤦.

[theigl]

@chrisr3:

I gave your issue more thought over the weekend:

Even if I were to merge this change, you would still need a lot of complex code to achieve your desired behavior. Including your CollectionSerializerAdapter. I'm reluctant to add any code that only a single user needs and that doesn't even elegantly solve their problem.

I see only one option that does not require you to write a lot of workarounds: Adding something like Kryo.setOptimizedCollections(false). Both CollectionSerializer and ObjectArraySerializer could check this flag and disable the optimization.

Again, this is most likely not a feature a lot of users are going to need. It adds to Kryo's public API and will have to be maintained in the future. On the other hand, it would be straightforward to implement and could even be made protected so only users who extend Kryo can set the flag.

[chrisr3]

@chrisr3:
Even if I were to merge this change, you would still need a lot of complex code to achieve your desired behavior. Including your CollectionSerializerAdapter. I'm reluctant to add any code that only a single user needs and that doesn't even elegantly solve their problem.

To be fair, I never intended to add my CollectionSerializerAdapter to this PR. I was simply looking to create an "officially supported" method of intercepting the creation of new serializers so that I could adapt CollectionSerializer and its sub-classes. (The ObjectArraySerializer has no sub-classes, and so I can simply override it with my own rather then adapting it.)

I see only one option that does not require you to write a lot of workarounds: Adding something like Kryo.setOptimizedCollections(false). Both CollectionSerializer and ObjectArraySerializer could check this flag and disable the optimization.

Again, this is most likely not a feature a lot of users are going to need. It adds to Kryo's public API and will have to be maintained in the future.

Yes, I agree. This is why I was only trying to create a more general extension API that my adapter could use.

[theigl]

To be fair, I never intended to add my CollectionSerializerAdapter to this PR. I was simply looking to create an "officially supported" method of intercepting the creation of new serializers so that I could adapt CollectionSerializer and its sub-classes. (The ObjectArraySerializer has no sub-classes, and so I can simply override it with my own rather then adapting it.)

I think we misunderstood each other slightly. ;) I understand that you don't plan to add your CollectionSerializerAdapter to this PR. My main concern was that if I merge this PR, you will still have to use a lot of complex and potentially brittle (reflection) code on your side to get this working. So I was wondering if we shouldn't go the other way and make the behavior of CollectionSerializer and ObjectArraySerializer configurable.

But I just gave this a quick try and I'm not happy with the solution. Even if I add a configuration flag, CollectionSerializer still tries to find a concrete element serializer by analyzing the collections generics. So if you don't turn off generics optimization, there is at least one other code path that potentially breaks your application. So I would need to add multiple conditions to the already complex serializer.

[theigl]

Regarding your PR:

I'm not really happy about going from Kryo to DefaultClassResolver and then calling back into Kryo. And since these classes are in different packages, this means that the method in Kryo has to be public.

Are you sure you can't simply override your class resolver directly? Or wrap it in a decorator that notifies your custom Kryo instance about new registrations?

[chrisr3]

But I just gave this a quick try and I'm not happy with the solution. Even if I add a configuration flag, CollectionSerializer still tries to find a concrete element serializer by analyzing the collections generics. So if you don't turn off generics optimization, there is at least one other code path that potentially breaks your application. So I would need to add multiple conditions to the already complex serializer.

We're already turning the generics optimisation off because we're using Kotlin - see #864. Actually, I think I should dig deeper into that particular issue now... 🤔.