Merge pull request #158 from firebase/fixes

Fixes and naming changes

.travis.yml

@@ -7,12 +7,6 @@ jdk: oraclejdk8
 sudo: required
 
 env:
-    matrix:
-        - ANDROID_TARGET=android-16 ANDROID_ABI=armeabi-v7a
-        - ANDROID_TARGET=android-17 ANDROID_ABI=armeabi-v7a
-        - ANDROID_TARGET=android-18 ANDROID_ABI=armeabi-v7a
-        - ANDROID_TARGET=android-19 ANDROID_ABI=armeabi-v7a
-        - ANDROID_TARGET=android-21 ANDROID_ABI=armeabi-v7a
     global:
         - ADB_INSTALL_TIMEOUT=10 # Default is 2 minutes, bump to 10
 
@@ -43,18 +37,6 @@ cache:
         - $HOME/.gradle/wrapper/
         - $HOME/.android/build-cache
 
-before_script:
-    # Print the list of available targets to aid in debugging CI issues
-    - android list targets
-    # Create the AVD (name "test")
-    - echo no | android create avd --force --name test --target $ANDROID_TARGET --abi $ANDROID_ABI
-    # Start the emulator in the background
-    - emulator -avd test -no-skin -no-audio -no-window &
-
 script:
-    - android-wait-for-emulator
-    - adb devices
-    # Simulate hitting the menu button
-    - adb shell input keyevent 82 &
     # Build the library + run unit tests, run device tests, build the test app
     - ./gradlew jobdispatcher:build testapp:assemble

README.md

@@ -79,7 +79,7 @@ Play services is unavailable.<br>
 Add the following to your `build.gradle`'s dependencies section:
 
 ```
-compile 'com.firebase:firebase-jobdispatcher:0.8.1'
+compile 'com.firebase:firebase-jobdispatcher:0.8.2'
 ```
 
 ### Usage

common/constants.gradle

