goodeggs / angular-cached-resource

An AngularJS module to interact with RESTful resources, even when browser is offline
MIT License
216 stars 29 forks source link

CachedResource saves resolve to same object when called in $q.all #80

Closed tgemes closed 9 years ago

tgemes commented 9 years ago

I have an Accounts service which is an angular-cached-resource:

angular.module('accounts').factory('Accounts', ['$cachedResource',
    function($cachedResource) {
        return $cachedResource('accounts', 'accounts/:accountId', { accountId: '@_id'
        }, {
            update: {
                method: 'PUT'
            }
        });
    }
]);

My createAll function takes an array and tries to post each element to the remote REST api:

$scope.createAll = function (newAccounts) {
        var promises = newAccounts.map(function(newAccount) {
            var account = new Accounts(newAccount);
            return account.$save();
        });

        $q.all(promises).then(function(results) {
          results.forEach(function(result) {
            console.log(result);
            result.$promise.then(function(promise_result) {
              console.log(promise_result);
            });
          });
        });
    };

Below is the karma test with example data. I'm expecting two POST requests made to the REST API since there are two elements in the array.

        it('$scope.createAll() should send multiple posts to XHR', inject(function(Accounts) {
            var newAccounts = [
            {
                name: 'New Account'
            },
            {
                name: 'New Account 2'
            }];

            var sampleAccountResponse1 = new Accounts({
                _id: '525cf20451979dea2c000001',
                name: 'New Account'
            });

            var sampleAccountResponse2 = new Accounts({
                _id: '525cf20451979dea2c000002',
                name: 'New Account 2'
            });

            $httpBackend.expectPOST('accounts', newAccounts[0]).respond(sampleAccountResponse1);
            $httpBackend.expectPOST('accounts', newAccounts[1]).respond(sampleAccountResponse2);

            // Run controller functionality
            scope.createAll(newAccounts);
            $httpBackend.flush();
        }));

The issue is that I only get one Post request, and the promises on the Cached Resources resolve to the same object. Test results for the karma test:

LOG: CachedResource{name: 'New Account', $resolved: false, $promise: Object{then
: function (callback, errback, progressback) { ... }, catch: function (callback)
 { ... }, finally: function (callback) { ... }}}
LOG: CachedResource{name: 'New Account 2', $resolved: false, $promise: Object{th
en: function (callback, errback, progressback) { ... }, catch: function (callbac
k) { ... }, finally: function (callback) { ... }}}

LOG: CachedResource{name: 'New Account', $resolved: true, $promise: Object{then:
 function (callback, errback, progressback) { ... }, catch: function (callback)
{ ... }, finally: function (callback) { ... }}, _id: '525cf20451979dea2c000001'}
LOG: CachedResource{name: 'New Account', $resolved: true, $promise: Object{then:
 function (callback, errback, progressback) { ... }, catch: function (callback)
{ ... }, finally: function (callback) { ... }}, _id: '525cf20451979dea2c000001'}

PhantomJS 1.9.8 (Windows 7) Accounts Controller Tests $scope.createAll() should
send multiple posts to XHR FAILED
        Error: Unsatisfied requests: POST accounts

If I run the same test with simply posting through the $http service, it works.

$scope.createAll = function (newAccounts) {
        var promises = newAccounts.map(function(newAccount) {
            var account = new Accounts(newAccount);
            return $http({
             url   : 'accounts',
             method: 'POST',
             data  : account
            });
            //return account.$save();
        });

        $q.all(promises).then(function(results) {
          results.forEach(function(result) {
            console.log(result);
            //result.$promise.then(function(promise_result) {
            //  console.log(promise_result);
            //});
          });
        });
    };
LOG: Object{data: Object{_id: '525cf20451979dea2c000001', name: 'New Account', $
cache: true, toJSON: function () { ... }, $params: function () { ... }, $$addToC
ache: function (dirty) { ... }, $save: function () { ... }, $remove: function ()
 { ... }, $delete: function () { ... }, $update: function () { ... }}, status: 2
00, headers: function (name) { ... }, config: Object{method: 'POST', transformRe
quest: [...], transformResponse: [...], url: 'accounts', data: CachedResource{na
me: ...}, headers: Object{Accept: ..., Content-Type: ...}}, statusText: ''}

LOG: Object{data: Object{_id: '525cf20451979dea2c000002', name: 'New Account 2',
 $cache: true, toJSON: function () { ... }, $params: function () { ... }, $$addT
oCache: function (dirty) { ... }, $save: function () { ... }, $remove: function
() { ... }, $delete: function () { ... }, $update: function () { ... }}, status:
 200, headers: function (name) { ... }, config: Object{method: 'POST', transform
Request: [...], transformResponse: [...], url: 'accounts', data: CachedResource{
name: ...}, headers: Object{Accept: ..., Content-Type: ...}}, statusText: ''}

Please correct me if I'm using something wrong, I'm new to this module. I'd gladly do some more in-depth testing if needed. I'd hate to have to revert to $http, this module has been really great so far, thanks for the awesome work put into it.

simpixelated commented 9 years ago

I think you're required to specify an id property. From the readme:

*Note: Internally, $cachedResource keeps track of writes by bound params to ensure that it doesn't duplicate writes. If your bound param is null (say you're relying on the server to generate ids), then every write will replace the previous one. To avoid this undesirable scenario, simply ensure the id is set on the client, and you can safely ignore it on the server.

The examples all use id, but I see your code is using _id. Maybe that's the problem? This was just added about a month ago, so you could have easily missed it (https://github.com/goodeggs/angular-cached-resource/pull/78)

tgemes commented 9 years ago

Thanks for the suggestion, you were completely right. I needed to add the _id to the accounts and then they didn't resolve to the same resource anymore. I also had to change the map function to explicitly return the promise of the save action. It works perfectly now. Thanks.

var promises = newAccounts.map(function(newAccount) {
    newAccount._id = IdStore.nextId('Accounts');
    var account = new Accounts(newAccount);
    return account.$save().$promise;
});

Note: the IdStore is a client side helper service I made to make sure no two accounts get the same Id. It simply returns a new unique Id for the account.