Merged
Conversation
WalkthroughAdds a new Frends task Frends.AmazonS3.ListObjectVersions: implementation, DTOs, helpers, tests, CI/release workflows, project/solution files, metadata, changelog, and documentation to list S3 bucket object versions. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Client
participant Task as AmazonS3.ListObjectVersions
participant Handler as AmazonS3Handler
participant AWSClient as AmazonS3Client
participant S3 as S3 Service
participant ErrorHandler as ErrorHandler
Caller->>Task: Invoke ListObjectVersions(Input, Connection, Options)
Task->>Handler: RegionSelection(connection.Region)
Task->>AWSClient: Create client(credentials, region)
Task->>Handler: CheckVersioningSetup(client, input)
Handler->>AWSClient: GetBucketVersioningAsync(bucket)
AWSClient->>S3: Request versioning status
S3-->>AWSClient: Versioning status
AWSClient-->>Handler: VersioningResponse
Handler->>AWSClient: ListVersionsAsync(request) [paginated]
AWSClient->>S3: ListObjectVersions request(s)
S3-->>AWSClient: Versions response(s)
AWSClient-->>Handler: VersionsResponse
Handler-->>Task: Mapped Result (Objects)
Task-->>Caller: Return Result
alt exception occurs
Task->>ErrorHandler: Handle(exception, options.ThrowErrorOnFailure, options.ErrorMessageOnFailure)
ErrorHandler-->>Task: throws or returns Result(Success=false, Error)
Task-->>Caller: throws or returns error result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @.github/workflows/ListObjectVersions_release.yml:
- Around line 8-16: The reusable workflow reference currently uses a mutable
branch ("uses: FrendsPlatform/FrendsTasks/.github/workflows/release.yml@main");
update the "uses" value to pin to an immutable commit SHA (e.g. replace "@main"
with "@<commit-sha>") so the build job named "build" always invokes the exact
release.yml commit; locate the "uses" entry in the job definition and substitute
the branch suffix with the repository commit SHA.
In @.github/workflows/ListObjectVersions_test_on_main.yml:
- Around line 13-25: The reusable workflow reference in the build job currently
uses an unstable ref
"FrendsPlatform/FrendsTasks/.github/workflows/linux_build_main.yml@main";
replace the "@main" ref with a pinned immutable commit SHA or a stable tag to
prevent CI behavior changes (update the value in the uses field for the build
job so it reads .../linux_build_main.yml@<COMMIT_SHA_OR_TAG>), and ensure the
chosen SHA/tag is committed/published in the FrendsPlatform/FrendsTasks repo
before pushing this change.
In `@Frends.AmazonS3.ListObjectVersions/CHANGELOG.md`:
- Around line 1-7: Update the CHANGELOG to follow Keep a Changelog by adding an
"## [Unreleased]" section above "## [1.0.0]" and move/split entries so that "###
Added" under Unreleased lists specific functional items (e.g., S3
ListObjectVersions task, versioning support, filtering options, authentication
details) rather than the vague "Initial implementation"; keep "## [1.0.0] -
2026-02-04" as the released snapshot and ensure any breaking changes would be
marked under "### Changed" or "### Removed" with upgrade notes; retain the "###
Added" heading name from the diff to locate where to place the specific feature
bullets.
In
`@Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.Tests/.env.example`:
- Around line 2-3: The .env.example currently uses quoted values which become
part of the variable value; update ACCESS_KEY and SECRET_ACCESS_KEY to remove
surrounding quotes so the values are plain (e.g., ACCESS_KEY=example and
SECRET_ACCESS_KEY=example2) ensuring clients copying the file get correct
credentials; edit the lines containing the ACCESS_KEY and SECRET_ACCESS_KEY
entries to remove the quote characters.
In
`@Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.Tests/FunctionalTests.cs`:
- Around line 60-79: The current cleanup uses s3Client.ListObjectsV2Async which
only returns current live objects and misses versioned objects and delete
markers; replace the logic that calls ListObjectsV2Async and builds
DeleteObjectsRequest with a loop using s3Client.ListVersionsAsync
(ListVersionsRequest) to enumerate all object versions and delete markers
(handle pagination via IsTruncated/NextKeyMarker/NextVersionIdMarker), build
DeleteObjectsRequest.Objects from each Version/DeleteMarker by including both
Key and VersionId (KeyVersion.VersionId) and call s3Client.DeleteObjectsAsync
for each batch until no versions remain so DeleteBucketAsync can succeed.
In
`@Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Definitions/BucketObjectVersions.cs`:
- Around line 20-25: Rename the BucketObjectVersions property from Etag to ETag
to follow abbreviation casing and update all usages; specifically change the
property in class BucketObjectVersions to ETag and update the mapping in
AmazonS3Handler where it assigns the value (look for the assignment that sets
Etag) to instead set ETag. Ensure any serialization attributes, XML/JSON
mappings, tests, and other references are updated to the new property name to
keep behavior unchanged.
In
`@Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Definitions/Result.cs`:
- Line 19: The XML doc comment above the Result class contains a typo in the
example tag: remove the extra leading slashes so the tag is a proper XML doc
comment (change "/// /// <example>[{}, {}]</example>" to "/// <example>[{},
{}]</example>") in the Result class's documentation to correct the comment
formatting.
In
`@Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.cs`:
- Line 24: Update the XML <returns> documentation to match the actual Result
type used by the operation: replace the incorrect "string Output" with the
correct property signature (List<BucketObjectVersions> Objects) and include the
Success and Error shapes to reflect the Result class; locate the documentation
on the method that returns the Result class (e.g., the ListObjectVersions API
method and the Result type definition) and update the comment to read something
like: object { bool Success, List<BucketObjectVersions> Objects, object Error {
string Message, Exception AdditionalInfo } }.
In
`@Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Helpers/AmazonS3Handler.cs`:
- Around line 14-39: The ListObjectVersions method currently fetches only the
first page; change it to fully paginate like ListObjects by looping while
response.IsTruncated is true: create a ListVersionsRequest once (set BucketName,
MaxKeys, Prefix) and in a do/while or while loop call
client.ListVersionsAsync(request, cancellationToken), add response.Versions to
result.Objects each iteration (same mapping to BucketObjectVersions), then set
request.KeyMarker = response.NextKeyMarker and request.VersionIdMarker =
response.NextVersionIdMarker before the next call; after the loop set
result.ResponseTruncated = false (or reflect final response.IsTruncated) and
return the accumulated result, keeping ConfigureAwait(false) on async calls.
🧹 Nitpick comments (8)
Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/GlobalSuppressions.cs (1)
3-10: Prefer narrowing suppressions to the smallest possible scope.Assembly-level suppressions apply to the whole project and can hide unrelated issues. If feasible, target specific namespaces/types/members via
Target/Scopeto keep StyleCop coverage meaningful.Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Helpers/AmazonS3Handler.cs (1)
55-56: Use a more specific exception type and consider null safety.Using generic
Exceptionmakes it harder for callers to distinguish this error from other failures. ConsiderInvalidOperationExceptionwhich better conveys "operation cannot proceed in this state."Additionally, while uncommon,
VersioningConfigcould theoretically be null for buckets that have never had versioning configured.♻️ Proposed fix
- if (response.VersioningConfig.Status != VersionStatus.Enabled) - throw new Exception("Versioning is not enabled on bucket."); + if (response.VersioningConfig?.Status != VersionStatus.Enabled) + throw new InvalidOperationException("Versioning is not enabled on bucket.");Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.Tests/Frends.AmazonS3.ListObjectVersions.Tests.csproj (1)
16-23: Consider pinning package versions for reproducible builds.Using wildcard versions (
6.*,18.*,4.*) can lead to non-reproducible builds if a breaking change is introduced in a newer minor/patch version. Consider pinning to specific versions for more predictable CI behavior.Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.Tests/TestBase.cs (1)
9-14: Consider adding null validation for environment variables.If the environment variables
ACCESS_KEYorSECRET_ACCESS_KEYare not set, the tests will fail with potentially confusing error messages later. Adding validation in the constructor could provide clearer feedback.💡 Suggested validation
protected TestBase() { DotEnv.Load(); AccessKey = Environment.GetEnvironmentVariable("ACCESS_KEY"); SecretAccessKey = Environment.GetEnvironmentVariable("SECRET_ACCESS_KEY"); + + if (string.IsNullOrEmpty(AccessKey) || string.IsNullOrEmpty(SecretAccessKey)) + { + throw new InvalidOperationException( + "AWS credentials not found. Ensure ACCESS_KEY and SECRET_ACCESS_KEY are set in the .env file."); + } }Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.Tests/ErrorHandlerTest.cs (2)
14-19: Redundant null check afterAssert.ThrowsAsync.When
Assert.ThrowsAsync<Exception>()succeeds, it guaranteesexis not null. The assertion on Line 18 is redundant and can be removed for cleaner test code.🧹 Suggested cleanup
[Test] public void Should_Throw_Error_When_ThrowErrorOnFailure_Is_True() { var ex = Assert.ThrowsAsync<Exception>(() => AmazonS3.ListObjectVersions(DefaultInput(), DefaultConnection(), DefaultOptions(), CancellationToken.None)); - Assert.That(ex, Is.Not.Null); + Assert.That(ex.Message, Is.Not.Empty); }
31-40: Same redundant null check; also consider asserting the custom message more precisely.Line 38's null check is redundant. Additionally, the test could verify the exact message match rather than just containing the substring.
🧹 Suggested cleanup
[Test] public void Should_Use_Custom_ErrorMessageOnFailure() { var options = DefaultOptions(); options.ErrorMessageOnFailure = CustomErrorMessage; var ex = Assert.ThrowsAsync<Exception>(() => AmazonS3.ListObjectVersions(DefaultInput(), DefaultConnection(), options, CancellationToken.None)); - Assert.That(ex, Is.Not.Null); Assert.That(ex.Message, Contains.Substring(CustomErrorMessage)); }Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.Tests/FunctionalTests.cs (2)
40-50: ArbitraryTask.Delaycalls may cause test flakiness.Fixed delays (e.g.,
Task.Delay(1000)) can be unreliable in CI environments. Consider using polling/retry logic or AWS SDK waiters for eventual consistency scenarios.
134-149: Consider assertingResponseTruncatedflag when testingMaxKeys.When
MaxKeyslimits results, theResponseTruncatedflag should betrue. Adding this assertion would strengthen the test.💡 Suggested enhancement
Assert.That(result.Success, Is.True); Assert.That(result.Objects, Is.Not.Null); Assert.That(result.Objects.Count, Is.EqualTo(2)); Assert.That(result.Error, Is.Null); + Assert.That(result.ResponseTruncated, Is.True); }
Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.Tests/.env.example
Show resolved
Hide resolved
Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.Tests/FunctionalTests.cs
Outdated
Show resolved
Hide resolved
...S3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Definitions/BucketObjectVersions.cs
Show resolved
Hide resolved
Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Definitions/Result.cs
Outdated
Show resolved
Hide resolved
....ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.cs
Outdated
Show resolved
Hide resolved
...ds.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Helpers/AmazonS3Handler.cs
Show resolved
Hide resolved
MichalFrends1
requested changes
Feb 9, 2026
Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Definitions/Error.cs
Outdated
Show resolved
Hide resolved
....ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.cs
Outdated
Show resolved
Hide resolved
Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Definitions/Result.cs
Outdated
Show resolved
Hide resolved
...ds.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Helpers/AmazonS3Handler.cs
Show resolved
Hide resolved
Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.Tests/FunctionalTests.cs
Outdated
Show resolved
Hide resolved
MichalFrends1
requested changes
Feb 11, 2026
....ListObjectVersions/Frends.AmazonS3.ListObjectVersions/Frends.AmazonS3.ListObjectVersions.cs
Outdated
Show resolved
Hide resolved
MichalFrends1
approved these changes
Feb 12, 2026
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
2 participants
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.
Review Checklist
Summary by CodeRabbit
New Features
Tests
Documentation
Chores