From 6aaf3b00a6ee951a43fecbfb933ff64a456ea13c Mon Sep 17 00:00:00 2001 From: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com> Date: Fri, 29 Nov 2024 17:52:13 +0100 Subject: [PATCH] H-3705, H-3706: Fix codegen for data type constraints and prohibit multiple formats in constraints (#5760) --- .../rust/src/schema/data_type/closed.rs | 2 + .../src/schema/data_type/constraint/array.rs | 10 ++-- .../schema/data_type/constraint/boolean.rs | 2 - .../src/schema/data_type/constraint/mod.rs | 30 +++++------ .../src/schema/data_type/constraint/null.rs | 2 - .../src/schema/data_type/constraint/object.rs | 8 +-- .../src/schema/data_type/constraint/string.rs | 54 +++++++++++++------ .../rust/src/schema/data_type/mod.rs | 21 +++----- 8 files changed, 65 insertions(+), 64 deletions(-) diff --git a/libs/@blockprotocol/type-system/rust/src/schema/data_type/closed.rs b/libs/@blockprotocol/type-system/rust/src/schema/data_type/closed.rs index 698b8f60a21..970a1444b03 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/data_type/closed.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/data_type/closed.rs @@ -72,6 +72,8 @@ pub enum ResolveClosedDataTypeError { ConflictingConstEnumValue(JsonValue, Vec), #[error("The constraint is unsatisfiable: {}", json!(.0))] UnsatisfiableConstraint(ValueConstraints), + #[error("The constraints are incompatible: {} <=> {}", json!(.0), json!(.1))] + IncompatibleConstraints(ValueConstraints, ValueConstraints), #[error("The combined constraints results in an empty `anyOf`")] EmptyAnyOf, } diff --git a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/array.rs b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/array.rs index a26f0755281..5a3260e420e 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/array.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/array.rs @@ -44,7 +44,7 @@ pub enum ArrayTypeTag { #[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))] #[serde(tag = "type", rename_all = "camelCase")] pub enum ArrayItemConstraints { - Boolean(BooleanSchema), + Boolean, Number(NumberSchema), String(StringSchema), } @@ -55,9 +55,7 @@ impl Constraint for ArrayItemConstraints { other: Self, ) -> Result<(Self, Option), Report> { match (self, other) { - (Self::Boolean(lhs), Self::Boolean(rhs)) => lhs - .intersection(rhs) - .map(|(lhs, rhs)| (Self::Boolean(lhs), rhs.map(Self::Boolean))), + (Self::Boolean, Self::Boolean) => Ok((Self::Boolean, None)), (Self::Number(lhs), Self::Number(rhs)) => lhs .intersection(rhs) .map(|(lhs, rhs)| (Self::Number(lhs), rhs.map(Self::Number))), @@ -74,7 +72,7 @@ impl ConstraintValidator for ArrayItemConstraints { fn is_valid(&self, value: &JsonValue) -> bool { match self { - Self::Boolean(schema) => schema.is_valid(value), + Self::Boolean => BooleanSchema.is_valid(value), Self::Number(schema) => schema.is_valid(value), Self::String(schema) => schema.is_valid(value), } @@ -82,7 +80,7 @@ impl ConstraintValidator for ArrayItemConstraints { fn validate_value(&self, value: &JsonValue) -> Result<(), Report> { match self { - Self::Boolean(schema) => schema.validate_value(value), + Self::Boolean => BooleanSchema.validate_value(value), Self::Number(schema) => schema.validate_value(value), Self::String(schema) => schema.validate_value(value), } diff --git a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/boolean.rs b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/boolean.rs index f2cc0c4c823..cc6e96d42bd 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/boolean.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/boolean.rs @@ -8,14 +8,12 @@ use crate::schema::{ }; #[derive(Debug, Clone, Serialize, Deserialize)] -#[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))] #[serde(rename_all = "camelCase")] pub enum BooleanTypeTag { Boolean, } #[derive(Debug, Clone, Serialize, Deserialize)] -#[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct BooleanSchema; diff --git a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/mod.rs b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/mod.rs index 8129ff53e02..1225607a46f 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/mod.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/mod.rs @@ -182,12 +182,12 @@ impl ConstraintValidator for ValueConstraints { #[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))] #[serde(tag = "type", rename_all = "camelCase")] pub enum SingleValueConstraints { - Null(NullSchema), - Boolean(BooleanSchema), + Null, + Boolean, Number(NumberSchema), String(StringSchema), Array(ArraySchema), - Object(ObjectSchema), + Object, } impl Constraint for SingleValueConstraints { @@ -196,12 +196,8 @@ impl Constraint for SingleValueConstraints { other: Self, ) -> Result<(Self, Option), Report> { match (self, other) { - (Self::Null(lhs), Self::Null(rhs)) => lhs - .intersection(rhs) - .map(|(lhs, rhs)| (Self::Null(lhs), rhs.map(Self::Null))), - (Self::Boolean(lhs), Self::Boolean(rhs)) => lhs - .intersection(rhs) - .map(|(lhs, rhs)| (Self::Boolean(lhs), rhs.map(Self::Boolean))), + (Self::Null, Self::Null) => Ok((Self::Null, None)), + (Self::Boolean, Self::Boolean) => Ok((Self::Boolean, None)), (Self::Number(lhs), Self::Number(rhs)) => lhs .intersection(rhs) .map(|(lhs, rhs)| (Self::Number(lhs), rhs.map(Self::Number))), @@ -211,9 +207,7 @@ impl Constraint for SingleValueConstraints { (Self::Array(lhs), Self::Array(rhs)) => lhs .intersection(rhs) .map(|(lhs, rhs)| (Self::Array(lhs), rhs.map(Self::Array))), - (Self::Object(lhs), Self::Object(rhs)) => lhs - .intersection(rhs) - .map(|(lhs, rhs)| (Self::Object(lhs), rhs.map(Self::Object))), + (Self::Object, Self::Object) => Ok((Self::Object, None)), _ => bail!(ResolveClosedDataTypeError::IntersectedDifferentTypes), } } @@ -224,23 +218,23 @@ impl ConstraintValidator for SingleValueConstraints { fn is_valid(&self, value: &JsonValue) -> bool { match self { - Self::Null(schema) => schema.is_valid(value), - Self::Boolean(schema) => schema.is_valid(value), + Self::Null => NullSchema.is_valid(value), + Self::Boolean => BooleanSchema.is_valid(value), Self::Number(schema) => schema.is_valid(value), Self::String(schema) => schema.is_valid(value), Self::Array(schema) => schema.is_valid(value), - Self::Object(schema) => schema.is_valid(value), + Self::Object => ObjectSchema::Constrained(ObjectConstraints).is_valid(value), } } fn validate_value(&self, value: &JsonValue) -> Result<(), Report> { match self { - Self::Null(schema) => schema.validate_value(value), - Self::Boolean(schema) => schema.validate_value(value), + Self::Null => NullSchema.validate_value(value), + Self::Boolean => BooleanSchema.validate_value(value), Self::Number(schema) => schema.validate_value(value), Self::String(schema) => schema.validate_value(value), Self::Array(schema) => schema.validate_value(value), - Self::Object(schema) => schema.validate_value(value), + Self::Object => ObjectSchema::Constrained(ObjectConstraints).validate_value(value), } } } diff --git a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/null.rs b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/null.rs index fdcb625b23b..fcb755d034a 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/null.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/null.rs @@ -8,14 +8,12 @@ use crate::schema::{ }; #[derive(Debug, Clone, Serialize, Deserialize)] -#[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))] #[serde(rename_all = "camelCase")] pub enum NullTypeTag { Null, } #[derive(Debug, Clone, Serialize, Deserialize)] -#[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct NullSchema; diff --git a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/object.rs b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/object.rs index ef8f940c5b4..0360c419dcd 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/object.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/object.rs @@ -21,7 +21,6 @@ pub enum ObjectTypeTag { } #[derive(Debug, Clone, Serialize, Deserialize)] -#[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))] #[serde(untagged, rename_all = "camelCase", deny_unknown_fields)] pub enum ObjectSchema { Constrained(ObjectConstraints), @@ -87,13 +86,8 @@ impl ConstraintValidator for ObjectSchema { } #[derive(Debug, Clone, Serialize, Deserialize)] -#[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))] #[serde(rename_all = "camelCase", deny_unknown_fields)] -#[expect( - clippy::empty_structs_with_brackets, - reason = "This struct is a placeholder for future functionality." -)] -pub struct ObjectConstraints {} +pub struct ObjectConstraints; impl Constraint for ObjectConstraints { fn intersection( diff --git a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/string.rs b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/string.rs index 8ea26718531..9c82a9b9c5b 100644 --- a/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/string.rs +++ b/libs/@blockprotocol/type-system/rust/src/schema/data_type/constraint/string.rs @@ -451,12 +451,28 @@ impl Constraint for StringConstraints { Some((lhs, rhs)) => Some(lhs.min(rhs)), None => self.max_length.or(other.max_length), }; - match self.format.zip(other.format) { - Some((lhs, rhs)) if lhs == rhs => {} - Some((_, rhs)) => { - remainder.get_or_insert_default().format = Some(rhs); + self.format = match self.format.zip(other.format) { + Some((lhs, rhs)) => { + ensure!( + lhs == rhs, + ResolveClosedDataTypeError::IncompatibleConstraints( + ValueConstraints::Typed(SingleValueConstraints::String( + StringSchema::Constrained(Self { + format: Some(lhs), + ..Self::default() + }), + )), + ValueConstraints::Typed(SingleValueConstraints::String( + StringSchema::Constrained(Self { + format: Some(rhs), + ..Self::default() + }), + )) + ) + ); + Some(lhs) } - None => self.format = self.format.or(other.format), + None => self.format.or(other.format), }; match self.pattern.as_ref().zip(other.pattern.as_ref()) { Some((lhs, rhs)) if lhs.as_str() == rhs.as_str() => {} @@ -854,17 +870,7 @@ mod tests { #[test] fn intersect_format_both_different() { - check_schema_intersection( - [ - json!({ - "type": "string", - "format": "uri", - }), - json!({ - "type": "string", - "format": "hostname", - }), - ], + check_schema_intersection_error( [ json!({ "type": "string", @@ -875,6 +881,22 @@ mod tests { "format": "hostname", }), ], + [ResolveClosedDataTypeError::IncompatibleConstraints( + from_value(json!( + { + "type": "string", + "format": "uri", + } + )) + .expect("schema should be a valid string schema"), + from_value(json!( + { + "type": "string", + "format": "hostname", + } + )) + .expect("schema should be a valid string schema"), + )], ); } 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 20a79bf6804..958297f5785 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 @@ -119,12 +119,12 @@ mod raw { use super::{DataTypeSchemaTag, DataTypeTag, ValueSchemaMetadata}; use crate::{ schema::{ - ArrayTypeTag, BooleanTypeTag, DataTypeReference, NullSchema, NullTypeTag, - NumberTypeTag, ObjectTypeTag, StringTypeTag, + ArrayTypeTag, BooleanTypeTag, DataTypeReference, NullTypeTag, NumberTypeTag, + ObjectTypeTag, StringTypeTag, data_type::constraint::{ - AnyOfConstraints, ArrayConstraints, ArraySchema, BooleanSchema, NumberConstraints, - NumberSchema, ObjectConstraints, ObjectSchema, SingleValueConstraints, - StringConstraints, StringSchema, TupleConstraints, ValueConstraints, + AnyOfConstraints, ArrayConstraints, ArraySchema, NumberConstraints, NumberSchema, + SingleValueConstraints, StringConstraints, StringSchema, TupleConstraints, + ValueConstraints, }, }, url::VersionedUrl, @@ -222,8 +222,6 @@ mod raw { base: DataTypeBase, #[serde(flatten)] metadata: ValueSchemaMetadata, - #[serde(flatten)] - constraints: ObjectConstraints, }, Array { r#type: ArrayTypeTag, @@ -292,7 +290,7 @@ mod raw { } => ( base, metadata, - ValueConstraints::Typed(SingleValueConstraints::Null(NullSchema)), + ValueConstraints::Typed(SingleValueConstraints::Null), ), DataType::Boolean { r#type: _, @@ -301,7 +299,7 @@ mod raw { } => ( base, metadata, - ValueConstraints::Typed(SingleValueConstraints::Boolean(BooleanSchema)), + ValueConstraints::Typed(SingleValueConstraints::Boolean), ), DataType::Number { r#type: _, @@ -379,13 +377,10 @@ mod raw { r#type: _, base, metadata, - constraints, } => ( base, metadata, - ValueConstraints::Typed(SingleValueConstraints::Object( - ObjectSchema::Constrained(constraints), - )), + ValueConstraints::Typed(SingleValueConstraints::Object), ), DataType::Array { r#type: _,