feat(filePath):s3 file upload into specified path #804
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
|
melihciray
requested a deployment
to
external-testing
GitHub Actions
Waiting
WalkthroughAdds optional objectPrefix support to S3Store, updates key generation to prepend the prefix, and refactors S3 operations to use centralized key helpers. Introduces new examples demonstrating various prefix strategies and adds documentation explaining configuration, usage, and migration. Public API updated to include objectPrefix in Options and a class property. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/s3-store/src/index.ts (1)
758-771: Bug: double-prefixing in deleteExpired leads to orphaned objects.expiredUpload.Key already includes the prefix; passing it to infoKey/partKey re-applies objectPrefix, generating wrong keys (e.g., uploads/uploads/.info). Delete will miss targets.
Apply this fix to derive related keys from the full S3 key without re-prefixing:
- all.push( - { - key: this.infoKey(expiredUpload.Key as string), - }, - { - key: this.partKey(expiredUpload.Key as string, true), - } - ) + const mainKey = expiredUpload.Key as string + all.push( + { key: `${mainKey}.info` }, + { key: `${mainKey}.part` } + )Consider adding a small helper to avoid future drift, e.g., infoKeyFromObjectKey(key) and partKeyFromObjectKey(key, true).
🧹 Nitpick comments (4)
packages/s3-store/OBJECT_PREFIX_EXAMPLE.md (1)
31-37: Add language to fenced code blocks (markdownlint MD040).Label the directory tree code blocks as text to satisfy MD040.
-``` +```text my-bucket/ ├── file-id-1 ├── file-id-1.info ├── file-id-2 └── file-id-2.info```diff -``` +```text my-bucket/ └── uploads/ ├── file-id-1 ├── file-id-1.info ├── file-id-2 └── file-id-2.info```diff -``` +```text my-bucket/ └── my-app/ └── uploads/ └── 2024/ ├── file-id-1 ├── file-id-1.info ├── file-id-2 └── file-id-2.infoAlso applies to: 40-47, 50-59 </blockquote></details> <details> <summary>packages/s3-store/QUICK_REFERENCE.md (1)</summary><blockquote> `72-79`: **Specify language for console directory trees (markdownlint MD040).** Use text for these blocks. ```diff -``` +```text 📁 my-bucket └── 📁 uploads ├── 📄 file-abc123 ├── 📄 file-abc123.info ├── 📄 file-xyz789 └── 📄 file-xyz789.info```diff -``` +```text 📁 my-bucket ├── 📄 file-abc123 ├── 📄 file-abc123.info ├── 📄 file-xyz789 └── 📄 file-xyz789.infoAlso applies to: 83-89 </blockquote></details> <details> <summary>packages/s3-store/IMPLEMENTATION_SUMMARY.md (1)</summary><blockquote> `65-71`: **Add language identifiers to directory tree code blocks (MD040).** Mark the folder trees as text. ```diff -``` +```text my-bucket/ ├── file-abc123 ├── file-abc123.info ├── file-xyz789 └── file-xyz789.info```diff -``` +```text my-bucket/ └── uploads/ ├── file-abc123 ├── file-abc123.info ├── file-xyz789 └── file-xyz789.info```diff -``` +```text my-bucket/ └── my-app/ └── uploads/ └── 2024/ ├── file-abc123 ├── file-abc123.info ├── file-xyz789 └── file-xyz789.infoAlso applies to: 74-81, 84-93 </blockquote></details> <details> <summary>packages/s3-store/examples.ts (1)</summary><blockquote> `87-113`: **Avoid per-request server instantiation.** Creating a new Server and S3Store on every /user-uploads request is expensive. Cache per-user instances or reuse a shared Server with dynamic datastore resolution. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yml **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 15b9c6063212e628cbcf97e51296575207f20948 and 24479853d1a55d0f18e054cc5bc13c5634d7a0dc. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `packages/s3-store/IMPLEMENTATION_SUMMARY.md` (1 hunks) * `packages/s3-store/OBJECT_PREFIX_EXAMPLE.md` (1 hunks) * `packages/s3-store/QUICK_REFERENCE.md` (1 hunks) * `packages/s3-store/examples.ts` (1 hunks) * `packages/s3-store/src/index.ts` (11 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>packages/s3-store/examples.ts (1)</summary><blockquote> <details> <summary>packages/s3-store/src/index.ts (1)</summary> * `S3Store` (101-838) </details> </blockquote></details> </details><details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>packages/s3-store/QUICK_REFERENCE.md</summary> 72-72: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 83-83: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>packages/s3-store/IMPLEMENTATION_SUMMARY.md</summary> 65-65: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 74-74: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 84-84: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>packages/s3-store/OBJECT_PREFIX_EXAMPLE.md</summary> 31-31: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 40-40: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 50-50: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>packages/s3-store/src/index.ts (2)</summary><blockquote> `224-237`: **Key helpers look good.** infoKey/partKey correctly prepend objectPrefix and preserve .part behavior. --- `467-471`: **Consistent use of key helpers.** All S3 operations now use partKey/infoKey; this maintains prefix correctness end-to-end. Also applies to: 493-499, 557-562, 585-590, 696-714 </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
2 tasks
|
Already supported as discussed in #805 |
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.
This pull request introduces the new
objectPrefixfeature to the S3Store, allowing users to organize uploaded files in logical folder structures within their S3 bucket. The changes are fully backward compatible and include updates to the implementation, documentation, and usage examples to support and demonstrate this feature.S3Store Implementation Updates
objectPrefixproperty to theOptionstype and theS3Storeclass, initialized in the constructor and defaulting to an empty string for backward compatibility. All S3 object key generation methods (infoKey,partKey, and all S3 operations) now use this prefix to create pseudo-directory structures. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Documentation and Example Additions
IMPLEMENTATION_SUMMARY.md,OBJECT_PREFIX_EXAMPLE.md, andQUICK_REFERENCE.md. These docs explain how folder structures are created, provide before-and-after examples, and detail use cases such as multi-tenant, date-based, and environment-based organization. [1] [2] [3]examples.tsfile with practical code samples for various usage scenarios, including multi-tenant, environment-based, per-user uploads, document type segregation, and lifecycle policy-friendly structures.Backward Compatibility and Migration
objectPrefixis not provided, files are stored at the bucket root, maintaining current behavior. Documentation provides migration strategies for existing setups. [1] [2]Benefits and Best Practices
Testing and S3 Console Visualization
Let me know if you have questions about how the prefixing logic works or how to use it in your own code!