Merge pull request #2621 from fetchai/feature/check_for_dependecnies_on_push

check dependencies present in registry on package push

Author: DavidMinarsch

Diff for: aea/cli/registry/push.py

@@ -21,13 +21,14 @@
 import os
 import shutil
 import tarfile
-from typing import cast
+from typing import List, Tuple, cast
 
 import click
 
 from aea.cli.registry.utils import (
     check_is_author_logged_in,
     clean_tarfiles,
+    list_missing_packages,
     request_api,
 )
 from aea.cli.utils.context import Context
@@ -39,6 +40,7 @@
     CONNECTIONS,
     CONTRACTS,
     DEFAULT_README_FILE,
+    ITEM_TYPE_PLURAL_TO_TYPE,
     PROTOCOLS,
     SKILLS,
 )
@@ -143,11 +145,20 @@ def push_item(ctx: Context, item_type: str, item_id: PublicId) -> None:
     }
 
     # dependencies
+    dependencies: List[Tuple[str, PublicId]] = []
     for key in [CONNECTIONS, CONTRACTS, PROTOCOLS, SKILLS]:
-        deps_list = item_config.get(key)
+        deps_list = item_config.get(key, [])
         if deps_list:
             data.update({key: deps_list})
+        for dep in deps_list:
+            dependencies.append((ITEM_TYPE_PLURAL_TO_TYPE[key], PublicId.from_str(dep)))
 
+    missing_dependencies = list_missing_packages(dependencies)
+
+    if missing_dependencies:
+        for package_type, package_id in missing_dependencies:
+            click.echo(f"Error: Cannot find {package_type} {package_id} in registry!")
+        raise click.ClickException("Found missing dependencies! Push canceled!")
     try:
         files = {"file": open(output_filepath, "rb")}
         readme_path = os.path.join(item_path, DEFAULT_README_FILE)
@@ -159,11 +170,11 @@ def push_item(ctx: Context, item_type: str, item_id: PublicId) -> None:
         resp = cast(
             JSONLike, request_api("POST", path, data=data, is_auth=True, files=files)
         )
+        click.echo(
+            "Successfully pushed {} {} to the Registry. Public ID: {}".format(
+                item_type, item_id.name, resp["public_id"]
+            )
+        )
     finally:
         for fd in files.values():
             fd.close()
-            click.echo(
-                "Successfully pushed {} {} to the Registry. Public ID: {}".format(
-                    item_type, item_id.name, resp["public_id"]
-                )
-            )

Diff for: aea/cli/registry/utils.py

@@ -17,11 +17,9 @@
 #
 # ------------------------------------------------------------------------------
 """Utils used for operating Registry with CLI."""
-
 import os
 import tarfile
-from json.decoder import JSONDecodeError
-from typing import Any, Callable, Dict, Optional, Tuple, Union, cast
+from typing import Any, Callable, Dict, List, Optional, Tuple, Union, cast
 
 import click
 
@@ -32,6 +30,7 @@
 from aea.cli.utils.package_utils import find_item_locally
 from aea.common import JSONLike
 from aea.configurations.base import PublicId
+from aea.configurations.constants import ITEM_TYPE_TO_PLURAL
 from aea.helpers import http_requests as requests
 
 
@@ -83,21 +82,12 @@ def request_api(
                 'Please sign in with "aea login" command.'
             )
         headers.update({"Authorization": "Token {}".format(token)})
-
-    request_kwargs = dict(
-        method=method,
-        url="{}{}".format(REGISTRY_API_URL, path),
-        params=params,
-        files=files,
-        data=data,
-        headers=headers,
-    )
     try:
-        resp = requests.request(**request_kwargs)
+        resp = _perform_registry_request(method, path, params, data, files, headers)
         resp_json = resp.json()
     except requests.exceptions.ConnectionError:
         raise click.ClickException("Registry server is not responding.")
-    except JSONDecodeError:
+    except requests.JSONDecodeError:
         resp_json = None
 
     if resp.status_code == 200:
