Skip to content

Commit

Permalink
feat: optimize github import
Browse files Browse the repository at this point in the history
  • Loading branch information
eyhn authored and forehalo committed Aug 29, 2022
1 parent 85da866 commit 2b34b85
Show file tree
Hide file tree
Showing 16 changed files with 444 additions and 111 deletions.
43 changes: 37 additions & 6 deletions packages/platform-server/src/modules/github/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,15 @@ class GithubInstallation {
account!: GithubAccount
}

@ObjectType()
export class GithubRepoVerificationResult {
@Field(() => Boolean)
ok!: boolean

@Field(() => String, { nullable: true })
error!: string | undefined
}

@ObjectType()
export class PaginatedGithubInstallations extends Paginated(GithubInstallation) {}

Expand All @@ -80,7 +89,7 @@ export class GithubIntegrationResolver {
})
async getGithubInstallations(
@CurrentUser() user: User,
@Args({ name: 'pagination', nullable: true, defaultValue: { first: 10 } })
@Args({ name: 'pagination', nullable: true, defaultValue: { first: 10, skip: 0 } })
paginationInput: PaginationInput,
) {
const githubAccount = await this.userService.getUserConnectedAccount(user, ExternalAccount.github)
Expand All @@ -91,23 +100,45 @@ export class GithubIntegrationResolver {
return this.service.getInstallationsByUser(paginationInput, githubAccount.accessToken)
}

@Query(() => GithubRepoVerificationResult, {
name: 'verifyGithubRepositoryPermission',
description:
'Verify that the github project exists and the current user has permissions to the project. Throws if user is not connected to github account.',
})
async verifyGithubRepositoryPermission(
@CurrentUser() user: User,
@Args({ name: 'owner', type: () => String }) owner: string,
@Args({ name: 'repo', type: () => String }) repo: string,
) {
return this.service.verifyGithubRepositoryPermission(user, owner, repo)
}

@Query(() => PaginatedGithubRepositories, {
name: 'githubInstallationRepositories',
name: 'githubSearchRepositories',
description:
'List all github repositories in the installation. Throws if user is not connected to github account. \n' +
'Search github repositories in the installation.\n' +
'Throws if user is not connected to github account.\n' +
'NOTE: Limited by github endpoint, pagination.skip must be a multiple of pagination.first for this function. pagination.after is not supported.',
})
async getGithubInstallationRepositories(
async searchGithubInstallationRepositories(
@CurrentUser() user: User,
@Args({ name: 'installationId', type: () => Int }) installationId: number,
@Args({ name: 'pagination', nullable: true, defaultValue: { first: 10 } })
@Args({ name: 'query', type: () => String }) query: string,
@Args({ name: 'pagination', nullable: true, defaultValue: { first: 10, skip: 0 } })
paginationInput: PaginationInput,
) {
const githubAccount = await this.userService.getUserConnectedAccount(user, ExternalAccount.github)
if (!githubAccount || !githubAccount.accessToken) {
throw new UserError('Please connect your github account first.')
}

return this.service.getUserInstallationRepositories(paginationInput, installationId, githubAccount.accessToken)
const installation = await this.service.getInstallationById(installationId)
const qualifier =
(installation.account.type === 'Organization'
? `org:${installation.account.login}`
: `user:${installation.account.login}`) + ' fork:true'
const installationToken = await this.service.getInstallationAccessToken(installationId)
const escapedQuery = '"' + query.split(/\s+/).join('" "') + '" ' + qualifier
return this.service.searchRepositories(escapedQuery, paginationInput, installationToken)
}
}
99 changes: 86 additions & 13 deletions packages/platform-server/src/modules/github/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@ import qs from 'query-string'
import { lastValueFrom } from 'rxjs'

import { Config } from '@perfsee/platform-server/config'
import { User } from '@perfsee/platform-server/db'
import { UserError } from '@perfsee/platform-server/error'
import { paginate, PaginationInput } from '@perfsee/platform-server/graphql'
import { RxFetch } from '@perfsee/platform-server/helpers'
import { Logger } from '@perfsee/platform-server/logger'
import { Redis } from '@perfsee/platform-server/redis'
import { ExternalAccount } from '@perfsee/shared'

import { UserService } from '../user'

import { githubAppJwt } from './github-app-jwt'
import {
Expand All @@ -47,6 +51,7 @@ export class GithubService {
available: boolean

constructor(
private readonly userService: UserService,
private readonly logger: Logger,
private readonly config: Config,
private readonly fetch: RxFetch,
Expand All @@ -63,7 +68,64 @@ export class GithubService {
}
}

async getUserInstallationRepositories(pagination: PaginationInput, installationId: number, userToken: string) {
/**
* Verify that the github project exists and the current user has permissions to the project.
*/
async verifyGithubRepositoryPermission(
user: User,
owner: string,
repo: string,
): Promise<{ ok: false; error: string } | { ok: true; caseSensitiveRepo: string; caseSensitiveOwner: string }> {
const githubAccount = await this.userService.getUserConnectedAccount(user, ExternalAccount.github)
if (!githubAccount || !githubAccount.accessToken) {
return {
ok: false,
error: `Please connect your github account first.`,
}
}

let repositoryInfo
try {
repositoryInfo = await this.getRepository(owner, repo, githubAccount.accessToken)
} catch (err) {
if (err instanceof GithubApiError && err.status === 404) {
return {
ok: false,
error: `Repository ${owner}/${repo} not found`,
}
}
throw err
}

const caseSensitiveRepo = repositoryInfo.name
const caseSensitiveOwner = repositoryInfo.owner.login

if (!(repositoryInfo.permissions.admin || repositoryInfo.permissions.maintain)) {
return {
ok: false,
error: `The github account ${githubAccount.externUsername} does not have maintain permission for the ${caseSensitiveOwner}/${caseSensitiveRepo}.`,
}
}

try {
await this.getInstallationByRepository(owner, repo)
} catch (err) {
if (err instanceof GithubApiError && err.status === 404) {
return {
ok: false,
error: `Can't find github app installation for ${caseSensitiveOwner}/${caseSensitiveRepo}.`,
}
}
throw err
}
return {
ok: true,
caseSensitiveRepo,
caseSensitiveOwner,
}
}

async searchRepositories(query: string, pagination: PaginationInput, installationAccessToken: string) {
if (pagination.after) {
throw new UserError('pagination.after is not supported for this function.')
}
Expand All @@ -76,19 +138,20 @@ export class GithubService {
const perPage = pagination.first
const result = await this.fetchApi<{
total_count: number
repositories: GithubRepository[]
items: GithubRepository[]
}>(
'GET',
`https://api.github.com/user/installations/${installationId}/repositories`,
`https://api.github.com/search/repositories`,
{
q: query,
page,
per_page: perPage,
},
userToken,
true,
installationAccessToken,
false,
)

return paginate(result.repositories, 'id', pagination, result.total_count)
return paginate(result.items, 'id', pagination, result.total_count)
}

updateCheckRun(
Expand Down Expand Up @@ -231,6 +294,14 @@ export class GithubService {
return data
}

async getRepository(owner: string, repo: string, userToken: string) {
return this.fetchApi<GithubRepository>('GET', `https://api.github.com/repos/${owner}/${repo}`, {}, userToken, true)
}

async getInstallationById(installationId: number) {
return this.fetchApi<GithubInstallation>('GET', `https://api.github.com/app/installations/${installationId}`)
}

getInstallUrl() {
return `https://github.com/apps/${this.config.github.appname}/installations/new`
}
Expand All @@ -253,13 +324,7 @@ export class GithubService {
method,
}),
).catch((res) => {
throw new Error(
`Fetch Github Api Error [${url}] \n\tbody: ${JSON.stringify(
body,
null,
2,
)} \n\tresponse: ${require('util').inspect(res)}`,
)
throw new GithubApiError(url, res)
}) as Promise<Response>
}

Expand All @@ -272,3 +337,11 @@ export class GithubService {
return this.cachedGithubAppJWT.token
}
}

export class GithubApiError extends Error {
readonly status: number
constructor(public readonly url: string, public readonly res: Response) {
super(`Fetch Github Api Error [${url}]\n\tstatus: ${res.status}`)
this.status = res.status
}
}
11 changes: 11 additions & 0 deletions packages/platform-server/src/modules/github/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface GithubInstallation {
account: {
login: string
avatar_url: string
type: 'Organization' | 'User'
}
permissions: GithubAppPermissions
}
Expand Down Expand Up @@ -77,4 +78,14 @@ export interface GithubRepository {
full_name: string
private: boolean
default_branch: string
owner: {
login: string
}
permissions: {
admin: boolean
maintain: boolean
push: boolean
triage: boolean
pull: boolean
}
}
3 changes: 2 additions & 1 deletion packages/platform-server/src/modules/project/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ import { Module } from '@nestjs/common'

import { DBModule } from '@perfsee/platform-server/db'

import { GithubModule } from '../github'
import { UserModule } from '../user'

import { ProjectResolver, UserProjectResolver } from './resolver'
import { ProjectService } from './service'

@Module({
imports: [DBModule, UserModule],
imports: [DBModule, UserModule, GithubModule],
providers: [ProjectService, ProjectResolver, UserProjectResolver],
exports: [ProjectService],
})
Expand Down
19 changes: 17 additions & 2 deletions packages/platform-server/src/modules/project/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { Injectable, NotFoundException } from '@nestjs/common'
import { differenceBy, isEmpty, isUndefined, omitBy } from 'lodash'
import { Brackets, In } from 'typeorm'

import { Config } from '@perfsee/platform-server/config'
import { Cron, CronExpression } from '@perfsee/platform-server/cron'
import {
DBService,
Expand All @@ -37,6 +38,7 @@ import { Metric } from '@perfsee/platform-server/metrics'
import { createDataLoader } from '@perfsee/platform-server/utils'
import { GitHost } from '@perfsee/shared'

import { GithubService } from '../github'
import { PermissionProvider, Permission } from '../permission'
import { UserService } from '../user'

Expand All @@ -52,10 +54,12 @@ export class ProjectService {

constructor(
private readonly permissionProvider: PermissionProvider,
private readonly github: GithubService,
private readonly db: DBService,
private readonly userService: UserService,
private readonly metricService: Metric,
private readonly internalIdService: InternalIdService,
private readonly config: Config,
) {}

async resolveRawProjectIdBySlug(slug: string) {
Expand Down Expand Up @@ -249,8 +253,19 @@ export class ProjectService {

const slugVerification = await this.verifyNewSlug(id)

if (slugVerification.error) {
throw new UserError(slugVerification.error)
if (!slugVerification.ok) {
throw new UserError('Invalid id, ' + slugVerification.error)
}

if (!this.config.testing && input.host === GitHost.Github) {
const githubVerification = await this.github.verifyGithubRepositoryPermission(user, input.namespace, input.name)

if (!githubVerification.ok) {
throw new UserError('GitHub permission verification error, ' + githubVerification.error)
}

input.namespace = githubVerification.caseSensitiveOwner
input.name = githubVerification.caseSensitiveRepo
}

const project = Project.create({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

import { Effect, EffectModule, ImmerReducer, Module } from '@sigi/core'
import { Draft } from 'immer'
import { endWith, exhaustMap, map, Observable, startWith, withLatestFrom } from 'rxjs'
import { endWith, exhaustMap, filter, map, Observable, startWith, withLatestFrom } from 'rxjs'

import { createErrorCatcher, GraphQLClient } from '@perfsee/platform/common'
import { githubInstallationsQuery, GithubInstallationsQuery } from '@perfsee/schema'
Expand Down Expand Up @@ -45,6 +45,9 @@ export class GithubInstallationModel extends EffectModule<State> {
loadMore(payload$: Observable<void>) {
return payload$.pipe(
withLatestFrom(this.state$),
filter(([_, state]) => {
return !(state.installations.length > 0 && state.installations.length === state.installationsTotalCount)
}),
exhaustMap(([_, state]) =>
this.client
.query({
Expand All @@ -54,7 +57,7 @@ export class GithubInstallationModel extends EffectModule<State> {
.pipe(
map((data) => {
return this.getActions().append({
repositories: data.githubInstallations.edges.map((edge) => edge.node),
installations: data.githubInstallations.edges.map((edge) => edge.node),
totalCount: data.githubInstallations.pageInfo.totalCount,
})
}),
Expand All @@ -72,8 +75,8 @@ export class GithubInstallationModel extends EffectModule<State> {
}

@ImmerReducer()
append(state: Draft<State>, { repositories, totalCount }: { repositories: Installation[]; totalCount: number }) {
state.installations.push(...repositories)
append(state: Draft<State>, { installations, totalCount }: { installations: Installation[]; totalCount: number }) {
state.installations.push(...installations)
state.installationsTotalCount = totalCount
}
}
Loading

0 comments on commit 2b34b85

Please sign in to comment.