Skip to content

Commit

Permalink
PR feedback - AmqpOrderedMap::get takes a reference to key, not a cap…
Browse files Browse the repository at this point in the history 
…tured key; Removed several requirements on AmqpOrderedMap K and V
  • Loading branch information
@LarryOsterman
LarryOsterman committed Nov 7, 2024
1 parent 565b409 commit 094575b
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 54 deletions.
40 changes: 20 additions & 20 deletions sdk/core/azure_core_amqp/src/connection.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,32 +30,32 @@ impl AmqpConnectionOptions {
pub fn builder() -> builders::AmqpConnectionOptionsBuilder {
builders::AmqpConnectionOptionsBuilder::new()
}
pub fn max_frame_size(&self) -> Option<u32> {
self.max_frame_size
pub fn max_frame_size(&self) -> Option<&u32> {
self.max_frame_size.as_ref()
}
pub fn channel_max(&self) -> Option<u16> {
self.channel_max
pub fn channel_max(&self) -> Option<&u16> {
self.channel_max.as_ref()
}
pub fn idle_timeout(&self) -> Option<Duration> {
self.idle_timeout
pub fn idle_timeout(&self) -> Option<&Duration> {
self.idle_timeout.as_ref()
}
pub fn outgoing_locales(&self) -> Option<Vec<String>> {
self.outgoing_locales.clone()
pub fn outgoing_locales(&self) -> Option<&Vec<String>> {
self.outgoing_locales.as_ref()
}
pub fn incoming_locales(&self) -> Option<Vec<String>> {
self.incoming_locales.clone()
pub fn incoming_locales(&self) -> Option<&Vec<String>> {
self.incoming_locales.as_ref()
}
pub fn offered_capabilities(&self) -> Option<Vec<AmqpSymbol>> {
self.offered_capabilities.clone()
pub fn offered_capabilities(&self) -> Option<&Vec<AmqpSymbol>> {
self.offered_capabilities.as_ref()
}
pub fn desired_capabilities(&self) -> Option<Vec<AmqpSymbol>> {
self.desired_capabilities.clone()
pub fn desired_capabilities(&self) -> Option<&Vec<AmqpSymbol>> {
self.desired_capabilities.as_ref()
}
pub fn properties(&self) -> Option<AmqpOrderedMap<AmqpSymbol, AmqpValue>> {
self.properties.clone()
pub fn properties(&self) -> Option<&AmqpOrderedMap<AmqpSymbol, AmqpValue>> {
self.properties.as_ref()
}
pub fn buffer_size(&self) -> Option<usize> {
self.buffer_size
pub fn buffer_size(&self) -> Option<&usize> {
self.buffer_size.as_ref()
}
}

Expand Down Expand Up @@ -156,8 +156,8 @@ pub mod builders {
}
pub fn with_properties<K, V>(mut self, properties: impl Into<AmqpOrderedMap<K, V>>) -> Self
where
K: Into<AmqpSymbol> + Debug + Default + PartialEq,
V: Into<AmqpValue> + Debug + Default,
K: Into<AmqpSymbol> + Debug + Clone + PartialEq,
V: Into<AmqpValue> + Debug + Clone,
{
let properties_map: AmqpOrderedMap<K, V> = properties.into();
let properties_map = properties_map
Expand Down
6 changes: 4 additions & 2 deletions sdk/core/azure_core_amqp/src/receiver.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
);
}
Expand Down Expand Up @@ -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!(
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/azure_core_amqp/src/sender.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
);

Expand Down
6 changes: 3 additions & 3 deletions sdk/core/azure_core_amqp/src/session.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ pub mod builders {
}
pub fn with_properties<K, V>(mut self, properties: impl Into<AmqpOrderedMap<K, V>>) -> Self
where
K: Into<AmqpSymbol> + PartialEq + Default + Debug,
V: Into<AmqpValue> + Default,
K: Into<AmqpSymbol> + PartialEq + Clone + Debug,
V: Into<AmqpValue> + Clone,
{
let properties_map: AmqpOrderedMap<AmqpSymbol, AmqpValue> = properties
.into()
Expand Down Expand Up @@ -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())
);

Expand Down
36 changes: 18 additions & 18 deletions sdk/core/azure_core_amqp/src/value.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ impl From<std::time::SystemTime> for AmqpTimestamp {
#[derive(Debug, PartialEq, Clone, Default)]
pub struct AmqpOrderedMap<K, V>
where
K: PartialEq + Default,
V: Default,
K: PartialEq,
V: Clone,
{
inner: Vec<(K, V)>,
}
Expand Down Expand Up @@ -256,8 +256,8 @@ impl Deserializable<AmqpValue> for AmqpValue {

impl<K, V> AmqpOrderedMap<K, V>
where
K: PartialEq + Clone + Default,
V: Clone + Default,
K: PartialEq + Clone,
V: Clone,
{
pub fn new() -> Self {
Self { inner: Vec::new() }
Expand All @@ -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 {
Expand All @@ -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<Item = (K, V)> + '_ {
Expand All @@ -297,8 +297,8 @@ where

impl<K, V> IntoIterator for AmqpOrderedMap<K, V>
where
K: PartialEq + Default,
V: Default,
K: PartialEq,
V: Clone,
{
type Item = (K, V);
type IntoIter = <Vec<(K, V)> as IntoIterator>::IntoIter;
Expand Down Expand Up @@ -426,8 +426,8 @@ impl From<Box<AmqpValue>> for AmqpValue {

impl<K, V> From<Vec<(K, V)>> for AmqpOrderedMap<K, V>
where
K: PartialEq + Default,
V: Default,
K: PartialEq,
V: Clone,
{
fn from(v: Vec<(K, V)>) -> Self {
AmqpOrderedMap {
Expand All @@ -438,8 +438,8 @@ where

impl<K, V> FromIterator<(K, V)> for AmqpOrderedMap<K, V>
where
K: PartialEq + Default,
V: Default,
K: PartialEq,
V: Clone,
{
fn from_iter<I: IntoIterator<Item = (K, V)>>(iter: I) -> Self {
AmqpOrderedMap {
Expand Down Expand Up @@ -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)]
Expand Down
20 changes: 10 additions & 10 deletions sdk/eventhubs/azure_messaging_eventhubs/src/common/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<SystemTime> = Into::<AmqpTimestamp>::into(
response
.get(EVENTHUB_PROPERTY_CREATED_AT.into())
.get(&EVENTHUB_PROPERTY_CREATED_AT.into())
.ok_or_else(|| azure_core::Error::from(ErrorKind::InvalidManagementResponse))?
.clone(),
)
Expand All @@ -71,7 +71,7 @@ impl ManagementInstance {
// Into::<i32>::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 {
Expand Down Expand Up @@ -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::<AmqpTimestamp>::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(),
Expand Down

0 comments on commit 094575b

Please sign in to comment.