Simplify service shutdown to prevent crash on screen rotation

lost-sample/src/main/java/com/example/lost/MultipleLocationListenerSingleClientActivity.java

@@ -101,15 +101,15 @@ private void initLocationTracking() {
       return;
     }
 
-    long interval = 3 * 60 * 1000; // 3 minutes
+    long interval = 30 * 1000; // 30 seconds
     LocationRequest request = LocationRequest.create()
         .setPriority(LocationRequest.PRIORITY_HIGH_ACCURACY)
         .setFastestInterval(interval)
         .setInterval(interval);
 
     LocationServices.FusedLocationApi.requestLocationUpdates(lostApiClient, request, listener);
 
-    interval = 30 * 1000; // 30 seconds
+    interval = 15 * 1000; // 15 seconds
     request = LocationRequest.create()
         .setPriority(LocationRequest.PRIORITY_HIGH_ACCURACY)
         .setFastestInterval(interval)

lost/src/main/java/com/mapzen/android/lost/internal/ClientManager.java

@@ -43,7 +43,6 @@ ReportedChanges sendPendingIntent(Context context, Location location,
   void reportProviderDisabled(String provider);
   void notifyLocationAvailability(final LocationAvailability availability);
   boolean hasNoListeners();
-  void shutdown();
   Map<LostApiClient, Set<LocationListener>> getLocationListeners();
   Map<LostApiClient, Set<PendingIntent>> getPendingIntents();
   Map<LostApiClient, Set<LocationCallback>> getLocationCallbacks();

lost/src/main/java/com/mapzen/android/lost/internal/FusedLocationProviderService.java

@@ -50,11 +50,6 @@ public FusedLocationProviderService getService() {
     serviceImpl = new FusedLocationProviderServiceImpl(this, LostClientManager.shared());
   }
 
-  @Override public void onDestroy() {
-    super.onDestroy();
-    serviceImpl.shutdown();
-  }
-
   public Location getLastLocation(LostApiClient client) {
     return serviceImpl.getLastLocation(client);
   }

lost/src/main/java/com/mapzen/android/lost/internal/FusedLocationProviderServiceImpl.java

@@ -40,11 +40,6 @@ public FusedLocationProviderServiceImpl(Context context, ClientManager manager)
     locationEngine = new FusionEngine(context, this);
   }
 
-  public void shutdown() {
-    locationEngine.setRequest(null);
-    clientManager.shutdown();
-  }
-
   public Location getLastLocation(LostApiClient client) {
     return locationEngine.getLastLocation();
   }

lost/src/main/java/com/mapzen/android/lost/internal/LostClientManager.java

@@ -13,6 +13,7 @@
 import android.location.Location;
 import android.os.Handler;
 import android.os.Looper;
+import android.support.annotation.VisibleForTesting;
 import android.util.Log;
 
 import java.util.HashMap;
@@ -55,37 +56,37 @@ public static LostClientManager shared() {
     return instance;
   }
 
