Skip to content

Commit 4e2fdfc

Browse files
jameslynnwugemini-code-assist[bot]
jameslynnwu
and
gemini-code-assist[bot]
authored
refactor(internal/sidekick/parser): migrate sidekick parsers to use ModelConfig (#3941)
This commit updates parser.CreateModel to use ModelConfig instead of sidekickconfig.Config. Part of #3662 --------- Signed-off-by: James Wu <jameslynnwu@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 9633c10 commit 4e2fdfc

File tree

19 files changed

+129
-111
lines changed

19 files changed

+129
-111
lines changed

‎internal/librarian/dart/generate.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func Generate(ctx context.Context, library *config.Library, sources *source.Sour
3434
if err != nil {
3535
return err
3636
}
37-
model, err := parser.CreateModel(sidekickConfig)
37+
model, err := parser.CreateModel(parser.NewModelConfigFromSidekickConfig(sidekickConfig))
3838
if err != nil {
3939
return err
4040
}

‎internal/librarian/rust/generate.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func Generate(ctx context.Context, library *config.Library, sources *source.Sour
4343
if err != nil {
4444
return err
4545
}
46-
model, err := parser.CreateModel(sidekickConfig)
46+
model, err := parser.CreateModel(parser.NewModelConfigFromSidekickConfig(sidekickConfig))
4747
if err != nil {
4848
return err
4949
}
@@ -96,7 +96,7 @@ func generateVeneer(ctx context.Context, library *config.Library, sources *sourc
9696
if err != nil {
9797
return fmt.Errorf("module %q: %w", module.Output, err)
9898
}
99-
model, err := parser.CreateModel(sidekickConfig)
99+
model, err := parser.CreateModel(parser.NewModelConfigFromSidekickConfig(sidekickConfig))
100100
if err != nil {
101101
return fmt.Errorf("module %q: %w", module.Output, err)
102102
}
@@ -185,7 +185,7 @@ func generateRustStorage(ctx context.Context, library *config.Library, moduleOut
185185
if err != nil {
186186
return fmt.Errorf("failed to create storage sidekick config: %w", err)
187187
}
188-
storageModel, err := parser.CreateModel(storageConfig)
188+
storageModel, err := parser.CreateModel(parser.NewModelConfigFromSidekickConfig(storageConfig))
189189
if err != nil {
190190
return fmt.Errorf("failed to create storage model: %w", err)
191191
}
@@ -199,7 +199,7 @@ func generateRustStorage(ctx context.Context, library *config.Library, moduleOut
199199
if err != nil {
200200
return fmt.Errorf("failed to create control sidekick config: %w", err)
201201
}
202-
controlModel, err := parser.CreateModel(controlConfig)
202+
controlModel, err := parser.CreateModel(parser.NewModelConfigFromSidekickConfig(controlConfig))
203203
if err != nil {
204204
return fmt.Errorf("failed to create control model: %w", err)
205205
}

‎internal/sidekick/codec_sample/generate_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func TestFromProtobuf(t *testing.T) {
5353
"proto:google.cloud.location": "package:google_cloud_location/location.dart",
5454
},
5555
}
56-
model, err := parser.CreateModel(cfg)
56+
model, err := parser.CreateModel(parser.NewModelConfigFromSidekickConfig(cfg))
5757
if err != nil {
5858
t.Fatal(err)
5959
}

‎internal/sidekick/dart/generate_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func TestFromProtobuf(t *testing.T) {
6161
"proto:google.cloud.location": "package:google_cloud_location/location.dart",
6262
},
6363
}
64-
model, err := parser.CreateModel(cfg)
64+
model, err := parser.CreateModel(parser.NewModelConfigFromSidekickConfig(cfg))
6565
if err != nil {
6666
t.Fatal(err)
6767
}

‎internal/sidekick/parser/disco.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import (
2525

2626
// ParseDisco reads discovery docs specifications and converts them into
2727
// the `api.API` model.
28-
func ParseDisco(cfg *config.Config) (*api.API, error) {
29-
source := cfg.General.SpecificationSource
28+
func ParseDisco(cfg ModelConfig) (*api.API, error) {
29+
source := cfg.SpecificationSource
3030
for _, opt := range config.SourceRoots(cfg.Source) {
3131
location, ok := cfg.Source[opt]
3232
if !ok {
@@ -47,7 +47,7 @@ func ParseDisco(cfg *config.Config) (*api.API, error) {
4747
if err != nil {
4848
return nil, err
4949
}
50-
result, err := discovery.NewAPI(serviceConfig, contents, cfg)
50+
result, err := discovery.NewAPI(serviceConfig, contents, cfg.Discovery)
5151
if err != nil {
5252
return nil, err
5353
}

‎internal/sidekick/parser/disco_test.go‎

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,13 @@ import (
2020
"github.com/google/go-cmp/cmp"
2121
"github.com/google/go-cmp/cmp/cmpopts"
2222
"github.com/googleapis/librarian/internal/sidekick/api"
23-
"github.com/googleapis/librarian/internal/sidekick/config"
2423
)
2524

2625
func TestDisco_Parse(t *testing.T) {
27-
cfg := &config.Config{
28-
General: config.GeneralConfig{
29-
// Mixing Compute and Secret Manager like this is fine in tests.
30-
SpecificationSource: discoSourceFile,
31-
ServiceConfig: secretManagerYamlFullPath,
32-
},
26+
cfg := ModelConfig{
27+
// Mixing Compute and Secret Manager like this is fine in tests.
28+
SpecificationSource: discoSourceFile,
29+
ServiceConfig: secretManagerYamlFullPath,
3330
}
3431
got, err := ParseDisco(cfg)
3532
if err != nil {
@@ -54,10 +51,8 @@ func TestDisco_Parse(t *testing.T) {
5451
}
5552

5653
func TestDisco_FindSources(t *testing.T) {
57-
cfg := &config.Config{
58-
General: config.GeneralConfig{
59-
SpecificationSource: discoSourceFileRelative,
60-
},
54+
cfg := ModelConfig{
55+
SpecificationSource: discoSourceFileRelative,
6156
Source: map[string]string{
6257
"test-root": mainTestdataDir,
6358
"roots": "undefined,test",
@@ -83,10 +78,8 @@ func TestDisco_FindSources(t *testing.T) {
8378
}
8479

8580
func TestDisco_ParseNoServiceConfig(t *testing.T) {
86-
cfg := &config.Config{
87-
General: config.GeneralConfig{
88-
SpecificationSource: discoSourceFile,
89-
},
81+
cfg := ModelConfig{
82+
SpecificationSource: discoSourceFile,
9083
}
9184
got, err := ParseDisco(cfg)
9285
if err != nil {
@@ -107,10 +100,8 @@ func TestDisco_ParseNoServiceConfig(t *testing.T) {
107100
}
108101

109102
func TestDisco_ParsePagination(t *testing.T) {
110-
cfg := &config.Config{
111-
General: config.GeneralConfig{
112-
SpecificationSource: discoSourceFile,
113-
},
103+
cfg := ModelConfig{
104+
SpecificationSource: discoSourceFile,
114105
}
115106
model, err := ParseDisco(cfg)
116107
if err != nil {
@@ -136,10 +127,8 @@ func TestDisco_ParsePagination(t *testing.T) {
136127
}
137128

138129
func TestDisco_ParsePaginationAggregate(t *testing.T) {
139-
cfg := &config.Config{
140-
General: config.GeneralConfig{
141-
SpecificationSource: discoSourceFile,
142-
},
130+
cfg := ModelConfig{
131+
SpecificationSource: discoSourceFile,
143132
}
144133
model, err := ParseDisco(cfg)
145134
if err != nil {
@@ -165,10 +154,8 @@ func TestDisco_ParsePaginationAggregate(t *testing.T) {
165154
}
166155

167156
func TestDisco_ParseDeprecatedEnum(t *testing.T) {
168-
cfg := &config.Config{
169-
General: config.GeneralConfig{
170-
SpecificationSource: discoSourceFile,
171-
},
157+
cfg := ModelConfig{
158+
SpecificationSource: discoSourceFile,
172159
}
173160
model, err := ParseDisco(cfg)
174161
if err != nil {
@@ -190,12 +177,12 @@ func TestDisco_ParseDeprecatedEnum(t *testing.T) {
190177
}
191178

192179
func TestDisco_ParseBadFiles(t *testing.T) {
193-
for _, cfg := range []config.GeneralConfig{
180+
for _, cfg := range []ModelConfig{
194181
{SpecificationSource: "-invalid-file-name-", ServiceConfig: secretManagerYamlFullPath},
195182
{SpecificationSource: discoSourceFile, ServiceConfig: "-invalid-file-name-"},
196183
{SpecificationSource: secretManagerYamlFullPath, ServiceConfig: secretManagerYamlFullPath},
197184
} {
198-
if got, err := ParseDisco(&config.Config{General: cfg}); err == nil {
185+
if got, err := ParseDisco(cfg); err == nil {
199186
t.Fatalf("expected error with missing source file, got=%v", got)
200187
}
201188
}

‎internal/sidekick/parser/discovery/discovery.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import (
3131
)
3232

3333
// NewAPI parses the discovery doc in `contents` and returns the corresponding `api.API` model.
34-
func NewAPI(serviceConfig *serviceconfig.Service, contents []byte, cfg *config.Config) (*api.API, error) {
34+
func NewAPI(serviceConfig *serviceconfig.Service, contents []byte, discoveryConfig *config.Discovery) (*api.API, error) {
3535
doc, err := newDiscoDocument(contents)
3636
if err != nil {
3737
return nil, err
@@ -102,7 +102,7 @@ func NewAPI(serviceConfig *serviceconfig.Service, contents []byte, cfg *config.C
102102
// output on each run.
103103
slices.SortStableFunc(result.Services, compareServices)
104104

105-
lroAnnotations(result, cfg)
105+
lroAnnotations(result, discoveryConfig)
106106

107107
return result, nil
108108
}

‎internal/sidekick/parser/discovery/discovery_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,11 +234,11 @@ func ComputeDisco(t *testing.T, sc *serviceconfig.Service) (*api.API, error) {
234234
return NewAPI(sc, contents, nil)
235235
}
236236

237-
func ComputeDiscoWithLros(t *testing.T, cfg *config.Config) (*api.API, error) {
237+
func ComputeDiscoWithLros(t *testing.T, discoveryConfig *config.Discovery) (*api.API, error) {
238238
t.Helper()
239239
contents, err := os.ReadFile(computeDiscoveryFile)
240240
if err != nil {
241241
return nil, err
242242
}
243-
return NewAPI(nil, contents, cfg)
243+
return NewAPI(nil, contents, discoveryConfig)
244244
}

‎internal/sidekick/parser/discovery/lro.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,21 +22,21 @@ import (
2222
"github.com/googleapis/librarian/internal/sidekick/config"
2323
)
2424

25-
func lroAnnotations(model *api.API, cfg *config.Config) error {
26-
if cfg == nil || cfg.Discovery == nil {
25+
func lroAnnotations(model *api.API, discoveryConfig *config.Discovery) error {
26+
if discoveryConfig == nil {
2727
return nil
2828
}
29-
lroServices := cfg.Discovery.LroServices()
29+
lroServices := discoveryConfig.LroServices()
3030
for _, svc := range model.Services {
3131
if _, ok := lroServices[svc.ID]; ok {
3232
continue
3333
}
3434
var svcMixin *api.Method
3535
for _, method := range svc.Methods {
36-
if method.OutputTypeID != cfg.Discovery.OperationID {
36+
if method.OutputTypeID != discoveryConfig.OperationID {
3737
continue
3838
}
39-
mixin, pathParams := lroFindPoller(method, model, cfg.Discovery)
39+
mixin, pathParams := lroFindPoller(method, model, discoveryConfig)
4040
if mixin == nil {
4141
continue
4242
}

‎internal/sidekick/parser/discovery/lro_test.go‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,14 @@ import (
2424
)
2525

2626
func TestLroAnnotations(t *testing.T) {
27-
cfg := &config.Config{
28-
Discovery: &config.Discovery{
29-
OperationID: "..Operation",
30-
Pollers: []*config.Poller{
31-
{Prefix: "compute/v1/projects/{project}/zones/{zone}", MethodID: "..zoneOperations.get"},
32-
{Prefix: "compute/v1/projects/{project}/regions/{region}", MethodID: "..regionOperations.get"},
33-
},
27+
discoveryConfig := &config.Discovery{
28+
OperationID: "..Operation",
29+
Pollers: []*config.Poller{
30+
{Prefix: "compute/v1/projects/{project}/zones/{zone}", MethodID: "..zoneOperations.get"},
31+
{Prefix: "compute/v1/projects/{project}/regions/{region}", MethodID: "..regionOperations.get"},
3432
},
3533
}
36-
model, err := ComputeDiscoWithLros(t, cfg)
34+
model, err := ComputeDiscoWithLros(t, discoveryConfig)
3735
if err != nil {
3836
t.Fatal(err)
3937
}
@@ -111,13 +109,11 @@ func TestLroAnnotations(t *testing.T) {
111109
}
112110

113111
func TestLroAnnotationsError(t *testing.T) {
114-
cfg := &config.Config{
115-
Discovery: &config.Discovery{
116-
OperationID: "..Operation",
117-
Pollers: []*config.Poller{
118-
{Prefix: "p/{project}/l/{zone}", MethodID: "..Operations.get_1"},
119-
{Prefix: "p/{project}/l/{region}", MethodID: "..Operations.get_2"},
120-
},
112+
discoveryConfig := &config.Discovery{
113+
OperationID: "..Operation",
114+
Pollers: []*config.Poller{
115+
{Prefix: "p/{project}/l/{zone}", MethodID: "..Operations.get_1"},
116+
{Prefix: "p/{project}/l/{region}", MethodID: "..Operations.get_2"},
121117
},
122118
}
123119

@@ -221,7 +217,7 @@ func TestLroAnnotationsError(t *testing.T) {
221217
}
222218

223219
model := api.NewTestAPI([]*api.Message{operation}, []*api.Enum{}, []*api.Service{badService, pollerService})
224-
if lroAnnotations(model, cfg) == nil {
220+
if lroAnnotations(model, discoveryConfig) == nil {
225221
t.Errorf("expected an error, got %v", badService)
226222
}
227223
}

0 commit comments

Comments
 (0)