Skip unreferenced static files when ingesting edX course archives

learning_resources/etl/edx_shared.py

@@ -15,10 +15,10 @@
 from learning_resources.etl.loaders import load_content_files
 from learning_resources.etl.utils import (
     calc_checksum,
+    excluded_olx_paths,
     get_bucket_by_name,
     get_edx_module_id,
     get_s3_prefix_for_source,
-    staff_only_olx_paths,
     transform_content_files,
 )
 from learning_resources.models import ContentFile, LearningResourceRun
@@ -369,27 +369,33 @@ def sync_edx_course_files(
     )
 
 
-def unpublish_staff_only_content_files(
-    etl_source: str, ids: list[int], keys: list[str]
-) -> int:
+def unpublish_excluded_content_files(
+    etl_source: str, ids: list[int], keys: list[str], *, dry_run: bool = False
+) -> list[dict]:
     """
-    Unpublish (and deindex) content files under staff-only OLX subtrees for the
-    runs matching the given archive keys, without re-extracting anything.
+    Unpublish (and deindex) content files the course does not use — staff-only
+    subtrees, asset manifests and unreferenced static files — for the runs
+    matching the given archive keys, without re-extracting anything.
 
     Args:
         etl_source(str): The edx ETL source
         ids(list of int): list of course ids to process
         keys(list[str]): list of S3 archive keys to search through
+        dry_run(bool): count the rows but leave them published and deindex nothing
 
     Returns:
-        int: number of content files unpublished
+        list of dict: a row per run whose archive excludes content files it has,
+            counting the excluded rows, the ones this call unpublished (or would
+            have, under dry_run) and the run's content files in total. Counts,
+            not paths, so the payload stays small enough to cross the celery
+            result backend for every run at once.
     """
     from learning_resources_search import tasks as search_tasks
     from vector_search import tasks as vector_tasks
 
     bucket = get_bucket_by_name(settings.COURSE_ARCHIVE_BUCKET_NAME)
     run_lookup = build_run_lookup(etl_source, ids)
-    total = 0
+    rows = []
     for key in keys:
         matching_runs = run_lookup.get(extract_run_id_from_key(etl_source, key))
         if not matching_runs:
@@ -408,23 +414,38 @@ def unpublish_staff_only_content_files(
             if olx_path is None:
                 continue
             try:
-                hidden_paths = staff_only_olx_paths(olx_path)
+                excluded_paths = excluded_olx_paths(olx_path)
             except ElementTree.ParseError:
                 log.exception("Malformed OLX in %s, skipping", key)
                 continue
-            hidden_keys = {get_edx_module_id(str(path), run) for path in hidden_paths}
-        if not hidden_keys:
+            excluded_keys = {
+                get_edx_module_id(str(path), run) for path in excluded_paths
+            }
+        if not excluded_keys:
             continue
         # scoped to this run: keys embed the run_id, but never rely on that alone
-        hidden_files = ContentFile.objects.filter(run=run, key__in=hidden_keys)
-        unpublished = hidden_files.filter(published=True).update(published=False)
-        total += unpublished
-        log.info(
-            "Unpublished %d staff-only content files for %s", unpublished, run.run_id
-        )
-        # dispatched whenever hidden rows exist, not only when this call flipped
-        # them, so a re-run after a failed deindex task cleans up the indexes
-        if hidden_files.exists():
+        excluded_files = ContentFile.objects.filter(run=run, key__in=excluded_keys)
+        excluded = excluded_files.count()
+        if not excluded:
+            continue
+        if dry_run:
+            unpublished = excluded_files.filter(published=True).count()
+        else:
+            unpublished = excluded_files.filter(published=True).update(published=False)
+            log.info(
+                "Unpublished %d excluded content files for %s", unpublished, run.run_id
+            )
+            # dispatched whenever excluded rows exist, not only when this call
+            # flipped them, so a re-run after a failed deindex task cleans up
+            # the indexes
             search_tasks.deindex_run_content_files.delay(run.id, unpublished_only=True)
             vector_tasks.remove_unpublished_run_content_files.delay(run.id)
-    return total
+        rows.append(
+            {
+                "run_id": run.run_id,
+                "excluded": excluded,
+                "unpublished": unpublished,
+                "total": ContentFile.objects.filter(run=run).count(),
+            }
+        )
+    return rows

learning_resources/etl/edx_shared_test.py

@@ -19,7 +19,7 @@
     normalize_run_id,
     process_course_archive,
     sync_edx_course_files,
-    unpublish_staff_only_content_files,
+    unpublish_excluded_content_files,
 )
 from learning_resources.etl.utils import get_edx_module_id, get_s3_prefix_for_source
 from learning_resources.factories import (
@@ -1670,6 +1670,8 @@ def _staff_only_archive(tmp_path) -> Path:
         "vertical/v_staff.xml": '<vertical><html url_name="h_staff"/></vertical>',
         "html/h_staff.xml": '<html filename="h_staff"/>',
         "html/h_staff.html": "<p>staff</p>",
+        # nothing links this, so it is from an earlier offering
+        "static/stale_syllabus.pdf": "stale",
     }
     for rel, text in files.items():
         (olx / rel).parent.mkdir(parents=True, exist_ok=True)
@@ -1712,7 +1714,7 @@ def mock_deindex_tasks(mocker):
     )
 
 
