refactor(storage): Migrate conformance tests to StorageTransport and enhance retry logic

.github/workflows/conformance-test.yaml

@@ -15,7 +15,7 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/setup-node@v4
         with:
-          node-version: 14
+          node-version: 18
       - run: node --version
       - run: cd handwritten/storage && npm install
       - run: cd handwritten/storage && npm run conformance-test

handwritten/storage/.github/sync-repo-settings.yaml

@@ -12,7 +12,6 @@ branchProtectionRules:
     requiredStatusCheckContexts:
       - "ci/kokoro: Samples test"
       - "ci/kokoro: System test"
-      - docs
       - lint
       - test (18)
       - test (20)

handwritten/storage/.github/workflows/ci.yaml

@@ -46,15 +46,3 @@ jobs:
           node-version: 18
       - run: npm install
       - run: npm run lint
-  docs:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-      - uses: actions/setup-node@v3
-        with:
-          node-version: 18
-      - run: npm install
-      - run: npm run docs
-      - uses: JustinBeckwith/linkinator-action@v1
-        with:
-          paths: docs/

handwritten/storage/conformance-test/conformanceCommon.ts

@@ -13,24 +13,19 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-// eslint-disable-next-line @typescript-eslint/no-unused-vars
 import * as jsonToNodeApiMapping from './test-data/retryInvocationMap.json';
 import * as libraryMethods from './libraryMethods';
-import {
-  Bucket,
-  File,
-  GaxiosOptions,
-  GaxiosOptionsPrepared,
-  HmacKey,
-  Notification,
-  Storage,
-} from '../src';
+import {Bucket, File, Gaxios, HmacKey, Notification, Storage} from '../src';
 import * as uuid from 'uuid';
 import * as assert from 'assert';
 import {
   StorageRequestOptions,
   StorageTransport,
+  StorageTransportCallback,
 } from '../src/storage-transport';
+import {getDirName} from '../src/util';
+import path from 'path';
+import {GoogleAuth} from 'google-auth-library';
 interface RetryCase {
   instructions: String[];
 }
@@ -60,16 +55,31 @@ interface ConformanceTestResult {
 
 type LibraryMethodsModuleType = typeof import('./libraryMethods');
 const methodMap: Map<String, String[]> = new Map(
-  Object.entries({}), // TODO: replace with Object.entries(jsonToNodeApiMapping)
+  Object.entries(jsonToNodeApiMapping),
 );
 
 const DURATION_SECONDS = 600; // 10 mins.
 const TESTS_PREFIX = `storage.retry.tests.${shortUUID()}.`;
 const TESTBENCH_HOST =
   process.env.STORAGE_EMULATOR_HOST || 'http://localhost:9000/';
-const CONF_TEST_PROJECT_ID = 'my-project-id';
+const CONF_TEST_PROJECT_ID = 'dummy-project-id';
 const TIMEOUT_FOR_INDIVIDUAL_TEST = 20000;
 const RETRY_MULTIPLIER_FOR_CONFORMANCE_TESTS = 0.01;
+const SERVICE_ACCOUNT = path.join(
+  getDirName(),
+  '../../../conformance-test/fixtures/signing-service-account.json',
+);
+
+const authClient = new GoogleAuth({
+  keyFilename: SERVICE_ACCOUNT,
+  scopes: ['https://www.googleapis.com/auth/devstorage.full_control'],
+}).fromJSON(require(SERVICE_ACCOUNT));
+
+authClient.getAccessToken = async () => ({token: 'unauthenticated-test-token'});
+authClient.request = async opts => {
+  const gaxios = new Gaxios();
+  return gaxios.request(opts);
+};
 
 export function executeScenario(testCase: RetryTestCase) {
   for (
@@ -89,16 +99,17 @@ export function executeScenario(testCase: RetryTestCase) {
         let bucket: Bucket;
         let file: File;
         let notification: Notification;
-        let creationResult: {id: string};
+        let creationResult: ConformanceTestCreationResult;
         let storage: Storage;
         let hmacKey: HmacKey;
         let storageTransport: StorageTransport;
 
         describe(`${storageMethodString}`, async () => {
           beforeEach(async () => {
-            storageTransport = new StorageTransport({
+            const rawTransport = new StorageTransport({
               apiEndpoint: TESTBENCH_HOST,
-              authClient: undefined,
+              authClient: authClient,
+              keyFilename: SERVICE_ACCOUNT,
               baseUrl: TESTBENCH_HOST,
               packageJson: {name: 'test-package', version: '1.0.0'},
               retryOptions: {
@@ -117,175 +128,176 @@ export function executeScenario(testCase: RetryTestCase) {
               timeout: DURATION_SECONDS,
             });
 
+            creationResult = await createTestBenchRetryTest(
+              instructionSet.instructions,
+              jsonMethod?.name.toString(),
+              rawTransport,
+            );
+
+            // Create a Proxy around rawStorageTransport to intercept makeRequest
+            storageTransport = createRetryProxy(
+              rawTransport,
+              creationResult.id,
+            );
+
             storage = new Storage({
               apiEndpoint: TESTBENCH_HOST,
               projectId: CONF_TEST_PROJECT_ID,
+              keyFilename: SERVICE_ACCOUNT,
+              authClient: authClient,
               retryOptions: {
                 retryDelayMultiplier: RETRY_MULTIPLIER_FOR_CONFORMANCE_TESTS,
               },
             });
 
-            creationResult = await createTestBenchRetryTest(
-              instructionSet.instructions,
-              jsonMethod?.name.toString(),
-              storageTransport,
+            bucket = await createBucketForTest(
+              storage,
+              testCase.preconditionProvided &&
+                !storageMethodString.includes('combine'),
+              storageMethodString,
+            );
+            file = await createFileForTest(
+              testCase.preconditionProvided,
+              storageMethodString,
+              bucket,
             );
-            if (storageMethodString.includes('InstancePrecondition')) {
-              bucket = await createBucketForTest(
-                storage,
-                testCase.preconditionProvided,
-                storageMethodString,
-              );
-              file = await createFileForTest(
-                testCase.preconditionProvided,
-                storageMethodString,
-                bucket,
-              );
-            } else {
-              bucket = await createBucketForTest(
-                storage,
-                false,
-                storageMethodString,
-              );
-              file = await createFileForTest(
-                false,
-                storageMethodString,
-                bucket,
-              );
-            }
             notification = bucket.notification(TESTS_PREFIX);
             await notification.create();
 
             [hmacKey] = await storage.createHmacKey(
               `${TESTS_PREFIX}@email.com`,
             );
-
-            storage.interceptors.push({
-              resolved: (
-                requestConfig: GaxiosOptionsPrepared,
-              ): Promise<GaxiosOptionsPrepared> => {
-                const config = requestConfig as GaxiosOptions;
-                config.headers = config.headers || {};
-                Object.assign(config.headers, {
-                  'x-retry-test-id': creationResult.id,
-                });
-                return Promise.resolve(config as GaxiosOptionsPrepared);
-              },
-              rejected: error => {
-                return Promise.reject(error);
-              },
-            });
           });
 
           it(`${instructionNumber}`, async () => {
             const methodParameters: libraryMethods.ConformanceTestOptions = {
-              storage: storage,
-              bucket: bucket,
-              file: file,
-              storageTransport: storageTransport,
-              notification: notification,
-              hmacKey: hmacKey,
+              storage,
+              bucket,
+              file,
+              storageTransport,
+              notification,
+              hmacKey,
+              projectId: CONF_TEST_PROJECT_ID,
+              preconditionRequired: testCase.preconditionProvided,
             };
-            if (testCase.preconditionProvided) {
-              methodParameters.preconditionRequired = true;
-            }
 
             if (testCase.expectSuccess) {
-              assert.ifError(await storageMethodObject(methodParameters));
+              await storageMethodObject(methodParameters);
+              const testBenchResult = await getTestBenchRetryTest(
+                creationResult.id,
+                storageTransport,
+              );
+              assert.strictEqual(testBenchResult.completed, true);
             } else {
               await assert.rejects(async () => {
                 await storageMethodObject(methodParameters);
               }, undefined);
             }
-
-            const testBenchResult = await getTestBenchRetryTest(
-              creationResult.id,
-              storageTransport,
-            );
-            assert.strictEqual(testBenchResult.completed, true);
           }).timeout(TIMEOUT_FOR_INDIVIDUAL_TEST);
         });
       });
     });
   }
 }
 
+/**
+ * Creates a Proxy to automatically inject x-retry-test-id into all requests
+ */
+function createRetryProxy(
+  transport: StorageTransport,
+  retryId: string,
+): StorageTransport {
+  return new Proxy(transport, {
+    get(target, prop, receiver) {
+      const original = Reflect.get(target, prop, receiver);
+      if (prop === 'makeRequest' && typeof original === 'function') {
+        return async (
+          reqOpts: StorageRequestOptions,
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          callback?: StorageTransportCallback<any>,
+        ) => {
+          reqOpts.headers = reqOpts.headers || {};
+
+          if (reqOpts.headers instanceof Headers) {
+            reqOpts.headers.set('x-retry-test-id', retryId);
+          } else {
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any
+            (reqOpts.headers as any)['x-retry-test-id'] = retryId;
+          }
+
+          return original.apply(target, [reqOpts, callback]);
+        };
+      }
+      return original;
+    },
+  });
+}
+
 async function createBucketForTest(
   storage: Storage,
-  preconditionShouldBeOnInstance: boolean,
-  storageMethodString: String,
+  withPrecondition: boolean,
+  method: String,
 ) {
-  const name = generateName(storageMethodString, 'bucket');
-  const bucket = storage.bucket(name);
-  await bucket.create();
+  const bucket = storage.bucket(generateName(method, 'bucket'));
+  const [metadata] = await bucket.create();
   await bucket.setRetentionPeriod(DURATION_SECONDS);
-
-  if (preconditionShouldBeOnInstance) {
+  if (withPrecondition) {
     return new Bucket(storage, bucket.name, {
       preconditionOpts: {
-        ifMetagenerationMatch: 2,
+        ifMetagenerationMatch: metadata.metageneration,
       },
     });
   }
   return bucket;
 }
 
 async function createFileForTest(
-  preconditionShouldBeOnInstance: boolean,
-  storageMethodString: String,
+  withPrecondition: boolean,
+  method: String,
   bucket: Bucket,
 ) {
-  const name = generateName(storageMethodString, 'file');
-  const file = bucket.file(name);
-  await file.save(name);
-  if (preconditionShouldBeOnInstance) {
+  const file = bucket.file(generateName(method, 'file'));
+  await file.save('test-content');
+  if (withPrecondition) {
+    const [metadata] = await file.getMetadata();
     return new File(bucket, file.name, {
       preconditionOpts: {
-        ifMetagenerationMatch: file.metadata.metageneration,
-        ifGenerationMatch: file.metadata.generation,
+        ifMetagenerationMatch: metadata.metageneration,
+        ifGenerationMatch: metadata.generation,
       },
     });
   }
   return file;
 }
 
-function generateName(storageMethodString: String, bucketOrFile: string) {
-  return `${TESTS_PREFIX}${storageMethodString.toLowerCase()}${bucketOrFile}.${shortUUID()}`;
-}
-
 async function createTestBenchRetryTest(
   instructions: String[],
   methodName: string,
-  storageTransport: StorageTransport,
+  transport: StorageTransport,
 ): Promise<ConformanceTestCreationResult> {
-  const requestBody = {instructions: {[methodName]: instructions}};
-
-  const requestOptions: StorageRequestOptions = {
+  return (await transport.makeRequest({
     method: 'POST',
     url: 'retry_test',
-    body: JSON.stringify(requestBody),
+    body: JSON.stringify({instructions: {[methodName]: instructions}}),
     headers: {'Content-Type': 'application/json'},
-  };
-
-  const response = await storageTransport.makeRequest(requestOptions);
-  return response as unknown as ConformanceTestCreationResult;
+  })) as ConformanceTestCreationResult;
 }
 
 async function getTestBenchRetryTest(
   testId: string,
-  storageTransport: StorageTransport,
+  transport: StorageTransport,
 ): Promise<ConformanceTestResult> {
-  const response = await storageTransport.makeRequest({
+  return (await transport.makeRequest({
     url: `retry_test/${testId}`,
     method: 'GET',
-    retry: true,
-    headers: {
-      'x-retry-test-id': testId,
-    },
-  });
-  return response as unknown as ConformanceTestResult;
+    headers: {'x-retry-test-id': testId},
+  })) as ConformanceTestResult;
+}
+
+function generateName(method: String, type: string) {
+  return `${TESTS_PREFIX}${method.toLowerCase()}${type}.${shortUUID()}`;
 }
 
 function shortUUID() {
-  return uuid.v1().split('-').shift();
+  return uuid.v4().split('-').shift();
 }