Skip to content

Commit

Permalink
Implement FetchResponse#clone and override the generic fetch instead.
Browse files Browse the repository at this point in the history
  • Loading branch information
Carlos Vializ committed Mar 3, 2017
1 parent 6e4297a commit c2fca22
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 63 deletions.
97 changes: 40 additions & 57 deletions src/service/cached-xhr-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,80 +40,63 @@ export class CachedXhr extends Xhr {
}

/**
* Attempt to cache the result of a fetch.
* @param {string} key The cache key
* @param {!Promise<!JSONType>|!Promise<string>|!Promise<!Document>} fetch
* @param {?./xhr-impl.FetchInitDef=} opt_init Fetch options object
* @return {!Promise<!JSONType>|!Promise<string>|!Promise<!Document>}
*/
cacheFetch_(key, fetch, opt_init) {
// A fetch is cachable if it's implicitly or explicitly a GET
const isCacheable =
(!opt_init || !opt_init.method || opt_init.method === 'GET');
if (isCacheable) {
this.cache_.put(key, fetch);
}
return fetch;
}

/**
* Attempt to retrieve a cached fetch.
* @param {string} key The cache key
* @return {?Promise<!JSONType>|?Promise<string>|?Promise<!Document>}
*/
getCachedFetch_(key) {
return this.cache_.get(key);
}

