Merge pull request #47 from gengo/fix/23-deep-wildcard

Consider tail segments after deep wildcard
grpc-ecosystem · Sep 2, 2015 · 54c3357 · 54c3357
2 parents 485354f + 960d05f
commit 54c3357
Show file tree

Hide file tree

Showing 2 changed files with 113 additions and 12 deletions.
diff --git a/runtime/pattern.go b/runtime/pattern.go
@@ -31,6 +31,8 @@ type Pattern struct {
 	vars []string
 	// stacksize is the max depth of the stack
 	stacksize int
+	// tailLen is the length of the fixed-size segments after a deep wildcard
+	tailLen int
 	// verb is the VERB part of the path pattern. It is empty if the pattern does not have VERB part.
 	verb string
 }
@@ -52,21 +54,38 @@ func NewPattern(version int, ops []int, pool []string, verb string) (Pattern, er
 		return Pattern{}, ErrInvalidPattern
 	}
 
-	var typedOps []op
-	var stack, maxstack int
-	var vars []string
+	var (
+		typedOps        []op
+		stack, maxstack int
+		tailLen         int
+		pushMSeen       bool
+		vars            []string
+	)
 	for i := 0; i < l; i += 2 {
 		op := op{code: utilities.OpCode(ops[i]), operand: ops[i+1]}
 		switch op.code {
 		case utilities.OpNop:
 			continue
-		case utilities.OpPush, utilities.OpPushM:
+		case utilities.OpPush:
+			if pushMSeen {
+				tailLen++
+			}
+			stack++
+		case utilities.OpPushM:
+			if pushMSeen {
+				glog.V(2).Info("pushM appears twice")
+				return Pattern{}, ErrInvalidPattern
+			}
+			pushMSeen = true
 			stack++
 		case utilities.OpLitPush:
 			if op.operand < 0 || len(pool) <= op.operand {
 				glog.V(2).Infof("negative literal index: %d", op.operand)
 				return Pattern{}, ErrInvalidPattern
 			}
+			if pushMSeen {
+				tailLen++
+			}
 			stack++
 		case utilities.OpConcatN:
 			if op.operand <= 0 {
@@ -108,6 +127,7 @@ func NewPattern(version int, ops []int, pool []string, verb string) (Pattern, er
 		pool:      pool,
 		vars:      vars,
 		stacksize: maxstack,
+		tailLen:   tailLen,
 		verb:      verb,
 	}, nil
 }
@@ -153,8 +173,13 @@ func (p Pattern) Match(components []string, verb string) (map[string]string, err
 			stack = append(stack, c)
 			pos++
 		case utilities.OpPushM:
-			stack = append(stack, strings.Join(components[pos:], "/"))
-			pos = len(components)
+			end := len(components)
+			if end < pos+p.tailLen {
+				return nil, ErrNotMatch
+			}
+			end -= p.tailLen
+			stack = append(stack, strings.Join(components[pos:end], "/"))
+			pos = end
 		case utilities.OpConcatN:
 			n := op.operand
 			l := len(stack) - n

diff --git a/runtime/pattern_test.go b/runtime/pattern_test.go
@@ -20,32 +20,37 @@ func TestNewPattern(t *testing.T) {
 		pool []string
 		verb string
 
-		stackSizeWant int
+		stackSizeWant, tailLenWant int
 	}{
 		{},
 		{
 			ops:           []int{int(utilities.OpNop), anything},
 			stackSizeWant: 0,
+			tailLenWant:   0,
 		},
 		{
 			ops:           []int{int(utilities.OpPush), anything},
 			stackSizeWant: 1,
+			tailLenWant:   0,
 		},
 		{
 			ops:           []int{int(utilities.OpLitPush), 0},
 			pool:          []string{"abc"},
 			stackSizeWant: 1,
+			tailLenWant:   0,
 		},
 		{
 			ops:           []int{int(utilities.OpPushM), anything},
 			stackSizeWant: 1,
+			tailLenWant:   0,
 		},
 		{
 			ops: []int{
 				int(utilities.OpPush), anything,
 				int(utilities.OpConcatN), 1,
 			},
 			stackSizeWant: 1,
+			tailLenWant:   0,
 		},
 		{
 			ops: []int{
@@ -55,6 +60,7 @@ func TestNewPattern(t *testing.T) {
 			},
 			pool:          []string{"abc"},
 			stackSizeWant: 1,
+			tailLenWant:   0,
 		},
 		{
 			ops: []int{
@@ -67,12 +73,40 @@ func TestNewPattern(t *testing.T) {
 			},
 			pool:          []string{"lit1", "lit2", "var1"},
 			stackSizeWant: 4,
+			tailLenWant:   0,
+		},
+		{
+			ops: []int{
+				int(utilities.OpPushM), anything,
+				int(utilities.OpConcatN), 1,
+				int(utilities.OpCapture), 2,
+				int(utilities.OpLitPush), 0,
+				int(utilities.OpLitPush), 1,
+			},
+			pool:          []string{"lit1", "lit2", "var1"},
+			stackSizeWant: 2,
+			tailLenWant:   2,
+		},
+		{
+			ops: []int{
+				int(utilities.OpLitPush), 0,
+				int(utilities.OpLitPush), 1,
+				int(utilities.OpPushM), anything,
+				int(utilities.OpLitPush), 2,
+				int(utilities.OpConcatN), 3,
+				int(utilities.OpLitPush), 3,
+				int(utilities.OpCapture), 4,
+			},
+			pool:          []string{"lit1", "lit2", "lit3", "lit4", "var1"},
+			stackSizeWant: 4,
+			tailLenWant:   2,
 		},
 		{
 			ops:           []int{int(utilities.OpLitPush), 0},
 			pool:          []string{"abc"},
-			stackSizeWant: 1,
 			verb:          "LOCK",
+			stackSizeWant: 1,
+			tailLenWant:   0,
 		},
 	} {
 		pat, err := NewPattern(validVersion, spec.ops, spec.pool, spec.verb)
@@ -83,6 +117,9 @@ func TestNewPattern(t *testing.T) {
 		if got, want := pat.stacksize, spec.stackSizeWant; got != want {
 			t.Errorf("pat.stacksize = %d; want %d", got, want)
 		}
+		if got, want := pat.tailLen, spec.tailLenWant; got != want {
+			t.Errorf("pat.stacksize = %d; want %d", got, want)
+		}
 	}
 }
 
@@ -129,6 +166,15 @@ func TestNewPatternWithWrongOp(t *testing.T) {
 			ops:  []int{int(utilities.OpCapture), 1},
 			pool: []string{"abc"},
 		},
+		{
+			// pushM appears twice
+			ops: []int{
+				int(utilities.OpPushM), anything,
+				int(utilities.OpLitPush), 0,
+				int(utilities.OpPushM), anything,
+			},
+			pool: []string{"abc"},
+		},
 	} {
 		_, err := NewPattern(validVersion, spec.ops, spec.pool, spec.verb)
 		if err == nil {
@@ -201,6 +247,18 @@ func TestMatch(t *testing.T) {
 			ops:   []int{int(utilities.OpPushM), anything},
 			match: []string{"", "abc", "abc/def", "abc/def/ghi"},
 		},
+		{
+			ops: []int{
+				int(utilities.OpPushM), anything,
+				int(utilities.OpLitPush), 0,
+			},
+			pool:  []string{"tail"},
+			match: []string{"tail", "abc/tail", "abc/def/tail"},
+			notMatch: []string{
+				"", "abc", "abc/def",
+				"tail/extra", "abc/tail/extra", "abc/def/tail/extra",
+			},
+		},
 		{
 			ops: []int{
 				int(utilities.OpLitPush), 0,
@@ -384,6 +442,22 @@ func TestMatchWithBinding(t *testing.T) {
 				"name": "o/my-bucket/dir/dir2/obj",
 			},
 		},
+		{
+			ops: []int{
+				int(utilities.OpLitPush), 0,
+				int(utilities.OpLitPush), 1,
+				int(utilities.OpPushM), anything,
+				int(utilities.OpLitPush), 2,
+				int(utilities.OpConcatN), 3,
+				int(utilities.OpCapture), 4,
+				int(utilities.OpLitPush), 3,
+			},
+			pool: []string{"v1", "o", ".ext", "tail", "name"},
+			path: "v1/o/my-bucket/dir/dir2/obj/.ext/tail",
+			want: map[string]string{
+				"name": "o/my-bucket/dir/dir2/obj/.ext",
+			},
+		},
 		{
 			ops: []int{
 				int(utilities.OpLitPush), 0,
@@ -485,11 +559,13 @@ func TestPatternString(t *testing.T) {
 				int(utilities.OpCapture), 2,
 				int(utilities.OpLitPush), 3,
 				int(utilities.OpPushM), anything,
-				int(utilities.OpConcatN), 2,
-				int(utilities.OpCapture), 4,
+				int(utilities.OpLitPush), 4,
+				int(utilities.OpConcatN), 3,
+				int(utilities.OpCapture), 6,
+				int(utilities.OpLitPush), 5,
 			},
-			pool: []string{"v1", "buckets", "bucket_name", "objects", "name"},
-			want: "/v1/{bucket_name=buckets/*}/{name=objects/**}",
+			pool: []string{"v1", "buckets", "bucket_name", "objects", ".ext", "tail", "name"},
+			want: "/v1/{bucket_name=buckets/*}/{name=objects/**/.ext}/tail",
 		},
 	} {
 		p, err := NewPattern(validVersion, spec.ops, spec.pool, "")