Merge pull request release-engineering#225 from rbikar/create-repo-di…

…stributor

Added support for Distributor when creating repository

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,58 @@ 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_resource(
+                        "importer",
+                        ["type_id", "config"],
+                        repo_updated.importer,
+                        repo_on_server.importer,
+                        repo_updated.id,
+                    )
+
+                extra_dist = set()
+                missing_dist = set()
+                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_resource(
+                                "distributor",
+                                ["id", "type_id", "last_publish"],
+                                expected_item,
+                                current_item,
+                                repo_updated.id,
                             )
+                            extra_dist.discard(current_item.type_id)
+                            missing_dist.discard(expected_item.type_id)
+                        else:
+                            extra_dist.add(current_item.type_id)
+                            missing_dist.add(expected_item.type_id)
+
+                for item in extra_dist:
+                    LOG.error(
+                        "Repository %s contains unexpected distributor with type: %s",
+                        repo_id,
+                        item,
+                    )
+                for item in missing_dist:
+                    LOG.error(
+                        "Repository %s is missing distributor with type: %s",
+                        repo_id,
+                        item,
+                    )
+
                 LOG.exception(
                     "Repository %s exists on server and contains unexpected values",
                     repo_id,
@@ -1131,3 +1171,22 @@ def check_repo(repo_on_server):
         out = f_flat_map(out, check_repo)
 
         return f_proxy(out)
+
+    @staticmethod
+    def _check_resource(
+        resource_type, fields, expected_resource, current_resource, repo_id
+    ):
+        for attr in fields:
+            expected = getattr(expected_resource, attr)
+            current = getattr(current_resource, attr)
+            if expected != current:
+                LOG.error(
+                    "Repository %s contains wrong %s %s\n"
+                    "\t expected: %s\n"
+                    "\t current: %s\n",
+                    repo_id,
+                    resource_type,
+                    attr,
+                    expected,
+                    current,
+                )

tests/fake/test_fake_create_repo.py

@@ -1,16 +1,31 @@
-from pubtools.pulplib import FakeController, Repository
+from pubtools.pulplib import FakeController, Repository, Distributor
 
 
 def test_create_repository():
     """Client.create_repository() with fake client adds new repositories to controller."""
     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()]

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,12 @@ 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_1", type_id="yum_distributor"),
+            Distributor(id="test_dist_2", type_id="rpm_rsync_distributor"),
+        ],
     )
 
     requests_mocker.post(
@@ -353,6 +413,18 @@ def test_create_repository_wrong_data(client, requests_mocker, caplog):
                         "config": {"current": "value"},
                     }
                 ],
+                "distributors": [
+                    {
+                        "id": "fake_id_1",
+                        "distributor_type_id": "yum_distributor",
+                        "config": {},
+                    },
+                    {
+                        "id": "fake_id_2",
+                        "distributor_type_id": "iso_distributor",
+                        "config": {},
+                    },
+                ],
             }
         ],
     )
@@ -363,6 +435,9 @@ 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 contains unexpected distributor with type: iso_distributor",
+            "Repository yum_repo_existing is missing distributor with type: rpm_rsync_distributor",
             "Repository yum_repo_existing exists on server and contains unexpected values",
         ):
             assert text in caplog.text