From e4474107fd02393591bae3a73f357c41e4a628e3 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Tue, 10 Dec 2024 00:28:11 +0100 Subject: [PATCH 1/8] Improve validation structure for entity properties --- Cargo.lock | 1 + .../rust/src/schema/data_type/mod.rs | 1 + .../type-system/rust/src/schema/mod.rs | 2 +- .../rust/src/schema/property_type/mod.rs | 18 + libs/@local/graph/api/openapi/openapi.json | 948 +++++++++++++++++- libs/@local/graph/api/src/rest/data_type.rs | 4 +- libs/@local/graph/api/src/rest/entity.rs | 35 +- .../graph/api/src/rest/property_type.rs | 6 +- .../src/snapshot/entity/batch.rs | 9 +- .../store/postgres/knowledge/entity/mod.rs | 17 +- .../@local/graph/sdk/typescript/src/entity.ts | 4 +- libs/@local/graph/store/src/entity/mod.rs | 4 +- .../store/src/entity/validation_report.rs | 34 +- libs/@local/graph/types/rust/Cargo.toml | 3 +- .../types/rust/src/knowledge/property/mod.rs | 13 + .../rust/src/knowledge/property/visitor.rs | 680 +++++++++---- .../rust/src/ontology/data_type/lookup.rs | 8 + .../graph/types/typescript/src/validation.ts | 202 ++++ .../graph/validation/src/entity_type.rs | 225 +++-- libs/@local/graph/validation/src/lib.rs | 6 +- 20 files changed, 1863 insertions(+), 357 deletions(-) create mode 100644 libs/@local/graph/types/typescript/src/validation.ts diff --git a/Cargo.lock b/Cargo.lock index 18d640aa71a..575478a118f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3107,6 +3107,7 @@ name = "hash-graph-types" version = "0.0.0" dependencies = [ "bytes", + "derive_more 1.0.0", "error-stack", "futures", "hash-codec", diff --git a/libs/@blockprotocol/type-system/rust/src/schema/data_type/mod.rs b/libs/@blockprotocol/type-system/rust/src/schema/data_type/mod.rs index 958297f5785..b090d18cb7f 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/data_type/mod.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/data_type/mod.rs @@ -34,6 +34,7 @@ use crate::{schema::data_type::constraint::ValueConstraints, url::VersionedUrl}; #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] #[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] #[serde(rename_all = "kebab-case")] pub enum JsonSchemaValueType { Null, diff --git a/libs/@blockprotocol/type-system/rust/src/schema/mod.rs b/libs/@blockprotocol/type-system/rust/src/schema/mod.rs index 2a8197d1a43..d6fa9ea5df6 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/mod.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/mod.rs @@ -48,6 +48,6 @@ pub use self::{ one_of::{OneOfSchema, OneOfSchemaValidationError, OneOfSchemaValidator}, property_type::{ PropertyType, PropertyTypeReference, PropertyTypeValidationError, PropertyTypeValidator, - PropertyValueSchema, PropertyValues, + PropertyValueSchema, PropertyValueType, PropertyValues, }, }; diff --git a/libs/@blockprotocol/type-system/rust/src/schema/property_type/mod.rs b/libs/@blockprotocol/type-system/rust/src/schema/property_type/mod.rs index 8a33f85d17b..827bdd898a7 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/property_type/mod.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/property_type/mod.rs @@ -68,6 +68,15 @@ pub enum PropertyValues { ArrayOfPropertyValues(PropertyValueArray>), } +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(rename_all = "kebab-case")] +pub enum PropertyValueType { + Value, + Array, + Object, +} + impl PropertyValues { #[must_use] fn data_type_references(&self) -> Vec<&DataTypeReference> { @@ -103,6 +112,15 @@ impl PropertyValues { .collect(), } } + + #[must_use] + pub const fn property_value_type(&self) -> PropertyValueType { + match self { + Self::DataTypeReference(_) => PropertyValueType::Value, + Self::PropertyTypeObject(_) => PropertyValueType::Object, + Self::ArrayOfPropertyValues(_) => PropertyValueType::Array, + } + } } pub trait PropertyValueSchema { diff --git a/libs/@local/graph/api/openapi/openapi.json b/libs/@local/graph/api/openapi/openapi.json index 4be6c0bfe65..744d4befcf1 100644 --- a/libs/@local/graph/api/openapi/openapi.json +++ b/libs/@local/graph/api/openapi/openapi.json @@ -3078,6 +3078,77 @@ }, "additionalProperties": false }, + "ArrayItemNumberMismatch": { + "oneOf": [ + { + "type": "object", + "required": [ + "type", + "data" + ], + "properties": { + "data": { + "type": "object", + "required": [ + "actual", + "min" + ], + "properties": { + "actual": { + "type": "integer", + "minimum": 0 + }, + "min": { + "type": "integer", + "minimum": 0 + } + } + }, + "type": { + "type": "string", + "enum": [ + "tooFew" + ] + } + } + }, + { + "type": "object", + "required": [ + "type", + "data" + ], + "properties": { + "data": { + "type": "object", + "required": [ + "actual", + "max" + ], + "properties": { + "actual": { + "type": "integer", + "minimum": 0 + }, + "max": { + "type": "integer", + "minimum": 0 + } + } + }, + "type": { + "type": "string", + "enum": [ + "tooMany" + ] + } + } + } + ], + "discriminator": { + "propertyName": "type" + } + }, "ArrayMetadata": { "type": "object", "properties": { @@ -3094,6 +3165,25 @@ }, "additionalProperties": false }, + "ArrayValidationReport": { + "type": "object", + "properties": { + "numItems": { + "allOf": [ + { + "$ref": "#/components/schemas/ArrayItemNumberMismatch" + } + ], + "nullable": true + }, + "properties": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/PropertyValidationReport" + } + } + } + }, "BaseUrl": { "type": "string", "format": "uri" @@ -3453,6 +3543,174 @@ "DataType": { "$ref": "./models/data_type.json" }, + "DataTypeCanonicalCalculation": { + "oneOf": [ + { + "type": "object", + "required": [ + "error", + "type" + ], + "properties": { + "error": { + "$ref": "#/components/schemas/Report" + }, + "type": { + "type": "string", + "enum": [ + "retrieval" + ] + } + } + }, + { + "type": "object", + "required": [ + "data", + "type" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/JsonSchemaValueTypeMismatch" + }, + "type": { + "type": "string", + "enum": [ + "wrongType" + ] + } + } + }, + { + "type": "object", + "required": [ + "data", + "type" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/InvalidCanonicalValue" + }, + "type": { + "type": "string", + "enum": [ + "invalidValue" + ] + } + } + } + ], + "discriminator": { + "propertyName": "type" + } + }, + "DataTypeConversionError": { + "oneOf": [ + { + "type": "object", + "required": [ + "error", + "type" + ], + "properties": { + "error": { + "$ref": "#/components/schemas/Report" + }, + "type": { + "type": "string", + "enum": [ + "retrieval" + ] + } + } + }, + { + "type": "object", + "required": [ + "data", + "type" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/JsonSchemaValueTypeMismatch" + }, + "type": { + "type": "string", + "enum": [ + "wrongType" + ] + } + } + } + ], + "discriminator": { + "propertyName": "type" + } + }, + "DataTypeInferenceError": { + "oneOf": [ + { + "type": "object", + "required": [ + "error", + "type" + ], + "properties": { + "error": { + "$ref": "#/components/schemas/Report" + }, + "type": { + "type": "string", + "enum": [ + "retrieval" + ] + } + } + }, + { + "type": "object", + "required": [ + "data", + "type" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/VersionedUrl" + }, + "type": { + "type": "string", + "enum": [ + "abstract" + ] + } + } + }, + { + "type": "object", + "required": [ + "data", + "type" + ], + "properties": { + "data": { + "type": "array", + "items": { + "$ref": "#/components/schemas/VersionedUrl" + } + }, + "type": { + "type": "string", + "enum": [ + "ambiguous" + ] + } + } + } + ], + "discriminator": { + "propertyName": "type" + } + }, "DataTypeMetadata": { "oneOf": [ { @@ -4857,7 +5115,10 @@ "$ref": "#/components/schemas/MetadataValidationReport" }, "properties": { - "$ref": "#/components/schemas/PropertyValidationReport" + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/ObjectPropertyValidationReport" + } } } }, @@ -6120,6 +6381,53 @@ }, "additionalProperties": false }, + "InvalidCanonicalValue": { + "type": "object", + "required": [ + "key", + "expected", + "actual" + ], + "properties": { + "actual": { + "type": "number", + "format": "double" + }, + "expected": { + "type": "number", + "format": "double" + }, + "key": { + "$ref": "#/components/schemas/BaseUrl" + } + } + }, + "JsonSchemaValueType": { + "type": "string", + "enum": [ + "null", + "boolean", + "number", + "string", + "array", + "object" + ] + }, + "JsonSchemaValueTypeMismatch": { + "type": "object", + "required": [ + "actual", + "expected" + ], + "properties": { + "actual": { + "$ref": "#/components/schemas/JsonSchemaValueType" + }, + "expected": { + "$ref": "#/components/schemas/JsonSchemaValueType" + } + } + }, "KnowledgeGraphEdgeKind": { "type": "string", "enum": [ @@ -6839,15 +7147,354 @@ }, "additionalProperties": false }, - "OntologyEdgeKind": { - "type": "string", - "enum": [ - "INHERITS_FROM", - "CONSTRAINS_VALUES_ON", - "CONSTRAINS_PROPERTIES_ON", - "CONSTRAINS_LINKS_ON", - "CONSTRAINS_LINK_DESTINATIONS_ON" - ] + "ObjectPropertyValidationReport": { + "oneOf": [ + { + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "unexpected" + ] + } + } + }, + { + "type": "object", + "required": [ + "error", + "type" + ], + "properties": { + "error": { + "$ref": "#/components/schemas/Report" + }, + "type": { + "type": "string", + "enum": [ + "retrieval" + ] + } + } + }, + { + "type": "object", + "required": [ + "data", + "type" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/PropertyValueTypeMismatch" + }, + "type": { + "type": "string", + "enum": [ + "wrongType" + ] + } + } + }, + { + "allOf": [ + { + "$ref": "#/components/schemas/OneOfPropertyValidationReports" + }, + { + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "value" + ] + } + } + } + ] + }, + { + "allOf": [ + { + "$ref": "#/components/schemas/OneOfArrayValidationReports" + }, + { + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "array" + ] + } + } + } + ] + }, + { + "allOf": [ + { + "$ref": "#/components/schemas/OneOfObjectValidationReports" + }, + { + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "object" + ] + } + } + } + ] + }, + { + "allOf": [ + { + "$ref": "#/components/schemas/ArrayValidationReport" + }, + { + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "propertyArray" + ] + } + } + } + ] + }, + { + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "missing" + ] + } + } + } + ], + "discriminator": { + "propertyName": "type" + } + }, + "ObjectValidationReport": { + "type": "object", + "properties": { + "properties": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/ObjectPropertyValidationReport" + } + } + } + }, + "OneOfArrayValidationReports": { + "type": "object", + "properties": { + "validations": { + "type": "array", + "items": { + "$ref": "#/components/schemas/OneOfArrayValidationStatus" + }, + "nullable": true + } + } + }, + "OneOfArrayValidationStatus": { + "oneOf": [ + { + "type": "object", + "required": [ + "status" + ], + "properties": { + "status": { + "type": "string", + "enum": [ + "success" + ] + } + } + }, + { + "type": "object", + "required": [ + "status", + "data" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/PropertyArrayValidationReport" + }, + "status": { + "type": "string", + "enum": [ + "failure" + ] + } + } + } + ], + "discriminator": { + "propertyName": "status" + } + }, + "OneOfObjectValidationReports": { + "type": "object", + "properties": { + "validations": { + "type": "array", + "items": { + "$ref": "#/components/schemas/OneOfObjectValidationStatus" + }, + "nullable": true + } + } + }, + "OneOfObjectValidationStatus": { + "oneOf": [ + { + "type": "object", + "required": [ + "status" + ], + "properties": { + "status": { + "type": "string", + "enum": [ + "success" + ] + } + } + }, + { + "type": "object", + "required": [ + "status", + "data" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/PropertyObjectValidationReport" + }, + "status": { + "type": "string", + "enum": [ + "failure" + ] + } + } + } + ], + "discriminator": { + "propertyName": "status" + } + }, + "OneOfPropertyValidationReports": { + "type": "object", + "properties": { + "canonicalValue": { + "type": "array", + "items": { + "$ref": "#/components/schemas/DataTypeCanonicalCalculation" + } + }, + "dataTypeInference": { + "type": "array", + "items": { + "$ref": "#/components/schemas/DataTypeInferenceError" + } + }, + "validations": { + "type": "array", + "items": { + "$ref": "#/components/schemas/OneOfValueValidationStatus" + }, + "nullable": true + }, + "valueConversion": { + "allOf": [ + { + "$ref": "#/components/schemas/DataTypeConversionError" + } + ], + "nullable": true + } + } + }, + "OneOfValueValidationStatus": { + "oneOf": [ + { + "type": "object", + "required": [ + "status" + ], + "properties": { + "status": { + "type": "string", + "enum": [ + "success" + ] + } + } + }, + { + "type": "object", + "required": [ + "status", + "data" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/PropertyValueValidationReport" + }, + "status": { + "type": "string", + "enum": [ + "failure" + ] + } + } + } + ], + "discriminator": { + "propertyName": "status" + } + }, + "OntologyEdgeKind": { + "type": "string", + "enum": [ + "INHERITS_FROM", + "CONSTRAINS_VALUES_ON", + "CONSTRAINS_PROPERTIES_ON", + "CONSTRAINS_LINKS_ON", + "CONSTRAINS_LINK_DESTINATIONS_ON" + ] }, "OntologyEditionProvenance": { "allOf": [ @@ -7444,6 +8091,49 @@ {} ] }, + "PropertyArrayValidationReport": { + "oneOf": [ + { + "type": "object", + "required": [ + "type", + "data" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/PropertyValueTypeMismatch" + }, + "type": { + "type": "string", + "enum": [ + "wrongType" + ] + } + } + }, + { + "type": "object", + "required": [ + "type", + "data" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/ArrayValidationReport" + }, + "type": { + "type": "string", + "enum": [ + "arrayValidation" + ] + } + } + } + ], + "discriminator": { + "propertyName": "type" + } + }, "PropertyDiff": { "oneOf": [ { @@ -7592,6 +8282,52 @@ "$ref": "#/components/schemas/Property" } }, + "PropertyObjectValidationReport": { + "oneOf": [ + { + "type": "object", + "required": [ + "data", + "type" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/PropertyValueTypeMismatch" + }, + "type": { + "type": "string", + "enum": [ + "wrongType" + ] + } + } + }, + { + "allOf": [ + { + "$ref": "#/components/schemas/ObjectValidationReport" + }, + { + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "objectValidation" + ] + } + } + } + ] + } + ], + "discriminator": { + "propertyName": "type" + } + }, "PropertyPatchOperation": { "oneOf": [ { @@ -8009,17 +8745,201 @@ } } }, + "PropertyValidationError": { + "oneOf": [ + { + "type": "object", + "required": [ + "type", + "report" + ], + "properties": { + "report": { + "$ref": "#/components/schemas/Report" + }, + "type": { + "type": "string", + "enum": [ + "value" + ] + } + } + }, + { + "type": "object", + "required": [ + "type", + "report" + ], + "properties": { + "report": { + "$ref": "#/components/schemas/Report" + }, + "type": { + "type": "string", + "enum": [ + "array" + ] + } + } + } + ], + "discriminator": { + "propertyName": "type" + } + }, "PropertyValidationReport": { - "type": "object", - "properties": { - "error": { + "oneOf": [ + { + "allOf": [ + { + "$ref": "#/components/schemas/OneOfPropertyValidationReports" + }, + { + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "value" + ] + } + } + } + ] + }, + { + "allOf": [ + { + "$ref": "#/components/schemas/OneOfArrayValidationReports" + }, + { + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "array" + ] + } + } + } + ] + }, + { "allOf": [ { + "$ref": "#/components/schemas/OneOfObjectValidationReports" + }, + { + "type": "object", + "required": [ + "type" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "object" + ] + } + } + } + ] + } + ], + "discriminator": { + "propertyName": "type" + } + }, + "PropertyValueType": { + "type": "string", + "enum": [ + "value", + "array", + "object" + ] + }, + "PropertyValueTypeMismatch": { + "type": "object", + "required": [ + "actual", + "expected" + ], + "properties": { + "actual": { + "$ref": "#/components/schemas/PropertyValueType" + }, + "expected": { + "$ref": "#/components/schemas/PropertyValueType" + } + } + }, + "PropertyValueValidationReport": { + "oneOf": [ + { + "type": "object", + "required": [ + "error", + "type" + ], + "properties": { + "error": { "$ref": "#/components/schemas/Report" + }, + "type": { + "type": "string", + "enum": [ + "retrieval" + ] + } + } + }, + { + "type": "object", + "required": [ + "data", + "type" + ], + "properties": { + "data": { + "$ref": "#/components/schemas/PropertyValueTypeMismatch" + }, + "type": { + "type": "string", + "enum": [ + "wrongType" + ] } + } + }, + { + "type": "object", + "required": [ + "error", + "type" ], - "nullable": true + "properties": { + "error": { + "$ref": "#/components/schemas/Report" + }, + "type": { + "type": "string", + "enum": [ + "valueValidation" + ] + } + } } + ], + "discriminator": { + "propertyName": "type" } }, "PropertyWithMetadata": { diff --git a/libs/@local/graph/api/src/rest/data_type.rs b/libs/@local/graph/api/src/rest/data_type.rs index afdce654cd5..74ba4ad866b 100644 --- a/libs/@local/graph/api/src/rest/data_type.rs +++ b/libs/@local/graph/api/src/rest/data_type.rs @@ -48,7 +48,8 @@ use time::OffsetDateTime; use type_system::{ schema::{ ConversionDefinition, ConversionExpression, ConversionValue, Conversions, DataType, - DataTypeUuid, DomainValidator, Operator, ValidateOntologyType as _, Variable, + DataTypeUuid, DomainValidator, JsonSchemaValueType, Operator, ValidateOntologyType as _, + Variable, }, url::{BaseUrl, OntologyTypeVersion, VersionedUrl}, }; @@ -100,6 +101,7 @@ use crate::rest::{ ArchiveDataTypeParams, UnarchiveDataTypeParams, ClosedDataTypeDefinition, + JsonSchemaValueType, ConversionDefinition, ConversionExpression, diff --git a/libs/@local/graph/api/src/rest/entity.rs b/libs/@local/graph/api/src/rest/entity.rs index fbcb4155a86..83f222b7a65 100644 --- a/libs/@local/graph/api/src/rest/entity.rs +++ b/libs/@local/graph/api/src/rest/entity.rs @@ -32,8 +32,8 @@ use hash_graph_store::{ GetEntitiesResponse, GetEntitySubgraphParams, LinkDataStateError, LinkDataValidationReport, LinkError, LinkTargetError, LinkValidationReport, LinkedEntityError, MetadataValidationReport, PatchEntityParams, PropertyMetadataValidationReport, - PropertyValidationReport, QueryConversion, UnexpectedEntityType, - UpdateEntityEmbeddingsParams, ValidateEntityComponents, ValidateEntityParams, + QueryConversion, UnexpectedEntityType, UpdateEntityEmbeddingsParams, + ValidateEntityComponents, ValidateEntityParams, }, entity_type::{EntityTypeResolveDefinitions, IncludeEntityTypeOption}, filter::Filter, @@ -59,6 +59,17 @@ use hash_graph_types::{ PropertyPathElement, PropertyProvenance, PropertyWithMetadata, PropertyWithMetadataArray, PropertyWithMetadataObject, PropertyWithMetadataValue, ValueMetadata, + visitor::{ + ArrayItemNumberMismatch, ArrayValidationReport, DataTypeCanonicalCalculation, + DataTypeConversionError, DataTypeInferenceError, InvalidCanonicalValue, + JsonSchemaValueTypeMismatch, ObjectPropertyValidationReport, + ObjectValidationReport, OneOfArrayValidationReports, OneOfArrayValidationStatus, + OneOfObjectValidationReports, OneOfObjectValidationStatus, + OneOfPropertyValidationReports, OneOfValueValidationStatus, + PropertyArrayValidationReport, PropertyObjectValidationReport, + PropertyValidationError, PropertyValidationReport, PropertyValueTypeMismatch, + PropertyValueValidationReport, + }, }, }, owned_by_id::OwnedById, @@ -175,7 +186,27 @@ use crate::rest::{ MetadataValidationReport, EntityTypesError, PropertyMetadataValidationReport, + ObjectPropertyValidationReport, + JsonSchemaValueTypeMismatch, + PropertyValidationError, + ArrayValidationReport, + ArrayItemNumberMismatch, PropertyValidationReport, + OneOfPropertyValidationReports, + PropertyValueValidationReport, + ObjectValidationReport, + DataTypeConversionError, + DataTypeCanonicalCalculation, + DataTypeInferenceError, + PropertyValueTypeMismatch, + InvalidCanonicalValue, + OneOfValueValidationStatus, + OneOfArrayValidationReports, + OneOfArrayValidationStatus, + PropertyArrayValidationReport, + OneOfObjectValidationReports, + OneOfObjectValidationStatus, + PropertyObjectValidationReport, DiffEntityParams, DiffEntityResult, diff --git a/libs/@local/graph/api/src/rest/property_type.rs b/libs/@local/graph/api/src/rest/property_type.rs index bc109894250..3f07716ba7d 100644 --- a/libs/@local/graph/api/src/rest/property_type.rs +++ b/libs/@local/graph/api/src/rest/property_type.rs @@ -47,7 +47,10 @@ use hash_temporal_client::TemporalClient; use serde::{Deserialize, Serialize}; use time::OffsetDateTime; use type_system::{ - schema::{DomainValidator, PropertyType, PropertyTypeUuid, ValidateOntologyType as _}, + schema::{ + DomainValidator, PropertyType, PropertyTypeUuid, PropertyValueType, + ValidateOntologyType as _, + }, url::{OntologyTypeVersion, VersionedUrl}, }; use utoipa::{OpenApi, ToSchema}; @@ -89,6 +92,7 @@ use crate::rest::{ PropertyTypeRelationAndSubject, ModifyPropertyTypeAuthorizationRelationship, PropertyTypeEmbedding, + PropertyValueType, CreatePropertyTypeRequest, LoadExternalPropertyTypeRequest, diff --git a/libs/@local/graph/postgres-store/src/snapshot/entity/batch.rs b/libs/@local/graph/postgres-store/src/snapshot/entity/batch.rs index f4e6048d572..7b5ad012801 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/entity/batch.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/entity/batch.rs @@ -6,10 +6,7 @@ use hash_graph_authorization::{ AuthorizationApi, backend::ZanzibarBackend, schema::EntityRelationAndSubject, }; use hash_graph_store::{ - entity::{ - EntityStore as _, EntityValidationReport, PropertyValidationReport, - ValidateEntityComponents, - }, + entity::{EntityStore as _, EntityValidationReport, ValidateEntityComponents}, error::InsertionError, filter::Filter, query::Read, @@ -343,7 +340,7 @@ where components: validation_components, }; - if let Err(error) = preprocessor + if let Err(property_validation) = preprocessor .visit_object( &entity_type, &mut property_with_metadata, @@ -352,7 +349,7 @@ where .await { validation_reports.entry(index).or_default().properties = - PropertyValidationReport { error: Some(error) }; + property_validation.properties; } let (properties, metadata) = property_with_metadata.into_parts(); diff --git a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs index 8ed1b01ad13..14949c5bc4a 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs @@ -20,9 +20,8 @@ use hash_graph_store::{ ClosedMultiEntityTypeMap, CountEntitiesParams, CreateEntityParams, EmptyEntityTypes, EntityQueryPath, EntityQuerySorting, EntityStore, EntityTypeRetrieval, EntityTypesError, EntityValidationReport, EntityValidationType, GetEntitiesParams, GetEntitiesResponse, - GetEntitySubgraphParams, GetEntitySubgraphResponse, PatchEntityParams, - PropertyValidationReport, QueryConversion, UpdateEntityEmbeddingsParams, - ValidateEntityComponents, ValidateEntityParams, + GetEntitySubgraphParams, GetEntitySubgraphResponse, PatchEntityParams, QueryConversion, + UpdateEntityEmbeddingsParams, ValidateEntityComponents, ValidateEntityParams, }, entity_type::IncludeEntityTypeOption, error::{InsertionError, QueryError, UpdateError}, @@ -801,12 +800,12 @@ where }; preprocessor.components.link_validation = self.settings.validate_links; - if let Err(error) = preprocessor + if let Err(property_validation) = preprocessor .visit_object(&entity_type, &mut params.properties, &validator_provider) .await { validation_reports.entry(index).or_default().properties = - PropertyValidationReport { error: Some(error) }; + property_validation.properties; } let (properties, property_metadata) = params.properties.into_parts(); @@ -1215,7 +1214,7 @@ where components: params.components, }; - if let Err(error) = preprocessor + if let Err(property_validation) = preprocessor .visit_object( schema.as_ref(), params.properties.to_mut(), @@ -1224,7 +1223,7 @@ where .await { validation_reports.entry(index).or_default().properties = - PropertyValidationReport { error: Some(error) }; + property_validation.properties; } validation_report.link = params @@ -1741,11 +1740,11 @@ where let mut preprocessor = EntityPreprocessor { components: validation_components, }; - if let Err(error) = preprocessor + if let Err(property_validation) = preprocessor .visit_object(&entity_type, &mut object, &validator_provider) .await { - validation_report.properties = PropertyValidationReport { error: Some(error) }; + validation_report.properties = property_validation.properties; } let (properties, property_metadata) = object.into_parts(); diff --git a/libs/@local/graph/sdk/typescript/src/entity.ts b/libs/@local/graph/sdk/typescript/src/entity.ts index dfb00dbb132..f471335ebbe 100644 --- a/libs/@local/graph/sdk/typescript/src/entity.ts +++ b/libs/@local/graph/sdk/typescript/src/entity.ts @@ -3,7 +3,6 @@ import { typedEntries, typedKeys } from "@local/advanced-types/typed-entries"; import type { CreateEntityRequest as GraphApiCreateEntityRequest, Entity as GraphApiEntity, - EntityValidationReport, GraphApi, OriginProvenance, PatchEntityParams as GraphApiPatchEntityParams, @@ -55,6 +54,7 @@ import type { CreatedAtDecisionTime, CreatedAtTransactionTime, } from "@local/hash-graph-types/temporal-versioning"; +import type { EntityValidationReport } from "@local/hash-graph-types/validation"; import type { OwnedById } from "@local/hash-graph-types/web"; import type { AuthenticationContext } from "./authentication-context.js"; @@ -1000,7 +1000,7 @@ export class Entity { ): Promise { return await graphAPI .validateEntity(authentication.actorId, params) - .then(({ data }) => data["0"]); + .then(({ data }) => data["0"] as EntityValidationReport); } public async patch( diff --git a/libs/@local/graph/store/src/entity/mod.rs b/libs/@local/graph/store/src/entity/mod.rs index 5e18edb0389..3d768944501 100644 --- a/libs/@local/graph/store/src/entity/mod.rs +++ b/libs/@local/graph/store/src/entity/mod.rs @@ -16,8 +16,8 @@ pub use self::{ EmptyEntityTypes, EntityRetrieval, EntityTypeRetrieval, EntityTypesError, EntityValidationReport, LinkDataStateError, LinkDataValidationReport, LinkError, LinkTargetError, LinkValidationReport, LinkedEntityError, MetadataValidationReport, - MissingLinkData, PropertyMetadataValidationReport, PropertyValidationReport, - UnexpectedEntityType, UnexpectedLinkData, + MissingLinkData, PropertyMetadataValidationReport, UnexpectedEntityType, + UnexpectedLinkData, }, }; diff --git a/libs/@local/graph/store/src/entity/validation_report.rs b/libs/@local/graph/store/src/entity/validation_report.rs index 3ce34a6a02c..b1e0ae7d350 100644 --- a/libs/@local/graph/store/src/entity/validation_report.rs +++ b/libs/@local/graph/store/src/entity/validation_report.rs @@ -1,8 +1,13 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use error_stack::Report; -use hash_graph_types::knowledge::{entity::EntityId, property::visitor::TraversalError}; -use type_system::{schema::ResolveClosedEntityTypeError, url::VersionedUrl}; +use hash_graph_types::knowledge::{ + entity::EntityId, property::visitor::ObjectPropertyValidationReport, +}; +use type_system::{ + schema::ResolveClosedEntityTypeError, + url::{BaseUrl, VersionedUrl}, +}; #[derive(Debug, derive_more::Display, derive_more::Error)] #[display("Could not read the entity")] @@ -172,28 +177,13 @@ impl MetadataValidationReport { } } -#[derive(Debug, Default, serde::Serialize)] -#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] -#[serde(rename_all = "camelCase")] -#[must_use] -pub struct PropertyValidationReport { - pub error: Option>, -} - -impl PropertyValidationReport { - #[must_use] - pub const fn is_valid(&self) -> bool { - self.error.is_none() - } -} - #[derive(Debug, Default, serde::Serialize)] #[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] #[serde(rename_all = "camelCase")] #[must_use] pub struct EntityValidationReport { - #[serde(skip_serializing_if = "PropertyValidationReport::is_valid")] - pub properties: PropertyValidationReport, + #[serde(skip_serializing_if = "HashMap::is_empty")] + pub properties: HashMap, #[serde(skip_serializing_if = "LinkValidationReport::is_valid")] pub link: LinkValidationReport, #[serde(skip_serializing_if = "MetadataValidationReport::is_valid")] @@ -202,7 +192,7 @@ pub struct EntityValidationReport { impl EntityValidationReport { #[must_use] - pub const fn is_valid(&self) -> bool { - self.link.is_valid() && self.metadata.is_valid() + pub fn is_valid(&self) -> bool { + self.properties.is_empty() && self.link.is_valid() && self.metadata.is_valid() } } diff --git a/libs/@local/graph/types/rust/Cargo.toml b/libs/@local/graph/types/rust/Cargo.toml index 2fa8c542372..0ab96161f9b 100644 --- a/libs/@local/graph/types/rust/Cargo.toml +++ b/libs/@local/graph/types/rust/Cargo.toml @@ -15,11 +15,12 @@ type-system = { workspace = true, public = true } semver = { workspace = true, public = true, features = ["serde"] } # Private workspace dependencies -error-stack = { workspace = true } +error-stack = { workspace = true, features = ["serde"] } hash-codec = { workspace = true, features = ["bytes"] } # Private third-party dependencies bytes = { workspace = true } +derive_more = { workspace = true, features = ["display", "error"] } futures = { workspace = true } postgres-types = { workspace = true, features = ["derive", "with-uuid-1", "with-serde_json-1"], optional = true } serde = { workspace = true, features = ["derive"] } diff --git a/libs/@local/graph/types/rust/src/knowledge/property/mod.rs b/libs/@local/graph/types/rust/src/knowledge/property/mod.rs index da3cfd161d6..4cf1d5c3334 100644 --- a/libs/@local/graph/types/rust/src/knowledge/property/mod.rs +++ b/libs/@local/graph/types/rust/src/knowledge/property/mod.rs @@ -1,6 +1,8 @@ pub mod error; pub mod visitor; +use type_system::schema::PropertyValueType; + pub use self::{ array::PropertyWithMetadataArray, diff::PropertyDiff, @@ -50,6 +52,17 @@ pub enum PropertyWithMetadata { Value(PropertyWithMetadataValue), } +impl PropertyWithMetadata { + #[must_use] + pub const fn property_value_type(&self) -> PropertyValueType { + match self { + Self::Array(_) => PropertyValueType::Array, + Self::Object(_) => PropertyValueType::Object, + Self::Value(_) => PropertyValueType::Value, + } + } +} + #[derive(Debug, thiserror::Error)] pub enum PropertyPathError { #[error("Property path is empty")] diff --git a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs index 17634e1d226..944b160a85c 100644 --- a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs +++ b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs @@ -1,12 +1,14 @@ use core::{borrow::Borrow as _, future::Future}; +use std::collections::{HashMap, HashSet}; -use error_stack::{Report, ReportSink, ResultExt as _, bail}; +use error_stack::{Report, ReportSink, ResultExt as _}; +use futures::FutureExt as _; use serde_json::Value as JsonValue; use type_system::{ schema::{ DataTypeReference, JsonSchemaValueType, PropertyObjectSchema, PropertyType, - PropertyTypeReference, PropertyValueArray, PropertyValueSchema, PropertyValues, - ValueOrArray, + PropertyTypeReference, PropertyValueArray, PropertyValueSchema, PropertyValueType, + PropertyValues, ValueOrArray, }, url::{BaseUrl, VersionedUrl}, }; @@ -15,7 +17,6 @@ use crate::{ knowledge::property::{ PropertyWithMetadata, PropertyWithMetadataArray, PropertyWithMetadataObject, PropertyWithMetadataValue, ValueMetadata, - error::{Actual, Expected}, }, ontology::{DataTypeLookup, DataTypeWithMetadata, OntologyTypeProvider}, }; @@ -96,6 +97,271 @@ pub enum TraversalError { }, } +#[derive(Debug, derive_more::Display, derive_more::Error)] +#[display("Could not read the data type {}", data_type_reference.url)] +#[must_use] +pub struct DataTypeRetrieval { + pub data_type_reference: DataTypeReference, +} + +#[derive(Debug, derive_more::Display, derive_more::Error)] +#[display("Could not read the property type {}", property_type_reference.url)] +#[must_use] +pub struct PropertyTypeRetrieval { + pub property_type_reference: PropertyTypeReference, +} + +#[derive(Debug, derive_more::Display, derive_more::Error)] +#[display("Could not find a conversion from {} to {}", current.url, target.url)] +#[must_use] +pub struct ConversionRetrieval { + pub current: DataTypeReference, + pub target: DataTypeReference, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(rename_all = "camelCase")] +pub struct JsonSchemaValueTypeMismatch { + pub actual: JsonSchemaValueType, + pub expected: JsonSchemaValueType, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(rename_all = "camelCase")] +pub struct PropertyValueTypeMismatch { + pub actual: PropertyValueType, + pub expected: PropertyValueType, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(rename_all = "camelCase")] +pub struct InvalidCanonicalValue { + pub key: BaseUrl, + pub expected: f64, + pub actual: f64, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", content = "report", rename_all = "camelCase")] +#[must_use] +pub enum PropertyValidationError { + Value(Report<[TraversalError]>), + Array(Report<[TraversalError]>), +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", rename_all = "camelCase")] +#[must_use] +pub enum DataTypeInferenceError { + Retrieval { + error: Report, + }, + Abstract { + data: VersionedUrl, + }, + Ambiguous { + #[cfg_attr(feature = "utoipa", schema(value_type = [VersionedUrl]))] + data: HashSet, + }, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", rename_all = "camelCase")] +#[must_use] +pub enum DataTypeConversionError { + Retrieval { error: Report }, + WrongType { data: JsonSchemaValueTypeMismatch }, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", rename_all = "camelCase")] +#[must_use] +pub enum DataTypeCanonicalCalculation { + Retrieval { error: Report }, + WrongType { data: JsonSchemaValueTypeMismatch }, + InvalidValue { data: InvalidCanonicalValue }, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", rename_all = "camelCase")] +#[must_use] +pub enum PropertyValueValidationReport { + Retrieval { error: Report }, + WrongType { data: PropertyValueTypeMismatch }, + ValueValidation { error: Report<[TraversalError]> }, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "status", content = "data", rename_all = "camelCase")] +#[must_use] +// Should be an `Option` or a generic `OneOfValidationStatus` but `utoipa` is not capable of +// properly deal with that. +pub enum OneOfValueValidationStatus { + Success, + Failure(PropertyValueValidationReport), +} + +#[derive(Debug, Default, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(rename_all = "camelCase")] +pub struct OneOfPropertyValidationReports { + #[serde(skip_serializing_if = "Option::is_none")] + pub validations: Option>, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub data_type_inference: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub value_conversion: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub canonical_value: Vec, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", content = "data", rename_all = "camelCase")] +#[must_use] +pub enum PropertyArrayValidationReport { + WrongType(PropertyValueTypeMismatch), + ArrayValidation(ArrayValidationReport), +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "status", content = "data", rename_all = "camelCase")] +#[must_use] +// Should be an `Option` or a generic `OneOfValidationStatus` but `utoipa` is not capable of +// properly deal with that. +pub enum OneOfArrayValidationStatus { + Success, + Failure(PropertyArrayValidationReport), +} + +#[derive(Debug, Default, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(rename_all = "camelCase")] +pub struct OneOfArrayValidationReports { + #[serde(skip_serializing_if = "Option::is_none")] + pub validations: Option>, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", rename_all = "camelCase")] +#[must_use] +pub enum PropertyObjectValidationReport { + WrongType { data: PropertyValueTypeMismatch }, + ObjectValidation(ObjectValidationReport), +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "status", content = "data", rename_all = "camelCase")] +#[must_use] +// Should be an `Option` or a generic `OneOfValidationStatus` but `utoipa` is not capable of +// properly deal with that. +pub enum OneOfObjectValidationStatus { + Success, + Failure(PropertyObjectValidationReport), +} + +#[derive(Debug, Default, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(rename_all = "camelCase")] +pub struct OneOfObjectValidationReports { + #[serde(skip_serializing_if = "Option::is_none")] + pub validations: Option>, +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", rename_all = "camelCase")] +#[must_use] +pub enum PropertyValidationReport { + Value(OneOfPropertyValidationReports), + Array(OneOfArrayValidationReports), + Object(OneOfObjectValidationReports), +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", content = "data", rename_all = "camelCase")] +#[must_use] +pub enum ArrayItemNumberMismatch { + TooFew { actual: usize, min: usize }, + TooMany { actual: usize, max: usize }, +} + +#[derive(Debug, Default, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(rename_all = "camelCase")] +#[must_use] +pub struct ArrayValidationReport { + #[serde(skip_serializing_if = "HashMap::is_empty")] + pub properties: HashMap, + #[serde(skip_serializing_if = "Option::is_none")] + pub num_items: Option, +} + +impl ArrayValidationReport { + #[must_use] + pub fn is_valid(&self) -> bool { + self.properties.is_empty() && self.num_items.is_none() + } +} + +#[derive(Debug, Default, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(rename_all = "camelCase")] +#[must_use] +pub struct ObjectValidationReport { + #[serde(skip_serializing_if = "HashMap::is_empty")] + pub properties: HashMap, +} + +impl ObjectValidationReport { + #[must_use] + pub fn is_valid(&self) -> bool { + self.properties.is_empty() + } +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", rename_all = "camelCase")] +#[must_use] +pub enum ObjectPropertyValidationReport { + Unexpected, + Retrieval { + error: Report, + }, + WrongType { + data: PropertyValueTypeMismatch, + }, + Value(OneOfPropertyValidationReports), + Array(OneOfArrayValidationReports), + Object(OneOfObjectValidationReports), + PropertyArray(ArrayValidationReport), + Missing, +} + +impl From for ObjectPropertyValidationReport { + fn from(property_validation: PropertyValidationReport) -> Self { + match property_validation { + PropertyValidationReport::Value(report) => Self::Value(report), + PropertyValidationReport::Array(report) => Self::Array(report), + PropertyValidationReport::Object(report) => Self::Object(report), + } + } +} + // TODO: Allow usage of other error types pub trait EntityVisitor: Sized + Send + Sync { /// Visits a leaf value. @@ -122,7 +388,7 @@ pub trait EntityVisitor: Sized + Send + Sync { schema: &PropertyType, property: &mut PropertyWithMetadata, type_provider: &P, - ) -> impl Future>> + Send + ) -> impl Future> + Send where P: DataTypeLookup + OntologyTypeProvider + Sync, { @@ -137,12 +403,21 @@ pub trait EntityVisitor: Sized + Send + Sync { schema: &PropertyValueArray, array: &mut PropertyWithMetadataArray, type_provider: &P, - ) -> impl Future>> + Send + ) -> impl Future> + Send where T: PropertyValueSchema + Sync, P: DataTypeLookup + OntologyTypeProvider + Sync, { - walk_array(self, schema, array, type_provider) + walk_array(self, schema, array, type_provider).map(|properties| { + if properties.is_empty() { + Ok(()) + } else { + Err(ArrayValidationReport { + properties, + ..ArrayValidationReport::default() + }) + } + }) } /// Visits an object property. @@ -153,12 +428,18 @@ pub trait EntityVisitor: Sized + Send + Sync { schema: &T, object: &mut PropertyWithMetadataObject, type_provider: &P, - ) -> impl Future>> + Send + ) -> impl Future> + Send where T: PropertyObjectSchema> + Sync, P: DataTypeLookup + OntologyTypeProvider + Sync, { - walk_object(self, schema, object, type_provider) + walk_object(self, schema, object, type_provider).map(|properties| { + if properties.is_empty() { + Ok(()) + } else { + Err(ObjectValidationReport { properties }) + } + }) } /// Visits a property value using the [`PropertyValues`] from a one-of schema. @@ -169,7 +450,7 @@ pub trait EntityVisitor: Sized + Send + Sync { schema: &[PropertyValues], property: &mut PropertyWithMetadataValue, type_provider: &P, - ) -> impl Future>> + Send + ) -> impl Future> + Send where P: DataTypeLookup + Sync, { @@ -184,7 +465,7 @@ pub trait EntityVisitor: Sized + Send + Sync { schema: &[PropertyValues], array: &mut PropertyWithMetadataArray, type_provider: &P, - ) -> impl Future>> + Send + ) -> impl Future> + Send where P: DataTypeLookup + OntologyTypeProvider + Sync, { @@ -199,7 +480,7 @@ pub trait EntityVisitor: Sized + Send + Sync { schema: &[PropertyValues], object: &mut PropertyWithMetadataObject, type_provider: &P, - ) -> impl Future>> + Send + ) -> impl Future> + Send where P: DataTypeLookup + OntologyTypeProvider + Sync, { @@ -265,36 +546,25 @@ pub async fn walk_property( schema: &PropertyType, property: &mut PropertyWithMetadata, type_provider: &P, -) -> Result<(), Report<[TraversalError]>> +) -> Result<(), PropertyValidationReport> where V: EntityVisitor, P: DataTypeLookup + OntologyTypeProvider + Sync, { - let mut status = ReportSink::new(); match property { - PropertyWithMetadata::Value(value) => status.attempt( - visitor - .visit_one_of_property(&schema.one_of, value, type_provider) - .await - .change_context_lazy(|| TraversalError::PropertyTypeUnfulfilled), - ), - PropertyWithMetadata::Array(array) => status.attempt( - visitor - .visit_one_of_array(&schema.one_of, array, type_provider) - .await - .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())) + PropertyWithMetadata::Value(value) => visitor + .visit_one_of_property(&schema.one_of, value, type_provider) + .await + .map_err(PropertyValidationReport::Value), + PropertyWithMetadata::Array(array) => visitor + .visit_one_of_array(&schema.one_of, array, type_provider) + .await + .map_err(PropertyValidationReport::Array), + PropertyWithMetadata::Object(object) => visitor + .visit_one_of_object(&schema.one_of, object, type_provider) + .await + .map_err(PropertyValidationReport::Object), + } } /// Walks through an array property using the provided schema. @@ -302,52 +572,51 @@ where /// Depending on the property, [`EntityVisitor::visit_one_of_property`], /// [`EntityVisitor::visit_one_of_array`], or [`EntityVisitor::visit_one_of_object`] is called. /// -/// # Errors -/// -/// Any error that can be returned by the visitor methods. +/// Returns a detailed report about the validation failures for each property in the array. pub async fn walk_array( visitor: &mut V, schema: &PropertyValueArray, array: &mut PropertyWithMetadataArray, type_provider: &P, -) -> Result<(), Report<[TraversalError]>> +) -> HashMap where V: EntityVisitor, S: PropertyValueSchema + Sync, P: DataTypeLookup + OntologyTypeProvider + Sync, { - let mut status = ReportSink::new(); + let mut properties = HashMap::new(); + let schema_possibilities = schema.items.possibilities(); - for property in &mut array.value { + for (index, property) in array.value.iter_mut().enumerate() { match property { PropertyWithMetadata::Value(value) => { - if let Err(error) = visitor - .visit_one_of_property(schema.items.possibilities(), value, type_provider) + let _: Result<(), ()> = visitor + .visit_one_of_property(schema_possibilities, value, type_provider) .await - { - status.append(error); - } + .map_err(|report| { + properties.insert(index, PropertyValidationReport::Value(report)); + }); } PropertyWithMetadata::Array(array) => { - if let Err(error) = visitor - .visit_one_of_array(schema.items.possibilities(), array, type_provider) + let _: Result<(), ()> = visitor + .visit_one_of_array(schema_possibilities, array, type_provider) .await - { - status.append(error); - } + .map_err(|report| { + properties.insert(index, PropertyValidationReport::Array(report)); + }); } PropertyWithMetadata::Object(object) => { - if let Err(error) = visitor - .visit_one_of_object(schema.items.possibilities(), object, type_provider) + let _: Result<(), ()> = visitor + .visit_one_of_object(schema_possibilities, object, type_provider) .await - { - status.append(error); - } + .map_err(|report| { + properties.insert(index, PropertyValidationReport::Object(report)); + }); } } } - status.finish() + properties } /// Walks through a property object using the provided schema. @@ -359,65 +628,84 @@ where /// Depending on the property, [`EntityVisitor::visit_property`] or [`EntityVisitor::visit_array`] /// is called. /// -/// # Errors -/// -/// - [`UnexpectedProperty`] if a property is specified that is not in the schema. -/// - [`PropertyTypeRetrieval`] if a property type could not be retrieved from the `type_provider`. -/// - [`InvalidType`] if the schema expects an array, but a value or object is provided. -/// - Any error that can be returned by the visitor methods. -/// -/// [`UnexpectedProperty`]: TraversalError::UnexpectedProperty -/// [`PropertyTypeRetrieval`]: TraversalError::PropertyTypeRetrieval -/// [`InvalidType`]: TraversalError::InvalidType +/// Returns a detailed report about the validation failures for each property in the object. pub async fn walk_object( visitor: &mut V, schema: &S, object: &mut PropertyWithMetadataObject, type_provider: &P, -) -> Result<(), Report<[TraversalError]>> +) -> HashMap where V: EntityVisitor, S: PropertyObjectSchema> + Sync, P: DataTypeLookup + OntologyTypeProvider + Sync, { - let mut status = ReportSink::new(); + let mut validation_map = HashMap::new(); for (base_url, property) in &mut object.value { let Some(property_type_reference) = schema.properties().get(base_url) else { - status.capture(TraversalError::UnexpectedProperty { - key: base_url.clone(), - }); - + validation_map.insert( + base_url.clone(), + ObjectPropertyValidationReport::Unexpected {}, + ); continue; }; match property_type_reference { ValueOrArray::Value(property_type_reference) => { - let property_type =

>::provide_type( + let Some(property_type) =

>::provide_type( type_provider, &property_type_reference.url, ) .await - .change_context_lazy(|| TraversalError::PropertyTypeRetrieval { - id: property_type_reference.clone(), - })?; - visitor + .map_err(|report| { + validation_map.insert( + base_url.clone(), + ObjectPropertyValidationReport::Retrieval { + error: report.change_context(PropertyTypeRetrieval { + property_type_reference: property_type_reference.clone(), + }), + }, + ); + }) + .ok() else { + continue; + }; + + let _: Result<(), ()> = visitor .visit_property(property_type.borrow(), property, type_provider) - .await?; + .await + .map_err(|property_validation| { + validation_map.insert( + base_url.clone(), + ObjectPropertyValidationReport::from(property_validation), + ); + }); } ValueOrArray::Array(array_schema) => match property { PropertyWithMetadata::Array(array) => { - let property_type =

>::provide_type( - type_provider, - &array_schema.items.url, - ) - .await - .change_context_lazy(|| { - TraversalError::PropertyTypeRetrieval { - id: array_schema.items.clone(), - } - })?; - let result = visitor + let Some(property_type) = +

>::provide_type( + type_provider, + &array_schema.items.url, + ) + .await + .map_err(|report| { + validation_map.insert( + base_url.clone(), + ObjectPropertyValidationReport::Retrieval { + error: report.change_context(PropertyTypeRetrieval { + property_type_reference: array_schema.items.clone(), + }), + }, + ); + }) + .ok() + else { + continue; + }; + + let _: Result<(), ()> = visitor .visit_array( &PropertyValueArray { items: property_type.borrow(), @@ -427,22 +715,30 @@ where array, type_provider, ) - .await; - if let Err(error) = result { - status.append(error); - } + .await + .map_err(|array_validation| { + validation_map.insert( + base_url.clone(), + ObjectPropertyValidationReport::PropertyArray(array_validation), + ); + }); } - PropertyWithMetadata::Object { .. } | PropertyWithMetadata::Value(_) => { - bail![TraversalError::InvalidType { - actual: property.json_type(), - expected: JsonSchemaValueType::Array, - },] + PropertyWithMetadata::Value(_) | PropertyWithMetadata::Object { .. } => { + validation_map.insert( + base_url.clone(), + ObjectPropertyValidationReport::WrongType { + data: PropertyValueTypeMismatch { + actual: property.property_value_type(), + expected: PropertyValueType::Array, + }, + }, + ); } }, }; } - status.finish() + validation_map } /// Walks through a property value using the provided schema list. @@ -462,23 +758,31 @@ pub async fn walk_one_of_property_value( schema: &[PropertyValues], property: &mut PropertyWithMetadataValue, type_provider: &P, -) -> Result<(), Report<[TraversalError]>> +) -> Result<(), OneOfPropertyValidationReports> where V: EntityVisitor, P: DataTypeLookup + Sync, { - let mut status = ReportSink::new(); - let mut passed: usize = 0; + let mut property_validations = Vec::with_capacity(schema.len()); + let mut num_passed: usize = 0; for schema in schema { match schema { PropertyValues::DataTypeReference(data_type_ref) => { - let data_type = type_provider - .lookup_data_type_by_ref(data_type_ref) - .await - .change_context_lazy(|| TraversalError::DataTypeRetrieval { - id: data_type_ref.clone(), - })?; + let data_type = match type_provider.lookup_data_type_by_ref(data_type_ref).await { + Ok(data_type) => data_type, + Err(error) => { + property_validations.push(OneOfValueValidationStatus::Failure( + PropertyValueValidationReport::Retrieval { + error: error.change_context(DataTypeRetrieval { + data_type_reference: data_type_ref.clone(), + }), + }, + )); + continue; + } + }; + if let Err(error) = visitor .visit_value( data_type.borrow(), @@ -487,41 +791,35 @@ 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); + property_validations.push(OneOfValueValidationStatus::Failure( + PropertyValueValidationReport::ValueValidation { error }, + )); } else { - passed += 1; + num_passed += 1; + property_validations.push(OneOfValueValidationStatus::Success); } } - PropertyValues::ArrayOfPropertyValues(_) => { - status.capture(TraversalError::ExpectedValue { - actual: JsonSchemaValueType::Array, - }); - } - PropertyValues::PropertyTypeObject(_) => { - status.capture(TraversalError::ExpectedValue { - actual: JsonSchemaValueType::Object, - }); + PropertyValues::ArrayOfPropertyValues(_) | PropertyValues::PropertyTypeObject(_) => { + property_validations.push(OneOfValueValidationStatus::Failure( + PropertyValueValidationReport::WrongType { + data: PropertyValueTypeMismatch { + actual: schema.property_value_type(), + expected: PropertyValueType::Value, + }, + }, + )); } } } - match passed { - 0 => status.finish(), - 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()), - }); - status.finish() - } + if num_passed == 1 { + Ok(()) + } else { + Err(OneOfPropertyValidationReports { + validations: Some(property_validations), + ..OneOfPropertyValidationReports::default() + }) } } @@ -540,52 +838,45 @@ pub async fn walk_one_of_array( schema: &[PropertyValues], array: &mut PropertyWithMetadataArray, type_provider: &P, -) -> Result<(), Report<[TraversalError]>> +) -> Result<(), OneOfArrayValidationReports> where V: EntityVisitor, P: DataTypeLookup + OntologyTypeProvider + Sync, { - let mut status = ReportSink::new(); - let mut passed: usize = 0; + let mut array_validations = Vec::with_capacity(schema.len()); + let mut num_passed: usize = 0; for schema in schema { match schema { - PropertyValues::DataTypeReference(_) => { - status.capture(TraversalError::ExpectedValue { - actual: JsonSchemaValueType::Array, - }); - } PropertyValues::ArrayOfPropertyValues(array_schema) => { - if let Err(error) = + if let Err(report) = Box::pin(visitor.visit_array(array_schema, array, type_provider)).await { - status.append(error); + array_validations.push(OneOfArrayValidationStatus::Failure( + PropertyArrayValidationReport::ArrayValidation(report), + )); } else { - passed += 1; + num_passed += 1; + array_validations.push(OneOfArrayValidationStatus::Success); } } - PropertyValues::PropertyTypeObject(_) => { - status.capture(TraversalError::ExpectedValue { - actual: JsonSchemaValueType::Object, - }); + PropertyValues::DataTypeReference(_) | PropertyValues::PropertyTypeObject(_) => { + array_validations.push(OneOfArrayValidationStatus::Failure( + PropertyArrayValidationReport::WrongType(PropertyValueTypeMismatch { + actual: schema.property_value_type(), + expected: PropertyValueType::Array, + }), + )); } } } - match passed { - 0 => status.finish(), - 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()), - }); - - status.finish() - } + if num_passed == 1 { + Ok(()) + } else { + Err(OneOfArrayValidationReports { + validations: Some(array_validations), + }) } } @@ -604,51 +895,46 @@ pub async fn walk_one_of_object( schema: &[PropertyValues], object: &mut PropertyWithMetadataObject, type_provider: &P, -) -> Result<(), Report<[TraversalError]>> +) -> Result<(), OneOfObjectValidationReports> where V: EntityVisitor, P: DataTypeLookup + OntologyTypeProvider + Sync, { - let mut status = ReportSink::new(); - let mut passed: usize = 0; + let mut object_validations = Vec::with_capacity(schema.len()); + let mut num_passed: usize = 0; for schema in schema { match schema { - PropertyValues::DataTypeReference(_) => { - status.capture(TraversalError::ExpectedValue { - actual: JsonSchemaValueType::Array, - }); - } - PropertyValues::ArrayOfPropertyValues(_) => { - status.capture(TraversalError::ExpectedValue { - actual: JsonSchemaValueType::Object, - }); - } PropertyValues::PropertyTypeObject(object_schema) => { - if let Err(error) = + if let Err(report) = Box::pin(visitor.visit_object(object_schema, object, type_provider)).await { - status.append(error); + object_validations.push(OneOfObjectValidationStatus::Failure( + PropertyObjectValidationReport::ObjectValidation(report), + )); } else { - passed += 1; + num_passed += 1; + object_validations.push(OneOfObjectValidationStatus::Success); } } + PropertyValues::DataTypeReference(_) | PropertyValues::ArrayOfPropertyValues(_) => { + object_validations.push(OneOfObjectValidationStatus::Failure( + PropertyObjectValidationReport::WrongType { + data: PropertyValueTypeMismatch { + actual: schema.property_value_type(), + expected: PropertyValueType::Object, + }, + }, + )); + } } } - match passed { - 0 => status.finish(), - 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()), - }); - - status.finish() - } + if num_passed == 1 { + Ok(()) + } else { + Err(OneOfObjectValidationReports { + validations: Some(object_validations), + }) } } diff --git a/libs/@local/graph/types/rust/src/ontology/data_type/lookup.rs b/libs/@local/graph/types/rust/src/ontology/data_type/lookup.rs index c8aabab0360..7103888c13f 100644 --- a/libs/@local/graph/types/rust/src/ontology/data_type/lookup.rs +++ b/libs/@local/graph/types/rust/src/ontology/data_type/lookup.rs @@ -27,6 +27,14 @@ pub trait DataTypeLookup { data_type_uuid: DataTypeUuid, ) -> Result>; + async fn lookup_closed_data_type_by_ref( + &self, + data_type_ref: &DataTypeReference, + ) -> Result> { + self.lookup_closed_data_type_by_uuid(DataTypeUuid::from_url(&data_type_ref.url)) + .attach_printable_lazy(|| data_type_ref.url.clone()) + } + async fn lookup_closed_data_type_by_uuid( &self, data_type_uuid: DataTypeUuid, diff --git a/libs/@local/graph/types/typescript/src/validation.ts b/libs/@local/graph/types/typescript/src/validation.ts new file mode 100644 index 00000000000..8509c95cacd --- /dev/null +++ b/libs/@local/graph/types/typescript/src/validation.ts @@ -0,0 +1,202 @@ +import type { BaseUrl, VersionedUrl } from "@blockprotocol/type-system"; +import type { Subtype } from "@local/advanced-types/subtype"; +import type { + ArrayItemNumberMismatch as ArrayItemNumberMismatchGraphApi, + ArrayValidationReport as ArrayValidationReportGraphApi, + DataTypeCanonicalCalculation as DataTypeCanonicalCalculationGraphApi, + DataTypeConversionError as DataTypeConversionErrorGraphApi, + DataTypeInferenceError as DataTypeInferenceErrorGraphApi, + EntityTypesError as EntityTypesErrorGraphApi, + EntityValidationReport as EntityValidationReportGraphApi, + JsonSchemaValueTypeMismatch as JsonSchemaValueTypeMismatchGraphApi, + LinkDataStateError as LinkDataStateErrorGraphApi, + LinkedEntityError as LinkedEntityErrorGraphApi, + LinkError as LinkErrorGraphApi, + LinkTargetError as LinkTargetErrorGraphApi, + LinkValidationReport as LinkValidationReportGraphApi, + MetadataValidationReport as MetadataValidationReportGraphApi, + ObjectPropertyValidationReport as ObjectPropertyValidationReportGraphApi, + ObjectValidationReport as ObjectValidationReportGraphApi, + PropertyArrayValidationReport as PropertyArrayValidationReportGraphApi, + PropertyObjectValidationReport as PropertyObjectValidationReportGraphApi, + PropertyValidationReport as PropertyValidationReportGraphApi, + PropertyValueTypeMismatch as PropertyValueTypeMismatchGraphApi, + PropertyValueValidationReport as PropertyValueValidationReportGraphApi, + Report, + UnexpectedEntityType as UnexpectedEntityTypeGraphApi, +} from "@local/hash-graph-client"; + +export type EntityValidationReport = Omit< + EntityValidationReportGraphApi, + "properties" | "link" | "metadata" +> & { + link?: LinkValidationReport; + metadata?: MetadataValidationReport; +} & ObjectValidationReport; + +export type ObjectPropertyValidationReport = Subtype< + ObjectPropertyValidationReportGraphApi, + | { + // The property was found at the entity but is not expected in the schema + type: "unexpected"; + } + | { + // It was not possible for the graph to read the property type + type: "retrieval"; + error: Report; + } + | { + // The property was found at the entity but the type is not the expected type from Athe schema + type: "wrongType"; + data: PropertyValueTypeMismatchGraphApi; + } + | PropertyValidationReport + | ({ + // The validation of the property array failed + type: "propertyArray"; + } & ArrayValidationReport) + | { + // The property is required by the schema but is missing from the entity + type: "missing"; + } +>; + +export type JsonSchemaValueTypeMismatch = Omit< + JsonSchemaValueTypeMismatchGraphApi, + "actual" | "expected" +> & { + actual: [VersionedUrl, ...VersionedUrl[]]; + expected: [VersionedUrl, ...VersionedUrl[]]; +}; + +export type PropertyValidationReport = Subtype< + PropertyValidationReportGraphApi, + | { + // The property value validation failed + type: "value"; + /* + * Validation for each constraint in the `oneOf` field. The validation is assumed to pass if exactly one of the + * constraints passes. In this case, this field will be omitted. Whenever this field is present it can be assumed + * that the validation failed. + */ + validations?: ( + | { status: "success" } + | { status: "failure"; data: PropertyValueValidationReportGraphApi } + )[]; + // It was not possible to infer the correct data type ID for the property. + dataTypeInference?: DataTypeInferenceError[]; + // Converting the data type from `originalDataTypeId` to `dataTypeId` failed + valueConversion?: DataTypeConversionErrorGraphApi; + // It was not possible to convert the value to its canonical forms. + canonicalValue?: DataTypeCanonicalCalculationGraphApi[]; + } + | { + // The property array validation failed + type: "array"; + /* + * Validation for each constraint in the `oneOf` field. The validation is assumed to pass if exactly one of the + * constraints passes. In this case, this field will be omitted. Whenever this field is present it can be assumed + * that the validation failed. + */ + validations?: ( + | { status: "success" } + | { status: "failure"; data: PropertyArrayValidationReport } + )[]; + } + | { + // The property object validation failed + type: "object"; + /* + * Validation for each constraint in the `oneOf` field. The validation is assumed to pass if exactly one of the + * constraints passes. In this case, this field will be omitted. Whenever this field is present it can be assumed + * that the validation failed. + */ + validations?: ( + | { status: "success" } + | { status: "failure"; data: PropertyObjectValidationReport } + )[]; + } +>; + +export type PropertyArrayValidationReport = Subtype< + PropertyArrayValidationReportGraphApi, + | { + type: "wrongType"; + data: PropertyValueTypeMismatchGraphApi; + } + | { + type: "arrayValidation"; + data: ArrayValidationReport; + } +>; + +export type ArrayValidationReport = Omit< + ArrayValidationReportGraphApi, + "numItems" | "properties" +> & { + numItems?: ArrayItemNumberMismatchGraphApi; + properties?: { [arrayIndex: string]: PropertyValidationReport }; +}; + +export type PropertyObjectValidationReport = Subtype< + PropertyObjectValidationReportGraphApi, + | { + type: "wrongType"; + data: PropertyValueTypeMismatchGraphApi; + } + | { + type: "objectValidation"; + data: ObjectValidationReport; + } +>; + +export type ObjectValidationReport = Omit< + ObjectValidationReportGraphApi, + "properties" +> & { + properties?: { [property: BaseUrl]: ObjectPropertyValidationReport }; +}; + +export type DataTypeInferenceError = Subtype< + DataTypeInferenceErrorGraphApi, + | { type: "retrieval"; error: Report } + | { type: "abstract"; data: VersionedUrl } + | { type: "ambiguous"; data: VersionedUrl[] } +>; + +export type MetadataValidationReport = Omit< + MetadataValidationReportGraphApi, + "entityTypes" | "properties" +> & { + entityTypes?: EntityTypesErrorGraphApi; + properties?: never; +}; + +export type LinkValidationReport = Omit< + LinkValidationReportGraphApi, + "linkData" | "leftEntity" | "linkType" | "rightEntity" | "targetType" +> & { + linkData?: LinkDataStateErrorGraphApi; + leftEntity?: LinkedEntityErrorGraphApi; + linkType?: LinkError; + rightEntity?: LinkedEntityErrorGraphApi; + targetType?: LinkTargetError; +}; + +export type LinkError = Subtype< + LinkErrorGraphApi, + { type: "unexpectedEntityType"; data: UnexpectedEntityType } +>; + +export type LinkTargetError = Subtype< + LinkTargetErrorGraphApi, + { type: "unexpectedEntityType"; data: UnexpectedEntityType } +>; + +export type UnexpectedEntityType = Omit< + UnexpectedEntityTypeGraphApi, + "actual" | "expected" +> & { + actual: [VersionedUrl, ...VersionedUrl[]]; + expected: [VersionedUrl, ...VersionedUrl[]]; +}; diff --git a/libs/@local/graph/validation/src/entity_type.rs b/libs/@local/graph/validation/src/entity_type.rs index 6855600bfc9..bfab7b615dc 100644 --- a/libs/@local/graph/validation/src/entity_type.rs +++ b/libs/@local/graph/validation/src/entity_type.rs @@ -18,8 +18,12 @@ use hash_graph_types::{ PropertyPath, PropertyWithMetadataArray, PropertyWithMetadataObject, PropertyWithMetadataValue, ValueMetadata, visitor::{ - EntityVisitor, TraversalError, walk_array, walk_object, walk_one_of_property_value, - walk_value, + ArrayItemNumberMismatch, ArrayValidationReport, ConversionRetrieval, + DataTypeCanonicalCalculation, DataTypeConversionError, DataTypeInferenceError, + DataTypeRetrieval, EntityVisitor, InvalidCanonicalValue, + JsonSchemaValueTypeMismatch, ObjectPropertyValidationReport, + ObjectValidationReport, OneOfPropertyValidationReports, TraversalError, walk_array, + walk_object, walk_one_of_property_value, walk_value, }, }, }, @@ -389,36 +393,42 @@ impl EntityVisitor for EntityPreprocessor { schema: &[PropertyValues], property: &mut PropertyWithMetadataValue, type_provider: &P, - ) -> Result<(), Report<[TraversalError]>> + ) -> Result<(), OneOfPropertyValidationReports> where P: DataTypeLookup + Sync, { - let mut status = ReportSink::new(); + let mut property_validation = OneOfPropertyValidationReports::default(); // We try to infer the data type ID // 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 Some(data_type) = infer_status.attempt( - type_provider - .lookup_data_type_by_ref(data_type_ref) - .await - .change_context_lazy(|| TraversalError::DataTypeRetrieval { - id: data_type_ref.clone(), - }), - ) else { + let Ok(data_type) = type_provider + .lookup_data_type_by_ref(data_type_ref) + .await + .map_err(|error| { + property_validation.data_type_inference.push( + DataTypeInferenceError::Retrieval { + error: error.change_context(DataTypeRetrieval { + data_type_reference: data_type_ref.clone(), + }), + }, + ); + }) + else { continue; }; if data_type.borrow().schema.r#abstract { - infer_status.capture(TraversalError::AbstractDataType { - id: data_type_ref.url.clone(), - }); + property_validation.data_type_inference.push( + DataTypeInferenceError::Abstract { + data: data_type_ref.url.clone(), + }, + ); continue; } @@ -426,21 +436,17 @@ impl EntityVisitor for EntityPreprocessor { } } - 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 inferred_successfully { + if property_validation.data_type_inference.is_empty() { if possible_data_types.len() == 1 { property.metadata.data_type_id = possible_data_types.into_iter().next(); } else { - status.capture(TraversalError::AmbiguousDataType); + property_validation.data_type_inference.push( + DataTypeInferenceError::Ambiguous { + data: possible_data_types, + }, + ); } } } @@ -459,24 +465,35 @@ impl EntityVisitor for EntityPreprocessor { let target_data_type_ref: &DataTypeReference = target_data_type_id.into(); if source_data_type_ref != target_data_type_ref { - let conversions = type_provider + match type_provider .find_conversion(source_data_type_ref, target_data_type_ref) .await - .change_context_lazy(|| TraversalError::ConversionRetrieval { - current: source_data_type_ref.clone(), - target: target_data_type_ref.clone(), - })?; - - if let Some(mut value) = property.value.as_f64() { - for conversion in conversions.borrow() { - value = conversion.evaluate(value); + { + Err(error) => { + property_validation.value_conversion = + Some(DataTypeConversionError::Retrieval { + error: error.change_context(ConversionRetrieval { + current: source_data_type_ref.clone(), + target: target_data_type_ref.clone(), + }), + }); + } + Ok(conversions) => { + if let Some(mut value) = property.value.as_f64() { + for conversion in conversions.borrow() { + value = conversion.evaluate(value); + } + property.value = JsonValue::from(value); + } else { + property_validation.value_conversion = + Some(DataTypeConversionError::WrongType { + data: JsonSchemaValueTypeMismatch { + actual: JsonSchemaValueType::from(&property.value), + expected: JsonSchemaValueType::Number, + }, + }); + } } - property.value = JsonValue::from(value); - } else { - status.capture(TraversalError::InvalidType { - actual: JsonSchemaValueType::from(&property.value), - expected: JsonSchemaValueType::Number, - }); } } } @@ -487,16 +504,21 @@ impl EntityVisitor for EntityPreprocessor { .canonical .insert(data_type_id.base_url.clone(), property.value.clone()); - let data_type_result = type_provider + match type_provider .lookup_data_type_by_ref(<&DataTypeReference>::from(data_type_id)) .await - .change_context_lazy(|| TraversalError::DataTypeRetrieval { - id: DataTypeReference { - url: data_type_id.clone(), - }, - }); - - match data_type_result { + { + Err(error) => { + property_validation.canonical_value.push( + DataTypeCanonicalCalculation::Retrieval { + error: error.change_context(DataTypeRetrieval { + data_type_reference: DataTypeReference { + url: data_type_id.clone(), + }, + }), + }, + ); + } Ok(data_type) => { if !data_type.borrow().metadata.conversions.is_empty() { // We only support conversion of numbers for now @@ -513,26 +535,26 @@ impl EntityVisitor for EntityPreprocessor { if f64::abs(current_value - converted_value) > f64::EPSILON { - status.capture( - TraversalError::InvalidCanonicalValue { - key: target.clone(), - actual: current_value, - expected: converted_value, + property_validation.canonical_value.push( + DataTypeCanonicalCalculation::InvalidValue { + data: InvalidCanonicalValue { + key: target.clone(), + actual: current_value, + expected: converted_value, + }, }, ); } } else { - status.append( - Report::new(TraversalError::InvalidType { - actual: JsonSchemaValueType::from( - &property.value, - ), - expected: JsonSchemaValueType::Number, - }) - .attach_printable( - "Values other than numbers are not yet \ - supported for conversions", - ), + property_validation.canonical_value.push( + DataTypeCanonicalCalculation::WrongType { + data: JsonSchemaValueTypeMismatch { + actual: JsonSchemaValueType::from( + &property.value, + ), + expected: JsonSchemaValueType::Number, + }, + }, ); } } @@ -545,31 +567,34 @@ impl EntityVisitor for EntityPreprocessor { } } } else { - status.append( - Report::new(TraversalError::InvalidType { - actual: JsonSchemaValueType::from(&property.value), - expected: JsonSchemaValueType::Number, - }) - .attach_printable( - "Values other than numbers are not yet supported for \ - conversions", - ), + property_validation.canonical_value.push( + DataTypeCanonicalCalculation::WrongType { + data: JsonSchemaValueTypeMismatch { + actual: JsonSchemaValueType::from(&property.value), + expected: JsonSchemaValueType::Number, + }, + }, ); } } } - Err(error) => { - status.append(error); - } } } if let Err(error) = walk_one_of_property_value(self, schema, property, type_provider).await { - status.append(error); - } + property_validation.validations = error.validations; + }; - status.finish() + if property_validation.validations.is_some() + || property_validation.value_conversion.is_some() + || !property_validation.data_type_inference.is_empty() + || !property_validation.canonical_value.is_empty() + { + Err(property_validation) + } else { + Ok(()) + } } async fn visit_array( @@ -577,20 +602,20 @@ impl EntityVisitor for EntityPreprocessor { schema: &PropertyValueArray, array: &mut PropertyWithMetadataArray, type_provider: &P, - ) -> Result<(), Report<[TraversalError]>> + ) -> Result<(), ArrayValidationReport> where T: PropertyValueSchema + Sync, P: DataTypeLookup + OntologyTypeProvider + Sync, { - let mut status = ReportSink::new(); - if let Err(error) = walk_array(self, schema, array, type_provider).await { - status.append(error); - } + let mut array_validation = ArrayValidationReport { + properties: walk_array(self, schema, array, type_provider).await, + ..ArrayValidationReport::default() + }; if self.components.num_items { if let Some(min) = schema.min_items { if array.value.len() < min { - status.capture(TraversalError::TooFewItems { + array_validation.num_items = Some(ArrayItemNumberMismatch::TooFew { actual: array.value.len(), min, }); @@ -599,7 +624,7 @@ impl EntityVisitor for EntityPreprocessor { if let Some(max) = schema.max_items { if array.value.len() > max { - status.capture(TraversalError::TooManyItems { + array_validation.num_items = Some(ArrayItemNumberMismatch::TooMany { actual: array.value.len(), max, }); @@ -607,7 +632,11 @@ impl EntityVisitor for EntityPreprocessor { } } - status.finish() + if array_validation.is_valid() { + Ok(()) + } else { + Err(array_validation) + } } async fn visit_object( @@ -615,27 +644,29 @@ impl EntityVisitor for EntityPreprocessor { schema: &T, object: &mut PropertyWithMetadataObject, type_provider: &P, - ) -> Result<(), Report<[TraversalError]>> + ) -> Result<(), ObjectValidationReport> where T: PropertyObjectSchema> + Sync, P: DataTypeLookup + OntologyTypeProvider + Sync, { - let mut status = ReportSink::new(); - if let Err(error) = walk_object(self, schema, object, type_provider).await { - status.append(error); - } + let mut properties = walk_object(self, schema, object, type_provider).await; if self.components.required_properties { for required_property in schema.required() { if !object.value.contains_key(required_property) { - status.capture(TraversalError::MissingRequiredProperty { - key: required_property.clone(), - }); + properties.insert( + required_property.clone(), + ObjectPropertyValidationReport::Missing, + ); } } } - status.finish() + if properties.is_empty() { + Ok(()) + } else { + Err(ObjectValidationReport { properties }) + } } } diff --git a/libs/@local/graph/validation/src/lib.rs b/libs/@local/graph/validation/src/lib.rs index d5d7053b749..c2731b80733 100644 --- a/libs/@local/graph/validation/src/lib.rs +++ b/libs/@local/graph/validation/src/lib.rs @@ -332,7 +332,8 @@ mod tests { EntityPreprocessor { components } .visit_object(&closed_multi_entity_type, &mut properties, &provider) - .await?; + .await + .expect("properties should be valid"); Ok(properties) } @@ -366,7 +367,8 @@ mod tests { .expect("failed to create property with metadata"); EntityPreprocessor { components } .visit_property(&property_type, &mut property, &provider) - .await?; + .await + .expect("Property should be valid"); Ok(property) } From 657b6f6dc8418e0f5b679d9c3c3422c746d327b0 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Tue, 10 Dec 2024 17:36:18 +0100 Subject: [PATCH 2/8] Properly report errors in tests --- .../rust/src/knowledge/property/visitor.rs | 56 ------------------- libs/@local/graph/validation/src/lib.rs | 16 +++--- 2 files changed, 9 insertions(+), 63 deletions(-) diff --git a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs index 944b160a85c..e5c9f3ab173 100644 --- a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs +++ b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs @@ -25,35 +25,8 @@ use crate::{ pub enum TraversalError { #[error("the validator was unable to read the data type `{}`", id.url)] DataTypeRetrieval { id: DataTypeReference }, - #[error( - "the validator was unable to read the data type conversion from `{}` to `{}`", current.url, target.url - )] - ConversionRetrieval { - current: DataTypeReference, - target: DataTypeReference, - }, - #[error("the validator was unable to read the property type `{}`", id.url)] - PropertyTypeRetrieval { id: PropertyTypeReference }, - - #[error("the property `{key}` was specified, but not in the schema")] - UnexpectedProperty { key: BaseUrl }, - #[error( - "the value provided does not match the property type schema, expected `{expected}`, got \ - `{actual}`" - )] - InvalidType { - actual: JsonSchemaValueType, - expected: JsonSchemaValueType, - }, - #[error("a value was expected, but the property provided was of type `{actual}`")] - ExpectedValue { actual: JsonSchemaValueType }, - #[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}` \ or a child of it, got `{actual}`" @@ -66,35 +39,6 @@ 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( - "the number of items in the array is too small, expected at least {min}, but found \ - {actual}" - )] - TooFewItems { actual: usize, min: usize }, - #[error( - "the number of items in the array is too large, expected at most {max}, but found {actual}" - )] - TooManyItems { actual: usize, max: usize }, - #[error( - "The provided canonical value `{actual}` for `{key}` is different than the calculated \ - value `{expected}`" - )] - InvalidCanonicalValue { - key: BaseUrl, - expected: f64, - actual: f64, - }, } #[derive(Debug, derive_more::Display, derive_more::Error)] diff --git a/libs/@local/graph/validation/src/lib.rs b/libs/@local/graph/validation/src/lib.rs index c2731b80733..48896a34c9f 100644 --- a/libs/@local/graph/validation/src/lib.rs +++ b/libs/@local/graph/validation/src/lib.rs @@ -56,7 +56,10 @@ mod tests { Property, PropertyMetadata, PropertyObject, PropertyProvenance, PropertyWithMetadata, PropertyWithMetadataObject, PropertyWithMetadataValue, ValueMetadata, error::install_error_stack_hooks, - visitor::{EntityVisitor as _, TraversalError}, + visitor::{ + EntityVisitor as _, ObjectValidationReport, PropertyValidationReport, + TraversalError, + }, }, ontology::{ DataTypeLookup, DataTypeMetadata, DataTypeWithMetadata, OntologyEditionProvenance, @@ -271,7 +274,7 @@ mod tests { property_types: impl IntoIterator + Send, data_types: impl IntoIterator + Send, components: ValidateEntityComponents, - ) -> Result> { + ) -> Result { install_error_stack_hooks(); let mut ontology_type_resolver = OntologyTypeResolver::default(); @@ -332,8 +335,7 @@ mod tests { EntityPreprocessor { components } .visit_object(&closed_multi_entity_type, &mut properties, &provider) - .await - .expect("properties should be valid"); + .await?; Ok(properties) } @@ -345,7 +347,7 @@ mod tests { property_types: impl IntoIterator + Send, data_types: impl IntoIterator + Send, components: ValidateEntityComponents, - ) -> Result> { + ) -> Result { install_error_stack_hooks(); let property = Property::deserialize(property).expect("failed to deserialize property"); @@ -367,8 +369,8 @@ mod tests { .expect("failed to create property with metadata"); EntityPreprocessor { components } .visit_property(&property_type, &mut property, &provider) - .await - .expect("Property should be valid"); + .await?; + Ok(property) } From 78f96268d086274457df05780fa4b276dff43042 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Tue, 10 Dec 2024 17:50:05 +0100 Subject: [PATCH 3/8] Fix documentation --- .../rust/src/knowledge/property/visitor.rs | 23 +++---------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs index e5c9f3ab173..90f2fe73521 100644 --- a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs +++ b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs @@ -689,14 +689,7 @@ where /// /// # Errors /// -/// - [`ExpectedValue`] if an array or object is provided. -/// - [`DataTypeRetrieval`] if a data type could not be retrieved from the `type_provider`. -/// - [`AmbiguousProperty`] if more than one schema is passed. -/// - Any error that can be returned by the visitor methods. -/// -/// [`ExpectedValue`]: TraversalError::ExpectedValue -/// [`DataTypeRetrieval`]: TraversalError::DataTypeRetrieval -/// [`AmbiguousProperty`]: TraversalError::AmbiguousProperty +/// Returns a detailed report about the validation failures for each property value. pub async fn walk_one_of_property_value( visitor: &mut V, schema: &[PropertyValues], @@ -771,12 +764,7 @@ where /// /// # Errors /// -/// - [`ExpectedValue`] if a value or object is provided. -/// - [`AmbiguousProperty`] if more than one schema is passed. -/// - Any error that can be returned by the visitor methods. -/// -/// [`ExpectedValue`]: TraversalError::ExpectedValue -/// [`AmbiguousProperty`]: TraversalError::AmbiguousProperty +/// Returns a detailed report about the validation failures for each property value. pub async fn walk_one_of_array( visitor: &mut V, schema: &[PropertyValues], @@ -828,12 +816,7 @@ where /// /// # Errors /// -/// - [`ExpectedValue`] if a value or array is provided. -/// - [`AmbiguousProperty`] if more than one schema is passed. -/// - Any error that can be returned by the visitor methods. -/// -/// [`ExpectedValue`]: TraversalError::ExpectedValue -/// [`AmbiguousProperty`]: TraversalError::AmbiguousProperty +/// Returns a detailed report about the validation failures for each property value. pub async fn walk_one_of_object( visitor: &mut V, schema: &[PropertyValues], From 69f8be58d1d87587a18f114f5e72f1a17a900a6a Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Tue, 10 Dec 2024 20:48:03 +0100 Subject: [PATCH 4/8] Improve structs returned by value validation --- libs/@local/graph/api/openapi/openapi.json | 147 ++++++++------ libs/@local/graph/api/src/rest/entity.rs | 7 +- .../rust/src/knowledge/property/visitor.rs | 150 +++++--------- .../graph/types/typescript/src/validation.ts | 35 +++- .../graph/validation/src/entity_type.rs | 187 +++++++++--------- libs/@local/graph/validation/src/lib.rs | 47 +++-- 6 files changed, 296 insertions(+), 277 deletions(-) diff --git a/libs/@local/graph/api/openapi/openapi.json b/libs/@local/graph/api/openapi/openapi.json index 744d4befcf1..816e73f9ecb 100644 --- a/libs/@local/graph/api/openapi/openapi.json +++ b/libs/@local/graph/api/openapi/openapi.json @@ -8745,49 +8745,6 @@ } } }, - "PropertyValidationError": { - "oneOf": [ - { - "type": "object", - "required": [ - "type", - "report" - ], - "properties": { - "report": { - "$ref": "#/components/schemas/Report" - }, - "type": { - "type": "string", - "enum": [ - "value" - ] - } - } - }, - { - "type": "object", - "required": [ - "type", - "report" - ], - "properties": { - "report": { - "$ref": "#/components/schemas/Report" - }, - "type": { - "type": "string", - "enum": [ - "array" - ] - } - } - } - ], - "discriminator": { - "propertyName": "type" - } - }, "PropertyValidationReport": { "oneOf": [ { @@ -8883,24 +8840,6 @@ }, "PropertyValueValidationReport": { "oneOf": [ - { - "type": "object", - "required": [ - "error", - "type" - ], - "properties": { - "error": { - "$ref": "#/components/schemas/Report" - }, - "type": { - "type": "string", - "enum": [ - "retrieval" - ] - } - } - }, { "type": "object", "required": [ @@ -8922,12 +8861,12 @@ { "type": "object", "required": [ - "error", + "data", "type" ], "properties": { - "error": { - "$ref": "#/components/schemas/Report" + "data": { + "$ref": "#/components/schemas/ValueValidationReport" }, "type": { "type": "string", @@ -9833,6 +9772,86 @@ }, "additionalProperties": false }, + "ValueValidationError": { + "oneOf": [ + { + "type": "object", + "required": [ + "error", + "type" + ], + "properties": { + "error": { + "$ref": "#/components/schemas/Report" + }, + "type": { + "type": "string", + "enum": [ + "retrieval" + ] + } + } + }, + { + "type": "object", + "required": [ + "error", + "type" + ], + "properties": { + "error": { + "$ref": "#/components/schemas/Report" + }, + "type": { + "type": "string", + "enum": [ + "constraints" + ] + } + } + } + ], + "discriminator": { + "propertyName": "type" + } + }, + "ValueValidationReport": { + "type": "object", + "properties": { + "abstract": { + "allOf": [ + { + "$ref": "#/components/schemas/VersionedUrl" + } + ], + "nullable": true + }, + "actual": { + "allOf": [ + { + "$ref": "#/components/schemas/ValueValidationError" + } + ], + "nullable": true + }, + "desired": { + "allOf": [ + { + "$ref": "#/components/schemas/ValueValidationError" + } + ], + "nullable": true + }, + "incompatible": { + "allOf": [ + { + "$ref": "#/components/schemas/VersionedUrl" + } + ], + "nullable": true + } + } + }, "Variable": { "type": "string", "enum": [ diff --git a/libs/@local/graph/api/src/rest/entity.rs b/libs/@local/graph/api/src/rest/entity.rs index 83f222b7a65..0f99a14ea58 100644 --- a/libs/@local/graph/api/src/rest/entity.rs +++ b/libs/@local/graph/api/src/rest/entity.rs @@ -67,8 +67,8 @@ use hash_graph_types::{ OneOfObjectValidationReports, OneOfObjectValidationStatus, OneOfPropertyValidationReports, OneOfValueValidationStatus, PropertyArrayValidationReport, PropertyObjectValidationReport, - PropertyValidationError, PropertyValidationReport, PropertyValueTypeMismatch, - PropertyValueValidationReport, + PropertyValidationReport, PropertyValueTypeMismatch, PropertyValueValidationReport, + ValueValidationError, ValueValidationReport, }, }, }, @@ -188,7 +188,6 @@ use crate::rest::{ PropertyMetadataValidationReport, ObjectPropertyValidationReport, JsonSchemaValueTypeMismatch, - PropertyValidationError, ArrayValidationReport, ArrayItemNumberMismatch, PropertyValidationReport, @@ -207,6 +206,8 @@ use crate::rest::{ OneOfObjectValidationReports, OneOfObjectValidationStatus, PropertyObjectValidationReport, + ValueValidationReport, + ValueValidationError, DiffEntityParams, DiffEntityResult, diff --git a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs index 90f2fe73521..00016c5e1f0 100644 --- a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs +++ b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs @@ -1,14 +1,14 @@ use core::{borrow::Borrow as _, future::Future}; use std::collections::{HashMap, HashSet}; -use error_stack::{Report, ReportSink, ResultExt as _}; +use error_stack::Report; use futures::FutureExt as _; use serde_json::Value as JsonValue; use type_system::{ schema::{ - DataTypeReference, JsonSchemaValueType, PropertyObjectSchema, PropertyType, - PropertyTypeReference, PropertyValueArray, PropertyValueSchema, PropertyValueType, - PropertyValues, ValueOrArray, + ConstraintError, DataTypeReference, JsonSchemaValueType, PropertyObjectSchema, + PropertyType, PropertyTypeReference, PropertyValueArray, PropertyValueSchema, + PropertyValueType, PropertyValues, ValueOrArray, }, url::{BaseUrl, VersionedUrl}, }; @@ -18,29 +18,9 @@ use crate::{ PropertyWithMetadata, PropertyWithMetadataArray, PropertyWithMetadataObject, PropertyWithMetadataValue, ValueMetadata, }, - ontology::{DataTypeLookup, DataTypeWithMetadata, OntologyTypeProvider}, + ontology::{DataTypeLookup, OntologyTypeProvider}, }; -#[derive(Debug, thiserror::Error)] -pub enum TraversalError { - #[error("the validator was unable to read the data type `{}`", id.url)] - DataTypeRetrieval { id: DataTypeReference }, - #[error("The data type ID was not specified and is ambiguous.")] - AmbiguousDataType, - #[error( - "the value provided does not match the data type in the metadata, expected `{expected}` \ - or a child of it, got `{actual}`" - )] - InvalidDataType { - actual: VersionedUrl, - expected: VersionedUrl, - }, - #[error("Values cannot be assigned to an abstract data type. `{id}` is abstract.")] - AbstractDataType { id: VersionedUrl }, - #[error("the value provided does not match the constraints of the data type")] - ConstraintUnfulfilled, -} - #[derive(Debug, derive_more::Display, derive_more::Error)] #[display("Could not read the data type {}", data_type_reference.url)] #[must_use] @@ -88,15 +68,6 @@ pub struct InvalidCanonicalValue { pub actual: f64, } -#[derive(Debug, serde::Serialize)] -#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] -#[serde(tag = "type", content = "report", rename_all = "camelCase")] -#[must_use] -pub enum PropertyValidationError { - Value(Report<[TraversalError]>), - Array(Report<[TraversalError]>), -} - #[derive(Debug, serde::Serialize)] #[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] #[serde(tag = "type", rename_all = "camelCase")] @@ -137,10 +108,43 @@ pub enum DataTypeCanonicalCalculation { #[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] #[serde(tag = "type", rename_all = "camelCase")] #[must_use] -pub enum PropertyValueValidationReport { +pub enum ValueValidationError { Retrieval { error: Report }, + Constraints { error: Report<[ConstraintError]> }, +} + +#[derive(Debug, Default, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(rename_all = "camelCase")] +#[must_use] +pub struct ValueValidationReport { + #[serde(skip_serializing_if = "Option::is_none")] + pub desired: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub actual: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub r#abstract: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub incompatible: Option, +} + +impl ValueValidationReport { + #[must_use] + pub const fn is_valid(&self) -> bool { + self.desired.is_none() + && self.actual.is_none() + && self.r#abstract.is_none() + && self.incompatible.is_none() + } +} + +#[derive(Debug, serde::Serialize)] +#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] +#[serde(tag = "type", rename_all = "camelCase")] +#[must_use] +pub enum PropertyValueValidationReport { WrongType { data: PropertyValueTypeMismatch }, - ValueValidation { error: Report<[TraversalError]> }, + ValueValidation { data: ValueValidationReport }, } #[derive(Debug, serde::Serialize)] @@ -313,16 +317,13 @@ pub trait EntityVisitor: Sized + Send + Sync { /// By default, this does nothing. fn visit_value

