From 1a71b72c391cd747f90099141c7946b5af52655b Mon Sep 17 00:00:00 2001
From: Christian Schwarz <christian@neon.tech>
Date: Fri, 15 Dec 2023 15:09:39 +0000
Subject: [PATCH] move serialization roundtrip to rust unit test

---
 pageserver/src/client/mgmt_api.rs          |  2 +-
 pageserver/src/http/models/partitioning.rs | 39 ++++++++++++++++++++++
 pageserver/src/http/routes.rs              | 16 +--------
 3 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/pageserver/src/client/mgmt_api.rs b/pageserver/src/client/mgmt_api.rs
index 66d7da9d0b..8e50741029 100644
--- a/pageserver/src/client/mgmt_api.rs
+++ b/pageserver/src/client/mgmt_api.rs
@@ -99,7 +99,7 @@ impl Client {
         timeline_id: TimelineId,
     ) -> Result<crate::http::models::partitioning::Partitioning> {
         let uri = format!(
-            "{}/v1/tenant/{tenant_id}/timeline/{timeline_id}/keyspace?check_serialization_roundtrip=true",
+            "{}/v1/tenant/{tenant_id}/timeline/{timeline_id}/keyspace",
             self.mgmt_api_endpoint
         );
         self.get(&uri)
diff --git a/pageserver/src/http/models/partitioning.rs b/pageserver/src/http/models/partitioning.rs
index bf0a62f3a7..d52794fd8c 100644
--- a/pageserver/src/http/models/partitioning.rs
+++ b/pageserver/src/http/models/partitioning.rs
@@ -110,3 +110,42 @@ impl<'a> serde::Deserialize<'a> for Partitioning {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_serialization_roundtrip() {
+        let reference = r#"
+        {
+            "keys": [
+              [
+                "000000000000000000000000000000000000",
+                "000000000000000000000000000000000001"
+              ],
+              [
+                "000000067F00000001000000000000000000",
+                "000000067F00000001000000000000000002"
+              ],
+              [
+                "030000000000000000000000000000000000",
+                "030000000000000000000000000000000003"
+              ]
+            ],
+            "at_lsn": "0/2240160"
+        }
+        "#;
+
+        let de: Partitioning = serde_json::from_str(reference).unwrap();
+
+        let ser = serde_json::to_string(&de).unwrap();
+
+        let ser_de: serde_json::Value = serde_json::from_str(&ser).unwrap();
+
+        assert_eq!(
+            ser_de,
+            serde_json::from_str::<'_, serde_json::Value>(reference).unwrap()
+        );
+    }
+}
diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs
index c63c691726..d5248fb996 100644
--- a/pageserver/src/http/routes.rs
+++ b/pageserver/src/http/routes.rs
@@ -1489,9 +1489,6 @@ async fn timeline_collect_keyspace(
 
     let at_lsn: Option<Lsn> = parse_query_param(&request, "at_lsn")?;
 
-    let check_serialization_roundtrip: bool =
-        parse_query_param(&request, "check_serialization_roundtrip")?.unwrap_or(false);
-
     async {
         let ctx = RequestContext::new(TaskKind::MgmtRequest, DownloadBehavior::Download);
         let timeline = active_timeline_of_active_tenant(tenant_shard_id, timeline_id).await?;
@@ -1502,18 +1499,7 @@ async fn timeline_collect_keyspace(
             .map_err(|e| ApiError::InternalServerError(e.into()))?;
 
         let res = crate::http::models::partitioning::Partitioning { keys, at_lsn };
-        if check_serialization_roundtrip {
-            (|| {
-                let ser = serde_json::ser::to_vec(&res).context("serialize")?;
-                let de: crate::http::models::partitioning::Partitioning =
-                    serde_json::from_slice(&ser).context("deserialize")?;
-                anyhow::ensure!(de == res, "not equal");
-                info!("passed serialization rountrip check");
-                Ok(())
-            })()
-            .context("serialization rountrip")
-            .map_err(ApiError::InternalServerError)?;
-        }
+      
         json_response(StatusCode::OK, res)
     }
     .instrument(info_span!("timeline_collect_keyspace", tenant_id = %tenant_shard_id.tenant_id, shard_id = %tenant_shard_id.shard_slug(), %timeline_id))