From 094575bb3d7a6ca4debd948a8d17cb979eac47e3 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 6 Nov 2024 14:31:45 -0800 Subject: [PATCH] PR feedback - AmqpOrderedMap::get takes a reference to key, not a captured key; Removed several requirements on AmqpOrderedMap K and V --- sdk/core/azure_core_amqp/src/connection.rs | 40 +++++++++---------- sdk/core/azure_core_amqp/src/receiver.rs | 6 ++- sdk/core/azure_core_amqp/src/sender.rs | 2 +- sdk/core/azure_core_amqp/src/session.rs | 6 +-- sdk/core/azure_core_amqp/src/value.rs | 36 ++++++++--------- .../src/common/mod.rs | 20 +++++----- 6 files changed, 56 insertions(+), 54 deletions(-) diff --git a/sdk/core/azure_core_amqp/src/connection.rs b/sdk/core/azure_core_amqp/src/connection.rs index ac2a91bc24..e73edd20f3 100644 --- a/sdk/core/azure_core_amqp/src/connection.rs +++ b/sdk/core/azure_core_amqp/src/connection.rs @@ -30,32 +30,32 @@ impl AmqpConnectionOptions { pub fn builder() -> builders::AmqpConnectionOptionsBuilder { builders::AmqpConnectionOptionsBuilder::new() } - pub fn max_frame_size(&self) -> Option { - self.max_frame_size + pub fn max_frame_size(&self) -> Option<&u32> { + self.max_frame_size.as_ref() } - pub fn channel_max(&self) -> Option { - self.channel_max + pub fn channel_max(&self) -> Option<&u16> { + self.channel_max.as_ref() } - pub fn idle_timeout(&self) -> Option { - self.idle_timeout + pub fn idle_timeout(&self) -> Option<&Duration> { + self.idle_timeout.as_ref() } - pub fn outgoing_locales(&self) -> Option> { - self.outgoing_locales.clone() + pub fn outgoing_locales(&self) -> Option<&Vec> { + self.outgoing_locales.as_ref() } - pub fn incoming_locales(&self) -> Option> { - self.incoming_locales.clone() + pub fn incoming_locales(&self) -> Option<&Vec> { + self.incoming_locales.as_ref() } - pub fn offered_capabilities(&self) -> Option> { - self.offered_capabilities.clone() + pub fn offered_capabilities(&self) -> Option<&Vec> { + self.offered_capabilities.as_ref() } - pub fn desired_capabilities(&self) -> Option> { - self.desired_capabilities.clone() + pub fn desired_capabilities(&self) -> Option<&Vec> { + self.desired_capabilities.as_ref() } - pub fn properties(&self) -> Option> { - self.properties.clone() + pub fn properties(&self) -> Option<&AmqpOrderedMap> { + self.properties.as_ref() } - pub fn buffer_size(&self) -> Option { - self.buffer_size + pub fn buffer_size(&self) -> Option<&usize> { + self.buffer_size.as_ref() } } @@ -156,8 +156,8 @@ pub mod builders { } pub fn with_properties(mut self, properties: impl Into>) -> Self where - K: Into + Debug + Default + PartialEq, - V: Into + Debug + Default, + K: Into + Debug + Clone + PartialEq, + V: Into + Debug + Clone, { let properties_map: AmqpOrderedMap = properties.into(); let properties_map = properties_map diff --git a/sdk/core/azure_core_amqp/src/receiver.rs b/sdk/core/azure_core_amqp/src/receiver.rs index 7cf16a5214..c76a84b0e6 100644 --- a/sdk/core/azure_core_amqp/src/receiver.rs +++ b/sdk/core/azure_core_amqp/src/receiver.rs @@ -215,7 +215,7 @@ mod tests { let properties = receiver_options.properties.clone().unwrap(); assert!(properties.contains_key(&AmqpSymbol::from("key").into())); assert_eq!( - *properties.get("key".into()).unwrap(), + *properties.get(&AmqpSymbol::from("key")).unwrap(), AmqpValue::String("value".to_string()) ); } @@ -303,7 +303,9 @@ mod tests { assert!(receiver_options.properties.is_some()); let properties_option = receiver_options.properties.unwrap(); assert_eq!( - *properties_option.get("combo_key".into()).unwrap(), + *properties_option + .get(&AmqpSymbol::from("combo_key")) + .unwrap(), AmqpValue::String("combo_value".to_string()) ); assert_eq!( diff --git a/sdk/core/azure_core_amqp/src/sender.rs b/sdk/core/azure_core_amqp/src/sender.rs index a8f8efeea5..312d06c43a 100644 --- a/sdk/core/azure_core_amqp/src/sender.rs +++ b/sdk/core/azure_core_amqp/src/sender.rs @@ -247,7 +247,7 @@ mod tests { let properties = sender_options.properties.clone().unwrap(); assert!(properties.contains_key(&AmqpSymbol::from("key"))); assert_eq!( - *properties.get("key".into()).unwrap(), + *properties.get(&AmqpSymbol::from("key")).unwrap(), AmqpValue::String("value".to_string()) ); diff --git a/sdk/core/azure_core_amqp/src/session.rs b/sdk/core/azure_core_amqp/src/session.rs index f1fbe730e4..9b021778f2 100644 --- a/sdk/core/azure_core_amqp/src/session.rs +++ b/sdk/core/azure_core_amqp/src/session.rs @@ -144,8 +144,8 @@ pub mod builders { } pub fn with_properties(mut self, properties: impl Into>) -> Self where - K: Into + PartialEq + Default + Debug, - V: Into + Default, + K: Into + PartialEq + Clone + Debug, + V: Into + Clone, { let properties_map: AmqpOrderedMap = properties .into() @@ -194,7 +194,7 @@ mod tests { let properties = session_options.properties.clone().unwrap(); assert!(properties.contains_key(&AmqpSymbol::from("key"))); assert_eq!( - *properties.get("key".into()).unwrap(), + *properties.get(&AmqpSymbol::from("key")).unwrap(), AmqpValue::String("value".to_string()) ); diff --git a/sdk/core/azure_core_amqp/src/value.rs b/sdk/core/azure_core_amqp/src/value.rs index b1760d7dcf..b8deb37693 100644 --- a/sdk/core/azure_core_amqp/src/value.rs +++ b/sdk/core/azure_core_amqp/src/value.rs @@ -99,8 +99,8 @@ impl From for AmqpTimestamp { #[derive(Debug, PartialEq, Clone, Default)] pub struct AmqpOrderedMap where - K: PartialEq + Default, - V: Default, + K: PartialEq, + V: Clone, { inner: Vec<(K, V)>, } @@ -256,8 +256,8 @@ impl Deserializable for AmqpValue { impl AmqpOrderedMap where - K: PartialEq + Clone + Default, - V: Clone + Default, + K: PartialEq + Clone, + V: Clone, { pub fn new() -> Self { Self { inner: Vec::new() } @@ -267,10 +267,10 @@ where self.inner.push((key, value)); } - pub fn get(&self, key: K) -> Option<&V> { + pub fn get(&self, key: &K) -> Option<&V> { self.inner .iter() - .find_map(|(k, v)| if *k == key.clone() { Some(v) } else { None }) + .find_map(|(k, v)| if k == key { Some(v) } else { None }) } pub fn len(&self) -> usize { @@ -287,7 +287,7 @@ where } pub fn contains_key(&self, key: &K) -> bool { - self.inner.iter().any(|(k, _)| *k == key.clone()) + self.inner.iter().any(|(k, _)| k == key) } pub fn iter(&self) -> impl Iterator + '_ { @@ -297,8 +297,8 @@ where impl IntoIterator for AmqpOrderedMap where - K: PartialEq + Default, - V: Default, + K: PartialEq, + V: Clone, { type Item = (K, V); type IntoIter = as IntoIterator>::IntoIter; @@ -426,8 +426,8 @@ impl From> for AmqpValue { impl From> for AmqpOrderedMap where - K: PartialEq + Default, - V: Default, + K: PartialEq, + V: Clone, { fn from(v: Vec<(K, V)>) -> Self { AmqpOrderedMap { @@ -438,8 +438,8 @@ where impl FromIterator<(K, V)> for AmqpOrderedMap where - K: PartialEq + Default, - V: Default, + K: PartialEq, + V: Clone, { fn from_iter>(iter: I) -> Self { AmqpOrderedMap { @@ -625,14 +625,14 @@ mod tests { map.insert("key2", 2); map.insert("key3", 3); - assert_eq!(map.get("key1"), Some(&1)); - assert_eq!(map.get("key2"), Some(&2)); - assert_eq!(map.get("key3"), Some(&3)); - assert_eq!(map.get("key4"), None); + assert_eq!(map.get(&"key1"), Some(&1)); + assert_eq!(map.get(&"key2"), Some(&2)); + assert_eq!(map.get(&"key3"), Some(&3)); + assert_eq!(map.get(&"key4"), None); assert_eq!(map.remove(&"key1"), Some(1)); assert_eq!(map.remove(&"key1"), None); - assert_eq!(map.get("key1"), None); + assert_eq!(map.get(&"key1"), None); } #[allow(clippy::approx_constant)] diff --git a/sdk/eventhubs/azure_messaging_eventhubs/src/common/mod.rs b/sdk/eventhubs/azure_messaging_eventhubs/src/common/mod.rs index c1dccfb609..36bb4218fa 100644 --- a/sdk/eventhubs/azure_messaging_eventhubs/src/common/mod.rs +++ b/sdk/eventhubs/azure_messaging_eventhubs/src/common/mod.rs @@ -56,13 +56,13 @@ impl ManagementInstance { return Err(ErrorKind::InvalidManagementResponse.into()); } let name: String = response - .get(EVENTHUB_PROPERTY_NAME.into()) + .get(&EVENTHUB_PROPERTY_NAME.into()) .ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))? .clone() .into(); let created_at: Option = Into::::into( response - .get(EVENTHUB_PROPERTY_CREATED_AT.into()) + .get(&EVENTHUB_PROPERTY_CREATED_AT.into()) .ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))? .clone(), ) @@ -71,7 +71,7 @@ impl ManagementInstance { // Into::::into(response.get("partition_count".to_string()).ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))?.clone()); let partition_ids = response - .get(EVENTHUB_PROPERTY_PARTITION_IDS.into()) + .get(&EVENTHUB_PROPERTY_PARTITION_IDS.into()) .ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))?; let partition_ids = match partition_ids { @@ -116,40 +116,40 @@ impl ManagementInstance { Ok(EventHubPartitionProperties { beginning_sequence_number: response - .get(EVENTHUB_PARTITION_PROPERTIES_BEGIN_SEQUENCE_NUMBER.into()) + .get(&EVENTHUB_PARTITION_PROPERTIES_BEGIN_SEQUENCE_NUMBER.into()) .ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))? .clone() .into(), id: response - .get(EVENTHUB_PROPERTY_PARTITION.into()) + .get(&EVENTHUB_PROPERTY_PARTITION.into()) .ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))? .clone() .into(), eventhub: response - .get(EVENTHUB_PROPERTY_NAME.into()) + .get(&EVENTHUB_PROPERTY_NAME.into()) .ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))? .clone() .into(), last_enqueued_sequence_number: response - .get(EVENTHUB_PARTITION_PROPERTIES_LAST_ENQUEUED_SEQUENCE_NUMBER.into()) + .get(&EVENTHUB_PARTITION_PROPERTIES_LAST_ENQUEUED_SEQUENCE_NUMBER.into()) .ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))? .clone() .into(), last_enqueued_offset: response - .get(EVENTHUB_PARTITION_PROPERTIES_LAST_ENQUEUED_OFFSET.into()) + .get(&EVENTHUB_PARTITION_PROPERTIES_LAST_ENQUEUED_OFFSET.into()) .ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))? .clone() .into(), last_enqueued_time_utc: Into::::into( response - .get(EVENTHUB_PARTITION_PROPERTIES_LAST_ENQUEUED_TIME_UTC.into()) + .get(&EVENTHUB_PARTITION_PROPERTIES_LAST_ENQUEUED_TIME_UTC.into()) .ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))? .clone(), ) .0, is_empty: response - .get(EVENTHUB_PARTITION_PROPERTIES_IS_EMPTY.into()) + .get(&EVENTHUB_PARTITION_PROPERTIES_IS_EMPTY.into()) .ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))? .clone() .into(),