Skip to content

Commit 0190366

Browse files
JoeWang1127JoeWang1127
authored
feat(internal/docker): extends configure-request.json (#1715)
Fixes #1653
1 parent 0c92cde commit 0190366

File tree

11 files changed

+207
-111
lines changed

11 files changed

+207
-111
lines changed

‎internal/config/state.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ import (
2121
"strings"
2222
)
2323

24+
const (
25+
StatusNew = "new"
26+
StatusExisting = "existing"
27+
)
28+
2429
// LibrarianState defines the contract for the state.yaml file.
2530
type LibrarianState struct {
2631
// The name and tag of the generator image to use. tag is required.
@@ -189,6 +194,9 @@ type API struct {
189194
Path string `yaml:"path" json:"path"`
190195
// The name of the service config file, relative to the API `path`.
191196
ServiceConfig string `yaml:"service_config" json:"service_config"`
197+
// The status of the API, one of "new" or "existing".
198+
// This field is ignored when writing to state.yaml.
199+
Status string `yaml:"-" json:"status"`
192200
}
193201

194202
// Validate checks that the API is valid.

‎internal/config/state_test.go‎

Lines changed: 97 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,16 @@
1515
package config
1616

1717
import (
18+
"strings"
1819
"testing"
1920
)
2021

2122
func TestLibrarianState_Validate(t *testing.T) {
2223
for _, test := range []struct {
23-
name string
24-
state *LibrarianState
25-
wantErr bool
24+
name string
25+
state *LibrarianState
26+
wantErr bool
27+
wantErrMsg string
2628
}{
2729
{
2830
name: "valid state",
@@ -56,29 +58,44 @@ func TestLibrarianState_Validate(t *testing.T) {
5658
},
5759
},
5860
},
59-
wantErr: true,
61+
wantErr: true,
62+
wantErrMsg: "image is required",
6063
},
6164
{
6265
name: "missing libraries",
6366
state: &LibrarianState{
6467
Image: "gcr.io/test/image:v1.2.3",
6568
},
66-
wantErr: true,
69+
wantErr: true,
70+
wantErrMsg: "libraries cannot be empty",
6771
},
6872
} {
6973
t.Run(test.name, func(t *testing.T) {
70-
if err := test.state.Validate(); (err != nil) != test.wantErr {
71-
t.Errorf("LibrarianState.Validate() error = %v, wantErr %v", err, test.wantErr)
74+
err := test.state.Validate()
75+
if test.wantErr {
76+
if err == nil {
77+
t.Error("Librarian.Validate() should fail")
78+
}
79+
if !strings.Contains(err.Error(), test.wantErrMsg) {
80+
t.Errorf("want error message %q, got %q", test.wantErrMsg, err.Error())
81+
}
82+
83+
return
84+
}
85+
86+
if err != nil {
87+
t.Errorf("Librarian.Validate() error = %v, wantErr %v", err, test.wantErr)
7288
}
7389
})
7490
}
7591
}
7692

7793
func TestLibrary_Validate(t *testing.T) {
7894
for _, test := range []struct {
79-
name string
80-
library *LibraryState
81-
wantErr bool
95+
name string
96+
library *LibraryState
97+
wantErr bool
98+
wantErrMsg string
8299
}{
83100
{
84101
name: "valid library",
@@ -93,42 +110,26 @@ func TestLibrary_Validate(t *testing.T) {
93110
},
94111
},
95112
{
96-
name: "missing id",
97-
library: &LibraryState{
98-
SourceRoots: []string{"src/a", "src/b"},
99-
APIs: []*API{
100-
{
101-
Path: "a/b/v1",
102-
},
103-
},
104-
},
105-
wantErr: true,
113+
name: "missing id",
114+
library: &LibraryState{},
115+
wantErr: true,
116+
wantErrMsg: "id is required",
106117
},
107118
{
108119
name: "id is dot",
109120
library: &LibraryState{
110-
ID: ".",
111-
SourceRoots: []string{"src/a", "src/b"},
112-
APIs: []*API{
113-
{
114-
Path: "a/b/v1",
115-
},
116-
},
121+
ID: ".",
117122
},
118-
wantErr: true,
123+
wantErr: true,
124+
wantErrMsg: "id cannot be",
119125
},
120126
{
121127
name: "id is double dot",
122128
library: &LibraryState{
123-
ID: "..",
124-
SourceRoots: []string{"src/a", "src/b"},
125-
APIs: []*API{
126-
{
127-
Path: "a/b/v1",
128-
},
129-
},
129+
ID: "..",
130130
},
131-
wantErr: true,
131+
wantErr: true,
132+
wantErrMsg: "id cannot be",
132133
},
133134
{
134135
name: "missing source paths",
@@ -140,15 +141,17 @@ func TestLibrary_Validate(t *testing.T) {
140141
},
141142
},
142143
},
143-
wantErr: true,
144+
wantErr: true,
145+
wantErrMsg: "source_roots cannot be empty",
144146
},
145147
{
146148
name: "missing apis",
147149
library: &LibraryState{
148150
ID: "a/b",
149151
SourceRoots: []string{"src/a", "src/b"},
150152
},
151-
wantErr: true,
153+
wantErr: true,
154+
wantErrMsg: "apis cannot be empty",
152155
},
153156
{
154157
name: "valid version without v prefix",
@@ -166,43 +169,28 @@ func TestLibrary_Validate(t *testing.T) {
166169
{
167170
name: "invalid id characters",
168171
library: &LibraryState{
169-
ID: "a/b!",
170-
SourceRoots: []string{"src/a", "src/b"},
171-
APIs: []*API{
172-
{
173-
Path: "a/b/v1",
174-
},
175-
},
172+
ID: "a/b!",
176173
},
177-
wantErr: true,
174+
wantErr: true,
175+
wantErrMsg: "invalid id",
178176
},
179177
{
180178
name: "invalid last generated commit non-hex",
181179
library: &LibraryState{
182180
ID: "a/b",
183181
LastGeneratedCommit: "not-a-hex-string",
184-
SourceRoots: []string{"src/a", "src/b"},
185-
APIs: []*API{
186-
{
187-
Path: "a/b/v1",
188-
},
189-
},
190182
},
191-
wantErr: true,
183+
wantErr: true,
184+
wantErrMsg: "last_generated_commit must be a hex string",
192185
},
193186
{
194187
name: "invalid last generated commit wrong length",
195188
library: &LibraryState{
196189
ID: "a/b",
197190
LastGeneratedCommit: "deadbeef",
198-
SourceRoots: []string{"src/a", "src/b"},
199-
APIs: []*API{
200-
{
201-
Path: "a/b/v1",
202-
},
203-
},
204191
},
205-
wantErr: true,
192+
wantErr: true,
193+
wantErrMsg: "last_generated_commit must be 40 characters",
206194
},
207195
{
208196
name: "valid preserve_regex",
@@ -221,7 +209,8 @@ func TestLibrary_Validate(t *testing.T) {
221209
APIs: []*API{{Path: "a/b/v1"}},
222210
PreserveRegex: []string{"["},
223211
},
224-
wantErr: true,
212+
wantErr: true,
213+
wantErrMsg: "invalid preserve_regex at index",
225214
},
226215
{
227216
name: "valid remove_regex",
@@ -240,7 +229,8 @@ func TestLibrary_Validate(t *testing.T) {
240229
APIs: []*API{{Path: "a/b/v1"}},
241230
RemoveRegex: []string{"("},
242231
},
243-
wantErr: true,
232+
wantErr: true,
233+
wantErrMsg: "invalid remove_regex at index",
244234
},
245235
{
246236
name: "valid release_exclude_path",
@@ -259,7 +249,8 @@ func TestLibrary_Validate(t *testing.T) {
259249
APIs: []*API{{Path: "a/b/v1"}},
260250
ReleaseExcludePaths: []string{"/a/b"},
261251
},
262-
wantErr: true,
252+
wantErr: true,
253+
wantErrMsg: "invalid release_exclude_path at index",
263254
},
264255
{
265256
name: "valid tag_format",
@@ -278,11 +269,24 @@ func TestLibrary_Validate(t *testing.T) {
278269
APIs: []*API{{Path: "a/b/v1"}},
279270
TagFormat: "{id}-{foo}",
280271
},
281-
wantErr: true,
272+
wantErr: true,
273+
wantErrMsg: "invalid placeholder in tag_format",
282274
},
283275
} {
284276
t.Run(test.name, func(t *testing.T) {
285-
if err := test.library.Validate(); (err != nil) != test.wantErr {
277+
err := test.library.Validate()
278+
if test.wantErr {
279+
if err == nil {
280+
t.Error("Library.Validate() should fail")
281+
}
282+
if !strings.Contains(err.Error(), test.wantErrMsg) {
283+
t.Errorf("want error message %q, got %q", test.wantErrMsg, err.Error())
284+
}
285+
286+
return
287+
}
288+
289+
if err != nil {
286290
t.Errorf("Library.Validate() error = %v, wantErr %v", err, test.wantErr)
287291
}
288292
})
@@ -291,24 +295,44 @@ func TestLibrary_Validate(t *testing.T) {
291295

292296
func TestAPI_Validate(t *testing.T) {
293297
for _, test := range []struct {
294-
name string
295-
api *API
296-
wantErr bool
298+
name string
299+
api *API
300+
wantErr bool
301+
wantErrMsg string
297302
}{
298303
{
299-
name: "valid api",
304+
name: "new api",
305+
api: &API{
306+
Path: "a/b/v1",
307+
},
308+
},
309+
{
310+
name: "existing api",
300311
api: &API{
301312
Path: "a/b/v1",
302313
},
303314
},
304315
{
305-
name: "missing path",
306-
api: &API{},
307-
wantErr: true,
316+
name: "missing path",
317+
api: &API{},
318+
wantErr: true,
319+
wantErrMsg: "invalid path",
308320
},
309321
} {
310322
t.Run(test.name, func(t *testing.T) {
311-
if err := test.api.Validate(); (err != nil) != test.wantErr {
323+
err := test.api.Validate()
324+
if test.wantErr {
325+
if err == nil {
326+
t.Error("API.Validate() should fail")
327+
}
328+
if !strings.Contains(err.Error(), test.wantErrMsg) {
329+
t.Errorf("want error message %q, got %q", test.wantErrMsg, err.Error())
330+
}
331+
332+
return
333+
}
334+
335+
if err != nil {
312336
t.Errorf("API.Validate() error = %v, wantErr %v", err, test.wantErr)
313337
}
314338
})

‎internal/docker/docker.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ func (c *Docker) Build(ctx context.Context, request *BuildRequest) error {
222222
// Returns the configured library id if the command succeeds.
223223
func (c *Docker) Configure(ctx context.Context, request *ConfigureRequest) (string, error) {
224224
requestFilePath := filepath.Join(request.RepoDir, config.LibrarianDir, config.ConfigureRequest)
225-
if err := writeLibraryState(request.State, request.LibraryID, requestFilePath); err != nil {
225+
if err := writeLibrarianState(request.State, requestFilePath); err != nil {
226226
return "", err
227227
}
228228
defer func() {

‎internal/docker/docker_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,7 @@ func TestWriteLibraryState(t *testing.T) {
820820
{
821821
Path: "google/cloud/compute/v1",
822822
ServiceConfig: "example_service_config.yaml",
823+
Status: "new",
823824
},
824825
},
825826
SourceRoots: []string{
@@ -839,6 +840,7 @@ func TestWriteLibraryState(t *testing.T) {
839840
{
840841
Path: "google/storage/v1",
841842
ServiceConfig: "storage_service_config.yaml",
843+
Status: "existing",
842844
},
843845
},
844846
},
@@ -1048,6 +1050,7 @@ func TestWriteLibrarianState(t *testing.T) {
10481050
{
10491051
Path: "google/cloud/compute/v1",
10501052
ServiceConfig: "example_service_config.yaml",
1053+
Status: "existing",
10511054
},
10521055
},
10531056
SourceRoots: []string{
@@ -1067,6 +1070,7 @@ func TestWriteLibrarianState(t *testing.T) {
10671070
{
10681071
Path: "google/storage/v1",
10691072
ServiceConfig: "storage_service_config.yaml",
1073+
Status: "existing",
10701074
},
10711075
},
10721076
},

‎internal/librarian/generate.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -515,10 +515,11 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context) (string, error
515515
return "", err
516516
}
517517

518-
// record to state, not write to state.yaml
518+
setAllAPIStatus(r.state, config.StatusExisting)
519+
// Record to state, not write to state.yaml
519520
r.state.Libraries = append(r.state.Libraries, &config.LibraryState{
520521
ID: r.cfg.Library,
521-
APIs: []*config.API{{Path: r.cfg.API}},
522+
APIs: []*config.API{{Path: r.cfg.API, Status: config.StatusNew}},
522523
})
523524

524525
if err := populateServiceConfigIfEmpty(
@@ -562,3 +563,11 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context) (string, error
562563

563564
return libraryState.ID, nil
564565
}
566+
567+
func setAllAPIStatus(state *config.LibrarianState, status string) {
568+
for _, library := range state.Libraries {
569+
for _, api := range library.APIs {
570+
api.Status = status
571+
}
572+
}
573+
}

0 commit comments

Comments
 (0)