Differential D4013 Diff 14314 swh/deposit/tests/api/test_deposit_update.py

Changeset View

Standalone View

swh/deposit/tests/api/test_deposit_update.py

# 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		# See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version		# License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information		# See top-level LICENSE file for more information

from io import BytesIO		from io import BytesIO

from django.core.files.uploadedfile import InMemoryUploadedFile		from django.core.files.uploadedfile import InMemoryUploadedFile
from django.urls import reverse		from django.urls import reverse
from rest_framework import status		from rest_framework import status

from swh.deposit.config import (		from swh.deposit.config import (
DEPOSIT_STATUS_DEPOSITED,		DEPOSIT_STATUS_DEPOSITED,
DEPOSIT_STATUS_PARTIAL,		DEPOSIT_STATUS_PARTIAL,
EDIT_SE_IRI,		EDIT_SE_IRI,
EM_IRI,		EM_IRI,
)		)
from swh.deposit.models import Deposit, DepositCollection, DepositRequest		from swh.deposit.models import Deposit, DepositCollection, DepositRequest
from swh.deposit.parsers import parse_xml		from swh.deposit.parsers import parse_xml
from swh.deposit.tests.common import check_archive, create_arborescence_archive		from swh.deposit.tests.common import check_archive, create_arborescence_archive
		from swh.model.hashutil import hash_to_bytes
		from swh.model.identifiers import parse_swhid, swhid


