juspay / hyperswitch

An open source payments switch written in Rust to make payments fast, reliable and affordable
https://hyperswitch.io/
Apache License 2.0
12.57k stars 1.35k forks source link

[FEATURE] move redis key creation to a common module #917

Open Narayanbhat166 opened 1 year ago

Narayanbhat166 commented 1 year ago

Feature Description

Few entities currently are stored in redis. We create a key for each entity. This key will be usually of the form {feature}_{merchant_id}_{extras}. The creation of this key is being done in multiple places. It will be better to move all of these functions to a common module, this will add uniformity and help someone to easily know the key format for a particular entity.

Possible Implementation

There can be a module that exports functions for each of the key creation. This function takes the dynamic variables ( like merchant_id and extra variables and build the key. The format of this function would be

fn create_access_token_key(arg1: String, arg2: String, ...) -> String {}

One may also think about using enums for this.

Have you spent some time to check if this feature request has been raised before?

Have you read the Contributing Guidelines?

Are you willing to submit a PR?

No, but I'm happy to collaborate on a PR with someone else

IsaacCloos commented 1 year ago

šŸ™‹šŸ»ā€ā™‚ļø

I'd like to draft a solution for this issue for you šŸ˜„. I suspect based on the criteria that there could be quite a bit of workshopping involved to iron this out, but I thrive on feedback and would enjoy that greatly.

āœ… I have read the contribution guidelines šŸ‘€ I saw this item linked in TWIR 491

SanchithHegde commented 1 year ago

Hey @IsaacCloos, I've assigned this issue to you.

Narayanbhat166 commented 1 year ago

Thanks @IsaacCloos for showing interest. Yes that would be great. We can discuss about the implementation.

EliKalter commented 1 year ago

I am interested in collaborating with someone on their project so I can learn the language while they get a sidekick. Let me Know if this is something that could work for you. I Would need you to explain and guide me through my first steps in the language (I already started learning) and set the standard for me in code reviews but after a short while I'm sure I'll become an asset to the project. (I have some experience in Typescript and Java and have learned C a few years ago).

IsaacCloos commented 1 year ago

I'm sorry for the delay! I have become busy with work and juggling a few other issues. Please feel free to reassign this issue as you see fit since I noticed it hasn't been labeled as low priority. I don't want to hold up any work by squatting on it.

I see @EliKalter is showing interest šŸ‘€


I'd be happy to share my experience with setting up the project for local development and testing if somebody else gets stuck šŸ™‚

EliKalter commented 1 year ago

Thanks @IsaacCloos , I appreciate the response, Honestly I'm not sure it was a good idea for me to offer help because I've never contributed to an open source before (so I'd need help with the most basic of tasks in that regard) nor do I know rust even remotely well (so I'd need help on that from as well), I'm thinking maybe I should learn rust better by myself and start contributing on other beginner friendly projects first and get back to contributing to rust projects some day down the line, what are your thoughts? (I don't like wasting peoples time...)

SanchithHegde commented 1 year ago

I'm sorry for the delay! I have become busy with work and juggling a few other issues.

@IsaacCloos No problem, we understand that contributors have a personal life, and contribute to our project during their free time and out of their own interest, not due to obligation.

Please feel free to reassign this issue as you see fit since I noticed it hasn't been labeled as low priority. I don't want to hold up any work by squatting on it.

I'm sorry for the confusion, this is indeed a low priority refactor and does not block any of our work. It's a good-to-have change, and not an immediate necessity. I've updated the labels on this issue to reflect the same.

Let us know if you're still interested in taking a look at this, we'll help @EliKalter with any of the other issues they've expressed their interest on.

SanchithHegde commented 1 year ago

[...] Honestly I'm not sure it was a good idea for me to offer help because I've never contributed to an open source before (so I'd need help with the most basic of tasks in that regard) nor do I know rust even remotely well (so I'd need help on that from as well), I'm thinking maybe I should learn rust better by myself and start contributing on other beginner friendly projects first and get back to contributing to rust projects some day down the line, what are your thoughts? (I don't like wasting peoples time...)

Folks being new to Rust and/or open source contributions is completely fine with us. We all started somewhere. Among the issues you expressed your interest on, #899 seems to be a pretty simple change. I can assign it to you and you can work on it at your own pace. Would that work for you? Feel free to ask any more questions you may have on that thread.

EliKalter commented 1 year ago

[...] Honestly I'm not sure it was a good idea for me to offer help because I've never contributed to an open source before (so I'd need help with the most basic of tasks in that regard) nor do I know rust even remotely well (so I'd need help on that from as well), I'm thinking maybe I should learn rust better by myself and start contributing on other beginner friendly projects first and get back to contributing to rust projects some day down the line, what are your thoughts? (I don't like wasting peoples time...)

