feat(router): add support for relay refund incoming webhooks #6974

ShankarSinghC · 2025-01-02T13:49:11Z

Type of Change

Description

Some connectors, like Adyen, do not support a refund sync API, so we need to rely on refund webhooks for the status. To address this, support for relay refund webhooks is being added.

To support this use case, a new webhook endpoint needs to be introduced for relay webhooks:
/webhooks/relay/{merchant_id}/{connector_id_or_name}

Once the webhook is identified as a relay webhook, it should be handled based on the flow type, such as refunds, payments, etc. For now, only support for relay refund webhooks has been added.

Additional Changes

This PR modifies the API contract
This PR modifies the database schema
This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Checklist

I formatted the code cargo +nightly fmt --all
I addressed lints thrown by cargo clippy
I reviewed the submitted code
I added unit tests for my changes where possible

semanticdiff-com · 2025-01-02T13:49:13Z

Review changes with

Changed Files

File	Status
crates/router/src/core/webhooks/incoming.rs	48% smaller
crates/router/src/routes/lock_utils.rs	30% smaller
crates/router/src/lib.rs	18% smaller
crates/router/src/routes.rs	7% smaller
crates/diesel_models/src/query/relay.rs	1% smaller
crates/api_models/src/webhooks.rs	0% smaller
crates/common_utils/src/id_type/relay.rs	0% smaller
crates/router/src/core/webhooks/incoming_v2.rs	0% smaller
crates/router/src/db/relay.rs	0% smaller
crates/router/src/routes/app.rs	0% smaller
crates/router/src/routes/webhooks.rs	0% smaller
crates/router/src/types/transformers.rs	0% smaller
crates/router_env/src/logger/types.rs	0% smaller
migrations/2025-01-07-105739_create_index_for_relay/down.sql	Unsupported file format
migrations/2025-01-07-105739_create_index_for_relay/up.sql	Unsupported file format

crates/router/src/core/webhooks/incoming.rs

sai-harsha-vardhan · 2025-01-06T08:01:52Z

crates/router/src/routes/webhooks.rs

+) -> impl Responder {
+    let flow = Flow::IncomingWebhookReceive;
+    let (merchant_id, connector_id_or_name) = path.into_inner();
+    let is_relay_webhook = true;


Move this to a const

Can we consider using an enum instead of a bool maybe?

crates/router/src/routes/app.rs

crates/router/src/core/webhooks/incoming.rs

…perswitch into relay/incoming-webhook

…ay/incoming-webhook

sai-harsha-vardhan · 2025-01-07T09:21:41Z

crates/diesel_models/src/query/relay.rs

+    ) -> StorageResult<Self> {
+        generics::generic_find_one::<<Self as HasTable>::Table, _, _>(
+            conn,
+            dsl::id.eq(connector_reference_id.to_owned()),


We should use dsl::connector_reference_id right?

sai-harsha-vardhan · 2025-01-07T09:22:14Z

crates/diesel_models/src/query/relay.rs

+
+    pub async fn find_by_connector_reference_id(
+        conn: &PgPooledConn,
+        connector_reference_id: &str,


As, it's not globally unique we may have to query this along with profile_id. You can add a index in DB too

sai-harsha-vardhan · 2025-01-07T09:26:30Z

crates/router/src/core/webhooks/incoming.rs

+
+            // Using early return ensures unsupported webhooks are acknowledged to the connector
+            if let Some(error) = relay_webhook_response.as_ref().err() {
+                if let errors::ApiErrorResponse::NotSupported { .. } = error.current_context() {


Can avoid this nested if, by getting the current context in the outer if statement

sai-harsha-vardhan · 2025-01-07T09:28:14Z

crates/router/src/core/webhooks/incoming.rs

-            ))
-            .await
-            .attach_printable("Incoming webhook flow for disputes failed"),
+                    metrics::WEBHOOK_EVENT_TYPE_IDENTIFICATION_FAILURE_COUNT.add(


This metric is not relevant in current case, this metric indicates event type identification failure where as our case is event type being identified correctly but not supported

sai-harsha-vardhan · 2025-01-07T09:28:46Z

crates/router/src/core/webhooks/incoming.rs

+                        .get_webhook_api_response(&request_details, None)
+                        .switch()
+                        .attach_printable(
+                            "Failed while early return in case of event type parsing",


Suggested change

"Failed while early return in case of event type parsing",

"Failed while early return in case of not supported event types in relay webhooks",

sai-harsha-vardhan · 2025-01-07T09:36:59Z

crates/router/src/core/webhooks/incoming.rs

+        ))
+        .await
+        .change_context(errors::ApiErrorResponse::InternalServerError)
+        .attach_printable("Failed to update relay")?;


message's incorrect here

sai-harsha-vardhan · 2025-01-07T09:37:26Z

crates/router/src/core/webhooks/incoming.rs

+            response
+        } else {
+            Err(errors::ApiErrorResponse::InternalServerError)
+                .attach_printable("Failed to force sync relay")?


Suggested change

.attach_printable("Failed to force sync relay")?

.attach_printable("Unexpcted response from force sync relay")?

sai-harsha-vardhan · 2025-01-07T09:39:58Z

crates/router/src/routes/app.rs

+impl RelayWebhooks {
+    pub fn server(state: AppState) -> Scope {
+        use api_models::webhooks as webhook_type;
+        web::scope("/webhooks")


Suggested change

web::scope("/webhooks")

web::scope("/webhooks/relay")

scope for RelayWebhooks would always be /webhooks/relay

…perswitch into relay/incoming-webhook

…ay/incoming-webhook

SanchithHegde · 2025-01-07T19:21:54Z

crates/diesel_models/src/query/relay.rs

+    pub async fn find_by_connector_reference_id(
+        conn: &PgPooledConn,
+        connector_reference_id: &str,
+        profile_id: &common_utils::id_type::ProfileId,
+    ) -> StorageResult<Self> {


Can you rename this accordingly:

Either:

Suggested change

pub async fn find_by_connector_reference_id(

conn: &PgPooledConn,

connector_reference_id: &str,

profile_id: &common_utils::id_type::ProfileId,

) -> StorageResult<Self> {

pub async fn find_by_profile_id_connector_reference_id(

conn: &PgPooledConn,

profile_id: &common_utils::id_type::ProfileId,

connector_reference_id: &str,

) -> StorageResult<Self> {

Or:

Suggested change

pub async fn find_by_connector_reference_id(

conn: &PgPooledConn,

connector_reference_id: &str,

profile_id: &common_utils::id_type::ProfileId,

) -> StorageResult<Self> {

pub async fn find_by_connector_reference_id_profile_id(

conn: &PgPooledConn,

connector_reference_id: &str,

profile_id: &common_utils::id_type::ProfileId,

) -> StorageResult<Self> {

I'd prefer the former. Then fix the param order in the usages as well.

SanchithHegde · 2025-01-07T19:24:19Z

crates/router/src/db/relay.rs

+    async fn find_relay_by_connector_reference_id(
+        &self,
+        key_manager_state: &KeyManagerState,
+        merchant_key_store: &domain::MerchantKeyStore,
+        connector_reference_id: &str,
+        profile_id: &common_utils::id_type::ProfileId,
+    ) -> CustomResult<hyperswitch_domain_models::relay::Relay, errors::StorageError>;


Same here:

Suggested change

async fn find_relay_by_connector_reference_id(

&self,

key_manager_state: &KeyManagerState,

merchant_key_store: &domain::MerchantKeyStore,

connector_reference_id: &str,

profile_id: &common_utils::id_type::ProfileId,

) -> CustomResult<hyperswitch_domain_models::relay::Relay, errors::StorageError>;

async fn find_relay_by_profile_id_connector_reference_id(

&self,

key_manager_state: &KeyManagerState,

merchant_key_store: &domain::MerchantKeyStore,

profile_id: &common_utils::id_type::ProfileId,

connector_reference_id: &str,

) -> CustomResult<hyperswitch_domain_models::relay::Relay, errors::StorageError>;

And fix it in trait implementations and usages.

SanchithHegde · 2025-01-07T19:31:32Z

crates/router/src/routes/app.rs

+impl RelayWebhooks {
+    pub fn server(state: AppState) -> Scope {
+        use api_models::webhooks as webhook_type;
+        web::scope("/webhooks/relay")


Have we confirmed that this doesn't affect the existing incoming webhooks flows, due to the possibly conflicting routes?

SanchithHegde · 2025-01-07T19:32:38Z

crates/router/src/routes/webhooks.rs

+) -> impl Responder {
+    let flow = Flow::IncomingWebhookReceive;
+    let (merchant_id, connector_id_or_name) = path.into_inner();
+    let is_relay_webhook = true;


Can we consider using an enum instead of a bool maybe?

SanchithHegde · 2025-01-07T19:33:03Z

crates/router/src/routes/webhooks.rs

+    path: web::Path<(common_utils::id_type::MerchantId, String)>,
+) -> impl Responder {
+    let flow = Flow::IncomingWebhookReceive;
+    let (merchant_id, connector_id_or_name) = path.into_inner();


Suggested change

let (merchant_id, connector_id_or_name) = path.into_inner();

let (merchant_id, connector_id) = path.into_inner();

SanchithHegde · 2025-01-07T19:33:50Z

crates/router/src/routes/webhooks.rs

+    state: web::Data<AppState>,
+    req: HttpRequest,
+    body: web::Bytes,
+    path: web::Path<(common_utils::id_type::MerchantId, String)>,


Can we use MerchantConnectorAccountId instead of a String, if we only expect a MCA ID in the path?

Suggested change

path: web::Path<(common_utils::id_type::MerchantId, String)>,

path: web::Path<(common_utils::id_type::MerchantId, common_utils::id_type::MerchantConnectorAccountId)>,

SanchithHegde · 2025-01-07T19:37:39Z

migrations/2025-01-07-105739_create_index_for_relay/up.sql

@@ -0,0 +1,2 @@
+-- Your SQL goes here
+CREATE UNIQUE INDEX relay_profile_id_connector_reference_id ON relay (profile_id, connector_reference_id);


Nit:

Suggested change

CREATE UNIQUE INDEX relay_profile_id_connector_reference_id ON relay (profile_id, connector_reference_id);

CREATE UNIQUE INDEX relay_profile_id_connector_reference_id_index ON relay (profile_id, connector_reference_id);

Update the down.sql file as well if you're taking up this change.

add support for relay refund incoming wehooks

94c7a43

ShankarSinghC added A-core Area: Core flows C-refactor Category: Refactor A-refunds Area: Refund flows labels Jan 2, 2025

ShankarSinghC self-assigned this Jan 2, 2025

ShankarSinghC requested review from a team as code owners January 2, 2025 13:49

chore: run formatter

981d0e9

ShankarSinghC changed the title ~~add support for relay refund incoming webhooks~~ feat(router): add support for relay refund incoming webhooks Jan 2, 2025

ShankarSinghC linked an issue Jan 2, 2025 that may be closed by this pull request

add support for relay refund incoming webhooks #6975

Open

sai-harsha-vardhan reviewed Jan 6, 2025

View reviewed changes

ShankarSinghC and others added 6 commits January 6, 2025 16:22

address pr comments

2f56a6a

address pr comments

9e822e1

address pr comments

bdbc7b7

Merge branch 'relay/incoming-webhook' of https://github.com/juspay/hy…

041909a

…perswitch into relay/incoming-webhook

Merge branch 'main' of https://github.com/juspay/hyperswitch into rel…

3fe9701

…ay/incoming-webhook

chore: run formatter

4586498

sai-harsha-vardhan reviewed Jan 7, 2025

View reviewed changes

ShankarSinghC added 3 commits January 7, 2025 16:51

address pr comments

09db34a

Merge branch 'relay/incoming-webhook' of https://github.com/juspay/hy…

d6eab60

…perswitch into relay/incoming-webhook

Merge branch 'main' of https://github.com/juspay/hyperswitch into rel…

0d74735

…ay/incoming-webhook

hyperswitch-bot bot added the M-database-changes Metadata: This PR involves database schema changes label Jan 7, 2025

chore: run formatter

8e59f55

sai-harsha-vardhan approved these changes Jan 7, 2025

View reviewed changes

SanchithHegde reviewed Jan 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(router): add support for relay refund incoming webhooks #6974

feat(router): add support for relay refund incoming webhooks #6974

ShankarSinghC commented Jan 2, 2025 •

edited

Loading

semanticdiff-com bot commented Jan 2, 2025 •

edited

Loading

sai-harsha-vardhan Jan 6, 2025

SanchithHegde Jan 7, 2025

sai-harsha-vardhan Jan 7, 2025

sai-harsha-vardhan Jan 7, 2025

sai-harsha-vardhan Jan 7, 2025

sai-harsha-vardhan Jan 7, 2025

sai-harsha-vardhan Jan 7, 2025

sai-harsha-vardhan Jan 7, 2025

sai-harsha-vardhan Jan 7, 2025

sai-harsha-vardhan Jan 7, 2025

sai-harsha-vardhan Jan 7, 2025

SanchithHegde Jan 7, 2025

SanchithHegde Jan 7, 2025

SanchithHegde Jan 7, 2025

SanchithHegde Jan 7, 2025

SanchithHegde Jan 7, 2025

SanchithHegde Jan 7, 2025

SanchithHegde Jan 7, 2025

	"Failed while early return in case of event type parsing",
	"Failed while early return in case of not supported event types in relay webhooks",

	.attach_printable("Failed to force sync relay")?
	.attach_printable("Unexpcted response from force sync relay")?

	let (merchant_id, connector_id_or_name) = path.into_inner();
	let (merchant_id, connector_id) = path.into_inner();

	path: web::Path<(common_utils::id_type::MerchantId, String)>,
	path: web::Path<(common_utils::id_type::MerchantId, common_utils::id_type::MerchantConnectorAccountId)>,

		@@ -0,0 +1,2 @@
		-- Your SQL goes here
		CREATE UNIQUE INDEX relay_profile_id_connector_reference_id ON relay (profile_id, connector_reference_id);

feat(router): add support for relay refund incoming webhooks #6974

Are you sure you want to change the base?

feat(router): add support for relay refund incoming webhooks #6974

Conversation

ShankarSinghC commented Jan 2, 2025 • edited Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

semanticdiff-com bot commented Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShankarSinghC commented Jan 2, 2025 •

edited

Loading

semanticdiff-com bot commented Jan 2, 2025 •

edited

Loading