fix: do not call .filter() on list before converting to queryset (#16752)

* fix: do not call .filter() on list before converting to queryset

* fix: optimize slicing of queryset

* fix: preserve existing filters and ordering on the queryset

awx/main/models/inventory.py

@@ -282,21 +282,17 @@ class Inventory(CommonModelNameNotUnique, RelatedJobsMixin, OpaQueryPathMixin):
 
     def get_sliced_hosts(self, host_queryset, slice_number, slice_count):
         """
-        Returns a slice of Hosts given a slice number and total slice count, or
+        Returns a queryset of Hosts for a given slice number and total slice
-        the original queryset if slicing is not requested.
+        count, or the original queryset if slicing is not requested.
-
-        NOTE: If slicing is performed, this will return a List[Host] with the
-        resulting slice. If slicing is not performed it will return the
-        original queryset (not evaluating it or forcing it to a list). This
-        puts the burden on the caller to check the resulting type. This is
-        non-ideal because it's easy to get wrong, but I think the only way
-        around it is to force the queryset which has memory implications for
-        large inventories.
         """
-
         if slice_count > 1 and slice_number > 0:
             offset = slice_number - 1
-            host_queryset = host_queryset[offset::slice_count]
+            # Fetch only PKs to avoid loading the full host records into memory
+            # before slicing the query set. Unfortunately it seems that there is
+            # no good way to slice a queryset without converting it to a list first
+            all_pks = list(host_queryset.values_list('pk', flat=True))
+            sliced_pks = all_pks[offset::slice_count]
+            host_queryset = host_queryset.filter(pk__in=sliced_pks)
         return host_queryset
 
     def get_script_data(self, hostvars=False, towervars=False, show_all=False, slice_number=1, slice_count=1):

awx/main/tests/functional/tasks/test_fact_cache.py

@@ -221,3 +221,54 @@ class TestConstructedInventoryFactCache:
 
         assert source_host.ansible_facts == {'role': 'web', 'deployed': True}
         assert not constructed_host.ansible_facts, f'Facts were stored on the constructed host: {constructed_host.ansible_facts!r}'
+
+
+@pytest.mark.django_db
+class TestSlicedJobFactCache:
+    """finish_fact_cache must work when get_hosts_for_fact_cache returns a sliced result."""
+
+    def test_sliced_hosts_persist_facts(self, organization, artifacts_dir):
+        """Sliced jobs must persist facts via get_sliced_hosts queryset.
+
+        Regression test for AAP-97902: get_sliced_hosts() used to return a list
+        when slice_count > 1, causing finish_fact_cache() to crash with
+        AttributeError when calling .filter(). get_sliced_hosts() now converts
+        the step-sliced list back to a QuerySet.
+        """
+        inv = Inventory.objects.create(organization=organization, name='slice-inv')
+        hosts = [inv.hosts.create(name=f'host{i}') for i in range(4)]
+
+        for h in hosts:
+            h.ansible_facts = {'old': True}
+            h.ansible_facts_modified = now()
+            h.save(update_fields=['ansible_facts', 'ansible_facts_modified'])
+
+        host_qs = inv.hosts.all()
+        sliced_qs = inv.get_sliced_hosts(host_qs, slice_number=1, slice_count=2)
+
+        # get_sliced_hosts must return a QuerySet, not a list
+        assert hasattr(sliced_qs, 'filter'), 'get_sliced_hosts must return a QuerySet'
+
+        sliced_hosts = list(sliced_qs)
+        start_fact_cache(sliced_qs, artifacts_dir=artifacts_dir, timeout=0, inventory_id=inv.id)
+
+        # Simulate ansible writing new facts for each host in the slice
+        for h in sliced_hosts:
+            fact_file = os.path.join(artifacts_dir, 'fact_cache', h.name)
+            future = time.time() + 60
+            with open(fact_file, 'w') as f:
+                json.dump({'updated': True, 'host': h.name}, f)
+            os.utime(fact_file, (future, future))
+
+        # This would crash with AttributeError before the fix
+        finish_fact_cache(sliced_qs, artifacts_dir=artifacts_dir, inventory_id=inv.id)
+
+        for h in hosts:
+            h.refresh_from_db()
+
+        sliced_names = {h.name for h in sliced_hosts}
+        for h in hosts:
+            if h.name in sliced_names:
+                assert h.ansible_facts.get('updated') is True, f'{h.name} should have updated facts'
+            else:
+                assert h.ansible_facts == {'old': True}, f'{h.name} should be unchanged'