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 <section> 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, allSections) {
const state = {};
let nodesToVisit = [ rootElement ];
while (nodesToVisit.length > 0) {
const currentNode = nodesToVisit.shift();
- visit(currentNode, allSections, state);
+ visit(logger, currentNode, allSections, state);
if (!state.pauseDescent) {
nodesToVisit = [
...(currentNode.childNodes || []), // depth first
@@ -117,12 +120,12 @@
}
}
ensureLeadSection(allSections);
- // throwIfPreviousSectionIsIncomplete(allSections);
+ validatePreviousSection(logger, allSections);
}
-function getSectionsText(doc) {
+function getSectionsText(doc, logger) {
const allSections = [];
- traverseDF(doc.body, allSections);
+ traverseDF(logger, doc.body, allSections);
return allSections;
}
@@ -130,6 +133,7 @@
addSectionTags,
getSectionsText,
testing: {
- parseSections: traverseDF
+ parseSections: traverseDF,
+ shouldLogInvalidSectionWarning
}
};
diff --git a/test/lib/parsoid/parsoid-sections-section-elements-tests.js
b/test/lib/parsoid/parsoid-sections-section-elements-tests.js
index 77cb023..0bc3dc5 100644
--- a/test/lib/parsoid/parsoid-sections-section-elements-tests.js
+++ b/test/lib/parsoid/parsoid-sections-section-elements-tests.js
@@ -3,6 +3,7 @@
const assert = require('../../utils/assert.js');
const domino = require('domino');
const parsoidSectionsUsingSectionTags =
require('../../../lib/parsoidSectionsUsingSectionTags');
+const shouldWarn =
parsoidSectionsUsingSectionTags.testing.shouldLogInvalidSectionWarning;
describe('lib:parsoid-sections (section elements)', function() {
/* eslint-disable prefer-template */
@@ -119,13 +120,16 @@
assertSection0(sections, sectionInDiv);
});
- it.skip('non-lead section without heading tag should throw error', () => {
- const doc = domino.createDocument(
- '<section data-mw-section-id="0">text0</section>' +
- '<section data-mw-section-id="1">text1</section>');
- assert.throws(() => {
- parsoidSectionsUsingSectionTags.getSectionsText(doc);
- }, /unsupported_section/);
+ it('should warn for non-lead section without heading tag', () => {
+ const allSections = [ { id: 0 }, { id: 1 } ];
+ // console.log(shouldWarn(allSections, allSections[1]));
+ assert.ok(shouldWarn(allSections, allSections[1]));
+ });
+
+ it('should not warn for non-lead -1 section without heading tag', () => {
+ const allSections = [ { id: 0 }, { id: -1 } ];
+ // console.log(!shouldWarn(allSections, allSections[1]));
+ assert.ok(!shouldWarn(allSections, allSections[1]));
});
/* eslint-enable prefer-template */
--
To view, visit https://gerrit.wikimedia.org/r/398082
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icac8b8899b91401b5ef85e37a29d97f70b755eca
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
