Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🚩 Add strict flag #42

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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