amark / mongous

Simple MongoDB driver for Node.js with jQuery like syntax.
MIT License
246 stars 30 forks source link

find method when there are more than 500 records #18

Closed acsaga closed 12 years ago

acsaga commented 12 years ago

find method will trigger the callback function multiple times if there are more than 500 records. I think it's a bug.

in find:

     if (msg.more && o.lim == 0) {
        //lim = o.lim - it < 500 ? 500 : o.lim - it;
        this.more(cmd.collectionName, 500, msg.cursorID, id);
      } else {
        con.c.removeListener(id.toString(), con.c.listeners(id.toString())[0]);
      }
      if (fn) {
        return fn(msg);
      }

I think it should be:

     if (msg.more && o.lim == 0) {
        //lim = o.lim - it < 500 ? 500 : o.lim - it;
        this.more(cmd.collectionName, 500, msg.cursorID, id);
      } else {
        con.c.removeListener(id.toString(), con.c.listeners(id.toString())[0]);
        if (fn) { /* move the callback inside the 'else' */
          return fn(msg);
        }
      }

how do you think about it?

acsaga commented 12 years ago

I just figure out it's an intend behavior and you already had clarified it in #issue 15.

so I will close this issue, sorry!

amark commented 12 years ago

Yes, it is "streaming" the response in chunks. Thanks for closing it. I am happy you are reading / understanding the code, though. Feel free to modify it if you want, just create an array that all the responses get merged into, then return it (as you noted) inside the else statement - this will allow you to receive everything all at once in a single callback. Understand there are caveats though, such as it being less safe.