Skip to content

Commit 5cd7d6e

Browse files
thiyaguk09thiyaguk09
committed
refactor: update header type definitions in resumable upload tests to use string | undefined
1 parent 7016a8c commit 5cd7d6e

5 files changed

Lines changed: 131 additions & 20 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ class ServiceObject<T, K extends BaseMetadata> extends EventEmitter {
532532
this.methods.setMetadata) ||
533533
{};
534534

535-
let url = `${this.baseUrl}/${this.name}`;
535+
let url = `${this.baseUrl}/${this.id || this.name}`;
536536
if (isBucket(this.parent)) {
537537
// TODO: remove any suppression during follow up PR to improve type safety.
538538
// eslint-disable-next-line @typescript-eslint/no-explicit-any

‎handwritten/storage/src/storage-transport.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,13 @@ export class StorageTransport {
181181
try {
182182
const requestPromise = this.authClient.request<T>({
183183
adapter: async (opts: GaxiosOptions) => {
184+
const urlHasParams = opts.url
185+
? opts.url.toString() !== requestUrl
186+
: false;
184187
const innerOpts = {
185188
...opts,
186189
adapter: undefined,
190+
params: urlHasParams ? undefined : opts.params,
187191
};
188192
return requestGaxiosInstance.request(innerOpts);
189193
},
@@ -265,6 +269,19 @@ export class StorageTransport {
265269
packageJson: this.packageJson,
266270
providedUserAgent: this.providedUserAgent,
267271
});
272+
273+
const hasContentType = Object.keys(headers).some(
274+
k => k.toLowerCase() === 'content-type',
275+
);
276+
if (!hasContentType && reqOpts.body && typeof reqOpts.body === 'string') {
277+
try {
278+
JSON.parse(reqOpts.body);
279+
headers['Content-Type'] = 'application/json';
280+
} catch {
281+
// Not a JSON string, leave Content-Type unset
282+
}
283+
}
284+
268285
return headers;
269286
}
270287

‎handwritten/storage/test/nodejs-common/service-object.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ describe('ServiceObject', () => {
566566
});
567567

568568
describe('setMetadata', () => {
569-
it('should make the correct request', async done => {
569+
it('should make the correct request', done => {
570570
const metadata = {metadataProperty: true};
571571
serviceObject.storageTransport.makeRequest = sandbox
572572
.stub()
@@ -578,13 +578,13 @@ describe('ServiceObject', () => {
578578
const body = JSON.parse(reqOpts.body);
579579
assert.strictEqual(this, serviceObject.storageTransport);
580580
assert.strictEqual(reqOpts.method, 'PATCH');
581-
assert.strictEqual(reqOpts.url, 'base-url/undefined');
581+
assert.strictEqual(reqOpts.url, 'base-url/id');
582582
assert.deepStrictEqual(body, metadata);
583583
done();
584584
callback!(null);
585585
return Promise.resolve();
586586
});
587-
await serviceObject.setMetadata(metadata, () => {});
587+
void serviceObject.setMetadata(metadata, () => {});
588588
});
589589

590590
it('should accept options', done => {

‎handwritten/storage/test/resumable-upload.ts‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -875,7 +875,7 @@ describe('resumable-upload', () => {
875875
delete metadataNoHeaders.contentType;
876876
assert.deepStrictEqual(reqOpts.data, metadataNoHeaders);
877877
assert(reqOpts.headers);
878-
const headers = reqOpts.headers as Record<string, any>;
878+
const headers = reqOpts.headers as Record<string, string | undefined>;
879879
const apiClientHeader = headers['x-goog-api-client'];
880880
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader as string);
881881
assert.ok(match);
@@ -897,7 +897,10 @@ describe('resumable-upload', () => {
897897

898898
up.authClient.request = async (combinedReqOpts: GaxiosOptions) => {
899899
assert(combinedReqOpts.headers);
900-
const headers = combinedReqOpts.headers as Record<string, any>;
900+
const headers = combinedReqOpts.headers as Record<
901+
string,
902+
string | undefined
903+
>;
901904
const apiClientHeader = headers['x-goog-api-client'];
902905
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader as string);
903906
assert.ok(match);
@@ -929,7 +932,10 @@ describe('resumable-upload', () => {
929932

930933
up.authClient.request = async (combinedReqOpts: GaxiosOptions) => {
931934
assert(combinedReqOpts.headers);
932-
const headers = combinedReqOpts.headers as Record<string, any>;
935+
const headers = combinedReqOpts.headers as Record<
936+
string,
937+
string | undefined
938+
>;
933939
const apiClientHeader = headers['x-goog-api-client'];
934940
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader as string);
935941
assert.ok(match);
@@ -957,7 +963,10 @@ describe('resumable-upload', () => {
957963

958964
up.authClient.request = async (combinedReqOpts: GaxiosOptions) => {
959965
assert(combinedReqOpts.headers);
960-
const headers = combinedReqOpts.headers as Record<string, any>;
966+
const headers = combinedReqOpts.headers as Record<
967+
string,
968+
string | undefined
969+
>;
961970
const apiClientHeader = headers['x-goog-api-client'];
962971
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader as string);
963972
assert.ok(match);
@@ -987,7 +996,7 @@ describe('resumable-upload', () => {
987996
up.makeRequest = async (reqOpts: GaxiosOptions) => {
988997
invocationCount++;
989998
assert(reqOpts.headers);
990-
const headers = reqOpts.headers as Record<string, any>;
999+
const headers = reqOpts.headers as Record<string, string | undefined>;
9911000
if (invocationCount === 1) {
9921001
token1 = headers['x-goog-gcs-idempotency-token'] as string;
9931002
const error = new GaxiosError(
@@ -1470,7 +1479,7 @@ describe('resumable-upload', () => {
14701479
await up.startUploading();
14711480

14721481
assert(capturedHeaders);
1473-
const headers = capturedHeaders as Record<string, any>;
1482+
const headers = capturedHeaders as Record<string, string | undefined>;
14741483
const apiClientHeader = headers['x-goog-api-client'] as string;
14751484
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader);
14761485
assert.ok(match);
@@ -1497,7 +1506,7 @@ describe('resumable-upload', () => {
14971506
await up.startUploading();
14981507

14991508
assert(capturedHeaders);
1500-
const headers = capturedHeaders as Record<string, any>;
1509+
const headers = capturedHeaders as Record<string, string | undefined>;
15011510
const apiClientHeader = headers['x-goog-api-client'] as string;
15021511
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader);
15031512
assert.ok(match);
@@ -1532,7 +1541,7 @@ describe('resumable-upload', () => {
15321541
await up.startUploading();
15331542

15341543
assert(capturedHeaders);
1535-
const headers = capturedHeaders as Record<string, any>;
1544+
const headers = capturedHeaders as Record<string, string | undefined>;
15361545
const apiClientHeader = headers['x-goog-api-client'] as string;
15371546
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader);
15381547
assert.ok(match);
@@ -1562,7 +1571,7 @@ describe('resumable-upload', () => {
15621571
await up.startUploading();
15631572

15641573
assert(capturedHeaders);
1565-
const headers = capturedHeaders as Record<string, any>;
1574+
const headers = capturedHeaders as Record<string, string | undefined>;
15661575
const apiClientHeader = headers['x-goog-api-client'] as string;
15671576
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader);
15681577
assert.ok(match);
@@ -1587,7 +1596,10 @@ describe('resumable-upload', () => {
15871596

15881597
up.makeRequestStream = async (requestOptions: GaxiosOptions) => {
15891598
invocationCount++;
1590-
const headers = requestOptions.headers as Record<string, any>;
1599+
const headers = requestOptions.headers as Record<
1600+
string,
1601+
string | undefined
1602+
>;
15911603
const token = headers['x-goog-gcs-idempotency-token'] as string;
15921604
if (invocationCount === 1) {
15931605
token1 = token;
@@ -1624,7 +1636,10 @@ describe('resumable-upload', () => {
16241636
const chunkInvocationIds: string[] = [];
16251637

16261638
up.makeRequestStream = async (requestOptions: GaxiosOptions) => {
1627-
const headers = requestOptions.headers as Record<string, any>;
1639+
const headers = requestOptions.headers as Record<
1640+
string,
1641+
string | undefined
1642+
>;
16281643
const token = headers['x-goog-gcs-idempotency-token'] as string;
16291644
const apiClientHeader = headers['x-goog-api-client'] as string;
16301645
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader);
@@ -2235,7 +2250,7 @@ describe('resumable-upload', () => {
22352250
await up.checkUploadStatus();
22362251

22372252
assert(capturedHeaders);
2238-
const headers = capturedHeaders as Record<string, any>;
2253+
const headers = capturedHeaders as Record<string, string | undefined>;
22392254
const apiClientHeader = headers['x-goog-api-client'] as string;
22402255
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader);
22412256
assert.ok(match);
@@ -2259,7 +2274,7 @@ describe('resumable-upload', () => {
22592274
await up.checkUploadStatus();
22602275

22612276
assert(capturedHeaders);
2262-
const headers = capturedHeaders as Record<string, any>;
2277+
const headers = capturedHeaders as Record<string, string | undefined>;
22632278
const apiClientHeader = headers['x-goog-api-client'] as string;
22642279
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader);
22652280
assert.ok(match);
@@ -2288,7 +2303,7 @@ describe('resumable-upload', () => {
22882303
await up.checkUploadStatus();
22892304

22902305
assert(capturedHeaders);
2291-
const headers = capturedHeaders as Record<string, any>;
2306+
const headers = capturedHeaders as Record<string, string | undefined>;
22922307
const apiClientHeader = headers['x-goog-api-client'] as string;
22932308
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader);
22942309
assert.ok(match);
@@ -2315,7 +2330,7 @@ describe('resumable-upload', () => {
23152330
await up.checkUploadStatus();
23162331

23172332
assert(capturedHeaders);
2318-
const headers = capturedHeaders as Record<string, any>;
2333+
const headers = capturedHeaders as Record<string, string | undefined>;
23192334
const apiClientHeader = headers['x-goog-api-client'] as string;
23202335
const match = X_GOOG_API_HEADER_REGEX.exec(apiClientHeader);
23212336
assert.ok(match);
@@ -2337,7 +2352,7 @@ describe('resumable-upload', () => {
23372352

23382353
up.makeRequest = async (reqOpts: GaxiosOptions) => {
23392354
invocationCount++;
2340-
const headers = reqOpts.headers as Record<string, any>;
2355+
const headers = reqOpts.headers as Record<string, string | undefined>;
23412356
const token = headers['x-goog-gcs-idempotency-token'] as string;
23422357
if (invocationCount === 1) {
23432358
token1 = token;

‎handwritten/storage/test/storage-transport.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,5 +444,84 @@ describe('Storage Transport', () => {
444444
true,
445445
);
446446
});
447+
448+
it('should retain query parameters in adapter if url was not modified', async () => {
449+
const gaxios = new Gaxios();
450+
let capturedOpts: any;
451+
sandbox.stub(gaxios, 'request').callsFake(async (opts: any) => {
452+
capturedOpts = opts;
453+
return {data: {}, headers: new Map(), status: 200} as any;
454+
});
455+
const customTransport = new StorageTransport({
456+
apiEndpoint: baseUrl,
457+
baseUrl,
458+
authClient: authClientStub,
459+
gaxiosInstance: gaxios,
460+
projectId: 'project-id',
461+
retryOptions: {
462+
maxRetries: 3,
463+
retryDelayMultiplier: 2,
464+
maxRetryDelay: 100,
465+
totalTimeout: 1000,
466+
retryableErrorFn: RETRYABLE_ERR_FN_DEFAULT,
467+
},
468+
scopes: ['https://www.googleapis.com/auth/cloud-platform'],
469+
packageJson: {name: 'test-package', version: '1.0.0'},
470+
});
471+
const requestStub = authClientStub.request as sinon.SinonStub;
472+
requestStub.callsFake(async (opts: any) => {
473+
return opts.adapter(opts);
474+
});
475+
476+
await customTransport.makeRequest({
477+
method: 'GET',
478+
url: '/test',
479+
queryParameters: {foo: 'bar'},
480+
});
481+
482+
assert.deepStrictEqual(capturedOpts.params, {foo: 'bar'});
483+
});
484+
485+
it('should clear params in adapter if url already has params serialized', async () => {
486+
const gaxios = new Gaxios();
487+
let capturedOpts: any;
488+
sandbox.stub(gaxios, 'request').callsFake(async (opts: any) => {
489+
capturedOpts = opts;
490+
return {data: {}, headers: new Map(), status: 200} as any;
491+
});
492+
const customTransport = new StorageTransport({
493+
apiEndpoint: baseUrl,
494+
baseUrl,
495+
authClient: authClientStub,
496+
gaxiosInstance: gaxios,
497+
projectId: 'project-id',
498+
retryOptions: {
499+
maxRetries: 3,
500+
retryDelayMultiplier: 2,
501+
maxRetryDelay: 100,
502+
totalTimeout: 1000,
503+
retryableErrorFn: RETRYABLE_ERR_FN_DEFAULT,
504+
},
505+
scopes: ['https://www.googleapis.com/auth/cloud-platform'],
506+
packageJson: {name: 'test-package', version: '1.0.0'},
507+
});
508+
const requestStub = authClientStub.request as sinon.SinonStub;
509+
requestStub.callsFake(async (opts: any) => {
510+
const preparedOpts = {
511+
...opts,
512+
url: `${opts.url}?foo=bar`,
513+
};
514+
return opts.adapter(preparedOpts);
515+
});
516+
517+
await customTransport.makeRequest({
518+
method: 'GET',
519+
url: '/test',
520+
queryParameters: {foo: 'bar'},
521+
});
522+
523+
assert.strictEqual(capturedOpts.params, undefined);
524+
assert.strictEqual(capturedOpts.url, `${baseUrl}/test?foo=bar`);
525+
});
447526
});
448527
});

0 commit comments

Comments
 (0)