From 6dba5e03809a275c6659adbe85c0f4fd0f4b22a1 Mon Sep 17 00:00:00 2001 From: Jesse MacFadyen Date: Wed, 29 Oct 2025 22:39:42 -0700 Subject: [PATCH 1/4] wip: allow setting cache control headers --- lib/remote-storage.js | 27 +++++++++++++++++++-------- package.json | 2 +- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/remote-storage.js b/lib/remote-storage.js index 0481539..ad5c4c4 100644 --- a/lib/remote-storage.js +++ b/lib/remote-storage.js @@ -135,6 +135,7 @@ module.exports = class RemoteStorage { const content = await fs.readFile(file) const mimeType = mime.lookup(path.extname(file)) const cacheControlString = this._getCacheControlConfig(mimeType, appConfig.app) + console.log('cacheControlString is', cacheControlString) // --- IGNORE --- const uploadParams = { Bucket: this.bucket, Key: urlJoin(prefix, path.basename(file)), @@ -143,14 +144,21 @@ module.exports = class RemoteStorage { } // add response headers if specified in manifest const responseHeaders = this.getResponseHeadersForFile(file, distRoot, appConfig) - if (responseHeaders) { - uploadParams.Metadata = responseHeaders + // here we allow overriding the cache control if specified in response headers + // this is considered more specific than the general cache control config + // ideally we deprecate cache control config in favor of response headers directly + if(responseHeaders['adp-cache-control']) { + uploadParams.CacheControl = responseHeaders['adp-cache-control'] + delete responseHeaders['adp-cache-control'] } + uploadParams.Metadata = responseHeaders ?? {} + + uploadParams.Metadata['adp-AuditUserId'] = appConfig.auditUserId // s3 misses some mime types like for css files if (mimeType) { uploadParams.ContentType = mimeType } - + console.log('Metadata is', uploadParams.Metadata) // --- IGNORE --- // Note: putObject is recommended for files < 100MB and has a limit of 5GB, which is ok for our use case of storing static web assets // if we intend to store larger files, we should use multipart upload and https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/modules/_aws_sdk_lib_storage.html return this.s3.putObject(uploadParams) @@ -183,7 +191,7 @@ module.exports = class RemoteStorage { if (folderPathToMatch.endsWith(path.sep)) { folderPathToMatch = folderPathToMatch.substring(0, folderPathToMatch.length - 1) // remove any trailing path separator } - + // is there a wildcard matching lib we can use for this? -jm minimatch? if (rule === '/*') { // all content return true } else if (rule.endsWith('/*')) { // all content in a folder ex. /test/* @@ -278,17 +286,20 @@ module.exports = class RemoteStorage { * @param {Object} appConfig - application config */ _getCacheControlConfig (mimeType, appConfig) { - const cacheControlStr = 's-maxage=0' + const cacheControlStr = '' if (!mimeType) { - return cacheControlStr + return null } else if (mimeType === mime.lookup('html')) { + console.log('html cache duration is', appConfig.htmlCacheDuration) // --- IGNORE --- return cacheControlStr + ', max-age=' + appConfig.htmlCacheDuration } else if (mimeType === mime.lookup('js')) { return cacheControlStr + ', max-age=' + appConfig.jsCacheDuration } else if (mimeType === mime.lookup('css')) { return cacheControlStr + ', max-age=' + appConfig.cssCacheDuration - } else if (mimeType.startsWith('image')) { + } else if (false && mimeType.startsWith('image')) { return cacheControlStr + ', max-age=' + appConfig.imageCacheDuration - } else { return cacheControlStr } + } else { + return null + } } } diff --git a/package.json b/package.json index baa5944..c4521d2 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "klaw": "^4", "lodash.clonedeep": "^4.5.0", "mime-types": "^2.1.24", - "parcel": "^2.7.0", + "parcel": "^2.15.4", "regenerator-runtime": "^0.13.7" }, "devDependencies": { From ea2a8c7f497e0d8823fd4d8d25f6315355905f68 Mon Sep 17 00:00:00 2001 From: Jesse MacFadyen Date: Wed, 12 Nov 2025 20:56:51 -0800 Subject: [PATCH 2/4] clean up console.log, default s-max-age=60 --- lib/remote-storage.js | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/remote-storage.js b/lib/remote-storage.js index ad5c4c4..f3f8253 100644 --- a/lib/remote-storage.js +++ b/lib/remote-storage.js @@ -134,20 +134,23 @@ module.exports = class RemoteStorage { } const content = await fs.readFile(file) const mimeType = mime.lookup(path.extname(file)) + // first we will grab it from the global config: htmlCacheDuration, etc. const cacheControlString = this._getCacheControlConfig(mimeType, appConfig.app) - console.log('cacheControlString is', cacheControlString) // --- IGNORE --- const uploadParams = { Bucket: this.bucket, Key: urlJoin(prefix, path.basename(file)), - Body: content, - CacheControl: cacheControlString + Body: content + } + // if we found it in the global config, we will use it ( for now ) + if(cacheControlString) { + uploadParams.CacheControl = cacheControlString } // add response headers if specified in manifest const responseHeaders = this.getResponseHeadersForFile(file, distRoot, appConfig) // here we allow overriding the cache control if specified in response headers // this is considered more specific than the general cache control config // ideally we deprecate cache control config in favor of response headers directly - if(responseHeaders['adp-cache-control']) { + if(responseHeaders?.['adp-cache-control']) { uploadParams.CacheControl = responseHeaders['adp-cache-control'] delete responseHeaders['adp-cache-control'] } @@ -158,7 +161,6 @@ module.exports = class RemoteStorage { if (mimeType) { uploadParams.ContentType = mimeType } - console.log('Metadata is', uploadParams.Metadata) // --- IGNORE --- // Note: putObject is recommended for files < 100MB and has a limit of 5GB, which is ok for our use case of storing static web assets // if we intend to store larger files, we should use multipart upload and https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/modules/_aws_sdk_lib_storage.html return this.s3.putObject(uploadParams) @@ -183,6 +185,13 @@ module.exports = class RemoteStorage { return responseHeaders } + /** + * Checks if a header can be added to a file based on the rule + * @param {string} file - file path + * @param {string} distRoot - distribution root + * @param {string} rule - rule to check + * @returns {boolean} true if header can be added, false otherwise + */ canAddHeader (file, distRoot, rule) { const filePath = path.parse(file) const normalisedRule = rule.replace(/\//g, path.sep) @@ -191,7 +200,6 @@ module.exports = class RemoteStorage { if (folderPathToMatch.endsWith(path.sep)) { folderPathToMatch = folderPathToMatch.substring(0, folderPathToMatch.length - 1) // remove any trailing path separator } - // is there a wildcard matching lib we can use for this? -jm minimatch? if (rule === '/*') { // all content return true } else if (rule.endsWith('/*')) { // all content in a folder ex. /test/* @@ -286,18 +294,17 @@ module.exports = class RemoteStorage { * @param {Object} appConfig - application config */ _getCacheControlConfig (mimeType, appConfig) { - const cacheControlStr = '' + const cacheControlStr = 's-maxage=60' if (!mimeType) { return null } else if (mimeType === mime.lookup('html')) { - console.log('html cache duration is', appConfig.htmlCacheDuration) // --- IGNORE --- - return cacheControlStr + ', max-age=' + appConfig.htmlCacheDuration + return `${cacheControlStr}, max-age=${appConfig.htmlCacheDuration}` } else if (mimeType === mime.lookup('js')) { - return cacheControlStr + ', max-age=' + appConfig.jsCacheDuration + return `${cacheControlStr}, max-age=${appConfig.jsCacheDuration}` } else if (mimeType === mime.lookup('css')) { - return cacheControlStr + ', max-age=' + appConfig.cssCacheDuration - } else if (false && mimeType.startsWith('image')) { - return cacheControlStr + ', max-age=' + appConfig.imageCacheDuration + return `${cacheControlStr}, max-age=${appConfig.cssCacheDuration}` + } else if (mimeType.startsWith('image')) { + return `${cacheControlStr}, max-age=${appConfig.imageCacheDuration}` } else { return null } From fa91c5642dbfc2a2481fbaeaf2626c64890e29a9 Mon Sep 17 00:00:00 2001 From: Jesse MacFadyen Date: Thu, 13 Nov 2025 12:03:06 -0800 Subject: [PATCH 3/4] tests & linting, updated eslintrc to 2020 for optional chaining --- .eslintrc | 2 +- lib/remote-storage.js | 4 +-- test/lib/remote-storage.test.js | 43 +++++++++++++++++++++++++++++---- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/.eslintrc b/.eslintrc index 5ad8bcd..7e41429 100644 --- a/.eslintrc +++ b/.eslintrc @@ -6,6 +6,6 @@ "r": "readonly" }, "parserOptions": { - "ecmaVersion": 2018 + "ecmaVersion": 2020 } } diff --git a/lib/remote-storage.js b/lib/remote-storage.js index 688e585..90bfcdb 100644 --- a/lib/remote-storage.js +++ b/lib/remote-storage.js @@ -157,7 +157,7 @@ module.exports = class RemoteStorage { Body: content } // if we found it in the global config, we will use it ( for now ) - if(cacheControlString) { + if (cacheControlString) { uploadParams.CacheControl = cacheControlString } // add response headers if specified in manifest @@ -165,7 +165,7 @@ module.exports = class RemoteStorage { // here we allow overriding the cache control if specified in response headers // this is considered more specific than the general cache control config // ideally we deprecate cache control config in favor of response headers directly - if(responseHeaders?.['adp-cache-control']) { + if (responseHeaders?.['adp-cache-control']) { uploadParams.CacheControl = responseHeaders['adp-cache-control'] delete responseHeaders['adp-cache-control'] } diff --git a/test/lib/remote-storage.test.js b/test/lib/remote-storage.test.js index abae930..6c4f045 100644 --- a/test/lib/remote-storage.test.js +++ b/test/lib/remote-storage.test.js @@ -300,31 +300,31 @@ describe('RemoteStorage', () => { test('cachecontrol string for html', async () => { const rs = new RemoteStorage(global.fakeTVMResponse) const response = rs._getCacheControlConfig('text/html', global.fakeConfig.app) - expect(response).toBe('s-maxage=0, max-age=60') + expect(response).toBe('s-maxage=60, max-age=60') }) test('cachecontrol string for JS', async () => { const rs = new RemoteStorage(global.fakeTVMResponse) const response = rs._getCacheControlConfig('application/javascript', global.fakeConfig.app) - expect(response).toBe('s-maxage=0, max-age=604800') + expect(response).toBe('s-maxage=60, max-age=604800') }) test('cachecontrol string for CSS', async () => { const rs = new RemoteStorage(global.fakeTVMResponse) const response = rs._getCacheControlConfig('text/css', global.fakeConfig.app) - expect(response).toBe('s-maxage=0, max-age=604800') + expect(response).toBe('s-maxage=60, max-age=604800') }) test('cachecontrol string for Image', async () => { const rs = new RemoteStorage(global.fakeTVMResponse) const response = rs._getCacheControlConfig('image/jpeg', global.fakeConfig.app) - expect(response).toBe('s-maxage=0, max-age=604800') + expect(response).toBe('s-maxage=60, max-age=604800') }) test('cachecontrol string for default', async () => { const rs = new RemoteStorage(global.fakeTVMResponse) const response = rs._getCacheControlConfig('application/pdf', global.fakeConfig.app) - expect(response).toBe('s-maxage=0') + expect(response).toBe(null) }) // response header tests @@ -573,4 +573,37 @@ describe('RemoteStorage', () => { } expect(mockS3.putObject).toHaveBeenCalledWith(expect.objectContaining(expected)) }) + + test('Cache control override from response headers', async () => { + global.addFakeFiles(vol, 'fakeDir', { 'index.html': 'fake content' }) + const rs = new RemoteStorage(global.fakeTVMResponse) + const files = await rs.walkDir('fakeDir') + const fakeDistRoot = path.parse(files[0]).dir + const filePath = files[0] // Use absolute path from walkDir + const newConfig = global.configWithModifiedWeb(global.fakeConfig, { + 'response-headers': { + '/*.html': { + 'cache-control': 'max-age=3600, s-maxage=7200', + testHeader: 'generic-header' + } + } + }) + await rs.uploadFile(filePath, 'fakeprefix', newConfig, fakeDistRoot) + const body = Buffer.from('fake content', 'utf8') + const expected = { + Bucket: 'fake-bucket', + Key: 'fakeprefix/index.html', + Body: body, + ContentType: 'text/html', + CacheControl: 'max-age=3600, s-maxage=7200', + Metadata: { + 'adp-testHeader': 'generic-header', + 'adp-AuditUserId': undefined + } + } + expect(mockS3.putObject).toHaveBeenCalledWith(expect.objectContaining(expected)) + // Verify that adp-cache-control was removed from metadata + const putObjectCall = mockS3.putObject.mock.calls[0][0] + expect(putObjectCall.Metadata).not.toHaveProperty('adp-cache-control') + }) }) From 9edb03b594e2f206443e89971f04ee4691fe02ad Mon Sep 17 00:00:00 2001 From: Jesse MacFadyen Date: Thu, 13 Nov 2025 13:05:38 -0800 Subject: [PATCH 4/4] more tests, upload metadata not set if empty --- lib/remote-storage.js | 12 +++++-- test/lib/remote-storage.test.js | 60 +++++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/lib/remote-storage.js b/lib/remote-storage.js index 90bfcdb..f5741ce 100644 --- a/lib/remote-storage.js +++ b/lib/remote-storage.js @@ -161,7 +161,7 @@ module.exports = class RemoteStorage { uploadParams.CacheControl = cacheControlString } // add response headers if specified in manifest - const responseHeaders = this.getResponseHeadersForFile(file, distRoot, appConfig) + const responseHeaders = this.getResponseHeadersForFile(file, distRoot, appConfig) ?? {} // here we allow overriding the cache control if specified in response headers // this is considered more specific than the general cache control config // ideally we deprecate cache control config in favor of response headers directly @@ -169,9 +169,15 @@ module.exports = class RemoteStorage { uploadParams.CacheControl = responseHeaders['adp-cache-control'] delete responseHeaders['adp-cache-control'] } - uploadParams.Metadata = responseHeaders ?? {} - uploadParams.Metadata['adp-AuditUserId'] = appConfig.auditUserId + if (appConfig.auditUserId) { + responseHeaders['adp-AuditUserId'] = appConfig.auditUserId + } + // we only set metadata if we have added anything to responseHeaders object + // it is not null, but could be empty + if (Object.keys(responseHeaders).length > 0) { + uploadParams.Metadata = responseHeaders + } // s3 misses some mime types like for css files if (mimeType) { uploadParams.ContentType = mimeType diff --git a/test/lib/remote-storage.test.js b/test/lib/remote-storage.test.js index 6c4f045..2a72b72 100644 --- a/test/lib/remote-storage.test.js +++ b/test/lib/remote-storage.test.js @@ -597,8 +597,7 @@ describe('RemoteStorage', () => { ContentType: 'text/html', CacheControl: 'max-age=3600, s-maxage=7200', Metadata: { - 'adp-testHeader': 'generic-header', - 'adp-AuditUserId': undefined + 'adp-testHeader': 'generic-header' } } expect(mockS3.putObject).toHaveBeenCalledWith(expect.objectContaining(expected)) @@ -606,4 +605,61 @@ describe('RemoteStorage', () => { const putObjectCall = mockS3.putObject.mock.calls[0][0] expect(putObjectCall.Metadata).not.toHaveProperty('adp-cache-control') }) + + test('uploadFile includes auditUserId in metadata when set', async () => { + global.addFakeFiles(vol, 'fakeDir', { 'index.js': 'fake content' }) + const rs = new RemoteStorage(global.fakeTVMResponse) + const fakeConfig = { ...global.fakeConfig, auditUserId: 'test-user-123' } + await rs.uploadFile('fakeDir/index.js', 'fakeprefix', fakeConfig, 'fakeDir') + const body = Buffer.from('fake content', 'utf8') + const expected = { + Bucket: 'fake-bucket', + Key: 'fakeprefix/index.js', + Body: body, + ContentType: 'application/javascript', + Metadata: expect.objectContaining({ + 'adp-AuditUserId': 'test-user-123' + }) + } + expect(mockS3.putObject).toHaveBeenCalledWith(expect.objectContaining(expected)) + }) + + test('uploadFile does not set Metadata when responseHeaders is empty and auditUserId is not set', async () => { + global.addFakeFiles(vol, 'fakeDir', { 'index.js': 'fake content' }) + const rs = new RemoteStorage(global.fakeTVMResponse) + const fakeConfig = { + app: global.fakeConfig.app + // No web.response-headers and no auditUserId + } + await rs.uploadFile('fakeDir/index.js', 'fakeprefix', fakeConfig, 'fakeDir') + const body = Buffer.from('fake content', 'utf8') + const putObjectCall = mockS3.putObject.mock.calls[0][0] + expect(putObjectCall).not.toHaveProperty('Metadata') + expect(putObjectCall).toMatchObject({ + Bucket: 'fake-bucket', + Key: 'fakeprefix/index.js', + Body: body, + ContentType: 'application/javascript' + }) + }) + + test('uploadFile sets CacheControl even when responseHeaders is empty and auditUserId is not set', async () => { + global.addFakeFiles(vol, 'fakeDir', { 'index.html': 'fake content' }) + const rs = new RemoteStorage(global.fakeTVMResponse) + const fakeConfig = { + app: global.fakeConfig.app + // No web.response-headers and no auditUserId + } + await rs.uploadFile('fakeDir/index.html', 'fakeprefix', fakeConfig, 'fakeDir') + const body = Buffer.from('fake content', 'utf8') + const putObjectCall = mockS3.putObject.mock.calls[0][0] + expect(putObjectCall).not.toHaveProperty('Metadata') + expect(putObjectCall).toMatchObject({ + Bucket: 'fake-bucket', + Key: 'fakeprefix/index.html', + Body: body, + ContentType: 'text/html', + CacheControl: 's-maxage=60, max-age=60' + }) + }) })