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.27k stars 1.31k forks source link

[FEATURE] Use the newtype pattern for ZIP/PIN codes #1492

Open lsampras opened 1 year ago

lsampras commented 1 year ago

Feature Description

This issue covers one of the problems mentioned in #119, about using the newtype pattern for zip/pin codes. We want to make sure that the we have valid zip/pin codes by assigning them to a new type...

This newtype should be validated as part of construction & deserialization, can refer to #851 as an example

Please also include unit tests in your PR.

For external contributors, clarify any questions you may have, and let us know that you're interested in picking this up. We'll assign this issue to you to reduce duplication of effort on the same task.

Possible Implementation

You could use a newtype for ZipCode. Also, implement Serialize and Deserialize on these two types.

You can ask about any external libraries that you think should ease this out in the comments

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?

None

slelyukh commented 1 year ago

Hello, can you assign this issue to me?

SanchithHegde commented 1 year ago

Sure @slelyukh, I've assigned this to you. Feel free to ask any queries you may have on this thread or on our Discord server.

slelyukh commented 1 year ago

It appears that ZIP code validation is not currently done anywhere in the repo except through an AVS (address verification service) check done by credit card issuers. I don't think it makes sense to validate the postal code on our own separately from this since postal code validation is not a trivial task and requires knowledge of . Also it seems to make more sense to validate the entire address instead of just the postal code(since an address with a valid postal code that doesn't contain the street in the address is still invalid) if we are to validate it anywhere (this would require querying some address validation API) and to do this during construction of Address types.

Something else I have noticed is that the codebase sometimes uses "zip code" and sometimes uses "postal code" which is inconsistent and also inaccurate since only the US uses ZIP codes. Maybe a refactoring issue should be opened to only use postal code in variable and field names.

Maybe the newtype strategy can instead be applied to address types as a whole with something like struct ValidAddress(Address);? though this seems a bit redundant.

*Edit: You can use country dependent regular expressions to filter out ZIP codes with incorrect formats though this does not guarantee validity since countries don't use all of their possible zip codes.

lsampras commented 1 year ago

Hey @slelyukh , the way I see it there are 3 levels of validation that can be done for this...

  1. Country agnostic loose format validation We loosely set constraints around what a postal code can be. for e.g

    1. a postal code has to be between 4-10 characters length...
    2. It can contain only alphanumeric characters
  2. Country specific format validation We add validation based on the country for e.g

    1. US should be 4-5 characters with alphanumeric
    2. India would be 6 characters numeric
    3. UK would be 6 character alphanumeric
  3. Country specific format & value validation This would not only validate the format for the postal code but also that it represents a valid place... This would be more cumbersome to implement and likely requires to maintain a directory...

We'd prefer the basic level of validation Country agnostic loose format validation. We could go with the intermediate level (Country specific format validation) as well if there's an external library that maintains these country specific requirements/validations, as keeping track of what is valid in each country would add unnecessary maintenance overhead to this project...

Would love to know your thoughts on this as well @SanchithHegde ...

SanchithHegde commented 1 year ago

We'd prefer the basic level of validation Country agnostic loose format validation. We could go with the intermediate level (Country specific format validation) as well if there's an external library that maintains these country specific requirements/validations, as keeping track of what is valid in each country would add unnecessary maintenance overhead to this project.

I agree with this, a basic validation should suffice, or if external libraries are available, country-specific validation would be great (but not a necessity).