[#380] It's not possible to define multiple @ProtoAdapters for a sing…

…le Interface type
infinispan · Dec 11, 2024 · e2ed575 · e2ed575
1 parent 2cc5f09
commit e2ed575
Show file tree

Hide file tree

Showing 11 changed files with 136 additions and 14 deletions.
diff --git a/core/src/main/java/org/infinispan/protostream/annotations/impl/BaseProtoSchemaGenerator.java b/core/src/main/java/org/infinispan/protostream/annotations/impl/BaseProtoSchemaGenerator.java
@@ -277,7 +277,8 @@ void collectMetadata(ProtoTypeMetadata protoTypeMetadata) {
                + " which was not added to the builder and 'autoImportClasses' is disabled.");
       }
 
-      ProtoTypeMetadata existingByClass = metadataByClass.get(protoTypeMetadata.getJavaClass());
+      XClass clazz = protoTypeMetadata.isInterfaceAdapter() ? protoTypeMetadata.getAnnotatedClass() : protoTypeMetadata.getJavaClass();
+      ProtoTypeMetadata existingByClass = metadataByClass.get(clazz);
       if (existingByClass != null) {
          throw new ProtoSchemaBuilderException("Found a duplicate type definition. Java type '" + protoTypeMetadata.getJavaClassName() + "' is defined by "
                + protoTypeMetadata.getAnnotatedClassName() + " and also by " + existingByClass.getAnnotatedClassName());
@@ -289,7 +290,7 @@ void collectMetadata(ProtoTypeMetadata protoTypeMetadata) {
                + protoTypeMetadata.getAnnotatedClassName() + " and also by " + existingByName.getAnnotatedClassName());
       }
       metadataByTypeName.put(fullName, protoTypeMetadata);
