fix: [gapic-generator-java] handle shortname type ambiguity in LRO parsing

gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java

@@ -824,39 +824,34 @@ static LongrunningOperation parseLro(
     boolean isResponseTypeNameShortOnly = lastDotIndex < 0;
     String responseTypeShortName =
         lastDotIndex >= 0 ? responseTypeName.substring(lastDotIndex + 1) : responseTypeName;
+    // When only shortname is provided, match on same proto package as method (See
+    // https://aip.dev/151)
+    String responseTypeFullName =
+        isResponseTypeNameShortOnly
+            ? methodDescriptor.getFile().getPackage() + "." + responseTypeShortName
+            : responseTypeName;
 
     lastDotIndex = metadataTypeName.lastIndexOf('.');
     boolean isMetadataTypeNameShortOnly = lastDotIndex < 0;
     String metadataTypeShortName =
         lastDotIndex >= 0 ? metadataTypeName.substring(lastDotIndex + 1) : metadataTypeName;
+    // When only shortname is provided, match on same proto package as method (See
+    // https://aip.dev/151)
+    String metadataTypeFullName =
+        isMetadataTypeNameShortOnly
+            ? methodDescriptor.getFile().getPackage() + "." + metadataTypeShortName
+            : metadataTypeName;
 
     // The messageTypes map keys to the Java fully-qualified name.
     for (Map.Entry<String, Message> messageEntry : messageTypes.entrySet()) {
-      String messageKey = messageEntry.getKey();
-      int messageLastDotIndex = messageEntry.getKey().lastIndexOf('.');
-      String messageShortName =
-          messageLastDotIndex >= 0 ? messageKey.substring(messageLastDotIndex + 1) : messageKey;
-      if (responseMessage == null) {
-        if (isResponseTypeNameShortOnly && responseTypeName.equals(messageShortName)) {
-          responseMessage = messageEntry.getValue();
-        } else if (!isResponseTypeNameShortOnly && responseTypeShortName.equals(messageShortName)) {
-          // Ensure that the full proto name matches.
-          Message candidateMessage = messageEntry.getValue();
-          if (candidateMessage.fullProtoName().equals(responseTypeName)) {
-            responseMessage = candidateMessage;
-          }
-        }
+      Message candidateMessage = messageEntry.getValue();
+      if (responseMessage == null
+          && candidateMessage.fullProtoName().equals(responseTypeFullName)) {
+        responseMessage = candidateMessage;
       }
-      if (metadataMessage == null) {
-        if (isMetadataTypeNameShortOnly && metadataTypeName.equals(messageShortName)) {
-          metadataMessage = messageEntry.getValue();
-        } else if (!isMetadataTypeNameShortOnly && metadataTypeShortName.equals(messageShortName)) {
-          // Ensure that the full proto name matches.
-          Message candidateMessage = messageEntry.getValue();
-          if (candidateMessage.fullProtoName().equals(metadataTypeName)) {
-            metadataMessage = candidateMessage;
-          }
-        }
+      if (metadataMessage == null
+          && candidateMessage.fullProtoName().equals(metadataTypeFullName)) {
+        metadataMessage = candidateMessage;
       }
     }
 

gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposerTest.java

@@ -82,4 +82,14 @@ public void generateGrpcServiceStubClass_createBatchingCallable() {
     Assert.assertGoldenClass(this.getClass(), clazz, "GrpcLoggingStub.golden");
     Assert.assertEmptySamples(clazz.samples());
   }
+
+  @Test
+  public void generateGrpcServiceStubClass_typeConflict() {
+    GapicContext context = GrpcTestProtoLoader.instance().parseTypeConflictTesting();
+    Service testProtoService = context.services().get(0);
+    GapicClass clazz = GrpcServiceStubClassComposer.instance().generate(context, testProtoService);
+
+    Assert.assertGoldenClass(this.getClass(), clazz, "GrpcTypeConflictTestingStub.golden");
+    Assert.assertEmptySamples(clazz.samples());
+  }
 }

gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcTypeConflictTestingStub.golden

@@ -0,0 +1,154 @@
+package com.google.testgapic.v1beta1.stub;
+
+import com.google.api.core.BetaApi;
+import com.google.api.gax.core.BackgroundResource;
+import com.google.api.gax.core.BackgroundResourceAggregation;
+import com.google.api.gax.grpc.GrpcCallSettings;
+import com.google.api.gax.grpc.GrpcStubCallableFactory;
+import com.google.api.gax.rpc.ClientContext;
+import com.google.api.gax.rpc.OperationCallable;
+import com.google.api.gax.rpc.UnaryCallable;
+import com.google.longrunning.Operation;
+import com.google.longrunning.stub.GrpcOperationsStub;
+import com.google.testgapic.v1beta1.Annotation;
+import com.google.testgapic.v1beta1.Location;
+import com.google.testgapic.v1beta1.Request;
+import io.grpc.MethodDescriptor;
+import io.grpc.protobuf.ProtoUtils;
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+import javax.annotation.Generated;
+
+// AUTO-GENERATED DOCUMENTATION AND CLASS.
+/**
+ * gRPC stub implementation for the TypeConflictTesting service API.
+ *
+ * <p>This class is for advanced usage and reflects the underlying API directly.
+ */
+@BetaApi
+@Generated("by gapic-generator-java")
+public class GrpcTypeConflictTestingStub extends TypeConflictTestingStub {
+  private static final MethodDescriptor<Request, Operation> annotationTestMethodDescriptor =
+      MethodDescriptor.<Request, Operation>newBuilder()
+          .setType(MethodDescriptor.MethodType.UNARY)
+          .setFullMethodName("google.testgapic.v1beta1.TypeConflictTesting/annotationTest")
+          .setRequestMarshaller(ProtoUtils.marshaller(Request.getDefaultInstance()))
+          .setResponseMarshaller(ProtoUtils.marshaller(Operation.getDefaultInstance()))
+          .build();
+
+  private final UnaryCallable<Request, Operation> annotationTestCallable;
+  private final OperationCallable<Request, Annotation, Location> annotationTestOperationCallable;
+
+  private final BackgroundResource backgroundResources;
+  private final GrpcOperationsStub operationsStub;
+  private final GrpcStubCallableFactory callableFactory;
+
+  public static final GrpcTypeConflictTestingStub create(TypeConflictTestingStubSettings settings)
+      throws IOException {
+    return new GrpcTypeConflictTestingStub(settings, ClientContext.create(settings));
+  }
+
+  public static final GrpcTypeConflictTestingStub create(ClientContext clientContext)
+      throws IOException {
+    return new GrpcTypeConflictTestingStub(
+        TypeConflictTestingStubSettings.newBuilder().build(), clientContext);
+  }
+
+  public static final GrpcTypeConflictTestingStub create(
+      ClientContext clientContext, GrpcStubCallableFactory callableFactory) throws IOException {
+    return new GrpcTypeConflictTestingStub(
+        TypeConflictTestingStubSettings.newBuilder().build(), clientContext, callableFactory);
+  }
+
+  /**
+   * Constructs an instance of GrpcTypeConflictTestingStub, using the given settings. This is
+   * protected so that it is easy to make a subclass, but otherwise, the static factory methods
+   * should be preferred.
+   */
+  protected GrpcTypeConflictTestingStub(
+      TypeConflictTestingStubSettings settings, ClientContext clientContext) throws IOException {
+    this(settings, clientContext, new GrpcTypeConflictTestingCallableFactory());
+  }
+
+  /**
+   * Constructs an instance of GrpcTypeConflictTestingStub, using the given settings. This is
+   * protected so that it is easy to make a subclass, but otherwise, the static factory methods
+   * should be preferred.
+   */
+  protected GrpcTypeConflictTestingStub(
+      TypeConflictTestingStubSettings settings,
+      ClientContext clientContext,
+      GrpcStubCallableFactory callableFactory)
+      throws IOException {
+    this.callableFactory = callableFactory;
+    this.operationsStub = GrpcOperationsStub.create(clientContext, callableFactory);
+
+    GrpcCallSettings<Request, Operation> annotationTestTransportSettings =
+        GrpcCallSettings.<Request, Operation>newBuilder()
+            .setMethodDescriptor(annotationTestMethodDescriptor)
+            .build();
+
+    this.annotationTestCallable =
+        callableFactory.createUnaryCallable(
+            annotationTestTransportSettings, settings.annotationTestSettings(), clientContext);
+    this.annotationTestOperationCallable =
+        callableFactory.createOperationCallable(
+            annotationTestTransportSettings,
+            settings.annotationTestOperationSettings(),
+            clientContext,
+            operationsStub);
+
+    this.backgroundResources =
+        new BackgroundResourceAggregation(clientContext.getBackgroundResources());
+  }
+
+  public GrpcOperationsStub getOperationsStub() {
+    return operationsStub;
+  }
+
+  @Override
+  public UnaryCallable<Request, Operation> annotationTestCallable() {
+    return annotationTestCallable;
+  }
+
+  @Override
+  public OperationCallable<Request, Annotation, Location> annotationTestOperationCallable() {
+    return annotationTestOperationCallable;
+  }
+
+  @Override
+  public final void close() {
+    try {
+      backgroundResources.close();
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new IllegalStateException("Failed to close resource", e);
+    }
+  }
+
+  @Override
+  public void shutdown() {
+    backgroundResources.shutdown();
+  }
+
+  @Override
+  public boolean isShutdown() {
+    return backgroundResources.isShutdown();
+  }
+
+  @Override
+  public boolean isTerminated() {
+    return backgroundResources.isTerminated();
+  }
+
+  @Override
+  public void shutdownNow() {
+    backgroundResources.shutdownNow();
+  }
+
+  @Override
+  public boolean awaitTermination(long duration, TimeUnit unit) throws InterruptedException {
+    return backgroundResources.awaitTermination(duration, unit);
+  }
+}

gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java

@@ -39,6 +39,7 @@
 import com.google.showcase.v1beta1.EchoOuterClass;
 import com.google.showcase.v1beta1.TestingOuterClass;
 import com.google.testgapic.v1beta1.LockerProto;
+import com.google.testgapic.v1beta1.TypeConflictTestingProto;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
@@ -194,6 +195,30 @@ public void parseMethods_basicLro() {
     assertThat(waitMethod.lro().metadataType()).isEqualTo(waitMetadataType);
   }
 
+  @Test
+  public void parseMethods_typeConflictLro() {
+    String testPackage = "com.google.testgapic.v1beta1";
+    FileDescriptor testFileDescriptor = TypeConflictTestingProto.getDescriptor();
+    ServiceDescriptor testService = testFileDescriptor.getServices().get(0);
+    assertEquals("TypeConflictTesting", testService.getName());
+    Map<String, Message> messageTypes = Parser.parseMessages(testFileDescriptor);
+    Map<String, ResourceName> resourceNames = Parser.parseResourceNames(testFileDescriptor);
+    Set<ResourceName> outputResourceNames = new HashSet<>();
+    List<Method> methods =
+        Parser.parseMethods(
+            testService,
+            testPackage,
+            messageTypes,
+            resourceNames,
+            Optional.empty(),
+            outputResourceNames,
+            Transport.GRPC);
+
+    Method firstMethod = methods.get(0);
+    TypeNode annotationType = messageTypes.get("com.google.testgapic.v1beta1.Annotation").type();
+    assertThat(firstMethod.lro().responseType()).isEqualTo(annotationType);
+  }
+
   @Test
   public void parseLro_missingResponseType() {
     Map<String, Message> messageTypes = Parser.parseMessages(echoFileDescriptor);

gapic-generator-java/src/test/java/com/google/api/generator/test/protoloader/TestProtoLoader.java

@@ -33,6 +33,7 @@
 import com.google.logging.v2.LoggingConfigProto;
 import com.google.logging.v2.LoggingMetricsProto;
 import com.google.logging.v2.LoggingProto;
+import com.google.protobuf.DescriptorProtos;
 import com.google.protobuf.Descriptors.FileDescriptor;
 import com.google.protobuf.Descriptors.ServiceDescriptor;
 import com.google.pubsub.v1.PubsubProto;
@@ -41,6 +42,7 @@
 import com.google.showcase.v1beta1.MessagingOuterClass;
 import com.google.showcase.v1beta1.TestingOuterClass;
 import com.google.testdata.v1.DeprecatedServiceOuterClass;
+import com.google.testgapic.v1beta1.TypeConflictTestingProto;
 import google.cloud.CommonResources;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -357,6 +359,35 @@ public GapicContext parseLogging() {
         .build();
   }
 
+  public GapicContext parseTypeConflictTesting() {
+    FileDescriptor testingFileDescriptor = TypeConflictTestingProto.getDescriptor();
+    FileDescriptor descriptorProtoFileDescriptor = DescriptorProtos.getDescriptor();
+    ServiceDescriptor testingService = testingFileDescriptor.getServices().get(0);
+    assertEquals(testingService.getName(), "TypeConflictTesting");
+
+    Map<String, Message> messageTypes = Parser.parseMessages(testingFileDescriptor);
+    messageTypes.putAll(Parser.parseMessages(descriptorProtoFileDescriptor));
+    Map<String, ResourceName> resourceNames = Parser.parseResourceNames(testingFileDescriptor);
+    messageTypes = Parser.updateResourceNamesInMessages(messageTypes, resourceNames.values());
+
+    Set<ResourceName> outputResourceNames = new HashSet<>();
+    List<Service> services =
+        Parser.parseService(
+            testingFileDescriptor,
+            messageTypes,
+            resourceNames,
+            Optional.empty(),
+            outputResourceNames);
+
+    return GapicContext.builder()
+        .setMessages(messageTypes)
+        .setResourceNames(resourceNames)
+        .setServices(services)
+        .setHelperResourceNames(outputResourceNames)
+        .setTransport(transport)
+        .build();
+  }
+
   public String getTestFilesDirectory() {
     return testFilesDirectory;
   }

gapic-generator-java/src/test/proto/type_conflict_testing.proto

@@ -0,0 +1,50 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+syntax = "proto3";
+
+import "google/api/client.proto";
+import "google/longrunning/operations.proto";
+
+package google.testgapic.v1beta1;
+
+option java_package = "com.google.testgapic.v1beta1";
+option java_outer_classname = "TypeConflictTestingProto";
+option java_multiple_files = true;
+
+service TypeConflictTesting {
+
+  option (google.api.default_host) = "localhost:7469";
+
+  rpc annotationTest(Request) returns (google.longrunning.Operation) {
+    option (google.longrunning.operation_info) = {
+      response_type: "Annotation"
+      metadata_type: "Location"
+    };
+  }
+}
+
+message Request {
+  string parent = 1;
+  Annotation annotation = 2;
+  Location location = 3;
+}
+
+message Annotation {
+  string name = 1;
+}
+
+message Location {
+  string name = 1;
+}