Injecting an item with relationships a second time breaks relationships

[OverZealous]

If you load an item from the server with partial server-side included information, then load it a second time with additional information, relationships will not be correctly injected and updated on the item.

Example showing issue: http://plnkr.co/edit/YpLgDBxL1ro05MKR3v9h

Notice, when you first load the page, the single injected Parent.children item is picked up. Upon clicking the button, you would expect the list of children to update, but they do not. This is because _injectRelations is not called if the item already exists in the cache.

The issues caused by this extend much further than just the items not being in the cache. The child items are actually no longer DS resources (even if they were previously), and so they are missing all functions and other behavior that would be expected.

[OverZealous]

It's probably obvious, but simply adding _injectRelations to the else side of _inject causes infinite recursion, so there's more to fixing this than that.

[OverZealous]

OK, I've found a solution that may or may not be acceptable, because it's a tiny-bit hackish:

At line 150 of inject.js, inserting this code works, while preventing infinite recursion:

//149     DS.utils.deepMixIn(item, attrs);
          if (definition.relations && !item.$$angular_data_did_inject) {
            item.$$angular_data_did_inject = true;
            _injectRelations.call(DS, definition, item, options);
            delete item.$$angular_data_did_inject;
          }
//        if (definition.resetHistoryOnInject) {

The hack is, hopefully obviously, that I needed to track which items were already injected this time around, so I set an arbitrary variable directly on the object, then delete it when I'm done.

This won't prevent relationship trees from being processed more than once, if (for example) the same item is attached multiple times, but it will at least guarantee you never get a stack overflow.

If this seems like an acceptable solution, I'd be happy to turn this into a PR. This is a pretty serious issue for the way we have been intending to use angular-data, and is going to really set me back in the design of the application.

[jmdobry]

Angular-data actually used to have the implementation you're suggesting, but there were other issues that were solved when I removed it. The relations are a complicated feature...

[OverZealous]

I'm trying desperately to find where it used to work like this, and what problems were fixed, but it's really hard to track down what changes were made to inject.js (pretty much every change just references a github issue).

Do you have any good starting point as to what issues were being caused? I'd hate to have to maintain a fork of this library because it's a fairly serious issue (and I can't imagine it's never caught anyone before, where a relation was loaded in two different ways).

Our use case right now is something like this:

1. User arrives at site, unauthenticated
2. We load a piece a general piece of information, that does not contain any Children, since they are private, called ParentA
3. User logs in
4. We load ParentB—private information—which has a relationship to ParentA, among other relationships. So we load ParentB->ParentA->Children.

I'm pretty sure if I tried to purge the ParentA object, it will break anything that has a reference to ParentA.

The only fix I can see is to manually hook into the load process for ParentA and manually extract the Children (which seems overly complicated, since we already have the _injectRelations function). That or don't use relations, which pretty much means I'm back to loading everything manually off the server.

[jmdobry]

The first thing I'd like to do for this issue is add tests for the use cases that have obviously been missed. I'll try to tackle the fix within the next day or two.

[OverZealous]

Ah, sorry I didn't reply sooner, I had already written a test to verify my hack.

[OverZealous]

And thank you for looking into this! We're rushing to get a product built, so I'm sorry if I sounded ungrateful.

[jmdobry]

I believe I have a working fix that does not rely on decorating items with angular-data-specific properties. Because angular-data uses identity-mapping, I can shift the injection of relations to the "pre-inject" phase and simply check the data store itself for items that have already been injected in the current injection process. I will push it shortly, though I'd like to hold off on tagging a new version of angular-data until you have a chance to test the fix with your code.

[jmdobry]

@OverZealous I tested the fix with your plunker and it worked, so I went ahead and created a release.

[OverZealous]

Looks like that worked for me, I really appreciate it!

FYI: there's a ton of console.logs left in that update. I noted a few on the commit. ;-)