Fix an issue pull-through caching (wasn't saving all metadata)

Assisted-By: claude-opus-4.6

Author: dralley

pulp_rust/app/models.py

@@ -3,6 +3,9 @@
 from logging import getLogger
 
 from django.db import models
+from django_lifecycle import hook, AFTER_CREATE
+
+from pulp_rust.app.utils import extract_cargo_toml, extract_dependencies
 
 from pulpcore.plugin.models import (
     Content,
@@ -106,15 +109,32 @@ def init_from_artifact_and_relative_path(artifact, relative_path):
         Create an unsaved RustContent from a downloaded .crate artifact.
 
         Called by pulpcore's content handler during pull-through caching.
-        Only populates name, version, and checksum -- dependency and feature
-        metadata is served from the upstream sparse index via the proxy.
+        Extracts full metadata (dependencies, features, etc.) from the
+        Cargo.toml inside the .crate tarball.
         """
         crate_name, version = _parse_crate_relative_path(relative_path)
-        return RustContent(
+        cargo_toml = extract_cargo_toml(artifact.file.path, crate_name, version)
+
+        content = RustContent(
             name=crate_name,
             vers=version,
             cksum=artifact.sha256,
+            features=cargo_toml.get("features", {}),
+            links=cargo_toml.get("package", {}).get("links"),
+            rust_version=cargo_toml.get("package", {}).get("rust-version"),
         )
+        # Store parsed dep data for the AFTER_CREATE hook to consume
+        content._parsed_deps = extract_dependencies(cargo_toml)
+        return content
+
+    @hook(AFTER_CREATE)
+    def _create_dependencies_from_parsed_data(self):
+        """Create RustDependency records from data parsed during pull-through."""
+        parsed_deps = getattr(self, "_parsed_deps", None)
+        if parsed_deps:
+            RustDependency.objects.bulk_create(
+                [RustDependency(content=self, **dep) for dep in parsed_deps]
+            )
 
     class Meta:
         default_related_name = "%(app_label)s_%(model_name)s"

pulp_rust/app/utils.py

@@ -0,0 +1,90 @@
+import tarfile
+
+try:
+    import tomllib
+except ModuleNotFoundError:
+    import tomli as tomllib
+
+
+def extract_cargo_toml(crate_path, crate_name, version):
+    """Extract and parse Cargo.toml from a .crate tarball."""
+    expected_path = f"{crate_name}-{version}/Cargo.toml"
+    with tarfile.open(crate_path, "r:gz") as tar:
+        cargo_toml_file = tar.extractfile(expected_path)
+        if cargo_toml_file is None:
+            raise FileNotFoundError(f"No Cargo.toml found in {crate_path} at {expected_path}")
+        return tomllib.load(cargo_toml_file)
+
+
+def _normalize_req(version_str):
+    """Normalize a Cargo version requirement to its explicit form.
+
+    In Cargo.toml, a bare version like "1.0" is shorthand for "^1.0".
+    The index format uses the explicit form with the comparator prefix.
+    """
+    if not version_str or version_str == "*":
+        return version_str
+    # Already has a comparator prefix
+    if version_str[0] in ("^", "~", "=", ">", "<"):
+        return version_str
+    return f"^{version_str}"
+
+
+def parse_dep(name, spec, kind="normal", target=None):
+    """Convert a single Cargo.toml dependency entry to index format."""
+    if isinstance(spec, str):
+        # Simple form: dep = "1.0"
+        return {
+            "name": name,
+            "req": _normalize_req(spec),
+            "features": [],
+            "optional": False,
+            "default_features": True,
+            "target": target,
+            "kind": kind,
+            "registry": None,
+            "package": None,
+        }
+
+    # Table form: dep = { version = "1.0", optional = true, ... }
+    dep = {
+        "name": name,
+        "req": _normalize_req(spec.get("version", "*")),
+        "features": spec.get("features", []),
+        "optional": spec.get("optional", False),
+        "default_features": spec.get("default-features", True),
+        "target": target,
+        "kind": kind,
+        "registry": spec.get("registry"),
+        "package": None,
+    }
+    # If the dep was renamed, "name" in the index is the alias (the key),
+    # and "package" is the real crate name
+    if "package" in spec:
+        dep["package"] = spec["package"]
+    return dep
+
+
+def extract_dependencies(cargo_toml):
+    """Extract all dependencies from a parsed Cargo.toml into index format."""
+    deps = []
+
+    for name, spec in cargo_toml.get("dependencies", {}).items():
+        deps.append(parse_dep(name, spec, kind="normal"))
+
+    for name, spec in cargo_toml.get("dev-dependencies", {}).items():
+        deps.append(parse_dep(name, spec, kind="dev"))
+
+    for name, spec in cargo_toml.get("build-dependencies", {}).items():
+        deps.append(parse_dep(name, spec, kind="build"))
+
+    # Platform-specific dependencies: [target.'cfg(...)'.dependencies]
+    for target, target_deps in cargo_toml.get("target", {}).items():
+        for name, spec in target_deps.get("dependencies", {}).items():
+            deps.append(parse_dep(name, spec, kind="normal", target=target))
+        for name, spec in target_deps.get("dev-dependencies", {}).items():
+            deps.append(parse_dep(name, spec, kind="dev", target=target))
+        for name, spec in target_deps.get("build-dependencies", {}).items():
+            deps.append(parse_dep(name, spec, kind="build", target=target))
+
+    return deps

pulp_rust/app/views.py

@@ -178,19 +178,21 @@ def _build_index_response(crate_versions):
         for crate_version in crate_versions:
             deps = []
             for dep in crate_version.dependencies.all():
-                deps.append(
-                    {
-                        "name": dep.name,
-                        "req": dep.req,
-                        "features": dep.features,
-                        "optional": dep.optional,
-                        "default_features": dep.default_features,
-                        "target": dep.target,
-                        "kind": dep.kind,
-                        "registry": dep.registry,
-                        "package": dep.package,
-                    }
-                )
+                dep_obj = {
+                    "name": dep.name,
+                    "req": dep.req,
+                    "features": dep.features,
+                    "optional": dep.optional,
+                    "default_features": dep.default_features,
+                    "target": dep.target,
+                    "kind": dep.kind,
+                }
+                # crates.io omits these keys when not set
+                if dep.registry is not None:
+                    dep_obj["registry"] = dep.registry
+                if dep.package is not None:
+                    dep_obj["package"] = dep.package
+                deps.append(dep_obj)
 
             version_obj = {
                 "name": crate_version.name,

pulp_rust/tests/functional/api/test_cargo_api.py

@@ -6,9 +6,7 @@
 import pytest
 from aiohttp.client_exceptions import ClientResponseError
 
-from pulp_rust.tests.functional.utils import download_file
-
-CRATES_IO_URL = "sparse+https://index.crates.io/"
+from pulp_rust.tests.functional.utils import CRATES_IO_URL, download_file
 
 
 def test_config_json(

pulp_rust/tests/functional/api/test_download_content.py

@@ -7,7 +7,7 @@
 import hashlib
 from urllib.parse import urljoin
 
-from pulp_rust.tests.functional.utils import download_file
+from pulp_rust.tests.functional.utils import CRATES_IO_URL, download_file
 
 
 def test_download_content(
@@ -27,7 +27,7 @@ def test_download_content(
     3. Verify that the content was automatically added to the repository.
     4. Remove the remote and verify the content is still served from cache.
     """
-    remote = rust_remote_factory(url="sparse+https://index.crates.io/")
+    remote = rust_remote_factory(url=CRATES_IO_URL)
     repository = rust_repo_factory(remote=remote.pulp_href)
     distribution = rust_distribution_factory(
         remote=remote.pulp_href, repository=repository.pulp_href

pulp_rust/tests/functional/api/test_pull_through_caching.py

@@ -1,13 +1,17 @@
 """Tests for Cargo pull-through caching via the sparse index proxy."""
 
 import hashlib
+import json
 from urllib.parse import urljoin
 
 import pytest
 
-from pulp_rust.tests.functional.utils import download_file
-
-CRATES_IO_URL = "sparse+https://index.crates.io/"
+from pulp_rust.tests.functional.utils import (
+    CRATES_IO_URL,
+    assert_index_entry_matches_upstream,
+    download_file,
+    get_index_entry,
+)
 
 
 def test_pull_through_sparse_index(
@@ -241,3 +245,157 @@ def test_pull_through_multiple_crates_on_demand(
 
     repository = rust_repo_api_client.read(repository.pulp_href)
     assert not repository.latest_version_href.endswith("/versions/0/")
+
+
+def test_pull_through_on_demand_preserves_metadata(
+    delete_orphans_pre,
+    rust_remote_factory,
+    rust_repo_factory,
+    rust_distribution_factory,
+    rust_repo_api_client,
+    rust_content_api_client,
+    monitor_task,
+    cargo_registry_url,
+):
+    """on_demand: cached content should have full metadata (deps, features).
+
+    Ensure that all metadata is saved appropriately when a package is created via
+    pull-through caching.
+    """
+    remote = rust_remote_factory(url=CRATES_IO_URL, policy="on_demand")
+    repository = rust_repo_factory(remote=remote.pulp_href)
+    distribution = rust_distribution_factory(
+        remote=remote.pulp_href, repository=repository.pulp_href
+    )
+
+    # serde has dependencies (serde_derive) and features in most versions
+    crate_name, crate_version = "serde", "1.0.210"
+    unit_path = f"api/v1/crates/{crate_name}/{crate_version}/download"
+    pulp_unit_url = urljoin(cargo_registry_url(distribution.base_path), unit_path)
+    download_file(pulp_unit_url)
+
+    # Verify the content record was created with full metadata
+    content_response = rust_content_api_client.list(name=crate_name, vers=crate_version)
+    assert content_response.count == 1
+    content = content_response.results[0]
+
+    # Should have dependencies (serde has serde_derive as an optional dep)
+    assert len(content.dependencies) > 0
+    dep_names = [d.name for d in content.dependencies]
+    assert "serde_derive" in dep_names
+
+    # Should have features
+    assert len(content.features) > 0
+    assert "derive" in content.features
+
+    # Add cached content and verify the locally-served index includes deps
+    monitor_task(
+        rust_repo_api_client.add_cached_content(
+            repository.pulp_href, {"remote": remote.pulp_href}
+        ).task
+    )
+
+    # Fetch the sparse index from Pulp (now served from local data)
+    index_url = urljoin(cargo_registry_url(distribution.base_path), f"se/rd/{crate_name}")
+    index_response = download_file(index_url)
+    assert index_response.response_obj.status == 200
+
+    body = index_response.body.decode("utf-8")
+    lines = body.strip().split("\n")
+    # Find the line for our version
+    version_entry = None
+    for line in lines:
+        entry = json.loads(line)
+        if entry["vers"] == crate_version:
+            version_entry = entry
+            break
+
+    assert version_entry is not None, f"Version {crate_version} not found in index"
+    assert len(version_entry["deps"]) > 0, "Index entry has no dependencies"
+    index_dep_names = [d["name"] for d in version_entry["deps"]]
+    assert "serde_derive" in index_dep_names
+    assert len(version_entry["features"]) > 0, "Index entry has no features"
+
+
+# ---------------------------------------------------------------------------
+# Index fidelity tests: compare Pulp output against crates.io for each mode
+# ---------------------------------------------------------------------------
+
+
+def test_index_fidelity_streamed(
+    rust_remote_factory,
+    rust_repo_factory,
+    rust_distribution_factory,
+    cargo_registry_url,
+    upstream_index_entry,
+):
+    """streamed: proxied sparse index entry should match crates.io exactly."""
+    remote = rust_remote_factory(url=CRATES_IO_URL, policy="streamed")
+    repository = rust_repo_factory(remote=remote.pulp_href)
+    distribution = rust_distribution_factory(
+        remote=remote.pulp_href, repository=repository.pulp_href
+    )
+
+    base = cargo_registry_url(distribution.base_path)
+    pulp_entry = get_index_entry(base, "se/rd/serde", "1.0.210")
+    assert_index_entry_matches_upstream(pulp_entry, upstream_index_entry)
+
+
+def test_index_fidelity_on_demand_proxied(
+    rust_remote_factory,
+    rust_repo_factory,
+    rust_distribution_factory,
+    cargo_registry_url,
+    upstream_index_entry,
+):
+    """on_demand: the first fetch (before caching) proxies upstream and should match."""
+    remote = rust_remote_factory(url=CRATES_IO_URL, policy="on_demand")
+    repository = rust_repo_factory(remote=remote.pulp_href)
+    distribution = rust_distribution_factory(
+        remote=remote.pulp_href, repository=repository.pulp_href
+    )
+
+    base = cargo_registry_url(distribution.base_path)
+
+    # First fetch — proxied from upstream (no local content yet)
+    pulp_entry = get_index_entry(base, "se/rd/serde", "1.0.210")
+    assert_index_entry_matches_upstream(pulp_entry, upstream_index_entry)
+
+
+def test_index_fidelity_on_demand_cached(
+    delete_orphans_pre,
+    rust_remote_factory,
+    rust_repo_factory,
+    rust_distribution_factory,
+    rust_repo_api_client,
+    rust_content_api_client,
+    monitor_task,
+    cargo_registry_url,
+    upstream_index_entry,
+):
+    """on_demand: after caching, the locally-served index entry should match crates.io."""
+    remote = rust_remote_factory(url=CRATES_IO_URL, policy="on_demand")
+    repository = rust_repo_factory(remote=remote.pulp_href)
+    distribution = rust_distribution_factory(
+        remote=remote.pulp_href, repository=repository.pulp_href
+    )
+
+    base = cargo_registry_url(distribution.base_path)
+
+    # Download the .crate to trigger content creation
+    download_file(urljoin(base, "api/v1/crates/serde/1.0.210/download"))
+
+    # Verify content was cached
+    content = rust_content_api_client.list(name="serde", vers="1.0.210")
+    assert content.count == 1, "Content was not cached after on_demand download"
+
+    # Promote cached content into the repository
+    monitor_task(
+        rust_repo_api_client.add_cached_content(
+            repository.pulp_href, {"remote": remote.pulp_href}
+        ).task
+    )
+
+    # Now the index is served from local data — compare against upstream
+    pulp_entry = get_index_entry(base, "se/rd/serde", "1.0.210")
+    assert_index_entry_matches_upstream(pulp_entry, upstream_index_entry)