From 53b702e569a26d3a50f258769de99bcb76c03a41 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Wed, 20 May 2026 12:36:21 +0200 Subject: [PATCH 1/3] Add chatIdOrNull to DuckChat api and drop chatID string duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "chatID" query parameter name was duplicated across three call sites extracting the chatID from a Duck.ai URL, each implementing the lookup differently and only two of them gating on isDuckChatUrl. Lift the operation onto the public DuckChat interface as chatIdOrNull(uri) so the literal lives in one place and the safety guard is enforced for every consumer — closing a latent foot-gun in DuckChatContextualViewModel.hasChatId that previously trusted any URL carrying a chatID query param. DuckChatConstants.CHAT_ID_PARAM is removed; the raw "chatID" string now only lives in RealDuckChat's private const. URL building stays on the impl-only DuckChatInternal since every builder is impl-internal. --- .../browser/nativeinput/NativeInputManager.kt | 3 +-- .../app/fire/DuckAiTabsCleanupPlugin.kt | 10 ++------ .../nativeinput/RealNativeInputManagerTest.kt | 12 ++++----- .../app/fire/DuckAiTabsCleanupPluginTest.kt | 21 ++++++++-------- .../com/duckduckgo/duckchat/api/DuckChat.kt | 6 +++++ .../duckchat/impl/DuckChatConstants.kt | 1 - .../duckduckgo/duckchat/impl/RealDuckChat.kt | 5 ++++ .../clearing/DuckChatDataClearingPlugin.kt | 9 +------ .../contextual/DuckChatContextualViewModel.kt | 7 +++--- .../ui/viewmodel/InputScreenViewModel.kt | 8 +----- .../impl/ui/NativeInputModeWidgetViewModel.kt | 9 +------ .../DuckChatDataClearingPluginTest.kt | 25 +++++++++---------- .../DuckChatContextualViewModelTest.kt | 3 +++ .../impl/messaging/fakes/FakeDuckChat.kt | 5 ++++ .../messaging/fakes/FakeDuckChatInternal.kt | 5 ++++ .../ui/NativeInputModeWidgetViewModelTest.kt | 5 ++-- .../inputscreen/InputScreenViewModelTest.kt | 1 + 17 files changed, 67 insertions(+), 68 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/nativeinput/NativeInputManager.kt b/app/src/main/java/com/duckduckgo/app/browser/nativeinput/NativeInputManager.kt index 9f5f6170506b..d3603c2d67ae 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/nativeinput/NativeInputManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/nativeinput/NativeInputManager.kt @@ -787,8 +787,7 @@ class RealNativeInputManager @Inject constructor( internal fun extractDuckAiChatId(rawUrl: String?): String? { if (rawUrl.isNullOrBlank()) return null val uri = runCatching { rawUrl.toUri() }.getOrNull() ?: return null - if (!duckChat.isDuckChatUrl(uri)) return null - return uri.getQueryParameter("chatID")?.takeIf { it.isNotBlank() } + return duckChat.chatIdOrNull(uri) } companion object { diff --git a/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt b/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt index 8413d232e3d8..bee638b1bdc0 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt @@ -16,7 +16,6 @@ package com.duckduckgo.app.fire -import android.net.Uri import androidx.core.net.toUri import com.duckduckgo.app.tabs.model.TabRepository import com.duckduckgo.dataclearing.api.plugin.ClearableData @@ -61,19 +60,14 @@ class DuckAiTabsCleanupPlugin @Inject constructor( */ private suspend fun closeTabsMatching(chatUrls: Set) { if (chatUrls.isEmpty()) return - val targetChatIds = chatUrls.mapNotNullTo(mutableSetOf()) { it.toUri().chatIdOrNull() } + val targetChatIds = chatUrls.mapNotNullTo(mutableSetOf()) { duckChat.chatIdOrNull(it.toUri()) } if (targetChatIds.isEmpty()) return val ids = tabRepository.getTabs() - .filter { tab -> tab.url?.toUri()?.chatIdOrNull() in targetChatIds } + .filter { tab -> tab.url?.toUri()?.let(duckChat::chatIdOrNull) in targetChatIds } .map { it.tabId } if (ids.isNotEmpty()) { logcat { "Closing ${ids.size} Duck.ai tab(s) matching the cleared subset" } tabRepository.deleteTabs(ids) } } - - private fun Uri.chatIdOrNull(): String? { - if (!duckChat.isDuckChatUrl(this)) return null - return getQueryParameter("chatID")?.takeIf { it.isNotBlank() } - } } diff --git a/app/src/test/java/com/duckduckgo/app/browser/nativeinput/RealNativeInputManagerTest.kt b/app/src/test/java/com/duckduckgo/app/browser/nativeinput/RealNativeInputManagerTest.kt index 529b63561bac..86e9594bc87b 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/nativeinput/RealNativeInputManagerTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/nativeinput/RealNativeInputManagerTest.kt @@ -72,40 +72,40 @@ class RealNativeInputManagerTest { @Test fun whenUrlIsNotDuckAiThenIsExistingDuckAiChatFalse() { - whenever(duckChat.isDuckChatUrl(any())).thenReturn(false) + whenever(duckChat.chatIdOrNull(any())).thenReturn(null) assertFalse(testee.isExistingDuckAiChat("https://example.com/?chatID=abcd")) } @Test fun whenDuckAiUrlWithoutChatIdThenIsExistingDuckAiChatFalse() { - whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(duckChat.chatIdOrNull(any())).thenReturn(null) assertFalse(testee.isExistingDuckAiChat("https://duck.ai/")) } @Test fun whenDuckAiUrlWithBlankChatIdThenIsExistingDuckAiChatFalse() { - whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(duckChat.chatIdOrNull(any())).thenReturn(null) assertFalse(testee.isExistingDuckAiChat("https://duck.ai/?chatID=")) } @Test fun whenDuckAiUrlWithChatIdThenIsExistingDuckAiChatTrue() { - whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(duckChat.chatIdOrNull(any())).thenReturn("abc-123") assertTrue(testee.isExistingDuckAiChat("https://duck.ai/?chatID=abc-123")) } @Test fun whenIsDuckChatUrlChecksParsedUriThenCheckedWithSameUri() { - whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(duckChat.chatIdOrNull(any())).thenReturn("xyz") val raw = "https://duck.ai/chat?chatID=xyz" testee.isExistingDuckAiChat(raw) - verify(duckChat).isDuckChatUrl(Uri.parse(raw)) + verify(duckChat).chatIdOrNull(Uri.parse(raw)) } @Test diff --git a/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt b/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt index 408c2f67060d..836b7fcfe5a3 100644 --- a/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt +++ b/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt @@ -91,9 +91,9 @@ class DuckAiTabsCleanupPluginTest { val unrelatedDuckAiTab = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=zzz") val browserTab = TabEntity(tabId = "tab3", url = "https://example.com") whenever(mockTabRepository.getTabs()).thenReturn(listOf(matchingTab, unrelatedDuckAiTab, browserTab)) - whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=abc".toUri()))).thenReturn(true) - whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=zzz".toUri()))).thenReturn(true) - whenever(mockDuckChat.isDuckChatUrl(eq("https://example.com".toUri()))).thenReturn(false) + whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=abc".toUri()))).thenReturn("abc") + whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=zzz".toUri()))).thenReturn("zzz") + whenever(mockDuckChat.chatIdOrNull(eq("https://example.com".toUri()))).thenReturn(null) testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) @@ -106,9 +106,9 @@ class DuckAiTabsCleanupPluginTest { val t2 = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=b") val t3 = TabEntity(tabId = "tab3", url = "https://example.com") whenever(mockTabRepository.getTabs()).thenReturn(listOf(t1, t2, t3)) - whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=a".toUri()))).thenReturn(true) - whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=b".toUri()))).thenReturn(true) - whenever(mockDuckChat.isDuckChatUrl(eq("https://example.com".toUri()))).thenReturn(false) + whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=a".toUri()))).thenReturn("a") + whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=b".toUri()))).thenReturn("b") + whenever(mockDuckChat.chatIdOrNull(eq("https://example.com".toUri()))).thenReturn(null) testee.onClearData( setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=a", "https://duck.ai?chatID=b"))), @@ -121,7 +121,8 @@ class DuckAiTabsCleanupPluginTest { fun `DuckChats Selected with no matching tab does not call deleteTabs`() = runTest { val unrelated = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=other") whenever(mockTabRepository.getTabs()).thenReturn(listOf(unrelated)) - whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=other".toUri()))).thenReturn("other") + whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=abc".toUri()))).thenReturn("abc") testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) @@ -134,7 +135,7 @@ class DuckAiTabsCleanupPluginTest { val t2 = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=abc") val t3 = TabEntity(tabId = "tab3", url = "https://duck.ai?chatID=abc") whenever(mockTabRepository.getTabs()).thenReturn(listOf(t1, t2, t3)) - whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(mockDuckChat.chatIdOrNull(any())).thenReturn("abc") testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) @@ -150,7 +151,7 @@ class DuckAiTabsCleanupPluginTest { val withExtraParams = TabEntity(tabId = "tab3", url = "https://duck.ai?chatID=abc&model=gpt&session=xyz") val withFragment = TabEntity(tabId = "tab4", url = "https://duck.ai?chatID=abc#message-5") whenever(mockTabRepository.getTabs()).thenReturn(listOf(original, withPath, withExtraParams, withFragment)) - whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(mockDuckChat.chatIdOrNull(any())).thenReturn("abc") testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) @@ -161,7 +162,7 @@ class DuckAiTabsCleanupPluginTest { fun `Selected url with no chatID query param is a no-op`() = runTest { val anyTab = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=abc") whenever(mockTabRepository.getTabs()).thenReturn(listOf(anyTab)) - whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai".toUri()))).thenReturn(null) testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai")))) diff --git a/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/DuckChat.kt b/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/DuckChat.kt index cb56b047cfbc..bfd62284ece9 100644 --- a/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/DuckChat.kt +++ b/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/DuckChat.kt @@ -62,6 +62,12 @@ interface DuckChat { */ fun isDuckChatUrl(uri: Uri): Boolean + /** + * Returns the chatID query parameter from [uri] when it is a Duck.ai chat URL with a + * non-blank chatID; null otherwise (including for non-Duck.ai URLs). + */ + fun chatIdOrNull(uri: Uri): String? + /** * Returns `true` if Duck Chat was ever opened before. */ diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/DuckChatConstants.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/DuckChatConstants.kt index 39fdbf34eb58..0737f6146f66 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/DuckChatConstants.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/DuckChatConstants.kt @@ -18,7 +18,6 @@ package com.duckduckgo.duckchat.impl object DuckChatConstants { const val JS_MESSAGING_FEATURE_NAME = "aiChat" - const val CHAT_ID_PARAM = "chatID" const val DUCK_AI_FEATURE_PAGE = "duckai" object JsResponseKeys { diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt index 68a278cef43f..6fbf635430e7 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt @@ -836,6 +836,11 @@ class RealDuckChat @Inject constructor( return appendParameters(mapOf(CHAT_ID_QUERY_NAME to chatId), getDuckChatLink()) } + override fun chatIdOrNull(uri: Uri): String? { + if (!isDuckChatUrl(uri)) return null + return uri.getQueryParameter(CHAT_ID_QUERY_NAME)?.takeIf { it.isNotBlank() } + } + override suspend fun setDefaultTogglePosition(position: DefaultTogglePosition) { duckChatFeatureRepository.setDefaultTogglePosition(position.name) } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPlugin.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPlugin.kt index ab1200898114..508a95535f59 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPlugin.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPlugin.kt @@ -22,7 +22,6 @@ import com.duckduckgo.dataclearing.api.plugin.ClearableData import com.duckduckgo.dataclearing.api.plugin.DataClearingPlugin import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.duckchat.api.DuckChat -import com.duckduckgo.duckchat.impl.DuckChatConstants.CHAT_ID_PARAM import com.duckduckgo.duckchat.impl.repository.DuckChatFeatureRepository import com.duckduckgo.duckchat.impl.sync.DuckChatSyncRepository import com.duckduckgo.sync.api.engine.SyncEngine @@ -75,7 +74,7 @@ class DuckChatDataClearingPlugin @Inject constructor( if (chatUrls.isEmpty()) return var anyDeleted = false chatUrls.forEach { chatUrl -> - val chatId = extractChatId(chatUrl) ?: return@forEach + val chatId = duckChat.chatIdOrNull(chatUrl.toUri()) ?: return@forEach if (duckChatDeleter.deleteChat(chatId)) { duckChatSyncRepository.recordSingleChatDeletion(chatId) anyDeleted = true @@ -84,10 +83,4 @@ class DuckChatDataClearingPlugin @Inject constructor( // One sync trigger per user-visible delete action, not N. if (anyDeleted) syncEngine.triggerSync(SyncEngine.SyncTrigger.DATA_CHANGE) } - - private fun extractChatId(url: String): String? { - val uri = url.toUri() - if (!duckChat.isDuckChatUrl(uri)) return null - return uri.getQueryParameter(CHAT_ID_PARAM)?.takeIf { it.isNotBlank() } - } } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt index f7496cb1bde9..8645ff4c7c6a 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt @@ -24,7 +24,6 @@ import com.duckduckgo.anvil.annotations.ContributesViewModel import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.FragmentScope import com.duckduckgo.duckchat.api.DuckChat -import com.duckduckgo.duckchat.impl.DuckChatConstants.CHAT_ID_PARAM import com.duckduckgo.duckchat.impl.DuckChatInternal import com.duckduckgo.duckchat.impl.feature.DuckChatFeature import com.duckduckgo.duckchat.impl.helper.DuckChatJSHelper @@ -666,8 +665,10 @@ class DuckChatContextualViewModel @Inject constructor( private fun hasChatId(url: String?): Boolean = !extractChatId(url).isNullOrBlank() - private fun extractChatId(url: String?): String? = - url?.toUri()?.getQueryParameter(CHAT_ID_PARAM)?.takeIf { it.isNotBlank() } + private fun extractChatId(url: String?): String? { + val uri = url?.toUri() ?: return null + return duckChat.chatIdOrNull(uri) + } // Owns the coupled (fullModeUrl, _chatId) invariant: _chatId is always extractChatId(fullModeUrl). private fun setSheetUrl(url: String) { diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/viewmodel/InputScreenViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/viewmodel/InputScreenViewModel.kt index 55547899c04b..cf314d0673e1 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/viewmodel/InputScreenViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/inputscreen/ui/viewmodel/InputScreenViewModel.kt @@ -44,7 +44,6 @@ import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.common.utils.SingleLiveEvent import com.duckduckgo.common.utils.extensions.toBinaryString import com.duckduckgo.duckchat.api.DuckAiFeatureState -import com.duckduckgo.duckchat.impl.DuckChatConstants.CHAT_ID_PARAM import com.duckduckgo.duckchat.impl.DuckChatInternal import com.duckduckgo.duckchat.impl.feature.DuckAiChatHistoryFeature import com.duckduckgo.duckchat.impl.feature.DuckChatFeature @@ -863,12 +862,7 @@ class InputScreenViewModel @AssistedInject constructor( saveLastUsedTogglePosition() duckChatJSHelper.clearTabContextPromptEvent() viewModelScope.launch { - val url = duckChat.getDuckChatUrl("", false) - .toUri() - .buildUpon() - .appendQueryParameter(CHAT_ID_PARAM, chatId) - .build() - .toString() + val url = duckChat.buildChatUrl(chatId) command.value = Command.SubmitSearch(url) if (pinned) { diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/NativeInputModeWidgetViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/NativeInputModeWidgetViewModel.kt index 60b56f76b25b..a870df281112 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/NativeInputModeWidgetViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/NativeInputModeWidgetViewModel.kt @@ -16,7 +16,6 @@ package com.duckduckgo.duckchat.impl.ui -import androidx.core.net.toUri import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.duckduckgo.anvil.annotations.ContributesViewModel @@ -40,7 +39,6 @@ import com.duckduckgo.duckchat.api.nativeinput.NativeInputState import com.duckduckgo.duckchat.api.nativeinput.NativeInputStateProvider import com.duckduckgo.duckchat.api.nativeinput.NativeInputStatePublisher import com.duckduckgo.duckchat.impl.ChatState -import com.duckduckgo.duckchat.impl.DuckChatConstants.CHAT_ID_PARAM import com.duckduckgo.duckchat.impl.DuckChatInternal import com.duckduckgo.duckchat.impl.feature.DuckAiChatHistoryFeature import com.duckduckgo.duckchat.impl.feature.maxUrlSuggestions @@ -396,12 +394,7 @@ class NativeInputModeWidgetViewModel @Inject constructor( } fun buildChatSuggestionUrl(suggestion: ChatSuggestion): String = - duckChatInternal.getDuckChatUrl("", false) - .toUri() - .buildUpon() - .appendQueryParameter(CHAT_ID_PARAM, suggestion.chatId) - .build() - .toString() + duckChatInternal.buildChatUrl(suggestion.chatId) private fun getInputMode( isEnabled: Boolean, diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt index 76c7cf2c5184..5ca5501310bb 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt @@ -110,7 +110,7 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected with one chat extracts chatId and deletes`() = runTest { val chatUrl = "https://duckduckgo.com/?ia=chat&chatID=abc-123" - whenever(duckChat.isDuckChatUrl(Uri.parse(chatUrl))).thenReturn(true) + whenever(duckChat.chatIdOrNull(Uri.parse(chatUrl))).thenReturn("abc-123") whenever(duckChatDeleter.deleteChat("abc-123")).thenReturn(true) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf(chatUrl)))) @@ -123,7 +123,7 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected with one chat does nothing when url is not a duck chat url`() = runTest { val chatUrl = "https://example.com/?chatID=abc-123" - whenever(duckChat.isDuckChatUrl(Uri.parse(chatUrl))).thenReturn(false) + whenever(duckChat.chatIdOrNull(Uri.parse(chatUrl))).thenReturn(null) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf(chatUrl)))) @@ -134,7 +134,7 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected with one chat does nothing when chatId is missing`() = runTest { val chatUrl = "https://duckduckgo.com/?ia=chat" - whenever(duckChat.isDuckChatUrl(Uri.parse(chatUrl))).thenReturn(true) + whenever(duckChat.chatIdOrNull(Uri.parse(chatUrl))).thenReturn(null) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf(chatUrl)))) @@ -145,7 +145,7 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected with one chat does not record sync when deletion fails`() = runTest { val chatUrl = "https://duckduckgo.com/?ia=chat&chatID=abc-123" - whenever(duckChat.isDuckChatUrl(Uri.parse(chatUrl))).thenReturn(true) + whenever(duckChat.chatIdOrNull(Uri.parse(chatUrl))).thenReturn("abc-123") whenever(duckChatDeleter.deleteChat("abc-123")).thenReturn(false) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf(chatUrl)))) @@ -157,8 +157,8 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected deletes each chat in the set and records per-chat sync deletions`() = runTest { - whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn(true) - whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=beta")))).thenReturn(true) + whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn("alpha") + whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://duck.ai?chatID=beta")))).thenReturn("beta") whenever(duckChatDeleter.deleteChat("alpha")).thenReturn(true) whenever(duckChatDeleter.deleteChat("beta")).thenReturn(true) @@ -176,7 +176,7 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected does not call deleteAllChats`() = runTest { - whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(duckChat.chatIdOrNull(any())).thenReturn("x") whenever(duckChatDeleter.deleteChat(any())).thenReturn(true) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=x")))) @@ -203,9 +203,8 @@ class DuckChatDataClearingPluginTest { @Test fun `DuckChats Selected deletes each chat in the set and records per-chat sync deletions`() = runTest { - whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) - whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn(true) - whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=beta")))).thenReturn(true) + whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn("alpha") + whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://duck.ai?chatID=beta")))).thenReturn("beta") whenever(duckChatDeleter.deleteChat("alpha")).thenReturn(true) whenever(duckChatDeleter.deleteChat("beta")).thenReturn(true) @@ -223,7 +222,7 @@ class DuckChatDataClearingPluginTest { @Test fun `DuckChats Selected does not call deleteAllChats`() = runTest { - whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) + whenever(duckChat.chatIdOrNull(any())).thenReturn("x") whenever(duckChatDeleter.deleteChat(any())).thenReturn(true) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=x")))) @@ -242,8 +241,8 @@ class DuckChatDataClearingPluginTest { @Test fun `DuckChats Selected skips urls that are not duck chat urls`() = runTest { - whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn(true) - whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://example.com")))).thenReturn(false) + whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn("alpha") + whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://example.com")))).thenReturn(null) whenever(duckChatDeleter.deleteChat("alpha")).thenReturn(true) plugin.onClearData( diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModelTest.kt index 20be0ece672b..6db15fb43f66 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModelTest.kt @@ -1578,6 +1578,9 @@ class DuckChatContextualViewModelTest { ): String = nextUrl override fun isDuckChatUrl(uri: android.net.Uri): Boolean = false + + override fun chatIdOrNull(uri: android.net.Uri): String? = + uri.getQueryParameter("chatID")?.takeIf { it.isNotBlank() } override suspend fun wasOpenedBefore(): Boolean = false override fun showNewAddressBarOptionChoiceScreen( context: android.content.Context, diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChat.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChat.kt index c57646d79760..9cef053f0686 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChat.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChat.kt @@ -66,6 +66,11 @@ class FakeDuckChat( return uri.toString().contains("duckchat") } + override fun chatIdOrNull(uri: Uri): String? { + if (!isDuckChatUrl(uri)) return null + return uri.getQueryParameter("chatID")?.takeIf { it.isNotBlank() } + } + override suspend fun wasOpenedBefore(): Boolean { return wasOpenedBeforeValue } diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt index 06431cd3fd2d..0b68d99dd22e 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt @@ -201,6 +201,11 @@ class FakeDuckChatInternal( override fun buildChatUrl(chatId: String): String = "https://duck.ai?chatID=$chatId" + override fun chatIdOrNull(uri: Uri): String? { + if (!isDuckChatUrl(uri)) return null + return uri.getQueryParameter("chatID")?.takeIf { it.isNotBlank() } + } + private val _defaultTogglePosition = MutableStateFlow(null) override suspend fun setDefaultTogglePosition(position: DefaultTogglePosition) { diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/NativeInputModeWidgetViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/NativeInputModeWidgetViewModelTest.kt index cfa039b974a4..1efb29b969d2 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/NativeInputModeWidgetViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/NativeInputModeWidgetViewModelTest.kt @@ -409,8 +409,9 @@ class NativeInputModeWidgetViewModelTest { } @Test - fun whenBuildChatSuggestionUrlThenAppendsChatIdParam() { - whenever(duckChatInternal.getDuckChatUrl("", false)).thenReturn("https://duckduckgo.com/?q=DuckDuckGo+AI+Chat&ia=chat&duckai=5") + fun whenBuildChatSuggestionUrlThenDelegatesToDuckChat() { + whenever(duckChatInternal.buildChatUrl("abc-123")) + .thenReturn("https://duckduckgo.com/?ia=chat&duckai=5&chatID=abc-123") val suggestion = ChatSuggestion( chatId = "abc-123", title = "Title", diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/inputscreen/InputScreenViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/inputscreen/InputScreenViewModelTest.kt index 306013b212e4..984359d06574 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/inputscreen/InputScreenViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/inputscreen/InputScreenViewModelTest.kt @@ -122,6 +122,7 @@ class InputScreenViewModelTest { ) whenever(duckChat.wasOpenedBefore()).thenReturn(false) whenever(duckChat.getDuckChatUrl(any(), any(), any())).thenReturn(duckChatURL) + whenever(duckChat.buildChatUrl(any())).thenAnswer { "$duckChatURL&chatID=${it.arguments[0]}" } whenever(duckChat.observeChatSuggestionsUserSettingEnabled()).thenReturn(flowOf(true)) whenever(chatSuggestionsReader.fetchSuggestions(any())).thenReturn(emptyList()) whenever(inputScreenConfigResolver.useTopBar()).thenReturn(true) From 06fda900c4e4869103626b8aa04471e804fc0163 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Thu, 21 May 2026 18:27:01 +0200 Subject: [PATCH 2/3] Add Uri.toChatIdOrNull(DuckChat) extension and drop chatID string duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "chatID" query parameter name was extracted in three different shapes across modules — a custom Uri extension in :app, a private helper in :duckchat-impl, and an inline lookup in DuckChatContextualViewModel — only two of which gated on isDuckChatUrl. Lift the operation onto a top-level Uri.toChatIdOrNull(DuckChat) extension in :duckchat-api so the literal lives in one place and the safety guard is enforced for every consumer, closing a latent foot-gun in DuckChatContextualViewModel.hasChatId that previously trusted any URL carrying a chatID query param. URL building stays on the impl-only DuckChatInternal since every builder is impl-internal and never crosses the module boundary. --- .../browser/nativeinput/NativeInputManager.kt | 3 +- .../app/fire/DuckAiTabsCleanupPlugin.kt | 5 ++-- .../nativeinput/RealNativeInputManagerTest.kt | 12 ++++---- .../app/fire/DuckAiTabsCleanupPluginTest.kt | 21 +++++++------- .../com/duckduckgo/duckchat/api/DuckChat.kt | 6 ---- .../duckduckgo/duckchat/api/UriExtension.kt | 28 +++++++++++++++++++ .../duckduckgo/duckchat/impl/RealDuckChat.kt | 5 ---- .../clearing/DuckChatDataClearingPlugin.kt | 3 +- .../contextual/DuckChatContextualViewModel.kt | 3 +- .../DuckChatDataClearingPluginTest.kt | 24 ++++++++-------- .../DuckChatContextualViewModelTest.kt | 6 ++-- .../impl/messaging/fakes/FakeDuckChat.kt | 5 ---- .../messaging/fakes/FakeDuckChatInternal.kt | 5 ---- 13 files changed, 67 insertions(+), 59 deletions(-) create mode 100644 duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/UriExtension.kt diff --git a/app/src/main/java/com/duckduckgo/app/browser/nativeinput/NativeInputManager.kt b/app/src/main/java/com/duckduckgo/app/browser/nativeinput/NativeInputManager.kt index d3603c2d67ae..b4c8e4014f3b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/nativeinput/NativeInputManager.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/nativeinput/NativeInputManager.kt @@ -41,6 +41,7 @@ import com.duckduckgo.common.ui.view.toPx import com.duckduckgo.di.scopes.FragmentScope import com.duckduckgo.duckchat.api.DuckAiFeatureState import com.duckduckgo.duckchat.api.DuckChat +import com.duckduckgo.duckchat.api.toChatIdOrNull import com.duckduckgo.duckchat.impl.ui.nativeinput.views.NativeInputWidget import com.duckduckgo.navigation.api.GlobalActivityStarter import com.duckduckgo.subscriptions.api.SubscriptionScreens.SubscriptionPurchase @@ -787,7 +788,7 @@ class RealNativeInputManager @Inject constructor( internal fun extractDuckAiChatId(rawUrl: String?): String? { if (rawUrl.isNullOrBlank()) return null val uri = runCatching { rawUrl.toUri() }.getOrNull() ?: return null - return duckChat.chatIdOrNull(uri) + return uri.toChatIdOrNull(duckChat) } companion object { diff --git a/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt b/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt index bee638b1bdc0..ee6cc3d9dc7d 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPlugin.kt @@ -22,6 +22,7 @@ import com.duckduckgo.dataclearing.api.plugin.ClearableData import com.duckduckgo.dataclearing.api.plugin.DataClearingPlugin import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.duckchat.api.DuckChat +import com.duckduckgo.duckchat.api.toChatIdOrNull import com.squareup.anvil.annotations.ContributesMultibinding import logcat.logcat import javax.inject.Inject @@ -60,10 +61,10 @@ class DuckAiTabsCleanupPlugin @Inject constructor( */ private suspend fun closeTabsMatching(chatUrls: Set) { if (chatUrls.isEmpty()) return - val targetChatIds = chatUrls.mapNotNullTo(mutableSetOf()) { duckChat.chatIdOrNull(it.toUri()) } + val targetChatIds = chatUrls.mapNotNullTo(mutableSetOf()) { it.toUri().toChatIdOrNull(duckChat) } if (targetChatIds.isEmpty()) return val ids = tabRepository.getTabs() - .filter { tab -> tab.url?.toUri()?.let(duckChat::chatIdOrNull) in targetChatIds } + .filter { tab -> tab.url?.toUri()?.toChatIdOrNull(duckChat) in targetChatIds } .map { it.tabId } if (ids.isNotEmpty()) { logcat { "Closing ${ids.size} Duck.ai tab(s) matching the cleared subset" } diff --git a/app/src/test/java/com/duckduckgo/app/browser/nativeinput/RealNativeInputManagerTest.kt b/app/src/test/java/com/duckduckgo/app/browser/nativeinput/RealNativeInputManagerTest.kt index 86e9594bc87b..529b63561bac 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/nativeinput/RealNativeInputManagerTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/nativeinput/RealNativeInputManagerTest.kt @@ -72,40 +72,40 @@ class RealNativeInputManagerTest { @Test fun whenUrlIsNotDuckAiThenIsExistingDuckAiChatFalse() { - whenever(duckChat.chatIdOrNull(any())).thenReturn(null) + whenever(duckChat.isDuckChatUrl(any())).thenReturn(false) assertFalse(testee.isExistingDuckAiChat("https://example.com/?chatID=abcd")) } @Test fun whenDuckAiUrlWithoutChatIdThenIsExistingDuckAiChatFalse() { - whenever(duckChat.chatIdOrNull(any())).thenReturn(null) + whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) assertFalse(testee.isExistingDuckAiChat("https://duck.ai/")) } @Test fun whenDuckAiUrlWithBlankChatIdThenIsExistingDuckAiChatFalse() { - whenever(duckChat.chatIdOrNull(any())).thenReturn(null) + whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) assertFalse(testee.isExistingDuckAiChat("https://duck.ai/?chatID=")) } @Test fun whenDuckAiUrlWithChatIdThenIsExistingDuckAiChatTrue() { - whenever(duckChat.chatIdOrNull(any())).thenReturn("abc-123") + whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) assertTrue(testee.isExistingDuckAiChat("https://duck.ai/?chatID=abc-123")) } @Test fun whenIsDuckChatUrlChecksParsedUriThenCheckedWithSameUri() { - whenever(duckChat.chatIdOrNull(any())).thenReturn("xyz") + whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) val raw = "https://duck.ai/chat?chatID=xyz" testee.isExistingDuckAiChat(raw) - verify(duckChat).chatIdOrNull(Uri.parse(raw)) + verify(duckChat).isDuckChatUrl(Uri.parse(raw)) } @Test diff --git a/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt b/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt index 836b7fcfe5a3..408c2f67060d 100644 --- a/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt +++ b/app/src/test/java/com/duckduckgo/app/fire/DuckAiTabsCleanupPluginTest.kt @@ -91,9 +91,9 @@ class DuckAiTabsCleanupPluginTest { val unrelatedDuckAiTab = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=zzz") val browserTab = TabEntity(tabId = "tab3", url = "https://example.com") whenever(mockTabRepository.getTabs()).thenReturn(listOf(matchingTab, unrelatedDuckAiTab, browserTab)) - whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=abc".toUri()))).thenReturn("abc") - whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=zzz".toUri()))).thenReturn("zzz") - whenever(mockDuckChat.chatIdOrNull(eq("https://example.com".toUri()))).thenReturn(null) + whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=abc".toUri()))).thenReturn(true) + whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=zzz".toUri()))).thenReturn(true) + whenever(mockDuckChat.isDuckChatUrl(eq("https://example.com".toUri()))).thenReturn(false) testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) @@ -106,9 +106,9 @@ class DuckAiTabsCleanupPluginTest { val t2 = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=b") val t3 = TabEntity(tabId = "tab3", url = "https://example.com") whenever(mockTabRepository.getTabs()).thenReturn(listOf(t1, t2, t3)) - whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=a".toUri()))).thenReturn("a") - whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=b".toUri()))).thenReturn("b") - whenever(mockDuckChat.chatIdOrNull(eq("https://example.com".toUri()))).thenReturn(null) + whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=a".toUri()))).thenReturn(true) + whenever(mockDuckChat.isDuckChatUrl(eq("https://duck.ai?chatID=b".toUri()))).thenReturn(true) + whenever(mockDuckChat.isDuckChatUrl(eq("https://example.com".toUri()))).thenReturn(false) testee.onClearData( setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=a", "https://duck.ai?chatID=b"))), @@ -121,8 +121,7 @@ class DuckAiTabsCleanupPluginTest { fun `DuckChats Selected with no matching tab does not call deleteTabs`() = runTest { val unrelated = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=other") whenever(mockTabRepository.getTabs()).thenReturn(listOf(unrelated)) - whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=other".toUri()))).thenReturn("other") - whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai?chatID=abc".toUri()))).thenReturn("abc") + whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) @@ -135,7 +134,7 @@ class DuckAiTabsCleanupPluginTest { val t2 = TabEntity(tabId = "tab2", url = "https://duck.ai?chatID=abc") val t3 = TabEntity(tabId = "tab3", url = "https://duck.ai?chatID=abc") whenever(mockTabRepository.getTabs()).thenReturn(listOf(t1, t2, t3)) - whenever(mockDuckChat.chatIdOrNull(any())).thenReturn("abc") + whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) @@ -151,7 +150,7 @@ class DuckAiTabsCleanupPluginTest { val withExtraParams = TabEntity(tabId = "tab3", url = "https://duck.ai?chatID=abc&model=gpt&session=xyz") val withFragment = TabEntity(tabId = "tab4", url = "https://duck.ai?chatID=abc#message-5") whenever(mockTabRepository.getTabs()).thenReturn(listOf(original, withPath, withExtraParams, withFragment)) - whenever(mockDuckChat.chatIdOrNull(any())).thenReturn("abc") + whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=abc")))) @@ -162,7 +161,7 @@ class DuckAiTabsCleanupPluginTest { fun `Selected url with no chatID query param is a no-op`() = runTest { val anyTab = TabEntity(tabId = "tab1", url = "https://duck.ai?chatID=abc") whenever(mockTabRepository.getTabs()).thenReturn(listOf(anyTab)) - whenever(mockDuckChat.chatIdOrNull(eq("https://duck.ai".toUri()))).thenReturn(null) + whenever(mockDuckChat.isDuckChatUrl(any())).thenReturn(true) testee.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai")))) diff --git a/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/DuckChat.kt b/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/DuckChat.kt index bfd62284ece9..cb56b047cfbc 100644 --- a/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/DuckChat.kt +++ b/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/DuckChat.kt @@ -62,12 +62,6 @@ interface DuckChat { */ fun isDuckChatUrl(uri: Uri): Boolean - /** - * Returns the chatID query parameter from [uri] when it is a Duck.ai chat URL with a - * non-blank chatID; null otherwise (including for non-Duck.ai URLs). - */ - fun chatIdOrNull(uri: Uri): String? - /** * Returns `true` if Duck Chat was ever opened before. */ diff --git a/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/UriExtension.kt b/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/UriExtension.kt new file mode 100644 index 000000000000..da10c344550d --- /dev/null +++ b/duckchat/duckchat-api/src/main/java/com/duckduckgo/duckchat/api/UriExtension.kt @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2026 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.duckchat.api + +import android.net.Uri + +/** + * Returns the chatID from this [Uri] when it is a Duck.ai chat URL with a non-blank chatID; + * null otherwise (including for non-Duck.ai URLs). + */ +fun Uri.toChatIdOrNull(duckChat: DuckChat): String? { + if (!duckChat.isDuckChatUrl(this)) return null + return getQueryParameter("chatID")?.takeIf { it.isNotBlank() } +} diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt index 6fbf635430e7..68a278cef43f 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt @@ -836,11 +836,6 @@ class RealDuckChat @Inject constructor( return appendParameters(mapOf(CHAT_ID_QUERY_NAME to chatId), getDuckChatLink()) } - override fun chatIdOrNull(uri: Uri): String? { - if (!isDuckChatUrl(uri)) return null - return uri.getQueryParameter(CHAT_ID_QUERY_NAME)?.takeIf { it.isNotBlank() } - } - override suspend fun setDefaultTogglePosition(position: DefaultTogglePosition) { duckChatFeatureRepository.setDefaultTogglePosition(position.name) } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPlugin.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPlugin.kt index 508a95535f59..32cf5533512c 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPlugin.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPlugin.kt @@ -22,6 +22,7 @@ import com.duckduckgo.dataclearing.api.plugin.ClearableData import com.duckduckgo.dataclearing.api.plugin.DataClearingPlugin import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.duckchat.api.DuckChat +import com.duckduckgo.duckchat.api.toChatIdOrNull import com.duckduckgo.duckchat.impl.repository.DuckChatFeatureRepository import com.duckduckgo.duckchat.impl.sync.DuckChatSyncRepository import com.duckduckgo.sync.api.engine.SyncEngine @@ -74,7 +75,7 @@ class DuckChatDataClearingPlugin @Inject constructor( if (chatUrls.isEmpty()) return var anyDeleted = false chatUrls.forEach { chatUrl -> - val chatId = duckChat.chatIdOrNull(chatUrl.toUri()) ?: return@forEach + val chatId = chatUrl.toUri().toChatIdOrNull(duckChat) ?: return@forEach if (duckChatDeleter.deleteChat(chatId)) { duckChatSyncRepository.recordSingleChatDeletion(chatId) anyDeleted = true diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt index 8645ff4c7c6a..bc8a2b86be17 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt @@ -24,6 +24,7 @@ import com.duckduckgo.anvil.annotations.ContributesViewModel import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.di.scopes.FragmentScope import com.duckduckgo.duckchat.api.DuckChat +import com.duckduckgo.duckchat.api.toChatIdOrNull import com.duckduckgo.duckchat.impl.DuckChatInternal import com.duckduckgo.duckchat.impl.feature.DuckChatFeature import com.duckduckgo.duckchat.impl.helper.DuckChatJSHelper @@ -667,7 +668,7 @@ class DuckChatContextualViewModel @Inject constructor( private fun extractChatId(url: String?): String? { val uri = url?.toUri() ?: return null - return duckChat.chatIdOrNull(uri) + return uri.toChatIdOrNull(duckChat) } // Owns the coupled (fullModeUrl, _chatId) invariant: _chatId is always extractChatId(fullModeUrl). diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt index 5ca5501310bb..e24683c07f55 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/clearing/DuckChatDataClearingPluginTest.kt @@ -110,7 +110,7 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected with one chat extracts chatId and deletes`() = runTest { val chatUrl = "https://duckduckgo.com/?ia=chat&chatID=abc-123" - whenever(duckChat.chatIdOrNull(Uri.parse(chatUrl))).thenReturn("abc-123") + whenever(duckChat.isDuckChatUrl(Uri.parse(chatUrl))).thenReturn(true) whenever(duckChatDeleter.deleteChat("abc-123")).thenReturn(true) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf(chatUrl)))) @@ -123,7 +123,7 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected with one chat does nothing when url is not a duck chat url`() = runTest { val chatUrl = "https://example.com/?chatID=abc-123" - whenever(duckChat.chatIdOrNull(Uri.parse(chatUrl))).thenReturn(null) + whenever(duckChat.isDuckChatUrl(Uri.parse(chatUrl))).thenReturn(false) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf(chatUrl)))) @@ -134,7 +134,7 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected with one chat does nothing when chatId is missing`() = runTest { val chatUrl = "https://duckduckgo.com/?ia=chat" - whenever(duckChat.chatIdOrNull(Uri.parse(chatUrl))).thenReturn(null) + whenever(duckChat.isDuckChatUrl(Uri.parse(chatUrl))).thenReturn(true) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf(chatUrl)))) @@ -145,7 +145,7 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected with one chat does not record sync when deletion fails`() = runTest { val chatUrl = "https://duckduckgo.com/?ia=chat&chatID=abc-123" - whenever(duckChat.chatIdOrNull(Uri.parse(chatUrl))).thenReturn("abc-123") + whenever(duckChat.isDuckChatUrl(Uri.parse(chatUrl))).thenReturn(true) whenever(duckChatDeleter.deleteChat("abc-123")).thenReturn(false) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf(chatUrl)))) @@ -157,8 +157,8 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected deletes each chat in the set and records per-chat sync deletions`() = runTest { - whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn("alpha") - whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://duck.ai?chatID=beta")))).thenReturn("beta") + whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn(true) + whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=beta")))).thenReturn(true) whenever(duckChatDeleter.deleteChat("alpha")).thenReturn(true) whenever(duckChatDeleter.deleteChat("beta")).thenReturn(true) @@ -176,7 +176,7 @@ class DuckChatDataClearingPluginTest { @Test fun `Selected does not call deleteAllChats`() = runTest { - whenever(duckChat.chatIdOrNull(any())).thenReturn("x") + whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) whenever(duckChatDeleter.deleteChat(any())).thenReturn(true) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=x")))) @@ -203,8 +203,8 @@ class DuckChatDataClearingPluginTest { @Test fun `DuckChats Selected deletes each chat in the set and records per-chat sync deletions`() = runTest { - whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn("alpha") - whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://duck.ai?chatID=beta")))).thenReturn("beta") + whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn(true) + whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=beta")))).thenReturn(true) whenever(duckChatDeleter.deleteChat("alpha")).thenReturn(true) whenever(duckChatDeleter.deleteChat("beta")).thenReturn(true) @@ -222,7 +222,7 @@ class DuckChatDataClearingPluginTest { @Test fun `DuckChats Selected does not call deleteAllChats`() = runTest { - whenever(duckChat.chatIdOrNull(any())).thenReturn("x") + whenever(duckChat.isDuckChatUrl(any())).thenReturn(true) whenever(duckChatDeleter.deleteChat(any())).thenReturn(true) plugin.onClearData(setOf(ClearableData.DuckChats.Selected(setOf("https://duck.ai?chatID=x")))) @@ -241,8 +241,8 @@ class DuckChatDataClearingPluginTest { @Test fun `DuckChats Selected skips urls that are not duck chat urls`() = runTest { - whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn("alpha") - whenever(duckChat.chatIdOrNull(eq(Uri.parse("https://example.com")))).thenReturn(null) + whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://duck.ai?chatID=alpha")))).thenReturn(true) + whenever(duckChat.isDuckChatUrl(eq(Uri.parse("https://example.com")))).thenReturn(false) whenever(duckChatDeleter.deleteChat("alpha")).thenReturn(true) plugin.onClearData( diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModelTest.kt index 6db15fb43f66..80e282c73808 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModelTest.kt @@ -1577,10 +1577,8 @@ class DuckChatContextualViewModelTest { sidebar: Boolean, ): String = nextUrl - override fun isDuckChatUrl(uri: android.net.Uri): Boolean = false - - override fun chatIdOrNull(uri: android.net.Uri): String? = - uri.getQueryParameter("chatID")?.takeIf { it.isNotBlank() } + override fun isDuckChatUrl(uri: android.net.Uri): Boolean = + uri.host == "duck.ai" || uri.host == "duckduckgo.com" override suspend fun wasOpenedBefore(): Boolean = false override fun showNewAddressBarOptionChoiceScreen( context: android.content.Context, diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChat.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChat.kt index 9cef053f0686..c57646d79760 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChat.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChat.kt @@ -66,11 +66,6 @@ class FakeDuckChat( return uri.toString().contains("duckchat") } - override fun chatIdOrNull(uri: Uri): String? { - if (!isDuckChatUrl(uri)) return null - return uri.getQueryParameter("chatID")?.takeIf { it.isNotBlank() } - } - override suspend fun wasOpenedBefore(): Boolean { return wasOpenedBeforeValue } diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt index 0b68d99dd22e..06431cd3fd2d 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/fakes/FakeDuckChatInternal.kt @@ -201,11 +201,6 @@ class FakeDuckChatInternal( override fun buildChatUrl(chatId: String): String = "https://duck.ai?chatID=$chatId" - override fun chatIdOrNull(uri: Uri): String? { - if (!isDuckChatUrl(uri)) return null - return uri.getQueryParameter("chatID")?.takeIf { it.isNotBlank() } - } - private val _defaultTogglePosition = MutableStateFlow(null) override suspend fun setDefaultTogglePosition(position: DefaultTogglePosition) { From 825f6047aa39cf60b00ad5c601e53e1324818110 Mon Sep 17 00:00:00 2001 From: Gerard Paligot Date: Fri, 22 May 2026 10:02:30 +0200 Subject: [PATCH 3/3] Include native-input query param in buildChatUrl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildChatUrl was the only Duck.ai URL builder that did not call nativeInputParameters(), so callers that previously composed their URL via getDuckChatUrl("", false) + manual appendQueryParameter("chatID", …) silently lost the native-input=true param after migrating onto buildChatUrl(chatId). Pull nativeInputParameters() into buildChatUrl so the "single source of truth for the Duck.ai chat URL shape" actually produces the same URL shape as every other builder. Adds two RealDuckChatTest cases that pin the contract: with the native input setting on, buildChatUrl emits ?chatID=…&native-input=true&duckai=5; with it off, native-input is absent. --- .../duckduckgo/duckchat/impl/RealDuckChat.kt | 2 +- .../duckchat/impl/RealDuckChatTest.kt | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt index 68a278cef43f..9aef6b8398af 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/RealDuckChat.kt @@ -833,7 +833,7 @@ class RealDuckChat @Inject constructor( } override fun buildChatUrl(chatId: String): String { - return appendParameters(mapOf(CHAT_ID_QUERY_NAME to chatId), getDuckChatLink()) + return appendParameters(mapOf(CHAT_ID_QUERY_NAME to chatId) + nativeInputParameters(), getDuckChatLink()) } override suspend fun setDefaultTogglePosition(position: DefaultTogglePosition) { diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/RealDuckChatTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/RealDuckChatTest.kt index 3938b5b91fbf..0a994ea930ce 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/RealDuckChatTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/RealDuckChatTest.kt @@ -1465,6 +1465,28 @@ class RealDuckChatTest { assertTrue(url == "https://duck.ai/chat?native-input=true&duckai=5") } + @Test + fun `when native input disabled and build chat url then url contains chatID without native input param`() = runTest { + whenever(mockDuckChatFeatureRepository.isNativeInputFieldUserSettingEnabled()).thenReturn(false) + testee.onPrivacyConfigDownloaded() + coroutineRule.testScope.advanceUntilIdle() + + val url = testee.buildChatUrl(chatId = "abc-123") + + assertTrue(url == "https://duck.ai/chat?chatID=abc-123&duckai=5") + } + + @Test + fun `when native input enabled and build chat url then url contains both chatID and native input param`() = runTest { + whenever(mockDuckChatFeatureRepository.isNativeInputFieldUserSettingEnabled()).thenReturn(true) + testee.onPrivacyConfigDownloaded() + coroutineRule.testScope.advanceUntilIdle() + + val url = testee.buildChatUrl(chatId = "abc-123") + + assertTrue(url == "https://duck.ai/chat?chatID=abc-123&native-input=true&duckai=5") + } + @Test fun `when url can be handled by webview return true`() { assertTrue(testee.canHandleOnAiWebView("https://duck.ai/somepath"))