You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by ib...@apache.org on 2021/07/02 09:28:11 UTC
[ignite-3] branch main updated: IGNITE-15018 Add support for
message inheritance for @Transferable (#193)
This is an automated email from the ASF dual-hosted git repository.
ibessonov 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 2a8a3f5 IGNITE-15018 Add support for message inheritance for @Transferable (#193)
2a8a3f5 is described below
commit 2a8a3f5c9897dc02f486cc2d81d4c432439398e9
Author: Alexander Polovtcev <al...@gmail.com>
AuthorDate: Fri Jul 2 12:28:05 2021 +0300
IGNITE-15018 Add support for message inheritance for @Transferable (#193)
---
.../ITTransferableObjectProcessorTest.java | 10 +++++
.../network/processor/InheritedMessageClash.java} | 18 ++++----
.../internal/network/processor/MessageClass.java | 44 +++++++++++++++----
.../processor/TransferableObjectProcessor.java | 18 ++++----
.../internal/network/processor/TypeUtils.java | 51 +++++++++++++---------
.../messages/MessageBuilderGenerator.java | 4 +-
...TestMessageGroup.java => InheritedMessage.java} | 28 ++++++++----
...ionOrderTest.java => InheritedMessageTest.java} | 44 +++++++++----------
.../network/processor/SerializationOrderTest.java | 8 ++--
.../network/processor/TestMessageGroup.java | 2 +
10 files changed, 143 insertions(+), 84 deletions(-)
diff --git a/modules/network-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/network/processor/ITTransferableObjectProcessorTest.java b/modules/network-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/network/processor/ITTransferableObjectProcessorTest.java
index d137fe3..0a23e0b 100644
--- a/modules/network-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/network/processor/ITTransferableObjectProcessorTest.java
+++ b/modules/network-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/network/processor/ITTransferableObjectProcessorTest.java
@@ -225,6 +225,16 @@ public class ITTransferableObjectProcessorTest {
}
/**
+ * Tests that if a message getter clashes with a getter in a superinterface, an appropriate error is displayed.
+ */
+ @Test
+ void testInheritedMessageClash() {
+ Compilation compilation = compile("InheritedMessageClash");
+
+ assertThat(compilation).hadErrorContaining("Getter with name 'x' is already defined");
+ }
+
+ /**
* Compiles the given network message.
*/
private Compilation compile(String messageSource) {
diff --git a/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/TestMessageGroup.java b/modules/network-annotation-processor/src/integrationTest/resources/org/apache/ignite/internal/network/processor/InheritedMessageClash.java
similarity index 73%
copy from modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/TestMessageGroup.java
copy to modules/network-annotation-processor/src/integrationTest/resources/org/apache/ignite/internal/network/processor/InheritedMessageClash.java
index d018391..9bb2d9c 100644
--- a/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/TestMessageGroup.java
+++ b/modules/network-annotation-processor/src/integrationTest/resources/org/apache/ignite/internal/network/processor/InheritedMessageClash.java
@@ -17,13 +17,15 @@
package org.apache.ignite.internal.network.processor;
-import org.apache.ignite.network.annotations.MessageGroup;
+import java.io.Serializable;
+import org.apache.ignite.network.NetworkMessage;
+import org.apache.ignite.network.annotations.Transferable;
-/**
- * Message group for tests.
- */
-@MessageGroup(groupType = 1, groupName = "TestMessages")
-public class TestMessageGroup {
- /** Type of {@link SerializationOrderMessage} */
- public static final short SERIALIZATION_ORDER_MESSAGE = 1;
+interface NetworkMessage1 extends NetworkMessage {
+ double x();
+}
+
+@Transferable(2)
+public interface InheritedMessageClash extends NetworkMessage1 {
+ double x();
}
diff --git a/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/MessageClass.java b/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/MessageClass.java
index 17c66e0..0f41fa7 100644
--- a/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/MessageClass.java
+++ b/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/MessageClass.java
@@ -17,14 +17,17 @@
package org.apache.ignite.internal.network.processor;
-import java.util.Comparator;
import java.util.List;
+import java.util.Map;
import java.util.Objects;
+import java.util.TreeMap;
import java.util.stream.Collectors;
+import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.TypeElement;
import com.squareup.javapoet.ClassName;
+import org.apache.ignite.network.NetworkMessage;
import org.apache.ignite.network.annotations.Transferable;
/**
@@ -50,21 +53,46 @@ public class MessageClass {
/**
* @param messageElement element marked with the {@link Transferable} annotation.
*/
- MessageClass(TypeElement messageElement) {
+ MessageClass(ProcessingEnvironment processingEnv, TypeElement messageElement) {
element = messageElement;
className = ClassName.get(messageElement);
- annotation = element.getAnnotation(Transferable.class);
- getters = element.getEnclosedElements().stream()
- .filter(element -> element.getKind() == ElementKind.METHOD)
- .sorted(Comparator.comparing(element -> element.getSimpleName().toString()))
- .map(ExecutableElement.class::cast)
- .collect(Collectors.toUnmodifiableList());
+ annotation = messageElement.getAnnotation(Transferable.class);
+ getters = extractGetters(processingEnv, messageElement);
if (annotation.value() < 0)
throw new ProcessingException("Message type must not be negative", null, element);
}
/**
+ * Finds all getters in the given element and all of its superinterfaces.
+ */
+ private static List<ExecutableElement> extractGetters(ProcessingEnvironment processingEnv, TypeElement element) {
+ var typeUtils = new TypeUtils(processingEnv);
+
+ Map<String, ExecutableElement> gettersByName = typeUtils.allInterfaces(element)
+ // this algorithm is suboptimal, since we have to scan over the same interfaces over and over again,
+ // but this shouldn't be an issue, because it is not expected to have deep inheritance hierarchies
+ .filter(e -> typeUtils.hasSuperInterface(e, NetworkMessage.class))
+ // remove the NetworkMessage interface itself
+ .filter(e -> !typeUtils.isSameType(e.asType(), NetworkMessage.class))
+ .flatMap(e -> e.getEnclosedElements().stream())
+ .filter(e -> e.getKind() == ElementKind.METHOD)
+ // use a tree map to sort getters by name and remove duplicates
+ .collect(Collectors.toMap(
+ e -> e.getSimpleName().toString(),
+ ExecutableElement.class::cast,
+ (e1, e2) -> {
+ throw new ProcessingException(
+ String.format("Getter with name '%s' is already defined", e2.getSimpleName()), null, e2
+ );
+ },
+ TreeMap::new
+ ));
+
+ return List.copyOf(gettersByName.values());
+ }
+
+ /**
* @return annotated element
*/
public TypeElement element() {
diff --git a/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/TransferableObjectProcessor.java b/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/TransferableObjectProcessor.java
index f8b6711..e5afad2 100644
--- a/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/TransferableObjectProcessor.java
+++ b/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/TransferableObjectProcessor.java
@@ -64,17 +64,17 @@ public class TransferableObjectProcessor extends AbstractProcessor {
/** {@inheritDoc} */
@Override public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
- List<MessageClass> messages = annotations.stream()
- .map(roundEnv::getElementsAnnotatedWith)
- .flatMap(Collection::stream)
- .map(TypeElement.class::cast)
- .map(MessageClass::new)
- .collect(Collectors.toList());
+ try {
+ List<MessageClass> messages = annotations.stream()
+ .map(roundEnv::getElementsAnnotatedWith)
+ .flatMap(Collection::stream)
+ .map(TypeElement.class::cast)
+ .map(e -> new MessageClass(processingEnv, e))
+ .collect(Collectors.toList());
- if (messages.isEmpty())
- return true;
+ if (messages.isEmpty())
+ return true;
- try {
validateMessages(messages);
MessageGroupWrapper messageGroup = getMessageGroup(roundEnv);
diff --git a/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/TypeUtils.java b/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/TypeUtils.java
index 5129679..6afd225 100644
--- a/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/TypeUtils.java
+++ b/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/TypeUtils.java
@@ -18,11 +18,13 @@
package org.apache.ignite.internal.network.processor;
import java.util.ArrayDeque;
+import java.util.Objects;
+import java.util.stream.Stream;
import javax.annotation.processing.ProcessingEnvironment;
-import javax.lang.model.element.Element;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.PrimitiveType;
import javax.lang.model.type.TypeMirror;
+import javax.lang.model.util.Elements;
import javax.lang.model.util.Types;
import org.jetbrains.annotations.Nullable;
@@ -31,13 +33,17 @@ import org.jetbrains.annotations.Nullable;
*/
public class TypeUtils {
/** */
- private final ProcessingEnvironment processingEnvironment;
+ private final Types types;
+
+ /** */
+ private final Elements elements;
/**
* @param processingEnvironment processing environment
*/
public TypeUtils(ProcessingEnvironment processingEnvironment) {
- this.processingEnvironment = processingEnvironment;
+ this.types = processingEnvironment.getTypeUtils();
+ this.elements = processingEnvironment.getElementUtils();
}
/**
@@ -50,7 +56,7 @@ public class TypeUtils {
public boolean isSameType(TypeMirror type1, Class<?> type2) {
TypeMirror type2Mirror = typeMirrorFromClass(type2);
- return processingEnvironment.getTypeUtils().isSameType(erasure(type1), erasure(type2Mirror));
+ return types.isSameType(erasure(type1), erasure(type2Mirror));
}
/**
@@ -63,7 +69,7 @@ public class TypeUtils {
@Nullable
public PrimitiveType unboxedType(TypeMirror type) {
try {
- return processingEnvironment.getTypeUtils().unboxedType(type);
+ return types.unboxedType(type);
}
catch (IllegalArgumentException ignored) {
return null;
@@ -79,38 +85,41 @@ public class TypeUtils {
* @return {@code true} if the given {@code element} is a subtype of {@code cls}
*/
public boolean hasSuperInterface(TypeElement element, Class<?> cls) {
- // perform BFS to find the given interface among all possible superinterfaces
- var queue = new ArrayDeque<Element>();
-
- queue.add(element);
-
- while (!queue.isEmpty()) {
- Element currentElement = queue.pop();
+ return allInterfaces(element).anyMatch(e -> isSameType(e.asType(), cls));
+ }
- if (isSameType(currentElement.asType(), cls))
- return true;
+ /**
+ * Creates a stream of elements representing all superinterfaces of the given element, including the element itself.
+ *
+ * @param start starting element for exploring the inheritance hierarchy
+ * @return stream of superinterfaces
+ */
+ public Stream<TypeElement> allInterfaces(TypeElement start) {
+ // perform BFS to explore all superinterfaces
+ var queue = new ArrayDeque<TypeElement>();
- ((TypeElement)currentElement).getInterfaces().stream()
- .map(processingEnvironment.getTypeUtils()::asElement)
+ return Stream.iterate(start, Objects::nonNull, currentElement -> {
+ currentElement.getInterfaces().stream()
+ .map(types::asElement)
+ .map(TypeElement.class::cast)
.forEach(queue::add);
- }
- return false;
+ return queue.poll();
+ });
}
/**
* Shortcut for the {@link Types#erasure(TypeMirror)} method.
*/
private TypeMirror erasure(TypeMirror type) {
- return processingEnvironment.getTypeUtils().erasure(type);
+ return types.erasure(type);
}
/**
* Creates a {@link TypeMirror} represented by the given {@link Class}.
*/
private TypeMirror typeMirrorFromClass(Class<?> cls) {
- return processingEnvironment
- .getElementUtils()
+ return elements
.getTypeElement(cls.getCanonicalName())
.asType();
}
diff --git a/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageBuilderGenerator.java b/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageBuilderGenerator.java
index 0d15cc3..ed2c1b4 100644
--- a/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageBuilderGenerator.java
+++ b/modules/network-annotation-processor/src/main/java/org/apache/ignite/internal/network/processor/messages/MessageBuilderGenerator.java
@@ -43,9 +43,7 @@ public class MessageBuilderGenerator {
* @param processingEnvironment processing environment
* @param messageGroup message group
*/
- public MessageBuilderGenerator(
- ProcessingEnvironment processingEnvironment, MessageGroupWrapper messageGroup
- ) {
+ public MessageBuilderGenerator(ProcessingEnvironment processingEnvironment, MessageGroupWrapper messageGroup) {
this.processingEnvironment = processingEnvironment;
this.messageGroup = messageGroup;
}
diff --git a/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/TestMessageGroup.java b/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/InheritedMessage.java
similarity index 65%
copy from modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/TestMessageGroup.java
copy to modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/InheritedMessage.java
index d018391..f53e3c1 100644
--- a/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/TestMessageGroup.java
+++ b/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/InheritedMessage.java
@@ -17,13 +17,25 @@
package org.apache.ignite.internal.network.processor;
-import org.apache.ignite.network.annotations.MessageGroup;
+import java.io.Serializable;
+import org.apache.ignite.network.NetworkMessage;
+import org.apache.ignite.network.annotations.Transferable;
-/**
- * Message group for tests.
- */
-@MessageGroup(groupType = 1, groupName = "TestMessages")
-public class TestMessageGroup {
- /** Type of {@link SerializationOrderMessage} */
- public static final short SERIALIZATION_ORDER_MESSAGE = 1;
+/** */
+interface NetworkMessage1 extends NetworkMessage {
+ /** */
+ int y();
+}
+
+/** */
+interface NetworkMessage2 extends NetworkMessage1, Serializable {
+ /** */
+ int x();
+}
+
+/** */
+@Transferable(TestMessageGroup.INHERITED_MESSAGE)
+public interface InheritedMessage extends NetworkMessage2 {
+ /** */
+ int z();
}
diff --git a/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/SerializationOrderTest.java b/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/InheritedMessageTest.java
similarity index 58%
copy from modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/SerializationOrderTest.java
copy to modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/InheritedMessageTest.java
index f4b83b9..7d8e163 100644
--- a/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/SerializationOrderTest.java
+++ b/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/InheritedMessageTest.java
@@ -32,64 +32,62 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
/**
- * Test class for checking that writing and reading fields in the generated (de-)serializers is ordered
- * alphanumerically.
+ * Tests for support of network message inheritance hierarchies.
+ *
+ * @see InheritedMessage
*/
-public class SerializationOrderTest {
+public class InheritedMessageTest {
/** */
private final TestMessagesFactory messageFactory = new TestMessagesFactory();
/** */
- private final SerializationOrderMessageSerializationFactory serializationFactory =
- new SerializationOrderMessageSerializationFactory(messageFactory);
+ private final InheritedMessageSerializationFactory serializationFactory =
+ new InheritedMessageSerializationFactory(messageFactory);
/**
- * Tests that a generated {@link MessageSerializer} writes message fields in alphanumerical order.
+ * Tests that the generated message implementation contains all fields from the superinterfaces and is serialized
+ * in the correct order.
*/
@Test
- void testSerializationOrder() {
- SerializationOrderMessage msg = messageFactory.serializationOrderMessage()
- .a(1).b("2").c(3).d("4")
+ void testSerialization() {
+ InheritedMessage msg = messageFactory.inheritedMessage()
+ .x(1).y(2).z(3)
.build();
- MessageSerializer<SerializationOrderMessage> serializer = serializationFactory.createSerializer();
+ MessageSerializer<InheritedMessage> serializer = serializationFactory.createSerializer();
var mockWriter = mock(MessageWriter.class);
when(mockWriter.isHeaderWritten()).thenReturn(true);
when(mockWriter.writeInt(anyString(), anyInt())).thenReturn(true);
- when(mockWriter.writeString(anyString(), anyString())).thenReturn(true);
serializer.writeMessage(msg, mockWriter);
InOrder inOrder = inOrder(mockWriter);
- inOrder.verify(mockWriter).writeInt(eq("a"), anyInt());
- inOrder.verify(mockWriter).writeString(eq("b"), anyString());
- inOrder.verify(mockWriter).writeInt(eq("c"), anyInt());
- inOrder.verify(mockWriter).writeString(eq("d"), anyString());
+ inOrder.verify(mockWriter).writeInt(eq("x"), eq(1));
+ inOrder.verify(mockWriter).writeInt(eq("y"), eq(2));
+ inOrder.verify(mockWriter).writeInt(eq("z"), eq(3));
}
/**
- * Tests that a generated {@link MessageDeserializer} reads message fields in alphanumerical order.
+ * Tests that the generated message implementation is deserialized in the correct order.
*/
@Test
- void testDeserializationOrder() {
- MessageDeserializer<SerializationOrderMessage> deserializer = serializationFactory.createDeserializer();
+ void testDeserialization() {
+ MessageDeserializer<InheritedMessage> deserializer = serializationFactory.createDeserializer();
var mockReader = mock(MessageReader.class);
when(mockReader.beforeMessageRead()).thenReturn(true);
when(mockReader.isLastRead()).thenReturn(true);
- when(mockReader.readString(anyString())).thenReturn("foobar");
deserializer.readMessage(mockReader);
InOrder inOrder = inOrder(mockReader);
- inOrder.verify(mockReader).readInt(eq("a"));
- inOrder.verify(mockReader).readString(eq("b"));
- inOrder.verify(mockReader).readInt(eq("c"));
- inOrder.verify(mockReader).readString(eq("d"));
+ inOrder.verify(mockReader).readInt(eq("x"));
+ inOrder.verify(mockReader).readInt(eq("y"));
+ inOrder.verify(mockReader).readInt(eq("z"));
}
}
diff --git a/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/SerializationOrderTest.java b/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/SerializationOrderTest.java
index f4b83b9..2fc3df4 100644
--- a/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/SerializationOrderTest.java
+++ b/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/SerializationOrderTest.java
@@ -64,10 +64,10 @@ public class SerializationOrderTest {
InOrder inOrder = inOrder(mockWriter);
- inOrder.verify(mockWriter).writeInt(eq("a"), anyInt());
- inOrder.verify(mockWriter).writeString(eq("b"), anyString());
- inOrder.verify(mockWriter).writeInt(eq("c"), anyInt());
- inOrder.verify(mockWriter).writeString(eq("d"), anyString());
+ inOrder.verify(mockWriter).writeInt(eq("a"), eq(1));
+ inOrder.verify(mockWriter).writeString(eq("b"), eq("2"));
+ inOrder.verify(mockWriter).writeInt(eq("c"), eq(3));
+ inOrder.verify(mockWriter).writeString(eq("d"), eq("4"));
}
/**
diff --git a/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/TestMessageGroup.java b/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/TestMessageGroup.java
index d018391..d1c8360 100644
--- a/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/TestMessageGroup.java
+++ b/modules/network-annotation-processor/src/test/java/org/apache/ignite/internal/network/processor/TestMessageGroup.java
@@ -26,4 +26,6 @@ import org.apache.ignite.network.annotations.MessageGroup;
public class TestMessageGroup {
/** Type of {@link SerializationOrderMessage} */
public static final short SERIALIZATION_ORDER_MESSAGE = 1;
+
+ public static final short INHERITED_MESSAGE = 2;
}