From 0746965b72cb756703c67fe331ecd1e745301827 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Tue, 3 Jan 2023 08:31:27 +0000 Subject: [PATCH] Address review comments --- .../cache/__tests__/cacheHttpClient.test.ts | 2 +- packages/cache/__tests__/restoreCache.test.ts | 8 ++--- packages/cache/__tests__/saveCache.test.ts | 6 ++-- packages/cache/package.json | 2 +- packages/cache/src/cache.ts | 34 +++++-------------- .../cache/src/internal/cacheHttpClient.ts | 16 ++++----- packages/cache/src/internal/contracts.d.ts | 2 +- 7 files changed, 24 insertions(+), 46 deletions(-) diff --git a/packages/cache/__tests__/cacheHttpClient.test.ts b/packages/cache/__tests__/cacheHttpClient.test.ts index efd859b8..5112c1a7 100644 --- a/packages/cache/__tests__/cacheHttpClient.test.ts +++ b/packages/cache/__tests__/cacheHttpClient.test.ts @@ -39,7 +39,7 @@ test('getCacheVersion with gzip compression does not change vesion', async () => ) }) -test('getCacheVersion with crossOsEnabled as false returns version on windows', async () => { +test('getCacheVersion with enableCrossOsArchive as false returns version on windows', async () => { if (process.platform === 'win32') { const paths = ['node_modules'] const result = getCacheVersion(paths) diff --git a/packages/cache/__tests__/restoreCache.test.ts b/packages/cache/__tests__/restoreCache.test.ts index 2b335686..996dd23e 100644 --- a/packages/cache/__tests__/restoreCache.test.ts +++ b/packages/cache/__tests__/restoreCache.test.ts @@ -143,7 +143,7 @@ test('restore with gzip compressed cache found', async () => { expect(cacheKey).toBe(key) expect(getCacheMock).toHaveBeenCalledWith([key], paths, { compressionMethod: compression, - crossOsEnabled: false + enableCrossOsArchive: false }) expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) expect(downloadCacheMock).toHaveBeenCalledWith( @@ -212,7 +212,7 @@ test('restore with zstd as default but gzip compressed cache found on windows', expect(cacheKey).toBe(key) expect(getCacheMock).toHaveBeenNthCalledWith(1, [key], paths, { compressionMethod: compression, - crossOsEnabled: false + enableCrossOsArchive: false }) expect(getCacheMock).toHaveBeenNthCalledWith(2, [key], paths, { compressionMethod: CompressionMethod.Gzip @@ -279,7 +279,7 @@ test('restore with zstd compressed cache found', async () => { expect(cacheKey).toBe(key) expect(getCacheMock).toHaveBeenCalledWith([key], paths, { compressionMethod: compression, - crossOsEnabled: false + enableCrossOsArchive: false }) expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) expect(downloadCacheMock).toHaveBeenCalledWith( @@ -337,7 +337,7 @@ test('restore with cache found for restore key', async () => { expect(cacheKey).toBe(restoreKey) expect(getCacheMock).toHaveBeenCalledWith([key, restoreKey], paths, { compressionMethod: compression, - crossOsEnabled: false + enableCrossOsArchive: false }) expect(createTempDirectoryMock).toHaveBeenCalledTimes(1) expect(downloadCacheMock).toHaveBeenCalledWith( diff --git a/packages/cache/__tests__/saveCache.test.ts b/packages/cache/__tests__/saveCache.test.ts index 3a8725eb..4d0027be 100644 --- a/packages/cache/__tests__/saveCache.test.ts +++ b/packages/cache/__tests__/saveCache.test.ts @@ -211,7 +211,7 @@ test('save with reserve cache failure should fail', async () => { expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey, paths, { cacheSize: undefined, compressionMethod: compression, - crossOsEnabled: false + enableCrossOsArchive: false }) expect(createTarMock).toHaveBeenCalledTimes(1) expect(saveCacheMock).toHaveBeenCalledTimes(0) @@ -257,7 +257,7 @@ test('save with server error should fail', async () => { expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey, [filePath], { cacheSize: undefined, compressionMethod: compression, - crossOsEnabled: false + enableCrossOsArchive: false }) const archiveFolder = '/foo/bar' const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) @@ -302,7 +302,7 @@ test('save with valid inputs uploads a cache', async () => { expect(reserveCacheMock).toHaveBeenCalledWith(primaryKey, [filePath], { cacheSize: undefined, compressionMethod: compression, - crossOsEnabled: false + enableCrossOsArchive: false }) const archiveFolder = '/foo/bar' const archiveFile = path.join(archiveFolder, CacheFilename.Zstd) diff --git a/packages/cache/package.json b/packages/cache/package.json index 44c677f8..4852c241 100644 --- a/packages/cache/package.json +++ b/packages/cache/package.json @@ -1,6 +1,6 @@ { "name": "@actions/cache", - "version": "3.1.0", + "version": "3.1.2", "preview": true, "description": "Actions cache lib", "keywords": [ diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 2c5bb75b..5180532d 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -62,7 +62,7 @@ export function isFeatureAvailable(): boolean { * @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 downloadOptions cache download options - * @param crossOsEnabled 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 */ export async function restoreCache( @@ -70,7 +70,7 @@ export async function restoreCache( primaryKey: string, restoreKeys?: string[], options?: DownloadOptions, - crossOsEnabled = false + enableCrossOsArchive = false ): Promise { checkPaths(paths) @@ -96,29 +96,11 @@ export async function restoreCache( // path are needed to compute version cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, { compressionMethod, - crossOsEnabled + enableCrossOsArchive }) if (!cacheEntry?.archiveLocation) { - // This is to support the old cache entry created by gzip on windows. - if ( - process.platform === 'win32' && - compressionMethod !== CompressionMethod.Gzip - ) { - compressionMethod = CompressionMethod.Gzip - cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, { - compressionMethod - }) - if (!cacheEntry?.archiveLocation) { - return undefined - } - - core.info( - "Couldn't find cache entry with zstd compression, falling back to gzip compression." - ) - } else { - // Cache not found - return undefined - } + // Cache not found + return undefined } archivePath = path.join( @@ -174,7 +156,7 @@ export async function restoreCache( * * @param paths a list of file paths to be cached * @param key an explicit key for restoring the cache - * @param crossOsEnabled an optional boolean enabled to save cache on windows which could be restored on any platform + * @param enableCrossOsArchive an optional boolean enabled to save cache on windows which could be restored on any platform * @param options cache upload options * @returns number returns cacheId if the cache was saved successfully and throws an error if save fails */ @@ -182,7 +164,7 @@ export async function saveCache( paths: string[], key: string, options?: UploadOptions, - crossOsEnabled = false + enableCrossOsArchive = false ): Promise { checkPaths(paths) checkKey(key) @@ -232,7 +214,7 @@ export async function saveCache( paths, { compressionMethod, - crossOsEnabled, + enableCrossOsArchive, cacheSize: archiveFileSize } ) diff --git a/packages/cache/src/internal/cacheHttpClient.ts b/packages/cache/src/internal/cacheHttpClient.ts index 46c09a4c..0f5903f2 100644 --- a/packages/cache/src/internal/cacheHttpClient.ts +++ b/packages/cache/src/internal/cacheHttpClient.ts @@ -74,17 +74,13 @@ function createHttpClient(): HttpClient { export function getCacheVersion( paths: string[], compressionMethod?: CompressionMethod, - crossOsEnabled = false + enableCrossOsArchive = false ): string { const components = paths + .concat([!compressionMethod ? '' : compressionMethod]) .concat( - !compressionMethod || compressionMethod === CompressionMethod.Gzip - ? [] - : [compressionMethod] - ) - .concat( - process.platform !== 'win32' || crossOsEnabled ? [] : ['windows-only'] - ) // Only check for windows platforms if crossOsEnabled is false + process.platform !== 'win32' || enableCrossOsArchive ? [] : ['windows-only'] + ) // Only check for windows platforms if enableCrossOsArchive is false // Add salt to cache version to support breaking changes in cache entry components.push(versionSalt) @@ -104,7 +100,7 @@ export async function getCacheEntry( const version = getCacheVersion( paths, options?.compressionMethod, - options?.crossOsEnabled + options?.enableCrossOsArchive ) const resource = `cache?keys=${encodeURIComponent( keys.join(',') @@ -193,7 +189,7 @@ export async function reserveCache( const version = getCacheVersion( paths, options?.compressionMethod, - options?.crossOsEnabled + options?.enableCrossOsArchive ) const reserveCacheRequest: ReserveCacheRequest = { diff --git a/packages/cache/src/internal/contracts.d.ts b/packages/cache/src/internal/contracts.d.ts index 9557218b..6fcd9427 100644 --- a/packages/cache/src/internal/contracts.d.ts +++ b/packages/cache/src/internal/contracts.d.ts @@ -35,7 +35,7 @@ export interface ReserveCacheResponse { export interface InternalCacheOptions { compressionMethod?: CompressionMethod - crossOsEnabled?: boolean + enableCrossOsArchive?: boolean cacheSize?: number }