impl(sidekick): bindings for discovery methods (#2259)

coryan · web-flow · commit b46e000ce055 · 2025-09-20T11:26:20.000-04:00
Parse the `path` field in a discovery method into the `PathInfo` field
of the `api.Method` objects.
diff --git a/internal/sidekick/internal/parser/discovery/discovery.go b/internal/sidekick/internal/parser/discovery/discovery.go
@@ -88,7 +88,7 @@ func NewAPI(serviceConfig *serviceconfig.Service, contents []byte) (*api.API, er
 	}
 
 	for _, resource := range doc.Resources {
-		if err := addServiceRecursive(result, resource); err != nil {
+		if err := addServiceRecursive(result, doc, resource); err != nil {
 			return nil, err
 		}
 	}
diff --git a/internal/sidekick/internal/parser/discovery/methods.go b/internal/sidekick/internal/parser/discovery/methods.go
@@ -16,14 +16,15 @@ package discovery
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/googleapis/librarian/internal/sidekick/internal/api"
 )
 
-func makeServiceMethods(model *api.API, serviceID string, resource *resource) ([]*api.Method, error) {
+func makeServiceMethods(model *api.API, serviceID string, doc *document, resource *resource) ([]*api.Method, error) {
 	var methods []*api.Method
 	for _, input := range resource.Methods {
-		method, err := makeMethod(model, serviceID, input)
+		method, err := makeMethod(model, serviceID, doc, input)
 		if err != nil {
 			return nil, err
 		}
@@ -33,7 +34,7 @@ func makeServiceMethods(model *api.API, serviceID string, resource *resource) ([
 	return methods, nil
 }
 
-func makeMethod(model *api.API, serviceID string, input *method) (*api.Method, error) {
+func makeMethod(model *api.API, serviceID string, doc *document, input *method) (*api.Method, error) {
 	id := fmt.Sprintf("%s.%s", serviceID, input.Name)
 	if input.MediaUpload != nil {
 		return nil, fmt.Errorf("media upload methods are not supported, id=%s", id)
@@ -46,6 +47,27 @@ func makeMethod(model *api.API, serviceID string, input *method) (*api.Method, e
 	if err != nil {
 		return nil, err
 	}
+	var uriTemplate string
+	if strings.HasSuffix(doc.ServicePath, "/") {
+		uriTemplate = fmt.Sprintf("%s%s", doc.ServicePath, input.Path)
+	} else {
+		uriTemplate = fmt.Sprintf("%s/%s", doc.ServicePath, input.Path)
+	}
+	uriTemplate = strings.TrimPrefix(uriTemplate, "/")
+	path, err := ParseUriTemplate(uriTemplate)
+	if err != nil {
+		return nil, err
+	}
+	binding := &api.PathBinding{
+		Verb:            input.HTTPMethod,
+		PathTemplate:    path,
+		QueryParameters: map[string]bool{},
+	}
+	for _, p := range input.Parameters {
+		if p.Location != "path" {
+			binding.QueryParameters[p.Name] = true
+		}
+	}
 	method := &api.Method{
 		ID:            id,
 		Name:          input.Name,
@@ -54,6 +76,10 @@ func makeMethod(model *api.API, serviceID string, input *method) (*api.Method, e
 		// Deprecated: ...,
 		InputTypeID:  inputID,
 		OutputTypeID: outputID,
+		PathInfo: &api.PathInfo{
+			Bindings:      []*api.PathBinding{binding},
+			BodyFieldPath: "*",
+		},
 	}
 	return method, nil
 }
diff --git a/internal/sidekick/internal/parser/discovery/methods_test.go b/internal/sidekick/internal/parser/discovery/methods_test.go
@@ -21,6 +21,7 @@ func TestMakeServiceMethodsError(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	doc := document{}
 	input := &resource{
 		Name: "testResource",
 		Methods: []*method{
@@ -30,7 +31,7 @@ func TestMakeServiceMethodsError(t *testing.T) {
 			},
 		},
 	}
-	if methods, err := makeServiceMethods(model, "..testResource", input); err == nil {
+	if methods, err := makeServiceMethods(model, "..testResource", &doc, input); err == nil {
 		t.Errorf("expected error on method with media upload, got=%v", methods)
 	}
 }
@@ -48,8 +49,10 @@ func TestMakeMethodError(t *testing.T) {
 		{"mediaUploadMustBeNil", method{MediaUpload: &mediaUpload{}}},
 		{"requestMustHaveRef", method{Request: &schema{}}},
 		{"responseMustHaveRef", method{Response: &schema{}}},
+		{"badPath", method{Path: "{+var"}},
 	} {
-		if method, err := makeMethod(model, "..Test", &test.Input); err == nil {
+		doc := document{}
+		if method, err := makeMethod(model, "..Test", &doc, &test.Input); err == nil {
 			t.Errorf("expected error on method[%s], got=%v", test.Name, method)
 		}
 	}
diff --git a/internal/sidekick/internal/parser/discovery/services.go b/internal/sidekick/internal/parser/discovery/services.go
@@ -20,23 +20,23 @@ import (
 	"github.com/googleapis/librarian/internal/sidekick/internal/api"
 )
 
-func addServiceRecursive(model *api.API, resource *resource) error {
+func addServiceRecursive(model *api.API, doc *document, resource *resource) error {
 	if len(resource.Methods) != 0 {
-		if err := addService(model, resource); err != nil {
+		if err := addService(model, doc, resource); err != nil {
 			return err
 		}
 	}
 	for _, child := range resource.Resources {
-		if err := addServiceRecursive(model, child); err != nil {
+		if err := addServiceRecursive(model, doc, child); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func addService(model *api.API, resource *resource) error {
+func addService(model *api.API, doc *document, resource *resource) error {
 	id := fmt.Sprintf(".%s.%s", model.PackageName, resource.Name)
-	methods, err := makeServiceMethods(model, id, resource)
+	methods, err := makeServiceMethods(model, id, doc, resource)
 	if err != nil {
 		return err
 	}
diff --git a/internal/sidekick/internal/parser/discovery/services_test.go b/internal/sidekick/internal/parser/discovery/services_test.go
@@ -44,13 +44,50 @@ func TestService(t *testing.T) {
 				Documentation: "Returns the specified Zone resource.",
 				InputTypeID:   ".google.protobuf.Empty",
 				OutputTypeID:  "..Zone",
+				PathInfo: &api.PathInfo{
+					Bindings: []*api.PathBinding{
+						{
+							Verb: "GET",
+							PathTemplate: api.NewPathTemplate().
+								WithLiteral("compute").
+								WithLiteral("v1").
+								WithLiteral("projects").
+								WithVariableNamed("project").
+								WithLiteral("zones").
+								WithVariableNamed("zone"),
+							QueryParameters: map[string]bool{},
+						},
+					},
+					BodyFieldPath: "*",
+				},
 			},
 			{
 				ID:            "..zones.list",
 				Name:          "list",
 				Documentation: "Retrieves the list of Zone resources available to the specified project.",
 				InputTypeID:   ".google.protobuf.Empty",
 				OutputTypeID:  "..ZoneList",
+				PathInfo: &api.PathInfo{
+					Bindings: []*api.PathBinding{
+						{
+							Verb: "GET",
+							PathTemplate: api.NewPathTemplate().
+								WithLiteral("compute").
+								WithLiteral("v1").
+								WithLiteral("projects").
+								WithVariableNamed("project").
+								WithLiteral("zones"),
+							QueryParameters: map[string]bool{
+								"filter":               true,
+								"maxResults":           true,
+								"orderBy":              true,
+								"pageToken":            true,
+								"returnPartialSuccess": true,
+							},
+						},
+					},
+					BodyFieldPath: "*",
+				},
 			},
 		},
 	}
@@ -62,12 +99,13 @@ func TestServiceTopLevelMethodErrors(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	doc := document{}
 	input := resource{
 		Methods: []*method{
 			{MediaUpload: &mediaUpload{}},
 		},
 	}
-	if err := addServiceRecursive(model, &input); err == nil {
+	if err := addServiceRecursive(model, &doc, &input); err == nil {
 		t.Errorf("expected error in addServiceRecursive invalid top-level method, got=%v", model.Services)
 	}
 }
@@ -77,6 +115,7 @@ func TestServiceChildMethodErrors(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	doc := document{}
 	input := resource{
 		Resources: []*resource{
 			{
@@ -86,7 +125,7 @@ func TestServiceChildMethodErrors(t *testing.T) {
 			},
 		},
 	}
-	if err := addServiceRecursive(model, &input); err == nil {
+	if err := addServiceRecursive(model, &doc, &input); err == nil {
 		t.Errorf("expected error in addServiceRecursive invalid child method, got=%v", model.Services)
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -88,7 +88,7 @@ func NewAPI(serviceConfig serviceconfig.Service, contents []byte) (api.API, er`
`88`	`88`	`}`
`89`	`89`
`90`	`90`	`for _, resource := range doc.Resources {`
`91`		`- if err := addServiceRecursive(result, resource); err != nil {`
	`91`	`+ if err := addServiceRecursive(result, doc, resource); err != nil {`
`92`	`92`	`return nil, err`
`93`	`93`	`}`
`94`	`94`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@ func TestMakeServiceMethodsError(t *testing.T) {`
`21`	`21`	`if err != nil {`
`22`	`22`	`t.Fatal(err)`
`23`	`23`	`}`
	`24`	`+ doc := document{}`
`24`	`25`	`input := &resource{`
`25`	`26`	`Name: "testResource",`
`26`	`27`	`Methods: []*method{`
`@@ -30,7 +31,7 @@ func TestMakeServiceMethodsError(t *testing.T) {`
`30`	`31`	`},`
`31`	`32`	`},`
`32`	`33`	`}`
`33`		`- if methods, err := makeServiceMethods(model, "..testResource", input); err == nil {`
	`34`	`+ if methods, err := makeServiceMethods(model, "..testResource", &doc, input); err == nil {`
`34`	`35`	`t.Errorf("expected error on method with media upload, got=%v", methods)`
`35`	`36`	`}`
`36`	`37`	`}`
`@@ -48,8 +49,10 @@ func TestMakeMethodError(t *testing.T) {`
`48`	`49`	`{"mediaUploadMustBeNil", method{MediaUpload: &mediaUpload{}}},`
`49`	`50`	`{"requestMustHaveRef", method{Request: &schema{}}},`
`50`	`51`	`{"responseMustHaveRef", method{Response: &schema{}}},`
	`52`	`+ {"badPath", method{Path: "{+var"}},`
`51`	`53`	`} {`
`52`		`- if method, err := makeMethod(model, "..Test", &test.Input); err == nil {`
	`54`	`+ doc := document{}`
	`55`	`+ if method, err := makeMethod(model, "..Test", &doc, &test.Input); err == nil {`
`53`	`56`	`t.Errorf("expected error on method[%s], got=%v", test.Name, method)`
`54`	`57`	`}`
`55`	`58`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,23 +20,23 @@ import (`
`20`	`20`	`"github.com/googleapis/librarian/internal/sidekick/internal/api"`
`21`	`21`	`)`
`22`	`22`
`23`		`-func addServiceRecursive(model api.API, resource resource) error {`
	`23`	`+func addServiceRecursive(model api.API, doc document, resource *resource) error {`
`24`	`24`	`if len(resource.Methods) != 0 {`
`25`		`- if err := addService(model, resource); err != nil {`
	`25`	`+ if err := addService(model, doc, resource); err != nil {`
`26`	`26`	`return err`
`27`	`27`	`}`
`28`	`28`	`}`
`29`	`29`	`for _, child := range resource.Resources {`
`30`		`- if err := addServiceRecursive(model, child); err != nil {`
	`30`	`+ if err := addServiceRecursive(model, doc, child); err != nil {`
`31`	`31`	`return err`
`32`	`32`	`}`
`33`	`33`	`}`
`34`	`34`	`return nil`
`35`	`35`	`}`
`36`	`36`
`37`		`-func addService(model api.API, resource resource) error {`
	`37`	`+func addService(model api.API, doc document, resource *resource) error {`
`38`	`38`	`id := fmt.Sprintf(".%s.%s", model.PackageName, resource.Name)`
`39`		`- methods, err := makeServiceMethods(model, id, resource)`
	`39`	`+ methods, err := makeServiceMethods(model, id, doc, resource)`
`40`	`40`	`if err != nil {`
`41`	`41`	`return err`
`42`	`42`	`}`