From 3c8cf0bd59d0334c5be3dc343a4015263e7a56ba Mon Sep 17 00:00:00 2001 From: Derek Cormier Date: Sun, 16 Jun 2024 20:21:54 -0700 Subject: [PATCH] refactor: use a dependency injection framework --- package.json | 6 +- src/application/notifications.ts | 5 +- src/application/release-event-handler.ts | 106 +++++--------- src/application/webhook/app.module.ts | 33 +++++ .../{cloudfunction => webhook}/index.ts | 2 +- .../main.ts} | 23 ++- src/application/webhook/providers.ts | 75 ++++++++++ src/domain/create-entry.spec.ts | 64 +++++++-- src/domain/create-entry.ts | 17 ++- src/domain/find-registry-fork.ts | 6 +- src/domain/publish-entry.ts | 4 +- src/domain/repository.spec.ts | 14 +- src/domain/repository.ts | 7 +- src/domain/ruleset-repository.spec.ts | 131 +++++++++--------- src/infrastructure/email.ts | 2 + src/infrastructure/git.ts | 2 + src/infrastructure/github.ts | 4 +- src/infrastructure/secrets.ts | 2 + tsconfig.json | 2 + yarn.lock | 78 ++++++++++- 20 files changed, 404 insertions(+), 179 deletions(-) create mode 100644 src/application/webhook/app.module.ts rename src/application/{cloudfunction => webhook}/index.ts (75%) rename src/application/{cloudfunction/github-webhook-entrypoint.ts => webhook/main.ts} (57%) create mode 100644 src/application/webhook/providers.ts diff --git a/package.json b/package.json index 2551c59..37b605a 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "publish-to-bcr", "private": true, "type": "module", - "main": "./application/cloudfunction/index.js", + "main": "./application/webhook/index.js", "engines": { "node": "^18" }, @@ -16,6 +16,8 @@ "dependencies": { "@google-cloud/functions-framework": "^3.1.2", "@google-cloud/secret-manager": "^5.0.1", + "@nestjs/common": "^10.3.9", + "@nestjs/core": "^10.3.9", "@octokit/auth-app": "^4.0.4", "@octokit/core": "^4.0.4", "@octokit/rest": "^19.0.3", @@ -29,6 +31,8 @@ "extract-zip": "^2.0.1", "gcp-metadata": "^6.0.0", "nodemailer": "^6.7.8", + "reflect-metadata": "^0.2.2", + "rxjs": "7.8.1", "simple-git": "^3.16.0", "source-map-support": "^0.5.21", "tar": "^6.2.0", diff --git a/src/application/notifications.ts b/src/application/notifications.ts index eb589d6..ee54a97 100644 --- a/src/application/notifications.ts +++ b/src/application/notifications.ts @@ -1,3 +1,4 @@ +import { Inject, Injectable } from "@nestjs/common"; import { UserFacingError } from "../domain/error.js"; import { Maintainer } from "../domain/metadata-file.js"; import { Repository } from "../domain/repository.js"; @@ -6,14 +7,16 @@ import { Authentication, EmailClient } from "../infrastructure/email.js"; import { GitHubClient } from "../infrastructure/github.js"; import { SecretsClient } from "../infrastructure/secrets.js"; +@Injectable() export class NotificationsService { private readonly sender: string; private readonly debugEmail?: string; private emailAuth: Authentication; + constructor( private readonly emailClient: EmailClient, private readonly secretsClient: SecretsClient, - private readonly githubClient: GitHubClient + @Inject("rulesetRepoGitHubClient") private githubClient: GitHubClient ) { if (process.env.NOTIFICATIONS_EMAIL === undefined) { throw new Error("Missing NOTIFICATIONS_EMAIL environment variable."); diff --git a/src/application/release-event-handler.ts b/src/application/release-event-handler.ts index e24bce4..f6b8a02 100644 --- a/src/application/release-event-handler.ts +++ b/src/application/release-event-handler.ts @@ -1,3 +1,5 @@ +import { Inject, Injectable } from "@nestjs/common"; +import { Octokit } from "@octokit/rest"; import { ReleasePublishedEvent } from "@octokit/webhooks-types"; import { HandlerFunction } from "@octokit/webhooks/dist-types/types"; import { CreateEntryService } from "../domain/create-entry.js"; @@ -10,15 +12,8 @@ import { RulesetRepository, } from "../domain/ruleset-repository.js"; import { User } from "../domain/user.js"; -import { EmailClient } from "../infrastructure/email.js"; -import { GitClient } from "../infrastructure/git.js"; import { GitHubClient } from "../infrastructure/github.js"; -import { SecretsClient } from "../infrastructure/secrets.js"; import { NotificationsService } from "./notifications.js"; -import { - createAppAuthorizedOctokit, - createBotAppAuthorizedOctokit, -} from "./octokit.js"; interface PublishAttempt { readonly successful: boolean; @@ -26,8 +21,17 @@ interface PublishAttempt { readonly error?: Error; } +@Injectable() export class ReleaseEventHandler { - constructor(private readonly secretsClient: SecretsClient) {} + constructor( + @Inject("rulesetRepoGitHubClient") + private rulesetRepoGitHubClient: GitHubClient, + @Inject("appOctokit") private appOctokit: Octokit, + private readonly findRegistryForkService: FindRegistryForkService, + private readonly createEntryService: CreateEntryService, + private readonly publishEntryService: PublishEntryService, + private readonly notificationsService: NotificationsService + ) {} public readonly handle: HandlerFunction<"release.published", unknown> = async (event) => { @@ -36,41 +40,8 @@ export class ReleaseEventHandler { process.env.BAZEL_CENTRAL_REGISTRY ); - // The "app" refers to the public facing GitHub app installed to users' - // ruleset repos and BCR Forks that creates and pushes the entry to the - // fork. The "bot app" refers to the private app only installed to the - // canonical BCR which has reduced permissions and only opens PRs. - const appOctokit = await createAppAuthorizedOctokit(this.secretsClient); - const rulesetGitHubClient = await GitHubClient.forRepoInstallation( - appOctokit, - repository, - event.payload.installation.id - ); - - const botAppOctokit = await createBotAppAuthorizedOctokit( - this.secretsClient - ); - const bcrGitHubClient = await GitHubClient.forRepoInstallation( - botAppOctokit, - bcr - ); - - const gitClient = new GitClient(); - Repository.gitClient = gitClient; - - const emailClient = new EmailClient(); - const findRegistryForkService = new FindRegistryForkService( - rulesetGitHubClient - ); - const publishEntryService = new PublishEntryService(bcrGitHubClient); - const notificationsService = new NotificationsService( - emailClient, - this.secretsClient, - rulesetGitHubClient - ); - const repoCanonicalName = `${event.payload.repository.owner.login}/${event.payload.repository.name}`; - let releaser = await rulesetGitHubClient.getRepoUser( + let releaser = await this.rulesetRepoGitHubClient.getRepoUser( event.payload.sender.login, repository ); @@ -82,8 +53,7 @@ export class ReleaseEventHandler { const createRepoResult = await this.validateRulesetRepoOrNotifyFailure( repository, tag, - releaser, - notificationsService + releaser ); if (!createRepoResult.successful) { return; @@ -93,18 +63,14 @@ export class ReleaseEventHandler { console.log(`Release author: ${releaser.username}`); - releaser = await this.overrideReleaser( - releaser, - rulesetRepo, - rulesetGitHubClient - ); + releaser = await this.overrideReleaser(releaser, rulesetRepo); console.log( `Release published: ${rulesetRepo.canonicalName}@${tag} by @${releaser.username}` ); const candidateBcrForks = - await findRegistryForkService.findCandidateForks( + await this.findRegistryForkService.findCandidateForks( rulesetRepo, releaser ); @@ -122,14 +88,9 @@ export class ReleaseEventHandler { for (let bcrFork of candidateBcrForks) { const forkGitHubClient = await GitHubClient.forRepoInstallation( - appOctokit, + this.appOctokit, bcrFork ); - const createEntryService = new CreateEntryService( - gitClient, - forkGitHubClient, - bcrGitHubClient - ); const attempt = await this.attemptPublish( rulesetRepo, @@ -139,8 +100,7 @@ export class ReleaseEventHandler { moduleRoot, releaser, releaseUrl, - createEntryService, - publishEntryService + forkGitHubClient ); attempts.push(attempt); @@ -152,7 +112,7 @@ export class ReleaseEventHandler { // Send out error notifications if none of the attempts succeeded if (!attempts.some((a) => a.successful)) { - await notificationsService.notifyError( + await this.notificationsService.notifyError( releaser, rulesetRepo.metadataTemplate(moduleRoot).maintainers, rulesetRepo, @@ -165,7 +125,7 @@ export class ReleaseEventHandler { // Handle any other unexpected errors console.log(error); - await notificationsService.notifyError( + await this.notificationsService.notifyError( releaser, [], Repository.fromCanonicalName(repoCanonicalName), @@ -180,8 +140,7 @@ export class ReleaseEventHandler { private async validateRulesetRepoOrNotifyFailure( repository: Repository, tag: string, - releaser: User, - notificationsService: NotificationsService + releaser: User ): Promise<{ rulesetRepo?: RulesetRepository; successful: boolean }> { try { const rulesetRepo = await RulesetRepository.create( @@ -217,7 +176,7 @@ export class ReleaseEventHandler { ); } - await notificationsService.notifyError( + await this.notificationsService.notifyError( releaser, maintainers, repository, @@ -240,32 +199,36 @@ export class ReleaseEventHandler { moduleRoot: string, releaser: User, releaseUrl: string, - createEntryService: CreateEntryService, - publishEntryService: PublishEntryService + bcrForkGitHubClient: GitHubClient ): Promise { console.log(`Attempting publish to fork ${bcrFork.canonicalName}.`); try { - const {moduleName} = await createEntryService.createEntryFiles( + const { moduleName } = await this.createEntryService.createEntryFiles( rulesetRepo, bcr, tag, moduleRoot ); - const branch = await createEntryService.commitEntryToNewBranch( + const branch = await this.createEntryService.commitEntryToNewBranch( rulesetRepo, bcr, tag, releaser ); - await createEntryService.pushEntryToFork(bcrFork, bcr, branch); + await this.createEntryService.pushEntryToFork( + bcrFork, + bcr, + branch, + bcrForkGitHubClient + ); console.log( `Pushed bcr entry for module '${moduleRoot}' to fork ${bcrFork.canonicalName} on branch ${branch}` ); - await publishEntryService.sendRequest( + await this.publishEntryService.sendRequest( tag, bcrFork, bcr, @@ -297,8 +260,7 @@ export class ReleaseEventHandler { private async overrideReleaser( releaser: User, - rulesetRepo: RulesetRepository, - githubClient: GitHubClient + rulesetRepo: RulesetRepository ): Promise { // Use the release author unless a fixedReleaser is configured if (rulesetRepo.config.fixedReleaser) { @@ -307,7 +269,7 @@ export class ReleaseEventHandler { ); // Fetch the releaser to get their name - const fixedReleaser = await githubClient.getRepoUser( + const fixedReleaser = await this.rulesetRepoGitHubClient.getRepoUser( rulesetRepo.config.fixedReleaser.login, rulesetRepo ); diff --git a/src/application/webhook/app.module.ts b/src/application/webhook/app.module.ts new file mode 100644 index 0000000..5107e70 --- /dev/null +++ b/src/application/webhook/app.module.ts @@ -0,0 +1,33 @@ +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 { EmailClient } from "../../infrastructure/email.js"; +import { GitClient } from "../../infrastructure/git.js"; +import { SecretsClient } from "../../infrastructure/secrets.js"; +import { NotificationsService } from "../notifications.js"; +import { ReleaseEventHandler } from "../release-event-handler.js"; +import { + APP_OCTOKIT_PROVIDER, + BCR_APP_OCTOKIT_PROVIDER, + BCR_GITHUB_CLIENT_PROVIDER, + RULESET_REPO_GITHUB_CLIENT_PROVIDER, +} from "./providers.js"; + +@Module({ + providers: [ + SecretsClient, + NotificationsService, + EmailClient, + GitClient, + ReleaseEventHandler, + CreateEntryService, + FindRegistryForkService, + PublishEntryService, + APP_OCTOKIT_PROVIDER, + BCR_APP_OCTOKIT_PROVIDER, + RULESET_REPO_GITHUB_CLIENT_PROVIDER, + BCR_GITHUB_CLIENT_PROVIDER, + ], +}) +export class AppModule {} diff --git a/src/application/cloudfunction/index.ts b/src/application/webhook/index.ts similarity index 75% rename from src/application/cloudfunction/index.ts rename to src/application/webhook/index.ts index caeac8b..3a1543d 100644 --- a/src/application/cloudfunction/index.ts +++ b/src/application/webhook/index.ts @@ -4,4 +4,4 @@ sourceMapSupport.install(); // Export all cloud function entrypoints here. The exported symbols // are inputs to deployed cloud functions and are invoked when the // function triggers. -export { handleGithubWebhookEvent } from "./github-webhook-entrypoint.js"; +export { handleGithubWebhookEvent } from "./main.js"; diff --git a/src/application/cloudfunction/github-webhook-entrypoint.ts b/src/application/webhook/main.ts similarity index 57% rename from src/application/cloudfunction/github-webhook-entrypoint.ts rename to src/application/webhook/main.ts index f7c1e8a..f19e542 100644 --- a/src/application/cloudfunction/github-webhook-entrypoint.ts +++ b/src/application/webhook/main.ts @@ -1,7 +1,9 @@ import { HttpFunction } from "@google-cloud/functions-framework"; +import { ContextIdFactory, NestFactory } from "@nestjs/core"; import { Webhooks } from "@octokit/webhooks"; import { SecretsClient } from "../../infrastructure/secrets.js"; import { ReleaseEventHandler } from "../release-event-handler.js"; +import { AppModule } from "./app.module.js"; // Handle incoming GitHub webhook messages. This is the entrypoint for // the webhook cloud function. @@ -9,19 +11,25 @@ export const handleGithubWebhookEvent: HttpFunction = async ( request, response ) => { - // Setup application dependencies using constructor dependency injection. - const secretsClient = new SecretsClient(); - - const releaseEventHandler = new ReleaseEventHandler(secretsClient); + const app = await NestFactory.createApplicationContext(AppModule); + const secretsClient = app.get(SecretsClient); const githubWebhookSecret = await secretsClient.accessSecret( "github-app-webhook-secret" ); const webhooks = new Webhooks({ secret: githubWebhookSecret }); - webhooks.on("release.published", (event) => - releaseEventHandler.handle(event) - ); + webhooks.on("release.published", async (event) => { + // Register the webhook event as the NestJS "request" so that it's available to inject. + const contextId = ContextIdFactory.create(); + app.registerRequestByContextId(event, contextId); + + const releaseEventHandler = await app.resolve( + ReleaseEventHandler, + contextId + ); + await releaseEventHandler.handle(event); + }); await webhooks.verifyAndReceive({ id: request.headers["x-github-delivery"] as string, @@ -30,5 +38,6 @@ export const handleGithubWebhookEvent: HttpFunction = async ( signature: request.headers["x-hub-signature-256"] as string, }); + await app.close(); response.status(200).send(); }; diff --git a/src/application/webhook/providers.ts b/src/application/webhook/providers.ts new file mode 100644 index 0000000..e73e5ec --- /dev/null +++ b/src/application/webhook/providers.ts @@ -0,0 +1,75 @@ +import { Provider, Scope } from "@nestjs/common"; +import { REQUEST } from "@nestjs/core"; +import { Octokit } from "@octokit/rest"; +import { EmitterWebhookEvent } from "@octokit/webhooks"; +import { Repository } from "../../domain/repository.js"; +import { GitHubClient } from "../../infrastructure/github.js"; +import { SecretsClient } from "../../infrastructure/secrets.js"; +import { + createAppAuthorizedOctokit, + createBotAppAuthorizedOctokit, +} from "../octokit.js"; + +export const APP_OCTOKIT_PROVIDER: Provider = { + // Provide an Octokit instance authorized for the user GitHub app, + // which users install to their ruleset and BCR fork. + provide: "appOctokit", + useFactory: (secretsClient: SecretsClient): Promise => { + return createAppAuthorizedOctokit(secretsClient); + }, + inject: [SecretsClient], +}; + +export const BCR_APP_OCTOKIT_PROVIDER: Provider = { + // Provide an Octokit instance authorized for the bcr GitHub app, + // which is installed to the BCR repo. + provide: "bcrAppOctokit", + useFactory: (secretsClient: SecretsClient): Promise => { + return createBotAppAuthorizedOctokit(secretsClient); + }, + inject: [SecretsClient], +}; + +export const RULESET_REPO_GITHUB_CLIENT_PROVIDER: Provider = { + // Get a GitHub client authorized for the installation of the user + // app to the ruleset repo. + provide: "rulesetRepoGitHubClient", + useFactory: async ( + event: EmitterWebhookEvent<"release.published">, + appOctokit: Octokit + ): Promise => { + const installationId = event.payload.installation.id; + + const rulesetRepo = new Repository( + event.payload.repository.name, + event.payload.repository.owner.login + ); + + const githubClient = await GitHubClient.forRepoInstallation( + appOctokit, + rulesetRepo, + installationId + ); + return githubClient; + }, + scope: Scope.REQUEST, + inject: [REQUEST, "appOctokit"], +}; + +export const BCR_GITHUB_CLIENT_PROVIDER: Provider = { + // Get a GitHub client authorized for the installation of the BCR + // app to the bazel central registry. + provide: "bcrGitHubClient", + useFactory: async (bcrAppOctokit: Octokit): Promise => { + const bcr = Repository.fromCanonicalName( + process.env.BAZEL_CENTRAL_REGISTRY + ); + + const githubClient = await GitHubClient.forRepoInstallation( + bcrAppOctokit, + bcr + ); + return githubClient; + }, + inject: ["bcrAppOctokit"], +}; diff --git a/src/domain/create-entry.spec.ts b/src/domain/create-entry.spec.ts index 5593369..24d2bf2 100644 --- a/src/domain/create-entry.spec.ts +++ b/src/domain/create-entry.spec.ts @@ -98,10 +98,8 @@ beforeEach(() => { mockBcrForkGitHubClient = mocked(new GitHubClient({} as any)); mockBcrGitHubClient = mocked(new GitHubClient({} as any)); mocked(computeIntegrityHash).mockReturnValue(`sha256-${randomUUID()}`); - Repository.gitClient = mockGitClient; createEntryService = new CreateEntryService( mockGitClient, - mockBcrForkGitHubClient, mockBcrGitHubClient ); }); @@ -192,13 +190,18 @@ describe("createEntryFiles", () => { }); test("returns the module name from the release archive", async () => { - mockRulesetFiles({extractedModuleName: "foomodule"}); + mockRulesetFiles({ extractedModuleName: "foomodule" }); const tag = "v1.2.3"; const rulesetRepo = await RulesetRepository.create("repo", "owner", tag); const bcrRepo = CANONICAL_BCR; - const result = await createEntryService.createEntryFiles(rulesetRepo, bcrRepo, tag, "."); + const result = await createEntryService.createEntryFiles( + rulesetRepo, + bcrRepo, + tag, + "." + ); expect(result.moduleName).toEqual("foomodule"); }); @@ -1093,7 +1096,12 @@ describe("pushEntryToFork", () => { const bcrForkRepo = new Repository("bazel-central-registry", "aspect"); const branchName = `repo/owner@v1.2.3`; - await createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName); + await createEntryService.pushEntryToFork( + bcrForkRepo, + bcrRepo, + branchName, + mockBcrForkGitHubClient + ); expect( mockBcrForkGitHubClient.getAuthenticatedRemoteUrl ).toHaveBeenCalledWith(bcrForkRepo); @@ -1109,7 +1117,12 @@ describe("pushEntryToFork", () => { Promise.resolve(authenticatedUrl) ); - await createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName); + await createEntryService.pushEntryToFork( + bcrForkRepo, + bcrRepo, + branchName, + mockBcrForkGitHubClient + ); expect(mockGitClient.addRemote).toHaveBeenCalledWith( bcrRepo.diskPath, expect.any(String), @@ -1127,7 +1140,12 @@ describe("pushEntryToFork", () => { Promise.resolve(authenticatedUrl) ); - await createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName); + await createEntryService.pushEntryToFork( + bcrForkRepo, + bcrRepo, + branchName, + mockBcrForkGitHubClient + ); expect(mockGitClient.addRemote).toHaveBeenCalledWith( expect.any(String), "authed-fork", @@ -1146,7 +1164,12 @@ describe("pushEntryToFork", () => { Promise.resolve(authenticatedUrl) ); - await createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName); + await createEntryService.pushEntryToFork( + bcrForkRepo, + bcrRepo, + branchName, + mockBcrForkGitHubClient + ); expect(mockGitClient.addRemote).not.toHaveBeenCalled(); }); @@ -1155,7 +1178,12 @@ describe("pushEntryToFork", () => { const bcrForkRepo = new Repository("bazel-central-registry", "aspect"); const branchName = `repo/owner@v1.2.3`; - await createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName); + await createEntryService.pushEntryToFork( + bcrForkRepo, + bcrRepo, + branchName, + mockBcrForkGitHubClient + ); expect(mockGitClient.push).toHaveBeenCalledWith( bcrRepo.diskPath, @@ -1185,7 +1213,12 @@ describe("pushEntryToFork", () => { .mockRejectedValueOnce(new Error("failed push")) .mockResolvedValueOnce(undefined); - await createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName); + await createEntryService.pushEntryToFork( + bcrForkRepo, + bcrRepo, + branchName, + mockBcrForkGitHubClient + ); expect(mockGitClient.push).toHaveBeenCalledTimes(5); }); @@ -1198,7 +1231,12 @@ describe("pushEntryToFork", () => { mockGitClient.push.mockRejectedValue(new Error("failed push")); await expect( - createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName) + createEntryService.pushEntryToFork( + bcrForkRepo, + bcrRepo, + branchName, + mockBcrForkGitHubClient + ) ).rejects.toThrow(); expect(mockGitClient.push).toHaveBeenCalledTimes(5); }); @@ -1260,6 +1298,10 @@ function mockRulesetFiles( } } ); + + mocked(GitClient).mockImplementation(() => { + return mockGitClient; + }); } function mockBcrMetadataExists( diff --git a/src/domain/create-entry.ts b/src/domain/create-entry.ts index 5625da6..25563b8 100644 --- a/src/domain/create-entry.ts +++ b/src/domain/create-entry.ts @@ -1,3 +1,4 @@ +import { Inject, Injectable } from "@nestjs/common"; import { createTwoFilesPatch, parsePatch } from "diff"; import { backOff } from "exponential-backoff"; import { randomBytes } from "node:crypto"; @@ -30,11 +31,11 @@ export class PatchModuleError extends UserFacingError { } } +@Injectable() export class CreateEntryService { constructor( private readonly gitClient: GitClient, - private readonly bcrForkGitHubClient: GitHubClient, - private readonly bcrGitHubClient: GitHubClient + @Inject("bcrGitHubClient") private bcrGitHubClient: GitHubClient ) {} public async createEntryFiles( @@ -42,7 +43,7 @@ export class CreateEntryService { bcrRepo: Repository, tag: string, moduleRoot: string - ): Promise<{moduleName: string}> { + ): Promise<{ moduleName: string }> { await Promise.all([rulesetRepo.checkout(tag), bcrRepo.checkout("main")]); const version = RulesetRepository.getVersionFromTag(tag); @@ -106,7 +107,7 @@ export class CreateEntryService { path.join(bcrVersionEntryPath, "presubmit.yml") ); - return {moduleName: moduleFile.moduleName }; + return { moduleName: moduleFile.moduleName }; } finally { releaseArchive.cleanup(); } @@ -152,10 +153,12 @@ export class CreateEntryService { public async pushEntryToFork( bcrForkRepo: Repository, bcr: Repository, - branch: string + branch: string, + githubClient: GitHubClient ): Promise { - const authenticatedRemoteUrl = - await this.bcrForkGitHubClient.getAuthenticatedRemoteUrl(bcrForkRepo); + const authenticatedRemoteUrl = await githubClient.getAuthenticatedRemoteUrl( + bcrForkRepo + ); if (!(await this.gitClient.hasRemote(bcr.diskPath, "authed-fork"))) { await this.gitClient.addRemote( diff --git a/src/domain/find-registry-fork.ts b/src/domain/find-registry-fork.ts index f505e0e..9e7ed47 100644 --- a/src/domain/find-registry-fork.ts +++ b/src/domain/find-registry-fork.ts @@ -1,3 +1,4 @@ +import { Inject, Injectable } from "@nestjs/common"; import { GitHubClient } from "../infrastructure/github.js"; import { UserFacingError } from "./error.js"; import { Repository } from "./repository.js"; @@ -20,8 +21,11 @@ Install the app here: https://github.com/apps/publish-to-bcr.`); } } +@Injectable() export class FindRegistryForkService { - constructor(private readonly githubClient: GitHubClient) {} + constructor( + @Inject("rulesetRepoGitHubClient") private githubClient: GitHubClient + ) {} // 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, diff --git a/src/domain/publish-entry.ts b/src/domain/publish-entry.ts index 9c5c959..5dccd0f 100644 --- a/src/domain/publish-entry.ts +++ b/src/domain/publish-entry.ts @@ -1,9 +1,11 @@ +import { Inject, Injectable } from "@nestjs/common"; import { GitHubClient } from "../infrastructure/github.js"; import { Repository } from "./repository.js"; import { RulesetRepository } from "./ruleset-repository.js"; +@Injectable() export class PublishEntryService { - constructor(private readonly githubClient: GitHubClient) {} + constructor(@Inject("bcrGitHubClient") private githubClient: GitHubClient) {} public async sendRequest( tag: string, diff --git a/src/domain/repository.spec.ts b/src/domain/repository.spec.ts index 584e0e7..24d9301 100644 --- a/src/domain/repository.spec.ts +++ b/src/domain/repository.spec.ts @@ -6,8 +6,7 @@ import { Repository } from "./repository"; jest.mock("../infrastructure/git"); beforeEach(() => { - mocked(GitClient, true).mockClear(); - Repository.gitClient = new GitClient(); + jest.clearAllMocks(); }); describe("fromCanonicalName", () => { @@ -48,11 +47,13 @@ describe("checkout", () => { test("clones and checks out the repository", async () => { const repository = new Repository("foo", "bar"); await repository.checkout("main"); - expect(Repository.gitClient.clone).toHaveBeenCalledWith( + + const mockGitClient = mocked(GitClient).mock.instances[0]; + expect(mockGitClient.clone).toHaveBeenCalledWith( repository.url, repository.diskPath ); - expect(Repository.gitClient.checkout).toHaveBeenCalledWith( + expect(mockGitClient.checkout).toHaveBeenCalledWith( repository.diskPath, "main" ); @@ -61,11 +62,12 @@ describe("checkout", () => { test("clones and checks out the default branch when branch not specified", async () => { const repository = new Repository("foo", "bar"); await repository.checkout(); - expect(Repository.gitClient.clone).toHaveBeenCalledWith( + const mockGitClient = mocked(GitClient).mock.instances[0]; + expect(mockGitClient.clone).toHaveBeenCalledWith( repository.url, repository.diskPath ); - expect(Repository.gitClient.checkout).toHaveBeenCalledWith( + expect(mockGitClient.checkout).toHaveBeenCalledWith( repository.diskPath, undefined ); diff --git a/src/domain/repository.ts b/src/domain/repository.ts index 912cf8a..1b6c9f9 100644 --- a/src/domain/repository.ts +++ b/src/domain/repository.ts @@ -4,8 +4,6 @@ import path from "node:path"; import { GitClient } from "../infrastructure/git.js"; export class Repository { - public static gitClient: GitClient; - private _diskPath: string | null = null; public static fromCanonicalName(canonicalName: string) { @@ -41,12 +39,13 @@ export class Repository { } public async checkout(ref?: string): Promise { + const gitClient = new GitClient(); if (!this.isCheckedOut()) { this._diskPath = path.join(os.tmpdir(), randomUUID(), this.name); - await Repository.gitClient.clone(this.url, this._diskPath); + await gitClient.clone(this.url, this._diskPath); } - await Repository.gitClient.checkout(this._diskPath, ref); + await gitClient.checkout(this._diskPath, ref); } public equals(other: Repository): boolean { diff --git a/src/domain/ruleset-repository.spec.ts b/src/domain/ruleset-repository.spec.ts index 8c3c116..371efa9 100644 --- a/src/domain/ruleset-repository.spec.ts +++ b/src/domain/ruleset-repository.spec.ts @@ -1,4 +1,4 @@ -import { Mocked, mocked } from "jest-mock"; +import { mocked } from "jest-mock"; import fs from "node:fs"; import path from "node:path"; import { GitClient } from "../infrastructure/git"; @@ -10,7 +10,6 @@ import { } from "../test/mock-template-files"; import { expectThrownError } from "../test/util"; import { FixedReleaser } from "./config"; -import { Repository } from "./repository"; import { InvalidConfigFileError, InvalidMetadataTemplateError, @@ -24,12 +23,8 @@ import { jest.mock("node:fs"); jest.mock("../infrastructure/git"); -let gitClient: Mocked; - beforeEach(() => { jest.clearAllMocks(); - gitClient = mocked(new GitClient()); - Repository.gitClient = gitClient; }); describe("create", () => { @@ -399,62 +394,72 @@ function mockRulesetFiles( fileContentMocks?: Record; } = {} ) { - gitClient.clone.mockImplementationOnce(async (url, repoPath) => { - const templatesDir = path.join( - repoPath, - RulesetRepository.BCR_TEMPLATE_DIR - ); - - mocked(fs.existsSync).mockImplementation(((p: string) => { - if ( - options.fileExistsMocks && - path.relative(repoPath, p) in options.fileExistsMocks! - ) { - return options.fileExistsMocks[path.relative(repoPath, p)]; - } else if (p === path.join(templatesDir, "metadata.template.json")) { - return !options.skipMetadataFile; - } else if (p === path.join(templatesDir, "presubmit.yml")) { - return !options.skipPresubmitFile; - } else if (p === path.join(templatesDir, "source.template.json")) { - return !options.skipSourceFile; - } else if ( - p === path.join(templatesDir, `config.${options.configExt || "yml"}`) - ) { - return options.configExists || options.configContent !== undefined; - } else if (p === repoPath) { - return true; - } - return (jest.requireActual("node:fs") as any).existsSync(path); - }) as any); - - mocked(fs.readFileSync).mockImplementation(((p: string, ...args: any[]) => { - if ( - options.fileContentMocks && - path.relative(repoPath, p) in options.fileContentMocks! - ) { - return options.fileContentMocks[path.relative(repoPath, p)]; - } else if (p === path.join(templatesDir, "metadata.template.json")) { - return fakeMetadataFile({ - malformed: options.invalidMetadataFile, - missingVersions: options.metadataMissingVersions, - }); - } else if (p === path.join(templatesDir, "source.template.json")) { - return fakeSourceFile({ malformed: options.invalidSourceTemplate }); - } else if (p === path.join(templatesDir, "presubmit.yml")) { - return fakePresubmitFile({ malformed: options.invalidPresubmit }); - } else if ( - p === path.join(templatesDir, `config.${options.configExt || "yml"}`) - ) { - return fakeConfigFile({ - content: options.configContent, - fixedReleaser: options.fixedReleaser, - invalidFixedReleaser: options.invalidFixedReleaser, - }); - } - return (jest.requireActual("node:fs") as any).readFileSync.apply([ - path, - ...args, - ]); - }) as any); + mocked(GitClient).mockImplementation(() => { + return { + checkout: jest.fn(), + clone: jest.fn().mockImplementation(async (url, repoPath) => { + const templatesDir = path.join( + repoPath, + RulesetRepository.BCR_TEMPLATE_DIR + ); + + mocked(fs.existsSync).mockImplementation(((p: string) => { + if ( + options.fileExistsMocks && + path.relative(repoPath, p) in options.fileExistsMocks! + ) { + return options.fileExistsMocks[path.relative(repoPath, p)]; + } else if (p === path.join(templatesDir, "metadata.template.json")) { + return !options.skipMetadataFile; + } else if (p === path.join(templatesDir, "presubmit.yml")) { + return !options.skipPresubmitFile; + } else if (p === path.join(templatesDir, "source.template.json")) { + return !options.skipSourceFile; + } else if ( + p === + path.join(templatesDir, `config.${options.configExt || "yml"}`) + ) { + return options.configExists || options.configContent !== undefined; + } else if (p === repoPath) { + return true; + } + return (jest.requireActual("node:fs") as any).existsSync(path); + }) as any); + + mocked(fs.readFileSync).mockImplementation((( + p: string, + ...args: any[] + ) => { + if ( + options.fileContentMocks && + path.relative(repoPath, p) in options.fileContentMocks! + ) { + return options.fileContentMocks[path.relative(repoPath, p)]; + } else if (p === path.join(templatesDir, "metadata.template.json")) { + return fakeMetadataFile({ + malformed: options.invalidMetadataFile, + missingVersions: options.metadataMissingVersions, + }); + } else if (p === path.join(templatesDir, "source.template.json")) { + return fakeSourceFile({ malformed: options.invalidSourceTemplate }); + } else if (p === path.join(templatesDir, "presubmit.yml")) { + return fakePresubmitFile({ malformed: options.invalidPresubmit }); + } else if ( + p === + path.join(templatesDir, `config.${options.configExt || "yml"}`) + ) { + return fakeConfigFile({ + content: options.configContent, + fixedReleaser: options.fixedReleaser, + invalidFixedReleaser: options.invalidFixedReleaser, + }); + } + return (jest.requireActual("node:fs") as any).readFileSync.apply([ + path, + ...args, + ]); + }) as any); + }), + } as any; }); } diff --git a/src/infrastructure/email.ts b/src/infrastructure/email.ts index 097bf3e..bf2d0a0 100644 --- a/src/infrastructure/email.ts +++ b/src/infrastructure/email.ts @@ -1,8 +1,10 @@ +import { Injectable } from "@nestjs/common"; import nodemailer from "nodemailer"; import { AuthenticationTypeLogin } from "nodemailer/lib/smtp-connection"; export type Authentication = AuthenticationTypeLogin; +@Injectable() export class EmailClient { private auth: Authentication | undefined; diff --git a/src/infrastructure/git.ts b/src/infrastructure/git.ts index ac8bdc1..bb9732c 100644 --- a/src/infrastructure/git.ts +++ b/src/infrastructure/git.ts @@ -1,5 +1,7 @@ +import { Injectable } from "@nestjs/common"; import { simpleGit } from "simple-git"; +@Injectable() export class GitClient { public async clone(url: string, repoPath: string): Promise { await simpleGit().clone(url, repoPath); diff --git a/src/infrastructure/github.ts b/src/infrastructure/github.ts index 8d3489f..4a25933 100644 --- a/src/infrastructure/github.ts +++ b/src/infrastructure/github.ts @@ -47,12 +47,12 @@ export function getAppAuthorizedOctokit( export async function getInstallationAuthorizedOctokit( appOctokit: Octokit, installationId: number, - repo: string + repository: string ): Promise { const octokit = await appOctokit.auth({ type: "installation", installationId, - repositoryNames: [repo], + repositoryNames: [repository], factory: (auth: any) => new Octokit({ authStrategy: createAppAuth, diff --git a/src/infrastructure/secrets.ts b/src/infrastructure/secrets.ts index 7674871..dca661f 100644 --- a/src/infrastructure/secrets.ts +++ b/src/infrastructure/secrets.ts @@ -1,6 +1,8 @@ import { SecretManagerServiceClient } from "@google-cloud/secret-manager"; +import { Injectable } from "@nestjs/common"; import gcpMetadata from "gcp-metadata"; +@Injectable() export class SecretsClient { private readonly googleSecretsClient; diff --git a/tsconfig.json b/tsconfig.json index 829f9db..ef290ae 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -8,6 +8,8 @@ "target": "es2020", "moduleResolution": "node", "allowSyntheticDefaultImports": true, + "experimentalDecorators": true, + "emitDecoratorMetadata": true, "allowJs": true, "types": ["jest", "node"] }, diff --git a/yarn.lock b/yarn.lock index c87d0d6..399a0e1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -777,6 +777,32 @@ resolved "https://registry.yarnpkg.com/@kwsites/promise-deferred/-/promise-deferred-1.1.1.tgz#8ace5259254426ccef57f3175bc64ed7095ed919" integrity sha512-GaHYm+c0O9MjZRu0ongGBRbinu8gVAMd2UZjji6jVmqKtZluZnptXGWhz1E8j8D2HJ3f/yMxKAUC0b+57wncIw== +"@lukeed/csprng@^1.0.0": + version "1.1.0" + resolved "https://registry.yarnpkg.com/@lukeed/csprng/-/csprng-1.1.0.tgz#1e3e4bd05c1cc7a0b2ddbd8a03f39f6e4b5e6cfe" + integrity sha512-Z7C/xXCiGWsg0KuKsHTKJxbWhpI3Vs5GwLfOean7MGyVFGqdRgBbAjOCh6u4bbjPc/8MJ2pZmK/0DLdCbivLDA== + +"@nestjs/common@^10.3.9": + version "10.3.9" + resolved "https://registry.yarnpkg.com/@nestjs/common/-/common-10.3.9.tgz#4e85d8fa6ae201a1c5f49d4d09b205bc3672ed1f" + integrity sha512-JAQONPagMa+sy/fcIqh/Hn3rkYQ9pQM51vXCFNOM5ujefxUVqn3gwFRMN8Y1+MxdUHipV+8daEj2jEm0IqJzOA== + dependencies: + uid "2.0.2" + iterare "1.2.1" + tslib "2.6.2" + +"@nestjs/core@^10.3.9": + version "10.3.9" + resolved "https://registry.yarnpkg.com/@nestjs/core/-/core-10.3.9.tgz#fe3645eb4974423de5503a19d08f85a67693e72f" + integrity sha512-NzZUfWAmaf8sqhhwoRA+CuqxQe+P4Rz8PZp5U7CdCbjyeB9ZVGcBkihcJC9wMdtiOWHRndB2J8zRfs5w06jK3w== + dependencies: + uid "2.0.2" + "@nuxtjs/opencollective" "0.3.2" + fast-safe-stringify "2.1.1" + iterare "1.2.1" + path-to-regexp "3.2.0" + tslib "2.6.2" + "@nodelib/fs.scandir@2.1.5": version "2.1.5" resolved "https://registry.yarnpkg.com/@nodelib/fs.scandir/-/fs.scandir-2.1.5.tgz#7619c2eb21b25483f6d167548b4cfd5a7488c3d5" @@ -798,6 +824,15 @@ "@nodelib/fs.scandir" "2.1.5" fastq "^1.6.0" +"@nuxtjs/opencollective@0.3.2": + version "0.3.2" + resolved "https://registry.yarnpkg.com/@nuxtjs/opencollective/-/opencollective-0.3.2.tgz#620ce1044f7ac77185e825e1936115bb38e2681c" + integrity sha512-um0xL3fO7Mf4fDxcqx9KryrB7zgRM5JSlvGN5AGkP6JLM5XEKyjeAiPbNxdXVXQ16isuAhYpvP88NgL2BGd6aA== + dependencies: + chalk "^4.1.0" + consola "^2.15.0" + node-fetch "^2.6.1" + "@octokit/auth-app@^4.0.4": version "4.0.13" resolved "https://registry.yarnpkg.com/@octokit/auth-app/-/auth-app-4.0.13.tgz#53323bee6bfefbb73ea544dd8e6a0144550e13e3" @@ -1879,7 +1914,7 @@ chalk@^2.4.2: escape-string-regexp "^1.0.5" supports-color "^5.3.0" -chalk@^4.0.0: +chalk@^4.0.0, chalk@^4.1.0: version "4.1.2" resolved "https://registry.yarnpkg.com/chalk/-/chalk-4.1.2.tgz#aac4e2b7734a740867aeb16bf02aad556a1e7a01" integrity sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA== @@ -2013,6 +2048,11 @@ connect@^3.7.0: parseurl "~1.3.3" utils-merge "1.0.1" +consola@^2.15.0: + version "2.15.3" + resolved "https://registry.yarnpkg.com/consola/-/consola-2.15.3.tgz#2e11f98d6a4be71ff72e0bdf07bd23e12cb61550" + integrity sha512-9vAdYbHj6x2fLKC4+oPH0kFzY/orMZyG2Aj+kNylHxKGJ/Ed4dpNyAQYwJOdqO4zdM7XpVHmyejQDcQHrnuXbw== + content-disposition@0.5.4: version "0.5.4" resolved "https://registry.yarnpkg.com/content-disposition/-/content-disposition-0.5.4.tgz#8b82b4efac82512a02bb0b1dcec9d2c5e8eb5bfe" @@ -2497,6 +2537,11 @@ fast-redact@^3.1.1: resolved "https://registry.yarnpkg.com/fast-redact/-/fast-redact-3.3.0.tgz#7c83ce3a7be4898241a46560d51de10f653f7634" integrity sha512-6T5V1QK1u4oF+ATxs1lWUmlEk6P2T9HqJG3e2DnHOdVgZy2rFJBoEnrIedcTXlkAHU/zKC+7KETJ+KGGKwxgMQ== +fast-safe-stringify@2.1.1: + version "2.1.1" + resolved "https://registry.yarnpkg.com/fast-safe-stringify/-/fast-safe-stringify-2.1.1.tgz#c406a83b6e70d9e35ce3b30a81141df30aeba884" + integrity sha512-W+KJc2dmILlPplD/H4K9l9LcAHAfPtP6BY84uVLXQ6Evcz9Lcg33Y2z1IVblT6xdY54PXYVHEv+0Wpq8Io6zkA== + fastq@^1.6.0: version "1.16.0" resolved "https://registry.yarnpkg.com/fastq/-/fastq-1.16.0.tgz#83b9a9375692db77a822df081edb6a9cf6839320" @@ -3202,6 +3247,11 @@ iterall@^1.2.1, iterall@^1.3.0: resolved "https://registry.yarnpkg.com/iterall/-/iterall-1.3.0.tgz#afcb08492e2915cbd8a0884eb93a8c94d0d72fea" integrity sha512-QZ9qOMdF+QLHxy1QIpUHUU1D5pS2CG2P69LF6L6CPjPYA/XMOmKV3PZpawHoAjHNyB0swdVTRxdYT4tbBbxqwg== +iterare@1.2.1: + version "1.2.1" + resolved "https://registry.yarnpkg.com/iterare/-/iterare-1.2.1.tgz#139c400ff7363690e33abffa33cbba8920f00042" + integrity sha512-RKYVTCjAnRthyJes037NX/IiqeidgN1xc3j1RjFfECFp28A1GVwK9nA+i0rJPaHqSZwygLzRnFlzUuHFoWWy+Q== + jest-changed-files@^28.1.3: version "28.1.3" resolved "https://registry.yarnpkg.com/jest-changed-files/-/jest-changed-files-28.1.3.tgz#d9aeee6792be3686c47cb988a8eaf82ff4238831" @@ -4238,6 +4288,11 @@ path-to-regexp@0.1.7: resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-0.1.7.tgz#df604178005f522f15eb4490e7247a1bfaa67f8c" integrity sha512-5DFkuoqlv1uYQKxy8omFBeJPQcdoE07Kv2sferDCrAq1ohOU+MSDswDIbnx3YAM60qIOnYa53wBhXW0EbMonrQ== +path-to-regexp@3.2.0: + version "3.2.0" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-3.2.0.tgz#fa7877ecbc495c601907562222453c43cc204a5f" + integrity sha512-jczvQbCUS7XmS7o+y1aEO9OBVFeZBQ1MDSEqmO7xSoPgOPoowY/SxLpZ6Vh97/8qHZOteiCKb7gkG9gA2ZUxJA== + path-type@^5.0.0: version "5.0.0" resolved "https://registry.yarnpkg.com/path-type/-/path-type-5.0.0.tgz#14b01ed7aea7ddf9c7c3f46181d4d04f9c785bb8" @@ -4545,6 +4600,11 @@ real-require@^0.2.0: resolved "https://registry.yarnpkg.com/real-require/-/real-require-0.2.0.tgz#209632dea1810be2ae063a6ac084fee7e33fba78" integrity sha512-57frrGM/OCTLqLOAh0mhVA9VBMHd+9U7Zb2THMGdBUoZVOtGbJzjxsYGDJ3A9AYYCP4hn6y1TVbaOfzWtm5GFg== +reflect-metadata@^0.2.2: + version "0.2.2" + resolved "https://registry.yarnpkg.com/reflect-metadata/-/reflect-metadata-0.2.2.tgz#400c845b6cba87a21f2c65c4aeb158f4fa4d9c5b" + integrity sha512-urBwgfrvVP/eAyXx4hluJivBKzuEbSQs9rKWCrCkbSxNv8mxPcUZKeuoF3Uy4mJl3Lwprp6yy5/39VWigZ4K6Q== + require-directory@^2.1.1: version "2.1.1" resolved "https://registry.yarnpkg.com/require-directory/-/require-directory-2.1.1.tgz#8c64ad5fd30dab1c976e2344ffe7f792a6a6df42" @@ -4624,6 +4684,13 @@ run-parallel@^1.1.9: dependencies: queue-microtask "^1.2.2" +rxjs@7.8.1: + version "7.8.1" + resolved "https://registry.yarnpkg.com/rxjs/-/rxjs-7.8.1.tgz#6f6f3d99ea8044291efd92e7c7fcf562c4057543" + integrity sha512-AA3TVj+0A2iuIoQkWEK/tqFjBq2j+6PO6Y0zJcvzLAFhEFIO3HL0vls9hWLncZbAAbK0mar7oZ4V079I/qPMxg== + dependencies: + tslib "^2.1.0" + safe-buffer@5.2.1, safe-buffer@^5.0.1, safe-buffer@^5.1.2, safe-buffer@~5.2.0: version "5.2.1" resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.2.1.tgz#1eaf9fa9bdb1fdd4ec75f58f9cdb4e6b7827eec6" @@ -5112,7 +5179,7 @@ ts-node@^10.9.1: v8-compile-cache-lib "^3.0.1" yn "3.1.1" -tslib@^2.0.1, tslib@^2.1.0, tslib@^2.4.0: +tslib@2.6.2, tslib@^2.0.1, tslib@^2.1.0, tslib@^2.4.0: version "2.6.2" resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.6.2.tgz#703ac29425e7b37cd6fd456e92404d46d1f3e4ae" integrity sha512-AEYxH93jGFPn/a2iVAwW87VuUIkR1FVUKB77NwMF7nBTDkDrrT/Hpt/IrCJ0QXhW27jTBDcf5ZY7w6RiqTMw2Q== @@ -5160,6 +5227,13 @@ uc.micro@^2.0.0: resolved "https://registry.yarnpkg.com/uc.micro/-/uc.micro-2.0.0.tgz#84b3c335c12b1497fd9e80fcd3bfa7634c363ff1" integrity sha512-DffL94LsNOccVn4hyfRe5rdKa273swqeA5DJpMOeFmEn1wCDc7nAbbB0gXlgBCL7TNzeTv6G7XVWzan7iJtfig== +uid@2.0.2: + version "2.0.2" + resolved "https://registry.yarnpkg.com/uid/-/uid-2.0.2.tgz#4b5782abf0f2feeefc00fa88006b2b3b7af3e3b9" + integrity sha512-u3xV3X7uzvi5b1MncmZo3i2Aw222Zk1keqLA1YkHldREkAhAqi65wuPfe7lHx8H/Wzy+8CE7S7uS3jekIM5s8g== + dependencies: + "@lukeed/csprng" "^1.0.0" + undici-types@~5.26.4: version "5.26.5" resolved "https://registry.yarnpkg.com/undici-types/-/undici-types-5.26.5.tgz#bcd539893d00b56e964fd2657a4866b221a65617"