Fix deb sync for missing indices and optimize enabled

CHANGES/719.bugfix

@@ -0,0 +1 @@
+Fixed optimized syncs failing with "'NoneType' object has no attribute 'artifact_set_sha256'" when a package index is new upstream. Optimized syncs of flat repositories no longer drop their package index when the Release file has not changed, and a distribution whose name is a prefix of another (for example "jammy" and "jammy-updates") no longer causes "Paths are duplicated" errors.

pulp_deb/app/tasks/synchronizing.py

@@ -591,7 +591,7 @@ async def _handle_distribution(self, distribution):
             )
             if previous_release_file.artifact_set_sha256 == release_file.artifact_set_sha256:
                 await _readd_previous_package_indices(
-                    self.previous_repo_version, self.new_version, stored_distribution
+                    self.previous_repo_version, self.new_version, release_file
                 )
                 message = 'ReleaseFile has not changed for distribution="{}". Skipping.'
                 log.info(_(message).format(distribution))
@@ -921,7 +921,12 @@ async def _handle_package_index(
             previous_package_index = await _get_previous_package_index(
                 self.previous_repo_version, relative_path
             )
-            if previous_package_index.artifact_set_sha256 == package_index.artifact_set_sha256:
+            # The previous version may lack this PackageIndex, for example if the index is new
+            # upstream. Then there is nothing to compare against.
+            if (
+                previous_package_index
+                and previous_package_index.artifact_set_sha256 == package_index.artifact_set_sha256
+            ):
                 message = 'PackageIndex has not changed for relative_path="{}". Skipped.'
                 log.info(_(message).format(relative_path))
                 async with ProgressReport(
@@ -1249,16 +1254,20 @@ def _get_content_artifact_file(content_artifact):
 
 
 @sync_to_async
-def _readd_previous_package_indices(previous_version, new_version, distribution):
+def _readd_previous_package_indices(previous_version, new_version, release_file):
+    # Match on the directory of the Release file, not on the distribution name: a flat repo is
+    # stored as "flat-repo", which is not part of its index paths, and a name like "jammy" is also
+    # a substring of "jammy-updates".
+    release_base_path = os.path.dirname(release_file.relative_path)
+    if release_base_path:
+        path_filter = {"relative_path__startswith": f"{release_base_path}/"}
+    else:
+        path_filter = {"relative_path__regex": r"^[^/]+$"}
     new_version.add_content(
-        previous_version.get_content(
-            PackageIndex.objects.filter(relative_path__contains=distribution)
-        )
+        previous_version.get_content(PackageIndex.objects.filter(**path_filter))
     )
     new_version.add_content(
-        previous_version.get_content(
-            InstallerFileIndex.objects.filter(relative_path__contains=distribution)
-        )
+        previous_version.get_content(InstallerFileIndex.objects.filter(**path_filter))
     )
 
 

pulp_deb/tests/functional/api/test_sync.py

@@ -10,6 +10,7 @@
     DEB_FIXTURE_ARCH_UPDATE,
     DEB_FIXTURE_COMPONENT,
     DEB_FIXTURE_COMPONENT_UPDATE,
+    DEB_FIXTURE_FLAT_REPOSITORY_NAME,
     DEB_FIXTURE_INVALID_REPOSITORY_NAME,
     DEB_FIXTURE_METADATA_UPDATE_REPOSITORY_NAME,
     DEB_FIXTURE_SINGLE_DIST,
@@ -466,6 +467,21 @@ def test_sync_optimize_with_mirror_enabled(deb_init_and_sync):
     assert is_sync_skipped(task, DEB_REPORT_CODE_SKIP_COMPLETE)
 
 
+@pytest.mark.parallel
+def test_sync_optimize_flat_repo_keeps_package_index(deb_init_and_sync, apt_package_indices_api):
+    """Test that an unchanged optimized resync of a flat repo keeps its package index."""
+    remote_args = {"distributions": "/"}
+    repo, remote = deb_init_and_sync(url=DEB_FIXTURE_FLAT_REPOSITORY_NAME, remote_args=remote_args)
+    repo_v1_href = repo.latest_version_href
+    assert apt_package_indices_api.list(repository_version=repo_v1_href).count > 0
+
+    repo, _, task = deb_init_and_sync(repository=repo, remote=remote, return_task=True)
+
+    # Before the fix, the resync created a new version without the package index.
+    assert is_sync_skipped(task, DEB_REPORT_CODE_SKIP_RELEASE)
+    assert repo.latest_version_href == repo_v1_href
+
+
 @pytest.mark.skipif(settings.DOMAIN_ENABLED, reason="Domain produces different results.")
 def test_sync_orphan_cleanup_fail(
     deb_init_and_sync,

pulp_deb/tests/unit/test_sync_optimize.py

@@ -0,0 +1,59 @@
+"""Tests for optimized syncs against a previous version that lacks a package index."""
+
+import io
+from unittest import IsolatedAsyncioTestCase, mock
+
+from pulp_deb.app.tasks.synchronizing import DebFirstStage, gen_remote_options
+
+SYNCHRONIZING = "pulp_deb.app.tasks.synchronizing"
+
+
+class TestOptimizeWithoutPreviousPackageIndex(IsolatedAsyncioTestCase):
+    """
+    An index that is new upstream, for example a new component or architecture, has no counterpart
+    in the previous repository version. See https://github.com/pulp/pulp_deb/issues/719.
+    """
+
+    def setUp(self):
+        remote = mock.Mock(
+            url="http://example.com/debian/",
+            distributions="stable",
+            components="main",
+            architectures="amd64",
+            policy="immediate",
+            sync_sources=False,
+            sync_udebs=False,
+            sync_installer=False,
+            gpgkey=None,
+            ignore_missing_package_indices=False,
+        )
+        previous_version = mock.Mock(
+            info={
+                "remote_options": gen_remote_options(remote),
+                "sync_options": {"optimize": True, "mirror": False},
+            }
+        )
+        self.stage = DebFirstStage(remote, True, False, previous_version)
+        self.stage._create_unit = mock.AsyncMock(
+            return_value=mock.Mock(artifact_set_sha256="new-index")
+        )
+
+    async def test_new_package_index_is_processed(self):
+        """The new index must be parsed, not compared with a previous index that is not there."""
+        self.assertTrue(self.stage.optimize)
+        main_artifact = mock.AsyncMock(return_value=mock.Mock(file=io.BytesIO(b"")))
+        with (
+            mock.patch(
+                f"{SYNCHRONIZING}._get_previous_package_index",
+                mock.AsyncMock(return_value=None),
+            ),
+            mock.patch(f"{SYNCHRONIZING}._get_main_artifact_blocking", main_artifact),
+        ):
+            await self.stage._handle_package_index(
+                release_file=mock.Mock(relative_path="dists/stable/Release", architectures="amd64"),
+                release_component=mock.Mock(plain_component="main", component="main"),
+                base_architecture="amd64",
+                index_architecture="amd64",
+                file_references={"main/binary-amd64/Packages": {"SHA256": "0" * 64, "Size": "0"}},
+            )
+        main_artifact.assert_awaited_once()