Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
42 changes: 42 additions & 0 deletions ansible_base/rbac/migrations/0009_objectrole_help_text_change.py
Original file line number Diff line number Diff line change
@@ -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),
),
]
13 changes: 10 additions & 3 deletions ansible_base/resource_registry/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Prefetch Queryset Compatibility Issue

The get_prefetch_queryset compatibility method changed its check from if not queryset to if queryset is None. This alters behavior when a falsy but non-None value, like an empty list, is passed as queryset. Instead of generating a default queryset as before, the method now passes the falsy value directly, which can lead to an empty queryset being used where a default was expected, potentially breaking prefetch functionality.

Fix in Cursor Fix in Web



class AnsibleResourceField(models.ForeignObject):
"""
Expand Down
29 changes: 28 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# if you are add a new feature which requires dependencies they should be in a separate requirements_<feature>.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
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements_dev.txt
Original file line number Diff line number Diff line change
@@ -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
Expand Down
8 changes: 8 additions & 0 deletions requirements/updater.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
92 changes: 90 additions & 2 deletions test_app/tests/resource_registry/models/test_resource_field.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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