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

Change _normalizeTypeKey to defer normalization to the container #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bantic
Copy link
Owner

@bantic bantic commented Jan 26, 2015

There are some failing tests and some additional work to do.
The issue is that in unit tests for ember-cli for models with dashes and
when using an adapter, the model's typeKey is normalized to camelCase
but store.modelFor(camelCasedName) will fail.

For example, with a model class 'ssh-key', create a record using the
string 'ssh-key'. This will work fine because the store will call modelFor('ssh-key'). This returns the correct model factory and it has the side effect of setting typeKey on that factory to sshKey.

Later, when saving that record, the store will call Adapter.createRecord which starts out by calling
store.serializerFor(type.typeKey). The store's serializerFor method passes this same typeKey (sshKey) to store.modelFor. But store.modelFor('sshKey') will fail because it will call container.lookupFactory('model:sshKey') and find nothing.

The call to lookupFactory('model:sshKey') will fail in the test because the test's container's normalizeFullName method is this:

function (fullName){
  return fullName;
}

When running an ember-cli normally (or when running an acceptance test), the container uses the ember-cli resolver and its normalizeFullName method becomes this:

function (fullName) {
        if (resolver.normalize) {
          return resolver.normalize(fullName);
        } else {
          Ember.deprecate('The Resolver should now provide a \'normalize\' function', false);
          return fullName;
        }
      }

In this case, resolver.normalize will return the same value ("model:ssh-key") when it is passed either "model:sshKey" or "model:ssh-key", so the app's store.modelFor('sshKey') and store.modelFor('ssh-key') return the same thing.

There's a sample ember-cli app showing this behavior. The important parts are in this commit. The failing test code looks like this:

  import {
    moduleForModel,
    test
  } from 'ember-qunit';
  import Ember from 'ember';

  moduleForModel('ssh-key', 'SshKey', {
    // Specify the other units that are required for this test.
    needs: ['adapter:application']
  });

  test('it can be looked up', function() {
    var store = this.store();

    var sshKey;

    Ember.run(function(){
      sshKey = store.createRecord('ssh-key');
    });

    return Ember.run(function(){
      return sshKey.save();
    });
  });

Screenshot of the failing test in the ember-cli app:
testtests tests 2015-01-26 18-44-41

cc @mixonic

There are some failing tests and some additional work to do.
The issue is that in unit tests for ember-cli for models with dashes and
when using an adapter, the model's typeKey is normalized to camelCase
but `store.modelFor(camelCasedName)` will fail.

For example, with a model class 'ssh-key', create a record using the
string 'ssh-key'. This will work fine, but later when saving that
record, the store will look up the adapter and then read off the model
class's typeKey (which is 'sshKey'), and pass it to store.modelFor which
will fail because it can't find a model called 'sshKey'.
@stefanpenner
Copy link

ping?

bantic pushed a commit that referenced this pull request Mar 29, 2016
[BUGFIX beta] Don't use old method names with the new injection system.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants