Remove initialization state from the SNTP service (#51)

Upstreaming an internal change authored by @buildbreaker (internal #60231).

> The app does a sync almost immediately after app launch. The state is almost always set to `SYNCING` before the get is called. This means that the `INIT` comparison is always false so the boot time check will never be triggered.

> This should fix a large class of issues related to NTP. Specifically, if the user restarts their phone and then opens the app in poor network connectivity, they'll likely fail all the NTP requests and will be stuck using the persisted NTP time. Since the user restarted their app, the calculations for response age would be completely off (since the current elapsed ticks would always be lower than the elapsed ticks in the persistence).

> Finally, we only do NTP syncs on an app open so if this were to trigger on a specific session, the rest of the session's timestamps would be off too.

PTAL @buildbreaker, @ameliariely, @amphora001

kronos-java/src/main/java/com/lyft/kronos/internal/ntp/SntpService.kt

@@ -69,14 +69,14 @@ internal class SntpServiceImpl @JvmOverloads constructor(private val sntpClient:
                                                          private val cacheExpirationMs: Long = CACHE_EXPIRATION_MS,
                                                          private val maxNtpResponseTimeMs: Long = MAX_NTP_RESPONSE_TIME_MS) : SntpService {
 
-    private val state = AtomicReference(State.INIT)
+    private val state = AtomicReference(State.IDLE)
     private val cachedSyncTime = AtomicLong(0)
     private val executor = Executors.newSingleThreadExecutor { Thread(it, "kronos-android") }
 
     private val response: SntpClient.Response?
         get() {
             val response = responseCache.get()
-            val isCachedFromPreviousBoot = state.compareAndSet(State.INIT, State.IDLE) && response != null && !response.isFromSameBoot
+            val isCachedFromPreviousBoot = state.get() == State.IDLE && response != null && !response.isFromSameBoot
             return if (isCachedFromPreviousBoot) {
                 responseCache.clear()
                 null
@@ -89,7 +89,6 @@ internal class SntpServiceImpl @JvmOverloads constructor(private val sntpClient:
         get() = deviceClock.getElapsedTimeMs() - cachedSyncTime.get()
 
     private enum class State {
-        INIT,
         IDLE,
         SYNCING,
         STOPPED

kronos-java/src/test/java/com/lyft/kronos/internal/ntp/SntpServiceTest.kt

@@ -94,4 +94,19 @@ class SntpServiceTest {
         verify(sntpSyncListener, times(1)).onError(eq("1.fake.pool.ntp.org"), any<NTPSyncException>())
         verify(sntpSyncListener, times(1)).onError(eq("2.fake.pool.ntp.org"), any<NTPSyncException>())
     }
+
+    @Test
+    fun shouldClearCacheWhenSyncFailsAndGettingCurrentTime() {
+        whenever(sntpClient.requestTime("1.fake.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
+        whenever(sntpClient.requestTime("2.fake.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
+        whenever(mockResponse.currentTimeMs).thenReturn(-1)
+        whenever(mockResponse.isFromSameBoot).thenReturn(false)
+        whenever(responseCache.get()).thenReturn(mockResponse)
+
+        assertThat(sntpService.sync()).isFalse
+
+        sntpService.currentTime()
+
+        verify(responseCache, times(1)).clear()
+    }
 }