Mholloway has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/390305 )
Change subject: Hygiene: Disentangle summary construction from lead object
construction
......................................................................
Hygiene: Disentangle summary construction from lead object construction
Change-Id: I3b07e97c7b0e72684cec59054467de4a3f8abe3c
---
M lib/mobile-util.js
M routes/mobile-sections.js
2 files changed, 63 insertions(+), 69 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/mobileapps
refs/changes/05/390305/1
diff --git a/lib/mobile-util.js b/lib/mobile-util.js
index 63b4e24..a2148e8 100644
--- a/lib/mobile-util.js
+++ b/lib/mobile-util.js
@@ -1,5 +1,6 @@
'use strict';
+const domino = require('domino');
const underscore = require('underscore');
const uuid = require('cassandra-uuid').TimeUuid;
const HTTPError = require('./util').HTTPError;
@@ -206,75 +207,96 @@
/**
* Builds a dictionary containing the various forms of a page title that a
client may need.
* @param {!Object} title a mediawiki-title Title object constructed from a
page title string
- * @param {!Object} lead a page lead object from buildLeadObject in
mobile-sections.js
+ * @param {!Object} meta page metadata
* @return {!Object} a set of useful page title strings
*/
-mUtil.buildTitleDictionary = function(title, lead) {
+mUtil.buildTitleDictionary = function(title, meta) {
return {
title: title.getPrefixedDBKey(),
- normalized_title: lead.normalizedtitle,
- display_title: lead.displaytitle,
- namespace_id: lead.ns,
- namespace_name: lead.ns_text,
+ normalized_title: meta.normalizedtitle,
+ display_title: meta.displaytitle,
+ namespace_id: meta.ns,
+ namespace_name: meta.ns_text,
};
};
/*
* Build a summary for the page given in req
+ * @param {!String} domain the request domain
* @param {!Object} title a mediawiki-title object for the page title
- * @param {!Object} lead a page lead object for the page
+ * @param {!Object} data raw page data for the page
* @return {!Object} a summary 2.0 spec-compliant page summary object
*/
-mUtil.buildSummary = function(domain, title, lead) {
+mUtil.buildSummary = function(domain, title, data) {
let summary = {};
- const type = 'standard';
- let code = 200;
+ const stubArticle = data.page.sections.length <= 1;
+ const lead = domino.createDocument(data.page.sections[0].text);
+ const isContentModelWikitext = data.meta.contentmodel === 'wikitext';
+ const isWhiteListedNamespace =
mUtil.SUMMARY_NS_WHITELIST.includes(data.meta.ns);
+ const isMainPage = data.meta.mainpage;
- if (!lead) {
- return false;
- } else if (lead.contentmodel ||
!mUtil.SUMMARY_NS_WHITELIST.includes(lead.ns)) {
- code = 204;
- } else if (lead.intro) {
- summary = transforms.summarize(lead.intro);
+ if (!isContentModelWikitext) {
+ return { code: 204 };
+ }
+
+ if (!isWhiteListedNamespace) {
+ return { code: 204 };
+ }
+
+ if (isMainPage) {
+ return { code: 204 };
+ }
+
+ // We should always extract the introduction as it's useful for
+ // things like the summary endpoint
+ // however on pages where there is only
+ // one section we shouldn't remove it from initial HTML as it may
+ // have an undesirable result.
+ const intro = transforms.extractLeadIntroduction(lead, !stubArticle);
+
+ if (intro) {
+ summary = transforms.summarize(intro);
} else {
// If the lead introduction is empty we should consider it
// a placeholder e.g. redirect page. To avoid sending an empty
// summary 204. (T176517)
- code = 204;
+ return { code: 204 };
}
return Object.assign({
- code,
- type,
- title: lead.normalizedtitle,
- displaytitle: lead.displaytitle,
- titles: mUtil.buildTitleDictionary(title, lead),
- pageid: lead.id,
- thumbnail: lead.thumbnail,
- originalimage: lead.originalimage,
- lang: lead.lang,
- dir: lead.dir,
- revision: lead.revision,
- timestamp: lead.lastmodified,
- description: lead.description,
- coordinates: lead.geo && {
- lat: lead.geo.latitude,
- lon: lead.geo.longitude
+ code: 200,
+ type: 'standard',
+ title: data.meta.normalizedtitle,
+ displaytitle: data.meta.displaytitle,
+ namespace_id: data.meta.ns,
+ namespace_text: data.meta.ns_text,
+ titles: mUtil.buildTitleDictionary(title, data.meta),
+ pageid: data.meta.id,
+ thumbnail: data.meta.thumbnail,
+ originalimage: data.meta.originalimage,
+ lang: data.meta.lang,
+ dir: data.meta.dir,
+ revision: data.page.revision,
+ timestamp: data.meta.lastmodified,
+ description: data.meta.description,
+ coordinates: data.meta.geo && {
+ lat: data.meta.geo.lat,
+ lon: data.meta.geo.lon
},
- content_urls: mUtil.buildContentUrls(domain, title, lead),
- api_urls: mUtil.buildApiUrls(domain, title, lead),
+ content_urls: mUtil.buildContentUrls(domain, title, data.meta),
+ api_urls: mUtil.buildApiUrls(domain, title, data.meta),
}, summary);
};
-mUtil.buildContentUrls = function(domain, title, lead) {
+mUtil.buildContentUrls = function(domain, title, meta) {
return {
page: `https://${domain}/wiki/${title.getPrefixedDBKey()}`,
revisions:
`https://${domain}/wiki/${title.getPrefixedDBKey()}?action=history`,
edit: `https://${domain}/wiki/${title.getPrefixedDBKey()}?action=edit`,
- talk: lead.talk_ns_text ?
`https://${domain}/wiki/${lead.talk_ns_text}:${title.getKey()}` : undefined,
+ talk: meta.talk_ns_text ?
`https://${domain}/wiki/${meta.talk_ns_text}:${title.getKey()}` : undefined,
};
};
-mUtil.buildApiUrls = function(domain, title, lead) {
+mUtil.buildApiUrls = function(domain, title, meta) {
return {
summary:
`https://${domain}/api/rest_v1/page/summary/${title.getPrefixedDBKey()}`,
mobile_sections:
`https://${domain}/api/rest_v1/page/mobile-sections/${title.getPrefixedDBKey()}`,
@@ -284,7 +306,7 @@
// references:
`https://${domain}/api/rest_v1/page/references/${title.getPrefixedDBKey()}`,
// media:
`https://${domain}/api/rest_v1/page/media/${title.getPrefixedDBKey()}`,
edit_html:
`https://${domain}/api/rest_v1/page/html/${title.getPrefixedDBKey()}`,
- talk_page_html: lead.talk_ns_text ?
`https://${domain}/api/rest_v1/page/html/${lead.talk_ns_text}:${title.getKey()}`
: undefined,
+ talk_page_html: meta.talk_ns_text ?
`https://${domain}/api/rest_v1/page/html/${meta.talk_ns_text}:${title.getKey()}`
: undefined,
};
};
diff --git a/routes/mobile-sections.js b/routes/mobile-sections.js
index 1ceb2d9..f6d5bb7 100644
--- a/routes/mobile-sections.js
+++ b/routes/mobile-sections.js
@@ -93,19 +93,12 @@
let text;
let disambiguation;
let contentmodel;
- let thumbnail;
if (input.meta.contentmodel !== 'wikitext') {
contentmodel = input.meta.contentmodel;
}
if (input.meta.pageprops && input.meta.pageprops.disambiguation !==
undefined) {
disambiguation = true;
}
- if (input.meta.thumbnail) {
- const scaledThumb = mwapi.scaledThumbObj(input.meta.thumbnail,
- input.meta.originalimage.width, mwapi.LEAD_IMAGE_S);
- thumbnail = Object.assign(input.meta.thumbnail, scaledThumb);
- }
-
if (!legacy && !input.meta.mainpage) {
const stubArticle = input.page.sections.length <= 1;
if (!stubArticle) {
@@ -127,9 +120,6 @@
return {
ns: input.meta.ns,
- ns_text: input.meta.ns_text,
- talk_ns: input.meta.talk_ns,
- talk_ns_text: input.meta.talk_ns_text,
contentmodel,
userinfo: input.meta.userinfo,
imageinfo: input.meta.imageinfo,
@@ -146,15 +136,11 @@
protection: input.meta.protection,
editable: input.meta.editable,
mainpage: input.meta.mainpage,
- dir: input.meta.dir,
- lang: input.meta.lang,
languagecount: input.meta.languagecount,
image: mUtil.defaultVal(mUtil.filterEmpty({
file: input.meta.image && input.meta.image.file,
urls: input.meta.thumb &&
mwapi.buildLeadImageUrls(input.meta.thumb.url)
})),
- thumbnail,
- originalimage: input.meta.originalimage,
pronunciation: input.page.pronunciation,
spoken: input.page.spoken,
hatnotes,
@@ -314,18 +300,6 @@
});
}
-// XXX: Remove lead properties used only for constructing summaries. Sometime
soon, let's
-// disentangle the underlying logic.
-function _stripUnwantedLeadProps(lead) {
- delete lead.ns_text;
- delete lead.talk_ns;
- delete lead.talk_ns_text;
- delete lead.dir;
- delete lead.lang;
- delete lead.thumbnail;
- delete lead.originalimage;
-}
-
/*
* @param {!Request} req
* @param {!Response} res
@@ -337,7 +311,6 @@
function buildAllResponse(req, res, legacy) {
return _collectRawPageData(req, legacy).then((response) => {
response = buildAll(response, legacy);
- _stripUnwantedLeadProps(response.lead);
res.status(200);
mUtil.setETag(res, response.lead.revision);
mUtil.setContentType(res, mUtil.CONTENT_TYPES.mobileSections);
@@ -371,7 +344,6 @@
*/
function buildLeadResponse(req, res, legacy) {
return buildLeadObject(req, legacy).then((response) => {
- _stripUnwantedLeadProps(response);
res.status(200);
mUtil.setETag(res, response.revision);
mUtil.setContentType(res, mUtil.CONTENT_TYPES.mobileSections);
@@ -432,9 +404,9 @@
router.get('/summary/:title/:revision?/:tid?', (req, res) => {
return BBPromise.props({
title: mwapi.getTitleObj(app, req),
- lead: buildLeadObject(req, false)
+ data: _collectRawPageData(req, false)
}).then((response) => {
- const summary = mUtil.buildSummary(req.params.domain, response.title,
response.lead);
+ const summary = mUtil.buildSummary(req.params.domain, response.title,
response.data);
if (summary) {
res.status(summary.code);
if (summary.code === 200) {
--
To view, visit https://gerrit.wikimedia.org/r/390305
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3b07e97c7b0e72684cec59054467de4a3f8abe3c
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/mobileapps
Gerrit-Branch: master
Gerrit-Owner: Mholloway <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
