Implement #745 for api::SchemaError

cedar-policy-validator/src/err.rs

@@ -22,6 +22,7 @@ use cedar_policy_core::{
 };
 use itertools::Itertools;
 use miette::Diagnostic;
+use smol_str::SmolStr;
 use thiserror::Error;
 
 use crate::human_schema;
@@ -140,28 +141,28 @@ pub enum SchemaError {
     #[diagnostic(help(
         "any entity types appearing anywhere in a schema need to be declared in `entityTypes`"
     ))]
-    UndeclaredEntityTypes(HashSet<String>),
+    UndeclaredEntityTypes(HashSet<Name>),
     /// Undeclared action(s) used in the `memberOf` field of an action.
     #[error("undeclared action(s): {0:?}")]
     #[diagnostic(help("any actions appearing in `memberOf` need to be declared in `actions`"))]
-    UndeclaredActions(HashSet<String>),
+    UndeclaredActions(HashSet<SmolStr>),
     /// This error occurs in either of the following cases (see discussion on #477):
     ///     - undeclared common type(s) appearing in entity or context attributes
     ///     - common type(s) (declared or not) appearing in declarations of other common types
     #[error("undeclared common type(s), or common type(s) used in the declaration of another common type: {0:?}")]
     #[diagnostic(help("any common types used in entity or context attributes need to be declared in `commonTypes`, and currently, common types may not reference other common types"))]
-    UndeclaredCommonTypes(HashSet<String>),
+    UndeclaredCommonTypes(HashSet<Name>),
     /// Duplicate specifications for an entity type. Argument is the name of
     /// the duplicate entity type.
     #[error("duplicate entity type `{0}`")]
-    DuplicateEntityType(String),
+    DuplicateEntityType(Name),
     /// Duplicate specifications for an action. Argument is the name of the
     /// duplicate action.
     #[error("duplicate action `{0}`")]
-    DuplicateAction(String),
+    DuplicateAction(SmolStr),
     /// Duplicate specification for a reusable type declaration.
     #[error("duplicate common type `{0}`")]
-    DuplicateCommonType(String),
+    DuplicateCommonType(Name),
     /// Cycle in the schema's action hierarchy.
     #[error("cycle in action hierarchy containing `{0}`")]
     CycleInActionHierarchy(EntityUID),
