jenkins-bot has submitted this change and it was merged. (
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, 59 insertions(+), 73 deletions(-)
Approvals:
BearND: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/mobile-util.js b/lib/mobile-util.js
index 79c99a7..84b4a94 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,76 +207,91 @@
/**
* 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 {
canonical: title.getPrefixedDBKey(),
- normalized: lead.normalizedtitle,
- display: lead.displaytitle,
+ normalized: meta.normalizedtitle,
+ display: meta.displaytitle,
};
};
/*
- * Build a summary for the page given in req
+ * Build a page summary
+ * @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} pageData 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, pageData) {
let summary = {};
- const type = 'standard';
- let code = 200;
+ const page = pageData.page;
+ const meta = pageData.meta;
+ const isContentModelWikitext = meta.contentmodel === 'wikitext';
+ const isWhiteListedNamespace =
mUtil.SUMMARY_NS_WHITELIST.includes(meta.ns);
+ const isMainPage = 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 };
+ }
+
+ const leadText = domino.createDocument(page.sections[0].text);
+ const intro = transforms.extractLeadIntroduction(leadText);
+
+ 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,
- namespace_id: lead.ns,
- namespace_text: lead.ns_text,
- titles: mUtil.buildTitleDictionary(title, lead),
- pageid: lead.id,
- thumbnail: lead.thumbnail,
- originalimage: lead.originalimage,
- lang: lead.lang,
- dir: lead.dir,
- revision: lead.revision,
- tid: lead.tid,
- timestamp: lead.lastmodified,
- description: lead.description,
- coordinates: lead.geo && {
- lat: lead.geo.latitude,
- lon: lead.geo.longitude
+ code: 200,
+ type: 'standard',
+ title: meta.normalizedtitle,
+ displaytitle: meta.displaytitle,
+ namespace_id: meta.ns,
+ namespace_text: meta.ns_text,
+ titles: mUtil.buildTitleDictionary(title, meta),
+ pageid: meta.id,
+ thumbnail: meta.thumbnail,
+ originalimage: meta.originalimage,
+ lang: meta.lang,
+ dir: meta.dir,
+ revision: page.revision,
+ tid: page.tid,
+ timestamp: meta.lastmodified,
+ description: meta.description,
+ coordinates: meta.geo && {
+ lat: meta.geo.lat,
+ lon: meta.geo.lon
},
- content_urls: mUtil.buildContentUrls(domain, title, lead),
- api_urls: mUtil.buildApiUrls(domain, title, lead),
+ content_urls: mUtil.buildContentUrls(domain, title, meta),
+ api_urls: mUtil.buildApiUrls(domain, title, 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()}`,
// read_html:
`https://${domain}/api/rest_v1/page/read-html/${title.getPrefixedDBKey()}`,
@@ -284,7 +300,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 b2614e5..b61484d 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,16 +120,12 @@
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,
id: input.meta.id,
issues,
revision: input.page.revision,
- tid: input.page.tid,
lastmodified: input.meta.lastmodified,
lastmodifier: input.meta.lastmodifier,
displaytitle: input.meta.displaytitle,
@@ -147,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,
@@ -315,19 +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;
- delete lead.tid;
-}
-
/*
* @param {!Request} req
* @param {!Response} res
@@ -342,7 +314,6 @@
res.status(200);
mUtil.setETag(res, response.lead.revision, response.lead.tid);
mUtil.setContentType(res, mUtil.CONTENT_TYPES.mobileSections);
- _stripUnwantedLeadProps(response.lead);
res.json(response).end();
});
}
@@ -376,7 +347,6 @@
res.status(200);
mUtil.setETag(res, response.revision, response.tid);
mUtil.setContentType(res, mUtil.CONTENT_TYPES.mobileSections);
- _stripUnwantedLeadProps(response);
res.json(response).end();
});
}
@@ -434,9 +404,9 @@
router.get('/summary/:title/:revision?/:tid?', (req, res) => {
return BBPromise.props({
title: mwapi.getTitleObj(app, req),
- lead: buildLeadObject(req, false)
+ pageData: _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.pageData);
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: merged
Gerrit-Change-Id: I3b07e97c7b0e72684cec59054467de4a3f8abe3c
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/services/mobileapps
Gerrit-Branch: master
Gerrit-Owner: Mholloway <[email protected]>
Gerrit-Reviewer: BearND <[email protected]>
Gerrit-Reviewer: Fjalapeno <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Mholloway <[email protected]>
Gerrit-Reviewer: Mhurd <[email protected]>
Gerrit-Reviewer: Ppchelko <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
