test: add test for impl From<HashMap> for Extensions, fix bug in ExtWrapper

Author: waynr

object_store/src/extensions.rs

@@ -53,50 +53,11 @@ impl Extensions {
         self.inner.insert(id, a);
     }
 
-    fn set_ext_wrapped<T: Any + Send + Sync + std::fmt::Debug + 'static>(&mut self, t: T) {
-        let id = TypeId::of::<T>();
-        //println!("inserting: {id:?}: {}", std::any::type_name::<T>());
-        let a = Arc::new(ExtWrapper::new(t));
-        self.inner.insert(id, a);
-    }
-
     pub(crate) fn get_ext<T: Any + Send + Sync + 'static>(&self) -> Option<Arc<T>> {
         let id = TypeId::of::<T>();
-        // println!(
-        //     "looking for        : {id:?}: {}",
-        //     std::any::type_name::<T>()
-        // );
         self.inner.get(&id).map(|e| {
-            // let id = e.type_id();
-            // println!(
-            //     "found value        : {id:?}: {}",
-            //     std::any::type_name_of_val(e)
-            // );
-            let v = Arc::clone(&e);
-            // let id = v.type_id();
-            // println!(
-            //     "cloned value       : {id:?}: {}",
-            //     std::any::type_name_of_val(&v)
-            // );
-            let v = v.as_any();
-            // let id = v.type_id();
-            // println!(
-            //     "as_any value       : {id:?}: {}",
-            //     std::any::type_name_of_val(&v)
-            // );
-            let v: Arc<T> =
-                Arc::downcast::<T>(v).expect("must be able to downcast to type if found in map");
-            // let id = v.type_id();
-            // println!(
-            //     "found value        : {id:?}: {}",
-            //     std::any::type_name_of_val(&v)
-            // );
-            // let id = v.as_ref().type_id();
-            // println!(
-            //     "found value (inner): {id:?}: {}",
-            //     std::any::type_name_of_val(v.as_ref())
-            // );
-            v
+            let v = Arc::clone(&e).as_any();
+            Arc::downcast::<T>(v).unwrap()
         })
     }
 }
