Skip to content

Commit

Permalink
[BUGFIX] fix single-table poymorphic-type-switch
Browse files Browse the repository at this point in the history
  • Loading branch information
@runspired
runspired committed Dec 13, 2019
1 parent d2bbf6c commit 972c811
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 73 deletions.
170 changes: 100 additions & 70 deletions packages/-ember-data/tests/integration/identifiers/polymorphic-scenarios-test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,76 +1,41 @@
import { module, test } from 'qunit';
import { resolve } from 'rsvp';

import { setupTest } from 'ember-qunit';
import { IDENTIFIERS } from '@ember-data/canary-features';
import Store from '@ember-data/store';
import Model, { attr, belongsTo } from '@ember-data/model';

import Adapter from '@ember-data/adapter';
import { IDENTIFIERS } from '@ember-data/canary-features';
import Model, { attr, belongsTo, hasMany } from '@ember-data/model';
import Serializer from '@ember-data/serializer';
import { resolve } from 'rsvp';
import Store from '@ember-data/store';

type RID = { type: string; id: string };

if (IDENTIFIERS) {
module('Integration | Identifiers - polymorphic scenarios', function(hooks) {
module('Integration | Identifiers - single-table-inheritance polymorphic scenarios', function(hooks) {
/*
In single-table polymorphism, each polymorphic type shares a common primaryKey field.
This is typically implemented in Databases as a single-table of which `type` is
just a field to filter by.
As such, in a single-table setup the exact same data can be represented by both a type
and the base type.
A common example is `bmw` and `ferrari` being polymorphic types implementing `car`.
In this case it is not possible for a `bwm` and a `ferrari` to share an identical `id`.
This results in it being true that `bmw:2` is the same record as `car:2` and `ferrari:1`
is the same record as `car:1`
*/
setupTest(hooks);

module('single-table', function(hooks) {
let store;
let calls;

class TestSerializer extends Serializer {
normalizeResponse(_, __, payload) {
return payload;
}
}
class TestAdapter extends Adapter {
shouldBackgroundReloadRecord() {
return false;
}
}

class CarAdapter extends TestAdapter {
findRecord() {
return resolve({
data: {
id: '1',
type: 'ferrari',
attributes: {
color: 'red'
},
},
});
}
}

class FerrariAdapter extends TestAdapter {
findRecord() {
return resolve({
data: {
id: '1',
type: 'ferrari',
attributes: {
color: 'red'
},
},
});
}
}

class DealershipAdapter extends TestAdapter {
findRecord() {
return resolve({
data: {
id: '1',
type: 'dealership',
attributes: {
name: 'Brand new* car'
},
relationships: {
car: {
data: { id: 1, type: 'ferrari' },
},
},
}
});
}
}

hooks.beforeEach(function() {
const { owner } = this;
Expand All @@ -79,36 +44,101 @@ if (IDENTIFIERS) {
@attr() color: string;
}

class Ferrari extends Car {
@attr() color: string;
}
class Ferrari extends Car {}
class Bmw extends Car {}

class Dealership extends Model {
@attr() name: string;
@belongsTo("car", { polymorphic: true }) car;
@belongsTo('car', { polymorphic: true, async: true, inverse: null }) bestCar;
@hasMany('car', { polymorphic: true, async: true, inverse: null }) allCars;
}

owner.register('adapter:car', CarAdapter);
owner.register('adapter:ferrari', FerrariAdapter);
owner.register('adapter:dealership', DealershipAdapter);
owner.register('serializer:application', TestSerializer);
owner.register('model:car', Car);
owner.register('model:ferrari', Ferrari);
owner.register('model:bmw', Bmw);
owner.register('model:dealership', Dealership);
owner.register('service:store', Store);

store = owner.lookup('service:store');

});

test(`Identity of polymorphic relations can change type`, async function(assert) {
const { owner } = this;
const requests: RID[] = [];
const expectedRequests = [
{ id: '1', type: 'ferrari' },
{ id: '1', type: 'car' },
{ id: '2', type: 'bmw' },
{ id: '2', type: 'car' },
];
class TestAdapter extends Adapter {
shouldBackgroundReloadRecord() {
return false;
}
findRecord(_, { modelName: type }, id) {
if (type === 'dealership') {
return resolve({
data: {
id: '1',
type: 'dealership',
attributes: {
name: 'Brand new* car',
},
relationships: {
bestCar: {
data: { id: '1', type: 'ferrari' },
},
allCars: {
data: [
{ id: '1', type: 'ferrari' },
{ id: '2', type: 'bmw' },
],
},
},
},
});
}
requests.push({ type, id });
// return the polymorphic type instead of 'car';
type = id === '1' ? 'ferrari' : 'bmw';
return resolve({
data: {
id,
type,
attributes: {
color: 'red',
},
},
});
}
}
owner.register('adapter:application', TestAdapter);
const topRecord = await store.findRecord('dealership', '1');
const relation = await topRecord.get('car');
const relation = await topRecord.bestCar;

assert.strictEqual(relation.id, '1', 'We found the right id');
assert.strictEqual(relation.constructor.modelName, 'ferrari', 'We found the right type');

const foundFerrari = await store.findRecord('car', '1');
assert.ok(relation === foundFerrari, 'We found the ferrari by finding car 1');

assert.strictEqual(relation.id, '1');
const allCars = await topRecord.allCars;
assert.deepEqual(
allCars.map(c => {
return { id: c.id, type: c.constructor.modelName };
}),
[
{ id: '1', type: 'ferrari' },
{ id: '2', type: 'bmw' },
],
'We fetched all the right cars'
);
const bmw = allCars.objectAt(1);
const foundBmw = await store.findRecord('car', '2');
assert.ok(foundBmw === bmw, 'We found the bmw by finding car 2');

const foundRecord = await store.findRecord('car', '1');
assert.strictEqual(foundRecord.id, '1');
assert.deepEqual(requests, expectedRequests, 'We triggered the expected requests');
});
});
});
Expand Down
19 changes: 16 additions & 3 deletions packages/store/addon/-private/identifiers/cache.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ export class IdentifierCache {
let newId = coerceId(data.id);

const keyOptions = getTypeIndex(this._cache.types, identifier.type);
let existingIdentifier = detectMerge(keyOptions, identifier, newId);
let existingIdentifier = detectMerge(keyOptions, identifier, data, newId, this._cache.lids);

if (existingIdentifier) {
identifier = this._mergeRecordIdentifiers(keyOptions, identifier, existingIdentifier, data, newId as string);
Expand Down Expand Up @@ -364,6 +364,9 @@ export class IdentifierCache {

// ensure a secondary cache entry for this id for the identifier we do keep
keyOptions.id[newId] = kept;
// ensure a secondary cache entry for this id for the abandoned identifier's type we do keep
let baseKeyOptions = getTypeIndex(this._cache.types, existingIdentifier.type);
baseKeyOptions.id[newId] = kept;

// make sure that the `lid` on the data we are processing matches the lid we kept
data.lid = kept.lid;
Expand Down Expand Up @@ -518,13 +521,23 @@ function performRecordIdentifierUpdate(
function detectMerge(
keyOptions: KeyOptions,
identifier: StableRecordIdentifier,
newId: string | null
data: ResourceIdentifierObject | ExistingResourceObject,
newId: string | null,
lids: IdentifierMap
): StableRecordIdentifier | false {
const { id } = identifier;
const { id, type, lid } = identifier;
if (id !== null && id !== newId && newId !== null) {
const existingIdentifier = keyOptions.id[newId];

return existingIdentifier !== undefined ? existingIdentifier : false;
} else {
let newType = normalizeModelName(data.type);

if (id !== null && id === newId && newType === type && data.lid && data.lid !== lid) {
const existingIdentifier = lids[data.lid];

return existingIdentifier !== undefined ? existingIdentifier : false;
}
}

return false;
Expand Down

0 comments on commit 972c811

Please sign in to comment.