Skip to content

Commit

Permalink
H-3705, H-3706: Fix codegen for data type constraints and prohibit mu…
Browse files Browse the repository at this point in the history
…ltiple formats in constraints (#5760)
  • Loading branch information
TimDiekmann authored Nov 29, 2024
1 parent 28f36a6 commit 6aaf3b0
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ pub enum ResolveClosedDataTypeError {
ConflictingConstEnumValue(JsonValue, Vec<JsonValue>),
#[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,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand All @@ -55,9 +55,7 @@ impl Constraint for ArrayItemConstraints {
other: Self,
) -> Result<(Self, Option<Self>), Report<ResolveClosedDataTypeError>> {
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))),
Expand All @@ -74,15 +72,15 @@ impl ConstraintValidator<JsonValue> 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),
}
}

fn validate_value(&self, value: &JsonValue) -> Result<(), Report<ConstraintError>> {
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),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,12 @@ impl ConstraintValidator<JsonValue> 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 {
Expand All @@ -196,12 +196,8 @@ impl Constraint for SingleValueConstraints {
other: Self,
) -> Result<(Self, Option<Self>), Report<ResolveClosedDataTypeError>> {
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))),
Expand All @@ -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),
}
}
Expand All @@ -224,23 +218,23 @@ impl ConstraintValidator<JsonValue> 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<ConstraintError>> {
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),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -87,13 +86,8 @@ impl ConstraintValidator<JsonObject> 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() => {}
Expand Down Expand Up @@ -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",
Expand All @@ -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"),
)],
);
}

Expand Down
21 changes: 8 additions & 13 deletions libs/@blockprotocol/type-system/rust/src/schema/data_type/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -222,8 +222,6 @@ mod raw {
base: DataTypeBase,
#[serde(flatten)]
metadata: ValueSchemaMetadata,
#[serde(flatten)]
constraints: ObjectConstraints,
},
Array {
r#type: ArrayTypeTag,
Expand Down Expand Up @@ -292,7 +290,7 @@ mod raw {
} => (
base,
metadata,
ValueConstraints::Typed(SingleValueConstraints::Null(NullSchema)),
ValueConstraints::Typed(SingleValueConstraints::Null),
),
DataType::Boolean {
r#type: _,
Expand All @@ -301,7 +299,7 @@ mod raw {
} => (
base,
metadata,
ValueConstraints::Typed(SingleValueConstraints::Boolean(BooleanSchema)),
ValueConstraints::Typed(SingleValueConstraints::Boolean),
),
DataType::Number {
r#type: _,
Expand Down Expand Up @@ -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: _,
Expand Down

0 comments on commit 6aaf3b0

Please sign in to comment.