def test_replace_archive_to_deposit_is_possible(		def test_replace_archive_to_deposit_is_possible(
tmp_path,		tmp_path,
partial_deposit,		partial_deposit,
deposit_collection,		deposit_collection,
authenticated_client,		authenticated_client,
sample_archive,		sample_archive,
▲ Show 20 Lines • Show All 97 Lines • ▼ Show 20 Lines	):
assert raw_metadata0 != raw_metadata1		assert raw_metadata0 != raw_metadata1
assert request_meta0 != request_meta1		assert request_meta0 != request_meta1

# check we did not touch the other parts		# check we did not touch the other parts
requests_archive1 = DepositRequest.objects.filter(deposit=deposit, type="archive")		requests_archive1 = DepositRequest.objects.filter(deposit=deposit, type="archive")
assert len(requests_archive1) == 1		assert len(requests_archive1) == 1
assert set(requests_archive0) == set(requests_archive1)		assert set(requests_archive0) == set(requests_archive1)


		moraneggUnsubmitted Done Inline Actions modify `1` into `nominal`. Best to have a meaningful test name. moranegg: modify `1` into `nominal`. Best to have a meaningful test name.
		def test_put_update_metadata_done_deposit_nominal(
		tmp_path,
		authenticated_client,
		complete_deposit,
		deposit_collection,
		atom_dataset,
		sample_data,
		swh_storage,
		):
		"""Nominal scenario, client send an update of metadata on a deposit with status "done"
		with an existing swhid. Such swhid has its metadata updated accordingly both in
		vlorentzUnsubmitted Not Done Inline Actions it's not updated, it's added in both vlorentz: it's not updated, it's added in both
		ardumontAuthorUnsubmitted Done Inline Actions yes, i used the wrong endpoint, should be in the other iri that allows adding new metadata to be added. Will change that on monday. Thanks! ardumont: yes, i used the wrong endpoint, should be in the other iri that allows adding new metadata to…
		the deposit backend and in the metadata storage.
		moraneggUnsubmitted Not Done Inline Actions complete deposit is an already done deposit which deposited content, for which we have a deposit_id and a swhid, right? might be useful to be more specific in the function's description. moranegg: complete deposit is an already done deposit which deposited content, for which we have a…
		ardumontAuthorUnsubmitted Done Inline Actions yes, it is. It's a pytest fixture with already predefined data (to avoid cluttering the tests with plenty of initialization step like it used to do in the past...) ok for improving the docstrings. ardumont: yes, it is. It's a pytest fixture with already predefined data (to avoid cluttering the tests…

		Response: 204
		moraneggUnsubmitted Done Inline Actions what's that? is that the hash in the deposit table or the hash given by the deposit request? moranegg: what's that? is that the hash in the deposit table or the hash given by the deposit request?
		vlorentzUnsubmitted Done Inline Actions from the deposit table, the request is only created later (line 163) vlorentz: from the deposit table, the request is only created later (line 163)
		ardumontAuthorUnsubmitted Done Inline Actions it's the "done" deposit's swhid we parsed into a directory id. I'm using that to demonstrate that it will be present in the storage prior to trigger the actual update scenario (nominal case). ardumont: it's the "done" deposit's swhid we parsed into a directory id. I'm using that to demonstrate…
		moraneggUnsubmitted Not Done Inline Actions I would rename that to something like `existing_directory_id` or commenting that it's the parsed swhid from the deposit table moranegg: I would rename that to something like `existing_directory_id` or commenting that it's the…

		"""
		deposit = complete_deposit

		vlorentzUnsubmitted Not Done Inline Actions you should check `deposit.swh_id` is a dir swhid, just to be sure there's no inconsistency vlorentz: you should check `deposit.swh_id` is a dir swhid, just to be sure there's no inconsistency
		directory_id = hash_to_bytes(parse_swhid(deposit.swh_id).object_id)
		# directory targeted by the complete_deposit does not exist in the storage
		assert list(swh_storage.directory_missing([directory_id])) == [directory_id]

		# so let's create a directory reference in the storage (current deposit targets an
		# unknown swhid)
		directory = sample_data.directory
		swh_storage.directory_add([directory])
		vlorentzUnsubmitted Not Done Inline Actions Remove this (see comment above) vlorentz: Remove this (see comment above)
		assert list(swh_storage.directory_missing([directory.id])) == []
		# and patch one complete deposit swh_id so it targets said reference
		deposit.swh_id = swhid("directory", directory.id)
		deposit.save()

		raw_metadata0 = atom_dataset["entry-data0"] % deposit.external_id.encode("utf-8")

		requests_archive0 = DepositRequest.objects.filter(deposit=deposit, type="archive")
		vlorentzUnsubmitted Not Done Inline Actions move this at the top of the test (and also check there's a deposit request with the metadata) vlorentz: move this at the top of the test (and also check there's a deposit request with the metadata)
		assert len(requests_archive0) == 1

		update_uri = reverse(EDIT_SE_IRI, args=[deposit_collection.name, deposit.id])

		response = authenticated_client.put(
		update_uri,
		content_type="application/atom+xml;type=entry",
		data=atom_dataset["entry-data1"],
		HTTP_X_CHECK_SWHID=deposit.swh_id,
		)

		assert response.status_code == status.HTTP_204_NO_CONTENT

		requests_meta = DepositRequest.objects.filter(deposit=deposit, type="metadata")

		vlorentzUnsubmitted Not Done Inline Actions 2 (see comment above) vlorentz: 2 (see comment above)
		assert len(requests_meta) == 1
		request_meta1 = requests_meta[0]
		raw_metadata1 = request_meta1.raw_metadata
		assert raw_metadata1 == atom_dataset["entry-data1"]
		assert raw_metadata0 != raw_metadata1

		# check we did not touch the other parts
		requests_archive1 = DepositRequest.objects.filter(deposit=deposit, type="archive")
		assert len(requests_archive1) == 1
		assert set(requests_archive0) == set(requests_archive1)

		# FIXME: Check the metadata storage information created is consistent
		pass


		moraneggUnsubmitted Done Inline Actions change `done_deposit_2` into `done_deposit_swhid_in_archive_failure` moranegg: change `done_deposit_2` into `done_deposit_swhid_in_archive_failure`
		def test_put_update_metadata_done_deposit_failure_swhid_unknown(
		tmp_path,
		authenticated_client,
		complete_deposit,
		deposit_collection,
		atom_dataset,
		swh_storage,
		):
		"""Failure: client updates metadata with a SWHID matching the deposit's. Said SWHID does
		not exist in the archive somehow.

		This should not happen though, it is still technically possible so it's
		covered...

		Response: 400

		"""
		# directory targeted by the complete_deposit does not exist in the storage
		missing_directory_id = hash_to_bytes(parse_swhid(complete_deposit.swh_id).object_id)
		assert list(swh_storage.directory_missing([missing_directory_id])) == [
		missing_directory_id
		]

		update_uri = reverse(
		EDIT_SE_IRI, args=[deposit_collection.name, complete_deposit.id]
		)

		response = authenticated_client.put(
		update_uri,
		content_type="application/atom+xml;type=entry",
		data=atom_dataset["entry-data1"],
		HTTP_X_CHECK_SWHID=complete_deposit.swh_id,
		)

		assert response.status_code == status.HTTP_400_BAD_REQUEST
		assert b"Unknown directory SWHID" in response.content


		def test_put_update_metadata_done_deposit_failure_mismatched_swhid(
		tmp_path,
		moraneggUnsubmitted Done Inline Actions `done_deposit_3` change into `done_deposit_swhid_match_failure` moranegg: `done_deposit_3` change into `done_deposit_swhid_match_failure`
		ardumontAuthorUnsubmitted Done Inline Actions i'd like to stop naming the test which tries to repeat poorly the docstring... ardumont: i'd like to stop naming the test which tries to repeat poorly the docstring...
		authenticated_client,
		complete_deposit,
		deposit_collection,
		atom_dataset,
		swh_storage,
		):
		"""failure: client updates metadata on deposit with SWHID not matching the deposit's.

		Response: 400

		"""
		incorrect_swhid = "swh:1:dir:ef04a768181417fbc5eef4243e2507915f24deea"
		assert complete_deposit.swh_id != incorrect_swhid

		update_uri = reverse(
		EDIT_SE_IRI, args=[deposit_collection.name, complete_deposit.id]
		)
		response = authenticated_client.put(
		update_uri,
		content_type="application/atom+xml;type=entry",
		data=atom_dataset["entry-data1"],
		HTTP_X_CHECK_SWHID=incorrect_swhid,
		)

		assert response.status_code == status.HTTP_400_BAD_REQUEST
		assert b"Mismatched provided SWHID" in response.content


		def test_put_update_metadata_done_deposit_failure_malformed_xml(
		tmp_path,
		authenticated_client,
		complete_deposit,
		deposit_collection,
		atom_dataset,
		swh_storage,
		):
		"""failure: client updates metadata on deposit done with a malformed xml

		Response: 400

		"""
		update_uri = reverse(
		EDIT_SE_IRI, args=[deposit_collection.name, complete_deposit.id]
		)
		response = authenticated_client.put(
		update_uri,
		content_type="application/atom+xml;type=entry",
		data=atom_dataset["entry-data-ko"],
		HTTP_X_CHECK_SWHID=complete_deposit.swh_id,
		)

		assert response.status_code == status.HTTP_400_BAD_REQUEST
		assert b"Malformed xml metadata" in response.content


		def test_put_update_metadata_done_deposit_failure_empty_xml(
		tmp_path,
		authenticated_client,
		complete_deposit,
		deposit_collection,
		atom_dataset,
		swh_storage,
		):
		"""failure: client updates metadata on deposit done with an empty xml.

		Response: 400

		"""
		update_uri = reverse(
		EDIT_SE_IRI, args=[deposit_collection.name, complete_deposit.id]
		)

		response = authenticated_client.put(
		update_uri,
		content_type="application/atom+xml;type=entry",
		data=atom_dataset["entry-data-empty-body"],
		HTTP_X_CHECK_SWHID=complete_deposit.swh_id,
		)

		assert response.status_code == status.HTTP_400_BAD_REQUEST
		ardumontAuthorUnsubmitted Done Inline Actions @moranegg empty body use case i kept talking about in D4047 ;) ardumont: @moranegg empty body use case i kept talking about in D4047 ;)
		assert b"Empty body request is not supported" in response.content


def test_add_archive_to_deposit_is_possible(		def test_add_archive_to_deposit_is_possible(
tmp_path,		tmp_path,
authenticated_client,		authenticated_client,
deposit_collection,		deposit_collection,
partial_deposit_with_metadata,		partial_deposit_with_metadata,
sample_archive,		sample_archive,
):		):
"""Add another archive to a deposit return a 201 response		"""Add another archive to a deposit return a 201 response
▲ Show 20 Lines • Show All 421 Lines • Show Last 20 Lines