feat(storage): add vector bucket CRUD (alpha) - #1153
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdded an experimental Sequence Diagram(s)sequenceDiagram
participant Caller
participant SupabaseStorageClient
participant StorageVectorsClient
participant VectorBucketAPI
Caller->>SupabaseStorageClient: Access vectors
SupabaseStorageClient-->>Caller: Return StorageVectorsClient
Caller->>StorageVectorsClient: Invoke bucket operation
StorageVectorsClient->>VectorBucketAPI: Send vector bucket request
VectorBucketAPI-->>StorageVectorsClient: Return bucket data or error
StorageVectorsClient-->>Caller: Return decoded result or StorageError
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 |
0dfb362 to
f67bd81
Compare
|
The following capabilities are marked
These may have been renamed, removed, or never registered. Please update the capability matrix. |
Implements the storage.vectors client by hand, using the same StorageApi/StorageHTTPSession HTTP stack as the rest of the Storage module, instead of a generated OpenAPI client.
f67bd81 to
4fd5e10
Compare
Verified against a local Supabase stack (supabase start): create, get, list (incl. prefix filtering), and delete, plus the 404 error shape for a missing bucket.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Tests/IntegrationTests/StorageVectorsClientIntegrationTests.swift`:
- Around line 13-14: Mark the StorageVectorsClientIntegrationTests suite as
serialized so vectorBucket_CRUD and listBucketsWithPrefix cannot run
concurrently against the shared test-vector-bucket resource. Keep the existing
INTEGRATION_TESTS enablement condition unchanged.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9097a4da-d49c-488a-b3cc-86b6a8c48999
📒 Files selected for processing (4)
Sources/Storage/StorageVectorsClient.swiftSources/Storage/SupabaseStorage.swiftTests/IntegrationTests/StorageVectorsClientIntegrationTests.swiftTests/StorageTests/StorageVectorsClientTests.swift
| @Suite(.enabled(if: ProcessInfo.processInfo.environment["INTEGRATION_TESTS"] != nil)) | ||
| struct StorageVectorsClientIntegrationTests { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching StorageVectorsClientIntegrationTests.swift:\n'
fd -a 'StorageVectorsClientIntegrationTests.swift' . || true
printf '\nRelevant file contents:\n'
f="$(fd 'StorageVectorsClientIntegrationTests.swift' . | head -n 1)"
if [ -n "${f:-}" ]; then
wc -l "$f"
sed -n '1,140p' "$f" | cat -n
fi
printf '\nSearch for test-vector-bucket usages:\n'
rg -n 'test-vector-bucket|vectorBucket_CRUD|listBucketsWithPrefix|`@Suite`|`@Test`' .Repository: supabase/supabase-swift
Length of output: 50379
🌐 Web query:
Swift Testing @suite .serialized trait documentation
💡 Result:
The.serialized trait in the Swift Testing framework is a trait used to control the execution order of tests by enforcing sequential (serial) rather than parallel execution [1][2]. Key characteristics and usage include: Application: It can be applied to both individual @Test functions and entire @Suite types [2][3]. Parameterized Tests: When applied to a parameterized test function, it ensures that all cases of that test run serially [2][4]. Suite Scope: When applied to a @Suite, the trait is applied recursively; all test functions and sub-suites contained within that suite will be executed serially [2][5]. Non-parameterized Tests: When applied to a non-parameterized, single test function, it generally has no effect, as those tests are naturally serial with respect to their internal code [2][4]. Scope of Effect: This trait only affects the serialization of the tests to which it is applied (or its children); it does not affect the execution of a test relative to its peers in unrelated suites or the global test execution [2][5]. Global Context: This trait is ignored if test parallelization is disabled globally (e.g., by passing the --no-parallel flag to the swift test command) [2][5]. The trait is available as a public API in the Testing framework and is documented as part of the ParallelizationTrait documentation [1][2][3].
Citations:
- 1: https://developer.apple.com/documentation/testing/trait/serialized
- 2: https://developer.apple.com/documentation/testing/parallelization
- 3: https://github.com/swiftlang/swift-evolution/blob/main/proposals/testing/0003-make-serialized-trait-api.md
- 4: https://www.hackingwithswift.com/quick-start/concurrency/how-to-serialize-parameterized-tests-with-swift-testing
- 5: https://github.com/swiftlang/swift-testing/blob/main/Sources/Testing/Traits/ParallelizationTrait.swift
Serialize test-vector-bucket integration tests.
vectorBucket_CRUD and listBucketsWithPrefix share the same bucket name. Swift Testing can run suite tests concurrently unless the suite is serialized, so one test can delete or create the bucket while another test reads it.
Proposed fix
-@Suite(.enabled(if: ProcessInfo.processInfo.environment["INTEGRATION_TESTS"] != nil))
+@Suite(.enabled(if: ProcessInfo.processInfo.environment["INTEGRATION_TESTS"] != nil), .serialized)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Suite(.enabled(if: ProcessInfo.processInfo.environment["INTEGRATION_TESTS"] != nil)) | |
| struct StorageVectorsClientIntegrationTests { | |
| `@Suite`(.enabled(if: ProcessInfo.processInfo.environment["INTEGRATION_TESTS"] != nil), .serialized) | |
| struct StorageVectorsClientIntegrationTests { |
🤖 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 `@Tests/IntegrationTests/StorageVectorsClientIntegrationTests.swift` around
lines 13 - 14, Mark the StorageVectorsClientIntegrationTests suite as serialized
so vectorBucket_CRUD and listBucketsWithPrefix cannot run concurrently against
the shared test-vector-bucket resource. Keep the existing INTEGRATION_TESTS
enablement condition unchanged.
The wire value is a raw UNIX timestamp (seconds), not an ISO8601 string, so Int obscured the semantics. Matches Session.expiresAt's existing TimeInterval convention for UNIX timestamps in this codebase.
Composition instead of subclassing StorageApi: the client now holds a StorageApi dependency passed in at init, rather than inheriting execute() and configuration directly. SupabaseStorageClient.vectors now passes itself as that dependency, so custom headers set via setHeader() are shared between the main client and .vectors.
Coverage Report for CI Build 31160573331Warning No base build found for commit Coverage: 83.944%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
Summary
storage.vectors.{createBucket,getBucket,listBuckets,deleteBucket}for Supabase Storage's alpha "vector buckets" feature, mirroring supabase-js'ssupabase.storage.vectorsclient. Indexes and vector data operations (put/get/query/delete vectors) are out of scope for this pass — start small.StorageApi/StorageHTTPSessionHTTP stack used by the rest of the Storage module (StorageBucketApi,StorageFileApi), not via a generated OpenAPI client — no new dependency ontools/openapi-codegenor theHTTPRuntimetransport bridge.main(previously based on an oldermainsnapshot).VectorBucket.creationTimeisTimeInterval?, notInt?: the wire value is a raw UNIX timestamp (seconds), not an ISO8601 string like this module's otherDatefields expect, soTimeIntervalround-trips directly without customCodablelogic. MatchesSession.expiresAt's existing convention for UNIX timestamps elsewhere in this codebase. (Required changing this file'simport Foundationtopublic import Foundation, sinceTimeIntervalis now exposed in apublicproperty.)StorageVectorsClientis astructholding aStorageApidependency (composition), not aStorageApisubclass — calls go throughapi.execute(...)/api.configurationinstead of inherited members.SupabaseStorageClient.vectorspassesselfas that dependency, so custom headers set viasetHeader(...)on the main storage client are shared with.vectorsrather than each getting an independentStorageApiinstance.Vector buckets are a public alpha feature upstream; the new API surface is gated behind
@_spi(Experimental)and documented as such.Test plan
swift buildswift test(full suite, 1062 tests)./scripts/format.sh