Code cleanup

Author: AlanCoding

ansible_base/rbac/api/views.py

@@ -38,7 +38,7 @@
 from ..models import DABContentType, DABPermission, get_evaluation_model
 from ..policies import check_content_obj_permission
 from ..remote import RemoteObject, get_resource_prefix
-from ..sync import maybe_reverse_sync_assignment, maybe_reverse_sync_unassignment, delayed_reverse_sync
+from ..sync import maybe_reverse_sync_assignment, maybe_reverse_sync_role_definition, maybe_reverse_sync_unassignment
 from .queries import assignment_qs_user_to_obj, assignment_qs_user_to_obj_perm
 
 
@@ -121,49 +121,33 @@ def _error_if_managed(self, instance):
             raise ValidationError(_('Role is managed by the system'))
 
     def create(self, request, *args, **kwargs):
-        """Create a new RoleDefinition with delayed reverse-sync.
+        """Create a new RoleDefinition with delayed reverse-sync."""
+        from ansible_base.resource_registry.signals.handlers import no_reverse_sync
 
-        This ensures that the RoleDefinition is fully created with
-        permissions before syncing to the resource server.
-        """
-        serializer = self.get_serializer(data=request.data)
-        serializer.is_valid(raise_exception=True)
+        with no_reverse_sync():
+            response = super().create(request, *args, **kwargs)
 
-        # Use delayed sync to avoid the timing issue where post_save fires
-        # before many-to-many permissions are saved
-        with delayed_reverse_sync(None, "create") as ctx:
-            # Create the instance but disable auto-sync
-            instance = serializer.save()
-            # Update the context manager with the actual instance
-            ctx.set_instance(instance)
+        # Manually sync after permissions are fully saved
+        instance = self.get_queryset().get(pk=response.data['id'])
+        maybe_reverse_sync_role_definition(instance, "create")
 
-        headers = self.get_success_headers(serializer.data)
-        return Response(serializer.data, status=201, headers=headers)
+        return response
 
     def update(self, request, *args, **kwargs):
-        """Update a RoleDefinition with delayed reverse-sync.
+        """Update a RoleDefinition with delayed reverse-sync."""
+        from ansible_base.resource_registry.signals.handlers import no_reverse_sync
 
-        This ensures that the RoleDefinition is fully updated with
-        permissions before syncing to the resource server.
-        """
-        partial = kwargs.pop('partial', False)
         instance = self.get_object()
         self._error_if_managed(instance)
 
-        serializer = self.get_serializer(instance, data=request.data, partial=partial)
-        serializer.is_valid(raise_exception=True)
+        with no_reverse_sync():
+            response = super().update(request, *args, **kwargs)
 
-        # Use delayed sync to avoid the timing issue where post_save fires
-        # before many-to-many permissions are saved
-        with delayed_reverse_sync(instance, "update"):
-            serializer.save()
+        # Manually sync after permissions are fully saved
+        instance.refresh_from_db()
+        maybe_reverse_sync_role_definition(instance, "update")
 
-        if getattr(instance, '_prefetched_objects_cache', None):
-            # If 'prefetch_related' has been applied to a queryset, we need to
-            # forcibly invalidate the prefetch cache on the instance.
-            instance._prefetched_objects_cache = {}
-
-        return Response(serializer.data)
+        return response
 
     def perform_update(self, serializer):
         self._error_if_managed(serializer.instance)

ansible_base/rbac/sync.py

@@ -15,30 +15,29 @@
 """
 
 import logging
-from contextlib import contextmanager
 
 logger = logging.getLogger('ansible_base.rbac.sync')
 
 
-def reverse_sync_enabled_all_conditions(assignment):
+def reverse_sync_enabled_all_conditions(instance):
     """This checks for basically all cases we do not reverse sync
-    1. object level flag for skipping the sync
-    2. environment variable to skip sync
-    3. context manager to disable sync
-    4. RESOURCE_SERVER setting not actually set
+    1. global reverse sync enabled flag (including context manager)
+    2. RESOURCE_SERVER setting not actually set
+    3. environment variable to skip sync
+    4. object level flag for skipping the sync
     """
     from ansible_base.resource_registry.apps import _should_reverse_sync
     from ansible_base.resource_registry.signals.handlers import reverse_sync_enabled
     from ansible_base.resource_registry.utils.sync_to_resource_server import should_skip_reverse_sync
 
-    if not _should_reverse_sync():
+    if not reverse_sync_enabled.enabled:
         return False
 
-    if not reverse_sync_enabled.enabled:
+    if not _should_reverse_sync():
         return False
 
-    if should_skip_reverse_sync(assignment):
-        return
+    if should_skip_reverse_sync(instance):
+        return False
 
     return True
 
@@ -63,86 +62,24 @@ def maybe_reverse_sync_unassignment(role_definition, actor, content_object):
     client.sync_unassignment(role_definition, actor, content_object)
 
 
-def _should_reverse_sync(instance) -> bool:
-    """Check if reverse sync should be performed for the given instance.
+def maybe_reverse_sync_role_definition(instance, action="update"):
+    """Manually trigger reverse-sync for a RoleDefinition if appropriate.
 
-    This checks the same conditions as the signal handler but without
-    relying on the global reverse_sync_enabled state being temporarily disabled.
-    """
-    from ansible_base.resource_registry.apps import _should_reverse_sync as _global_should_sync
-    from ansible_base.resource_registry.utils.sync_to_resource_server import should_skip_reverse_sync
-
-    # Check global RESOURCE_SERVER configuration
-    if not _global_should_sync():
-        return False
-
-    # Check instance-specific skip conditions
-    if should_skip_reverse_sync(instance):
-        return False
-
-    return True
-
-
-class DelayedReverseSyncContext:
-    """Context class for delayed reverse sync operations."""
-
-    def __init__(self, action="update"):
-        self.action = action
-        self.instance = None
-
-    def set_instance(self, instance):
-        """Set the instance to sync after the context exits."""
-        self.instance = instance
-
-
-@contextmanager
-def delayed_reverse_sync(instance, action="update"):
-    """Context manager that disables reverse-sync during execution, then manually syncs afterward.
-
-    This is useful for operations where the instance needs to be fully constructed
-    (including many-to-many relationships) before syncing to the resource server.
-
-    This solves the timing issue where post_save fires before many-to-many
-    relations are saved, causing empty permissions to be synced.
+    This should be called after the instance is fully constructed with
+    all many-to-many relationships saved.
 
     Args:
-        instance: The Django model instance to sync (can be None for create operations)
+        instance: The RoleDefinition instance to sync
         action: The action type ("create" or "update")
-
-    Usage:
-        # For updates
-        with delayed_reverse_sync(role_def, "update"):
-            # Perform operations that modify the instance
-            role_def.permissions.set(permissions)
-        # Sync happens here if appropriate
-
-        # For creates
-        ctx = delayed_reverse_sync(None, "create")
-        with ctx:
-            instance = serializer.save()
-            ctx.set_instance(instance)
-        # Sync happens here if appropriate
     """
-    from ansible_base.resource_registry.signals.handlers import no_reverse_sync, sync_to_resource_server_post_save
-
-    # Create a context object that can be updated
-    context = DelayedReverseSyncContext(action)
-    context.instance = instance
-
-    with no_reverse_sync():
-        yield context
-
-    # After the context, check if we should sync and do it manually
-    final_instance = context.instance
-    if final_instance and _should_reverse_sync(final_instance):
-        logger.debug(f"Performing delayed reverse-sync for {final_instance} (action: {action})")
-        # Use the same logic as the post_save signal handler
-        created = (action == "create")
-        sync_to_resource_server_post_save(
-            sender=type(final_instance),
-            instance=final_instance,
-            created=created,
-            update_fields=None
-        )
-    else:
-        logger.debug(f"Skipping delayed reverse-sync for {final_instance} (action: {action})")
+    if not reverse_sync_enabled_all_conditions(instance):
+        logger.debug(f"Skipping reverse-sync for {instance} (action: {action})")
+        return
+
+    logger.debug(f"Performing reverse-sync for {instance} (action: {action})")
+
+    # Use the same logic as the post_save signal handler
+    from ansible_base.resource_registry.signals.handlers import sync_to_resource_server_post_save
+
+    created = action == "create"
+    sync_to_resource_server_post_save(sender=type(instance), instance=instance, created=created, update_fields=None)