Skip to content

Commit

Permalink
feat: introduce CommitSigner interface (#442)
Browse files Browse the repository at this point in the history
* feat: introduce CommitSigner interface

* chore: fix lint

* ignore eslint config

* address review
  • Loading branch information
chingor13 authored Jan 25, 2023
1 parent 83ef40a commit 4801592
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 13 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"extends": "./node_modules/gts"
"extends": "./node_modules/gts",
"root": true
}
1 change: 1 addition & 0 deletions owlbot.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@
'.github/release-please.yml',
'renovate.json',
'.eslintignore',
'.eslintrc.json',
'.prettierignore',
])
12 changes: 9 additions & 3 deletions src/github/commit-and-push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
} from '../types';
import {Octokit} from '@octokit/rest';
import {logger} from '../logger';
import {createCommit} from './create-commit';
import {createCommit, CreateCommitOptions} from './create-commit';
import {CommitError} from '../errors';

const DEFAULT_FILES_PER_COMMIT = 100;
Expand Down Expand Up @@ -142,6 +142,10 @@ export async function updateRef(
}
}

interface CommitAndPushOptions extends CreateCommitOptions {
filesPerCommit?: number;
}

/**
* Given a set of changes, apply the commit(s) on top of the given branch's head and upload it to GitHub
* Rejects if GitHub V3 API fails with the GitHub error response
Expand All @@ -162,8 +166,9 @@ export async function commitAndPush(
originBranch: BranchDomain,
commitMessage: string,
force: boolean,
filesPerCommit: number = DEFAULT_FILES_PER_COMMIT
options?: CommitAndPushOptions
) {
const filesPerCommit = options?.filesPerCommit ?? DEFAULT_FILES_PER_COMMIT;
const tree = generateTreeObjects(changes);
for (const treeGroup of inGroupsOf(tree, filesPerCommit)) {
const treeSha = await createTree(octokit, originBranch, refHead, treeGroup);
Expand All @@ -172,7 +177,8 @@ export async function commitAndPush(
originBranch,
refHead,
treeSha,
commitMessage
commitMessage,
options
);
}
await updateRef(octokit, originBranch, refHead, force);
Expand Down
24 changes: 22 additions & 2 deletions src/github/create-commit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,16 @@
// limitations under the License.

import {Octokit} from '@octokit/rest';
import {RepoDomain} from '../types';
import {RepoDomain, CommitSigner, UserData} from '../types';
import {logger} from '../logger';
import {CommitError} from '../errors';

export interface CreateCommitOptions {
signer?: CommitSigner;
author?: UserData;
committer?: UserData;
}

/**
* Create a commit with a repo snapshot SHA on top of the reference HEAD
* and resolves with the SHA of the commit.
Expand All @@ -27,15 +33,26 @@ import {CommitError} from '../errors';
* @param {string} treeSha the tree SHA that this commit will point to
* @param {string} message the message of the new commit
* @returns {Promise<string>} the new commit SHA
* @see https://docs.github.com/en/rest/git/commits?apiVersion=2022-11-28#create-a-commit
*/
export async function createCommit(
octokit: Octokit,
origin: RepoDomain,
refHead: string,
treeSha: string,
message: string
message: string,
options: CreateCommitOptions = {}
): Promise<string> {
try {
const signature = options.signer
? await options.signer.generateSignature({
message,
tree: treeSha,
parents: [refHead],
author: options.author,
committer: options.committer,
})
: undefined;
const {
data: {sha, url},
} = await octokit.git.createCommit({
Expand All @@ -44,6 +61,9 @@ export async function createCommit(
message,
tree: treeSha,
parents: [refHead],
signature,
author: options.author,
committer: options.committer,
});
logger.info(`Successfully created commit. See commit at ${url}`);
return sha;
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
FileDiffContent,
CreateReviewCommentUserOptions,
} from './types';
export {Changes} from './types';
export {Changes, CommitData, CommitSigner} from './types';
import {Octokit} from '@octokit/rest';
import {logger, setupLogger} from './logger';
import {
Expand Down Expand Up @@ -192,7 +192,7 @@ async function createPullRequest(
originBranch,
gitHubConfigs.message,
gitHubConfigs.force,
gitHubConfigs.filesPerCommit
options
);

const description: Description = {
Expand Down
21 changes: 20 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {CreateCommitOptions} from './github/create-commit';

export type FileMode = '100644' | '100755' | '040000' | '160000' | '120000';

/**
Expand Down Expand Up @@ -72,7 +74,7 @@ export interface Description {
/**
* The user options for creating GitHub PRs
*/
export interface CreatePullRequestUserOptions {
export interface CreatePullRequestUserOptions extends CreateCommitOptions {
// the owner of the target fork repository
upstreamOwner: string;
// the name of the target fork repository
Expand Down Expand Up @@ -200,3 +202,20 @@ export interface Logger {
debug: LogFn;
trace: LogFn;
}

export interface UserData {
name: string;
email: string;
}

export interface CommitData {
message: string;
tree: string;
parents: string[];
author?: UserData;
committer?: UserData;
}

export interface CommitSigner {
generateSignature(commit: CommitData): Promise<string>;
}
108 changes: 104 additions & 4 deletions test/commit-and-push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,14 @@ import * as sinon from 'sinon';
import {GetResponseTypeFromEndpointMethod} from '@octokit/types';
import * as handler from '../src/github/commit-and-push';
import * as createCommitModule from '../src/github/create-commit';
import {Changes, FileData, TreeObject, RepoDomain} from '../src/types';
import {
Changes,
FileData,
TreeObject,
RepoDomain,
CommitData,
CommitSigner,
} from '../src/types';
import {createCommit} from '../src/github/create-commit';
import {CommitError} from '../src/errors';

Expand All @@ -37,6 +44,17 @@ type CreateCommitResponse = GetResponseTypeFromEndpointMethod<
typeof octokit.git.createCommit
>;

class FakeCommitSigner implements CommitSigner {
signature: string;
constructor(signature: string) {
this.signature = signature;
}
/* eslint-disable @typescript-eslint/no-unused-vars */
async generateSignature(_commit: CommitData): Promise<string> {
return this.signature;
}
}

before(() => {
setup();
});
Expand Down Expand Up @@ -199,7 +217,7 @@ describe('Commit', () => {
// tests
const sha = await createCommit(octokit, origin, head, treeSha, message);
assert.strictEqual(sha, createCommitResponse.data.sha);
sandbox.assert.calledOnceWithExactly(stubCreateCommit, {
sandbox.assert.calledOnceWithMatch(stubCreateCommit, {
owner: origin.owner,
repo: origin.repo,
message,
Expand Down Expand Up @@ -324,12 +342,85 @@ describe('Commit and push function', async () => {
],
base_tree: getCommitResponse.data.tree.sha,
});
sandbox.assert.calledOnceWithExactly(stubCreateCommit, {
sandbox.assert.calledOnceWithMatch(stubCreateCommit, {
owner: origin.owner,
repo: origin.repo,
message,
tree: createTreeResponse.data.sha,
parents: [oldHeadSha],
});
sandbox.assert.calledOnceWithExactly(stubUpdateRef, {
owner: origin.owner,
repo: origin.repo,
sha: createCommitResponse.data.sha,
ref: 'heads/test-branch-name',
force: true,
});
});
it('allows configuring a commit signer', async () => {
changes.set('foo.txt', {
mode: '100755',
content: 'some file content',
});
// setup
const stubGetCommit = sandbox
.stub(octokit.git, 'getCommit')
.resolves(getCommitResponse);
const stubCreateTree = sandbox
.stub(octokit.git, 'createTree')
.resolves(createTreeResponse);
const stubCreateCommit = sandbox
.stub(octokit.git, 'createCommit')
.resolves(createCommitResponse);
const stubUpdateRef = sandbox.stub(octokit.git, 'updateRef');
const fakeSigner = new FakeCommitSigner('fake-signature');
const options = {
author: {
name: 'Test Committer',
email: '[email protected]',
},
signer: fakeSigner,
};
const signatureSpy = sandbox.spy(fakeSigner, 'generateSignature');
// tests
await handler.commitAndPush(
octokit,
oldHeadSha,
changes,
{branch: branchName, ...origin},
message,
true,
options
);
sandbox.assert.calledOnceWithExactly(stubGetCommit, {
owner: origin.owner,
repo: origin.repo,
commit_sha: oldHeadSha,
});
sandbox.assert.calledWithExactly(stubCreateTree, {
owner: origin.owner,
repo: origin.repo,
tree: [
{
path: 'foo.txt',
mode: '100755',
type: 'blob',
content: 'some file content',
},
],
base_tree: getCommitResponse.data.tree.sha,
});
sandbox.assert.calledOnceWithMatch(stubCreateCommit, {
owner: origin.owner,
repo: origin.repo,
message,
tree: createTreeResponse.data.sha,
parents: [oldHeadSha],
signature: 'fake-signature',
author: {
name: 'Test Committer',
email: '[email protected]',
},
});
sandbox.assert.calledOnceWithExactly(stubUpdateRef, {
owner: origin.owner,
Expand All @@ -338,6 +429,15 @@ describe('Commit and push function', async () => {
ref: 'heads/test-branch-name',
force: true,
});
sandbox.assert.calledOnceWithMatch(signatureSpy, {
message,
tree: createTreeResponse.data.sha,
parents: [oldHeadSha],
author: {
name: 'Test Committer',
email: '[email protected]',
},
});
});
it('Forwards GitHub error if getCommit fails', async () => {
const error = new Error('Error committing');
Expand Down Expand Up @@ -396,7 +496,7 @@ describe('Commit and push function', async () => {
{branch: branchName, ...origin},
message,
true,
6
{filesPerCommit: 6}
);

sinon.assert.calledTwice(createTreeStub);
Expand Down

0 comments on commit 4801592

Please sign in to comment.