Add hasOne association #475

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
Contributor

pivotal-medici commented Nov 14, 2012

Models can now define a hasOne association as the inverse of a belongsTo
association:

App.Person = DS.Model.extend({
  heart: DS.hasOne(App.Heart)
});

App.Heart = DS.Model.extend({
  person: DS.belongsTo(App.Person)
});

Includes updates for DS.RESTSerializer to parse hasOne information out
of incoming record data but not include it with outgoing data.

pivotal-medici referenced this pull request Nov 14, 2012

Closed

hasOne support #380

Member

tchak commented Nov 14, 2012

Hey, nice work!
But I think we want to keep DS.OneToManyChange.
We should add DS.OneToOneChange and DS.ManyToManyChange, can you try to refactor in this direction?

Contributor

pivotal-medici commented Nov 14, 2012

@tchak But if the change is coming from the belongsTo side, you won't know what type of change you have when you're instantiating it.

Contributor

pivotal-medici commented Nov 14, 2012

I guess the real question is, why do you want to keep those classes? Is it (a) just to make the association observers more readable, (b) to make the relationship change internals more readable, or (c) to plan for some expected future complexity?

(A) If it's just to make the association observers more readable, I fear there may not be much we can do. As I said above, the belongsTo association doesn't know if it's paired with a hasMany or a hasOne. (That's partly because the association property doesn't know what model it's been attached to until runtime.) And if you ever hope to implement many-to-many relationships in the future, the hasMany side could no longer assume it was mapped to a belongsTo association. (Never mind the possibility of having one-sided relationships, when you might prefer not to declare an association on the opposite side at all.) The point is, neither side can be entirely sure what it's pointed at.

Not to say that you couldn't look that up at runtime. But you'd probably still be going through some kind of proxy object to do it, e.g., DS.RelationshipChange.findOrCreate(). At that point, the presence of DS.OneToManyChange (or any other kind of change) is invisible to the association observers.

(B) If this is just about making the relationship change internals more readable, then I'd be more than happy to take a crack at it. But it wouldn't eliminate the need for some public-facing DS.RelationshipChange class to manage all of that (for the aforementioned reasons).

(C) But if this is about planning for some future change, then I might caution against a refactor attached to this new implementation. I'd rather see that functionality in place before making any decisions about how best to refactor for it. This stuff is pretty complex and many-to-many will only make it more so.

Contributor

elliterate commented Nov 19, 2012

After chatting with @wycats and @tomdale about it, I'm going to take a crack at refactoring the internals.

Contributor

pivotal-medici commented Nov 27, 2012

We've done a refactor pass. Any thoughts?

pex commented Nov 27, 2012

👍

Owner

tomdale commented Nov 28, 2012

As you may have noticed, I just merged our adapter dirtiness branch into master. Let's let that shake out for a day or two, then discuss bringing this PR up-to-date and getting it merged in.

Contributor

ahawkins commented Dec 21, 2012

yes please. What can I do to help with getting 1-1 associations back?

SweeD commented Feb 6, 2013

Hey cool. Nice work! ❤️

Are there any news about this?

Owner

trek commented Feb 10, 2013

@tomdale this seems like it's been hanging in limbo for a bit too long.

Contributor

ahawkins commented Feb 10, 2013

Agree
On Feb 10, 2013 4:23 PM, "Trek Glowacki" notifications@github.com wrote:

@tomdale https://github.com/tomdale this seems like it's been hanging
in limbo for a bit too long.


Reply to this email directly or view it on GitHubhttps://github.com/emberjs/data/pull/475#issuecomment-13351262..

Member

darthdeus commented Feb 10, 2013

👍

Contributor

Globegitter commented Feb 13, 2013

👍

👍 This would be really useful

ilovett commented Feb 17, 2013

+1 I have a need for this

👍

Contributor

pivotal-medici commented Feb 27, 2013

This has been rewritten against master.

Member

tchak commented Feb 28, 2013

@pivotal-medici could you add necessary hooks to fixuture adapter/serializer ?

@elliterate elliterate Add hasOne relationship
Models can now define a hasOne relationship as the inverse of a belongsTo
relationship:

  App.Person = DS.Model.extend({
    heart: DS.hasOne(App.Heart)
  });

  App.Heart = DS.Model.extend({
    person: DS.belongsTo(App.Person)
  });
8833f8d
Contributor

pivotal-medici commented Mar 4, 2013

@tchak Done.

Owner

igorT commented Apr 8, 2013

The way you do the 1-1 relationships is to declare a belongsTo on both sides. The only conceptual difference between a pair of belongsTo and a belongsTo/hasOne is that the second one implicitly tells the server on what side the relationship is saved(that the foreign key is on the hasOne side) while in the first case you need to tell that to the adapter yourself. From a conceptual standpoint you really should be doing this in an adapter and not in your model layer as you can imagine different adapters saving the foreign key on different sides. However if we provided hasOne as a sugar for belongsTo with a mapping for an adapter that could work. @wycats @tomdale? This much code for such a simple convenience scares me, so I am closing this for now but would be open for some simple sugar around belongsTo.

igorT closed this Apr 8, 2013

Contributor

pivotal-medici commented Apr 8, 2013

Being able to specify the parent in a 1:1 relationship is critical for features like #440, which allows parents and their children to be saved in the same commit. The adapter needs that information to decide who should be saved first. As such, it seems absurd to implement this as an adapter mapping/configuration, as it's absolutely required for your application to function correctly in the presence of a 1:1 relationship. (In fact, I'd dare say that implementing a 1:1 relationship with two belongsTo's or two hasOne's should throw an error.)

Yes, different adapters will save different sides, but that doesn't change the fact that conceptually one object can be thought to be the parent of another. The concept is already present thanks to the hasMany/belongsTo language. The model layer is intrinsically relational. As such, it seems all too appropriate that the DSL of model definition allow you to fully express the variety of those relationships. The adapter can then decide what, if anything, to do with that information.

— Ian (@elliterate)

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