[MediaWiki-commits] [Gerrit] mediawiki...mobileapps[master]: Update section validation logic, take 2
Mholloway has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/398181 )
Change subject: Update section validation logic, take 2
..
Update section validation logic, take 2
Updated to use the bunyan Logger class correctly! ;)
1) Log warnings rather than throwing errors, and
2) Only for sections with IDs >= 0 (i.e., not non-editable or pseudo-
sections).
Bug: T182774
Change-Id: I2b037f5c98edf8c964548c011973924aa6ae14cb
---
M lib/parsoid-access.js
M lib/parsoidSections.js
M lib/parsoidSectionsUsingSectionTags.js
M test/lib/parsoid/parsoid-sections-section-elements-tests.js
4 files changed, 57 insertions(+), 31 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/mobileapps
refs/changes/81/398181/1
diff --git a/lib/parsoid-access.js b/lib/parsoid-access.js
index 338ce33..3ae3308 100644
--- a/lib/parsoid-access.js
+++ b/lib/parsoid-access.js
@@ -119,7 +119,7 @@
transforms.stripUnneededMarkup(doc, legacy);
parsoidSections.addSectionDivs(doc);
-page.sections = parsoidSections.getSectionsText(doc);
+page.sections = parsoidSections.getSectionsText(doc, req.logger);
return page;
});
}
diff --git a/lib/parsoidSections.js b/lib/parsoidSections.js
index dcdbb72..f8e21d6 100644
--- a/lib/parsoidSections.js
+++ b/lib/parsoidSections.js
@@ -27,13 +27,14 @@
* Gets the sections array, including HTML string and metadata for all sections
* (id, anchor, line, toclevel).
* @param {!document} doc the parsed DOM Document of the Parsoid output
+ * @param {!Logger} logger the app's bunyan logger
* @return {!sections[]} an array of section JSON elements
*/
-function getSectionsText(doc) {
+function getSectionsText(doc, logger) {
if (!hasParsoidSections(doc)) {
return parsoidSectionsUsingDivs.getSectionsText(doc);
} else {
-return parsoidSectionsUsingSectionTags.getSectionsText(doc);
+return parsoidSectionsUsingSectionTags.getSectionsText(doc, logger);
}
}
diff --git a/lib/parsoidSectionsUsingSectionTags.js
b/lib/parsoidSectionsUsingSectionTags.js
index 76e471d..6fb3b72 100644
--- a/lib/parsoidSectionsUsingSectionTags.js
+++ b/lib/parsoidSectionsUsingSectionTags.js
@@ -1,8 +1,6 @@
'use strict';
const parsoidDomUtils = require('parsoid-dom-utils');
-// const sUtil = require('./util');
-// const HTTPError = sUtil.HTTPError;
const NodeType = require('./nodeType');
/**
@@ -39,7 +37,13 @@
}
}
-/* function throwIfPreviousSectionIsIncomplete(allSections, sectionObj) {
+// skip lead sections since they don't have headings;
+// no line and no anchor means incomplete section
+function shouldLogInvalidSectionWarning(sectionObj) {
+return sectionObj.id >= 1 && !sectionObj.line && !sectionObj.anchor;
+}
+
+function validatePreviousSection(logger, allSections, sectionObj) {
if (allSections.length === 0) {
return;
}
@@ -47,30 +51,29 @@
if (!sectionObj) {
sectionObj = allSections[allSections.length - 1];
}
-// skip lead sections since they don't have headings;
-// no line and no anchor means incomplete section
-if (allSections.length > 1 && !sectionObj.line && !sectionObj.anchor) {
-throw new HTTPError({
-status: 502,
-type: 'unsupported_section',
-title: 'Section structure not supported',
+
+if (shouldLogInvalidSectionWarning(sectionObj)) {
+logger.log('warn', {
+warning: 'invalid_section',
+title: 'Found section without expected heading',
detail: `Cannot find heading for section number ${sectionObj.id}.`
});
}
-} */
+}
/**
* Visits one DOM node. Do the stuff that needs to be done when a single DOM
node is handled.
* In this case, starts a new section object when a tag is
encountered that is a
* direct descendant
+ * @param {!Logger} logger the app's bunyan logger
* @param {!DOMNode} node the node to visit
* @param {!Object[]} allSections the array containing the results
* @param {!Object} state some state to pass around
*/
-function visit(node, allSections, state) {
+function visit(logger, node, allSections, state) {
let sectionObj = allSections.length > 0 ? allSections[allSections.length -
1] : undefined;
if (node.tagName === 'SECTION') {
-// throwIfPreviousSectionIsIncomplete(allSections, sectionObj);
+validatePreviousSection(logger, allSections, sectionObj);
sectionObj = { id: getSectionNumber(node), text: '' };
allSections.push(sectionObj);
@@ -99,16 +102,17 @@
* This started out with a traditional iterative DFS algorithm but added a
boolean to stop
* descending so we don't duplicate content since elsewhere outerHTML is used
(which contains
* content of sub-nodes).
+ * @param {!Logger} logger the app's bunyan logger
* @param {!DOMElement} rootElemen
[MediaWiki-commits] [Gerrit] mediawiki...mobileapps[master]: Update section validation logic
jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/398082 )
Change subject: Update section validation logic
..
Update section validation logic
1) Log warnings rather than throwing errors, and
2) Only for sections with IDs >= 0 (i.e., not non-editable or pseudo-
sections).
Bug: T182774
Change-Id: Icac8b8899b91401b5ef85e37a29d97f70b755eca
---
M lib/parsoid-access.js
M lib/parsoidSections.js
M lib/parsoidSectionsUsingSectionTags.js
M test/lib/parsoid/parsoid-sections-section-elements-tests.js
4 files changed, 57 insertions(+), 31 deletions(-)
Approvals:
BearND: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/parsoid-access.js b/lib/parsoid-access.js
index ba16da9..8633db3 100644
--- a/lib/parsoid-access.js
+++ b/lib/parsoid-access.js
@@ -129,7 +129,7 @@
transforms.stripUnneededMarkup(doc, legacy);
parsoidSections.addSectionDivs(doc);
-page.sections = parsoidSections.getSectionsText(doc);
+page.sections = parsoidSections.getSectionsText(doc, req.logger);
return page;
});
}
diff --git a/lib/parsoidSections.js b/lib/parsoidSections.js
index dcdbb72..f8e21d6 100644
--- a/lib/parsoidSections.js
+++ b/lib/parsoidSections.js
@@ -27,13 +27,14 @@
* Gets the sections array, including HTML string and metadata for all sections
* (id, anchor, line, toclevel).
* @param {!document} doc the parsed DOM Document of the Parsoid output
+ * @param {!Logger} logger the app's bunyan logger
* @return {!sections[]} an array of section JSON elements
*/
-function getSectionsText(doc) {
+function getSectionsText(doc, logger) {
if (!hasParsoidSections(doc)) {
return parsoidSectionsUsingDivs.getSectionsText(doc);
} else {
-return parsoidSectionsUsingSectionTags.getSectionsText(doc);
+return parsoidSectionsUsingSectionTags.getSectionsText(doc, logger);
}
}
diff --git a/lib/parsoidSectionsUsingSectionTags.js
b/lib/parsoidSectionsUsingSectionTags.js
index 76e471d..ce27178 100644
--- a/lib/parsoidSectionsUsingSectionTags.js
+++ b/lib/parsoidSectionsUsingSectionTags.js
@@ -1,8 +1,6 @@
'use strict';
const parsoidDomUtils = require('parsoid-dom-utils');
-// const sUtil = require('./util');
-// const HTTPError = sUtil.HTTPError;
const NodeType = require('./nodeType');
/**
@@ -39,7 +37,13 @@
}
}
-/* function throwIfPreviousSectionIsIncomplete(allSections, sectionObj) {
+// skip lead sections since they don't have headings;
+// no line and no anchor means incomplete section
+function shouldLogInvalidSectionWarning(sectionObj) {
+return sectionObj.id >= 1 && !sectionObj.line && !sectionObj.anchor;
+}
+
+function validatePreviousSection(logger, allSections, sectionObj) {
if (allSections.length === 0) {
return;
}
@@ -47,30 +51,29 @@
if (!sectionObj) {
sectionObj = allSections[allSections.length - 1];
}
-// skip lead sections since they don't have headings;
-// no line and no anchor means incomplete section
-if (allSections.length > 1 && !sectionObj.line && !sectionObj.anchor) {
-throw new HTTPError({
-status: 502,
-type: 'unsupported_section',
-title: 'Section structure not supported',
+
+if (shouldLogInvalidSectionWarning(sectionObj)) {
+logger.warn({
+warning: 'invalid_section',
+title: 'Found section without expected heading',
detail: `Cannot find heading for section number ${sectionObj.id}.`
});
}
-} */
+}
/**
* Visits one DOM node. Do the stuff that needs to be done when a single DOM
node is handled.
* In this case, starts a new section object when a tag is
encountered that is a
* direct descendant
+ * @param {!Logger} logger the app's bunyan logger
* @param {!DOMNode} node the node to visit
* @param {!Object[]} allSections the array containing the results
* @param {!Object} state some state to pass around
*/
-function visit(node, allSections, state) {
+function visit(logger, node, allSections, state) {
let sectionObj = allSections.length > 0 ? allSections[allSections.length -
1] : undefined;
if (node.tagName === 'SECTION') {
-// throwIfPreviousSectionIsIncomplete(allSections, sectionObj);
+validatePreviousSection(logger, allSections, sectionObj);
sectionObj = { id: getSectionNumber(node), text: '' };
allSections.push(sectionObj);
@@ -99,16 +102,17 @@
* This started out with a traditional iterative DFS algorithm but added a
boolean to stop
* descending so we don't duplicate content since elsewhere outerHTML is used
(which contains
* content of sub-nodes).
+ * @param {!Logger} logger the app's bunyan logger
* @param {!DOMElement} rootElement the root of the DOM tree which needs to be
traversed
* @param {!Object[]} allSections hol
[MediaWiki-commits] [Gerrit] mediawiki...mobileapps[master]: Update section validation logic
Mholloway has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/398082 )
Change subject: Update section validation logic
..
Update section validation logic
1) Log warnings rather than throwing errors, and
2) Only for sections with IDs >= 0 (i.e., not non-editable or pseudo-
sections).
Bug: T182774
Change-Id: Icac8b8899b91401b5ef85e37a29d97f70b755eca
---
M lib/parsoid-access.js
M lib/parsoidSections.js
M lib/parsoidSectionsUsingSectionTags.js
M test/lib/parsoid/parsoid-sections-section-elements-tests.js
4 files changed, 36 insertions(+), 27 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/mobileapps
refs/changes/82/398082/1
diff --git a/lib/parsoid-access.js b/lib/parsoid-access.js
index ba16da9..8633db3 100644
--- a/lib/parsoid-access.js
+++ b/lib/parsoid-access.js
@@ -129,7 +129,7 @@
transforms.stripUnneededMarkup(doc, legacy);
parsoidSections.addSectionDivs(doc);
-page.sections = parsoidSections.getSectionsText(doc);
+page.sections = parsoidSections.getSectionsText(doc, req.logger);
return page;
});
}
diff --git a/lib/parsoidSections.js b/lib/parsoidSections.js
index dcdbb72..f8e21d6 100644
--- a/lib/parsoidSections.js
+++ b/lib/parsoidSections.js
@@ -27,13 +27,14 @@
* Gets the sections array, including HTML string and metadata for all sections
* (id, anchor, line, toclevel).
* @param {!document} doc the parsed DOM Document of the Parsoid output
+ * @param {!Logger} logger the app's bunyan logger
* @return {!sections[]} an array of section JSON elements
*/
-function getSectionsText(doc) {
+function getSectionsText(doc, logger) {
if (!hasParsoidSections(doc)) {
return parsoidSectionsUsingDivs.getSectionsText(doc);
} else {
-return parsoidSectionsUsingSectionTags.getSectionsText(doc);
+return parsoidSectionsUsingSectionTags.getSectionsText(doc, logger);
}
}
diff --git a/lib/parsoidSectionsUsingSectionTags.js
b/lib/parsoidSectionsUsingSectionTags.js
index 76e471d..5582972 100644
--- a/lib/parsoidSectionsUsingSectionTags.js
+++ b/lib/parsoidSectionsUsingSectionTags.js
@@ -1,8 +1,6 @@
'use strict';
const parsoidDomUtils = require('parsoid-dom-utils');
-// const sUtil = require('./util');
-// const HTTPError = sUtil.HTTPError;
const NodeType = require('./nodeType');
/**
@@ -39,7 +37,11 @@
}
}
-/* function throwIfPreviousSectionIsIncomplete(allSections, sectionObj) {
+function shouldLogInvalidSectionWarning(allSections, sectionObj) {
+return allSections.length > 1 && sectionObj.id >= 0 && !sectionObj.line &&
!sectionObj.anchor;
+}
+
+function validatePreviousSection(logger, allSections, sectionObj) {
if (allSections.length === 0) {
return;
}
@@ -49,28 +51,28 @@
}
// skip lead sections since they don't have headings;
// no line and no anchor means incomplete section
-if (allSections.length > 1 && !sectionObj.line && !sectionObj.anchor) {
-throw new HTTPError({
-status: 502,
-type: 'unsupported_section',
-title: 'Section structure not supported',
+if (shouldLogInvalidSectionWarning(allSections, sectionObj)) {
+logger.warn({
+warning: 'invalid_section',
+title: 'Found section without expected heading',
detail: `Cannot find heading for section number ${sectionObj.id}.`
});
}
-} */
+}
/**
* Visits one DOM node. Do the stuff that needs to be done when a single DOM
node is handled.
* In this case, starts a new section object when a tag is
encountered that is a
* direct descendant
+ * @param {!Logger} logger the app's bunyan logger
* @param {!DOMNode} node the node to visit
* @param {!Object[]} allSections the array containing the results
* @param {!Object} state some state to pass around
*/
-function visit(node, allSections, state) {
+function visit(logger, node, allSections, state) {
let sectionObj = allSections.length > 0 ? allSections[allSections.length -
1] : undefined;
if (node.tagName === 'SECTION') {
-// throwIfPreviousSectionIsIncomplete(allSections, sectionObj);
+validatePreviousSection(logger, allSections, sectionObj);
sectionObj = { id: getSectionNumber(node), text: '' };
allSections.push(sectionObj);
@@ -99,16 +101,17 @@
* This started out with a traditional iterative DFS algorithm but added a
boolean to stop
* descending so we don't duplicate content since elsewhere outerHTML is used
(which contains
* content of sub-nodes).
+ * @param {!Logger} logger the app's bunyan logger
* @param {!DOMElement} rootElement the root of the DOM tree which needs to be
traversed
* @param {!Object[]} allSections holds the results
*/
-function traverseDF(rootElement, allSections) {
+function traverseDF(logger, rootElement, allSection
