Skip to content

Commit

Permalink
fix($urlMatcherFactory): make optional params regex grouping optional
Browse files Browse the repository at this point in the history
- Previously, the optional parameter's pattern itself was assumed to match 0-length strings
- That did not work if a regexp that was provided for a param required at least one character , ie, [A-Z0-9]{10,16}
- Now, we wrap the pattern group with a question mark if the parameter is optional (pattern)?

Closes #1576
  • Loading branch information
christopherthielen committed Nov 21, 2014
1 parent c3d543a commit 06f7379
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/urlMatcherFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,13 @@ function UrlMatcher(pattern, config, parentMatcher) {
return params[id];
}

function quoteRegExp(string, pattern, squash) {
function quoteRegExp(string, pattern, squash, optional) {
var surroundPattern = ['',''], result = string.replace(/[\\\[\]\^$*+?.()|{}]/g, "\\$&");
if (!pattern) return result;
switch(squash) {
case false: surroundPattern = ['(', ')']; break;
case false: surroundPattern = ['(', ')' + (optional ? "?" : "")]; break;
case true: surroundPattern = ['?(', ')?']; break;
default: surroundPattern = ['(' + squash + "|", ')?']; break;
default: surroundPattern = ['(' + squash + "|", ')?']; break;
}
return result + surroundPattern[0] + pattern + surroundPattern[1];
}
Expand All @@ -131,7 +131,7 @@ function UrlMatcher(pattern, config, parentMatcher) {
if (p.segment.indexOf('?') >= 0) break; // we're into the search part

param = addParameter(p.id, p.type, p.cfg, "path");
compiled += quoteRegExp(p.segment, param.type.pattern.source, param.squash);
compiled += quoteRegExp(p.segment, param.type.pattern.source, param.squash, param.isOptional);
segments.push(p.segment);
last = placeholder.lastIndex;
}
Expand Down
8 changes: 8 additions & 0 deletions test/urlMatcherFactorySpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,14 @@ describe("urlMatcherFactory", function () {
expect(m.exec('/users/bar/2')).toBeNull();
});

it("should populate even if the regexp requires 1 or more chars", function() {
var m = new UrlMatcher('/record/{appId}/{recordId:[0-9a-fA-F]{10,24}}', {
params: { appId: null, recordId: null }
});
expect(m.exec("/record/546a3e4dd273c60780e35df3/"))
.toEqual({ appId: "546a3e4dd273c60780e35df3", recordId: null });
});

it("should allow shorthand definitions", function() {
var m = new UrlMatcher('/foo/:foo', {
params: { foo: "bar" }
Expand Down

0 comments on commit 06f7379

Please sign in to comment.