Skip to content

Commit

Permalink
Improve matchLiteral further and add some more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
kozlovic committed Aug 17, 2017
1 parent 42b27d2 commit 0cc49ec
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 36 deletions.
72 changes: 45 additions & 27 deletions server/sublist.go
Expand Up @@ -615,42 +615,60 @@ func matchLiteral(literal, subject string) bool {
if li >= ll {
return false
}
b := subject[i]
switch b {
// This function has been optimized for speed.
// For instance, do not set b:=subject[i] here since
// we may bump `i` in this loop to avoid `continue` or
// skiping common test in a particular test.
// Run Benchmark_SublistMatchLiteral before making any change.
switch subject[i] {
case pwc:
// NOTE: This is not testing validity of a subject, instead ensures
// that wildcards are treated as such if they follow some basic rules.
// For `*` it means:
// only character in the string
// or first character and followed by a `.`
// or last character and preceded by a `.`
// or preceded and followed by a `.`
if (i == 0 || subject[i-1] == btsep) && (i == ls-1 || subject[i+1] == btsep) {
// Skip token in literal
for {
if li >= ll || literal[li] == btsep {
li--
break
// that wildcards are treated as such if they follow some basic rules,
// namely that they are a token on their own.
if i == 0 || subject[i-1] == btsep {
if i == ls-1 {
// There is no more token in the subject after this wildcard.
// Skip token in literal and expect to not find a separator.
for {
// End of literal, this is a match.
if li >= ll {
return true
}
// Presence of separator, this can't be a match.
if literal[li] == btsep {
return false
}
li++
}
li++
} else if subject[i+1] == btsep {
// There is another token in the subject after this wildcard.
// Skip token in literal and expect to get a separator.
for {
// We found the end of the literal before finding a separator,
// this can't be a match.
if li >= ll {
return false
}
if literal[li] == btsep {
break
}
li++
}
// Bump `i` since we know there is a `.` following, we are
// safe. The common test below is going to check `.` with `.`
// which is good. A `continue` here is too costly.
i++
}
} else if b != literal[li] {
return false
}
case fwc:
// For `>` to be a wildcard, it means:
// only character in the string
// or last character preceded by `.`
// For `>` to be a wildcard, it means being the only or last character
// in the string preceded by a `.`
if (i == 0 || subject[i-1] == btsep) && i == ls-1 {
return true
}
if b != literal[li] {
return false
}
default:
if b != literal[li] {
return false
}
}
if subject[i] != literal[li] {
return false
}
li++
}
Expand Down
20 changes: 11 additions & 9 deletions server/sublist_test.go
Expand Up @@ -428,18 +428,20 @@ func TestSublistMatchLiterals(t *testing.T) {
checkBool(matchLiteral("foo.bar", "foo"), false, t)
checkBool(matchLiteral("stats.test.foos", "stats.test.foos"), true, t)
checkBool(matchLiteral("stats.test.foos", "stats.test.foo"), false, t)
checkBool(matchLiteral("stats.test", "stats.test.*"), false, t)
checkBool(matchLiteral("stats.test.foos", "stats.*"), false, t)
checkBool(matchLiteral("stats.test.foos", "stats.*.*.foos"), false, t)

// These are cases where wildcards characters should not be considered
// wildcards since they do not follow the rules of wildcards.
checkBool(matchLiteral("*bar", "*.*"), false, t)
checkBool(matchLiteral("*bar", "*.>"), false, t)
checkBool(matchLiteral("*bar", "*bar"), true, t) // match literally
checkBool(matchLiteral("foo*", "foo.*"), false, t)
checkBool(matchLiteral("foo*", "foo.>"), false, t)
checkBool(matchLiteral("foo*", "foo*"), true, t) // match literally
checkBool(matchLiteral("foo*bar", "foo.*.bar"), false, t)
checkBool(matchLiteral("foo*bar", "foo.>"), false, t)
checkBool(matchLiteral("foo*bar", "foo*bar"), true, t) // match literally
checkBool(matchLiteral("*bar", "*bar"), true, t)
checkBool(matchLiteral("foo*", "foo*"), true, t)
checkBool(matchLiteral("foo*bar", "foo*bar"), true, t)
checkBool(matchLiteral("foo.***.bar", "foo.***.bar"), true, t)
checkBool(matchLiteral(">bar", ">bar"), true, t)
checkBool(matchLiteral("foo>", "foo>"), true, t)
checkBool(matchLiteral("foo>bar", "foo>bar"), true, t)
checkBool(matchLiteral("foo.>>>.bar", "foo.>>>.bar"), true, t)
}

func TestSublistBadSubjectOnRemove(t *testing.T) {
Expand Down

0 comments on commit 0cc49ec

Please sign in to comment.