From 122dacf0918f5d3a88823f751a3e5f65aca6e44c Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Tue, 7 Jun 2022 19:53:15 +0000 Subject: [PATCH 01/58] test: updated conformance tests for precondition updates --- conformance-test/conformanceCommon.ts | 43 ++- conformance-test/globalHooks.ts | 16 +- conformance-test/libraryMethods.ts | 273 ++++++++++++++++-- .../test-data/retryInvocationMap.json | 29 ++ 4 files changed, 310 insertions(+), 51 deletions(-) diff --git a/conformance-test/conformanceCommon.ts b/conformance-test/conformanceCommon.ts index c442aa2f6..7b0bc55de 100644 --- a/conformance-test/conformanceCommon.ts +++ b/conformance-test/conformanceCommon.ts @@ -92,16 +92,31 @@ 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("InstancePreconditon")) { + console.log(storageMethodString); + 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(); @@ -158,7 +173,7 @@ export function executeScenario(testCase: RetryTestCase) { async function createBucketForTest( storage: Storage, - preconditionProvided: boolean, + preconditionShouldBeOnInstance: boolean, storageMethodString: String ) { const name = generateName(storageMethodString, 'bucket'); @@ -166,7 +181,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 +192,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/globalHooks.ts b/conformance-test/globalHooks.ts index 0775b7457..2567d5349 100644 --- a/conformance-test/globalHooks.ts +++ b/conformance-test/globalHooks.ts @@ -25,17 +25,17 @@ const TIME_TO_WAIT_FOR_CONTAINER_READY = 10000; // eslint-disable-next-line @typescript-eslint/no-explicit-any export async function mochaGlobalSetup(this: any) { // Increase the timeout for this before block so that the docker images have time to download and run. - this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; - await getTestBenchDockerImage(); - await runTestBenchDockerImage(); - await new Promise(resolve => - setTimeout(resolve, TIME_TO_WAIT_FOR_CONTAINER_READY) - ); + // this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; + // await getTestBenchDockerImage(); + // await runTestBenchDockerImage(); + // await new Promise(resolve => + // setTimeout(resolve, TIME_TO_WAIT_FOR_CONTAINER_READY) + // ); } // eslint-disable-next-line @typescript-eslint/no-explicit-any export async function mochaGlobalTeardown(this: any) { // Increase the timeout for this block so that docker has time to stop the container. - this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; - await stopTestBenchDockerImage(); + // this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; + // await stopTestBenchDockerImage(); } diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 6dfa7e284..b82194fb8 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -16,11 +16,25 @@ import {Bucket, File, Notification, Storage, HmacKey} from '../src'; import * as path from 'path'; import {ApiError} from '../src/nodejs-common'; + +const METAGENERATION_PRECONDITION = { + ifMetagenerationMatch: 2, +} + ///////////////////////////////////////////////// //////////////////// BUCKET ///////////////////// ///////////////////////////////////////////////// -export async function addLifecycleRule(bucket: Bucket) { +export async function addLifecycleRuleInstancePrecondition(bucket: Bucket) { + await bucket.addLifecycleRule({ + action: 'delete', + condition: { + age: 365 * 3, // Specified in days. + }, + }); +} + +export async function addLifecycleRule(bucket: Bucket) { //TODO await bucket.addLifecycleRule({ action: 'delete', condition: { @@ -29,7 +43,34 @@ export async function addLifecycleRule(bucket: Bucket) { }); } -export async function combine(bucket: Bucket) { +export async function combineInstancePrecondition(bucket: Bucket) { + bucket = new Bucket(bucket.storage, bucket.name, { + preconditionOpts: { + ifMetagenerationMatch: 1, + }, + }); + const file1 = bucket.file('file1.txt'); + const file2 = 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 + await allFiles.save('allfiles contents'); + await bucket.combine(sources, allFiles); +} + +export async function combine(bucket: Bucket) { //TODO bucket = new Bucket(bucket.storage, bucket.name, { preconditionOpts: { ifMetagenerationMatch: 1, @@ -76,29 +117,52 @@ export async function deleteBucket(bucket: Bucket) { await bucket.delete(); } -export async function deleteFiles(bucket: Bucket) { +export async function deleteFilesInstancePrecondition(bucket: Bucket) { + await bucket.deleteFiles(); +} + +export async function deleteFiles(bucket: Bucket) { //TODO await bucket.deleteFiles(); } -export async function deleteLabels(bucket: Bucket) { +export async function deleteLabelsInstancePrecondition(bucket: Bucket) { await bucket.deleteLabels(); } -export async function disableRequesterPays(bucket: Bucket) { +export async function deleteLabels(bucket: Bucket) { //TODO -- no options to modify + await bucket.deleteLabels(); +} + +export async function disableRequesterPaysInstancePrecondition(bucket: Bucket) { await bucket.disableRequesterPays(); } -export async function enableLogging(bucket: Bucket) { +export async function disableRequesterPays(bucket: Bucket) { //TODO -- no options + await bucket.disableRequesterPays(); +} + +export async function enableLoggingInstancePrecondition(bucket: Bucket) { const config = { prefix: 'log', }; await bucket.enableLogging(config); } -export async function enableRequesterPays(bucket: Bucket) { +export async function enableLogging(bucket: Bucket) { //TODO + const config = { + prefix: 'log', + }; + await bucket.enableLogging(config); +} + +export async function enableRequesterPaysInstancePrecondition(bucket: Bucket) { await bucket.enableRequesterPays(); } +export async function enableRequesterPays(bucket: Bucket) { + await bucket.enableRequesterPays(); //TODO +} + export async function bucketExists(bucket: Bucket) { await bucket.exists(); } @@ -134,7 +198,11 @@ export async function lock(bucket: Bucket) { await bucket.lock(metageneration); } -export async function bucketMakePrivate(bucket: Bucket) { +export async function bucketMakePrivateInstancePrecondition(bucket: Bucket) { + await bucket.makePrivate(); +} + +export async function bucketMakePrivate(bucket: Bucket) { //TODO await bucket.makePrivate(); } @@ -142,16 +210,33 @@ export async function bucketMakePublic(bucket: Bucket) { await bucket.makePublic(); } -export async function removeRetentionPeriod(bucket: Bucket) { +export async function removeRetentionPeriodInstancePrecondition(bucket: Bucket) { await bucket.removeRetentionPeriod(); } -export async function setCorsConfiguration(bucket: Bucket) { +export async function removeRetentionPeriod(bucket: Bucket) { //TODO + await bucket.removeRetentionPeriod(); +} + +export async function setCorsConfigurationInstancePrecondition(bucket: Bucket) { const corsConfiguration = [{maxAgeSeconds: 3600}]; // 1 hour await bucket.setCorsConfiguration(corsConfiguration); } -export async function setLabels(bucket: Bucket) { +export async function setCorsConfiguration(bucket: Bucket) { //TODO + const corsConfiguration = [{maxAgeSeconds: 3600}]; // 1 hour + await bucket.setCorsConfiguration(corsConfiguration); +} + +export async function setLabelsInstancePrecondition(bucket: Bucket) { + const labels = { + labelone: 'labelonevalue', + labeltwo: 'labeltwovalue', + }; + await bucket.setLabels(labels); +} + +export async function setLabels(bucket: Bucket) { //TODO const labels = { labelone: 'labelonevalue', labeltwo: 'labeltwovalue', @@ -159,7 +244,17 @@ export async function setLabels(bucket: Bucket) { await bucket.setLabels(labels); } -export async function bucketSetMetadata(bucket: Bucket) { +export async function bucketSetMetadataInstancePrecondition(bucket: Bucket) { + const metadata = { + website: { + mainPageSuffix: 'http://example.com', + notFoundPage: 'http://example.com/404.html', + }, + }; + await bucket.setMetadata(metadata); +} + +export async function bucketSetMetadata(bucket: Bucket) { //TODO const metadata = { website: { mainPageSuffix: 'http://example.com', @@ -169,16 +264,25 @@ export async function bucketSetMetadata(bucket: Bucket) { await bucket.setMetadata(metadata); } -export async function setRetentionPeriod(bucket: Bucket) { +export async function setRetentionPeriodInstancePrecondition(bucket: Bucket) { const DURATION_SECONDS = 15780000; // 6 months. await bucket.setRetentionPeriod(DURATION_SECONDS); } -export async function bucketSetStorageClass(bucket: Bucket) { +export async function setRetentionPeriod(bucket: Bucket) { //TODO + const DURATION_SECONDS = 15780000; // 6 months. + await bucket.setRetentionPeriod(DURATION_SECONDS); +} + +export async function bucketSetStorageClassInstancePrecondition(bucket: Bucket) { await bucket.setStorageClass('nearline'); } -export async function bucketUploadResumable(bucket: Bucket) { +export async function bucketSetStorageClass(bucket: Bucket) { //TODO + await bucket.setStorageClass('nearline'); +} + +export async function bucketUploadResumableInstancePrecondition(bucket: Bucket) { if (bucket.instancePreconditionOpts) { bucket.instancePreconditionOpts.ifGenerationMatch = 0; } @@ -191,7 +295,34 @@ export async function bucketUploadResumable(bucket: Bucket) { ); } -export async function bucketUploadMultipart(bucket: Bucket) { +export async function bucketUploadResumable(bucket: Bucket) { //TODO + if (bucket.instancePreconditionOpts) { + bucket.instancePreconditionOpts.ifGenerationMatch = 0; + } + await bucket.upload( + path.join( + __dirname, + '../../conformance-test/test-data/retryStrategyTestData.json' + ), + {resumable: true} + ); +} + +export async function bucketUploadMultipartInstancePrecondition(bucket: Bucket) { + if (bucket.instancePreconditionOpts) { + delete bucket.instancePreconditionOpts.ifMetagenerationMatch; + bucket.instancePreconditionOpts.ifGenerationMatch = 0; + } + await bucket.upload( + path.join( + __dirname, + '../../conformance-test/test-data/retryStrategyTestData.json' + ), + {resumable: false} + ); +} + +export async function bucketUploadMultipart(bucket: Bucket) { //TODO if (bucket.instancePreconditionOpts) { delete bucket.instancePreconditionOpts.ifMetagenerationMatch; bucket.instancePreconditionOpts.ifGenerationMatch = 0; @@ -209,7 +340,11 @@ export async function bucketUploadMultipart(bucket: Bucket) { //////////////////// FILE ///////////////////// ///////////////////////////////////////////////// -export async function copy(_bucket: Bucket, file: File) { +export async function copyInstancePrecondition(_bucket: Bucket, file: File) { + await file.copy('a-different-file.png'); +} + +export async function copy(_bucket: Bucket, file: File) { //TODO await file.copy('a-different-file.png'); } @@ -223,11 +358,19 @@ export async function createReadStream(_bucket: Bucket, file: File) { }); } -export async function createResumableUpload(_bucket: Bucket, file: File) { +export async function createResumableUploadInstancePrecondition(_bucket: Bucket, file: File) { + await file.createResumableUpload(); +} + +export async function createResumableUpload(_bucket: Bucket, file: File) { //TODO await file.createResumableUpload(); } -export async function fileDelete(_bucket: Bucket, file: File) { +export async function fileDeleteInstancePrecondition(_bucket: Bucket, file: File) { + await file.delete(); +} + +export async function fileDelete(_bucket: Bucket, file: File) { //TODO await file.delete(); } @@ -255,7 +398,11 @@ export async function isPublic(_bucket: Bucket, file: File) { await file.isPublic(); } -export async function fileMakePrivate(_bucket: Bucket, file: File) { +export async function fileMakePrivateInstancePrecondition(_bucket: Bucket, file: File) { + await file.makePrivate(); +} + +export async function fileMakePrivate(_bucket: Bucket, file: File) { //TODO await file.makePrivate(); } @@ -263,15 +410,32 @@ export async function fileMakePublic(_bucket: Bucket, file: File) { await file.makePublic(); } -export async function move(_bucket: Bucket, file: File) { +export async function moveInstancePrecondition(_bucket: Bucket, file: File) { await file.move('new-file'); } -export async function rename(_bucket: Bucket, file: File) { +export async function move(_bucket: Bucket, file: File) { //TODO + await file.move('new-file'); +} + +export async function renameInstancePrecondition(_bucket: Bucket, file: File) { + await file.rename('new-name'); +} + +export async function rename(_bucket: Bucket, file: File) { //TODO await file.rename('new-name'); } -export async function rotateEncryptionKey(_bucket: Bucket, file: File) { +export async function rotateEncryptionKeyInstancePrecondition(_bucket: Bucket, file: File) { + const crypto = require('crypto'); + const buffer = crypto.randomBytes(32); + const newKey = buffer.toString('base64'); + await file.rotateEncryptionKey({ + encryptionKey: Buffer.from(newKey, 'base64'), + }); +} + +export async function rotateEncryptionKey(_bucket: Bucket, file: File) { //TODO const crypto = require('crypto'); const buffer = crypto.randomBytes(32); const newKey = buffer.toString('base64'); @@ -280,15 +444,23 @@ export async function rotateEncryptionKey(_bucket: Bucket, file: File) { }); } -export async function saveResumable(_bucket: Bucket, file: File) { +export async function saveResumableInstancePrecondition(_bucket: Bucket, file: File) { + await file.save('testdata', {resumable: true}); +} + +export async function saveResumable(_bucket: Bucket, file: File) { //TODO await file.save('testdata', {resumable: true}); } -export async function saveMultipart(_bucket: Bucket, file: File) { +export async function saveMultipartInstancePrecondition(_bucket: Bucket, file: File) { + await file.save('testdata', {resumable: false}); +} + +export async function saveMultipart(_bucket: Bucket, file: File) { //TODO await file.save('testdata', {resumable: false}); } -export async function setMetadata(_bucket: Bucket, file: File) { +export async function setMetadataInstancePrecondition(_bucket: Bucket, file: File) { const metadata = { contentType: 'application/x-font-ttf', metadata: { @@ -299,7 +471,25 @@ export async function setMetadata(_bucket: Bucket, file: File) { await file.setMetadata(metadata); } -export async function setStorageClass(_bucket: Bucket, file: File) { +export async function setMetadata(_bucket: Bucket, file: File) { //TODO + const metadata = { + contentType: 'application/x-font-ttf', + metadata: { + my: 'custom', + properties: 'go here', + }, + }; + await file.setMetadata(metadata); +} + +export async function setStorageClassInstancePrecondition(_bucket: Bucket, file: File) { + const result = await file.setStorageClass('nearline'); + if (!result) { + throw Error(); + } +} + +export async function setStorageClass(_bucket: Bucket, file: File) { //TODO const result = await file.setStorageClass('nearline'); if (!result) { throw Error(); @@ -344,7 +534,20 @@ export async function getMetadataHMAC( await hmacKey.getMetadata(); } -export async function setMetadataHMAC( +export async function setMetadataHMACInstancePrecondition( + _bucket: Bucket, + file: File, + _notification: Notification, + _storage: Storage, + hmacKey: HmacKey +) { + const metadata = { + state: 'INACTIVE', + }; + await hmacKey.setMetadata(metadata); +} + +export async function setMetadataHMAC( //TODO _bucket: Bucket, file: File, _notification: Notification, @@ -365,7 +568,19 @@ export async function iamGetPolicy(bucket: Bucket) { await bucket.iam.getPolicy({requestedPolicyVersion: 1}); } -export async function iamSetPolicy(bucket: Bucket) { +export async function iamSetPolicyInstancePrecondition(bucket: Bucket) { + const testPolicy = { + bindings: [ + { + role: 'roles/storage.admin', + members: ['serviceAccount:myotherproject@appspot.gserviceaccount.com'], + }, + ], + }; + await bucket.iam.setPolicy(testPolicy); +} + +export async function iamSetPolicy(bucket: Bucket) { //TODO const testPolicy = { bindings: [ { diff --git a/conformance-test/test-data/retryInvocationMap.json b/conformance-test/test-data/retryInvocationMap.json index e92e7abd7..565fc97a6 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" ], @@ -46,6 +60,11 @@ "fileMakePublic" ], "storage.objects.rewrite": [ + "copyInstancePrecondition", + "moveInstancePrecondition", + "renameInstancePrecondition", + "rotateEncryptionKeyInstancePrecondition", + "setStorageClassInstancePrecondition", "copy", "move", "rename", @@ -53,6 +72,11 @@ "setStorageClass" ], "storage.objects.insert": [ + "bucketUploadResumableInstancePrecondition", + "saveResumableInstancePrecondition", + "bucketUploadMultipartInstancePrecondition", + "saveMultipartInstancePrecondition", + "createResumableUploadInstancePrecondition", "bucketUploadResumable", "saveResumable", "bucketUploadMultipart", @@ -60,6 +84,8 @@ "createResumableUpload" ], "storage.objects.delete": [ + "deleteFilesInstancePrecondition", + "fileDeleteInstancePrecondition", "deleteFiles", "fileDelete" ], @@ -88,6 +114,7 @@ "iamGetPolicy" ], "storage.buckets.setIamPolicy": [ + "iamSetPolicyInstancePrecondition", "iamSetPolicy" ], "storage.buckets.testIamPermission": [ @@ -98,6 +125,7 @@ "getBucketsStream" ], "storage.objects.compose": [ + "combineInstancePrecondition", "combine" ], "storage.hmacKey.delete": [ @@ -108,6 +136,7 @@ "getMetadataHMAC" ], "storage.hmacKey.update": [ + "setMetadataHMACInstancePrecondition", "setMetadataHMAC" ], "storage.hmacKey.create": [ From 091686feee7ff7d919eeb28f99e899bbba09ac0d Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Tue, 7 Jun 2022 20:13:22 +0000 Subject: [PATCH 02/58] fixed typo --- conformance-test/conformanceCommon.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/conformance-test/conformanceCommon.ts b/conformance-test/conformanceCommon.ts index 7b0bc55de..4f5f2b619 100644 --- a/conformance-test/conformanceCommon.ts +++ b/conformance-test/conformanceCommon.ts @@ -92,8 +92,7 @@ export function executeScenario(testCase: RetryTestCase) { instructionSet.instructions, jsonMethod?.name.toString() ); - if (storageMethodString.includes("InstancePreconditon")) { - console.log(storageMethodString); + if (storageMethodString.includes("InstancePrecondition")) { bucket = await createBucketForTest( storage, testCase.preconditionProvided, From fa48fec919a9a922122274a18badb98dbbea2f3d Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Fri, 24 Jun 2022 17:30:59 +0000 Subject: [PATCH 03/58] updated functions to pass preconditions --- conformance-test/libraryMethods.ts | 42 ++++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index b82194fb8..2b5adc220 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -295,16 +295,13 @@ export async function bucketUploadResumableInstancePrecondition(bucket: Bucket) ); } -export async function bucketUploadResumable(bucket: Bucket) { //TODO - if (bucket.instancePreconditionOpts) { - bucket.instancePreconditionOpts.ifGenerationMatch = 0; - } +export async function bucketUploadResumable(bucket: Bucket) { await bucket.upload( path.join( __dirname, '../../conformance-test/test-data/retryStrategyTestData.json' ), - {resumable: true} + {preconditionOpts: {ifMetagenerationMatch: 2, ifGenerationMatch:0}} ); } @@ -322,17 +319,16 @@ export async function bucketUploadMultipartInstancePrecondition(bucket: Bucket) ); } -export async function bucketUploadMultipart(bucket: Bucket) { //TODO +export async function bucketUploadMultipart(bucket: Bucket) { if (bucket.instancePreconditionOpts) { delete bucket.instancePreconditionOpts.ifMetagenerationMatch; - bucket.instancePreconditionOpts.ifGenerationMatch = 0; } await bucket.upload( path.join( __dirname, '../../conformance-test/test-data/retryStrategyTestData.json' ), - {resumable: false} + {resumable: false, preconditionOpts: {ifGenerationMatch: 0}} ); } @@ -344,8 +340,8 @@ export async function copyInstancePrecondition(_bucket: Bucket, file: File) { await file.copy('a-different-file.png'); } -export async function copy(_bucket: Bucket, file: File) { //TODO - await file.copy('a-different-file.png'); +export async function copy(_bucket: Bucket, file: File) { + await file.copy('a-different-file.png', {preconditionOpts: {ifGenerationMatch: 0}}); } export async function createReadStream(_bucket: Bucket, file: File) { @@ -362,8 +358,8 @@ export async function createResumableUploadInstancePrecondition(_bucket: Bucket, await file.createResumableUpload(); } -export async function createResumableUpload(_bucket: Bucket, file: File) { //TODO - await file.createResumableUpload(); +export async function createResumableUpload(_bucket: Bucket, file: File) { + await file.createResumableUpload({preconditionOpts: {ifGenerationMatch: 0}}); } export async function fileDeleteInstancePrecondition(_bucket: Bucket, file: File) { @@ -414,16 +410,16 @@ export async function moveInstancePrecondition(_bucket: Bucket, file: File) { await file.move('new-file'); } -export async function move(_bucket: Bucket, file: File) { //TODO - await file.move('new-file'); +export async function move(_bucket: Bucket, file: File) { + await file.move('new-file', {preconditionOpts: {ifGenerationMatch: 0}}); } export async function renameInstancePrecondition(_bucket: Bucket, file: File) { await file.rename('new-name'); } -export async function rename(_bucket: Bucket, file: File) { //TODO - await file.rename('new-name'); +export async function rename(_bucket: Bucket, file: File) { + await file.rename('new-name', {preconditionOpts: {ifGenerationMatch: 0}}); } export async function rotateEncryptionKeyInstancePrecondition(_bucket: Bucket, file: File) { @@ -448,16 +444,22 @@ export async function saveResumableInstancePrecondition(_bucket: Bucket, file: F await file.save('testdata', {resumable: true}); } -export async function saveResumable(_bucket: Bucket, file: File) { //TODO - await file.save('testdata', {resumable: true}); +export async function saveResumable(_bucket: Bucket, file: File) { + await file.save('testdata', {resumable: true, preconditionOpts: { + ifGenerationMatch: file.metadata.generation, + ifMetagenerationMatch: file.metadata.metageneration + }}); } export async function saveMultipartInstancePrecondition(_bucket: Bucket, file: File) { await file.save('testdata', {resumable: false}); } -export async function saveMultipart(_bucket: Bucket, file: File) { //TODO - await file.save('testdata', {resumable: false}); +export async function saveMultipart(_bucket: Bucket, file: File) { + await file.save('testdata', {resumable: false, preconditionOpts: { + ifGenerationMatch: file.metadata.generation, + ifMetagenerationMatch: file.metadata.metageneration + }}); } export async function setMetadataInstancePrecondition(_bucket: Bucket, file: File) { From 7a7b30440261d18baca8972c7ecebc092e86a1a0 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 27 Jun 2022 18:50:22 +0000 Subject: [PATCH 04/58] removed IAM and HMAC test changes --- conformance-test/libraryMethods.ts | 29 ++----------------- .../test-data/retryInvocationMap.json | 2 -- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 2b5adc220..793ec9468 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -536,20 +536,7 @@ export async function getMetadataHMAC( await hmacKey.getMetadata(); } -export async function setMetadataHMACInstancePrecondition( - _bucket: Bucket, - file: File, - _notification: Notification, - _storage: Storage, - hmacKey: HmacKey -) { - const metadata = { - state: 'INACTIVE', - }; - await hmacKey.setMetadata(metadata); -} - -export async function setMetadataHMAC( //TODO +export async function setMetadataHMAC( _bucket: Bucket, file: File, _notification: Notification, @@ -570,19 +557,7 @@ export async function iamGetPolicy(bucket: Bucket) { await bucket.iam.getPolicy({requestedPolicyVersion: 1}); } -export async function iamSetPolicyInstancePrecondition(bucket: Bucket) { - const testPolicy = { - bindings: [ - { - role: 'roles/storage.admin', - members: ['serviceAccount:myotherproject@appspot.gserviceaccount.com'], - }, - ], - }; - await bucket.iam.setPolicy(testPolicy); -} - -export async function iamSetPolicy(bucket: Bucket) { //TODO +export async function iamSetPolicy(bucket: Bucket) { const testPolicy = { bindings: [ { diff --git a/conformance-test/test-data/retryInvocationMap.json b/conformance-test/test-data/retryInvocationMap.json index 565fc97a6..528500eb9 100644 --- a/conformance-test/test-data/retryInvocationMap.json +++ b/conformance-test/test-data/retryInvocationMap.json @@ -114,7 +114,6 @@ "iamGetPolicy" ], "storage.buckets.setIamPolicy": [ - "iamSetPolicyInstancePrecondition", "iamSetPolicy" ], "storage.buckets.testIamPermission": [ @@ -136,7 +135,6 @@ "getMetadataHMAC" ], "storage.hmacKey.update": [ - "setMetadataHMACInstancePrecondition", "setMetadataHMAC" ], "storage.hmacKey.create": [ From 3e0647d9bd7fe55f57a6a4b19b45f69e27294792 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 27 Jun 2022 19:48:42 +0000 Subject: [PATCH 05/58] implemented local preconditions for bucketmakeprivate --- conformance-test/libraryMethods.ts | 4 ++-- src/bucket.ts | 31 +++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 793ec9468..844ace639 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -202,8 +202,8 @@ export async function bucketMakePrivateInstancePrecondition(bucket: Bucket) { await bucket.makePrivate(); } -export async function bucketMakePrivate(bucket: Bucket) { //TODO - await bucket.makePrivate(); +export async function bucketMakePrivate(bucket: Bucket) { + await bucket.makePrivate({preconditionOpts: {ifMetagenerationMatch: 2}}); } export async function bucketMakePublic(bucket: Bucket) { diff --git a/src/bucket.ts b/src/bucket.ts index f6aef95d5..0a3d0f2a0 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -77,7 +77,10 @@ interface CreateNotificationQuery { interface MetadataOptions { predefinedAcl: string; userProject?: string; -} + ifGenerationMatch?: number; + ifGenerationNotMatch?: number; + ifMetagenerationMatch?: number; + ifMetagenerationNotMatch?: number;} export type GetFilesResponse = [File[], {}, Metadata]; export interface GetFilesCallback { @@ -288,6 +291,7 @@ export interface MakeBucketPrivateOptions { force?: boolean; metadata?: Metadata; userProject?: string; + preconditionOpts?: PreconditionOptions } interface MakeBucketPrivateRequest extends MakeBucketPrivateOptions { @@ -3079,9 +3083,25 @@ class Bucket extends ServiceObject { query.userProject = options.userProject; } + 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; + } + this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + AvailableServiceObjectMethods.setMetadata, options.preconditionOpts ); // You aren't allowed to set both predefinedAcl & acl properties on a bucket @@ -4138,11 +4158,12 @@ 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 && + (typeof coreOpts === 'object' && + coreOpts?.reqOpts?.qs?.ifMetagenerationMatch === undefined && localPreconditionOptions?.ifMetagenerationMatch === undefined) && (methodType === AvailableServiceObjectMethods.setMetadata || methodType === AvailableServiceObjectMethods.delete) && this.storage.retryOptions.idempotencyStrategy === From 0b31ad1a3155471e23ac314bd853755fe02ee144 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 27 Jun 2022 20:15:21 +0000 Subject: [PATCH 06/58] added preconditions to enableLogging --- conformance-test/libraryMethods.ts | 5 ++++- src/bucket.ts | 9 ++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 844ace639..018ef6ce2 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -148,9 +148,12 @@ export async function enableLoggingInstancePrecondition(bucket: Bucket) { await bucket.enableLogging(config); } -export async function enableLogging(bucket: Bucket) { //TODO +export async function enableLogging(bucket: Bucket) { const config = { prefix: 'log', + preconditionOpts: { + ifMetagenerationMatch: 2 + } }; await bucket.enableLogging(config); } diff --git a/src/bucket.ts b/src/bucket.ts index 0a3d0f2a0..e6cb3b911 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -64,6 +64,7 @@ import { } from './signer'; import {Readable} from 'stream'; import {CRC32CValidatorGenerator} from './crc32c'; +import { options } from 'yargs'; interface SourceObject { name: string; @@ -115,6 +116,7 @@ export interface LifecycleRule { export interface EnableLoggingOptions { bucket?: string | Bucket; prefix: string; + preconditionOpts?: PreconditionOptions } export interface GetFilesOptions { @@ -2208,14 +2210,15 @@ class Bucket extends ServiceObject { await this.iam.setPolicy(policy); this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + AvailableServiceObjectMethods.setMetadata, + config.preconditionOpts ); [setMetadataResponse] = await this.setMetadata({ logging: { logBucket, logObjectPrefix: config.prefix, - }, - }); + } + }, config.preconditionOpts); } catch (e) { callback!(e as Error); return; From 79d5a05b1e2303e283bec002ee6967411942347f Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Tue, 28 Jun 2022 09:15:36 +0000 Subject: [PATCH 07/58] linted files --- conformance-test/conformanceCommon.ts | 5 +- conformance-test/libraryMethods.ts | 139 ++++++++++++++++++-------- src/bucket.ts | 44 ++++---- 3 files changed, 126 insertions(+), 62 deletions(-) diff --git a/conformance-test/conformanceCommon.ts b/conformance-test/conformanceCommon.ts index 4f5f2b619..8ae7e7647 100644 --- a/conformance-test/conformanceCommon.ts +++ b/conformance-test/conformanceCommon.ts @@ -92,7 +92,7 @@ export function executeScenario(testCase: RetryTestCase) { instructionSet.instructions, jsonMethod?.name.toString() ); - if (storageMethodString.includes("InstancePrecondition")) { + if (storageMethodString.includes('InstancePrecondition')) { bucket = await createBucketForTest( storage, testCase.preconditionProvided, @@ -103,8 +103,7 @@ export function executeScenario(testCase: RetryTestCase) { storageMethodString, bucket ); - } - else { + } else { bucket = await createBucketForTest( storage, false, diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 018ef6ce2..9c1a7f960 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -16,10 +16,9 @@ import {Bucket, File, Notification, Storage, HmacKey} from '../src'; import * as path from 'path'; import {ApiError} from '../src/nodejs-common'; - const METAGENERATION_PRECONDITION = { ifMetagenerationMatch: 2, -} +}; ///////////////////////////////////////////////// //////////////////// BUCKET ///////////////////// @@ -34,7 +33,8 @@ export async function addLifecycleRuleInstancePrecondition(bucket: Bucket) { }); } -export async function addLifecycleRule(bucket: Bucket) { //TODO +export async function addLifecycleRule(bucket: Bucket) { + //TODO await bucket.addLifecycleRule({ action: 'delete', condition: { @@ -70,7 +70,8 @@ export async function combineInstancePrecondition(bucket: Bucket) { await bucket.combine(sources, allFiles); } -export async function combine(bucket: Bucket) { //TODO +export async function combine(bucket: Bucket) { + //TODO bucket = new Bucket(bucket.storage, bucket.name, { preconditionOpts: { ifMetagenerationMatch: 1, @@ -121,7 +122,8 @@ export async function deleteFilesInstancePrecondition(bucket: Bucket) { await bucket.deleteFiles(); } -export async function deleteFiles(bucket: Bucket) { //TODO +export async function deleteFiles(bucket: Bucket) { + //TODO await bucket.deleteFiles(); } @@ -129,7 +131,8 @@ export async function deleteLabelsInstancePrecondition(bucket: Bucket) { await bucket.deleteLabels(); } -export async function deleteLabels(bucket: Bucket) { //TODO -- no options to modify +export async function deleteLabels(bucket: Bucket) { + //TODO -- no options to modify await bucket.deleteLabels(); } @@ -137,7 +140,8 @@ export async function disableRequesterPaysInstancePrecondition(bucket: Bucket) { await bucket.disableRequesterPays(); } -export async function disableRequesterPays(bucket: Bucket) { //TODO -- no options +export async function disableRequesterPays(bucket: Bucket) { + //TODO -- no options await bucket.disableRequesterPays(); } @@ -152,8 +156,8 @@ export async function enableLogging(bucket: Bucket) { const config = { prefix: 'log', preconditionOpts: { - ifMetagenerationMatch: 2 - } + ifMetagenerationMatch: 2, + }, }; await bucket.enableLogging(config); } @@ -213,11 +217,14 @@ export async function bucketMakePublic(bucket: Bucket) { await bucket.makePublic(); } -export async function removeRetentionPeriodInstancePrecondition(bucket: Bucket) { +export async function removeRetentionPeriodInstancePrecondition( + bucket: Bucket +) { await bucket.removeRetentionPeriod(); } -export async function removeRetentionPeriod(bucket: Bucket) { //TODO +export async function removeRetentionPeriod(bucket: Bucket) { + //TODO await bucket.removeRetentionPeriod(); } @@ -226,7 +233,8 @@ export async function setCorsConfigurationInstancePrecondition(bucket: Bucket) { await bucket.setCorsConfiguration(corsConfiguration); } -export async function setCorsConfiguration(bucket: Bucket) { //TODO +export async function setCorsConfiguration(bucket: Bucket) { + //TODO const corsConfiguration = [{maxAgeSeconds: 3600}]; // 1 hour await bucket.setCorsConfiguration(corsConfiguration); } @@ -239,7 +247,8 @@ export async function setLabelsInstancePrecondition(bucket: Bucket) { await bucket.setLabels(labels); } -export async function setLabels(bucket: Bucket) { //TODO +export async function setLabels(bucket: Bucket) { + //TODO const labels = { labelone: 'labelonevalue', labeltwo: 'labeltwovalue', @@ -257,7 +266,8 @@ export async function bucketSetMetadataInstancePrecondition(bucket: Bucket) { await bucket.setMetadata(metadata); } -export async function bucketSetMetadata(bucket: Bucket) { //TODO +export async function bucketSetMetadata(bucket: Bucket) { + //TODO const metadata = { website: { mainPageSuffix: 'http://example.com', @@ -272,20 +282,26 @@ export async function setRetentionPeriodInstancePrecondition(bucket: Bucket) { await bucket.setRetentionPeriod(DURATION_SECONDS); } -export async function setRetentionPeriod(bucket: Bucket) { //TODO +export async function setRetentionPeriod(bucket: Bucket) { + //TODO const DURATION_SECONDS = 15780000; // 6 months. await bucket.setRetentionPeriod(DURATION_SECONDS); } -export async function bucketSetStorageClassInstancePrecondition(bucket: Bucket) { +export async function bucketSetStorageClassInstancePrecondition( + bucket: Bucket +) { await bucket.setStorageClass('nearline'); } -export async function bucketSetStorageClass(bucket: Bucket) { //TODO +export async function bucketSetStorageClass(bucket: Bucket) { + //TODO await bucket.setStorageClass('nearline'); } -export async function bucketUploadResumableInstancePrecondition(bucket: Bucket) { +export async function bucketUploadResumableInstancePrecondition( + bucket: Bucket +) { if (bucket.instancePreconditionOpts) { bucket.instancePreconditionOpts.ifGenerationMatch = 0; } @@ -304,11 +320,13 @@ export async function bucketUploadResumable(bucket: Bucket) { __dirname, '../../conformance-test/test-data/retryStrategyTestData.json' ), - {preconditionOpts: {ifMetagenerationMatch: 2, ifGenerationMatch:0}} + {preconditionOpts: {ifMetagenerationMatch: 2, ifGenerationMatch: 0}} ); } -export async function bucketUploadMultipartInstancePrecondition(bucket: Bucket) { +export async function bucketUploadMultipartInstancePrecondition( + bucket: Bucket +) { if (bucket.instancePreconditionOpts) { delete bucket.instancePreconditionOpts.ifMetagenerationMatch; bucket.instancePreconditionOpts.ifGenerationMatch = 0; @@ -344,7 +362,9 @@ export async function copyInstancePrecondition(_bucket: Bucket, file: File) { } export async function copy(_bucket: Bucket, file: File) { - await file.copy('a-different-file.png', {preconditionOpts: {ifGenerationMatch: 0}}); + await file.copy('a-different-file.png', { + preconditionOpts: {ifGenerationMatch: 0}, + }); } export async function createReadStream(_bucket: Bucket, file: File) { @@ -357,7 +377,10 @@ export async function createReadStream(_bucket: Bucket, file: File) { }); } -export async function createResumableUploadInstancePrecondition(_bucket: Bucket, file: File) { +export async function createResumableUploadInstancePrecondition( + _bucket: Bucket, + file: File +) { await file.createResumableUpload(); } @@ -365,11 +388,15 @@ export async function createResumableUpload(_bucket: Bucket, file: File) { await file.createResumableUpload({preconditionOpts: {ifGenerationMatch: 0}}); } -export async function fileDeleteInstancePrecondition(_bucket: Bucket, file: File) { +export async function fileDeleteInstancePrecondition( + _bucket: Bucket, + file: File +) { await file.delete(); } -export async function fileDelete(_bucket: Bucket, file: File) { //TODO +export async function fileDelete(_bucket: Bucket, file: File) { + //TODO await file.delete(); } @@ -397,11 +424,15 @@ export async function isPublic(_bucket: Bucket, file: File) { await file.isPublic(); } -export async function fileMakePrivateInstancePrecondition(_bucket: Bucket, file: File) { +export async function fileMakePrivateInstancePrecondition( + _bucket: Bucket, + file: File +) { await file.makePrivate(); } -export async function fileMakePrivate(_bucket: Bucket, file: File) { //TODO +export async function fileMakePrivate(_bucket: Bucket, file: File) { + //TODO await file.makePrivate(); } @@ -425,7 +456,10 @@ export async function rename(_bucket: Bucket, file: File) { await file.rename('new-name', {preconditionOpts: {ifGenerationMatch: 0}}); } -export async function rotateEncryptionKeyInstancePrecondition(_bucket: Bucket, file: File) { +export async function rotateEncryptionKeyInstancePrecondition( + _bucket: Bucket, + file: File +) { const crypto = require('crypto'); const buffer = crypto.randomBytes(32); const newKey = buffer.toString('base64'); @@ -434,7 +468,8 @@ export async function rotateEncryptionKeyInstancePrecondition(_bucket: Bucket, f }); } -export async function rotateEncryptionKey(_bucket: Bucket, file: File) { //TODO +export async function rotateEncryptionKey(_bucket: Bucket, file: File) { + //TODO const crypto = require('crypto'); const buffer = crypto.randomBytes(32); const newKey = buffer.toString('base64'); @@ -443,29 +478,44 @@ export async function rotateEncryptionKey(_bucket: Bucket, file: File) { //TODO }); } -export async function saveResumableInstancePrecondition(_bucket: Bucket, file: File) { +export async function saveResumableInstancePrecondition( + _bucket: Bucket, + file: File +) { await file.save('testdata', {resumable: true}); } export async function saveResumable(_bucket: Bucket, file: File) { - await file.save('testdata', {resumable: true, preconditionOpts: { - ifGenerationMatch: file.metadata.generation, - ifMetagenerationMatch: file.metadata.metageneration - }}); + await file.save('testdata', { + resumable: true, + preconditionOpts: { + ifGenerationMatch: file.metadata.generation, + ifMetagenerationMatch: file.metadata.metageneration, + }, + }); } -export async function saveMultipartInstancePrecondition(_bucket: Bucket, file: File) { +export async function saveMultipartInstancePrecondition( + _bucket: Bucket, + file: File +) { await file.save('testdata', {resumable: false}); } export async function saveMultipart(_bucket: Bucket, file: File) { - await file.save('testdata', {resumable: false, preconditionOpts: { - ifGenerationMatch: file.metadata.generation, - ifMetagenerationMatch: file.metadata.metageneration - }}); + await file.save('testdata', { + resumable: false, + preconditionOpts: { + ifGenerationMatch: file.metadata.generation, + ifMetagenerationMatch: file.metadata.metageneration, + }, + }); } -export async function setMetadataInstancePrecondition(_bucket: Bucket, file: File) { +export async function setMetadataInstancePrecondition( + _bucket: Bucket, + file: File +) { const metadata = { contentType: 'application/x-font-ttf', metadata: { @@ -476,7 +526,8 @@ export async function setMetadataInstancePrecondition(_bucket: Bucket, file: Fil await file.setMetadata(metadata); } -export async function setMetadata(_bucket: Bucket, file: File) { //TODO +export async function setMetadata(_bucket: Bucket, file: File) { + //TODO const metadata = { contentType: 'application/x-font-ttf', metadata: { @@ -487,14 +538,18 @@ export async function setMetadata(_bucket: Bucket, file: File) { //TODO await file.setMetadata(metadata); } -export async function setStorageClassInstancePrecondition(_bucket: Bucket, file: File) { +export async function setStorageClassInstancePrecondition( + _bucket: Bucket, + file: File +) { const result = await file.setStorageClass('nearline'); if (!result) { throw Error(); } } -export async function setStorageClass(_bucket: Bucket, file: File) { //TODO +export async function setStorageClass(_bucket: Bucket, file: File) { + //TODO const result = await file.setStorageClass('nearline'); if (!result) { throw Error(); diff --git a/src/bucket.ts b/src/bucket.ts index e6cb3b911..7761775ad 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -64,7 +64,7 @@ import { } from './signer'; import {Readable} from 'stream'; import {CRC32CValidatorGenerator} from './crc32c'; -import { options } from 'yargs'; +import {options} from 'yargs'; interface SourceObject { name: string; @@ -81,7 +81,8 @@ interface MetadataOptions { ifGenerationMatch?: number; ifGenerationNotMatch?: number; ifMetagenerationMatch?: number; - ifMetagenerationNotMatch?: number;} + ifMetagenerationNotMatch?: number; +} export type GetFilesResponse = [File[], {}, Metadata]; export interface GetFilesCallback { @@ -116,7 +117,7 @@ export interface LifecycleRule { export interface EnableLoggingOptions { bucket?: string | Bucket; prefix: string; - preconditionOpts?: PreconditionOptions + preconditionOpts?: PreconditionOptions; } export interface GetFilesOptions { @@ -293,7 +294,7 @@ export interface MakeBucketPrivateOptions { force?: boolean; metadata?: Metadata; userProject?: string; - preconditionOpts?: PreconditionOptions + preconditionOpts?: PreconditionOptions; } interface MakeBucketPrivateRequest extends MakeBucketPrivateOptions { @@ -2213,12 +2214,15 @@ class Bucket extends ServiceObject { AvailableServiceObjectMethods.setMetadata, config.preconditionOpts ); - [setMetadataResponse] = await this.setMetadata({ - logging: { - logBucket, - logObjectPrefix: config.prefix, - } - }, config.preconditionOpts); + [setMetadataResponse] = await this.setMetadata( + { + logging: { + logBucket, + logObjectPrefix: config.prefix, + }, + }, + config.preconditionOpts + ); } catch (e) { callback!(e as Error); return; @@ -3091,20 +3095,24 @@ class Bucket extends ServiceObject { } if (options.preconditionOpts?.ifGenerationNotMatch) { - query.ifGenerationNotMatch = options.preconditionOpts.ifGenerationNotMatch; + query.ifGenerationNotMatch = + options.preconditionOpts.ifGenerationNotMatch; } if (options.preconditionOpts?.ifMetagenerationMatch) { - query.ifMetagenerationMatch = options.preconditionOpts.ifMetagenerationMatch; + query.ifMetagenerationMatch = + options.preconditionOpts.ifMetagenerationMatch; } if (options.preconditionOpts?.ifMetagenerationNotMatch) { - query.ifMetagenerationNotMatch = options.preconditionOpts.ifMetagenerationNotMatch; + query.ifMetagenerationNotMatch = + options.preconditionOpts.ifMetagenerationNotMatch; } this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata, options.preconditionOpts + AvailableServiceObjectMethods.setMetadata, + options.preconditionOpts ); // You aren't allowed to set both predefinedAcl & acl properties on a bucket @@ -4162,11 +4170,13 @@ class Bucket extends ServiceObject { // eslint-disable-next-line @typescript-eslint/no-explicit-any coreOpts: any, // eslint-disable-next-line @typescript-eslint/no-explicit-any - methodType: AvailableServiceObjectMethods, localPreconditionOptions?: PreconditionOptions + methodType: AvailableServiceObjectMethods, + localPreconditionOptions?: PreconditionOptions ): void { if ( - (typeof coreOpts === 'object' && - coreOpts?.reqOpts?.qs?.ifMetagenerationMatch === undefined && localPreconditionOptions?.ifMetagenerationMatch === undefined) && + typeof coreOpts === 'object' && + coreOpts?.reqOpts?.qs?.ifMetagenerationMatch === undefined && + localPreconditionOptions?.ifMetagenerationMatch === undefined && (methodType === AvailableServiceObjectMethods.setMetadata || methodType === AvailableServiceObjectMethods.delete) && this.storage.retryOptions.idempotencyStrategy === From daa9eb7c92f18fc199a7c3a207544482bf80aa3a Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Thu, 21 Jul 2022 23:21:53 +0000 Subject: [PATCH 08/58] implemented more preconditions --- conformance-test/libraryMethods.ts | 16 ++++++++++++++- src/bucket.ts | 31 ++++++++++++++++++++++-------- src/nodejs-common/util.ts | 2 ++ 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 9c1a7f960..a4b961d3f 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -15,6 +15,7 @@ import {Bucket, File, Notification, Storage, HmacKey} from '../src'; import * as path from 'path'; import {ApiError} from '../src/nodejs-common'; +import { options } from 'yargs'; const METAGENERATION_PRECONDITION = { ifMetagenerationMatch: 2, @@ -40,6 +41,11 @@ export async function addLifecycleRule(bucket: Bucket) { condition: { age: 365 * 3, // Specified in days. }, + }, + { + preconditionOpts: { + ifMetagenerationMatch: 2, + }, }); } @@ -142,7 +148,11 @@ export async function disableRequesterPaysInstancePrecondition(bucket: Bucket) { export async function disableRequesterPays(bucket: Bucket) { //TODO -- no options - await bucket.disableRequesterPays(); + bucket.disableRequesterPays(() => {}, { + preconditionOpts: { + ifMetagenerationMatch: 2, + } + }); } export async function enableLoggingInstancePrecondition(bucket: Bucket) { @@ -751,3 +761,7 @@ export async function getServiceAccount( ) { await storage.getServiceAccount(); } +function preconditionOpts(arg0: () => void, preconditionOpts: any, arg2: { ifMetagenerationMatch: number; }) { + throw new Error('Function not implemented.'); +} + diff --git a/src/bucket.ts b/src/bucket.ts index 63caa52c9..7270a1467 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -65,6 +65,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; @@ -106,6 +107,7 @@ interface WatchAllOptions { export interface AddLifecycleRuleOptions { append?: boolean; + preconditionOpts?: PreconditionOptions } export interface LifecycleRule { @@ -202,6 +204,10 @@ export type DeleteLabelsResponse = [Metadata]; export type DeleteLabelsCallback = SetLabelsCallback; +export interface DisableRequesterPaysOptions { + preconditionOpts?: PreconditionOptions +} + export type DisableRequesterPaysResponse = [Metadata]; export interface DisableRequesterPaysCallback { @@ -1367,11 +1373,16 @@ class Bucket extends ServiceObject { this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + AvailableServiceObjectMethods.setMetadata, + options.preconditionOpts ); if (options.append === false) { - this.setMetadata({lifecycle: {rule: newLifecycleRules}}) + this.setMetadata({ + lifecycle: { + rule: newLifecycleRules, + }, + }, options?.preconditionOpts) //TODO: pass precondition options properly .then(resp => callback!(null, ...resp)) .catch(callback!) .finally(() => { @@ -1395,8 +1406,8 @@ class Bucket extends ServiceObject { this.setMetadata({ lifecycle: { rule: currentLifecycleRules.concat(newLifecycleRules), - }, - }) + } + }) .then(resp => callback!(null, ...resp)) .catch(callback!) .finally(() => { @@ -2123,6 +2134,9 @@ class Bucket extends ServiceObject { disableRequesterPays(): Promise; disableRequesterPays(callback: DisableRequesterPaysCallback): void; + disableRequesterPays( + callback: DisableRequesterPaysCallback, options: DisableRequesterPaysOptions, + ): Promise | void; /** * @typedef {array} DisableRequesterPaysResponse * @property {object} 0 The full API response. @@ -2170,19 +2184,20 @@ class Bucket extends ServiceObject { * Example of disabling requester pays: */ disableRequesterPays( - callback?: DisableRequesterPaysCallback - ): Promise | void { + callback?: DisableRequesterPaysCallback, options?: DisableRequesterPaysOptions, + ): Promise | void { const cb = callback || util.noop; this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + AvailableServiceObjectMethods.setMetadata, + options?.preconditionOpts ); this.setMetadata({ billing: { requesterPays: false, }, - }) + }, options?.preconditionOpts) .then(resp => cb(null, ...resp)) .catch(cb) .finally(() => { diff --git a/src/nodejs-common/util.ts b/src/nodejs-common/util.ts index 8b1d6d990..9237b8655 100644 --- a/src/nodejs-common/util.ts +++ b/src/nodejs-common/util.ts @@ -873,6 +873,8 @@ export class Util { totalTimeout: config.retryOptions?.totalTimeout, } as {} as retryRequest.Options; + console.log(options) + if (typeof reqOpts.maxRetries === 'number') { options.retries = reqOpts.maxRetries; } From bff28624b56a27b12f6d6f3691e49c1b135825df Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Thu, 21 Jul 2022 23:24:34 +0000 Subject: [PATCH 09/58] general cleanup --- conformance-test/libraryMethods.ts | 8 -------- src/bucket.ts | 1 - src/nodejs-common/util.ts | 2 -- 3 files changed, 11 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index a4b961d3f..7dda72793 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -15,11 +15,6 @@ import {Bucket, File, Notification, Storage, HmacKey} from '../src'; import * as path from 'path'; import {ApiError} from '../src/nodejs-common'; -import { options } from 'yargs'; - -const METAGENERATION_PRECONDITION = { - ifMetagenerationMatch: 2, -}; ///////////////////////////////////////////////// //////////////////// BUCKET ///////////////////// @@ -761,7 +756,4 @@ export async function getServiceAccount( ) { await storage.getServiceAccount(); } -function preconditionOpts(arg0: () => void, preconditionOpts: any, arg2: { ifMetagenerationMatch: number; }) { - throw new Error('Function not implemented.'); -} diff --git a/src/bucket.ts b/src/bucket.ts index 7270a1467..6b2748bfc 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -65,7 +65,6 @@ import { import {Readable} from 'stream'; import {CRC32CValidatorGenerator} from './crc32c'; import {URL} from 'url'; -import { SetMetadataOptions } from './nodejs-common/service-object'; interface SourceObject { name: string; diff --git a/src/nodejs-common/util.ts b/src/nodejs-common/util.ts index 9237b8655..8b1d6d990 100644 --- a/src/nodejs-common/util.ts +++ b/src/nodejs-common/util.ts @@ -873,8 +873,6 @@ export class Util { totalTimeout: config.retryOptions?.totalTimeout, } as {} as retryRequest.Options; - console.log(options) - if (typeof reqOpts.maxRetries === 'number') { options.retries = reqOpts.maxRetries; } From a192cf99761804725d365e8458d3d2467ea57d02 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Fri, 22 Jul 2022 14:40:13 +0000 Subject: [PATCH 10/58] implemented precondition on combine --- conformance-test/libraryMethods.ts | 44 ++++++------------------------ src/bucket.ts | 41 ++++++++++------------------ 2 files changed, 22 insertions(+), 63 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 7dda72793..0a0d58ca3 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -45,58 +45,30 @@ export async function addLifecycleRule(bucket: Bucket) { } export async function combineInstancePrecondition(bucket: Bucket) { - bucket = new Bucket(bucket.storage, bucket.name, { - preconditionOpts: { - ifMetagenerationMatch: 1, - }, - }); const file1 = bucket.file('file1.txt'); const file2 = 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, { + const sources = [file1, file2]; + const allFiles = bucket.file('all-files.txt', { preconditionOpts: { - ifGenerationMatch: file2.metadata.generation, - }, + ifGenerationMatch: 0, + } }); - 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 - await allFiles.save('allfiles contents'); await bucket.combine(sources, allFiles); } export async function combine(bucket: Bucket) { - //TODO - bucket = new Bucket(bucket.storage, bucket.name, { - preconditionOpts: { - ifMetagenerationMatch: 1, - }, - }); const file1 = bucket.file('file1.txt'); const file2 = 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 sources = [file1, file2]; 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 await allFiles.save('allfiles contents'); - await bucket.combine(sources, allFiles); + await bucket.combine(sources, allFiles, { + ifGenerationMatch: allFiles.metadata.generation, + }); } export async function create(bucket: Bucket) { diff --git a/src/bucket.ts b/src/bucket.ts index 6b2748bfc..4254d3151 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -138,6 +138,7 @@ export interface GetFilesOptions { export interface CombineOptions extends PreconditionOptions { kmsKeyName?: string; userProject?: string; + // TODO (breaking change): instead of extending precondition options, define it as preconditionOpts } export interface CombineCallback { @@ -1543,19 +1544,20 @@ 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( @@ -1567,22 +1569,7 @@ class Bucket extends ServiceObject { destination: { contentType: destinationFile.metadata.contentType, }, - sourceObjects: (sources as File[]).map(source => { - const sourceObject = { - name: source.name, - } as SourceObject; - - if ( - source?.metadata?.generation || - source?.instancePreconditionOpts?.ifGenerationMatch - ) { - sourceObject.generation = - source?.metadata?.generation || - source?.instancePreconditionOpts?.ifGenerationMatch; - } - - return sourceObject; - }), + sourceObjects: sources }, qs: options, }, From f65883a173a6652d250c66bf676a16d86310e8ca Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 25 Jul 2022 19:45:57 +0000 Subject: [PATCH 11/58] added preconditions for copy and move --- conformance-test/libraryMethods.ts | 15 +++++---------- .../test-data/retryInvocationMap.json | 2 -- src/file.ts | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 0a0d58ca3..837dbb412 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -334,13 +334,12 @@ export async function bucketUploadMultipart(bucket: Bucket) { //////////////////// FILE ///////////////////// ///////////////////////////////////////////////// -export async function copyInstancePrecondition(_bucket: Bucket, file: File) { - await file.copy('a-different-file.png'); -} - -export async function copy(_bucket: Bucket, file: File) { +export async function copy(bucket: Bucket, file: File) { + const newFile = new File(bucket, 'a-different-file.png'); + await newFile.save('a-different-file.png'); + await file.copy('a-different-file.png', { - preconditionOpts: {ifGenerationMatch: 0}, + preconditionOpts: {ifGenerationMatch: newFile.metadata.generation}, }); } @@ -417,10 +416,6 @@ export async function fileMakePublic(_bucket: Bucket, file: File) { await file.makePublic(); } -export async function moveInstancePrecondition(_bucket: Bucket, file: File) { - await file.move('new-file'); -} - export async function move(_bucket: Bucket, file: File) { await file.move('new-file', {preconditionOpts: {ifGenerationMatch: 0}}); } diff --git a/conformance-test/test-data/retryInvocationMap.json b/conformance-test/test-data/retryInvocationMap.json index 528500eb9..b773a14e4 100644 --- a/conformance-test/test-data/retryInvocationMap.json +++ b/conformance-test/test-data/retryInvocationMap.json @@ -60,8 +60,6 @@ "fileMakePublic" ], "storage.objects.rewrite": [ - "copyInstancePrecondition", - "moveInstancePrecondition", "renameInstancePrecondition", "rotateEncryptionKeyInstancePrecondition", "setStorageClassInstancePrecondition", diff --git a/src/file.ts b/src/file.ts index 6c7fc5cf0..70f45826d 100644 --- a/src/file.ts +++ b/src/file.ts @@ -1226,6 +1226,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 +1250,7 @@ class File extends ServiceObject { headers, }, (err, resp) => { + this.storage.retryOptions.autoRetry = this.instanceRetryValue; if (err) { callback!(err, null, resp); return; From 6f98536b4eca4ec6c70c050383fce69de2377db0 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 25 Jul 2022 19:49:16 +0000 Subject: [PATCH 12/58] rename --- conformance-test/libraryMethods.ts | 4 ---- conformance-test/test-data/retryInvocationMap.json | 1 - 2 files changed, 5 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 837dbb412..a98eb554f 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -420,10 +420,6 @@ export async function move(_bucket: Bucket, file: File) { await file.move('new-file', {preconditionOpts: {ifGenerationMatch: 0}}); } -export async function renameInstancePrecondition(_bucket: Bucket, file: File) { - await file.rename('new-name'); -} - export async function rename(_bucket: Bucket, file: File) { await file.rename('new-name', {preconditionOpts: {ifGenerationMatch: 0}}); } diff --git a/conformance-test/test-data/retryInvocationMap.json b/conformance-test/test-data/retryInvocationMap.json index b773a14e4..a5f414f43 100644 --- a/conformance-test/test-data/retryInvocationMap.json +++ b/conformance-test/test-data/retryInvocationMap.json @@ -60,7 +60,6 @@ "fileMakePublic" ], "storage.objects.rewrite": [ - "renameInstancePrecondition", "rotateEncryptionKeyInstancePrecondition", "setStorageClassInstancePrecondition", "copy", From b71a6f9fb1f43a8449d83856210cb3be373fa21e Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 25 Jul 2022 19:52:06 +0000 Subject: [PATCH 13/58] removed tests for instance precondition where instance precondition is not supported --- conformance-test/libraryMethods.ts | 22 ------------------- .../test-data/retryInvocationMap.json | 2 -- 2 files changed, 24 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index a98eb554f..1b503ff03 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -424,18 +424,6 @@ export async function rename(_bucket: Bucket, file: File) { await file.rename('new-name', {preconditionOpts: {ifGenerationMatch: 0}}); } -export async function rotateEncryptionKeyInstancePrecondition( - _bucket: Bucket, - file: File -) { - const crypto = require('crypto'); - const buffer = crypto.randomBytes(32); - const newKey = buffer.toString('base64'); - await file.rotateEncryptionKey({ - encryptionKey: Buffer.from(newKey, 'base64'), - }); -} - export async function rotateEncryptionKey(_bucket: Bucket, file: File) { //TODO const crypto = require('crypto'); @@ -506,16 +494,6 @@ export async function setMetadata(_bucket: Bucket, file: File) { await file.setMetadata(metadata); } -export async function setStorageClassInstancePrecondition( - _bucket: Bucket, - file: File -) { - const result = await file.setStorageClass('nearline'); - if (!result) { - throw Error(); - } -} - export async function setStorageClass(_bucket: Bucket, file: File) { //TODO const result = await file.setStorageClass('nearline'); diff --git a/conformance-test/test-data/retryInvocationMap.json b/conformance-test/test-data/retryInvocationMap.json index a5f414f43..1a054d588 100644 --- a/conformance-test/test-data/retryInvocationMap.json +++ b/conformance-test/test-data/retryInvocationMap.json @@ -60,8 +60,6 @@ "fileMakePublic" ], "storage.objects.rewrite": [ - "rotateEncryptionKeyInstancePrecondition", - "setStorageClassInstancePrecondition", "copy", "move", "rename", From d48c6cf889e0b3c8d2dbdf0dd861264047b42399 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 25 Jul 2022 20:01:15 +0000 Subject: [PATCH 14/58] support preconditions for rotateencryptionkey --- conformance-test/libraryMethods.ts | 2 +- src/file.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 1b503ff03..3b4a2d2cf 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -425,12 +425,12 @@ export async function rename(_bucket: Bucket, file: File) { } export async function rotateEncryptionKey(_bucket: Bucket, file: File) { - //TODO const crypto = require('crypto'); const buffer = crypto.randomBytes(32); const newKey = buffer.toString('base64'); await file.rotateEncryptionKey({ encryptionKey: Buffer.from(newKey, 'base64'), + preconditionOpts: {ifGenerationMatch: file.metadata.generation} }); } diff --git a/src/file.ts b/src/file.ts index 70f45826d..3bbf02d21 100644 --- a/src/file.ts +++ b/src/file.ts @@ -264,6 +264,7 @@ export type RotateEncryptionKeyOptions = string | Buffer | EncryptionKeyOptions; export interface EncryptionKeyOptions { encryptionKey?: string | Buffer; kmsKeyName?: string; + preconditionOpts?: PreconditionOptions } export type RotateEncryptionKeyCallback = CopyCallback; @@ -3547,7 +3548,8 @@ 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; From e5afd02c5da9487286e81a3fca4a692a29bab125 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 25 Jul 2022 20:03:29 +0000 Subject: [PATCH 15/58] support set storage class --- conformance-test/libraryMethods.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 3b4a2d2cf..cb562edf5 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -495,8 +495,11 @@ export async function setMetadata(_bucket: Bucket, file: File) { } export async function setStorageClass(_bucket: Bucket, file: File) { - //TODO - const result = await file.setStorageClass('nearline'); + const result = await file.setStorageClass('nearline', { + preconditionOpts: { + ifGenerationMatch: file.metadata.generation + } + }); if (!result) { throw Error(); } From d09cddbf9bd77d36401eb3cd554bcc0f2e83f521 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 25 Jul 2022 20:15:19 +0000 Subject: [PATCH 16/58] linted files --- conformance-test/libraryMethods.ts | 37 +++++++++++------------ src/bucket.ts | 47 ++++++++++++++++++------------ src/file.ts | 7 +++-- 3 files changed, 52 insertions(+), 39 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index cb562edf5..3a38e9d32 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -31,17 +31,19 @@ export async function addLifecycleRuleInstancePrecondition(bucket: Bucket) { export async function addLifecycleRule(bucket: Bucket) { //TODO - await bucket.addLifecycleRule({ - action: 'delete', - condition: { - age: 365 * 3, // Specified in days. - }, - }, - { - preconditionOpts: { - ifMetagenerationMatch: 2, + await bucket.addLifecycleRule( + { + action: 'delete', + condition: { + age: 365 * 3, // Specified in days. + }, }, - }); + { + preconditionOpts: { + ifMetagenerationMatch: 2, + }, + } + ); } export async function combineInstancePrecondition(bucket: Bucket) { @@ -53,7 +55,7 @@ export async function combineInstancePrecondition(bucket: Bucket) { const allFiles = bucket.file('all-files.txt', { preconditionOpts: { ifGenerationMatch: 0, - } + }, }); await bucket.combine(sources, allFiles); } @@ -67,7 +69,7 @@ export async function combine(bucket: Bucket) { const allFiles = bucket.file('all-files.txt'); await allFiles.save('allfiles contents'); await bucket.combine(sources, allFiles, { - ifGenerationMatch: allFiles.metadata.generation, + ifGenerationMatch: allFiles.metadata.generation, }); } @@ -118,7 +120,7 @@ export async function disableRequesterPays(bucket: Bucket) { bucket.disableRequesterPays(() => {}, { preconditionOpts: { ifMetagenerationMatch: 2, - } + }, }); } @@ -337,7 +339,7 @@ export async function bucketUploadMultipart(bucket: Bucket) { export async function copy(bucket: Bucket, file: File) { const newFile = new File(bucket, 'a-different-file.png'); await newFile.save('a-different-file.png'); - + await file.copy('a-different-file.png', { preconditionOpts: {ifGenerationMatch: newFile.metadata.generation}, }); @@ -430,7 +432,7 @@ export async function rotateEncryptionKey(_bucket: Bucket, file: File) { const newKey = buffer.toString('base64'); await file.rotateEncryptionKey({ encryptionKey: Buffer.from(newKey, 'base64'), - preconditionOpts: {ifGenerationMatch: file.metadata.generation} + preconditionOpts: {ifGenerationMatch: file.metadata.generation}, }); } @@ -497,8 +499,8 @@ export async function setMetadata(_bucket: Bucket, file: File) { export async function setStorageClass(_bucket: Bucket, file: File) { const result = await file.setStorageClass('nearline', { preconditionOpts: { - ifGenerationMatch: file.metadata.generation - } + ifGenerationMatch: file.metadata.generation, + }, }); if (!result) { throw Error(); @@ -700,4 +702,3 @@ export async function getServiceAccount( ) { await storage.getServiceAccount(); } - diff --git a/src/bucket.ts b/src/bucket.ts index 4254d3151..24cd1339d 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -106,7 +106,7 @@ interface WatchAllOptions { export interface AddLifecycleRuleOptions { append?: boolean; - preconditionOpts?: PreconditionOptions + preconditionOpts?: PreconditionOptions; } export interface LifecycleRule { @@ -205,7 +205,7 @@ export type DeleteLabelsResponse = [Metadata]; export type DeleteLabelsCallback = SetLabelsCallback; export interface DisableRequesterPaysOptions { - preconditionOpts?: PreconditionOptions + preconditionOpts?: PreconditionOptions; } export type DisableRequesterPaysResponse = [Metadata]; @@ -1378,11 +1378,14 @@ class Bucket extends ServiceObject { ); if (options.append === false) { - this.setMetadata({ - lifecycle: { - rule: newLifecycleRules, + this.setMetadata( + { + lifecycle: { + rule: newLifecycleRules, + }, }, - }, options?.preconditionOpts) //TODO: pass precondition options properly + options?.preconditionOpts + ) //TODO: pass precondition options properly .then(resp => callback!(null, ...resp)) .catch(callback!) .finally(() => { @@ -1406,8 +1409,8 @@ class Bucket extends ServiceObject { this.setMetadata({ lifecycle: { rule: currentLifecycleRules.concat(newLifecycleRules), - } - }) + }, + }) .then(resp => callback!(null, ...resp)) .catch(callback!) .finally(() => { @@ -1545,7 +1548,8 @@ class Bucket extends ServiceObject { let maxRetries = this.storage.retryOptions.maxRetries; if ( - (destinationFile?.instancePreconditionOpts?.ifGenerationMatch === undefined && + (destinationFile?.instancePreconditionOpts?.ifGenerationMatch === + undefined && options.ifGenerationMatch === undefined && this.storage.retryOptions.idempotencyStrategy === IdempotencyStrategy.RetryConditional) || @@ -1555,7 +1559,7 @@ class Bucket extends ServiceObject { maxRetries = 0; } - if (options.ifGenerationMatch === undefined){ + if (options.ifGenerationMatch === undefined) { Object.assign(options, destinationFile.instancePreconditionOpts, options); } @@ -1569,7 +1573,7 @@ class Bucket extends ServiceObject { destination: { contentType: destinationFile.metadata.contentType, }, - sourceObjects: sources + sourceObjects: sources, }, qs: options, }, @@ -2121,8 +2125,9 @@ class Bucket extends ServiceObject { disableRequesterPays(): Promise; disableRequesterPays(callback: DisableRequesterPaysCallback): void; disableRequesterPays( - callback: DisableRequesterPaysCallback, options: DisableRequesterPaysOptions, - ): Promise | void; + callback: DisableRequesterPaysCallback, + options: DisableRequesterPaysOptions + ): Promise | void; /** * @typedef {array} DisableRequesterPaysResponse * @property {object} 0 The full API response. @@ -2170,8 +2175,9 @@ class Bucket extends ServiceObject { * Example of disabling requester pays: */ disableRequesterPays( - callback?: DisableRequesterPaysCallback, options?: DisableRequesterPaysOptions, - ): Promise | void { + callback?: DisableRequesterPaysCallback, + options?: DisableRequesterPaysOptions + ): Promise | void { const cb = callback || util.noop; this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, @@ -2179,11 +2185,14 @@ class Bucket extends ServiceObject { options?.preconditionOpts ); - this.setMetadata({ - billing: { - requesterPays: false, + this.setMetadata( + { + billing: { + requesterPays: false, + }, }, - }, options?.preconditionOpts) + options?.preconditionOpts + ) .then(resp => cb(null, ...resp)) .catch(cb) .finally(() => { diff --git a/src/file.ts b/src/file.ts index 3bbf02d21..f01abca8d 100644 --- a/src/file.ts +++ b/src/file.ts @@ -264,7 +264,7 @@ export type RotateEncryptionKeyOptions = string | Buffer | EncryptionKeyOptions; export interface EncryptionKeyOptions { encryptionKey?: string | Buffer; kmsKeyName?: string; - preconditionOpts?: PreconditionOptions + preconditionOpts?: PreconditionOptions; } export type RotateEncryptionKeyCallback = CopyCallback; @@ -3548,7 +3548,10 @@ class File extends ServiceObject { } const newFile = this.bucket.file(this.id!, options); - const copyOptions = options.preconditionOpts?.ifGenerationMatch !== undefined ? {preconditionOpts: options.preconditionOpts} : {} + const copyOptions = + options.preconditionOpts?.ifGenerationMatch !== undefined + ? {preconditionOpts: options.preconditionOpts} + : {}; this.copy(newFile, copyOptions, callback!); } From fccd871ab3340a7aeddf07352d678197d0354554 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 25 Jul 2022 20:25:38 +0000 Subject: [PATCH 17/58] fixed tests --- src/bucket.ts | 14 ++++++++++++-- test/bucket.ts | 36 ------------------------------------ test/file.ts | 3 ++- 3 files changed, 14 insertions(+), 39 deletions(-) diff --git a/src/bucket.ts b/src/bucket.ts index 24cd1339d..900228eb1 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -68,7 +68,7 @@ import {URL} from 'url'; interface SourceObject { name: string; - generation?: number; + generation?: number; } interface CreateNotificationQuery { @@ -1573,7 +1573,17 @@ class Bucket extends ServiceObject { destination: { contentType: destinationFile.metadata.contentType, }, - sourceObjects: sources, + sourceObjects: (sources as File[]).map(source => { + const sourceObject = { + name: source.name, + } as SourceObject; + + if (source.metadata && source.metadata.generation) { + sourceObject.generation = source.metadata.generation; + } + + return sourceObject; + }), }, qs: options, }, diff --git a/test/bucket.ts b/test/bucket.ts index 44243cb14..097ca9dae 100644 --- a/test/bucket.ts +++ b/test/bucket.ts @@ -853,42 +853,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'); diff --git a/test/file.ts b/test/file.ts index 9c9a2ffe8..8f17d2afa 100644 --- a/test/file.ts +++ b/test/file.ts @@ -4173,8 +4173,9 @@ 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() }; From 7a95592fb462fbaeab77f1370b02743a8382c6dc Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Tue, 26 Jul 2022 15:56:33 +0000 Subject: [PATCH 18/58] deleteLabels and setLabels --- conformance-test/libraryMethods.ts | 11 +++++--- src/bucket.ts | 44 +++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 3a38e9d32..1714cd3af 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -107,8 +107,10 @@ export async function deleteLabelsInstancePrecondition(bucket: Bucket) { } export async function deleteLabels(bucket: Bucket) { - //TODO -- no options to modify - await bucket.deleteLabels(); + //TODO: not retrying for some reason + bucket.deleteLabels({ + ifMetagenerationMatch: 2 + }); } export async function disableRequesterPaysInstancePrecondition(bucket: Bucket) { @@ -227,12 +229,13 @@ export async function setLabelsInstancePrecondition(bucket: Bucket) { } export async function setLabels(bucket: Bucket) { - //TODO const labels = { labelone: 'labelonevalue', labeltwo: 'labeltwovalue', }; - await bucket.setLabels(labels); + await bucket.setLabels(labels, { + ifMetagenerationMatch: 2 + }); } export async function bucketSetMetadataInstancePrecondition(bucket: Bucket) { diff --git a/src/bucket.ts b/src/bucket.ts index 900228eb1..2bfd74882 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -138,7 +138,6 @@ export interface GetFilesOptions { export interface CombineOptions extends PreconditionOptions { kmsKeyName?: string; userProject?: string; - // TODO (breaking change): instead of extending precondition options, define it as preconditionOpts } export interface CombineCallback { @@ -203,6 +202,7 @@ export interface DeleteFilesCallback { export type DeleteLabelsResponse = [Metadata]; export type DeleteLabelsCallback = SetLabelsCallback; +export type DeleteLabelsOptions = PreconditionOptions; export interface DisableRequesterPaysOptions { preconditionOpts?: PreconditionOptions; @@ -344,7 +344,7 @@ export interface Labels { [key: string]: string; } -export interface SetLabelsOptions { +export interface SetLabelsOptions extends PreconditionOptions{ userProject?: string; } @@ -2048,8 +2048,8 @@ class Bucket extends ServiceObject { } deleteLabels(labels?: string | string[]): Promise; - deleteLabels(callback: DeleteLabelsCallback): void; - deleteLabels(labels: string | string[], callback: DeleteLabelsCallback): void; + deleteLabels(optionsOrCallback: DeleteLabelsCallback | DeleteLabelsOptions ): void; + deleteLabels(labels: string | string[], optionsOrCallback: DeleteLabelsCallback | DeleteLabelsOptions): void; /** * @typedef {array} DeleteLabelsResponse * @property {object} 0 The full API response. @@ -2064,7 +2064,8 @@ 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 {DeleteLabelsCallback | DeleteLabelsOptions} [optionsOrCallback] + * Callback function or precondition options. * @returns {Promise} * * @example @@ -2100,14 +2101,25 @@ class Bucket extends ServiceObject { * ``` */ deleteLabels( - labelsOrCallback?: string | string[] | DeleteLabelsCallback, - callback?: DeleteLabelsCallback + labelsOrCallbackOrOptions?: string | string[] | DeleteLabelsCallback | DeleteLabelsOptions, + optionsOrCallback?: DeleteLabelsCallback | DeleteLabelsOptions, ): Promise | void { let labels = new Array(); - if (typeof labelsOrCallback === 'function') { - callback = labelsOrCallback; - } else if (labelsOrCallback) { - labels = arrify(labelsOrCallback); + let options: DeleteLabelsOptions = {}; + let callback: DeleteLabelsCallback; + + 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[]) => { @@ -2116,7 +2128,12 @@ 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) { @@ -3528,8 +3545,9 @@ class Bucket extends ServiceObject { this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + AvailableServiceObjectMethods.setMetadata, options ); + this.setMetadata({labels}, options) .then(resp => callback!(null, ...resp)) .catch(callback!) From 408b45cf10e75ded1fabfd4c12644ef96e01c6d5 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Tue, 26 Jul 2022 17:19:32 +0000 Subject: [PATCH 19/58] more precondition implementations --- conformance-test/libraryMethods.ts | 31 +++++++++++----- src/bucket.ts | 56 ++++++++++++++++++++++------- src/nodejs-common/service-object.ts | 10 +++++- 3 files changed, 74 insertions(+), 23 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 1714cd3af..09e898cf3 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -30,7 +30,7 @@ export async function addLifecycleRuleInstancePrecondition(bucket: Bucket) { } export async function addLifecycleRule(bucket: Bucket) { - //TODO + //TODO -- swallowing options into function[anonymous] await bucket.addLifecycleRule( { action: 'delete', @@ -99,7 +99,9 @@ export async function deleteFilesInstancePrecondition(bucket: Bucket) { export async function deleteFiles(bucket: Bucket) { //TODO - await bucket.deleteFiles(); + await bucket.deleteFiles({ + ifMetagenerationMatch: 2 + }); } export async function deleteLabelsInstancePrecondition(bucket: Bucket) { @@ -108,8 +110,10 @@ export async function deleteLabelsInstancePrecondition(bucket: Bucket) { export async function deleteLabels(bucket: Bucket) { //TODO: not retrying for some reason - bucket.deleteLabels({ - ifMetagenerationMatch: 2 + await bucket.deleteLabels({ + preconditionOpts: { + ifMetagenerationMatch: 2 + } }); } @@ -118,8 +122,8 @@ export async function disableRequesterPaysInstancePrecondition(bucket: Bucket) { } export async function disableRequesterPays(bucket: Bucket) { - //TODO -- no options - bucket.disableRequesterPays(() => {}, { + //TODO: not retrying for some reason + await bucket.disableRequesterPays(() => {}, { preconditionOpts: { ifMetagenerationMatch: 2, }, @@ -148,7 +152,12 @@ export async function enableRequesterPaysInstancePrecondition(bucket: Bucket) { } export async function enableRequesterPays(bucket: Bucket) { - await bucket.enableRequesterPays(); //TODO + // TODO -- the promise isnt resolving and test is timing out + await bucket.enableRequesterPays({ + preconditionOpts: { + ifMetagenerationMatch: 2 + } + }); } export async function bucketExists(bucket: Bucket) { @@ -205,8 +214,12 @@ export async function removeRetentionPeriodInstancePrecondition( } export async function removeRetentionPeriod(bucket: Bucket) { - //TODO - await bucket.removeRetentionPeriod(); + //TODO -- the promise isnt resolving and test is timing out + await bucket.removeRetentionPeriod({ + preconditionOpts: { + ifMetagenerationMatch: 2 + } + }); } export async function setCorsConfigurationInstancePrecondition(bucket: Bucket) { diff --git a/src/bucket.ts b/src/bucket.ts index 2bfd74882..a286d958c 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -202,7 +202,9 @@ export interface DeleteFilesCallback { export type DeleteLabelsResponse = [Metadata]; export type DeleteLabelsCallback = SetLabelsCallback; -export type DeleteLabelsOptions = PreconditionOptions; +export interface DeleteLabelsOptions { + preconditionOpts?: PreconditionOptions +} export interface DisableRequesterPaysOptions { preconditionOpts?: PreconditionOptions; @@ -220,6 +222,9 @@ export interface EnableRequesterPaysCallback { (err?: Error | null, apiResponse?: Metadata): void; } +export interface EnableRequesterPaysOptions { + preconditionOpts?: PreconditionOptions; +} export interface BucketExistsOptions extends GetConfig { userProject?: string; } @@ -326,6 +331,7 @@ export type MakeBucketPublicResponse = [File[]]; export interface SetBucketMetadataOptions { userProject?: string; + preconditionOpts?: PreconditionOptions } export type SetBucketMetadataResponse = [Metadata]; @@ -2048,8 +2054,10 @@ class Bucket extends ServiceObject { } deleteLabels(labels?: string | string[]): Promise; - deleteLabels(optionsOrCallback: DeleteLabelsCallback | DeleteLabelsOptions ): void; - deleteLabels(labels: string | string[], optionsOrCallback: DeleteLabelsCallback | DeleteLabelsOptions): void; + deleteLabels(options: DeleteLabelsOptions ): Promise; + deleteLabels(callback: DeleteLabelsCallback): void; + deleteLabels(labels: string | string[], options: DeleteLabelsOptions): Promise; + deleteLabels(labels: string | string[], callback: DeleteLabelsCallback): void; /** * @typedef {array} DeleteLabelsResponse * @property {object} 0 The full API response. @@ -2128,8 +2136,8 @@ class Bucket extends ServiceObject { return nullLabelMap; }, {}); - if (options?.ifMetagenerationMatch !== undefined) { - this.setLabels(nullLabelMap, options, callback!); + if (options?.preconditionOpts?.ifMetagenerationMatch !== undefined) { + this.setLabels(nullLabelMap, options.preconditionOpts, callback!); } else { this.setLabels(nullLabelMap, callback!); @@ -2345,6 +2353,7 @@ class Bucket extends ServiceObject { enableRequesterPays(): Promise; enableRequesterPays(callback: EnableRequesterPaysCallback): void; + enableRequesterPays(options: EnableRequesterPaysOptions): Promise; /** * @typedef {array} EnableRequesterPaysResponse * @property {object} 0 The full API response. @@ -2366,7 +2375,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 @@ -2394,21 +2404,33 @@ class Bucket extends ServiceObject { * Example of enabling requester pays: */ enableRequesterPays( - callback?: EnableRequesterPaysCallback + optionsOrCallback?: EnableRequesterPaysCallback | EnableRequesterPaysOptions ): Promise | void { - const cb = callback || util.noop; + + let cb: EnableRequesterPaysCallback = util.noop; + let options: EnableRequesterPaysOptions = {}; + if (typeof optionsOrCallback === 'function') { + cb = optionsOrCallback; + } else if (optionsOrCallback) { + options = optionsOrCallback; + } + this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + AvailableServiceObjectMethods.setMetadata, + options.preconditionOpts ); + this.setMetadata({ billing: { requesterPays: true, }, + options }) .then(resp => cb(null, ...resp)) .catch(cb) .finally(() => { + console.log("in the finally") this.storage.retryOptions.autoRetry = this.instanceRetryValue; }); } @@ -3405,6 +3427,7 @@ class Bucket extends ServiceObject { removeRetentionPeriod(): Promise; removeRetentionPeriod(callback: SetBucketMetadataCallback): void; + removeRetentionPeriod(options: SetBucketMetadataOptions): Promise /** * Remove an already-existing retention policy from this bucket, if it is not * locked. @@ -3428,16 +3451,23 @@ class Bucket extends ServiceObject { * ``` */ removeRetentionPeriod( - callback?: SetBucketMetadataCallback + optionsOrCallback?: SetBucketMetadataOptions | SetBucketMetadataCallback ): Promise | void { - const cb = callback || util.noop; + + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + const cb = + typeof optionsOrCallback === 'function' ? optionsOrCallback : util.noop; + this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + AvailableServiceObjectMethods.setMetadata, + options.preconditionOpts ); + this.setMetadata({ retentionPolicy: null, - }) + }, options.preconditionOpts) .then(resp => cb(null, ...resp)) .catch(cb) .finally(() => { diff --git a/src/nodejs-common/service-object.ts b/src/nodejs-common/service-object.ts index 1ba6d3a47..ab8e45a58 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; } @@ -282,6 +288,8 @@ class ServiceObject extends EventEmitter { const ignoreNotFound = options.ignoreNotFound!; delete options.ignoreNotFound; + + const methodConfig = (typeof this.methods.delete === 'object' && this.methods.delete) || {}; From 98796723b87c4d20a67a1991f7fd1cc7c7a316b9 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Tue, 26 Jul 2022 18:58:04 +0000 Subject: [PATCH 20/58] minor progress --- conformance-test/libraryMethods.ts | 11 +++++------ src/bucket.ts | 24 +++++++++++------------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 09e898cf3..4a625de8b 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -105,11 +105,10 @@ export async function deleteFiles(bucket: Bucket) { } export async function deleteLabelsInstancePrecondition(bucket: Bucket) { - await bucket.deleteLabels(); + bucket.deleteLabels(() => {}); } export async function deleteLabels(bucket: Bucket) { - //TODO: not retrying for some reason await bucket.deleteLabels({ preconditionOpts: { ifMetagenerationMatch: 2 @@ -123,7 +122,7 @@ export async function disableRequesterPaysInstancePrecondition(bucket: Bucket) { export async function disableRequesterPays(bucket: Bucket) { //TODO: not retrying for some reason - await bucket.disableRequesterPays(() => {}, { + bucket.disableRequesterPays(() => {}, { preconditionOpts: { ifMetagenerationMatch: 2, }, @@ -152,7 +151,7 @@ export async function enableRequesterPaysInstancePrecondition(bucket: Bucket) { } export async function enableRequesterPays(bucket: Bucket) { - // TODO -- the promise isnt resolving and test is timing out + //TODO -- swallowing options into function[anonymous] await bucket.enableRequesterPays({ preconditionOpts: { ifMetagenerationMatch: 2 @@ -214,8 +213,8 @@ export async function removeRetentionPeriodInstancePrecondition( } export async function removeRetentionPeriod(bucket: Bucket) { - //TODO -- the promise isnt resolving and test is timing out - await bucket.removeRetentionPeriod({ + //TODO -- not retrying + bucket.removeRetentionPeriod(() => {}, { preconditionOpts: { ifMetagenerationMatch: 2 } diff --git a/src/bucket.ts b/src/bucket.ts index a286d958c..b6a25e804 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -2162,7 +2162,7 @@ class Bucket extends ServiceObject { disableRequesterPays( callback: DisableRequesterPaysCallback, options: DisableRequesterPaysOptions - ): Promise | void; + ): void; /** * @typedef {array} DisableRequesterPaysResponse * @property {object} 0 The full API response. @@ -2421,16 +2421,16 @@ class Bucket extends ServiceObject { options.preconditionOpts ); + const setMetadataOptions = options.preconditionOpts; this.setMetadata({ billing: { requesterPays: true, }, - options + setMetadataOptions }) .then(resp => cb(null, ...resp)) .catch(cb) .finally(() => { - console.log("in the finally") this.storage.retryOptions.autoRetry = this.instanceRetryValue; }); } @@ -3427,7 +3427,7 @@ class Bucket extends ServiceObject { removeRetentionPeriod(): Promise; removeRetentionPeriod(callback: SetBucketMetadataCallback): void; - removeRetentionPeriod(options: SetBucketMetadataOptions): Promise + removeRetentionPeriod(callback: SetBucketMetadataCallback, options: SetBucketMetadataOptions): void; /** * Remove an already-existing retention policy from this bucket, if it is not * locked. @@ -3451,23 +3451,21 @@ class Bucket extends ServiceObject { * ``` */ removeRetentionPeriod( - optionsOrCallback?: SetBucketMetadataOptions | SetBucketMetadataCallback + callback?: SetBucketMetadataCallback, + options?: SetBucketMetadataOptions ): Promise | void { - - const options = - typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; - const cb = - typeof optionsOrCallback === 'function' ? optionsOrCallback : util.noop; + + const cb = callback || util.noop; this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, AvailableServiceObjectMethods.setMetadata, - options.preconditionOpts + options?.preconditionOpts ); - + this.setMetadata({ retentionPolicy: null, - }, options.preconditionOpts) + }, options?.preconditionOpts) .then(resp => cb(null, ...resp)) .catch(cb) .finally(() => { From 3ac678c7202ea7d6011fba472c37180056ffde74 Mon Sep 17 00:00:00 2001 From: Denis DelGrosso Date: Wed, 27 Jul 2022 15:30:52 +0000 Subject: [PATCH 21/58] fix(refactor): Simplify logic around disabling autoretry for setmetadata --- src/bucket.ts | 228 ++++++++++++++++++++++--------------------------- src/file.ts | 52 ++++++++--- test/bucket.ts | 197 +++++++++++++++--------------------------- test/file.ts | 21 ++--- 4 files changed, 222 insertions(+), 276 deletions(-) diff --git a/src/bucket.ts b/src/bucket.ts index 8512652c7..3ce3211d4 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; @@ -1316,7 +1319,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 +1362,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 +1383,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! + ); }); } @@ -2167,21 +2163,16 @@ class Bucket extends ServiceObject { callback?: DisableRequesterPaysCallback ): Promise | void { const cb = callback || util.noop; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); - this.setMetadata({ - billing: { - requesterPays: false, + this.setMetadata( + { + billing: { + requesterPays: false, + }, }, - }) - .then(resp => cb(null, ...resp)) - .catch(cb) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + {}, + cb! + ); } enableLogging( @@ -2266,8 +2257,6 @@ class Bucket extends ServiceObject { : this.id; (async () => { - let setMetadataResponse; - try { const [policy] = await this.iam.getPolicy(); policy.bindings.push({ @@ -2275,24 +2264,20 @@ 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, + }, }, - }); + {}, + callback! + ); } catch (e) { callback!(e as Error); return; - } finally { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; } - - callback!(null, setMetadataResponse); })(); } @@ -2350,20 +2335,15 @@ class Bucket extends ServiceObject { callback?: EnableRequesterPaysCallback ): Promise | void { const cb = callback || util.noop; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); - this.setMetadata({ - billing: { - requesterPays: true, + this.setMetadata( + { + billing: { + requesterPays: true, + }, }, - }) - .then(resp => cb(null, ...resp)) - .catch(cb) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + {}, + cb + ); } /** @@ -3154,29 +3134,26 @@ class Bucket extends ServiceObject { query.userProject = options.userProject; } - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); - // 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( @@ -3364,18 +3341,13 @@ class Bucket extends ServiceObject { callback?: SetBucketMetadataCallback ): Promise | void { const cb = callback || util.noop; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + this.setMetadata( + { + retentionPolicy: null, + }, + {}, + cb ); - 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,13 +3448,39 @@ 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 { this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, AvailableServiceObjectMethods.setMetadata ); - this.setMetadata({labels}, options) - .then(resp => callback!(null, ...resp)) - .catch(callback!) + 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; }); @@ -3535,20 +3533,15 @@ class Bucket extends ServiceObject { callback?: SetBucketMetadataCallback ): Promise | void { const cb = callback || util.noop; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); - this.setMetadata({ - retentionPolicy: { - retentionPeriod: duration, + this.setMetadata( + { + retentionPolicy: { + retentionPeriod: duration, + }, }, - }) - .then(resp => cb(null, ...resp)) - .catch(cb) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); + {}, + cb + ); } setCorsConfiguration( @@ -3608,19 +3601,13 @@ class Bucket extends ServiceObject { callback?: SetBucketMetadataCallback ): Promise | void { const cb = callback || util.noop; - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + this.setMetadata( + { + cors: corsConfiguration, + }, + {}, + cb ); - - this.setMetadata({ - cors: corsConfiguration, - }) - .then(resp => cb(null, ...resp)) - .catch(cb) - .finally(() => { - this.storage.retryOptions.autoRetry = this.instanceRetryValue; - }); } setStorageClass( @@ -3693,10 +3680,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 +3688,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!); } /** diff --git a/src/file.ts b/src/file.ts index 6c7fc5cf0..dedce43ed 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,7 @@ import {HashStreamValidator} from './hash-stream-validator'; import {URL} from 'url'; import retry = require('async-retry'); +import {SetMetadataOptions} from './nodejs-common/service-object'; export type GetExpirationDateResponse = [Date]; export interface GetExpirationDateCallback { @@ -3069,22 +3072,12 @@ class File extends ServiceObject { 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; @@ -3660,6 +3653,43 @@ 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 { + this.disableAutoRetryConditionallyIdempotent_( + this.methods.setMetadata, + AvailableServiceObjectMethods.setMetadata + ); + + 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; + }); + } + setStorageClass( storageClass: string, options?: SetStorageClassOptions diff --git a/test/bucket.ts b/test/bucket.ts index 44243cb14..71658f656 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', () => { @@ -1427,21 +1410,29 @@ 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) => { - assert.strictEqual(callback, undefined); + bucket.setMetadata = ( + metadata: {}, + optionsOrCallback: {}, + callback: Function + ) => { + assert.strictEqual(callback, util.noop); done(); }; @@ -1450,7 +1441,9 @@ describe('Bucket', () => { it('should set autoRetry to false when ifMetagenerationMatch is undefined', done => { bucket.setMetadata = () => { - assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); + Promise.resolve().then(() => { + assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); + }); done(); }; bucket.disableRequesterPays(); @@ -1595,7 +1588,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 +1620,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(util.noop, callback); done(); }; @@ -2137,13 +2127,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 +2150,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 +2180,7 @@ describe('Bucket', () => { }; bucket.setMetadata = (metadata: {}, options_: SetFileMetadataOptions) => { assert.strictEqual(options_.userProject, options.userProject); - return Promise.resolve(); + done(); }; bucket.makePrivate(options, done); }); @@ -2221,14 +2215,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 +2309,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 +2400,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 +2420,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 +2448,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 +2495,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 +2653,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..9714f247f 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', () => { From ed5e4079b0b82d207ce54d8364dc39498175bf98 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 27 Jul 2022 15:35:01 +0000 Subject: [PATCH 22/58] setcorsconfiguration --- conformance-test/libraryMethods.ts | 8 ++++++-- src/bucket.ts | 14 +++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 4a625de8b..4c0c2b791 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -227,9 +227,13 @@ export async function setCorsConfigurationInstancePrecondition(bucket: Bucket) { } export async function setCorsConfiguration(bucket: Bucket) { - //TODO + //TODO -- not retrying const corsConfiguration = [{maxAgeSeconds: 3600}]; // 1 hour - await bucket.setCorsConfiguration(corsConfiguration); + bucket.setCorsConfiguration(corsConfiguration, () => {}, { + preconditionOpts: { + ifMetagenerationMatch: 2 + } + }); } export async function setLabelsInstancePrecondition(bucket: Bucket) { diff --git a/src/bucket.ts b/src/bucket.ts index b6a25e804..5d3af0784 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -3654,6 +3654,11 @@ class Bucket extends ServiceObject { corsConfiguration: Cors[], callback: SetBucketMetadataCallback ): void; + setCorsConfiguration( + corsConfiguration: Cors[], + callback: SetBucketMetadataCallback, + options: SetBucketMetadataOptions + ): void; /** * * @typedef {object} Cors @@ -3681,6 +3686,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 @@ -3701,17 +3707,19 @@ class Bucket extends ServiceObject { */ setCorsConfiguration( corsConfiguration: Cors[], - callback?: SetBucketMetadataCallback + callback?: SetBucketMetadataCallback, + options?: SetBucketMetadataOptions ): Promise | void { const cb = callback || util.noop; this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata + AvailableServiceObjectMethods.setMetadata, + options?.preconditionOpts ); this.setMetadata({ cors: corsConfiguration, - }) + }, options?.preconditionOpts) .then(resp => cb(null, ...resp)) .catch(cb) .finally(() => { From 5b26bbc090042cc1954ca6750a76d9d99f4099f6 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 27 Jul 2022 17:38:54 +0000 Subject: [PATCH 23/58] set retention period --- conformance-test/libraryMethods.ts | 2 +- src/bucket.ts | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 4c0c2b791..1ed11e6cf 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -281,7 +281,7 @@ export async function setRetentionPeriodInstancePrecondition(bucket: Bucket) { } export async function setRetentionPeriod(bucket: Bucket) { - //TODO + //TODO - not retrying when there's a callback const DURATION_SECONDS = 15780000; // 6 months. await bucket.setRetentionPeriod(DURATION_SECONDS); } diff --git a/src/bucket.ts b/src/bucket.ts index 5c44b6e45..979b5483e 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -3563,6 +3563,11 @@ class Bucket extends ServiceObject { duration: number, callback: SetBucketMetadataCallback ): void; + setRetentionPeriod( + duration: number, + callback: SetBucketMetadataCallback, + options: SetBucketMetadataOptions + ): void; /** * Lock all objects contained in the bucket, based on their creation time. Any * attempt to overwrite or delete objects younger than the retention period @@ -3578,6 +3583,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 @@ -3602,7 +3608,8 @@ class Bucket extends ServiceObject { */ setRetentionPeriod( duration: number, - callback?: SetBucketMetadataCallback + callback?: SetBucketMetadataCallback, + options?: SetBucketMetadataOptions ): Promise | void { const cb = callback || util.noop; this.setMetadata( @@ -3611,7 +3618,7 @@ class Bucket extends ServiceObject { retentionPeriod: duration, }, }, - {}, + options?.preconditionOpts as SetBucketMetadataOptions, cb ); } From 0e8ebcdb1300823409d58bd3ab755d209f6ac385 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 27 Jul 2022 17:41:58 +0000 Subject: [PATCH 24/58] bucketSetStorageClass --- conformance-test/libraryMethods.ts | 7 +++++-- src/bucket.ts | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 1ed11e6cf..99ea51c6e 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -293,8 +293,11 @@ export async function bucketSetStorageClassInstancePrecondition( } export async function bucketSetStorageClass(bucket: Bucket) { - //TODO - await bucket.setStorageClass('nearline'); + await bucket.setStorageClass('nearline', { + preconditionOpts: { + ifMetagenerationMatch: 2 + } + }); } export async function bucketUploadResumableInstancePrecondition( diff --git a/src/bucket.ts b/src/bucket.ts index 979b5483e..920630c48 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -365,6 +365,7 @@ export interface SetLabelsCallback { export interface SetBucketStorageClassOptions { userProject?: string; + preconditionOpts?: PreconditionOptions } export interface SetBucketStorageClassCallback { @@ -3774,7 +3775,7 @@ class Bucket extends ServiceObject { }) .toUpperCase(); - this.setMetadata({storageClass}, options, callback!); + this.setMetadata({storageClass}, options.preconditionOpts as SetMetadataOptions, callback!); } /** From ce06b2689239c02bdc8dd51bf1c3bb182fac6577 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 27 Jul 2022 19:10:06 +0000 Subject: [PATCH 25/58] fileMakePrivate --- conformance-test/libraryMethods.ts | 7 +++++-- src/file.ts | 21 +++++++++++++++------ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 99ea51c6e..5ccfefee1 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -432,8 +432,11 @@ export async function fileMakePrivateInstancePrecondition( } export async function fileMakePrivate(_bucket: Bucket, file: File) { - //TODO - await file.makePrivate(); + await file.makePrivate({ + preconditionOpts: { + ifGenerationMatch: file.metadata.generation + } + }); } export async function fileMakePublic(_bucket: Bucket, file: File) { diff --git a/src/file.ts b/src/file.ts index 76578cced..f1a9e9382 100644 --- a/src/file.ts +++ b/src/file.ts @@ -225,6 +225,7 @@ export interface MakeFilePrivateOptions { metadata?: Metadata; strict?: boolean; userProject?: string; + preconditionOpts?: PreconditionOptions } export type MakeFilePrivateResponse = [Metadata]; @@ -3083,6 +3084,11 @@ 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; } @@ -3688,11 +3694,6 @@ class File extends ServiceObject { optionsOrCallback: SetMetadataOptions | MetadataCallback, cb?: MetadataCallback ): Promise | void { - this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata - ); - const options = typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; cb = @@ -3700,6 +3701,12 @@ class File extends ServiceObject { ? (optionsOrCallback as MetadataCallback) : cb; + this.disableAutoRetryConditionallyIdempotent_( + this.methods.setMetadata, + AvailableServiceObjectMethods.setMetadata, + options + ); + super .setMetadata(metadata, options) .then(resp => cb!(null, ...resp)) @@ -3973,11 +3980,13 @@ 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 && + localPreconditionOptions?.ifGenerationMatch === undefined && methodType === AvailableServiceObjectMethods.setMetadata && this.storage.retryOptions.idempotencyStrategy === IdempotencyStrategy.RetryConditional) || From 05ee7ee5c1683b50d22c72d49176a5d8f45a9359 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 27 Jul 2022 19:12:06 +0000 Subject: [PATCH 26/58] file set metadata --- conformance-test/libraryMethods.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 5ccfefee1..fb4fd32e6 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -518,7 +518,9 @@ export async function setMetadata(_bucket: Bucket, file: File) { properties: 'go here', }, }; - await file.setMetadata(metadata); + await file.setMetadata(metadata, { + ifGenerationMatch: file.metadata.generation + }); } export async function setStorageClass(_bucket: Bucket, file: File) { From f3a348bf0354d2aff26525d5e77fdcb6e2282b0b Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 27 Jul 2022 19:17:29 +0000 Subject: [PATCH 27/58] bucket set metadata --- conformance-test/libraryMethods.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index fb4fd32e6..9a5b4fd5e 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -98,7 +98,7 @@ export async function deleteFilesInstancePrecondition(bucket: Bucket) { } export async function deleteFiles(bucket: Bucket) { - //TODO + //TODO -- how to pass preconditions on the file? await bucket.deleteFiles({ ifMetagenerationMatch: 2 }); @@ -272,7 +272,9 @@ export async function bucketSetMetadata(bucket: Bucket) { notFoundPage: 'http://example.com/404.html', }, }; - await bucket.setMetadata(metadata); + await bucket.setMetadata(metadata, { + ifMetagenerationMatch: 2 + }); } export async function setRetentionPeriodInstancePrecondition(bucket: Bucket) { From 44aaba64dc0eba5410c4ebc86e56bc07e8331b81 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 27 Jul 2022 19:20:49 +0000 Subject: [PATCH 28/58] file delete --- conformance-test/libraryMethods.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 9a5b4fd5e..4ecb25c8b 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -265,7 +265,6 @@ export async function bucketSetMetadataInstancePrecondition(bucket: Bucket) { } export async function bucketSetMetadata(bucket: Bucket) { - //TODO const metadata = { website: { mainPageSuffix: 'http://example.com', @@ -399,7 +398,9 @@ export async function fileDeleteInstancePrecondition( export async function fileDelete(_bucket: Bucket, file: File) { //TODO - await file.delete(); + await file.delete({ + ifGenerationMatch: file.metadata.generation + }); } export async function download(_bucket: Bucket, file: File) { From 982b9f91ab858f160fb6a6b2cb003e1276f3d4f2 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 27 Jul 2022 19:22:57 +0000 Subject: [PATCH 29/58] more precondition updates --- conformance-test/libraryMethods.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 4ecb25c8b..b9b1420ba 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -397,7 +397,6 @@ export async function fileDeleteInstancePrecondition( } export async function fileDelete(_bucket: Bucket, file: File) { - //TODO await file.delete({ ifGenerationMatch: file.metadata.generation }); @@ -513,7 +512,6 @@ export async function setMetadataInstancePrecondition( } export async function setMetadata(_bucket: Bucket, file: File) { - //TODO const metadata = { contentType: 'application/x-font-ttf', metadata: { From e48a46732ae5ab1e4c3734da8632a5592ac873ae Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 27 Jul 2022 21:27:13 +0000 Subject: [PATCH 30/58] precondition refactor --- conformance-test/libraryMethods.ts | 34 ++------ src/bucket.ts | 131 ++++++++++++++++------------- test/bucket.ts | 4 +- 3 files changed, 80 insertions(+), 89 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index b9b1420ba..ede586ea7 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -30,7 +30,6 @@ export async function addLifecycleRuleInstancePrecondition(bucket: Bucket) { } export async function addLifecycleRule(bucket: Bucket) { - //TODO -- swallowing options into function[anonymous] await bucket.addLifecycleRule( { action: 'delete', @@ -39,9 +38,7 @@ export async function addLifecycleRule(bucket: Bucket) { }, }, { - preconditionOpts: { ifMetagenerationMatch: 2, - }, } ); } @@ -110,9 +107,7 @@ export async function deleteLabelsInstancePrecondition(bucket: Bucket) { export async function deleteLabels(bucket: Bucket) { await bucket.deleteLabels({ - preconditionOpts: { ifMetagenerationMatch: 2 - } }); } @@ -121,11 +116,8 @@ export async function disableRequesterPaysInstancePrecondition(bucket: Bucket) { } export async function disableRequesterPays(bucket: Bucket) { - //TODO: not retrying for some reason - bucket.disableRequesterPays(() => {}, { - preconditionOpts: { + await bucket.disableRequesterPays({ ifMetagenerationMatch: 2, - }, }); } @@ -139,9 +131,7 @@ export async function enableLoggingInstancePrecondition(bucket: Bucket) { export async function enableLogging(bucket: Bucket) { const config = { prefix: 'log', - preconditionOpts: { ifMetagenerationMatch: 2, - }, }; await bucket.enableLogging(config); } @@ -151,11 +141,8 @@ export async function enableRequesterPaysInstancePrecondition(bucket: Bucket) { } export async function enableRequesterPays(bucket: Bucket) { - //TODO -- swallowing options into function[anonymous] await bucket.enableRequesterPays({ - preconditionOpts: { ifMetagenerationMatch: 2 - } }); } @@ -213,11 +200,8 @@ export async function removeRetentionPeriodInstancePrecondition( } export async function removeRetentionPeriod(bucket: Bucket) { - //TODO -- not retrying - bucket.removeRetentionPeriod(() => {}, { - preconditionOpts: { + await bucket.removeRetentionPeriod({ ifMetagenerationMatch: 2 - } }); } @@ -227,12 +211,9 @@ export async function setCorsConfigurationInstancePrecondition(bucket: Bucket) { } export async function setCorsConfiguration(bucket: Bucket) { - //TODO -- not retrying const corsConfiguration = [{maxAgeSeconds: 3600}]; // 1 hour - bucket.setCorsConfiguration(corsConfiguration, () => {}, { - preconditionOpts: { + await bucket.setCorsConfiguration(corsConfiguration, { ifMetagenerationMatch: 2 - } }); } @@ -282,9 +263,10 @@ export async function setRetentionPeriodInstancePrecondition(bucket: Bucket) { } export async function setRetentionPeriod(bucket: Bucket) { - //TODO - not retrying when there's a callback const DURATION_SECONDS = 15780000; // 6 months. - await bucket.setRetentionPeriod(DURATION_SECONDS); + await bucket.setRetentionPeriod(DURATION_SECONDS, { + ifMetagenerationMatch: 2 + }); } export async function bucketSetStorageClassInstancePrecondition( @@ -295,10 +277,8 @@ export async function bucketSetStorageClassInstancePrecondition( export async function bucketSetStorageClass(bucket: Bucket) { await bucket.setStorageClass('nearline', { - preconditionOpts: { ifMetagenerationMatch: 2 - } - }); + }); } export async function bucketUploadResumableInstancePrecondition( diff --git a/src/bucket.ts b/src/bucket.ts index 920630c48..3a0000de4 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -107,9 +107,8 @@ interface WatchAllOptions { versions?: boolean; } -export interface AddLifecycleRuleOptions { +export interface AddLifecycleRuleOptions extends PreconditionOptions { append?: boolean; - preconditionOpts?: PreconditionOptions; } export interface LifecycleRule { @@ -118,10 +117,9 @@ export interface LifecycleRule { storageClass?: string; } -export interface EnableLoggingOptions { +export interface EnableLoggingOptions extends PreconditionOptions { bucket?: string | Bucket; prefix: string; - preconditionOpts?: PreconditionOptions; } export interface GetFilesOptions { @@ -205,13 +203,10 @@ export interface DeleteFilesCallback { export type DeleteLabelsResponse = [Metadata]; export type DeleteLabelsCallback = SetLabelsCallback; -export interface DeleteLabelsOptions { - preconditionOpts?: PreconditionOptions -} -export interface DisableRequesterPaysOptions { - preconditionOpts?: PreconditionOptions; -} +export type DeleteLabelsOptions = PreconditionOptions; + +export type DisableRequesterPaysOptions = PreconditionOptions; export type DisableRequesterPaysResponse = [Metadata]; @@ -225,9 +220,7 @@ export interface EnableRequesterPaysCallback { (err?: Error | null, apiResponse?: Metadata): void; } -export interface EnableRequesterPaysOptions { - preconditionOpts?: PreconditionOptions; -} +export type EnableRequesterPaysOptions = PreconditionOptions; export interface BucketExistsOptions extends GetConfig { userProject?: string; } @@ -332,9 +325,8 @@ export interface MakeBucketPublicCallback { export type MakeBucketPublicResponse = [File[]]; -export interface SetBucketMetadataOptions { +export interface SetBucketMetadataOptions extends PreconditionOptions { userProject?: string; - preconditionOpts?: PreconditionOptions } export type SetBucketMetadataResponse = [Metadata]; @@ -363,9 +355,8 @@ export interface SetLabelsCallback { (err?: Error | null, metadata?: Metadata): void; } -export interface SetBucketStorageClassOptions { +export interface SetBucketStorageClassOptions extends PreconditionOptions { userProject?: string; - preconditionOpts?: PreconditionOptions } export interface SetBucketStorageClassCallback { @@ -1384,7 +1375,7 @@ class Bucket extends ServiceObject { if (options.append === false) { this.setMetadata( {lifecycle: {rule: newLifecycleRules}}, - options?.preconditionOpts as SetMetadataOptions, + options, callback! ); return; @@ -2125,8 +2116,8 @@ class Bucket extends ServiceObject { return nullLabelMap; }, {}); - if (options?.preconditionOpts?.ifMetagenerationMatch !== undefined) { - this.setLabels(nullLabelMap, options.preconditionOpts, callback!); + if (options?.ifMetagenerationMatch !== undefined) { + this.setLabels(nullLabelMap, options, callback!); } else { this.setLabels(nullLabelMap, callback!); @@ -2146,11 +2137,11 @@ class Bucket extends ServiceObject { } } - disableRequesterPays(): Promise; + disableRequesterPays(options?: DisableRequesterPaysOptions): Promise; disableRequesterPays(callback: DisableRequesterPaysCallback): void; disableRequesterPays( - callback: DisableRequesterPaysCallback, - options: DisableRequesterPaysOptions + options: DisableRequesterPaysOptions, + callback: DisableRequesterPaysCallback ): void; /** * @typedef {array} DisableRequesterPaysResponse @@ -2199,10 +2190,16 @@ class Bucket extends ServiceObject { * Example of disabling requester pays: */ disableRequesterPays( - callback?: DisableRequesterPaysCallback, - options?: DisableRequesterPaysOptions + optionsOrCallback?: DisableRequesterPaysOptions | DisableRequesterPaysCallback, + callback?: DisableRequesterPaysCallback ): Promise | void { - const cb = callback || util.noop; + + let options: DisableRequesterPaysOptions = {} + if (typeof optionsOrCallback === 'function') { + callback = optionsOrCallback; + } else if (optionsOrCallback) { + options = optionsOrCallback; + } this.setMetadata( { @@ -2210,8 +2207,8 @@ class Bucket extends ServiceObject { requesterPays: false, }, }, - options?.preconditionOpts as SetMetadataOptions, - cb! + options, + callback! ); } @@ -2311,7 +2308,7 @@ class Bucket extends ServiceObject { logObjectPrefix: config.prefix, }, }, - config.preconditionOpts as SetMetadataOptions, + config, callback! ); } catch (e) { @@ -2321,9 +2318,10 @@ class Bucket extends ServiceObject { })(); } - enableRequesterPays(): Promise; + enableRequesterPays(options?: EnableRequesterPaysOptions): Promise; enableRequesterPays(callback: EnableRequesterPaysCallback): void; - enableRequesterPays(options: EnableRequesterPaysOptions): Promise; + enableRequesterPays(options: EnableRequesterPaysOptions, callback: EnableRequesterPaysCallback): void; + /** * @typedef {array} EnableRequesterPaysResponse * @property {object} 0 The full API response. @@ -2374,10 +2372,10 @@ class Bucket extends ServiceObject { * Example of enabling requester pays: */ enableRequesterPays( - optionsOrCallback?: EnableRequesterPaysCallback | EnableRequesterPaysOptions + optionsOrCallback?: EnableRequesterPaysCallback | EnableRequesterPaysOptions, + cb?: EnableRequesterPaysCallback ): Promise | void { - let cb: EnableRequesterPaysCallback = util.noop; let options: EnableRequesterPaysOptions = {}; if (typeof optionsOrCallback === 'function') { cb = optionsOrCallback; @@ -2391,8 +2389,8 @@ class Bucket extends ServiceObject { requesterPays: true, }, }, - options.preconditionOpts as SetMetadataOptions, - cb + options, + cb! ); } @@ -3382,9 +3380,9 @@ class Bucket extends ServiceObject { return new Notification(this, id); } - removeRetentionPeriod(): Promise; + removeRetentionPeriod(options?: SetBucketMetadataOptions): Promise; removeRetentionPeriod(callback: SetBucketMetadataCallback): void; - removeRetentionPeriod(callback: SetBucketMetadataCallback, options: SetBucketMetadataOptions): void; + removeRetentionPeriod(options: SetBucketMetadataOptions, callback: SetBucketMetadataCallback): void; /** * Remove an already-existing retention policy from this bucket, if it is not * locked. @@ -3408,17 +3406,20 @@ class Bucket extends ServiceObject { * ``` */ removeRetentionPeriod( - callback?: SetBucketMetadataCallback, - options?: SetBucketMetadataOptions + optionsOrCallback?: SetBucketMetadataOptions | SetBucketMetadataCallback, + callback?: SetBucketMetadataCallback ): Promise | void { - - const cb = callback || util.noop; + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + callback = + typeof optionsOrCallback === 'function' ? optionsOrCallback : callback; + this.setMetadata( { retentionPolicy: null, }, - options?.preconditionOpts as SetMetadataOptions, - cb + options, + callback! ); } @@ -3550,6 +3551,8 @@ class Bucket extends ServiceObject { AvailableServiceObjectMethods.setMetadata, options ); + console.log(options); + super .setMetadata(metadata, options) .then(resp => cb!(null, ...resp)) @@ -3559,15 +3562,16 @@ class Bucket extends ServiceObject { }); } - setRetentionPeriod(duration: number): Promise; + setRetentionPeriod(duration: number, + options?: SetBucketMetadataOptions): Promise; setRetentionPeriod( duration: number, callback: SetBucketMetadataCallback ): void; setRetentionPeriod( duration: number, - callback: SetBucketMetadataCallback, - options: SetBucketMetadataOptions + options: SetBucketMetadataOptions, + callback: SetBucketMetadataCallback ): void; /** * Lock all objects contained in the bucket, based on their creation time. Any @@ -3609,23 +3613,27 @@ class Bucket extends ServiceObject { */ setRetentionPeriod( duration: number, - callback?: SetBucketMetadataCallback, - options?: SetBucketMetadataOptions + optionsOrCallback?: SetBucketMetadataOptions | SetBucketMetadataCallback, + callback?: SetBucketMetadataCallback ): Promise | void { - const cb = callback || util.noop; + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + callback = + typeof optionsOrCallback === 'function' ? optionsOrCallback : callback; this.setMetadata( { retentionPolicy: { retentionPeriod: duration, }, }, - options?.preconditionOpts as SetBucketMetadataOptions, - cb + options, + callback! ); } setCorsConfiguration( - corsConfiguration: Cors[] + corsConfiguration: Cors[], + options?: SetBucketMetadataOptions ): Promise; setCorsConfiguration( corsConfiguration: Cors[], @@ -3633,8 +3641,8 @@ class Bucket extends ServiceObject { ): void; setCorsConfiguration( corsConfiguration: Cors[], - callback: SetBucketMetadataCallback, - options: SetBucketMetadataOptions + options: SetBucketMetadataOptions, + callback: SetBucketMetadataCallback ): void; /** * @@ -3684,16 +3692,19 @@ class Bucket extends ServiceObject { */ setCorsConfiguration( corsConfiguration: Cors[], - callback?: SetBucketMetadataCallback, - options?: SetBucketMetadataOptions + optionsOrCallback?: SetBucketMetadataOptions | SetBucketMetadataCallback, + callback?: SetBucketMetadataCallback ): Promise | void { - const cb = callback || util.noop; + const options = + typeof optionsOrCallback === 'object' ? optionsOrCallback : {}; + callback = + typeof optionsOrCallback === 'function' ? optionsOrCallback : callback; this.setMetadata( { cors: corsConfiguration, }, - options?.preconditionOpts as SetMetadataOptions, - cb + options, + callback! ); } @@ -3775,7 +3786,7 @@ class Bucket extends ServiceObject { }) .toUpperCase(); - this.setMetadata({storageClass}, options.preconditionOpts as SetMetadataOptions, callback!); + this.setMetadata({storageClass}, options, callback!); } /** diff --git a/test/bucket.ts b/test/bucket.ts index 877c81eb4..ce65bed8d 100644 --- a/test/bucket.ts +++ b/test/bucket.ts @@ -1396,7 +1396,7 @@ describe('Bucket', () => { optionsOrCallback: {}, callback: Function ) => { - assert.strictEqual(callback, util.noop); + assert.strictEqual(callback, undefined); done(); }; @@ -1610,7 +1610,7 @@ describe('Bucket', () => { optionsOrCallback: {}, callback: Function ) => { - assert.equal(util.noop, callback); + assert.equal(callback, undefined); done(); }; From 8455c02b05fdd94a4c0cf1d330e25894877c3206 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Wed, 27 Jul 2022 21:28:11 +0000 Subject: [PATCH 31/58] removed log statement --- src/bucket.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/bucket.ts b/src/bucket.ts index 3a0000de4..828c3cb36 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -3551,8 +3551,6 @@ class Bucket extends ServiceObject { AvailableServiceObjectMethods.setMetadata, options ); - console.log(options); - super .setMetadata(metadata, options) .then(resp => cb!(null, ...resp)) From a326cd2450d5e4ca1b4d51bad46c6c97a1a11803 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Thu, 28 Jul 2022 15:47:12 +0000 Subject: [PATCH 32/58] change delete labels signature --- src/bucket.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bucket.ts b/src/bucket.ts index 828c3cb36..dbf311251 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -2038,6 +2038,7 @@ class Bucket extends ServiceObject { 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. @@ -2091,10 +2092,10 @@ class Bucket extends ServiceObject { deleteLabels( labelsOrCallbackOrOptions?: string | string[] | DeleteLabelsCallback | DeleteLabelsOptions, optionsOrCallback?: DeleteLabelsCallback | DeleteLabelsOptions, + callback?: DeleteLabelsCallback ): Promise | void { let labels = new Array(); let options: DeleteLabelsOptions = {}; - let callback: DeleteLabelsCallback; if (typeof labelsOrCallbackOrOptions === 'function') { callback = labelsOrCallbackOrOptions; From 3b89bddac16012b100448539e83c6617aaa8a13e Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Thu, 28 Jul 2022 15:49:44 +0000 Subject: [PATCH 33/58] fix delete labels --- conformance-test/libraryMethods.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index ede586ea7..f4c52c339 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -102,7 +102,7 @@ export async function deleteFiles(bucket: Bucket) { } export async function deleteLabelsInstancePrecondition(bucket: Bucket) { - bucket.deleteLabels(() => {}); + await bucket.deleteLabels(); } export async function deleteLabels(bucket: Bucket) { From 21df169889caeeb82198b505de741c27f246a7b6 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Thu, 28 Jul 2022 15:55:57 +0000 Subject: [PATCH 34/58] fixed save multipart --- conformance-test/libraryMethods.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index f4c52c339..2b13fc338 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -472,7 +472,6 @@ export async function saveMultipart(_bucket: Bucket, file: File) { resumable: false, preconditionOpts: { ifGenerationMatch: file.metadata.generation, - ifMetagenerationMatch: file.metadata.metageneration, }, }); } From 95dcf78fc870184a2ab0adf973779e4896223217 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Thu, 28 Jul 2022 16:31:06 +0000 Subject: [PATCH 35/58] put docker code back --- conformance-test/globalHooks.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/conformance-test/globalHooks.ts b/conformance-test/globalHooks.ts index 2567d5349..0775b7457 100644 --- a/conformance-test/globalHooks.ts +++ b/conformance-test/globalHooks.ts @@ -25,17 +25,17 @@ const TIME_TO_WAIT_FOR_CONTAINER_READY = 10000; // eslint-disable-next-line @typescript-eslint/no-explicit-any export async function mochaGlobalSetup(this: any) { // Increase the timeout for this before block so that the docker images have time to download and run. - // this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; - // await getTestBenchDockerImage(); - // await runTestBenchDockerImage(); - // await new Promise(resolve => - // setTimeout(resolve, TIME_TO_WAIT_FOR_CONTAINER_READY) - // ); + this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; + await getTestBenchDockerImage(); + await runTestBenchDockerImage(); + await new Promise(resolve => + setTimeout(resolve, TIME_TO_WAIT_FOR_CONTAINER_READY) + ); } // eslint-disable-next-line @typescript-eslint/no-explicit-any export async function mochaGlobalTeardown(this: any) { // Increase the timeout for this block so that docker has time to stop the container. - // this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; - // await stopTestBenchDockerImage(); + this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; + await stopTestBenchDockerImage(); } From bb9c2d87302874b34da911522e5ae3edafa1806f Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Thu, 28 Jul 2022 16:39:34 +0000 Subject: [PATCH 36/58] linted files --- conformance-test/libraryMethods.ts | 34 ++++++------- src/bucket.ts | 78 ++++++++++++++++++++--------- src/file.ts | 8 +-- src/nodejs-common/service-object.ts | 4 +- test/file.ts | 8 ++- 5 files changed, 82 insertions(+), 50 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 2b13fc338..954c4f584 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -38,7 +38,7 @@ export async function addLifecycleRule(bucket: Bucket) { }, }, { - ifMetagenerationMatch: 2, + ifMetagenerationMatch: 2, } ); } @@ -97,7 +97,7 @@ export async function deleteFilesInstancePrecondition(bucket: Bucket) { export async function deleteFiles(bucket: Bucket) { //TODO -- how to pass preconditions on the file? await bucket.deleteFiles({ - ifMetagenerationMatch: 2 + ifMetagenerationMatch: 2, }); } @@ -107,7 +107,7 @@ export async function deleteLabelsInstancePrecondition(bucket: Bucket) { export async function deleteLabels(bucket: Bucket) { await bucket.deleteLabels({ - ifMetagenerationMatch: 2 + ifMetagenerationMatch: 2, }); } @@ -117,7 +117,7 @@ export async function disableRequesterPaysInstancePrecondition(bucket: Bucket) { export async function disableRequesterPays(bucket: Bucket) { await bucket.disableRequesterPays({ - ifMetagenerationMatch: 2, + ifMetagenerationMatch: 2, }); } @@ -131,7 +131,7 @@ export async function enableLoggingInstancePrecondition(bucket: Bucket) { export async function enableLogging(bucket: Bucket) { const config = { prefix: 'log', - ifMetagenerationMatch: 2, + ifMetagenerationMatch: 2, }; await bucket.enableLogging(config); } @@ -142,7 +142,7 @@ export async function enableRequesterPaysInstancePrecondition(bucket: Bucket) { export async function enableRequesterPays(bucket: Bucket) { await bucket.enableRequesterPays({ - ifMetagenerationMatch: 2 + ifMetagenerationMatch: 2, }); } @@ -201,7 +201,7 @@ export async function removeRetentionPeriodInstancePrecondition( export async function removeRetentionPeriod(bucket: Bucket) { await bucket.removeRetentionPeriod({ - ifMetagenerationMatch: 2 + ifMetagenerationMatch: 2, }); } @@ -213,7 +213,7 @@ export async function setCorsConfigurationInstancePrecondition(bucket: Bucket) { export async function setCorsConfiguration(bucket: Bucket) { const corsConfiguration = [{maxAgeSeconds: 3600}]; // 1 hour await bucket.setCorsConfiguration(corsConfiguration, { - ifMetagenerationMatch: 2 + ifMetagenerationMatch: 2, }); } @@ -231,7 +231,7 @@ export async function setLabels(bucket: Bucket) { labeltwo: 'labeltwovalue', }; await bucket.setLabels(labels, { - ifMetagenerationMatch: 2 + ifMetagenerationMatch: 2, }); } @@ -253,7 +253,7 @@ export async function bucketSetMetadata(bucket: Bucket) { }, }; await bucket.setMetadata(metadata, { - ifMetagenerationMatch: 2 + ifMetagenerationMatch: 2, }); } @@ -265,7 +265,7 @@ export async function setRetentionPeriodInstancePrecondition(bucket: Bucket) { export async function setRetentionPeriod(bucket: Bucket) { const DURATION_SECONDS = 15780000; // 6 months. await bucket.setRetentionPeriod(DURATION_SECONDS, { - ifMetagenerationMatch: 2 + ifMetagenerationMatch: 2, }); } @@ -277,8 +277,8 @@ export async function bucketSetStorageClassInstancePrecondition( export async function bucketSetStorageClass(bucket: Bucket) { await bucket.setStorageClass('nearline', { - ifMetagenerationMatch: 2 - }); + ifMetagenerationMatch: 2, + }); } export async function bucketUploadResumableInstancePrecondition( @@ -378,7 +378,7 @@ export async function fileDeleteInstancePrecondition( export async function fileDelete(_bucket: Bucket, file: File) { await file.delete({ - ifGenerationMatch: file.metadata.generation + ifGenerationMatch: file.metadata.generation, }); } @@ -416,8 +416,8 @@ export async function fileMakePrivateInstancePrecondition( export async function fileMakePrivate(_bucket: Bucket, file: File) { await file.makePrivate({ preconditionOpts: { - ifGenerationMatch: file.metadata.generation - } + ifGenerationMatch: file.metadata.generation, + }, }); } @@ -499,7 +499,7 @@ export async function setMetadata(_bucket: Bucket, file: File) { }, }; await file.setMetadata(metadata, { - ifGenerationMatch: file.metadata.generation + ifGenerationMatch: file.metadata.generation, }); } diff --git a/src/bucket.ts b/src/bucket.ts index dbf311251..a9044e027 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -71,7 +71,7 @@ import {SetMetadataOptions} from './nodejs-common/service-object'; interface SourceObject { name: string; - generation?: number; + generation?: number; } interface CreateNotificationQuery { @@ -345,7 +345,7 @@ export interface Labels { [key: string]: string; } -export interface SetLabelsOptions extends PreconditionOptions{ +export interface SetLabelsOptions extends PreconditionOptions { userProject?: string; } @@ -2034,11 +2034,18 @@ class Bucket extends ServiceObject { } deleteLabels(labels?: string | string[]): Promise; - deleteLabels(options: DeleteLabelsOptions ): Promise; + deleteLabels(options: DeleteLabelsOptions): Promise; deleteLabels(callback: DeleteLabelsCallback): void; - deleteLabels(labels: string | string[], options: DeleteLabelsOptions): Promise; + deleteLabels( + labels: string | string[], + options: DeleteLabelsOptions + ): Promise; deleteLabels(labels: string | string[], callback: DeleteLabelsCallback): void; - deleteLabels(labels: string | string[], options: DeleteLabelsOptions, callback: DeleteLabelsCallback): void; + deleteLabels( + labels: string | string[], + options: DeleteLabelsOptions, + callback: DeleteLabelsCallback + ): void; /** * @typedef {array} DeleteLabelsResponse * @property {object} 0 The full API response. @@ -2053,7 +2060,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 | DeleteLabelsOptions} [optionsOrCallback] + * @param {DeleteLabelsCallback | DeleteLabelsOptions} [optionsOrCallback] * Callback function or precondition options. * @returns {Promise} * @@ -2090,7 +2097,11 @@ class Bucket extends ServiceObject { * ``` */ deleteLabels( - labelsOrCallbackOrOptions?: string | string[] | DeleteLabelsCallback | DeleteLabelsOptions, + labelsOrCallbackOrOptions?: + | string + | string[] + | DeleteLabelsCallback + | DeleteLabelsOptions, optionsOrCallback?: DeleteLabelsCallback | DeleteLabelsOptions, callback?: DeleteLabelsCallback ): Promise | void { @@ -2099,7 +2110,10 @@ class Bucket extends ServiceObject { if (typeof labelsOrCallbackOrOptions === 'function') { callback = labelsOrCallbackOrOptions; - } else if (typeof labelsOrCallbackOrOptions === 'string' || Array.isArray(labelsOrCallbackOrOptions)){ + } else if ( + typeof labelsOrCallbackOrOptions === 'string' || + Array.isArray(labelsOrCallbackOrOptions) + ) { labels = arrify(labelsOrCallbackOrOptions); } else if (labelsOrCallbackOrOptions) { options = labelsOrCallbackOrOptions; @@ -2119,8 +2133,7 @@ class Bucket extends ServiceObject { if (options?.ifMetagenerationMatch !== undefined) { this.setLabels(nullLabelMap, options, callback!); - } - else { + } else { this.setLabels(nullLabelMap, callback!); } }; @@ -2138,7 +2151,9 @@ class Bucket extends ServiceObject { } } - disableRequesterPays(options?: DisableRequesterPaysOptions): Promise; + disableRequesterPays( + options?: DisableRequesterPaysOptions + ): Promise; disableRequesterPays(callback: DisableRequesterPaysCallback): void; disableRequesterPays( options: DisableRequesterPaysOptions, @@ -2191,11 +2206,12 @@ class Bucket extends ServiceObject { * Example of disabling requester pays: */ disableRequesterPays( - optionsOrCallback?: DisableRequesterPaysOptions | DisableRequesterPaysCallback, + optionsOrCallback?: + | DisableRequesterPaysOptions + | DisableRequesterPaysCallback, callback?: DisableRequesterPaysCallback ): Promise | void { - - let options: DisableRequesterPaysOptions = {} + let options: DisableRequesterPaysOptions = {}; if (typeof optionsOrCallback === 'function') { callback = optionsOrCallback; } else if (optionsOrCallback) { @@ -2319,9 +2335,14 @@ class Bucket extends ServiceObject { })(); } - enableRequesterPays(options?: EnableRequesterPaysOptions): Promise; + enableRequesterPays( + options?: EnableRequesterPaysOptions + ): Promise; enableRequesterPays(callback: EnableRequesterPaysCallback): void; - enableRequesterPays(options: EnableRequesterPaysOptions, callback: EnableRequesterPaysCallback): void; + enableRequesterPays( + options: EnableRequesterPaysOptions, + callback: EnableRequesterPaysCallback + ): void; /** * @typedef {array} EnableRequesterPaysResponse @@ -2344,7 +2365,7 @@ 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 | EnableRequesterPaysOptions} [optionsOrCallback] + * @param {EnableRequesterPaysCallback | EnableRequesterPaysOptions} [optionsOrCallback] * Callback function or precondition options. * @returns {Promise} * @@ -2373,10 +2394,11 @@ class Bucket extends ServiceObject { * Example of enabling requester pays: */ enableRequesterPays( - optionsOrCallback?: EnableRequesterPaysCallback | EnableRequesterPaysOptions, + optionsOrCallback?: + | EnableRequesterPaysCallback + | EnableRequesterPaysOptions, cb?: EnableRequesterPaysCallback ): Promise | void { - let options: EnableRequesterPaysOptions = {}; if (typeof optionsOrCallback === 'function') { cb = optionsOrCallback; @@ -3381,9 +3403,14 @@ class Bucket extends ServiceObject { return new Notification(this, id); } - removeRetentionPeriod(options?: SetBucketMetadataOptions): Promise; + removeRetentionPeriod( + options?: SetBucketMetadataOptions + ): Promise; removeRetentionPeriod(callback: SetBucketMetadataCallback): void; - removeRetentionPeriod(options: SetBucketMetadataOptions, callback: SetBucketMetadataCallback): void; + removeRetentionPeriod( + options: SetBucketMetadataOptions, + callback: SetBucketMetadataCallback + ): void; /** * Remove an already-existing retention policy from this bucket, if it is not * locked. @@ -3549,7 +3576,8 @@ class Bucket extends ServiceObject { this.disableAutoRetryConditionallyIdempotent_( this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata, options + AvailableServiceObjectMethods.setMetadata, + options ); super @@ -3561,8 +3589,10 @@ class Bucket extends ServiceObject { }); } - setRetentionPeriod(duration: number, - options?: SetBucketMetadataOptions): Promise; + setRetentionPeriod( + duration: number, + options?: SetBucketMetadataOptions + ): Promise; setRetentionPeriod( duration: number, callback: SetBucketMetadataCallback diff --git a/src/file.ts b/src/file.ts index f1a9e9382..c0ff10b40 100644 --- a/src/file.ts +++ b/src/file.ts @@ -225,7 +225,7 @@ export interface MakeFilePrivateOptions { metadata?: Metadata; strict?: boolean; userProject?: string; - preconditionOpts?: PreconditionOptions + preconditionOpts?: PreconditionOptions; } export type MakeFilePrivateResponse = [Metadata]; @@ -1239,7 +1239,7 @@ class File extends ServiceObject { this.storage.retryOptions.autoRetry = false; } - if (options.preconditionOpts?.ifGenerationMatch != undefined) { + if (options.preconditionOpts?.ifGenerationMatch !== undefined) { query.ifGenerationMatch = options.preconditionOpts?.ifGenerationMatch; delete options.preconditionOpts; } @@ -3084,7 +3084,7 @@ class File extends ServiceObject { // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; - if (options.preconditionOpts?.ifGenerationMatch != undefined) { + if (options.preconditionOpts?.ifGenerationMatch !== undefined) { query.ifGenerationMatch = options.preconditionOpts?.ifGenerationMatch; delete options.preconditionOpts; } @@ -3706,7 +3706,7 @@ class File extends ServiceObject { AvailableServiceObjectMethods.setMetadata, options ); - + super .setMetadata(metadata, options) .then(resp => cb!(null, ...resp)) diff --git a/src/nodejs-common/service-object.ts b/src/nodejs-common/service-object.ts index ab8e45a58..5e8ead0e6 100644 --- a/src/nodejs-common/service-object.ts +++ b/src/nodejs-common/service-object.ts @@ -114,7 +114,7 @@ export interface CreateCallback { } export type DeleteOptions = { - ignoreNotFound?: boolean + ignoreNotFound?: boolean; ifGenerationMatch?: number; ifGenerationNotMatch?: number; ifMetagenerationMatch?: number; @@ -288,8 +288,6 @@ class ServiceObject extends EventEmitter { const ignoreNotFound = options.ignoreNotFound!; delete options.ignoreNotFound; - - const methodConfig = (typeof this.methods.delete === 'object' && this.methods.delete) || {}; diff --git a/test/file.ts b/test/file.ts index b6efaae1c..00e6de506 100644 --- a/test/file.ts +++ b/test/file.ts @@ -4170,9 +4170,13 @@ describe('File', () => { return newFile; }; - file.copy = (destination: string, options: object, callback: Function) => { + file.copy = ( + destination: string, + options: object, + callback: Function + ) => { assert.strictEqual(destination, newFile); - assert.deepStrictEqual(options, {}) + assert.deepStrictEqual(options, {}); callback(); // done() }; From 859b98dd2a2c16e5ad110ecdeb3bb0262dba8769 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Thu, 28 Jul 2022 16:58:12 +0000 Subject: [PATCH 37/58] docs and cleanup --- conformance-test/libraryMethods.ts | 12 ++---------- conformance-test/test-data/retryInvocationMap.json | 2 -- src/bucket.ts | 9 +++++++-- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 954c4f584..638d186f8 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -90,16 +90,8 @@ export async function deleteBucket(bucket: Bucket) { await bucket.delete(); } -export async function deleteFilesInstancePrecondition(bucket: Bucket) { - await bucket.deleteFiles(); -} - -export async function deleteFiles(bucket: Bucket) { - //TODO -- how to pass preconditions on the file? - await bucket.deleteFiles({ - ifMetagenerationMatch: 2, - }); -} +// Note: bucket.deleteFiles is missing from these tests +// Preconditions cannot be implemented with current setup. export async function deleteLabelsInstancePrecondition(bucket: Bucket) { await bucket.deleteLabels(); diff --git a/conformance-test/test-data/retryInvocationMap.json b/conformance-test/test-data/retryInvocationMap.json index 1a054d588..c9ecf0f4d 100644 --- a/conformance-test/test-data/retryInvocationMap.json +++ b/conformance-test/test-data/retryInvocationMap.json @@ -79,9 +79,7 @@ "createResumableUpload" ], "storage.objects.delete": [ - "deleteFilesInstancePrecondition", "fileDeleteInstancePrecondition", - "deleteFiles", "fileDelete" ], "storage.objects.get": [ diff --git a/src/bucket.ts b/src/bucket.ts index a9044e027..bea594a0e 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -1945,6 +1945,9 @@ class Bucket extends ServiceObject { * breaks the loop and will execute the provided callback with it. Specify * `{ 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}. @@ -2060,8 +2063,8 @@ 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 | DeleteLabelsOptions} [optionsOrCallback] - * Callback function or precondition options. + * @param {DeleteLabelsCallback} [callback] Callback function. + * @param {DeleteLabelsOptions} [options] Options, including precondition options * @returns {Promise} * * @example @@ -2179,6 +2182,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 @@ -3416,6 +3420,7 @@ class Bucket extends ServiceObject { * locked. * * @param {SetBucketMetadataCallback} [callback] Callback function. + * @param {SetBucketMetadataOptions} [options] Options, including precondition options * @returns {Promise} * * @example From a03baaac191a47d6e6a39982878f22e95f2063c3 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Thu, 28 Jul 2022 20:06:34 +0000 Subject: [PATCH 38/58] refactored conformance tests --- conformance-test/conformanceCommon.ts | 33 +- conformance-test/libraryMethods.ts | 826 +++++++++++++++----------- 2 files changed, 489 insertions(+), 370 deletions(-) diff --git a/conformance-test/conformanceCommon.ts b/conformance-test/conformanceCommon.ts index 8ae7e7647..9a37d3eb9 100644 --- a/conformance-test/conformanceCommon.ts +++ b/conformance-test/conformanceCommon.ts @@ -134,29 +134,24 @@ export function executeScenario(testCase: RetryTestCase) { }); it(`${instructionNumber}`, async () => { + let 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 - ) + 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(async () => { + await storageMethodObject(methodParameters); + }); } const testBenchResult = await getTestBenchRetryTest( creationResult.id diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 638d186f8..a90e4c5ac 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -15,13 +15,25 @@ import {Bucket, File, Notification, Storage, HmacKey} from '../src'; import * as path from 'path'; import {ApiError} from '../src/nodejs-common'; +import { option } from 'yargs'; + +export interface ConformanceTestOptions { + bucket?: Bucket; + file?: File; + notification?: Notification; + storage?: Storage; + hmacKey?: HmacKey; + preconditionRequired?: boolean; +} ///////////////////////////////////////////////// //////////////////// BUCKET ///////////////////// ///////////////////////////////////////////////// -export async function addLifecycleRuleInstancePrecondition(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,257 +41,362 @@ export async function addLifecycleRuleInstancePrecondition(bucket: Bucket) { }); } -export async function addLifecycleRule(bucket: Bucket) { - await bucket.addLifecycleRule( - { - action: 'delete', - condition: { - age: 365 * 3, // Specified in days. +export async function addLifecycleRule(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.addLifecycleRule( + { + action: 'delete', + condition: { + age: 365 * 3, // Specified in days. + }, }, - }, - { - ifMetagenerationMatch: 2, - } - ); + { + ifMetagenerationMatch: 2, + } + ); + } + else { + await options.bucket!.addLifecycleRule( + { + action: 'delete', + condition: { + age: 365 * 3, // Specified in days. + }, + } + ); + } } -export async function combineInstancePrecondition(bucket: Bucket) { - const file1 = bucket.file('file1.txt'); - const file2 = bucket.file('file2.txt'); +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'); + let allFiles; const sources = [file1, file2]; - const allFiles = bucket.file('all-files.txt', { - preconditionOpts: { - ifGenerationMatch: 0, - }, - }); - await bucket.combine(sources, allFiles); + 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(bucket: Bucket) { - const file1 = bucket.file('file1.txt'); - const file2 = bucket.file('file2.txt'); +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 = bucket.file('all-files.txt'); + const allFiles = options.bucket!.file('all-files.txt'); await allFiles.save('allfiles contents'); - await bucket.combine(sources, allFiles, { - ifGenerationMatch: allFiles.metadata.generation, - }); + 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(); } // Note: bucket.deleteFiles is missing from these tests // Preconditions cannot be implemented with current setup. -export async function deleteLabelsInstancePrecondition(bucket: Bucket) { - await bucket.deleteLabels(); +export async function deleteLabelsInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.deleteLabels(); } -export async function deleteLabels(bucket: Bucket) { - await bucket.deleteLabels({ - ifMetagenerationMatch: 2, - }); +export async function deleteLabels(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.deleteLabels({ + ifMetagenerationMatch: 2, + }); + } + else { + await options.bucket!.deleteLabels(); + } } -export async function disableRequesterPaysInstancePrecondition(bucket: Bucket) { - await bucket.disableRequesterPays(); +export async function disableRequesterPaysInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.disableRequesterPays(); } -export async function disableRequesterPays(bucket: Bucket) { - await bucket.disableRequesterPays({ - ifMetagenerationMatch: 2, - }); +export async function disableRequesterPays(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.disableRequesterPays({ + ifMetagenerationMatch: 2, + }); + } + else { + await options.bucket!.disableRequesterPays(); + } } -export async function enableLoggingInstancePrecondition(bucket: Bucket) { +export async function enableLoggingInstancePrecondition( + options: ConformanceTestOptions +) { const config = { prefix: 'log', }; - await bucket.enableLogging(config); + await options.bucket!.enableLogging(config); } -export async function enableLogging(bucket: Bucket) { - const config = { - prefix: 'log', - ifMetagenerationMatch: 2, - }; - await 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 enableRequesterPaysInstancePrecondition(bucket: Bucket) { - await bucket.enableRequesterPays(); +export async function enableRequesterPaysInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.enableRequesterPays(); } -export async function enableRequesterPays(bucket: Bucket) { - await bucket.enableRequesterPays({ - ifMetagenerationMatch: 2, - }); +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(bucket: Bucket) { - await bucket.makePrivate(); +export async function bucketMakePrivateInstancePrecondition( + options: ConformanceTestOptions +) { + await options.bucket!.makePrivate(); } -export async function bucketMakePrivate(bucket: Bucket) { - await bucket.makePrivate({preconditionOpts: {ifMetagenerationMatch: 2}}); +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(bucket: Bucket) { - await bucket.makePublic(); +export async function bucketMakePublic(options: ConformanceTestOptions) { + await options.bucket!.makePublic(); } export async function removeRetentionPeriodInstancePrecondition( - bucket: Bucket + options: ConformanceTestOptions ) { - await bucket.removeRetentionPeriod(); + await options.bucket!.removeRetentionPeriod(); } -export async function removeRetentionPeriod(bucket: Bucket) { - await bucket.removeRetentionPeriod({ - ifMetagenerationMatch: 2, - }); +export async function removeRetentionPeriod(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.bucket!.removeRetentionPeriod({ + ifMetagenerationMatch: 2, + }); + } + else { + await options.bucket!.removeRetentionPeriod(); + } } -export async function setCorsConfigurationInstancePrecondition(bucket: Bucket) { +export async function setCorsConfigurationInstancePrecondition( + options: ConformanceTestOptions +) { const corsConfiguration = [{maxAgeSeconds: 3600}]; // 1 hour - await bucket.setCorsConfiguration(corsConfiguration); + 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, { - ifMetagenerationMatch: 2, - }); + if (options.preconditionRequired) { + await options.bucket!.setCorsConfiguration(corsConfiguration, { + ifMetagenerationMatch: 2, + }); + } + else { + await options.bucket!.setCorsConfiguration(corsConfiguration); + } } -export async function setLabelsInstancePrecondition(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 setLabels(bucket: Bucket) { +export async function setLabels(options: ConformanceTestOptions) { const labels = { labelone: 'labelonevalue', labeltwo: 'labeltwovalue', }; - await bucket.setLabels(labels, { - ifMetagenerationMatch: 2, - }); + if (options.preconditionRequired) { + await options.bucket!.setLabels(labels, { + ifMetagenerationMatch: 2, + }); + } + else { + await options.bucket!.setLabels(labels); + } + } -export async function bucketSetMetadataInstancePrecondition(bucket: Bucket) { +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 bucketSetMetadata(bucket: Bucket) { +export async function bucketSetMetadata(options: ConformanceTestOptions) { const metadata = { website: { mainPageSuffix: 'http://example.com', notFoundPage: 'http://example.com/404.html', }, }; - await bucket.setMetadata(metadata, { - ifMetagenerationMatch: 2, - }); + if (options.preconditionRequired) { + await options.bucket!.setMetadata(metadata, { + ifMetagenerationMatch: 2, + }); + } + else { + await options.bucket!.setMetadata(metadata); + } } -export async function setRetentionPeriodInstancePrecondition(bucket: Bucket) { +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(bucket: Bucket) { +export async function setRetentionPeriod(options: ConformanceTestOptions) { const DURATION_SECONDS = 15780000; // 6 months. - await bucket.setRetentionPeriod(DURATION_SECONDS, { - ifMetagenerationMatch: 2, - }); + if (options.preconditionRequired) { + await options.bucket!.setRetentionPeriod(DURATION_SECONDS, { + ifMetagenerationMatch: 2, + }); + } + else { + await options.bucket!.setRetentionPeriod(DURATION_SECONDS); + } } export async function bucketSetStorageClassInstancePrecondition( - bucket: Bucket + options: ConformanceTestOptions ) { - await bucket.setStorageClass('nearline'); + await options.bucket!.setStorageClass('nearline'); } -export async function bucketSetStorageClass(bucket: Bucket) { - await bucket.setStorageClass('nearline', { - ifMetagenerationMatch: 2, - }); +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 bucketUploadResumableInstancePrecondition( - bucket: Bucket + options: ConformanceTestOptions ) { - if (bucket.instancePreconditionOpts) { - bucket.instancePreconditionOpts.ifGenerationMatch = 0; + 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' @@ -288,24 +405,34 @@ export async function bucketUploadResumableInstancePrecondition( ); } -export async function bucketUploadResumable(bucket: Bucket) { - await bucket.upload( - path.join( - __dirname, - '../../conformance-test/test-data/retryStrategyTestData.json' - ), - {preconditionOpts: {ifMetagenerationMatch: 2, 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' + ) + ); + } } export async function bucketUploadMultipartInstancePrecondition( - bucket: Bucket + options: ConformanceTestOptions ) { - if (bucket.instancePreconditionOpts) { - delete bucket.instancePreconditionOpts.ifMetagenerationMatch; - bucket.instancePreconditionOpts.ifGenerationMatch = 0; + if (options.bucket!.instancePreconditionOpts) { + delete options.bucket!.instancePreconditionOpts.ifMetagenerationMatch; + options.bucket!.instancePreconditionOpts.ifGenerationMatch = 0; } - await bucket.upload( + await options.bucket!.upload( path.join( __dirname, '../../conformance-test/test-data/retryStrategyTestData.json' @@ -314,36 +441,53 @@ export async function bucketUploadMultipartInstancePrecondition( ); } -export async function bucketUploadMultipart(bucket: Bucket) { - if (bucket.instancePreconditionOpts) { - delete bucket.instancePreconditionOpts.ifMetagenerationMatch; +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} + ); } - await bucket.upload( - path.join( - __dirname, - '../../conformance-test/test-data/retryStrategyTestData.json' - ), - {resumable: false, preconditionOpts: {ifGenerationMatch: 0}} - ); } ///////////////////////////////////////////////// //////////////////// FILE ///////////////////// ///////////////////////////////////////////////// -export async function copy(bucket: Bucket, file: File) { - const newFile = new File(bucket, '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'); - await file.copy('a-different-file.png', { - preconditionOpts: {ifGenerationMatch: newFile.metadata.generation}, - }); + 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)); @@ -351,126 +495,173 @@ export async function createReadStream(_bucket: Bucket, file: File) { } export async function createResumableUploadInstancePrecondition( - _bucket: Bucket, - file: File + options: ConformanceTestOptions ) { - await file.createResumableUpload(); + await options.file!.createResumableUpload(); } -export async function createResumableUpload(_bucket: Bucket, file: File) { - await file.createResumableUpload({preconditionOpts: {ifGenerationMatch: 0}}); +export async function createResumableUpload(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + await options.file!.createResumableUpload({ + preconditionOpts: {ifGenerationMatch: 0}, + }); + } + else { + await options.file!.createResumableUpload(); + } + } export async function fileDeleteInstancePrecondition( - _bucket: Bucket, - file: File + options: ConformanceTestOptions ) { - await file.delete(); + await options.file!.delete(); } -export async function fileDelete(_bucket: Bucket, file: File) { - await file.delete({ - ifGenerationMatch: file.metadata.generation, - }); +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 download(_bucket: Bucket, file: File) { - await file.download(); +export async function download(options: ConformanceTestOptions) { + await options.file!.download(); } -export async function exists(_bucket: Bucket, file: File) { - await file.exists(); +export async function exists(options: ConformanceTestOptions) { + await options.file!.exists(); } -export async function get(_bucket: Bucket, file: File) { - await file.get(); +export async function get(options: ConformanceTestOptions) { + await options.file!.get(); } -export async function getExpirationDate(bucket: Bucket, file: File) { - await file.getExpirationDate(); +export async function getExpirationDate(options: ConformanceTestOptions) { + await options.file!.getExpirationDate(); } -export async function getMetadata(_bucket: Bucket, file: File) { - await file.getMetadata(); +export async function getMetadata(options: ConformanceTestOptions) { + await options.file!.getMetadata(); } -export async function isPublic(_bucket: Bucket, file: File) { - await file.isPublic(); +export async function isPublic(options: ConformanceTestOptions) { + await options.file!.isPublic(); } export async function fileMakePrivateInstancePrecondition( - _bucket: Bucket, - file: File + options: ConformanceTestOptions ) { - await file.makePrivate(); + await options.file!.makePrivate(); } -export async function fileMakePrivate(_bucket: Bucket, file: File) { - await file.makePrivate({ - preconditionOpts: { - ifGenerationMatch: file.metadata.generation, - }, - }); +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 fileMakePublic(_bucket: Bucket, file: File) { - await file.makePublic(); +export async function fileMakePublic(options: ConformanceTestOptions) { + await options.file!.makePublic(); } -export async function move(_bucket: Bucket, file: File) { - await file.move('new-file', {preconditionOpts: {ifGenerationMatch: 0}}); +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(_bucket: Bucket, file: File) { - await file.rename('new-name', {preconditionOpts: {ifGenerationMatch: 0}}); +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(_bucket: Bucket, file: File) { +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'), - preconditionOpts: {ifGenerationMatch: file.metadata.generation}, - }); + 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( - _bucket: Bucket, - file: File + options: ConformanceTestOptions ) { - await file.save('testdata', {resumable: true}); + await options.file!.save('testdata', {resumable: true}); } -export async function saveResumable(_bucket: Bucket, file: File) { - await file.save('testdata', { - resumable: true, - preconditionOpts: { - ifGenerationMatch: file.metadata.generation, - ifMetagenerationMatch: file.metadata.metageneration, - }, - }); +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 saveMultipartInstancePrecondition( - _bucket: Bucket, - file: File + options: ConformanceTestOptions ) { - await file.save('testdata', {resumable: false}); + await options.file!.save('testdata', {resumable: false}); } -export async function saveMultipart(_bucket: Bucket, file: File) { - await file.save('testdata', { - resumable: false, - preconditionOpts: { - ifGenerationMatch: file.metadata.generation, - }, - }); +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 setMetadataInstancePrecondition( - _bucket: Bucket, - file: File + options: ConformanceTestOptions ) { const metadata = { contentType: 'application/x-font-ttf', @@ -479,10 +670,10 @@ export async function setMetadataInstancePrecondition( properties: 'go here', }, }; - await file.setMetadata(metadata); + await options.file!.setMetadata(metadata); } -export async function setMetadata(_bucket: Bucket, file: File) { +export async function setMetadata(options: ConformanceTestOptions) { const metadata = { contentType: 'application/x-font-ttf', metadata: { @@ -490,19 +681,26 @@ export async function setMetadata(_bucket: Bucket, file: File) { properties: 'go here', }, }; - await file.setMetadata(metadata, { - ifGenerationMatch: file.metadata.generation, - }); + if (options.preconditionRequired) { + await options.file!.setMetadata(metadata, { + ifGenerationMatch: options.file!.metadata.generation, + }); + } + else { + await options.file!.setMetadata(metadata); + } } -export async function setStorageClass(_bucket: Bucket, file: File) { - const result = await file.setStorageClass('nearline', { - preconditionOpts: { - ifGenerationMatch: file.metadata.generation, - }, - }); - if (!result) { - throw Error(); +export async function setStorageClass(options: ConformanceTestOptions) { + if (options.preconditionRequired) { + const result = await options.file!.setStorageClass('nearline', { + preconditionOpts: { + ifGenerationMatch: options.file!.metadata.generation, + }, + }); + } + else { + const result = await options.file!.setStorageClass('nearline'); } } @@ -510,62 +708,38 @@ 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(); + 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) { +export async function iamSetPolicy(options: ConformanceTestOptions) { const testPolicy = { bindings: [ { @@ -574,130 +748,80 @@ export async function iamSetPolicy(bucket: Bucket) { }, ], }; - await bucket.iam.setPolicy(testPolicy); + 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(); } From bd7a1f5ed761e6995d22478c102d6f78bc170c49 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Thu, 28 Jul 2022 21:16:57 +0000 Subject: [PATCH 39/58] remove iam test from being in the conformance tests --- conformance-test/libraryMethods.ts | 12 ------------ conformance-test/test-data/retryInvocationMap.json | 1 - src/iam.ts | 7 +++++-- src/nodejs-common/util.ts | 5 ++++- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index a90e4c5ac..d821068ac 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -739,18 +739,6 @@ export async function iamGetPolicy(options: ConformanceTestOptions) { await options.bucket!.iam.getPolicy({requestedPolicyVersion: 1}); } -export async function iamSetPolicy(options: ConformanceTestOptions) { - const testPolicy = { - bindings: [ - { - role: 'roles/storage.admin', - members: ['serviceAccount:myotherproject@appspot.gserviceaccount.com'], - }, - ], - }; - await options.bucket!.iam.setPolicy(testPolicy); -} - export async function iamTestPermissions(options: ConformanceTestOptions) { const permissionToTest = 'storage.buckets.delete'; await options.bucket!.iam.testPermissions(permissionToTest); diff --git a/conformance-test/test-data/retryInvocationMap.json b/conformance-test/test-data/retryInvocationMap.json index c9ecf0f4d..31d62bf1d 100644 --- a/conformance-test/test-data/retryInvocationMap.json +++ b/conformance-test/test-data/retryInvocationMap.json @@ -107,7 +107,6 @@ "iamGetPolicy" ], "storage.buckets.setIamPolicy": [ - "iamSetPolicy" ], "storage.buckets.testIamPermission": [ "iamTestPermissions" diff --git a/src/iam.ts b/src/iam.ts index d75f97e5b..6a591a2ef 100644 --- a/src/iam.ts +++ b/src/iam.ts @@ -22,6 +22,7 @@ import arrify = require('arrify'); import {Bucket} from './bucket'; import {normalize} from './util'; +import { PreconditionOptions } from './storage'; export interface GetPolicyOptions { userProject?: string; @@ -45,7 +46,7 @@ export interface GetPolicyCallback { * @param {string} [userProject] The ID of the project which will be * billed for the request. */ -export interface SetPolicyOptions { +export interface SetPolicyOptions extends PreconditionOptions { userProject?: string; } @@ -344,10 +345,12 @@ class Iam { SetPolicyCallback >(optionsOrCallback, callback); + let maxRetries = 0; // We don't support ETag this.request_( { method: 'PUT', uri: '/iam', + maxRetries, json: Object.assign( { resourceId: this.resourceId_, @@ -357,7 +360,7 @@ class Iam { qs: options, }, cb - ); + ) } testPermissions( diff --git a/src/nodejs-common/util.ts b/src/nodejs-common/util.ts index 8b1d6d990..684cec6ec 100644 --- a/src/nodejs-common/util.ts +++ b/src/nodejs-common/util.ts @@ -850,7 +850,10 @@ export class Util { } let maxRetryValue = MAX_RETRY_DEFAULT; - if (config.maxRetries) { + if (reqOpts.maxRetries) { + maxRetryValue = reqOpts.maxRetries + } + else if (config.maxRetries) { maxRetryValue = config.maxRetries; } else if (config.retryOptions?.maxRetries) { maxRetryValue = config.retryOptions.maxRetries; From 67a66d14bfe44222dc4b77a0cd4f7010af618ba9 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Fri, 29 Jul 2022 15:26:17 +0000 Subject: [PATCH 40/58] linted files --- conformance-test/conformanceCommon.ts | 10 +-- conformance-test/globalHooks.ts | 16 ++-- conformance-test/libraryMethods.ts | 106 +++++++++----------------- src/bucket.ts | 2 +- src/iam.ts | 6 +- src/nodejs-common/util.ts | 7 +- test/iam.ts | 1 + 7 files changed, 56 insertions(+), 92 deletions(-) diff --git a/conformance-test/conformanceCommon.ts b/conformance-test/conformanceCommon.ts index 9a37d3eb9..d21a38dd0 100644 --- a/conformance-test/conformanceCommon.ts +++ b/conformance-test/conformanceCommon.ts @@ -134,20 +134,18 @@ export function executeScenario(testCase: RetryTestCase) { }); it(`${instructionNumber}`, async () => { - let methodParameters : libraryMethods.ConformanceTestOptions = { + const methodParameters: libraryMethods.ConformanceTestOptions = { bucket: bucket, file: file, notification: notification, storage: storage, hmacKey: hmacKey, - } + }; if (testCase.preconditionProvided) { - methodParameters.preconditionRequired = true; + methodParameters.preconditionRequired = true; } if (testCase.expectSuccess) { - assert.ifError( - await storageMethodObject(methodParameters) - ); + assert.ifError(await storageMethodObject(methodParameters)); } else { await assert.rejects(async () => { await storageMethodObject(methodParameters); diff --git a/conformance-test/globalHooks.ts b/conformance-test/globalHooks.ts index 0775b7457..2567d5349 100644 --- a/conformance-test/globalHooks.ts +++ b/conformance-test/globalHooks.ts @@ -25,17 +25,17 @@ const TIME_TO_WAIT_FOR_CONTAINER_READY = 10000; // eslint-disable-next-line @typescript-eslint/no-explicit-any export async function mochaGlobalSetup(this: any) { // Increase the timeout for this before block so that the docker images have time to download and run. - this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; - await getTestBenchDockerImage(); - await runTestBenchDockerImage(); - await new Promise(resolve => - setTimeout(resolve, TIME_TO_WAIT_FOR_CONTAINER_READY) - ); + // this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; + // await getTestBenchDockerImage(); + // await runTestBenchDockerImage(); + // await new Promise(resolve => + // setTimeout(resolve, TIME_TO_WAIT_FOR_CONTAINER_READY) + // ); } // eslint-disable-next-line @typescript-eslint/no-explicit-any export async function mochaGlobalTeardown(this: any) { // Increase the timeout for this block so that docker has time to stop the container. - this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; - await stopTestBenchDockerImage(); + // this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; + // await stopTestBenchDockerImage(); } diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index d821068ac..4ac9fe855 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -15,7 +15,6 @@ import {Bucket, File, Notification, Storage, HmacKey} from '../src'; import * as path from 'path'; import {ApiError} from '../src/nodejs-common'; -import { option } from 'yargs'; export interface ConformanceTestOptions { bucket?: Bucket; @@ -54,16 +53,13 @@ export async function addLifecycleRule(options: ConformanceTestOptions) { ifMetagenerationMatch: 2, } ); - } - else { - await options.bucket!.addLifecycleRule( - { - action: 'delete', - condition: { - age: 365 * 3, // Specified in days. - }, - } - ); + } else { + await options.bucket!.addLifecycleRule({ + action: 'delete', + condition: { + age: 365 * 3, // Specified in days. + }, + }); } } @@ -82,8 +78,7 @@ export async function combineInstancePrecondition( ifGenerationMatch: 0, }, }); - } - else { + } else { allFiles = options.bucket!.file('all-files.txt'); } @@ -102,8 +97,7 @@ export async function combine(options: ConformanceTestOptions) { await options.bucket!.combine(sources, allFiles, { ifGenerationMatch: allFiles.metadata.generation, }); - } - else { + } else { await options.bucket!.combine(sources, allFiles); } } @@ -142,8 +136,7 @@ export async function deleteLabels(options: ConformanceTestOptions) { await options.bucket!.deleteLabels({ ifMetagenerationMatch: 2, }); - } - else { + } else { await options.bucket!.deleteLabels(); } } @@ -159,8 +152,7 @@ export async function disableRequesterPays(options: ConformanceTestOptions) { await options.bucket!.disableRequesterPays({ ifMetagenerationMatch: 2, }); - } - else { + } else { await options.bucket!.disableRequesterPays(); } } @@ -181,8 +173,7 @@ export async function enableLogging(options: ConformanceTestOptions) { prefix: 'log', ifMetagenerationMatch: 2, }; - } - else { + } else { config = { prefix: 'log', }; @@ -201,8 +192,7 @@ export async function enableRequesterPays(options: ConformanceTestOptions) { await options.bucket!.enableRequesterPays({ ifMetagenerationMatch: 2, }); - } - else { + } else { await options.bucket!.enableRequesterPays(); } } @@ -253,8 +243,7 @@ export async function bucketMakePrivate(options: ConformanceTestOptions) { await options.bucket!.makePrivate({ preconditionOpts: {ifMetagenerationMatch: 2}, }); - } - else { + } else { await options.bucket!.makePrivate(); } } @@ -274,8 +263,7 @@ export async function removeRetentionPeriod(options: ConformanceTestOptions) { await options.bucket!.removeRetentionPeriod({ ifMetagenerationMatch: 2, }); - } - else { + } else { await options.bucket!.removeRetentionPeriod(); } } @@ -293,8 +281,7 @@ export async function setCorsConfiguration(options: ConformanceTestOptions) { await options.bucket!.setCorsConfiguration(corsConfiguration, { ifMetagenerationMatch: 2, }); - } - else { + } else { await options.bucket!.setCorsConfiguration(corsConfiguration); } } @@ -318,11 +305,9 @@ export async function setLabels(options: ConformanceTestOptions) { await options.bucket!.setLabels(labels, { ifMetagenerationMatch: 2, }); - } - else { + } else { await options.bucket!.setLabels(labels); } - } export async function bucketSetMetadataInstancePrecondition( @@ -348,8 +333,7 @@ export async function bucketSetMetadata(options: ConformanceTestOptions) { await options.bucket!.setMetadata(metadata, { ifMetagenerationMatch: 2, }); - } - else { + } else { await options.bucket!.setMetadata(metadata); } } @@ -367,8 +351,7 @@ export async function setRetentionPeriod(options: ConformanceTestOptions) { await options.bucket!.setRetentionPeriod(DURATION_SECONDS, { ifMetagenerationMatch: 2, }); - } - else { + } else { await options.bucket!.setRetentionPeriod(DURATION_SECONDS); } } @@ -384,8 +367,7 @@ export async function bucketSetStorageClass(options: ConformanceTestOptions) { await options.bucket!.setStorageClass('nearline', { ifMetagenerationMatch: 2, }); - } - else { + } else { await options.bucket!.setStorageClass('nearline'); } } @@ -414,8 +396,7 @@ export async function bucketUploadResumable(options: ConformanceTestOptions) { ), {preconditionOpts: {ifMetagenerationMatch: 2, ifGenerationMatch: 0}} ); - } - else { + } else { await options.bucket!.upload( path.join( __dirname, @@ -454,8 +435,7 @@ export async function bucketUploadMultipart(options: ConformanceTestOptions) { ), {resumable: false, preconditionOpts: {ifGenerationMatch: 0}} ); - } - else { + } else { await options.bucket!.upload( path.join( __dirname, @@ -478,8 +458,7 @@ export async function copy(options: ConformanceTestOptions) { await options.file!.copy('a-different-file.png', { preconditionOpts: {ifGenerationMatch: newFile.metadata.generation}, }); - } - else { + } else { await options.file!.copy('a-different-file.png'); } } @@ -505,11 +484,9 @@ export async function createResumableUpload(options: ConformanceTestOptions) { await options.file!.createResumableUpload({ preconditionOpts: {ifGenerationMatch: 0}, }); - } - else { + } else { await options.file!.createResumableUpload(); } - } export async function fileDeleteInstancePrecondition( @@ -523,8 +500,7 @@ export async function fileDelete(options: ConformanceTestOptions) { await options.file!.delete({ ifGenerationMatch: options.file!.metadata.generation, }); - } - else { + } else { await options.file!.delete(); } } @@ -566,8 +542,7 @@ export async function fileMakePrivate(options: ConformanceTestOptions) { ifGenerationMatch: options.file!.metadata.generation, }, }); - } - else { + } else { await options.file!.makePrivate(); } } @@ -581,11 +556,9 @@ export async function move(options: ConformanceTestOptions) { await options.file!.move('new-file', { preconditionOpts: {ifGenerationMatch: 0}, }); - } - else { + } else { await options.file!.move('new-file'); } - } export async function rename(options: ConformanceTestOptions) { @@ -593,8 +566,7 @@ export async function rename(options: ConformanceTestOptions) { await options.file!.rename('new-name', { preconditionOpts: {ifGenerationMatch: 0}, }); - } - else { + } else { await options.file!.rename('new-name'); } } @@ -608,10 +580,10 @@ export async function rotateEncryptionKey(options: ConformanceTestOptions) { encryptionKey: Buffer.from(newKey, 'base64'), preconditionOpts: {ifGenerationMatch: options.file!.metadata.generation}, }); - } - else { + } else { await options.file!.rotateEncryptionKey({ - encryptionKey: Buffer.from(newKey, 'base64')}); + encryptionKey: Buffer.from(newKey, 'base64'), + }); } } @@ -630,8 +602,7 @@ export async function saveResumable(options: ConformanceTestOptions) { ifMetagenerationMatch: options.file!.metadata.metageneration, }, }); - } - else { + } else { await options.file!.save('testdata', { resumable: true, }); @@ -652,8 +623,7 @@ export async function saveMultipart(options: ConformanceTestOptions) { ifGenerationMatch: options.file!.metadata.generation, }, }); - } - else { + } else { await options.file!.save('testdata', { resumable: false, }); @@ -685,22 +655,20 @@ export async function setMetadata(options: ConformanceTestOptions) { await options.file!.setMetadata(metadata, { ifGenerationMatch: options.file!.metadata.generation, }); - } - else { + } else { await options.file!.setMetadata(metadata); } } export async function setStorageClass(options: ConformanceTestOptions) { if (options.preconditionRequired) { - const result = await options.file!.setStorageClass('nearline', { + await options.file!.setStorageClass('nearline', { preconditionOpts: { ifGenerationMatch: options.file!.metadata.generation, }, }); - } - else { - const result = await options.file!.setStorageClass('nearline'); + } else { + await options.file!.setStorageClass('nearline'); } } diff --git a/src/bucket.ts b/src/bucket.ts index bea594a0e..5d5adacff 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -1945,7 +1945,7 @@ class Bucket extends ServiceObject { * breaks the loop and will execute the provided callback with it. Specify * `{ 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. * diff --git a/src/iam.ts b/src/iam.ts index 6a591a2ef..cf30f87f9 100644 --- a/src/iam.ts +++ b/src/iam.ts @@ -22,7 +22,7 @@ import arrify = require('arrify'); import {Bucket} from './bucket'; import {normalize} from './util'; -import { PreconditionOptions } from './storage'; +import {PreconditionOptions} from './storage'; export interface GetPolicyOptions { userProject?: string; @@ -345,7 +345,7 @@ class Iam { SetPolicyCallback >(optionsOrCallback, callback); - let maxRetries = 0; // We don't support ETag + const maxRetries = 0; // We don't support ETag this.request_( { method: 'PUT', @@ -360,7 +360,7 @@ class Iam { qs: options, }, cb - ) + ); } testPermissions( diff --git a/src/nodejs-common/util.ts b/src/nodejs-common/util.ts index 684cec6ec..27d203111 100644 --- a/src/nodejs-common/util.ts +++ b/src/nodejs-common/util.ts @@ -850,12 +850,9 @@ export class Util { } let maxRetryValue = MAX_RETRY_DEFAULT; - if (reqOpts.maxRetries) { - maxRetryValue = reqOpts.maxRetries - } - else 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; } 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_, From f991b958addcfdacba43bd4db4cf6851b434d0a1 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Fri, 29 Jul 2022 15:27:14 +0000 Subject: [PATCH 41/58] put docker commands back --- conformance-test/globalHooks.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/conformance-test/globalHooks.ts b/conformance-test/globalHooks.ts index 2567d5349..0775b7457 100644 --- a/conformance-test/globalHooks.ts +++ b/conformance-test/globalHooks.ts @@ -25,17 +25,17 @@ const TIME_TO_WAIT_FOR_CONTAINER_READY = 10000; // eslint-disable-next-line @typescript-eslint/no-explicit-any export async function mochaGlobalSetup(this: any) { // Increase the timeout for this before block so that the docker images have time to download and run. - // this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; - // await getTestBenchDockerImage(); - // await runTestBenchDockerImage(); - // await new Promise(resolve => - // setTimeout(resolve, TIME_TO_WAIT_FOR_CONTAINER_READY) - // ); + this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; + await getTestBenchDockerImage(); + await runTestBenchDockerImage(); + await new Promise(resolve => + setTimeout(resolve, TIME_TO_WAIT_FOR_CONTAINER_READY) + ); } // eslint-disable-next-line @typescript-eslint/no-explicit-any export async function mochaGlobalTeardown(this: any) { // Increase the timeout for this block so that docker has time to stop the container. - // this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; - // await stopTestBenchDockerImage(); + this.suite._timeout = TIMEOUT_FOR_DOCKER_OPS; + await stopTestBenchDockerImage(); } From 40e084226a096572e9f4d01d583acaaf4346909a Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Fri, 29 Jul 2022 15:35:00 +0000 Subject: [PATCH 42/58] fixed combine retries --- src/bucket.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/bucket.ts b/src/bucket.ts index 5d5adacff..224e6f807 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -1512,6 +1512,12 @@ class Bucket extends ServiceObject { options = optionsOrCallback; } + this.disableAutoRetryConditionallyIdempotent_( + this.methods.setMetadata, + AvailableServiceObjectMethods.setMetadata, + options + ); + const convertToFile = (file: string | File): File => { if (file instanceof File) { return file; @@ -1574,6 +1580,7 @@ class Bucket extends ServiceObject { qs: options, }, (err, resp) => { + this.storage.retryOptions.autoRetry = this.instanceRetryValue; if (err) { callback!(err, null, resp); return; From 3896201f427b7d48da1c3eebecf88b40ee07e5be Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Fri, 29 Jul 2022 15:42:12 +0000 Subject: [PATCH 43/58] added comments --- src/bucket.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bucket.ts b/src/bucket.ts index 224e6f807..bd6d8f34f 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -1513,8 +1513,8 @@ class Bucket extends ServiceObject { } this.disableAutoRetryConditionallyIdempotent_( - this.methods.setMetadata, - AvailableServiceObjectMethods.setMetadata, + this.methods.setMetadata, // Not relevant but param is required + AvailableServiceObjectMethods.setMetadata, // Same as above options ); From db80d3b7d467da149d574179997f136a1dd0785f Mon Sep 17 00:00:00 2001 From: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> Date: Fri, 29 Jul 2022 12:19:04 -0400 Subject: [PATCH 44/58] 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 --- conformance-test/libraryMethods.ts | 2 +- .../test-data/retryInvocationMap.json | 1 - src/hmacKey.ts | 77 ++++++++++++++++++- test/hmacKey.ts | 31 +++++++- 4 files changed, 106 insertions(+), 5 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 4ac9fe855..bb04d8520 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -680,7 +680,7 @@ export async function deleteHMAC(options: ConformanceTestOptions) { const metadata = { state: 'INACTIVE', }; - options.hmacKey!.setMetadata(metadata); + await options.hmacKey!.setMetadata(metadata); await options.hmacKey!.delete(); } diff --git a/conformance-test/test-data/retryInvocationMap.json b/conformance-test/test-data/retryInvocationMap.json index 31d62bf1d..1f936db09 100644 --- a/conformance-test/test-data/retryInvocationMap.json +++ b/conformance-test/test-data/retryInvocationMap.json @@ -127,7 +127,6 @@ "getMetadataHMAC" ], "storage.hmacKey.update": [ - "setMetadataHMAC" ], "storage.hmacKey.create": [ "createHMACKey" 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/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); + }); }); }); From 80909b5e48c20d298d8a9315742a490e7b86a8f1 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Fri, 29 Jul 2022 16:24:44 +0000 Subject: [PATCH 45/58] retry based on idempotency strategy --- src/iam.ts | 13 +++++++++++-- test/iam.ts | 7 +++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/iam.ts b/src/iam.ts index cf30f87f9..7a76e3881 100644 --- a/src/iam.ts +++ b/src/iam.ts @@ -22,7 +22,7 @@ import arrify = require('arrify'); import {Bucket} from './bucket'; import {normalize} from './util'; -import {PreconditionOptions} from './storage'; +import {IdempotencyStrategy, PreconditionOptions} from './storage'; export interface GetPolicyOptions { userProject?: string; @@ -149,10 +149,12 @@ class Iam { callback: BodyResponseCallback ) => void; private resourceId_: string; + private bucket_: Bucket; constructor(bucket: Bucket) { this.request_ = bucket.request.bind(bucket); this.resourceId_ = 'buckets/' + bucket.getId(); + this.bucket_ = bucket; } getPolicy(options?: GetPolicyOptions): Promise; @@ -345,7 +347,14 @@ class Iam { SetPolicyCallback >(optionsOrCallback, callback); - const maxRetries = 0; // We don't support ETag + let maxRetries = this.bucket_.storage.retryOptions.maxRetries; + // ETag preconditions are not currently supported. Retries should be disabled if the idempotency strategy is not set to RetryAlways + if ( + this.bucket_.storage.retryOptions.idempotencyStrategy !== + IdempotencyStrategy.RetryAlways + ) { + maxRetries = 0; + } this.request_( { method: 'PUT', diff --git a/test/iam.ts b/test/iam.ts index 9e03ff508..8c2d45f95 100644 --- a/test/iam.ts +++ b/test/iam.ts @@ -17,6 +17,7 @@ import * as assert from 'assert'; import {describe, it, before, beforeEach} from 'mocha'; import * as proxyquire from 'proxyquire'; import {IAMExceptionMessages} from '../src/iam'; +import { IdempotencyStrategy } from '../src'; describe('storage/iam', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -48,6 +49,12 @@ describe('storage/iam', () => { id, request: util.noop, getId: () => id, + storage: { + retryOptions: { + idempotencyStrategy: IdempotencyStrategy.RetryConditional, + maxRetries: 3 + } + } }; iam = new Iam(BUCKET_INSTANCE); From 0014091d3942c4172e7da00d091214dd67eed027 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Fri, 29 Jul 2022 16:31:57 +0000 Subject: [PATCH 46/58] linted file --- test/iam.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/iam.ts b/test/iam.ts index 8c2d45f95..e2dcf0836 100644 --- a/test/iam.ts +++ b/test/iam.ts @@ -17,7 +17,7 @@ import * as assert from 'assert'; import {describe, it, before, beforeEach} from 'mocha'; import * as proxyquire from 'proxyquire'; import {IAMExceptionMessages} from '../src/iam'; -import { IdempotencyStrategy } from '../src'; +import {IdempotencyStrategy} from '../src'; describe('storage/iam', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -52,9 +52,9 @@ describe('storage/iam', () => { storage: { retryOptions: { idempotencyStrategy: IdempotencyStrategy.RetryConditional, - maxRetries: 3 - } - } + maxRetries: 3, + }, + }, }; iam = new Iam(BUCKET_INSTANCE); From 722f97551a462d8dbc3b18efef54eb5b293f1001 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Fri, 29 Jul 2022 16:45:55 +0000 Subject: [PATCH 47/58] Revert "retry based on idempotency strategy" This reverts commit 80909b5e48c20d298d8a9315742a490e7b86a8f1. --- src/iam.ts | 13 ++----------- test/iam.ts | 7 ------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/src/iam.ts b/src/iam.ts index 7a76e3881..cf30f87f9 100644 --- a/src/iam.ts +++ b/src/iam.ts @@ -22,7 +22,7 @@ import arrify = require('arrify'); import {Bucket} from './bucket'; import {normalize} from './util'; -import {IdempotencyStrategy, PreconditionOptions} from './storage'; +import {PreconditionOptions} from './storage'; export interface GetPolicyOptions { userProject?: string; @@ -149,12 +149,10 @@ class Iam { callback: BodyResponseCallback ) => void; private resourceId_: string; - private bucket_: Bucket; constructor(bucket: Bucket) { this.request_ = bucket.request.bind(bucket); this.resourceId_ = 'buckets/' + bucket.getId(); - this.bucket_ = bucket; } getPolicy(options?: GetPolicyOptions): Promise; @@ -347,14 +345,7 @@ class Iam { SetPolicyCallback >(optionsOrCallback, callback); - let maxRetries = this.bucket_.storage.retryOptions.maxRetries; - // ETag preconditions are not currently supported. Retries should be disabled if the idempotency strategy is not set to RetryAlways - if ( - this.bucket_.storage.retryOptions.idempotencyStrategy !== - IdempotencyStrategy.RetryAlways - ) { - maxRetries = 0; - } + const maxRetries = 0; // We don't support ETag this.request_( { method: 'PUT', diff --git a/test/iam.ts b/test/iam.ts index e2dcf0836..9e03ff508 100644 --- a/test/iam.ts +++ b/test/iam.ts @@ -17,7 +17,6 @@ import * as assert from 'assert'; import {describe, it, before, beforeEach} from 'mocha'; import * as proxyquire from 'proxyquire'; import {IAMExceptionMessages} from '../src/iam'; -import {IdempotencyStrategy} from '../src'; describe('storage/iam', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -49,12 +48,6 @@ describe('storage/iam', () => { id, request: util.noop, getId: () => id, - storage: { - retryOptions: { - idempotencyStrategy: IdempotencyStrategy.RetryConditional, - maxRetries: 3, - }, - }, }; iam = new Iam(BUCKET_INSTANCE); From 65268f0101687ef63db2ad801ac7bab5e146dc75 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Tue, 2 Aug 2022 15:06:50 +0000 Subject: [PATCH 48/58] don't retry acl adds --- src/acl.ts | 1 + 1 file changed, 1 insertion(+) 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(), From 44c28c6c2ff05a315710c85b4dc30487c7991a6c Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Tue, 2 Aug 2022 20:25:10 +0000 Subject: [PATCH 49/58] changed HEAD request to GET request --- src/file.ts | 2 +- test/file.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/file.ts b/src/file.ts index c0ff10b40..91db2c670 100644 --- a/src/file.ts +++ b/src/file.ts @@ -2984,7 +2984,7 @@ class File extends ServiceObject { util.makeRequest( { - method: 'HEAD', + method: 'GET', uri: `${this.storage.apiEndpoint}/${ this.bucket.name }/${encodeURIComponent(this.name)}`, diff --git a/test/file.ts b/test/file.ts index 00e6de506..1cce36f12 100644 --- a/test/file.ts +++ b/test/file.ts @@ -3827,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) => { From 6af60de732f679a0867f7f44f0a216d8616f6da4 Mon Sep 17 00:00:00 2001 From: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> Date: Tue, 2 Aug 2022 16:43:54 -0400 Subject: [PATCH 50/58] 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 --- src/file.ts | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/src/file.ts b/src/file.ts index 91db2c670..9831333ee 100644 --- a/src/file.ts +++ b/src/file.ts @@ -73,7 +73,12 @@ import {HashStreamValidator} from './hash-stream-validator'; import {URL} from 'url'; import retry = require('async-retry'); -import {SetMetadataOptions} from './nodejs-common/service-object'; +import { + DeleteCallback, + DeleteOptions, + SetMetadataOptions, +} from './nodejs-common/service-object'; +import * as r from 'teeny-request'; export type GetExpirationDateResponse = [Date]; export interface GetExpirationDateCallback { @@ -2035,6 +2040,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; @@ -3987,7 +4025,8 @@ class File extends ServiceObject { (typeof coreOpts === 'object' && coreOpts?.reqOpts?.qs?.ifGenerationMatch === undefined && localPreconditionOptions?.ifGenerationMatch === undefined && - methodType === AvailableServiceObjectMethods.setMetadata && + (methodType === AvailableServiceObjectMethods.setMetadata || + methodType === AvailableServiceObjectMethods.delete) && this.storage.retryOptions.idempotencyStrategy === IdempotencyStrategy.RetryConditional) || this.storage.retryOptions.idempotencyStrategy === From ccde4f5151e8859830c94412615993bf82376729 Mon Sep 17 00:00:00 2001 From: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> Date: Thu, 4 Aug 2022 13:25:23 -0400 Subject: [PATCH 51/58] 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 --- conformance-test/conformanceCommon.ts | 4 +- .../test-data/retryStrategyTestData.json | 812 +++++++++--------- src/nodejs-common/util.ts | 1 + src/storage.ts | 21 +- 4 files changed, 424 insertions(+), 414 deletions(-) diff --git a/conformance-test/conformanceCommon.ts b/conformance-test/conformanceCommon.ts index d21a38dd0..a08f1301e 100644 --- a/conformance-test/conformanceCommon.ts +++ b/conformance-test/conformanceCommon.ts @@ -147,9 +147,7 @@ export function executeScenario(testCase: RetryTestCase) { if (testCase.expectSuccess) { assert.ifError(await storageMethodObject(methodParameters)); } else { - await assert.rejects(async () => { - await storageMethodObject(methodParameters); - }); + await assert.rejects(storageMethodObject(methodParameters)); } const testBenchResult = await getTestBenchRetryTest( creationResult.id 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/nodejs-common/util.ts b/src/nodejs-common/util.ts index 27d203111..3a765ac5c 100644 --- a/src/nodejs-common/util.ts +++ b/src/nodejs-common/util.ts @@ -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; } } From 2d56d9a8bf5175c4572e38737eb5eb99d34fa79f Mon Sep 17 00:00:00 2001 From: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> Date: Fri, 5 Aug 2022 12:27:02 -0400 Subject: [PATCH 52/58] fix: pass appropriate preconditions from enableLogging to setMetadata (#2018) --- src/bucket.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/bucket.ts b/src/bucket.ts index bd6d8f34f..994199d47 100644 --- a/src/bucket.ts +++ b/src/bucket.ts @@ -2321,6 +2321,13 @@ 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 () => { try { const [policy] = await this.iam.getPolicy(); @@ -2336,7 +2343,7 @@ class Bucket extends ServiceObject { logObjectPrefix: config.prefix, }, }, - config, + options, callback! ); } catch (e) { From 50129895da02b841abab9a9a0d1f0751ab597e3b Mon Sep 17 00:00:00 2001 From: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> Date: Fri, 5 Aug 2022 16:54:12 -0400 Subject: [PATCH 53/58] tests: remove callback waterfall from make bucket private system test (#2020) * tests: remove callback waterfall from make bucket private system test * cleaner implementation --- system-test/storage.ts | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/system-test/storage.ts b/system-test/storage.ts index 6904081dc..1b23c00e0 100644 --- a/system-test/storage.ts +++ b/system-test/storage.ts @@ -363,22 +363,17 @@ describe('storage', () => { ]); }); - it('should make a bucket private', done => { - bucket.makePublic(err => { - assert.ifError(err); - bucket.makePrivate(err => { - assert.ifError(err); - bucket.acl.get({entity: 'allUsers'}, (err, aclObject) => { - assert.strictEqual((err as ApiError).code, 404); - assert.strictEqual( - (err as ApiError).errors![0].reason, - 'notFound' - ); - assert.strictEqual(aclObject, null); - done(); - }); + it('should make a bucket private', async () => { + 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 () => { From 36d220b3538c2eebcb9db7657422a9a0bd829c02 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 8 Aug 2022 16:05:19 +0000 Subject: [PATCH 54/58] moved done() --- test/bucket.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/bucket.ts b/test/bucket.ts index ce65bed8d..f9f040616 100644 --- a/test/bucket.ts +++ b/test/bucket.ts @@ -1407,8 +1407,8 @@ describe('Bucket', () => { bucket.setMetadata = () => { Promise.resolve().then(() => { assert.strictEqual(bucket.storage.retryOptions.autoRetry, false); + done(); }); - done(); }; bucket.disableRequesterPays(); }); From bbddedaf4c8c0f8d9939ec1e842e996cf25ae82d Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 8 Aug 2022 16:25:38 +0000 Subject: [PATCH 55/58] removed precondition from policyoptions --- src/iam.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iam.ts b/src/iam.ts index cf30f87f9..27109152c 100644 --- a/src/iam.ts +++ b/src/iam.ts @@ -46,7 +46,7 @@ export interface GetPolicyCallback { * @param {string} [userProject] The ID of the project which will be * billed for the request. */ -export interface SetPolicyOptions extends PreconditionOptions { +export interface SetPolicyOptions { userProject?: string; } From 7c38ef60ee0b1618bb086bf3b4029937bcdc3d72 Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 8 Aug 2022 18:34:00 +0000 Subject: [PATCH 56/58] added retries for setPolicy --- conformance-test/libraryMethods.ts | 18 +++++++++++++++++- .../test-data/retryInvocationMap.json | 1 + src/iam.ts | 6 +++++- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index bb04d8520..6289fcff6 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -12,7 +12,7 @@ // 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'; @@ -707,6 +707,22 @@ export async function iamGetPolicy(options: ConformanceTestOptions) { await options.bucket!.iam.getPolicy({requestedPolicyVersion: 1}); } +export async function iamSetPolicy(options: ConformanceTestOptions) { + let testPolicy : Policy = { + bindings: [ + { + role: 'roles/storage.admin', + members: ['serviceAccount:myotherproject@appspot.gserviceaccount.com'], + }, + ], + }; + 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(options: ConformanceTestOptions) { const permissionToTest = 'storage.buckets.delete'; await options.bucket!.iam.testPermissions(permissionToTest); diff --git a/conformance-test/test-data/retryInvocationMap.json b/conformance-test/test-data/retryInvocationMap.json index 1f936db09..63dfb0032 100644 --- a/conformance-test/test-data/retryInvocationMap.json +++ b/conformance-test/test-data/retryInvocationMap.json @@ -107,6 +107,7 @@ "iamGetPolicy" ], "storage.buckets.setIamPolicy": [ + "iamSetPolicy" ], "storage.buckets.testIamPermission": [ "iamTestPermissions" diff --git a/src/iam.ts b/src/iam.ts index 27109152c..397d8d0c4 100644 --- a/src/iam.ts +++ b/src/iam.ts @@ -345,7 +345,11 @@ class Iam { SetPolicyCallback >(optionsOrCallback, callback); - const maxRetries = 0; // We don't support ETag + let maxRetries; + if (policy.etag === undefined) { + maxRetries = 0; + } + this.request_( { method: 'PUT', From 3a8cf2344159308da2cdfcd7111173d11023f878 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Mon, 8 Aug 2022 18:35:53 +0000 Subject: [PATCH 57/58] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- conformance-test/libraryMethods.ts | 2 +- src/iam.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/conformance-test/libraryMethods.ts b/conformance-test/libraryMethods.ts index 6289fcff6..01a35fe50 100644 --- a/conformance-test/libraryMethods.ts +++ b/conformance-test/libraryMethods.ts @@ -708,7 +708,7 @@ export async function iamGetPolicy(options: ConformanceTestOptions) { } export async function iamSetPolicy(options: ConformanceTestOptions) { - let testPolicy : Policy = { + const testPolicy: Policy = { bindings: [ { role: 'roles/storage.admin', diff --git a/src/iam.ts b/src/iam.ts index 397d8d0c4..8695d671b 100644 --- a/src/iam.ts +++ b/src/iam.ts @@ -345,11 +345,11 @@ class Iam { SetPolicyCallback >(optionsOrCallback, callback); - let maxRetries; + let maxRetries; if (policy.etag === undefined) { maxRetries = 0; } - + this.request_( { method: 'PUT', From b537854cdcf89c21c971137f3969d53be891b30a Mon Sep 17 00:00:00 2001 From: Sameena Shaffeeullah Date: Mon, 8 Aug 2022 19:29:17 +0000 Subject: [PATCH 58/58] removed unused import --- src/iam.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/iam.ts b/src/iam.ts index 397d8d0c4..12d50ba98 100644 --- a/src/iam.ts +++ b/src/iam.ts @@ -22,7 +22,6 @@ import arrify = require('arrify'); import {Bucket} from './bucket'; import {normalize} from './util'; -import {PreconditionOptions} from './storage'; export interface GetPolicyOptions { userProject?: string;