Skip to content

Commit aeb3a02

Browse files
fix(storage): resolve transport and retry issues (googleapis#8235)
* fix(storage): standardize URL formatting and enhance transport retry * fix storage transport & retry issues * fix * Update handwritten/storage/src/file.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * fix(storage): interceptors test * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * feat: implement robust storage conformance test retry framework with request interception and test bench integration * fix: correct response handler for binary/resumable uploads and improve etag check - Updated `responseHandler` to correctly handle different payload types: - Plain objects are mutated with `.headers` and `.status` and returned. - Binary payloads (Buffer/Stream) return raw data to prevent dangerous mutations. - Primitives (e.g., empty strings) return the full `GaxiosResponse` wrapper to preserve access to headers like `Location` for resumable upload initiation. - Fixed `hasPrecondition` logic to safely parse stringified JSON or inspect objects directly for an `etag` property. This prevents false positives on raw text payloads containing the word "etag" and false negatives on object payloads. * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * refactor(storage): replace constructor-based type checks with structural checks and decouple retry logic into idempotent and transient error utilities. * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * refactor(storage): update storage-transport to return full GaxiosResponse and align downstream resource methods * fix: update file request URL construction to support custom protocol endpoints * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * refactor(storage): introduce ENCRYPTION_ALGORITHM_AES256 constant to replace hardcoded strings in File class * fix(storage): merge request headers correctly in file.ts and add missing linting suppressions to ServiceObject * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * test: add bytes method to mock Gaxios responses in storage tests * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * test: add bytes method to mock Gaxios response in acl and headers tests * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * test: add bytes method to mock Gaxios response in acl and headers tests * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * test: add bytes method to mock Gaxios response in acl and headers tests * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * test: add bytes method to mock Gaxios response in acl and headers tests * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * fix(storage): standardize URL formatting and enhance transport retry * refactor(storage): remove Service.ts and migrate logic to StorageTransport (googleapis#8283) - Remove Service.ts and common.ts files from handwritten/storage - Migrate remaining functionality to StorageTransport - chore(ci): upgrade conformance tests to Node 18 * refactor: improve type safety and validation logic in isBucket helper function --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent bae6491 commit aeb3a02

8 files changed

Lines changed: 652 additions & 160 deletions

File tree

handwritten/storage/src/bucket.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3470,13 +3470,13 @@ class Bucket extends ServiceObject<Bucket, BucketMetadata> {
34703470
* @returns {Promise<Bucket>}
34713471
*/
34723472
async restore(options: RestoreOptions): Promise<Bucket> {
3473-
const bucket = await this.storageTransport.makeRequest<Bucket>({
3473+
const response = await this.storageTransport.makeRequest<Bucket>({
34743474
method: 'POST',
34753475
url: `${this.baseUrl}/${this.name}/restore`,
34763476
queryParameters: options as unknown as StorageQueryParameters,
34773477
});
34783478

3479-
return bucket as Bucket;
3479+
return response.data as Bucket;
34803480
}
34813481

34823482
makePrivate(

handwritten/storage/src/file.ts

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,11 @@ export const STORAGE_POST_POLICY_BASE_URL = 'https://storage.googleapis.com';
349349
*/
350350
const GS_URL_REGEXP = /^gs:\/\/([a-z0-9_.-]+)\/(.+)$/;
351351

352+
/**
353+
* @private
354+
*/
355+
const ENCRYPTION_ALGORITHM_AES256 = 'AES256';
356+
352357
/**
353358
* @private
354359
* This regex will match compressible content types. These are primarily text/*, +json, +text, +xml content types.
@@ -1393,7 +1398,10 @@ class File extends ServiceObject<File, FileMetadata> {
13931398
const headers = new Headers();
13941399

13951400
if (this.encryptionKey !== undefined) {
1396-
headers.set('x-goog-copy-source-encryption-algorithm', 'AES256');
1401+
headers.set(
1402+
'x-goog-copy-source-encryption-algorithm',
1403+
ENCRYPTION_ALGORITHM_AES256,
1404+
);
13971405
headers.set(
13981406
'x-goog-copy-source-encryption-key',
13991407
this.encryptionKeyBase64!,
@@ -1405,7 +1413,12 @@ class File extends ServiceObject<File, FileMetadata> {
14051413
}
14061414

14071415
if (newFile.encryptionKey !== undefined) {
1408-
this.setEncryptionKey(newFile.encryptionKey!);
1416+
headers.set('x-goog-encryption-algorithm', ENCRYPTION_ALGORITHM_AES256);
1417+
headers.set('x-goog-encryption-key', newFile.encryptionKeyBase64 || '');
1418+
headers.set(
1419+
'x-goog-encryption-key-sha256',
1420+
newFile.encryptionKeyHash || '',
1421+
);
14091422
} else if (options.destinationKmsKeyName !== undefined) {
14101423
query.destinationKmsKeyName = options.destinationKmsKeyName;
14111424
delete options.destinationKmsKeyName;
@@ -1639,6 +1652,8 @@ class File extends ServiceObject<File, FileMetadata> {
16391652
}
16401653

16411654
const headers = response.headers;
1655+
const isStoredCompressed =
1656+
headers.get('x-goog-stored-content-encoding') === 'gzip';
16421657
const isCompressed = headers.get('content-encoding') === 'gzip';
16431658
const hashes: {crc32c?: string; md5?: string} = {};
16441659

@@ -1652,7 +1667,7 @@ class File extends ServiceObject<File, FileMetadata> {
16521667

16531668
const transformStreams: Transform[] = [];
16541669

1655-
if (shouldRunValidation) {
1670+
if (shouldRunValidation && !isStoredCompressed) {
16561671
// The x-goog-hash header should be set with a crc32c and md5 hash.
16571672
// ex: headers.set('x-goog-hash', 'crc32c=xxxx,md5=xxxx')
16581673
if (typeof headers.get('x-goog-hash') === 'string') {
@@ -1721,6 +1736,7 @@ class File extends ServiceObject<File, FileMetadata> {
17211736
const headers = {
17221737
'Accept-Encoding': 'gzip',
17231738
'Cache-Control': 'no-store',
1739+
...(this.encryptionKeyHeaders || {}),
17241740
} as Headers;
17251741

17261742
if (rangeRequest) {
@@ -1735,7 +1751,9 @@ class File extends ServiceObject<File, FileMetadata> {
17351751
headers,
17361752
queryParameters: query as unknown as StorageQueryParameters,
17371753
responseType: 'stream',
1738-
};
1754+
decompress: options.decompress,
1755+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1756+
} as any;
17391757

17401758
if (options[GCCL_GCS_CMD_KEY]) {
17411759
reqOpts[GCCL_GCS_CMD_KEY] = options[GCCL_GCS_CMD_KEY];
@@ -2426,6 +2444,18 @@ class File extends ServiceObject<File, FileMetadata> {
24262444
}
24272445
}
24282446

2447+
get encryptionKeyHeaders(): Record<string, string> | undefined {
2448+
if (!this.encryptionKey) {
2449+
return undefined;
2450+
}
2451+
2452+
return {
2453+
'x-goog-encryption-algorithm': ENCRYPTION_ALGORITHM_AES256,
2454+
'x-goog-encryption-key': this.encryptionKey.toString('base64'),
2455+
'x-goog-encryption-key-sha256': this.encryptionKeyHash || '',
2456+
};
2457+
}
2458+
24292459
/**
24302460
* The Storage API allows you to use a custom key for server-side encryption.
24312461
*
@@ -2485,7 +2515,10 @@ class File extends ServiceObject<File, FileMetadata> {
24852515
this.encryptionKeyInterceptor = {
24862516
resolved: reqOpts => {
24872517
reqOpts.headers = new Headers(reqOpts.headers || {});
2488-
reqOpts.headers.set('x-goog-encryption-algorithm', 'AES256');
2518+
reqOpts.headers.set(
2519+
'x-goog-encryption-algorithm',
2520+
ENCRYPTION_ALGORITHM_AES256,
2521+
);
24892522
reqOpts.headers.set('x-goog-encryption-key', this.encryptionKeyBase64!);
24902523
reqOpts.headers.set(
24912524
'x-goog-encryption-key-sha256',
@@ -3315,7 +3348,11 @@ class File extends ServiceObject<File, FileMetadata> {
33153348
undefined,
33163349
callback,
33173350
);
3318-
const url = `https://${this.storage.apiEndpoint}/storage/v1/b/${this.bucket.name}/o/${encodeURIComponent(this.name)}`;
3351+
const baseUrl = this.storage.apiEndpoint.startsWith('http')
3352+
? this.storage.apiEndpoint
3353+
: `https://${this.storage.apiEndpoint}`;
3354+
3355+
const url = `${baseUrl}/storage/v1/b/${this.bucket.name}/o/${encodeURIComponent(this.name)}`;
33193356

33203357
const gaxios = new Gaxios();
33213358
const storageInterceptors = this.storage?.interceptors || [];
@@ -4025,12 +4062,12 @@ class File extends ServiceObject<File, FileMetadata> {
40254062
* @returns {Promise<File>}
40264063
*/
40274064
async restore(options: RestoreOptions): Promise<File> {
4028-
const file = await this.storageTransport.makeRequest<File>({
4065+
const response = await this.storageTransport.makeRequest<File>({
40294066
method: 'POST',
40304067
url: `/storage/v1/b/${this.bucket.name}/o/${encodeURIComponent(this.name)}/restore`,
40314068
queryParameters: options as unknown as StorageQueryParameters,
40324069
});
4033-
return file as File;
4070+
return response.data as File;
40344071
}
40354072

40364073
rotateEncryptionKey(
@@ -4562,6 +4599,17 @@ class File extends ServiceObject<File, FileMetadata> {
45624599
},
45634600
];
45644601

4602+
const headers: Record<string, string> = {};
4603+
if (this.encryptionKey) {
4604+
headers['x-goog-encryption-algorithm'] = ENCRYPTION_ALGORITHM_AES256;
4605+
headers['x-goog-encryption-key'] = this.encryptionKeyBase64!;
4606+
headers['x-goog-encryption-key-sha256'] = this.encryptionKeyHash!;
4607+
}
4608+
reqOpts.headers = {
4609+
...reqOpts.headers,
4610+
...headers,
4611+
};
4612+
45654613
this.storageTransport
45664614
.makeRequest(reqOpts as StorageRequestOptions, (err, body, resp) => {
45674615
if (err) {

handwritten/storage/src/nodejs-common/service-object.ts

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,30 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
import {promisifyAll} from '@google-cloud/promisify';
17-
import {EventEmitter} from 'events';
18-
import {util} from './util.js';
19-
import {Bucket} from '../bucket.js';
20-
import {StorageRequestOptions, StorageTransport} from '../storage-transport.js';
16+
import { promisifyAll } from '@google-cloud/promisify';
17+
import { EventEmitter } from 'events';
18+
import { util } from './util.js';
19+
import { StorageRequestOptions, StorageTransport } from '../storage-transport.js';
2120
import {
2221
GaxiosError,
2322
GaxiosInterceptor,
2423
GaxiosOptionsPrepared,
2524
GaxiosResponse,
2625
} from 'gaxios';
26+
import type { Bucket } from '../bucket.js';
27+
28+
function isBucket(parent: unknown): parent is Bucket {
29+
if (!parent || typeof parent !== 'object') {
30+
return false;
31+
}
32+
33+
const obj = parent as Record<string, unknown>;
34+
return (
35+
typeof obj.getFiles === 'function' &&
36+
typeof obj.upload === 'function' &&
37+
typeof obj.exists === 'function'
38+
);
39+
}
2740

2841
export type GetMetadataOptions = object;
2942

@@ -97,7 +110,7 @@ export interface InstanceResponseCallback<T> {
97110
}
98111

99112
// eslint-disable-next-line @typescript-eslint/no-empty-interface
100-
export interface CreateOptions {}
113+
export interface CreateOptions { }
101114
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unused-vars
102115
export type CreateResponse<T> = any[];
103116
export interface CreateCallback<T> {
@@ -208,8 +221,8 @@ class ServiceObject<T, K extends BaseMetadata> extends EventEmitter {
208221
// The ServiceObject didn't redefine the method.
209222
// eslint-disable-next-line @typescript-eslint/no-explicit-any
210223
(this as any)[methodName] ===
211-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
212-
(ServiceObject.prototype as any)[methodName] &&
224+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
225+
(ServiceObject.prototype as any)[methodName] &&
213226
// This method isn't wanted.
214227
!config.methods![methodName]
215228
);
@@ -294,8 +307,10 @@ class ServiceObject<T, K extends BaseMetadata> extends EventEmitter {
294307
(typeof this.methods.delete === 'object' && this.methods.delete) || {};
295308

296309
let url = `${this.baseUrl}/${this.id}`;
297-
if (this.parent instanceof Bucket) {
298-
url = `${this.parent.baseUrl}/${this.parent.id}${url}`;
310+
if (isBucket(this.parent)) {
311+
// TODO: remove any suppression during follow up PR to improve type safety.
312+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
313+
url = `${this.parent.baseUrl}/${(this.parent as any).id}${url}`;
299314
}
300315

301316
this.storageTransport
@@ -441,20 +456,39 @@ class ServiceObject<T, K extends BaseMetadata> extends EventEmitter {
441456
{};
442457

443458
let url = `${this.baseUrl}/${this.id}`;
444-
if (this.parent instanceof Bucket) {
445-
url = `${this.parent.baseUrl}/${this.parent.id}${url}`;
459+
if (isBucket(this.parent)) {
460+
// TODO: remove any suppression during follow up PR to improve type safety.
461+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
462+
url = `${this.parent.baseUrl}/${(this.parent as any).id}${url}`;
446463
}
447464

465+
// TODO: remove any suppression during follow up PR to improve type safety.
466+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
467+
const encryptionHeaders = (this as any).encryptionKeyHeaders || {};
468+
469+
const headers = {
470+
...encryptionHeaders,
471+
...methodConfig.reqOpts?.headers,
472+
// TODO: remove any suppression during follow up PR to improve type safety.
473+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
474+
...(options as any).headers,
475+
};
476+
477+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
478+
const query = { ...options } as any;
479+
delete query.headers;
480+
448481
this.storageTransport
449482
.makeRequest<K>(
450483
{
451484
method: 'GET',
452485
responseType: 'json',
453486
url,
454487
...methodConfig.reqOpts,
488+
headers,
455489
queryParameters: {
456490
...methodConfig.reqOpts?.queryParameters,
457-
...options,
491+
...query,
458492
},
459493
},
460494
(err, data, resp) => {
@@ -499,8 +533,10 @@ class ServiceObject<T, K extends BaseMetadata> extends EventEmitter {
499533
{};
500534

501535
let url = `${this.baseUrl}/${this.name}`;
502-
if (this.parent instanceof Bucket) {
503-
url = `${this.parent.baseUrl}/${this.parent.name}${url}`;
536+
if (isBucket(this.parent)) {
537+
// TODO: remove any suppression during follow up PR to improve type safety.
538+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
539+
url = `${this.parent.baseUrl}/${(this.parent as any).name}${url}`;
504540
}
505541

506542
const body = Object.assign({}, methodConfig.reqOpts?.body, metadata);
@@ -530,6 +566,6 @@ class ServiceObject<T, K extends BaseMetadata> extends EventEmitter {
530566
}
531567
}
532568

533-
promisifyAll(ServiceObject, {exclude: ['getRequestInterceptors']});
569+
promisifyAll(ServiceObject, { exclude: ['getRequestInterceptors'] });
534570

535-
export {ServiceObject};
571+
export { ServiceObject };

0 commit comments

Comments
 (0)