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

دلیل استفاده از crc32 توی بعضی از درایور ها چیه؟ #124

Open
m-ostadi opened this issue Feb 1, 2022 · 8 comments

Comments

@m-ostadi
Copy link

m-ostadi commented Feb 1, 2022

بعضی جاها روی orderId از crc32 استفاده شده و این باعث میشه uuid ای که خودمون بهش دادیم رو موقع برگشت نتونیم پیدا کنیم . ازونجایی هم که crc32 احتمال تکراری شدن هش رو داره امکان duplicate شدن orderId ها پیش میاد توی دیتابیس که زیاد جالب نیس.
اگر نقش crc32 فقط کوتاه تر کردن uuid و عددی کردنش هس به نظرم بهتره که قبلش چک بشه تا اگه orderId نبود این اتفاق بیفته

@khanzadimahdi
Copy link
Member

سلام وقت بخیر. اوردر ایدی باید عددی باشه به همین دلیل از crc32 استفاده شده. راجب تکراری شدنه موافق هستم باهاتون. اگه امکانش هست فیکسش کنید و مرج بزارید براش.

@m-ostadi
Copy link
Author

m-ostadi commented Feb 1, 2022

تمام

@AmirRezaM75
Copy link

AmirRezaM75 commented Jul 2, 2023

I think this is not the responsibility of payment driver to change unique id of the invoice. It can check if the type is correct or not! Also we need to store crc32 version of our invoices in the database in order to find it in callback and unique index is not reliable there.

@m-ostadi
Copy link
Author

m-ostadi commented May 10, 2024

#135 وقتی قبول دارین این مشکل وجود داره این مرج رو چرا نزدین؟؟

@khanzadimahdi
Copy link
Member

khanzadimahdi commented May 10, 2024

سلام وقتتون بخیر. من دوباره چک کردم. ما وقتی میخوای یه دیتایی رو به بانک بفرستیم و بانک اون دیتا رو فقط عددی قبول میکنه دوتا روش وجود داره:

۱- از همون اول دیتا رو عددی تولید کنیم
۲- هرجا که بانک نیاز داشت اون دیتا رو به عددی تبدیل کنی و به بانک بدیم (در صورتی که موقع برگشت به uuid اولیه دسترسی داشته باشیم)

اینکه با استفاده از crc32 دیتا بعضی وقتا حالت یونیک بودنشو از دست میده درسته! اگه الگوریتم بهتری مد نظرتونه یا تغییراتی نیازه میتونید انجامش بدید و مرج میشه حتما.

دلیل اینکه PR کلوز شده احتمالا به خاطر طولانی بودن مدت باز بودنشه. الان دوباره بازش میکنم. اما پیاده سازی انجام شده مورد قبول نیست! شما نباید لاجیک داخل یک فانکشن رو با ارسال فلگ بولین کنترل کنید! با اینکار اصل SRP رو نقض کردین و فانکشن شما داره بیشتر از یک کار انجام میده.

داپلیکیت شدن پیمنت خیلی موضوع جدی هست. منم این اخر هفته وقت میزارم و دوباره داکیومنت بانک ها رو چک میکنم. ممنونم از اینکه پیگیری میکنید

@m-ostadi
Copy link
Author

سلام وقت شما هم بخیر. راه دیگش بنظرم اینه که یه property به اسم isUUIDProvidedByUser بزاریم توی کلاس invoice و موقع ست کردن uuid ترو بشه مقدارش. ازینور هم هر جا که crc32 گذاشتیم قبلش اون پراپرتی رو چک کنیم. حالا برای تمیزی میتونه داخل یه متود باشه این لاجیک یا هم توی کل کد پخش بشه .
در هر صورت نباید وقتی دولوپر خودش مقدار uuid رو ست میکنه تغییری توش داد.

@m-ostadi
Copy link
Author

یا یه متود جدید اضافه کنیم بجای استفاده از فلگ needNumericUUID و جاهایی که نیازه crc32 باشه ازون استفاده کنیم :
public function getNumericUuid() { return !is_numeric($this->uuid) ? crc32($this->uuid) : $this->uuid; }

@m-ostadi
Copy link
Author

m-ostadi commented May 11, 2024

برنچ قبلی چندتا کانفلیکت داشت. این رو زدم بجاش #238 . یه چک بکنین.
@khanzadimahdi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants