From e1f7a5975a8ab638aa430cb4b4a67bc3d749c93a Mon Sep 17 00:00:00 2001 From: Sergey Kulanov Date: Tue, 7 Jun 2016 18:42:29 +0300 Subject: [PATCH] [priority] Merge repos with the same priority 1) We need to merge trees with the same priority in order to catch case when we have package with different versions in repos with the same priority, eg: repo2, priority=10, package=foo-5.3 repo1, priority=5, package=asd-1.2 repo3, priority=10, package=foo-7.1 --> sort(key=priority) can result: repo1, priority=5, package=asd-1.2 repo2, priority=10, package=foo-5.3 repo3, priority=10, package=foo-7.1 so we will return: foo-5.3 instead we must return foo-7.1 2) priority must be a mandatory parameter for Repository 3) Align rpm default priority _DEFAULT_PRIORITY: N, where N is an integer from 1 to 99. The default priority for repositories is 99. The repositories with the lowest numerical priority number have the highest priority. Closes-bug: #1590525 Change-Id: I8299b1502775cc68bd1c783487ce7dc802a31a05 --- packetary/api/repositories.py | 5 ++++- packetary/controllers/repository.py | 2 +- packetary/drivers/base.py | 4 ++-- packetary/drivers/deb_driver.py | 8 ++++--- packetary/drivers/rpm_driver.py | 16 +++++++------- packetary/objects/packages_forest.py | 19 ++++++++++------ packetary/objects/repository.py | 4 +++- packetary/tests/test_deb_driver.py | 4 ++-- packetary/tests/test_packages_forest.py | 29 +++++++++++++++++++------ packetary/tests/test_rpm_driver.py | 4 ++-- 10 files changed, 61 insertions(+), 34 deletions(-) diff --git a/packetary/api/repositories.py b/packetary/api/repositories.py index 94d4ff8..e22110c 100644 --- a/packetary/api/repositories.py +++ b/packetary/api/repositories.py @@ -151,7 +151,10 @@ class RepositoryApi(object): for repo in repositories: self.controller.load_packages( repo, - compose(forest.add_tree().add, packages_traverse) + compose( + forest.add_tree(repo.priority).add, + packages_traverse + ) ) return forest.get_packages( package_relations, requirements.get('mandatory', True) diff --git a/packetary/controllers/repository.py b/packetary/controllers/repository.py index 99c2ab1..c7568ad 100644 --- a/packetary/controllers/repository.py +++ b/packetary/controllers/repository.py @@ -67,7 +67,7 @@ class RepositoryController(object): """ connection = self.context.connection - repositories_data.sort(key=self.driver.priority_sort) + repositories_data.sort(key=self.driver.get_priority) repos = [] for repo_data in repositories_data: self.driver.get_repository( diff --git a/packetary/drivers/base.py b/packetary/drivers/base.py index b456c88..efd0f3b 100644 --- a/packetary/drivers/base.py +++ b/packetary/drivers/base.py @@ -103,8 +103,8 @@ class RepositoryDriverBase(object): """ @abc.abstractmethod - def priority_sort(self, repo_data): - """Key method to sort repositories data by priority. + def get_priority(self, repo_data): + """Get repository priority. :param repo_data: the repository`s description :return: the integer value that is relevant repository`s priority diff --git a/packetary/drivers/deb_driver.py b/packetary/drivers/deb_driver.py index 2d4cae8..ece9fa1 100644 --- a/packetary/drivers/deb_driver.py +++ b/packetary/drivers/deb_driver.py @@ -87,7 +87,7 @@ class DebRepositoryDriver(RepositoryDriverBase): def get_repository_data_schema(self): return DEB_REPO_SCHEMA - def priority_sort(self, repo_data): + def get_priority(self, repo_data): # DEB repository expects general values from 0 to 1000. 0 # to have lowest priority and 1000 -- the highest. Note that a # priority above 1000 will allow even downgrades no matter the version @@ -127,7 +127,8 @@ class DebRepositoryDriver(RepositoryDriverBase): origin=deb_release["origin"], url=url, section=(suite, component), - path=path + path=path, + priority=self.get_priority(repository_data) )) def get_packages(self, connection, repository, consumer): @@ -226,7 +227,8 @@ class DebRepositoryDriver(RepositoryDriverBase): architecture=arch, origin=origin, section=(suite, component), - path=path + path=path, + priority=self.get_priority(repository_data) ) self._create_repository_structure(repository) self.logger.info("Created: %d repository.", repository.name) diff --git a/packetary/drivers/rpm_driver.py b/packetary/drivers/rpm_driver.py index 2a52fac..1f9090c 100644 --- a/packetary/drivers/rpm_driver.py +++ b/packetary/drivers/rpm_driver.py @@ -65,7 +65,7 @@ _OPERATORS_MAPPING = { 'LE': '<=', } -_DEFAULT_PRIORITY = 10 +_DEFAULT_PRIORITY = 99 class CreaterepoCallBack(object): @@ -146,11 +146,9 @@ class RpmRepositoryDriver(RepositoryDriverBase): def get_repository_data_schema(self): return RPM_REPO_SCHEMA - def priority_sort(self, repo_data): - # DEB repository expects general values from 0 to 1000. 0 - # to have lowest priority and 1000 -- the highest. Note that a - # priority above 1000 will allow even downgrades no matter the version - # of the prioritary package + def get_priority(self, repo_data): + # RPM repository expects general values from 0 to 99. 0 + # to have the highest priority and 99 -- the lowest. priority = repo_data.get('priority') if priority is None: priority = _DEFAULT_PRIORITY @@ -162,7 +160,8 @@ class RpmRepositoryDriver(RepositoryDriverBase): url=utils.normalize_repository_url(repository_data["uri"]), architecture=arch, path=repository_data.get('path'), - origin="" + origin="", + priority=self.get_priority(repository_data) )) def get_packages(self, connection, repository, consumer): @@ -234,7 +233,8 @@ class RpmRepositoryDriver(RepositoryDriverBase): url=utils.normalize_repository_url(repository_data["uri"]), architecture=arch, path=repository_data.get('path'), - origin=repository_data.get('origin') + origin=repository_data.get('origin'), + priority=self.get_priority(repository_data) ) utils.ensure_dir_exist(utils.get_path_from_url(repository.url)) self._rebuild_repository(connection, repository, None, None) diff --git a/packetary/objects/packages_forest.py b/packetary/objects/packages_forest.py index 19a9fdc..1e7be01 100644 --- a/packetary/objects/packages_forest.py +++ b/packetary/objects/packages_forest.py @@ -17,7 +17,9 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. import logging +import six +from collections import OrderedDict from packetary.objects.packages_tree import PackagesTree @@ -28,16 +30,19 @@ class PackagesForest(object): """Helper class to deal with dependency graph.""" def __init__(self): - self.trees = [] + self.trees = OrderedDict() - def add_tree(self): + def add_tree(self, priority): """Add new tree to end of forest. :return: The added tree """ - tree = PackagesTree() - self.trees.append(tree) - return tree + + try: + return self.trees[priority] + except KeyError: + tree = self.trees[priority] = PackagesTree() + return tree def get_packages(self, requirements, include_mandatory=False): """Get the packages according requirements. @@ -57,7 +62,7 @@ class PackagesForest(object): stack = [(None, requirements)] if include_mandatory: - for tree in self.trees: + for tree in six.itervalues(self.trees): for mandatory in tree.mandatory_packages: resolved.add(mandatory) stack.append((mandatory, mandatory.requires)) @@ -85,7 +90,7 @@ class PackagesForest(object): :param relation: the package relation :return: the packages from first tree if found otherwise empty list """ - for tree in self.trees: + for tree in six.itervalues(self.trees): candidate = tree.find(relation.name, relation.version) if candidate is not None: return candidate diff --git a/packetary/objects/repository.py b/packetary/objects/repository.py index 4b3ba859..ab949ff 100644 --- a/packetary/objects/repository.py +++ b/packetary/objects/repository.py @@ -20,13 +20,14 @@ class Repository(object): """Structure to describe repository object.""" - def __init__(self, name, url, architecture, origin=None, + def __init__(self, name, url, architecture, priority, origin=None, path=None, section=None): """Initialises. :param name: the repository`s name, may be tuple of strings :param url: the repository`s URL :param architecture: the repository`s architecture + :param priority: repository priority :param origin: optional, the repository`s origin :param path: the repository relative path, used for mirroring :param section: the repository section @@ -37,6 +38,7 @@ class Repository(object): self.url = url self.section = section self.path = path + self.priority = priority def __str__(self): if not self.section: diff --git a/packetary/tests/test_deb_driver.py b/packetary/tests/test_deb_driver.py index 3359a9b..27d2ad7 100644 --- a/packetary/tests/test_deb_driver.py +++ b/packetary/tests/test_deb_driver.py @@ -53,7 +53,7 @@ class TestDebDriver(base.TestCase): {"name": "repo2", "priority": 1000}, {"name": "repo3", "priority": None} ] - repos.sort(key=self.driver.priority_sort) + repos.sort(key=self.driver.get_priority) self.assertEqual( ["repo2", "repo0", "repo3", "repo1"], @@ -311,7 +311,7 @@ class TestDebDriver(base.TestCase): def test_create_repository(self, mkdir_mock, deb822, gzip, open, os): repository_data = { "name": "Test", "uri": "file:///repo", "suite": "trusty", - "section": "main", "type": "rpm", "priority": "100", + "section": "main", "type": "rpm", "priority": 100, "origin": "Origin", "path": "/repo" } repo = self.driver.create_repository( diff --git a/packetary/tests/test_packages_forest.py b/packetary/tests/test_packages_forest.py index 594560e..0cd0704 100644 --- a/packetary/tests/test_packages_forest.py +++ b/packetary/tests/test_packages_forest.py @@ -54,13 +54,18 @@ class TestPackagesForest(base.TestCase): requires=[generator.gen_relation("package2")] ), ] - self._add_packages(forest.add_tree(), packages1) - self._add_packages(forest.add_tree(), packages2) + self._add_packages(forest.add_tree(priority=10), packages1) + self._add_packages(forest.add_tree(priority=10), packages2) def test_add_tree(self): forest = PackagesForest() - tree = forest.add_tree() - self.assertIs(tree, forest.trees[-1]) + tree = forest.add_tree(priority=10) + self.assertIs(tree, forest.trees[10]) + # test that trees with the same priority are merged + tree = forest.add_tree(priority=10) + self.assertEqual(1, len(forest.trees)) + tree = forest.add_tree(priority=50) + self.assertEqual(2, len(forest.trees)) def test_find(self): forest = PackagesForest() @@ -68,10 +73,15 @@ class TestPackagesForest(base.TestCase): p12 = generator.gen_package(name="package1", version=2) p21 = generator.gen_package(name="package2", version=1) p22 = generator.gen_package(name="package2", version=2) - self._add_packages(forest.add_tree(), [p11, p22]) - self._add_packages(forest.add_tree(), [p12, p21]) + p33 = generator.gen_package(name="package2", version=10) + self._add_packages(forest.add_tree(priority=10), [p11, p22]) + self._add_packages(forest.add_tree(priority=10), [p12, p21]) + self._add_packages(forest.add_tree(priority=20), [p33]) self.assertEqual( - p11, forest.find(generator.gen_relation("package1", [">=", 1])) + p11, forest.find(generator.gen_relation("package1", ["=", 1])) + ) + self.assertEqual( + p12, forest.find(generator.gen_relation("package1", [">=", 1])) ) self.assertEqual( p12, forest.find(generator.gen_relation("package1", [">", 1])) @@ -80,6 +90,11 @@ class TestPackagesForest(base.TestCase): self.assertEqual( p21, forest.find(generator.gen_relation("package2", ["<", 2])) ) + # select package from the repo with highest priority + # p33 has version 10, but in the repo with lower priority + self.assertEqual( + p22, forest.find(generator.gen_relation("package2", [">=", 2])) + ) def test_get_packages_with_mandatory(self): forest = PackagesForest() diff --git a/packetary/tests/test_rpm_driver.py b/packetary/tests/test_rpm_driver.py index b4bb762..09358bb 100644 --- a/packetary/tests/test_rpm_driver.py +++ b/packetary/tests/test_rpm_driver.py @@ -82,10 +82,10 @@ class TestRpmDriver(base.TestCase): {"name": "repo2", "priority": 99}, {"name": "repo3", "priority": None} ] - repos.sort(key=self.driver.priority_sort) + repos.sort(key=self.driver.get_priority) self.assertEqual( - ["repo1", "repo0", "repo3", "repo2"], + ["repo1", "repo0", "repo2", "repo3"], [x['name'] for x in repos] )