-
Notifications
You must be signed in to change notification settings - Fork 173
Add concurrent refund race condition test #1498
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: dev
Are you sure you want to change the base?
Changes from all commits
9a0a477
e934e9e
7ead44e
2452ba0
1c18298
dd1040c
90d6025
ad7e31b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| # API Tests |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| """ | ||
| Test fixtures for API tests. | ||
|
|
||
| These fixtures provide common test data for testing the API endpoints. | ||
| """ | ||
| from datetime import datetime, timezone | ||
| from decimal import Decimal | ||
|
|
||
| import pytest | ||
| from django.test import utils | ||
| from django_scopes import scopes_disabled | ||
| from rest_framework.test import APIClient | ||
|
|
||
| from eventyay.base.models import ( | ||
| Device, | ||
| Event, | ||
| Order, | ||
| OrderFee, | ||
| OrderPayment, | ||
| OrderPosition, | ||
| Organizer, | ||
| Team, | ||
| User, | ||
| ) | ||
| from eventyay.base.models.devices import generate_api_token | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def client(): | ||
| """Return an API test client.""" | ||
| return APIClient() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| @scopes_disabled() | ||
| def organizer(): | ||
| """Create a test organizer.""" | ||
| return Organizer.objects.create(name='Test Organizer', slug='test-organizer') | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| @scopes_disabled() | ||
| def event(organizer): | ||
| """Create a test event.""" | ||
| e = Event.objects.create( | ||
| organizer=organizer, | ||
| name='Test Event', | ||
| slug='test-event', | ||
| date_from=datetime(2025, 12, 27, 10, 0, 0, tzinfo=timezone.utc), | ||
| plugins='eventyay.plugins.banktransfer,eventyay.plugins.ticketoutputpdf', | ||
| is_public=True, | ||
| ) | ||
| e.settings.timezone = 'UTC' | ||
| return e | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| @scopes_disabled() | ||
| def team(organizer): | ||
| """Create a test team with full permissions.""" | ||
| return Team.objects.create( | ||
| organizer=organizer, | ||
| name='Test Team', | ||
| can_change_teams=True, | ||
| can_manage_gift_cards=True, | ||
| can_change_items=True, | ||
| can_create_events=True, | ||
| can_change_event_settings=True, | ||
| can_change_vouchers=True, | ||
| can_view_vouchers=True, | ||
| can_change_orders=True, | ||
| can_change_organizer_settings=True, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| @scopes_disabled() | ||
| def device(organizer): | ||
| """Create a test API device.""" | ||
| return Device.objects.create( | ||
| organizer=organizer, | ||
| all_events=True, | ||
| name='Test Device', | ||
| initialized=datetime.now(timezone.utc), | ||
| api_token=generate_api_token(), | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def user(): | ||
| """Create a test user.""" | ||
| return User.objects.create_user('[email protected]', 'testpassword') | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| @scopes_disabled() | ||
| def user_client(client, team, user): | ||
| """Return an API client authenticated as a user.""" | ||
| team.can_view_orders = True | ||
| team.can_view_vouchers = True | ||
| team.all_events = True | ||
| team.save() | ||
| team.members.add(user) | ||
| client.force_authenticate(user=user) | ||
| return client | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| @scopes_disabled() | ||
| def token_client(client, team): | ||
| """Return an API client authenticated with a team token.""" | ||
| team.can_view_orders = True | ||
| team.can_change_orders = True | ||
| team.can_view_vouchers = True | ||
| team.all_events = True | ||
| team.save() | ||
| t = team.tokens.create(name='Test Token') | ||
| client.credentials(HTTP_AUTHORIZATION='Token ' + t.token) | ||
| return client | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def device_client(client, device): | ||
| """Return an API client authenticated as a device.""" | ||
| client.credentials(HTTP_AUTHORIZATION='Device ' + device.api_token) | ||
| return client | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| @scopes_disabled() | ||
| def item(event): | ||
| """Create a test product/item.""" | ||
| return event.products.create(name='Test Ticket', default_price=Decimal('100.00')) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| @scopes_disabled() | ||
| def taxrule(event): | ||
| """Create a test tax rule.""" | ||
| return event.tax_rules.create(name='VAT', rate=Decimal('19.00')) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| @scopes_disabled() | ||
| def order(event, item, taxrule): | ||
| """Create a test order with a payment.""" | ||
| o = Order.objects.create( | ||
| code='TEST01', | ||
| event=event, | ||
| email='[email protected]', | ||
| status=Order.STATUS_PENDING, | ||
| secret='testsecretkey12345', | ||
| datetime=datetime(2025, 1, 1, 10, 0, 0, tzinfo=timezone.utc), | ||
| expires=datetime(2025, 1, 10, 10, 0, 0, tzinfo=timezone.utc), | ||
| total=Decimal('100.00'), | ||
| locale='en', | ||
| ) | ||
|
|
||
| # Create a confirmed payment | ||
| o.payments.create( | ||
| provider='manual', | ||
| state=OrderPayment.PAYMENT_STATE_CONFIRMED, | ||
| amount=Decimal('100.00'), | ||
| payment_date=datetime(2025, 1, 1, 10, 0, 0, tzinfo=timezone.utc), | ||
| ) | ||
|
|
||
| # Create a fee | ||
| o.fees.create( | ||
| fee_type=OrderFee.FEE_TYPE_PAYMENT, | ||
| value=Decimal('0.00'), | ||
| tax_rate=Decimal('0.00'), | ||
| tax_value=Decimal('0.00'), | ||
| ) | ||
|
|
||
| # Create an order position | ||
| OrderPosition.objects.create( | ||
| order=o, | ||
| product=item, | ||
| variation=None, | ||
| price=Decimal('100.00'), | ||
| attendee_name_parts={'full_name': 'Test Customer', '_scheme': 'full'}, | ||
| secret='testpositionsecret123', | ||
| pseudonymization_id='TESTPSEUDO', | ||
| ) | ||
|
|
||
| return o | ||
|
|
||
|
|
||
| # Apply scopes_disabled to database setup | ||
| utils.setup_databases = scopes_disabled()(utils.setup_databases) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,104 @@ | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| Test for concurrent refund race condition prevention. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Validates that the select_for_update() fix in OrderPaymentViewSet.refund() | ||||||||||||||||||||||||||
| prevents multiple concurrent refunds from exceeding the payment amount. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Related: app/eventyay/api/views/order.py lines 1251-1276 | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||
| import threading | ||||||||||||||||||||||||||
| from decimal import Decimal | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import pytest | ||||||||||||||||||||||||||
| from django.db import connection | ||||||||||||||||||||||||||
| from django.test import Client | ||||||||||||||||||||||||||
| from django_scopes import scopes_disabled | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from eventyay.base.models.orders import OrderPayment | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| @pytest.mark.django_db(transaction=True) | ||||||||||||||||||||||||||
| class TestConcurrentRefunds: | ||||||||||||||||||||||||||
| """Test suite for concurrent refund race condition prevention.""" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def test_concurrent_refunds_race_condition_prevented( | ||||||||||||||||||||||||||
| self, token_client, organizer, event, order | ||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| Test that concurrent refund requests are properly serialized. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Two threads attempt to refund $80 each from a $100 payment. | ||||||||||||||||||||||||||
| With select_for_update(): only one succeeds, preventing double refund. | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| # Set up a payment with confirmed state | ||||||||||||||||||||||||||
| with scopes_disabled(): | ||||||||||||||||||||||||||
| payment = order.payments.first() | ||||||||||||||||||||||||||
| payment.amount = Decimal('100.00') | ||||||||||||||||||||||||||
| payment.state = OrderPayment.PAYMENT_STATE_CONFIRMED | ||||||||||||||||||||||||||
| payment.save() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| results = [] | ||||||||||||||||||||||||||
| errors = [] | ||||||||||||||||||||||||||
| auth_header = token_client.defaults.get('HTTP_AUTHORIZATION') | ||||||||||||||||||||||||||
| start_barrier = threading.Barrier(2) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def make_refund_request(amount, barrier): | ||||||||||||||||||||||||||
| """Make a refund request in a separate thread.""" | ||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||
| barrier.wait() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| client = Client() | ||||||||||||||||||||||||||
| client.defaults['HTTP_AUTHORIZATION'] = auth_header | ||||||||||||||||||||||||||
| response = client.post( | ||||||||||||||||||||||||||
| f'/api/v1/organizers/{organizer.slug}/events/{event.slug}/' | ||||||||||||||||||||||||||
| f'orders/{order.code}/payments/{payment.local_id}/refund/', | ||||||||||||||||||||||||||
| data=json.dumps({'amount': str(amount)}), | ||||||||||||||||||||||||||
| content_type='application/json', | ||||||||||||||||||||||||||
|
Comment on lines
+52
to
+57
|
||||||||||||||||||||||||||
| client.defaults['HTTP_AUTHORIZATION'] = auth_header | |
| response = client.post( | |
| f'/api/v1/organizers/{organizer.slug}/events/{event.slug}/' | |
| f'orders/{order.code}/payments/{payment.local_id}/refund/', | |
| data=json.dumps({'amount': str(amount)}), | |
| content_type='application/json', | |
| response = client.post( | |
| f'/api/v1/organizers/{organizer.slug}/events/{event.slug}/' | |
| f'orders/{order.code}/payments/{payment.local_id}/refund/', | |
| data=json.dumps({'amount': str(amount)}), | |
| content_type='application/json', | |
| HTTP_AUTHORIZATION=auth_header, |
ArnavBallinCode marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 19, 2025
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 f-string contains double curly braces which will be printed literally instead of interpolating the variables. The braces inside an f-string should be single to enable variable interpolation.
| f"{{thread1 alive: {{thread1.is_alive()}}}}, thread2 alive: {{thread2.is_alive()}}}}" | |
| f"thread1 alive: {thread1.is_alive()}, thread2 alive: {thread2.is_alive()}" |
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 test uses the 'manual' payment provider which does not support refunds by default. The BasePaymentProvider.payment_partial_refund_supported method returns False, and ManualPayment doesn't override it. This will cause both refund requests to fail with a 400 error at lines 1280-1283 of api/views/order.py (since the test attempts partial refunds of 80 from a 100 payment), rather than testing the intended race condition behavior. Consider using a payment provider that supports partial refunds, such as creating a test-specific payment provider that overrides payment_partial_refund_supported to return True.