diff --git a/internal/config/state.go b/internal/config/state.go index e608f813d6..a5e4d7ab51 100644 --- a/internal/config/state.go +++ b/internal/config/state.go @@ -21,6 +21,11 @@ import ( "strings" ) +const ( + StatusNew = "new" + StatusExisting = "existing" +) + // LibrarianState defines the contract for the state.yaml file. type LibrarianState struct { // The name and tag of the generator image to use. tag is required. @@ -189,6 +194,9 @@ type API struct { Path string `yaml:"path" json:"path"` // The name of the service config file, relative to the API `path`. ServiceConfig string `yaml:"service_config" json:"service_config"` + // The status of the API, one of "new" or "existing". + // This field is ignored when writing to state.yaml. + Status string `yaml:"-" json:"status"` } // Validate checks that the API is valid. diff --git a/internal/config/state_test.go b/internal/config/state_test.go index e6fb06f84c..2510feec56 100644 --- a/internal/config/state_test.go +++ b/internal/config/state_test.go @@ -15,14 +15,16 @@ package config import ( + "strings" "testing" ) func TestLibrarianState_Validate(t *testing.T) { for _, test := range []struct { - name string - state *LibrarianState - wantErr bool + name string + state *LibrarianState + wantErr bool + wantErrMsg string }{ { name: "valid state", @@ -56,19 +58,33 @@ func TestLibrarianState_Validate(t *testing.T) { }, }, }, - wantErr: true, + wantErr: true, + wantErrMsg: "image is required", }, { name: "missing libraries", state: &LibrarianState{ Image: "gcr.io/test/image:v1.2.3", }, - wantErr: true, + wantErr: true, + wantErrMsg: "libraries cannot be empty", }, } { t.Run(test.name, func(t *testing.T) { - if err := test.state.Validate(); (err != nil) != test.wantErr { - t.Errorf("LibrarianState.Validate() error = %v, wantErr %v", err, test.wantErr) + err := test.state.Validate() + if test.wantErr { + if err == nil { + t.Error("Librarian.Validate() should fail") + } + if !strings.Contains(err.Error(), test.wantErrMsg) { + t.Errorf("want error message %q, got %q", test.wantErrMsg, err.Error()) + } + + return + } + + if err != nil { + t.Errorf("Librarian.Validate() error = %v, wantErr %v", err, test.wantErr) } }) } @@ -76,9 +92,10 @@ func TestLibrarianState_Validate(t *testing.T) { func TestLibrary_Validate(t *testing.T) { for _, test := range []struct { - name string - library *LibraryState - wantErr bool + name string + library *LibraryState + wantErr bool + wantErrMsg string }{ { name: "valid library", @@ -93,42 +110,26 @@ func TestLibrary_Validate(t *testing.T) { }, }, { - name: "missing id", - library: &LibraryState{ - SourceRoots: []string{"src/a", "src/b"}, - APIs: []*API{ - { - Path: "a/b/v1", - }, - }, - }, - wantErr: true, + name: "missing id", + library: &LibraryState{}, + wantErr: true, + wantErrMsg: "id is required", }, { name: "id is dot", library: &LibraryState{ - ID: ".", - SourceRoots: []string{"src/a", "src/b"}, - APIs: []*API{ - { - Path: "a/b/v1", - }, - }, + ID: ".", }, - wantErr: true, + wantErr: true, + wantErrMsg: "id cannot be", }, { name: "id is double dot", library: &LibraryState{ - ID: "..", - SourceRoots: []string{"src/a", "src/b"}, - APIs: []*API{ - { - Path: "a/b/v1", - }, - }, + ID: "..", }, - wantErr: true, + wantErr: true, + wantErrMsg: "id cannot be", }, { name: "missing source paths", @@ -140,7 +141,8 @@ func TestLibrary_Validate(t *testing.T) { }, }, }, - wantErr: true, + wantErr: true, + wantErrMsg: "source_roots cannot be empty", }, { name: "missing apis", @@ -148,7 +150,8 @@ func TestLibrary_Validate(t *testing.T) { ID: "a/b", SourceRoots: []string{"src/a", "src/b"}, }, - wantErr: true, + wantErr: true, + wantErrMsg: "apis cannot be empty", }, { name: "valid version without v prefix", @@ -166,43 +169,28 @@ func TestLibrary_Validate(t *testing.T) { { name: "invalid id characters", library: &LibraryState{ - ID: "a/b!", - SourceRoots: []string{"src/a", "src/b"}, - APIs: []*API{ - { - Path: "a/b/v1", - }, - }, + ID: "a/b!", }, - wantErr: true, + wantErr: true, + wantErrMsg: "invalid id", }, { name: "invalid last generated commit non-hex", library: &LibraryState{ ID: "a/b", LastGeneratedCommit: "not-a-hex-string", - SourceRoots: []string{"src/a", "src/b"}, - APIs: []*API{ - { - Path: "a/b/v1", - }, - }, }, - wantErr: true, + wantErr: true, + wantErrMsg: "last_generated_commit must be a hex string", }, { name: "invalid last generated commit wrong length", library: &LibraryState{ ID: "a/b", LastGeneratedCommit: "deadbeef", - SourceRoots: []string{"src/a", "src/b"}, - APIs: []*API{ - { - Path: "a/b/v1", - }, - }, }, - wantErr: true, + wantErr: true, + wantErrMsg: "last_generated_commit must be 40 characters", }, { name: "valid preserve_regex", @@ -221,7 +209,8 @@ func TestLibrary_Validate(t *testing.T) { APIs: []*API{{Path: "a/b/v1"}}, PreserveRegex: []string{"["}, }, - wantErr: true, + wantErr: true, + wantErrMsg: "invalid preserve_regex at index", }, { name: "valid remove_regex", @@ -240,7 +229,8 @@ func TestLibrary_Validate(t *testing.T) { APIs: []*API{{Path: "a/b/v1"}}, RemoveRegex: []string{"("}, }, - wantErr: true, + wantErr: true, + wantErrMsg: "invalid remove_regex at index", }, { name: "valid release_exclude_path", @@ -259,7 +249,8 @@ func TestLibrary_Validate(t *testing.T) { APIs: []*API{{Path: "a/b/v1"}}, ReleaseExcludePaths: []string{"/a/b"}, }, - wantErr: true, + wantErr: true, + wantErrMsg: "invalid release_exclude_path at index", }, { name: "valid tag_format", @@ -278,11 +269,24 @@ func TestLibrary_Validate(t *testing.T) { APIs: []*API{{Path: "a/b/v1"}}, TagFormat: "{id}-{foo}", }, - wantErr: true, + wantErr: true, + wantErrMsg: "invalid placeholder in tag_format", }, } { t.Run(test.name, func(t *testing.T) { - if err := test.library.Validate(); (err != nil) != test.wantErr { + err := test.library.Validate() + if test.wantErr { + if err == nil { + t.Error("Library.Validate() should fail") + } + if !strings.Contains(err.Error(), test.wantErrMsg) { + t.Errorf("want error message %q, got %q", test.wantErrMsg, err.Error()) + } + + return + } + + if err != nil { t.Errorf("Library.Validate() error = %v, wantErr %v", err, test.wantErr) } }) @@ -291,24 +295,44 @@ func TestLibrary_Validate(t *testing.T) { func TestAPI_Validate(t *testing.T) { for _, test := range []struct { - name string - api *API - wantErr bool + name string + api *API + wantErr bool + wantErrMsg string }{ { - name: "valid api", + name: "new api", + api: &API{ + Path: "a/b/v1", + }, + }, + { + name: "existing api", api: &API{ Path: "a/b/v1", }, }, { - name: "missing path", - api: &API{}, - wantErr: true, + name: "missing path", + api: &API{}, + wantErr: true, + wantErrMsg: "invalid path", }, } { t.Run(test.name, func(t *testing.T) { - if err := test.api.Validate(); (err != nil) != test.wantErr { + err := test.api.Validate() + if test.wantErr { + if err == nil { + t.Error("API.Validate() should fail") + } + if !strings.Contains(err.Error(), test.wantErrMsg) { + t.Errorf("want error message %q, got %q", test.wantErrMsg, err.Error()) + } + + return + } + + if err != nil { t.Errorf("API.Validate() error = %v, wantErr %v", err, test.wantErr) } }) diff --git a/internal/docker/docker.go b/internal/docker/docker.go index 18b14fb570..39ad9a309b 100644 --- a/internal/docker/docker.go +++ b/internal/docker/docker.go @@ -222,7 +222,7 @@ func (c *Docker) Build(ctx context.Context, request *BuildRequest) error { // Returns the configured library id if the command succeeds. func (c *Docker) Configure(ctx context.Context, request *ConfigureRequest) (string, error) { requestFilePath := filepath.Join(request.RepoDir, config.LibrarianDir, config.ConfigureRequest) - if err := writeLibraryState(request.State, request.LibraryID, requestFilePath); err != nil { + if err := writeLibrarianState(request.State, requestFilePath); err != nil { return "", err } defer func() { diff --git a/internal/docker/docker_test.go b/internal/docker/docker_test.go index 6b2b017976..8ae65a3679 100644 --- a/internal/docker/docker_test.go +++ b/internal/docker/docker_test.go @@ -820,6 +820,7 @@ func TestWriteLibraryState(t *testing.T) { { Path: "google/cloud/compute/v1", ServiceConfig: "example_service_config.yaml", + Status: "new", }, }, SourceRoots: []string{ @@ -839,6 +840,7 @@ func TestWriteLibraryState(t *testing.T) { { Path: "google/storage/v1", ServiceConfig: "storage_service_config.yaml", + Status: "existing", }, }, }, @@ -1048,6 +1050,7 @@ func TestWriteLibrarianState(t *testing.T) { { Path: "google/cloud/compute/v1", ServiceConfig: "example_service_config.yaml", + Status: "existing", }, }, SourceRoots: []string{ @@ -1067,6 +1070,7 @@ func TestWriteLibrarianState(t *testing.T) { { Path: "google/storage/v1", ServiceConfig: "storage_service_config.yaml", + Status: "existing", }, }, }, diff --git a/internal/librarian/generate.go b/internal/librarian/generate.go index d0b35bac3a..25c96d1bf1 100644 --- a/internal/librarian/generate.go +++ b/internal/librarian/generate.go @@ -515,10 +515,11 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context) (string, error return "", err } - // record to state, not write to state.yaml + setAllAPIStatus(r.state, config.StatusExisting) + // Record to state, not write to state.yaml r.state.Libraries = append(r.state.Libraries, &config.LibraryState{ ID: r.cfg.Library, - APIs: []*config.API{{Path: r.cfg.API}}, + APIs: []*config.API{{Path: r.cfg.API, Status: config.StatusNew}}, }) if err := populateServiceConfigIfEmpty( @@ -562,3 +563,11 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context) (string, error return libraryState.ID, nil } + +func setAllAPIStatus(state *config.LibrarianState, status string) { + for _, library := range state.Libraries { + for _, api := range library.APIs { + api.Status = status + } + } +} diff --git a/internal/librarian/generate_test.go b/internal/librarian/generate_test.go index 577793a74d..5602c04cd4 100644 --- a/internal/librarian/generate_test.go +++ b/internal/librarian/generate_test.go @@ -460,8 +460,14 @@ func TestNewGenerateRunner(t *testing.T) { Image: "some/image:v1.2.3", Libraries: []*config.LibraryState{ { - ID: "some-library", - APIs: []*config.API{{Path: "some/api", ServiceConfig: "api_config.yaml"}}, + ID: "some-library", + APIs: []*config.API{ + { + Path: "some/api", + ServiceConfig: "api_config.yaml", + Status: config.StatusExisting, + }, + }, SourceRoots: []string{"src/a"}, }, }, diff --git a/internal/librarian/state_test.go b/internal/librarian/state_test.go index da301bcea8..124134ab9c 100644 --- a/internal/librarian/state_test.go +++ b/internal/librarian/state_test.go @@ -263,6 +263,7 @@ func TestSaveLibrarianState(t *testing.T) { { Path: "a/b/v1", ServiceConfig: "a/b/v1/service.yaml", + Status: "existing", }, }, PreserveRegex: []string{}, @@ -283,7 +284,8 @@ func TestSaveLibrarianState(t *testing.T) { if err := yaml.Unmarshal(gotBytes, gotState); err != nil { t.Fatalf("yaml.Unmarshal() failed: %v", err) } - + // API status should be ignored when writing to yaml. + state.Libraries[0].APIs[0].Status = "" if diff := cmp.Diff(state, gotState); diff != "" { t.Errorf("saveLibrarianState() mismatch (-want +got): %s", diff) } diff --git a/internal/librarian/tag_and_release_test.go b/internal/librarian/tag_and_release_test.go index 4b1d741386..326730e8b6 100644 --- a/internal/librarian/tag_and_release_test.go +++ b/internal/librarian/tag_and_release_test.go @@ -51,4 +51,4 @@ func TestNewTagAndReleaseRunner(t *testing.T) { } }) } -} \ No newline at end of file +} diff --git a/testdata/e2e_func.go b/testdata/e2e_func.go index cb0deee193..e72ac9abb7 100644 --- a/testdata/e2e_func.go +++ b/testdata/e2e_func.go @@ -51,12 +51,12 @@ func doConfigure(args []string) error { return err } - library, err := readCommandRequest(filepath.Join(request.librarianDir, configureRequest)) + state, err := readConfigureRequest(filepath.Join(request.librarianDir, configureRequest)) if err != nil { return err } - return writeConfigureResponse(request, library) + return writeConfigureResponse(request, state) } func doGenerate(args []string) error { @@ -68,7 +68,7 @@ func doGenerate(args []string) error { return err } - if _, err := readCommandRequest(filepath.Join(request.librarianDir, generateRequest)); err != nil { + if _, err := readGenerateRequest(filepath.Join(request.librarianDir, generateRequest)); err != nil { return err } @@ -125,45 +125,79 @@ func validateLibrarianDir(dir, requestFile string) error { return nil } -// readCommandRequest reads the command request file, e.g., configure-request.json -// or generate-request.json. -func readCommandRequest(path string) (*libraryState, error) { - library := &libraryState{} +// readConfigureRequest reads the configure request file and creates a librarianState +// object. +func readConfigureRequest(path string) (*librarianState, error) { + state := &librarianState{} data, err := os.ReadFile(path) if err != nil { return nil, err } - if err := json.Unmarshal(data, library); err != nil { + if err := json.Unmarshal(data, state); err != nil { return nil, err } - if library.ID == simulateCommandErrorID { - // Simulate a command error - return nil, errors.New("simulate command error") + for _, library := range state.Libraries { + if library.ID == simulateCommandErrorID { + return nil, errors.New("simulate command error") + } } - return library, nil + return state, nil } -func writeConfigureResponse(option *configureOption, library *libraryState) error { - library = populateAdditionalFields(library) - data, err := json.MarshalIndent(library, "", " ") - if err != nil { - return err +func writeConfigureResponse(option *configureOption, state *librarianState) error { + for _, library := range state.Libraries { + needConfigure := false + for _, oneAPI := range library.APIs { + if oneAPI.Status == "new" { + needConfigure = true + } + } + + if !needConfigure { + continue + } + + populateAdditionalFields(library) + data, err := json.MarshalIndent(library, "", " ") + if err != nil { + return err + } + + jsonFilePath := filepath.Join(option.librarianDir, configureResponse) + jsonFile, err := os.Create(jsonFilePath) + if err != nil { + return err + } + + if _, err := jsonFile.Write(data); err != nil { + return err + } + log.Print("write configure response to " + jsonFilePath) } - jsonFilePath := filepath.Join(option.librarianDir, configureResponse) - jsonFile, err := os.Create(jsonFilePath) + return nil +} + +// readGenerateRequest reads the generate request file and creates a libraryState +// object. +func readGenerateRequest(path string) (*libraryState, error) { + library := &libraryState{} + data, err := os.ReadFile(path) if err != nil { - return err + return nil, err + } + if err := json.Unmarshal(data, library); err != nil { + return nil, err } - if _, err := jsonFile.Write(data); err != nil { - return err + if library.ID == simulateCommandErrorID { + // Simulate a command error + return nil, errors.New("simulate command error") } - log.Print("write configure response to " + jsonFilePath) - return nil + return library, nil } func writeGenerateResponse(option *generateOption) (err error) { @@ -187,19 +221,19 @@ func writeGenerateResponse(option *generateOption) (err error) { return err } -func populateAdditionalFields(library *libraryState) *libraryState { +func populateAdditionalFields(library *libraryState) { library.Version = "1.0.0" library.SourceRoots = []string{"example-source-path", "example-source-path-2"} library.PreserveRegex = []string{"example-preserve-regex", "example-preserve-regex-2"} library.RemoveRegex = []string{"example-remove-regex", "example-remove-regex-2"} - - return library + for _, oneAPI := range library.APIs { + oneAPI.Status = "existing" + } } type configureOption struct { intputDir string librarianDir string - libraryID string sourceDir string } @@ -210,6 +244,11 @@ type generateOption struct { sourceDir string } +type librarianState struct { + Image string `json:"image"` + Libraries []*libraryState `json:"libraries"` +} + type libraryState struct { ID string `json:"id"` Version string `json:"version"` @@ -222,4 +261,5 @@ type libraryState struct { type api struct { Path string `json:"path"` ServiceConfig string `json:"service_config"` + Status string `json:"status"` } diff --git a/testdata/test-write-librarian-state/write-librarian-state-example.json b/testdata/test-write-librarian-state/write-librarian-state-example.json index c2bf1805c5..d1ac7131b4 100644 --- a/testdata/test-write-librarian-state/write-librarian-state-example.json +++ b/testdata/test-write-librarian-state/write-librarian-state-example.json @@ -8,7 +8,8 @@ "apis": [ { "path": "google/cloud/compute/v1", - "service_config": "example_service_config.yaml" + "service_config": "example_service_config.yaml", + "status": "existing" } ], "source_roots": [ @@ -28,7 +29,8 @@ "apis": [ { "path": "google/storage/v1", - "service_config": "storage_service_config.yaml" + "service_config": "storage_service_config.yaml", + "status": "existing" } ], "source_roots": null, diff --git a/testdata/test-write-library-state/successful-marshaling-and-writing.json b/testdata/test-write-library-state/successful-marshaling-and-writing.json index 0ff6dc7299..7bd7517d89 100644 --- a/testdata/test-write-library-state/successful-marshaling-and-writing.json +++ b/testdata/test-write-library-state/successful-marshaling-and-writing.json @@ -5,7 +5,8 @@ "apis": [ { "path": "google/cloud/compute/v1", - "service_config": "example_service_config.yaml" + "service_config": "example_service_config.yaml", + "status": "new" } ], "source_roots": [