-
Notifications
You must be signed in to change notification settings - Fork 76
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
Bidirectional hasMany relationships are not loadable/saveable from the many side #16
Comments
Sounds reasonable. I'd be happy with the copy-paste solution until a better solution gets cooked up by the Ember Data team. |
Eep, I did read this thread, but not closely enough, apparently. I think the “from the many side” confused me, but I should have recognised this as my problem. I tried adding your serializer. Now there’s the opposite problem: the ID of the parent model is now being stored as null. If you try out the modified example application, the features are properly pre-loaded, but the relation is lost when you click the Maybe it has to do with the way I’m constructing the models. It’s an artificial example because it’s what I’m using to set up tests, so it’s not UI-driven. Or maybe a Thanks for noticing my floundering in that other thread! |
Never mind, I was able to get the child model to have the parent model ID by either passing in the parent when constructing the child, or saving the child after adding it to the parent. Still learning hehe |
This sounds reasonable. Any chance of a PR or even just a modification to the readme? :) |
So, this is more of a |
So I admit I'm still confused: what exactly needs to change to satisfy Ember Data? In relational-pouch, we recently added async relations, which I maybe naively assumed cohered to Ember Data's requirements. What I'm hearing is that when you fetch a |
So "async" is definitely the way to go. Sideloading is interesting in case of http backend (less requests), but for local db it only adds complexity. ember-data 1.0 will remove non async relationships (you still will be able to sideload in case of rest adapter). The problem here is the fact that What you saying is that |
Ah okay, so if I just make |
@nolanlawson It seems so. But right now, options are passed down to |
Don't worry, I'll just wait for further instructions, and also anybody who wants to tackle this is more than welcome; I am an expert on PouchDB but very much the opposite when it comes to Ember. 😛 |
Right. It's not a great solution, but it's 1) simple to implement and 2) the same as all the other client-side persistence options that I've examined. Another intermediate step would be to both change the serializer and also have ember-pouch handle doing the saves on both ends. The data is still un-normalized, but then the client application wouldn't need to be aware of it. I could see that being surprising, though — changing an association could result in other changes to related objects being silently persisted.
Async relations are part of ember-data's understanding of the world, but do not address this problem directly. They say "when you do a From my outside-the-project perspective, I think the overall disconnect is that ember-data's design is based on doing object mapping for documents received from/sent to an API server. There's an assumption that you can have have different representations for serialization vs. deserialization because there's another layer (the API server) which is handling translations to and from a canonical data store. If we want ember-pouch to behave transparently for bidirectional relationships, ember-pouch (or pouch-relational) will have to take on the relationship-management tasks that ember-data currently assumes the server is handling. This would mean that ember-pouch (or pouch-relational) would need to understand all the ember-data association types and handle doing the appropriate queries to resolve either the ids (for an
It would be great if ember-data would handle this stuff for us. I wonder how likely that is, though — for its primary use case of data-loaded-from-a-server, requiring a separate query to determine the content of a |
@rsutphin Another thing to keep in mind is that, unlike a remote REST API, PouchDB should give you roughly the same performance whether you do |
Agreed, @nolanlawson. I think the possibly-noticeable difference for ember-pouch would be whether the next level of relationships are loaded also. |
I'm not sure when ember-data added _shouldSerializeHasMany: function() { return true; } |
Or use an initializer to import it? Example: https://github.com/firebase/emberfire/blob/master/addon/initializers/emberfire.js |
I'm more in favor of an explicit import. import {Model, Adapter, Serializer} from 'ember-pouch'; |
I'm in favor of an explicit import also. An initializer is harder for a user of the library to discover & override. |
If it is well documented for beginners lets do the explicit import. |
@patuku-roy I'm not 100% on this but you should use a serializer per type that extends your pouch serializer (it's just the RESTSerializer) and declare the export default DS.RESTSerializer.extend(DS.EmbeddedRecordsMixin, {
attrs: {
comments: {serialize: true}
}
}); See the docs for more infos about DS.EmbeddedRecordsMixin |
@fsmanuel Thanks, I did per type serializer and work like a charm, except I can not use Another problem coming, every time I saved the model, I can not fetch the record until I refresh the route for couple times, but I suspect this is Ember runloop issue when reload the model rather than ember-pouch issue. // app/models/post.js
export default DS.Model.extend({
rev: DS.attr("string"),
comments: DS.hasMany("comment", {async: true})
});
// app/serializers/post.js
export default DS.RESTSerializer.extend(DS.EmbeddedRecordsMixin, {
attrs: {
comments: {serialize: "ids"}
}
}); and I save the model like this // app/routes/post.js
this.store.createRecord("comment", {
// ... put the data here ...
}).save().then(function(comment) {
// save relation to each side
post.get("comments").get("content").pushObject(comment);
post.save();
}); |
what do you mean by "every time I saved the model, I can not fetch the record until I refresh the route for couple times"? |
@fsmanuel My bad, I use transitionTo before the record saved, after I'm waiting the record saved, the model can show up without any problem. This is the working version, // app/routes/post.js
var self = this;
this.store.createRecord("comment", {
// ... put the data here ...
}).save().then(function(comment) {
// save relation to each side
post.get("comments").get("content").pushObject(comment);
post.save();
self.set("didSave", true);
}); and add new method to use transitionTo when the record saved. // app/routes/post.js
didSave: false,
_transitionAfterSave: function(transitionTo) {
var self = this;
this.addObserver("didSave", function() {
if(self.get("didSave"))
{
self.set("didSave", false);
self.transitionTo(transitionTo);
}
});
}, Thanks anyway ... |
@fsmanuel Do you have time to write a PR for |
Is there any progress on this one? After reading through the issue I'm still not sure how to do this the correct way. |
@MSchmidt I got this to work by adding an application serializer that extends ember-pouch's serializer:
Now the ids for the manyToOne relationship are stored on the hasMany side as well. Here is the private method we are overriding Ember: 2.2.0 |
@openhouse thanks for your simple solution! |
@openhouse could you open a PR? If your method is successful, it should be added to the repo. :) |
This incorporates the proposed fix for pouchdb-community#16, but it doesn’t actually fix the problem. Perhaps I’m misunderstanding!
I made a PR with the suggested fix but it doesn’t seem like it works, to me… though maybe I’m missing something? |
Maybe it is working in my application, but I don’t know why the test in my PR is failing. This situation where the child record needs to be saved, inserted into the parent collection, and then the parent saved seems unwieldy and confusing for people who are familiar with Ember Data. Is anyone knowledgable enough about Ember Data to know whether there’s a way to cause a parent model to be automatically saved when a child model is? I’ve poked around the code and it seems like maybe it’s not possible, but maybe someone has a better understanding. |
This is the technique described in pouchdb-community/ember-pouch#16 It’s awkward to have to save the parent model, but better than having to reconstruct the relationships!
@openhouse thanks a lot! It's a lifesaver! |
Hey, so I submitted another PR that incorporates the I’ll have to come back to see why that test is failing; it’s a seemingly-unrelated one. |
As suggested by @fsmanuel: #16 (comment)
* upstream/master: (32 commits) fixing readme reference to config key, concerning adapter blueprint. 3.2.1 Fix(Addon): Call super in init 3.2.0 Add relationship documentation Update README.md update readme update readme 3.1.1 (pouchdb-community#16) - Add override so serialiser saves hasMany Pouch adapter now calls a hook method when encountering a change for a record that is not yet loaded. update readme 3.1.0 Fix version check for Ember Data 3.2.x Update changelog for pouchdb-community#103 Tests for existing change watcher behavior Factor out integration test setup into module helper Move blueprint files into explicit directories Changelog for pouchdb-community#101 and pouchdb-community#102. created blueprint for pouch-adapter ...
* upstream/master: (32 commits) fixing readme reference to config key, concerning adapter blueprint. 3.2.1 Fix(Addon): Call super in init 3.2.0 Add relationship documentation Update README.md update readme update readme 3.1.1 (pouchdb-community#16) - Add override so serialiser saves hasMany Pouch adapter now calls a hook method when encountering a change for a record that is not yet loaded. update readme 3.1.0 Fix version check for Ember Data 3.2.x Update changelog for pouchdb-community#103 Tests for existing change watcher behavior Factor out integration test setup into module helper Move blueprint files into explicit directories Changelog for pouchdb-community#101 and pouchdb-community#102. created blueprint for pouch-adapter ...
* upstream/master: (32 commits) fixing readme reference to config key, concerning adapter blueprint. 3.2.1 Fix(Addon): Call super in init 3.2.0 Add relationship documentation Update README.md update readme update readme 3.1.1 (pouchdb-community#16) - Add override so serialiser saves hasMany Pouch adapter now calls a hook method when encountering a change for a record that is not yet loaded. update readme 3.1.0 Fix version check for Ember Data 3.2.x Update changelog for pouchdb-community#103 Tests for existing change watcher behavior Factor out integration test setup into module helper Move blueprint files into explicit directories Changelog for pouchdb-community#101 and pouchdb-community#102. created blueprint for pouch-adapter ...
The default behavior of
DS.JSONSerializer
is to not serialize the many side of bidirectional hasMany relationships. See emberjs/data#2494 for links to discussion about this.The upshot for
ember-pouch
is that the contents of a bidirectional hasMany relationship isn't saved or loaded when working from the many side. E.g., say you have these models:In this case you won't be able to load or save a person's sandwiches from the person. (You will be able to if you remove
owner
from sandwich, though.)The workaround for this that other similar adapters use (ember-localstorage-adapter, ember-indexeddb-adapter) is to provide a serializer which forces these associations to be serialized on the hasMany side (as well as the belongsTo side). I'm doing this now in my own application to work around this issue.
Unfortunately this requires copying a bunch of code from JSONSerializer (hence emberjs/data#2494). It also duplicates the information about the relationship, requiring both sides to be saved manually whenever the relationship changes.
I'm not sure if there's a practical better solution. It appears that the ember-data solution is to have the server use different serializations for reads vs. writes, but that doesn't work for a local database. One idea I had was that the adapter could look up the IDs for the hasMany relationship dynamically — since all the data is local this might work, but it also might be complex to get right.
The text was updated successfully, but these errors were encountered: