stephanenicolas / robospice

Repo of the Open Source Android library : RoboSpice. RoboSpice is a modular android library that makes writing asynchronous long running tasks easy. It is specialized in network requests, supports caching and offers REST requests out-of-the box using extension modules.
Apache License 2.0
2.95k stars 545 forks source link

How can I wait until all requests finish in SpiceManager queue? #308

Open geovanisouza92 opened 10 years ago

geovanisouza92 commented 10 years ago

Hello guys,

I started this discussion in Google Groups, and after my recent tests, I have a suggestion to reach this behavior.

I pulled source code and integrated with my Android Studio project (otherwise, I could make the pull request myself, sorry), and changed somethings.

The behavior is exactly what I desire: each requests is processed completely before another starts. But, in case that some requests throws an unhandled exception, the manager loose control and some requests "escape" in Thread realm.

I let this issue here to document the solution and suggest to be integrated with source code.

Thank you.

stephanenicolas commented 10 years ago

Hi @geovanisouza92 , I close this issue as the discussion on the group is satisfying IMHO, and the workaround you submitted doesn't make much sense. Multi threading is a core feature in RS, we worked quite hard to make a robust multi thread approach to boost performances in real life apps through concurrent request processing.

Alfaifi commented 10 years ago

I think this is very much needed,

mainly when you want to show a progress dialog for multiple requests, you need to remove the progress once all requests are finished, it would be nice if I could just do one method call in SpiceManager,

for example spiceManager.isQueueEmpty()

I know you tagged this as wontfix! but I think it is a must :) a workaround for this would be to use a countdown latch, but i dont like that way :(

softwaremaverick commented 10 years ago

I wouldn't want to see this either as if i remember rightly the spice service listener now supports detecting when all requests are finished anyway doesn't it?

Then implement something from that maybe

On 3 September 2014 17:44:29 BST, Muhammad Alfaifi notifications@github.com wrote:

I think this is very much needed,

mainly when you want to show a progress dialog for multiple requests, you need to remove the progress once all requests are finished, it would be nice if I could just do one method call in SpiceManager,

for example spiceManager.isQueueEmpty()

I know you tagged this as wontfix! but I think it is a must :) a workaround for this would be to use a countdown latch, but i dont like that way :(


Reply to this email directly or view it on GitHub: https://github.com/stephanenicolas/robospice/issues/308#issuecomment-54326775

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Alfaifi commented 10 years ago

@softwaremaverick I'm not sure it does, can you elaborate please ? If you are talking about spiceManager.getPendingRequestCount() sadly it doesn't work in the intended way, see This stack overflow post.

softwaremaverick commented 10 years ago

I'll need to look at the code to see what the state is at the moment.

One thing i did want to add at one point which may have been useful to you is the ability to add additional custom commands in the spice manager and spice service.

I believe last time i looked there was a private method that needed to be made protected for this to work.

On 4 September 2014 05:19:04 BST, Muhammad Alfaifi notifications@github.com wrote:

@softwaremaverick I'm not sure it does, can you elaborate please ? If you are talking about spiceManager.getPendingRequestCount() see here as this approuch doesn't work. see here Stack Overflow


Reply to this email directly or view it on GitHub: https://github.com/stephanenicolas/robospice/issues/308#issuecomment-54404919

Sent from my Android device with K-9 Mail. Please excuse my brevity.

stephanenicolas commented 10 years ago

Hi all,

I am pretty busy at the moment, but I would thankfully accept a PR (+tests).

Stéphane

stephanenicolas commented 10 years ago

@softwaremaverick please do contribute if you feel like it. I re-open the issue to keep track of it.

softwaremaverick commented 10 years ago

Hi, the method I was originally thinking about appears to already be protected.

protected <T> Future<T> executeCommand(SpiceManagerCommand<T> spiceManagerCommand);

Looking at the git history it seems like I merged in the changes with request observers.

commit 7180210376bb0f9fd689a2acba7068479e5b291b Author: Andrew Clark Andrew.Clark@bjss.com 2013-07-22 13:51:52 Committer: Stéphane Nicolas steff.nicolas@gmail.com 2013-09-01 10:02:23 Parent: 32b2d38e730fde22f616acc0b00fc447705995b3 (Merge of https://github.com/octo-online/robospice/pull/185) Child: fc4019bde6066251ffb83a6b77e78c0628f2b793 (Formatting) Branches: addsynchronization, master, nonetwork-noretry, nonetwork-retryifblip, origin/addsynchronization, origin/master, origin/nonetwork-noretry, origin/nonetwork-retryifblip, upstream/1.4.14, upstream/PR259, upstream/fast-test, upstream/master, upstream/release, upstream/scroll-listener

Implement Request Observers and Request Tracker

softwaremaverick commented 10 years ago

As such, if anyone wanted to write a custom command that was fired off from the SpiceManager to the SpiceService they could simply extend SpiceManager, add their command as a method and make it call executeCommand.

Further investigation reveals the

protected RequestProcessorListener createRequestProcessorListener() { return new SelfStopperRequestProcessorListener(); ` }

where the SelfStopperRequestProcessorListener implements RequestProcessorListener

which itself added allRequestComplete by yours truly.

As such, overriding createRequestProcessorListener with a custom implementation which still calls the SelfStopperRequestProcessorListener might be a workable solution.

The more things that are override-able the better in my opinion and there seems to be a fair few sockets to plug your code into right now which I like.

softwaremaverick commented 10 years ago

Another option which I used previously and may or may not be useful for you is to implement your own version of RequestListenerNotifier which potentially sends more information to the listeners like request details if you were needing enhanced progress updates.

softwaremaverick commented 10 years ago

Actually checking git history I added the requestsInProgress() in RequestProcessorListener not allRequestComplete

stephanenicolas commented 10 years ago

Hi @softwaremaverick. Feel free to contribute again to RS if you need any change.

One thing I wanted to implement would be to pass the request execution into the command execution which is cleaner...

S. Le 2014-09-06 13:19, "softwaremaverick" notifications@github.com a écrit :

Actually checking git history I added the requestsInProgress() in RequestProcessorListener not allRequestComplete

;-)

— Reply to this email directly or view it on GitHub https://github.com/stephanenicolas/robospice/issues/308#issuecomment-54721222 .