Skip to content

Commit

Permalink
[fixes #4826] make async DS.hasMany promises stable
Browse files Browse the repository at this point in the history
  • Loading branch information
stefanpenner committed Mar 8, 2017
1 parent 12abeb2 commit a1b0c41
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 16 deletions.
6 changes: 2 additions & 4 deletions addon/-private/system/promise-proxies.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,9 @@ export function proxyToContent(method) {

export const PromiseManyArray = PromiseArray.extend({
reload() {
//I don't think this should ever happen right now, but worth guarding if we refactor the async relationships
assert('You are trying to reload an async manyArray before it has been created', get(this, 'content'));
return PromiseManyArray.create({
promise: get(this, 'content').reload()
});
this.set('promise', this.get('content').reload())
return this;
},

createRecord: proxyToContent('createRecord'),
Expand Down
42 changes: 31 additions & 11 deletions addon/-private/system/relationships/state/has-many.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { assert } from 'ember-data/-private/debug';
import { PromiseManyArray, promiseManyArray } from 'ember-data/-private/system/promise-proxies';
import { PromiseManyArray } from 'ember-data/-private/system/promise-proxies';
import Relationship from 'ember-data/-private/system/relationships/state/relationship';
import OrderedSet from 'ember-data/-private/system/ordered-set';
import ManyArray from 'ember-data/-private/system/many-array';
Expand All @@ -13,6 +13,24 @@ export default class ManyRelationship extends Relationship {
this.canonicalState = [];
this.isPolymorphic = relationshipMeta.options.polymorphic;
this._manyArray = null;
this.__loadingPromise = null;
}

get _loadingPromise() { return this.__loadingPromise; }
_updateLoadingPromise(promise, content) {
if (this.__loadingPromise) {
if (content) {
this.__loadingPromise.set('content', content)
}
this.__loadingPromise.set('promise', promise)
} else {
this.__loadingPromise = new PromiseManyArray({
promise,
content
});
}

return this.__loadingPromise;
}

getManyArray() {
Expand All @@ -36,6 +54,10 @@ export default class ManyRelationship extends Relationship {
this._manyArray.destroy();
this._manyArray = null;
}

if (this._loadingPromise) {
this._loadingPromise.destroy();
}
}

updateMeta(meta) {
Expand Down Expand Up @@ -128,13 +150,15 @@ export default class ManyRelationship extends Relationship {
}
}

let promise;
if (this.link) {
this._loadingPromise = promiseManyArray(this.fetchLink(), 'Reload with link');
return this._loadingPromise;
promise = this.fetchLink();
} else {
this._loadingPromise = promiseManyArray(this.store._scheduleFetchMany(manyArray.currentState).then(() => manyArray), 'Reload with ids');
return this._loadingPromise;
promise = this.store._scheduleFetchMany(manyArray.currentState).then(() => manyArray);
}

this._updateLoadingPromise(promise);
return this._loadingPromise;
}

computeChanges(records) {
Expand Down Expand Up @@ -197,7 +221,7 @@ export default class ManyRelationship extends Relationship {
//TODO(Igor) sync server here, once our syncing is not stupid
let manyArray = this.getManyArray();
if (this.isAsync) {
var promise;
let promise;
if (this.link) {
if (this.hasLoaded) {
promise = this.findRecords();
Expand All @@ -207,11 +231,7 @@ export default class ManyRelationship extends Relationship {
} else {
promise = this.findRecords();
}
this._loadingPromise = PromiseManyArray.create({
content: manyArray,
promise: promise
});
return this._loadingPromise;
return this._updateLoadingPromise(promise, manyArray);
} else {
assert(`You looked up the '${this.key}' relationship on a '${this.record.type.modelName}' with id ${this.record.id} but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async ('DS.hasMany({ async: true })')`, manyArray.isEvery('isEmpty', false));

Expand Down
56 changes: 56 additions & 0 deletions tests/unit/model/relationships/has-many-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,62 @@ test("DS.hasMany is async by default", function(assert) {
});
});

test('DS.hasMany is stable', function(assert) {
const Tag = DS.Model.extend({
name: DS.attr('string'),
people: DS.hasMany('person')
});

const Person = DS.Model.extend({
name: DS.attr('string'),
tag: DS.belongsTo('tag', { async: false })
});

let { store } = setupStore({ tag: Tag, person: Person });

let tag = run(() => store.createRecord('tag'));
let people = tag.get('people');
let peopleCached = tag.get('people');

assert.equal(people, peopleCached);

tag.notifyPropertyChange('people');
let notifiedPeople = tag.get('people');

assert.equal(people, notifiedPeople);

return Ember.RSVP.Promise.all([
people
]);
});

test('DS.hasMany proxy is destroyed', function(assert) {
const Tag = DS.Model.extend({
name: DS.attr('string'),
people: DS.hasMany('person')
});

const Person = DS.Model.extend({
name: DS.attr('string'),
tag: DS.belongsTo('tag', { async: false })
});

let { store } = setupStore({ tag: Tag, person: Person });

let tag = run(() => store.createRecord('tag'));
let people = tag.get('people');

return people.then(() => {
Ember.run(() => {
tag.unloadRecord();
assert.equal(people.get('isDestroying'), true);
assert.equal(people.get('isDestroyed'), false);
});
assert.equal(people.get('isDestroying'), true);
assert.equal(people.get('isDestroyed'), true);
})
});

test('DS.ManyArray is lazy', function(assert) {
let peopleDidChange = 0;
const Tag = DS.Model.extend({
Expand Down
89 changes: 88 additions & 1 deletion tests/unit/promise-proxies-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ test('.reload should NOT leak the internal promise, rather return another promis
content.reload = () => Ember.RSVP.Promise.resolve(content);

let array = DS.PromiseManyArray.create({
content: content
content
});

let reloaded = array.reload();
Expand All @@ -23,3 +23,90 @@ test('.reload should NOT leak the internal promise, rather return another promis

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

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

let content = Ember.A();

content.reload = () => Ember.RSVP.Promise.resolve(content);
let promise = Ember.RSVP.Promise.resolve(content);

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

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

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

let reloaded = array.reload();

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

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

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

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

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

let content = Ember.A([1,2,3]);

let promise = Ember.RSVP.Promise.resolve(content);

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

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

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

array.set('promise', Ember.RSVP.Promise.resolve(content));

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

assert.ok(array instanceof DS.PromiseManyArray);

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

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

0 comments on commit a1b0c41

Please sign in to comment.