@@ -105,7 +66,7 @@ impl From<HashMap<TypeId, Ext>> for Extensions {
     fn from(other: HashMap<TypeId, Ext>) -> Self {
         let mut s = Self::default();
         for (k, v) in other {
-            let v = Arc::new(ExtWrapper::new(v));
+            let v = Arc::new(ExtWrapper { inner: v });
             s.inner.insert(k, v);
         }
         s
@@ -120,18 +81,17 @@ impl From<HashMap<TypeId, Ext>> for Extensions {
 /// trait objects; because [`std::cmp::PartialEq`] has `Self` as a generic type parameter, the type
 /// system won't let us set it as a trait bound on incoming trait objects when constructing
 /// ExtWrapper.
-#[derive(Debug)]
-struct ExtWrapper<T> {
-    inner: Arc<T>,
+struct ExtWrapper {
+    inner: Arc<dyn Any + Send + Sync + 'static>,
 }
 
-impl<T: std::fmt::Debug + Send + Sync> ExtWrapper<T> {
-    fn new(v: T) -> Self {
-        Self { inner: Arc::new(v) }
+impl std::fmt::Debug for ExtWrapper {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "ExtWrapper2")
     }
 }
 
-impl<T: std::fmt::Debug + Send + Sync + 'static> Extension for ExtWrapper<T> {
+impl Extension for ExtWrapper {
     fn as_any(self: Arc<Self>) -> Ext {
         self.inner.clone()
     }
@@ -159,20 +119,8 @@ mod test {
         }
 
         fn partial_eq(self: Arc<Self>, other: Ext) -> bool {
-            let self_id = self.type_id();
-            let other_id = other.type_id();
-            println!("MyExt.partial_eq");
-            println!(
-                "  self  : {self_id:?}, {}",
-                std::any::type_name_of_val(self.as_ref())
-            );
-            println!(
-                "  other : {other_id:?}, {}",
-                std::any::type_name_of_val(other.as_ref())
-            );
             other
                 .downcast_ref::<Self>()
-                .inspect(|v| println!("{v:?}"))
                 .map(|other| self.x == other.x)
                 .unwrap_or_default()
         }
@@ -183,8 +131,6 @@ mod test {
         let mut exts1 = Extensions::default();
         let myext1 = MyExt { x: 0 };
         exts1.set_ext(myext1);
-        let t1 = TypeId::of::<MyExt>();
-        println!("type id of MyExt: {t1:?}");
 
         let mut exts2 = Extensions::default();
         let myext2 = MyExt { x: 1 };
@@ -205,6 +151,28 @@ mod test {
         );
     }
 
+    impl Extensions {
+        fn set_ext_wrapped<T: Any + Send + Sync + std::fmt::Debug + 'static>(&mut self, t: T) {
+            let id = TypeId::of::<T>();
+            let a = Arc::new(ExtWrapper { inner: Arc::new(t) });
+            self.inner.insert(id, a);
+        }
+    }
+
+    #[test]
+    fn not_equal_if_missing_entry() {
+        let mut exts1 = Extensions::default();
+        exts1.set_ext_wrapped(0);
+        exts1.set_ext_wrapped(String::from("meow"));
+
+        let mut exts2 = Extensions::default();
+        exts2.set_ext_wrapped(1);
+
+        assert_ne!(
+            exts1, exts2,
+            "two different Extensions cannot be equal if they don't carry the same types"
+        );
+    }
     #[test]
     fn equality_ext_wrapper() {
         let mut exts1 = Extensions::default();
@@ -248,14 +216,37 @@ mod test {
     fn one_instance_of_same_type_ext_wrapper() {
         let mut exts = Extensions::default();
 
-        println!("validate ExtWrapper<i32> with value 0");
+        println!("validate ExtWrapper with value 0");
         exts.set_ext_wrapped(0i32);
         let expected = exts.get_ext::<i32>().expect("must get a value");
         assert_eq!(0, *expected, "return the same instance we just added");
 
-        println!("validate replacing previous ExtWrapper<i32> with value 1");
+        println!("validate replacing previous ExtWrapper with value 1");
         exts.set_ext_wrapped(1i32);
         let expected = exts.get_ext::<i32>().expect("must get a value");
         assert_eq!(1, *expected, "return the same instance we just added");
     }
+
+    #[test]
+    fn from_hashmap_of_exts() {
+        let mut map: HashMap<TypeId, Ext> = HashMap::new();
+
+        let v = 0i32;
+        let id = v.type_id();
+        assert!(map.insert(id, Arc::new(v)).is_none());
+
+        let s: String = "meow".to_string();
+        let id = s.type_id();
+        assert!(map.insert(id, Arc::new(s)).is_none());
+
+        assert!(map.len() == 2);
+
+        let exts: Extensions = map.into();
+
+        let v = exts.get_ext::<i32>().expect("must get a value");
+        assert_eq!(0i32, *v.as_ref());
+
+        let v = exts.get_ext::<String>().expect("must get a value");
+        assert_eq!(String::from("meow"), *v.as_ref());
+    }
 }

object_store/src/lib.rs

@@ -965,9 +965,8 @@ pub struct GetOptions {
     /// <https://datatracker.ietf.org/doc/html/rfc9110#name-head>
     pub head: bool,
 
-    /// Implementation-specific extensions.  but
-    /// intended for use by [`ObjectStore`] implementations that need to pass context-specific
-    /// information (like tracing spans) via trait methods.
+    /// Implementation-specific extensions. Intended for use by [`ObjectStore`] implementations
+    /// that need to pass context-specific information (like tracing spans) via trait methods.
     ///
     /// These extensions are ignored entirely by backends offered through this crate.
     pub extensions: Extensions,