Chore: Update move logic now to support non-HNS buckets (#2666)

* Chore: Update move logic to support non-HNS buckets

The existing logic for file move operations was implicitly tied to
features available only in Hierarchical Namespace (HNS) enabled buckets.

This update refactors the underlying move implementation to correctly
handle the different API requirements and constraints of **non-HNS
buckets**.

* refactor: system test

* add more assert

* bug fix

* addressing comments

Author: thiyaguk09

samples/moveFileAtomic.js

@@ -59,7 +59,7 @@ function main(
       },
     };
 
-    // Moves the file automatically within the HNS enabled bucket
+    // Moves the file atomically within the bucket
     await storage
       .bucket(bucketName)
       .file(srcFileName)

samples/system-test/files.test.js

@@ -30,8 +30,6 @@ const storage = new Storage();
 const cwd = path.join(__dirname, '..');
 const bucketName = generateName();
 const bucket = storage.bucket(bucketName);
-const hnsBucketName = generateName();
-const hnsBucket = storage.bucket(hnsBucketName);
 const objectRetentionBucketName = generateName();
 const objectRetentionBucket = storage.bucket(objectRetentionBucketName);
 const fileContents = 'these-are-my-contents';
@@ -237,6 +235,25 @@ describe('file', () => {
     assert.strictEqual(exists, true);
   });
 
