feat(tags api)!: return number of tags removed by delete operations (#178)

## Description

It's a huge footgun to silently return `Ok(())`, even when no tags are
actually removed. Returning the number of tags removed. Also adds
documentation calling out this behaviour.

## Breaking Changes

breaks the return value of the tags API

## Notes & open questions

Should we do this with rename, and other mutating operations?

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [ ] Tests if relevant.
- [ ] All breaking changes documented.

---------

Co-authored-by: Ruediger Klaehn <[email protected]>

Author: b5

src/api/proto.rs

@@ -118,7 +118,7 @@ pub enum Request {
     ListTags(ListTagsRequest),
     #[rpc(tx = oneshot::Sender<super::Result<()>>)]
     SetTag(SetTagRequest),
-    #[rpc(tx = oneshot::Sender<super::Result<()>>)]
+    #[rpc(tx = oneshot::Sender<super::Result<u64>>)]
     DeleteTags(DeleteTagsRequest),
     #[rpc(tx = oneshot::Sender<super::Result<()>>)]
     RenameTag(RenameTagRequest),

src/api/tags.rs

@@ -107,21 +107,28 @@ impl Tags {
         self.list_with_opts(ListOptions::hash_seq()).await
     }
 
-    /// Deletes a tag.
-    pub async fn delete_with_opts(&self, options: DeleteOptions) -> super::RequestResult<()> {
+    /// Deletes a tag, with full control over options. All other delete methods
+    /// wrap this.
+    ///
+    /// Returns the number of tags actually removed. Attempting to delete a non-existent tag will *not* fail.
+    pub async fn delete_with_opts(&self, options: DeleteOptions) -> super::RequestResult<u64> {
         trace!("{:?}", options);
-        self.client.rpc(options).await??;
-        Ok(())
+        let deleted = self.client.rpc(options).await??;
+        Ok(deleted)
     }
 
     /// Deletes a tag.
-    pub async fn delete(&self, name: impl AsRef<[u8]>) -> super::RequestResult<()> {
+    ///
+    /// Returns the number of tags actually removed. Attempting to delete a non-existent tag will *not* fail.
+    pub async fn delete(&self, name: impl AsRef<[u8]>) -> super::RequestResult<u64> {
         self.delete_with_opts(DeleteOptions::single(name.as_ref()))
             .await
     }
 
     /// Deletes a range of tags.
-    pub async fn delete_range<R, E>(&self, range: R) -> super::RequestResult<()>
+    ///
+    /// Returns the number of tags actually removed. Attempting to delete a non-existent tag will *not* fail.
+    pub async fn delete_range<R, E>(&self, range: R) -> super::RequestResult<u64>
     where
         R: RangeBounds<E>,
         E: AsRef<[u8]>,
@@ -130,13 +137,17 @@ impl Tags {
     }
 
     /// Delete all tags with the given prefix.
-    pub async fn delete_prefix(&self, prefix: impl AsRef<[u8]>) -> super::RequestResult<()> {
+    ///
+    /// Returns the number of tags actually removed. Attempting to delete a non-existent tag will *not* fail.
+    pub async fn delete_prefix(&self, prefix: impl AsRef<[u8]>) -> super::RequestResult<u64> {
         self.delete_with_opts(DeleteOptions::prefix(prefix.as_ref()))
             .await
     }
 
     /// Delete all tags. Use with care. After this, all data will be garbage collected.
-    pub async fn delete_all(&self) -> super::RequestResult<()> {
+    ///
+    /// Returns the number of tags actually removed. Attempting to delete a non-existent tag will *not* fail.
+    pub async fn delete_all(&self) -> super::RequestResult<u64> {
         self.delete_with_opts(DeleteOptions {
             from: None,
             to: None,

src/store/fs/meta.rs

@@ -631,10 +631,12 @@ impl Actor {
             .extract_from_if((from, to), |_, _| true)
             .context(StorageSnafu)?;
         // drain the iterator to actually remove the tags
+        let mut deleted = 0;
         for res in removing {
             res.context(StorageSnafu)?;
+            deleted += 1;
         }
-        tx.send(Ok(())).await.ok();
+        tx.send(Ok(deleted)).await.ok();
         Ok(())
     }
 

src/store/mem.rs

@@ -227,6 +227,7 @@ impl Actor {
                 info!("deleting tags from {:?} to {:?}", from, to);
                 // state.tags.remove(&from.unwrap());
                 // todo: more efficient impl
+                let mut deleted = 0;
                 self.state.tags.retain(|tag, _| {
                     if let Some(from) = &from {
                         if tag < from {
@@ -239,9 +240,10 @@ impl Actor {
                         }
                     }
                     info!("    removing {:?}", tag);
+                    deleted += 1;
                     false
                 });
-                tx.send(Ok(())).await.ok();
+                tx.send(Ok(deleted)).await.ok();
             }
             Command::RenameTag(cmd) => {
                 let RenameTagMsg {