Paths

Table of Contentst

-
services/comm-services-lib/src/
-
comm-services-lib/
-
src/
-
database.rs

[services-lib] Implement DDB Attr conversions, deprecate old methods
ClosedPublic
Actions

Authored by bartek on Aug 10 2023, 3:44 PM.

Details

Reviewers

michal
• jon
varun

Commits

rCOMMfc8dbbfd4294: [services-lib] Implement DDB Attr conversions, deprecate old methods

Summary

Implements traits introduced in the previous diffs for basic types and marks legacy methods as deprecated.

Depends on D8789

Test Plan

Tested later in the stack

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Aug 10 2023, 3:44 PM

bartek held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptAug 10 2023, 3:44 PM

Herald added subscribers: tomek, ashoat. · View Herald Transcript

bartek added a child revision: D8791: [services-lib] Add BlobInfo utility type.Aug 10 2023, 3:47 PM

Harbormaster completed remote builds in B21708: Diff 29833.Aug 10 2023, 4:02 PM

bartek published this revision for review.Aug 11 2023, 2:26 AM

bartek added a reviewer: varun.

Potential ideas: (both of them are kinda iffy so up to you!)

Merge datetime and timestamp parsing logic and use the attribute type (String/Number) to differentiate between the two
We could do something like this for numbers (the dummy trait is required so there are conflicting implementations for FromStr types)

trait NumberFromAttr: FromStr<Err = ParseIntError> {}

impl NumberFromAttr for u8 {}
// ...
impl NumberFromAttr for i64 {}

impl<T: NumberFromAttr> TryFromAttribute for T {
  fn try_from_attr(
    attribute_name: impl Into<String>,
    attribute: Option<AttributeValue>,
  ) -> Result<Self, DBItemError> {
    match &attribute {
      Some(AttributeValue::N(numeric_str)) => {
        parse_integer(attribute_name, numeric_str)
      }
      Some(_) => Err(DBItemError::new(
        attribute_name.into(),
        Value::AttributeValue(attribute),
        DBItemAttributeError::IncorrectType,
      )),
      None => Err(DBItemError::new(
        attribute_name.into(),
        Value::AttributeValue(attribute),
        DBItemAttributeError::Missing,
      )),
    }
  }
}

