From 8811f1a85f4f0c8f065a165e51fe2406e34f610b Mon Sep 17 00:00:00 2001 From: Shaobo He Date: Mon, 13 Jan 2025 13:54:11 -0800 Subject: [PATCH] unified errors in various places Signed-off-by: Shaobo He --- .../src/entities/conformance/err.rs | 51 +++++++++++++++---- .../src/entities/json/entities.rs | 9 +--- cedar-policy-validator/src/coreschema.rs | 34 +++---------- cedar-policy-validator/src/diagnostics.rs | 9 ++-- .../src/diagnostics/validation_errors.rs | 11 ++-- cedar-policy/src/api/err.rs | 2 +- 6 files changed, 63 insertions(+), 53 deletions(-) diff --git a/cedar-policy-core/src/entities/conformance/err.rs b/cedar-policy-core/src/entities/conformance/err.rs index 8983f2512..3216a5483 100644 --- a/cedar-policy-core/src/entities/conformance/err.rs +++ b/cedar-policy-core/src/entities/conformance/err.rs @@ -15,8 +15,10 @@ */ //! This module cotnains errors around entities not conforming to schemas use super::TypeMismatchError; -use crate::ast::{EntityType, EntityUID}; +use crate::ast::{Eid, EntityType, EntityUID}; use crate::extensions::ExtensionFunctionLookupError; +use crate::impl_diagnostic_from_method_on_field; +use itertools::Itertools; use miette::Diagnostic; use smol_str::SmolStr; use thiserror::Error; @@ -139,12 +141,15 @@ impl EntitySchemaConformanceError { pub(crate) fn invalid_enum_entity( uid: EntityUID, - choices: impl IntoIterator, + choices: impl IntoIterator, ) -> Self { - Self::InvalidEnumEntity(InvalidEnumEntity { - uid, - choices: choices.into_iter().collect(), - }) + Self::InvalidEnumEntity( + InvalidEnumEntityError { + uid, + choices: choices.into_iter().collect(), + } + .into(), + ) } } @@ -298,10 +303,38 @@ impl Diagnostic for UnexpectedEntityTypeError { // Don't make fields `pub`, don't make breaking changes, and use caution // when adding public methods. #[derive(Debug, Error, Diagnostic)] -#[error("entity `{uid}` has invalid UID: `{}`", uid.eid().escaped())] +#[error(transparent)] +#[diagnostic(transparent)] pub struct InvalidEnumEntity { + err: InvalidEnumEntityError, +} + +impl From for InvalidEnumEntity { + fn from(value: InvalidEnumEntityError) -> Self { + Self { err: value } + } +} + +/// Returned when an entity is of an enumerated entity type but has invalid EID +#[derive(Debug, Error, Clone, PartialEq, Eq, Hash)] +#[error("entity `{uid}` is of an enumerated entity type, but `\"{}\"` is not declared as a valid eid", uid.eid().escaped())] +pub struct InvalidEnumEntityError { /// Entity where the error occurred - uid: EntityUID, + pub uid: EntityUID, /// Name of the attribute where the error occurred - choices: Vec, + pub choices: Vec, +} + +impl Diagnostic for InvalidEnumEntityError { + impl_diagnostic_from_method_on_field!(uid, loc); + + fn help<'a>(&'a self) -> Option> { + Some(Box::new(format!( + "valid entity eids: {}", + self.choices + .iter() + .map(|e| format!("\"{}\"", e.escaped())) + .join(", ") + ))) + } } diff --git a/cedar-policy-core/src/entities/json/entities.rs b/cedar-policy-core/src/entities/json/entities.rs index b9d3d7d28..fe2bcc74b 100644 --- a/cedar-policy-core/src/entities/json/entities.rs +++ b/cedar-policy-core/src/entities/json/entities.rs @@ -19,7 +19,7 @@ use super::{ CedarValueJson, EntityTypeDescription, EntityUidJson, NoEntitiesSchema, Schema, TypeAndId, ValueParser, }; -use crate::ast::{BorrowedRestrictedExpr, Eid, Entity, EntityUID, PartialValue, RestrictedExpr}; +use crate::ast::{BorrowedRestrictedExpr, Entity, EntityUID, PartialValue, RestrictedExpr}; use crate::entities::conformance::EntitySchemaConformanceChecker; use crate::entities::{ conformance::err::{EntitySchemaConformanceError, UnexpectedEntityTypeError}, @@ -302,12 +302,7 @@ impl<'e, 's, S: Schema> EntityJsonParser<'e, 's, S> { if let Some(choices) = desc.enum_enity_eids() { if !choices.contains(&uid.eid()) { return Err(JsonDeserializationError::EntitySchemaConformance( - EntitySchemaConformanceError::invalid_enum_entity( - uid.clone(), - choices - .iter() - .map(|e| >::as_ref(e).clone()), - ), + EntitySchemaConformanceError::invalid_enum_entity(uid.clone(), choices), )); } } diff --git a/cedar-policy-validator/src/coreschema.rs b/cedar-policy-validator/src/coreschema.rs index 90b7ad7a5..3c5919953 100644 --- a/cedar-policy-validator/src/coreschema.rs +++ b/cedar-policy-validator/src/coreschema.rs @@ -15,11 +15,11 @@ */ use crate::{ValidatorActionId, ValidatorEntityType, ValidatorEntityTypeKind, ValidatorSchema}; use cedar_policy_core::ast::{Eid, EntityType, EntityUID}; +use cedar_policy_core::entities::conformance::err::InvalidEnumEntityError; use cedar_policy_core::extensions::{ExtensionFunctionLookupError, Extensions}; use cedar_policy_core::{ast, entities}; use miette::Diagnostic; use nonempty::NonEmpty; -use request_validation_errors::InvalidEnumEntityError; use smol_str::SmolStr; use std::collections::hash_map::Values; use std::collections::HashSet; @@ -186,9 +186,9 @@ impl ast::RequestSchema for ValidatorSchema { } = et { if !choices.contains(euid.eid().as_ref()) { - return Err(request_validation_errors::InvalidEnumEntityError { - euid: euid.clone().into(), - choices: choices.clone(), + return Err(InvalidEnumEntityError { + uid: euid.clone(), + choices: choices.into_iter().map(|s| Eid::new(s.clone())).collect(), } .into()); } @@ -210,9 +210,9 @@ impl ast::RequestSchema for ValidatorSchema { } = et { if !choices.contains(euid.eid().as_ref()) { - return Err(request_validation_errors::InvalidEnumEntityError { - euid: euid.clone().into(), - choices: choices.clone(), + return Err(InvalidEnumEntityError { + uid: euid.clone(), + choices: choices.into_iter().map(|s| Eid::new(s.clone())).collect(), } .into()); } @@ -361,8 +361,6 @@ pub mod request_validation_errors { use cedar_policy_core::impl_diagnostic_from_method_on_field; use itertools::Itertools; use miette::Diagnostic; - use nonempty::NonEmpty; - use smol_str::SmolStr; use std::sync::Arc; use thiserror::Error; @@ -568,24 +566,6 @@ pub mod request_validation_errors { &self.action } } - - /// Request principal or resource is an invalid enumerated entity - #[derive(Debug, Error)] - #[error("entity `{euid}` is not declared as an enumerated entity but has invalid eid: `{}`", euid.eid().escaped())] - pub struct InvalidEnumEntityError { - /// problematic EUID - pub(super) euid: Arc, - /// valid entity EIDs - pub(super) choices: NonEmpty, - } - - impl Diagnostic for InvalidEnumEntityError { - impl_diagnostic_from_method_on_field!(euid, loc); - - fn help<'a>(&'a self) -> Option> { - Some(Box::new(format!("valid entity eids: {:?}", self.choices))) - } - } } /// Struct which carries enough information that it can impl Core's diff --git a/cedar-policy-validator/src/diagnostics.rs b/cedar-policy-validator/src/diagnostics.rs index dd9a59c54..6035e6827 100644 --- a/cedar-policy-validator/src/diagnostics.rs +++ b/cedar-policy-validator/src/diagnostics.rs @@ -17,6 +17,7 @@ //! This module contains the diagnostics (i.e., errors and warnings) that are //! returned by the validator. +use cedar_policy_core::entities::conformance::err::InvalidEnumEntityError; use miette::Diagnostic; use smol_str::SmolStr; use thiserror::Error; @@ -24,7 +25,7 @@ use validation_errors::UnrecognizedActionIdHelp; use std::collections::BTreeSet; -use cedar_policy_core::ast::{EntityType, EntityUID, Expr, PolicyID}; +use cedar_policy_core::ast::{Eid, EntityType, EntityUID, Expr, PolicyID}; use cedar_policy_core::parser::Loc; use crate::types::{EntityLUB, Type}; @@ -414,8 +415,10 @@ impl ValidationError { validation_errors::InvalidEnumEntity { source_loc, policy_id, - entity, - choices: choices.into_iter().collect(), + err: InvalidEnumEntityError { + uid: entity, + choices: choices.into_iter().map(|s| Eid::new(s)).collect(), + }, } .into() } diff --git a/cedar-policy-validator/src/diagnostics/validation_errors.rs b/cedar-policy-validator/src/diagnostics/validation_errors.rs index fdca37c7a..d82b79a38 100644 --- a/cedar-policy-validator/src/diagnostics/validation_errors.rs +++ b/cedar-policy-validator/src/diagnostics/validation_errors.rs @@ -16,6 +16,7 @@ //! Defines errors returned by the validator. +use cedar_policy_core::entities::conformance::err::InvalidEnumEntityError; use miette::Diagnostic; use thiserror::Error; @@ -720,23 +721,21 @@ impl Display for AttributeAccess { /// Returned when an entity literal is of an enumerated entity type but has /// undeclared UID #[derive(Debug, Clone, Error, Hash, Eq, PartialEq)] -#[error("for policy `{policy_id}`, entity `{entity}`'s UID is illegal")] +#[error("for policy `{policy_id}`: {err}")] pub struct InvalidEnumEntity { /// Source location pub source_loc: Option, /// Policy ID where the error occurred pub policy_id: PolicyID, - /// UID of the problematic entity - pub entity: EntityUID, - /// UID choices of the enumerated entity type - pub choices: Vec, + /// The error + pub err: InvalidEnumEntityError, } impl Diagnostic for InvalidEnumEntity { impl_diagnostic_from_source_loc_opt_field!(source_loc); fn help<'a>(&'a self) -> Option> { - Some(Box::new(format!("allowed UID values: {:?}", self.choices))) + self.err.help() } } diff --git a/cedar-policy/src/api/err.rs b/cedar-policy/src/api/err.rs index 30d45788b..598845f05 100644 --- a/cedar-policy/src/api/err.rs +++ b/cedar-policy/src/api/err.rs @@ -1250,7 +1250,7 @@ pub mod request_validation_errors { #[error(transparent)] #[diagnostic(transparent)] pub struct InvalidEnumEntityError( - #[from] cedar_policy_validator::request_validation_errors::InvalidEnumEntityError, + #[from] cedar_policy_core::entities::conformance::err::InvalidEnumEntityError, ); }