policy: Check for redundant selectors and filters #154

Merged
fyrchik merged 1 commit from acid-ant/frostfs-sdk-go:bugfix/150-check-redundant into master 2023-09-04 14:12:55 +00:00
3 changed files with 107 additions and 63 deletions

View file

@ -551,7 +551,7 @@ func (p *PlacementPolicy) DecodeString(s string) error {
return errors.New("parsed nil value") return errors.New("parsed nil value")
} }
if err := validatePolicy(*p); err != nil { if err := validatePolicy(*parsed); err != nil {
return fmt.Errorf("invalid policy: %w", err) return fmt.Errorf("invalid policy: %w", err)
} }
@ -605,6 +605,13 @@ var (
errUnknownSelector = errors.New("policy: selector not found") errUnknownSelector = errors.New("policy: selector not found")
// errSyntaxError is returned for errors found by ANTLR parser. // errSyntaxError is returned for errors found by ANTLR parser.
errSyntaxError = errors.New("policy: syntax error") errSyntaxError = errors.New("policy: syntax error")
// errRedundantSelector is returned for errors found by selectors policy validator.
errRedundantSelector = errors.New("policy: found redundant selector")
// errUnnamedSelector is returned for errors found by selectors policy validator.
errUnnamedSelector = errors.New("policy: unnamed selectors are useless, " +
"make sure to pair REP and SELECT clauses: \"REP .. IN X\" + \"SELECT ... AS X\"")
// errRedundantSelector is returned for errors found by filters policy validator.
errRedundantFilter = errors.New("policy: found redundant filter")
) )
type policyVisitor struct { type policyVisitor struct {
@ -846,24 +853,50 @@ func (p *policyVisitor) VisitExpr(ctx *parser.ExprContext) any {
// being actually defined in FILTER section. // being actually defined in FILTER section.
func validatePolicy(p PlacementPolicy) error { func validatePolicy(p PlacementPolicy) error {
seenFilters := map[string]bool{} seenFilters := map[string]bool{}
expectedFilters := map[string]struct{}{}
for i := range p.filters { for i := range p.filters {
seenFilters[p.filters[i].GetName()] = true seenFilters[p.filters[i].GetName()] = true
} for _, f := range p.filters[i].GetFilters() {
if f.GetName() != "" {
seenSelectors := map[string]bool{} expectedFilters[f.GetName()] = struct{}{}
}
for i := range p.selectors {
if flt := p.selectors[i].GetFilter(); flt != mainFilterName && !seenFilters[flt] {
return fmt.Errorf("%w: '%s'", errUnknownFilter, flt)
} }
seenSelectors[p.selectors[i].GetName()] = true
} }
seenSelectors := map[string]*netmap.Selector{}
for i := range p.selectors {
if p.selectors[i].GetName() == "" {
return errUnnamedSelector
}
if flt := p.selectors[i].GetFilter(); flt != mainFilterName {
expectedFilters[flt] = struct{}{}
if !seenFilters[flt] {
return fmt.Errorf("%w: '%s'", errUnknownFilter, flt)
}
}
seenSelectors[p.selectors[i].GetName()] = &p.selectors[i]
}
for _, f := range p.filters {
if _, ok := expectedFilters[f.GetName()]; !ok {
return fmt.Errorf("%w: '%s'", errRedundantFilter, f.GetName())
}
}
expectedSelectors := map[string]struct{}{}
for i := range p.replicas { for i := range p.replicas {
if sel := p.replicas[i].GetSelector(); sel != "" && !seenSelectors[sel] { selName := p.replicas[i].GetSelector()
return fmt.Errorf("%w: '%s'", errUnknownSelector, sel) if selName != "" {
expectedSelectors[selName] = struct{}{}
if seenSelectors[selName] == nil {
return fmt.Errorf("%w: '%s'", errUnknownSelector, selName)
}
}
}
for _, s := range p.selectors {
if _, ok := expectedSelectors[s.GetName()]; !ok {
return fmt.Errorf("%w: to use selector '%s' use keyword IN", errRedundantSelector, s.GetName())
} }
} }

View file

@ -0,0 +1,61 @@
package netmap
import (
"strings"
"testing"
"github.com/stretchr/testify/require"
)
func TestDecodeString(t *testing.T) {
testCases := []string{
`REP 1 IN X
CBF 1
SELECT 2 IN SAME Location FROM * AS X`,
`REP 1 IN X
REP 2 IN Y
CBF 1
SELECT 2 FROM * AS X
SELECT 3 FROM * AS Y`,
`REP 1 IN X
SELECT 2 IN City FROM Good AS X
FILTER Country EQ RU AS FromRU
FILTER Country EQ EN AS FromEN
FILTER @FromRU AND @FromEN AND Rating GT 7 AS Good`,
`REP 7 IN SPB
SELECT 1 IN City FROM SPBSSD AS SPB
FILTER City EQ SPB AND SSD EQ true OR City EQ SPB AND Rating GE 5 AS SPBSSD`,
`REP 7 IN SPB
SELECT 1 IN City FROM SPBSSD AS SPB
FILTER NOT (NOT (City EQ SPB) AND SSD EQ true OR City EQ SPB AND Rating GE 5) AS SPBSSD`,
`UNIQUE
REP 1
REP 1`,
}
var p PlacementPolicy
for _, testCase := range testCases {
require.NoError(t, p.DecodeString(testCase), "unable parse %s", testCase)
var b strings.Builder
require.NoError(t, p.WriteStringTo(&b))
require.Equal(t, testCase, b.String())
}
invalidTestCases := map[string]error{
`?REP 1`: errSyntaxError,
`REP 1 trailing garbage`: errSyntaxError,
`REP 1 SELECT 4 FROM * `: errUnnamedSelector,
`REP 1 SELECT 4 FROM * AS X`: errRedundantSelector,
`REP 1 IN X REP 2 SELECT 4 FROM * AS X FILTER 'UN-LOCODE' EQ 'RU LED' AS F`: errRedundantFilter,
}
for i := range invalidTestCases {
require.ErrorIs(t, p.DecodeString(i), invalidTestCases[i], "#%s", i)
}
}

View file

@ -1,7 +1,6 @@
package netmap_test package netmap_test
import ( import (
"strings"
"testing" "testing"
. "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap" . "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap"
@ -9,55 +8,6 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
func TestEncode(t *testing.T) {
testCases := []string{
`REP 1 IN X
CBF 1
SELECT 2 IN SAME Location FROM * AS X`,
`REP 1
SELECT 2 IN City FROM Good
FILTER Country EQ RU AS FromRU
FILTER @FromRU AND Rating GT 7 AS Good`,
`REP 7 IN SPB
SELECT 1 IN City FROM SPBSSD AS SPB
FILTER City EQ SPB AND SSD EQ true OR City EQ SPB AND Rating GE 5 AS SPBSSD`,
`REP 7 IN SPB
SELECT 1 IN City FROM SPBSSD AS SPB
FILTER NOT (NOT (City EQ SPB) AND SSD EQ true OR City EQ SPB AND Rating GE 5) AS SPBSSD`,
`UNIQUE
REP 1
REP 1`,
`REP 1 IN X
SELECT 1 FROM F AS X
FILTER 'UN-LOCODE' EQ 'RU LED' AS F`,
}
var p PlacementPolicy
for _, testCase := range testCases {
require.NoError(t, p.DecodeString(testCase))
var b strings.Builder
require.NoError(t, p.WriteStringTo(&b))
require.Equal(t, testCase, b.String())
}
invalidTestCases := []string{
`?REP 1`,
`REP 1 trailing garbage`,
}
for i := range invalidTestCases {
require.Error(t, p.DecodeString(invalidTestCases[i]), "#%d", i)
}
}
func TestPlacementPolicyEncoding(t *testing.T) { func TestPlacementPolicyEncoding(t *testing.T) {
v := netmaptest.PlacementPolicy() v := netmaptest.PlacementPolicy()