feat(storage): support object version operations#103
Conversation
Add S3 object-version operations on snapshot-enabled buckets:
- New `listVersions({ prefix, delimiter, limit, keyMarker, versionIdMarker })`
returning the object versions and delete markers for a prefix, plus
`nextKeyMarker` / `nextVersionIdMarker` for pagination.
- `head`, `get`, and `remove` now accept a `versionId` option. On `head` /
`get` it selects the version; on `remove` it permanently deletes that
version (without one, `remove` creates a delete marker on a versioned
bucket, matching S3 semantics).
Versioning is enabled implicitly by snapshots — pass `enableSnapshot: true`
to `createBucket`.
Assisted-by: Claude Opus 4.7 (1M context) via Claude Code
Greptile SummaryThis PR adds S3 object-version support to the storage SDK: a new
Confidence Score: 3/5Safe to merge after fixing the empty-string versionId fallback in list-versions.ts; the get/head/remove changes are minimal and low-risk. The packages/storage/src/lib/object/list-versions.ts — specifically the versionId fallback value and the corresponding type declarations Important Files Changed
Reviews (1): Last reviewed commit: "feat(storage): support object version op..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| versions: | ||
| res.Versions?.map((v) => ({ | ||
| name: v.Key ?? '', | ||
| versionId: v.VersionId ?? '', | ||
| isLatest: v.IsLatest ?? false, | ||
| size: v.Size ?? 0, | ||
| lastModified: v.LastModified ?? new Date(), | ||
| })) ?? [], | ||
| deleteMarkers: | ||
| res.DeleteMarkers?.map((m) => ({ | ||
| name: m.Key ?? '', | ||
| versionId: m.VersionId ?? '', | ||
| isLatest: m.IsLatest ?? false, | ||
| lastModified: m.LastModified ?? new Date(), | ||
| })) ?? [], |
There was a problem hiding this comment.
versionId fallback to empty string in ObjectVersion and DeleteMarker is dangerous. When VersionId is undefined (which can occur for objects in unversioned buckets or delete markers on suspended-versioning buckets), the returned versionId is ''. A caller who then passes v.versionId directly into get, head, or remove will send an empty string as the VersionId parameter, which S3 interprets as a malformed request and returns an error — rather than quietly fetching/deleting the latest version as the caller probably intended.
| versions: | |
| res.Versions?.map((v) => ({ | |
| name: v.Key ?? '', | |
| versionId: v.VersionId ?? '', | |
| isLatest: v.IsLatest ?? false, | |
| size: v.Size ?? 0, | |
| lastModified: v.LastModified ?? new Date(), | |
| })) ?? [], | |
| deleteMarkers: | |
| res.DeleteMarkers?.map((m) => ({ | |
| name: m.Key ?? '', | |
| versionId: m.VersionId ?? '', | |
| isLatest: m.IsLatest ?? false, | |
| lastModified: m.LastModified ?? new Date(), | |
| })) ?? [], | |
| versions: | |
| res.Versions?.map((v) => ({ | |
| name: v.Key ?? '', | |
| versionId: v.VersionId ?? undefined, | |
| isLatest: v.IsLatest ?? false, | |
| size: v.Size ?? 0, | |
| lastModified: v.LastModified ?? new Date(), | |
| })) ?? [], | |
| deleteMarkers: | |
| res.DeleteMarkers?.map((m) => ({ | |
| name: m.Key ?? '', | |
| versionId: m.VersionId ?? undefined, | |
| isLatest: m.IsLatest ?? false, | |
| lastModified: m.LastModified ?? new Date(), | |
| })) ?? [], |
| export type ObjectVersion = { | ||
| name: string; | ||
| versionId: string; | ||
| isLatest: boolean; | ||
| size: number; | ||
| lastModified: Date; | ||
| }; | ||
|
|
||
| export type DeleteMarker = { | ||
| name: string; | ||
| versionId: string; | ||
| isLatest: boolean; | ||
| lastModified: Date; | ||
| }; |
There was a problem hiding this comment.
ObjectVersion type declares versionId as string (non-optional), but the mapped value can be undefined when the SDK omits VersionId. The type definitions should be updated to reflect that versionId may be absent so callers are forced to handle the undefined case before passing it downstream.
| export type ObjectVersion = { | |
| name: string; | |
| versionId: string; | |
| isLatest: boolean; | |
| size: number; | |
| lastModified: Date; | |
| }; | |
| export type DeleteMarker = { | |
| name: string; | |
| versionId: string; | |
| isLatest: boolean; | |
| lastModified: Date; | |
| }; | |
| export type ObjectVersion = { | |
| name: string; | |
| versionId: string | undefined; | |
| isLatest: boolean; | |
| size: number; | |
| lastModified: Date; | |
| }; | |
| export type DeleteMarker = { | |
| name: string; | |
| versionId: string | undefined; | |
| isLatest: boolean; | |
| lastModified: Date; | |
| }; |
| export type ListVersionsOptions = { | ||
| delimiter?: string; | ||
| prefix?: string; | ||
| limit?: number; | ||
| keyMarker?: string; | ||
| versionIdMarker?: string; | ||
| config?: TigrisStorageConfig; | ||
| }; |
There was a problem hiding this comment.
VersionIdMarker silently ignored without KeyMarker
The S3 API states that version-id-marker is silently ignored when key-marker is not also present, causing the listing to restart from the beginning of the bucket. The current type exposes both fields independently, so a caller who passes versionIdMarker alone will get results starting from the top of the bucket with no error. Consider documenting this coupling in a JSDoc comment or asserting that keyMarker must be set whenever versionIdMarker is provided.
1 participant
Summary
listVersions()to enumerate object versions + delete markers for a prefix, with key-marker / version-id-marker pagination.versionIdoption tohead,get, andremove— fetch / inspect a specific historical version, or permanently delete one (vs. the default delete-marker behavior on a versioned bucket).enableSnapshot: truetocreateBucket. Closes TIG-8283.All four operations route through the AWS SDK (
ListObjectVersionsCommand/GetObjectCommand/HeadObjectCommand/DeleteObjectCommand), so SigV4 path encoding is handled by the SDK — the custom HTTP client gotcha documented in AGENTS.md does not apply here.Test plan
pnpm --filter @tigrisdata/storage test— 150 / 150 passingpnpm --filter @tigrisdata/storage build— cleanpnpm lint/pnpm format— cleanobject-versions.integration.test.tsexercises:folder/key)get(versionId)returning prior content;head(versionId)succeedingremove()creating a delete marker on a versioned bucketremove({ versionId })permanently deleting one versionkeyMarker/versionIdMarkerpaginationNote
Medium Risk
Adds new version-listing API and changes
get/head/removeto optionally target specific object versions, which can affect deletion semantics on snapshot/versioned buckets if misused.Overview
Adds S3 object version support for snapshot-enabled buckets by introducing
listVersions()to return object versions, delete markers, and pagination markers (nextKeyMarker/nextVersionIdMarker).Extends
get,head, andremovewith an optionalversionIdthat fetches/inspects a specific version or permanently deletes that version (otherwiseremovefollows S3 delete-marker behavior). Includes a new integration test suite covering version listing, historical reads, delete markers vs permanent deletes, and marker-based pagination.Reviewed by Cursor Bugbot for commit 42fb3c9. Bugbot is set up for automated code reviews on this repo. Configure here.