diff --git a/packages/-ember-data/tests/acceptance/relationships/belongs-to-test.js b/packages/-ember-data/tests/acceptance/relationships/belongs-to-test.js index 05dfc4f15ba..3c022afa9a7 100644 --- a/packages/-ember-data/tests/acceptance/relationships/belongs-to-test.js +++ b/packages/-ember-data/tests/acceptance/relationships/belongs-to-test.js @@ -1,4 +1,4 @@ -import { module, test, skip } from 'qunit'; +import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; import JSONAPIAdapter from '@ember-data/adapter/json-api'; import Model from '@ember-data/model'; @@ -374,8 +374,8 @@ module('async belongs-to rendering tests', function(hooks) { assert.equal(this.element.textContent.trim(), 'Kevin has two children and one parent'); }); - skip('Rendering an async belongs-to whose fetch fails does not trigger a new request', async function(assert) { - assert.expect(15); + test('Rendering an async belongs-to whose fetch fails does not trigger a new request', async function(assert) { + assert.expect(14); let people = makePeopleWithRelationshipData(); let sedona = store.push({ data: people.dict['5:has-parent-no-children'], @@ -412,10 +412,16 @@ module('async belongs-to rendering tests', function(hooks) { assert.equal(this.element.textContent.trim(), '', 'we have no parent'); let relationshipState = sedona.belongsTo('parent').belongsToRelationship; + let RelationshipPromiseCache = sedona._internalModel._relationshipPromisesCache; + let RelationshipProxyCache = sedona._internalModel._relationshipProxyCache; assert.equal(relationshipState.isAsync, true, 'The relationship is async'); assert.equal(relationshipState.relationshipIsEmpty, false, 'The relationship is not empty'); - assert.equal(relationshipState.relationshipIsStale, true, 'The relationship is still stale'); + assert.equal( + relationshipState.hasDematerializedInverse, + true, + 'The relationship inverse is dematerialized' + ); assert.equal( relationshipState.allInverseRecordsAreLoaded, false, @@ -427,9 +433,9 @@ module('async belongs-to rendering tests', function(hooks) { 'The relationship knows which record it needs' ); assert.equal( - relationshipState.fetchPromise === null, - true, - 'The relationship has no fetchPromise' + !!RelationshipPromiseCache['parent'], + false, + 'The relationship has no fetch promise' ); assert.equal( relationshipState.hasFailedLoadAttempt === true, @@ -442,18 +448,18 @@ module('async belongs-to rendering tests', function(hooks) { 'The relationship will not force a reload' ); assert.equal( - relationshipState._promiseProxy !== null, + !!RelationshipProxyCache['parent'], true, - 'The relationship has a loadingPromise' + 'The relationship has a promise proxy' ); assert.equal(!!relationshipState.link, false, 'The relationship does not have a link'); - assert.equal( - relationshipState.shouldMakeRequest(), - false, - 'The relationship does not need to make a request' - ); - let result = await sedona.get('parent'); - assert.ok(result === null, 're-access is safe'); + + try { + let result = await sedona.get('parent'); + assert.ok(result === null, 're-access is safe'); + } catch (e) { + assert.ok(false, `Accessing resulted in rejected promise error: ${e.message}`); + } Ember.onerror = originalOnError; }); diff --git a/packages/-ember-data/tests/acceptance/relationships/has-many-test.js b/packages/-ember-data/tests/acceptance/relationships/has-many-test.js index 0dccfe1cbb0..6b877adab55 100644 --- a/packages/-ember-data/tests/acceptance/relationships/has-many-test.js +++ b/packages/-ember-data/tests/acceptance/relationships/has-many-test.js @@ -1,4 +1,4 @@ -import { module, test, skip } from 'qunit'; +import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; import JSONAPIAdapter from '@ember-data/adapter/json-api'; import Model from '@ember-data/model'; @@ -287,7 +287,7 @@ module('async has-many rendering tests', function(hooks) { ); }); - skip('Rendering an async hasMany whose fetch fails does not trigger a new request', async function(assert) { + test('Rendering an async hasMany whose fetch fails does not trigger a new request', async function(assert) { assert.expect(12); let people = makePeopleWithRelationshipData(); let parent = store.push({ @@ -330,10 +330,16 @@ module('async has-many rendering tests', function(hooks) { ); let relationshipState = parent.hasMany('children').hasManyRelationship; + let RelationshipPromiseCache = parent._internalModel._relationshipPromisesCache; + let RelationshipProxyCache = parent._internalModel._relationshipProxyCache; assert.equal(relationshipState.isAsync, true, 'The relationship is async'); assert.equal(relationshipState.relationshipIsEmpty, false, 'The relationship is not empty'); - assert.equal(relationshipState.relationshipIsStale, true, 'The relationship is still stale'); + assert.equal( + relationshipState.hasDematerializedInverse, + true, + 'The relationship has a dematerialized inverse' + ); assert.equal( relationshipState.allInverseRecordsAreLoaded, false, @@ -345,9 +351,9 @@ module('async has-many rendering tests', function(hooks) { 'The relationship knows which record it needs' ); assert.equal( - relationshipState.fetchPromise === null, - true, - 'The relationship has no fetchPromise' + !!RelationshipPromiseCache['children'], + false, + 'The relationship has no fetch promise' ); assert.equal( relationshipState.hasFailedLoadAttempt === true, @@ -355,9 +361,9 @@ module('async has-many rendering tests', function(hooks) { 'The relationship has attempted a load' ); assert.equal( - relationshipState._promiseProxy !== null, + !!RelationshipProxyCache['children'], true, - 'The relationship has a loadingPromise' + 'The relationship has a promise proxy' ); assert.equal(!!relationshipState.link, false, 'The relationship does not have a link'); @@ -444,7 +450,7 @@ module('async has-many rendering tests', function(hooks) { ); }); - skip('Rendering an async hasMany with a link whose fetch fails does not trigger a new request', async function(assert) { + test('Rendering an async hasMany with a link whose fetch fails does not trigger a new request', async function(assert) { assert.expect(12); let people = makePeopleWithRelationshipLinks(true); let parent = store.push({ @@ -489,6 +495,8 @@ module('async has-many rendering tests', function(hooks) { assert.deepEqual(names, [], 'We rendered no names'); let relationshipState = parent.hasMany('children').hasManyRelationship; + let RelationshipPromiseCache = parent._internalModel._relationshipPromisesCache; + let RelationshipProxyCache = parent._internalModel._relationshipProxyCache; assert.equal(relationshipState.isAsync, true, 'The relationship is async'); assert.equal( @@ -508,19 +516,19 @@ module('async has-many rendering tests', function(hooks) { 'The relationship knows which record it needs' ); assert.equal( - relationshipState.fetchPromise === null, - true, - 'The relationship has no fetchPromise' + !!RelationshipPromiseCache['children'], + false, + 'The relationship has no fetch promise' ); assert.equal( - relationshipState.hasFailedLoadAttempt === true, + !!RelationshipProxyCache['children'], true, - 'The relationship has attempted a load' + 'The relationship has a promise proxy' ); assert.equal( - relationshipState._promiseProxy !== null, + relationshipState.hasFailedLoadAttempt === true, true, - 'The relationship has a loadingPromise' + 'The relationship has attempted a load' ); assert.equal(!!relationshipState.link, true, 'The relationship has a link'); diff --git a/packages/-ember-data/tests/integration/relationships/has-many-test.js b/packages/-ember-data/tests/integration/relationships/has-many-test.js index 2f53987f264..e477e4205e1 100644 --- a/packages/-ember-data/tests/integration/relationships/has-many-test.js +++ b/packages/-ember-data/tests/integration/relationships/has-many-test.js @@ -6,7 +6,7 @@ import { get } from '@ember/object'; import { run } from '@ember/runloop'; import setupStore from 'dummy/tests/helpers/store'; import testInDebug from 'dummy/tests/helpers/test-in-debug'; -import { module, test, skip } from 'qunit'; +import { module, test } from 'qunit'; import { relationshipStateFor, relationshipsFor } from 'ember-data/-private'; import DS from 'ember-data'; @@ -955,8 +955,8 @@ module('integration/relationships/has_many - Has-Many Relationships', function(h }); }); - skip('A hasMany relationship can be reloaded even if it failed at the first time', async function(assert) { - assert.expect(6); + test('A hasMany relationship can be reloaded even if it failed at the first time', async function(assert) { + assert.expect(7); const { store, adapter } = env; @@ -964,7 +964,7 @@ module('integration/relationships/has_many - Has-Many Relationships', function(h comments: DS.hasMany('comment', { async: true }), }); - adapter.findRecord = function(store, type, id) { + adapter.findRecord = function() { return resolve({ data: { id: 1, @@ -978,10 +978,10 @@ module('integration/relationships/has_many - Has-Many Relationships', function(h }); }; - let loadingCount = -1; - adapter.findHasMany = function(store, record, link, relationship) { + let loadingCount = 0; + adapter.findHasMany = function() { loadingCount++; - if (loadingCount % 2 === 0) { + if (loadingCount % 2 === 1) { return reject({ data: null }); } else { return resolve({ @@ -995,16 +995,24 @@ module('integration/relationships/has_many - Has-Many Relationships', function(h let post = await store.findRecord('post', 1); let comments = post.get('comments'); - let manyArray = await comments.catch(() => { - assert.ok(true, 'An error was thrown on the first reload of comments'); - return comments.reload(); - }); + let manyArray; + + try { + manyArray = await comments; + assert.ok(false, 'Expected exception to be raised'); + } catch (e) { + assert.ok(true, `An error was thrown on the first reload of comments: ${e.message}`); + manyArray = await comments.reload(); + } assert.equal(manyArray.get('isLoaded'), true, 'the reload worked, comments are now loaded'); - await manyArray.reload().catch(() => { - assert.ok(true, 'An error was thrown on the second reload via manyArray'); - }); + try { + await manyArray.reload(); + assert.ok(false, 'Expected exception to be raised'); + } catch (e) { + assert.ok(true, `An error was thrown on the second reload via manyArray: ${e.message}`); + } assert.equal( manyArray.get('isLoaded'), @@ -1020,6 +1028,7 @@ module('integration/relationships/has_many - Has-Many Relationships', function(h 'the third reload worked, comments are loaded again' ); assert.ok(reloadedManyArray === manyArray, 'the many array stays the same'); + assert.equal(loadingCount, 4, 'We only fired 4 requests'); }); test('A hasMany relationship can be directly reloaded if it was fetched via links', function(assert) { @@ -2135,25 +2144,26 @@ module('integration/relationships/has_many - Has-Many Relationships', function(h ); }); - skip('A sync hasMany errors out if there are unloaded records in it', function(assert) { - let post = run(() => { - env.store.push({ - data: { - type: 'post', - id: '1', - relationships: { - comments: { - data: [{ type: 'comment', id: '1' }, { type: 'comment', id: '2' }], - }, + test('A sync hasMany errors out if there are unloaded records in it', function(assert) { + let post = env.store.push({ + data: { + type: 'post', + id: '1', + relationships: { + comments: { + data: [{ type: 'comment', id: '1' }, { type: 'comment', id: '2' }], }, }, - }); - return env.store.peekRecord('post', 1); + }, }); + const assertionMessage = /You looked up the 'comments' relationship on a 'post' with id 1 but some of the associated records were not loaded./; - assert.expectAssertion(() => { - run(post, 'get', 'comments'); - }, /You looked up the 'comments' relationship on a 'post' with id 1 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 }\)`\)/); + try { + post.get('comments'); + assert.ok(false, 'expected assertion'); + } catch (e) { + assert.ok(assertionMessage.test(e.message), 'correct assertion'); + } }); test('After removing and unloading a record, a hasMany relationship should still be valid', function(assert) { diff --git a/packages/store/addon/-private/system/many-array.js b/packages/store/addon/-private/system/many-array.js index 95cdf7095d2..1033631cad5 100644 --- a/packages/store/addon/-private/system/many-array.js +++ b/packages/store/addon/-private/system/many-array.js @@ -57,6 +57,10 @@ import recordDataFor from './record-data-for'; @uses Ember.MutableArray, Ember.Evented */ export default EmberObject.extend(MutableArray, Evented, { + // here to make TS happy + _inverseIsAsync: false, + isLoaded: false, + init() { this._super(...arguments); @@ -65,7 +69,7 @@ export default EmberObject.extend(MutableArray, Evented, { @property {Boolean} isLoaded */ - this.isLoaded = false; + this.isLoaded = this.isLoaded || false; this.length = 0; /** diff --git a/packages/store/addon/-private/system/model/internal-model.ts b/packages/store/addon/-private/system/model/internal-model.ts index c3a340633f8..b79eff477c4 100644 --- a/packages/store/addon/-private/system/model/internal-model.ts +++ b/packages/store/addon/-private/system/model/internal-model.ts @@ -4,7 +4,7 @@ import { default as EmberArray, A } from '@ember/array'; import { setOwner, getOwner } from '@ember/application'; import { run } from '@ember/runloop'; import { assign } from '@ember/polyfills'; -import RSVP, { Promise } from 'rsvp'; +import RSVP, { Promise, resolve } from 'rsvp'; import Ember from 'ember'; import { DEBUG } from '@glimmer/env'; import { assert, inspect } from '@ember/debug'; @@ -13,12 +13,22 @@ import Snapshot from '../snapshot'; import OrderedSet from '../ordered-set'; import ManyArray from '../many-array'; import { PromiseBelongsTo, PromiseManyArray } from '../promise-proxies'; +import Store from '../store'; import { RecordReference, BelongsToReference, HasManyReference } from '../references'; import { default as recordDataFor, relationshipStateFor } from '../record-data-for'; import RecordDataDefault from './record-data'; import RecordData from '../../ts-interfaces/record-data'; import { JsonApiResource } from '../../ts-interfaces/record-data-json-api'; +import { Dict } from '../../types'; +import BelongsToRelationship from '../relationships/state/belongs-to'; + +interface BelongsToMetaWrapper { + key: string; + store: InstanceType; + originatingInternalModel: InternalModel; + modelName: string; +} /* The TransitionChainMap caches the `state.enters`, `state.setups`, and final state reached @@ -63,7 +73,7 @@ let InternalModelReferenceId = 1; */ export default class InternalModel { id: string | null; - store: any; + store: InstanceType; modelName: string; clientId: string | null; __recordData: RecordData | null; @@ -84,9 +94,17 @@ export default class InternalModel { __recordArrays: any; _references: any; _recordReference: any; - _manyArrayCache: any; - _retainedManyArrayCache: any; - _relationshipPromisesCache: any; + _manyArrayCache: Dict> = Object.create(null); + + // The previous ManyArrays for this relationship which will be destroyed when + // we create a new ManyArray, but in the interim the retained version will be + // updated if inverse internal models are unloaded. + _retainedManyArrayCache: Dict> = Object.create(null); + _relationshipPromisesCache: Dict> = Object.create(null); + _relationshipProxyCache: Dict< + string, + InstanceType | InstanceType + > = Object.create(null); currentState: any; error: any; @@ -123,13 +141,6 @@ export default class InternalModel { this.__recordArrays = null; this._references = null; this._recordReference = null; - - this._manyArrayCache = Object.create(null); - // The previous ManyArrays for this relationship which will be destroyed when - // we create a new ManyArray, but in the interim the retained version will be - // updated if inverse internal models are unloaded. - this._retainedManyArrayCache = Object.create(null); - this._relationshipPromisesCache = Object.create(null); } get modelClass() { @@ -293,12 +304,8 @@ export default class InternalModel { let additionalCreateOptions = this._recordData._initRecordCreateOptions(properties); assign(createOptions, additionalCreateOptions); - if (setOwner) { - // ensure that `getOwner(this)` works inside a model instance - setOwner(createOptions, getOwner(store)); - } else { - createOptions.container = store.container; - } + // ensure that `getOwner(this)` works inside a model instance + setOwner(createOptions, getOwner(store)); this._record = store._modelFactoryFor(this.modelName).create(createOptions); @@ -325,12 +332,12 @@ export default class InternalModel { if (this._record) { this._record.destroy(); - Object.keys(this._relationshipPromisesCache).forEach(key => { + Object.keys(this._relationshipProxyCache).forEach(key => { // TODO Igor cleanup the guard - if (this._relationshipPromisesCache[key].destroy) { - this._relationshipPromisesCache[key].destroy(); + if (this._relationshipProxyCache[key].destroy) { + this._relationshipProxyCache[key].destroy(); } - delete this._relationshipPromisesCache[key]; + delete this._relationshipProxyCache[key]; }); Object.keys(this._manyArrayCache).forEach(key => { let manyArray = (this._retainedManyArrayCache[key] = this._manyArrayCache[key]); @@ -496,6 +503,22 @@ export default class InternalModel { return this.modelClass.eachRelationship(callback, binding); } + _findBelongsTo(key, resource, relationshipMeta, options) { + return this.store + ._findBelongsToByJsonApiResource(resource, this, relationshipMeta, options) + .then( + internalModel => + handleCompletedRelationshipRequest( + this, + key, + resource._relationship, + internalModel, + null + ), + e => handleCompletedRelationshipRequest(this, key, resource._relationship, null, e) + ); + } + getBelongsTo(key, options) { let resource = this._recordData.getBelongsTo(key); let relationshipMeta = this.store._relationshipMetaFor(this.modelName, null, key); @@ -503,25 +526,27 @@ export default class InternalModel { let parentInternalModel = this; let async = relationshipMeta.options.async; let isAsync = typeof async === 'undefined' ? true : async; - let _belongsToState = { - key, - store, + let _belongsToState: BelongsToMetaWrapper = { + key, + store, originatingInternalModel: this, - modelName: relationshipMeta.type - } + modelName: relationshipMeta.type, + }; if (isAsync) { let internalModel = resource && resource.data ? store._internalModelForResource(resource.data) : null; - return PromiseBelongsTo.create({ - _belongsToState, - promise: store._findBelongsToByJsonApiResource( - resource, - parentInternalModel, - relationshipMeta, - options - ), + + if (resource!._relationship!.hasFailedLoadAttempt) { + return this._relationshipProxyCache[key]; + } + + let promise = this._findBelongsTo(key, resource, relationshipMeta, options); + + return this._updatePromiseProxyFor('belongsTo', key, { + promise, content: internalModel ? internalModel.getRecord() : null, + _belongsToState, }); } else { if (!resource || !resource.data) { @@ -545,7 +570,7 @@ export default class InternalModel { } // TODO Igor consider getting rid of initial state - getManyArray(key) { + getManyArray(key, isAsync = false) { let relationshipMeta = this.store._relationshipMetaFor(this.modelName, null, key); let jsonApi = this._recordData.getHasMany(key); let manyArray = this._manyArrayCache[key]; @@ -569,6 +594,7 @@ export default class InternalModel { initialState: initialState.slice(), _inverseIsAsync: inverseIsAsync, internalModel: this, + isLoaded: !isAsync, }); this._manyArrayCache[key] = manyArray; } @@ -581,21 +607,28 @@ export default class InternalModel { return manyArray; } - fetchAsyncHasMany(relationshipMeta, jsonApi, manyArray, options) { - let promise = this.store._findHasManyByJsonApiResource( - jsonApi, - this, - relationshipMeta, - options - ); - promise = promise.then(initialState => { - // TODO why don't we do this in the store method - manyArray.retrieveLatest(); - manyArray.set('isLoaded', true); + fetchAsyncHasMany(key, relationshipMeta, jsonApi, manyArray, options): RSVP.Promise { + let loadingPromise = this._relationshipPromisesCache[key]; + if (loadingPromise) { + return loadingPromise; + } - return manyArray; - }); - return promise; + loadingPromise = this.store + ._findHasManyByJsonApiResource(jsonApi, this, relationshipMeta, options) + .then(initialState => { + // TODO why don't we do this in the store method + manyArray.retrieveLatest(); + manyArray.set('isLoaded', true); + + return manyArray; + }) + .then( + manyArray => + handleCompletedRelationshipRequest(this, key, jsonApi._relationship, manyArray, null), + e => handleCompletedRelationshipRequest(this, key, jsonApi._relationship, null, e) + ); + this._relationshipPromisesCache[key] = loadingPromise; + return loadingPromise; } getHasMany(key, options) { @@ -603,22 +636,17 @@ export default class InternalModel { let relationshipMeta = this.store._relationshipMetaFor(this.modelName, null, key); let async = relationshipMeta.options.async; let isAsync = typeof async === 'undefined' ? true : async; - let manyArray = this.getManyArray(key); + let manyArray = this.getManyArray(key, isAsync); if (isAsync) { - let promiseArray = this._relationshipPromisesCache[key]; - - if (!promiseArray) { - promiseArray = PromiseManyArray.create({ - promise: this.fetchAsyncHasMany(relationshipMeta, jsonApi, manyArray, options), - content: manyArray, - }); - this._relationshipPromisesCache[key] = promiseArray; + if (jsonApi!._relationship!.hasFailedLoadAttempt) { + return this._relationshipProxyCache[key]; } - return promiseArray; + let promise = this.fetchAsyncHasMany(key, relationshipMeta, jsonApi, manyArray, options); + + return this._updatePromiseProxyFor('hasMany', key, { promise, content: manyArray }); } else { - manyArray.set('isLoaded', true); assert( `You looked up the '${key}' relationship on a '${this.type.modelName}' with id ${ this.id @@ -630,59 +658,83 @@ export default class InternalModel { } } - _updateLoadingPromiseForHasMany(key, promise, content?) { - let loadingPromise = this._relationshipPromisesCache[key]; - if (loadingPromise) { - if (content) { - loadingPromise.set('content', content); + _updatePromiseProxyFor( + kind: 'hasMany' | 'belongsTo', + key: string, + args: { + promise: RSVP.Promise; + // TODO @runspired + // follow up with @mikenorth about what to do here as + // we don't have a clear way to type instances of records + // it can only be + // + // * ManyArray + // * null + // * a record instance (where record may be user defined. instanceof DS.Model is not accurate) + // * possibly? undefined when the initial promise has not resolved (I suspect null in this case in reality) + // + // unknown seems better as this is something we can "know" later and it is one of a finite set of things, + // but instance types do not seem to accept `unknown` when we do + // `promiseProxy.set('content', args.content);` whereas they do accept `any` + content?: any; + _belongsToState?: BelongsToMetaWrapper; + } + ) { + let promiseProxy = this._relationshipProxyCache[key]; + if (promiseProxy) { + if (args.content !== undefined) { + promiseProxy.set('content', args.content); } - loadingPromise.set('promise', promise); + promiseProxy.set('promise', args.promise); } else { - this._relationshipPromisesCache[key] = PromiseManyArray.create({ - promise, - content, - }); + const klass = kind === 'hasMany' ? PromiseManyArray : PromiseBelongsTo; + this._relationshipProxyCache[key] = klass.create(args); } - return this._relationshipPromisesCache[key]; + return this._relationshipProxyCache[key]; } reloadHasMany(key, options) { let loadingPromise = this._relationshipPromisesCache[key]; if (loadingPromise) { - if (loadingPromise.get('isPending')) { - return loadingPromise; - } - /* TODO Igor check wtf this is about - if (loadingPromise.get('isRejected')) { - manyArray.set('isLoaded', manyArrayLoadedState); - } - */ + return loadingPromise; } let jsonApi = this._recordData.getHasMany(key); // TODO move this to a public api if (jsonApi._relationship) { - jsonApi._relationship.setRelationshipIsStale(true); + jsonApi._relationship.setHasFailedLoadAttempt(false); + jsonApi._relationship.setShouldForceReload(true); } let relationshipMeta = this.store._relationshipMetaFor(this.modelName, null, key); let manyArray = this.getManyArray(key); - let promise = this.fetchAsyncHasMany(relationshipMeta, jsonApi, manyArray, options); + let promise = this.fetchAsyncHasMany(key, relationshipMeta, jsonApi, manyArray, options); + + if (this._relationshipProxyCache[key]) { + return this._updatePromiseProxyFor('hasMany', key, { promise }); + } - // TODO igor Seems like this would mess with promiseArray wrapping, investigate - this._updateLoadingPromiseForHasMany(key, promise); return promise; } reloadBelongsTo(key, options) { + let loadingPromise = this._relationshipPromisesCache[key]; + if (loadingPromise) { + return loadingPromise; + } + let resource = this._recordData.getBelongsTo(key); // TODO move this to a public api if (resource._relationship) { - resource._relationship.setRelationshipIsStale(true); + resource._relationship.setHasFailedLoadAttempt(false); + resource._relationship.setShouldForceReload(true); } let relationshipMeta = this.store._relationshipMetaFor(this.modelName, null, key); - - return this.store._findBelongsToByJsonApiResource(resource, this, relationshipMeta, options); + let promise = this._findBelongsTo(key, resource, relationshipMeta, options); + if (this._relationshipProxyCache[key]) { + return this._updatePromiseProxyFor('belongsTo', key, { promise }); + } + return promise; } destroyFromRecordData() { @@ -879,11 +931,6 @@ export default class InternalModel { // // that said, also not clear why we haven't moved this to retainedmanyarray so maybe that's the bit that's just not workign manyArray.retrieveLatest(); - // TODO Igor be rigorous about when to delete this - // TODO: igor check for case where we later unload again - if (this._relationshipPromisesCache[key] && manyArray.anyUnloaded()) { - delete this._relationshipPromisesCache[key]; - } } this.updateRecordArrays(); } @@ -904,15 +951,13 @@ export default class InternalModel { let manyArray = this._manyArrayCache[key] || this._retainedManyArrayCache[key]; if (manyArray) { let didRemoveUnloadedModel = manyArray.removeUnloadedInternalModel(); + + // TODO @runspired why do we do this? if (this._manyArrayCache[key] && didRemoveUnloadedModel) { this._retainedManyArrayCache[key] = this._manyArrayCache[key]; delete this._manyArrayCache[key]; } } - if (this._relationshipPromisesCache[key]) { - this._relationshipPromisesCache[key].destroy(); - delete this._relationshipPromisesCache[key]; - } } didCreateRecord() { @@ -1112,6 +1157,7 @@ export default class InternalModel { @private */ updateRecordArrays() { + // @ts-ignore: Store is untyped and typescript does not detect instance props set in `init` this.store.recordArrayManager.recordDidChange(this); } @@ -1266,6 +1312,41 @@ export default class InternalModel { } } +// TODO @runspired follow up if parent isNew then we should not be attempting load here + +function handleCompletedRelationshipRequest(internalModel, key, relationship, value, error) { + delete internalModel._relationshipPromisesCache[key]; + relationship.setShouldForceReload(false); + + if (error) { + relationship.setHasFailedLoadAttempt(true); + let proxy = internalModel._relationshipProxyCache[key]; + // belongsTo relationships are sometimes unloaded + // when a load fails, in this case we need + // to make sure that we aren't proxying + // to destroyed content + if (relationship.kind === 'belongsTo') { + if (proxy.content.isDestroying) { + proxy.set('content', null); + } + + // clear the promise to make re-access safe + // e.g. after initial rejection, don't replay + // rejection on subsequent access, otherwise + // templates cause lots of rejected promise blow-ups + proxy.set('promise', resolve(null)); + } + + throw error; + } + + relationship.setHasFailedLoadAttempt(false); + // only set to not stale if no error is thrown + relationship.setRelationshipIsStale(false); + + return value; +} + function assertRecordsPassedToHasMany(records) { // TODO only allow native arrays assert( diff --git a/packages/store/addon/-private/system/promise-proxies.js b/packages/store/addon/-private/system/promise-proxies.js index ea36f38b50a..d77c8e41ea8 100644 --- a/packages/store/addon/-private/system/promise-proxies.js +++ b/packages/store/addon/-private/system/promise-proxies.js @@ -108,6 +108,12 @@ export const PromiseBelongsTo = PromiseObject.extend({ }, }); +export function proxyToContent(method) { + return function() { + return get(this, 'content')[method](...arguments); + }; +} + /** A PromiseManyArray is a PromiseArray that also proxies certain method calls to the underlying manyArray. @@ -125,13 +131,6 @@ export const PromiseBelongsTo = PromiseObject.extend({ @namespace DS @extends Ember.ArrayProxy */ - -export function proxyToContent(method) { - return function() { - return get(this, 'content')[method](...arguments); - }; -} - export const PromiseManyArray = PromiseArray.extend({ reload(options) { assert( diff --git a/packages/store/addon/-private/system/references/belongs-to.js b/packages/store/addon/-private/system/references/belongs-to.js index f75d0ae4ab4..740ddbdc21b 100644 --- a/packages/store/addon/-private/system/references/belongs-to.js +++ b/packages/store/addon/-private/system/references/belongs-to.js @@ -313,31 +313,9 @@ export default class BelongsToReference extends Reference { @param {Object} options the options to pass in. @return {Promise} a promise that resolves with the record in this belongs-to relationship after the reload has completed. */ - // TODO IGOR CHECK FOR OBJECT PROXIES reload(options) { - let resource = this._resource(); - if (resource && resource.links && resource.links.related) { - return this.store._fetchBelongsToLinkFromResource( - resource, - this.parentInternalModel, - this.belongsToRelationship.relationshipMeta, - options - ); - } - if (resource && resource.data) { - if (resource.data && (resource.data.id || resource.data.clientId)) { - let internalModel = this.store._internalModelForResource(resource.data); - if (internalModel.isLoaded()) { - return internalModel.reload(options).then(internalModel => { - if (internalModel) { - return internalModel.getRecord(); - } - return null; - }); - } else { - return this.store._findByInternalModel(internalModel, options); - } - } - } + return this.parentInternalModel.reloadBelongsTo(this.key, options).then(internalModel => { + return this.value(); + }); } } diff --git a/packages/store/addon/-private/system/relationships/state/relationship.ts b/packages/store/addon/-private/system/relationships/state/relationship.ts index 19d2ef5d461..27c31be2189 100644 --- a/packages/store/addon/-private/system/relationships/state/relationship.ts +++ b/packages/store/addon/-private/system/relationships/state/relationship.ts @@ -4,13 +4,13 @@ import { relationshipStateFor } from '../../record-data-for'; import { assert, warn } from '@ember/debug'; import OrderedSet from '../../ordered-set'; import _normalizeLink from '../../normalize-link'; -import { RelationshipRecordData } from "../../..//ts-interfaces/relationship-record-data"; +import { RelationshipRecordData } from '../../..//ts-interfaces/relationship-record-data'; import RecordData from '../../../ts-interfaces/record-data'; -import { JsonApiRelationship } from "../../../ts-interfaces/record-data-json-api"; -import { RelationshipSchema } from "../../../ts-interfaces/record-data-schemas"; +import { JsonApiRelationship } from '../../../ts-interfaces/record-data-json-api'; +import { RelationshipSchema } from '../../../ts-interfaces/record-data-schemas'; interface ImplicitRelationshipMeta { - key?: string, + key?: string; kind?: string; options: any; } @@ -31,15 +31,22 @@ export default class Relationship { meta: any; __inverseMeta: any; _tempModelName: string; - shouldForceReload: boolean; + shouldForceReload: boolean = false; relationshipIsStale: boolean; hasDematerializedInverse: boolean; hasAnyRelationshipData: boolean; relationshipIsEmpty: boolean; + hasFailedLoadAttempt: boolean = false; link?: string | null; willSync?: boolean; - constructor(store: any, inverseKey: string, relationshipMeta: ImplicitRelationshipMeta, recordData: RelationshipRecordData, inverseIsAsync?: boolean) { + constructor( + store: any, + inverseKey: string, + relationshipMeta: ImplicitRelationshipMeta, + recordData: RelationshipRecordData, + inverseIsAsync?: boolean + ) { this.inverseIsAsync = inverseIsAsync; this.kind = relationshipMeta.kind; let async = relationshipMeta.options.async; @@ -63,7 +70,7 @@ export default class Relationship { This flag forces fetch. `true` for a single request once `reload()` has been called `false` at all other times. */ - this.shouldForceReload = false; + // this.shouldForceReload = false; /* This flag indicates whether we should @@ -214,12 +221,10 @@ export default class Relationship { } recordDataDidDematerialize() { - const inverseKey = this.inverseKey + const inverseKey = this.inverseKey; if (!inverseKey) { return; } - // TODO @runspired fairly sure we need to become stale here - // this.setRelationshipIsStale(true); // we actually want a union of members and canonicalMembers // they should be disjoint but currently are not due to a bug @@ -277,12 +282,11 @@ export default class Relationship { } } - updateMeta(meta : any) { + updateMeta(meta: any) { this.meta = meta; } clear() { - let members = this.members.list; while (members.length > 0) { let member = members[0]; @@ -360,7 +364,7 @@ export default class Relationship { relationship = relationships[this.inverseKeyForImplicit] = new Relationship( this.store, // we know we are not an implicit relationship here - (this.key as string), + this.key as string, { options: { async: this.isAsync } }, recordData ); @@ -410,7 +414,7 @@ export default class Relationship { recordData._implicitRelationships[this.inverseKeyForImplicit] = new Relationship( this.store, // we know we are not an implicit relationship here - (this.key as string), + this.key as string, { options: { async: this.isAsync } }, recordData, this.isAsync @@ -597,6 +601,14 @@ export default class Relationship { this.relationshipIsEmpty = value; } + setShouldForceReload(value: boolean) { + this.shouldForceReload = value; + } + + setHasFailedLoadAttempt(value: boolean) { + this.hasFailedLoadAttempt = value; + } + /* `push` for a relationship allows the store to push a JSON API Relationship Object onto the relationship. The relationship will then extract and set the @@ -646,6 +658,7 @@ export default class Relationship { IF contains only links relationshipIsStale -> true */ + this.setHasFailedLoadAttempt(false); if (hasRelationshipDataProperty) { let relationshipIsEmpty = payload.data === null || (Array.isArray(payload.data) && payload.data.length === 0); @@ -665,7 +678,7 @@ export default class Relationship { recordData.id, recordData.clientId, // We know we are not an implicit relationship here - (this.key as string) + this.key as string ); } } diff --git a/packages/store/addon/-private/system/store.js b/packages/store/addon/-private/system/store.js index 1e4614e7de0..63d7028c9e1 100644 --- a/packages/store/addon/-private/system/store.js +++ b/packages/store/addon/-private/system/store.js @@ -1340,12 +1340,14 @@ const Store = Service.extend({ hasDematerializedInverse, hasAnyRelationshipData, relationshipIsEmpty, + shouldForceReload, } = resource._relationship; let shouldFindViaLink = resource.links && resource.links.related && - (hasDematerializedInverse || + (shouldForceReload || + hasDematerializedInverse || relationshipIsStale || (!allInverseRecordsAreLoaded && !relationshipIsEmpty)); @@ -1373,7 +1375,7 @@ const Store = Service.extend({ (relationshipIsEmpty && Array.isArray(resource.data) && resource.data.length > 0); // fetch using data, pulling from local cache if possible - if (!relationshipIsStale && (preferLocalCache || hasLocalPartialData)) { + if (!shouldForceReload && !relationshipIsStale && (preferLocalCache || hasLocalPartialData)) { let internalModels = resource.data.map(json => this._internalModelForResource(json)); return this.findMany(internalModels, options); @@ -1462,12 +1464,14 @@ const Store = Service.extend({ hasDematerializedInverse, hasAnyRelationshipData, relationshipIsEmpty, + shouldForceReload, } = resource._relationship; let shouldFindViaLink = resource.links && resource.links.related && - (hasDematerializedInverse || + (shouldForceReload || + hasDematerializedInverse || relationshipIsStale || (!allInverseRecordsAreLoaded && !relationshipIsEmpty)); @@ -1495,7 +1499,7 @@ const Store = Service.extend({ let localDataIsEmpty = resource.data === undefined || resource.data === null; // fetch using data, pulling from local cache if possible - if (!relationshipIsStale && (preferLocalCache || hasLocalPartialData)) { + if (!shouldForceReload && !relationshipIsStale && (preferLocalCache || hasLocalPartialData)) { /* We have canonical data, but our local state is empty */ diff --git a/packages/store/addon/-private/system/store/record-data-store-wrapper.ts b/packages/store/addon/-private/system/store/record-data-store-wrapper.ts index 1bc57f2f482..ae387fc5d2a 100644 --- a/packages/store/addon/-private/system/store/record-data-store-wrapper.ts +++ b/packages/store/addon/-private/system/store/record-data-store-wrapper.ts @@ -1,10 +1,10 @@ -import { RecordDataStoreWrapper as IRecordDataStoreWrapper } from "../../ts-interfaces/record-data-store-wrapper"; +import { RecordDataStoreWrapper as IRecordDataStoreWrapper } from '../../ts-interfaces/record-data-store-wrapper'; export default class RecordDataStoreWrapper implements IRecordDataStoreWrapper { store: any; - _willUpdateManyArrays: boolean; + _willUpdateManyArrays: boolean; _pendingManyArrayUpdates: string[]; - + constructor(store) { this.store = store; this._willUpdateManyArrays = false;