services/comm-services-lib/src/database.rs
147–164 ↗	(On Diff #29833)	Shouldn't we also handle incorrect type case?

This revision is now accepted and ready to land.Aug 11 2023, 4:14 AM

In D8790#259498, @michal wrote:

Potential ideas: (both of them are kinda iffy so up to you!)

Merge datetime and timestamp parsing logic and use the attribute type (String/Number) to differentiate between the two

We could do something like this for numbers (the dummy trait is required so there are conflicting implementations for FromStr types)

Thanks for these ideas! For sure we need to do a better number support, but I'd like to design this carefully. I'll create a follow-up task for these

EDIT: https://linear.app/comm/issue/ENG-4658/improve-database-type-conversions-for-numbers

services/comm-services-lib/src/database.rs
147–164 ↗	(On Diff #29833)	Yep, we can follow the same convention as the others

Make timestamp parsing follow conventions

Harbormaster completed remote builds in B21796: Diff 29951.Aug 16 2023, 7:13 AM

Closed by commit rCOMMfc8dbbfd4294: [services-lib] Implement DDB Attr conversions, deprecate old methods (authored by bartek). · Explain WhyAug 16 2023, 7:26 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMMfc8dbbfd4294: [services-lib] Implement DDB Attr conversions, deprecate old methods.

Revision Contents
Changeset List

Path

Size

services/

comm-services-lib/

src/

database.rs

198 lines

Diff 29951

View Options

services/comm-services-lib/src/database.rs

Show First 20 Lines • Show All 92 Lines • ▼ Show 20 Lines	impl<T: TryFromAttribute> AttributeTryInto<T> for Option<AttributeValue> {
fn attr_try_into(		fn attr_try_into(
self,		self,
attribute_name: impl Into<String>,		attribute_name: impl Into<String>,
) -> Result<T, DBItemError> {		) -> Result<T, DBItemError> {
T::try_from_attr(attribute_name, self)		T::try_from_attr(attribute_name, self)
}		}
}		}

pub fn parse_string_attribute(		impl TryFromAttribute for String {
		fn try_from_attr(
attribute_name: impl Into<String>,		attribute_name: impl Into<String>,
attribute_value: Option<AttributeValue>,		attribute_value: Option<AttributeValue>,
) -> Result<String, DBItemError> {		) -> Result<Self, DBItemError> {
match attribute_value {		match attribute_value {
Some(AttributeValue::S(value)) => Ok(value),		Some(AttributeValue::S(value)) => Ok(value),
Some(_) => Err(DBItemError::new(		Some(_) => Err(DBItemError::new(
attribute_name.into(),		attribute_name.into(),
Value::AttributeValue(attribute_value),		Value::AttributeValue(attribute_value),
DBItemAttributeError::IncorrectType,		DBItemAttributeError::IncorrectType,
)),		)),
None => Err(DBItemError::new(		None => Err(DBItemError::new(
attribute_name.into(),		attribute_name.into(),
Value::AttributeValue(attribute_value),		Value::AttributeValue(attribute_value),
DBItemAttributeError::Missing,		DBItemAttributeError::Missing,
)),		)),
}		}
}		}
		}

pub fn parse_bool_attribute(		impl TryFromAttribute for bool {
		fn try_from_attr(
attribute_name: impl Into<String>,		attribute_name: impl Into<String>,
attribute_value: Option<AttributeValue>,		attribute_value: Option<AttributeValue>,
) -> Result<bool, DBItemError> {		) -> Result<Self, DBItemError> {
match attribute_value {		match attribute_value {
Some(AttributeValue::Bool(value)) => Ok(value),		Some(AttributeValue::Bool(value)) => Ok(value),
Some(_) => Err(DBItemError::new(		Some(_) => Err(DBItemError::new(
attribute_name.into(),		attribute_name.into(),
Value::AttributeValue(attribute_value),		Value::AttributeValue(attribute_value),
DBItemAttributeError::IncorrectType,		DBItemAttributeError::IncorrectType,
)),		)),
None => Err(DBItemError::new(		None => Err(DBItemError::new(
attribute_name.into(),		attribute_name.into(),
Value::AttributeValue(attribute_value),		Value::AttributeValue(attribute_value),
DBItemAttributeError::Missing,		DBItemAttributeError::Missing,
)),		)),
}		}
}		}
		}

pub fn parse_int_attribute<T>(		impl TryFromAttribute for DateTime<Utc> {
		fn try_from_attr(
		attribute_name: impl Into<String>,
		attribute: Option<AttributeValue>,
		) -> Result<Self, DBItemError> {
		match &attribute {
		Some(AttributeValue::S(datetime)) => datetime.parse().map_err(\|e\| {
		DBItemError::new(
		attribute_name.into(),
		Value::AttributeValue(attribute),
		DBItemAttributeError::InvalidTimestamp(e),
		)
		}),
		Some(_) => Err(DBItemError::new(
		attribute_name.into(),
		Value::AttributeValue(attribute),
		DBItemAttributeError::IncorrectType,
		)),
		None => Err(DBItemError::new(
		attribute_name.into(),
		Value::AttributeValue(attribute),
		DBItemAttributeError::Missing,
		)),
		}
		}
		}

		impl TryFromAttribute for HashMap<String, AttributeValue> {
		fn try_from_attr(
attribute_name: impl Into<String>,		attribute_name: impl Into<String>,
attribute_value: Option<AttributeValue>,		attribute_value: Option<AttributeValue>,
) -> Result<T, DBItemError>		) -> Result<Self, DBItemError> {
where		match attribute_value {
T: FromStr<Err = ParseIntError>,		Some(AttributeValue::M(map)) => Ok(map),
{		Some(_) => Err(DBItemError::new(
match &attribute_value {		attribute_name.into(),
Some(AttributeValue::N(numeric_str)) => {		Value::AttributeValue(attribute_value),
parse_integer(attribute_name, numeric_str)		DBItemAttributeError::IncorrectType,
		)),
		None => Err(DBItemError::new(
		attribute_name.into(),
		Value::AttributeValue(attribute_value),
		DBItemAttributeError::Missing,
		)),
}		}
		}
		}

		impl TryFromAttribute for Vec<u8> {
		fn try_from_attr(
		attribute_name: impl Into<String>,
		attribute_value: Option<AttributeValue>,
		) -> Result<Self, DBItemError> {
		match attribute_value {
		Some(AttributeValue::B(data)) => Ok(data.into_inner()),
Some(_) => Err(DBItemError::new(		Some(_) => Err(DBItemError::new(
attribute_name.into(),		attribute_name.into(),
Value::AttributeValue(attribute_value),		Value::AttributeValue(attribute_value),
DBItemAttributeError::IncorrectType,		DBItemAttributeError::IncorrectType,
)),		)),
None => Err(DBItemError::new(		None => Err(DBItemError::new(
attribute_name.into(),		attribute_name.into(),
Value::AttributeValue(attribute_value),		Value::AttributeValue(attribute_value),
DBItemAttributeError::Missing,		DBItemAttributeError::Missing,
)),		)),
}		}
}		}
		}

		#[deprecated = "Use `String::try_from_attr()` instead"]
		pub fn parse_string_attribute(
		attribute_name: impl Into<String>,
		attribute_value: Option<AttributeValue>,
		) -> Result<String, DBItemError> {
		String::try_from_attr(attribute_name, attribute_value)
		}

		#[deprecated = "Use `bool::try_from_attr()` instead"]
		pub fn parse_bool_attribute(
		attribute_name: impl Into<String>,
		attribute_value: Option<AttributeValue>,
		) -> Result<bool, DBItemError> {
		bool::try_from_attr(attribute_name, attribute_value)
		}

		#[deprecated = "Use `DateTime::<Utc>::try_from_attr()` instead"]
pub fn parse_datetime_attribute(		pub fn parse_datetime_attribute(
attribute_name: impl Into<String>,		attribute_name: impl Into<String>,
attribute_value: Option<AttributeValue>,		attribute_value: Option<AttributeValue>,
) -> Result<DateTime<Utc>, DBItemError> {		) -> Result<DateTime<Utc>, DBItemError> {
if let Some(AttributeValue::S(datetime)) = &attribute_value {		DateTime::<Utc>::try_from_attr(attribute_name, attribute_value)
// parse() accepts a relaxed RFC3339 string		}
datetime.parse().map_err(\|e\| {
DBItemError::new(		#[deprecated = "Use `HashMap::<String, AttributeValue>::try_from_attr()` instead"]
		pub fn parse_map_attribute(
		attribute_name: impl Into<String>,
		attribute_value: Option<AttributeValue>,
		) -> Result<HashMap<String, AttributeValue>, DBItemError> {
		attribute_value.attr_try_into(attribute_name)
		}

		pub fn parse_int_attribute<T>(
		attribute_name: impl Into<String>,
		attribute_value: Option<AttributeValue>,
		) -> Result<T, DBItemError>
		where
		T: FromStr<Err = ParseIntError>,
		{
		match &attribute_value {
		Some(AttributeValue::N(numeric_str)) => {
		parse_integer(attribute_name, numeric_str)
		}
		Some(_) => Err(DBItemError::new(
attribute_name.into(),		attribute_name.into(),
Value::AttributeValue(attribute_value),		Value::AttributeValue(attribute_value),
DBItemAttributeError::InvalidTimestamp(e),		DBItemAttributeError::IncorrectType,
)		)),
})		None => Err(DBItemError::new(
} else {
Err(DBItemError::new(
attribute_name.into(),		attribute_name.into(),
Value::AttributeValue(attribute_value),		Value::AttributeValue(attribute_value),
DBItemAttributeError::Missing,		DBItemAttributeError::Missing,
))		)),
}		}
}		}

/// Parses the UTC timestamp in milliseconds from a DynamoDB numeric attribute		/// Parses the UTC timestamp in milliseconds from a DynamoDB numeric attribute
pub fn parse_timestamp_attribute(		pub fn parse_timestamp_attribute(
attribute_name: impl Into<String>,		attribute_name: impl Into<String>,
attribute_value: Option<AttributeValue>,		attribute_value: Option<AttributeValue>,
) -> Result<DateTime<Utc>, DBItemError> {		) -> Result<DateTime<Utc>, DBItemError> {
let attribute_name: String = attribute_name.into();		let attribute_name: String = attribute_name.into();
let timestamp = parse_int_attribute::<i64>(		let timestamp = parse_int_attribute::<i64>(
attribute_name.clone(),		attribute_name.clone(),
attribute_value.clone(),		attribute_value.clone(),
)?;		)?;
let naive_datetime = chrono::NaiveDateTime::from_timestamp_millis(timestamp)		let naive_datetime = chrono::NaiveDateTime::from_timestamp_millis(timestamp)
.ok_or_else(\|\| {		.ok_or_else(\|\| {
DBItemError::new(		DBItemError::new(
attribute_name,		attribute_name,
Value::AttributeValue(attribute_value),		Value::AttributeValue(attribute_value),
DBItemAttributeError::TimestampOutOfRange,		DBItemAttributeError::TimestampOutOfRange,
)		)
})?;		})?;
Ok(DateTime::from_utc(naive_datetime, Utc))		Ok(DateTime::from_utc(naive_datetime, Utc))
}		}

pub fn parse_map_attribute(
attribute_name: impl Into<String>,
attribute_value: Option<AttributeValue>,
) -> Result<HashMap<String, AttributeValue>, DBItemError> {
match attribute_value {
Some(AttributeValue::M(map)) => Ok(map),
Some(_) => Err(DBItemError::new(
attribute_name.into(),
Value::AttributeValue(attribute_value),
DBItemAttributeError::IncorrectType,
)),
None => Err(DBItemError::new(
attribute_name.into(),
Value::AttributeValue(attribute_value),
DBItemAttributeError::Missing,
)),
}
}

pub fn parse_integer<T>(		pub fn parse_integer<T>(
attribute_name: impl Into<String>,		attribute_name: impl Into<String>,
attribute_value: &str,		attribute_value: &str,
) -> Result<T, DBItemError>		) -> Result<T, DBItemError>
where		where
T: FromStr<Err = ParseIntError>,		T: FromStr<Err = ParseIntError>,
{		{
attribute_value.parse::<T>().map_err(\|e\| {		attribute_value.parse::<T>().map_err(\|e\| {
▲ Show 20 Lines • Show All 54 Lines • Show Last 20 Lines

[services-lib] Implement DDB Attr conversions, deprecate old methodsClosedPublicActions