Skip to content

Commit 66cc88c

Browse files
petebacondarwinIgorMinar
authored andcommitted
fix(aio): ensure that only one request per document is made
1 parent b44bc9c commit 66cc88c

2 files changed

Lines changed: 127 additions & 22 deletions

File tree

Lines changed: 121 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,132 @@
11
import { ReflectiveInjector } from '@angular/core';
2-
import { Location, LocationStrategy } from '@angular/common';
3-
import { MockLocationStrategy } from '@angular/common/testing';
4-
import { Http, ConnectionBackend, RequestOptions, BaseRequestOptions } from '@angular/http';
2+
import { Http, ConnectionBackend, RequestOptions, BaseRequestOptions, Response, ResponseOptions } from '@angular/http';
3+
4+
import { Observable } from 'rxjs/Observable';
5+
import { Subscription } from 'rxjs/Subscription';
6+
import { BehaviorSubject } from 'rxjs/BehaviorSubject';
7+
58
import { MockBackend } from '@angular/http/testing';
69
import { LocationService } from 'app/shared/location.service';
10+
import { MockLocationService } from 'testing/location.service';
711
import { Logger } from 'app/shared/logger.service';
8-
import { DocumentService } from './document.service';
12+
import { MockLogger } from 'testing/logger.service';
13+
import { DocumentService, DocumentContents } from './document.service';
914