/**
* Fetches and caches a JSON request.
* Fetch and attempt to cache, or get an already cached fetch,
* then clone and return the response.
*
* @param {string} input URL
* @param {?./xhr-impl.FetchInitDef=} opt_init
* @return {!Promise<!JSONType>}
* @param {?./xhr-impl.FetchInitDef=} opt_init Fetch options object.
* @return {!Promise<!./xhr-impl.FetchResponse>}
* @override
*/
fetchJson(input, opt_init) {
const key = this.getCacheKey_(input, 'json');
fetch(input, opt_init) {
const accept =
(opt_init && opt_init.headers && opt_init.headers['Accept']) || '';
const key = this.getCacheKey_(input, accept);
return this.getCachedFetch_(key) ||
this.cacheFetch_(key, super.fetchJson(input, opt_init), opt_init);
this.cacheFetch_(key, super.fetch(input, opt_init), opt_init);
}

/**
* Fetches and caches a text request.
* @param {string} input
* @param {?./xhr-impl.FetchInitDef=} opt_init
* @return {!Promise<string>}
* @override
* Creates a cache key for a fetch.
*
* @param {string} input URL
* @param {string} responseType
* @return {string}
* @private
*/
fetchText(input, opt_init) {
const key = this.getCacheKey_(input, 'text');
return this.getCachedFetch_(key) ||
this.cacheFetch_(key, super.fetchText(input, opt_init), opt_init);
getCacheKey_(input, responseType) {
return removeFragment(input) + responseType;
}

/**
* Fetches and caches a document request.
* @param {string} input
* @param {?./xhr-impl.FetchInitDef=} opt_init
* @return {!Promise<!Document>}
* @override
* Attempt to retrieve a cached fetch.
*
* @param {string} key The cache key
* @return {?Promise<!./xhr-impl.FetchResponse>}
*/
fetchDocument(input, opt_init) {
const key = this.getCacheKey_(input, 'document');
return this.getCachedFetch_(key) ||
this.cacheFetch_(key, super.fetchDocument(input, opt_init), opt_init);
getCachedFetch_(key) {
const fetch = this.cache_.get(key);
return fetch && fetch.then(response => response.clone());
}

/**
* Creates a cache key for a fetch.
* Attempt to cache the result of a fetch.
*
* @param {string} input URL
* @param {string} responseType
* @return {string}
* @private
* @param {string} key The cache key
* @param {!Promise<!./xhr-impl.FetchResponse>} fetch
* @param {?./xhr-impl.FetchInitDef=} opt_init Fetch options object
* @return {!Promise<!./xhr-impl.FetchResponse>}
*/
getCacheKey_(input, responseType) {
return removeFragment(input) + responseType;
cacheFetch_(key, fetch, opt_init) {
// A fetch is cachable if it's implicitly or explicitly a GET
const isCacheable =
(!opt_init || !opt_init.method || opt_init.method === 'GET');
if (isCacheable) {
this.cache_.put(key, fetch);
return fetch.then(response => response.clone());
} else {
return fetch;
}
}
}

Expand Down
20 changes: 16 additions & 4 deletions src/service/xhr-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ export class Xhr {
init.headers['Content-Type'] = 'application/json;charset=utf-8';
init.body = JSON.stringify(init.body);
}
return this.fetch(input, init).then(response => response.json());
return this.fetch(input, init)
.then(response => response.json());
}

/**
Expand All @@ -207,7 +208,8 @@ export class Xhr {
*/
fetchText(input, opt_init) {
const init = setupInit(opt_init, 'text/plain');
return this.fetch(input, init).then(response => response.text());
return this.fetch(input, init)
.then(response => response.text());
}

/**
Expand All @@ -221,7 +223,8 @@ export class Xhr {
fetchDocument(input, opt_init) {
const init = setupInit(opt_init, 'text/html');
init.responseType = 'document';
return this.fetch(input, init).then(response => response.document_());
return this.fetch(input, init)
.then(response => response.document_());
}

/**
Expand All @@ -232,7 +235,7 @@ export class Xhr {
fetch(input, opt_init) {
const init = setupInit(opt_init);
return this.fetchAmpCors_(input, init).then(response =>
assertSuccess(response));
assertSuccess(response));
}

/**
Expand Down Expand Up @@ -454,6 +457,15 @@ export class FetchResponse {
this.bodyUsed = false;
}

/**
* Create a copy of the response and return it.
* @return {!FetchResponse}
*/
clone() {
dev().assert(!this.bodyUsed, 'Body already used');
return new FetchResponse(this.xhr_);
}

/**
* Drains the response and returns the text.
* @return {!Promise<string>}
Expand Down
3 changes: 1 addition & 2 deletions src/utils/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@ export function hasOwn(obj, key) {
* e.g.
* `getPath({a: {b: [{c: 2}]}}, 'a.b[0].c') === 2`
*
* @param {T} obj a map-like value
* @param {*} obj a map-like value
* @param {string} path a dot-separated list of keys to reference a value
* @return {*}
* @template T
*/
export function getPath(obj, path) {
const arrayIndexRe = /\[(\d+)\]/g;
Expand Down
57 changes: 57 additions & 0 deletions test/functional/test-cached-xhr.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,63 @@ describes.sandboxed('CachedXhr', {}, () => {
};
}

describes.fakeWin('#fetch', getPolyfillWin(), env => {
const TEST_OBJECT = {a: {b: [{c: 2}, {d: 4}]}};
const TEST_RESPONSE = JSON.stringify(TEST_OBJECT);
const mockXhr = {
status: 200,
responseText: TEST_RESPONSE,
};
const textInit = {headers: {'Accept': 'text/plain'}};
const jsonInit = {headers: {'Accept': 'application/json'}};
let xhr;
let fetchStub;

beforeEach(() => {
xhr = installCachedXhrService(env.win);
fetchStub = env.sandbox.stub(xhr, 'fetchAmpCors_',
() => Promise.resolve(new FetchResponse(mockXhr)));
});

it('should fetch a generic request once for identical URLs', () => {
return Promise.all([
xhr.fetch('/get?k=v1').then(response => response.text()),
xhr.fetch('/get?k=v1').then(response => response.text()),
]).then(results => {
expect(fetchStub).to.be.calledOnce;
expect(results[0]).to.equal(TEST_RESPONSE);
expect(results[1]).to.equal(TEST_RESPONSE);
});
});

it('should separately cache generic fetches with identical URLs' +
'but different "Accept" headers', () => {
return Promise.all([
xhr.fetch('/get?k=v1', textInit).then(response => response.text()),
xhr.fetch('/get?k=v1', textInit).then(response => response.text()),
xhr.fetch('/get?k=v1', jsonInit).then(response => response.json()),
xhr.fetch('/get?k=v1', jsonInit).then(response => response.json()),
]).then(results => {
expect(fetchStub).to.be.calledTwice;
expect(results[0]).to.equal(TEST_RESPONSE);
expect(results[1]).to.equal(TEST_RESPONSE);
expect(results[2]).to.jsonEqual(TEST_OBJECT);
expect(results[3]).to.jsonEqual(TEST_OBJECT);
});
});

it('should cache the same as the convenience methods', () => {
return Promise.all([
xhr.fetch('/get?k=v1', jsonInit).then(response => response.json()),
xhr.fetchJson('/get?k=v1'),
]).then(results => {
expect(fetchStub).to.be.calledOnce;
expect(results[0]).to.jsonEqual(TEST_OBJECT);
expect(results[1]).to.jsonEqual(TEST_OBJECT);
});
});
});

describes.fakeWin('#fetchJson', getPolyfillWin(), env => {
const TEST_OBJECT = {a: {b: [{c: 2}, {d: 4}]}};
const TEST_RESPONSE = JSON.stringify(TEST_OBJECT);
Expand Down
23 changes: 23 additions & 0 deletions test/functional/test-xhr.js
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,29 @@ describe('XHR', function() {
});
});

it('should be cloneable and each instance should provide text', () => {
const response = new FetchResponse(mockXhr);
const clone = response.clone();
return response.text()
.then(result => {
const callCloneText = () => clone.text().then(clonedText => {
expect(clonedText).to.equal(TEST_TEXT);
});
expect(callCloneText, 'should not throw').to.not.throw(Error);
expect(result).to.equal(TEST_TEXT);
});
});

it('should not be cloneable if body is already accessed', () => {
const response = new FetchResponse(mockXhr);
return response.text()
.then(() => {
expect(() => response.clone(), 'should throw').to.throw(
Error,
/Body already used/);
});
});

scenarios.forEach(test => {
if (test.desc === 'Polyfill') {
// FetchRequest is only returned by the Polyfill version of Xhr.
Expand Down

0 comments on commit c2fca22

Please sign in to comment.