Skip to content

Commit

Permalink
🚩 Add strict flag
Browse files Browse the repository at this point in the history
This change adds a `strict` flag to control whether we have the stricter
type checking introduced in #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.
  • Loading branch information
alecgibson committed Jul 12, 2021
1 parent b89153c commit 6c1dc82
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 38 deletions.
21 changes: 6 additions & 15 deletions lib/json0.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions lib/options.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
strict: false
};
3 changes: 2 additions & 1 deletion lib/text0.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand Down
65 changes: 43 additions & 22 deletions test/json0.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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', ->
Expand Down Expand Up @@ -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', ->
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 6c1dc82

Please sign in to comment.