From 49d30a7189f3ce0a2dc7594a1a01ef2e0d63942d Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Wed, 21 Aug 2019 09:19:26 -0700 Subject: [PATCH] feat: make delete idempotent fixes #104 --- autobatch/autobatch.go | 4 ---- basic_ds.go | 3 --- batch.go | 5 ----- datastore.go | 7 ++++--- examples/fs.go | 8 ++++++-- mount/mount.go | 2 +- mount/mount_test.go | 4 ++-- 7 files changed, 13 insertions(+), 20 deletions(-) diff --git a/autobatch/autobatch.go b/autobatch/autobatch.go index aeca9a4..66d701e 100644 --- a/autobatch/autobatch.go +++ b/autobatch/autobatch.go @@ -75,10 +75,6 @@ func (d *Datastore) Flush() error { var err error if o.delete { err = b.Delete(k) - if err == ds.ErrNotFound { - // Ignore these, let delete be idempotent. - err = nil - } } else { err = b.Put(k, o.value) } diff --git a/basic_ds.go b/basic_ds.go index 19b74e2..08ae2af 100644 --- a/basic_ds.go +++ b/basic_ds.go @@ -53,9 +53,6 @@ func (d *MapDatastore) GetSize(key Key) (size int, err error) { // Delete implements Datastore.Delete func (d *MapDatastore) Delete(key Key) (err error) { - if _, found := d.values[key]; !found { - return ErrNotFound - } delete(d.values, key) return nil } diff --git a/batch.go b/batch.go index 57880dd..41e23ff 100644 --- a/batch.go +++ b/batch.go @@ -35,11 +35,6 @@ func (bt *basicBatch) Commit() error { for k, op := range bt.ops { if op.delete { err = bt.target.Delete(k) - // We could try to do something smarter but I really - // don't care. Delete should be idempotent anyways. - if err == ErrNotFound { - err = nil - } } else { err = bt.target.Put(k, op.value) } diff --git a/datastore.go b/datastore.go index eec1932..604ff71 100644 --- a/datastore.go +++ b/datastore.go @@ -50,7 +50,8 @@ type Write interface { // type-safe interface to your application, and do the checking up-front. Put(key Key, value []byte) error - // Delete removes the value for given `key`. + // Delete removes the value for given `key`. If the key is not in the + // datastore, this method returns no error. Delete(key Key) error } @@ -191,8 +192,8 @@ type TxnDatastore interface { // Errors -// ErrNotFound is returned by Get, Has, and Delete when a datastore does not -// map the given key to a value. +// ErrNotFound is returned by Get and GetSize when a datastore does not map the +// given key to a value. var ErrNotFound = errors.New("datastore: key not found") // ErrInvalidType is returned by Put when a given value is incopatible with diff --git a/examples/fs.go b/examples/fs.go index 94cefab..87369e8 100644 --- a/examples/fs.go +++ b/examples/fs.go @@ -86,10 +86,14 @@ func (d *Datastore) GetSize(key ds.Key) (size int, err error) { func (d *Datastore) Delete(key ds.Key) (err error) { fn := d.KeyFilename(key) if !isFile(fn) { - return ds.ErrNotFound + return nil } - return os.Remove(fn) + err = os.Remove(fn) + if os.IsNotExist(err) { + err = nil // idempotent + } + return err } // Query implements Datastore.Query diff --git a/mount/mount.go b/mount/mount.go index 326277a..71ed8e8 100644 --- a/mount/mount.go +++ b/mount/mount.go @@ -216,7 +216,7 @@ func (d *Datastore) GetSize(key ds.Key) (size int, err error) { func (d *Datastore) Delete(key ds.Key) error { cds, _, k := d.lookup(key) if cds == nil { - return ds.ErrNotFound + return nil } return cds.Delete(k) } diff --git a/mount/mount_test.go b/mount/mount_test.go index 5a32772..3613536 100644 --- a/mount/mount_test.go +++ b/mount/mount_test.go @@ -171,8 +171,8 @@ func TestDeleteNotFound(t *testing.T) { }) err := m.Delete(datastore.NewKey("/quux/thud")) - if g, e := err, datastore.ErrNotFound; g != e { - t.Fatalf("expected ErrNotFound, got: %v\n", g) + if err != nil { + t.Fatalf("expected nil, got: %v\n", err) } }