Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Followup to #145 #155

Merged
merged 2 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cedar-policy-generators/src/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ impl PrincipalOrResourceConstraint {
1 => Ok(Self::IsTypeInSlot(hierarchy.arbitrary_entity_type(u)?))
)
} else {
// 32% Eq, 32% In (16% with and without Is), 32% Is (16% with and without In)
// 32% Eq, 16% In, 16% Is, 16% IsIn
cdisselkoen marked this conversation as resolved.
Show resolved Hide resolved
let uid = hierarchy.arbitrary_uid(u)?;
gen!(u,
2 => Ok(Self::Eq(uid)),
Expand Down
46 changes: 28 additions & 18 deletions cedar-policy-generators/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1034,15 +1034,20 @@ impl Schema {
hierarchy: &Hierarchy,
u: &mut Unstructured<'_>,
) -> Result<PrincipalOrResourceConstraint> {
// 20% of the time, NoConstraint; 40%, Eq; 40%, In
gen!(u,
2 => Ok(PrincipalOrResourceConstraint::NoConstraint),
4 => Ok(PrincipalOrResourceConstraint::Eq(
self.exprgenerator(Some(hierarchy)).arbitrary_principal_uid(u)?,
)),
4 => Ok(PrincipalOrResourceConstraint::In(
self.exprgenerator(Some(hierarchy)).arbitrary_principal_uid(u)?,
)))
// 20% of the time, NoConstraint
if u.ratio(1, 5)? {
Ok(PrincipalOrResourceConstraint::NoConstraint)
} else {
// 32% Eq, 16% In, 16% Is, 16% IsIn
let uid = self.exprgenerator(Some(hierarchy)).arbitrary_principal_uid(u)?;
let ety = u.choose(self.entity_types())?.clone();
gen!(u,
2 => Ok(PrincipalOrResourceConstraint::Eq(uid)),
1 => Ok(PrincipalOrResourceConstraint::In(uid)),
1 => Ok(PrincipalOrResourceConstraint::IsType(ety)),
1 => Ok(PrincipalOrResourceConstraint::IsTypeIn(ety, uid))
)
}
}
fn arbitrary_principal_constraint_size_hint(depth: usize) -> (usize, Option<usize>) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether/how the *_size_hint functions need to be updated. Suggestions welcome.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdisselkoen may have written the original

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are mostly performance optimizations; see docs on Arbitrary::size_hint. If/when we migrate to Bolero, they will go away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it's fine to leave these functions as-is?

arbitrary::size_hint::and(
Expand All @@ -1060,15 +1065,20 @@ impl Schema {
hierarchy: &Hierarchy,
u: &mut Unstructured<'_>,
) -> Result<PrincipalOrResourceConstraint> {
// 20% of the time, NoConstraint; 40%, Eq; 40%, In
gen!(u,
2 => Ok(PrincipalOrResourceConstraint::NoConstraint),
4 => Ok(PrincipalOrResourceConstraint::Eq(
self.exprgenerator(Some(hierarchy)).arbitrary_resource_uid(u)?,
)),
4 => Ok(PrincipalOrResourceConstraint::In(
self.exprgenerator(Some(hierarchy)).arbitrary_resource_uid(u)?,
)))
// 20% of the time, NoConstraint
if u.ratio(1, 5)? {
Ok(PrincipalOrResourceConstraint::NoConstraint)
} else {
// 32% Eq, 16% In, 16% Is, 16% IsIn
let uid = self.exprgenerator(Some(hierarchy)).arbitrary_resource_uid(u)?;
let ety = u.choose(self.entity_types())?.clone();
gen!(u,
2 => Ok(PrincipalOrResourceConstraint::Eq(uid)),
1 => Ok(PrincipalOrResourceConstraint::In(uid)),
1 => Ok(PrincipalOrResourceConstraint::IsType(ety)),
1 => Ok(PrincipalOrResourceConstraint::IsTypeIn(ety, uid))
)
}
}
fn arbitrary_resource_constraint_size_hint(depth: usize) -> (usize, Option<usize>) {
arbitrary::size_hint::and(
Expand Down
Loading