From 863277af2da7738a8cb95283870fdae46bd326ef Mon Sep 17 00:00:00 2001 From: Simone Gotti Date: Sat, 8 Jun 2019 16:30:36 +0200 Subject: [PATCH] runservice types: refactor unmarshal * Don't use a complex UnmarshalJSON for RunConfigTask and ExecutorTask but introduce a Steps type as a slice of Step (where Step is an empty interface) and declare an UnmarshalJSON method on the Step type. --- internal/runconfig/runconfig.go | 2 +- internal/runconfig/runconfig_test.go | 16 +-- .../services/runservice/action/action_test.go | 20 ++-- .../services/runservice/scheduler_test.go | 20 ++-- internal/services/runservice/types/types.go | 103 +++--------------- 5 files changed, 47 insertions(+), 114 deletions(-) diff --git a/internal/runconfig/runconfig.go b/internal/runconfig/runconfig.go index 651d571..f57f1a2 100644 --- a/internal/runconfig/runconfig.go +++ b/internal/runconfig/runconfig.go @@ -196,7 +196,7 @@ func GenRunConfigTasks(uuid util.UUIDGenerator, c *config.Config, runName string for _, ct := range cr.Tasks { include := types.MatchWhen(whenFromConfigWhen(ct.When), branch, tag, ref) - steps := make([]interface{}, len(ct.Steps)) + steps := make(rstypes.Steps, len(ct.Steps)) for i, cpts := range ct.Steps { steps[i] = stepFromConfigStep(cpts, variables) } diff --git a/internal/runconfig/runconfig_test.go b/internal/runconfig/runconfig_test.go index d555dae..ba0c5d6 100644 --- a/internal/runconfig/runconfig_test.go +++ b/internal/runconfig/runconfig_test.go @@ -748,10 +748,10 @@ func TestGenRunConfig(t *testing.T) { "ENV01": "ENV01", "ENVFROMVARIABLE01": "VARVALUE01", }, - Steps: []interface{}{ - &rstypes.RunStep{Step: rstypes.Step{Type: "run", Name: "command01"}, Command: "command01", Environment: map[string]string{}}, - &rstypes.RunStep{Step: rstypes.Step{Type: "run", Name: "name different than command"}, Command: "command02", Environment: map[string]string{}}, - &rstypes.RunStep{Step: rstypes.Step{Type: "run", Name: "command03"}, Command: "command03", Environment: map[string]string{"ENV01": "ENV01", "ENVFROMVARIABLE01": "VARVALUE01"}}, + Steps: rstypes.Steps{ + &rstypes.RunStep{BaseStep: rstypes.BaseStep{Type: "run", Name: "command01"}, Command: "command01", Environment: map[string]string{}}, + &rstypes.RunStep{BaseStep: rstypes.BaseStep{Type: "run", Name: "name different than command"}, Command: "command02", Environment: map[string]string{}}, + &rstypes.RunStep{BaseStep: rstypes.BaseStep{Type: "run", Name: "command03"}, Command: "command03", Environment: map[string]string{"ENV01": "ENV01", "ENVFROMVARIABLE01": "VARVALUE01"}}, }, Skip: true, }, @@ -820,8 +820,8 @@ func TestGenRunConfig(t *testing.T) { }, }, Environment: map[string]string{}, - Steps: []interface{}{ - &rstypes.RunStep{Step: rstypes.Step{Type: "run", Name: "command01"}, Command: "command01", Environment: map[string]string{}}, + Steps: rstypes.Steps{ + &rstypes.RunStep{BaseStep: rstypes.BaseStep{Type: "run", Name: "command01"}, Command: "command01", Environment: map[string]string{}}, }, }, }, @@ -917,8 +917,8 @@ func TestGenRunConfig(t *testing.T) { }, }, Environment: map[string]string{}, - Steps: []interface{}{ - &rstypes.RunStep{Step: rstypes.Step{Type: "run", Name: "command01"}, Command: "command01", Environment: map[string]string{}}, + Steps: rstypes.Steps{ + &rstypes.RunStep{BaseStep: rstypes.BaseStep{Type: "run", Name: "command01"}, Command: "command01", Environment: map[string]string{}}, }, }, }, diff --git a/internal/services/runservice/action/action_test.go b/internal/services/runservice/action/action_test.go index 585a424..ddb3af0 100644 --- a/internal/services/runservice/action/action_test.go +++ b/internal/services/runservice/action/action_test.go @@ -46,7 +46,7 @@ func TestRecreateRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, inuuid("task02"): &types.RunConfigTask{ @@ -59,7 +59,7 @@ func TestRecreateRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, inuuid("task03"): &types.RunConfigTask{ @@ -70,7 +70,7 @@ func TestRecreateRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, inuuid("task04"): &types.RunConfigTask{ @@ -80,7 +80,7 @@ func TestRecreateRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, inuuid("task05"): &types.RunConfigTask{ @@ -94,7 +94,7 @@ func TestRecreateRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, }, @@ -111,7 +111,7 @@ func TestRecreateRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, outuuid("task02"): &types.RunConfigTask{ @@ -124,7 +124,7 @@ func TestRecreateRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, outuuid("task03"): &types.RunConfigTask{ @@ -135,7 +135,7 @@ func TestRecreateRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, outuuid("task04"): &types.RunConfigTask{ @@ -145,7 +145,7 @@ func TestRecreateRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, outuuid("task05"): &types.RunConfigTask{ @@ -159,7 +159,7 @@ func TestRecreateRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, }, diff --git a/internal/services/runservice/scheduler_test.go b/internal/services/runservice/scheduler_test.go index e69f1a2..5be49ff 100644 --- a/internal/services/runservice/scheduler_test.go +++ b/internal/services/runservice/scheduler_test.go @@ -35,7 +35,7 @@ func TestAdvanceRunTasks(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, "task02": &types.RunConfigTask{ @@ -48,7 +48,7 @@ func TestAdvanceRunTasks(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, "task03": &types.RunConfigTask{ @@ -59,7 +59,7 @@ func TestAdvanceRunTasks(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, "task04": &types.RunConfigTask{ @@ -69,7 +69,7 @@ func TestAdvanceRunTasks(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, "task05": &types.RunConfigTask{ @@ -83,7 +83,7 @@ func TestAdvanceRunTasks(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, }, @@ -383,7 +383,7 @@ func TestGetTasksToRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, "task02": &types.RunConfigTask{ @@ -396,7 +396,7 @@ func TestGetTasksToRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, "task03": &types.RunConfigTask{ @@ -407,7 +407,7 @@ func TestGetTasksToRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, "task04": &types.RunConfigTask{ @@ -417,7 +417,7 @@ func TestGetTasksToRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, "task05": &types.RunConfigTask{ @@ -431,7 +431,7 @@ func TestGetTasksToRun(t *testing.T) { Containers: []*types.Container{{Image: "image01"}}, }, Environment: map[string]string{}, - Steps: []interface{}{}, + Steps: types.Steps{}, Skip: false, }, }, diff --git a/internal/services/runservice/types/types.go b/internal/services/runservice/types/types.go index 069e697..7298ab2 100644 --- a/internal/services/runservice/types/types.go +++ b/internal/services/runservice/types/types.go @@ -326,7 +326,7 @@ type RunConfigTask struct { WorkingDir string `json:"working_dir,omitempty"` Shell string `json:"shell,omitempty"` User string `json:"user,omitempty"` - Steps []interface{} `json:"steps,omitempty"` + Steps Steps `json:"steps,omitempty"` IgnoreFailure bool `json:"ignore_failure,omitempty"` NeedsApproval bool `json:"needs_approval,omitempty"` Skip bool `json:"skip,omitempty"` @@ -378,75 +378,17 @@ type Runtime struct { Containers []*Container `json:"containers,omitempty"` } -func (rct *RunConfigTask) UnmarshalJSON(b []byte) error { - type rctask RunConfigTask +type Step interface{} - type task struct { - Steps []json.RawMessage `json:"steps,omitempty"` - } +type Steps []Step - rctt := (*rctask)(rct) - if err := json.Unmarshal(b, &rctt); err != nil { - return err - } - - var st task - if err := json.Unmarshal(b, &st); err != nil { - return err - } - - steps := make([]interface{}, len(st.Steps)) - for i, step := range st.Steps { - var bs Step - if err := json.Unmarshal(step, &bs); err != nil { - return err - } - switch bs.Type { - case "run": - var s RunStep - if err := json.Unmarshal(step, &s); err != nil { - return err - } - steps[i] = &s - case "save_to_workspace": - var s SaveToWorkspaceStep - if err := json.Unmarshal(step, &s); err != nil { - return err - } - steps[i] = &s - case "restore_workspace": - var s RestoreWorkspaceStep - if err := json.Unmarshal(step, &s); err != nil { - return err - } - steps[i] = &s - case "save_cache": - var s SaveCacheStep - if err := json.Unmarshal(step, &s); err != nil { - return err - } - steps[i] = &s - case "restore_cache": - var s RestoreCacheStep - if err := json.Unmarshal(step, &s); err != nil { - return err - } - steps[i] = &s - } - } - - rct.Steps = steps - - return nil -} - -type Step struct { +type BaseStep struct { Type string `json:"type,omitempty"` Name string `json:"name,omitempty"` } type RunStep struct { - Step + BaseStep Command string `json:"command,omitempty"` Environment map[string]string `json:"environment,omitempty"` WorkingDir string `json:"working_dir,omitempty"` @@ -461,23 +403,23 @@ type SaveContent struct { } type SaveToWorkspaceStep struct { - Step + BaseStep Contents []SaveContent `json:"contents,omitempty"` } type RestoreWorkspaceStep struct { - Step + BaseStep DestDir string `json:"dest_dir,omitempty"` } type SaveCacheStep struct { - Step + BaseStep Key string `json:"key,omitempty"` Contents []SaveContent `json:"contents,omitempty"` } type RestoreCacheStep struct { - Step + BaseStep Keys []string `json:"keys,omitempty"` DestDir string `json:"dest_dir,omitempty"` } @@ -512,7 +454,7 @@ type ExecutorTask struct { DockerRegistriesAuth map[string]DockerRegistryAuth `json:"docker_registries_auth"` - Steps []interface{} `json:"steps,omitempty"` + Steps Steps `json:"steps,omitempty"` Status ExecutorTaskStatus `json:"status,omitempty"` SetupError string `fail_reason:"setup_error,omitempty"` @@ -562,26 +504,17 @@ type WorkspaceOperation struct { Overwrite bool `json:"overwrite,omitempty"` } -func (et *ExecutorTask) UnmarshalJSON(b []byte) error { - type etask ExecutorTask +func (et *Steps) UnmarshalJSON(b []byte) error { + type rawSteps []json.RawMessage - type task struct { - Steps []json.RawMessage `json:"steps,omitempty"` - } - - ett := (*etask)(et) - if err := json.Unmarshal(b, &ett); err != nil { + var rs rawSteps + if err := json.Unmarshal(b, &rs); err != nil { return err } - var st task - if err := json.Unmarshal(b, &st); err != nil { - return err - } - - steps := make([]interface{}, len(ett.Steps)) - for i, step := range st.Steps { - var bs Step + steps := make(Steps, len(rs)) + for i, step := range rs { + var bs BaseStep if err := json.Unmarshal(step, &bs); err != nil { return err } @@ -619,7 +552,7 @@ func (et *ExecutorTask) UnmarshalJSON(b []byte) error { } } - et.Steps = steps + *et = steps return nil }