From a53175949e6df33be2ccd8034bf54e999e94e50e Mon Sep 17 00:00:00 2001
From: Gyu-Ho Lee <gyuhox@gmail.com>
Date: Tue, 13 Dec 2016 15:42:12 -0800
Subject: [PATCH] auth: improve 'removeSubsetRangePerms' to O(n)

---
 auth/range_perm_cache.go      | 47 ++++++++++++++---------------------
 auth/range_perm_cache_test.go | 46 ++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 28 deletions(-)

diff --git a/auth/range_perm_cache.go b/auth/range_perm_cache.go
index 86461992b..3cd1ad2a4 100644
--- a/auth/range_perm_cache.go
+++ b/auth/range_perm_cache.go
@@ -49,38 +49,30 @@ func isRangeEqual(a, b *rangePerm) bool {
 
 // removeSubsetRangePerms removes any rangePerms that are subsets of other rangePerms.
 // If there are equal ranges, removeSubsetRangePerms only keeps one of them.
-func removeSubsetRangePerms(perms []*rangePerm) []*rangePerm {
-	// TODO(mitake): currently it is O(n^2), we need a better algorithm
-	var newp []*rangePerm
-
+// It returns a sorted rangePerm slice.
+func removeSubsetRangePerms(perms []*rangePerm) (newp []*rangePerm) {
+	sort.Sort(RangePermSliceByBegin(perms))
+	var prev *rangePerm
 	for i := range perms {
-		skip := false
-
-		for j := range perms {
-			if i == j {
-				continue
-			}
-
-			if isRangeEqual(perms[i], perms[j]) {
-				// if ranges are equal, we only keep the first range.
-				if i > j {
-					skip = true
-					break
-				}
-			} else if isSubset(perms[i], perms[j]) {
-				// if a range is a strict subset of the other one, we skip the subset.
-				skip = true
-				break
-			}
-		}
-
-		if skip {
+		if i == 0 {
+			prev = perms[i]
+			newp = append(newp, perms[i])
 			continue
 		}
-
+		if isRangeEqual(perms[i], prev) {
+			continue
+		}
+		if isSubset(perms[i], prev) {
+			continue
+		}
+		if isSubset(prev, perms[i]) {
+			prev = perms[i]
+			newp[len(newp)-1] = perms[i]
+			continue
+		}
+		prev = perms[i]
 		newp = append(newp, perms[i])
 	}
-
 	return newp
 }
 
@@ -88,7 +80,6 @@ func removeSubsetRangePerms(perms []*rangePerm) []*rangePerm {
 func mergeRangePerms(perms []*rangePerm) []*rangePerm {
 	var merged []*rangePerm
 	perms = removeSubsetRangePerms(perms)
-	sort.Sort(RangePermSliceByBegin(perms))
 
 	i := 0
 	for i < len(perms) {
diff --git a/auth/range_perm_cache_test.go b/auth/range_perm_cache_test.go
index 07a7b5c9c..22745158a 100644
--- a/auth/range_perm_cache_test.go
+++ b/auth/range_perm_cache_test.go
@@ -16,6 +16,8 @@ package auth
 
 import (
 	"bytes"
+	"fmt"
+	"reflect"
 	"testing"
 )
 
@@ -131,3 +133,47 @@ func TestGetMergedPerms(t *testing.T) {
 		}
 	}
 }
+
+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 printPerms(rs []*rangePerm) (txt string) {
+	for i, p := range rs {
+		if i != 0 {
+			txt += ","
+		}
+		txt += fmt.Sprintf("%+v", *p)
+	}
+	return
+}