Skip to content

Commit 69c2cfb

Browse files
authored
feat(internal/surfer): add command groups (#3544)
## PR Summary: Generate `__init__.py` for proper command group detection This PR updates the `gcloud` surface generator to include `__init__.py` files in the generated directory structure. These files are required by the `gcloud` CLI framework to correctly identify and load command groups and their nested hierarchies. ### Key Changes 1. **Generation Logic (`generate.go`):** - Implemented a `writeInitPy` helper function to create empty package markers. - Updated `generateService` to ensure the service root directory (e.g., `parallelstore/`) contains an `__init__.py`. - Updated `generateResourceCommands` to ensure each resource group directory (e.g., `parallelstore/instances/`) contains an `__init__.py`. - Fixed an issue where the resource directory was not created before attempting to write the initialization file. 2. **Verification:** - Updated `TestGenerateResourceCommands` to verify the presence of `__init__.py` in generated resource directories. - Manually verified the generated file structure for the Parallelstore API. ### File Structure Comparison **Old Structure:** ``` parallelstore └── instances ├── _partials/ │   └── ... ├── create.yaml └── ... ``` **New Structure:** ``` parallelstore ├── __init__.py └── instances ├── __init__.py ├── _partials/ │   └── ... ├── create.yaml └── ... ``` Fixes #3543
1 parent 61ab83b commit 69c2cfb

6 files changed

Lines changed: 257 additions & 6 deletions

File tree

internal/surfer/gcloud/generate.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,18 @@
1515
package gcloud
1616

1717
import (
18+
"bytes"
1819
"context"
1920
"fmt"
2021
"os"
2122
"path/filepath"
2223
"strings"
24+
"text/template"
2325

2426
"github.com/googleapis/librarian/internal/sidekick/api"
2527
"github.com/googleapis/librarian/internal/surfer/gcloud/utils"
2628
"github.com/googleapis/librarian/internal/yaml"
29+
"github.com/iancoleman/strcase"
2730
)
2831

2932
// partialsHeader is the directive that tells gcloud to look in the `_partials` directory
@@ -69,6 +72,21 @@ func generateService(service *api.Service, overrides *Config, model *api.API, ou
6972
// `{outdir}/{shortServiceName}/`
7073
surfaceDir := filepath.Join(output, shortServiceName)
7174

75+
if err := os.MkdirAll(surfaceDir, 0755); err != nil {
76+
return fmt.Errorf("failed to create surface directory for %q: %w", shortServiceName, err)
77+
}
78+
79+
track := strings.ToUpper(utils.InferTrackFromPackage(service.Package))
80+
data := commandGroupData{
81+
ServiceTitle: utils.GetServiceTitle(model, shortServiceName),
82+
ClassNamePrefix: strcase.ToCamel(shortServiceName),
83+
Tracks: []string{track},
84+
}
85+
86+
if err := writeCommandGroupFile(surfaceDir, data); err != nil {
87+
return fmt.Errorf("failed to write command group file for service %q: %w", shortServiceName, err)
88+
}
89+
7290
// gcloud commands are resource-centric commands (e.g., `gcloud parallelstore instances create`),
7391
// so we first need to group all the API methods by the resource they operate on.
7492
// We'll create a map where the key is the resource's collection ID (e.g., "instances")
@@ -106,10 +124,35 @@ func generateService(service *api.Service, overrides *Config, model *api.API, ou
106124
// For a given collectionID like "instances", this function will create a directory
107125
// `instances/` and populate it with `create.yaml`, `delete.yaml`, etc.
108126
func generateResourceCommands(collectionID string, methods []*api.Method, baseDir string, overrides *Config, model *api.API, service *api.Service) error {
127+
if len(methods) == 0 {
128+
return nil
129+
}
130+
109131
// The main directory for the resource is named after its collection ID.
110132
// Example: `{baseDir}/instances`
111133
resourceDir := filepath.Join(baseDir, collectionID)
112134

135+
if err := os.MkdirAll(resourceDir, 0755); err != nil {
136+
return fmt.Errorf("failed to create resource directory for %q: %w", collectionID, err)
137+
}
138+
139+
singular := utils.GetSingularResourceNameForMethod(methods[0], model)
140+
141+
// We determine the short service name from the default host to use as a fallback title.
142+
shortServiceName, _, _ := strings.Cut(service.DefaultHost, ".")
143+
144+
track := strings.ToUpper(utils.InferTrackFromPackage(service.Package))
145+
data := commandGroupData{
146+
ServiceTitle: utils.GetServiceTitle(model, shortServiceName),
147+
ResourceSingular: singular,
148+
ClassNamePrefix: strcase.ToCamel(collectionID),
149+
Tracks: []string{track},
150+
}
151+
152+
if err := writeCommandGroupFile(resourceDir, data); err != nil {
153+
return fmt.Errorf("failed to write command group file for resource %q: %w", collectionID, err)
154+
}
155+
113156
// Gcloud commands are defined in a `_partials` directory. This allows
114157
// for sharing command definitions across different release tracks (GA, Beta, Alpha).
115158
partialsDir := filepath.Join(resourceDir, "_partials")
@@ -165,3 +208,36 @@ func generateResourceCommands(collectionID string, methods []*api.Method, baseDi
165208
}
166209
return nil
167210
}
211+
212+
func writeCommandGroupFile(dir string, data commandGroupData) error {
213+
var buf bytes.Buffer
214+
if err := commandGroupTemplate.Execute(&buf, data); err != nil {
215+
return err
216+
}
217+
path := filepath.Join(dir, "__init__.py")
218+
return os.WriteFile(path, buf.Bytes(), 0644)
219+
}
220+
221+
var commandGroupTemplate = template.Must(template.New("__init__.py").Funcs(template.FuncMap{
222+
"toCamel": strcase.ToCamel,
223+
}).Parse(`# NOTE: This file is autogenerated and should not be edited by hand.
224+
"""Manage {{.ServiceTitle}}{{if .ResourceSingular}} {{.ResourceSingular}}{{end}} resources."""
225+
226+
from googlecloudsdk.calliope import base
227+
228+
229+
{{range .Tracks}}@base.ReleaseTracks(base.ReleaseTrack.{{.}})
230+
@base.Autogenerated
231+
@base.Hidden
232+
class {{$.ClassNamePrefix}}{{. | toCamel}}(base.Group):
233+
"""Manage {{$.ServiceTitle}}{{if $.ResourceSingular}} {{$.ResourceSingular}}{{end}} resources."""
234+
235+
236+
{{end}}`))
237+
238+
type commandGroupData struct {
239+
ServiceTitle string
240+
ResourceSingular string
241+
ClassNamePrefix string
242+
Tracks []string
243+
}

internal/surfer/gcloud/generate_test.go

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package gcloud
1616

1717
import (
18+
"bytes"
1819
"os"
1920
"path/filepath"
2021
"testing"
@@ -48,7 +49,7 @@ func TestGenerateService(t *testing.T) {
4849
},
4950
},
5051
model: &api.API{
51-
ResourceDefinitions: []*api.Resource{},
52+
Title: "Parallelstore API",
5253
},
5354
wantErr: false,
5455
},
@@ -57,8 +58,11 @@ func TestGenerateService(t *testing.T) {
5758
service: &api.Service{
5859
Name: "parallelstore.googleapis.com",
5960
DefaultHost: "",
61+
Package: "google.cloud.parallelstore.v1",
62+
},
63+
model: &api.API{
64+
Title: "Parallelstore API",
6065
},
61-
model: &api.API{},
6266
wantErr: true,
6367
},
6468
} {
@@ -79,11 +83,13 @@ func TestGenerateResourceCommands(t *testing.T) {
7983

8084
err := generateResourceCommands("instances", []*api.Method{
8185
{
82-
Name: "CreateInstance",
83-
Service: &api.Service{Package: "google.cloud.parallelstore.v1"},
86+
Name: "CreateInstance",
87+
Service: &api.Service{
88+
Package: "google.cloud.parallelstore.v1",
89+
},
8490
InputType: &api.Message{},
8591
},
86-
}, tmpDir, &Config{}, &api.API{}, &api.Service{DefaultHost: "parallelstore.googleapis.com"})
92+
}, tmpDir, &Config{}, &api.API{Title: "Parallelstore API"}, &api.Service{DefaultHost: "parallelstore.googleapis.com"})
8793

8894
if err != nil {
8995
t.Fatalf("generateResourceCommands() error = %v", err)
@@ -101,4 +107,19 @@ func TestGenerateResourceCommands(t *testing.T) {
101107
if diff := cmp.Diff(wantContent, string(content)); diff != "" {
102108
t.Errorf("main file content mismatch (-want +got):\n%s", diff)
103109
}
110+
111+
// Check __init__.py content
112+
initFile := filepath.Join(tmpDir, "instances", "__init__.py")
113+
if _, err := os.Stat(initFile); os.IsNotExist(err) {
114+
t.Errorf("expected file %s to exist", initFile)
115+
}
116+
initContent, _ := os.ReadFile(initFile)
117+
// The mock model is empty, so we expect some defaults.
118+
// But let's see if it matches the general structure.
119+
if !bytes.Contains(initContent, []byte(`"""Manage Parallelstore resources."""`)) {
120+
t.Errorf("__init__.py content missing expected docstring:\n%s", string(initContent))
121+
}
122+
if !bytes.Contains(initContent, []byte("class InstancesGa(base.Group):")) {
123+
t.Errorf("__init__.py content missing expected class definition:\n%s", string(initContent))
124+
}
104125
}

internal/surfer/gcloud/utils/resource.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,19 @@ func GetPluralResourceNameForMethod(method *api.Method, model *api.API) string {
195195
}
196196
return ""
197197
}
198+
199+
// GetSingularResourceNameForMethod determines the singular name of a resource. It follows a clear
200+
// hierarchy of truth: first, the explicit `singular` field in the resource
201+
// definition, and second, inference from the resource pattern.
202+
func GetSingularResourceNameForMethod(method *api.Method, model *api.API) string {
203+
resource := GetResourceForMethod(method, model)
204+
if resource != nil {
205+
if resource.Singular != "" {
206+
return resource.Singular
207+
}
208+
if len(resource.Patterns) > 0 {
209+
return GetSingularFromSegments(resource.Patterns[0])
210+
}
211+
}
212+
return ""
213+
}

internal/surfer/gcloud/utils/resource_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,3 +446,77 @@ func TestGetPluralResourceNameForMethod(t *testing.T) {
446446
})
447447
}
448448
}
449+
450+
func TestGetSingularResourceNameForMethod(t *testing.T) {
451+
instanceResource := &api.Resource{
452+
Type: "example.googleapis.com/Instance",
453+
Patterns: []api.ResourcePattern{
454+
{
455+
*api.NewPathSegment().WithLiteral("instances"),
456+
*api.NewPathSegment().WithVariable(api.NewPathVariable("instance").WithMatch()),
457+
},
458+
},
459+
}
460+
model := &api.API{
461+
ResourceDefinitions: []*api.Resource{
462+
instanceResource,
463+
},
464+
}
465+
466+
for _, test := range []struct {
467+
name string
468+
method *api.Method
469+
want string
470+
}{
471+
{
472+
name: "Inferred from Pattern",
473+
method: &api.Method{
474+
Name: "ListInstances",
475+
InputType: &api.Message{
476+
Fields: []*api.Field{
477+
{
478+
Name: "parent",
479+
ResourceReference: &api.ResourceReference{
480+
ChildType: "example.googleapis.com/Instance",
481+
},
482+
},
483+
},
484+
},
485+
},
486+
want: "instance",
487+
},
488+
{
489+
name: "Explicit Singular",
490+
method: &api.Method{
491+
Name: "ListBooks",
492+
InputType: &api.Message{
493+
Fields: []*api.Field{
494+
{
495+
Name: "parent",
496+
ResourceReference: &api.ResourceReference{
497+
ChildType: "example.googleapis.com/Book",
498+
},
499+
},
500+
},
501+
},
502+
},
503+
want: "book", // Assuming we mock a Book resource with Singular="book" below
504+
},
505+
} {
506+
// Setup explicit singular for the second case
507+
if test.name == "Explicit Singular" {
508+
bookResource := &api.Resource{
509+
Type: "example.googleapis.com/Book",
510+
Singular: "book",
511+
}
512+
model.ResourceDefinitions = append(model.ResourceDefinitions, bookResource)
513+
}
514+
515+
t.Run(test.name, func(t *testing.T) {
516+
got := GetSingularResourceNameForMethod(test.method, model)
517+
if diff := cmp.Diff(test.want, got); diff != "" {
518+
t.Errorf("GetSingularResourceNameForMethod mismatch (-want +got):\n%s", diff)
519+
}
520+
})
521+
}
522+
}

