Add idempotent object deletion
This commit is contained in:
@@ -16,6 +16,7 @@ operator-selected storage fields and credential mechanisms belong in
|
||||
- `Download(ctx, key, localPath)`
|
||||
- `Upload(ctx, localPath, key, opts)`
|
||||
- `UploadConditional(ctx, source, key, opts, condition)`
|
||||
- `Delete(ctx, key)` removes one object and succeeds when it is already absent
|
||||
- `Exists(ctx, key)`
|
||||
|
||||
Key invariant:
|
||||
@@ -47,6 +48,8 @@ not own discovery, defaults, or configuration validation.
|
||||
- `Read` binds a returned body to its S3 ETag. `UploadConditional` maps an ETag
|
||||
match or absence precondition directly to the provider request and reports a
|
||||
failed precondition without performing a local check-then-write replacement.
|
||||
- `Delete` maps one normalized key to the provider delete-object operation and
|
||||
treats an absent object as success.
|
||||
- `Exists` maps not-found responses to `false`.
|
||||
|
||||
## Invariants
|
||||
|
||||
@@ -84,7 +84,7 @@ message. Update documentation alongside observable changes.
|
||||
|
||||
## Stage 1 — Add Idempotent Object Deletion
|
||||
|
||||
**Status:** Pending
|
||||
**Status:** Completed
|
||||
|
||||
### Goal
|
||||
|
||||
|
||||
@@ -23,11 +23,13 @@ type FakeBackend struct {
|
||||
Uploads []FakeUploadCall
|
||||
Downloads []FakeDownloadCall
|
||||
Reads []FakeReadCall
|
||||
Deletes []string
|
||||
ExistsKeys []string
|
||||
|
||||
ListErr error
|
||||
DownloadErr error
|
||||
UploadErr error
|
||||
DeleteErr error
|
||||
ExistsErr error
|
||||
UploadHook func(FakeUploadCall) error
|
||||
DownloadHook func(FakeDownloadCall) error
|
||||
@@ -316,6 +318,25 @@ func fakeObjectETag(data []byte) string {
|
||||
return hex.EncodeToString(sum[:])
|
||||
}
|
||||
|
||||
// Delete removes one object. Removing an absent object succeeds.
|
||||
func (f *FakeBackend) Delete(ctx context.Context, key string) error {
|
||||
if err := ctx.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
if f.DeleteErr != nil {
|
||||
return f.DeleteErr
|
||||
}
|
||||
normalizedKey := normalizeObjectKey(key)
|
||||
if normalizedKey == "" {
|
||||
return fmt.Errorf("delete object: key is required")
|
||||
}
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
f.Deletes = append(f.Deletes, normalizedKey)
|
||||
delete(f.Objects, normalizedKey)
|
||||
return nil
|
||||
}
|
||||
|
||||
// Exists checks object presence.
|
||||
func (f *FakeBackend) Exists(ctx context.Context, key string) (bool, error) {
|
||||
if err := ctx.Err(); err != nil {
|
||||
|
||||
@@ -71,6 +71,38 @@ func TestFakeBackendUploadAndExists(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestFakeBackendDeleteIsIdempotent(t *testing.T) {
|
||||
fake := &FakeBackend{}
|
||||
fake.SeedObject(FakeObject{Key: "locks/source.lock", Data: []byte("lock")})
|
||||
|
||||
for range 2 {
|
||||
if err := fake.Delete(context.Background(), `locks\source.lock`); err != nil {
|
||||
t.Fatalf("Delete() error = %v", err)
|
||||
}
|
||||
}
|
||||
if _, ok := fake.Objects["locks/source.lock"]; ok {
|
||||
t.Fatal("Delete() left the object present")
|
||||
}
|
||||
if len(fake.Deletes) != 2 || fake.Deletes[0] != "locks/source.lock" || fake.Deletes[1] != "locks/source.lock" {
|
||||
t.Fatalf("Delete() calls = %#v, want two normalized keys", fake.Deletes)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFakeBackendDeleteHonorsContextAndInjectedError(t *testing.T) {
|
||||
canceled, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
fake := &FakeBackend{}
|
||||
if err := fake.Delete(canceled, "locks/source.lock"); !errors.Is(err, context.Canceled) {
|
||||
t.Fatalf("Delete() canceled error = %v, want context.Canceled", err)
|
||||
}
|
||||
|
||||
wantErr := errors.New("delete fail")
|
||||
fake.DeleteErr = wantErr
|
||||
if err := fake.Delete(context.Background(), "locks/source.lock"); !errors.Is(err, wantErr) {
|
||||
t.Fatalf("Delete() injected error = %v, want %v", err, wantErr)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFakeBackendConditionalUploadRejectsStaleGeneration(t *testing.T) {
|
||||
fake := &FakeBackend{}
|
||||
fake.SeedObject(FakeObject{Key: "locks.yml", Data: []byte("old")})
|
||||
@@ -87,7 +119,7 @@ func TestFakeBackendConditionalUploadRejectsStaleGeneration(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestFakeBackendObjectErrors(t *testing.T) {
|
||||
fake := &FakeBackend{DownloadErr: errors.New("download fail"), UploadErr: errors.New("upload fail"), ListErr: errors.New("list fail"), ExistsErr: errors.New("exists fail")}
|
||||
fake := &FakeBackend{DownloadErr: errors.New("download fail"), UploadErr: errors.New("upload fail"), ListErr: errors.New("list fail"), DeleteErr: errors.New("delete fail"), ExistsErr: errors.New("exists fail")}
|
||||
|
||||
if _, err := fake.List(context.Background(), "x"); err == nil || !strings.Contains(err.Error(), "list fail") {
|
||||
t.Fatalf("List() error = %v, want list fail", err)
|
||||
@@ -100,6 +132,9 @@ func TestFakeBackendObjectErrors(t *testing.T) {
|
||||
if _, err := fake.Upload(context.Background(), local, "x", UploadOptions{}); err == nil || !strings.Contains(err.Error(), "upload fail") {
|
||||
t.Fatalf("Upload() error = %v, want upload fail", err)
|
||||
}
|
||||
if err := fake.Delete(context.Background(), "x"); err == nil || !strings.Contains(err.Error(), "delete fail") {
|
||||
t.Fatalf("Delete() error = %v, want delete fail", err)
|
||||
}
|
||||
if _, err := fake.Exists(context.Background(), "x"); err == nil || !strings.Contains(err.Error(), "exists fail") {
|
||||
t.Fatalf("Exists() error = %v, want exists fail", err)
|
||||
}
|
||||
|
||||
@@ -28,6 +28,7 @@ type ObjectStore interface {
|
||||
Download(ctx context.Context, key, localPath string) error
|
||||
Upload(ctx context.Context, localPath, key string, opts UploadOptions) (ObjectInfo, error)
|
||||
UploadConditional(ctx context.Context, source io.Reader, key string, opts UploadOptions, condition WriteCondition) (ObjectInfo, error)
|
||||
Delete(ctx context.Context, key string) error
|
||||
Exists(ctx context.Context, key string) (bool, error)
|
||||
}
|
||||
|
||||
|
||||
@@ -23,6 +23,7 @@ type s3API interface {
|
||||
ListObjectsV2(ctx context.Context, params *s3.ListObjectsV2Input, optFns ...func(*s3.Options)) (*s3.ListObjectsV2Output, error)
|
||||
GetObject(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error)
|
||||
PutObject(ctx context.Context, params *s3.PutObjectInput, optFns ...func(*s3.Options)) (*s3.PutObjectOutput, error)
|
||||
DeleteObject(ctx context.Context, params *s3.DeleteObjectInput, optFns ...func(*s3.Options)) (*s3.DeleteObjectOutput, error)
|
||||
HeadObject(ctx context.Context, params *s3.HeadObjectInput, optFns ...func(*s3.Options)) (*s3.HeadObjectOutput, error)
|
||||
}
|
||||
|
||||
@@ -300,6 +301,25 @@ func (b *S3Backend) uploadReader(ctx context.Context, source io.Reader, key stri
|
||||
return info, nil
|
||||
}
|
||||
|
||||
// Delete removes one object. S3 deletion is idempotent when the key is absent.
|
||||
func (b *S3Backend) Delete(ctx context.Context, key string) error {
|
||||
if err := ctx.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
normalizedKey := normalizeObjectKey(key)
|
||||
if normalizedKey == "" {
|
||||
return fmt.Errorf("delete object: key is required")
|
||||
}
|
||||
_, err := b.client.DeleteObject(ctx, &s3.DeleteObjectInput{
|
||||
Bucket: &b.bucket,
|
||||
Key: &normalizedKey,
|
||||
})
|
||||
if err != nil {
|
||||
return fmt.Errorf("delete object %q: %w", normalizedKey, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// Exists checks whether one object key exists.
|
||||
func (b *S3Backend) Exists(ctx context.Context, key string) (bool, error) {
|
||||
normalizedKey := normalizeObjectKey(key)
|
||||
|
||||
@@ -29,16 +29,18 @@ type fakeS3API struct {
|
||||
getETag *string
|
||||
getLastModified *time.Time
|
||||
|
||||
putOut *s3.PutObjectOutput
|
||||
putErr error
|
||||
putOut *s3.PutObjectOutput
|
||||
putErr error
|
||||
deleteErr error
|
||||
|
||||
headErr error
|
||||
|
||||
lastList *s3.ListObjectsV2Input
|
||||
lastLists []*s3.ListObjectsV2Input
|
||||
lastGet *s3.GetObjectInput
|
||||
lastPut *s3.PutObjectInput
|
||||
lastHead *s3.HeadObjectInput
|
||||
lastList *s3.ListObjectsV2Input
|
||||
lastLists []*s3.ListObjectsV2Input
|
||||
lastGet *s3.GetObjectInput
|
||||
lastPut *s3.PutObjectInput
|
||||
lastDelete *s3.DeleteObjectInput
|
||||
lastHead *s3.HeadObjectInput
|
||||
}
|
||||
|
||||
func (f *fakeS3API) ListObjectsV2(_ context.Context, params *s3.ListObjectsV2Input, _ ...func(*s3.Options)) (*s3.ListObjectsV2Output, error) {
|
||||
@@ -116,6 +118,14 @@ func (f *fakeS3API) PutObject(_ context.Context, params *s3.PutObjectInput, _ ..
|
||||
return f.putOut, nil
|
||||
}
|
||||
|
||||
func (f *fakeS3API) DeleteObject(_ context.Context, params *s3.DeleteObjectInput, _ ...func(*s3.Options)) (*s3.DeleteObjectOutput, error) {
|
||||
f.lastDelete = params
|
||||
if f.deleteErr != nil {
|
||||
return nil, f.deleteErr
|
||||
}
|
||||
return &s3.DeleteObjectOutput{}, nil
|
||||
}
|
||||
|
||||
func (f *fakeS3API) HeadObject(_ context.Context, params *s3.HeadObjectInput, _ ...func(*s3.Options)) (*s3.HeadObjectOutput, error) {
|
||||
f.lastHead = params
|
||||
if f.headErr != nil {
|
||||
@@ -275,6 +285,37 @@ func TestS3BackendExistsNotFound(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestS3BackendDeleteMapsRequestAndErrors(t *testing.T) {
|
||||
client := &fakeS3API{}
|
||||
backend := &S3Backend{bucket: "bucket-1", client: client}
|
||||
if err := backend.Delete(context.Background(), `locks\source.lock`); err != nil {
|
||||
t.Fatalf("Delete() error = %v", err)
|
||||
}
|
||||
if client.lastDelete == nil || *client.lastDelete.Bucket != "bucket-1" || *client.lastDelete.Key != "locks/source.lock" {
|
||||
t.Fatalf("DeleteObject input = %#v, want normalized bucket and key", client.lastDelete)
|
||||
}
|
||||
|
||||
wantErr := errors.New("permission denied")
|
||||
client.deleteErr = wantErr
|
||||
err := backend.Delete(context.Background(), "locks/source.lock")
|
||||
if !errors.Is(err, wantErr) || !strings.Contains(err.Error(), "locks/source.lock") {
|
||||
t.Fatalf("Delete() error = %v, want contextual backend error", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestS3BackendDeleteHonorsCancellation(t *testing.T) {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
client := &fakeS3API{}
|
||||
err := (&S3Backend{bucket: "bucket-1", client: client}).Delete(ctx, "locks/source.lock")
|
||||
if !errors.Is(err, context.Canceled) {
|
||||
t.Fatalf("Delete() error = %v, want context.Canceled", err)
|
||||
}
|
||||
if client.lastDelete != nil {
|
||||
t.Fatal("Delete() called S3 after cancellation")
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewS3BackendFromConfigUsesClientOptions(t *testing.T) {
|
||||
original := newS3Client
|
||||
t.Cleanup(func() { newS3Client = original })
|
||||
|
||||
@@ -610,6 +610,10 @@ func (s *failKeyStore) UploadConditional(ctx context.Context, source io.Reader,
|
||||
return s.delegate.UploadConditional(ctx, source, key, opts, condition)
|
||||
}
|
||||
|
||||
func (s *failKeyStore) Delete(ctx context.Context, key string) error {
|
||||
return s.delegate.Delete(ctx, key)
|
||||
}
|
||||
|
||||
func (s *failKeyStore) Exists(ctx context.Context, key string) (bool, error) {
|
||||
return s.delegate.Exists(ctx, key)
|
||||
}
|
||||
|
||||
@@ -1853,6 +1853,11 @@ func (s *analyzeObjectStoreTracker) UploadConditional(context.Context, io.Reader
|
||||
return storage.ObjectInfo{}, errors.New("unexpected conditional object store upload call")
|
||||
}
|
||||
|
||||
func (s *analyzeObjectStoreTracker) Delete(context.Context, string) error {
|
||||
s.called = true
|
||||
return errors.New("unexpected object store delete call")
|
||||
}
|
||||
|
||||
func (s *analyzeObjectStoreTracker) Exists(context.Context, string) (bool, error) {
|
||||
s.called = true
|
||||
return false, errors.New("unexpected object store exists call")
|
||||
|
||||
@@ -425,6 +425,10 @@ func (s *preparePreviousCaptureStore) UploadConditional(ctx context.Context, sou
|
||||
return s.delegate.UploadConditional(ctx, source, key, opts, condition)
|
||||
}
|
||||
|
||||
func (s *preparePreviousCaptureStore) Delete(ctx context.Context, key string) error {
|
||||
return s.delegate.Delete(ctx, key)
|
||||
}
|
||||
|
||||
func (s *preparePreviousCaptureStore) Exists(ctx context.Context, key string) (bool, error) {
|
||||
s.existsKeys = append(s.existsKeys, key)
|
||||
return s.delegate.Exists(ctx, key)
|
||||
|
||||
Reference in New Issue
Block a user