From 97819c38ec2bbea17a52a5dd412edb0fea1cc687 Mon Sep 17 00:00:00 2001 From: Alec Gibson <12036746+alecgibson@users.noreply.github.com> Date: Mon, 12 Jul 2021 11:26:18 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=A9=20Add=20`strict`=20flag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change adds a `strict` flag to control whether we have the stricter type checking introduced in https://github.com/ottypes/json0/pull/40 Strict mode will be off by default (to maintain compatibility with old `json0` versions). In order to add this flag, we also add a new `options` object to the `type`. Strict mode is enabled on the type by setting the flag: ```js type.options.strict = true ``` Note that `text0` will share the same options as `json0` (ie enabling strict mode for `json0` also enables it for `text0`). In this change we also tidy up some unused utility functions from a previous commit. --- README.md | 22 ++++++++++++---- lib/json0.js | 21 +++++---------- lib/options.js | 3 +++ lib/text0.js | 3 ++- test/json0.coffee | 65 +++++++++++++++++++++++++++++++---------------- 5 files changed, 71 insertions(+), 43 deletions(-) create mode 100644 lib/options.js diff --git a/README.md b/README.md index cc1de54..c3d4f60 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ Lists and objects have the same set of operations (*Insert*, *Delete*, *Replace*, *Move*) but their semantics are very different. List operations shuffle adjacent list items left or right to make space (or to remove space). Object operations do not. You should pick the data structure which will give -you the behaviour you want when you design your data model. +you the behaviour you want when you design your data model. To make it clear what the semantics of operations will be, list operations and object operations are named differently. (`li`, `ld`, `lm` for lists and `oi`, @@ -201,9 +201,9 @@ There is (unfortunately) no equivalent for list move with objects. ### Subtype operations Usage: - + {p:PATH, t:SUBTYPE, o:OPERATION} - + `PATH` is the path to the object that will be modified by the subtype. `SUBTYPE` is the name of the subtype, e.g. `"text0"`. `OPERATION` is the subtype operation itself. @@ -242,7 +242,7 @@ the subtype operation. Usage: {p:PATH, t:'text0', o:[{p:OFFSET, i:TEXT}]} - + Insert `TEXT` to the string specified by `PATH` at the position specified by `OFFSET`. ##### Delete from a string @@ -250,7 +250,7 @@ Insert `TEXT` to the string specified by `PATH` at the position specified by `OF Usage: {p:PATH, t:'text0', o:[{p:OFFSET, d:TEXT}]} - + Delete `TEXT` in the string specified by `PATH` at the position specified by `OFFSET`. --- @@ -292,6 +292,18 @@ Usage: Delete `TEXT` at the location specified by `PATH`. The path must specify an offset in a string. `TEXT` must be contained at the location specified. +## Options + +### Strict + +`json0` supports a "strict" mode which performs additional type checking of the +ops on `apply()`. This mode is off by default to preserve backwards-compatibility, +but can be enabled by setting the flag: + +```js +type.options.strict = true +``` + --- # Commentary diff --git a/lib/json0.js b/lib/json0.js index a93ffe4..678f478 100644 --- a/lib/json0.js +++ b/lib/json0.js @@ -43,23 +43,14 @@ var clone = function(o) { return JSON.parse(JSON.stringify(o)); }; -var validateListIndex = function(key) { - if (typeof key !== 'number') - throw new Error('List index must be a number'); -}; - -var validateObjectKey = function (key) { - if (typeof key !== 'string') - throw new Error('Object key must be a number'); -}; - /** * JSON OT Type * @type {*} */ var json = { name: 'json0', - uri: 'http://sharejs.org/types/JSONv0' + uri: 'http://sharejs.org/types/JSONv0', + options: require('./options') }; // You can register another OT type as a subtype in a JSON document using @@ -172,10 +163,10 @@ json.apply = function(snapshot, op) { elem = elem[key]; key = p; - if (isArray(elem) && typeof key !== 'number') + if (json.options.strict && isArray(elem) && typeof key !== 'number') throw new Error('List index must be a number'); - if (isObject(elem) && typeof key !== 'string') + if (json.options.strict && isObject(elem) && typeof key !== 'string') throw new Error('Object key must be a string'); if (parent == null) @@ -191,7 +182,7 @@ json.apply = function(snapshot, op) { if (typeof elem[key] != 'number') throw new Error('Referenced element not a number'); - if (typeof c.na !== 'number') + if (json.options.strict && typeof c.na !== 'number') throw new Error('Number addition is not a number'); elem[key] += c.na; @@ -219,7 +210,7 @@ json.apply = function(snapshot, op) { // List move else if (c.lm !== void 0) { - if (typeof c.lm !== 'number') + if (json.options.strict && typeof c.lm !== 'number') throw new Error('List move target index must be a number'); json.checkList(elem); diff --git a/lib/options.js b/lib/options.js new file mode 100644 index 0000000..a45db2d --- /dev/null +++ b/lib/options.js @@ -0,0 +1,3 @@ +module.exports = { + strict: false +}; diff --git a/lib/text0.js b/lib/text0.js index 238d448..d73f83f 100644 --- a/lib/text0.js +++ b/lib/text0.js @@ -24,6 +24,7 @@ var text = module.exports = { name: 'text0', uri: 'http://sharejs.org/types/textv0', + options: require('./options'), create: function(initial) { if ((initial != null) && typeof initial !== 'string') { throw new Error('Initial data must be a string'); @@ -61,7 +62,7 @@ text.apply = function(snapshot, op) { var deleted; var type = typeof snapshot; - if (type !== 'string') + if (text.options.strict && type !== 'string') throw new Error('text0 operations cannot be applied to type: ' + type); checkValidOp(op); diff --git a/test/json0.coffee b/test/json0.coffee index e2ee6df..af4b416 100644 --- a/test/json0.coffee +++ b/test/json0.coffee @@ -64,11 +64,18 @@ genTests = (type) -> c_s = type.apply leftHas, right_ assert.deepEqual s_c, c_s - it 'throws when adding a string to a number', -> - assert.throws -> type.apply 1, [{p: [], na: 'a'}] + describe 'strict', -> + beforeEach -> + type.options.strict = true - it 'throws when adding a number to a string', -> - assert.throws -> type.apply 'a', [{p: [], na: 1}] + afterEach -> + type.options.strict = false + + it 'throws when adding a string to a number', -> + assert.throws -> type.apply 1, [{p: [], na: 'a'}] + + it 'throws when adding a number to a string', -> + assert.throws -> type.apply 'a', [{p: [], na: 1}] # Strings should be handled internally by the text type. We'll just do some basic sanity checks here. describe 'string', -> @@ -138,23 +145,30 @@ genTests = (type) -> assert.deepEqual ['a', 'b', 'c'], type.apply ['b', 'a', 'c'], [{p:[1], lm:0}] assert.deepEqual ['a', 'b', 'c'], type.apply ['b', 'a', 'c'], [{p:[0], lm:1}] - it 'throws when keying a list replacement with a string', -> - assert.throws -> type.apply ['a', 'b', 'c'], [{p: ['0'], li: 'x', ld: 'a'}] + describe 'strict', -> + beforeEach -> + type.options.strict = true + + afterEach -> + type.options.strict = false - it 'throws when keying a list insertion with a string', -> - assert.throws -> type.apply ['a', 'b', 'c'], [{p: ['0'], li: 'x'}] + it 'throws when keying a list replacement with a string', -> + assert.throws -> type.apply ['a', 'b', 'c'], [{p: ['0'], li: 'x', ld: 'a'}] - it 'throws when keying a list deletion with a string', -> - assert.throws -> type.apply ['a', 'b', 'c'], [{p: ['0'], ld: 'a'}] + it 'throws when keying a list insertion with a string', -> + assert.throws -> type.apply ['a', 'b', 'c'], [{p: ['0'], li: 'x'}] - it 'throws when keying a list move with a string', -> - assert.throws -> type.apply ['a', 'b', 'c'], [{p: ['0'], lm: 0}] + it 'throws when keying a list deletion with a string', -> + assert.throws -> type.apply ['a', 'b', 'c'], [{p: ['0'], ld: 'a'}] - it 'throws when specifying a string as a list move target', -> - assert.throws -> type.apply ['a', 'b', 'c'], [{p: [1], lm: '0'}] + it 'throws when keying a list move with a string', -> + assert.throws -> type.apply ['a', 'b', 'c'], [{p: ['0'], lm: 0}] - it 'throws when an array index part-way through the path is a string', -> - assert.throws -> type.apply {arr:[{x:'a'}]}, [{p:['arr', '0', 'x'], od: 'a'}] + it 'throws when specifying a string as a list move target', -> + assert.throws -> type.apply ['a', 'b', 'c'], [{p: [1], lm: '0'}] + + it 'throws when an array index part-way through the path is a string', -> + assert.throws -> type.apply {arr:[{x:'a'}]}, [{p:['arr', '0', 'x'], od: 'a'}] ### 'null moves compose to nops', -> @@ -418,14 +432,21 @@ genTests = (type) -> assert.deepEqual [], type.transform [{p:['k'], od:'x'}], [{p:['k'], od:'x'}], 'left' assert.deepEqual [], type.transform [{p:['k'], od:'x'}], [{p:['k'], od:'x'}], 'right' - it 'throws when the insertion key is a number', -> - assert.throws -> type.apply {'1':'a'}, [{p:[2], oi: 'a'}] + describe 'strict', -> + beforeEach -> + type.options.strict = true + + afterEach -> + type.options.strict = false + + it 'throws when the insertion key is a number', -> + assert.throws -> type.apply {'1':'a'}, [{p:[2], oi: 'a'}] - it 'throws when the deletion key is a number', -> - assert.throws -> type.apply {'1':'a'}, [{p:[1], od: 'a'}] + it 'throws when the deletion key is a number', -> + assert.throws -> type.apply {'1':'a'}, [{p:[1], od: 'a'}] - it 'throws when an object key part-way through the path is a number', -> - assert.throws -> type.apply {'1': {x: 'a'}}, [{p:[1, 'x'], od: 'a'}] + it 'throws when an object key part-way through the path is a number', -> + assert.throws -> type.apply {'1': {x: 'a'}}, [{p:[1, 'x'], od: 'a'}] describe 'randomizer', -> @timeout 20000