Skip to content

Commit 60554f6

Browse files
JoeWang1127JoeWang1127
authored
fix(internal/librarian): read configure response (#989)
Fixes #985
1 parent 6d33faf commit 60554f6

14 files changed

+687
-94
lines changed

‎internal/config/config.go‎

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

2626
const (
2727
// BuildRequest is a JSON file that describes which library to build/test.
28-
BuildRequest string = "build-request.json"
28+
BuildRequest = "build-request.json"
2929
// ConfigureRequest is a JSON file that describes which library to configure.
30-
ConfigureRequest string = "configure-request.json"
30+
ConfigureRequest = "configure-request.json"
31+
// ConfigureResponse is a JSON file that describes which library to change
32+
// after initial configuration.
33+
ConfigureResponse = "configure-response.json"
3134
// GeneratorInputDir is the default directory to store files that generator
3235
// needs to regenerate libraries from an empty directory.
33-
GeneratorInputDir string = ".librarian/generator-input"
36+
GeneratorInputDir = ".librarian/generator-input"
3437
// GenerateRequest is a JSON file that describes which library to generate.
35-
GenerateRequest string = "generate-request.json"
38+
GenerateRequest = "generate-request.json"
3639
// LibrarianDir is the default directory to store librarian state/config files,
3740
// along with any additional configuration.
38-
LibrarianDir string = ".librarian"
41+
LibrarianDir = ".librarian"
3942
)
4043

4144
// Config holds all configuration values parsed from flags or environment

‎internal/docker/docker.go‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -190,17 +190,19 @@ func (c *Docker) Build(ctx context.Context, request *BuildRequest) error {
190190

191191
// Configure configures an API within a repository, either adding it to an
192192
// existing library or creating a new library.
193-
func (c *Docker) Configure(ctx context.Context, request *ConfigureRequest) error {
194-
jsonFilePath := filepath.Join(request.RepoDir, config.LibrarianDir, config.ConfigureRequest)
195-
if err := writeRequest(request.State, request.LibraryID, jsonFilePath); err != nil {
196-
return err
193+
//
194+
// Returns the configured library id if the command succeeds.
195+
func (c *Docker) Configure(ctx context.Context, request *ConfigureRequest) (string, error) {
196+
requestFilePath := filepath.Join(request.RepoDir, config.LibrarianDir, config.ConfigureRequest)
197+
if err := writeRequest(request.State, request.LibraryID, requestFilePath); err != nil {
198+
return "", err
197199
}
198-
defer func(name string) {
199-
err := os.Remove(name)
200+
defer func() {
201+
err := os.Remove(requestFilePath)
200202
if err != nil {
201-
slog.Warn("fail to remove file", slog.String("name", name), slog.Any("err", err))
203+
slog.Warn("fail to remove file", slog.String("name", requestFilePath), slog.Any("err", err))
202204
}
203-
}(jsonFilePath)
205+
}()
204206
commandArgs := []string{
205207
"--librarian=/librarian",
206208
"--input=/input",
@@ -214,7 +216,11 @@ func (c *Docker) Configure(ctx context.Context, request *ConfigureRequest) error
214216
fmt.Sprintf("%s:/source:ro", request.ApiRoot), // readonly volume
215217
}
216218

217-
return c.runDocker(ctx, request.Cfg, CommandConfigure, mounts, commandArgs)
219+
if err := c.runDocker(ctx, request.Cfg, CommandConfigure, mounts, commandArgs); err != nil {
220+
return "", err
221+
}
222+
223+
return request.LibraryID, nil
218224
}
219225

220226
func (c *Docker) runDocker(_ context.Context, cfg *config.Config, command Command, mounts []string, commandArgs []string) (err error) {

‎internal/docker/docker_test.go‎

Lines changed: 122 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package docker
1616

1717
import (
1818
"context"
19+
"encoding/json"
1920
"errors"
2021
"fmt"
2122
"os"
@@ -55,20 +56,19 @@ func TestNew(t *testing.T) {
5556

5657
func TestDockerRun(t *testing.T) {
5758
const (
58-
mockImage = "mockImage"
59-
testAPIPath = "testAPIPath"
60-
testAPIRoot = "testAPIRoot"
61-
testGeneratorInput = "testGeneratorInput"
62-
testImage = "testImage"
63-
testLibraryID = "testLibraryID"
64-
testOutput = "testOutput"
59+
mockImage = "mockImage"
60+
testAPIRoot = "testAPIRoot"
61+
testImage = "testImage"
62+
testLibraryID = "testLibraryID"
63+
testOutput = "testOutput"
6564
)
6665

6766
state := &config.LibrarianState{}
6867
cfg := &config.Config{}
6968
cfgInDocker := &config.Config{
7069
HostMount: "hostDir:localDir",
7170
}
71+
repoDir := filepath.Join(os.TempDir())
7272
for _, test := range []struct {
7373
name string
7474
docker *Docker
@@ -85,17 +85,18 @@ func TestDockerRun(t *testing.T) {
8585
generateRequest := &GenerateRequest{
8686
Cfg: cfg,
8787
State: state,
88-
RepoDir: "absolute/path/to/repo",
88+
RepoDir: repoDir,
8989
ApiRoot: testAPIRoot,
9090
Output: testOutput,
9191
LibraryID: testLibraryID,
9292
}
93+
9394
return d.Generate(ctx, generateRequest)
9495
},
9596
want: []string{
9697
"run", "--rm",
97-
"-v", "absolute/path/to/repo/.librarian:/librarian:ro",
98-
"-v", "absolute/path/to/repo/.librarian/generator-input:/input",
98+
"-v", fmt.Sprintf("%s/.librarian:/librarian:ro", repoDir),
99+
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
99100
"-v", fmt.Sprintf("%s:/output", testOutput),
100101
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
101102
testImage,
@@ -135,11 +136,12 @@ func TestDockerRun(t *testing.T) {
135136
generateRequest := &GenerateRequest{
136137
Cfg: cfg,
137138
State: state,
138-
RepoDir: ".",
139+
RepoDir: repoDir,
139140
ApiRoot: testAPIRoot,
140141
Output: testOutput,
141142
LibraryID: testLibraryID,
142143
}
144+
143145
return d.Generate(ctx, generateRequest)
144146
},
145147
want: []string{},
@@ -154,17 +156,18 @@ func TestDockerRun(t *testing.T) {
154156
generateRequest := &GenerateRequest{
155157
Cfg: cfgInDocker,
156158
State: state,
157-
RepoDir: "absolute/path/to/repo",
159+
RepoDir: repoDir,
158160
ApiRoot: testAPIRoot,
159161
Output: "hostDir",
160162
LibraryID: testLibraryID,
161163
}
164+
162165
return d.Generate(ctx, generateRequest)
163166
},
164167
want: []string{
165168
"run", "--rm",
166-
"-v", "absolute/path/to/repo/.librarian:/librarian:ro",
167-
"-v", "absolute/path/to/repo/.librarian/generator-input:/input",
169+
"-v", fmt.Sprintf("%s/.librarian:/librarian:ro", repoDir),
170+
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
168171
"-v", "localDir:/output",
169172
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
170173
testImage,
@@ -186,14 +189,15 @@ func TestDockerRun(t *testing.T) {
186189
Cfg: cfg,
187190
State: state,
188191
LibraryID: testLibraryID,
189-
RepoDir: "absolute/path/to/repo",
192+
RepoDir: repoDir,
190193
}
194+
191195
return d.Build(ctx, buildRequest)
192196
},
193197
want: []string{
194198
"run", "--rm",
195-
"-v", "absolute/path/to/repo/.librarian:/librarian:ro",
196-
"-v", "absolute/path/to/repo:/repo",
199+
"-v", fmt.Sprintf("%s/.librarian:/librarian:ro", repoDir),
200+
"-v", fmt.Sprintf("%s:/repo", repoDir),
197201
testImage,
198202
string(CommandBuild),
199203
"--librarian=/librarian",
@@ -228,8 +232,9 @@ func TestDockerRun(t *testing.T) {
228232
Cfg: cfg,
229233
State: state,
230234
LibraryID: testLibraryID,
231-
RepoDir: "absolute/path/to/repo",
235+
RepoDir: repoDir,
232236
}
237+
233238
return d.Build(ctx, buildRequest)
234239
},
235240
want: []string{},
@@ -245,15 +250,103 @@ func TestDockerRun(t *testing.T) {
245250
Cfg: cfg,
246251
State: state,
247252
LibraryID: testLibraryID,
248-
RepoDir: "absolute/path/to/repo",
253+
RepoDir: repoDir,
254+
ApiRoot: testAPIRoot,
255+
}
256+
jsonData, _ := json.MarshalIndent(&config.LibraryState{}, "", " ")
257+
if err := os.MkdirAll(filepath.Join(configureRequest.RepoDir, config.LibrarianDir), 0755); err != nil {
258+
return err
259+
}
260+
jsonFilePath := filepath.Join(configureRequest.RepoDir, config.LibrarianDir, config.ConfigureResponse)
261+
if err := os.WriteFile(jsonFilePath, jsonData, 0644); err != nil {
262+
return err
263+
}
264+
265+
_, err := d.Configure(ctx, configureRequest)
266+
267+
return err
268+
},
269+
want: []string{
270+
"run", "--rm",
271+
"-v", fmt.Sprintf("%s/.librarian:/librarian", repoDir),
272+
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
273+
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
274+
testImage,
275+
string(CommandConfigure),
276+
"--librarian=/librarian",
277+
"--input=/input",
278+
"--source=/source",
279+
},
280+
wantErr: false,
281+
},
282+
{
283+
name: "Configure with multiple libraries in librarian state",
284+
docker: &Docker{
285+
Image: testImage,
286+
},
287+
runCommand: func(ctx context.Context, d *Docker) error {
288+
curState := &config.LibrarianState{
289+
Image: testImage,
290+
Libraries: []*config.LibraryState{
291+
{
292+
ID: testLibraryID,
293+
APIs: []*config.API{
294+
{
295+
Path: "example/path/v1",
296+
},
297+
},
298+
},
299+
{
300+
ID: "another-example-library",
301+
APIs: []*config.API{
302+
{
303+
Path: "another/example/path/v1",
304+
ServiceConfig: "another_v1.yaml",
305+
},
306+
},
307+
SourcePaths: []string{
308+
"another-example-source-path",
309+
},
310+
},
311+
},
312+
}
313+
configureRequest := &ConfigureRequest{
314+
Cfg: cfg,
315+
State: curState,
316+
LibraryID: testLibraryID,
317+
RepoDir: repoDir,
249318
ApiRoot: testAPIRoot,
250319
}
251-
return d.Configure(ctx, configureRequest)
320+
jsonData, _ := json.MarshalIndent(&config.LibraryState{
321+
ID: testLibraryID,
322+
APIs: []*config.API{
323+
{
324+
Path: "example/path/v1",
325+
ServiceConfig: "generated_example_v1.yaml",
326+
},
327+
},
328+
}, "", " ")
329+
330+
if err := os.MkdirAll(filepath.Join(configureRequest.RepoDir, config.LibrarianDir), 0755); err != nil {
331+
return err
332+
}
333+
334+
jsonFilePath := filepath.Join(configureRequest.RepoDir, config.LibrarianDir, config.ConfigureResponse)
335+
if err := os.WriteFile(jsonFilePath, jsonData, 0644); err != nil {
336+
return err
337+
}
338+
339+
configuredLibrary, err := d.Configure(ctx, configureRequest)
340+
if configuredLibrary != testLibraryID {
341+
return errors.New("configured library, " + configuredLibrary + " is wrong")
342+
}
343+
344+
return err
252345
},
253346
want: []string{
254347
"run", "--rm",
255-
"-v", "absolute/path/to/repo/.librarian:/librarian",
256-
"-v", "absolute/path/to/repo/.librarian/generator-input:/input",
348+
"-v", fmt.Sprintf("%s/.librarian:/librarian", repoDir),
349+
"-v", fmt.Sprintf("%s/.librarian/generator-input:/input", repoDir),
257350
"-v", fmt.Sprintf("%s:/source:ro", testAPIRoot),
258351
testImage,
259352
string(CommandConfigure),
@@ -276,7 +369,8 @@ func TestDockerRun(t *testing.T) {
276369
RepoDir: "/non-exist-dir",
277370
ApiRoot: testAPIRoot,
278371
}
279-
return d.Configure(ctx, configureRequest)
372+
_, err := d.Configure(ctx, configureRequest)
373+
return err
280374
},
281375
want: []string{},
282376
wantErr: true,
@@ -291,10 +385,13 @@ func TestDockerRun(t *testing.T) {
291385
Cfg: cfg,
292386
State: state,
293387
LibraryID: testLibraryID,
294-
RepoDir: ".",
388+
RepoDir: repoDir,
295389
ApiRoot: testAPIRoot,
296390
}
297-
return d.Configure(ctx, configureRequest)
391+
392+
_, err := d.Configure(ctx, configureRequest)
393+
394+
return err
298395
},
299396
want: []string{},
300397
wantErr: true,
@@ -323,8 +420,6 @@ func TestDockerRun(t *testing.T) {
323420
if err != nil {
324421
t.Fatal(err)
325422
}
326-
327-
os.RemoveAll(".librarian")
328423
})
329424
}
330425
}

0 commit comments

Comments
 (0)