Conversation
server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java
Fixed
Show fixed
Hide fixed
server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java
Fixed
Show fixed
Hide fixed
server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java
Fixed
Show fixed
Hide fixed
server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java
Fixed
Show fixed
Hide fixed
9c312f3 to
d07e22d
Compare
d07e22d to
3a3a33a
Compare
3a3a33a to
ac68688
Compare
ac68688 to
e1633d9
Compare
e1633d9 to
d673c89
Compare
Includes minor fixes.
d673c89 to
a94ac89
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds server-side validation for S3 object tags, introduces a new INVALID_TAG exception, and updates related tests and documentation.
- Implements
verifyObjectTagsinObjectServiceand calls it in the controller - Defines
INVALID_TAGinS3Exceptionand adds unit tests covering various tag validation scenarios - Renames
writeVersionsfiletowriteVersionsFileinObjectStoreand applies minor metadata updates
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java | Add verifyObjectTags and helper methods for tag validation |
| server/src/main/java/com/adobe/testing/s3mock/S3Exception.java | Add INVALID_TAG exception constant |
| server/src/main/java/com/adobe/testing/s3mock/ObjectController.java | Call verifyObjectTags before applying tags |
| server/src/main/java/com/adobe/testing/s3mock/store/ObjectStore.java | Rename writeVersionsfile → writeVersionsFile and null checks |
| server/src/main/java/com/adobe/testing/s3mock/store/S3ObjectMetadata.java | Annotate deleteMarker’s versionId parameter as @Nullable |
| server/src/test/kotlin/com/adobe/testing/s3mock/service/ObjectServiceTest.kt | Add tests for valid/invalid tag lists |
| README.md | Fix typos, headings, wording, and add "Powered by" section |
| CHANGELOG.md | Document tag verification feature and minor version bumps |
Comments suppressed due to low confidence (5)
server/src/test/kotlin/com/adobe/testing/s3mock/service/ObjectServiceTest.kt:314
- [nitpick] Test name refers to 'store tags' but the method under test is
verifyObjectTags; consider renaming to 'verify tags succeeds' for clarity.
fun `store tags succeeds`() {
server/src/test/kotlin/com/adobe/testing/s3mock/service/ObjectServiceTest.kt:334
- [nitpick] Add boundary tests for exactly
MAX_ALLOWED_TAGS(50) and just above (51) to ensure edge conditions are handled correctly.
for (i in 0..60) {
server/src/main/java/com/adobe/testing/s3mock/store/ObjectStore.java:518
- [nitpick] The method name
writeMetafileis inconsistent withwriteVersionsFile; consider renaming towriteMetaFilefor consistent camel-case.
writeMetafile(bucket, S3ObjectMetadata.deleteMarker(s3ObjectMetadata, versionId));
server/src/main/java/com/adobe/testing/s3mock/store/S3ObjectMetadata.java:79
- Make sure the correct
@Nullableimport (e.g.,org.jspecify.annotations.Nullable) is present to avoid compilation errors.
public static S3ObjectMetadata deleteMarker(S3ObjectMetadata metadata, @Nullable String versionId) {
README.md:576
- [nitpick] Consider splitting this sentence or replacing the comma with a semicolon to avoid a run-on sentence.
`S3Mock` dependencies are updated regularly, any update could break any number of projects.
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
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
Tags are now verified for correctness.
Related Issue
N/A
Tasks