Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
4 changes: 1 addition & 3 deletions internal/surfer/gcloud/command_group_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,8 @@ func newCommandGroupBuilder(model *api.API, service *api.Service, config *provid
}

func (b *commandGroupBuilder) buildRoot() *CommandGroup {
// TODO (https://github.com/googleapis/librarian/issues/4945): Name can be an empty string here.
// Need clear error handling rather than silent skipping
return &CommandGroup{
Name: b.model.Name,
Name: provider.ResolveRootPackage(b.model),
HelpText: fmt.Sprintf("Manage %s resources.", b.title),
Groups: make(map[string]*CommandGroup),
Commands: make(map[string]*Command),
Expand Down
24 changes: 12 additions & 12 deletions internal/surfer/gcloud/command_tree_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,16 @@ func (b *commandTreeBuilder) build() (*CommandGroupsByTrack, error) {
// literal path segments of the method and walks the tree, creating missing
// groups if they do not yet exist.
func (b *commandTreeBuilder) insert(root *CommandGroup, groupBuilder *commandGroupBuilder, method *api.Method) error {
segments := b.getLiteralSegments(method)
if provider.IsSingletonResourceMethod(method, b.model) {
return nil
}

binding := provider.PrimaryBinding(method)
if binding == nil {
return nil
}

segments := provider.GetLiteralSegments(binding.PathTemplate.Segments)
if len(segments) == 0 {
return nil
}
Expand All @@ -73,7 +82,8 @@ func (b *commandTreeBuilder) insert(root *CommandGroup, groupBuilder *commandGro
if b.isTerminatedSegment(seg) {
return nil
}
if b.isFlattenedSegment(seg) {
isLeaf := i == len(segments)-1
if b.isFlattenedSegment(seg) && !isLeaf {
continue
}

Expand Down Expand Up @@ -124,19 +134,9 @@ var flattenedSegments = map[string]bool{
}

func (b *commandTreeBuilder) isFlattenedSegment(lit string) bool {
// TODO (https://github.com/googleapis/librarian/issues/4980): these should not be flattened if
// there are commands that should be generated for them.
return flattenedSegments[lit]
}

func (b *commandTreeBuilder) isTerminatedSegment(lit string) bool {
return lit == "operations" && !provider.ShouldGenerateOperations(b.config)
}

func (b *commandTreeBuilder) getLiteralSegments(method *api.Method) []string {
if method.PathInfo == nil || len(method.PathInfo.Bindings) == 0 {
return nil
}
// TODO(https://github.com/googleapis/librarian/issues/3415): handle multiple bindings
return provider.GetLiteralSegments(method.PathInfo.Bindings[0].PathTemplate.Segments)
}
37 changes: 16 additions & 21 deletions internal/surfer/gcloud/command_tree_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@
package gcloud

import (
"fmt"
"path"
"slices"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/googleapis/librarian/internal/sidekick/api"
"github.com/googleapis/librarian/internal/sidekick/parser/httprule"
"github.com/googleapis/librarian/internal/surfer/gcloud/provider"
)

Expand All @@ -43,9 +44,9 @@ func TestCommandTreeBuilder_Build_EmptyDefaultHost(t *testing.T) {

func TestCommandTreeBuilder_Build_Structure(t *testing.T) {
service := mockService("parallelstore.googleapis.com",
mockMethod("CreateInstance", "v1/projects/locations/instances"),
mockMethod("ListInstances", "v1/projects/locations/instances"),
mockMethod("GetOperation", "v1/projects/locations/operations"),
mockMethod("CreateInstance", "v1/{parent=projects/*/locations/*}/instances"),
mockMethod("ListInstances", "v1/{parent=projects/*/locations/*}/instances"),
mockMethod("GetOperation", "v1/{name=projects/*/locations/*/operations/*}"),
)
model := &api.API{
Name: "parallelstore",
Expand Down Expand Up @@ -81,7 +82,7 @@ func TestCommandTreeBuilder_Build_Structure(t *testing.T) {
}

func TestCommandTreeBuilder_Build_Operations_Disabled(t *testing.T) {
service := mockService("parallelstore.googleapis.com", mockMethod("GetOperation", "v1/projects/locations/operations"))
service := mockService("parallelstore.googleapis.com", mockMethod("GetOperation", "v1/{name=projects/*/locations/*/operations/*}"))

model := &api.API{
Name: "parallelstore",
Expand All @@ -101,7 +102,7 @@ func TestCommandTreeBuilder_Build_Operations_Disabled(t *testing.T) {
}

func TestCommandTreeBuilder_Build_Operations_Enabled(t *testing.T) {
service := mockService("parallelstore.googleapis.com", mockMethod("GetOperation", "v1/projects/locations/operations"))
service := mockService("parallelstore.googleapis.com", mockMethod("GetOperation", "v1/{name=projects/*/locations/*/operations/*}"))

model := &api.API{
Name: "parallelstore",
Expand All @@ -125,8 +126,8 @@ func TestCommandTreeBuilder_Build_Operations_Enabled(t *testing.T) {
}

func TestCommandTreeBuilder_Build_MultipleServices(t *testing.T) {
serviceOne := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1/projects/locations/instances"))
serviceTwo := mockService("OtherParallelstoreService", mockMethod("CreateOtherInstance", "v1/projects/locations/otherInstances"))
serviceOne := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1/{parent=projects/*/locations/*}/instances"))
serviceTwo := mockService("OtherParallelstoreService", mockMethod("CreateOtherInstance", "v1/{parent=projects/*/locations/*}/otherInstances"))

model := &api.API{
Name: "parallelstore",
Expand All @@ -151,8 +152,8 @@ func TestCommandTreeBuilder_Build_MultipleServices(t *testing.T) {
}

func TestCommandTreeBuilder_Build_MultipleReleaseTracks(t *testing.T) {
serviceOne := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1/projects/locations/instances"))
serviceTwo := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1alpha/projects/locations/instances"))
serviceOne := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1/{parent=projects/*/locations/*}/instances"))
serviceTwo := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1alpha/{parent=projects/*/locations/*}/instances"))
serviceTwo.Package = "google.cloud.parallelstore.v1alpha"

model := &api.API{
Expand Down Expand Up @@ -203,23 +204,17 @@ func flattenTree(g *CommandGroup) []string {
return paths
}

func parseLiteralPath(path string) *api.PathTemplate {
pt := api.NewPathTemplate()
for _, part := range strings.Split(path, "/") {
if part != "" {
pt = pt.WithLiteral(part)
}
}
return pt
}

func mockMethod(name, path string) *api.Method {
pt, err := httprule.ParseResourcePattern(path)
if err != nil {
panic(fmt.Sprintf("failed to parse path %q: %v", path, err))
}
return &api.Method{
Name: name,
PathInfo: &api.PathInfo{
Bindings: []*api.PathBinding{
{
PathTemplate: parseLiteralPath(path),
PathTemplate: pt,
},
},
},
Expand Down
23 changes: 21 additions & 2 deletions internal/surfer/gcloud/provider/method.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,18 @@ func IsStandardMethod(m *api.Method) bool {
return IsGet(m) || IsList(m) || IsCreate(m) || IsUpdate(m) || IsDelete(m)
}

// PrimaryBinding returns the primary path binding for the method, or nil if not available.
func PrimaryBinding(m *api.Method) *api.PathBinding {
if m.PathInfo == nil || len(m.PathInfo.Bindings) == 0 {
return nil
}
return m.PathInfo.Bindings[0]
}

// GetHTTPVerb returns the HTTP verb from the primary binding, or an empty string if not available.
func GetHTTPVerb(m *api.Method) string {
if m.PathInfo != nil && len(m.PathInfo.Bindings) > 0 {
return m.PathInfo.Bindings[0].Verb
if b := PrimaryBinding(m); b != nil {
return b.Verb
}
return ""
}
Expand Down Expand Up @@ -193,3 +201,14 @@ func FindResourceMessage(outputType *api.Message) *api.Message {
}
return nil
}

// IsSingletonResourceMethod determines whether a resource is a singleton.
// It checks if the resource associated with the method is a singleton.
func IsSingletonResourceMethod(method *api.Method, model *api.API) bool {
if method == nil {
return false
}

resource := GetResourceForMethod(method, model)
return isSingletonResource(resource)
}
125 changes: 125 additions & 0 deletions internal/surfer/gcloud/provider/method_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,3 +305,128 @@ func TestIsStandardMethod(t *testing.T) {
})
}
}

func TestIsSingletonResourceMethod(t *testing.T) {
model := &api.API{
ResourceDefinitions: []*api.Resource{
{
Type: "test.googleapis.com/Singleton",
Patterns: []api.ResourcePattern{
[]api.PathSegment{
*api.NewPathSegment().WithLiteral("projects"),
*api.NewPathSegment().WithVariable(api.NewPathVariable("project")),
*api.NewPathSegment().WithLiteral("singletonConfig"),
},
},
},
{
Type: "test.googleapis.com/AdjacentLiterals",
Patterns: []api.ResourcePattern{
[]api.PathSegment{
*api.NewPathSegment().WithLiteral("projects"),
*api.NewPathSegment().WithVariable(api.NewPathVariable("project")),
*api.NewPathSegment().WithLiteral("literal1"),
*api.NewPathSegment().WithLiteral("literal2"),
*api.NewPathSegment().WithVariable(api.NewPathVariable("instance")),
},
},
},
{
Type: "test.googleapis.com/Standard",
Patterns: []api.ResourcePattern{
[]api.PathSegment{
*api.NewPathSegment().WithLiteral("projects"),
*api.NewPathSegment().WithVariable(api.NewPathVariable("project")),
*api.NewPathSegment().WithLiteral("instances"),
*api.NewPathSegment().WithVariable(api.NewPathVariable("instance")),
},
},
},
},
}

for _, test := range []struct {
name string
method *api.Method
model *api.API
want bool
}{
{
name: "Nil Method",
method: nil,
model: &api.API{},
want: false,
},
{
name: "Fallback: No Resource Found, Path Ends in Variable",
method: api.NewTestMethod("Test").WithPathTemplate(&api.PathTemplate{Segments: parseResourcePattern("projects/{project}/instances/{instance}")}),
model: &api.API{},
want: false,
},
{
name: "Fallback: No Resource Found, Path Ends in Literal",
method: api.NewTestMethod("Test").WithPathTemplate(&api.PathTemplate{Segments: parseResourcePattern("projects/{project}/singletonConfig")}),
model: &api.API{},
want: false, // Was true when fallback existed.
},
{
name: "Resource Found: Singleton Pattern",
method: func() *api.Method {
m := api.NewTestMethod("GetSingleton")
m.InputType = &api.Message{
Fields: []*api.Field{{
Name: "name",
ResourceReference: &api.ResourceReference{
Type: "test.googleapis.com/Singleton",
},
}},
}
return m
}(),
model: model,
want: true,
},
{
name: "Resource Found: Adjacent Literals Pattern",
method: func() *api.Method {
m := api.NewTestMethod("GetAdjacent")
m.InputType = &api.Message{
Fields: []*api.Field{{
Name: "name",
ResourceReference: &api.ResourceReference{
Type: "test.googleapis.com/AdjacentLiterals",
},
}},
}
return m
}(),
model: model,
want: true,
},
{
name: "Resource Found: Standard Pattern",
method: func() *api.Method {
m := api.NewTestMethod("GetStandard")
m.InputType = &api.Message{
Fields: []*api.Field{{
Name: "name",
ResourceReference: &api.ResourceReference{
Type: "test.googleapis.com/Standard",
},
}},
}
return m
}(),
model: model,
want: false,
},
} {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
got := IsSingletonResourceMethod(test.method, test.model)
if diff := cmp.Diff(test.want, got); diff != "" {
t.Errorf("mismatch (-want +got):\n%s", diff)
}
})
}
}
29 changes: 29 additions & 0 deletions internal/surfer/gcloud/provider/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,35 @@ func GetResourceForMethod(method *api.Method, model *api.API) *api.Resource {
return nil
}

// isSingletonResource determines whether a resource is a singleton.
// It checks if any of the resource's canonical patterns end in a literal segment
// or contain two adjacent literal segments.
func isSingletonResource(resource *api.Resource) bool {
if resource == nil {
return false
}

for _, pattern := range resource.Patterns {
if len(pattern) == 0 {
continue
}

// A pattern ending in a literal is a singleton (e.g., projects/{project}/singletonConfig).
if pattern[len(pattern)-1].Literal != nil {
return true
}

// Adjacent literals anywhere in the pattern signify a singleton (e.g., .../literal1/literal2/...).
for i := 0; i < len(pattern)-1; i++ {
if pattern[i].Literal != nil && pattern[i+1].Literal != nil {
return true
}
}
}

return false
}

// GetPluralResourceNameForMethod determines the plural name of a resource. It follows a clear
// hierarchy of truth: first, the explicit `plural` field in the resource
// definition, and second, inference from the resource pattern.
Expand Down
9 changes: 9 additions & 0 deletions internal/surfer/gcloud/provider/service_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,12 @@ func GetServiceTitle(model *api.API, shortServiceName string) string {
}
return strcase.ToCamel(shortServiceName)
}

// ResolveRootPackage extracts the service name from the package name (second to last element),
// falling back to the provided fallback if there are not enough segments.
func ResolveRootPackage(model *api.API) string {
if parts := strings.Split(model.PackageName, "."); len(parts) >= 2 {
return parts[len(parts)-2]
}
return model.Name
}
20 changes: 20 additions & 0 deletions internal/surfer/gcloud/provider/service_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,23 @@ func TestGetServiceTitle(t *testing.T) {
})
}
}

func TestResolveRootPackage(t *testing.T) {
for _, test := range []struct {
name string
model *api.API
want string
}{
{"Standard package", &api.API{PackageName: "google.cloud.parallelstore.v1", Name: "fallback"}, "parallelstore"},
{"Synthetic package", &api.API{PackageName: "resource_standard.v1", Name: "fallback"}, "resource_standard"},
{"No dots", &api.API{PackageName: "parallelstore", Name: "fallback"}, "fallback"},
{"Empty package", &api.API{PackageName: "", Name: "fallback"}, "fallback"},
} {
t.Run(test.name, func(t *testing.T) {
got := ResolveRootPackage(test.model)
if got != test.want {
t.Errorf("ResolveRootPackage(%v) = %q, want %q", test.model, got, test.want)
}
})
}
}
Loading
Loading