Weather API: Return ID rather than RequestInfo [1/2]

Instead of exposing the RequestInfo object created by the WeatherMgr
return an ID to identify the request. This ID can be later used to
cancel the request if needed. The WeatherProviderService base class
keeps track of the ongoing requests and can map this ID to the
corresponding request

This patch also include the following minor changes:
- Use List instead of ArrayList in API
- Update javadoc to public methods to reflect API changes
- Use UUID random generator in immutable classes to generate the
  hashcode rather than relying solely in the hashcode of the builder
  object.

Change-Id: Ib88dd0ecddd6fdb016b77ac29709fbae092dea29
TICKET: CYNGNOS-2425
TICKET: CYNGNOS-2423

5 files changed, 122 insertions, 87 deletions

diff --git a/sdk/src/java/cyanogenmod/weather/CMWeatherManager.java b/sdk/src/java/cyanogenmod/weather/CMWeatherManager.java
index 3ab510e..2613ea5 100644
--- a/sdk/src/java/cyanogenmod/weather/CMWeatherManager.java
+++ b/sdk/src/java/cyanogenmod/weather/CMWeatherManager.java
@@ -131,14 +131,14 @@ public class CMWeatherManager {
      * @param listener {@link WeatherUpdateRequestListener} To be notified once the active weather
      *                                                     service provider has finished
      *                                                     processing your request
-     * @return A {@link RequestInfo} identifying the request submitted to the weather service.
-     * Note that this method might return null if an error occurred while trying to submit
+     * @return An integer that identifies the request submitted to the weather service
+     * Note that this method might return -1 if an error occurred while trying to submit
      * the request.
      */
-    public RequestInfo requestWeatherUpdate(@NonNull Location location,
+    public int requestWeatherUpdate(@NonNull Location location,
             @NonNull WeatherUpdateRequestListener listener) {
         if (sWeatherManagerService == null) {
-            return null;
+            return -1;
         }
 
         try {
@@ -148,9 +148,9 @@ public class CMWeatherManager {
                     .build();
             if (listener != null) mWeatherUpdateRequestListeners.put(info, listener);
             sWeatherManagerService.updateWeather(info);
-            return info;
+            return info.hashCode();
         } catch (RemoteException e) {
-            return null;
+            return -1;
         }
     }
 
@@ -164,14 +164,14 @@ public class CMWeatherManager {
      * @param listener {@link WeatherUpdateRequestListener} To be notified once the active weather
      *                                                     service provider has finished
      *                                                     processing your request
-     * @return A {@link RequestInfo} identifying the request submitted to the weather service.
-     * Note that this method might return null if an error occurred while trying to submit
+     * @return An integer that identifies the request submitted to the weather service.
+     * Note that this method might return -1 if an error occurred while trying to submit
      * the request.
      */
-    public RequestInfo requestWeatherUpdate(@NonNull WeatherLocation weatherLocation,
+    public int requestWeatherUpdate(@NonNull WeatherLocation weatherLocation,
             @NonNull WeatherUpdateRequestListener listener) {
         if (sWeatherManagerService == null) {
-            return null;
+            return -1;
         }
 
         try {
@@ -181,9 +181,9 @@ public class CMWeatherManager {
                     .build();
             if (listener != null) mWeatherUpdateRequestListeners.put(info, listener);
             sWeatherManagerService.updateWeather(info);
-            return info;
+            return info.hashCode();
         } catch (RemoteException e) {
-            return null;
+            return -1;
         }
     }
 
@@ -195,13 +195,13 @@ public class CMWeatherManager {
      *                                                  completed. Upon success, a list of
      *                                                  {@link cyanogenmod.weather.WeatherLocation}
      *                                                  will be provided
-     * @return A {@link RequestInfo} identifying the request submitted to the weather service.
-     * Note that this method might return null if an error occurred while trying to submit
+     * @return An integer that identifies the request submitted to the weather service.
+     * Note that this method might return -1 if an error occurred while trying to submit
      * the request.
      */
-    public RequestInfo lookupCity(@NonNull String city, @NonNull LookupCityRequestListener listener) {
+    public int lookupCity(@NonNull String city, @NonNull LookupCityRequestListener listener) {
         if (sWeatherManagerService == null) {
-            return null;
+            return -1;
         }
         try {
             RequestInfo info = new RequestInfo
@@ -210,26 +210,23 @@ public class CMWeatherManager {
                     .build();
             if (listener != null) mLookupNameRequestListeners.put(info, listener);
             sWeatherManagerService.lookupCity(info);
-            return info;
+            return info.hashCode();
         } catch (RemoteException e) {
-            return null;
+            return -1;
         }
     }
 
     /**
      * Cancels a request that was previously submitted to the weather service.
-     * @param info The {@link RequestInfo} that you received when the request was submitted
+     * @param requestId The ID that you received when the request was submitted
      */
-    public void cancelRequest(RequestInfo info) {
+    public void cancelRequest(int requestId) {
         if (sWeatherManagerService == null) {
             return;
         }
-        if (info == null) {
-            return;
-        }
 
         try {
-            sWeatherManagerService.cancelRequest(info);
+            sWeatherManagerService.cancelRequest(requestId);
         }catch (RemoteException e){
         }
     }
@@ -345,11 +342,7 @@ public class CMWeatherManager {
                 mHandler.post(new Runnable() {
                     @Override
                     public void run() {
-                        ArrayList<WeatherLocation> list = null;
-                        if (weatherLocations != null) {
-                            list = new ArrayList<>(weatherLocations);
-                        }
-                        listener.onLookupCityRequestCompleted(list);
+                        listener.onLookupCityRequestCompleted(weatherLocations);
                     }
                 });
             }
@@ -386,7 +379,7 @@ public class CMWeatherManager {
          *
          * @param locations
          */
-        void onLookupCityRequestCompleted(ArrayList<WeatherLocation> locations);
+        void onLookupCityRequestCompleted(List<WeatherLocation> locations);
     }
 
     /**
diff --git a/sdk/src/java/cyanogenmod/weather/ICMWeatherManager.aidl b/sdk/src/java/cyanogenmod/weather/ICMWeatherManager.aidl
index 8caac49..c933ae6 100644
--- a/sdk/src/java/cyanogenmod/weather/ICMWeatherManager.aidl
+++ b/sdk/src/java/cyanogenmod/weather/ICMWeatherManager.aidl
@@ -28,5 +28,5 @@ interface ICMWeatherManager {
     oneway void unregisterWeatherServiceProviderChangeListener(
         in IWeatherServiceProviderChangeListener listener);
     String getActiveWeatherServiceProviderLabel();
-    oneway void cancelRequest(in RequestInfo info);
+    oneway void cancelRequest(int requestId);
 }
\ No newline at end of file
diff --git a/sdk/src/java/cyanogenmod/weather/RequestInfo.java b/sdk/src/java/cyanogenmod/weather/RequestInfo.java
index bb05301..44805a2 100644
--- a/sdk/src/java/cyanogenmod/weather/RequestInfo.java
+++ b/sdk/src/java/cyanogenmod/weather/RequestInfo.java
@@ -20,11 +20,14 @@ import android.location.Location;
 import android.os.Parcel;
 import android.os.Parcelable;
 
+import android.text.TextUtils;
 import cyanogenmod.os.Build;
 import cyanogenmod.os.Concierge;
 import cyanogenmod.os.Concierge.ParcelInfo;
 import cyanogenmod.providers.WeatherContract;
 
+import java.util.UUID;
+
 /**
  * This class holds the information of a request submitted to the active weather provider service
  */
@@ -36,7 +39,7 @@ public final class RequestInfo implements Parcelable {
     private int mRequestType;
     private IRequestInfoListener mListener;
     private int mTempUnit;
-    private int mKey;
+    private String mKey;
     private boolean mIsQueryOnly;
 
     /**
@@ -155,6 +158,10 @@ public final class RequestInfo implements Parcelable {
             return this;
         }
 
+        /**
+         * Combine all of the options that have been set and return a new {@link RequestInfo} object
+         * @return {@link RequestInfo}
+         */
         public RequestInfo build() {
             RequestInfo info = new RequestInfo();
             info.mListener = this.mListener;
@@ -164,7 +171,7 @@ public final class RequestInfo implements Parcelable {
             info.mLocation = this.mLocation;
             info.mTempUnit = this.mTempUnit;
             info.mIsQueryOnly = this.mIsQueryOnly;
-            info.mKey = this.hashCode();
+            info.mKey = UUID.randomUUID().toString();
             return info;
         }
 
@@ -186,7 +193,7 @@ public final class RequestInfo implements Parcelable {
         int parcelableVersion = parcelInfo.getParcelVersion();
 
         if (parcelableVersion >= Build.CM_VERSION_CODES.ELDERBERRY) {
-            mKey = parcel.readInt();
+            mKey = parcel.readString();
             mRequestType = parcel.readInt();
             switch (mRequestType) {
                 case TYPE_WEATHER_BY_GEO_LOCATION_REQ:
@@ -298,7 +305,7 @@ public final class RequestInfo implements Parcelable {
         ParcelInfo parcelInfo = Concierge.prepareParcel(dest);
 
         // ==== ELDERBERRY =====
-        dest.writeInt(mKey);
+        dest.writeString(mKey);
         dest.writeInt(mRequestType);
         switch (mRequestType) {
             case TYPE_WEATHER_BY_GEO_LOCATION_REQ:
@@ -352,18 +359,19 @@ public final class RequestInfo implements Parcelable {
 
     @Override
     public int hashCode() {
-        //The hashcode of this object was stored when it was built. This is an
-        //immutable object but we need to preserve the hashcode since this object is parcelable and
-        //it's reconstructed over IPC, and clients of this object might want to store it in a
-        //collection that relies on this code to identify the object
-        return mKey;
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ((mKey != null) ? mKey.hashCode() : 0);
+        return result;
     }
 
     @Override
     public boolean equals(Object obj) {
-        if (obj instanceof RequestInfo) {
+        if (obj == null) return false;
+
+        if (getClass() == obj.getClass()) {
             RequestInfo info = (RequestInfo) obj;
-            return (info.hashCode() == this.mKey);
+            return (TextUtils.equals(mKey, info.mKey));
         }
         return false;
     }
diff --git a/sdk/src/java/cyanogenmod/weather/WeatherInfo.java b/sdk/src/java/cyanogenmod/weather/WeatherInfo.java
index 95fb2b7..aaac116 100755
--- a/sdk/src/java/cyanogenmod/weather/WeatherInfo.java
+++ b/sdk/src/java/cyanogenmod/weather/WeatherInfo.java
@@ -19,6 +19,7 @@ package cyanogenmod.weather;
 import android.annotation.NonNull;
 import android.os.Parcel;
 import android.os.Parcelable;
+import android.text.TextUtils;
 import cyanogenmod.os.Build;
 import cyanogenmod.os.Concierge;
 import cyanogenmod.os.Concierge.ParcelInfo;
@@ -27,6 +28,8 @@ import cyanogenmod.weatherservice.ServiceRequest;
 import cyanogenmod.weatherservice.ServiceRequestResult;
 
 import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
 
 /**
  * This class represents the weather information that a
@@ -51,8 +54,8 @@ public final class WeatherInfo implements Parcelable {
     private double mWindDirection;
     private int mWindSpeedUnit;
     private long mTimestamp;
-    private ArrayList<DayForecast> mForecastList;
-    private int mKey;
+    private List<DayForecast> mForecastList;
+    private String mKey;
 
     private WeatherInfo() {}
 
@@ -71,7 +74,7 @@ public final class WeatherInfo implements Parcelable {
         private double mWindDirection = Double.NaN;
         private int mWindSpeedUnit = WeatherContract.WeatherColumns.WindSpeedUnit.MPH;
         private long mTimestamp = -1;
-        private ArrayList<DayForecast> mForecastList = new ArrayList<>(0);
+        private List<DayForecast> mForecastList = new ArrayList<>(0);
 
         /**
          * @param cityName A valid city name. Attempting to pass null will get you an
@@ -169,7 +172,7 @@ public final class WeatherInfo implements Parcelable {
          *                  null will get you an IllegalArgumentException'
          * @return The {@link Builder} instance
          */
-        public Builder setForecast(@NonNull ArrayList<DayForecast> forecasts) {
+        public Builder setForecast(@NonNull List<DayForecast> forecasts) {
             if (forecasts == null) {
                 throw new IllegalArgumentException("Forecast list can't be null");
             }
@@ -220,7 +223,7 @@ public final class WeatherInfo implements Parcelable {
             info.mWindSpeedUnit = this.mWindSpeedUnit;
             info.mTimestamp = this.mTimestamp == -1 ? System.currentTimeMillis() : this.mTimestamp;
             info.mForecastList = this.mForecastList;
-            info.mKey = this.hashCode();
+            info.mKey = UUID.randomUUID().toString();
             return info;
         }
 
@@ -338,7 +341,7 @@ public final class WeatherInfo implements Parcelable {
      * the forecast weather for the upcoming days. If you want to know today's high and low
      * temperatures, use {@link WeatherInfo#getTodaysHigh()} and {@link WeatherInfo#getTodaysLow()}
      */
-    public ArrayList<DayForecast> getForecasts() {
+    public List<DayForecast> getForecasts() {
         return new ArrayList<>(mForecastList);
     }
 
@@ -348,7 +351,7 @@ public final class WeatherInfo implements Parcelable {
         int parcelableVersion = parcelInfo.getParcelVersion();
 
         if (parcelableVersion >= Build.CM_VERSION_CODES.ELDERBERRY) {
-            mKey = parcel.readInt();
+            mKey = parcel.readString();
             mCity = parcel.readString();
             mConditionCode = parcel.readInt();
             mTemperature = parcel.readDouble();
@@ -383,7 +386,7 @@ public final class WeatherInfo implements Parcelable {
         ParcelInfo parcelInfo = Concierge.prepareParcel(dest);
 
         // ==== ELDERBERRY =====
-        dest.writeInt(mKey);
+        dest.writeString(mKey);
         dest.writeString(mCity);
         dest.writeInt(mConditionCode);
         dest.writeDouble(mTemperature);
@@ -428,7 +431,7 @@ public final class WeatherInfo implements Parcelable {
         double mLow;
         double mHigh;
         int mConditionCode;
-        int mKey;
+        String mKey;
 
         private DayForecast() {}
 
@@ -490,7 +493,7 @@ public final class WeatherInfo implements Parcelable {
                 forecast.mLow = this.mLow;
                 forecast.mHigh = this.mHigh;
                 forecast.mConditionCode = this.mConditionCode;
-                forecast.mKey = this.hashCode();
+                forecast.mKey = UUID.randomUUID().toString();
                 return forecast;
             }
         }
@@ -527,7 +530,7 @@ public final class WeatherInfo implements Parcelable {
             ParcelInfo parcelInfo = Concierge.prepareParcel(dest);
 
             // ==== ELDERBERRY =====
-            dest.writeInt(mKey);
+            dest.writeString(mKey);
             dest.writeDouble(mLow);
             dest.writeDouble(mHigh);
             dest.writeInt(mConditionCode);
@@ -555,7 +558,7 @@ public final class WeatherInfo implements Parcelable {
             int parcelableVersion = parcelInfo.getParcelVersion();
 
             if (parcelableVersion >= Build.CM_VERSION_CODES.ELDERBERRY) {
-                mKey = parcel.readInt();
+                mKey = parcel.readString();
                 mLow = parcel.readDouble();
                 mHigh = parcel.readDouble();
                 mConditionCode = parcel.readInt();
@@ -576,18 +579,19 @@ public final class WeatherInfo implements Parcelable {
 
         @Override
         public int hashCode() {
-            //The hashcode of this object was stored when it was built. This is an
-            //immutable object but we need to preserve the hashcode since this object is parcelable
-            //and it's reconstructed over IPC, and clients of this object might want to store it in
-            //a collection that relies on this code to identify the object
-            return mKey;
+            final int prime = 31;
+            int result = 1;
+            result = prime * result + ((mKey != null) ? mKey.hashCode() : 0);
+            return result;
         }
 
         @Override
         public boolean equals(Object obj) {
-            if (obj instanceof DayForecast) {
+            if (obj == null) return false;
+
+            if (getClass() == obj.getClass()) {
                 DayForecast forecast = (DayForecast) obj;
-                return (forecast.hashCode() == this.mKey);
+                return (TextUtils.equals(mKey, forecast.mKey));
             }
             return false;
         }
@@ -615,18 +619,19 @@ public final class WeatherInfo implements Parcelable {
 
     @Override
     public int hashCode() {
-        //The hashcode of this object was stored when it was built. This is an
-        //immutable object but we need to preserve the hashcode since this object is parcelable and
-        //it's reconstructed over IPC, and clients of this object might want to store it in a
-        //collection that relies on this code to identify the object
-        return mKey;
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ((mKey != null) ? mKey.hashCode() : 0);
+        return result;
     }
 
     @Override
     public boolean equals(Object obj) {
-        if (obj instanceof WeatherInfo) {
+        if (obj == null) return false;
+
+        if (getClass() == obj.getClass()) {
             WeatherInfo info = (WeatherInfo) obj;
-            return (info.hashCode() == this.mKey);
+            return (TextUtils.equals(mKey, info.mKey));
         }
         return false;
     }
diff --git a/sdk/src/java/cyanogenmod/weather/WeatherLocation.java b/sdk/src/java/cyanogenmod/weather/WeatherLocation.java
index 6cc4741..6befbb4 100644
--- a/sdk/src/java/cyanogenmod/weather/WeatherLocation.java
+++ b/sdk/src/java/cyanogenmod/weather/WeatherLocation.java
@@ -24,6 +24,8 @@ import cyanogenmod.os.Build;
 import cyanogenmod.os.Concierge;
 import cyanogenmod.os.Concierge.ParcelInfo;
 
+import java.util.UUID;
+
 /**
  * A class representing a geographical location that a weather service provider can use to
  * get weather data from. Each service provider will potentially populate objects of this class
@@ -37,7 +39,7 @@ public final class WeatherLocation implements Parcelable{
     private String mPostal;
     private String mCountryId;
     private String mCountry;
-    private int mKey;
+    private String mKey;
 
     private WeatherLocation() {}
 
@@ -57,7 +59,7 @@ public final class WeatherLocation implements Parcelable{
          * @param cityName The name of the city
          */
         public Builder(String cityId, String cityName) {
-            if (cityId == null && cityName == null) {
+            if (cityId == null || cityName == null) {
                 throw new IllegalArgumentException("Illegal to set city id AND city to null");
             }
             this.mCityId = cityId;
@@ -65,16 +67,36 @@ public final class WeatherLocation implements Parcelable{
         }
 
         /**
+         * @param cityName The name of the city
+         */
+        public Builder(String cityName) {
+            if (cityName == null) {
+                throw new IllegalArgumentException("City name can't be null");
+            }
+            this.mCity = cityName;
+        }
+
+        /**
          * @param countryId An identifier for the country (for example ISO alpha-2, ISO alpha-3,
          *                 ISO 3166-1 numeric-3, etc)
-         * @param country The country name
          * @return The {@link Builder} instance
          */
-        public Builder setCountry(String countryId, String country) {
-            if (countryId == null && country == null) {
-                throw new IllegalArgumentException("Illegal to set country id AND country to null");
+        public Builder setCountryId(String countryId) {
+            if (countryId == null) {
+                throw new IllegalArgumentException("Country ID can't be null");
             }
             this.mCountryId = countryId;
+            return this;
+        }
+
+        /**
+         * @param country The country name
+         * @return The {@link Builder} instance
+         */
+        public Builder setCountry(String country) {
+            if (country == null) {
+                throw new IllegalArgumentException("Country can't be null");
+            }
             this.mCountry = country;
             return this;
         }
@@ -116,48 +138,50 @@ public final class WeatherLocation implements Parcelable{
             weatherLocation.mPostal = this.mPostal;
             weatherLocation.mCountryId = this.mCountryId;
             weatherLocation.mCountry = this.mCountry;
-            weatherLocation.mKey = this.hashCode();
+            weatherLocation.mKey = UUID.randomUUID().toString();
             return weatherLocation;
         }
     }
 
     /**
-     * @return The city ID
+     * @return The city ID. This method will return an empty string if the city ID was not set
      */
     public String getCityId() {
         return mCityId;
     }
 
     /**
-     * @return The city name
+     * @return The city name. This method will return an empty string if the city name was not set
      */
     public String getCity() {
         return mCity;
     }
 
     /**
-     * @return The state name
+     * @return The state name. This method will return an empty string if the state was not set
      */
     public String getState() {
         return mState;
     }
 
     /**
-     * @return The postal/ZIP code
+     * @return The postal/ZIP code. This method will return an empty string if the postal/ZIP code
+     * was not set
      */
     public String getPostalCode() {
         return mPostal;
     }
 
     /**
-     * @return The country ID
+     * @return The country ID. This method will return an empty string if the country ID was not set
      */
     public String getCountryId() {
         return mCountryId;
     }
 
     /**
-     * @return The country name
+     * @return The country name. This method will return an empty string if the country ID was not
+     * set
      */
     public String getCountry() {
         return mCountry;
@@ -169,7 +193,7 @@ public final class WeatherLocation implements Parcelable{
         int parcelableVersion = parcelInfo.getParcelVersion();
 
         if (parcelableVersion >= Build.CM_VERSION_CODES.ELDERBERRY) {
-            mKey = in.readInt();
+            mKey = in.readString();
             mCityId = in.readString();
             mCity = in.readString();
             mState = in.readString();
@@ -205,7 +229,7 @@ public final class WeatherLocation implements Parcelable{
         ParcelInfo parcelInfo = Concierge.prepareParcel(dest);
 
         // ==== ELDERBERRY =====
-        dest.writeInt(mKey);
+        dest.writeString(mKey);
         dest.writeString(mCityId);
         dest.writeString(mCity);
         dest.writeString(mState);
@@ -231,14 +255,19 @@ public final class WeatherLocation implements Parcelable{
 
     @Override
     public int hashCode() {
-        return mKey;
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ((mKey != null) ? mKey.hashCode() : 0);
+        return result;
     }
 
     @Override
     public boolean equals(Object obj) {
-        if (obj instanceof WeatherLocation) {
-            WeatherLocation info = (WeatherLocation) obj;
-            return (info.hashCode() == this.mKey);
+        if (obj == null) return false;
+
+        if (getClass() == obj.getClass()) {
+            WeatherLocation location = (WeatherLocation) obj;
+            return (TextUtils.equals(mKey, location.mKey));
         }
         return false;
     }