From b5bc532b809241ea07df2b51ba7827f076aa0cb2 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 7 Sep 2025 22:32:43 -0400 Subject: [PATCH] refactor --- ...g_action.rs => artcraft_billing_action.rs} | 5 +- ...ing_event.rs => enriched_webhook_event.rs} | 8 +-- .../src/endpoints/webhook/common/mod.rs | 6 +-- ...ummary.rs => webhook_event_log_summary.rs} | 0 .../stripe_artcraft_webhook_handler.rs | 49 ++++++++++--------- ....rs => handle_webhook_event_enrichment.rs} | 27 ++++------ .../webhook/webhook_event_handlers/mod.rs | 2 +- .../payment_intent_succeeded_handler_2.rs | 48 ++++++++---------- ...tionally_fulfill_artcraft_billing_event.rs | 4 +- .../utils/verify_stripe_webhook_ip_address.rs | 9 ++-- 10 files changed, 73 insertions(+), 85 deletions(-) rename crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/{billing_action.rs => artcraft_billing_action.rs} (91%) rename crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/{artcraft_billing_event.rs => enriched_webhook_event.rs} (57%) rename crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/{event_log_summary.rs => webhook_event_log_summary.rs} (100%) rename crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/{webhook_event_to_artcraft_event.rs => handle_webhook_event_enrichment.rs} (92%) diff --git a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/billing_action.rs b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/artcraft_billing_action.rs similarity index 91% rename from crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/billing_action.rs rename to crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/artcraft_billing_action.rs index 86b40acc21..c83048b511 100644 --- a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/billing_action.rs +++ b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/artcraft_billing_action.rs @@ -4,10 +4,7 @@ use tokens::tokens::wallets::WalletToken; pub enum BillingAction { /// An ignorable event. - IgnorableEvent{ - event_type: IgnoreableEventType, - description: String - }, + IgnorableEvent, /// A user purchased wallet credits. WalletCreditsPurchase(WalletCreditsPurchaseEvent), diff --git a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/artcraft_billing_event.rs b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/enriched_webhook_event.rs similarity index 57% rename from crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/artcraft_billing_event.rs rename to crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/enriched_webhook_event.rs index 3615c19f23..c3085b2fe5 100644 --- a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/artcraft_billing_event.rs +++ b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/enriched_webhook_event.rs @@ -1,10 +1,10 @@ -use crate::endpoints::webhook::common::billing_action::BillingAction; -use crate::endpoints::webhook::common::event_log_summary::WebhookEventLogSummary; +use crate::endpoints::webhook::common::artcraft_billing_action::BillingAction; +use crate::endpoints::webhook::common::webhook_event_log_summary::WebhookEventLogSummary; -pub struct ArtcraftBillingEvent { +pub struct EnrichedWebhookEvent { /// Higher-level events that we care about handling. /// Not all webhook requests have them. - pub action: BillingAction, + pub maybe_billing_action: Option, /// Details of what we'll write to the webhook event logs. /// In addition to important events, this includes some no-op events, diff --git a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/mod.rs b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/mod.rs index adf000d2eb..b930f8e269 100644 --- a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/mod.rs +++ b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/mod.rs @@ -1,3 +1,3 @@ -pub mod event_log_summary; -pub mod artcraft_billing_event; -pub mod billing_action; \ No newline at end of file +pub mod webhook_event_log_summary; +pub mod enriched_webhook_event; +pub mod artcraft_billing_action; \ No newline at end of file diff --git a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/event_log_summary.rs b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/webhook_event_log_summary.rs similarity index 100% rename from crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/event_log_summary.rs rename to crates/service/plugins/billing_artcraft/src/endpoints/webhook/common/webhook_event_log_summary.rs diff --git a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/stripe_artcraft_webhook_handler.rs b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/stripe_artcraft_webhook_handler.rs index 3b86d41d4a..a5f45f3040 100644 --- a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/stripe_artcraft_webhook_handler.rs +++ b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/stripe_artcraft_webhook_handler.rs @@ -1,17 +1,17 @@ use actix_web::web::{Bytes, Data, Json}; use actix_web::{web, HttpRequest, HttpResponse}; -use crate::endpoints::webhook::common::artcraft_billing_event::ArtcraftBillingEvent; +use crate::endpoints::webhook::common::enriched_webhook_event::EnrichedWebhookEvent; use crate::endpoints::webhook::webhook_event_handlers::checkout_session::checkout_session_completed_handler::checkout_session_completed_handler; use crate::endpoints::webhook::webhook_event_handlers::customer_subscription::customer_subscription_created_handler::customer_subscription_created_handler; use crate::endpoints::webhook::webhook_event_handlers::customer_subscription::customer_subscription_updated_handler::customer_subscription_updated_handler; +use crate::endpoints::webhook::webhook_event_handlers::handle_webhook_event_enrichment::handle_webhook_event_enrichment; use crate::endpoints::webhook::webhook_event_handlers::handle_webhook_payload::handle_webhook_payload; use crate::endpoints::webhook::webhook_event_handlers::invoice::invoice_paid_handler::invoice_paid_handler; use crate::endpoints::webhook::webhook_event_handlers::invoice::invoice_payment_failed::invoice_payment_failed_handler; use crate::endpoints::webhook::webhook_event_handlers::payment_intent::payment_intent_succeeded_handler::payment_intent_succeeded_handler; use crate::endpoints::webhook::webhook_event_handlers::stripe_artcraft_webhook_error::StripeArtcraftWebhookError; use crate::endpoints::webhook::webhook_event_handlers::stripe_artcraft_webhook_summary::StripeArtcraftWebhookSummary; -use crate::endpoints::webhook::webhook_event_handlers::webhook_event_to_artcraft_event::webhook_event_to_artcraft_event; use crate::fulfillment::transactionally_fulfill_artcraft_billing_event::transactionally_fulfill_artcraft_billing_action; use crate::utils::artcraft_stripe_config::ArtcraftStripeConfigWithClient; use crate::utils::verify_stripe_webhook_ip_address::verify_stripe_webhook_ip_address; @@ -61,11 +61,11 @@ pub async fn stripe_artcraft_webhook_handler( mysql_pool: Data, ) -> Result, StripeArtcraftWebhookError> { - verify_stripe_webhook_ip_address(&http_request) + verify_stripe_webhook_ip_address(&http_request, **server_environment) .map_err(|e| { let reason = format!("Improper client IP address. Error: {:?}", e); error!("{}", &reason); - StripeArtcraftWebhookError::BadRequest("bad request".to_string()) + StripeArtcraftWebhookError::BadRequest(reason) })?; let secret_signing_key = &stripe_config.secret_webhook_signing_key; @@ -91,13 +91,9 @@ pub async fn stripe_artcraft_webhook_handler( // Events can be re-sent, so we need to make handling them idempotent. let stripe_event_id = webhook_payload.id.to_string(); - - let stripe_event_created_at = NaiveDateTime::from_timestamp(webhook_payload.created, 0); - let stripe_event_type = webhook_payload.type_.as_str().to_string(); - - // NB: Whether this was test data or live data - let stripe_is_production = webhook_payload.livemode; + let stripe_event_created_at = NaiveDateTime::from_timestamp(webhook_payload.created, 0); + let stripe_is_production = webhook_payload.livemode; // NB: Whether this was 'test' data or live data info!("Stripe webhook event type: {} ({:?}); is production: {}, created at: {}, pending events to be handled: {}", &stripe_event_type, @@ -121,12 +117,12 @@ pub async fn stripe_artcraft_webhook_handler( StripeArtcraftWebhookError::ServerError("database error".to_string()) })?; - // TODO: Not sure this retry / idempotency logic is correct + // TODO: This might be extraneous. Inserting logs is transactionally unique and can only happen once. if let Some(event) = maybe_previously_played_event { // The event is being replayed by Stripe, and we've already handled it. // We'll ignore it so that we remain idempotent. if event.should_ignore_retry { - warn!("Stripe is replying event with ID={}. Ignoring it since we have previously handled it.", + warn!("Stripe is replaying event ID={} ; ignoring it since we have already previously handled it.", &stripe_event_id); return Ok(Json(StripeArtcraftWebhookSuccessResponse { @@ -135,7 +131,7 @@ pub async fn stripe_artcraft_webhook_handler( } } - let artcraft_event = webhook_event_to_artcraft_event( + let enriched_event = handle_webhook_event_enrichment( &stripe_config.client, **server_environment, webhook_payload, @@ -144,10 +140,10 @@ pub async fn stripe_artcraft_webhook_handler( let mut transaction = mysql_connection.begin().await?; - let result = process_event( + let result = process_enriched_event( **server_environment, - &stripe_config, - artcraft_event, + &stripe_config, + enriched_event, stripe_event_id.clone(), stripe_event_created_at, stripe_event_type.clone(), @@ -161,7 +157,9 @@ pub async fn stripe_artcraft_webhook_handler( }, Err(err) => { error!("Error handling Stripe webhook event `{}` ({}): {:?}", - stripe_event_type,stripe_event_id, err); + stripe_event_type, + stripe_event_id, + err); transaction.rollback().await?; @@ -174,10 +172,10 @@ pub async fn stripe_artcraft_webhook_handler( })) } -async fn process_event( +async fn process_enriched_event( server_environment: ServerEnvironment, stripe_config: &ArtcraftStripeConfigWithClient, - artcraft_event: ArtcraftBillingEvent, + artcraft_event: EnrichedWebhookEvent, stripe_event_id: String, stripe_event_created_at: NaiveDateTime, stripe_event_type: String, @@ -185,11 +183,14 @@ async fn process_event( transaction: &mut Transaction<'_, MySql> ) -> Result<(), StripeArtcraftWebhookError> { - // TODO: Maybe grab the primary key of the impacted entity? - transactionally_fulfill_artcraft_billing_action( - &artcraft_event.action, - transaction, - ).await?; + if let Some(billing_action) = &artcraft_event.maybe_billing_action { + // This is where we fulfill the purchase, subscription, non-payment, etc.! + // TODO: Maybe grab the primary key of the impacted entity? + transactionally_fulfill_artcraft_billing_action( + billing_action, + transaction, + ).await?; + } // NB: These records are uniquely keyed by ID, so this only happens once. let query = InsertStripeWebhookEventLog { diff --git a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/webhook_event_to_artcraft_event.rs b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/handle_webhook_event_enrichment.rs similarity index 92% rename from crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/webhook_event_to_artcraft_event.rs rename to crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/handle_webhook_event_enrichment.rs index 67d9f7041a..93a0aaef93 100644 --- a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/webhook_event_to_artcraft_event.rs +++ b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/handle_webhook_event_enrichment.rs @@ -1,7 +1,7 @@ -use crate::endpoints::webhook::common::artcraft_billing_event::ArtcraftBillingEvent; -use crate::endpoints::webhook::common::billing_action::BillingAction::IgnorableEvent; -use crate::endpoints::webhook::common::billing_action::{BillingAction, IgnoreableEventType}; -use crate::endpoints::webhook::common::event_log_summary::WebhookEventLogSummary; +use crate::endpoints::webhook::common::artcraft_billing_action::BillingAction::IgnorableEvent; +use crate::endpoints::webhook::common::artcraft_billing_action::{BillingAction, IgnoreableEventType}; +use crate::endpoints::webhook::common::enriched_webhook_event::EnrichedWebhookEvent; +use crate::endpoints::webhook::common::webhook_event_log_summary::WebhookEventLogSummary; use crate::endpoints::webhook::webhook_event_handlers::checkout_session::checkout_session_completed_handler::checkout_session_completed_handler; use crate::endpoints::webhook::webhook_event_handlers::customer_subscription::customer_subscription_created_handler::customer_subscription_created_handler; use crate::endpoints::webhook::webhook_event_handlers::customer_subscription::customer_subscription_deleted_handler::customer_subscription_deleted_handler; @@ -20,21 +20,17 @@ use sqlx::{MySql, Transaction}; use stripe::Client; use stripe_webhook::{Event, EventObject}; -pub async fn webhook_event_to_artcraft_event( +pub async fn handle_webhook_event_enrichment( stripe_client: &Client, server_environment: ServerEnvironment, webhook_payload: Event, stripe_event_type: &str, -) -> Result { +) -> Result { // TODO: Cleanup - if let Some(_summary) = ignore_known_unwanted_events(&webhook_payload) { - return Ok(ArtcraftBillingEvent { - action: BillingAction::IgnorableEvent { - event_type: IgnoreableEventType::Other, - description: "TODO".to_string(), - - }, + return Ok(EnrichedWebhookEvent { + maybe_billing_action: None, webhook_event_log_summary: WebhookEventLogSummary { maybe_user_token: None, maybe_event_entity_id: None, @@ -217,11 +213,8 @@ pub async fn webhook_event_to_artcraft_event( &webhook_payload.type_); } - Ok(ArtcraftBillingEvent { - action: IgnorableEvent { - event_type: IgnoreableEventType::Other, - description: "todo".to_string(), - }, + Ok(EnrichedWebhookEvent { + maybe_billing_action: None, webhook_event_log_summary: webhook_summary, }) } diff --git a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/mod.rs b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/mod.rs index 01444bbac8..fb7faf4f13 100644 --- a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/mod.rs +++ b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/mod.rs @@ -1,6 +1,6 @@ mod ignore_known_unwanted_events; -pub (crate) mod webhook_event_to_artcraft_event; +pub (crate) mod handle_webhook_event_enrichment; pub mod handle_webhook_payload; pub mod stripe_artcraft_webhook_error; pub(crate) mod checkout_session; diff --git a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/payment_intent/payment_intent_succeeded_handler_2.rs b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/payment_intent/payment_intent_succeeded_handler_2.rs index de1a45c05c..5af3a1540c 100644 --- a/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/payment_intent/payment_intent_succeeded_handler_2.rs +++ b/crates/service/plugins/billing_artcraft/src/endpoints/webhook/webhook_event_handlers/payment_intent/payment_intent_succeeded_handler_2.rs @@ -1,9 +1,9 @@ use crate::configs::get_artcraft_product_by_stripe_id_and_env::get_artcraft_product_by_stripe_id_and_env; use crate::configs::stripe_artcraft_generic_product_info::StripeArtcraftGenericProductInfo; use crate::configs::subscriptions::stripe_artcraft_subscription_info::StripeArtcraftSubscriptionInfo; -use crate::endpoints::webhook::common::artcraft_billing_event::ArtcraftBillingEvent; -use crate::endpoints::webhook::common::billing_action::{BillingAction, IgnoreableEventType, WalletCreditsPurchaseEvent}; -use crate::endpoints::webhook::common::event_log_summary::WebhookEventLogSummary; +use crate::endpoints::webhook::common::artcraft_billing_action::{BillingAction, IgnoreableEventType, WalletCreditsPurchaseEvent}; +use crate::endpoints::webhook::common::enriched_webhook_event::EnrichedWebhookEvent; +use crate::endpoints::webhook::common::webhook_event_log_summary::WebhookEventLogSummary; use crate::endpoints::webhook::webhook_event_handlers::stripe_artcraft_webhook_error::StripeArtcraftWebhookError; use crate::endpoints::webhook::webhook_event_handlers::stripe_artcraft_webhook_summary::StripeArtcraftWebhookSummary; use crate::fulfillment::credits_pack::complete_credits_pack_purchase::complete_credits_pack_purchase; @@ -25,7 +25,7 @@ pub async fn payment_intent_succeeded_handler_2( payment_intent: &PaymentIntent, server_environment: ServerEnvironment, stripe_client: &Client, -) -> Result { +) -> Result { let payment_intent_id = payment_intent.id.to_string(); @@ -47,7 +47,7 @@ pub async fn payment_intent_succeeded_handler_2( return Err(StripeArtcraftWebhookError::BadRequest("no user token in payment intent".to_string())); } }; - + let mut event_log_summary = WebhookEventLogSummary { maybe_stripe_customer_id, maybe_user_token, @@ -63,15 +63,12 @@ pub async fn payment_intent_succeeded_handler_2( stripe_shared::PaymentIntentStatus::Succeeded => true, _ => false, }; - + if !payment_succeeded { event_log_summary.should_ignore_retry = true; - - return Ok(ArtcraftBillingEvent { - action: BillingAction::IgnorableEvent { - description: "payment_intent not succeeded".to_string(), - event_type: IgnoreableEventType::PaymentIntentFailed, - }, + // description: "payment_intent not succeeded".to_string(), + return Ok(EnrichedWebhookEvent { + maybe_billing_action: None, webhook_event_log_summary: event_log_summary, }); } @@ -84,16 +81,15 @@ pub async fn payment_intent_succeeded_handler_2( error!("Error looking up purchase from payment intent {}: {:?}", &payment_intent_id, err); StripeArtcraftWebhookError::ServerError("error looking up purchase".to_string()) })?; - + if !purchase.has_invoice { // Subscription purchase. Let `invoice.paid` event handle this instead. event_log_summary.should_ignore_retry = true; - - return Ok(ArtcraftBillingEvent { - action: BillingAction::IgnorableEvent { - event_type: IgnoreableEventType::PaymentIntentForSubscription, - description: "payment_intent for subscription; will handle via invoice.paid".to_string(), - }, + + // description: "payment_intent for subscription; will handle via invoice.paid".to_string(), + + return Ok(EnrichedWebhookEvent { + maybe_billing_action: None, webhook_event_log_summary: event_log_summary, }); } @@ -107,24 +103,22 @@ pub async fn payment_intent_succeeded_handler_2( info!("Do not handle subscriptions as one-off payments: {}", purchase.product_id); event_log_summary.should_ignore_retry = true; - return Ok(ArtcraftBillingEvent { - action: BillingAction::IgnorableEvent { - event_type: IgnoreableEventType::PaymentIntentForSubscription, - description: "payment_intent for subscription; will handle via invoice.paid".to_string(), - }, + //description: "payment_intent for subscription; will handle via invoice.paid".to_string(), + return Ok(EnrichedWebhookEvent { + maybe_billing_action: None, webhook_event_log_summary: event_log_summary, }); } Some(StripeArtcraftGenericProductInfo::CreditsPack(credits_pack)) => credits_pack, }; - Ok(ArtcraftBillingEvent { - action: BillingAction::WalletCreditsPurchase(WalletCreditsPurchaseEvent { + Ok(EnrichedWebhookEvent { + maybe_billing_action: Some(BillingAction::WalletCreditsPurchase(WalletCreditsPurchaseEvent { owner_user_token: user_token, maybe_wallet_token: None, // TODO: Use the checkout session to set this ! pack: credits_pack.clone(), quantity: purchase.quantity, - }), + })), webhook_event_log_summary: event_log_summary, }) } diff --git a/crates/service/plugins/billing_artcraft/src/fulfillment/transactionally_fulfill_artcraft_billing_event.rs b/crates/service/plugins/billing_artcraft/src/fulfillment/transactionally_fulfill_artcraft_billing_event.rs index 62838a9a2f..aa5a10c4b8 100644 --- a/crates/service/plugins/billing_artcraft/src/fulfillment/transactionally_fulfill_artcraft_billing_event.rs +++ b/crates/service/plugins/billing_artcraft/src/fulfillment/transactionally_fulfill_artcraft_billing_event.rs @@ -1,4 +1,4 @@ -use crate::endpoints::webhook::common::billing_action::BillingAction; +use crate::endpoints::webhook::common::artcraft_billing_action::BillingAction; use crate::fulfillment::credits_pack::complete_credits_pack_purchase::complete_credits_pack_purchase; use anyhow::anyhow; use errors::AnyhowResult; @@ -11,7 +11,7 @@ pub async fn transactionally_fulfill_artcraft_billing_action( ) -> AnyhowResult<()> { match event { - BillingAction::IgnorableEvent { .. } => { + BillingAction::IgnorableEvent => { warn!("Received ignorable billing action; nothing to fulfill."); return Ok(()) } diff --git a/crates/service/plugins/billing_artcraft/src/utils/verify_stripe_webhook_ip_address.rs b/crates/service/plugins/billing_artcraft/src/utils/verify_stripe_webhook_ip_address.rs index 02d814f281..f1a4adc3e8 100644 --- a/crates/service/plugins/billing_artcraft/src/utils/verify_stripe_webhook_ip_address.rs +++ b/crates/service/plugins/billing_artcraft/src/utils/verify_stripe_webhook_ip_address.rs @@ -6,6 +6,7 @@ use errors::AnyhowResult; use once_cell::sync::Lazy; use http_server_common::request::get_request_ip::get_request_ip; +use reusable_types::server_environment::ServerEnvironment; /// List of IP addresses that send webhook requests /// From: https://stripe.com/docs/ips @@ -32,15 +33,17 @@ static STRIPE_WEBHOOK_IP_ADDRESSES : Lazy> = Lazy::new(|| { /// Verify that the request comes from a Stripe webhook client IP /// Recommendation from: https://stripe.com/docs/webhooks/best-practices -pub fn verify_stripe_webhook_ip_address(http_request: &HttpRequest) -> AnyhowResult<()> { +pub fn verify_stripe_webhook_ip_address(http_request: &HttpRequest, server_environment: ServerEnvironment) -> AnyhowResult<()> { let ip_address = get_request_ip(http_request); if STRIPE_WEBHOOK_IP_ADDRESSES.contains(&ip_address) { return Ok(()); } - // TODO: Only allow this in development - if ip_address == "127.0.0.1" { + let is_development = server_environment == ServerEnvironment::Development; + let is_localhost = ip_address == "127.0.0.1"; + + if is_development && is_localhost { return Ok(()); }