Skip to content

Commit

Permalink
unified errors in various places
Browse files Browse the repository at this point in the history
Signed-off-by: Shaobo He <[email protected]>
  • Loading branch information
shaobo-he-aws committed Jan 13, 2025
1 parent f73d838 commit 8811f1a
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 53 deletions.
51 changes: 42 additions & 9 deletions cedar-policy-core/src/entities/conformance/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -139,12 +141,15 @@ impl EntitySchemaConformanceError {

pub(crate) fn invalid_enum_entity(
uid: EntityUID,
choices: impl IntoIterator<Item = SmolStr>,
choices: impl IntoIterator<Item = Eid>,
) -> Self {
Self::InvalidEnumEntity(InvalidEnumEntity {
uid,
choices: choices.into_iter().collect(),
})
Self::InvalidEnumEntity(
InvalidEnumEntityError {
uid,
choices: choices.into_iter().collect(),
}
.into(),
)
}
}

Expand Down Expand Up @@ -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<InvalidEnumEntityError> 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<SmolStr>,
pub choices: Vec<Eid>,
}

impl Diagnostic for InvalidEnumEntityError {
impl_diagnostic_from_method_on_field!(uid, loc);

fn help<'a>(&'a self) -> Option<Box<dyn std::fmt::Display + 'a>> {
Some(Box::new(format!(
"valid entity eids: {}",
self.choices
.iter()
.map(|e| format!("\"{}\"", e.escaped()))
.join(", ")
)))
}
}
9 changes: 2 additions & 7 deletions cedar-policy-core/src/entities/json/entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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| <Eid as AsRef<SmolStr>>::as_ref(e).clone()),
),
EntitySchemaConformanceError::invalid_enum_entity(uid.clone(), choices),
));
}
}
Expand Down
34 changes: 7 additions & 27 deletions cedar-policy-validator/src/coreschema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand All @@ -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());
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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<ast::EntityUID>,
/// valid entity EIDs
pub(super) choices: NonEmpty<SmolStr>,
}

impl Diagnostic for InvalidEnumEntityError {
impl_diagnostic_from_method_on_field!(euid, loc);

fn help<'a>(&'a self) -> Option<Box<dyn std::fmt::Display + 'a>> {
Some(Box::new(format!("valid entity eids: {:?}", self.choices)))
}
}
}

/// Struct which carries enough information that it can impl Core's
Expand Down
9 changes: 6 additions & 3 deletions cedar-policy-validator/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@
//! 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;
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};
Expand Down Expand Up @@ -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()
}
Expand Down
11 changes: 5 additions & 6 deletions cedar-policy-validator/src/diagnostics/validation_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

//! Defines errors returned by the validator.
use cedar_policy_core::entities::conformance::err::InvalidEnumEntityError;
use miette::Diagnostic;
use thiserror::Error;

Expand Down Expand Up @@ -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<Loc>,
/// 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<SmolStr>,
/// The error
pub err: InvalidEnumEntityError,
}

impl Diagnostic for InvalidEnumEntity {
impl_diagnostic_from_source_loc_opt_field!(source_loc);

fn help<'a>(&'a self) -> Option<Box<dyn std::fmt::Display + 'a>> {
Some(Box::new(format!("allowed UID values: {:?}", self.choices)))
self.err.help()
}
}

Expand Down
2 changes: 1 addition & 1 deletion cedar-policy/src/api/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}

Expand Down

0 comments on commit 8811f1a

Please sign in to comment.