From 405f9c503c704ecb5dae569f54529b46e5e53282 Mon Sep 17 00:00:00 2001 From: Stanley Stuart Date: Sat, 16 May 2015 19:07:52 -0500 Subject: [PATCH] remove instances of DS.Store from factories 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. --- .../lib/serializers/embedded-records-mixin.js | 2 +- .../lib/serializers/json-serializer.js | 2 +- .../lib/system/relationship-meta.js | 20 ++++-------- .../lib/system/relationships/ext.js | 31 ++++++++++--------- .../system/relationships/state/belongs-to.js | 12 +++---- .../lib/system/relationships/state/create.js | 2 +- .../system/relationships/state/has-many.js | 14 ++++----- packages/ember-data/lib/system/store.js | 1 - .../ember-data/lib/system/store/finders.js | 10 +++--- .../tests/integration/inverse-test.js | 18 ++++++----- .../relationships/belongs-to-test.js | 13 ++++++-- .../relationships/has-many-test.js | 8 ++--- .../tests/unit/model/relationships-test.js | 4 +-- .../unit/model/relationships/has-many-test.js | 8 ++--- .../tests/unit/store/create-record-test.js | 4 +-- 15 files changed, 77 insertions(+), 72 deletions(-) diff --git a/packages/ember-data/lib/serializers/embedded-records-mixin.js b/packages/ember-data/lib/serializers/embedded-records-mixin.js index adc0ddcbc99..0e4a105755b 100644 --- a/packages/ember-data/lib/serializers/embedded-records-mixin.js +++ b/packages/ember-data/lib/serializers/embedded-records-mixin.js @@ -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); if (relationship.kind === "hasMany") { if (relationship.options.polymorphic) { extractEmbeddedHasManyPolymorphic(store, key, partial); diff --git a/packages/ember-data/lib/serializers/json-serializer.js b/packages/ember-data/lib/serializers/json-serializer.js index e81133c090b..533b6822c6d 100644 --- a/packages/ember-data/lib/serializers/json-serializer.js +++ b/packages/ember-data/lib/serializers/json-serializer.js @@ -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 }); diff --git a/packages/ember-data/lib/system/relationship-meta.js b/packages/ember-data/lib/system/relationship-meta.js index 42244245a85..3ec11c098f8 100644 --- a/packages/ember-data/lib/system/relationship-meta.js +++ b/packages/ember-data/lib/system/relationship-meta.js @@ -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)); } -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 diff --git a/packages/ember-data/lib/system/relationships/ext.js b/packages/ember-data/lib/system/relationships/ext.js index a610e220203..7c89de481de 100644 --- a/packages/ember-data/lib/system/relationships/ext.js +++ b/packages/ember-data/lib/system/relationships/ext.js @@ -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, @@ -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); @@ -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); } }); @@ -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() { @@ -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; } @@ -277,9 +278,9 @@ Model.reopenClass({ var possibleRelationships = relationshipsSoFar || []; var relationshipMap = get(inverseType, 'relationships'); - if (!relationshipMap) { return; } + if (!relationshipMap) { return possibleRelationships; } - var relationships = relationshipMap.get(type); + var relationships = relationshipMap.get(type.modelName); relationships = filter.call(relationships, function(relationship) { var optionsForRelationship = inverseType.metaForProperty(relationship.name).options; @@ -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) { @@ -617,7 +618,7 @@ Model.reopen({ }, inverseFor: function(key) { - return this.constructor.inverseFor(key); + return this.constructor.inverseFor(key, this.store); } }); diff --git a/packages/ember-data/lib/system/relationships/state/belongs-to.js b/packages/ember-data/lib/system/relationships/state/belongs-to.js index a515aa6aa57..b658ca20cdf 100644 --- a/packages/ember-data/lib/system/relationships/state/belongs-to.js +++ b/packages/ember-data/lib/system/relationships/state/belongs-to.js @@ -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) { diff --git a/packages/ember-data/lib/system/relationships/state/create.js b/packages/ember-data/lib/system/relationships/state/create.js index 9d4bd93ae4c..778d5d44087 100644 --- a/packages/ember-data/lib/system/relationships/state/create.js +++ b/packages/ember-data/lib/system/relationships/state/create.js @@ -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; diff --git a/packages/ember-data/lib/system/relationships/state/has-many.js b/packages/ember-data/lib/system/relationships/state/has-many.js index 0c774db41ec..775f47a08a7 100644 --- a/packages/ember-data/lib/system/relationships/state/has-many.js +++ b/packages/ember-data/lib/system/relationships/state/has-many.js @@ -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), record: record }); this.isPolymorphic = relationshipMeta.options.polymorphic; @@ -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); diff --git a/packages/ember-data/lib/system/store.js b/packages/ember-data/lib/system/store.js index bc84050d148..4777fc8d4f6 100644 --- a/packages/ember-data/lib/system/store.js +++ b/packages/ember-data/lib/system/store.js @@ -1498,7 +1498,6 @@ Store = Service.extend({ }); } - factory.store = this; return factory; }, diff --git a/packages/ember-data/lib/system/store/finders.js b/packages/ember-data/lib/system/store/finders.js index c633f5f3252..0296ef02fc1 100644 --- a/packages/ember-data/lib/system/store/finders.js +++ b/packages/ember-data/lib/system/store/finders.js @@ -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; @@ -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'); @@ -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); @@ -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; diff --git a/packages/ember-data/tests/integration/inverse-test.js b/packages/ember-data/tests/integration/inverse-test.js index e55f15ec4b9..d0861e54ae4 100644 --- a/packages/ember-data/tests/integration/inverse-test.js +++ b/packages/ember-data/tests/integration/inverse-test.js @@ -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() { @@ -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' @@ -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' @@ -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 () { @@ -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"); }); @@ -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 () { diff --git a/packages/ember-data/tests/integration/relationships/belongs-to-test.js b/packages/ember-data/tests/integration/relationships/belongs-to-test.js index 9679be315a2..db4ba5d33e7 100644 --- a/packages/ember-data/tests/integration/relationships/belongs-to-test.js +++ b/packages/ember-data/tests/integration/relationships/belongs-to-test.js @@ -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 }); @@ -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() { @@ -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"); @@ -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"); diff --git a/packages/ember-data/tests/integration/relationships/has-many-test.js b/packages/ember-data/tests/integration/relationships/has-many-test.js index ad4981cfa39..42584542cd7 100644 --- a/packages/ember-data/tests/integration/relationships/has-many-test.js +++ b/packages/ember-data/tests/integration/relationships/has-many-test.js @@ -150,7 +150,7 @@ test("A serializer can materialize a hasMany as an opaque token that can be lazi env.adapter.findHasMany = function(store, snapshot, link, relationship) { equal(link, "/posts/1/comments", "findHasMany link was /posts/1/comments"); - equal(relationship.type.modelName, "comment", "relationship was passed correctly"); + equal(relationship.type, "comment", "relationship was passed correctly"); return Ember.RSVP.resolve([ { id: 1, body: "First" }, @@ -340,7 +340,7 @@ test("A hasMany relationship can be reloaded if it was fetched via a link", func }; env.adapter.findHasMany = function(store, snapshot, link, relationship) { - equal(relationship.type, Comment, "findHasMany relationship type was Comment"); + equal(relationship.type, 'comment', "findHasMany relationship type was Comment"); equal(relationship.key, 'comments', "findHasMany relationship key was comments"); equal(link, "/posts/1/comments", "findHasMany link was /posts/1/comments"); @@ -358,7 +358,7 @@ test("A hasMany relationship can be reloaded if it was fetched via a link", func equal(comments.get('length'), 2, "comments have 2 length"); env.adapter.findHasMany = function(store, snapshot, link, relationship) { - equal(relationship.type, Comment, "findHasMany relationship type was Comment"); + equal(relationship.type, 'comment', "findHasMany relationship type was Comment"); equal(relationship.key, 'comments', "findHasMany relationship key was comments"); equal(link, "/posts/1/comments", "findHasMany link was /posts/1/comments"); @@ -570,7 +570,7 @@ test("An updated `links` value should invalidate a relationship cache", function }); env.adapter.findHasMany = function(store, snapshot, link, relationship) { - equal(relationship.type.modelName, "comment", "relationship was passed correctly"); + equal(relationship.type, "comment", "relationship was passed correctly"); if (link === '/first') { return Ember.RSVP.resolve([ diff --git a/packages/ember-data/tests/unit/model/relationships-test.js b/packages/ember-data/tests/unit/model/relationships-test.js index f110aefd703..30dd0ec96e3 100644 --- a/packages/ember-data/tests/unit/model/relationships-test.js +++ b/packages/ember-data/tests/unit/model/relationships-test.js @@ -27,11 +27,11 @@ test("exposes a hash of the relationships on a model", function() { }); var relationships = get(Person, 'relationships'); - deepEqual(relationships.get(Person), [ + deepEqual(relationships.get('person'), [ { name: "people", kind: "hasMany" }, { name: "parent", kind: "belongsTo" } ]); - deepEqual(relationships.get(Occupation), [ + deepEqual(relationships.get('occupation'), [ { name: "occupations", kind: "hasMany" } ]); }); diff --git a/packages/ember-data/tests/unit/model/relationships/has-many-test.js b/packages/ember-data/tests/unit/model/relationships/has-many-test.js index a637bcab2f9..beb12f47d68 100644 --- a/packages/ember-data/tests/unit/model/relationships/has-many-test.js +++ b/packages/ember-data/tests/unit/model/relationships/has-many-test.js @@ -177,7 +177,7 @@ test("should be able to retrieve the type for a hasMany relationship without spe person: Person }); - equal(env.store.modelFor('person').typeForRelationship('tags'), Tag, "returns the relationship type"); + equal(env.store.modelFor('person').typeForRelationship('tags', env.store), Tag, "returns the relationship type"); }); test("should be able to retrieve the type for a hasMany relationship specified using a string from its metadata", function() { @@ -192,7 +192,7 @@ test("should be able to retrieve the type for a hasMany relationship specified u person: Person }); - equal(env.store.modelFor('person').typeForRelationship('tags'), Tag, "returns the relationship type"); + equal(env.store.modelFor('person').typeForRelationship('tags', env.store), Tag, "returns the relationship type"); }); test("should be able to retrieve the type for a belongsTo relationship without specifying a type from its metadata", function() { @@ -207,7 +207,7 @@ test("should be able to retrieve the type for a belongsTo relationship without s person: Person }); - equal(env.store.modelFor('person').typeForRelationship('tag'), Tag, "returns the relationship type"); + equal(env.store.modelFor('person').typeForRelationship('tag', env.store), Tag, "returns the relationship type"); }); test("should be able to retrieve the type for a belongsTo relationship specified using a string from its metadata", function() { @@ -224,7 +224,7 @@ test("should be able to retrieve the type for a belongsTo relationship specified person: Person }); - equal(env.store.modelFor('person').typeForRelationship('tags'), Tag, "returns the relationship type"); + equal(env.store.modelFor('person').typeForRelationship('tags', env.store), Tag, "returns the relationship type"); }); test("relationships work when declared with a string path", function() { diff --git a/packages/ember-data/tests/unit/store/create-record-test.js b/packages/ember-data/tests/unit/store/create-record-test.js index 1e555d6ac1a..9620ef2a1db 100644 --- a/packages/ember-data/tests/unit/store/create-record-test.js +++ b/packages/ember-data/tests/unit/store/create-record-test.js @@ -33,8 +33,8 @@ test("doesn't modify passed in properties hash", function() { test("allow passing relationships as well as attributes", function() { var records, storage; run(function() { - records = store.pushMany(Record, [{ id: 1, title: "it's a beautiful day" }, { id: 2, title: "it's a beautiful day" }]); - storage = store.createRecord(Storage, { name: 'Great store', records: records }); + records = store.pushMany('record', [{ id: 1, title: "it's a beautiful day" }, { id: 2, title: "it's a beautiful day" }]); + storage = store.createRecord('storage', { name: 'Great store', records: records }); }); equal(storage.get('name'), 'Great store', "The attribute is well defined");