From 8a13cfcbdee4d14322c64eb9d9b92b816604ba8c Mon Sep 17 00:00:00 2001 From: John Yun Date: Tue, 17 Dec 2024 11:27:52 -0800 Subject: [PATCH 1/3] add endPoint implementation when the driver makes a request with Axios --- lib/file_transfer_agent/gcs_util.js | 58 +++++++++++++---------- test/unit/file_transfer_agent/gcs_test.js | 7 ++- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/lib/file_transfer_agent/gcs_util.js b/lib/file_transfer_agent/gcs_util.js index fc46557ac..7e767aa27 100644 --- a/lib/file_transfer_agent/gcs_util.js +++ b/lib/file_transfer_agent/gcs_util.js @@ -26,7 +26,11 @@ const EXPIRED_TOKEN = 'ExpiredToken'; const ERRORNO_WSAECONNABORTED = 10053; // network connection was aborted -// GCS Location +/** + * @typedef {object} GCSLocation + * @property {string} bucketName + * @property {string} path + */ function GCSLocation(bucketName, path) { return { 'bucketName': bucketName, @@ -69,14 +73,7 @@ function GCSUtil(httpclient, filestream) { } }); - //TODO: SNOW-1789759 hardcoded region will be replaced in the future - const isRegionalUrlEnabled = (stageInfo.region).toLowerCase() === 'me-central2' || stageInfo.useRegionalUrl; - let endPoint = null; - if (stageInfo['endPoint']) { - endPoint = stageInfo['endPoint']; - } else if (isRegionalUrlEnabled) { - endPoint = `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com`; - } + const endPoint = this.getGCSCustomEndPoint(stageInfo); const storage = endPoint ? new Storage({ interceptors_: interceptors, apiEndpoint: endPoint }) : new Storage({ interceptors_: interceptors }); client = { gcsToken: gcsToken, gcsClient: storage }; } else { @@ -91,7 +88,7 @@ function GCSUtil(httpclient, filestream) { * * @param {String} stageLocation * - * @returns {Object} + * @returns {GCSLocation} */ this.extractBucketNameAndPath = function (stageLocation) { let containerName = stageLocation; @@ -135,7 +132,7 @@ function GCSUtil(httpclient, filestream) { } }); } else { - const url = this.generateFileURL(meta['stageInfo']['location'], lstrip(filename, '/')); + const url = this.generateFileURL(meta.stageInfo, lstrip(filename, '/')); const accessToken = meta['client'].gcsToken; const gcsHeaders = { 'Authorization': `Bearer ${accessToken}` }; let encryptionMetadata; @@ -239,7 +236,7 @@ function GCSUtil(httpclient, filestream) { if (!uploadUrl) { const tempFilename = meta['dstFileName'].substring(meta['dstFileName'].indexOf('/') + 1, meta['dstFileName'].length); - uploadUrl = this.generateFileURL(meta['stageInfo']['location'], tempFilename); + uploadUrl = this.generateFileURL(meta.stageInfo, tempFilename); accessToken = meta['client'].gcsToken; } let contentEncoding = ''; @@ -346,7 +343,7 @@ function GCSUtil(httpclient, filestream) { if (!downloadUrl) { downloadUrl = this.generateFileURL( - meta.stageInfo['location'], lstrip(meta['srcFileName'], '/') + meta.stageInfo, lstrip(meta['srcFileName'], '/') ); accessToken = meta['client'].gcsToken; gcsHeaders = { 'Authorization': `Bearer ${accessToken}` }; @@ -445,26 +442,39 @@ function GCSUtil(httpclient, filestream) { /** * Generate file URL based on bucket. * - * @param {String} stageLocation + * @param {Object} stageInfo * @param {String} filename * * @returns {String} */ - this.generateFileURL = function (stageLocation, filename) { - const gcsLocation = this.extractBucketNameAndPath(stageLocation); + this.generateFileURL = function (stageInfo, filename) { + const gcsLocation = this.extractBucketNameAndPath(stageInfo.location); const fullFilePath = `${gcsLocation.path}${filename}`; - const link = 'https://storage.googleapis.com/' + gcsLocation.bucketName + '/' + fullFilePath; + const endPoint = this.getGCSCustomEndPoint(stageInfo); + const link = `${endPoint != null ? endPoint : 'https://storage.googleapis.com'}/${gcsLocation.bucketName}/${fullFilePath}`; return link; }; + this.getGCSCustomEndPoint = function (stageInfo) { + //TODO: SNOW-1789759 hardcoded region will be replaced in the future + const isRegionalUrlEnabled = (stageInfo.region).toLowerCase() === 'me-central2' || stageInfo.useRegionalUrl; + let endPoint = null; + if (stageInfo['endPoint']) { + endPoint = stageInfo['endPoint']; + } else if (isRegionalUrlEnabled) { + endPoint = `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com`; + } + return endPoint; + }; + /** -* Left strip the specified character from a string. -* -* @param {String} str -* @param {Character} remove -* -* @returns {String} -*/ + * Left strip the specified character from a string. + * + * @param {String} str + * @param {Character} remove + * + * @returns {String} + */ function lstrip(str, remove) { while (str.length > 0 && remove.indexOf(str.charAt(0)) !== -1) { str = str.substr(1); diff --git a/test/unit/file_transfer_agent/gcs_test.js b/test/unit/file_transfer_agent/gcs_test.js index 6ce0c84d9..fd2ff96ad 100644 --- a/test/unit/file_transfer_agent/gcs_test.js +++ b/test/unit/file_transfer_agent/gcs_test.js @@ -34,7 +34,10 @@ describe('GCS client', function () { meta = { stageInfo: { location: mockLocation, - path: mockTable + '/' + mockPath + '/' + path: mockTable + '/' + mockPath + '/', + endPoint: null, + useRegionalUrl: false, + region: 'mockLocation', }, presignedUrl: mockPresignedUrl, dstFileName: mockPresignedUrl, @@ -133,7 +136,7 @@ describe('GCS client', function () { testCases.forEach(({ name, stageInfo, result }) => { it(name, () => { - const client = GCS.createClient({ ...stageInfo, ...meta.stageInfo, creds: { GCS_ACCESS_TOKEN: 'mockToken' } }); + const client = GCS.createClient({ ...meta.stageInfo, ...stageInfo, creds: { GCS_ACCESS_TOKEN: 'mockToken' } }); assert.strictEqual(client.gcsClient.apiEndpoint, result); } ); From 9077c7ebee96405fb00a43e27dfb02eb49061a74 Mon Sep 17 00:00:00 2001 From: John Yun Date: Thu, 19 Dec 2024 09:39:41 -0800 Subject: [PATCH 2/3] add testing --- lib/file_transfer_agent/gcs_util.js | 17 +--------------- lib/util.js | 17 +++++++++++++++- test/unit/util_test.js | 30 +++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/lib/file_transfer_agent/gcs_util.js b/lib/file_transfer_agent/gcs_util.js index 7e767aa27..c565a204d 100644 --- a/lib/file_transfer_agent/gcs_util.js +++ b/lib/file_transfer_agent/gcs_util.js @@ -4,7 +4,7 @@ const EncryptionMetadata = require('./encrypt_util').EncryptionMetadata; const FileHeader = require('./file_util').FileHeader; -const { shouldPerformGCPBucket } = require('../util'); +const { shouldPerformGCPBucket, lstrip } = require('../util'); const GCS_METADATA_PREFIX = 'x-goog-meta-'; const SFC_DIGEST = 'sfc-digest'; @@ -466,21 +466,6 @@ function GCSUtil(httpclient, filestream) { } return endPoint; }; - - /** - * Left strip the specified character from a string. - * - * @param {String} str - * @param {Character} remove - * - * @returns {String} - */ - function lstrip(str, remove) { - while (str.length > 0 && remove.indexOf(str.charAt(0)) !== -1) { - str = str.substr(1); - } - return str; - } } module.exports = GCSUtil; diff --git a/lib/util.js b/lib/util.js index cc22322d3..5359e902a 100644 --- a/lib/util.js +++ b/lib/util.js @@ -753,4 +753,19 @@ exports.isNotEmptyAsString = function (variable) { exports.isWindows = function () { return os.platform() === 'win32'; -}; \ No newline at end of file +}; + +/** +* Left strip the specified character from a string. +* +* @param {String} str +* @param {Character} remove +* +* @returns {String} +*/ +exports.lstrip = function(str, remove) { + while (str.length > 0 && remove.indexOf(str.charAt(0)) !== -1) { + str = str.substr(1); + } + return str; +} \ No newline at end of file diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 967c424ad..6a058dbbf 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -7,6 +7,7 @@ const assert = require('assert'); const path = require('path'); const fsPromises = require('fs/promises'); const os = require('os'); +const { lstat } = require('fs'); describe('Util', function () { it('Util.isFunction()', function () { @@ -1215,4 +1216,33 @@ describe('Util', function () { } }); + describe('lstrip function Test', function () { + const testCases = [ + { + name: 'remove consecutive characters /', + str: '///////////helloworld', + remove: '/', + result: 'helloworld' + }, + { + name: 'when the first character is not matched with the remove character', + str: '/\\/\\helloworld', + remove: '\\', + result: '/\\/\\helloworld' + }, + { + name: 'when the first and the third characters are matched', + str: '@1@12345helloworld', + remove: '@', + result: '1@12345helloworld' + }, + ]; + + for (const {name, str, remove, result } of testCases) { + it(name, function () { + assert.strictEqual(Util.lstrip(str, remove), result); + }); + } + }); + }); From e825561e21c2542799b41c8541c09926dcfc8fce Mon Sep 17 00:00:00 2001 From: John Yun Date: Thu, 19 Dec 2024 09:42:08 -0800 Subject: [PATCH 3/3] lint fix --- lib/util.js | 4 ++-- test/unit/util_test.js | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/util.js b/lib/util.js index 5359e902a..b1d7830f1 100644 --- a/lib/util.js +++ b/lib/util.js @@ -763,9 +763,9 @@ exports.isWindows = function () { * * @returns {String} */ -exports.lstrip = function(str, remove) { +exports.lstrip = function (str, remove) { while (str.length > 0 && remove.indexOf(str.charAt(0)) !== -1) { str = str.substr(1); } return str; -} \ No newline at end of file +}; \ No newline at end of file diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 6a058dbbf..107e7ac57 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -7,7 +7,6 @@ const assert = require('assert'); const path = require('path'); const fsPromises = require('fs/promises'); const os = require('os'); -const { lstat } = require('fs'); describe('Util', function () { it('Util.isFunction()', function () { @@ -1238,7 +1237,7 @@ describe('Util', function () { }, ]; - for (const {name, str, remove, result } of testCases) { + for (const { name, str, remove, result } of testCases) { it(name, function () { assert.strictEqual(Util.lstrip(str, remove), result); });