chore: use real structs in hoghook payloads

rust/hook-api/src/handlers/webhook.rs

@@ -65,79 +65,69 @@ pub async fn post_webhook(
     Ok(Json(WebhookPostResponse { error: None }))
 }
 
-#[derive(Debug, Deserialize)]
-pub struct HogFetchParameters {
+#[derive(Debug, Serialize, Deserialize, Clone)]
+pub struct HoghookFetchParameters {
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub body: Option<String>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub headers: Option<HashMap<String, String>>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub method: Option<HttpMethod>,
 }
 
-// Hoghook expects a JSON payload in the format of `HogFunctionInvocationResult` (as seen in
-// plugin-server), but we accept a plain `Json<Value>` via Axum here, and this is why:
-// * The reason we don't decode that into a `HogFunctionInvocationResult`-shaped Rust struct is that
-//   there's no benefit in mirroring the exact shape of that type (and keeping it sync with the
-//   plugin-server type).
-// * Hoghook only cares about a small subset of the payload (the `asyncFunctionRequest` field), and
-//   the reason we don't decode *that* into a Rust struct is because the function args are a simple
-//   array (because this type is used for more than just `fetch` requests), and so we would need to
-//   manually validate and destructure the array elements anyway.
-// * Additionally, don't want to discard the  rest of the payload because we pass it back to the
-//   plugin-server after receiving the response body from the remote server. By accepting a plain
-//   `Json<Value>` we only decode the JSON once, we can do our minimal validation/extraction, and we
-//   can save the rest of the payload for later.
+#[derive(Debug, Serialize, Deserialize)]
+struct HoghookArgs(
+    String,
+    #[serde(default, skip_serializing_if = "Option::is_none")] Option<HoghookFetchParameters>,
+);
+
+#[derive(Debug, Serialize, Deserialize)]
+struct HoghookAsyncFunctionRequest {
+    name: String,
+    args: HoghookArgs,
+}
+
 pub async fn post_hoghook(
     State(pg_queue): State<PgQueue>,
-    Json(mut payload): Json<Value>,
+    Json(payload): Json<Value>,
 ) -> Result<Json<WebhookPostResponse>, (StatusCode, Json<WebhookPostResponse>)> {
     debug!("received payload: {:?}", payload);
 
-    let parameters: WebhookJobParameters = match &mut payload {
-        Value::Object(object) => {
-            let async_fn_request = object
-                .get("asyncFunctionRequest")
-                .ok_or_else(|| bad_request("missing required field 'asyncFunctionRequest'"))?;
-
-            let name = async_fn_request
-                .get("name")
-                .ok_or_else(|| bad_request("missing required field 'asyncFunctionRequest.name'"))?;
-
-            if name != "fetch" {
-                return Err(bad_request("asyncFunctionRequest.name must be 'fetch'"));
-            }
-
-            let args = async_fn_request
-                .get("args")
-                .ok_or_else(|| bad_request("missing required field 'asyncFunctionRequest.args'"))?;
-
-            // Note that the URL is parsed (and thus validated as a valid URL) as part of
-            // `get_hostname` below.
-            let url = args.get(0).ok_or_else(|| {
-                bad_request("missing required field 'asyncFunctionRequest.args[0]'")
-            })?;
-
-            let fetch_options: HogFetchParameters = if let Some(value) = args.get(1) {
-                serde_json::from_value(value.clone()).map_err(|_| {
-                    bad_request("failed to deserialize asyncFunctionRequest.args[1]")
-                })?
-            } else {
-                HogFetchParameters {
-                    body: None,
-                    headers: None,
-                    method: None,
-                }
-            };
-
-            WebhookJobParameters {
-                body: fetch_options.body.unwrap_or("".to_owned()),
-                headers: fetch_options.headers.unwrap_or_default(),
-                method: fetch_options.method.unwrap_or(HttpMethod::POST),
-                url: url
-                    .as_str()
-                    .ok_or_else(|| bad_request("url must be a string"))?
-                    .to_owned(),
-            }
+    // We deserialize a copy of the `asyncFunctionRequest` field here because we want to leave
+    // the original payload unmodified so that it can be passed through exactly as it came to us.
+    let async_function_request = payload
+        .get("asyncFunctionRequest")
+        .ok_or_else(|| bad_request("missing required field 'asyncFunctionRequest'"))?
+        .clone();
+    let async_function_request: HoghookAsyncFunctionRequest =
+        serde_json::from_value(async_function_request).map_err(|err| {
+            let msg = format!("unable to deserialize 'asyncFunctionRequest': {}", err);
+            bad_request(&msg)
+        })?;
+
+    if async_function_request.name != "fetch" {
+        return Err(bad_request("asyncFunctionRequest.name must be 'fetch'"));
+    }
+
+    // Note that the URL is parsed (and thus validated as a valid URL) as part of
+    // `get_hostname` below.
+    let url = async_function_request.args.0.clone();
+    let parameters = if let Some(fetch_options) = async_function_request.args.1 {
+        WebhookJobParameters {
+            body: fetch_options.body.unwrap_or("".to_owned()),
+            headers: fetch_options.headers.unwrap_or_default(),
+            method: fetch_options.method.unwrap_or(HttpMethod::POST),
+            url,
+        }
+    } else {
+        WebhookJobParameters {
+            body: "".to_owned(),
+            headers: HashMap::new(),
+            method: HttpMethod::POST,
+            url,
         }
-        _ => return Err(bad_request("expected JSON object")),
     };
 
     let url_hostname = get_hostname(&parameters.url)?;
@@ -491,7 +481,10 @@ mod tests {
                 .await
                 .unwrap();
 
-            assert_eq!(response.status(), StatusCode::BAD_REQUEST);
+            assert!(
+                response.status() == StatusCode::BAD_REQUEST
+                    || response.status() == StatusCode::UNPROCESSABLE_ENTITY
+            );
         }
     }
 }