From 8f606682c2651cedb342d9a4a406b37a8dfe0eb7 Mon Sep 17 00:00:00 2001 From: John Sudol <24583161+johnsudol@users.noreply.github.com> Date: Sun, 24 Nov 2024 18:44:39 +0000 Subject: [PATCH 01/32] Add saveCacheV2 tests --- packages/cache/__tests__/saveCacheV2.test.ts | 311 ++++++++++++++++++ packages/cache/src/cache.ts | 4 +- .../cache/src/internal/blob/upload-cache.ts | 3 +- 3 files changed, 315 insertions(+), 3 deletions(-) create mode 100644 packages/cache/__tests__/saveCacheV2.test.ts diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts new file mode 100644 index 00000000..fdbf596f --- /dev/null +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -0,0 +1,311 @@ +import * as core from '@actions/core' +import * as path from 'path' +import { saveCache } from '../src/cache' +import * as cacheUtils from '../src/internal/cacheUtils' +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' + +let logDebugMock: jest.SpyInstance + +jest.mock('../src/internal/cacheUtils') +jest.mock('../src/internal/tar') + +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(cacheUtils, 'getCacheFileName').mockImplementation(cm => { + const actualUtils = jest.requireActual('../src/internal/cacheUtils') + return actualUtils.getCacheFileName(cm) + }) + jest.spyOn(cacheUtils, 'getCacheVersion').mockImplementation((paths, cm) => { + const actualUtils = jest.requireActual('../src/internal/cacheUtils') + return actualUtils.getCacheVersion(paths, cm) + }) + jest.spyOn(cacheUtils, 'resolvePaths').mockImplementation(async filePaths => { + return filePaths.map(x => path.resolve(x)) + }) + jest.spyOn(cacheUtils, 'createTempDirectory').mockImplementation(async () => { + return Promise.resolve('/foo/bar') + }) + + // Ensure that we're using v2 for these tests + jest.spyOn(config, 'getCacheServiceVersion').mockReturnValue('v2') + + logDebugMock = jest.spyOn(core, 'debug') +}) + +afterEach(() => { + expect(logDebugMock).toHaveBeenCalledWith('Cache service version: v2') + jest.clearAllMocks() +}) + +test('save with missing input should fail', async () => { + const paths: string[] = [] + const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + + await expect(saveCache(paths, primaryKey)).rejects.toThrowError( + `Path Validation Error: At least one directory or file path is required` + ) +}) + +test('save with large cache outputs should fail using v2 saveCache', async () => { + const filePath = 'node_modules' + const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const cachePaths = [path.resolve(filePath)] + + const createTarMock = jest.spyOn(tar, 'createTar') + const logWarningMock = jest.spyOn(core, 'warning') + + const cacheSize = 11 * 1024 * 1024 * 1024 //~11GB, over the 10GB limit + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(cacheSize) + const compression = CompressionMethod.Gzip + const getCompressionMock = jest + .spyOn(cacheUtils, 'getCompressionMethod') + .mockReturnValueOnce(Promise.resolve(compression)) + + const cacheId = await saveCache([filePath], primaryKey) + expect(cacheId).toBe(-1) + expect(logWarningMock).toHaveBeenCalledTimes(1) + expect(logWarningMock).toHaveBeenCalledWith( + 'Failed to save: Cache size of ~11264 MB (11811160064 B) is over the 10GB limit, not saving cache.' + ) + + const archiveFolder = '/foo/bar' + + expect(createTarMock).toHaveBeenCalledTimes(1) + expect(createTarMock).toHaveBeenCalledWith( + archiveFolder, + cachePaths, + compression + ) + expect(getCompressionMock).toHaveBeenCalledTimes(1) +}) + +test('create cache entry failure', async () => { + const paths = ['node_modules'] + const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const infoLogMock = jest.spyOn(core, 'info') + + const createCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockReturnValue(Promise.resolve({ ok: false, signedUploadUrl: '' })) + + const createTarMock = jest.spyOn(tar, 'createTar') + const finalizeCacheEntryMock = jest.spyOn( + CacheServiceClientJSON.prototype, + 'FinalizeCacheEntryUpload' + ) + const compression = CompressionMethod.Zstd + const getCompressionMock = jest + .spyOn(cacheUtils, 'getCompressionMethod') + .mockReturnValueOnce(Promise.resolve(compression)) + const cacheVersion = cacheUtils.getCacheVersion(paths, compression) + const uploadCacheFileMock = jest + .spyOn(uploadCacheModule, 'uploadCacheFile') + .mockReturnValue( + Promise.resolve({ + _response: { + status: 200 + } + } as BlobUploadCommonResponse) + ) + + const cacheId = await saveCache(paths, primaryKey) + expect(cacheId).toBe(-1) + expect(infoLogMock).toHaveBeenCalledTimes(1) + expect(infoLogMock).toHaveBeenCalledWith( + `Failed to save: Unable to reserve cache with key ${primaryKey}, another job may be creating this cache.` + ) + + expect(createCacheEntryMock).toHaveBeenCalledTimes(1) + expect(createCacheEntryMock).toHaveBeenCalledWith({ + key: primaryKey, + version: cacheVersion + }) + expect(createTarMock).toHaveBeenCalledTimes(1) + expect(getCompressionMock).toHaveBeenCalledTimes(1) + expect(finalizeCacheEntryMock).toHaveBeenCalledTimes(0) + expect(uploadCacheFileMock).toHaveBeenCalledTimes(0) +}) + +test('finalize save cache failure', async () => { + const filePath = 'node_modules' + const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const cachePaths = [path.resolve(filePath)] + const logWarningMock = jest.spyOn(core, 'warning') + const signedUploadURL = 'https://blob-storage.local?signed=true' + + const createCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockReturnValue( + Promise.resolve({ ok: true, signedUploadUrl: signedUploadURL }) + ) + + const createTarMock = jest.spyOn(tar, 'createTar') + + const uploadCacheMock = jest.spyOn(uploadCacheModule, 'uploadCacheFile') + uploadCacheMock.mockReturnValue( + Promise.resolve({ + _response: { + status: 200 + } + } as BlobUploadCommonResponse) + ) + + const compression = CompressionMethod.Zstd + const getCompressionMock = jest + .spyOn(cacheUtils, 'getCompressionMethod') + .mockReturnValueOnce(Promise.resolve(compression)) + + const cacheVersion = cacheUtils.getCacheVersion([filePath], compression) + const archiveFileSize = 1024 + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(archiveFileSize) + + const finalizeCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') + .mockReturnValue(Promise.resolve({ ok: false, entryId: '' })) + + const cacheId = await saveCache([filePath], primaryKey) + + expect(createCacheEntryMock).toHaveBeenCalledTimes(1) + expect(createCacheEntryMock).toHaveBeenCalledWith({ + key: primaryKey, + version: cacheVersion + }) + + const archiveFolder = '/foo/bar' + const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) + expect(createTarMock).toHaveBeenCalledTimes(1) + expect(createTarMock).toHaveBeenCalledWith( + archiveFolder, + cachePaths, + compression + ) + + expect(uploadCacheMock).toHaveBeenCalledTimes(1) + expect(uploadCacheMock).toHaveBeenCalledWith(signedUploadURL, archiveFile) + expect(getCompressionMock).toHaveBeenCalledTimes(1) + + expect(finalizeCacheEntryMock).toHaveBeenCalledTimes(1) + expect(finalizeCacheEntryMock).toHaveBeenCalledWith({ + key: primaryKey, + version: cacheVersion, + sizeBytes: archiveFileSize.toString() + }) + + expect(cacheId).toBe(-1) + expect(logWarningMock).toHaveBeenCalledTimes(1) + expect(logWarningMock).toHaveBeenCalledWith( + `Failed to save: Unable to finalize cache with key ${primaryKey}, another job may be finalizing this cache.` + ) +}) + +test('save with uploadCache Server error will fail', async () => { + const filePath = 'node_modules' + const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const logWarningMock = jest.spyOn(core, 'warning') + const signedUploadURL = 'https://signed-upload-url.com' + jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockReturnValue( + Promise.resolve({ ok: true, signedUploadUrl: signedUploadURL }) + ) + + jest + .spyOn(uploadCacheModule, 'uploadCacheFile') + .mockReturnValueOnce(Promise.reject(new Error('HTTP Error Occurred'))) + + const cacheId = await saveCache([filePath], primaryKey) + + expect(logWarningMock).toHaveBeenCalledTimes(1) + expect(logWarningMock).toHaveBeenCalledWith( + `Failed to save: HTTP Error Occurred` + ) + expect(cacheId).toBe(-1) +}) + +test('save with valid inputs uploads a cache', async () => { + const filePath = 'node_modules' + const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const cachePaths = [path.resolve(filePath)] + const signedUploadURL = 'https://blob-storage.local?signed=true' + const createTarMock = jest.spyOn(tar, 'createTar') + + const archiveFileSize = 1024 + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(archiveFileSize) + + const cacheId = 4 + jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockReturnValue( + Promise.resolve({ ok: true, signedUploadUrl: signedUploadURL }) + ) + + const uploadCacheMock = jest + .spyOn(uploadCacheModule, 'uploadCacheFile') + .mockReturnValue( + Promise.resolve({ + _response: { + status: 200 + } + } as BlobUploadCommonResponse) + ) + + const compression = CompressionMethod.Zstd + const getCompressionMock = jest + .spyOn(cacheUtils, 'getCompressionMethod') + .mockReturnValue(Promise.resolve(compression)) + const cacheVersion = cacheUtils.getCacheVersion([filePath], compression) + + const finalizeCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') + .mockReturnValue(Promise.resolve({ ok: true, entryId: cacheId.toString() })) + + const expectedCacheId = await saveCache([filePath], primaryKey) + + const archiveFolder = '/foo/bar' + const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) + expect(uploadCacheMock).toHaveBeenCalledTimes(1) + expect(uploadCacheMock).toHaveBeenCalledWith(signedUploadURL, archiveFile) + expect(createTarMock).toHaveBeenCalledTimes(1) + expect(createTarMock).toHaveBeenCalledWith( + archiveFolder, + cachePaths, + compression + ) + + expect(finalizeCacheEntryMock).toHaveBeenCalledTimes(1) + expect(finalizeCacheEntryMock).toHaveBeenCalledWith({ + key: primaryKey, + version: cacheVersion, + sizeBytes: archiveFileSize.toString() + }) + + expect(getCompressionMock).toHaveBeenCalledTimes(1) + expect(expectedCacheId).toBe(cacheId) +}) + +test('save with non existing path should not save cache using v2 saveCache', async () => { + const path = 'node_modules' + const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + jest.spyOn(cacheUtils, 'resolvePaths').mockImplementation(async () => { + return [] + }) + await expect(saveCache([path], primaryKey)).rejects.toThrowError( + `Path Validation Error: Path(s) specified in the action for caching do(es) not exist, hence no cache is being saved.` + ) +}) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 8b7a8d02..53813f85 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -328,10 +328,10 @@ export async function saveCache( options?: UploadOptions, enableCrossOsArchive = false ): Promise { + const cacheServiceVersion: string = getCacheServiceVersion() + core.debug(`Cache service version: ${cacheServiceVersion}`) checkPaths(paths) checkKey(key) - - const cacheServiceVersion: string = getCacheServiceVersion() switch (cacheServiceVersion) { case 'v2': return await saveCacheV2(paths, key, options, enableCrossOsArchive) diff --git a/packages/cache/src/internal/blob/upload-cache.ts b/packages/cache/src/internal/blob/upload-cache.ts index 15c913ed..a171c9da 100644 --- a/packages/cache/src/internal/blob/upload-cache.ts +++ b/packages/cache/src/internal/blob/upload-cache.ts @@ -1,6 +1,7 @@ import * as core from '@actions/core' import { BlobClient, + BlobUploadCommonResponse, BlockBlobClient, BlockBlobParallelUploadOptions } from '@azure/storage-blob' @@ -8,7 +9,7 @@ import { export async function uploadCacheFile( signedUploadURL: string, archivePath: string -): Promise<{}> { +): Promise { // Specify data transfer options const uploadOptions: BlockBlobParallelUploadOptions = { blockSize: 4 * 1024 * 1024, // 4 MiB max block size From 1f087496cab0a5ec5e38471f1f1b6c00f280f70c Mon Sep 17 00:00:00 2001 From: John Sudol <24583161+johnsudol@users.noreply.github.com> Date: Tue, 26 Nov 2024 00:39:01 +0000 Subject: [PATCH 02/32] Add debug message for uploadResponse --- packages/cache/__tests__/saveCacheV2.test.ts | 2 +- packages/cache/src/cache.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index fdbf596f..509f97ab 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -56,7 +56,7 @@ test('save with missing input should fail', async () => { ) }) -test('save with large cache outputs should fail using v2 saveCache', async () => { +test('save with large cache outputs should fail using', async () => { const filePath = 'node_modules' const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' const cachePaths = [path.resolve(filePath)] diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 53813f85..a8e741cd 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -518,7 +518,8 @@ async function saveCacheV2( } core.debug(`Attempting to upload cache located at: ${archivePath}`) - await uploadCacheFile(response.signedUploadUrl, archivePath) + const uploadResponse = await uploadCacheFile(response.signedUploadUrl, archivePath) + core.debug(`Download response status: ${uploadResponse._response.status}`) const finalizeRequest: FinalizeCacheEntryUploadRequest = { key, From 46174ed57357a1af42417af67762fe66445da64c Mon Sep 17 00:00:00 2001 From: John Sudol <24583161+johnsudol@users.noreply.github.com> Date: Tue, 26 Nov 2024 00:56:07 +0000 Subject: [PATCH 03/32] run prettier --- packages/cache/__tests__/saveCacheV2.test.ts | 30 ++++++++++---------- packages/cache/src/cache.ts | 5 +++- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 509f97ab..7263ea89 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -1,13 +1,13 @@ 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 {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 {BlobUploadCommonResponse} from '@azure/storage-blob' let logDebugMock: jest.SpyInstance @@ -16,11 +16,11 @@ jest.mock('../src/internal/tar') 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, 'getCacheFileName').mockImplementation(cm => { const actualUtils = jest.requireActual('../src/internal/cacheUtils') return actualUtils.getCacheFileName(cm) @@ -98,7 +98,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( @@ -148,7 +148,7 @@ 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') @@ -175,7 +175,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([filePath], primaryKey) @@ -220,7 +220,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}) ) jest @@ -252,7 +252,7 @@ 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 @@ -273,7 +273,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([filePath], primaryKey) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index a8e741cd..0a73059a 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -518,7 +518,10 @@ async function saveCacheV2( } core.debug(`Attempting to upload cache located at: ${archivePath}`) - const uploadResponse = await uploadCacheFile(response.signedUploadUrl, archivePath) + const uploadResponse = await uploadCacheFile( + response.signedUploadUrl, + archivePath + ) core.debug(`Download response status: ${uploadResponse._response.status}`) const finalizeRequest: FinalizeCacheEntryUploadRequest = { From 208dbe21316f19ce222330e23e3329fb716cd5f5 Mon Sep 17 00:00:00 2001 From: John Sudol <24583161+johnsudol@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:36:12 +0000 Subject: [PATCH 04/32] PR feedback --- packages/cache/__tests__/saveCacheV2.test.ts | 73 ++++++++------------ 1 file changed, 30 insertions(+), 43 deletions(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 7263ea89..28e82ae0 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -49,17 +49,17 @@ afterEach(() => { test('save with missing input should fail', async () => { const paths: string[] = [] - const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' - await expect(saveCache(paths, primaryKey)).rejects.toThrowError( + await expect(saveCache(paths, key)).rejects.toThrowError( `Path Validation Error: At least one directory or file path is required` ) }) test('save with large cache outputs should fail using', async () => { - const filePath = 'node_modules' - const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' - const cachePaths = [path.resolve(filePath)] + const paths = 'node_modules' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const cachePaths = [path.resolve(paths)] const createTarMock = jest.spyOn(tar, 'createTar') const logWarningMock = jest.spyOn(core, 'warning') @@ -73,16 +73,14 @@ test('save with large cache outputs should fail using', async () => { .spyOn(cacheUtils, 'getCompressionMethod') .mockReturnValueOnce(Promise.resolve(compression)) - const cacheId = await saveCache([filePath], primaryKey) + const cacheId = await saveCache([paths], key) expect(cacheId).toBe(-1) - expect(logWarningMock).toHaveBeenCalledTimes(1) expect(logWarningMock).toHaveBeenCalledWith( 'Failed to save: Cache size of ~11264 MB (11811160064 B) is over the 10GB limit, not saving cache.' ) const archiveFolder = '/foo/bar' - expect(createTarMock).toHaveBeenCalledTimes(1) expect(createTarMock).toHaveBeenCalledWith( archiveFolder, cachePaths, @@ -93,7 +91,7 @@ test('save with large cache outputs should fail using', async () => { test('create cache entry failure', async () => { const paths = ['node_modules'] - const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' const infoLogMock = jest.spyOn(core, 'info') const createCacheEntryMock = jest @@ -120,16 +118,14 @@ test('create cache entry failure', async () => { } as BlobUploadCommonResponse) ) - const cacheId = await saveCache(paths, primaryKey) + const cacheId = await saveCache(paths, key) expect(cacheId).toBe(-1) - expect(infoLogMock).toHaveBeenCalledTimes(1) expect(infoLogMock).toHaveBeenCalledWith( - `Failed to save: Unable to reserve cache with key ${primaryKey}, another job may be creating this cache.` + `Failed to save: Unable to reserve cache with key ${key}, another job may be creating this cache.` ) - expect(createCacheEntryMock).toHaveBeenCalledTimes(1) expect(createCacheEntryMock).toHaveBeenCalledWith({ - key: primaryKey, + key, version: cacheVersion }) expect(createTarMock).toHaveBeenCalledTimes(1) @@ -139,9 +135,9 @@ test('create cache entry failure', async () => { }) test('finalize save cache failure', async () => { - const filePath = 'node_modules' - const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' - const cachePaths = [path.resolve(filePath)] + const paths = 'node_modules' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const cachePaths = [path.resolve(paths)] const logWarningMock = jest.spyOn(core, 'warning') const signedUploadURL = 'https://blob-storage.local?signed=true' @@ -167,7 +163,7 @@ test('finalize save cache failure', async () => { .spyOn(cacheUtils, 'getCompressionMethod') .mockReturnValueOnce(Promise.resolve(compression)) - const cacheVersion = cacheUtils.getCacheVersion([filePath], compression) + const cacheVersion = cacheUtils.getCacheVersion([paths], compression) const archiveFileSize = 1024 jest .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') @@ -177,46 +173,41 @@ test('finalize save cache failure', async () => { .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') .mockReturnValue(Promise.resolve({ok: false, entryId: ''})) - const cacheId = await saveCache([filePath], primaryKey) + const cacheId = await saveCache([paths], key) - expect(createCacheEntryMock).toHaveBeenCalledTimes(1) expect(createCacheEntryMock).toHaveBeenCalledWith({ - key: primaryKey, + key, version: cacheVersion }) const archiveFolder = '/foo/bar' const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) - expect(createTarMock).toHaveBeenCalledTimes(1) expect(createTarMock).toHaveBeenCalledWith( archiveFolder, cachePaths, compression ) - expect(uploadCacheMock).toHaveBeenCalledTimes(1) expect(uploadCacheMock).toHaveBeenCalledWith(signedUploadURL, archiveFile) expect(getCompressionMock).toHaveBeenCalledTimes(1) - expect(finalizeCacheEntryMock).toHaveBeenCalledTimes(1) expect(finalizeCacheEntryMock).toHaveBeenCalledWith({ - key: primaryKey, + key, version: cacheVersion, sizeBytes: archiveFileSize.toString() }) expect(cacheId).toBe(-1) - expect(logWarningMock).toHaveBeenCalledTimes(1) expect(logWarningMock).toHaveBeenCalledWith( - `Failed to save: Unable to finalize cache with key ${primaryKey}, another job may be finalizing this cache.` + `Failed to save: Unable to finalize cache with key ${key}, another job may be finalizing this cache.` ) }) test('save with uploadCache Server error will fail', async () => { - const filePath = 'node_modules' - const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const paths = 'node_modules' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' const logWarningMock = jest.spyOn(core, 'warning') - const signedUploadURL = 'https://signed-upload-url.com' + const signedUploadURL = 'https://blob-storage.local?signed=true' jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') .mockReturnValue( @@ -227,9 +218,8 @@ test('save with uploadCache Server error will fail', async () => { .spyOn(uploadCacheModule, 'uploadCacheFile') .mockReturnValueOnce(Promise.reject(new Error('HTTP Error Occurred'))) - const cacheId = await saveCache([filePath], primaryKey) + const cacheId = await saveCache([paths], key) - expect(logWarningMock).toHaveBeenCalledTimes(1) expect(logWarningMock).toHaveBeenCalledWith( `Failed to save: HTTP Error Occurred` ) @@ -237,9 +227,9 @@ test('save with uploadCache Server error will fail', async () => { }) test('save with valid inputs uploads a cache', async () => { - const filePath = 'node_modules' - const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' - const cachePaths = [path.resolve(filePath)] + const paths = 'node_modules' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const cachePaths = [path.resolve(paths)] const signedUploadURL = 'https://blob-storage.local?signed=true' const createTarMock = jest.spyOn(tar, 'createTar') @@ -269,28 +259,25 @@ test('save with valid inputs uploads a cache', async () => { const getCompressionMock = jest .spyOn(cacheUtils, 'getCompressionMethod') .mockReturnValue(Promise.resolve(compression)) - const cacheVersion = cacheUtils.getCacheVersion([filePath], compression) + const cacheVersion = cacheUtils.getCacheVersion([paths], compression) const finalizeCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') .mockReturnValue(Promise.resolve({ok: true, entryId: cacheId.toString()})) - const expectedCacheId = await saveCache([filePath], primaryKey) + const expectedCacheId = await saveCache([paths], key) const archiveFolder = '/foo/bar' const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) - expect(uploadCacheMock).toHaveBeenCalledTimes(1) expect(uploadCacheMock).toHaveBeenCalledWith(signedUploadURL, archiveFile) - expect(createTarMock).toHaveBeenCalledTimes(1) expect(createTarMock).toHaveBeenCalledWith( archiveFolder, cachePaths, compression ) - expect(finalizeCacheEntryMock).toHaveBeenCalledTimes(1) expect(finalizeCacheEntryMock).toHaveBeenCalledWith({ - key: primaryKey, + key, version: cacheVersion, sizeBytes: archiveFileSize.toString() }) @@ -301,11 +288,11 @@ test('save with valid inputs uploads a cache', async () => { test('save with non existing path should not save cache using v2 saveCache', async () => { const path = 'node_modules' - const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' jest.spyOn(cacheUtils, 'resolvePaths').mockImplementation(async () => { return [] }) - await expect(saveCache([path], primaryKey)).rejects.toThrowError( + await expect(saveCache([path], key)).rejects.toThrowError( `Path Validation Error: Path(s) specified in the action for caching do(es) not exist, hence no cache is being saved.` ) }) From 94f18eb26eb16f9d1d79470a9146d4a3cd0cff08 Mon Sep 17 00:00:00 2001 From: John Sudol <24583161+johnsudol@users.noreply.github.com> Date: Tue, 26 Nov 2024 23:05:11 +0000 Subject: [PATCH 05/32] Only mock the cacheUtil methods we need --- packages/cache/__tests__/saveCacheV2.test.ts | 24 ++++++++----------- .../cache/src/internal/blob/upload-cache.ts | 11 ++++++++- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 28e82ae0..23869a1f 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -8,10 +8,10 @@ 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' let logDebugMock: jest.SpyInstance -jest.mock('../src/internal/cacheUtils') jest.mock('../src/internal/tar') beforeAll(() => { @@ -21,14 +21,6 @@ beforeAll(() => { jest.spyOn(core, 'info').mockImplementation(() => {}) jest.spyOn(core, 'warning').mockImplementation(() => {}) jest.spyOn(core, 'error').mockImplementation(() => {}) - jest.spyOn(cacheUtils, 'getCacheFileName').mockImplementation(cm => { - const actualUtils = jest.requireActual('../src/internal/cacheUtils') - return actualUtils.getCacheFileName(cm) - }) - jest.spyOn(cacheUtils, 'getCacheVersion').mockImplementation((paths, cm) => { - const actualUtils = jest.requireActual('../src/internal/cacheUtils') - return actualUtils.getCacheVersion(paths, cm) - }) jest.spyOn(cacheUtils, 'resolvePaths').mockImplementation(async filePaths => { return filePaths.map(x => path.resolve(x)) }) @@ -107,6 +99,10 @@ test('create cache entry failure', async () => { const getCompressionMock = jest .spyOn(cacheUtils, 'getCompressionMethod') .mockReturnValueOnce(Promise.resolve(compression)) + const archiveFileSize = 1024 + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(archiveFileSize) const cacheVersion = cacheUtils.getCacheVersion(paths, compression) const uploadCacheFileMock = jest .spyOn(uploadCacheModule, 'uploadCacheFile') @@ -214,15 +210,15 @@ test('save with uploadCache Server error will fail', async () => { Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) ) + const archiveFileSize = 1024 + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(archiveFileSize) jest .spyOn(uploadCacheModule, 'uploadCacheFile') - .mockReturnValueOnce(Promise.reject(new Error('HTTP Error Occurred'))) + .mockRejectedValueOnce(new InvalidResponseError('boom')) const cacheId = await saveCache([paths], key) - - expect(logWarningMock).toHaveBeenCalledWith( - `Failed to save: HTTP Error Occurred` - ) expect(cacheId).toBe(-1) }) diff --git a/packages/cache/src/internal/blob/upload-cache.ts b/packages/cache/src/internal/blob/upload-cache.ts index a171c9da..934ecb6f 100644 --- a/packages/cache/src/internal/blob/upload-cache.ts +++ b/packages/cache/src/internal/blob/upload-cache.ts @@ -5,6 +5,7 @@ import { BlockBlobClient, BlockBlobParallelUploadOptions } from '@azure/storage-blob' +import {InvalidResponseError} from '../shared/errors' export async function uploadCacheFile( signedUploadURL: string, @@ -24,5 +25,13 @@ export async function uploadCacheFile( `BlobClient: ${blobClient.name}:${blobClient.accountName}:${blobClient.containerName}` ) - return blockBlobClient.uploadFile(archivePath, uploadOptions) + const resp = await blockBlobClient.uploadFile(archivePath, uploadOptions) + + if (resp._response.status >= 400) { + throw new InvalidResponseError( + `Upload failed with status code: ${resp._response.status}` + ) + } + + return resp } From 5d0a4af70a2d75b0de58cc726474410a7dc92112 Mon Sep 17 00:00:00 2001 From: John Sudol <24583161+johnsudol@users.noreply.github.com> Date: Tue, 26 Nov 2024 23:33:19 +0000 Subject: [PATCH 06/32] Remove unused mock --- packages/cache/__tests__/saveCacheV2.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 23869a1f..5ae79d99 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -202,7 +202,6 @@ test('finalize save cache failure', async () => { test('save with uploadCache Server error will fail', async () => { const paths = 'node_modules' const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' - const logWarningMock = jest.spyOn(core, 'warning') const signedUploadURL = 'https://blob-storage.local?signed=true' jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') From b050504b2d9a98762b983a04418dc1c7b3c57ecc Mon Sep 17 00:00:00 2001 From: John Sudol <24583161+johnsudol@users.noreply.github.com> Date: Wed, 27 Nov 2024 01:45:46 +0000 Subject: [PATCH 07/32] Add test case for when the uploadFile fails on the blobclient --- packages/cache/__tests__/saveCacheV2.test.ts | 48 +++++++++++++++++-- .../cache/src/internal/blob/upload-cache.ts | 2 +- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 5ae79d99..67c7f1de 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -14,6 +14,18 @@ let logDebugMock: jest.SpyInstance jest.mock('../src/internal/tar') +let uploadFileMock = jest.fn() +const blockBlobClientMock = jest.fn().mockImplementation(() => ({ + uploadFile: uploadFileMock +})) +jest.mock('@azure/storage-blob', () => ({ + BlobClient: jest.fn().mockImplementation(() => { + return { + getBlockBlobClient: blockBlobClientMock + } + }) +})) + beforeAll(() => { process.env['ACTIONS_RUNTIME_TOKEN'] = 'token' jest.spyOn(console, 'log').mockImplementation(() => {}) @@ -106,7 +118,7 @@ test('create cache entry failure', async () => { const cacheVersion = cacheUtils.getCacheVersion(paths, compression) const uploadCacheFileMock = jest .spyOn(uploadCacheModule, 'uploadCacheFile') - .mockReturnValue( + .mockReturnValueOnce( Promise.resolve({ _response: { status: 200 @@ -146,7 +158,7 @@ test('finalize save cache failure', async () => { const createTarMock = jest.spyOn(tar, 'createTar') const uploadCacheMock = jest.spyOn(uploadCacheModule, 'uploadCacheFile') - uploadCacheMock.mockReturnValue( + uploadCacheMock.mockReturnValueOnce( Promise.resolve({ _response: { status: 200 @@ -221,6 +233,36 @@ test('save with uploadCache Server error will fail', async () => { expect(cacheId).toBe(-1) }) +test('uploadFile returns 500', async () => { + const paths = 'node_modules' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const signedUploadURL = 'https://blob-storage.local?signed=true' + const logWarningMock = jest.spyOn(core, 'warning') + jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockReturnValue( + Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) + ) + + const archiveFileSize = 1024 + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(archiveFileSize) + jest.spyOn(uploadCacheModule, 'uploadCacheFile').mockRestore() + + uploadFileMock = jest.fn().mockResolvedValueOnce({ + _response: { + status: 500 + } + }) + const cacheId = await saveCache([paths], key) + + expect(logWarningMock).toHaveBeenCalledWith( + 'Failed to save: Upload failed with status code 500' + ) + expect(cacheId).toBe(-1) +}) + test('save with valid inputs uploads a cache', async () => { const paths = 'node_modules' const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' @@ -242,7 +284,7 @@ test('save with valid inputs uploads a cache', async () => { const uploadCacheMock = jest .spyOn(uploadCacheModule, 'uploadCacheFile') - .mockReturnValue( + .mockReturnValueOnce( Promise.resolve({ _response: { status: 200 diff --git a/packages/cache/src/internal/blob/upload-cache.ts b/packages/cache/src/internal/blob/upload-cache.ts index 934ecb6f..b9970c46 100644 --- a/packages/cache/src/internal/blob/upload-cache.ts +++ b/packages/cache/src/internal/blob/upload-cache.ts @@ -29,7 +29,7 @@ export async function uploadCacheFile( if (resp._response.status >= 400) { throw new InvalidResponseError( - `Upload failed with status code: ${resp._response.status}` + `Upload failed with status code ${resp._response.status}` ) } From 27e5cf25146e49cc5006adc7b2d289faeea7392f Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Wed, 27 Nov 2024 04:51:21 -0800 Subject: [PATCH 08/32] Replace downloadCacheFile with downloadCacheStorageSDK --- .../cache/__tests__/restoreCacheV2.test.ts | 75 ++++++++----------- packages/cache/src/cache.ts | 18 +++-- .../cache/src/internal/blob/download-cache.ts | 16 +++- 3 files changed, 59 insertions(+), 50 deletions(-) diff --git a/packages/cache/__tests__/restoreCacheV2.test.ts b/packages/cache/__tests__/restoreCacheV2.test.ts index cc4f9e3c..365afdf0 100644 --- a/packages/cache/__tests__/restoreCacheV2.test.ts +++ b/packages/cache/__tests__/restoreCacheV2.test.ts @@ -3,11 +3,11 @@ import * as path from 'path' import * as tar from '../src/internal/tar' import * as config from '../src/internal/config' import * as cacheUtils from '../src/internal/cacheUtils' -import * as downloadCacheModule from '../src/internal/blob/download-cache' +import * as downloadUtils from '../src/internal/downloadUtils' import {restoreCache} from '../src/cache' import {CacheFilename, CompressionMethod} from '../src/internal/constants' import {CacheServiceClientJSON} from '../src/generated/results/api/v1/cache.twirp' -import {BlobDownloadResponseParsed} from '@azure/storage-blob' +import {DownloadOptions} from '../src/options' jest.mock('../src/internal/cacheHttpClient') jest.mock('../src/internal/cacheUtils') @@ -142,6 +142,7 @@ test('restore with gzip compressed cache found', async () => { const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = 'd90f107aaeb22920dba0c637a23c37b5bc497b4dfa3b07fe3f79bf88a273c11b' + const options: DownloadOptions = {timeoutInMs: 30000} const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -169,17 +170,11 @@ test('restore with gzip compressed cache found', async () => { }) const archivePath = path.join(tempPath, CacheFilename.Gzip) - const downloadCacheFileMock = jest.spyOn( - downloadCacheModule, - 'downloadCacheFile' - ) - downloadCacheFileMock.mockReturnValue( - Promise.resolve({ - _response: { - status: 200 - } - } as BlobDownloadResponseParsed) + const downloadCacheStorageSDKMock = jest.spyOn( + downloadUtils, + 'downloadCacheStorageSDK' ) + downloadCacheStorageSDKMock.mockReturnValue(Promise.resolve()) const fileSize = 142 const getArchiveFileSizeInBytesMock = jest @@ -203,9 +198,10 @@ test('restore with gzip compressed cache found', async () => { version: cacheVersion }) expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) - expect(downloadCacheFileMock).toHaveBeenCalledWith( + expect(downloadCacheStorageSDKMock).toHaveBeenCalledWith( signedDownloadUrl, - archivePath + archivePath, + options ) expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`) @@ -226,6 +222,7 @@ test('restore with zstd compressed cache found', async () => { const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = '8e2e96a184cb0cd6b48285b176c06a418f3d7fce14c29d9886fd1bb4f05c513d' + const options: DownloadOptions = {timeoutInMs: 30000} const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -253,17 +250,11 @@ test('restore with zstd compressed cache found', async () => { }) const archivePath = path.join(tempPath, CacheFilename.Zstd) - const downloadCacheFileMock = jest.spyOn( - downloadCacheModule, - 'downloadCacheFile' - ) - downloadCacheFileMock.mockReturnValue( - Promise.resolve({ - _response: { - status: 200 - } - } as BlobDownloadResponseParsed) + const downloadCacheStorageSDKMock = jest.spyOn( + downloadUtils, + 'downloadCacheStorageSDK' ) + downloadCacheStorageSDKMock.mockReturnValue(Promise.resolve()) const fileSize = 62915000 const getArchiveFileSizeInBytesMock = jest @@ -287,9 +278,10 @@ test('restore with zstd compressed cache found', async () => { version: cacheVersion }) expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) - expect(downloadCacheFileMock).toHaveBeenCalledWith( + expect(downloadCacheStorageSDKMock).toHaveBeenCalledWith( signedDownloadUrl, - archivePath + archivePath, + options ) expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~60 MB (62915000 B)`) @@ -311,6 +303,7 @@ test('restore with cache found for restore key', async () => { const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = 'b8b58e9bd7b1e8f83d9f05c7e06ea865ba44a0330e07a14db74ac74386677bed' + const options: DownloadOptions = {timeoutInMs: 30000} const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -338,17 +331,11 @@ test('restore with cache found for restore key', async () => { }) const archivePath = path.join(tempPath, CacheFilename.Gzip) - const downloadCacheFileMock = jest.spyOn( - downloadCacheModule, - 'downloadCacheFile' - ) - downloadCacheFileMock.mockReturnValue( - Promise.resolve({ - _response: { - status: 200 - } - } as BlobDownloadResponseParsed) + const downloadCacheStorageSDKMock = jest.spyOn( + downloadUtils, + 'downloadCacheStorageSDK' ) + downloadCacheStorageSDKMock.mockReturnValue(Promise.resolve()) const fileSize = 142 const getArchiveFileSizeInBytesMock = jest @@ -372,9 +359,10 @@ test('restore with cache found for restore key', async () => { version: cacheVersion }) expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) - expect(downloadCacheFileMock).toHaveBeenCalledWith( + expect(downloadCacheStorageSDKMock).toHaveBeenCalledWith( signedDownloadUrl, - archivePath + archivePath, + options ) expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`) @@ -391,11 +379,11 @@ test('restore with cache found for restore key', async () => { test('restore with dry run', async () => { const paths = ['node_modules'] const key = 'node-test' - const options = {lookupOnly: true} const compressionMethod = CompressionMethod.Gzip const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = 'd90f107aaeb22920dba0c637a23c37b5bc497b4dfa3b07fe3f79bf88a273c11b' + const options: DownloadOptions = {lookupOnly: true, timeoutInMs: 30000} const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -416,10 +404,11 @@ test('restore with dry run', async () => { ) const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') - const downloadCacheFileMock = jest.spyOn( - downloadCacheModule, - 'downloadCacheFile' + const downloadCacheStorageSDKMock = jest.spyOn( + downloadUtils, + 'downloadCacheStorageSDK' ) + downloadCacheStorageSDKMock.mockReturnValue(Promise.resolve()) const cacheKey = await restoreCache(paths, key, undefined, options) @@ -438,5 +427,5 @@ test('restore with dry run', async () => { // creating a tempDir and downloading the cache are skipped expect(createTempDirectoryMock).toHaveBeenCalledTimes(0) - expect(downloadCacheFileMock).toHaveBeenCalledTimes(0) + expect(downloadCacheStorageSDKMock).toHaveBeenCalledTimes(0) }) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 0a73059a..0e17e3af 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -3,6 +3,7 @@ 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' @@ -14,7 +15,6 @@ import { } from './generated/results/api/v1/cache' import {CacheFileSizeLimit} from './internal/constants' import {uploadCacheFile} from './internal/blob/upload-cache' -import {downloadCacheFile} from './internal/blob/download-cache' export class ValidationError extends Error { constructor(message: string) { super(message) @@ -161,11 +161,14 @@ async function restoreCacheV1( ) core.debug(`Archive Path: ${archivePath}`) - // Download the cache from the cache entry + // Download the cache archive from from blob storage await cacheHttpClient.downloadCache( cacheEntry.archiveLocation, archivePath, - options + options || + ({ + timeoutInMs: 30000 + } as DownloadOptions) ) if (core.isDebug()) { @@ -271,11 +274,14 @@ async function restoreCacheV2( core.debug(`Archive path: ${archivePath}`) core.debug(`Starting download of archive to: ${archivePath}`) - const downloadResponse = await downloadCacheFile( + await downloadCacheStorageSDK( response.signedDownloadUrl, - archivePath + archivePath, + options || + ({ + timeoutInMs: 30000 + } as DownloadOptions) ) - core.debug(`Download response status: ${downloadResponse._response.status}`) const archiveFileSize = utils.getArchiveFileSizeInBytes(archivePath) core.info( diff --git a/packages/cache/src/internal/blob/download-cache.ts b/packages/cache/src/internal/blob/download-cache.ts index e974cb2f..38384b10 100644 --- a/packages/cache/src/internal/blob/download-cache.ts +++ b/packages/cache/src/internal/blob/download-cache.ts @@ -22,10 +22,24 @@ export async function downloadCacheFile( `BlobClient: ${blobClient.name}:${blobClient.accountName}:${blobClient.containerName}` ) - return blockBlobClient.downloadToFile( + const response = await blockBlobClient.downloadToFile( archivePath, 0, undefined, downloadOptions ) + + switch (response._response.status) { + case 200: + core.info(`Cache downloaded from "${signedUploadURL}"`) + break + case 304: + core.info(`Cache not found at "${signedUploadURL}"`) + break + default: + core.info(`Unexpected HTTP response: ${response._response.status}`) + break + } + + return response } From af3981c955a097619c92e0db536b44cbaea0187f Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Wed, 27 Nov 2024 05:50:01 -0800 Subject: [PATCH 09/32] Update the useragent of the old http client to pass cache version --- packages/cache/src/cache.ts | 31 +++++++++---------- .../cache/src/internal/cacheHttpClient.ts | 16 +++++----- 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 0e17e3af..05dd4a0d 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 { uploadCacheFile } from './internal/blob/upload-cache' export class ValidationError extends Error { constructor(message: string) { super(message) @@ -161,14 +161,11 @@ async function restoreCacheV1( ) core.debug(`Archive Path: ${archivePath}`) - // Download the cache archive from from blob storage + // Download the cache from the cache entry await cacheHttpClient.downloadCache( cacheEntry.archiveLocation, archivePath, - options || - ({ - timeoutInMs: 30000 - } as DownloadOptions) + options ) if (core.isDebug()) { @@ -278,9 +275,9 @@ async function restoreCacheV2( response.signedDownloadUrl, archivePath, options || - ({ - timeoutInMs: 30000 - } as DownloadOptions) + ({ + timeoutInMs: 30000 + } as DownloadOptions) ) const archiveFileSize = utils.getArchiveFileSizeInBytes(archivePath) @@ -417,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( diff --git a/packages/cache/src/internal/cacheHttpClient.ts b/packages/cache/src/internal/cacheHttpClient.ts index 051348ec..6cb8ae7e 100644 --- a/packages/cache/src/internal/cacheHttpClient.ts +++ b/packages/cache/src/internal/cacheHttpClient.ts @@ -1,12 +1,12 @@ import * as core from '@actions/core' -import {HttpClient} from '@actions/http-client' -import {BearerCredentialHandler} from '@actions/http-client/lib/auth' +import { HttpClient } from '@actions/http-client' +import { BearerCredentialHandler } from '@actions/http-client/lib/auth' import { RequestOptions, TypedResponse } from '@actions/http-client/lib/interfaces' import * as fs from 'fs' -import {URL} from 'url' +import { URL } from 'url' import * as utils from './cacheUtils' import { ArtifactCacheEntry, @@ -33,7 +33,8 @@ import { retryHttpClientResponse, retryTypedResponse } from './requestUtils' -import {getCacheServiceURL} from './config' +import { getCacheServiceURL } from './config' +import { getUserAgentString } from './shared/user-agent' function getCacheApiUrl(resource: string): string { const baseUrl: string = getCacheServiceURL() @@ -65,7 +66,7 @@ function createHttpClient(): HttpClient { const bearerCredentialHandler = new BearerCredentialHandler(token) return new HttpClient( - 'actions/cache', + getUserAgentString(), [bearerCredentialHandler], getRequestOptions() ) @@ -216,8 +217,7 @@ async function uploadChunk( end: number ): Promise { core.debug( - `Uploading chunk of size ${ - end - start + 1 + `Uploading chunk of size ${end - start + 1 } bytes at offset ${start} with content range: ${getContentRange( start, end @@ -313,7 +313,7 @@ async function commitCache( cacheId: number, filesize: number ): Promise> { - const commitCacheRequest: CommitCacheRequest = {size: filesize} + const commitCacheRequest: CommitCacheRequest = { size: filesize } return await retryTypedResponse('commitCache', async () => httpClient.postJson( getCacheApiUrl(`caches/${cacheId.toString()}`), From 35d87ab1299a1c5d1adb11612007c560ff5aed8a Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Wed, 27 Nov 2024 05:58:22 -0800 Subject: [PATCH 10/32] Refactor code formatting for consistency and readability --- packages/cache/src/cache.ts | 24 +++++++++---------- .../cache/src/internal/cacheHttpClient.ts | 15 ++++++------ 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 05dd4a0d..fada75f2 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 {uploadCacheFile} from './internal/blob/upload-cache' 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( diff --git a/packages/cache/src/internal/cacheHttpClient.ts b/packages/cache/src/internal/cacheHttpClient.ts index 6cb8ae7e..c219000b 100644 --- a/packages/cache/src/internal/cacheHttpClient.ts +++ b/packages/cache/src/internal/cacheHttpClient.ts @@ -1,12 +1,12 @@ import * as core from '@actions/core' -import { HttpClient } from '@actions/http-client' -import { BearerCredentialHandler } from '@actions/http-client/lib/auth' +import {HttpClient} from '@actions/http-client' +import {BearerCredentialHandler} from '@actions/http-client/lib/auth' import { RequestOptions, TypedResponse } from '@actions/http-client/lib/interfaces' import * as fs from 'fs' -import { URL } from 'url' +import {URL} from 'url' import * as utils from './cacheUtils' import { ArtifactCacheEntry, @@ -33,8 +33,8 @@ import { retryHttpClientResponse, retryTypedResponse } from './requestUtils' -import { getCacheServiceURL } from './config' -import { getUserAgentString } from './shared/user-agent' +import {getCacheServiceURL} from './config' +import {getUserAgentString} from './shared/user-agent' function getCacheApiUrl(resource: string): string { const baseUrl: string = getCacheServiceURL() @@ -217,7 +217,8 @@ async function uploadChunk( end: number ): Promise { core.debug( - `Uploading chunk of size ${end - start + 1 + `Uploading chunk of size ${ + end - start + 1 } bytes at offset ${start} with content range: ${getContentRange( start, end @@ -313,7 +314,7 @@ async function commitCache( cacheId: number, filesize: number ): Promise> { - const commitCacheRequest: CommitCacheRequest = { size: filesize } + const commitCacheRequest: CommitCacheRequest = {size: filesize} return await retryTypedResponse('commitCache', async () => httpClient.postJson( getCacheApiUrl(`caches/${cacheId.toString()}`), From c5a5de05f6ebb26bc5bd41837b2a5536d36b8132 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Thu, 28 Nov 2024 03:36:32 -0800 Subject: [PATCH 11/32] Delete download-cache --- .../cache/src/internal/blob/download-cache.ts | 45 ------------------- 1 file changed, 45 deletions(-) delete mode 100644 packages/cache/src/internal/blob/download-cache.ts diff --git a/packages/cache/src/internal/blob/download-cache.ts b/packages/cache/src/internal/blob/download-cache.ts deleted file mode 100644 index 38384b10..00000000 --- a/packages/cache/src/internal/blob/download-cache.ts +++ /dev/null @@ -1,45 +0,0 @@ -import * as core from '@actions/core' - -import { - BlobClient, - BlockBlobClient, - BlobDownloadOptions, - BlobDownloadResponseParsed -} from '@azure/storage-blob' - -export async function downloadCacheFile( - signedUploadURL: string, - archivePath: string -): Promise { - const downloadOptions: BlobDownloadOptions = { - maxRetryRequests: 5 - } - - const blobClient: BlobClient = new BlobClient(signedUploadURL) - const blockBlobClient: BlockBlobClient = blobClient.getBlockBlobClient() - - core.debug( - `BlobClient: ${blobClient.name}:${blobClient.accountName}:${blobClient.containerName}` - ) - - const response = await blockBlobClient.downloadToFile( - archivePath, - 0, - undefined, - downloadOptions - ) - - switch (response._response.status) { - case 200: - core.info(`Cache downloaded from "${signedUploadURL}"`) - break - case 304: - core.info(`Cache not found at "${signedUploadURL}"`) - break - default: - core.info(`Unexpected HTTP response: ${response._response.status}`) - break - } - - return response -} 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 12/32] 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 From c1fb081674639f34502220b5c10235ad92584e75 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Thu, 28 Nov 2024 03:53:34 -0800 Subject: [PATCH 13/32] Linter fixes --- packages/cache/__tests__/saveCacheV2.test.ts | 34 ++++++++++---------- packages/cache/src/cache.ts | 24 +++++++------- packages/cache/src/internal/uploadUtils.ts | 11 +++---- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 2c69c5ee..ab98bc81 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 {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' +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( @@ -152,7 +152,7 @@ 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') @@ -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 @@ -241,7 +241,7 @@ 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 @@ -279,7 +279,7 @@ 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 @@ -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 adc8b915..1617b793 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 { uploadCacheArchiveSDK } from './internal/uploadUtils' +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( diff --git a/packages/cache/src/internal/uploadUtils.ts b/packages/cache/src/internal/uploadUtils.ts index ffcb37f8..60b4a315 100644 --- a/packages/cache/src/internal/uploadUtils.ts +++ b/packages/cache/src/internal/uploadUtils.ts @@ -5,13 +5,12 @@ import { BlockBlobClient, BlockBlobParallelUploadOptions } from '@azure/storage-blob' -import { InvalidResponseError } from './shared/errors' +import {InvalidResponseError} from './shared/errors' -export async function uploadCacheArchiveSDK - ( - 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 From eaf0083ee213751b2e128f7efa0b79bfb2630350 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Thu, 28 Nov 2024 04:56:37 -0800 Subject: [PATCH 14/32] Respect download options for restore --- .../cache/__tests__/restoreCacheV2.test.ts | 47 ++++++------------- packages/cache/src/cache.ts | 8 +--- 2 files changed, 16 insertions(+), 39 deletions(-) diff --git a/packages/cache/__tests__/restoreCacheV2.test.ts b/packages/cache/__tests__/restoreCacheV2.test.ts index 365afdf0..ae366412 100644 --- a/packages/cache/__tests__/restoreCacheV2.test.ts +++ b/packages/cache/__tests__/restoreCacheV2.test.ts @@ -3,7 +3,7 @@ import * as path from 'path' import * as tar from '../src/internal/tar' import * as config from '../src/internal/config' import * as cacheUtils from '../src/internal/cacheUtils' -import * as downloadUtils from '../src/internal/downloadUtils' +import * as cacheHttpClient from '../src/internal/cacheHttpClient' import {restoreCache} from '../src/cache' import {CacheFilename, CompressionMethod} from '../src/internal/constants' import {CacheServiceClientJSON} from '../src/generated/results/api/v1/cache.twirp' @@ -142,7 +142,6 @@ test('restore with gzip compressed cache found', async () => { const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = 'd90f107aaeb22920dba0c637a23c37b5bc497b4dfa3b07fe3f79bf88a273c11b' - const options: DownloadOptions = {timeoutInMs: 30000} const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -170,11 +169,7 @@ test('restore with gzip compressed cache found', async () => { }) const archivePath = path.join(tempPath, CacheFilename.Gzip) - const downloadCacheStorageSDKMock = jest.spyOn( - downloadUtils, - 'downloadCacheStorageSDK' - ) - downloadCacheStorageSDKMock.mockReturnValue(Promise.resolve()) + const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') const fileSize = 142 const getArchiveFileSizeInBytesMock = jest @@ -198,10 +193,10 @@ test('restore with gzip compressed cache found', async () => { version: cacheVersion }) expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) - expect(downloadCacheStorageSDKMock).toHaveBeenCalledWith( + expect(downloadCacheMock).toHaveBeenCalledWith( signedDownloadUrl, archivePath, - options + undefined ) expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`) @@ -222,7 +217,6 @@ test('restore with zstd compressed cache found', async () => { const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = '8e2e96a184cb0cd6b48285b176c06a418f3d7fce14c29d9886fd1bb4f05c513d' - const options: DownloadOptions = {timeoutInMs: 30000} const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -250,11 +244,7 @@ test('restore with zstd compressed cache found', async () => { }) const archivePath = path.join(tempPath, CacheFilename.Zstd) - const downloadCacheStorageSDKMock = jest.spyOn( - downloadUtils, - 'downloadCacheStorageSDK' - ) - downloadCacheStorageSDKMock.mockReturnValue(Promise.resolve()) + const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') const fileSize = 62915000 const getArchiveFileSizeInBytesMock = jest @@ -278,10 +268,10 @@ test('restore with zstd compressed cache found', async () => { version: cacheVersion }) expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) - expect(downloadCacheStorageSDKMock).toHaveBeenCalledWith( + expect(downloadCacheMock).toHaveBeenCalledWith( signedDownloadUrl, archivePath, - options + undefined ) expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~60 MB (62915000 B)`) @@ -303,7 +293,6 @@ test('restore with cache found for restore key', async () => { const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = 'b8b58e9bd7b1e8f83d9f05c7e06ea865ba44a0330e07a14db74ac74386677bed' - const options: DownloadOptions = {timeoutInMs: 30000} const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -331,11 +320,7 @@ test('restore with cache found for restore key', async () => { }) const archivePath = path.join(tempPath, CacheFilename.Gzip) - const downloadCacheStorageSDKMock = jest.spyOn( - downloadUtils, - 'downloadCacheStorageSDK' - ) - downloadCacheStorageSDKMock.mockReturnValue(Promise.resolve()) + const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') const fileSize = 142 const getArchiveFileSizeInBytesMock = jest @@ -359,10 +344,10 @@ test('restore with cache found for restore key', async () => { version: cacheVersion }) expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) - expect(downloadCacheStorageSDKMock).toHaveBeenCalledWith( + expect(downloadCacheMock).toHaveBeenCalledWith( signedDownloadUrl, archivePath, - options + undefined ) expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`) @@ -376,14 +361,14 @@ test('restore with cache found for restore key', async () => { expect(compressionMethodMock).toHaveBeenCalledTimes(1) }) -test('restore with dry run', async () => { +test('restore with lookup only enabled', async () => { const paths = ['node_modules'] const key = 'node-test' const compressionMethod = CompressionMethod.Gzip const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = 'd90f107aaeb22920dba0c637a23c37b5bc497b4dfa3b07fe3f79bf88a273c11b' - const options: DownloadOptions = {lookupOnly: true, timeoutInMs: 30000} + const options = {lookupOnly: true} as DownloadOptions const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -404,11 +389,7 @@ test('restore with dry run', async () => { ) const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory') - const downloadCacheStorageSDKMock = jest.spyOn( - downloadUtils, - 'downloadCacheStorageSDK' - ) - downloadCacheStorageSDKMock.mockReturnValue(Promise.resolve()) + const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache') const cacheKey = await restoreCache(paths, key, undefined, options) @@ -427,5 +408,5 @@ test('restore with dry run', async () => { // creating a tempDir and downloading the cache are skipped expect(createTempDirectoryMock).toHaveBeenCalledTimes(0) - expect(downloadCacheStorageSDKMock).toHaveBeenCalledTimes(0) + expect(downloadCacheMock).toHaveBeenCalledTimes(0) }) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 1617b793..ddf4e7fb 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -3,7 +3,6 @@ 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' @@ -271,13 +270,10 @@ async function restoreCacheV2( core.debug(`Archive path: ${archivePath}`) core.debug(`Starting download of archive to: ${archivePath}`) - await downloadCacheStorageSDK( + await cacheHttpClient.downloadCache( response.signedDownloadUrl, archivePath, - options || - ({ - timeoutInMs: 30000 - } as DownloadOptions) + options ) const archiveFileSize = utils.getArchiveFileSizeInBytes(archivePath) From 62f5f1885b005bf104d61bfecc827d289016f5de Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Thu, 28 Nov 2024 07:22:01 -0800 Subject: [PATCH 15/32] Refactor saveCacheV2 to use saveCache from cacheHttpClient --- packages/cache/__tests__/options.test.ts | 6 +- packages/cache/__tests__/saveCache.test.ts | 14 +- packages/cache/__tests__/saveCacheV2.test.ts | 165 ++++++++---------- packages/cache/src/cache.ts | 15 +- .../cache/src/internal/cacheHttpClient.ts | 50 ++++-- packages/cache/src/internal/uploadUtils.ts | 10 +- packages/cache/src/options.ts | 14 ++ 7 files changed, 158 insertions(+), 116 deletions(-) diff --git a/packages/cache/__tests__/options.test.ts b/packages/cache/__tests__/options.test.ts index 7585b60f..fd742487 100644 --- a/packages/cache/__tests__/options.test.ts +++ b/packages/cache/__tests__/options.test.ts @@ -47,14 +47,16 @@ test('getUploadOptions sets defaults', async () => { expect(actualOptions).toEqual({ uploadConcurrency, - uploadChunkSize + uploadChunkSize, + useAzureSdk }) }) test('getUploadOptions overrides all settings', async () => { const expectedOptions: UploadOptions = { uploadConcurrency: 2, - uploadChunkSize: 16 * 1024 * 1024 + uploadChunkSize: 16 * 1024 * 1024, + useAzureSdk: true } const actualOptions = getUploadOptions(expectedOptions) diff --git a/packages/cache/__tests__/saveCache.test.ts b/packages/cache/__tests__/saveCache.test.ts index 81049e0a..e5ed695b 100644 --- a/packages/cache/__tests__/saveCache.test.ts +++ b/packages/cache/__tests__/saveCache.test.ts @@ -270,7 +270,12 @@ test('save with server error should fail', async () => { compression ) expect(saveCacheMock).toHaveBeenCalledTimes(1) - expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archiveFile, undefined) + expect(saveCacheMock).toHaveBeenCalledWith( + cacheId, + archiveFile, + '', + undefined + ) expect(getCompressionMock).toHaveBeenCalledTimes(1) }) @@ -315,7 +320,12 @@ test('save with valid inputs uploads a cache', async () => { compression ) expect(saveCacheMock).toHaveBeenCalledTimes(1) - expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archiveFile, undefined) + expect(saveCacheMock).toHaveBeenCalledWith( + cacheId, + archiveFile, + '', + undefined + ) expect(getCompressionMock).toHaveBeenCalledTimes(1) }) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index ab98bc81..3a18272a 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -6,15 +6,14 @@ 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/uploadUtils' -import {BlobUploadCommonResponse} from '@azure/storage-blob' -import {InvalidResponseError} from '../src/internal/shared/errors' +import * as cacheHttpClient from '../src/internal/cacheHttpClient' +import {UploadOptions} from '../src/options' let logDebugMock: jest.SpyInstance jest.mock('../src/internal/tar') -let uploadFileMock = jest.fn() +const uploadFileMock = jest.fn() const blockBlobClientMock = jest.fn().mockImplementation(() => ({ uploadFile: uploadFileMock })) @@ -116,15 +115,7 @@ test('create cache entry failure', async () => { .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') .mockReturnValueOnce(archiveFileSize) const cacheVersion = cacheUtils.getCacheVersion(paths, compression) - const uploadCacheArchiveSDKMock = jest - .spyOn(uploadCacheModule, 'uploadCacheArchiveSDK') - .mockReturnValueOnce( - Promise.resolve({ - _response: { - status: 200 - } - } as BlobUploadCommonResponse) - ) + const saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache') const cacheId = await saveCache(paths, key) expect(cacheId).toBe(-1) @@ -139,15 +130,15 @@ test('create cache entry failure', async () => { expect(createTarMock).toHaveBeenCalledTimes(1) expect(getCompressionMock).toHaveBeenCalledTimes(1) expect(finalizeCacheEntryMock).toHaveBeenCalledTimes(0) - expect(uploadCacheArchiveSDKMock).toHaveBeenCalledTimes(0) + expect(saveCacheMock).toHaveBeenCalledTimes(0) }) -test('finalize save cache failure', async () => { +test('save cache fails if a signedUploadURL was not passed', async () => { const paths = 'node_modules' const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' const cachePaths = [path.resolve(paths)] - const logWarningMock = jest.spyOn(core, 'warning') - const signedUploadURL = 'https://blob-storage.local?signed=true' + const signedUploadURL = '' + const options: UploadOptions = {useAzureSdk: true} const createCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') @@ -156,16 +147,63 @@ test('finalize save cache failure', async () => { ) const createTarMock = jest.spyOn(tar, 'createTar') + const saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache') - const uploadCacheMock = jest.spyOn(uploadCacheModule, 'uploadCacheArchiveSDK') - uploadCacheMock.mockReturnValueOnce( - Promise.resolve({ - _response: { - status: 200 - } - } as BlobUploadCommonResponse) + const compression = CompressionMethod.Zstd + const getCompressionMock = jest + .spyOn(cacheUtils, 'getCompressionMethod') + .mockReturnValueOnce(Promise.resolve(compression)) + + const cacheVersion = cacheUtils.getCacheVersion([paths], compression) + const archiveFileSize = 1024 + jest + .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') + .mockReturnValueOnce(archiveFileSize) + + const cacheId = await saveCache([paths], key, options) + + expect(cacheId).toBe(-1) + expect(createCacheEntryMock).toHaveBeenCalledWith({ + key, + version: cacheVersion + }) + + const archiveFolder = '/foo/bar' + const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) + expect(createTarMock).toHaveBeenCalledWith( + archiveFolder, + cachePaths, + compression ) + expect(saveCacheMock).toHaveBeenCalledWith( + -1, + archiveFile, + signedUploadURL, + options + ) + expect(getCompressionMock).toHaveBeenCalledTimes(1) +}) + +test('finalize save cache failure', async () => { + const paths = 'node_modules' + const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' + const cachePaths = [path.resolve(paths)] + const logWarningMock = jest.spyOn(core, 'warning') + const signedUploadURL = 'https://blob-storage.local?signed=true' + const options: UploadOptions = {useAzureSdk: true} + + const createCacheEntryMock = jest + .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') + .mockReturnValue( + Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) + ) + + const createTarMock = jest.spyOn(tar, 'createTar') + const saveCacheMock = jest + .spyOn(cacheHttpClient, 'saveCache') + .mockResolvedValue(Promise.resolve()) + const compression = CompressionMethod.Zstd const getCompressionMock = jest .spyOn(cacheUtils, 'getCompressionMethod') @@ -181,7 +219,7 @@ test('finalize save cache failure', async () => { .spyOn(CacheServiceClientJSON.prototype, 'FinalizeCacheEntryUpload') .mockReturnValue(Promise.resolve({ok: false, entryId: ''})) - const cacheId = await saveCache([paths], key) + const cacheId = await saveCache([paths], key, options) expect(createCacheEntryMock).toHaveBeenCalledWith({ key, @@ -196,7 +234,12 @@ test('finalize save cache failure', async () => { compression ) - expect(uploadCacheMock).toHaveBeenCalledWith(signedUploadURL, archiveFile) + expect(saveCacheMock).toHaveBeenCalledWith( + -1, + archiveFile, + signedUploadURL, + options + ) expect(getCompressionMock).toHaveBeenCalledTimes(1) expect(finalizeCacheEntryMock).toHaveBeenCalledWith({ @@ -211,64 +254,13 @@ test('finalize save cache failure', async () => { ) }) -test('save with uploadCache Server error will fail', async () => { - const paths = 'node_modules' - const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' - const signedUploadURL = 'https://blob-storage.local?signed=true' - jest - .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') - .mockReturnValue( - Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) - ) - - const archiveFileSize = 1024 - jest - .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') - .mockReturnValueOnce(archiveFileSize) - jest - .spyOn(uploadCacheModule, 'uploadCacheArchiveSDK') - .mockRejectedValueOnce(new InvalidResponseError('boom')) - - const cacheId = await saveCache([paths], key) - expect(cacheId).toBe(-1) -}) - -test('uploadFile returns 500', async () => { - const paths = 'node_modules' - const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' - const signedUploadURL = 'https://blob-storage.local?signed=true' - const logWarningMock = jest.spyOn(core, 'warning') - jest - .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') - .mockReturnValue( - Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) - ) - - const archiveFileSize = 1024 - jest - .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') - .mockReturnValueOnce(archiveFileSize) - jest.spyOn(uploadCacheModule, 'uploadCacheArchiveSDK').mockRestore() - - uploadFileMock = jest.fn().mockResolvedValueOnce({ - _response: { - status: 500 - } - }) - const cacheId = await saveCache([paths], key) - - expect(logWarningMock).toHaveBeenCalledWith( - 'Failed to save: Upload failed with status code 500' - ) - expect(cacheId).toBe(-1) -}) - test('save with valid inputs uploads a cache', async () => { const paths = 'node_modules' const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' const cachePaths = [path.resolve(paths)] const signedUploadURL = 'https://blob-storage.local?signed=true' const createTarMock = jest.spyOn(tar, 'createTar') + const options: UploadOptions = {useAzureSdk: true} const archiveFileSize = 1024 jest @@ -282,15 +274,7 @@ test('save with valid inputs uploads a cache', async () => { Promise.resolve({ok: true, signedUploadUrl: signedUploadURL}) ) - const uploadCacheMock = jest - .spyOn(uploadCacheModule, 'uploadCacheArchiveSDK') - .mockReturnValueOnce( - Promise.resolve({ - _response: { - status: 200 - } - } as BlobUploadCommonResponse) - ) + const saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache') const compression = CompressionMethod.Zstd const getCompressionMock = jest @@ -306,7 +290,12 @@ test('save with valid inputs uploads a cache', async () => { const archiveFolder = '/foo/bar' const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) - expect(uploadCacheMock).toHaveBeenCalledWith(signedUploadURL, archiveFile) + expect(saveCacheMock).toHaveBeenCalledWith( + -1, + archiveFile, + signedUploadURL, + options + ) expect(createTarMock).toHaveBeenCalledWith( archiveFolder, cachePaths, diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index ddf4e7fb..173a1a87 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -13,7 +13,6 @@ import { GetCacheEntryDownloadURLRequest } from './generated/results/api/v1/cache' import {CacheFileSizeLimit} from './internal/constants' -import {uploadCacheArchiveSDK} from './internal/uploadUtils' export class ValidationError extends Error { constructor(message: string) { super(message) @@ -421,7 +420,7 @@ async function saveCacheV1( } core.debug(`Saving Cache (ID: ${cacheId})`) - await cacheHttpClient.saveCache(cacheId, archivePath, options) + await cacheHttpClient.saveCache(cacheId, archivePath, '', options) } catch (error) { const typedError = error as Error if (typedError.name === ValidationError.name) { @@ -458,6 +457,11 @@ async function saveCacheV2( options?: UploadOptions, enableCrossOsArchive = false ): Promise { + // Override UploadOptions to force the use of Azure + options = { + ...options, + useAzureSdk: true + } const compressionMethod = await utils.getCompressionMethod() const twirpClient = cacheTwirpClient.internalCacheTwirpClient() let cacheId = -1 @@ -517,11 +521,12 @@ async function saveCacheV2( } core.debug(`Attempting to upload cache located at: ${archivePath}`) - const uploadResponse = await uploadCacheArchiveSDK( + await cacheHttpClient.saveCache( + cacheId, + archivePath, response.signedUploadUrl, - archivePath + options ) - core.debug(`Download response status: ${uploadResponse._response.status}`) const finalizeRequest: FinalizeCacheEntryUploadRequest = { key, diff --git a/packages/cache/src/internal/cacheHttpClient.ts b/packages/cache/src/internal/cacheHttpClient.ts index c219000b..2470555b 100644 --- a/packages/cache/src/internal/cacheHttpClient.ts +++ b/packages/cache/src/internal/cacheHttpClient.ts @@ -8,6 +8,7 @@ import { import * as fs from 'fs' import {URL} from 'url' import * as utils from './cacheUtils' +import {uploadCacheArchiveSDK} from './uploadUtils' import { ArtifactCacheEntry, InternalCacheOptions, @@ -326,26 +327,45 @@ async function commitCache( export async function saveCache( cacheId: number, archivePath: string, + signedUploadURL?: string, options?: UploadOptions ): Promise { - const httpClient = createHttpClient() + const uploadOptions = getUploadOptions(options) - core.debug('Upload cache') - await uploadFile(httpClient, cacheId, archivePath, options) + if (uploadOptions.useAzureSdk) { + // Use Azure storage SDK to upload caches directly to Azure + if (!signedUploadURL) { + throw new Error( + 'Azure Storage SDK can only be used when a signed URL is provided.' + ) + } + await uploadCacheArchiveSDK(signedUploadURL, archivePath, options) + } else { + const httpClient = createHttpClient() - // Commit Cache - core.debug('Commiting cache') - const cacheSize = utils.getArchiveFileSizeInBytes(archivePath) - core.info( - `Cache Size: ~${Math.round(cacheSize / (1024 * 1024))} MB (${cacheSize} B)` - ) + core.debug('Upload cache') + await uploadFile(httpClient, cacheId, archivePath, options) - const commitCacheResponse = await commitCache(httpClient, cacheId, cacheSize) - if (!isSuccessStatusCode(commitCacheResponse.statusCode)) { - throw new Error( - `Cache service responded with ${commitCacheResponse.statusCode} during commit cache.` + // Commit Cache + core.debug('Commiting cache') + const cacheSize = utils.getArchiveFileSizeInBytes(archivePath) + core.info( + `Cache Size: ~${Math.round( + cacheSize / (1024 * 1024) + )} MB (${cacheSize} B)` ) - } - core.info('Cache saved successfully') + const commitCacheResponse = await commitCache( + httpClient, + cacheId, + cacheSize + ) + if (!isSuccessStatusCode(commitCacheResponse.statusCode)) { + throw new Error( + `Cache service responded with ${commitCacheResponse.statusCode} during commit cache.` + ) + } + + core.info('Cache saved successfully') + } } diff --git a/packages/cache/src/internal/uploadUtils.ts b/packages/cache/src/internal/uploadUtils.ts index 60b4a315..a3376d9d 100644 --- a/packages/cache/src/internal/uploadUtils.ts +++ b/packages/cache/src/internal/uploadUtils.ts @@ -6,16 +6,18 @@ import { BlockBlobParallelUploadOptions } from '@azure/storage-blob' import {InvalidResponseError} from './shared/errors' +import {UploadOptions} from '../options' export async function uploadCacheArchiveSDK( signedUploadURL: string, - archivePath: string + archivePath: string, + options?: UploadOptions ): Promise { // Specify data transfer options const uploadOptions: BlockBlobParallelUploadOptions = { - blockSize: 4 * 1024 * 1024, // 4 MiB max block size - concurrency: 4, // maximum number of parallel transfer workers - maxSingleShotSize: 8 * 1024 * 1024 // 8 MiB initial transfer size + blockSize: options?.uploadChunkSize, + concurrency: options?.uploadConcurrency, // maximum number of parallel transfer workers + maxSingleShotSize: 128 * 1024 * 1024 // 128 MiB initial transfer size } const blobClient: BlobClient = new BlobClient(signedUploadURL) diff --git a/packages/cache/src/options.ts b/packages/cache/src/options.ts index d768ff54..778c6a0e 100644 --- a/packages/cache/src/options.ts +++ b/packages/cache/src/options.ts @@ -4,6 +4,14 @@ import * as core from '@actions/core' * Options to control cache upload */ export interface UploadOptions { + /** + * Indicates whether to use the Azure Blob SDK to download caches + * that are stored on Azure Blob Storage to improve reliability and + * performance + * + * @default false + */ + useAzureSdk?: boolean /** * Number of parallel cache upload * @@ -77,11 +85,16 @@ export interface DownloadOptions { */ export function getUploadOptions(copy?: UploadOptions): UploadOptions { const result: UploadOptions = { + useAzureSdk: false, uploadConcurrency: 4, uploadChunkSize: 32 * 1024 * 1024 } if (copy) { + if (typeof copy.useAzureSdk === 'boolean') { + result.useAzureSdk = copy.useAzureSdk + } + if (typeof copy.uploadConcurrency === 'number') { result.uploadConcurrency = copy.uploadConcurrency } @@ -91,6 +104,7 @@ export function getUploadOptions(copy?: UploadOptions): UploadOptions { } } + core.debug(`Use Azure SDK: ${result.useAzureSdk}`) core.debug(`Upload concurrency: ${result.uploadConcurrency}`) core.debug(`Upload chunk size: ${result.uploadChunkSize}`) From 8c5f6f2dc5acc4574678e4e51df57f2f1779473a Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Thu, 28 Nov 2024 07:42:07 -0800 Subject: [PATCH 16/32] Force use of Azure for restoreCacheV2 --- packages/cache/__tests__/restoreCacheV2.test.ts | 17 ++++++++++------- packages/cache/src/cache.ts | 5 +++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/cache/__tests__/restoreCacheV2.test.ts b/packages/cache/__tests__/restoreCacheV2.test.ts index ae366412..edcb16d7 100644 --- a/packages/cache/__tests__/restoreCacheV2.test.ts +++ b/packages/cache/__tests__/restoreCacheV2.test.ts @@ -142,6 +142,7 @@ test('restore with gzip compressed cache found', async () => { const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = 'd90f107aaeb22920dba0c637a23c37b5bc497b4dfa3b07fe3f79bf88a273c11b' + const options = {useAzureSdk: true} as DownloadOptions const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -179,7 +180,7 @@ test('restore with gzip compressed cache found', async () => { const extractTarMock = jest.spyOn(tar, 'extractTar') const unlinkFileMock = jest.spyOn(cacheUtils, 'unlinkFile') - const cacheKey = await restoreCache(paths, key) + const cacheKey = await restoreCache(paths, key, [], options) expect(cacheKey).toBe(key) expect(getCacheVersionMock).toHaveBeenCalledWith( @@ -196,7 +197,7 @@ test('restore with gzip compressed cache found', async () => { expect(downloadCacheMock).toHaveBeenCalledWith( signedDownloadUrl, archivePath, - undefined + options ) expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`) @@ -217,6 +218,7 @@ test('restore with zstd compressed cache found', async () => { const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = '8e2e96a184cb0cd6b48285b176c06a418f3d7fce14c29d9886fd1bb4f05c513d' + const options = {useAzureSdk: true} as DownloadOptions const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -254,7 +256,7 @@ test('restore with zstd compressed cache found', async () => { const extractTarMock = jest.spyOn(tar, 'extractTar') const unlinkFileMock = jest.spyOn(cacheUtils, 'unlinkFile') - const cacheKey = await restoreCache(paths, key) + const cacheKey = await restoreCache(paths, key, [], options) expect(cacheKey).toBe(key) expect(getCacheVersionMock).toHaveBeenCalledWith( @@ -271,7 +273,7 @@ test('restore with zstd compressed cache found', async () => { expect(downloadCacheMock).toHaveBeenCalledWith( signedDownloadUrl, archivePath, - undefined + options ) expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~60 MB (62915000 B)`) @@ -293,6 +295,7 @@ test('restore with cache found for restore key', async () => { const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = 'b8b58e9bd7b1e8f83d9f05c7e06ea865ba44a0330e07a14db74ac74386677bed' + const options = {useAzureSdk: true} as DownloadOptions const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) @@ -330,7 +333,7 @@ test('restore with cache found for restore key', async () => { const extractTarMock = jest.spyOn(tar, 'extractTar') const unlinkFileMock = jest.spyOn(cacheUtils, 'unlinkFile') - const cacheKey = await restoreCache(paths, key, restoreKeys) + const cacheKey = await restoreCache(paths, key, restoreKeys, options) expect(cacheKey).toBe(restoreKeys[0]) expect(getCacheVersionMock).toHaveBeenCalledWith( @@ -347,7 +350,7 @@ test('restore with cache found for restore key', async () => { expect(downloadCacheMock).toHaveBeenCalledWith( signedDownloadUrl, archivePath, - undefined + options ) expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath) expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`) @@ -368,7 +371,7 @@ test('restore with lookup only enabled', async () => { const signedDownloadUrl = 'https://blob-storage.local?signed=true' const cacheVersion = 'd90f107aaeb22920dba0c637a23c37b5bc497b4dfa3b07fe3f79bf88a273c11b' - const options = {lookupOnly: true} as DownloadOptions + const options = {lookupOnly: true, useAzureSdk: true} as DownloadOptions const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion') getCacheVersionMock.mockReturnValue(cacheVersion) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 173a1a87..0bfbf894 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -218,6 +218,11 @@ async function restoreCacheV2( options?: DownloadOptions, enableCrossOsArchive = false ): Promise { + // Override UploadOptions to force the use of Azure + options = { + ...options, + useAzureSdk: true + } restoreKeys = restoreKeys || [] const keys = [primaryKey, ...restoreKeys] From 65892d5ffe49d496db47651191918073a9b5c90a Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Fri, 29 Nov 2024 07:09:05 -0800 Subject: [PATCH 17/32] Fine tune blob uploads --- packages/cache/src/cache.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 0bfbf894..139512f9 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -465,6 +465,8 @@ async function saveCacheV2( // Override UploadOptions to force the use of Azure options = { ...options, + uploadChunkSize: 64 * 1024 * 1024, // 128MiB + uploadConcurrency: 8, // 8 workers for parallel upload useAzureSdk: true } const compressionMethod = await utils.getCompressionMethod() From 1d403c2fd88438aeb755bc61ffd22630e0a3fea2 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Fri, 29 Nov 2024 07:36:51 -0800 Subject: [PATCH 18/32] Fix tests --- packages/cache/__tests__/saveCacheV2.test.ts | 18 +++++++++++++++--- packages/cache/src/cache.ts | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 3a18272a..94a2462e 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -138,7 +138,11 @@ test('save cache fails if a signedUploadURL was not passed', async () => { const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' const cachePaths = [path.resolve(paths)] const signedUploadURL = '' - const options: UploadOptions = {useAzureSdk: true} + const options: UploadOptions = { + useAzureSdk: true, + uploadChunkSize: 64 * 1024 * 1024, + uploadConcurrency: 8 + } const createCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') @@ -191,7 +195,11 @@ test('finalize save cache failure', async () => { const cachePaths = [path.resolve(paths)] const logWarningMock = jest.spyOn(core, 'warning') const signedUploadURL = 'https://blob-storage.local?signed=true' - const options: UploadOptions = {useAzureSdk: true} + const options: UploadOptions = { + useAzureSdk: true, + uploadChunkSize: 64 * 1024 * 1024, + uploadConcurrency: 8 + } const createCacheEntryMock = jest .spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry') @@ -260,7 +268,11 @@ test('save with valid inputs uploads a cache', async () => { const cachePaths = [path.resolve(paths)] const signedUploadURL = 'https://blob-storage.local?signed=true' const createTarMock = jest.spyOn(tar, 'createTar') - const options: UploadOptions = {useAzureSdk: true} + const options: UploadOptions = { + useAzureSdk: true, + uploadChunkSize: 64 * 1024 * 1024, + uploadConcurrency: 8 + } const archiveFileSize = 1024 jest diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 139512f9..2a89d50d 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -465,7 +465,7 @@ async function saveCacheV2( // Override UploadOptions to force the use of Azure options = { ...options, - uploadChunkSize: 64 * 1024 * 1024, // 128MiB + uploadChunkSize: 64 * 1024 * 1024, // 64 MiB uploadConcurrency: 8, // 8 workers for parallel upload useAzureSdk: true } From c6f1224d30a062385d872421e3a5c0efab0923e7 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 02:33:27 -0800 Subject: [PATCH 19/32] Add progress tracking for blob uploads --- packages/cache/__tests__/uploadUtils.test.ts | 58 +++++++ packages/cache/src/internal/uploadUtils.ts | 155 +++++++++++++++++-- 2 files changed, 199 insertions(+), 14 deletions(-) create mode 100644 packages/cache/__tests__/uploadUtils.test.ts diff --git a/packages/cache/__tests__/uploadUtils.test.ts b/packages/cache/__tests__/uploadUtils.test.ts new file mode 100644 index 00000000..6a4876d1 --- /dev/null +++ b/packages/cache/__tests__/uploadUtils.test.ts @@ -0,0 +1,58 @@ +import {UploadProgress} from '../src/internal/uploadUtils' +import {TransferProgressEvent} from '@azure/ms-rest-js' + +test('upload progress tracked correctly', () => { + const progress = new UploadProgress(1000) + + expect(progress.contentLength).toBe(1000) + expect(progress.sentBytes).toBe(0) + expect(progress.displayedComplete).toBe(false) + expect(progress.timeoutHandle).toBeUndefined() + expect(progress.getTransferredBytes()).toBe(0) + expect(progress.isDone()).toBe(false) + + progress.onProgress()({loadedBytes: 0} as TransferProgressEvent) + + expect(progress.contentLength).toBe(1000) + expect(progress.sentBytes).toBe(0) + expect(progress.displayedComplete).toBe(false) + expect(progress.timeoutHandle).toBeUndefined() + expect(progress.getTransferredBytes()).toBe(0) + expect(progress.isDone()).toBe(false) + + progress.onProgress()({loadedBytes: 250} as TransferProgressEvent) + + expect(progress.contentLength).toBe(1000) + expect(progress.sentBytes).toBe(250) + expect(progress.displayedComplete).toBe(false) + expect(progress.timeoutHandle).toBeUndefined() + expect(progress.getTransferredBytes()).toBe(250) + expect(progress.isDone()).toBe(false) + + progress.onProgress()({loadedBytes: 500} as TransferProgressEvent) + + expect(progress.contentLength).toBe(1000) + expect(progress.sentBytes).toBe(500) + expect(progress.displayedComplete).toBe(false) + expect(progress.timeoutHandle).toBeUndefined() + expect(progress.getTransferredBytes()).toBe(500) + expect(progress.isDone()).toBe(false) + + progress.onProgress()({loadedBytes: 750} as TransferProgressEvent) + + expect(progress.contentLength).toBe(1000) + expect(progress.sentBytes).toBe(750) + expect(progress.displayedComplete).toBe(false) + expect(progress.timeoutHandle).toBeUndefined() + expect(progress.getTransferredBytes()).toBe(750) + expect(progress.isDone()).toBe(false) + + progress.onProgress()({loadedBytes: 1000} as TransferProgressEvent) + + expect(progress.contentLength).toBe(1000) + expect(progress.sentBytes).toBe(1000) + expect(progress.displayedComplete).toBe(false) + expect(progress.timeoutHandle).toBeUndefined() + expect(progress.getTransferredBytes()).toBe(1000) + expect(progress.isDone()).toBe(true) +}) diff --git a/packages/cache/src/internal/uploadUtils.ts b/packages/cache/src/internal/uploadUtils.ts index a3376d9d..5ba98f91 100644 --- a/packages/cache/src/internal/uploadUtils.ts +++ b/packages/cache/src/internal/uploadUtils.ts @@ -5,35 +5,162 @@ import { BlockBlobClient, BlockBlobParallelUploadOptions } from '@azure/storage-blob' +import {TransferProgressEvent} from '@azure/ms-rest-js' import {InvalidResponseError} from './shared/errors' import {UploadOptions} from '../options' +/** + * Class for tracking the upload state and displaying stats. + */ +export class UploadProgress { + contentLength: number + sentBytes: number + startTime: number + displayedComplete: boolean + timeoutHandle?: ReturnType + + constructor(contentLength: number) { + this.contentLength = contentLength + this.sentBytes = 0 + this.displayedComplete = false + this.startTime = Date.now() + } + + /** + * Sets the number of bytes sent + * + * @param sentBytes the number of bytes sent + */ + setSentBytes(sentBytes: number): void { + this.sentBytes = sentBytes + } + + /** + * Returns the total number of bytes transferred. + */ + getTransferredBytes(): number { + return this.sentBytes + } + + /** + * Returns true if the upload is complete. + */ + isDone(): boolean { + return this.getTransferredBytes() === this.contentLength + } + + /** + * Prints the current upload stats. Once the upload completes, this will print one + * last line and then stop. + */ + display(): void { + if (this.displayedComplete) { + return + } + + const transferredBytes = this.sentBytes + const percentage = (100 * (transferredBytes / this.contentLength)).toFixed( + 1 + ) + const elapsedTime = Date.now() - this.startTime + const uploadSpeed = ( + transferredBytes / + (1024 * 1024) / + (elapsedTime / 1000) + ).toFixed(1) + + core.info( + `Sent ${transferredBytes} of ${this.contentLength} (${percentage}%), ${uploadSpeed} MBs/sec` + ) + + if (this.isDone()) { + this.displayedComplete = true + } + } + + /** + * Returns a function used to handle TransferProgressEvents. + */ + onProgress(): (progress: TransferProgressEvent) => void { + return (progress: TransferProgressEvent) => { + this.setSentBytes(progress.loadedBytes) + } + } + + /** + * Starts the timer that displays the stats. + * + * @param delayInMs the delay between each write + */ + startDisplayTimer(delayInMs = 1000): void { + const displayCallback = (): void => { + this.display() + + if (!this.isDone()) { + this.timeoutHandle = setTimeout(displayCallback, delayInMs) + } + } + + this.timeoutHandle = setTimeout(displayCallback, delayInMs) + } + + /** + * Stops the timer that displays the stats. As this typically indicates the upload + * is complete, this will display one last line, unless the last line has already + * been written. + */ + stopDisplayTimer(): void { + if (this.timeoutHandle) { + clearTimeout(this.timeoutHandle) + this.timeoutHandle = undefined + } + + this.display() + } +} + export async function uploadCacheArchiveSDK( signedUploadURL: string, archivePath: string, options?: UploadOptions ): Promise { + const blobClient: BlobClient = new BlobClient(signedUploadURL) + const blockBlobClient: BlockBlobClient = blobClient.getBlockBlobClient() + + const properties = await blobClient.getProperties() + const contentLength = properties.contentLength ?? -1 + + const uploadProgress = new UploadProgress(contentLength) + // Specify data transfer options const uploadOptions: BlockBlobParallelUploadOptions = { blockSize: options?.uploadChunkSize, concurrency: options?.uploadConcurrency, // maximum number of parallel transfer workers - maxSingleShotSize: 128 * 1024 * 1024 // 128 MiB initial transfer size + maxSingleShotSize: 128 * 1024 * 1024, // 128 MiB initial transfer size + onProgress: uploadProgress.onProgress() } - const blobClient: BlobClient = new BlobClient(signedUploadURL) - const blockBlobClient: BlockBlobClient = blobClient.getBlockBlobClient() + try { + uploadProgress.startDisplayTimer() - core.debug( - `BlobClient: ${blobClient.name}:${blobClient.accountName}:${blobClient.containerName}` - ) - - const resp = await blockBlobClient.uploadFile(archivePath, uploadOptions) - - if (resp._response.status >= 400) { - throw new InvalidResponseError( - `Upload failed with status code ${resp._response.status}` + core.debug( + `BlobClient: ${blobClient.name}:${blobClient.accountName}:${blobClient.containerName}` ) - } - return resp + const response = await blockBlobClient.uploadFile( + archivePath, + uploadOptions + ) + + // TODO: better management of non-retryable errors + if (response._response.status >= 400) { + throw new InvalidResponseError( + `Upload failed with status code ${response._response.status}` + ) + } + + return response + } finally { + uploadProgress.stopDisplayTimer() + } } From ee1c07d0aafbf266d3a58a4f7bc73efde3f47414 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 02:38:51 -0800 Subject: [PATCH 20/32] Add error handling for failed uploads --- packages/cache/src/internal/uploadUtils.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/cache/src/internal/uploadUtils.ts b/packages/cache/src/internal/uploadUtils.ts index 5ba98f91..efb80de1 100644 --- a/packages/cache/src/internal/uploadUtils.ts +++ b/packages/cache/src/internal/uploadUtils.ts @@ -160,6 +160,9 @@ export async function uploadCacheArchiveSDK( } return response + } catch (error) { + core.debug(`Error uploading cache archive: ${error}`) + throw error } finally { uploadProgress.stopDisplayTimer() } From 4a272e90530ba272dcc251d638f0aa4bffe91f61 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 03:08:05 -0800 Subject: [PATCH 21/32] Troubleshoot --- packages/cache/src/internal/uploadUtils.ts | 43 ++++++++++------------ 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/packages/cache/src/internal/uploadUtils.ts b/packages/cache/src/internal/uploadUtils.ts index efb80de1..372b2176 100644 --- a/packages/cache/src/internal/uploadUtils.ts +++ b/packages/cache/src/internal/uploadUtils.ts @@ -140,30 +140,27 @@ export async function uploadCacheArchiveSDK( onProgress: uploadProgress.onProgress() } - try { - uploadProgress.startDisplayTimer() + // try { + uploadProgress.startDisplayTimer() - core.debug( - `BlobClient: ${blobClient.name}:${blobClient.accountName}:${blobClient.containerName}` + core.debug( + `BlobClient: ${blobClient.name}:${blobClient.accountName}:${blobClient.containerName}` + ) + + const response = await blockBlobClient.uploadFile(archivePath, uploadOptions) + + // TODO: better management of non-retryable errors + if (response._response.status >= 400) { + throw new InvalidResponseError( + `Upload failed with status code ${response._response.status}` ) - - const response = await blockBlobClient.uploadFile( - archivePath, - uploadOptions - ) - - // TODO: better management of non-retryable errors - if (response._response.status >= 400) { - throw new InvalidResponseError( - `Upload failed with status code ${response._response.status}` - ) - } - - return response - } catch (error) { - core.debug(`Error uploading cache archive: ${error}`) - throw error - } finally { - uploadProgress.stopDisplayTimer() } + + return response + // } catch (error) { + // core.debug(`Error uploading cache archive: ${error}`) + // throw error + // } finally { + // uploadProgress.stopDisplayTimer() + // } } From db1d01308c2999654e70c419d5a545c345ed5fa2 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 03:35:20 -0800 Subject: [PATCH 22/32] Troubleshoot --- packages/cache/__tests__/uploadUtils.test.ts | 26 +++++++++++- packages/cache/src/cache.ts | 1 + packages/cache/src/internal/uploadUtils.ts | 43 +++++++++++--------- 3 files changed, 48 insertions(+), 22 deletions(-) diff --git a/packages/cache/__tests__/uploadUtils.test.ts b/packages/cache/__tests__/uploadUtils.test.ts index 6a4876d1..65fed6f7 100644 --- a/packages/cache/__tests__/uploadUtils.test.ts +++ b/packages/cache/__tests__/uploadUtils.test.ts @@ -1,8 +1,8 @@ -import {UploadProgress} from '../src/internal/uploadUtils' +import * as uploadUtils from '../src/internal/uploadUtils' import {TransferProgressEvent} from '@azure/ms-rest-js' test('upload progress tracked correctly', () => { - const progress = new UploadProgress(1000) + const progress = new uploadUtils.UploadProgress(1000) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(0) @@ -56,3 +56,25 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(1000) expect(progress.isDone()).toBe(true) }) + +// test('upload to azure blob storage is successful', () => { +// const archivePath = 'path/to/archive.tzst' +// const signedUploadURL = 'https://storage10.blob.core.windows.net/cache-container/3fe-60?se=2024-12-002T11%3A08%3A58Z&sv=2024-11-04' +// const options: UploadOptions = { +// useAzureSdk: true, +// uploadChunkSize: 64 * 1024 * 1024, +// uploadConcurrency: 8 +// } + +// jest.spyOn(uploadUtils.UploadProgress.prototype, 'onProgress').mockImplementation(() => (progress: TransferProgressEvent) => { +// return progress.loadedBytes +// }) + +// jest.spyOn(uploadUtils.UploadProgress.prototype, 'onProgress').mockImplementation(() => (progress: TransferProgressEvent) => { +// return progress.loadedBytes +// }) + +// const response = uploadUtils.uploadCacheArchiveSDK(signedUploadURL, archivePath, options) + +// expect(response).toBeInstanceOf(Promise) +// }) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 2a89d50d..69931e29 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -561,6 +561,7 @@ async function saveCacheV2( } else { core.warning(`Failed to save: ${typedError.message}`) } + throw error } finally { // Try to delete the archive to save space try { diff --git a/packages/cache/src/internal/uploadUtils.ts b/packages/cache/src/internal/uploadUtils.ts index 372b2176..efb80de1 100644 --- a/packages/cache/src/internal/uploadUtils.ts +++ b/packages/cache/src/internal/uploadUtils.ts @@ -140,27 +140,30 @@ export async function uploadCacheArchiveSDK( onProgress: uploadProgress.onProgress() } - // try { - uploadProgress.startDisplayTimer() + try { + uploadProgress.startDisplayTimer() - core.debug( - `BlobClient: ${blobClient.name}:${blobClient.accountName}:${blobClient.containerName}` - ) - - const response = await blockBlobClient.uploadFile(archivePath, uploadOptions) - - // TODO: better management of non-retryable errors - if (response._response.status >= 400) { - throw new InvalidResponseError( - `Upload failed with status code ${response._response.status}` + core.debug( + `BlobClient: ${blobClient.name}:${blobClient.accountName}:${blobClient.containerName}` ) - } - return response - // } catch (error) { - // core.debug(`Error uploading cache archive: ${error}`) - // throw error - // } finally { - // uploadProgress.stopDisplayTimer() - // } + const response = await blockBlobClient.uploadFile( + archivePath, + uploadOptions + ) + + // TODO: better management of non-retryable errors + if (response._response.status >= 400) { + throw new InvalidResponseError( + `Upload failed with status code ${response._response.status}` + ) + } + + return response + } catch (error) { + core.debug(`Error uploading cache archive: ${error}`) + throw error + } finally { + uploadProgress.stopDisplayTimer() + } } From d89855bb90ff9a5c406c07a99f409d4bbb66e9a0 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 03:55:57 -0800 Subject: [PATCH 23/32] Fix upload progress bug --- packages/cache/src/cache.ts | 5 ++++- packages/cache/src/internal/uploadUtils.ts | 10 ++++------ packages/cache/src/options.ts | 4 ++++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 69931e29..78439141 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -509,6 +509,10 @@ async function saveCacheV2( ) } + // Set the archive size in the options, will be used to display the upload + // progress + options.archiveSizeBytes = archiveFileSize + core.debug('Reserving Cache') const version = utils.getCacheVersion( paths, @@ -561,7 +565,6 @@ async function saveCacheV2( } else { core.warning(`Failed to save: ${typedError.message}`) } - throw error } finally { // Try to delete the archive to save space try { diff --git a/packages/cache/src/internal/uploadUtils.ts b/packages/cache/src/internal/uploadUtils.ts index efb80de1..7d471a0d 100644 --- a/packages/cache/src/internal/uploadUtils.ts +++ b/packages/cache/src/internal/uploadUtils.ts @@ -126,11 +126,7 @@ export async function uploadCacheArchiveSDK( ): Promise { const blobClient: BlobClient = new BlobClient(signedUploadURL) const blockBlobClient: BlockBlobClient = blobClient.getBlockBlobClient() - - const properties = await blobClient.getProperties() - const contentLength = properties.contentLength ?? -1 - - const uploadProgress = new UploadProgress(contentLength) + const uploadProgress = new UploadProgress(options?.archiveSizeBytes ?? 0) // Specify data transfer options const uploadOptions: BlockBlobParallelUploadOptions = { @@ -161,7 +157,9 @@ export async function uploadCacheArchiveSDK( return response } catch (error) { - core.debug(`Error uploading cache archive: ${error}`) + core.warning( + `uploadCacheArchiveSDK: internal error uploading cache archive: ${error.message}` + ) throw error } finally { uploadProgress.stopDisplayTimer() diff --git a/packages/cache/src/options.ts b/packages/cache/src/options.ts index 778c6a0e..08e71c10 100644 --- a/packages/cache/src/options.ts +++ b/packages/cache/src/options.ts @@ -24,6 +24,10 @@ export interface UploadOptions { * @default 32MB */ uploadChunkSize?: number + /** + * Archive size in bytes + */ + archiveSizeBytes?: number } /** From a762876d6d79617f23705f7dd9b6a71ec69c11aa Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 04:08:21 -0800 Subject: [PATCH 24/32] Minor refactoring --- packages/cache/src/cache.ts | 27 +++++++++++----------- packages/cache/src/internal/uploadUtils.ts | 12 +++++++++- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 78439141..f94ccc6b 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -106,12 +106,12 @@ export async function restoreCache( /** * Restores cache using the legacy Cache Service * - * @param paths - * @param primaryKey - * @param restoreKeys - * @param options - * @param enableCrossOsArchive - * @returns + * @param paths a list of file paths to restore from the cache + * @param primaryKey an explicit key for restoring the cache + * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for key + * @param options cache download options + * @param enableCrossOsArchive an optional boolean enabled to restore on windows any cache created on any platform + * @returns string returns the key for the cache hit, otherwise returns undefined */ async function restoreCacheV1( paths: string[], @@ -202,7 +202,7 @@ async function restoreCacheV1( } /** - * Restores cache using the new Cache Service + * Restores cache using Cache Service v2 * * @param paths a list of file paths to restore from the cache * @param primaryKey an explicit key for restoring the cache @@ -448,12 +448,12 @@ async function saveCacheV1( } /** - * Save cache using the new Cache Service + * Save cache using Cache Service v2 * - * @param paths - * @param key - * @param options - * @param enableCrossOsArchive + * @param paths a list of file paths to restore from the cache + * @param key an explicit key for restoring the cache + * @param options cache upload options + * @param enableCrossOsArchive an optional boolean enabled to save cache on windows which could be restored on any platform * @returns */ async function saveCacheV2( @@ -509,8 +509,7 @@ async function saveCacheV2( ) } - // Set the archive size in the options, will be used to display the upload - // progress + // Set the archive size in the options, will be used to display the upload progress options.archiveSizeBytes = archiveFileSize core.debug('Reserving Cache') diff --git a/packages/cache/src/internal/uploadUtils.ts b/packages/cache/src/internal/uploadUtils.ts index 7d471a0d..1b4f7af0 100644 --- a/packages/cache/src/internal/uploadUtils.ts +++ b/packages/cache/src/internal/uploadUtils.ts @@ -119,6 +119,16 @@ export class UploadProgress { } } +/** + * Uploads a cache archive directly to Azure Blob Storage using the Azure SDK. + * This function will display progress information to the console. Concurrency of the + * upload is determined by the calling functions. + * + * @param signedUploadURL + * @param archivePath + * @param options + * @returns + */ export async function uploadCacheArchiveSDK( signedUploadURL: string, archivePath: string, @@ -151,7 +161,7 @@ export async function uploadCacheArchiveSDK( // TODO: better management of non-retryable errors if (response._response.status >= 400) { throw new InvalidResponseError( - `Upload failed with status code ${response._response.status}` + `uploadCacheArchiveSDK: upload failed with status code ${response._response.status}` ) } From 87171e29ca36c26436ba9ad4ccd469538bf746b7 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 04:18:46 -0800 Subject: [PATCH 25/32] Fix tests --- packages/cache/__tests__/saveCacheV2.test.ts | 39 +++++++++++--------- packages/cache/__tests__/uploadUtils.test.ts | 34 +++-------------- 2 files changed, 27 insertions(+), 46 deletions(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 94a2462e..285d973b 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -1,13 +1,13 @@ 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 { CacheServiceClientJSON } from '../src/generated/results/api/v1/cache.twirp' import * as cacheHttpClient from '../src/internal/cacheHttpClient' -import {UploadOptions} from '../src/options' +import { UploadOptions } from '../src/options' let logDebugMock: jest.SpyInstance @@ -27,11 +27,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)) }) @@ -99,7 +99,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( @@ -138,7 +138,9 @@ test('save cache fails if a signedUploadURL was not passed', async () => { const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43' const cachePaths = [path.resolve(paths)] const signedUploadURL = '' + const archiveFileSize = 1024 const options: UploadOptions = { + archiveSizeBytes: archiveFileSize, // These should always match useAzureSdk: true, uploadChunkSize: 64 * 1024 * 1024, uploadConcurrency: 8 @@ -147,7 +149,7 @@ test('save cache fails if a signedUploadURL was not passed', 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') @@ -159,7 +161,6 @@ test('save cache fails if a signedUploadURL was not passed', async () => { .mockReturnValueOnce(Promise.resolve(compression)) const cacheVersion = cacheUtils.getCacheVersion([paths], compression) - const archiveFileSize = 1024 jest .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') .mockReturnValueOnce(archiveFileSize) @@ -195,7 +196,9 @@ test('finalize save cache failure', async () => { const cachePaths = [path.resolve(paths)] const logWarningMock = jest.spyOn(core, 'warning') const signedUploadURL = 'https://blob-storage.local?signed=true' + const archiveFileSize = 1024 const options: UploadOptions = { + archiveSizeBytes: archiveFileSize, // These should always match useAzureSdk: true, uploadChunkSize: 64 * 1024 * 1024, uploadConcurrency: 8 @@ -204,7 +207,7 @@ 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') @@ -218,14 +221,13 @@ test('finalize save cache failure', async () => { .mockReturnValueOnce(Promise.resolve(compression)) const cacheVersion = cacheUtils.getCacheVersion([paths], compression) - const archiveFileSize = 1024 jest .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') .mockReturnValueOnce(archiveFileSize) 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, options) @@ -268,13 +270,14 @@ test('save with valid inputs uploads a cache', async () => { const cachePaths = [path.resolve(paths)] const signedUploadURL = 'https://blob-storage.local?signed=true' const createTarMock = jest.spyOn(tar, 'createTar') + const archiveFileSize = 1024 const options: UploadOptions = { + archiveSizeBytes: archiveFileSize, // These should always match useAzureSdk: true, uploadChunkSize: 64 * 1024 * 1024, uploadConcurrency: 8 } - const archiveFileSize = 1024 jest .spyOn(cacheUtils, 'getArchiveFileSizeInBytes') .mockReturnValueOnce(archiveFileSize) @@ -283,7 +286,7 @@ 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 saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache') @@ -296,7 +299,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/__tests__/uploadUtils.test.ts b/packages/cache/__tests__/uploadUtils.test.ts index 65fed6f7..786b1d3f 100644 --- a/packages/cache/__tests__/uploadUtils.test.ts +++ b/packages/cache/__tests__/uploadUtils.test.ts @@ -1,5 +1,5 @@ import * as uploadUtils from '../src/internal/uploadUtils' -import {TransferProgressEvent} from '@azure/ms-rest-js' +import { TransferProgressEvent } from '@azure/ms-rest-js' test('upload progress tracked correctly', () => { const progress = new uploadUtils.UploadProgress(1000) @@ -11,7 +11,7 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(0) expect(progress.isDone()).toBe(false) - progress.onProgress()({loadedBytes: 0} as TransferProgressEvent) + progress.onProgress()({ loadedBytes: 0 } as TransferProgressEvent) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(0) @@ -20,7 +20,7 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(0) expect(progress.isDone()).toBe(false) - progress.onProgress()({loadedBytes: 250} as TransferProgressEvent) + progress.onProgress()({ loadedBytes: 250 } as TransferProgressEvent) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(250) @@ -29,7 +29,7 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(250) expect(progress.isDone()).toBe(false) - progress.onProgress()({loadedBytes: 500} as TransferProgressEvent) + progress.onProgress()({ loadedBytes: 500 } as TransferProgressEvent) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(500) @@ -38,7 +38,7 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(500) expect(progress.isDone()).toBe(false) - progress.onProgress()({loadedBytes: 750} as TransferProgressEvent) + progress.onProgress()({ loadedBytes: 750 } as TransferProgressEvent) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(750) @@ -47,7 +47,7 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(750) expect(progress.isDone()).toBe(false) - progress.onProgress()({loadedBytes: 1000} as TransferProgressEvent) + progress.onProgress()({ loadedBytes: 1000 } as TransferProgressEvent) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(1000) @@ -56,25 +56,3 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(1000) expect(progress.isDone()).toBe(true) }) - -// test('upload to azure blob storage is successful', () => { -// const archivePath = 'path/to/archive.tzst' -// const signedUploadURL = 'https://storage10.blob.core.windows.net/cache-container/3fe-60?se=2024-12-002T11%3A08%3A58Z&sv=2024-11-04' -// const options: UploadOptions = { -// useAzureSdk: true, -// uploadChunkSize: 64 * 1024 * 1024, -// uploadConcurrency: 8 -// } - -// jest.spyOn(uploadUtils.UploadProgress.prototype, 'onProgress').mockImplementation(() => (progress: TransferProgressEvent) => { -// return progress.loadedBytes -// }) - -// jest.spyOn(uploadUtils.UploadProgress.prototype, 'onProgress').mockImplementation(() => (progress: TransferProgressEvent) => { -// return progress.loadedBytes -// }) - -// const response = uploadUtils.uploadCacheArchiveSDK(signedUploadURL, archivePath, options) - -// expect(response).toBeInstanceOf(Promise) -// }) From 7ad18fd6bd04185a7f728e401936e1d2b5dbe281 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 04:24:17 -0800 Subject: [PATCH 26/32] Fix linter complaints --- packages/cache/__tests__/saveCacheV2.test.ts | 30 ++++++++++---------- packages/cache/__tests__/uploadUtils.test.ts | 12 ++++---- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index 285d973b..6744425d 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -1,13 +1,13 @@ 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 {CacheServiceClientJSON} from '../src/generated/results/api/v1/cache.twirp' import * as cacheHttpClient from '../src/internal/cacheHttpClient' -import { UploadOptions } from '../src/options' +import {UploadOptions} from '../src/options' let logDebugMock: jest.SpyInstance @@ -27,11 +27,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)) }) @@ -99,7 +99,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( @@ -149,7 +149,7 @@ test('save cache fails if a signedUploadURL was not passed', 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') @@ -207,7 +207,7 @@ 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') @@ -227,7 +227,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, options) @@ -286,7 +286,7 @@ 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 saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache') @@ -299,7 +299,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/__tests__/uploadUtils.test.ts b/packages/cache/__tests__/uploadUtils.test.ts index 786b1d3f..2f0b8b55 100644 --- a/packages/cache/__tests__/uploadUtils.test.ts +++ b/packages/cache/__tests__/uploadUtils.test.ts @@ -1,5 +1,5 @@ import * as uploadUtils from '../src/internal/uploadUtils' -import { TransferProgressEvent } from '@azure/ms-rest-js' +import {TransferProgressEvent} from '@azure/ms-rest-js' test('upload progress tracked correctly', () => { const progress = new uploadUtils.UploadProgress(1000) @@ -11,7 +11,7 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(0) expect(progress.isDone()).toBe(false) - progress.onProgress()({ loadedBytes: 0 } as TransferProgressEvent) + progress.onProgress()({loadedBytes: 0} as TransferProgressEvent) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(0) @@ -20,7 +20,7 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(0) expect(progress.isDone()).toBe(false) - progress.onProgress()({ loadedBytes: 250 } as TransferProgressEvent) + progress.onProgress()({loadedBytes: 250} as TransferProgressEvent) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(250) @@ -29,7 +29,7 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(250) expect(progress.isDone()).toBe(false) - progress.onProgress()({ loadedBytes: 500 } as TransferProgressEvent) + progress.onProgress()({loadedBytes: 500} as TransferProgressEvent) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(500) @@ -38,7 +38,7 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(500) expect(progress.isDone()).toBe(false) - progress.onProgress()({ loadedBytes: 750 } as TransferProgressEvent) + progress.onProgress()({loadedBytes: 750} as TransferProgressEvent) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(750) @@ -47,7 +47,7 @@ test('upload progress tracked correctly', () => { expect(progress.getTransferredBytes()).toBe(750) expect(progress.isDone()).toBe(false) - progress.onProgress()({ loadedBytes: 1000 } as TransferProgressEvent) + progress.onProgress()({loadedBytes: 1000} as TransferProgressEvent) expect(progress.contentLength).toBe(1000) expect(progress.sentBytes).toBe(1000) From 792ec716de29f031413f3e4f810142cd223f1773 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 07:32:33 -0800 Subject: [PATCH 27/32] Tune upload options --- packages/cache/__tests__/options.test.ts | 41 ++++++++++++++++++++---- packages/cache/src/options.ts | 20 ++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/packages/cache/__tests__/options.test.ts b/packages/cache/__tests__/options.test.ts index fd742487..b4c5a1f1 100644 --- a/packages/cache/__tests__/options.test.ts +++ b/packages/cache/__tests__/options.test.ts @@ -11,8 +11,6 @@ const downloadConcurrency = 8 const timeoutInMs = 30000 const segmentTimeoutInMs = 600000 const lookupOnly = false -const uploadConcurrency = 4 -const uploadChunkSize = 32 * 1024 * 1024 test('getDownloadOptions sets defaults', async () => { const actualOptions = getDownloadOptions() @@ -43,13 +41,14 @@ test('getDownloadOptions overrides all settings', async () => { }) test('getUploadOptions sets defaults', async () => { + const expectedOptions: UploadOptions = { + uploadConcurrency: 4, + uploadChunkSize: 32 * 1024 * 1024, + useAzureSdk: false + } const actualOptions = getUploadOptions() - expect(actualOptions).toEqual({ - uploadConcurrency, - uploadChunkSize, - useAzureSdk - }) + expect(actualOptions).toEqual(expectedOptions) }) test('getUploadOptions overrides all settings', async () => { @@ -64,6 +63,34 @@ test('getUploadOptions overrides all settings', async () => { expect(actualOptions).toEqual(expectedOptions) }) +test('env variables override all getUploadOptions settings', async () => { + const expectedOptions: UploadOptions = { + uploadConcurrency: 16, + uploadChunkSize: 64 * 1024 * 1024, + useAzureSdk: true + } + + process.env.CACHE_UPLOAD_CONCURRENCY = '16' + process.env.CACHE_UPLOAD_CHUNK_SIZE = '64' + + const actualOptions = getUploadOptions(expectedOptions) + expect(actualOptions).toEqual(expectedOptions) +}) + +test('env variables override all getUploadOptions settings but do not exceed caps', async () => { + const expectedOptions: UploadOptions = { + uploadConcurrency: 32, + uploadChunkSize: 128 * 1024 * 1024, + useAzureSdk: true + } + + process.env.CACHE_UPLOAD_CONCURRENCY = '64' + process.env.CACHE_UPLOAD_CHUNK_SIZE = '256' + + const actualOptions = getUploadOptions(expectedOptions) + expect(actualOptions).toEqual(expectedOptions) +}) + test('getDownloadOptions overrides download timeout minutes', async () => { const expectedOptions: DownloadOptions = { useAzureSdk: false, diff --git a/packages/cache/src/options.ts b/packages/cache/src/options.ts index 08e71c10..3e4063f2 100644 --- a/packages/cache/src/options.ts +++ b/packages/cache/src/options.ts @@ -88,6 +88,7 @@ export interface DownloadOptions { * @param copy the original upload options */ export function getUploadOptions(copy?: UploadOptions): UploadOptions { + // Defaults if not overriden const result: UploadOptions = { useAzureSdk: false, uploadConcurrency: 4, @@ -108,6 +109,25 @@ export function getUploadOptions(copy?: UploadOptions): UploadOptions { } } + /** + * Add env var overrides + */ + // Cap the uploadConcurrency at 32 + result.uploadConcurrency = !isNaN( + Number(process.env['CACHE_UPLOAD_CONCURRENCY']) + ) + ? Math.min(32, Number(process.env['CACHE_UPLOAD_CONCURRENCY'])) + : result.uploadConcurrency + // Cap the uploadChunkSize at 128MiB + result.uploadChunkSize = !isNaN( + Number(process.env['CACHE_UPLOAD_CHUNK_SIZE']) + ) + ? Math.min( + 128 * 1024 * 1024, + Number(process.env['CACHE_UPLOAD_CHUNK_SIZE']) * 1024 * 1024 + ) + : result.uploadChunkSize + core.debug(`Use Azure SDK: ${result.useAzureSdk}`) core.debug(`Upload concurrency: ${result.uploadConcurrency}`) core.debug(`Upload chunk size: ${result.uploadChunkSize}`) From b24632bd8043752827cc8295ef756969acf9ae21 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 19:46:11 +0100 Subject: [PATCH 28/32] Fix comments Co-authored-by: Josh Gross --- packages/cache/src/cache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index f94ccc6b..2959cc62 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -110,7 +110,7 @@ export async function restoreCache( * @param primaryKey an explicit key for restoring the cache * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for key * @param options cache download options - * @param enableCrossOsArchive an optional boolean enabled to restore on windows any cache created on any platform + * @param enableCrossOsArchive an optional boolean enabled to restore on Windows any cache created on any platform * @returns string returns the key for the cache hit, otherwise returns undefined */ async function restoreCacheV1( From 3f7df8ec5a47cccf436f2f782b98daa012db818b Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 19:46:18 +0100 Subject: [PATCH 29/32] Fix comments Co-authored-by: Josh Gross --- packages/cache/src/cache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 2959cc62..6b79be52 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -108,7 +108,7 @@ export async function restoreCache( * * @param paths a list of file paths to restore from the cache * @param primaryKey an explicit key for restoring the cache - * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for key + * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for primaryKey * @param options cache download options * @param enableCrossOsArchive an optional boolean enabled to restore on Windows any cache created on any platform * @returns string returns the key for the cache hit, otherwise returns undefined From 502e8ce6515cdb6d68920039301ee221781bb97a Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 10:53:29 -0800 Subject: [PATCH 30/32] Minor comment adjustments --- packages/cache/src/cache.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index f94ccc6b..f3724c20 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -64,7 +64,7 @@ export function isFeatureAvailable(): boolean { * Restores cache from keys * * @param paths a list of file paths to restore from the cache - * @param primaryKey an explicit key for restoring the cache + * @param primaryKey an explicit key for restoring the cache. Lookup is done with prefix matching. * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for key * @param downloadOptions cache download options * @param enableCrossOsArchive an optional boolean enabled to restore on windows any cache created on any platform @@ -107,7 +107,7 @@ export async function restoreCache( * Restores cache using the legacy Cache Service * * @param paths a list of file paths to restore from the cache - * @param primaryKey an explicit key for restoring the cache + * @param primaryKey an explicit key for restoring the cache. Lookup is done with prefix matching. * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for key * @param options cache download options * @param enableCrossOsArchive an optional boolean enabled to restore on windows any cache created on any platform @@ -205,7 +205,7 @@ async function restoreCacheV1( * Restores cache using Cache Service v2 * * @param paths a list of file paths to restore from the cache - * @param primaryKey an explicit key for restoring the cache + * @param primaryKey an explicit key for restoring the cache. Lookup is done with prefix matching * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for key * @param downloadOptions cache download options * @param enableCrossOsArchive an optional boolean enabled to restore on windows any cache created on any platform @@ -463,6 +463,8 @@ async function saveCacheV2( enableCrossOsArchive = false ): Promise { // Override UploadOptions to force the use of Azure + // ...options goes first because we want to override the default values + // set in UploadOptions with these specific figures options = { ...options, uploadChunkSize: 64 * 1024 * 1024, // 64 MiB From c649df4b940f300d98c8a43f0d37aa626be1f282 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 10:55:33 -0800 Subject: [PATCH 31/32] Minor comment adjustments --- packages/cache/src/cache.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index e4b53faf..8eb69044 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -65,7 +65,7 @@ export function isFeatureAvailable(): boolean { * * @param paths a list of file paths to restore from the cache * @param primaryKey an explicit key for restoring the cache. Lookup is done with prefix matching. - * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for key + * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for primaryKey * @param downloadOptions cache download options * @param enableCrossOsArchive an optional boolean enabled to restore on windows any cache created on any platform * @returns string returns the key for the cache hit, otherwise returns undefined @@ -206,7 +206,7 @@ async function restoreCacheV1( * * @param paths a list of file paths to restore from the cache * @param primaryKey an explicit key for restoring the cache. Lookup is done with prefix matching - * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for key + * @param restoreKeys an optional ordered list of keys to use for restoring the cache if no cache hit occurred for primaryKey * @param downloadOptions cache download options * @param enableCrossOsArchive an optional boolean enabled to restore on windows any cache created on any platform * @returns string returns the key for the cache hit, otherwise returns undefined From c02c929c562af2419e21be2bdff6fe8c257c7e02 Mon Sep 17 00:00:00 2001 From: Bassem Dghaidi <568794+Link-@users.noreply.github.com> Date: Mon, 2 Dec 2024 11:10:25 -0800 Subject: [PATCH 32/32] Minor comment adjustments --- packages/cache/src/cache.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 8eb69044..9b02489f 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -3,16 +3,16 @@ 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 { getCacheServiceVersion, isGhes } from './internal/config' -import { DownloadOptions, UploadOptions } from './options' -import { createTar, extractTar, listTar } from './internal/tar' +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 {CacheFileSizeLimit} from './internal/constants' export class ValidationError extends Error { constructor(message: string) { super(message) @@ -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(