-
Notifications
You must be signed in to change notification settings - Fork 90
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
Improve cedar policy API to access request context information #1318
Improve cedar policy API to access request context information #1318
Conversation
562d7b7
to
20c5164
Compare
cedar-policy/src/api.rs
Outdated
/// } | ||
/// assert_eq!(context.get("nonexistent"), None); | ||
/// ``` | ||
pub fn get(&self, key: &str) -> Option<&Value> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the public API this should probably be an EvalResult
instead of a Value
since Value
isn't re-exported from cedar-policy-core
. I think there's a already a From
impl for this conversion, though it will mean this function can't return a reference anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, John. I've pushed an update to match these requirements.
8a92f2e
to
13f32a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
also, maybe add a line to the changelog for the new functionality |
Signed-off-by: Tamas Jozsa <tamas@cloudflare.com>
13f32a3
to
2c1a700
Compare
done and done |
Description of changes
Added a new
get
helper method to Context that allows easy extraction of generic values from the context by key. This method simplifies the common use case of retrieving values from Context objects.Issue #, if available
Add get convenience method to Context for generic value extraction
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy
(e.g., changes to the signature of an existing API).cedar-policy
(e.g., addition of a new API).cedar-policy
.cedar-policy-core
,cedar-validator
, etc.)I confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec
(choose one, and delete the other options):cedar-spec
, and how you have tested that your updates are correct.)cedar-spec
. (Post your PR anyways, and we'll discuss in the comments.)I confirm that
docs.cedarpolicy.com
(choose one, and delete the other options):cedar-docs
. PRs should be targeted at astaging-X.Y
branch, notmain
.)