From e5de1f771a549e1bbe529890f5f8a3d61faac464 Mon Sep 17 00:00:00 2001 From: Brian Ford Date: Mon, 22 Jun 2015 12:14:19 -0700 Subject: [PATCH] refactor(router): refactor BrowserLocation into LocationStrategy This makes it easy to mock browser location and paves the way to implementing hash routing. --- modules/angular2/router.ts | 8 ++- ...tion_mock.ts => mock_location_strategy.ts} | 17 ++---- ...location.ts => html5_location_strategy.ts} | 4 +- modules/angular2/src/router/location.ts | 57 ++++++++++++------- .../angular2/src/router/location_strategy.ts | 14 +++++ .../angular2/src/test_lib/test_injector.ts | 3 + modules/angular2/test/router/location_spec.ts | 26 ++++----- .../test/router/router_integration_spec.ts | 19 +++---- 8 files changed, 86 insertions(+), 62 deletions(-) rename modules/angular2/src/mock/{browser_location_mock.ts => mock_location_strategy.ts} (61%) rename modules/angular2/src/router/{browser_location.ts => html5_location_strategy.ts} (86%) create mode 100644 modules/angular2/src/router/location_strategy.ts diff --git a/modules/angular2/router.ts b/modules/angular2/router.ts index 47ccdcc44f..70a041e09b 100644 --- a/modules/angular2/router.ts +++ b/modules/angular2/router.ts @@ -11,12 +11,14 @@ export {RouterOutlet} from './src/router/router_outlet'; export {RouterLink} from './src/router/router_link'; export {RouteParams} from './src/router/instruction'; export {RouteRegistry} from './src/router/route_registry'; -export {BrowserLocation} from './src/router/browser_location'; +export {LocationStrategy} from './src/router/location_strategy'; +export {HTML5LocationStrategy} from './src/router/html5_location_strategy'; export {Location, appBaseHrefToken} from './src/router/location'; export {Pipeline} from './src/router/pipeline'; export * from './src/router/route_config_decorator'; -import {BrowserLocation} from './src/router/browser_location'; +import {LocationStrategy} from './src/router/location_strategy'; +import {HTML5LocationStrategy} from './src/router/html5_location_strategy'; import {Router, RootRouter} from './src/router/router'; import {RouterOutlet} from './src/router/router_outlet'; import {RouterLink} from './src/router/router_link'; @@ -33,7 +35,7 @@ export const routerDirectives: List = CONST_EXPR([RouterOutlet, RouterLink] export var routerInjectables: List = [ RouteRegistry, Pipeline, - BrowserLocation, + bind(LocationStrategy).toClass(HTML5LocationStrategy), Location, bind(Router) .toFactory((registry, pipeline, location, diff --git a/modules/angular2/src/mock/browser_location_mock.ts b/modules/angular2/src/mock/mock_location_strategy.ts similarity index 61% rename from modules/angular2/src/mock/browser_location_mock.ts rename to modules/angular2/src/mock/mock_location_strategy.ts index f18d319e2b..689e0434ec 100644 --- a/modules/angular2/src/mock/browser_location_mock.ts +++ b/modules/angular2/src/mock/mock_location_strategy.ts @@ -1,12 +1,9 @@ -import {proxy, SpyObject} from 'angular2/test_lib'; -import {IMPLEMENTS, BaseException} from 'angular2/src/facade/lang'; import {EventEmitter, ObservableWrapper} from 'angular2/src/facade/async'; -import {List, ListWrapper} from 'angular2/src/facade/collection'; -import {BrowserLocation} from 'angular2/src/router/browser_location'; +import {List} from 'angular2/src/facade/collection'; +import {LocationStrategy} from 'angular2/src/router/location_strategy'; -@proxy -@IMPLEMENTS(BrowserLocation) -export class DummyBrowserLocation extends SpyObject { + +export class MockLocationStrategy extends LocationStrategy { internalBaseHref: string = '/'; internalPath: string = '/'; internalTitle: string = ''; @@ -31,13 +28,7 @@ export class DummyBrowserLocation extends SpyObject { this.urlChanges.push(url); } - forward(): void { throw new BaseException('Not implemented yet!'); } - - back(): void { throw new BaseException('Not implemented yet!'); } - onPopState(fn): void { ObservableWrapper.subscribe(this._subject, fn); } getBaseHref(): string { return this.internalBaseHref; } - - noSuchMethod(m) { return super.noSuchMethod(m); } } diff --git a/modules/angular2/src/router/browser_location.ts b/modules/angular2/src/router/html5_location_strategy.ts similarity index 86% rename from modules/angular2/src/router/browser_location.ts rename to modules/angular2/src/router/html5_location_strategy.ts index 67adaf8660..cedfc6e4cf 100644 --- a/modules/angular2/src/router/browser_location.ts +++ b/modules/angular2/src/router/html5_location_strategy.ts @@ -1,14 +1,16 @@ import {DOM} from 'angular2/src/dom/dom_adapter'; import {Injectable} from 'angular2/di'; import {EventListener, History, Location} from 'angular2/src/facade/browser'; +import {LocationStrategy} from './location_strategy'; @Injectable() -export class BrowserLocation { +export class HTML5LocationStrategy extends LocationStrategy { private _location: Location; private _history: History; private _baseHref: string; constructor() { + super(); this._location = DOM.getLocation(); this._history = DOM.getHistory(); this._baseHref = DOM.getBaseHref(); diff --git a/modules/angular2/src/router/location.ts b/modules/angular2/src/router/location.ts index b17a510990..a058355866 100644 --- a/modules/angular2/src/router/location.ts +++ b/modules/angular2/src/router/location.ts @@ -1,44 +1,57 @@ -import {BrowserLocation} from './browser_location'; +import {LocationStrategy} from './location_strategy'; import {StringWrapper, isPresent, CONST_EXPR} from 'angular2/src/facade/lang'; import {EventEmitter, ObservableWrapper} from 'angular2/src/facade/async'; import {OpaqueToken, Injectable, Optional, Inject} from 'angular2/di'; export const appBaseHrefToken: OpaqueToken = CONST_EXPR(new OpaqueToken('locationHrefToken')); +/** + * This is the service that an application developer will directly interact with. + * + * Responsible for normalizing the URL against the application's base href. + * A normalized URL is absolute from the URL host, includes the application's base href, and has no + * trailing slash: + * - `/my/app/user/123` is normalized + * - `my/app/user/123` **is not** normalized + * - `/my/app/user/123/` **is not** normalized + */ @Injectable() export class Location { private _subject: EventEmitter; private _baseHref: string; - constructor(public _browserLocation: BrowserLocation, + constructor(public _platformStrategy: LocationStrategy, @Optional() @Inject(appBaseHrefToken) href?: string) { this._subject = new EventEmitter(); - this._baseHref = stripIndexHtml(isPresent(href) ? href : this._browserLocation.getBaseHref()); - this._browserLocation.onPopState((_) => this._onPopState(_)); + this._baseHref = stripTrailingSlash( + stripIndexHtml(isPresent(href) ? href : this._platformStrategy.getBaseHref())); + this._platformStrategy.onPopState((_) => this._onPopState(_)); } _onPopState(_): void { ObservableWrapper.callNext(this._subject, {'url': this.path()}); } - path(): string { return this.normalize(this._browserLocation.path()); } + path(): string { return this.normalize(this._platformStrategy.path()); } - normalize(url: string): string { return this._stripBaseHref(stripIndexHtml(url)); } + normalize(url: string): string { + return stripTrailingSlash(this._stripBaseHref(stripIndexHtml(url))); + } normalizeAbsolutely(url: string): string { - if (url.length > 0 && url[0] != '/') { + if (!url.startsWith('/')) { url = '/' + url; } - return this._addBaseHref(url); + return stripTrailingSlash(this._addBaseHref(url)); } _stripBaseHref(url: string): string { - if (this._baseHref.length > 0 && StringWrapper.startsWith(url, this._baseHref)) { - return StringWrapper.substring(url, this._baseHref.length); + if (this._baseHref.length > 0 && url.startsWith(this._baseHref)) { + return url.substring(this._baseHref.length); } return url; } _addBaseHref(url: string): string { - if (!StringWrapper.startsWith(url, this._baseHref)) { + if (!url.startsWith(this._baseHref)) { return this._baseHref + url; } return url; @@ -46,12 +59,12 @@ export class Location { go(url: string): void { var finalUrl = this.normalizeAbsolutely(url); - this._browserLocation.pushState(null, '', finalUrl); + this._platformStrategy.pushState(null, '', finalUrl); } - forward(): void { this._browserLocation.forward(); } + forward(): void { this._platformStrategy.forward(); } - back(): void { this._browserLocation.back(); } + back(): void { this._platformStrategy.back(); } subscribe(onNext, onThrow = null, onReturn = null): void { ObservableWrapper.subscribe(this._subject, onNext, onThrow, onReturn); @@ -61,12 +74,16 @@ export class Location { function stripIndexHtml(url: string): string { - // '/index.html'.length == 11 - if (url.length > 10 && StringWrapper.substring(url, url.length - 11) == '/index.html') { - return StringWrapper.substring(url, 0, url.length - 11); - } - if (url.length > 1 && url[url.length - 1] == '/') { - url = StringWrapper.substring(url, 0, url.length - 1); + if (/\/index.html$/g.test(url)) { + // '/index.html'.length == 11 + return url.substring(0, url.length - 11); + } + return url; +} + +function stripTrailingSlash(url: string): string { + if (/\/$/g.test(url)) { + url = url.substring(0, url.length - 1); } return url; } diff --git a/modules/angular2/src/router/location_strategy.ts b/modules/angular2/src/router/location_strategy.ts new file mode 100644 index 0000000000..2b4a686677 --- /dev/null +++ b/modules/angular2/src/router/location_strategy.ts @@ -0,0 +1,14 @@ +import {BaseException} from 'angular2/src/facade/lang'; + +function _abstract() { + return new BaseException('This method is abstract'); +} + +export class LocationStrategy { + path(): string { throw _abstract(); } + pushState(ctx: any, title: string, url: string): void { throw _abstract(); } + forward(): void { throw _abstract(); } + back(): void { throw _abstract(); } + onPopState(fn): void { throw _abstract(); } + getBaseHref(): string { throw _abstract(); } +} diff --git a/modules/angular2/src/test_lib/test_injector.ts b/modules/angular2/src/test_lib/test_injector.ts index 64f1a90fcc..6657ca0e32 100644 --- a/modules/angular2/src/test_lib/test_injector.ts +++ b/modules/angular2/src/test_lib/test_injector.ts @@ -32,6 +32,8 @@ import {EventManager, DomEventsPlugin} from 'angular2/src/render/dom/events/even import {MockTemplateResolver} from 'angular2/src/mock/template_resolver_mock'; import {MockXHR} from 'angular2/src/render/xhr_mock'; +import {MockLocationStrategy} from 'angular2/src/mock/mock_location_strategy'; +import {LocationStrategy} from 'angular2/src/router/location_strategy'; import {MockNgZone} from 'angular2/src/mock/ng_zone_mock'; import {TestBed} from './test_bed'; @@ -109,6 +111,7 @@ function _getAppBindings() { Parser, Lexer, ExceptionHandler, + bind(LocationStrategy).toClass(MockLocationStrategy), bind(XHR).toClass(MockXHR), ComponentUrlMapper, UrlResolver, diff --git a/modules/angular2/test/router/location_spec.ts b/modules/angular2/test/router/location_spec.ts index 0d21d54e51..f92e6f2d23 100644 --- a/modules/angular2/test/router/location_spec.ts +++ b/modules/angular2/test/router/location_spec.ts @@ -15,19 +15,19 @@ import { import {Injector, bind} from 'angular2/di'; import {CONST_EXPR} from 'angular2/src/facade/lang'; import {Location, appBaseHrefToken} from 'angular2/src/router/location'; -import {BrowserLocation} from 'angular2/src/router/browser_location'; -import {DummyBrowserLocation} from 'angular2/src/mock/browser_location_mock'; +import {LocationStrategy} from 'angular2/src/router/location_strategy'; +import {MockLocationStrategy} from 'angular2/src/mock/mock_location_strategy'; export function main() { describe('Location', () => { - var browserLocation, location; + var locationStrategy, location; function makeLocation(baseHref: string = '/my/app', binding: any = CONST_EXPR([])): Location { - browserLocation = new DummyBrowserLocation(); - browserLocation.internalBaseHref = baseHref; + locationStrategy = new MockLocationStrategy(); + locationStrategy.internalBaseHref = baseHref; let injector = Injector.resolveAndCreate( - [Location, bind(BrowserLocation).toValue(browserLocation), binding]); + [Location, bind(LocationStrategy).toValue(locationStrategy), binding]); return location = injector.get(Location); } @@ -35,11 +35,11 @@ export function main() { it('should normalize relative urls on navigate', () => { location.go('user/btford'); - expect(browserLocation.path()).toEqual('/my/app/user/btford'); + expect(locationStrategy.path()).toEqual('/my/app/user/btford'); }); it('should not prepend urls with starting slash when an empty URL is provided', - () => { expect(location.normalizeAbsolutely('')).toEqual(browserLocation.getBaseHref()); }); + () => { expect(location.normalizeAbsolutely('')).toEqual(locationStrategy.getBaseHref()); }); it('should not prepend path with an extra slash when a baseHref has a trailing slash', () => { let location = makeLocation('/my/slashed/app/'); @@ -48,17 +48,17 @@ export function main() { it('should not append urls with leading slash on navigate', () => { location.go('/my/app/user/btford'); - expect(browserLocation.path()).toEqual('/my/app/user/btford'); + expect(locationStrategy.path()).toEqual('/my/app/user/btford'); }); it('should remove index.html from base href', () => { let location = makeLocation('/my/app/index.html'); location.go('user/btford'); - expect(browserLocation.path()).toEqual('/my/app/user/btford'); + expect(locationStrategy.path()).toEqual('/my/app/user/btford'); }); it('should normalize urls on popstate', inject([AsyncTestCompleter], (async) => { - browserLocation.simulatePopState('/my/app/user/btford'); + locationStrategy.simulatePopState('/my/app/user/btford'); location.subscribe((ev) => { expect(ev['url']).toEqual('/user/btford'); async.done(); @@ -66,14 +66,14 @@ export function main() { })); it('should normalize location path', () => { - browserLocation.internalPath = '/my/app/user/btford'; + locationStrategy.internalPath = '/my/app/user/btford'; expect(location.path()).toEqual('/user/btford'); }); it('should use optional base href param', () => { let location = makeLocation('/', bind(appBaseHrefToken).toValue('/my/custom/href')); location.go('user/btford'); - expect(browserLocation.path()).toEqual('/my/custom/href/user/btford'); + expect(locationStrategy.path()).toEqual('/my/custom/href/user/btford'); }); }); } diff --git a/modules/angular2/test/router/router_integration_spec.ts b/modules/angular2/test/router/router_integration_spec.ts index f6a2840586..d34b46a763 100644 --- a/modules/angular2/test/router/router_integration_spec.ts +++ b/modules/angular2/test/router/router_integration_spec.ts @@ -20,8 +20,8 @@ import {RouteConfig} from 'angular2/src/router/route_config_decorator'; import {PromiseWrapper} from 'angular2/src/facade/async'; import {BaseException} from 'angular2/src/facade/lang'; import {routerInjectables, Router, appBaseHrefToken, routerDirectives} from 'angular2/router'; -import {BrowserLocation} from 'angular2/src/router/browser_location'; -import {DummyBrowserLocation} from 'angular2/src/mock/browser_location_mock'; +import {LocationStrategy} from 'angular2/src/router/location_strategy'; +import {MockLocationStrategy} from 'angular2/src/mock/mock_location_strategy'; export function main() { describe('router injectables', () => { @@ -32,12 +32,7 @@ export function main() { DOM.appendChild(fakeDoc.body, el); testBindings = [ routerInjectables, - bind(BrowserLocation) - .toFactory(() => { - var browserLocation = new DummyBrowserLocation(); - browserLocation.spy('pushState'); - return browserLocation; - }), + bind(LocationStrategy).toClass(MockLocationStrategy), bind(DOCUMENT_TOKEN).toValue(fakeDoc) ]; }); @@ -48,7 +43,7 @@ export function main() { var router = applicationRef.hostComponent.router; router.subscribe((_) => { expect(el).toHaveText('outer { hello }'); - expect(applicationRef.hostComponent.location.path()).toEqual('/'); + expect(applicationRef.hostComponent.location.path()).toEqual(''); async.done(); }); }); @@ -109,7 +104,7 @@ class HelloCmp { @View({template: "outer { }", directives: routerDirectives}) @RouteConfig([{path: '/', component: HelloCmp}]) class AppCmp { - constructor(public router: Router, public location: BrowserLocation) {} + constructor(public router: Router, public location: LocationStrategy) {} } @@ -124,7 +119,7 @@ class ParentCmp { @View({template: `root { }`, directives: routerDirectives}) @RouteConfig([{path: '/parent/...', component: ParentCmp}]) class HierarchyAppCmp { - constructor(public router: Router, public location: BrowserLocation) {} + constructor(public router: Router, public location: LocationStrategy) {} } @Component({selector: 'oops-cmp'}) @@ -137,5 +132,5 @@ class BrokenCmp { @View({template: "outer { }", directives: routerDirectives}) @RouteConfig([{path: '/cause-error', component: BrokenCmp}]) class BrokenAppCmp { - constructor(public router: Router, public location: BrowserLocation) {} + constructor(public router: Router, public location: LocationStrategy) {} }