Skip to content

Commit

Permalink
doc: Fix intersection of shape defaults
Browse files Browse the repository at this point in the history
We weren't correctly narrowing the default of intersected shapes. For example, if we intersected one shape like
```json
{
    "type": ["string", "null"],
    "default": "null"
}
```

with another shape like
```json
{
    "type": "string"
}
```

We'd end up with a shape of
```json
{
    "type": "string",
    "default": null
}
```

Which isn't correct, as when we try to actually emit a document for this shape that's missing the field
in question and provide its default value of null, that will subsequently fail to validate against the schema.

Fixes #1944
  • Loading branch information
jshearer committed Feb 20, 2025
1 parent de9662f commit 819fb98
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 1 deletion.
58 changes: 58 additions & 0 deletions crates/dekaf/tests/dekaf_schema_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,64 @@ async fn roundtrip(
.collect::<Result<Vec<_>, _>>()
}

#[tokio::test]
async fn test_allof_with_null_default() -> anyhow::Result<()> {
for output in roundtrip(
connector::DekafConfig {
deletions: connector::DeletionMode::Kafka,
token: "1234".to_string(),
strict_topic_names: false,
},
json!({
"schema": {
"allOf": [
{
"properties": {
"id": {
"title": "Id",
"type": "integer"
},
"conflicts": {
"type": ["integer", "null"],
"default": null,
"title": "Updatedbyuserid"
}
},
"required": ["id"],
"type": "object"
},
{
"properties": {
"id": {
"title": "Id",
"type": "integer"
},
"conflicts": {
"type": "integer"
}
},
"required": ["id"],
"type": "object"
}
]
},
"key": ["/id"]
}),
json!({
"recommended": true
}),
vec![json!({
"id": 671963468
})],
)
.await?
{
insta::assert_debug_snapshot!(output?);
}

Ok(())
}

#[tokio::test]
async fn test_field_selection_specific_fields() -> anyhow::Result<()> {
for output in roundtrip(
Expand Down
57 changes: 56 additions & 1 deletion crates/doc/src/shape/intersect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Intersected Shapes impose *all* of their constraints,
// like a JSON Schema `allOf` keyword.
use super::*;
use crate::FailedValidation;
use itertools::{EitherOrBoth, Itertools};

impl Reduction {
Expand Down Expand Up @@ -235,7 +236,7 @@ impl Shape {
let description = lhs.description.or(rhs.description);
let reduction = lhs.reduction.intersect(rhs.reduction);
let provenance = lhs.provenance.intersect(rhs.provenance);
let default = lhs.default.or(rhs.default);
let default = intersect_default(type_, lhs.default, rhs.default);
let secret = lhs.secret.or(rhs.secret);

let mut annotations = rhs.annotations;
Expand Down Expand Up @@ -310,6 +311,32 @@ pub fn intersect_enum(
}
}

pub fn intersect_default(
type_: types::Set,
lhs: Option<Box<(Value, Option<FailedValidation>)>>,
rhs: Option<Box<(Value, Option<FailedValidation>)>>,
) -> Option<Box<(Value, Option<FailedValidation>)>> {
match (lhs, rhs) {
(None, None) => None,
(Some(l), None) | (None, Some(l)) => {
if type_.overlaps(types::Set::for_value(&l.as_ref().0)) {
Some(l)
} else {
None
}
}
(Some(l), Some(r)) => {
if type_.overlaps(types::Set::for_value(&l.as_ref().0)) {
Some(l)
} else if type_.overlaps(types::Set::for_value(&r.as_ref().0)) {
Some(r)
} else {
None
}
}
}
}

fn intersect_additional(lhs: Option<Box<Shape>>, rhs: Option<Box<Shape>>) -> Option<Box<Shape>> {
match (lhs, rhs) {
(Some(lhs), Some(rhs)) => Some(Box::new(Shape::intersect(
Expand Down Expand Up @@ -392,4 +419,32 @@ mod test {
actual
);
}

#[test]
fn test_default_intersection() {
let shape_with_reasonable_default = shape_from(
r#"
allOf:
- type: ["string", "null"]
default: "hello"
- type: "string"
"#,
);

assert_eq!(
shape_with_reasonable_default.default.unwrap().as_ref().0,
serde_json::json!("hello")
);

let shape = shape_from(
r#"
allOf:
- type: ["string", "null"]
default: null
- type: "string"
"#,
);

assert_eq!(shape.default, None);
}
}

0 comments on commit 819fb98

Please sign in to comment.