Folks being new to Rust and/or open source contributions is completely fine with us. We all started somewhere. Among the issues you expressed your interest on, #899 seems to be a pretty simple change. I can assign it to you and you can work on it at your own pace. Would that work for you? Feel free to ask any more questions you may have on that thread.

Thanks @SanchithHegde , I assume this thread is being taken care of by @IsaacCloos and that's way you suggested #899?! I'll take a look at #899 and hopefully feel comfortable taking care of it, anyway I'll move my comments to that thread now, thank you

SanchithHegde commented 1 year ago

I assume this thread is being taken care of by @IsaacCloos and that's way you suggested #899?!

I suggested #899 since we're not yet sure if @IsaacCloos would take this one up.

SanchithHegde commented 1 year ago

@EliKalter I'll be assigning this to you, as discussed over Discord.

SanchithHegde commented 1 year ago

@EliKalter Please let us know if you're still working on this issue or still interested in working on it. There hasn't been any recent activity on this thread, or any open PRs.

We hope to hear from you by the end of June 2023, or we'll open this for other contributors to pick up.

SanchithHegde commented 1 year ago

I'll be opening this issue for other contributors to pick up.

lokesh185 commented 1 year ago

@SanchithHegde ,I would like to work on this if this is free .

SanchithHegde commented 1 year ago

Sure @lokesh185, I'll assign this to you. You can reach out to us in case of any queries either on this thread, or on our Discord server.

deepanshu-iiitu commented 1 year ago

Hey @lokesh185 Just checking in - are you working on it? Do you need help from our side?

lokesh185 commented 1 year ago

@deepanshu-iiitu yes i am still working on it . this is the enum and impl that i was thinking of using , does this look good ?

pub enum RedisKey<'a> {
    // "access_token_{merchant_id}_{connector_name}"
    AccsesToken {
        merchant_id: &'a str,
        connector_name: &'a str,
    },
    // for "mcd_{merchant_id}_{creds_identifier}"
    MCDCredsId {
        merchant_id: &'a str,
        creds_identifier: &'a str,
    },
    // for "connector_resp_{merchant_id}_{payment_id}_{attempt_id}"
    ConnectionResponse {
        merchant_id: &'a str,
        payment_id: &'a str,
        attempt_id: &'a str,
    },
    // for "pi_{payment_id}"
    PaymentId {
        payment_id: &'a str,
    },
    // for "mid_{merchant_id}_pid_{payment_id}"
    MerchantPaymentIds {
        merchant_id: &'a str,
        payment_id: &'a str,
    },
    // for "whconf_{merchant_id}_{connector_id}"
    WhConf {
        merchant_id: &'a str,
        connector_id: &'a str,
    },
    // for "whsec_verification_{connector_label}_{merchant_id}"
    WhSecVerification {
        connector_label: &'a str,
        merchant_id: &'a str,
    },
}
impl ToString for RedisKey<'_> {
    fn to_string(&self) -> String {
        match self {
            Self::AccsesToken {
                merchant_id,
                connector_name,
            } => {
                format!("access_token_{merchant_id}_{connector_name}")
            }
            Self::ConnectionResponse {
                merchant_id,
                payment_id,
                attempt_id,
            } => {
                format!("connector_resp_{merchant_id}_{payment_id}_{attempt_id}")
            }
            Self::MCDCredsId {
                merchant_id,
                creds_identifier,
            } => {
                format!("mcd_{merchant_id}_{creds_identifier}")
            }
            Self::PaymentId { payment_id } => {
                format!("pi_{payment_id}")
            }
            Self::MerchantPaymentIds {
                merchant_id,
                payment_id,
            } => {
                format!("mid_{merchant_id}_pid_{payment_id}")
            }
            Self::WhConf {
                merchant_id,
                connector_id,
            } => {
                format!("whconf_{merchant_id}_{connector_id}")
            }
            Self::WhSecVerification {
                connector_label,
                merchant_id,
            } => {
                format!("whsec_verification_{connector_label}_{merchant_id}")
            }
        }
    }
}

this is how to make the redis key

    let key = RedisKey::AccsesToken {
        merchant_id: "abcd",
        connector_name: "efgh",
    }
    .to_string();
    assert_eq!(key, "access_token_abcd_efgh".to_string());
Narayanbhat166 commented 1 year ago

This seems right, good work @lokesh185, you will have to fix few of the spellings though!