diff --git a/CHANGELOG.md b/CHANGELOG.md index c43a7c116..43d9673b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,23 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [v0.56.0](https://github.com/isomerpages/isomercms-backend/compare/v0.55.0...v0.56.0) + +- fix(rollback handler): convert to ts for safety [`#1044`](https://github.com/isomerpages/isomercms-backend/pull/1044) +- test(gitCommitService): add test cases [`#1047`](https://github.com/isomerpages/isomercms-backend/pull/1047) +- feat(gitFileSystem): safer api [`#1046`](https://github.com/isomerpages/isomercms-backend/pull/1046) +- test(githubService): add tests [`#1045`](https://github.com/isomerpages/isomercms-backend/pull/1045) +- chore(quickie): delete quickie for gh [`#1043`](https://github.com/isomerpages/isomercms-backend/pull/1043) +- feat(quickie): only gitfile should have quickie [`#1042`](https://github.com/isomerpages/isomercms-backend/pull/1042) +- hotfix(repair-form): set remote url correctly [`#1048`](https://github.com/isomerpages/isomercms-backend/pull/1048) +- build(deps): bump sequelize-typescript from 2.1.5 to 2.1.6 [`#1041`](https://github.com/isomerpages/isomercms-backend/pull/1041) +- Added swagger doc for GIG DNS API [`#1017`](https://github.com/isomerpages/isomercms-backend/pull/1017) +- 0.55.0 [`#1039`](https://github.com/isomerpages/isomercms-backend/pull/1039) + #### [v0.55.0](https://github.com/isomerpages/isomercms-backend/compare/v0.54.0...v0.55.0) +> 16 November 2023 + - fix(siteCreate): add redirect rules [`#1036`](https://github.com/isomerpages/isomercms-backend/pull/1036) - chore: remove extra and unused submodules [`#1031`](https://github.com/isomerpages/isomercms-backend/pull/1031) - release/0.54.0 [`#1033`](https://github.com/isomerpages/isomercms-backend/pull/1033) @@ -21,6 +36,7 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). - fix(pagination): images fix pagination [`#1026`](https://github.com/isomerpages/isomercms-backend/pull/1026) - fix(media): change media sorting to addedTime descending [`#1019`](https://github.com/isomerpages/isomercms-backend/pull/1019) - 0.53.0 [`#1024`](https://github.com/isomerpages/isomercms-backend/pull/1024) +- fix(mediafileservice): disable sneky cloudmersive [`#1025`](https://github.com/isomerpages/isomercms-backend/pull/1025) - release(0.52.0): merge to prod [`#1013`](https://github.com/isomerpages/isomercms-backend/pull/1013) - Release/0.51.0 [`#1002`](https://github.com/isomerpages/isomercms-backend/pull/1002) - * refactor(formsg-site-clone): remove and add to site creation (#971) [`#992`](https://github.com/isomerpages/isomercms-backend/pull/992) @@ -94,7 +110,6 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). > 9 November 2023 -- fix(mediafileservice): disable sneky cloudmersive [`#1025`](https://github.com/isomerpages/isomercms-backend/pull/1025) - fix(file ext): fix casing + better logging [`#1020`](https://github.com/isomerpages/isomercms-backend/pull/1020) - fix(rr): capture file extensions that are in uppercase [`#1016`](https://github.com/isomerpages/isomercms-backend/pull/1016) - release(0.52.0): merge to develop [`#1012`](https://github.com/isomerpages/isomercms-backend/pull/1012) diff --git a/docs/gigDnsApi.yaml b/docs/gigDnsApi.yaml new file mode 100644 index 000000000..5c0b71705 --- /dev/null +++ b/docs/gigDnsApi.yaml @@ -0,0 +1,230 @@ +openapi: 3.0.0 +info: + version: "1.0.0" + title: "GIG DNS Management API" + description: "An API to manage DNS records for GovTech Domains" + +paths: + /dns-records: + post: + summary: Add a DNS record + description: Add a DNS record of various types to the DNS management system along with a contact email. + security: + - ApiKeyAuth: [] + parameters: + - in: header + name: X-API-KEY + required: true + schema: + type: string + description: API key required to authorize requests. + example: "1234567890abcdef" + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - email + - record + properties: + email: + type: string + format: email + description: The contact email address for this DNS record. + record: + $ref: "#/components/schemas/DNSRecord" + example: + email: "admin@example.com" + record: + type: "A" + name: "example.com" + content: "192.168.1.1" + responses: + "201": + description: DNS record successfully added. + content: + application/json: + schema: + type: object + properties: + message: + type: string + example: "DNS record added successfully." + "401": + description: Unauthorized. + "400": + description: Invalid request data. + content: + application/json: + schema: + type: object + properties: + message: + type: string + example: "Invalid data provided." + "500": + description: Server error. + content: + application/json: + schema: + type: object + properties: + message: + type: string + example: "Internal server error." + get: + summary: Retrieve DNS records + description: Get a list of DNS records for a specified name. + parameters: + - in: query + name: name + required: true + schema: + type: string + description: The name of the DNS record to retrieve. + responses: + "200": + description: A list of DNS records. + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/DNSRecord" + "401": + description: Unauthorized. + "404": + description: No records found for the specified name. + delete: + summary: Delete a DNS record by name and type + description: Delete a specific DNS record identified by its name and type. + parameters: + - in: query + name: name + required: true + schema: + type: string + description: The name of the DNS record to be deleted. + - in: query + name: type + required: true + schema: + type: string + enum: + [ + "A", + "AAAA", + "CNAME", + "TXT", + "SRV", + "LOC", + "MX", + "NS", + "SPF", + "CAA", + "DNSKEY", + "DS", + ] + description: The type of the DNS record to be deleted. + responses: + "200": + description: DNS record successfully deleted. + content: + application/json: + schema: + type: object + properties: + message: + type: string + example: "DNS record successfully deleted" + "401": + description: Unauthorized. + "404": + description: DNS record not found. +components: + securitySchemes: + ApiKeyAuth: + type: apiKey + in: header + name: X-API-KEY + description: API key required to authorize requests. + schemas: + DNSRecord: + type: object + required: + - type + - name + - content + properties: + type: + type: string + description: The type of the DNS record. + enum: + [ + "A", + "AAAA", + "CNAME", + "TXT", + "SRV", + "LOC", + "MX", + "NS", + "SPF", + "CAA", + "DNSKEY", + "DS", + ] + name: + type: string + description: The name of the DNS record. + content: + oneOf: + - type: string + - $ref: "#/components/schemas/DNSKEYData" + - $ref: "#/components/schemas/DSData" + description: The content of the DNS record. Varies depending on the record type. + priority: + type: integer + description: Priority of the record (used for MX and SRV records). + DNSKEYData: + type: object + required: + - flags + - protocol + - algorithm + - publicKey + properties: + flags: + type: integer + description: An unsigned 16-bit integer representing DNSKEY flags. + protocol: + type: integer + description: The protocol field. Should always be 3. + algorithm: + type: integer + description: An 8-bit integer identifying the DNSKEY's cryptographic algorithm. + publicKey: + type: string + description: The public key material, encoded in Base64. + DSData: + type: object + required: + - keyTag + - algorithm + - digestType + - digest + properties: + keyTag: + type: integer + description: An unsigned 16-bit integer representing the key tag. + algorithm: + type: integer + description: An 8-bit integer identifying the DS record's cryptographic algorithm. + digestType: + type: integer + description: An 8-bit integer representing the type of the digest. + digest: + type: string + description: The digest value, encoded in Base64. diff --git a/package-lock.json b/package-lock.json index 7de7e5b90..0491adcf8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "isomercms", - "version": "0.55.0", + "version": "0.56.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "isomercms", - "version": "0.55.0", + "version": "0.56.0", "dependencies": { "@aws-sdk/client-amplify": "^3.370.0", "@aws-sdk/client-cloudwatch-logs": "^3.370.0", @@ -66,7 +66,7 @@ "query-string": "^6.14.1", "reflect-metadata": "^0.1.13", "sequelize": "^6.29.0", - "sequelize-typescript": "^2.1.3", + "sequelize-typescript": "^2.1.6", "serialize-error": "^7.0.1", "simple-git": "^3.19.1", "slugify": "^1.6.0", @@ -14691,9 +14691,9 @@ } }, "node_modules/sequelize-typescript": { - "version": "2.1.5", - "resolved": "https://registry.npmjs.org/sequelize-typescript/-/sequelize-typescript-2.1.5.tgz", - "integrity": "sha512-x1CNODct8gJyfZPwEZBU5uVGNwgJI2Fda913ZxD5ZtCSRyTDPBTS/0uXciF+MlCpyqjpmoCAPtudQWzw579bzA==", + "version": "2.1.6", + "resolved": "https://registry.npmjs.org/sequelize-typescript/-/sequelize-typescript-2.1.6.tgz", + "integrity": "sha512-Vc2N++3en346RsbGjL3h7tgAl2Y7V+2liYTAOZ8XL0KTw3ahFHsyAUzOwct51n+g70I1TOUDgs06Oh6+XGcFkQ==", "dependencies": { "glob": "7.2.0" }, diff --git a/package.json b/package.json index e65f2e46e..11bb185e5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "isomercms", - "version": "0.55.0", + "version": "0.56.0", "private": true, "scripts": { "build": "tsc -p tsconfig.build.json", @@ -82,7 +82,7 @@ "query-string": "^6.14.1", "reflect-metadata": "^0.1.13", "sequelize": "^6.29.0", - "sequelize-typescript": "^2.1.3", + "sequelize-typescript": "^2.1.6", "serialize-error": "^7.0.1", "simple-git": "^3.19.1", "slugify": "^1.6.0", diff --git a/src/fixtures/sessionData.ts b/src/fixtures/sessionData.ts index 661515caa..2e1c6e72d 100644 --- a/src/fixtures/sessionData.ts +++ b/src/fixtures/sessionData.ts @@ -4,6 +4,7 @@ import GithubSessionData from "@root/classes/GithubSessionData" import UserSessionData from "@root/classes/UserSessionData" import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" import { FeatureFlags } from "@root/types/featureFlags" +import { RawGitTreeEntry } from "@root/types/github" import { MOCK_USER_EMAIL_ONE, @@ -25,6 +26,24 @@ export const mockCurrentCommitSha = "mockCurrentCommitSha" export const mockSiteName = "mockSiteName" export const mockGrowthBook = new GrowthBook() +export const gitTree: RawGitTreeEntry[] = [ + { + path: "directory/file1.txt", + type: "tree", + sha: "fake-sha-1", + mode: "100644", + url: "fake-url-1", + }, + { + path: "directory/file2.txt", + type: "file", + sha: "fake-sha-2", + mode: "100644", + url: "fake-url-2", + size: 100, + }, +] + export const mockGithubState = { treeSha: mockTreeSha, currentCommitSha: mockCurrentCommitSha, diff --git a/src/integration/Notifications.spec.ts b/src/integration/Notifications.spec.ts index b25a4b660..3fc89a30a 100644 --- a/src/integration/Notifications.spec.ts +++ b/src/integration/Notifications.spec.ts @@ -34,7 +34,6 @@ import { import { getAuthorizationMiddleware } from "@root/middleware" import { NotificationsRouter as _NotificationsRouter } from "@root/routes/v2/authenticated/notifications" import GitFileCommitService from "@root/services/db/GitFileCommitService" -import GitHubCommitService from "@root/services/db/GithubCommitService" import { BaseDirectoryService } from "@root/services/directoryServices/BaseDirectoryService" import { ResourceRoomDirectoryService } from "@root/services/directoryServices/ResourceRoomDirectoryService" import { CollectionPageService } from "@root/services/fileServices/MdPageServices/CollectionPageService" @@ -71,12 +70,10 @@ const MOCK_SITE_MEMBER_ID = "1" const gitFileSystemService = new GitFileSystemService(simpleGit()) const gitFileCommitService = new GitFileCommitService(gitFileSystemService) -const gitHubCommitService = new GitHubCommitService(isomerRepoAxiosInstance) const gitHubService = new RepoService({ isomerRepoAxiosInstance, gitFileSystemService, gitFileCommitService, - gitHubCommitService, }) const identityAuthService = getIdentityAuthService(gitHubService) const usersService = getUsersService(sequelize) diff --git a/src/integration/Privatisation.spec.ts b/src/integration/Privatisation.spec.ts index d47157bc0..c129a5569 100644 --- a/src/integration/Privatisation.spec.ts +++ b/src/integration/Privatisation.spec.ts @@ -45,7 +45,6 @@ import { AuthorizationMiddleware } from "@root/middleware/authorization" import { SettingsRouter as _SettingsRouter } from "@root/routes/v2/authenticatedSites/settings" import { SettingsService } from "@root/services/configServices/SettingsService" import GitFileCommitService from "@root/services/db/GitFileCommitService" -import GitHubCommitService from "@root/services/db/GithubCommitService" import { BaseDirectoryService } from "@root/services/directoryServices/BaseDirectoryService" import { ResourceRoomDirectoryService } from "@root/services/directoryServices/ResourceRoomDirectoryService" import { CollectionPageService } from "@root/services/fileServices/MdPageServices/CollectionPageService" @@ -100,12 +99,11 @@ jest.mock("../services/identity/DeploymentClient", () => ) const gitFileSystemService = new GitFileSystemService(simpleGit()) const gitFileCommitService = new GitFileCommitService(gitFileSystemService) -const gitHubCommitService = new GitHubCommitService(isomerRepoAxiosInstance) + const gitHubService = new RepoService({ isomerRepoAxiosInstance, gitFileSystemService, gitFileCommitService, - gitHubCommitService, }) const configYmlService = new ConfigYmlService({ gitHubService }) const usersService = getUsersService(sequelize) diff --git a/src/integration/Reviews.spec.ts b/src/integration/Reviews.spec.ts index 927dc7956..a63d551a5 100644 --- a/src/integration/Reviews.spec.ts +++ b/src/integration/Reviews.spec.ts @@ -75,7 +75,6 @@ import { } from "@fixtures/users" import { ReviewRequestStatus } from "@root/constants" import GitFileCommitService from "@root/services/db/GitFileCommitService" -import GitHubCommitService from "@root/services/db/GithubCommitService" import { BaseDirectoryService } from "@root/services/directoryServices/BaseDirectoryService" import { ResourceRoomDirectoryService } from "@root/services/directoryServices/ResourceRoomDirectoryService" import { CollectionPageService } from "@root/services/fileServices/MdPageServices/CollectionPageService" @@ -105,12 +104,11 @@ import { sequelize } from "@tests/database" const gitFileSystemService = new GitFileSystemService(simpleGit()) const gitFileCommitService = new GitFileCommitService(gitFileSystemService) -const gitHubCommitService = new GitHubCommitService(isomerRepoAxiosInstance) + const gitHubService = new RepoService({ isomerRepoAxiosInstance, gitFileSystemService, gitFileCommitService, - gitHubCommitService, }) const configYmlService = new ConfigYmlService({ gitHubService }) const usersService = getUsersService(sequelize) diff --git a/src/integration/Sites.spec.ts b/src/integration/Sites.spec.ts index 0df835d79..61d3f5a1c 100644 --- a/src/integration/Sites.spec.ts +++ b/src/integration/Sites.spec.ts @@ -26,7 +26,6 @@ import { statsMiddleware } from "@root/middleware/stats" import { SitesRouter as _SitesRouter } from "@root/routes/v2/authenticated/sites" import { isomerRepoAxiosInstance } from "@root/services/api/AxiosInstance" import GitFileCommitService from "@root/services/db/GitFileCommitService" -import GitHubCommitService from "@root/services/db/GithubCommitService" import { BaseDirectoryService } from "@root/services/directoryServices/BaseDirectoryService" import { ResourceRoomDirectoryService } from "@root/services/directoryServices/ResourceRoomDirectoryService" import { CollectionPageService } from "@root/services/fileServices/MdPageServices/CollectionPageService" @@ -67,13 +66,12 @@ const mockPermissions = { push: true } const mockPrivate = true const gitFileSystemService = new GitFileSystemService(simpleGit()) -const gitHubCommitService = new GitHubCommitService(isomerRepoAxiosInstance) + const gitFileCommitService = new GitFileCommitService(gitFileSystemService) const gitHubService = new RepoService({ isomerRepoAxiosInstance, gitFileSystemService, gitFileCommitService, - gitHubCommitService, }) const configYmlService = new ConfigYmlService({ gitHubService }) const usersService = getUsersService(sequelize) diff --git a/src/middleware/routeHandler.js b/src/middleware/routeHandler.js deleted file mode 100644 index 147c75534..000000000 --- a/src/middleware/routeHandler.js +++ /dev/null @@ -1,202 +0,0 @@ -const { backOff } = require("exponential-backoff") -const SimpleGit = require("simple-git") - -const { config } = require("@config/config") - -const logger = require("@logger/logger").default - -const { default: GithubSessionData } = require("@classes/GithubSessionData") - -const { lock, unlock } = require("@utils/mutex-utils") -const { getCommitAndTreeSha, revertCommit } = require("@utils/utils.js") - -const { MAX_CONCURRENT_GIT_PROCESSES } = require("@constants/constants") - -const { FEATURE_FLAGS } = require("@root/constants/featureFlags") -const GitFileSystemError = require("@root/errors/GitFileSystemError").default -const LockedError = require("@root/errors/LockedError").default -const { - default: GitFileSystemService, -} = require("@services/db/GitFileSystemService") - -const BRANCH_REF = config.get("github.branchRef") - -const gitFileSystemService = new GitFileSystemService( - new SimpleGit({ maxConcurrentProcesses: MAX_CONCURRENT_GIT_PROCESSES }) -) - -const handleGitFileLock = async (repoName, next) => { - const result = await gitFileSystemService.hasGitFileLock(repoName) - if (result.isErr()) { - next(result.err) - return false - } - const isGitLocked = result.value - if (isGitLocked) { - logger.error(`Failed to lock repo ${repoName}: git file system in use.`) - next( - new LockedError( - `Someone else is currently modifying repo ${repoName}. Please try again later.` - ) - ) - return false - } - return true -} - -// Used when there are no write API calls to the repo on GitHub -const attachReadRouteHandlerWrapper = (routeHandler) => async ( - req, - res, - next -) => { - routeHandler(req, res).catch((err) => { - next(err) - }) -} - -// Used when there are write API calls to the repo on GitHub -const attachWriteRouteHandlerWrapper = (routeHandler) => async ( - req, - res, - next -) => { - const { siteName } = req.params - const { growthbook } = req - - let isGitAvailable = true - - // only check git file lock if the repo is ggs enabled - if (growthbook?.getFeatureValue(FEATURE_FLAGS.IS_GGS_ENABLED, false)) { - isGitAvailable = await handleGitFileLock(siteName, next) - } - - if (!isGitAvailable) return - - try { - await lock(siteName) - } catch (err) { - next(err) - return - } - - await routeHandler(req, res, next).catch(async (err) => { - await unlock(siteName) - next(err) - }) - - try { - await unlock(siteName) - } catch (err) { - next(err) - } -} - -const attachRollbackRouteHandlerWrapper = (routeHandler) => async ( - req, - res, - next -) => { - const { userSessionData } = res.locals - const { siteName } = req.params - - const { accessToken } = userSessionData - const { growthbook } = req - - const shouldUseGitFileSystem = !!growthbook?.getFeatureValue( - FEATURE_FLAGS.IS_GGS_ENABLED, - false - ) - - const isGitAvailable = await handleGitFileLock(siteName, next) - if (!isGitAvailable) return - try { - await lock(siteName) - } catch (err) { - next(err) - return - } - - let originalCommitSha - if (shouldUseGitFileSystem) { - const result = await gitFileSystemService.getLatestCommitOfBranch( - siteName, - BRANCH_REF - ) - if (result.isErr()) { - await unlock(siteName) - next(result.err) - return - } - originalCommitSha = result.value.sha - if (!originalCommitSha) { - await unlock(siteName) - next(result.err) - return - } - // Unused for git file system, but to maintain existing structure - res.locals.githubSessionData = new GithubSessionData({ - currentCommitSha: "", - treeSha: "", - }) - } else { - try { - const { currentCommitSha, treeSha } = await getCommitAndTreeSha( - siteName, - accessToken - ) - - const githubSessionData = new GithubSessionData({ - currentCommitSha, - treeSha, - }) - res.locals.githubSessionData = githubSessionData - - originalCommitSha = currentCommitSha - } catch (err) { - await unlock(siteName) - next(err) - return - } - } - await routeHandler(req, res, next).catch(async (err) => { - try { - if (shouldUseGitFileSystem) { - await backOff(() => { - const rollbackRes = gitFileSystemService - .rollback(siteName, originalCommitSha) - .unwrapOr(false) - if (!rollbackRes) throw new GitFileSystemError("Rollback failure") - }) - await backOff(() => { - const pushRes = gitFileSystemService - .push(siteName, true) - .unwrapOr(false) - if (!pushRes) throw new GitFileSystemError("Push failure") - }) - } else { - await backOff(() => { - revertCommit(originalCommitSha, siteName, accessToken) - }) - } - } catch (retryErr) { - await unlock(siteName) - next(retryErr) - return - } - await unlock(siteName) - next(err) - }) - - try { - await unlock(siteName) - } catch (err) { - next(err) - } -} - -module.exports = { - attachReadRouteHandlerWrapper, - attachWriteRouteHandlerWrapper, - attachRollbackRouteHandlerWrapper, -} diff --git a/src/middleware/routeHandler.ts b/src/middleware/routeHandler.ts new file mode 100644 index 000000000..46d9352a0 --- /dev/null +++ b/src/middleware/routeHandler.ts @@ -0,0 +1,284 @@ +import { GrowthBook } from "@growthbook/growthbook" +import { BackoffOptions, backOff } from "exponential-backoff" +import { ResultAsync } from "neverthrow" +import simpleGit from "simple-git" + +import { config } from "@config/config" + +import GithubSessionData from "@classes/GithubSessionData" + +import { lock, unlock } from "@utils/mutex-utils" +import { getCommitAndTreeSha, revertCommit } from "@utils/utils.js" + +import { + MAX_CONCURRENT_GIT_PROCESSES, + STAGING_BRANCH, + STAGING_LITE_BRANCH, +} from "@constants/constants" + +import { FEATURE_FLAGS } from "@root/constants/featureFlags" +import LockedError from "@root/errors/LockedError" +import logger from "@root/logger/logger" +import { FeatureFlags } from "@root/types/featureFlags" +import convertNeverThrowToPromise from "@root/utils/neverthrow" +import GitFileSystemService from "@services/db/GitFileSystemService" + +const BRANCH_REF = config.get("github.branchRef") + +const backoffOptions: BackoffOptions = { + numOfAttempts: 5, +} +const simpleGitInstance = simpleGit({ + maxConcurrentProcesses: MAX_CONCURRENT_GIT_PROCESSES, +}) +const gitFileSystemService = new GitFileSystemService(simpleGitInstance) + +const handleGitFileLock = async ( + repoName: string, + next: (arg0: any) => void +) => { + const result = await gitFileSystemService.hasGitFileLock(repoName, true) + if (result.isErr()) { + next(result.error) + return false + } + const isGitLocked = result.value + if (isGitLocked) { + logger.error(`Failed to lock repo ${repoName}: git file system in use.`) + next( + new LockedError( + `Someone else is currently modifying repo ${repoName}. Please try again later.` + ) + ) + return false + } + return true +} + +const isGitFileAndIsGitAvail = async ( + growthbook: GrowthBook, + siteName: string, + next: any +) => { + let isGitAvailable = true + + // only check git file lock if the repo is ggs enabled + if (growthbook?.getFeatureValue(FEATURE_FLAGS.IS_GGS_ENABLED, false)) { + isGitAvailable = await handleGitFileLock(siteName, next) + } + return isGitAvailable +} + +// Used when there are no write API calls to the repo on GitHub +export const attachReadRouteHandlerWrapper = (routeHandler: any) => async ( + req: any, + res: any, + next: any +) => { + routeHandler(req, res).catch((err: any) => { + next(err) + }) +} + +// Used when there are write API calls to the repo on GitHub +export const attachWriteRouteHandlerWrapper = (routeHandler: any) => async ( + req: any, + res: any, + next: any +) => { + const { siteName } = req.params + const { growthbook } = req + + let isGitAvailable = true + + // only check git file lock if the repo is ggs enabled + if (growthbook?.getFeatureValue(FEATURE_FLAGS.IS_GGS_ENABLED, false)) { + isGitAvailable = await handleGitFileLock(siteName, next) + } + + if (!isGitAvailable) return + + try { + await lock(siteName) + } catch (err) { + next(err) + return + } + + await routeHandler(req, res, next).catch(async (err: any) => { + await unlock(siteName) + next(err) + }) + + try { + await unlock(siteName) + } catch (err) { + next(err) + } +} + +export const attachRollbackRouteHandlerWrapper = (routeHandler: any) => async ( + req: any, + res: any, + next: any +) => { + const { userSessionData } = res.locals + const { siteName } = req.params + + const { accessToken } = userSessionData + const { growthbook } = req + + const isGitAvailable = await isGitFileAndIsGitAvail( + growthbook, + siteName, + next + ) + + if (!isGitAvailable) return + try { + await lock(siteName) + } catch (err) { + next(err) + return + } + + let originalStagingCommitSha: string + let originalStagingLiteCommitSha: string + + const shouldUseGitFileSystem = growthbook?.getFeatureValue( + FEATURE_FLAGS.IS_GGS_ENABLED, + false + ) + if (shouldUseGitFileSystem) { + const results = await ResultAsync.combine([ + gitFileSystemService.getLatestCommitOfBranch(siteName, STAGING_BRANCH), + gitFileSystemService.getLatestCommitOfBranch( + siteName, + STAGING_LITE_BRANCH + ), + ]) + + if (results.isErr()) { + await unlock(siteName) + next(results.error) + return + } + const [stagingResult, stagingLiteResult] = results.value + if (!stagingResult.sha || !stagingLiteResult.sha) { + await unlock(siteName) + return + } + + originalStagingCommitSha = stagingResult.sha + originalStagingLiteCommitSha = stagingLiteResult.sha + // Unused for git file system, but to maintain existing structure + res.locals.githubSessionData = new GithubSessionData({ + currentCommitSha: "", + treeSha: "", + }) + } else { + try { + const { + currentCommitSha: currentStgCommitSha, + treeSha: stgTreeSha, + } = await getCommitAndTreeSha(siteName, accessToken, STAGING_BRANCH) + + const { + currentCommitSha: currentStgLiteCommitSha, + } = await getCommitAndTreeSha(siteName, accessToken, STAGING_LITE_BRANCH) + + const githubSessionData = new GithubSessionData({ + currentCommitSha: currentStgCommitSha, + treeSha: stgTreeSha, + }) + res.locals.githubSessionData = githubSessionData + + originalStagingCommitSha = currentStgCommitSha + originalStagingLiteCommitSha = currentStgLiteCommitSha + } catch (err) { + await unlock(siteName) + logger.error(`Failed to rollback repo ${siteName}: ${err}`) + next(err) + return + } + } + + await routeHandler(req, res, next).catch(async (err: any) => { + try { + if (shouldUseGitFileSystem) { + await backOff( + () => + convertNeverThrowToPromise( + gitFileSystemService.rollback( + siteName, + originalStagingCommitSha, + STAGING_BRANCH + ) + ), + backoffOptions + ) + + await backOff( + () => + convertNeverThrowToPromise( + gitFileSystemService.rollback( + siteName, + originalStagingLiteCommitSha, + STAGING_LITE_BRANCH + ) + ), + backoffOptions + ) + + await backOff(() => { + let pushRes = gitFileSystemService.push( + siteName, + STAGING_BRANCH, + true + ) + if (originalStagingLiteCommitSha) { + pushRes = pushRes.andThen(() => + gitFileSystemService.push(siteName, STAGING_LITE_BRANCH, true) + ) + } + + return convertNeverThrowToPromise(pushRes) + }, backoffOptions) + } else { + await backOff( + () => + revertCommit( + originalStagingCommitSha, + siteName, + accessToken, + STAGING_BRANCH + ), + backoffOptions + ) + await backOff( + () => + revertCommit( + originalStagingLiteCommitSha, + siteName, + accessToken, + STAGING_LITE_BRANCH + ), + backoffOptions + ) + } + } catch (retryErr) { + await unlock(siteName) + logger.error(`Failed to rollback repo ${siteName}: ${retryErr}`) + next(retryErr) + return + } + await unlock(siteName) + next(err) + }) + + try { + await unlock(siteName) + } catch (err) { + next(err) + } +} diff --git a/src/routes/formsg/formsgGGsRepair.ts b/src/routes/formsg/formsgGGsRepair.ts index c216879a4..869902145 100644 --- a/src/routes/formsg/formsgGGsRepair.ts +++ b/src/routes/formsg/formsgGGsRepair.ts @@ -116,7 +116,7 @@ export class FormsgGGsRepairRouter { const clonedStagingRepos: string[] = [] const syncedStagingAndStagingLiteRepos: string[] = [] repoNames.forEach((repoName) => { - const repoUrl = `git@github.com:isomerpages/${repoName}` + const repoUrl = `git@github.com:isomerpages/${repoName}.git` repairs.push( this.doesRepoNeedClone(repoName) @@ -230,7 +230,7 @@ ${syncedRepos.map((repo) => `
  • ${repo}
    • `)} doesRepoNeedClone(repoName: string): ResultAsync { return this.gitFileSystemService - .isGitInitialized(repoName) + .isGitInitialized(repoName, true) .andThen((isRepoInEfs) => { if (isRepoInEfs) { return errAsync(false) diff --git a/src/routes/v2/authenticatedSites/contactUs.js b/src/routes/v2/authenticatedSites/contactUs.js index b146aa1f9..2da7c57dc 100644 --- a/src/routes/v2/authenticatedSites/contactUs.js +++ b/src/routes/v2/authenticatedSites/contactUs.js @@ -6,7 +6,7 @@ const { BadRequestError } = require("@errors/BadRequestError") const { attachReadRouteHandlerWrapper, - attachWriteRouteHandlerWrapper, + attachRollbackRouteHandlerWrapper, } = require("@middleware/routeHandler") const { UpdateContactUsSchema } = require("@validators/RequestSchema") @@ -53,7 +53,7 @@ class ContactUsRouter { const router = express.Router({ mergeParams: true }) router.get("/", attachReadRouteHandlerWrapper(this.readContactUs)) - router.post("/", attachWriteRouteHandlerWrapper(this.updateContactUs)) + router.post("/", attachRollbackRouteHandlerWrapper(this.updateContactUs)) return router } diff --git a/src/routes/v2/authenticatedSites/navigation.js b/src/routes/v2/authenticatedSites/navigation.js index 291942a04..0092fa1b8 100644 --- a/src/routes/v2/authenticatedSites/navigation.js +++ b/src/routes/v2/authenticatedSites/navigation.js @@ -6,7 +6,7 @@ const { BadRequestError } = require("@errors/BadRequestError") const { attachReadRouteHandlerWrapper, - attachWriteRouteHandlerWrapper, + attachRollbackRouteHandlerWrapper, } = require("@middleware/routeHandler") const { UpdateNavigationRequestSchema } = require("@validators/RequestSchema") @@ -52,7 +52,7 @@ class NavigationRouter { const router = express.Router({ mergeParams: true }) router.get("/", attachReadRouteHandlerWrapper(this.readNavigation)) - router.post("/", attachWriteRouteHandlerWrapper(this.updateNavigation)) + router.post("/", attachRollbackRouteHandlerWrapper(this.updateNavigation)) return router } diff --git a/src/routes/v2/authenticatedSites/settings.js b/src/routes/v2/authenticatedSites/settings.js index 3e9f2a1f1..4005a3977 100644 --- a/src/routes/v2/authenticatedSites/settings.js +++ b/src/routes/v2/authenticatedSites/settings.js @@ -6,8 +6,8 @@ const { BadRequestError } = require("@errors/BadRequestError") // Import middleware const { - attachReadRouteHandlerWrapper, attachWriteRouteHandlerWrapper, + attachReadRouteHandlerWrapper, attachRollbackRouteHandlerWrapper, } = require("@middleware/routeHandler") diff --git a/src/server.js b/src/server.js index 73027b686..302e10ea9 100644 --- a/src/server.js +++ b/src/server.js @@ -79,7 +79,6 @@ import { SgidAuthRouter } from "./routes/v2/sgidAuth" import RepoManagementService from "./services/admin/RepoManagementService" import GitFileCommitService from "./services/db/GitFileCommitService" import GitFileSystemService from "./services/db/GitFileSystemService" -import GitHubCommitService from "./services/db/GithubCommitService" import RepoService from "./services/db/RepoService" import { PageService } from "./services/fileServices/MdPageServices/PageService" import { ConfigService } from "./services/fileServices/YmlFileServices/ConfigService" @@ -170,14 +169,12 @@ const simpleGitInstance = new simpleGit({ }) const gitFileSystemService = new GitFileSystemService(simpleGitInstance) -const gitHubCommitService = new GitHubCommitService(isomerRepoAxiosInstance) const gitFileCommitService = new GitFileCommitService(gitFileSystemService) const gitHubService = new RepoService({ isomerRepoAxiosInstance, gitFileSystemService, gitFileCommitService, - gitHubCommitService, }) const repoManagementService = new RepoManagementService({ diff --git a/src/services/db/GitFileCommitService.ts b/src/services/db/GitFileCommitService.ts index d89b47cf2..4dc640dff 100644 --- a/src/services/db/GitFileCommitService.ts +++ b/src/services/db/GitFileCommitService.ts @@ -1,6 +1,6 @@ import GithubSessionData from "@root/classes/GithubSessionData" import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" -import { STAGING_BRANCH } from "@root/constants" +import { STAGING_BRANCH, STAGING_LITE_BRANCH } from "@root/constants" import logger from "@root/logger/logger" import { GitCommitResult } from "@root/types/gitfilesystem" import isFileAsset from "@root/utils/commit-utils" @@ -14,14 +14,34 @@ import GitFileSystemService from "./GitFileSystemService" * 2. Creates non-asset related commits to staging-lite */ export default class GitFileCommitService { - private readonly STAGING_LITE_BRANCH = "staging-lite" - private readonly gitFileSystemService: GitFileSystemService constructor(gitFileSystemService: GitFileSystemService) { this.gitFileSystemService = gitFileSystemService } + async pushToGithub( + sessionData: UserWithSiteSessionData, + shouldUpdateStagingLite: boolean + ) { + // We await the push to staging FIRST, and then push to staging-lite + // We don't want a case when staging lite updates but staging doesn't + const res = this.gitFileSystemService.push( + sessionData.siteName, + STAGING_BRANCH + ) + + if (shouldUpdateStagingLite) { + res.andThen(() => + this.gitFileSystemService.push( + sessionData.siteName, + STAGING_LITE_BRANCH + ) + ) + } + return res + } + async create( sessionData: UserWithSiteSessionData, { @@ -36,51 +56,43 @@ export default class GitFileCommitService { isMedia?: boolean } ): Promise<{ sha: string }> { - const createPromises = [ - this.gitFileSystemService.create( + const stagingCreateResult = await this.gitFileSystemService.create( + sessionData.siteName, + sessionData.isomerUserId, + content, + directoryName, + fileName, + isMedia ? "base64" : "utf-8", + STAGING_BRANCH + ) + const shouldUpdateStagingLite = + isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && + !isFileAsset({ directoryName, fileName }) + + let stagingLiteCreateResult + if (shouldUpdateStagingLite) { + stagingLiteCreateResult = await this.gitFileSystemService.create( sessionData.siteName, sessionData.isomerUserId, content, directoryName, fileName, isMedia ? "base64" : "utf-8", - STAGING_BRANCH - ), - ] - const shouldUpdateStagingLite = - isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && - !isFileAsset({ directoryName, fileName }) - - if (shouldUpdateStagingLite) { - createPromises.push( - this.gitFileSystemService.create( - sessionData.siteName, - sessionData.isomerUserId, - content, - directoryName, - fileName, - isMedia ? "base64" : "utf-8", - this.STAGING_LITE_BRANCH - ) + STAGING_LITE_BRANCH ) } - const [stagingCreateResult, stagingLiteCreateResult] = await Promise.all( - createPromises - ) if (stagingCreateResult.isErr()) { throw stagingCreateResult.error - } else if (shouldUpdateStagingLite && stagingLiteCreateResult.isErr()) { + } else if ( + shouldUpdateStagingLite && + stagingLiteCreateResult && + stagingLiteCreateResult.isErr() + ) { throw stagingLiteCreateResult.error } - this.gitFileSystemService.push(sessionData.siteName, STAGING_BRANCH) - if (shouldUpdateStagingLite) { - this.gitFileSystemService.push( - sessionData.siteName, - this.STAGING_LITE_BRANCH - ) - } + this.pushToGithub(sessionData, shouldUpdateStagingLite) return { sha: stagingCreateResult.value.newSha } } @@ -101,51 +113,43 @@ export default class GitFileCommitService { const defaultBranch = STAGING_BRANCH logger.info("Updating file in local Git file system") const filePath = directoryName ? `${directoryName}/${fileName}` : fileName - const updatePromises = [ - this.gitFileSystemService.update( - sessionData.siteName, - filePath, - fileContent, - sha, - sessionData.isomerUserId, - defaultBranch - ), - ] + const stagingUpdateResult = await this.gitFileSystemService.update( + sessionData.siteName, + filePath, + fileContent, + sha, + sessionData.isomerUserId, + defaultBranch + ) const shouldUpdateStagingLite = - filePath && + !!filePath && isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && !isFileAsset({ fileName, directoryName }) + let stagingLiteUpdateResult if (shouldUpdateStagingLite) { - updatePromises.push( - this.gitFileSystemService.update( - sessionData.siteName, - filePath, - fileContent, - sha, - sessionData.isomerUserId, - this.STAGING_LITE_BRANCH - ) + stagingLiteUpdateResult = await this.gitFileSystemService.update( + sessionData.siteName, + filePath, + fileContent, + sha, + sessionData.isomerUserId, + STAGING_LITE_BRANCH ) } - const results = await Promise.all(updatePromises) - const [stagingUpdateResult, stagingLiteUpdateResult] = results - if (stagingUpdateResult.isErr()) { throw stagingUpdateResult.error - } else if (shouldUpdateStagingLite && stagingLiteUpdateResult.isErr()) { + } else if ( + shouldUpdateStagingLite && + stagingLiteUpdateResult && + stagingLiteUpdateResult.isErr() + ) { throw stagingLiteUpdateResult.error } - this.gitFileSystemService.push(sessionData.siteName, defaultBranch) - if (shouldUpdateStagingLite) { - this.gitFileSystemService.push( - sessionData.siteName, - this.STAGING_LITE_BRANCH - ) - } + this.pushToGithub(sessionData, shouldUpdateStagingLite) return { newSha: stagingUpdateResult.value } } @@ -161,50 +165,41 @@ export default class GitFileCommitService { logger.info( `Deleting directory in local Git file system for repo: ${sessionData.siteName}, directory name: ${directoryName}` ) - const deletePromises = [ - this.gitFileSystemService.delete( - sessionData.siteName, - directoryName, - "", - sessionData.isomerUserId, - true, - STAGING_BRANCH - ), - ] + const stagingDeleteResult = await this.gitFileSystemService.delete( + sessionData.siteName, + directoryName, + "", + sessionData.isomerUserId, + true, + defaultBranch + ) const shouldUpdateStagingLite = isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && !isFileAsset({ directoryName }) - + let stagingLiteDeleteResult if (shouldUpdateStagingLite) { - deletePromises.push( - this.gitFileSystemService.delete( - sessionData.siteName, - directoryName, - "", - sessionData.isomerUserId, - true, - this.STAGING_LITE_BRANCH - ) + stagingLiteDeleteResult = await this.gitFileSystemService.delete( + sessionData.siteName, + directoryName, + "", + sessionData.isomerUserId, + true, + STAGING_LITE_BRANCH ) } - const results = await Promise.all(deletePromises) - const [stagingDeleteResult, stagingLiteDeleteResult] = results - if (stagingDeleteResult.isErr()) { throw stagingDeleteResult.error - } else if (shouldUpdateStagingLite && stagingLiteDeleteResult.isErr()) { + } else if ( + shouldUpdateStagingLite && + stagingLiteDeleteResult && + stagingLiteDeleteResult.isErr() + ) { throw stagingLiteDeleteResult.error } - this.gitFileSystemService.push(sessionData.siteName, defaultBranch) - if (shouldUpdateStagingLite) { - this.gitFileSystemService.push( - sessionData.siteName, - this.STAGING_LITE_BRANCH - ) - } + this.pushToGithub(sessionData, shouldUpdateStagingLite) } async delete( @@ -222,55 +217,45 @@ export default class GitFileCommitService { logger.info( `Deleting file in local Git file system for repo: ${sessionData.siteName}, directory name: ${directoryName}, file name: ${fileName}` ) - const defaultBranch = STAGING_BRANCH const filePath = directoryName ? `${directoryName}/${fileName}` : fileName - const deletePromises = [ - this.gitFileSystemService.delete( - sessionData.siteName, - filePath, - sha, - sessionData.isomerUserId, - false, - STAGING_BRANCH - ), - ] + const stagingDeleteResult = await this.gitFileSystemService.delete( + sessionData.siteName, + filePath, + sha, + sessionData.isomerUserId, + false, + STAGING_BRANCH + ) const shouldUpdateStagingLite = isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && !isFileAsset({ directoryName }) + let stagingLiteDeleteResult if (shouldUpdateStagingLite) { - deletePromises.push( - this.gitFileSystemService.delete( - sessionData.siteName, - filePath, - sha, - sessionData.isomerUserId, - false, - this.STAGING_LITE_BRANCH - ) + stagingLiteDeleteResult = await this.gitFileSystemService.delete( + sessionData.siteName, + filePath, + sha, + sessionData.isomerUserId, + false, + STAGING_LITE_BRANCH ) } - const [stagingDeleteResult, stagingLiteDeleteResult] = await Promise.all( - deletePromises - ) - if (stagingDeleteResult.isErr()) { throw stagingDeleteResult.error - } else if (shouldUpdateStagingLite && stagingLiteDeleteResult.isErr()) { + } else if ( + shouldUpdateStagingLite && + stagingLiteDeleteResult && + stagingLiteDeleteResult.isErr() + ) { throw stagingLiteDeleteResult.error } - this.gitFileSystemService.push(sessionData.siteName, defaultBranch) - if (shouldUpdateStagingLite) { - this.gitFileSystemService.push( - sessionData.siteName, - this.STAGING_LITE_BRANCH - ) - } + this.pushToGithub(sessionData, shouldUpdateStagingLite) } async renameSinglePath( @@ -283,50 +268,38 @@ export default class GitFileCommitService { const defaultBranch = STAGING_BRANCH logger.info("Renaming file/directory in local Git file system") - const renamePromises = [ - this.gitFileSystemService.renameSinglePath( - sessionData.siteName, - oldPath, - newPath, - sessionData.isomerUserId, - defaultBranch, - message - ), - ] - - const shouldUpdateStagingLite = - isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && - !isFileAsset({ directoryName: oldPath }) - - if (shouldUpdateStagingLite) { - renamePromises.push( - this.gitFileSystemService.renameSinglePath( - sessionData.siteName, - oldPath, - newPath, - sessionData.isomerUserId, - this.STAGING_LITE_BRANCH, - message - ) - ) - } - - const results = await Promise.all(renamePromises) - const [stagingRenameResult, stagingLiteRenameResult] = results + const stagingRenameResult = await this.gitFileSystemService.renameSinglePath( + sessionData.siteName, + oldPath, + newPath, + sessionData.isomerUserId, + defaultBranch, + message + ) if (stagingRenameResult.isErr()) { throw stagingRenameResult.error - } else if (shouldUpdateStagingLite && stagingLiteRenameResult.isErr()) { - throw stagingLiteRenameResult.error } - this.gitFileSystemService.push(sessionData.siteName, defaultBranch) + const shouldUpdateStagingLite = + isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && + !isFileAsset({ directoryName: oldPath }) + if (shouldUpdateStagingLite) { - this.gitFileSystemService.push( + const stagingLiteRenameResult = await this.gitFileSystemService.renameSinglePath( sessionData.siteName, - this.STAGING_LITE_BRANCH + oldPath, + newPath, + sessionData.isomerUserId, + STAGING_LITE_BRANCH, + message ) + if (stagingLiteRenameResult.isErr()) { + throw stagingLiteRenameResult.error + } } + + this.pushToGithub(sessionData, shouldUpdateStagingLite) return { newSha: stagingRenameResult.value } } @@ -340,51 +313,39 @@ export default class GitFileCommitService { ): Promise { logger.info("Moving files in local Git file system") const defaultBranch = STAGING_BRANCH - const mvFilesResults = [ - this.gitFileSystemService.moveFiles( + const stagingMvFilesResult = await this.gitFileSystemService.moveFiles( + sessionData.siteName, + oldPath, + newPath, + sessionData.isomerUserId, + targetFiles, + defaultBranch, + message + ) + if (stagingMvFilesResult.isErr()) { + throw stagingMvFilesResult.error + } + + const shouldUpdateStagingLite = + isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && + !isFileAsset({ directoryName: oldPath }) + + if (shouldUpdateStagingLite) { + const stagingLiteMvFilesResult = await this.gitFileSystemService.moveFiles( sessionData.siteName, oldPath, newPath, sessionData.isomerUserId, targetFiles, - defaultBranch, + STAGING_LITE_BRANCH, message - ), - ] - const shouldUpdateStagingLite = - isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && - !isFileAsset({ directoryName: oldPath }) - - if (shouldUpdateStagingLite) { - mvFilesResults.push( - this.gitFileSystemService.moveFiles( - sessionData.siteName, - oldPath, - newPath, - sessionData.isomerUserId, - targetFiles, - this.STAGING_LITE_BRANCH, - message - ) ) + if (stagingLiteMvFilesResult.isErr()) { + throw stagingLiteMvFilesResult.error + } } - const results = await Promise.all(mvFilesResults) - const [stagingMvFilesResult, stagingLiteMvFilesResult] = results - - if (stagingMvFilesResult.isErr()) { - throw stagingMvFilesResult.error - } else if (shouldUpdateStagingLite && stagingLiteMvFilesResult.isErr()) { - throw stagingLiteMvFilesResult.error - } - - this.gitFileSystemService.push(sessionData.siteName, defaultBranch) - if (shouldUpdateStagingLite) { - this.gitFileSystemService.push( - sessionData.siteName, - this.STAGING_LITE_BRANCH - ) - } + this.pushToGithub(sessionData, shouldUpdateStagingLite) return { newSha: stagingMvFilesResult.value } } } diff --git a/src/services/db/GitFileSystemService.ts b/src/services/db/GitFileSystemService.ts index 9c8b42a9b..348dd1ad2 100644 --- a/src/services/db/GitFileSystemService.ts +++ b/src/services/db/GitFileSystemService.ts @@ -46,16 +46,20 @@ export default class GitFileSystemService { this.git = git } - private getEfsVolPathFromBranch(branchName: string) { + private getEfsVolPathFromBranch(branchName: string): string { return branchName === STAGING_LITE_BRANCH ? EFS_VOL_PATH_STAGING_LITE : EFS_VOL_PATH_STAGING } - private getEfsVolPath(isStaging: boolean) { + private getEfsVolPath(isStaging: boolean): string { return isStaging ? EFS_VOL_PATH_STAGING : EFS_VOL_PATH_STAGING_LITE } + private isStagingFromBranchName(branchName: string): boolean { + return branchName !== STAGING_LITE_BRANCH + } + /** * NOTE: We can do concurrent writes to the staging branch and the staging lite branch * since they exist in different folders. @@ -97,7 +101,7 @@ export default class GitFileSystemService { isGitInitialized( repoName: string, - isStaging = true + isStaging: boolean ): ResultAsync { const repoPath = isStaging ? `${EFS_VOL_PATH_STAGING}/${repoName}` @@ -122,7 +126,7 @@ export default class GitFileSystemService { isOriginRemoteCorrect( repoName: string, - isStaging = true + isStaging: boolean ): ResultAsync { const originUrl = `git@github.com:${ISOMER_GITHUB_ORG_NAME}/${repoName}.git` const repoPath = isStaging @@ -149,11 +153,9 @@ export default class GitFileSystemService { repoName: string, branchName: string ): ResultAsync { - return this.getFilePathStats( - repoName, - "", - branchName !== STAGING_LITE_BRANCH - ) + const isStaging = this.isStagingFromBranchName(branchName) + + return this.getFilePathStats(repoName, "", isStaging) .andThen((stats) => { if (!stats.isDirectory()) { // Return as an error to prevent further processing @@ -168,14 +170,14 @@ export default class GitFileSystemService { } return err(error) }) - .andThen(() => this.isGitInitialized(repoName)) + .andThen(() => this.isGitInitialized(repoName, isStaging)) .andThen((isGitInitialized) => { if (!isGitInitialized) { return err(false) } return ok(true) }) - .andThen(() => this.isOriginRemoteCorrect(repoName)) + .andThen(() => this.isOriginRemoteCorrect(repoName, isStaging)) .andThen((isOriginRemoteCorrect) => { if (!isOriginRemoteCorrect) { return err(false) @@ -235,7 +237,7 @@ export default class GitFileSystemService { getGitBlobHash( repoName: string, filePath: string, - isStaging = true + isStaging: boolean ): ResultAsync { const efsVolPath = isStaging ? EFS_VOL_PATH_STAGING @@ -403,7 +405,7 @@ export default class GitFileSystemService { }).map(() => `${efsVolPath}/${repoName}`) } - return this.isGitInitialized(repoName) + return this.isGitInitialized(repoName, isStaging) .andThen((isGitInitialized) => { if (!isGitInitialized) { return errAsync( @@ -416,7 +418,7 @@ export default class GitFileSystemService { } return okAsync(true) }) - .andThen(() => this.isOriginRemoteCorrect(repoName)) + .andThen(() => this.isOriginRemoteCorrect(repoName, isStaging)) .andThen((isOriginRemoteCorrect) => { if (!isOriginRemoteCorrect) { return errAsync( @@ -540,10 +542,33 @@ export default class GitFileSystemService { isForce ? this.git .cwd({ path: `${efsVolPath}/${repoName}`, root: false }) - .push(["--force"]) + .push([...gitOptions, "--force"]) + : this.git + .cwd({ path: `${efsVolPath}/${repoName}`, root: false }) + .push(gitOptions), + (error) => { + logger.error(`Error when pushing ${repoName}: ${error}`) + + if (error instanceof GitError) { + return new GitFileSystemError( + "Unable to push latest changes of repo" + ) + } + + return new GitFileSystemError("An unknown error occurred") + } + ) + ) + .orElse(() => + // Retry push twice + ResultAsync.fromPromise( + isForce + ? this.git + .cwd({ path: `${efsVolPath}/${repoName}`, root: false }) + .push([...gitOptions, "--force"]) : this.git .cwd({ path: `${efsVolPath}/${repoName}`, root: false }) - .push(), + .push(gitOptions), (error) => { logger.error(`Error when pushing ${repoName}: ${error}`) @@ -791,7 +816,7 @@ export default class GitFileSystemService { return new GitFileSystemError("An unknown error occurred") } ), - this.getGitBlobHash(repoName, filePath), + this.getGitBlobHash(repoName, filePath, true), ]).map((contentAndHash) => { const [content, sha] = contentAndHash const result: GitFile = { @@ -858,7 +883,7 @@ export default class GitFileSystemService { mediaUrl: `${dataUrlPrefix},${file.content}`, mediaPath: `${directoryName}/${fileName}`, type: fileType, - addedTime: stats.ctimeMs, + addedTime: stats.birthtimeMs, size: stats.size, }) }) @@ -871,6 +896,7 @@ export default class GitFileSystemService { branchName: string ): ResultAsync { const efsVolPath = this.getEfsVolPathFromBranch(branchName) + const isStaging = this.isStagingFromBranchName(branchName) return this.getFilePathStats( repoName, directoryPath, @@ -909,7 +935,7 @@ export default class GitFileSystemService { const path = directoryPath === "" ? name : `${directoryPath}/${name}` const type = isDirectory ? "dir" : "file" - return this.getGitBlobHash(repoName, path) + return this.getGitBlobHash(repoName, path, isStaging) .orElse(() => okAsync("")) .andThen((sha) => ResultAsync.combine([ @@ -929,7 +955,7 @@ export default class GitFileSystemService { sha, path, size: type === "dir" ? 0 : stats.size, - addedTime: stats.ctimeMs, + addedTime: stats.birthtimeMs, } return okAsync(result) @@ -955,6 +981,7 @@ export default class GitFileSystemService { ): ResultAsync { let oldStateSha = "" const efsVolPath = this.getEfsVolPathFromBranch(branchName) + const isStaging = this.isStagingFromBranchName(branchName) return this.getLatestCommitOfBranch(repoName, branchName) .andThen((latestCommit) => { // It is guaranteed that the latest commit contains the SHA hash @@ -979,7 +1006,7 @@ export default class GitFileSystemService { return okAsync(true) }) .andThen(() => - this.getGitBlobHash(repoName, filePath).andThen((sha) => { + this.getGitBlobHash(repoName, filePath, isStaging).andThen((sha) => { if (sha !== oldSha) { return errAsync( new ConflictError( @@ -1044,6 +1071,8 @@ export default class GitFileSystemService { ): ResultAsync { let oldStateSha = "" const efsVolPath = this.getEfsVolPathFromBranch(branchName) + const isStaging = this.isStagingFromBranchName(branchName) + return this.getLatestCommitOfBranch(repoName, branchName) .andThen((latestCommit) => { if (!latestCommit.sha) { @@ -1084,7 +1113,7 @@ export default class GitFileSystemService { if (isDir) { return okAsync(true) // If it's a directory, skip the blob hash verification } - return this.getGitBlobHash(repoName, path).andThen((sha) => { + return this.getGitBlobHash(repoName, path, isStaging).andThen((sha) => { if (sha !== oldSha) { return errAsync( new ConflictError( diff --git a/src/services/db/GitHubService.ts b/src/services/db/GitHubService.ts index 285183a80..c8c7d1f46 100644 --- a/src/services/db/GitHubService.ts +++ b/src/services/db/GitHubService.ts @@ -10,7 +10,7 @@ import { isAxiosError, validateStatus } from "@utils/axios-utils" import GithubSessionData from "@root/classes/GithubSessionData" import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" -import { STAGING_BRANCH, STAGING_LITE_BRANCH } from "@root/constants" +import { STAGING_BRANCH } from "@root/constants" import logger from "@root/logger/logger" import { GitCommitResult } from "@root/types/gitfilesystem" import { RawGitTreeEntry } from "@root/types/github" @@ -132,13 +132,11 @@ export default class GitHubService { fileName, directoryName, isMedia = false, - branchName = STAGING_BRANCH, }: { content: string fileName: string directoryName: string isMedia: boolean - branchName: string } ) { const { accessToken, siteName, isomerUserId: userId } = sessionData @@ -185,7 +183,7 @@ export default class GitHubService { const params = { message, content: encodedContent, - branch: branchName, + branch: STAGING_BRANCH, } const resp = await this.axiosInstance.put(endpoint, params, { @@ -209,18 +207,14 @@ export default class GitHubService { async read( sessionData: UserWithSiteSessionData, - { - fileName, - directoryName, - branchName = STAGING_BRANCH, - }: { fileName: any; directoryName: any; branchName?: string } + { fileName, directoryName }: { fileName: any; directoryName: any } ) { const { accessToken } = sessionData const { siteName } = sessionData const endpoint = this.getFilePath({ siteName, fileName, directoryName }) const params = { - ref: branchName, + ref: STAGING_BRANCH, } const resp = await this.axiosInstance.get(endpoint, { @@ -241,10 +235,7 @@ export default class GitHubService { async readMedia( sessionData: UserWithSiteSessionData, - { - fileSha, - branchName = STAGING_BRANCH, - }: { fileSha: any; branchName?: string } + { fileSha }: { fileSha: string } ) { /** * Files that are bigger than 1 MB needs to be retrieved @@ -254,7 +245,7 @@ export default class GitHubService { const { accessToken } = sessionData const { siteName } = sessionData const params = { - ref: branchName, + ref: STAGING_BRANCH, } const blobEndpoint = this.getBlobPath({ siteName, fileSha }) @@ -277,17 +268,14 @@ export default class GitHubService { async readDirectory( sessionData: UserWithSiteSessionData, - { - directoryName, - branchName = STAGING_BRANCH, - }: { directoryName: any; branchName?: string } + { directoryName }: { directoryName: string } ) { const { accessToken } = sessionData const { siteName } = sessionData const endpoint = this.getFolderPath({ siteName, directoryName }) const params = { - ref: branchName, + ref: STAGING_BRANCH, } const resp = await this.axiosInstance.get(endpoint, { @@ -309,13 +297,11 @@ export default class GitHubService { sha, fileName, directoryName, - branchName, }: { fileContent: string sha: string fileName: string directoryName: string | undefined - branchName: string } ): Promise { const { accessToken, siteName, isomerUserId: userId } = sessionData @@ -344,7 +330,7 @@ export default class GitHubService { const params = { message, content: encodedNewContent, - branch: branchName, + branch: STAGING_BRANCH, sha: fileSha, } @@ -379,12 +365,10 @@ export default class GitHubService { sha, fileName, directoryName, - branchName = STAGING_BRANCH, }: { sha: string fileName: string directoryName: string - branchName?: string } ) { const { accessToken, siteName, isomerUserId: userId } = sessionData @@ -392,11 +376,10 @@ export default class GitHubService { const endpoint = this.getFilePath({ siteName, fileName, directoryName }) let fileSha = sha - if (!sha || branchName === STAGING_LITE_BRANCH) { + if (!sha) { const { sha: retrievedSha } = await this.read(sessionData, { fileName, directoryName, - branchName, }) fileSha = retrievedSha } @@ -408,7 +391,7 @@ export default class GitHubService { }) const params = { message, - branch: branchName, + branch: STAGING_BRANCH, sha: fileSha, } @@ -531,8 +514,7 @@ export default class GitHubService { async getTree( sessionData: UserWithSiteSessionData, githubSessionData: GithubSessionData, - { isRecursive }: any, - isStaging = true + { isRecursive }: any ): Promise { const { accessToken } = sessionData const { siteName } = sessionData @@ -540,7 +522,7 @@ export default class GitHubService { const url = `${siteName}/git/trees/${treeSha}` const params = { - ref: isStaging ? STAGING_BRANCH : STAGING_LITE_BRANCH, + ref: STAGING_BRANCH, recursive: false, } @@ -559,8 +541,7 @@ export default class GitHubService { async updateTree( sessionData: UserWithSiteSessionData, githubSessionData: GithubSessionData, - { gitTree, message }: { gitTree: any; message: any }, - isStaging: boolean + { gitTree, message }: { gitTree: any; message: any } ) { const { accessToken, siteName, isomerUserId: userId } = sessionData const { treeSha, currentCommitSha } = githubSessionData.getGithubState() @@ -604,16 +585,171 @@ export default class GitHubService { return newCommitSha } - async updateRepoState( + async deleteDirectory( sessionData: UserWithSiteSessionData, { - commitSha, - branchName = STAGING_BRANCH, - }: { commitSha: any; branchName?: string } + directoryName, + message, + githubSessionData, + }: { + directoryName: string + message: string + githubSessionData: GithubSessionData + } + ): Promise { + // GitHub flow + const gitTree = await this.getTree(sessionData, githubSessionData, { + isRecursive: true, + }) + + // Retrieve removed items and set their sha to null + const newGitTree = gitTree + .filter( + (item) => + item.path.startsWith(`${directoryName}/`) && item.type !== "tree" + ) + .map((item) => ({ + ...item, + sha: null, + })) + + const newCommitSha = await this.updateTree(sessionData, githubSessionData, { + gitTree: newGitTree, + message, + }) + + await this.updateRepoState(sessionData, { + commitSha: newCommitSha, + }) + } + + async moveFiles( + sessionData: UserWithSiteSessionData, + githubSessionData: GithubSessionData, + oldPath: string, + newPath: string, + targetFiles: string[], + message?: string + ): Promise { + const gitTree = await this.getTree(sessionData, githubSessionData, { + isRecursive: true, + }) + const newGitTree: any[] = [] + + gitTree.forEach((item: any) => { + if (item.path.startsWith(`${newPath}/`) && item.type !== "tree") { + const fileName = item.path + .split(`${newPath}/`) + .slice(1) + .join(`${newPath}/`) + if (targetFiles.includes(fileName)) { + // Conflicting file + throw new ConflictError("File already exists in target directory") + } + } + if (item.path.startsWith(`${oldPath}/`) && item.type !== "tree") { + const fileName = item.path + .split(`${oldPath}/`) + .slice(1) + .join(`${oldPath}/`) + if (targetFiles.includes(fileName)) { + // Add file to target directory + newGitTree.push({ + ...item, + path: `${newPath}/${fileName}`, + }) + // Delete old file + newGitTree.push({ + ...item, + sha: null, + }) + } + } + }) + + const newCommitSha = await this.updateTree(sessionData, githubSessionData, { + gitTree: newGitTree, + message, + }) + + await this.updateRepoState(sessionData, { + commitSha: newCommitSha, + }) + + return { newSha: newCommitSha } + } + + async renameSinglePath( + sessionData: UserWithSiteSessionData, + githubSessionData: GithubSessionData, + oldPath: string, + newPath: string, + message?: string + ): Promise { + const gitTree = await this.getTree(sessionData, githubSessionData, { + isRecursive: true, + }) + const newGitTree: any[] = [] + const isMovingDirectory = + gitTree.find((item: any) => item.path === oldPath)?.type === "tree" || + false + + gitTree.forEach((item: any) => { + if (isMovingDirectory) { + if (item.path === newPath && item.type === "tree") { + throw new ConflictError("Target directory already exists") + } else if (item.path === oldPath && item.type === "tree") { + // Rename old subdirectory to new name + newGitTree.push({ + ...item, + path: newPath, + }) + } else if ( + item.path.startsWith(`${oldPath}/`) && + item.type !== "tree" + ) { + // Delete old files + newGitTree.push({ + ...item, + sha: null, + }) + } + } else if (item.path === newPath && item.type !== "tree") { + throw new ConflictError("Target file already exists") + } else if (item.path === oldPath && item.type !== "tree") { + // Add file to new directory + newGitTree.push({ + ...item, + path: newPath, + }) + // Delete old file + newGitTree.push({ + ...item, + sha: null, + }) + } + }) + + const newCommitSha = await this.updateTree(sessionData, githubSessionData, { + gitTree: newGitTree, + message, + }) + await this.updateRepoState(sessionData, { + commitSha: newCommitSha, + }) + + return { newSha: newCommitSha } + } + + async updateRepoState( + sessionData: UserWithSiteSessionData, + { commitSha, branchName }: { commitSha: any; branchName?: string } ) { const { accessToken } = sessionData const { siteName } = sessionData - const refEndpoint = `${siteName}/git/refs/heads/${branchName}` + const refEndpoint = `${siteName}/git/refs/heads/${ + branchName || STAGING_BRANCH + }` const headers = { Authorization: `token ${accessToken}`, } diff --git a/src/services/db/GithubCommitService.ts b/src/services/db/GithubCommitService.ts deleted file mode 100644 index 102639c69..000000000 --- a/src/services/db/GithubCommitService.ts +++ /dev/null @@ -1,390 +0,0 @@ -import { AxiosCacheInstance } from "axios-cache-interceptor" - -import GithubSessionData from "@root/classes/GithubSessionData" -import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" -import { STAGING_BRANCH, STAGING_LITE_BRANCH } from "@root/constants" -import { ConflictError } from "@root/errors/ConflictError" -import { GitCommitResult } from "@root/types/gitfilesystem" -import isFileAsset from "@root/utils/commit-utils" -import { isReduceBuildTimesWhitelistedRepo } from "@root/utils/growthbook-utils" -import GitHubService from "@services/db/GitHubService" - -/** - * Responsibilities of this class - * 1. Creates all commits to staging - * 2. Creates non-asset related commits to staging-lite - */ -export default class GitHubCommitService extends GitHubService { - constructor(axiosInstance: AxiosCacheInstance) { - super({ axiosInstance }) - } - - async create( - sessionData: UserWithSiteSessionData, - { - content, - fileName, - directoryName, - isMedia = false, - }: { - content: string - fileName: string - directoryName: string - isMedia?: boolean - } - ): Promise<{ - sha: string - }> { - const createPromises = [ - super.create(sessionData, { - content, - fileName, - directoryName, - isMedia, - branchName: STAGING_BRANCH, - }), - ] - const shouldStagingLiteUpdate = - !isFileAsset({ directoryName, fileName }) && - isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) - - if (shouldStagingLiteUpdate) { - createPromises.push( - super.create(sessionData, { - content, - directoryName, - fileName, - isMedia, - branchName: STAGING_LITE_BRANCH, - }) - ) - } - - // We still await and throw error if the commit to staging-lite fails, - // but the caller only gets the `resultToStaging` commit returned - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const [resultToStaging, _] = await Promise.all(createPromises) - - return resultToStaging - } - - async update( - sessionData: UserWithSiteSessionData, - { - fileContent, - sha, - fileName, - directoryName, - }: { - fileContent: string - sha: string - fileName: string - directoryName: string | undefined - } - ): Promise { - const updatePromises = [ - super.update(sessionData, { - fileContent, - sha, - fileName, - directoryName, - branchName: STAGING_BRANCH, - }), - ] - const shouldStagingLiteUpdate = - !isFileAsset({ directoryName, fileName }) && - isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) - - if (shouldStagingLiteUpdate) { - updatePromises.push( - super.update(sessionData, { - fileContent, - sha, - fileName, - directoryName, - branchName: STAGING_LITE_BRANCH, - }) - ) - } - - // We still await and throw error if the commit to staging-lite fails, - // but the caller only gets the `resultToStaging` commit returned - const [resultToStaging, _] = await Promise.all(updatePromises) - - return resultToStaging - } - - async deleteDirectory( - sessionData: UserWithSiteSessionData, - { - directoryName, - message, - githubSessionData, - isStaging = true, - }: { - directoryName: string - message: string - githubSessionData: GithubSessionData - isStaging?: boolean - } - ): Promise { - // GitHub flow - const gitTree = await this.getTree( - sessionData, - githubSessionData, - { - isRecursive: true, - }, - isStaging - ) - - // Retrieve removed items and set their sha to null - const newGitTree = gitTree - .filter( - (item) => - item.path.startsWith(`${directoryName}/`) && item.type !== "tree" - ) - .map((item) => ({ - ...item, - sha: null, - })) - - const newCommitSha = await this.updateTree( - sessionData, - githubSessionData, - { - gitTree: newGitTree, - message, - }, - !!isStaging - ) - - const deletePromises = [ - this.updateRepoState(sessionData, { - commitSha: newCommitSha, - }), - ] - - if ( - isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && - !isFileAsset({ directoryName }) - ) { - deletePromises.push( - this.deleteDirectory(sessionData, { - directoryName, - message, - githubSessionData, - isStaging: false, - }) - ) - } - await Promise.all(deletePromises) - } - - async delete( - sessionData: UserWithSiteSessionData, - { - sha, - fileName, - directoryName, - }: { - sha: string - fileName: string - directoryName: string - } - ): Promise { - const deletePromises = [ - super.delete(sessionData, { - sha, - fileName, - directoryName, - }), - ] - const shouldStagingLiteUpdate = - !isFileAsset({ directoryName, fileName }) && - isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) - if (shouldStagingLiteUpdate) { - deletePromises.push( - super.delete(sessionData, { - sha, - fileName, - directoryName, - branchName: STAGING_LITE_BRANCH, - }) - ) - } - - await Promise.all(deletePromises) - } - - async renameSinglePath( - sessionData: UserWithSiteSessionData, - githubSessionData: GithubSessionData, - oldPath: string, - newPath: string, - message?: string, - isStaging = true - ): Promise { - const gitTree = await super.getTree( - sessionData, - githubSessionData, - { - isRecursive: true, - }, - !!isStaging - ) - const newGitTree: any[] = [] - const isMovingDirectory = - gitTree.find((item: any) => item.path === oldPath)?.type === "tree" || - false - - gitTree.forEach((item: any) => { - if (isMovingDirectory) { - if (item.path === newPath && item.type === "tree") { - throw new ConflictError("Target directory already exists") - } else if (item.path === oldPath && item.type === "tree") { - // Rename old subdirectory to new name - newGitTree.push({ - ...item, - path: newPath, - }) - } else if ( - item.path.startsWith(`${oldPath}/`) && - item.type !== "tree" - ) { - // Delete old files - newGitTree.push({ - ...item, - sha: null, - }) - } - } else if (item.path === newPath && item.type !== "tree") { - throw new ConflictError("Target file already exists") - } else if (item.path === oldPath && item.type !== "tree") { - // Add file to new directory - newGitTree.push({ - ...item, - path: newPath, - }) - // Delete old file - newGitTree.push({ - ...item, - sha: null, - }) - } - }) - - const newCommitSha = await super.updateTree( - sessionData, - githubSessionData, - { - gitTree: newGitTree, - message, - }, - !!isStaging - ) - await super.updateRepoState(sessionData, { - commitSha: newCommitSha, - }) - - const shouldStagingLiteUpdate = - isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && - !isFileAsset({ directoryName: oldPath }) - - if (shouldStagingLiteUpdate) { - // we await this call, but we do not need to return this result - await this.renameSinglePath( - sessionData, - githubSessionData, - oldPath, - newPath, - message, - false - ) - } - - return { newSha: newCommitSha } - } - - async moveFiles( - sessionData: UserWithSiteSessionData, - githubSessionData: GithubSessionData, - oldPath: string, - newPath: string, - targetFiles: string[], - message?: string, - isStaging = true - ): Promise { - const gitTree = await super.getTree( - sessionData, - githubSessionData, - { - isRecursive: true, - }, - isStaging - ) - const newGitTree: any[] = [] - - gitTree.forEach((item: any) => { - if (item.path.startsWith(`${newPath}/`) && item.type !== "tree") { - const fileName = item.path - .split(`${newPath}/`) - .slice(1) - .join(`${newPath}/`) - if (targetFiles.includes(fileName)) { - // Conflicting file - throw new ConflictError("File already exists in target directory") - } - } - if (item.path.startsWith(`${oldPath}/`) && item.type !== "tree") { - const fileName = item.path - .split(`${oldPath}/`) - .slice(1) - .join(`${oldPath}/`) - if (targetFiles.includes(fileName)) { - // Add file to target directory - newGitTree.push({ - ...item, - path: `${newPath}/${fileName}`, - }) - // Delete old file - newGitTree.push({ - ...item, - sha: null, - }) - } - } - }) - - const newCommitSha = await super.updateTree( - sessionData, - githubSessionData, - { - gitTree: newGitTree, - message, - }, - !!isStaging - ) - - await super.updateRepoState(sessionData, { - commitSha: newCommitSha, - }) - - const shouldUpdateStagingLite = - isReduceBuildTimesWhitelistedRepo(sessionData.growthbook) && - !isFileAsset({ directoryName: oldPath }) - if (shouldUpdateStagingLite) { - // We don't have to return the sha, just update this should be ok - await this.moveFiles( - sessionData, - githubSessionData, - oldPath, - newPath, - targetFiles, - message, - false - ) - } - return { newSha: newCommitSha } - } -} diff --git a/src/services/db/RepoService.ts b/src/services/db/RepoService.ts index 7e386e04a..eacd93f07 100644 --- a/src/services/db/RepoService.ts +++ b/src/services/db/RepoService.ts @@ -20,7 +20,6 @@ import { getMediaFileInfo } from "@root/utils/media-utils" import GitFileCommitService from "./GitFileCommitService" import GitFileSystemService from "./GitFileSystemService" -import GitHubCommitService from "./GithubCommitService" import GitHubService from "./GitHubService" import * as ReviewApi from "./review" @@ -79,7 +78,6 @@ interface RepoServiceParams { isomerRepoAxiosInstance: AxiosCacheInstance gitFileSystemService: GitFileSystemService gitFileCommitService: GitFileCommitService - gitHubCommitService: GitHubCommitService } export default class RepoService extends GitHubService { @@ -87,18 +85,14 @@ export default class RepoService extends GitHubService { private readonly gitFileCommitService: GitFileCommitService - private readonly githubCommitService: GitHubCommitService - constructor({ isomerRepoAxiosInstance, gitFileSystemService, gitFileCommitService, - gitHubCommitService, }: RepoServiceParams) { super({ axiosInstance: isomerRepoAxiosInstance }) this.gitFileSystemService = gitFileSystemService this.gitFileCommitService = gitFileCommitService - this.githubCommitService = gitHubCommitService } getCommitDiff(siteName: string, base?: string, head?: string) { @@ -209,7 +203,7 @@ export default class RepoService extends GitHubService { isMedia, }) } - return this.githubCommitService.create(sessionData, { + return super.create(sessionData, { content, fileName, directoryName, @@ -369,9 +363,9 @@ export default class RepoService extends GitHubService { dirContent = result.value } else { - dirContent = (await super.readDirectory(sessionData, { + dirContent = await super.readDirectory(sessionData, { directoryName, - })) as GitDirectoryItem[] + }) } const { directories, files, total } = getPaginatedDirectoryContents( @@ -419,7 +413,7 @@ export default class RepoService extends GitHubService { }) } - return this.githubCommitService.update(sessionData, { + return super.update(sessionData, { fileContent, sha, fileName, @@ -451,7 +445,7 @@ export default class RepoService extends GitHubService { return } - await this.githubCommitService.deleteDirectory(sessionData, { + super.deleteDirectory(sessionData, { directoryName, message, githubSessionData, @@ -486,7 +480,7 @@ export default class RepoService extends GitHubService { } // GitHub flow - await this.githubCommitService.delete(sessionData, { + await super.delete(sessionData, { sha, fileName, directoryName, @@ -514,7 +508,7 @@ export default class RepoService extends GitHubService { message ) } - return this.githubCommitService.renameSinglePath( + return super.renameSinglePath( sessionData, githubSessionData, oldPath, @@ -547,7 +541,7 @@ export default class RepoService extends GitHubService { ) } - return this.githubCommitService.moveFiles( + return super.moveFiles( sessionData, githubSessionData, oldPath, @@ -604,18 +598,12 @@ export default class RepoService extends GitHubService { async updateTree( sessionData: any, githubSessionData: any, - { gitTree, message }: any, - isStaging: boolean + { gitTree, message }: any ): Promise { - return super.updateTree( - sessionData, - githubSessionData, - { - gitTree, - message, - }, - isStaging - ) + return super.updateTree(sessionData, githubSessionData, { + gitTree, + message, + }) } async updateRepoState( @@ -623,7 +611,7 @@ export default class RepoService extends GitHubService { { commitSha, branchName = BRANCH_REF, - }: { commitSha: string; branchName?: string } + }: { commitSha: string; branchName: string } ): Promise { const { siteName } = sessionData if ( @@ -646,7 +634,7 @@ export default class RepoService extends GitHubService { return } - await super.updateRepoState(sessionData, { commitSha, branchName }) + await super.updateRepoState(sessionData, { commitSha }) } async checkHasAccess(sessionData: any): Promise { diff --git a/src/services/db/__tests__/GitFileCommitService.spec.ts b/src/services/db/__tests__/GitFileCommitService.spec.ts new file mode 100644 index 000000000..3eb4914e7 --- /dev/null +++ b/src/services/db/__tests__/GitFileCommitService.spec.ts @@ -0,0 +1,625 @@ +import fs, { Stats } from "fs" + +import mockFs from "mock-fs" +import { ResultAsync, ok, okAsync } from "neverthrow" +import { GitError, SimpleGit } from "simple-git" + +import config from "@config/config" + +import { BadRequestError } from "@errors/BadRequestError" +import { ConflictError } from "@errors/ConflictError" +import GitFileSystemError from "@errors/GitFileSystemError" +import GitFileSystemNeedsRollbackError from "@errors/GitFileSystemNeedsRollbackError" +import { NotFoundError } from "@errors/NotFoundError" + +import { + EFS_VOL_PATH_STAGING, + EFS_VOL_PATH_STAGING_LITE, + ISOMER_GITHUB_ORG_NAME, + STAGING_BRANCH, + STAGING_LITE_BRANCH, +} from "@constants/constants" + +import { + MOCK_GITHUB_FILENAME_ALPHA_ONE, + MOCK_GITHUB_COMMIT_MESSAGE_ALPHA_ONE, +} from "@fixtures/github" +import { MOCK_USER_ID_ONE } from "@fixtures/users" +import { MediaTypeError } from "@root/errors/MediaTypeError" +import { + mockGithubSessionData, + mockUserWithSiteSessionData, + mockUserWithSiteSessionDataAndGrowthBook, +} from "@root/fixtures/sessionData" +import { MediaFileOutput } from "@root/types" +import { GitHubCommitData } from "@root/types/commitData" +import { GitDirectoryItem, GitFile } from "@root/types/gitfilesystem" +import _GitFileCommitService from "@services/db/GitFileCommitService" +import _GitFileSystemService from "@services/db/GitFileSystemService" + +import * as gbUtils from "../../../utils/growthbook-utils" + +const MockSimpleGit = { + clone: jest.fn(), + cwd: jest.fn(), +} + +const gitFileSystemService = new _GitFileSystemService( + (MockSimpleGit as unknown) as SimpleGit +) + +const gitFileCommitService = new _GitFileCommitService(gitFileSystemService) + +const BRANCH_REF = config.get("github.branchRef") +const DEFAULT_BRANCH = "staging" +const sessionData = mockUserWithSiteSessionData +describe("GitFileCommitService", () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + describe("pushToGithub", () => { + it("should push to staging and staging-lite", async () => { + // Arrange + const pushSpy = jest + .spyOn(gitFileSystemService, "push") + .mockImplementation((siteName, branchName) => + ResultAsync.fromPromise( + Promise.resolve("push ok"), + () => new GitFileSystemError("push failed") + ) + ) + + // Act + await gitFileCommitService.pushToGithub(sessionData, true) + + // Assert + expect(pushSpy).toHaveBeenCalledWith(sessionData.siteName, STAGING_BRANCH) + expect(pushSpy).toHaveBeenCalledWith( + sessionData.siteName, + STAGING_LITE_BRANCH + ) + }) + + it("should only push to staging if shouldUpdateStagingLite is false", async () => { + // Arrange + const pushSpy = jest + .spyOn(gitFileSystemService, "push") + .mockResolvedValue(okAsync("push ok")) + + // Act + await gitFileCommitService.pushToGithub(sessionData, false) + + // Assert + expect(pushSpy).toHaveBeenCalledWith(sessionData.siteName, STAGING_BRANCH) + expect(pushSpy).toHaveBeenCalledTimes(1) + }) + + describe("create", () => { + it("should create a file and push to GitHub", async () => { + // Arrange + + const content = "file content" + const fileName = "file.txt" + const directoryName = "directory" + const isMedia = false + + const createSpy = jest + .spyOn(gitFileSystemService, "create") + .mockResolvedValue(okAsync({ newSha: "new-sha" })) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(true) + // Act + const result = await gitFileCommitService.create(sessionData, { + content, + fileName, + directoryName, + isMedia, + }) + + // Assert + expect(createSpy).toHaveBeenCalledWith( + sessionData.siteName, + sessionData.isomerUserId, + content, + directoryName, + fileName, + "utf-8", + STAGING_BRANCH + ) + expect(createSpy).toHaveBeenCalledWith( + sessionData.siteName, + sessionData.isomerUserId, + content, + directoryName, + fileName, + "utf-8", + STAGING_LITE_BRANCH + ) + expect(pushSpy).toHaveBeenCalledWith(sessionData, expect.any(Boolean)) + expect(result).toEqual({ sha: "new-sha" }) + }) + + it("should create a file and commit only to staging branch when not whitelisted", async () => { + const content = "file content" + const fileName = "file.txt" + const directoryName = "directory" + const isMedia = false + + const createSpy = jest + .spyOn(gitFileSystemService, "create") + .mockResolvedValue(okAsync({ newSha: "new-sha" })) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(false) + + // Act + const result = await gitFileCommitService.create(sessionData, { + content, + fileName, + directoryName, + isMedia, + }) + + // Assert + expect(createSpy).toHaveBeenCalledWith( + sessionData.siteName, + sessionData.isomerUserId, + content, + directoryName, + fileName, + isMedia ? "base64" : "utf-8", + STAGING_BRANCH + ) + expect(createSpy).toHaveBeenCalledTimes(1) // create is called only once for the staging branch + expect(pushSpy).toHaveBeenCalledWith(sessionData, false) // push is called with shouldUpdateStagingLite as false + expect(result).toEqual({ sha: "new-sha" }) + }) + }) + + describe("update", () => { + it("should update a file and push to GitHub", async () => { + // Arrange + const fileContent = "updated file content" + const sha = "old-sha" + const fileName = "file.txt" + const directoryName = "directory" + + const updateSpy = jest + .spyOn(gitFileSystemService, "update") + .mockResolvedValue(okAsync("new-sha")) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(true) + + // Act + const result = await gitFileCommitService.update(sessionData, { + fileContent, + sha, + fileName, + directoryName, + }) + + // Assert + const filePath = directoryName + ? `${directoryName}/${fileName}` + : fileName + expect(updateSpy).toHaveBeenCalledWith( + sessionData.siteName, + filePath, + fileContent, + sha, + sessionData.isomerUserId, + STAGING_BRANCH + ) + + expect(updateSpy).toHaveBeenCalledWith( + sessionData.siteName, + filePath, + fileContent, + sha, + sessionData.isomerUserId, + STAGING_LITE_BRANCH + ) + expect(pushSpy).toHaveBeenCalledWith(sessionData, expect.any(Boolean)) + expect(result).toEqual({ newSha: "new-sha" }) + }) + + it("should update a file and commit only to staging branch when not whitelisted", async () => { + const fileContent = "updated file content" + const sha = "old-sha" + const fileName = "file.txt" + const directoryName = "directory" + + const updateSpy = jest + .spyOn(gitFileSystemService, "update") + .mockResolvedValue(okAsync("new-sha")) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(false) + + // Act + const result = await gitFileCommitService.update(sessionData, { + fileContent, + sha, + fileName, + directoryName, + }) + + // Assert + const filePath = directoryName + ? `${directoryName}/${fileName}` + : fileName + expect(updateSpy).toHaveBeenCalledWith( + sessionData.siteName, + filePath, + fileContent, + sha, + sessionData.isomerUserId, + STAGING_BRANCH + ) + expect(updateSpy).toHaveBeenCalledTimes(1) // update is called only once for the staging branch + expect(pushSpy).toHaveBeenCalledWith(sessionData, false) // push is called with shouldUpdateStagingLite as false + expect(result).toEqual({ newSha: "new-sha" }) + }) + }) + + describe("deleteDirectory", () => { + it("should delete a directory and push to GitHub", async () => { + // Arrange + const directoryName = "directory" + + const deleteSpy = jest + .spyOn(gitFileSystemService, "delete") + .mockResolvedValue(okAsync("")) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(true) + + // Act + await gitFileCommitService.deleteDirectory(sessionData, { + directoryName, + }) + + // Assert + expect(deleteSpy).toHaveBeenCalledWith( + sessionData.siteName, + directoryName, + "", + sessionData.isomerUserId, + true, + STAGING_BRANCH + ) + expect(deleteSpy).toHaveBeenCalledWith( + sessionData.siteName, + directoryName, + "", + sessionData.isomerUserId, + true, + STAGING_LITE_BRANCH + ) + expect(pushSpy).toHaveBeenCalledWith(sessionData, expect.any(Boolean)) + }) + + it("should delete a directory and commit only to staging branch when not whitelisted", async () => { + const directoryName = "directory" + + const deleteSpy = jest + .spyOn(gitFileSystemService, "delete") + .mockResolvedValue(okAsync("")) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(false) + + // Act + await gitFileCommitService.deleteDirectory(sessionData, { + directoryName, + }) + + // Assert + expect(deleteSpy).toHaveBeenCalledWith( + sessionData.siteName, + directoryName, + "", + sessionData.isomerUserId, + true, + STAGING_BRANCH + ) + expect(deleteSpy).toHaveBeenCalledTimes(1) // delete is called only once for the staging branch + expect(pushSpy).toHaveBeenCalledWith(sessionData, false) // push is called with shouldUpdateStagingLite as false + }) + }) + + describe("delete", () => { + it("should delete a file and push to GitHub", async () => { + // Arrange + const sha = "file-sha" + const fileName = "file.txt" + const directoryName = "directory" + + const deleteSpy = jest + .spyOn(gitFileSystemService, "delete") + .mockResolvedValue(okAsync("")) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(true) + + // Act + await gitFileCommitService.delete(sessionData, { + sha, + fileName, + directoryName, + }) + + // Assert + const filePath = directoryName + ? `${directoryName}/${fileName}` + : fileName + expect(deleteSpy).toHaveBeenCalledWith( + sessionData.siteName, + filePath, + sha, + sessionData.isomerUserId, + false, + STAGING_BRANCH + ) + expect(deleteSpy).toHaveBeenCalledWith( + sessionData.siteName, + filePath, + sha, + sessionData.isomerUserId, + false, + STAGING_LITE_BRANCH + ) + expect(pushSpy).toHaveBeenCalledWith(sessionData, expect.any(Boolean)) + }) + + it("should delete a file and commit only to staging branch when not whitelisted", async () => { + const sha = "file-sha" + const fileName = "file.txt" + const directoryName = "directory" + + const deleteSpy = jest + .spyOn(gitFileSystemService, "delete") + .mockResolvedValue(okAsync("")) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(false) + + // Act + await gitFileCommitService.delete(sessionData, { + sha, + fileName, + directoryName, + }) + + // Assert + const filePath = directoryName + ? `${directoryName}/${fileName}` + : fileName + expect(deleteSpy).toHaveBeenCalledWith( + sessionData.siteName, + filePath, + sha, + sessionData.isomerUserId, + false, + STAGING_BRANCH + ) + expect(deleteSpy).toHaveBeenCalledTimes(1) // delete is called only once for the staging branch + expect(pushSpy).toHaveBeenCalledWith(sessionData, false) // push is called with shouldUpdateStagingLite as false + }) + }) + + describe("renameSinglePath", () => { + it("should rename a file or directory and push to GitHub", async () => { + // Arrange + const oldPath = "old-path" + const newPath = "new-path" + const message = "rename message" + + const renameSpy = jest + .spyOn(gitFileSystemService, "renameSinglePath") + .mockResolvedValue(okAsync("new-sha")) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(true) + + // Act + const result = await gitFileCommitService.renameSinglePath( + sessionData, + mockGithubSessionData, + oldPath, + newPath, + message + ) + + // Assert + expect(renameSpy).toHaveBeenCalledWith( + sessionData.siteName, + oldPath, + newPath, + sessionData.isomerUserId, + STAGING_BRANCH, + message + ) + expect(renameSpy).toHaveBeenCalledWith( + sessionData.siteName, + oldPath, + newPath, + sessionData.isomerUserId, + STAGING_LITE_BRANCH, + message + ) + expect(pushSpy).toHaveBeenCalledWith(sessionData, expect.any(Boolean)) + expect(result).toEqual({ newSha: "new-sha" }) + }) + + it("should rename a file or directory and commit only to staging branch when not whitelisted", async () => { + const oldPath = "old-path" + const newPath = "new-path" + const message = "rename message" + + const renameSpy = jest + .spyOn(gitFileSystemService, "renameSinglePath") + .mockResolvedValue(okAsync("new-sha")) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(false) + + // Act + const result = await gitFileCommitService.renameSinglePath( + sessionData, + mockGithubSessionData, + oldPath, + newPath, + message + ) + + // Assert + expect(renameSpy).toHaveBeenCalledWith( + sessionData.siteName, + oldPath, + newPath, + sessionData.isomerUserId, + STAGING_BRANCH, + message + ) + expect(renameSpy).toHaveBeenCalledTimes(1) // renameSinglePath is called only once for the staging branch + expect(pushSpy).toHaveBeenCalledWith(sessionData, false) // push is called with shouldUpdateStagingLite as false + expect(result).toEqual({ newSha: "new-sha" }) + }) + }) + + describe("moveFiles", () => { + it("should move files and push to GitHub", async () => { + // Arrange + const oldPath = "old-path" + const newPath = "new-path" + const targetFiles = ["file1.txt", "file2.txt"] + const message = "move files message" + + const moveFilesSpy = jest + .spyOn(gitFileSystemService, "moveFiles") + .mockResolvedValue(okAsync("new-sha")) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(true) + + // Act + const result = await gitFileCommitService.moveFiles( + sessionData, + mockGithubSessionData, + oldPath, + newPath, + targetFiles, + message + ) + + // Assert + expect(moveFilesSpy).toHaveBeenCalledWith( + sessionData.siteName, + oldPath, + newPath, + sessionData.isomerUserId, + targetFiles, + STAGING_BRANCH, + message + ) + expect(moveFilesSpy).toHaveBeenCalledWith( + sessionData.siteName, + oldPath, + newPath, + sessionData.isomerUserId, + targetFiles, + STAGING_LITE_BRANCH, + message + ) + expect(pushSpy).toHaveBeenCalledWith(sessionData, expect.any(Boolean)) + expect(result).toEqual({ newSha: "new-sha" }) + }) + + it("should move files and commit only to staging branch when not whitelisted", async () => { + const oldPath = "old-path" + const newPath = "new-path" + const targetFiles = ["file1.txt", "file2.txt"] + const message = "move files message" + + const moveFilesSpy = jest + .spyOn(gitFileSystemService, "moveFiles") + .mockResolvedValue(okAsync("new-sha")) + const pushSpy = jest + .spyOn(gitFileCommitService, "pushToGithub") + .mockResolvedValue(ok("")) + + jest + .spyOn(gbUtils, "isReduceBuildTimesWhitelistedRepo") + .mockReturnValue(false) + + // Act + const result = await gitFileCommitService.moveFiles( + sessionData, + mockGithubSessionData, + oldPath, + newPath, + targetFiles, + message + ) + + // Assert + expect(moveFilesSpy).toHaveBeenCalledWith( + sessionData.siteName, + oldPath, + newPath, + sessionData.isomerUserId, + targetFiles, + STAGING_BRANCH, + message + ) + expect(moveFilesSpy).toHaveBeenCalledTimes(1) // moveFiles is called only once for the staging branch + expect(pushSpy).toHaveBeenCalledWith(sessionData, false) // push is called with shouldUpdateStagingLite as false + expect(result).toEqual({ newSha: "new-sha" }) + }) + }) + }) +}) diff --git a/src/services/db/__tests__/GitFileSystemService.spec.ts b/src/services/db/__tests__/GitFileSystemService.spec.ts index a6887c2c5..f7fa089bf 100644 --- a/src/services/db/__tests__/GitFileSystemService.spec.ts +++ b/src/services/db/__tests__/GitFileSystemService.spec.ts @@ -248,7 +248,10 @@ describe("GitFileSystemService", () => { checkIsRepo: jest.fn().mockResolvedValueOnce(true), }) - const result = await GitFileSystemService.isGitInitialized("fake-repo") + const result = await GitFileSystemService.isGitInitialized( + "fake-repo", + true + ) expect(result._unsafeUnwrap()).toBeTrue() }) @@ -258,7 +261,10 @@ describe("GitFileSystemService", () => { checkIsRepo: jest.fn().mockResolvedValueOnce(false), }) - const result = await GitFileSystemService.isGitInitialized("fake-repo") + const result = await GitFileSystemService.isGitInitialized( + "fake-repo", + true + ) expect(result._unsafeUnwrap()).toBeFalse() }) @@ -288,7 +294,8 @@ describe("GitFileSystemService", () => { }) const result = await GitFileSystemService.isOriginRemoteCorrect( - "fake-repo" + "fake-repo", + true ) expect(result._unsafeUnwrap()).toBeTrue() @@ -304,7 +311,8 @@ describe("GitFileSystemService", () => { }) const result = await GitFileSystemService.isOriginRemoteCorrect( - "fake-repo" + "fake-repo", + true ) expect(result._unsafeUnwrap()).toBeFalse() @@ -316,7 +324,8 @@ describe("GitFileSystemService", () => { }) const result = await GitFileSystemService.isOriginRemoteCorrect( - "fake-repo" + "fake-repo", + true ) expect(result._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) @@ -515,7 +524,8 @@ describe("GitFileSystemService", () => { const result = await GitFileSystemService.getGitBlobHash( "fake-repo", - "fake-dir/fake-file" + "fake-dir/fake-file", + true ) expect(result._unsafeUnwrap()).toBe("fake-hash") @@ -528,7 +538,8 @@ describe("GitFileSystemService", () => { const result = await GitFileSystemService.getGitBlobHash( "fake-repo", - "fake-dir/fake-file" + "fake-dir/fake-file", + true ) expect(result._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) @@ -890,6 +901,9 @@ describe("GitFileSystemService", () => { MockSimpleGit.cwd.mockReturnValueOnce({ push: jest.fn().mockRejectedValueOnce(new GitError()), }) + MockSimpleGit.cwd.mockReturnValueOnce({ + push: jest.fn().mockRejectedValueOnce(new GitError()), + }) const result = await GitFileSystemService.push("fake-repo", BRANCH_REF) diff --git a/src/services/db/__tests__/GitHubService.spec.ts b/src/services/db/__tests__/GitHubService.spec.ts index be83fe6da..75866351a 100644 --- a/src/services/db/__tests__/GitHubService.spec.ts +++ b/src/services/db/__tests__/GitHubService.spec.ts @@ -15,9 +15,12 @@ import { mockCurrentCommitSha, mockGithubSessionData, mockIsomerUserId, + gitTree, } from "@fixtures/sessionData" +import { STAGING_BRANCH, STAGING_LITE_BRANCH } from "@root/constants" import { indexHtmlContent } from "@root/fixtures/markdown-fixtures" import { collectionYmlContent } from "@root/fixtures/yaml-fixtures" +import { RawGitTreeEntry } from "@root/types/github" import GitHubService from "@services/db/GitHubService" // using es6 gives some error @@ -173,7 +176,6 @@ describe("Github Service", () => { fileName, directoryName, isMedia: false, - branchName: BRANCH_REF, }) ).resolves.toMatchObject({ sha, @@ -200,7 +202,6 @@ describe("Github Service", () => { fileName: topLevelDirectoryFileName, directoryName, isMedia: false, - branchName: BRANCH_REF, }) ).resolves.toMatchObject({ sha, @@ -228,7 +229,6 @@ describe("Github Service", () => { fileName: resourceCategoryFileName, directoryName, isMedia: false, - branchName: BRANCH_REF, }) ).resolves.toMatchObject({ sha, @@ -257,7 +257,6 @@ describe("Github Service", () => { fileName, directoryName, isMedia: true, - branchName: BRANCH_REF, }) ).resolves.toMatchObject({ sha, @@ -289,7 +288,6 @@ describe("Github Service", () => { fileName, directoryName, isMedia: false, - branchName: BRANCH_REF, }) ).rejects.toThrowError(ConflictError) expect(mockAxiosInstance.put).toHaveBeenCalledWith( @@ -309,7 +307,6 @@ describe("Github Service", () => { fileName, directoryName, isMedia: false, - branchName: BRANCH_REF, }) ).rejects.toThrowError(NotFoundError) expect(mockAxiosInstance.get).toHaveBeenCalledWith(folderParentEndpoint, { @@ -332,7 +329,6 @@ describe("Github Service", () => { fileName: subDirectoryFileName, directoryName: subDirectoryName, isMedia: false, - branchName: BRANCH_REF, }) ).rejects.toThrowError() expect(mockAxiosInstance.get).toHaveBeenCalledWith(fileParentEndpoint, { @@ -354,7 +350,6 @@ describe("Github Service", () => { fileName, directoryName: `${resourceCategoryName}/_posts`, isMedia: false, - branchName: BRANCH_REF, }) ).rejects.toThrowError(NotFoundError) expect(mockAxiosInstance.get).toHaveBeenCalledWith(resourceRoomEndpoint, { @@ -539,7 +534,6 @@ describe("Github Service", () => { directoryName, fileContent: content, sha, - branchName: BRANCH_REF, }) ).resolves.toMatchObject({ newSha: sha, @@ -568,7 +562,6 @@ describe("Github Service", () => { directoryName, fileContent: content, sha, - branchName: BRANCH_REF, }) ).rejects.toThrowError(NotFoundError) expect(mockAxiosInstance.put).toHaveBeenCalledWith( @@ -604,7 +597,6 @@ describe("Github Service", () => { directoryName, fileContent: content, sha: "", - branchName: BRANCH_REF, }) ).resolves.toMatchObject({ newSha: sha, @@ -636,7 +628,6 @@ describe("Github Service", () => { directoryName, fileContent: content, sha: "", - branchName: BRANCH_REF, }) ).rejects.toThrowError(NotFoundError) expect(mockAxiosInstance.get).toHaveBeenCalledWith(endpoint, { @@ -892,15 +883,10 @@ describe("Github Service", () => { .mockResolvedValueOnce(firstResp) .mockResolvedValueOnce(secondResp) await expect( - service.updateTree( - sessionData, - mockGithubSessionData, - { - gitTree, - message, - }, - true - ) + service.updateTree(sessionData, mockGithubSessionData, { + gitTree, + message, + }) ).resolves.toEqual(secondSha) expect(mockAxiosInstance.post).toHaveBeenCalledWith( url, @@ -998,4 +984,154 @@ describe("Github Service", () => { expect(resp.isOk()).toEqual(true) }) }) + + describe("deleteDirectory", () => { + const message = JSON.stringify({ + message: `Delete directory: ${directoryName}`, + directoryName, + userId, + }) + const params = { + message, + branch: BRANCH_REF, + sha: treeSha, + force: true, + } + + const endpoint = `${siteName}/git/refs/heads/${STAGING_BRANCH}` + const stagingLiteEndpoint = `${siteName}/git/refs/heads/${STAGING_LITE_BRANCH}` + + it("should delete a directory correctly", async () => { + // Arrange + const getTreeSpy = jest.spyOn(service, "getTree") + getTreeSpy.mockResolvedValueOnce(gitTree) + mockAxiosInstance.get.mockImplementation(() => + Promise.resolve({ data: gitTree }) + ) + const updateTreeSpy = jest.spyOn(service, "updateTree") + updateTreeSpy.mockResolvedValueOnce(treeSha) + // Act + await service.deleteDirectory(sessionData, { + directoryName, + message, + githubSessionData: mockGithubSessionData, + }) + + // Assert + expect(mockAxiosInstance.patch).toHaveBeenCalledWith( + endpoint, + { + force: true, + sha: treeSha, + }, + { + headers: { Authorization: `token ${accessToken}` }, + } + ) + }) + + it("should throw the correct error if directory cannot be found", async () => { + // Arrange + const getTreeSpy = jest.spyOn(service, "getTree") + getTreeSpy.mockResolvedValueOnce(gitTree) + const updateTreeSpy = jest.spyOn(service, "updateTree") + updateTreeSpy.mockResolvedValueOnce(treeSha) + mockAxiosInstance.patch.mockImplementation(() => { + const err = { + response: { + status: 404, + }, + isAxiosError: true, + } + throw err + }) + + // Act + await expect( + service.deleteDirectory(sessionData, { + directoryName, + message, + githubSessionData: mockGithubSessionData, + }) + ).rejects.toStrictEqual({ isAxiosError: true, response: { status: 404 } }) + + // Assert + expect(mockAxiosInstance.patch).toHaveBeenCalledWith( + endpoint, + { + force: true, + sha: "mockTreeSha", + }, + { + headers: authHeader.headers, + } + ) + }) + }) + + describe("renameSinglePath", () => { + it("should rename a file correctly", async () => { + // Arrange + + const oldPath = "old/path.txt" + const newPath = "new/path.txt" + const message = "Renaming file" + + const newGitTree: RawGitTreeEntry[] = [ + { + path: oldPath, + type: "file", + sha: "new-sha2", + mode: "100644", + url: "", + }, + ] + + const resolvedTree = [ + { + path: newPath, + type: "file", + sha: "new-sha2", + mode: "100644", + url: "", + }, + { + path: oldPath, + type: "file", + sha: null, + mode: "100644", + url: "", + }, + ] + const newCommitSha = "new-commit-sha" + jest.spyOn(service, "getTree").mockResolvedValueOnce(newGitTree) + jest.spyOn(service, "updateTree").mockResolvedValueOnce(newCommitSha) + jest.spyOn(service, "updateRepoState").mockResolvedValueOnce() + + // Act + const result = await service.renameSinglePath( + sessionData, + mockGithubSessionData, + oldPath, + newPath, + message + ) + + // Assert + expect(service.getTree).toHaveBeenCalledWith( + sessionData, + mockGithubSessionData, + { isRecursive: true } + ) + expect(service.updateTree).toHaveBeenCalledWith( + sessionData, + mockGithubSessionData, + { gitTree: resolvedTree, message } + ) + expect(service.updateRepoState).toHaveBeenCalledWith(sessionData, { + commitSha: newCommitSha, + }) + expect(result).toEqual({ newSha: newCommitSha }) + }) + }) }) diff --git a/src/services/db/__tests__/RepoService.spec.ts b/src/services/db/__tests__/RepoService.spec.ts index 7f482bf60..19f8b9fb7 100644 --- a/src/services/db/__tests__/RepoService.spec.ts +++ b/src/services/db/__tests__/RepoService.spec.ts @@ -8,7 +8,6 @@ import { mockGithubSessionData, mockGrowthBook, mockIsomerUserId, - mockSiteName, mockUserWithSiteSessionDataAndGrowthBook, } from "@fixtures/sessionData" import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" @@ -24,7 +23,6 @@ import GitFileSystemService from "@services/db/GitFileSystemService" import _RepoService from "@services/db/RepoService" import GitFileCommitService from "../GitFileCommitService" -import GitHubCommitService from "../GithubCommitService" import GitHubService from "../GitHubService" const MockAxiosInstance = { @@ -58,7 +56,7 @@ const MockGitFileCommitService = { moveFiles: jest.fn(), } -const MockGitHubCommitService = { +const MockGitHubService = { create: jest.fn(), update: jest.fn(), delete: jest.fn(), @@ -71,7 +69,6 @@ const RepoService = new _RepoService({ isomerRepoAxiosInstance: (MockAxiosInstance as unknown) as AxiosCacheInstance, gitFileSystemService: (MockGitFileSystemService as unknown) as GitFileSystemService, gitFileCommitService: (MockGitFileCommitService as unknown) as GitFileCommitService, - gitHubCommitService: (MockGitHubCommitService as unknown) as GitHubCommitService, }) describe("RepoService", () => { @@ -164,7 +161,7 @@ describe("RepoService", () => { ) }) - it("should create files on GitHub directly if the repo is not ggs enabled", async () => { + it("should create files on GitHub directly if the repo is not whitelisted", async () => { const mockContent = "content" const mockFileName = "test.md" const mockDirectoryName = "" @@ -179,17 +176,18 @@ describe("RepoService", () => { const expected = { sha: "test-sha", } - MockGitHubCommitService.create.mockResolvedValueOnce(expected) + const gitHubServiceCreate = jest.spyOn(GitHubService.prototype, "create") + gitHubServiceCreate.mockResolvedValueOnce(expected) const actual = await RepoService.create(sessionData, { - content: mockContent, - fileName: mockFileName, - directoryName: mockDirectoryName, + content: "content", + fileName: "test.md", + directoryName: "", isMedia, }) expect(actual).toEqual(expected) - expect(MockGitHubCommitService.create).toHaveBeenCalledWith(sessionData, { + expect(gitHubServiceCreate).toHaveBeenCalledWith(sessionData, { content: mockContent, fileName: mockFileName, directoryName: mockDirectoryName, @@ -568,9 +566,8 @@ describe("RepoService", () => { email: mockEmail, siteName: "not-whitelisted", }) - MockGitHubCommitService.update.mockResolvedValueOnce({ - newSha: expectedSha, - }) + const gitHubServiceUpdate = jest.spyOn(GitHubService.prototype, "update") + gitHubServiceUpdate.mockResolvedValueOnce({ newSha: expectedSha }) const actual = await RepoService.update(sessionData, { fileContent: "test content", @@ -616,200 +613,221 @@ describe("RepoService", () => { siteName: "not-whitelisted", }) + const mockedGitHubService = jest + .spyOn(GitHubService.prototype, "delete") + .mockResolvedValueOnce(undefined) + await RepoService.delete(sessionData, { sha: "fake-original-sha", fileName: "test.md", directoryName: "pages", }) - expect(MockGitHubCommitService.delete).toBeCalledTimes(1) - expect(MockGitHubCommitService.delete).toBeCalledWith(sessionData, { + expect(mockedGitHubService).toBeCalledWith(sessionData, { sha: "fake-original-sha", fileName: "test.md", directoryName: "pages", }) }) - }) - - describe("renameSinglePath", () => { - it("should rename using the local Git file system if the repo is ggs enabled", async () => { - const expected: GitCommitResult = { newSha: "fake-commit-sha" } - MockGitFileCommitService.renameSinglePath.mockResolvedValueOnce(expected) - gbSpy.mockReturnValueOnce(true) - const actual = await RepoService.renameSinglePath( - mockUserWithSiteSessionDataAndGrowthBook, - mockGithubSessionData, - "fake-old-path", - "fake-new-path", - "fake-commit-message" - ) - - expect(actual).toEqual(expected) - }) - - it("should rename file using GitHub directly if the repo is not ggs enabled", async () => { - const expectedSha = "fake-commit-sha" - const fakeCommitMessage = "fake-commit-message" - const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData({ - githubId: mockGithubId, - accessToken: mockAccessToken, - isomerUserId: mockIsomerUserId, - email: mockEmail, - siteName: "not-whitelisted", + describe("renameSinglePath", () => { + it("should rename using the local Git file system if the repo is ggs enabled", async () => { + const expected: GitCommitResult = { newSha: "fake-commit-sha" } + MockGitFileCommitService.renameSinglePath.mockResolvedValueOnce( + expected + ) + gbSpy.mockReturnValueOnce(true) + + const actual = await RepoService.renameSinglePath( + mockUserWithSiteSessionDataAndGrowthBook, + mockGithubSessionData, + "fake-old-path", + "fake-new-path", + "fake-commit-message" + ) + + expect(actual).toEqual(expected) }) - MockGitHubCommitService.renameSinglePath.mockResolvedValueOnce({ - newSha: expectedSha, + it("should rename file using GitHub directly if the repo is not ggs enabled", async () => { + const expectedSha = "fake-commit-sha" + const fakeCommitMessage = "fake-commit-message" + const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData( + { + githubId: mockGithubId, + accessToken: mockAccessToken, + isomerUserId: mockIsomerUserId, + email: mockEmail, + siteName: "not-whitelisted", + } + ) + + const gitHubServiceRenameSinglePath = jest.spyOn( + GitHubService.prototype, + "renameSinglePath" + ) + gitHubServiceRenameSinglePath.mockResolvedValueOnce({ + newSha: expectedSha, + }) + + const actual = await RepoService.renameSinglePath( + sessionData, + mockGithubSessionData, + "fake-path/old-fake-file.md", + "fake-path/new-fake-file.md", + fakeCommitMessage + ) + + expect(actual).toEqual({ newSha: expectedSha }) }) - - const actual = await RepoService.renameSinglePath( - sessionData, - mockGithubSessionData, - "fake-path/old-fake-file.md", - "fake-path/new-fake-file.md", - fakeCommitMessage - ) - - expect(actual).toEqual({ newSha: expectedSha }) }) - }) - describe("moveFiles", () => { - it("should move files using the Git local file system if the repo is ggs enabled", async () => { - const expected = { newSha: "fake-commit-sha" } - MockGitFileCommitService.moveFiles.mockResolvedValueOnce(expected) - gbSpy.mockReturnValueOnce(true) - // MockCommitServiceGitFile.push.mockReturnValueOnce(undefined) - - const actual = await RepoService.moveFiles( - mockUserWithSiteSessionDataAndGrowthBook, - mockGithubSessionData, - "fake-old-path", - "fake-new-path", - ["fake-file1", "fake-file2"], - "fake-commit-message" - ) - - expect(actual).toEqual(expected) - }) - - it("should move files using GitHub directly if the repo is not ggs enabled", async () => { - const expected = { newSha: "fake-commit-sha" } - const fakeCommitMessage = "fake-commit-message" - const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData({ - githubId: mockGithubId, - accessToken: mockAccessToken, - isomerUserId: mockIsomerUserId, - email: mockEmail, - siteName: "not-whitelisted", + describe("moveFiles", () => { + it("should move files using the Git local file system if the repo is ggs enabled", async () => { + const expected = { newSha: "fake-commit-sha" } + MockGitFileCommitService.moveFiles.mockResolvedValueOnce(expected) + gbSpy.mockReturnValueOnce(true) + // MockCommitServiceGitFile.push.mockReturnValueOnce(undefined) + + const actual = await RepoService.moveFiles( + mockUserWithSiteSessionDataAndGrowthBook, + mockGithubSessionData, + "fake-old-path", + "fake-new-path", + ["fake-file1", "fake-file2"], + "fake-commit-message" + ) + + expect(actual).toEqual(expected) }) - MockGitHubCommitService.moveFiles.mockResolvedValueOnce(expected) - - const actual = await RepoService.moveFiles( - sessionData, - mockGithubSessionData, - "fake-path", - "fake-new-path", - ["old-fake-file.md", "old-fake-file-two.md"], - fakeCommitMessage - ) - - expect(actual).toEqual(expected) + it("should move files using GitHub directly if the repo is not ggs enabled", async () => { + const expected = { newSha: "fake-commit-sha" } + const fakeCommitMessage = "fake-commit-message" + const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData( + { + githubId: mockGithubId, + accessToken: mockAccessToken, + isomerUserId: mockIsomerUserId, + email: mockEmail, + siteName: "not-whitelisted", + } + ) + + const gitHubServiceMoveFiles = jest.spyOn( + GitHubService.prototype, + "moveFiles" + ) + gitHubServiceMoveFiles.mockResolvedValueOnce(expected) + + const actual = await RepoService.moveFiles( + sessionData, + mockGithubSessionData, + "fake-path", + "fake-new-path", + ["old-fake-file.md", "old-fake-file-two.md"], + fakeCommitMessage + ) + + expect(actual).toEqual(expected) + }) }) - }) - - describe("getLatestCommitOfBranch", () => { - it("should read the latest commit data from the local Git file system if the repo is ggs enabled", async () => { - const expected: GitHubCommitData = { - author: { - name: "test author", - email: "test@email.com", - date: "2023-07-20T11:25:05+08:00", - }, - sha: "test-sha", - message: "test message", - } - gbSpy.mockReturnValueOnce(true) - MockGitFileSystemService.getLatestCommitOfBranch.mockResolvedValueOnce( - okAsync(expected) - ) - const actual = await RepoService.getLatestCommitOfBranch( - mockUserWithSiteSessionDataAndGrowthBook, - "master" - ) - expect(actual).toEqual(expected) - }) + describe("getLatestCommitOfBranch", () => { + it("should read the latest commit data from the local Git file system if the repo is ggs enabled", async () => { + const expected: GitHubCommitData = { + author: { + name: "test author", + email: "test@email.com", + date: "2023-07-20T11:25:05+08:00", + }, + sha: "test-sha", + message: "test message", + } + gbSpy.mockReturnValueOnce(true) + MockGitFileSystemService.getLatestCommitOfBranch.mockResolvedValueOnce( + okAsync(expected) + ) + + const actual = await RepoService.getLatestCommitOfBranch( + mockUserWithSiteSessionDataAndGrowthBook, + "master" + ) + expect(actual).toEqual(expected) + }) - it("should read latest commit data from GitHub if the repo is not ggs enabled", async () => { - const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData({ - githubId: mockGithubId, - accessToken: mockAccessToken, - isomerUserId: mockIsomerUserId, - email: mockEmail, - siteName: "not-whitelisted", + it("should read latest commit data from GitHub if the repo is not ggs enabled", async () => { + const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData( + { + githubId: mockGithubId, + accessToken: mockAccessToken, + isomerUserId: mockIsomerUserId, + email: mockEmail, + siteName: "not-whitelisted", + } + ) + const expected: GitHubCommitData = { + author: { + name: "test author", + email: "test@email.com", + date: "2023-07-20T11:25:05+08:00", + }, + message: "test message", + } + const gitHubServiceReadDirectory = jest.spyOn( + GitHubService.prototype, + "getLatestCommitOfBranch" + ) + gitHubServiceReadDirectory.mockResolvedValueOnce(expected) + const actual = await RepoService.getLatestCommitOfBranch( + sessionData, + "master" + ) + expect(actual).toEqual(expected) }) - const expected: GitHubCommitData = { - author: { - name: "test author", - email: "test@email.com", - date: "2023-07-20T11:25:05+08:00", - }, - message: "test message", - } - const gitHubServiceReadDirectory = jest.spyOn( - GitHubService.prototype, - "getLatestCommitOfBranch" - ) - gitHubServiceReadDirectory.mockResolvedValueOnce(expected) - const actual = await RepoService.getLatestCommitOfBranch( - sessionData, - "master" - ) - expect(actual).toEqual(expected) }) - }) - describe("updateRepoState", () => { - it("should update the repo state on the local Git file system if the repo is ggs enabled", async () => { - MockGitFileSystemService.updateRepoState.mockResolvedValueOnce( - okAsync(undefined) - ) - gbSpy.mockReturnValueOnce(true) + describe("updateRepoState", () => { + it("should update the repo state on the local Git file system if the repo is ggs enabled", async () => { + MockGitFileSystemService.updateRepoState.mockResolvedValueOnce( + okAsync(undefined) + ) + gbSpy.mockReturnValueOnce(true) + + await RepoService.updateRepoState( + mockUserWithSiteSessionDataAndGrowthBook, + { + commitSha: "fake-sha", + branchName: "master", + } + ) + + expect(MockGitFileSystemService.updateRepoState).toBeCalledTimes(1) + }) - await RepoService.updateRepoState( - mockUserWithSiteSessionDataAndGrowthBook, - { + it("should update the repo state on GitHub if the repo is not ggs enabled", async () => { + const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData( + { + githubId: mockGithubId, + accessToken: mockAccessToken, + isomerUserId: mockIsomerUserId, + email: mockEmail, + siteName: "not-whitelisted", + } + ) + const gitHubServiceUpdateRepoState = jest.spyOn( + GitHubService.prototype, + "updateRepoState" + ) + gitHubServiceUpdateRepoState.mockResolvedValueOnce(undefined) + + await RepoService.updateRepoState(sessionData, { commitSha: "fake-sha", branchName: "master", - } - ) + }) - expect(MockGitFileSystemService.updateRepoState).toBeCalledTimes(1) - }) - - it("should update the repo state on GitHub if the repo is not ggs enabled", async () => { - const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData({ - githubId: mockGithubId, - accessToken: mockAccessToken, - isomerUserId: mockIsomerUserId, - email: mockEmail, - siteName: "not-whitelisted", - }) - const gitHubServiceUpdateRepoState = jest.spyOn( - GitHubService.prototype, - "updateRepoState" - ) - gitHubServiceUpdateRepoState.mockResolvedValueOnce(undefined) - - await RepoService.updateRepoState(sessionData, { - commitSha: "fake-sha", - branchName: "master", + expect(gitHubServiceUpdateRepoState).toBeCalledTimes(1) }) - - expect(gitHubServiceUpdateRepoState).toBeCalledTimes(1) }) }) }) diff --git a/src/utils/neverthrow.ts b/src/utils/neverthrow.ts new file mode 100644 index 000000000..5f29b15c5 --- /dev/null +++ b/src/utils/neverthrow.ts @@ -0,0 +1,15 @@ +import { ResultAsync } from "neverthrow" + +/** + * This function is only used when integrating with third party libraries that + * expect a .catch() method on the returned promise. This should not be used in most + * control flows as it removes the benefits that neverthrow provides. + */ +const convertNeverThrowToPromise = async ( + x: ResultAsync +): Promise => { + const res = await x + return res._unsafeUnwrap() +} + +export default convertNeverThrowToPromise