fix: fetch correct asset URL after finalization; test; some refactoring (#738)

README.md

@@ -213,7 +213,7 @@ The following outputs can be accessed via `${{ steps.<step-id>.outputs }}` from
 | `url`        | String | Github.com URL for the release                                                                                                                                                            |
 | `id`         | String | Release ID                                                                                                                                                                                |
 | `upload_url` | String | URL for uploading assets to the release                                                                                                                                                   |
-| `assets`     | String | JSON array containing information about each uploaded asset, in the format given [here](https://docs.github.com/en/rest/releases/assets#get-a-release-asset) (minus the `uploader` field) |
+| `assets`     | String | JSON array containing information about each updated (newly uploaded or overwritten) asset, in the format given [here](https://docs.github.com/en/rest/releases/assets#get-a-release-asset) (minus the `uploader` field) |
 
 As an example, you can use `${{ fromJSON(steps.<step-id>.outputs.assets)[0].browser_download_url }}` to get the download URL of the first asset.
 

__tests__/github.test.ts

@@ -1,15 +1,37 @@
 import {
   asset,
   findTagFromReleases,
+  finalizeRelease,
   mimeOrDefault,
   release,
   Release,
   Releaser,
 } from '../src/github';
 
-import { assert, describe, it } from 'vitest';
+import { assert, describe, expect, it, vi } from 'vitest';
 
 describe('github', () => {
+  const config = {
+    github_token: 'test-token',
+    github_ref: 'refs/tags/v1.0.0',
+    github_repository: 'owner/repo',
+    input_tag_name: undefined,
+    input_name: undefined,
+    input_body: undefined,
+    input_body_path: undefined,
+    input_files: [],
+    input_draft: undefined,
+    input_prerelease: undefined,
+    input_preserve_order: undefined,
+    input_overwrite_files: undefined,
+    input_fail_on_unmatched_files: false,
+    input_target_commitish: undefined,
+    input_discussion_category_name: undefined,
+    input_generate_release_notes: false,
+    input_append_body: false,
+    input_make_latest: undefined,
+  };
+
   describe('mimeOrDefault', () => {
     it('returns a specific mime for common path', async () => {
       assert.equal(mimeOrDefault('foo.tar.gz'), 'application/gzip');
@@ -53,6 +75,9 @@ describe('github', () => {
       allReleases: async function* () {
         yield { data: [mockRelease] };
       },
+      listReleaseAssets: () => Promise.reject('Not implemented'),
+      deleteReleaseAsset: () => Promise.reject('Not implemented'),
+      uploadReleaseAsset: () => Promise.reject('Not implemented'),
     } as const;
 
     describe('when the tag_name is not an empty string', () => {
@@ -236,6 +261,76 @@ describe('github', () => {
     });
   });
 
+  describe('finalizeRelease input_draft behavior', () => {
+    const draftRelease: Release = {
+      id: 1,
+      upload_url: 'test',
+      html_url: 'test',
+      tag_name: 'v1.0.0',
+      name: 'test',
+      body: 'test',
+      target_commitish: 'main',
+      draft: true,
+      prerelease: false,
+      assets: [],
+    };
+
+    const finalizedRelease: Release = {
+      ...draftRelease,
+      draft: false,
+    };
+
+    it.each([
+      {
+        name: 'returns early when input_draft is true',
+        input_draft: true,
+        expectedCalls: 0,
+        expectedResult: draftRelease,
+      },
+      {
+        name: 'finalizes release when input_draft is false',
+        input_draft: false,
+        expectedCalls: 1,
+        expectedResult: finalizedRelease,
+      },
+    ])('$name', async ({ input_draft, expectedCalls, expectedResult }) => {
+      const finalizeReleaseSpy = vi.fn(async () => ({ data: finalizedRelease }));
+
+      const releaser: Releaser = {
+        getReleaseByTag: () => Promise.reject('Not implemented'),
+        createRelease: () => Promise.reject('Not implemented'),
+        updateRelease: () => Promise.reject('Not implemented'),
+        finalizeRelease: finalizeReleaseSpy,
+        allReleases: async function* () {
+          throw new Error('Not implemented');
+        },
+        listReleaseAssets: () => Promise.reject('Not implemented'),
+        deleteReleaseAsset: () => Promise.reject('Not implemented'),
+        uploadReleaseAsset: () => Promise.reject('Not implemented'),
+      };
+
+      const result = await finalizeRelease(
+        {
+          ...config,
+          input_draft,
+        },
+        releaser,
+        draftRelease,
+      );
+
+      expect(finalizeReleaseSpy).toHaveBeenCalledTimes(expectedCalls);
+      assert.strictEqual(result, expectedResult);
+
+      if (expectedCalls === 1) {
+        expect(finalizeReleaseSpy).toHaveBeenCalledWith({
+          owner: 'owner',
+          repo: 'repo',
+          release_id: draftRelease.id,
+        });
+      }
+    });
+  });
+
   describe('error handling', () => {
     it('handles 422 already_exists error gracefully', async () => {
       const mockReleaser: Releaser = {
@@ -260,7 +355,7 @@ describe('github', () => {
               assets: [],
             },
           }),
-        finalizeRelease: async () => {},
+        finalizeRelease: () => Promise.reject('Not implemented'),
         allReleases: async function* () {
           yield {
             data: [
@@ -279,29 +374,11 @@ describe('github', () => {
             ],
           };
         },
+        listReleaseAssets: () => Promise.reject('Not implemented'),
+        deleteReleaseAsset: () => Promise.reject('Not implemented'),
+        uploadReleaseAsset: () => Promise.reject('Not implemented'),
       } as const;
 
-      const config = {
-        github_token: 'test-token',
-        github_ref: 'refs/tags/v1.0.0',
-        github_repository: 'owner/repo',
-        input_tag_name: undefined,
-        input_name: undefined,
-        input_body: undefined,
-        input_body_path: undefined,
-        input_files: [],
-        input_draft: undefined,
-        input_prerelease: undefined,
-        input_preserve_order: undefined,
-        input_overwrite_files: undefined,
-        input_fail_on_unmatched_files: false,
-        input_target_commitish: undefined,
-        input_discussion_category_name: undefined,
-        input_generate_release_notes: false,
-        input_append_body: false,
-        input_make_latest: undefined,
-      };
-
       const result = await release(config, mockReleaser, 1);
       assert.ok(result);
       assert.equal(result.id, 1);

src/github.ts

@@ -65,6 +65,22 @@ export interface Releaser {
   }): Promise<{ data: Release }>;
 
   allReleases(params: { owner: string; repo: string }): AsyncIterable<{ data: Release[] }>;
+
+  listReleaseAssets(params: {
+    owner: string;
+    repo: string;
+    release_id: number;
+  }): Promise<Array<{ id: number; name: string; [key: string]: any }>>;
+
+  deleteReleaseAsset(params: { owner: string; repo: string; asset_id: number }): Promise<void>;
+
+  uploadReleaseAsset(params: {
+    url: string;
+    size: number;
+    mime: string;
+    token: string;
+    data: any;
+  }): Promise<{ status: number; data: any }>;
 }
 
 export class GitHubReleaser implements Releaser {
@@ -181,6 +197,44 @@ export class GitHubReleaser implements Releaser {
       this.github.rest.repos.listReleases.endpoint.merge(updatedParams),
     );
   }
+
+  async listReleaseAssets(params: {
+    owner: string;
+    repo: string;
+    release_id: number;
+  }): Promise<Array<{ id: number; name: string; [key: string]: any }>> {
+    return this.github.paginate(this.github.rest.repos.listReleaseAssets, {
+      ...params,
+      per_page: 100,
+    });
+  }
+
+  async deleteReleaseAsset(params: {
+    owner: string;
+    repo: string;
+    asset_id: number;
+  }): Promise<void> {
+    await this.github.rest.repos.deleteReleaseAsset(params);
+  }
+
+  async uploadReleaseAsset(params: {
+    url: string;
+    size: number;
+    mime: string;
+    token: string;
+    data: any;
+  }): Promise<{ status: number; data: any }> {
+    return this.github.request({
+      method: 'POST',
+      url: params.url,
+      headers: {
+        'content-length': `${params.size}`,
+        'content-type': params.mime,
+        authorization: `token ${params.token}`,
+      },
+      data: params.data,
+    });
+  }
 }
 
 export const asset = (path: string): ReleaseAsset => {
@@ -197,7 +251,7 @@ export const mimeOrDefault = (path: string): string => {
 
 export const upload = async (
   config: Config,
-  github: GitHub,
+  releaser: Releaser,
   url: string,
   path: string,
   currentAssets: Array<{ id: number; name: string }>,
@@ -216,7 +270,7 @@ export const upload = async (
       return null;
     } else {
       console.log(`♻️ Deleting previously uploaded asset ${name}...`);
-      await github.rest.repos.deleteReleaseAsset({
+      await releaser.deleteReleaseAsset({
         asset_id: currentAsset.id || 1,
         owner,
         repo,
@@ -228,14 +282,11 @@ export const upload = async (
   endpoint.searchParams.append('name', name);
   const fh = await open(path);
   try {
-    const resp = await github.request({
+    const resp = await releaser.uploadReleaseAsset({
-      method: 'POST',
       url: endpoint.toString(),
-      headers: {
+      size,
-        'content-length': `${size}`,
+      mime,
-        'content-type': mime,
+      token: config.github_token,
-        authorization: `token ${config.github_token}`,
-      },
       data: fh.readableWebStream({ type: 'bytes' }),
     });
     const json = resp.data;
@@ -399,6 +450,41 @@ export const finalizeRelease = async (
   }
 };
 
+/**
+ * Lists assets belonging to a release.
+ *
+ * @param config - Release configuration as specified by user
+ * @param releaser - The GitHub API wrapper for release operations
+ * @param release - The existing release to be checked
+ * @param maxRetries - The maximum number of attempts
+ */
+export const listReleaseAssets = async (
+  config: Config,
+  releaser: Releaser,
+  release: Release,
+  maxRetries: number = 3,
+): Promise<Array<{ id: number; name: string; [key: string]: any }>> => {
+  if (maxRetries <= 0) {
+    console.log(`❌ Too many retries. Aborting...`);
+    throw new Error('Too many retries.');
+  }
+
+  const [owner, repo] = config.github_repository.split('/');
+  try {
+    const assets = await releaser.listReleaseAssets({
+      owner,
+      repo,
+      release_id: release.id,
+    });
+
+    return assets;
+  } catch (error) {
+    console.warn(`error listing assets of release: ${error}`);
+    console.log(`retrying... (${maxRetries - 1} retries remaining)`);
+    return listReleaseAssets(config, releaser, release, maxRetries - 1);
+  }
+};
+
 /**
  * Finds a release by tag name from all a repository's releases.
  *

src/main.ts

@@ -1,6 +1,6 @@
 import { setFailed, setOutput } from '@actions/core';
 import { getOctokit } from '@actions/github';
-import { GitHubReleaser, release, finalizeRelease, upload } from './github';
+import { GitHubReleaser, release, finalizeRelease, upload, listReleaseAssets } from './github';
 import { isTag, parseConfig, paths, unmatchedPatterns, uploadUrl } from './util';
 
 import { env } from 'process';
@@ -50,6 +50,7 @@ async function run() {
     //);
     const releaser = new GitHubReleaser(gh);
     let rel = await release(config, releaser);
+    let uploadedAssetIds: Set<number> = new Set();
     if (config.input_files && config.input_files.length > 0) {
       const files = paths(config.input_files, config.input_working_directory);
       if (files.length == 0) {
@@ -61,15 +62,12 @@ async function run() {
       }
       const currentAssets = rel.assets;
 
-      const uploadFile = async (path) => {
+      const uploadFile = async (path: string) => {
-        const json = await upload(config, gh, uploadUrl(rel.upload_url), path, currentAssets);
+        const json = await upload(config, releaser, uploadUrl(rel.upload_url), path, currentAssets);
-        if (json) {
+        return json ? (json.id as number) : undefined;
-          delete json.uploader;
-        }
-        return json;
       };
 
-      let results: (any | null)[];
+      let results: (number | undefined)[];
       if (!config.input_preserve_order) {
         results = await Promise.all(files.map(uploadFile));
       } else {
@@ -79,13 +77,29 @@ async function run() {
         }
       }
 
-      const assets = results.filter(Boolean);
+      uploadedAssetIds = new Set(results.filter((id): id is number => id !== undefined));
-      setOutput('assets', assets);
     }
 
     console.log('Finalizing release...');
     rel = await finalizeRelease(config, releaser, rel);
 
+    // Draft releases use temporary "untagged-..." URLs for assets.
+    // URLs will be changed to correct ones once the release is published.
+    console.log('Getting assets list...');
+    {
+      let assets: any[] = [];
+      if (uploadedAssetIds.size > 0) {
+        const updatedAssets = await listReleaseAssets(config, releaser, rel);
+        assets = updatedAssets
+          .filter((a) => uploadedAssetIds.has(a.id))
+          .map((a) => {
+            const { uploader, ...rest } = a;
+            return rest;
+          });
+      }
+      setOutput('assets', assets);
+    }
+
     console.log(`🎉 Release ready at ${rel.html_url}`);
     setOutput('url', rel.html_url);
     setOutput('id', rel.id.toString());