Pier Angelo Vendrame pushed to branch tor-browser-128.3.0esr-14.0-1 at The Tor Project / Applications / Tor Browser

Commits: 264c385b by Dan Ballard at 2024-09-26T17:20:57+00:00 fixup! [android] Disable features and functionality

Bug 43113: remove remote settings and SERPTelemetry

- - - - -

4 changed files:

- mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/telemetry/SerpTelemetryRepository.kt - mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/telemetry/BaseSearchTelemetryTest.kt - mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/telemetry/SerpTelemetryRepositoryTest.kt - mobile/android/android-components/components/support/remotesettings/src/main/java/mozilla/components/support/remotesettings/RemoteSettingsClient.kt

Changes:

===================================== mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/telemetry/SerpTelemetryRepository.kt ===================================== @@ -8,7 +8,7 @@ import mozilla.appservices.remotesettings.RemoteSettingsResponse import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.ktx.android.org.json.asSequence import mozilla.components.support.ktx.android.org.json.toList -import mozilla.components.support.remotesettings.RemoteSettingsClient +//import mozilla.components.support.remotesettings.RemoteSettingsClient import mozilla.components.support.remotesettings.RemoteSettingsResult import org.jetbrains.annotations.VisibleForTesting import org.json.JSONArray @@ -23,22 +23,22 @@ internal const val REMOTE_ENDPOINT_BUCKET_NAME = "main" * Parse SERP Telemetry json from remote config. */ class SerpTelemetryRepository( - rootStorageDirectory: File, +// rootStorageDirectory: File, private val readJson: () -> JSONObject, - collectionName: String, - serverUrl: String = REMOTE_PROD_ENDPOINT_URL, - bucketName: String = REMOTE_ENDPOINT_BUCKET_NAME, +// collectionName: String, +// serverUrl: String = REMOTE_PROD_ENDPOINT_URL, +// bucketName: String = REMOTE_ENDPOINT_BUCKET_NAME, ) { val logger = Logger("SerpTelemetryRepository") private var providerList: List<SearchProviderModel> = emptyList()

- @VisibleForTesting - internal var remoteSettingsClient = RemoteSettingsClient( - serverUrl = serverUrl, - bucketName = bucketName, - collectionName = collectionName, - storageRootDirectory = rootStorageDirectory, - ) +// @VisibleForTesting +// internal var remoteSettingsClient = RemoteSettingsClient( +// serverUrl = serverUrl, +// bucketName = bucketName, +// collectionName = collectionName, +// storageRootDirectory = rootStorageDirectory, +// )

