-
Notifications
You must be signed in to change notification settings - Fork 4
fix: simplify URI handling when the same deployment URL is already opened #227
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
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
25e2e27
emit flow events from the same thread
fioan89 c9eb261
fix: simplify URI handling when the same deployment URL is already op…
fioan89 6390b42
Merge branch 'main' into fix-uri-handling-on-linux
fioan89 cd2abb4
impl: rework the URI handler
fioan89 e4b4fca
chore: update Changelog
fioan89 269ccd4
Merge branch 'main' into fix-uri-handling-on-linux
fioan89 451aa74
chore: next version is 0.8.1
fioan89 9097242
impl: improve URI flow when Toolbox is closed
fioan89 72650d9
fix: logout for a deployment launched via URI
fioan89 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| version=0.8.0 | ||
| version=0.8.1 | ||
| group=com.coder.toolbox | ||
| name=coder-toolbox |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,20 @@ package com.coder.toolbox | |
|
|
||
| import com.coder.toolbox.browser.browse | ||
| import com.coder.toolbox.cli.CoderCLIManager | ||
| import com.coder.toolbox.plugin.PluginManager | ||
| import com.coder.toolbox.sdk.CoderRestClient | ||
| import com.coder.toolbox.sdk.ex.APIResponseException | ||
| import com.coder.toolbox.sdk.v2.models.WorkspaceStatus | ||
| import com.coder.toolbox.util.CoderProtocolHandler | ||
| import com.coder.toolbox.util.DialogUi | ||
| import com.coder.toolbox.util.TOKEN | ||
| import com.coder.toolbox.util.URL | ||
| import com.coder.toolbox.util.WebUrlValidationResult.Invalid | ||
| import com.coder.toolbox.util.toQueryParameters | ||
| import com.coder.toolbox.util.toURL | ||
| import com.coder.toolbox.util.token | ||
| import com.coder.toolbox.util.url | ||
| import com.coder.toolbox.util.validateStrictWebUrl | ||
| import com.coder.toolbox.util.waitForTrue | ||
| import com.coder.toolbox.util.withPath | ||
| import com.coder.toolbox.views.Action | ||
|
|
@@ -37,13 +46,16 @@ import kotlinx.coroutines.launch | |
| import kotlinx.coroutines.selects.onTimeout | ||
| import kotlinx.coroutines.selects.select | ||
| import java.net.URI | ||
| import java.net.URL | ||
| import java.util.concurrent.atomic.AtomicBoolean | ||
| import kotlin.coroutines.cancellation.CancellationException | ||
| import kotlin.time.Duration.Companion.seconds | ||
| import kotlin.time.TimeSource | ||
| import com.jetbrains.toolbox.api.ui.components.AccountDropdownField as DropDownMenu | ||
| import com.jetbrains.toolbox.api.ui.components.AccountDropdownField as dropDownFactory | ||
|
|
||
| private val POLL_INTERVAL = 5.seconds | ||
| private const val CAN_T_HANDLE_URI_TITLE = "Can't handle URI" | ||
|
|
||
| @OptIn(ExperimentalCoroutinesApi::class) | ||
| class CoderRemoteProvider( | ||
|
|
@@ -61,11 +73,13 @@ class CoderRemoteProvider( | |
|
|
||
| // The REST client, if we are signed in | ||
| private var client: CoderRestClient? = null | ||
| private var cli: CoderCLIManager? = null | ||
|
|
||
| // On the first load, automatically log in if we can. | ||
| private var firstRun = true | ||
|
|
||
| private val isInitialized: MutableStateFlow<Boolean> = MutableStateFlow(false) | ||
| private val isHandlingUri: AtomicBoolean = AtomicBoolean(false) | ||
| private val coderHeaderPage = NewEnvironmentPage(context.i18n.pnotr(context.deploymentUrl.toString())) | ||
| private val settingsPage: CoderSettingsPage = CoderSettingsPage(context, triggerSshConfig) { | ||
| client?.let { restClient -> | ||
|
|
@@ -82,7 +96,7 @@ class CoderRemoteProvider( | |
| providerVisible = false | ||
| ) | ||
| ) | ||
| private val linkHandler = CoderProtocolHandler(context, dialogUi, settingsPage, visibilityState, isInitialized) | ||
| private val linkHandler = CoderProtocolHandler(context) | ||
|
|
||
| override val loadingEnvironmentsDescription: LocalizableString = context.i18n.ptrl("Loading workspaces...") | ||
| override val environments: MutableStateFlow<LoadableState<List<CoderRemoteEnvironment>>> = MutableStateFlow( | ||
|
|
@@ -254,6 +268,17 @@ class CoderRemoteProvider( | |
| * Also called as part of our own logout. | ||
| */ | ||
| override fun close() { | ||
| softClose() | ||
| client = null | ||
| cli = null | ||
| lastEnvironments.clear() | ||
| environments.value = LoadableState.Value(emptyList()) | ||
| isInitialized.update { false } | ||
| CoderCliSetupWizardState.goToFirstStep() | ||
| context.logger.info("Coder plugin is now closed") | ||
| } | ||
|
|
||
| private fun softClose() { | ||
| pollJob?.let { | ||
| it.cancel() | ||
| context.logger.info("Cancelled workspace poll job ${pollJob.toString()}") | ||
|
|
@@ -262,12 +287,6 @@ class CoderRemoteProvider( | |
| it.close() | ||
| context.logger.info("REST API client closed and resources released") | ||
| } | ||
| client = null | ||
| lastEnvironments.clear() | ||
| environments.value = LoadableState.Value(emptyList()) | ||
| isInitialized.update { false } | ||
| CoderCliSetupWizardState.goToFirstStep() | ||
| context.logger.info("Coder plugin is now closed") | ||
| } | ||
|
|
||
| override val svgIcon: SvgIcon = | ||
|
|
@@ -331,27 +350,49 @@ class CoderRemoteProvider( | |
| */ | ||
| override suspend fun handleUri(uri: URI) { | ||
| try { | ||
| linkHandler.handle( | ||
| uri, | ||
| shouldDoAutoSetup() | ||
| ) { restClient, cli -> | ||
| context.logger.info("Stopping workspace polling and de-initializing resources") | ||
| close() | ||
| isInitialized.update { | ||
| false | ||
| val params = uri.toQueryParameters() | ||
| if (params.isEmpty()) { | ||
| // probably a plugin installation scenario | ||
| context.logAndShowInfo("URI will not be handled", "No query parameters were provided") | ||
| return | ||
| } | ||
| isHandlingUri.set(true) | ||
| // this switches to the main plugin screen, even | ||
| // if last opened provider was not Coder | ||
| context.envPageManager.showPluginEnvironmentsPage() | ||
| coderHeaderPage.isBusy.update { true } | ||
| context.logger.info("Handling $uri...") | ||
| val newUrl = resolveDeploymentUrl(params)?.toURL() ?: return | ||
| val newToken = if (context.settingsStore.requiresMTlsAuth) null else resolveToken(params) ?: return | ||
| if (sameUrl(newUrl, client?.url)) { | ||
| if (context.settingsStore.requiresTokenAuth) { | ||
| newToken?.let { | ||
| refreshSession(newUrl, it) | ||
| } | ||
| } | ||
| } else { | ||
| CoderCliSetupContext.apply { | ||
| url = newUrl | ||
| token = newToken | ||
| } | ||
| context.logger.info("Starting initialization with the new settings") | ||
| [email protected] = restClient | ||
| if (context.settingsStore.useAppNameAsTitle) { | ||
| coderHeaderPage.setTitle(context.i18n.pnotr(restClient.appName)) | ||
| } else { | ||
| coderHeaderPage.setTitle(context.i18n.pnotr(restClient.url.toString())) | ||
| CoderCliSetupWizardState.goToStep(WizardStep.CONNECT) | ||
| CoderCliSetupWizardPage( | ||
| context, settingsPage, visibilityState, | ||
| initialAutoSetup = true, | ||
| jumpToMainPageOnError = true, | ||
| connectSynchronously = true, | ||
| onConnect = ::onConnect | ||
| ).apply { | ||
| beforeShow() | ||
| } | ||
| environments.showLoadingMessage() | ||
| pollJob = poll(restClient, cli) | ||
| context.logger.info("Workspace poll job with name ${pollJob.toString()} was created while handling URI $uri") | ||
| isInitialized.waitForTrue() | ||
| } | ||
| // force the poll loop to run | ||
| triggerProviderVisible.send(true) | ||
| // wait for environments to be populated | ||
| isInitialized.waitForTrue() | ||
|
|
||
| linkHandler.handle(params, newUrl, this.client!!, this.cli!!) | ||
| coderHeaderPage.isBusy.update { false } | ||
| } catch (ex: Exception) { | ||
| val textError = if (ex is APIResponseException) { | ||
| if (!ex.reason.isNullOrBlank()) { | ||
|
|
@@ -363,7 +404,63 @@ class CoderRemoteProvider( | |
| textError ?: "" | ||
| ) | ||
| context.envPageManager.showPluginEnvironmentsPage() | ||
| } finally { | ||
| coderHeaderPage.isBusy.update { false } | ||
| isHandlingUri.set(false) | ||
| firstRun = false | ||
| } | ||
| } | ||
|
|
||
| private suspend fun resolveDeploymentUrl(params: Map<String, String>): String? { | ||
| val deploymentURL = params.url() ?: askUrl() | ||
| if (deploymentURL.isNullOrBlank()) { | ||
| context.logAndShowError(CAN_T_HANDLE_URI_TITLE, "Query parameter \"${URL}\" is missing from URI") | ||
| return null | ||
| } | ||
| val validationResult = deploymentURL.validateStrictWebUrl() | ||
| if (validationResult is Invalid) { | ||
| context.logAndShowError(CAN_T_HANDLE_URI_TITLE, "\"$URL\" is invalid: ${validationResult.reason}") | ||
| return null | ||
| } | ||
| return deploymentURL | ||
| } | ||
|
|
||
| private suspend fun resolveToken(params: Map<String, String>): String? { | ||
| val token = params.token() | ||
| if (token.isNullOrBlank()) { | ||
| context.logAndShowError(CAN_T_HANDLE_URI_TITLE, "Query parameter \"$TOKEN\" is missing from URI") | ||
| return null | ||
| } | ||
| return token | ||
| } | ||
|
|
||
| private fun sameUrl(first: URL, second: URL?): Boolean = first.toURI().normalize() == second?.toURI()?.normalize() | ||
|
|
||
| private suspend fun refreshSession(url: URL, token: String): Pair<CoderRestClient, CoderCLIManager> { | ||
| context.logger.info("Stopping workspace polling and re-initializing the http client and cli with a new token") | ||
| softClose() | ||
| val newRestClient = CoderRestClient( | ||
| context, | ||
| url, | ||
| token, | ||
| PluginManager.pluginInfo.version, | ||
| ).apply { initializeSession() } | ||
| val newCli = CoderCLIManager(context, url).apply { | ||
| login(token) | ||
| } | ||
| this.client = newRestClient | ||
| this.cli = newCli | ||
| pollJob = poll(newRestClient, newCli) | ||
| context.logger.info("Workspace poll job with name ${pollJob.toString()} was created while handling URI") | ||
| return newRestClient to newCli | ||
| } | ||
|
|
||
| private suspend fun askUrl(): String? { | ||
| context.popupPluginMainPage() | ||
| return dialogUi.ask( | ||
| context.i18n.ptrl("Deployment URL"), | ||
| context.i18n.ptrl("Enter the full URL of your Coder deployment") | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -373,6 +470,9 @@ class CoderRemoteProvider( | |
| * list. | ||
| */ | ||
| override fun getOverrideUiPage(): UiPage? { | ||
| if (isHandlingUri.get()) { | ||
| return null | ||
| } | ||
| // Show the setup page if we have not configured the client yet. | ||
| if (client == null) { | ||
| // When coming back to the application, initializeSession immediately. | ||
|
|
@@ -420,6 +520,7 @@ class CoderRemoteProvider( | |
|
|
||
| private fun onConnect(client: CoderRestClient, cli: CoderCLIManager) { | ||
| // Store the URL and token for use next time. | ||
| close() | ||
| context.settingsStore.updateLastUsedUrl(client.url) | ||
| if (context.settingsStore.requiresTokenAuth) { | ||
| context.secrets.storeTokenFor(client.url, client.token ?: "") | ||
|
|
@@ -428,10 +529,7 @@ class CoderRemoteProvider( | |
| context.logger.info("Deployment URL was stored and will be available for automatic connection") | ||
| } | ||
| this.client = client | ||
| pollJob?.let { | ||
| it.cancel() | ||
| context.logger.info("Cancelled workspace poll job ${pollJob.toString()} in order to start a new one") | ||
| } | ||
| this.cli = cli | ||
| environments.showLoadingMessage() | ||
| if (context.settingsStore.useAppNameAsTitle) { | ||
| coderHeaderPage.setTitle(context.i18n.pnotr(client.appName)) | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Just kind of thinking out loud, not something we have to do now or in this PR, but it looks like the pattern we have is to close the client and cli and then immediately set/unset them, so I wonder if we would benefit from formalizing that, like if we had something like
update(newClient, newCli)that closed the old ones if any and set new ones, so it is never possible to accidentally be in a sort of desynced state where you have a closed client/cli but have not updated or unset the client/cli.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.
Should be possible, I'll add it on my TODO list.