From 66cc88c8a87206153e4fe82e49bef250ebffa480 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sat, 4 Mar 2017 16:05:37 +0000 Subject: [PATCH] fix(aio): ensure that only one request per document is made --- .../app/documents/document.service.spec.ts | 141 +++++++++++++++--- aio/src/app/documents/document.service.ts | 8 +- 2 files changed, 127 insertions(+), 22 deletions(-) diff --git a/aio/src/app/documents/document.service.spec.ts b/aio/src/app/documents/document.service.spec.ts index 2f185eb507..0449c23405 100644 --- a/aio/src/app/documents/document.service.spec.ts +++ b/aio/src/app/documents/document.service.spec.ts @@ -1,31 +1,132 @@ import { ReflectiveInjector } from '@angular/core'; -import { Location, LocationStrategy } from '@angular/common'; -import { MockLocationStrategy } from '@angular/common/testing'; -import { Http, ConnectionBackend, RequestOptions, BaseRequestOptions } from '@angular/http'; +import { Http, ConnectionBackend, RequestOptions, BaseRequestOptions, Response, ResponseOptions } from '@angular/http'; + +import { Observable } from 'rxjs/Observable'; +import { Subscription } from 'rxjs/Subscription'; +import { BehaviorSubject } from 'rxjs/BehaviorSubject'; + import { MockBackend } from '@angular/http/testing'; import { LocationService } from 'app/shared/location.service'; +import { MockLocationService } from 'testing/location.service'; import { Logger } from 'app/shared/logger.service'; -import { DocumentService } from './document.service'; +import { MockLogger } from 'testing/logger.service'; +import { DocumentService, DocumentContents } from './document.service'; + + +const CONTENT_URL_PREFIX = 'content/docs/'; + +function createResponse(body: any) { + return new Response(new ResponseOptions({ body: JSON.stringify(body) })); +} + +function createInjector(initialUrl: string) { + return ReflectiveInjector.resolveAndCreate([ + DocumentService, + { provide: LocationService, useFactory: () => new MockLocationService(initialUrl) }, + { provide: ConnectionBackend, useClass: MockBackend }, + { provide: RequestOptions, useClass: BaseRequestOptions }, + { provide: Logger, useClass: MockLogger }, + Http, + ]); +} + +function getServices(initialUrl: string = '') { + const injector = createInjector(initialUrl); + return { + backend: injector.get(ConnectionBackend) as MockBackend, + location: injector.get(LocationService) as MockLocationService, + service: injector.get(DocumentService) as DocumentService, + logger: injector.get(Logger) as MockLogger + }; +} describe('DocumentService', () => { - let injector: ReflectiveInjector; - - beforeEach(() => { - injector = ReflectiveInjector.resolveAndCreate([ - DocumentService, - LocationService, - Location, - { provide: LocationStrategy, useClass: MockLocationStrategy }, - { provide: ConnectionBackend, useClass: MockBackend }, - { provide: RequestOptions, useClass: BaseRequestOptions }, - Http, - Logger - ]); - }); - it('should be creatable', () => { - const service: DocumentService = injector.get(DocumentService); + const { service } = getServices(); expect(service).toBeTruthy(); }); + + describe('currentDocument', () => { + + it('should fetch a document for the initial location url', () => { + const { service, backend } = getServices('initial/url'); + const connections = backend.connectionsArray; + service.currentDocument.subscribe(); + + expect(connections.length).toEqual(1); + expect(connections[0].request.url).toEqual(CONTENT_URL_PREFIX + 'initial/url.json'); + expect(backend.connectionsArray[0].request.url).toEqual(CONTENT_URL_PREFIX + 'initial/url.json'); + }); + + it('should emit a document each time the location changes', () => { + let latestDocument: DocumentContents; + const doc0 = { title: 'doc 0' }; + const doc1 = { title: 'doc 1' }; + const { service, backend, location } = getServices('initial/url'); + const connections = backend.connectionsArray; + + service.currentDocument.subscribe(doc => latestDocument = doc); + expect(latestDocument).toBeUndefined(); + + connections[0].mockRespond(createResponse(doc0)); + expect(latestDocument).toEqual(doc0); + + location.urlSubject.next('new/url'); + connections[1].mockRespond(createResponse(doc1)); + expect(latestDocument).toEqual(doc1); + }); + + it('should emit the not-found document if the document is not found on the server', () => { + + }); + + it('should not make a request to the server if the doc is in the cache already', () => { + let latestDocument: DocumentContents; + let subscription: Subscription; + + const doc0 = { title: 'doc 0' }; + const doc1 = { title: 'doc 1' }; + const { service, backend, location } = getServices('url/0'); + const connections = backend.connectionsArray; + + subscription = service.currentDocument.subscribe(doc => latestDocument = doc); + expect(connections.length).toEqual(1); + connections[0].mockRespond(createResponse(doc0)); + expect(latestDocument).toEqual(doc0); + subscription.unsubscribe(); + + // modify the response so we can check that future subscriptions do not trigger another request + connections[0].response.next(createResponse({ title: 'error 0' })); + + subscription = service.currentDocument.subscribe(doc => latestDocument = doc); + location.urlSubject.next('url/1'); + expect(connections.length).toEqual(2); + connections[1].mockRespond(createResponse(doc1)); + expect(latestDocument).toEqual(doc1); + subscription.unsubscribe(); + + // modify the response so we can check that future subscriptions do not trigger another request + connections[1].response.next(createResponse({ title: 'error 1' })); + + subscription = service.currentDocument.subscribe(doc => latestDocument = doc); + location.urlSubject.next('url/0'); + expect(connections.length).toEqual(2); + expect(latestDocument).toEqual(doc0); + subscription.unsubscribe(); + + subscription = service.currentDocument.subscribe(doc => latestDocument = doc); + location.urlSubject.next('url/1'); + expect(connections.length).toEqual(2); + expect(latestDocument).toEqual(doc1); + subscription.unsubscribe(); + }); + + + it('should map the "empty" location to the correct document request', () => { + let latestDocument: DocumentContents; + const { service, backend } = getServices(); + service.currentDocument.subscribe(doc => latestDocument = doc); + }); + }); }); diff --git a/aio/src/app/documents/document.service.ts b/aio/src/app/documents/document.service.ts index 1563571c05..fb15f22cb9 100644 --- a/aio/src/app/documents/document.service.ts +++ b/aio/src/app/documents/document.service.ts @@ -2,6 +2,7 @@ import { Injectable } from '@angular/core'; import { Http, Response } from '@angular/http'; import { Observable } from 'rxjs/Observable'; +import { AsyncSubject } from 'rxjs/AsyncSubject'; import 'rxjs/add/operator/switchMap'; import { LocationService } from 'app/shared/location.service'; @@ -37,7 +38,8 @@ export class DocumentService { private fetchDocument(url: string) { const path = this.computePath(url); this.logger.log('fetching document from', path); - return this.http + const subject = new AsyncSubject(); + this.http .get(path) .map(res => res.json()) .catch((error: Response) => { @@ -48,7 +50,9 @@ export class DocumentService { } else { throw error; } - }); + }) + .subscribe(subject); + return subject.asObservable(); } private computePath(url) {