-1

I would like to iterate thru an array of students and make http call for each of them and parse the response and insert into mongodb, so I would like to do this for each student one by one untill all data are inserted then continue with the next one so that it would be better for the CPU and RAM Memory...

So far I am doing this, but this for some reason is not what I wanted...

var startDate = new Date("February 20, 2016 00:00:00");  //Start from February
var from = new Date(startDate).getTime() / 1000;
startDate.setDate(startDate.getDate() + 30);
var to = new Date(startDate).getTime() / 1000;

iterateThruAllStudents(from, to);

function iterateThruAllStudents(from, to) {
    Student.find({status: 'student'})
        .populate('user')
        .exec(function (err, students) {
            if (err) {
                throw err;
            }

            async.eachSeries(students, function iteratee(student, callback) {
                if (student.worksnap.user != null) {
                    var worksnapOptions = {
                        hostname: 'worksnaps.com',
                        path: '/api/projects/' + project_id + '/time_entries.xml?user_ids=' + student.worksnap.user.user_id + '&from_timestamp=' + from + '&to_timestamp=' + to,
                        headers: {
                            'Authorization': 'Basic xxx='
                        },
                        method: 'GET'
                    };

                    promisedRequest(worksnapOptions)
                        .then(function (response) { //callback invoked on deferred.resolve
                            parser.parseString(response, function (err, results) {
                                var json_string = JSON.stringify(results.time_entries);
                                var timeEntries = JSON.parse(json_string);
                                _.forEach(timeEntries, function (timeEntry) {
                                    _.forEach(timeEntry, function (item) {
                                        saveTimeEntry(item);
                                    });
                                });
                                callback(null);
                            });
                        }, function (newsError) { //callback invoked on deferred.reject
                            console.log(newsError);
                        });
                }
            });
        });
}

function saveTimeEntry(item) {
    Student.findOne({
            'worksnap.user.user_id': item.user_id[0]
        })
        .populate('user')
        .exec(function (err, student) {
            if (err) {
                throw err;
            }
            student.timeEntries.push(item);
            student.save(function (err) {
                if (err) {
                    console.log(err);
                } else {
                    console.log(Math.random());
                }
            });

        });
}

function promisedRequest(requestOptions) {
    //create a deferred object from Q
    process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0";
    var deferred = Q.defer();
    var req = http.request(requestOptions, function (response) {
        //set the response encoding to parse json string
        response.setEncoding('utf8');
        var responseData = '';
        //append data to responseData variable on the 'data' event emission
        response.on('data', function (data) {
            responseData += data;
        });
        //listen to the 'end' event
        response.on('end', function () {
            //resolve the deferred object with the response
            console.log('http call finished');
            deferred.resolve(responseData);
        });
    });

    //listen to the 'error' event
    req.on('error', function (err) {
        //if an error occurs reject the deferred
        deferred.reject(err);
    });
    req.end();
    //we are returning a promise object
    //if we returned the deferred object
    //deferred object reject and resolve could potentially be modified
    //violating the expected behavior of this function
    return deferred.promise;
}

It also seem that the saveEntry() inside .then is being called for all students at once, and that seems problematic.

I am new to Javascript especially when it comes to promises, callbacks... Anyone has any idea of achieving such thing...

Lulzim Fazlija
  • 865
  • 2
  • 16
  • 37
  • What is `parser.parseString()`? Is that a synchronous or asynchronous operation? – jfriend00 Apr 09 '16 at 17:37
  • 1
    Also, you should know that using `throw` inside an async callback does you absolutely no good unless it's inside a promise `.then()` handler. It just throws back into the bowels of an async operation and you can never catch it anywhere. Don't use throw in that case. – jfriend00 Apr 09 '16 at 17:38
  • Also, why`_.forEach(timeEntry)`? What is `timeEntry` at that point? – jfriend00 Apr 09 '16 at 18:11
  • @jfriend00 parser.parseString() I dont know if its async or sync , I had to use it to parse xml... seems async... do u have any idea if I can add then() into that so I can add saveTimeEntry() there? – Lulzim Fazlija Apr 10 '16 at 00:53

2 Answers2

0

Ok I did resolved this problem and if someone in future has this problem here is a solution I came of.

    var startDate = new Date("February 20, 2016 00:00:00");  //Start from February
