parse-community / Parse-SDK-Android

The Android SDK for Parse Platform
https://parseplatform.org/
Other
1.89k stars 735 forks source link

Cancel login, register, and reset password tasks #695

Open addisonElliott opened 7 years ago

addisonElliott commented 7 years ago

I've been searching throughout the documentation and code for methods of cancelling the login, register and reset password tasks.

The functions are logInInBackground, signUpInBackground, and requestPasswordResetInBackground.

ParseQuery stores each of the TaskCompletionSource objects and cancels that when you call cancel. However, is this possible with the user functions?

I need this to cancel the functions if the user changes screen orientation. If the callback occurs, it will update the old activity instead of the newly created one.

natario1 commented 7 years ago

It's possible to add cancellation tokens and let you cancel functions, but this gets very broad once you start thinking about it.

First of all the legacy policy of the SDK seems to be to not allow cancellations, in order to keep the developer experience "easy" (the only exception being ParseQuery.cancel()). There are, however, TODOs here and there that suggest CancellationToken to be passed around.

Second, the CancellationToken approach is bolts-dependent, and I am not sure we are wishing to keep going down that path. There was some discussion about moving to a different async engine or at least trying to abstract our components in order to plug in different engines.

Third, canceling a log-in is not very clear to me. What do you do once the request has hit the server?

Fourth, and this is just my opinion, the whole doInBackground(callback) thing is great in pure Java, but kind of breaks when dealing with Android specific stuff like config changes. The only safe async callback in Android is the one started with startActivityForResult(), like the Facebook SDK works, or using services.

We should really think about this sooner or later, IMO.

addisonElliott commented 7 years ago

You bring up some interesting points. I have an application that could greatly benefit from being able to cancel these queries, especially when the user goes to a new application. With that in mind, I would like to contribute and submit a pull request for cancellation.

I was unaware that there was discussion of switching to a different async engine. Can you name a few other alternative async engines? I am somewhat new to Android/Java and upon some basic searching, I did not find many options. Maybe I wasnt searching with the right terms though.

If switching to a new async engine or abstracting out the async engine is desired, then I would be glad to try and work on this pull request. I definitely think that there may be some benefits to using a more popular async engine.

With your fourth point, why do you believe doInBackground is not compatible with Android? In your opinion, what is the correct method of handling asynchronous tasks?

My assumption is that you think this because the async callbacks are usually created as inner classes that reference the parent class, which is typically the activity. If the parent class is destroyed before the async callback is finished, then an exception is thrown. If that is what you mean, then listeners seem to be a standard of Android and I think the appropriate response is to remove/cancel the callback when the activity is destroyed.

addisonElliott commented 7 years ago

Oh yeah, to your third point, it can be a bit confusing to cancel a login request or sign up request.

Especially with the sign up request, cancelling that could be disastorous because if you cancel it after the request went through, then the user would have been created but the person is confused.

I haven't done much research on this, but is there any way to notify the server that the request was cancelled. I don't mean to send a separate request to indicate that it was cancelled, but rather know if the current request is cancelled?

natario1 commented 7 years ago

Yeah, if the request left the device, there's nothing you can do currently. That's why I was saying that, rather than canceling, one should aim at unregistering the callback and register a new one with the new activity.

I think you could set up a login Service that does the work, and activity binds to it and checks its state (idle, logging, logged). But I don't see how this could get into the SDK currently...

natario1 commented 7 years ago

See #646 for async framework discussion