0

I have the following code:

var method = PushLoop.prototype;
var agent = require('./_header')  
var request = require('request');
var User = require('../models/user_model.js');
var Message = require('../models/message_model.js');
var async = require('async')

function PushLoop() {};

    method.startPushLoop = function() {

     getUserList()

    function getUserList() {        

        User.find({}, function(err, users) {            
            if (err) throw err;
            if (users.length > 0) {              
                 getUserMessages(users) 
            } else {              
                setTimeout(getUserList, 3000)
            }                   
        });
    }

    function getUserMessages(users) {
         // console.log("getUserMessages")
         async.eachSeries(users, function (user, callback) {

              var params = {
                  email: user.email,
                pwd: user.password,
                token: user.device_token
              }                       
                  messageRequest(params)                             
              callback(); 
            }, function (err) {
              if (err) {
                console.log(err)
                    setTimeout(getUserList, 3000)                
              }               
            });
    }

    function messageRequest(params) {

            var url = "https://voip.ms/api/v1/rest.php?api_username="+ params.email +"&api_password="+ params.pwd +"&method=getSMS&type=1&limit=5"                                      
            request(url, function(err, response, body){ 

                if (!err) {

                    var responseObject = JSON.parse(body);                                          
                var messages = responseObject.sms   

                    if (responseObject["status"] == "success")  {                                                                                                               
                        async.eachSeries(messages, function(message, callback){     
                          console.log(params.token)         
                            saveMessage(message, params.token)
                            callback();

                        }, function(err) {
                            if (err) {
                                console.log(err)
                            }
                            // setTimeout(getUserList, 3000)                            
                        })
                    } else {
                        // setTimeout(getUserList, 3000)    
                    }
                } else {
                    console.log(err)
                    // setTimeout(getUserList, 3000)
                }           

            });
            setTimeout(getUserList, 3000)
    }

    function saveMessage(message, token) {
         // { $and: [ { price: { $ne: 1.99 } }, { price: { $exists: true } }
        // Message.find({ $and: [{ message_id: message.id}, {device_token: token}]}, function (err, doc){             
            Message.findOne({message_id: message.id}, function (err, doc){

          if (!doc) {                    
                  console.log('emtpy today')                
                    var m = new Message({
                          message_id: message.id, 
                          did: message.did,
                          contact: message.contact, 
                          message: message.message,
                          date: message.date,
                          created_at: new Date().toLocaleString(),
                          updated_at: new Date().toLocaleString(),
                          device_token: token    
                        });             
                      m.save(function(e) {
                            if (e) {                                
                                console.log(e)
                            }   else {
                                 agent.createMessage()               
                                  .device(token)
                                  .alert(message.message)
                                  .set('contact', message.contact)
                                  .set('did', message.did)
                                  .set('id', message.id)
                                  .set('date', message.date)
                                  .set('message', message.message)                    
                                  .send();                                
                            }                                       
                        });       
          }                                                     
        }) //.limit(1); 
    }

};

module.exports = PushLoop;

