googleapis / nodejs-storage

Node.js client for Google Cloud Storage: unified object storage for developers and enterprises, from live data serving to data analytics/ML to data archiving.
https://cloud.google.com/storage/
Apache License 2.0
897 stars 369 forks source link

preconditions not working #2475

Closed MattLysinger closed 4 months ago

MattLysinger commented 4 months ago

hello,

I am using the firebase emulator suite to test cloud functions that perform crud operations in cloud storage. I am using admin SDKs for all cloud functions. See below for sample code that uploads a file into cloud storage.

exports.bucketTest = onCall(
   async (request, response) => {
      const testFile = 'TEST_FILE.txt';
      const filePath = `../${testFile}`;
      const destFileName = 'this/is/a/TEST_FILE.txt';

      async function uploadFile() {
         const options = {
            destination: destFileName,
            preconditionOpts: {ifGenerationMatch: 0}
         };

         await defaultBucket.upload(filePath, options);
         console.log(`${filePath} uploaded to ${destFileName}`);
      }

      await uploadFile().catch(console.error);
   }
);

The precondition seems to be ignored. I have it set to 0 which, as i understand it, should mean that the file should only be created if no file already exist. No matter what i set the precondition to, the upload always works even when that file already exist at the destination.

After reading through the source code for the bucket.upload method ( bucket.ts ) lines 4372 through 4407 show the following code.

    // Do not retry if precondition option ifGenerationMatch is not set
    // because this is a file operation
    let maxRetries = this.storage.retryOptions.maxRetries;
    if (
      (options?.preconditionOpts?.ifGenerationMatch === undefined &&
        this.instancePreconditionOpts?.ifGenerationMatch === undefined &&
        this.storage.retryOptions.idempotencyStrategy ===
          IdempotencyStrategy.RetryConditional) ||
      this.storage.retryOptions.idempotencyStrategy ===
        IdempotencyStrategy.RetryNever
    ) {
      maxRetries = 0;
    }

    let newFile: File;
    if (options.destination instanceof File) {
      newFile = options.destination;
    } else if (
      options.destination !== null &&
      typeof options.destination === 'string'
    ) {
      // Use the string as the name of the file.
      newFile = this.file(options.destination, {
        encryptionKey: options.encryptionKey,
        kmsKeyName: options.kmsKeyName,
        preconditionOpts: this.instancePreconditionOpts,
      });
    } else {
      // Resort to using the name of the incoming file.
      const destination = path.basename(pathString);
      newFile = this.file(destination, {
        encryptionKey: options.encryptionKey,
        kmsKeyName: options.kmsKeyName,
        preconditionOpts: this.instancePreconditionOpts,
      });
    }

The code shown above from bucket.ts is the only place in the upload method where preconditions are mentioned and, while i am not 100% sure, it seems that the preconditions only serve to set the maxRetries variable. I posted this as a question vs a bug because i am not sure if what i just said is accurate.

The screenshot below is taken from the guide on preconditions ( guide ).

Capture

Could someone please let me know if preconditions work with the nodejs sdk and, if so, how they can be implemented in the code that i am using (shown in the first code block).

cojenco commented 4 months ago

I tested against live GCS server the bucket.upload() code sample with ifGenerationMatch:0 and can confirm that preconditions work with the nodejs storage sdk.

I was able to see the 412 Precondition Failed error when attempting to upload an existing file with the precondition set

{
  code: 412,
  message: 'At least one of the pre-conditions you specified did not hold.',
  errors: [
    {
      message: 'At least one of the pre-conditions you specified did not hold.',
      domain: 'global',
      reason: 'conditionNotMet',
      locationType: 'header',
      location: 'If-Match'
    }
  ]
}

However, I haven't had the chance to test against firebase emulator and am not sure if this is fully supported in the firebase emulator. If you haven't already, please raise this question in the firebase tools repository.

MattLysinger commented 4 months ago

Thank you for testing that, i appreciate it.

I will direct this issue to the firebase tools repo.