diff --git a/lost/src/main/java/com/mapzen/android/lost/api/GeofencingApi.java b/lost/src/main/java/com/mapzen/android/lost/api/GeofencingApi.java index ddecda6..e665794 100644 --- a/lost/src/main/java/com/mapzen/android/lost/api/GeofencingApi.java +++ b/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 addGeofences(LostApiClient client, GeofencingRequest geofencingRequest, @@ -51,6 +52,7 @@ PendingResult 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 addGeofences(LostApiClient client, List geofences, @@ -61,6 +63,7 @@ PendingResult addGeofences(LostApiClient client, List 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 removeGeofences(LostApiClient client, List geofenceRequestIds); @@ -69,6 +72,7 @@ PendingResult addGeofences(LostApiClient client, List 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 removeGeofences(LostApiClient client, PendingIntent pendingIntent); diff --git a/lost/src/main/java/com/mapzen/android/lost/internal/ApiImpl.java b/lost/src/main/java/com/mapzen/android/lost/internal/ApiImpl.java new file mode 100644 index 0000000..045f14c --- /dev/null +++ b/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."); + } + } +} diff --git a/lost/src/main/java/com/mapzen/android/lost/internal/FusedLocationProviderApiImpl.java b/lost/src/main/java/com/mapzen/android/lost/internal/FusedLocationProviderApiImpl.java index 8bbaa81..cb2884a 100644 --- a/lost/src/main/java/com/mapzen/android/lost/internal/FusedLocationProviderApiImpl.java +++ b/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."); - } - } } diff --git a/lost/src/main/java/com/mapzen/android/lost/internal/GeofencingApiImpl.java b/lost/src/main/java/com/mapzen/android/lost/internal/GeofencingApiImpl.java index add8122..7425a31 100644 --- a/lost/src/main/java/com/mapzen/android/lost/internal/GeofencingApiImpl.java +++ b/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 addGeofences(LostApiClient client, GeofencingRequest geofencingRequest, PendingIntent pendingIntent) throws SecurityException { + throwIfNotConnected(client); List geofences = geofencingRequest.getGeofences(); addGeofences(client, geofences, pendingIntent); return new SimplePendingResult(true); @@ -72,6 +73,7 @@ public PendingResult addGeofences(LostApiClient client, @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION}) @Override public PendingResult addGeofences(LostApiClient client, List geofences, PendingIntent pendingIntent) throws SecurityException { + throwIfNotConnected(client); for (Geofence geofence : geofences) { addGeofence(client, geofence, pendingIntent); } @@ -81,9 +83,7 @@ public PendingResult addGeofences(LostApiClient client, @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION}) private PendingResult 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 addGeofence(LostApiClient client, Geofence geofenc @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION}) @Override public PendingResult removeGeofences(LostApiClient client, List 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 removeGeofences(LostApiClient client, PendingIntent pendingIntent) throws SecurityException { + throwIfNotConnected(client); boolean hasResult = false; if (pendingIntentMap.values().contains(pendingIntent)) { hasResult = true; diff --git a/lost/src/test/java/com/mapzen/android/lost/internal/GeofencingApiTest.java b/lost/src/test/java/com/mapzen/android/lost/internal/GeofencingApiTest.java index 36d20e9..25e76c1 100644 --- a/lost/src/test/java/com/mapzen/android/lost/internal/GeofencingApiTest.java +++ b/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)); + } } diff --git a/lost/src/test/java/com/mapzen/android/lost/internal/GeofencingIntentSenderTest.java b/lost/src/test/java/com/mapzen/android/lost/internal/GeofencingIntentSenderTest.java index 23060a4..93a96e8 100644 --- a/lost/src/test/java/com/mapzen/android/lost/internal/GeofencingIntentSenderTest.java +++ b/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 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 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 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 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 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 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 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 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 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 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 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 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 allGeofences = new ArrayList<>(); allGeofences.add(geofence); - geofencingApi.addGeofences(null, allGeofences, null); + geofencingApi.addGeofences(client, allGeofences, null); boolean shouldSendIntent = intentSender.shouldSendIntent(intent); assertThat(shouldSendIntent).isTrue();