Merge branch 'repository-shared-files' of salsa.debian.org:lyknode/debexpo into live

MR: !181


Signed-off-by: Mattia Rizzolo <mattia@debian.org>

debexpo/repository/migrations/0002_change_unique.py

+# Generated by Django 2.2.17 on 2021-08-28 12:39
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('repository', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='repositoryfile',
+            name='path',
+            field=models.TextField(verbose_name='Path'),
+        ),
+        migrations.AlterUniqueTogether(
+            name='repositoryfile',
+            unique_together={('package', 'version', 'path')},
+        ),
+    ]

debexpo/repository/models.py

@@ -46,6 +46,13 @@ log = logging.getLogger(__name__)
 
 class RepositoryFileManager(models.Manager):
     def create_from_file(self, sumed_file, basedir, changes):
+        if RepositoryFile.objects.filter(
+            path__endswith=join(basedir, str(sumed_file)),
+            package=changes.source,
+            version=changes.version
+                ).exists():
+            return None
+
         repository_file = RepositoryFile()
         repository_file.path = join(basedir, str(sumed_file))
         repository_file.size = sumed_file.size
@@ -68,15 +75,21 @@ class RepositoryFile(models.Model):
     distribution = models.CharField(max_length=32,
                                     verbose_name=_('Distribution'))
 
-    path = models.TextField(verbose_name=_('Path'), unique=True)
+    path = models.TextField(verbose_name=_('Path'))
     size = models.IntegerField(verbose_name=_('Size'))
     sha256sum = models.CharField(max_length=64, verbose_name=_('SHA256'))
 
     objects = RepositoryFileManager()
 
+    class Meta:
+        unique_together = ('package', 'version', 'path')
+
     def __str__(self):
         return self.path
 
+    def is_shared(self):
+        return RepositoryFile.objects.filter(path=self.path).count() > 1
+
 
 class Repository():
     """
@@ -241,13 +254,12 @@ class Repository():
 
     def _cleanup_previous_entries(self, files_to_install, pool_dir):
         for sumed_file in files_to_install:
-            # Remove old entry from database
-            try:
-                previous_entry = RepositoryFile.objects.get(path__endswith=join(
-                    '/', str(sumed_file)))
-            except RepositoryFile.DoesNotExist:
-                pass
-            else:
+            # Remove old entry from database if checksum mismatch
+            previous_entries = RepositoryFile.objects.filter(
+                path__endswith=join('/', str(sumed_file))
+            ).exclude(sha256sum=sumed_file.checksums['sha256']).all()
+
+            for previous_entry in previous_entries:
                 self.remove(previous_entry.package, previous_entry.version)
 
     def _install_new_entries(self, files_to_install, pool_dir, changes):
@@ -263,8 +275,9 @@ class Repository():
             entry = RepositoryFile.objects.create_from_file(sumed_file,
                                                             pool_dir,
                                                             changes)
-            entry.full_clean()
-            entry.save()
+            if entry:
+                entry.full_clean()
+                entry.save()
 
     def fetch_from_pool(self, package, component, filename, dest_dir):
         filename = join(self.repository, 'pool', component,
@@ -298,8 +311,10 @@ class Repository():
         for repository_file in repository_files:
             path = join(self.repository, repository_file.path)
             if isfile(path):
-                unlink(path)
+                if not repository_file.is_shared():
+                    unlink(path)
+
                 self.pending.add((repository_file.distribution,
                                   repository_file.component,))
 
-        repository_files.delete()
+            repository_file.delete()

tests/functional/repository/test_repository.py

@@ -128,7 +128,7 @@ class TestRepositoryController(TestController):
         sources = self._parse_sources()
 
         # Assert DB entries
-        packages_in_db = set()
+        packages_in_db = []
         for entry in db_entires:
             entry_package = {
                 'name': entry.package,
@@ -138,15 +138,18 @@ class TestRepositoryController(TestController):
             }
 
             self.assertIn(entry_package, packages)
-            packages_in_db.add(packages.index(entry_package))
+            packages_in_db.append(entry_package)
 
-        self.assertEquals(len(packages_in_db), len(packages))
+        for package in packages:
+            self.assertIn(package, packages_in_db)
 
         # Assert pool files
         for entry in db_entires:
             self.assertIn(join(str(self.repository), entry.path), pool_files)
 
-        self.assertEquals(len(db_entires), len(pool_files))
+        for pool_file in pool_files:
+            self.assertIn(pool_file, [join(str(self.repository), entry.path) for
+                                      entry in db_entires])
 
         # Assert files modes
         for entry in pool_files:
@@ -174,6 +177,8 @@ class TestRepositoryController(TestController):
 
     def test_repository_install_package(self):
         package = self.package.copy()
+        package_version = self.package.copy()
+        package_distribution = self.package.copy()
 
         # First time import
         self._repo_install_package(**package)
@@ -184,27 +189,30 @@ class TestRepositoryController(TestController):
         self._assert_package_in_repo([package])
 
         # Change the debian revision
-        package['version'] = '1.0-2'
-        self._repo_install_package(**package)
-        self._assert_package_in_repo([package])
+        package_version['version'] = '1.0-2'
+        self._repo_install_package(**package_version)
+        self._assert_package_in_repo([package, package_version])
 
         # Change of distribution
-        package['distribution'] = 'UNRELEASED'
-        self._repo_install_package(**package)
-        self._assert_package_in_repo([package])
+        package_distribution['distribution'] = 'UNRELEASED'
+        self._repo_install_package(**package_distribution)
+        self._assert_package_in_repo([package_version,
+                                      package_distribution])
 
         # Upload another package
         another = self.package.copy()
         another['name'] = 'libhello'
         self._repo_install_package(**another)
-        self._assert_package_in_repo([package, another])
+        self._assert_package_in_repo([package_version,
+                                      package_distribution, another])
 
         # Remove first package
-        self._repo_remove_package(**package)
-        self._assert_package_in_repo([another])
+        self._repo_remove_package(**package_version)
+        self._assert_package_in_repo([package_distribution, another])
 
-        # Remove last package
+        # Remove last packages
         self._repo_remove_package(**another)
+        self._repo_remove_package(**package_distribution)
 
     def test_inconsistent_repository(self):
         self._repo_install_package(**self.package)