internal/surfer/gcloud/utils/service.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,12 @@
1414

1515
package utils
1616

17-
import "strings"
17+
import (
18+
"strings"
19+
20+
"github.com/googleapis/librarian/internal/sidekick/api"
21+
"github.com/iancoleman/strcase"
22+
)
1823

1924
// InferTrackFromPackage infers the release track from the proto package name.
2025
// as mandated per AIP-185
@@ -36,3 +41,12 @@ func InferTrackFromPackage(pkg string) string {
3641
}
3742
return "ga"
3843
}
44+
45+
// GetServiceTitle returns the service title for documentation.
46+
// It tries to use the API title, falling back to a CamelCase version of the short service name.
47+
func GetServiceTitle(model *api.API, shortServiceName string) string {
48+
if t := strings.TrimSuffix(model.Title, " API"); t != "" {
49+
return t
50+
}
51+
return strcase.ToCamel(shortServiceName)
52+
}

internal/surfer/gcloud/utils/service_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"testing"
1919

2020
"github.com/google/go-cmp/cmp"
21+
"github.com/googleapis/librarian/internal/sidekick/api"
2122
)
2223

2324
func TestInferTrackFromPackage(t *testing.T) {
@@ -41,3 +42,52 @@ func TestInferTrackFromPackage(t *testing.T) {
4142
})
4243
}
4344
}
45+
46+
func TestGetServiceTitle(t *testing.T) {
47+
for _, test := range []struct {
48+
name string
49+
model *api.API
50+
shortServiceName string
51+
want string
52+
}{
53+
{
54+
name: "With API Suffix",
55+
model: &api.API{
56+
Title: "Parallelstore API",
57+
},
58+
shortServiceName: "parallelstore",
59+
want: "Parallelstore",
60+
},
61+
{
62+
name: "Without API Suffix",
63+
model: &api.API{
64+
Title: "Parallelstore",
65+
},
66+
shortServiceName: "parallelstore",
67+
want: "Parallelstore",
68+
},
69+
{
70+
name: "Empty Title",
71+
model: &api.API{
72+
Title: "",
73+
},
74+
shortServiceName: "parallelstore",
75+
want: "Parallelstore",
76+
},
77+
{
78+
name: "Empty Title and different short name",
79+
model: &api.API{
80+
Title: "",
81+
},
82+
shortServiceName: "cloudbuild",
83+
want: "Cloudbuild",
84+
},
85+
} {
86+
t.Run(test.name, func(t *testing.T) {
87+
got := GetServiceTitle(test.model, test.shortServiceName)
88+
if diff := cmp.Diff(test.want, got); diff != "" {
89+
t.Errorf("mismatch (-want +got):\n%s", diff)
90+
}
91+
})
92+
}
93+
}

0 commit comments

Comments
 (0)