From 65cf6886eca1930b42de043f927a2c2cd5d518cc Mon Sep 17 00:00:00 2001
From: Floris Bruynooghe <flub@n0.computer>
Date: Mon, 13 Jan 2025 11:36:03 +0100
Subject: [PATCH] fix(iroh): Implement Clone for StaticProvider discovery
 (#3108)

## Description

This is not very useful without Clone, it already is Arc<RwLock<T>> on
the inside so this clearly was intended. Write a test demonstrating why
this is needed.

## Breaking Changes

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [x] Self-review.
- [x] 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.
- [x] Tests if relevant.
- [x] All breaking changes documented.
---
 iroh/src/discovery.rs                 | 16 ++++++++++
 iroh/src/discovery/static_provider.rs | 46 ++++++++++++++++++++++++++-
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/iroh/src/discovery.rs b/iroh/src/discovery.rs
index b37cae1522..90627e99a1 100644
--- a/iroh/src/discovery.rs
+++ b/iroh/src/discovery.rs
@@ -448,6 +448,7 @@ mod tests {
     use anyhow::Context;
     use iroh_base::SecretKey;
     use rand::Rng;
+    use testresult::TestResult;
     use tokio_util::task::AbortOnDropHandle;
 
     use super::*;
@@ -734,6 +735,21 @@ mod tests {
             .expect("time drift")
             .as_micros() as u64
     }
+
+    #[tokio::test]
+    async fn test_arc_discovery() -> TestResult {
+        let discovery = Arc::new(EmptyDiscovery);
+
+        let _ep = Endpoint::builder()
+            .add_discovery({
+                let discovery = discovery.clone();
+                move |_| Some(discovery)
+            })
+            .bind()
+            .await?;
+
+        Ok(())
+    }
 }
 
 /// This module contains end-to-end tests for DNS node discovery.
diff --git a/iroh/src/discovery/static_provider.rs b/iroh/src/discovery/static_provider.rs
index 34cb76f480..12f1d14871 100644
--- a/iroh/src/discovery/static_provider.rs
+++ b/iroh/src/discovery/static_provider.rs
@@ -12,7 +12,7 @@ use iroh_base::{NodeAddr, NodeId, RelayUrl};
 use super::{Discovery, DiscoveryItem};
 
 /// A static discovery implementation that allows providing info for nodes manually.
-#[derive(Debug, Default)]
+#[derive(Debug, Default, Clone)]
 #[repr(transparent)]
 pub struct StaticProvider {
     nodes: Arc<RwLock<BTreeMap<NodeId, NodeInfo>>>,
@@ -170,3 +170,47 @@ impl Discovery for StaticProvider {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use anyhow::Context;
+    use iroh_base::SecretKey;
+    use testresult::TestResult;
+
+    use super::*;
+    use crate::Endpoint;
+
+    #[tokio::test]
+    async fn test_basic() -> TestResult {
+        let discovery = StaticProvider::new();
+
+        let _ep = Endpoint::builder()
+            .add_discovery({
+                let discovery = discovery.clone();
+                move |_| Some(discovery)
+            })
+            .bind()
+            .await?;
+
+        let key = SecretKey::from_bytes(&[0u8; 32]);
+        let addr = NodeAddr {
+            node_id: key.public(),
+            relay_url: Some("https://example.com".parse()?),
+            direct_addresses: Default::default(),
+        };
+        discovery.add_node_addr(addr.clone());
+
+        let back = discovery.get_node_addr(key.public()).context("no addr")?;
+
+        assert_eq!(back, addr);
+
+        let removed = discovery
+            .remove_node_addr(key.public())
+            .context("nothing removed")?;
+        assert_eq!(removed, addr);
+        let res = discovery.get_node_addr(key.public());
+        assert!(res.is_none());
+
+        Ok(())
+    }
+}