-def test_unpublish_staff_only_content_files(staff_only_run, mock_deindex_tasks):
+def test_unpublish_excluded_content_files(staff_only_run, mock_deindex_tasks):
     """Only the matching run's staff-only content files are unpublished and deindexed"""
     run = staff_only_run.run
     other_run = LearningResourceRunFactory.create(
@@ -1728,11 +1730,11 @@ def test_unpublish_staff_only_content_files(staff_only_run, mock_deindex_tasks):
     for cf_key in run_keys.values():
         ContentFileFactory.create(run=other_run, key=cf_key, published=True)
 
-    unpublished = unpublish_staff_only_content_files(
+    rows = unpublish_excluded_content_files(
         staff_only_run.source, [staff_only_run.course.id], [staff_only_run.key]
     )
 
-    assert unpublished == 1
+    assert [row["unpublished"] for row in rows] == [1]
     assert not ContentFile.objects.filter(
         run=run, key=run_keys["html/h_staff.xml"], published=True
     ).exists()
@@ -1744,7 +1746,7 @@ def test_unpublish_staff_only_content_files(staff_only_run, mock_deindex_tasks):
     mock_deindex_tasks.qdrant.assert_called_once_with(run.id)
 
 
-def test_unpublish_staff_only_content_files_nothing_hidden(
+def test_unpublish_excluded_content_files_nothing_hidden(
     staff_only_run, mock_deindex_tasks
 ):
     """No deindex tasks are queued when a run has no staff-only content files"""
@@ -1754,20 +1756,20 @@ def test_unpublish_staff_only_content_files_nothing_hidden(
     )
 
     assert (
-        unpublish_staff_only_content_files(
+        unpublish_excluded_content_files(
             staff_only_run.source, [staff_only_run.course.id], [staff_only_run.key]
         )
-        == 0
+        == []
     )
     mock_deindex_tasks.opensearch.assert_not_called()
 
 
-def test_unpublish_staff_only_content_files_malformed_archive(
+def test_unpublish_excluded_content_files_malformed_archive(
     staff_only_run, mock_deindex_tasks, mocker
 ):
     """A malformed archive is skipped without touching its content files"""
     mocker.patch(
-        "learning_resources.etl.edx_shared.staff_only_olx_paths",
+        "learning_resources.etl.edx_shared.excluded_olx_paths",
         side_effect=ElementTree.ParseError("bad"),
     )
     ContentFileFactory.create(
@@ -1777,16 +1779,16 @@ def test_unpublish_staff_only_content_files_malformed_archive(
     )
 
     assert (
-        unpublish_staff_only_content_files(
+        unpublish_excluded_content_files(
             staff_only_run.source, [staff_only_run.course.id], [staff_only_run.key]
         )
-        == 0
+        == []
     )
     assert ContentFile.objects.filter(run=staff_only_run.run, published=True).exists()
     mock_deindex_tasks.opensearch.assert_not_called()
 
 
-def test_unpublish_staff_only_content_files_rerun_redeindexes(
+def test_unpublish_excluded_content_files_rerun_redeindexes(
     staff_only_run, mock_deindex_tasks
 ):
     """A re-run with already-unpublished hidden files still queues the deindex tasks"""
@@ -1795,11 +1797,51 @@ def test_unpublish_staff_only_content_files_rerun_redeindexes(
         run=run, key=get_edx_module_id("course/html/h_staff.xml", run), published=False
     )
 
-    assert (
-        unpublish_staff_only_content_files(
-            staff_only_run.source, [staff_only_run.course.id], [staff_only_run.key]
-        )
-        == 0
+    rows = unpublish_excluded_content_files(
+        staff_only_run.source, [staff_only_run.course.id], [staff_only_run.key]
     )
+
+    # still counted as excluded, but this call had nothing left to flip
+    assert rows == [{"run_id": run.run_id, "excluded": 1, "unpublished": 0, "total": 1}]
     mock_deindex_tasks.opensearch.assert_called_once_with(run.id, unpublished_only=True)
     mock_deindex_tasks.qdrant.assert_called_once_with(run.id)
+
+
+def test_unpublish_excluded_content_files_drops_unreferenced_static(
+    staff_only_run, mock_deindex_tasks
+):
+    """A static file no block refers to is unpublished alongside staff-only files"""
+    run = staff_only_run.run
+    stale_key = get_edx_module_id("course/static/stale_syllabus.pdf", run)
+    ContentFileFactory.create(run=run, key=stale_key, published=True)
+
+    rows = unpublish_excluded_content_files(
+        staff_only_run.source, [staff_only_run.course.id], [staff_only_run.key]
+    )
+
+    assert [row["unpublished"] for row in rows] == [1]
+    assert not ContentFile.objects.filter(
+        run=run, key=stale_key, published=True
+    ).exists()
+
+
+def test_unpublish_excluded_content_files_dry_run(staff_only_run, mock_deindex_tasks):
+    """A dry run counts the rows but changes nothing and deindexes nothing"""
+    run = staff_only_run.run
+    ContentFileFactory.create(
+        run=run,
+        key=get_edx_module_id("course/static/stale_syllabus.pdf", run),
+        published=True,
+    )
+
+    rows = unpublish_excluded_content_files(
+        staff_only_run.source,
+        [staff_only_run.course.id],
+        [staff_only_run.key],
+        dry_run=True,
+    )
+
+    assert [row["unpublished"] for row in rows] == [1]
+    assert ContentFile.objects.filter(run=run, published=True).count() == 1
+    mock_deindex_tasks.opensearch.assert_not_called()
+    mock_deindex_tasks.qdrant.assert_not_called()