$q.all() performing synchronously not asynchronously

Question

I'm developing an app to retrieve data from my API server, compare it to new data, and then push new data (if it doesn't exist) to the API server. Currently I'm working on the functionality to retrieve the existing data on the server using $http, promises and $q.all(). When I push all the promises to an array and then use $q.all(), it fires synchronously and doesn't wait for the promises to resolve.

I've already gone through and, to my knowledge, put in returns in all the operations I want to run asynchronously. I've also read many of the posts with similar problems and made those adjustments with no success.

This is the relevant code from the fileData service:

var apiPromise = function (url) {
    var deferred = $q.defer();
    $http.get(url)
        .then(function (result) {
            deferred.resolve(result.data);
            return;
        });
    return deferred.promise;
};

this.GetExistingStudentTest=function(studentid){
    return apiPromise('http://localhost:65196/api/PS/GetStudentTests?studentid=' + studentid + '&testid=2')
};

this.GetExistingStudentTestScores = function (testid) {
    return apiPromise('http://localhost:65196/api/PS/GetStudentTestScore?testid=' + testid)
};

and the code from the controller that is being used is:

$scope.previewUpload = function () {
    var promises = [];
    for (var i = 0; i < $scope.students.count; i++) {
        var student = $scope.students.students[i];
        promises.push(fileData.GetExistingStudentTest($scope.students.students[i].studentId)
            .then(function (data) {
                for (var j = 0; j < data.length; j++) {
                    var prevTest = {
                        testId: data[j].id,
                        studentId: student.studentId,
                        rawDate: data[j].test_date.substr(5, 2)+data[j].test_date.substr(2,2),
                        date: data[j].test_date,
                        testGradeLevel: data[j].grade_level,
                        scaleScores: {}
                    };
                    promises.push(fileData.GetExistingStudentTestScores(prevTest.testId)
                        .then(function (scoreData) {
                            console.log(scoreData);
                            for (var k = 0; k < scoreData.length; k++) {
                                switch (scoreData[k].testscoreid) {
                                    case "1":
                                        prevTest.scaleScores.english = scoreData[k].numscore;
                                        break;
                                    case "2":
                                        prevTest.scaleScores.math = scoreData[k].numscore;
                                        break;
                                    case "3":
                                        prevTest.scaleScores.reading = scoreData[k].numscore;
                                        break;
                                    case "4":
                                        prevTest.scaleScores.science = scoreData[k].numscore;
                                        break;
                                    case "5":
                                        prevTest.scaleScores.writing = scoreData[k].numscore;
                                        break;
                                    case "6":
                                        prevTest.scaleScores.composite = scoreData[k].numscore;
                                        break;
                                    case "451":
                                        prevTest.scaleScores.ELA = scoreData[k].numscore;
                                        break;
                                    case "452":
                                        prevTest.scaleScores.STEM = scoreData[k].numscore;
                                        break;
                                }
                            }
                            $scope.tests.previous.tests.push(prevTest);
                            $scope.tests.previous.count++;
                            return scoreData;
                        })
                    );
                }
                return data;
            })
            );

    }



    $q.all(promises).then(function () {
        console.log('Completed Test Retrieval');
        for (i = 0; i < $scope.tests.refined.count; i++) {
            console.log($scope.tests.refined.tests[i]);
            console.log($scope.tests.previous)
            for (j = 0; j < $scope.tests.previous.count; j++) {
                console.log($scope.tests.previous.tests[j]);
                if ($scope.tests.previous[j].studentId === $scope.tests.refined[i].studentId && $scope.tests.previous[j].rawDate === $scope.tests.refined[i].rawDate) {
                    console.log('Match');
                }
            }
        }

    });

    $scope.validateTests = true;
};

What I need to see is that the test scores return, the test is pushed to the proper array and then the $q.all() resolves to allow the new data and existing data to be compared. What is actually happening is as the inner promises are being resolved, $q.all() resolves and the nested for loop doesn't run because there are no values in the array.

This code is really hard to read. I suggest looking into a more functional approach to things. What's more, you are having your client do entirely too much. This would be made simpler if the client just sent its data, and let the server figure out what data needs to be updated. — Pytth, Jan 03 '19 at 18:44
@Pytth Agreed. That said, part of the functionality is to allow the user to determine what should be uploaded and this will only make suggestions on matches, not strictly rule out what is or isn't a match. Unfortunately, the data that goes in from outside sources is usually poorly cared for and there is no way to know what is a piece of duplicate data in every case. — Z. Campbell, Jan 03 '19 at 19:15
Those `for` loops are likely to have closure issues. See [JavaScript closure inside loops – simple practical example](https://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example/750506#750506). — georgeawg, Jan 03 '19 at 22:00
I had the same problem. I fixed it by using "new Promise()" before pushing the promise to the array. — mruanova, Jan 03 '19 at 22:01
The `apiPromise` function is a deferred anti-pattern that will hang the `$q.defer` if the API returns an error. See [Is this a “Deferred Antipattern”?](https://stackoverflow.com/questions/30750207/is-this-a-deferred-antipattern). — georgeawg, Jan 03 '19 at 22:02

score 2 · Accepted Answer · edited Jan 03 '19 at 22:04

First, I would start by getting rid of the defer, I don't think you need it. $http returns a promise so you can just that. Please read this article it is a game changer! https://www.codelord.net/2015/09/24/%24q-dot-defer-youre-doing-it-wrong/

var apiPromise = function (url) {
    return $http.get(url).then(function (result) {
       return result.data;
    });
};

Secondly, you shouldn't have code outside of the $q that executes on data that may not be resolved yet. You either need to nest that logic or write a function

// get all tests
var tests = students.map(function(s){
    return fileData.GetExistingStudentTest(s.studentId)
});

// wait for tests to resolve
$q.all(tests).then(function(resolvedTests){
   var transformedTests = // transform logic
   // get all test scores
   var testScores = transformedTests.map(function(t){
       return fileData.GetExistingStudentTestScores(t.testId);
   });
   // wait for test scores to resolve
   $q.all(testScores).then(function(resolvedTestScores){
      // you now have all the tests and test scores resolved...
      processTests(transformedTests,resolvedTestScores);
   });

});

This is MONEY! Thanks for your help. Never though to wrap the logic in the then function but I'm not sure why. — Z. Campbell, Jan 03 '19 at 22:18

$q.all() performing synchronously not asynchronously

1 Answers1