Page MenuHomePhabricator
Files F3403978

D10762.diff
No OneTemporary
Actions

D10762.diff
View Options

diff --git a/services/identity/src/database.rs b/services/identity/src/database.rs
--- a/services/identity/src/database.rs
+++ b/services/identity/src/database.rs
@@ -48,7 +48,6 @@
USERS_TABLE_USERNAME_ATTRIBUTE, USERS_TABLE_USERNAME_INDEX,
USERS_TABLE_WALLET_ADDRESS_ATTRIBUTE, USERS_TABLE_WALLET_ADDRESS_INDEX,
};
-use crate::error::AttributeValueFromHashMap;
use crate::id::generate_uuid;
use crate::nonce::NonceData;
use crate::token::{AccessTokenData, AuthType};
@@ -423,17 +422,14 @@
// Attempt to delete the one-time keys individually, a successful delete
// mints the one-time key to the requester
for item in item_vec {
- let pk = item.get_string(otk_table::PARTITION_KEY)?;
- let otk = item.get_string(otk_table::SORT_KEY)?;
+ let pk: String = item.get_attr(otk_table::PARTITION_KEY)?;
+ let otk: String = item.get_attr(otk_table::SORT_KEY)?;
let composite_key = HashMap::from([
- (
- otk_table::PARTITION_KEY.to_string(),
- AttributeValue::S(pk.to_string()),
- ),
+ (otk_table::PARTITION_KEY.to_string(), AttributeValue::S(pk)),
(
otk_table::SORT_KEY.to_string(),
- AttributeValue::S(otk.to_string()),
+ AttributeValue::S(otk.clone()),
),
]);
@@ -447,7 +443,7 @@
.await
{
Ok(_) => {
- result = Some(otk.to_string());
+ result = Some(otk);
break;
}
// This err should only happen if a delete occurred between the read
diff --git a/services/identity/src/database/device_list.rs b/services/identity/src/database/device_list.rs
--- a/services/identity/src/database/device_list.rs
+++ b/services/identity/src/database/device_list.rs
@@ -23,8 +23,7 @@
USERS_TABLE, USERS_TABLE_DEVICELIST_TIMESTAMP_ATTRIBUTE_NAME,
USERS_TABLE_PARTITION_KEY,
},
- ddb_utils::AttributesOptionExt,
- error::{DeviceListError, Error, FromAttributeValue},
+ error::{DeviceListError, Error},
grpc_services::protos::{self, unauth::DeviceType},
grpc_utils::DeviceKeysInfo,
};
@@ -192,24 +191,15 @@
})?;
let device_key_info = attrs
- .remove(ATTR_DEVICE_KEY_INFO)
- .ok_or_missing(ATTR_DEVICE_KEY_INFO)?
- .to_hashmap(ATTR_DEVICE_KEY_INFO)
- .cloned()
+ .take_attr::<AttributeMap>(ATTR_DEVICE_KEY_INFO)
.and_then(IdentityKeyInfo::try_from)?;
let content_prekey = attrs
- .remove(ATTR_CONTENT_PREKEY)
- .ok_or_missing(ATTR_CONTENT_PREKEY)?
- .to_hashmap(ATTR_CONTENT_PREKEY)
- .cloned()
+ .take_attr::<AttributeMap>(ATTR_CONTENT_PREKEY)
.and_then(PreKey::try_from)?;
let notif_prekey = attrs
- .remove(ATTR_NOTIF_PREKEY)
- .ok_or_missing(ATTR_NOTIF_PREKEY)?
- .to_hashmap(ATTR_NOTIF_PREKEY)
- .cloned()
+ .take_attr::<AttributeMap>(ATTR_NOTIF_PREKEY)
.and_then(PreKey::try_from)?;
let code_version = attrs
@@ -302,10 +292,7 @@
let key_payload = attrs.take_attr(ATTR_KEY_PAYLOAD)?;
let key_payload_signature = attrs.take_attr(ATTR_KEY_PAYLOAD_SIGNATURE)?;
// social proof is optional
- let social_proof = attrs
- .remove(ATTR_SOCIAL_PROOF)
- .map(|attr| attr.to_string(ATTR_SOCIAL_PROOF).cloned())
- .transpose()?;
+ let social_proof: Option<String> = attrs.take_attr(ATTR_SOCIAL_PROOF)?;
Ok(Self {
key_payload,
diff --git a/services/identity/src/ddb_utils.rs b/services/identity/src/ddb_utils.rs
--- a/services/identity/src/ddb_utils.rs
+++ b/services/identity/src/ddb_utils.rs
@@ -6,8 +6,6 @@
use std::collections::HashMap;
use std::iter::IntoIterator;
-use comm_lib::database::{DBItemAttributeError, DBItemError};
-
use crate::constants::{
USERS_TABLE_DEVICES_MAP_SOCIAL_PROOF_ATTRIBUTE_NAME,
USERS_TABLE_USERNAME_ATTRIBUTE, USERS_TABLE_WALLET_ADDRESS_ATTRIBUTE,
@@ -68,26 +66,6 @@
.collect()
}
-pub trait AttributesOptionExt<T> {
- fn ok_or_missing(
- self,
- attr_name: impl Into<String>,
- ) -> Result<T, DBItemError>;
-}
-
-impl<T> AttributesOptionExt<T> for Option<T> {
- fn ok_or_missing(
- self,
- attr_name: impl Into<String>,
- ) -> Result<T, DBItemError> {
- self.ok_or_else(|| DBItemError {
- attribute_name: attr_name.into(),
- attribute_value: None.into(),
- attribute_error: DBItemAttributeError::Missing,
- })
- }
-}
-
pub trait DateTimeExt {
fn from_utc_timestamp_millis(timestamp: i64) -> Option<DateTime<Utc>>;
}
@@ -95,7 +73,7 @@
impl DateTimeExt for DateTime<Utc> {
fn from_utc_timestamp_millis(timestamp: i64) -> Option<Self> {
let naive = NaiveDateTime::from_timestamp_millis(timestamp)?;
- Some(Self::from_utc(naive, Utc))
+ Some(Self::from_naive_utc_and_offset(naive, Utc))
}
}
diff --git a/services/identity/src/error.rs b/services/identity/src/error.rs
--- a/services/identity/src/error.rs
+++ b/services/identity/src/error.rs
@@ -1,7 +1,5 @@
-use comm_lib::aws::ddb::types::AttributeValue;
use comm_lib::aws::DynamoDBError;
-use comm_lib::database::{DBItemAttributeError, DBItemError, Value};
-use std::collections::hash_map::HashMap;
+use comm_lib::database::DBItemError;
use tracing::error;
#[derive(
@@ -31,94 +29,6 @@
ConcurrentUpdateError,
}
-#[deprecated(note = "Use comm-lib traits instead")]
-pub trait FromAttributeValue {
- fn to_vec(
- &self,
- attr_name: &str,
- ) -> Result<&Vec<AttributeValue>, DBItemError>;
- fn to_string(&self, attr_name: &str) -> Result<&String, DBItemError>;
- fn to_hashmap(
- &self,
- attr_name: &str,
- ) -> Result<&HashMap<String, AttributeValue>, DBItemError>;
-}
-
-fn handle_attr_failure(value: &AttributeValue, attr_name: &str) -> DBItemError {
- DBItemError {
- attribute_name: attr_name.to_string(),
- attribute_value: Value::AttributeValue(Some(value.clone())),
- attribute_error: DBItemAttributeError::IncorrectType,
- }
-}
-
-impl FromAttributeValue for AttributeValue {
- fn to_vec(
- &self,
- attr_name: &str,
- ) -> Result<&Vec<AttributeValue>, DBItemError> {
- self.as_l().map_err(|e| handle_attr_failure(e, attr_name))
- }
-
- fn to_string(&self, attr_name: &str) -> Result<&String, DBItemError> {
- self.as_s().map_err(|e| handle_attr_failure(e, attr_name))
- }
-
- fn to_hashmap(
- &self,
- attr_name: &str,
- ) -> Result<&HashMap<String, AttributeValue>, DBItemError> {
- self.as_m().map_err(|e| handle_attr_failure(e, attr_name))
- }
-}
-
-pub trait AttributeValueFromHashMap {
- fn get_string(&self, key: &str) -> Result<&String, DBItemError>;
- fn get_map(
- &self,
- key: &str,
- ) -> Result<&HashMap<String, AttributeValue>, DBItemError>;
- fn get_vec(&self, key: &str) -> Result<&Vec<AttributeValue>, DBItemError>;
-}
-
-impl AttributeValueFromHashMap for HashMap<String, AttributeValue> {
- fn get_string(&self, key: &str) -> Result<&String, DBItemError> {
- self
- .get(key)
- .ok_or(DBItemError {
- attribute_name: key.to_string(),
- attribute_value: None.into(),
- attribute_error: DBItemAttributeError::Missing,
- })?
- .to_string(key)
- }
-
- fn get_map(
- &self,
- key: &str,
- ) -> Result<&HashMap<String, AttributeValue>, DBItemError> {
- self
- .get(key)
- .ok_or(DBItemError {
- attribute_name: key.to_string(),
- attribute_value: None.into(),
- attribute_error: DBItemAttributeError::Missing,
- })?
- .to_hashmap(key)
- }
-
- fn get_vec(&self, key: &str) -> Result<&Vec<AttributeValue>, DBItemError> {
- self
- .get(key)
- .ok_or(DBItemError {
- attribute_name: key.to_string(),
- attribute_value: None.into(),
- attribute_error: DBItemAttributeError::Missing,
- })?
- .to_vec(key)
- }
-}
-
pub fn consume_error<T>(result: Result<T, Error>) {
match result {
Ok(_) => (),
diff --git a/shared/comm-lib/src/database.rs b/shared/comm-lib/src/database.rs
--- a/shared/comm-lib/src/database.rs
+++ b/shared/comm-lib/src/database.rs
@@ -157,6 +157,30 @@
}
}
+// this allows us to get optional attributes
+impl<T> TryFromAttribute for Option<T>
+where
+ T: TryFromAttribute,
+{
+ fn try_from_attr(
+ attribute_name: impl Into<String>,
+ attribute: Option<AttributeValue>,
+ ) -> Result<Self, DBItemError> {
+ if attribute.is_none() {
+ return Ok(None);
+ }
+
+ match T::try_from_attr(attribute_name, attribute) {
+ Ok(value) => Ok(Some(value)),
+ Err(DBItemError {
+ attribute_error: DBItemAttributeError::Missing,
+ ..
+ }) => Ok(None),
+ Err(error) => Err(error),
+ }
+ }
+}
+
impl TryFromAttribute for String {
fn try_from_attr(
attribute_name: impl Into<String>,
@@ -817,4 +841,20 @@
DBItemAttributeError::TimestampOutOfRange
));
}
+
+ #[test]
+ fn test_optional_attribute() {
+ let mut attrs = AttributeMap::from([(
+ "foo".to_string(),
+ AttributeValue::S("bar".to_string()),
+ )]);
+
+ let foo: Option<String> =
+ attrs.take_attr("foo").expect("failed to parse arg 'foo'");
+ let bar: Option<String> =
+ attrs.take_attr("bar").expect("failed to parse arg 'bar'");
+
+ assert!(foo.is_some());
+ assert!(bar.is_none());
+ }
}

File Metadata

Mime Type
text/plain
Expires
Wed, Dec 4, 5:31 AM (7 h, 19 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
2611643
Default Alt Text
D10762.diff (9 KB)

Event Timeline