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

fix: account for multi-options-per-identifer possibility #8113

Merged
merged 12 commits into from
Aug 12, 2022
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
10 changes: 9 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ jobs:
ember-observer,
# ember-resource-metadata,
factory-guy,
# ilios-frontend,
ilios-frontend,
model-fragments,
storefront,
travis-web,
Expand All @@ -330,10 +330,14 @@ jobs:
continue-on-error: true
- partner: travis-web
continue-on-error: true
- partner: ilios-frontend
continue-on-error: true
- partner: model-fragments
continue-on-error: true
- partner: ember-observer
continue-on-error: true
- partner: ember-m3
continue-on-error: true
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
Expand All @@ -345,8 +349,12 @@ jobs:
- name: Generate package tarballs
run: node ./scripts/packages-for-commit.js
- name: Run Tests
id: test-partner
timeout-minutes: 16
env:
CI: true
run: yarn test-external:${{ matrix.partner }}
continue-on-error: ${{ matrix['continue-on-error'] == true }}
- name: Check on failures
if: ${{ matrix['continue-on-error'] == true && steps.test-partner.outcome == 'success' }}
run: exit 1
38 changes: 20 additions & 18 deletions packages/-ember-data/tests/integration/adapter/rest-adapter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ module('integration/adapter/rest_adapter - REST Adapter', function (hooks) {

await store.findAll('post');

let comment = store.peekRecord('comment', 1);
let comment = store.peekRecord('comment', '1');
assert.deepEqual(comment.getProperties('id', 'name'), { id: '1', name: 'FIRST' });
});

