Skip to content

Commit 99885c3

Browse files
Alec Gibsonalecgibson
authored andcommitted
Throw when array and object deletion values do not match current version
This change adds validation to array and object deletion. If the values provided in either `ld` or `od` do not match the current value, then `apply` will `throw`. It will also throw if `oi` overwrites an existing value without providing `od`. The primary motivation of this change is to ensure that all submitted ops remain reversible. At the moment, the following series of ops is possible: - start with `{ foo: 'bar' }` - `apply` this op: `{ p: ['foo'], oi: 'baz' }` - ...resulting in `{ foo: 'baz' }` - `invert` the previous op: `{ p: ['foo'], od: 'baz' }` - and `apply` the inverted op: `{}` When I apply, invert and apply, I should always wind up where I started, but in this case I clearly do not. By ensuring that the `od` matches the current value, we make sure that all ops remain reversible. Deep equality adds a dependency on [`fast-deep-equal`][1]. [1]: https://github.com/epoberezkin/fast-deep-equal
1 parent b89153c commit 99885c3

File tree

3 files changed

+30
-9
lines changed

3 files changed

+30
-9
lines changed

lib/json0.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
var deepEqual = require('fast-deep-equal');
2+
13
/*
24
This is the implementation of the JSON OT type.
35
@@ -200,7 +202,8 @@ json.apply = function(snapshot, op) {
200202
// List replace
201203
else if (c.li !== void 0 && c.ld !== void 0) {
202204
json.checkList(elem);
203-
// Should check the list element matches c.ld
205+
if (!deepEqual(elem[key], c.ld))
206+
throw new Error('List deletion value does not match current value')
204207
elem[key] = c.li;
205208
}
206209

@@ -213,7 +216,8 @@ json.apply = function(snapshot, op) {
213216
// List delete
214217
else if (c.ld !== void 0) {
215218
json.checkList(elem);
216-
// Should check the list element matches c.ld here too.
219+
if (!deepEqual(elem[key], c.ld))
220+
throw new Error('List deletion value does not match current value')
217221
elem.splice(key,1);
218222
}
219223

@@ -236,15 +240,17 @@ json.apply = function(snapshot, op) {
236240
else if (c.oi !== void 0) {
237241
json.checkObj(elem);
238242

239-
// Should check that elem[key] == c.od
243+
if (!deepEqual(elem[key], c.od))
244+
throw new Error('Object deletion value does not match current value')
240245
elem[key] = c.oi;
241246
}
242247

243248
// Object delete
244249
else if (c.od !== void 0) {
245250
json.checkObj(elem);
246251

247-
// Should check that elem[key] == c.od
252+
if (!deepEqual(elem[key], c.od))
253+
throw new Error('Object deletion value does not match current value')
248254
delete elem[key];
249255
}
250256

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
"directories": {
77
"test": "test"
88
},
9-
"dependencies": {},
9+
"dependencies": {
10+
"fast-deep-equal": "^2.0.1"
11+
},
1012
"devDependencies": {
1113
"coffee-script": "^1.7.1",
1214
"mocha": "^9.0.2",

test/json0.coffee

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,14 @@ genTests = (type) ->
7272

7373
# Strings should be handled internally by the text type. We'll just do some basic sanity checks here.
7474
describe 'string', ->
75-
describe '#apply()', -> it 'works', ->
76-
assert.deepEqual 'abc', type.apply 'a', [{p:[1], si:'bc'}]
77-
assert.deepEqual 'bc', type.apply 'abc', [{p:[0], sd:'a'}]
78-
assert.deepEqual {x:'abc'}, type.apply {x:'a'}, [{p:['x', 1], si:'bc'}]
75+
describe '#apply()', ->
76+
it 'works', ->
77+
assert.deepEqual 'abc', type.apply 'a', [{p:[1], si:'bc'}]
78+
assert.deepEqual 'bc', type.apply 'abc', [{p:[0], sd:'a'}]
79+
assert.deepEqual {x:'abc'}, type.apply {x:'a'}, [{p:['x', 1], si:'bc'}]
80+
81+
it 'throws when the deletion target does not match', ->
82+
assert.throws -> type.apply 'abc', [{p:[0], sd:'x'}]
7983

8084
it 'throws when the target is not a string', ->
8185
assert.throws -> type.apply [1], [{p: [0], si: 'a'}]
@@ -138,6 +142,10 @@ genTests = (type) ->
138142
assert.deepEqual ['a', 'b', 'c'], type.apply ['b', 'a', 'c'], [{p:[1], lm:0}]
139143
assert.deepEqual ['a', 'b', 'c'], type.apply ['b', 'a', 'c'], [{p:[0], lm:1}]
140144

145+
it 'throws when the deletion target does not match', ->
146+
assert.throws -> type.apply ['a', 'b', 'c'], [{p:[0], ld: 'x'}]
147+
assert.throws -> type.apply ['a', 'b', 'c'], [{p:[0], li: 'd', ld: 'x'}]
148+
141149
it 'throws when keying a list replacement with a string', ->
142150
assert.throws -> type.apply ['a', 'b', 'c'], [{p: ['0'], li: 'x', ld: 'a'}]
143151

@@ -418,6 +426,11 @@ genTests = (type) ->
418426
assert.deepEqual [], type.transform [{p:['k'], od:'x'}], [{p:['k'], od:'x'}], 'left'
419427
assert.deepEqual [], type.transform [{p:['k'], od:'x'}], [{p:['k'], od:'x'}], 'right'
420428

429+
it 'throws when the deletion target does not match', ->
430+
assert.throws -> type.apply {x:'a'}, [{p:['x'], od: 'b'}]
431+
assert.throws -> type.apply {x:'a'}, [{p:['x'], oi: 'c', od: 'b'}]
432+
assert.throws -> type.apply {x:'a'}, [{p:['x'], oi: 'b'}]
433+
421434
it 'throws when the insertion key is a number', ->
422435
assert.throws -> type.apply {'1':'a'}, [{p:[2], oi: 'a'}]
423436

0 commit comments

Comments
 (0)