Added reverseKey support for Many relations

[ronen]

Hi. Well, since I mouthed off in #51 about how much I wanted reverse keys, I figured I should put my money where my mouth is. So I've taken the liberty of coding up an implementation of reverseKey (for Many relations only).

Here are some notes/documentation. For the sake of discussion, assume a relation of the form

Zoo = Backbone.AssociatedModel.extend({
                 relations: [{
                      type: Backbone.Many,
                      key: 'animals,
                      relatedModel: Animal,
                      reverseKey: 'zoo'
                 }],
                 defaults: {
                      children: []
                 }
             });

• You can add an Animal to a Zoo's "animals" collection from either direction:

  animal.set("zoo", zoo)
  zoo.get("animals").add(animal)  // or set(animal) or reset([animal])
  

• You can likewise remove an Animal from a Zoo's "animals" collection from either direction:

  animal.set("zoo", null)
  animal.set("zoo", otherZoo) // removes from current zoo and adds to otherZoo
  zoo.get("animals").remove(animal) // or reset([otherAnimal]) which implicitly removes animal
  animal.destroy()
  

• No events are sent until the object graph is completely updated. Here's a quick rundown of the events:

  • When an Animal is added to a Zoo, in addition to the "add" event, the new zoo's "animals" collection gets a "change" and a "change:zoo" event noting that the animal's "zoo" attribute has been set, with the change-related API working properly, and the events properly bubbling to the zoo model.
  • When an Animal is removed from a Zoo, in addition to the "remove" event, the new zoo's "animals" collection gets a "change" and a "change:zoo" event noting that the animal's "zoo" attribute has been set, with the change-related API working properly, and the events properly bubbling to the zoo model. (Note at the time the change event is triggered, the animal is no longer in the collection; in the zoo's event path for the change event, the index refers to the index prior to removal).
  • The only exception when adding is that if an Animal is created with a zoo provided as an initialization attribute, that zoo's "animals" collection gets an "add" event but no "change" events (it's not considered a change since that's the initial zoo value).
  • The only exception when deleting is that if an Animal is destroyed, it's zoo's "animals" collection gets a "remove" event but no "change" events (it's not considered a change, since that's the final zoo value).
  • When creating a new Animal model with a zoo provided as an initialization attribute, or when creating a new Zoo model with a set of models as an initialization attribute, the relations are not processed until after initialization of the new model. This ensures that the new model has been completely initialized before the related objects start handling it. (One quirk: within the new model's initializer, the new model's own attributes will have been set, but the related objects will not have been updated. That's arguably something that should be changed, so that the related objects will have been updated, although no events triggered yet.)

(Getting the events right was the trickiest part of the implementation!)

I've included a whole bunch of tests for the new behavior, and have verified that all the old tests still pass. I tried to minimize/localize the impact on the code, and of course to have minimal runtime impact if relationKey is not specified. I've tried it out in my app and it seems to be working well. Hope you like it!

And of course I'm open to suggestions If you think I did anything wrong in terms of functionality or implementation.

Happy new year :)

[dhruvaray]

Wow...That's one mammoth PR!

Thank you and a a happy new year as well!

I am going to have to spend some serious time on this before getting back to you...So please hold on!

[ronen]

Yes of course it'll take some reading. Take your time. And if you have any questions feel free to ask (in this thread or via email).

FYI I just pushed one more (last I hope! :) commit to this PR. This closes a hole that I had left open: If you set a reverseKey property in a Many relation, it doesn't create the implied reverse One relation until the forward key is set at least once (which typically happens by the defaults upon instantiation of the parent). For the above example, that means that if you do:

 animal = new Animal({
      species: 'Giraffe',
      zoo: {
         name: "Bronx Zoo",
    });

before you've ever created any other zoo, the reverse relation won't exist and the new animal will simply get the raw object as the value of zoo'

The latest PR lets you fix this in two ways:

• You can now define a reverseKey on a Backbone.One relation. If you know that you'll be creating animals before zoos, you can define it once there instead of .
• You can now define both the forward and reverse relations explicitly, with the corresponding reverseKey values. If you don't want to worry about which will be instantiated first, this is the safest way. It's not as DRY, but the code checks to make sure that the two relations are consistent so you shouldn't get into trouble.

Thus:

Animal = Backbone.AssociatedModel.extend({
             relations: [{
                  type: Backbone.One,
                  key: 'zoo',
                  relatedModel: 'Zoo',
                  reverseKey: 'animals'
             }],
         });

Zoo = Backbone.AssociatedModel.extend({
             relations: [{
                  type: Backbone.Many,
                  key: 'animals,
                  relatedModel: Animal,
                  reverseKey: 'zoo'
             }],
             defaults: {
                  children: []
             }
         });

Let me know if you want me to squash the commits together and issue a clean PR.

Cheers

[dhruvaray]

@ronen : After spending some time with the code, I thought it would be prudent to refer folks to your repo changes - in our documentation - should they feel the need of auto-reverse relations.

The cost of auto-relations has added a lot of incidental complexity (and corner cases) in the library. I yet believe that the extra cost of specifying the reverse relation explicitly (by the application) is well worth the additional complexity which would otherwise get added to the base library if we incorporated this feature.

I hope you understand!

I would however love to see this branch merged to our repo HEAD.

[ronen]

@dhruvaray sorry for the delay, i'm currently on (paternity) leave, focusing on non-tech stuff for a while.

... corner cases ... by the application ...

to me, the existence of corner cases is why it should be in the library rather than the application; otherwise each application developer would need to find and debug those cases independently.

I would however love to see this branch merged to our repo HEAD.

will try to do that when i get a chance.

I thought it would be prudent to refer folks to your repo changes - in our documentation - should they feel the need of auto-reverse relations.

doesn't completely sit right to me to maintain a ghost side branch. seems like it'd be candidate for getting out of sync, incompatible changes, etc. perhaps after merging against HEAD will see how simple/complex it gets.