fix: System, unit, and conformance test cases

Author: thiyaguk09

handwritten/storage/src/storage-transport.ts

@@ -133,6 +133,10 @@ export class StorageTransport {
         reqOpts.projectId || (await this.authClient.getProjectId());
     }
 
+    if (reqOpts.queryParameters && this.projectId) {
+      reqOpts.queryParameters.project = this.projectId;
+    }
+
     // Header Construction
     const headers = this.#prepareHeaders(reqOpts);
 
@@ -145,7 +149,12 @@ export class StorageTransport {
     }
 
     const urlString = reqOpts.url?.toString() || '';
-    const isAbsolute = urlString.startsWith('http');
+    const isAbsolute = this.#isValidUrl(urlString);
+
+    // Determine the base URL for the request
+    const requestUrl = isAbsolute
+      ? urlString
+      : new URL(urlString, this.baseUrl).toString();
 
     try {
       const requestPromise = this.authClient.request<T>({
@@ -159,20 +168,22 @@ export class StorageTransport {
         },
         ...reqOpts,
         data: reqOpts.body,
-        params: isAbsolute ? undefined : reqOpts.queryParameters,
+        params: reqOpts.queryParameters,
+        paramsSerializer: this.#paramsSerializer,
         headers,
-        url: isAbsolute
-          ? urlString
-          : this.#buildUrl(reqOpts.url?.toString(), reqOpts.queryParameters),
+        url: requestUrl,
         timeout: this.timeout,
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
         ...({decompress: false} as any),
         validateStatus: status => {
-          if (urlString.includes('uploadType=resumable')) {
-            return (status >= 200 && status < 300) || status === 308;
-          }
-          return status >= 200 && status < 300;
-        }, //status => status >= 200 && status < 300,
+          const isResumable =
+            reqOpts.queryParameters?.uploadType === 'resumable' ||
+            reqOpts.url?.toString().includes('uploadType=resumable');
+
+          return (
+            (status >= 200 && status < 300) || (isResumable && status === 308)
+          );
+        },
       });
 
       // Response Handling
@@ -218,25 +229,6 @@ export class StorageTransport {
     return finalHeaders;
   }
 
-  #buildUrl(pathUri = '', queryParameters: StorageQueryParameters = {}): URL {
-    // Sync project ID in params if necessary
-    if (
-      'project' in queryParameters &&
-      queryParameters.project !== this.projectId
-    ) {
-      queryParameters.project = this.projectId;
-    }
-
-    let url: URL;
-    if (this.#isValidUrl(pathUri)) {
-      url = new URL(pathUri);
-    } else {
-      url = new URL(pathUri, this.baseUrl); // Safer construction
-    }
-    url.search = this.#buildRequestQueryParams(queryParameters);
-    return url;
-  }
-
   #isValidUrl(url: string): boolean {
     try {
       return Boolean(new URL(url));
@@ -245,6 +237,26 @@ export class StorageTransport {
     }
   }
 
+  /**
+   * Serializes query parameters into a string.
+   * Specifically handles arrays by appending each value individually 
+   * to satisfy GCS "repeated key" requirements (e.g., for IAM permissions).
+   */
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  #paramsSerializer = (params: Record<string, any>): string => {
+    const searchParams = new URLSearchParams();
+    for (const [key, value] of Object.entries(params)) {
+      if (value === undefined) continue;
+
+      if (Array.isArray(value)) {
+        value.forEach(v => searchParams.append(key, String(v)));
+      } else {
+        searchParams.set(key, String(value));
+      }
+    }
+    return searchParams.toString();
+  };
+
   #buildRequestHeaders(requestHeaders = {}) {
     const headers = new Headers(requestHeaders);
     headers.set('User-Agent', this.#getUserAgentString());
@@ -255,21 +267,6 @@ export class StorageTransport {
     return headers;
   }
 
-  #buildRequestQueryParams(queryParameters: StorageQueryParameters): string {
-    const qp = new URLSearchParams();
-    for (const [key, value] of Object.entries(queryParameters)) {
-      if (value === undefined) continue;
-
-      if (Array.isArray(value)) {
-        // This is the fix: append each item individually for repeated keys
-        value.forEach(item => qp.append(key, String(item)));
-      } else {
-        qp.set(key, String(value));
-      }
-    }
-    return qp.toString();
-  }
-
   #getUserAgentString(): string {
     const base = getUserAgentString();
     return this.providedUserAgent ? `${this.providedUserAgent} ${base}` : base;

handwritten/storage/system-test/fixtures/index-cjs.js

@@ -12,11 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// eslint-disable-next-line no-undef
+/* eslint-disable node/no-missing-require, no-unused-vars, no-undef */
 const {Storage} = require('@google-cloud/storage');
 
 function main() {
-  // eslint-disable-next-line no-unused-vars
   const storage = new Storage();
 }
 

handwritten/storage/system-test/fixtures/index-esm.js

@@ -12,11 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// eslint-disable-next-line no-undef
-const {Storage} = require('@google-cloud/storage');
+/* eslint-disable node/no-missing-import, no-unused-vars */
+import {Storage} from '@google-cloud/storage';
 
 function main() {
-  // eslint-disable-next-line no-unused-vars
   const storage = new Storage();
 }
 

handwritten/storage/system-test/kitchen.ts

@@ -54,7 +54,10 @@ describe('resumable-upload', () => {
     retryableErrorFn: RETRYABLE_ERR_FN_DEFAULT,
   };
 
-  const bucket = new Storage({retryOptions}).bucket(bucketName);
+  const bucket = new Storage({
+    projectId: process.env.PROJECT_ID,
+    retryOptions: retryOptions,
+  }).bucket(bucketName);
   let filePath: string;
 
   before(async () => {
@@ -96,7 +99,7 @@ describe('resumable-upload', () => {
     // see: https://cloud.google.com/storage/docs/exponential-backoff:
     const ms = Math.pow(2, retries) * 1000 + Math.random() * 2000;
     console.info(`retrying "${title}" in ${ms}ms`);
-    setTimeout(done(), ms);
+    setTimeout(() => done, ms);
   }
 
   it('should work', done => {