diff --git a/packages/artifact/__tests__/get-artifact.test.ts b/packages/artifact/__tests__/get-artifact.test.ts new file mode 100644 index 00000000..82008a4b --- /dev/null +++ b/packages/artifact/__tests__/get-artifact.test.ts @@ -0,0 +1,245 @@ +import * as github from '@actions/github' +import type {RequestInterface} from '@octokit/types' +import { + getArtifactInternal, + getArtifactPublic +} from '../src/internal/find/get-artifact' +import * as config from '../src/internal/shared/config' +import {ArtifactServiceClientJSON, Timestamp} from '../src/generated' +import * as util from '../src/internal/shared/util' +import {noopLogs} from './common' + +type MockedRequest = jest.MockedFunction> + +jest.mock('@actions/github', () => ({ + getOctokit: jest.fn().mockReturnValue({ + request: jest.fn() + }) +})) + +const fixtures = { + repo: 'toolkit', + owner: 'actions', + token: 'ghp_1234567890', + runId: 123, + backendIds: { + workflowRunBackendId: 'c4d7c21f-ba3f-4ddc-a8c8-6f2f626f8422', + workflowJobRunBackendId: '760803a1-f890-4d25-9a6e-a3fc01a0c7cf' + }, + artifacts: [ + { + id: 1, + name: 'my-artifact', + size: 456, + createdAt: new Date('2023-12-01') + }, + { + id: 2, + name: 'my-artifact', + size: 456, + createdAt: new Date('2023-12-02') + } + ] +} + +describe('get-artifact', () => { + beforeAll(() => { + noopLogs() + }) + + describe('public', () => { + it('should return the artifact if it is found', async () => { + const mockRequest = github.getOctokit(fixtures.token) + .request as MockedRequest + mockRequest.mockResolvedValueOnce({ + status: 200, + headers: {}, + url: '', + data: { + artifacts: [ + { + name: fixtures.artifacts[0].name, + id: fixtures.artifacts[0].id, + size_in_bytes: fixtures.artifacts[0].size, + created_at: fixtures.artifacts[0].createdAt.toISOString() + } + ] + } + }) + + const response = await getArtifactPublic( + fixtures.artifacts[0].name, + fixtures.runId, + fixtures.owner, + fixtures.repo, + fixtures.token + ) + + expect(response).toEqual({ + success: true, + artifact: fixtures.artifacts[0] + }) + }) + + it('should return the latest artifact if multiple are found', async () => { + const mockRequest = github.getOctokit(fixtures.token) + .request as MockedRequest + mockRequest.mockResolvedValueOnce({ + status: 200, + headers: {}, + url: '', + data: { + artifacts: fixtures.artifacts.map(artifact => ({ + name: artifact.name, + id: artifact.id, + size_in_bytes: artifact.size, + created_at: artifact.createdAt.toISOString() + })) + } + }) + + const response = await getArtifactPublic( + fixtures.artifacts[0].name, + fixtures.runId, + fixtures.owner, + fixtures.repo, + fixtures.token + ) + + expect(response).toEqual({ + success: true, + artifact: fixtures.artifacts[1] + }) + }) + + it('should fail if no artifacts are found', async () => { + const mockRequest = github.getOctokit(fixtures.token) + .request as MockedRequest + mockRequest.mockResolvedValueOnce({ + status: 200, + headers: {}, + url: '', + data: { + artifacts: [] + } + }) + + const response = await getArtifactPublic( + fixtures.artifacts[0].name, + fixtures.runId, + fixtures.owner, + fixtures.repo, + fixtures.token + ) + + expect(response).toEqual({ + success: false + }) + }) + + it('should fail if non-200 response', async () => { + const mockRequest = github.getOctokit(fixtures.token) + .request as MockedRequest + mockRequest.mockResolvedValueOnce({ + status: 404, + headers: {}, + url: '', + data: {} + }) + + const response = await getArtifactPublic( + fixtures.artifacts[0].name, + fixtures.runId, + fixtures.owner, + fixtures.repo, + fixtures.token + ) + + expect(response).toEqual({ + success: false + }) + }) + }) + + describe('internal', () => { + beforeEach(() => { + jest.spyOn(config, 'getRuntimeToken').mockReturnValue('test-token') + + jest + .spyOn(util, 'getBackendIdsFromToken') + .mockReturnValue(fixtures.backendIds) + + jest + .spyOn(config, 'getResultsServiceUrl') + .mockReturnValue('https://results.local') + }) + + it('should return the artifact if it is found', async () => { + jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockResolvedValue({ + artifacts: [ + { + ...fixtures.backendIds, + databaseId: fixtures.artifacts[0].id.toString(), + name: fixtures.artifacts[0].name, + size: fixtures.artifacts[0].size.toString(), + createdAt: Timestamp.fromDate(fixtures.artifacts[0].createdAt) + } + ] + }) + + const response = await getArtifactInternal(fixtures.artifacts[0].name) + + expect(response).toEqual({ + success: true, + artifact: fixtures.artifacts[0] + }) + }) + + it('should return the latest artifact if multiple are found', async () => { + jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockResolvedValue({ + artifacts: fixtures.artifacts.map(artifact => ({ + ...fixtures.backendIds, + databaseId: artifact.id.toString(), + name: artifact.name, + size: artifact.size.toString(), + createdAt: Timestamp.fromDate(artifact.createdAt) + })) + }) + + const response = await getArtifactInternal(fixtures.artifacts[0].name) + + expect(response).toEqual({ + success: true, + artifact: fixtures.artifacts[1] + }) + }) + + it('should fail if no artifacts are found', async () => { + jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockResolvedValue({ + artifacts: [] + }) + + const response = await getArtifactInternal(fixtures.artifacts[0].name) + + expect(response).toEqual({ + success: false + }) + }) + + it('should fail if non-200 response', async () => { + jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockRejectedValue(new Error('test error')) + + await expect( + getArtifactInternal(fixtures.artifacts[0].name) + ).rejects.toThrow('test error') + }) + }) +}) diff --git a/packages/artifact/__tests__/list-artifacts.test.ts b/packages/artifact/__tests__/list-artifacts.test.ts new file mode 100644 index 00000000..7c8699e7 --- /dev/null +++ b/packages/artifact/__tests__/list-artifacts.test.ts @@ -0,0 +1,242 @@ +import * as github from '@actions/github' +import type {RestEndpointMethods} from '@octokit/plugin-rest-endpoint-methods/dist-types/generated/method-types' +import type {RestEndpointMethodTypes} from '@octokit/plugin-rest-endpoint-methods/dist-types/generated/parameters-and-response-types' +import { + listArtifactsInternal, + listArtifactsPublic +} from '../src/internal/find/list-artifacts' +import * as config from '../src/internal/shared/config' +import {ArtifactServiceClientJSON, Timestamp} from '../src/generated' +import * as util from '../src/internal/shared/util' +import {noopLogs} from './common' +import {Artifact} from '../src/internal/shared/interfaces' + +type MockedListWorkflowRunArtifacts = jest.MockedFunction< + RestEndpointMethods['actions']['listWorkflowRunArtifacts'] +> + +jest.mock('@actions/github', () => ({ + getOctokit: jest.fn().mockReturnValue({ + rest: { + actions: { + listWorkflowRunArtifacts: jest.fn() + } + } + }) +})) + +const artifactsToListResponse = ( + artifacts: Artifact[] +): RestEndpointMethodTypes['actions']['listWorkflowRunArtifacts']['response']['data'] => { + return { + total_count: artifacts.length, + artifacts: artifacts.map(artifact => ({ + name: artifact.name, + id: artifact.id, + size_in_bytes: artifact.size, + created_at: artifact.createdAt?.toISOString() || '', + run_id: fixtures.runId, + // unused fields for tests + url: '', + archive_download_url: '', + expired: false, + expires_at: '', + node_id: '', + run_url: '', + type: '', + updated_at: '' + })) + } +} + +const fixtures = { + repo: 'toolkit', + owner: 'actions', + token: 'ghp_1234567890', + runId: 123, + backendIds: { + workflowRunBackendId: 'c4d7c21f-ba3f-4ddc-a8c8-6f2f626f8422', + workflowJobRunBackendId: '760803a1-f890-4d25-9a6e-a3fc01a0c7cf' + }, + artifacts: [ + { + id: 1, + name: 'my-artifact', + size: 456, + createdAt: new Date('2023-12-01') + }, + { + id: 2, + name: 'my-artifact', + size: 456, + createdAt: new Date('2023-12-02') + } + ] +} + +describe('list-artifact', () => { + beforeAll(() => { + noopLogs() + }) + + describe('public', () => { + it('should return a list of artifacts', async () => { + const mockListArtifacts = github.getOctokit(fixtures.token).rest.actions + .listWorkflowRunArtifacts as MockedListWorkflowRunArtifacts + + mockListArtifacts.mockResolvedValueOnce({ + status: 200, + headers: {}, + url: '', + data: artifactsToListResponse(fixtures.artifacts) + }) + + const response = await listArtifactsPublic( + fixtures.runId, + fixtures.owner, + fixtures.repo, + fixtures.token, + false + ) + + expect(response).toEqual({ + artifacts: fixtures.artifacts + }) + }) + + it('should return the latest artifact when latest is specified', async () => { + const mockListArtifacts = github.getOctokit(fixtures.token).rest.actions + .listWorkflowRunArtifacts as MockedListWorkflowRunArtifacts + + mockListArtifacts.mockResolvedValueOnce({ + status: 200, + headers: {}, + url: '', + data: artifactsToListResponse(fixtures.artifacts) + }) + + const response = await listArtifactsPublic( + fixtures.runId, + fixtures.owner, + fixtures.repo, + fixtures.token, + true + ) + + expect(response).toEqual({ + artifacts: [fixtures.artifacts[1]] + }) + }) + + it('can return empty artifacts', async () => { + const mockListArtifacts = github.getOctokit(fixtures.token).rest.actions + .listWorkflowRunArtifacts as MockedListWorkflowRunArtifacts + + mockListArtifacts.mockResolvedValueOnce({ + status: 200, + headers: {}, + url: '', + data: { + total_count: 0, + artifacts: [] + } + }) + + const response = await listArtifactsPublic( + fixtures.runId, + fixtures.owner, + fixtures.repo, + fixtures.token, + true + ) + + expect(response).toEqual({ + artifacts: [] + }) + }) + + it('should fail if non-200 response', async () => { + const mockListArtifacts = github.getOctokit(fixtures.token).rest.actions + .listWorkflowRunArtifacts as MockedListWorkflowRunArtifacts + + mockListArtifacts.mockRejectedValue(new Error('boom')) + + await expect( + listArtifactsPublic( + fixtures.runId, + fixtures.owner, + fixtures.repo, + fixtures.token, + false + ) + ).rejects.toThrow('boom') + }) + }) + + describe('internal', () => { + beforeEach(() => { + jest.spyOn(config, 'getRuntimeToken').mockReturnValue('test-token') + jest + .spyOn(util, 'getBackendIdsFromToken') + .mockReturnValue(fixtures.backendIds) + jest + .spyOn(config, 'getResultsServiceUrl') + .mockReturnValue('https://results.local') + }) + + it('should return a list of artifacts', async () => { + jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockResolvedValue({ + artifacts: fixtures.artifacts.map(artifact => ({ + ...fixtures.backendIds, + databaseId: artifact.id.toString(), + name: artifact.name, + size: artifact.size.toString(), + createdAt: Timestamp.fromDate(artifact.createdAt) + })) + }) + const response = await listArtifactsInternal(false) + expect(response).toEqual({ + artifacts: fixtures.artifacts + }) + }) + + it('should return the latest artifact when latest is specified', async () => { + jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockResolvedValue({ + artifacts: fixtures.artifacts.map(artifact => ({ + ...fixtures.backendIds, + databaseId: artifact.id.toString(), + name: artifact.name, + size: artifact.size.toString(), + createdAt: Timestamp.fromDate(artifact.createdAt) + })) + }) + const response = await listArtifactsInternal(true) + expect(response).toEqual({ + artifacts: [fixtures.artifacts[1]] + }) + }) + + it('can return empty artifacts', async () => { + jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockResolvedValue({ + artifacts: [] + }) + const response = await listArtifactsInternal(false) + expect(response).toEqual({ + artifacts: [] + }) + }) + + it('should fail if non-200 response', async () => { + jest + .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') + .mockRejectedValue(new Error('boom')) + await expect(listArtifactsInternal(false)).rejects.toThrow('boom') + }) + }) +}) diff --git a/packages/artifact/src/generated/results/api/v1/artifact.ts b/packages/artifact/src/generated/results/api/v1/artifact.ts index adabae5e..acce3037 100644 --- a/packages/artifact/src/generated/results/api/v1/artifact.ts +++ b/packages/artifact/src/generated/results/api/v1/artifact.ts @@ -163,6 +163,12 @@ export interface ListArtifactsResponse_MonolithArtifact { * @generated from protobuf field: int64 size = 5; */ size: string; + /** + * When the artifact was created in the monolith + * + * @generated from protobuf field: google.protobuf.Timestamp created_at = 6; + */ + createdAt?: Timestamp; } /** * @generated from protobuf message github.actions.results.api.v1.GetSignedArtifactURLRequest @@ -571,7 +577,8 @@ class ListArtifactsResponse_MonolithArtifact$Type extends MessageType Timestamp } ]); } create(value?: PartialMessage): ListArtifactsResponse_MonolithArtifact { @@ -601,6 +608,9 @@ class ListArtifactsResponse_MonolithArtifact$Type extends MessageType + options?: UploadArtifactOptions + ): Promise /** * Lists all artifacts that are part of the current workflow run. @@ -44,10 +45,13 @@ export interface ArtifactClient { * @param options Extra options that allow for the customization of the list behavior * @returns ListArtifactResponse object */ - listArtifacts(options?: FindOptions): Promise + listArtifacts( + options?: ListArtifactsOptions & FindOptions + ): Promise /** * Finds an artifact by name. + * If there are multiple artifacts with the same name in the same workflow run, this will return the latest. * * If options.findBy is specified, this will use the public List Artifacts API with a name filter which can get artifacts from other runs. * https://docs.github.com/en/rest/actions/artifacts?apiVersion=2022-11-28#list-workflow-run-artifacts @@ -93,8 +97,8 @@ export class Client implements ArtifactClient { name: string, files: string[], rootDirectory: string, - options?: UploadOptions - ): Promise { + options?: UploadArtifactOptions + ): Promise { if (isGhes()) { warning( `@actions/artifact v2.0.0+ and upload-artifact@v4+ are not currently supported on GHES.` @@ -171,7 +175,9 @@ If the error persists, please check whether Actions and API requests are operati /** * List Artifacts */ - async listArtifacts(options?: FindOptions): Promise { + async listArtifacts( + options?: ListArtifactsOptions & FindOptions + ): Promise { if (isGhes()) { warning( `@actions/artifact v2.0.0+ and download-artifact@v4+ are not currently supported on GHES.` @@ -191,11 +197,12 @@ If the error persists, please check whether Actions and API requests are operati workflowRunId, repositoryOwner, repositoryName, - token + token, + options?.latest ) } - return listArtifactsInternal() + return listArtifactsInternal(options?.latest) } catch (error: unknown) { warning( `Listing Artifacts failed with error: ${error}. diff --git a/packages/artifact/src/internal/find/get-artifact.ts b/packages/artifact/src/internal/find/get-artifact.ts index c15a653b..91c85818 100644 --- a/packages/artifact/src/internal/find/get-artifact.ts +++ b/packages/artifact/src/internal/find/get-artifact.ts @@ -9,7 +9,7 @@ import {GetArtifactResponse} from '../shared/interfaces' import {getBackendIdsFromToken} from '../shared/util' import {getUserAgentString} from '../shared/user-agent' import {internalArtifactTwirpClient} from '../shared/artifact-twirp-client' -import {ListArtifactsRequest, StringValue} from '../../generated' +import {ListArtifactsRequest, StringValue, Timestamp} from '../../generated' export async function getArtifactPublic( artifactName: string, @@ -54,18 +54,21 @@ export async function getArtifactPublic( } } + let artifact = getArtifactResp.data.artifacts[0] if (getArtifactResp.data.artifacts.length > 1) { - core.warning( - 'more than one artifact found for a single name, returning first' + artifact = getArtifactResp.data.artifacts.sort((a, b) => b.id - a.id)[0] + core.debug( + `More than one artifact found for a single name, returning newest (id: ${artifact.id})` ) } return { success: true, artifact: { - name: getArtifactResp.data.artifacts[0].name, - id: getArtifactResp.data.artifacts[0].id, - size: getArtifactResp.data.artifacts[0].size_in_bytes + name: artifact.name, + id: artifact.id, + size: artifact.size_in_bytes, + createdAt: artifact.created_at ? new Date(artifact.created_at) : undefined } } } @@ -93,26 +96,26 @@ export async function getArtifactInternal( } } + let artifact = res.artifacts[0] if (res.artifacts.length > 1) { - core.warning( - 'more than one artifact found for a single name, returning first' + artifact = res.artifacts.sort( + (a, b) => Number(b.databaseId) - Number(a.databaseId) + )[0] + + core.debug( + `more than one artifact found for a single name, returning newest (id: ${artifact.databaseId})` ) } - // In the case of reruns, we may have artifacts with the same name scoped under the same workflow run. - // Let's prefer the artifact closest scoped to this run. - // If it doesn't exist (e.g. partial rerun) we'll use the first match. - const artifact = - res.artifacts.find( - artifact => artifact.workflowRunBackendId === workflowRunBackendId - ) || res.artifacts[0] - return { success: true, artifact: { name: artifact.name, id: Number(artifact.databaseId), - size: Number(artifact.size) + size: Number(artifact.size), + createdAt: artifact.createdAt + ? Timestamp.toDate(artifact.createdAt) + : undefined } } } diff --git a/packages/artifact/src/internal/find/list-artifacts.ts b/packages/artifact/src/internal/find/list-artifacts.ts index c25a58bd..cd83320c 100644 --- a/packages/artifact/src/internal/find/list-artifacts.ts +++ b/packages/artifact/src/internal/find/list-artifacts.ts @@ -9,7 +9,7 @@ import {retry} from '@octokit/plugin-retry' import {OctokitOptions} from '@octokit/core/dist-types/types' import {internalArtifactTwirpClient} from '../shared/artifact-twirp-client' import {getBackendIdsFromToken} from '../shared/util' -import {ListArtifactsRequest} from 'src/generated' +import {ListArtifactsRequest, Timestamp} from '../../generated' // Limiting to 1000 for perf reasons const maximumArtifactCount = 1000 @@ -20,13 +20,14 @@ export async function listArtifactsPublic( workflowRunId: number, repositoryOwner: string, repositoryName: string, - token: string + token: string, + latest = false ): Promise { info( `Fetching artifact list for workflow run ${workflowRunId} in repository ${repositoryOwner}/${repositoryName}` ) - const artifacts: Artifact[] = [] + let artifacts: Artifact[] = [] const [retryOpts, requestOpts] = getRetryOptions(defaultGitHubOptions) const opts: OctokitOptions = { @@ -65,7 +66,8 @@ export async function listArtifactsPublic( artifacts.push({ name: artifact.name, id: artifact.id, - size: artifact.size_in_bytes + size: artifact.size_in_bytes, + createdAt: artifact.created_at ? new Date(artifact.created_at) : undefined }) } @@ -91,11 +93,18 @@ export async function listArtifactsPublic( artifacts.push({ name: artifact.name, id: artifact.id, - size: artifact.size_in_bytes + size: artifact.size_in_bytes, + createdAt: artifact.created_at + ? new Date(artifact.created_at) + : undefined }) } } + if (latest) { + artifacts = filterLatest(artifacts) + } + info(`Found ${artifacts.length} artifact(s)`) return { @@ -103,7 +112,9 @@ export async function listArtifactsPublic( } } -export async function listArtifactsInternal(): Promise { +export async function listArtifactsInternal( + latest = false +): Promise { const artifactClient = internalArtifactTwirpClient() const {workflowRunBackendId, workflowJobRunBackendId} = @@ -115,15 +126,40 @@ export async function listArtifactsInternal(): Promise { } const res = await artifactClient.ListArtifacts(req) - const artifacts = res.artifacts.map(artifact => ({ + let artifacts: Artifact[] = res.artifacts.map(artifact => ({ name: artifact.name, id: Number(artifact.databaseId), - size: Number(artifact.size) + size: Number(artifact.size), + createdAt: artifact.createdAt + ? Timestamp.toDate(artifact.createdAt) + : undefined })) + if (latest) { + artifacts = filterLatest(artifacts) + } + info(`Found ${artifacts.length} artifact(s)`) return { artifacts } } + +/** + * Filters a list of artifacts to only include the latest artifact for each name + * @param artifacts The artifacts to filter + * @returns The filtered list of artifacts + */ +function filterLatest(artifacts: Artifact[]): Artifact[] { + artifacts.sort((a, b) => b.id - a.id) + const latestArtifacts: Artifact[] = [] + const seenArtifactNames = new Set() + for (const artifact of artifacts) { + if (!seenArtifactNames.has(artifact.name)) { + latestArtifacts.push(artifact) + seenArtifactNames.add(artifact.name) + } + } + return latestArtifacts +} diff --git a/packages/artifact/src/internal/shared/interfaces.ts b/packages/artifact/src/internal/shared/interfaces.ts index bd764412..61994b2b 100644 --- a/packages/artifact/src/internal/shared/interfaces.ts +++ b/packages/artifact/src/internal/shared/interfaces.ts @@ -3,7 +3,7 @@ * UploadArtifact * * * *****************************************************************************/ -export interface UploadResponse { +export interface UploadArtifactResponse { /** * Denotes if an artifact was successfully uploaded */ @@ -21,7 +21,7 @@ export interface UploadResponse { id?: number } -export interface UploadOptions { +export interface UploadArtifactOptions { /** * Duration after which artifact will expire in days. * @@ -74,6 +74,15 @@ export interface GetArtifactResponse { * ListArtifact * * * *****************************************************************************/ + +export interface ListArtifactsOptions { + /** + * Filter the workflow run's artifacts to the latest by name + * In the case of reruns, this can be useful to avoid duplicates + */ + latest?: boolean +} + export interface ListArtifactsResponse { /** * A list of artifacts that were found @@ -124,6 +133,11 @@ export interface Artifact { * The size of the artifact in bytes */ size: number + + /** + * The time when the artifact was created + */ + createdAt?: Date } // FindOptions are for fetching Artifact(s) out of the scope of the current run. diff --git a/packages/artifact/src/internal/upload/upload-artifact.ts b/packages/artifact/src/internal/upload/upload-artifact.ts index 011dc4c8..6546f98f 100644 --- a/packages/artifact/src/internal/upload/upload-artifact.ts +++ b/packages/artifact/src/internal/upload/upload-artifact.ts @@ -1,5 +1,8 @@ import * as core from '@actions/core' -import {UploadOptions, UploadResponse} from '../shared/interfaces' +import { + UploadArtifactOptions, + UploadArtifactResponse +} from '../shared/interfaces' import {getExpiration} from './retention' import {validateArtifactName} from './path-and-artifact-name-validation' import {internalArtifactTwirpClient} from '../shared/artifact-twirp-client' @@ -21,8 +24,8 @@ export async function uploadArtifact( name: string, files: string[], rootDirectory: string, - options?: UploadOptions | undefined -): Promise { + options?: UploadArtifactOptions | undefined +): Promise { validateArtifactName(name) validateRootDirectory(rootDirectory)