From 9d78e98467f57eb0e64de1703734034132ae9141 Mon Sep 17 00:00:00 2001
From: Christian Schwarz <christian@neon.tech>
Date: Mon, 27 Mar 2023 18:27:29 +0200
Subject: [PATCH] refactor: move the percentage value deserialization to
 newtype in `utils`

---
 libs/utils/src/lib.rs                      |  2 +
 libs/utils/src/serde_percent.rs            | 83 ++++++++++++++++++++++
 pageserver/src/disk_usage_eviction_task.rs | 18 +----
 3 files changed, 87 insertions(+), 16 deletions(-)
 create mode 100644 libs/utils/src/serde_percent.rs

diff --git a/libs/utils/src/lib.rs b/libs/utils/src/lib.rs
index 2c8bb6001e..6435b18d4b 100644
--- a/libs/utils/src/lib.rs
+++ b/libs/utils/src/lib.rs
@@ -52,6 +52,8 @@ pub mod history_buffer;
 
 pub mod measured_stream;
 
+pub mod serde_percent;
+
 /// use with fail::cfg("$name", "return(2000)")
 #[macro_export]
 macro_rules! failpoint_sleep_millis_async {
diff --git a/libs/utils/src/serde_percent.rs b/libs/utils/src/serde_percent.rs
new file mode 100644
index 0000000000..f9eab3269c
--- /dev/null
+++ b/libs/utils/src/serde_percent.rs
@@ -0,0 +1,83 @@
+//! A serde::Desierialize type for percentages.
+//!
+//! See [`Value`] for details.
+
+use serde::{Deserialize, Serialize};
+
+/// If the value is not an integer between 0 and 100,
+/// deserialization fails with a descriptive error.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
+#[serde(transparent)]
+pub struct Value(#[serde(deserialize_with = "deserialize_pct_0_to_100")] u8);
+
+impl Value {
+    pub fn get(&self) -> u8 {
+        self.0
+    }
+}
+
+fn deserialize_pct_0_to_100<'de, D>(deserializer: D) -> Result<u8, D::Error>
+where
+    D: serde::de::Deserializer<'de>,
+{
+    let v: u8 = serde::de::Deserialize::deserialize(deserializer)?;
+    if v > 100 {
+        return Err(serde::de::Error::custom(
+            "must be an integer between 0 and 100",
+        ));
+    }
+    Ok(v)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::Value;
+
+    #[derive(serde::Deserialize, serde::Serialize, Debug, PartialEq, Eq)]
+    struct Foo {
+        bar: Value,
+    }
+
+    #[test]
+    fn basics() {
+        let input = r#"{ "bar": 50 }"#;
+        let foo: Foo = serde_json::from_str(input).unwrap();
+        assert_eq!(foo.bar.get(), 50);
+    }
+    #[test]
+    fn null_handling() {
+        let input = r#"{ "bar": null }"#;
+        let res: Result<Foo, _> = serde_json::from_str(input);
+        assert!(res.is_err());
+    }
+    #[test]
+    fn zero() {
+        let input = r#"{ "bar": 0 }"#;
+        let foo: Foo = serde_json::from_str(input).unwrap();
+        assert_eq!(foo.bar.get(), 0);
+    }
+    #[test]
+    fn out_of_range_above() {
+        let input = r#"{ "bar": 101 }"#;
+        let res: Result<Foo, _> = serde_json::from_str(input);
+        assert!(res.is_err());
+    }
+    #[test]
+    fn out_of_range_below() {
+        let input = r#"{ "bar": -1 }"#;
+        let res: Result<Foo, _> = serde_json::from_str(input);
+        assert!(res.is_err());
+    }
+    #[test]
+    fn float() {
+        let input = r#"{ "bar": 50.5 }"#;
+        let res: Result<Foo, _> = serde_json::from_str(input);
+        assert!(res.is_err());
+    }
+    #[test]
+    fn string() {
+        let input = r#"{ "bar": "50 %" }"#;
+        let res: Result<Foo, _> = serde_json::from_str(input);
+        assert!(res.is_err());
+    }
+}
diff --git a/pageserver/src/disk_usage_eviction_task.rs b/pageserver/src/disk_usage_eviction_task.rs
index 86654134af..b3ca59a4af 100644
--- a/pageserver/src/disk_usage_eviction_task.rs
+++ b/pageserver/src/disk_usage_eviction_task.rs
@@ -38,7 +38,7 @@ use sync_wrapper::SyncWrapper;
 use tokio::time::Instant;
 use tokio_util::sync::CancellationToken;
 use tracing::{debug, error, info, instrument, warn, Instrument};
-use utils::{approx_accurate::ApproxAccurate, id::TenantId};
+use utils::{approx_accurate::ApproxAccurate, id::TenantId, serde_percent};
 
 use crate::{
     config::PageServerConf,
@@ -46,23 +46,9 @@ use crate::{
     tenant::{self, LocalLayerInfoForDiskUsageEviction, Timeline},
 };
 
-fn deserialize_pct_0_to_100<'de, D>(deserializer: D) -> Result<u64, D::Error>
-where
-    D: serde::de::Deserializer<'de>,
-{
-    let v: u64 = serde::de::Deserialize::deserialize(deserializer)?;
-    if v > 100 {
-        return Err(serde::de::Error::custom(
-            "must be an integer between 0 and 100",
-        ));
-    }
-    Ok(v)
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct DiskUsageEvictionTaskConfig {
-    #[serde(deserialize_with = "deserialize_pct_0_to_100")]
-    pub max_usage_pct: u64,
+    pub max_usage_pct: serde_percent::Value,
     pub min_avail_bytes: u64,
     #[serde(with = "humantime_serde")]
     pub period: Duration,