Create NavigationViewRouter timeout to unblock routing state

Author: danesfeder

libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationViewRouter.java

@@ -15,6 +15,7 @@
 import com.mapbox.services.android.navigation.v5.route.RouteListener;
 import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;
 
+import java.util.Date;
 import java.util.List;
 
 class NavigationViewRouter implements RouteListener {
@@ -29,7 +30,7 @@ class NavigationViewRouter implements RouteListener {
   private RouteOptions routeOptions;
   private DirectionsRoute currentRoute;
   private Location location;
-  private boolean isRouting;
+  private RouteCallStatus callStatus;
 
   NavigationViewRouter(RouteFetcher onlineRouter, ConnectivityStatusProvider connectivityStatus,
                        ViewRouteListener listener) {
@@ -43,25 +44,28 @@ class NavigationViewRouter implements RouteListener {
   // Extra fields for testing purposes
   NavigationViewRouter(RouteFetcher onlineRouter, NavigationViewOfflineRouter offlineRouter,
                        ConnectivityStatusProvider connectivityStatus, RouteComparator routeComparator,
-                       ViewRouteListener listener) {
+                       ViewRouteListener listener, RouteCallStatus callStatus) {
     this.onlineRouter = onlineRouter;
     this.offlineRouter = offlineRouter;
     this.connectivityStatus = connectivityStatus;
-    this.listener = listener;
     this.routeComparator = routeComparator;
+    this.listener = listener;
+    this.callStatus = callStatus;
     onlineRouter.addRouteListener(this);
   }
 
   @Override
   public void onResponseReceived(DirectionsResponse response, @Nullable RouteProgress routeProgress) {
-    routeComparator.compare(response, currentRoute);
-    isRouting = false;
+    if (validRouteResponse(response)) {
+      routeComparator.compare(response, currentRoute);
+    }
+    updateCallStatusReceived();
   }
 
   @Override
   public void onErrorReceived(Throwable throwable) {
     onRequestError(throwable.getMessage());
-    isRouting = false;
+    updateCallStatusReceived();
   }
 
   void extractRouteOptions(NavigationViewOptions options) {
@@ -70,19 +74,16 @@ void extractRouteOptions(NavigationViewOptions options) {
   }
 
   void findRouteFrom(@Nullable RouteProgress routeProgress) {
-    if (isRouting) {
+    if (isRouting()) {
       return;
     }
     NavigationRoute.Builder builder = onlineRouter.buildRequestFrom(location, routeProgress);
     if (connectivityStatus.isConnectedFast()) {
-      onlineRouter.findRouteWith(builder);
-      isRouting = true;
+      findOnlineRouteWith(builder);
     } else if (isOfflineConfigured()) {
-      offlineRouter.findRouteWith(builder);
-      isRouting = true;
+      findOfflineRouteWith(builder);
     } else if (connectivityStatus.isConnected()) {
-      onlineRouter.findRouteWith(builder);
-      isRouting = true;
+      findOnlineRouteWith(builder);
     }
   }
 
@@ -95,6 +96,12 @@ void updateCurrentRoute(DirectionsRoute currentRoute) {
     listener.onRouteUpdate(currentRoute);
   }
 
+  void updateCallStatusReceived() {
+    if (callStatus != null) {
+      callStatus.setResponseReceived();
+    }
+  }
+
   void onRequestError(String errorMessage) {
     listener.onRouteRequestError(errorMessage);
   }
@@ -104,6 +111,10 @@ void onDestroy() {
     onlineRouter.clearListeners();
   }
 
+  private boolean validRouteResponse(DirectionsResponse response) {
+    return response != null && !response.routes().isEmpty();
+  }
+
   private void extractRouteFrom(NavigationViewOptions options) {
     DirectionsRoute route = options.directionsRoute();
     cacheRouteOptions(route.routeOptions());
@@ -115,6 +126,16 @@ private void cacheRouteOptions(RouteOptions routeOptions) {
     cacheRouteDestination();
   }
 
+  private void cacheRouteDestination() {
+    boolean hasValidCoordinates = routeOptions != null && !routeOptions.coordinates().isEmpty();
+    if (hasValidCoordinates) {
+      List<Point> coordinates = routeOptions.coordinates();
+      int destinationCoordinate = coordinates.size() - 1;
+      Point destinationPoint = coordinates.get(destinationCoordinate);
+      listener.onDestinationSet(destinationPoint);
+    }
+  }
+
   private void initializeOfflineFrom(NavigationViewOptions options) {
     String offlinePath = options.offlineRoutingTilesPath();
     String offlineTilesVersion = options.offlineRoutingTilesVersion();
@@ -131,13 +152,21 @@ private boolean isOfflineConfigured() {
     return offlineRouter != null && offlineRouter.isConfigured();
   }
 
-  private void cacheRouteDestination() {
-    boolean hasValidCoordinates = routeOptions != null && !routeOptions.coordinates().isEmpty();
-    if (hasValidCoordinates) {
-      List<Point> coordinates = routeOptions.coordinates();
-      int destinationCoordinate = coordinates.size() - 1;
-      Point destinationPoint = coordinates.get(destinationCoordinate);
-      listener.onDestinationSet(destinationPoint);
+  private void findOnlineRouteWith(NavigationRoute.Builder builder) {
+    onlineRouter.cancelRouteCall();
+    onlineRouter.findRouteWith(builder);
+    callStatus = new RouteCallStatus(new Date());
+  }
+
+  private void findOfflineRouteWith(NavigationRoute.Builder builder) {
+    offlineRouter.findRouteWith(builder);
+    callStatus = new RouteCallStatus(new Date());
+  }
+
+  private boolean isRouting() {
+    if (callStatus == null) {
+      return false;
     }
+    return callStatus.isRouting(new Date());
   }
 }

libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/OfflineRouteFoundCallback.java

@@ -17,10 +17,12 @@ class OfflineRouteFoundCallback implements OnOfflineRouteFoundCallback {
   @Override
   public void onRouteFound(@NonNull DirectionsRoute offlineRoute) {
     router.updateCurrentRoute(offlineRoute);
+    router.updateCallStatusReceived();
   }
 
   @Override
   public void onError(@NonNull OfflineError error) {
     router.onRequestError(error.getMessage());
+    router.updateCallStatusReceived();
   }
 }

libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/RouteCallStatus.java

@@ -0,0 +1,29 @@
+package com.mapbox.services.android.navigation.ui.v5;
+
+import java.util.Date;
+
+class RouteCallStatus {
+
+  private static final int FIVE_SECONDS_IN_MILLISECONDS = 5000;
+  private boolean responseReceived;
+  private final Date callDate;
+
+  RouteCallStatus(Date callDate) {
+    this.callDate = callDate;
+  }
+
+  void setResponseReceived() {
+    this.responseReceived = true;
+  }
+
+  boolean isRouting(Date currentDate) {
+    if (responseReceived) {
+      return false;
+    }
+    return diffInMilliseconds(callDate, currentDate) < FIVE_SECONDS_IN_MILLISECONDS;
+  }
+
+  private long diffInMilliseconds(Date callDate, Date currentDate) {
+    return currentDate.getTime() - callDate.getTime();
+  }
+}

libandroid-navigation-ui/src/test/java/com/mapbox/services/android/navigation/ui/v5/NavigationViewRouterTest.java

@@ -103,7 +103,8 @@ public void findRouteFrom_fastConnectionGoesToOnline() {
       null, // Null offline (simulate no data)
       status,
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
     router.updateLocation(mock(Location.class));
 
@@ -125,7 +126,8 @@ public void findRouteFrom_nullOfflineAndSlowConnectionGoesToOnline() {
       null, // Null offline (simulate no data)
       status,
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
     router.updateLocation(mock(Location.class));
 
@@ -148,7 +150,8 @@ public void findRouteFrom_slowConnectionGoesToOffline() {
       offlineRouter,
       status,
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
     router.updateLocation(mock(Location.class));
 
@@ -171,7 +174,8 @@ public void findRouteFrom_secondRequestIgnored() {
       offlineRouter,
       status,
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
     router.updateLocation(mock(Location.class));
 
@@ -189,7 +193,8 @@ public void onDestroy_clearsListeners() {
       mock(NavigationViewOfflineRouter.class),
       mock(ConnectivityStatusProvider.class),
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
 
     router.onDestroy();
@@ -205,7 +210,8 @@ public void onDestroy_cancelsOnlineRouteCall() {
       mock(NavigationViewOfflineRouter.class),
       mock(ConnectivityStatusProvider.class),
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
 
     router.onDestroy();
@@ -227,7 +233,8 @@ public void checksOfflineRouterIsConfiguredIfPathAndTilesVersionArePresent() thr
       offlineRouter,
       mock(ConnectivityStatusProvider.class),
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
 
     router.extractRouteOptions(options);
@@ -249,7 +256,8 @@ public void checksOfflineRouterIsNotConfiguredIfPathIsNull() throws Exception {
       offlineRouter,
       mock(ConnectivityStatusProvider.class),
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
 
     router.extractRouteOptions(options);
@@ -271,7 +279,8 @@ public void checksOfflineRouterIsNotConfiguredIfPathIsEmpty() throws Exception {
       offlineRouter,
       mock(ConnectivityStatusProvider.class),
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
 
     router.extractRouteOptions(options);
@@ -294,7 +303,8 @@ public void checksOfflineRouterIsNotConfiguredIfTilesVersionIsNull() throws Exce
       offlineRouter,
       mock(ConnectivityStatusProvider.class),
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
 
     router.extractRouteOptions(options);
@@ -317,7 +327,8 @@ public void checksOfflineRouterIsNotConfiguredIfTilesVersionIsEmpty() throws Exc
       offlineRouter,
       mock(ConnectivityStatusProvider.class),
       mock(RouteComparator.class),
-      mock(ViewRouteListener.class)
+      mock(ViewRouteListener.class),
+      mock(RouteCallStatus.class)
     );
 
     router.extractRouteOptions(options);

libandroid-navigation-ui/src/test/java/com/mapbox/services/android/navigation/ui/v5/OfflineRouteFoundCallbackTest.java

@@ -23,6 +23,17 @@ public void onRouteFound_routerIsUpdated() {
     verify(router).updateCurrentRoute(offlineRoute);
   }
 
+  @Test
+  public void onRouteFound_callStatusIsUpdated() {
+    NavigationViewRouter router = mock(NavigationViewRouter.class);
+    DirectionsRoute offlineRoute = mock(DirectionsRoute.class);
+    OfflineRouteFoundCallback callback = new OfflineRouteFoundCallback(router);
+
+    callback.onRouteFound(offlineRoute);
+
+    verify(router).updateCallStatusReceived();
+  }
+
   @Test
   public void onError_routerReceivesErrorMessage() {
     NavigationViewRouter router = mock(NavigationViewRouter.class);
@@ -35,4 +46,17 @@ public void onError_routerReceivesErrorMessage() {
 
     verify(router).onRequestError(eq(errorMessage));
   }
+
+  @Test
+  public void onError_callStatusIsUpdated() {
+    NavigationViewRouter router = mock(NavigationViewRouter.class);
+    OfflineError error = mock(OfflineError.class);
+    String errorMessage = "error message";
+    when(error.getMessage()).thenReturn(errorMessage);
+    OfflineRouteFoundCallback callback = new OfflineRouteFoundCallback(router);
+
+    callback.onError(error);
+
+    verify(router).updateCallStatusReceived();
+  }
 }

libandroid-navigation-ui/src/test/java/com/mapbox/services/android/navigation/ui/v5/RouteCallStatusTest.java

@@ -0,0 +1,49 @@
+package com.mapbox.services.android.navigation.ui.v5;
+
+import org.junit.Test;
+
+import java.util.Date;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class RouteCallStatusTest {
+
+  @Test
+  public void setResponseReceived_isNoLongerRouting() {
+    Date callDate = mock(Date.class);
+    RouteCallStatus callStatus = new RouteCallStatus(callDate);
+
+    callStatus.setResponseReceived();
+
+    assertFalse(callStatus.isRouting(mock(Date.class)));
+  }
+
+  @Test
+  public void isRouting_returnsTrueUnderTwoSeconds() {
+    Date callDate = mock(Date.class);
+    when(callDate.getTime()).thenReturn(0L);
+    Date currentDate = mock(Date.class);
+    when((currentDate.getTime())).thenReturn(1000L);
+    RouteCallStatus callStatus = new RouteCallStatus(callDate);
+
+    boolean isRouting = callStatus.isRouting(currentDate);
+
+    assertTrue(isRouting);
+  }
+
+  @Test
+  public void isRouting_returnsFalseOverFiveSeconds() {
+    Date callDate = mock(Date.class);
+    when(callDate.getTime()).thenReturn(0L);
+    Date currentDate = mock(Date.class);
+    when((currentDate.getTime())).thenReturn(5100L);
+    RouteCallStatus callStatus = new RouteCallStatus(callDate);
+
+    boolean isRouting = callStatus.isRouting(currentDate);
+
+    assertFalse(isRouting);
+  }
+}

libandroid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/route/RouteFetcher.java

@@ -215,9 +215,6 @@ private boolean invalid(Context context, Location location, RouteProgress routeP
   private Callback<DirectionsResponse> directionsResponseCallback = new Callback<DirectionsResponse>() {
     @Override
     public void onResponse(@NonNull Call<DirectionsResponse> call, @NonNull Response<DirectionsResponse> response) {
-      if (!response.isSuccessful()) {
-        return;
-      }
       updateListeners(response.body(), routeProgress);
     }