Refactor request tracker out of lifecycle manager

library/src/main/java/com/bumptech/glide/manager/LifecycleRequestManager.java

@@ -3,74 +3,53 @@ package com.bumptech.glide.manager;
 import android.content.Context;
 import com.bumptech.glide.request.Request;
 
-import java.util.Collections;
-import java.util.Set;
-import java.util.WeakHashMap;
+class LifecycleRequestManager implements RequestManager {
 
-class LifecycleRequestManager implements RequestManager, ConnectivityMonitor.ConnectivityListener {
-    // Most requests will be for views and will therefore be held strongly (and safely) by the view via the tag.
-    // However, a user can always pass in a different type of target which may end up not being strongly referenced even
-    // though the user still would like the request to finish. Weak references are therefore only really functional in
-    // this context for view targets. Despite the side affects, WeakReferences are still essentially required. A user
-    // can always make repeated requests into targets other than views, or use an activity manager in a fragment pager
-    // where holding strong references would steadily leak bitmaps and/or views.
-    private final Set<Request> requests = Collections.newSetFromMap(new WeakHashMap<Request, Boolean>());
     private final ConnectivityMonitor connectivityMonitor;
+    private final RequestTracker requestTracker;
 
     LifecycleRequestManager(Context context) {
-        this(context, new ConnectivityMonitorFactory());
+        this(context, new RequestTracker(), new ConnectivityMonitorFactory());
     }
 
-    LifecycleRequestManager(Context context, ConnectivityMonitorFactory factory) {
-        this.connectivityMonitor = factory.build(context, this);
+    LifecycleRequestManager(Context context, RequestTracker requestTracker, ConnectivityMonitorFactory factory) {
+        this.requestTracker = requestTracker;
+        this.connectivityMonitor = factory.build(context, new RequestManagerConnectivityListener());
         connectivityMonitor.register();
     }
 
     @Override
     public void addRequest(Request request) {
-        requests.add(request);
+        requestTracker.addRequest(request);
     }
 
     @Override
     public void removeRequest(Request request) {
-        requests.remove(request);
+        requestTracker.removeRequest(request);
     }
 
-    public void onStart() {
+    void onStart() {
         // onStart might not be called because this object may be created after the fragment/activity's onStart method.
         connectivityMonitor.register();
 
-        for (Request request : requests) {
-            if (!request.isComplete() && !request.isRunning()) {
-                request.run();
-            }
-        }
-
+        requestTracker.resumeRequests();
     }
 
-    public void onStop() {
+    void onStop() {
         connectivityMonitor.unregister();
-        for (Request request : requests) {
-            if (!request.isComplete() && !request.isFailed()) {
-                request.clear();
-            }
-        }
+        requestTracker.pauseRequests();
     }
 
-    public void onDestroy() {
-        for (Request request : requests) {
-            request.clear();
-        }
+    void onDestroy() {
+        requestTracker.clearRequests();
     }
 
-    @Override
-    public void onConnectivityChanged(boolean isConnected) {
-        for (Request request : requests) {
-            if (request.isFailed()) {
-                request.run();
-            } else if (!request.isComplete()) {
-                request.clear();
-                request.run();
+    private class RequestManagerConnectivityListener implements ConnectivityMonitor.ConnectivityListener {
+
+        @Override
+        public void onConnectivityChanged(boolean isConnected) {
+            if (isConnected) {
+                requestTracker.restartRequests();
             }
         }
     }

library/src/main/java/com/bumptech/glide/manager/RequestTracker.java

+package com.bumptech.glide.manager;
+
+import com.bumptech.glide.request.Request;
+
+import java.util.Collections;
+import java.util.Set;
+import java.util.WeakHashMap;
+
+public class RequestTracker {
+    // Most requests will be for views and will therefore be held strongly (and safely) by the view via the tag.
+    // However, a user can always pass in a different type of target which may end up not being strongly referenced even
+    // though the user still would like the request to finish. Weak references are therefore only really functional in
+    // this context for view targets. Despite the side affects, WeakReferences are still essentially required. A user
+    // can always make repeated requests into targets other than views, or use an activity manager in a fragment pager
+    // where holding strong references would steadily leak bitmaps and/or views.
+    private final Set<Request> requests = Collections.newSetFromMap(new WeakHashMap<Request, Boolean>());
+
+    public void addRequest(Request request) {
+        requests.add(request);
+    }
+
+    public void removeRequest(Request request) {
+        requests.remove(request);
+    }
+
+    /**
+     * Stops any in progress requests.
+     */
+    public void pauseRequests() {
+        for (Request request : requests) {
+            if (!request.isComplete() && !request.isFailed()) {
+                request.clear();
+            }
+        }
+    }
+
+    /**
+     * Starts any not yet completed or failed requests.
+     */
+    public void resumeRequests() {
+        for (Request request : requests) {
+            if (!request.isComplete() && !request.isRunning()) {
+                request.run();
+            }
+        }
+    }
+
+    /**
+     * Cancels all requests and clears their resources.
+     */
+    public void clearRequests() {
+        for (Request request : requests) {
+            request.clear();
+        }
+    }
+
+    /**
+     * Restarts failed requests and cancels and restarts in progress requests.
+     */
+    public void restartRequests() {
+        for (Request request : requests) {
+            if (request.isFailed()) {
+                request.run();
+            } else if (!request.isComplete()) {
+                request.clear();
+                request.run();
+            }
+        }
+    }
+}

library/src/test/java/com/bumptech/glide/manager/LifecycleRequestManagerTest.java

 package com.bumptech.glide.manager;
 
 import android.content.Context;
-import com.bumptech.glide.request.Request;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 import org.robolectric.Robolectric;
 import org.robolectric.RobolectricTestRunner;
 
@@ -19,135 +20,45 @@ import static org.mockito.Mockito.when;
 public class LifecycleRequestManagerTest {
     private LifecycleRequestManager manager;
     private ConnectivityMonitor connectivityMonitor;
+    private RequestTracker requestTracker;
+    private ConnectivityMonitor.ConnectivityListener connectivityListener;
 
     @Before
     public void setUp() {
         connectivityMonitor = mock(ConnectivityMonitor.class);
         ConnectivityMonitorFactory factory = mock(ConnectivityMonitorFactory.class);
         when(factory.build(any(Context.class), any(ConnectivityMonitor.ConnectivityListener.class)))
-                .thenReturn(connectivityMonitor);
-        manager = new LifecycleRequestManager(Robolectric.application, factory);
+                .thenAnswer(new Answer<Object>() {
+                    @Override
+                    public Object answer(InvocationOnMock invocation) throws Throwable {
+                        connectivityListener = (ConnectivityMonitor.ConnectivityListener) invocation.getArguments()[1];
+                        return connectivityMonitor;
+                    }
+                });
+        requestTracker = mock(RequestTracker.class);
+        manager = new LifecycleRequestManager(Robolectric.application, requestTracker, factory);
     }
 
     @Test
-    public void testCancelsRequestsOnStop() {
+    public void testPausesRequestsOnStop() {
         manager.onStart();
-        Request request = mock(Request.class);
-        manager.addRequest(request);
         manager.onStop();
 
-        verify(request).clear();
+        verify(requestTracker).pauseRequests();
     }
 
     @Test
-    public void testCanAddAndRemoveRequest() {
+    public void testResumesRequestsOnStart() {
         manager.onStart();
-        Request request = mock(Request.class);
-        manager.addRequest(request);
-        manager.removeRequest(request);
 
-        manager.onStop();
-
-        verify(request, never()).clear();
-    }
-
-    @Test
-    public void testCanAddMultipleRequests() {
-        manager.onStart();
-        Request first = mock(Request.class);
-        Request second = mock(Request.class);
-        manager.addRequest(first);
-        manager.addRequest(second);
-
-        manager.onStop();
-
-        verify(first).clear();
-        verify(second).clear();
-    }
-
-    @Test
-    public void testDoesNotClearCompleteRequestsOnStop() {
-        manager.onStart();
-        Request request = mock(Request.class);
-        when(request.isComplete()).thenReturn(true);
-        manager.addRequest(request);
-
-        manager.onStop();
-
-        verify(request, never()).clear();
-    }
-
-    @Test
-    public void testDoesNotClearFailedRequestsOnStop() {
-        manager.onStart();
-        Request request = mock(Request.class);
-        when(request.isFailed()).thenReturn(true);
-        manager.addRequest(request);
-
-        manager.onStop();
-
-        verify(request, never()).clear();
-    }
-
-    @Test
-    public void testRestartsStoppedRequestOnStart() {
-        Request request = mock(Request.class);
-        manager.addRequest(request);
-
-        manager.onStart();
-
-        verify(request).run();
+        verify(requestTracker).resumeRequests();
     }
 
     @Test
-    public void testDoesNotRestartCompletedRequestsOnStart() {
-        Request request = mock(Request.class);
-        when(request.isComplete()).thenReturn(true);
-        manager.addRequest(request);
-
-        manager.onStart();
-
-        verify(request, never()).run();
-    }
-
-    @Test
-    public void testDoesRestartFailedRequestsOnStart() {
-        Request request = mock(Request.class);
-        when(request.isFailed()).thenReturn(true);
-        manager.addRequest(request);
-
-        manager.onStart();
-
-        verify(request).run();
-    }
-
-    @Test
-    public void testDoesNotStartStartedRequestsOnStart() {
-        Request request = mock(Request.class);
-        when(request.isRunning()).thenReturn(true);
-        manager.addRequest(request);
-
-        manager.onStart();
-
-        verify(request, never()).run();
-    }
-
-    @Test
-    public void testClearsAllRequestsOnDestroy() {
-        Request first = mock(Request.class);
-        Request second = mock(Request.class);
-        when(second.isComplete()).thenReturn(true);
-        Request third = mock(Request.class);
-        when(third.isFailed()).thenReturn(true);
-        manager.addRequest(first);
-        manager.addRequest(second);
-        manager.addRequest(third);
-
+    public void testClearsRequestsOnDestroy() {
         manager.onDestroy();
 
-        verify(first).clear();
-        verify(second).clear();
-        verify(third).clear();
+        verify(requestTracker).clearRequests();
     }
 
     @Test
@@ -170,25 +81,16 @@ public class LifecycleRequestManagerTest {
     }
 
     @Test
-    public void testRestartsFailedRequestOnConnected() {
-        Request request = mock(Request.class);
-        when(request.isFailed()).thenReturn(true);
-        manager.addRequest(request);
-
-        manager.onConnectivityChanged(true);
+    public void testRestartsRequestOnConnected() {
+        connectivityListener.onConnectivityChanged(true);
 
-        verify(request).run();
+        verify(requestTracker).restartRequests();
     }
 
     @Test
-    public void testCancelsAndRestartsNotYetFinishedRequestsWhenBecomesConnected() {
-        Request request = mock(Request.class);
-        when(request.isComplete()).thenReturn(false);
-        manager.addRequest(request);
-
-        manager.onConnectivityChanged(true);
+    public void testDoesNotRestartRequestsOnDisconnected() {
+        connectivityListener.onConnectivityChanged(false);
 
-        verify(request).clear();
-        verify(request).run();
+        verify(requestTracker, never()).restartRequests();
     }
 }

library/src/test/java/com/bumptech/glide/manager/RequestTrackerTest.java

+package com.bumptech.glide.manager;
+
+import com.bumptech.glide.request.Request;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class RequestTrackerTest {
+    private RequestTracker tracker;
+
+    @Before
+    public void setUp() {
+        tracker = new RequestTracker();
+    }
+
+    @Test
+    public void testClearRequestsClearsAddedRequests() {
+        Request request = mock(Request.class);
+        tracker.addRequest(request);
+
+        tracker.clearRequests();
+
+        verify(request).clear();
+    }
+
+    @Test
+    public void testCanAddAndRemoveRequest() {
+        Request request = mock(Request.class);
+        tracker.addRequest(request);
+        tracker.removeRequest(request);
+
+        tracker.clearRequests();
+
+        verify(request, never()).clear();
+    }
+
+    @Test
+    public void testCanAddMultipleRequests() {
+        Request first = mock(Request.class);
+        Request second = mock(Request.class);
+        tracker.addRequest(first);
+        tracker.addRequest(second);
+
+        tracker.clearRequests();
+
+        verify(first).clear();
+        verify(second).clear();
+    }
+
+    @Test
+    public void testClearsInProgressRequestsOnPause() {
+        Request request = mock(Request.class);
+        when(request.isRunning()).thenReturn(true);
+        tracker.addRequest(request);
+
+        tracker.pauseRequests();
+
+        verify(request).clear();
+    }
+
+    @Test
+    public void testDoesNotClearCompleteRequestsOnPause() {
+        Request request = mock(Request.class);
+        when(request.isComplete()).thenReturn(true);
+        tracker.addRequest(request);
+
+        tracker.pauseRequests();
+
+        verify(request, never()).clear();
+    }
+
+    @Test
+    public void testDoesNotClearFailedRequestsOnPause() {
+        Request request = mock(Request.class);
+        when(request.isFailed()).thenReturn(true);
+        tracker.addRequest(request);
+
+        tracker.pauseRequests();
+
+        verify(request, never()).clear();
+    }
+
+    @Test
+    public void testRestartsStoppedRequestOnResume() {
+        Request request = mock(Request.class);
+        tracker.addRequest(request);
+
+        tracker.resumeRequests();
+
+        verify(request).run();
+    }
+
+    @Test
+    public void testDoesNotRestartCompletedRequestsOnResume() {
+        Request request = mock(Request.class);
+        when(request.isComplete()).thenReturn(true);
+        tracker.addRequest(request);
+
+        tracker.resumeRequests();
+
+        verify(request, never()).run();
+    }
+
+    @Test
+    public void testDoesRestartFailedRequestsOnResume() {
+        Request request = mock(Request.class);
+        when(request.isFailed()).thenReturn(true);
+        tracker.addRequest(request);
+
+        tracker.resumeRequests();
+
+        verify(request).run();
+    }
+
+    @Test
+    public void testDoesNotStartStartedRequestsOnResume() {
+        Request request = mock(Request.class);
+        when(request.isRunning()).thenReturn(true);
+        tracker.addRequest(request);
+
+        tracker.resumeRequests();
+
+        verify(request, never()).run();
+    }
+
+    @Test
+    public void testRestartsFailedRequestRestart() {
+        Request request = mock(Request.class);
+        when(request.isFailed()).thenReturn(true);
+        tracker.addRequest(request);
+
+        tracker.restartRequests();
+
+        verify(request).run();
+    }
+
+    @Test
+    public void testCancelsAndRestartsNotYetFinishedRequestsOnRestart() {
+        Request request = mock(Request.class);
+        when(request.isComplete()).thenReturn(false);
+        tracker.addRequest(request);
+
+        tracker.restartRequests();
+
+        verify(request).clear();
+        verify(request).run();
+    }
+}
\ No newline at end of file