Skip to content
This repository has been archived by the owner on Apr 23, 2023. It is now read-only.

Commit

Permalink
Shutdown Client Manager (#234)
Browse files Browse the repository at this point in the history
* inject client manager

* clear client manager location data to always report initial loc when service started

* add reported changes tests
  • Loading branch information
sarahsnow1 authored and tallytalwar committed Jul 18, 2017
1 parent 338575f commit 8efddea
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.mapzen.android.lost.internal.FusedLocationServiceConnectionManager;
import com.mapzen.android.lost.internal.GeofencingApiImpl;
import com.mapzen.android.lost.internal.GeofencingServiceIntentFactory;
import com.mapzen.android.lost.internal.LostClientManager;
import com.mapzen.android.lost.internal.LostRequestManager;
import com.mapzen.android.lost.internal.PendingIntentIdGenerator;
import com.mapzen.android.lost.internal.SettingsApiImpl;
Expand All @@ -20,7 +21,8 @@ public class LocationServices {
*/
public static final FusedLocationProviderApi FusedLocationApi =
new FusedLocationProviderApiImpl(new FusedLocationServiceConnectionManager(),
new FusedLocationServiceCallbackManager(), LostRequestManager.shared());
new FusedLocationServiceCallbackManager(), LostRequestManager.shared(),
LostClientManager.shared());

/**
* Entry point for APIs concerning geofences.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

/**
* Used by {@link LostApiClientImpl} to manage connected clients and by
* {@link FusedLocationProviderServiceDelegate} to manage client's {@link LocationListener}s,
* {@link FusedLocationProviderApiImpl} to manage client's {@link LocationListener}s,
* {@link PendingIntent}s, and {@link LocationCallback}s.
*/
public interface ClientManager {
Expand All @@ -41,6 +41,7 @@ ReportedChanges sendPendingIntent(Context context, Location location,
void updateReportedValues(ReportedChanges changes);
void notifyLocationAvailability(final LocationAvailability availability);
boolean hasNoListeners();
void shutdown();
Map<LostApiClient, Set<LocationListener>> getLocationListeners();
Map<LostApiClient, Set<PendingIntent>> getPendingIntents();
Map<LostApiClient, Set<LocationCallback>> getLocationCallbacks();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public class FusedLocationProviderApiImpl extends ApiImpl
private FusedLocationServiceConnectionManager serviceConnectionManager;
private FusedLocationServiceCallbackManager serviceCallbackManager;
private RequestManager requestManager;
private ClientManager clientManager;
private boolean isBound;

IFusedLocationProviderService service;
Expand All @@ -56,7 +57,6 @@ public void onLocationChanged(final Location location) throws RemoteException {
@Override public void run() {
// #224: this call is async, service may have been legally set to null in the meantime
if (service != null) {
final LostClientManager clientManager = LostClientManager.shared();
serviceCallbackManager.onLocationChanged(context, location, clientManager, service);
}
}
Expand All @@ -65,7 +65,6 @@ public void onLocationChanged(final Location location) throws RemoteException {

@Override public void onLocationAvailabilityChanged(LocationAvailability locationAvailability)
throws RemoteException {
LostClientManager clientManager = LostClientManager.shared();
serviceCallbackManager.onLocationAvailabilityChanged(locationAvailability, clientManager);
}
};
Expand All @@ -88,7 +87,7 @@ public void onLocationChanged(final Location location) throws RemoteException {
context.unbindService(this);
isBound = false;
}

clientManager.shutdown();
service = null;
}

Expand All @@ -103,10 +102,12 @@ public void onLocationChanged(final Location location) throws RemoteException {
}

public FusedLocationProviderApiImpl(FusedLocationServiceConnectionManager connectionManager,
FusedLocationServiceCallbackManager callbackManager, RequestManager requestManager) {
FusedLocationServiceCallbackManager callbackManager, RequestManager requestManager,
ClientManager clientManager) {
serviceConnectionManager = connectionManager;
serviceCallbackManager = callbackManager;
this.requestManager = requestManager;
this.clientManager = clientManager;
serviceConnectionManager.setEventCallbacks(this);
}

Expand Down Expand Up @@ -158,7 +159,7 @@ public boolean isConnected() {
LocationRequest request, LocationListener listener) {
throwIfNotConnected(client);
requestManager.requestLocationUpdates(client, request, listener);
LostClientManager.shared().addListener(client, request, listener);
clientManager.addListener(client, request, listener);
requestLocationUpdatesInternal(request);
return new SimplePendingResult(true);
}
Expand All @@ -172,7 +173,7 @@ public boolean isConnected() {
LocationRequest request, LocationCallback callback, Looper looper) {
throwIfNotConnected(client);
requestManager.requestLocationUpdates(client, request, callback);
LostClientManager.shared().addLocationCallback(client, request, callback, looper);
clientManager.addLocationCallback(client, request, callback, looper);
requestLocationUpdatesInternal(request);
return new SimplePendingResult(true);
}
Expand All @@ -181,7 +182,7 @@ public boolean isConnected() {
LocationRequest request, PendingIntent callbackIntent) {
throwIfNotConnected(client);
requestManager.requestLocationUpdates(client, request, callbackIntent);
LostClientManager.shared().addPendingIntent(client, request, callbackIntent);
clientManager.addPendingIntent(client, request, callbackIntent);
requestLocationUpdatesInternal(request);
return new SimplePendingResult(true);
}
Expand Down Expand Up @@ -214,7 +215,7 @@ private void removeLocationUpdatesInternal(List<LocationRequest> requests) {
throwIfNotConnected(client);
List<LocationRequest> requests = requestManager.removeLocationUpdates(client, listener);
removeLocationUpdatesInternal(requests);
boolean hasResult = LostClientManager.shared().removeListener(client, listener);
boolean hasResult = clientManager.removeListener(client, listener);
checkAllListenersPendingIntentsAndCallbacks();
return new SimplePendingResult(hasResult);
}
Expand All @@ -224,7 +225,7 @@ private void removeLocationUpdatesInternal(List<LocationRequest> requests) {
throwIfNotConnected(client);
List<LocationRequest> requests = requestManager.removeLocationUpdates(client, callbackIntent);
removeLocationUpdatesInternal(requests);
boolean hasResult = LostClientManager.shared().removePendingIntent(client, callbackIntent);
boolean hasResult = clientManager.removePendingIntent(client, callbackIntent);
checkAllListenersPendingIntentsAndCallbacks();
return new SimplePendingResult(hasResult);
}
Expand All @@ -234,7 +235,7 @@ private void removeLocationUpdatesInternal(List<LocationRequest> requests) {
throwIfNotConnected(client);
List<LocationRequest> requests = requestManager.removeLocationUpdates(client, callback);
removeLocationUpdatesInternal(requests);
boolean hasResult = LostClientManager.shared().removeLocationCallback(client, callback);
boolean hasResult = clientManager.removeLocationCallback(client, callback);
checkAllListenersPendingIntentsAndCallbacks();
return new SimplePendingResult(hasResult);
}
Expand All @@ -245,7 +246,7 @@ private void removeLocationUpdatesInternal(List<LocationRequest> requests) {
*/
private void checkAllListenersPendingIntentsAndCallbacks() {
//TODO: potentially remove hasNoListeners method, not needed anymore
//if (LostClientManager.shared().hasNoListeners()) {
//if (clientManager.hasNoListeners()) {
// try {
// service.removeAllLocationUpdates();
// } catch (RemoteException e) {
Expand Down Expand Up @@ -288,7 +289,7 @@ private void checkAllListenersPendingIntentsAndCallbacks() {
}

public Map<LostApiClient, Set<LocationListener>> getLocationListeners() {
return LostClientManager.shared().getLocationListeners();
return clientManager.getLocationListeners();
}

void removeConnectionCallbacks(LostApiClient.ConnectionCallbacks callbacks) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class FusedLocationServiceCallbackManager {
* @param clientManager
* @param service
*/
void onLocationChanged(Context context, Location location, LostClientManager clientManager,
void onLocationChanged(Context context, Location location, ClientManager clientManager,
IFusedLocationProviderService service) {

ReportedChanges changes = clientManager.reportLocationChanged(location);
Expand All @@ -53,13 +53,13 @@ void onLocationChanged(Context context, Location location, LostClientManager cli
}

/**
* Handles notifying all registered {@link LocationCallback}s that {@link LocationAvailability}
* has changed.
* Handles notifying all registered {@link com.mapzen.android.lost.api.LocationCallback}s that
* {@link LocationAvailability} has changed.
* @param locationAvailability
* @param clientManager
*/
void onLocationAvailabilityChanged(LocationAvailability locationAvailability,
LostClientManager clientManager) {
ClientManager clientManager) {
clientManager.notifyLocationAvailability(locationAvailability);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

/**
* Used by {@link LostApiClientImpl} to manage connected clients and by
* {@link FusedLocationProviderServiceDelegate} to manage client's {@link LocationListener}s,
* {@link FusedLocationProviderApiImpl} to manage client's {@link LocationListener}s,
* {@link PendingIntent}s, and {@link LocationCallback}s.
*/
public class LostClientManager implements ClientManager {
Expand Down Expand Up @@ -212,6 +212,10 @@ private void throwIfClientNotAdded(LostApiClient client) {
return true;
}

@Override public void shutdown() {
reportedChanges.clearAll();
}

@VisibleForTesting void clearClients() {
clients.clear();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,9 @@ public void putAll(ReportedChanges changes) {
updatedRequestToReportedTime.putAll(changes.timeChanges());
updatedRequestToReportedLocation.putAll(changes.locationChanges());
}

public void clearAll() {
updatedRequestToReportedTime.clear();
updatedRequestToReportedLocation.clear();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,16 @@ public class FusedLocationProviderApiImplTest extends BaseRobolectricTest {
private FusedLocationProviderApiImpl api;
private IFusedLocationProviderService service = mock(IFusedLocationProviderService.class);
private FusedLocationServiceConnectionManager connectionManager;
private ClientManager clientManager = new LostClientManager();

@Before public void setUp() throws Exception {
LostClientManager.shared().clearClients();
connectedClient = new LostApiClient.Builder(RuntimeEnvironment.application).build();
connectedClient = new LostApiClientImpl(RuntimeEnvironment.application, null,
clientManager);
connectedClient.connect();

// do not call connect on this!
disconnectedClient = new LostApiClient.Builder(RuntimeEnvironment.application).build();
disconnectedClient = new LostApiClientImpl(RuntimeEnvironment.application, null,
clientManager);

connectionManager = spy(new FusedLocationServiceConnectionManager());
requestManager = mock(RequestManager.class);
Expand All @@ -67,7 +69,7 @@ public class FusedLocationProviderApiImplTest extends BaseRobolectricTest {
Mockito.doCallRealMethod().when(connectionManager).connect(any(Context.class), any(
LostApiClient.ConnectionCallbacks.class));
api = new FusedLocationProviderApiImpl(connectionManager,
new FusedLocationServiceCallbackManager(), requestManager);
new FusedLocationServiceCallbackManager(), requestManager, clientManager);
api.connect(application, null);
api.service = service;
}
Expand Down Expand Up @@ -317,6 +319,16 @@ public void setMockTrace_shouldThrowIfNotConnected() throws Exception {
verify(service).remove(api.remoteCallback);
}

@Test public void onDisconnect_shouldShutdownClientManager() throws Exception {
Context context = mock(Context.class);
clientManager = mock(LostClientManager.class);
FusedLocationProviderApiImpl apiImpl = new FusedLocationProviderApiImpl(connectionManager,
new FusedLocationServiceCallbackManager(), requestManager, clientManager);
apiImpl.onConnect(context);
apiImpl.onDisconnect();
verify(clientManager).shutdown();
}

@Test public void removeLocationUpdates_shouldReturnStatusSuccessIfListenerRemoved() {
TestResultCallback callback = new TestResultCallback();
TestLocationListener listener = new TestLocationListener();
Expand All @@ -329,7 +341,7 @@ public void setMockTrace_shouldThrowIfNotConnected() throws Exception {
@Test public void removeLocationUpdates_shouldNotReturnStatusSuccessIfListenerNotRemoved() {
TestResultCallback callback = new TestResultCallback();
TestLocationListener listener = new TestLocationListener();
LostClientManager.shared().removeListener(connectedClient, listener);
clientManager.removeListener(connectedClient, listener);
PendingResult<Status> result = api.removeLocationUpdates(connectedClient, listener);
result.setResultCallback(callback);
assertThat(callback.status).isNull();
Expand Down Expand Up @@ -374,7 +386,8 @@ public void setMockTrace_shouldThrowIfNotConnected() throws Exception {
TestLocationListener listener = new TestLocationListener();
api.requestLocationUpdates(connectedClient, LocationRequest.create(), listener);

LostApiClient otherClient = new LostApiClient.Builder(RuntimeEnvironment.application).build();
LostApiClient otherClient = new LostApiClientImpl(RuntimeEnvironment.application, null,
clientManager);
otherClient.connect();
api.requestLocationUpdates(otherClient, LocationRequest.create(), listener);
api.removeLocationUpdates(connectedClient, listener);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package com.mapzen.android.lost.internal;

import com.mapzen.android.lost.api.LocationRequest;

import org.junit.Before;
import org.junit.Test;

import android.location.Location;

import java.util.HashMap;
import java.util.Map;

import static org.fest.assertions.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

public class ReportedChangesTest {

Map<LocationRequest, Long> timeChanges = new HashMap<>();
Map<LocationRequest, Location> locationChanges = new HashMap<>();
ReportedChanges changes;

@Before public void setup() {
changes = new ReportedChanges(timeChanges, locationChanges);
}

@Test public void putAll_shouldUpdateTimeChanges() {
Map<LocationRequest, Long> otherTimeChanges = new HashMap<>();
LocationRequest locRequest = LocationRequest.create(new TestPidReader());
otherTimeChanges.put(locRequest, 1234L);
Map<LocationRequest, Location> otherLocationChanges = new HashMap<>();
ReportedChanges otherChanges = new ReportedChanges(otherTimeChanges, otherLocationChanges);
changes.putAll(otherChanges);
assertThat(changes.timeChanges().get(locRequest)).isEqualTo(1234L);
}

@Test public void putAll_shouldUpdateLocationChanges() {
Map<LocationRequest, Long> otherTimeChanges = new HashMap<>();
Map<LocationRequest, Location> otherLocationChanges = new HashMap<>();
LocationRequest locRequest = LocationRequest.create(new TestPidReader());
Location loc = mock(Location.class);
otherLocationChanges.put(locRequest, loc);
ReportedChanges otherChanges = new ReportedChanges(otherTimeChanges, otherLocationChanges);
changes.putAll(otherChanges);
assertThat(changes.locationChanges().get(locRequest)).isEqualTo(loc);
}

@Test public void clearAll_shouldClearTimeChanges() {
Map<LocationRequest, Long> otherTimeChanges = new HashMap<>();
LocationRequest locRequest = LocationRequest.create(new TestPidReader());
otherTimeChanges.put(locRequest, 1234L);
Map<LocationRequest, Location> otherLocationChanges = new HashMap<>();
Location loc = mock(Location.class);
otherLocationChanges.put(locRequest, loc);
ReportedChanges otherChanges = new ReportedChanges(otherTimeChanges, otherLocationChanges);
changes.putAll(otherChanges);
changes.clearAll();
assertThat(changes.timeChanges()).isEmpty();
}

@Test public void clearAll_shouldClearLocationChanges() {
Map<LocationRequest, Long> otherTimeChanges = new HashMap<>();
LocationRequest locRequest = LocationRequest.create(new TestPidReader());
otherTimeChanges.put(locRequest, 1234L);
Map<LocationRequest, Location> otherLocationChanges = new HashMap<>();
Location loc = mock(Location.class);
otherLocationChanges.put(locRequest, loc);
ReportedChanges otherChanges = new ReportedChanges(otherTimeChanges, otherLocationChanges);
changes.putAll(otherChanges);
changes.clearAll();
assertThat(changes.locationChanges()).isEmpty();
}

}

0 comments on commit 8efddea

Please sign in to comment.