Skip to content

Commit

Permalink
service: display tombstone for deleted record's files
Browse files Browse the repository at this point in the history
  • Loading branch information
jrcastro2 committed Sep 26, 2023
1 parent 3a90f32 commit b434491
Show file tree
Hide file tree
Showing 11 changed files with 515 additions and 11 deletions.
54 changes: 50 additions & 4 deletions invenio_records_resources/resources/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
# details.

"""Common Errors handling for Resources."""
import json
from json import JSONDecodeError

import marshmallow as ma
from flask import jsonify, make_response, request, url_for
from flask_resources import HTTPJSONException, create_error_handler
from flask import g, jsonify, make_response, request, url_for
from flask_resources import HTTPJSONException as _HTTPJSONException
from flask_resources import create_error_handler
from flask_resources.serializers.json import JSONEncoder
from invenio_i18n import lazy_gettext as _
from invenio_pidstore.errors import (
PIDAlreadyExists,
Expand All @@ -36,11 +39,12 @@
FileKeyNotFoundError,
PermissionDeniedError,
QuerystringValidationError,
RecordDeletedException,
RevisionIdMismatchError,
)


class HTTPJSONValidationException(HTTPJSONException):
class HTTPJSONValidationException(_HTTPJSONException):
"""HTTP exception serializing to JSON and reflecting Marshmallow errors."""

description = "A validation error occurred."
Expand All @@ -50,7 +54,7 @@ def __init__(self, exception):
super().__init__(code=400, errors=validation_error_to_list_errors(exception))


class HTTPJSONSearchRequestError(HTTPJSONException):
class HTTPJSONSearchRequestError(_HTTPJSONException):
"""HTTP exception responsible for mapping search engine errors."""

causes_responses = {
Expand All @@ -70,6 +74,37 @@ def __init__(self, error):
super().__init__(code=500, description=_("Internal server error"))


class HTTPJSONException(_HTTPJSONException):
"""HTTPJSONException that supports setting some extra body fields."""

def __init__(self, code=None, errors=None, **kwargs):
"""Constructor."""
description = kwargs.pop("description", None)
response = kwargs.pop("response", None)
super().__init__(code, errors, description=description, response=response)
self._extra_fields = kwargs

def get_body(self, environ=None, scope=None):
"""Get the response body."""
body = {
"status": self.code,
"message": self.get_description(environ),
**self._extra_fields,
}

errors = self.get_errors()
if errors:
body["errors"] = errors

# TODO: Revisit how to integrate error monitoring services.
# See issue https://github.com/inveniosoftware/flask-resources/issues/56
# Temporarily kept for expediency and backward-compatibility
if self.code and (self.code >= 500) and hasattr(g, "sentry_event_id"):
body["error_id"] = str(g.sentry_event_id)

return json.dumps(body, cls=JSONEncoder)


def create_pid_redirected_error_handler():
"""Creates an error handler for `PIDRedirectedError` error."""

Expand Down Expand Up @@ -186,4 +221,15 @@ class ErrorHandlersMixin:
search.exceptions.RequestError: create_error_handler(
lambda e: HTTPJSONSearchRequestError(e)
),
RecordDeletedException: create_error_handler(
lambda e: (
HTTPJSONException(code=404, description=_("Record not found"))
if not e.record.tombstone.is_visible
else HTTPJSONException(
code=410,
description=_("Record deleted"),
tombstone=e.record.tombstone.dump(),
)
)
),
}
18 changes: 18 additions & 0 deletions invenio_records_resources/resources/files/args.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# -*- coding: utf-8 -*-
#
# Copyright (C) 2023 CERN.
#
# Invenio-Records-Resources is free software; you can redistribute it and/or
# modify it under the terms of the MIT License; see LICENSE file for more
# details.

"""Schemas for parameter parsing."""

from flask_resources.parsers import MultiDictSchema
from marshmallow import Schema, fields


class RequestExtraArgsSchema(MultiDictSchema):
"""Schema for request extra args."""

include_deleted = fields.Bool()
4 changes: 4 additions & 0 deletions invenio_records_resources/resources/files/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

from flask_resources import ResourceConfig

from invenio_records_resources.resources.files.args import RequestExtraArgsSchema


class FileResourceConfig(ResourceConfig):
"""File resource config."""
Expand All @@ -27,3 +29,5 @@ class FileResourceConfig(ResourceConfig):
"item-commit": "/files/<key>/commit",
"list-archive": "/files-archive",
}

request_extra_args = RequestExtraArgsSchema
21 changes: 18 additions & 3 deletions invenio_records_resources/resources/files/resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
from contextlib import ExitStack

import marshmallow as ma
from flask import Response, abort, current_app, g, stream_with_context
from flask import Response, current_app, g, stream_with_context
from flask_resources import (
JSONDeserializer,
RequestBodyParser,
Resource,
from_conf,
request_body_parser,
request_parser,
resource_requestctx,
Expand Down Expand Up @@ -48,6 +49,8 @@
default_content_type="application/octet-stream",
)

request_extra_args = request_parser(from_conf("request_extra_args"), location="args")


#
# Resource
Expand Down Expand Up @@ -84,13 +87,16 @@ def create_url_rules(self):
]
return url_rules

@request_extra_args
@request_view_args
@response_handler(many=True)
def search(self):
"""List files."""
include_deleted = resource_requestctx.args.get("include_deleted", False)
files = self.service.list_files(
g.identity,
resource_requestctx.view_args["pid_value"],
include_deleted=include_deleted,
)
return files.to_dict(), 200

Expand All @@ -116,14 +122,17 @@ def create(self):
)
return item.to_dict(), 201

@request_extra_args
@request_view_args
@response_handler()
def read(self):
"""Read a single file."""
include_deleted = resource_requestctx.args.get("include_deleted", False)
item = self.service.read_file_metadata(
g.identity,
resource_requestctx.view_args["pid_value"],
resource_requestctx.view_args["key"],
include_deleted=include_deleted,
)

return item.to_dict(), 200
Expand Down Expand Up @@ -164,13 +173,16 @@ def create_commit(self):
)
return item.to_dict(), 200

@request_extra_args
@request_view_args
def read_content(self):
"""Read file content."""
include_deleted = resource_requestctx.args.get("include_deleted", False)
item = self.service.get_file_content(
g.identity,
resource_requestctx.view_args["pid_value"],
resource_requestctx.view_args["key"],
include_deleted=include_deleted,
)

# emit file download stats event
Expand All @@ -181,12 +193,15 @@ def read_content(self):

return item.send_file(), 200

@request_extra_args
@request_view_args
def read_archive(self):
"""Read a zipped version of all files."""
id_ = resource_requestctx.view_args["pid_value"]
files = self.service.list_files(g.identity, id_)

include_deleted = resource_requestctx.args.get("include_deleted", False)
files = self.service.list_files(
g.identity, id_, include_deleted=include_deleted
)
# emit file download stats events for each file
emitter = current_stats.get_event_emitter("file-download")
for f in files._results:
Expand Down
9 changes: 9 additions & 0 deletions invenio_records_resources/services/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,12 @@ def __init__(self, recid, file_key):
)
self.recid = recid
self.file_key = file_key


class RecordDeletedException(Exception):
"""Exception denoting that the record was deleted."""

def __init__(self, record, result_item=None):
"""Constructor."""
self.record = record
self.result_item = result_item
26 changes: 22 additions & 4 deletions invenio_records_resources/services/files/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"""File Service API."""

from ..base import LinksTemplate, Service
from ..errors import FileKeyNotFoundError
from ..errors import FileKeyNotFoundError, RecordDeletedException
from ..records.schema import ServiceSchemaWrapper
from ..uow import RecordCommitOp, unit_of_work

Expand Down Expand Up @@ -65,13 +65,27 @@ def _get_record(self, id_, identity, action, file_key=None):

return record

def _ensure_record_not_deleted(self, record, identity, include_deleted):
"""Ensure that the record exists (not deleted) or raise."""
if not record.is_published:
return
if record.deletion_status.is_deleted and not include_deleted:
raise RecordDeletedException(record)
if record.deletion_status.is_deleted and include_deleted:
can_read_deleted = self.check_permission(
identity, "read_deleted_files", record=record
)
if not can_read_deleted:
raise RecordDeletedException(record)

#
# High-level API
#
def list_files(self, identity, id_):
def list_files(self, identity, id_, include_deleted=False):
"""List the files of a record."""
record = self._get_record(id_, identity, "read_files")

self._ensure_record_not_deleted(record, identity, include_deleted)
self.run_components("list_files", id_, identity, record)

return self.file_result_list(
Expand Down Expand Up @@ -119,13 +133,15 @@ def update_file_metadata(self, identity, id_, file_key, data, uow=None):
links_tpl=self.file_links_item_tpl(id_),
)

def read_file_metadata(self, identity, id_, file_key):
def read_file_metadata(self, identity, id_, file_key, include_deleted=False):
"""Read the metadata of a file.
:raises FileKeyNotFoundError: If the record has no file for the ``file_key``
"""
record = self._get_record(id_, identity, "read_files", file_key=file_key)

self._ensure_record_not_deleted(record, identity, include_deleted)

self.run_components("read_file_metadata", identity, id_, file_key, record)

return self.file_result_item(
Expand Down Expand Up @@ -259,13 +275,15 @@ def set_file_content(
links_tpl=self.file_links_item_tpl(id_),
)

def get_file_content(self, identity, id_, file_key):
def get_file_content(self, identity, id_, file_key, include_deleted=False):
"""Retrieve file content.
:raises FileKeyNotFoundError: If the record has no file for the ``file_key``
"""
record = self._get_record(id_, identity, "get_content_files", file_key=file_key)

self._ensure_record_not_deleted(record, identity, include_deleted)

self.run_components("get_file_content", identity, id_, file_key, record)

return self.file_result_item(
Expand Down
47 changes: 47 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

import pytest
from flask_principal import Identity, Need, UserNeed
from invenio_access import ActionRoles, superuser_access
from invenio_accounts.models import Role
from invenio_app.factory import create_api as _create_api
from mock_module.config import MockFileServiceConfig, ServiceConfig

Expand Down Expand Up @@ -100,3 +102,48 @@ def identity_simple():
i.provides.add(UserNeed(1))
i.provides.add(Need(method="system_role", value="any_user"))
return i


@pytest.fixture()
def superuser_role_need(db):
"""Store 1 role with 'superuser-access' ActionNeed.
WHY: This is needed because expansion of ActionNeed is
done on the basis of a User/Role being associated with that Need.
If no User/Role is associated with that Need (in the DB), the
permission is expanded to an empty list.
"""
role = Role(name="superuser-access")
db.session.add(role)

action_role = ActionRoles.create(action=superuser_access, role=role)
db.session.add(action_role)

db.session.commit()

return action_role.need


@pytest.fixture()
def superuser(UserFixture, app, db, superuser_role_need):
"""Superuser."""
u = UserFixture(
email="[email protected]",
password="superuser",
)
u.create(app, db)

datastore = app.extensions["security"].datastore
_, role = datastore._prepare_role_modify_args(u.user, "superuser-access")

datastore.add_role_to_user(u.user, role)
db.session.commit()
return u


@pytest.fixture()
def superuser_identity(superuser_role_need):
"""Superuser identity fixture."""
identity = Identity(1)
identity.provides.add(superuser_role_need)
return identity
Loading

0 comments on commit b434491

Please sign in to comment.