-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Duck.ai - support for standalone migration #7046
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,12 @@ interface DuckDuckGoUrlDetector { | |
| */ | ||
| fun isDuckDuckGoUrl(url: String): Boolean | ||
|
|
||
| /** | ||
| * This method takes a [url] and returns `true` or `false`. | ||
| * @return `true` if the given [url] belongs to the duck.ai domain (apex or subdomain) and `false` otherwise. | ||
| */ | ||
| fun isDuckAiUrl(url: String): Boolean | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn’t be in the |
||
|
|
||
| /** | ||
| * This method takes a [uri] and returns `true` or `false`. | ||
| * @return `true` if the given [uri] is a DuckDuckGo query and `false` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ class AppUrl { | |
|
|
||
| object Url { | ||
| const val HOST = "duckduckgo.com" | ||
| const val HOST_DUCKAI = "duck.ai" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
| const val API = "https://$HOST" | ||
| const val HOME = "https://$HOST" | ||
| const val COOKIES = "https://$HOST" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -181,6 +181,11 @@ interface DuckChatInternal : DuckChat { | |
| */ | ||
| fun isImageUploadEnabled(): Boolean | ||
|
|
||
| /** | ||
| * Returns whether standalone migration is supported. | ||
| */ | ||
| fun isStandaloneMigrationEnabled(): Boolean | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs tests |
||
|
|
||
| /** | ||
| * Returns the time a Duck Chat session should be kept alive | ||
| */ | ||
|
|
@@ -315,6 +320,7 @@ class RealDuckChat @Inject constructor( | |
| private var isAddressBarEntryPointEnabled: Boolean = false | ||
| private var isVoiceSearchEntryPointEnabled: Boolean = false | ||
| private var isImageUploadEnabled: Boolean = false | ||
| private var isStandaloneMigrationEnabled: Boolean = false | ||
shakyShane marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| private var keepSessionAliveInMinutes: Int = DEFAULT_SESSION_ALIVE | ||
| private var clearChatHistory: Boolean = true | ||
| private var inputScreenMainButtonsEnabled = false | ||
|
|
@@ -462,6 +468,8 @@ class RealDuckChat @Inject constructor( | |
|
|
||
| override fun isImageUploadEnabled(): Boolean = isImageUploadEnabled | ||
|
|
||
| override fun isStandaloneMigrationEnabled(): Boolean = isStandaloneMigrationEnabled | ||
|
|
||
| override fun keepSessionIntervalInMinutes() = keepSessionAliveInMinutes | ||
|
|
||
| override fun openDuckChat() { | ||
|
|
@@ -694,6 +702,7 @@ class RealDuckChat @Inject constructor( | |
| isAddressBarEntryPointEnabled = settingsJson?.addressBarEntryPoint ?: false | ||
| isVoiceSearchEntryPointEnabled = duckChatFeature.duckAiVoiceSearch().isEnabled() | ||
| isImageUploadEnabled = imageUploadFeature.self().isEnabled() | ||
| isStandaloneMigrationEnabled = duckChatFeature.standaloneMigration().isEnabled() | ||
|
|
||
| keepSession.value = duckChatFeature.keepSession().isEnabled() | ||
| keepSessionAliveInMinutes = settingsJson?.sessionTimeoutMinutes ?: DEFAULT_SESSION_ALIVE | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
|
|
||
| package com.duckduckgo.duckchat.impl.helper | ||
|
|
||
| import com.duckduckgo.common.utils.DispatcherProvider | ||
| import com.duckduckgo.di.scopes.AppScope | ||
| import com.duckduckgo.duckchat.impl.ChatState | ||
| import com.duckduckgo.duckchat.impl.ChatState.HIDE | ||
|
|
@@ -28,6 +29,7 @@ import com.duckduckgo.duckchat.impl.store.DuckChatDataStore | |
| import com.duckduckgo.js.messaging.api.JsCallbackData | ||
| import com.squareup.anvil.annotations.ContributesBinding | ||
| import kotlinx.coroutines.runBlocking | ||
| import kotlinx.coroutines.withContext | ||
| import org.json.JSONObject | ||
| import java.util.regex.Pattern | ||
| import javax.inject.Inject | ||
|
|
@@ -47,7 +49,9 @@ class RealDuckChatJSHelper @Inject constructor( | |
| private val duckChatPixels: DuckChatPixels, | ||
| private val dataStore: DuckChatDataStore, | ||
| private val duckAiMetricCollector: DuckAiMetricCollector, | ||
| private val dispatchers: DispatcherProvider, | ||
| ) : DuckChatJSHelper { | ||
| private val migrationItems = mutableListOf<String>() | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @joshliebe is this ok here? the data is not designed to stick around, so this seemed ok - but please let me know otherwise!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shakyShane what is being stored here? is it a blob?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, just a string
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens in the unhappy case? If the app is killed while this is processing will the user lose all their data? |
||
| override suspend fun processJsCallbackMessage( | ||
| featureName: String, | ||
| method: String, | ||
|
|
@@ -117,6 +121,22 @@ class RealDuckChatJSHelper @Inject constructor( | |
| null | ||
| } | ||
|
|
||
| METHOD_STORE_MIGRATION_DATA -> id?.let { | ||
| getStoreMigrationDataResponse(featureName, method, it, data) | ||
| } | ||
|
|
||
| METHOD_GET_MIGRATION_INFO -> id?.let { | ||
| getMigrationInfoResponse(featureName, method, it) | ||
| } | ||
|
|
||
| METHOD_GET_MIGRATION_DATA_BY_INDEX -> id?.let { | ||
| getMigrationDataByIndexResponse(featureName, method, it, data) | ||
| } | ||
|
|
||
| METHOD_CLEAR_MIGRATION_DATA -> id?.let { | ||
| getClearMigrationDataResponse(featureName, method, it) | ||
| } | ||
|
|
||
| else -> null | ||
| } | ||
|
|
||
|
|
@@ -148,6 +168,7 @@ class RealDuckChatJSHelper @Inject constructor( | |
| put(SUPPORTS_NATIVE_CHAT_INPUT, false) | ||
| put(SUPPORTS_CHAT_ID_RESTORATION, duckChat.isDuckChatFullScreenModeEnabled()) | ||
| put(SUPPORTS_IMAGE_UPLOAD, duckChat.isImageUploadEnabled()) | ||
| put(SUPPORTS_STANDALONE_MIGRATION, duckChat.isStandaloneMigrationEnabled()) | ||
| } | ||
| return JsCallbackData(jsonPayload, featureName, method, id) | ||
| } | ||
|
|
@@ -191,6 +212,90 @@ class RealDuckChatJSHelper @Inject constructor( | |
| } | ||
| } | ||
|
|
||
| /** | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These changes are susceptible to race conditions
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's at most 1 caller in the entire applications lifespan per message - that's by design. There's no situation in which 2 of these messages could be processed concurrently. and if there was, it's easy to put behind a mutex? This is not like other FE features where things can be called at arbitrary times, it's forced to be in sequence |
||
| * Accept incoming JSON payload { "serializedMigrationFile": "..." } | ||
| * Store the string value in an ordered list for later retrieval | ||
| */ | ||
| private suspend fun getStoreMigrationDataResponse( | ||
| featureName: String, | ||
| method: String, | ||
| id: String, | ||
| data: JSONObject?, | ||
| ): JsCallbackData { | ||
| return withContext(dispatchers.io()) { | ||
| val item = data?.optString(SERIALIZED_MIGRATION_FILE) | ||
| val jsonPayload = JSONObject() | ||
| if (item != null && item != JSONObject.NULL) { | ||
| migrationItems.add(item) | ||
| jsonPayload.put(OK, true) | ||
| } else { | ||
| jsonPayload.put(OK, false) | ||
| jsonPayload.put(REASON, "Missing or invalid serializedMigrationFile") | ||
| } | ||
| JsCallbackData(jsonPayload, featureName, method, id) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Return the count of strings previously stored. | ||
| * It's ok to return 0 if no items have been stored | ||
| */ | ||
| private suspend fun getMigrationInfoResponse( | ||
| featureName: String, | ||
| method: String, | ||
| id: String, | ||
| ): JsCallbackData { | ||
| return withContext(dispatchers.io()) { | ||
| val count = migrationItems.size | ||
| val jsonPayload = JSONObject().apply { | ||
| put(OK, true) | ||
| put(COUNT, count) | ||
| } | ||
| JsCallbackData(jsonPayload, featureName, method, id) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Try to lookup a string by index | ||
| * - when found, return { ok: true, serializedMigrationFile: '...' } | ||
| * - when missing, return { ok: false, reason: '...' } | ||
| */ | ||
| private suspend fun getMigrationDataByIndexResponse( | ||
| featureName: String, | ||
| method: String, | ||
| id: String, | ||
| data: JSONObject?, | ||
| ): JsCallbackData { | ||
| return withContext(dispatchers.io()) { | ||
| val index = data?.optInt(INDEX, -1) ?: -1 | ||
| val value = migrationItems.getOrNull(index) | ||
| val jsonPayload = JSONObject() | ||
| if (value == null) { | ||
| jsonPayload.put(OK, false) | ||
| jsonPayload.put(REASON, "nothing at index: $index") | ||
| } else { | ||
| jsonPayload.put(OK, true) | ||
| jsonPayload.put(SERIALIZED_MIGRATION_FILE, value) | ||
| } | ||
| JsCallbackData(jsonPayload, featureName, method, id) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Clear migration data, returning { ok: true } when complete | ||
| */ | ||
| private suspend fun getClearMigrationDataResponse( | ||
| featureName: String, | ||
| method: String, | ||
| id: String, | ||
| ): JsCallbackData { | ||
| return withContext(dispatchers.io()) { | ||
| migrationItems.clear() | ||
| val jsonPayload = JSONObject().apply { put(OK, true) } | ||
| JsCallbackData(jsonPayload, featureName, method, id) | ||
| } | ||
| } | ||
|
|
||
| companion object { | ||
| const val DUCK_CHAT_FEATURE_NAME = "aiChat" | ||
| private const val METHOD_GET_AI_CHAT_NATIVE_HANDOFF_DATA = "getAIChatNativeHandoffData" | ||
|
|
@@ -210,12 +315,25 @@ class RealDuckChatJSHelper @Inject constructor( | |
| private const val SUPPORTS_NATIVE_CHAT_INPUT = "supportsNativeChatInput" | ||
| private const val SUPPORTS_IMAGE_UPLOAD = "supportsImageUpload" | ||
| private const val SUPPORTS_CHAT_ID_RESTORATION = "supportsURLChatIDRestoration" | ||
| private const val SUPPORTS_STANDALONE_MIGRATION = "supportsStandaloneMigration" | ||
| private const val REPORT_METRIC = "reportMetric" | ||
| private const val PLATFORM = "platform" | ||
| private const val ANDROID = "android" | ||
| const val SELECTOR = "selector" | ||
| private const val DEFAULT_SELECTOR = "'user-prompt'" | ||
| private const val SUCCESS = "success" | ||
| private const val ERROR = "error" | ||
| private const val OK = "ok" | ||
| private const val REASON = "reason" | ||
|
|
||
| // Migration messaging constants | ||
| private const val METHOD_STORE_MIGRATION_DATA = "storeMigrationData" | ||
| private const val METHOD_GET_MIGRATION_INFO = "getMigrationInfo" | ||
| private const val METHOD_GET_MIGRATION_DATA_BY_INDEX = "getMigrationDataByIndex" | ||
| private const val METHOD_CLEAR_MIGRATION_DATA = "clearMigrationData" | ||
|
|
||
| private const val SERIALIZED_MIGRATION_FILE = "serializedMigrationFile" | ||
| private const val COUNT = "count" | ||
| private const val INDEX = "index" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ import androidx.lifecycle.ViewModelProvider | |
| import androidx.lifecycle.flowWithLifecycle | ||
| import androidx.lifecycle.lifecycleScope | ||
| import com.duckduckgo.anvil.annotations.InjectWith | ||
| import com.duckduckgo.app.browser.DuckDuckGoUrlDetector | ||
| import com.duckduckgo.app.di.AppCoroutineScope | ||
| import com.duckduckgo.app.tabs.BrowserNav | ||
| import com.duckduckgo.appbuildconfig.api.AppBuildConfig | ||
|
|
@@ -130,6 +131,9 @@ open class DuckChatWebViewFragment : DuckDuckGoFragment(R.layout.activity_duck_c | |
| @Inject | ||
| lateinit var duckChatJSHelper: DuckChatJSHelper | ||
|
|
||
| @Inject | ||
| lateinit var duckDuckGoUrlDetector: DuckDuckGoUrlDetector | ||
|
|
||
| @Inject | ||
| lateinit var subscriptionsHandler: SubscriptionsHandler | ||
|
|
||
|
|
@@ -209,7 +213,12 @@ open class DuckChatWebViewFragment : DuckDuckGoFragment(R.layout.activity_duck_c | |
| view?.requestFocusNodeHref(resultMsg) | ||
| val newWindowUrl = resultMsg?.data?.getString("url") | ||
| if (newWindowUrl != null) { | ||
| startActivity(browserNav.openInNewTab(requireContext(), newWindowUrl)) | ||
| if (duckDuckGoUrlDetector.isDuckAiUrl(newWindowUrl)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic should not be in the |
||
| // Allow Duck.ai links to load within the same WebView (in-sheet navigation) | ||
| simpleWebview.loadUrl(newWindowUrl) | ||
| } else { | ||
| startActivity(browserNav.openInNewTab(requireContext(), newWindowUrl)) | ||
| } | ||
| return true | ||
| } | ||
| return false | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn’t be here, belongs in
duck-chatmodule