Merge pull request #100 from salesforce/bugfix/multi-files

Correctly handle nested types when java_multiple_files=true

Author: rmichela

jprotoc/jprotoc-test/src/test/java/com/salesforce/jprotoc/ProtoTypeMapTest.java

@@ -1,6 +1,7 @@
 package com.salesforce.jprotoc;
 
 import com.google.common.io.ByteStreams;
+import com.google.protobuf.DescriptorProtos;
 import com.google.protobuf.ExtensionRegistry;
 import com.google.protobuf.compiler.PluginProtos;
 import org.junit.BeforeClass;
@@ -9,6 +10,7 @@
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -29,7 +31,8 @@ public static void buildProtoTypeMap() throws IOException {
         byte[] generatorRequestBytes = ByteStreams.toByteArray(new FileInputStream(new File(dumpPath)));
         PluginProtos.CodeGeneratorRequest request = PluginProtos.CodeGeneratorRequest.parseFrom(
                 generatorRequestBytes, ExtensionRegistry.newInstance());
-        protoTypeMap =  ProtoTypeMap.of(request.getProtoFileList());
+        List<DescriptorProtos.FileDescriptorProto> fileProtos = request.getProtoFileList();
+        protoTypeMap =  ProtoTypeMap.of(fileProtos);
     }
 
 
@@ -60,6 +63,18 @@ public void nestedTypeMappings() {
         assertProtoTypeMapping(".nested.Outer.MiddleBB.Inner", nested.NestedOuterClass.Outer.MiddleBB.Inner.class);
     }
 
+    /**
+     * Verify that nested proto message types map correctly when {@code option java_multiple_files = true}.
+     */
+    @Test
+    public void nestedTypeMappingsMultipleFiles() {
+        assertProtoTypeMapping(".nested_multiple_files.Outer", nested_multiple_files.Outer.class);
+        assertProtoTypeMapping(".nested_multiple_files.Outer.MiddleAA", nested_multiple_files.Outer.MiddleAA.class);
+        assertProtoTypeMapping(".nested_multiple_files.Outer.MiddleAA.Inner", nested_multiple_files.Outer.MiddleAA.Inner.class);
+        assertProtoTypeMapping(".nested_multiple_files.Outer.MiddleBB", nested_multiple_files.Outer.MiddleBB.class);
+        assertProtoTypeMapping(".nested_multiple_files.Outer.MiddleBB.Inner", nested_multiple_files.Outer.MiddleBB.Inner.class);
+    }
+
     /**
      * Verify that types with nested enums sharing the same name as a top-level type don't conflict.
      */

jprotoc/jprotoc-test/src/test/proto/nested_multiple_files.proto

@@ -0,0 +1,32 @@
+syntax = "proto3";
+
+package nested_multiple_files;
+
+option java_multiple_files = true;
+
+message Outer {  // Level 0
+    enum FooEnum {
+        FOO = 0;
+        BAR = 1;
+        CHEESE = 2;
+    }
+    message MiddleAA {  // Level 1
+
+        message Inner {   // Level 2
+            int64 ival = 1;
+            bool  booly = 2;
+            Outer.FooEnum enum = 3;
+        }
+    }
+    message MiddleBB {  // Level 1
+        message Inner {   // Level 2
+            int32 ival = 1;
+            bool  booly = 2;
+            Outer.FooEnum enum = 3;
+        }
+    }
+}
+
+service Nested {
+    rpc doNested (Outer.MiddleAA.Inner) returns (Outer.MiddleBB.Inner) {}
+}

jprotoc/jprotoc/src/main/java/com/salesforce/jprotoc/ProtoTypeMap.java

@@ -14,13 +14,13 @@
 import java.util.Collection;
 
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 
 /**
  * {@code ProtoTypeMap} maintains a dictionary for looking up Java type names when given proto types.
  */
 public final class ProtoTypeMap {
 
+    private static final Joiner DOT_JOINER = Joiner.on('.').skipNulls();
     private final ImmutableMap<String, String> types;
 
     private ProtoTypeMap(@Nonnull ImmutableMap<String, String> types) {
@@ -58,7 +58,7 @@ public static ProtoTypeMap of(@Nonnull Collection<DescriptorProtos.FileDescripto
             fileDescriptor.getEnumTypeList().forEach(
                 e -> types.put(
                         protoPackage + "." + e.getName(),
-                        toJavaTypeName(e.getName(), enclosingClassName, javaPackage)));
+                        DOT_JOINER.join(javaPackage, enclosingClassName, e.getName())));
 
             // Identify top-level messages, and nested types
             fileDescriptor.getMessageTypeList().forEach(
@@ -74,23 +74,21 @@ private static void recursivelyAddTypes(ImmutableMap.Builder<String, String> typ
         String protoTypeName = protoPackage + "." + m.getName();
         types.put(
             protoTypeName,
-            toJavaTypeName(m.getName(), enclosingClassName, javaPackage));
+            DOT_JOINER.join(javaPackage, enclosingClassName, m.getName()));
 
         // Identify any nested Enums
         m.getEnumTypeList().forEach(
             e -> types.put(
                 protoPackage + "." + m.getName() + "." + e.getName(),
-                toJavaTypeName(e.getName(),
-                enclosingClassName + "." + m.getName(),
-                javaPackage)));
+                DOT_JOINER.join(javaPackage, enclosingClassName, m.getName(), e.getName())));
 
         // Recursively identify any nested types
         m.getNestedTypeList().forEach(
             n -> recursivelyAddTypes(
                 types,
                 n,
                 protoPackage + "." + m.getName(),
-                enclosingClassName + "." + m.getName(),
+                DOT_JOINER.join(enclosingClassName, m.getName()),
                 javaPackage));
     }
 
@@ -104,24 +102,6 @@ public String toJavaTypeName(@Nonnull String protoTypeName) {
         return types.get(protoTypeName);
     }
 
-    /**
-     * Returns the full Java type name based on the given protobuf type parameters.
-     *
-     * @param className the protobuf type name
-     * @param enclosingClassName the optional enclosing class for the given type
-     * @param javaPackage the proto file's configured java package name
-     */
-    public static String toJavaTypeName(
-            @Nonnull String className,
-            @Nullable String enclosingClassName,
-            @Nullable String javaPackage) {
-
-        Preconditions.checkNotNull(className, "className");
-
-        Joiner dotJoiner = Joiner.on('.').skipNulls();
-        return dotJoiner.join(javaPackage, enclosingClassName, className);
-    }
-
     private static String getJavaOuterClassname(
             DescriptorProtos.FileDescriptorProto fileDescriptor,
             DescriptorProtos.FileOptions fileOptions) {