Add self_profile_processed types for processed self-profile endpoint

Instead of reusing `self_profile_raw` request and response types, use
separate `self_profile_processed` types.

Also, remove `get_self_profile_raw` and use serde to deserialize
requests, which automatically enables support for the previously
non-functioning `cid` parameter.

Author: tgnottingham

site/src/api.rs

@@ -266,6 +266,38 @@ pub mod self_profile_raw {
     }
 }
 
+pub mod self_profile_processed {
+    use serde::{Deserialize, Serialize};
+
+    #[derive(Debug, PartialEq, Copy, Clone, Serialize, Deserialize)]
+    #[serde(rename_all = "lowercase")]
+    pub enum ProcessorType {
+        #[serde(rename = "codegen-schedule")]
+        CodegenSchedule,
+        Crox,
+        Flamegraph,
+    }
+
+    #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
+    pub struct Request {
+        pub commit: String,
+        pub benchmark: String,
+        pub run_name: String,
+        pub cid: Option<i32>,
+        #[serde(rename = "type")]
+        pub processor_type: ProcessorType,
+        #[serde(default, flatten)]
+        pub params: std::collections::HashMap<String, String>,
+    }
+
+    #[derive(Debug, Clone, Serialize)]
+    pub struct Response {
+        pub cids: Vec<i32>,
+        pub cid: i32,
+        pub url: String,
+    }
+}
+
 pub mod self_profile {
     use database::QueryLabel;
     use serde::{Deserialize, Serialize};

site/src/request_handlers.rs

@@ -12,8 +12,8 @@ pub use github::handle_github;
 pub use graph::{handle_graph, handle_graph_new};
 pub use next_commit::handle_next_commit;
 pub use self_profile::{
-    get_self_profile_raw, handle_self_profile, handle_self_profile_processed_download,
-    handle_self_profile_raw, handle_self_profile_raw_download,
+    handle_self_profile, handle_self_profile_processed_download, handle_self_profile_raw,
+    handle_self_profile_raw_download,
 };
 pub use status_page::handle_status_page;
 

site/src/request_handlers/self_profile.rs

@@ -1,31 +1,29 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::HashSet;
 use std::io::Read;
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 
 use bytes::Buf;
 use headers::{ContentType, Header};
 use hyper::StatusCode;
-use log::error;
 
-use crate::api::{self_profile, self_profile_raw, ServerResult};
+use crate::api::{self_profile, self_profile_processed, self_profile_raw, ServerResult};
 use crate::db::ArtifactId;
 use crate::load::SiteCtxt;
 use crate::selector::{self, Tag};
-use crate::server::{Request, Response, ResponseHeaders};
+use crate::server::{Response, ResponseHeaders};
 
 pub async fn handle_self_profile_processed_download(
-    body: self_profile_raw::Request,
-    mut params: HashMap<String, String>,
+    body: self_profile_processed::Request,
     ctxt: &SiteCtxt,
 ) -> http::Response<hyper::Body> {
+    let mut params = body.params.clone();
     let diff_against = params.remove("base_commit");
-    if params
-        .get("type")
-        .map_or(false, |t| t != "codegen-schedule")
+
+    if body.processor_type != self_profile_processed::ProcessorType::CodegenSchedule
         && diff_against.is_some()
     {
-        let mut resp = Response::new("Only codegen_schedule supports diffing right now.".into());
+        let mut resp = Response::new("Only codegen-schedule supports diffing right now.".into());
         *resp.status_mut() = StatusCode::BAD_REQUEST;
         return resp;
     }
@@ -75,7 +73,17 @@ pub async fn handle_self_profile_processed_download(
         None
     };
 
-    let data = match handle_self_profile_raw(body, ctxt).await {
+    let data = match handle_self_profile_raw(
+        self_profile_raw::Request {
+            commit: body.commit,
+            benchmark: body.benchmark.clone(),
+            run_name: body.run_name.clone(),
+            cid: body.cid,
+        },
+        ctxt,
+    )
+    .await
+    {
         Ok(v) => match get_self_profile_raw_data(&v.url).await {
             Ok(v) => v,
             Err(e) => return e,
@@ -89,15 +97,16 @@ pub async fn handle_self_profile_processed_download(
 
     log::trace!("got data in {:?}", start.elapsed());
 
-    let output = match crate::self_profile::generate(&title, base_data, data, params) {
-        Ok(c) => c,
-        Err(e) => {
-            log::error!("Failed to generate json {:?}", e);
-            let mut resp = http::Response::new(format!("{:?}", e).into());
-            *resp.status_mut() = StatusCode::INTERNAL_SERVER_ERROR;
-            return resp;
-        }
-    };
+    let output =
+        match crate::self_profile::generate(&title, body.processor_type, base_data, data, params) {
+            Ok(c) => c,
+            Err(e) => {
+                log::error!("Failed to generate json {:?}", e);
+                let mut resp = http::Response::new(format!("{:?}", e).into());
+                *resp.status_mut() = StatusCode::INTERNAL_SERVER_ERROR;
+                return resp;
+            }
+        };
     let mut builder = http::Response::builder()
         .header_typed(if output.filename.ends_with("json") {
             ContentType::json()
@@ -436,60 +445,6 @@ pub async fn handle_self_profile_raw_download(
     server_resp
 }
 
-pub fn get_self_profile_raw(
-    req: &Request,
-) -> Result<(HashMap<String, String>, self_profile_raw::Request), http::Response<hyper::Body>> {
-    // FIXME: how should this look?
-    let url = match url::Url::parse(&format!("http://example.com{}", req.uri())) {
-        Ok(v) => v,
-        Err(e) => {
-            error!("failed to parse url {}: {:?}", req.uri(), e);
-            return Err(http::Response::builder()
-                .header_typed(ContentType::text_utf8())
-                .status(StatusCode::BAD_REQUEST)
-                .body(hyper::Body::from(format!(
-                    "failed to parse url {}: {:?}",
-                    req.uri(),
-                    e
-                )))
-                .unwrap());
-        }
-    };
-    let mut parts = url
-        .query_pairs()
-        .into_owned()
-        .collect::<HashMap<String, String>>();
-    macro_rules! key_or_error {
-        ($ident:ident) => {
-            if let Some(v) = parts.remove(stringify!($ident)) {
-                v
-            } else {
-                error!(
-                    "failed to deserialize request {}: missing {} in query string",
-                    req.uri(),
-                    stringify!($ident)
-                );
-                return Err(http::Response::builder()
-                    .header_typed(ContentType::text_utf8())
-                    .status(StatusCode::BAD_REQUEST)
-                    .body(hyper::Body::from(format!(
-                        "failed to deserialize request {}: missing {} in query string",
-                        req.uri(),
-                        stringify!($ident)
-                    )))
-                    .unwrap());
-            }
-        };
-    }
-    let request = self_profile_raw::Request {
-        commit: key_or_error!(commit),
-        benchmark: key_or_error!(benchmark),
-        run_name: key_or_error!(run_name),
-        cid: None,
-    };
-    return Ok((parts, request));
-}
-
 async fn tarball(resp: reqwest::Response, mut sender: hyper::body::Sender) {
     // Ideally, we would stream the response though the snappy decoding, but
     // snappy doesn't support that AFAICT -- we'd need it to implement AsyncRead

site/src/self_profile.rs

@@ -8,6 +8,8 @@ mod codegen_schedule;
 pub mod crox;
 pub mod flamegraph;
 
+pub type ProcessorType = crate::api::self_profile_processed::ProcessorType;
+
 pub struct Output {
     pub data: Vec<u8>,
     pub filename: &'static str,
@@ -16,13 +18,13 @@ pub struct Output {
 
 pub fn generate(
     title: &str,
+    processor_type: ProcessorType,
     self_profile_base_data: Option<Vec<u8>>,
     self_profile_data: Vec<u8>,
-    mut params: HashMap<String, String>,
+    params: HashMap<String, String>,
 ) -> anyhow::Result<Output> {
-    let removed = params.remove("type");
-    match removed.as_deref() {
-        Some("crox") => {
+    match processor_type {
+        ProcessorType::Crox => {
             let opt = serde_json::from_str(&serde_json::to_string(&params).unwrap())
                 .context("crox opts")?;
             Ok(Output {
@@ -31,7 +33,7 @@ pub fn generate(
                 is_download: true,
             })
         }
-        Some("flamegraph") => {
+        ProcessorType::Flamegraph => {
             let opt = serde_json::from_str(&serde_json::to_string(&params).unwrap())
                 .context("flame opts")?;
             Ok(Output {
@@ -40,7 +42,7 @@ pub fn generate(
                 is_download: false,
             })
         }
-        Some("codegen-schedule") => {
+        ProcessorType::CodegenSchedule => {
             let opt =
                 serde_json::from_str(&serde_json::to_string(&params).unwrap()).context("params")?;
             Ok(Output {

site/src/server.rs

@@ -313,7 +313,9 @@ async fn serve_req(server: Server, req: Request) -> Result<Response, ServerError
         "/perf/triage" if *req.method() == http::Method::GET => {
             let ctxt: Arc<SiteCtxt> = server.ctxt.read().as_ref().unwrap().clone();
             let input: triage::Request = check!(parse_query_string(req.uri()));
-            return Ok(to_triage_response(crate::comparison::handle_triage(input, &ctxt).await));
+            return Ok(to_triage_response(
+                crate::comparison::handle_triage(input, &ctxt).await,
+            ));
         }
         "/perf/metrics" => {
             return Ok(server.handle_metrics(req).await);
@@ -327,13 +329,9 @@ async fn serve_req(server: Server, req: Request) -> Result<Response, ServerError
             return Ok(request_handlers::handle_self_profile_raw_download(req, &ctxt).await);
         }
         "/perf/processed-self-profile" => {
-            return match request_handlers::get_self_profile_raw(&req) {
-                Ok((parts, v)) => {
-                    let ctxt: Arc<SiteCtxt> = server.ctxt.read().as_ref().unwrap().clone();
-                    Ok(request_handlers::handle_self_profile_processed_download(v, parts, &ctxt).await)
-                }
-                Err(e) => Ok(e),
-            };
+            let ctxt: Arc<SiteCtxt> = server.ctxt.read().as_ref().unwrap().clone();
+            let req = check!(parse_query_string(req.uri()));
+            return Ok(request_handlers::handle_self_profile_processed_download(req, &ctxt).await);
         }
         _ if req.method() == http::Method::GET => return Ok(not_found()),
         _ => {}
@@ -451,7 +449,7 @@ async fn serve_req(server: Server, req: Request) -> Result<Response, ServerError
             },
         ),
         "/perf/triage" => Ok(to_triage_response(
-            crate::comparison::handle_triage(check!(parse_body(&body)), &ctxt).await
+            crate::comparison::handle_triage(check!(parse_body(&body)), &ctxt).await,
         )),
         _ => Ok(http::Response::builder()
             .header_typed(ContentType::html())
@@ -605,13 +603,11 @@ fn to_triage_response(result: ServerResult<api::triage::Response>) -> Response {
             let response = http::Response::builder().header_typed(ContentType::text());
             response.body(hyper::Body::from(result.0)).unwrap()
         }
-        Err(err) => {
-            http::Response::builder()
-                .status(StatusCode::INTERNAL_SERVER_ERROR)
-                .header_typed(ContentType::text_utf8())
-                .body(hyper::Body::from(err.to_string()))
-                .unwrap()
-        }
+        Err(err) => http::Response::builder()
+            .status(StatusCode::INTERNAL_SERVER_ERROR)
+            .header_typed(ContentType::text_utf8())
+            .body(hyper::Body::from(err.to_string()))
+            .unwrap(),
     }
 }