Fix OperationScheduler moratorium calculation for clock rollback case.

Make the unit test exercise some clock-rollback scenarios, using
a properly injected artificial clock.

Bug: 2579585
Change-Id: I6f81c32318ba27429bd30ff53b48449218e4ac64

2 files changed, 114 insertions, 55 deletions

diff --git a/common/java/com/android/common/OperationScheduler.java b/common/java/com/android/common/OperationScheduler.java
index 0a48fe7..08cc25b 100644
--- a/common/java/com/android/common/OperationScheduler.java
+++ b/common/java/com/android/common/OperationScheduler.java
@@ -124,7 +124,8 @@ public class OperationScheduler {
     }
 
     /**
-     * Compute the time of the next operation.  Does not modify any state.
+     * Compute the time of the next operation.  Does not modify any state
+     * (unless the clock rolls backwards, in which case timers are reset).
      *
      * @param options to use for this computation.
      * @return the wall clock time ({@link System#currentTimeMillis()}) when the
@@ -143,11 +144,11 @@ public class OperationScheduler {
         // clipped to the current time so we don't languish forever.
 
         int errorCount = mStorage.getInt(PREFIX + "errorCount", 0);
-        long now = System.currentTimeMillis();
+        long now = currentTimeMillis();
         long lastSuccessTimeMillis = getTimeBefore(PREFIX + "lastSuccessTimeMillis", now);
         long lastErrorTimeMillis = getTimeBefore(PREFIX + "lastErrorTimeMillis", now);
         long triggerTimeMillis = mStorage.getLong(PREFIX + "triggerTimeMillis", Long.MAX_VALUE);
-        long moratoriumSetMillis = mStorage.getLong(PREFIX + "moratoriumSetTimeMillis", 0);
+        long moratoriumSetMillis = getTimeBefore(PREFIX + "moratoriumSetTimeMillis", now);
         long moratoriumTimeMillis = getTimeBefore(PREFIX + "moratoriumTimeMillis",
                 moratoriumSetMillis + options.maxMoratoriumMillis);
 
@@ -155,9 +156,8 @@ public class OperationScheduler {
         if (options.periodicIntervalMillis > 0) {
             time = Math.min(time, lastSuccessTimeMillis + options.periodicIntervalMillis);
         }
-        if (time >= moratoriumTimeMillis - options.maxMoratoriumMillis) {
-            time = Math.max(time, moratoriumTimeMillis);
-        }
+
+        time = Math.max(time, moratoriumTimeMillis);
         time = Math.max(time, lastSuccessTimeMillis + options.minTriggerMillis);
         if (errorCount > 0) {
             time = Math.max(time, lastErrorTimeMillis + options.backoffFixedMillis +
@@ -205,7 +205,7 @@ public class OperationScheduler {
     /**
      * Request an operation to be performed at a certain time.  The actual
      * scheduled time may be affected by error backoff logic and defined
-     * minimum intervals.
+     * minimum intervals.  Use {@link Long#MAX_VALUE} to disable triggering.
      *
      * @param millis wall clock time ({@link System#currentTimeMillis()}) to
      * trigger another operation; 0 to trigger immediately
@@ -218,13 +218,13 @@ public class OperationScheduler {
      * Forbid any operations until after a certain (absolute) time.
      * Limited by {@link #Options.maxMoratoriumMillis}.
      *
-     * @param millis wall clock time ({@link System#currentTimeMillis()}) to
-     * wait before attempting any more operations; 0 to remove moratorium
+     * @param millis wall clock time ({@link System#currentTimeMillis()})
+     * when operations should be allowed again; 0 to remove moratorium
      */
     public void setMoratoriumTimeMillis(long millis) {
         mStorage.edit()
                 .putLong(PREFIX + "moratoriumTimeMillis", millis)
-                .putLong(PREFIX + "moratoriumSetTimeMillis", System.currentTimeMillis())
+                .putLong(PREFIX + "moratoriumSetTimeMillis", currentTimeMillis())
                 .commit();
     }
 
