Merge pull request #7313 from gyuho/simplify-auth

auth: simplify merging range perm

auth/range_perm_cache.go

@@ -47,59 +47,53 @@ func isRangeEqual(a, b *rangePerm) bool {
 	return bytes.Equal(a.begin, b.begin) && bytes.Equal(a.end, b.end)
 }
 
-// removeSubsetRangePerms removes any rangePerms that are subsets of other rangePerms.
-// If there are equal ranges, removeSubsetRangePerms only keeps one of them.
-// It returns a sorted rangePerm slice.
-func removeSubsetRangePerms(perms []*rangePerm) (newp []*rangePerm) {
+// mergeRangePerms merges adjacent rangePerms.
+func mergeRangePerms(perms []*rangePerm) (rs []*rangePerm) {
+	if len(perms) < 2 {
+		return perms
+	}
 	sort.Sort(RangePermSliceByBegin(perms))
-	var prev *rangePerm
-	for i := range perms {
-		if i == 0 {
-			prev = perms[i]
-			newp = append(newp, perms[i])
+
+	// initialize with first rangePerm
+	rs = append(rs, perms[0])
+
+	// merge in-place from 2nd
+	for i := 1; i < len(perms); i++ {
+		rp := rs[len(rs)-1]
+
+		// skip duplicate range
+		if isRangeEqual(rp, perms[i]) {
 			continue
 		}
-		if isRangeEqual(perms[i], prev) {
+
+		// merge ["a", "") with ["a", "c")
+		if bytes.Equal(rp.begin, perms[i].begin) && len(rp.end) == 0 && len(perms[i].end) > 0 {
+			rp.end = perms[i].end
 			continue
 		}
-		if isSubset(perms[i], prev) {
+
+		// do not merge ["a", "b") with ["b", "")
+		if bytes.Equal(rp.end, perms[i].begin) && len(perms[i].end) == 0 {
+			rs = append(rs, perms[i])
 			continue
 		}
-		if isSubset(prev, perms[i]) {
-			prev = perms[i]
-			newp[len(newp)-1] = perms[i]
+
+		// rp.end < perms[i].begin; found beginning of next range
+		if bytes.Compare(rp.end, perms[i].begin) < 0 {
+			rs = append(rs, perms[i])
 			continue
 		}
-		prev = perms[i]
-		newp = append(newp, perms[i])
+
+		rp.end = getMax(rp.end, perms[i].end)
 	}
-	return newp
+	return
 }
 
-// mergeRangePerms merges adjacent rangePerms.
-func mergeRangePerms(perms []*rangePerm) []*rangePerm {
-	var merged []*rangePerm
-	perms = removeSubsetRangePerms(perms)
-
-	i := 0
-	for i < len(perms) {
-		begin, next := i, i
-		for next+1 < len(perms) && bytes.Compare(perms[next].end, perms[next+1].begin) >= 0 {
-			next++
-		}
-		// don't merge ["a", "b") with ["b", ""), because perms[next+1].end is empty.
-		if next != begin && len(perms[next].end) > 0 {
-			merged = append(merged, &rangePerm{begin: perms[begin].begin, end: perms[next].end})
-		} else {
-			merged = append(merged, perms[begin])
-			if next != begin {
-				merged = append(merged, perms[next])
-			}
-		}
-		i = next + 1
+func getMax(a, b []byte) []byte {
+	if bytes.Compare(a, b) > 0 {
+		return a
 	}
-
-	return merged
+	return b
 }
 
 func getMergedPerms(tx backend.BatchTx, userName string) *unifiedRangePermissions {

auth/range_perm_cache_test.go

@@ -17,7 +17,6 @@ package auth
 import (
 	"bytes"
 	"fmt"
-	"reflect"
 	"testing"
 )
 
@@ -44,136 +43,127 @@ func TestGetMergedPerms(t *testing.T) {
 		params []*rangePerm
 		want   []*rangePerm
 	}{
+		{ // subsets converge
+			[]*rangePerm{{[]byte{2}, []byte{3}}, {[]byte{2}, []byte{5}}, {[]byte{1}, []byte{4}}},
+			[]*rangePerm{{[]byte{1}, []byte{5}}},
+		},
+		{ // subsets converge
+			[]*rangePerm{{[]byte{0}, []byte{3}}, {[]byte{0}, []byte{1}}, {[]byte{2}, []byte{4}}, {[]byte{0}, []byte{2}}},
+			[]*rangePerm{{[]byte{0}, []byte{4}}},
+		},
+		{ // biggest range at the end
+			[]*rangePerm{{[]byte{2}, []byte{3}}, {[]byte{0}, []byte{2}}, {[]byte{1}, []byte{4}}, {[]byte{0}, []byte{5}}},
+			[]*rangePerm{{[]byte{0}, []byte{5}}},
+		},
+		{ // biggest range at the beginning
+			[]*rangePerm{{[]byte{0}, []byte{5}}, {[]byte{2}, []byte{3}}, {[]byte{0}, []byte{2}}, {[]byte{1}, []byte{4}}},
+			[]*rangePerm{{[]byte{0}, []byte{5}}},
+		},
+		{ // no overlapping ranges
+			[]*rangePerm{{[]byte{2}, []byte{3}}, {[]byte{0}, []byte{1}}, {[]byte{4}, []byte{7}}, {[]byte{8}, []byte{15}}},
+			[]*rangePerm{{[]byte{0}, []byte{1}}, {[]byte{2}, []byte{3}}, {[]byte{4}, []byte{7}}, {[]byte{8}, []byte{15}}},
+		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("b")}},
-			[]*rangePerm{{[]byte("a"), []byte("b")}},
+			[]*rangePerm{makePerm("00", "03"), makePerm("18", "19"), makePerm("02", "08"), makePerm("10", "12")},
+			[]*rangePerm{makePerm("00", "08"), makePerm("10", "12"), makePerm("18", "19")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("")}},
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("")}},
+			[]*rangePerm{makePerm("a", "b")},
+			[]*rangePerm{makePerm("a", "b")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("c")}},
-			[]*rangePerm{{[]byte("a"), []byte("c")}},
+			[]*rangePerm{makePerm("a", "b"), makePerm("b", "")},
+			[]*rangePerm{makePerm("a", "b"), makePerm("b", "")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("c")}, {[]byte("b"), []byte("d")}},
-			[]*rangePerm{{[]byte("a"), []byte("d")}},
+			[]*rangePerm{makePerm("a", "b"), makePerm("b", "c")},
+			[]*rangePerm{makePerm("a", "c")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("c")}, {[]byte("d"), []byte("e")}},
-			[]*rangePerm{{[]byte("a"), []byte("c")}, {[]byte("d"), []byte("e")}},
+			[]*rangePerm{makePerm("a", "c"), makePerm("b", "d")},
+			[]*rangePerm{makePerm("a", "d")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("e"), []byte("f")}},
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("e"), []byte("f")}},
+			[]*rangePerm{makePerm("a", "b"), makePerm("b", "c"), makePerm("d", "e")},
+			[]*rangePerm{makePerm("a", "c"), makePerm("d", "e")},
 		},
 		{
-			[]*rangePerm{{[]byte("e"), []byte("f")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("b")}},
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("e"), []byte("f")}},
+			[]*rangePerm{makePerm("a", "b"), makePerm("c", "d"), makePerm("e", "f")},
+			[]*rangePerm{makePerm("a", "b"), makePerm("c", "d"), makePerm("e", "f")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("z")}},
-			[]*rangePerm{{[]byte("a"), []byte("z")}},
+			[]*rangePerm{makePerm("e", "f"), makePerm("c", "d"), makePerm("a", "b")},
+			[]*rangePerm{makePerm("a", "b"), makePerm("c", "d"), makePerm("e", "f")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("z")}, {[]byte("1"), []byte("9")}},
-			[]*rangePerm{{[]byte("1"), []byte("9")}, {[]byte("a"), []byte("z")}},
+			[]*rangePerm{makePerm("a", "b"), makePerm("c", "d"), makePerm("a", "z")},
+			[]*rangePerm{makePerm("a", "z")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("c"), []byte("d")}, {[]byte("a"), []byte("z")}, {[]byte("1"), []byte("a")}},
-			[]*rangePerm{{[]byte("1"), []byte("z")}},
+			[]*rangePerm{makePerm("a", "b"), makePerm("c", "d"), makePerm("a", "z"), makePerm("1", "9")},
+			[]*rangePerm{makePerm("1", "9"), makePerm("a", "z")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("a"), []byte("z")}, {[]byte("5"), []byte("6")}, {[]byte("1"), []byte("9")}},
-			[]*rangePerm{{[]byte("1"), []byte("9")}, {[]byte("a"), []byte("z")}},
+			[]*rangePerm{makePerm("a", "b"), makePerm("c", "d"), makePerm("a", "z"), makePerm("1", "a")},
+			[]*rangePerm{makePerm("1", "z")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("b")}, {[]byte("b"), []byte("c")}, {[]byte("c"), []byte("d")}, {[]byte("d"), []byte("f")}, {[]byte("1"), []byte("9")}},
-			[]*rangePerm{{[]byte("1"), []byte("9")}, {[]byte("a"), []byte("f")}},
+			[]*rangePerm{makePerm("a", "b"), makePerm("a", "z"), makePerm("5", "6"), makePerm("1", "9")},
+			[]*rangePerm{makePerm("1", "9"), makePerm("a", "z")},
+		},
+		{
+			[]*rangePerm{makePerm("a", "b"), makePerm("b", "c"), makePerm("c", "d"), makePerm("b", "f"), makePerm("1", "9")},
+			[]*rangePerm{makePerm("1", "9"), makePerm("a", "f")},
 		},
 		// overlapping
 		{
-			[]*rangePerm{{[]byte("a"), []byte("f")}, {[]byte("b"), []byte("g")}},
-			[]*rangePerm{{[]byte("a"), []byte("g")}},
+			[]*rangePerm{makePerm("a", "f"), makePerm("b", "g")},
+			[]*rangePerm{makePerm("a", "g")},
 		},
 		// keys
 		{
-			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("")}},
-			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("")}},
+			[]*rangePerm{makePerm("a", ""), makePerm("b", "")},
+			[]*rangePerm{makePerm("a", ""), makePerm("b", "")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("a"), []byte("c")}},
-			[]*rangePerm{{[]byte("a"), []byte("c")}},
+			[]*rangePerm{makePerm("a", ""), makePerm("a", "c")},
+			[]*rangePerm{makePerm("a", "c")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("a"), []byte("c")}, {[]byte("b"), []byte("")}},
-			[]*rangePerm{{[]byte("a"), []byte("c")}},
+			[]*rangePerm{makePerm("a", ""), makePerm("a", "c"), makePerm("b", "")},
+			[]*rangePerm{makePerm("a", "c")},
 		},
 		{
-			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("c")}, {[]byte("b"), []byte("")}, {[]byte("c"), []byte("")}, {[]byte("d"), []byte("")}},
-			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("b"), []byte("c")}, {[]byte("c"), []byte("")}, {[]byte("d"), []byte("")}},
+			[]*rangePerm{makePerm("a", ""), makePerm("b", "c"), makePerm("b", ""), makePerm("c", ""), makePerm("d", "")},
+			[]*rangePerm{makePerm("a", ""), makePerm("b", "c"), makePerm("c", ""), makePerm("d", "")},
 		},
 		// duplicate ranges
 		{
-			[]*rangePerm{{[]byte("a"), []byte("f")}, {[]byte("a"), []byte("f")}},
-			[]*rangePerm{{[]byte("a"), []byte("f")}},
+			[]*rangePerm{makePerm("a", "f"), makePerm("a", "f")},
+			[]*rangePerm{makePerm("a", "f")},
 		},
 		// duplicate keys
 		{
-			[]*rangePerm{{[]byte("a"), []byte("")}, {[]byte("a"), []byte("")}, {[]byte("a"), []byte("")}},
-			[]*rangePerm{{[]byte("a"), []byte("")}},
+			[]*rangePerm{makePerm("a", ""), makePerm("a", ""), makePerm("a", "")},
+			[]*rangePerm{makePerm("a", "")},
 		},
 	}
 
 	for i, tt := range tests {
 		result := mergeRangePerms(tt.params)
 		if !isPermsEqual(result, tt.want) {
-			t.Errorf("#%d: result=%q, want=%q", i, result, tt.want)
+			t.Errorf("#%d: result=%q, want=%q", i, sprintPerms(result), sprintPerms(tt.want))
 		}
 	}
 }
 
