Weather API: Delegate the responsability of rejecting back to back requests [1/2]

The Weather Manager Service should not impose rules regarding how
a weather provide service wants to react to back to back weather
update requests, nor impose the time a caller should wait before
a new request can be submitted. These constraints vary between
weather services, so it's up the each implementation to enforce
these constraints.

With this patch, the Weather manager service will pass the requests
as they come from the requester and it will be up to the active service
provider decide whether process or reject the request.

Changes to API:
- Moved the request statuses to new inner class RequestStatus in
 CMWeatherManager
- Pass status arg to onLookupCityRequestCompleted()
- Added reject(int) method to ServiceRequest

Change-Id: I3512490688255e25395e955d506fe42ed52f8fe0
TICKET: CYNGNOS-2430

1 files changed, 31 insertions, 83 deletions

diff --git a/cm/lib/main/java/org/cyanogenmod/platform/internal/CMWeatherManagerService.java b/cm/lib/main/java/org/cyanogenmod/platform/internal/CMWeatherManagerService.java
index 69392fd..856bf2a 100644
--- a/cm/lib/main/java/org/cyanogenmod/platform/internal/CMWeatherManagerService.java
+++ b/cm/lib/main/java/org/cyanogenmod/platform/internal/CMWeatherManagerService.java
@@ -31,7 +31,6 @@ import android.os.Binder;
 import android.os.IBinder;
 import android.os.RemoteCallbackList;
 import android.os.RemoteException;
-import android.os.SystemClock;
 import android.os.UserHandle;
 import android.text.TextUtils;
 import android.util.Slog;