@@ -139,6 +129,27 @@ def request_api(
     return resp_json
 
 
+def _perform_registry_request(
+    method: str,
+    path: str,
+    params: Optional[Dict] = None,
+    data: Optional[Dict] = None,
+    files: Optional[Dict] = None,
+    headers: Optional[Dict] = None,
+) -> requests.Response:
+    """Perform HTTP request and resturn response object."""
+    request_kwargs = dict(
+        method=method,
+        url="{}{}".format(REGISTRY_API_URL, path),
+        params=params,
+        files=files,
+        data=data,
+        headers=headers,
+    )
+    resp = requests.request(**request_kwargs)
+    return resp
+
+
 def download_file(url: str, cwd: str, timeout: float = FILE_DOWNLOAD_TIMEOUT) -> str:
     """
     Download file from URL and save it in CWD (current working directory).
@@ -322,3 +333,21 @@ def get_latest_version_available_in_registry(
         )
 
     return latest_item_public_id
+
+
+def list_missing_packages(
+    packages: List[Tuple[str, PublicId]]
+) -> List[Tuple[str, PublicId]]:
+    """Get list of packages not currently present in registry."""
+    result: List[Tuple[str, PublicId]] = []
+
+    for package_type, package_id in packages:
+        api_path = f"/{ITEM_TYPE_TO_PLURAL[package_type]}/{package_id.author}/{package_id.name}/{package_id.version}"
+        resp = _perform_registry_request("GET", api_path)
+        if resp.status_code == 404:
+            result.append((package_type, package_id))
+        elif resp.status_code == 200:
+            pass
+        else:  # pragma: nocover
+            raise ValueError("Error on registry request")
+    return result

Diff for: aea/helpers/http_requests.py

@@ -28,6 +28,15 @@
 DEFAULT_TIMEOUT = NETWORK_REQUEST_DEFAULT_TIMEOUT
 
 
+# requests can use one of these
+try:
+    from simplejson.errors import (  # type: ignore  # pylint: disable=unused-import
+        JSONDecodeError,
+    )
+except ModuleNotFoundError:  # pragma: nocover
+    from json.decoder import JSONDecodeError  # noqa  # pylint: disable=unused-import
+
+
 def add_default_timeout(fn: Callable, timeout: float) -> Callable:
     """Add default timeout for requests methods."""
 

Diff for: tests/test_cli/test_registry/test_push.py

@@ -19,7 +19,7 @@
 """Test module for Registry push methods."""
 import os
 from unittest import TestCase, mock
-from unittest.mock import mock_open
+from unittest.mock import mock_open, patch
 
 import pytest
 from click import ClickException
@@ -37,6 +37,7 @@
 
 @mock.patch("builtins.open", mock_open(read_data="opened_file"))
 @mock.patch("aea.cli.registry.push.check_is_author_logged_in")
+@mock.patch("aea.cli.registry.push.list_missing_packages", return_value=[])
 @mock.patch("aea.cli.registry.utils._rm_tarfiles")
 @mock.patch("aea.cli.registry.push.os.getcwd", return_value="cwd")
 @mock.patch("aea.cli.registry.push._compress_dir")
@@ -47,7 +48,7 @@
         "version": PublicIdMock.DEFAULT_VERSION,
         "author": "some_author",
         "name": "some_name",
-        "protocols": ["protocol_id"],
+        "protocols": ["some/protocol:0.1.2"],
     },
 )
 @mock.patch(
@@ -68,6 +69,7 @@ def test_push_item_positive(
         getcwd_mock,
         rm_tarfiles_mock,
         check_is_author_logged_in_mock,
+        *_
     ):
         """Test for push_item positive result."""
         public_id = PublicIdMock(
@@ -83,12 +85,42 @@ def test_push_item_positive(
                 "name": "some_name",
                 "description": "some-description",
                 "version": PublicIdMock.DEFAULT_VERSION,
-                "protocols": ["protocol_id"],
+                "protocols": ["some/protocol:0.1.2"],
             },
             is_auth=True,
             files={"file": open("file.1"), "readme": open("file.2")},
         )
 
+    @mock.patch("aea.cli.registry.push.os.path.exists", return_value=True)
+    @mock.patch("aea.cli.registry.push.is_readme_present", return_value=True)
+    def test_push_dependency_fail(
+        self,
+        is_readme_present_mock,
+        path_exists_mock,
+        request_api_mock,
+        load_yaml_mock,
+        compress_mock,
+        getcwd_mock,
+        rm_tarfiles_mock,
+        check_is_author_logged_in_mock,
+        *_
+    ):
+        """Test for push_item fails cause dependencies check."""
+        public_id = PublicIdMock(
+            name="some_name",
+            author="some_author",
+            version="{}".format(PublicIdMock.DEFAULT_VERSION),
+        )
+
+        with patch(
+            "aea.cli.registry.push.list_missing_packages",
+            return_value=[("some", PublicId.from_str("some/pack:0.1.0"))],
+        ):
+            with pytest.raises(
+                ClickException, match="Found missed dependencies! Push canceled!"
+            ):
+                push_item(ContextMock(), "some-type", public_id)
+
     @mock.patch("aea.cli.registry.push.os.path.exists", return_value=True)
     @mock.patch("aea.cli.registry.push.is_readme_present", return_value=False)
     def test_push_item_positive_without_readme(
@@ -108,7 +140,7 @@ def test_push_item_positive_without_readme(
                 "name": "some_name",
                 "description": "some-description",
                 "version": PublicIdMock.DEFAULT_VERSION,
-                "protocols": ["protocol_id"],
+                "protocols": ["some/protocol:0.1.2"],
             },
             is_auth=True,
             files={"file": open("opened_file", "r")},
@@ -124,6 +156,7 @@ def test_push_item_item_not_found(
         getcwd_mock,
         rm_tarfiles_mock,
         check_is_author_logged_in_mock,
+        *_
     ):
         """Test for push_item - item not found."""
         with self.assertRaises(ClickException):

Diff for: tests/test_cli/test_registry/test_utils.py

@@ -23,7 +23,7 @@
 from json.decoder import JSONDecodeError
 from pathlib import Path
 from unittest import TestCase, mock
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 import pytest
 from click import ClickException
@@ -41,6 +41,7 @@
     get_latest_version_available_in_registry,
     get_package_meta,
     is_auth_token_present,
+    list_missing_packages,
     request_api,
 )
 from aea.cli.utils.exceptions import AEAConfigException
@@ -394,3 +395,22 @@ def test_get_latest_version_available_in_registry_remote_mode(*_mocks):
         context_mock, "protocol", DefaultMessage.protocol_id
     )
     assert result == DefaultMessage.protocol_id
+
+
+def test_list_missing_packages():
+    """Test 'list_missing_packages'."""
+    packages = [("connection", PublicId.from_str("test/test:0.1.2"))]
+
+    resp_ok = MagicMock()
+    resp_ok.status_code = 200
+    with patch(
+        "aea.cli.registry.utils._perform_registry_request", return_value=resp_ok
+    ):
+        assert list_missing_packages(packages) == []
+
+    resp_404 = MagicMock()
+    resp_404.status_code = 404
+    with patch(
+        "aea.cli.registry.utils._perform_registry_request", return_value=resp_404
+    ):
+        assert list_missing_packages(packages) == packages