/** * Provides list of search providers from cache or dump and fetches from remotes server . @@ -65,7 +65,7 @@ class SerpTelemetryRepository( val remoteResponse = fetchRemoteResponse() if (remoteResponse.lastModified > cacheLastModified) { providerList = parseRemoteResponse(remoteResponse) - writeToCache(remoteResponse) + //writeToCache(remoteResponse) } }

@@ -73,8 +73,9 @@ class SerpTelemetryRepository( * Writes data to local cache. */ @VisibleForTesting - internal suspend fun writeToCache(records: RemoteSettingsResponse): RemoteSettingsResult { - return remoteSettingsClient.write(records) + internal suspend fun writeToCache(/*records: RemoteSettingsResponse*/): RemoteSettingsResult { + return RemoteSettingsResult.NetworkFailure(Exception("Bug-43113: no remote fetching")) +// return remoteSettingsClient.write(records) }

/** @@ -104,12 +105,12 @@ class SerpTelemetryRepository( */ @VisibleForTesting internal suspend fun fetchRemoteResponse(): RemoteSettingsResponse { - val result = remoteSettingsClient.fetch() - return if (result is RemoteSettingsResult.Success) { - result.response - } else { - RemoteSettingsResponse(emptyList(), 0u) - } +// val result = remoteSettingsClient.fetch() +// return if (result is RemoteSettingsResult.Success) { +// result.response +// } else { + return RemoteSettingsResponse(emptyList(), 0u) +// } }

/** @@ -117,16 +118,16 @@ class SerpTelemetryRepository( */ @VisibleForTesting internal suspend fun loadProvidersFromCache(): Pair<ULong, List<SearchProviderModel>> { - val result = remoteSettingsClient.read() - return if (result is RemoteSettingsResult.Success) { - val response = result.response.records.mapNotNull { - it.fields.toSearchProviderModel() - } - val lastModified = result.response.lastModified - Pair(lastModified, response) - } else { - Pair(0u, emptyList()) - } +// val result = remoteSettingsClient.read() +// return if (result is RemoteSettingsResult.Success) { +// val response = result.response.records.mapNotNull { +// it.fields.toSearchProviderModel() +// } +// val lastModified = result.response.lastModified +// Pair(lastModified, response) +// } else { + return Pair(0u, emptyList()) +// } } }

===================================== mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/telemetry/BaseSearchTelemetryTest.kt ===================================== @@ -27,161 +27,161 @@ import java.io.File @RunWith(AndroidJUnit4::class) class BaseSearchTelemetryTest {

- private lateinit var baseTelemetry: BaseSearchTelemetry - private lateinit var handler: BaseSearchTelemetry.SearchTelemetryMessageHandler - - @Mock - private lateinit var mockRepo: SerpTelemetryRepository - - private val mockReadJson: () -> JSONObject = mock() - private val mockRootStorageDirectory: File = mock() - - private fun createMockProviderList(): List<SearchProviderModel> = listOf( - SearchProviderModel( - schema = 1698656464939, - taggedCodes = listOf("monline_7_dg"), - telemetryId = "baidu", - organicCodes = emptyList(), - codeParamName = "tn", - queryParamNames = listOf("wd"), - searchPageRegexp = "^https://(?:m%7Cwww)%5C%5C%5C%5C.baidu%5C%5C%5C%5C.com/(?:s%7Cbaidu)", - followOnParamNames = listOf("oq"), - extraAdServersRegexps = listOf("^https?://www\\.baidu\\.com/baidu\\.php?"), - expectedOrganicCodes = emptyList(), - ), - ) - - private val rawJson = """ - { - "data": [ - { - "schema": 1698656464939, - "taggedCodes": [ - "monline_7_dg" - ], - "telemetryId": "baidu", - "organicCodes": [], - "codeParamName": "tn", - "queryParamNames": [ - "wd" - ], - "searchPageRegexp": "^https://(?:m%7Cwww)%5C%5C.baidu%5C%5C.com/(?:s%7Cbaidu)", - "followOnParamNames": [ - "oq" - ], - "extraAdServersRegexps": [ - "^https?://www\.baidu\.com/baidu\.php?" - ], - "id": "19c434a3-d173-4871-9743-290ac92a3f6a", - "last_modified": 1698666532326 - }], - "timestamp": 16 -} - """.trimIndent() - - @Before - fun setup() { - baseTelemetry = spy( - object : BaseSearchTelemetry() { - override suspend fun install( - engine: Engine, - store: BrowserStore, - providerList: List<SearchProviderModel>, - ) { - // mock, do nothing - } - - override fun processMessage(message: JSONObject) { - // mock, do nothing - } - }, - ) - handler = baseTelemetry.SearchTelemetryMessageHandler() - mockRepo = spy(SerpTelemetryRepository(mockRootStorageDirectory, mockReadJson, "test")) - } - - @Test - fun `GIVEN an engine WHEN installWebExtension is called THEN the provided extension is installed in engine`() { - val engine: Engine = mock() - val store: BrowserStore = mock() - val id = "id" - val resourceUrl = "resourceUrl" - val messageId = "messageId" - val extensionInfo = ExtensionInfo(id, resourceUrl, messageId) - - baseTelemetry.installWebExtension(engine, store, extensionInfo) - - verify(engine).installBuiltInWebExtension( - id = eq(id), - url = eq(resourceUrl), - onSuccess = any(), - onError = any(), - ) - } - - @Test - fun `GIVEN a search provider does not exist for the url WHEN getProviderForUrl is called THEN return null`() { - val url = "https://www.mozilla.com/search?q=firefox" - baseTelemetry.providerList = createMockProviderList() - - assertEquals(null, baseTelemetry.getProviderForUrl(url)) - } - - @Test(expected = IllegalStateException::class) - fun `GIVEN an extension message WHEN that cannot be processed THEN throw IllegalStateException`() { - val message = "message" - - handler.onMessage(message, mock()) - } - - @Test - fun `GIVEN an extension message WHEN received THEN pass it to processMessage`() { - val message = JSONObject() - - handler.onMessage(message, mock()) - - verify(baseTelemetry).processMessage(message) - } - - @Test - fun `GIVEN empty cacheResponse WHEN initializeProviderList is called THEN update providerList`(): Unit = - runBlocking { - val localResponse = JSONObject(rawJson) - val cacheResponse: Pair<ULong, List<SearchProviderModel>> = Pair(0u, emptyList()) - - `when`(mockRepo.loadProvidersFromCache()).thenReturn(cacheResponse) - doAnswer { - localResponse - }.`when`(mockReadJson)() - - `when`(mockRepo.parseLocalPreinstalledData(localResponse)).thenReturn(createMockProviderList()) - doReturn(Unit).`when`(mockRepo).fetchRemoteResponse(any()) - - baseTelemetry.setProviderList(mockRepo.updateProviderList()) - - assertEquals(baseTelemetry.providerList.toString(), createMockProviderList().toString()) - } - - @Test - fun `GIVEN non-empty cacheResponse WHEN initializeProviderList is called THEN update providerList`(): Unit = - runBlocking { - val localResponse = JSONObject(rawJson) - val cacheResponse: Pair<ULong, List<SearchProviderModel>> = Pair(123u, createMockProviderList()) - - `when`(mockRepo.loadProvidersFromCache()).thenReturn(cacheResponse) - doAnswer { - localResponse - }.`when`(mockReadJson)() - doReturn(Unit).`when`(mockRepo).fetchRemoteResponse(any()) - - baseTelemetry.setProviderList(mockRepo.updateProviderList()) - - assertEquals(baseTelemetry.providerList.toString(), createMockProviderList().toString()) - } - - fun getProviderForUrl(url: String): SearchProviderModel? { - return createMockProviderList().find { provider -> - provider.searchPageRegexp.pattern.toRegex().containsMatchIn(url) - } - } +// private lateinit var baseTelemetry: BaseSearchTelemetry +// private lateinit var handler: BaseSearchTelemetry.SearchTelemetryMessageHandler +// +// @Mock +// private lateinit var mockRepo: SerpTelemetryRepository +// +// private val mockReadJson: () -> JSONObject = mock() +// private val mockRootStorageDirectory: File = mock() +// +// private fun createMockProviderList(): List<SearchProviderModel> = listOf( +// SearchProviderModel( +// schema = 1698656464939, +// taggedCodes = listOf("monline_7_dg"), +// telemetryId = "baidu", +// organicCodes = emptyList(), +// codeParamName = "tn", +// queryParamNames = listOf("wd"), +// searchPageRegexp = "^https://(?:m%7Cwww)%5C%5C%5C%5C.baidu%5C%5C%5C%5C.com/(?:s%7Cbaidu)", +// followOnParamNames = listOf("oq"), +// extraAdServersRegexps = listOf("^https?://www\\.baidu\\.com/baidu\\.php?"), +// expectedOrganicCodes = emptyList(), +// ), +// ) +// +// private val rawJson = """ +// { +// "data": [ +// { +// "schema": 1698656464939, +// "taggedCodes": [ +// "monline_7_dg" +// ], +// "telemetryId": "baidu", +// "organicCodes": [], +// "codeParamName": "tn", +// "queryParamNames": [ +// "wd" +// ], +// "searchPageRegexp": "^https://(?:m%7Cwww)%5C%5C.baidu%5C%5C.com/(?:s%7Cbaidu)", +// "followOnParamNames": [ +// "oq" +// ], +// "extraAdServersRegexps": [ +// "^https?://www\.baidu\.com/baidu\.php?" +// ], +// "id": "19c434a3-d173-4871-9743-290ac92a3f6a", +// "last_modified": 1698666532326 +// }], +// "timestamp": 16 +//} +// """.trimIndent() +// +// @Before +// fun setup() { +// baseTelemetry = spy( +// object : BaseSearchTelemetry() { +// override suspend fun install( +// engine: Engine, +// store: BrowserStore, +// providerList: List<SearchProviderModel>, +// ) { +// // mock, do nothing +// } +// +// override fun processMessage(message: JSONObject) { +// // mock, do nothing +// } +// }, +// ) +// handler = baseTelemetry.SearchTelemetryMessageHandler() +// mockRepo = spy(SerpTelemetryRepository(mockRootStorageDirectory, mockReadJson, "test")) +// } +// +// @Test +// fun `GIVEN an engine WHEN installWebExtension is called THEN the provided extension is installed in engine`() { +// val engine: Engine = mock() +// val store: BrowserStore = mock() +// val id = "id" +// val resourceUrl = "resourceUrl" +// val messageId = "messageId" +// val extensionInfo = ExtensionInfo(id, resourceUrl, messageId) +// +// baseTelemetry.installWebExtension(engine, store, extensionInfo) +// +// verify(engine).installBuiltInWebExtension( +// id = eq(id), +// url = eq(resourceUrl), +// onSuccess = any(), +// onError = any(), +// ) +// } +// +// @Test +// fun `GIVEN a search provider does not exist for the url WHEN getProviderForUrl is called THEN return null`() { +// val url = "https://www.mozilla.com/search?q=firefox" +// baseTelemetry.providerList = createMockProviderList() +// +// assertEquals(null, baseTelemetry.getProviderForUrl(url)) +// } +// +// @Test(expected = IllegalStateException::class) +// fun `GIVEN an extension message WHEN that cannot be processed THEN throw IllegalStateException`() { +// val message = "message" +// +// handler.onMessage(message, mock()) +// } +// +// @Test +// fun `GIVEN an extension message WHEN received THEN pass it to processMessage`() { +// val message = JSONObject() +// +// handler.onMessage(message, mock()) +// +// verify(baseTelemetry).processMessage(message) +// } +// +// @Test +// fun `GIVEN empty cacheResponse WHEN initializeProviderList is called THEN update providerList`(): Unit = +// runBlocking { +// val localResponse = JSONObject(rawJson) +// val cacheResponse: Pair<ULong, List<SearchProviderModel>> = Pair(0u, emptyList()) +// +// `when`(mockRepo.loadProvidersFromCache()).thenReturn(cacheResponse) +// doAnswer { +// localResponse +// }.`when`(mockReadJson)() +// +// `when`(mockRepo.parseLocalPreinstalledData(localResponse)).thenReturn(createMockProviderList()) +// doReturn(Unit).`when`(mockRepo).fetchRemoteResponse(any()) +// +// baseTelemetry.setProviderList(mockRepo.updateProviderList()) +// +// assertEquals(baseTelemetry.providerList.toString(), createMockProviderList().toString()) +// } +// +// @Test +// fun `GIVEN non-empty cacheResponse WHEN initializeProviderList is called THEN update providerList`(): Unit = +// runBlocking { +// val localResponse = JSONObject(rawJson) +// val cacheResponse: Pair<ULong, List<SearchProviderModel>> = Pair(123u, createMockProviderList()) +// +// `when`(mockRepo.loadProvidersFromCache()).thenReturn(cacheResponse) +// doAnswer { +// localResponse +// }.`when`(mockReadJson)() +// doReturn(Unit).`when`(mockRepo).fetchRemoteResponse(any()) +// +// baseTelemetry.setProviderList(mockRepo.updateProviderList()) +// +// assertEquals(baseTelemetry.providerList.toString(), createMockProviderList().toString()) +// } +// +// fun getProviderForUrl(url: String): SearchProviderModel? { +// return createMockProviderList().find { provider -> +// provider.searchPageRegexp.pattern.toRegex().containsMatchIn(url) +// } +// } }

===================================== mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/telemetry/SerpTelemetryRepositoryTest.kt ===================================== @@ -23,70 +23,70 @@ import org.robolectric.RobolectricTestRunner

@RunWith(RobolectricTestRunner::class) class SerpTelemetryRepositoryTest { - @Mock - private lateinit var mockRemoteSettingsClient: RemoteSettingsClient - - private lateinit var serpTelemetryRepository: SerpTelemetryRepository - - @Before - fun setup() { - MockitoAnnotations.openMocks(this) - serpTelemetryRepository = SerpTelemetryRepository( - rootStorageDirectory = mock(), - readJson = mock(), - collectionName = "", - serverUrl = "https://test.server", - bucketName = "", - ) - - serpTelemetryRepository.remoteSettingsClient = mockRemoteSettingsClient - } - - @Test - fun `GIVEN non-empty response WHEN writeToCache is called THEN the result is a success`() = runBlocking { - val records = listOf( - RemoteSettingsRecord("1", 123u, false, null, JSONObject()), - RemoteSettingsRecord("2", 456u, true, null, JSONObject()), - ) - val response = RemoteSettingsResponse(records, 125614567U) - - `when`(mockRemoteSettingsClient.write(response)) - .thenReturn(RemoteSettingsResult.Success(response)) - - val result = serpTelemetryRepository.writeToCache(response) - - assertTrue(result is RemoteSettingsResult.Success) - assertEquals(response, (result as RemoteSettingsResult.Success).response) - } - - @Test - fun `GIVEN non-empty response WHEN fetchRemoteResponse is called THEN the result is equal to the response`() = runBlocking { - val records = listOf( - RemoteSettingsRecord("1", 123u, false, null, JSONObject()), - RemoteSettingsRecord("2", 456u, true, null, JSONObject()), - ) - val response = RemoteSettingsResponse(records, 125614567U) - `when`(mockRemoteSettingsClient.fetch()) - .thenReturn(RemoteSettingsResult.Success(response)) - - val result = serpTelemetryRepository.fetchRemoteResponse() - - assertEquals(response, result) - } - - @Test - fun `GIVEN non-empty response WHEN loadProvidersFromCache is called THEN the result is equal to the response`() = runBlocking { - val records = listOf( - RemoteSettingsRecord("1", 123u, false, null, JSONObject()), - RemoteSettingsRecord("2", 456u, true, null, JSONObject()), - ) - val response = RemoteSettingsResponse(records, 125614567U) - `when`(mockRemoteSettingsClient.read()) - .thenReturn(RemoteSettingsResult.Success(response)) - - val result = serpTelemetryRepository.loadProvidersFromCache() - - assertEquals(response.lastModified, result.first) - assertEquals(response.records.mapNotNull { it.fields.toSearchProviderModel() }, result.second) - } +// @Mock +// private lateinit var mockRemoteSettingsClient: RemoteSettingsClient +// +// private lateinit var serpTelemetryRepository: SerpTelemetryRepository +// +// @Before +// fun setup() { +// MockitoAnnotations.openMocks(this) +// serpTelemetryRepository = SerpTelemetryRepository( +// rootStorageDirectory = mock(), +// readJson = mock(), +// collectionName = "", +// serverUrl = "https://test.server", +// bucketName = "", +// ) +// +// serpTelemetryRepository.remoteSettingsClient = mockRemoteSettingsClient +// } +// +// @Test +// fun `GIVEN non-empty response WHEN writeToCache is called THEN the result is a success`() = runBlocking { +// val records = listOf( +// RemoteSettingsRecord("1", 123u, false, null, JSONObject()), +// RemoteSettingsRecord("2", 456u, true, null, JSONObject()), +// ) +// val response = RemoteSettingsResponse(records, 125614567U) +// +// `when`(mockRemoteSettingsClient.write(response)) +// .thenReturn(RemoteSettingsResult.Success(response)) +// +// val result = serpTelemetryRepository.writeToCache(response) +// +// assertTrue(result is RemoteSettingsResult.Success) +// assertEquals(response, (result as RemoteSettingsResult.Success).response) +// } +// +// @Test +// fun `GIVEN non-empty response WHEN fetchRemoteResponse is called THEN the result is equal to the response`() = runBlocking { +// val records = listOf( +// RemoteSettingsRecord("1", 123u, false, null, JSONObject()), +// RemoteSettingsRecord("2", 456u, true, null, JSONObject()), +// ) +// val response = RemoteSettingsResponse(records, 125614567U) +// `when`(mockRemoteSettingsClient.fetch()) +// .thenReturn(RemoteSettingsResult.Success(response)) +// +// val result = serpTelemetryRepository.fetchRemoteResponse() +// +// assertEquals(response, result) +// } +// +// @Test +// fun `GIVEN non-empty response WHEN loadProvidersFromCache is called THEN the result is equal to the response`() = runBlocking { +// val records = listOf( +// RemoteSettingsRecord("1", 123u, false, null, JSONObject()), +// RemoteSettingsRecord("2", 456u, true, null, JSONObject()), +// ) +// val response = RemoteSettingsResponse(records, 125614567U) +// `when`(mockRemoteSettingsClient.read()) +// .thenReturn(RemoteSettingsResult.Success(response)) +// +// val result = serpTelemetryRepository.loadProvidersFromCache() +// +// assertEquals(response.lastModified, result.first) +// assertEquals(response.records.mapNotNull { it.fields.toSearchProviderModel() }, result.second) +// } }

===================================== mobile/android/android-components/components/support/remotesettings/src/main/java/mozilla/components/support/remotesettings/RemoteSettingsClient.kt ===================================== @@ -64,18 +64,20 @@ class RemoteSettingsClient( */ @Suppress("TooGenericExceptionCaught") suspend fun fetch(): RemoteSettingsResult = withContext(Dispatchers.IO) { - try { - val serverRecords = RemoteSettings(config).use { - it.getRecords() - } - RemoteSettingsResult.Success(serverRecords) - } catch (e: RemoteSettingsException) { - Logger.error(e.message.toString()) - RemoteSettingsResult.NetworkFailure(e) - } catch (e: NullPointerException) { - Logger.error(e.message.toString()) - RemoteSettingsResult.NetworkFailure(e) - } +// try { +// val serverRecords = RemoteSettings(config).use { +// it.getRecords() +// } +// RemoteSettingsResult.Success(serverRecords) +// } catch (e: RemoteSettingsException) { +// Logger.error(e.message.toString()) +// RemoteSettingsResult.NetworkFailure(e) +// } catch (e: NullPointerException) { +// Logger.error(e.message.toString()) +// RemoteSettingsResult.NetworkFailure(e) +// } + RemoteSettingsResult.NetworkFailure(Exception("tb-43113: no remote fetching")) + }

/** @@ -123,7 +125,8 @@ class RemoteSettingsClient( suspend fun RemoteSettingsClient.readOrFetch(): RemoteSettingsResult { val readResult = read() return if (readResult is RemoteSettingsResult.DiskFailure) { - fetch() + //fetch() + RemoteSettingsResult.NetworkFailure(Exception("tb-43113: no remote fetching")) } else { readResult }

View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/264c385b...