Skip to content

Commit a5aad0e

Browse files
committed
feat!: relax Reflection's validation requirements
BEFORE: A `Reflection` node is valid only if the *sum* of all its children sizes is less than or equal to the size of the node. AFTER: A `Reflection` node is valid only if there's no child whose size is greater than the size of the node. BREAKING: `data_tree::reflection::ConversionError::ExcessiveChildren` now has a different set of fields.
1 parent 0d725d4 commit a5aad0e

3 files changed

Lines changed: 24 additions & 46 deletions

File tree

src/data_tree/reflection.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,14 @@ pub struct Reflection<Name, Size: size::Size> {
4444
#[derive(Debug, Clone, PartialEq, Eq)]
4545
#[non_exhaustive]
4646
pub enum ConversionError<Name, Size: size::Size> {
47-
/// When a node's size is less than the sum of its children.
47+
/// When a node's size is less than one of its children.
4848
ExcessiveChildren {
4949
/// Path from root to the node.
5050
path: VecDeque<Name>,
5151
/// Size hold by the node.
5252
size: Size,
53-
/// Children of the node.
54-
children: Vec<Reflection<Name, Size>>,
55-
/// Sum of size hold by children of the node.
56-
children_sum: Size,
53+
/// The child whose size was greater than that of the node.
54+
child: Reflection<Name, Size>,
5755
},
5856
}
5957

@@ -65,19 +63,16 @@ where
6563
fn fmt(&self, formatter: &mut Formatter<'_>) -> Result<(), Error> {
6664
use ConversionError::*;
6765
match self {
68-
ExcessiveChildren {
69-
path,
70-
size,
71-
children_sum,
72-
..
73-
} => {
66+
ExcessiveChildren { path, size, child } => {
7467
let path = path
7568
.iter()
7669
.map(PathBuf::from)
7770
.fold(PathBuf::new(), |acc, x| acc.join(x));
7871
write!(
7972
formatter,
80-
"ExcessiveChildren: {path:?}: {size:?} is less than {children_sum:?}",
73+
"ExcessiveChildren: {path:?} ({size:?}) is less than a child named {child_name:?} ({child_size:?})",
74+
child_name = child.name,
75+
child_size = child.size,
8176
)
8277
}
8378
}

src/data_tree/reflection/par_methods.rs

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,15 @@ where
1515
size,
1616
children,
1717
} = self;
18-
let children_sum = children.iter().map(|child| child.size).sum();
19-
if size < children_sum {
20-
return Err(ConversionError::ExcessiveChildren {
21-
path: once(name).collect(),
22-
size,
23-
children,
24-
children_sum,
25-
});
18+
let excess_child = children
19+
.iter()
20+
.enumerate()
21+
.find(|(_, child)| child.size > size);
22+
if let Some((index, _)) = excess_child {
23+
let path = once(name).collect();
24+
let mut children = children;
25+
let child = children.swap_remove(index); // this still does the unnecessary work of swapping the elements, how to skip it?
26+
return Err(ConversionError::ExcessiveChildren { path, size, child });
2627
}
2728
let children: Result<Vec<_>, _> = children
2829
.into_par_iter()
@@ -33,16 +34,10 @@ where
3334
Err(ConversionError::ExcessiveChildren {
3435
mut path,
3536
size,
36-
children,
37-
children_sum,
37+
child,
3838
}) => {
3939
path.push_front(name);
40-
return Err(ConversionError::ExcessiveChildren {
41-
path,
42-
size,
43-
children,
44-
children_sum,
45-
});
40+
return Err(ConversionError::ExcessiveChildren { path, size, child });
4641
}
4742
};
4843
Ok(DataTree {

tests/data_tree_reflection.rs

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -121,23 +121,11 @@ fn invalid_conversion_excessive_children() {
121121
let expected = ConversionError::ExcessiveChildren {
122122
path: vec!["root", "b", "0"].into_iter().collect(),
123123
size: Bytes::new(321),
124-
children: vec![
125-
Reflection {
126-
name: "abc",
127-
size: Bytes::new(123),
128-
children: vec![Reflection {
129-
name: "xyz",
130-
size: Bytes::new(4321),
131-
children: Vec::new(),
132-
}],
133-
},
134-
Reflection {
135-
name: "def",
136-
size: Bytes::new(456),
137-
children: Vec::new(),
138-
},
139-
],
140-
children_sum: Bytes::new(123 + 456),
124+
child: Reflection {
125+
name: "def",
126+
size: Bytes::new(456),
127+
children: Vec::new(),
128+
},
141129
};
142130
assert_eq!(actual, expected);
143131
}
@@ -148,7 +136,7 @@ fn display_excessive_children() {
148136
.par_try_into_tree()
149137
.expect_err("create error")
150138
.to_string();
151-
let expected = r#"ExcessiveChildren: "root/b/0": Bytes(321) is less than Bytes(579)"#;
139+
let expected = r#"ExcessiveChildren: "root/b/0" (Bytes(321)) is less than a child named "def" (Bytes(456))"#;
152140
assert_eq!(actual, expected);
153141
}
154142

0 commit comments

Comments
 (0)