From b59eb5c7fd4e9d7886f4c942d1e67150ef22fe55 Mon Sep 17 00:00:00 2001 From: Amayyas Date: Tue, 14 Jul 2026 13:35:35 +0200 Subject: [PATCH 1/3] feat(storage): support ifMetagenerationNotMatch Adds an `ifMetagenerationNotMatch` parameter to `Storage.patchBucket`, `Storage.uploadObject` and `Storage.uploadObjectFromString`, resolving the three TODOs that referenced issue #115. Google Cloud Storage reports an unsatisfied `ifMetagenerationNotMatch` precondition with a "304 Not Modified" status and an empty body, rather than with the "412 Precondition Failed" used by the `*Match` preconditions. That empty body is now translated into a new `NotModifiedException` instead of surfacing as a bare `ServiceException` whose message is "unknown error". Fixes #115 --- pkgs/google_cloud_storage/CHANGELOG.md | 4 + pkgs/google_cloud_storage/lib/src/client.dart | 129 +++++++----- .../lib/src/exceptions.dart | 18 ++ .../if_metageneration_not_match_test.dart | 198 ++++++++++++++++++ 4 files changed, 297 insertions(+), 52 deletions(-) create mode 100644 pkgs/google_cloud_storage/test/if_metageneration_not_match_test.dart diff --git a/pkgs/google_cloud_storage/CHANGELOG.md b/pkgs/google_cloud_storage/CHANGELOG.md index 5e50d32c..82e668ea 100644 --- a/pkgs/google_cloud_storage/CHANGELOG.md +++ b/pkgs/google_cloud_storage/CHANGELOG.md @@ -4,6 +4,10 @@ `Storage.listObjects`. * Add `Storage.rewriteObject`. * Add `StorageObject.rewrite`. +* Add an `ifMetagenerationNotMatch` parameter to `Storage.patchBucket`, + `Storage.uploadObject`, and `Storage.uploadObjectFromString`. +* Add `NotModifiedException`, thrown when an `ifMetagenerationNotMatch` + precondition is not satisfied. ## 0.6.2 diff --git a/pkgs/google_cloud_storage/lib/src/client.dart b/pkgs/google_cloud_storage/lib/src/client.dart index 0de06b32..4cc60143 100644 --- a/pkgs/google_cloud_storage/lib/src/client.dart +++ b/pkgs/google_cloud_storage/lib/src/client.dart @@ -486,6 +486,11 @@ final class Storage { /// value. If the metageneration does not match, a /// [PreconditionFailedException] is thrown. /// + /// If set, [ifMetagenerationNotMatch] makes updating the bucket metadata + /// conditional on whether the bucket's metageneration does *not* match the + /// provided value. If the metageneration does match, the bucket is left + /// unchanged and a [NotModifiedException] is thrown. + /// /// If set, [predefinedAcl] applies a predefined set of access controls to the /// bucket, such as `"publicRead"`. If [UniformBucketLevelAccess.enabled] is /// `true`, then setting `predefinedAcl` will result in a @@ -518,35 +523,33 @@ final class Storage { String bucket, BucketMetadataPatchBuilder metadata, { BigInt? ifMetagenerationMatch, - // TODO(https://github.com/googleapis/google-cloud-dart/issues/115): - // support ifMetagenerationNotMatch. - // - // If `ifMetagenerationNotMatch` is set, the server will respond with a 304 - // status code and an empty body. This will cause `buckets.patch` to throw - // `TypeError` during JSON deserialization. + BigInt? ifMetagenerationNotMatch, String? predefinedAcl, String? predefinedDefaultObjectAcl, String? projection, String? userProject, RetryRunner retry = defaultRetry, - }) => retry.run(() async { - final serviceClient = await _serviceClient; - final url = _requestUrl( - ['storage', 'v1', 'b', bucket], - { - 'ifMetagenerationMatch': ?ifMetagenerationMatch?.toString(), - 'predefinedAcl': ?predefinedAcl, - 'predefinedDefaultObjectAcl': ?predefinedDefaultObjectAcl, - 'projection': ?projection, - 'userProject': ?userProject, - }, - ); - final j = await serviceClient.patch( - url, - body: BucketMetadataPatchBuilderJsonEncodable(metadata), - ); - return bucketMetadataFromJson(j as Map); - }, isIdempotent: ifMetagenerationMatch != null); + }) => _translateNotModified( + () => retry.run(() async { + final serviceClient = await _serviceClient; + final url = _requestUrl( + ['storage', 'v1', 'b', bucket], + { + 'ifMetagenerationMatch': ?ifMetagenerationMatch?.toString(), + 'ifMetagenerationNotMatch': ?ifMetagenerationNotMatch?.toString(), + 'predefinedAcl': ?predefinedAcl, + 'predefinedDefaultObjectAcl': ?predefinedDefaultObjectAcl, + 'projection': ?projection, + 'userProject': ?userProject, + }, + ); + final j = await serviceClient.patch( + url, + body: BucketMetadataPatchBuilderJsonEncodable(metadata), + ); + return bucketMetadataFromJson(j as Map); + }, isIdempotent: ifMetagenerationMatch != null), + ); /// Patches an Access Control List (ACL) entry on the specified /// [Google Cloud Storage bucket]. @@ -1364,6 +1367,11 @@ final class Storage { /// generation does not match, a [PreconditionFailedException] is thrown. /// A value of [BigInt.zero] indicates that the object must not already exist. /// + /// If set, [ifMetagenerationNotMatch] makes updating the object content + /// conditional on whether the object's metageneration does *not* match the + /// provided value. If the metageneration does match, the object is left + /// unchanged and a [NotModifiedException] is thrown. + /// /// If set, `predefinedAcl` applies a predefined set of access controls to the /// object, such as `"publicRead"`. If [UniformBucketLevelAccess.enabled] is /// `true`, then setting `predefinedAcl` will result in a @@ -1397,34 +1405,32 @@ final class Storage { List content, { ObjectMetadata? metadata, BigInt? ifGenerationMatch, - // TODO(https://github.com/googleapis/google-cloud-dart/issues/115): - // support ifMetagenerationNotMatch. - // - // If `ifMetagenerationNotMatch` is set, the server will respond with a 304 - // status code and an empty body. This will cause `objects.insert` to throw - // `TypeError` during JSON deserialization. + BigInt? ifMetagenerationNotMatch, String? predefinedAcl, String? projection, String? userProject, RetryRunner retry = defaultRetry, - }) => retry.run( - () async => uploadFile( - await _httpClient, - _requestUrl( - ['upload', 'storage', 'v1', 'b', bucket, 'o'], - { - 'uploadType': 'multipart', - 'name': name, - 'ifGenerationMatch': ?ifGenerationMatch?.toString(), - 'predefinedAcl': ?predefinedAcl, - 'projection': ?projection, - 'userProject': ?userProject, - }, + }) => _translateNotModified( + () => retry.run( + () async => uploadFile( + await _httpClient, + _requestUrl( + ['upload', 'storage', 'v1', 'b', bucket, 'o'], + { + 'uploadType': 'multipart', + 'name': name, + 'ifGenerationMatch': ?ifGenerationMatch?.toString(), + 'ifMetagenerationNotMatch': ?ifMetagenerationNotMatch?.toString(), + 'predefinedAcl': ?predefinedAcl, + 'projection': ?projection, + 'userProject': ?userProject, + }, + ), + content, + metadata: metadata, ), - content, - metadata: metadata, + isIdempotent: ifGenerationMatch != null, ), - isIdempotent: ifGenerationMatch != null, ); /// Creates or updates the content of a [Google Cloud Storage object][] using @@ -1492,6 +1498,11 @@ final class Storage { /// generation does not match, a [PreconditionFailedException] is thrown. /// A value of `0` indicates that the object must not already exist. /// + /// If set, [ifMetagenerationNotMatch] makes updating the object content + /// conditional on whether the object's metageneration does *not* match the + /// provided value. If the metageneration does match, the object is left + /// unchanged and a [NotModifiedException] is thrown. + /// /// If set, `predefinedAcl` applies a predefined set of access controls to the /// object, such as `"publicRead"`. If [UniformBucketLevelAccess.enabled] is /// `true`, then setting `predefinedAcl` will result in a @@ -1525,12 +1536,7 @@ final class Storage { String content, { ObjectMetadata? metadata, BigInt? ifGenerationMatch, - // TODO(https://github.com/googleapis/google-cloud-dart/issues/115): - // support ifMetagenerationNotMatch. - // - // If `ifMetagenerationNotMatch` is set, the server will respond with a 304 - // status code and an empty body. This will cause `objects.insert` to throw - // `TypeError` during JSON deserialization. + BigInt? ifMetagenerationNotMatch, String? predefinedAcl, String? projection, String? userProject, @@ -1547,6 +1553,7 @@ final class Storage { utf8.encode(content), metadata: md, ifGenerationMatch: ifGenerationMatch, + ifMetagenerationNotMatch: ifMetagenerationNotMatch, predefinedAcl: predefinedAcl, projection: projection, userProject: userProject, @@ -1554,3 +1561,21 @@ final class Storage { ); } } + +/// Runs [operation], translating the "304 Not Modified" response that Google +/// Cloud Storage returns for an unsatisfied `ifMetagenerationNotMatch` +/// precondition into a [NotModifiedException]. +/// +/// A 304 response has an empty body, so it cannot be deserialized into the +/// metadata that the operation would otherwise return. +Future _translateNotModified(Future Function() operation) async { + try { + return await operation(); + } on ServiceException catch (e) { + if (e.statusCode != 304) rethrow; + throw NotModifiedException( + 'The operation was not performed because the ' + '"ifMetagenerationNotMatch" precondition was not satisfied.', + ); + } +} diff --git a/pkgs/google_cloud_storage/lib/src/exceptions.dart b/pkgs/google_cloud_storage/lib/src/exceptions.dart index 5f9b605b..be14644e 100644 --- a/pkgs/google_cloud_storage/lib/src/exceptions.dart +++ b/pkgs/google_cloud_storage/lib/src/exceptions.dart @@ -22,3 +22,21 @@ class ChecksumValidationException implements Exception { @override String toString() => 'ChecksumValidationException: $message'; } + +/// Exception thrown when an `ifMetagenerationNotMatch` precondition is not +/// satisfied and the requested operation was therefore not performed. +/// +/// Google Cloud Storage signals this by responding with a "304 Not Modified" +/// status and an empty body, rather than with the "412 Precondition Failed" +/// status used for the `ifGenerationMatch` and `ifMetagenerationMatch` +/// preconditions. +/// +/// See [request preconditions](https://cloud.google.com/storage/docs/request-preconditions). +class NotModifiedException implements Exception { + final String message; + + NotModifiedException(this.message); + + @override + String toString() => 'NotModifiedException: $message'; +} diff --git a/pkgs/google_cloud_storage/test/if_metageneration_not_match_test.dart b/pkgs/google_cloud_storage/test/if_metageneration_not_match_test.dart new file mode 100644 index 00000000..d5e69b67 --- /dev/null +++ b/pkgs/google_cloud_storage/test/if_metageneration_not_match_test.dart @@ -0,0 +1,198 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import 'package:google_cloud_storage/google_cloud_storage.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:test/test.dart'; + +import 'test_utils.dart'; + +/// Responds as Google Cloud Storage does when an `ifMetagenerationNotMatch` +/// precondition is not satisfied: a "304 Not Modified" with an empty body. +MockClient _notModifiedClient(void Function(http.Request) onRequest) => + MockClient((request) async { + onRequest(request); + return http.Response('', 304); + }); + +void main() { + group('ifMetagenerationNotMatch', () { + group('storage-testbench', tags: ['storage-testbench'], () { + late Storage storage; + + setUp(() { + (_, storage) = createStorageTestbenchClient(); + }); + + tearDown(() => storage.close()); + + test('patchBucket throws when the metageneration matches', () async { + final bucketName = bucketNameWithTearDown(storage, 'imnm_pch_bkt'); + final created = await storage.createBucket( + BucketMetadata(name: bucketName), + ); + + await expectLater( + storage.patchBucket( + bucketName, + BucketMetadataPatchBuilder()..labels = {'color': 'red'}, + ifMetagenerationNotMatch: created.metageneration, + ), + throwsA(isA()), + ); + + // The bucket must be left untouched. + final actual = await storage.bucketMetadata(bucketName); + expect(actual.labels, anyOf(isNull, isEmpty)); + expect(actual.metageneration, created.metageneration); + }); + + test('patchBucket succeeds when the metageneration differs', () async { + final bucketName = bucketNameWithTearDown(storage, 'imnm_pch_bkt_ok'); + final created = await storage.createBucket( + BucketMetadata(name: bucketName), + ); + + final actual = await storage.patchBucket( + bucketName, + BucketMetadataPatchBuilder()..labels = {'color': 'red'}, + ifMetagenerationNotMatch: created.metageneration! + BigInt.one, + ); + + expect(actual.labels, containsPair('color', 'red')); + }); + + test('uploadObject throws when the metageneration matches', () async { + final bucketName = bucketNameWithTearDown(storage, 'imnm_upl_obj'); + await storage.createBucket(BucketMetadata(name: bucketName)); + + final created = await storage.uploadObject(bucketName, 'file.txt', [ + 1, + 2, + 3, + ]); + + await expectLater( + storage.uploadObject(bucketName, 'file.txt', [ + 4, + 5, + 6, + ], ifMetagenerationNotMatch: created.metageneration), + throwsA(isA()), + ); + + // The content must be left untouched. + final content = await storage.downloadObject(bucketName, 'file.txt'); + expect(content, [1, 2, 3]); + }); + }); + + test('patchBucket sends the query parameter', () async { + late Uri requestUrl; + final storage = Storage( + client: _notModifiedClient((request) => requestUrl = request.url), + projectId: 'fake project', + ); + + await expectLater( + storage.patchBucket( + 'bucket', + BucketMetadataPatchBuilder()..labels = {'color': 'red'}, + ifMetagenerationNotMatch: BigInt.two, + ), + throwsA(isA()), + ); + + expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); + }); + + test('uploadObject sends the query parameter', () async { + late Uri requestUrl; + final storage = Storage( + client: _notModifiedClient((request) => requestUrl = request.url), + projectId: 'fake project', + ); + + await expectLater( + storage.uploadObject('bucket', 'file.txt', [ + 1, + 2, + 3, + ], ifMetagenerationNotMatch: BigInt.two), + throwsA(isA()), + ); + + expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); + }); + + test('uploadObjectFromString sends the query parameter', () async { + late Uri requestUrl; + final storage = Storage( + client: _notModifiedClient((request) => requestUrl = request.url), + projectId: 'fake project', + ); + + await expectLater( + storage.uploadObjectFromString( + 'bucket', + 'file.txt', + 'hello', + ifMetagenerationNotMatch: BigInt.two, + ), + throwsA(isA()), + ); + + expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); + }); + + test('a 304 response is reported as a NotModifiedException', () async { + final storage = Storage( + client: _notModifiedClient((_) {}), + projectId: 'fake project', + ); + + await expectLater( + storage.patchBucket( + 'bucket', + BucketMetadataPatchBuilder()..labels = {'color': 'red'}, + ifMetagenerationNotMatch: BigInt.one, + ), + throwsA( + isA().having( + (e) => e.toString(), + 'toString()', + contains('ifMetagenerationNotMatch'), + ), + ), + ); + }); + + test('other error responses are not translated', () async { + final storage = Storage( + client: MockClient((_) async => http.Response('{}', 412)), + projectId: 'fake project', + ); + + await expectLater( + storage.patchBucket( + 'bucket', + BucketMetadataPatchBuilder()..labels = {'color': 'red'}, + ifMetagenerationNotMatch: BigInt.one, + ), + throwsA(isA()), + ); + }); + }); +} From 6e378905676b5ea9ccfeb9f4040ebd3fa6628ce4 Mon Sep 17 00:00:00 2001 From: Amayyas Date: Fri, 17 Jul 2026 13:29:32 +0200 Subject: [PATCH 2/3] refactor: decode 304 into NotModifiedException in google_cloud_rpc Addresses review feedback: - Move the "304 Not Modified" decoding into `ServiceException._fromDecodedResponse`, alongside the other status code mappings, so every API benefits rather than just storage. This drops the `_translateNotModified` wrapper and the storage-local exception class. - A 304 never carries a body, so `fromHttpResponse` now uses a descriptive message for it instead of the generic "unknown error". - Move the `ifMetagenerationNotMatch` tests into the test files for the methods that accept the argument, and cover the status mapping itself in `google_cloud_rpc`'s `exceptions_test.dart`. Also moves the CHANGELOG entry to a new 0.6.4-wip section, since 0.6.3 has been published. --- .../google_cloud_rpc/lib/src/exceptions.dart | 28 ++- .../test/exceptions_test.dart | 16 ++ pkgs/google_cloud_storage/CHANGELOG.md | 10 +- pkgs/google_cloud_storage/lib/src/client.dart | 94 ++++----- .../lib/src/exceptions.dart | 18 -- pkgs/google_cloud_storage/pubspec.yaml | 2 +- .../if_metageneration_not_match_test.dart | 198 ------------------ .../test/patch_bucket_test.dart | 79 +++++++ .../test/upload_object_from_string_test.dart | 24 +++ .../test/upload_object_test.dart | 65 ++++++ 10 files changed, 254 insertions(+), 280 deletions(-) delete mode 100644 pkgs/google_cloud_storage/test/if_metageneration_not_match_test.dart diff --git a/generated/google_cloud_rpc/lib/src/exceptions.dart b/generated/google_cloud_rpc/lib/src/exceptions.dart index 4f16304a..3c50486f 100644 --- a/generated/google_cloud_rpc/lib/src/exceptions.dart +++ b/generated/google_cloud_rpc/lib/src/exceptions.dart @@ -64,6 +64,12 @@ final class ServiceException implements Exception { String? responseBody, Status? status, }) => switch (response.statusCode) { + 304 => NotModifiedException( + message, + response: response, + responseBody: responseBody, + status: status, + ), 400 => BadRequestException( message, response: response, @@ -183,7 +189,11 @@ final class ServiceException implements Exception { ) { if (responseBody == null || responseBody.isEmpty) { return ServiceException._fromDecodedResponse( - 'unknown error', + // A "304 Not Modified" response never has a body, so there is no + // server-provided message to describe it. + response.statusCode == 304 + ? 'the resource was not modified' + : 'unknown error', response: response, responseBody: responseBody, ); @@ -231,6 +241,22 @@ final class ServiceException implements Exception { String toString() => '$_name: $message'; } +/// Exception thrown when the server returns a "304 Not Modified" response. +/// +/// This indicates that the requested operation was not performed because a +/// precondition, such as `ifMetagenerationNotMatch`, was not satisfied. +final class NotModifiedException extends ServiceException { + NotModifiedException( + super.message, { + required super.response, + required super.responseBody, + super.status, + }) : super(statusCode: 304); + + @override + String get _name => 'NotModifiedException'; +} + /// Exception thrown when the server returns a "400 Bad Request" response. final class BadRequestException extends ServiceException { BadRequestException( diff --git a/generated/google_cloud_rpc/test/exceptions_test.dart b/generated/google_cloud_rpc/test/exceptions_test.dart index e2b4700a..a267127f 100644 --- a/generated/google_cloud_rpc/test/exceptions_test.dart +++ b/generated/google_cloud_rpc/test/exceptions_test.dart @@ -147,6 +147,22 @@ void main() { expect(e.toString(), 'ServiceException: internal error'); }); + // A "304 Not Modified" response never has a body. + test('empty body 304', () { + final response = http.Response('', 304); + final e = ServiceException.fromHttpResponse(response, ''); + expect(e, isA()); + expect(e.message, 'the resource was not modified'); + expect(e.statusCode, 304); + expect(e.response, response); + expect(e.responseBody, ''); + expect(e.status, isNull); + expect( + e.toString(), + 'NotModifiedException: the resource was not modified', + ); + }); + test('valid error 401', () { final response = http.Response( '{"error": {"message": "unauthorized"}}', diff --git a/pkgs/google_cloud_storage/CHANGELOG.md b/pkgs/google_cloud_storage/CHANGELOG.md index 7e8c1d77..21317c0c 100644 --- a/pkgs/google_cloud_storage/CHANGELOG.md +++ b/pkgs/google_cloud_storage/CHANGELOG.md @@ -1,13 +1,15 @@ +## 0.6.4-wip + +* Add an `ifMetagenerationNotMatch` parameter to `Storage.patchBucket`, + `Storage.uploadObject`, and `Storage.uploadObjectFromString`. If the + precondition is not satisfied, a `NotModifiedException` is thrown. + ## 0.6.3 * Add `prefix`, `delimiter`, and `includeTrailingDelimiter` parameters to `Storage.listObjects`. * Add `Storage.rewriteObject`. * Add `StorageObject.rewrite`. -* Add an `ifMetagenerationNotMatch` parameter to `Storage.patchBucket`, - `Storage.uploadObject`, and `Storage.uploadObjectFromString`. -* Add `NotModifiedException`, thrown when an `ifMetagenerationNotMatch` - precondition is not satisfied. ## 0.6.2 diff --git a/pkgs/google_cloud_storage/lib/src/client.dart b/pkgs/google_cloud_storage/lib/src/client.dart index 4cc60143..56526f7f 100644 --- a/pkgs/google_cloud_storage/lib/src/client.dart +++ b/pkgs/google_cloud_storage/lib/src/client.dart @@ -529,27 +529,25 @@ final class Storage { String? projection, String? userProject, RetryRunner retry = defaultRetry, - }) => _translateNotModified( - () => retry.run(() async { - final serviceClient = await _serviceClient; - final url = _requestUrl( - ['storage', 'v1', 'b', bucket], - { - 'ifMetagenerationMatch': ?ifMetagenerationMatch?.toString(), - 'ifMetagenerationNotMatch': ?ifMetagenerationNotMatch?.toString(), - 'predefinedAcl': ?predefinedAcl, - 'predefinedDefaultObjectAcl': ?predefinedDefaultObjectAcl, - 'projection': ?projection, - 'userProject': ?userProject, - }, - ); - final j = await serviceClient.patch( - url, - body: BucketMetadataPatchBuilderJsonEncodable(metadata), - ); - return bucketMetadataFromJson(j as Map); - }, isIdempotent: ifMetagenerationMatch != null), - ); + }) => retry.run(() async { + final serviceClient = await _serviceClient; + final url = _requestUrl( + ['storage', 'v1', 'b', bucket], + { + 'ifMetagenerationMatch': ?ifMetagenerationMatch?.toString(), + 'ifMetagenerationNotMatch': ?ifMetagenerationNotMatch?.toString(), + 'predefinedAcl': ?predefinedAcl, + 'predefinedDefaultObjectAcl': ?predefinedDefaultObjectAcl, + 'projection': ?projection, + 'userProject': ?userProject, + }, + ); + final j = await serviceClient.patch( + url, + body: BucketMetadataPatchBuilderJsonEncodable(metadata), + ); + return bucketMetadataFromJson(j as Map); + }, isIdempotent: ifMetagenerationMatch != null); /// Patches an Access Control List (ACL) entry on the specified /// [Google Cloud Storage bucket]. @@ -1410,27 +1408,25 @@ final class Storage { String? projection, String? userProject, RetryRunner retry = defaultRetry, - }) => _translateNotModified( - () => retry.run( - () async => uploadFile( - await _httpClient, - _requestUrl( - ['upload', 'storage', 'v1', 'b', bucket, 'o'], - { - 'uploadType': 'multipart', - 'name': name, - 'ifGenerationMatch': ?ifGenerationMatch?.toString(), - 'ifMetagenerationNotMatch': ?ifMetagenerationNotMatch?.toString(), - 'predefinedAcl': ?predefinedAcl, - 'projection': ?projection, - 'userProject': ?userProject, - }, - ), - content, - metadata: metadata, + }) => retry.run( + () async => uploadFile( + await _httpClient, + _requestUrl( + ['upload', 'storage', 'v1', 'b', bucket, 'o'], + { + 'uploadType': 'multipart', + 'name': name, + 'ifGenerationMatch': ?ifGenerationMatch?.toString(), + 'ifMetagenerationNotMatch': ?ifMetagenerationNotMatch?.toString(), + 'predefinedAcl': ?predefinedAcl, + 'projection': ?projection, + 'userProject': ?userProject, + }, ), - isIdempotent: ifGenerationMatch != null, + content, + metadata: metadata, ), + isIdempotent: ifGenerationMatch != null, ); /// Creates or updates the content of a [Google Cloud Storage object][] using @@ -1561,21 +1557,3 @@ final class Storage { ); } } - -/// Runs [operation], translating the "304 Not Modified" response that Google -/// Cloud Storage returns for an unsatisfied `ifMetagenerationNotMatch` -/// precondition into a [NotModifiedException]. -/// -/// A 304 response has an empty body, so it cannot be deserialized into the -/// metadata that the operation would otherwise return. -Future _translateNotModified(Future Function() operation) async { - try { - return await operation(); - } on ServiceException catch (e) { - if (e.statusCode != 304) rethrow; - throw NotModifiedException( - 'The operation was not performed because the ' - '"ifMetagenerationNotMatch" precondition was not satisfied.', - ); - } -} diff --git a/pkgs/google_cloud_storage/lib/src/exceptions.dart b/pkgs/google_cloud_storage/lib/src/exceptions.dart index be14644e..5f9b605b 100644 --- a/pkgs/google_cloud_storage/lib/src/exceptions.dart +++ b/pkgs/google_cloud_storage/lib/src/exceptions.dart @@ -22,21 +22,3 @@ class ChecksumValidationException implements Exception { @override String toString() => 'ChecksumValidationException: $message'; } - -/// Exception thrown when an `ifMetagenerationNotMatch` precondition is not -/// satisfied and the requested operation was therefore not performed. -/// -/// Google Cloud Storage signals this by responding with a "304 Not Modified" -/// status and an empty body, rather than with the "412 Precondition Failed" -/// status used for the `ifGenerationMatch` and `ifMetagenerationMatch` -/// preconditions. -/// -/// See [request preconditions](https://cloud.google.com/storage/docs/request-preconditions). -class NotModifiedException implements Exception { - final String message; - - NotModifiedException(this.message); - - @override - String toString() => 'NotModifiedException: $message'; -} diff --git a/pkgs/google_cloud_storage/pubspec.yaml b/pkgs/google_cloud_storage/pubspec.yaml index d6df5a98..6ad4c907 100644 --- a/pkgs/google_cloud_storage/pubspec.yaml +++ b/pkgs/google_cloud_storage/pubspec.yaml @@ -16,7 +16,7 @@ name: google_cloud_storage description: >- A client for Google Cloud Storage, a managed service for storing, archiving, and serving unstructured data of any size. -version: 0.6.3 +version: 0.6.4-wip repository: https://github.com/googleapis/google-cloud-dart/tree/main/pkgs/google_cloud_storage environment: diff --git a/pkgs/google_cloud_storage/test/if_metageneration_not_match_test.dart b/pkgs/google_cloud_storage/test/if_metageneration_not_match_test.dart deleted file mode 100644 index d5e69b67..00000000 --- a/pkgs/google_cloud_storage/test/if_metageneration_not_match_test.dart +++ /dev/null @@ -1,198 +0,0 @@ -// Copyright 2026 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import 'package:google_cloud_storage/google_cloud_storage.dart'; -import 'package:http/http.dart' as http; -import 'package:http/testing.dart'; -import 'package:test/test.dart'; - -import 'test_utils.dart'; - -/// Responds as Google Cloud Storage does when an `ifMetagenerationNotMatch` -/// precondition is not satisfied: a "304 Not Modified" with an empty body. -MockClient _notModifiedClient(void Function(http.Request) onRequest) => - MockClient((request) async { - onRequest(request); - return http.Response('', 304); - }); - -void main() { - group('ifMetagenerationNotMatch', () { - group('storage-testbench', tags: ['storage-testbench'], () { - late Storage storage; - - setUp(() { - (_, storage) = createStorageTestbenchClient(); - }); - - tearDown(() => storage.close()); - - test('patchBucket throws when the metageneration matches', () async { - final bucketName = bucketNameWithTearDown(storage, 'imnm_pch_bkt'); - final created = await storage.createBucket( - BucketMetadata(name: bucketName), - ); - - await expectLater( - storage.patchBucket( - bucketName, - BucketMetadataPatchBuilder()..labels = {'color': 'red'}, - ifMetagenerationNotMatch: created.metageneration, - ), - throwsA(isA()), - ); - - // The bucket must be left untouched. - final actual = await storage.bucketMetadata(bucketName); - expect(actual.labels, anyOf(isNull, isEmpty)); - expect(actual.metageneration, created.metageneration); - }); - - test('patchBucket succeeds when the metageneration differs', () async { - final bucketName = bucketNameWithTearDown(storage, 'imnm_pch_bkt_ok'); - final created = await storage.createBucket( - BucketMetadata(name: bucketName), - ); - - final actual = await storage.patchBucket( - bucketName, - BucketMetadataPatchBuilder()..labels = {'color': 'red'}, - ifMetagenerationNotMatch: created.metageneration! + BigInt.one, - ); - - expect(actual.labels, containsPair('color', 'red')); - }); - - test('uploadObject throws when the metageneration matches', () async { - final bucketName = bucketNameWithTearDown(storage, 'imnm_upl_obj'); - await storage.createBucket(BucketMetadata(name: bucketName)); - - final created = await storage.uploadObject(bucketName, 'file.txt', [ - 1, - 2, - 3, - ]); - - await expectLater( - storage.uploadObject(bucketName, 'file.txt', [ - 4, - 5, - 6, - ], ifMetagenerationNotMatch: created.metageneration), - throwsA(isA()), - ); - - // The content must be left untouched. - final content = await storage.downloadObject(bucketName, 'file.txt'); - expect(content, [1, 2, 3]); - }); - }); - - test('patchBucket sends the query parameter', () async { - late Uri requestUrl; - final storage = Storage( - client: _notModifiedClient((request) => requestUrl = request.url), - projectId: 'fake project', - ); - - await expectLater( - storage.patchBucket( - 'bucket', - BucketMetadataPatchBuilder()..labels = {'color': 'red'}, - ifMetagenerationNotMatch: BigInt.two, - ), - throwsA(isA()), - ); - - expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); - }); - - test('uploadObject sends the query parameter', () async { - late Uri requestUrl; - final storage = Storage( - client: _notModifiedClient((request) => requestUrl = request.url), - projectId: 'fake project', - ); - - await expectLater( - storage.uploadObject('bucket', 'file.txt', [ - 1, - 2, - 3, - ], ifMetagenerationNotMatch: BigInt.two), - throwsA(isA()), - ); - - expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); - }); - - test('uploadObjectFromString sends the query parameter', () async { - late Uri requestUrl; - final storage = Storage( - client: _notModifiedClient((request) => requestUrl = request.url), - projectId: 'fake project', - ); - - await expectLater( - storage.uploadObjectFromString( - 'bucket', - 'file.txt', - 'hello', - ifMetagenerationNotMatch: BigInt.two, - ), - throwsA(isA()), - ); - - expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); - }); - - test('a 304 response is reported as a NotModifiedException', () async { - final storage = Storage( - client: _notModifiedClient((_) {}), - projectId: 'fake project', - ); - - await expectLater( - storage.patchBucket( - 'bucket', - BucketMetadataPatchBuilder()..labels = {'color': 'red'}, - ifMetagenerationNotMatch: BigInt.one, - ), - throwsA( - isA().having( - (e) => e.toString(), - 'toString()', - contains('ifMetagenerationNotMatch'), - ), - ), - ); - }); - - test('other error responses are not translated', () async { - final storage = Storage( - client: MockClient((_) async => http.Response('{}', 412)), - projectId: 'fake project', - ); - - await expectLater( - storage.patchBucket( - 'bucket', - BucketMetadataPatchBuilder()..labels = {'color': 'red'}, - ifMetagenerationNotMatch: BigInt.one, - ), - throwsA(isA()), - ); - }); - }); -} diff --git a/pkgs/google_cloud_storage/test/patch_bucket_test.dart b/pkgs/google_cloud_storage/test/patch_bucket_test.dart index 835f1dd0..c0c3b430 100644 --- a/pkgs/google_cloud_storage/test/patch_bucket_test.dart +++ b/pkgs/google_cloud_storage/test/patch_bucket_test.dart @@ -916,6 +916,85 @@ void main() async { }); }); + group('storage-testbench', tags: ['storage-testbench'], () { + late Storage testbenchStorage; + + setUp(() { + (_, testbenchStorage) = createStorageTestbenchClient(); + }); + + tearDown(() => testbenchStorage.close()); + + test( + 'ifMetagenerationNotMatch throws when the metageneration matches', + () async { + final bucketName = bucketNameWithTearDown( + testbenchStorage, + 'pch_bkt_imnm', + ); + final created = await testbenchStorage.createBucket( + BucketMetadata(name: bucketName), + ); + + await expectLater( + testbenchStorage.patchBucket( + bucketName, + BucketMetadataPatchBuilder()..labels = {'color': 'red'}, + ifMetagenerationNotMatch: created.metageneration, + ), + throwsA(isA()), + ); + + // The bucket must be left unchanged. + final actual = await testbenchStorage.bucketMetadata(bucketName); + expect(actual.labels, anyOf(isNull, isEmpty)); + expect(actual.metageneration, created.metageneration); + }, + ); + + test( + 'ifMetagenerationNotMatch succeeds when the metageneration differs', + () async { + final bucketName = bucketNameWithTearDown( + testbenchStorage, + 'pch_bkt_imnm_ok', + ); + final created = await testbenchStorage.createBucket( + BucketMetadata(name: bucketName), + ); + + final actual = await testbenchStorage.patchBucket( + bucketName, + BucketMetadataPatchBuilder()..labels = {'color': 'red'}, + ifMetagenerationNotMatch: created.metageneration! + BigInt.one, + ); + + expect(actual.labels, containsPair('color', 'red')); + }, + ); + }); + + test('ifMetagenerationNotMatch is sent as a query parameter', () async { + late Uri requestUrl; + final mockClient = MockClient((request) async { + requestUrl = request.url; + return http.Response('', 304); + }); + + final storage = Storage(client: mockClient, projectId: 'fake project'); + + await expectLater( + storage.patchBucket( + 'bucket', + BucketMetadataPatchBuilder()..labels = {'color': 'red'}, + ifMetagenerationNotMatch: BigInt.two, + ), + throwsA(isA()), + ); + + expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); + }); + test('idempotent transport failure', () async { var count = 0; final mockClient = MockClient((request) async { diff --git a/pkgs/google_cloud_storage/test/upload_object_from_string_test.dart b/pkgs/google_cloud_storage/test/upload_object_from_string_test.dart index d160fa27..0f4559f4 100644 --- a/pkgs/google_cloud_storage/test/upload_object_from_string_test.dart +++ b/pkgs/google_cloud_storage/test/upload_object_from_string_test.dart @@ -15,6 +15,8 @@ import 'dart:convert'; import 'package:google_cloud_storage/google_cloud_storage.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; import 'package:test/test.dart'; import 'test_utils.dart'; @@ -122,5 +124,27 @@ void main() { uploadObjectFromStringTest(() => storage, createBucketWithTearDown); }); + + test('ifMetagenerationNotMatch is sent as a query parameter', () async { + late Uri requestUrl; + final mockClient = MockClient((request) async { + requestUrl = request.url; + return http.Response('', 304); + }); + + final storage = Storage(client: mockClient, projectId: 'fake project'); + + await expectLater( + storage.uploadObjectFromString( + 'bucket', + 'object', + 'Hello, World!', + ifMetagenerationNotMatch: BigInt.two, + ), + throwsA(isA()), + ); + + expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); + }); }); } diff --git a/pkgs/google_cloud_storage/test/upload_object_test.dart b/pkgs/google_cloud_storage/test/upload_object_test.dart index af46ca09..b9794ecb 100644 --- a/pkgs/google_cloud_storage/test/upload_object_test.dart +++ b/pkgs/google_cloud_storage/test/upload_object_test.dart @@ -289,6 +289,71 @@ void main() async { }); }); + group('storage-testbench', tags: ['storage-testbench'], () { + late Storage testbenchStorage; + + setUp(() { + (_, testbenchStorage) = createStorageTestbenchClient(); + }); + + tearDown(() => testbenchStorage.close()); + + test( + 'ifMetagenerationNotMatch throws when the metageneration matches', + () async { + final bucketName = bucketNameWithTearDown( + testbenchStorage, + 'upl_obj_imnm', + ); + await testbenchStorage.createBucket(BucketMetadata(name: bucketName)); + + final created = await testbenchStorage.uploadObject( + bucketName, + 'object1', + utf8.encode('Hello World!'), + ); + + await expectLater( + testbenchStorage.uploadObject( + bucketName, + 'object1', + utf8.encode('Goodbye World!'), + ifMetagenerationNotMatch: created.metageneration, + ), + throwsA(isA()), + ); + + // The object content must be left unchanged. + final content = await testbenchStorage.downloadObject( + bucketName, + 'object1', + ); + expect(utf8.decode(content), 'Hello World!'); + }, + ); + }); + + test('ifMetagenerationNotMatch is sent as a query parameter', () async { + late Uri requestUrl; + final mockClient = MockClient((request) async { + requestUrl = request.url; + return http.Response('', 304); + }); + + final storage = Storage(client: mockClient, projectId: 'fake project'); + + await expectLater( + storage.uploadObject('bucket', 'object', [ + 1, + 2, + 3, + ], ifMetagenerationNotMatch: BigInt.two), + throwsA(isA()), + ); + + expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); + }); + test('idempotent transport failure', () async { final responses = <(String, Future Function(http.Request))>[ From 95315fa6352a755080b865388f561f3b8121aaa1 Mon Sep 17 00:00:00 2001 From: Amayyas Date: Fri, 17 Jul 2026 22:01:12 +0200 Subject: [PATCH 3/3] =?UTF-8?q?test:=20address=20review=20=E2=80=94=20pref?= =?UTF-8?q?er=20integration=20tests,=20tidy=20rpc=20exception?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move the `ifMetagenerationNotMatch` tests into the existing `google-cloud` groups so they run as integration tests against a real project, matching the testing philosophy of this package, and drop the MockClient-based unit tests. - Drop the storage-specific sentence from `NotModifiedException`'s doc comment in `google_cloud_rpc`, since that package is generic. --- .../google_cloud_rpc/lib/src/exceptions.dart | 3 -- .../test/patch_bucket_test.dart | 51 +++---------------- .../test/upload_object_from_string_test.dart | 24 --------- .../test/upload_object_test.dart | 47 +++-------------- 4 files changed, 13 insertions(+), 112 deletions(-) diff --git a/generated/google_cloud_rpc/lib/src/exceptions.dart b/generated/google_cloud_rpc/lib/src/exceptions.dart index 3c50486f..6cf1c35e 100644 --- a/generated/google_cloud_rpc/lib/src/exceptions.dart +++ b/generated/google_cloud_rpc/lib/src/exceptions.dart @@ -242,9 +242,6 @@ final class ServiceException implements Exception { } /// Exception thrown when the server returns a "304 Not Modified" response. -/// -/// This indicates that the requested operation was not performed because a -/// precondition, such as `ifMetagenerationNotMatch`, was not satisfied. final class NotModifiedException extends ServiceException { NotModifiedException( super.message, { diff --git a/pkgs/google_cloud_storage/test/patch_bucket_test.dart b/pkgs/google_cloud_storage/test/patch_bucket_test.dart index c0c3b430..98926daf 100644 --- a/pkgs/google_cloud_storage/test/patch_bucket_test.dart +++ b/pkgs/google_cloud_storage/test/patch_bucket_test.dart @@ -914,30 +914,17 @@ void main() async { ); expect(actualMetadata.metageneration, BigInt.from(2)); }); - }); - - group('storage-testbench', tags: ['storage-testbench'], () { - late Storage testbenchStorage; - - setUp(() { - (_, testbenchStorage) = createStorageTestbenchClient(); - }); - - tearDown(() => testbenchStorage.close()); test( 'ifMetagenerationNotMatch throws when the metageneration matches', () async { - final bucketName = bucketNameWithTearDown( - testbenchStorage, - 'pch_bkt_imnm', - ); - final created = await testbenchStorage.createBucket( + final bucketName = bucketNameWithTearDown(storage, 'pch_bkt_imnm'); + final created = await storage.createBucket( BucketMetadata(name: bucketName), ); await expectLater( - testbenchStorage.patchBucket( + storage.patchBucket( bucketName, BucketMetadataPatchBuilder()..labels = {'color': 'red'}, ifMetagenerationNotMatch: created.metageneration, @@ -946,7 +933,7 @@ void main() async { ); // The bucket must be left unchanged. - final actual = await testbenchStorage.bucketMetadata(bucketName); + final actual = await storage.bucketMetadata(bucketName); expect(actual.labels, anyOf(isNull, isEmpty)); expect(actual.metageneration, created.metageneration); }, @@ -955,15 +942,12 @@ void main() async { test( 'ifMetagenerationNotMatch succeeds when the metageneration differs', () async { - final bucketName = bucketNameWithTearDown( - testbenchStorage, - 'pch_bkt_imnm_ok', - ); - final created = await testbenchStorage.createBucket( + final bucketName = bucketNameWithTearDown(storage, 'pch_bkt_imnm_ok'); + final created = await storage.createBucket( BucketMetadata(name: bucketName), ); - final actual = await testbenchStorage.patchBucket( + final actual = await storage.patchBucket( bucketName, BucketMetadataPatchBuilder()..labels = {'color': 'red'}, ifMetagenerationNotMatch: created.metageneration! + BigInt.one, @@ -974,27 +958,6 @@ void main() async { ); }); - test('ifMetagenerationNotMatch is sent as a query parameter', () async { - late Uri requestUrl; - final mockClient = MockClient((request) async { - requestUrl = request.url; - return http.Response('', 304); - }); - - final storage = Storage(client: mockClient, projectId: 'fake project'); - - await expectLater( - storage.patchBucket( - 'bucket', - BucketMetadataPatchBuilder()..labels = {'color': 'red'}, - ifMetagenerationNotMatch: BigInt.two, - ), - throwsA(isA()), - ); - - expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); - }); - test('idempotent transport failure', () async { var count = 0; final mockClient = MockClient((request) async { diff --git a/pkgs/google_cloud_storage/test/upload_object_from_string_test.dart b/pkgs/google_cloud_storage/test/upload_object_from_string_test.dart index 0f4559f4..d160fa27 100644 --- a/pkgs/google_cloud_storage/test/upload_object_from_string_test.dart +++ b/pkgs/google_cloud_storage/test/upload_object_from_string_test.dart @@ -15,8 +15,6 @@ import 'dart:convert'; import 'package:google_cloud_storage/google_cloud_storage.dart'; -import 'package:http/http.dart' as http; -import 'package:http/testing.dart'; import 'package:test/test.dart'; import 'test_utils.dart'; @@ -124,27 +122,5 @@ void main() { uploadObjectFromStringTest(() => storage, createBucketWithTearDown); }); - - test('ifMetagenerationNotMatch is sent as a query parameter', () async { - late Uri requestUrl; - final mockClient = MockClient((request) async { - requestUrl = request.url; - return http.Response('', 304); - }); - - final storage = Storage(client: mockClient, projectId: 'fake project'); - - await expectLater( - storage.uploadObjectFromString( - 'bucket', - 'object', - 'Hello, World!', - ifMetagenerationNotMatch: BigInt.two, - ), - throwsA(isA()), - ); - - expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); - }); }); } diff --git a/pkgs/google_cloud_storage/test/upload_object_test.dart b/pkgs/google_cloud_storage/test/upload_object_test.dart index b9794ecb..64e28d97 100644 --- a/pkgs/google_cloud_storage/test/upload_object_test.dart +++ b/pkgs/google_cloud_storage/test/upload_object_test.dart @@ -287,34 +287,23 @@ void main() async { throwsA(isA()), ); }); - }); - - group('storage-testbench', tags: ['storage-testbench'], () { - late Storage testbenchStorage; - - setUp(() { - (_, testbenchStorage) = createStorageTestbenchClient(); - }); - - tearDown(() => testbenchStorage.close()); test( 'ifMetagenerationNotMatch throws when the metageneration matches', () async { - final bucketName = bucketNameWithTearDown( - testbenchStorage, - 'upl_obj_imnm', + final bucketName = await createBucketWithTearDown( + storage, + 'ul_obj_imnm', ); - await testbenchStorage.createBucket(BucketMetadata(name: bucketName)); - final created = await testbenchStorage.uploadObject( + final created = await storage.uploadObject( bucketName, 'object1', utf8.encode('Hello World!'), ); await expectLater( - testbenchStorage.uploadObject( + storage.uploadObject( bucketName, 'object1', utf8.encode('Goodbye World!'), @@ -324,36 +313,12 @@ void main() async { ); // The object content must be left unchanged. - final content = await testbenchStorage.downloadObject( - bucketName, - 'object1', - ); + final content = await storage.downloadObject(bucketName, 'object1'); expect(utf8.decode(content), 'Hello World!'); }, ); }); - test('ifMetagenerationNotMatch is sent as a query parameter', () async { - late Uri requestUrl; - final mockClient = MockClient((request) async { - requestUrl = request.url; - return http.Response('', 304); - }); - - final storage = Storage(client: mockClient, projectId: 'fake project'); - - await expectLater( - storage.uploadObject('bucket', 'object', [ - 1, - 2, - 3, - ], ifMetagenerationNotMatch: BigInt.two), - throwsA(isA()), - ); - - expect(requestUrl.queryParameters['ifMetagenerationNotMatch'], '2'); - }); - test('idempotent transport failure', () async { final responses = <(String, Future Function(http.Request))>[