DS.InMemoryAdapter and DS.NullSerializer #512

Closed
wants to merge 12 commits into
from

Conversation

Projects
None yet
8 participants
@ahawkins
Contributor

ahawkins commented Dec 11, 2012

InMemoryAdapter & NullSerializer

EDIT: This PR is still WIP. I've pushed the code for feedback from the core team and community members.

I've given a lot of thought to this PR and the issues's surrounding it. I don't
think they are only related to my app, but to general practice for development
and testing. I've written this code and made this PR with these things in mind.
I will describe them in detail. Warning: this is going to be long.

Background

I think that development and testing are essentially the same environment. All
ember-data backed applications are going to have data dependence in tests. In
order to write effective tests we need to ensure these things work:

  1. Reset application data back to an empty state
  2. Create, add, and delete records from inside the application
  3. Create, add, and delete records from outside the application
  4. Inspect what get's sent back to the "server". I'll come back to this later.

Developers also need to do these same things in development to some degree.
Most of the time we just need some dummy data to develop against. This was
previously done by using DS.FixtureAdapter. More on that later.

Ember applications also enforce boundarys between different parts of the
application. The store is one boundary. The store also creates it's own
internal boundaries by using the adapter and serializer. I think these
boundaries are very imporant because they enable developers to forget about
what is happening on the other side. More importantly, boundaries should be
stubable or easily swapped out if you like. You can, and should, leverage
architectual boundaries as much as you can. It will make development much
better.

With those principles in mind, I set out to make it so. This is the solution I
came to.

The Existing Problem

I needed the absolute most simple possible store. All I wanted to do was create
Ember.Objects according to my application semantics and persist them in
memory. I needed to persist them in memory somewhere so I could run queries
against them. I wanted to put this in my in memory adapter because the server
would usually do it, but since there is a boundary between the server and app,
I wanted to leverage it and exploit it. The closest thing to this is
DS.FixtureAdapter. This works for very simple use cases but it does not
scale. I'll do my best to enumerabe the problems with using
DS.FixtureAdapter.

  1. Records are pushed onto global constants in multiple objects. This
    does not lend itself to setup/teardown use cases.
  2. Dated pushed into .FIXTURES must be compliant with whatever happens
    with the transforms. This can be confusing. Things just happen to work
    becuase of the transform implementations (since the current default serializer
    is DS.JSONSerializer).
  3. Continuation of point 2, but from a different perspective. The data
    in .FIXTURES does not mirror application data. Here's an example
    if you work with Dates you will have Date objects inside your application
    but you will have formatted strings inside .FIXTURES. You have to
    work with two different kinds of data. I think this is fundamentally
    flawed.
  4. Changes to DS.Model objects are not commited to .FIXTURES arrays.
    This makes it impossible to get full use out of the fixture adapter
    because changes are not persistent. Here's an example: You have a simple
    list of user fixtures. There are two users. "Tom" and "Yehuda". You find
    "Tom" and change his name to "Tom Dale". Now you need to do fire a findQuery
    for all the user's named "Tom Dale". You cannot do this because "Tom"
    is still persisted in User.FIXTURES. You can override create/update/delete
    record to do this if you like. I think this functionality is confusing to users.
    Hell, you can delete a record and find it again. That's kinda werid.
  5. DS.FixtureAdapter does not respect boundaries. Ember applications are MVC.
    That creates a familar set of boundaries. Ember-Data introduces a new boundary
    between your model and the data. It also introduces additional boundaries between
    data consumers and providers. Your application code only communicates with the store.
    This is the boundary and it should be respected. There is a reason the adapter
    is not exposed directly to the application. Using DS.FixtureAdapter forces
    you to work with the adapter instead of the store. Pushing data into .FIXTURES
    to load data is an antipattern. store.createRecord should be used instead.
    This point is also somewhat related to #4 because the code does not communicate
    changes in the store back to it's storage so must interact with .FIXTURES.

I've run into these problems by pushing the uses cases to it's limits. I don't
think the fixture adapter is bad, I just think it serves a limited number of
use cases. It's use cases can be modeled using an in memory store, similarly
to how you can construct a syncronous system from an asyncronous one, but not
the other way around.

These problems have led me to this solution.

Towards a Better Solution

