Skip to content

Commit 0275057

Browse files
committed
Get rid of pk=1 bad pattern and test it
1 parent 85a85d7 commit 0275057

File tree

2 files changed

+74
-6
lines changed

2 files changed

+74
-6
lines changed

ansible_base/resource_registry/models/service_identifier.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,15 @@ def service_id():
3030
if settings.DEBUG or "pytest" in sys.argv:
3131
try:
3232
with transaction.atomic():
33-
obj = ServiceID.objects.create(pk=1)
33+
obj = ServiceID.objects.create()
34+
# Check if another process also created one during the race
35+
if ServiceID.objects.count() > 1:
36+
# We lost the race, delete ours and use the other
37+
obj.delete()
38+
obj = ServiceID.objects.first()
3439
except IntegrityError:
3540
# Another thread/process won the race—read it
36-
obj = ServiceID.objects.get(pk=1)
41+
obj = ServiceID.objects.first()
3742
else:
3843
raise RuntimeError('Expected ServiceID to be created in data migrations but was not found')
3944
_service_id = str(obj.pk)

test_app/tests/resource_registry/models/test_service_id.py

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def test_service_id_function_handles_integrity_error():
6262

6363
# Pre-create an object to simulate another process winning the race
6464
# We'll create it before the patched create is called
65-
ServiceID.objects.create(pk=1)
65+
existing_obj = ServiceID.objects.create()
6666

6767
# Delete from cache and reset so service_id() will try to create
6868
sid_module._service_id = None
@@ -75,11 +75,74 @@ def mock_create(*args, **kwargs):
7575

7676
# Ensure pytest is in sys.argv to trigger auto-creation path
7777
with patch.object(sys, 'argv', ['pytest']):
78-
# Mock objects.first() to return None to force the creation path
79-
with patch.object(ServiceID.objects, 'first', return_value=None):
78+
# Mock objects.first() to return None first time, then the actual object
79+
with patch.object(ServiceID.objects, 'first', side_effect=[None, existing_obj]):
8080
with patch.object(ServiceID.objects, 'create', side_effect=mock_create):
8181
result = service_id()
8282

8383
assert result is not None
84-
# The IntegrityError was handled and it fetched the existing record via get(pk=1)
84+
# The IntegrityError was handled and it fetched the existing record via first()
8585
assert ServiceID.objects.count() == 1
86+
87+
88+
@pytest.mark.django_db
89+
def test_service_id_function_cleans_up_duplicate_on_race():
90+
"""
91+
Generated by Claude Sonnet 4.5
92+
Test that service_id() detects when two objects were created in a race and cleans up the duplicate.
93+
"""
94+
import sys
95+
96+
import ansible_base.resource_registry.models.service_identifier as sid_module
97+
98+
sid_module._service_id = None
99+
ServiceID.objects.all().delete()
100+
101+
# Create the "winner" object that another process created
102+
winner_obj = ServiceID.objects.create()
103+
winner_pk = winner_obj.pk
104+
105+
sid_module._service_id = None
106+
107+
# Track what we create and delete
108+
created_obj = None
109+
created_pk = None
110+
deleted_pks = []
111+
112+
# Mock create to bypass save() exists check and track the created object
113+
def mock_create(*args, **kwargs):
114+
nonlocal created_obj, created_pk
115+
import uuid
116+
117+
from django.db import models
118+
119+
created_obj = ServiceID(id=uuid.uuid4())
120+
# Bypass the exists check in save()
121+
models.Model.save(created_obj)
122+
created_pk = created_obj.pk # Capture pk before it gets deleted
123+
return created_obj
124+
125+
# Track deletions
126+
original_delete = ServiceID.delete
127+
128+
def track_delete(self, *args, **kwargs):
129+
deleted_pks.append(self.pk)
130+
return original_delete(self, *args, **kwargs)
131+
132+
with patch.object(sys, 'argv', ['pytest']):
133+
# first() returns None to trigger creation path, then winner_obj for cleanup
134+
with patch.object(ServiceID.objects, 'first', side_effect=[None, winner_obj]):
135+
with patch.object(ServiceID.objects, 'create', side_effect=mock_create):
136+
# count() returns 2 to trigger cleanup, then actual count
137+
with patch.object(ServiceID.objects, 'count', side_effect=[2, 1]):
138+
with patch.object(ServiceID, 'delete', track_delete):
139+
result = service_id()
140+
141+
# Verify our created object was deleted
142+
assert created_obj is not None
143+
assert created_pk in deleted_pks
144+
# Verify we returned the winner's ID
145+
assert result == str(winner_pk)
146+
# Verify only one object remains in the actual database
147+
assert ServiceID.objects.count() == 1
148+
assert ServiceID.objects.filter(pk=winner_pk).exists()

0 commit comments

Comments
 (0)