10-
describe('DocumentService', () => {
1115

12-
let injector: ReflectiveInjector;
13-
14-
beforeEach(() => {
15-
injector = ReflectiveInjector.resolveAndCreate([
16-
DocumentService,
17-
LocationService,
18-
Location,
19-
{ provide: LocationStrategy, useClass: MockLocationStrategy },
20-
{ provide: ConnectionBackend, useClass: MockBackend },
21-
{ provide: RequestOptions, useClass: BaseRequestOptions },
22-
Http,
23-
Logger
24-
]);
25-
});
16+
const CONTENT_URL_PREFIX = 'content/docs/';
17+
18+
function createResponse(body: any) {
19+
return new Response(new ResponseOptions({ body: JSON.stringify(body) }));
20+
}
21+
22+
function createInjector(initialUrl: string) {
23+
return ReflectiveInjector.resolveAndCreate([
24+
DocumentService,
25+
{ provide: LocationService, useFactory: () => new MockLocationService(initialUrl) },
26+
{ provide: ConnectionBackend, useClass: MockBackend },
27+
{ provide: RequestOptions, useClass: BaseRequestOptions },
28+
{ provide: Logger, useClass: MockLogger },
29+
Http,
30+
]);
31+
}
32+
33+
function getServices(initialUrl: string = '') {
34+
const injector = createInjector(initialUrl);
35+
return {
36+
backend: injector.get(ConnectionBackend) as MockBackend,
37+
location: injector.get(LocationService) as MockLocationService,
38+
service: injector.get(DocumentService) as DocumentService,
39+
logger: injector.get(Logger) as MockLogger
40+
};
41+
}
42+
43+
describe('DocumentService', () => {
2644

2745
it('should be creatable', () => {
28-
const service: DocumentService = injector.get(DocumentService);
46+
const { service } = getServices();
2947
expect(service).toBeTruthy();
3048
});
49+
50+
describe('currentDocument', () => {
51+
52+
it('should fetch a document for the initial location url', () => {
53+
const { service, backend } = getServices('initial/url');
54+
const connections = backend.connectionsArray;
55+
service.currentDocument.subscribe();
56+
57+
expect(connections.length).toEqual(1);
58+
expect(connections[0].request.url).toEqual(CONTENT_URL_PREFIX + 'initial/url.json');
59+
expect(backend.connectionsArray[0].request.url).toEqual(CONTENT_URL_PREFIX + 'initial/url.json');
60+
});
61+
62+
it('should emit a document each time the location changes', () => {
63+
let latestDocument: DocumentContents;
64+
const doc0 = { title: 'doc 0' };
65+
const doc1 = { title: 'doc 1' };
66+
const { service, backend, location } = getServices('initial/url');
67+
const connections = backend.connectionsArray;
68+
69+
service.currentDocument.subscribe(doc => latestDocument = doc);
70+
expect(latestDocument).toBeUndefined();
71+
72+
connections[0].mockRespond(createResponse(doc0));
73+
expect(latestDocument).toEqual(doc0);
74+
75+
location.urlSubject.next('new/url');
76+
connections[1].mockRespond(createResponse(doc1));
77+
expect(latestDocument).toEqual(doc1);
78+
});
79+
80+
it('should emit the not-found document if the document is not found on the server', () => {
81+
82+
});
83+
84+
it('should not make a request to the server if the doc is in the cache already', () => {
85+
let latestDocument: DocumentContents;
86+
let subscription: Subscription;
87+
88+
const doc0 = { title: 'doc 0' };
89+
const doc1 = { title: 'doc 1' };
90+
const { service, backend, location } = getServices('url/0');
91+
const connections = backend.connectionsArray;
92+
93+
subscription = service.currentDocument.subscribe(doc => latestDocument = doc);
94+
expect(connections.length).toEqual(1);
95+
connections[0].mockRespond(createResponse(doc0));
96+
expect(latestDocument).toEqual(doc0);
97+
subscription.unsubscribe();
98+
99+
// modify the response so we can check that future subscriptions do not trigger another request
100+
connections[0].response.next(createResponse({ title: 'error 0' }));
101+
102+
subscription = service.currentDocument.subscribe(doc => latestDocument = doc);
103+
location.urlSubject.next('url/1');
104+
expect(connections.length).toEqual(2);
105+
connections[1].mockRespond(createResponse(doc1));
106+
expect(latestDocument).toEqual(doc1);
107+
subscription.unsubscribe();
108+
109+
// modify the response so we can check that future subscriptions do not trigger another request
110+
connections[1].response.next(createResponse({ title: 'error 1' }));
111+
112+
subscription = service.currentDocument.subscribe(doc => latestDocument = doc);
113+
location.urlSubject.next('url/0');
114+
expect(connections.length).toEqual(2);
115+
expect(latestDocument).toEqual(doc0);
116+
subscription.unsubscribe();
117+
118+
subscription = service.currentDocument.subscribe(doc => latestDocument = doc);
119+
location.urlSubject.next('url/1');
120+
expect(connections.length).toEqual(2);
121+
expect(latestDocument).toEqual(doc1);
122+
subscription.unsubscribe();
123+
});
124+
125+
126+
it('should map the "empty" location to the correct document request', () => {
127+
let latestDocument: DocumentContents;
128+
const { service, backend } = getServices();
129+
service.currentDocument.subscribe(doc => latestDocument = doc);
130+
});
131+
});
31132
});

‎aio/src/app/documents/document.service.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Injectable } from '@angular/core';
22
import { Http, Response } from '@angular/http';
33

44
import { Observable } from 'rxjs/Observable';
5+
import { AsyncSubject } from 'rxjs/AsyncSubject';
56
import 'rxjs/add/operator/switchMap';
67

78
import { LocationService } from 'app/shared/location.service';
@@ -37,7 +38,8 @@ export class DocumentService {
3738
private fetchDocument(url: string) {
3839
const path = this.computePath(url);
3940
this.logger.log('fetching document from', path);
40-
return this.http
41+
const subject = new AsyncSubject();
42+
this.http
4143
.get(path)
4244
.map(res => res.json())
4345
.catch((error: Response) => {
@@ -48,7 +50,9 @@ export class DocumentService {
4850
} else {
4951
throw error;
5052
}
51-
});
53+
})
54+
.subscribe(subject);
55+
return subject.asObservable();
5256
}
5357

5458
private computePath(url) {

0 commit comments

Comments
 (0)