Unverified Commit 7ed51b21 authored by ysaito1001's avatar ysaito1001 Committed by GitHub
Browse files

Add `CloneableLayer` that can be `Clone`d (#2784) 



## Motivation and Context
Alternative to #2776, making `Layer` cloneable so it can be stored in a
type that implements `Clone`.

## Description
In an attempt to reduce struct fields of a service config builder so it
only contains `Layer` (plus some exceptions such as `interceptors`), we
need to make `Layer` clone since the service config builder
`#[derive(Clone)]`.

There are two options. One, adopted by this PR, is to have a separate
`CloneableLayer` that primarily caters for the need for service config
builders. The other option is #2776, where we require that items in
`Layer` be `Clone`. Since the latter is rather a sledge hammer, changing
the requirement for all of the items to be stored, we will go for the
former, which is a more targeted approach and a two-way door if we want
to switch to the latter in the future.

## Testing
Added unit tests in `type_erase.rs` and `config_bag.rs`.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

---------

Co-authored-by: default avatarYuki Saito <awsaito@amazon.com>
Co-authored-by: default avatarZelda Hessler <zhessler@amazon.com>
parent d5a315fa

rust-runtime/aws-smithy-types/src/config_bag.rs

+170 −3
Original line number Diff line number Diff line
@@ -19,7 +19,7 @@ use std::borrow::Cow; 
use std::fmt::{Debug, Formatter};

use std::iter::Rev;

use std::marker::PhantomData;

use std::ops::Deref;

use std::ops::{Deref, DerefMut};

use std::slice::Iter;

use std::sync::Arc;
@@ -65,7 +65,7 @@ impl Deref for FrozenLayer { 


/// Private module to keep Value type while avoiding "private type in public latest"

pub(crate) mod value {

    #[derive(Debug)]

    #[derive(Clone, Debug)]

    pub enum Value<T> {

        Set(T),

        ExplicitlyUnset(&'static str),
@@ -79,6 +79,123 @@ impl<T: Default> Default for Value<T> { 
    }

}



/// [`CloneableLayer`] allows itself to be cloned. This is useful when a type that implements

/// `Clone` wishes to store a config layer.

///

/// It ensures that all the items in `CloneableLayer` are `Clone` upon entry, e.g. when they are

/// first stored, the mutable methods require that they have a `Clone` bound on them.

///

/// While [`FrozenLayer`] is also cloneable, which is a shallow clone via `Arc`, `CloneableLayer`

/// performs a deep clone that newly allocates all the items stored in it.

#[derive(Debug)]

pub struct CloneableLayer(Layer);



impl Deref for CloneableLayer {

    type Target = Layer;



    fn deref(&self) -> &Self::Target {

        &self.0

    }

}



impl DerefMut for CloneableLayer {

    fn deref_mut(&mut self) -> &mut Self::Target {

        &mut self.0

    }

}



impl Clone for CloneableLayer {

    fn clone(&self) -> Self {

        Self(

            self.try_clone()

                .expect("only cloneable types can be inserted"),

        )

    }

}



// We need to "override" the mutable methods to encode the information that an item being stored

// implements `Clone`. For the immutable methods, they can just be delegated via the `Deref` trait.

impl CloneableLayer {

    /// Creates a new `CloneableLayer` with a given name

    pub fn new(name: impl Into<Cow<'static, str>>) -> Self {

        Self(Layer::new(name))

    }



    pub fn freeze(self) -> FrozenLayer {

        self.0.into()

    }



    /// Removes `T` from this bag

    pub fn unset<T: Send + Sync + Clone + Debug + 'static>(&mut self) -> &mut Self {

        self.put_directly::<StoreReplace<T>>(Value::ExplicitlyUnset(type_name::<T>()));

        self

    }



    fn put_directly<T: Store>(&mut self, value: T::StoredType) -> &mut Self

    where

        T::StoredType: Clone,

    {

        self.props

            .insert(TypeId::of::<T>(), TypeErasedBox::new_with_clone(value));

        self

    }



    /// Stores `item` of type `T` into the config bag, overriding a previous value of the same type

    pub fn store_put<T>(&mut self, item: T) -> &mut Self

    where

        T: Storable<Storer = StoreReplace<T>> + Clone,

    {

        self.put_directly::<StoreReplace<T>>(Value::Set(item));

        self

    }



    /// Stores `item` of type `T` into the config bag, overriding a previous value of the same type,

    /// or unsets it by passing a `None`

    pub fn store_or_unset<T>(&mut self, item: Option<T>) -> &mut Self

    where

        T: Storable<Storer = StoreReplace<T>> + Clone,

    {

        let item = match item {

            Some(item) => Value::Set(item),

            None => Value::ExplicitlyUnset(type_name::<T>()),

        };

        self.put_directly::<StoreReplace<T>>(item);

        self

    }



    /// Stores `item` of type `T` into the config bag, appending it to the existing list of the same

    /// type

    pub fn store_append<T>(&mut self, item: T) -> &mut Self

    where

        T: Storable<Storer = StoreAppend<T>> + Clone,

    {

        match self.get_mut_or_default::<StoreAppend<T>>() {

            Value::Set(list) => list.push(item),

            v @ Value::ExplicitlyUnset(_) => *v = Value::Set(vec![item]),

        }

        self

    }



    /// Clears the value of type `T` from the config bag

    pub fn clear<T>(&mut self)

    where

        T: Storable<Storer = StoreAppend<T>> + Clone,

    {

        self.put_directly::<StoreAppend<T>>(Value::ExplicitlyUnset(type_name::<T>()));

    }



    fn get_mut_or_default<T: Send + Sync + Store + 'static>(&mut self) -> &mut T::StoredType

    where

        T::StoredType: Default + Clone,

    {

        self.props

            .entry(TypeId::of::<T>())

            .or_insert_with(|| TypeErasedBox::new_with_clone(T::StoredType::default()))

            .downcast_mut()

            .expect("typechecked")

    }

}



/// A named layer comprising a config bag

pub struct Layer {

    name: Cow<'static, str>,
@@ -101,6 +218,22 @@ impl Debug for Layer { 
}



impl Layer {

    fn try_clone(&self) -> Option<Self> {

        let new_props = self

            .props

            .iter()

            .flat_map(|(tyid, erased)| erased.try_clone().map(|e| (*tyid, e)))

            .collect::<TypeIdMap<_>>();

        if new_props.len() == self.props.len() {

            Some(Layer {

                name: self.name.clone(),

                props: new_props,

            })

        } else {

            None

        }

    }



    /// Inserts `value` into the layer directly

    fn put_directly<T: Store>(&mut self, value: T::StoredType) -> &mut Self {

        self.props
@@ -485,7 +618,7 @@ impl From<Layer> for FrozenLayer { 
#[cfg(test)]

mod test {

    use super::ConfigBag;

    use crate::config_bag::{Layer, Storable, StoreAppend, StoreReplace};

    use crate::config_bag::{CloneableLayer, Layer, Storable, StoreAppend, StoreReplace};



    #[test]

    fn layered_property_bag() {
@@ -672,4 +805,38 @@ mod test { 
        assert_eq!(bag.get_mut::<Foo>(), None);

        assert_eq!(bag.get_mut_or_default::<Foo>(), &Foo(0));

    }



    #[test]

    fn cloning_layers() {

        #[derive(Clone, Debug)]

        struct TestStr(String);

        impl Storable for TestStr {

            type Storer = StoreReplace<TestStr>;

        }

        let mut layer_1 = CloneableLayer::new("layer_1");

        let expected_str = "I can be cloned";

        layer_1.store_put(TestStr(expected_str.to_owned()));

        let layer_1_cloned = layer_1.clone();

        assert_eq!(expected_str, &layer_1_cloned.load::<TestStr>().unwrap().0);



        #[derive(Clone, Debug)]

        struct Rope(String);

        impl Storable for Rope {

            type Storer = StoreAppend<Rope>;

        }

        let mut layer_2 = CloneableLayer::new("layer_2");

        layer_2.store_append(Rope("A".to_owned()));

        layer_2.store_append(Rope("big".to_owned()));

        layer_2.store_append(Rope("rope".to_owned()));

        let layer_2_cloned = layer_2.clone();

        let rope = layer_2_cloned.load::<Rope>().cloned().collect::<Vec<_>>();

        assert_eq!(

            "A big rope",

            rope.iter()

                .rev()

                .map(|r| r.0.clone())

                .collect::<Vec<_>>()

                .join(" ")

        );

    }

}

rust-runtime/aws-smithy-types/src/type_erasure.rs

+50 −7
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ use std::error::Error as StdError; 
use std::fmt;

use std::marker::PhantomData;

use std::ops::{Deref, DerefMut};

use std::sync::Arc;



/// A [`TypeErasedBox`] with type information tracked via generics at compile-time

///
@@ -101,9 +102,11 @@ impl<T: fmt::Debug + Send + Sync + 'static> DerefMut for TypedBox<T> { 
pub struct TypeErasedBox {

    field: Box<dyn Any + Send + Sync>,

    #[allow(clippy::type_complexity)]

    debug: Box<

    debug: Arc<

        dyn Fn(&Box<dyn Any + Send + Sync>, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,

    >,

    #[allow(clippy::type_complexity)]

    clone: Option<Arc<dyn Fn(&Box<dyn Any + Send + Sync>) -> TypeErasedBox + Send + Sync>>,

}



#[cfg(feature = "test-util")]
@@ -132,14 +135,41 @@ impl TypeErasedBox { 
        };

        Self {

            field: Box::new(value),

            debug: Box::new(debug),

            debug: Arc::new(debug),

            clone: None,

        }

    }



    pub fn new_with_clone<T: Send + Sync + Clone + fmt::Debug + 'static>(value: T) -> Self {

        let debug = |value: &Box<dyn Any + Send + Sync>, f: &mut fmt::Formatter<'_>| {

            fmt::Debug::fmt(value.downcast_ref::<T>().expect("type-checked"), f)

        };

        let clone = |value: &Box<dyn Any + Send + Sync>| {

            TypeErasedBox::new(value.downcast_ref::<T>().expect("typechecked").clone())

        };

        Self {

            field: Box::new(value),

            debug: Arc::new(debug),

            clone: Some(Arc::new(clone)),

        }

    }



    pub fn try_clone(&self) -> Option<Self> {

        Some((self.clone.as_ref()?)(&self.field))

    }



    /// Downcast into a `Box<T>`, or return `Self` if it is not a `T`.

    pub fn downcast<T: fmt::Debug + Send + Sync + 'static>(self) -> Result<Box<T>, Self> {

        let TypeErasedBox { field, debug } = self;

        field.downcast().map_err(|field| Self { field, debug })

        let TypeErasedBox {

            field,

            debug,

            clone,

        } = self;

        field.downcast().map_err(|field| Self {

            field,

            debug,

            clone,

        })

    }



    /// Downcast as a `&T`, or return `None` if it is not a `T`.
@@ -158,6 +188,7 @@ impl From<TypeErasedError> for TypeErasedBox { 
        TypeErasedBox {

            field: value.field,

            debug: value.debug,

            clone: None,

        }

    }

}
@@ -166,7 +197,7 @@ impl From<TypeErasedError> for TypeErasedBox { 
pub struct TypeErasedError {

    field: Box<dyn Any + Send + Sync>,

    #[allow(clippy::type_complexity)]

    debug: Box<

    debug: Arc<

        dyn Fn(&Box<dyn Any + Send + Sync>, &mut fmt::Formatter<'_>) -> fmt::Result + Send + Sync,

    >,

    #[allow(clippy::type_complexity)]
@@ -200,7 +231,7 @@ impl TypeErasedError { 
        };

        Self {

            field: Box::new(value),

            debug: Box::new(debug),

            debug: Arc::new(debug),

            as_error: Box::new(|value: &TypeErasedError| {

                value.downcast_ref::<T>().expect("typechecked") as _

            }),
@@ -238,7 +269,7 @@ impl TypeErasedError { 


#[cfg(test)]

mod tests {

    use super::{TypeErasedError, TypedBox};

    use super::{TypeErasedBox, TypeErasedError, TypedBox};

    use std::fmt;



    #[derive(Debug)]
@@ -320,4 +351,16 @@ mod tests { 
            .unwrap();

        assert_eq!(test_err, actual);

    }



    #[test]

    fn test_typed_cloneable_boxes() {

        let expected_str = "I can be cloned";

        let cloneable = TypeErasedBox::new_with_clone(expected_str.to_owned());

        // ensure it can be cloned

        let cloned = cloneable.try_clone().unwrap();

        let actual_str = cloned.downcast_ref::<String>().unwrap();

        assert_eq!(expected_str, actual_str);

        // they should point to different addresses

        assert_ne!(format!("{expected_str:p}"), format! {"{actual_str:p}"});

    }

}