Skip to content

Commit ecc424d

Browse files
chingor13Benjamin E. Coe
andauthored
feat: handle extremely large pull request body fields (#1689)
* feat: add ability to upload file to new branch * feat: introduce PullRequestOverflowHandler and implement writing to file on new branch * test: add tests for FilePullRequestOverflowHandler * test: add tests for GitHub.createFileOnNewBranch * test: add failing test for manifest parsing release notes * fix: manifest uses overflow handler when parsing releases to create * address comments Co-authored-by: Benjamin E. Coe <bencoe@google.com>
1 parent 6657691 commit ecc424d

9 files changed

Lines changed: 795 additions & 28 deletions

File tree

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
exports['FilePullRequestOverflowHandler handleOverflow writes large pull request body contents to a file 1'] = `
2+
This release is too large to preview in the pull request body. View the full release notes here: https://github.com/test-owner/test-repo/blob/my-head-branch--release-notes/release-notes.md
3+
`
4+
5+
exports['FilePullRequestOverflowHandler parseOverflow ignores small pull request body contents 1'] = `
6+
:robot: I have created a release \\*beep\\* \\*boop\\*
7+
---
8+
9+
10+
<details><summary>@google-automations/bot-config-utils: 3.2.0</summary>
11+
12+
### Features
13+
14+
* upgrade to gcf-utils@12 ([#2262](https://www.github.com/googleapis/repo-automation-bots/issues/2262)) ([bd04376](https://www.github.com/googleapis/repo-automation-bots/commit/bd043767ae59a4eed450f1d18741111dc4c3f8e8))
15+
</details>
16+
17+
<details><summary>@google-automations/label-utils: 1.1.0</summary>
18+
19+
### Features
20+
21+
* upgrade to gcf-utils@12 ([#2262](https://www.github.com/googleapis/repo-automation-bots/issues/2262)) ([bd04376](https://www.github.com/googleapis/repo-automation-bots/commit/bd043767ae59a4eed450f1d18741111dc4c3f8e8))
22+
</details>
23+
24+
<details><summary>@google-automations/object-selector: 1.1.0</summary>
25+
26+
### Features
27+
28+
* upgrade to gcf-utils@12 ([#2262](https://www.github.com/googleapis/repo-automation-bots/issues/2262)) ([bd04376](https://www.github.com/googleapis/repo-automation-bots/commit/bd043767ae59a4eed450f1d18741111dc4c3f8e8))
29+
</details>
30+
31+
<details><summary>@google-automations/datastore-lock: 2.1.0</summary>
32+
33+
### Features
34+
35+
* upgrade to gcf-utils@12 ([#2262](https://www.github.com/googleapis/repo-automation-bots/issues/2262)) ([bd04376](https://www.github.com/googleapis/repo-automation-bots/commit/bd043767ae59a4eed450f1d18741111dc4c3f8e8))
36+
</details>
37+
38+
---
39+
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
40+
`
41+
42+
exports['FilePullRequestOverflowHandler parseOverflow parses overflow body and reads file contents 1'] = `
43+
:robot: I have created a release \\*beep\\* \\*boop\\*
44+
---
45+
46+
47+
<details><summary>@google-automations/bot-config-utils: 3.2.0</summary>
48+
49+
### Features
50+
51+
* upgrade to gcf-utils@12 ([#2262](https://www.github.com/googleapis/repo-automation-bots/issues/2262)) ([bd04376](https://www.github.com/googleapis/repo-automation-bots/commit/bd043767ae59a4eed450f1d18741111dc4c3f8e8))
52+
</details>
53+
54+
<details><summary>@google-automations/label-utils: 1.1.0</summary>
55+
56+
### Features
57+
58+
* upgrade to gcf-utils@12 ([#2262](https://www.github.com/googleapis/repo-automation-bots/issues/2262)) ([bd04376](https://www.github.com/googleapis/repo-automation-bots/commit/bd043767ae59a4eed450f1d18741111dc4c3f8e8))
59+
</details>
60+
61+
<details><summary>@google-automations/object-selector: 1.1.0</summary>
62+
63+
### Features
64+
65+
* upgrade to gcf-utils@12 ([#2262](https://www.github.com/googleapis/repo-automation-bots/issues/2262)) ([bd04376](https://www.github.com/googleapis/repo-automation-bots/commit/bd043767ae59a4eed450f1d18741111dc4c3f8e8))
66+
</details>
67+
68+
<details><summary>@google-automations/datastore-lock: 2.1.0</summary>
69+
70+
### Features
71+
72+
* upgrade to gcf-utils@12 ([#2262](https://www.github.com/googleapis/repo-automation-bots/issues/2262)) ([bd04376](https://www.github.com/googleapis/repo-automation-bots/commit/bd043767ae59a4eed450f1d18741111dc4c3f8e8))
73+
</details>
74+
75+
---
76+
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
77+
`

src/github.ts

Lines changed: 183 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
GitHubAPIError,
2525
DuplicateReleaseError,
2626
FileNotFoundError,
27+
ConfigurationError,
2728
} from './errors';
2829

