fix: resolve TODO/FIXME from router.routes (#283)

crates/router/src/core/payments/operations/payment_capture.rs

@@ -115,7 +115,6 @@ impl<F: Send + Clone> GetTracker<F, payments::PaymentData<F>, api::PaymentsCaptu
         )
         .await?;
 
-        // TODO: get payment method data for response
         Ok((
             Box::new(self),
             payments::PaymentData {

crates/router/src/routes/metrics.rs

@@ -1,5 +1,3 @@
-// TODO: Move this to router_env https://juspay.atlassian.net/browse/ORCA-345
-
 use once_cell::sync::Lazy;
 use router_env::opentelemetry::{
     global,

crates/router/src/routes/payments.rs

@@ -1,38 +1,26 @@
 use std::borrow::Cow;
 
-use actix_web::{
-    body::{BoxBody, MessageBody},
-    web, HttpRequest, HttpResponse, Responder,
-};
+use actix_web::{web, Responder};
 use error_stack::report;
 use router_env::{
     tracing::{self, instrument},
     Flow,
 };
 
-use super::app::AppState;
 use crate::{
+    self as app,
     core::{errors::http_not_implemented, payments},
     services::api,
-    types::api::{
-        self as api_types, enums as api_enums,
-        payments::{
-            PaymentIdType, PaymentListConstraints, PaymentsCancelRequest, PaymentsCaptureRequest,
-            PaymentsRequest, PaymentsResponse, PaymentsRetrieveRequest,
-        },
-        Authorize, Capture, PSync, PaymentRetrieveBody, PaymentsSessionRequest,
-        PaymentsSessionResponse, PaymentsStartRequest, Session, Verify, Void,
-    },
-    //FIXME: remove specific imports
+    types::api::{self as api_types, enums as api_enums, payments as payment_types},
 };
 
 #[instrument(skip_all, fields(flow = ?Flow::PaymentsCreate))]
 // #[post("")]
 pub async fn payments_create(
-    state: web::Data<AppState>,
-    req: HttpRequest,
-    json_payload: web::Json<PaymentsRequest>,
-) -> HttpResponse {
+    state: web::Data<app::AppState>,
+    req: actix_web::HttpRequest,
+    json_payload: web::Json<payment_types::PaymentsRequest>,
+) -> impl Responder {
     let payload = json_payload.into_inner();
 
     if let Some(api_enums::CaptureMethod::Scheduled) = payload.capture_method {
@@ -44,37 +32,13 @@ pub async fn payments_create(
         &req,
         payload,
         |state, merchant_account, req| {
-            // TODO: Change for making it possible for the flow to be inferred internally or through validation layer
-            async {
-                let connector = req.connector;
-                match req.amount.as_ref() {
-                    Some(api_types::Amount::Value(_)) | None => {
-                        payments::payments_core::<Authorize, PaymentsResponse, _, _, _>(
-                            state,
-                            merchant_account,
-                            payments::PaymentCreate,
-                            req,
-                            api::AuthFlow::Merchant,
-                            connector,
-                            payments::CallConnectorAction::Trigger,
-                        )
-                        .await
-                    }
-
-                    Some(api_types::Amount::Zero) => {
-                        payments::payments_core::<Verify, PaymentsResponse, _, _, _>(
-                            state,
-                            merchant_account,
-                            payments::PaymentCreate,
-                            req,
-                            api::AuthFlow::Merchant,
-                            connector,
-                            payments::CallConnectorAction::Trigger,
-                        )
-                        .await
-                    }
-                }
-            }
+            authorize_verify_select(
+                payments::PaymentCreate,
+                state,
+                merchant_account,
+                req,
+                api::AuthFlow::Merchant,
+            )
         },
         api::MerchantAuthentication::ApiKey,
     )
@@ -83,12 +47,12 @@ pub async fn payments_create(
 
 #[instrument(skip(state), fields(flow = ?Flow::PaymentsStart))]
 pub async fn payments_start(
-    state: web::Data<AppState>,
-    req: HttpRequest,
+    state: web::Data<app::AppState>,
+    req: actix_web::HttpRequest,
     path: web::Path<(String, String, String)>,
-) -> HttpResponse {
+) -> impl Responder {
     let (payment_id, merchant_id, attempt_id) = path.into_inner();
-    let payload = PaymentsStartRequest {
+    let payload = payment_types::PaymentsStartRequest {
         payment_id: payment_id.clone(),
         merchant_id: merchant_id.clone(),
         txn_id: attempt_id.clone(),
@@ -98,7 +62,7 @@ pub async fn payments_start(
         &req,
         payload,
         |state, merchant_account, req| {
-            payments::payments_core::<Authorize, PaymentsResponse, _, _, _>(
+            payments::payments_core::<api_types::Authorize, payment_types::PaymentsResponse, _, _, _>(
                 state,
                 merchant_account,
                 payments::operations::PaymentStart,
@@ -116,13 +80,13 @@ pub async fn payments_start(
 #[instrument(skip(state), fields(flow = ?Flow::PaymentsRetrieve))]
 // #[get("/{payment_id}")]
 pub async fn payments_retrieve(
-    state: web::Data<AppState>,
-    req: HttpRequest,
+    state: web::Data<app::AppState>,
+    req: actix_web::HttpRequest,
     path: web::Path<String>,
-    json_payload: web::Query<PaymentRetrieveBody>,
-) -> HttpResponse {
-    let payload = PaymentsRetrieveRequest {
-        resource_id: PaymentIdType::PaymentIntentId(path.to_string()),
+    json_payload: web::Query<payment_types::PaymentRetrieveBody>,
+) -> impl Responder {
+    let payload = payment_types::PaymentsRetrieveRequest {
+        resource_id: payment_types::PaymentIdType::PaymentIntentId(path.to_string()),
         merchant_id: json_payload.merchant_id.clone(),
         force_sync: json_payload.force_sync.unwrap_or(false),
         param: None,
@@ -139,7 +103,7 @@ pub async fn payments_retrieve(
         &req,
         payload,
         |state, merchant_account, req| {
-            payments::payments_core::<PSync, PaymentsResponse, _, _, _>(
+            payments::payments_core::<api_types::PSync, payment_types::PaymentsResponse, _, _, _>(
                 state,
                 merchant_account,
                 payments::PaymentStatus,
@@ -157,11 +121,11 @@ pub async fn payments_retrieve(
 #[instrument(skip_all, fields(flow = ?Flow::PaymentsUpdate))]
 // #[post("/{payment_id}")]
 pub async fn payments_update(
-    state: web::Data<AppState>,
-    req: HttpRequest,
-    json_payload: web::Json<PaymentsRequest>,
+    state: web::Data<app::AppState>,
+    req: actix_web::HttpRequest,
+    json_payload: web::Json<payment_types::PaymentsRequest>,
     path: web::Path<String>,
-) -> HttpResponse {
+) -> impl Responder {
     let mut payload = json_payload.into_inner();
 
     if let Some(api_enums::CaptureMethod::Scheduled) = payload.capture_method {
@@ -170,7 +134,7 @@ pub async fn payments_update(
 
     let payment_id = path.into_inner();
 
-    payload.payment_id = Some(PaymentIdType::PaymentIntentId(payment_id));
+    payload.payment_id = Some(payment_types::PaymentIdType::PaymentIntentId(payment_id));
 
     let auth_type;
     (payload, auth_type) = match api::get_auth_type_and_check_client_secret(&req, payload) {
@@ -185,15 +149,12 @@ pub async fn payments_update(
         &req,
         payload,
         |state, merchant_account, req| {
-            let connector = req.connector;
-            payments::payments_core::<Authorize, PaymentsResponse, _, _, _>(
+            authorize_verify_select(
+                payments::PaymentUpdate,
                 state,
                 merchant_account,
-                payments::PaymentUpdate,
                 req,
                 auth_flow,
-                connector,
-                payments::CallConnectorAction::Trigger,
             )
         },
         auth_type,
@@ -204,11 +165,11 @@ pub async fn payments_update(
 #[instrument(skip_all, fields(flow = ?Flow::PaymentsConfirm))]
 // #[post("/{payment_id}/confirm")]
 pub async fn payments_confirm(
-    state: web::Data<AppState>,
-    req: HttpRequest,
-    json_payload: web::Json<PaymentsRequest>,
+    state: web::Data<app::AppState>,
+    req: actix_web::HttpRequest,
+    json_payload: web::Json<payment_types::PaymentsRequest>,
     path: web::Path<String>,
-) -> HttpResponse {
+) -> impl Responder {
     let mut payload = json_payload.into_inner();
 
     if let Some(api_enums::CaptureMethod::Scheduled) = payload.capture_method {
@@ -216,7 +177,7 @@ pub async fn payments_confirm(
     };
 
     let payment_id = path.into_inner();
-    payload.payment_id = Some(PaymentIdType::PaymentIntentId(payment_id));
+    payload.payment_id = Some(payment_types::PaymentIdType::PaymentIntentId(payment_id));
     payload.confirm = Some(true);
 
     let auth_type;
@@ -231,15 +192,12 @@ pub async fn payments_confirm(
         &req,
         payload,
         |state, merchant_account, req| {
-            let connector = req.connector;
-            payments::payments_core::<Authorize, PaymentsResponse, _, _, _>(
+            authorize_verify_select(
+                payments::PaymentConfirm,
                 state,
                 merchant_account,
-                payments::PaymentConfirm,
                 req,
                 auth_flow,
-                connector,
-                payments::CallConnectorAction::Trigger,
             )
         },
         auth_type,
@@ -250,12 +208,12 @@ pub async fn payments_confirm(
 #[instrument(skip_all, fields(flow = ?Flow::PaymentsCapture))]
 // #[post("/{payment_id}/capture")]
 pub async fn payments_capture(
-    state: web::Data<AppState>,
-    req: HttpRequest,
-    json_payload: web::Json<PaymentsCaptureRequest>,
+    state: web::Data<app::AppState>,
+    req: actix_web::HttpRequest,
+    json_payload: web::Json<payment_types::PaymentsCaptureRequest>,
     path: web::Path<String>,
-) -> HttpResponse {
-    let capture_payload = PaymentsCaptureRequest {
+) -> impl Responder {
+    let capture_payload = payment_types::PaymentsCaptureRequest {
         payment_id: Some(path.into_inner()),
         ..json_payload.into_inner()
     };
@@ -265,7 +223,7 @@ pub async fn payments_capture(
         &req,
         capture_payload,
         |state, merchant_account, payload| {
-            payments::payments_core::<Capture, PaymentsResponse, _, _, _>(
+            payments::payments_core::<api_types::Capture, payment_types::PaymentsResponse, _, _, _>(
                 state,
                 merchant_account,
                 payments::PaymentCapture,
@@ -282,10 +240,10 @@ pub async fn payments_capture(
 
 #[instrument(skip_all, fields(flow = ?Flow::PaymentsSessionToken))]
 pub async fn payments_connector_session(
-    state: web::Data<AppState>,
-    req: HttpRequest,
-    json_payload: web::Json<PaymentsSessionRequest>,
-) -> HttpResponse {
+    state: web::Data<app::AppState>,
+    req: actix_web::HttpRequest,
+    json_payload: web::Json<payment_types::PaymentsSessionRequest>,
+) -> impl Responder {
     let sessions_payload = json_payload.into_inner();
 
     api::server_wrap(
@@ -293,7 +251,13 @@ pub async fn payments_connector_session(
         &req,
         sessions_payload,
         |state, merchant_account, payload| {
-            payments::payments_core::<Session, PaymentsSessionResponse, _, _, _>(
+            payments::payments_core::<
+                api_types::Session,
+                payment_types::PaymentsSessionResponse,
+                _,
+                _,
+                _,
+            >(
                 state,
                 merchant_account,
                 payments::PaymentSession,
@@ -310,15 +274,15 @@ pub async fn payments_connector_session(
 
 #[instrument(skip_all)]
 pub async fn payments_response(
-    state: web::Data<AppState>,
-    req: HttpRequest,
+    state: web::Data<app::AppState>,
+    req: actix_web::HttpRequest,
     path: web::Path<(String, String, String)>,
-) -> HttpResponse {
+) -> impl Responder {
     let (payment_id, merchant_id, connector) = path.into_inner();
     let param_string = req.query_string();
 
-    let payload = PaymentsRetrieveRequest {
-        resource_id: PaymentIdType::PaymentIntentId(payment_id),
+    let payload = payment_types::PaymentsRetrieveRequest {
+        resource_id: payment_types::PaymentIdType::PaymentIntentId(payment_id),
         merchant_id: Some(merchant_id.clone()),
         force_sync: true,
         param: Some(param_string.to_string()),
@@ -329,7 +293,11 @@ pub async fn payments_response(
         &req,
         payload,
         |state, merchant_account, req| {
-            payments::handle_payments_redirect_response::<PSync>(state, merchant_account, req)
+            payments::handle_payments_redirect_response::<api_types::PSync>(
+                state,
+                merchant_account,
+                req,
+            )
         },
         api::MerchantAuthentication::MerchantId(Cow::Borrowed(&merchant_id)),
     )
@@ -339,9 +307,9 @@ pub async fn payments_response(
 #[instrument(skip_all, fields(flow = ?Flow::PaymentsCancel))]
 // #[post("/{payment_id}/cancel")]
 pub async fn payments_cancel(
-    state: web::Data<AppState>,
-    req: HttpRequest,
-    json_payload: web::Json<PaymentsCancelRequest>,
+    state: web::Data<app::AppState>,
+    req: actix_web::HttpRequest,
+    json_payload: web::Json<payment_types::PaymentsCancelRequest>,
     path: web::Path<String>,
 ) -> impl Responder {
     let mut payload = json_payload.into_inner();
@@ -353,7 +321,7 @@ pub async fn payments_cancel(
         &req,
         payload,
         |state, merchant_account, req| {
-            payments::payments_core::<Void, PaymentsResponse, _, _, _>(
+            payments::payments_core::<api_types::Void, payment_types::PaymentsResponse, _, _, _>(
                 state,
                 merchant_account,
                 payments::PaymentCancel,
@@ -371,10 +339,10 @@ pub async fn payments_cancel(
 #[instrument(skip_all, fields(flow = ?Flow::PaymentsList))]
 // #[get("/list")]
 pub async fn payments_list(
-    state: web::Data<AppState>,
-    req: HttpRequest,
-    payload: web::Query<PaymentListConstraints>,
-) -> HttpResponse {
+    state: web::Data<app::AppState>,
+    req: actix_web::HttpRequest,
+    payload: web::Query<payment_types::PaymentListConstraints>,
+) -> impl Responder {
     let payload = payload.into_inner();
     api::server_wrap(
         &state,
@@ -388,9 +356,56 @@ pub async fn payments_list(
     .await
 }
 
-fn _http_response<T: MessageBody + 'static>(response: T) -> HttpResponse<BoxBody> {
-    HttpResponse::Ok()
-        .content_type("application/json")
-        .append_header(("Via", "Juspay_router"))
-        .body(response)
+async fn authorize_verify_select<Op>(
+    operation: Op,
+    state: &app::AppState,
+    merchant_account: storage_models::merchant_account::MerchantAccount,
+    req: api_models::payments::PaymentsRequest,
+    auth_flow: api::AuthFlow,
+) -> app::core::errors::RouterResponse<api_models::payments::PaymentsResponse>
+where
+    Op: Sync
+        + Clone
+        + std::fmt::Debug
+        + payments::operations::Operation<api_types::Authorize, api_models::payments::PaymentsRequest>
+        + payments::operations::Operation<api_types::Verify, api_models::payments::PaymentsRequest>,
+{
+    // TODO: Change for making it possible for the flow to be inferred internally or through validation layer
+    // This is a temporary fix.
+    // After analyzing the code structure,
+    // the operation are flow agnostic, and the flow is only required in the post_update_tracker
+    // Thus the flow can be generated just before calling the connector instead of explicitly passing it here.
+
+    let connector = req.connector;
+    match req.amount.as_ref() {
+        Some(api_types::Amount::Value(_)) | None => payments::payments_core::<
+            api_types::Authorize,
+            payment_types::PaymentsResponse,
+            _,
+            _,
+            _,
+        >(
+            state,
+            merchant_account,
+            operation,
+            req,
+            auth_flow,
+            connector,
+            payments::CallConnectorAction::Trigger,
+        )
+        .await,
+
+        Some(api_types::Amount::Zero) => {
+            payments::payments_core::<api_types::Verify, payment_types::PaymentsResponse, _, _, _>(
+                state,
+                merchant_account,
+                operation,
+                req,
+                auth_flow,
+                connector,
+                payments::CallConnectorAction::Trigger,
+            )
+            .await
+        }
+    }
 }