From 0a68d4de792ef97a6733dc8cb4bb030ec8657f14 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Wed, 1 Apr 2026 02:11:30 -0400 Subject: [PATCH] refactor --- .../fal_client/src/webhook_api/mod.rs | 5 +- .../parse_webhook_inner_payload.rs | 80 ++++++------------- .../src/webhook_api/parse_webhook_payload.rs | 5 ++ .../fal_client/src/webhook_api/payload/mod.rs | 2 + .../payload/webhook_inner_payload.rs | 32 ++++++++ .../webhook_payload.rs} | 8 +- .../endpoints/webhooks/fal_webhook_handler.rs | 4 +- 7 files changed, 71 insertions(+), 65 deletions(-) create mode 100644 crates/api_clients/fal_client/src/webhook_api/parse_webhook_payload.rs create mode 100644 crates/api_clients/fal_client/src/webhook_api/payload/mod.rs create mode 100644 crates/api_clients/fal_client/src/webhook_api/payload/webhook_inner_payload.rs rename crates/api_clients/fal_client/src/webhook_api/{raw_webhook_payload.rs => payload/webhook_payload.rs} (88%) diff --git a/crates/api_clients/fal_client/src/webhook_api/mod.rs b/crates/api_clients/fal_client/src/webhook_api/mod.rs index 3e39cd1796..b456285068 100644 --- a/crates/api_clients/fal_client/src/webhook_api/mod.rs +++ b/crates/api_clients/fal_client/src/webhook_api/mod.rs @@ -1,3 +1,4 @@ pub mod parse_webhook_inner_payload; -pub mod raw_webhook_payload; -pub mod webhook_error_type; \ No newline at end of file +pub mod parse_webhook_payload; +pub mod webhook_error_type; +pub mod payload; \ No newline at end of file diff --git a/crates/api_clients/fal_client/src/webhook_api/parse_webhook_inner_payload.rs b/crates/api_clients/fal_client/src/webhook_api/parse_webhook_inner_payload.rs index 1120eb9b2b..abef113ddb 100644 --- a/crates/api_clients/fal_client/src/webhook_api/parse_webhook_inner_payload.rs +++ b/crates/api_clients/fal_client/src/webhook_api/parse_webhook_inner_payload.rs @@ -1,48 +1,20 @@ use serde_json::Value; -use crate::webhook_api::raw_webhook_payload::{RawWebhookPayload, WebhookStatus}; +use crate::webhook_api::payload::webhook_inner_payload::{WebhookInnerPayload, SuccessData, ErrorData, PayloadErrorData}; +use crate::webhook_api::payload::webhook_payload::{WebhookPayload, WebhookStatus}; use crate::webhook_api::webhook_error_type::WebhookErrorType; -/// The parsed inner payload of a FAL webhook. -#[derive(Debug)] -pub enum PayloadCase { - /// The webhook reported success and has a payload. - Success(SuccessData), - /// The webhook reported an error (status=ERROR) with optional detail info. - Error(ErrorData), - /// The webhook reported success but had no payload and instead had a payload_error. - PayloadError(PayloadErrorData), -} - -#[derive(Debug)] -pub struct SuccessData { - pub payload: Value, -} - -#[derive(Debug)] -pub struct ErrorData { - /// The first human-readable message from `payload.detail[].msg`, if any. - pub message: Option, - /// The first machine-readable error type from `payload.detail[].type`, if any. - pub error_type: Option, -} - -#[derive(Debug)] -pub struct PayloadErrorData { - pub payload_error: String, -} - /// Parse the inner payload of a FAL webhook into one of three cases. /// /// 1. If status is ERROR, parse out the first `detail` entry's `msg` and `type`. /// 2. If status is OK but there's no payload and there is a `payload_error`, return PayloadError. /// 3. Otherwise, return Success with the payload. -pub fn parse_webhook_inner_payload(webhook: &RawWebhookPayload) -> PayloadCase { +pub fn parse_webhook_inner_payload(webhook: &WebhookPayload) -> WebhookInnerPayload { match webhook.status { WebhookStatus::Error => { let (message, error_type) = extract_first_detail(&webhook.payload); - PayloadCase::Error(ErrorData { + WebhookInnerPayload::Error(ErrorData { message, error_type, }) @@ -51,13 +23,13 @@ pub fn parse_webhook_inner_payload(webhook: &RawWebhookPayload) -> PayloadCase { // Check for payload_error case: status=OK but no payload, has payload_error. if webhook.payload.is_none() { if let Some(ref payload_error) = webhook.payload_error { - return PayloadCase::PayloadError(PayloadErrorData { + return WebhookInnerPayload::PayloadError(PayloadErrorData { payload_error: payload_error.clone(), }); } } - PayloadCase::Success(SuccessData { + WebhookInnerPayload::Success(SuccessData { payload: webhook.payload.clone().unwrap_or(Value::Null), }) } @@ -95,7 +67,7 @@ fn extract_first_detail(payload: &Option) -> (Option, Option RawWebhookPayload { + fn load_test_webhook(filename: &str) -> WebhookPayload { let path = format!("test_data/webhooks/{}", filename); let json = std::fs::read_to_string(&path) .unwrap_or_else(|e| panic!("Failed to read {}: {}", path, e)); @@ -109,7 +81,7 @@ mod tests { let result = parse_webhook_inner_payload(&webhook); match result { - PayloadCase::Error(data) => { + WebhookInnerPayload::Error(data) => { assert_eq!( data.error_type, Some(WebhookErrorType::ContentPolicyViolation), @@ -119,7 +91,7 @@ mod tests { Some("The content could not be processed because it contained material flagged by a content checker."), ); } - other => panic!("Expected PayloadCase::Error, got {:?}", other), + other => panic!("Expected WebhookInnerPayload::Error, got {:?}", other), } } @@ -129,7 +101,7 @@ mod tests { let result = parse_webhook_inner_payload(&webhook); match result { - PayloadCase::Error(data) => { + WebhookInnerPayload::Error(data) => { assert_eq!( data.error_type, Some(WebhookErrorType::Unknown("invalid_api_key".to_string())), @@ -139,7 +111,7 @@ mod tests { Some("Invalid API key"), ); } - other => panic!("Expected PayloadCase::Error, got {:?}", other), + other => panic!("Expected WebhookInnerPayload::Error, got {:?}", other), } } @@ -149,7 +121,7 @@ mod tests { let result = parse_webhook_inner_payload(&webhook); match result { - PayloadCase::Error(data) => { + WebhookInnerPayload::Error(data) => { assert_eq!( data.error_type, Some(WebhookErrorType::FileTooLarge), @@ -159,7 +131,7 @@ mod tests { Some("File size exceeds the maximum allowed size of 10485760 bytes. Please upload a smaller file."), ); } - other => panic!("Expected PayloadCase::Error, got {:?}", other), + other => panic!("Expected WebhookInnerPayload::Error, got {:?}", other), } } @@ -169,7 +141,7 @@ mod tests { let result = parse_webhook_inner_payload(&webhook); match result { - PayloadCase::Error(data) => { + WebhookInnerPayload::Error(data) => { assert_eq!( data.error_type, Some(WebhookErrorType::ContentPolicyViolation), @@ -179,7 +151,7 @@ mod tests { Some("The content could not be processed because it contained material flagged by a content checker."), ); } - other => panic!("Expected PayloadCase::Error, got {:?}", other), + other => panic!("Expected WebhookInnerPayload::Error, got {:?}", other), } } @@ -189,7 +161,7 @@ mod tests { let result = parse_webhook_inner_payload(&webhook); match result { - PayloadCase::Error(data) => { + WebhookInnerPayload::Error(data) => { assert_eq!( data.error_type, Some(WebhookErrorType::NoMediaGenerated), @@ -198,13 +170,13 @@ mod tests { data.message.as_deref().unwrap().starts_with("The model did not generate"), ); } - other => panic!("Expected PayloadCase::Error, got {:?}", other), + other => panic!("Expected WebhookInnerPayload::Error, got {:?}", other), } } #[test] fn payload_error_case() { - let webhook = RawWebhookPayload { + let webhook = WebhookPayload { request_id: "test-123".to_string(), gateway_request_id: "test-123".to_string(), status: WebhookStatus::Ok, @@ -216,16 +188,16 @@ mod tests { let result = parse_webhook_inner_payload(&webhook); match result { - PayloadCase::PayloadError(data) => { + WebhookInnerPayload::PayloadError(data) => { assert_eq!(data.payload_error, "encoding error occurred"); } - other => panic!("Expected PayloadCase::PayloadError, got {:?}", other), + other => panic!("Expected WebhookInnerPayload::PayloadError, got {:?}", other), } } #[test] fn success_case() { - let webhook = RawWebhookPayload { + let webhook = WebhookPayload { request_id: "test-456".to_string(), gateway_request_id: "test-456".to_string(), status: WebhookStatus::Ok, @@ -237,16 +209,16 @@ mod tests { let result = parse_webhook_inner_payload(&webhook); match result { - PayloadCase::Success(data) => { + WebhookInnerPayload::Success(data) => { assert!(data.payload.get("images").is_some()); } - other => panic!("Expected PayloadCase::Success, got {:?}", other), + other => panic!("Expected WebhookInnerPayload::Success, got {:?}", other), } } #[test] fn error_with_no_payload() { - let webhook = RawWebhookPayload { + let webhook = WebhookPayload { request_id: "test-789".to_string(), gateway_request_id: "test-789".to_string(), status: WebhookStatus::Error, @@ -258,11 +230,11 @@ mod tests { let result = parse_webhook_inner_payload(&webhook); match result { - PayloadCase::Error(data) => { + WebhookInnerPayload::Error(data) => { assert_eq!(data.message, None); assert_eq!(data.error_type, None); } - other => panic!("Expected PayloadCase::Error, got {:?}", other), + other => panic!("Expected WebhookInnerPayload::Error, got {:?}", other), } } } diff --git a/crates/api_clients/fal_client/src/webhook_api/parse_webhook_payload.rs b/crates/api_clients/fal_client/src/webhook_api/parse_webhook_payload.rs new file mode 100644 index 0000000000..38c270fc0c --- /dev/null +++ b/crates/api_clients/fal_client/src/webhook_api/parse_webhook_payload.rs @@ -0,0 +1,5 @@ +use crate::webhook_api::payload::webhook_payload::WebhookPayload; + +pub fn parse_webhook_payload(json: &str) -> Result { + serde_json::from_str(json) +} diff --git a/crates/api_clients/fal_client/src/webhook_api/payload/mod.rs b/crates/api_clients/fal_client/src/webhook_api/payload/mod.rs new file mode 100644 index 0000000000..a118d21b55 --- /dev/null +++ b/crates/api_clients/fal_client/src/webhook_api/payload/mod.rs @@ -0,0 +1,2 @@ +pub mod webhook_inner_payload; +pub mod webhook_payload; \ No newline at end of file diff --git a/crates/api_clients/fal_client/src/webhook_api/payload/webhook_inner_payload.rs b/crates/api_clients/fal_client/src/webhook_api/payload/webhook_inner_payload.rs new file mode 100644 index 0000000000..ddbbcdc8ce --- /dev/null +++ b/crates/api_clients/fal_client/src/webhook_api/payload/webhook_inner_payload.rs @@ -0,0 +1,32 @@ +use serde_json::Value; + +use crate::webhook_api::webhook_error_type::WebhookErrorType; + +/// The parsed inner payload of a FAL webhook. +#[derive(Debug)] +pub enum WebhookInnerPayload { + /// The webhook reported success and has a payload. + Success(SuccessData), + /// The webhook reported an error (status=ERROR) with optional detail info. + Error(ErrorData), + /// The webhook reported success but had no payload and instead had a payload_error. + PayloadError(PayloadErrorData), +} + +#[derive(Debug)] +pub struct SuccessData { + pub payload: Value, +} + +#[derive(Debug)] +pub struct ErrorData { + /// The first human-readable message from `payload.detail[].msg`, if any. + pub message: Option, + /// The first machine-readable error type from `payload.detail[].type`, if any. + pub error_type: Option, +} + +#[derive(Debug)] +pub struct PayloadErrorData { + pub payload_error: String, +} diff --git a/crates/api_clients/fal_client/src/webhook_api/raw_webhook_payload.rs b/crates/api_clients/fal_client/src/webhook_api/payload/webhook_payload.rs similarity index 88% rename from crates/api_clients/fal_client/src/webhook_api/raw_webhook_payload.rs rename to crates/api_clients/fal_client/src/webhook_api/payload/webhook_payload.rs index dd57f40cc1..961003c5d4 100644 --- a/crates/api_clients/fal_client/src/webhook_api/raw_webhook_payload.rs +++ b/crates/api_clients/fal_client/src/webhook_api/payload/webhook_payload.rs @@ -3,7 +3,7 @@ use serde_json::Value; /// The value FAL sends to webhook endpoints. #[derive(Deserialize, Debug, Clone)] -pub struct RawWebhookPayload { +pub struct WebhookPayload { /// Value originally used in the queue API pub request_id: String, @@ -41,9 +41,3 @@ pub enum WebhookStatus { #[serde(alias = "ERROR")] Error } - -impl RawWebhookPayload { - pub fn try_from_json(json: &str) -> Result { - serde_json::from_str(json) - } -} diff --git a/crates/service/web/storyteller_web/src/http_server/endpoints/webhooks/fal_webhook_handler.rs b/crates/service/web/storyteller_web/src/http_server/endpoints/webhooks/fal_webhook_handler.rs index 85c078189f..49cc52f7f3 100644 --- a/crates/service/web/storyteller_web/src/http_server/endpoints/webhooks/fal_webhook_handler.rs +++ b/crates/service/web/storyteller_web/src/http_server/endpoints/webhooks/fal_webhook_handler.rs @@ -10,7 +10,7 @@ use actix_web::web::Bytes; use actix_web::web::Json; use actix_web::{web, HttpRequest, HttpResponse}; use anyhow::Error; -use fal_client::webhook_api::raw_webhook_payload::{RawWebhookPayload, WebhookStatus}; +use fal_client::webhook_api::payload::webhook_payload::{WebhookPayload, WebhookStatus}; use http_server_common::response::response_success_helpers::SimpleGenericJsonSuccess; use http_server_common::response::serialize_as_json_error::serialize_as_json_error; use log::{error, info, warn}; @@ -112,7 +112,7 @@ pub async fn fal_webhook_handler( info!("Received FAL webhook body: {}", webhook_payload); - let webhook_payload = RawWebhookPayload::try_from_json(&webhook_payload) + let webhook_payload = WebhookPayload::try_from_json(&webhook_payload) .map_err(|err| { error!("Fal Webhook: could not parse webhook payload as JSON: {:?}", err); FalWebhookError::BadInput("Could not parse webhook payload as JSON".to_string())