diff --git a/src/pubtools/pulplib/_impl/client/client.py b/src/pubtools/pulplib/_impl/client/client.py index 2e3f13ff..c2c47b8e 100644 --- a/src/pubtools/pulplib/_impl/client/client.py +++ b/src/pubtools/pulplib/_impl/client/client.py @@ -10,7 +10,7 @@ from more_executors import Executors from more_executors.futures import f_map, f_flat_map, f_return, f_proxy, f_sequence from io import StringIO - +from ..compat_attr import evolve from ..model.repository.repo_lock import LOCK_CLAIM_STR from ..page import Page from ..criteria import Criteria @@ -1083,6 +1083,13 @@ def create_repository(self, repo): body["importer_type_id"] = importer[0]["importer_type_id"] if importer else None body["importer_config"] = importer[0]["config"] if importer else None + # re-key distributor dict keys due to pulp API inconsistency + for item in body["distributors"]: + item["distributor_id"] = item["id"] + item["distributor_config"] = item["config"] + del item["config"] + del item["id"] + def log_existing_repo(exception): if ( getattr(exception, "response", None) is not None @@ -1094,25 +1101,37 @@ def log_existing_repo(exception): raise exception def check_repo(repo_on_server): + # evolve some fields that don't have to be set before repository creation + # but they're typically set automatically after creation call which results in + # inequality between `repo_on_server`` and `repo` + dists = [evolve(item, repo_id=repo_id) for item in repo.distributors] + repo_updated = evolve( + repo, relative_url=repo_on_server.relative_url, distributors=dists + ) try: assert ( - repo_on_server == repo + repo_on_server == repo_updated ), "Repo exists on server with unexpected values" except AssertionError: if importer: - for attr in ["type_id", "config"]: - expected = getattr(repo.importer, attr) - current = getattr(repo_on_server.importer, attr) - if expected != current: - LOG.error( - "Repository %s contains wrong importer %s\n" - "\t expected: %s\n" - "\t current: %s\n", - repo_id, - attr, - expected, - current, + self._check_unit( + "importer", + ["type_id", "config"], + repo_updated.importer, + repo_on_server.importer, + repo_updated.id, + ) + for expected_item in repo_updated.distributors: + for current_item in repo_on_server.distributors: + if expected_item.type_id == current_item.type_id: + self._check_unit( + "distributor", + ["id", "type_id", "last_publish"], + expected_item, + current_item, + repo_updated.id, ) + LOG.exception( "Repository %s exists on server and contains unexpected values", repo_id, @@ -1131,3 +1150,20 @@ def check_repo(repo_on_server): out = f_flat_map(out, check_repo) return f_proxy(out) + + @staticmethod + def _check_unit(unit_type, fields, expected_unit, current_unit, repo_id): + for attr in fields: + expected = getattr(expected_unit, attr) + current = getattr(current_unit, attr) + if expected != current: + LOG.error( + "Repository %s contains wrong %s %s\n" + "\t expected: %s\n" + "\t current: %s\n", + repo_id, + unit_type, + attr, + expected, + current, + ) diff --git a/tests/fake/test_fake_create_repo.py b/tests/fake/test_fake_create_repo.py index 06363e69..7d8d5978 100644 --- a/tests/fake/test_fake_create_repo.py +++ b/tests/fake/test_fake_create_repo.py @@ -1,4 +1,4 @@ -from pubtools.pulplib import FakeController, Repository +from pubtools.pulplib import FakeController, Repository, Distributor def test_create_repository(): @@ -6,11 +6,26 @@ def test_create_repository(): controller = FakeController() client = controller.client - repo_1 = client.create_repository(Repository(id="repo1")) - repo_2 = client.create_repository(Repository(id="repo2")) + repo_1 = client.create_repository( + Repository( + id="repo1", + distributors=[Distributor(id="dist1", type_id="yum_distributor")], + ) + ) + repo_2 = client.create_repository( + Repository( + id="repo2", + distributors=[Distributor(id="dist2", type_id="yum_distributor")], + ) + ) # adding already existing repository has no effect - _ = client.create_repository(Repository(id="repo1")) + _ = client.create_repository( + Repository( + id="repo1", + distributors=[Distributor(id="dist1", type_id="yum_distributor")], + ) + ) # The change should be reflected in the controller, # with two repositories present assert controller.repositories == [repo_1.result(), repo_2.result()] diff --git a/tests/repository/test_yum_repository.py b/tests/repository/test_yum_repository.py index b082139a..458bdb5e 100644 --- a/tests/repository/test_yum_repository.py +++ b/tests/repository/test_yum_repository.py @@ -2,7 +2,14 @@ import logging import requests -from pubtools.pulplib import Repository, YumRepository, DetachedException, YumImporter +import pubtools.pulplib._impl.compat_attr as attr +from pubtools.pulplib import ( + Repository, + YumRepository, + DetachedException, + YumImporter, + Distributor, +) def test_from_data_gives_yum_repository(): @@ -216,7 +223,16 @@ def test_related_repositories_detached_client(): def test_create_repository(client, requests_mocker): - repo = YumRepository(id="yum_repo_new") + repo = YumRepository( + id="yum_repo_new", + distributors=[ + Distributor( + id="fake_id", + type_id="yum_distributor", + relative_url="yum_repo_new/foo/bar", + ) + ], + ) # create request requests_mocker.post( @@ -237,13 +253,44 @@ def test_create_repository(client, requests_mocker): "config": {}, } ], + "distributors": [ + { + "id": "fake_id", + "distributor_type_id": "yum_distributor", + "config": {"relative_url": "yum_repo_new/foo/bar"}, + "repo_id": "yum_repo_new", + } + ], } ], ) - out = client.create_repository(repo) + out = client.create_repository(repo).result() # check return value of create_repository() call - assert out.result() == repo + # relative_url wasn't set before request for `repo` + assert repo.relative_url is None + # but it's set after creation + assert out.relative_url == "yum_repo_new/foo/bar" + + # repo_id of distributor isn't set before creation call + assert repo.distributors[0].repo_id is None + # but it's set after creation + assert out.distributors[0].repo_id == "yum_repo_new" + # evolve original repo object with new values + repo = attr.evolve( + repo, + relative_url="yum_repo_new/foo/bar", + distributors=[ + Distributor( + id="fake_id", + type_id="yum_distributor", + relative_url="yum_repo_new/foo/bar", + repo_id="yum_repo_new", + ) + ], + ) + # and do full assert + assert out == repo hist = requests_mocker.request_history # there should be exactly 2 requests sent - create and search @@ -256,6 +303,14 @@ def test_create_repository(client, requests_mocker): # are automatically added for yum repository assert create_query["importer_type_id"] == "yum_importer" assert create_query["importer_config"] == {} + # check distributor data + assert len(create_query["distributors"]) == 1 + assert create_query["distributors"][0]["distributor_id"] == "fake_id" + assert create_query["distributors"][0]["distributor_type_id"] == "yum_distributor" + assert ( + create_query["distributors"][0]["distributor_config"]["relative_url"] + == "yum_repo_new/foo/bar" + ) # check the search request for created repo search_query = hist[1].json() @@ -331,7 +386,9 @@ def test_create_repository_already_exists(client, requests_mocker, caplog): def test_create_repository_wrong_data(client, requests_mocker, caplog): repo = YumRepository( - id="yum_repo_existing", importer=YumImporter(config={"new": "value"}) + id="yum_repo_existing", + importer=YumImporter(config={"new": "value"}), + distributors=[Distributor(id="test_dist", type_id="yum_distributor")], ) requests_mocker.post( @@ -353,6 +410,13 @@ def test_create_repository_wrong_data(client, requests_mocker, caplog): "config": {"current": "value"}, } ], + "distributors": [ + { + "id": "fake_id", + "distributor_type_id": "yum_distributor", + "config": {}, + } + ], } ], ) @@ -363,6 +427,7 @@ def test_create_repository_wrong_data(client, requests_mocker, caplog): for text in ( "Repository yum_repo_existing already exists", "Repository yum_repo_existing contains wrong importer config", + "Repository yum_repo_existing contains wrong distributor id", "Repository yum_repo_existing exists on server and contains unexpected values", ): assert text in caplog.text