@@ -239,7 +239,7 @@ public class OperationScheduler {
     public boolean setMoratoriumTimeHttp(String retryAfter) {
         try {
             long ms = Long.valueOf(retryAfter) * 1000;
-            setMoratoriumTimeMillis(ms + System.currentTimeMillis());
+            setMoratoriumTimeMillis(ms + currentTimeMillis());
             return true;
         } catch (NumberFormatException nfe) {
             try {
@@ -269,13 +269,12 @@ public class OperationScheduler {
     public void onSuccess() {
         resetTransientError();
         resetPermanentError();
-        long now = System.currentTimeMillis();
         mStorage.edit()
                 .remove(PREFIX + "errorCount")
                 .remove(PREFIX + "lastErrorTimeMillis")
                 .remove(PREFIX + "permanentError")
                 .remove(PREFIX + "triggerTimeMillis")
-                .putLong(PREFIX + "lastSuccessTimeMillis", now).commit();
+                .putLong(PREFIX + "lastSuccessTimeMillis", currentTimeMillis()).commit();
     }
 
     /**
@@ -284,8 +283,7 @@ public class OperationScheduler {
      * purposes.
      */
     public void onTransientError() {
-        long now = System.currentTimeMillis();
-        mStorage.edit().putLong(PREFIX + "lastErrorTimeMillis", now).commit();
+        mStorage.edit().putLong(PREFIX + "lastErrorTimeMillis", currentTimeMillis()).commit();
         mStorage.edit().putInt(PREFIX + "errorCount",
                 mStorage.getInt(PREFIX + "errorCount", 0) + 1).commit();
     }
@@ -338,4 +336,13 @@ public class OperationScheduler {
         }
         return out.append("]").toString();
     }
+
+    /**
+     * Gets the current time.  Can be overridden for unit testing.
+     *
+     * @return {@link System#currentTimeMillis()}
+     */
+    protected long currentTimeMillis() {
+        return System.currentTimeMillis();
+    }
 }
diff --git a/common/tests/src/com/android/common/OperationSchedulerTest.java b/common/tests/src/com/android/common/OperationSchedulerTest.java
index 866d1a8..955508f 100644
--- a/common/tests/src/com/android/common/OperationSchedulerTest.java
+++ b/common/tests/src/com/android/common/OperationSchedulerTest.java
@@ -22,19 +22,34 @@ import android.test.suitebuilder.annotation.MediumTest;
 import android.test.suitebuilder.annotation.SmallTest;
 
 public class OperationSchedulerTest extends AndroidTestCase {
+    /**
+     * OperationScheduler subclass which uses an artificial time.
+     * Set {@link #timeMillis} to whatever value you like.
+     */
+    private class TimeTravelScheduler extends OperationScheduler {
+        static final long DEFAULT_TIME = 1250146800000L;  // 13-Aug-2009, 12:00:00 am
+        public long timeMillis = DEFAULT_TIME;
+
+        @Override
+        protected long currentTimeMillis() { return timeMillis; }
+        public TimeTravelScheduler() { super(getFreshStorage()); }
+    }
+
+    private SharedPreferences getFreshStorage() {
+        SharedPreferences sp = getContext().getSharedPreferences("OperationSchedulerTest", 0);
+        sp.edit().clear().commit();
+        return sp;
+    }
+
     @MediumTest
     public void testScheduler() throws Exception {
-        String name = "OperationSchedulerTest.testScheduler";
-        SharedPreferences storage = getContext().getSharedPreferences(name, 0);
-        storage.edit().clear().commit();
-
-        OperationScheduler scheduler = new OperationScheduler(storage);
+        TimeTravelScheduler scheduler = new TimeTravelScheduler();
         OperationScheduler.Options options = new OperationScheduler.Options();
         assertEquals(Long.MAX_VALUE, scheduler.getNextTimeMillis(options));
         assertEquals(0, scheduler.getLastSuccessTimeMillis());
         assertEquals(0, scheduler.getLastAttemptTimeMillis());
 
-        long beforeTrigger = System.currentTimeMillis();
+        long beforeTrigger = scheduler.timeMillis;
         scheduler.setTriggerTimeMillis(beforeTrigger + 1000000);
         assertEquals(beforeTrigger + 1000000, scheduler.getNextTimeMillis(options));
 
@@ -51,33 +66,26 @@ public class OperationSchedulerTest extends AndroidTestCase {
         assertEquals(beforeTrigger + 1500000, scheduler.getNextTimeMillis(options));
 
         // Backoff interval after an error
-        long beforeError = System.currentTimeMillis();
+        long beforeError = (scheduler.timeMillis += 100);
         scheduler.onTransientError();
-        long afterError = System.currentTimeMillis();
         assertEquals(0, scheduler.getLastSuccessTimeMillis());
-        assertTrue(beforeError <= scheduler.getLastAttemptTimeMillis());
-        assertTrue(afterError >= scheduler.getLastAttemptTimeMillis());
+        assertEquals(beforeError, scheduler.getLastAttemptTimeMillis());
         assertEquals(beforeTrigger + 1500000, scheduler.getNextTimeMillis(options));
         options.backoffFixedMillis = 1000000;
         options.backoffIncrementalMillis = 500000;
-        assertTrue(beforeError + 1500000 <= scheduler.getNextTimeMillis(options));
-        assertTrue(afterError + 1500000 >= scheduler.getNextTimeMillis(options));
+        assertEquals(beforeError + 1500000, scheduler.getNextTimeMillis(options));
 
         // Two errors: backoff interval increases
-        beforeError = System.currentTimeMillis();
+        beforeError = (scheduler.timeMillis += 100);
         scheduler.onTransientError();
-        afterError = System.currentTimeMillis();
-        assertTrue(beforeError <= scheduler.getLastAttemptTimeMillis());
-        assertTrue(afterError >= scheduler.getLastAttemptTimeMillis());
-        assertTrue(beforeError + 2000000 <= scheduler.getNextTimeMillis(options));
-        assertTrue(afterError + 2000000 >= scheduler.getNextTimeMillis(options));
+        assertEquals(beforeError, scheduler.getLastAttemptTimeMillis());
+        assertEquals(beforeError + 2000000, scheduler.getNextTimeMillis(options));
 
         // Reset transient error: no backoff interval
         scheduler.resetTransientError();
         assertEquals(0, scheduler.getLastSuccessTimeMillis());
         assertEquals(beforeTrigger + 1500000, scheduler.getNextTimeMillis(options));
-        assertTrue(beforeError <= scheduler.getLastAttemptTimeMillis());
-        assertTrue(afterError >= scheduler.getLastAttemptTimeMillis());
+        assertEquals(beforeError, scheduler.getLastAttemptTimeMillis());
 
         // Permanent error holds true even if transient errors are reset
         // However, we remember that the transient error was reset...
@@ -89,30 +97,26 @@ public class OperationSchedulerTest extends AndroidTestCase {
         assertEquals(beforeTrigger + 1500000, scheduler.getNextTimeMillis(options));
 
         // Success resets the trigger
-        long beforeSuccess = System.currentTimeMillis();
+        long beforeSuccess = (scheduler.timeMillis += 100);
         scheduler.onSuccess();
-        long afterSuccess = System.currentTimeMillis();
-        assertTrue(beforeSuccess <= scheduler.getLastAttemptTimeMillis());
-        assertTrue(afterSuccess >= scheduler.getLastAttemptTimeMillis());
-        assertTrue(beforeSuccess <= scheduler.getLastSuccessTimeMillis());
-        assertTrue(afterSuccess >= scheduler.getLastSuccessTimeMillis());
+        assertEquals(beforeSuccess, scheduler.getLastAttemptTimeMillis());
+        assertEquals(beforeSuccess, scheduler.getLastSuccessTimeMillis());
         assertEquals(Long.MAX_VALUE, scheduler.getNextTimeMillis(options));
 
         // The moratorium is not reset by success!
-        scheduler.setTriggerTimeMillis(beforeSuccess + 500000);
+        scheduler.setTriggerTimeMillis(0);
         assertEquals(beforeTrigger + 1500000, scheduler.getNextTimeMillis(options));
         scheduler.setMoratoriumTimeMillis(0);
-        assertEquals(beforeSuccess + 500000, scheduler.getNextTimeMillis(options));
+        assertEquals(beforeSuccess, scheduler.getNextTimeMillis(options));
 
         // Periodic interval after success
         options.periodicIntervalMillis = 250000;
-        assertTrue(beforeSuccess + 250000 <= scheduler.getNextTimeMillis(options));
-        assertTrue(afterSuccess + 250000 >= scheduler.getNextTimeMillis(options));
+        scheduler.setTriggerTimeMillis(Long.MAX_VALUE);
+        assertEquals(beforeSuccess + 250000, scheduler.getNextTimeMillis(options));
 
         // Trigger minimum is also since the last success
         options.minTriggerMillis = 1000000;
-        assertTrue(beforeSuccess + 1000000 <= scheduler.getNextTimeMillis(options));
-        assertTrue(afterSuccess + 1000000 >= scheduler.getNextTimeMillis(options));
+        assertEquals(beforeSuccess + 1000000, scheduler.getNextTimeMillis(options));
     }
 
     @SmallTest
@@ -138,23 +142,19 @@ public class OperationSchedulerTest extends AndroidTestCase {
 
     @SmallTest
     public void testMoratoriumWithHttpDate() throws Exception {
-        String name = "OperationSchedulerTest.testMoratoriumWithHttpDate";
-        SharedPreferences storage = getContext().getSharedPreferences(name, 0);
-        storage.edit().clear().commit();
-
-        OperationScheduler scheduler = new OperationScheduler(storage);
+        TimeTravelScheduler scheduler = new TimeTravelScheduler();
         OperationScheduler.Options options = new OperationScheduler.Options();
 
-        long beforeTrigger = System.currentTimeMillis();
+        long beforeTrigger = scheduler.timeMillis;
         scheduler.setTriggerTimeMillis(beforeTrigger + 1000000);
         assertEquals(beforeTrigger + 1000000, scheduler.getNextTimeMillis(options));
 
         scheduler.setMoratoriumTimeMillis(beforeTrigger + 2000000);
         assertEquals(beforeTrigger + 2000000, scheduler.getNextTimeMillis(options));
 
-        long beforeMoratorium = System.currentTimeMillis();
+        long beforeMoratorium = scheduler.timeMillis;
         assertTrue(scheduler.setMoratoriumTimeHttp("3000"));
-        long afterMoratorium = System.currentTimeMillis();
+        long afterMoratorium = scheduler.timeMillis;
         assertTrue(beforeMoratorium + 3000000 <= scheduler.getNextTimeMillis(options));
         assertTrue(afterMoratorium + 3000000 >= scheduler.getNextTimeMillis(options));
 
@@ -164,4 +164,56 @@ public class OperationSchedulerTest extends AndroidTestCase {
 
         assertFalse(scheduler.setMoratoriumTimeHttp("not actually a date"));
     }
+
+    @SmallTest
+    public void testClockRollbackScenario() throws Exception {
+        TimeTravelScheduler scheduler = new TimeTravelScheduler();
+        OperationScheduler.Options options = new OperationScheduler.Options();
+        options.minTriggerMillis = 2000;
+
+        // First, set up a scheduler with reasons to wait: a transient
+        // error with backoff and a moratorium for a few minutes.
+
+        long beforeTrigger = scheduler.timeMillis;
+        long triggerTime = beforeTrigger - 10000000;
+        scheduler.setTriggerTimeMillis(triggerTime);
+        assertEquals(triggerTime, scheduler.getNextTimeMillis(options));
+        assertEquals(0, scheduler.getLastAttemptTimeMillis());
+
+        long beforeSuccess = (scheduler.timeMillis += 100);
+        scheduler.onSuccess();
+        scheduler.setTriggerTimeMillis(triggerTime);
+        assertEquals(beforeSuccess, scheduler.getLastAttemptTimeMillis());
+        assertEquals(beforeSuccess + 2000, scheduler.getNextTimeMillis(options));
+
+        long beforeError = (scheduler.timeMillis += 100);
+        scheduler.onTransientError();
+        assertEquals(beforeError, scheduler.getLastAttemptTimeMillis());
+        assertEquals(beforeError + 5000, scheduler.getNextTimeMillis(options));
+
+        long beforeMoratorium = (scheduler.timeMillis += 100);
+        scheduler.setMoratoriumTimeMillis(beforeTrigger + 1000000);
+        assertEquals(beforeTrigger + 1000000, scheduler.getNextTimeMillis(options));
+
+        // Now set the time back a few seconds.
+        // The moratorium time should still be honored.
+        long beforeRollback = (scheduler.timeMillis = beforeTrigger - 10000);
+        assertEquals(beforeTrigger + 1000000, scheduler.getNextTimeMillis(options));
+
+        // The rollback also moved the last-attempt clock back to the rollback time.
+        assertEquals(scheduler.timeMillis, scheduler.getLastAttemptTimeMillis());
+
+        // But if we set the time back more than a day, the moratorium
+        // resets to the maximum moratorium (a day, by default), exposing
+        // the original trigger time.
+        beforeRollback = (scheduler.timeMillis = beforeTrigger - 100000000);
+        assertEquals(triggerTime, scheduler.getNextTimeMillis(options));
+        assertEquals(beforeRollback, scheduler.getLastAttemptTimeMillis());
+
+        // If we roll forward until after the re-set moratorium, then it expires.
+        scheduler.timeMillis = triggerTime + 5000000;
+        assertEquals(triggerTime, scheduler.getNextTimeMillis(options));
+        assertEquals(beforeRollback, scheduler.getLastAttemptTimeMillis());
+        assertEquals(beforeRollback, scheduler.getLastSuccessTimeMillis());
+    }
 }