CODEWITHSUNDEEP
javascriptnode.jspromiseq
Ryan
Ryan

Reputation: 343

return q.all() not waiting to resolve unless wrapped in deferred.resolve

So I realized today I've been iterating through promises badly by using recursion and nested promises and wanted to learn how to use Q.all() correctly. I am trying to iterate through a series of async functions and waiting for all of them to resolve before continuing. In this current implementation, Q.all is returned immediately without waiting for the promises to resolve. Here is my code for the iterating function

var updateNewReleasePlaylists = function () {
    var self = this;
    var promises = [];
    var deferred = Q.defer();

    // query for all users who have new releases
    User.find({
            $and: [{
                    'new_releases': {
                        $exists: true,
                        $ne: []
                    }
                },
                {
                    'refresh_token': {
                        $exists: true
                    }
                }
            ]
        }, 'new_releases',
        function (err, users) {
            if (err) {
                throw new Error(err);
            }
            for (var i = 0; i < users.length; i++) {
                console.log('when?')
                promises.push(updatePlaylist(users[i]));
            }
        });
return Q.all(promises);

}

and the here is the async function it is calling: 

function updatePlaylist(user) {
    var deferred = Q.defer();
    User.findOne({'_id': user._id}, function(err, user) {
        if (err) {
            deferred.reject(err);
        }
        console.log(user);
        deferred.resolve();
    })
    return deferred.promise;
}

If I change the implementation like so it works totally fine:

var updateNewReleasePlaylists = function () {
    var self = this;
    var promises = [];
    var deferred = Q.defer();

    // query for all users who have new releases
    User.find({
            $and: [{
                    'new_releases': {
                        $exists: true,
                        $ne: []
                    }
                },
                {
                    'refresh_token': {
                        $exists: true
                    }
                }
            ]
        }, 'new_releases',
        function (err, users) {
            if (err) {
                throw new Error(err);
            }
            for (var i = 0; i < users.length; i++) {
                console.log('when?')
                promises.push(updatePlaylist(users[i]));
            }
            deferred.resolve(Q.all(promises)); // DIFFERENT
        });
        return deferred.promise; // DIFFERENT
}

From what I can tell, this is an incorrect way of implementing this however and I would like to get it right the first time. If it's any help, here is where I'm calling the updateNewReleasePlaylists function for testing. 

it('updateNewReleasePlaylists should properly resolve after all users playlists have been updated', function (done) {
        this.timeout(60000);
        testHelper.stageSpotifyUser(20)
            .then(testHelper.stageSpotifyUser(20))
            .then(testHelper.stageSpotifyUser(20))
            .then(function () {
                playlist.updateNewReleasePlaylists()
                    .then(function (promises) {
                        console.log(promises.length);
                        console.log('should be after');
                        done();
                    })
            })
    })

Thanks for your help ahead of time.

Upvotes: 0

Views: 704

Answers (1)

JohnPan
JohnPan

Reputation: 1210

If the promises array is empty then it seems right to get your return, because there is nothing to wait for. When find() is completed, due to its async nature, promises array is still empty, since the function (err, users) has not been invoked yet.

So, if I am right about find() being async, you should return the whole User.find() or promise-wrap if needed and return that.

Upvotes: 1

Related Questions