Skip to content

Commit

Permalink
Replace internal ::new() with From/Into for EntityUid and `En…
Browse files Browse the repository at this point in the history 
…tityTypeName` (#993)

Signed-off-by: Craig Disselkoen <cdiss@amazon.com>
  • Loading branch information
@cdisselkoen
cdisselkoen authored Jun 19, 2024
1 parent 0659bae commit 8980445
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 50 deletions.
51 changes: 22 additions & 29 deletions cedar-policy/src/api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ impl Entity {
) -> Result<Self, EntityAttrEvaluationError> {
// note that we take a "parents" parameter here; we will compute TC when
// the `Entities` object is created
// INVARIANT(UidOfEntityNotUnspecified): by invariant on `EntityUid`
Ok(Self(ast::Entity::new(
uid.into(),
attrs
Expand All @@ -106,7 +105,6 @@ impl Entity {
pub fn new_no_attrs(uid: EntityUid, parents: HashSet<EntityUid>) -> Self {
// note that we take a "parents" parameter here; we will compute TC when
// the `Entities` object is created
// INVARIANT(UidOfEntityNotUnspecified): by invariant on `EntityUid`
Self(ast::Entity::new_with_attr_partial_value(
uid.into(),
HashMap::new(),
Expand All @@ -125,7 +123,6 @@ impl Entity {
/// # cool_asserts::assert_matches!(alice.attr("age"), None);
/// ```
pub fn with_uid(uid: EntityUid) -> Self {
// INVARIANT(UidOfEntityNotUnspecified): by invariant on `EntityUid`
Self(ast::Entity::with_uid(uid.into()))
}

Expand All @@ -140,7 +137,7 @@ impl Entity {
/// assert_eq!(alice.uid(), euid);
/// ```
pub fn uid(&self) -> EntityUid {
EntityUid::new(self.0.uid().clone())
self.0.uid().clone().into()
}

/// Get the value for the given attribute, or `None` if not present.
Expand Down Expand Up @@ -199,9 +196,9 @@ impl Entity {
.collect();

(
EntityUid::new(uid),
uid.into(),
attrs,
ancestors.into_iter().map(EntityUid::new).collect(),
ancestors.into_iter().map(Into::into).collect(),
)
}

Expand Down Expand Up @@ -2150,24 +2147,24 @@ impl Template {
ast::PrincipalOrResourceConstraint::Any => TemplatePrincipalConstraint::Any,
ast::PrincipalOrResourceConstraint::In(eref) => {
TemplatePrincipalConstraint::In(match eref {
ast::EntityReference::EUID(e) => Some(EntityUid::new(e.as_ref().clone())),
ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
ast::EntityReference::Slot => None,
})
}
ast::PrincipalOrResourceConstraint::Eq(eref) => {
TemplatePrincipalConstraint::Eq(match eref {
ast::EntityReference::EUID(e) => Some(EntityUid::new(e.as_ref().clone())),
ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
ast::EntityReference::Slot => None,
})
}
ast::PrincipalOrResourceConstraint::Is(entity_type) => {
TemplatePrincipalConstraint::Is(EntityTypeName::new(entity_type.as_ref().clone()))
TemplatePrincipalConstraint::Is(entity_type.as_ref().clone().into())
}
ast::PrincipalOrResourceConstraint::IsIn(entity_type, eref) => {
TemplatePrincipalConstraint::IsIn(
EntityTypeName::new(entity_type.as_ref().clone()),
entity_type.as_ref().clone().into(),
match eref {
ast::EntityReference::EUID(e) => Some(EntityUid::new(e.as_ref().clone())),
ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
ast::EntityReference::Slot => None,
},
)
Expand All @@ -2180,14 +2177,10 @@ impl Template {
// Clone the data from Core to be consistent with the other constraints
match self.ast.action_constraint() {
ast::ActionConstraint::Any => ActionConstraint::Any,
ast::ActionConstraint::In(ids) => ActionConstraint::In(
ids.iter()
.map(|id| EntityUid::new(id.as_ref().clone()))
.collect(),
),
ast::ActionConstraint::Eq(id) => {
ActionConstraint::Eq(EntityUid::new(id.as_ref().clone()))
ast::ActionConstraint::In(ids) => {
ActionConstraint::In(ids.iter().map(|id| id.as_ref().clone().into()).collect())
}
ast::ActionConstraint::Eq(id) => ActionConstraint::Eq(id.as_ref().clone().into()),
}
}

Expand All @@ -2197,24 +2190,24 @@ impl Template {
ast::PrincipalOrResourceConstraint::Any => TemplateResourceConstraint::Any,
ast::PrincipalOrResourceConstraint::In(eref) => {
TemplateResourceConstraint::In(match eref {
ast::EntityReference::EUID(e) => Some(EntityUid::new(e.as_ref().clone())),
ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
ast::EntityReference::Slot => None,
})
}
ast::PrincipalOrResourceConstraint::Eq(eref) => {
TemplateResourceConstraint::Eq(match eref {
ast::EntityReference::EUID(e) => Some(EntityUid::new(e.as_ref().clone())),
ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
ast::EntityReference::Slot => None,
})
}
ast::PrincipalOrResourceConstraint::Is(entity_type) => {
TemplateResourceConstraint::Is(EntityTypeName::new(entity_type.as_ref().clone()))
TemplateResourceConstraint::Is(entity_type.as_ref().clone().into())
}
ast::PrincipalOrResourceConstraint::IsIn(entity_type, eref) => {
TemplateResourceConstraint::IsIn(
EntityTypeName::new(entity_type.as_ref().clone()),
entity_type.as_ref().clone().into(),
match eref {
ast::EntityReference::EUID(e) => Some(EntityUid::new(e.as_ref().clone())),
ast::EntityReference::EUID(e) => Some(e.as_ref().clone().into()),
ast::EntityReference::Slot => None,
},
)
Expand Down Expand Up @@ -2407,7 +2400,7 @@ impl Policy {
.ast
.env()
.iter()
.map(|(key, value)| ((*key).into(), EntityUid::new(value.clone())))
.map(|(key, value)| ((*key).into(), value.clone().into()))
.collect();
Some(wrapped_vals)
}
Expand Down Expand Up @@ -2463,11 +2456,11 @@ impl Policy {
PrincipalConstraint::Eq(self.convert_entity_reference(eref, slot_id).clone())
}
ast::PrincipalOrResourceConstraint::Is(entity_type) => {
PrincipalConstraint::Is(EntityTypeName::new(entity_type.as_ref().clone()))
PrincipalConstraint::Is(entity_type.as_ref().clone().into())
}
ast::PrincipalOrResourceConstraint::IsIn(entity_type, eref) => {
PrincipalConstraint::IsIn(
EntityTypeName::new(entity_type.as_ref().clone()),
entity_type.as_ref().clone().into(),
self.convert_entity_reference(eref, slot_id).clone(),
)
}
Expand Down Expand Up @@ -2501,11 +2494,11 @@ impl Policy {
ResourceConstraint::Eq(self.convert_entity_reference(eref, slot_id).clone())
}
ast::PrincipalOrResourceConstraint::Is(entity_type) => {
ResourceConstraint::Is(EntityTypeName::new(entity_type.as_ref().clone()))
ResourceConstraint::Is(entity_type.as_ref().clone().into())
}
ast::PrincipalOrResourceConstraint::IsIn(entity_type, eref) => {
ResourceConstraint::IsIn(
EntityTypeName::new(entity_type.as_ref().clone()),
entity_type.as_ref().clone().into(),
self.convert_entity_reference(eref, slot_id).clone(),
)
}
Expand Down Expand Up @@ -3492,7 +3485,7 @@ impl From<ast::Value> for EvalResult {
ast::ValueKind::Lit(ast::Literal::Long(i)) => Self::Long(i),
ast::ValueKind::Lit(ast::Literal::String(s)) => Self::String(s.to_string()),
ast::ValueKind::Lit(ast::Literal::EntityUID(e)) => {
Self::EntityUid(EntityUid::new(ast::EntityUID::clone(&e)))
Self::EntityUid(ast::EntityUID::clone(&e).into())
}
ast::ValueKind::Set(set) => Self::Set(Set(set
.authoritative
Expand Down
2 changes: 1 addition & 1 deletion cedar-policy/src/api/err.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ impl EntityAttrEvaluationError {
impl From<ast::EntityAttrEvaluationError> for EntityAttrEvaluationError {
fn from(err: ast::EntityAttrEvaluationError) -> Self {
Self {
uid: EntityUid::new(err.uid),
uid: err.uid.into(),
attr: err.attr,
err: err.err,
}
Expand Down
47 changes: 27 additions & 20 deletions cedar-policy/src/api/id.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,6 @@ impl EntityTypeName {
pub fn namespace(&self) -> String {
self.0.name().namespace()
}

/// Construct an [`EntityTypeName`] from the internal type.
/// This function is only intended to be used internally.
pub(crate) fn new(ty: ast::EntityType) -> Self {
Self(ty)
}
}

/// This `FromStr` implementation requires the _normalized_ representation of the
Expand All @@ -160,6 +154,20 @@ impl std::fmt::Display for EntityTypeName {
}
}

#[doc(hidden)]
impl From<ast::Name> for EntityTypeName {
fn from(name: ast::Name) -> Self {
Self(name.into())
}
}

#[doc(hidden)]
impl From<ast::EntityType> for EntityTypeName {
fn from(ty: ast::EntityType) -> Self {
Self(ty)
}
}

/// Unique id for an entity, such as `User::"alice"`.
///
/// An `EntityUid` contains an [`EntityTypeName`] and [`EntityId`]. It can
Expand Down Expand Up @@ -215,10 +223,6 @@ impl EntityUid {

/// Creates `EntityUid` from a JSON value, which should have
/// either the implicit or explicit `__entity` form.
///
/// Examples:
/// * `{ "__entity": { "type": "User", "id": "123abc" } }`
/// * `{ "type": "User", "id": "123abc" }`
/// ```
/// # use cedar_policy::{Entity, EntityId, EntityTypeName, EntityUid};
/// # use std::str::FromStr;
Expand All @@ -230,9 +234,11 @@ impl EntityUid {
#[allow(clippy::result_large_err)]
pub fn from_json(json: serde_json::Value) -> Result<Self, impl miette::Diagnostic> {
let parsed: cedar_policy_core::entities::EntityUidJson = serde_json::from_value(json)?;
Ok::<Self, JsonDeserializationError>(Self::new(
parsed.into_euid(|| JsonDeserializationErrorContext::EntityUid)?,
))
Ok::<Self, JsonDeserializationError>(
parsed
.into_euid(|| JsonDeserializationErrorContext::EntityUid)?
.into(),
)
}

/// Testing utility for creating `EntityUids` a bit easier
Expand All @@ -243,12 +249,6 @@ impl EntityUid {
EntityId::from_str(id).unwrap(),
)
}

/// Construct an [`EntityUid`] from the internal type.
/// This function is only intended to be used internally.
pub(crate) fn new(uid: ast::EntityUID) -> Self {
Self(uid)
}
}

impl FromStr for EntityUid {
Expand Down Expand Up @@ -279,7 +279,7 @@ impl FromStr for EntityUid {
/// If you have separate components of an [`EntityUid`], use [`EntityUid::from_type_name_and_id`]
fn from_str(uid_str: &str) -> Result<Self, Self::Err> {
ast::EntityUID::from_normalized_str(uid_str)
.map(Self::new)
.map(Into::into)
.map_err(Into::into)
}
}
Expand All @@ -304,6 +304,13 @@ impl From<EntityUid> for ast::EntityUID {
}
}

#[doc(hidden)]
impl From<ast::EntityUID> for EntityUid {
fn from(uid: ast::EntityUID) -> Self {
Self(uid)
}
}

/// Unique ids assigned to policies and templates.
///
/// A [`PolicyId`] can can be constructed using [`PolicyId::from_str`] or by
Expand Down

0 comments on commit 8980445

Please sign in to comment.