Skip to content

Commit

Permalink
🐛 bug: fix route constraints problems
Browse files Browse the repository at this point in the history
  • Loading branch information
efectn committed Aug 19, 2022
1 parent 32d311c commit db0c28d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 43 deletions.
32 changes: 18 additions & 14 deletions helpers.go
Expand Up @@ -698,18 +698,22 @@ const (

// Route Constraints
const (
ConstraintInt = "int"
ConstraintBool = "bool"
ConstraintFloat = "float"
ConstraintAlpha = "alpha"
ConstraintGuid = "guid"
ConstraintMinLen = "minLen"
ConstraintMaxLen = "maxLen"
ConstraintExactLen = "exactLen"
ConstraintBetweenLen = "betweenLen"
ConstraintMin = "min"
ConstraintMax = "max"
ConstraintRange = "range"
ConstraintDatetime = "datetime"
ConstraintRegex = "regex"
ConstraintInt = "int"
ConstraintBool = "bool"
ConstraintFloat = "float"
ConstraintAlpha = "alpha"
ConstraintGuid = "guid"
ConstraintMinLen = "minLen"
ConstraintMaxLen = "maxLen"
ConstraintExactLen = "exactLen"
ConstraintBetweenLen = "betweenLen"
ConstraintMinLenLower = "minlen"
ConstraintMaxLenLower = "maxlen"
ConstraintExactLenLower = "exactlen"
ConstraintBetweenLenLower = "betweenlen"
ConstraintMin = "min"
ConstraintMax = "max"
ConstraintRange = "range"
ConstraintDatetime = "datetime"
ConstraintRegex = "regex"
)
27 changes: 21 additions & 6 deletions path.go
Expand Up @@ -261,7 +261,7 @@ func (routeParser *routeParser) analyseParameterPart(pattern string) (string, *r

if hasConstraint := (parameterConstraintStart != -1 && parameterConstraintEnd != -1); hasConstraint {
constraintString := pattern[parameterConstraintStart+1 : parameterConstraintEnd]
userconstraints := strings.Split(constraintString, string(parameterConstraintSeparatorChars))
userconstraints := splitNonEscaped(constraintString, string(parameterConstraintSeparatorChars))
constraints = make([]*Constraint, 0, len(userconstraints))

for _, c := range userconstraints {
Expand All @@ -272,7 +272,7 @@ func (routeParser *routeParser) analyseParameterPart(pattern string) (string, *r
if start != -1 && end != -1 {
constraint := &Constraint{
ID: getParamConstraintType(c[:start]),
Data: strings.Split(RemoveEscapeChar(c[start+1:end]), string(parameterConstraintDataSeparatorChars)),
Data: splitNonEscaped(RemoveEscapeChar(c[start+1:end]), string(parameterConstraintDataSeparatorChars)),
}

// Precompile regex if has regex constraint
Expand Down Expand Up @@ -384,6 +384,21 @@ func findNextNonEscapedCharsetPosition(search string, charset []byte) int {
return pos
}

// splitNonEscaped slices s into all substrings separated by sep and returns a slice of the substrings between those separators
// This function also takes a care of escape char when splitting.
func splitNonEscaped(s, sep string) []string {
var result []string
i := findNextNonEscapedCharsetPosition(s, []byte(sep))

for i > -1 {
result = append(result, s[:i])
s = s[i+len(sep):]
i = findNextNonEscapedCharsetPosition(s, []byte(sep))
}

return append(result, s)
}

// getMatch parses the passed url and tries to match it against the route segments and determine the parameter positions
func (routeParser *routeParser) getMatch(detectionPath, path string, params *[maxParams]string, partialCheck bool) bool {
var i, paramsIterator, partLen int
Expand Down Expand Up @@ -526,13 +541,13 @@ func getParamConstraintType(constraintPart string) TypeConstraint {
return alphaConstraint
case ConstraintGuid:
return guidConstraint
case ConstraintMinLen:
case ConstraintMinLen, ConstraintMinLenLower:
return minLenConstraint
case ConstraintMaxLen:
case ConstraintMaxLen, ConstraintMaxLenLower:
return maxLenConstraint
case ConstraintExactLen:
case ConstraintExactLen, ConstraintExactLenLower:
return exactLenConstraint
case ConstraintBetweenLen:
case ConstraintBetweenLen, ConstraintBetweenLenLower:
return betweenLenConstraint
case ConstraintMin:
return minConstraint
Expand Down
34 changes: 11 additions & 23 deletions path_test.go
Expand Up @@ -549,6 +549,12 @@ func Test_Path_matchParams(t *testing.T) {
{url: "/api/v1/25", params: []string{"25"}, match: true},
{url: "/api/v1/true", params: []string{"true"}, match: false},
})
testCase("/api/v1/:param<int\\;range(10,30)>", []testparams{
{url: "/api/v1/entity", params: []string{"entity"}, match: true},
{url: "/api/v1/87283827683", params: []string{"87283827683"}, match: true},
{url: "/api/v1/25", params: []string{"25"}, match: true},
{url: "/api/v1/true", params: []string{"true"}, match: true},
})
}

func Test_Utils_GetTrimmedParam(t *testing.T) {
Expand Down Expand Up @@ -755,28 +761,10 @@ func Benchmark_Path_matchParams(t *testing.B) {
{url: "/api/v1/25", params: []string{"25"}, match: true},
{url: "/api/v1/true", params: []string{"true"}, match: false},
})
}

func Test_Path_matchParams0(t *testing.T) {
t.Parallel()
type testparams struct {
url string
params []string
match bool
partialCheck bool
}
var ctxParams [maxParams]string
testCase := func(r string, cases []testparams) {
parser := parseRoute(r)
for _, c := range cases {
match := parser.getMatch(c.url, c.url, &ctxParams, c.partialCheck)
utils.AssertEqual(t, c.match, match, fmt.Sprintf("route: '%s', url: '%s'", r, c.url))
if match && len(c.params) > 0 {
utils.AssertEqual(t, c.params[0:len(c.params)], ctxParams[0:len(c.params)], fmt.Sprintf("route: '%s', url: '%s'", r, c.url))
}
}
}
testCase("/api/v1/:param<datetime(2006-01-02)>", []testparams{
{url: "/api/v1/2005-11-01", params: []string{"2005-11-01"}, match: true},
benchCase("/api/v1/:param<int\\;range(10,30)>", []testparams{
{url: "/api/v1/entity", params: []string{"entity"}, match: true},
{url: "/api/v1/87283827683", params: []string{"87283827683"}, match: true},
{url: "/api/v1/25", params: []string{"25"}, match: true},
{url: "/api/v1/true", params: []string{"true"}, match: true},
})
}

3 comments on commit db0c28d

@ReneWerner87
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: db0c28d Previous: 73d0b71 Ratio
Benchmark_Ctx_Protocol 16 ns/op 0 B/op 0 allocs/op 3.192 ns/op 0 B/op 0 allocs/op 5.01

This comment was automatically generated by workflow using github-action-benchmark.

@ReneWerner87
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: db0c28d Previous: be38feb Ratio
Benchmark_Ctx_Protocol 19.31 ns/op 0 B/op 0 allocs/op 2.43 ns/op 0 B/op 0 allocs/op 7.95
Benchmark_StatusMessage/default 17.24 ns/op 0 B/op 0 allocs/op 4.018 ns/op 0 B/op 0 allocs/op 4.29

This comment was automatically generated by workflow using github-action-benchmark.

@ReneWerner87
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: db0c28d Previous: cdccee7 Ratio
Benchmark_AcquireCtx 1691 ns/op 1568 B/op 5 allocs/op 566.5 ns/op 1440 B/op 5 allocs/op 2.98
Benchmark_Ctx_Protocol 19.93 ns/op 0 B/op 0 allocs/op 2.344 ns/op 0 B/op 0 allocs/op 8.50

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.