Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CHORE] - Removes allInverseRecordsAreLoaded from has-many and belongs-to state classes #6574

Merged
merged 3 commits into from
Nov 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -194,22 +194,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 @@ -255,30 +255,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 @@ -1615,12 +1617,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 @@ -1733,12 +1735,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 @@ -3659,3 +3661,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);
}