diff --git a/requirements-swh.txt b/requirements-swh.txt --- a/requirements-swh.txt +++ b/requirements-swh.txt @@ -4,4 +4,4 @@ swh.scheduler >= 0.0.72 swh.search >= 0.0.4 swh.storage >= 0.0.182 -swh.vault >= 0.0.32 \ No newline at end of file +swh.vault >= 0.0.33 \ No newline at end of file diff --git a/swh/web/api/views/vault.py b/swh/web/api/views/vault.py --- a/swh/web/api/views/vault.py +++ b/swh/web/api/views/vault.py @@ -1,4 +1,4 @@ -# Copyright (C) 2015-2019 The Software Heritage developers +# Copyright (C) 2015-2020 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -18,13 +18,15 @@ # XXX: a bit spaghetti. Would be better with class-based views. def _dispatch_cook_progress(request, obj_type, obj_id): hex_id = hashutil.hash_to_hex(obj_id) - object_name = obj_type.split("_")[0].title() + object_name = obj_type.split("_")[0] if request.method == "GET": return api_lookup( service.vault_progress, obj_type, obj_id, - notfound_msg=("{} '{}' was never requested.".format(object_name, hex_id)), + notfound_msg=( + "Cooking of {} '{}' was never requested.".format(object_name, hex_id) + ), request=request, ) elif request.method == "POST": @@ -34,7 +36,7 @@ obj_type, obj_id, email, - notfound_msg=("{} '{}' not found.".format(object_name, hex_id)), + notfound_msg=("{} '{}' not found.".format(object_name.title(), hex_id)), request=request, ) @@ -136,7 +138,7 @@ service.vault_fetch, "directory", obj_id, - notfound_msg="Directory with ID '{}' not found.".format(dir_id), + notfound_msg="Cooked archive for directory '{}' not found.".format(dir_id), request=request, ) fname = "{}.tar.gz".format(dir_id) @@ -243,7 +245,7 @@ service.vault_fetch, "revision_gitfast", obj_id, - notfound_msg="Revision with ID '{}' not found.".format(rev_id), + notfound_msg="Cooked archive for revision '{}' not found.".format(rev_id), request=request, ) fname = "{}.gitfast.gz".format(rev_id) diff --git a/swh/web/common/service.py b/swh/web/common/service.py --- a/swh/web/common/service.py +++ b/swh/web/common/service.py @@ -11,10 +11,9 @@ from typing import Any, Dict, List, Set, Iterator, Optional, Tuple from swh.model import hashutil - -from swh.storage.algos import diff, revisions_walker - from swh.model.identifiers import CONTENT, DIRECTORY, RELEASE, REVISION, SNAPSHOT +from swh.storage.algos import diff, revisions_walker +from swh.vault.exc import NotFoundExc as VaultNotFoundExc from swh.web import config from swh.web.common import converters from swh.web.common import query @@ -1090,22 +1089,29 @@ return (rev["id"], lookup_directory_with_revision(rev["id"], path, with_data)) +def _vault_request(vault_fn, *args, **kwargs): + try: + return vault_fn(*args, **kwargs) + except VaultNotFoundExc: + return None + + def vault_cook(obj_type, obj_id, email=None): """Cook a vault bundle. """ - return vault.cook(obj_type, obj_id, email=email) + return _vault_request(vault.cook, obj_type, obj_id, email=email) def vault_fetch(obj_type, obj_id): """Fetch a vault bundle. """ - return vault.fetch(obj_type, obj_id) + return _vault_request(vault.fetch, obj_type, obj_id) def vault_progress(obj_type, obj_id): """Get the current progress of a vault bundle. """ - return vault.progress(obj_type, obj_id) + return _vault_request(vault.progress, obj_type, obj_id) def diff_revision(rev_id): diff --git a/swh/web/tests/api/views/test_vault.py b/swh/web/tests/api/views/test_vault.py --- a/swh/web/tests/api/views/test_vault.py +++ b/swh/web/tests/api/views/test_vault.py @@ -1,28 +1,36 @@ -# Copyright (C) 2017-2019 The Software Heritage developers +# Copyright (C) 2017-2020 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information +from hypothesis import given + from swh.model import hashutil +from swh.vault.exc import NotFoundExc from swh.web.common.utils import reverse - -TEST_OBJ_ID = "d4905454cc154b492bd6afed48694ae3c579345e" - -OBJECT_TYPES = ("directory", "revision_gitfast") +from swh.web.tests.strategies import ( + directory, + revision, + unknown_directory, + unknown_revision, +) -def test_api_vault_cook(api_client, mocker): +@given(directory(), revision()) +def test_api_vault_cook(api_client, mocker, directory, revision): mock_service = mocker.patch("swh.web.api.views.vault.service") - for obj_type in OBJECT_TYPES: + for obj_type, obj_id in ( + ("directory", directory), + ("revision_gitfast", revision), + ): fetch_url = reverse( - f"api-1-vault-fetch-{obj_type}", - url_args={f"{obj_type[:3]}_id": TEST_OBJ_ID}, + f"api-1-vault-fetch-{obj_type}", url_args={f"{obj_type[:3]}_id": obj_id}, ) stub_cook = { "fetch_url": fetch_url, - "obj_id": TEST_OBJ_ID, + "obj_id": obj_id, "obj_type": obj_type, "progress_message": None, "status": "done", @@ -34,7 +42,7 @@ mock_service.vault_fetch.return_value = stub_fetch url = reverse( - f"api-1-vault-cook-{obj_type}", url_args={f"{obj_type[:3]}_id": TEST_OBJ_ID} + f"api-1-vault-cook-{obj_type}", url_args={f"{obj_type[:3]}_id": obj_id} ) rv = api_client.post(url, {"email": "test@test.mail"}) @@ -48,7 +56,7 @@ assert rv.data == stub_cook mock_service.vault_cook.assert_called_with( - obj_type, hashutil.hash_to_bytes(TEST_OBJ_ID), "test@test.mail" + obj_type, hashutil.hash_to_bytes(obj_id), "test@test.mail" ) rv = api_client.get(fetch_url) @@ -57,31 +65,35 @@ assert rv["Content-Type"] == "application/gzip" assert rv.content == stub_fetch mock_service.vault_fetch.assert_called_with( - obj_type, hashutil.hash_to_bytes(TEST_OBJ_ID) + obj_type, hashutil.hash_to_bytes(obj_id) ) -def test_api_vault_cook_uppercase_hash(api_client): +@given(directory(), revision()) +def test_api_vault_cook_uppercase_hash(api_client, directory, revision): - for obj_type in OBJECT_TYPES: + for obj_type, obj_id in ( + ("directory", directory), + ("revision_gitfast", revision), + ): url = reverse( f"api-1-vault-cook-{obj_type}-uppercase-checksum", - url_args={f"{obj_type[:3]}_id": TEST_OBJ_ID.upper()}, + url_args={f"{obj_type[:3]}_id": obj_id.upper()}, ) rv = api_client.post(url, {"email": "test@test.mail"}) assert rv.status_code == 302 redirect_url = reverse( - f"api-1-vault-cook-{obj_type}", url_args={f"{obj_type[:3]}_id": TEST_OBJ_ID} + f"api-1-vault-cook-{obj_type}", url_args={f"{obj_type[:3]}_id": obj_id} ) assert rv["location"] == redirect_url fetch_url = reverse( f"api-1-vault-fetch-{obj_type}-uppercase-checksum", - url_args={f"{obj_type[:3]}_id": TEST_OBJ_ID.upper()}, + url_args={f"{obj_type[:3]}_id": obj_id.upper()}, ) rv = api_client.get(fetch_url) @@ -89,21 +101,51 @@ assert rv.status_code == 302 redirect_url = reverse( - f"api-1-vault-fetch-{obj_type}", - url_args={f"{obj_type[:3]}_id": TEST_OBJ_ID}, + f"api-1-vault-fetch-{obj_type}", url_args={f"{obj_type[:3]}_id": obj_id}, ) assert rv["location"] == redirect_url -def test_api_vault_cook_notfound(api_client, mocker): - mock_service = mocker.patch("swh.web.api.views.vault.service") - mock_service.vault_cook.return_value = None - mock_service.vault_fetch.return_value = None +@given(directory(), revision(), unknown_directory(), unknown_revision()) +def test_api_vault_cook_notfound( + api_client, mocker, directory, revision, unknown_directory, unknown_revision +): + mock_vault = mocker.patch("swh.web.common.service.vault") + mock_vault.cook.side_effect = NotFoundExc("object not found") + mock_vault.fetch.side_effect = NotFoundExc("cooked archive not found") + mock_vault.progress.side_effect = NotFoundExc("cooking request not found") + + for obj_type, obj_id in ( + ("directory", directory), + ("revision_gitfast", revision), + ): + + obj_name = obj_type.split("_")[0] + + url = reverse( + f"api-1-vault-cook-{obj_type}", url_args={f"{obj_type[:3]}_id": obj_id}, + ) + + rv = api_client.get(url) + + assert rv.status_code == 404, rv.data + assert rv["Content-Type"] == "application/json" + assert rv.data["exception"] == "NotFoundExc" + assert ( + rv.data["reason"] + == f"Cooking of {obj_name} '{obj_id}' was never requested." + ) + mock_vault.progress.assert_called_with(obj_type, hashutil.hash_to_bytes(obj_id)) + + for obj_type, obj_id in ( + ("directory", unknown_directory), + ("revision_gitfast", unknown_revision), + ): + obj_name = obj_type.split("_")[0] - for obj_type in OBJECT_TYPES: url = reverse( - f"api-1-vault-cook-{obj_type}", url_args={f"{obj_type[:3]}_id": TEST_OBJ_ID} + f"api-1-vault-cook-{obj_type}", url_args={f"{obj_type[:3]}_id": obj_id} ) rv = api_client.post(url) @@ -111,13 +153,13 @@ assert rv["Content-Type"] == "application/json" assert rv.data["exception"] == "NotFoundExc" - mock_service.vault_cook.assert_called_with( - obj_type, hashutil.hash_to_bytes(TEST_OBJ_ID), None + assert rv.data["reason"] == f"{obj_name.title()} '{obj_id}' not found." + mock_vault.cook.assert_called_with( + obj_type, hashutil.hash_to_bytes(obj_id), email=None ) fetch_url = reverse( - f"api-1-vault-fetch-{obj_type}", - url_args={f"{obj_type[:3]}_id": TEST_OBJ_ID}, + f"api-1-vault-fetch-{obj_type}", url_args={f"{obj_type[:3]}_id": obj_id}, ) rv = api_client.get(fetch_url) @@ -125,6 +167,7 @@ assert rv.status_code == 404, rv.data assert rv["Content-Type"] == "application/json" assert rv.data["exception"] == "NotFoundExc" - mock_service.vault_fetch.assert_called_with( - obj_type, hashutil.hash_to_bytes(TEST_OBJ_ID) + assert ( + rv.data["reason"] == f"Cooked archive for {obj_name} '{obj_id}' not found." ) + mock_vault.fetch.assert_called_with(obj_type, hashutil.hash_to_bytes(obj_id))