var from = new Date(startDate).getTime() / 1000;
startDate.setDate(startDate.getDate() + 30);
var to = new Date(startDate).getTime() / 1000;

iterateThruAllStudents(from, to);

function iterateThruAllStudents(from, to) {
    Student.find({status: 'student'})
        .populate('user')
        .exec(function (err, students) {
            if (err) {
                throw err;
            }

            var counter = 1;
            async.eachSeries(students, function iteratee(student, callback) {
                counter++;
                if (student.worksnap.user != null) {
                    console.log('');
                    console.log('--------------');
                    console.log(student.worksnap.user.user_id);
                    var worksnapOptions = {
                        hostname: 'worksnaps.com',
                        path: '/api/projects/' + project_id + '/time_entries.xml?user_ids=' + student.worksnap.user.user_id + '&from_timestamp=' + from + '&to_timestamp=' + to,
                        headers: {
                            'Authorization': 'Basic xxxxx'
                        },
                        method: 'GET'
                    };

                    promisedRequest(worksnapOptions)
                        .then(function (response) { //callback invoked on deferred.resolve
                            parser.parseString(response, function (err, results) {
                                var json_string = JSON.stringify(results.time_entries);
                                var timeEntries = JSON.parse(json_string);
                                var isEmpty = _.isEmpty(timeEntries); // true
                                if (isEmpty) {
                                    callback(null);
                                }
                                saveTimeEntry(timeEntries).then(function (response) {
                                    console.log('all timeEntries for one student finished....Student: ' + student.worksnap.user.user_id + ' Student Counter: ' + counter);
                                    callback(null);
                                });
                            });
                        }, function (newsError) { //callback invoked on deferred.reject
                            console.log(newsError);
                        });
                } else {
                    callback(null);
                }
            });
        });
}

function saveTimeEntry(timeEntries) {
    var deferred = Q.defer();
    _.forEach(timeEntries, function (timeEntry) {
        _.forEach(timeEntry, function (item) {
            Student.findOne({
                    'worksnap.user.user_id': item.user_id[0]
                })
                .populate('user')
                .exec(function (err, student) {
                    if (err) {
                        //throw err;
                        console.log(err);
                    }
                    student.timeEntries.push(item);
                    student.save(function (err) {
                        if (err) {
                            console.log(err);
                            deferred.reject(err);
                        } else {
                            //console.log(Math.random());
                        }
                    });

                });
        });
        deferred.resolve('finished saving timeEntries for one student...');
    });

    return deferred.promise;
}

