Skip to content

Commit

Permalink
[CHORE] - Removes allInverseRecordsAreLoaded from has-many and belong…
Browse files Browse the repository at this point in the history
…s-to state classes (#6574)

Co-authored-by: Chris Thoburn <[email protected]>
  • Loading branch information
David Tang and runspired committed Nov 12, 2019
1 parent a544b5b commit b1c9fe8
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -500,11 +500,6 @@ module('async belongs-to rendering tests', function(hooks) {
assert.equal(relationshipState.isAsync, true, 'The relationship is async');
assert.equal(relationshipState.relationshipIsEmpty, false, 'The relationship is not empty');
assert.equal(relationshipState.hasDematerializedInverse, true, 'The relationship inverse is dematerialized');
assert.equal(
relationshipState.allInverseRecordsAreLoaded,
false,
'The relationship is missing some or all related resources'
);
assert.equal(relationshipState.hasAnyRelationshipData, true, 'The relationship knows which record it needs');
assert.equal(!!RelationshipPromiseCache['parent'], false, 'The relationship has no fetch promise');
assert.equal(relationshipState.hasFailedLoadAttempt === true, true, 'The relationship has attempted a load');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ module('async has-many rendering tests', function(hooks) {
});

test('Rendering an async hasMany whose fetch fails does not trigger a new request', async function(assert) {
assert.expect(12);
assert.expect(11);
let people = makePeopleWithRelationshipData();
let parent = store.push({
data: people.dict['3:has-2-children-and-parent'],
Expand Down Expand Up @@ -320,11 +320,6 @@ module('async has-many rendering tests', function(hooks) {
assert.equal(relationshipState.isAsync, true, 'The relationship is async');
assert.equal(relationshipState.relationshipIsEmpty, false, 'The relationship is not empty');
assert.equal(relationshipState.hasDematerializedInverse, true, 'The relationship has a dematerialized inverse');
assert.equal(
relationshipState.allInverseRecordsAreLoaded,
false,
'The relationship is missing some or all related resources'
);
assert.equal(relationshipState.hasAnyRelationshipData, true, 'The relationship knows which record it needs');
assert.equal(!!RelationshipPromiseCache['children'], false, 'The relationship has no fetch promise');
assert.equal(relationshipState.hasFailedLoadAttempt === true, true, 'The relationship has attempted a load');
Expand Down Expand Up @@ -399,7 +394,7 @@ module('async has-many rendering tests', function(hooks) {
});

test('Rendering an async hasMany with a link whose fetch fails does not trigger a new request', async function(assert) {
assert.expect(12);
assert.expect(11);
let people = makePeopleWithRelationshipLinks(true);
let parent = store.push({
data: people.dict['3:has-2-children-and-parent'],
Expand Down Expand Up @@ -448,11 +443,6 @@ module('async has-many rendering tests', function(hooks) {
'The relationship is empty because no signal has been received as to true state'
);
assert.equal(relationshipState.relationshipIsStale, true, 'The relationship is still stale');
assert.equal(
relationshipState.allInverseRecordsAreLoaded,
true,
'The relationship is missing some or all related resources'
);
assert.equal(relationshipState.hasAnyRelationshipData, false, 'The relationship knows which record it needs');
assert.equal(!!RelationshipPromiseCache['children'], false, 'The relationship has no fetch promise');
assert.equal(!!RelationshipProxyCache['children'], true, 'The relationship has a promise proxy');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1461,7 +1461,7 @@ module('integration/relationship/belongs_to Belongs-To Relationships', function(
});
});

test('Related link should take precedence over relationship data if no local record data is available', function(assert) {
test('Related link should take precedence over relationship data if no local record data is available', async function(assert) {
assert.expect(2);

Book.reopen({
Expand All @@ -1486,26 +1486,24 @@ module('integration/relationship/belongs_to Belongs-To Relationships', function(
assert.ok(false, "The adapter's findRecord method should not be called");
};

return run(() => {
let book = store.push({
data: {
type: 'book',
id: '1',
relationships: {
author: {
links: {
related: 'author',
},
data: { type: 'author', id: '1' },
let book = store.push({
data: {
type: 'book',
id: '1',
relationships: {
author: {
links: {
related: 'author',
},
data: { type: 'author', id: '1' },
},
},
});

return book.get('author').then(author => {
assert.equal(author.get('name'), 'This is author', 'author name is correct');
});
},
});

const author = await book.get('author');

assert.equal(author.get('name'), 'This is author', 'author name is correct');
});

test('Relationship data should take precedence over related link when local record data is available', function(assert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ module('integration/relationships/one_to_one_test - OneToOne relationships', fun
true,
'The relationship considers its canonical data complete'
);
assert.equal(user1bestFriendState.allInverseRecordsAreLoaded, true, 'The relationship has all required data');
});

test('Fetching a belongsTo that is set to a different record, sets the old relationship to null - sync', async function(assert) {
Expand Down Expand Up @@ -383,7 +382,6 @@ module('integration/relationships/one_to_one_test - OneToOne relationships', fun
assert.equal(user1JobState.relationshipIsStale, false, 'The relationship is not stale');
assert.equal(user1JobState.shouldForceReload, false, 'The relationship does not require reload');
assert.equal(user1JobState.hasAnyRelationshipData, true, 'The relationship considers its canonical data complete');
assert.equal(user1JobState.allInverseRecordsAreLoaded, true, 'The relationship has all required data');
});

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,22 +191,6 @@ export default class BelongsToRelationship extends Relationship {
return payload;
}

/**
* Flag indicating whether all inverse records are available
*
* true if the inverse exists and is loaded (not empty)
* true if there is no inverse
* false if the inverse exists and is not loaded (empty)
*
* @return {boolean}
*/
get allInverseRecordsAreLoaded(): boolean {
let recordData = this.inverseRecordData;
let isEmpty = recordData !== null && recordData.isEmpty();

return !isEmpty;
}

updateData(data: ExistingResourceIdentifierObject, initial: boolean) {
let recordData;
if (isNone(data)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,30 +253,6 @@ export default class ManyRelationship extends Relationship {
this.updateRecordDatasFromAdapter(recordDatas);
}
}

/**
* Flag indicating whether all inverse records are available
*
* true if inverse records exist and are all loaded (all not empty)
* true if there are no inverse records
* false if the inverse records exist and any are not loaded (any empty)
*
* @return {boolean}
*/
get allInverseRecordsAreLoaded(): boolean {
// check currentState for unloaded records
let hasEmptyRecords = this.currentState.reduce((hasEmptyModel, i) => {
return hasEmptyModel || i.isEmpty();
}, false);
// check un-synced state for unloaded records
if (!hasEmptyRecords && this.willSync) {
hasEmptyRecords = this.canonicalState.reduce((hasEmptyModel, i) => {
return hasEmptyModel || !i.isEmpty();
}, false);
}

return !hasEmptyRecords;
}
}

function setForArray(array) {
Expand Down
46 changes: 44 additions & 2 deletions packages/store/addon/-private/system/core-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ import {
CUSTOM_MODEL_CLASS,
} from '@ember-data/canary-features';
import { Record } from '../ts-interfaces/record';
import { JsonApiRelationship } from '../ts-interfaces/record-data-json-api';
import { ResourceIdentifierObject } from '../ts-interfaces/ember-data-json-api';

import promiseRecord from '../utils/promise-record';
import { identifierCacheFor, IdentifierCache } from '../identifiers/cache';
Expand Down Expand Up @@ -1624,12 +1626,12 @@ abstract class CoreStore extends Service {

let {
relationshipIsStale,
allInverseRecordsAreLoaded,
hasDematerializedInverse,
hasAnyRelationshipData,
relationshipIsEmpty,
shouldForceReload,
} = resource._relationship;
const allInverseRecordsAreLoaded = areAllInverseRecordsLoaded(this, resource);

let shouldFindViaLink =
resource.links &&
Expand Down Expand Up @@ -1724,12 +1726,12 @@ abstract class CoreStore extends Service {
const internalModel = resource.data ? this._internalModelForResource(resource.data) : null;
let {
relationshipIsStale,
allInverseRecordsAreLoaded,
hasDematerializedInverse,
hasAnyRelationshipData,
relationshipIsEmpty,
shouldForceReload,
} = resource._relationship;
const allInverseRecordsAreLoaded = areAllInverseRecordsLoaded(this, resource);

let shouldFindViaLink =
resource.links &&
Expand Down Expand Up @@ -3649,3 +3651,43 @@ if (DEBUG) {
}
};
}

/**
* Flag indicating whether all inverse records are available
*
* true if the inverse exists and is loaded (not empty)
* true if there is no inverse
* false if the inverse exists and is not loaded (empty)
*
* @return {boolean}
*/
function areAllInverseRecordsLoaded(store: CoreStore, resource: JsonApiRelationship): boolean {
const cache = identifierCacheFor(store);

if (Array.isArray(resource.data)) {
// treat as collection
// check for unloaded records
let hasEmptyRecords = resource.data.reduce((hasEmptyModel, resourceIdentifier) => {
return hasEmptyModel || internalModelForRelatedResource(store, cache, resourceIdentifier).isEmpty();
}, false);

return !hasEmptyRecords;
} else {
// treat as single resource
if (!resource.data) {
return true;
} else {
const internalModel = internalModelForRelatedResource(store, cache, resource.data);
return !internalModel.isEmpty();
}
}
}

function internalModelForRelatedResource(
store: CoreStore,
cache: IdentifierCache,
resource: ResourceIdentifierObject
): InternalModel {
const identifier = cache.getOrCreateRecordIdentifier(resource);
return store._internalModelForResource(identifier);
}

0 comments on commit b1c9fe8

Please sign in to comment.