add non-channel query iterator method

[kevina]

Continuation of #53

[whyrusleeping]

@Kubuxu could you help CR this one?

[whyrusleeping]

Overall LGTM, the 'having to switch back to legacy mode' kinda sucks. Maybe theres a better way of doing that? Not a huge deal though.

[whyrusleeping]

Maybe on conversion to legacy we swap out the next function in case the user calls NextSync again?

[kevina]

@whyrusleeping I confused, what are you getting at. If Next() is called and then NextSync() do you want the NextSync() method to go through legacyResults instead? Or do you want NextSync() to error out?

[whyrusleeping]

Lets just have NextSync go through the legacy method instead. That way we don't accidentally miss an entry thats trying to be sent on the legacy channel.

[kevina]

Okay will do.

[kevina]

@Kubuxu @whyrusleeping one thing I was unsure of is the handling of the automatic closing in NextSync(). Even if there is not a go process, there might still be something that needs to be closed, for example the leveldb iterator should probably be closed when done to help free up resources. Comments/reviews on this aspect will be appreciated. Also see ipfs/go-ds-leveldb#1 for some context.

[whyrusleeping]

@kevina I'm thinking we should require that the user always call Close on the query when they are done with it.

[kevina]

@whyrusleeping okay I cleaned up the logic of when legacyResults is used and when close() is called.

This still needs testing.

[whyrusleeping]

@kevina sounds good, ping me when you've added more tests then

[kevina]

@whyrusleeping Unit tests added.

I will do a manual integration test tomorrow, but other than that it should be good to go.

[kevina]

@kevina I'm thinking we should require that the user always call Close on the query when they are done with it.

From IRC:

kevina: whyrusleeping: in order to match the interface it seams NextSync() should call the close function if it is defined
whyrusleeping: okay, so if NextSync returns 'false' we assume things have been closed?
kevina: I think, that is what we have to do to match the interface.
kevina: In go-ds-leveldb i.Release() will be called when it has no more results to send in runQuery(), see https://github.com/ipfs/go-ds-leveldb/blob/a37289361537b8109b1a4ec3c7f194c5d2b39414/datastore.go#L141
kevina: when not using a channel the close function passes in will serve to call i.Release() see https://github.com/ipfs/go-ds-leveldb/blob/a37289361537b8109b1a4ec3c7f194c5d2b39414/datastore.go#L108
kevina: basically I think I figured this out :)
whyrusleeping: Okay, that makes sense

[whyrusleeping]

Is this assertion always safe?

[kevina]

Yes based on the code for ResultBuilder.Results() in the same file.

[kevina]

@whyrusleeping All tests pass when using this version with go-ipfs. I would call this good to go.

[whyrusleeping]

@kevina Cool, Would just like to have @Kubuxu and/or @lgierth review this first