( &mut self, - data_type: &DataTypeWithMetadata, + desired_data_type_reference: &DataTypeReference, value: &mut JsonValue, metadata: &mut ValueMetadata, type_provider: &P, - ) -> impl Future>> + Send + ) -> impl Future> + Send where - P: DataTypeLookup + Sync, - { - walk_value(self, data_type, value, metadata, type_provider) - } + P: DataTypeLookup + Sync; /// Visits a property. /// @@ -432,51 +433,6 @@ pub trait EntityVisitor: Sized + Send + Sync { } } -/// Walks through a JSON value using the provided schema. -/// -/// For all referenced data types [`EntityVisitor::visit_value`] is called. -/// -/// # Errors -/// -/// Any error that can be returned by the visitor methods. -pub async fn walk_value( - visitor: &mut V, - data_type: &DataTypeWithMetadata, - value: &mut JsonValue, - metadata: &mut ValueMetadata, - type_provider: &P, -) -> Result<(), Report<[TraversalError]>> -where - V: EntityVisitor, - P: DataTypeLookup + Sync, -{ - let mut status = ReportSink::new(); - - for parent in &data_type.schema.all_of { - match type_provider - .lookup_data_type_by_ref(parent) - .await - .change_context_lazy(|| TraversalError::DataTypeRetrieval { id: parent.clone() }) - { - Ok(parent) => { - if let Err(error) = visitor - .visit_value(parent.borrow(), value, metadata, type_provider) - .await - { - status.append(error); - } - } - Err(error) => { - status.append(error); - - continue; - } - } - } - - status.finish() -} - /// Walks through a property using the provided schema. /// /// Depending on the property, [`EntityVisitor::visit_one_of_property`], @@ -706,23 +662,9 @@ where for schema in schema { match schema { PropertyValues::DataTypeReference(data_type_ref) => { - let data_type = match type_provider.lookup_data_type_by_ref(data_type_ref).await { - Ok(data_type) => data_type, - Err(error) => { - property_validations.push(OneOfValueValidationStatus::Failure( - PropertyValueValidationReport::Retrieval { - error: error.change_context(DataTypeRetrieval { - data_type_reference: data_type_ref.clone(), - }), - }, - )); - continue; - } - }; - - if let Err(error) = visitor + if let Err(report) = visitor .visit_value( - data_type.borrow(), + data_type_ref, &mut property.value, &mut property.metadata, type_provider, @@ -730,7 +672,7 @@ where .await { property_validations.push(OneOfValueValidationStatus::Failure( - PropertyValueValidationReport::ValueValidation { error }, + PropertyValueValidationReport::ValueValidation { data: report }, )); } else { num_passed += 1; diff --git a/libs/@local/graph/types/typescript/src/validation.ts b/libs/@local/graph/types/typescript/src/validation.ts index 8509c95cacd..6cb937408bc 100644 --- a/libs/@local/graph/types/typescript/src/validation.ts +++ b/libs/@local/graph/types/typescript/src/validation.ts @@ -24,6 +24,8 @@ import type { PropertyValueValidationReport as PropertyValueValidationReportGraphApi, Report, UnexpectedEntityType as UnexpectedEntityTypeGraphApi, + ValueValidationError as ValueValidationErrorGraphApi, + ValueValidationReport as ValueValidationReportGraphApi, } from "@local/hash-graph-client"; export type EntityValidationReport = Omit< @@ -81,7 +83,7 @@ export type PropertyValidationReport = Subtype< */ validations?: ( | { status: "success" } - | { status: "failure"; data: PropertyValueValidationReportGraphApi } + | { status: "failure"; data: PropertyValueValidationReport } )[]; // It was not possible to infer the correct data type ID for the property. dataTypeInference?: DataTypeInferenceError[]; @@ -118,6 +120,37 @@ export type PropertyValidationReport = Subtype< } >; +export type PropertyValueValidationReport = Subtype< + PropertyValueValidationReportGraphApi, + | { + type: "wrongType"; + data: PropertyValueTypeMismatchGraphApi; + } + | { + type: "valueValidation"; + data: ValueValidationReport; + } +>; + +export type ValueValidationReport = Omit< + ValueValidationReportGraphApi, + "actual" | "desired" | "abstract" | "incompatible" +> & { + // The actual value has a validation error + actual?: ValueValidationError; + // The value could not be validated against the data type specified in the schema + desired?: ValueValidationError; + // The actual schema is abstract + abstract?: VersionedUrl; + // The actual schema is incompatible with the desired schema, i.e. the actual schema is not a subtype of the desired schema + incompatible?: VersionedUrl; +}; + +export type ValueValidationError = Subtype< + ValueValidationErrorGraphApi, + { type: "retrieval"; error: Report } | { type: "constraints"; error: Report } +>; + export type PropertyArrayValidationReport = Subtype< PropertyArrayValidationReportGraphApi, | { diff --git a/libs/@local/graph/validation/src/entity_type.rs b/libs/@local/graph/validation/src/entity_type.rs index bfab7b615dc..2780b5703bc 100644 --- a/libs/@local/graph/validation/src/entity_type.rs +++ b/libs/@local/graph/validation/src/entity_type.rs @@ -2,7 +2,9 @@ use alloc::collections::BTreeSet; use core::borrow::Borrow as _; use std::collections::{HashSet, hash_map::RawEntryMut}; -use error_stack::{FutureExt as _, Report, ReportSink, ResultExt as _, TryReportStreamExt as _}; +use error_stack::{ + FutureExt as _, Report, ResultExt as _, TryReportIteratorExt as _, TryReportStreamExt as _, +}; use futures::{StreamExt as _, TryStreamExt as _, stream}; use hash_graph_store::entity::{ EntityRetrieval, EntityTypeRetrieval, LinkDataStateError, LinkDataValidationReport, LinkError, @@ -22,20 +24,21 @@ use hash_graph_types::{ DataTypeCanonicalCalculation, DataTypeConversionError, DataTypeInferenceError, DataTypeRetrieval, EntityVisitor, InvalidCanonicalValue, JsonSchemaValueTypeMismatch, ObjectPropertyValidationReport, - ObjectValidationReport, OneOfPropertyValidationReports, TraversalError, walk_array, - walk_object, walk_one_of_property_value, walk_value, + ObjectValidationReport, OneOfPropertyValidationReports, ValueValidationError, + ValueValidationReport, walk_array, walk_object, walk_one_of_property_value, }, }, }, - ontology::{DataTypeLookup, DataTypeWithMetadata, OntologyTypeProvider}, + ontology::{DataTypeLookup, OntologyTypeProvider}, }; use serde_json::Value as JsonValue; use thiserror::Error; use type_system::{ schema::{ - ClosedEntityType, ClosedMultiEntityType, ConstraintValidator as _, DataTypeReference, - JsonSchemaValueType, PropertyObjectSchema, PropertyType, PropertyTypeReference, - PropertyValueArray, PropertyValueSchema, PropertyValues, ValueOrArray, + ClosedDataType, ClosedEntityType, ClosedMultiEntityType, ConstraintValidator as _, + DataTypeReference, JsonSchemaValueType, PropertyObjectSchema, PropertyType, + PropertyTypeReference, PropertyValueArray, PropertyValueSchema, PropertyValues, + ValueOrArray, }, url::VersionedUrl, }; @@ -281,110 +284,110 @@ pub struct EntityPreprocessor { pub components: ValidateEntityComponents, } -struct ValueValidator; - -impl EntityVisitor for ValueValidator { - async fn visit_value

