diff --git a/docs/endpoints/status.rst b/docs/endpoints/status.rst --- a/docs/endpoints/status.rst +++ b/docs/endpoints/status.rst @@ -58,7 +58,7 @@ done The deposit has been successfully loaded into the Software Heritage archive swh:1:dir:d83b7dda887dc790f7207608474650d4344b8df9 - swh:1:dir:d83b7dda887dc790f7207608474650d4344b8df9;origin=https://forge.softwareheritage.org/source/jesuisgpl/ + swh:1:dir:d83b7dda887dc790f7207608474650d4344b8df9;origin=https://forge.softwareheritage.org/source/jesuisgpl/;visit=swh:1:snp:68c0d26104d47e278dd6be07ed61fafb561d0d20;anchor=swh:1:rev:e76ea49c9ffbb7f73611087ba6e999b19e5d71eb;path=/ swh:1:rev:e76ea49c9ffbb7f73611087ba6e999b19e5d71eb swh:1:rev:e76ea49c9ffbb7f73611087ba6e999b19e5d71eb;origin=https://forge.softwareheritage.org/source/jesuisgpl/ diff --git a/docs/getting-started.rst b/docs/getting-started.rst --- a/docs/getting-started.rst +++ b/docs/getting-started.rst @@ -279,7 +279,7 @@ 'deposit_id': '11', 'deposit_status': 'done', 'deposit_swh_id': 'swh:1:dir:d83b7dda887dc790f7207608474650d4344b8df9', - 'deposit_swh_id_context': 'swh:1:dir:d83b7dda887dc790f7207608474650d4344b8df9;origin=https://forge.softwareheritage.org/source/jesuisgpl/', + 'deposit_swh_id_context': 'swh:1:dir:d83b7dda887dc790f7207608474650d4344b8df9;origin=https://forge.softwareheritage.org/source/jesuisgpl/;visit=swh:1:snp:68c0d26104d47e278dd6be07ed61fafb561d0d20;anchor=swh:1:rev:e76ea49c9ffbb7f73611087ba6e999b19e5d71eb;path=/', 'deposit_swh_anchor_id': 'swh:1:rev:e76ea49c9ffbb7f73611087ba6e999b19e5d71eb', 'deposit_swh_anchor_id_context': 'swh:1:rev:e76ea49c9ffbb7f73611087ba6e999b19e5d71eb;origin=https://forge.softwareheritage.org/source/jesuisgpl/', 'deposit_status_detail': 'The deposit has been successfully \ diff --git a/requirements-swh-server.txt b/requirements-swh-server.txt --- a/requirements-swh-server.txt +++ b/requirements-swh-server.txt @@ -1,4 +1,4 @@ swh.core[http] swh.loader.core >= 0.0.71 swh.scheduler >= 0.0.39 -swh.model >= 0.0.26 +swh.model >= 0.1.0 diff --git a/swh/deposit/api/private/deposit_update_status.py b/swh/deposit/api/private/deposit_update_status.py --- a/swh/deposit/api/private/deposit_update_status.py +++ b/swh/deposit/api/private/deposit_update_status.py @@ -1,11 +1,11 @@ -# 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 General Public License version 3, or any later version # See top-level LICENSE file for more information from rest_framework.parsers import JSONParser -from swh.model.identifiers import persistent_identifier, REVISION, DIRECTORY +from swh.model.identifiers import DIRECTORY, persistent_identifier, REVISION, SNAPSHOT from . import SWHPrivateAPIView from ..common import SWHPutDepositAPI @@ -14,6 +14,9 @@ from ...models import DEPOSIT_STATUS_LOAD_SUCCESS +MANDATORY_KEYS = ["origin_url", "revision_id", "directory_id", "snapshot_id"] + + class SWHUpdateStatusDeposit(SWHPrivateAPIView, SWHPutDepositAPI): """Deposit request class to update the deposit's status. @@ -29,6 +32,7 @@ New checks: - Ensure the status is provided - Ensure it exists + - no missing information on load success update """ data = request.data @@ -44,38 +48,63 @@ return make_error_dict(BAD_REQUEST, msg) if status == DEPOSIT_STATUS_LOAD_SUCCESS: - swh_id = data.get("revision_id") - if not swh_id: - msg = "Updating status to %s requires a revision_id key" % (status,) + missing_keys = [] + for key in MANDATORY_KEYS: + value = data.get(key) + if value is None: + missing_keys.append(key) + + if missing_keys: + msg = ( + f"Updating deposit status to {status}" + f" requires information {','.join(missing_keys)}" + ) return make_error_dict(BAD_REQUEST, msg) return {} def process_put(self, request, headers, collection_name, deposit_id): - """Update the deposit's status + """Update the deposit with status and SWHIDs Returns: 204 No content + 400 Bad request if checks fail """ - deposit = Deposit.objects.get(pk=deposit_id) - deposit.status = request.data["status"] # checks already done before + data = request.data - origin_url = request.data.get("origin_url") + deposit = Deposit.objects.get(pk=deposit_id) - dir_id = request.data.get("directory_id") - if dir_id: - deposit.swh_id = persistent_identifier(DIRECTORY, dir_id) + status = data["status"] + deposit.status = status + if status == DEPOSIT_STATUS_LOAD_SUCCESS: + origin_url = data["origin_url"] + directory_id = data["directory_id"] + revision_id = data["revision_id"] + dir_id = persistent_identifier(DIRECTORY, directory_id) + snp_id = persistent_identifier(SNAPSHOT, data["snapshot_id"]) + rev_id = persistent_identifier(REVISION, revision_id) + + deposit.swh_id = dir_id + # new id with contextual information deposit.swh_id_context = persistent_identifier( - DIRECTORY, dir_id, metadata={"origin": origin_url} + DIRECTORY, + directory_id, + metadata={ + "origin": origin_url, + "visit": snp_id, + "anchor": rev_id, + "path": "/", + }, ) - rev_id = request.data.get("revision_id") - if rev_id: - deposit.swh_anchor_id = persistent_identifier(REVISION, rev_id) + # backward compatibility for now + deposit.swh_anchor_id = rev_id deposit.swh_anchor_id_context = persistent_identifier( - REVISION, rev_id, metadata={"origin": origin_url} + REVISION, revision_id, metadata={"origin": origin_url} ) + else: # rejected + deposit.status = status deposit.save() diff --git a/swh/deposit/tests/api/test_deposit_private_update_status.py b/swh/deposit/tests/api/test_deposit_private_update_status.py --- a/swh/deposit/tests/api/test_deposit_private_update_status.py +++ b/swh/deposit/tests/api/test_deposit_private_update_status.py @@ -1,18 +1,23 @@ -# 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 General Public License version 3, or any later version # See top-level LICENSE file for more information +import copy import json from django.urls import reverse from rest_framework import status -from swh.deposit.models import Deposit, DEPOSIT_STATUS_DETAIL +from swh.model.identifiers import DIRECTORY, persistent_identifier, REVISION, SNAPSHOT + +from swh.deposit.api.private.deposit_update_status import MANDATORY_KEYS + +from swh.deposit.models import Deposit from swh.deposit.config import ( PRIVATE_PUT_DEPOSIT, - DEPOSIT_STATUS_VERIFIED, DEPOSIT_STATUS_LOAD_SUCCESS, + DEPOSIT_STATUS_LOAD_FAILURE, ) @@ -27,84 +32,125 @@ ] -def test_update_deposit_status( +def test_update_deposit_status_success_with_info( authenticated_client, deposit_collection, ready_deposit_verified ): - """Existing status for update should return a 204 response + """Update deposit with load success should require all information to succeed """ deposit = ready_deposit_verified + expected_status = DEPOSIT_STATUS_LOAD_SUCCESS + origin_url = "something" + directory_id = "42a13fc721c8716ff695d0d62fc851d641f3a12b" + revision_id = "47dc6b4636c7f6cba0df83e3d5490bf4334d987e" + snapshot_id = "68c0d26104d47e278dd6be07ed61fafb561d0d20" + + full_body_info = { + "status": DEPOSIT_STATUS_LOAD_SUCCESS, + "revision_id": revision_id, + "directory_id": directory_id, + "snapshot_id": snapshot_id, + "origin_url": origin_url, + } for url in private_check_url_endpoints(deposit_collection, deposit): - possible_status = set(DEPOSIT_STATUS_DETAIL.keys()) - set( - [DEPOSIT_STATUS_LOAD_SUCCESS] + dir_id = persistent_identifier(DIRECTORY, directory_id) + rev_id = persistent_identifier(REVISION, revision_id) + snp_id = persistent_identifier(SNAPSHOT, snapshot_id) + + expected_swh_id = "swh:1:dir:%s" % directory_id + expected_swh_id_context = ( + f"{dir_id};origin={origin_url};" + f"visit={snp_id};anchor={rev_id};path=/" ) + expected_swh_anchor_id = rev_id + expected_swh_anchor_id_context = f"{rev_id};origin={origin_url}" - for _status in possible_status: - response = authenticated_client.put( - url, - content_type="application/json", - data=json.dumps({"status": _status}), - ) + response = authenticated_client.put( + url, content_type="application/json", data=json.dumps(full_body_info), + ) - assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.status_code == status.HTTP_204_NO_CONTENT - deposit = Deposit.objects.get(pk=deposit.id) - assert deposit.status == _status + deposit = Deposit.objects.get(pk=deposit.id) + assert deposit.status == expected_status + assert deposit.swh_id == expected_swh_id + assert deposit.swh_id_context == expected_swh_id_context + assert deposit.swh_anchor_id == expected_swh_anchor_id + assert deposit.swh_anchor_id_context == expected_swh_anchor_id_context - deposit.status = DEPOSIT_STATUS_VERIFIED - deposit.save() # hack the same deposit + # Reset deposit + deposit = ready_deposit_verified + deposit.save() -def test_update_deposit_status_with_info( +def test_update_deposit_status_rejected_with_info( authenticated_client, deposit_collection, ready_deposit_verified ): - """Existing status for update with info should return a 204 response + """Update deposit with rejected status needs few information to succeed """ deposit = ready_deposit_verified - for url in private_check_url_endpoints(deposit_collection, deposit): - expected_status = DEPOSIT_STATUS_LOAD_SUCCESS - origin_url = "something" - directory_id = "42a13fc721c8716ff695d0d62fc851d641f3a12b" - revision_id = "47dc6b4636c7f6cba0df83e3d5490bf4334d987e" - expected_swh_id = "swh:1:dir:%s" % directory_id - expected_swh_id_context = "swh:1:dir:%s;origin=%s" % (directory_id, origin_url) - expected_swh_anchor_id = "swh:1:rev:%s" % revision_id - expected_swh_anchor_id_context = "swh:1:rev:%s;origin=%s" % ( - revision_id, - origin_url, - ) + for url in private_check_url_endpoints(deposit_collection, deposit): response = authenticated_client.put( url, content_type="application/json", - data=json.dumps( - { - "status": expected_status, - "revision_id": revision_id, - "directory_id": directory_id, - "origin_url": origin_url, - } - ), + data=json.dumps({"status": DEPOSIT_STATUS_LOAD_FAILURE}), ) assert response.status_code == status.HTTP_204_NO_CONTENT deposit = Deposit.objects.get(pk=deposit.id) - assert deposit.status == expected_status - assert deposit.swh_id == expected_swh_id - assert deposit.swh_id_context == expected_swh_id_context - assert deposit.swh_anchor_id == expected_swh_anchor_id - assert deposit.swh_anchor_id_context == expected_swh_anchor_id_context + assert deposit.status == DEPOSIT_STATUS_LOAD_FAILURE + + assert deposit.swh_id is None + assert deposit.swh_id_context is None + assert deposit.swh_anchor_id is None + assert deposit.swh_anchor_id_context is None - deposit.swh_id = None - deposit.swh_id_context = None - deposit.swh_anchor_id = None - deposit.swh_anchor_id_context = None - deposit.status = DEPOSIT_STATUS_VERIFIED + # Reset status + deposit = ready_deposit_verified deposit.save() +def test_update_deposit_status_success_with_incomplete_data( + authenticated_client, deposit_collection, ready_deposit_verified +): + """Update deposit status with status success and incomplete information should fail + + """ + deposit = ready_deposit_verified + + origin_url = "something" + directory_id = "42a13fc721c8716ff695d0d62fc851d641f3a12b" + revision_id = "47dc6b4636c7f6cba0df83e3d5490bf4334d987e" + snapshot_id = "68c0d26104d47e278dd6be07ed61fafb561d0d20" + + new_status = DEPOSIT_STATUS_LOAD_SUCCESS + full_body_info = { + "status": new_status, + "revision_id": revision_id, + "directory_id": directory_id, + "snapshot_id": snapshot_id, + "origin_url": origin_url, + } + + for url in private_check_url_endpoints(deposit_collection, deposit): + for key in MANDATORY_KEYS: + # Crafting body with missing information so that it raises + body = copy.deepcopy(full_body_info) + body.pop(key) # make the body incomplete + + response = authenticated_client.put( + url, content_type="application/json", data=json.dumps(body), + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert ( + f"deposit status to {new_status} requires information {key}" + in response.content.decode("utf-8") + ) + + def test_update_deposit_status_will_fail_with_unknown_status( authenticated_client, deposit_collection, ready_deposit_verified ):