Skip to content

Commit

Permalink
[BUGFIX beta] Mutating an arranged ArrayProxy is not allowed
Browse files Browse the repository at this point in the history
  • Loading branch information
@mmun
mmun committed Jan 20, 2018
1 parent f91a8da commit a73f809
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 244 deletions.
13 changes: 0 additions & 13 deletions packages/ember-runtime/lib/mixins/array.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,19 +270,6 @@ const ArrayMixin = Mixin.create(Enumerable, {
return indexes.map(idx => objectAt(this, idx));
},

/**
@method nextObject
@param {Number} index the current index of the iteration
@param {Object} previousObject the value returned by the last call to
`nextObject`.
@param {Object} context a context object you can use to maintain state.
@return {Object} the next object in the iteration or undefined
@private
*/
nextObject(idx) {
return objectAt(this, idx);
},

/**
This is the handler for the special array content property. If you get
this property, it will return this. If you set this property to a new
Expand Down
123 changes: 8 additions & 115 deletions packages/ember-runtime/lib/system/array_proxy.js
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
/**
@module @ember/array
*/

import {
get,
computed,
_beforeObserver,
observer,
beginPropertyChanges,
endPropertyChanges,
alias
} from 'ember-metal';
import {
Expand All @@ -17,15 +19,7 @@ import {
removeArrayObserver,
objectAt
} from '../mixins/array';
import { assert, Error as EmberError } from 'ember-debug';

/**
@module @ember/array
*/

const OUT_OF_RANGE_EXCEPTION = 'Index out of range';
const EMPTY = [];

import { assert } from 'ember-debug';

/**
An ArrayProxy wraps any other object that implements `Array` and/or
Expand Down Expand Up @@ -182,110 +176,9 @@ export default EmberObject.extend(MutableArray, {
// No dependencies since Enumerable notifies length of change
}),

_replace(idx, amt, objects) {
let content = get(this, 'content');
assert(`The content property of ${this.constructor} should be set before modifying it`, content);
if (content) {
this.replaceContent(idx, amt, objects);
}

return this;
},

replace() {
if (get(this, 'arrangedContent') === get(this, 'content')) {
this._replace(...arguments);
} else {
throw new EmberError('Using replace on an arranged ArrayProxy is not allowed.');
}
},

_insertAt(idx, object) {
if (idx > get(this, 'content.length')) {
throw new EmberError(OUT_OF_RANGE_EXCEPTION);
}

this._replace(idx, 0, [object]);
return this;
},

insertAt(idx, object) {
if (get(this, 'arrangedContent') === get(this, 'content')) {
return this._insertAt(idx, object);
} else {
throw new EmberError('Using insertAt on an arranged ArrayProxy is not allowed.');
}
},

removeAt(start, len) {
if ('number' === typeof start) {
let content = get(this, 'content');
let arrangedContent = get(this, 'arrangedContent');
let indices = [];

if ((start < 0) || (start >= get(this, 'length'))) {
throw new EmberError(OUT_OF_RANGE_EXCEPTION);
}

if (len === undefined) {
len = 1;
}

// Get a list of indices in original content to remove
for (let i = start; i < start + len; i++) {
// Use arrangedContent here so we avoid confusion with objects transformed by objectAtContent
indices.push(content.indexOf(objectAt(arrangedContent, i)));
}

// Replace in reverse order since indices will change
indices.sort((a, b) => b - a);

beginPropertyChanges();
for (let i = 0; i < indices.length; i++) {
this._replace(indices[i], 1, EMPTY);
}
endPropertyChanges();
}

return this;
},

pushObject(obj) {
this._insertAt(get(this, 'content.length'), obj);
return obj;
},

pushObjects(objects) {
if (!isArray(objects)) {
throw new TypeError('Must pass Enumerable to MutableArray#pushObjects');
}
this._replace(get(this, 'length'), 0, objects);
return this;
},

setObjects(objects) {
if (objects.length === 0) {
return this.clear();
}

let len = get(this, 'length');
this._replace(0, len, objects);
return this;
},

unshiftObject(obj) {
this._insertAt(0, obj);
return obj;
},

unshiftObjects(objects) {
this._replace(0, 0, objects);
return this;
},

slice() {
let arr = this.toArray();
return arr.slice(...arguments);
replace(idx, amt, objects) {
assert('Mutating an arranged ArrayProxy is not allowed', get(this, 'arrangedContent') === get(this, 'content') );
this.replaceContent(idx, amt, objects);
},

arrangedContentArrayWillChange(item, idx, removedCnt, addedCnt) {
Expand Down
1 change: 0 additions & 1 deletion packages/ember-runtime/tests/mixins/array_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ QUnit.test('slice supports negative index arguments', function(assert) {
//

const DummyArray = EmberObject.extend(EmberArray, {
nextObject() {},
length: 0,
objectAt(idx) { return 'ITEM-' + idx; }
});
Expand Down
159 changes: 44 additions & 115 deletions packages/ember-runtime/tests/system/array_proxy/arranged_content_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,6 @@ QUnit.module('ArrayProxy - arrangedContent', {
}
});

QUnit.test('addObject - adds to end of \'content\' if not present', function(assert) {
run(() => array.addObject(3));

assert.deepEqual(array.get('content'), [1, 2, 4, 5, 3], 'adds to end of content');
assert.deepEqual(array.get('arrangedContent'), [5, 4, 3, 2, 1], 'arrangedContent stays sorted');

run(() => array.addObject(1));

assert.deepEqual(array.get('content'), [1, 2, 4, 5, 3], 'does not add existing number to content');
});

QUnit.test('addObjects - adds to end of \'content\' if not present', function(assert) {
run(() => array.addObjects([1, 3, 6]));

assert.deepEqual(array.get('content'), [1, 2, 4, 5, 3, 6], 'adds to end of content');
assert.deepEqual(array.get('arrangedContent'), [6, 5, 4, 3, 2, 1], 'arrangedContent stays sorted');
});

QUnit.test('compact - returns arrangedContent without nulls and undefined', function(assert) {
run(() => array.set('content', emberA([1, 3, null, 2, undefined])));

Expand All @@ -55,20 +37,11 @@ QUnit.test('indexOf - returns index of object in arrangedContent', function(asse
assert.equal(array.indexOf(4), 1, 'returns arranged index');
});

QUnit.test('insertAt - raises, indeterminate behavior', function(assert) {
assert.throws(() => run(() => array.insertAt(2, 3)));
});

QUnit.test('lastIndexOf - returns last index of object in arrangedContent', function(assert) {
run(() => array.pushObject(4));

array.get('content').pushObject(4);
assert.equal(array.lastIndexOf(4), 2, 'returns last arranged index');
});

QUnit.test('nextObject - returns object at index in arrangedContent', function(assert) {
assert.equal(array.nextObject(1), 4, 'returns object at index');
});

QUnit.test('objectAt - returns object at index in arrangedContent', function(assert) {
assert.equal(objectAt(array, 1), 4, 'returns object at index');
});
Expand All @@ -82,38 +55,6 @@ QUnit.test('objectsAt - returns objects at indices in arrangedContent', function
assert.deepEqual(array.objectsAt([0, 2, 4]), [5, 2, undefined], 'returns objects at indices');
});

QUnit.test('popObject - removes last object in arrangedContent', function(assert) {
let popped;
run(() => popped = array.popObject());
assert.equal(popped, 1, 'returns last object');
assert.deepEqual(array.get('content'), [2, 4, 5], 'removes from content');
});

QUnit.test('pushObject - adds to end of content even if it already exists', function(assert) {
run(() => array.pushObject(1));
assert.deepEqual(array.get('content'), [1, 2, 4, 5, 1], 'adds to end of content');
});

QUnit.test('pushObjects - adds multiple to end of content even if it already exists', function(assert) {
run(() => array.pushObjects([1, 2, 4]));
assert.deepEqual(array.get('content'), [1, 2, 4, 5, 1, 2, 4], 'adds to end of content');
});

QUnit.test('removeAt - removes from index in arrangedContent', function(assert) {
run(() => array.removeAt(1, 2));
assert.deepEqual(array.get('content'), [1, 5]);
});

QUnit.test('removeObject - removes object from content', function(assert) {
run(() => array.removeObject(2));
assert.deepEqual(array.get('content'), [1, 4, 5]);
});

QUnit.test('removeObjects - removes objects from content', function(assert) {
run(() => array.removeObjects([2, 4, 6]));
assert.deepEqual(array.get('content'), [1, 5]);
});

QUnit.test('replace - raises, indeterminate behavior', function(assert) {
assert.throws(() => run(() => array.replace(1, 2, [3])));
});
Expand All @@ -123,22 +64,6 @@ QUnit.test('replaceContent - does a standard array replace on content', function
assert.deepEqual(array.get('content'), [1, 3, 5]);
});

QUnit.test('reverseObjects - raises, use Sortable#sortAscending', function(assert) {
assert.throws(() => run(() => array.reverseObjects()));
});

QUnit.test('setObjects - replaces entire content', function(assert) {
run(() => array.setObjects([6, 7, 8]));
assert.deepEqual(array.get('content'), [6, 7, 8], 'replaces content');
});

QUnit.test('shiftObject - removes from start of arrangedContent', function(assert) {
let shifted = run(() => array.shiftObject());

assert.equal(shifted, 5, 'returns first object');
assert.deepEqual(array.get('content'), [1, 2, 4], 'removes object from content');
});

QUnit.test('slice - returns a slice of the arrangedContent', function(assert) {
assert.deepEqual(array.slice(1, 3), [4, 2], 'returns sliced arrangedContent');
});
Expand All @@ -147,16 +72,6 @@ QUnit.test('toArray - returns copy of arrangedContent', function(assert) {
assert.deepEqual(array.toArray(), [5, 4, 2, 1]);
});

QUnit.test('unshiftObject - adds to start of content', function(assert) {
run(() => array.unshiftObject(6));
assert.deepEqual(array.get('content'), [6, 1, 2, 4, 5], 'adds to start of content');
});

QUnit.test('unshiftObjects - adds to start of content', function(assert) {
run(function() { array.unshiftObjects([6, 7]); });
assert.deepEqual(array.get('content'), [6, 7, 1, 2, 4, 5], 'adds to start of content');
});

QUnit.test('without - returns arrangedContent without object', function(assert) {
assert.deepEqual(array.without(2), [5, 4, 1], 'returns arranged without object');
});
Expand Down Expand Up @@ -234,14 +149,10 @@ QUnit.test('indexOf - returns index of object in arrangedContent', function(asse
});

QUnit.test('lastIndexOf - returns last index of object in arrangedContent', function(assert) {
run(function() { array.pushObject(4); });
array.get('content').pushObject(4);
assert.equal(array.lastIndexOf('4'), 2, 'returns last arranged index');
});

QUnit.test('nextObject - returns object at index in arrangedContent', function(assert) {
assert.equal(array.nextObject(1), '4', 'returns object at index');
});

QUnit.test('objectAt - returns object at index in arrangedContent', function(assert) {
assert.equal(objectAt(array, 1), '4', 'returns object at index');
});
Expand All @@ -255,30 +166,6 @@ QUnit.test('objectsAt - returns objects at indices in arrangedContent', function
assert.deepEqual(array.objectsAt([0, 2, 4]), ['5', '2', undefined], 'returns objects at indices');
});

QUnit.test('popObject - removes last object in arrangedContent', function(assert) {
let popped;
run(function() { popped = array.popObject(); });
assert.equal(popped, '1', 'returns last object');
assert.deepEqual(array.get('content'), [2, 4, 5], 'removes from content');
});

QUnit.test('removeObject - removes object from content', function(assert) {
run(function() { array.removeObject('2'); });
assert.deepEqual(array.get('content'), [1, 4, 5]);
});

QUnit.test('removeObjects - removes objects from content', function(assert) {
run(function() { array.removeObjects(['2', '4', '6']); });
assert.deepEqual(array.get('content'), [1, 5]);
});

QUnit.test('shiftObject - removes from start of arrangedContent', function(assert) {
let shifted;
run(function() { shifted = array.shiftObject(); });
assert.equal(shifted, '5', 'returns first object');
assert.deepEqual(array.get('content'), [1, 2, 4], 'removes object from content');
});

QUnit.test('slice - returns a slice of the arrangedContent', function(assert) {
assert.deepEqual(array.slice(1, 3), ['4', '2'], 'returns sliced arrangedContent');
});
Expand All @@ -299,6 +186,48 @@ QUnit.test('firstObject - returns first arranged object', function(assert) {
assert.equal(array.get('firstObject'), '5', 'returns first arranged object');
});

QUnit.module('ArrayProxy - with transforms', {
beforeEach() {
run(function() {
array = ArrayProxy.extend({
objectAtContent(idx) {
let obj = objectAt(this.get('arrangedContent'), idx);
return obj && obj.toString();
}
}).create({
content: emberA([1, 2, 4, 5])
});
});
},
afterEach() {
run(function() {
array.destroy();
});
}
});

QUnit.test('popObject - removes last object in arrangedContent', function(assert) {
let popped = array.popObject();
assert.equal(popped, '5', 'returns last object');
assert.deepEqual(array.toArray(), ['1', '2', '4'], 'removes from content');
});

QUnit.test('removeObject - removes object from content', function(assert) {
array.removeObject('2');
assert.deepEqual(array.toArray(), ['1', '4', '5']);
});

QUnit.test('removeObjects - removes objects from content', function(assert) {
array.removeObjects(['2', '4', '6']);
assert.deepEqual(array.toArray(), ['1', '5']);
});

QUnit.test('shiftObject - removes from start of arrangedContent', function(assert) {
let shifted = array.shiftObject();
assert.equal(shifted, '1', 'returns first object');
assert.deepEqual(array.toArray(), ['2', '4', '5'], 'removes object from content');
});

QUnit.test('arrangedContentArray{Will,Did}Change are called when the arranged content changes', function(assert) {
// The behavior covered by this test may change in the future if we decide
// that built-in array methods are not overridable.
Expand Down

0 comments on commit a73f809

Please sign in to comment.