@@ -2,7 +2,7 @@
 project.ext {
     projectName = "firebase-jobdispatcher-android"
     group = "com.firebase"
-    version = "0.8.1"
+    version = "0.8.2"
     buildtools = "25.0.0"
     supportLibraryVersion = "25.0.0"
     compileSdk = 25

jobdispatcher/src/main/java/com/firebase/jobdispatcher/ExecutionDelegator.java

@@ -44,7 +44,8 @@ interface JobFinishedCallback {
   }
 
   /** A mapping of {@link JobInvocation} to (local) binder connections. Synchronized by itself. */
-  private final SimpleArrayMap<JobInvocation, JobServiceConnection> serviceConnections =
+  @VisibleForTesting
+  static final SimpleArrayMap<JobInvocation, JobServiceConnection> serviceConnections =
       new SimpleArrayMap<>();
 
   private final ResponseHandler responseHandler =
@@ -69,7 +70,8 @@ boolean executeJob(JobInvocation jobInvocation) {
     }
 
     JobServiceConnection conn =
-        new JobServiceConnection(jobInvocation, responseHandler.obtainMessage(JOB_FINISHED));
+        new JobServiceConnection(
+            jobInvocation, responseHandler.obtainMessage(JOB_FINISHED), context);
 
     synchronized (serviceConnections) {
       JobServiceConnection oldConnection = serviceConnections.put(jobInvocation, conn);
@@ -87,30 +89,21 @@ private Intent createBindIntent(JobParameters jobParameters) {
     return execReq;
   }
 
-  void stopJob(JobInvocation job) {
+  static void stopJob(JobInvocation job, boolean needToSendResult) {
     synchronized (serviceConnections) {
       JobServiceConnection jobServiceConnection = serviceConnections.remove(job);
       if (jobServiceConnection != null) {
-        jobServiceConnection.onStop();
-        safeUnbindService(jobServiceConnection);
-      }
-    }
-  }
-
-  private void safeUnbindService(JobServiceConnection connection) {
-    if (connection != null && connection.isBound()) {
-      try {
-        context.unbindService(connection);
-      } catch (IllegalArgumentException e) {
-        Log.w(TAG, "Error unbinding service: " + e.getMessage());
+        jobServiceConnection.onStop(needToSendResult);
       }
     }
   }
 
   private void onJobFinishedMessage(JobInvocation jobInvocation, int result) {
     synchronized (serviceConnections) {
       JobServiceConnection connection = serviceConnections.remove(jobInvocation);
-      safeUnbindService(connection);
+      if (connection != null) {
+        connection.unbind();
+      }
     }
 
     jobFinishedCallback.onJobFinished(jobInvocation, result);

jobdispatcher/src/main/java/com/firebase/jobdispatcher/FirebaseJobDispatcher.java

@@ -55,20 +55,20 @@ public final class FirebaseJobDispatcher {
   /** Indicates the cancel request failed because the driver was unavailable. */
   public static final int CANCEL_RESULT_NO_DRIVER_AVAILABLE = 2;
   /** The backing Driver for this instance. */
-  private final Driver mDriver;
+  private final Driver driver;
   /** The ValidationEnforcer configured for the current Driver. */
-  private final ValidationEnforcer mValidator;
+  private final ValidationEnforcer validator;
   /**
    * Single instance of a RetryStrategy.Builder, configured with the current driver's validation
    * settings. We can do this because the RetryStrategy.Builder is stateless.
    */
-  private final RetryStrategy.Builder mRetryStrategyBuilder;
+  private final RetryStrategy.Builder retryStrategyBuilder;
 
   /** Instantiates a new FirebaseJobDispatcher using the provided Driver. */
   public FirebaseJobDispatcher(Driver driver) {
-    mDriver = driver;
-    mValidator = new ValidationEnforcer(mDriver.getValidator());
-    mRetryStrategyBuilder = new RetryStrategy.Builder(mValidator);
+    this.driver = driver;
+    validator = new ValidationEnforcer(driver.getValidator());
+    retryStrategyBuilder = new RetryStrategy.Builder(validator);
   }
 
   /**
@@ -78,11 +78,11 @@ public FirebaseJobDispatcher(Driver driver) {
    */
   @ScheduleResult
   public int schedule(@NonNull Job job) {
-    if (!mDriver.isAvailable()) {
+    if (!driver.isAvailable()) {
       return SCHEDULE_RESULT_NO_DRIVER_AVAILABLE;
     }
 
-    return mDriver.schedule(job);
+    return driver.schedule(job);
   }
 
   /**
@@ -92,11 +92,11 @@ public int schedule(@NonNull Job job) {
    */
   @CancelResult
   public int cancel(@NonNull String tag) {
-    if (!mDriver.isAvailable()) {
+    if (!driver.isAvailable()) {
       return CANCEL_RESULT_NO_DRIVER_AVAILABLE;
     }
 
-    return mDriver.cancel(tag);
+    return driver.cancel(tag);
   }
 
   /**
@@ -106,11 +106,11 @@ public int cancel(@NonNull String tag) {
    */
   @CancelResult
   public int cancelAll() {
-    if (!mDriver.isAvailable()) {
+    if (!driver.isAvailable()) {
       return CANCEL_RESULT_NO_DRIVER_AVAILABLE;
     }
 
-    return mDriver.cancelAll();
+    return driver.cancelAll();
   }
 
   /**
@@ -126,13 +126,13 @@ public void mustSchedule(Job job) {
 
   /** Returns a ValidationEnforcer configured for the current Driver. */
   public ValidationEnforcer getValidator() {
-    return mValidator;
+    return validator;
   }
 
   /** Creates a new Job.Builder, configured with the current driver's validation settings. */
   @NonNull
   public Job.Builder newJobBuilder() {
-    return new Job.Builder(mValidator);
+    return new Job.Builder(validator);
   }
 
   /**
@@ -148,7 +148,7 @@ public Job.Builder newJobBuilder() {
   public RetryStrategy newRetryStrategy(
       @RetryPolicy int policy, int initialBackoff, int maximumBackoff) {
 
-    return mRetryStrategyBuilder.build(policy, initialBackoff, maximumBackoff);
+    return retryStrategyBuilder.build(policy, initialBackoff, maximumBackoff);
   }
 
   /** Results that can legally be returned from {@link #schedule(Job)} calls. */

jobdispatcher/src/main/java/com/firebase/jobdispatcher/GooglePlayDriver.java

@@ -49,55 +49,57 @@ public final class GooglePlayDriver implements Driver {
   private static final int JOB_DISPATCHER_SOURCE_CODE = 1 << 3;
   private static final int JOB_DISPATCHER_SOURCE_VERSION_CODE = 1;
 
-  private final JobValidator mValidator;
+  private final JobValidator validator;
   /** The application Context. Used to send broadcasts. */
-  private final Context mContext;
+  private final Context context;
   /**
    * A PendingIntent from this package. Passed inside the broadcast so the receiver can verify the
    * sender's package.
    */
-  private final PendingIntent mToken;
+  private final PendingIntent token;
   /** Turns Jobs into Bundles. */
-  private final GooglePlayJobWriter mWriter;
+  private final GooglePlayJobWriter writer;
   /**
    * This is hardcoded to true to avoid putting an unnecessary dependency on the Google Play
    * services library.
    */
   // TODO: this is an unsatisfying solution
-  private final boolean mAvailable = true;
+  private final boolean available = true;
 
   /** Instantiates a new GooglePlayDriver. */
   public GooglePlayDriver(Context context) {
-    mContext = context;
-    mToken = PendingIntent.getBroadcast(context, 0, new Intent(), 0);
-    mWriter = new GooglePlayJobWriter();
-    mValidator = new DefaultJobValidator(context);
+    this.context = context;
+    token = PendingIntent.getBroadcast(context, 0, new Intent(), 0);
+    writer = new GooglePlayJobWriter();
+    validator = new DefaultJobValidator(context);
   }
 
   @Override
   public boolean isAvailable() {
-    return mAvailable;
+    return available;
   }
 
   /** Schedules the provided Job. */
   @Override
   @ScheduleResult
   public int schedule(@NonNull Job job) {
-    mContext.sendBroadcast(createScheduleRequest(job));
+    context.sendBroadcast(createScheduleRequest(job));
+
+    GooglePlayReceiver.onSchedule(job); // need to stop the job if it is running.
 
     return FirebaseJobDispatcher.SCHEDULE_RESULT_SUCCESS;
   }
 
   @Override
   public int cancel(@NonNull String tag) {
-    mContext.sendBroadcast(createCancelRequest(tag));
+    context.sendBroadcast(createCancelRequest(tag));
 
     return FirebaseJobDispatcher.CANCEL_RESULT_SUCCESS;
   }
 
   @Override
   public int cancelAll() {
-    mContext.sendBroadcast(createBatchCancelRequest());
+    context.sendBroadcast(createBatchCancelRequest());
 
     return FirebaseJobDispatcher.CANCEL_RESULT_SUCCESS;
   }
@@ -106,14 +108,14 @@ public int cancelAll() {
   protected Intent createCancelRequest(@NonNull String tag) {
     Intent cancelReq = createSchedulerIntent(SCHEDULER_ACTION_CANCEL_TASK);
     cancelReq.putExtra(BUNDLE_PARAM_TAG, tag);
-    cancelReq.putExtra(BUNDLE_PARAM_COMPONENT, new ComponentName(mContext, getReceiverClass()));
+    cancelReq.putExtra(BUNDLE_PARAM_COMPONENT, new ComponentName(context, getReceiverClass()));
     return cancelReq;
   }
 
   @NonNull
   protected Intent createBatchCancelRequest() {
     Intent cancelReq = createSchedulerIntent(SCHEDULER_ACTION_CANCEL_ALL);
-    cancelReq.putExtra(BUNDLE_PARAM_COMPONENT, new ComponentName(mContext, getReceiverClass()));
+    cancelReq.putExtra(BUNDLE_PARAM_COMPONENT, new ComponentName(context, getReceiverClass()));
     return cancelReq;
   }
 
@@ -125,13 +127,13 @@ protected Class<GooglePlayReceiver> getReceiverClass() {
   @NonNull
   @Override
   public JobValidator getValidator() {
-    return mValidator;
+    return validator;
   }
 
   @NonNull
   private Intent createScheduleRequest(JobParameters job) {
     Intent scheduleReq = createSchedulerIntent(SCHEDULER_ACTION_SCHEDULE_TASK);
-    scheduleReq.putExtras(mWriter.writeToBundle(job, scheduleReq.getExtras()));
+    scheduleReq.putExtras(writer.writeToBundle(job, scheduleReq.getExtras()));
     return scheduleReq;
   }
 
@@ -141,7 +143,7 @@ private Intent createSchedulerIntent(String schedulerAction) {
 
     scheduleReq.setPackage(BACKEND_PACKAGE);
     scheduleReq.putExtra(BUNDLE_PARAM_SCHEDULER_ACTION, schedulerAction);
-    scheduleReq.putExtra(BUNDLE_PARAM_TOKEN, mToken);
+    scheduleReq.putExtra(BUNDLE_PARAM_TOKEN, token);
     scheduleReq.putExtra(INTENT_PARAM_SOURCE, JOB_DISPATCHER_SOURCE_CODE);
     scheduleReq.putExtra(INTENT_PARAM_SOURCE_VERSION, JOB_DISPATCHER_SOURCE_VERSION_CODE);
 

jobdispatcher/src/main/java/com/firebase/jobdispatcher/GooglePlayJobCallback.java

@@ -27,10 +27,10 @@
   /** The only supported transaction ID. */
   private static final int TRANSACTION_TASK_FINISHED = IBinder.FIRST_CALL_TRANSACTION + 1;
 
-  private final IBinder mRemote;
+  private final IBinder remote;
 
   public GooglePlayJobCallback(IBinder binder) {
-    mRemote = binder;
+    remote = binder;
   }
 
   @Override
@@ -41,7 +41,7 @@ public void jobFinished(@JobService.JobResult int status) {
       request.writeInterfaceToken(DESCRIPTOR);
       request.writeInt(status);
 
-      mRemote.transact(TRANSACTION_TASK_FINISHED, request, response, 0);
+      remote.transact(TRANSACTION_TASK_FINISHED, request, response, 0);
 
       response.readException();
     } catch (RemoteException e) {

jobdispatcher/src/main/java/com/firebase/jobdispatcher/GooglePlayMessageHandler.java

@@ -107,6 +107,6 @@ private void handleStopMessage(Message message) {
       return;
     }
     JobInvocation job = builder.build();
-    googlePlayReceiver.getExecutionDelegator().stopJob(job);
+    ExecutionDelegator.stopJob(job, true);
   }
 }

jobdispatcher/src/main/java/com/firebase/jobdispatcher/GooglePlayReceiver.java

@@ -74,9 +74,15 @@ public class GooglePlayReceiver extends Service implements ExecutionDelegator.Jo
   private int latestStartId;
 
   /** Endpoint (String) -> Tag (String) -> JobCallback */
-  private final SimpleArrayMap<String, SimpleArrayMap<String, JobCallback>> callbacks =
+  private static final SimpleArrayMap<String, SimpleArrayMap<String, JobCallback>> callbacks =
       new SimpleArrayMap<>(1);
 
+  static void clearCallbacks() {
+    synchronized (callbacks) {
+      callbacks.clear();
+    }
+  }
+
   private static void sendResultSafely(JobCallback callback, int result) {
     try {
       callback.jobFinished(result);
@@ -249,4 +255,31 @@ private static boolean needsToBeRescheduled(JobParameters job, int result) {
   static JobCoder getJobCoder() {
     return prefixedCoder;
   }
+
+  /**
+   * Stops the job if it is running.
+   *
+   * <p>Needed to avoid possibility of sending job result before the reschedule request is received
+   * by Google Play services.
+   */
+  static void onSchedule(Job job) {
+    // Stop if running
+    synchronized (callbacks) {
+      SimpleArrayMap<String, JobCallback> jobs = callbacks.get(job.getService());
+      if (jobs == null) { // not running
+        return;
+      }
+      JobCallback jobCallback = jobs.get(job.getTag());
+      if (jobCallback == null) { // not running
+        return;
+      }
+      JobInvocation key =
+          new JobInvocation.Builder()
+              .setTag(job.getTag())
+              .setService(job.getService())
+              .setTrigger(job.getTrigger())
+              .build();
+      ExecutionDelegator.stopJob(key, false /* must not send the result */);
+    }
+  }
 }