-      metadataByClass.put(protoTypeMetadata.getJavaClass(), protoTypeMetadata);
+      metadataByClass.put(clazz, protoTypeMetadata);
    }
 
    protected boolean isUnknownClass(XClass c) {

diff --git a/core/src/main/java/org/infinispan/protostream/annotations/impl/ProtoMessageTypeMetadata.java b/core/src/main/java/org/infinispan/protostream/annotations/impl/ProtoMessageTypeMetadata.java
@@ -1,3 +1,4 @@
+
 package org.infinispan.protostream.annotations.impl;
 
 import java.io.ByteArrayOutputStream;
@@ -92,11 +93,12 @@ protected ProtoMessageTypeMetadata(BaseProtoSchemaGenerator protoSchemaGenerator
    }
 
    private static String getProtoName(XClass annotatedClass, XClass javaClass) {
+      var clazz = javaClass.isInterface() ? annotatedClass : javaClass;
       ProtoName annotation = annotatedClass.getAnnotation(ProtoName.class);
       if (annotation != null) {
-         return annotation.value().isEmpty() ? javaClass.getSimpleName() : annotation.value();
+         return annotation.value().isEmpty() ? clazz.getSimpleName() : annotation.value();
       } else {
-         return javaClass.getSimpleName();
+         return clazz.getSimpleName();
       }
    }
 

diff --git a/core/src/main/java/org/infinispan/protostream/annotations/impl/ProtoTypeMetadata.java b/core/src/main/java/org/infinispan/protostream/annotations/impl/ProtoTypeMetadata.java
@@ -112,6 +112,10 @@ public boolean isAdapter() {
       return false;
    }
 
+   public boolean isInterfaceAdapter() {
+      return javaClass.isInterface();
+   }
+
    /**
     * Indicates if this type comes from the currently processed/generated schema of from an external schema.
     */

diff --git a/core/src/main/java/org/infinispan/protostream/impl/SerializationContextImpl.java b/core/src/main/java/org/infinispan/protostream/impl/SerializationContextImpl.java
@@ -242,6 +242,7 @@ public void registerMarshaller(BaseMarshaller<?> marshaller) {
 
       long stamp = manifestLock.writeLock();
       try {
+         var isInterface = marshaller.getJavaClass().isInterface();
          Registration existingByName = marshallersByName.get(marshaller.getTypeName());
          Registration existingByClass = marshallersByClass.get(marshaller.getJavaClass());
          if (existingByName != null && existingByName.marshallerProvider != null ||
@@ -259,17 +260,19 @@ public void registerMarshaller(BaseMarshaller<?> marshaller) {
 
          if (existingByName != null) {
             Registration anotherByClass = marshallersByClass.get(existingByName.marshallerDelegate.getMarshaller().getJavaClass());
-            if (anotherByClass == null) {
-               throw new IllegalStateException("Inconsistent marshaller definitions!");
-            }
-            if (anotherByClass.marshallerProvider != null) {
-               throw new IllegalArgumentException("The given marshaller attempts to override an existing marshaller registered indirectly via an InstanceMarshallerProvider. Please unregister that first.");
-            } else {
-               if (!anotherByClass.marshallerDelegate.getMarshaller().getTypeName().equals(marshaller.getTypeName())) {
+            if (!isInterface) {
+               if (anotherByClass == null) {
                   throw new IllegalStateException("Inconsistent marshaller definitions!");
                }
+               if (anotherByClass.marshallerProvider != null) {
+                  throw new IllegalArgumentException("The given marshaller attempts to override an existing marshaller registered indirectly via an InstanceMarshallerProvider. Please unregister that first.");
+               } else {
+                  if (!anotherByClass.marshallerDelegate.getMarshaller().getTypeName().equals(marshaller.getTypeName())) {
+                     throw new IllegalStateException("Inconsistent marshaller definitions!");
+                  }
+               }
+               marshallersByClass.remove(existingByName.marshallerDelegate.getMarshaller().getJavaClass());
             }
-            marshallersByClass.remove(existingByName.marshallerDelegate.getMarshaller().getJavaClass());
             for (Class<?> subClass : subClasses) {
                marshallersByClass.remove(subClass);
             }
@@ -279,7 +282,10 @@ public void registerMarshaller(BaseMarshaller<?> marshaller) {
          }
 
          Registration registration = new Registration(makeMarshallerDelegate(marshaller));
-         marshallersByClass.put(marshaller.getJavaClass(), registration);
+         // If the Class associated with the marshaller is an interface, then we only add the subClassName implementations
+         if (!isInterface)
+            marshallersByClass.put(marshaller.getJavaClass(), registration);
+
          marshallersByName.put(marshaller.getTypeName(), registration);
          for (Class<?> subClass : subClasses) {
             marshallersByClass.put(subClass, registration);

diff --git a/.../org/infinispan/protostream/annotations/impl/processor/MarshallerSourceCodeGenerator.java b/.../org/infinispan/protostream/annotations/impl/processor/MarshallerSourceCodeGenerator.java
@@ -208,7 +208,7 @@ private void generateMessageMarshaller(ProtoMessageTypeMetadata pmtm) throws IOE
          iw.println("@Override");
          iw.println("public String[] getSubClassNames() {");
          iw.inc().print("return new String[] {");
-         iw.print(Arrays.stream(subClassNames).collect(Collectors.joining(",", "\"", "\"")));
+         iw.print(Arrays.stream(subClassNames).map(s -> "\"" + s + "\"").collect(Collectors.joining(",")));
          iw.println("};");
          iw.dec().println("}");
       }

diff --git a/...n/java/org/infinispan/protostream/annotations/impl/processor/types/MirrorTypeFactory.java b/...n/java/org/infinispan/protostream/annotations/impl/processor/types/MirrorTypeFactory.java
@@ -698,6 +698,11 @@ public boolean isLocal() {
          return nestingKind == NestingKind.LOCAL || nestingKind == NestingKind.ANONYMOUS;
       }
 
+      @Override
+      public boolean isInterface() {
+         return typeElement.getKind().isInterface();
+      }
+
       @Override
       public XConstructor getDeclaredConstructor(XClass... argTypes) {
          return cacheConstructor(findExecutableElement(true, null, argTypes));

diff --git a/types/src/test/java/org/infinispan/protostream/types/java/EmptyListAdapter.java b/types/src/test/java/org/infinispan/protostream/types/java/EmptyListAdapter.java
@@ -0,0 +1,18 @@
+package org.infinispan.protostream.types.java;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.infinispan.protostream.annotations.ProtoAdapter;
+import org.infinispan.protostream.annotations.ProtoFactory;
+
+@ProtoAdapter(
+      value = List.class,
+      subClassNames = "java.util.Collections$EmptyList"
+)
+public class EmptyListAdapter {
+   @ProtoFactory
+   List<?> create() {
+      return Collections.emptyList();
+   }
+}
diff --git a/types/src/test/java/org/infinispan/protostream/types/java/ListOfAdapter.java b/types/src/test/java/org/infinispan/protostream/types/java/ListOfAdapter.java
@@ -0,0 +1,29 @@
+package org.infinispan.protostream.types.java;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.infinispan.protostream.WrappedMessage;
+import org.infinispan.protostream.annotations.ProtoAdapter;
+import org.infinispan.protostream.annotations.ProtoFactory;
+import org.infinispan.protostream.annotations.ProtoField;
+
+@ProtoAdapter(
+      value = List.class,
+      subClassNames = {
+            "java.util.ImmutableCollections$ListN",
+            "java.util.ImmutableCollections$List12"
+      }
+)
+public class ListOfAdapter {
+   @ProtoFactory
+   List<?> create(WrappedMessage[] elements) {
+      return elements == null ? null :
+      List.of(Arrays.stream(elements).map(WrappedMessage::getValue).toArray());
+   }
+
+   @ProtoField(1)
+   WrappedMessage[] elements(List<?> list) {
+      return list == null ? null : list.stream().map(WrappedMessage::new).toArray(WrappedMessage[]::new);
+   }
+}
diff --git a/types/src/test/java/org/infinispan/protostream/types/java/ListSchema.java b/types/src/test/java/org/infinispan/protostream/types/java/ListSchema.java
@@ -0,0 +1,16 @@
+package org.infinispan.protostream.types.java;
+
+import org.infinispan.protostream.GeneratedSchema;
+import org.infinispan.protostream.annotations.ProtoSchema;
+
+@ProtoSchema(
+      includeClasses = {
+            ListOfAdapter.class,
+            EmptyListAdapter.class,
+            SingletonListAdapter.class
+      },
+      schemaFileName = "list.proto",
+      schemaFilePath = "proto/",
+      schemaPackageName = "collections")
+public interface ListSchema extends GeneratedSchema {
+}
diff --git a/types/src/test/java/org/infinispan/protostream/types/java/SingletonListAdapter.java b/types/src/test/java/org/infinispan/protostream/types/java/SingletonListAdapter.java
@@ -0,0 +1,27 @@
+package org.infinispan.protostream.types.java;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.infinispan.protostream.WrappedMessage;
+import org.infinispan.protostream.annotations.ProtoAdapter;
+import org.infinispan.protostream.annotations.ProtoFactory;
+import org.infinispan.protostream.annotations.ProtoField;
+
+@ProtoAdapter(
+      value = List.class,
+      subClassNames = "java.util.Collections$SingletonList"
+)
+public class SingletonListAdapter {
+
+   @ProtoFactory
+   List<?> create(WrappedMessage element) {
+      return Collections.singletonList(element == null ? null : element.getValue());
+   }
+
+   @ProtoField(1)
+   WrappedMessage getElement(List<?> list) {
+      var val = list.get(0);
+      return val == null ? null : new WrappedMessage(val);
+   }
+}
diff --git a/types/src/test/java/org/infinispan/protostream/types/java/TypesMarshallingTest.java b/types/src/test/java/org/infinispan/protostream/types/java/TypesMarshallingTest.java
@@ -26,6 +26,7 @@
 import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
@@ -42,6 +43,7 @@
 import org.infinispan.protostream.SerializationContext;
 import org.infinispan.protostream.config.Configuration;
 import org.infinispan.protostream.impl.Log;
+import org.junit.Assume;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -217,6 +219,17 @@ public void testZonedTime() throws IOException {
       testConfiguration.method.marshallAndUnmarshallTest(time, context, false);
    }
 
+   @Test
+   public void testMultipleAdaptersForInterface() throws IOException {
+      // Skip JSON test as WrappedMessage instances are not serialized/parsed with JSON correctly
+      // https://github.com/infinispan/protostream/issues/379
+      Assume.assumeFalse(testConfiguration.method == MarshallingMethodType.JSON);
+      testConfiguration.method.marshallAndUnmarshallTest(Collections.emptyList(), context, false);
+      testConfiguration.method.marshallAndUnmarshallTest(Collections.singletonList("1"), context, false);
+      testConfiguration.method.marshallAndUnmarshallTest(List.of(), context, false);
+      testConfiguration.method.marshallAndUnmarshallTest(List.of(1), context, false);
+   }
+
    @FunctionalInterface
    public interface MarshallingMethod {
       void marshallAndUnmarshallTest(Object original, ImmutableSerializationContext ctx, boolean isArray) throws IOException;
@@ -233,6 +246,7 @@ private static ImmutableSerializationContext newContext(boolean wrapCollectionEl
       register(new CommonTypesSchema(), ctx);
       register(new CommonContainerTypesSchema(), ctx);
       register(new BookSchemaImpl(), ctx);
+      register(new ListSchemaImpl(), ctx);
       return ctx;
    }