diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 0afcaae4a3..f0ae279af4 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -10,3 +10,10 @@ # references = ["smithy-rs#920"] # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} # author = "rcoh" + +[[smithy-rs]] +message = "The `Debug` implementation for `PropertyBag` now prints a list of the types it contains. This significantly improves debuggability." +author = "rcoh" +references = ["smithy-rs#2612"] +meta = { "breaking" = false, "tada" = false, "bug" = false } + diff --git a/rust-runtime/aws-smithy-http/src/property_bag.rs b/rust-runtime/aws-smithy-http/src/property_bag.rs index 4964b7f09b..1ac6adc989 100644 --- a/rust-runtime/aws-smithy-http/src/property_bag.rs +++ b/rust-runtime/aws-smithy-http/src/property_bag.rs @@ -13,12 +13,38 @@ use std::any::{Any, TypeId}; use std::collections::HashMap; use std::fmt; -use std::fmt::Debug; +use std::fmt::{Debug, Formatter}; use std::hash::{BuildHasherDefault, Hasher}; use std::ops::{Deref, DerefMut}; use std::sync::{Arc, Mutex}; -type AnyMap = HashMap, BuildHasherDefault>; +type AnyMap = HashMap>; + +struct NamedType { + name: &'static str, + value: Box, +} + +impl NamedType { + fn as_mut(&mut self) -> Option<&mut T> { + self.value.downcast_mut() + } + + fn as_ref(&self) -> Option<&T> { + self.value.downcast_ref() + } + + fn assume(self) -> Option { + self.value.downcast().map(|t| *t).ok() + } + + fn new(value: T) -> Self { + Self { + name: std::any::type_name::(), + value: Box::new(value), + } + } +} // With TypeIds as keys, there's no need to hash them. They are already hashes // themselves, coming from the compiler. The IdHasher just holds the u64 of @@ -82,13 +108,8 @@ impl PropertyBag { /// ``` pub fn insert(&mut self, val: T) -> Option { self.map - .insert(TypeId::of::(), Box::new(val)) - .and_then(|boxed| { - (boxed as Box) - .downcast() - .ok() - .map(|boxed| *boxed) - }) + .insert(TypeId::of::(), NamedType::new(val)) + .and_then(|val| val.assume()) } /// Get a reference to a type previously inserted on this `PropertyBag`. @@ -106,7 +127,16 @@ impl PropertyBag { pub fn get(&self) -> Option<&T> { self.map .get(&TypeId::of::()) - .and_then(|boxed| (&**boxed as &(dyn Any + 'static)).downcast_ref()) + .and_then(|val| val.as_ref()) + } + + /// Returns an iterator of the types contained in this PropertyBag + /// + /// # Stability + /// This method is unstable and may be removed or changed in a future release. The exact + /// format of the returned types may also change. + pub fn contents(&self) -> impl Iterator + '_ { + self.map.values().map(|tpe| tpe.name) } /// Get a mutable reference to a type previously inserted on this `PropertyBag`. @@ -124,7 +154,7 @@ impl PropertyBag { pub fn get_mut(&mut self) -> Option<&mut T> { self.map .get_mut(&TypeId::of::()) - .and_then(|boxed| (&mut **boxed as &mut (dyn Any + 'static)).downcast_mut()) + .map(|val| val.as_mut().expect("type mismatch!")) } /// Remove a type from this `PropertyBag`. @@ -141,8 +171,8 @@ impl PropertyBag { /// assert!(props.get::().is_none()); /// ``` pub fn remove(&mut self) -> Option { - self.map.remove(&TypeId::of::()).and_then(|boxed| { - (boxed as Box) + self.map.remove(&TypeId::of::()).and_then(|tpe| { + (tpe.value as Box) .downcast() .ok() .map(|boxed| *boxed) @@ -168,7 +198,16 @@ impl PropertyBag { impl fmt::Debug for PropertyBag { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("PropertyBag").finish() + let mut fmt = f.debug_struct("PropertyBag"); + + struct Contents<'a>(&'a PropertyBag); + impl<'a> Debug for Contents<'a> { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.debug_list().entries(self.0.contents()).finish() + } + } + fmt.field("contents", &Contents(self)); + fmt.finish() } } @@ -225,22 +264,30 @@ impl From for SharedPropertyBag { } #[cfg(test)] -#[test] -fn test_extensions() { - #[derive(Debug, PartialEq)] - struct MyType(i32); +mod test { + use crate::property_bag::PropertyBag; - let mut extensions = PropertyBag::new(); + #[test] + fn test_extensions() { + #[derive(Debug, PartialEq)] + struct MyType(i32); - extensions.insert(5i32); - extensions.insert(MyType(10)); + let mut property_bag = PropertyBag::new(); - assert_eq!(extensions.get(), Some(&5i32)); - assert_eq!(extensions.get_mut(), Some(&mut 5i32)); + property_bag.insert(5i32); + property_bag.insert(MyType(10)); - assert_eq!(extensions.remove::(), Some(5i32)); - assert!(extensions.get::().is_none()); + assert_eq!(property_bag.get(), Some(&5i32)); + assert_eq!(property_bag.get_mut(), Some(&mut 5i32)); - assert_eq!(extensions.get::(), None); - assert_eq!(extensions.get(), Some(&MyType(10))); + assert_eq!(property_bag.remove::(), Some(5i32)); + assert!(property_bag.get::().is_none()); + + assert_eq!(property_bag.get::(), None); + assert_eq!(property_bag.get(), Some(&MyType(10))); + assert_eq!( + format!("{:?}", property_bag), + r#"PropertyBag { contents: ["aws_smithy_http::property_bag::test::test_extensions::MyType"] }"# + ); + } } diff --git a/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs b/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs index bf8d42e6cb..418fbd9b13 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/config_bag.rs @@ -10,7 +10,7 @@ //! 1. A new layer of configuration may be applied onto an existing configuration structure without modifying it or taking ownership. //! 2. No lifetime shenanigans to deal with use aws_smithy_http::property_bag::PropertyBag; -use std::fmt::Debug; +use std::fmt::{Debug, Formatter}; use std::ops::Deref; use std::sync::Arc; @@ -18,12 +18,32 @@ use std::sync::Arc; /// /// [`ConfigBag`] is the "unlocked" form of the bag. Only the top layer of the bag may be unlocked. #[must_use] -#[derive(Debug)] pub struct ConfigBag { head: Layer, tail: Option, } +impl Debug for ConfigBag { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + struct Layers<'a>(&'a ConfigBag); + impl Debug for Layers<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let mut list = f.debug_list(); + list.entry(&self.0.head); + let mut us = self.0.tail.as_ref(); + while let Some(bag) = us { + list.entry(&bag.head); + us = bag.tail.as_ref() + } + list.finish() + } + } + f.debug_struct("ConfigBag") + .field("layers", &Layers(self)) + .finish() + } +} + /// Layered Configuration Structure /// /// [`FrozenConfigBag`] is the "locked" form of the bag. @@ -55,12 +75,26 @@ enum Value { ExplicitlyUnset, } -#[derive(Debug)] struct Layer { name: &'static str, props: PropertyBag, } +impl Debug for Layer { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + struct Contents<'a>(&'a Layer); + impl Debug for Contents<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_list().entries(self.0.props.contents()).finish() + } + } + f.debug_struct("Layer") + .field("name", &self.name) + .field("properties", &Contents(self)) + .finish() + } +} + fn no_op(_: &mut ConfigBag) {} impl FrozenConfigBag { @@ -258,6 +292,7 @@ mod test { assert!(final_bag.get::().is_some()); // we unset prop3 assert!(final_bag.get::().is_none()); + println!("{:#?}", final_bag); } #[test]