Merge pull request #112 from adolgiy/hotfix/fragment-factory-npe

Fix NPE when localStackCopy.size() == 0 and FragmentFactory is used
terrakok · May 10, 2020 · 7c31cdc · 7c31cdc
2 parents dcf1532 + 05a8ec8
commit 7c31cdc
Showing 1 changed file with 58 additions and 44 deletions.
diff --git a/library/src/main/java/ru/terrakok/cicerone/android/support/SupportAppNavigator.java b/library/src/main/java/ru/terrakok/cicerone/android/support/SupportAppNavigator.java
@@ -3,17 +3,24 @@
 import android.app.Activity;
 import android.content.Intent;
 import android.os.Bundle;
+
 import androidx.fragment.app.Fragment;
 import androidx.fragment.app.FragmentActivity;
 import androidx.fragment.app.FragmentManager;
 import androidx.fragment.app.FragmentTransaction;
+
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
-import ru.terrakok.cicerone.Navigator;
-import ru.terrakok.cicerone.commands.*;
 
 import java.util.LinkedList;
 
+import ru.terrakok.cicerone.Navigator;
+import ru.terrakok.cicerone.commands.Back;
+import ru.terrakok.cicerone.commands.BackTo;
+import ru.terrakok.cicerone.commands.Command;
+import ru.terrakok.cicerone.commands.Forward;
+import ru.terrakok.cicerone.commands.Replace;
+
 /**
  * Navigator implementation for launch fragments and activities.<br>
  * Feature {@link BackTo} works only for fragments.<br>
@@ -98,24 +105,7 @@ protected void fragmentForward(@NotNull Forward command) {
         FragmentParams fragmentParams = screen.getFragmentParams();
         Fragment fragment = fragmentParams == null ? createFragment(screen) : null;
 
-        FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
-
-        setupFragmentTransaction(
-                command,
-                fragmentManager.findFragmentById(containerId),
-                fragment,
-                fragmentTransaction);
-
-        if (fragmentParams != null) {
-            fragmentTransaction.replace(containerId, fragmentParams.getFragmentClass(), fragmentParams.getArguments());
-        } else {
-            fragmentTransaction.replace(containerId, fragment);
-        }
-
-        fragmentTransaction
-                .addToBackStack(screen.getScreenKey())
-                .commit();
-        localStackCopy.add(screen.getScreenKey());
+        forwardFragmentInternal(command, screen, fragmentParams, fragment);
     }
 
     protected void fragmentBack() {
@@ -155,6 +145,9 @@ protected void fragmentReplace(@NotNull Replace command) {
             fragmentManager.popBackStack();
             localStackCopy.removeLast();
 
+            forwardFragmentInternal(command, screen, fragmentParams, fragment);
+
+        } else {
             FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
 
             setupFragmentTransaction(
@@ -164,32 +157,53 @@ protected void fragmentReplace(@NotNull Replace command) {
                     fragmentTransaction
             );
 
-            if (fragmentParams != null) {
-                fragmentTransaction.replace(
-                        containerId,
-                        fragmentParams.getFragmentClass(),
-                        fragmentParams.getArguments());
-            } else {
-                fragmentTransaction.replace(containerId, fragment);
-            }
-            fragmentTransaction
-                    .addToBackStack(screen.getScreenKey())
-                    .commit();
-            localStackCopy.add(screen.getScreenKey());
+            replaceFragmentInternal(fragmentTransaction, screen, fragmentParams, fragment);
 
-        } else {
-            FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
+            fragmentTransaction.commit();
+        }
+    }
 
-            setupFragmentTransaction(
-                    command,
-                    fragmentManager.findFragmentById(containerId),
-                    fragment,
-                    fragmentTransaction
-            );
+    private void forwardFragmentInternal(
+            @NotNull Command command,
+            @NotNull SupportAppScreen screen,
+            @Nullable FragmentParams fragmentParams,
+            @Nullable Fragment fragment
+    ) {
+        FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
+
+        setupFragmentTransaction(
+                command,
+                fragmentManager.findFragmentById(containerId),
+                fragment,
+                fragmentTransaction
+        );
+
+        replaceFragmentInternal(fragmentTransaction, screen, fragmentParams, fragment);
+
+        fragmentTransaction
+                .addToBackStack(screen.getScreenKey())
+                .commit();
 
-            fragmentTransaction
-                    .replace(containerId, fragment)
-                    .commit();
+        localStackCopy.add(screen.getScreenKey());
+    }
+
+    private void replaceFragmentInternal(
+            @NotNull FragmentTransaction transaction,
+            @NotNull SupportAppScreen screen,
+            @Nullable FragmentParams params,
+            @Nullable Fragment fragment
+    ) {
+        if (params != null) {
+            transaction.replace(
+                    containerId,
+                    params.getFragmentClass(),
+                    params.getArguments()
+            );
+        } else if (fragment != null) {
+            transaction.replace(containerId, fragment);
+        } else {
+            throw new IllegalArgumentException("Either 'params' or 'fragment' shouldn't " +
+                    "be null for " + screen.getScreenKey());
         }
     }
 
@@ -307,7 +321,7 @@ protected void errorWhileCreatingScreen(@NotNull SupportAppScreen screen) {
      * Override this method if you want to handle apply command error.
      *
      * @param command command
-     * @param error error
+     * @param error   error
      */
     protected void errorOnApplyCommand(
             @NotNull Command command,