[MediaWiki-commits] [Gerrit] mediawiki...mobileapps[master]: Feed endpoints: Various bug fixes
Mobrovac has submitted this change and it was merged.
Change subject: Feed endpoints: Various bug fixes
..
Feed endpoints: Various bug fixes
- When an individual endpoint is called with aggregated=true, ensure
that (a) the ETag is not set; and (b) the response contains at least
an empty object, so as not to break client connections
- In lib/feed/most-read.js, module.exports.promise should not have the
aggregated param, but should be deduced from the request query
- Add a test for the tfa endpoint with aggregated=true
Bug: T143912
Change-Id: I99d28e8c21548143c6d3a3d886d5028635ba6bab
---
M lib/feed/featured-image.js
M lib/feed/featured.js
M lib/feed/most-read.js
M lib/feed/news.js
M lib/mobile-util.js
M routes/featured-image.js
M routes/featured.js
M routes/most-read.js
M routes/news.js
M test/features/featured/pagecontent.js
10 files changed, 33 insertions(+), 17 deletions(-)
Approvals:
Mobrovac: Verified; Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/feed/featured-image.js b/lib/feed/featured-image.js
index 93e9514..cfd0c1b 100644
--- a/lib/feed/featured-image.js
+++ b/lib/feed/featured-image.js
@@ -4,6 +4,7 @@
'use strict';
+var BBPromise = require('bluebird');
var preq = require('preq');
var api = require('../api-util');
var dateUtil = require('../dateUtil');
@@ -118,7 +119,7 @@
}).catch(function(err) {
if (err.status === 504) {
if (aggregated) {
-return { payload: undefined, meta: undefined };
+return BBPromise.resolve({ payload: undefined, meta: undefined
});
}
throw new HTTPError({
status: 404,
diff --git a/lib/feed/featured.js b/lib/feed/featured.js
index 46e3772..3ea0538 100644
--- a/lib/feed/featured.js
+++ b/lib/feed/featured.js
@@ -76,7 +76,7 @@
var aggregated = !!req.query.aggregated;
if (req.params.domain.indexOf('en') !== 0) {
if (aggregated) {
-return {};
+return BBPromise.resolve({});
} else {
throw new HTTPError({
status: 501,
diff --git a/lib/feed/most-read.js b/lib/feed/most-read.js
index a996ba8..8e95f14 100644
--- a/lib/feed/most-read.js
+++ b/lib/feed/most-read.js
@@ -4,6 +4,7 @@
'use strict';
+var BBPromise = require('bluebird');
var mUtil = require('../mobile-util');
var dateUtil = require('../dateUtil');
var api = require('../api-util');
@@ -22,8 +23,7 @@
return result;
}
-function getTopPageviews(app, req) {
-var aggregated = !!req.query.aggregated;
+function getTopPageviews(app, req, aggregated) {
var yesterday = new Date(dateUtil.getRequestedDate(req) -
dateUtil.ONE_DAY);
var year = aggregated ? yesterday.getUTCFullYear() : req.params.;
var month = aggregated ? dateUtil.pad(yesterday.getUTCMonth() + 1) :
req.params.mm;
@@ -46,7 +46,8 @@
});
}
-function promise(app, req, aggregated) {
+function promise(app, req) {
+var aggregated = !!req.query.aggregated;
var goodTitles, resultsDate;
dateUtil.validate(dateUtil.hyphenDelimitedDateString(req));
return getTopPageviews(app, req, aggregated).then(function (response) {
@@ -109,7 +110,7 @@
// Catch and handle the error if this is an aggregated request and the
// pageview data are not yet loaded.
if (aggregated && err.status === 404) {
-return {};
+return BBPromise.resolve({});
}
throw err;
});
diff --git a/lib/feed/news.js b/lib/feed/news.js
index 081c268..27449a6 100644
--- a/lib/feed/news.js
+++ b/lib/feed/news.js
@@ -1,5 +1,6 @@
'use strict';
+var BBPromise = require('bluebird');
var domino = require('domino');
var api = require('../api-util');
var mUtil = require('../mobile-util');
@@ -40,7 +41,7 @@
var aggregated = !!req.query.aggregated;
if (!newsTemplates[lang]) {
if (aggregated) {
-return { payload: undefined, meta: undefined };
+return BBPromise.resolve({ payload: undefined, meta: undefined });
}
throw new HTTPError({
status: 501,
diff --git a/lib/mobile-util.js b/lib/mobile-util.js
index 74b1cbe..d32108e 100644
--- a/lib/mobile-util.js
+++ b/lib/mobile-util.js
@@ -69,7 +69,9 @@
* @param {Object} value to set the ETag to
*/
mUtil.setETagToValue = function(response, value) {
-response.set('etag', '' + value);
+if (value) {
+response.set('etag', '' + value);
+}
};
/**
diff --git a/routes/featured-image.js b/routes/featured-image.js
index 60b1263..934402c 100644
--- a/routes/featured-image.js
+++ b/routes/featured-image.js
@@ -27,8 +27,8 @@
return featured.promise(app, req)
.then(function (response) {
res.status(200);
-mUtil.setETagToValue(res, response.meta.etag);
-res.json(response.payload).end();
+mUtil.setETagToValue(res, re
[MediaWiki-commits] [Gerrit] mediawiki...mobileapps[master]: Feed endpoints: Various bug fixes
Mobrovac has uploaded a new change for review.
https://gerrit.wikimedia.org/r/309004
Change subject: Feed endpoints: Various bug fixes
..
Feed endpoints: Various bug fixes
- When an individual endpoint is called with aggregated=true, ensure
that (a) the ETag is not set; and (b) the response contains at least
an empty object, so as not to break client connections
- In lib/feed/most-read.js, module.exports.promise should not have the
aggregated param, but should be deduced from the request query
- Add a test for the tfa endpoint with aggregated=true
Bug: T143912
Change-Id: I99d28e8c21548143c6d3a3d886d5028635ba6bab
---
M lib/feed/featured-image.js
M lib/feed/featured.js
M lib/feed/most-read.js
M lib/feed/news.js
M lib/mobile-util.js
M routes/featured-image.js
M routes/featured.js
M routes/most-read.js
M routes/news.js
M test/features/featured/pagecontent.js
10 files changed, 33 insertions(+), 17 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/mobileapps
refs/changes/04/309004/1
diff --git a/lib/feed/featured-image.js b/lib/feed/featured-image.js
index 93e9514..cfd0c1b 100644
--- a/lib/feed/featured-image.js
+++ b/lib/feed/featured-image.js
@@ -4,6 +4,7 @@
'use strict';
+var BBPromise = require('bluebird');
var preq = require('preq');
var api = require('../api-util');
var dateUtil = require('../dateUtil');
@@ -118,7 +119,7 @@
}).catch(function(err) {
if (err.status === 504) {
if (aggregated) {
-return { payload: undefined, meta: undefined };
+return BBPromise.resolve({ payload: undefined, meta: undefined
});
}
throw new HTTPError({
status: 404,
diff --git a/lib/feed/featured.js b/lib/feed/featured.js
index 46e3772..3ea0538 100644
--- a/lib/feed/featured.js
+++ b/lib/feed/featured.js
@@ -76,7 +76,7 @@
var aggregated = !!req.query.aggregated;
if (req.params.domain.indexOf('en') !== 0) {
if (aggregated) {
-return {};
+return BBPromise.resolve({});
} else {
throw new HTTPError({
status: 501,
diff --git a/lib/feed/most-read.js b/lib/feed/most-read.js
index a996ba8..8e95f14 100644
--- a/lib/feed/most-read.js
+++ b/lib/feed/most-read.js
@@ -4,6 +4,7 @@
'use strict';
+var BBPromise = require('bluebird');
var mUtil = require('../mobile-util');
var dateUtil = require('../dateUtil');
var api = require('../api-util');
@@ -22,8 +23,7 @@
return result;
}
-function getTopPageviews(app, req) {
-var aggregated = !!req.query.aggregated;
+function getTopPageviews(app, req, aggregated) {
var yesterday = new Date(dateUtil.getRequestedDate(req) -
dateUtil.ONE_DAY);
var year = aggregated ? yesterday.getUTCFullYear() : req.params.;
var month = aggregated ? dateUtil.pad(yesterday.getUTCMonth() + 1) :
req.params.mm;
@@ -46,7 +46,8 @@
});
}
-function promise(app, req, aggregated) {
+function promise(app, req) {
+var aggregated = !!req.query.aggregated;
var goodTitles, resultsDate;
dateUtil.validate(dateUtil.hyphenDelimitedDateString(req));
return getTopPageviews(app, req, aggregated).then(function (response) {
@@ -109,7 +110,7 @@
// Catch and handle the error if this is an aggregated request and the
// pageview data are not yet loaded.
if (aggregated && err.status === 404) {
-return {};
+return BBPromise.resolve({});
}
throw err;
});
diff --git a/lib/feed/news.js b/lib/feed/news.js
index 081c268..27449a6 100644
--- a/lib/feed/news.js
+++ b/lib/feed/news.js
@@ -1,5 +1,6 @@
'use strict';
+var BBPromise = require('bluebird');
var domino = require('domino');
var api = require('../api-util');
var mUtil = require('../mobile-util');
@@ -40,7 +41,7 @@
var aggregated = !!req.query.aggregated;
if (!newsTemplates[lang]) {
if (aggregated) {
-return { payload: undefined, meta: undefined };
+return BBPromise.resolve({ payload: undefined, meta: undefined });
}
throw new HTTPError({
status: 501,
diff --git a/lib/mobile-util.js b/lib/mobile-util.js
index 74b1cbe..d32108e 100644
--- a/lib/mobile-util.js
+++ b/lib/mobile-util.js
@@ -69,7 +69,9 @@
* @param {Object} value to set the ETag to
*/
mUtil.setETagToValue = function(response, value) {
-response.set('etag', '' + value);
+if (value) {
+response.set('etag', '' + value);
+}
};
/**
diff --git a/routes/featured-image.js b/routes/featured-image.js
index 60b1263..934402c 100644
--- a/routes/featured-image.js
+++ b/routes/featured-image.js
@@ -27,8 +27,8 @@
return featured.promise(app, req)
.then(function (response) {
res.status(200);
-mUtil.setETagToValue(res, response.meta.etag);
-res.json(response.payload).
