diff --git a/docs/specs/spec-loading.rst b/docs/specs/spec-loading.rst index c1420b4c091c48a949a769a25318e9b2ad655c33..171400794f0f35a54feef6d4ec21c8c53bddf93f 100644 --- a/docs/specs/spec-loading.rst +++ b/docs/specs/spec-loading.rst @@ -316,7 +316,6 @@ It contains the same fields as any revision object; in particular: "codemeta:url": "https://hal.archives-ouvertes.fr/hal-01883795", "codemeta:version": "1", "committer": "David Picard", - "external_identifier": "hal-01883795", "id": "hal-01883795" }, "parents": [], diff --git a/swh/deposit/api/checks.py b/swh/deposit/api/checks.py index 3b942a5c59613320d1cc8a0a2e230fa4e4b1026f..8237ff59bb6a893fed45daf3bac05ae949432130 100644 --- a/swh/deposit/api/checks.py +++ b/swh/deposit/api/checks.py @@ -61,9 +61,6 @@ ATOM_ELEMENTS = [ "summary", "title", "updated", - # we used to recommend this, so we still need to support it so we don't break - # existing clients - "external_identifier", ] # from https://github.com/codemeta/codemeta/blob/2.0/codemeta.jsonld @@ -350,7 +347,16 @@ def check_metadata(metadata: ElementTree.Element) -> Tuple[bool, Optional[Dict]] if element.tag.startswith("{http://www.w3.org/2005/Atom}"): _, local_name = element.tag.split("}", 1) if local_name not in ATOM_ELEMENTS: - if local_name in CODEMETA2_CONTEXT: + if local_name == "external_identifier": + detail.append( + { + "fields": [local_name], + "summary": "<external_identifier> is not supported anymore, " + "<swh:create_origin> or <swh:add_to_origin> should be used " + "instead.", + } + ) + elif local_name in CODEMETA2_CONTEXT: # Probably confused the two namespaces, display a nicer error detail.append( { diff --git a/swh/deposit/api/common.py b/swh/deposit/api/common.py index 7e970d0517d396c41c368d847203905de49a3038..7f82365e996ffd0effb64d9a0ccbf688365f1c03 100644 --- a/swh/deposit/api/common.py +++ b/swh/deposit/api/common.py @@ -29,11 +29,7 @@ from swh.deposit.api.converters import convert_status_detail from swh.deposit.auth import HasDepositPermission, KeycloakBasicAuthentication from swh.deposit.models import DEPOSIT_METADATA_ONLY, Deposit from swh.deposit.parsers import parse_xml -from swh.deposit.utils import ( - NAMESPACES, - compute_metadata_context, - parse_swh_metadata_provenance, -) +from swh.deposit.utils import compute_metadata_context, parse_swh_metadata_provenance from swh.model import hashutil from swh.model.model import ( MetadataAuthority, @@ -945,29 +941,6 @@ class APIBase(APIConfig, APIView, metaclass=ABCMeta): ) deposit.origin_url = origin_url - external_identifier_element = metadata.find( - "atom:external_identifier", namespaces=NAMESPACES - ) - if external_identifier_element is not None: - # Deprecated tag. - # When clients stopped using it, this should raise an error - # unconditionally - - if deposit.origin_url: - raise DepositError( - BAD_REQUEST, - "<external_identifier> is deprecated, you should only use " - "<swh:create_origin> and <swh:add_to_origin> from now on.", - ) - - if headers.slug and external_identifier_element.text != headers.slug: - raise DepositError( - BAD_REQUEST, - "The <external_identifier> tag and Slug header are deprecated, " - "<swh:create_origin> or <swh:add_to_origin> " - "should be used instead.", - ) - def _empty_post( self, request: Request, diff --git a/swh/deposit/tests/api/test_collection_add_to_origin.py b/swh/deposit/tests/api/test_collection_add_to_origin.py index 666bedc676fbc6d4954739f8944b868608f76b28..91cef83f86238d1f9902c0b38c29dedb35f43b18 100644 --- a/swh/deposit/tests/api/test_collection_add_to_origin.py +++ b/swh/deposit/tests/api/test_collection_add_to_origin.py @@ -109,30 +109,6 @@ def test_add_deposit_add_to_wrong_origin( assert b"must start with" in response.content -def test_add_deposit_with_add_to_origin_and_external_identifier( - authenticated_client, - deposit_collection, - completed_deposit, - atom_dataset, - deposit_user, -): - """Posting deposit with <swh:add_to_origin> creates a new deposit with parent""" - # given multiple deposit already loaded - origin_url = deposit_user.provider_url + completed_deposit.external_id - - # adding a new deposit with the same external id as a completed deposit - # creates the parenting chain - response = post_atom( - authenticated_client, - reverse(COL_IRI, args=[deposit_collection.name]), - data=atom_dataset["entry-data-with-both-add-to-origin-and-external-id"] - % origin_url, - ) - - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert b"<external_identifier> is deprecated" in response.content - - def test_post_deposit_atom_403_add_to_wrong_origin_url_prefix( authenticated_client, deposit_collection, atom_dataset, deposit_user ): diff --git a/swh/deposit/tests/api/test_collection_post_atom.py b/swh/deposit/tests/api/test_collection_post_atom.py index a613b4ec40692551eafc2ff25c0342c2642bdc64..7b2b0f2c21e23bc77f91ca3c93c6eb47bfba8708 100644 --- a/swh/deposit/tests/api/test_collection_post_atom.py +++ b/swh/deposit/tests/api/test_collection_post_atom.py @@ -286,90 +286,6 @@ def test_post_deposit_atom_no_origin_url_nor_slug_header( assert deposit.status == DEPOSIT_STATUS_DEPOSITED -def test_post_deposit_atom_with_slug_and_external_identifier( - authenticated_client, deposit_collection, deposit_user, atom_dataset, mocker -): - """Even though <external_identifier> is deprecated, it should still be - allowed when it matches the slug, so that we don't break existing clients - - """ - url = reverse(COL_IRI, args=[deposit_collection.name]) - - slug = str(uuid.uuid4()) - - # when - response = post_atom( - authenticated_client, - url, - data=atom_dataset["error-with-external-identifier"] % slug, - HTTP_IN_PROGRESS="false", - HTTP_SLUG=slug, - ) - - assert response.status_code == status.HTTP_201_CREATED - response_content = ElementTree.fromstring(response.content) - deposit_id = int(response_content.findtext("swh:deposit_id", namespaces=NAMESPACES)) - - deposit = Deposit.objects.get(pk=deposit_id) - assert deposit.collection == deposit_collection - assert deposit.origin_url == deposit_user.provider_url + slug - assert deposit.status == DEPOSIT_STATUS_DEPOSITED - - -def test_post_deposit_atom_with_mismatched_slug_and_external_identifier( - authenticated_client, deposit_collection, atom_dataset -): - """Posting an atom entry with mismatched slug header and external_identifier - should return a 400 - - """ - external_id = "foobar" - url = reverse(COL_IRI, args=[deposit_collection.name]) - - # when - response = post_atom( - authenticated_client, - url, - data=atom_dataset["error-with-external-identifier"] % external_id, - HTTP_IN_PROGRESS="false", - HTTP_SLUG="something", - ) - - assert ( - b"The <external_identifier> tag and Slug header are deprecated" - in response.content - ) - assert response.status_code == status.HTTP_400_BAD_REQUEST - - -def test_post_deposit_atom_with_create_origin_and_external_identifier( - authenticated_client, deposit_collection, atom_dataset, deposit_user -): - """<atom:external_identifier> was deprecated before <swh:create_origin> - was introduced, clients should get an error when trying to use both - - """ - external_id = "foobar" - origin_url = deposit_user.provider_url + external_id - url = reverse(COL_IRI, args=[deposit_collection.name]) - - document = atom_dataset["error-with-external-identifier-and-create-origin"].format( - external_id=external_id, - url=origin_url, - ) - - # when - response = post_atom( - authenticated_client, - url, - data=document, - HTTP_IN_PROGRESS="false", - ) - - assert b"<external_identifier> is deprecated" in response.content - assert response.status_code == status.HTTP_400_BAD_REQUEST - - def test_post_deposit_atom_with_create_origin_and_reference( authenticated_client, deposit_collection, atom_dataset, deposit_user ): diff --git a/swh/deposit/tests/api/test_collection_reuse_slug.py b/swh/deposit/tests/api/test_collection_reuse_slug.py index ed6fe1c063e2f9f769c70255b041b582a95d1dd1..6968f51bdd0c44ac30589e16a05eed0f76e4306c 100644 --- a/swh/deposit/tests/api/test_collection_reuse_slug.py +++ b/swh/deposit/tests/api/test_collection_reuse_slug.py @@ -145,46 +145,6 @@ def test_add_deposit_when_done_makes_new_deposit_with_parent_old_one( assert new_deposit.origin_url == origin_url -def test_add_deposit_with_external_identifier( - authenticated_client, - deposit_collection, - completed_deposit, - atom_dataset, - deposit_user, -): - """Even though <external_identifier> is deprecated, it should still be - allowed when it matches the slug, so that we don't break existing clients - - """ - # given multiple deposit already loaded - deposit = completed_deposit - assert deposit.status == DEPOSIT_STATUS_LOAD_SUCCESS - origin_url = deposit_user.provider_url + deposit.external_id - - # adding a new deposit with the same external id as a completed deposit - # creates the parenting chain - response = post_atom( - authenticated_client, - reverse(COL_IRI, args=[deposit_collection.name]), - data=atom_dataset["error-with-external-identifier"] % deposit.external_id, - HTTP_SLUG=deposit.external_id, - ) - - assert response.status_code == status.HTTP_201_CREATED - response_content = parse_xml(response.content) - deposit_id = int(response_content.findtext("swh:deposit_id", namespaces=NAMESPACES)) - - assert deposit_id != deposit.id - - new_deposit = Deposit.objects.get(pk=deposit_id) - assert deposit.collection == new_deposit.collection - assert deposit.origin_url == origin_url - - assert new_deposit != deposit - assert new_deposit.parent == deposit - assert new_deposit.origin_url == origin_url - - def test_add_deposit_external_id_conflict_no_parent( authenticated_client, deposit_collection, diff --git a/swh/deposit/tests/api/test_deposit_update_atom.py b/swh/deposit/tests/api/test_deposit_update_atom.py index 4809d982dd0d02904b57a6576ae820191e7512f4..d8ac723e806d8bda3594c7134f7557436b25de0b 100644 --- a/swh/deposit/tests/api/test_deposit_update_atom.py +++ b/swh/deposit/tests/api/test_deposit_update_atom.py @@ -523,43 +523,6 @@ def test_put_update_metadata_done_deposit_failure_functional_checks( assert msg in response.content -def test_put_atom_with_create_origin_and_external_identifier( - authenticated_client, deposit_collection, atom_dataset, deposit_user -): - """<atom:external_identifier> was deprecated before <swh:create_origin> - was introduced, clients should get an error when trying to use both - - """ - external_id = "foobar" - origin_url = deposit_user.provider_url + external_id - url = reverse(COL_IRI, args=[deposit_collection.name]) - - response = post_atom( - authenticated_client, - url, - data=atom_dataset["entry-data0"] % origin_url, - HTTP_IN_PROGRESS="true", - ) - - assert response.status_code == status.HTTP_201_CREATED - response_content = parse_xml(response.content) - - edit_iri = response_content.find( - "atom:link[@rel='edit']", namespaces=NAMESPACES - ).attrib["href"] - - # when - response = put_atom( - authenticated_client, - edit_iri, - data=atom_dataset["error-with-external-identifier"] % external_id, - HTTP_IN_PROGRESS="false", - ) - - assert b"<external_identifier> is deprecated" in response.content - assert response.status_code == status.HTTP_400_BAD_REQUEST - - def test_put_atom_with_create_origin_and_reference( authenticated_client, deposit_collection, atom_dataset, deposit_user ): diff --git a/swh/deposit/tests/data/atom/entry-data-with-both-add-to-origin-and-external-id.xml b/swh/deposit/tests/data/atom/entry-data-with-both-add-to-origin-and-external-id.xml deleted file mode 100644 index 9c188f84267c8e9db1e0db5f7904adf3dca3ce9b..0000000000000000000000000000000000000000 --- a/swh/deposit/tests/data/atom/entry-data-with-both-add-to-origin-and-external-id.xml +++ /dev/null @@ -1,14 +0,0 @@ -<?xml version="1.0"?> -<entry xmlns="http://www.w3.org/2005/Atom" - xmlns:codemeta="https://doi.org/10.5063/SCHEMA/CODEMETA-2.0" - xmlns:swh="https://www.softwareheritage.org/schema/2018/deposit"> - <title>Awesome Compiler</title> - <id>urn:uuid:1225c695-cfb8-4ebb-aaaa-80da344efa6a</id> - <author>dudess</author> - <external_identifier>foo</external_identifier> - <swh:deposit> - <swh:add_to_origin> - <swh:origin url="%s" /> - </swh:add_to_origin> - </swh:deposit> -</entry> diff --git a/swh/deposit/tests/data/atom/error-with-external-identifier-and-create-origin.xml b/swh/deposit/tests/data/atom/error-with-external-identifier-and-create-origin.xml deleted file mode 100644 index 4b6436f3f798fa4c84547e9f24e99b5419cd0e3f..0000000000000000000000000000000000000000 --- a/swh/deposit/tests/data/atom/error-with-external-identifier-and-create-origin.xml +++ /dev/null @@ -1,14 +0,0 @@ -<?xml version="1.0" encoding="utf-8"?> -<entry xmlns="http://www.w3.org/2005/Atom" - xmlns:codemeta="https://doi.org/10.5063/SCHEMA/CODEMETA-2.0" - xmlns:swh="https://www.softwareheritage.org/schema/2018/deposit"> - <title>Composing a Web of Audio Applications</title> - <id>hal-01243065</id> - <external_identifier>{external_id}</external_identifier> - <author>someone</author> - <swh:deposit> - <swh:create_origin> - <swh:origin url="{url}" /> - </swh:create_origin> - </swh:deposit> -</entry> diff --git a/swh/deposit/tests/data/atom/error-with-external-identifier.xml b/swh/deposit/tests/data/atom/error-with-external-identifier.xml deleted file mode 100644 index 7c9703f5cefec544276bf252ad12c3621171e6fb..0000000000000000000000000000000000000000 --- a/swh/deposit/tests/data/atom/error-with-external-identifier.xml +++ /dev/null @@ -1,7 +0,0 @@ -<?xml version="1.0" encoding="utf-8"?> -<entry xmlns="http://www.w3.org/2005/Atom" xmlns:codemeta="https://doi.org/10.5063/SCHEMA/CODEMETA-2.0"> - <title>Composing a Web of Audio Applications</title> - <id>hal-01243065</id> - <author>someone</author> - <external_identifier>%s</external_identifier> -</entry> diff --git a/swh/deposit/tests/loader/data/https_deposit.softwareheritage.org/1_private_test_999_meta b/swh/deposit/tests/loader/data/https_deposit.softwareheritage.org/1_private_test_999_meta index 01419e3d28dcc92a452ff740cdda113eac0be63c..34a5804ebddf78f5ba507fbb3506feaa83e82b45 100644 --- a/swh/deposit/tests/loader/data/https_deposit.softwareheritage.org/1_private_test_999_meta +++ b/swh/deposit/tests/loader/data/https_deposit.softwareheritage.org/1_private_test_999_meta @@ -13,7 +13,6 @@ "no one" ], "codemeta:dateCreated": "2017-10-07T15:17:08Z", - "external_identifier": "some-external-id", "url": "https://hal-test.archives-ouvertes.fr/some-external-id" }, "provider": { @@ -57,7 +56,6 @@ "another one", "no one"], "codemeta:dateCreated": "2017-10-07T15:17:08Z", - "external_identifier": "some-external-id", "url": "https://hal-test.archives-ouvertes.fr/some-external-id" }, "synthetic": "true",