feat: add native S3 upload handler and receiver - #192
Merged
Conversation
📝 WalkthroughWhat changed:
Risk: medium Security-sensitive areas touched:
Test coverage impact:
Deployment, migration, rollback, operational concerns:
WalkthroughAdds two new native adapters for S3-backed receive and upload flows, plus their startup input models, package dependency, and dependency-injection registration. ChangesS3 Native Adapters
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SW.Bitween.NativeAdapters/S3Receiver/NativeS3Receiver.cs`:
- Around line 58-72: The delete-move logic in NativeS3Receiver.DeleteFile is
flattening nested keys by using Path.GetFileName(fileId), which can cause
collisions when ListAsync returns subdirectory paths. Update DeleteFile to
preserve the relative path from FolderName when building the targetKey, and make
sure the moved object keeps its nested directory structure under
_options.DeleteMovesFileTo rather than collapsing everything to the base
filename.
- Around line 49-55: `NativeS3Receiver` is calling `ToLower()` on
`ResponseEncoding` without handling null, which can crash when
`settings.ConvertTo<S3ReceiverInput>()` populates a null value. Update the logic
around the `ResponseEncoding` switch to guard against null before normalizing
the string, and make the `Load`/receiver path in `NativeS3Receiver` fail with a
clear `ArgumentException` if `ResponseEncoding` is missing or invalid rather
than dereferencing it.
- Around line 25-29: `NativeS3Receiver` currently only disposes `_cloudFiles`
inside `Finalize()`, so exceptions in `ListFiles`, `GetFile`, or `DeleteFile`
can leave the S3 client undisposed. Update `NativeS3Receiver` to implement
`IDisposable` and move the cleanup into a proper `Dispose` path (keeping
`Finalize()` delegating to it if needed), so the scoped receiver always releases
`_cloudFiles` regardless of success or failure. Use the `NativeS3Receiver` class
and its `Finalize()` method as the main touchpoints.
- Around line 62-68: The move logic in NativeS3Receiver currently buffers the
entire object into a MemoryStream before re-uploading it, which can exhaust
memory on large files. Update the path around _cloudFiles.OpenReadAsync and
_cloudFiles.WriteAsync to use a server-side copy or another streaming-friendly
adapter method if available, and avoid CopyToAsync into an in-memory buffer.
Keep the target key construction with Path.GetFileName(fileId) and
_options.DeleteMovesFileTo, but change the transfer approach so the file is
moved without fully loading it into memory.
In `@SW.Bitween.NativeAdapters/S3UploadHandler/NativeS3UploadHandler.cs`:
- Around line 20-27: The S3 object key generation in NativeS3UploadHandler
currently uses a second-precision timestamp when _options.FileName is empty,
which can produce duplicate keys and overwrites for rapid uploads. Update the
key creation logic in the upload path to include higher-resolution uniqueness,
such as milliseconds and a unique suffix, while preserving the existing
_options.FolderName prefix behavior. Keep the fix localized to the key-building
block that uses DateTime.UtcNow and _options.FileExtension so each upload gets a
distinct S3 key.
- Line 23: The key generation in NativeS3UploadHandler uses
_options.FileExtension directly, which can produce malformed S3 object keys when
the extension includes a leading dot or is empty. Normalize FileExtension before
building the key by stripping any leading dot and handling null/empty so the
generated value in the key assignment always has a valid format without double
dots or trailing dots.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: simplify9/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6596375b-6ea3-4418-85a3-2754f3043896
📒 Files selected for processing (6)
SW.Bitween.NativeAdapters/S3Receiver/NativeS3Receiver.csSW.Bitween.NativeAdapters/S3Receiver/S3ReceiverInput.csSW.Bitween.NativeAdapters/S3UploadHandler/NativeS3UploadHandler.csSW.Bitween.NativeAdapters/S3UploadHandler/S3UploadHandlerInput.csSW.Bitween.NativeAdapters/SW.Bitween.NativeAdapters.csprojSW.Bitween.NativeAdapters/ServiceCollectionExtensions.cs
📜 Review details
🔇 Additional comments (4)
SW.Bitween.NativeAdapters/S3Receiver/S3ReceiverInput.cs (1)
6-30: LGTM!SW.Bitween.NativeAdapters/S3UploadHandler/S3UploadHandlerInput.cs (1)
6-29: LGTM!SW.Bitween.NativeAdapters/SW.Bitween.NativeAdapters.csproj (1)
25-25: LGTM!SW.Bitween.NativeAdapters/ServiceCollectionExtensions.cs (1)
7-8: LGTM!Also applies to: 43-48
Port the serverless S3 handler and receiver into in-process native adapters (NativeS3UploadHandler, NativeS3Receiver), using the same SimplyWorks.CloudFiles.S3 library already referenced by Bitween-api. - Receiver's DeleteFile now always removes the fetched object; the original only deleted when DeleteMovesFileTo was set, so files were reprocessed on every poll cycle by default - DeleteMovesFileTo is reinterpreted as a copy to another key/prefix in the same bucket; the original wrote to local disk, which has no meaning for an in-process API pod - Receiver's ContentType field renamed to ResponseEncoding (utf8/base64) to match the FTP/Pop3 native adapters, since it was actually selecting response encoding, not a MIME content type - Fields renamed for clarity (Url -> ServiceUrl, TargetPath -> BucketName) to match the underlying CloudFilesOptions shape
Guard against null encoding, use S3 server-side copy instead of buffering in memory, avoid filename collisions on move and upload, and dispose the S3 client reliably via IDisposable.
hamzahalq
force-pushed
the
hamza/feature/native-s3-adapters
branch
from
July 8, 2026 09:53
5dbd0cd to
b88b006
Compare
mmalkhatib
approved these changes
Jul 8, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
SW.Bitween.NativeAdapters/S3Receiver/NativeS3Receiver.cs (1)
72-89: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winTrailing slash in
FolderNamebreaks relative-path calculation inDeleteFile.If
FolderNameis configured with a trailing slash (e.g.,"input/"), theStartsWith(_options.FolderName + "/")check becomesStartsWith("input//"), which never matches. TherelativePathfalls back to the fullfileId, so the moved object lands atDeleteMovesFileTo/input/file.txtinstead ofDeleteMovesFileTo/file.txt.🛡️ Proposed fix: trim trailing slashes before comparison
public async Task DeleteFile(string fileId) { if (!string.IsNullOrWhiteSpace(_options.DeleteMovesFileTo)) { + var folderName = _options.FolderName?.TrimEnd('/'); // Preserve the path relative to FolderName so files with the same name in // different subdirectories don't collide at the destination. - var relativePath = !string.IsNullOrEmpty(_options.FolderName) && fileId.StartsWith(_options.FolderName + "/") - ? fileId[(_options.FolderName.Length + 1)..] + var relativePath = !string.IsNullOrEmpty(folderName) && fileId.StartsWith(folderName + "/") + ? fileId[(folderName.Length + 1)..] : fileId;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SW.Bitween.NativeAdapters/S3Receiver/NativeS3Receiver.cs` around lines 72 - 89, In NativeS3Receiver.DeleteFile, the relative-path logic fails when _options.FolderName includes a trailing slash because the StartsWith check is built against FolderName + "/". Normalize FolderName by trimming trailing slashes before comparing and slicing fileId, so the relative path is computed correctly regardless of whether the configured folder ends with "/". Keep the fix localized to DeleteFile and use the existing _options.FolderName, fileId, and relativePath flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SW.Bitween.NativeAdapters/SW.Bitween.NativeAdapters.csproj`:
- Line 25: The package reference for SimplyWorks.CloudFiles.S3 is outdated and
should be updated to the requested patch release. Change the Version on the
PackageReference in the SW.Bitween.NativeAdapters project file from the current
8.1.1 to 8.1.6, keeping the package name unchanged and ensuring the project
still restores cleanly after the version bump.
---
Duplicate comments:
In `@SW.Bitween.NativeAdapters/S3Receiver/NativeS3Receiver.cs`:
- Around line 72-89: In NativeS3Receiver.DeleteFile, the relative-path logic
fails when _options.FolderName includes a trailing slash because the StartsWith
check is built against FolderName + "/". Normalize FolderName by trimming
trailing slashes before comparing and slicing fileId, so the relative path is
computed correctly regardless of whether the configured folder ends with "/".
Keep the fix localized to DeleteFile and use the existing _options.FolderName,
fileId, and relativePath flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: simplify9/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 52683d03-d044-471e-91b5-19b8a1bc08d1
📒 Files selected for processing (6)
SW.Bitween.NativeAdapters/S3Receiver/NativeS3Receiver.csSW.Bitween.NativeAdapters/S3Receiver/S3ReceiverInput.csSW.Bitween.NativeAdapters/S3UploadHandler/NativeS3UploadHandler.csSW.Bitween.NativeAdapters/S3UploadHandler/S3UploadHandlerInput.csSW.Bitween.NativeAdapters/SW.Bitween.NativeAdapters.csprojSW.Bitween.NativeAdapters/ServiceCollectionExtensions.cs
📜 Review details
🔇 Additional comments (5)
SW.Bitween.NativeAdapters/S3Receiver/S3ReceiverInput.cs (1)
1-30: LGTM!SW.Bitween.NativeAdapters/S3Receiver/NativeS3Receiver.cs (1)
1-70: LGTM! Past review issues (IDisposable, ResponseEncoding null guard, Path.GetFileName collision, in-memory buffering for move) have been resolved.Also applies to: 91-99
SW.Bitween.NativeAdapters/S3UploadHandler/S3UploadHandlerInput.cs (1)
1-29: LGTM!SW.Bitween.NativeAdapters/S3UploadHandler/NativeS3UploadHandler.cs (1)
1-48: LGTM! Past review issues (second-precision timestamp collisions,FileExtensionleading-dot normalization) have been resolved with GUID suffix andTrimStart('.').SW.Bitween.NativeAdapters/ServiceCollectionExtensions.cs (1)
7-8: LGTM! Scoped registrations follow the existing adapter pattern and feedNativeAdapterDiscoveryServicecorrectly.Also applies to: 43-47
| <PackageReference Include="Rebex.Pop3" Version="8.0.9673" /> | ||
| <PackageReference Include="Rebex.Sftp" Version="8.0.9673" /> | ||
| <PackageReference Include="Scriban" Version="7.0.6" /> | ||
| <PackageReference Include="SimplyWorks.CloudFiles.S3" Version="8.1.1" /> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check NuGet for the latest version and any security advisories.
curl -s "https://api.nuget.org/v3-flatcontainer/simplyworks.cloudfiles.s3/index.json" | jq '.versions[-3:]'
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NUGET, package: "SimplyWorks.CloudFiles.S3") {
nodes {
advisory { summary severity publishedAt }
vulnerableVersionRange
firstPatchedVersion { identifier }
}
}
}'Repository: simplify9/Bitween-api
Length of output: 244
Upgrade SimplyWorks.CloudFiles.S3 to 8.1.6 8.1.1 is several patch releases behind the latest available release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SW.Bitween.NativeAdapters/SW.Bitween.NativeAdapters.csproj` at line 25, The
package reference for SimplyWorks.CloudFiles.S3 is outdated and should be
updated to the requested patch release. Change the Version on the
PackageReference in the SW.Bitween.NativeAdapters project file from the current
8.1.1 to 8.1.6, keeping the package name unchanged and ensuring the project
still restores cleanly after the version bump.
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.
Port the serverless S3 handler and receiver into in-process native adapters (NativeS3UploadHandler, NativeS3Receiver), using the same SimplyWorks.CloudFiles.S3 library already referenced by Bitween-api.