From d2425815b8487f2fcf2c12839c336d30cc6d8abe Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Thu, 5 Apr 2018 18:18:13 -0700 Subject: [PATCH] fix belongsTo reload for async relationships --- addon/-private/system/promise-proxies.js | 22 +++++--- .../system/relationships/state/belongs-to.js | 50 ++++++++++++++----- .../relationships/belongs-to-test.js | 12 ++--- .../relationships/json-api-links-test.js | 8 +-- 4 files changed, 62 insertions(+), 30 deletions(-) diff --git a/addon/-private/system/promise-proxies.js b/addon/-private/system/promise-proxies.js index f2a6bb814a8..f89ecef6d79 100644 --- a/addon/-private/system/promise-proxies.js +++ b/addon/-private/system/promise-proxies.js @@ -82,6 +82,21 @@ export function promiseArray(promise, label) { }); } +export function proxyToContent(method) { + return function() { + return get(this, 'content')[method](...arguments); + }; +} + +export const PromiseBelongsTo = PromiseObject.extend({ + reload() { + assert('You are trying to reload an async belongsTo before it has been created', get(this, 'content')); + this.get('_belongsToState').reload(); + + return this; + } +}); + /** A PromiseManyArray is a PromiseArray that also proxies certain method calls to the underlying manyArray. @@ -99,13 +114,6 @@ export function promiseArray(promise, label) { @namespace DS @extends Ember.ArrayProxy */ - -export function proxyToContent(method) { - return function() { - return get(this, 'content')[method](...arguments); - }; -} - export const PromiseManyArray = PromiseArray.extend({ reload() { assert('You are trying to reload an async manyArray before it has been created', get(this, 'content')); diff --git a/addon/-private/system/relationships/state/belongs-to.js b/addon/-private/system/relationships/state/belongs-to.js index 6179b9ed7a4..ecbb00a4e8e 100644 --- a/addon/-private/system/relationships/state/belongs-to.js +++ b/addon/-private/system/relationships/state/belongs-to.js @@ -2,7 +2,7 @@ import { Promise as EmberPromise } from 'rsvp'; import { assert, inspect } from '@ember/debug'; import { assertPolymorphicType } from 'ember-data/-debug'; import { - PromiseObject + PromiseBelongsTo } from "../../promise-proxies"; import Relationship from "./relationship"; @@ -13,6 +13,7 @@ export default class BelongsToRelationship extends Relationship { this.key = relationshipMeta.key; this.inverseInternalModel = null; this.canonicalState = null; + this._loadingPromise = null; } setInternalModel(internalModel) { @@ -172,10 +173,9 @@ export default class BelongsToRelationship extends Relationship { promise = this.findRecord(); } - return PromiseObject.create({ - promise: promise, - content: this.inverseInternalModel ? this.inverseInternalModel.getRecord() : null - }); + let record = this.inverseInternalModel ? this.inverseInternalModel.getRecord() : null + + return this._updateLoadingPromise(promise, record); } else { if (this.inverseInternalModel === null) { return null; @@ -186,19 +186,43 @@ export default class BelongsToRelationship extends Relationship { } } - reload() { - // TODO handle case when reload() is triggered multiple times + _updateLoadingPromise(promise, content) { + if (this._loadingPromise) { + if (content) { + this._loadingPromise.set('content', content) + } + this._loadingPromise.set('promise', promise) + } else { + this._loadingPromise = PromiseBelongsTo.create({ + _belongsToState: this, + promise, + content + }); + } - if (this.link) { - return this.fetchLink(); + return this._loadingPromise; + } + + reload() { + // we've already fired off a request + if (this._loadingPromise) { + if (this._loadingPromise.get('isPending')) { + return this._loadingPromise; + } } - // reload record, if it is already loaded - if (this.inverseInternalModel && this.inverseInternalModel.hasRecord) { - return this.inverseInternalModel.getRecord().reload(); + let promise; + + if (this.link) { + promise = this.fetchLink(); + } else if (this.inverseInternalModel && this.inverseInternalModel.hasRecord) { + // reload record, if it is already loaded + promise = this.inverseInternalModel.getRecord().reload(); + } else { + promise = this.findRecord(); } - return this.findRecord(); + return this._updateLoadingPromise(promise); } localStateIsEmpty() { diff --git a/tests/integration/relationships/belongs-to-test.js b/tests/integration/relationships/belongs-to-test.js index 40ef976c660..204c69faeb4 100644 --- a/tests/integration/relationships/belongs-to-test.js +++ b/tests/integration/relationships/belongs-to-test.js @@ -1380,9 +1380,9 @@ test("A belongsTo relationship can be reloaded using the reference if it was fet }); }); -test("A sync belongsTo relationship can be reloaded using a reference if it was fetched via id", function(assert) { +test("A synchronous belongsTo relationship can be reloaded using a reference if it was fetched via id", function(assert) { Chapter.reopen({ - book: DS.belongsTo() + book: DS.belongsTo({ async: false }) }); let chapter; @@ -1390,10 +1390,10 @@ test("A sync belongsTo relationship can be reloaded using a reference if it was chapter = env.store.push({ data: { type: 'chapter', - id: 1, + id: '1', relationships: { book: { - data: { type: 'book', id: 1 } + data: { type: 'book', id: '1' } } } } @@ -1401,7 +1401,7 @@ test("A sync belongsTo relationship can be reloaded using a reference if it was env.store.push({ data: { type: 'book', - id: 1, + id: '1', attributes: { name: "book title" } @@ -1412,7 +1412,7 @@ test("A sync belongsTo relationship can be reloaded using a reference if it was env.adapter.findRecord = function() { return resolve({ data: { - id: 1, + id: '1', type: 'book', attributes: { name: 'updated book title' } } diff --git a/tests/integration/relationships/json-api-links-test.js b/tests/integration/relationships/json-api-links-test.js index c020907e5ee..f24dcdc2221 100644 --- a/tests/integration/relationships/json-api-links-test.js +++ b/tests/integration/relationships/json-api-links-test.js @@ -774,7 +774,7 @@ function shouldFetchLinkTests(description, payloads) { assert.ok(!!home, 'We found our home'); - run(() => home.then(h => h.reload())); + run(() => home.reload()); }); test(`get+unload+get belongsTo with ${description}`, function(assert) { assert.expect(3); @@ -1080,7 +1080,7 @@ function shouldReloadWithLinkTests(description, payloads) { assert.ok(!!home, 'We found our home'); - run(() => home.then(h => h.reload())); + run(() => home.reload()); }); test(`get+unload+get belongsTo with ${description}`, function(assert) { assert.expect(2); @@ -1356,7 +1356,7 @@ test(`get+reload belongsTo with data, no links`, function(assert) { assert.ok(!!home, 'We found our home'); - run(() => home.then(h => h.reload())); + run(() => home.reload()); }); test(`get+unload+get belongsTo with data, no links`, function(assert) { assert.expect(3); @@ -1616,7 +1616,7 @@ test(`get+reload belongsTo with missing data setup from the other side, no links assert.ok(!!home, 'We found our home'); - run(() => home.then(h => h.reload())); + run(() => home.reload()); }); test(`get+unload+get belongsTo with missing data setup from the other side, no links`, function(assert) { assert.expect(2);