From df166709a33b5c2af2322833ce0e038e71a2f71e Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Thu, 28 Nov 2024 03:52:09 -0800 Subject: [PATCH] Refactor cache upload functionality and improve test cases --- packages/cache/__tests__/saveCacheV2.test.ts | 50 +++++++++---------- packages/cache/src/cache.ts | 26 +++++----- .../{blob/upload-cache.ts => uploadUtils.ts} | 11 ++-- 3 files changed, 44 insertions(+), 43 deletions(-) rename packages/cache/src/internal/{blob/upload-cache.ts => uploadUtils.ts} (82%) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 67c7f1de..2c69c5ee 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -1,14 +1,14 @@ import * as core from '@actions/core' import * as path from 'path' -import {saveCache} from '../src/cache' +import { saveCache } from '../src/cache' import * as cacheUtils from '../src/internal/cacheUtils' -import {CacheFilename, CompressionMethod} from '../src/internal/constants' +import { CacheFilename, CompressionMethod } from '../src/internal/constants' import * as config from '../src/internal/config' import * as tar from '../src/internal/tar' -import {CacheServiceClientJSON} from '../src/generated/results/api/v1/cache.twirp' -import * as uploadCacheModule from '../src/internal/blob/upload-cache' -import {BlobUploadCommonResponse} from '@azure/storage-blob' -import {InvalidResponseError} from '../src/internal/shared/errors' +import { CacheServiceClientJSON } from '../src/generated/results/api/v1/cache.twirp' +import * as uploadCacheModule from '../src/internal/uploadUtils' +import { BlobUploadCommonResponse } from '@azure/storage-blob' +import { InvalidResponseError } from '../src/internal/shared/errors' let logDebugMock: jest.SpyInstance @@ -28,11 +28,11 @@ jest.mock('@azure/storage-blob', () => ({ beforeAll(() => { process.env['ACTIONS_RUNTIME_TOKEN'] = 'token' - jest.spyOn(console, 'log').mockImplementation(() => {}) - jest.spyOn(core, 'debug').mockImplementation(() => {}) - jest.spyOn(core, 'info').mockImplementation(() => {}) - jest.spyOn(core, 'warning').mockImplementation(() => {}) - jest.spyOn(core, 'error').mockImplementation(() => {}) + jest.spyOn(console, 'log').mockImplementation(() => { }) + jest.spyOn(core, 'debug').mockImplementation(() => { }) + jest.spyOn(core, 'info').mockImplementation(() => { }) + jest.spyOn(core, 'warning').mockImplementation(() => { }) + jest.spyOn(core, 'error').mockImplementation(() => { }) jest.spyOn(cacheUtils, 'resolvePaths').mockImplementation(async filePaths => { return filePaths.map(x => path.resolve(x)) }) @@ -100,7 +100,7 @@ test('create cache entry failure', async () => { const createCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') - .mockReturnValue(Promise.resolve({ok: false, signedUploadUrl: ''})) + .mockReturnValue(Promise.resolve({ ok: false, signedUploadUrl: '' })) const createTarMock = jest.spyOn(tar, 'createTar') const finalizeCacheEntryMock = jest.spyOn( @@ -116,8 +116,8 @@ test('create cache entry failure', async () => { .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') .mockReturnValueOnce(archiveFileSize) const cacheVersion = cacheUtils.getCacheVersion(paths, compression) - const uploadCacheFileMock = jest - .spyOn(uploadCacheModule, 'uploadCacheFile') + const uploadCacheArchiveSDKMock = jest + .spyOn(uploadCacheModule, 'uploadCacheArchiveSDK') .mockReturnValueOnce( Promise.resolve({ _response: { @@ -139,7 +139,7 @@ test('create cache entry failure', async () => { expect(createTarMock).toHaveBeenCalledTimes(1) expect(getCompressionMock).toHaveBeenCalledTimes(1) expect(finalizeCacheEntryMock).toHaveBeenCalledTimes(0) - expect(uploadCacheFileMock).toHaveBeenCalledTimes(0) + expect(uploadCacheArchiveSDKMock).toHaveBeenCalledTimes(0) }) test('finalize save cache failure', async () => { @@ -152,12 +152,12 @@ test('finalize save cache failure', async () => { const createCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') .mockReturnValue( - Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) + Promise.resolve({ ok: true, signedUploadUrl: signedUploadURL }) ) const createTarMock = jest.spyOn(tar, 'createTar') - const uploadCacheMock = jest.spyOn(uploadCacheModule, 'uploadCacheFile') + const uploadCacheMock = jest.spyOn(uploadCacheModule, 'uploadCacheArchiveSDK') uploadCacheMock.mockReturnValueOnce( Promise.resolve({ _response: { @@ -179,7 +179,7 @@ test('finalize save cache failure', async () => { const finalizeCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') - .mockReturnValue(Promise.resolve({ok: false, entryId: ''})) + .mockReturnValue(Promise.resolve({ ok: false, entryId: '' })) const cacheId = await saveCache([paths], key) @@ -218,7 +218,7 @@ test('save with uploadCache Server error will fail', async () => { jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') .mockReturnValue( - Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) + Promise.resolve({ ok: true, signedUploadUrl: signedUploadURL }) ) const archiveFileSize = 1024 @@ -226,7 +226,7 @@ test('save with uploadCache Server error will fail', async () => { .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') .mockReturnValueOnce(archiveFileSize) jest - .spyOn(uploadCacheModule, 'uploadCacheFile') + .spyOn(uploadCacheModule, 'uploadCacheArchiveSDK') .mockRejectedValueOnce(new InvalidResponseError('boom')) const cacheId = await saveCache([paths], key) @@ -241,14 +241,14 @@ test('uploadFile returns 500', async () => { jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') .mockReturnValue( - Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) + Promise.resolve({ ok: true, signedUploadUrl: signedUploadURL }) ) const archiveFileSize = 1024 jest .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') .mockReturnValueOnce(archiveFileSize) - jest.spyOn(uploadCacheModule, 'uploadCacheFile').mockRestore() + jest.spyOn(uploadCacheModule, 'uploadCacheArchiveSDK').mockRestore() uploadFileMock = jest.fn().mockResolvedValueOnce({ _response: { @@ -279,11 +279,11 @@ test('save with valid inputs uploads a cache', async () => { jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') .mockReturnValue( - Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) + Promise.resolve({ ok: true, signedUploadUrl: signedUploadURL }) ) const uploadCacheMock = jest - .spyOn(uploadCacheModule, 'uploadCacheFile') + .spyOn(uploadCacheModule, 'uploadCacheArchiveSDK') .mockReturnValueOnce( Promise.resolve({ _response: { @@ -300,7 +300,7 @@ test('save with valid inputs uploads a cache', async () => { const finalizeCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') - .mockReturnValue(Promise.resolve({ok: true, entryId: cacheId.toString()})) + .mockReturnValue(Promise.resolve({ ok: true, entryId: cacheId.toString() })) const expectedCacheId = await saveCache([paths], key) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index fada75f2..adc8b915 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -3,18 +3,18 @@ import * as path from 'path' import * as utils from './internal/cacheUtils' import * as cacheHttpClient from './internal/cacheHttpClient' import * as cacheTwirpClient from './internal/shared/cacheTwirpClient' -import {downloadCacheStorageSDK} from './internal/downloadUtils' -import {getCacheServiceVersion, isGhes} from './internal/config' -import {DownloadOptions, UploadOptions} from './options' -import {createTar, extractTar, listTar} from './internal/tar' +import { downloadCacheStorageSDK } from './internal/downloadUtils' +import { getCacheServiceVersion, isGhes } from './internal/config' +import { DownloadOptions, UploadOptions } from './options' +import { createTar, extractTar, listTar } from './internal/tar' import { CreateCacheEntryRequest, FinalizeCacheEntryUploadRequest, FinalizeCacheEntryUploadResponse, GetCacheEntryDownloadURLRequest } from './generated/results/api/v1/cache' -import {CacheFileSizeLimit} from './internal/constants' -import {uploadCacheFile} from './internal/blob/upload-cache' +import { CacheFileSizeLimit } from './internal/constants' +import { uploadCacheArchiveSDK } from './internal/uploadUtils' export class ValidationError extends Error { constructor(message: string) { super(message) @@ -275,9 +275,9 @@ async function restoreCacheV2( response.signedDownloadUrl, archivePath, options || - ({ - timeoutInMs: 30000 - } as DownloadOptions) + ({ + timeoutInMs: 30000 + } as DownloadOptions) ) const archiveFileSize = utils.getArchiveFileSizeInBytes(archivePath) @@ -414,9 +414,9 @@ async function saveCacheV1( } else if (reserveCacheResponse?.statusCode === 400) { throw new Error( reserveCacheResponse?.error?.message ?? - `Cache size of ~${Math.round( - archiveFileSize / (1024 * 1024) - )} MB (${archiveFileSize} B) is over the data cap limit, not saving cache.` + `Cache size of ~${Math.round( + archiveFileSize / (1024 * 1024) + )} MB (${archiveFileSize} B) is over the data cap limit, not saving cache.` ) } else { throw new ReserveCacheError( @@ -521,7 +521,7 @@ async function saveCacheV2( } core.debug(`Attempting to upload cache located at: ${archivePath}`) - const uploadResponse = await uploadCacheFile( + const uploadResponse = await uploadCacheArchiveSDK( response.signedUploadUrl, archivePath ) diff --git a/packages/cache/src/internal/blob/upload-cache.ts b/packages/cache/src/internal/uploadUtils.ts similarity index 82% rename from packages/cache/src/internal/blob/upload-cache.ts rename to packages/cache/src/internal/uploadUtils.ts index b9970c46..ffcb37f8 100644 --- a/packages/cache/src/internal/blob/upload-cache.ts +++ b/packages/cache/src/internal/uploadUtils.ts @@ -5,12 +5,13 @@ import { BlockBlobClient, BlockBlobParallelUploadOptions } from '@azure/storage-blob' -import {InvalidResponseError} from '../shared/errors' +import { InvalidResponseError } from './shared/errors' -export async function uploadCacheFile( - signedUploadURL: string, - archivePath: string -): Promise { +export async function uploadCacheArchiveSDK + ( + signedUploadURL: string, + archivePath: string + ): Promise { // Specify data transfer options const uploadOptions: BlockBlobParallelUploadOptions = { blockSize: 4 * 1024 * 1024, // 4 MiB max block size