diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d8574f27..66b77d383 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,18 @@ jobs: python-version: "3.11" sonar: false junit-xml-upload: false + - env: py310-django4 + python-version: "3.10" + sonar: false + junit-xml-upload: false + - env: py311-django4 + python-version: "3.11" + sonar: false + junit-xml-upload: false + - env: py312-django4 + python-version: "3.12" + sonar: false + junit-xml-upload: false steps: - uses: actions/checkout@v4 with: diff --git a/ansible_base/rbac/migrations/0009_objectrole_help_text_change.py b/ansible_base/rbac/migrations/0009_objectrole_help_text_change.py new file mode 100644 index 000000000..bffb59782 --- /dev/null +++ b/ansible_base/rbac/migrations/0009_objectrole_help_text_change.py @@ -0,0 +1,42 @@ +# Generated by Django 5.2.7 on 2025-10-06 13:50 +# +# This migration updates the help_text for the `users` and `teams` ManyToManyField +# on the ObjectRole model to add trailing periods for consistency. +# +# WHY DJANGO 5 CREATES THIS BUT DJANGO 4 DOES NOT: +# +# Django 4's migration detector has a bug/limitation where it does not properly detect +# help_text changes on ManyToManyField instances that use custom `through` tables. +# The help_text was updated in the model code (ansible_base/rbac/models/role.py:518, 525) +# to add trailing periods, but Django 4's makemigrations did not detect this change. +# +# Django 5 improved its field state serialization and comparison logic for ManyToManyFields, +# particularly for fields with through tables, and now properly detects these help_text changes. +# +# This is a harmless migration that only updates field metadata (help_text) and does not +# modify the database schema or affect data. + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('dab_rbac', '0008_remote_permissions_cleanup'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + migrations.swappable_dependency(settings.ANSIBLE_BASE_TEAM_MODEL), + ] + + operations = [ + migrations.AlterField( + model_name='objectrole', + name='teams', + 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), + ), + migrations.AlterField( + model_name='objectrole', + name='users', + 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), + ), + ] diff --git a/ansible_base/resource_registry/fields.py b/ansible_base/resource_registry/fields.py index 6ffcbc994..350a945df 100644 --- a/ansible_base/resource_registry/fields.py +++ b/ansible_base/resource_registry/fields.py @@ -13,9 +13,10 @@ class CustomForwardOneToOneDescriptor(ForwardOneToOneDescriptor): def get_queryset(self, **hints): return self.field.remote_field.model._base_manager.db_manager(hints=hints).filter(content_type=ContentType.objects.get_for_model(self.field.model)) - def get_prefetch_queryset(self, instances, queryset=None): - if not queryset: - queryset = self.get_queryset() + def get_prefetch_querysets(self, instances, querysets=None): + if querysets and len(querysets) != 1: + raise ValueError("querysets argument of get_prefetch_querysets() should have a length of 1.") + queryset = querysets[0] if querysets else self.get_queryset() queryset._add_hints(instance=instances[0]) query = models.Q.create( @@ -47,6 +48,12 @@ def get_prefetch_queryset(self, instances, queryset=None): False, ) + def get_prefetch_queryset(self, instances, queryset=None): + # Django 4 compatibility: renamed to get_prefetch_querysets in Django 5 + if queryset is None: + return self.get_prefetch_querysets(instances) + return self.get_prefetch_querysets(instances, [queryset]) + class AnsibleResourceField(models.ForeignObject): """ diff --git a/pyproject.toml b/pyproject.toml index 639fc48ce..7246936a3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -102,8 +102,11 @@ legacy_tox_ini = """ py310 py311 py311sqlite + py310-django4 + py311-django4 + py312-django4 labels = - test = py312, py310, py311, py311sqlite, check + test = py312, py310, py311, py311sqlite, py310-django4, py311-django4, py312-django4, check lint = flake8, black, isort [testenv] @@ -128,6 +131,30 @@ legacy_tox_ini = """ setenv = DJANGO_SETTINGS_MODULE = test_app.sqlite3settings + [testenv:py310-django4] + deps = + -r{toxinidir}/requirements/requirements_all.txt + -r{toxinidir}/requirements/requirements_dev.txt + commands_pre = + python -m pip install --upgrade "Django>=4.2.21,<4.3.0" + docker = db + + [testenv:py311-django4] + deps = + -r{toxinidir}/requirements/requirements_all.txt + -r{toxinidir}/requirements/requirements_dev.txt + commands_pre = + python -m pip install --upgrade "Django>=4.2.21,<4.3.0" + docker = db + + [testenv:py312-django4] + deps = + -r{toxinidir}/requirements/requirements_all.txt + -r{toxinidir}/requirements/requirements_dev.txt + commands_pre = + python -m pip install --upgrade "Django>=4.2.21,<4.3.0" + docker = db + [docker:db] dockerfile = {toxinidir}/tools/dev_postgres/Dockerfile expose = diff --git a/requirements/requirements.in b/requirements/requirements.in index a9a6767b9..7aba5641a 100644 --- a/requirements/requirements.in +++ b/requirements/requirements.in @@ -3,7 +3,7 @@ # if you are add a new feature which requires dependencies they should be in a separate requirements_.in file # cryptography -Django>=4.2.21,<4.3.0 # CVE-2024-45230, CVE-2024-56374 +Django>=4.2.21,<6.0 djangorestframework django-crum inflection diff --git a/requirements/requirements_all.txt b/requirements/requirements_all.txt index 6e5317289..ad8006457 100644 --- a/requirements/requirements_all.txt +++ b/requirements/requirements_all.txt @@ -24,7 +24,7 @@ defusedxml==0.8.0rc2 # via # python3-openid # social-auth-core -django==4.2.21 +django==5.2.7 # via # -r requirements/requirements.in # channels diff --git a/requirements/requirements_dev.txt b/requirements/requirements_dev.txt index 1b651c8e4..7a8463633 100644 --- a/requirements/requirements_dev.txt +++ b/requirements/requirements_dev.txt @@ -1,7 +1,7 @@ ansible # Used in build process to generate some configs black==25.1.0 # Linting tool, if changed update pyproject.toml as well build -django==4.2.21 +django==5.2.7 django-debug-toolbar django-extensions djangorestframework diff --git a/requirements/updater.sh b/requirements/updater.sh index 7c48c7428..bf5e6557f 100755 --- a/requirements/updater.sh +++ b/requirements/updater.sh @@ -3,6 +3,14 @@ set -ue PYTHON=python3.11 +# Ensure script is run from the requirements/ directory +if [[ "$(basename $(pwd))" != "requirements" ]]; then + echo "ERROR: This script must be run from the requirements/ directory" + echo "Current directory: $(pwd)" + echo "Please run: cd requirements && ./updater.sh [run|upgrade]" + exit 1 +fi + for FILE in requirements.in requirements_all.txt ; do if [ ! -f ${FILE} ] ; then touch ${FILE} diff --git a/test_app/tests/resource_registry/models/test_resource_field.py b/test_app/tests/resource_registry/models/test_resource_field.py index b3ac4bd3f..194bffbf6 100644 --- a/test_app/tests/resource_registry/models/test_resource_field.py +++ b/test_app/tests/resource_registry/models/test_resource_field.py @@ -1,8 +1,8 @@ import pytest from django.contrib.contenttypes.models import ContentType -from ansible_base.resource_registry.models import Resource -from test_app.models import Organization +from ansible_base.resource_registry.models import Resource, ResourceType +from test_app.models import Inventory, Organization @pytest.mark.django_db @@ -100,3 +100,91 @@ def test_resource_field_filtering(organization): org = Organization.objects.get(resource__name=organization.name) assert org.resource.pk == resource.pk + + +@pytest.mark.django_db +def test_resource_field_prefetch_related_across_foreign_key(organization, organization_1, organization_2, django_assert_num_queries): + """ + Generated by Claude Code (claude-sonnet-4-5@20250929) + Test that prefetch_related works across a ForeignKey to a model with a resource field. + This tests prefetching organization__resource on Inventory objects. + """ + # Create inventory objects linked to organizations + Inventory.objects.create(name="Inventory 1", organization=organization) + Inventory.objects.create(name="Inventory 2", organization=organization_1) + Inventory.objects.create(name="Inventory 3", organization=organization_2) + + org_ctype = ContentType.objects.get_for_model(Organization) + + # Prefetch organization__resource should result in 3 queries: + # 1. Fetch all Inventory objects + # 2. Fetch all related Organization objects + # 3. Fetch all related Resource objects + with django_assert_num_queries(3) as captured: + inventory_qs = list(Inventory.objects.prefetch_related("organization__resource").all()) + + # Verify the queries were as expected + assert "test_app_inventory" in captured[0]["sql"] + assert "test_app_organization" in captured[1]["sql"] + assert "dab_resource_registry_resource" in captured[2]["sql"] + + assert len(inventory_qs) == 3 + + # Collect resource pks for later verification + resource_pks = {} + with django_assert_num_queries(0): + for inv in inventory_qs: + assert inv.organization is not None + assert inv.organization.resource is not None + # Verify the resource data is correct + assert inv.organization.name == inv.organization.resource.name + assert str(inv.organization.pk) == inv.organization.resource.object_id + resource_pks[inv.organization.pk] = inv.organization.resource.pk + + # Verify the resources match what's in the database + for org_pk, resource_pk in resource_pks.items(): + expected_resource = Resource.objects.get(object_id=org_pk, content_type=org_ctype) + assert resource_pk == expected_resource.pk + + +@pytest.mark.django_db +def test_resource_field_prefetch_resource_type_from_organization(organization, organization_1, organization_2, django_assert_num_queries): + """ + Generated by Claude Code (claude-sonnet-4-5@20250929) + Test that prefetch_related works from Organization through resource to resource_type. + This tests prefetching resource__content_type__resource_type on Organization objects. + """ + org_ctype = ContentType.objects.get_for_model(Organization) + + # Prefetch resource__content_type__resource_type should result in 4 queries: + # 1. Fetch all Organization objects + # 2. Fetch all related Resource objects + # 3. Fetch all related ContentType objects + # 4. Fetch all related ResourceType objects + with django_assert_num_queries(4) as captured: + org_qs = list(Organization.objects.prefetch_related("resource__content_type__resource_type").all()) + + # Verify the queries were as expected + assert "test_app_organization" in captured[0]["sql"] + assert "dab_resource_registry_resource" in captured[1]["sql"] + assert "django_content_type" in captured[2]["sql"] + assert "dab_resource_registry_resourcetype" in captured[3]["sql"] + + assert len(org_qs) > 2 + + # Collect resource type data for later verification + resource_type_data = {} + with django_assert_num_queries(0): + for org in org_qs: + assert org.resource is not None + assert org.resource.content_type is not None + assert org.resource.content_type.resource_type is not None + # Verify the resource type is correct + resource_type = org.resource.content_type.resource_type + assert resource_type.content_type == org_ctype + resource_type_data[org.pk] = resource_type.pk + + # Verify the resource types match what's in the database + expected_resource_type = ResourceType.objects.get(content_type=org_ctype) + for org_pk, resource_type_pk in resource_type_data.items(): + assert resource_type_pk == expected_resource_type.pk