Skip to content

Commit

Permalink
Clean up some comments (following suggestions from cdisselkoen)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewmwells-amazon committed Nov 27, 2023
1 parent b2d2391 commit 770677e
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 12 deletions.
14 changes: 7 additions & 7 deletions cedar-policy-core/src/ast/policy_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ use thiserror::Error;
pub struct PolicySet {
/// `templates` contains all bodies of policies in the `PolicySet`
/// A body is either:
/// A Body of a `Template`, which has slots that need to be filled in
/// A Body of an `StaticPolicy`, which has been converted into a `Template` that has zero slots
/// The static policy's PolicyID is the same in both `templates` and `links`
/// - A Body of a `Template`, which has slots that need to be filled in
/// - A Body of a `StaticPolicy`, which has been converted into a `Template` that has zero slots
/// The static policy's [`PolicyID`] is the same in both `templates` and `links`
templates: HashMap<PolicyID, Arc<Template>>,
/// `links` contains all of the executable policies in the `PolicySet`
/// A `StaticPolicy` must have exactly one `Policy` in `links`
/// (this is managed by `PolicySet::add)
/// (this is managed by `PolicySet::add`)
/// The static policy's PolicyID is the same in both `templates` and `links`
/// A `Template` may have zero or many links
links: HashMap<PolicyID, Policy>,
Expand Down Expand Up @@ -296,7 +296,7 @@ impl PolicySet {
}

/// Add a template to the policy set.
/// If a link with the same name already exists, this will error.
/// If a link, static policy or template with the same name already exists, this will error.
pub fn add_template(&mut self, t: Template) -> Result<(), PolicySetError> {
if self.links.contains_key(t.id()) {
return Err(PolicySetError::Occupied { id: t.id().clone() });
Expand All @@ -318,8 +318,8 @@ impl PolicySet {
}

/// Remove a template from the policy set.
/// If any policy is linked to the template this will error
/// If it is not a template this will error.
/// This will error if any policy is linked to the template.
/// This will error if `policy_id` is not a template.
pub fn remove_template(
&mut self,
policy_id: &PolicyID,
Expand Down
10 changes: 6 additions & 4 deletions cedar-policy/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1889,8 +1889,9 @@ impl PolicySet {
}
}

/// Remove a static `Policy` from the `PolicySet`
/// If the policy is not a static policy this will error
/// Remove a static `Policy` from the `PolicySet`.
///
/// This will error if the policy is not a static policy.
pub fn remove_static(&mut self, policy_id: PolicyId) -> Result<Policy, PolicySetError> {
let Some(policy) = self.policies.remove(&policy_id) else {
return Err(PolicySetError::PolicyNonexistentError(policy_id));
Expand All @@ -1917,8 +1918,9 @@ impl PolicySet {
}

/// Remove a `Template` from the `PolicySet`.
/// If any policy is linked to the template, this will error
/// If the policy is not a template this will error
///
/// This will error if any policy is linked to the template.
/// This will error if `policy_id` is not a template.
pub fn remove_template(&mut self, template_id: PolicyId) -> Result<Template, PolicySetError> {
let Some(template) = self.templates.remove(&template_id) else {
return Err(PolicySetError::TemplateNonexistentError(template_id));
Expand Down
1 change: 0 additions & 1 deletion cedar-policy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,6 @@ mod policy_set_tests {
pset.remove_template(PolicyId::from_str("policy3").unwrap()),
Err(PolicySetError::TemplateNonexistentError(_))
);
//Should not panic
}

#[test]
Expand Down

0 comments on commit 770677e

Please sign in to comment.