Skip to content

Commit

Permalink
fix($urlMatcherFactory): no longer generate unroutable urls
Browse files Browse the repository at this point in the history 
Previously, urlMatcherFactory would squash default param values and any double slashes from generated urls (by default).  This behaviour caused urls generated with default values in them to not route back to the state that generated them.  Change the default behaviour to squash only the param-value and not the URLs.

Closes #1487

feat($urlMatcherFactory): configurable url param squashing

Allow configuration of the squashing behavior of default-url-parameters on a per-param basis with reasonable default setting.
  • Loading branch information
@christopherthielen
christopherthielen committed Nov 8, 2014
1 parent 725cda6 commit cb9fd9d
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 47 deletions.
100 changes: 76 additions & 24 deletions src/urlMatcherFactory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,15 @@ function UrlMatcher(pattern, config) {
return params[id];
}

function quoteRegExp(string, pattern, isOptional) {
var result = string.replace(/[\\\[\]\^$*+?.()|{}]/g, "\\$&");
function quoteRegExp(string, pattern, squashPolicy) {
var flags = ['',''], result = string.replace(/[\\\[\]\^$*+?.()|{}]/g, "\\$&");
if (!pattern) return result;
var flag = isOptional ? '?' : '';
return result + flag + '(' + pattern + ')' + flag;
switch(squashPolicy) {
case "nosquash": flags = ['', '']; break;
case "value": flags = ['', '?']; break;
case "slash": flags = ['?', '?']; break;
}
return result + flags[0] + '(' + pattern + ')' + flags[1];
}

this.source = pattern;
Expand All @@ -122,7 +126,7 @@ function UrlMatcher(pattern, config) {
if (p.segment.indexOf('?') >= 0) break; // we're into the search part

param = addParameter(p.id, p.type, p.cfg);
compiled += quoteRegExp(p.segment, param.type.pattern.source, param.isOptional);
compiled += quoteRegExp(p.segment, param.type.pattern.source, param.squash);
segments.push(p.segment);
last = placeholder.lastIndex;
}
Expand Down Expand Up @@ -290,37 +294,47 @@ UrlMatcher.prototype.validates = function (params) {
*/
UrlMatcher.prototype.format = function (values) {
var segments = this.segments, params = this.parameters();

if (!values) return segments.join('').replace('//', '/');
var paramset = this.params;
values = values || {};

var nPath = segments.length - 1, nTotal = params.length,
result = segments[0], i, search, value, param, cfg, array;
result = segments[0], i, search, value, name, param, array, isDefaultValue;

if (!this.validates(values)) return null;

for (i = 0; i < nPath; i++) {
param = params[i];
value = values[param];
cfg = this.params[param];

if (!isDefined(value) && (segments[i] === '/' && segments[i + 1] === '/')) continue;
if (value != null) result += encodeURIComponent(cfg.type.encode(value));
result += segments[i + 1];
name = params[i];
param = paramset[name];
value = param.value(values[name]);
isDefaultValue = param.isOptional && param.type.equals(param.value(), value);
var squash = isDefaultValue ? param.squash : "nosquash";
var encoded = param.type.encode(value);

var nextSegment = segments[i + 1];
if (squash === "nosquash") {
if (encoded != null) result += encodeURIComponent(encoded);
result += nextSegment;
} else if (squash === "value") {
result += nextSegment;
} else if (squash === "slash") {
var capture = result.match(/\/$/) ? /\/?(.*)/ : /(.*)/;
result += nextSegment.match(capture)[1];
}
}

for (/**/; i < nTotal; i++) {
param = params[i];
value = values[param];
name = params[i];
value = values[name];
if (value == null) continue;
array = isArray(value);

if (array) {
value = value.map(encodeURIComponent).join('&' + param + '=');
value = value.map(encodeURIComponent).join('&' + name + '=');
}
result += (search ? '&' : '?') + param + '=' + (array ? value : encodeURIComponent(value));
result += (search ? '&' : '?') + name + '=' + (array ? value : encodeURIComponent(value));
search = true;
}
return result.replace('//', '/');
return result;
};

/**
Expand Down Expand Up @@ -446,9 +460,9 @@ Type.prototype.pattern = /.*/;
function $UrlMatcherFactory() {
$$UMFP = this;

var isCaseInsensitive = false, isStrictMode = true;
var isCaseInsensitive = false, isStrictMode = true, defaultSquashPolicy = "nosquash";

function safeString(val) { return isDefined(val) ? val.toString() : val; }
function safeString(val) { return val != null ? val.toString() : val; }
function coerceEquals(left, right) { return left == right; }
function angularEquals(left, right) { return angular.equals(left, right); }
// TODO: function regexpMatches(val) { return isDefined(val) && this.pattern.test(val); }
Expand Down Expand Up @@ -569,6 +583,28 @@ function $UrlMatcherFactory() {
isStrictMode = value;
};

/**
* @ngdoc function
* @name ui.router.util.$urlMatcherFactory#defaultSquashPolicy
* @methodOf ui.router.util.$urlMatcherFactory
*
* @description
* Sets the default behavior when generating or matching URLs with default parameter values.
*
* @param {string} value A string that defines the default parameter URL squashing behavior.
* `nosquash`: When generating an href with a default parameter value, do not squash the parameter value from the URL
* `value`: When generating an href with a default parameter value, squash (remove) the parameter value from the URL
* `slash`: When generating an href with a default parameter value, squash (remove) the parameter value, and, if the
* parameter is surrounded by slashes, squash (remove) one slash from the URL
*/
this.defaultSquashPolicy = function(value) {
if (!value) return defaultSquashPolicy;
if (value !== "nosquash" && value !== "value" && value !== "slash")
throw new Error("Invalid squash policy: " + value + ". Valid policies: 'nosquash', 'value', 'slash'");
defaultSquashPolicy = value;
return value;
};

/**
* @ngdoc function
* @name ui.router.util.$urlMatcherFactory#compile
Expand Down Expand Up @@ -758,10 +794,12 @@ function $UrlMatcherFactory() {
var defaultValueConfig = getDefaultValueConfig(config);
config = config || {};
type = getType(config, type);
var isOptional = defaultValueConfig.value !== undefined;
var squash = getSquashPolicy(config, isOptional);

function getDefaultValueConfig(config) {
var keys = isObject(config) ? objectKeys(config) : [];
var isShorthand = keys.indexOf("value") === -1 && keys.indexOf("type") === -1;
var isShorthand = keys.indexOf("value") === -1 && keys.indexOf("type") === -1 && keys.indexOf("squash") === -1;
var configValue = isShorthand ? config : config.value;
return {
fn: isInjectable(configValue) ? configValue : function () { return configValue; },
Expand All @@ -776,6 +814,19 @@ function $UrlMatcherFactory() {
return config.type instanceof Type ? config.type : new Type(config.type);
}

/**
* returns "nosquash", "value", "slash" to indicate the "default parameter url squash policy".
* undefined aliases to urlMatcherFactory default. `false` aliases to "nosquash". `true` aliases to "slash".
*/
function getSquashPolicy(config, isOptional) {
var squash = config.squash;
if (!isOptional || squash === false) return "nosquash";
if (!isDefined(squash)) return defaultSquashPolicy;
if (squash === true) return "slash";
if (squash === "nosquash" || squash === "value" || squash === "slash") return squash;
throw new Error("Invalid squash policy: '" + squash + "'. Valid policies: 'nosquash' (false), 'value', 'slash' (true)");
}

/**
* [Internal] Get the default value of a parameter, which may be an injectable function.
*/
Expand All @@ -796,8 +847,9 @@ function $UrlMatcherFactory() {
id: id,
type: type,
config: config,
squash: squash,
dynamic: undefined,
isOptional: defaultValueConfig.value !== undefined,
isOptional: isOptional,
value: $value
});
};
Expand Down
44 changes: 21 additions & 23 deletions test/urlMatcherFactorySpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ describe("urlMatcherFactory", function () {
describe("optional parameters", function() {
it("should match with or without values", function () {
var m = new UrlMatcher('/users/{id:int}', {
params: { id: { value: null } }
params: { id: { value: null, squash: true } }
});
expect(m.exec('/users/1138')).toEqual({ id: 1138 });
expect(m.exec('/users/').id).toBeNull();
Expand All @@ -289,7 +289,7 @@ describe("urlMatcherFactory", function () {

it("should correctly match multiple", function() {
var m = new UrlMatcher('/users/{id:int}/{state:[A-Z]+}', {
params: { id: { value: null }, state: { value: null } }
params: { id: { value: null, squash: true }, state: { value: null, squash: true } }
});
expect(m.exec('/users/1138')).toEqual({ id: 1138, state: null });
expect(m.exec('/users/1138/NY')).toEqual({ id: 1138, state: "NY" });
Expand All @@ -314,7 +314,7 @@ describe("urlMatcherFactory", function () {

it("should correctly format multiple", function() {
var m = new UrlMatcher('/users/{id:int}/{state:[A-Z]+}', {
params: { id: { value: null }, state: { value: null } }
params: { id: { value: null, squash: true }, state: { value: null, squash: true } }
});

expect(m.format()).toBe("/users/");
Expand All @@ -325,7 +325,7 @@ describe("urlMatcherFactory", function () {

it("should match in between static segments", function() {
var m = new UrlMatcher('/users/{user:int}/photos', {
params: { user: 5 }
params: { user: { value: 5, squash: true } }
});
expect(m.exec('/users/photos').user).toBe(5);
expect(m.exec('/users/6/photos').user).toBe(6);
Expand All @@ -334,20 +334,20 @@ describe("urlMatcherFactory", function () {
});

it("should correctly format with an optional followed by a required parameter", function() {
var m = new UrlMatcher('/:user/gallery/photos/:photo', {
var m = new UrlMatcher('/home/:user/gallery/photos/:photo', {
params: {
user: {value: null},
photo: {}
user: {value: null, squash: true},
photo: undefined
}
});
expect(m.format({ photo: 12 })).toBe("/gallery/photos/12");
expect(m.format({ user: 1138, photo: 13 })).toBe("/1138/gallery/photos/13");
expect(m.format({ photo: 12 })).toBe("/home/gallery/photos/12");
expect(m.format({ user: 1138, photo: 13 })).toBe("/home/1138/gallery/photos/13");
});

describe("default values", function() {
it("should populate if not supplied in URL", function() {
var m = new UrlMatcher('/users/{id:int}/{test}', {
params: { id: { value: 0 }, test: { value: "foo" } }
params: { id: { value: 0, squash: true }, test: { value: "foo", squash: true } }
});
expect(m.exec('/users')).toEqual({ id: 0, test: "foo" });
expect(m.exec('/users/2')).toEqual({ id: 2, test: "foo" });
Expand All @@ -360,7 +360,7 @@ describe("urlMatcherFactory", function () {
var m = new UrlMatcher('/foo/:foo', {
params: { foo: "bar" }
});
expect(m.exec("/foo")).toEqual({ foo: "bar" });
expect(m.exec("/foo/")).toEqual({ foo: "bar" });
});

it("should populate query params", function() {
Expand All @@ -372,21 +372,19 @@ describe("urlMatcherFactory", function () {
});

it("should allow function-calculated values", function() {
function barFn() { return "Value from bar()"; }
var m = new UrlMatcher('/foo/:bar', {
params: {
bar: function() {
return "Value from bar()";
}
}
params: { bar: barFn }
});
expect(m.exec('/foo/').bar).toBe("Value from bar()");

m = new UrlMatcher('/foo/:bar', {
params: { bar: { value: barFn, squash: true } }
});
expect(m.exec('/foo').bar).toBe("Value from bar()");

var m = new UrlMatcher('/foo?bar', {
params: {
bar: function() {
return "Value from bar()";
}
}
m = new UrlMatcher('/foo?bar', {
params: { bar: barFn }
});
expect(m.exec('/foo').bar).toBe("Value from bar()");
});
Expand All @@ -402,7 +400,7 @@ describe("urlMatcherFactory", function () {
var user = { name: "Bob" };

$stateParams.user = user;
expect(m.exec('/users').user).toBe(user);
expect(m.exec('/users/').user).toBe(user);
}));
});
});
Expand Down

0 comments on commit cb9fd9d

Please sign in to comment.