-
Notifications
You must be signed in to change notification settings - Fork 152
Fix voucher redemption race condition using pessimistic locking #1399
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
base: enext
Are you sure you want to change the base?
Fix voucher redemption race condition using pessimistic locking #1399
Conversation
Currently translated at 0.1% (3 of 4121 strings) Translation: eventyay/Eventyay Tickets Translate-URL: https://hosted.weblate.org/projects/open-event/eventyay-tickets/zh_Hant/
Currently translated at 0.2% (9 of 4121 strings) Translation: eventyay/Eventyay Tickets Translate-URL: https://hosted.weblate.org/projects/open-event/eventyay-tickets/zh_Hant/
Currently translated at 0.4% (18 of 4121 strings) Translation: eventyay/Eventyay Tickets Translate-URL: https://hosted.weblate.org/projects/open-event/eventyay-tickets/pl_INFORMAL/
Currently translated at 2.9% (123 of 4121 strings) Translation: eventyay/eventyay Translate-URL: https://hosted.weblate.org/projects/eventyay/eventyay/zh_Hant/
Currently translated at 2.9% (123 of 4121 strings) Translation: eventyay/eventyay Translate-URL: https://hosted.weblate.org/projects/eventyay/eventyay/zh_Hant/
The issue was caused by unconditional access to test_form.cleaned_data without checking if the form validation succeeded. When test_form.is_valid() returned False, accessing cleaned_data could raise AttributeError or return incomplete data, causing a 500 error. Solution: Added conditional check to only access cleaned_data when form is valid, otherwise use empty dict for initial values. This ensures the export page loads properly even when no valid GET parameters are provided. Changes: - Modified ExportMixin.exporters property in control/views/orders.py - Added validation check before accessing test_form.cleaned_data - Fallback to empty dict when form is invalid
Applied the same fix from issueto the organizer-level export
functionality. The ExportMixin in organizer.py had the identical issue
where test_form.cleaned_data was accessed without checking if the form
validation succeeded first.
This prevents potential HTTP 500 errors when accessing:
- /control/organizer/{organizer}/export/
Changes:
- Modified ExportMixin.exporters in control/views/organizer.py
- Added validation check before accessing test_form.cleaned_data
- Fallback to empty dict when form is invalid
- Use f-strings instead of string concatenation for better readability - Rename 'id' variable to 'identifier' to avoid shadowing builtin - Apply improvements to both orders.py and organizer.py Addresses Sourcery suggestions...
- Add JSON_FIELD_AVAILABLE setting based on database backend (postgresql = True) - Fix checkinlists exporter using old Event.items instead of Event.products - Resolves AttributeError when accessing export functionality
- Changed from if/else validation check to getattr() to preserve partial cleaned_data - This allows useful defaults even when form is partially invalid - Reverted unnecessary 'id' to 'identifier' rename in organizer.py - Renamed 'items' field to 'products' in checkinlists exporter for consistency - Updated form_data['items'] to form_data['products'] reference
…fossasia#1157) * Fix navigation button border radius inconsistency - Added border-radius: 0 to .header-nav class in orga/_layout.css - Makes Talk component navigation buttons match Tickets component style - Ensures consistent sharp corners across all navigation buttons - Maintains visual consistency throughout the platform Fixes fossasia#1156 * Add inset shadow on hover to match Tickets component - Added hover and active states with inset box-shadow - Matches the hover effect from btn-success in Tickets component - Uses rgba(0, 128, 0, 0.25) for green inset shadow * Fix navigation button active state to match Tickets component - Added .header-nav.active state with proper inset shadow - Fixed depth and consistency of hover, active, and current page states - Current page button now has same darker border effect as Tickets - All navigation buttons now have identical visual feedback * Improve CSS: use variables and remove important declarations * Fix CSS indentation formatting * Update app/eventyay/static/orga/css/_layout.css * Update app/eventyay/static/orga/css/_layout.css * Fix navigation buttons: sharp corners and inset shadow to match Tickets component --------- Co-authored-by: Mario Behling <[email protected]>
Currently translated at 100.0% (1568 of 1568 strings) Translation: eventyay/eventyay Translate-URL: https://hosted.weblate.org/projects/eventyay/eventyay/de_FORMAL/
i18n(translations): update localized strings from Weblate
…speaker-acceptance-link-oof Fix Speaker Acceptance Confirmation Link Returning 500 Error
* fix(translations): Add missing languages * add(translation): Change language selection to drop down from check-box * fix(translation): Add changes suggested by ai comments * fix(translation): Updated Ukrainian to use the standard Django code `uk` --------- Co-authored-by: Mario Behling <[email protected]>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideImplements pessimistic row-level locking and post-lock availability validation for voucher redemption to prevent race-condition-based over-redemption in the cart service. Sequence diagram for concurrent voucher redemption with pessimistic lockingsequenceDiagram
actor User1
actor User2
participant CartService1
participant CartService2
participant DB
User1->>CartService1: redeem_voucher(voucher_code)
User2->>CartService2: redeem_voucher(voucher_code)
CartService1->>DB: SELECT voucher FOR UPDATE
activate DB
DB-->>CartService1: locked Voucher row
deactivate DB
CartService2->>DB: SELECT voucher FOR UPDATE
activate DB
DB-->>CartService2: waits (row lock held)
CartService1->>CartService1: compute cart_count, v_avail
CartService1->>CartService1: if v_avail < requested_count: raise CartError
CartService1->>DB: update voucher.redeemed
DB-->>CartService1: commit transaction, release lock
deactivate DB
DB-->>CartService2: locked Voucher row (after lock released)
CartService2->>CartService2: refresh redeemed, compute cart_count, v_avail
CartService2->>CartService2: if v_avail < requested_count: raise CartError
CartService2->>DB: update voucher.redeemed or abort
DB-->>CartService2: commit/rollback
CartService1-->>User1: success or CartError
CartService2-->>User2: success or CartError
Flow diagram for updated _get_voucher_availability voucher locking and validationflowchart TD
A["Start _get_voucher_availability"] --> B["Iterate over voucher_use_diff (voucher, count)"]
B --> C["Lock voucher row: select_for_update by pk"]
C --> D{"voucher.valid_until is not None and voucher.valid_until < now_dt?"}
D -- Yes --> E["Raise CartError voucher_expired"]
D -- No --> F{"voucher.reusable or not voucher.is_valid_for_event(event)?"}
F -- Yes --> G["Raise CartError voucher_invalid_for_event"]
F -- No --> H["Compute redeemed_in_carts excluding ExtendOperation positions"]
H --> I["cart_count = redeemed_in_carts.count"]
I --> J["v_avail = voucher.max_usages - voucher.redeemed - cart_count"]
J --> K{"v_avail < count?"}
K -- Yes --> L["Raise CartError voucher_redeemed (over-redemption prevented)"]
K -- No --> M{"cart_count > 0?"}
M -- Yes --> N["Add voucher to _voucher_depend_on_cart"]
M -- No --> O["Skip dependency tracking for this voucher"]
N --> P["vouchers_ok[voucher] = v_avail"]
O --> P
P --> Q{"More vouchers to process?"}
Q -- Yes --> B
Q -- No --> R["Return vouchers_ok and finish"]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…4.* (fossasia#1401) Updates the requirements on [beautifulsoup4](https://www.crummy.com/software/BeautifulSoup/bs4/) to permit the latest version. --- updated-dependencies: - dependency-name: beautifulsoup4 dependency-version: 4.14.3 dependency-type: direct:production ... Signed-off-by: Mario Behling <[email protected]>
…sia#1400) Updates the requirements on [celery](https://github.com/celery/celery) to permit the latest version. - [Release notes](https://github.com/celery/celery/releases) - [Changelog](https://github.com/celery/celery/blob/main/Changelog.rst) - [Commits](celery/celery@v5.4.0rc1...v5.6.0) --- updated-dependencies: - dependency-name: celery dependency-version: 5.6.0 dependency-type: direct:production ... Signed-off-by: Mario Behling <[email protected]>
…rder (fossasia#1375) Co-authored-by: Mario Behling <[email protected]>
mariobehling
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docker build is failing. Please check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a race condition in voucher redemption that allowed multiple concurrent requests to over-redeem vouchers beyond their max_usages limit. The fix implements pessimistic locking using Django's select_for_update() to ensure atomic validation and redemption of vouchers.
Key Changes:
- Replaced
refresh_from_db()withselect_for_update().get()to acquire row-level locks before validation - Added explicit availability check after lock acquisition to prevent over-redemption
- Ensures thread-safe voucher validation within database transactions
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
It was due to some network issue... |
Fix: Voucher Redemption Race Condition
Fix voucher redemption race condition using pessimistic locking
Problem:
Solution:
Fixes #1398
Summary by Sourcery
Prevent race conditions during voucher redemption by validating availability under a database row lock.
Bug Fixes:
Enhancements: