salesforce / grpc-java-contrib

Useful extensions for the grpc-java library
BSD 3-Clause "New" or "Revised" License
220 stars 34 forks source link

jprotoc version 0.8.1 IllegalArgumentException from ProtoTypeMap.of() #91

Closed wfhartford closed 5 years ago

wfhartford commented 5 years ago

An upgrade to version 0.8.1 of jprotoc caused a new error in my build because of a duplicate entry in the ImmutableMap. Error output is:

  java.lang.IllegalArgumentException: Multiple entries with same key: .google.protobuf.Type=com.google.protobuf.DescriptorProtos.Type and .google.protobuf.Type=com.google.protobuf.Type
        at com.google.common.collect.ImmutableMap.checkNoConflict(ImmutableMap.java:136)
        at com.google.common.collect.RegularImmutableMap.checkNoConflictInKeyBucket(RegularImmutableMap.java:100)
        at com.google.common.collect.RegularImmutableMap.fromEntryArray(RegularImmutableMap.java:86)
        at com.google.common.collect.ImmutableMap$Builder.build(ImmutableMap.java:300)
        at com.salesforce.jprotoc.ProtoTypeMap.of(ProtoTypeMap.java:69)
        at io.rouz.grpc.kotlin.GrpcKotlinGenerator.generate(GrpcKotlinGenerator.java:65)
        at com.salesforce.jprotoc.ProtocPlugin.lambda$generate$0(ProtocPlugin.java:77)
        at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:271)
        at java.base/java.util.Collections$2.tryAdvance(Collections.java:4727)
        at java.base/java.util.Collections$2.forEachRemaining(Collections.java:4735)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
        at java.base/java.util.stream.StreamSpliterators$WrappingSpliterator.forEachRemaining(StreamSpliterators.java:312)
        at java.base/java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:734)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
        at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
        at com.salesforce.jprotoc.ProtocPlugin.generate(ProtocPlugin.java:86)
        at com.salesforce.jprotoc.ProtocPlugin.generate(ProtocPlugin.java:45)
        at com.salesforce.jprotoc.ProtocPlugin.generate(ProtocPlugin.java:37)
        at io.rouz.grpc.kotlin.GrpcKotlinGenerator.main(GrpcKotlinGenerator.java:59)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:564)
        at org.springframework.boot.loader.MainMethodRunner.run(MainMethodRunner.java:48)
        at org.springframework.boot.loader.Launcher.launch(Launcher.java:87)
        at org.springframework.boot.loader.Launcher.launch(Launcher.java:50)
        at org.springframework.boot.loader.JarLauncher.main(JarLauncher.java:51)
  --grpc-kotlin_out: protoc-gen-grpc-kotlin: Plugin failed with status code 1.

I will do some further investigation and try to make a minimal reproduction case. My project is using the grpc-kotlin plugin which in turn uses jprotoc. I've tested with version 0.7.1 and 0.8.0, both of which do not produce the above error, only version 0.8.1 does.

rmichela commented 5 years ago

Can you provide the minimum set of protos to reproduce the problem? I can build a unit test with them. Protoc has a bunch of undocumented edge cases.

On Mon, Oct 29, 2018 at 4:13 PM Wesley Hartford notifications@github.com wrote:

An upgrade to version 0.8.1 of jprotoc caused a new error in my build because of a duplicate entry in the ImmutableMap. Error output is:

java.lang.IllegalArgumentException: Multiple entries with same key: .google.protobuf.Type=com.google.protobuf.DescriptorProtos.Type and .google.protobuf.Type=com.google.protobuf.Type at com.google.common.collect.ImmutableMap.checkNoConflict(ImmutableMap.java:136) at com.google.common.collect.RegularImmutableMap.checkNoConflictInKeyBucket(RegularImmutableMap.java:100) at com.google.common.collect.RegularImmutableMap.fromEntryArray(RegularImmutableMap.java:86) at com.google.common.collect.ImmutableMap$Builder.build(ImmutableMap.java:300) at com.salesforce.jprotoc.ProtoTypeMap.of(ProtoTypeMap.java:69) at io.rouz.grpc.kotlin.GrpcKotlinGenerator.generate(GrpcKotlinGenerator.java:65) at com.salesforce.jprotoc.ProtocPlugin.lambda$generate$0(ProtocPlugin.java:77) at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:271) at java.base/java.util.Collections$2.tryAdvance(Collections.java:4727) at java.base/java.util.Collections$2.forEachRemaining(Collections.java:4735) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) at java.base/java.util.stream.StreamSpliterators$WrappingSpliterator.forEachRemaining(StreamSpliterators.java:312) at java.base/java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:734) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578) at com.salesforce.jprotoc.ProtocPlugin.generate(ProtocPlugin.java:86) at com.salesforce.jprotoc.ProtocPlugin.generate(ProtocPlugin.java:45) at com.salesforce.jprotoc.ProtocPlugin.generate(ProtocPlugin.java:37) at io.rouz.grpc.kotlin.GrpcKotlinGenerator.main(GrpcKotlinGenerator.java:59) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at org.springframework.boot.loader.MainMethodRunner.run(MainMethodRunner.java:48) at org.springframework.boot.loader.Launcher.launch(Launcher.java:87) at org.springframework.boot.loader.Launcher.launch(Launcher.java:50) at org.springframework.boot.loader.JarLauncher.main(JarLauncher.java:51) --grpc-kotlin_out: protoc-gen-grpc-kotlin: Plugin failed with status code 1.

I will do some further investigation and try to make a minimal reproduction case. My project is using the grpc-kotlin https://github.com/rouzwawi/grpc-kotlin plugin which in turn uses jprotoc. I've tested with version 0.7.1 and 0.8.0, both of which do not produce the above error, only version 0.8.1 does.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/salesforce/grpc-java-contrib/issues/91, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKPNQ3Ao19ItDdySXhQSGO42hjZmkcnks5up4uAgaJpZM4YA1lN .

-- Ryan Michela Principal Member of Technical Staff | Salesforce 

http://smart.salesforce.com/sig/rmichela//us_mb/default/link.html

wfhartford commented 5 years ago

I've almost got a minimal set to show you, but the main cause seems to be a dependency relationship between two projects containing .proto files.

Project base has a .proto file which includes some message types used by other projects; project importer depends on that project to use the base message types within its own. The issue is caused by the inter project dependency not the import within the .proto file.

wfhartford commented 5 years ago

A minimal reproducing gradle project is available at https://github.com/wfhartford/jprotoc-bug-reproduce

This project is pretty much what I described in my previous comment. It uses jprotoc version 0.8.1 via the grpc-kotlin plugin version 0.0.4. Switch this plugin to version 0.0.2 to revert to jprotoc 0.7.1 and the build will pass. I have also tested with jprotoc 0.8.0 which passes.

To build the project, simply run ./gradlew build in the current configuration this will fail, showing the error message above. The relavent code from the grpc-kotlin plugin is at https://github.com/rouzwawi/grpc-kotlin/blob/master/grpc-kotlin-gen/src/main/java/io/rouz/grpc/kotlin/GrpcKotlinGenerator.java:

public class GrpcKotlinGenerator extends Generator {
  public static void main(String[] args) {
    ProtocPlugin.generate(new GrpcKotlinGenerator());
  }

  @Override
  public List<CodeGeneratorResponse.File> generateFiles(CodeGeneratorRequest request)
      throws GeneratorException {
    // Exception thrown from this call
    final ProtoTypeMap typeMap = ProtoTypeMap.of(request.getProtoFileList());

    // ...
  }
wfhartford commented 5 years ago

By the way, I checked the request object passed to generateFiles() and the object is identical in versions 0.7.1 and 0.8.1.

pusolito commented 5 years ago

I'm also seeing this issue. Any updates?

rmichela commented 5 years ago

Ok. I've been digging into this. Sorry for the delay. I had a baby shortly after this bug was submitted. 👶 🍼

The problem is ultimately caused by a behavior of protobuf-gradle-plugin. Swap the protobuf dependency to a compile dependency. When a protobuf dependency is used, protobuf-gradle-plugin ends up passing the standard protobuf protos to protoc twice, once through the extracted-protos directory and once through the extracted-include-protos.

This behavior has always been there and is arguably correct for protobuf-gradle-plugin. I'm still digging into why jprotoc started having problems.

rmichela commented 5 years ago

Further digging shows a nested Enum with the same name as a top-level type will trigger this bug.

syntax = "proto3";

package nested_overlap;

message Outer {  
     enum Foo {
         FOO = 0;
         BAR = 1;
         CHEESE = 2;
     }
     Outer.Foo e = 1;
}

message Foo {
    string bar = 1;
}

service Nested {
    rpc doNested (Outer) returns (Foo) {}
}