I mucked with the fixture adapter and various things. I came up with these
use cases.

  1. Isolate tests from the store. Some tests needed to access the store, but
    not the application. I don't need an application to test the store.
    (Boundaries again). I do need a store that I can instantiate whenever
    I need to that has no dependencies.
  2. The store itself must be resetable. Integration tests will create data.
    The test will finish and the next one needs to run. Data must be wiped
    before the next test. There is also another problem at play here. There
    may be multiple references to the store through the application code.
    The store itself is instantiated in an injection then set on your
    router and controllers. This makes reassigning the store difficult because
    you have to know eveywhere it should be assigned. You can currently
    "reset" the store by destroying it. This is suboptimal.
  3. Provide the simplest possible "persistance" strategy. Here's my defintion
    of simple: what's passed to create/update get's stored in memory. That's all.
    No fancy tranforms or serialization magic. There is no need for these for
    these when you have control over the data format!
  4. Create/Update/Delete operations must be persisted in memory. This enables
    the querying use case described earlier.
  5. All internal state is localized to instance variables.
  6. Respect boundaries as much as possible. Work with the store and not
    the adapter, but expose parts of the adapter useful for testing (IE the records
    currently in memory).

After looking at this problem (and struggling with implemenations) I came to
the conclusion that Ember-Data needs two things: A simple in memory adapter and
a null serializer. The in memory adapter is a slight refactoring of the
existing fixture adapter. The null serializer is an implemenation of the null
object pattern for serializers.

DS.InMemoryAdapter and DS.NullSerializer

I mentioned that DS.InMemoryAdapter is a refactoring of DS.FixtureAdapter.
In fact the only two differences:

  1. create/update/delete commits are kept in memory
  2. loaded records are kept in an internal Ember.Map instead in global
    .FIXTURE arrays.

DS.NullSerializer is more fun. It simply ignores everything. It ignores
transformations and mappings. Why do we need a null serializer? Using a null
serializer allows whatever is passed into create/update to exist in memory.
There is no need to work with two different sets of data. This enables you to
simply shove whatever you want into the store without having to worry about
anything. Using DS.NullSerializer in combination with DS.InMemoryAdapter
allows you to create store as essentially an array of objects. This is the
simplest possible thing that could work. Most importantly: You can work
directly with application level objects inside the store AND you don't have to
follow any semantics at all.
You simply create DS.Model objects you want
and call store.commit() and things are saved.

Here's an example:

var store = DS.create({
  adapter: DS.InMemoryAdapter.create(); // Uses DS.NullSerializer by default
});

// let's build some crazy stuff, but first here's my real world 
// use case

// Ember.DateTime is an example of a complex object that is
// no supported in the default tranforms. It's not part of
// Ember itself. It is a separate project.

store.createRecord(Email, {
  id: '1',
  sentAt: Ember.DateTime.create()
});
store.commit();

var email = store.find(Email, 1);

var email.get('sentAt') // Ember.DateTime. Exactly what I put on it.
                        // No fuss no muss
// Writing an integration test
// assuming the store from the previous example

module("Login", {
  // ensures each test exeuctes with no data
  // or existing state
  setup: function() {
    App.get('store').reset();
  },

  teardown: function() {
    App.get('store').reset();
  },
});

test("the user is logged in", function() {
  // do stuff
});

You can also use DS.InMemoryStore as a mock. It implements no behavior so you
can effectively substitute it for anything dependent on a store.

Why This is Good

I belive these changes are good for many reasons. There are two main reasons:

  1. DS.InMemoryStore is completely portable can be instantiated, reset, and torn
    down as needed--wether it's in tests or in development.
  2. Completely disregards problems and API's that exist for when you don't have
    constrol of the data your application needs. If you control the date you
    have no need for transforms and maps. You just construct the object you
    want and save it.

I also thing these things are more inline with how Ember applications should
be architected. It pains me to see DS.FixtureAdapter and custom serializers
with maps and transforms, or hell, even bending fixtures to match REST semantics.
All of that is complete nonsense. Leverage the boundaries to make your development
easier. Work with application level objects and not some other abstraction. Remember:
the adapter's role is to provide you with application objects! So why not just tell
it to store the ones you create?

Change Summary

  • Create DS.InMemoryAdapter as described above. It can still simulate remote responses.
    That behavior is off by default.
  • Create DS.NullSerializer.
  • breaking: Remove DS.FixtureAdapter since it's a subset of DS.InMemoryAdapter. The existence of both adapters is confusing.
  • implement DS.Serializer.transformFor to make implementing the null serializer easy as pie.
  • Add support for error and validations in DS.InMemoryAdapter.

