[ggj][codegen] fix: support non-name fields with res-refs in resname def parsing

src/main/java/com/google/api/generator/gapic/protoparser/ResourceNameParser.java

@@ -22,9 +22,11 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
+import com.google.protobuf.DescriptorProtos.FieldOptions;
 import com.google.protobuf.DescriptorProtos.FileOptions;
 import com.google.protobuf.DescriptorProtos.MessageOptions;
 import com.google.protobuf.Descriptors.Descriptor;
+import com.google.protobuf.Descriptors.FieldDescriptor;
 import com.google.protobuf.Descriptors.FileDescriptor;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -107,12 +109,25 @@ static Optional<ResourceName> parseResourceNameFromMessageType(
     }
 
     ResourceDescriptor protoResource = messageOptions.getExtension(ResourceProto.resource);
-    // aip.dev/4231.
+    // Validation - check that a resource name field is present.
     if (Strings.isNullOrEmpty(protoResource.getNameField())) {
-      Preconditions.checkNotNull(
-          messageTypeDescriptor.findFieldByName(ResourceNameConstants.NAME_FIELD_NAME),
+      // aip.dev/4231
+      boolean resourceNameFieldFound =
+          messageTypeDescriptor.findFieldByName(ResourceNameConstants.NAME_FIELD_NAME) != null;
+      // If this is null, look for a field with a resource reference is found.
+      // Example: AccountBudgetProposal.
+      for (FieldDescriptor fieldDescriptor : messageTypeDescriptor.getFields()) {
+        FieldOptions fieldOptions = fieldDescriptor.getOptions();
+        if (fieldOptions.hasExtension(ResourceProto.resourceReference)) {
+          resourceNameFieldFound = true;
+          break;
+        }
+      }
+      Preconditions.checkState(
+          resourceNameFieldFound,
           String.format(
-              "Message %s has a resource annotation but no \"name\" field",
+              "Message %s has a resource annotation but no field titled \"name\" or containing a"
+                  + " resource reference",
               messageTypeDescriptor.getName()));
     }
 

src/test/java/com/google/api/generator/gapic/protoparser/ResourceNameParserTest.java

@@ -125,7 +125,7 @@ public void parseResourceNames_messageResourceDefinition() {
     List<Descriptor> messageDescriptors = lockerServiceFileDescriptor.getMessageTypes();
     Map<String, ResourceName> typeStringsToResourceNames =
         ResourceNameParser.parseResourceNamesFromMessages(messageDescriptors, pakkage);
-    assertEquals(1, typeStringsToResourceNames.size());
+    assertEquals(2, typeStringsToResourceNames.size());
 
     ResourceName resourceName = typeStringsToResourceNames.get("testgapic.googleapis.com/Document");
     assertEquals(2, resourceName.patterns().size());
@@ -140,7 +140,31 @@ public void parseResourceNames_messageResourceDefinition() {
   }
 
   @Test
-  public void parseResourceNames_messageWithoutResourceDefinition() {
+  public void parseResourceNames_badMessageResourceNameDefinitionMissingNameField() {
+    FileDescriptor protoFileDescriptor = BadMessageResnameDefProto.getDescriptor();
+    List<Descriptor> messageDescriptors = protoFileDescriptor.getMessageTypes();
+    Descriptor messageDescriptor = messageDescriptors.get(0);
+    String pakkage = TypeParser.getPackage(protoFileDescriptor);
+
+    assertThrows(
+        IllegalStateException.class,
+        () -> ResourceNameParser.parseResourceNameFromMessageType(messageDescriptor, pakkage));
+  }
+
+  @Test
+  public void parseResourceNameFromMessage_basicResourceDefinition() {
+    String pakkage = TypeParser.getPackage(lockerServiceFileDescriptor);
+    List<Descriptor> messageDescriptors = lockerServiceFileDescriptor.getMessageTypes();
+    Descriptor documentMessageDescriptor = messageDescriptors.get(0);
+    assertEquals("Document", documentMessageDescriptor.getName());
+    Optional<ResourceName> resourceNameOpt =
+        ResourceNameParser.parseResourceNameFromMessageType(documentMessageDescriptor, pakkage);
+    assertTrue(resourceNameOpt.isPresent());
+    assertEquals("testgapic.googleapis.com/Document", resourceNameOpt.get().resourceTypeString());
+  }
+
+  @Test
+  public void parseResourceNamesFromMessage_noResourceDefinition() {
     String pakkage = TypeParser.getPackage(lockerServiceFileDescriptor);
     List<Descriptor> messageDescriptors = lockerServiceFileDescriptor.getMessageTypes();
     Descriptor folderMessageDescriptor = messageDescriptors.get(1);
@@ -151,15 +175,28 @@ public void parseResourceNames_messageWithoutResourceDefinition() {
   }
 
   @Test
-  public void parseResourceNames_badMessageResourceNameDefinitionMissingNameField() {
+  public void parseResourceNameFromMessage_nonNameResourceReferenceField() {
+    String pakkage = TypeParser.getPackage(lockerServiceFileDescriptor);
+    List<Descriptor> messageDescriptors = lockerServiceFileDescriptor.getMessageTypes();
+    Descriptor binderMessageDescriptor = messageDescriptors.get(2);
+    assertEquals("Binder", binderMessageDescriptor.getName());
+    Optional<ResourceName> resourceNameOpt =
+        ResourceNameParser.parseResourceNameFromMessageType(binderMessageDescriptor, pakkage);
+    assertTrue(resourceNameOpt.isPresent());
+    assertEquals("testgapic.googleapis.com/Binder", resourceNameOpt.get().resourceTypeString());
+  }
+
+  @Test
+  public void parseResourceNamesFromMessage_noNameOrResourceReferenceField() {
     FileDescriptor protoFileDescriptor = BadMessageResnameDefProto.getDescriptor();
-    List<Descriptor> messageDescriptors = protoFileDescriptor.getMessageTypes();
-    Descriptor messageDescriptor = messageDescriptors.get(0);
     String pakkage = TypeParser.getPackage(protoFileDescriptor);
-
+    List<Descriptor> messageDescriptors = protoFileDescriptor.getMessageTypes();
+    Descriptor pencilMessageDescriptor = messageDescriptors.get(1);
+    assertEquals("Pencil", pencilMessageDescriptor.getName());
     assertThrows(
-        NullPointerException.class,
-        () -> ResourceNameParser.parseResourceNameFromMessageType(messageDescriptor, pakkage));
+        IllegalStateException.class,
+        () ->
+            ResourceNameParser.parseResourceNameFromMessageType(pencilMessageDescriptor, pakkage));
   }
 
   @Test

src/test/java/com/google/api/generator/gapic/testdata/bad_message_resname_def.proto

@@ -32,3 +32,14 @@ message BarFoo {
 
   string display_name = 1;
 }
+
+// A proto with a resource definition, but missing a field titled "name" or
+// containing a resource reference.
+message Pencil {
+  option (google.api.resource) = {
+    type: "testgapic.googleapis.com/Pencil"
+    pattern: "pencils/{pencil}"
+  };
+
+  string owner = 1;
+}

src/test/java/com/google/api/generator/gapic/testdata/locker.proto

@@ -79,7 +79,7 @@ message Document {
     pattern: "*"
   };
 
-  // The resource name of the user.
+  // The resource name of the document.
   string name = 1;
 }
 
@@ -88,6 +88,17 @@ message Folder {
                        "cloudresourcemanager.googleapis.com/Folder"];
 }
 
+message Binder {
+  option (google.api.resource) = {
+    type: "testgapic.googleapis.com/Binder"
+    pattern: "binders/{binder}"
+  };
+
+  // The resource name of the binder.
+  string binder_name = 1 [(google.api.resource_reference).type =
+                              "testgapic.googleapis.com/Binder"];
+}
+
 message GetFolderRequest {
   string name = 1 [
     (google.api.resource_reference).type =