Skip to content

Commit

Permalink
Three more cherrypicks to 4.2.x (#1309)
Browse files Browse the repository at this point in the history 
Signed-off-by: Craig Disselkoen <[email protected]>
Signed-off-by: Thomas Hill <[email protected]>
Signed-off-by: John Kastner <[email protected]>
Co-authored-by: Thomas Hill <[email protected]>
Co-authored-by: John Kastner <[email protected]>
  • Loading branch information
@cdisselkoen @tomlikestorock @john-h-kastner-aws
3 people authored Nov 8, 2024
1 parent be4d70f commit 4772404
Show file tree
Hide file tree
Showing 6 changed files with 271 additions and 21 deletions.
20 changes: 20 additions & 0 deletions cedar-policy-core/src/ast/entity.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use serde_with::{serde_as, TryFromInto};
use smol_str::SmolStr;
use std::collections::{BTreeMap, HashMap, HashSet};
use std::str::FromStr;
use std::sync::Arc;
use thiserror::Error;

/// The entity type that Actions must have
Expand Down Expand Up @@ -605,6 +606,25 @@ impl TCNode<EntityUID> for Entity {
}
}

impl TCNode<EntityUID> for Arc<Entity> {
fn get_key(&self) -> EntityUID {
self.uid().clone()
}

fn add_edge_to(&mut self, k: EntityUID) {
// Use Arc::make_mut to get a mutable reference to the inner value
Arc::make_mut(self).add_ancestor(k)
}

fn out_edges(&self) -> Box<dyn Iterator<Item = &EntityUID> + '_> {
Box::new(self.ancestors())
}

fn has_edge_to(&self, e: &EntityUID) -> bool {
self.is_descendant_of(e)
}
}

