diff --git a/jprotoc/jprotoc-test/src/test/java/com/salesforce/jprotoc/ProtoTypeMapTest.java b/jprotoc/jprotoc-test/src/test/java/com/salesforce/jprotoc/ProtoTypeMapTest.java index caf994b9..b77f3913 100644 --- a/jprotoc/jprotoc-test/src/test/java/com/salesforce/jprotoc/ProtoTypeMapTest.java +++ b/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 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. */ diff --git a/jprotoc/jprotoc-test/src/test/proto/nested_multiple_files.proto b/jprotoc/jprotoc-test/src/test/proto/nested_multiple_files.proto new file mode 100644 index 00000000..b05ffec6 --- /dev/null +++ b/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) {} +} \ No newline at end of file diff --git a/jprotoc/jprotoc/src/main/java/com/salesforce/jprotoc/ProtoTypeMap.java b/jprotoc/jprotoc/src/main/java/com/salesforce/jprotoc/ProtoTypeMap.java index 48b4fc9a..680a8710 100644 --- a/jprotoc/jprotoc/src/main/java/com/salesforce/jprotoc/ProtoTypeMap.java +++ b/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 types; private ProtoTypeMap(@Nonnull ImmutableMap types) { @@ -58,7 +58,7 @@ public static ProtoTypeMap of(@Nonnull Collection 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,15 +74,13 @@ private static void recursivelyAddTypes(ImmutableMap.Builder 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( @@ -90,7 +88,7 @@ private static void recursivelyAddTypes(ImmutableMap.Builder typ 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) { diff --git a/jprotoc/jprotoc/src/test/java/com/salesforce/jprotoc/ProtoTypeUtilsTest.java b/jprotoc/jprotoc/src/test/java/com/salesforce/jprotoc/ProtoTypeUtilsTest.java deleted file mode 100644 index ab86cda9..00000000 --- a/jprotoc/jprotoc/src/test/java/com/salesforce/jprotoc/ProtoTypeUtilsTest.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright (c) 2019, Salesforce.com, Inc. - * All rights reserved. - * Licensed under the BSD 3-Clause license. - * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause - */ - -package com.salesforce.jprotoc; - -import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; -import static org.assertj.core.api.Assertions.assertThat; -import org.junit.Test; - -import java.util.concurrent.ThreadLocalRandom; - -public class ProtoTypeUtilsTest { - - @Test - public void classWithEnclosingClassAndJavaPackage() { - final String className = randomAlphabetic(ThreadLocalRandom.current().nextInt(5, 10)); - final String enclosingClassName = randomAlphabetic(ThreadLocalRandom.current().nextInt(5, 10)); - final String javaPackage = randomAlphabetic(ThreadLocalRandom.current().nextInt(5, 10)); - - assertThat(ProtoTypeMap.toJavaTypeName(className, enclosingClassName, javaPackage)) - .isEqualTo(javaPackage + "." + enclosingClassName + "." + className); - } - - @Test - public void classWithEnclosingClass() { - final String className = randomAlphabetic(ThreadLocalRandom.current().nextInt(5, 10)); - final String enclosingClassName = randomAlphabetic(ThreadLocalRandom.current().nextInt(5, 10)); - - assertThat(ProtoTypeMap.toJavaTypeName(className, enclosingClassName, null)) - .isEqualTo(enclosingClassName + "." + className); - } - - @Test - public void classWithJavaPackage() { - final String className = randomAlphabetic(ThreadLocalRandom.current().nextInt(5, 10)); - final String javaPackage = randomAlphabetic(ThreadLocalRandom.current().nextInt(5, 10)); - - assertThat(ProtoTypeMap.toJavaTypeName(className, null, javaPackage)) - .isEqualTo(javaPackage + "." + className); - } -}