Add APIs to get schema annotations

[shaobo-he-aws]

Description of changes

Issue #, if available

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

• A backwards-compatible change requiring a minor version bump to cedar-policy (e.g., addition of a new API).

I confirm that this PR (choose one, and delete the other options):

• Updates the "Unreleased" section of the CHANGELOG with a description of my change (required for major/minor version bumps).

I confirm that cedar-spec (choose one, and delete the other options):

• Does not require updates because my change does not impact the Cedar formal model or DRT infrastructure.

I confirm that docs.cedarpolicy.com (choose one, and delete the other options):

• Does not require updates because my change does not impact the Cedar language specification.

[john-h-kastner-aws]

overloading the None case here isn't great. Maybe we could publicly expose an Id data type so that error can be reported sooner? Though, using that in other places in the API would probably be a breaking change, so perhaps it's better to stick with this signature for now

[john-h-kastner-aws]

Also, this doesn't add an API for getting annotations on entity/context attributes. That's probably a more complicated API due to the recursive structure of types, but can you open a issue so we don't forget about it if you don't plan on doing it in this PR?

[shaobo-he-aws]

Also, this doesn't add an API for getting annotations on entity/context attributes. That's probably a more complicated API due to the recursive structure of types, but can you open a issue so we don't forget about it if you don't plan on doing it in this PR?

Good idea. Issue: #1404

[cdisselkoen]

Having all of these APIs return Iterator<Item = (&str, &str)> is good, but I wonder if we need an additional API to get the value of an annotation with a user-specified key. For instance if I want to specifically look up the value of the foo annotation on a particular namespace. As is, I'd be forced to iterate through the entire iterator looking for the key foo. We could solve this either by adding a separate API namespace_annotation(&self, namespace, key) or perhaps by returning a map structure (HashMap or BTreeMap) instead of an iterator in this API, so that users could do their own lookups.

Same for the other new annotation-getting APIs.

[shaobo-he-aws]

Good point. I think that having an iterator is a generic. That is, users can collect them into a HashMap or BTreeMap given their preferences (e.g., performance vs order). I also think it may be a good idea to let user collect the iterator and get the value by key, instead of having APIs. Maybe I'm too paranoid about breaking changes.

[cdisselkoen]

I guess I'm concerned that having the user collect the iterator in order to get the value by key is

1. a little complicated, compared to having a direct method for it -- so, creates a suboptimal user experience
2. potentially poor performance, as getting any value by key at all requires collecting the entire iterator

If we're worried about breaking changes, we don't have to return a map structure, we can just implement our own getter, something like:

pub fn namespace_annotation(
    &self,
    namespace: EntityNamespace,
    annotation_key: &str,
) -> Option<&str> { ... }

that gets a particular annotation value given a key. I don't think adding a method like this is overly committing to any particular implementation details

[shaobo-he-aws]

Sure. I can do it.

[john-h-kastner-aws]

Approving since the code looks good to merge as is. I agree with craig that we should provide direct accessor methods also/instead of the iterators

[shaobo-he-aws]

Approving since the code looks good to merge as is. I agree with craig that we should provide direct accessor methods also/instead of the iterators

Thanks for the review. I'll add these APIs.