diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 8f2ee3b2e4d6f..be592b9ac0b25 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -723,6 +723,7 @@ MAINPID majorly MAKECMDGOALS Makefiles +markability markdownify markdownlintrc marketo diff --git a/lib/vector-config/src/schema/helpers.rs b/lib/vector-config/src/schema/helpers.rs index c1baffd0731a3..61f8cccba65ea 100644 --- a/lib/vector-config/src/schema/helpers.rs +++ b/lib/vector-config/src/schema/helpers.rs @@ -12,7 +12,7 @@ use crate::{ num::ConfigurableNumber, Configurable, ConfigurableRef, GenerateError, Metadata, ToValue, }; -use super::visitors::{DisallowedUnevaluatedPropertiesVisitor, InlineSingleUseReferencesVisitor}; +use super::visitors::{DisallowUnevaluatedPropertiesVisitor, InlineSingleUseReferencesVisitor}; /// Applies metadata to the given schema. /// @@ -482,7 +482,7 @@ pub fn generate_internal_tagged_variant_schema( pub fn default_schema_settings() -> SchemaSettings { SchemaSettings::new() .with_visitor(InlineSingleUseReferencesVisitor::from_settings) - .with_visitor(DisallowedUnevaluatedPropertiesVisitor::from_settings) + .with_visitor(DisallowUnevaluatedPropertiesVisitor::from_settings) } pub fn generate_root_schema() -> Result diff --git a/lib/vector-config/src/schema/visitors/mod.rs b/lib/vector-config/src/schema/visitors/mod.rs index 59802e0ebe10b..3d919f2dea740 100644 --- a/lib/vector-config/src/schema/visitors/mod.rs +++ b/lib/vector-config/src/schema/visitors/mod.rs @@ -7,4 +7,4 @@ mod unevaluated; pub(self) mod test; pub use self::inline_single::InlineSingleUseReferencesVisitor; -pub use self::unevaluated::DisallowedUnevaluatedPropertiesVisitor; +pub use self::unevaluated::DisallowUnevaluatedPropertiesVisitor; diff --git a/lib/vector-config/src/schema/visitors/unevaluated.rs b/lib/vector-config/src/schema/visitors/unevaluated.rs index cfab8822df5be..1619465cde24a 100644 --- a/lib/vector-config/src/schema/visitors/unevaluated.rs +++ b/lib/vector-config/src/schema/visitors/unevaluated.rs @@ -27,12 +27,12 @@ use super::scoped_visit::{ /// with advanced subschema validation, such as `oneOf` or `allOf`, as `unevaluatedProperties` /// cannot simply be applied to any and all schemas indiscriminately. #[derive(Debug, Default)] -pub struct DisallowedUnevaluatedPropertiesVisitor { +pub struct DisallowUnevaluatedPropertiesVisitor { scope_stack: SchemaScopeStack, eligible_to_flatten: HashMap>, } -impl DisallowedUnevaluatedPropertiesVisitor { +impl DisallowUnevaluatedPropertiesVisitor { pub fn from_settings(_: &SchemaSettings) -> Self { Self { scope_stack: SchemaScopeStack::default(), @@ -41,7 +41,7 @@ impl DisallowedUnevaluatedPropertiesVisitor { } } -impl Visitor for DisallowedUnevaluatedPropertiesVisitor { +impl Visitor for DisallowUnevaluatedPropertiesVisitor { fn visit_root_schema(&mut self, root: &mut RootSchema) { let eligible_to_flatten = build_closed_schema_flatten_eligibility_mappings(root); @@ -132,7 +132,7 @@ impl Visitor for DisallowedUnevaluatedPropertiesVisitor { } } -impl ScopedVisitor for DisallowedUnevaluatedPropertiesVisitor { +impl ScopedVisitor for DisallowUnevaluatedPropertiesVisitor { fn push_schema_scope>(&mut self, scope: S) { self.scope_stack.push(scope.into()); } @@ -237,12 +237,22 @@ fn build_closed_schema_flatten_eligibility_mappings( Schema::Object(schema) => schema, }; + debug!( + "Evaluating schema definition '{}' for markability.", + definition_name + ); + // If a schema itself would not be considered markable, then we don't need to consider the // eligibility between parent/child since there's nothing to drive the "now unmark the child // schemas" logic. if !is_markable_schema(&root_schema.definitions, parent_schema) { debug!("Schema definition '{}' not markable.", definition_name); continue; + } else { + debug!( + "Schema definition '{}' markable. Collecting referents.", + definition_name + ); } // Collect all referents for this definition, which includes both property-based referents @@ -251,6 +261,13 @@ fn build_closed_schema_flatten_eligibility_mappings( let mut referents = HashSet::new(); get_referents(parent_schema, &mut referents); + debug!( + "Collected {} referents for '{}': {:?}", + referents.len(), + definition_name, + referents + ); + // Store the parent/child mapping. parent_to_child.insert(SchemaReference::from(definition_name), referents); } @@ -328,12 +345,14 @@ fn is_markable_schema(definitions: &Map, schema: &SchemaObject) // subschemas: { "type": "null" } and { "$ref": "#/definitions/T" }. If the schema for `T` is, // say, just a scalar schema, instead of an object schema... then it wouldn't be marked, and in // turn, we wouldn't need to mark the schema for `Option`: there's no properties at all. - // - // We'll follow schema references in subschemas up to one level deep trying to figure this out. if let Some(subschema) = schema.subschemas.as_ref() { let subschemas = get_object_subschemas_from_parent(subschema).collect::>(); - let has_object_subschema = subschemas.iter().any(|schema| is_object_schema(schema)); + debug!("{} subschemas detected.", subschemas.len()); + + let has_object_subschema = subschemas + .iter() + .any(|schema| is_markable_schema(definitions, schema)); let has_referenced_object_subschema = subschemas .iter() .map(|subschema| { @@ -342,13 +361,33 @@ fn is_markable_schema(definitions: &Map, schema: &SchemaObject) .as_ref() .and_then(|reference| { let reference = get_cleaned_schema_reference(reference); - definitions.get(reference) + definitions.get_full(reference) + }) + .and_then(|(_, name, schema)| schema.as_object().map(|schema| (name, schema))) + .map_or(false, |(name, schema)| { + debug!( + "Following schema reference '{}' for subschema markability.", + name + ); + is_markable_schema(definitions, schema) }) - .and_then(Schema::as_object) - .map_or(false, is_object_schema) }) .any(identity); + debug!( + "Schema {} object subschema(s) and {} referenced subschemas.", + if has_object_subschema { + "has" + } else { + "does not have" + }, + if has_referenced_object_subschema { + "has" + } else { + "does not have" + }, + ); + if has_object_subschema || has_referenced_object_subschema { return true; } @@ -504,7 +543,7 @@ mod tests { use crate::schema::visitors::test::{as_schema, assert_schemas_eq}; - use super::DisallowedUnevaluatedPropertiesVisitor; + use super::DisallowUnevaluatedPropertiesVisitor; #[test] fn basic_object_schema() { @@ -515,7 +554,7 @@ mod tests { } })); - let mut visitor = DisallowedUnevaluatedPropertiesVisitor::default(); + let mut visitor = DisallowUnevaluatedPropertiesVisitor::default(); visitor.visit_root_schema(&mut actual_schema); let expected_schema = as_schema(json!({ @@ -543,7 +582,7 @@ mod tests { } })); - let mut visitor = DisallowedUnevaluatedPropertiesVisitor::default(); + let mut visitor = DisallowUnevaluatedPropertiesVisitor::default(); visitor.visit_root_schema(&mut actual_schema); let expected_schema = as_schema(json!({ @@ -580,7 +619,7 @@ mod tests { }] })); - let mut visitor = DisallowedUnevaluatedPropertiesVisitor::default(); + let mut visitor = DisallowUnevaluatedPropertiesVisitor::default(); visitor.visit_root_schema(&mut actual_schema); let expected_schema = as_schema(json!({ @@ -621,7 +660,7 @@ mod tests { }] })); - let mut visitor = DisallowedUnevaluatedPropertiesVisitor::default(); + let mut visitor = DisallowUnevaluatedPropertiesVisitor::default(); visitor.visit_root_schema(&mut actual_schema); let expected_schema = as_schema(json!({ @@ -662,7 +701,7 @@ mod tests { }] })); - let mut visitor = DisallowedUnevaluatedPropertiesVisitor::default(); + let mut visitor = DisallowUnevaluatedPropertiesVisitor::default(); visitor.visit_root_schema(&mut actual_schema); let expected_schema = as_schema(json!({ @@ -696,7 +735,7 @@ mod tests { })); let expected_schema = actual_schema.clone(); - let mut visitor = DisallowedUnevaluatedPropertiesVisitor::default(); + let mut visitor = DisallowUnevaluatedPropertiesVisitor::default(); visitor.visit_root_schema(&mut actual_schema); assert_schemas_eq(expected_schema, actual_schema); @@ -712,7 +751,7 @@ mod tests { "additionalProperties": false })); - let mut visitor = DisallowedUnevaluatedPropertiesVisitor::default(); + let mut visitor = DisallowUnevaluatedPropertiesVisitor::default(); visitor.visit_root_schema(&mut actual_schema); let expected_schema = as_schema(json!({ @@ -758,7 +797,7 @@ mod tests { } })); - let mut visitor = DisallowedUnevaluatedPropertiesVisitor::default(); + let mut visitor = DisallowUnevaluatedPropertiesVisitor::default(); visitor.visit_root_schema(&mut actual_schema); let expected_schema = as_schema(json!({ @@ -813,7 +852,7 @@ mod tests { } })); - let mut visitor = DisallowedUnevaluatedPropertiesVisitor::default(); + let mut visitor = DisallowUnevaluatedPropertiesVisitor::default(); visitor.visit_root_schema(&mut actual_schema); let expected_schema = as_schema(json!({ @@ -879,7 +918,7 @@ mod tests { } })); - let mut visitor = DisallowedUnevaluatedPropertiesVisitor::default(); + let mut visitor = DisallowUnevaluatedPropertiesVisitor::default(); visitor.visit_root_schema(&mut actual_schema); // Expectations: