Skip to content

Commit e799a3e

Browse files
authored
refactor(internal/surfer/gcloud): remove utils package (#4211)
Utils should generally be avoided as a package name, and the utils package was only used by two files in the parent gcloud package (builder.go and generate.go). This moves all functions into the gcloud package directly and unexports them, since they are internal implementation details. Files are reorganized by concern: method.go for method classification, resource.go for resource pattern parsing, service_info.go for service metadata, and field.go for field type mapping. Fixes #3364
1 parent e00efac commit e799a3e

10 files changed

Lines changed: 184 additions & 198 deletions

File tree

internal/surfer/gcloud/builder.go

Lines changed: 32 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"strings"
2121

2222
"github.com/googleapis/librarian/internal/sidekick/api"
23-
"github.com/googleapis/librarian/internal/surfer/gcloud/utils"
2423
"github.com/iancoleman/strcase"
2524
)
2625

@@ -55,7 +54,7 @@ func NewCommand(method *api.Method, overrides *Config, model *api.API, service *
5554

5655
// Infer default release track from proto package.
5756
// TODO(https://github.com/googleapis/librarian/issues/3289): Allow gcloud config to overwrite the track for this command.
58-
inferredTrack := utils.InferTrackFromPackage(method.Service.Package)
57+
inferredTrack := inferTrackFromPackage(method.Service.Package)
5958
cmd.ReleaseTracks = []string{strings.ToUpper(inferredTrack)}
6059

6160
// The core of the command generation happens here: we generate the arguments,
@@ -67,7 +66,7 @@ func NewCommand(method *api.Method, overrides *Config, model *api.API, service *
6766
cmd.Arguments = args
6867
cmd.Request = newRequest(method, overrides, model, service)
6968

70-
if utils.IsList(method) {
69+
if isList(method) {
7170
// List commands should have an id_field to enable the --uri flag.
7271
cmd.Response = &Response{
7372
IDField: "name",
@@ -76,7 +75,7 @@ func NewCommand(method *api.Method, overrides *Config, model *api.API, service *
7675
cmd.Output = newOutputConfig(method, model)
7776
}
7877

79-
if utils.IsUpdate(method) {
78+
if isUpdate(method) {
8079
// Standard Update methods in gcloud use the Read-Modify-Update pattern.
8180
cmd.Update = &UpdateConfig{
8281
ReadModifyUpdate: true,
@@ -115,7 +114,7 @@ func newArguments(method *api.Method, overrides *Config, model *api.API, service
115114
// shouldSkipParam determines if a field should be excluded from the generated command arguments.
116115
func shouldSkipParam(field *api.Field, method *api.Method) bool {
117116
// We don't skip the primary resource field, even if it's named "parent" or "name".
118-
if utils.IsPrimaryResource(field, method) {
117+
if isPrimaryResource(field, method) {
119118
return false
120119
}
121120

@@ -135,7 +134,7 @@ func shouldSkipParam(field *api.Field, method *api.Method) bool {
135134
}
136135

137136
// For List methods, standard pagination/filtering arguments are handled by gcloud.
138-
if utils.IsList(method) {
137+
if isList(method) {
139138
switch field.Name {
140139
case "page_size", "page_token", "filter", "order_by":
141140
return true
@@ -148,7 +147,7 @@ func shouldSkipParam(field *api.Field, method *api.Method) bool {
148147
}
149148

150149
// For Update commands, fields marked as IMMUTABLE cannot be changed and should be hidden.
151-
if utils.IsUpdate(method) && slices.Contains(field.Behavior, api.FIELD_BEHAVIOR_IMMUTABLE) {
150+
if isUpdate(method) && slices.Contains(field.Behavior, api.FIELD_BEHAVIOR_IMMUTABLE) {
152151
return true
153152
}
154153

@@ -166,7 +165,7 @@ func addFlattenedParams(field *api.Field, prefix string, args *Arguments, overri
166165

167166
// We check if the current field represents the primary resource of the command.
168167
// This check happens at every level of nesting (e.g., `instance.name`).
169-
if utils.IsPrimaryResource(field, method) {
168+
if isPrimaryResource(field, method) {
170169
param := newPrimaryResourceParam(field, method, model, overrides, service)
171170
args.Params = append(args.Params, param)
172171
return nil
@@ -242,11 +241,11 @@ func newParam(field *api.Field, apiField string, overrides *Config, model *api.A
242241
} else {
243242
// If it's a scalar type (string, int, bool, etc.), we map its proto type
244243
// to the corresponding gcloud type.
245-
param.Type = utils.GetGcloudType(field.Typez)
244+
param.Type = getGcloudType(field.Typez)
246245
}
247246

248247
// For Update commands, maps and repeated fields are often clearable.
249-
if utils.IsUpdate(method) && param.Repeated {
248+
if isUpdate(method) && param.Repeated {
250249
param.Clearable = true
251250
}
252251

@@ -265,56 +264,56 @@ func newParam(field *api.Field, apiField string, overrides *Config, model *api.A
265264
// This is the argument that represents the resource being acted upon (e.g., the instance name).
266265
func newPrimaryResourceParam(field *api.Field, method *api.Method, model *api.API, _ *Config, service *api.Service) Param {
267266
// We first need to get the full resource definition for the method.
268-
resource := utils.GetResourceForMethod(method, model)
267+
resource := getResourceForMethod(method, model)
269268
var segments []api.PathSegment
270269
// TODO(https://github.com/googleapis/librarian/issues/3415): Support multiple resource patterns and multitype resources.
271270
if resource != nil && len(resource.Patterns) > 0 {
272271
segments = resource.Patterns[0]
273272
}
274273

275274
// For List methods, the primary resource is the parent of the method's resource.
276-
if utils.IsList(method) {
277-
segments = utils.GetParentFromSegments(segments)
275+
if isList(method) {
276+
segments = getParentFromSegments(segments)
278277
}
279278

280279
// We determine the singular name of the resource.
281280
resourceName := strcase.ToSnake(strings.TrimSuffix(field.Name, "_id"))
282-
if field.Name == "name" || utils.IsList(method) {
283-
resourceName = utils.GetSingularFromSegments(segments)
281+
if field.Name == "name" || isList(method) {
282+
resourceName = getSingularFromSegments(segments)
284283
}
285284

286285
// We generate a helpful help text.
287286
var helpText string
288287
switch {
289-
case utils.IsCreate(method):
288+
case isCreate(method):
290289
helpText = fmt.Sprintf("The %s to create.", resourceName)
291-
case utils.IsList(method):
292-
helpText = fmt.Sprintf("The project and location for which to retrieve %s information.", utils.GetPluralFromSegments(segments))
290+
case isList(method):
291+
helpText = fmt.Sprintf("The project and location for which to retrieve %s information.", getPluralFromSegments(segments))
293292
default:
294293
helpText = fmt.Sprintf("The %s to operate on.", resourceName)
295294
}
296295

297296
// We construct the gcloud collection path from the resource's pattern string.
298-
collectionPath := utils.GetCollectionPathFromSegments(segments)
297+
collectionPath := getCollectionPathFromSegments(segments)
299298
hostParts := strings.Split(service.DefaultHost, ".")
300299
shortServiceName := hostParts[0]
301300

302301
// We assemble the final `Param` struct.
303302
param := Param{
304303
HelpText: helpText,
305-
IsPositional: !utils.IsList(method),
304+
IsPositional: !isList(method),
306305
IsPrimaryResource: true,
307306
Required: true,
308307
ResourceSpec: &ResourceSpec{
309308
Name: resourceName,
310-
PluralName: utils.GetPluralFromSegments(segments),
309+
PluralName: getPluralFromSegments(segments),
311310
Collection: fmt.Sprintf("%s.%s", shortServiceName, collectionPath),
312311
DisableAutoCompleters: false,
313312
Attributes: newAttributesFromSegments(segments),
314313
},
315314
}
316315

317-
if utils.IsCreate(method) {
316+
if isCreate(method) {
318317
param.RequestIDField = strcase.ToLowerCamel(field.Name)
319318
}
320319

@@ -338,17 +337,17 @@ func newResourceReferenceSpec(field *api.Field, model *api.API, _ *Config, servi
338337
// and falling back to parsing the pattern otherwise.
339338
pluralName := def.Plural
340339
if pluralName == "" {
341-
pluralName = utils.GetPluralFromSegments(segments)
340+
pluralName = getPluralFromSegments(segments)
342341
}
343342

344343
// We determine the singular name from the pattern.
345-
name := utils.GetSingularFromSegments(segments)
344+
name := getSingularFromSegments(segments)
346345

347346
// We construct the full gcloud collection path for the referenced resource
348347
//assuming the current service is the current command.
349348
hostParts := strings.Split(service.DefaultHost, ".")
350349
shortServiceName := hostParts[0]
351-
baseCollectionPath := utils.GetCollectionPathFromSegments(segments)
350+
baseCollectionPath := getCollectionPathFromSegments(segments)
352351
fullCollectionPath := fmt.Sprintf("%s.%s", shortServiceName, baseCollectionPath)
353352

354353
// We assemble and return the `ResourceSpec`.
@@ -420,9 +419,9 @@ func newRequest(method *api.Method, overrides *Config, _ *api.API, service *api.
420419
// MUST match the custom verb defined in the HTTP binding (e.g., ":exportData" -> "exportData").
421420
if len(method.PathInfo.Bindings) > 0 && method.PathInfo.Bindings[0].PathTemplate.Verb != nil {
422421
req.Method = *method.PathInfo.Bindings[0].PathTemplate.Verb
423-
} else if !utils.IsStandardMethod(method) {
422+
} else if !isStandardMethod(method) {
424423
// We treat any non-standard method as a custom method (AIP-136).
425-
commandName, _ := utils.GetCommandName(method)
424+
commandName, _ := getCommandName(method)
426425
// GetCommandName returns snake_case (e.g. "export_data"), but request.method expects camelCase (e.g. "exportData").
427426
req.Method = strcase.ToLowerCamel(commandName)
428427
}
@@ -439,7 +438,7 @@ func newAsync(method *api.Method, model *api.API, _ *Config, service *api.Servic
439438
// Determine if the operation result should be extracted as the resource.
440439
// This is true if the operation response type matches the method's resource type.
441440
// We reuse the 'resource' variable looked up earlier.
442-
resource := utils.GetResourceForMethod(method, model)
441+
resource := getResourceForMethod(method, model)
443442
if resource == nil {
444443
return async
445444
}
@@ -453,7 +452,7 @@ func newAsync(method *api.Method, model *api.API, _ *Config, service *api.Servic
453452
responseTypeName = responseTypeID[idx+1:]
454453
}
455454

456-
singular := utils.GetSingularResourceNameForMethod(method, model)
455+
singular := getSingularResourceNameForMethod(method, model)
457456
if strings.EqualFold(responseTypeName, singular) || strings.HasSuffix(resource.Type, "/"+responseTypeName) {
458457
async.ExtractResourceResult = true
459458
} else {
@@ -477,7 +476,7 @@ func newCollectionPath(method *api.Method, service *api.Service, isAsync bool) [
477476
continue
478477
}
479478

480-
basePath := utils.ExtractPathFromSegments(binding.PathTemplate.Segments)
479+
basePath := extractPathFromSegments(binding.PathTemplate.Segments)
481480

482481
if basePath == "" {
483482
continue
@@ -507,11 +506,11 @@ func newCollectionPath(method *api.Method, service *api.Service, isAsync bool) [
507506
// newOutputConfig generates the output configuration for List commands.
508507
func newOutputConfig(method *api.Method, _ *api.API) *OutputConfig {
509508
// We only generate output config for list methods.
510-
if !utils.IsList(method) {
509+
if !isList(method) {
511510
return nil
512511
}
513512

514-
resourceMsg := utils.FindResourceMessage(method.OutputType)
513+
resourceMsg := findResourceMessage(method.OutputType)
515514
if resourceMsg == nil {
516515
return nil
517516
}
@@ -533,7 +532,7 @@ func newFormat(message *api.Message) string {
533532

534533
for _, f := range message.Fields {
535534
// Sanitize field name to prevent DSL injection.
536-
if !utils.IsSafeName(f.JSONName) {
535+
if !isSafeName(f.JSONName) {
537536
continue
538537
}
539538

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,16 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
// Package utils provides utility functions for the gcloud generator.
16-
package utils
15+
package gcloud
1716

1817
import (
1918
"fmt"
2019

2120
"github.com/googleapis/librarian/internal/sidekick/api"
2221
)
2322

24-
// GetGcloudType maps an API field type to the corresponding gcloud argparse type.
25-
func GetGcloudType(t api.Typez) string {
23+
// getGcloudType maps an API field type to the corresponding gcloud argparse type.
24+
func getGcloudType(t api.Typez) string {
2625
switch t {
2726
case api.DOUBLE_TYPE, api.FLOAT_TYPE:
2827
return "float"
@@ -43,9 +42,9 @@ func GetGcloudType(t api.Typez) string {
4342
}
4443
}
4544

46-
// IsSafeName checks if a name contains only safe characters (alphanumeric, underscores, dots).
45+
// isSafeName checks if a name contains only safe characters (alphanumeric, underscores, dots).
4746
// This prevents injection vulnerabilities when generating code or templates.
48-
func IsSafeName(name string) bool {
47+
func isSafeName(name string) bool {
4948
for _, r := range name {
5049
switch {
5150
case r >= 'a' && r <= 'z':

internal/surfer/gcloud/utils/field_test.go renamed to internal/surfer/gcloud/field_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 Google LLC
1+
// Copyright 2026 Google LLC
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -12,7 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
package utils
15+
package gcloud
1616

1717
import (
1818
"testing"
@@ -40,9 +40,9 @@ func TestGetGcloudType(t *testing.T) {
4040
} {
4141
t.Run(test.name, func(t *testing.T) {
4242
t.Parallel()
43-
got := GetGcloudType(test.typez)
43+
got := getGcloudType(test.typez)
4444
if got != test.want {
45-
t.Errorf("GetGcloudType() = %q, want %q", got, test.want)
45+
t.Errorf("getGcloudType(%v) = %q, want %q", test.typez, got, test.want)
4646
}
4747
})
4848
}
@@ -64,9 +64,9 @@ func TestIsSafeName(t *testing.T) {
6464
} {
6565
t.Run(test.name, func(t *testing.T) {
6666
t.Parallel()
67-
got := IsSafeName(test.name)
67+
got := isSafeName(test.name)
6868
if got != test.want {
69-
t.Errorf("IsSafeName() = %v, want %v", got, test.want)
69+
t.Errorf("isSafeName(%q) = %v, want %v", test.name, got, test.want)
7070
}
7171
})
7272
}
@@ -76,8 +76,8 @@ func TestGetGcloudType_Panic(t *testing.T) {
7676
t.Parallel()
7777
defer func() {
7878
if r := recover(); r == nil {
79-
t.Errorf("GetGcloudType() did not panic for unsupported type")
79+
t.Errorf("getGcloudType() did not panic for unsupported type")
8080
}
8181
}()
82-
GetGcloudType(api.Typez(999))
82+
getGcloudType(api.Typez(999))
8383
}

internal/surfer/gcloud/generate.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"text/template"
2525

2626
"github.com/googleapis/librarian/internal/sidekick/api"
27-
"github.com/googleapis/librarian/internal/surfer/gcloud/utils"
2827
"github.com/googleapis/librarian/internal/yaml"
2928
"github.com/iancoleman/strcase"
3029
)
@@ -76,9 +75,9 @@ func generateService(service *api.Service, overrides *Config, model *api.API, ou
7675
return fmt.Errorf("failed to create surface directory for %q: %w", shortServiceName, err)
7776
}
7877

79-
track := strings.ToUpper(utils.InferTrackFromPackage(service.Package))
78+
track := strings.ToUpper(inferTrackFromPackage(service.Package))
8079
data := commandGroupData{
81-
ServiceTitle: utils.GetServiceTitle(model, shortServiceName),
80+
ServiceTitle: getServiceTitle(model, shortServiceName),
8281
ClassNamePrefix: strcase.ToCamel(shortServiceName),
8382
Tracks: []string{track},
8483
}
@@ -97,7 +96,7 @@ func generateService(service *api.Service, overrides *Config, model *api.API, ou
9796
// For each method, we determine the plural name of the resource it operates on.
9897
// This plural name (e.g., "instances") will serve as our collection ID.
9998
// Example: For the `CreateInstance` method, this will return "instances".
100-
collectionID := utils.GetPluralResourceNameForMethod(method, model)
99+
collectionID := getPluralResourceNameForMethod(method, model)
101100

102101
// If a collection ID is found, we add the method to our map.
103102
if collectionID != "" {
@@ -136,14 +135,14 @@ func generateResourceCommands(collectionID string, methods []*api.Method, baseDi
136135
return fmt.Errorf("failed to create resource directory for %q: %w", collectionID, err)
137136
}
138137

139-
singular := utils.GetSingularResourceNameForMethod(methods[0], model)
138+
singular := getSingularResourceNameForMethod(methods[0], model)
140139

141140
// We determine the short service name from the default host to use as a fallback title.
142141
shortServiceName, _, _ := strings.Cut(service.DefaultHost, ".")
143142

144-
track := strings.ToUpper(utils.InferTrackFromPackage(service.Package))
143+
track := strings.ToUpper(inferTrackFromPackage(service.Package))
145144
data := commandGroupData{
146-
ServiceTitle: utils.GetServiceTitle(model, shortServiceName),
145+
ServiceTitle: getServiceTitle(model, shortServiceName),
147146
ResourceSingular: singular,
148147
ClassNamePrefix: strcase.ToCamel(collectionID),
149148
Tracks: []string{track},
@@ -164,7 +163,7 @@ func generateResourceCommands(collectionID string, methods []*api.Method, baseDi
164163
for _, method := range methods {
165164
// We map the API method to a standard gcloud command name.
166165
// Example: `CreateInstance` -> "create"
167-
verb, err := utils.GetCommandName(method)
166+
verb, err := getCommandName(method)
168167
if err != nil {
169168
// Continue to the next method if we can't determine a command name,
170169
// logging the issue might be useful here in the future.

0 commit comments

Comments
 (0)