Skip to content
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

CID-3368: Paginate getting installations & organisations #69

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/main/kotlin/net/leanix/githubagent/client/GitHubClient.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ interface GitHubClient {
): GitHubAppResponse

@GetMapping("/api/v3/app/installations")
fun getInstallations(@RequestHeader("Authorization") jwt: String): List<Installation>
fun getInstallations(
@RequestHeader("Authorization") jwt: String,
@RequestParam("per_page", defaultValue = "30") perPage: Int,
@RequestParam("page", defaultValue = "1") page: Int
): List<Installation>

@GetMapping("/api/v3/app/installations/{installationId}")
fun getInstallation(
Expand All @@ -43,7 +47,11 @@ interface GitHubClient {
): InstallationTokenResponse

@GetMapping("/api/v3/organizations")
fun getOrganizations(@RequestHeader("Authorization") token: String): List<Organization>
fun getOrganizations(
@RequestHeader("Authorization") jwt: String,
@RequestParam("per_page", defaultValue = "30") perPage: Int,
@RequestParam("since", defaultValue = "1") since: Int
): List<Organization>

@GetMapping("/api/v3/orgs/{org}/repos")
fun getRepositories(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package net.leanix.githubagent.services

import net.leanix.githubagent.client.GitHubClient
import net.leanix.githubagent.dto.Installation
import net.leanix.githubagent.dto.Organization
import org.springframework.stereotype.Service

@Service
class GitHubAPIService(
private val gitHubClient: GitHubClient,
) {

companion object {
private const val PAGE_SIZE = 30 // Maximum allowed by GitHub API is 100
}

fun getPaginatedInstallations(jwtToken: String): List<Installation> {
val installations = mutableListOf<Installation>()
var page = 1
var currentInstallations: List<Installation>

do {
currentInstallations = gitHubClient.getInstallations("Bearer $jwtToken", PAGE_SIZE, page)
if (currentInstallations.isNotEmpty()) installations.addAll(currentInstallations) else break
page++
} while (currentInstallations.size == PAGE_SIZE)
return installations
}

fun getPaginatedOrganizations(installationToken: String): List<Organization> {
val organizations = mutableListOf<Organization>()
var since = 1
var currentOrganizations: List<Organization>

do {
currentOrganizations = gitHubClient.getOrganizations("Bearer $installationToken", PAGE_SIZE, since)
if (currentOrganizations.isNotEmpty()) organizations.addAll(currentOrganizations) else break
since = currentOrganizations.last().id
} while (currentOrganizations.size == PAGE_SIZE)
return organizations
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import net.leanix.githubagent.client.GitHubClient
import net.leanix.githubagent.config.GitHubEnterpriseProperties
import net.leanix.githubagent.dto.Installation
import net.leanix.githubagent.exceptions.FailedToCreateJWTException
import net.leanix.githubagent.exceptions.JwtTokenNotFound
import org.bouncycastle.jce.provider.BouncyCastleProvider
import org.slf4j.LoggerFactory
import org.springframework.core.io.ResourceLoader
Expand All @@ -26,7 +27,8 @@ class GitHubAuthenticationService(
private val githubEnterpriseProperties: GitHubEnterpriseProperties,
private val resourceLoader: ResourceLoader,
private val gitHubEnterpriseService: GitHubEnterpriseService,
private val gitHubClient: GitHubClient
private val gitHubClient: GitHubClient,
private val gitHubAPIService: GitHubAPIService,
) {

companion object {
Expand All @@ -38,11 +40,9 @@ class GitHubAuthenticationService(

fun refreshTokens() {
generateAndCacheJwtToken()
val jwtToken = cachingService.get("jwtToken")
generateAndCacheInstallationTokens(
gitHubClient.getInstallations("Bearer $jwtToken"),
jwtToken.toString()
)
val jwtToken = cachingService.get("jwtToken") ?: throw JwtTokenNotFound()
val installations = gitHubAPIService.getPaginatedInstallations(jwtToken.toString())
generateAndCacheInstallationTokens(installations, jwtToken.toString())
}

fun generateAndCacheJwtToken() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class GitHubScanningService(
private val syncLogService: SyncLogService,
private val rateLimitHandler: RateLimitHandler,
private val gitHubEnterpriseService: GitHubEnterpriseService,
private val gitHubAPIService: GitHubAPIService,
) {

private val logger = LoggerFactory.getLogger(GitHubScanningService::class.java)
Expand Down Expand Up @@ -67,7 +68,7 @@ class GitHubScanningService(
}

private fun getInstallations(jwtToken: String): List<Installation> {
val installations = gitHubClient.getInstallations("Bearer $jwtToken")
val installations = gitHubAPIService.getPaginatedInstallations(jwtToken)
gitHubAuthenticationService.generateAndCacheInstallationTokens(installations, jwtToken)
return installations
}
Expand All @@ -81,7 +82,7 @@ class GitHubScanningService(
return
}
val installationToken = cachingService.get("installationToken:${installations.first().id}")
val organizations = gitHubClient.getOrganizations("Bearer $installationToken")
val organizations = gitHubAPIService.getPaginatedOrganizations(installationToken.toString())
.map { organization ->
if (installations.find { it.account.login == organization.login } != null) {
OrganizationDto(organization.id, organization.login, true)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package net.leanix.githubagent.services

import io.mockk.every
import io.mockk.mockk
import net.leanix.githubagent.client.GitHubClient
import net.leanix.githubagent.dto.Account
import net.leanix.githubagent.dto.Installation
import net.leanix.githubagent.dto.Organization
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test

class GitHubAPIServiceTest {

private val gitHubClient = mockk<GitHubClient>()
private val gitHubAPIService = GitHubAPIService(gitHubClient)

private val permissions = mapOf("administration" to "read", "contents" to "read", "metadata" to "read")
private val events = listOf("label", "public", "repository", "push")

@Test
fun `test getPaginatedInstallations with one page`() {
val jwtToken = "test-jwt-token"
val installationsPage1 = listOf(
Installation(1, Account("test-account"), permissions, events),
Installation(2, Account("test-account"), permissions, events)
)

every { gitHubClient.getInstallations(any(), any(), any()) } returns installationsPage1

val installations = gitHubAPIService.getPaginatedInstallations(jwtToken)
assertEquals(2, installations.size)
assertEquals(installationsPage1, installations)
}

@Test
fun `test getPaginatedInstallations with multiple pages`() {
val jwtToken = "test-jwt-token"
val perPage = 30
val totalInstallations = 100
val installations = (1..totalInstallations).map {
Installation(it.toLong(), Account("test-account-$it"), permissions, events)
}
val pages = installations.chunked(perPage)

every { gitHubClient.getInstallations(any(), any(), any()) } returnsMany pages + listOf(emptyList())

val result = gitHubAPIService.getPaginatedInstallations(jwtToken)
assertEquals(totalInstallations, result.size)
assertEquals(installations, result)
}

@Test
fun `test getPaginatedOrganizations with one page`() {
val installationToken = "test-installation-token"
val organizationsPage1 = listOf(
Organization("org-1", 1),
Organization("org-2", 2)
)

every { gitHubClient.getOrganizations(any(), any(), any()) } returns organizationsPage1

val organizations = gitHubAPIService.getPaginatedOrganizations(installationToken)
assertEquals(2, organizations.size)
assertEquals(organizationsPage1, organizations)
}

@Test
fun `test getPaginatedOrganizations with multiple pages`() {
val installationToken = "test-installation-token"
val perPage = 30
val totalOrganizations = 100
val organizations = (1..totalOrganizations).map { Organization("org-$it", it) }
val pages = organizations.chunked(perPage)

every { gitHubClient.getOrganizations(any(), any(), any()) } returnsMany pages + listOf(emptyList())

val result = gitHubAPIService.getPaginatedOrganizations(installationToken)
assertEquals(totalOrganizations, result.size)
assertEquals(organizations, result)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ class GitHubAuthenticationServiceTest {
private val gitHubEnterpriseService = mockk<GitHubEnterpriseService>()
private val gitHubClient = mockk<GitHubClient>()
private val syncLogService = mockk<SyncLogService>()
private val gitHubAPIService = mockk<GitHubAPIService>()
private val githubAuthenticationService = GitHubAuthenticationService(
cachingService,
githubEnterpriseProperties,
resourceLoader,
gitHubEnterpriseService,
gitHubClient
gitHubClient,
gitHubAPIService
)

@BeforeEach
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class GitHubScanningServiceTest {
private val gitHubAuthenticationService = mockk<GitHubAuthenticationService>()
private val syncLogService = mockk<SyncLogService>(relaxUnitFun = true)
private val rateLimitHandler = mockk<RateLimitHandler>(relaxUnitFun = true)
private val gitHubAPIService = mockk<GitHubAPIService>()
private val gitHubEnterpriseService = GitHubEnterpriseService(gitHubClient, syncLogService)
private val gitHubScanningService = GitHubScanningService(
gitHubClient,
Expand All @@ -45,6 +46,7 @@ class GitHubScanningServiceTest {
syncLogService,
rateLimitHandler,
gitHubEnterpriseService,
gitHubAPIService
)
private val runId = UUID.randomUUID()

Expand All @@ -54,13 +56,13 @@ class GitHubScanningServiceTest {
@BeforeEach
fun setup() {
every { cachingService.get(any()) } returns "value"
every { gitHubClient.getInstallations(any()) } returns listOf(
every { gitHubAPIService.getPaginatedInstallations(any()) } returns listOf(
Installation(1, Account("testInstallation"), permissions, events)
)
every { gitHubClient.createInstallationToken(1, any()) } returns
InstallationTokenResponse("testToken", "2024-01-01T00:00:00Z", mapOf(), "all")
every { cachingService.set(any(), any(), any()) } returns Unit
every { gitHubClient.getOrganizations(any()) } returns listOf(Organization("testOrganization", 1))
every { gitHubAPIService.getPaginatedOrganizations(any()) } returns listOf(Organization("testOrganization", 1))
every { gitHubGraphQLService.getRepositories(any(), any()) } returns PagedRepositories(
repositories = emptyList(),
hasNextPage = false,
Expand Down Expand Up @@ -92,7 +94,7 @@ class GitHubScanningServiceTest {
val runId = UUID.randomUUID()

every { cachingService.get("runId") } returns runId
every { gitHubClient.getInstallations(any()) } returns emptyList()
every { gitHubAPIService.getPaginatedInstallations(any()) } returns emptyList()

gitHubScanningService.scanGitHubResources()

Expand Down Expand Up @@ -236,7 +238,7 @@ class GitHubScanningServiceTest {
@Test
fun `scanGitHubResources should skip organizations without correct permissions and events`() {
every { cachingService.get("runId") } returns runId
every { gitHubClient.getInstallations(any()) } returns listOf(
every { gitHubAPIService.getPaginatedInstallations(any()) } returns listOf(
Installation(1, Account("testInstallation1"), mapOf(), listOf()),
Installation(2, Account("testInstallation2"), permissions, events),
Installation(3, Account("testInstallation3"), permissions, events)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,14 @@ class WebhookEventServiceTest {
@MockkBean
private lateinit var gitHubAuthenticationService: GitHubAuthenticationService

@Autowired
private lateinit var webhookEventService: WebhookEventService

@MockkBean
private lateinit var gitHubClient: GitHubClient

@Autowired
private lateinit var webhookEventService: WebhookEventService
@MockkBean
private lateinit var gitHubAPIService: GitHubAPIService

private val permissions = mapOf("administration" to "read", "contents" to "read", "metadata" to "read")
private val events = listOf("label", "public", "repository", "push")
Expand All @@ -53,7 +56,7 @@ class WebhookEventServiceTest {
every { webSocketService.sendMessage(any(), any()) } returns Unit
every { cachingService.get(any()) } returns "token"
every { gitHubGraphQLService.getManifestFileContent(any(), any(), any(), any()) } returns "content"
every { gitHubClient.getInstallations(any()) } returns listOf(installation)
every { gitHubAPIService.getPaginatedInstallations(any()) } returns listOf(installation)
every { gitHubClient.getInstallation(any(), any()) } returns installation
}

Expand Down Expand Up @@ -379,7 +382,8 @@ class WebhookEventServiceTest {
every { cachingService.get("runId") } returnsMany listOf("value", null, runId)
every { cachingService.set("runId", any(), any()) } just runs
every { cachingService.remove("runId") } just runs
every { gitHubClient.getOrganizations(any()) } returns listOf(Organization("testOrganization", 1))
every { gitHubAPIService.getPaginatedOrganizations(any()) } returns
listOf(Organization("testOrganization", 1))

val eventType = "INSTALLATION"
val payload = """{
Expand Down
Loading