Ongoing Concerns

  • More tests around associations
  • Implement map ignoring.
@ppcano

This comment has been minimized.

Show comment Hide comment
@ppcano

ppcano Dec 12, 2012

Contributor

This is absolutely useful, I has also missed the features to manage the situation you mention.

Contributor

ppcano commented Dec 12, 2012

This is absolutely useful, I has also missed the features to manage the situation you mention.

@kurko

View changes

packages/ember-data/lib/adapters/in_memory_adapter.js
+DS.InMemoryAdapter = DS.Adapter.extend({
+ serializer: DS.NullSerializer,
+
+ simulateRemoteReseponse: false,

This comment has been minimized.

Show comment Hide comment
@kurko

kurko Dec 12, 2012

Contributor

Typo? Reseponse

@kurko

kurko Dec 12, 2012

Contributor

Typo? Reseponse

This comment has been minimized.

Show comment Hide comment
@kurko

kurko Dec 12, 2012

Contributor

Pardon my ignorance, but if it's a typo and no tests failed, does it mean there are missing tests? Or maybe it's a flag for an Ajax mechanism (which I'm unaware of), which then make it difficult to test in the current PR?

@kurko

kurko Dec 12, 2012

Contributor

Pardon my ignorance, but if it's a typo and no tests failed, does it mean there are missing tests? Or maybe it's a flag for an Ajax mechanism (which I'm unaware of), which then make it difficult to test in the current PR?

This comment has been minimized.

Show comment Hide comment
@ahawkins

ahawkins Dec 12, 2012

Contributor

Yes. I need to port over the tests from the fixture adapter as unit test for the this adapter. Those tests would fail.

@ahawkins

ahawkins Dec 12, 2012

Contributor

Yes. I need to port over the tests from the fixture adapter as unit test for the this adapter. Those tests would fail.

@kurko

This comment has been minimized.

Show comment Hide comment
@kurko

kurko Dec 12, 2012

Contributor

I think it's perfectly valid. It's something similar to in-memory SQLite data in Rails projects, except that its main use case, as far as I can see, is to setup data before a test, have the data available ubiquitously, then reset it later. I'm not sure how useful it would be in development though.

I'm +1 for this.

I think that development and testing are essentially the same environment.

I don't think development and test environment are the same mainly because test data are temporary, being reset after each test, whereas that would be unfeasible in the development environment.

Contributor

kurko commented Dec 12, 2012

I think it's perfectly valid. It's something similar to in-memory SQLite data in Rails projects, except that its main use case, as far as I can see, is to setup data before a test, have the data available ubiquitously, then reset it later. I'm not sure how useful it would be in development though.

I'm +1 for this.

I think that development and testing are essentially the same environment.

I don't think development and test environment are the same mainly because test data are temporary, being reset after each test, whereas that would be unfeasible in the development environment.

@mikegrassotti

This comment has been minimized.

Show comment Hide comment
@mikegrassotti

mikegrassotti Dec 12, 2012

+1 - Awesome work, totally needed.

I've spent the past few weeks exploring how to use konacha for integration testing our ember app. DS.FixtureAdapter is OK for very simple specs but is not gonna work beyond that. This forces us to run most of our integration tests against the full-stack by spinning up a test-rails server, using the RestAdapter and wiping the db between tests. Total overkill as 90% of these specs are ember-specific.

This PR is a really elegant approach, looking forward to kicking the tires.

+1 - Awesome work, totally needed.

I've spent the past few weeks exploring how to use konacha for integration testing our ember app. DS.FixtureAdapter is OK for very simple specs but is not gonna work beyond that. This forces us to run most of our integration tests against the full-stack by spinning up a test-rails server, using the RestAdapter and wiping the db between tests. Total overkill as 90% of these specs are ember-specific.

This PR is a really elegant approach, looking forward to kicking the tires.

@ahawkins

This comment has been minimized.

Show comment Hide comment
@ahawkins

ahawkins Dec 12, 2012

Contributor

This forces us to run most of our integration tests against the full-stack by spinning up a test-rails server, using the RestAdapter and wiping the db between tests. Total overkill as 90% of these specs are ember-specific.

@grasscode Ya. That's bad. I couldn't even do that if I wanted to since our api and frontend app are completely separate projects. No easy way to run integration tests between the client and the server.

I don't think development and test environment are the same mainly because test data are temporary, being reset after each test, whereas that would be unfeasible in the development environment.

@kurko Development and test are the same in that they both need temporal data. The data is wiped between every test. In development you need fresh data for each page reload. You won't be wiping the data in development, but you do need an easy way to push data in once and use it throughout development.

Contributor

ahawkins commented Dec 12, 2012

This forces us to run most of our integration tests against the full-stack by spinning up a test-rails server, using the RestAdapter and wiping the db between tests. Total overkill as 90% of these specs are ember-specific.

@grasscode Ya. That's bad. I couldn't even do that if I wanted to since our api and frontend app are completely separate projects. No easy way to run integration tests between the client and the server.

I don't think development and test environment are the same mainly because test data are temporary, being reset after each test, whereas that would be unfeasible in the development environment.

@kurko Development and test are the same in that they both need temporal data. The data is wiped between every test. In development you need fresh data for each page reload. You won't be wiping the data in development, but you do need an easy way to push data in once and use it throughout development.

@dagda1

This comment has been minimized.

Show comment Hide comment
@dagda1

dagda1 Dec 12, 2012

Huge + 1 for this. It addresses the following which make testing painful:

  1. I cannot adequately teardown the store after each test.
  2. I get caught up in semantics and ceremony when all I want to do is work with plain old objects for my tests, e.g. making sure belongsTo keys are user_id in my sample data.
  3. All I can do is load the FIXTURES array.

This will make our testing lives so much easier.

dagda1 commented Dec 12, 2012

Huge + 1 for this. It addresses the following which make testing painful:

  1. I cannot adequately teardown the store after each test.
  2. I get caught up in semantics and ceremony when all I want to do is work with plain old objects for my tests, e.g. making sure belongsTo keys are user_id in my sample data.
  3. All I can do is load the FIXTURES array.

This will make our testing lives so much easier.

@ahawkins

This comment has been minimized.

Show comment Hide comment
@ahawkins

ahawkins Dec 12, 2012

Contributor

I get caught up in semantics and ceremony when all I want to do is work with plain old objects for my tests, e.g. making sure belongsTo keys are user_id in my sample data.

@dagda1. Same thing for development. Who needs freaking semantics :)

Contributor

ahawkins commented Dec 12, 2012

I get caught up in semantics and ceremony when all I want to do is work with plain old objects for my tests, e.g. making sure belongsTo keys are user_id in my sample data.

@dagda1. Same thing for development. Who needs freaking semantics :)

@ahawkins

This comment has been minimized.

Show comment Hide comment
@ahawkins

ahawkins Dec 17, 2012

Contributor

I am going to add error support to this as well.

Contributor

ahawkins commented Dec 17, 2012

I am going to add error support to this as well.

@walter

This comment has been minimized.

Show comment Hide comment
@walter

walter Dec 23, 2012

Just a heads up for those dealing with this sort of problem (API call stubbing) while this issue's code solidifies. Here's the technique I'm trying:

I believe I have this working with a very simple test, but I'm just getting into the code that needs this. I'll let you know how it progresses.

walter commented Dec 23, 2012

Just a heads up for those dealing with this sort of problem (API call stubbing) while this issue's code solidifies. Here's the technique I'm trying:

I believe I have this working with a very simple test, but I'm just getting into the code that needs this. I'll let you know how it progresses.

@drogus

This comment has been minimized.

Show comment Hide comment
@drogus

drogus Jan 20, 2013

Contributor

@walter the problem with this approach is that you're bound to the API that you're using at the moment, so if you change the API you need to change your tests, which should not be the case most of the time

Also, bumping this PR, because it would be really cool to have something like that.

Contributor

drogus commented Jan 20, 2013

@walter the problem with this approach is that you're bound to the API that you're using at the moment, so if you change the API you need to change your tests, which should not be the case most of the time

Also, bumping this PR, because it would be really cool to have something like that.

@walter

This comment has been minimized.

Show comment Hide comment
@walter

walter Jan 20, 2013

@drogus, yep. Just mentioning for those looking for something they can use while this gets fleshed out. I've actually switched DS.FixtureAdapter for the moment. I wouldn't recommend Sinon.JS's FakeServer unless you absolutely need it.

walter commented Jan 20, 2013

@drogus, yep. Just mentioning for those looking for something they can use while this gets fleshed out. I've actually switched DS.FixtureAdapter for the moment. I wouldn't recommend Sinon.JS's FakeServer unless you absolutely need it.

@ahawkins

This comment has been minimized.

Show comment Hide comment
@ahawkins

ahawkins Jan 25, 2013

Contributor

@tomdale @wycats I think we have a problem. In master the FixtureAdapter uses JSONSerializer. It also bypasses adapter.didFindQuery and those things. adapterDidFindQuery uses the loader which does all the crazy crap to handle all the loading--most notably expecting there to be a root element. So when FixtureAdapter is updated to work the same way as RESTAdapter with didFindXXX things blow up. It seems the existing FixtureAdapter got aroudn this by interacting with the store and RecordArrays directly. I don't know if this is a good thing.

This is the problem. Say you have this in FIXTURES

App.Person.FIXTURES = [{
    id: 'wycats',
    firstName: "Yehuda",
    lastName: "Katz",

    height: 65
  },

  {
    id: 'ebryn',
    firstName: "Erik",
    lastName: "Brynjolffsosysdfon",

    height: 70
  }];

This doesn't work because the JSON serializer is thinking: I need to sideload the "firstName", "lastName", and "height" associations.

It really expects this:

App.Person.FIXTURES = [{
    person: {
      id: 'wycats',
      firstName: "Yehuda",
      lastName: "Katz",

      height: 65
  },

  {
    person: {
      id: 'ebryn',
      firstName: "Erik",
      lastName: "Brynjolffsosysdfon",

      height: 70
    }
  }];

I'm not sure how to proceed at this point.

EDIT: I've pushed my commits. All the tests in fixture_adapter_test.js are failing for this reason. Everything else is backward compatible at this point.

Contributor

ahawkins commented Jan 25, 2013

@tomdale @wycats I think we have a problem. In master the FixtureAdapter uses JSONSerializer. It also bypasses adapter.didFindQuery and those things. adapterDidFindQuery uses the loader which does all the crazy crap to handle all the loading--most notably expecting there to be a root element. So when FixtureAdapter is updated to work the same way as RESTAdapter with didFindXXX things blow up. It seems the existing FixtureAdapter got aroudn this by interacting with the store and RecordArrays directly. I don't know if this is a good thing.

This is the problem. Say you have this in FIXTURES

App.Person.FIXTURES = [{
    id: 'wycats',
    firstName: "Yehuda",
    lastName: "Katz",

    height: 65
  },

  {
    id: 'ebryn',
    firstName: "Erik",
    lastName: "Brynjolffsosysdfon",

    height: 70
  }];

This doesn't work because the JSON serializer is thinking: I need to sideload the "firstName", "lastName", and "height" associations.

It really expects this:

App.Person.FIXTURES = [{
    person: {
      id: 'wycats',
      firstName: "Yehuda",
      lastName: "Katz",

      height: 65
  },

  {
    person: {
      id: 'ebryn',
      firstName: "Erik",
      lastName: "Brynjolffsosysdfon",

      height: 70
    }
  }];

I'm not sure how to proceed at this point.

EDIT: I've pushed my commits. All the tests in fixture_adapter_test.js are failing for this reason. Everything else is backward compatible at this point.

@ahawkins

This comment has been minimized.

Show comment Hide comment
@ahawkins

ahawkins Feb 12, 2013

Contributor

@tchak PR's have fixed most of this.

Contributor

ahawkins commented Feb 12, 2013

@tchak PR's have fixed most of this.

@ahawkins ahawkins closed this Feb 12, 2013

@ryanjm

This comment has been minimized.

Show comment Hide comment
@ryanjm

ryanjm Feb 28, 2013

@twinturbo - Can you link to which PR's fixed this issue? I'm trying to figure out how to properly test my app and it would be helpful mock out the Adapter.

ryanjm commented Feb 28, 2013

@twinturbo - Can you link to which PR's fixed this issue? I'm trying to figure out how to properly test my app and it would be helpful mock out the Adapter.

@ahawkins

This comment has been minimized.

Show comment Hide comment
@ahawkins

ahawkins Mar 1, 2013

Contributor

@ryanjm the current fixture adapter generally handles the serialization cases described in this PR. #728 brings in the rest.

Contributor

ahawkins commented Mar 1, 2013

@ryanjm the current fixture adapter generally handles the serialization cases described in this PR. #728 brings in the rest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment