Skip to content

Commit f31b1f2

Browse files
authored
AAP-51350 Support Django 5.2 as well as Django 4.y (#855)
Get DAB CI passing with Django 5 & 4 for the compatibility phase.
1 parent eae6509 commit f31b1f2

File tree

9 files changed

+217
-9
lines changed

9 files changed

+217
-9
lines changed

.github/workflows/ci.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,18 @@ jobs:
3434
python-version: "3.11"
3535
sonar: false
3636
junit-xml-upload: false
37+
- env: py310-django4
38+
python-version: "3.10"
39+
sonar: false
40+
junit-xml-upload: false
41+
- env: py311-django4
42+
python-version: "3.11"
43+
sonar: false
44+
junit-xml-upload: false
45+
- env: py312-django4
46+
python-version: "3.12"
47+
sonar: false
48+
junit-xml-upload: false
3749
steps:
3850
- uses: actions/checkout@v4
3951
with:
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Generated by Django 5.2.7 on 2025-10-06 13:50
2+
#
3+
# This migration updates the help_text for the `users` and `teams` ManyToManyField
4+
# on the ObjectRole model to add trailing periods for consistency.
5+
#
6+
# WHY DJANGO 5 CREATES THIS BUT DJANGO 4 DOES NOT:
7+
#
8+
# Django 4's migration detector has a bug/limitation where it does not properly detect
9+
# help_text changes on ManyToManyField instances that use custom `through` tables.
10+
# The help_text was updated in the model code (ansible_base/rbac/models/role.py:518, 525)
11+
# to add trailing periods, but Django 4's makemigrations did not detect this change.
12+
#
13+
# Django 5 improved its field state serialization and comparison logic for ManyToManyFields,
14+
# particularly for fields with through tables, and now properly detects these help_text changes.
15+
#
16+
# This is a harmless migration that only updates field metadata (help_text) and does not
17+
# modify the database schema or affect data.
18+
19+
from django.conf import settings
20+
from django.db import migrations, models
21+
22+
23+
class Migration(migrations.Migration):
24+
25+
dependencies = [
26+
('dab_rbac', '0008_remote_permissions_cleanup'),
27+
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
28+
migrations.swappable_dependency(settings.ANSIBLE_BASE_TEAM_MODEL),
29+
]
30+
31+
operations = [
32+
migrations.AlterField(
33+
model_name='objectrole',
34+
name='teams',
35+
field=models.ManyToManyField(help_text='Teams or groups who have access to the permissions defined by this object role.', related_name='has_roles', through='dab_rbac.RoleTeamAssignment', through_fields=('object_role', 'team'), to=settings.ANSIBLE_BASE_TEAM_MODEL),
36+
),
37+
migrations.AlterField(
38+
model_name='objectrole',
39+
name='users',
40+
field=models.ManyToManyField(help_text='Users who have access to the permissions defined by this object role.', related_name='has_roles', through='dab_rbac.RoleUserAssignment', through_fields=('object_role', 'user'), to=settings.AUTH_USER_MODEL),
41+
),
42+
]

ansible_base/resource_registry/fields.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ class CustomForwardOneToOneDescriptor(ForwardOneToOneDescriptor):
1313
def get_queryset(self, **hints):
1414
return self.field.remote_field.model._base_manager.db_manager(hints=hints).filter(content_type=ContentType.objects.get_for_model(self.field.model))
1515

16-
def get_prefetch_queryset(self, instances, queryset=None):
17-
if not queryset:
18-
queryset = self.get_queryset()
16+
def get_prefetch_querysets(self, instances, querysets=None):
17+
if querysets and len(querysets) != 1:
18+
raise ValueError("querysets argument of get_prefetch_querysets() should have a length of 1.")
19+
queryset = querysets[0] if querysets else self.get_queryset()
1920
queryset._add_hints(instance=instances[0])
2021

2122
query = models.Q.create(
@@ -47,6 +48,12 @@ def get_prefetch_queryset(self, instances, queryset=None):
4748
False,
4849
)
4950

51+
def get_prefetch_queryset(self, instances, queryset=None): # NOSONAR
52+
# Django 4 compatibility: renamed to get_prefetch_querysets in Django 5
53+
if queryset is None:
54+
return self.get_prefetch_querysets(instances)
55+
return self.get_prefetch_querysets(instances, [queryset])
56+
5057

5158
class AnsibleResourceField(models.ForeignObject):
5259
"""

pyproject.toml

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,11 @@ legacy_tox_ini = """
102102
py310
103103
py311
104104
py311sqlite
105+
py310-django4
106+
py311-django4
107+
py312-django4
105108
labels =
106-
test = py312, py310, py311, py311sqlite, check
109+
test = py312, py310, py311, py311sqlite, py310-django4, py311-django4, py312-django4, check
107110
lint = flake8, black, isort
108111
109112
[testenv]
@@ -128,6 +131,30 @@ legacy_tox_ini = """
128131
setenv =
129132
DJANGO_SETTINGS_MODULE = test_app.sqlite3settings
130133
134+
[testenv:py310-django4]
135+
deps =
136+
-r{toxinidir}/requirements/requirements_all.txt
137+
-r{toxinidir}/requirements/requirements_dev.txt
138+
commands_pre =
139+
python -m pip install --upgrade "Django>=4.2.21,<4.3.0"
140+
docker = db
141+
142+
[testenv:py311-django4]
143+
deps =
144+
-r{toxinidir}/requirements/requirements_all.txt
145+
-r{toxinidir}/requirements/requirements_dev.txt
146+
commands_pre =
147+
python -m pip install --upgrade "Django>=4.2.21,<4.3.0"
148+
docker = db
149+
150+
[testenv:py312-django4]
151+
deps =
152+
-r{toxinidir}/requirements/requirements_all.txt
153+
-r{toxinidir}/requirements/requirements_dev.txt
154+
commands_pre =
155+
python -m pip install --upgrade "Django>=4.2.21,<4.3.0"
156+
docker = db
157+
131158
[docker:db]
132159
dockerfile = {toxinidir}/tools/dev_postgres/Dockerfile
133160
expose =

requirements/requirements.in

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# if you are add a new feature which requires dependencies they should be in a separate requirements_<feature>.in file
44
#
55
cryptography
6-
Django>=4.2.21,<4.3.0 # CVE-2024-45230, CVE-2024-56374
6+
Django>=4.2.21,<6.0
77
djangorestframework
88
django-crum
99
inflection

requirements/requirements_all.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ defusedxml==0.8.0rc2
2424
# via
2525
# python3-openid
2626
# social-auth-core
27-
django==4.2.21
27+
django==5.2.7
2828
# via
2929
# -r requirements/requirements.in
3030
# channels

requirements/requirements_dev.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
ansible # Used in build process to generate some configs
22
black==25.1.0 # Linting tool, if changed update pyproject.toml as well
33
build
4-
django==4.2.21
4+
django==5.2.7
55
django-debug-toolbar
66
django-extensions
77
djangorestframework

requirements/updater.sh

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,14 @@ set -ue
33

44
PYTHON=python3.11
55

6+
# Ensure script is run from the requirements/ directory
7+
if [[ "$(basename $(pwd))" != "requirements" ]]; then
8+
echo "ERROR: This script must be run from the requirements/ directory"
9+
echo "Current directory: $(pwd)"
10+
echo "Please run: cd requirements && ./updater.sh [run|upgrade]"
11+
exit 1
12+
fi
13+
614
for FILE in requirements.in requirements_all.txt ; do
715
if [ ! -f ${FILE} ] ; then
816
touch ${FILE}

test_app/tests/resource_registry/models/test_resource_field.py

Lines changed: 114 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import pytest
2+
from django import VERSION
23
from django.contrib.contenttypes.models import ContentType
34

4-
from ansible_base.resource_registry.models import Resource
5-
from test_app.models import Organization
5+
from ansible_base.resource_registry.models import Resource, ResourceType
6+
from test_app.models import Inventory, Organization
67

78

89
@pytest.mark.django_db
@@ -100,3 +101,114 @@ def test_resource_field_filtering(organization):
100101

101102
org = Organization.objects.get(resource__name=organization.name)
102103
assert org.resource.pk == resource.pk
104+
105+
106+
@pytest.mark.django_db
107+
def test_resource_field_prefetch_related_across_foreign_key(organization, organization_1, organization_2, django_assert_num_queries):
108+
"""
109+
Generated by Claude Code (claude-sonnet-4-5@20250929)
110+
Test that prefetch_related works across a ForeignKey to a model with a resource field.
111+
This tests prefetching organization__resource on Inventory objects.
112+
"""
113+
# Create inventory objects linked to organizations
114+
Inventory.objects.create(name="Inventory 1", organization=organization)
115+
Inventory.objects.create(name="Inventory 2", organization=organization_1)
116+
Inventory.objects.create(name="Inventory 3", organization=organization_2)
117+
118+
org_ctype = ContentType.objects.get_for_model(Organization)
119+
120+
# Prefetch organization__resource should result in 3 queries:
121+
# 1. Fetch all Inventory objects
122+
# 2. Fetch all related Organization objects
123+
# 3. Fetch all related Resource objects
124+
with django_assert_num_queries(3) as captured:
125+
inventory_qs = list(Inventory.objects.prefetch_related("organization__resource").all())
126+
127+
# Verify the queries were as expected
128+
assert "test_app_inventory" in captured[0]["sql"]
129+
assert "test_app_organization" in captured[1]["sql"]
130+
assert "dab_resource_registry_resource" in captured[2]["sql"]
131+
132+
assert len(inventory_qs) == 3
133+
134+
# Collect resource pks for later verification
135+
resource_pks = {}
136+
with django_assert_num_queries(0):
137+
for inv in inventory_qs:
138+
assert inv.organization is not None
139+
assert inv.organization.resource is not None
140+
# Verify the resource data is correct
141+
assert inv.organization.name == inv.organization.resource.name
142+
assert str(inv.organization.pk) == inv.organization.resource.object_id
143+
resource_pks[inv.organization.pk] = inv.organization.resource.pk
144+
145+
# Verify the resources match what's in the database
146+
for org_pk, resource_pk in resource_pks.items():
147+
expected_resource = Resource.objects.get(object_id=org_pk, content_type=org_ctype)
148+
assert resource_pk == expected_resource.pk
149+
150+
151+
@pytest.mark.django_db
152+
def test_resource_field_prefetch_resource_type_from_organization(organization, organization_1, organization_2, django_assert_num_queries):
153+
"""
154+
Generated by Claude Code (claude-sonnet-4-5@20250929)
155+
Test that prefetch_related works from Organization through resource to resource_type.
156+
This tests prefetching resource__content_type__resource_type on Organization objects.
157+
"""
158+
org_ctype = ContentType.objects.get_for_model(Organization)
159+
160+
# Prefetch resource__content_type__resource_type should result in 4 queries:
161+
# 1. Fetch all Organization objects
162+
# 2. Fetch all related Resource objects
163+
# 3. Fetch all related ContentType objects
164+
# 4. Fetch all related ResourceType objects
165+
with django_assert_num_queries(4) as captured:
166+
org_qs = list(Organization.objects.prefetch_related("resource__content_type__resource_type").all())
167+
168+
# Verify the queries were as expected
169+
assert "test_app_organization" in captured[0]["sql"]
170+
assert "dab_resource_registry_resource" in captured[1]["sql"]
171+
assert "django_content_type" in captured[2]["sql"]
172+
assert "dab_resource_registry_resourcetype" in captured[3]["sql"]
173+
174+
assert len(org_qs) > 2
175+
176+
# Collect resource type data for later verification
177+
resource_type_data = {}
178+
with django_assert_num_queries(0):
179+
for org in org_qs:
180+
assert org.resource is not None
181+
assert org.resource.content_type is not None
182+
assert org.resource.content_type.resource_type is not None
183+
# Verify the resource type is correct
184+
resource_type = org.resource.content_type.resource_type
185+
assert resource_type.content_type == org_ctype
186+
resource_type_data[org.pk] = resource_type.pk
187+
188+
# Verify the resource types match what's in the database
189+
expected_resource_type = ResourceType.objects.get(content_type=org_ctype)
190+
for org_pk, resource_type_pk in resource_type_data.items():
191+
assert resource_type_pk == expected_resource_type.pk
192+
193+
194+
@pytest.mark.skipif(VERSION[0] < 5, reason='get_prefetch_querysets() is only valid for Django 5+')
195+
@pytest.mark.django_db
196+
@pytest.mark.parametrize(
197+
"querysets_arg",
198+
[
199+
pytest.param([Resource.objects.none(), Resource.objects.none()], id="two_querysets"),
200+
pytest.param([Resource.objects.none(), Resource.objects.none(), Resource.objects.none()], id="three_querysets"),
201+
],
202+
)
203+
def test_resource_field_get_prefetch_querysets_invalid_length(organization, querysets_arg):
204+
"""
205+
Generated by Claude Code (claude-sonnet-4-5@20250929)
206+
Test that get_prefetch_querysets raises ValueError when querysets argument has invalid length.
207+
The method expects exactly 1 queryset if querysets is provided, so multiple querysets should raise.
208+
"""
209+
# Get the forward descriptor for the resource field
210+
descriptor = Organization.resource.field.forward_related_accessor_class(Organization.resource.field)
211+
212+
# Call get_prefetch_querysets with invalid querysets argument
213+
with pytest.raises(ValueError, match='querysets argument of get_prefetch_querysets\\(\\) should have a length of 1\\.'):
214+
descriptor.get_prefetch_querysets([organization], querysets=querysets_arg)

0 commit comments

Comments
 (0)