diff --git a/e2e/e2e.spec.ts b/e2e/e2e.spec.ts index f15bf41..fdc1f96 100644 --- a/e2e/e2e.spec.ts +++ b/e2e/e2e.spec.ts @@ -729,7 +729,7 @@ describe("e2e tests", () => { }); fakeGitHub.mockUser({ - login: GitHubClient.GITHUB_ACTIONS_BOT.username, + login: GitHubClient.GITHUB_ACTIONS_BOT.login, id: GitHubClient.GITHUB_ACTIONS_BOT.id, }); fakeGitHub.mockUser({ login: "publish-to-bcr-bot[bot]", id: 123 }); diff --git a/src/application/notifications.ts b/src/application/notifications.ts index 74072d9..f09eda1 100644 --- a/src/application/notifications.ts +++ b/src/application/notifications.ts @@ -1,10 +1,9 @@ -import { Inject, Injectable } from "@nestjs/common"; +import { Injectable } from "@nestjs/common"; import { UserFacingError } from "../domain/error.js"; import { Maintainer } from "../domain/metadata-file.js"; import { Repository } from "../domain/repository.js"; -import { User } from "../domain/user.js"; +import { User, UserService } from "../domain/user.js"; import { Authentication, EmailClient } from "../infrastructure/email.js"; -import { GitHubClient } from "../infrastructure/github.js"; import { SecretsClient } from "../infrastructure/secrets.js"; @Injectable() @@ -16,7 +15,7 @@ export class NotificationsService { constructor( private readonly emailClient: EmailClient, private readonly secretsClient: SecretsClient, - @Inject("rulesetRepoGitHubClient") private githubClient: GitHubClient + private readonly userService: UserService ) { if (process.env.NOTIFICATIONS_EMAIL === undefined) { throw new Error("Missing NOTIFICATIONS_EMAIL environment variable."); @@ -70,7 +69,7 @@ export class NotificationsService { const fetchedEmails = ( await Promise.all( maintainersWithOnlyGithubHandle.map((m) => - this.githubClient.getRepoUser(m.github, rulesetRepo) + this.userService.getUser(m.github) ) ) ) diff --git a/src/application/release-event-handler.ts b/src/application/release-event-handler.ts index 3ba2458..81c2ee0 100644 --- a/src/application/release-event-handler.ts +++ b/src/application/release-event-handler.ts @@ -11,7 +11,7 @@ import { RulesetRepoError, RulesetRepository, } from "../domain/ruleset-repository.js"; -import { User } from "../domain/user.js"; +import { User, UserService } from "../domain/user.js"; import { GitHubClient } from "../infrastructure/github.js"; import { NotificationsService } from "./notifications.js"; @@ -24,9 +24,8 @@ interface PublishAttempt { @Injectable() export class ReleaseEventHandler { constructor( - @Inject("rulesetRepoGitHubClient") - private rulesetRepoGitHubClient: GitHubClient, @Inject("appOctokit") private appOctokit: Octokit, + private readonly userService: UserService, private readonly findRegistryForkService: FindRegistryForkService, private readonly createEntryService: CreateEntryService, private readonly publishEntryService: PublishEntryService, @@ -40,10 +39,7 @@ export class ReleaseEventHandler { process.env.BAZEL_CENTRAL_REGISTRY ); - let releaser = await this.rulesetRepoGitHubClient.getRepoUser( - event.payload.sender.login, - repository - ); + let releaser = await this.userService.getUser(event.payload.sender.login); const releaseUrl = event.payload.release.html_url; const tag = event.payload.release.tag_name; @@ -117,7 +113,8 @@ export class ReleaseEventHandler { for (let bcrFork of candidateBcrForks) { const bcrForkGitHubClient = await GitHubClient.forRepoInstallation( this.appOctokit, - bcrFork + bcrFork.owner, + bcrFork.name ); const attempt = await this.attemptPublish( @@ -275,9 +272,8 @@ export class ReleaseEventHandler { ); // Fetch the releaser to get their name - const fixedReleaser = await this.rulesetRepoGitHubClient.getRepoUser( - rulesetRepo.config.fixedReleaser.login, - rulesetRepo + const fixedReleaser = await this.userService.getUser( + rulesetRepo.config.fixedReleaser.login ); return { diff --git a/src/application/webhook/app.module.ts b/src/application/webhook/app.module.ts index 5107e70..d69b57f 100644 --- a/src/application/webhook/app.module.ts +++ b/src/application/webhook/app.module.ts @@ -2,6 +2,8 @@ import { Module } from "@nestjs/common"; import { CreateEntryService } from "../../domain/create-entry.js"; import { FindRegistryForkService } from "../../domain/find-registry-fork.js"; import { PublishEntryService } from "../../domain/publish-entry.js"; +import { RepositoryService } from "../../domain/repository.js"; +import { UserService } from "../../domain/user.js"; import { EmailClient } from "../../infrastructure/email.js"; import { GitClient } from "../../infrastructure/git.js"; import { SecretsClient } from "../../infrastructure/secrets.js"; @@ -24,6 +26,8 @@ import { CreateEntryService, FindRegistryForkService, PublishEntryService, + RepositoryService, + UserService, APP_OCTOKIT_PROVIDER, BCR_APP_OCTOKIT_PROVIDER, RULESET_REPO_GITHUB_CLIENT_PROVIDER, diff --git a/src/application/webhook/providers.ts b/src/application/webhook/providers.ts index e73e5ec..2963c6f 100644 --- a/src/application/webhook/providers.ts +++ b/src/application/webhook/providers.ts @@ -47,7 +47,8 @@ export const RULESET_REPO_GITHUB_CLIENT_PROVIDER: Provider = { const githubClient = await GitHubClient.forRepoInstallation( appOctokit, - rulesetRepo, + rulesetRepo.owner, + rulesetRepo.name, installationId ); return githubClient; @@ -67,7 +68,8 @@ export const BCR_GITHUB_CLIENT_PROVIDER: Provider = { const githubClient = await GitHubClient.forRepoInstallation( bcrAppOctokit, - bcr + bcr.owner, + bcr.name ); return githubClient; }, diff --git a/src/domain/create-entry.spec.ts b/src/domain/create-entry.spec.ts index a67621f..f56c258 100644 --- a/src/domain/create-entry.spec.ts +++ b/src/domain/create-entry.spec.ts @@ -6,7 +6,11 @@ import fs, { PathLike } from "node:fs"; import os from "node:os"; import path from "node:path"; import { GitClient } from "../infrastructure/git"; -import { GitHubApp, GitHubClient } from "../infrastructure/github"; +import { + GitHubApp, + GitHubClient, + User as GitHubUser, +} from "../infrastructure/github"; import { fakeMetadataFile, fakeModuleFile, @@ -26,7 +30,7 @@ import { ModuleFile } from "./module-file"; import { ReleaseArchive } from "./release-archive"; import { Repository } from "./repository"; import { RulesetRepository } from "./ruleset-repository"; -import { User } from "./user"; +import { User, UserService } from "./user"; let createEntryService: CreateEntryService; let mockGitClient: Mocked; @@ -942,16 +946,18 @@ describe("commitEntryToNewBranch", () => { const tag = "v1.2.3"; const rulesetRepo = await RulesetRepository.create("repo", "owner", tag); const bcrRepo = CANONICAL_BCR; - const releaser = GitHubClient.GITHUB_ACTIONS_BOT; - const botUser: User = { + const releaser = UserService.fromGitHubUser( + GitHubClient.GITHUB_ACTIONS_BOT + ); + const botUser: Partial = { name: "publish-to-bcr", - username: "publish-to-bcr[bot]", + login: "publish-to-bcr[bot]", email: `12345+"publish-to-bcr[bot]@users.noreply.github.com`, }; const botApp = { slug: "publish-to-bcr" } as GitHubApp; mockBcrGitHubClient.getApp.mockResolvedValue(botApp); - mockBcrGitHubClient.getBotAppUser.mockResolvedValue(botUser); + mockBcrGitHubClient.getBotAppUser.mockResolvedValue(botUser as GitHubUser); await createEntryService.commitEntryToNewBranch( rulesetRepo, @@ -1105,7 +1111,7 @@ describe("pushEntryToFork", () => { ); expect( mockBcrForkGitHubClient.getAuthenticatedRemoteUrl - ).toHaveBeenCalledWith(bcrForkRepo); + ).toHaveBeenCalledWith(bcrForkRepo.owner, bcrForkRepo.name); }); test("adds a remote with the authenticated url for the fork to the local bcr repo", async () => { diff --git a/src/domain/create-entry.ts b/src/domain/create-entry.ts index dc0f7a1..1481655 100644 --- a/src/domain/create-entry.ts +++ b/src/domain/create-entry.ts @@ -17,7 +17,7 @@ import { ReleaseArchive } from "./release-archive.js"; import { Repository } from "./repository.js"; import { RulesetRepository } from "./ruleset-repository.js"; import { SourceTemplate } from "./source-template.js"; -import { User } from "./user.js"; +import { User, UserService } from "./user.js"; export class VersionAlreadyPublishedError extends UserFacingError { public constructor(version: string) { @@ -44,7 +44,10 @@ export class CreateEntryService { tag: string, moduleRoot: string ): Promise<{ moduleName: string }> { - await Promise.all([rulesetRepo.shallowCloneAndCheckout(tag), bcrRepo.shallowCloneAndCheckout("main")]); + await Promise.all([ + rulesetRepo.shallowCloneAndCheckout(tag), + bcrRepo.shallowCloneAndCheckout("main"), + ]); const version = RulesetRepository.getVersionFromTag(tag); @@ -123,7 +126,7 @@ export class CreateEntryService { const branchName = `${repoAndVersion}-${randomBytes(4).toString("hex")}`; let commitAuthor: Partial = releaser; - if (releaser.username === GitHubClient.GITHUB_ACTIONS_BOT.username) { + if (UserService.isGitHubActionsBot(releaser)) { const botApp = await this.bcrGitHubClient.getApp(); const botAppUser = await this.bcrGitHubClient.getBotAppUser(botApp); @@ -157,7 +160,8 @@ export class CreateEntryService { githubClient: GitHubClient ): Promise { const authenticatedRemoteUrl = await githubClient.getAuthenticatedRemoteUrl( - bcrForkRepo + bcrForkRepo.owner, + bcrForkRepo.name ); if (!(await this.gitClient.hasRemote(bcr.diskPath, "authed-fork"))) { diff --git a/src/domain/find-registry-fork.spec.ts b/src/domain/find-registry-fork.spec.ts index c5d861a..21f17a6 100644 --- a/src/domain/find-registry-fork.spec.ts +++ b/src/domain/find-registry-fork.spec.ts @@ -1,18 +1,26 @@ import { mocked, Mocked } from "jest-mock"; -import { GitHubClient } from "../infrastructure/github"; import { expectThrownError } from "../test/util"; import { CANONICAL_BCR, FindRegistryForkService, NoCandidateForksError, } from "./find-registry-fork"; -import { Repository } from "./repository"; +import { Repository, RepositoryService } from "./repository"; import { RulesetRepository } from "./ruleset-repository"; -jest.mock("../infrastructure/github"); +jest.mock("./repository", () => ({ + Repository: jest.requireActual("./repository").Repository, + RepositoryService: jest.fn().mockImplementation(() => { + return { + getSourceRepository: jest.fn(), + getForkedRepositoriesByOwner: jest.fn(), + hasAppInstallation: jest.fn(), + }; + }), +})); let findRegistryForkService: FindRegistryForkService; -let mockGithubClient: Mocked; +let mockRepositoryService: Mocked; // Mock RulesetRepostory.create to avoid network call and necessary file checks const mockRulesetRepoCreate = jest @@ -22,11 +30,11 @@ const mockRulesetRepoCreate = jest }); beforeEach(() => { - mocked(GitHubClient).mockClear(); + mocked(RepositoryService).mockClear(); mockRulesetRepoCreate.mockClear(); - mockGithubClient = mocked(new GitHubClient({} as any)); - findRegistryForkService = new FindRegistryForkService(mockGithubClient); + mockRepositoryService = mocked(new RepositoryService({} as any)); + findRegistryForkService = new FindRegistryForkService(mockRepositoryService); }); describe("findCandidateForks", () => { @@ -44,7 +52,7 @@ describe("findCandidateForks", () => { ); const ownerBcrFork = new Repository("bazel-central-registry", owner); - mockGithubClient.getForkedRepositoriesByOwner.mockImplementation( + mockRepositoryService.getForkedRepositoriesByOwner.mockImplementation( async (repoOwner) => { if (repoOwner === owner) { return [new Repository("a", owner), ownerBcrFork]; @@ -53,7 +61,7 @@ describe("findCandidateForks", () => { } ); - mockGithubClient.getSourceRepository.mockImplementation( + mockRepositoryService.getSourceRepository.mockImplementation( async (repository) => { if (repository.equals(ownerBcrFork)) { return CANONICAL_BCR; @@ -62,8 +70,8 @@ describe("findCandidateForks", () => { } ); - mockGithubClient.hasAppInstallation.mockImplementation(async (repository) => - repository.equals(ownerBcrFork) + mockRepositoryService.hasAppInstallation.mockImplementation( + async (repository) => repository.equals(ownerBcrFork) ); const forks = await findRegistryForkService.findCandidateForks( @@ -91,7 +99,7 @@ describe("findCandidateForks", () => { releaser.username ); - mockGithubClient.getForkedRepositoriesByOwner.mockImplementation( + mockRepositoryService.getForkedRepositoriesByOwner.mockImplementation( async (repoOwner) => { if (repoOwner === releaser.username) { return [new Repository("a", releaser.username), releaserBcrFork]; @@ -100,7 +108,7 @@ describe("findCandidateForks", () => { } ); - mockGithubClient.getSourceRepository.mockImplementation( + mockRepositoryService.getSourceRepository.mockImplementation( async (repository) => { if (repository.equals(releaserBcrFork)) { return CANONICAL_BCR; @@ -109,8 +117,8 @@ describe("findCandidateForks", () => { } ); - mockGithubClient.hasAppInstallation.mockImplementation(async (repository) => - repository.equals(releaserBcrFork) + mockRepositoryService.hasAppInstallation.mockImplementation( + async (repository) => repository.equals(releaserBcrFork) ); const forks = await findRegistryForkService.findCandidateForks( @@ -139,7 +147,7 @@ describe("findCandidateForks", () => { releaser.username ); - mockGithubClient.getForkedRepositoriesByOwner.mockImplementation( + mockRepositoryService.getForkedRepositoriesByOwner.mockImplementation( async (repoOwner) => { if (repoOwner === owner) { return [new Repository("a", owner), ownerBcrFork]; @@ -150,7 +158,7 @@ describe("findCandidateForks", () => { } ); - mockGithubClient.getSourceRepository.mockImplementation( + mockRepositoryService.getSourceRepository.mockImplementation( async (repository) => { if ( repository.equals(releaserBcrFork) || @@ -162,7 +170,7 @@ describe("findCandidateForks", () => { } ); - mockGithubClient.hasAppInstallation.mockImplementation( + mockRepositoryService.hasAppInstallation.mockImplementation( async (repository) => repository.equals(ownerBcrFork) || repository.equals(releaserBcrFork) ); @@ -191,7 +199,7 @@ describe("findCandidateForks", () => { ); const ownerBcrFork = new Repository("bazel-central-registry", owner); - mockGithubClient.getForkedRepositoriesByOwner.mockImplementation( + mockRepositoryService.getForkedRepositoriesByOwner.mockImplementation( async (repoOwner) => { if (repoOwner === owner) { return [new Repository("a", owner), ownerBcrFork]; @@ -200,7 +208,7 @@ describe("findCandidateForks", () => { } ); - mockGithubClient.getSourceRepository.mockImplementation( + mockRepositoryService.getSourceRepository.mockImplementation( async (repository) => { if (repository.equals(ownerBcrFork)) { return new Repository("bazel-central-registry", "not-google"); @@ -232,7 +240,7 @@ describe("findCandidateForks", () => { releaser.username ); - mockGithubClient.getForkedRepositoriesByOwner.mockImplementation( + mockRepositoryService.getForkedRepositoriesByOwner.mockImplementation( async (repoOwner) => { if (repoOwner === releaser.username) { return [new Repository("a", releaser.username), releaserBcrFork]; @@ -241,7 +249,7 @@ describe("findCandidateForks", () => { } ); - mockGithubClient.getSourceRepository.mockImplementation( + mockRepositoryService.getSourceRepository.mockImplementation( async (repository) => { if (repository.equals(releaserBcrFork)) { return CANONICAL_BCR; @@ -250,7 +258,9 @@ describe("findCandidateForks", () => { } ); - mockGithubClient.hasAppInstallation.mockReturnValue(Promise.resolve(false)); + mockRepositoryService.hasAppInstallation.mockReturnValue( + Promise.resolve(false) + ); await expectThrownError( () => findRegistryForkService.findCandidateForks(rulesetRepo, releaser), diff --git a/src/domain/find-registry-fork.ts b/src/domain/find-registry-fork.ts index 9e7ed47..44ee887 100644 --- a/src/domain/find-registry-fork.ts +++ b/src/domain/find-registry-fork.ts @@ -1,7 +1,6 @@ -import { Inject, Injectable } from "@nestjs/common"; -import { GitHubClient } from "../infrastructure/github.js"; +import { Injectable } from "@nestjs/common"; import { UserFacingError } from "./error.js"; -import { Repository } from "./repository.js"; +import { Repository, RepositoryService } from "./repository.js"; import { RulesetRepository } from "./ruleset-repository.js"; import { User } from "./user.js"; @@ -23,9 +22,7 @@ Install the app here: https://github.com/apps/publish-to-bcr.`); @Injectable() export class FindRegistryForkService { - constructor( - @Inject("rulesetRepoGitHubClient") private githubClient: GitHubClient - ) {} + constructor(private repositoryService: RepositoryService) {} // Find potential bcr forks that can be pushed to. Will return a fork // owned by the ruleset owner, followed by a fork owned by the releaser, @@ -41,7 +38,7 @@ export class FindRegistryForkService { const allForks = ( await Promise.all( Array.from(potentialForkOwners.values()).map((owner) => - this.githubClient.getForkedRepositoriesByOwner(owner) + this.repositoryService.getForkedRepositoriesByOwner(owner) ) ) ).reduce((acc, curr) => acc.concat(curr), []); @@ -52,7 +49,9 @@ export class FindRegistryForkService { // Only consider forks named `bazel-central-registry` const sourceRepos = await Promise.all( - candidateForks.map((fork) => this.githubClient.getSourceRepository(fork)) + candidateForks.map((fork) => + this.repositoryService.getSourceRepository(fork) + ) ); candidateForks = candidateForks.filter((_, index) => sourceRepos[index].equals(CANONICAL_BCR) @@ -60,7 +59,9 @@ export class FindRegistryForkService { // Filter out BCR forks that don't have the app installed const appInstalledToFork = await Promise.all( - candidateForks.map((fork) => this.githubClient.hasAppInstallation(fork)) + candidateForks.map((fork) => + this.repositoryService.hasAppInstallation(fork) + ) ); candidateForks = candidateForks.filter( (_, index) => appInstalledToFork[index] diff --git a/src/domain/publish-entry.spec.ts b/src/domain/publish-entry.spec.ts index 32d27cd..ee25e59 100644 --- a/src/domain/publish-entry.spec.ts +++ b/src/domain/publish-entry.spec.ts @@ -30,9 +30,10 @@ describe("publish", () => { ); expect(mockGithubClient.createPullRequest).toHaveBeenCalledWith( - bcrFork, + bcrFork.owner, branch, - bcr, + bcr.owner, + bcr.name, "main", expect.any(String), expect.any(String) @@ -55,17 +56,19 @@ describe("publish", () => { ); expect(mockGithubClient.createPullRequest).toHaveBeenCalledWith( - expect.any(Repository), expect.any(String), - expect.any(Repository), + expect.any(String), + expect.any(String), + expect.any(String), expect.any(String), expect.stringContaining("rules_foo"), expect.any(String) ); expect(mockGithubClient.createPullRequest).toHaveBeenCalledWith( - expect.any(Repository), expect.any(String), - expect.any(Repository), + expect.any(String), + expect.any(String), + expect.any(String), expect.any(String), expect.stringContaining("1.0.0"), expect.any(String) @@ -88,25 +91,28 @@ describe("publish", () => { ); expect(mockGithubClient.createPullRequest).toHaveBeenCalledWith( - expect.any(Repository), expect.any(String), - expect.any(Repository), + expect.any(String), + expect.any(String), + expect.any(String), expect.any(String), expect.stringContaining("rules_foo"), expect.any(String) ); expect(mockGithubClient.createPullRequest).toHaveBeenCalledWith( - expect.any(Repository), expect.any(String), - expect.any(Repository), + expect.any(String), + expect.any(String), + expect.any(String), expect.any(String), expect.stringContaining("rules_bar"), expect.any(String) ); expect(mockGithubClient.createPullRequest).toHaveBeenCalledWith( - expect.any(Repository), expect.any(String), - expect.any(Repository), + expect.any(String), + expect.any(String), + expect.any(String), expect.any(String), expect.stringContaining("1.0.0"), expect.any(String) @@ -129,9 +135,10 @@ describe("publish", () => { ); expect(mockGithubClient.createPullRequest).toHaveBeenCalledWith( - expect.any(Repository), expect.any(String), - expect.any(Repository), + expect.any(String), + expect.any(String), + expect.any(String), expect.any(String), expect.any(String), expect.stringContaining( diff --git a/src/domain/publish-entry.ts b/src/domain/publish-entry.ts index 2477287..0e20b12 100644 --- a/src/domain/publish-entry.ts +++ b/src/domain/publish-entry.ts @@ -18,9 +18,10 @@ export class PublishEntryService { const version = RulesetRepository.getVersionFromTag(tag); const pullNumber = await this.githubClient.createPullRequest( - bcrForkRepo, + bcrForkRepo.owner, branch, - bcr, + bcr.owner, + bcr.name, "main", moduleNames.map((moduleName) => `${moduleName}@${version}`).join(", "), `\ @@ -30,9 +31,11 @@ _Automated by [Publish to BCR](https://github.com/apps/publish-to-bcr)_` ); try { - await this.githubClient.enableAutoMerge(bcr, pullNumber); + await this.githubClient.enableAutoMerge(bcr.owner, bcr.name, pullNumber); } catch (e) { - console.error(`Error: Failed to enable auto-merge on pull request github.com/${bcr.canonicalName}/pull/${pullNumber}.`) + console.error( + `Error: Failed to enable auto-merge on pull request github.com/${bcr.canonicalName}/pull/${pullNumber}.` + ); } return pullNumber; diff --git a/src/domain/repository.ts b/src/domain/repository.ts index eae0eda..ba45660 100644 --- a/src/domain/repository.ts +++ b/src/domain/repository.ts @@ -1,7 +1,9 @@ +import { Inject, Injectable } from "@nestjs/common"; import { randomUUID } from "node:crypto"; import os from "node:os"; import path from "node:path"; import { GitClient } from "../infrastructure/git.js"; +import { GitHubClient } from "../infrastructure/github.js"; export class Repository { private _diskPath: string | null = null; @@ -50,3 +52,40 @@ export class Repository { return this.name == other.name && this.owner === other.owner; } } + +@Injectable() +export class RepositoryService { + constructor( + @Inject("rulesetRepoGitHubClient") private githubClient: GitHubClient + ) {} + + public async getSourceRepository( + repository: Repository + ): Promise { + const repo = await this.githubClient.getRepository( + repository.owner, + repository.name + ); + if (repo.source) { + return new Repository(repo.source.name, repo.source.owner.login); + } + return null; + } + + public async getForkedRepositoriesByOwner( + owner: string + ): Promise { + const repositories = await this.githubClient.listRepositoriesForUser(owner); + + return repositories + .filter((repo) => repo.fork) + .map((repo) => new Repository(repo.name, repo.owner.login)); + } + + public async hasAppInstallation(repository: Repository): Promise { + return this.githubClient.hasAppInstallation( + repository.owner, + repository.name + ); + } +} diff --git a/src/domain/user.ts b/src/domain/user.ts index 9d7cf5b..b481475 100644 --- a/src/domain/user.ts +++ b/src/domain/user.ts @@ -1,6 +1,34 @@ +import { Inject, Injectable } from "@nestjs/common"; +import { GitHubClient, User as GitHubUser } from "../infrastructure/github.js"; + export interface User { readonly id?: number; readonly name?: string; readonly username: string; readonly email: string; } + +@Injectable() +export class UserService { + constructor( + @Inject("rulesetRepoGitHubClient") private githubClient: GitHubClient + ) {} + + public static isGitHubActionsBot(user: User): boolean { + return user.username === GitHubClient.GITHUB_ACTIONS_BOT.login; + } + + public static fromGitHubUser(user: GitHubUser): User { + return { + id: user.id, + name: user.name, + username: user.login, + email: user.email, + }; + } + + public async getUser(username: string): Promise { + const user = await this.githubClient.getUserByUsername(username); + return UserService.fromGitHubUser(user); + } +} diff --git a/src/infrastructure/github.ts b/src/infrastructure/github.ts index 59455ad..6d2e994 100644 --- a/src/infrastructure/github.ts +++ b/src/infrastructure/github.ts @@ -1,16 +1,17 @@ import { createAppAuth } from "@octokit/auth-app"; import { Octokit } from "@octokit/rest"; import { Endpoints } from "@octokit/types"; -import { Repository } from "../domain/repository.js"; -import { User } from "../domain/user.js"; export type Installation = Endpoints["GET /repos/{owner}/{repo}/installation"]["response"]["data"]; export type GitHubApp = Endpoints["GET /app"]["response"]["data"]; +export type User = Endpoints["GET /users/{username}"]["response"]["data"]; +export type Repository = + Endpoints["GET /repos/{owner}/{repo}"]["response"]["data"]; export class MissingRepositoryInstallationError extends Error { - constructor(repository: Repository) { - super(`Missing installation for repository ${repository.canonicalName}`); + constructor(owner: string, repo: string) { + super(`Missing installation for repository ${owner}/${repo}`); } } @@ -74,22 +75,24 @@ export class GitHubClient { // commit. Hardcode the (stable) name and email so that we can also author commits // as the GitHub actions bot. // See https://github.com/orgs/community/discussions/26560#discussioncomment-3252340. - public static readonly GITHUB_ACTIONS_BOT: User = { + public static readonly GITHUB_ACTIONS_BOT: Readonly = { id: 41898282, - username: "github-actions[bot]", + login: "github-actions[bot]", name: "github-actions[bot]", email: "41898282+github-actions[bot]@users.noreply.github.com", - }; + } as User; public static async forRepoInstallation( appOctokit: Octokit, - repository: Repository, + owner: string, + repo: string, installationId?: number ): Promise { if (installationId === undefined) { const appClient = new GitHubClient(appOctokit); const installation = await appClient.getRepositoryInstallation( - repository + owner, + repo ); installationId = installation.id; } @@ -97,7 +100,7 @@ export class GitHubClient { const installationOctokit = await getInstallationAuthorizedOctokit( appOctokit, installationId, - repository.name + repo ); const client = new GitHubClient(installationOctokit); @@ -106,50 +109,41 @@ export class GitHubClient { public constructor(private readonly octokit: Octokit) {} - public async getForkedRepositoriesByOwner( - owner: string - ): Promise { + public async listRepositoriesForUser(owner: string): Promise { // This endpoint works for org owners as well as user owners - const response = await this.octokit.rest.repos.listForUser({ + const { data: repositories } = await this.octokit.rest.repos.listForUser({ username: owner, type: "owner", per_page: 100, }); - return response.data - .filter((repo) => repo.fork) - .map((repo) => new Repository(repo.name, repo.owner.login)); + return repositories.map((repo) => repo as Repository); } - public async getSourceRepository( - repository: Repository - ): Promise { - const response = await this.octokit.rest.repos.get({ - owner: repository.owner, - repo: repository.name, + public async getRepository(owner: string, repo: string): Promise { + const { data: repository } = await this.octokit.rest.repos.get({ + owner, + repo, }); - const repo = response.data; - if (repo.source) { - return new Repository(repo.source.name, repo.source.owner.login); - } - return null; + return repository; } public async createPullRequest( - fromRepo: Repository, + fromOwner: string, fromBranch: string, - toRepo: Repository, + toOwner: string, + toRepo: string, toBranch: string, title: string, body: string ): Promise { const { data: pull } = await this.octokit.rest.pulls.create({ - owner: toRepo.owner, - repo: toRepo.name, + owner: toOwner, + repo: toRepo, title: title, body, - head: `${fromRepo.owner}:${fromBranch}`, + head: `${fromOwner}:${fromBranch}`, base: toBranch, maintainer_can_modify: false, }); @@ -157,29 +151,35 @@ export class GitHubClient { return pull.number; } - public async enableAutoMerge(repo: Repository, pullNumber: number) { + public async enableAutoMerge( + owner: string, + repo: string, + pullNumber: number + ) { await this.octokit.rest.pulls.update({ - owner: repo.owner, - repo: repo.name, + owner, + repo, pull_number: pullNumber, allow_auto_merge: true, }); } - public async getRepoUser( - username: string, - repository: Repository - ): Promise { - if (username === GitHubClient.GITHUB_ACTIONS_BOT.username) { + public async getUserByUsername(username: string): Promise { + if (username === GitHubClient.GITHUB_ACTIONS_BOT.login) { return GitHubClient.GITHUB_ACTIONS_BOT; } - const { data } = await this.octokit.rest.users.getByUsername({ username }); - return { name: data.name, username, email: data.email }; + const { data: user } = await this.octokit.rest.users.getByUsername({ + username, + }); + return user; } - public async hasAppInstallation(repository: Repository): Promise { + public async hasAppInstallation( + owner: string, + repo: string + ): Promise { try { - await this.getRepositoryInstallation(repository); + await this.getRepositoryInstallation(owner, repo); return true; } catch (error) { if (error instanceof MissingRepositoryInstallationError) { @@ -190,43 +190,48 @@ export class GitHubClient { } public async getRepositoryInstallation( - repository: Repository + owner: string, + repo: string ): Promise { try { const { data: installation } = await this.octokit.rest.apps.getRepoInstallation({ - owner: repository.owner, - repo: repository.name, + owner, + repo, }); return installation; } catch (error) { if (error.status === 404) { - throw new MissingRepositoryInstallationError(repository); + throw new MissingRepositoryInstallationError(owner, repo); } throw new Error( - `Could not access app installation for repo ${repository.canonicalName}; returned status ${status}` + `Could not access app installation for repo ${owner}/${repo}; returned status ${status}` ); } } - public async getInstallationToken(repository: Repository): Promise { - const installationId = (await this.getRepositoryInstallation(repository)) + public async getInstallationToken( + owner: string, + repo: string + ): Promise { + const installationId = (await this.getRepositoryInstallation(owner, repo)) .id; const auth = (await this.octokit.auth({ type: "installation", installationId: installationId, - repositoryNames: [repository.name], + repositoryNames: [repo], })) as any; return auth.token; } public async getAuthenticatedRemoteUrl( - repository: Repository + owner: string, + repo: string ): Promise { - const token = await this.getInstallationToken(repository); - return `https://x-access-token:${token}@github.com/${repository.canonicalName}.git`; + const token = await this.getInstallationToken(owner, repo); + return `https://x-access-token:${token}@github.com/${owner}/${repo}.git`; } public async getApp(): Promise { @@ -248,8 +253,9 @@ export class GitHubClient { }); return { + ...user, name: botApp.slug, - username: botUsername, + login: botUsername, email: `${user.id}+${botUsername}@users.noreply.github.com`, }; }