refactor: pass metadata bytes through batch, not temp path

Address review feedback round 2:

- Extract metadata_content bytes via extract_wheel_metadata while
  temp file exists, delete temp file immediately, pass bytes through
  the metadata batch instead of a stale file path
- Add metadata_content_to_artifact in utils.py — creates Artifact
  from raw bytes, shared by artifact_to_metadata_artifact and the
  repair batch
- Revert artifact_to_metadata_artifact to original signature (remove
  unused temp_path param)
- Remove redundant test_metadata_repair_batch_boundary

JIRA: PULP-1573

Author: decko

pulp_python/app/tasks/repair.py

@@ -9,10 +9,11 @@
 from django.db.models.query import QuerySet
 from pulp_python.app.models import PythonPackageContent, PythonRepository
 from pulp_python.app.utils import (
-    artifact_to_metadata_artifact,
     artifact_to_python_content_data,
     copy_artifact_to_temp_file,
+    extract_wheel_metadata,
     fetch_json_release_metadata,
+    metadata_content_to_artifact,
     parse_metadata,
 )
 from pulpcore.plugin.models import Artifact, ContentArtifact, ProgressReport
@@ -120,24 +121,27 @@ def repair_metadata(content: QuerySet[PythonPackageContent]) -> tuple[int, set[s
                 .first()
                 .artifact
             )
-            # Copy artifact to temp file once, reuse for both content data and metadata
+            # Copy artifact to temp file once, extract both content data and metadata
             temp_path = copy_artifact_to_temp_file(main_artifact, package.filename)
             try:
                 new_data = artifact_to_python_content_data(
                     package.filename, main_artifact, domain, temp_path=temp_path
                 )
-                total_metadata_repaired += update_metadata_artifact_if_needed(
-                    package,
-                    new_data.get("metadata_sha256"),
-                    main_artifact,
-                    metadata_batch,
-                    pkgs_metadata_not_repaired,
-                )
-                total_repaired += update_package_if_needed(
-                    package, new_data, batch, set_of_update_fields
+                metadata_content = (
+                    extract_wheel_metadata(temp_path) if package.filename.endswith(".whl") else None
                 )
             finally:
                 os.unlink(temp_path)
+            total_metadata_repaired += update_metadata_artifact_if_needed(
+                package,
+                new_data.get("metadata_sha256"),
+                metadata_content,
+                metadata_batch,
+                pkgs_metadata_not_repaired,
+            )
+            total_repaired += update_package_if_needed(
+                package, new_data, batch, set_of_update_fields
+            )
 
         # For on-demand content, we expect that:
         # 1. PythonPackageContent always has correct name and version
@@ -245,7 +249,7 @@ def update_package_if_needed(
 def update_metadata_artifact_if_needed(
     package: PythonPackageContent,
     new_metadata_sha256: str | None,
-    main_artifact: Artifact,
+    metadata_content: bytes | None,
     metadata_batch: list[tuple],
     pkgs_metadata_not_repaired: set[str],
 ) -> int:
@@ -257,7 +261,7 @@ def update_metadata_artifact_if_needed(
     Args:
         package: Package to check for metadata changes.
         new_metadata_sha256: The correct metadata_sha256 extracted from the main artifact, or None.
-        main_artifact: The main package artifact used to generate metadata.
+        metadata_content: Raw metadata bytes extracted from the wheel, or None.
         metadata_batch: List of tuples for batch processing (updated in-place).
         pkgs_metadata_not_repaired: Set of package PKs that failed repair (updated in-place).
 
@@ -274,13 +278,13 @@ def update_metadata_artifact_if_needed(
 
     # Create missing
     if not cas:
-        metadata_batch.append((package, main_artifact))
+        metadata_batch.append((package, metadata_content))
     # Fix existing
     elif new_metadata_sha256 != original_metadata_sha256:
         ca = cas.first()
         metadata_artifact = ca.artifact
         if metadata_artifact is None or (metadata_artifact.sha256 != new_metadata_sha256):
-            metadata_batch.append((package, main_artifact))
+            metadata_batch.append((package, metadata_content))
 
     if len(metadata_batch) == BULK_SIZE:
         not_repaired = _process_metadata_batch(metadata_batch)
@@ -297,16 +301,16 @@ def _process_metadata_batch(metadata_batch: list[tuple]) -> set[str]:
     and their corresponding ContentArtifacts.
 
     Args:
-        metadata_batch: List of (package, main_artifact) tuples.
+        metadata_batch: List of (package, metadata_content) tuples.
 
     Returns:
         Set of package PKs for which metadata artifacts could not be created.
     """
     not_repaired = set()
     content_artifacts = []
 
-    for package, main_artifact in metadata_batch:
-        metadata_artifact = artifact_to_metadata_artifact(package.filename, main_artifact)
+    for package, metadata_content in metadata_batch:
+        metadata_artifact = metadata_content_to_artifact(metadata_content)
         if metadata_artifact:
             ca = ContentArtifact(
                 artifact=metadata_artifact,

pulp_python/app/utils.py

@@ -281,31 +281,34 @@ def artifact_to_python_content_data(filename, artifact, domain=None, temp_path=N
 
 
 def artifact_to_metadata_artifact(
-    filename: str, artifact: Artifact, tmp_dir: str = ".", temp_path: str | None = None
+    filename: str, artifact: Artifact, tmp_dir: str = "."
 ) -> Artifact | None:
     """
     Creates artifact for metadata from the provided wheel artifact.
-
-    If temp_path is provided, uses it instead of copying the artifact to a new temp file.
     """
     if not filename.endswith(".whl"):
         return None
 
-    if temp_path:
-        temp_wheel_path = temp_path
-    else:
-        with tempfile.NamedTemporaryFile(
-            "wb", dir=tmp_dir, suffix=filename, delete=False
-        ) as temp_file:
-            temp_wheel_path = temp_file.name
-            artifact.file.seek(0)
-            shutil.copyfileobj(artifact.file, temp_file)
-            temp_file.flush()
+    with tempfile.NamedTemporaryFile("wb", dir=tmp_dir, suffix=filename, delete=False) as temp_file:
+        temp_wheel_path = temp_file.name
+        artifact.file.seek(0)
+        shutil.copyfileobj(artifact.file, temp_file)
+        temp_file.flush()
 
     metadata_content = extract_wheel_metadata(temp_wheel_path)
     if not metadata_content:
         return None
 
+    return metadata_content_to_artifact(metadata_content, tmp_dir)
+
+
+def metadata_content_to_artifact(metadata_content: bytes, tmp_dir: str = ".") -> Artifact | None:
+    """
+    Creates an Artifact from raw metadata content bytes.
+    """
+    if not metadata_content:
+        return None
+
     with tempfile.NamedTemporaryFile(
         "wb", dir=tmp_dir, suffix=".metadata", delete=False
     ) as temp_md:

pulp_python/tests/functional/api/test_repair.py

@@ -331,55 +331,3 @@ def test_metadata_artifact_repair_endpoint(
     # Check deduplication
     assert len(main_artifact_hrefs) == 4
     assert len(metadata_artifact_hrefs) == 3
-
-
-def test_metadata_repair_batch_boundary(
-    create_content_direct,
-    delete_orphans_pre,
-    download_python_file,
-    monitor_task,
-    move_to_repository,
-    python_bindings,
-    python_repo_factory,
-):
-    """
-    Test that repair_metadata correctly handles packages across batch boundaries.
-
-    Verifies that the batch flush (at BULK_SIZE) does not lose or corrupt metadata
-    for packages processed before and after the flush point.
-    """
-    python_repo = python_repo_factory()
-
-    # Create multiple wheel packages with wrong metadata
-    wheel_files = [
-        ("scipy-1.1.0-cp27-none-win32.whl", "ME"),
-        ("scipy-1.1.0-cp27-none-win_amd64.whl", "ME"),
-        ("scipy-1.1.0-cp27-cp27m-manylinux1_x86_64.whl", "ME"),
-    ]
-    content_hrefs = []
-    for filename, author in wheel_files:
-        url = urljoin(urljoin(PYTHON_FIXTURES_URL, "packages/"), filename)
-        file_path = download_python_file(filename, url)
-        data = {
-            "name": "scipy",
-            "version": "1.1.0",
-            "filename": filename,
-            "author": author,
-            "packagetype": "bdist",
-        }
-        content = create_content_direct(file_path, data)
-        assert content.author == "ME"
-        content_hrefs.append(content.pulp_href)
-
-    move_to_repository(python_repo.pulp_href, content_hrefs)
-
-    # Repair
-    response = python_bindings.RepositoriesPythonApi.repair_metadata(python_repo.pulp_href)
-    monitor_task(response.task)
-
-    # All packages should have repaired metadata (author != "ME")
-    for href in content_hrefs:
-        content = python_bindings.ContentPackagesApi.read(href)
-        assert content.author != "ME", f"Package {content.filename} was not repaired"
-        assert content.packagetype == "bdist_wheel"
-        assert content.metadata_sha256 is not None