From 746b63db7e9640cfe9d852be0cc9c8506fd1968d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20M=C4=99drzycki?= Date: Thu, 8 Nov 2018 22:05:59 +0100 Subject: [PATCH] Improve schema validation for nodes and min/max (#2388) * Better schema violations for underflow and overflow * Rename child_required_under/overflow to child_min/max_invalid * Remove tailing whitespace * Add tests to cover more branches * Insert missing comma --- docs/reference/slate/schema.md | 21 ++- examples/forced-layout/index.js | 2 +- packages/slate/src/plugins/core.js | 2 +- packages/slate/src/plugins/schema.js | 151 ++++++++++++++---- .../child-max-invalid-at-end-default.js | 44 +++++ .../schema/custom/child-max-invalid-custom.js | 52 ++++++ .../custom/child-max-invalid-default.js | 47 ++++++ ....js => child-min-invalid-at-end-custom.js} | 2 +- ...js => child-min-invalid-at-end-default.js} | 0 .../schema/custom/child-min-invalid-custom.js | 56 +++++++ .../custom/child-min-invalid-default.js | 39 +++++ .../child-min-invalid-with-invalid-default.js | 59 +++++++ .../schema/custom/child-unknown-custom.js | 2 +- .../schema/custom/child-unknown-default.js | 2 +- 14 files changed, 439 insertions(+), 40 deletions(-) create mode 100644 packages/slate/test/schema/custom/child-max-invalid-at-end-default.js create mode 100644 packages/slate/test/schema/custom/child-max-invalid-custom.js create mode 100644 packages/slate/test/schema/custom/child-max-invalid-default.js rename packages/slate/test/schema/custom/{child-required-custom.js => child-min-invalid-at-end-custom.js} (95%) rename packages/slate/test/schema/custom/{child-required-default.js => child-min-invalid-at-end-default.js} (100%) create mode 100644 packages/slate/test/schema/custom/child-min-invalid-custom.js create mode 100644 packages/slate/test/schema/custom/child-min-invalid-default.js create mode 100644 packages/slate/test/schema/custom/child-min-invalid-with-invalid-default.js diff --git a/docs/reference/slate/schema.md b/docs/reference/slate/schema.md index 7175bffd2..7c47c9e75 100644 --- a/docs/reference/slate/schema.md +++ b/docs/reference/slate/schema.md @@ -303,17 +303,34 @@ When supplying your own `normalize` property for a schema rule, it will be calle Raised when the `object` property of a child node is invalid. -### `'child_required'` +### `child_min_invalid` ```js { index: Number, + count: Number, + limit: Number, node: Node, rule: Object, } ``` -Raised when a child node was required but none was found. +Raised when a child node repeats less than required by a rule's `min` property. + +### `child_max_invalid` + +```js +{ + index: Number, + count: Number, + limit: Number, + node: Node, + rule: Object, +} +``` + +Raised when a child node repeats more than permitted by a rule's `max` +property. ### `'child_type_invalid'` diff --git a/examples/forced-layout/index.js b/examples/forced-layout/index.js index a75e6c065..8241405f5 100644 --- a/examples/forced-layout/index.js +++ b/examples/forced-layout/index.js @@ -22,7 +22,7 @@ const schema = { const type = index === 0 ? 'title' : 'paragraph' return editor.setNodeByKey(child.key, type) } - case 'child_required': { + case 'child_min_invalid': { const block = Block.create(index === 0 ? 'title' : 'paragraph') return editor.insertNodeByKey(node.key, index, block) } diff --git a/packages/slate/src/plugins/core.js b/packages/slate/src/plugins/core.js index f6396acd5..4e91638ba 100644 --- a/packages/slate/src/plugins/core.js +++ b/packages/slate/src/plugins/core.js @@ -100,7 +100,7 @@ function CorePlugin(options = {}) { normalize: (editor, error) => { const { code, node } = error - if (code === 'child_required') { + if (code === 'child_min_invalid' && node.nodes.isEmpty()) { editor.insertNodeByKey(node.key, 0, Text.create()) } }, diff --git a/packages/slate/src/plugins/schema.js b/packages/slate/src/plugins/schema.js index cdd7adffb..ab62a0ad4 100644 --- a/packages/slate/src/plugins/schema.js +++ b/packages/slate/src/plugins/schema.js @@ -157,7 +157,7 @@ function SchemaPlugin(schema) { */ function defaultNormalize(editor, error) { - const { code, node, child, next, previous, key, mark } = error + const { code, node, child, next, previous, key, mark, index } = error switch (code) { case 'child_object_invalid': @@ -192,7 +192,7 @@ function defaultNormalize(editor, error) { : editor.removeNodeByKey(next.key) } - case 'child_required': + case 'child_min_invalid': case 'node_text_invalid': case 'parent_object_invalid': case 'parent_type_invalid': { @@ -201,6 +201,10 @@ function defaultNormalize(editor, error) { : editor.removeNodeByKey(node.key) } + case 'child_max_invalid': { + return editor.removeNodeByKey(node.nodes.get(index).key) + } + case 'node_data_invalid': { return node.data.get(key) === undefined && node.object !== 'document' ? editor.removeNodeByKey(node.key) @@ -359,36 +363,42 @@ function validateNodes(node, rule, rules = []) { const children = node.nodes.toArray() const defs = rule.nodes != null ? rule.nodes.slice() : [] - let offset - let min - let index - let def - let max - let child - let previous - let next + let count = 0 + let lastCount = 0 + let min = null + let index = -1 + let def = null + let max = null + let child = null + let previous = null + let next = null function nextDef() { - offset = offset == null ? null : 0 + if (defs.length === 0) return false def = defs.shift() - min = def && def.min - max = def && def.max - return !!def + lastCount = count + count = 0 + min = def.min || null + max = def.max || null + return true } function nextChild() { - index = index == null ? 0 : index + 1 - offset = offset == null ? 0 : offset + 1 + index += 1 previous = child child = children[index] next = children[index + 1] - if (max != null && offset == max) nextDef() - return !!child + if (!child) return false + lastCount = count + count += 1 + return true } - function rewind() { - offset -= 1 - index -= 1 + function rewind(pastZero = false) { + if (index > 0 || pastZero) { + index -= 1 + count = lastCount + } } if (rule.nodes != null) { @@ -411,12 +421,71 @@ function validateNodes(node, rule, rules = []) { if (def.match) { const error = validateRules(child, def.match) - if (error && offset >= min && nextDef()) { - rewind() - continue - } - if (error) { + if (max != null && count - 1 > max) { + // Since we want to report overflow on last matching child we don't + // immediately check for count > max, but instead do so once we find + // a child that doesn't match. + rewind() + return fail('child_max_invalid', { + rule, + node, + index, + count, + limit: max, + }) + } + + const lastMin = min + + // If there are more groups after this one then child might actually + // be valid. + if (nextDef()) { + // We already have all children required for current group, so this + // error can safely be ignored. + if (lastCount - 1 >= lastMin) { + rewind(true) + continue + } + + // Otherwise we know that current value is underflowing. There are + // three possible causes: there might just not be enough elements + // for current group, and current child is in fact the first of + // the next group; current group is underflowing, but there is also + // an invalid child before the next group; or current group is not + // underflowing but it appears so because there's an invalid child + // between its members. + if (validateRules(child, def.match) == null) { + // It's the first case, so we just report an underflow. + rewind() + return fail('child_min_invalid', { + rule, + node, + index, + count: lastCount - 1, + limit: lastMin, + }) + } + // It's either the second or third case. If it's the second then + // we could report an underflow, but presence of an invalid child + // is arguably more important, so we report it first. It also lets + // us avoid checking for which case exactly is it. + + error.rule = rule + error.node = node + error.child = child + error.index = index + error.code = error.code.replace('node_', 'child_') + return error + } + + // Otherwise either we exhausted the last group, in which case it's + // an unknown child, ... + if (max != null && count > max) { + return fail('child_unknown', { rule, node, child, index }) + } + + // ... or it's an invalid child for the last group. error.rule = rule error.node = node error.child = child @@ -428,14 +497,30 @@ function validateNodes(node, rule, rules = []) { } } - if (rule.nodes != null) { - while (min != null) { - if (offset < min) { - return fail('child_required', { rule, node, index }) - } + if (max != null && count > max) { + // Since we want to report overflow on last matching child we don't + // immediately check for count > max, but do so after processing all nodes. + return fail('child_max_invalid', { + rule, + node, + index: index - 1, + count, + limit: max, + }) + } - nextDef() - } + if (rule.nodes != null) { + do { + if (count < min) { + return fail('child_min_invalid', { + rule, + node, + index, + count, + limit: min, + }) + } + } while (nextDef()) } } diff --git a/packages/slate/test/schema/custom/child-max-invalid-at-end-default.js b/packages/slate/test/schema/custom/child-max-invalid-at-end-default.js new file mode 100644 index 000000000..5fe55d446 --- /dev/null +++ b/packages/slate/test/schema/custom/child-max-invalid-at-end-default.js @@ -0,0 +1,44 @@ +/** @jsx h */ + +import h from '../../helpers/h' + +export const schema = { + blocks: { + paragraph: {}, + quote: { + nodes: [ + { + match: [{ type: 'paragraph' }], + max: 1, + }, + ], + }, + }, +} + +export const input = ( + + + + + + + + + + + + +) + +export const output = ( + + + + + + + + + +) diff --git a/packages/slate/test/schema/custom/child-max-invalid-custom.js b/packages/slate/test/schema/custom/child-max-invalid-custom.js new file mode 100644 index 000000000..de9958b28 --- /dev/null +++ b/packages/slate/test/schema/custom/child-max-invalid-custom.js @@ -0,0 +1,52 @@ +/** @jsx h */ + +import h from '../../helpers/h' + +export const schema = { + blocks: { + paragraph: {}, + quote: { + nodes: [ + { + match: [{ type: 'title' }], + max: 1, + }, + { + match: [{ type: 'paragraph' }], + }, + ], + normalize: (editor, { code, node, index }) => { + if (code == 'child_max_invalid') { + editor.mergeNodeByKey(node.nodes.get(index).key) + } + }, + }, + }, +} + +export const input = ( + + + + One + Two + + + + + + +) + +export const output = ( + + + + OneTwo + + + + + + +) diff --git a/packages/slate/test/schema/custom/child-max-invalid-default.js b/packages/slate/test/schema/custom/child-max-invalid-default.js new file mode 100644 index 000000000..49c31b68a --- /dev/null +++ b/packages/slate/test/schema/custom/child-max-invalid-default.js @@ -0,0 +1,47 @@ +/** @jsx h */ + +import h from '../../helpers/h' + +export const schema = { + blocks: { + paragraph: {}, + quote: { + nodes: [ + { + match: [{ type: 'title' }], + max: 1, + }, + { + match: [{ type: 'paragraph' }], + }, + ], + }, + }, +} + +export const input = ( + + + + One + Two + + + + + + +) + +export const output = ( + + + + One + + + + + + +) diff --git a/packages/slate/test/schema/custom/child-required-custom.js b/packages/slate/test/schema/custom/child-min-invalid-at-end-custom.js similarity index 95% rename from packages/slate/test/schema/custom/child-required-custom.js rename to packages/slate/test/schema/custom/child-min-invalid-at-end-custom.js index 1b91aad90..0f398f6f0 100644 --- a/packages/slate/test/schema/custom/child-required-custom.js +++ b/packages/slate/test/schema/custom/child-min-invalid-at-end-custom.js @@ -13,7 +13,7 @@ export const schema = { }, ], normalize: (editor, { code, node, index }) => { - if (code == 'child_required') { + if (code == 'child_min_invalid') { editor.insertNodeByKey(node.key, index, { object: 'block', type: 'paragraph', diff --git a/packages/slate/test/schema/custom/child-required-default.js b/packages/slate/test/schema/custom/child-min-invalid-at-end-default.js similarity index 100% rename from packages/slate/test/schema/custom/child-required-default.js rename to packages/slate/test/schema/custom/child-min-invalid-at-end-default.js diff --git a/packages/slate/test/schema/custom/child-min-invalid-custom.js b/packages/slate/test/schema/custom/child-min-invalid-custom.js new file mode 100644 index 000000000..8313a860f --- /dev/null +++ b/packages/slate/test/schema/custom/child-min-invalid-custom.js @@ -0,0 +1,56 @@ +/** @jsx h */ + +import h from '../../helpers/h' + +export const schema = { + blocks: { + paragraph: {}, + title: {}, + quote: { + nodes: [ + { + match: [{ type: 'title' }], + min: 1, + }, + { + match: [{ type: 'paragraph' }], + }, + ], + normalize: (editor, { code, node, index }) => { + if (code == 'child_min_invalid' && index == 0) { + editor.insertNodeByKey(node.key, index, { + object: 'block', + type: 'title', + }) + } + }, + }, + }, +} + +export const input = ( + + + + + + + + + +) + +export const output = ( + + + + + + + + + + + + +) diff --git a/packages/slate/test/schema/custom/child-min-invalid-default.js b/packages/slate/test/schema/custom/child-min-invalid-default.js new file mode 100644 index 000000000..d78cce964 --- /dev/null +++ b/packages/slate/test/schema/custom/child-min-invalid-default.js @@ -0,0 +1,39 @@ +/** @jsx h */ + +import h from '../../helpers/h' + +export const schema = { + blocks: { + paragraph: {}, + title: {}, + quote: { + nodes: [ + { + match: [{ type: 'title' }], + min: 1, + }, + { + match: [{ type: 'paragraph' }], + }, + ], + }, + }, +} + +export const input = ( + + + + + + + + + +) + +export const output = ( + + + +) diff --git a/packages/slate/test/schema/custom/child-min-invalid-with-invalid-default.js b/packages/slate/test/schema/custom/child-min-invalid-with-invalid-default.js new file mode 100644 index 000000000..d97cc63e6 --- /dev/null +++ b/packages/slate/test/schema/custom/child-min-invalid-with-invalid-default.js @@ -0,0 +1,59 @@ +/** @jsx h */ + +import h from '../../helpers/h' + +export const schema = { + blocks: { + paragraph: {}, + quote: { + nodes: [ + { + match: [{ type: 'paragraph' }], + min: 2, + }, + { + match: [{ type: 'final' }], + }, + ], + }, + }, +} + +export const input = ( + + + + + + + + + + + + + + + + + + +) + +export const output = ( + + + + + + + + + + + + + + + +) diff --git a/packages/slate/test/schema/custom/child-unknown-custom.js b/packages/slate/test/schema/custom/child-unknown-custom.js index ba61aff84..eefd2d653 100644 --- a/packages/slate/test/schema/custom/child-unknown-custom.js +++ b/packages/slate/test/schema/custom/child-unknown-custom.js @@ -33,7 +33,7 @@ export const input = ( one - two + two diff --git a/packages/slate/test/schema/custom/child-unknown-default.js b/packages/slate/test/schema/custom/child-unknown-default.js index d1fd166d3..7629d2160 100644 --- a/packages/slate/test/schema/custom/child-unknown-default.js +++ b/packages/slate/test/schema/custom/child-unknown-default.js @@ -21,7 +21,7 @@ export const input = ( one - two + two