Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(connector_token): Move config redis #4540

Merged
merged 9 commits into from May 13, 2024
Merged

Conversation

akshay-97
Copy link
Contributor

@akshay-97 akshay-97 commented May 3, 2024

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

  1. added connector account details to redis instead of config and having a ttl of 15 mins
  2. adding back config default write changes https://github.com/juspay/hyperswitch/pull/4330/files that got reverted

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?

Need to test stripe compatability,Tunnel using any creds and verify in the db,config is not storing.
tested using /payment_intents flow with passing different creds

curl --location 'http://localhost:8080/vs/v1/payment_intents' \
--header 'Content-Type: application/x-www-form-urlencoded' \
--header 'User-Agent: helloMozilla/5.0 (Linux; Android 12; SM-S906N Build/QP1A.190711.020; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/80.0.3987.119 Mobile Safari/537.36' \
--header 'api-key:***' \
--data-urlencode 'amount=2000' \
--data-urlencode 'currency=AED' \
--data-urlencode 'confirm=true' \
--data-urlencode 'payment_method_data%5Btype%5D=card' \
--data-urlencode 'payment_method_data%5Bcard%5D%5Bnumber%5D=4456530000001096' \
--data-urlencode 'payment_method_data%5Bcard%5D%5Bexp_month%5D=12' \
--data-urlencode 'payment_method_data%5Bcard%5D%5Bexp_year%5D=2030' \
--data-urlencode 'payment_method_data%5Bcard%5D%5Bcvc%5D=123' \
--data-urlencode 'connector%5B%5D=noon' \
--data-urlencode 'capture_method=automatic' \
--data-urlencode 'customer=sahkal12' \
--data-urlencode 'description=Card Payment' \
--data-urlencode 'off_session=true' \
--data-urlencode 'mandate_data%5Bcustomer_acceptance%5D%5Bonline%5D%5Buser_agent%5D=helloMozilla/5.0 (Linux; Android 12; SM-S906N Build/QP1A.190711.020; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/80.0.3987.119 Mobile Safari/537.36' \
--data-urlencode 'mandate_data%5Bcustomer_acceptance%5D%5Bonline%5D%5Bip_address%5D=123.32.25.123' \
--data-urlencode 'mandate_data%5Bamount%5D=20877' \
--data-urlencode 'setup_future_usage=off_session' \
--data-urlencode 'return_url=https://juspay.in' \
--data-urlencode 'metadata%5Btxn_Id%5D=sahkal_payment' \
--data-urlencode 'metadata%5BtxnUuid%5D=94hfdmoakosdkifdhaisl' \
--data-urlencode 'metadata%5Bmerchant_id%5D=sahkal' \
--data-urlencode 'metadata%5Beuler_merchant_id%5D=global_installment' \
--data-urlencode 'connector_metadata%5Bnoon%5D%5Border_category%5D=pay' \
--data-urlencode 'payment_method_data%5Bcard%5D%5Bholder_name%5D=sahkal' \
--data-urlencode 'merchant_connector_details%5Bencoded_data%5D=*****' \
--data-urlencode 'merchant_connector_details%5Bcreds_identifier%5D=test123'

test screenshots:
Screenshot 2024-05-09 at 16 00 33
Screenshot 2024-05-09 at 16 00 41

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

@akshay-97 akshay-97 requested review from a team as code owners May 3, 2024 11:24
);

redis
.serialize_and_set_key_with_expiry(key.as_str(), &encoded_data.peek(), i64::from(900))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we not use hardcoded 900 value instead use const something like TOKEN_TTL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

id: key,
},
)
.attach_printable("Failed to get redis Value")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also specify which data we failed to retrieve from redis, just for better tracking since MerchantConnectorAccountNotFound does not specify that it is a value from external system

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the result from this function logged or returned somewhere?

@sahkal
Copy link
Contributor

sahkal commented May 3, 2024

Also, Please put testing screenshots from stripe compatibility layer to ensure validation.

@@ -113,3 +113,5 @@ pub const POLL_ID_TTL: i64 = 900;
// Default Poll Config
pub const DEFAULT_POLL_DELAY_IN_SECS: i8 = 2;
pub const DEFAULT_POLL_FREQUENCY: i8 = 5;

pub const CONNECTOR_TOKEN_TTL: i64 = 900;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rename it to connector creds token

sahkal
sahkal previously approved these changes May 9, 2024
@sahkal sahkal self-requested a review May 9, 2024 11:53
@sahkal
Copy link
Contributor

sahkal commented May 9, 2024

CI checks are failing

@sahkal sahkal added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed A-core Area: Core flows labels May 10, 2024
@sahkal sahkal added this to the May 2024 Release milestone May 10, 2024
@akshay-97 akshay-97 changed the title Move config redis fix(connector_token): Move config redis May 10, 2024
@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue May 13, 2024
Merged via the queue into main with commit 1602eb5 May 13, 2024
12 of 15 checks passed
@Gnanasundari24 Gnanasundari24 deleted the move_config_redis branch May 13, 2024 13:20
@SanchithHegde SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core flows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants