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 102a214 commit bf7006d
Show file tree
Hide file tree
Showing 4 changed files with 182 additions and 21 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
50 changes: 35 additions & 15 deletions addon/-private/system/relationships/state/has-many.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { assert, assertPolymorphicType } from "ember-data/-private/debug";
import { PromiseManyArray, promiseManyArray } from "../../promise-proxies";
import Relationship from "./relationship";
import OrderedSet from "../../ordered-set";
import ManyArray from "../../many-array";
import { assert, assertPolymorphicType } from 'ember-data/-private/debug';
import { PromiseManyArray } from '../../promise-proxies';
import Relationship from './relationship';
import OrderedSet from '../../ordered-set';
import ManyArray from '../../many-array';

export default class ManyRelationship extends Relationship {
constructor(store, record, inverseKey, relationshipMeta) {
Expand All @@ -11,6 +11,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;
}

get manyArray() {
Expand All @@ -34,6 +52,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 @@ -126,13 +148,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 @@ -190,7 +214,7 @@ export default class ManyRelationship extends Relationship {
//TODO(Igor) sync server here, once our syncing is not stupid
let manyArray = this.manyArray;
if (this.isAsync) {
var promise;
let promise;
if (this.link) {
if (this.hasLoaded) {
promise = this.findRecords();
Expand All @@ -200,11 +224,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
91 changes: 89 additions & 2 deletions tests/unit/promise-proxies-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ 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
let array = new DS.PromiseManyArray({
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 = new DS.PromiseManyArray({
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 = new DS.PromiseManyArray({
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 bf7006d

Please sign in to comment.