From da7fbe0638ca9040a69abc02bb36a391f54eebc0 Mon Sep 17 00:00:00 2001 From: Erez Rokah Date: Tue, 9 Jun 2020 19:03:19 +0300 Subject: [PATCH] Fix: handle branch names conflicts (#3879) --- .../netlify-cms-backend-bitbucket/src/API.ts | 30 ++++++++++--- .../netlify-cms-backend-github/src/API.ts | 42 +++++++++++++++++- .../src/GraphQLAPI.ts | 43 +++++++++++++++++-- .../netlify-cms-backend-gitlab/src/API.ts | 35 ++++++++++----- packages/netlify-cms-lib-util/src/API.ts | 39 +++++++++++++++++ packages/netlify-cms-lib-util/src/index.ts | 3 ++ 6 files changed, 170 insertions(+), 22 deletions(-) diff --git a/packages/netlify-cms-backend-bitbucket/src/API.ts b/packages/netlify-cms-backend-bitbucket/src/API.ts index cba72079..92cc996b 100644 --- a/packages/netlify-cms-backend-bitbucket/src/API.ts +++ b/packages/netlify-cms-backend-bitbucket/src/API.ts @@ -26,6 +26,7 @@ import { branchFromContentKey, requestWithBackoff, readFileMetadata, + throwOnConflictingBranches, } from 'netlify-cms-lib-util'; import { oneLine } from 'common-tags'; import { parse } from 'what-the-diff'; @@ -250,10 +251,18 @@ export default class API { return response.ok; }; + getBranch = async (branchName: string) => { + const branch: BitBucketBranch = await this.requestJSON( + `${this.repoURL}/refs/branches/${branchName}`, + ); + + return branch; + }; + branchCommitSha = async (branch: string) => { const { target: { hash: branchSha }, - }: BitBucketBranch = await this.requestJSON(`${this.repoURL}/refs/branches/${branch}`); + }: BitBucketBranch = await this.getBranch(branch); return branchSha; }; @@ -442,11 +451,20 @@ export default class API { formData.append('parents', parentSha); } - await this.request({ - url: `${this.repoURL}/src`, - method: 'POST', - body: formData, - }); + try { + await this.requestText({ + url: `${this.repoURL}/src`, + method: 'POST', + body: formData, + }); + } catch (error) { + const message = error.message || ''; + // very descriptive message from Bitbucket + if (parentSha && message.includes('Something went wrong')) { + await throwOnConflictingBranches(branch, name => this.getBranch(name), API_NAME); + } + throw error; + } return files; } diff --git a/packages/netlify-cms-backend-github/src/API.ts b/packages/netlify-cms-backend-github/src/API.ts index 73e1acc1..6eebf40e 100644 --- a/packages/netlify-cms-backend-github/src/API.ts +++ b/packages/netlify-cms-backend-github/src/API.ts @@ -27,6 +27,7 @@ import { requestWithBackoff, unsentRequest, ApiRequest, + throwOnConflictingBranches, } from 'netlify-cms-lib-util'; import { Octokit } from '@octokit/rest'; @@ -1253,8 +1254,45 @@ export default class API { return result; } - createBranch(branchName: string, sha: string) { - return this.createRef('heads', branchName, sha); + async backupBranch(branchName: string) { + try { + const existingBranch = await this.getBranch(branchName); + await this.createBranch( + existingBranch.name.replace( + new RegExp(`${CMS_BRANCH_PREFIX}/`), + `${CMS_BRANCH_PREFIX}_${Date.now()}/`, + ), + existingBranch.commit.sha, + ); + } catch (e) { + console.warn(e); + } + } + + async createBranch(branchName: string, sha: string) { + try { + const result = await this.createRef('heads', branchName, sha); + return result; + } catch (e) { + const message = String(e.message || ''); + if (message === 'Reference update failed') { + await throwOnConflictingBranches(branchName, name => this.getBranch(name), API_NAME); + } else if ( + message === 'Reference already exists' && + branchName.startsWith(`${CMS_BRANCH_PREFIX}/`) + ) { + try { + // this can happen if the branch wasn't deleted when the PR was merged + // we backup the existing branch just in case and patch it with the new sha + await this.backupBranch(branchName); + const result = await this.patchBranch(branchName, sha, { force: true }); + return result; + } catch (e) { + console.log(e); + } + } + throw e; + } } assertCmsBranch(branchName: string) { diff --git a/packages/netlify-cms-backend-github/src/GraphQLAPI.ts b/packages/netlify-cms-backend-github/src/GraphQLAPI.ts index 9275e074..e8b7d382 100644 --- a/packages/netlify-cms-backend-github/src/GraphQLAPI.ts +++ b/packages/netlify-cms-backend-github/src/GraphQLAPI.ts @@ -14,8 +14,9 @@ import { DEFAULT_PR_BODY, branchFromContentKey, CMS_BRANCH_PREFIX, + throwOnConflictingBranches, } from 'netlify-cms-lib-util'; -import { trim } from 'lodash'; +import { trim, trimStart } from 'lodash'; import introspectionQueryResultData from './fragmentTypes'; import API, { Config, BlobArgs, API_NAME, PullRequestState, MOCK_PULL_REQUEST } from './API'; import * as queries from './queries'; @@ -134,10 +135,44 @@ export default class GraphQLAPI extends API { }); } - mutate(options: MutationOptions) { - return this.client.mutate(options).catch(error => { + async mutate(options: MutationOptions) { + try { + const result = await this.client.mutate(options); + return result; + } catch (error) { + const errors = error.graphQLErrors; + if (Array.isArray(errors) && errors.some(e => e.message === 'Ref cannot be created.')) { + const refName = options?.variables?.createRefInput?.name || ''; + const branchName = trimStart(refName, 'refs/heads/'); + if (branchName) { + await throwOnConflictingBranches(branchName, name => this.getBranch(name), API_NAME); + } + } else if ( + Array.isArray(errors) && + errors.some(e => + new RegExp( + `A ref named "refs/heads/${CMS_BRANCH_PREFIX}/.+?" already exists in the repository.`, + ).test(e.message), + ) + ) { + const refName = options?.variables?.createRefInput?.name || ''; + const sha = options?.variables?.createRefInput?.oid || ''; + const branchName = trimStart(refName, 'refs/heads/'); + if (branchName && branchName.startsWith(`${CMS_BRANCH_PREFIX}/`) && sha) { + try { + // this can happen if the branch wasn't deleted when the PR was merged + // we backup the existing branch just in case an re-run the mutation + await this.backupBranch(branchName); + await this.deleteBranch(branchName); + const result = await this.client.mutate(options); + return result; + } catch (e) { + console.log(e); + } + } + } throw new APIError(error.message, 500, 'GitHub'); - }); + } } async hasWriteAccess() { diff --git a/packages/netlify-cms-backend-gitlab/src/API.ts b/packages/netlify-cms-backend-gitlab/src/API.ts index b2d01cb5..3b43cceb 100644 --- a/packages/netlify-cms-backend-gitlab/src/API.ts +++ b/packages/netlify-cms-backend-gitlab/src/API.ts @@ -25,6 +25,7 @@ import { requestWithBackoff, readFileMetadata, FetchError, + throwOnConflictingBranches, } from 'netlify-cms-lib-util'; import { Base64 } from 'js-base64'; import { Map } from 'immutable'; @@ -407,7 +408,14 @@ export default class API { toBase64 = (str: string) => Promise.resolve(Base64.encode(str)); fromBase64 = (str: string) => Base64.decode(str); - uploadAndCommit( + async getBranch(branchName: string) { + const branch: GitLabBranch = await this.requestJSON( + `${this.repoURL}/repository/branches/${encodeURIComponent(branchName)}`, + ); + return branch; + } + + async uploadAndCommit( items: CommitItem[], { commitMessage = '', branch = this.branch, newBranch = false }, ) { @@ -434,12 +442,21 @@ export default class API { commitParams.author_email = email; } - return this.requestJSON({ - url: `${this.repoURL}/repository/commits`, - method: 'POST', - headers: { 'Content-Type': 'application/json; charset=utf-8' }, - body: JSON.stringify(commitParams), - }); + try { + const result = await this.requestJSON({ + url: `${this.repoURL}/repository/commits`, + method: 'POST', + headers: { 'Content-Type': 'application/json; charset=utf-8' }, + body: JSON.stringify(commitParams), + }); + return result; + } catch (error) { + const message = error.message || ''; + if (newBranch && message.includes(`Could not update ${branch}`)) { + await throwOnConflictingBranches(branch, name => this.getBranch(name), API_NAME); + } + throw error; + } } async getCommitItems(files: (Entry | AssetProxy)[], branch: string) { @@ -781,9 +798,7 @@ export default class API { } async getDefaultBranch() { - const branch: GitLabBranch = await this.requestJSON( - `${this.repoURL}/repository/branches/${encodeURIComponent(this.branch)}`, - ); + const branch: GitLabBranch = await this.getBranch(this.branch); return branch; } diff --git a/packages/netlify-cms-lib-util/src/API.ts b/packages/netlify-cms-lib-util/src/API.ts index 682df958..0b2eb3cd 100644 --- a/packages/netlify-cms-lib-util/src/API.ts +++ b/packages/netlify-cms-lib-util/src/API.ts @@ -1,5 +1,6 @@ import { asyncLock, AsyncLock } from './asyncLock'; import unsentRequest from './unsentRequest'; +import APIError from './APIError'; export interface FetchError extends Error { status: number; @@ -174,3 +175,41 @@ export const getPreviewStatus = ( return isPreviewContext(context, previewContext); }); }; + +const getConflictingBranches = (branchName: string) => { + // for cms/posts/post-1, conflicting branches are cms/posts, cms + const parts = branchName.split('/'); + parts.pop(); + + const conflictingBranches = parts.reduce((acc, _, index) => { + acc = [...acc, parts.slice(0, index + 1).join('/')]; + return acc; + }, [] as string[]); + + return conflictingBranches; +}; + +export const throwOnConflictingBranches = async ( + branchName: string, + getBranch: (name: string) => Promise<{ name: string }>, + apiName: string, +) => { + const possibleConflictingBranches = getConflictingBranches(branchName); + + const conflictingBranches = await Promise.all( + possibleConflictingBranches.map(b => + getBranch(b) + .then(b => b.name) + .catch(() => ''), + ), + ); + + const conflictingBranch = conflictingBranches.filter(Boolean)[0]; + if (conflictingBranch) { + throw new APIError( + `Failed creating branch '${branchName}' since there is already a branch named '${conflictingBranch}'. Please delete the '${conflictingBranch}' branch and try again`, + 500, + apiName, + ); + } +}; diff --git a/packages/netlify-cms-lib-util/src/index.ts b/packages/netlify-cms-lib-util/src/index.ts index bcb1adc9..d637c5dc 100644 --- a/packages/netlify-cms-lib-util/src/index.ts +++ b/packages/netlify-cms-lib-util/src/index.ts @@ -49,6 +49,7 @@ import { FetchError as FE, ApiRequest as AR, requestWithBackoff, + throwOnConflictingBranches, } from './API'; import { CMS_BRANCH_PREFIX, @@ -140,6 +141,7 @@ export const NetlifyCmsLibUtil = { requestWithBackoff, allEntriesByFolder, AccessTokenError, + throwOnConflictingBranches, }; export { APIError, @@ -195,4 +197,5 @@ export { requestWithBackoff, allEntriesByFolder, AccessTokenError, + throwOnConflictingBranches, };