Merge pull request #12647 from chlins/refactor/preheat-policy-serialize

refactor(preheat): refactor policy schema serialize funcs

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
-}

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
+}

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)
+}

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 = schemas[i].Decode(); err != nil {
-		if 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
-}

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))
-}