@@ -192,7 +193,7 @@ pub enum SchemaError {
     /// An action entity (transitively) has an attribute of unsupported type (`ExprEscape`, `EntityEscape` or `ExtnEscape`).
     /// This error variant should only be used when `PermitAttributes` is enabled.
     #[error("action `{0}` has an attribute with unsupported JSON representation: {1}")]
-    UnsupportedActionAttribute(EntityUID, String),
+    UnsupportedActionAttribute(EntityUID, SmolStr),
     /// Error when evaluating an action attribute
     #[error(transparent)]
     #[diagnostic(transparent)]

cedar-policy-validator/src/schema.rs

@@ -30,6 +30,7 @@ use cedar_policy_core::{
 };
 use serde::{Deserialize, Serialize};
 use serde_with::serde_as;
+use smol_str::ToSmolStr;
 
 use super::NamespaceDefinition;
 use crate::{
@@ -251,7 +252,7 @@ impl ValidatorSchema {
                 match type_defs.entry(name) {
                     Entry::Vacant(v) => v.insert(ty),
                     Entry::Occupied(o) => {
-                        return Err(SchemaError::DuplicateCommonType(o.key().to_string()));
+                        return Err(SchemaError::DuplicateCommonType(o.key().clone()));
                     }
                 };
             }
@@ -260,7 +261,7 @@ impl ValidatorSchema {
                 match entity_type_fragments.entry(name) {
                     Entry::Vacant(v) => v.insert(entity_type),
                     Entry::Occupied(o) => {
-                        return Err(SchemaError::DuplicateEntityType(o.key().to_string()))
+                        return Err(SchemaError::DuplicateEntityType(o.key().clone()))
                     }
                 };
             }
@@ -269,7 +270,7 @@ impl ValidatorSchema {
                 match action_fragments.entry(action_euid) {
                     Entry::Vacant(v) => v.insert(action),
                     Entry::Occupied(o) => {
-                        return Err(SchemaError::DuplicateAction(o.key().to_string()))
+                        return Err(SchemaError::DuplicateAction(o.key().to_smolstr()))
                     }
                 };
             }
@@ -398,7 +399,6 @@ impl ValidatorSchema {
         // any undeclared entity types which appeared in a `memberOf` list.
         let mut undeclared_e = undeclared_parent_entities
             .into_iter()
-            .map(|n| n.to_string())
             .collect::<HashSet<_>>();
         // Looking at entity types, we need to check entity references in
         // attribute types. We already know that all elements of the
@@ -418,7 +418,7 @@ impl ValidatorSchema {
         // Undeclared actions in a `memberOf` list.
         let undeclared_a = undeclared_parent_actions
             .into_iter()
-            .map(|n| n.to_string())
+            .map(|n| n.to_smolstr())
             .collect::<HashSet<_>>();
         // For actions, we check entity references in the context attribute
         // types and `appliesTo` lists. See the `entity_types` loop for why the
@@ -430,7 +430,7 @@ impl ValidatorSchema {
                 match p_entity {
                     EntityType::Specified(p_entity) => {
                         if !entity_types.contains_key(p_entity) {
-                            undeclared_e.insert(p_entity.to_string());
+                            undeclared_e.insert(p_entity.clone());
                         }
                     }
                     EntityType::Unspecified => (),
@@ -441,7 +441,7 @@ impl ValidatorSchema {
                 match r_entity {
                     EntityType::Specified(r_entity) => {
                         if !entity_types.contains_key(r_entity) {
-                            undeclared_e.insert(r_entity.to_string());
+                            undeclared_e.insert(r_entity.clone());
                         }
                     }
                     EntityType::Unspecified => (),
@@ -474,13 +474,13 @@ impl ValidatorSchema {
     fn check_undeclared_in_type(
         ty: &Type,
         entity_types: &HashMap<Name, ValidatorEntityType>,
-        undeclared_types: &mut HashSet<String>,
+        undeclared_types: &mut HashSet<Name>,
     ) {
         match ty {
             Type::EntityOrRecord(EntityRecordKind::Entity(lub)) => {
                 for name in lub.iter() {
                     if !entity_types.contains_key(name) {
-                        undeclared_types.insert(name.to_string());
+                        undeclared_types.insert(name.clone());
                     }
                 }
             }
@@ -794,7 +794,7 @@ impl<'a> CommonTypeResolver<'a> {
             SchemaType::TypeDef { type_name } => resolve_table
                 .get(&type_name)
                 .ok_or(SchemaError::UndeclaredCommonTypes(HashSet::from_iter(
-                    std::iter::once(type_name.to_string()),
+                    std::iter::once(type_name),
                 )))
                 .cloned(),
             SchemaType::Type(SchemaTypeVariant::Set { element }) => {
@@ -1040,7 +1040,7 @@ mod test {
         match schema {
             Ok(_) => panic!("try_into should have failed"),
             Err(SchemaError::UndeclaredEntityTypes(v)) => {
-                assert_eq!(v, HashSet::from(["Bar::Group".to_string()]))
+                assert_eq!(v, HashSet::from(["Bar::Group".parse().unwrap()]))
             }
             _ => panic!("Unexpected error from try_into"),
         }
@@ -1067,7 +1067,7 @@ mod test {
             Err(SchemaError::UndeclaredEntityTypes(v)) => {
                 assert_eq!(
                     v,
-                    HashSet::from(["Bar::Photo".to_string(), "Bar::User".to_string()])
+                    HashSet::from(["Bar::Photo".parse().unwrap(), "Bar::User".parse().unwrap()])
                 )
             }
             _ => panic!("Unexpected error from try_into"),
@@ -1267,7 +1267,7 @@ mod test {
         let schema: Result<ValidatorSchema> = schema_json.try_into();
         match schema {
             Err(SchemaError::UndeclaredEntityTypes(tys)) => {
-                assert_eq!(tys, HashSet::from(["C::D::Foo".to_string()]))
+                assert_eq!(tys, HashSet::from(["C::D::Foo".parse().unwrap()]))
             }
             _ => panic!("Schema construction should have failed due to undeclared entity type."),
         }
@@ -1833,7 +1833,7 @@ mod test {
         );
 
         match schema {
-            Err(SchemaError::DuplicateCommonType(s)) if s.contains("A::MyLong") => (),
+            Err(SchemaError::DuplicateCommonType(s)) if s == "A::MyLong".parse().unwrap() => (),
             _ => panic!("should have errored because schema fragments have duplicate types"),
         };
     }
@@ -2190,7 +2190,7 @@ mod test {
         );
         let schema = ValidatorSchema::from_json_value(src, Extensions::all_available());
         assert_matches!(schema, Err(SchemaError::UndeclaredCommonTypes(types)) =>
-            assert_eq!(types, HashSet::from(["Demo::id".to_string()])));
+            assert_eq!(types, HashSet::from(["Demo::id".parse().unwrap()])));
     }
 
     #[test]
@@ -2224,7 +2224,7 @@ mod test {
         );
         let schema = ValidatorSchema::from_json_value(src, Extensions::all_available());
         assert_matches!(schema, Err(SchemaError::UndeclaredCommonTypes(types)) =>
-            assert_eq!(types, HashSet::from(["Demo::id".to_string()])));
+            assert_eq!(types, HashSet::from(["Demo::id".parse().unwrap()])));
     }
 
     #[test]

cedar-policy-validator/src/schema/namespace_def.rs

@@ -244,7 +244,7 @@ impl ValidatorNamespaceDef {
         let mut type_defs = HashMap::with_capacity(schema_file_type_def.len());
         for (id, schema_ty) in schema_file_type_def {
             if Self::is_builtin_type_name(id.as_ref()) {
-                return Err(SchemaError::DuplicateCommonType(id.to_string()));
+                return Err(SchemaError::DuplicateCommonType(Name::unqualified_name(id)));
             }
             let name = Name::from(id.clone()).prefix_namespace_if_unqualified(schema_namespace);
             match type_defs.entry(name) {
@@ -254,7 +254,7 @@ impl ValidatorNamespaceDef {
                     );
                 }
                 Entry::Occupied(_) => {
-                    return Err(SchemaError::DuplicateCommonType(id.to_string()));
+                    return Err(SchemaError::DuplicateCommonType(Name::unqualified_name(id)));
                 }
             }
         }
@@ -288,7 +288,7 @@ impl ValidatorNamespaceDef {
                     });
                 }
                 Entry::Occupied(_) => {
-                    return Err(SchemaError::DuplicateEntityType(id.to_string()));
+                    return Err(SchemaError::DuplicateEntityType(Name::unqualified_name(id)));
                 }
             }
         }
@@ -337,24 +337,24 @@ impl ValidatorNamespaceDef {
             CedarValueJson::EntityEscape { __entity: _ } => {
                 Err(SchemaError::UnsupportedActionAttribute(
                     action_id.clone(),
-                    "entity escape (`__entity`)".to_owned(),
+                    "entity escape (`__entity`)".into(),
                 ))
             }
             CedarValueJson::ExprEscape { __expr: _ } => {
                 Err(SchemaError::UnsupportedActionAttribute(
                     action_id.clone(),
-                    "expression escape (`__expr`)".to_owned(),
+                    "expression escape (`__expr`)".into(),
                 ))
             }
             CedarValueJson::ExtnEscape { __extn: _ } => {
                 Err(SchemaError::UnsupportedActionAttribute(
                     action_id.clone(),
-                    "extension function escape (`__extn`)".to_owned(),
+                    "extension function escape (`__extn`)".into(),
                 ))
             }
             CedarValueJson::Null => Err(SchemaError::UnsupportedActionAttribute(
                 action_id.clone(),
-                "null".to_owned(),
+                "null".into(),
             )),
         }
     }
@@ -470,7 +470,7 @@ impl ValidatorNamespaceDef {
                     });
                 }
                 Entry::Occupied(_) => {
-                    return Err(SchemaError::DuplicateAction(action_id_str.to_string()));
+                    return Err(SchemaError::DuplicateAction(action_id_str));
                 }
             }
         }
@@ -669,9 +669,7 @@ impl ValidatorNamespaceDef {
                     type_name.prefix_namespace_if_unqualified(default_namespace);
                 Ok(WithUnresolvedTypeDefs::new(move |typ_defs| {
                     typ_defs.get(&defined_type_name).cloned().ok_or(
-                        SchemaError::UndeclaredCommonTypes(HashSet::from([
-                            defined_type_name.to_string()
-                        ])),
+                        SchemaError::UndeclaredCommonTypes(HashSet::from([defined_type_name])),
                     )
                 }))
             }

cedar-policy/src/api.rs

@@ -1213,12 +1213,14 @@ impl SchemaFragment {
 
     /// Serialize this [`SchemaFragment`] as a json value
     pub fn to_json_value(self) -> Result<serde_json::Value, SchemaError> {
-        serde_json::to_value(self.lossless).map_err(|e| SchemaError::JsonSerialization(e).into())
+        serde_json::to_value(self.lossless)
+            .map_err(|e| SchemaError::JsonSerialization(JsonSerializationError { error: e }).into())
     }
 
     /// Serialize this [`SchemaFragment`] as a json value
     pub fn as_json_string(&self) -> Result<String, SchemaError> {
-        serde_json::to_string(&self.lossless).map_err(|e| SchemaError::JsonSerialization(e).into())
+        serde_json::to_string(&self.lossless)
+            .map_err(|e| SchemaError::JsonSerialization(JsonSerializationError { error: e }).into())
     }
 
     /// Serialize this [`SchemaFragment`] into the natural syntax