Skip to content

Commit

Permalink
Fix 'cache is not defined' errors in tile_request_cache.js & add tests (
Browse files Browse the repository at this point in the history
#8788)

* Move cacheGet/Put catches to end of chain to avoid 'cache is not defined' TypeErrors
* Add some tests for tile_request_cache
  • Loading branch information
Anjana Sofia Vakil authored Sep 24, 2019
1 parent ce1ab92 commit 30db469
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 6 deletions.
13 changes: 7 additions & 6 deletions src/util/tile_request_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ export function cachePut(request: Request, response: Response, requestTime: numb
const clonedResponse = new window.Response(body, options);

window.caches.open(CACHE_NAME)
.catch(e => warnOnce(e.message))
.then(cache => cache.put(stripQueryParameters(request.url), clonedResponse));
.then(cache => cache.put(stripQueryParameters(request.url), clonedResponse))
.catch(e => warnOnce(e.message));
});
}

Expand All @@ -77,12 +77,10 @@ export function cacheGet(request: Request, callback: (error: ?any, response: ?Re
const strippedURL = stripQueryParameters(request.url);

window.caches.open(CACHE_NAME)
.catch(callback)
.then(cache => {
// manually strip URL instead of `ignoreSearch: true` because of a known
// performance issue in Chrome https://github.com/mapbox/mapbox-gl-js/issues/8431
cache.match(strippedURL)
.catch(callback)
.then(response => {
const fresh = isFresh(response);

Expand All @@ -94,8 +92,11 @@ export function cacheGet(request: Request, callback: (error: ?any, response: ?Re
}

callback(null, response, fresh);
});
});
})
.catch(callback);
})
.catch(callback);

}

function isFresh(response) {
Expand Down
93 changes: 93 additions & 0 deletions test/unit/util/tile_request_cache.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import {test} from '../../util/test';
import {cacheGet, cachePut} from '../../../src/util/tile_request_cache';
import window from '../../../src/util/window';
import sinon from 'sinon';

test('tile_request_cache', (t) => {
t.beforeEach(callback => {
window.caches = sinon.stub();
callback();
});

t.afterEach(callback => {
window.restore();
callback();
});

t.test('cachePut, no window.caches', (t) => {
delete window.caches;

let result;
try {
result = cachePut({url:''});
t.pass('should return successfully');
t.notOk(result, 'should return null');
} catch (e) {
t.ifError(e, 'should not result in error');
}
t.end();
});

t.test('cacheGet, no window.caches', (t) => {
delete window.caches;

cacheGet({url:''}, (result) => {
t.ifError(result, 'should not result in error');
t.equals(result, null, 'should return null');
t.end();
});
});

t.test('cacheGet, cache open error', (t) => {
window.caches.open = sinon.stub().rejects(new Error('The operation is insecure'));

cacheGet({url:''}, (error) => {
t.ok(error, 'should result in error');
t.equals(error.message, 'The operation is insecure', 'should give the right error message');
t.end();
});
});

t.test('cacheGet, cache match error', (t) => {
const fakeCache = sinon.stub();
fakeCache.match = sinon.stub().withArgs('someurl').rejects(new Error('ohno'));
window.caches.open = sinon.stub().resolves(fakeCache);

cacheGet({url:'someurl'}, (error) => {
t.ok(error, 'should result in error');
t.equals(error.message, 'ohno', 'should give the right error message');
t.end();
});
});

t.test('cacheGet, happy path', (t) => {
const fakeResponse = {
headers: {get: sinon.stub()},
clone: sinon.stub(),
body: 'yay'
};
fakeResponse.headers.get.withArgs('Expires').returns('2300-01-01');
fakeResponse.headers.get.withArgs('Cache-Control').returns(null);
fakeResponse.clone.returns(fakeResponse);

const fakeCache = sinon.stub();
fakeCache.match = sinon.stub().withArgs('someurl').resolves(fakeResponse);
fakeCache.delete = sinon.stub();
fakeCache.put = sinon.stub();

window.caches.open = sinon.stub().resolves(fakeCache);

cacheGet({url:'someurl'}, (error, response, fresh) => {
t.ifError(error, 'should not result in error');
t.ok(fakeCache.match.calledWith('someurl'), 'should call cache.match with correct url');
t.ok(fakeCache.delete.calledWith('someurl'), 'should call cache.delete with correct url');
t.ok(response, 'should give a response');
t.equals(response.body, 'yay', 'should give the right response object');
t.ok(fresh, 'should consider a response with a future expiry date as "fresh"');
t.ok(fakeCache.put.calledWith('someurl', fakeResponse), 'should call cache.put for fresh response');
t.end();
});
});

t.end();
});

0 comments on commit 30db469

Please sign in to comment.