datastore: fix runQuery end logic

[stephenplusplus]

Fixes #1200

To Dos

• Unit tests
• @pcostell review

[stephenplusplus]

@mcfarljw - feel free to try this branch out:

$ npm install --save stephenplusplus/gcloud-node

@pcostell - I'll need to think this through a little bit more, so no rush to review. However, if you catch anything I should be aware of, let me know!

[jgeewax]

Was this related to #1181 ?

[stephenplusplus]

@pcostell -- after we get NOT_FINISHED, we build a nextQuery and run it to get the rest of the results. Previously, we didn't update the limit, so if a user ran a query that had a limit of 500, and we got a NOT_FINISHED before reaching 500, the nextQuery we run would still have a limit of 500, thus perpetually racking up all of the entities in the datastore, effectively ignoring the limit.

This PR sets nextQuery.limit = originalLimit - numReceivedEntities-- this is only for the NOT_FINISHED responses when we run nextQuery automatically for the user. When we stop getting NOT_FINISHED, we restore nextQuery.limit = originalQuery.limit and return nextQuery to the user.

[stephenplusplus]

@jgeewax that was in reference our old logic, pre v1beta3: https://github.com/GoogleCloudPlatform/gcloud-node/blob/807a61641401e375c72d59b0535eaebe4f8a7e5b/lib/datastore/request.js#L644 -- which was changed when we released v1beta3: https://github.com/GoogleCloudPlatform/gcloud-node/blob/cd0c3e98505c5e29004d063b37a8605b7230a504/lib/datastore/request.js#L458 (discussion)

I thought it was "fixed" because we were no longer used the "entities.length" to determine if there were more results.

[jgeewax]

This seems weird to me.... (changing the nextQuery.limit value). Shouldn't the query cursor know what's up? and keep going until it hits 500?

[stephenplusplus]

I'll have to defer to @pcostell on that one.

[pcostell]

Ah sorry you're totally right, I should have seen this.

Yes you do have to track the limit (in the same way you have to track the offset).

@jgeewax -- the cursor is a static pointer, not just a page token. So, for example, you could use the cursor to issue the same query but backwards from that specific point.

[pcostell]

LGTM (Although I've been wrong before :-) )

[stephenplusplus]

:) Thanks for the review!

[stephenplusplus]

@callmehiphop ptal 👍

[callmehiphop]

@stephenplusplus could we benefit from an additional system test (or tests) here?

[stephenplusplus]

I think that's a good idea, but I don't know how practical it is, since we need the API to return "NOT_FINISHED", which I've only reproduced when I am querying for upwards of 500 entities. Since we don't create that many entities in our tests, I'm not sure we can create this condition in the system tests, and might have to use our unit tests to catch it.