Cleanup in frontend code #740
Conversation
khieta
requested review from
andrewmwells-amazon
and removed request for
cdisselkoen
| let policy_set = match policies { | ||
| PolicySpecification::Concatenated(policies) => match PolicySet::from_str(&policies) { |
There was a problem hiding this comment.
Fyi (to help processing the diff): This parsing code, and the parse_policy_set_from_individual_policies function below were copied verbatim to util.rs.
|
If you rebase, the downstream build should hopefully pass now |
Signed-off-by: Kesha Hietala <khieta@amazon.com>
Signed-off-by: Craig Disselkoen <cdiss@amazon.com> Signed-off-by: Kesha Hietala <khieta@amazon.com>
Signed-off-by: Andrew Wells <anmwells@amazon.com> Signed-off-by: Kesha Hietala <khieta@amazon.com>
Signed-off-by: Kesha Hietala <khieta@amazon.com>
| #[serde(default)] | ||
| #[serde(rename = "validationSettings")] | ||
| validation_settings: ValidationSettings, | ||
| schema: cedar_policy_validator::SchemaFragment, | ||
| /// Schema in JSON format | ||
| schema: JsonValueWithNoDuplicateKeys, |
There was a problem hiding this comment.
What's the benefit of this change?
There was a problem hiding this comment.
Consistency with AuthorizationCall
| #[derive(Serialize, Deserialize)] | ||
| #[cfg_attr(feature = "wasm", derive(tsify::Tsify))] | ||
| #[cfg_attr(feature = "wasm", tsify(into_wasm_abi, from_wasm_abi))] | ||
| struct ValidateCall { |
There was a problem hiding this comment.
The Validate to Validation change looks nice, but I wonder if it's worth making a breaking change to the API. Depends on how much we care about keeping this interface stable.
There was a problem hiding this comment.
I think this isn't breaking since the ValidateCall type was only ever exposed indirectly via its serialized JSON format.
Signed-off-by: Kesha Hietala <khieta@amazon.com>
Description of changes
This PR fixes a few inconsistencies in our frontend, which will become visible once we make these structures public in #737. I believe that all of these changes are non-breaking because they affect private data structures... but please double check my work.
ValidateCall->ValidationCallfor consistency withAuthorizationCallValidateAnswer->ValidationAnswerfor consistency withAuthorizationAnswerParseFailed->Failuresince failures encompass more than just parsing (e.g., failure to convert aSchemato the internalValidatorSchematype)validateto call the public function incedar_policyinstead of the internal one incedar_policy_validatorValidationMode->ValidationEnabledsince "mode" usually refers to "strict" vs. "permissive"ValidationCallto be aJsonValueWithNoDuplicateKeysfor consistency withAuthorizationCallA few other things I'd like to change, but didn't because they will be breaking (will make in a followup PR):
InterfaceResponse,InterfaceDiagnosticstoResponse,Diagnostics. We are already redefining types (e.g.,ValidationError) so I don't think this is too confusing.InterfaceResultand instead use theFailurefield ofAuthorizationAnswer,ValidationAnswerto report errors. I don't think it's worth the extra infrastructure to distinguish between internal and non-internal errors.json_is_authorizedtojson_str_is_authorized& update it to return a json string encoding of aAuthorizationAnswer. Similar forjson_validate.Issue #, if available
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
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):