From 3db2b7544d7d03434f30161d917560d117f278cc Mon Sep 17 00:00:00 2001 From: whywaita Date: Thu, 14 Oct 2021 23:51:28 +0900 Subject: [PATCH] Implement UpdateParam in web --- pkg/datastore/interface.go | 2 +- pkg/datastore/memory/memory.go | 17 ++++++- pkg/datastore/mysql/target.go | 27 ++++++++-- pkg/datastore/mysql/target_test.go | 79 +++++++++++++++++++++++++----- pkg/web/target.go | 59 +++++++++++++++++++--- pkg/web/target_create.go | 13 ++++- pkg/web/target_test.go | 64 +++++++++++++++++------- 7 files changed, 219 insertions(+), 42 deletions(-) diff --git a/pkg/datastore/interface.go b/pkg/datastore/interface.go index 82397f1..3ef27d5 100644 --- a/pkg/datastore/interface.go +++ b/pkg/datastore/interface.go @@ -40,7 +40,7 @@ type Datastore interface { UpdateTargetStatus(ctx context.Context, targetID uuid.UUID, newStatus TargetStatus, description string) error UpdateToken(ctx context.Context, targetID uuid.UUID, newToken string, newExpiredAt time.Time) error - UpdateResourceType(ctx context.Context, targetID uuid.UUID, newResourceType ResourceType) error + UpdateTargetParam(ctx context.Context, targetID uuid.UUID, newResourceType ResourceType, newRunnerVersion, newRunnerUser, newProviderURL string) error EnqueueJob(ctx context.Context, job Job) error ListJobs(ctx context.Context) ([]Job, error) diff --git a/pkg/datastore/memory/memory.go b/pkg/datastore/memory/memory.go index 5306c40..87f5293 100644 --- a/pkg/datastore/memory/memory.go +++ b/pkg/datastore/memory/memory.go @@ -2,6 +2,7 @@ package memory import ( "context" + "database/sql" "fmt" "sync" "time" @@ -141,8 +142,8 @@ func (m *Memory) UpdateToken(ctx context.Context, targetID uuid.UUID, newToken s return nil } -// UpdateResourceType update resource_type in target -func (m *Memory) UpdateResourceType(ctx context.Context, targetID uuid.UUID, newResourceType datastore.ResourceType) error { +// UpdateTargetParam update parameter of target +func (m *Memory) UpdateTargetParam(ctx context.Context, targetID uuid.UUID, newResourceType datastore.ResourceType, newRunnerVersion, newRunnerUser, newProviderURL string) error { m.mu.Lock() defer m.mu.Unlock() @@ -151,6 +152,18 @@ func (m *Memory) UpdateResourceType(ctx context.Context, targetID uuid.UUID, new return fmt.Errorf("not found") } t.ResourceType = newResourceType + t.RunnerVersion = sql.NullString{ + String: newRunnerVersion, + Valid: true, + } + t.RunnerUser = sql.NullString{ + String: newRunnerUser, + Valid: true, + } + t.ProviderURL = sql.NullString{ + String: newProviderURL, + Valid: true, + } m.targets[targetID] = t return nil diff --git a/pkg/datastore/mysql/target.go b/pkg/datastore/mysql/target.go index 9874d5a..61d5152 100644 --- a/pkg/datastore/mysql/target.go +++ b/pkg/datastore/mysql/target.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "strings" "time" uuid "github.com/satori/go.uuid" @@ -109,12 +110,30 @@ func (m *MySQL) UpdateToken(ctx context.Context, targetID uuid.UUID, newToken st return nil } -// UpdateResourceType update resource type in target -func (m *MySQL) UpdateResourceType(ctx context.Context, targetID uuid.UUID, newResourceType datastore.ResourceType) error { - query := `UPDATE targets SET resource_type = ? WHERE uuid = ?` - if _, err := m.Conn.ExecContext(ctx, query, newResourceType, targetID.String()); err != nil { +// UpdateTargetParam update parameter of target +func (m *MySQL) UpdateTargetParam(ctx context.Context, targetID uuid.UUID, newResourceType datastore.ResourceType, newRunnerVersion, newRunnerUser, newProviderURL string) error { + newRV := toSQLNullString(newRunnerVersion) + newRU := toSQLNullString(newRunnerUser) + newPU := toSQLNullString(newProviderURL) + + query := `UPDATE targets SET resource_type = ?, runner_version = ?, runner_user = ?, provider_url = ? WHERE uuid = ?` + if _, err := m.Conn.ExecContext(ctx, query, newResourceType, newRV, newRU, newPU, targetID.String()); err != nil { return fmt.Errorf("failed to execute UPDATE query: %w", err) } return nil } + +func toSQLNullString(input string) sql.NullString { + if strings.EqualFold(input, "") { + return sql.NullString{ + Valid: false, + String: "", + } + } + + return sql.NullString{ + Valid: true, + String: input, + } +} diff --git a/pkg/datastore/mysql/target_test.go b/pkg/datastore/mysql/target_test.go index cdbf13d..a861893 100644 --- a/pkg/datastore/mysql/target_test.go +++ b/pkg/datastore/mysql/target_test.go @@ -22,6 +22,7 @@ var testScopeOrg = "octocat" var testScopeRepo = "octocat/hello-world" var testGitHubToken = "this-code-is-github-token" var testRunnerVersion = "v999.99.9" +var testRunnerUser = "testing-super-user" var testProviderURL = "/shoes-mock" var testTime = time.Date(2037, 9, 3, 0, 0, 0, 0, time.UTC) @@ -702,18 +703,57 @@ func TestMySQL_UpdateToken(t *testing.T) { } } -func TestMySQL_UpdateResourceType(t *testing.T) { +func TestMySQL_UpdateTargetParam(t *testing.T) { testDatastore, teardown := testutils.GetTestDatastore() defer teardown() testDB, _ := testutils.GetTestDB() + type input struct { + resourceType datastore.ResourceType + runnerVersion string + runnerUser string + providerURL string + } + tests := []struct { - input datastore.ResourceType + input input want *datastore.Target err bool }{ { - input: datastore.ResourceTypeNano, + input: input{ + resourceType: datastore.ResourceTypeLarge, + runnerVersion: "", + runnerUser: "", + providerURL: "", + }, + want: &datastore.Target{ + Scope: testScopeRepo, + GitHubToken: testGitHubToken, + ResourceType: datastore.ResourceTypeLarge, + RunnerVersion: sql.NullString{ + String: "", + Valid: false, + }, + ProviderURL: sql.NullString{ + String: "", + Valid: false, + }, + Status: datastore.TargetStatusActive, + StatusDescription: sql.NullString{ + String: "", + Valid: false, + }, + }, + err: false, + }, + { + input: input{ + resourceType: datastore.ResourceTypeLarge, + runnerVersion: testRunnerVersion, + runnerUser: testRunnerUser, + providerURL: testProviderURL, + }, want: &datastore.Target{ Scope: testScopeRepo, GitHubToken: testGitHubToken, @@ -722,6 +762,10 @@ func TestMySQL_UpdateResourceType(t *testing.T) { String: testRunnerVersion, Valid: true, }, + RunnerUser: sql.NullString{ + String: testRunnerUser, + Valid: true, + }, ProviderURL: sql.NullString{ String: testProviderURL, Valid: true, @@ -735,7 +779,12 @@ func TestMySQL_UpdateResourceType(t *testing.T) { err: false, }, { - input: datastore.ResourceType4XLarge, + input: input{ + resourceType: datastore.ResourceTypeLarge, + runnerVersion: testRunnerVersion, + runnerUser: testRunnerUser, + providerURL: "", + }, want: &datastore.Target{ Scope: testScopeRepo, GitHubToken: testGitHubToken, @@ -744,10 +793,14 @@ func TestMySQL_UpdateResourceType(t *testing.T) { String: testRunnerVersion, Valid: true, }, - ProviderURL: sql.NullString{ - String: testProviderURL, + RunnerUser: sql.NullString{ + String: testRunnerUser, Valid: true, }, + ProviderURL: sql.NullString{ + String: "", + Valid: false, + }, Status: datastore.TargetStatusActive, StatusDescription: sql.NullString{ String: "", @@ -765,20 +818,24 @@ func TestMySQL_UpdateResourceType(t *testing.T) { Scope: testScopeRepo, GitHubToken: testGitHubToken, TokenExpiredAt: testTime, - ResourceType: test.input, + ResourceType: datastore.ResourceTypeNano, RunnerVersion: sql.NullString{ - String: testRunnerVersion, - Valid: true, + String: "", + Valid: false, + }, + RunnerUser: sql.NullString{ + String: "", + Valid: false, }, ProviderURL: sql.NullString{ - String: testProviderURL, + String: "test-default-string", Valid: true, }, }); err != nil { t.Fatalf("failed to create target: %+v", err) } - if err := testDatastore.UpdateResourceType(context.Background(), tID, test.want.ResourceType); err != nil { + if err := testDatastore.UpdateTargetParam(context.Background(), tID, test.input.resourceType, test.input.runnerVersion, test.input.runnerUser, test.input.providerURL); err != nil { t.Fatalf("failed to UpdateResourceTyoe: %+v", err) } diff --git a/pkg/web/target.go b/pkg/web/target.go index 68b9d4a..8f71abb 100644 --- a/pkg/web/target.go +++ b/pkg/web/target.go @@ -218,14 +218,25 @@ func handleTargetUpdate(w http.ResponseWriter, r *http.Request, ds datastore.Dat outputErrorMsg(w, http.StatusBadRequest, "incorrect target id (not found)") return } - if err := validateUpdateTarget(oldTarget, &newTarget); err != nil { + if err := validateUpdateTarget(*oldTarget, newTarget); err != nil { logger.Logf(false, "input error in validateUpdateTarget: %+v", err) outputErrorMsg(w, http.StatusBadRequest, "request parameter has value of not updatable") return } - if err := ds.UpdateResourceType(ctx, targetID, inputTarget.ResourceType); err != nil { - logger.Logf(false, "failed to ds.UpdateResourceType: %+v", err) + updateParam := getWillUpdateTargetVariable(updatableVariable{ + resourceType: oldTarget.ResourceType, + runnerVersion: oldTarget.RunnerVersion.String, + runnerUser: oldTarget.RunnerUser.String, + providerURL: oldTarget.ProviderURL.String, + }, updatableVariable{ + resourceType: inputTarget.ResourceType, + runnerVersion: inputTarget.RunnerVersion, + runnerUser: inputTarget.RunnerUser, + providerURL: inputTarget.ProviderURL, + }) + if err := ds.UpdateTargetParam(ctx, targetID, updateParam.resourceType, updateParam.runnerVersion, updateParam.runnerUser, updateParam.providerURL); err != nil { + logger.Logf(false, "failed to ds.UpdateTargetParam: %+v", err) outputErrorMsg(w, http.StatusInternalServerError, "datastore update error") return } @@ -301,12 +312,18 @@ func outputErrorMsg(w http.ResponseWriter, status int, msg string) { } // validateUpdateTarget check input target that can valid input in update. -func validateUpdateTarget(old, new *datastore.Target) error { - for _, t := range []*datastore.Target{old, new} { +func validateUpdateTarget(old, new datastore.Target) error { + oldv := old + newv := new + + for _, t := range []*datastore.Target{&oldv, &newv} { t.UUID = uuid.UUID{} // can update variables t.ResourceType = datastore.ResourceTypeUnknown + t.RunnerVersion = sql.NullString{} + t.RunnerUser = sql.NullString{} + t.ProviderURL = sql.NullString{} // time t.TokenExpiredAt = time.Time{} @@ -319,9 +336,39 @@ func validateUpdateTarget(old, new *datastore.Target) error { t.GitHubToken = "" } - if diff := cmp.Diff(old, new); diff != "" { + if diff := cmp.Diff(oldv, newv); diff != "" { return fmt.Errorf("mismatch (-want +got):\n%s", diff) } return nil } + +type updatableVariable struct { + resourceType datastore.ResourceType + runnerVersion string + runnerUser string + providerURL string +} + +func getWillUpdateTargetVariable(oldParam, newParam updatableVariable) updatableVariable { + var result updatableVariable + + if newParam.resourceType == datastore.ResourceTypeUnknown { + result.resourceType = oldParam.resourceType + } else { + result.resourceType = newParam.resourceType + } + + result.runnerVersion = getWillUpdateTargetVariableString(oldParam.runnerVersion, newParam.runnerVersion) + result.runnerUser = getWillUpdateTargetVariableString(oldParam.runnerUser, newParam.runnerUser) + result.providerURL = getWillUpdateTargetVariableString(oldParam.providerURL, newParam.providerURL) + + return result +} + +func getWillUpdateTargetVariableString(old, new string) string { + if strings.EqualFold(new, "") { + return old + } + return new +} diff --git a/pkg/web/target_create.go b/pkg/web/target_create.go index c0941f1..47d94ee 100644 --- a/pkg/web/target_create.go +++ b/pkg/web/target_create.go @@ -86,7 +86,18 @@ func handleTargetCreate(w http.ResponseWriter, r *http.Request, ds datastore.Dat outputErrorMsg(w, http.StatusInternalServerError, "datastore recreate error") return } - if err := ds.UpdateResourceType(ctx, target.UUID, t.ResourceType); err != nil { + updateParam := getWillUpdateTargetVariable(updatableVariable{ + resourceType: target.ResourceType, + runnerVersion: target.RunnerVersion.String, + runnerUser: target.RunnerUser.String, + providerURL: target.ProviderURL.String, + }, updatableVariable{ + resourceType: inputTarget.ResourceType, + runnerVersion: inputTarget.RunnerVersion, + runnerUser: inputTarget.RunnerUser, + providerURL: inputTarget.ProviderURL, + }) + if err := ds.UpdateTargetParam(ctx, target.UUID, updateParam.resourceType, updateParam.runnerVersion, updateParam.runnerUser, updateParam.providerURL); err != nil { logger.Logf(false, "failed to update resource type in recreating target: %+v", err) outputErrorMsg(w, http.StatusInternalServerError, "update resource type error") return diff --git a/pkg/web/target_test.go b/pkg/web/target_test.go index 1ebaa38..705257a 100644 --- a/pkg/web/target_test.go +++ b/pkg/web/target_test.go @@ -350,22 +350,6 @@ func Test_handleTargetUpdate(t *testing.T) { setStubFunctions() - target := `{"scope": "repo", "resource_type": "micro", "runner_user": "ubuntu"}` - - resp, err := http.Post(testURL+"/target", "application/json", bytes.NewBufferString(target)) - if err != nil { - t.Fatalf("failed to POST request: %+v", err) - } - content, statusCode := parseResponse(resp) - if statusCode != http.StatusCreated { - t.Fatalf("must be response statuscode is 201, but got %d: %+v", resp.StatusCode, string(content)) - } - var respTarget web.UserTarget - if err := json.Unmarshal(content, &respTarget); err != nil { - t.Fatalf("failed to unmarshal response JSON: %+v", err) - } - targetUUID := respTarget.UUID - tests := []struct { input string want *web.UserTarget @@ -374,17 +358,60 @@ func Test_handleTargetUpdate(t *testing.T) { { input: `{"scope": "repo", "resource_type": "nano", "runner_user": "ubuntu"}`, want: &web.UserTarget{ - UUID: targetUUID, + UUID: uuid.UUID{}, Scope: "repo", TokenExpiredAt: testTime, ResourceType: datastore.ResourceTypeNano.String(), RunnerUser: "ubuntu", + RunnerVersion: "", + ProviderURL: "", + Status: datastore.TargetStatusActive, + }, + }, + { + input: `{"scope": "repo", "resource_type": "micro", "runner_user": "super-user", "runner_version": "v9.999.9", "provider_url": "https://example.com/shoes-provider"}`, + want: &web.UserTarget{ + UUID: uuid.UUID{}, + Scope: "repo", + TokenExpiredAt: testTime, + ResourceType: datastore.ResourceTypeMicro.String(), + RunnerUser: "super-user", + RunnerVersion: "v9.999.9", + ProviderURL: "https://example.com/shoes-provider", + Status: datastore.TargetStatusActive, + }, + }, + { + input: `{"scope": "repo", "resource_type": "nano"}`, + want: &web.UserTarget{ + UUID: uuid.UUID{}, + Scope: "repo", + TokenExpiredAt: testTime, + ResourceType: datastore.ResourceTypeNano.String(), + RunnerUser: "ubuntu", + RunnerVersion: "", + ProviderURL: "", Status: datastore.TargetStatusActive, }, }, } for _, test := range tests { + target := `{"scope": "repo", "resource_type": "micro", "runner_user": "ubuntu"}` + respCreate, err := http.Post(testURL+"/target", "application/json", bytes.NewBufferString(target)) + if err != nil { + t.Fatalf("failed to POST request: %+v", err) + } + contentCreate, statusCode := parseResponse(respCreate) + if statusCode != http.StatusCreated { + t.Fatalf("must be response statuscode is 201, but got %d: %+v", respCreate.StatusCode, string(contentCreate)) + } + var respTarget web.UserTarget + if err := json.Unmarshal(contentCreate, &respTarget); err != nil { + t.Fatalf("failed to unmarshal response JSON: %+v", err) + } + targetUUID := respTarget.UUID + resp, err := http.Post(fmt.Sprintf("%s/target/%s", testURL, targetUUID.String()), "application/json", bytes.NewBufferString(test.input)) if !test.err && err != nil { t.Fatalf("failed to POST request: %+v", err) @@ -399,12 +426,15 @@ func Test_handleTargetUpdate(t *testing.T) { t.Fatalf("failed to unmarshal resoponse content: %+v", err) } + got.UUID = uuid.UUID{} got.CreatedAt = time.Time{} got.UpdatedAt = time.Time{} if diff := cmp.Diff(test.want, &got); diff != "" { t.Errorf("mismatch (-want +got):\n%s", diff) } + + teardown() } }