Skip to content

Commit

Permalink
Fixes several embedded bugs.
Browse files Browse the repository at this point in the history 
1. Ensure that embedded records returned in
   payloads containing multiple records (e.g.,
   in responses to queries or "find alls") are
   extracted.
2. Make sure that loading records with embedded
   data multiple times is handled correctly.
  • Loading branch information
tomhuda committed Jan 4, 2013
1 parent 753b92c commit a72ed12
Show file tree
Hide file tree
Showing 12 changed files with 183 additions and 40 deletions.
16 changes: 13 additions & 3 deletions packages/ember-data/lib/serializers/json_serializer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,26 +121,36 @@ DS.JSONSerializer = DS.Serializer.extend({

// EXTRACTION

extract: function(loader, json, type) {
extract: function(loader, json, type, record) {
var root = this.rootForType(type);

this.sideload(loader, type, json, root);
this.extractMeta(loader, type, json);

if (json[root]) {
if (record) { loader.updateId(record, json[root]); }
this.extractRecordRepresentation(loader, type, json[root]);
}
},

extractMany: function(loader, json, type) {
extractMany: function(loader, json, type, records) {
var root = this.rootForType(type);
root = this.pluralize(root);

this.sideload(loader, type, json, root);
this.extractMeta(loader, type, json);

if (json[root]) {
loader.loadMany(type, json[root]);
var objects = json[root], references = [];
if (records) { records = records.toArray(); }

for (var i = 0; i < objects.length; i++) {
if (records) { loader.updateId(records[i], objects[i]); }
var reference = this.extractRecordRepresentation(loader, type, objects[i]);
references.push(reference);
}

loader.populateArray(references);
}
},

Expand Down
20 changes: 9 additions & 11 deletions packages/ember-data/lib/system/adapter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ function loaderFor(store) {
return store.loadMany(type, array);
},

updateId: function(record, data) {
return store.updateId(record, data);
},

populateArray: Ember.K,

sideload: function(type, data) {
return store.load(type, data);
},
Expand Down Expand Up @@ -189,18 +195,9 @@ DS.Adapter = Ember.Object.extend(DS._Mappable, {
store.didSaveRecord(record);
}, this);

records = records.toArray();

if (payload) {
var loader = DS.loaderFor(store);
loader.loadMany = function(type, array) {
for (var i = 0; i < array.length; i++) {
store.updateId(records[i], array[i]);
}
store.loadMany(type, array);
};

get(this, 'serializer').extractMany(loader, payload, type);
get(this, 'serializer').extractMany(loader, payload, type, records);
}
},

Expand Down Expand Up @@ -403,7 +400,8 @@ DS.Adapter = Ember.Object.extend(DS._Mappable, {
*/
didFindQuery: function(store, type, payload, recordArray) {
var loader = DS.loaderFor(store);
loader.loadMany = function(type, data) {

loader.populateArray = function(data) {
recordArray.load(data);
};

Expand Down
2 changes: 2 additions & 0 deletions packages/ember-data/lib/system/model/model.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ DS.Model = Ember.Object.extend(Ember.Evented, LoadPromise, {
var ids = this._data.hasMany[key] || [];

var references = map(ids, function(id) {
// if it was already a reference, return the reference
if (typeof id === 'object') { return id; }
return store.referenceForId(type, id);
});

Expand Down
4 changes: 1 addition & 3 deletions packages/ember-data/lib/system/record_arrays/adapter_populated_record_array.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@ DS.AdapterPopulatedRecordArray = DS.RecordArray.extend({
throw new Error("The result of a server query (on " + type + ") is immutable.");
},

load: function(array) {
load: function(references) {
var store = get(this, 'store'), type = get(this, 'type');

var references = store.loadMany(type, array);

this.beginPropertyChanges();
set(this, 'content', Ember.A(references));
set(this, 'isLoaded', true);
Expand Down
2 changes: 0 additions & 2 deletions packages/ember-data/lib/system/record_arrays/many_array.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,6 @@ DS.ManyArray = DS.RecordArray.extend({
// the change.
for (var i=index; i<index+removed; i++) {
var reference = get(this, 'content').objectAt(i);
//var record = this.objectAt(i);
//if (!record) { continue; }

var change = DS.RelationshipChange.createChange(owner.get('clientId'), reference.clientId, get(this, 'store'), {
parentType: owner.constructor,
Expand Down
12 changes: 8 additions & 4 deletions packages/ember-data/lib/system/serializer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,15 @@ DS.Serializer = Ember.Object.extend({
extract: mustImplement('extract'),
extractMany: mustImplement('extractMany'),

extractRecordRepresentation: function(loader, type, json) {
extractRecordRepresentation: function(loader, type, json, shouldSideload) {
var mapping = this.mappingForType(type);
var embeddedData, prematerialized = {};
var embeddedData, prematerialized = {}, reference;

var reference = loader.load(type, json);
if (shouldSideload) {
reference = loader.sideload(type, json);
} else {
reference = loader.load(type, json);
}

this.eachEmbeddedHasMany(type, function(name, relationship) {
var embeddedData = json[this.keyFor(relationship)];
Expand All @@ -230,7 +234,7 @@ DS.Serializer = Ember.Object.extend({
var references = map.call(array, function(item) {
if (!item) { return; }

var reference = this.extractRecordRepresentation(loader, relationship.type, item);
var reference = this.extractRecordRepresentation(loader, relationship.type, item, true);

// If the embedded record should also be saved back when serializing the parent,
// make sure we set its parent since it will not have an ID.
Expand Down
6 changes: 3 additions & 3 deletions packages/ember-data/lib/system/store.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ require("ember-data/system/record_arrays");
require("ember-data/system/transaction");
require("ember-data/system/mixins/mappable");

var get = Ember.get, set = Ember.set, fmt = Ember.String.fmt;
var get = Ember.get, set = Ember.set, fmt = Ember.String.fmt, once = Ember.run.once;
var forEach = Ember.EnumerableUtils.forEach;
// These values are used in the data cache when clientIds are
// needed but the underlying data has not yet been loaded by
Expand Down Expand Up @@ -1548,14 +1548,14 @@ DS.Store = Ember.Object.extend(DS._Mappable, {

var record = this.recordCache[clientId];
if (record) {
record.loadedData();
once(record, 'loadedData');

This comment has been minimized.

Sign in to view

Copy link
@ppcano

ppcano Jan 4, 2013

Contributor

@wycats @tomdale , delaying the loadedData call allows to execute the event when the record has been moved to other state where the loadedData event cannot be handled.

I reproduce an example here
}
} else {
clientId = this.pushData(data, id, type);
cidToPrematerialized[clientId] = prematerialized;
}

this.updateRecordArrays(type, clientId);
this.updateRecordArraysLater(type, clientId);

return this.referenceForClientId(clientId);
},
Expand Down
1 change: 1 addition & 0 deletions packages/ember-data/tests/integration/application_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ if (Ember.Application.initializer) {

teardown: function() {
app.destroy();
Ember.BOOTED = false;
}
});

Expand Down
126 changes: 124 additions & 2 deletions packages/ember-data/tests/integration/embedded/embedded_loading_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ var originalLookup = Ember.lookup, lookup;

var Adapter, store, adapter;

var Person = DS.Model.extend();
var App = Ember.Namespace.create({
name: "App"
});

var Person = App.Person = DS.Model.extend();

var Comment = DS.Model.extend({
var Comment = App.Comment = DS.Model.extend({
user: DS.belongsTo(Person)
});

Expand Down Expand Up @@ -96,4 +100,122 @@ Ember.ArrayPolyfills.forEach.call([Person, "Person"], function(mapping) {
strictEqual(person1.get('comments').objectAt(0), comment1);
strictEqual(person2.get('comments').objectAt(0), comment1);
});

asyncTest("An embedded hasMany relationship can be extracted if the JSON is returned in response to a find", function() {
Adapter.map(mapping, {
comments: { embedded: 'load' }
});

adapter = Adapter.create();
store.set('adapter', adapter);

adapter.find = function(store, type, id) {
var self = this;

setTimeout(function() {
Ember.run(function() {
self.didFindRecord(store, type, {
person: {
id: 1,
name: "Erik Brynroflsson",
comments: [{ id: 1 }, { id: 2 }]
}
}, id);
});

done();
});
};

store.find(Person, 1);

function done() {
start();

var person1 = store.find(Person, 1);
var comment1 = store.find(Comment, 1);
var comment2 = store.find(Comment, 2);

strictEqual(person1.get('comments').objectAt(0), comment1);
}
});

asyncTest("An embedded hasMany relationship can be extracted if the JSON is returned in response to a findAll", function() {
Adapter.map(mapping, {
comments: { embedded: 'load' }
});

adapter = Adapter.create();
store.set('adapter', adapter);

adapter.findAll = function(store, type) {
var self = this;

setTimeout(function() {
Ember.run(function() {
self.didFindAll(store, type, {
persons: [{
id: 1,
name: "Erik Brynroflsson",
comments: [{ id: 1 }, { id: 2 }]
}, {
id: 2,
name: "Patrick Gibson",
comments: [{ id: 1 }, { id: 2 }]
}]
});
});

done();
});
};

store.find(Person);

function done() {
start();

var person1 = store.find(Person, 1);
var person2 = store.find(Person, 2);
var comment1 = store.find(Comment, 1);
var comment2 = store.find(Comment, 2);

strictEqual(person1.get('comments').objectAt(0), comment1);
strictEqual(person2.get('comments').objectAt(0), comment1);
}
});

test("Loading the same record with embedded hasMany multiple times works correctly", function() {
Adapter.map(mapping, {
comments: { embedded: 'load' }
});

adapter = Adapter.create();
store.set('adapter', adapter);

Ember.run(function() {
adapter.load(store, Person, {
id: 1,
name: "Erik Brynroflsson",
comments: [{ id: 1 }, { id: 2 }]
});
});

var person = store.find(Person, 1);
person.get('comments');

// Load the same data twice
Ember.run(function() {
adapter.load(store, Person, {
id: 1,
name: "Erik Brynroflsson",
comments: [{ id: 1 }, { id: 2 }]
});
});

var comment1 = person.get('comments').objectAt(0);

equal(comment1.get('id'), 1, "comment with ID 1 was loaded");
});
});

8 changes: 6 additions & 2 deletions packages/ember-data/tests/integration/queries_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ var Person, store, adapter;

module("Queries", {
setup: function() {
Person = DS.Model.extend({
var App = Ember.Namespace.create({ name: "App" });

Person = App.Person = DS.Model.extend({
updatedAt: DS.attr('string'),
name: DS.attr('string'),
firstName: DS.attr('string'),
Expand All @@ -28,10 +30,12 @@ test("When a query is made, the adapter should receive a record array it can pop

stop();

var self = this;

// Simulate latency to ensure correct behavior in asynchronous conditions.
// Once 100ms has passed, load the results of the query into the record array.
setTimeout(function() {
recordArray.load([{ id: 1, name: "Peter Wagenet" }, { id: 2, name: "Brohuda Katz" }]);
self.didFindQuery(store, type, { persons: [{ id: 1, name: "Peter Wagenet" }, { id: 2, name: "Brohuda Katz" }] }, recordArray);
}, 100);
};

Expand Down
8 changes: 5 additions & 3 deletions packages/ember-data/tests/integration/store_adapter_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ var Person, store, adapter;

module("DS.Store and DS.Adapter integration test", {
setup: function() {
Person = DS.Model.extend({
var App = Ember.Namespace.create({ name: "App" });

App.Person = Person = DS.Model.extend({
updatedAt: DS.attr('string'),
name: DS.attr('string'),
firstName: DS.attr('string'),
Expand Down Expand Up @@ -341,10 +343,10 @@ test("can be created after the DS.Store", function() {

test("the filter method can optionally take a server query as well", function() {
adapter.findQuery = function(store, type, query, array) {
array.load([
this.didFindQuery(store, type, { persons: [
{ id: 1, name: "Yehuda Katz" },
{ id: 2, name: "Tom Dale" }
]);
]}, array);
};

var filter = store.filter(Person, { page: 1 }, function(data) {
Expand Down
Loading

0 comments on commit a72ed12

Please sign in to comment.