aartikov / Alligator

Alligator is Android navigation library that will help to organize your navigation code in clean and testable way.
MIT License
298 stars 17 forks source link

"Race condition" when using switchTo() #8

Closed emanzanoaxa closed 6 years ago

emanzanoaxa commented 6 years ago

I have activity A that is calling switchTo() to load a fragment, but the command is queued because I have a LockActivity that shows a pin lock screen when "onStart()" is called from any Activity. That makes the switchTo() command to wait in the queue because the navigation to the lock activity is being executed. Then when the navigation context is set on that lock activity the switchTo command is executed, but on that activity I don't have a ScreenSwitcher set because I don't need to load any fragment there. That throws a NavigationException.

I propose to give access to the command queue to be able to manage that weird cases, but maybe there's a more elegant or more robust solution for that.

EDIT: I added a flag on the command interface meaning if the command can be discarded if no screen switcher is present, by default is always false, I'll make a pull request if you want to accept it.

aartikov commented 6 years ago

In what order do you call navigation method? This order goForward, switchTo means for a navigator "go to a lock screen and then switch screens there". In that case NavigationException is expected because a lock screen can't switch screens. So an order should be opposite - switchTo, goForward. Or you can use ScreenResult if you want to switch screens after an user returns from a lock screen.

emanzanoaxa commented 6 years ago

Yes I know but I cannot switch that order because of the implementation of the app.

The thing is: 1 - Open activity that will load a fragment with switchTo(). 2 - While the activity is being opened, on the "onStart()" method, a goForward() command is executed to navigate to a LockActivity because you have to set a pincode to access the app. 3 - Before the "goForward()" is executed, the first activity is not stopped yet and the "switchTo()" command is added to the queue. 4 - Alligator executes the goForward() to the LockActivity and then when the navigation context is set, the pending switchTo() command is executed without a ScreenSwitcher added.

In my concrete implementation, as the navigation to the LockActivity is done on an external lifecycle callback, I cannot know if the LockActivity is pending to be opened from any activity. If I could access the navigation queue (at least read only), I could check if there is a goForward() pending before adding the switchTo() command.

aartikov commented 6 years ago

Still don't understand where and when do you call switchTo().

emanzanoaxa commented 6 years ago

I call it after onStart(), but the first activity creation lifecycle is still running when I am navigating to the LockActivity, so switchTo() is called after goForward().

aartikov commented 6 years ago

In my opinion the best solution would be rethink how navigation to a lock screen is called. The order of navigation method calls is crucial here. It is hard to give a concrete advice because I don't know how your implementation looks like.

If I could access the navigation queue (at least read only), I could check if there is a goForward() pending before adding the switchTo() command.

I'm not sure that giving a direct access to the navigation queue is a good idea. It is supposed to be implementation detail but not a pulic api.

aartikov commented 6 years ago

I thought about the issue some more and decided that these methods should be added to Navigator:

You can use it like that:

@Override
protected void onStart() {
    super.onStart();
    if (mScreenSwitcher.getCurrentFragment() == null && !mNavigator.hasPendingCommands()) {
        mNavigator.switchTo(new SomeScreen());
    }
}

Now this change is in the develop branch. Take it from jitpack.io. Give me to know if it is helpful for you. Then I will make a release.

emanzanoaxa commented 6 years ago

That's a more robust solution! Yes is absolutely helpful, many thanks. This library is the best solution for android navigation I've ever seen, sorry if I bother you too much with my issues but I really want to contribute to it :)

aartikov commented 6 years ago

I am very glad. Thank you for helping to make Alligator better. Version 2.1.0 is ready!