Added support for Distributor when creating repository

Previously it wasn't possible to create repository with distributor
because of inconsintent naming of various fields which resulted
in Bad requests.

This is now fixed and distributors can be created along repository.
Only mininal configuration is currently supported.

Author: rbikar

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,49 @@ 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()
+                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)
+                        else:
+                            extra_dist.add(current_item.type_id)
+
+                for item in extra_dist:
+                    LOG.error(
+                        "Repository %s contains unexpected distributor with type: %s",
+                        repo_id,
+                        item,
+                    )
+
                 LOG.exception(
                     "Repository %s exists on server and contains unexpected values",
                     repo_id,
@@ -1131,3 +1162,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,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,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 +432,8 @@ 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 exists on server and contains unexpected values",
         ):
             assert text in caplog.text