2930
const MAX_ISSUE_BODY_SIZE = 65536;
@@ -193,6 +194,11 @@ interface FileDiff {
193194
}
194195
export type ChangeSet = Map<string, FileDiff>;
195196

197+
interface CreatePullRequestOptions {
198+
fork?: boolean;
199+
draft?: boolean;
200+
}
201+
196202
export class GitHub {
197203
readonly repository: Repository;
198204
private octokit: OctokitType;
@@ -931,6 +937,7 @@ export class GitHub {
931937
* @param {string} path The path to the file in the repository
932938
* @param {string} branch The branch to fetch from
933939
* @returns {GitHubFileContents}
940+
* @throws {FileNotFoundError} if the file cannot be found
934941
* @throws {GitHubAPIError} on other API errors
935942
*/
936943
async getFileContentsOnBranch(
@@ -1046,6 +1053,8 @@ export class GitHub {
10461053
/**
10471054
* Open a pull request
10481055
*
1056+
* @deprecated This logic is handled by the Manifest class now as it
1057+
* can be more complicated if the release notes are too big
10491058
* @param {ReleasePullRequest} releasePullRequest Pull request data to update
10501059
* @param {string} targetBranch The base branch of the pull request
10511060
* @param {GitHubPR} options The pull request options
@@ -1087,16 +1096,23 @@ export class GitHub {
10871096
);
10881097
}
10891098

1099+
/**
1100+
* Open a pull request
1101+
*
1102+
* @param {PullRequest} pullRequest Pull request data to update
1103+
* @param {string} targetBranch The base branch of the pull request
1104+
* @param {string} message The commit message for the commit
1105+
* @param {Update[]} updates The files to update
1106+
* @param {CreatePullRequestOptions} options The pull request options
1107+
* @throws {GitHubAPIError} on an API error
1108+
*/
10901109
createPullRequest = wrapAsync(
10911110
async (
10921111
pullRequest: PullRequest,
10931112
targetBranch: string,
10941113
message: string,
10951114
updates: Update[],
1096-
options?: {
1097-
fork?: boolean;
1098-
draft?: boolean;
1099-
}
1115+
options?: CreatePullRequestOptions
11001116
): Promise<PullRequest> => {
11011117
// Update the files for the release if not already supplied
11021118
const changes = await this.buildChangeSet(updates, targetBranch);
@@ -1448,6 +1464,169 @@ export class GitHub {
14481464
});
14491465
return resp.data.body;
14501466
}
1467+
1468+
/**
1469+
* Create a single file on a new branch based on an existing
1470+
* branch. This will force-push to that branch.
1471+
* @param {string} filename Filename with path in the repository
1472+
* @param {string} contents Contents of the file
1473+
* @param {string} newBranchName Name of the new branch
1474+
* @param {string} baseBranchName Name of the base branch (where
1475+
* new branch is forked from)
1476+
* @returns {string} HTML URL of the new file
1477+
*/
1478+
async createFileOnNewBranch(
1479+
filename: string,
1480+
contents: string,
1481+
newBranchName: string,
1482+
baseBranchName: string
1483+
): Promise<string> {
1484+
// create or update new branch to match base branch
1485+
await this.forkBranch(newBranchName, baseBranchName);
1486+
1487+
// use the single file upload API
1488+
const {
1489+
data: {content},
1490+
} = await this.octokit.repos.createOrUpdateFileContents({
1491+
owner: this.repository.owner,
1492+
repo: this.repository.repo,
1493+
path: filename,
1494+
// contents need to be base64 encoded
1495+
content: Buffer.from(contents, 'binary').toString('base64'),
1496+
message: 'Saving release notes',
1497+
branch: newBranchName,
1498+
});
1499+
1500+
if (!content?.html_url) {
1501+
throw new Error(
1502+
`Failed to write to file: ${filename} on branch: ${newBranchName}`
1503+
);
1504+
}
1505+
1506+
return content.html_url;
1507+
}
1508+
1509+
/**
1510+
* Helper to fetch the SHA of a branch
1511+
* @param {string} branchName The name of the branch
1512+
* @return {string | undefined} Returns the SHA of the branch
1513+
* or undefined if it can't be found.
1514+
*/
1515+
private async getBranchSha(branchName: string): Promise<string | undefined> {
1516+
this.logger.debug(`Looking up SHA for branch: ${branchName}`);
1517+
try {
1518+
const {
1519+
data: {
1520+
object: {sha},
1521+
},
1522+
} = await this.octokit.git.getRef({
1523+
owner: this.repository.owner,
1524+
repo: this.repository.repo,
1525+
ref: `heads/${branchName}`,
1526+
});
1527+
this.logger.debug(`SHA for branch: ${sha}`);
1528+
return sha;
1529+
} catch (e) {
1530+
if (e instanceof RequestError && e.status === 404) {
1531+
this.logger.debug(`Branch: ${branchName} does not exist`);
1532+
return undefined;
1533+
}
1534+
throw e;
1535+
}
1536+
}
1537+
1538+
/**
1539+
* Helper to fork a branch from an existing branch. Uses `force` so
1540+
* it will overwrite the contents of `targetBranchName` to match
1541+
* the current contents of `baseBranchName`.
1542+
*
1543+
* @param {string} targetBranchName The name of the new forked branch
1544+
* @param {string} baseBranchName The base branch from which to fork.
1545+
* @returns {string} The branch SHA
1546+
* @throws {ConfigurationError} if the base branch cannot be found.
1547+
*/
1548+
private async forkBranch(
1549+
targetBranchName: string,
1550+
baseBranchName: string
1551+
): Promise<string> {
1552+
const baseBranchSha = await this.getBranchSha(baseBranchName);
1553+
if (!baseBranchSha) {
1554+
// this is highly unlikely to be thrown as we will have
1555+
// already attempted to read from the branch
1556+
throw new ConfigurationError(
1557+
`Unable to find base branch: ${baseBranchName}`,
1558+
'core',
1559+
`${this.repository.owner}/${this.repository.repo}`
1560+
);
1561+
}
1562+
// see if newBranchName exists
1563+
if (await this.getBranchSha(targetBranchName)) {
1564+
// branch already exists, update it to the match the base branch
1565+
const branchSha = await this.updateBranchSha(
1566+
targetBranchName,
1567+
baseBranchSha
1568+
);
1569+
this.logger.debug(
1570+
`Updated ${targetBranchName} to match ${baseBranchName} at ${branchSha}`
1571+
);
1572+
return branchSha;
1573+
} else {
1574+
// branch does not exist, create a new branch from the base branch
1575+
const branchSha = await this.createNewBranch(
1576+
targetBranchName,
1577+
baseBranchSha
1578+
);
1579+
this.logger.debug(
1580+
`Forked ${targetBranchName} from ${baseBranchName} at ${branchSha}`
1581+
);
1582+
return branchSha;
1583+
}
1584+
}
1585+
1586+
/**
1587+
* Helper to create a new branch from a given SHA.
1588+
* @param {string} branchName The new branch name
1589+
* @param {string} branchSha The SHA of the branch
1590+
* @returns {string} The SHA of the new branch
1591+
*/
1592+
private async createNewBranch(
1593+
branchName: string,
1594+
branchSha: string
1595+
): Promise<string> {
1596+
this.logger.debug(`Creating new branch: ${branchName} at ${branchSha}`);
1597+
const {
1598+
data: {
1599+
object: {sha},
1600+
},
1601+
} = await this.octokit.git.createRef({
1602+
owner: this.repository.owner,
1603+
repo: this.repository.repo,
1604+
ref: `refs/heads/${branchName}`,
1605+
sha: branchSha,
1606+
});
1607+
this.logger.debug(`New branch: ${branchName} at ${sha}`);
1608+
return sha;
1609+
}
1610+
1611+
private async updateBranchSha(
1612+
branchName: string,
1613+
branchSha: string
1614+
): Promise<string> {
1615+
this.logger.debug(`Updating branch ${branchName} to ${branchSha}`);
1616+
const {
1617+
data: {
1618+
object: {sha},
1619+
},
1620+
} = await this.octokit.git.updateRef({
1621+
owner: this.repository.owner,
1622+
repo: this.repository.repo,
1623+
ref: `heads/${branchName}`,
1624+
sha: branchSha,
1625+
force: true,
1626+
});
1627+
this.logger.debug(`Updated branch: ${branchName} to ${sha}`);
1628+
return sha;
1629+
}
14511630
}
14521631

14531632
/**

0 commit comments

Comments
 (0)