From ee1c068f612a1293ebb0955ceabf3bcb89a63257 Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Sun, 15 Dec 2019 21:51:12 -0500 Subject: [PATCH 1/6] Closes #5315 - Create a Top Sites storage component --- .buildconfig.yml | 4 + README.md | 8 +- components/feature/top-sites/README.md | 19 +++ components/feature/top-sites/build.gradle | 72 +++++++++++ .../feature/top-sites/proguard-rules.pro | 21 ++++ .../1.json | 52 ++++++++ .../feature/top/sites/TopSiteStorageTest.kt | 113 ++++++++++++++++++ .../feature/top/sites/db/TopSiteDaoTest.kt | 95 +++++++++++++++ .../top-sites/src/main/AndroidManifest.xml | 5 + .../components/feature/top/sites/TopSite.kt | 25 ++++ .../feature/top/sites/TopSiteStorage.kt | 62 ++++++++++ .../top/sites/adapter/TopSiteAdapter.kt | 33 +++++ .../feature/top/sites/db/TopSiteDao.kt | 33 +++++ .../feature/top/sites/db/TopSiteDatabase.kt | 35 ++++++ .../feature/top/sites/db/TopSiteEntity.kt | 28 +++++ .../org.mockito.plugins.MockMaker | 2 + 16 files changed, 604 insertions(+), 3 deletions(-) create mode 100644 components/feature/top-sites/README.md create mode 100644 components/feature/top-sites/build.gradle create mode 100644 components/feature/top-sites/proguard-rules.pro create mode 100644 components/feature/top-sites/schemas/mozilla.components.feature.top.sites.db.TopSiteDatabase/1.json create mode 100644 components/feature/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/TopSiteStorageTest.kt create mode 100644 components/feature/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/db/TopSiteDaoTest.kt create mode 100644 components/feature/top-sites/src/main/AndroidManifest.xml create mode 100644 components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSite.kt create mode 100644 components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSiteStorage.kt create mode 100644 components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/adapter/TopSiteAdapter.kt create mode 100644 components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteDao.kt create mode 100644 components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteDatabase.kt create mode 100644 components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteEntity.kt create mode 100644 components/feature/top-sites/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/.buildconfig.yml b/.buildconfig.yml index 4e52694ba83..d7b390eb80f 100644 --- a/.buildconfig.yml +++ b/.buildconfig.yml @@ -92,6 +92,10 @@ projects: path: components/feature/toolbar description: 'Feature implementation connecting a toolbar implementation with the session module.' publish: true + feature-top-sites: + path: components/feature/top-sites + description: 'Feature implementation for saving and removing top sites.' + publish: true feature-downloads: path: components/feature/downloads description: 'Feature implementation for apps that want to use Android downloads manager.' diff --git a/README.md b/README.md index 4831632fdce..1334429d6d1 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,8 @@ _Combined components to implement feature-specific use cases._ * 🔴 [**Toolbar**](components/feature/toolbar/README.md) - A component that connects a (concept) toolbar implementation with the browser session module. +* 🔴 [**Top Sites**](components/feature/top-sites/README.md) - Feature implementation for saving and removing top sites. + * ⚪ [**Prompts**](components/feature/prompts/README.md) - A component that will handle all the common prompt dialogs from web content. * ⚪ [**Push**](components/feature/push/README.md) - A component that provides Autopush messages with help from a supported push service. @@ -268,7 +270,7 @@ _Sample apps using various components._ * [**Nearby Chat**](samples/nearby-chat) - An app demoing how to use the [**Nearby**](components/lib/nearby/README.md) library for peer-to-peer communication between devices. -* [**Toolbar**](samples/toolbar) - An app demoing multiple customized toolbars using the [**browser-toolbar**](components/browser/toolbar/README.md) component. +* [**Toolbar**](samples/toolbar) - An app demoing multiple customized toolbars using the [**browser-toolbar**](components/browser/toolbar/README.md) component. # Building # @@ -282,11 +284,11 @@ $ ./gradlew assemble ## Android Studio ## -If the environment variable `JAVA_HOME` is not defined, you will need to set it. If you would like to use the JDK installed by Android Studio, here's how to find it: +If the environment variable `JAVA_HOME` is not defined, you will need to set it. If you would like to use the JDK installed by Android Studio, here's how to find it: 1. Open Android Studio. 2. Select "Configure". -3. Select "Default Project Structure". You should now see the Android JDK location. +3. Select "Default Project Structure". You should now see the Android JDK location. 4. Set the environment variable `JAVA_HOME` to the location. (How you set an environment variable depends on your OS.) 5. Restart Android Studio. diff --git a/components/feature/top-sites/README.md b/components/feature/top-sites/README.md new file mode 100644 index 00000000000..43da2f004ba --- /dev/null +++ b/components/feature/top-sites/README.md @@ -0,0 +1,19 @@ +# [Android Components](../../../README.md) > Feature > Top Sites + +Feature implementation for saving and removing top sites. + +## Usage + +### Setting up the dependency + +Use Gradle to download the library from [maven.mozilla.org](https://maven.mozilla.org/) ([Setup repository](../../../README.md#maven-repository)): + +```Groovy +implementation "org.mozilla.components:feature-top-sites:{latest-version}" +``` + +## License + + This Source Code Form is subject to the terms of the Mozilla Public + License, v. 2.0. If a copy of the MPL was not distributed with this + file, You can obtain one at http://mozilla.org/MPL/2.0/ diff --git a/components/feature/top-sites/build.gradle b/components/feature/top-sites/build.gradle new file mode 100644 index 00000000000..537939c1be8 --- /dev/null +++ b/components/feature/top-sites/build.gradle @@ -0,0 +1,72 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +apply plugin: 'com.android.library' +apply plugin: 'kotlin-android' +apply plugin: 'kotlin-kapt' + +android { + compileSdkVersion config.compileSdkVersion + + defaultConfig { + minSdkVersion config.minSdkVersion + targetSdkVersion config.targetSdkVersion + testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" + + kapt { + arguments { + arg("room.schemaLocation", "$projectDir/schemas".toString()) + } + } + } + + buildTypes { + release { + minifyEnabled false + proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro' + } + } + + packagingOptions { + exclude 'META-INF/proguard/androidx-annotations.pro' + } + + sourceSets { + androidTest.assets.srcDirs += files("$projectDir/schemas".toString()) + } +} + +dependencies { + implementation project(':support-ktx') + implementation project(':support-base') + + implementation Dependencies.kotlin_stdlib + implementation Dependencies.kotlin_coroutines + + implementation Dependencies.androidx_paging + implementation Dependencies.androidx_lifecycle_extensions + kapt Dependencies.androidx_lifecycle_compiler + + implementation Dependencies.androidx_room_runtime + kapt Dependencies.androidx_room_compiler + + testImplementation project(':support-test') + + testImplementation Dependencies.androidx_test_core + testImplementation Dependencies.testing_junit + testImplementation Dependencies.testing_mockito + testImplementation Dependencies.testing_robolectric + testImplementation Dependencies.kotlin_coroutines + + androidTestImplementation project(':support-android-test') + + androidTestImplementation Dependencies.androidx_room_testing + androidTestImplementation Dependencies.androidx_arch_core_testing + androidTestImplementation Dependencies.androidx_test_core + androidTestImplementation Dependencies.androidx_test_runner + androidTestImplementation Dependencies.androidx_test_rules +} + +apply from: '../../../publish.gradle' +ext.configurePublish(config.componentsGroupId, archivesBaseName, project.ext.description) diff --git a/components/feature/top-sites/proguard-rules.pro b/components/feature/top-sites/proguard-rules.pro new file mode 100644 index 00000000000..f1b424510da --- /dev/null +++ b/components/feature/top-sites/proguard-rules.pro @@ -0,0 +1,21 @@ +# Add project specific ProGuard rules here. +# You can control the set of applied configuration files using the +# proguardFiles setting in build.gradle. +# +# For more details, see +# http://developer.android.com/guide/developing/tools/proguard.html + +# If your project uses WebView with JS, uncomment the following +# and specify the fully qualified class name to the JavaScript interface +# class: +#-keepclassmembers class fqcn.of.javascript.interface.for.webview { +# public *; +#} + +# Uncomment this to preserve the line number information for +# debugging stack traces. +#-keepattributes SourceFile,LineNumberTable + +# If you keep the line number information, uncomment this to +# hide the original source file name. +#-renamesourcefileattribute SourceFile diff --git a/components/feature/top-sites/schemas/mozilla.components.feature.top.sites.db.TopSiteDatabase/1.json b/components/feature/top-sites/schemas/mozilla.components.feature.top.sites.db.TopSiteDatabase/1.json new file mode 100644 index 00000000000..f1de827c0c5 --- /dev/null +++ b/components/feature/top-sites/schemas/mozilla.components.feature.top.sites.db.TopSiteDatabase/1.json @@ -0,0 +1,52 @@ +{ + "formatVersion": 1, + "database": { + "version": 1, + "identityHash": "ce733d9c47cd10312a1c13de8efb7e8d", + "entities": [ + { + "tableName": "top_sites", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT, `title` TEXT NOT NULL, `url` TEXT NOT NULL, `created_at` INTEGER NOT NULL)", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": false + }, + { + "fieldPath": "title", + "columnName": "title", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "url", + "columnName": "url", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "createdAt", + "columnName": "created_at", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "columnNames": [ + "id" + ], + "autoGenerate": true + }, + "indices": [], + "foreignKeys": [] + } + ], + "views": [], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'ce733d9c47cd10312a1c13de8efb7e8d')" + ] + } +} \ No newline at end of file diff --git a/components/feature/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/TopSiteStorageTest.kt b/components/feature/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/TopSiteStorageTest.kt new file mode 100644 index 00000000000..ab6b790cd63 --- /dev/null +++ b/components/feature/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/TopSiteStorageTest.kt @@ -0,0 +1,113 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.top.sites + +import android.content.Context +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import androidx.paging.PagedList +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import mozilla.components.feature.top.sites.db.TopSiteDatabase +import mozilla.components.support.android.test.awaitValue +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors + +@Suppress("LargeClass") +class TopSiteStorageTest { + private lateinit var context: Context + private lateinit var storage: TopSiteStorage + private lateinit var executor: ExecutorService + + @get:Rule + var instantTaskExecutorRule = InstantTaskExecutorRule() + + @Before + fun setUp() { + executor = Executors.newSingleThreadExecutor() + + context = ApplicationProvider.getApplicationContext() + val database = Room.inMemoryDatabaseBuilder(context, TopSiteDatabase::class.java).build() + + storage = TopSiteStorage(context) + storage.database = lazy { database } + } + + @After + fun tearDown() { + executor.shutdown() + } + + @Test + fun testAddingTopSite() { + storage.addTopSite("Mozilla", "https://www.mozilla.org") + storage.addTopSite("Firefox", "https://www.firefox.com") + + val topSites = getAllTopSites() + + assertEquals(2, topSites.size) + + assertEquals("Mozilla", topSites[0].title) + assertEquals("https://www.mozilla.org", topSites[0].url) + assertEquals("Firefox", topSites[1].title) + assertEquals("https://www.firefox.com", topSites[1].url) + } + + @Test + fun testRemovingTopSites() { + storage.addTopSite("Mozilla", "https://www.mozilla.org") + storage.addTopSite("Firefox", "https://www.firefox.com") + + getAllTopSites().let { topSites -> + assertEquals(2, topSites.size) + + storage.removeTopSite(topSites[0]) + } + + getAllTopSites().let { topSites -> + assertEquals(1, topSites.size) + + assertEquals("Firefox", topSites[0].title) + assertEquals("https://www.firefox.com", topSites[0].url) + } + } + + @Test + fun testGettingTopSites() { + storage.addTopSite("Mozilla", "https://www.mozilla.org") + storage.addTopSite("Firefox", "https://www.firefox.com") + + val topSites = storage.getTopSites().awaitValue() + + assertNotNull(topSites!!) + assertEquals(2, topSites.size) + + with(topSites[0]) { + assertEquals("Mozilla", title) + assertEquals("https://www.mozilla.org", url) + } + + with(topSites[1]) { + assertEquals("Firefox", title) + assertEquals("https://www.firefox.com", url) + } + } + + private fun getAllTopSites(): List { + val dataSource = storage.getTopSitesPaged().create() + + val pagedList = PagedList.Builder(dataSource, 10) + .setNotifyExecutor(executor) + .setFetchExecutor(executor) + .build() + + return pagedList.toList() + } +} diff --git a/components/feature/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/db/TopSiteDaoTest.kt b/components/feature/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/db/TopSiteDaoTest.kt new file mode 100644 index 00000000000..c74b21be925 --- /dev/null +++ b/components/feature/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/db/TopSiteDaoTest.kt @@ -0,0 +1,95 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.top.sites.db + +import android.content.Context +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import androidx.paging.PagedList +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors + +class TopSiteDaoTest { + private val context: Context + get() = ApplicationProvider.getApplicationContext() + + private lateinit var database: TopSiteDatabase + private lateinit var topSiteDao: TopSiteDao + private lateinit var executor: ExecutorService + + @get:Rule + var instantTaskExecutorRule = InstantTaskExecutorRule() + + @Before + fun setUp() { + database = Room.inMemoryDatabaseBuilder(context, TopSiteDatabase::class.java).build() + topSiteDao = database.topSiteDao() + executor = Executors.newSingleThreadExecutor() + } + + @After + fun tearDown() { + database.close() + executor.shutdown() + } + + @Test + fun testAddingTopSite() { + val topSite = TopSiteEntity( + title = "Mozilla", + url = "https://www.mozilla.org", + createdAt = 200 + ).also { + it.id = topSiteDao.insertTopSite(it) + } + + val dataSource = topSiteDao.getTopSitesPaged().create() + + val pagedList = PagedList.Builder(dataSource, 10) + .setNotifyExecutor(executor) + .setFetchExecutor(executor) + .build() + + assertEquals(1, pagedList.size) + assertEquals(topSite, pagedList[0]!!) + } + + @Test + fun testRemovingTopSite() { + val topSite1 = TopSiteEntity( + title = "Mozilla", + url = "https://www.mozilla.org", + createdAt = 200 + ).also { + it.id = topSiteDao.insertTopSite(it) + } + + val topSite2 = TopSiteEntity( + title = "Firefox", + url = "https://www.firefox.com", + createdAt = 100 + ).also { + it.id = topSiteDao.insertTopSite(it) + } + + topSiteDao.deleteTopSite(topSite1) + + val dataSource = topSiteDao.getTopSitesPaged().create() + + val pagedList = PagedList.Builder(dataSource, 10) + .setNotifyExecutor(executor) + .setFetchExecutor(executor) + .build() + + assertEquals(1, pagedList.size) + assertEquals(topSite2, pagedList[0]) + } +} diff --git a/components/feature/top-sites/src/main/AndroidManifest.xml b/components/feature/top-sites/src/main/AndroidManifest.xml new file mode 100644 index 00000000000..914a72c3e91 --- /dev/null +++ b/components/feature/top-sites/src/main/AndroidManifest.xml @@ -0,0 +1,5 @@ + + diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSite.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSite.kt new file mode 100644 index 00000000000..a559c3d4963 --- /dev/null +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSite.kt @@ -0,0 +1,25 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.top.sites + +/** + * A top site. + */ +interface TopSite { + /** + * Unique ID of this top site. + */ + val id: Long + + /** + * The title of the top site. + */ + val title: String + + /** + * The URL of the top site. + */ + val url: String +} diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSiteStorage.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSiteStorage.kt new file mode 100644 index 00000000000..94aa1ef0dc4 --- /dev/null +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSiteStorage.kt @@ -0,0 +1,62 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.top.sites + +import android.content.Context +import androidx.lifecycle.LiveData +import androidx.lifecycle.Transformations +import androidx.paging.DataSource +import mozilla.components.feature.top.sites.adapter.TopSiteAdapter +import mozilla.components.feature.top.sites.db.TopSiteDatabase +import mozilla.components.feature.top.sites.db.TopSiteEntity + +/** + * A storage implementation for organizing top sites. + */ +class TopSiteStorage( + context: Context +) { + internal var database: Lazy = lazy { TopSiteDatabase.get(context) } + + /** + * Adds a new [TopSite]. + */ + fun addTopSite(title: String, url: String) { + TopSiteEntity( + title = title, + url = url, + createdAt = System.currentTimeMillis() + ).also { entity -> + entity.id = database.value.topSiteDao().insertTopSite(entity) + } + } + + /** + * Returns a [LiveData] list of all the [TopSite] instances. + */ + fun getTopSites(): LiveData> { + return Transformations.map( + database.value.topSiteDao().getTopSites() + ) { list -> + list.map { entity -> TopSiteAdapter(entity) } + } + } + + /** + * Returns all [TopSite]s as a [DataSource.Factory]. + */ + fun getTopSitesPaged(): DataSource.Factory = database.value + .topSiteDao() + .getTopSitesPaged() + .map { entity -> TopSiteAdapter(entity) } + + /** + * Removes the given [TopSite]. + */ + fun removeTopSite(site: TopSite) { + val topSiteEntity = (site as TopSiteAdapter).entity + database.value.topSiteDao().deleteTopSite(topSiteEntity) + } +} diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/adapter/TopSiteAdapter.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/adapter/TopSiteAdapter.kt new file mode 100644 index 00000000000..2584dcd7d42 --- /dev/null +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/adapter/TopSiteAdapter.kt @@ -0,0 +1,33 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.top.sites.adapter + +import mozilla.components.feature.top.sites.TopSite +import mozilla.components.feature.top.sites.db.TopSiteEntity + +internal class TopSiteAdapter( + internal val entity: TopSiteEntity +) : TopSite { + override val id: Long + get() = entity.id!! + + override val title: String + get() = entity.title + + override val url: String + get() = entity.url + + override fun equals(other: Any?): Boolean { + if (other !is TopSiteAdapter) { + return false + } + + return entity == other.entity + } + + override fun hashCode(): Int { + return entity.hashCode() + } +} diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteDao.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteDao.kt new file mode 100644 index 00000000000..c5c40980b48 --- /dev/null +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteDao.kt @@ -0,0 +1,33 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.top.sites.db + +import androidx.lifecycle.LiveData +import androidx.paging.DataSource +import androidx.room.Dao +import androidx.room.Delete +import androidx.room.Insert +import androidx.room.Query +import androidx.room.Transaction + +/** + * Internal DAO for accessing [TopSiteEntity] instances. + */ +@Dao +internal interface TopSiteDao { + @Insert + fun insertTopSite(site: TopSiteEntity): Long + + @Delete + fun deleteTopSite(site: TopSiteEntity) + + @Transaction + @Query("SELECT * FROM top_sites") + fun getTopSites(): LiveData> + + @Transaction + @Query("SELECT * FROM top_sites") + fun getTopSitesPaged(): DataSource.Factory +} diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteDatabase.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteDatabase.kt new file mode 100644 index 00000000000..5b0d9620319 --- /dev/null +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteDatabase.kt @@ -0,0 +1,35 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.top.sites.db + +import android.content.Context +import androidx.room.Database +import androidx.room.Room +import androidx.room.RoomDatabase + +/** + * Internal database for storing top sites. + */ +@Database(entities = [TopSiteEntity::class], version = 1) +internal abstract class TopSiteDatabase : RoomDatabase() { + abstract fun topSiteDao(): TopSiteDao + + companion object { + @Volatile private var instance: TopSiteDatabase? = null + + @Synchronized + fun get(context: Context): TopSiteDatabase { + instance?.let { return it } + + return Room.databaseBuilder( + context, + TopSiteDatabase::class.java, + "top_sites" + ).build().also { + instance = it + } + } + } +} diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteEntity.kt b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteEntity.kt new file mode 100644 index 00000000000..cba04063221 --- /dev/null +++ b/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/TopSiteEntity.kt @@ -0,0 +1,28 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.feature.top.sites.db + +import androidx.room.ColumnInfo +import androidx.room.Entity +import androidx.room.PrimaryKey + +/** + * Internal entity representing a top site. + */ +@Entity(tableName = "top_sites") +internal data class TopSiteEntity( + @PrimaryKey(autoGenerate = true) + @ColumnInfo(name = "id") + var id: Long? = null, + + @ColumnInfo(name = "title") + var title: String, + + @ColumnInfo(name = "url") + var url: String, + + @ColumnInfo(name = "created_at") + var createdAt: Long = System.currentTimeMillis() +) diff --git a/components/feature/top-sites/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/components/feature/top-sites/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 00000000000..cf1c399ea81 --- /dev/null +++ b/components/feature/top-sites/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1,2 @@ +mock-maker-inline +// This allows mocking final classes (classes are final by default in Kotlin) From 52ffb8549dc25553b6f8ff8004e2e2be1dc888fa Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Wed, 11 Dec 2019 16:07:21 -0800 Subject: [PATCH 2/6] Part 1: Add PasswordsPing to support-telemetry-sync It is a verbatim copy of the HistoryPing. --- .../support/sync-telemetry/docs/metrics.md | 16 + .../support/sync-telemetry/metrics.yaml | 130 +++++++- components/support/sync-telemetry/pings.yaml | 11 + .../support/sync/telemetry/SyncTelemetry.kt | 53 ++++ .../sync/telemetry/SyncTelemetryTest.kt | 285 ++++++++++++++++++ 5 files changed, 493 insertions(+), 2 deletions(-) diff --git a/components/support/sync-telemetry/docs/metrics.md b/components/support/sync-telemetry/docs/metrics.md index 2d151200e03..a87af317312 100644 --- a/components/support/sync-telemetry/docs/metrics.md +++ b/components/support/sync-telemetry/docs/metrics.md @@ -9,6 +9,7 @@ This means you might have to go searching through the dependency tree to get a f - [bookmarks-sync](#bookmarks-sync) - [history-sync](#history-sync) + - [logins-sync](#logins-sync) ## bookmarks-sync @@ -42,6 +43,21 @@ The following metrics are added to the ping: | history_sync.started_at |[datetime](https://mozilla.github.io/glean/book/user/metrics/datetime.html) |Records when the history sync started. |[1](https://github.com/mozilla-mobile/android-components/pull/3092)||never | | history_sync.uid |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |The user's hashed Firefox Account ID. |[1](https://github.com/mozilla-mobile/android-components/pull/3092)||never | +## logins-sync +A ping sent for every logins/passwords sync. It doesn't include the `client_id` because it reports a hashed version of the user's Firefox Account ID. + +The following metrics are added to the ping: + +| Name | Type | Description | Data reviews | Extras | Expiration | +| --- | --- | --- | --- | --- | --- | +| logins_sync.failure_reason |[labeled_string](https://mozilla.github.io/glean/book/user/metrics/labeled_strings.html) |Records why the passwords sync failed: either due to an authentication error, unexpected exception, or other error. The error strings are truncated and sanitized to omit PII, like usernames and passwords. |[1](https://github.com/mozilla-mobile/android-components/pull/5294)|
  • other
  • unexpected
  • auth
