Skip to content

Commit

Permalink
settings: explicitly convert integer to HumanByteSize
Browse files Browse the repository at this point in the history
Since config::Value type was lax about data types, an integer value could be
deserialized through a string. This won't apply to new toml_edit-based value
types.
  • Loading branch information
yuja committed Dec 8, 2024
1 parent 2461602 commit 85816f2
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 7 deletions.
15 changes: 15 additions & 0 deletions cli/tests/test_working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,21 @@ fn test_snapshot_large_file() {
This will increase the maximum file size allowed for new files, for this command only.
"###);

// test invalid configuration
let stderr = test_env.jj_cmd_failure(
&repo_path,
&[
"file",
"list",
"--config-toml=snapshot.max-new-file-size = []",
],
);
insta::assert_snapshot!(stderr, @r"
Config error: Invalid type or value for snapshot.max-new-file-size
Caused by: Expected a positive integer or a string in '<number><unit>' form
For help, see https://martinvonz.github.io/jj/latest/config/.
");

// No error if we disable auto-tracking of the path
test_env.add_config(r#"snapshot.auto-track = 'none()'"#);
let stdout = test_env.jj_cmd_success(&repo_path, &["file", "list"]);
Expand Down
38 changes: 31 additions & 7 deletions lib/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,8 @@ impl UserSettings {

pub fn max_new_file_size(&self) -> Result<u64, ConfigGetError> {
let cfg = self
.get::<HumanByteSize>("snapshot.max-new-file-size")
.map(|x| x.0);
.get_value_with("snapshot.max-new-file-size", TryInto::try_into)
.map(|HumanByteSize(x)| x);
match cfg {
Ok(0) => Ok(u64::MAX),
x @ Ok(_) => x,
Expand Down Expand Up @@ -338,8 +338,7 @@ impl JJRng {
}

/// A size in bytes optionally formatted/serialized with binary prefixes
#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, serde::Deserialize)]
#[serde(try_from = "String")]
#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
pub struct HumanByteSize(pub u64);

impl std::fmt::Display for HumanByteSize {
Expand All @@ -363,11 +362,18 @@ impl FromStr for HumanByteSize {
}
}

impl TryFrom<String> for HumanByteSize {
impl TryFrom<ConfigValue> for HumanByteSize {
type Error = &'static str;

fn try_from(value: String) -> Result<Self, Self::Error> {
value.parse()
fn try_from(value: ConfigValue) -> Result<Self, Self::Error> {
if let Ok(n) = value.clone().into_int() {
let n = u64::try_from(n).map_err(|_| "Integer out of range")?;
Ok(HumanByteSize(n))
} else if let Ok(s) = value.into_string() {
s.parse()
} else {
Err("Expected a positive integer or a string in '<number><unit>' form")
}
}
}

Expand Down Expand Up @@ -398,6 +404,8 @@ fn parse_human_byte_size(v: &str) -> Result<u64, &'static str> {

#[cfg(test)]
mod tests {
use assert_matches::assert_matches;

use super::*;

#[test]
Expand All @@ -422,4 +430,20 @@ mod tests {
);
assert_eq!(parse_human_byte_size(""), Err("must start with a number"));
}

#[test]
fn byte_size_from_config_value() {
assert_eq!(
HumanByteSize::try_from(ConfigValue::from(42)).unwrap(),
HumanByteSize(42)
);
assert_eq!(
HumanByteSize::try_from(ConfigValue::from("42K")).unwrap(),
HumanByteSize(42 * 1024)
);
assert_matches!(
HumanByteSize::try_from(ConfigValue::from(-1)),
Err("Integer out of range")
);
}
}

0 comments on commit 85816f2

Please sign in to comment.