( - &mut self, - data_type: &DataTypeWithMetadata, - value: &mut JsonValue, - metadata: &mut ValueMetadata, - _: &P, - ) -> Result<(), Report<[TraversalError]>> - where - P: DataTypeLookup + Sync, - { - let mut status = ReportSink::new(); - - status.attempt( - data_type - .schema - .constraints - .validate_value(value) - .change_context(TraversalError::ConstraintUnfulfilled), - ); - - if metadata.data_type_id.as_ref() == Some(&data_type.schema.id) - && data_type.schema.r#abstract - { - status.capture(TraversalError::AbstractDataType { - id: data_type.schema.id.clone(), - }); - } - - status.finish() - } -} - impl EntityVisitor for EntityPreprocessor { async fn visit_value

( &mut self, - data_type: &DataTypeWithMetadata, + desired_data_type_reference: &DataTypeReference, value: &mut JsonValue, metadata: &mut ValueMetadata, type_provider: &P, - ) -> Result<(), Report<[TraversalError]>> + ) -> Result<(), ValueValidationReport> where P: DataTypeLookup + Sync, { - let mut status = ReportSink::new(); + let mut validation_report = ValueValidationReport::default(); - if let Some(data_type_url) = &metadata.data_type_id { - let data_type_ref: &DataTypeReference = data_type_url.into(); - if data_type.schema.id != *data_type_url { - let is_compatible = type_provider - .is_parent_of(data_type_ref, &data_type.schema.id.base_url) - .await - .change_context_lazy(|| TraversalError::DataTypeRetrieval { - id: DataTypeReference { - url: data_type.schema.id.clone(), - }, - })?; + let actual_data_type_reference = metadata + .data_type_id + .as_ref() + .map_or(desired_data_type_reference, <&DataTypeReference>::from); - if !is_compatible { - status.capture(TraversalError::InvalidDataType { - actual: data_type_url.clone(), - expected: data_type.schema.id.clone(), - }); + match type_provider + .lookup_closed_data_type_by_ref(actual_data_type_reference) + .await + { + Ok(actual_data_type) => { + let actual_data_type: &ClosedDataType = actual_data_type.borrow(); + + if let Err(error) = actual_data_type + .borrow() + .all_of + .iter() + .map(|constraints| constraints.validate_value(value)) + .try_collect_reports::<()>() + { + validation_report.actual = Some(ValueValidationError::Constraints { error }); + }; + + if actual_data_type.r#abstract { + validation_report.r#abstract = Some(actual_data_type.id.clone()); } + } + Err(report) => { + validation_report.actual = Some(ValueValidationError::Retrieval { + error: report.change_context(DataTypeRetrieval { + data_type_reference: actual_data_type_reference.clone(), + }), + }); + } + } - let desired_data_type = type_provider - .lookup_data_type_by_ref(data_type_ref) - .await - .change_context_lazy(|| TraversalError::DataTypeRetrieval { - id: DataTypeReference { - url: data_type.schema.id.clone(), - }, - })?; + if actual_data_type_reference != desired_data_type_reference { + match type_provider + .lookup_closed_data_type_by_ref(desired_data_type_reference) + .await + { + Ok(desired_data_type) => { + let desired_data_type: &ClosedDataType = desired_data_type.borrow(); - if let Err(error) = ValueValidator - .visit_value(desired_data_type.borrow(), value, metadata, type_provider) - .await - { - status.append(error); + if let Err(error) = desired_data_type + .borrow() + .all_of + .iter() + .map(|constraints| constraints.validate_value(value)) + .try_collect_reports::<()>() + { + validation_report.desired = + Some(ValueValidationError::Constraints { error }); + }; + } + Err(report) => { + validation_report.desired = Some(ValueValidationError::Retrieval { + error: report.change_context(DataTypeRetrieval { + data_type_reference: desired_data_type_reference.clone(), + }), + }); } } - } else { - status.capture(TraversalError::AmbiguousDataType); - } - if let Err(error) = ValueValidator - .visit_value(data_type, value, metadata, type_provider) - .await - { - status.append(error); + match type_provider + .is_parent_of( + actual_data_type_reference, + &desired_data_type_reference.url.base_url, + ) + .await + { + Ok(is_compatible) => { + if !is_compatible { + validation_report.incompatible = + Some(actual_data_type_reference.url.clone()); + } + } + Err(report) => { + validation_report.desired = Some(ValueValidationError::Retrieval { + error: report.change_context(DataTypeRetrieval { + data_type_reference: desired_data_type_reference.clone(), + }), + }); + } + } } - walk_value( - &mut ValueValidator, - data_type, - value, - metadata, - type_provider, - ) - .await?; - - status.finish() + if validation_report.is_valid() { + Ok(()) + } else { + Err(validation_report) + } } #[expect(clippy::too_many_lines, reason = "Need to refactor this function")] diff --git a/libs/@local/graph/validation/src/lib.rs b/libs/@local/graph/validation/src/lib.rs index 48896a34c9f..0fbb8eb95b6 100644 --- a/libs/@local/graph/validation/src/lib.rs +++ b/libs/@local/graph/validation/src/lib.rs @@ -58,7 +58,7 @@ mod tests { error::install_error_stack_hooks, visitor::{ EntityVisitor as _, ObjectValidationReport, PropertyValidationReport, - TraversalError, + ValueValidationReport, }, }, ontology::{ @@ -221,7 +221,7 @@ mod tests { } impl DataTypeLookup for Provider { - type ClosedDataType = Arc; + type ClosedDataType = ClosedDataType; type DataTypeWithMetadata = Arc; type Error = InvalidDataType; @@ -237,9 +237,25 @@ mod tests { async fn lookup_closed_data_type_by_uuid( &self, - _: DataTypeUuid, + data_type_uuid: DataTypeUuid, ) -> Result> { - unimplemented!() + let mut ontology_type_resolver = OntologyTypeResolver::default(); + + for (data_type_id, data_type) in &self.data_types { + ontology_type_resolver + .add_unresolved_data_type(*data_type_id, Arc::new(data_type.schema.clone())); + } + + let schema_metadata = ontology_type_resolver + .resolve_data_type_metadata(data_type_uuid) + .change_context(InvalidDataType)?; + let data_type = self + .data_types + .get(&data_type_uuid) + .ok_or_else(|| Report::new(InvalidDataType))?; + + ClosedDataType::from_resolve_data(data_type.schema.clone(), &schema_metadata) + .change_context(InvalidDataType) } async fn is_parent_of( @@ -379,10 +395,10 @@ mod tests { data_type: &str, data_types: impl IntoIterator + Send, components: ValidateEntityComponents, - ) -> Result> { + ) -> Result { install_error_stack_hooks(); - let provider = Provider::new( + let mut provider = Provider::new( [], [], [], @@ -393,22 +409,27 @@ mod tests { }), ); - let data_type = generate_data_type_metadata( - serde_json::from_str(data_type) - .attach_printable_lazy(|| data_type.to_owned()) - .expect("failed to parse data type"), + let data_type = serde_json::from_str::(data_type) + .attach_printable(data_type.to_owned()) + .expect("failed to parse data type"); + let data_type_ref = DataTypeReference { + url: data_type.id.clone(), + }; + provider.data_types.insert( + DataTypeUuid::from_url(&data_type.id), + Arc::new(generate_data_type_metadata(data_type)), ); let mut metadata = ValueMetadata { - data_type_id: Some(data_type.schema.id.clone()), - original_data_type_id: Some(data_type.schema.id.clone()), + data_type_id: Some(data_type_ref.url.clone()), + original_data_type_id: Some(data_type_ref.url.clone()), provenance: PropertyProvenance::default(), confidence: None, canonical: HashMap::default(), }; EntityPreprocessor { components } - .visit_value(&data_type, &mut value, &mut metadata, &provider) + .visit_value(&data_type_ref, &mut value, &mut metadata, &provider) .await?; Ok(PropertyWithMetadataValue { value, metadata }) } From 8fa0baa5fa34141623ff8e99f5733f3ad388bf91 Mon Sep 17 00:00:00 2001 From: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com> Date: Wed, 11 Dec 2024 11:43:37 +0100 Subject: [PATCH 5/8] Rename `actual` and `desired` to `provided` and `target` --- libs/@local/graph/api/openapi/openapi.json | 10 +++++----- .../types/rust/src/knowledge/property/visitor.rs | 8 ++++---- .../graph/types/typescript/src/validation.ts | 16 +++++++++------- libs/@local/graph/validation/src/entity_type.rs | 10 +++++----- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/libs/@local/graph/api/openapi/openapi.json b/libs/@local/graph/api/openapi/openapi.json index 816e73f9ecb..c36012e9562 100644 --- a/libs/@local/graph/api/openapi/openapi.json +++ b/libs/@local/graph/api/openapi/openapi.json @@ -9826,15 +9826,15 @@ ], "nullable": true }, - "actual": { + "incompatible": { "allOf": [ { - "$ref": "#/components/schemas/ValueValidationError" + "$ref": "#/components/schemas/VersionedUrl" } ], "nullable": true }, - "desired": { + "provided": { "allOf": [ { "$ref": "#/components/schemas/ValueValidationError" @@ -9842,10 +9842,10 @@ ], "nullable": true }, - "incompatible": { + "target": { "allOf": [ { - "$ref": "#/components/schemas/VersionedUrl" + "$ref": "#/components/schemas/ValueValidationError" } ], "nullable": true diff --git a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs index 00016c5e1f0..67d949d1dab 100644 --- a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs +++ b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs @@ -119,9 +119,9 @@ pub enum ValueValidationError { #[must_use] pub struct ValueValidationReport { #[serde(skip_serializing_if = "Option::is_none")] - pub desired: Option, + pub provided: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub actual: Option, + pub target: Option, #[serde(skip_serializing_if = "Option::is_none")] pub r#abstract: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -131,8 +131,8 @@ pub struct ValueValidationReport { impl ValueValidationReport { #[must_use] pub const fn is_valid(&self) -> bool { - self.desired.is_none() - && self.actual.is_none() + self.provided.is_none() + && self.target.is_none() && self.r#abstract.is_none() && self.incompatible.is_none() } diff --git a/libs/@local/graph/types/typescript/src/validation.ts b/libs/@local/graph/types/typescript/src/validation.ts index 6cb937408bc..ab18777f2d7 100644 --- a/libs/@local/graph/types/typescript/src/validation.ts +++ b/libs/@local/graph/types/typescript/src/validation.ts @@ -48,7 +48,7 @@ export type ObjectPropertyValidationReport = Subtype< error: Report; } | { - // The property was found at the entity but the type is not the expected type from Athe schema + // The property was found at the entity but the property type is not the expected type from the schema type: "wrongType"; data: PropertyValueTypeMismatchGraphApi; } @@ -136,13 +136,15 @@ export type ValueValidationReport = Omit< ValueValidationReportGraphApi, "actual" | "desired" | "abstract" | "incompatible" > & { - // The actual value has a validation error - actual?: ValueValidationError; - // The value could not be validated against the data type specified in the schema - desired?: ValueValidationError; - // The actual schema is abstract + // The value could not be validated against the provided data type. + provided?: ValueValidationError; + // The value could not be validated against the data type specified in the schema. + // This will only be reported if the provided data type differes from the target data type. + target?: ValueValidationError; + // The provided DataType is abstract abstract?: VersionedUrl; - // The actual schema is incompatible with the desired schema, i.e. the actual schema is not a subtype of the desired schema + // The provided DataType is incompatible with the desired DataType, + // i.e. the actual DataType is not a subtype of the target DataType incompatible?: VersionedUrl; }; diff --git a/libs/@local/graph/validation/src/entity_type.rs b/libs/@local/graph/validation/src/entity_type.rs index 2780b5703bc..98d841805c5 100644 --- a/libs/@local/graph/validation/src/entity_type.rs +++ b/libs/@local/graph/validation/src/entity_type.rs @@ -316,7 +316,7 @@ impl EntityVisitor for EntityPreprocessor { .map(|constraints| constraints.validate_value(value)) .try_collect_reports::<()>() { - validation_report.actual = Some(ValueValidationError::Constraints { error }); + validation_report.provided = Some(ValueValidationError::Constraints { error }); }; if actual_data_type.r#abstract { @@ -324,7 +324,7 @@ impl EntityVisitor for EntityPreprocessor { } } Err(report) => { - validation_report.actual = Some(ValueValidationError::Retrieval { + validation_report.provided = Some(ValueValidationError::Retrieval { error: report.change_context(DataTypeRetrieval { data_type_reference: actual_data_type_reference.clone(), }), @@ -347,12 +347,12 @@ impl EntityVisitor for EntityPreprocessor { .map(|constraints| constraints.validate_value(value)) .try_collect_reports::<()>() { - validation_report.desired = + validation_report.target = Some(ValueValidationError::Constraints { error }); }; } Err(report) => { - validation_report.desired = Some(ValueValidationError::Retrieval { + validation_report.target = Some(ValueValidationError::Retrieval { error: report.change_context(DataTypeRetrieval { data_type_reference: desired_data_type_reference.clone(), }), @@ -374,7 +374,7 @@ impl EntityVisitor for EntityPreprocessor { } } Err(report) => { - validation_report.desired = Some(ValueValidationError::Retrieval { + validation_report.target = Some(ValueValidationError::Retrieval { error: report.change_context(DataTypeRetrieval { data_type_reference: desired_data_type_reference.clone(), }), From d16937832db3cca912018537662005193d7fce69 Mon Sep 17 00:00:00 2001 From: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com> Date: Wed, 11 Dec 2024 11:56:17 +0100 Subject: [PATCH 6/8] Validate `oneOf` as if it would be an `anyOf` --- libs/@local/graph/api/openapi/openapi.json | 123 +--------------- libs/@local/graph/api/src/rest/entity.rs | 14 +- .../rust/src/knowledge/property/visitor.rs | 133 +++++------------- .../graph/types/typescript/src/validation.ts | 17 +-- 4 files changed, 49 insertions(+), 238 deletions(-) diff --git a/libs/@local/graph/api/openapi/openapi.json b/libs/@local/graph/api/openapi/openapi.json index c36012e9562..bb2786f0112 100644 --- a/libs/@local/graph/api/openapi/openapi.json +++ b/libs/@local/graph/api/openapi/openapi.json @@ -7319,102 +7319,24 @@ "validations": { "type": "array", "items": { - "$ref": "#/components/schemas/OneOfArrayValidationStatus" + "$ref": "#/components/schemas/PropertyArrayValidationReport" }, "nullable": true } } }, - "OneOfArrayValidationStatus": { - "oneOf": [ - { - "type": "object", - "required": [ - "status" - ], - "properties": { - "status": { - "type": "string", - "enum": [ - "success" - ] - } - } - }, - { - "type": "object", - "required": [ - "status", - "data" - ], - "properties": { - "data": { - "$ref": "#/components/schemas/PropertyArrayValidationReport" - }, - "status": { - "type": "string", - "enum": [ - "failure" - ] - } - } - } - ], - "discriminator": { - "propertyName": "status" - } - }, "OneOfObjectValidationReports": { "type": "object", "properties": { "validations": { "type": "array", "items": { - "$ref": "#/components/schemas/OneOfObjectValidationStatus" + "$ref": "#/components/schemas/PropertyObjectValidationReport" }, "nullable": true } } }, - "OneOfObjectValidationStatus": { - "oneOf": [ - { - "type": "object", - "required": [ - "status" - ], - "properties": { - "status": { - "type": "string", - "enum": [ - "success" - ] - } - } - }, - { - "type": "object", - "required": [ - "status", - "data" - ], - "properties": { - "data": { - "$ref": "#/components/schemas/PropertyObjectValidationReport" - }, - "status": { - "type": "string", - "enum": [ - "failure" - ] - } - } - } - ], - "discriminator": { - "propertyName": "status" - } - }, "OneOfPropertyValidationReports": { "type": "object", "properties": { @@ -7433,7 +7355,7 @@ "validations": { "type": "array", "items": { - "$ref": "#/components/schemas/OneOfValueValidationStatus" + "$ref": "#/components/schemas/PropertyValueValidationReport" }, "nullable": true }, @@ -7447,45 +7369,6 @@ } } }, - "OneOfValueValidationStatus": { - "oneOf": [ - { - "type": "object", - "required": [ - "status" - ], - "properties": { - "status": { - "type": "string", - "enum": [ - "success" - ] - } - } - }, - { - "type": "object", - "required": [ - "status", - "data" - ], - "properties": { - "data": { - "$ref": "#/components/schemas/PropertyValueValidationReport" - }, - "status": { - "type": "string", - "enum": [ - "failure" - ] - } - } - } - ], - "discriminator": { - "propertyName": "status" - } - }, "OntologyEdgeKind": { "type": "string", "enum": [ diff --git a/libs/@local/graph/api/src/rest/entity.rs b/libs/@local/graph/api/src/rest/entity.rs index 0f99a14ea58..7c803a7bb2c 100644 --- a/libs/@local/graph/api/src/rest/entity.rs +++ b/libs/@local/graph/api/src/rest/entity.rs @@ -63,12 +63,11 @@ use hash_graph_types::{ ArrayItemNumberMismatch, ArrayValidationReport, DataTypeCanonicalCalculation, DataTypeConversionError, DataTypeInferenceError, InvalidCanonicalValue, JsonSchemaValueTypeMismatch, ObjectPropertyValidationReport, - ObjectValidationReport, OneOfArrayValidationReports, OneOfArrayValidationStatus, - OneOfObjectValidationReports, OneOfObjectValidationStatus, - OneOfPropertyValidationReports, OneOfValueValidationStatus, - PropertyArrayValidationReport, PropertyObjectValidationReport, - PropertyValidationReport, PropertyValueTypeMismatch, PropertyValueValidationReport, - ValueValidationError, ValueValidationReport, + ObjectValidationReport, OneOfArrayValidationReports, OneOfObjectValidationReports, + OneOfPropertyValidationReports, PropertyArrayValidationReport, + PropertyObjectValidationReport, PropertyValidationReport, + PropertyValueTypeMismatch, PropertyValueValidationReport, ValueValidationError, + ValueValidationReport, }, }, }, @@ -199,12 +198,9 @@ use crate::rest::{ DataTypeInferenceError, PropertyValueTypeMismatch, InvalidCanonicalValue, - OneOfValueValidationStatus, OneOfArrayValidationReports, - OneOfArrayValidationStatus, PropertyArrayValidationReport, OneOfObjectValidationReports, - OneOfObjectValidationStatus, PropertyObjectValidationReport, ValueValidationReport, ValueValidationError, diff --git a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs index 67d949d1dab..0bf0b1a693c 100644 --- a/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs +++ b/libs/@local/graph/types/rust/src/knowledge/property/visitor.rs @@ -147,23 +147,12 @@ pub enum PropertyValueValidationReport { ValueValidation { data: ValueValidationReport }, } -#[derive(Debug, serde::Serialize)] -#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] -#[serde(tag = "status", content = "data", rename_all = "camelCase")] -#[must_use] -// Should be an `Option` or a generic `OneOfValidationStatus` but `utoipa` is not capable of -// properly deal with that. -pub enum OneOfValueValidationStatus { - Success, - Failure(PropertyValueValidationReport), -} - #[derive(Debug, Default, serde::Serialize)] #[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] #[serde(rename_all = "camelCase")] pub struct OneOfPropertyValidationReports { #[serde(skip_serializing_if = "Option::is_none")] - pub validations: Option>, + pub validations: Option>, #[serde(skip_serializing_if = "Vec::is_empty")] pub data_type_inference: Vec, #[serde(skip_serializing_if = "Option::is_none")] @@ -181,23 +170,12 @@ pub enum PropertyArrayValidationReport { ArrayValidation(ArrayValidationReport), } -#[derive(Debug, serde::Serialize)] -#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] -#[serde(tag = "status", content = "data", rename_all = "camelCase")] -#[must_use] -// Should be an `Option` or a generic `OneOfValidationStatus` but `utoipa` is not capable of -// properly deal with that. -pub enum OneOfArrayValidationStatus { - Success, - Failure(PropertyArrayValidationReport), -} - #[derive(Debug, Default, serde::Serialize)] #[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] #[serde(rename_all = "camelCase")] pub struct OneOfArrayValidationReports { #[serde(skip_serializing_if = "Option::is_none")] - pub validations: Option>, + pub validations: Option>, } #[derive(Debug, serde::Serialize)] @@ -209,23 +187,12 @@ pub enum PropertyObjectValidationReport { ObjectValidation(ObjectValidationReport), } -#[derive(Debug, serde::Serialize)] -#[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] -#[serde(tag = "status", content = "data", rename_all = "camelCase")] -#[must_use] -// Should be an `Option` or a generic `OneOfValidationStatus` but `utoipa` is not capable of -// properly deal with that. -pub enum OneOfObjectValidationStatus { - Success, - Failure(PropertyObjectValidationReport), -} - #[derive(Debug, Default, serde::Serialize)] #[cfg_attr(feature = "utoipa", derive(utoipa::ToSchema))] #[serde(rename_all = "camelCase")] pub struct OneOfObjectValidationReports { #[serde(skip_serializing_if = "Option::is_none")] - pub validations: Option>, + pub validations: Option>, } #[derive(Debug, serde::Serialize)] @@ -656,8 +623,7 @@ where V: EntityVisitor, P: DataTypeLookup + Sync, { - let mut property_validations = Vec::with_capacity(schema.len()); - let mut num_passed: usize = 0; + let mut property_validations = Vec::new(); for schema in schema { match schema { @@ -671,35 +637,27 @@ where ) .await { - property_validations.push(OneOfValueValidationStatus::Failure( - PropertyValueValidationReport::ValueValidation { data: report }, - )); + property_validations + .push(PropertyValueValidationReport::ValueValidation { data: report }); } else { - num_passed += 1; - property_validations.push(OneOfValueValidationStatus::Success); + return Ok(()); } } PropertyValues::ArrayOfPropertyValues(_) | PropertyValues::PropertyTypeObject(_) => { - property_validations.push(OneOfValueValidationStatus::Failure( - PropertyValueValidationReport::WrongType { - data: PropertyValueTypeMismatch { - actual: schema.property_value_type(), - expected: PropertyValueType::Value, - }, + property_validations.push(PropertyValueValidationReport::WrongType { + data: PropertyValueTypeMismatch { + actual: schema.property_value_type(), + expected: PropertyValueType::Value, }, - )); + }); } } } - if num_passed == 1 { - Ok(()) - } else { - Err(OneOfPropertyValidationReports { - validations: Some(property_validations), - ..OneOfPropertyValidationReports::default() - }) - } + Err(OneOfPropertyValidationReports { + validations: Some(property_validations), + ..OneOfPropertyValidationReports::default() + }) } /// Walks through an array property using the provided schema list. @@ -717,8 +675,7 @@ where V: EntityVisitor, P: DataTypeLookup + OntologyTypeProvider + Sync, { - let mut array_validations = Vec::with_capacity(schema.len()); - let mut num_passed: usize = 0; + let mut array_validations = Vec::new(); for schema in schema { match schema { @@ -726,32 +683,25 @@ where if let Err(report) = Box::pin(visitor.visit_array(array_schema, array, type_provider)).await { - array_validations.push(OneOfArrayValidationStatus::Failure( - PropertyArrayValidationReport::ArrayValidation(report), - )); + array_validations.push(PropertyArrayValidationReport::ArrayValidation(report)); } else { - num_passed += 1; - array_validations.push(OneOfArrayValidationStatus::Success); + return Ok(()); } } PropertyValues::DataTypeReference(_) | PropertyValues::PropertyTypeObject(_) => { - array_validations.push(OneOfArrayValidationStatus::Failure( - PropertyArrayValidationReport::WrongType(PropertyValueTypeMismatch { + array_validations.push(PropertyArrayValidationReport::WrongType( + PropertyValueTypeMismatch { actual: schema.property_value_type(), expected: PropertyValueType::Array, - }), + }, )); } } } - if num_passed == 1 { - Ok(()) - } else { - Err(OneOfArrayValidationReports { - validations: Some(array_validations), - }) - } + Err(OneOfArrayValidationReports { + validations: Some(array_validations), + }) } /// Walks through an object property using the provided schema list. @@ -769,8 +719,7 @@ where V: EntityVisitor, P: DataTypeLookup + OntologyTypeProvider + Sync, { - let mut object_validations = Vec::with_capacity(schema.len()); - let mut num_passed: usize = 0; + let mut object_validations = Vec::new(); for schema in schema { match schema { @@ -778,32 +727,24 @@ where if let Err(report) = Box::pin(visitor.visit_object(object_schema, object, type_provider)).await { - object_validations.push(OneOfObjectValidationStatus::Failure( - PropertyObjectValidationReport::ObjectValidation(report), - )); + object_validations + .push(PropertyObjectValidationReport::ObjectValidation(report)); } else { - num_passed += 1; - object_validations.push(OneOfObjectValidationStatus::Success); + return Ok(()); } } PropertyValues::DataTypeReference(_) | PropertyValues::ArrayOfPropertyValues(_) => { - object_validations.push(OneOfObjectValidationStatus::Failure( - PropertyObjectValidationReport::WrongType { - data: PropertyValueTypeMismatch { - actual: schema.property_value_type(), - expected: PropertyValueType::Object, - }, + object_validations.push(PropertyObjectValidationReport::WrongType { + data: PropertyValueTypeMismatch { + actual: schema.property_value_type(), + expected: PropertyValueType::Object, }, - )); + }); } } } - if num_passed == 1 { - Ok(()) - } else { - Err(OneOfObjectValidationReports { - validations: Some(object_validations), - }) - } + Err(OneOfObjectValidationReports { + validations: Some(object_validations), + }) } diff --git a/libs/@local/graph/types/typescript/src/validation.ts b/libs/@local/graph/types/typescript/src/validation.ts index ab18777f2d7..4ce8d5961fa 100644 --- a/libs/@local/graph/types/typescript/src/validation.ts +++ b/libs/@local/graph/types/typescript/src/validation.ts @@ -81,10 +81,7 @@ export type PropertyValidationReport = Subtype< * constraints passes. In this case, this field will be omitted. Whenever this field is present it can be assumed * that the validation failed. */ - validations?: ( - | { status: "success" } - | { status: "failure"; data: PropertyValueValidationReport } - )[]; + validations?: PropertyValueValidationReport[]; // It was not possible to infer the correct data type ID for the property. dataTypeInference?: DataTypeInferenceError[]; // Converting the data type from `originalDataTypeId` to `dataTypeId` failed @@ -100,23 +97,17 @@ export type PropertyValidationReport = Subtype< * constraints passes. In this case, this field will be omitted. Whenever this field is present it can be assumed * that the validation failed. */ - validations?: ( - | { status: "success" } - | { status: "failure"; data: PropertyArrayValidationReport } - )[]; + validations?: PropertyArrayValidationReport[]; } | { // The property object validation failed type: "object"; /* - * Validation for each constraint in the `oneOf` field. The validation is assumed to pass if exactly one of the + * Validation for each constraint in the `oneOf` field. The validation is assumed to pass if at least one of the * constraints passes. In this case, this field will be omitted. Whenever this field is present it can be assumed * that the validation failed. */ - validations?: ( - | { status: "success" } - | { status: "failure"; data: PropertyObjectValidationReport } - )[]; + validations?: PropertyObjectValidationReport[]; } >; From 59f72104d3cc1f879c009218225a1cdd28adc8f6 Mon Sep 17 00:00:00 2001 From: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com> Date: Wed, 11 Dec 2024 11:57:38 +0100 Subject: [PATCH 7/8] Fix comments --- libs/@local/graph/types/typescript/src/validation.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/@local/graph/types/typescript/src/validation.ts b/libs/@local/graph/types/typescript/src/validation.ts index 4ce8d5961fa..eb8b0838117 100644 --- a/libs/@local/graph/types/typescript/src/validation.ts +++ b/libs/@local/graph/types/typescript/src/validation.ts @@ -77,7 +77,7 @@ export type PropertyValidationReport = Subtype< // The property value validation failed type: "value"; /* - * Validation for each constraint in the `oneOf` field. The validation is assumed to pass if exactly one of the + * Validation for each constraint in the `oneOf` field. The validation is assumed to pass if at least one of the * constraints passes. In this case, this field will be omitted. Whenever this field is present it can be assumed * that the validation failed. */ @@ -93,7 +93,7 @@ export type PropertyValidationReport = Subtype< // The property array validation failed type: "array"; /* - * Validation for each constraint in the `oneOf` field. The validation is assumed to pass if exactly one of the + * Validation for each constraint in the `oneOf` field. The validation is assumed to pass if at least one of the * constraints passes. In this case, this field will be omitted. Whenever this field is present it can be assumed * that the validation failed. */ From 6a3e522ccaadb43911301dc208139b1215c8ee66 Mon Sep 17 00:00:00 2001 From: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com> Date: Wed, 11 Dec 2024 12:34:57 +0100 Subject: [PATCH 8/8] Allow test to pass --- tests/graph/integration/postgres/data_type.rs | 73 +++++++++---------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/tests/graph/integration/postgres/data_type.rs b/tests/graph/integration/postgres/data_type.rs index addf76ed5ad..0fbba800d88 100644 --- a/tests/graph/integration/postgres/data_type.rs +++ b/tests/graph/integration/postgres/data_type.rs @@ -300,44 +300,43 @@ async fn inheritance() { .await .expect_err("could create ambiguous entity"); - // We specify `meter` as data type, it could be the child of `length` or `meter` but only one is - // allowed. This is expected to be lifted in the future. - _ = api - .create_entity(api.account_id, CreateEntityParams { - owned_by_id: OwnedById::new(api.account_id.into_uuid()), - entity_uuid: None, - decision_time: None, - entity_type_ids: HashSet::from([VersionedUrl::from_str( - "http://localhost:3000/@alice/types/entity-type/line/v/1", - ) - .expect("couldn't construct Base URL")]), - properties: PropertyWithMetadataObject { - value: HashMap::from([( - BaseUrl::new( - "http://localhost:3000/@alice/types/property-type/length/".to_owned(), - ) + // We specify `meter` as data type, it could be the child of `length` or `meter`. We treat + // `oneOf` in property types as an `anyOf`, so this is allowed. + // TODO: Change the type system to use `anyOf` instead + // see https://linear.app/hash/issue/H-3263/fix-type-system-to-use-anyof-instead-of-oneof + api.create_entity(api.account_id, CreateEntityParams { + owned_by_id: OwnedById::new(api.account_id.into_uuid()), + entity_uuid: None, + decision_time: None, + entity_type_ids: HashSet::from([VersionedUrl::from_str( + "http://localhost:3000/@alice/types/entity-type/line/v/1", + ) + .expect("couldn't construct Base URL")]), + properties: PropertyWithMetadataObject { + value: HashMap::from([( + BaseUrl::new("http://localhost:3000/@alice/types/property-type/length/".to_owned()) .expect("couldn't construct Base URL"), - PropertyWithMetadata::Value(PropertyWithMetadataValue { - value: json!(10), - metadata: ValueMetadata { - provenance: PropertyProvenance::default(), - confidence: None, - data_type_id: Some(meter_dt_v1.id.clone()), - original_data_type_id: None, - canonical: HashMap::default(), - }, - }), - )]), - metadata: ObjectMetadata::default(), - }, - confidence: None, - link_data: None, - draft: false, - relationships: [], - provenance: ProvidedEntityEditionProvenance::default(), - }) - .await - .expect_err("could create ambiguous entity"); + PropertyWithMetadata::Value(PropertyWithMetadataValue { + value: json!(10), + metadata: ValueMetadata { + provenance: PropertyProvenance::default(), + confidence: None, + data_type_id: Some(meter_dt_v1.id.clone()), + original_data_type_id: None, + canonical: HashMap::default(), + }, + }), + )]), + metadata: ObjectMetadata::default(), + }, + confidence: None, + link_data: None, + draft: false, + relationships: [], + provenance: ProvidedEntityEditionProvenance::default(), + }) + .await + .expect("should be able to create entity"); // We specify `centimeter` as data type, so the validation for `length` passes, the validation // for `meter` fails, and the entity is created.