|never | +| logins_sync.finished_at |[datetime](https://mozilla.github.io/glean/book/user/metrics/datetime.html) |Records when the passwords sync finished. This includes the time to download, apply, and upload all records. |[1](https://github.com/mozilla-mobile/android-components/pull/5294)||never | +| logins_sync.incoming |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Records incoming passwords record counts. `applied` is the number of incoming passwords entries that were successfully stored or updated in the local database. `failed_to_apply` is the number of entries that were ignored due to errors. `reconciled` is the number of entries with changes both locally and remotely that were merged. |[1](https://github.com/mozilla-mobile/android-components/pull/5294)|
  • applied
  • failed_to_apply
  • reconciled
|never | +| logins_sync.outgoing |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Records outgoing passwords record counts. `uploaded` is the number of records that were successfully sent to the server. `failed_to_upload` is the number of records that weren't uploaded, and will be retried on the next sync. |[1](https://github.com/mozilla-mobile/android-components/pull/5294)|
  • uploaded
  • failed_to_upload
|never | +| logins_sync.outgoing_batches |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |Records the number of batches needed to upload all outgoing records. The Sync server has a hard limit on the number of records (and request body bytes) on the number of records that can fit into a single batch, and large syncs may require multiple batches. |[1](https://github.com/mozilla-mobile/android-components/pull/5294)||never | +| logins_sync.started_at |[datetime](https://mozilla.github.io/glean/book/user/metrics/datetime.html) |Records when the passwords sync started. |[1](https://github.com/mozilla-mobile/android-components/pull/5294)||never | +| logins_sync.uid |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |The user's hashed Firefox Account ID. |[1](https://github.com/mozilla-mobile/android-components/pull/5294)||never | + diff --git a/components/support/sync-telemetry/metrics.yaml b/components/support/sync-telemetry/metrics.yaml index 2ddbd88fa17..48abd33bc23 100644 --- a/components/support/sync-telemetry/metrics.yaml +++ b/components/support/sync-telemetry/metrics.yaml @@ -4,8 +4,8 @@ $schema: moz://mozilla.org/schemas/glean/metrics/1-0-0 -# The `sync.history` and `sync.bookmarks` metrics use the same structure, -# but must be specified twice. We can't define them once and use +# `history-sync`, `logins-sync` and `bookmarks-sync` metrics all use the same structure, +# but must be specified individually. We can't define them once and use # `send_in_pings` because the stores might be synced in parallel, and we can't # guarantee that a ping for one store would be sent before the others. history_sync: @@ -134,6 +134,132 @@ history_sync: expires: never lifetime: ping +logins_sync: + uid: + type: string + description: > + The user's hashed Firefox Account ID. + send_in_pings: + - logins-sync + bugs: + - https://github.com/mozilla-mobile/android-components/issues/4556 + data_reviews: + - https://github.com/mozilla-mobile/android-components/pull/5294 + notification_emails: + - sync-core@mozilla.com + expires: never + lifetime: ping + started_at: + type: datetime + time_unit: millisecond + description: > + Records when the passwords sync started. + send_in_pings: + - logins-sync + bugs: + - https://github.com/mozilla-mobile/android-components/issues/4556 + data_reviews: + - https://github.com/mozilla-mobile/android-components/pull/5294 + notification_emails: + - sync-core@mozilla.com + expires: never + lifetime: ping + finished_at: + type: datetime + time_unit: millisecond + description: > + Records when the passwords sync finished. This includes the time to + download, apply, and upload all records. + send_in_pings: + - logins-sync + bugs: + - https://github.com/mozilla-mobile/android-components/issues/4556 + data_reviews: + - https://github.com/mozilla-mobile/android-components/pull/5294 + notification_emails: + - sync-core@mozilla.com + expires: never + lifetime: ping + incoming: + type: labeled_counter + labels: + - applied + - failed_to_apply + - reconciled + description: > + Records incoming passwords record counts. `applied` is the number of + incoming passwords entries that were successfully stored or updated in the + local database. `failed_to_apply` is the number of entries that were + ignored due to errors. `reconciled` is the number of entries with changes + both locally and remotely that were merged. + send_in_pings: + - logins-sync + bugs: + - https://github.com/mozilla-mobile/android-components/issues/4556 + data_reviews: + - https://github.com/mozilla-mobile/android-components/pull/5294 + notification_emails: + - sync-core@mozilla.com + expires: never + lifetime: ping + outgoing: + type: labeled_counter + labels: + - uploaded + - failed_to_upload + description: > + Records outgoing passwords record counts. `uploaded` is the number of + records that were successfully sent to the server. `failed_to_upload` + is the number of records that weren't uploaded, and will be retried + on the next sync. + send_in_pings: + - logins-sync + bugs: + - https://github.com/mozilla-mobile/android-components/issues/4556 + data_reviews: + - https://github.com/mozilla-mobile/android-components/pull/5294 + notification_emails: + - sync-core@mozilla.com + expires: never + lifetime: ping + outgoing_batches: + type: counter + description: > + Records the number of batches needed to upload all outgoing records. The + Sync server has a hard limit on the number of records (and request body + bytes) on the number of records that can fit into a single batch, and + large syncs may require multiple batches. + send_in_pings: + - logins-sync + bugs: + - https://github.com/mozilla-mobile/android-components/issues/4556 + data_reviews: + - https://github.com/mozilla-mobile/android-components/pull/5294 + notification_emails: + - sync-core@mozilla.com + expires: never + lifetime: ping + failure_reason: + type: labeled_string + labels: + - other + - unexpected + - auth + description: > + Records why the passwords sync failed: either due to an authentication + error, unexpected exception, or other error. The error strings are + truncated and sanitized to omit PII, like usernames and passwords. + send_in_pings: + - logins-sync + bugs: + - https://github.com/mozilla-mobile/android-components/issues/4556 + data_reviews: + - https://github.com/mozilla-mobile/android-components/pull/5294 + notification_emails: + - sync-core@mozilla.com + expires: never + lifetime: ping + bookmarks_sync: uid: type: string diff --git a/components/support/sync-telemetry/pings.yaml b/components/support/sync-telemetry/pings.yaml index db4f977cf47..b6ba738522a 100644 --- a/components/support/sync-telemetry/pings.yaml +++ b/components/support/sync-telemetry/pings.yaml @@ -26,3 +26,14 @@ bookmarks-sync: - sync-core@mozilla.com data_reviews: - https://github.com/mozilla-mobile/android-components/pull/3092 +logins-sync: + description: > + A ping sent for every logins/passwords sync. It doesn't include the `client_id` + because it reports a hashed version of the user's Firefox Account ID. + include_client_id: false + bugs: + - https://github.com/mozilla-mobile/android-components/issues/4556 + notification_emails: + - sync-core@mozilla.com + data_reviews: + - https://github.com/mozilla-mobile/android-components/pull/5294 diff --git a/components/support/sync-telemetry/src/main/java/mozilla/components/support/sync/telemetry/SyncTelemetry.kt b/components/support/sync-telemetry/src/main/java/mozilla/components/support/sync/telemetry/SyncTelemetry.kt index 7b2543b05ed..edc6dee2461 100644 --- a/components/support/sync-telemetry/src/main/java/mozilla/components/support/sync/telemetry/SyncTelemetry.kt +++ b/components/support/sync-telemetry/src/main/java/mozilla/components/support/sync/telemetry/SyncTelemetry.kt @@ -11,6 +11,7 @@ import mozilla.components.service.glean.private.LabeledMetricType import mozilla.components.service.glean.private.StringMetricType import mozilla.components.support.sync.telemetry.GleanMetrics.BookmarksSync import mozilla.components.support.sync.telemetry.GleanMetrics.HistorySync +import mozilla.components.support.sync.telemetry.GleanMetrics.LoginsSync import mozilla.components.support.sync.telemetry.GleanMetrics.Pings const val MAX_FAILURE_REASON_LENGTH = 100 @@ -71,6 +72,58 @@ object SyncTelemetry { return true } + /** + * Processes a passwords-related ping information from the [ping]. + * @return 'false' if global error was encountered, 'true' otherwise. + */ + @Suppress("ComplexMethod", "NestedBlockDepth") + fun processPasswordsPing(ping: SyncTelemetryPing, sendPing: () -> Unit = { Pings.loginsSync.send() }): Boolean { + ping.syncs.forEach eachSync@{ sync -> + sync.failureReason?.let { + recordFailureReason(it, LoginsSync.failureReason) + sendPing() + return false + } + sync.engines.forEach eachEngine@{ engine -> + if (engine.name != "passwords") { + return@eachEngine + } + LoginsSync.apply { + val base = BaseGleanSyncPing.fromEngineInfo(ping.uid, engine) + uid.set(base.uid) + startedAt.set(base.startedAt) + finishedAt.set(base.finishedAt) + if (base.applied > 0) { + // Since all Sync ping counters have `lifetime: ping`, and + // we send the ping immediately after, we don't need to + // reset the counters before calling `add`. + incoming["applied"].add(base.applied) + } + if (base.failedToApply > 0) { + incoming["failed_to_apply"].add(base.failedToApply) + } + if (base.reconciled > 0) { + incoming["reconciled"].add(base.reconciled) + } + if (base.uploaded > 0) { + outgoing["uploaded"].add(base.uploaded) + } + if (base.failedToUpload > 0) { + outgoing["failed_to_upload"].add(base.failedToUpload) + } + if (base.outgoingBatches > 0) { + outgoingBatches.add(base.outgoingBatches) + } + base.failureReason?.let { + recordFailureReason(it, failureReason) + } + } + sendPing() + } + } + return true + } + /** * Processes a bookmarks-related ping information from the [ping]. * @return 'false' if global error was encountered, 'true' otherwise. diff --git a/components/support/sync-telemetry/src/test/java/mozilla/components/support/sync/telemetry/SyncTelemetryTest.kt b/components/support/sync-telemetry/src/test/java/mozilla/components/support/sync/telemetry/SyncTelemetryTest.kt index 7fbc9fef828..16506d68bcf 100644 --- a/components/support/sync-telemetry/src/test/java/mozilla/components/support/sync/telemetry/SyncTelemetryTest.kt +++ b/components/support/sync-telemetry/src/test/java/mozilla/components/support/sync/telemetry/SyncTelemetryTest.kt @@ -16,6 +16,7 @@ import mozilla.appservices.sync15.SyncTelemetryPing import mozilla.appservices.sync15.ValidationInfo import mozilla.components.service.glean.testing.GleanTestRule import mozilla.components.support.sync.telemetry.GleanMetrics.BookmarksSync +import mozilla.components.support.sync.telemetry.GleanMetrics.LoginsSync import mozilla.components.support.sync.telemetry.GleanMetrics.HistorySync import mozilla.components.support.sync.telemetry.GleanMetrics.Pings import mozilla.components.support.test.robolectric.testContext @@ -324,6 +325,290 @@ class SyncTelemetryTest { assertFalse(noGlobalError) } + @Test + fun `sends passwords telemetry pings on success`() { + val noGlobalError = SyncTelemetry.processPasswordsPing(SyncTelemetryPing( + version = 1, + uid = "abc123", + syncs = listOf( + SyncInfo( + at = now, + took = 10000, + engines = listOf( + EngineInfo( + name = "history", + at = now + 5, + took = 5000, + incoming = IncomingInfo( + applied = 10, + failed = 2, + newFailed = 3, + reconciled = 2 + ), + outgoing = emptyList(), + failureReason = null, + validation = null + ), + EngineInfo( + name = "passwords", + at = now, + took = 5000, + incoming = IncomingInfo( + applied = 5, + failed = 4, + newFailed = 3, + reconciled = 2 + ), + outgoing = listOf( + OutgoingInfo( + sent = 10, + failed = 5 + ), + OutgoingInfo( + sent = 4, + failed = 2 + ) + ), + failureReason = null, + validation = null + ) + ), + failureReason = null + ), + SyncInfo( + at = now + 10, + took = 5000, + engines = listOf( + EngineInfo( + name = "passwords", + at = now + 10, + took = 5000, + incoming = null, + outgoing = emptyList(), + failureReason = null, + validation = null + ) + ), + failureReason = null + ) + ), + events = emptyList() + )) { + when (pingCount) { + 0 -> { + LoginsSync.apply { + assertEquals("abc123", uid.testGetValue()) + assertEquals(now, startedAt.testGetValue().asSeconds()) + assertEquals(now + 5, finishedAt.testGetValue().asSeconds()) + assertEquals(5, incoming["applied"].testGetValue()) + assertEquals(7, incoming["failed_to_apply"].testGetValue()) + assertEquals(2, incoming["reconciled"].testGetValue()) + assertEquals(14, outgoing["uploaded"].testGetValue()) + assertEquals(7, outgoing["failed_to_upload"].testGetValue()) + assertEquals(2, outgoingBatches.testGetValue()) + } + } + 1 -> { + LoginsSync.apply { + assertEquals("abc123", uid.testGetValue()) + assertEquals(now + 10, startedAt.testGetValue().asSeconds()) + assertEquals(now + 15, finishedAt.testGetValue().asSeconds()) + assertTrue(listOf( + incoming["applied"], + incoming["failed_to_apply"], + incoming["reconciled"], + outgoing["uploaded"], + outgoing["failed_to_upload"], + outgoingBatches + ).none { it.testHasValue() }) + } + } + else -> fail() + } + // We still need to send the ping, so that the counters are + // cleared out between calls to `sendPasswordsPing`. + Pings.loginsSync.send() + pingCount++ + } + + assertEquals(2, pingCount) + assertTrue(noGlobalError) + } + + @Test + fun `sends passwords telemetry pings on engine failure`() { + val noGlobalError = SyncTelemetry.processPasswordsPing(SyncTelemetryPing( + version = 1, + uid = "abc123", + syncs = listOf( + SyncInfo( + at = now, + took = 5000, + engines = listOf( + // We should ignore any engines that aren't + // passwords. + EngineInfo( + name = "bookmarks", + at = now + 1, + took = 1000, + incoming = null, + outgoing = emptyList(), + failureReason = FailureReason(FailureName.Unknown, "Boxes not locked"), + validation = null + ), + // Multiple passwords engine syncs per sync isn't + // expected, but it's easier to test the + // different failure types this way, instead of + // creating a top-level `SyncInfo` for each + // one. + EngineInfo( + name = "passwords", + at = now + 2, + took = 1000, + incoming = null, + outgoing = emptyList(), + failureReason = FailureReason(FailureName.Shutdown), + validation = null + ), + EngineInfo( + name = "passwords", + at = now + 3, + took = 1000, + incoming = null, + outgoing = emptyList(), + failureReason = FailureReason(FailureName.Unknown, "Synergies not aligned"), + validation = null + ), + EngineInfo( + name = "passwords", + at = now + 4, + took = 1000, + incoming = null, + outgoing = emptyList(), + failureReason = FailureReason(FailureName.Http, code = 418), + validation = null + ) + ), + failureReason = null + ), + // ...But, just in case, we also test multiple top-level + // syncs. + SyncInfo( + at = now + 5, + took = 4000, + engines = listOf( + EngineInfo( + name = "passwords", + at = now + 6, + took = 1000, + incoming = null, + outgoing = emptyList(), + failureReason = FailureReason(FailureName.Auth, "Splines not reticulated", 999), + validation = null + ), + EngineInfo( + name = "passwords", + at = now + 7, + took = 1000, + incoming = null, + outgoing = emptyList(), + failureReason = FailureReason(FailureName.Unexpected, "Kaboom!"), + validation = null + ), + EngineInfo( + name = "passwords", + at = now + 8, + took = 1000, + incoming = null, + outgoing = emptyList(), + failureReason = FailureReason(FailureName.Other, "Qualia unsynchronized"), // other + validation = null + ) + ), + failureReason = null + ) + ), + events = emptyList() + )) { + when (pingCount) { + 0 -> { + // Shutdown errors shouldn't be reported at all. + assertTrue(listOf( + "other", + "unexpected", + "auth" + ).none { LoginsSync.failureReason[it].testHasValue() }) + } + 1 -> LoginsSync.apply { + assertEquals("Synergies not aligned", failureReason["other"].testGetValue()) + assertFalse(failureReason["unexpected"].testHasValue()) + assertFalse(failureReason["auth"].testHasValue()) + } + 2 -> LoginsSync.apply { + assertEquals("Unexpected error: 418", failureReason["unexpected"].testGetValue()) + assertFalse(failureReason["other"].testHasValue()) + assertFalse(failureReason["auth"].testHasValue()) + } + 3 -> LoginsSync.apply { + assertEquals("Splines not reticulated", failureReason["auth"].testGetValue()) + assertFalse(failureReason["other"].testHasValue()) + assertFalse(failureReason["unexpected"].testHasValue()) + } + 4 -> LoginsSync.apply { + assertEquals("Kaboom!", failureReason["unexpected"].testGetValue()) + assertFalse(failureReason["other"].testHasValue()) + assertFalse(failureReason["auth"].testHasValue()) + } + 5 -> LoginsSync.apply { + assertEquals("Qualia unsynchronized", failureReason["other"].testGetValue()) + assertFalse(failureReason["unexpected"].testHasValue()) + assertFalse(failureReason["auth"].testHasValue()) + } + else -> fail() + } + // We still need to send the ping, so that the counters are + // cleared out between calls to `sendPasswordsPing`. + Pings.loginsSync.send() + pingCount++ + } + + assertEquals(6, pingCount) + assertTrue(noGlobalError) + } + + @Test + fun `sends passwords telemetry pings on sync failure`() { + val noGlobalError = SyncTelemetry.processPasswordsPing(SyncTelemetryPing( + version = 1, + uid = "abc123", + syncs = listOf( + SyncInfo( + at = now, + took = 5000, + engines = emptyList(), + failureReason = FailureReason(FailureName.Unknown, "Synergies not aligned") + ) + ), + events = emptyList() + )) { + when (pingCount) { + 0 -> LoginsSync.apply { + assertEquals("Synergies not aligned", failureReason["other"].testGetValue()) + assertFalse(failureReason["unexpected"].testHasValue()) + assertFalse(failureReason["auth"].testHasValue()) + } + else -> fail() + } + // We still need to send the ping, so that the counters are + // cleared out between calls to `sendHistoryPing`. + Pings.loginsSync.send() + pingCount++ + } + + assertEquals(1, pingCount) + assertFalse(noGlobalError) + } + @Test fun `sends bookmarks telemetry pings on success`() { val noGlobalError = SyncTelemetry.processBookmarksPing(SyncTelemetryPing( From 8fab95d26195dade294b0625d282c2fb154683be Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Wed, 11 Dec 2019 16:07:55 -0800 Subject: [PATCH 3/6] Part 2: Enable emitting of the passwords ping via SyncManager --- .../service/fxa/sync/WorkManagerSyncManager.kt | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt index 6098a238655..f6701831da8 100644 --- a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt @@ -436,19 +436,20 @@ class WorkManagerSyncWorker( // Process telemetry. syncResult.telemetry?.let { - // Yes, this is complete garbage. + // Yes, this is non-ideal... // But, what this does: individual 'process' function will report global sync errors - // as part of its corresponding ping. We don't want to report a global sync error twice, + // as part of its corresponding ping. We don't want to report a global sync error multiple times, // so we check for the boolean flag that indicates if this happened or not. - // There's a complete mismatch between what Glean supports and what we need - // it to do here. They don't support "nested metrics" and so we resort to these hacks. + // There's a complete mismatch between what Glean supports and what we need it to do here. + // Glean doesn't support "nested metrics" and so we resort to these hacks. // It shouldn't matter in which order these 'process' functions are called. - val noGlobalErrorsReported = SyncTelemetry.processBookmarksPing(it) + var noGlobalErrorsReported = SyncTelemetry.processBookmarksPing(it) if (noGlobalErrorsReported) { - SyncTelemetry.processHistoryPing(it) + noGlobalErrorsReported = SyncTelemetry.processHistoryPing(it) + } + if (noGlobalErrorsReported) { + SyncTelemetry.processPasswordsPing(it) } - // TODO telemetry for the passwords engine. - // See https://github.com/mozilla-mobile/android-components/issues/4556 } // Finally, declare success, failure or request a retry based on 'sync status'. From 39292a17dd81390433f3b49113fc809d072a8361 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Wed, 11 Dec 2019 16:08:30 -0800 Subject: [PATCH 4/6] Part 3: Enable emitting of the password ping via AsyncLoginsStorage's sync --- components/service/sync-logins/build.gradle | 1 + .../components/service/sync/logins/AsyncLoginsStorage.kt | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/components/service/sync-logins/build.gradle b/components/service/sync-logins/build.gradle index c1ff64990af..a6c55da95c4 100644 --- a/components/service/sync-logins/build.gradle +++ b/components/service/sync-logins/build.gradle @@ -36,6 +36,7 @@ dependencies { api project(':concept-sync') implementation project(':concept-storage') + implementation project(':support-sync-telemetry') implementation Dependencies.kotlin_stdlib implementation Dependencies.kotlin_coroutines diff --git a/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/AsyncLoginsStorage.kt b/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/AsyncLoginsStorage.kt index c219bfaff43..31503a090d7 100644 --- a/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/AsyncLoginsStorage.kt +++ b/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/AsyncLoginsStorage.kt @@ -17,6 +17,7 @@ import mozilla.components.concept.sync.SyncAuthInfo import mozilla.components.concept.sync.SyncStatus import mozilla.appservices.sync15.SyncTelemetryPing import mozilla.components.concept.sync.LockableStore +import mozilla.components.support.sync.telemetry.SyncTelemetry /** * This type contains the set of information required to successfully @@ -331,7 +332,9 @@ open class AsyncLoginsStorageAdapter(private val wrapped: T) override fun sync(syncInfo: SyncUnlockInfo): Deferred { return scope.async { - wrapped.sync(syncInfo) + val ping = wrapped.sync(syncInfo) + SyncTelemetry.processPasswordsPing(ping) + ping } } From 0ffd3702451f4bd0b9613c49a198436c3242cf72 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Wed, 11 Dec 2019 16:08:49 -0800 Subject: [PATCH 5/6] Post: Add password synchronization to samples-sync app --- samples/sync/build.gradle | 2 ++ .../org/mozilla/samples/sync/MainActivity.kt | 33 +++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/samples/sync/build.gradle b/samples/sync/build.gradle index 73705c628fe..4a6b8b562b4 100644 --- a/samples/sync/build.gradle +++ b/samples/sync/build.gradle @@ -39,9 +39,11 @@ dependencies { implementation project(':concept-storage') implementation project(':browser-storage-sync') implementation project(':service-firefox-accounts') + implementation project(':service-sync-logins') implementation project(':support-rustlog') implementation project(':support-rusthttp') implementation project(':lib-fetch-httpurlconnection') + implementation project(':lib-dataprotect') implementation Dependencies.kotlin_reflect implementation Dependencies.androidx_recyclerview diff --git a/samples/sync/src/main/java/org/mozilla/samples/sync/MainActivity.kt b/samples/sync/src/main/java/org/mozilla/samples/sync/MainActivity.kt index 6de82d956a3..f8e6d75a601 100644 --- a/samples/sync/src/main/java/org/mozilla/samples/sync/MainActivity.kt +++ b/samples/sync/src/main/java/org/mozilla/samples/sync/MainActivity.kt @@ -11,6 +11,7 @@ import android.widget.TextView import android.widget.Toast import kotlinx.android.synthetic.main.activity_main.syncStatus import androidx.appcompat.app.AppCompatActivity +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -30,6 +31,8 @@ import mozilla.components.concept.sync.DeviceType import mozilla.components.concept.sync.DeviceEventsObserver import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.Profile +import mozilla.components.lib.dataprotect.SecureAbove22Preferences +import mozilla.components.lib.dataprotect.generateEncryptionKey import mozilla.components.service.fxa.manager.FxaAccountManager import mozilla.components.service.fxa.DeviceConfig import mozilla.components.service.fxa.ServerConfig @@ -42,6 +45,8 @@ import mozilla.components.service.fxa.FxaAuthData import mozilla.components.service.fxa.SyncEngine import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.service.fxa.toAuthType +import mozilla.components.service.sync.logins.AsyncLoginsStorageAdapter +import mozilla.components.service.sync.logins.SyncableLoginsStore import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.base.log.sink.AndroidLogSink import mozilla.components.support.rusthttp.RustHttpConfig @@ -49,6 +54,8 @@ import mozilla.components.support.rustlog.RustLog import java.lang.Exception import kotlin.coroutines.CoroutineContext +private const val PASSWORDS_ENCRYPTION_KEY_STRENGTH = 256 + class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteListener, @@ -62,9 +69,26 @@ class MainActivity : PlacesBookmarksStorage(this) } - init { + private val securePreferences by lazy { SecureAbove22Preferences(this, "key_store") } + + private val passwordsStorage by lazy { + val passwordsEncryptionKey = securePreferences.getString(SyncEngine.Passwords.nativeName) + ?: generateEncryptionKey(PASSWORDS_ENCRYPTION_KEY_STRENGTH).also { + securePreferences.putString(SyncEngine.Passwords.nativeName, it) + } + + SyncableLoginsStore( + AsyncLoginsStorageAdapter.forDatabase(this.getDatabasePath("logins.sqlite").absolutePath) + ) { + CompletableDeferred(passwordsEncryptionKey) + } + } + + private fun initializeSyncProviders() { GlobalSyncableStoreProvider.configureStore(SyncEngine.History to historyStorage) GlobalSyncableStoreProvider.configureStore(SyncEngine.Bookmarks to bookmarksStorage) + GlobalSyncableStoreProvider.configureStore(SyncEngine.Passwords to passwordsStorage) + GlobalSyncableStoreProvider.configureKeyStorage(securePreferences) } private val accountManager by lazy { @@ -77,7 +101,10 @@ class MainActivity : capabilities = setOf(DeviceCapability.SEND_TAB), secureStateAtRest = true ), - SyncConfig(setOf(SyncEngine.History, SyncEngine.Bookmarks), syncPeriodInMinutes = 15L) + SyncConfig( + setOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Passwords), + syncPeriodInMinutes = 15L + ) ) } @@ -99,6 +126,8 @@ class MainActivity : Log.addSink(AndroidLogSink()) + initializeSyncProviders() + setContentView(R.layout.activity_main) findViewById(R.id.buttonSignIn).setOnClickListener { From 5d3f7f4fc4bcfaafda9dc9068a760d207f9a497a Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Wed, 11 Dec 2019 16:17:25 -0800 Subject: [PATCH 6/6] Post: add telemetry notices and update changelog --- components/service/sync-logins/README.md | 5 +++++ docs/changelog.md | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/components/service/sync-logins/README.md b/components/service/sync-logins/README.md index 2e2d8e16c0f..04e6c57e295 100644 --- a/components/service/sync-logins/README.md +++ b/components/service/sync-logins/README.md @@ -2,6 +2,11 @@ A library for integrating with Firefox Sync - Logins. +## Before using this component +Products sending telemetry and using this component *must request* a data-review following [this process](https://wiki.mozilla.org/Firefox/Data_Collection). +This component provides data collection using the [Glean SDK](https://mozilla.github.io/glean/book/index.html). +The list of metrics being collected is available in the [metrics documentation](../../support/sync-telemetry/docs/metrics.md). + ## Motivation The **Firefox Sync - Logins Component** provides a way for Android applications to do the following: diff --git a/docs/changelog.md b/docs/changelog.md index ab6f2e97d4e..ebd6f6390ac 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -25,6 +25,20 @@ permalink: /changelog/ context.components.appLinksInterceptor.onLaunchIntentRequest(engineSession, uri, hasUserGesture, isSameDomain) ``` +* **support-telemetry-sync** + * Added new telemetry ping, to support password sync: `passwords_sync`. + +* **service-firefox-accounts** + * 🕵️ **New Telemetry Notice** + * Added telemetry for password sync, via the new `passwords_sync` in **support-telemetry-sync** + +* **sync-logins** + * 🕵️ **New Telemetry Notice** + * Added telemetry for password sync, via the new `passwords_sync` in **support-telemetry-sync** + +* **samples-sync** + * Added support for password synchronization (not reflected in the UI, but demonstrates how to integrate the component). + # 26.0.0 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v25.0.0...v26.0.0) @@ -39,6 +53,7 @@ permalink: /changelog/ * `browser-engine-gecko-beta`: GeckoView 72.0 * `browser-engine-gecko-nightly`: GeckoView 73.0 +<<<<<<< HEAD * **browser-engine-system** and **browser-engine-gecko-nightly** * Added `EngineView.canClearSelection()` and `EngineView.clearSelection()` for clearing the selection.