-  public void addClient(LostApiClient client) {
+  @Override public void addClient(LostApiClient client) {
     clients.put(client, new LostClientWrapper(client));
   }
 
-  public void removeClient(LostApiClient client) {
+  @Override public void removeClient(LostApiClient client) {
     clients.remove(client);
   }
 
-  public boolean containsClient(LostApiClient client) {
+  @Override public boolean containsClient(LostApiClient client) {
     return clients.containsKey(client);
   }
 
-  public int numberOfClients() {
+  @Override public int numberOfClients() {
     return clients.size();
   }
 
-  public void addListener(LostApiClient client, LocationRequest request,
+  @Override public void addListener(LostApiClient client, LocationRequest request,
       LocationListener listener) {
     throwIfClientNotAdded(client);
     clients.get(client).locationListeners().add(listener);
     listenerToLocationRequests.put(listener, request);
   }
 
-  public void addPendingIntent(LostApiClient client, LocationRequest request,
+  @Override public void addPendingIntent(LostApiClient client, LocationRequest request,
       PendingIntent callbackIntent) {
     throwIfClientNotAdded(client);
     clients.get(client).pendingIntents().add(callbackIntent);
     intentToLocationRequests.put(callbackIntent, request);
   }
 
-  public void addLocationCallback(LostApiClient client, LocationRequest request,
+  @Override public void addLocationCallback(LostApiClient client, LocationRequest request,
       LocationCallback callback, Looper looper) {
     throwIfClientNotAdded(client);
     clients.get(client).locationCallbacks().add(callback);
@@ -99,7 +100,7 @@ private void throwIfClientNotAdded(LostApiClient client) {
     }
   }
 
-  public boolean removeListener(LostApiClient client, LocationListener listener) {
+  @Override public boolean removeListener(LostApiClient client, LocationListener listener) {
     final Set<LocationListener> listeners = clients.get(client).locationListeners();
     boolean removed = false;
 
@@ -112,7 +113,7 @@ public boolean removeListener(LostApiClient client, LocationListener listener) {
     return removed;
   }
 
-  public boolean removePendingIntent(LostApiClient client, PendingIntent callbackIntent) {
+  @Override public boolean removePendingIntent(LostApiClient client, PendingIntent callbackIntent) {
     final Set<PendingIntent> pendingIntents = clients.get(client).pendingIntents();
     boolean removed = false;
 
@@ -125,7 +126,7 @@ public boolean removePendingIntent(LostApiClient client, PendingIntent callbackI
     return removed;
   }
 
-  public boolean removeLocationCallback(LostApiClient client, LocationCallback callback) {
+  @Override public boolean removeLocationCallback(LostApiClient client, LocationCallback callback) {
     final Set<LocationCallback> callbacks = clients.get(client).locationCallbacks();
     boolean removed = false;
 
@@ -148,7 +149,7 @@ public boolean removeLocationCallback(LostApiClient client, LocationCallback cal
    * @param location
    * @return
    */
-  public ReportedChanges reportLocationChanged(final Location location) {
+  @Override public ReportedChanges reportLocationChanged(final Location location) {
     return iterateAndNotify(location, getLocationListeners(), listenerToLocationRequests,
         new Notifier<LocationListener>() {
           @Override public void notify(LostApiClient client, LocationListener listener) {
@@ -166,7 +167,7 @@ public ReportedChanges reportLocationChanged(final Location location) {
    * @param location
    * @return
    */
-  public ReportedChanges sendPendingIntent(final Context context,
+  @Override public ReportedChanges sendPendingIntent(final Context context,
       final Location location, final LocationAvailability availability,
       final LocationResult result) {
     return iterateAndNotify(location,
@@ -177,7 +178,7 @@ public ReportedChanges sendPendingIntent(final Context context,
         });
   }
 
-  public ReportedChanges reportLocationResult(Location location,
+  @Override public ReportedChanges reportLocationResult(Location location,
       final LocationResult result) {
     return iterateAndNotify(location,
         getLocationCallbacks(), callbackToLocationRequests, new Notifier<LocationCallback>() {
@@ -187,35 +188,35 @@ public ReportedChanges reportLocationResult(Location location,
         });
   }
 
-  public void updateReportedValues(ReportedChanges changes) {
+  @Override public void updateReportedValues(ReportedChanges changes) {
     reportedChanges.putAll(changes);
   }
 
-  public void reportProviderEnabled(String provider) {
+  @Override public void reportProviderEnabled(String provider) {
     for (LostClientWrapper wrapper : clients.values()) {
       for (LocationListener listener : wrapper.locationListeners()) {
         listener.onProviderEnabled(provider);
       }
     }
   }
 
-  public void reportProviderDisabled(String provider) {
+  @Override public void reportProviderDisabled(String provider) {
     for (LostClientWrapper wrapper : clients.values()) {
       for (LocationListener listener : wrapper.locationListeners()) {
         listener.onProviderDisabled(provider);
       }
     }
   }
 
-  public void notifyLocationAvailability(final LocationAvailability availability) {
+  @Override public void notifyLocationAvailability(final LocationAvailability availability) {
     for (LostClientWrapper wrapper : clients.values()) {
       for (LocationCallback callback : wrapper.locationCallbacks()) {
         notifyAvailability(wrapper.client(), callback, availability);
       }
     }
   }
 
-  public boolean hasNoListeners() {
+  @Override public boolean hasNoListeners() {
     for (LostClientWrapper wrapper : clients.values()) {
       if (!wrapper.locationListeners().isEmpty()
           || !wrapper.pendingIntents().isEmpty()
@@ -227,11 +228,11 @@ public boolean hasNoListeners() {
     return true;
   }
 
-  public void shutdown() {
+  @VisibleForTesting void clearClients() {
     clients.clear();
   }
 
-  public Map<LostApiClient, Set<LocationListener>> getLocationListeners() {
+  @Override public Map<LostApiClient, Set<LocationListener>> getLocationListeners() {
     final Map<LostApiClient, Set<LocationListener>> clientToListeners = new HashMap<>();
     for (LostApiClient client : clients.keySet()) {
       clientToListeners.put(client, clients.get(client).locationListeners());
@@ -240,7 +241,7 @@ public Map<LostApiClient, Set<LocationListener>> getLocationListeners() {
     return clientToListeners;
   }
 
-  public Map<LostApiClient, Set<PendingIntent>> getPendingIntents() {
+  @Override public Map<LostApiClient, Set<PendingIntent>> getPendingIntents() {
     final Map<LostApiClient, Set<PendingIntent>> clientToPendingIntents = new HashMap<>();
     for (LostApiClient client : clients.keySet()) {
       clientToPendingIntents.put(client, clients.get(client).pendingIntents());
@@ -249,7 +250,7 @@ public Map<LostApiClient, Set<PendingIntent>> getPendingIntents() {
     return clientToPendingIntents;
   }
 
-  public Map<LostApiClient, Set<LocationCallback>> getLocationCallbacks() {
+  @Override public Map<LostApiClient, Set<LocationCallback>> getLocationCallbacks() {
     final Map<LostApiClient, Set<LocationCallback>> clientToLocationCallbacks = new HashMap<>();
     for (LostApiClient client : clients.keySet()) {
       clientToLocationCallbacks.put(client, clients.get(client).locationCallbacks());

lost/src/test/java/com/mapzen/android/lost/internal/FusedLocationProviderServiceImplTest.java

@@ -80,7 +80,6 @@ public class FusedLocationProviderServiceImplTest extends BaseRobolectricTest {
   @After public void tearDown() {
     client.disconnect();
     otherClient.disconnect();
-    clientManager.shutdown();
   }
 
   private void mockService() {
@@ -649,28 +648,6 @@ private File getTestGpxTrace() throws IOException {
     assertThat(shadowLocationManager.getRequestLocationUpdateListeners()).isEmpty();
   }
 
-  @Test public void shutdown_shouldUnregisterLocationUpdateListeners() throws Exception {
-    api.requestLocationUpdates(client, LocationRequest.create(),
-        new TestLocationListener());
-
-    api.shutdown();
-    assertThat(shadowLocationManager.getRequestLocationUpdateListeners()).isEmpty();
-  }
-
-  @Test public void shutdown_shouldClearListeners() {
-    api.requestLocationUpdates(client, LocationRequest.create(),
-        new TestLocationListener());
-    api.shutdown();
-    assertThat(api.getLocationListeners()).isEmpty();
-  }
-
-  @Test public void shutdown_shouldClearPendingIntents() {
-    api.requestLocationUpdates(client, LocationRequest.create(),
-        mock(PendingIntent.class));
-    api.shutdown();
-    assertThat(api.getPendingIntents()).isEmpty();
-  }
-
   @Test public void requestLocationUpdates_shouldModifyOnlyClientListeners() {
     client.connect();
     api.requestLocationUpdates(client, LocationRequest.create(),

lost/src/test/java/com/mapzen/android/lost/internal/LostClientManagerTest.java

@@ -31,12 +31,12 @@
 @Config(constants = BuildConfig.class, sdk = 21, manifest = Config.NONE)
 public class LostClientManagerTest extends BaseRobolectricTest {
 
-  ClientManager manager = LostClientManager.shared();
+  LostClientManager manager = LostClientManager.shared();
   Context context = mock(Context.class);
   LostApiClient client = new LostApiClient.Builder(context).build();
 
   @After public void tearDown() {
-    manager.shutdown();
+    manager.clearClients();
   }
 
   @Test public void shouldHaveZeroClientCount() {
@@ -258,23 +258,4 @@ public void addLocationCallback_shouldThrowExceptionIfClientWasNotAdded() throws
     manager.removeClient(client);
     assertThat(manager.getLocationCallbacks().get(client)).isNull();
   }
-
-  @Test public void shutdown_shouldClearAllMaps() {
-    manager.addClient(client);
-    LocationRequest request = LocationRequest.create();
-    TestLocationListener listener = new TestLocationListener();
-    manager.addListener(client, request, listener);
-
-    PendingIntent pendingIntent = mock(PendingIntent.class);
-    manager.addPendingIntent(client, request, pendingIntent);
-
-    TestLocationCallback callback = new TestLocationCallback();
-    Looper looper = mock(Looper.class);
-    manager.addLocationCallback(client, request, callback, looper);
-
-    manager.shutdown();
-    assertThat(manager.getLocationListeners()).isEmpty();
-    assertThat(manager.getPendingIntents()).isEmpty();
-    assertThat(manager.getLocationCallbacks()).isEmpty();
-  }
 }