diff --git a/README.md b/README.md index f6b849b..8a69a39 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,7 @@ This action will create a github release and optionally upload an artifact to it - **draft**: Optionally marks this release as a draft release. Set to `true` to enable. - **name**: An optional name for the release. If this is omitted the tag will be used. - **prerelease**: Optionally marks this release as prerelease. Set to true to enable. +- **replacesArtifacts**: Indicates if existing release artifacts should be replaced. Defaults to true. - **tag**: An optional tag for the release. If this is omitted the git ref will be used (if it is a tag). - **token**: (**Required**) The Github token. Typically this will be `${{ secrets.GITHUB_TOKEN }}`. diff --git a/__tests__/Action.test.ts b/__tests__/Action.test.ts index ae638a5..0975071 100644 --- a/__tests__/Action.test.ts +++ b/__tests__/Action.test.ts @@ -5,7 +5,9 @@ import { Releases } from "../src/Releases"; import { ArtifactUploader } from "../src/ArtifactUploader"; const createMock = jest.fn() +const deleteMock = jest.fn() const getMock = jest.fn() +const listArtifactsMock = jest.fn() const listMock = jest.fn() const updateMock = jest.fn() const uploadMock = jest.fn() @@ -21,6 +23,8 @@ const draft = true const id = 100 const name = 'name' const prerelease = true +const releaseId = 101 +const replacesArtifacts = true const tag = 'tag' const token = 'token' const url = 'http://api.example.com' @@ -51,7 +55,7 @@ describe("Action", () => { await action.perform() expect(createMock).toBeCalledWith(tag, body, commit, draft, name, prerelease) - expect(uploadMock).toBeCalledWith(artifacts, url) + expect(uploadMock).toBeCalledWith(artifacts, releaseId, url) }) it('creates release if no draft releases', async () => { @@ -67,7 +71,7 @@ describe("Action", () => { await action.perform() expect(createMock).toBeCalledWith(tag, body, commit, draft, name, prerelease) - expect(uploadMock).toBeCalledWith(artifacts, url) + expect(uploadMock).toBeCalledWith(artifacts, releaseId, url) }) @@ -77,7 +81,7 @@ describe("Action", () => { await action.perform() expect(createMock).toBeCalledWith(tag, body, commit, draft, name, prerelease) - expect(uploadMock).toBeCalledWith(artifacts, url) + expect(uploadMock).toBeCalledWith(artifacts, releaseId, url) }) it('throws error when create fails', async () => { @@ -148,7 +152,7 @@ describe("Action", () => { } expect(createMock).toBeCalledWith(tag, body, commit, draft, name, prerelease) - expect(uploadMock).toBeCalledWith(artifacts, url) + expect(uploadMock).toBeCalledWith(artifacts, releaseId, url) }) it('updates draft release', async () => { @@ -165,7 +169,7 @@ describe("Action", () => { await action.perform() expect(updateMock).toBeCalledWith(id, tag, body, commit, draft, name, prerelease) - expect(uploadMock).toBeCalledWith(artifacts, url) + expect(uploadMock).toBeCalledWith(artifacts, releaseId, url) }) @@ -185,7 +189,7 @@ describe("Action", () => { await action.perform() expect(updateMock).toBeCalledWith(id, tag, body, commit, draft, name, prerelease) - expect(uploadMock).toBeCalledWith(artifacts, url) + expect(uploadMock).toBeCalledWith(artifacts, releaseId, url) }) @@ -199,15 +203,18 @@ describe("Action", () => { const MockReleases = jest.fn(() => { return { create: createMock, + deleteArtifact: deleteMock, getByTag: getMock, + listArtifactsForRelease: listArtifactsMock, listReleases: listMock, update: updateMock, - uploadArtifact: uploadMock + uploadArtifact: jest.fn() } }) createMock.mockResolvedValue({ data: { + id: releaseId, upload_url: url } }) @@ -221,6 +228,7 @@ describe("Action", () => { }) updateMock.mockResolvedValue({ data: { + id: releaseId, upload_url: url } }) @@ -235,6 +243,7 @@ describe("Action", () => { draft: draft, name: name, prerelease: prerelease, + replacesArtifacts: replacesArtifacts, tag: tag, token: token, readArtifact: () => artifactData diff --git a/__tests__/ArtifactUploader.test.ts b/__tests__/ArtifactUploader.test.ts index 3f93a27..bd3c672 100644 --- a/__tests__/ArtifactUploader.test.ts +++ b/__tests__/ArtifactUploader.test.ts @@ -8,9 +8,13 @@ const artifacts = [ ] const fileContents = Buffer.from('artful facts', 'utf-8') const contentLength = 42 -const uploadMock = jest.fn() +const releaseId = 100 const url = 'http://api.example.com' +const deleteMock = jest.fn() +const listArtifactsMock = jest.fn() +const uploadMock = jest.fn() + jest.mock('fs', () => { return { readFileSync: () => fileContents, @@ -19,28 +23,115 @@ jest.mock('fs', () => { }) describe('ArtifactUploader', () => { - it('uploads artifacts', () => { - const uploader = createUploader() - - uploader.uploadArtifacts(artifacts, url) - + beforeEach(() => { + deleteMock.mockClear() + listArtifactsMock.mockClear() + uploadMock.mockClear() + }) + + it('replaces all artifacts', async () => { + mockDeleteSuccess() + mockListWithAssets() + const uploader = createUploader(true) + + await uploader.uploadArtifacts(artifacts, releaseId, url) + expect(uploadMock).toBeCalledTimes(2) expect(uploadMock) .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1') expect(uploadMock) .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2') + + expect(deleteMock).toBeCalledTimes(2) + expect(deleteMock).toBeCalledWith(1) + expect(deleteMock).toBeCalledWith(2) + }) + + it('replaces no artifacts when previous asset list empty', async () => { + mockDeleteSuccess() + mockListWithoutAssets() + const uploader = createUploader(true) + + await uploader.uploadArtifacts(artifacts, releaseId, url) + + expect(uploadMock).toBeCalledTimes(2) + expect(uploadMock) + .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1') + expect(uploadMock) + .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2') + + expect(deleteMock).toBeCalledTimes(0) }) - function createUploader(): GithubArtifactUploader { + it('throws error from replace', async () => { + mockDeleteError() + mockListWithAssets() + const uploader = createUploader(true) + + expect.hasAssertions() + try { + await uploader.uploadArtifacts(artifacts, releaseId, url) + } catch (error) { + expect(error).toEqual("error") + } + }) + + it('updates all artifacts, delete none', async () => { + mockDeleteError() + mockListWithAssets() + const uploader = createUploader(false) + + await uploader.uploadArtifacts(artifacts, releaseId, url) + + expect(uploadMock).toBeCalledTimes(2) + expect(uploadMock) + .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1') + expect(uploadMock) + .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2') + + expect(deleteMock).toBeCalledTimes(0) + }) + + function createUploader(replaces: boolean): GithubArtifactUploader { + uploadMock.mockResolvedValue({}) const MockReleases = jest.fn(() => { return { - getByTag: jest.fn(), create: jest.fn(), + deleteArtifact: deleteMock, + getByTag: jest.fn(), + listArtifactsForRelease: listArtifactsMock, listReleases: jest.fn(), update: jest.fn(), uploadArtifact: uploadMock } }) - return new GithubArtifactUploader(new MockReleases()) + return new GithubArtifactUploader(new MockReleases(), replaces) + } + + function mockDeleteError(): any { + deleteMock.mockRejectedValue("error") + } + + function mockDeleteSuccess(): any { + deleteMock.mockResolvedValue({}) + } + + function mockListWithAssets() { + listArtifactsMock.mockResolvedValue({ + data: [ + { + name: "art1", + id: 1 + }, + { + name: "art2", + id: 2 + } + ] + }) + } + + function mockListWithoutAssets() { + listArtifactsMock.mockResolvedValue({ data: [] }) } }); \ No newline at end of file diff --git a/__tests__/Inputs.test.ts b/__tests__/Inputs.test.ts index 8c762aa..5583932 100644 --- a/__tests__/Inputs.test.ts +++ b/__tests__/Inputs.test.ts @@ -144,6 +144,17 @@ describe('Inputs', () => { }) }) + describe('replacesArtifacts', () => { + it('returns false', () => { + expect(inputs.replacesArtifacts).toBe(false) + }) + + it('returns true', () => { + mockGetInput.mockReturnValue('true') + expect(inputs.replacesArtifacts).toBe(true) + }) + }) + describe('tag', () => { it('returns input tag', () => { mockGetInput.mockReturnValue('tag') diff --git a/action.yml b/action.yml index 7a63b9f..efd44f4 100644 --- a/action.yml +++ b/action.yml @@ -31,7 +31,10 @@ inputs: default: '' prerelease: description: "Optionally marks this release as prerelease. Set to true to enable." - default: '' + default: '' + replacesArtifacts: + description: "Indicates if existing release artifacts should be replaced. Defaults to true." + default: 'true' tag: description: 'An optional tag for the release. If this is omitted the git ref will be used (if it is a tag).' default: '' diff --git a/lib/Action.js b/lib/Action.js index 98122b4..10810a3 100644 --- a/lib/Action.js +++ b/lib/Action.js @@ -18,10 +18,12 @@ class Action { } perform() { return __awaiter(this, void 0, void 0, function* () { - const uploadUrl = yield this.createOrUpdateRelease(); + const releaseResponse = yield this.createOrUpdateRelease(); + const releaseId = releaseResponse.id; + const uploadUrl = releaseResponse.upload_url; const artifacts = this.inputs.artifacts; if (artifacts.length > 0) { - yield this.uploader.uploadArtifacts(artifacts, uploadUrl); + yield this.uploader.uploadArtifacts(artifacts, releaseId, uploadUrl); } }); } @@ -49,7 +51,7 @@ class Action { updateRelease(id) { return __awaiter(this, void 0, void 0, function* () { const response = yield this.releases.update(id, this.inputs.tag, this.inputs.body, this.inputs.commit, this.inputs.draft, this.inputs.name, this.inputs.prerelease); - return response.data.upload_url; + return response.data; }); } noPublishedRelease(error) { @@ -80,7 +82,7 @@ class Action { createRelease() { return __awaiter(this, void 0, void 0, function* () { const response = yield this.releases.create(this.inputs.tag, this.inputs.body, this.inputs.commit, this.inputs.draft, this.inputs.name, this.inputs.prerelease); - return response.data.upload_url; + return response.data; }); } } diff --git a/lib/ArtifactUploader.js b/lib/ArtifactUploader.js index 1c9ac01..5ef5276 100644 --- a/lib/ArtifactUploader.js +++ b/lib/ArtifactUploader.js @@ -18,12 +18,17 @@ var __importStar = (this && this.__importStar) || function (mod) { Object.defineProperty(exports, "__esModule", { value: true }); const core = __importStar(require("@actions/core")); class GithubArtifactUploader { - constructor(releases) { + constructor(releases, replacesExistingArtifacts) { + this.replacesExistingArtifacts = true; this.releases = releases; + this.replacesExistingArtifacts = replacesExistingArtifacts; } - uploadArtifacts(artifacts, uploadUrl) { + uploadArtifacts(artifacts, releaseId, uploadUrl) { return __awaiter(this, void 0, void 0, function* () { - artifacts.forEach((artifact) => __awaiter(this, void 0, void 0, function* () { + if (this.replacesExistingArtifacts) { + yield this.deleteUpdatedArtifacts(artifacts, releaseId); + } + for (const artifact of artifacts) { try { yield this.releases.uploadArtifact(uploadUrl, artifact.contentLength, artifact.contentType, artifact.readFile(), artifact.name); } @@ -31,7 +36,24 @@ class GithubArtifactUploader { const message = `Failed to upload artifact ${artifact.name}. Does it already exist?`; core.warning(message); } - })); + } + return Promise.resolve(); + }); + } + deleteUpdatedArtifacts(artifacts, releaseId) { + return __awaiter(this, void 0, void 0, function* () { + const response = yield this.releases.listArtifactsForRelease(releaseId); + const releaseAssets = response.data; + const assetByName = new Map(); + releaseAssets.forEach(asset => { + assetByName[asset.name] = asset; + }); + for (const artifact of artifacts) { + const asset = assetByName[artifact.name]; + if (asset) { + yield this.releases.deleteArtifact(asset.id); + } + } }); } } diff --git a/lib/Inputs.js b/lib/Inputs.js index 9b5066e..ff61295 100644 --- a/lib/Inputs.js +++ b/lib/Inputs.js @@ -59,8 +59,12 @@ class CoreInputs { return this.tag; } get prerelease() { - const draft = core.getInput('prerelease'); - return draft == 'true'; + const preRelease = core.getInput('prerelease'); + return preRelease == 'true'; + } + get replacesArtifacts() { + const replaces = core.getInput('replacesArtifacts'); + return replaces == 'true'; } get tag() { const tag = core.getInput('tag'); diff --git a/lib/Main.js b/lib/Main.js index 6346902..57903ff 100644 --- a/lib/Main.js +++ b/lib/Main.js @@ -43,7 +43,7 @@ function createAction() { const globber = new ArtifactGlobber_1.FileArtifactGlobber(); const inputs = new Inputs_1.CoreInputs(globber, context); const releases = new Releases_1.GithubReleases(context, git); - const uploader = new ArtifactUploader_1.GithubArtifactUploader(releases); + const uploader = new ArtifactUploader_1.GithubArtifactUploader(releases, inputs.replacesArtifacts); return new Action_1.Action(inputs, releases, uploader); } run(); diff --git a/lib/Releases.js b/lib/Releases.js index 2762cb8..77b7097 100644 --- a/lib/Releases.js +++ b/lib/Releases.js @@ -28,6 +28,24 @@ class GithubReleases { }); }); } + deleteArtifact(assetId) { + return __awaiter(this, void 0, void 0, function* () { + return this.git.repos.deleteReleaseAsset({ + asset_id: assetId, + owner: this.context.repo.owner, + repo: this.context.repo.repo + }); + }); + } + listArtifactsForRelease(releaseId) { + return __awaiter(this, void 0, void 0, function* () { + return this.git.repos.listAssetsForRelease({ + owner: this.context.repo.owner, + release_id: releaseId, + repo: this.context.repo.repo + }); + }); + } listReleases() { return __awaiter(this, void 0, void 0, function* () { return this.git.repos.listReleases({ diff --git a/src/Action.ts b/src/Action.ts index 896e3ff..00a1291 100644 --- a/src/Action.ts +++ b/src/Action.ts @@ -1,5 +1,6 @@ import { Inputs } from "./Inputs"; import { Releases } from "./Releases"; +import { ReposCreateReleaseResponse } from "@octokit/rest"; import { ArtifactUploader } from "./ArtifactUploader"; import { ErrorMessage } from "./ErrorMessage"; @@ -15,15 +16,17 @@ export class Action { } async perform() { - const uploadUrl = await this.createOrUpdateRelease() + const releaseResponse = await this.createOrUpdateRelease(); + const releaseId = releaseResponse.id + const uploadUrl = releaseResponse.upload_url const artifacts = this.inputs.artifacts if (artifacts.length > 0) { - await this.uploader.uploadArtifacts(artifacts, uploadUrl) + await this.uploader.uploadArtifacts(artifacts, releaseId, uploadUrl) } } - private async createOrUpdateRelease(): Promise { + private async createOrUpdateRelease(): Promise { if (this.inputs.allowUpdates) { try { const getResponse = await this.releases.getByTag(this.inputs.tag) @@ -40,7 +43,7 @@ export class Action { } } - private async updateRelease(id: number): Promise { + private async updateRelease(id: number): Promise { const response = await this.releases.update( id, this.inputs.tag, @@ -51,7 +54,7 @@ export class Action { this.inputs.prerelease ) - return response.data.upload_url + return response.data } private noPublishedRelease(error: any): boolean { @@ -59,7 +62,7 @@ export class Action { return errorMessage.status == 404 } - private async updateDraftOrCreateRelease(): Promise { + private async updateDraftOrCreateRelease(): Promise { const draftReleaseId = await this.findMatchingDraftReleaseId() if (draftReleaseId) { return await this.updateRelease(draftReleaseId) @@ -77,7 +80,7 @@ export class Action { return draftRelease?.id } - private async createRelease(): Promise { + private async createRelease(): Promise { const response = await this.releases.create( this.inputs.tag, this.inputs.body, @@ -87,6 +90,6 @@ export class Action { this.inputs.prerelease ) - return response.data.upload_url + return response.data } } \ No newline at end of file diff --git a/src/ArtifactUploader.ts b/src/ArtifactUploader.ts index dd6b676..9cd0b28 100644 --- a/src/ArtifactUploader.ts +++ b/src/ArtifactUploader.ts @@ -1,30 +1,56 @@ import * as core from '@actions/core'; import { Artifact } from "./Artifact"; import { Releases } from "./Releases"; +import { ReposListAssetsForReleaseResponseItem } from "@octokit/rest"; +import { ErrorMessage } from './ErrorMessage'; export interface ArtifactUploader { - uploadArtifacts(artifacts: Artifact[], uploadUrl: string): Promise + uploadArtifacts(artifacts: Artifact[], releaseId: number, uploadUrl: string): Promise } export class GithubArtifactUploader implements ArtifactUploader { private releases: Releases + private replacesExistingArtifacts: boolean = true - constructor(releases: Releases) { + constructor(releases: Releases, replacesExistingArtifacts: boolean) { this.releases = releases + this.replacesExistingArtifacts = replacesExistingArtifacts } - async uploadArtifacts(artifacts: Artifact[], uploadUrl: string) { - artifacts.forEach(async artifact => { + async uploadArtifacts(artifacts: Artifact[], + releaseId: number, + uploadUrl: string): Promise { + if(this.replacesExistingArtifacts) { + await this.deleteUpdatedArtifacts(artifacts, releaseId) + } + + for (const artifact of artifacts) { try { await this.releases.uploadArtifact(uploadUrl, artifact.contentLength, artifact.contentType, artifact.readFile(), artifact.name) - } catch(error) { + } catch (error) { const message = `Failed to upload artifact ${artifact.name}. Does it already exist?` core.warning(message) } + } + return Promise.resolve() + } + + async deleteUpdatedArtifacts(artifacts: Artifact[], releaseId: number) { + const response = await this.releases.listArtifactsForRelease(releaseId) + const releaseAssets = response.data + const assetByName = new Map() + releaseAssets.forEach(asset => { + assetByName[asset.name] = asset }); + for (const artifact of artifacts) { + const asset = assetByName[artifact.name] + if (asset) { + await this.releases.deleteArtifact(asset.id) + } + } } } \ No newline at end of file diff --git a/src/Inputs.ts b/src/Inputs.ts index d11d2b8..8e88777 100644 --- a/src/Inputs.ts +++ b/src/Inputs.ts @@ -12,6 +12,7 @@ export interface Inputs { readonly draft: boolean readonly name: string readonly prerelease: boolean + readonly replacesArtifacts: boolean readonly tag: string readonly token: string } @@ -79,8 +80,13 @@ export class CoreInputs implements Inputs { } get prerelease(): boolean { - const draft = core.getInput('prerelease') - return draft == 'true' + const preRelease = core.getInput('prerelease') + return preRelease == 'true' + } + + get replacesArtifacts(): boolean { + const replaces = core.getInput('replacesArtifacts') + return replaces == 'true' } get tag(): string { diff --git a/src/Main.ts b/src/Main.ts index 2c1abdf..00e83f0 100644 --- a/src/Main.ts +++ b/src/Main.ts @@ -25,7 +25,7 @@ function createAction(): Action { const inputs = new CoreInputs(globber, context) const releases = new GithubReleases(context, git) - const uploader = new GithubArtifactUploader(releases) + const uploader = new GithubArtifactUploader(releases, inputs.replacesArtifacts) return new Action(inputs, releases, uploader) } diff --git a/src/Releases.ts b/src/Releases.ts index 5c86207..cef79e3 100644 --- a/src/Releases.ts +++ b/src/Releases.ts @@ -1,6 +1,6 @@ import { Context } from "@actions/github/lib/context"; import { GitHub } from "@actions/github"; -import { AnyResponse, Response, ReposCreateReleaseResponse, ReposGetReleaseByTagResponse, ReposListReleasesResponse } from "@octokit/rest"; +import { AnyResponse, Response, ReposDeleteReleaseAssetResponse, ReposListAssetsForReleaseResponse, ReposCreateReleaseResponse, ReposGetReleaseByTagResponse, ReposListReleasesResponse } from "@octokit/rest"; export interface Releases { create( @@ -12,8 +12,12 @@ export interface Releases { prerelease?: boolean ): Promise> + deleteArtifact(assetId: number): Promise> + getByTag(tag: string): Promise> + listArtifactsForRelease(releaseId: number): Promise> + listReleases(): Promise> update( @@ -64,6 +68,26 @@ export class GithubReleases implements Releases { }) } + async deleteArtifact( + assetId: number + ): Promise> { + return this.git.repos.deleteReleaseAsset({ + asset_id: assetId, + owner: this.context.repo.owner, + repo: this.context.repo.repo + }) + } + + async listArtifactsForRelease( + releaseId: number + ): Promise> { + return this.git.repos.listAssetsForRelease({ + owner: this.context.repo.owner, + release_id: releaseId, + repo: this.context.repo.repo + }) + } + async listReleases(): Promise> { return this.git.repos.listReleases({ owner: this.context.repo.owner,