Skip to content

Commit 7e9060c

Browse files
committed
Handle case where JSON response is returned as content-type text/html
As noted in #146, the API response is not properly parsed by superagent when a caching plugin is enabled: the `.body` property of the response ends up null, so an error is thrown when the pagination object is tacked on. This seems to originate from the caching plugin (WP SuperCache) forcing a "text/html" content-type header on the API responses. The workaround in this PR is to detect for the text/html content type, and to parse the JSON in the response `.text` property should the response body itself end up empty. Fixes #146
1 parent 944f79a commit 7e9060c

File tree

4 files changed

+154
-9
lines changed

4 files changed

+154
-9
lines changed

lib/http-transport.js

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ var url = require( 'url' );
1313
var WPRequest = require( './constructors/wp-request' );
1414
var checkMethodSupport = require( './util/check-method-support' );
1515
var objectReduce = require( './util/object-reduce' );
16+
var isEmptyObject = require( './util/is-empty-object' );
1617

1718
/**
1819
* Conditionally set basic authentication on a server request object
@@ -78,6 +79,29 @@ function mergeUrl( endpoint, linkPath ) {
7879
return url.format( request );
7980
}
8081

82+
/**
83+
* Extract the body property from the superagent response, or else try to parse
84+
* the response text to get a JSON object.
85+
*
86+
* @param {Object} response The response object from the HTTP request
87+
* @param {String} response.text The response content as text
88+
* @param {Object} response.body The response content as a JS object
89+
* @returns {Object} The response content as a JS object
90+
*/
91+
function extractResponseBody( response ) {
92+
var responseBody = response.body;
93+
if ( isEmptyObject( responseBody ) && response.type === 'text/html' ) {
94+
// Response may have come back as HTML due to caching plugin; try to parse
95+
// the response text into JSON
96+
try {
97+
responseBody = JSON.parse( response.text );
98+
} catch ( e ) {
99+
// Swallow errors, it's OK to fall back to returning the body
100+
}
101+
}
102+
return responseBody;
103+
}
104+
81105
/**
82106
* If the response is not paged, return the body as-is. If pagination
83107
* information is present in the response headers, parse those headers into
@@ -98,46 +122,48 @@ function mergeUrl( endpoint, linkPath ) {
98122
* @returns {Object} The body of the HTTP request, conditionally augmented with
99123
* pagination metadata
100124
*/
101-
function paginateResponse( result, endpoint, httpTransport ) {
125+
function createPaginationObject( result, endpoint, httpTransport ) {
126+
var _paging = null;
127+
102128
if ( ! result.headers || ! result.headers[ 'x-wp-totalpages' ] ) {
103129
// No headers: return as-is
104-
return result;
130+
return _paging;
105131
}
106132

107133
var totalPages = result.headers[ 'x-wp-totalpages' ];
108134

109135
if ( ! totalPages || totalPages === '0' ) {
110136
// No paging: return as-is
111-
return result;
137+
return _paging;
112138
}
113139

114140
// Decode the link header object
115141
var links = result.headers.link ? parseLinkHeader( result.headers.link ) : {};
116142

117143
// Store pagination data from response headers on the response collection
118-
result.body._paging = {
144+
_paging = {
119145
total: result.headers[ 'x-wp-total' ],
120146
totalPages: totalPages,
121147
links: links
122148
};
123149

124150
// Create a WPRequest instance pre-bound to the "next" page, if available
125151
if ( links.next ) {
126-
result.body._paging.next = new WPRequest({
152+
_paging.next = new WPRequest({
127153
transport: httpTransport,
128154
endpoint: mergeUrl( endpoint, links.next )
129155
});
130156
}
131157

132158
// Create a WPRequest instance pre-bound to the "prev" page, if available
133159
if ( links.prev ) {
134-
result.body._paging.prev = new WPRequest({
160+
_paging.prev = new WPRequest({
135161
transport: httpTransport,
136162
endpoint: mergeUrl( endpoint, links.prev )
137163
});
138164
}
139165

140-
return result;
166+
return _paging;
141167
}
142168

143169
// HTTP-Related Helpers
@@ -153,7 +179,6 @@ function paginateResponse( result, endpoint, httpTransport ) {
153179
* @return {Promise} A promise to the superagent request
154180
*/
155181
function invokeAndPromisify( request, callback, transform ) {
156-
157182
return new Promise(function( resolve, reject ) {
158183
// Fire off the result
159184
request.end(function( err, result ) {
@@ -206,7 +231,12 @@ function invokeAndPromisify( request, callback, transform ) {
206231
function returnBody( wpreq, result ) {
207232
var endpoint = wpreq._options.endpoint;
208233
var httpTransport = wpreq.transport;
209-
return paginateResponse( result, endpoint, httpTransport ).body;
234+
var body = extractResponseBody( result );
235+
var _paging = createPaginationObject( result, endpoint, httpTransport );
236+
if ( _paging ) {
237+
body._paging = _paging;
238+
}
239+
return body;
210240
}
211241

212242
/**

lib/util/is-empty-object.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
'use strict';
2+
3+
module.exports = function( value ) {
4+
// If the value is not object-like, then it is certainly not an empty object
5+
if ( typeof value !== 'object' ) {
6+
return false;
7+
}
8+
9+
// For our purposes an empty array should not be treated as an empty object
10+
// (Since this is used to process invalid content-type responses, )
11+
if ( Array.isArray( value ) ) {
12+
return false;
13+
}
14+
15+
for ( var key in value ) {
16+
if ( value.hasOwnProperty( key ) ) {
17+
return false;
18+
}
19+
}
20+
21+
return true;
22+
};

tests/integration/posts.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,17 @@ describe( 'integration: posts()', function() {
9696
return expect( prom ).to.eventually.equal( SUCCESS );
9797
});
9898

99+
it( 'properly parses responses returned from server as text/html', function() {
100+
var prom = wp.posts()
101+
.param( '_wpapi_force_html', true )
102+
.get()
103+
.then(function( posts ) {
104+
expect( getTitles( posts ) ).to.deep.equal( expectedResults.titles.page1 );
105+
return SUCCESS;
106+
});
107+
return expect( prom ).to.eventually.equal( SUCCESS );
108+
});
109+
99110
describe( 'paging properties', function() {
100111

101112
it( 'are exposed as _paging on the response array', function() {
@@ -109,6 +120,18 @@ describe( 'integration: posts()', function() {
109120
return expect( prom ).to.eventually.equal( SUCCESS );
110121
});
111122

123+
it( 'are exposed as _paging on the response array when response is text/html', function() {
124+
var prom = wp.posts()
125+
.param( '_wpapi_force_html', true )
126+
.get()
127+
.then(function( posts ) {
128+
expect( posts ).to.have.property( '_paging' );
129+
expect( posts._paging ).to.be.an( 'object' );
130+
return SUCCESS;
131+
});
132+
return expect( prom ).to.eventually.equal( SUCCESS );
133+
});
134+
112135
it( 'include the total number of posts: use .headers() for coverage reasons', function() {
113136
var prom = wp.posts()
114137
.headers()
@@ -170,6 +193,21 @@ describe( 'integration: posts()', function() {
170193
return expect( prom ).to.eventually.equal( SUCCESS );
171194
});
172195

196+
it( 'allows access to the next page of results via .next when response is text/html', function() {
197+
var prom = wp.posts()
198+
.param( '_wpapi_force_html', true )
199+
.get()
200+
.then(function( posts ) {
201+
return posts._paging.next
202+
.get()
203+
.then(function( posts ) {
204+
expect( posts ).to.be.an( 'array' );
205+
return SUCCESS;
206+
});
207+
});
208+
return expect( prom ).to.eventually.equal( SUCCESS );
209+
});
210+
173211
it( 'provides a bound WPRequest for the previous page as .prev', function() {
174212
var prom = wp.posts()
175213
.get()
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
'use strict';
2+
var expect = require( 'chai' ).expect;
3+
4+
var isEmptyObject = require( '../../../../lib/util/is-empty-object' );
5+
6+
describe( 'isEmptyObject utility', function() {
7+
8+
it( 'is defined', function() {
9+
expect( isEmptyObject ).to.exist;
10+
});
11+
12+
it( 'is a function', function() {
13+
expect( isEmptyObject ).to.be.a( 'function' );
14+
});
15+
16+
it( 'returns true if passed an empty object', function() {
17+
expect( isEmptyObject( {} ) ).to.equal( true );
18+
});
19+
20+
it( 'returns true if passed a constructed object with no instance properties', function() {
21+
function Ctor() {}
22+
Ctor.prototype.prop = 'val';
23+
expect( isEmptyObject( new Ctor() ) ).to.equal( true );
24+
});
25+
26+
it( 'returns false if passed an object with own properties', function() {
27+
expect( isEmptyObject({ prop: 'value' }) ).to.equal( false );
28+
});
29+
30+
it( 'returns false if passed a constructed object with instance properties', function() {
31+
function Ctor() {
32+
this.prop = 'val';
33+
}
34+
expect( isEmptyObject( new Ctor() ) ).to.equal( false );
35+
});
36+
37+
it( 'returns false if passed a string', function() {
38+
expect( isEmptyObject( '{}' ) ).to.equal( false );
39+
});
40+
41+
it( 'returns false if passed an empty array', function() {
42+
expect( isEmptyObject( [] ) ).to.equal( false );
43+
});
44+
45+
it( 'returns false if passed a boolean', function() {
46+
expect( isEmptyObject( true ) ).to.equal( false );
47+
expect( isEmptyObject( false ) ).to.equal( false );
48+
});
49+
50+
it( 'returns false if passed a number', function() {
51+
expect( isEmptyObject( 0 ) ).to.equal( false );
52+
expect( isEmptyObject( 1337 ) ).to.equal( false );
53+
});
54+
55+
});

0 commit comments

Comments
 (0)