Which actually works perfectly fine in my development environment - However in production (i'm using Openshift) the mongo documents get saved in an endless loop so it looks like the (if (!doc)) condition always return true therefore the document gets created each time. Not sure if this could be a mongoose issue - I also tried the "find" method instead of "findOne". My dev env has node 0.12.7 and Openshift has 0.10.x - this could be the issue, and i'm still investigating - but if anybody can spot an error I cannot see in my logic/code please let me know

thanks!

fredp613
  • 107
  • 1
  • 8
  • The usage of `setTimeout` here is pretty horrible. You already have the `async` library imported, so use it for the flow control. Also using `.findOne()` and `.save()` is a horrible pattern. You should be applying `.update()` instead with atomic operators. But main point here is if you "presume" things are complete after a set time, then you are just asking for trouble. Refactor to respect each callback. – Blakes Seven Sep 02 '15 at 02:40
  • What i'm trying to accomplish here is to poll the third party messaging server every few seconds for new messages. The server usually returns the last 10 messages and I store them in my mongo instance. So the purpose of the find query is to find out if I already saved that message, if not, then save it to mongo. So are you saying that if I use update() instead of save() mongo will save the new document? thanks for your reply btw – fredp613 Sep 02 '15 at 02:50
  • 1
    What I am saying is you should read up on the ["upsert"](http://docs.mongodb.org/manual/reference/method/db.collection.update/#upsert-option) option as "find or create" is basically what it does. Also even if "polling" you should not be doing so until you know that all updates in the cycle from the response are complete. If not you run the risk at the very least of building up queued requests and possibly even overwriting data. But proper research on `.update()` and "upserts" and associated operators should help with that. Your code needs some serious refactoring. – Blakes Seven Sep 02 '15 at 02:57
  • ok thanks, so by "all updates in the cycle from the response are complete", I thought by getting a response from the server, in this case checking for a "success" response from the server would infer that the cycle is complete? – fredp613 Sep 02 '15 at 03:06

1 Answers1

0

I solved this issue by using a "series" like pattern and using the shift method on the users array. The mongoose upsert findOneOrCreate is good however if there is a found document, the document is returned, if one isn't found and therefore created, it's also returned. Therefore I could not distinguish between the newly insert doc vs. a found doc, so used the same findOne function which returns null if no doc is found I just create it and send the push notification. Still abit ugly, and I know I could have used promises or the async lib, might refactor in the future. This works for now

function PushLoop() {};

    var results = [];   
    method.go = function() {
        var userArr = [];
        startLoop()

        function startLoop() {  

        User.find({},function(err, users) {         
            if (err) throw err;
            users.forEach(function(u) {             
                userArr.push(u)                         
            })                      
            function async(arg, callback) {

                var url = "https://voip.ms/api/v1/rest.php?api_username="+ arg.email +"&api_password="+ arg.password +"&method=getSMS&type=1&limit=5"                                       

                    request.get(url, {timeout: 30000}, function(err, response, body){ 
                        if (!err) {                                         
                            var responseObject = JSON.parse(body);                                          
                        var messages = responseObject.sms   
                        var status = responseObject.status
                        if (status === "success") {


                        messages.forEach(function(m) {

                            var message = new Message({
                                      message_id: m.id, 
                                      did: m.did,
                                      contact: m.contact, 
                                      message: m.message,
                                      date: m.date,
                                      created_at: new Date().toLocaleString(),
                                      updated_at: new Date().toLocaleString(),
                                      device_token: arg.device_token     
                                    });                             
                                    var query = { $and : [{message_id: m.id}, {device_token: arg.device_token}] }
                                    var query1 = { message_id: m.id }


                                    Message.findOne(query).lean().exec(function (err, doc){

                                             if (!doc || doc == null) {                                              
                                                            message.save(function(e) {
                                                                console.log("message saved")
                                                                if (e) {
                                                                    console.log("there is an error")
                                                                    console.log(e)
                                                                } else {

                                                                    console.log(message.device_token)
                                                                        var messageStringCleaned = message.message.toString().replace(/\\/g,"");                                                                                                                                                                            

                                                                        var payload = {
                                                                            "contact" : message.contact,
                                                                            "did" : message.did,
                                                                            "id" : message.message_id,
                                                                            "date" : message.date,
                                                                            "message" : messageStringCleaned
                                                                        }                                                                       

                                                                    var note = new apns.Notification();
                                                                    var myDevice = new apns.Device(message.device_token);
                                                                    note.expiry = Math.floor(Date.now() / 1000) + 3600; // Expires 1 hour from now.
                                                                    note.badge = 3;                                                                 
                                                                    note.alert = messageStringCleaned;
                                                                    note.payload = payload;
                                                                    apnsConnection.pushNotification(note, myDevice);                                                                    
                                                            }
                                                            })                                                                                                                                                                                    
                                                }                                                                                                                                                               
                                            }); 
                                    });                     
                        }
                          else {
                            console.log(err)
                          }
                        }                   
                    }); 

              setTimeout(function() {             
                callback(arg + "testing 12"); 
              }, 1000);
            }
            // Final task (same in all the examples)            
            function series(item) {
                  if(item) {
                    async( item, function(result) {                                             
                      results.push(result);       
                      return series(userArr.shift());
                    });
                  } else {
                    return final();
                  }
            }
            function final() { 
                console.log('Done'); 
                startLoop();
            }

            series(userArr.shift())
        });
    }
}


module.exports = PushLoop;
fredp613
  • 107
  • 1
  • 8