function promisedRequest(requestOptions) {
    //create a deferred object from Q
    process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0";
    var deferred = Q.defer();
    var req = http.request(requestOptions, function (response) {
        //set the response encoding to parse json string
        response.setEncoding('utf8');
        var responseData = '';
        //append data to responseData variable on the 'data' event emission
        response.on('data', function (data) {
            responseData += data;
        });
        //listen to the 'end' event
        response.on('end', function () {
            //resolve the deferred object with the response
            console.log('http call finished');
            deferred.resolve(responseData);
        });
    });

    //listen to the 'error' event
    req.on('error', function (err) {
        //if an error occurs reject the deferred
        console.log('inside On error.');
        console.log(err);
        deferred.reject(err);
    });
    req.end();
    //we are returning a promise object
    //if we returned the deferred object
    //deferred object reject and resolve could potentially be modified
    //violating the expected behavior of this function
    return deferred.promise;
}
Lulzim Fazlija
  • 865
  • 2
  • 16
  • 37
  • You are missing lots of error handling to detect and handle errors properly. Mixing regular callbacks and promises is pretty much never a good idea. A good design here would be to switch all your async operations over to promises and then write all the async logic using promises. I was going to do that for you, but you did not answer the questions I asked in comments so I can't. – jfriend00 Apr 09 '16 at 19:19
  • @jfriend00, If you have a better solution, please do post so I can accept it as best answer... This was just a solution that worked for me but not as clean as it should be done.... – Lulzim Fazlija Apr 09 '16 at 22:49
  • I left multiple comments asking you questions that I need to know the answers to in order to offer you a better answer. I tried, but ran into missing info in order to create an answer. I don't understand why you don't answer my questions. – jfriend00 Apr 09 '16 at 23:03
  • I didnt know what to answer regarding this Why _.forEach(timeEntry), What did you mean by this? I used this for iterating thru time entries...In fact I added maybe an extra foreach, and u r right for that, I just had to add it, it didnt work only with the first foreach and Idk why.. – Lulzim Fazlija Apr 10 '16 at 00:43
  • Programming in the dark without knowing what a `timeEntry` is will just lead you to trouble. If you don't know what it is, then either do a `console.log(timeEntry)` to see exactly what it is or break on it in the debugger and examine it so you can write the appropriate code for the data you have. Guessing when programming is a horrible idea. – jfriend00 Apr 10 '16 at 00:59
  • ok that's an array inside object timeEntries, I guess I better do one foreach like timeEntries.time_entry? let me check this – Lulzim Fazlija Apr 10 '16 at 01:06
  • I know you r seeing lot's of things that you can improve on this code, why don't u just write a solution and let's everybody see it too :) – Lulzim Fazlija Apr 10 '16 at 01:07
  • ok I did it with only one foreach: _.forEach(timeEntries.time_entry, function (item) {....} and it works... idk why i was doing that either :s... sometimes I just wanna write more code... stupid me – Lulzim Fazlija Apr 10 '16 at 01:11
0

First off, multiple nested operations are going to be a lot easier to code and handle errors reliably if you use promises for all async operations. That means learning how to use the promises that are built into your database (I assume you're using mongoose) and then wrapping any other async operations to use promises. Here are a couple links on using promises with mongoose:

Switching to use promises in Mongoose

Plugging in your own promise library into Mongoose

So, looking at the Mongoose documentation, you can see that .exec() and .save() already return promises so we can use them directly.

Adding this line of code will tell Mongoose to use Q promises (since that's the promise library you show):

// tell mongoose to use Q promises
mongoose.Promise = require('q').Promise;

Then, you need to promisify some operations that don't use promises such as the parsing step:

function parse(r) {
    var deferred = Q.defer();
    parser.parseString(r, function(err, results) {
        if (err) {
            deferred.reject(err);
        } else {
            deferred.resolve(results);
        }
    });
    return deferred.promise;
}

saveTimeEntry() can easily be written to return a promise, just using the promise support already in your database:

function saveTimeEntry(item) {
    return Student.findOne({'worksnap.user.user_id': item.user_id[0]}).populate('user').exec().then(function(student) {
        student.timeEntries.push(item);
        return student.save();
    });
}

So, now you have all the right pieces to rewrite the main logic using promises. The key thing to remember here is that if you return a promise from a .then() handler, it chains that promise onto the parent promise. We will use that a lot in your processing. Further, a common design pattern for sequencing promises that are iterating through an array is to use array.reduce() like this:

return array.reduce(function(p, item) {
    return p.then(function() {
         return someAsyncPromiseOperation(item);
    });
}, Q());

We will use that structure in a couple of places to rewrite the core logic using promises:

// tell mongoose to use Q promises
mongoose.Promise = require('q').Promise;

iterateThruAllStudents(from, to).then(function() {
    // done successfully here
}, function(err) {
    // error occurred here
});

function iterateThruAllStudents(from, to, callback) {
    return Student.find({status: 'student'}).populate('user').exec().then(function (students) {
        // iterate through the students array sequentially
        students.reduce(function(p, student) {
            return p.then(function() {
                if (student.worksnap.user != null) {
                    var worksnapOptions = {
                        hostname: 'worksnaps.com',
                        path: '/api/projects/' + project_id + '/time_entries.xml?user_ids=' + student.worksnap.user.user_id + 
                              '&from_timestamp=' + from + '&to_timestamp=' + to,
                        headers: {'Authorization': 'Basic xxx='},
                        method: 'GET'
                    };

                    return promisedRequest(worksnapOptions).then(function(response) {
                        return parse(response).then(function(results) {
                            // assuming results.time_entries is an array
                            return results.time_entries.reduce(function(p, item) {
                                return p.then(function() {
                                    return saveTimeEntry(item);
                                });
                            }, Q());
                        });
                    });
                }
            });
        }, Q())
    });
}

function parse(r) {
    var deferred = Q.defer();
    parser.parseString(r, function(err, results) {
        if (err) {
            deferred.reject(err);
        } else {
            deferred.resolve(results);
        }
    });
    return deferred.promise;
}

function saveTimeEntry(item) {
    return Student.findOne({'worksnap.user.user_id': item.user_id[0]}).populate('user').exec().then(function(student) {
        student.timeEntries.push(item);
        return student.save();
    });
}

function promisedRequest(requestOptions) {
    //create a deferred object from Q
    process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0";
    var deferred = Q.defer();
    var req = http.request(requestOptions, function (response) {
        //set the response encoding to parse json string
        response.setEncoding('utf8');
        var responseData = '';
        //append data to responseData variable on the 'data' event emission
        response.on('data', function (data) {
            responseData += data;
        });
        //listen to the 'end' event
        response.on('end', function () {
            //resolve the deferred object with the response
            console.log('http call finished');
            deferred.resolve(responseData);
        });
    });

    //listen to the 'error' event
    req.on('error', function (err) {
        //if an error occurs reject the deferred
        deferred.reject(err);
    });
    req.end();
    return deferred.promise;
}

One thing this offers that your code does not is that ANY error that occurs will percolate all the way back to the returned promise from iterateThruAllStudents() so no errors are hidden.


And, then cleaning it up some to reduce nested indentation and adding a useful utility function, it looks like this:

// tell mongoose to use Q promises
mongoose.Promise = require('q').Promise;

// create utility function for promise sequencing through an array
function sequence(array, iterator) {
    return array.reduce(function(p, item) {
        return p.then(function() {
            return iterator(item);
        });
    }, Q());
}

iterateThruAllStudents(from, to).then(function() {
    // done successfully here
}, function(err) {
    // error occurred here
});

function iterateThruAllStudents(from, to, callback) {
    return Student.find({status: 'student'}).populate('user').exec().then(function (students) {
        // iterate through the students array sequentially
        return sequence(students, function(item) {
            if (student.worksnap.user != null) {
                var worksnapOptions = {
                    hostname: 'worksnaps.com',
                    path: '/api/projects/' + project_id + '/time_entries.xml?user_ids=' + student.worksnap.user.user_id + 
                          '&from_timestamp=' + from + '&to_timestamp=' + to,
                    headers: {'Authorization': 'Basic xxx='},
                    method: 'GET'
                };
                return promisedRequest(worksnapOptions).then(function(response) {
                    return parse(response);
                }).then(function(results) {
                    // assuming results.time_entries is an array
                    return sequence(results.time_entries, saveTimeEntry);
                });
            }
        });
    });
}

function parse(r) {
    var deferred = Q.defer();
    parser.parseString(r, function(err, results) {
        if (err) {
            deferred.reject(err);
        } else {
            deferred.resolve(results);
        }
    });
    return deferred.promise;
}

function saveTimeEntry(item) {
    return Student.findOne({'worksnap.user.user_id': item.user_id[0]}).populate('user').exec().then(function(student) {
        student.timeEntries.push(item);
        return student.save();
    });
}

function promisedRequest(requestOptions) {
    //create a deferred object from Q
    process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0";
    var deferred = Q.defer();
    var req = http.request(requestOptions, function (response) {
        //set the response encoding to parse json string
        response.setEncoding('utf8');
        var responseData = '';
        //append data to responseData variable on the 'data' event emission
        response.on('data', function (data) {
            responseData += data;
        });
        //listen to the 'end' event
        response.on('end', function () {
            //resolve the deferred object with the response
            console.log('http call finished');
            deferred.resolve(responseData);
        });
    });

    //listen to the 'error' event
    req.on('error', function (err) {
        //if an error occurs reject the deferred
        deferred.reject(err);
    });
    req.end();
    //we are returning a promise object
    //if we returned the deferred object
    //deferred object reject and resolve could potentially be modified
    //violating the expected behavior of this function
    return deferred.promise;
}

Of course, this is a lot of code and I have no way to test it and I've never written Mongoose code before, so there very well could be some goofs in here, but hopefully you can see the general idea and work through any mistakes I might have made.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Well, this is indeed way too better then I did... I really appreciate your effort and time you have given to look at this problem. I hope it helped everyone else too except me... I did change my code like urs and it seems working all good :) – Lulzim Fazlija Apr 10 '16 at 16:50