Merge pull request #38 from nspcc-dev/fix/invalid_filter

Assume filter failed if bucket is non-existent

Author: fyrchik

policy.go

@@ -227,17 +227,34 @@ func (b Bucket) IsValid() bool {
 	return len(nodes) == len(ns)
 }
 
-func (b Bucket) findForbidden(fs []Filter) (forbidden Nodes) {
-	// if root does not satisfy any filter it must be forbidden
-	for _, f := range fs {
-		if b.Key == f.Key && !f.Check(b) {
-			return b.nodes
+func (b Bucket) findAllowed(fs []Filter) (nodes Nodes) {
+	nodes = b.nodes
+
+	for i := range fs {
+		var allowed Nodes
+		for _, c := range b.findKey(fs[i].Key) {
+			if fs[i].F.Check(c.Value) {
+				allowed = append(allowed, c.nodes...)
+			}
 		}
+
+		sort.Sort(allowed)
+		nodes = intersect(nodes, allowed)
 	}
 
-	for _, c := range b.children {
-		forbidden = union(forbidden, c.findForbidden(fs))
+	return
+}
+
+func (b *Bucket) findKey(key string) (bs []*Bucket) {
+	if b.Key == key {
+		bs = append(bs, b)
+		return
 	}
+
+	for i := range b.children {
+		bs = append(bs, b.children[i].findKey(key)...)
+	}
+
 	return
 }
 
@@ -329,15 +346,15 @@ func (b Bucket) getMaxSelectionC(ss []Select, filter FilterFunc, cut bool) (*Buc
 // any other subgraph satisfying specified selects and filters.
 func (b Bucket) GetMaxSelection(s SFGroup) (r *Bucket) {
 	var (
-		forbidden = b.findForbidden(s.Filters)
-		excludes  = make(map[uint32]struct{}, len(forbidden)+len(s.Exclude))
+		allowed  = b.findAllowed(s.Filters)
+		excludes = make(map[uint32]bool, len(s.Exclude))
 	)
 
-	for _, c := range forbidden {
-		excludes[c.N] = struct{}{}
+	for _, c := range allowed {
+		excludes[c.N] = false
 	}
 	for _, c := range s.Exclude {
-		excludes[c] = struct{}{}
+		excludes[c] = true
 	}
 
 	r, _ = b.getMaxSelection(s.Selectors, func(nodes Nodes) Nodes {

policy_test.go

@@ -88,6 +88,23 @@ func TestBucket_RuntimeError(t *testing.T) {
 		require.NotEqual(t, r2.nodes, r3.nodes)
 	})
 
+	t.Run("non-existing bucket", func(t *testing.T) {
+		buckets := []bucket{
+			{"/Location:Europe/Country:Spain/City:Madrid", []uint32{17, 18}},
+			{"/Location:Europe/Country:Spain/City:Barcelona", []uint32{16, 19}},
+		}
+		root, err := newRoot(buckets...)
+		require.NoError(t, err)
+
+		ss := []Select{
+			{Key: "InvalidSelect", Count: 1},
+			{Key: NodesBucket, Count: 1},
+		}
+
+		r := root.GetSelection(ss, defaultPivot)
+		require.Nil(t, r)
+	})
+
 	t.Run("regression test", func(t *testing.T) {
 		buckets := []bucket{
 			{"/Location:Europe/Country:Spain/City:Madrid", []uint32{17, 18}},
@@ -367,6 +384,24 @@ func TestBucket_GetMaxSelection(t *testing.T) {
 	root, err = newRoot(buckets...)
 	require.NoError(t, err)
 
+	t.Run("invalid filter", func(t *testing.T) {
+		ss = []Select{{Key: "Country", Count: 1}}
+		fs = []Filter{{Key: "InvalidFilter", F: FilterIn("Germany", "Spain")}}
+		r = root.GetMaxSelection(SFGroup{Selectors: ss, Filters: fs})
+		require.Nil(t, r)
+	})
+
+	t.Run("match uniq attribute", func(t *testing.T) {
+		ss = []Select{{Key: NodesBucket, Count: 2}}
+		fs = []Filter{{Key: "City", F: FilterEQ("Madrid")}}
+		r = root.GetMaxSelection(SFGroup{Selectors: ss, Filters: fs})
+		require.NotNil(t, r)
+
+		ss = []Select{{Key: NodesBucket, Count: 3}}
+		r = root.GetMaxSelection(SFGroup{Selectors: ss, Filters: fs})
+		require.Nil(t, r)
+	})
+
 	buckets = []bucket{
 		{"/Location:Europe/Country:Germany/City:Berlin", []uint32{9, 10}},
 		{"/Location:Europe/Country:Germany/City:Hamburg", []uint32{25}},
@@ -466,7 +501,6 @@ func TestBucket_GetMaxSelection(t *testing.T) {
 	fs = []Filter{{Key: "Country", F: FilterEQ("Germany")}}
 	r = root.GetMaxSelection(SFGroup{Selectors: ss, Filters: fs})
 	require.Equal(t, &exp, r)
-
 }
 
 func TestNetMap_GetNodesByOption(t *testing.T) {

utils.go

@@ -48,10 +48,10 @@ func intersect(a, b Nodes) Nodes {
 	return c
 }
 
-func diff(a Nodes, b map[uint32]struct{}) (c Nodes) {
+func diff(a Nodes, exclude map[uint32]bool) (c Nodes) {
 	c = make(Nodes, 0, len(a))
 	for _, e := range a {
-		if _, ok := b[e.N]; !ok {
+		if excl, ok := exclude[e.N]; ok && !excl {
 			c = append(c, e)
 		}
 	}