Skip to content

Commit

Permalink
Perf: Refactor PromiseManyArray and prep for RFC#745 (#7505)
Browse files Browse the repository at this point in the history
* perf: custom PromiseManyArray

* fix: entangle [] tag for ember-source 3.20

* fix type declaration

* fix perf test
  • Loading branch information
runspired authored May 12, 2021
1 parent cf71018 commit ac5d113
Show file tree
Hide file tree
Showing 11 changed files with 460 additions and 175 deletions.
14 changes: 14 additions & 0 deletions packages/-ember-data/node-tests/fixtures/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -402,5 +402,19 @@ module.exports = {
"(public) @ember-data/adapter/error @ember-data/adapter/error#errorsArrayToHash",
"(public) @ember-data/adapter/error @ember-data/adapter/error#errorsHashToArray",
"(public) @ember-data/store Store#identifierCache",
'(private) @ember-data/model PromiseManyArray#forEach',
'(public) @ember-data/model PromiseManyArray#isFulfilled',
'(public) @ember-data/model PromiseManyArray#isPending',
'(public) @ember-data/model PromiseManyArray#isRejected',
'(public) @ember-data/model PromiseManyArray#isSettled',
'(public) @ember-data/model PromiseManyArray#length',
'(public) @ember-data/model PromiseManyArray#links',
"(public) @ember-data/model PromiseManyArray#catch",
"(public) @ember-data/model PromiseManyArray#finally",
"(public) @ember-data/model PromiseManyArray#meta",
"(public) @ember-data/model PromiseManyArray#reload",
"(public) @ember-data/model PromiseManyArray#then",
'(public) @ember-data/store ManyArray#links',
'(public) @ember-data/store Store#identifierCache'
],
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import RESTAdapter from '@ember-data/adapter/rest';
import Model, { attr, belongsTo, hasMany } from '@ember-data/model';
import JSONAPISerializer from '@ember-data/serializer/json-api';
import RESTSerializer from '@ember-data/serializer/rest';
import { PromiseArray, Snapshot } from '@ember-data/store/-private';
import { Snapshot } from '@ember-data/store/-private';
import testInDebug from '@ember-data/unpublished-test-infra/test-support/test-in-debug';

function moveRecordOutOfInFlight(record) {
Expand Down Expand Up @@ -1144,7 +1144,7 @@ module('integration/adapter/store-adapter - DS.Store and DS.Adapter integration
},
},
});
assert.ok(tom.get('dogs') instanceof PromiseArray, 'dogs is a promise');
assert.ok(typeof tom.dogs.then === 'function', 'dogs is a thenable');
return tom.get('dogs');
})
.then((dogs) => {
Expand Down Expand Up @@ -1180,12 +1180,12 @@ module('integration/adapter/store-adapter - DS.Store and DS.Adapter integration
let tom = store.createRecord('person', { name: 'Tom Dale' });

run(() => {
assert.ok(tom.get('dogs') instanceof PromiseArray, 'dogs is a promise before save');
assert.ok(typeof tom.dogs.then === 'function', 'dogs is a thenable before save');
});

return run(() => {
return tom.save().then(() => {
assert.ok(tom.get('dogs') instanceof PromiseArray, 'dogs is a promise after save');
assert.ok(typeof tom.dogs.then === 'function', 'dogs is a thenable after save');
});
});
});
Expand Down
28 changes: 18 additions & 10 deletions packages/-ember-data/tests/unit/many-array-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,28 +101,35 @@ module('unit/many_array - DS.ManyArray', function (hooks) {
test('manyArray trigger arrayContentChange functions with the correct values', function (assert) {
assert.expect(6);

const TestManyArray = DS.ManyArray.proto();

let willChangeStartIdx;
let willChangeRemoveAmt;
let willChangeAddAmt;

// We aren't actually adding any observers in this test
// just testing the observer codepaths, so we use this to
// force the ManyArray instance to take the observer paths.
DS.ManyArray.proto().__hasArrayObservers = true;
let originalArrayContentWillChange = DS.ManyArray.proto().arrayContentWillChange;
let originalArrayContentDidChange = DS.ManyArray.proto().arrayContentDidChange;
let originalArrayContentWillChange = TestManyArray.arrayContentWillChange;
let originalArrayContentDidChange = TestManyArray.arrayContentDidChange;
let originalInit = TestManyArray.init;

// override DS.ManyArray temp (cleanup occures in afterTest);

DS.ManyArray.proto().arrayContentWillChange = function (startIdx, removeAmt, addAmt) {
TestManyArray.init = function (...args) {
// We aren't actually adding any observers in this test
// just testing the observer codepaths, so we use this to
// force the ManyArray instance to take the observer paths.
this.__hasArrayObservers = true;
originalInit.call(this, ...args);
};

TestManyArray.arrayContentWillChange = function (startIdx, removeAmt, addAmt) {
willChangeStartIdx = startIdx;
willChangeRemoveAmt = removeAmt;
willChangeAddAmt = addAmt;

return originalArrayContentWillChange.apply(this, arguments);
};

DS.ManyArray.proto().arrayContentDidChange = function (startIdx, removeAmt, addAmt) {
TestManyArray.arrayContentDidChange = function (startIdx, removeAmt, addAmt) {
assert.equal(startIdx, willChangeStartIdx, 'WillChange and DidChange startIdx should match');
assert.equal(removeAmt, willChangeRemoveAmt, 'WillChange and DidChange removeAmt should match');
assert.equal(addAmt, willChangeAddAmt, 'WillChange and DidChange addAmt should match');
Expand Down Expand Up @@ -184,8 +191,9 @@ module('unit/many_array - DS.ManyArray', function (hooks) {
});
});
} finally {
DS.ManyArray.proto().arrayContentWillChange = originalArrayContentWillChange;
DS.ManyArray.proto().arrayContentDidChange = originalArrayContentDidChange;
TestManyArray.arrayContentWillChange = originalArrayContentWillChange;
TestManyArray.arrayContentDidChange = originalArrayContentDidChange;
TestManyArray.init = originalInit;
}
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { get, observer } from '@ember/object';
import { run } from '@ember/runloop';
import settled from '@ember/test-helpers/settled';

import { module, test } from 'qunit';
import { hash, Promise as EmberPromise } from 'rsvp';
Expand Down Expand Up @@ -2326,7 +2327,7 @@ module('unit/model/relationships - DS.hasMany', function (hooks) {
let store = this.owner.lookup('service:store');
let tag = store.createRecord('tag');

assert.ok(tag.get('people') instanceof DS.PromiseArray, 'people should be an async relationship');
assert.ok(tag.get('people') instanceof DS.PromiseManyArray, 'people should be an async relationship');
});

test('hasMany is stable', function (assert) {
Expand Down Expand Up @@ -2378,30 +2379,21 @@ module('unit/model/relationships - DS.hasMany', function (hooks) {

return peopleProxy.then((people) => {
run(() => {
let isRecordDataBuild = people.recordData !== undefined;
tag.unloadRecord();
// TODO Check all unloading behavior
assert.false(people.isDestroying, 'people is NOT destroying sync after unloadRecord');
assert.false(people.isDestroyed, 'people is NOT destroyed sync after unloadRecord');

// unload is not the same as destroy, and we may cancel
// prior to RecordData, this was coupled to the destroy
// of the relationship, which was async and possibly could
// be cancelled were an unload to be aborted.
assert.equal(
peopleProxy.isDestroying,
isRecordDataBuild,
'peopleProxy is not destroying sync after unloadRecord'
);
assert.false(peopleProxy.isDestroyed, 'peopleProxy is NOT YET destroyed sync after unloadRecord');
assert.true(peopleProxy.isDestroying, 'peopleProxy is destroying sync after unloadRecord');
assert.true(peopleProxy.isDestroyed, 'peopleProxy is destroyed sync after unloadRecord');
});

assert.true(peopleProxy.isDestroying, 'peopleProxy is destroying after the run post unloadRecord');
assert.true(peopleProxy.isDestroyed, 'peopleProxy is destroyed after the run post unloadRecord');
});
});

test('DS.ManyArray is lazy', function (assert) {
test('DS.ManyArray is lazy', async function (assert) {
let peopleDidChange = 0;
const Tag = DS.Model.extend({
name: DS.attr('string'),
Expand All @@ -2426,11 +2418,11 @@ module('unit/model/relationships - DS.hasMany', function (hooks) {

//assert.ok(!hasManyRelationship._manyArray);

run(() => {
assert.equal(peopleDidChange, 0, 'expect people hasMany to not emit a change event (before access)');
tag.get('people');
assert.equal(peopleDidChange, 0, 'expect people hasMany to not emit a change event (sync after access)');
});
assert.equal(peopleDidChange, 0, 'expect people hasMany to not emit a change event (before access)');
tag.people; // access async relationship
assert.equal(peopleDidChange, 0, 'expect people hasMany to not emit a change event (sync after access)');

await settled();

assert.equal(
peopleDidChange,
Expand All @@ -2441,18 +2433,10 @@ module('unit/model/relationships - DS.hasMany', function (hooks) {

let person = store.createRecord('person');

run(() => {
assert.equal(peopleDidChange, 0, 'expect people hasMany to not emit a change event (before access)');
tag.get('people').addObject(person);
/*
We expect two notifications here because `people` is an async relationship.
For async relationships we notify the primary key in addition to notifying
the ManyArray that it needs to dirty and retrieveLatest.
Since our observer watches `people.@each` it will receive both notifications.
*/
assert.equal(peopleDidChange, 2, 'expect people hasMany to have notified twice');
});
assert.equal(peopleDidChange, 0, 'expect people hasMany to not emit a change event (before access)');
const people = await tag.people;
people.addObject(person);
assert.strictEqual(peopleDidChange, 1, 'expect people hasMany to have changed exactly once');
});

test('fetch hasMany loads full relationship after a parent and child have been loaded', function (assert) {
Expand Down
109 changes: 54 additions & 55 deletions packages/-ember-data/tests/unit/promise-proxies-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import JSONAPISerializer from '@ember-data/serializer/json-api';

module('PromiseManyArray', function () {
test('.reload should NOT leak the internal promise, rather return another promiseArray', function (assert) {
assert.expect(2);
assert.expect(1);

let content = A();

Expand All @@ -24,56 +24,57 @@ module('PromiseManyArray', function () {

let reloaded = array.reload();

assert.ok(reloaded instanceof DS.PromiseManyArray);

return reloaded.then((value) => assert.equal(content, value));
assert.ok(reloaded === array);
});

test('.reload should be stable', function (assert) {
test('.reload should be stable', async function (assert) {
assert.expect(19);

let content = A();
let array;

content.reload = () => EmberPromise.resolve(content);
content.reload = () => {
let p = EmberPromise.resolve(content);
array._update(p);
return p;
};
let promise = EmberPromise.resolve(content);

let array = DS.PromiseManyArray.create({
array = DS.PromiseManyArray.create({
promise,
});

assert.false(array.get('isRejected'), 'should NOT be rejected');
assert.true(array.get('isPending'), 'should be pending');
assert.false(array.get('isSettled'), 'should NOT be settled');
assert.false(array.get('isFulfilled'), 'should NOT be fulfilled');
assert.false(array.isRejected, 'should NOT be rejected');
assert.true(array.isPending, 'should be pending');
assert.false(array.isSettled, 'should NOT be settled');
assert.false(array.isFulfilled, 'should NOT be fulfilled');

return array.then(() => {
assert.false(array.get('isRejected'), 'should NOT be rejected');
assert.false(array.get('isPending'), 'should NOT be pending');
assert.true(array.get('isSettled'), 'should be settled');
assert.true(array.get('isFulfilled'), 'should be fulfilled');
await array;
assert.false(array.isRejected, 'should NOT be rejected');
assert.false(array.isPending, 'should NOT be pending');
assert.true(array.isSettled, 'should be settled');
assert.true(array.isFulfilled, 'should be fulfilled');

let reloaded = array.reload();
let reloaded = array.reload();

assert.false(array.get('isRejected'), 'should NOT be rejected');
assert.true(array.get('isPending'), 'should be pending');
assert.false(array.get('isSettled'), 'should NOT be settled');
assert.false(array.get('isFulfilled'), 'should NOT be fulfilled');
assert.false(array.isRejected, 'should NOT be rejected');
assert.true(array.isPending, 'should be pending');
assert.false(array.isSettled, 'should NOT be settled');
assert.false(array.isFulfilled, 'should NOT be fulfilled');

assert.ok(reloaded instanceof DS.PromiseManyArray);
assert.equal(reloaded, array);
assert.ok(reloaded instanceof DS.PromiseManyArray);
assert.equal(reloaded, array);

return reloaded.then((value) => {
assert.false(array.get('isRejected'), 'should NOT be rejected');
assert.false(array.get('isPending'), 'should NOT be pending');
assert.true(array.get('isSettled'), 'should be settled');
assert.true(array.get('isFulfilled'), 'should be fulfilled');
let value = await reloaded;
assert.false(array.isRejected, 'should NOT be rejected');
assert.false(array.isPending, 'should NOT be pending');
assert.true(array.isSettled, 'should be settled');
assert.true(array.isFulfilled, 'should be fulfilled');

assert.equal(content, value);
});
});
assert.equal(content, value);
});

test('.set to new promise should be like reload', function (assert) {
test('.set to new promise should be like reload', async function (assert) {
assert.expect(18);

let content = A([1, 2, 3]);
Expand All @@ -84,35 +85,33 @@ module('PromiseManyArray', function () {
promise,
});

assert.false(array.get('isRejected'), 'should NOT be rejected');
assert.true(array.get('isPending'), 'should be pending');
assert.false(array.get('isSettled'), 'should NOT be settled');
assert.false(array.get('isFulfilled'), 'should NOT be fulfilled');
assert.false(array.isRejected, 'should NOT be rejected');
assert.true(array.isPending, 'should be pending');
assert.false(array.isSettled, 'should NOT be settled');
assert.false(array.isFulfilled, 'should NOT be fulfilled');

return array.then(() => {
assert.false(array.get('isRejected'), 'should NOT be rejected');
assert.false(array.get('isPending'), 'should NOT be pending');
assert.true(array.get('isSettled'), 'should be settled');
assert.true(array.get('isFulfilled'), 'should be fulfilled');
await array;
assert.false(array.isRejected, 'should NOT be rejected');
assert.false(array.isPending, 'should NOT be pending');
assert.true(array.isSettled, 'should be settled');
assert.true(array.isFulfilled, 'should be fulfilled');

array.set('promise', EmberPromise.resolve(content));
array._update(EmberPromise.resolve(content));

assert.false(array.get('isRejected'), 'should NOT be rejected');
assert.true(array.get('isPending'), 'should be pending');
assert.false(array.get('isSettled'), 'should NOT be settled');
assert.false(array.get('isFulfilled'), 'should NOT be fulfilled');
assert.false(array.isRejected, 'should NOT be rejected');
assert.true(array.isPending, 'should be pending');
assert.false(array.isSettled, 'should NOT be settled');
assert.false(array.isFulfilled, 'should NOT be fulfilled');

assert.ok(array instanceof DS.PromiseManyArray);
assert.ok(array instanceof DS.PromiseManyArray);

return array.then((value) => {
assert.false(array.get('isRejected'), 'should NOT be rejected');
assert.false(array.get('isPending'), 'should NOT be pending');
assert.true(array.get('isSettled'), 'should be settled');
assert.true(array.get('isFulfilled'), 'should be fulfilled');
let value = await array;
assert.false(array.isRejected, 'should NOT be rejected');
assert.false(array.isPending, 'should NOT be pending');
assert.true(array.isSettled, 'should be settled');
assert.true(array.isFulfilled, 'should be fulfilled');

assert.equal(content, value);
});
});
assert.equal(content, value);
});
});

Expand Down
Loading

0 comments on commit ac5d113

Please sign in to comment.