Fail PUT object with match on non-existent keys & code optimizations #2503
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Overlooked the potential NPE here...
Make them a bit easier to read.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Exclusion is brittle as new StorageClass values may break tests.
5fa1cb1 to
2ba51da
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements correct failure behavior for PUT requests with If-Match on non-existent keys, cleans up unused imports and tooling configs, updates Kotlin compiler settings, refactors test support for stability, and bumps the version to 4.6.0-SNAPSHOT.
- Enforce
NO_SUCH_KEYexception whenIf-Matchis used on missing objects inObjectService - Added integration tests for
if-match/if-none-matchPUT semantics and fixed storage-class test enumeration - Bumped project version and Kotlin compiler api/language levels, updated changelog, and removed unused imports
Reviewed Changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| testsupport/*/pom.xml | Bumped snapshot versions to 4.6.0-SNAPSHOT |
| testsupport/common/src/.../S3MockStarter.java and DigestUtil.java | Removed unused @NonNull imports |
| server/src/.../StoreConfiguration.java | Switched to mockRegion.id() for bucket creation |
| server/src/.../ObjectService.java | Throw NO_SUCH_KEY when matching non-existent objects and refactored conditions |
| server/src/.../S3MockProperties.java | Removed unused Region import |
| pom.xml (root) | Updated Kotlin compiler settings for 2.2 |
| integration-tests/src/.../*.kt | Migrated Java-style annotations to Kotlin annotation class and added PUT tests |
| integration-tests/src/.../S3TestBase.kt | Replaced dynamic StorageClass stream with fixed list |
| Makefile | Added build, verify, and install targets |
| CHANGELOG.md | Advanced planned release to 4.7.0, added 4.6.0 release notes |
Comments suppressed due to low confidence (2)
pom.xml:24
- The CHANGELOG shows '4.7.0 - PLANNED', but the POM is set to 4.6.0-SNAPSHOT. Consider bumping the project version to 4.7.0-SNAPSHOT to match the planned release.
<version>4.6.0-SNAPSHOT</version>
pom.xml:86
- The CHANGELOG indicates bumping aws-v2.version to 2.31.77, but this property is still 2.31.67. Update it to 2.31.77 for consistency.
<aws-v2.version>2.31.67</aws-v2.version>
server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
StorageClassvalues in tests. New values added by AWS sometimes break tests. We want to make sure to test a few different storage classes, no need to test every one.Related Issue
#2502
Tasks