Skip to content

Commit

Permalink
ddl: add format error for incorrect dict syntax in the placement rule
Browse files Browse the repository at this point in the history
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
  • Loading branch information
hawkingrei committed Dec 22, 2021
1 parent 529ce88 commit 419b035
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 16 deletions.
4 changes: 4 additions & 0 deletions ddl/placement/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,8 @@ var (
ErrNoRulesToDrop = errors.New("no rule of such role to drop")
// ErrInvalidPlacementOptions is from bundle.go.
ErrInvalidPlacementOptions = errors.New("invalid placement option")
// ErrInvalidConstraintsMappingWrongSeparator is wrong separator in mapping.
ErrInvalidConstraintsMappingWrongSeparator = errors.New("mappings use a colon and space (“: ”) to mark each key/value pair")
// ErrInvalidConstraintsMappingNoColonFound is no colon found in mapping.
ErrInvalidConstraintsMappingNoColonFound = errors.New("no colon found")
)
16 changes: 16 additions & 0 deletions ddl/placement/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package placement

import (
"fmt"
"regexp"
"strings"

"gopkg.in/yaml.v2"
Expand Down Expand Up @@ -62,6 +63,18 @@ func NewRule(role PeerRoleType, replicas uint64, cnst Constraints) *Rule {
}
}

var wrongSeparatorRegexp = regexp.MustCompile(`[^"':]+:\d`)

func getYamlMapFormatError(str string) error {
if !strings.Contains(str, ":") {
return ErrInvalidConstraintsMappingNoColonFound
}
if wrongSeparatorRegexp.MatchString(str) {
return ErrInvalidConstraintsMappingWrongSeparator
}
return nil
}

// NewRules constructs []*Rule from a yaml-compatible representation of
// 'array' or 'dict' constraints.
// Refer to https://github.com/pingcap/tidb/blob/master/docs/design/2020-06-24-placement-rules-in-sql.md.
Expand All @@ -87,6 +100,9 @@ func NewRules(role PeerRoleType, replicas uint64, cnstr string) ([]*Rule, error)
ruleCnt := 0
for labels, cnt := range constraints2 {
if cnt <= 0 {
if err := getYamlMapFormatError(string(cnstbytes)); err != nil {
return rules, err
}
return rules, fmt.Errorf("%w: count of labels '%s' should be positive, but got %d", ErrInvalidConstraintsMapcnt, labels, cnt)
}
ruleCnt += cnt
Expand Down
53 changes: 37 additions & 16 deletions ddl/placement/rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,20 @@ package placement

import (
"errors"
"reflect"
"testing"

. "github.com/pingcap/check"
"github.com/stretchr/testify/require"
)

var _ = Suite(&testRuleSuite{})

type testRuleSuite struct{}

func (t *testRuleSuite) TestClone(c *C) {
func TestClone(t *testing.T) {
rule := &Rule{ID: "434"}
newRule := rule.Clone()
newRule.ID = "121"

c.Assert(rule, DeepEquals, &Rule{ID: "434"})
c.Assert(newRule, DeepEquals, &Rule{ID: "121"})
require.Equal(t, &Rule{ID: "434"}, rule)
require.Equal(t, &Rule{ID: "121"}, newRule)
}

func matchRules(t1, t2 []*Rule, prefix string, c *C) {
Expand All @@ -50,15 +49,30 @@ func matchRules(t1, t2 []*Rule, prefix string, c *C) {
}
}

func (t *testRuleSuite) TestNewRuleAndNewRules(c *C) {
func matchRulesT(t1, t2 []*Rule, prefix string, t *testing.T) {
require.Equal(t, len(t2), len(t1), prefix)
for i := range t1 {
found := false
for j := range t2 {
ok := reflect.DeepEqual(t2[j], t1[i])
if ok {
found = true
break
}
}
require.True(t, found, "%s\n\ncan not found %d rule\n%+v\n%+v", prefix, i, t1[i], t2)
}
}

func TestNewRuleAndNewRules(t *testing.T) {
type TestCase struct {
name string
input string
replicas uint64
output []*Rule
err error
}
tests := []TestCase{}
var tests []TestCase

tests = append(tests, TestCase{
name: "empty constraints",
Expand Down Expand Up @@ -175,14 +189,21 @@ func (t *testRuleSuite) TestNewRuleAndNewRules(c *C) {
err: ErrInvalidConstraintFormat,
})

for _, t := range tests {
comment := Commentf("[%s]", t.name)
output, err := NewRules(Voter, t.replicas, t.input)
if t.err == nil {
c.Assert(err, IsNil, comment)
matchRules(t.output, output, comment.CheckCommentString(), c)
tests = append(tests, TestCase{
name: "invalid map separator",
input: `{+region=us-east-2:2}`,
replicas: 6,
err: ErrInvalidConstraintsMappingWrongSeparator,
})

for _, tt := range tests {
comment := Commentf("[%s]", tt.name)
output, err := NewRules(Voter, tt.replicas, tt.input)
if tt.err == nil {
require.NoError(t, err, comment)
matchRulesT(tt.output, output, comment.CheckCommentString(), t)
} else {
c.Assert(errors.Is(err, t.err), IsTrue, Commentf("[%s]\n%s\n%s\n", t.name, err, t.err))
require.True(t, errors.Is(err, tt.err), "[%s]\n%s\n%s\n", tt.name, err, tt.err)
}
}
}

0 comments on commit 419b035

Please sign in to comment.