From 21f173eb17d4216e2b42ffdd1ed0104aeda7cf13 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 10 Aug 2022 07:37:05 -0700 Subject: [PATCH] feat: add functionality for passing preconditions at the function level (#1993) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: updated conformance tests for precondition updates * fixed typo * updated functions to pass preconditions * removed IAM and HMAC test changes * implemented local preconditions for bucketmakeprivate * added preconditions to enableLogging * linted files * implemented more preconditions * general cleanup * implemented precondition on combine * added preconditions for copy and move * rename * removed tests for instance precondition where instance precondition is not supported * support preconditions for rotateencryptionkey * support set storage class * linted files * fixed tests * deleteLabels and setLabels * more precondition implementations * minor progress * fix(refactor): Simplify logic around disabling autoretry for setmetadata * setcorsconfiguration * set retention period * bucketSetStorageClass * fileMakePrivate * file set metadata * bucket set metadata * file delete * more precondition updates * precondition refactor * removed log statement * change delete labels signature * fix delete labels * fixed save multipart * put docker code back * linted files * docs and cleanup * refactored conformance tests * remove iam test from being in the conformance tests * linted files * put docker commands back * fixed combine retries * added comments * fix: implement setMetadata in HmacKey and fix associated tests (#2009) * fix: implement setMetadata in HmacKey and fix associated tests * fix merge problem, check idempotency strategy * retry based on idempotency strategy * linted file * Revert "retry based on idempotency strategy" This reverts commit 80909b5e48c20d298d8a9315742a490e7b86a8f1. * don't retry acl adds * changed HEAD request to GET request * fix(refactor): Add a call from file.delete to the parent class delete (#2014) * fix(refactor): Add a call from file.delete to the parent class delete * add delete to checked methods for conditionally idempotent file ops * fix: fix noResponseRetries so it respects reqOpts.maxRetries (#2015) * fix: fix noResponseRetries so it respects reqOpts.maxRetries * fix situation where err.code is actually a string during connection resets * log error type * removed passing functions * restored retryInvocationMap * added instance precondition back to insert * restored scenario 2 * restored all scenarios * linted files * Revert "linted files" This reverts commit d2cb27b0abd4ce0594e6ded97953b3d2696a16cf. * removed logs Co-authored-by: Sameena Shaffeeullah * fix: pass appropriate preconditions from enableLogging to setMetadata (#2018) * tests: remove callback waterfall from make bucket private system test (#2020) * tests: remove callback waterfall from make bucket private system test * cleaner implementation * moved done() * removed precondition from policyoptions * added retries for setPolicy * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * removed unused import Co-authored-by: Denis DelGrosso Co-authored-by: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> Co-authored-by: Owl Bot --- conformance-test/conformanceCommon.ts | 74 +- conformance-test/libraryMethods.ts | 782 +++++++++++------ .../test-data/retryInvocationMap.json | 23 +- .../test-data/retryStrategyTestData.json | 812 +++++++++--------- src/acl.ts | 1 + src/bucket.ts | 461 ++++++---- src/file.ts | 127 ++- src/hmacKey.ts | 77 +- src/iam.ts | 6 + src/nodejs-common/service-object.ts | 8 +- src/nodejs-common/util.ts | 5 +- src/storage.ts | 21 +- system-test/storage.ts | 21 +- test/bucket.ts | 233 ++--- test/file.ts | 32 +- test/hmacKey.ts | 31 +- test/iam.ts | 1 + 17 files changed, 1646 insertions(+), 1069 deletions(-) diff --git a/conformance-test/conformanceCommon.ts b/conformance-test/conformanceCommon.ts index c442aa2f6..a08f1301e 100644 --- a/conformance-test/conformanceCommon.ts +++ b/conformance-test/conformanceCommon.ts @@ -92,16 +92,29 @@ export function executeScenario(testCase: RetryTestCase) { instructionSet.instructions, jsonMethod?.name.toString() ); - bucket = await createBucketForTest( - storage, - testCase.preconditionProvided, - 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(); @@ -121,29 +134,20 @@ export function executeScenario(testCase: RetryTestCase) { }); it(`${instructionNumber}`, async () => { + const methodParameters: libraryMethods.ConformanceTestOptions = { + bucket: bucket, + file: file, + notification: notification, + storage: storage, + hmacKey: hmacKey, + }; + if (testCase.preconditionProvided) { + methodParameters.preconditionRequired = true; + } if (testCase.expectSuccess) { - assert.ifError( - await storageMethodObject( - bucket, - file, - notification, - storage, - hmacKey - ) - ); + assert.ifError(await storageMethodObject(methodParameters)); } else { - try { - await storageMethodObject( - bucket, - file, - notification, - storage, - hmacKey - ); - throw Error(`${storageMethodString} was supposed to throw.`); - } catch (e) { - assert.notStrictEqual(e, undefined); - } + await assert.rejects(storageMethodObject(methodParameters)); } const testBenchResult = await getTestBenchRetryTest( creationResult.id @@ -158,7 +162,7 @@ export function executeScenario(testCase: RetryTestCase) { async function createBucketForTest( storage: Storage, - preconditionProvided: boolean, + preconditionShouldBeOnInstance: boolean, storageMethodString: String ) { const name = generateName(storageMethodString, 'bucket'); @@ -166,7 +170,7 @@ async function createBucketForTest( await bucket.create(); await bucket.setRetentionPeriod(DURATION_SECONDS); - if (preconditionProvided) { + if (preconditionShouldBeOnInstance) { return new Bucket(storage, bucket.name, { preconditionOpts: { ifMetagenerationMatch: 2, @@ -177,14 +181,14 @@ async function createBucketForTest( } async function createFileForTest( - preconditionProvided: boolean, + preconditionShouldBeOnInstance: boolean, storageMethodString: String, bucket: Bucket ) { const name = generateName(storageMethodString, 'file'); const file = bucket.file(name); await file.save(name); - if (preconditionProvided) { + if (preconditionShouldBeOnInstance) { return new File(bucket, file.name, { preconditionOpts: { ifMetagenerationMatch: file.metadata.metageneration, diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 6dfa7e284..01a35fe50 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -12,16 +12,27 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {Bucket, File, Notification, Storage, HmacKey} from '../src'; +import {Bucket, File, Notification, Storage, HmacKey, Policy} from '../src'; import * as path from 'path'; import {ApiError} from '../src/nodejs-common'; +export interface ConformanceTestOptions { + bucket?: Bucket; + file?: File; + notification?: Notification; + storage?: Storage; + hmacKey?: HmacKey; + preconditionRequired?: boolean; +} + ///////////////////////////////////////////////// //////////////////// BUCKET ///////////////////// ///////////////////////////////////////////////// -export async function addLifecycleRule(bucket: Bucket) { - await bucket.addLifecycleRule({ +export async function addLifecycleRuleInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.addLifecycleRule({ action: 'delete', condition: { age: 365 * 3, // Specified in days. @@ -29,160 +40,345 @@ export async function addLifecycleRule(bucket: Bucket) { }); } -export async function combine(bucket: Bucket) { - bucket = new Bucket(bucket.storage, bucket.name, { - preconditionOpts: { - ifMetagenerationMatch: 1, - }, - }); - const file1 = bucket.file('file1.txt'); - const file2 = bucket.file('file2.txt'); +export async function addLifecycleRule(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.addLifecycleRule( + { + action: 'delete', + condition: { + age: 365 * 3, // Specified in days. + }, + }, + { + ifMetagenerationMatch: 2, + } + ); + } else { + await options.bucket!.addLifecycleRule({ + action: 'delete', + condition: { + age: 365 * 3, // Specified in days. + }, + }); + } +} + +export async function combineInstancePrecondition( + options: ConformanceTestOptions +) { + const file1 = options.bucket!.file('file1.txt'); + const file2 = options.bucket!.file('file2.txt'); await file1.save('file1 contents'); await file2.save('file2 contents'); - const f1WithPrecondition = new File(file1.bucket, file1.name, { - preconditionOpts: { - ifGenerationMatch: file1.metadata.generation, - }, - }); - const f2WithPrecondition = new File(file2.bucket, file2.name, { - preconditionOpts: { - ifGenerationMatch: file2.metadata.generation, - }, - }); - const sources = [f1WithPrecondition, f2WithPrecondition]; - const allFiles = bucket.file('all-files.txt'); - // If we are using a precondition we must make sure the file exists and the metageneration matches that provided as a query parameter + let allFiles; + const sources = [file1, file2]; + if (options.preconditionRequired) { + allFiles = options.bucket!.file('all-files.txt', { + preconditionOpts: { + ifGenerationMatch: 0, + }, + }); + } else { + allFiles = options.bucket!.file('all-files.txt'); + } + + await options.bucket!.combine(sources, allFiles); +} + +export async function combine(options: ConformanceTestOptions) { + const file1 = options.bucket!.file('file1.txt'); + const file2 = options.bucket!.file('file2.txt'); + await file1.save('file1 contents'); + await file2.save('file2 contents'); + const sources = [file1, file2]; + const allFiles = options.bucket!.file('all-files.txt'); await allFiles.save('allfiles contents'); - await bucket.combine(sources, allFiles); + if (options.preconditionRequired) { + await options.bucket!.combine(sources, allFiles, { + ifGenerationMatch: allFiles.metadata.generation, + }); + } else { + await options.bucket!.combine(sources, allFiles); + } } -export async function create(bucket: Bucket) { - const [bucketExists] = await bucket.exists(); +export async function create(options: ConformanceTestOptions) { + const [bucketExists] = await options.bucket!.exists(); if (bucketExists) { - await bucket.deleteFiles(); - await bucket.delete({ + await options.bucket!.deleteFiles(); + await options.bucket!.delete({ ignoreNotFound: true, }); } - await bucket.create(); + await options.bucket!.create(); } -export async function createNotification(bucket: Bucket) { - await bucket.createNotification('my-topic'); +export async function createNotification(options: ConformanceTestOptions) { + await options.bucket!.createNotification('my-topic'); } -export async function deleteBucket(bucket: Bucket) { - await bucket.deleteFiles(); - await bucket.delete(); +export async function deleteBucket(options: ConformanceTestOptions) { + await options.bucket!.deleteFiles(); + await options.bucket!.delete(); } -export async function deleteFiles(bucket: Bucket) { - await bucket.deleteFiles(); +// Note: bucket.deleteFiles is missing from these tests +// Preconditions cannot be implemented with current setup. + +export async function deleteLabelsInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.deleteLabels(); } -export async function deleteLabels(bucket: Bucket) { - await bucket.deleteLabels(); +export async function deleteLabels(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.deleteLabels({ + ifMetagenerationMatch: 2, + }); + } else { + await options.bucket!.deleteLabels(); + } } -export async function disableRequesterPays(bucket: Bucket) { - await bucket.disableRequesterPays(); +export async function disableRequesterPaysInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.disableRequesterPays(); } -export async function enableLogging(bucket: Bucket) { +export async function disableRequesterPays(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.disableRequesterPays({ + ifMetagenerationMatch: 2, + }); + } else { + await options.bucket!.disableRequesterPays(); + } +} + +export async function enableLoggingInstancePrecondition( + options: ConformanceTestOptions +) { const config = { prefix: 'log', }; - await bucket.enableLogging(config); + await options.bucket!.enableLogging(config); +} + +export async function enableLogging(options: ConformanceTestOptions) { + let config; + if (options.preconditionRequired) { + config = { + prefix: 'log', + ifMetagenerationMatch: 2, + }; + } else { + config = { + prefix: 'log', + }; + } + await options.bucket!.enableLogging(config); } -export async function enableRequesterPays(bucket: Bucket) { - await bucket.enableRequesterPays(); +export async function enableRequesterPaysInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.enableRequesterPays(); +} + +export async function enableRequesterPays(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.enableRequesterPays({ + ifMetagenerationMatch: 2, + }); + } else { + await options.bucket!.enableRequesterPays(); + } } -export async function bucketExists(bucket: Bucket) { - await bucket.exists(); +export async function bucketExists(options: ConformanceTestOptions) { + await options.bucket!.exists(); } -export async function bucketGet(bucket: Bucket) { - await bucket.get(); +export async function bucketGet(options: ConformanceTestOptions) { + await options.bucket!.get(); } -export async function getFilesStream(bucket: Bucket) { +export async function getFilesStream(options: ConformanceTestOptions) { return new Promise((resolve, reject) => { - bucket - .getFilesStream() + options + .bucket!.getFilesStream() .on('data', () => {}) .on('end', () => resolve(undefined)) .on('error', (err: ApiError) => reject(err)); }); } -export async function getLabels(bucket: Bucket) { - await bucket.getLabels(); +export async function getLabels(options: ConformanceTestOptions) { + await options.bucket!.getLabels(); } -export async function bucketGetMetadata(bucket: Bucket) { - await bucket.getMetadata(); +export async function bucketGetMetadata(options: ConformanceTestOptions) { + await options.bucket!.getMetadata(); } -export async function getNotifications(bucket: Bucket) { - await bucket.getNotifications(); +export async function getNotifications(options: ConformanceTestOptions) { + await options.bucket!.getNotifications(); } -export async function lock(bucket: Bucket) { +export async function lock(options: ConformanceTestOptions) { const metageneration = 0; - await bucket.lock(metageneration); + await options.bucket!.lock(metageneration); +} + +export async function bucketMakePrivateInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.makePrivate(); } -export async function bucketMakePrivate(bucket: Bucket) { - await bucket.makePrivate(); +export async function bucketMakePrivate(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.makePrivate({ + preconditionOpts: {ifMetagenerationMatch: 2}, + }); + } else { + await options.bucket!.makePrivate(); + } +} + +export async function bucketMakePublic(options: ConformanceTestOptions) { + await options.bucket!.makePublic(); +} + +export async function removeRetentionPeriodInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.removeRetentionPeriod(); } -export async function bucketMakePublic(bucket: Bucket) { - await bucket.makePublic(); +export async function removeRetentionPeriod(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.removeRetentionPeriod({ + ifMetagenerationMatch: 2, + }); + } else { + await options.bucket!.removeRetentionPeriod(); + } } -export async function removeRetentionPeriod(bucket: Bucket) { - await bucket.removeRetentionPeriod(); +export async function setCorsConfigurationInstancePrecondition( + options: ConformanceTestOptions +) { + const corsConfiguration = [{maxAgeSeconds: 3600}]; // 1 hour + await options.bucket!.setCorsConfiguration(corsConfiguration); } -export async function setCorsConfiguration(bucket: Bucket) { +export async function setCorsConfiguration(options: ConformanceTestOptions) { const corsConfiguration = [{maxAgeSeconds: 3600}]; // 1 hour - await bucket.setCorsConfiguration(corsConfiguration); + if (options.preconditionRequired) { + await options.bucket!.setCorsConfiguration(corsConfiguration, { + ifMetagenerationMatch: 2, + }); + } else { + await options.bucket!.setCorsConfiguration(corsConfiguration); + } } -export async function setLabels(bucket: Bucket) { +export async function setLabelsInstancePrecondition( + options: ConformanceTestOptions +) { const labels = { labelone: 'labelonevalue', labeltwo: 'labeltwovalue', }; - await bucket.setLabels(labels); + await options.bucket!.setLabels(labels); } -export async function bucketSetMetadata(bucket: Bucket) { +export async function setLabels(options: ConformanceTestOptions) { + const labels = { + labelone: 'labelonevalue', + labeltwo: 'labeltwovalue', + }; + if (options.preconditionRequired) { + await options.bucket!.setLabels(labels, { + ifMetagenerationMatch: 2, + }); + } else { + await options.bucket!.setLabels(labels); + } +} + +export async function bucketSetMetadataInstancePrecondition( + options: ConformanceTestOptions +) { const metadata = { website: { mainPageSuffix: 'http://example.com', notFoundPage: 'http://example.com/404.html', }, }; - await bucket.setMetadata(metadata); + await options.bucket!.setMetadata(metadata); } -export async function setRetentionPeriod(bucket: Bucket) { +export async function bucketSetMetadata(options: ConformanceTestOptions) { + const metadata = { + website: { + mainPageSuffix: 'http://example.com', + notFoundPage: 'http://example.com/404.html', + }, + }; + if (options.preconditionRequired) { + await options.bucket!.setMetadata(metadata, { + ifMetagenerationMatch: 2, + }); + } else { + await options.bucket!.setMetadata(metadata); + } +} + +export async function setRetentionPeriodInstancePrecondition( + options: ConformanceTestOptions +) { const DURATION_SECONDS = 15780000; // 6 months. - await bucket.setRetentionPeriod(DURATION_SECONDS); + await options.bucket!.setRetentionPeriod(DURATION_SECONDS); +} + +export async function setRetentionPeriod(options: ConformanceTestOptions) { + const DURATION_SECONDS = 15780000; // 6 months. + if (options.preconditionRequired) { + await options.bucket!.setRetentionPeriod(DURATION_SECONDS, { + ifMetagenerationMatch: 2, + }); + } else { + await options.bucket!.setRetentionPeriod(DURATION_SECONDS); + } } -export async function bucketSetStorageClass(bucket: Bucket) { - await bucket.setStorageClass('nearline'); +export async function bucketSetStorageClassInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.setStorageClass('nearline'); +} + +export async function bucketSetStorageClass(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.setStorageClass('nearline', { + ifMetagenerationMatch: 2, + }); + } else { + await options.bucket!.setStorageClass('nearline'); + } } -export async function bucketUploadResumable(bucket: Bucket) { - if (bucket.instancePreconditionOpts) { - bucket.instancePreconditionOpts.ifGenerationMatch = 0; +export async function bucketUploadResumableInstancePrecondition( + options: ConformanceTestOptions +) { + if (options.bucket!.instancePreconditionOpts) { + options.bucket!.instancePreconditionOpts.ifGenerationMatch = 0; } - await bucket.upload( + await options.bucket!.upload( path.join( __dirname, '../../conformance-test/test-data/retryStrategyTestData.json' @@ -191,12 +387,33 @@ export async function bucketUploadResumable(bucket: Bucket) { ); } -export async function bucketUploadMultipart(bucket: Bucket) { - if (bucket.instancePreconditionOpts) { - delete bucket.instancePreconditionOpts.ifMetagenerationMatch; - bucket.instancePreconditionOpts.ifGenerationMatch = 0; +export async function bucketUploadResumable(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.upload( + path.join( + __dirname, + '../../conformance-test/test-data/retryStrategyTestData.json' + ), + {preconditionOpts: {ifMetagenerationMatch: 2, ifGenerationMatch: 0}} + ); + } else { + await options.bucket!.upload( + path.join( + __dirname, + '../../conformance-test/test-data/retryStrategyTestData.json' + ) + ); } - await bucket.upload( +} + +export async function bucketUploadMultipartInstancePrecondition( + options: ConformanceTestOptions +) { + if (options.bucket!.instancePreconditionOpts) { + delete options.bucket!.instancePreconditionOpts.ifMetagenerationMatch; + options.bucket!.instancePreconditionOpts.ifGenerationMatch = 0; + } + await options.bucket!.upload( path.join( __dirname, '../../conformance-test/test-data/retryStrategyTestData.json' @@ -205,90 +422,217 @@ export async function bucketUploadMultipart(bucket: Bucket) { ); } +export async function bucketUploadMultipart(options: ConformanceTestOptions) { + if (options.bucket!.instancePreconditionOpts) { + delete options.bucket!.instancePreconditionOpts.ifMetagenerationMatch; + } + + if (options.preconditionRequired) { + await options.bucket!.upload( + path.join( + __dirname, + '../../conformance-test/test-data/retryStrategyTestData.json' + ), + {resumable: false, preconditionOpts: {ifGenerationMatch: 0}} + ); + } else { + await options.bucket!.upload( + path.join( + __dirname, + '../../conformance-test/test-data/retryStrategyTestData.json' + ), + {resumable: false} + ); + } +} + ///////////////////////////////////////////////// //////////////////// FILE ///////////////////// ///////////////////////////////////////////////// -export async function copy(_bucket: Bucket, file: File) { - await file.copy('a-different-file.png'); +export async function copy(options: ConformanceTestOptions) { + const newFile = new File(options.bucket!, 'a-different-file.png'); + await newFile.save('a-different-file.png'); + + if (options.preconditionRequired) { + await options.file!.copy('a-different-file.png', { + preconditionOpts: {ifGenerationMatch: newFile.metadata.generation}, + }); + } else { + await options.file!.copy('a-different-file.png'); + } } -export async function createReadStream(_bucket: Bucket, file: File) { +export async function createReadStream(options: ConformanceTestOptions) { return new Promise((resolve, reject) => { - file - .createReadStream() + options + .file!.createReadStream() .on('data', () => {}) .on('end', () => resolve(undefined)) .on('error', (err: ApiError) => reject(err)); }); } -export async function createResumableUpload(_bucket: Bucket, file: File) { - await file.createResumableUpload(); +export async function createResumableUploadInstancePrecondition( + options: ConformanceTestOptions +) { + await options.file!.createResumableUpload(); } -export async function fileDelete(_bucket: Bucket, file: File) { - await file.delete(); +export async function createResumableUpload(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.file!.createResumableUpload({ + preconditionOpts: {ifGenerationMatch: 0}, + }); + } else { + await options.file!.createResumableUpload(); + } } -export async function download(_bucket: Bucket, file: File) { - await file.download(); +export async function fileDeleteInstancePrecondition( + options: ConformanceTestOptions +) { + await options.file!.delete(); } -export async function exists(_bucket: Bucket, file: File) { - await file.exists(); +export async function fileDelete(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.file!.delete({ + ifGenerationMatch: options.file!.metadata.generation, + }); + } else { + await options.file!.delete(); + } } -export async function get(_bucket: Bucket, file: File) { - await file.get(); +export async function download(options: ConformanceTestOptions) { + await options.file!.download(); } -export async function getExpirationDate(bucket: Bucket, file: File) { - await file.getExpirationDate(); +export async function exists(options: ConformanceTestOptions) { + await options.file!.exists(); } -export async function getMetadata(_bucket: Bucket, file: File) { - await file.getMetadata(); +export async function get(options: ConformanceTestOptions) { + await options.file!.get(); } -export async function isPublic(_bucket: Bucket, file: File) { - await file.isPublic(); +export async function getExpirationDate(options: ConformanceTestOptions) { + await options.file!.getExpirationDate(); } -export async function fileMakePrivate(_bucket: Bucket, file: File) { - await file.makePrivate(); +export async function getMetadata(options: ConformanceTestOptions) { + await options.file!.getMetadata(); } -export async function fileMakePublic(_bucket: Bucket, file: File) { - await file.makePublic(); +export async function isPublic(options: ConformanceTestOptions) { + await options.file!.isPublic(); } -export async function move(_bucket: Bucket, file: File) { - await file.move('new-file'); +export async function fileMakePrivateInstancePrecondition( + options: ConformanceTestOptions +) { + await options.file!.makePrivate(); } -export async function rename(_bucket: Bucket, file: File) { - await file.rename('new-name'); +export async function fileMakePrivate(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.file!.makePrivate({ + preconditionOpts: { + ifGenerationMatch: options.file!.metadata.generation, + }, + }); + } else { + await options.file!.makePrivate(); + } } -export async function rotateEncryptionKey(_bucket: Bucket, file: File) { +export async function fileMakePublic(options: ConformanceTestOptions) { + await options.file!.makePublic(); +} + +export async function move(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.file!.move('new-file', { + preconditionOpts: {ifGenerationMatch: 0}, + }); + } else { + await options.file!.move('new-file'); + } +} + +export async function rename(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.file!.rename('new-name', { + preconditionOpts: {ifGenerationMatch: 0}, + }); + } else { + await options.file!.rename('new-name'); + } +} + +export async function rotateEncryptionKey(options: ConformanceTestOptions) { const crypto = require('crypto'); const buffer = crypto.randomBytes(32); const newKey = buffer.toString('base64'); - await file.rotateEncryptionKey({ - encryptionKey: Buffer.from(newKey, 'base64'), - }); + if (options.preconditionRequired) { + await options.file!.rotateEncryptionKey({ + encryptionKey: Buffer.from(newKey, 'base64'), + preconditionOpts: {ifGenerationMatch: options.file!.metadata.generation}, + }); + } else { + await options.file!.rotateEncryptionKey({ + encryptionKey: Buffer.from(newKey, 'base64'), + }); + } +} + +export async function saveResumableInstancePrecondition( + options: ConformanceTestOptions +) { + await options.file!.save('testdata', {resumable: true}); } -export async function saveResumable(_bucket: Bucket, file: File) { - await file.save('testdata', {resumable: true}); +export async function saveResumable(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.file!.save('testdata', { + resumable: true, + preconditionOpts: { + ifGenerationMatch: options.file!.metadata.generation, + ifMetagenerationMatch: options.file!.metadata.metageneration, + }, + }); + } else { + await options.file!.save('testdata', { + resumable: true, + }); + } } -export async function saveMultipart(_bucket: Bucket, file: File) { - await file.save('testdata', {resumable: false}); +export async function saveMultipartInstancePrecondition( + options: ConformanceTestOptions +) { + await options.file!.save('testdata', {resumable: false}); +} + +export async function saveMultipart(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.file!.save('testdata', { + resumable: false, + preconditionOpts: { + ifGenerationMatch: options.file!.metadata.generation, + }, + }); + } else { + await options.file!.save('testdata', { + resumable: false, + }); + } } -export async function setMetadata(_bucket: Bucket, file: File) { +export async function setMetadataInstancePrecondition( + options: ConformanceTestOptions +) { const metadata = { contentType: 'application/x-font-ttf', metadata: { @@ -296,13 +640,35 @@ export async function setMetadata(_bucket: Bucket, file: File) { properties: 'go here', }, }; - await file.setMetadata(metadata); + await options.file!.setMetadata(metadata); } -export async function setStorageClass(_bucket: Bucket, file: File) { - const result = await file.setStorageClass('nearline'); - if (!result) { - throw Error(); +export async function setMetadata(options: ConformanceTestOptions) { + const metadata = { + contentType: 'application/x-font-ttf', + metadata: { + my: 'custom', + properties: 'go here', + }, + }; + if (options.preconditionRequired) { + await options.file!.setMetadata(metadata, { + ifGenerationMatch: options.file!.metadata.generation, + }); + } else { + await options.file!.setMetadata(metadata); + } +} + +export async function setStorageClass(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.file!.setStorageClass('nearline', { + preconditionOpts: { + ifGenerationMatch: options.file!.metadata.generation, + }, + }); + } else { + await options.file!.setStorageClass('nearline'); } } @@ -310,63 +676,39 @@ export async function setStorageClass(_bucket: Bucket, file: File) { // /////////////////// HMAC KEY //////////////////// // ///////////////////////////////////////////////// -export async function deleteHMAC( - _bucket: Bucket, - file: File, - _notification: Notification, - _storage: Storage, - hmacKey: HmacKey -) { +export async function deleteHMAC(options: ConformanceTestOptions) { const metadata = { state: 'INACTIVE', }; - hmacKey.setMetadata(metadata); - await hmacKey.delete(); + await options.hmacKey!.setMetadata(metadata); + await options.hmacKey!.delete(); } -export async function getHMAC( - _bucket: Bucket, - file: File, - _notification: Notification, - _storage: Storage, - hmacKey: HmacKey -) { - await hmacKey.get(); +export async function getHMAC(options: ConformanceTestOptions) { + await options.hmacKey!.get(); } -export async function getMetadataHMAC( - _bucket: Bucket, - file: File, - _notification: Notification, - _storage: Storage, - hmacKey: HmacKey -) { - await hmacKey.getMetadata(); +export async function getMetadataHMAC(options: ConformanceTestOptions) { + await options.hmacKey!.getMetadata(); } -export async function setMetadataHMAC( - _bucket: Bucket, - file: File, - _notification: Notification, - _storage: Storage, - hmacKey: HmacKey -) { +export async function setMetadataHMAC(options: ConformanceTestOptions) { const metadata = { state: 'INACTIVE', }; - await hmacKey.setMetadata(metadata); + await options.hmacKey!.setMetadata(metadata); } ///////////////////////////////////////////////// ////////////////////// IAM ////////////////////// ///////////////////////////////////////////////// -export async function iamGetPolicy(bucket: Bucket) { - await bucket.iam.getPolicy({requestedPolicyVersion: 1}); +export async function iamGetPolicy(options: ConformanceTestOptions) { + await options.bucket!.iam.getPolicy({requestedPolicyVersion: 1}); } -export async function iamSetPolicy(bucket: Bucket) { - const testPolicy = { +export async function iamSetPolicy(options: ConformanceTestOptions) { + const testPolicy: Policy = { bindings: [ { role: 'roles/storage.admin', @@ -374,130 +716,84 @@ export async function iamSetPolicy(bucket: Bucket) { }, ], }; - await bucket.iam.setPolicy(testPolicy); + if (options.preconditionRequired) { + const currentPolicy = await options.bucket!.iam.getPolicy(); + testPolicy.etag = currentPolicy[0].etag; + } + await options.bucket!.iam.setPolicy(testPolicy); } -export async function iamTestPermissions(bucket: Bucket) { +export async function iamTestPermissions(options: ConformanceTestOptions) { const permissionToTest = 'storage.buckets.delete'; - await bucket.iam.testPermissions(permissionToTest); + await options.bucket!.iam.testPermissions(permissionToTest); } ///////////////////////////////////////////////// ///////////////// NOTIFICATION ////////////////// ///////////////////////////////////////////////// -export async function notificationDelete( - _bucket: Bucket, - _file: File, - notification: Notification -) { - await notification.delete(); +export async function notificationDelete(options: ConformanceTestOptions) { + await options.notification!.delete(); } -export async function notificationCreate( - _bucket: Bucket, - _file: File, - notification: Notification -) { - await notification.create(); +export async function notificationCreate(options: ConformanceTestOptions) { + await options.notification!.create(); } -export async function notificationExists( - _bucket: Bucket, - _file: File, - notification: Notification -) { - await notification.exists(); +export async function notificationExists(options: ConformanceTestOptions) { + await options.notification!.exists(); } -export async function notificationGet( - _bucket: Bucket, - _file: File, - notification: Notification -) { - await notification.get(); +export async function notificationGet(options: ConformanceTestOptions) { + await options.notification!.get(); } -export async function notificationGetMetadata( - _bucket: Bucket, - _file: File, - notification: Notification -) { - await notification.getMetadata(); +export async function notificationGetMetadata(options: ConformanceTestOptions) { + await options.notification!.getMetadata(); } ///////////////////////////////////////////////// /////////////////// STORAGE ///////////////////// ///////////////////////////////////////////////// -export async function createBucket( - _bucket: Bucket, - _file: File, - _notification: Notification, - storage: Storage -) { - const bucket = storage.bucket('test-creating-bucket'); +export async function createBucket(options: ConformanceTestOptions) { + const bucket = options.storage!.bucket('test-creating-bucket'); const [exists] = await bucket.exists(); if (exists) { bucket.delete(); } - await storage.createBucket('test-creating-bucket'); + await options.storage!.createBucket('test-creating-bucket'); } -export async function createHMACKey( - _bucket: Bucket, - _file: File, - _notification: Notification, - storage: Storage -) { +export async function createHMACKey(options: ConformanceTestOptions) { const serviceAccountEmail = 'my-service-account@appspot.gserviceaccount.com'; - await storage.createHmacKey(serviceAccountEmail); + await options.storage!.createHmacKey(serviceAccountEmail); } -export async function getBuckets( - _bucket: Bucket, - _file: File, - _notification: Notification, - storage: Storage -) { - await storage.getBuckets(); +export async function getBuckets(options: ConformanceTestOptions) { + await options.storage!.getBuckets(); } -export async function getBucketsStream( - _bucket: Bucket, - _file: File, - _notification: Notification, - storage: Storage -) { +export async function getBucketsStream(options: ConformanceTestOptions) { return new Promise((resolve, reject) => { - storage - .getBucketsStream() + options + .storage!.getBucketsStream() .on('data', () => {}) .on('end', () => resolve(undefined)) .on('error', err => reject(err)); }); } -export function getHMACKeyStream( - _bucket: Bucket, - _file: File, - _notification: Notification, - storage: Storage -) { +export function getHMACKeyStream(options: ConformanceTestOptions) { return new Promise((resolve, reject) => { - storage - .getHmacKeysStream() + options + .storage!.getHmacKeysStream() .on('data', () => {}) .on('end', () => resolve(undefined)) .on('error', err => reject(err)); }); } -export async function getServiceAccount( - _bucket: Bucket, - _file: File, - _notification: Notification, - storage: Storage -) { - await storage.getServiceAccount(); +export async function getServiceAccount(options: ConformanceTestOptions) { + await options.storage!.getServiceAccount(); } diff --git a/conformance-test/test-data/retryInvocationMap.json b/conformance-test/test-data/retryInvocationMap.json index e92e7abd7..63dfb0032 100644 --- a/conformance-test/test-data/retryInvocationMap.json +++ b/conformance-test/test-data/retryInvocationMap.json @@ -3,6 +3,18 @@ "deleteBucket" ], "storage.buckets.patch": [ + "addLifecycleRuleInstancePrecondition", + "bucketMakePrivateInstancePrecondition", + "deleteLabelsInstancePrecondition", + "disableRequesterPaysInstancePrecondition", + "enableRequesterPaysInstancePrecondition", + "enableLoggingInstancePrecondition", + "removeRetentionPeriodInstancePrecondition", + "setCorsConfigurationInstancePrecondition", + "setLabelsInstancePrecondition", + "setRetentionPeriodInstancePrecondition", + "bucketSetStorageClassInstancePrecondition", + "bucketSetMetadataInstancePrecondition", "addLifecycleRule", "bucketMakePrivate", "deleteLabels", @@ -36,6 +48,8 @@ "lock" ], "storage.objects.patch": [ + "fileMakePrivateInstancePrecondition", + "setMetadataInstancePrecondition", "fileMakePrivate", "setMetadata" ], @@ -53,6 +67,11 @@ "setStorageClass" ], "storage.objects.insert": [ + "bucketUploadResumableInstancePrecondition", + "saveResumableInstancePrecondition", + "bucketUploadMultipartInstancePrecondition", + "saveMultipartInstancePrecondition", + "createResumableUploadInstancePrecondition", "bucketUploadResumable", "saveResumable", "bucketUploadMultipart", @@ -60,7 +79,7 @@ "createResumableUpload" ], "storage.objects.delete": [ - "deleteFiles", + "fileDeleteInstancePrecondition", "fileDelete" ], "storage.objects.get": [ @@ -98,6 +117,7 @@ "getBucketsStream" ], "storage.objects.compose": [ + "combineInstancePrecondition", "combine" ], "storage.hmacKey.delete": [ @@ -108,7 +128,6 @@ "getMetadataHMAC" ], "storage.hmacKey.update": [ - "setMetadataHMAC" ], "storage.hmacKey.create": [ "createHMACKey" diff --git a/conformance-test/test-data/retryStrategyTestData.json b/conformance-test/test-data/retryStrategyTestData.json index 2817ea4fe..c1c30b51b 100644 --- a/conformance-test/test-data/retryStrategyTestData.json +++ b/conformance-test/test-data/retryStrategyTestData.json @@ -1,407 +1,407 @@ { - "retryStrategyTests": [ - { - "id": 1, - "description": "always idempotent", - "cases": [ - { - "instructions": [ - "return-503", - "return-503" - ] - } - ], - "methods": [ - { - "name": "storage.bucket_acl.get", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.bucket_acl.list", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.buckets.delete", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.buckets.get", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.buckets.getIamPolicy", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.buckets.insert", - "resources": [] - }, - { - "name": "storage.buckets.list", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.buckets.lockRetentionPolicy", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.buckets.testIamPermissions", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.default_object_acl.get", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.default_object_acl.list", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.hmacKey.delete", - "resources": [] - }, - { - "name": "storage.hmacKey.get", - "resources": [] - }, - { - "name": "storage.hmacKey.list", - "resources": [] - }, - { - "name": "storage.notifications.delete", - "resources": [ - "BUCKET", - "NOTIFICATION" - ] - }, - { - "name": "storage.notifications.get", - "resources": [ - "BUCKET", - "NOTIFICATION" - ] - }, - { - "name": "storage.notifications.list", - "resources": [ - "BUCKET", - "NOTIFICATION" - ] - }, - { - "name": "storage.object_acl.get", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.object_acl.list", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.objects.get", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.objects.list", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.serviceaccount.get", - "resources": [] - } - ], - "preconditionProvided": false, - "expectSuccess": true - }, - { - "id": 2, - "description": "conditionally idempotent retries when precondition is present", - "cases": [ - { - "instructions": [ - "return-503", - "return-503" - ] - } - ], - "methods": [ - { - "name": "storage.buckets.patch", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.buckets.setIamPolicy", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.buckets.update", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.hmacKey.update", - "resources": [] - }, - { - "name": "storage.objects.compose", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.objects.copy", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.objects.delete", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.objects.insert", - "resources": [ - "BUCKET" - ] - }, - { - "name": "storage.objects.patch", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.objects.rewrite", - "resources": [ - "BUCKET", - "OBJECT" - ] - }, - { - "name": "storage.objects.update", - "resources": [ - "BUCKET", - "OBJECT" - ] - } - ], - "preconditionProvided": true, - "expectSuccess": true - }, - { - "id": 3, - "description": "conditionally_idempotent_no_retries_when_precondition_is_absent", - "cases": [ - { - "instructions": ["return-503"] - }, - { - "instructions": ["return-reset-connection"] - } - ], - "methods": [ - {"name": "storage.buckets.patch", "resources": ["BUCKET"]}, - {"name": "storage.buckets.setIamPolicy", "resources": ["BUCKET"]}, - {"name": "storage.buckets.update", "resources": ["BUCKET"]}, - {"name": "storage.hmacKey.update", "resources": ["HMAC_KEY"]}, - {"name": "storage.objects.compose", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.copy", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.delete", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.insert", "resources": ["BUCKET"]}, - {"name": "storage.objects.patch", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.rewrite", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.update", "resources": ["BUCKET", "OBJECT"]} - ], - "preconditionProvided": false, - "expectSuccess": false - }, - { - "id": 4, - "description": "non_idempotent", - "cases": [ - { - "instructions": ["return-503"] - }, - { - "instructions": ["return-reset-connection"] - } - ], - "methods": [ - {"name": "storage.bucket_acl.delete", "resources": ["BUCKET"]}, - {"name": "storage.bucket_acl.insert", "resources": ["BUCKET"]}, - {"name": "storage.bucket_acl.patch", "resources": ["BUCKET"]}, - {"name": "storage.bucket_acl.update", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.delete", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.insert", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.patch", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.update", "resources": ["BUCKET"]}, - {"name": "storage.hmacKey.create", "resources": []}, - {"name": "storage.notifications.insert", "resources": ["BUCKET"]}, - {"name": "storage.object_acl.delete", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.object_acl.insert", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.object_acl.patch", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.object_acl.update", "resources": ["BUCKET", "OBJECT"]} - ], - "preconditionProvided": false, - "expectSuccess": false - }, - { - "id": 5, - "description": "non_retryable_errors", - "cases": [ - { - "instructions": ["return-400"] - }, - { - "instructions": ["return-401"] - } - ], - "methods": [ - {"name": "storage.bucket_acl.delete", "resources": ["BUCKET"]}, - {"name": "storage.bucket_acl.get", "resources": ["BUCKET"]}, - {"name": "storage.bucket_acl.insert", "resources": ["BUCKET"]}, - {"name": "storage.bucket_acl.list", "resources": ["BUCKET"]}, - {"name": "storage.bucket_acl.patch", "resources": ["BUCKET"]}, - {"name": "storage.bucket_acl.update", "resources": ["BUCKET"]}, - {"name": "storage.buckets.delete", "resources": ["BUCKET"]}, - {"name": "storage.buckets.get", "resources": ["BUCKET"]}, - {"name": "storage.buckets.getIamPolicy", "resources": ["BUCKET"]}, - {"name": "storage.buckets.insert", "resources": ["BUCKET"]}, - {"name": "storage.buckets.list", "resources": ["BUCKET"]}, - {"name": "storage.buckets.lockRetentionPolicy", "resources": ["BUCKET"]}, - {"name": "storage.buckets.patch", "resources": ["BUCKET"]}, - {"name": "storage.buckets.setIamPolicy", "resources": ["BUCKET"]}, - {"name": "storage.buckets.testIamPermissions", "resources": ["BUCKET"]}, - {"name": "storage.buckets.update", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.delete", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.get", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.insert", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.list", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.patch", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.update", "resources": ["BUCKET"]}, - {"name": "storage.hmacKey.create", "resources": []}, - {"name": "storage.hmacKey.delete", "resources": ["HMAC_KEY"]}, - {"name": "storage.hmacKey.get", "resources": ["HMAC_KEY"]}, - {"name": "storage.hmacKey.list", "resources": ["HMAC_KEY"]}, - {"name": "storage.hmacKey.update", "resources": ["HMAC_KEY"]}, - {"name": "storage.notifications.delete", "resources": ["BUCKET", "NOTIFICATION"]}, - {"name": "storage.notifications.get", "resources": ["BUCKET", "NOTIFICATION"]}, - {"name": "storage.notifications.insert", "resources": ["BUCKET", "NOTIFICATION"]}, - {"name": "storage.notifications.list", "resources": ["BUCKET", "NOTIFICATION"]}, - {"name": "storage.object_acl.delete", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.object_acl.get", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.object_acl.insert", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.object_acl.list", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.object_acl.patch", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.object_acl.update", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.compose", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.copy", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.delete", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.get", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.insert", "resources": ["BUCKET"]}, - {"name": "storage.objects.list", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.patch", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.rewrite", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.update", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.serviceaccount.get", "resources": []} - ], - "preconditionProvided": false, - "expectSuccess": false - }, - { - "id": 6, - "description": "mix_retryable_non_retryable_errors", - "cases": [ - { - "instructions": ["return-503", "return-400"] - }, - { - "instructions": ["return-reset-connection", "return-401"] - } - ], - "methods": [ - {"name": "storage.bucket_acl.get", "resources": ["BUCKET"]}, - {"name": "storage.bucket_acl.list", "resources": ["BUCKET"]}, - {"name": "storage.buckets.delete", "resources": ["BUCKET"]}, - {"name": "storage.buckets.get", "resources": ["BUCKET"]}, - {"name": "storage.buckets.getIamPolicy", "resources": ["BUCKET"]}, - {"name": "storage.buckets.insert", "resources": []}, - {"name": "storage.buckets.list", "resources": ["BUCKET"]}, - {"name": "storage.buckets.lockRetentionPolicy", "resources": ["BUCKET"]}, - {"name": "storage.buckets.patch", "resources": ["BUCKET"]}, - {"name": "storage.buckets.setIamPolicy", "resources": ["BUCKET"]}, - {"name": "storage.buckets.testIamPermissions", "resources": ["BUCKET"]}, - {"name": "storage.buckets.update", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.get", "resources": ["BUCKET"]}, - {"name": "storage.default_object_acl.list", "resources": ["BUCKET"]}, - {"name": "storage.hmacKey.delete", "resources": ["HMAC_KEY"]}, - {"name": "storage.hmacKey.get", "resources": ["HMAC_KEY"]}, - {"name": "storage.hmacKey.list", "resources": ["HMAC_KEY"]}, - {"name": "storage.hmacKey.update", "resources": ["HMAC_KEY"]}, - {"name": "storage.notifications.delete", "resources": ["BUCKET", "NOTIFICATION"]}, - {"name": "storage.notifications.get", "resources": ["BUCKET", "NOTIFICATION"]}, - {"name": "storage.notifications.list", "resources": ["BUCKET", "NOTIFICATION"]}, - {"name": "storage.object_acl.get", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.object_acl.list", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.compose", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.copy", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.delete", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.get", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.list", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.insert", "resources": ["BUCKET"]}, - {"name": "storage.objects.patch", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.rewrite", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.objects.update", "resources": ["BUCKET", "OBJECT"]}, - {"name": "storage.serviceaccount.get", "resources": []} - ], - "preconditionProvided": true, - "expectSuccess": false - } - ] - } \ No newline at end of file + "retryStrategyTests": [ + { + "id": 1, + "description": "always idempotent", + "cases": [ + { + "instructions": [ + "return-503", + "return-503" + ] + } + ], + "methods": [ + { + "name": "storage.bucket_acl.get", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.bucket_acl.list", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.buckets.delete", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.buckets.get", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.buckets.getIamPolicy", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.buckets.insert", + "resources": [] + }, + { + "name": "storage.buckets.list", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.buckets.lockRetentionPolicy", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.buckets.testIamPermissions", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.default_object_acl.get", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.default_object_acl.list", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.hmacKey.delete", + "resources": [] + }, + { + "name": "storage.hmacKey.get", + "resources": [] + }, + { + "name": "storage.hmacKey.list", + "resources": [] + }, + { + "name": "storage.notifications.delete", + "resources": [ + "BUCKET", + "NOTIFICATION" + ] + }, + { + "name": "storage.notifications.get", + "resources": [ + "BUCKET", + "NOTIFICATION" + ] + }, + { + "name": "storage.notifications.list", + "resources": [ + "BUCKET", + "NOTIFICATION" + ] + }, + { + "name": "storage.object_acl.get", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.object_acl.list", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.objects.get", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.objects.list", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.serviceaccount.get", + "resources": [] + } + ], + "preconditionProvided": false, + "expectSuccess": true + }, + { + "id": 2, + "description": "conditionally idempotent retries when precondition is present", + "cases": [ + { + "instructions": [ + "return-503", + "return-503" + ] + } + ], + "methods": [ + { + "name": "storage.buckets.patch", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.buckets.setIamPolicy", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.buckets.update", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.hmacKey.update", + "resources": [] + }, + { + "name": "storage.objects.compose", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.objects.copy", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.objects.delete", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.objects.insert", + "resources": [ + "BUCKET" + ] + }, + { + "name": "storage.objects.patch", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.objects.rewrite", + "resources": [ + "BUCKET", + "OBJECT" + ] + }, + { + "name": "storage.objects.update", + "resources": [ + "BUCKET", + "OBJECT" + ] + } + ], + "preconditionProvided": true, + "expectSuccess": true + }, + { + "id": 3, + "description": "conditionally_idempotent_no_retries_when_precondition_is_absent", + "cases": [ + { + "instructions": ["return-503"] + }, + { + "instructions": ["return-reset-connection"] + } + ], + "methods": [ + {"name": "storage.buckets.patch", "resources": ["BUCKET"]}, + {"name": "storage.buckets.setIamPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.update", "resources": ["BUCKET"]}, + {"name": "storage.hmacKey.update", "resources": ["HMAC_KEY"]}, + {"name": "storage.objects.compose", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.copy", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.delete", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.insert", "resources": ["BUCKET"]}, + {"name": "storage.objects.patch", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.rewrite", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.update", "resources": ["BUCKET", "OBJECT"]} + ], + "preconditionProvided": false, + "expectSuccess": false + }, + { + "id": 4, + "description": "non_idempotent", + "cases": [ + { + "instructions": ["return-503"] + }, + { + "instructions": ["return-reset-connection"] + } + ], + "methods": [ + {"name": "storage.bucket_acl.delete", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.insert", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.patch", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.update", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.delete", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.insert", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.patch", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.update", "resources": ["BUCKET"]}, + {"name": "storage.hmacKey.create", "resources": []}, + {"name": "storage.notifications.insert", "resources": ["BUCKET"]}, + {"name": "storage.object_acl.delete", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.insert", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.patch", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.update", "resources": ["BUCKET", "OBJECT"]} + ], + "preconditionProvided": false, + "expectSuccess": false + }, + { + "id": 5, + "description": "non_retryable_errors", + "cases": [ + { + "instructions": ["return-400"] + }, + { + "instructions": ["return-401"] + } + ], + "methods": [ + {"name": "storage.bucket_acl.delete", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.get", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.insert", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.list", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.patch", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.update", "resources": ["BUCKET"]}, + {"name": "storage.buckets.delete", "resources": ["BUCKET"]}, + {"name": "storage.buckets.get", "resources": ["BUCKET"]}, + {"name": "storage.buckets.getIamPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.insert", "resources": ["BUCKET"]}, + {"name": "storage.buckets.list", "resources": ["BUCKET"]}, + {"name": "storage.buckets.lockRetentionPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.patch", "resources": ["BUCKET"]}, + {"name": "storage.buckets.setIamPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.testIamPermissions", "resources": ["BUCKET"]}, + {"name": "storage.buckets.update", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.delete", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.get", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.insert", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.list", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.patch", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.update", "resources": ["BUCKET"]}, + {"name": "storage.hmacKey.create", "resources": []}, + {"name": "storage.hmacKey.delete", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.get", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.list", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.update", "resources": ["HMAC_KEY"]}, + {"name": "storage.notifications.delete", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.notifications.get", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.notifications.insert", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.notifications.list", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.object_acl.delete", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.get", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.insert", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.list", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.patch", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.update", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.compose", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.copy", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.delete", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.get", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.insert", "resources": ["BUCKET"]}, + {"name": "storage.objects.list", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.patch", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.rewrite", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.update", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.serviceaccount.get", "resources": []} + ], + "preconditionProvided": false, + "expectSuccess": false + }, + { + "id": 6, + "description": "mix_retryable_non_retryable_errors", + "cases": [ + { + "instructions": ["return-503", "return-400"] + }, + { + "instructions": ["return-reset-connection", "return-401"] + } + ], + "methods": [ + {"name": "storage.bucket_acl.get", "resources": ["BUCKET"]}, + {"name": "storage.bucket_acl.list", "resources": ["BUCKET"]}, + {"name": "storage.buckets.delete", "resources": ["BUCKET"]}, + {"name": "storage.buckets.get", "resources": ["BUCKET"]}, + {"name": "storage.buckets.getIamPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.insert", "resources": []}, + {"name": "storage.buckets.list", "resources": ["BUCKET"]}, + {"name": "storage.buckets.lockRetentionPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.patch", "resources": ["BUCKET"]}, + {"name": "storage.buckets.setIamPolicy", "resources": ["BUCKET"]}, + {"name": "storage.buckets.testIamPermissions", "resources": ["BUCKET"]}, + {"name": "storage.buckets.update", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.get", "resources": ["BUCKET"]}, + {"name": "storage.default_object_acl.list", "resources": ["BUCKET"]}, + {"name": "storage.hmacKey.delete", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.get", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.list", "resources": ["HMAC_KEY"]}, + {"name": "storage.hmacKey.update", "resources": ["HMAC_KEY"]}, + {"name": "storage.notifications.delete", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.notifications.get", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.notifications.list", "resources": ["BUCKET", "NOTIFICATION"]}, + {"name": "storage.object_acl.get", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.object_acl.list", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.compose", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.copy", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.delete", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.get", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.list", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.insert", "resources": ["BUCKET"]}, + {"name": "storage.objects.patch", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.rewrite", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.objects.update", "resources": ["BUCKET", "OBJECT"]}, + {"name": "storage.serviceaccount.get", "resources": []} + ], + "preconditionProvided": true, + "expectSuccess": false + } + ] +} \ No newline at end of file diff --git a/src/acl.ts b/src/acl.ts index e7e0c3456..e7fbc2088 100644 --- a/src/acl.ts +++ b/src/acl.ts @@ -510,6 +510,7 @@ class Acl extends AclRoleAccessorMethods { method: 'POST', uri: '', qs: query, + maxRetries: 0, //explicitly set this value since this is a non-idempotent function json: { entity: options.entity, role: options.role.toUpperCase(), diff --git a/src/bucket.ts b/src/bucket.ts index 8512652c7..994199d47 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -20,8 +20,10 @@ import { ExistsCallback, GetConfig, Metadata, + MetadataCallback, ResponseBody, ServiceObject, + SetMetadataResponse, util, } from './nodejs-common'; import {paginator} from '@google-cloud/paginator'; @@ -65,6 +67,7 @@ import { import {Readable} from 'stream'; import {CRC32CValidatorGenerator} from './crc32c'; import {URL} from 'url'; +import {SetMetadataOptions} from './nodejs-common/service-object'; interface SourceObject { name: string; @@ -78,6 +81,10 @@ interface CreateNotificationQuery { interface MetadataOptions { predefinedAcl: string; userProject?: string; + ifGenerationMatch?: number; + ifGenerationNotMatch?: number; + ifMetagenerationMatch?: number; + ifMetagenerationNotMatch?: number; } export type GetFilesResponse = [File[], {}, Metadata]; @@ -100,7 +107,7 @@ interface WatchAllOptions { versions?: boolean; } -export interface AddLifecycleRuleOptions { +export interface AddLifecycleRuleOptions extends PreconditionOptions { append?: boolean; } @@ -110,7 +117,7 @@ export interface LifecycleRule { storageClass?: string; } -export interface EnableLoggingOptions { +export interface EnableLoggingOptions extends PreconditionOptions { bucket?: string | Bucket; prefix: string; } @@ -197,6 +204,10 @@ export type DeleteLabelsResponse = [Metadata]; export type DeleteLabelsCallback = SetLabelsCallback; +export type DeleteLabelsOptions = PreconditionOptions; + +export type DisableRequesterPaysOptions = PreconditionOptions; + export type DisableRequesterPaysResponse = [Metadata]; export interface DisableRequesterPaysCallback { @@ -209,6 +220,7 @@ export interface EnableRequesterPaysCallback { (err?: Error | null, apiResponse?: Metadata): void; } +export type EnableRequesterPaysOptions = PreconditionOptions; export interface BucketExistsOptions extends GetConfig { userProject?: string; } @@ -289,6 +301,7 @@ export interface MakeBucketPrivateOptions { force?: boolean; metadata?: Metadata; userProject?: string; + preconditionOpts?: PreconditionOptions; } interface MakeBucketPrivateRequest extends MakeBucketPrivateOptions { @@ -312,7 +325,7 @@ export interface MakeBucketPublicCallback { export type MakeBucketPublicResponse = [File[]]; -export interface SetBucketMetadataOptions { +export interface SetBucketMetadataOptions extends PreconditionOptions { userProject?: string; } @@ -332,7 +345,7 @@ export interface Labels { [key: string]: string; } -export interface SetLabelsOptions { +export interface SetLabelsOptions extends PreconditionOptions { userProject?: string; } @@ -342,7 +355,7 @@ export interface SetLabelsCallback { (err?: Error | null, metadata?: Metadata): void; } -export interface SetBucketStorageClassOptions { +export interface SetBucketStorageClassOptions extends PreconditionOptions { userProject?: string; } @@ -1316,7 +1329,7 @@ class Bucket extends ServiceObject { optionsOrCallback?: AddLifecycleRuleOptions | SetBucketMetadataCallback, callback?: SetBucketMetadataCallback ): Promise | void { - let options; + let options: AddLifecycleRuleOptions = {}; if (typeof optionsOrCallback === 'function') { callback = optionsOrCallback; @@ -1359,18 +1372,12 @@ class Bucket extends ServiceObject { return apiFormattedRule; }); - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); - if (options.append === false) { - this.setMetadata({lifecycle: {rule: newLifecycleRules}}) - .then(resp => callback!(null, ...resp)) - .catch(callback!) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + this.setMetadata( + {lifecycle: {rule: newLifecycleRules}}, + options, + callback! + ); return; } @@ -1386,16 +1393,15 @@ class Bucket extends ServiceObject { metadata.lifecycle && metadata.lifecycle.rule ); - this.setMetadata({ - lifecycle: { - rule: currentLifecycleRules.concat(newLifecycleRules), + this.setMetadata( + { + lifecycle: { + rule: currentLifecycleRules.concat(newLifecycleRules), + }, }, - }) - .then(resp => callback!(null, ...resp)) - .catch(callback!) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + options as AddLifecycleRuleOptions, + callback! + ); }); } @@ -1506,6 +1512,12 @@ class Bucket extends ServiceObject { options = optionsOrCallback; } + this.disableAutoRetryConditionallyIdempotent_( + this.methods.setMetadata, // Not relevant but param is required + AvailableServiceObjectMethods.setMetadata, // Same as above + options + ); + const convertToFile = (file: string | File): File => { if (file instanceof File) { return file; @@ -1527,19 +1539,21 @@ class Bucket extends ServiceObject { } let maxRetries = this.storage.retryOptions.maxRetries; - (sources as File[]).forEach(source => { - if ( - (source?.instancePreconditionOpts?.ifGenerationMatch === undefined && - this.storage.retryOptions.idempotencyStrategy === - IdempotencyStrategy.RetryConditional) || + if ( + (destinationFile?.instancePreconditionOpts?.ifGenerationMatch === + undefined && + options.ifGenerationMatch === undefined && this.storage.retryOptions.idempotencyStrategy === - IdempotencyStrategy.RetryNever - ) { - maxRetries = 0; - } - }); + IdempotencyStrategy.RetryConditional) || + this.storage.retryOptions.idempotencyStrategy === + IdempotencyStrategy.RetryNever + ) { + maxRetries = 0; + } - Object.assign(options, this.instancePreconditionOpts, options); + if (options.ifGenerationMatch === undefined) { + Object.assign(options, destinationFile.instancePreconditionOpts, options); + } // Make the request from the destination File object. destinationFile.request( @@ -1556,13 +1570,8 @@ class Bucket extends ServiceObject { name: source.name, } as SourceObject; - if ( - source?.metadata?.generation || - source?.instancePreconditionOpts?.ifGenerationMatch - ) { - sourceObject.generation = - source?.metadata?.generation || - source?.instancePreconditionOpts?.ifGenerationMatch; + if (source.metadata && source.metadata.generation) { + sourceObject.generation = source.metadata.generation; } return sourceObject; @@ -1571,6 +1580,7 @@ class Bucket extends ServiceObject { qs: options, }, (err, resp) => { + this.storage.retryOptions.autoRetry = this.instanceRetryValue; if (err) { callback!(err, null, resp); return; @@ -1943,6 +1953,9 @@ class Bucket extends ServiceObject { * `{ force: true }` to suppress the errors until all files have had a chance * to be processed. * + * File preconditions cannot be passed to this function. It will not retry unless + * the idempotency strategy is set to retry always. + * * The `query` object passed as the first argument will also be passed to * {@link Bucket#getFiles}. * @@ -2031,8 +2044,18 @@ class Bucket extends ServiceObject { } deleteLabels(labels?: string | string[]): Promise; + deleteLabels(options: DeleteLabelsOptions): Promise; deleteLabels(callback: DeleteLabelsCallback): void; + deleteLabels( + labels: string | string[], + options: DeleteLabelsOptions + ): Promise; deleteLabels(labels: string | string[], callback: DeleteLabelsCallback): void; + deleteLabels( + labels: string | string[], + options: DeleteLabelsOptions, + callback: DeleteLabelsCallback + ): void; /** * @typedef {array} DeleteLabelsResponse * @property {object} 0 The full API response. @@ -2048,6 +2071,7 @@ class Bucket extends ServiceObject { * @param {string|string[]} [labels] The labels to delete. If no labels are * provided, all of the labels are removed. * @param {DeleteLabelsCallback} [callback] Callback function. + * @param {DeleteLabelsOptions} [options] Options, including precondition options * @returns {Promise} * * @example @@ -2083,14 +2107,32 @@ class Bucket extends ServiceObject { * ``` */ deleteLabels( - labelsOrCallback?: string | string[] | DeleteLabelsCallback, + labelsOrCallbackOrOptions?: + | string + | string[] + | DeleteLabelsCallback + | DeleteLabelsOptions, + optionsOrCallback?: DeleteLabelsCallback | DeleteLabelsOptions, callback?: DeleteLabelsCallback ): Promise | void { let labels = new Array(); - if (typeof labelsOrCallback === 'function') { - callback = labelsOrCallback; - } else if (labelsOrCallback) { - labels = arrify(labelsOrCallback); + let options: DeleteLabelsOptions = {}; + + if (typeof labelsOrCallbackOrOptions === 'function') { + callback = labelsOrCallbackOrOptions; + } else if ( + typeof labelsOrCallbackOrOptions === 'string' || + Array.isArray(labelsOrCallbackOrOptions) + ) { + labels = arrify(labelsOrCallbackOrOptions); + } else if (labelsOrCallbackOrOptions) { + options = labelsOrCallbackOrOptions; + } + + if (typeof optionsOrCallback === 'function') { + callback = optionsOrCallback; + } else if (optionsOrCallback) { + options = optionsOrCallback; } const deleteLabels = (labels: string[]) => { @@ -2099,7 +2141,11 @@ class Bucket extends ServiceObject { return nullLabelMap; }, {}); - this.setLabels(nullLabelMap, callback!); + if (options?.ifMetagenerationMatch !== undefined) { + this.setLabels(nullLabelMap, options, callback!); + } else { + this.setLabels(nullLabelMap, callback!); + } }; if (labels.length === 0) { @@ -2115,8 +2161,14 @@ class Bucket extends ServiceObject { } } - disableRequesterPays(): Promise; + disableRequesterPays( + options?: DisableRequesterPaysOptions + ): Promise; disableRequesterPays(callback: DisableRequesterPaysCallback): void; + disableRequesterPays( + options: DisableRequesterPaysOptions, + callback: DisableRequesterPaysCallback + ): void; /** * @typedef {array} DisableRequesterPaysResponse * @property {object} 0 The full API response. @@ -2137,6 +2189,7 @@ class Bucket extends ServiceObject { * Disable `requesterPays` functionality from this bucket. * * @param {DisableRequesterPaysCallback} [callback] Callback function. + * @param {DisableRequesterPaysOptions} [options] Options, including precondition options * @returns {Promise} * * @example @@ -2164,24 +2217,27 @@ class Bucket extends ServiceObject { * Example of disabling requester pays: */ disableRequesterPays( + optionsOrCallback?: + | DisableRequesterPaysOptions + | DisableRequesterPaysCallback, callback?: DisableRequesterPaysCallback ): Promise | void { - const cb = callback || util.noop; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); + let options: DisableRequesterPaysOptions = {}; + if (typeof optionsOrCallback === 'function') { + callback = optionsOrCallback; + } else if (optionsOrCallback) { + options = optionsOrCallback; + } - this.setMetadata({ - billing: { - requesterPays: false, + this.setMetadata( + { + billing: { + requesterPays: false, + }, }, - }) - .then(resp => cb(null, ...resp)) - .catch(cb) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + options, + callback! + ); } enableLogging( @@ -2265,9 +2321,14 @@ class Bucket extends ServiceObject { ? (config.bucket as Bucket).id || config.bucket : this.id; + const options: PreconditionOptions = {}; + if (config?.ifMetagenerationMatch) { + options.ifMetagenerationMatch = config.ifMetagenerationMatch; + } + if (config?.ifMetagenerationNotMatch) { + options.ifMetagenerationNotMatch = config.ifMetagenerationNotMatch; + } (async () => { - let setMetadataResponse; - try { const [policy] = await this.iam.getPolicy(); policy.bindings.push({ @@ -2275,29 +2336,32 @@ class Bucket extends ServiceObject { role: 'roles/storage.objectCreator', }); await this.iam.setPolicy(policy); - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); - [setMetadataResponse] = await this.setMetadata({ - logging: { - logBucket, - logObjectPrefix: config.prefix, + this.setMetadata( + { + logging: { + logBucket, + logObjectPrefix: config.prefix, + }, }, - }); + options, + callback! + ); } catch (e) { callback!(e as Error); return; - } finally { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; } - - callback!(null, setMetadataResponse); })(); } - enableRequesterPays(): Promise; + enableRequesterPays( + options?: EnableRequesterPaysOptions + ): Promise; enableRequesterPays(callback: EnableRequesterPaysCallback): void; + enableRequesterPays( + options: EnableRequesterPaysOptions, + callback: EnableRequesterPaysCallback + ): void; + /** * @typedef {array} EnableRequesterPaysResponse * @property {object} 0 The full API response. @@ -2319,7 +2383,8 @@ class Bucket extends ServiceObject { * bucket owner, to have the requesting user assume the charges for the access * to your bucket and its contents. * - * @param {EnableRequesterPaysCallback} [callback] Callback function. + * @param {EnableRequesterPaysCallback | EnableRequesterPaysOptions} [optionsOrCallback] + * Callback function or precondition options. * @returns {Promise} * * @example @@ -2347,23 +2412,27 @@ class Bucket extends ServiceObject { * Example of enabling requester pays: */ enableRequesterPays( - callback?: EnableRequesterPaysCallback + optionsOrCallback?: + | EnableRequesterPaysCallback + | EnableRequesterPaysOptions, + cb?: EnableRequesterPaysCallback ): Promise | void { - const cb = callback || util.noop; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); - this.setMetadata({ - billing: { - requesterPays: true, + let options: EnableRequesterPaysOptions = {}; + if (typeof optionsOrCallback === 'function') { + cb = optionsOrCallback; + } else if (optionsOrCallback) { + options = optionsOrCallback; + } + + this.setMetadata( + { + billing: { + requesterPays: true, + }, }, - }) - .then(resp => cb(null, ...resp)) - .catch(cb) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + options, + cb! + ); } /** @@ -3154,29 +3223,45 @@ class Bucket extends ServiceObject { query.userProject = options.userProject; } - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); + if (options.preconditionOpts?.ifGenerationMatch) { + query.ifGenerationMatch = options.preconditionOpts.ifGenerationMatch; + } + + if (options.preconditionOpts?.ifGenerationNotMatch) { + query.ifGenerationNotMatch = + options.preconditionOpts.ifGenerationNotMatch; + } + + if (options.preconditionOpts?.ifMetagenerationMatch) { + query.ifMetagenerationMatch = + options.preconditionOpts.ifMetagenerationMatch; + } + + if (options.preconditionOpts?.ifMetagenerationNotMatch) { + query.ifMetagenerationNotMatch = + options.preconditionOpts.ifMetagenerationNotMatch; + } // You aren't allowed to set both predefinedAcl & acl properties on a bucket // so acl must explicitly be nullified. const metadata = extend({}, options.metadata, {acl: null}); - this.setMetadata(metadata, query) - .then(() => { + this.setMetadata(metadata, query, err => { + if (err) { + callback!(err); + } + const internalCall = () => { if (options.includeFiles) { return promisify( this.makeAllFilesPublicPrivate_ ).call(this, options); } - return []; - }) - .then(files => callback!(null, files)) - .catch(callback!) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + return Promise.resolve([] as File[]); + }; + internalCall() + .then(files => callback!(null, files)) + .catch(callback!); + }); } makePublic( @@ -3336,13 +3421,20 @@ class Bucket extends ServiceObject { return new Notification(this, id); } - removeRetentionPeriod(): Promise; + removeRetentionPeriod( + options?: SetBucketMetadataOptions + ): Promise; removeRetentionPeriod(callback: SetBucketMetadataCallback): void; + removeRetentionPeriod( + options: SetBucketMetadataOptions, + callback: SetBucketMetadataCallback + ): void; /** * Remove an already-existing retention policy from this bucket, if it is not * locked. * * @param {SetBucketMetadataCallback} [callback] Callback function. + * @param {SetBucketMetadataOptions} [options] Options, including precondition options * @returns {Promise} * * @example @@ -3361,21 +3453,21 @@ class Bucket extends ServiceObject { * ``` */ removeRetentionPeriod( + optionsOrCallback?: SetBucketMetadataOptions | SetBucketMetadataCallback, callback?: SetBucketMetadataCallback ): Promise | void { - const cb = callback || util.noop; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + callback = + typeof optionsOrCallback === 'function' ? optionsOrCallback : callback; + + this.setMetadata( + { + retentionPolicy: null, + }, + options, + callback! ); - this.setMetadata({ - retentionPolicy: null, - }) - .then(resp => cb(null, ...resp)) - .catch(cb) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); } request(reqOpts: DecorateRequestOptions): Promise<[ResponseBody, Metadata]>; @@ -3476,21 +3568,57 @@ class Bucket extends ServiceObject { callback = callback || util.noop; + this.setMetadata({labels}, options, callback); + } + + setMetadata( + metadata: Metadata, + options?: SetMetadataOptions + ): Promise; + setMetadata(metadata: Metadata, callback: MetadataCallback): void; + setMetadata( + metadata: Metadata, + options: SetMetadataOptions, + callback: MetadataCallback + ): void; + setMetadata( + metadata: Metadata, + optionsOrCallback: SetMetadataOptions | MetadataCallback, + cb?: MetadataCallback + ): Promise | void { + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + cb = + typeof optionsOrCallback === 'function' + ? (optionsOrCallback as MetadataCallback) + : cb; + this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + AvailableServiceObjectMethods.setMetadata, + options ); - this.setMetadata({labels}, options) - .then(resp => callback!(null, ...resp)) - .catch(callback!) + + super + .setMetadata(metadata, options) + .then(resp => cb!(null, ...resp)) + .catch(cb!) .finally(() => { this.storage.retryOptions.autoRetry = this.instanceRetryValue; }); } - setRetentionPeriod(duration: number): Promise; setRetentionPeriod( duration: number, + options?: SetBucketMetadataOptions + ): Promise; + setRetentionPeriod( + duration: number, + callback: SetBucketMetadataCallback + ): void; + setRetentionPeriod( + duration: number, + options: SetBucketMetadataOptions, callback: SetBucketMetadataCallback ): void; /** @@ -3508,6 +3636,7 @@ class Bucket extends ServiceObject { * @param {*} duration In seconds, the minimum retention time for all objects * contained in this bucket. * @param {SetBucketMetadataCallback} [callback] Callback function. + * @param {SetBucketMetadataCallback} [options] Options, including precondition options. * @returns {Promise} * * @example @@ -3532,32 +3661,37 @@ class Bucket extends ServiceObject { */ setRetentionPeriod( duration: number, + optionsOrCallback?: SetBucketMetadataOptions | SetBucketMetadataCallback, callback?: SetBucketMetadataCallback ): Promise | void { - const cb = callback || util.noop; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); - this.setMetadata({ - retentionPolicy: { - retentionPeriod: duration, + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + callback = + typeof optionsOrCallback === 'function' ? optionsOrCallback : callback; + this.setMetadata( + { + retentionPolicy: { + retentionPeriod: duration, + }, }, - }) - .then(resp => cb(null, ...resp)) - .catch(cb) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + options, + callback! + ); } setCorsConfiguration( - corsConfiguration: Cors[] + corsConfiguration: Cors[], + options?: SetBucketMetadataOptions ): Promise; setCorsConfiguration( corsConfiguration: Cors[], callback: SetBucketMetadataCallback ): void; + setCorsConfiguration( + corsConfiguration: Cors[], + options: SetBucketMetadataOptions, + callback: SetBucketMetadataCallback + ): void; /** * * @typedef {object} Cors @@ -3585,6 +3719,7 @@ class Bucket extends ServiceObject { * @param {string[]} [corsConfiguration.responseHeader] A header allowed for cross origin * resource sharing with this bucket. * @param {SetBucketMetadataCallback} [callback] Callback function. + * @param {SetBucketMetadataOptions} [options] Options, including precondition options. * @returns {Promise} * * @example @@ -3605,22 +3740,20 @@ class Bucket extends ServiceObject { */ setCorsConfiguration( corsConfiguration: Cors[], + optionsOrCallback?: SetBucketMetadataOptions | SetBucketMetadataCallback, callback?: SetBucketMetadataCallback ): Promise | void { - const cb = callback || util.noop; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + callback = + typeof optionsOrCallback === 'function' ? optionsOrCallback : callback; + this.setMetadata( + { + cors: corsConfiguration, + }, + options, + callback! ); - - this.setMetadata({ - cors: corsConfiguration, - }) - .then(resp => cb(null, ...resp)) - .catch(cb) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); } setStorageClass( @@ -3693,10 +3826,6 @@ class Bucket extends ServiceObject { callback = typeof optionsOrCallback === 'function' ? optionsOrCallback : callback; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); // In case we get input like `storageClass`, convert to `storage_class`. storageClass = storageClass .replace(/-/g, '_') @@ -3705,12 +3834,7 @@ class Bucket extends ServiceObject { }) .toUpperCase(); - this.setMetadata({storageClass}, options) - .then(() => callback!()) - .catch(callback!) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + this.setMetadata({storageClass}, options, callback!); } /** @@ -4229,11 +4353,14 @@ class Bucket extends ServiceObject { disableAutoRetryConditionallyIdempotent_( // eslint-disable-next-line @typescript-eslint/no-explicit-any coreOpts: any, - methodType: AvailableServiceObjectMethods + // eslint-disable-next-line @typescript-eslint/no-explicit-any + methodType: AvailableServiceObjectMethods, + localPreconditionOptions?: PreconditionOptions ): void { if ( typeof coreOpts === 'object' && coreOpts?.reqOpts?.qs?.ifMetagenerationMatch === undefined && + localPreconditionOptions?.ifMetagenerationMatch === undefined && (methodType === AvailableServiceObjectMethods.setMetadata || methodType === AvailableServiceObjectMethods.delete) && this.storage.retryOptions.idempotencyStrategy === diff --git a/src/file.ts b/src/file.ts index 9132f972d..551b2903a 100644 --- a/src/file.ts +++ b/src/file.ts @@ -18,7 +18,9 @@ import { GetConfig, Interceptor, Metadata, + MetadataCallback, ServiceObject, + SetMetadataResponse, util, } from './nodejs-common'; import {promisifyAll} from '@google-cloud/promisify'; @@ -71,6 +73,12 @@ import {HashStreamValidator} from './hash-stream-validator'; import {URL} from 'url'; import retry = require('async-retry'); +import { + DeleteCallback, + DeleteOptions, + SetMetadataOptions, +} from './nodejs-common/service-object'; +import * as r from 'teeny-request'; export type GetExpirationDateResponse = [Date]; export interface GetExpirationDateCallback { @@ -222,6 +230,7 @@ export interface MakeFilePrivateOptions { metadata?: Metadata; strict?: boolean; userProject?: string; + preconditionOpts?: PreconditionOptions; } export type MakeFilePrivateResponse = [Metadata]; @@ -264,6 +273,7 @@ export type RotateEncryptionKeyOptions = string | Buffer | EncryptionKeyOptions; export interface EncryptionKeyOptions { encryptionKey?: string | Buffer; kmsKeyName?: string; + preconditionOpts?: PreconditionOptions; } export type RotateEncryptionKeyCallback = CopyCallback; @@ -1226,6 +1236,19 @@ class File extends ServiceObject { } } + if ( + !this.shouldRetryBasedOnPreconditionAndIdempotencyStrat( + options?.preconditionOpts + ) + ) { + this.storage.retryOptions.autoRetry = false; + } + + if (options.preconditionOpts?.ifGenerationMatch !== undefined) { + query.ifGenerationMatch = options.preconditionOpts?.ifGenerationMatch; + delete options.preconditionOpts; + } + this.request( { method: 'POST', @@ -1237,6 +1260,7 @@ class File extends ServiceObject { headers, }, (err, resp) => { + this.storage.retryOptions.autoRetry = this.instanceRetryValue; if (err) { callback!(err, null, resp); return; @@ -2021,6 +2045,39 @@ class File extends ServiceObject { return stream as Writable; } + /** + * Delete the object. + * + * @param {function=} callback - The callback function. + * @param {?error} callback.err - An error returned while making this request. + * @param {object} callback.apiResponse - The full API response. + */ + delete(options?: DeleteOptions): Promise<[r.Response]>; + delete(options: DeleteOptions, callback: DeleteCallback): void; + delete(callback: DeleteCallback): void; + delete( + optionsOrCallback?: DeleteOptions | DeleteCallback, + cb?: DeleteCallback + ): Promise<[r.Response]> | void { + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + cb = typeof optionsOrCallback === 'function' ? optionsOrCallback : cb; + + this.disableAutoRetryConditionallyIdempotent_( + this.methods.delete, + AvailableServiceObjectMethods.delete, + options + ); + + super + .delete(options) + .then(resp => cb!(null, ...resp)) + .catch(cb!) + .finally(() => { + this.storage.retryOptions.autoRetry = this.instanceRetryValue; + }); + } + download(options?: DownloadOptions): Promise; download(options: DownloadOptions, callback: DownloadCallback): void; download(callback: DownloadCallback): void; @@ -2970,7 +3027,7 @@ class File extends ServiceObject { util.makeRequest( { - method: 'HEAD', + method: 'GET', uri: `${this.storage.apiEndpoint}/${ this.bucket.name }/${encodeURIComponent(this.name)}`, @@ -3070,26 +3127,21 @@ class File extends ServiceObject { // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; + if (options.preconditionOpts?.ifGenerationMatch !== undefined) { + query.ifGenerationMatch = options.preconditionOpts?.ifGenerationMatch; + delete options.preconditionOpts; + } + if (options.userProject) { query.userProject = options.userProject; } - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); - // You aren't allowed to set both predefinedAcl & acl properties on a file, // so acl must explicitly be nullified, destroying all previous acls on the // file. const metadata = extend({}, options.metadata, {acl: null}); - this.setMetadata(metadata, query) - .then(resp => callback!(null, ...resp)) - .catch(callback!) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + this.setMetadata(metadata, query, callback!); } makePublic(): Promise; @@ -3538,7 +3590,11 @@ class File extends ServiceObject { } const newFile = this.bucket.file(this.id!, options); - this.copy(newFile, callback!); + const copyOptions = + options.preconditionOpts?.ifGenerationMatch !== undefined + ? {preconditionOpts: options.preconditionOpts} + : {}; + this.copy(newFile, copyOptions, callback!); } save(data: string | Buffer, options?: SaveOptions): Promise; @@ -3665,6 +3721,44 @@ class File extends ServiceObject { .catch(callback); } } + + setMetadata( + metadata: Metadata, + options?: SetMetadataOptions + ): Promise; + setMetadata(metadata: Metadata, callback: MetadataCallback): void; + setMetadata( + metadata: Metadata, + options: SetMetadataOptions, + callback: MetadataCallback + ): void; + setMetadata( + metadata: Metadata, + optionsOrCallback: SetMetadataOptions | MetadataCallback, + cb?: MetadataCallback + ): Promise | void { + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + cb = + typeof optionsOrCallback === 'function' + ? (optionsOrCallback as MetadataCallback) + : cb; + + this.disableAutoRetryConditionallyIdempotent_( + this.methods.setMetadata, + AvailableServiceObjectMethods.setMetadata, + options + ); + + super + .setMetadata(metadata, options) + .then(resp => cb!(null, ...resp)) + .catch(cb!) + .finally(() => { + this.storage.retryOptions.autoRetry = this.instanceRetryValue; + }); + } + setStorageClass( storageClass: string, options?: SetStorageClassOptions @@ -3929,12 +4023,15 @@ class File extends ServiceObject { disableAutoRetryConditionallyIdempotent_( // eslint-disable-next-line @typescript-eslint/no-explicit-any coreOpts: any, - methodType: AvailableServiceObjectMethods + methodType: AvailableServiceObjectMethods, + localPreconditionOptions?: PreconditionOptions ): void { if ( (typeof coreOpts === 'object' && coreOpts?.reqOpts?.qs?.ifGenerationMatch === undefined && - methodType === AvailableServiceObjectMethods.setMetadata && + localPreconditionOptions?.ifGenerationMatch === undefined && + (methodType === AvailableServiceObjectMethods.setMetadata || + methodType === AvailableServiceObjectMethods.delete) && this.storage.retryOptions.idempotencyStrategy === IdempotencyStrategy.RetryConditional) || this.storage.retryOptions.idempotencyStrategy === diff --git a/src/hmacKey.ts b/src/hmacKey.ts index 296f39978..33f11431a 100644 --- a/src/hmacKey.ts +++ b/src/hmacKey.ts @@ -12,8 +12,16 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {Metadata, ServiceObject, Methods} from './nodejs-common'; -import {Storage} from './storage'; +import { + Metadata, + ServiceObject, + Methods, + MetadataCallback, + SetMetadataResponse, +} from './nodejs-common'; +import {SetMetadataOptions} from './nodejs-common/service-object'; +import {IdempotencyStrategy, Storage} from './storage'; +import {promisifyAll} from '@google-cloud/promisify'; export interface HmacKeyOptions { projectId?: string; @@ -68,6 +76,14 @@ export type HmacKeyMetadataResponse = [HmacKeyMetadata, Metadata]; */ export class HmacKey extends ServiceObject { metadata: HmacKeyMetadata | undefined; + /** + * A reference to the {@link Storage} associated with this {@link HmacKey} + * instance. + * @name HmacKey#storage + * @type {Storage} + */ + storage: Storage; + private instanceRetryValue?: boolean; /** * @typedef {object} HmacKeyOptions @@ -339,5 +355,62 @@ export class HmacKey extends ServiceObject { baseUrl: `/projects/${projectId}/hmacKeys`, methods, }); + + this.storage = storage; + this.instanceRetryValue = storage.retryOptions.autoRetry; + } + + /** + * Set the metadata for this object. + * + * @param {object} metadata - The metadata to set on this object. + * @param {object=} options - Configuration options. + * @param {function=} callback - The callback function. + * @param {?error} callback.err - An error returned while making this request. + * @param {object} callback.apiResponse - The full API response. + */ + setMetadata( + metadata: Metadata, + options?: SetMetadataOptions + ): Promise; + setMetadata(metadata: Metadata, callback: MetadataCallback): void; + setMetadata( + metadata: Metadata, + options: SetMetadataOptions, + callback: MetadataCallback + ): void; + setMetadata( + metadata: Metadata, + optionsOrCallback: SetMetadataOptions | MetadataCallback, + cb?: MetadataCallback + ): Promise | void { + // ETag preconditions are not currently supported. Retries should be disabled if the idempotency strategy is not set to RetryAlways + if ( + this.storage.retryOptions.idempotencyStrategy !== + IdempotencyStrategy.RetryAlways + ) { + this.storage.retryOptions.autoRetry = false; + } + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + cb = + typeof optionsOrCallback === 'function' + ? (optionsOrCallback as MetadataCallback) + : cb; + + super + .setMetadata(metadata, options) + .then(resp => cb!(null, ...resp)) + .catch(cb!) + .finally(() => { + this.storage.retryOptions.autoRetry = this.instanceRetryValue; + }); } } + +/*! Developer Documentation + * + * All async methods (except for streams) will return a Promise in the event + * that a callback is omitted. + */ +promisifyAll(HmacKey); diff --git a/src/iam.ts b/src/iam.ts index d75f97e5b..ec48acdc6 100644 --- a/src/iam.ts +++ b/src/iam.ts @@ -344,10 +344,16 @@ class Iam { SetPolicyCallback >(optionsOrCallback, callback); + let maxRetries; + if (policy.etag === undefined) { + maxRetries = 0; + } + this.request_( { method: 'PUT', uri: '/iam', + maxRetries, json: Object.assign( { resourceId: this.resourceId_, diff --git a/src/nodejs-common/service-object.ts b/src/nodejs-common/service-object.ts index 1ba6d3a47..5e8ead0e6 100644 --- a/src/nodejs-common/service-object.ts +++ b/src/nodejs-common/service-object.ts @@ -113,7 +113,13 @@ export interface CreateCallback { (err: ApiError | null, instance?: T | null, ...args: any[]): void; } -export type DeleteOptions = {ignoreNotFound?: boolean} & object; +export type DeleteOptions = { + ignoreNotFound?: boolean; + ifGenerationMatch?: number; + ifGenerationNotMatch?: number; + ifMetagenerationMatch?: number; + ifMetagenerationNotMatch?: number; +} & object; export interface DeleteCallback { (err: Error | null, apiResponse?: r.Response): void; } diff --git a/src/nodejs-common/util.ts b/src/nodejs-common/util.ts index 8b1d6d990..3a765ac5c 100644 --- a/src/nodejs-common/util.ts +++ b/src/nodejs-common/util.ts @@ -850,9 +850,9 @@ export class Util { } let maxRetryValue = MAX_RETRY_DEFAULT; - if (config.maxRetries) { + if (config.maxRetries !== undefined) { maxRetryValue = config.maxRetries; - } else if (config.retryOptions?.maxRetries) { + } else if (config.retryOptions?.maxRetries !== undefined) { maxRetryValue = config.retryOptions.maxRetries; } @@ -875,6 +875,7 @@ export class Util { if (typeof reqOpts.maxRetries === 'number') { options.retries = reqOpts.maxRetries; + options.noResponseRetries = reqOpts.maxRetries; } if (!config.stream) { diff --git a/src/storage.ts b/src/storage.ts index 8c8f38d1f..5268cdd7b 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -261,19 +261,30 @@ const IDEMPOTENCY_STRATEGY_DEFAULT = IdempotencyStrategy.RetryConditional; * @return {boolean} True if the API request should be retried, false otherwise. */ export const RETRYABLE_ERR_FN_DEFAULT = function (err?: ApiError) { + const isConnectionProblem = (reason: string | undefined) => { + return ( + (reason && reason.includes('eai_again')) || //DNS lookup error + reason === 'econnreset' || + reason === 'unexpected connection closure' + ); + }; + if (err) { if ([408, 429, 500, 502, 503, 504].indexOf(err.code!) !== -1) { return true; } + if (typeof err.code === 'string') { + const reason = (err.code as string).toLowerCase(); + if (isConnectionProblem(reason)) { + return true; + } + } + if (err.errors) { for (const e of err.errors) { const reason = e?.reason?.toString().toLowerCase(); - if ( - (reason && reason.includes('eai_again')) || //DNS lookup error - reason === 'econnreset' || - reason === 'unexpected connection closure' - ) { + if (isConnectionProblem(reason)) { return true; } } diff --git a/system-test/storage.ts b/system-test/storage.ts index c45d8dfd3..aa7514065 100644 --- a/system-test/storage.ts +++ b/system-test/storage.ts @@ -302,17 +302,16 @@ describe('storage', () => { }); it('should make a bucket private', async () => { - await bucket.makePublic(); - await bucket.makePrivate(); - const validateMakeBucketPrivateRejects = (err: ApiError) => { - assert.strictEqual(err.code, 404); - assert.strictEqual((err as ApiError).errors![0].reason, 'notFound'); - return true; - }; - await assert.rejects( - bucket.acl.get({entity: 'allUsers'}), - validateMakeBucketPrivateRejects - ); + try { + await bucket.makePublic(); + await bucket.makePrivate(); + assert.rejects(bucket.acl.get({entity: 'allUsers'}), err => { + assert.strictEqual((err as ApiError).code, 404); + assert.strictEqual((err as ApiError).errors![0].reason, 'notFound'); + }); + } catch (err) { + assert.ifError(err); + } }); it('should make files private', async () => { diff --git a/test/bucket.ts b/test/bucket.ts index 44243cb14..f9f040616 100644 --- a/test/bucket.ts +++ b/test/bucket.ts @@ -642,23 +642,6 @@ describe('Bucket', () => { done(); }); }); - - it('should disable auto-retries when ifMetagenerationMatch is not set', done => { - const rule = { - action: { - type: 'type', - }, - condition: {}, - }; - - bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); - return Promise.resolve(); - }; - - bucket.addLifecycleRule(rule, assert.ifError); - done(); - }); }); describe('combine', () => { @@ -853,42 +836,6 @@ describe('Bucket', () => { bucket.combine(sources, destination, options, assert.ifError); }); - it('should respect constructor precondition options', done => { - bucket = new Bucket(STORAGE, BUCKET_NAME, { - preconditionOpts: { - ifGenerationMatch: 301, - ifGenerationNotMatch: 302, - ifMetagenerationMatch: 303, - ifMetagenerationNotMatch: 304, - }, - }); - const sources = [bucket.file('1.txt'), bucket.file('2.txt')]; - const destination = bucket.file('destination.txt'); - - const options = {}; - destination.request = (reqOpts: DecorateRequestOptions) => { - assert.strictEqual( - reqOpts.qs.ifGenerationMatch, - bucket.instancePreconditionOpts.ifGenerationMatch - ); - assert.strictEqual( - reqOpts.qs.ifGenerationNotMatch, - bucket.instancePreconditionOpts.ifGenerationNotMatch - ); - assert.strictEqual( - reqOpts.qs.ifMetagenerationMatch, - bucket.instancePreconditionOpts.ifMetagenerationMatch - ); - assert.strictEqual( - reqOpts.qs.ifMetagenerationNotMatch, - bucket.instancePreconditionOpts.ifMetagenerationNotMatch - ); - done(); - }; - - bucket.combine(sources, destination, options, assert.ifError); - }); - it('should execute the callback', done => { const sources = [bucket.file('1.txt'), bucket.file('2.txt')]; const destination = bucket.file('destination.txt'); @@ -1427,20 +1374,28 @@ describe('Bucket', () => { describe('disableRequesterPays', () => { it('should call setMetadata correctly', done => { - bucket.setMetadata = (metadata: {}) => { + bucket.setMetadata = ( + metadata: {}, + _optionsOrCallback: {}, + callback: Function + ) => { assert.deepStrictEqual(metadata, { billing: { requesterPays: false, }, }); - return Promise.resolve([]); + Promise.resolve([]).then(resp => callback(null, ...resp)); }; bucket.disableRequesterPays(done); }); it('should not require a callback', done => { - bucket.setMetadata = (metadata: {}, callback: Function) => { + bucket.setMetadata = ( + metadata: {}, + optionsOrCallback: {}, + callback: Function + ) => { assert.strictEqual(callback, undefined); done(); }; @@ -1450,8 +1405,10 @@ describe('Bucket', () => { it('should set autoRetry to false when ifMetagenerationMatch is undefined', done => { bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); - done(); + Promise.resolve().then(() => { + assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); + done(); + }); }; bucket.disableRequesterPays(); }); @@ -1595,7 +1552,15 @@ describe('Bucket', () => { it('should execute the callback with the setMetadata response', done => { const setMetadataResponse = {}; - bucket.setMetadata = () => Promise.resolve([setMetadataResponse]); + bucket.setMetadata = ( + metadata: {}, + optionsOrCallback: {}, + callback: Function + ) => { + Promise.resolve([setMetadataResponse]).then(resp => + callback(null, ...resp) + ); + }; bucket.enableLogging( {prefix: PREFIX}, @@ -1619,44 +1584,33 @@ describe('Bucket', () => { done(); }); }); - - it('should disable autoRetry when ifMetagenerationMatch is undefined', done => { - bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); - }; - - bucket.enableLogging({prefix: PREFIX}, () => { - done(); - }); - }); }); describe('enableRequesterPays', () => { it('should call setMetadata correctly', done => { - bucket.setMetadata = (metadata: {}) => { + bucket.setMetadata = ( + metadata: {}, + optionsOrCallback: {}, + callback: Function + ) => { assert.deepStrictEqual(metadata, { billing: { requesterPays: true, }, }); - return Promise.resolve([]); + Promise.resolve([]).then(resp => callback(null, ...resp)); }; bucket.enableRequesterPays(done); }); it('should not require a callback', done => { - bucket.setMetadata = (metadata: {}, callback: Function) => { - assert.equal(undefined, callback); - done(); - }; - - bucket.enableRequesterPays(); - }); - - it('should disable autoRetry when ifMetagenerationMatch is undefined', done => { - bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); + bucket.setMetadata = ( + metadata: {}, + optionsOrCallback: {}, + callback: Function + ) => { + assert.equal(callback, undefined); done(); }; @@ -2137,13 +2091,17 @@ describe('Bucket', () => { it('should set predefinedAcl & privatize files', done => { let didSetPredefinedAcl = false; let didMakeFilesPrivate = false; + const opts = { + includeFiles: true, + force: true, + }; - bucket.setMetadata = (metadata: {}, options: {}) => { + bucket.setMetadata = (metadata: {}, options: {}, callback: Function) => { assert.deepStrictEqual(metadata, {acl: null}); assert.deepStrictEqual(options, {predefinedAcl: 'projectPrivate'}); didSetPredefinedAcl = true; - return Promise.resolve(); + bucket.makeAllFilesPublicPrivate_(opts, callback); }; bucket.makeAllFilesPublicPrivate_ = ( @@ -2156,7 +2114,7 @@ describe('Bucket', () => { callback(); }; - bucket.makePrivate({includeFiles: true, force: true}, (err: Error) => { + bucket.makePrivate(opts, (err: Error) => { assert.ifError(err); assert(didSetPredefinedAcl); assert(didMakeFilesPrivate); @@ -2186,7 +2144,7 @@ describe('Bucket', () => { }; bucket.setMetadata = (metadata: {}, options_: SetFileMetadataOptions) => { assert.strictEqual(options_.userProject, options.userProject); - return Promise.resolve(); + done(); }; bucket.makePrivate(options, done); }); @@ -2221,14 +2179,6 @@ describe('Bucket', () => { done(); }); }); - - it('should disable autoRetry when ifMetagenerationMatch is undefined', done => { - bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); - return Promise.resolve(); - }; - bucket.makePrivate({}, done); - }); }); describe('makePublic', () => { @@ -2323,21 +2273,16 @@ describe('Bucket', () => { describe('removeRetentionPeriod', () => { it('should call setMetadata correctly', done => { - bucket.setMetadata = (metadata: {}) => { + bucket.setMetadata = ( + metadata: {}, + _optionsOrCallback: {}, + callback: Function + ) => { assert.deepStrictEqual(metadata, { retentionPolicy: null, }); - return Promise.resolve([]); - }; - - bucket.removeRetentionPeriod(done); - }); - - it('should disable autoRetry when ifMetagenerationMatch is undefined', done => { - bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); - return Promise.resolve([]); + Promise.resolve([]).then(resp => callback(null, ...resp)); }; bucket.removeRetentionPeriod(done); @@ -2419,9 +2364,13 @@ describe('Bucket', () => { describe('setLabels', () => { it('should correctly call setMetadata', done => { const labels = {}; - bucket.setMetadata = (metadata: Metadata) => { + bucket.setMetadata = ( + metadata: Metadata, + _callbackOrOptions: {}, + callback: Function + ) => { assert.strictEqual(metadata.labels, labels); - return Promise.resolve([]); + Promise.resolve([]).then(resp => callback(null, ...resp)); }; bucket.setLabels(labels, done); }); @@ -2435,39 +2384,24 @@ describe('Bucket', () => { }; bucket.setLabels(labels, options, done); }); - - it('should disable autoRetry when isMetagenerationMatch is undefined', done => { - bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); - return Promise.resolve([]); - }; - bucket.setLabels({}, done); - }); }); describe('setRetentionPeriod', () => { it('should call setMetadata correctly', done => { const duration = 90000; - bucket.setMetadata = (metadata: {}) => { + bucket.setMetadata = ( + metadata: {}, + _callbackOrOptions: {}, + callback: Function + ) => { assert.deepStrictEqual(metadata, { retentionPolicy: { retentionPeriod: duration, }, }); - return Promise.resolve([]); - }; - - bucket.setRetentionPeriod(duration, done); - }); - - it('should disable autoRetry when ifMetagenerationMatch is undefined', done => { - const duration = 90000; - - bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); - return Promise.resolve([]); + Promise.resolve([]).then(resp => callback(null, ...resp)); }; bucket.setRetentionPeriod(duration, done); @@ -2478,23 +2412,16 @@ describe('Bucket', () => { it('should call setMetadata correctly', done => { const corsConfiguration = [{maxAgeSeconds: 3600}]; - bucket.setMetadata = (metadata: {}) => { + bucket.setMetadata = ( + metadata: {}, + _callbackOrOptions: {}, + callback: Function + ) => { assert.deepStrictEqual(metadata, { cors: corsConfiguration, }); - return Promise.resolve([]); - }; - - bucket.setCorsConfiguration(corsConfiguration, done); - }); - - it('should disable autoRetry when ifMetagenerationMatch is undefined', done => { - const corsConfiguration = [{maxAgeSeconds: 3600}]; - - bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); - return Promise.resolve([]); + return Promise.resolve([]).then(resp => callback(null, ...resp)); }; bucket.setCorsConfiguration(corsConfiguration, done); @@ -2532,21 +2459,11 @@ describe('Bucket', () => { ) => { assert.deepStrictEqual(metadata, {storageClass: STORAGE_CLASS}); assert.strictEqual(options, OPTIONS); - assert.strictEqual(callback, undefined); - return Promise.resolve([]); + Promise.resolve([]).then(resp => callback(null, ...resp)); }; bucket.setStorageClass(STORAGE_CLASS, OPTIONS, CALLBACK); }); - - it('should disable autoRetry when ifMetagenerationMatch is undefined', done => { - bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); - return Promise.resolve([]); - }; - - bucket.setStorageClass(STORAGE_CLASS, OPTIONS, done); - }); }); describe('setUserProject', () => { @@ -2700,18 +2617,6 @@ describe('Bucket', () => { }); }); - it('should disable autoRetry when ifMetagenerationMatch is undefined', done => { - const fakeFile = new FakeFile(bucket, 'file-name'); - fakeFile.isSameFile = () => { - return true; - }; - const options = {destination: fakeFile, metadata}; - bucket.upload(filepath, options, () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); - done(); - }); - }); - describe('resumable uploads', () => { class DelayedStream500Error extends Transform { retryCount: number; diff --git a/test/file.ts b/test/file.ts index 9c9a2ffe8..1cce36f12 100644 --- a/test/file.ts +++ b/test/file.ts @@ -16,6 +16,8 @@ import { ApiError, BodyResponseCallback, DecorateRequestOptions, + Metadata, + MetadataCallback, ServiceObject, ServiceObjectConfig, util, @@ -55,6 +57,7 @@ import { } from '../src/file'; import {ExceptionMessages, IdempotencyStrategy} from '../src/storage'; import {formatAsUTCISO} from '../src/util'; +import {SetMetadataOptions} from '../src/nodejs-common/service-object'; class HTTPError extends Error { code: number; @@ -3642,8 +3645,12 @@ describe('File', () => { it('should execute callback with API response', done => { const apiResponse = {}; - file.setMetadata = () => { - return Promise.resolve([apiResponse]); + file.setMetadata = ( + metadata: Metadata, + optionsOrCallback: SetMetadataOptions | MetadataCallback, + cb: MetadataCallback + ) => { + Promise.resolve([apiResponse]).then(resp => cb(null, ...resp)); }; file.makePrivate((err: Error, apiResponse_: {}) => { @@ -3701,16 +3708,6 @@ describe('File', () => { file.makePrivate(options, assert.ifError); }); - - it('should disable autoRetry when ifMetagenerationMatch is undefined', done => { - file.setMetadata = () => { - assert.strictEqual(file.storage.retryOptions.autoRetry, false); - done(); - }; - - file.makePrivate({}, assert.ifError); - assert.strictEqual(file.storage.retryOptions.autoRetry, true); - }); }); describe('makePublic', () => { @@ -3830,13 +3827,13 @@ describe('File', () => { }); }); - it('should correctly send a HEAD request', done => { + it('should correctly send a GET request', done => { fakeUtil.makeRequest = function ( reqOpts: DecorateRequestOptions, config: object, callback: BodyResponseCallback ) { - assert.strictEqual(reqOpts.method, 'HEAD'); + assert.strictEqual(reqOpts.method, 'GET'); callback(null); }; file.isPublic((err: ApiError) => { @@ -4173,8 +4170,13 @@ describe('File', () => { return newFile; }; - file.copy = (destination: string, callback: Function) => { + file.copy = ( + destination: string, + options: object, + callback: Function + ) => { assert.strictEqual(destination, newFile); + assert.deepStrictEqual(options, {}); callback(); // done() }; diff --git a/test/hmacKey.ts b/test/hmacKey.ts index ff016e1ef..9ceb342a4 100644 --- a/test/hmacKey.ts +++ b/test/hmacKey.ts @@ -16,7 +16,8 @@ import * as sinon from 'sinon'; import * as proxyquire from 'proxyquire'; import * as assert from 'assert'; import {describe, it, beforeEach, afterEach} from 'mocha'; -import {util, ServiceObject} from '../src/nodejs-common'; +import {util, ServiceObject, Metadata} from '../src/nodejs-common'; +import {IdempotencyStrategy} from '../src'; const sandbox = sinon.createSandbox(); // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -26,6 +27,14 @@ let hmacKey: any; const ACCESS_ID = 'fake-access-id'; +class HTTPError extends Error { + code: number; + constructor(message: string, code: number) { + super(message); + this.code = code; + } +} + describe('HmacKey', () => { afterEach(() => sandbox.restore()); @@ -48,6 +57,17 @@ describe('HmacKey', () => { STORAGE = { request: util.noop, projectId: 'my-project', + retryOptions: { + autoRetry: true, + maxRetries: 3, + retryDelayMultipier: 2, + totalTimeout: 600, + maxRetryDelay: 60, + retryableErrorFn: (err: HTTPError) => { + return err.code === 500; + }, + idempotencyStrategy: IdempotencyStrategy.RetryConditional, + }, }; hmacKey = new HmacKey(STORAGE, ACCESS_ID); @@ -76,5 +96,14 @@ describe('HmacKey', () => { const ctorArg = serviceObjectSpy.firstCall.args[0]; assert(ctorArg.baseUrl, '/projects/another-project/hmacKeys'); }); + + it('should correctly call setMetadata', done => { + hmacKey.setMetadata = (metadata: Metadata, callback: Function) => { + assert.deepStrictEqual(metadata.accessId, ACCESS_ID); + Promise.resolve([]).then(resp => callback(null, ...resp)); + }; + + hmacKey.setMetadata({accessId: ACCESS_ID}, done); + }); }); }); diff --git a/test/iam.ts b/test/iam.ts index 6d4c693be..9e03ff508 100644 --- a/test/iam.ts +++ b/test/iam.ts @@ -135,6 +135,7 @@ describe('storage/iam', () => { assert.deepStrictEqual(reqOpts, { method: 'PUT', uri: '/iam', + maxRetries: 0, json: Object.assign( { resourceId: iam.resourceId_,