[github] Extract request executor management logic

Build proper dispose chain to accomodate future multi-parent for holder
2026-09-27 10:03:11 +07:00 · 2018-08-29 14:56:44 +03:00
parent 1e8a061e00
commit 2ba4263ae9
5 changed files with 98 additions and 56 deletions
@@ -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()
@@ -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<GithubPullRequestsLoader>()

  private val executor = AppExecutorUtil.createBoundedApplicationPoolExecutor("GitHub PR loading breaker", 1)
  private var progressIndicator = EmptyProgressIndicator()
  private var query: String = buildQuery(null)
  private var nextPageRequest: GithubApiRequest<GithubResponsePage<GithubSearchedIssue>>? = 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 {
@@ -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()
@@ -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() {}
@@ -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
  }
 }