Skip to content

Commit

Permalink
Merge pull request #4810 from emberjs/fix-stuff
Browse files Browse the repository at this point in the history
[Fixes #4807] realize class + factory seperation
  • Loading branch information
bmac authored Feb 22, 2017
2 parents e2f35a7 + 314ae0b commit 08b0462
Show file tree
Hide file tree
Showing 6 changed files with 1,238 additions and 1,206 deletions.
2 changes: 1 addition & 1 deletion addon/-private/system/model/internal-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ export default class InternalModel {
createOptions.container = this.store.container;
}

this._record = this.modelClass._create(createOptions);
this._record = this.store.modelFactoryFor(this.modelName).create(createOptions);

this._triggerDeferredTriggers();
heimdall.stop(token);
Expand Down
29 changes: 14 additions & 15 deletions addon/-private/system/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
relatedTypesDescriptor,
relationshipsDescriptor
} from 'ember-data/-private/system/relationships/ext';
import { runInDebug } from 'ember-data/-private/debug';

const {
get,
Expand All @@ -21,7 +22,6 @@ const {
@module ember-data
*/


function findPossibleInverses(type, inverseType, name, relationshipsSoFar) {
let possibleRelationships = relationshipsSoFar || [];

Expand Down Expand Up @@ -1173,17 +1173,20 @@ Object.defineProperty(Model.prototype, 'data', {
}
});

Model.reopenClass({
/**
Alias DS.Model's `create` method to `_create`. This allows us to create DS.Model
instances from within the store, but if end users accidentally call `create()`
(instead of `createRecord()`), we can raise an error.
runInDebug(function() {
Model.reopen({
init() {
this._super(...arguments);

@method _create
@private
@static
*/
_create: Model.create,
if (!this._internalModel) {
throw new Ember.Error('You should not call `create` on a model. Instead, call `store.createRecord` with the attributes you would like to set.');
}
}
});
});

Model.reopenClass({
isModel: true,

/**
Override the class' `create()` method to raise an error. This
Expand All @@ -1196,10 +1199,6 @@ Model.reopenClass({
@private
@static
*/
create() {
throw new Ember.Error("You should not call `create` on a model. Instead, call `store.createRecord` with the attributes you would like to set.");
},

/**
Represents the model's class name as a string. This can be used to look up the model through
DS.Store's modelFor method.
Expand Down
23 changes: 15 additions & 8 deletions addon/-private/system/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -2087,6 +2087,12 @@ Store = Service.extend({
@private
*/
_modelFor(modelName) {
let maybeFactory = this._modelFactoryFor(modelName);
// for factorFor factory/class split
return maybeFactory.class ? maybeFactory.class : maybeFactory;
},

_modelFactoryFor(modelName) {
heimdall.increment(modelFor);
let factory = this._modelClassCache[modelName];

Expand All @@ -2101,9 +2107,13 @@ Store = Service.extend({
throw new EmberError(`No model was found for '${modelName}'`);
}

assert(`'${inspect(factory)}' does not appear to be an ember-data model`, (typeof factory._create === 'function') );
// interopt with the future
let klass = getOwner(this).factoryFor ? factory.class : factory;

assert(`'${inspect(klass)}' does not appear to be an ember-data model`, klass.isModel);

factory.modelName = factory.modelName || modelName;
// TODO: deprecate this
klass.modelName = klass.modelName || modelName;

this._modelClassCache[modelName] = factory;
}
Expand All @@ -2116,16 +2126,13 @@ Store = Service.extend({
*/
modelFactoryFor(modelName) {
heimdall.increment(modelFactoryFor);
assert("You need to pass a model name to the store's modelFactoryFor method", isPresent(modelName));
assert('Passing classes to store methods has been removed. Please pass a dasherized string instead of '+ inspect(modelName), typeof modelName === 'string');
assert(`You need to pass a model name to the store's modelFactoryFor method`, isPresent(modelName));
assert(`Passing classes to store methods has been removed. Please pass a dasherized string instead of ${modelName}`, typeof modelName === 'string');
let trueModelName = this._classKeyFor(modelName);
let owner = getOwner(this);

if (owner.factoryFor) {
let MaybeModel = owner.factoryFor(`model:${trueModelName}`);
let MaybeModelFactory = MaybeModel && MaybeModel.class;

return MaybeModelFactory;
return owner.factoryFor(`model:${trueModelName}`);
} else {
return owner._lookupFactory(`model:${trueModelName}`);
}
Expand Down
76 changes: 76 additions & 0 deletions tests/integration/injection-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import setupStore from 'dummy/tests/helpers/store';
import Ember from 'ember';
import DS from 'ember-data';
import { module, test } from 'qunit';

let env, originalFactoryFor, originalMODEL_FACTORY_INJECTIONS = Ember.MODEL_FACTORY_INJECTIONS;
const { run } = Ember;

const factory = {
isFactory: true,
class: {
isModel: true,
_create() { }
}
};

module('integration/injection factoryFor enabled', {
setup() {
env = setupStore();

originalFactoryFor = Ember.getOwner(env.store).factoryFor;

Ember.getOwner(env.store).factoryFor = function(name) {
return factory;
};
},

teardown() {
Ember.getOwner(env.store).factoryFor = originalFactoryFor;

run(env.store, 'destroy');
}
});

test('modelFactoryFor', function(assert) {
const modelFactory = env.store.modelFactoryFor('super-villain');

assert.equal(modelFactory, factory, 'expected the factory itself to be returned');
});

test('modelFor', function(assert) {
const modelFactory = env.store.modelFor('super-villain');

assert.equal(modelFactory, factory.class, 'expected the factory itself to be returned');

// TODO: we should deprecate this next line. Resolved state on the class is fraught with peril
assert.equal(modelFactory.modelName, 'super-villain', 'expected the factory itself to be returned');
});

module('integration/injection eager injections', {
setup() {
Ember.MODEL_FACTORY_INJECTIONS = true;
env = setupStore();

env.registry.injection('model:foo', 'apple', 'service:apple');
env.registry.register('model:foo', DS.Model);
env.registry.register('service:apple', Ember.Object.extend({ isService: true }));
// container injection
},

teardown() {
// can be removed once we no longer support ember versions without lookupFactory
Ember.MODEL_FACTORY_INJECTIONS = originalMODEL_FACTORY_INJECTIONS;

run(env.store, 'destroy');
}
});

test('did inject', function(assert) {
let foo = run(() => env.store.createRecord('foo'));
let apple = foo.get('apple');
let Apple = env.registry.registrations['service:apple'];

assert.ok(apple, `'model:foo' instance should have an 'apple' property`);
assert.ok(apple instanceof Apple, `'model:foo'.apple should be an instance of 'service:apple'`);
});
2 changes: 1 addition & 1 deletion tests/unit/model-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var run = Ember.run;

var Person, store, env;

module("unit/model - DS.Model", {
module('unit/model - DS.Model', {
beforeEach() {
Person = DS.Model.extend({
name: DS.attr('string'),
Expand Down
Loading

0 comments on commit 08b0462

Please sign in to comment.