impl std::fmt::Display for Entity {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
Expand Down
49 changes: 32 additions & 17 deletions cedar-policy-core/src/entities.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub struct Entities {
/// Important internal invariant: for any `Entities` object that exists, the
/// the `ancestor` relation is transitively closed.
#[serde_as(as = "Vec<(_, _)>")]
entities: HashMap<EntityUID, Entity>,
entities: HashMap<EntityUID, Arc<Entity>>,

/// The mode flag determines whether this store functions as a partial store or
/// as a fully concrete store.
Expand Down Expand Up @@ -109,7 +109,7 @@ impl Entities {

/// Iterate over the `Entity`s in the `Entities`
pub fn iter(&self) -> impl Iterator<Item = &Entity> {
self.entities.values()
self.entities.values().map(|e| e.as_ref())
}

/// Adds the [`crate::ast::Entity`]s in the iterator to this [`Entities`].
Expand All @@ -125,7 +125,7 @@ impl Entities {
/// responsible for ensuring that TC and DAG hold before calling this method.
pub fn add_entities(
mut self,
collection: impl IntoIterator<Item = Entity>,
collection: impl IntoIterator<Item = Arc<Entity>>,
schema: Option<&impl Schema>,
tc_computation: TCComputation,
extensions: &Extensions<'_>,
Expand Down Expand Up @@ -174,7 +174,7 @@ impl Entities {
tc_computation: TCComputation,
extensions: &Extensions<'_>,
) -> Result<Self> {
let mut entity_map = create_entity_map(entities.into_iter())?;
let mut entity_map = create_entity_map(entities.into_iter().map(Arc::new))?;
if let Some(schema) = schema {
// Validate non-action entities against schema.
// We do this before adding the actions, because we trust the
Expand Down Expand Up @@ -213,7 +213,7 @@ impl Entities {
schema
.action_entities()
.into_iter()
.map(|e| (e.uid().clone(), Arc::unwrap_or_clone(e))),
.map(|e: Arc<Entity>| (e.uid().clone(), e)),
);
}
Ok(Self {
Expand Down Expand Up @@ -252,6 +252,7 @@ impl Entities {
fn to_ejsons(&self) -> Result<Vec<EntityJson>> {
self.entities
.values()
.map(Arc::as_ref)
.map(EntityJson::from_entity)
.collect::<std::result::Result<_, JsonSerializationError>>()
.map_err(Into::into)
Expand Down Expand Up @@ -322,7 +323,9 @@ impl Entities {
}

/// Create a map from EntityUids to Entities, erroring if there are any duplicates
fn create_entity_map(es: impl Iterator<Item = Entity>) -> Result<HashMap<EntityUID, Entity>> {
fn create_entity_map(
es: impl Iterator<Item = Arc<Entity>>,
) -> Result<HashMap<EntityUID, Arc<Entity>>> {
let mut map = HashMap::new();
for e in es {
match map.entry(e.uid().clone()) {
Expand All @@ -338,10 +341,13 @@ fn create_entity_map(es: impl Iterator<Item = Entity>) -> Result<HashMap<EntityU
impl IntoIterator for Entities {
type Item = Entity;

type IntoIter = hash_map::IntoValues<EntityUID, Entity>;
type IntoIter = std::iter::Map<
std::collections::hash_map::IntoValues<EntityUID, Arc<Entity>>,
fn(Arc<Entity>) -> Entity,
>;

fn into_iter(self) -> Self::IntoIter {
self.entities.into_values()
self.entities.into_values().map(Arc::unwrap_or_clone)
}
}

Expand Down Expand Up @@ -497,7 +503,8 @@ mod json_parsing_tests {

let addl_entities = parser
.iter_from_json_value(new)
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)));
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)))
.map(Arc::new);
let err = simple_entities(&parser).add_entities(
addl_entities,
None::<&NoEntitiesSchema>,
Expand Down Expand Up @@ -537,7 +544,8 @@ mod json_parsing_tests {

let addl_entities = parser
.iter_from_json_value(new)
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)));
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)))
.map(Arc::new);
let err = simple_entities(&parser).add_entities(
addl_entities,
None::<&NoEntitiesSchema>,
Expand Down Expand Up @@ -576,7 +584,8 @@ mod json_parsing_tests {

let addl_entities = parser
.iter_from_json_value(new)
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)));
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)))
.map(Arc::new);
let err = simple_entities(&parser).add_entities(
addl_entities,
None::<&NoEntitiesSchema>,
Expand Down Expand Up @@ -619,7 +628,8 @@ mod json_parsing_tests {

let addl_entities = parser
.iter_from_json_value(new)
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)));
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)))
.map(Arc::new);
let es = simple_entities(&parser)
.add_entities(
addl_entities,
Expand Down Expand Up @@ -658,7 +668,8 @@ mod json_parsing_tests {

let addl_entities = parser
.iter_from_json_value(new)
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)));
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)))
.map(Arc::new);
let es = simple_entities(&parser)
.add_entities(
addl_entities,
Expand Down Expand Up @@ -699,7 +710,8 @@ mod json_parsing_tests {

let addl_entities = parser
.iter_from_json_value(new)
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)));
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)))
.map(Arc::new);
let es = simple_entities(&parser)
.add_entities(
addl_entities,
Expand Down Expand Up @@ -739,7 +751,8 @@ mod json_parsing_tests {

let addl_entities = parser
.iter_from_json_value(new)
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)));
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)))
.map(Arc::new);
let es = simple_entities(&parser)
.add_entities(
addl_entities,
Expand All @@ -766,7 +779,8 @@ mod json_parsing_tests {

let addl_entities = parser
.iter_from_json_value(new)
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)));
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)))
.map(Arc::new);
let err = simple_entities(&parser)
.add_entities(
addl_entities,
Expand All @@ -787,7 +801,8 @@ mod json_parsing_tests {
let new = serde_json::json!([{"uid":{ "type": "Test", "id": "alice" }, "attrs" : {}, "parents" : []}]);
let addl_entities = parser
.iter_from_json_value(new)
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)));
.unwrap_or_else(|e| panic!("{:?}", &miette::Report::new(e)))
.map(Arc::new);
let err = simple_entities(&parser).add_entities(
addl_entities,
None::<&NoEntitiesSchema>,
Expand Down
3 changes: 3 additions & 0 deletions cedar-policy-validator/src/schema/test_579.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@
//! we only do the test for the more sensible one. (For instance, for 1a1, we
//! only test an entity type reference, not a common type reference.)
// Don't warn on the test case names based on above scenario classification scheme
#![allow(non_snake_case)]

use super::{test::utils::collect_warnings, SchemaWarning, ValidatorSchema};
use cedar_policy_core::extensions::Extensions;
use cedar_policy_core::test_utils::{
Expand Down
4 changes: 4 additions & 0 deletions cedar-policy/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ harness = false
name = "extension_fn_validation"
harness = false

[[bench]]
name = "deeply_nested_est"
harness = false

[package.metadata.docs.rs]
features = ["experimental"]
rustdoc-args = ["--cfg", "docsrs"]
Loading

0 comments on commit 4772404

Please sign in to comment.