Skip to content
This repository has been archived by the owner on Oct 6, 2020. It is now read-only.

Fix issues when the initial (cached) request is json #4

Closed
wants to merge 5 commits into from
Closed

Fix issues when the initial (cached) request is json #4

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
60642ca
Tests demonstrating an issue when the initially cached response is json
Mar 8, 2016
f9b2427
Fix issues when the initial (cached) request is json
Mar 8, 2016
fe1869c
Fix some whitespace issues in new tests
Mar 9, 2016
57e391c
Fix an issue with modified cached json responses
Mar 9, 2016
9e5aca4
Merge pull request #1 from cpacey/cpacey/json_fixes_plus
Mar 9, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,12 @@ HttpCache.prototype = {
if (options.json) {
/* Use the cached response */
try {
var parsed = JSON.parse(cachedContent.body);
var parsed;
if( typeof cachedContent.body === 'object' ) {
parsed = cachedContent.body;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to return a deep clone, otherwise client changes to the object would bleed between requests.

} else {
parsed = JSON.parse(cachedContent.body);
}
return callback(null, response, parsed);
} catch(e) {
this.stats.increment('json.parse.error');
Expand All @@ -310,7 +315,13 @@ HttpCache.prototype = {
}

/* Use the cached response */
return callback(null, response, cachedContent.body);
var stringified;
if( typeof cachedContent.body === 'string' ) {
stringified = cachedContent.body;
} else {
stringified = JSON.stringify(cachedContent.body);
}
return callback(null, response, stringified);
}
};

Expand Down
99 changes: 99 additions & 0 deletions test/request-http-cache-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,105 @@ describe('request-http-cache', function() {
});

});

describe('it should handle json initial requests', function() {

describe('when subsequent request is json', function() {
var httpRequestCache;
var err, path, response, body;

beforeEach(function(done) {
var mockBackend = new RequestHttpCache.backends.InMemory();
var k = keyGenerator('https://api.github.com' + path, {}, null);

mockBackend.store(k, {
url: 'https://api.github.com' + path,
statusCode: 200,
etag: '1234',
expiry: Date.now() + 1000,
headers: {
'content-type': MIME_JSON
},
body: { hello: 'cached' } // JSON data
}, function() {});

httpRequestCache = new RequestHttpCache({
backend: mockBackend
});

httpRequestCache.extension({
url: 'https://api.github.com' + path,
json: true
}, function(_err, _response, _body) {
err = _err;
response = _response;
body = _body;
done();
}, request);

});


before(function() {
path = '/hit1';
});

it('should return cached response', function() {
assert(!err);
assert.strictEqual(response.statusCode, 200);
assert.strictEqual(response.headers['content-type'], MIME_JSON);
assert.deepEqual(body, { hello: 'cached' });
});
});

describe('when subsequent request is not json', function() {
var httpRequestCache;
var err, path, response, body;

beforeEach(function(done) {
var mockBackend = new RequestHttpCache.backends.InMemory();
var k = keyGenerator('https://api.github.com' + path, {}, null);

mockBackend.store(k, {
url: 'https://api.github.com' + path,
statusCode: 200,
etag: '1234',
expiry: Date.now() + 1000,
headers: {
'content-type': MIME_JSON
},
body: { hello: 'cached' } // JSON data
}, function() {});

httpRequestCache = new RequestHttpCache({
backend: mockBackend
});

httpRequestCache.extension({
url: 'https://api.github.com' + path,
json: false
}, function(_err, _response, _body) {
err = _err;
response = _response;
body = _body;
done();
}, request);

});


before(function() {
path = '/hit1';
});

it('should return cached response', function() {
assert(!err);
assert.strictEqual(response.statusCode, 200);
assert.strictEqual(response.headers['content-type'], MIME_JSON);
assert.deepEqual(body, "{\"hello\":\"cached\"}");
});
});
});
});

describe('backend failures', function() {
Expand Down