Fixes #231

mvi/src/main/java/com/hannesdorfmann/mosby3/ActivityMviDelegateImpl.java

@@ -207,7 +207,9 @@ static boolean retainPresenterInstance(boolean keepPresenterInstance, Activity a
     }
 
     if (!retainPresenterInstance){
-      PresenterManager.remove(activity, mosbyViewId);
+      if (mosbyViewId != null) { // mosbyViewId == null if keepPresenterInstance == false
+        PresenterManager.remove(activity, mosbyViewId);
+      }
       Log.d(DEBUG_TAG, "Destroying Presenter permanently " + presenter);
     }
   }

mvi/src/main/java/com/hannesdorfmann/mosby3/FragmentMviDelegateImpl.java

@@ -189,7 +189,7 @@ private boolean retainPresenterInstance(boolean keepPresenterOnBackstack, Activi
         retainPresenterInstance(keepPresenterOnBackstack, activity, fragment);
 
     presenter.detachView(retainPresenterInstance);
-    if (!retainPresenterInstance) {
+    if (!retainPresenterInstance && mosbyViewId != null) { // mosbyViewId == null if keepPresenterDuringScreenOrientationChange == false
       PresenterManager.remove(activity, mosbyViewId);
     }
 

mvi/src/main/java/com/hannesdorfmann/mosby3/ViewGroupMviDelegateImpl.java

@@ -173,7 +173,9 @@ private P createViewIdAndCreatePresenter() {
               + " and removing presenter permanently from internal cache because the hosting Activity will be destroyed permanently");
         }
 
-        PresenterManager.remove(activity, mosbyViewId);
+        if (mosbyViewId != null) { // mosbyViewId == null if keepPresenterDuringScreenOrientationChange == false
+          PresenterManager.remove(activity, mosbyViewId);
+        }
         mosbyViewId = null;
         presenter.detachView(false);
       } else {
@@ -209,7 +211,9 @@ private P createViewIdAndCreatePresenter() {
     } else {
       // retain instance feature disabled
       presenter.detachView(false);
-      PresenterManager.remove(activity, mosbyViewId);
+      if (mosbyViewId != null) { // mosbyViewId == null if keepPresenterDuringScreenOrientationChange == false
+        PresenterManager.remove(activity, mosbyViewId);
+      }
       mosbyViewId = null;
     }
   }

mvp/src/main/java/com/hannesdorfmann/mosby3/mvp/delegate/ActivityMvpDelegateImpl.java

@@ -179,7 +179,19 @@ private V getMvpView() {
   }
 
   @Override public void onDestroy() {
-    getPresenter().detachView(retainPresenterInstance(keepPresenterInstance, activity));
+    boolean retainPresenterInstance = retainPresenterInstance(keepPresenterInstance, activity);
+    getPresenter().detachView(retainPresenterInstance);
+    if (!retainPresenterInstance && mosbyViewId != null){
+      PresenterManager.remove(activity, mosbyViewId);
+    }
+
+    if (DEBUG) {
+      if (retainPresenterInstance) {
+        Log.d(DEBUG_TAG, "View" + getMvpView() + " destroyed temporarily. View detached from presenter "+getPresenter());
+      } else {
+        Log.d(DEBUG_TAG, "View" + getMvpView() + " destroyed permanently. View detached permanently from presenter "+getPresenter());
+      }
+    }
   }
 
   @Override public void onPause() {

mvp/src/main/java/com/hannesdorfmann/mosby3/mvp/delegate/FragmentMvpDelegateImpl.java

@@ -224,7 +224,7 @@ protected boolean retainPresenterInstance() {
 
     P presenter = getPresenter();
     presenter.detachView(retainPresenterInstance);
-    if (!retainPresenterInstance) {
+    if (!retainPresenterInstance && mosbyViewId != null) { // mosbyViewId is null if keepPresenterInstanceDuringScreenOrientationChanges  == false
       PresenterManager.remove(activity, mosbyViewId);
     }
 

mvp/src/main/java/com/hannesdorfmann/mosby3/mvp/delegate/ViewGroupMvpDelegateImpl.java

@@ -201,7 +201,9 @@ protected P createViewIdAndCreatePresenter() {
               + " and removing presenter permanently from internal cache because the hosting Activity will be destroyed permanently");
         }
 
-        PresenterManager.remove(activity, mosbyViewId);
+        if (mosbyViewId != null) { // mosbyViewId == null if keepPresenterDuringScreenOrientationChange == false
+          PresenterManager.remove(activity, mosbyViewId);
+        }
         mosbyViewId = null;
         presenter.detachView(false);
       } else {
@@ -235,9 +237,18 @@ protected P createViewIdAndCreatePresenter() {
         }
       }
     } else {
+      if (DEBUG) {
+        Log.d(DEBUG_TAG, "Detaching View "
+            + delegateCallback.getMvpView()
+            + " from Presenter "
+            + presenter
+            + " permanently");
+      }
       // retain instance feature disabled
       presenter.detachView(false);
-      PresenterManager.remove(activity, mosbyViewId);
+      if (mosbyViewId != null) { // mosbyViewId == null if keepPresenterDuringScreenOrientationChange == false
+        PresenterManager.remove(activity, mosbyViewId);
+      }
       mosbyViewId = null;
     }
   }

