Skip to content

Commit

Permalink
remove instances of DS.Store from factories
Browse files Browse the repository at this point in the history
When using multiple stores with Ember Data, we want to avoid storing the
state on the factory itself. Because multiple stores currently share the
same factory, this means that the first store to look up the factory
will store itself as the `store` on that factory. That means that any
stores thereafter will receive factories whose `store` property
references the first store, not their own.
  • Loading branch information
Stanley Stuart committed May 22, 2015
1 parent b213ce1 commit 9c9420f
Show file tree
Hide file tree
Showing 15 changed files with 77 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ function extractEmbeddedRecords(serializer, store, typeClass, partial) {

typeClass.eachRelationship(function(key, relationship) {
if (serializer.hasDeserializeRecordsOption(key)) {
var embeddedTypeClass = store.modelFor(relationship.type.modelName);
var embeddedTypeClass = store.modelFor(relationship.type);

This comment has been minimized.

Copy link
@igorT

igorT May 24, 2015

Member

This seems like a breaking change, can we deprecate?

if (relationship.kind === "hasMany") {
if (relationship.options.polymorphic) {
extractEmbeddedHasManyPolymorphic(store, key, partial);
Expand Down
2 changes: 1 addition & 1 deletion packages/ember-data/lib/serializers/json-serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ export default Serializer.extend({
payloadKey = this.keyForRelationship(key, "hasMany", "serialize");
}

var relationshipType = snapshot.type.determineRelationshipType(relationship);
var relationshipType = snapshot.type.determineRelationshipType(relationship, this.store);

if (relationshipType === 'manyToNone' || relationshipType === 'manyToMany') {
json[payloadKey] = snapshot.hasMany(key, { ids: true });
Expand Down
20 changes: 6 additions & 14 deletions packages/ember-data/lib/system/relationship-meta.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,18 @@
import {singularize} from 'ember-inflector/lib/system/string';
import normalizeModelName from 'ember-data/system/normalize-model-name';

export function typeForRelationshipMeta(store, meta) {
var modelName, typeClass;
export function typeForRelationshipMeta(meta) {
var modelName;

modelName = meta.type || meta.key;
if (typeof modelName === 'string') {
if (meta.kind === 'hasMany') {
modelName = singularize(modelName);
}
typeClass = store.modelFor(modelName);
} else {
typeClass = meta.type;
}

return typeClass;
return singularize(normalizeModelName(modelName));

This comment has been minimized.

Copy link
@igorT

igorT May 24, 2015

Member

We only want to singularize for hasMany.

}

export function relationshipFromMeta(store, meta) {
export function relationshipFromMeta(meta) {
return {
key: meta.key,
kind: meta.kind,
type: typeForRelationshipMeta(store, meta),
type: typeForRelationshipMeta(meta),
options: meta.options,
parentType: meta.parentType,
isRelationship: true
Expand Down
31 changes: 16 additions & 15 deletions packages/ember-data/lib/system/relationships/ext.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var relationshipsDescriptor = Ember.computed(function() {
// it to the map.
if (meta.isRelationship) {
meta.key = name;
var relationshipsForType = map.get(typeForRelationshipMeta(this.store, meta));
var relationshipsForType = map.get(typeForRelationshipMeta(meta));

relationshipsForType.push({
name: name,
Expand All @@ -52,7 +52,7 @@ var relatedTypesDescriptor = Ember.computed(function() {
this.eachComputedProperty(function(name, meta) {
if (meta.isRelationship) {
meta.key = name;
modelName = typeForRelationshipMeta(this.store, meta);
modelName = typeForRelationshipMeta(meta);

Ember.assert("You specified a hasMany (" + meta.type + ") on " + meta.parentType + " but " + meta.type + " was not found.", modelName);

Expand All @@ -76,8 +76,8 @@ var relationshipsByNameDescriptor = Ember.computed(function() {
this.eachComputedProperty(function(name, meta) {
if (meta.isRelationship) {
meta.key = name;
var relationship = relationshipFromMeta(this.store, meta);
relationship.type = typeForRelationshipMeta(this.store, meta);
var relationship = relationshipFromMeta(meta);
relationship.type = typeForRelationshipMeta(meta);
map.set(name, relationship);
}
});
Expand Down Expand Up @@ -175,11 +175,12 @@ Model.reopenClass({
@method typeForRelationship
@static
@param {String} name the name of the relationship
@param {store} store an instance of DS.Store
@return {subclass of DS.Model} the type of the relationship, or undefined
*/
typeForRelationship: function(name) {
typeForRelationship: function(name, store) {
var relationship = get(this, 'relationshipsByName').get(name);
return relationship && relationship.type;
return relationship && store.modelFor(relationship.type);
},

inverseMap: Ember.computed(function() {
Expand Down Expand Up @@ -209,21 +210,21 @@ Model.reopenClass({
@param {String} name the name of the relationship
@return {Object} the inverse relationship, or null
*/
inverseFor: function(name) {
inverseFor: function(name, store) {
var inverseMap = get(this, 'inverseMap');
if (inverseMap[name]) {
return inverseMap[name];
} else {
var inverse = this._findInverseFor(name);
var inverse = this._findInverseFor(name, store);
inverseMap[name] = inverse;
return inverse;
}
},

//Calculate the inverse, ignoring the cache
_findInverseFor: function(name) {
_findInverseFor: function(name, store) {

var inverseType = this.typeForRelationship(name);
var inverseType = this.typeForRelationship(name, store);
if (!inverseType) {
return null;
}
Expand Down Expand Up @@ -277,9 +278,9 @@ Model.reopenClass({
var possibleRelationships = relationshipsSoFar || [];

var relationshipMap = get(inverseType, 'relationships');
if (!relationshipMap) { return; }
if (!relationshipMap) { return possibleRelationships; }

This comment has been minimized.

Copy link
@igorT

igorT May 24, 2015

Member

What is up with this? Seems unrelated to the PR?


var relationships = relationshipMap.get(type);
var relationships = relationshipMap.get(type.modelName);

relationships = filter.call(relationships, function(relationship) {
var optionsForRelationship = inverseType.metaForProperty(relationship.name).options;
Expand Down Expand Up @@ -535,10 +536,10 @@ Model.reopenClass({
});
},

determineRelationshipType: function(knownSide) {
determineRelationshipType: function(knownSide, store) {
var knownKey = knownSide.key;
var knownKind = knownSide.kind;
var inverse = this.inverseFor(knownKey);
var inverse = this.inverseFor(knownKey, store);
var key, otherKind;

if (!inverse) {
Expand Down Expand Up @@ -617,7 +618,7 @@ Model.reopen({
},

inverseFor: function(key) {
return this.constructor.inverseFor(key);
return this.constructor.inverseFor(key, this.store);
}

});
12 changes: 6 additions & 6 deletions packages/ember-data/lib/system/relationships/state/belongs-to.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ BelongsToRelationship.prototype.flushCanonical = function() {
BelongsToRelationship.prototype._super$addRecord = Relationship.prototype.addRecord;
BelongsToRelationship.prototype.addRecord = function(newRecord) {
if (this.members.has(newRecord)) { return;}
var type = this.relationshipMeta.type;
Ember.assert("You cannot add a '" + newRecord.constructor.modelName + "' record to the '" + this.record.constructor.modelName + "." + this.key +"'. " + "You can only add a '" + type.modelName + "' record to this relationship.", (function () {
if (type.__isMixin) {
return type.__mixin.detect(newRecord);
var typeClass = this.store.modelFor(this.relationshipMeta.type);
Ember.assert("You cannot add a '" + newRecord.constructor.modelName + "' record to the '" + this.record.constructor.modelName + "." + this.key +"'. " + "You can only add a '" + typeClass.modelName + "' record to this relationship.", (function () {
if (typeClass.__isMixin) {
return typeClass.__mixin.detect(newRecord);
}
if (Ember.MODEL_FACTORY_INJECTIONS) {
type = type.superclass;
typeClass = typeClass.superclass;
}
return newRecord instanceof type;
return newRecord instanceof typeClass;
})());

if (this.inverseRecord) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import BelongsToRelationship from "ember-data/system/relationships/state/belongs

var createRelationshipFor = function(record, relationshipMeta, store) {
var inverseKey;
var inverse = record.constructor.inverseFor(relationshipMeta.key);
var inverse = record.constructor.inverseFor(relationshipMeta.key, store);

if (inverse) {
inverseKey = inverse.name;
Expand Down
14 changes: 7 additions & 7 deletions packages/ember-data/lib/system/relationships/state/has-many.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ var ManyRelationship = function(store, record, inverseKey, relationshipMeta) {
canonicalState: this.canonicalState,
store: this.store,
relationship: this,
type: this.belongsToType,
type: this.store.modelFor(this.belongsToType),

This comment has been minimized.

Copy link
@igorT

igorT May 24, 2015

Member

shouldn't this be typeClass now?

record: record
});
this.isPolymorphic = relationshipMeta.options.polymorphic;
Expand Down Expand Up @@ -84,15 +84,15 @@ ManyRelationship.prototype.removeRecordFromOwn = function(record, idx) {
};

ManyRelationship.prototype.notifyRecordRelationshipAdded = function(record, idx) {
var type = this.relationshipMeta.type;
Ember.assert("You cannot add '" + record.constructor.modelName + "' records to the " + this.record.constructor.modelName + "." + this.key + " relationship (only '" + this.belongsToType.modelName + "' allowed)", (function () {
if (type.__isMixin) {
return type.__mixin.detect(record);
var typeClass = this.store.modelFor(this.relationshipMeta.type);
Ember.assert("You cannot add '" + record.constructor.modelName + "' records to the " + this.record.constructor.modelName + "." + this.key + " relationship (only '" + typeClass.modelName + "' allowed)", (function () {
if (typeClass.__isMixin) {
return typeClass.__mixin.detect(record);
}
if (Ember.MODEL_FACTORY_INJECTIONS) {
type = type.superclass;
typeClass = typeClass.superclass;
}
return record instanceof type;
return record instanceof typeClass;
})());

this.record.notifyHasManyAdded(this.key, record, idx);
Expand Down
1 change: 0 additions & 1 deletion packages/ember-data/lib/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,6 @@ Store = Service.extend({
});
}

factory.store = this;
return factory;
},

Expand Down
10 changes: 6 additions & 4 deletions packages/ember-data/lib/system/store/finders.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ export function _findMany(adapter, store, typeClass, ids, records) {

export function _findHasMany(adapter, store, record, link, relationship) {
var snapshot = record._createSnapshot();
var modelName = relationship.type.modelName;
var modelName = relationship.type;
var typeClass = store.modelFor(modelName);
var promise = adapter.findHasMany(store, snapshot, link, relationship);
var serializer = serializerForAdapter(store, adapter, modelName);
var label = "DS: Handle Adapter#findHasMany of " + record + " : " + relationship.type;
Expand All @@ -78,7 +79,7 @@ export function _findHasMany(adapter, store, record, link, relationship) {

return promise.then(function(adapterPayload) {
return store._adapterRun(function() {
var payload = serializer.extract(store, relationship.type, adapterPayload, null, 'findHasMany');
var payload = serializer.extract(store, typeClass, adapterPayload, null, 'findHasMany');

Ember.assert("The response from a findHasMany must be an Array, not " + Ember.inspect(payload), Ember.typeOf(payload) === 'array');

Expand All @@ -89,7 +90,8 @@ export function _findHasMany(adapter, store, record, link, relationship) {
}

export function _findBelongsTo(adapter, store, record, link, relationship) {
var modelName = relationship.type.modelName;
var modelName = relationship.type;
var typeClass = store.modelFor(modelName);
var snapshot = record._createSnapshot();
var promise = adapter.findBelongsTo(store, snapshot, link, relationship);
var serializer = serializerForAdapter(store, adapter, modelName);
Expand All @@ -101,7 +103,7 @@ export function _findBelongsTo(adapter, store, record, link, relationship) {

return promise.then(function(adapterPayload) {
return store._adapterRun(function() {
var payload = serializer.extract(store, relationship.type, adapterPayload, null, 'findBelongsTo');
var payload = serializer.extract(store, typeClass, adapterPayload, null, 'findBelongsTo');

if (!payload) {
return null;
Expand Down
18 changes: 11 additions & 7 deletions packages/ember-data/tests/integration/inverse-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ module('integration/inverse_test - inverseFor', {
});

store = env.store;

Job = store.modelFor('job');
User = store.modelFor('user');
ReflexiveModel = store.modelFor('reflexive-model');
},

teardown: function() {
Expand All @@ -49,7 +53,7 @@ test("Finds the inverse when there is only one possible available", function ()
//Maybe store is evaluated lazily, so we need this :(
run(store, 'push', 'user', { id: 1 });

deepEqual(Job.inverseFor('user'), {
deepEqual(Job.inverseFor('user', store), {
type: User,
name: 'job',
kind: 'belongsTo'
Expand All @@ -72,13 +76,13 @@ test("Finds the inverse when only one side has defined it manually", function ()
job = store.push('user', { id: 1 });
});

deepEqual(Job.inverseFor('owner'), {
deepEqual(Job.inverseFor('owner', store), {
type: User, //the model's type
name: 'previousJob', //the models relationship key
kind: 'belongsTo'
}, 'Gets correct type, name and kind');

deepEqual(User.inverseFor('previousJob'), {
deepEqual(User.inverseFor('previousJob', store), {
type: Job, //the model's type
name: 'owner', //the models relationship key
kind: 'belongsTo'
Expand All @@ -99,7 +103,7 @@ test("Returns null if inverse relationship it is manually set with a different r
user = store.push('user', { id: 1 });
});

equal(User.inverseFor('job'), null, 'There is no inverse');
equal(User.inverseFor('job', store), null, 'There is no inverse');
});

test("Errors out if you define 2 inverses to the same model", function () {
Expand All @@ -117,7 +121,7 @@ test("Errors out if you define 2 inverses to the same model", function () {
run(function() {
store.push('user', { id: 1 });
});
User.inverseFor('job');
User.inverseFor('job', store);
}, "You defined the 'job' relationship on user, but you defined the inverse relationships of type job multiple times. Look at http://emberjs.com/guides/models/defining-models/#toc_explicit-inverses for how to explicitly specify inverses");
});

Expand All @@ -129,12 +133,12 @@ test("Caches findInverseFor return value", function () {
store.push('user', { id: 1 });
});

var inverseForUser = Job.inverseFor('user');
var inverseForUser = Job.inverseFor('user', store);
Job.findInverseFor = function() {
ok(false, 'Find is not called anymore');
};

equal(inverseForUser, Job.inverseFor('user'), 'Inverse cached succesfully');
equal(inverseForUser, Job.inverseFor('user', store), 'Inverse cached succesfully');
});

test("Errors out if you do not define an inverse for a reflexive relationship", function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ module("integration/relationship/belongs_to Belongs-To Relationships", {
author: Author
});


env.registry.optionsForType('serializer', { singleton: false });
env.registry.optionsForType('adapter', { singleton: false });

Expand All @@ -82,6 +81,14 @@ module("integration/relationship/belongs_to Belongs-To Relationships", {
}));

store = env.store;

User = store.modelFor('user');
Post = store.modelFor('post');
Comment = store.modelFor('comment');
Message = store.modelFor('message');
Book = store.modelFor('book');
Chapter = store.modelFor('chapter');
Author = store.modelFor('author');
},

teardown: function() {
Expand Down Expand Up @@ -227,7 +234,7 @@ test("A serializer can materialize a belongsTo as a link that gets sent back to
};

env.adapter.findBelongsTo = async(function(store, snapshot, link, relationship) {
equal(relationship.type, Group);
equal(relationship.type, 'group');
equal(relationship.key, 'group');
equal(link, "/people/1/group");

Expand Down Expand Up @@ -380,7 +387,7 @@ test("relationshipsByName does not cache a factory", function() {
// A model is looked up in the store based on a string, via user input
var messageModelFromStore = store.modelFor('message');
// And the model is lookup up internally via the relationship type
var messageModelFromRelationType = store.modelFor(messageType.modelName);
var messageModelFromRelationType = store.modelFor(messageType);

equal(messageModelFromRelationType, messageModelFromStore,
"model factory based on relationship type matches the model based on store.modelFor");
Expand Down
Loading

0 comments on commit 9c9420f

Please sign in to comment.