Merge pull request #132 from ipfs/feat/idempotent-delete

feat: make delete idempotent

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)
 		}

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
 }

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)
 		}

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

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

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)
 }

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)
 	}
 }