Merge branch 'release/5.6.1'

Author: CDRussell

.travis.yml

@@ -6,7 +6,7 @@ apply plugin: 'kotlin-kapt'
 apply from: '../versioning.gradle'
 
 ext {
-    VERSION_NAME = "5.6.0"
+    VERSION_NAME = "5.6.1"
     USE_ORCHESTRATOR = project.hasProperty('orchestrator') ? project.property('orchestrator') : false
 }
 
@@ -78,7 +78,7 @@ android {
 
 ext {
     supportLibrary = "27.1.1"
-    architectureComponents = "1.0.0"
+    architectureComponents = "1.1.0"
     architectureComponentsExtensions = "1.1.1"
     androidKtx = "0.3"
     dagger = "2.14.1"

app/build.gradle

@@ -51,44 +51,44 @@ class HttpsUpgradeDomainDaoTest {
 
     @Test
     fun whenExactMatchDomainAddedAndThenAllDeletedThenDoesNotContainExactMatchDomain() {
-        dao.insertAll(HttpsUpgradeDomain(exactMatchDomain))
+        dao.insertAll(listOf(HttpsUpgradeDomain(exactMatchDomain)))
         dao.deleteAll()
         assertFalse(dao.hasDomain(exactMatchDomain))
     }
 
     @Test
     fun whenWildcardDomainInsertedModelThenDoesNotContainParentOfWildcardDomain() {
-        dao.insertAll(HttpsUpgradeDomain(wildcardDomain))
+        dao.insertAll(listOf(HttpsUpgradeDomain(wildcardDomain)))
         assertFalse(dao.hasDomain(parentOfWildcardDomain))
     }
 
     @Test
     fun whenOtherWildcardDomainInsertedThenModelDoesNotContainExampleWildcardDomain() {
-        dao.insertAll(HttpsUpgradeDomain(otherWildcardDomain))
+        dao.insertAll(listOf(HttpsUpgradeDomain(otherWildcardDomain)))
         assertFalse(dao.hasDomain(exampleWildcardDomain))
     }
 
     @Test
     fun whenWildcardDomainInsertedThenModelDoesNotContainExactMatchDomain() {
-        dao.insertAll(HttpsUpgradeDomain(wildcardDomain))
+        dao.insertAll(listOf(HttpsUpgradeDomain(wildcardDomain)))
         assertFalse(dao.hasDomain(exactMatchDomain))
     }
 
     @Test
     fun whenWildcardDomainInsertedThenModelContainsExampleWildcardDomain() {
-        dao.insertAll(HttpsUpgradeDomain(wildcardDomain))
+        dao.insertAll(listOf(HttpsUpgradeDomain(wildcardDomain)))
         assertTrue(dao.hasDomain("*.$parentOfWildcardDomain"))
     }
 
     @Test
     fun whenExactMatchDomainInsertedThenModelDoesNotContainOtherDomain() {
-        dao.insertAll(HttpsUpgradeDomain(exactMatchDomain))
+        dao.insertAll(listOf(HttpsUpgradeDomain(exactMatchDomain)))
         assertFalse(dao.hasDomain(otherDomain))
     }
 
     @Test
     fun whenExactMatchDomainIsInsertedThenModelContainsExactMatchDomain() {
-        dao.insertAll(HttpsUpgradeDomain(exactMatchDomain))
+        dao.insertAll(listOf(HttpsUpgradeDomain(exactMatchDomain)))
         assertTrue(dao.hasDomain(exactMatchDomain))
     }
 
@@ -104,7 +104,7 @@ class HttpsUpgradeDomainDaoTest {
 
     @Test
     fun whenModelContainsTwoItemsThenCountIsTwo() {
-        dao.insertAll(HttpsUpgradeDomain(exactMatchDomain), HttpsUpgradeDomain(otherDomain))
+        dao.insertAll(listOf(HttpsUpgradeDomain(exactMatchDomain), HttpsUpgradeDomain(otherDomain)))
         assertEquals(2, dao.count())
     }
 

app/src/androidTest/java/com/duckduckgo/app/httpsupgrade/db/HttpsUpgradeDomainDAOTest.kt

@@ -101,7 +101,7 @@ class HttpsUpgraderPerformanceTest {
     private fun ingest(size: Int): Array<String> {
         val start = System.currentTimeMillis()
         for (i in 0 .. size) {
-            dao.insertAll(HttpsUpgradeDomain("domain$i.com"))
+            dao.insertAll(listOf(HttpsUpgradeDomain("domain$i.com")))
         }
 
         val testDomains = Array(size, {

app/src/androidTest/java/com/duckduckgo/app/httpsupgrade/db/HttpsUpgradePerformanceTest.kt

@@ -16,16 +16,12 @@
 
 package com.duckduckgo.app.autocomplete.api
 
-import com.duckduckgo.app.browser.omnibar.QueryUrlConverter
 import com.duckduckgo.app.global.UriString
 import io.reactivex.Observable
 import javax.inject.Inject
 
 
-open class AutoCompleteApi @Inject constructor(
-        private val autoCompleteService: AutoCompleteService,
-        private val queryUrlConverter: QueryUrlConverter
-) {
+open class AutoCompleteApi @Inject constructor(private val autoCompleteService: AutoCompleteService) {
 
     fun autoComplete(query: String): Observable<AutoCompleteApi.AutoCompleteResult> {
 
@@ -34,16 +30,18 @@ open class AutoCompleteApi @Inject constructor(
         }
 
         return autoCompleteService.autoComplete(query)
-                .flatMapIterable { it -> it }
-                .map { AutoCompleteSuggestion(it.phrase, UriString.isWebUrl(it.phrase)) }
-                .toList()
-                .onErrorReturn { emptyList() }
-                .map { AutoCompleteResult(query = query, suggestions = it) }
-                .toObservable()
+            .flatMapIterable { it -> it }
+            .map { AutoCompleteSuggestion(it.phrase, UriString.isWebUrl(it.phrase)) }
+            .toList()
+            .onErrorReturn { emptyList() }
+            .map { AutoCompleteResult(query = query, suggestions = it) }
+            .toObservable()
     }
 
-    data class AutoCompleteResult(val query: String,
-                                  val suggestions: List<AutoCompleteSuggestion>)
+    data class AutoCompleteResult(
+        val query: String,
+        val suggestions: List<AutoCompleteSuggestion>
+    )
 
     data class AutoCompleteSuggestion(val phrase: String, val isUrl: Boolean)
 

app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoCompleteApi.kt

@@ -341,13 +341,6 @@ class BrowserTabFragment : Fragment(), FindListener {
 
         if (shouldUpdateOmnibarTextInput(viewState, viewState.omnibarText)) {
             omnibarTextInput.setText(viewState.omnibarText)
-
-            // ensures caret sits at the end of the query
-            omnibarTextInput.post {
-                omnibarTextInput?.let {
-                    it.setSelection(it.text.length)
-                }
-            }
             appBarLayout.setExpanded(true, true)
         }
 

app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt

@@ -18,41 +18,76 @@ package com.duckduckgo.app.httpsupgrade.api
 
 import com.duckduckgo.app.global.api.isCached
 import com.duckduckgo.app.global.db.AppDatabase
+import com.duckduckgo.app.httpsupgrade.db.HttpsUpgradeDbWriteStatusStore
 import com.duckduckgo.app.httpsupgrade.db.HttpsUpgradeDomainDao
 import io.reactivex.Completable
 import timber.log.Timber
 import java.io.IOException
 import javax.inject.Inject
 
+/**
+ * For performance reasons, we can't insert all the HTTPS rows in a single transaction as it blocks other DB writes from happening for a while
+ * To avoid the performance problem of a single transaction, we can chunk the data and perform many smaller transactions.
+ * However, if we don't use a single transaction, there is the risk the process could die or exception be thrown while iterating through the HTTPS inserts, leaving the DB in a bad state.
+ * The network cache could therefore deliver the cached value and we could skip writing to the DB thinking we'd already successfully written it all.
+ *
+ * To counter this, we need an external mechanism to track whether the write completed successfully or not.
+ * Using this, we can detect if the write failed when we next download the data. We'll only skip the DB inserts if cache returns the list and the
+ * the eTag received in the response headers matches the eTag we stored on last successful write, indicating that it finished writing successfully.
+ * @see HttpsUpgradeDbWriteStatusStore for how this status flag is stored.
+ */
 class HttpsUpgradeListDownloader @Inject constructor(
-        private val service: HttpsUpgradeListService,
-        private val database: AppDatabase,
-        private val httpsUpgradeDao: HttpsUpgradeDomainDao) {
+    private val service: HttpsUpgradeListService,
+    private val database: AppDatabase,
+    private val httpsUpgradeDao: HttpsUpgradeDomainDao,
+    private val dbWriteStatusStore: HttpsUpgradeDbWriteStatusStore
+) {
 
-    fun downloadList(): Completable {
+    fun downloadList(chunkSize: Int = INSERTION_CHUNK_SIZE): Completable {
 
         Timber.d("Downloading HTTPS Upgrade data")
 
         return Completable.fromAction {
 
             val call = service.https()
             val response = call.execute()
+            val eTag = response.headers().get(HEADER_ETAG)
 
-            if (response.isCached && httpsUpgradeDao.count() != 0) {
+            if (response.isCached && dbWriteStatusStore.isMatchingETag(eTag)) {
                 Timber.d("HTTPS data already cached and stored")
                 return@fromAction
             }
 
             if (response.isSuccessful) {
                 Timber.d("Updating HTTPS upgrade list from server")
-                val domains = response.body()!!.toTypedArray()
-                database.runInTransaction {
-                    httpsUpgradeDao.deleteAll()
-                    httpsUpgradeDao.insertAll(*domains)
+
+                val domains = response.body() ?: throw IllegalStateException("Failed to obtain HTTPS upgrade list")
+
+                val startTime = System.currentTimeMillis()
+
+                httpsUpgradeDao.deleteAll()
+                Timber.v("Took ${System.currentTimeMillis() - startTime}ms to delete existing records")
+
+                val chunks = domains.chunked(chunkSize)
+                Timber.i("Received ${domains.size} HTTPS domains; chunking by $chunkSize into ${chunks.size} separate DB transactions")
+
+                chunks.forEach {
+                    database.runInTransaction {
+                        httpsUpgradeDao.insertAll(it)
+                    }
                 }
+
+                dbWriteStatusStore.saveETag(eTag)
+                Timber.i("Successfully wrote HTTPS data; took ${System.currentTimeMillis() - startTime}ms")
+
             } else {
                 throw IOException("Status: ${response.code()} - ${response.errorBody()?.string()}")
             }
         }
     }
+
+    companion object {
+        private const val INSERTION_CHUNK_SIZE = 1_000
+        private const val HEADER_ETAG = "etag"
+    }
 }

app/src/main/java/com/duckduckgo/app/httpsupgrade/api/HttpsUpgradeListDownloader.kt

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2018 DuckDuckGo
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.duckduckgo.app.httpsupgrade.db
+
+import android.content.Context
+import android.content.SharedPreferences
+import android.support.annotation.VisibleForTesting
+import androidx.core.content.edit
+import javax.inject.Inject
+
+interface HttpsUpgradeDbWriteStatusStore {
+
+    fun saveETag(eTag: String?)
+    fun isMatchingETag(eTag: String?): Boolean
+}
+
+class HttpsUpgradeDbWriteStatusSharedPreferences @Inject constructor(private val context: Context) : HttpsUpgradeDbWriteStatusStore {
+
+    override fun saveETag(eTag: String?) {
+        preferences.edit { putString(KEY_ETAG_OF_LAST_FULL_WRITE, eTag) }
+    }
+
+    override fun isMatchingETag(eTag: String?): Boolean {
+        val existingETag = preferences.getString(KEY_ETAG_OF_LAST_FULL_WRITE, null)
+        return eTag == existingETag
+    }
+
+    private val preferences: SharedPreferences
+        get() = context.getSharedPreferences(FILENAME, Context.MODE_PRIVATE)
+
+    companion object {
+
+        @VisibleForTesting
+        const val FILENAME = "com.duckduckgo.app.httpsupgrade.db.HttpsUpgradeDbWriteStatus"
+        const val KEY_ETAG_OF_LAST_FULL_WRITE = "KEY_ETAG_OF_LAST_FULL_WRITE"
+    }
+}

app/src/main/java/com/duckduckgo/app/httpsupgrade/db/HttpsUpgradeDbWriteStatusStore.kt

@@ -20,15 +20,13 @@ import android.arch.persistence.room.Dao
 import android.arch.persistence.room.Insert
 import android.arch.persistence.room.OnConflictStrategy
 import android.arch.persistence.room.Query
-import org.intellij.lang.annotations.Language
 
 @Dao
 interface HttpsUpgradeDomainDao {
 
     @Insert(onConflict = OnConflictStrategy.REPLACE)
-    fun insertAll(vararg domains: HttpsUpgradeDomain)
+    fun insertAll(domains: List<HttpsUpgradeDomain>)
 
-    @Language("RoomSql")
     @Query("select count(1) > 0 from https_upgrade_domain where domain = :domain")
     fun hasDomain(domain: String) : Boolean
 

app/src/main/java/com/duckduckgo/app/httpsupgrade/db/HttpsUpgradeDomainDao.kt

@@ -16,11 +16,15 @@
 
 package com.duckduckgo.app.httpsupgrade.di
 
+import android.content.Context
 import com.duckduckgo.app.httpsupgrade.HttpsUpgrader
 import com.duckduckgo.app.httpsupgrade.HttpsUpgraderImpl
+import com.duckduckgo.app.httpsupgrade.db.HttpsUpgradeDbWriteStatusSharedPreferences
+import com.duckduckgo.app.httpsupgrade.db.HttpsUpgradeDbWriteStatusStore
 import com.duckduckgo.app.httpsupgrade.db.HttpsUpgradeDomainDao
 import dagger.Module
 import dagger.Provides
+import javax.inject.Singleton
 
 @Module
 class HttpsUpgraderModule {
@@ -29,4 +33,10 @@ class HttpsUpgraderModule {
     fun httpsUpgrader(dao: HttpsUpgradeDomainDao): HttpsUpgrader {
         return HttpsUpgraderImpl(dao)
     }
+
+    @Provides
+    @Singleton
+    fun httpsUpgradeDbWriteStatusStore(context: Context): HttpsUpgradeDbWriteStatusStore {
+        return HttpsUpgradeDbWriteStatusSharedPreferences(context)
+    }
 }