Skip to content

Commit 8f578c2

Browse files
committed
fix: Change hoist_any_of_option_enum to cater to nested any_of as well as enum
All tests in this PR are passing Note: The function should be renamed, but it needs some work to not only do optional things.
1 parent 8abef36 commit 8f578c2

File tree

1 file changed

+63
-22
lines changed

1 file changed

+63
-22
lines changed

kube-core/src/schema.rs

Lines changed: 63 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ fn hoist_one_of_enum(incoming: SchemaObject) -> SchemaObject {
442442

443443
*new_instance_type = Some(hoisted_instance_type.clone());
444444

445+
// Merge the enums:
445446
// For each `oneOf` entry, iterate over the `enum` and `const` values.
446447
// Panic on an entry that doesn't contain an `enum` or `const`.
447448
let new_enums = one_of.iter().flat_map(|obj| match obj {
@@ -499,49 +500,85 @@ fn hoist_any_of_option_enum(incoming: SchemaObject) -> SchemaObject {
499500
"nullable": true
500501
});
501502

502-
// iter through any_of for matching null
503-
let results: [bool; 2] = any_of
503+
// iter through any_of entries, converting them to Value,
504+
// and build a truth table for null matches
505+
let entry_is_null: [bool; 2] = any_of
504506
.iter()
505507
.map(|x| serde_json::to_value(x).expect("schema should be able to convert to JSON"))
506508
.map(|x| x == null)
507509
.collect::<Vec<_>>()
508510
.try_into()
509511
.expect("there should be exactly 2 elements. We checked earlier");
510512

511-
let to_hoist = match results {
512-
[true, true] => panic!("Too many nulls, not enough drinks"),
513+
// Get the `any_of` entry that isn't the null entry
514+
let non_null_any_of_entry = match entry_is_null {
515+
[true, true] => return incoming,
513516
[true, false] => &any_of[1],
514517
[false, true] => &any_of[0],
515518
[false, false] => return incoming,
516519
};
517520

518521
// my goodness!
519-
let Schema::Object(to_hoist) = to_hoist else {
522+
let Schema::Object(to_hoist) = non_null_any_of_entry else {
520523
panic!("Somehow we have stumbled across a bool schema");
521524
};
522525

523526
let mut new_schema = incoming.clone();
527+
// only do this if we still have `non_null_any_of_entry`. If the code changes, we need to revise this
528+
new_schema.other["nullable"] = true.into();
524529

525530
let mut new_metadata = incoming.metadata.clone().unwrap_or_default();
526531
new_metadata.description = to_hoist.metadata.as_ref().and_then(|m| m.description.clone());
527532

528533
new_schema.metadata = Some(new_metadata);
529534
new_schema.instance_type = to_hoist.instance_type.clone();
530-
new_schema.enum_values = to_hoist.enum_values.clone();
531-
new_schema.other["nullable"] = true.into();
532-
533-
new_schema
535+
if to_hoist.enum_values.as_ref().is_some_and(|v| !v.is_empty()) {
536+
// need to hoist enum up to parent
537+
new_schema.enum_values = to_hoist.enum_values.clone();
538+
539+
// should we clear out subschemas?
540+
// we do need to clear out anyOf, since that's where we got the enum that is now hoisted.
541+
// but if this code changes to handle one_of too, then we need to ensure we don't drop data that we haven't hoisted yet.
542+
new_schema
543+
.subschemas
544+
.as_mut()
545+
.expect("we have asserted that there is any_of")
546+
.any_of = None;
547+
} else if to_hoist
548+
.subschemas
549+
.as_ref()
550+
.is_some_and(|s| s.any_of.as_ref().is_some_and(|x| !x.is_empty()))
551+
{
552+
// Replace the parent any_of with the nested any_of
553+
// TODO (@NickLarsenNZ): Might need to find the null object and clear it out.
554+
new_schema
555+
.subschemas
556+
.as_mut()
557+
.expect("We have asserted there is any_of")
558+
.any_of = to_hoist
559+
.subschemas
560+
.as_ref()
561+
.expect("We have asserted there is any_of")
562+
.any_of
563+
.clone();
564+
// Bring across the properties, etc...
565+
// Is this ok?
566+
new_schema.object = to_hoist.object.clone();
567+
} else if to_hoist
534568
.subschemas
535-
.as_mut()
536-
.expect("we have asserted that there is any_of")
537-
.any_of = None;
569+
.as_ref()
570+
.is_some_and(|s| s.one_of.as_ref().is_some_and(|x| !x.is_empty()))
571+
{
572+
panic!("We have an unexpected one_of nested under an any_of. Not yet sure what and how to hoist")
573+
}
538574

539575
new_schema
540576
}
541577

542578

543579
impl Transform for StructuralSchemaRewriter {
544580
fn transform(&mut self, transform_schema: &mut schemars::Schema) {
581+
// Apply this (self) transform to all subschemas
545582
schemars::transform::transform_subschemas(self, transform_schema);
546583

547584
// TODO (@NickLarsenNZ): Replace with conversion function
@@ -551,20 +588,23 @@ impl Transform for StructuralSchemaRewriter {
551588
};
552589
let schema = hoist_one_of_enum(schema);
553590
let schema = hoist_any_of_option_enum(schema);
554-
// todo: let schema = strip_any_of_empty_object_entry(schema);
591+
// todo: let schema = strip_any_of_empty_object_entry(schema); // this is currently done in an `else if` block in hoist_subschema_properties()
555592
let mut schema = schema;
593+
556594
if let Some(subschemas) = &mut schema.subschemas {
557-
if let Some(one_of) = subschemas.one_of.as_mut() {
558-
// Tagged enums are serialized using `one_of`
559-
hoist_subschema_properties(one_of, &mut schema.object, &mut schema.instance_type);
595+
// // if let Some(one_of) = subschemas.one_of.as_mut() {
596+
// // // Tagged enums are serialized using `one_of`
597+
// // // Maybe we also have to hoist properties?
598+
// // hoist_subschema_properties(one_of, &mut schema.object, &mut schema.instance_type);
560599

561-
// "Plain" enums are serialized using `one_of` if they have doc tags
562-
hoist_subschema_enum_values(one_of, &mut schema.enum_values, &mut schema.instance_type);
600+
// // // "Plain" enums are serialized using `one_of` if they have doc tags
601+
// // // This is now done by hoist_one_of_enum
602+
// // hoist_subschema_enum_values(one_of, &mut schema.enum_values, &mut schema.instance_type);
563603

564-
if one_of.is_empty() {
565-
subschemas.one_of = None;
566-
}
567-
}
604+
// // if one_of.is_empty() {
605+
// // subschemas.one_of = None;
606+
// // }
607+
// // }
568608

569609
if let Some(any_of) = &mut subschemas.any_of {
570610
// Untagged enums are serialized using `any_of`
@@ -594,6 +634,7 @@ impl Transform for StructuralSchemaRewriter {
594634
array.unique_items = None;
595635
}
596636

637+
// Convert back to schemars::Schema
597638
if let Ok(schema) = serde_json::to_value(schema) {
598639
if let Ok(transformed) = serde_json::from_value(schema) {
599640
*transform_schema = transformed;

0 commit comments

Comments
 (0)