D4505.id15986.diff
No OneTemporary
Actions

Size

22 KB

Subscribers

None

D4505.id15986.diff
View Options

	diff --git a/swh/deposit/api/common.py b/swh/deposit/api/common.py
	--- a/swh/deposit/api/common.py
	+++ b/swh/deposit/api/common.py
	@@ -63,10 +63,7 @@
	PARSING_ERROR,
	DepositError,
	ParserError,
	- make_error_dict,
	- make_error_response,
	- make_error_response_from_dict,
	- make_missing_slug_error,
	+ raise_missing_slug_error,
	)
	from ..models import DepositClient, DepositCollection, DepositRequest
	from ..parsers import parse_swh_reference, parse_xml
	@@ -290,9 +287,7 @@
	try:
	deposit = Deposit.objects.get(pk=deposit_id)
	except Deposit.DoesNotExist:
	- return make_error_dict(
	- NOT_FOUND, f"The deposit {deposit_id} does not exist"
	- )
	+ raise DepositError(NOT_FOUND, f"The deposit {deposit_id} does not exist")
	DepositRequest.objects.filter(deposit=deposit, type=ARCHIVE_TYPE).delete()

	return {}
	@@ -312,9 +307,7 @@
	try:
	deposit = Deposit.objects.get(pk=deposit_id)
	except Deposit.DoesNotExist:
	- return make_error_dict(
	- NOT_FOUND, f"The deposit {deposit_id} does not exist"
	- )
	+ raise DepositError(NOT_FOUND, f"The deposit {deposit_id} does not exist")

	if deposit.collection.name != collection_name:
	summary = "Cannot delete a deposit from another collection"
	@@ -322,7 +315,7 @@
	deposit_id,
	collection_name,
	)
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST, summary=summary, verbose_description=description
	)

	@@ -350,7 +343,7 @@
	max_upload_size = self.config["max_upload_size"]
	if content_length:
	if content_length > max_upload_size:
	- return make_error_dict(
	+ raise DepositError(
	MAX_UPLOAD_SIZE_EXCEEDED,
	f"Upload size limit exceeded (max {max_upload_size} bytes)."
	"Please consider sending the archive in multiple steps.",
	@@ -358,14 +351,12 @@

	length = filehandler.size
	if length != content_length:
	- return make_error_dict(
	- status.HTTP_412_PRECONDITION_FAILED, "Wrong length"
	- )
	+ raise DepositError(status.HTTP_412_PRECONDITION_FAILED, "Wrong length")

	if md5sum:
	_md5sum = _compute_md5(filehandler)
	if _md5sum != md5sum:
	- return make_error_dict(
	+ raise DepositError(
	CHECKSUM_MISMATCH,
	"Wrong md5 hash",
	f"The checksum sent {hashutil.hash_to_hex(md5sum)} and the actual "
	@@ -424,7 +415,7 @@
	"""
	content_length = headers.content_length
	if not content_length:
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST,
	"CONTENT_LENGTH header is mandatory",
	"For archive deposit, the CONTENT_LENGTH header must be sent.",
	@@ -432,7 +423,7 @@

	content_disposition = headers.content_disposition
	if not content_disposition:
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST,
	"CONTENT_DISPOSITION header is mandatory",
	"For archive deposit, the CONTENT_DISPOSITION header must be sent.",
	@@ -440,7 +431,7 @@

	packaging = headers.packaging
	if packaging and packaging not in ACCEPT_PACKAGINGS:
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST,
	f"Only packaging {ACCEPT_PACKAGINGS} is supported",
	f"The packaging provided {packaging} is not supported",
	@@ -457,7 +448,7 @@

	slug = headers.slug
	if check_slug_is_present and not slug:
	- return make_missing_slug_error()
	+ raise_missing_slug_error()

	# actual storage of data
	archive_metadata = filehandler
	@@ -542,7 +533,7 @@
	"""
	slug = headers.slug
	if check_slug_is_present and not slug:
	- return make_missing_slug_error()
	+ raise_missing_slug_error()

	content_types_present = set()

	@@ -555,7 +546,7 @@
	fh = value
	content_type = fh.content_type
	if content_type in content_types_present:
	- return make_error_dict(
	+ raise DepositError(
	ERROR_CONTENT,
	"Only 1 application/zip (or application/x-tar) archive "
	"and 1 atom+xml entry is supported (as per sword2.0 "
	@@ -570,7 +561,7 @@
	data[content_type] = fh

	if len(content_types_present) != 2:
	- return make_error_dict(
	+ raise DepositError(
	ERROR_CONTENT,
	"You must provide both 1 application/zip (or "
	"application/x-tar) and 1 atom+xml entry for multipart "
	@@ -594,7 +585,7 @@
	try:
	raw_metadata, metadata = self._read_metadata(data["application/atom+xml"])
	except ParserError:
	- return make_error_dict(
	+ raise DepositError(
	PARSING_ERROR,
	"Malformed xml metadata",
	"The xml received is malformed. "
	@@ -765,7 +756,7 @@
	try:
	raw_metadata, metadata = self._read_metadata(request.data)
	except ParserError:
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST,
	"Malformed xml metadata",
	"The xml received is malformed. "
	@@ -773,7 +764,7 @@
	)

	if not metadata:
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST,
	"Empty body request is not supported",
	"Atom entry deposit is supposed to send for metadata. "
	@@ -784,14 +775,16 @@
	try:
	swhid = parse_swh_reference(metadata)
	except ValidationError as e:
	- return make_error_dict(PARSING_ERROR, "Invalid SWHID reference", str(e),)
	+ raise DepositError(
	+ PARSING_ERROR, "Invalid SWHID reference", str(e),
	+ )

	if swhid is not None:
	external_id = metadata.get("external_identifier", headers.slug)
	else:
	slug = headers.slug
	if check_slug_is_present and not slug:
	- return make_missing_slug_error()
	+ raise_missing_slug_error()

	external_id = metadata.get("external_identifier", slug)

	@@ -803,12 +796,9 @@
	)

	if swhid is not None:
	- try:
	- swhid, swhid_ref, depo, depo_request = self._store_metadata_deposit(
	- deposit, swhid, metadata, raw_metadata
	- )
	- except DepositError as deposit_error:
	- return deposit_error.to_dict()
	+ swhid, swhid_ref, depo, depo_request = self._store_metadata_deposit(
	+ deposit, swhid, metadata, raw_metadata
	+ )

	deposit.status = DEPOSIT_STATUS_LOAD_SUCCESS
	if isinstance(swhid_ref, SWHID):
	@@ -893,9 +883,7 @@
	try:
	self._collection = DepositCollection.objects.get(name=collection_name)
	except DepositCollection.DoesNotExist:
	- return make_error_dict(
	- NOT_FOUND, f"Unknown collection name {collection_name}"
	- )
	+ raise DepositError(NOT_FOUND, f"Unknown collection name {collection_name}")
	assert self._collection is not None

	username = request.user.username
	@@ -905,13 +893,13 @@
	username=username
	)
	except DepositClient.DoesNotExist:
	- return make_error_dict(NOT_FOUND, f"Unknown client name {username}")
	+ raise DepositError(NOT_FOUND, f"Unknown client name {username}")

	collection_id = self._collection.id
	collections = self._client.collections
	assert collections is not None
	if collection_id not in collections:
	- return make_error_dict(
	+ raise DepositError(
	FORBIDDEN,
	f"Client {username} cannot access collection {collection_name}",
	)
	@@ -922,27 +910,23 @@
	try:
	deposit = Deposit.objects.get(pk=deposit_id)
	except Deposit.DoesNotExist:
	- return make_error_dict(
	+ raise DepositError(
	NOT_FOUND, f"Deposit with id {deposit_id} does not exist"
	)

	assert deposit is not None
	- checks = self.restrict_access(request, headers, deposit)
	- if checks:
	- return checks
	+ self.restrict_access(request, headers, deposit)

	if headers.on_behalf_of:
	- return make_error_dict(MEDIATION_NOT_ALLOWED, "Mediation is not supported.")
	+ raise DepositError(MEDIATION_NOT_ALLOWED, "Mediation is not supported.")

	- checks = self.additional_checks(request, headers, collection_name, deposit_id)
	- if "error" in checks:
	- return checks
	+ self.additional_checks(request, headers, collection_name, deposit_id)

	return {"headers": headers}

	def restrict_access(
	self, request: Request, headers: ParsedRequestHeaders, deposit: Deposit
	- ) -> Dict[str, Any]:
	+ ) -> None:
	"""Allow modifications on deposit with status 'partial' only, reject the rest.

	"""
	@@ -951,14 +935,12 @@
	DEPOSIT_STATUS_PARTIAL,
	)
	description = f"This deposit has status '{deposit.status}'"
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST, summary=summary, verbose_description=description
	)
	- return {}

	def _basic_not_allowed_method(self, request: Request, method: str):
	- return make_error_response(
	- request,
	+ raise DepositError(
	METHOD_NOT_ALLOWED,
	f"{method} method is not supported on this endpoint",
	)
	@@ -1000,9 +982,7 @@
	404 if the deposit or the collection does not exist

	"""
	- checks = self.checks(request, collection_name, deposit_id)
	- if "error" in checks:
	- return make_error_response_from_dict(request, checks["error"])
	+ self.checks(request, collection_name, deposit_id)

	r = self.process_get(request, collection_name, deposit_id)

	@@ -1048,18 +1028,12 @@

	"""
	checks = self.checks(request, collection_name, deposit_id)
	- if "error" in checks:
	- return make_error_response_from_dict(request, checks["error"])

	headers = checks["headers"]
	status, iri_key, data = self.process_post(
	request, headers, collection_name, deposit_id
	)

	- error = data.get("error")
	- if error:
	- return make_error_response_from_dict(request, error)
	-
	return self._make_deposit_receipt(
	request, collection_name, status, iri_key, data,
	)
	@@ -1131,15 +1105,8 @@

	"""
	checks = self.checks(request, collection_name, deposit_id)
	- if "error" in checks:
	- return make_error_response_from_dict(request, checks["error"])
	-
	headers = checks["headers"]
	- data = self.process_put(request, headers, collection_name, deposit_id)
	-
	- error = data.get("error")
	- if error:
	- return make_error_response_from_dict(request, error)
	+ self.process_put(request, headers, collection_name, deposit_id)

	return HttpResponse(status=status.HTTP_204_NO_CONTENT)

	@@ -1176,15 +1143,9 @@
	404 if the deposit or the collection does not exist

	"""
	- checks = self.checks(request, collection_name, deposit_id)
	- if "error" in checks:
	- return make_error_response_from_dict(request, checks["error"])
	-
	+ self.checks(request, collection_name, deposit_id)
	assert deposit_id is not None
	- data = self.process_delete(request, collection_name, deposit_id)
	- error = data.get("error")
	- if error:
	- return make_error_response_from_dict(request, error)
	+ self.process_delete(request, collection_name, deposit_id)

	return HttpResponse(status=status.HTTP_204_NO_CONTENT)

	diff --git a/swh/deposit/api/content.py b/swh/deposit/api/content.py
	--- a/swh/deposit/api/content.py
	+++ b/swh/deposit/api/content.py
	@@ -7,7 +7,7 @@
	from django.shortcuts import render
	from rest_framework import status

	-from ..errors import NOT_FOUND, make_error_response, make_error_response_from_dict
	+from ..errors import NOT_FOUND, make_error_response
	from ..models import DEPOSIT_STATUS_DETAIL, Deposit, DepositRequest
	from .common import APIBase

	@@ -22,9 +22,7 @@
	"""

	def get(self, req, collection_name: str, deposit_id: int) -> HttpResponse:
	- checks = self.checks(req, collection_name, deposit_id)
	- if "error" in checks:
	- return make_error_response_from_dict(req, checks["error"])
	+ self.checks(req, collection_name, deposit_id)

	try:
	deposit = Deposit.objects.get(pk=deposit_id)
	diff --git a/swh/deposit/api/edit.py b/swh/deposit/api/edit.py
	--- a/swh/deposit/api/edit.py
	+++ b/swh/deposit/api/edit.py
	@@ -11,7 +11,7 @@
	from swh.model.identifiers import parse_swhid

	from ..config import DEPOSIT_STATUS_LOAD_SUCCESS
	-from ..errors import BAD_REQUEST, DepositError, ParserError, make_error_dict
	+from ..errors import BAD_REQUEST, DepositError, ParserError
	from ..parsers import SWHAtomEntryParser, SWHMultiPartParser
	from .common import APIDelete, APIPut, ParsedRequestHeaders

	@@ -29,7 +29,7 @@

	def restrict_access(
	self, request: Request, headers: ParsedRequestHeaders, deposit: Deposit
	- ) -> Dict[str, Any]:
	+ ) -> None:
	"""Relax restriction access to allow metadata update on deposit with status "done" when
	a swhid is provided.

	@@ -40,9 +40,9 @@
	and deposit.status == DEPOSIT_STATUS_LOAD_SUCCESS
	):
	# Allow metadata update on deposit with status "done" when swhid provided
	- return {}
	+ return
	# otherwise, let the standard access restriction check occur
	- return super().restrict_access(request, headers, deposit)
	+ super().restrict_access(request, headers, deposit)

	def process_put(
	self,
	@@ -104,7 +104,7 @@
	assert deposit.status == DEPOSIT_STATUS_LOAD_SUCCESS

	if swhid != deposit.swhid:
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST,
	f"Mismatched provided SWHID {swhid} with deposit's {deposit.swhid}.",
	"The provided SWHID does not match the deposit to update. "
	@@ -114,7 +114,7 @@
	try:
	raw_metadata, metadata = self._read_metadata(request.data)
	except ParserError:
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST,
	"Malformed xml metadata",
	"The xml received is malformed. "
	@@ -122,19 +122,16 @@
	)

	if not metadata:
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST,
	"Empty body request is not supported",
	"Atom entry deposit is supposed to send for metadata. "
	"If the body is empty, there is no metadata.",
	)

	- try:
	- _, _, deposit, deposit_request = self._store_metadata_deposit(
	- deposit, parse_swhid(swhid), metadata, raw_metadata, deposit.origin_url,
	- )
	- except DepositError as deposit_error:
	- return deposit_error.to_dict()
	+ _, _, deposit, deposit_request = self._store_metadata_deposit(
	+ deposit, parse_swhid(swhid), metadata, raw_metadata, deposit.origin_url,
	+ )

	return {
	"deposit_id": deposit.id,
	diff --git a/swh/deposit/api/edit_media.py b/swh/deposit/api/edit_media.py
	--- a/swh/deposit/api/edit_media.py
	+++ b/swh/deposit/api/edit_media.py
	@@ -8,7 +8,7 @@
	from rest_framework import status

	from ..config import CONT_FILE_IRI
	-from ..errors import BAD_REQUEST, make_error_dict
	+from ..errors import BAD_REQUEST, DepositError
	from ..parsers import SWHFileUploadTarParser, SWHFileUploadZipParser
	from .common import (
	ACCEPT_ARCHIVE_CONTENT_TYPES,
	@@ -48,7 +48,7 @@
	msg = "Packaging format supported is restricted to %s" % (
	", ".join(ACCEPT_ARCHIVE_CONTENT_TYPES)
	)
	- return make_error_dict(BAD_REQUEST, msg)
	+ raise DepositError(BAD_REQUEST, msg)

	return self._binary_upload(
	req, headers, collection_name, deposit_id=deposit_id, replace_archives=True
	@@ -76,8 +76,7 @@
	msg = "Packaging format supported is restricted to %s" % (
	", ".join(ACCEPT_ARCHIVE_CONTENT_TYPES)
	)
	- unused = 0
	- return unused, "unused", make_error_dict(BAD_REQUEST, msg)
	+ raise DepositError(BAD_REQUEST, msg)

	return (
	status.HTTP_201_CREATED,
	diff --git a/swh/deposit/api/private/__init__.py b/swh/deposit/api/private/__init__.py
	--- a/swh/deposit/api/private/__init__.py
	+++ b/swh/deposit/api/private/__init__.py
	@@ -9,7 +9,7 @@

	from swh.deposit import utils
	from swh.deposit.api.common import AuthenticatedAPIView
	-from swh.deposit.errors import NOT_FOUND, make_error_dict
	+from swh.deposit.errors import NOT_FOUND, DepositError

	from ...config import METADATA_TYPE, APIConfig
	from ...models import Deposit, DepositRequest
	@@ -81,14 +81,12 @@
	try:
	Deposit.objects.get(pk=deposit_id)
	except Deposit.DoesNotExist:
	- return make_error_dict(
	+ raise DepositError(
	NOT_FOUND, "Deposit with id %s does not exist" % deposit_id
	)

	headers = self._read_headers(req)
	- checks = self.additional_checks(req, headers, collection_name, deposit_id)
	- if "error" in checks:
	- return checks
	+ self.additional_checks(req, headers, collection_name, deposit_id)

	return {"headers": headers}

	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
	@@ -10,7 +10,7 @@
	from swh.model.identifiers import DIRECTORY, REVISION, SNAPSHOT, swhid

	from . import APIPrivateView
	-from ...errors import BAD_REQUEST, make_error_dict
	+from ...errors import BAD_REQUEST, DepositError
	from ...models import DEPOSIT_STATUS_DETAIL, DEPOSIT_STATUS_LOAD_SUCCESS, Deposit
	from ..common import APIPut, ParsedRequestHeaders

	@@ -43,11 +43,11 @@
	msg = "The status key is mandatory with possible values %s" % list(
	DEPOSIT_STATUS_DETAIL.keys()
	)
	- return make_error_dict(BAD_REQUEST, msg)
	+ raise DepositError(BAD_REQUEST, msg)

	if status not in DEPOSIT_STATUS_DETAIL:
	msg = "Possible status in %s" % list(DEPOSIT_STATUS_DETAIL.keys())
	- return make_error_dict(BAD_REQUEST, msg)
	+ raise DepositError(BAD_REQUEST, msg)

	if status == DEPOSIT_STATUS_LOAD_SUCCESS:
	missing_keys = []
	@@ -61,7 +61,7 @@
	f"Updating deposit status to {status}"
	f" requires information {','.join(missing_keys)}"
	)
	- return make_error_dict(BAD_REQUEST, msg)
	+ raise DepositError(BAD_REQUEST, msg)

	return {}

	diff --git a/swh/deposit/api/state.py b/swh/deposit/api/state.py
	--- a/swh/deposit/api/state.py
	+++ b/swh/deposit/api/state.py
	@@ -7,7 +7,7 @@
	from django.shortcuts import render
	from rest_framework import status

	-from ..errors import NOT_FOUND, make_error_response, make_error_response_from_dict
	+from ..errors import NOT_FOUND, make_error_response
	from ..models import DEPOSIT_STATUS_DETAIL, Deposit
	from .common import APIBase
	from .converters import convert_status_detail
	@@ -23,9 +23,7 @@
	"""

	def get(self, req, collection_name: str, deposit_id: int) -> HttpResponse:
	- checks = self.checks(req, collection_name, deposit_id)
	- if "error" in checks:
	- return make_error_response_from_dict(req, checks["error"])
	+ self.checks(req, collection_name, deposit_id)

	try:
	deposit = Deposit.objects.get(pk=deposit_id)
	diff --git a/swh/deposit/errors.py b/swh/deposit/errors.py
	--- a/swh/deposit/errors.py
	+++ b/swh/deposit/errors.py
	@@ -7,7 +7,7 @@

	"""

	-from typing import Any, Dict
	+from typing import NoReturn

	from django.shortcuts import render
	from rest_framework import status
	@@ -152,11 +152,11 @@
	return make_error_response_from_dict(req, error["error"])


	-def make_missing_slug_error() -> Dict[str, Any]:
	+def raise_missing_slug_error() -> NoReturn:
	"""Returns a missing slug header error dict

	"""
	- return make_error_dict(
	+ raise DepositError(
	BAD_REQUEST,
	"Missing SLUG header",
	verbose_description=(
	@@ -171,10 +171,31 @@

	"""

	- def __init__(self, key, summary, verbose_description):
	+ def __init__(self, key, summary, verbose_description=None):
	self.key = key
	self.summary = summary
	self.verbose_description = verbose_description

	def to_dict(self):
	return make_error_dict(self.key, self.summary, self.verbose_description)
	+
	+
	+class DepositErrorMiddleware:
	+ """A Django middleware that catches DepositError and returns a proper
	+ error response."""
	+
	+ # __init__ and __call__ are boilerplate to make a pass-through Django
	+ # middleware
	+
	+ def __init__(self, get_response):
	+ self.get_response = get_response
	+
	+ def __call__(self, request):
	+ response = self.get_response(request)
	+ return response
	+
	+ def process_exception(self, request, exception):
	+ if isinstance(exception, DepositError):
	+ return make_error_response_from_dict(request, exception.to_dict()["error"])
	+ else:
	+ return None
	diff --git a/swh/deposit/settings/common.py b/swh/deposit/settings/common.py
	--- a/swh/deposit/settings/common.py
	+++ b/swh/deposit/settings/common.py
	@@ -48,6 +48,7 @@
	"django.contrib.messages.middleware.MessageMiddleware",
	"django.middleware.clickjacking.XFrameOptionsMiddleware",
	"swh.deposit.auth.WrapBasicAuthenticationResponseMiddleware",
	+ "swh.deposit.errors.DepositErrorMiddleware",
	]

	ROOT_URLCONF = "swh.deposit.urls"

File Metadata

Mime Type: text/plain
Expires: Tue, Dec 17, 10:03 AM (3 d, 7 h ago)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 3226268

D4505.id15986.diffNo OneTemporaryActions

D4505.id15986.diffView Options

File Metadata

Event Timeline

D4505.id15986.diff
No OneTemporary
Actions

D4505.id15986.diff
View Options