DefaultClassResolver : No back/forward compatibility with kryo.setRegistrationRequired(true) ?

[LouisVN]

Setup
kryo-shaded-4.0.0

Former configuration :

		public Kryo create() {
			Kryo kryo = new Kryo();
			kryo.setDefaultSerializer(TaggedFieldSerializer.class);
			return kryo;
		}

Intended configuration :

		public Kryo create() {
			Kryo kryo = new Kryo();
			kryo.setRegistrationRequired(true);
			kryo.setDefaultSerializer(TaggedFieldSerializer.class);
			return kryo;
		}

Problem :
com.esotericsoftware.kryo.util.DefaultClassResolver relies on nameIdToClass member.

    protected Registration readName(Input input) {
        int nameId = input.readVarInt(true);
        if(this.nameIdToClass == null) {
            this.nameIdToClass = new IntMap();
        }

        Class type = (Class)this.nameIdToClass.get(nameId);

If kryo.setRegistrationRequired(false); it seems that this expected information (nameId) was not serialized. Thus, cannot be retrieved when set to kryo.setRegistrationRequired(true);

There seems to be more about this in DefaultClassResolver :

    public Registration writeClass(Output output, Class type) {
        if(type != null) {
            Registration registration = this.kryo.getRegistration(type);
   
    ...   

    public void reset() {
        if(!this.kryo.isRegistrationRequired()) {
            if(this.classToNameId != null) {
                this.classToNameId.clear();
            }
            if(this.nameIdToClass != null) {
                this.nameIdToClass.clear();
            }
            this.nextNameId = 0;
        }
    }

Expected solution :
Back/forward compatibility when trying to force registration of objects, even if that was not done with entries already serialized.
This is intended to address the following : #398

Am I missing something and does that sound reasonable/feasible ?

[magro]

I didn't have a close look at the code, but it should be possible to make this issue reproducable with a unit test by using two differently configured kryo instances, right?

Do you have an idea how to achieve the expected solution?

[LouisVN]

Sorry for the delay but I was on vacation.
You will find a unit test here : https://github.com/LouisVN/kryo/blob/master/test/com/esotericsoftware/kryo/RegistrationCompatibilityTest.java
Let me know if I can help furthermore.

[NathanSweet]

testBackwardRegistrationSerialization is not supported. If classes are written as IDs, you can't later read them without registering the classes. Kryo reads the int ID but has no idea what class that means.

testForwardRegistrationSerialization should work. You have a bug in your test, you use TaggedFieldSerializer (set explicitly with setDefaultSerializer) when writing, but FieldSerializer (the default when setDefaultSerializer is not used) when reading. The fixed test passes:

	public void testForwardRegistrationSerialization () throws Exception {
		// Do not register when serialize at first
		kryo.setDefaultSerializer(TaggedFieldSerializer.class);
		kryo.setRegistrationRequired(false);
		serializeDummy();

		// Deserialize without forcing registration
		DummyObject dummyObject = deserializeDummy();
		assertObject(dummyObject);

		// Reset kryo and force registration
		setUp();
		kryo.setRegistrationRequired(true);
		kryo.setDefaultSerializer(TaggedFieldSerializer.class); // <-- added
		kryo.register(DummyObject.class);
		kryo.register(DummyObject.DummyInnerObject.class);
		kryo.register(DummyObject.DummyInnerObject.DummyInnerNestedObject.class);
		kryo.register(ArrayList.class);
		kryo.register(Date.class);
		dummyObject = deserializeDummy();
		assertObject(dummyObject);
	}