@@ -58,16 +57,9 @@ import java.util.List;
 public class CMWeatherManagerService extends SystemService{
 
     private static final String TAG = CMWeatherManagerService.class.getSimpleName();
-    /**
-     * How long clients will have to wait until a new weather update request can be honored
-     * TODO Allow weather service providers to specify this threshold
-     */
-    private static final long REQUEST_THRESHOLD_MILLIS = 1000L * 60L * 10L;
 
     private IWeatherProviderService mWeatherProviderService;
     private boolean mIsWeatherProviderServiceBound;
-    private long mLastWeatherUpdateRequestTimestamp = -REQUEST_THRESHOLD_MILLIS;
-    private boolean mIsProcessingRequest = false;
     private Object mMutex = new Object();
     private Context mContext;
     private final RemoteCallbackList<IWeatherServiceProviderChangeListener> mProviderChangeListeners
@@ -78,12 +70,16 @@ public class CMWeatherManagerService extends SystemService{
             = new IWeatherProviderServiceClient.Stub() {
         @Override
         public void setServiceRequestState(RequestInfo requestInfo,
-                ServiceRequestResult result, int state) {
+                ServiceRequestResult result, int status) {
             synchronized (mMutex) {
 
                 if (requestInfo == null) {
                     //Invalid request info object
-                    mIsProcessingRequest = false;
+                    return;
+                }
+
+                if (!isValidRequestInfoStatus(status)) {
+                    //Invalid request status
                     return;
                 }
 
@@ -93,19 +89,14 @@ public class CMWeatherManagerService extends SystemService{
                 switch (requestType) {
                     case RequestInfo.TYPE_WEATHER_BY_GEO_LOCATION_REQ:
                     case RequestInfo.TYPE_WEATHER_BY_WEATHER_LOCATION_REQ:
-                        if (!isValidRequestInfoState(requestType, state)) {
-                            //We received an invalid state, silently disregard the request
-                            mIsProcessingRequest = false;
-                            return;
-                        }
                         WeatherInfo weatherInfo = null;
-                        if (state == CMWeatherManager.WEATHER_REQUEST_COMPLETED) {
+                        if (status == CMWeatherManager.RequestStatus.COMPLETED) {
                             weatherInfo = (result != null) ? result.getWeatherInfo() : null;
                             if (weatherInfo == null) {
                                 //This should never happen! WEATHER_REQUEST_COMPLETED is set
                                 //only if the weatherinfo object was not null when the request
                                 //was marked as completed
-                                state = CMWeatherManager.WEATHER_REQUEST_FAILED;
+                                status = CMWeatherManager.RequestStatus.FAILED;
                             } else {
                                 if (!requestInfo.isQueryOnlyWeatherRequest()) {
                                     final long identity = Binder.clearCallingIdentity();
@@ -119,7 +110,8 @@ public class CMWeatherManagerService extends SystemService{
                         }
                         if (isValidListener(listener)) {
                             try {
-                                listener.onWeatherRequestCompleted(requestInfo, state, weatherInfo);
+                                listener.onWeatherRequestCompleted(requestInfo, status,
+                                        weatherInfo);
                             } catch (RemoteException e) {
                             }
                         }
@@ -128,40 +120,25 @@ public class CMWeatherManagerService extends SystemService{
                         if (isValidListener(listener)) {
                             try {
                                 //Result might be null if the provider marked the request as failed
-                                listener.onLookupCityRequestCompleted(requestInfo,
+                                listener.onLookupCityRequestCompleted(requestInfo, status,
                                         result != null ? result.getLocationLookupList() : null);
                             } catch (RemoteException e) {
                             }
                         }
                         break;
                 }
-                mIsProcessingRequest = false;
             }
         }
     };
 
-    private boolean isValidRequestInfoState(int requestType, int state) {
-        switch (requestType) {
-            case RequestInfo.TYPE_WEATHER_BY_GEO_LOCATION_REQ:
-            case RequestInfo.TYPE_WEATHER_BY_WEATHER_LOCATION_REQ:
-                switch (state) {
-                    case CMWeatherManager.WEATHER_REQUEST_COMPLETED:
-                    case CMWeatherManager.WEATHER_REQUEST_SUBMITTED_TOO_SOON:
-                    case CMWeatherManager.WEATHER_REQUEST_FAILED:
-                    case CMWeatherManager.WEATHER_REQUEST_ALREADY_IN_PROGRESS:
-                        return true;
-                    default:
-                        return false;
-                }
-            case RequestInfo.TYPE_LOOKUP_CITY_NAME_REQ:
-                switch (state) {
-                    case CMWeatherManager.LOOKUP_REQUEST_COMPLETED:
-                    case CMWeatherManager.LOOKUP_REQUEST_FAILED:
-                    case CMWeatherManager.LOOKUP_REQUEST_NO_MATCH_FOUND:
-                        return true;
-                    default:
-                        return false;
-                }
+    private boolean isValidRequestInfoStatus(int state) {
+        switch (state) {
+            case CMWeatherManager.RequestStatus.COMPLETED:
+            case CMWeatherManager.RequestStatus.ALREADY_IN_PROGRESS:
+            case CMWeatherManager.RequestStatus.FAILED:
+            case CMWeatherManager.RequestStatus.NO_MATCH_FOUND:
+            case CMWeatherManager.RequestStatus.SUBMITTED_TOO_SOON:
+                return true;
             default:
                 return false;
         }
@@ -271,36 +248,14 @@ public class CMWeatherManagerService extends SystemService{
         }
     }
 
-    private boolean canProcessWeatherUpdateRequest(RequestInfo info, long currentTimeMillis) {
+    private boolean canProcessWeatherUpdateRequest(RequestInfo info) {
         final IRequestInfoListener listener = info.getRequestListener();
 
-        if ((mLastWeatherUpdateRequestTimestamp + REQUEST_THRESHOLD_MILLIS) > currentTimeMillis) {
-            if (listener != null && listener.asBinder().pingBinder()) {
-                try {
-                    listener.onWeatherRequestCompleted(info,
-                            CMWeatherManager.WEATHER_REQUEST_SUBMITTED_TOO_SOON, null);
-                } catch (RemoteException e) {
-                }
-            }
-            return false;
-        }
-
-        if (mIsProcessingRequest) {
-            if (listener != null && listener.asBinder().pingBinder()) {
-                try {
-                    listener.onWeatherRequestCompleted(info,
-                            CMWeatherManager.WEATHER_REQUEST_ALREADY_IN_PROGRESS, null);
-                } catch (RemoteException e) {
-                }
-            }
-            return false;
-        }
-
         if (!mIsWeatherProviderServiceBound) {
             if (listener != null && listener.asBinder().pingBinder()) {
                 try {
                     listener.onWeatherRequestCompleted(info,
-                            CMWeatherManager.WEATHER_REQUEST_FAILED, null);
+                            CMWeatherManager.RequestStatus.FAILED, null);
                 } catch (RemoteException e) {
                 }
             }
@@ -310,12 +265,7 @@ public class CMWeatherManagerService extends SystemService{
     }
 
     private synchronized void processWeatherUpdateRequest(RequestInfo info) {
-        final long currentTimeMillis = SystemClock.elapsedRealtime();
-
-        if (!canProcessWeatherUpdateRequest(info, currentTimeMillis)) return;
-
-        mLastWeatherUpdateRequestTimestamp = currentTimeMillis;
-        mIsProcessingRequest = true;
+        if (!canProcessWeatherUpdateRequest(info)) return;
         try {
             mWeatherProviderService.processWeatherUpdateRequest(info);
         } catch (RemoteException e) {
@@ -327,13 +277,13 @@ public class CMWeatherManagerService extends SystemService{
             final IRequestInfoListener listener = info.getRequestListener();
             if (listener != null && listener.asBinder().pingBinder()) {
                 try {
-                    listener.onLookupCityRequestCompleted(info, null);
+                    listener.onLookupCityRequestCompleted(info,
+                            CMWeatherManager.RequestStatus.FAILED, null);
                 } catch (RemoteException e) {
                 }
             }
             return;
         }
-
         try {
             mWeatherProviderService.processCityNameLookupRequest(info);
         } catch(RemoteException e){
@@ -368,9 +318,6 @@ public class CMWeatherManagerService extends SystemService{
         public void onServiceDisconnected(ComponentName name) {
             mWeatherProviderService = null;
             mIsWeatherProviderServiceBound = false;
-            //We can't talk to the current service anyway...
-            mIsProcessingRequest = false;
-            mLastWeatherUpdateRequestTimestamp = -REQUEST_THRESHOLD_MILLIS;
             Slog.d(TAG, "Connection with " + name.flattenToString() + " has been closed");
         }
     };
@@ -543,16 +490,17 @@ public class CMWeatherManagerService extends SystemService{
 
     private synchronized void disconnectClient() {
         if (mIsWeatherProviderServiceBound) {
-            if (mIsProcessingRequest) {
-                try {
-                    mWeatherProviderService.cancelOngoingRequests();
-                } catch (RemoteException e) {
-                }
-                mIsProcessingRequest = false;
+            //let's cancel any pending request
+            try {
+                mWeatherProviderService.cancelOngoingRequests();
+            } catch (RemoteException e) {
+                Slog.d(TAG, "Error occurred while trying to cancel ongoing requests");
             }
+            //Disconnect from client
             try {
                 mWeatherProviderService.setServiceClient(null);
             } catch (RemoteException e) {
+                Slog.d(TAG, "Error occurred while disconnecting client");
             }
 
             getContext().unbindService(mWeatherServiceProviderConnection);