diff --git a/plugins/github/src/org/jetbrains/plugins/github/api/GithubApiRequestExecutorManager.kt b/plugins/github/src/org/jetbrains/plugins/github/api/GithubApiRequestExecutorManager.kt index c627667fe222..652699d984cc 100644 --- a/plugins/github/src/org/jetbrains/plugins/github/api/GithubApiRequestExecutorManager.kt +++ b/plugins/github/src/org/jetbrains/plugins/github/api/GithubApiRequestExecutorManager.kt @@ -1,13 +1,21 @@ // Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package org.jetbrains.plugins.github.api +import com.intellij.openapi.Disposable +import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.application.runInEdt import com.intellij.openapi.components.service import com.intellij.openapi.project.Project +import com.intellij.util.EventDispatcher +import org.jetbrains.annotations.CalledInAny import org.jetbrains.annotations.CalledInAwt import org.jetbrains.plugins.github.authentication.GithubAuthenticationManager +import org.jetbrains.plugins.github.authentication.accounts.AccountTokenChangedListener import org.jetbrains.plugins.github.authentication.accounts.GithubAccount +import org.jetbrains.plugins.github.authentication.accounts.GithubAccountManager import org.jetbrains.plugins.github.exceptions.GithubMissingTokenException import java.awt.Component +import java.util.* /** * Allows to acquire API executor without exposing the auth token to external code @@ -20,6 +28,14 @@ class GithubApiRequestExecutorManager(private val authenticationManager: GithubA ?.let(requestExecutorFactory::create) } + @CalledInAwt + fun getManagedHolder(account: GithubAccount, project: Project): ManagedHolder? { + val requestExecutor = getExecutor(account, project) ?: return null + val holder = ManagedHolder(account, requestExecutor) + ApplicationManager.getApplication().messageBus.connect(holder).subscribe(GithubAccountManager.ACCOUNT_TOKEN_CHANGED_TOPIC, holder) + return holder + } + @CalledInAwt fun getExecutor(account: GithubAccount, parentComponent: Component): GithubApiRequestExecutor? { return authenticationManager.getOrRequestTokenForAccount(account, null, parentComponent) @@ -33,6 +49,46 @@ class GithubApiRequestExecutorManager(private val authenticationManager: GithubA ?: throw GithubMissingTokenException(account) } + inner class ManagedHolder internal constructor(private val account: GithubAccount, initialExecutor: GithubApiRequestExecutor) + : Disposable, AccountTokenChangedListener { + + private var isDisposed = false + var executor: GithubApiRequestExecutor = initialExecutor + @CalledInAny + get() { + if (isDisposed) throw IllegalStateException("Already disposed") + return field + } + @CalledInAwt + internal set(value) { + field = value + eventDispatcher.multicaster.executorChanged() + } + + private val eventDispatcher = EventDispatcher.create(ExecutorChangeListener::class.java) + + override fun tokenChanged(account: GithubAccount) { + if (account == this.account) runInEdt { + try { + executor = getExecutor(account) + } + catch (e: GithubMissingTokenException) { + //token is missing, so content will be closed anyway + } + } + } + + fun addListener(listener: ExecutorChangeListener, disposable: Disposable) = eventDispatcher.addListener(listener, disposable) + + override fun dispose() { + isDisposed = true + } + } + + interface ExecutorChangeListener : EventListener { + fun executorChanged() + } + companion object { @JvmStatic fun getInstance(): GithubApiRequestExecutorManager = service() diff --git a/plugins/github/src/org/jetbrains/plugins/github/pullrequest/data/GithubPullRequestsLoader.kt b/plugins/github/src/org/jetbrains/plugins/github/pullrequest/data/GithubPullRequestsLoader.kt index 465f2604a892..4c115857b769 100644 --- a/plugins/github/src/org/jetbrains/plugins/github/pullrequest/data/GithubPullRequestsLoader.kt +++ b/plugins/github/src/org/jetbrains/plugins/github/pullrequest/data/GithubPullRequestsLoader.kt @@ -2,13 +2,11 @@ package org.jetbrains.plugins.github.pullrequest.data import com.intellij.openapi.Disposable -import com.intellij.openapi.application.runInEdt import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.progress.EmptyProgressIndicator import com.intellij.openapi.progress.ProcessCanceledException import com.intellij.openapi.progress.ProgressIndicator import com.intellij.openapi.progress.ProgressManager -import com.intellij.openapi.project.Project import com.intellij.util.EventDispatcher import com.intellij.util.concurrency.AppExecutorUtil import org.jetbrains.annotations.CalledInAwt @@ -18,26 +16,28 @@ import org.jetbrains.plugins.github.api.data.GithubResponsePage import org.jetbrains.plugins.github.api.data.GithubSearchedIssue import org.jetbrains.plugins.github.api.search.GithubIssueSearchType import org.jetbrains.plugins.github.api.util.GithubApiSearchQueryBuilder -import org.jetbrains.plugins.github.authentication.accounts.AccountTokenChangedListener -import org.jetbrains.plugins.github.authentication.accounts.GithubAccount -import org.jetbrains.plugins.github.authentication.accounts.GithubAccountManager -import org.jetbrains.plugins.github.exceptions.GithubMissingTokenException import org.jetbrains.plugins.github.pullrequest.search.GithubPullRequestSearchQuery import java.util.* -class GithubPullRequestsLoader private constructor(private val progressManager: ProgressManager, - private var requestExecutor: GithubApiRequestExecutor, - private val serverPath: GithubServerPath, - private val repoPath: GithubFullPath) : Disposable { +class GithubPullRequestsLoader(private val progressManager: ProgressManager, + private val requestExecutorHolder: GithubApiRequestExecutorManager.ManagedHolder, + private val serverPath: GithubServerPath, + private val repoPath: GithubFullPath) + : Disposable, GithubApiRequestExecutorManager.ExecutorChangeListener { private val LOG = logger() private val executor = AppExecutorUtil.createBoundedApplicationPoolExecutor("GitHub PR loading breaker", 1) private var progressIndicator = EmptyProgressIndicator() private var query: String = buildQuery(null) private var nextPageRequest: GithubApiRequest>? = createInitialRequest() + private var isDisposed = false private val stateEventDispatcher = EventDispatcher.create(StateListener::class.java) + init { + requestExecutorHolder.addListener(this, this) + } + private fun createInitialRequest() = GithubApiRequests.Search.Issues.get(serverPath, query) @CalledInAwt @@ -55,14 +55,16 @@ class GithubPullRequestsLoader private constructor(private val progressManager: @CalledInAwt fun requestLoadMore() { + if (isDisposed) return LOG.debug("Requested more pull requests") val indicator = progressIndicator + val requestExecutor = requestExecutorHolder.executor executor.execute { if (indicator.isCanceled) return@execute try { stateEventDispatcher.multicaster.loadingStarted() LOG.debug("Starting listeners notified") - progressManager.runProcess({ loadMore(indicator) }, indicator) + progressManager.runProcess({ loadMore(requestExecutor, indicator) }, indicator) } catch (pce: ProcessCanceledException) { // ignore @@ -75,7 +77,7 @@ class GithubPullRequestsLoader private constructor(private val progressManager: } @CalledInBackground - private fun loadMore(progressIndicator: ProgressIndicator) { + private fun loadMore(requestExecutor: GithubApiRequestExecutor, progressIndicator: ProgressIndicator) { try { LOG.debug("Loading pull requests") val request = nextPageRequest @@ -101,8 +103,13 @@ class GithubPullRequestsLoader private constructor(private val progressManager: } } + override fun executorChanged() { + reset() + } + @CalledInAwt fun reset() { + if (isDisposed) return progressIndicator.cancel() progressIndicator = object : EmptyProgressIndicator() { override fun start() { @@ -117,36 +124,11 @@ class GithubPullRequestsLoader private constructor(private val progressManager: } } - fun addStateListener(listener: StateListener) = stateEventDispatcher.addListener(listener) - - fun removeStateListener(listener: StateListener) = stateEventDispatcher.removeListener(listener) + fun addStateListener(listener: StateListener, disposable: Disposable) = stateEventDispatcher.addListener(listener, disposable) override fun dispose() { progressIndicator.cancel() - } - - companion object { - @JvmStatic - fun create(project: Project, progressManager: ProgressManager, requestExecutorManager: GithubApiRequestExecutorManager, - accountToUse: GithubAccount, repoPath: GithubFullPath): GithubPullRequestsLoader? { - - val requestExecutor = requestExecutorManager.getExecutor(accountToUse, project) ?: return null - val loader = GithubPullRequestsLoader(progressManager, requestExecutor, accountToUse.server, repoPath) - project.messageBus.connect(loader).subscribe(GithubAccountManager.ACCOUNT_TOKEN_CHANGED_TOPIC, object : AccountTokenChangedListener { - override fun tokenChanged(account: GithubAccount) { - if (account == accountToUse) runInEdt { - try { - loader.requestExecutor = requestExecutorManager.getExecutor(account) - loader.reset() - } - catch (e: GithubMissingTokenException) { - //token is missing, so content will be closed anyway - } - } - } - }) - return loader - } + isDisposed = true } interface StateListener : EventListener { diff --git a/plugins/github/src/org/jetbrains/plugins/github/pullrequest/search/GithubPullRequestSearchModel.kt b/plugins/github/src/org/jetbrains/plugins/github/pullrequest/search/GithubPullRequestSearchModel.kt index 65b01c5ac1e0..d40e07c5dae1 100644 --- a/plugins/github/src/org/jetbrains/plugins/github/pullrequest/search/GithubPullRequestSearchModel.kt +++ b/plugins/github/src/org/jetbrains/plugins/github/pullrequest/search/GithubPullRequestSearchModel.kt @@ -1,6 +1,7 @@ // Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package org.jetbrains.plugins.github.pullrequest.search +import com.intellij.openapi.Disposable import com.intellij.util.EventDispatcher import org.jetbrains.annotations.CalledInAwt import java.util.* @@ -16,9 +17,7 @@ class GithubPullRequestSearchModel { private val stateEventDispatcher = EventDispatcher.create(StateListener::class.java) - fun addStateListener(listener: StateListener) = stateEventDispatcher.addListener(listener) - - fun removeStateListener(listener: StateListener) = stateEventDispatcher.removeListener(listener) + fun addListener(listener: StateListener, disposable: Disposable) = stateEventDispatcher.addListener(listener, disposable) interface StateListener : EventListener { fun queryChanged() diff --git a/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/GithubPullRequestsComponentFactory.kt b/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/GithubPullRequestsComponentFactory.kt index 047d389e9161..8d5f6762bb64 100644 --- a/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/GithubPullRequestsComponentFactory.kt +++ b/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/GithubPullRequestsComponentFactory.kt @@ -23,22 +23,25 @@ class GithubPullRequestsComponentFactory(private val project: Project, private val popupFactory: JBPopupFactory) { fun createComponent(remoteUrl: String, account: GithubAccount): JComponent? { - val loader = GithubPullRequestsLoader.create(project, progressManager, requestExecutorManager, - account, GithubUrlUtil.getUserAndRepositoryFromRemoteUrl(remoteUrl)!!) ?: return null - val list = GithubPullRequestsListComponent(project, actionManager, autoPopupController, popupFactory, loader) - Disposer.register(list, loader) - return GithubPullRequestsComponent(list) + val requestExecutorHolder = requestExecutorManager.getManagedHolder(account, project) ?: return null + val loader = GithubPullRequestsLoader(progressManager, requestExecutorHolder, + account.server, GithubUrlUtil.getUserAndRepositoryFromRemoteUrl(remoteUrl)!!) + val list = GithubPullRequestsListComponent(project, actionManager, autoPopupController, popupFactory, loader) + + val wrapper = DisposableWrapper(list) + Disposer.register(wrapper, Disposable { + Disposer.dispose(list) + Disposer.dispose(loader) + Disposer.dispose(requestExecutorHolder) + }) + return wrapper } companion object { - private class GithubPullRequestsComponent(list: GithubPullRequestsListComponent) - : Wrapper(), Disposable { - + private class DisposableWrapper(wrapped: JComponent) : Wrapper(wrapped), Disposable { init { isFocusCycleRoot = true - Disposer.register(this, list) - setContent(list) } override fun dispose() {} diff --git a/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/GithubPullRequestsListComponent.kt b/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/GithubPullRequestsListComponent.kt index 38e6cf0118fd..8abef1da487d 100644 --- a/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/GithubPullRequestsListComponent.kt +++ b/plugins/github/src/org/jetbrains/plugins/github/pullrequest/ui/GithubPullRequestsListComponent.kt @@ -42,20 +42,21 @@ class GithubPullRequestsListComponent(project: Project, verticalScrollBar.model.addChangeListener { potentiallyLoadMore() } } private var loadOnScrollThreshold = true + private var isDisposed = false private val errorPanel = HtmlErrorPanel() private val progressStripe = ProgressStripe(scrollPane, this, ProgressWindow.DEFAULT_PROGRESS_DIALOG_POSTPONE_TIME_MILLIS) private val searchModel = GithubPullRequestSearchModel() private val search = GithubPullRequestSearchComponent(project, autoPopupController, popupFactory, searchModel) init { - loader.addStateListener(this) + loader.addStateListener(this, this) - searchModel.addStateListener(object : GithubPullRequestSearchModel.StateListener { + searchModel.addListener(object : GithubPullRequestSearchModel.StateListener { override fun queryChanged() { loader.setSearchQuery(searchModel.query) loader.reset() } - }) + }, this) val refreshAction = object : DumbAwareAction("Refresh", null, AllIcons.Actions.Refresh) { override fun actionPerformed(e: AnActionEvent) = loader.reset() @@ -83,6 +84,7 @@ class GithubPullRequestsListComponent(project: Project, } private fun loadMore() { + if(isDisposed) return loadOnScrollThreshold = false loader.requestLoadMore() } @@ -167,6 +169,6 @@ class GithubPullRequestsListComponent(project: Project, private fun addSpaceIfNeeded(line: String) = if (line.endsWith(' ')) line else "$line " override fun dispose() { - loader.removeStateListener(this) + isDisposed = true } } \ No newline at end of file