This is an automated email from the ASF dual-hosted git repository. sdanilov pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push: new 6df971910 IGNITE-16573 Support schema changes concerning externalizability status (#785) 6df971910 is described below commit 6df971910d4bcee7f0916d258c2b00dbfd0b1b8f Author: Roman Puchkovskiy <roman.puchkovs...@gmail.com> AuthorDate: Wed Apr 20 14:20:37 2022 +0400 IGNITE-16573 Support schema changes concerning externalizability status (#785) --- .../serialization/ClassDescriptorFactory.java | 3 +- .../marshal/DefaultUserObjectMarshaller.java | 84 ++++++++------- .../marshal/ExternalizableMarshaller.java | 72 +++++++++++-- .../marshal/SchemaMismatchHandler.java | 30 +++++- .../marshal/SchemaMismatchHandlers.java | 9 ++ .../marshal/UosObjectInputStream.java | 2 +- .../marshal/UosObjectOutputStream.java | 15 +++ .../marshal/DefaultSchemaMismatchHandlerTest.java | 22 +++- ...UserObjectMarshallerWithExternalizableTest.java | 8 +- ...ltUserObjectMarshallerWithSchemaChangeTest.java | 120 ++++++++++++++++++++- 10 files changed, 310 insertions(+), 55 deletions(-) diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/ClassDescriptorFactory.java b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/ClassDescriptorFactory.java index ad72aad98..99c6bf7a2 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/ClassDescriptorFactory.java +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/ClassDescriptorFactory.java @@ -32,7 +32,6 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.ArrayDeque; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.Queue; @@ -137,7 +136,7 @@ public class ClassDescriptorFactory { descriptorId, superClassDescriptor(clazz), componentTypeDescriptor(clazz), - Collections.emptyList(), + fields(clazz), new Serialization( SerializationType.EXTERNALIZABLE, NO_WRITE_OBJECT, diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshaller.java b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshaller.java index 3f2a064da..15dfd5f57 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshaller.java +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshaller.java @@ -92,7 +92,8 @@ public class DefaultUserObjectMarshaller implements UserObjectMarshaller, Schema this::marshalUnshared, this::unmarshalShared, this::unmarshalUnshared, - structuredObjectMarshaller + structuredObjectMarshaller, + schemaMismatchHandlers ); proxyMarshaller = new ProxyMarshaller(this::marshalShared, this::unmarshalShared); @@ -326,9 +327,9 @@ public class DefaultUserObjectMarshaller implements UserObjectMarshaller, Schema UnmarshallingContext context, boolean unshared ) throws IOException, UnmarshalException { - ClassDescriptor descriptor = resolveDescriptor(input, declaredType, context); + ClassDescriptor remoteDescriptor = resolveDescriptor(input, declaredType, context); - if (mayHaveObjectIdentity(descriptor)) { + if (mayHaveObjectIdentity(remoteDescriptor)) { int objectId = peekObjectId(input, context); if (context.isKnownObjectId(objectId)) { // this is a back-reference @@ -336,9 +337,9 @@ public class DefaultUserObjectMarshaller implements UserObjectMarshaller, Schema } } - Object readObject = readObject(input, context, descriptor, unshared); + Object readObject = readObject(input, context, remoteDescriptor, unshared); - @SuppressWarnings("unchecked") T resolvedObject = (T) applyReadResolveIfNeeded(readObject, descriptor); + @SuppressWarnings("unchecked") T resolvedObject = (T) applyReadResolveIfNeeded(readObject, remoteDescriptor); return resolvedObject; } @@ -374,14 +375,14 @@ public class DefaultUserObjectMarshaller implements UserObjectMarshaller, Schema } @Nullable - private Object readObject(IgniteDataInput input, UnmarshallingContext context, ClassDescriptor descriptor, boolean unshared) + private Object readObject(IgniteDataInput input, UnmarshallingContext context, ClassDescriptor remoteDescriptor, boolean unshared) throws IOException, UnmarshalException { - if (!mayHaveObjectIdentity(descriptor)) { - return readValue(input, descriptor, context); - } else if (mustBeReadInOneStage(descriptor)) { - return readIdentifiableInOneStage(input, descriptor, context, unshared); + if (!mayHaveObjectIdentity(remoteDescriptor)) { + return readValue(input, remoteDescriptor, context); + } else if (mustBeReadInOneStage(remoteDescriptor)) { + return readIdentifiableInOneStage(input, remoteDescriptor, context, unshared); } else { - return readIdentifiableInTwoStages(input, descriptor, context, unshared); + return readIdentifiableInTwoStages(input, remoteDescriptor, context, unshared); } } @@ -410,55 +411,56 @@ public class DefaultUserObjectMarshaller implements UserObjectMarshaller, Schema private Object readIdentifiableInTwoStages( IgniteDataInput input, - ClassDescriptor descriptor, + ClassDescriptor remoteDescriptor, UnmarshallingContext context, boolean unshared ) throws IOException, UnmarshalException { int objectId = readObjectId(input); - Object preInstantiatedObject = preInstantiate(descriptor, input, context); + Object preInstantiatedObject = preInstantiate(remoteDescriptor, input, context); context.registerReference(objectId, preInstantiatedObject, unshared); - fillObjectFrom(input, preInstantiatedObject, descriptor, context); + fillObjectFrom(input, preInstantiatedObject, remoteDescriptor, context); return preInstantiatedObject; } - private Object preInstantiate(ClassDescriptor descriptor, IgniteDataInput input, UnmarshallingContext context) + private Object preInstantiate(ClassDescriptor remoteDescriptor, IgniteDataInput input, UnmarshallingContext context) throws IOException, UnmarshalException { - if (isBuiltInNonContainer(descriptor)) { - throw new IllegalStateException("Should not be here, descriptor is " + descriptor); - } else if (isBuiltInCollection(descriptor)) { - return builtInContainerMarshallers.preInstantiateBuiltInMutableCollection(descriptor, input, context); - } else if (isBuiltInMap(descriptor)) { - return builtInContainerMarshallers.preInstantiateBuiltInMutableMap(descriptor, input, context); - } else if (descriptor.isArray()) { + if (isBuiltInNonContainer(remoteDescriptor)) { + throw new IllegalStateException("Should not be here, descriptor is " + remoteDescriptor); + } else if (isBuiltInCollection(remoteDescriptor)) { + return builtInContainerMarshallers.preInstantiateBuiltInMutableCollection(remoteDescriptor, input, context); + } else if (isBuiltInMap(remoteDescriptor)) { + return builtInContainerMarshallers.preInstantiateBuiltInMutableMap(remoteDescriptor, input, context); + } else if (remoteDescriptor.isArray()) { return builtInContainerMarshallers.preInstantiateGenericRefArray(input, context); - } else if (descriptor.isExternalizable()) { - return externalizableMarshaller.preInstantiateExternalizable(descriptor); - } else if (descriptor.isProxy()) { + } else if (remoteDescriptor.isExternalizable()) { + return externalizableMarshaller.preInstantiateExternalizable(remoteDescriptor); + } else if (remoteDescriptor.isProxy()) { return proxyMarshaller.preInstantiateProxy(input, context); } else { - return structuredObjectMarshaller.preInstantiateStructuredObject(descriptor); + return structuredObjectMarshaller.preInstantiateStructuredObject(remoteDescriptor); } } - private void fillObjectFrom(IgniteDataInput input, Object objectToFill, ClassDescriptor descriptor, UnmarshallingContext context) + private void fillObjectFrom(IgniteDataInput input, Object objectToFill, ClassDescriptor remoteDescriptor, UnmarshallingContext context) throws UnmarshalException, IOException { - if (isBuiltInNonContainer(descriptor)) { - throw new IllegalStateException("Cannot fill " + descriptor.className() + ", this is a programmatic error"); - } else if (isBuiltInCollection(descriptor)) { - fillBuiltInCollectionFrom(input, (Collection<?>) objectToFill, descriptor, context); - } else if (isBuiltInMap(descriptor)) { + if (isBuiltInNonContainer(remoteDescriptor)) { + throw new IllegalStateException("Cannot fill " + remoteDescriptor.className() + ", this is a programmatic error"); + } else if (isBuiltInCollection(remoteDescriptor)) { + fillBuiltInCollectionFrom(input, (Collection<?>) objectToFill, remoteDescriptor, context); + } else if (isBuiltInMap(remoteDescriptor)) { fillBuiltInMapFrom(input, (Map<?, ?>) objectToFill, context); - } else if (descriptor.isArray()) { - fillGenericRefArrayFrom(input, (Object[]) objectToFill, descriptor, context); - } else if (descriptor.isExternalizable()) { - externalizableMarshaller.fillExternalizableFrom(input, (Externalizable) objectToFill, context); - } else if (descriptor.isProxy()) { + } else if (remoteDescriptor.isArray()) { + fillGenericRefArrayFrom(input, (Object[]) objectToFill, remoteDescriptor, context); + } else if (remoteDescriptor.isExternalizable()) { + externalizableMarshaller.fillFromRemotelyExternalizable(input, objectToFill, context); + } else if (remoteDescriptor.isProxy()) { proxyMarshaller.fillProxyFrom(input, objectToFill, context); } else { - structuredObjectMarshaller.fillStructuredObjectFrom(input, objectToFill, descriptor, context); + structuredObjectMarshaller.fillStructuredObjectFrom(input, objectToFill, remoteDescriptor, context); + fireExternalizableMissedIfExternalizableLocally(objectToFill); } } @@ -517,6 +519,12 @@ public class DefaultUserObjectMarshaller implements UserObjectMarshaller, Schema } } + private void fireExternalizableMissedIfExternalizableLocally(Object objectToFill) throws SchemaMismatchException { + if (objectToFill instanceof Externalizable) { + schemaMismatchHandlers.onExternalizableMissed(objectToFill); + } + } + /** {@inheritDoc} */ @Override public <T> void replaceSchemaMismatchHandler(Class<T> layerClass, SchemaMismatchHandler<T> handler) { diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/ExternalizableMarshaller.java b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/ExternalizableMarshaller.java index 65808405d..cdd53aede 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/ExternalizableMarshaller.java +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/ExternalizableMarshaller.java @@ -22,6 +22,7 @@ import java.io.IOException; import org.apache.ignite.internal.network.serialization.ClassDescriptor; import org.apache.ignite.internal.util.io.IgniteDataInput; import org.apache.ignite.internal.util.io.IgniteDataOutput; +import org.apache.ignite.internal.util.io.IgniteUnsafeDataInput; /** * (Um)marshalling specific to EXTERNALIZABLE serialization type. @@ -33,6 +34,8 @@ class ExternalizableMarshaller { private final TypedValueWriter unsharedWriter; private final DefaultFieldsReaderWriter defaultFieldsReaderWriter; + private final SchemaMismatchHandlers schemaMismatchHandlers; + private final NoArgConstructorInstantiation instantiation = new NoArgConstructorInstantiation(); ExternalizableMarshaller( @@ -40,13 +43,15 @@ class ExternalizableMarshaller { TypedValueWriter unsharedWriter, TypedValueReader valueReader, TypedValueReader unsharedReader, - DefaultFieldsReaderWriter defaultFieldsReaderWriter + DefaultFieldsReaderWriter defaultFieldsReaderWriter, + SchemaMismatchHandlers schemaMismatchHandlers ) { this.valueWriter = typedValueWriter; this.unsharedWriter = unsharedWriter; this.valueReader = valueReader; this.unsharedReader = unsharedReader; this.defaultFieldsReaderWriter = defaultFieldsReaderWriter; + this.schemaMismatchHandlers = schemaMismatchHandlers; } void writeExternalizable(Externalizable externalizable, ClassDescriptor descriptor, IgniteDataOutput output, MarshallingContext context) @@ -65,23 +70,50 @@ class ExternalizableMarshaller { context.endWritingWithWriteObject(); try { - externalizable.writeExternal(oos); - oos.flush(); + writeWithLength(externalizable, oos); } finally { oos.restoreCurrentPutFieldTo(oldPut); } } - @SuppressWarnings("unchecked") - <T extends Externalizable> T preInstantiateExternalizable(ClassDescriptor descriptor) throws UnmarshalException { + private void writeWithLength(Externalizable externalizable, UosObjectOutputStream oos) throws IOException { + // NB: this only works with purely in-memory IgniteDataInput implementations! + + int offsetBefore = oos.memoryBufferOffset(); + + writeLengthPlaceholder(oos); + + externalizable.writeExternal(oos); + oos.flush(); + + int externalDataLength = oos.memoryBufferOffset() - offsetBefore - Integer.BYTES; + + oos.writeIntAtOffset(offsetBefore, externalDataLength); + } + + private void writeLengthPlaceholder(UosObjectOutputStream oos) throws IOException { + oos.writeInt(0); + } + + Object preInstantiateExternalizable(ClassDescriptor descriptor) throws UnmarshalException { try { - return (T) instantiation.newInstance(descriptor.localClass()); + return instantiation.newInstance(descriptor.localClass()); } catch (InstantiationException e) { throw new UnmarshalException("Cannot instantiate " + descriptor.className(), e); } } - <T extends Externalizable> void fillExternalizableFrom(IgniteDataInput input, T object, UnmarshallingContext context) + void fillFromRemotelyExternalizable(IgniteDataInput input, Object object, UnmarshallingContext context) + throws UnmarshalException, IOException { + if (object instanceof Externalizable) { + fillExternalizableFrom(input, (Externalizable) object, context); + } else { + // it was serialized as an Externalizable, but locally it is not Externalizable; delegate to handler + fireExternalizableIgnored(object, input, context); + } + } + + private <T extends Externalizable> void fillExternalizableFrom(IgniteDataInput input, T object, UnmarshallingContext context) throws IOException, UnmarshalException { // Do not close the stream yet! UosObjectInputStream ois = context.objectInputStream(input, valueReader, unsharedReader, defaultFieldsReaderWriter); @@ -90,11 +122,35 @@ class ExternalizableMarshaller { context.endReadingWithReadObject(); try { - object.readExternal(ois); + readFramed(object, ois); } catch (ClassNotFoundException e) { throw new UnmarshalException("Cannot unmarshal due to a missing class", e); } finally { ois.restoreCurrentGetFieldTo(oldGet); } } + + private <T extends Externalizable> void readFramed(T object, UosObjectInputStream ois) throws IOException, ClassNotFoundException { + skipExternalDataLength(ois); + + object.readExternal(ois); + } + + private void skipExternalDataLength(UosObjectInputStream ois) throws IOException { + ois.readInt(); + } + + private void fireExternalizableIgnored(Object object, IgniteDataInput input, UnmarshallingContext context) + throws SchemaMismatchException, IOException { + // We have additional allocations and copying here. It simplifies the code a lot, and it seems that we should + // not optimize for this rare corner case. + + int externalDataLength = input.readInt(); + byte[] externalDataBytes = input.readByteArray(externalDataLength); + IgniteDataInput externalDataInput = new IgniteUnsafeDataInput(externalDataBytes); + + try (var oos = new UosObjectInputStream(externalDataInput, valueReader, unsharedReader, defaultFieldsReaderWriter, context)) { + schemaMismatchHandlers.onExternalizableIgnored(object, oos); + } + } } diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/SchemaMismatchHandler.java b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/SchemaMismatchHandler.java index b0f0965e2..19e9a4399 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/SchemaMismatchHandler.java +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/SchemaMismatchHandler.java @@ -17,6 +17,9 @@ package org.apache.ignite.internal.network.serialization.marshal; +import java.io.ObjectInput; +import java.io.ObjectOutput; + /** * Handles situations when the schema that was used to serialize an object (remote schema) is different from the * schema used to deserialize the object (local schema). @@ -65,6 +68,31 @@ public interface SchemaMismatchHandler<T> { * @throws SchemaMismatchException thrown if the handler wills to stop the deserialization with an error */ default void onFieldTypeChanged(T instance, String fieldName, Class<?> remoteType, Object fieldValue) throws SchemaMismatchException { - throw new SchemaMismatchException(fieldName + " type changed, serialized as " + remoteType.getName() + ", value " + fieldValue); + throw new SchemaMismatchException(fieldName + " type changed, serialized as " + remoteType.getName() + ", value " + fieldValue + + " of type " + fieldName.getClass().getName()); + } + + /** + * Called when a remote class implements {@link java.io.Externalizable}, but local class does not. + * + * @param instance the object that was constructed, but not yet filled + * @param externalData externalized data that represents the object (it was written + * using {@link java.io.Externalizable#writeExternal(ObjectOutput)} + * @throws SchemaMismatchException thrown if the handler wills to stop the deserialization with an error + */ + default void onExternalizableIgnored(T instance, ObjectInput externalData) throws SchemaMismatchException { + throw new SchemaMismatchException("Class " + instance.getClass().getName() + + " was serialized as an Externalizable remotely, but locally it is not an Externalizable"); + } + + /** + * Called when a remote class does not implement {@link java.io.Externalizable}, but local class does. The method is called after + * all read fields are assigned to the instance. + * + * @param instance the instance that has already been filled + * @throws SchemaMismatchException thrown if the handler wills to stop the deserialization with an error + */ + default void onExternalizableMissed(T instance) throws SchemaMismatchException { + // no-op } } diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/SchemaMismatchHandlers.java b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/SchemaMismatchHandlers.java index 402b834d0..e67e6e4ab 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/SchemaMismatchHandlers.java +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/SchemaMismatchHandlers.java @@ -17,6 +17,7 @@ package org.apache.ignite.internal.network.serialization.marshal; +import java.io.ObjectInput; import java.util.HashMap; import java.util.Map; @@ -52,4 +53,12 @@ class SchemaMismatchHandlers { throws SchemaMismatchException { handlerFor(layerClass).onFieldTypeChanged(instance, fieldName, remoteType, fieldValue); } + + void onExternalizableIgnored(Object instance, ObjectInput externalData) throws SchemaMismatchException { + handlerFor(instance.getClass()).onExternalizableIgnored(instance, externalData); + } + + void onExternalizableMissed(Object instance) throws SchemaMismatchException { + handlerFor(instance.getClass()).onExternalizableMissed(instance); + } } diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/UosObjectInputStream.java b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/UosObjectInputStream.java index 0072a0c58..d7fc2dd2d 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/UosObjectInputStream.java +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/UosObjectInputStream.java @@ -276,7 +276,7 @@ class UosObjectInputStream extends ObjectInputStream { /** {@inheritDoc} */ @Override - public boolean defaulted(String name) throws IOException { + public boolean defaulted(String name) { // TODO: IGNITE-16571 - actually take into account whether it's defaulted or not return false; } diff --git a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/UosObjectOutputStream.java b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/UosObjectOutputStream.java index 6dddd4ef8..98f8e0ca7 100644 --- a/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/UosObjectOutputStream.java +++ b/modules/network/src/main/java/org/apache/ignite/internal/network/serialization/marshal/UosObjectOutputStream.java @@ -237,6 +237,21 @@ class UosObjectOutputStream extends ObjectOutputStream { currentPut = newPut; } + int memoryBufferOffset() { + return output.offset(); + } + + void writeIntAtOffset(int offset, int value) throws IOException { + int oldOffset = output.offset(); + + try { + output.offset(offset); + output.writeInt(value); + } finally { + output.offset(oldOffset); + } + } + class UosPutField extends PutField { private final ClassDescriptor descriptor; diff --git a/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultSchemaMismatchHandlerTest.java b/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultSchemaMismatchHandlerTest.java index f25947ec8..4a524863b 100644 --- a/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultSchemaMismatchHandlerTest.java +++ b/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultSchemaMismatchHandlerTest.java @@ -17,9 +17,13 @@ package org.apache.ignite.internal.network.serialization.marshal; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import java.io.ObjectInput; import org.junit.jupiter.api.Test; class DefaultSchemaMismatchHandlerTest { @@ -37,6 +41,22 @@ class DefaultSchemaMismatchHandlerTest { @Test void throwsOnFieldTypeChanged() { - assertThrows(SchemaMismatchException.class, () -> handler.onFieldTypeChanged(new Object(), "field", int.class, "value")); + var ex = assertThrows(SchemaMismatchException.class, () -> handler.onFieldTypeChanged(new Object(), "field", int.class, "value")); + assertThat(ex.getMessage(), is("field type changed, serialized as int, value value of type java.lang.String")); + } + + @Test + void throwsOnExternalizableIgnored() { + var ex = assertThrows(SchemaMismatchException.class, + () -> handler.onExternalizableIgnored(new Object(), mock(ObjectInput.class)) + ); + assertThat(ex.getMessage(), + is("Class java.lang.Object was serialized as an Externalizable remotely, but locally it is not an Externalizable") + ); + } + + @Test + void doesNothingOnExternalizableMissed() { + assertDoesNotThrow(() -> handler.onExternalizableMissed(new Object())); } } diff --git a/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshallerWithExternalizableTest.java b/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshallerWithExternalizableTest.java index e088eb1f6..98184d546 100644 --- a/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshallerWithExternalizableTest.java +++ b/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshallerWithExternalizableTest.java @@ -29,8 +29,6 @@ import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.io.ByteArrayInputStream; -import java.io.DataInputStream; import java.io.Externalizable; import java.io.IOException; import java.io.ObjectInput; @@ -42,6 +40,7 @@ import java.util.Set; import org.apache.ignite.internal.network.serialization.ClassDescriptor; import org.apache.ignite.internal.network.serialization.ClassDescriptorFactory; import org.apache.ignite.internal.network.serialization.ClassDescriptorRegistry; +import org.apache.ignite.internal.util.io.IgniteUnsafeDataInput; import org.junit.jupiter.api.Test; /** @@ -214,12 +213,15 @@ class DefaultUserObjectMarshallerWithExternalizableTest { void writingObjectInsideWriteExternalMarshalsTheObjectInOurFormat() throws Exception { MarshalledObject marshalled = marshaller.marshal(new ExternalizableWritingAndReadingObject(new IntHolder(42))); - DataInputStream dis = new DataInputStream(new ByteArrayInputStream(marshalled.bytes())); + IgniteUnsafeDataInput dis = new IgniteUnsafeDataInput(marshalled.bytes()); ProtocolMarshalling.readDescriptorOrCommandId(dis); ProtocolMarshalling.readObjectId(dis); + int externalDataLength = dis.readInt(); byte[] externalBytes = dis.readAllBytes(); + assertThat(externalBytes.length, is(externalDataLength)); + IntHolder nested = marshaller.unmarshal(externalBytes, descriptorRegistry); assertThat(nested, is(notNullValue())); assertThat(nested.value, is(42)); diff --git a/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshallerWithSchemaChangeTest.java b/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshallerWithSchemaChangeTest.java index 7b246e91c..2555559e1 100644 --- a/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshallerWithSchemaChangeTest.java +++ b/modules/network/src/test/java/org/apache/ignite/internal/network/serialization/marshal/DefaultUserObjectMarshallerWithSchemaChangeTest.java @@ -22,11 +22,20 @@ import static net.bytebuddy.dynamic.loading.ClassLoadingStrategy.Default.CHILD_F import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import it.unimi.dsi.fastutil.ints.Int2ObjectMaps; +import java.io.Externalizable; +import java.io.IOException; +import java.io.InputStream; +import java.io.ObjectInput; +import java.io.ObjectOutput; import java.lang.reflect.Constructor; import java.util.List; import java.util.Map; @@ -161,12 +170,12 @@ class DefaultUserObjectMarshallerWithSchemaChangeTest { verify(schemaMismatchHandler).onFieldTypeChanged(unmarshalled, "value", String.class, "fourty two"); } - @SuppressWarnings("unchecked") @Test void nonPrimitiveFieldTypeChangedToSuperClassIsCompatibleChange() throws Exception { Class<?> remoteClass = addFieldTo(Empty.class, "value", String.class); Class<?> localClass = addFieldTo(Empty.class, "value", CharSequence.class); + //noinspection unchecked localMarshaller.replaceSchemaMismatchHandler((Class<Object>) localClass, schemaMismatchHandler); Object remoteInstance = instantiate(remoteClass); @@ -259,6 +268,97 @@ class DefaultUserObjectMarshallerWithSchemaChangeTest { } } + @Test + void removalOfExternalizableInterfaceCausesUnfilledDeserializationResult() throws Exception { + Object unmarshalled = marshalExternalizableUnmarshalNonExternalizableBasedOn(ExternalizationReady.class); + + int localFieldValue = (Integer) IgniteTestUtils.getFieldValue(unmarshalled, unmarshalled.getClass(), "value"); + assertThat(localFieldValue, is(0)); + } + + @Test + void removalOfExternalizableInterfaceTriggersHandlerInvocation() throws Exception { + Object unmarshalled = marshalExternalizableUnmarshalNonExternalizableBasedOn(ExternalizationReady.class); + + verify(schemaMismatchHandler).onExternalizableIgnored(eq(unmarshalled), any()); + } + + @Test + void onExternalizableIgnoredReceivesStreamWithExactlyExternalizedDataAvailable() throws Exception { + doAnswer(invocation -> { + InputStream externalDataStream = invocation.getArgument(1); + byte[] externalData = externalDataStream.readAllBytes(); + + byte[] int42BytesInLittleEndian = {42, 0, 0, 0}; + assertThat(externalData, is(int42BytesInLittleEndian)); + + return null; + }).when(schemaMismatchHandler).onExternalizableIgnored(any(), any()); + + marshalExternalizableUnmarshalNonExternalizableBasedOn(ExternalizationReady.class); + } + + @Test + void onExternalizableIgnoredSkipsExternalDataEvenIfHandlerDoesNotReadIt() throws Exception { + doNothing().when(schemaMismatchHandler).onExternalizableIgnored(any(), any()); + + assertDoesNotThrow(() -> marshalExternalizableUnmarshalNonExternalizableBasedOn(ExternalizationReady.class)); + } + + private Object marshalExternalizableUnmarshalNonExternalizableBasedOn(Class<?> baseClass) + throws ReflectiveOperationException, MarshalException, UnmarshalException { + @SuppressWarnings("UnnecessaryLocalVariable") + Class<?> localClass = baseClass; + Class<?> remoteClass = addExternalizableInterface(baseClass); + + //noinspection unchecked + localMarshaller.replaceSchemaMismatchHandler((Class<Object>) localClass, schemaMismatchHandler); + + Object remoteInstance = instantiate(remoteClass); + IgniteTestUtils.setFieldValue(remoteInstance, "value", 42); + + return marshalRemotelyAndUnmarshalLocally(remoteInstance, localClass, remoteClass); + } + + private Class<?> addExternalizableInterface(Class<?> baseClass) { + return new ByteBuddy() + .redefine(baseClass) + .implement(Externalizable.class) + .make() + .load(getClass().getClassLoader(), CHILD_FIRST) + .getLoaded(); + } + + @Test + void additionOfExternalizableInterfaceCausesStandardDeserialization() throws Exception { + Object unmarshalled = marshalNonExternalizableUnmarshalExternalizableBasedOn(ExternalizationReady.class); + + int localFieldValue = (Integer) IgniteTestUtils.getFieldValue(unmarshalled, unmarshalled.getClass(), "value"); + assertThat(localFieldValue, is(42)); + } + + @Test + void additionOfExternalizableInterfaceTriggersHandlerInvocation() throws Exception { + Object unmarshalled = marshalNonExternalizableUnmarshalExternalizableBasedOn(ExternalizationReady.class); + + verify(schemaMismatchHandler).onExternalizableMissed(eq(unmarshalled)); + } + + private Object marshalNonExternalizableUnmarshalExternalizableBasedOn(Class<?> baseClass) + throws ReflectiveOperationException, MarshalException, UnmarshalException { + @SuppressWarnings("UnnecessaryLocalVariable") + Class<?> remoteClass = baseClass; + Class<?> localClass = addExternalizableInterface(baseClass); + + //noinspection unchecked + localMarshaller.replaceSchemaMismatchHandler((Class<Object>) localClass, schemaMismatchHandler); + + Object remoteInstance = instantiate(remoteClass); + IgniteTestUtils.setFieldValue(remoteInstance, "value", 42); + + return marshalRemotelyAndUnmarshalLocally(remoteInstance, localClass, remoteClass); + } + private static class Empty { } @@ -280,4 +380,22 @@ class DefaultUserObjectMarshallerWithSchemaChangeTest { + '}'; } } + + private static class ExternalizationReady { + private int value; + + public ExternalizationReady() { + // no-op + } + + @SuppressWarnings("unused") + public void writeExternal(ObjectOutput out) throws IOException { + out.writeInt(value); + } + + @SuppressWarnings("unused") + public void readExternal(ObjectInput in) throws IOException { + value = in.readInt(); + } + } }