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 97819c3
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 43 deletions.
22 changes: 17 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -242,15 +242,15 @@ 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

Usage:

{p:PATH, t:'text0', o:[{p:OFFSET, d:TEXT}]}

Delete `TEXT` in the string specified by `PATH` at the position specified by `OFFSET`.

---
Expand Down Expand Up @@ -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
Expand Down
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 97819c3

Please sign in to comment.