feat(storage): support ifMetagenerationNotMatch

[Amayyas]

Adds an ifMetagenerationNotMatch parameter to Storage.patchBucket,
Storage.uploadObject and Storage.uploadObjectFromString, resolving the three
TODOs that referenced this issue.

The failure mode described in #115 no longer applies

The issue reports a TypeError during JSON deserialization, coming from
package:googleapis' BucketsResource.patch. That dependency is gone: both
request paths (ServiceClient._makeRequest and uploadFile) now reject any
non-2xx response before deserializing, so a 304 never reaches jsonDecode.

What a 304 does produce today is a bare ServiceException with
statusCode: 304 and the message "unknown error", which is why this PR still
adds a translation step.

Behaviour

Verified against the storage-testbench: an unsatisfied ifMetagenerationNotMatch
returns 304 Not Modified with an empty body — on buckets.patch and
objects.insert alike — rather than the 412 used by the *Match
preconditions. This PR maps that 304 to a new NotModifiedException.

ServiceException is a final class, so NotModifiedException cannot extend it
and is declared alongside ChecksumValidationException instead. If you would
rather have a generic 304 -> NotModifiedException mapping in
google_cloud_rpc (i.e. in Librarian), I am happy to pursue that instead — it
would be reusable across APIs.

Tests

• 5 unit tests (MockClient) covering the query parameter and the 304
  translation, including a check that a 412 is still reported as a
  PreconditionFailedException.
• 3 storage-testbench tests asserting the real server behaviour, and that the
  bucket/object is genuinely left unchanged.

dart format, dart analyze, dart test (272 passing) and
dart test -P storage-testbench (75 passing) are all clean.

Fixes #115

[gemini-code-assist]

Code Review

This pull request adds support for the ifMetagenerationNotMatch precondition parameter in Storage.patchBucket, Storage.uploadObject, and Storage.uploadObjectFromString. It introduces a new NotModifiedException which is thrown when a '304 Not Modified' response is received from Google Cloud Storage, indicating that the precondition was not met. Comprehensive tests have been added to verify this behavior. As there are no review comments, I have no feedback to provide.

[brianquinlan]

Maybe the exception decoding logic should live in package:google_cloud_rpc around here

This seems consistent with the approach taken by other languages e.g. [python with NotModified(https://github.com/googleapis/google-cloud-python/blob/9eead955b654bec382106e08fbf1baddb250b297/packages/google-api-core/google/api_core/exceptions.py#L239)

Could you move the ifMetadataNotMatch tests into the tests for the method that accepts that argument?

[brianquinlan]

/gcbrun

[Amayyas]

Thanks — both done.

1. 304 is now mapped in ServiceException._fromDecodedResponse alongside the other status codes, so NotModifiedException is a real ServiceException subclass available to every API. The _translateNotModified wrapper and the storage-local class are gone. (My note in the PR description about final class was wrong — subclassing works fine from within the same library.)

One thing to flag: a 304 never has a body, so it was hitting the empty-body branch and surfacing as NotModifiedException: unknown error. fromHttpResponse now uses 'the resource was not modified' instead. Happy to split that out if you'd rather keep this PR narrow.

2. Tests moved into patch_bucket_test.dart, upload_object_test.dart and upload_object_from_string_test.dart; the status mapping is covered in google_cloud_rpc's exceptions_test.dart.

I left google_cloud_rpc at 0.5.3 (unreleased) and moved the storage entry to a new 0.6.4-wip section since 0.6.3 is published — let me know if versioning is handled elsewhere.

[brianquinlan]

Thanks — both done.

1. 304 is now mapped in ServiceException._fromDecodedResponse alongside the other status codes, so NotModifiedException is a real ServiceException subclass available to every API. The _translateNotModified wrapper and the storage-local class are gone. (My note in the PR description about final class was wrong — subclassing works fine from within the same library.)

One thing to flag: a 304 never has a body, so it was hitting the empty-body branch and surfacing as NotModifiedException: unknown error. fromHttpResponse now uses 'the resource was not modified' instead. Happy to split that out if you'd rather keep this PR narrow.

No, what you did is great!

2. Tests moved into patch_bucket_test.dart, upload_object_test.dart and upload_object_from_string_test.dart; the status mapping is covered in google_cloud_rpc's exceptions_test.dart.

I left google_cloud_rpc at 0.5.3 (unreleased) and moved the storage entry to a new 0.6.4-wip section since 0.6.3 is published — let me know if versioning is handled elsewhere.

That's the correct way to do it, thanks!

The philosophy of testing in this package is to do integration tests against a GCP test project whenever possible.

[Amayyas]

Thanks, all addressed:

• Integration tests — moved the patchBucket and uploadObject cases into the existing google-cloud groups so they run against a real project, and dropped the MockClient unit tests (patch/upload/upload-from-string).
• rpc exception doc — removed the storage-specific sentence from NotModifiedException; it's just "304 Not Modified" now.

One judgement call: I left the single mapping test in google_cloud_rpc's exceptions_test.dart (304 → NotModifiedException), since that's the generic layer where the decoding now lives and it doesn't need a backend. Happy to drop it too if you'd rather keep everything as integration tests.

/gcbrun when you get a chance — the integration tests only run there.

[brianquinlan]

/gcbrun

[brianquinlan]

/gcbrun

[brianquinlan]

Amazing contribution, thank you so much!