Skip to content

Commit d33029a

Browse files
authored
feat(internal/librarian/nodejs): add omit_common_resources flag (#6181)
This PR resolves the global path helper methods drift across the Node.js monorepo by introducing and auto-detecting a new `omit_common_resources` configuration flag.
1 parent 21476f7 commit d33029a

6 files changed

Lines changed: 320 additions & 18 deletions

File tree

doc/config-schema.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,7 @@ This document describes the schema for the librarian.yaml.
346346
| :--- | :--- | :--- |
347347
| `additional_protos` | list of string | Is a list of additional proto files to include in generation. |
348348
| `diregapic` | bool | Indicates whether generation uses DIREGAPIC (Discovery REST GAPICs). This is typically false. Used for the GCE (compute) client. |
349+
| `omit_common_resources` | bool | Indicates whether to omit the default inclusion of google/cloud/common_resources.proto. |
349350
| `path` | string | Is the source path. |
350351

351352
## NodejsPackage Configuration

internal/config/language.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,10 @@ type NodejsAPI struct {
779779
// This is typically false. Used for the GCE (compute) client.
780780
DIREGAPIC bool `yaml:"diregapic,omitempty"`
781781

782+
// OmitCommonResources indicates whether to omit the default inclusion of
783+
// google/cloud/common_resources.proto.
784+
OmitCommonResources bool `yaml:"omit_common_resources,omitempty"`
785+
782786
// Path is the source path.
783787
Path string `yaml:"path,omitempty"`
784788
}

internal/librarian/nodejs/generate.go

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -127,28 +127,44 @@ func generateAPI(ctx context.Context, api *config.API, library *config.Library,
127127
// applying default values if no explicit configuration is found in the library.
128128
func resolveNodejsAPI(library *config.Library, api *config.API) *config.NodejsAPI {
129129
res := &config.NodejsAPI{
130-
Path: api.Path,
131-
AdditionalProtos: []string{commonProtos},
130+
Path: api.Path,
132131
}
132+
133+
var apiConfig *config.NodejsAPI
134+
if library.Nodejs != nil {
135+
for _, nodejsAPI := range library.Nodejs.NodejsAPIs {
136+
if nodejsAPI.Path == api.Path {
137+
apiConfig = nodejsAPI
138+
break
139+
}
140+
}
141+
}
142+
143+
omitCommon := false
144+
if apiConfig != nil {
145+
omitCommon = apiConfig.OmitCommonResources
146+
res.DIREGAPIC = apiConfig.DIREGAPIC
147+
res.OmitCommonResources = apiConfig.OmitCommonResources
148+
}
149+
150+
var protos []string
151+
if !omitCommon {
152+
protos = append(protos, commonProtos)
153+
}
154+
133155
if library.Nodejs == nil {
156+
res.AdditionalProtos = protos
134157
return res
135158
}
136159

137-
// Always include commonProtos as a base.
138-
protos := []string{commonProtos}
139-
140160
// Add package-level additional protos.
141161
protos = append(protos, library.Nodejs.AdditionalProtos...)
142162

143-
for _, nodejsAPI := range library.Nodejs.NodejsAPIs {
144-
if nodejsAPI.Path != api.Path {
145-
continue
146-
}
147-
// Add API-level additional protos.
148-
protos = append(protos, nodejsAPI.AdditionalProtos...)
149-
res.DIREGAPIC = nodejsAPI.DIREGAPIC
150-
break
163+
// Add API-level additional protos.
164+
if apiConfig != nil {
165+
protos = append(protos, apiConfig.AdditionalProtos...)
151166
}
167+
152168
res.AdditionalProtos = unique(protos)
153169
return res
154170
}

internal/librarian/nodejs/generate_test.go

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,6 +1214,128 @@ func TestResolveNodejsAPI(t *testing.T) {
12141214
DIREGAPIC: true,
12151215
},
12161216
},
1217+
{
1218+
name: "omit common resources is true",
1219+
library: &config.Library{
1220+
Nodejs: &config.NodejsPackage{
1221+
NodejsAPIs: []*config.NodejsAPI{
1222+
{
1223+
Path: "google/api/cloudquotas/v1",
1224+
OmitCommonResources: true,
1225+
},
1226+
},
1227+
},
1228+
},
1229+
api: &config.API{Path: "google/api/cloudquotas/v1"},
1230+
want: &config.NodejsAPI{
1231+
Path: "google/api/cloudquotas/v1",
1232+
OmitCommonResources: true,
1233+
AdditionalProtos: nil,
1234+
},
1235+
},
1236+
{
1237+
name: "omit common resources is true, package-level additional protos preserved",
1238+
library: &config.Library{
1239+
Nodejs: &config.NodejsPackage{
1240+
AdditionalProtos: []string{"pkg.proto"},
1241+
NodejsAPIs: []*config.NodejsAPI{
1242+
{
1243+
Path: "google/cloud/secretmanager/v1",
1244+
OmitCommonResources: true,
1245+
},
1246+
},
1247+
},
1248+
},
1249+
api: &config.API{Path: "google/cloud/secretmanager/v1"},
1250+
want: &config.NodejsAPI{
1251+
Path: "google/cloud/secretmanager/v1",
1252+
OmitCommonResources: true,
1253+
AdditionalProtos: []string{"pkg.proto"},
1254+
},
1255+
},
1256+
{
1257+
name: "omit common resources is true, api-level additional protos preserved",
1258+
library: &config.Library{
1259+
Nodejs: &config.NodejsPackage{
1260+
NodejsAPIs: []*config.NodejsAPI{
1261+
{
1262+
Path: "google/cloud/secretmanager/v1",
1263+
OmitCommonResources: true,
1264+
AdditionalProtos: []string{"api.proto"},
1265+
},
1266+
},
1267+
},
1268+
},
1269+
api: &config.API{Path: "google/cloud/secretmanager/v1"},
1270+
want: &config.NodejsAPI{
1271+
Path: "google/cloud/secretmanager/v1",
1272+
OmitCommonResources: true,
1273+
AdditionalProtos: []string{"api.proto"},
1274+
},
1275+
},
1276+
{
1277+
name: "omit common resources is true, package-level and api-level protos combined",
1278+
library: &config.Library{
1279+
Nodejs: &config.NodejsPackage{
1280+
AdditionalProtos: []string{"pkg.proto"},
1281+
NodejsAPIs: []*config.NodejsAPI{
1282+
{
1283+
Path: "google/cloud/secretmanager/v1",
1284+
OmitCommonResources: true,
1285+
AdditionalProtos: []string{"api.proto"},
1286+
},
1287+
},
1288+
},
1289+
},
1290+
api: &config.API{Path: "google/cloud/secretmanager/v1"},
1291+
want: &config.NodejsAPI{
1292+
Path: "google/cloud/secretmanager/v1",
1293+
OmitCommonResources: true,
1294+
AdditionalProtos: []string{"pkg.proto", "api.proto"},
1295+
},
1296+
},
1297+
{
1298+
name: "omit common resources is false, all preserved",
1299+
library: &config.Library{
1300+
Nodejs: &config.NodejsPackage{
1301+
AdditionalProtos: []string{"pkg.proto"},
1302+
NodejsAPIs: []*config.NodejsAPI{
1303+
{
1304+
Path: "google/cloud/secretmanager/v1",
1305+
OmitCommonResources: false,
1306+
AdditionalProtos: []string{"api.proto"},
1307+
},
1308+
},
1309+
},
1310+
},
1311+
api: &config.API{Path: "google/cloud/secretmanager/v1"},
1312+
want: &config.NodejsAPI{
1313+
Path: "google/cloud/secretmanager/v1",
1314+
OmitCommonResources: false,
1315+
AdditionalProtos: []string{commonProtos, "pkg.proto", "api.proto"},
1316+
},
1317+
},
1318+
{
1319+
name: "duplicated protos",
1320+
library: &config.Library{
1321+
Nodejs: &config.NodejsPackage{
1322+
AdditionalProtos: []string{"pkg.proto", "dup.proto"},
1323+
NodejsAPIs: []*config.NodejsAPI{
1324+
{
1325+
Path: "google/cloud/secretmanager/v1",
1326+
OmitCommonResources: true,
1327+
AdditionalProtos: []string{"dup.proto", "api.proto"},
1328+
},
1329+
},
1330+
},
1331+
},
1332+
api: &config.API{Path: "google/cloud/secretmanager/v1"},
1333+
want: &config.NodejsAPI{
1334+
Path: "google/cloud/secretmanager/v1",
1335+
OmitCommonResources: true,
1336+
AdditionalProtos: []string{"pkg.proto", "dup.proto", "api.proto"},
1337+
},
1338+
},
12171339
} {
12181340
t.Run(test.name, func(t *testing.T) {
12191341
got := resolveNodejsAPI(test.library, test.api)

tool/cmd/migrate/nodejs.go

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"sort"
2828
"strings"
2929

30+
"github.com/bazelbuild/buildtools/build"
3031
"github.com/googleapis/librarian/internal/config"
3132
"github.com/googleapis/librarian/internal/librarian"
3233
"github.com/googleapis/librarian/internal/yaml"
@@ -60,6 +61,7 @@ type nodejsGapicInfo struct {
6061
handwrittenLayer bool
6162
mainService string
6263
mixins string
64+
omitCommonResources bool
6365
}
6466

6567
// owlBotSourceRegex extracts the base API path from an .OwlBot.yaml
@@ -265,11 +267,20 @@ func buildNodejsLibraryAPIs(googleapisDir string, apis []*config.API) []*config.
265267
var nodejsAPIs []*config.NodejsAPI
266268
for _, api := range apis {
267269
info, err := parseBazelNodejsInfo(googleapisDir, api.Path)
268-
if err == nil && info != nil && info.diregapic {
269-
nodejsAPIs = append(nodejsAPIs, &config.NodejsAPI{
270-
Path: api.Path,
271-
DIREGAPIC: true,
272-
})
270+
if err != nil || info == nil {
271+
continue
272+
}
273+
if info.diregapic || info.omitCommonResources {
274+
apiConfig := &config.NodejsAPI{
275+
Path: api.Path,
276+
}
277+
if info.diregapic {
278+
apiConfig.DIREGAPIC = true
279+
}
280+
if info.omitCommonResources {
281+
apiConfig.OmitCommonResources = true
282+
}
283+
nodejsAPIs = append(nodejsAPIs, apiConfig)
273284
}
274285
}
275286
return nodejsAPIs
@@ -369,12 +380,39 @@ func parseBazelNodejsInfo(googleapisDir, apiDir string) (*nodejsGapicInfo, error
369380
if len(extraProtocParameters) == 0 {
370381
extraProtocParameters = nil
371382
}
383+
384+
src := rule.AttrString("src")
385+
omitCommon := false
386+
if src != "" {
387+
srcName := strings.TrimPrefix(src, ":")
388+
protoRules := file.Rules("proto_library_with_info")
389+
var protoRule *build.Rule
390+
for _, r := range protoRules {
391+
if r.AttrString("name") == srcName {
392+
protoRule = r
393+
break
394+
}
395+
}
396+
if protoRule != nil {
397+
if attr := protoRule.Attr("deps"); attr != nil {
398+
omitCommon = true
399+
for _, dep := range extractStrings(attr) {
400+
if strings.HasSuffix(dep, "google/cloud:common_resources_proto") {
401+
omitCommon = false
402+
break
403+
}
404+
}
405+
}
406+
}
407+
}
408+
372409
info := &nodejsGapicInfo{
373410
packageName: rule.AttrString("package_name"),
374411
bundleConfig: rule.AttrString("bundle_config"),
375412
extraProtocParameters: extraProtocParameters,
376413
mainService: rule.AttrString("main_service"),
377414
mixins: rule.AttrString("mixins"),
415+
omitCommonResources: omitCommon,
378416
}
379417
if rule.AttrLiteral("diregapic") == "True" {
380418
info.diregapic = true

0 commit comments

Comments
 (0)