feat(sidekick): implement explicit target resource identification (googleapis#4105)

haphungw · web-flow · commit 751eac271800 · 2026-02-19T13:27:00.000-08:00
Implement Priority 1 of the resource naming logic. Identify the target resource by resolving path variables to input message fields and checking for `google.api.resource_reference` annotations. Similar logic does exist but resides in the Rust generator (`internal/sidekick/rust/annotate.go`, `findResourceNameCandidates`). Later, Rust will use this instead for resource naming. Fixes googleapis#4099
diff --git a/internal/sidekick/api/model.go b/internal/sidekick/api/model.go
@@ -1516,7 +1516,4 @@ type TargetResource struct {
 	// FieldPaths is a list of field name sequences that, when joined, form a resource name.
 	// For example, [["project"], ["zone"], ["instance"]] identifies a multi-part resource.
 	FieldPaths [][]string
-	// PathTemplate is the template for the resource name.
-	// It uses AIP-122 style placeholders, e.g., "projects/{projects}/zones/{zones}/instances/{instance}".
-	PathTemplate *PathTemplate
 }
diff --git a/internal/sidekick/api/model_test.go b/internal/sidekick/api/model_test.go
@@ -237,10 +237,6 @@ func TestPathTemplateBuilder(t *testing.T) {
 func TestPathBindingHeuristic(t *testing.T) {
 	heuristic := &TargetResource{
 		FieldPaths: [][]string{{"project"}, {"zone"}, {"instance"}},
-		PathTemplate: NewPathTemplate().
-			WithLiteral("projects").WithVariableNamed("project").
-			WithLiteral("zones").WithVariableNamed("zone").
-			WithLiteral("instances").WithVariableNamed("instance"),
 	}
 	binding := &PathBinding{
 		Verb:           "GET",
diff --git a/internal/sidekick/api/resource_identification.go b/internal/sidekick/api/resource_identification.go
@@ -14,6 +14,11 @@
 
 package api
 
+import (
+	"fmt"
+	"slices"
+)
+
 // IdentifyTargetResources populates the TargetResource field in PathBinding
 // for all methods in the API.
 //
@@ -22,38 +27,104 @@ package api
 //     with fields present in the PathTemplate.
 //  2. Heuristic Identification: For allow-listed services, uses path segment
 //     patterns to identify resources when annotations are missing.
-func IdentifyTargetResources(model *API) {
+func IdentifyTargetResources(model *API) error {
 	for _, service := range model.Services {
 		for _, method := range service.Methods {
 			if method.PathInfo == nil {
 				continue
 			}
 			for _, binding := range method.PathInfo.Bindings {
-				identifyTargetResourceForBinding(method, binding)
+				if err := identifyTargetResourceForBinding(method, binding); err != nil {
+					return err
+				}
 			}
 		}
 	}
+	return nil
 }
 
 // identifyTargetResourceForBinding processes a single path binding to identify its target resource.
-func identifyTargetResourceForBinding(method *Method, binding *PathBinding) {
+func identifyTargetResourceForBinding(method *Method, binding *PathBinding) error {
 	if binding.PathTemplate == nil {
-		return
+		return nil
 	}
 
 	// Priority 1: Explicit Identification
 	// Matches google.api.resource_reference annotations.
-	if target := identifyExplicitTarget(method, binding); target != nil {
+	target, err := identifyExplicitTarget(method, binding)
+	if err != nil {
+		return err
+	}
+	if target != nil {
 		binding.TargetResource = target
-		return
+		return nil
 	}
 
 	// Priority 2: Heuristic Identification
 	// Uses path segment patterns to guess the resource.
 	// TODO(#4100): Implement IdentifyTargetResources for allow-listed services using heuristic path segment patterns.
+	return nil
 }
 
-func identifyExplicitTarget(_ *Method, _ *PathBinding) *TargetResource {
-	// TODO(#4099): Implement IdentifyTargetResources for consistent explicit resource identification in the sidekick parser.
-	return nil
+func identifyExplicitTarget(method *Method, binding *PathBinding) (*TargetResource, error) {
+	var fieldPaths [][]string
+	if method.InputType == nil {
+		return nil, fmt.Errorf("consistency error: method %q has no InputType", method.Name)
+	}
+
+	// Collect field paths corresponding to variable segments in the path template
+	for _, segment := range binding.PathTemplate.Segments {
+		if segment.Variable == nil {
+			continue
+		}
+
+		fieldPath := segment.Variable.FieldPath
+		field, err := findField(method.InputType, fieldPath)
+		if err != nil {
+			return nil, err
+		}
+		if field == nil {
+			return nil, fmt.Errorf("consistency error: field %v not found in message %q", fieldPath, method.InputType.Name)
+		}
+		if !field.IsResourceReference() {
+			return nil, nil
+		}
+		fieldPaths = append(fieldPaths, fieldPath)
+	}
+
+	if len(fieldPaths) == 0 {
+		return nil, nil
+	}
+	return &TargetResource{
+		FieldPaths: fieldPaths,
+	}, nil
+}
+
+// findField traverses the (nested) message structure to find a field by its field path.
+func findField(msg *Message, path []string) (*Field, error) {
+	if len(path) == 0 {
+		return nil, fmt.Errorf("consistency error: empty field path in msg: %s", msg.ID)
+	}
+
+	current := msg
+	var field *Field
+
+	for i, name := range path {
+		idx := slices.IndexFunc(current.Fields, func(f *Field) bool {
+			return f.Name == name
+		})
+		if idx == -1 {
+			return nil, fmt.Errorf("consistency error: field %s not found in message %q", name, current.Name)
+		}
+		field = current.Fields[idx]
+
+		if i < len(path)-1 {
+			if field.MessageType == nil {
+				return nil, fmt.Errorf("consistency error: field %s in message %s has no MessageType", field.Name, current.Name)
+			}
+			current = field.MessageType
+		}
+	}
+
+	return field, nil
 }
diff --git a/internal/sidekick/api/resource_identification_test.go b/internal/sidekick/api/resource_identification_test.go
@@ -16,8 +16,159 @@ package api
 
 import (
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
+// setupTestModel helper creates a minimal API model for testing resource identification.
+func setupTestModel(serviceID string, pathTemplate *PathTemplate, fields []*Field) (*API, *PathBinding) {
+	binding := &PathBinding{PathTemplate: pathTemplate}
+	method := &Method{
+		Name: "TestMethod",
+		InputType: &Message{
+			Fields: fields,
+		},
+		PathInfo: &PathInfo{
+			Bindings: []*PathBinding{binding},
+		},
+	}
+	service := &Service{
+		ID:      serviceID,
+		Methods: []*Method{method},
+	}
+	model := &API{
+		Services: []*Service{service},
+	}
+	return model, binding
+}
+
 func TestIdentifyTargetResources(t *testing.T) {
-	// TODO(#4099): Implement IdentifyTargetResources for consistent explicit resource identification in the sidekick parser.
+	for _, test := range []struct {
+		name      string
+		serviceID string
+		path      *PathTemplate
+		fields    []*Field
+		want      *TargetResource
+	}{
+		{
+			name:      "explicit: standard resource reference",
+			serviceID: "any.service",
+			path: NewPathTemplate().
+				WithLiteral("projects").WithVariableNamed("project"),
+			fields: []*Field{
+				{
+					Name:              "project",
+					Typez:             STRING_TYPE,
+					ResourceReference: &ResourceReference{Type: "cloudresourcemanager.googleapis.com/Project"},
+				},
+			},
+			want: &TargetResource{
+				FieldPaths: [][]string{{"project"}},
+			},
+		},
+		{
+			name:      "explicit: multiple resource references",
+			serviceID: "any.service",
+			path: NewPathTemplate().
+				WithLiteral("projects").WithVariableNamed("project").
+				WithLiteral("locations").WithVariableNamed("location"),
+			fields: []*Field{
+				{
+					Name:              "project",
+					Typez:             STRING_TYPE,
+					ResourceReference: &ResourceReference{Type: "cloudresourcemanager.googleapis.com/Project"},
+				},
+				{
+					Name:              "location",
+					Typez:             STRING_TYPE, // Often locations are string IDs
+					ResourceReference: &ResourceReference{Type: "locations.googleapis.com/Location"},
+				},
+			},
+			want: &TargetResource{
+				FieldPaths: [][]string{{"project"}, {"location"}},
+			},
+		},
+		{
+			name:      "explicit: nested field reference",
+			serviceID: "any.service",
+			path: NewPathTemplate().
+				WithLiteral("projects").WithVariableNamed("parent", "project"),
+			fields: []*Field{
+				{
+					Name:  "parent",
+					Typez: MESSAGE_TYPE,
+					MessageType: &Message{
+						Fields: []*Field{
+							{
+								Name:              "project",
+								Typez:             STRING_TYPE,
+								ResourceReference: &ResourceReference{Type: "cloudresourcemanager.googleapis.com/Project"},
+							},
+						},
+					},
+				},
+			},
+			want: &TargetResource{
+				FieldPaths: [][]string{{"parent", "project"}},
+			},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			model, binding := setupTestModel(test.serviceID, test.path, test.fields)
+			IdentifyTargetResources(model)
+
+			got := binding.TargetResource
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
+
+func TestIdentifyTargetResources_NoMatch(t *testing.T) {
+	for _, test := range []struct {
+		name      string
+		serviceID string
+		path      *PathTemplate
+		fields    []*Field
+	}{
+		{
+			name:      "Explicit: missing reference returns nil",
+			serviceID: "any.service",
+			path: NewPathTemplate().
+				WithLiteral("projects").WithVariableNamed("project"),
+			fields: []*Field{
+				{Name: "project", Typez: STRING_TYPE}, // No ResourceReference
+			},
+		},
+		{
+			name:      "Explicit: partial reference returns nil",
+			serviceID: "any.service",
+			path: NewPathTemplate().
+				WithLiteral("projects").WithVariableNamed("project").
+				WithLiteral("glossaries").WithVariableNamed("glossary"),
+			fields: []*Field{
+				{
+					Name:              "project",
+					Typez:             STRING_TYPE,
+					ResourceReference: &ResourceReference{Type: "cloudresourcemanager.googleapis.com/Project"},
+				},
+				{
+					Name:  "glossary",
+					Typez: STRING_TYPE,
+					// No ResourceReference on the second variable
+				},
+			},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			model, binding := setupTestModel(test.serviceID, test.path, test.fields)
+			IdentifyTargetResources(model)
+
+			got := binding.TargetResource
+			if got != nil {
+				t.Errorf("IdentifyTargetResources() = %v, want nil", got)
+			}
+		})
+	}
 }
diff --git a/internal/sidekick/parser/parser.go b/internal/sidekick/parser/parser.go
@@ -79,7 +79,9 @@ func CreateModel(cfg *ModelConfig) (*api.API, error) {
 	if err := api.CrossReference(model); err != nil {
 		return nil, err
 	}
-	api.IdentifyTargetResources(model)
+	if err := api.IdentifyTargetResources(model); err != nil {
+		return nil, err
+	}
 	if err := api.SkipModelElements(model, cfg.Override); err != nil {
 		return nil, err
 	}

Original file line number	Diff line number	Diff line change
`@@ -1516,7 +1516,4 @@ type TargetResource struct {`
`1516`	`1516`	`// FieldPaths is a list of field name sequences that, when joined, form a resource name.`
`1517`	`1517`	`// For example, [["project"], ["zone"], ["instance"]] identifies a multi-part resource.`
`1518`	`1518`	`FieldPaths [][]string`
`1519`		`- // PathTemplate is the template for the resource name.`
`1520`		`- // It uses AIP-122 style placeholders, e.g., "projects/{projects}/zones/{zones}/instances/{instance}".`
`1521`		`- PathTemplate *PathTemplate`
`1522`	`1519`	`}`
Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,9 @@ func CreateModel(cfg ModelConfig) (api.API, error) {`
`79`	`79`	`if err := api.CrossReference(model); err != nil {`
`80`	`80`	`return nil, err`
`81`	`81`	`}`
`82`		`- api.IdentifyTargetResources(model)`
	`82`	`+ if err := api.IdentifyTargetResources(model); err != nil {`
	`83`	`+ return nil, err`
	`84`	`+ }`
`83`	`85`	`if err := api.SkipModelElements(model, cfg.Override); err != nil {`
`84`	`86`	`return nil, err`
`85`	`87`	`}`