Throw IllegalStateException (#194)

* move exception check into ApiImpl

* throw exception in geofencingapi if not connected

lost/src/main/java/com/mapzen/android/lost/api/GeofencingApi.java

@@ -35,6 +35,7 @@ public interface GeofencingApi {
    * @param client Connected client to receive geofence updates for.
    * @param geofencingRequest Request containing geofences to receive updates for.
    * @param pendingIntent Intent to be notified when geofences are entered/exited.
+   * @throws IllegalStateException if the client is not connected at the time of this call.
    */
   @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
   PendingResult<Status> addGeofences(LostApiClient client, GeofencingRequest geofencingRequest,
@@ -51,6 +52,7 @@ PendingResult<Status> addGeofences(LostApiClient client, GeofencingRequest geofe
    * @param client Connected client to receive geofence updates for.
    * @param geofences Geofences to receive updates for.
    * @param pendingIntent Intent to be notified when geofences are entered/exited.
+   * @throws IllegalStateException if the client is not connected at the time of this call.
    */
   @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
   PendingResult<Status> addGeofences(LostApiClient client, List<Geofence> geofences,
@@ -61,6 +63,7 @@ PendingResult<Status> addGeofences(LostApiClient client, List<Geofence> geofence
    *
    * @param client Connected client to remove geofence updates for.
    * @param geofenceRequestIds Geofence ids to remove updates for.
+   * @throws IllegalStateException if the client is not connected at the time of this call.
    */
   @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
   PendingResult<Status> removeGeofences(LostApiClient client, List<String> geofenceRequestIds);
@@ -69,6 +72,7 @@ PendingResult<Status> addGeofences(LostApiClient client, List<Geofence> geofence
    * Removes geofences for a given {@link PendingIntent}
    * @param client Connected client to remove geofence updates for.
    * @param pendingIntent Intent to remove updates for.
+   * @throws IllegalStateException if the client is not connected at the time of this call.
    */
   @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
   PendingResult<Status> removeGeofences(LostApiClient client, PendingIntent pendingIntent);

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

@@ -0,0 +1,14 @@
+package com.mapzen.android.lost.internal;
+
+import com.mapzen.android.lost.api.LostApiClient;
+
+/**
+ * Superclass for all {@link com.mapzen.android.lost.api.LocationServices} implementations.
+ */
+class ApiImpl {
+  void throwIfNotConnected(LostApiClient client) {
+    if (!client.isConnected()) {
+      throw new IllegalStateException("LostApiClient is not connected.");
+    }
+  }
+}

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

@@ -29,7 +29,7 @@
 /**
  * Implementation of the {@link FusedLocationProviderApi}.
  */
-public class FusedLocationProviderApiImpl
+public class FusedLocationProviderApiImpl extends ApiImpl
     implements FusedLocationProviderApi, EventCallbacks, ServiceConnection {
 
   private Context context;
@@ -278,10 +278,4 @@ void removeConnectionCallbacks(LostApiClient.ConnectionCallbacks callbacks) {
   FusedLocationServiceConnectionManager getServiceConnectionManager() {
     return serviceConnectionManager;
   }
-
-  private void throwIfNotConnected(LostApiClient client) {
-    if (!client.isConnected()) {
-      throw new IllegalStateException("LostApiClient is not connected.");
-    }
-  }
 }

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

@@ -25,7 +25,7 @@
 /**
  * Implementation of the {@link GeofencingApi}.
  */
-public class GeofencingApiImpl implements GeofencingApi {
+public class GeofencingApiImpl extends ApiImpl implements GeofencingApi {
 
   private Context context;
   private LocationManager locationManager;
@@ -64,6 +64,7 @@ public void disconnect() {
   @Override
   public PendingResult<Status> addGeofences(LostApiClient client,
       GeofencingRequest geofencingRequest, PendingIntent pendingIntent) throws SecurityException {
+    throwIfNotConnected(client);
     List<Geofence> geofences = geofencingRequest.getGeofences();
     addGeofences(client, geofences, pendingIntent);
     return new SimplePendingResult(true);
@@ -72,6 +73,7 @@ public PendingResult<Status> addGeofences(LostApiClient client,
   @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
   @Override public PendingResult<Status> addGeofences(LostApiClient client,
       List<Geofence> geofences, PendingIntent pendingIntent) throws SecurityException {
+    throwIfNotConnected(client);
     for (Geofence geofence : geofences) {
       addGeofence(client, geofence, pendingIntent);
     }
@@ -81,9 +83,7 @@ public PendingResult<Status> addGeofences(LostApiClient client,
   @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
   private PendingResult<Status> addGeofence(LostApiClient client, Geofence geofence,
       PendingIntent pendingIntent) throws SecurityException {
-
     checkGeofence(geofence);
-
     int pendingIntentId = idGenerator.generateId();
     internalIntent = geofencingServiceIntentFactory.createIntent(context);
     internalIntent.addCategory(String.valueOf(pendingIntentId));
@@ -112,6 +112,7 @@ private PendingResult<Status> addGeofence(LostApiClient client, Geofence geofenc
   @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
   @Override public PendingResult<Status> removeGeofences(LostApiClient client,
       List<String> geofenceRequestIds) {
+    throwIfNotConnected(client);
     boolean hasResult = false;
     for (String geofenceRequestId : geofenceRequestIds) {
       if (pendingIntentMap.containsKey(geofenceRequestId)) {
@@ -132,6 +133,7 @@ private void removeGeofences(LostApiClient client, String geofenceRequestId)
   @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
   @Override public PendingResult<Status> removeGeofences(LostApiClient client,
       PendingIntent pendingIntent) throws SecurityException {
+    throwIfNotConnected(client);
     boolean hasResult = false;
     if (pendingIntentMap.values().contains(pendingIntent)) {
       hasResult = true;

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

@@ -39,6 +39,7 @@ public class GeofencingApiTest {
   IntentFactory geofencingIntentFactory;
   IntentFactory dwellIntentFactory;
   AlarmManager alarmManager;
+  LostApiClient disconnectedClient;
 
   @Before public void setUp() throws Exception {
     context = mock(Context.class);
@@ -51,7 +52,9 @@ public class GeofencingApiTest {
     geofencingApi = new GeofencingApiImpl(geofencingIntentFactory, dwellIntentFactory,
         new PendingIntentIdGenerator());
     geofencingApi.connect(context);
-    client = new LostApiClient.Builder(context).build();
+    client = mock(LostApiClientImpl.class);
+    when(client.isConnected()).thenReturn(true);
+    disconnectedClient = mock(LostApiClientImpl.class);
   }
 
   @Test public void shouldNotBeNull() throws Exception {
@@ -289,4 +292,24 @@ public void requestGeofence_shouldThrowExceptionForMissingLoitering() {
     PendingIntent intent = Mockito.mock(PendingIntent.class);
     geofencingApi.addGeofences(client, request, intent);
   }
+
+  @Test(expected = IllegalStateException.class)
+  public void addGeofences_request_shouldThrowException() {
+    geofencingApi.addGeofences(disconnectedClient, mock(GeofencingRequest.class), null);
+  }
+
+  @Test(expected = IllegalStateException.class)
+  public void addGeofences_list_shouldThrowException() {
+    geofencingApi.addGeofences(disconnectedClient, mock(ArrayList.class), null);
+  }
+
+  @Test(expected = IllegalStateException.class)
+  public void removeGeofences_intent_shouldThrowException() {
+    geofencingApi.removeGeofences(disconnectedClient, mock(PendingIntent.class));
+  }
+
+  @Test(expected = IllegalStateException.class)
+  public void removeGeofences_list_shouldThrowException() {
+    geofencingApi.removeGeofences(disconnectedClient, mock(ArrayList.class));
+  }
 }

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

@@ -4,6 +4,7 @@
 import com.mapzen.android.lost.api.Geofence;
 import com.mapzen.android.lost.api.GeofencingApi;
 import com.mapzen.android.lost.api.GeofencingIntentSender;
+import com.mapzen.android.lost.api.LostApiClient;
 import com.mapzen.lost.BuildConfig;
 
 import org.junit.Before;
@@ -34,6 +35,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
   GeofencingApiImpl geofencingApi;
   Context context;
   int geofenceId = 123;
+  LostApiClient client;
 
   @Before public void setup() {
     IdGenerator idGenerator = new TestIdGenerator();
@@ -44,6 +46,8 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
     when(context.getSystemService(Context.LOCATION_SERVICE)).thenReturn(
         mock(LocationManager.class));
     geofencingApi.connect(context);
+    client = mock(LostApiClient.class);
+    when(client.isConnected()).thenReturn(true);
   }
 
   @Test public void generateIntent_shouldHaveExtras() {
@@ -52,7 +56,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_ENTER, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     Intent intent = new Intent("");
     Bundle extras = new Bundle();
@@ -83,7 +87,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_ENTER, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isTrue();
@@ -102,7 +106,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_EXIT, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isFalse();
@@ -121,7 +125,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_ENTER | Geofence.GEOFENCE_TRANSITION_EXIT, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isTrue();
@@ -140,7 +144,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_DWELL, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isFalse();
@@ -159,7 +163,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_ENTER, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isFalse();
@@ -178,7 +182,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_EXIT, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isTrue();
@@ -197,7 +201,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_ENTER | Geofence.GEOFENCE_TRANSITION_EXIT, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isTrue();
@@ -216,7 +220,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_DWELL, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isFalse();
@@ -234,7 +238,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_ENTER, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isFalse();
@@ -252,7 +256,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_EXIT, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isFalse();
@@ -271,7 +275,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
             | Geofence.GEOFENCE_TRANSITION_DWELL, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isTrue();
@@ -289,7 +293,7 @@ public class GeofencingIntentSenderTest extends BaseRobolectricTest {
         Geofence.GEOFENCE_TRANSITION_DWELL, 0);
     ArrayList<Geofence> allGeofences = new ArrayList<>();
     allGeofences.add(geofence);
-    geofencingApi.addGeofences(null, allGeofences, null);
+    geofencingApi.addGeofences(client, allGeofences, null);
 
     boolean shouldSendIntent = intentSender.shouldSendIntent(intent);
     assertThat(shouldSendIntent).isTrue();