diff --git a/Cargo.lock b/Cargo.lock index ef276ad663b..a9f89863fdd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2598,6 +2598,7 @@ dependencies = [ "futures", "graph", "graph-api", + "graph-types", "hash-tracing", "mimalloc", "regex", diff --git a/apps/hash-graph/bins/cli/Cargo.toml b/apps/hash-graph/bins/cli/Cargo.toml index 9d6ac6d7a7f..84995927b2f 100644 --- a/apps/hash-graph/bins/cli/Cargo.toml +++ b/apps/hash-graph/bins/cli/Cargo.toml @@ -19,6 +19,7 @@ codec = { workspace = true } error-stack = { workspace = true } graph = { workspace = true, features = ["clap"] } graph-api = { workspace = true } +graph-types = { workspace = true } hash-tracing = { workspace = true, features = ["clap"] } temporal-client = { workspace = true } test-server = { workspace = true, optional = true } diff --git a/apps/hash-graph/bins/cli/package.json b/apps/hash-graph/bins/cli/package.json index 0abe61c0f8e..c172bde3838 100644 --- a/apps/hash-graph/bins/cli/package.json +++ b/apps/hash-graph/bins/cli/package.json @@ -20,6 +20,7 @@ "@rust/error-stack": "0.5.0", "@rust/graph": "0.0.0-private", "@rust/graph-api": "0.0.0-private", + "@rust/graph-types": "0.0.0-private", "@rust/hash-tracing": "0.0.0-private", "@rust/temporal-client": "0.0.0-private", "@rust/test-server": "0.0.0-private", diff --git a/apps/hash-graph/bins/cli/src/main.rs b/apps/hash-graph/bins/cli/src/main.rs index a77932363c4..ecd54dd228f 100644 --- a/apps/hash-graph/bins/cli/src/main.rs +++ b/apps/hash-graph/bins/cli/src/main.rs @@ -22,7 +22,7 @@ static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc; fn main() -> Result<(), GraphError> { load_env(None); - validation::error::install_error_stack_hooks(); + graph_types::knowledge::property::error::install_error_stack_hooks(); let Args { subcommand, diff --git a/apps/hash-graph/libs/graph/src/store/validation.rs b/apps/hash-graph/libs/graph/src/store/validation.rs index 4a34004fff1..7a4b9e6b78b 100644 --- a/apps/hash-graph/libs/graph/src/store/validation.rs +++ b/apps/hash-graph/libs/graph/src/store/validation.rs @@ -231,51 +231,6 @@ where .get(0)) } - #[expect(refining_impl_trait)] - async fn has_children(&self, data_type: &VersionedUrl) -> Result> { - let client = self.store.as_client().client(); - - Ok(client - .query_one( - " - SELECT EXISTS ( - SELECT 1 FROM data_type_inherits_from - WHERE target_data_type_ontology_id = $1 - ); - ", - &[&DataTypeId::from_url(data_type)], - ) - .await - .change_context(QueryError)? - .get(0)) - } - - #[expect(refining_impl_trait)] - async fn has_non_abstract_parents( - &self, - data_type: &VersionedUrl, - ) -> Result> { - let client = self.store.as_client().client(); - - Ok(client - .query_one( - " - SELECT EXISTS ( - SELECT 1 - FROM data_type_inherits_from - JOIN data_types - ON data_types.ontology_id = target_data_type_ontology_id - WHERE source_data_type_ontology_id = $1 - AND data_types.schema->>'abstract' = 'false' - ); - ", - &[&DataTypeId::from_url(data_type)], - ) - .await - .change_context(QueryError)? - .get(0)) - } - #[expect(refining_impl_trait)] async fn find_conversion( &self, diff --git a/apps/hash-graph/libs/store/src/filter/mod.rs b/apps/hash-graph/libs/store/src/filter/mod.rs index 402c144ae2f..14f0bca125a 100644 --- a/apps/hash-graph/libs/store/src/filter/mod.rs +++ b/apps/hash-graph/libs/store/src/filter/mod.rs @@ -532,14 +532,6 @@ mod tests { unimplemented!() } - async fn has_children(&self, _: &VersionedUrl) -> Result> { - unimplemented!() - } - - async fn has_non_abstract_parents(&self, _: &VersionedUrl) -> Result> { - unimplemented!() - } - async fn find_conversion( &self, _: &VersionedUrl, diff --git a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/any_of.rs b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/any_of.rs index f650d42a6aa..eefbe5ce014 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/any_of.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/any_of.rs @@ -30,6 +30,8 @@ impl AnyOfConstraints { .attempt(schema.constraints.validate_value(value)) .is_some() { + // We found a valid schema, so we can return early. + let _: Result<(), _> = status.finish(); return Ok(()); } } diff --git a/libs/@local/hash-validation/src/error.rs b/libs/@local/hash-graph-types/rust/src/knowledge/property/error.rs similarity index 96% rename from libs/@local/hash-validation/src/error.rs rename to libs/@local/hash-graph-types/rust/src/knowledge/property/error.rs index 8dbf9a916f7..3cca577c4c1 100644 --- a/libs/@local/hash-validation/src/error.rs +++ b/libs/@local/hash-graph-types/rust/src/knowledge/property/error.rs @@ -1,13 +1,14 @@ use std::collections::HashSet; use error_stack::Report; -use graph_types::knowledge::property::{PropertyWithMetadata, PropertyWithMetadataObject}; use serde_json::Value as JsonValue; use type_system::{ schema::{ClosedEntityType, DataType, PropertyType}, url::VersionedUrl, }; +use crate::knowledge::property::{PropertyWithMetadata, PropertyWithMetadataObject}; + pub fn install_error_stack_hooks() { Report::install_debug_hook::(|actual, context| match actual { Actual::Json(json) => context.push_body(format!("actual: {json:#}")), diff --git a/libs/@local/hash-graph-types/rust/src/knowledge/property/mod.rs b/libs/@local/hash-graph-types/rust/src/knowledge/property/mod.rs index 0a8a43ad10c..f73558c95f0 100644 --- a/libs/@local/hash-graph-types/rust/src/knowledge/property/mod.rs +++ b/libs/@local/hash-graph-types/rust/src/knowledge/property/mod.rs @@ -1,3 +1,4 @@ +pub mod error; pub mod visitor; pub use self::{ diff --git a/libs/@local/hash-graph-types/rust/src/knowledge/property/visitor.rs b/libs/@local/hash-graph-types/rust/src/knowledge/property/visitor.rs index 5d6d9e16670..1ee762cae57 100644 --- a/libs/@local/hash-graph-types/rust/src/knowledge/property/visitor.rs +++ b/libs/@local/hash-graph-types/rust/src/knowledge/property/visitor.rs @@ -15,6 +15,7 @@ use crate::{ knowledge::property::{ PropertyWithMetadata, PropertyWithMetadataArray, PropertyWithMetadataObject, PropertyWithMetadataValue, ValueMetadata, + error::{Actual, Expected}, }, ontology::{ DataTypeProvider, DataTypeWithMetadata, OntologyTypeProvider, PropertyTypeProvider, @@ -47,10 +48,12 @@ pub enum TraversalError { }, #[error("a value was expected, but the property provided was of type `{actual}`")] ExpectedValue { actual: JsonSchemaValueType }, - #[error("The property provided is ambiguous")] + #[error("The property provided is ambiguous, more than one schema passed the validation.")] AmbiguousProperty { actual: PropertyWithMetadata }, #[error("The data type ID was not specified and is ambiguous.")] AmbiguousDataType, + #[error("Could not find a suitable data type for the property")] + DataTypeUnspecified, #[error( "the value provided does not match the data type in the metadata, expected `{expected}` \ @@ -64,6 +67,15 @@ pub enum TraversalError { AbstractDataType { id: VersionedUrl }, #[error("the value provided does not match the constraints of the data type")] ConstraintUnfulfilled, + #[error("the value provided does not match the data type")] + DataTypeUnfulfilled, + #[error( + "the value provided does not match the property type. Exactly one constraint has to be \ + fulfilled." + )] + PropertyTypeUnfulfilled, + #[error("the entity provided does not match the entity type")] + EntityTypeUnfulfilled, #[error("the property `{key}` was required, but not specified")] MissingRequiredProperty { key: BaseUrl }, #[error( @@ -260,23 +272,31 @@ where V: EntityVisitor, P: DataTypeProvider + PropertyTypeProvider + Sync, { + let mut status = ReportSink::new(); match property { - PropertyWithMetadata::Value(value) => { + PropertyWithMetadata::Value(value) => status.attempt( visitor .visit_one_of_property(&schema.one_of, value, type_provider) .await - } - PropertyWithMetadata::Array(array) => { + .change_context_lazy(|| TraversalError::PropertyTypeUnfulfilled), + ), + PropertyWithMetadata::Array(array) => status.attempt( visitor .visit_one_of_array(&schema.one_of, array, type_provider) .await - } - PropertyWithMetadata::Object(object) => { + .change_context_lazy(|| TraversalError::PropertyTypeUnfulfilled), + ), + PropertyWithMetadata::Object(object) => status.attempt( visitor .visit_one_of_object(&schema.one_of, object, type_provider) .await - } - } + .change_context_lazy(|| TraversalError::PropertyTypeUnfulfilled), + ), + }; + status + .finish() + .attach_lazy(|| Expected::PropertyType(schema.clone())) + .attach_lazy(|| Actual::Property(property.clone())) } /// Walks through an array property using the provided schema. @@ -469,6 +489,9 @@ where type_provider, ) .await + .attach_lazy(|| Expected::DataType(data_type.borrow().schema.clone())) + .attach_lazy(|| Actual::Json(property.value.clone())) + .change_context(TraversalError::DataTypeUnfulfilled) { status.append(error); } else { @@ -490,7 +513,11 @@ where match passed { 0 => status.finish(), - 1 => Ok(()), + 1 => { + // We ignore potential errors here, as we have exactly one successful result. + let _: Result<(), _> = status.finish(); + Ok(()) + } _ => { status.capture(TraversalError::AmbiguousProperty { actual: PropertyWithMetadata::Value(property.clone()), @@ -549,7 +576,11 @@ where match passed { 0 => status.finish(), - 1 => Ok(()), + 1 => { + // We ignore potential errors here, as we have exactly one successful result. + let _: Result<(), _> = status.finish(); + Ok(()) + } _ => { status.capture(TraversalError::AmbiguousProperty { actual: PropertyWithMetadata::Array(array.clone()), @@ -609,7 +640,11 @@ where match passed { 0 => status.finish(), - 1 => Ok(()), + 1 => { + // We ignore potential errors here, as we have exactly one successful result. + let _: Result<(), _> = status.finish(); + Ok(()) + } _ => { status.capture(TraversalError::AmbiguousProperty { actual: PropertyWithMetadata::Object(object.clone()), diff --git a/libs/@local/hash-graph-types/rust/src/ontology/mod.rs b/libs/@local/hash-graph-types/rust/src/ontology/mod.rs index 7858ef11707..279f4bf63a7 100644 --- a/libs/@local/hash-graph-types/rust/src/ontology/mod.rs +++ b/libs/@local/hash-graph-types/rust/src/ontology/mod.rs @@ -184,20 +184,6 @@ pub trait DataTypeProvider: OntologyTypeProvider { parent: &BaseUrl, ) -> impl Future>> + Send; - // TODO: Remove when the data type ID is forced to be passed - // see https://linear.app/hash/issue/H-2800/validate-that-a-data-type-id-is-always-specified - fn has_children( - &self, - data_type: &VersionedUrl, - ) -> impl Future>> + Send; - - // TODO: Remove when the data type ID is forced to be passed - // see https://linear.app/hash/issue/H-2800/validate-that-a-data-type-id-is-always-specified - fn has_non_abstract_parents( - &self, - data_type: &VersionedUrl, - ) -> impl Future>> + Send; - fn find_conversion( &self, source_data_type_id: &VersionedUrl, diff --git a/libs/@local/hash-validation/src/entity_type.rs b/libs/@local/hash-validation/src/entity_type.rs index b176b7787bf..40e8b78bd78 100644 --- a/libs/@local/hash-validation/src/entity_type.rs +++ b/libs/@local/hash-validation/src/entity_type.rs @@ -380,43 +380,49 @@ impl EntityVisitor for EntityPreprocessor { // TODO: Remove when the data type ID is forced to be passed // see https://linear.app/hash/issue/H-2800/validate-that-a-data-type-id-is-always-specified if property.metadata.data_type_id.is_none() { + let mut infer_status = ReportSink::new(); let mut possible_data_types = HashSet::new(); for values in schema { if let PropertyValues::DataTypeReference(data_type_ref) = values { - let has_children = type_provider - .has_children(&data_type_ref.url) - .await - .change_context_lazy(|| TraversalError::DataTypeRetrieval { - id: data_type_ref.clone(), - })?; - if has_children { - status.capture(TraversalError::AmbiguousDataType); - possible_data_types.clear(); - break; - } - - let has_non_abstract_parents = type_provider - .has_non_abstract_parents(&data_type_ref.url) - .await - .change_context_lazy(|| TraversalError::DataTypeRetrieval { - id: data_type_ref.clone(), - })?; - - if has_non_abstract_parents { - status.capture(TraversalError::AmbiguousDataType); - possible_data_types.clear(); - break; + let Some(data_type) = infer_status.attempt( + type_provider + .provide_type(&data_type_ref.url) + .await + .change_context_lazy(|| TraversalError::DataTypeRetrieval { + id: data_type_ref.clone(), + }), + ) else { + continue; + }; + + if data_type.borrow().schema.r#abstract { + infer_status.capture(TraversalError::AbstractDataType { + id: data_type_ref.url.clone(), + }); + continue; } possible_data_types.insert(data_type_ref.url.clone()); } } + let inferred_successfully = status + .attempt( + infer_status + .finish() + .change_context(TraversalError::DataTypeUnspecified), + ) + .is_some(); + // Only if there is really a single valid data type ID, we set it. Note, that this is // done before the actual validation step. - if possible_data_types.len() == 1 { - property.metadata.data_type_id = possible_data_types.into_iter().next(); + if inferred_successfully { + if possible_data_types.len() == 1 { + property.metadata.data_type_id = possible_data_types.into_iter().next(); + } else { + status.capture(TraversalError::AmbiguousDataType); + } } } @@ -538,8 +544,6 @@ impl EntityVisitor for EntityPreprocessor { status.append(error); } } - } else { - status.capture(TraversalError::AmbiguousDataType); } if let Err(error) = walk_one_of_property_value(self, schema, property, type_provider).await diff --git a/libs/@local/hash-validation/src/lib.rs b/libs/@local/hash-validation/src/lib.rs index 2c111a08eab..61d0bd201cc 100644 --- a/libs/@local/hash-validation/src/lib.rs +++ b/libs/@local/hash-validation/src/lib.rs @@ -3,8 +3,6 @@ extern crate alloc; -pub mod error; - pub use self::entity_type::{EntityPreprocessor, EntityValidationError}; mod entity_type; @@ -102,6 +100,7 @@ mod tests { knowledge::property::{ Property, PropertyMetadata, PropertyObject, PropertyProvenance, PropertyWithMetadata, PropertyWithMetadataObject, PropertyWithMetadataValue, ValueMetadata, + error::install_error_stack_hooks, visitor::{EntityVisitor, TraversalError}, }, ontology::{ @@ -122,7 +121,6 @@ mod tests { use uuid::Uuid; use super::*; - use crate::error::install_error_stack_hooks; fn generate_data_type_metadata(schema: DataType) -> DataTypeWithMetadata { let actor = AccountId::new(Uuid::nil()); @@ -315,22 +313,6 @@ mod tests { ) } - #[expect(refining_impl_trait)] - async fn has_children( - &self, - _data_type: &VersionedUrl, - ) -> Result> { - Ok(false) - } - - #[expect(refining_impl_trait)] - async fn has_non_abstract_parents( - &self, - _data_type: &VersionedUrl, - ) -> Result> { - Ok(false) - } - #[expect(refining_impl_trait)] async fn find_conversion( &self,