+  it('should atomically move a file', async () => {
+    const movedFileName = 'test1.txt';
+    const file = bucket.file(fileName);
+    await file.save(fileName);
+    const output = execSync(
+      `node moveFileAtomic.js ${bucketName} ${fileName} ${movedFileName} ${doesNotExistPrecondition}`
+    );
+    assert.include(
+      output,
+      `gs://${bucketName}/${fileName} moved to gs://${bucketName}/${movedFileName}`
+    );
+    const [[destExists], [sourceExists]] = await Promise.all([
+      bucket.file(movedFileName).exists(),
+      bucket.file(fileName).exists(),
+    ]);
+    assert.strictEqual(destExists, true);
+    assert.strictEqual(sourceExists, false);
+  });
+
   it('should copy a file', async () => {
     const output = execSync(
       `node copyFile.js ${bucketName} ${movedFileName} ${bucketName} ${copiedFileName} ${doesNotExistPrecondition}`
@@ -599,33 +616,6 @@ describe('file', () => {
       assert(metadata.retention.mode.toUpperCase(), 'UNLOCKED');
     });
   });
-
-  describe('HNS Bucket Move Object', () => {
-    before(async () => {
-      await storage.createBucket(hnsBucketName, {
-        hierarchicalNamespace: {enabled: true},
-        iamConfiguration: {
-          uniformBucketLevelAccess: {
-            enabled: true,
-          },
-        },
-      });
-    });
-
-    it('should move a file', async () => {
-      const file = hnsBucket.file(fileName);
-      await file.save(fileName);
-      const output = execSync(
-        `node moveFileAtomic.js ${hnsBucketName} ${fileName} ${movedFileName} ${doesNotExistPrecondition}`
-      );
-      assert.include(
-        output,
-        `gs://${hnsBucketName}/${fileName} moved to gs://${hnsBucketName}/${movedFileName}`
-      );
-      const [exists] = await hnsBucket.file(movedFileName).exists();
-      assert.strictEqual(exists, true);
-    });
-  });
 });
 
 function generateName() {

src/file.ts

@@ -3485,7 +3485,7 @@ class File extends ServiceObject<File, FileMetadata> {
    * @property {number} [preconditionOpts.ifGenerationMatch] Makes the operation conditional on whether the object's current generation matches the given value.
    */
   /**
-   * Move this file within the same HNS-enabled bucket.
+   * Move this file within the same bucket.
    * The source object must exist and be a live object.
    * The source and destination object IDs must be different.
    * Overwriting the destination object is allowed by default, but can be prevented
@@ -3507,17 +3507,17 @@ class File extends ServiceObject<File, FileMetadata> {
    * const storage = new Storage();
    *
    * //-
-   * // Assume 'my-hns-bucket' is an HNS-enabled bucket.
+   * // Assume 'my-bucket' is a bucket.
    * //-
-   * const bucket = storage.bucket('my-hns-bucket');
+   * const bucket = storage.bucket('my-bucket');
    * const file = bucket.file('my-image.png');
    *
    * //-
    * // If you pass in a string for the destination, the file is copied to its
    * // current bucket, under the new name provided.
    * //-
    * file.moveFileAtomic('moved-image.png', function(err, movedFile, apiResponse) {
-   *   // `my-hns-bucket` now contains:
+   *   // `my-bucket` now contains:
    *   // - "moved-image.png"
    *
    *   // `movedFile` is an instance of a File object that refers to your new
@@ -3528,7 +3528,7 @@ class File extends ServiceObject<File, FileMetadata> {
    * // Move the file to a subdirectory, creating parent folders if necessary.
    * //-
    * file.moveFileAtomic('new-folder/subfolder/moved-image.png', function(err, movedFile, apiResponse) {
-   * // `my-hns-bucket` now contains:
+   * // `my-bucket` now contains:
    * // - "new-folder/subfolder/moved-image.png"
    * });
    *
@@ -3557,7 +3557,7 @@ class File extends ServiceObject<File, FileMetadata> {
    *
    * ```
    * @example <caption>include:samples/files.js</caption>
-   * region_tag:storage_move_file_hns
+   * region_tag:storage_move_file
    * Another example:
    */
   moveFileAtomic(

system-test/storage.ts

@@ -1693,52 +1693,6 @@ describe('storage', function () {
       assert(metadata.hierarchicalNamespace);
       assert.strictEqual(metadata.hierarchicalNamespace.enabled, true);
     });
-
-    describe('file#moveFileAtomic', async () => {
-      let hnsBucket: Bucket;
-
-      afterEach(async () => {
-        try {
-          await hnsBucket.delete();
-        } catch {
-          //Ignore errors
-        }
-      });
-
-      it('Should move a file to a new name within the same HNS-enabled bucket.', async () => {
-        hnsBucket = storage.bucket(generateName());
-        await storage.createBucket(hnsBucket.name, {
-          hierarchicalNamespace: {enabled: true},
-          iamConfiguration: {
-            uniformBucketLevelAccess: {
-              enabled: true,
-            },
-          },
-        });
-        // Create a source file in the bucket and save some content.
-        const f1 = hnsBucket.file('move-src-obj');
-        await f1.save('move-src-obj');
-        assert(f1);
-        const [f1_metadata] = await f1.getMetadata();
-
-        // Move the source file to a new destination name within the same bucket.
-        await f1.moveFileAtomic('move-dst-obj');
-        const f2 = hnsBucket.file('move-dst-obj');
-        assert(f2);
-        const [f2_metadata] = await f2.getMetadata();
-
-        // Assert that the generation of the destination file is different from the source file,
-        // indicating a new file was created.
-        assert.notStrictEqual(f1_metadata.generation, f2_metadata.generation);
-
-        const [f1_exists] = await f1.exists();
-        const [f2_exists] = await f2.exists();
-        // Assert that the source file no longer exists after the move.
-        assert.strictEqual(f1_exists, false);
-        // Assert that the destination file exists after the move.
-        assert.strictEqual(f2_exists, true);
-      });
-    });
   });
 
   describe('bucket retention policies', () => {
@@ -3218,6 +3172,33 @@ describe('storage', function () {
     });
   });
 
+  describe('file#moveFileAtomic', async () => {
+    it('Should move a file to a new name within the bucket.', async () => {
+      // Create a source file in the bucket and save some content.
+      const f1 = bucket.file('move-src-obj');
+      await f1.save('move-src-obj');
+      assert(f1);
+      const [f1_metadata] = await f1.getMetadata();
+
+      // Move the source file to a new destination name within the same bucket.
+      await f1.moveFileAtomic('move-dst-obj');
+      const f2 = bucket.file('move-dst-obj');
+      assert(f2);
+      const [f2_metadata] = await f2.getMetadata();
+
+      // Assert that the generation of the destination file is different from the source file,
+      // indicating a new file was created.
+      assert.notStrictEqual(f1_metadata.generation, f2_metadata.generation);
+
+      const [f1_exists] = await f1.exists();
+      const [f2_exists] = await f2.exists();
+      // Assert that the source file no longer exists after the move.
+      assert.strictEqual(f1_exists, false);
+      // Assert that the destination file exists after the move.
+      assert.strictEqual(f2_exists, true);
+    });
+  });
+
   describe('combine files', () => {
     it('should combine multiple files into one', async () => {
       const files = [