Expand Down Expand Up @@ -1509,7 +1509,7 @@ module('integration/adapter/rest_adapter - REST Adapter', function (hooks) {
this.owner.register('model:comment', Comment);
adapter.coalesceFindRequests = true;

store.push({
const post = store.push({
data: {
type: 'post',
id: '1',
Expand All @@ -1528,7 +1528,6 @@ module('integration/adapter/rest_adapter - REST Adapter', function (hooks) {
},
});

let post = await store.findRecord('post', 1);
ajaxResponse({
comments: [
{ id: '1', name: 'FIRST' },
Expand All @@ -1541,11 +1540,11 @@ module('integration/adapter/rest_adapter - REST Adapter', function (hooks) {

let comments = await post.comments;

let comment1 = store.peekRecord('comment', 1);
let comment2 = store.peekRecord('comment', 2);
let comment3 = store.peekRecord('comment', 3);
let comment4 = store.peekRecord('comment', 4);
let post2 = store.peekRecord('post', 2);
let comment1 = store.peekRecord('comment', '1');
let comment2 = store.peekRecord('comment', '2');
let comment3 = store.peekRecord('comment', '3');
let comment4 = store.peekRecord('comment', '4');
let post2 = store.peekRecord('post', '2');

assert.deepEqual(comments.toArray(), [comment1, comment2, comment3], 'The correct records are in the array');

Expand Down Expand Up @@ -1948,16 +1947,19 @@ module('integration/adapter/rest_adapter - REST Adapter', function (hooks) {
},
});

assert.expectWarning(async () => {
try {
await post.comments;
} catch (e) {
assert.strictEqual(
e.message,
`Expected: '<comment:2>' to be present in the adapter provided payload, but it was not found.`
);
}
}, /expected to find records with the following ids in the adapter response but they were missing: \[ "2", "3" \]/);
assert.expectWarning(
async () => {
try {
await post.comments;
} catch (e) {
assert.strictEqual(
e.message,
`Expected: '<comment:2>' to be present in the adapter provided payload, but it was not found.`
);
}
},
{ id: 'ds.store.missing-records-from-adapter' }
);
}
);

Expand Down
35 changes: 19 additions & 16 deletions packages/-ember-data/tests/unit/store/adapter-interop-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ module('unit/store/adapter-interop - Store working with a Adapter', function (ho
});

testInDebug(
'store._fetchRecord reject records that were not found, even when those requests were coalesced with records that were found',
'store.findRecord reject records that were not found, even when those requests were coalesced with records that were found',
function (assert) {
assert.expect(3);

Expand Down Expand Up @@ -862,7 +862,7 @@ module('unit/store/adapter-interop - Store working with a Adapter', function (ho
}
);

testInDebug('store._fetchRecord warns when records are missing', function (assert) {
testInDebug('store.findRecord warns when records are missing', function (assert) {
const ApplicationAdapter = Adapter.extend({
findMany(store, type, ids, snapshots) {
let records = ids.map((id) => ({ id, type: 'test' })).filter(({ id }) => id === 'david');
Expand All @@ -880,20 +880,23 @@ module('unit/store/adapter-interop - Store working with a Adapter', function (ho
let wait = [];
let igorDidReject = true;

assert.expectWarning(() => {
run(() => {
wait.push(store.findRecord('test', 'david'));
wait.push(
store.findRecord('test', 'igor').catch((e) => {
igorDidReject = true;
assert.strictEqual(
e.message,
`Expected: '<test:igor>' to be present in the adapter provided payload, but it was not found.`
);
})
);
});
}, /expected to find records with the following ids in the adapter response but they were missing/);
assert.expectWarning(
() => {
run(() => {
wait.push(store.findRecord('test', 'david'));
wait.push(
store.findRecord('test', 'igor').catch((e) => {
igorDidReject = true;
assert.strictEqual(
e.message,
`Expected: '<test:igor>' to be present in the adapter provided payload, but it was not found.`
);
})
);
});
},
{ id: 'ds.store.missing-records-from-adapter' }
);

return EmberPromise.all(wait).then(() => {
assert.ok(
Expand Down
2 changes: 1 addition & 1 deletion packages/record-data/addon/-private/record-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ export default class RecordDataDefault implements RelationshipRecordData {
@private
*/
/*
TODO IGOR DAVID
TODO @deprecate IGOR DAVID
There seems to be a potential bug here, where we will return keys that are not
in the schema
*/
Expand Down
1 change: 0 additions & 1 deletion packages/serializer/addon/json-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,6 @@ const JSONAPISerializer = JSONSerializer.extend({
@param {String} modelName
@return {String}
*/
// TODO @deprecated Use payloadTypeFromModelName instead
payloadKeyFromModelName(modelName) {
return pluralize(modelName);
},
Expand Down
19 changes: 8 additions & 11 deletions packages/store/addon/-private/caches/identifier-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,11 @@ export class IdentifierCache {
lids: Object.create(null) as IdentifierMap,
types: Object.create(null) as TypeMap,
};
private _generate: GenerationMethod;
private _update: UpdateMethod;
private _forget: ForgetMethod;
private _reset: ResetMethod;
private _merge: MergeMethod;
declare _generate: GenerationMethod;
declare _update: UpdateMethod;
declare _forget: ForgetMethod;
declare _reset: ResetMethod;
declare _merge: MergeMethod;

constructor() {
// we cache the user configuredGenerationMethod at init because it must
Expand Down Expand Up @@ -156,12 +156,9 @@ export class IdentifierCache {
* @method _getRecordIdentifier
* @private
*/
private _getRecordIdentifier(resource: ResourceIdentifierObject, shouldGenerate: true): StableRecordIdentifier;
private _getRecordIdentifier(
resource: ResourceIdentifierObject,
shouldGenerate: false
): StableRecordIdentifier | undefined;
private _getRecordIdentifier(
_getRecordIdentifier(resource: ResourceIdentifierObject, shouldGenerate: true): StableRecordIdentifier;
_getRecordIdentifier(resource: ResourceIdentifierObject, shouldGenerate: false): StableRecordIdentifier | undefined;
_getRecordIdentifier(
resource: ResourceIdentifierObject,
shouldGenerate: boolean = false
): StableRecordIdentifier | undefined {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ import type Store from '../store-service';
@extends Reference
*/
export default class RecordReference {
declare store: Store;
// unsubscribe token given to us by the notification manager
#token!: Object;
#identifier: StableRecordIdentifier;

@tracked _ref = 0;

constructor(public store: Store, identifier: StableRecordIdentifier) {
constructor(store: Store, identifier: StableRecordIdentifier) {
this.store = store;
this.#identifier = identifier;
this.#token = store._notificationManager.subscribe(
identifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@ function mapFromHash<T>(hash: Dict<T>): Map<string, T> {

// Mimics the static apis of DSModel
export default class ShimModelClass implements ModelSchema {
// TODO Maybe expose the class here?
constructor(private __store: Store, public modelName: string) {}
declare __store: Store;
declare modelName: string;
constructor(store: Store, modelName: string) {
this.__store = store;
this.modelName = modelName;
}

get fields(): Map<string, 'attribute' | 'belongsTo' | 'hasMany'> {
let attrs = this.__store.getSchemaDefinitionService().attributesDefinitionFor({ type: this.modelName });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ export function unsubscribe(token: UnsubscribeToken) {
Currently only support a single callback per identifier
*/
export default class NotificationManager {
constructor(private store: Store) {}
declare store: Store;
constructor(store: Store) {
this.store = store;
}

subscribe(identifier: StableRecordIdentifier, callback: NotificationCallback): UnsubscribeToken {
assert(`Expected to receive a stable Identifier to subscribe to`, isStableIdentifier(identifier));
Expand Down
Loading