mvp/src/test/java/com/hannesdorfmann/mosby3/mvp/delegate/FragmentMvpDelegateImplTest.java

@@ -24,21 +24,17 @@
 import com.hannesdorfmann.mosby3.mvp.MvpPresenter;
 import com.hannesdorfmann.mosby3.mvp.MvpView;
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mockito;
-import org.mockito.junit.MockitoJUnit;
-import org.mockito.junit.MockitoRule;
 import org.powermock.api.mockito.PowerMockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
 /**
  * @author Hannes Dorfmann
  */
-@RunWith(PowerMockRunner.class)
-@PrepareForTest({Fragment.class})
+@RunWith(PowerMockRunner.class) @PrepareForTest({ Fragment.class })
 public class FragmentMvpDelegateImplTest {
 
   // TODO write test for retaining fragment
@@ -73,39 +69,80 @@ public class FragmentMvpDelegateImplTest {
   }
 
   @Test public void appStartWithScreenOrientationChangeAndFinallyFinishing() {
-    startFragment(null, 1, 1, 1);
+    startFragment(delegate, null, 1, 1, 1);
     Bundle bundle = BundleMocker.create();
-    finishFragment(bundle, 1, true, true, false);
-    startFragment(bundle, 1, 2, 2);
-    finishFragment(bundle, 1, false, false, true);
+    finishFragment(delegate, bundle, 1, true, true, false);
+    startFragment(delegate, bundle, 1, 2, 2);
+    finishFragment(delegate, bundle, 1, false, false, true);
   }
 
   @Test public void appStartFinishing() {
-    startFragment(null, 1, 1, 1);
+    startFragment(delegate, null, 1, 1, 1);
     Bundle bundle = BundleMocker.create();
-    finishFragment(bundle, 1, false, false, true);
+    finishFragment(delegate, bundle, 1, false, false, true);
   }
 
   @Test public void dontKeepPresenter() {
     delegate = new FragmentMvpDelegateImpl<>(fragment, callback, false, false);
-    startFragment(null, 1, 1, 1);
+    startFragment(delegate, null, 1, 1, 1);
     Bundle bundle = BundleMocker.create();
-    finishFragment(bundle, 1, false, true, false);
-    startFragment(null, 2, 2, 2);
-    finishFragment(bundle, 2, false, false, true);
+    finishFragment(delegate, bundle, 1, false, true, false);
+    startFragment(delegate, null, 2, 2, 2);
+    finishFragment(delegate, bundle, 2, false, false, true);
   }
 
-  private void startFragment(Bundle bundle, int createPresenter, int setPresenter, int attachView) {
+  /**
+   * Checks if two Fragments one that keeps presenter, the other who doesn't keep presenter during
+   * screen orientation changes work properly
+   *
+   * https://github.com/sockeqwe/mosby/issues/231
+   */
+  @Test public void dontKeepPresenterWithSecondFragmentInPresenterManager() {
+
+    MvpView view1 = new MvpView() {
+    };
+
+    MvpPresenter<MvpView> presenter1 = Mockito.mock(MvpPresenter.class);
+    Fragment fragment1 = PowerMockito.mock(Fragment.class);
+    PartialMvpDelegateCallbackImpl callback1 = Mockito.mock(PartialMvpDelegateCallbackImpl.class);
+    Mockito.doCallRealMethod().when(callback1).setPresenter(presenter1);
+    Mockito.doCallRealMethod().when(callback1).getPresenter();
+    Mockito.when(callback1.getMvpView()).thenReturn(view1);
+    Mockito.when(fragment1.getActivity()).thenReturn(activity);
+    Mockito.when(callback1.createPresenter()).thenReturn(presenter1);
+
+    FragmentMvpDelegateImpl<MvpView, MvpPresenter<MvpView>> keepDelegate =
+        new FragmentMvpDelegateImpl<>(fragment1, callback1, true, false);
+
+    startFragment(keepDelegate, null);
+
+    FragmentMvpDelegateImpl<MvpView, MvpPresenter<MvpView>> dontKeepDelegate =
+        new FragmentMvpDelegateImpl<>(fragment, callback, false, false);
+
+
+    startFragment(dontKeepDelegate, null, 1, 1, 1);
+    Bundle bundle = BundleMocker.create();
+    finishFragment(dontKeepDelegate, bundle, 1, false, true, false);
+    startFragment(dontKeepDelegate, null, 2, 2, 2);
+    finishFragment(dontKeepDelegate, bundle, 2, false, false, true);
+
+    Bundle bundle2 = BundleMocker.create();
+    finishFragment(keepDelegate, bundle2);
+  }
+
+  private void startFragment(FragmentMvpDelegateImpl<MvpView, MvpPresenter<MvpView>> delegate,
+      Bundle bundle, int createPresenter, int setPresenter, int attachView) {
     Mockito.when(callback.createPresenter()).thenReturn(presenter);
 
-    startFragment(bundle);
+    startFragment(delegate, bundle);
 
     Mockito.verify(callback, Mockito.times(createPresenter)).createPresenter();
     Mockito.verify(callback, Mockito.times(setPresenter)).setPresenter(presenter);
     Mockito.verify(presenter, Mockito.times(attachView)).attachView(view);
   }
 
-  private void startFragment(Bundle bundle) {
+  private void startFragment(FragmentMvpDelegateImpl<MvpView, MvpPresenter<MvpView>> delegate,
+      Bundle bundle) {
     delegate.onAttach(activity);
     delegate.onCreate(bundle);
     delegate.onViewCreated(null, bundle);
@@ -114,18 +151,24 @@ private void startFragment(Bundle bundle) {
     delegate.onResume();
   }
 
-  private void finishFragment(Bundle bundle, int detachViewCount, boolean expectKeepPresenter,
-      boolean changingConfigurations, boolean isFinishing) {
-    Mockito.when(callback.getPresenter()).thenReturn(presenter);
-    Mockito.when(activity.isChangingConfigurations()).thenReturn(changingConfigurations);
-    Mockito.when(activity.isFinishing()).thenReturn(isFinishing);
-
+  private void finishFragment(FragmentMvpDelegateImpl<MvpView, MvpPresenter<MvpView>> delegate,
+      Bundle bundle) {
     delegate.onPause();
     delegate.onSaveInstanceState(bundle);
     delegate.onStop();
     delegate.onDestroyView();
     delegate.onDestroy();
     delegate.onDetach();
+  }
+
+  private void finishFragment(FragmentMvpDelegateImpl<MvpView, MvpPresenter<MvpView>> delegate,
+      Bundle bundle, int detachViewCount, boolean expectKeepPresenter,
+      boolean changingConfigurations, boolean isFinishing) {
+    Mockito.when(callback.getPresenter()).thenReturn(presenter);
+    Mockito.when(activity.isChangingConfigurations()).thenReturn(changingConfigurations);
+    Mockito.when(activity.isFinishing()).thenReturn(isFinishing);
+
+    finishFragment(delegate, bundle);
 
     Mockito.verify(presenter, Mockito.times(detachViewCount)).detachView(expectKeepPresenter);
   }

mvp/src/test/java/com/hannesdorfmann/mosby3/mvp/delegate/ViewGroupMvpDelegateImplTest.java

@@ -83,6 +83,46 @@ public class ViewGroupMvpDelegateImplTest {
     finishViewGroup(2, false, false, true);
   }
 
+
+  /**
+   * Checks if two Views one that keeps presenter, the other who doesn't keep presenter during
+   * screen orientation changes work properly
+   *
+   * https://github.com/sockeqwe/mosby/issues/231
+   */
+  @Test public void dontKeepPresenterIfSecondPresenterInPresenterManager() {
+
+    MvpView view1 = new MvpView() {
+    };
+
+    MvpPresenter<MvpView> presenter1 = Mockito.mock(MvpPresenter.class);
+    PartialViewGroupMvpDelegateCallbackImpl callback1 = Mockito.mock(PartialViewGroupMvpDelegateCallbackImpl.class);
+    Mockito.doCallRealMethod().when(callback1).setPresenter(presenter1);
+    Mockito.doCallRealMethod().when(callback1).getPresenter();
+    View androidView1 = Mockito.mock(View.class);
+
+    Mockito.when(callback1.getMvpView()).thenReturn(view1);
+    Mockito.when(callback1.getContext()).thenReturn(activity);
+    Mockito.when(androidView1.isInEditMode()).thenReturn(false);
+    Mockito.when(callback1.createPresenter()).thenReturn(presenter1);
+
+    ViewGroupMvpDelegateImpl<MvpView, MvpPresenter<MvpView>> keepDeelgate
+        = new ViewGroupMvpDelegateImpl<MvpView, MvpPresenter<MvpView>>(androidView1, callback1, true);
+
+
+    delegate = new ViewGroupMvpDelegateImpl<MvpView, MvpPresenter<MvpView>>(androidView, callback, false);
+
+    keepDeelgate.onAttachedToWindow();
+
+    startViewGroup(1, 1, 1);
+    finishViewGroup(1, false, true, false);
+    startViewGroup(2, 2, 2);
+    finishViewGroup(2, false, false, true);
+
+    keepDeelgate.onDetachedFromWindow();
+  }
+
+
   private void startViewGroup(int createPresenter, int setPresenter, int attachView) {
     Mockito.when(callback.createPresenter()).thenReturn(presenter);
 

sample-mail/src/main/java/com/hannesdorfmann/mosby3/sample/mail/statistics/StatisticsDialog.java

@@ -81,6 +81,11 @@ public class StatisticsDialog extends AppCompatDialogFragment implements Statist
     contentView.setLayoutManager(new LinearLayoutManager(getActivity()));
   }
 
+  @Override public void onDestroyView() {
+    super.onDestroyView();
+    delegate.onDestroyView();
+  }
+
   @Override public void onStart() {
     super.onStart();
     delegate.onStart();

viewstate/src/main/java/com/hannesdorfmann/mosby3/mvp/delegate/ViewGroupMvpViewStateDelegateImpl.java

@@ -272,7 +272,6 @@ private VS createViewState() {
 
       if (destroyedPermanently) {
         // Whole activity will be destroyed
-        // Internally Orientation manager already does the clean up
         if (DEBUG) {
           Log.d(DEBUG_TAG, "Detaching View "
               + delegateCallback.getMvpView()
@@ -281,7 +280,9 @@ private VS createViewState() {
               + " and removing presenter permanently from internal cache because the hosting Activity will be destroyed permanently");
         }
 
-        PresenterManager.remove(activity, mosbyViewId);
+        if (mosbyViewId != null) { // mosbyViewId == null if keepPresenter == false
+          PresenterManager.remove(activity, mosbyViewId);
+        }
         mosbyViewId = null;
         presenter.detachView(false);
       } else {
@@ -316,8 +317,19 @@ private VS createViewState() {
       }
     } else {
       // retain instance feature disabled
+
+      if (DEBUG) {
+        Log.d(DEBUG_TAG, "Detaching View "
+            + delegateCallback.getMvpView()
+            + " from Presenter "
+            + presenter
+            + " permanently");
+      }
+
       presenter.detachView(false);
-      PresenterManager.remove(activity, mosbyViewId);
+      if (mosbyViewId != null) { // mosbyViewId == null if keepPresenter == false
+        PresenterManager.remove(activity, mosbyViewId);
+      }
       mosbyViewId = null;
     }
   }