Skip to content

Commit

Permalink
Investigating back nav injection issue
Browse files Browse the repository at this point in the history
  • Loading branch information
@kejunxia
kejunxia committed Nov 29, 2015
1 parent d3b4dad commit c157d19
Show file tree
Hide file tree
Showing 8 changed files with 154 additions and 60 deletions.
47 changes: 24 additions & 23 deletions .../android-mvc-controller/src/main/java/com/shipdream/lib/android/mvc/__MvcGraphHelper.java
View file
Expand Up @@ -14,45 +14,46 @@
* Helper class to work with MvcGraph. Internal use only. Don't use it in your app.
*/
public class __MvcGraphHelper {
private static Map<NavigationController.EventC2V.OnLocationForward, Collection<Provider>> retainedProviders = new HashMap<>();
private static Map<NavigationController.EventC2V.OnLocationChanged, Collection<Provider>> retainedProviders = new HashMap<>();

/**
* Internal use. Don't call it in app directly.
*/
public static void retainCachedObjectsBeforeNavigation(
NavigationController.EventC2V.OnLocationForward navigationEvent, MvcGraph mvcGraph) {
//Retain all cached items before navigation.
Collection<ScopeCache.CachedItem> cachedItems = mvcGraph.singletonScopeCache.getCachedItems();
final NavigationController.EventC2V.OnLocationChanged navigationEvent,
final MvcGraph mvcGraph) {
//Retain all cached items before navigation.
Collection<ScopeCache.CachedItem> cachedItems = mvcGraph.singletonScopeCache.getCachedItems();

List<Provider> providers = new ArrayList<>();
retainedProviders.put(navigationEvent, providers);
for (ScopeCache.CachedItem cachedItem : cachedItems) {
Provider provider = cachedItem.getProvider();
if (provider != null) {
providers.add(provider);
provider.retain();
List<Provider> providers = new ArrayList<>();
retainedProviders.put(navigationEvent, providers);
for (ScopeCache.CachedItem cachedItem : cachedItems) {
Provider provider = cachedItem.getProvider();
if (provider != null) {
providers.add(provider);
provider.retain();
}
}
}
}

/**
* Internal use. Don't call it in app directly.
*/
public static void releaseCachedItemsAfterNavigation(
NavigationController.EventC2V.OnLocationForward navigationEvent, MvcGraph mvcGraph) {

//Release all cached items after the fragment navigated to is ready to show.
Collection<Provider> providers = retainedProviders.get(navigationEvent);
if (providers != null) {
for (Provider provider : providers) {
if (provider != null) {
provider.release();
final NavigationController.EventC2V.OnLocationChanged navigationEvent,
final MvcGraph mvcGraph) {
//Release all cached items after the fragment navigated to is ready to show.
Collection<Provider> providers = retainedProviders.get(navigationEvent);
if (providers != null) {
for (Provider provider : providers) {
if (provider != null) {
provider.release();
}
}
providers.clear();
}
providers.clear();
}

retainedProviders.remove(navigationEvent);
retainedProviders.remove(navigationEvent);
}

/**
Expand Down
10 changes: 8 additions & 2 deletions ...ntroller/src/main/java/com/shipdream/lib/android/mvc/controller/NavigationController.java
View file
Expand Up @@ -104,10 +104,16 @@ public interface NavigationController extends BaseController<NavigationControlle
void navigateBack(Object sender, String toLocationId);

interface EventC2V {
abstract class OnLocationChanged extends ValueChangeEventC2V<NavLocation> {
public OnLocationChanged(Object sender, NavLocation lastValue, NavLocation currentValue) {
super(sender, lastValue, currentValue);
}
}

/**
* Event to notify views navigation will move forward.
*/
class OnLocationForward extends ValueChangeEventC2V<NavLocation> {
class OnLocationForward extends OnLocationChanged {
private boolean clearHistory;
private NavLocation locationWhereHistoryClearedUpTo;

Expand Down Expand Up @@ -147,7 +153,7 @@ public NavLocation getLocationWhereHistoryClearedUpTo() {
/**
* Event to notify views navigation will move backward.
*/
class OnLocationBack extends ValueChangeEventC2V<NavLocation> {
class OnLocationBack extends OnLocationChanged {
private boolean fastRewind;

public OnLocationBack(Object sender, NavLocation lastValue, NavLocation currentValue,
Expand Down
9 changes: 8 additions & 1 deletion ...main/java/com/shipdream/lib/android/mvc/controller/internal/NavigationControllerImpl.java
View file
Expand Up @@ -131,7 +131,14 @@ public void navigateBack(Object sender) {

NavLocation previousLoc = currentLoc.getPreviousLocation();
getModel().setCurrentLocation(previousLoc);
postC2VEvent(new EventC2V.OnLocationBack(sender, currentLoc, previousLoc, false));

EventC2V.OnLocationBack navEvent = new EventC2V.OnLocationBack(sender, currentLoc, previousLoc, false);

if (previousLoc != null ) {
__MvcGraphHelper.retainCachedObjectsBeforeNavigation(navEvent, Injector.getGraph());
}

postC2VEvent(navEvent);

logger.trace("Nav Controller: Backward: {} -> {}", currentLoc.getLocationId(),
previousLoc == null ? "null" : previousLoc.getLocationId());
Expand Down
13 changes: 8 additions & 5 deletions ...ndroid-mvc-controller/src/test/java/com/shipdream/lib/android/mvc/TestMvcGraphHelper.java
View file
Expand Up @@ -150,6 +150,7 @@ protected ExecutorService createExecutorService() {
ControllerA controllerAInViewA = viewA.controllerA;
ManageA manageAInViewA = ((ControllerAImpl)viewA.controllerA).manageA;

//Simulate navigate to viewB
NavigationController.EventC2V.OnLocationForward navEventB = mock(
NavigationController.EventC2V.OnLocationForward.class);
__MvcGraphHelper.retainCachedObjectsBeforeNavigation(navEventB, Injector.getGraph());
Expand All @@ -158,25 +159,27 @@ protected ExecutorService createExecutorService() {
ControllerA controllerAInViewB = viewB.controllerA;
ManageA manageAInViewB = ((ControllerAImpl)viewA.controllerA).manageA;

//release viewA
Injector.getGraph().release(viewA);
__MvcGraphHelper.releaseCachedItemsAfterNavigation(navEventB, Injector.getGraph());
verify(manageAMock, times(0)).onDisposed();

//Simulate navigate to viewC
NavigationController.EventC2V.OnLocationForward navEventC = mock(
NavigationController.EventC2V.OnLocationForward.class);
__MvcGraphHelper.retainCachedObjectsBeforeNavigation(navEventC, Injector.getGraph());

Injector.getGraph().release(viewB);
__MvcGraphHelper.releaseCachedItemsAfterNavigation(navEventC, Injector.getGraph());
verify(manageAMock, times(0)).onDisposed();

Injector.getGraph().inject(viewC);
ControllerA controllerAInViewC = viewB.controllerA;
ManageA manageAInViewC = ((ControllerAImpl)viewA.controllerA).manageA;

//release viewB
Injector.getGraph().release(viewB);
__MvcGraphHelper.releaseCachedItemsAfterNavigation(navEventC, Injector.getGraph());
verify(manageAMock, times(0)).onDisposed();

Assert.assertTrue(manageAInViewA == manageAInViewB);
Assert.assertTrue(manageAInViewA == manageAInViewC);

}

}
17 changes: 16 additions & 1 deletion .../src/androidTest/java/com/shipdream/lib/android/mvc/view/nav/TestCaseNavigationBasic.java
View file
Expand Up @@ -28,6 +28,10 @@

import org.junit.Assert;
import org.junit.Test;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.inject.Inject;
import javax.inject.Singleton;
Expand All @@ -36,12 +40,16 @@
import static android.support.test.espresso.assertion.ViewAssertions.matches;
import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.withText;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class TestCaseNavigationBasic extends BaseTestCase <MvcTestActivityNavigation> {
private Logger logger = LoggerFactory.getLogger(getClass());

@Inject
private AnotherController anotherController;

Expand Down Expand Up @@ -97,6 +105,13 @@ protected void injectDependencies(ScopeCache mvcSingletonCache) {
super.injectDependencies(mvcSingletonCache);

disposeCheckerAMock = mock(DisposeCheckerA.class);
doAnswer(new Answer() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
logger.debug("Dispose checker A");
return null;
}
}).when(disposeCheckerAMock).onDisposed();
disposeCheckerBMock = mock(DisposeCheckerB.class);
disposeCheckerCMock = mock(DisposeCheckerC.class);
disposeCheckerDMock = mock(DisposeCheckerD.class);
Expand Down Expand Up @@ -141,7 +156,7 @@ public void testShouldReleaseInjectionsAfterFragmentsArePoppedOut() throws Throw
verify(disposeCheckerDMock, times(0)).onDisposed();
navigationController.navigateBack(this);
waitTest();
waitTest(1000);
waitTest(2000);
verify(disposeCheckerAMock, times(1)).onDisposed();
verify(disposeCheckerBMock, times(1)).onDisposed();
verify(disposeCheckerCMock, times(1)).onDisposed();
Expand Down
80 changes: 55 additions & 25 deletions ...oidTest/java/com/shipdream/lib/android/mvc/view/nav/TestCaseNavigationFromController.java
View file
Expand Up @@ -22,25 +22,33 @@
import com.shipdream.lib.android.mvc.view.BaseTestCase;
import com.shipdream.lib.poke.Component;
import com.shipdream.lib.poke.Consumer;
import com.shipdream.lib.poke.Provides;
import com.shipdream.lib.poke.ScopeCache;

import org.junit.Test;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Random;

import javax.inject.Inject;
import javax.inject.Singleton;

import static android.support.test.espresso.Espresso.onView;
import static android.support.test.espresso.assertion.ViewAssertions.matches;
import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.withText;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

public class TestCaseNavigationFromController extends BaseTestCase <MvcTestActivityNavigation> {
private Logger logger = LoggerFactory.getLogger(getClass());

@Inject
private NavigationController navigationController;

Expand All @@ -65,32 +73,53 @@ public static class Comp extends Component{
super(scopeCache);
}

// @Singleton
// @Provides
// public DisposeCheckerE providesDisposeCheckerE() {
// return testCaseNavigation.disposeCheckerEMock;
// }
//
// @Singleton
// @Provides
// public DisposeCheckerF providesDisposeCheckerF() {
// return testCaseNavigation.disposeCheckerFMock;
// }
//
// @Singleton
// @Provides
// public DisposeCheckerG providesDisposeCheckerG() {
// return testCaseNavigation.disposeCheckerGMock;
// }
@Singleton
@Provides
public DisposeCheckerE providesDisposeCheckerE() {
return testCaseNavigation.disposeCheckerEMock;
}

@Singleton
@Provides
public DisposeCheckerF providesDisposeCheckerF() {
return testCaseNavigation.disposeCheckerFMock;
}

@Singleton
@Provides
public DisposeCheckerG providesDisposeCheckerG() {
return testCaseNavigation.disposeCheckerGMock;
}
}

@Override
protected void injectDependencies(ScopeCache mvcSingletonCache) {
super.injectDependencies(mvcSingletonCache);

disposeCheckerEMock = mock(DisposeCheckerE.class);
doAnswer(new Answer() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
logger.debug("Dispose checker E");
return null;
}
}).when(disposeCheckerEMock).onDisposed();
disposeCheckerFMock = mock(DisposeCheckerF.class);
doAnswer(new Answer() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
logger.debug("Dispose checker F");
return null;
}
}).when(disposeCheckerFMock).onDisposed();
disposeCheckerGMock = mock(DisposeCheckerG.class);
doAnswer(new Answer() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
logger.debug("Dispose checker G");
return null;
}
}).when(disposeCheckerGMock).onDisposed();
comp = new Comp(mvcSingletonCache);
comp.testCaseNavigation = this;
AndroidMvc.graph().register(comp);
Expand Down Expand Up @@ -187,18 +216,19 @@ public void consume(ControllerG instance) {
navigationController.navigateBack(this);

onView(withText(valE)).check(matches(isDisplayed()));
waitTest(1000);
waitTest();
verify(disposeCheckerEMock, times(0)).onDisposed();
// verify(disposeCheckerFMock, times(1)).onDisposed();
// verify(disposeCheckerGMock, times(1)).onDisposed();
// resetDisposeCheckers();
LoggerFactory.getLogger(getClass()).debug("DisposeCheck, checking F");
verify(disposeCheckerFMock, times(1)).onDisposed();
//__MvcGraphHelper retaining all cache is dangerous. Try to only retain relevant injected instances.
verify(disposeCheckerGMock, times(0)).onDisposed();
resetDisposeCheckers();
LoggerFactory.getLogger(getClass()).debug("DisposeCheck nav back");
navigationController.navigateBack(this);

waitTest(1000);
// verify(disposeCheckerEMock, times(1)).onDisposed();
// verify(disposeCheckerFMock, times(0)).onDisposed();
// verify(disposeCheckerGMock, times(0)).onDisposed();
verify(disposeCheckerEMock, times(1)).onDisposed();
verify(disposeCheckerFMock, times(0)).onDisposed();
verify(disposeCheckerGMock, times(1)).onDisposed();
}

private void resetDisposeCheckers() {
Expand Down
33 changes: 31 additions & 2 deletions library/android-mvc/src/main/java/com/shipdream/lib/android/mvc/view/MvcActivity.java
View file
Expand Up @@ -485,7 +485,7 @@ public void run() {
}
}

private void performBackNav(NavigationController.EventC2V.OnLocationBack event) {
private void performBackNav(final NavigationController.EventC2V.OnLocationBack event) {
NavLocation currentLoc = event.getCurrentValue();
if (currentLoc == null) {
MvcActivity mvcActivity = ((MvcActivity) getActivity());
Expand All @@ -510,6 +510,33 @@ private void performBackNav(NavigationController.EventC2V.OnLocationBack event)
}
}
}

MvcFragment lastFrag = null;
if (event.getLastValue() != null && event.getLastValue().getLocationId() != null) {
lastFrag = (MvcFragment) fm.findFragmentByTag(
getFragmentTag(event.getLastValue().getLocationId()));
}
final MvcFragment finalLastFrag = lastFrag;

if (finalLastFrag != null) {
finalLastFrag.selfRelease = false;
}

currentFrag.registerOnViewReadyListener(new Runnable() {
@Override
public void run() {
//Release reference count to pair the retaining by NavigationControllerImpl
// with Injector.getGraph().retainCachedObjectsBeforeNavigation();
__MvcGraphHelper.releaseCachedItemsAfterNavigation(event, Injector.getGraph());

if (finalLastFrag != null) {
finalLastFrag.releaseDependencies();
finalLastFrag.selfRelease = true;
}

currentFrag.unregisterOnViewReadyListener(this);
}
});
}

if (event.isFastRewind()) {
Expand Down Expand Up @@ -544,7 +571,9 @@ private void performBackNav(NavigationController.EventC2V.OnLocationBack event)
}
} else {
fm.popBackStack();
logger.trace("Navigation back: On step back to location {}", currentLoc.getLocationId());
logger.trace("Navigation back: On step back from {} to location {}",
event.getLastValue() != null ? event.getLastValue().getLocationId() : null,
currentLoc.getLocationId());
}
}
}
Expand Down

0 comments on commit c157d19

Please sign in to comment.