From 44463023308e7ad156e8bf7cfb7a63fe04a89d0a Mon Sep 17 00:00:00 2001 From: chlins Date: Sun, 2 Aug 2020 13:59:54 +0800 Subject: [PATCH] refactor(preheat): refactor policy schema serialize funcs Signed-off-by: chlins --- src/controller/p2p/preheat/controller.go | 26 +----- src/pkg/p2p/preheat/models/policy/policy.go | 90 +++++++++++++++++++ .../p2p/preheat/models/policy/policy_test.go | 62 +++++++++++++ src/pkg/p2p/preheat/policy/manager.go | 87 ++---------------- src/pkg/p2p/preheat/policy/manager_test.go | 48 ---------- 5 files changed, 165 insertions(+), 148 deletions(-) diff --git a/src/controller/p2p/preheat/controller.go b/src/controller/p2p/preheat/controller.go index b8634e714..84cef2ac5 100644 --- a/src/controller/p2p/preheat/controller.go +++ b/src/controller/p2p/preheat/controller.go @@ -2,7 +2,6 @@ package preheat import ( "context" - "encoding/json" "time" "github.com/goharbor/harbor/src/lib/errors" @@ -223,7 +222,7 @@ func (c *controller) CreatePolicy(ctx context.Context, schema *policyModels.Sche schema.UpdatedTime = now // Get full model of policy schema - _, err = policy.ParsePolicy(schema) + err = schema.Decode() if err != nil { return 0, err } @@ -244,7 +243,7 @@ func (c *controller) CreatePolicy(ctx context.Context, schema *policyModels.Sche return 0, err } - if err = decodeSchema(schema); err == nil { + if err = schema.Encode(); err == nil { err = c.pManager.Update(ctx, schema, "trigger") } @@ -288,7 +287,7 @@ func (c *controller) UpdatePolicy(ctx context.Context, schema *policyModels.Sche } // Get full model of updating policy - _, err = policy.ParsePolicy(schema) + err = schema.Decode() if err != nil { return err } @@ -337,7 +336,7 @@ func (c *controller) UpdatePolicy(ctx context.Context, schema *policyModels.Sche return err } schema.Trigger.Settings.JobID = jobid - if err := decodeSchema(schema); err != nil { + if err := schema.Encode(); err != nil { // Possible // TODO: Refactor return err // whether update or not has been not important as the jon reference will be lost @@ -405,20 +404,3 @@ func (c *controller) CheckHealth(ctx context.Context, instance *providerModels.I return nil } - -// decodeSchema decodes the trigger object to JSON string -func decodeSchema(schema *policyModels.Schema) error { - if schema.Trigger == nil { - // do nothing - return nil - } - - b, err := json.Marshal(schema.Trigger) - if err != nil { - return err - } - - schema.TriggerStr = string(b) - - return nil -} diff --git a/src/pkg/p2p/preheat/models/policy/policy.go b/src/pkg/p2p/preheat/models/policy/policy.go index 69656bd69..72d0bcfe7 100644 --- a/src/pkg/p2p/preheat/models/policy/policy.go +++ b/src/pkg/p2p/preheat/models/policy/policy.go @@ -15,11 +15,14 @@ package policy import ( + "encoding/json" "fmt" + "strconv" "time" beego_orm "github.com/astaxie/beego/orm" "github.com/astaxie/beego/validation" + "github.com/goharbor/harbor/src/lib/errors" "github.com/robfig/cron" ) @@ -159,3 +162,90 @@ func (s *Schema) Valid(v *validation.Validation) { } } } + +// Encode encodes policy schema. +func (s *Schema) Encode() error { + if s.Filters != nil { + filterStr, err := json.Marshal(s.Filters) + if err != nil { + return err + } + s.FiltersStr = string(filterStr) + } + + if s.Trigger != nil { + triggerStr, err := json.Marshal(s.Trigger) + if err != nil { + return err + } + s.TriggerStr = string(triggerStr) + } + + return nil +} + +// Decode decodes policy schema. +func (s *Schema) Decode() error { + // parse filters + filters, err := decodeFilters(s.FiltersStr) + if err != nil { + return err + } + s.Filters = filters + + // parse trigger + trigger, err := decodeTrigger(s.TriggerStr) + if err != nil { + return err + } + s.Trigger = trigger + + return nil +} + +// decodeFilters parse filterStr to filter. +func decodeFilters(filterStr string) ([]*Filter, error) { + // Filters are required + if len(filterStr) == 0 { + return nil, errors.New("missing filters in preheat policy schema") + } + + var filters []*Filter + if err := json.Unmarshal([]byte(filterStr), &filters); err != nil { + return nil, err + } + + // Convert value type + // TODO: remove switch after UI bug #12579 fixed + for _, f := range filters { + if f.Type == FilterTypeVulnerability { + switch f.Value.(type) { + case string: + sev, err := strconv.ParseInt(f.Value.(string), 10, 32) + if err != nil { + return nil, errors.Wrapf(err, "parse filters") + } + f.Value = (int)(sev) + case float64: + f.Value = (int)(f.Value.(float64)) + } + } + } + + return filters, nil +} + +// decodeTrigger parse triggerStr to trigger. +func decodeTrigger(triggerStr string) (*Trigger, error) { + // trigger must be existing, at least is a "manual" trigger. + if len(triggerStr) == 0 { + return nil, errors.New("missing trigger settings in preheat policy schema") + } + + trigger := &Trigger{} + if err := json.Unmarshal([]byte(triggerStr), trigger); err != nil { + return nil, err + } + + return trigger, nil +} diff --git a/src/pkg/p2p/preheat/models/policy/policy_test.go b/src/pkg/p2p/preheat/models/policy/policy_test.go index cf1aff195..a67ba47a1 100644 --- a/src/pkg/p2p/preheat/models/policy/policy_test.go +++ b/src/pkg/p2p/preheat/models/policy/policy_test.go @@ -129,3 +129,65 @@ func (p *PolicyTestSuite) TestValid() { p.schema.Valid(v) require.False(p.T(), v.HasErrors(), "should return nil error") } + +// TestDecode tests decode. +func (p *PolicyTestSuite) TestDecode() { + s := &Schema{ + ID: 100, + Name: "test-for-decode", + Description: "", + ProjectID: 1, + ProviderID: 1, + Filters: nil, + FiltersStr: "[{\"type\":\"repository\",\"value\":\"**\"},{\"type\":\"tag\",\"value\":\"**\"},{\"type\":\"label\",\"value\":\"test\"}]", + Trigger: nil, + TriggerStr: "{\"type\":\"event_based\",\"trigger_setting\":{\"cron\":\"\"}}", + Enabled: false, + } + p.NoError(s.Decode()) + p.Len(s.Filters, 3) + p.NotNil(s.Trigger) + + // invalid filter or trigger + s.FiltersStr = "" + s.TriggerStr = "invalid" + p.Error(s.Decode()) + + s.FiltersStr = "invalid" + s.TriggerStr = "" + p.Error(s.Decode()) +} + +// TestEncode tests encode. +func (p *PolicyTestSuite) TestEncode() { + s := &Schema{ + ID: 101, + Name: "test-for-encode", + Description: "", + ProjectID: 2, + ProviderID: 2, + Filters: []*Filter{ + { + Type: FilterTypeRepository, + Value: "**", + }, + { + Type: FilterTypeTag, + Value: "**", + }, + { + Type: FilterTypeLabel, + Value: "test", + }, + }, + FiltersStr: "", + Trigger: &Trigger{ + Type: "event_based", + }, + TriggerStr: "", + Enabled: false, + } + p.NoError(s.Encode()) + p.Equal(`[{"type":"repository","value":"**"},{"type":"tag","value":"**"},{"type":"label","value":"test"}]`, s.FiltersStr) + p.Equal(`{"type":"event_based","trigger_setting":{}}`, s.TriggerStr) +} diff --git a/src/pkg/p2p/preheat/policy/manager.go b/src/pkg/p2p/preheat/policy/manager.go index def4c9d3e..756fc1596 100644 --- a/src/pkg/p2p/preheat/policy/manager.go +++ b/src/pkg/p2p/preheat/policy/manager.go @@ -16,10 +16,7 @@ package policy import ( "context" - "encoding/json" - "strconv" - "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/q" dao "github.com/goharbor/harbor/src/pkg/p2p/preheat/dao/policy" "github.com/goharbor/harbor/src/pkg/p2p/preheat/models/policy" @@ -81,7 +78,10 @@ func (m *manager) Get(ctx context.Context, id int64) (schema *policy.Schema, err return nil, err } - return ParsePolicy(schema) + if err = schema.Decode(); err != nil { + return nil, err + } + return schema, nil } // Get the policy schema by name @@ -91,7 +91,10 @@ func (m *manager) GetByName(ctx context.Context, projectID int64, name string) ( return nil, err } - return ParsePolicy(schema) + if err = schema.Decode(); err != nil { + return nil, err + } + return schema, nil } // Delete the policy schema by id @@ -107,11 +110,9 @@ func (m *manager) ListPolicies(ctx context.Context, query *q.Query) (schemas []* } for i := range schemas { - schema, err := ParsePolicy(schemas[i]) - if err != nil { + if err = schemas[i].Decode(); err != nil { return nil, err } - schemas[i] = schema } return schemas, nil @@ -131,73 +132,3 @@ func (m *manager) ListPoliciesByProject(ctx context.Context, project int64, quer return m.ListPolicies(ctx, query) } - -// ParsePolicy parses persisting data to policy model. -func ParsePolicy(schema *policy.Schema) (*policy.Schema, error) { - if schema == nil { - return nil, errors.New("policy schema can not be nil") - } - - // parse filters - filters, err := parseFilters(schema.FiltersStr) - if err != nil { - return nil, err - } - schema.Filters = filters - - // parse trigger - trigger, err := parseTrigger(schema.TriggerStr) - if err != nil { - return nil, err - } - schema.Trigger = trigger - - return schema, nil -} - -// parseFilters parse filterStr to filter. -func parseFilters(filterStr string) ([]*policy.Filter, error) { - // Filters are required - if len(filterStr) == 0 { - return nil, errors.New("missing filters in preheat policy schema") - } - - var filters []*policy.Filter - if err := json.Unmarshal([]byte(filterStr), &filters); err != nil { - return nil, err - } - - // Convert value type - // TODO: remove switch after UI bug #12579 fixed - for _, f := range filters { - if f.Type == policy.FilterTypeVulnerability { - switch f.Value.(type) { - case string: - sev, err := strconv.ParseInt(f.Value.(string), 10, 32) - if err != nil { - return nil, errors.Wrapf(err, "parse filters") - } - f.Value = (int)(sev) - case float64: - f.Value = (int)(f.Value.(float64)) - } - } - } - - return filters, nil -} - -// parseTrigger parse triggerStr to trigger. -func parseTrigger(triggerStr string) (*policy.Trigger, error) { - // trigger must be existing, at least is a "manual" trigger. - if len(triggerStr) == 0 { - return nil, errors.New("missing trigger settings in preheat policy schema") - } - - trigger := &policy.Trigger{} - if err := json.Unmarshal([]byte(triggerStr), trigger); err != nil { - return nil, err - } - - return trigger, nil -} diff --git a/src/pkg/p2p/preheat/policy/manager_test.go b/src/pkg/p2p/preheat/policy/manager_test.go index 6e0823a17..d63fbcace 100644 --- a/src/pkg/p2p/preheat/policy/manager_test.go +++ b/src/pkg/p2p/preheat/policy/manager_test.go @@ -159,51 +159,3 @@ func (m *managerTestSuite) TestListPoliciesByProject() { _, err := m.mgr.ListPoliciesByProject(nil, 1, nil) m.Require().Nil(err) } - -// TestParsePolicy tests parsePolicy. -func (m *managerTestSuite) TestParsePolicy() { - schema := &policy.Schema{FiltersStr: "invalid"} - _, err := ParsePolicy(schema) - m.Require().Error(err) - - schema = &policy.Schema{TriggerStr: "invalid"} - _, err = ParsePolicy(schema) - m.Require().Error(err) - - schema = &policy.Schema{ - FiltersStr: `[ - { - "type": "repository", - "value": "dev**" - }, - { - "type": "tag", - "value": "v1*" - }, - { - "type": "label", - "value": [ - "lb1", - "lb2" - ] - }, - { - "type": "signature", - "value": true - } -]`, - TriggerStr: `{ - "type": "scheduled", - "trigger_setting": { - "cron": "0 0 2 1 * ? *" - } -}`, - } - schema, err = ParsePolicy(schema) - m.Require().NoError(err) - m.Require().NotNil(schema.Trigger) - m.Require().Equal("0 0 2 1 * ? *", schema.Trigger.Settings.Cron) - m.Require().NotNil(schema.Filters) - m.Require().Len(schema.Filters, 4) - m.Require().True(schema.Filters[3].Value.(bool)) -}