Skip to content

chore(config): Fix InlineSingleUseReferencesVisitor failing merges #23207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pront merged 2 commits into from
Jun 17, 2025
Merged

chore(config): Fix InlineSingleUseReferencesVisitor failing merges #23207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7aff4f5
chore(config): Fix `InlineSingleUseReferencesVisitor` failing tests
bruceg Jun 13, 2025
e81f5d9
Regenerate component docs
bruceg Jun 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 131 additions & 36 deletions lib/vector-config/src/schema/visitors/inline_single.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl Visitor for InlineSingleUseReferencesVisitor {
// entire schema, by visiting the root schema in a recursive fashion, using a helper visitor.
let mut occurrence_visitor = OccurrenceVisitor::default();
occurrence_visitor.visit_root_schema(root);
let occurrence_map = occurrence_visitor.into_occurrences();
let occurrence_map = occurrence_visitor.occurrence_map;

self.eligible_to_inline = occurrence_map
.into_iter()
Expand Down Expand Up @@ -141,16 +141,7 @@ fn is_inlineable_schema(definition_name: &str, schema: &SchemaObject) -> bool {
#[derive(Debug, Default)]
struct OccurrenceVisitor {
scope_stack: SchemaScopeStack,
occurrence_map: HashMap<SchemaReference, HashSet<SchemaReference>>,
}

impl OccurrenceVisitor {
fn into_occurrences(self) -> HashMap<SchemaReference, usize> {
self.occurrence_map
.into_iter()
.map(|(k, v)| (k, v.len()))
.collect()
}
occurrence_map: HashMap<SchemaReference, usize>,
}

impl Visitor for OccurrenceVisitor {
Expand All @@ -162,23 +153,11 @@ impl Visitor for OccurrenceVisitor {
visit_schema_object_scoped(self, definitions, schema);

if let Some(current_schema_ref) = schema.reference.as_ref() {
// Track the named "parent" schema for the schema we're currently visiting so that if we
// visit this schema again, we don't double count any schema references that it has. The
// "parent" schema is simply the closest ancestor schema that was itself a schema
// reference, or "Root", which represents the oldest schema ancestor in the document.
//
// This lets us couple with scenarios where schema A references schema B, and is the
// only actual direct schema reference to schema B, but due to multiple schemas
// referencing schema A, would otherwise lead to both A and B being visited multiple
// times.
let current_parent_schema_ref = self.get_current_schema_scope().clone();
let current_schema_ref = get_cleaned_schema_reference(current_schema_ref);

let occurrences = self
*self
.occurrence_map
.entry(current_schema_ref.into())
.or_default();
occurrences.insert(current_parent_schema_ref);
.or_default() += 1;
}
}
}
Expand Down Expand Up @@ -257,17 +236,7 @@ mod tests {
assert_schemas_eq(expected_schema, actual_schema);
}

// TODO(tobz): These two tests are ignored because the inliner currently works off of schema
// reference scopes, so two object properties within the same schema don't count as two
// instances of a schema being referenced.
//
// We need to refactor schema scopes to be piecemeal extensible more in the way of how
// `jsonschema` does it rather than an actual stack.... the current approach is good enough for
// now, but not optimal in the way that it could be.
//
// These are here for when I improve the situation after getting this merged.
#[test]
#[ignore]
fn single_ref_multiple_usages() {
let mut actual_schema = as_schema(json!({
"definitions": {
Expand All @@ -294,7 +263,6 @@ mod tests {
}

#[test]
#[ignore]
fn multiple_refs_mixed_usages() {
let mut actual_schema = as_schema(json!({
"definitions": {
Expand Down Expand Up @@ -346,4 +314,131 @@ mod tests {

assert_schemas_eq(expected_schema, actual_schema);
}

#[test]
fn reference_in_multiple_arrays() {
let mut actual_schema = as_schema(json!({
"definitions": {
"item": {
"type": "object",
"properties": {
"x": { "type": "string" }
}
}
},
"type": "object",
"properties": {
"arr1": { "type": "array", "items": { "$ref": "#/definitions/item" } },
"arr2": { "type": "array", "items": { "$ref": "#/definitions/item" } }
}
}));

let expected_schema = actual_schema.clone();

let mut visitor = InlineSingleUseReferencesVisitor::default();
visitor.visit_root_schema(&mut actual_schema);

assert_schemas_eq(expected_schema, actual_schema);
}

#[test]
fn reference_in_oneof_anyof_allof() {
let mut actual_schema = as_schema(json!({
"definitions": {
"shared": {
"type": "object",
"properties": {
"y": { "type": "string" }
}
}
},
"type": "object",
"properties": {
"choice": {
"oneOf": [
{ "$ref": "#/definitions/shared" },
{ "$ref": "#/definitions/shared" }
],
"anyOf": [
{ "$ref": "#/definitions/shared" },
{ "type": "null" }
],
"allOf": [
{ "$ref": "#/definitions/shared" },
{ "type": "object" }
]
}
}
}));

let expected_schema = actual_schema.clone();

let mut visitor = InlineSingleUseReferencesVisitor::default();
visitor.visit_root_schema(&mut actual_schema);

assert_schemas_eq(expected_schema, actual_schema);
}

#[test]
fn reference_in_additional_properties() {
let mut actual_schema = as_schema(json!({
"definitions": {
"val": {
"type": "object",
"properties": {
"z": { "type": "string" }
}
}
},
"type": "object",
"properties": {
"obj1": {
"type": "object",
"additionalProperties": { "$ref": "#/definitions/val" }
},
"obj2": {
"type": "object",
"additionalProperties": { "$ref": "#/definitions/val" }
}
}
}));

let expected_schema = actual_schema.clone();

let mut visitor = InlineSingleUseReferencesVisitor::default();
visitor.visit_root_schema(&mut actual_schema);

assert_schemas_eq(expected_schema, actual_schema);
}

#[test]
fn reference_in_pattern_properties() {
let mut actual_schema = as_schema(json!({
"definitions": {
"pat": {
"type": "object",
"properties": {
"w": { "type": "string" }
}
}
},
"type": "object",
"properties": {
"obj": {
"type": "object",
"patternProperties": {
"^foo$": { "$ref": "#/definitions/pat" },
"^bar$": { "$ref": "#/definitions/pat" }
}
}
}
}));

let expected_schema = actual_schema.clone();

let mut visitor = InlineSingleUseReferencesVisitor::default();
visitor.visit_root_schema(&mut actual_schema);

assert_schemas_eq(expected_schema, actual_schema);
}
}
8 changes: 2 additions & 6 deletions website/cue/reference/components/sinks/base/http.cue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,12 +683,8 @@ base: components: sinks: http: configuration: {
}
}
method: {
description: """
HTTP method.

The HTTP method to use when making the request.
"""
required: false
description: "The HTTP method to use when making the request."
required: false
type: string: {
default: "post"
enum: {
Expand Down
8 changes: 2 additions & 6 deletions website/cue/reference/components/sinks/base/opentelemetry.cue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,12 +686,8 @@ base: components: sinks: opentelemetry: configuration: protocol: {
}
}
method: {
description: """
HTTP method.

The HTTP method to use when making the request.
"""
required: false
description: "The HTTP method to use when making the request."
required: false
type: string: {
default: "post"
enum: {
Expand Down
8 changes: 2 additions & 6 deletions website/cue/reference/components/sources/base/static_metrics.cue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,8 @@ base: components: sources: static_metrics: configuration: {
type: float: {}
}
bins: {
description: """
A split representation of sketch bins.
Copy link
Member

@pront pront Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting that this bit was deleted.


The bins within the sketch.
"""
required: true
description: "The bins within the sketch."
required: true
type: object: options: {
k: {
description: "The bin keys."
Expand Down
Loading