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 @@ -7,7 +7,10 @@ types::{AttributeValue, PutRequest, ReturnConsumedCapacity, WriteRequest}, }; use comm_lib::aws::{AwsConfig, DynamoDBClient}; -use comm_lib::database::{AttributeMap, DBItemAttributeError, DBItemError}; +use comm_lib::database::{ + AttributeExtractor, AttributeMap, DBItemAttributeError, DBItemError, + TryFromAttribute, +}; use constant_time_eq::constant_time_eq; use std::collections::{HashMap, HashSet}; use std::str::FromStr; @@ -723,7 +726,7 @@ item: Some(mut item), .. }) => { - let created = parse_date_time_attribute( + let created = DateTime::::try_from_attr( ACCESS_TOKEN_TABLE_CREATED_ATTRIBUTE, item.remove(ACCESS_TOKEN_TABLE_CREATED_ATTRIBUTE), )?; @@ -978,15 +981,13 @@ mut user: AttributeMap, get_one_time_keys: bool, ) -> Result, Error> { - let devices = parse_map_attribute( - USERS_TABLE_DEVICES_ATTRIBUTE, - user.remove(USERS_TABLE_DEVICES_ATTRIBUTE), - )?; + let devices: AttributeMap = + user.take_attr(USERS_TABLE_DEVICES_ATTRIBUTE)?; let mut devices_response = HashMap::with_capacity(devices.len()); for (device_id_key, device_info) in devices { let device_info_map = - parse_map_attribute(&device_id_key, Some(device_info))?; + AttributeMap::try_from_attr(&device_id_key, Some(device_info))?; let mut device_info_string_map = HashMap::new(); for (attribute_name, attribute_value) in device_info_map { @@ -1000,7 +1001,7 @@ } let attribute_value_str = - parse_string_attribute(&attribute_name, Some(attribute_value))?; + String::try_from_attr(&attribute_name, Some(attribute_value))?; device_info_string_map.insert(attribute_name, attribute_value_str); } @@ -1036,12 +1037,10 @@ .get_user_from_user_info(user_info.clone(), auth_type) .await { - Ok(Some(mut user)) => parse_string_attribute( - USERS_TABLE_PARTITION_KEY, - user.remove(USERS_TABLE_PARTITION_KEY), - ) - .map(Some) - .map_err(Error::Attribute), + Ok(Some(mut user)) => user + .take_attr(USERS_TABLE_PARTITION_KEY) + .map(Some) + .map_err(Error::Attribute), Ok(_) => Ok(None), Err(e) => Err(e), } @@ -1056,10 +1055,7 @@ .await { Ok(Some(mut user)) => { - let user_id = parse_string_attribute( - USERS_TABLE_PARTITION_KEY, - user.remove(USERS_TABLE_PARTITION_KEY), - )?; + let user_id = user.take_attr(USERS_TABLE_PARTITION_KEY)?; let password_file = parse_registration_data_attribute( user.remove(USERS_TABLE_REGISTRATION_ATTRIBUTE), )?; @@ -1115,10 +1111,9 @@ let mut result = Vec::new(); if let Some(attributes) = scan_output.items { for mut attribute in attributes { - if let Ok(username) = parse_string_attribute( - USERS_TABLE_USERNAME_ATTRIBUTE, - attribute.remove(USERS_TABLE_USERNAME_ATTRIBUTE), - ) { + if let Ok(username) = + attribute.take_attr(USERS_TABLE_USERNAME_ATTRIBUTE) + { result.push(username); } } @@ -1178,19 +1173,14 @@ return Ok(None); }; - let nonce = parse_string_attribute( - NONCE_TABLE_PARTITION_KEY, - item.remove(&NONCE_TABLE_PARTITION_KEY.to_string()), - )?; - - let created = parse_date_time_attribute( + let nonce = item.take_attr(NONCE_TABLE_PARTITION_KEY)?; + let created = DateTime::::try_from_attr( NONCE_TABLE_CREATED_ATTRIBUTE, - item.remove(&NONCE_TABLE_CREATED_ATTRIBUTE.to_string()), + item.remove(NONCE_TABLE_CREATED_ATTRIBUTE), )?; - - let expiration_time = parse_date_time_attribute( + let expiration_time = DateTime::::try_from_attr( NONCE_TABLE_EXPIRATION_TIME_ATTRIBUTE, - item.remove(&NONCE_TABLE_EXPIRATION_TIME_ATTRIBUTE.to_string()), + item.remove(NONCE_TABLE_EXPIRATION_TIME_ATTRIBUTE), )?; Ok(Some(NonceData { @@ -1326,27 +1316,6 @@ primary_key } -fn parse_date_time_attribute( - attribute_name: &str, - attribute: Option, -) -> Result, DBItemError> { - if let Some(AttributeValue::S(created)) = &attribute { - created.parse().map_err(|e| { - DBItemError::new( - attribute_name.to_string(), - attribute.into(), - DBItemAttributeError::InvalidTimestamp(e), - ) - }) - } else { - Err(DBItemError::new( - attribute_name.to_string(), - attribute.into(), - DBItemAttributeError::Missing, - )) - } -} - fn parse_auth_type_attribute( attribute: Option, ) -> Result { @@ -1425,6 +1394,7 @@ } } +#[deprecated(note = "Use `comm_lib` counterpart instead")] #[allow(dead_code)] fn parse_map_attribute( attribute_name: &str, @@ -1460,40 +1430,6 @@ } } -fn parse_string_attribute( - attribute_name: &str, - attribute_value: Option, -) -> Result { - match attribute_value { - Some(AttributeValue::S(value)) => Ok(value), - Some(_) => { - error!( - attribute = attribute_name, - value = ?attribute_value, - error_type = "IncorrectType", - "Unexpected attribute type when parsing string attribute" - ); - Err(DBItemError::new( - attribute_name.to_string(), - attribute_value.into(), - DBItemAttributeError::IncorrectType, - )) - } - None => { - error!( - attribute = attribute_name, - error_type = "Missing", - "Attribute is missing" - ); - Err(DBItemError::new( - attribute_name.to_string(), - attribute_value.into(), - DBItemAttributeError::Missing, - )) - } - } -} - fn create_device_info( flattened_device_key_upload: FlattenedDeviceKeyUpload, social_proof: Option, @@ -1583,7 +1519,7 @@ fn validate_keys() { // Taken from test user let example_payload = r#"{\"notificationIdentityPublicKeys\":{\"curve25519\":\"DYmV8VdkjwG/VtC8C53morogNJhpTPT/4jzW0/cxzQo\",\"ed25519\":\"D0BV2Y7Qm36VUtjwyQTJJWYAycN7aMSJmhEsRJpW2mk\"},\"primaryIdentityPublicKeys\":{\"curve25519\":\"Y4ZIqzpE1nv83kKGfvFP6rifya0itRg2hifqYtsISnk\",\"ed25519\":\"cSlL+VLLJDgtKSPlIwoCZg0h0EmHlQoJC08uV/O+jvg\"}}"#; - let serialized_payload = KeyPayload::from_str(&example_payload).unwrap(); + let serialized_payload = KeyPayload::from_str(example_payload).unwrap(); assert_eq!( serialized_payload 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 @@ -9,7 +9,10 @@ TransactWriteItem, Update, WriteRequest, }, }, - database::{AttributeMap, DBItemAttributeError, DBItemError, DynamoDBError}, + database::{ + AttributeExtractor, AttributeMap, DBItemAttributeError, DBItemError, + DynamoDBError, TryFromAttribute, + }, }; use tracing::{error, warn}; @@ -20,13 +23,12 @@ USERS_TABLE, USERS_TABLE_DEVICELIST_TIMESTAMP_ATTRIBUTE_NAME, USERS_TABLE_PARTITION_KEY, }, - database::parse_string_attribute, ddb_utils::AttributesOptionExt, error::{DeviceListError, Error, FromAttributeValue}, grpc_services::protos::unauth::DeviceType, }; -use super::{parse_date_time_attribute, DatabaseClient}; +use super::DatabaseClient; #[derive(Clone, Debug)] pub struct DeviceRow { @@ -119,7 +121,7 @@ impl TryFrom> for DeviceIDAttribute { type Error = DBItemError; fn try_from(value: Option) -> Result { - let item_id = parse_string_attribute(ATTR_ITEM_ID, value)?; + let item_id = String::try_from_attr(ATTR_ITEM_ID, value)?; // remove the device- prefix let device_id = item_id @@ -138,7 +140,7 @@ impl TryFrom> for DeviceListKeyAttribute { type Error = DBItemError; fn try_from(value: Option) -> Result { - let item_id = parse_string_attribute(ATTR_ITEM_ID, value)?; + let item_id = String::try_from_attr(ATTR_ITEM_ID, value)?; // remove the device-list- prefix, then parse the timestamp let timestamp: DateTime = item_id @@ -166,12 +168,10 @@ type Error = DBItemError; fn try_from(mut attrs: AttributeMap) -> Result { - let user_id = - parse_string_attribute(ATTR_USER_ID, attrs.remove(ATTR_USER_ID))?; + let user_id = attrs.take_attr(ATTR_USER_ID)?; let DeviceIDAttribute(device_id) = attrs.remove(ATTR_ITEM_ID).try_into()?; - let raw_device_type = - parse_string_attribute(ATTR_DEVICE_TYPE, attrs.remove(ATTR_DEVICE_TYPE))?; + let raw_device_type: String = attrs.take_attr(ATTR_DEVICE_TYPE)?; let device_type = DeviceType::from_str_name(&raw_device_type).ok_or_else(|| { DBItemError::new( @@ -260,12 +260,8 @@ impl TryFrom for IdentityKeyInfo { type Error = DBItemError; fn try_from(mut attrs: AttributeMap) -> Result { - let key_payload = - parse_string_attribute(ATTR_KEY_PAYLOAD, attrs.remove(ATTR_KEY_PAYLOAD))?; - let key_payload_signature = parse_string_attribute( - ATTR_KEY_PAYLOAD_SIGNATURE, - attrs.remove(ATTR_KEY_PAYLOAD_SIGNATURE), - )?; + 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) @@ -296,12 +292,8 @@ impl TryFrom for PreKey { type Error = DBItemError; fn try_from(mut attrs: AttributeMap) -> Result { - let pre_key = - parse_string_attribute(ATTR_PREKEY, attrs.remove(ATTR_PREKEY))?; - let pre_key_signature = parse_string_attribute( - ATTR_PREKEY_SIGNATURE, - attrs.remove(ATTR_PREKEY_SIGNATURE), - )?; + let pre_key = attrs.take_attr(ATTR_PREKEY)?; + let pre_key_signature = attrs.take_attr(ATTR_PREKEY_SIGNATURE)?; Ok(Self { pre_key, pre_key_signature, @@ -313,8 +305,7 @@ type Error = DBItemError; fn try_from(mut attrs: AttributeMap) -> Result { - let user_id = - parse_string_attribute(ATTR_USER_ID, attrs.remove(ATTR_USER_ID))?; + let user_id = attrs.take_attr(ATTR_USER_ID)?; let DeviceListKeyAttribute(timestamp) = attrs.remove(ATTR_ITEM_ID).try_into()?; @@ -333,20 +324,7 @@ ); } - // this should be a list of strings - let device_ids = attrs - .remove(ATTR_DEVICE_IDS) - .ok_or_else(|| { - DBItemError::new( - ATTR_DEVICE_IDS.to_string(), - None.into(), - DBItemAttributeError::Missing, - ) - })? - .to_vec(ATTR_DEVICE_IDS)? - .iter() - .map(|v| v.to_string("device_ids[?]").cloned()) - .collect::, DBItemError>>()?; + let device_ids: Vec = attrs.take_attr(ATTR_DEVICE_IDS)?; Ok(Self { user_id, @@ -725,7 +703,7 @@ return Ok(None); } - let timestamp = parse_date_time_attribute( + let timestamp = DateTime::::try_from_attr( USERS_TABLE_DEVICELIST_TIMESTAMP_ATTRIBUTE_NAME, raw_datetime, )?; 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 @@ -29,6 +29,7 @@ ConcurrentUpdateError, } +#[deprecated(note = "Use comm-lib traits instead")] pub trait FromAttributeValue { fn to_vec( &self,