From bfb59e5353bafae40360a683debbc4d2f477734c Mon Sep 17 00:00:00 2001 From: Xi Chen Date: Tue, 10 Oct 2017 16:55:40 -0400 Subject: [PATCH] Reuse RuleSetSnapshot struct to simplify code --- rules/ruleset.go | 16 ++++++------- rules/validator.go | 60 +++++++++++----------------------------------- 2 files changed, 22 insertions(+), 54 deletions(-) diff --git a/rules/ruleset.go b/rules/ruleset.go index ccbf2bb..bd3f62a 100644 --- a/rules/ruleset.go +++ b/rules/ruleset.go @@ -967,31 +967,31 @@ func (rs *ruleSet) getRollupRuleByID(id string) (*rollupRule, error) { return nil, errNoSuchRule } -func (rs *ruleSet) latestMappingRules() ([]*MappingRuleView, error) { +func (rs *ruleSet) latestMappingRules() (map[string]*MappingRuleView, error) { mrs, err := rs.MappingRules() if err != nil { return nil, err } - result := make([]*MappingRuleView, 0, len(mrs)) + result := make(map[string]*MappingRuleView, len(mrs)) for _, m := range mrs { if len(m) > 0 && !m[0].Tombstoned { // views included in m are sorted latest first. - result = append(result, m[0]) + result[m[0].ID] = m[0] } } return result, nil } -func (rs *ruleSet) latestRollupRules() ([]*RollupRuleView, error) { +func (rs *ruleSet) latestRollupRules() (map[string]*RollupRuleView, error) { rrs, err := rs.RollupRules() if err != nil { return nil, err } - result := make([]*RollupRuleView, 0, len(rrs)) + result := make(map[string]*RollupRuleView, len(rrs)) for _, r := range rrs { if len(r) > 0 && !r[0].Tombstoned { // views included in m are sorted latest first. - result = append(result, r[0]) + result[r[0].ID] = r[0] } } return result, nil @@ -1151,8 +1151,8 @@ type RuleSetSnapshot struct { Namespace string Version int CutoverNanos int64 - MappingRules []*MappingRuleView - RollupRules []*RollupRuleView + MappingRules map[string]*MappingRuleView + RollupRules map[string]*RollupRuleView } func (rs ruleSet) validateMappingRuleUpdate(mrv MappingRuleView) error { diff --git a/rules/validator.go b/rules/validator.go index 77504e0..2e29b32 100644 --- a/rules/validator.go +++ b/rules/validator.go @@ -21,7 +21,6 @@ package rules import ( - "errors" "fmt" "github.com/m3db/m3metrics/filters" @@ -29,11 +28,6 @@ import ( "github.com/m3db/m3metrics/policy" ) -var ( - errEmptyMappingRuleViewList = errors.New("empty mapping rule view list") - errEmptyRollupRuleViewList = errors.New("empty rollup rule view list") -) - // Validator validates a ruleset. type Validator interface { // Validate validates a ruleset. @@ -50,42 +44,27 @@ func NewValidator(opts ValidatorOptions) Validator { } func (v *validator) Validate(rs RuleSet) error { + // Only the latest (a.k.a. the first) view needs to be validated + // because that is the view that may be invalid due to latest update. latest, err := rs.Latest() - - // Validate mapping rules. - mappingRules, err := rs.MappingRules() if err != nil { - return err + return NewValidationError(fmt.Sprintf("could not get the latest ruleset snapshot: %v", err)) } - if err := v.validateMappingRules(mappingRules); err != nil { - return err + if err := v.validateMappingRules(latest.MappingRules); err != nil { + return NewValidationError(fmt.Sprintf("could not validate mapping rules: %v", err)) } - - // Validate rollup rules. - rollupRules, err := rs.RollupRules() - if err != nil { - return err + if err := v.validateRollupRules(latest.RollupRules); err != nil { + return NewValidationError(fmt.Sprintf("could not validate rollup rules: %v", err)) } - return v.validateRollupRules(rollupRules) + return nil } -func (v *validator) validateMappingRules(rules MappingRules) error { +func (v *validator) validateMappingRules(rules map[string]*MappingRuleView) error { namesSeen := make(map[string]struct{}, len(rules)) - for _, views := range rules { - if len(views) == 0 { - return errEmptyMappingRuleViewList - } - // Only the latest (a.k.a. the first) view needs to be validated - // because that is the view that may be invalid due to latest update. - view := views[0] - if view.Tombstoned { - continue - } - + for _, view := range rules { // Validate that no rules with the same name exist. if _, exists := namesSeen[view.Name]; exists { - msg := fmt.Sprintf("mapping rule %s already exists", view.Name) - return RuleConflictError{msg: msg, ConflictRuleUUID: view.ID} + return NewRuleConflictError(fmt.Sprintf("mapping rule %s already exists", view.Name)) } namesSeen[view.Name] = struct{}{} @@ -105,23 +84,12 @@ func (v *validator) validateMappingRules(rules MappingRules) error { return nil } -func (v *validator) validateRollupRules(rules RollupRules) error { +func (v *validator) validateRollupRules(rules map[string]*RollupRuleView) error { namesSeen := make(map[string]struct{}, len(rules)) - for _, views := range rules { - if len(views) == 0 { - return errEmptyRollupRuleViewList - } - // Only the latest (a.k.a. the first) view needs to be validated - // because that is the view that may be invalid due to latest update. - view := views[0] - if view.Tombstoned { - continue - } - + for _, view := range rules { // Validate that no rules with the same name exist. if _, exists := namesSeen[view.Name]; exists { - msg := fmt.Sprintf("rollup rule %s already exists", view.Name) - return RuleConflictError{msg: msg, ConflictRuleUUID: view.ID} + return NewRuleConflictError(fmt.Sprintf("rollup rule %s already exists", view.Name)) } namesSeen[view.Name] = struct{}{}