Skip to content

Commit

Permalink
fix:gh-api-request-executor fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
cunla committed Apr 8, 2024
1 parent 9b727c0 commit 8f153b8
Show file tree
Hide file tree
Showing 11 changed files with 96 additions and 56 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,15 @@

## 1.20.0

### 🚀 Features

- Tooltip on tab showing real repo and GitHub account
- Prefer GitHub account that is the owner of the repository

### 🐛 Bug Fixes

- Using new error handlers.
- Create GitHub Request executor that supports redirects. # 119

### 🧰 Maintenance

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.dsoftware.ghmanager.api.model.Job
import com.dsoftware.ghmanager.api.model.JobStep
import com.dsoftware.ghmanager.i18n.MessagesBundle.message
import com.intellij.openapi.diagnostic.logger
import com.intellij.util.io.HttpSecurityUtil
import kotlinx.datetime.Instant
import org.jetbrains.annotations.VisibleForTesting
import org.jetbrains.plugins.github.api.GithubApiRequest
Expand All @@ -17,7 +16,8 @@ import java.io.InputStreamReader

typealias JobLog = Map<Int, StringBuilder>

class GetJobLogRequest(private val job: Job) : GithubApiRequest.Get<String>(job.url + "/logs") {
class GetJobLogRequest(private val job: Job) :
GithubApiRequest.Get<String>(job.url + "/logs", acceptMimeType = "application/vnd.github+json") {
private val stepsMap = job.steps.associateBy { step -> step.number }
private val lastStepNumber: Int = stepsMap.keys.maxOrNull() ?: 0

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package com.dsoftware.ghmanager.api

import com.intellij.openapi.components.service
import com.intellij.openapi.progress.ProcessCanceledException
import com.intellij.openapi.progress.ProgressIndicator
import com.intellij.util.io.HttpSecurityUtil
import org.jetbrains.plugins.github.api.GHRequestExecutorBreaker
import org.jetbrains.plugins.github.api.GithubApiRequest
import org.jetbrains.plugins.github.api.GithubApiRequestExecutor
import org.jetbrains.plugins.github.util.GithubSettings
import java.io.IOException

/**
* Executes API requests taking care of authentication, headers, proxies, timeouts, etc.
*/
class GhApiRequestExecutor(
githubSettings: GithubSettings,
private val token: String,
private val useProxy: Boolean,
) : GithubApiRequestExecutor.Base(githubSettings) {

@Throws(IOException::class, ProcessCanceledException::class)
override fun <T> execute(indicator: ProgressIndicator, request: GithubApiRequest<T>): T {
check(!service<GHRequestExecutorBreaker>().isRequestsShouldFail) {
"Request failure was triggered by user action. This a pretty long description of this failure that should resemble some long error which can go out of bounds."
}

indicator.checkCanceled()
return createRequestBuilder(request)
.tuner { connection ->
request.additionalHeaders.forEach(connection::addRequestProperty)
if (request.url.contains(connection.url.host)) {
connection.addRequestProperty(HttpSecurityUtil.AUTHORIZATION_HEADER_NAME, "Bearer ${token}")
}
}
.useProxy(useProxy)
.execute(request, indicator)
}


companion object {
fun create(token: String): GhApiRequestExecutor =
GhApiRequestExecutor(GithubSettings.getInstance(), token, true)
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.dsoftware.ghmanager.data

import com.dsoftware.ghmanager.api.GhApiRequestExecutor
import com.dsoftware.ghmanager.ui.settings.GhActionsSettingsService
import com.intellij.openapi.components.Service
import com.intellij.openapi.components.service
Expand All @@ -11,7 +12,6 @@ import com.intellij.openapi.util.CheckedDisposable
import com.intellij.openapi.wm.ToolWindow
import com.intellij.util.concurrency.annotations.RequiresEdt
import org.jetbrains.plugins.github.api.GHRepositoryPath
import org.jetbrains.plugins.github.api.GithubApiRequestExecutor
import org.jetbrains.plugins.github.api.GithubServerPath
import org.jetbrains.plugins.github.authentication.accounts.GithubAccount
import org.jetbrains.plugins.github.exceptions.GithubMissingTokenException
Expand Down Expand Up @@ -50,8 +50,7 @@ class WorkflowDataContextService(private val project: Project) {
} else {
settingsService.state.apiToken
}

val requestExecutor = GithubApiRequestExecutor.Factory.getInstance().create(token)
val requestExecutor = GhApiRequestExecutor.create(token)
if (checkedDisposable.isDisposed) {
throw ProcessCanceledException(
RuntimeException("Skipped creating data context for ${repositoryMapping.remote.url} because it was disposed")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.dsoftware.ghmanager.data

import com.dsoftware.ghmanager.api.GhApiRequestExecutor
import com.dsoftware.ghmanager.api.WorkflowRunFilter
import com.dsoftware.ghmanager.api.model.Job
import com.dsoftware.ghmanager.api.model.WorkflowRun
Expand All @@ -17,7 +18,6 @@ import com.intellij.openapi.util.Disposer
import com.intellij.openapi.wm.ToolWindow
import com.intellij.util.EventDispatcher
import org.jetbrains.plugins.github.api.GithubApiRequest
import org.jetbrains.plugins.github.api.GithubApiRequestExecutor
import org.jetbrains.plugins.github.api.data.GHUser
import org.jetbrains.plugins.github.authentication.accounts.GithubAccount
import org.jetbrains.plugins.github.util.GHGitRepositoryMapping
Expand All @@ -31,7 +31,7 @@ class WorkflowRunSelectionContext internal constructor(
val toolWindow: ToolWindow,
val account: GithubAccount,
val repositoryMapping: GHGitRepositoryMapping,
val requestExecutor: GithubApiRequestExecutor,
val requestExecutor: GhApiRequestExecutor,
val runSelectionHolder: WorkflowRunListSelectionHolder = WorkflowRunListSelectionHolder(),
val jobSelectionHolder: JobListSelectionHolder = JobListSelectionHolder(),
) : Disposable.Parent {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,51 +1,48 @@
package com.dsoftware.ghmanager.data.providers

import com.dsoftware.ghmanager.api.GhApiRequestExecutor
import com.intellij.collaboration.async.CompletableFutureUtil.submitIOTask
import com.intellij.collaboration.util.ProgressIndicatorsProvider
import com.intellij.openapi.Disposable
import com.intellij.openapi.diagnostic.thisLogger
import com.intellij.openapi.progress.ProgressManager
import com.intellij.util.EventDispatcher
import org.jetbrains.plugins.github.api.GithubApiRequest
import org.jetbrains.plugins.github.api.GithubApiRequestExecutor
import org.jetbrains.plugins.github.exceptions.GithubStatusCodeException
import org.jetbrains.plugins.github.util.LazyCancellableBackgroundProcessValue
import java.io.IOException
import java.util.EventListener
import java.util.concurrent.CompletableFuture
import kotlin.properties.ReadOnlyProperty

open class DataProvider<T>(
private val requestExecutor: GithubApiRequestExecutor,
private val requestExecutor: GhApiRequestExecutor,
private val githubApiRequest: GithubApiRequest<T>,
private val errorValue: T?,
) {
private val runChangesEventDispatcher = EventDispatcher.create(DataProviderChangeListener::class.java)

private val processValue: LazyCancellableBackgroundProcessValue<T> =
LazyCancellableBackgroundProcessValue.create(ProgressManager.getInstance()) {
try {
LOG.info("Executing ${githubApiRequest.url}")
val request = githubApiRequest
val response = requestExecutor.execute(it, request)
response
} catch (e: GithubStatusCodeException) {
LOG.warn("Error when getting $githubApiRequest.url: $e")
errorValue ?: throw e
} catch (ioe: IOException) {
LOG.warn("Error when getting $githubApiRequest.url: $ioe")
errorValue ?: throw ioe
}
private val progressManager = ProgressManager.getInstance()
private val indicatorsProvider: ProgressIndicatorsProvider = ProgressIndicatorsProvider()

private val processValue = progressManager.submitIOTask(indicatorsProvider, true) {
try {
LOG.info("Executing ${githubApiRequest.url}")
val request = githubApiRequest
val response = requestExecutor.execute(it, request)
response
} catch (e: GithubStatusCodeException) {
LOG.warn("Error when getting $githubApiRequest.url: status code ${e.statusCode}: ${e.message}")
errorValue ?: throw e
} catch (ioe: IOException) {
LOG.warn("Error when getting $githubApiRequest.url: $ioe")
errorValue ?: throw ioe
}
}

val request by backgroundProcessValue(processValue)

private fun <T> backgroundProcessValue(backingValue: LazyCancellableBackgroundProcessValue<T>)
: ReadOnlyProperty<Any?, CompletableFuture<T>> =
ReadOnlyProperty { _, _ -> backingValue.value }
val request
get() = processValue

fun url(): String = githubApiRequest.url

fun reload() {
processValue.drop()
processValue.cancel(true)
runChangesEventDispatcher.multicaster.changed()
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package com.dsoftware.ghmanager.data.providers

import com.dsoftware.ghmanager.api.GhApiRequestExecutor
import com.dsoftware.ghmanager.api.GithubApi
import com.dsoftware.ghmanager.api.model.WorkflowRunJobs
import org.jetbrains.plugins.github.api.GithubApiRequestExecutor

class JobsDataProvider(
requestExecutor: GithubApiRequestExecutor,
requestExecutor: GhApiRequestExecutor,
jobsUrl: String
) : DataProvider<WorkflowRunJobs>(
requestExecutor,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package com.dsoftware.ghmanager.data.providers

import com.dsoftware.ghmanager.api.GhApiRequestExecutor
import com.dsoftware.ghmanager.api.GithubApi
import com.dsoftware.ghmanager.api.model.Job
import org.jetbrains.plugins.github.api.GithubApiRequestExecutor

class LogDataProvider(
requestExecutor: GithubApiRequestExecutor,
requestExecutor: GhApiRequestExecutor,
job: Job
) : DataProvider<String>(
requestExecutor,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class GhActionsMgrToolWindowContent(val toolWindow: ToolWindow) : Disposable {
val disposable = Disposer.newDisposable("gha-manager ${repo.repositoryPath} tab disposable")
Disposer.register(toolWindow.disposable, disposable)
setDisposer(disposable)
description="repo:${repo.repository.repositoryPath} using account:${ghAccount.name}"
description = "repo:${repo.repository.repositoryPath} using account:${ghAccount.name}"
}
val controller = RepoTabController(toolWindow, ghAccount, repo, tab.disposer!!)
tab.component.apply {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.dsoftware.ghmanager

import com.dsoftware.ghmanager.api.GhApiRequestExecutor
import com.dsoftware.ghmanager.api.model.WorkflowRun
import com.dsoftware.ghmanager.api.model.WorkflowRuns
import com.dsoftware.ghmanager.api.model.WorkflowType
Expand Down Expand Up @@ -36,7 +37,7 @@ import javax.swing.JTextPane
@ExtendWith(MockKExtension::class)
class TestRepoTabControllerWorkflowRunsPanel : GitHubActionsManagerBaseTest() {
@MockK
lateinit var executorMock: GithubApiRequestExecutor
lateinit var executorMock: GhApiRequestExecutor

init {
mockkStatic(GHCompatibilityUtil::class)
Expand Down Expand Up @@ -132,10 +133,8 @@ class TestRepoTabControllerWorkflowRunsPanel : GitHubActionsManagerBaseTest() {
execute(any(), matchApiRequestUrl<WorkflowTypes>("/actions/workflows"))
} returns workflowTypesResponse
}
mockkObject(GithubApiRequestExecutor.Factory)
every { GithubApiRequestExecutor.Factory.getInstance() } returns mockk<GithubApiRequestExecutor.Factory> {
every { create(token = any()) } returns executorMock
}
mockkObject(GhApiRequestExecutor)
every { GhApiRequestExecutor.create(token = any()) } returns executorMock
}

fun assertTabsAndPanels(): Triple<WorkflowRunsListPanel, JComponent, JComponent> {
Expand Down
20 changes: 7 additions & 13 deletions src/test/kotlin/com/dsoftware/ghmanager/ToolWindowFactoryTest.kt
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
package com.dsoftware.ghmanager

import com.dsoftware.ghmanager.api.GhApiRequestExecutor
import com.dsoftware.ghmanager.i18n.MessagesBundle.message
import com.dsoftware.ghmanager.ui.GhActionsMgrToolWindowContent
import com.dsoftware.ghmanager.ui.settings.GithubActionsManagerSettings
import com.intellij.ui.SimpleColoredComponent
import com.intellij.ui.components.JBPanelWithEmptyText
import io.mockk.Called
import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.junit5.MockKExtension
import io.mockk.mockkObject
import io.mockk.mockkStatic
import io.mockk.verify
import org.jetbrains.plugins.github.api.GithubApiRequestExecutor
import org.jetbrains.plugins.github.util.GHCompatibilityUtil
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.BeforeEach
Expand All @@ -24,17 +23,12 @@ import javax.swing.JPanel

@ExtendWith(MockKExtension::class)
class ToolWindowFactoryTest : GitHubActionsManagerBaseTest() {
@MockK
private lateinit var requestExecutorfactoryMock: GithubApiRequestExecutor.Factory

@BeforeEach
override fun setUp(testInfo: TestInfo) {
super.setUp(testInfo)
requestExecutorfactoryMock.apply {
every { create(token = any()) } throws Exception("No executor")
}
mockkObject(GithubApiRequestExecutor.Factory)
every { GithubApiRequestExecutor.Factory.getInstance() } returns requestExecutorfactoryMock
mockkObject(GhApiRequestExecutor)
every { GhApiRequestExecutor.create(token = any()) } throws Exception("No executor")
}

@Test
Expand All @@ -59,7 +53,7 @@ class ToolWindowFactoryTest : GitHubActionsManagerBaseTest() {
Assertions.assertEquals(message("factory.go.to.github-settings"), subComponents[1].getCharSequence(true))
Assertions.assertEquals(message("factory.go.to.ghmanager-settings"), subComponents[2].getCharSequence(true))
verify {
requestExecutorfactoryMock.create(token = any()) wasNot Called
GhApiRequestExecutor.create(token = any()) wasNot Called
}
}

Expand All @@ -81,7 +75,7 @@ class ToolWindowFactoryTest : GitHubActionsManagerBaseTest() {
val panel = component as JBPanelWithEmptyText
Assertions.assertEquals(message("factory.empty-panel.no-repos-in-project"), panel.emptyText.text)
verify {
requestExecutorfactoryMock.create(token = any()) wasNot Called
GhApiRequestExecutor.create(token = any()) wasNot Called
}

}
Expand All @@ -104,7 +98,7 @@ class ToolWindowFactoryTest : GitHubActionsManagerBaseTest() {
val panel = content.component as JPanel
Assertions.assertEquals(1, panel.componentCount)
verify {
requestExecutorfactoryMock.create(token = any())
GhApiRequestExecutor.create(token = any())
}


Expand Down Expand Up @@ -143,7 +137,7 @@ class ToolWindowFactoryTest : GitHubActionsManagerBaseTest() {
)
Assertions.assertEquals(message("factory.go.to.ghmanager-settings"), subComponents[1].getCharSequence(true))
verify {
requestExecutorfactoryMock.create(token = any()) wasNot Called
GhApiRequestExecutor.create(token = any()) wasNot Called
}

}
Expand Down

0 comments on commit 8f153b8

Please sign in to comment.