-func TestRemoveSubsetRangePerms(t *testing.T) {
-	tests := []struct {
-		perms  []*rangePerm
-		expect []*rangePerm
-	}{
-		{ // subsets converge
-			[]*rangePerm{{[]byte{2}, []byte{3}}, {[]byte{2}, []byte{5}}, {[]byte{1}, []byte{4}}},
-			[]*rangePerm{{[]byte{1}, []byte{4}}, {[]byte{2}, []byte{5}}},
-		},
-		{ // subsets converge
-			[]*rangePerm{{[]byte{0}, []byte{3}}, {[]byte{0}, []byte{1}}, {[]byte{2}, []byte{4}}, {[]byte{0}, []byte{2}}},
-			[]*rangePerm{{[]byte{0}, []byte{3}}, {[]byte{2}, []byte{4}}},
-		},
-		{ // biggest range at the end
-			[]*rangePerm{{[]byte{2}, []byte{3}}, {[]byte{0}, []byte{2}}, {[]byte{1}, []byte{4}}, {[]byte{0}, []byte{5}}},
-			[]*rangePerm{{[]byte{0}, []byte{5}}},
-		},
-		{ // biggest range at the beginning
-			[]*rangePerm{{[]byte{0}, []byte{5}}, {[]byte{2}, []byte{3}}, {[]byte{0}, []byte{2}}, {[]byte{1}, []byte{4}}},
-			[]*rangePerm{{[]byte{0}, []byte{5}}},
-		},
-		{ // no overlapping ranges
-			[]*rangePerm{{[]byte{2}, []byte{3}}, {[]byte{0}, []byte{1}}, {[]byte{4}, []byte{7}}, {[]byte{8}, []byte{15}}},
-			[]*rangePerm{{[]byte{0}, []byte{1}}, {[]byte{2}, []byte{3}}, {[]byte{4}, []byte{7}}, {[]byte{8}, []byte{15}}},
-		},
-	}
-	for i, tt := range tests {
-		rs := removeSubsetRangePerms(tt.perms)
-		if !reflect.DeepEqual(rs, tt.expect) {
-			t.Fatalf("#%d: unexpected rangePerms %q, got %q", i, printPerms(rs), printPerms(tt.expect))
-		}
-	}
+func makePerm(a, b string) *rangePerm {
+	return &rangePerm{begin: []byte(a), end: []byte(b)}
 }
 
-func printPerms(rs []*rangePerm) (txt string) {
-	for i, p := range rs {
-		if i != 0 {
-			txt += ","
-		}
-		txt += fmt.Sprintf("%+v", *p)
+func sprintPerms(rs []*rangePerm) (s string) {
+	for _, rp := range rs {
+		s += fmt.Sprintf("[%s %s] ", rp.begin, rp.end)
 	}
 	return
 }