From 88c0ab90aae97eece9fa27c7bb80f1e2d090f7c7 Mon Sep 17 00:00:00 2001 From: Mirek Simek Date: Tue, 23 Jul 2024 15:27:17 +0200 Subject: [PATCH] Transfer abstraction + multipart upload abstraction --- invenio_records_resources/ext.py | 20 ++ invenio_records_resources/proxies.py | 4 + invenio_records_resources/records/resolver.py | 1 - .../records/systemfields/entity_reference.py | 1 - .../records/systemfields/files/manager.py | 19 +- .../resources/files/config.py | 1 + .../resources/files/resource.py | 43 +++- .../services/custom_fields/base.py | 2 - .../services/files/components/__init__.py | 2 + .../services/files/components/base.py | 11 +- .../services/files/components/content.py | 11 +- .../services/files/components/metadata.py | 113 +++++++++- .../services/files/config.py | 2 + .../services/files/generators.py | 57 ++--- .../services/files/results.py | 33 ++- .../services/files/schema.py | 104 ++++------ .../services/files/service.py | 67 ++++++ .../services/files/tasks.py | 15 +- .../services/files/transfer.py | 194 ------------------ .../services/records/facets/facets.py | 1 - .../services/records/params/querystr.py | 2 +- .../services/references/schema.py | 2 +- setup.cfg | 2 +- tests/factories/conftest.py | 1 - tests/factories/test_factory.py | 2 +- tests/mock_module/api.py | 3 + tests/mock_module/permissions.py | 42 +++- tests/records/test_systemfield_index.py | 2 - tests/records/test_systemfield_modelpid.py | 1 - tests/records/test_systemfield_pid.py | 1 - tests/resources/test_files_resource.py | 82 +++++++- tests/services/files/conftest.py | 2 +- tests/services/files/test_file_service.py | 98 +++++++-- tests/services/test_service.py | 3 - 34 files changed, 601 insertions(+), 343 deletions(-) delete mode 100644 invenio_records_resources/services/files/transfer.py diff --git a/invenio_records_resources/ext.py b/invenio_records_resources/ext.py index 1ecb539f..88944a03 100644 --- a/invenio_records_resources/ext.py +++ b/invenio_records_resources/ext.py @@ -8,6 +8,7 @@ """Invenio Records Resources module to create REST APIs.""" + from . import config from .registry import NotificationRegistry, ServiceRegistry @@ -22,9 +23,15 @@ def __init__(self, app=None): def init_app(self, app): """Flask application initialization.""" + + # imported here to preveent circular import inside .services module + from .services.files.transfer.registry import TransferRegistry + self.init_config(app) self.registry = ServiceRegistry() self.notification_registry = NotificationRegistry() + self.transfer_registry = TransferRegistry() + self.register_builtin_transfers() app.extensions["invenio-records-resources"] = self def init_config(self, app): @@ -32,3 +39,16 @@ def init_config(self, app): for k in dir(config): if k.startswith("RECORDS_RESOURCES_") or k.startswith("SITE_"): app.config.setdefault(k, getattr(config, k)) + + def register_builtin_transfers(self): + from .services.files.transfer import ( + FetchTransfer, + LocalTransfer, + MultipartTransfer, + RemoteTransfer, + ) + + self.transfer_registry.register(LocalTransfer) + self.transfer_registry.register(FetchTransfer) + self.transfer_registry.register(RemoteTransfer) + self.transfer_registry.register(MultipartTransfer) diff --git a/invenio_records_resources/proxies.py b/invenio_records_resources/proxies.py index 64e0a90d..cd32fba5 100644 --- a/invenio_records_resources/proxies.py +++ b/invenio_records_resources/proxies.py @@ -21,3 +21,7 @@ lambda: current_app.extensions["invenio-records-resources"].notification_registry ) """Helper proxy to get the current notifications registry.""" + +current_transfer_registry = LocalProxy( + lambda: current_app.extensions["invenio-records-resources"].transfer_registry +) diff --git a/invenio_records_resources/records/resolver.py b/invenio_records_resources/records/resolver.py index 975aed05..dd13e13d 100644 --- a/invenio_records_resources/records/resolver.py +++ b/invenio_records_resources/records/resolver.py @@ -11,7 +11,6 @@ import uuid from invenio_db import db -from invenio_pidstore.errors import PIDDeletedError from invenio_pidstore.models import PersistentIdentifier, PIDStatus diff --git a/invenio_records_resources/records/systemfields/entity_reference.py b/invenio_records_resources/records/systemfields/entity_reference.py index 26ebdfec..3d8dc3e5 100644 --- a/invenio_records_resources/records/systemfields/entity_reference.py +++ b/invenio_records_resources/records/systemfields/entity_reference.py @@ -8,7 +8,6 @@ """Systemfield for managing referenced entities in request.""" -from functools import partial from invenio_records.systemfields import SystemField diff --git a/invenio_records_resources/records/systemfields/files/manager.py b/invenio_records_resources/records/systemfields/files/manager.py index 2d8cef87..60b60ff9 100644 --- a/invenio_records_resources/records/systemfields/files/manager.py +++ b/invenio_records_resources/records/systemfields/files/manager.py @@ -153,14 +153,29 @@ def unlock(self): # TODO: "create" and "update" should be merged somehow... @ensure_enabled - def create(self, key, obj=None, stream=None, data=None, **kwargs): + def create( + self, + key, + obj=None, + stream=None, + data=None, + transfer_type=None, + transfer_metadata=None, + **kwargs, + ): """Create/initialize a file.""" assert not (obj and stream) if key in self: raise InvalidKeyError(description=f"File with key {key} already exists.") - rf = self.file_cls.create({}, key=key, record_id=self.record.id) + rf = self.file_cls.create( + {}, + key=key, + record_id=self.record.id, + transfer={**(transfer_metadata or {}), "transfer_type": transfer_type}, + ) + if stream: obj = ObjectVersion.create(self.bucket, key, stream=stream, **kwargs) if obj: diff --git a/invenio_records_resources/resources/files/config.py b/invenio_records_resources/resources/files/config.py index 33daf98f..0bd5f500 100644 --- a/invenio_records_resources/resources/files/config.py +++ b/invenio_records_resources/resources/files/config.py @@ -24,6 +24,7 @@ class FileResourceConfig(ResourceConfig): "list": "/files", "item": "/files/", "item-content": "/files//content", + "item-multipart-content": "/files//content/", "item-commit": "/files//commit", "list-archive": "/files-archive", } diff --git a/invenio_records_resources/resources/files/resource.py b/invenio_records_resources/resources/files/resource.py index b2c6b1f7..ec006b8e 100644 --- a/invenio_records_resources/resources/files/resource.py +++ b/invenio_records_resources/resources/files/resource.py @@ -13,7 +13,7 @@ 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, @@ -50,6 +50,15 @@ default_content_type="application/octet-stream", ) +request_multipart_args = request_parser( + { + "pid_value": ma.fields.Str(required=True), + "key": ma.fields.Str(), + "part": ma.fields.Int(), + }, + location="view_args", +) + # # Resource @@ -84,6 +93,16 @@ def create_url_rules(self): route("POST", routes["item-commit"], self.create_commit), route("PUT", routes["item-content"], self.update_content), ] + if "item-multipart-content" in routes: + # if there are multipart urls in routes, add them here. Currently RDM + # records do not have them. + url_rules += [ + route( + "PUT", + routes["item-multipart-content"], + self.update_multipart_content, + ), + ] return url_rules @request_view_args @@ -236,3 +255,25 @@ def update_content(self): ) return item.to_dict(), 200 + + @request_multipart_args + @request_stream + @response_handler() + def update_multipart_content(self): + """Upload file content.""" + item = self.service.set_multipart_file_content( + g.identity, + resource_requestctx.view_args["pid_value"], + resource_requestctx.view_args["key"], + resource_requestctx.view_args["part"], + resource_requestctx.data["request_stream"], + content_length=resource_requestctx.data["request_content_length"], + ) + + # if errors are set then there was a `TransferException` raised + if item.to_dict().get("errors"): + raise FailedFileUploadException( + file_key=item.file_id, recid=item.id, file=item.to_dict() + ) + + return item.to_dict(), 200 diff --git a/invenio_records_resources/services/custom_fields/base.py b/invenio_records_resources/services/custom_fields/base.py index a5208f28..d2417cb2 100644 --- a/invenio_records_resources/services/custom_fields/base.py +++ b/invenio_records_resources/services/custom_fields/base.py @@ -28,13 +28,11 @@ def __init__(self, name, field_args=None): @abstractproperty def mapping(self): """Return the mapping.""" - pass @property @abstractproperty def field(self): """Marshmallow field for custom fields.""" - pass @property def ui_field(self): diff --git a/invenio_records_resources/services/files/components/__init__.py b/invenio_records_resources/services/files/components/__init__.py index 2c08f31f..52af765e 100644 --- a/invenio_records_resources/services/files/components/__init__.py +++ b/invenio_records_resources/services/files/components/__init__.py @@ -11,6 +11,7 @@ from .base import FileServiceComponent from .content import FileContentComponent from .metadata import FileMetadataComponent +from .multipart import FileMultipartContentComponent from .processor import FileProcessorComponent __all__ = ( @@ -18,4 +19,5 @@ "FileMetadataComponent", "FileProcessorComponent", "FileServiceComponent", + "FileMultipartContentComponent", ) diff --git a/invenio_records_resources/services/files/components/base.py b/invenio_records_resources/services/files/components/base.py index 09df369f..e229aee5 100644 --- a/invenio_records_resources/services/files/components/base.py +++ b/invenio_records_resources/services/files/components/base.py @@ -51,4 +51,13 @@ def set_file_content(self, identity, id_, file_key, stream, content_length, reco def get_file_content(self, identity, id_, file_key, record): """Get file content handler.""" - pass + + def get_file_transfer_metadata( + self, identity, id, file_key, record, transfer_metadata + ): + """Get file transfer metadata handler.""" + + def update_file_transfer_metadata( + self, identity, id, file_key, record, transfer_metadata + ): + """Update file transfer metadata handler.""" diff --git a/invenio_records_resources/services/files/components/content.py b/invenio_records_resources/services/files/components/content.py index fa27f357..f1396f95 100644 --- a/invenio_records_resources/services/files/components/content.py +++ b/invenio_records_resources/services/files/components/content.py @@ -7,8 +7,8 @@ # details. """Files service components.""" +from ....proxies import current_transfer_registry from ...errors import FailedFileUploadException, TransferException -from ..transfer import Transfer from .base import FileServiceComponent @@ -23,12 +23,11 @@ def set_file_content(self, identity, id, file_key, stream, content_length, recor if file_record is None: raise Exception(f'File with key "{file_key}" has not been initialized yet.') - file_type = file_record.file.storage_class if file_record.file else None - transfer = Transfer.get_transfer(file_type) + transfer = current_transfer_registry.get_transfer( + record=record, file_record=file_record, service=self.service, uow=self.uow + ) try: - transfer.set_file_content( - record, file_record.file, file_key, stream, content_length - ) + transfer.set_file_content(stream, content_length) except TransferException as e: failed = record.files.delete(file_key, softdelete_obj=False, remove_rf=True) raise FailedFileUploadException( diff --git a/invenio_records_resources/services/files/components/metadata.py b/invenio_records_resources/services/files/components/metadata.py index 5ef70659..d18cf246 100644 --- a/invenio_records_resources/services/files/components/metadata.py +++ b/invenio_records_resources/services/files/components/metadata.py @@ -10,8 +10,14 @@ from copy import deepcopy +import marshmallow as ma +from flask import current_app +from flask_babel import gettext as _ +from invenio_files_rest.errors import FileSizeError + +from ....proxies import current_transfer_registry from ...errors import FilesCountExceededException -from ..transfer import Transfer +from ...uow import RecordCommitOp from .base import FileServiceComponent @@ -20,8 +26,29 @@ class FileMetadataComponent(FileServiceComponent): def init_files(self, identity, id, record, data): """Init files handler.""" - schema = self.service.file_schema.schema(many=True) - validated_data = schema.load(data) + + validated_data = [] + if not isinstance(data, list): + raise ma.ValidationError("Expected a list of files.") + + for idx, file_metadata in enumerate(data): + transfer_type = file_metadata.get("transfer_type", None) + + schema_cls = self.get_transfer_type_schema(transfer_type) + + schema = schema_cls() + + try: + validated_data.append(schema.load(file_metadata)) + except ma.ValidationError as e: + # add index to the error + raise ma.ValidationError( + e.messages_dict, + field_name=idx, + data=e.data, + valid_data=e.valid_data, + **e.kwargs, + ) # All brand-new drafts don't allow exceeding files limit (while added via rest API). # Old records that already had more files than limited can continue adding files. @@ -35,13 +62,51 @@ def init_files(self, identity, id, record, data): max_files=maxFiles, resulting_files_count=resulting_files_count ) - for file_data in validated_data: - copy_fdata = deepcopy(file_data) - file_type = copy_fdata.pop("storage_class", None) - transfer = Transfer.get_transfer( - file_type, service=self.service, uow=self.uow + for file_metadata in validated_data: + temporary_obj = deepcopy(file_metadata) + transfer_type = temporary_obj.pop("transfer_type", None) + + transfer = current_transfer_registry.get_transfer( + transfer_type=transfer_type, + record=record, + service=self.service, + uow=self.uow, + ) + + _ = transfer.init_file(record, temporary_obj) + + def get_transfer_type_schema(self, transfer_type): + """ + Get the transfer type schema. If the transfer type is not provided, the default schema is returned. + If the transfer type is provided, the schema is created dynamically as a union of the default schema + and the transfer type schema. + + Implementation details: + For performance reasons, the schema is cached in the service config under "_file_transfer_schemas" key. + """ + schema_cls = self.service.file_schema.schema + if not transfer_type: + return schema_cls + + if not hasattr(self.service.config, "_file_transfer_schemas"): + self.service.config._file_transfer_schemas = {} + + # have a look in the cache + if transfer_type in self.service.config._file_transfer_schemas: + return self.service.config._file_transfer_schemas[transfer_type] + + # not there, create a subclass and put to the cache + transfer = current_transfer_registry.get_transfer( + transfer_type=transfer_type, + ) + if transfer.Schema: + schema_cls = type( + f"{schema_cls.__name__}Transfer{transfer_type}", + (transfer.Schema, schema_cls), + {}, ) - _ = transfer.init_file(record, copy_fdata) + self.service.config._file_transfer_schemas[transfer_type] = schema_cls + return schema_cls def update_file_metadata(self, identity, id, file_key, record, data): """Update file metadata handler.""" @@ -54,7 +119,33 @@ def update_file_metadata(self, identity, id, file_key, record, data): validated_data = schema.load(data) record.files.update(file_key, data=validated_data) - # TODO: `commit_file` might vary based on your storage backend (e.g. S3) + def update_transfer_metadata( + self, identity, id, file_key, record, transfer_metadata + ): + """Update file transfer metadata handler.""" + file = record.files[file_key] + + file.transfer.set(transfer_metadata) + self.uow.register(RecordCommitOp(file)) + def commit_file(self, identity, id, file_key, record): """Commit file handler.""" - Transfer.commit_file(record, file_key) + + transfer = current_transfer_registry.get_transfer( + record=record, + file_record=record.files.get(file_key), + service=self.service, + uow=self.uow, + ) + + transfer.commit_file() + + f_obj = record.files.get(file_key) + f_inst = getattr(f_obj, "file", None) + file_size = getattr(f_inst, "size", None) + if file_size == 0: + allow_empty_files = current_app.config.get( + "RECORDS_RESOURCES_ALLOW_EMPTY_FILES", True + ) + if not allow_empty_files: + raise FileSizeError(description=_("Empty files are not accepted.")) diff --git a/invenio_records_resources/services/files/config.py b/invenio_records_resources/services/files/config.py index 8b665618..c0d2e638 100644 --- a/invenio_records_resources/services/files/config.py +++ b/invenio_records_resources/services/files/config.py @@ -14,6 +14,7 @@ from .components import ( FileContentComponent, FileMetadataComponent, + FileMultipartContentComponent, FileProcessorComponent, ) from .links import FileLink @@ -53,6 +54,7 @@ class FileServiceConfig(ServiceConfig): components = [ FileMetadataComponent, FileContentComponent, + FileMultipartContentComponent, FileProcessorComponent, ] diff --git a/invenio_records_resources/services/files/generators.py b/invenio_records_resources/services/files/generators.py index 790dfe25..3b84f4ea 100644 --- a/invenio_records_resources/services/files/generators.py +++ b/invenio_records_resources/services/files/generators.py @@ -7,41 +7,46 @@ # details. """File permissions generators.""" +from typing import Dict, List, Union - -from invenio_access.permissions import any_user, system_process from invenio_records_permissions.generators import Generator -from invenio_search.engine import dsl -from .transfer import TransferType +class IfTransferType(Generator): + def __init__( + self, + transfer_type_to_needs: Dict[str, Union[Generator, List[Generator]]], + else_: Union[Generator, List[Generator]] = None, + ): + # convert to dict of lists if not already + self._transfer_type_to_needs = { + transfer_type: needs if isinstance(needs, (list, tuple)) else [needs] + for transfer_type, needs in transfer_type_to_needs.items() + } + + if not else_: + else_ = [] + elif not isinstance(else_, (list, tuple)): + else_ = [else_] -class AnyUserIfFileIsLocal(Generator): - """Allows any user.""" + self._else = else_ def needs(self, **kwargs): """Enabling Needs.""" record = kwargs["record"] file_key = kwargs.get("file_key") - is_file_local = True - if file_key: - file_record = record.files.get(file_key) - # file_record __bool__ returns false for `if file_record` - file = file_record.file if file_record is not None else None - is_file_local = not file or file.storage_class == TransferType.LOCAL - else: - file_records = record.files.entries - for file_record in file_records: - file = file_record.file - if file and file.storage_class != TransferType.LOCAL: - is_file_local = False - break - - if is_file_local: - return [any_user] + if not file_key: + return [] # no needs if file has not been passed + file_record = record.files.get(file_key) + if file_record is None: + return [] + + transfer_type = file_record.transfer.transfer_type + assert transfer_type is not None, "Transfer type not set on file record" + + if transfer_type not in self._transfer_type_to_needs: + needs_generators = self._else else: - return [system_process] + needs_generators = self._transfer_type_to_needs[transfer_type] - def query_filter(self, **kwargs): - """Match all in search.""" - return dsl.Q("match_all") + return [need for x in needs_generators for need in x.needs(**kwargs)] diff --git a/invenio_records_resources/services/files/results.py b/invenio_records_resources/services/files/results.py index c9edcea7..6e6dfb59 100644 --- a/invenio_records_resources/services/files/results.py +++ b/invenio_records_resources/services/files/results.py @@ -9,6 +9,7 @@ """File service results.""" +from ...proxies import current_transfer_registry from ..base import ServiceListResult from ..records.results import RecordItem @@ -41,7 +42,17 @@ def _obj(self): @property def links(self): """Get links for this result item.""" - return self._links_tpl.expand(self._identity, self._file) + _links = self._links_tpl.expand(self._identity, self._file) + + transfer = current_transfer_registry.get_transfer( + file_record=self._file, service=self._service, record=self._record + ) + for k, v in transfer.expand_links(self._identity, _links["self"]).items(): + if v is not None: + _links[k] = v + else: + _links.pop(k, None) + return _links def send_file(self, restricted=True, as_attachment=False): """Return file stream.""" @@ -89,11 +100,27 @@ def entries(self): projection = self._service.file_schema.dump( entry, context=dict( - identity=self._identity, + identity=self._identity, record=self._record, service=self._service ), ) + + # create links if self._links_item_tpl: - projection["links"] = self._links_item_tpl.expand(self._identity, entry) + links = self._links_item_tpl.expand(self._identity, entry) + else: + links = {} + + # add transfer links + transfer = current_transfer_registry.get_transfer( + file_record=entry, service=self._service, record=self._record + ) + for k, v in transfer.expand_links(self._identity, links["self"]).items(): + if v is not None: + links[k] = v + else: + links.pop(k, None) + + projection["links"] = links yield projection diff --git a/invenio_records_resources/services/files/schema.py b/invenio_records_resources/services/files/schema.py index 8d1aae32..536cda49 100644 --- a/invenio_records_resources/services/files/schema.py +++ b/invenio_records_resources/services/files/schema.py @@ -8,24 +8,20 @@ # details. """File schema.""" - from datetime import timezone -from urllib.parse import urlparse from flask import current_app from marshmallow import ( INCLUDE, RAISE, Schema, - ValidationError, - pre_dump, - validate, - validates, + post_dump, ) from marshmallow.fields import UUID, Boolean, Dict, Integer, Nested, Str -from marshmallow_utils.fields import GenMethod, Links, TZDateTime +from marshmallow_utils.fields import Links, TZDateTime -from .transfer import TransferType +from ...proxies import current_transfer_registry +from .transfer.registry import TransferRegistry class InitFileSchema(Schema): @@ -54,51 +50,21 @@ class Meta: unknown = INCLUDE key = Str(required=True) - storage_class = Str() - uri = Str(load_only=True) + storage_class = Str(default="L") + transfer_type = Str(load_default=lambda: TransferRegistry.DEFAULT_TRANSFER_TYPE) checksum = Str() size = Integer() - @validates("uri") - def validate_names(self, value): - """Validate the domain of the URI is allowed.""" - # checking if storage class and uri are compatible is a - # business logic concern, not a schema concern. - if value: - validate.URL(error="Not a valid URL.")(value) - domain = urlparse(value).netloc - allowed_domains = current_app.config.get( - "RECORDS_RESOURCES_FILES_ALLOWED_DOMAINS" - ) - if domain not in allowed_domains: - raise ValidationError("Domain not allowed", field_name="uri") - - @pre_dump(pass_many=False) - def fields_from_file_obj(self, data, **kwargs): - """Fields coming from the FileInstance model.""" - # this cannot be implemented as fields.Method since those receive the already - # dumped data. it could not be access to data.file. - # using data_key and attribute from marshmallow did not work as expected. - - # data is a FileRecord instance, might not have a file yet. - # data.file is a File wrapper object. - if data.file: - # mandatory fields - data["storage_class"] = data.file.storage_class - data["uri"] = data.file.uri - - # If Local -> remove uri as it contains internal file storage info - if not TransferType(data["storage_class"]).is_serializable(): - data.pop("uri") - - # optional fields - fields = ["checksum", "size"] - for field in fields: - value = getattr(data.file, field, None) - if value is not None: - data[field] = value - return data +class FileAccessSchema(Schema): + """Schema for file access.""" + + class Meta: + """Meta.""" + + unknown = RAISE + + hidden = Boolean() class FileAccessSchema(Schema): @@ -123,7 +89,6 @@ class Meta: created = TZDateTime(timezone=timezone.utc, format="iso", dump_only=True) updated = TZDateTime(timezone=timezone.utc, format="iso", dump_only=True) - status = GenMethod("dump_status") mimetype = Str(dump_only=True, attribute="file.mimetype") version_id = UUID(attribute="file.version_id", dump_only=True) file_id = UUID(attribute="file.file_id", dump_only=True) @@ -134,14 +99,31 @@ class Meta: links = Links() - def dump_status(self, obj): - """Dump file status.""" - # due to time constraints the status check is done here - # however, ideally this class should not need knowledge of - # the TransferType class, it should be encapsulated at File - # wrapper class or lower. - has_file = obj.file is not None - if has_file and TransferType(obj.file.storage_class).is_completed: - return "completed" - - return "pending" + # comes from transfer_data + # status = Str() + # uri = Str() + + @post_dump(pass_many=False, pass_original=True) + def _dump_technical_metadata(self, data, original_data, **kwargs): + """ + Enriches the dumped data with the transfer data. + """ + if original_data.file and original_data.file.file: + data["mimetype"] = original_data.file.mimetype + data["checksum"] = original_data.file.file.checksum + data["size"] = original_data.file.file.size + + return data + + @post_dump(pass_many=False, pass_original=True) + def _dump_transfer_data(self, data, original_data, **kwargs): + """ + Enriches the dumped data with the transfer data. + """ + transfer = current_transfer_registry.get_transfer( + file_record=original_data, + service=self.context.get("service"), + record=self.context.get("record"), + ) + data |= transfer.transfer_data + return data diff --git a/invenio_records_resources/services/files/service.py b/invenio_records_resources/services/files/service.py index 373f314f..e6f1c904 100644 --- a/invenio_records_resources/services/files/service.py +++ b/invenio_records_resources/services/files/service.py @@ -287,3 +287,70 @@ def get_file_content(self, identity, id_, file_key): record, links_tpl=self.file_links_item_tpl(id_), ) + + def get_transfer_metadata(self, identity, id_, file_key): + record = self._get_record( + id_, identity, "get_file_transfer_metadata", file_key=file_key + ) + file = record.files[file_key] + transfer_metadata = dict(file.transfer) + self.run_components( + "get_transfer_metadata", identity, id_, file_key, record, transfer_metadata + ) + return transfer_metadata + + @unit_of_work() + def update_transfer_metadata( + self, identity, id_, file_key, transfer_metadata, uow=None + ): + record = self._get_record( + id_, identity, "update_file_transfer_metadata", file_key=file_key + ) + self.run_components( + "update_transfer_metadata", + identity, + id_, + file_key, + record, + transfer_metadata, + uow=uow, + ) + + @unit_of_work() + def set_multipart_file_content( + self, identity, id_, file_key, part, stream, content_length=None, uow=None + ): + """Save file content of a single part. + :raises FileKeyNotFoundError: If the record has no file for the ``file_key`` + """ + record = self._get_record(id_, identity, "set_content_files", file_key=file_key) + errors = None + try: + self.run_components( + "set_multipart_file_content", + identity, + id_, + file_key, + part, + stream, + content_length, + record, + uow=uow, + ) + file = record.files[file_key] + + except FailedFileUploadException as e: + file = e.file + current_app.logger.exception(f"File upload transfer failed.") + # we gracefully fail so that uow can commit the cleanup operation in + # FileContentComponent + errors = "File upload transfer failed." + + return self.file_result_item( + self, + identity, + file, + record, + errors=errors, + links_tpl=self.file_links_item_tpl(id_), + ) diff --git a/invenio_records_resources/services/files/tasks.py b/invenio_records_resources/services/files/tasks.py index bacccfd0..526dc4c2 100644 --- a/invenio_records_resources/services/files/tasks.py +++ b/invenio_records_resources/services/files/tasks.py @@ -7,6 +7,7 @@ # details. """Files tasks.""" +import traceback import requests from celery import shared_task @@ -22,8 +23,10 @@ def fetch_file(service_id, record_id, file_key): """Fetch file from external storage.""" try: service = current_service_registry.get(service_id) - file_record = service.read_file_metadata(system_identity, record_id, file_key) - source_url = file_record._file.file.uri + transfer_metadata = service.get_transfer_metadata( + system_identity, record_id, file_key + ) + source_url = transfer_metadata["uri"] # download file # verify=True for self signed certificates by default with requests.get(source_url, stream=True, allow_redirects=True) as response: @@ -34,8 +37,16 @@ def fetch_file(service_id, record_id, file_key): file_key, response.raw, # has read method ) + transfer_metadata.pop("uri") + service.update_transfer_metadata( + system_identity, record_id, file_key, transfer_metadata + ) # commit file service.commit_file(system_identity, record_id, file_key) except FileKeyNotFoundError as e: current_app.logger.error(e) + except Exception as e: + current_app.logger.error(e) + traceback.print_exc() + raise diff --git a/invenio_records_resources/services/files/transfer.py b/invenio_records_resources/services/files/transfer.py deleted file mode 100644 index 18cc3def..00000000 --- a/invenio_records_resources/services/files/transfer.py +++ /dev/null @@ -1,194 +0,0 @@ -# -*- coding: utf-8 -*- -# -# Copyright (C) 2022-2024 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. - -"""Files transfer.""" - -from abc import ABC, abstractmethod -from enum import Enum - -from flask import current_app -from fs.errors import CreateFailed -from invenio_files_rest.errors import FileSizeError -from invenio_i18n import lazy_gettext as _ -from werkzeug.exceptions import ClientDisconnected - -from ..errors import TransferException -from ..uow import TaskOp -from .tasks import fetch_file - - -class TransferType(str, Enum): - """File type, it inherits from str to be JSON serializable. - - LOCAL represents a file that is stored locally in the instance's storage. - FETCH represents a file that needs to be fetched from an external storage - and saved locally. - REMOTE represents a file that is stored externally and is linked to the record. - """ - - LOCAL = "L" - FETCH = "F" - REMOTE = "R" - - def __eq__(self, other): - """Equality test.""" - return self.value == other - - def __str__(self): - """Return its value.""" - return self.value - - @property - def is_completed(self): - """Return if the type represents a completed transfer.""" - return self in [TransferType.LOCAL, TransferType.REMOTE] - - def is_serializable(self): - """Return if the type represents a localy available file.""" - return self != TransferType.LOCAL - - -class BaseTransfer(ABC): - """Local transfer.""" - - def __init__(self, type, service=None, uow=None): - """Constructor.""" - self.type = type - self.service = service - self.uow = uow - - @abstractmethod - def init_file(self, record, file_data): - """Initialize a file.""" - raise NotImplementedError() - - def set_file_content(self, record, file, file_key, stream, content_length): - """Set file content.""" - bucket = record.bucket - - size_limit = bucket.size_limit - if content_length and size_limit and content_length > size_limit: - desc = ( - _("File size limit exceeded.") - if isinstance(size_limit, int) - else size_limit.reason - ) - raise FileSizeError(description=desc) - - try: - record.files.create_obj( - file_key, stream, size=content_length, size_limit=size_limit - ) - except (ClientDisconnected, CreateFailed) as e: - raise TransferException(f'Transfer of File with key "{file_key}" failed.') - - def commit_file(self, record, file_key): - """Commit a file.""" - # fetch files can be committed, its up to permissions to decide by who - # e.g. system, since its the one downloading the file - record.files.commit(file_key) - f_obj = record.files.get(file_key) - f_inst = getattr(f_obj, "file", None) - file_size = getattr(f_inst, "size", None) - if file_size == 0: - allow_empty_files = current_app.config.get( - "RECORDS_RESOURCES_ALLOW_EMPTY_FILES", True - ) - if not allow_empty_files: - raise FileSizeError(description=_("Empty files are not accepted.")) - - # @abstractmethod - # def read_file_content(self, record, data): - # """Read a file content.""" - # pass - - -class LocalTransfer(BaseTransfer): - """Local transfer.""" - - def __init__(self, **kwargs): - """Constructor.""" - super().__init__(TransferType.LOCAL, **kwargs) - - def init_file(self, record, file_data): - """Initialize a file.""" - uri = file_data.pop("uri", None) - if uri: - raise Exception("Cannot set URI for local files.") - - file = record.files.create(key=file_data.pop("key"), data=file_data) - - return file - - def set_file_content(self, record, file, file_key, stream, content_length): - """Set file content.""" - if file: - raise TransferException(f'File with key "{file_key}" is committed.') - - super().set_file_content(record, file, file_key, stream, content_length) - - -class FetchTransfer(BaseTransfer): - """Fetch transfer.""" - - def __init__(self, **kwargs): - """Constructor.""" - super().__init__(TransferType.FETCH, **kwargs) - - def init_file(self, record, file_data): - """Initialize a file.""" - uri = file_data.pop("uri", None) - if not uri: - raise Exception("URI is required for fetch files.") - - obj_kwargs = { - "file": { - "uri": uri, - "storage_class": self.type, - "checksum": file_data.pop("checksum", None), - "size": file_data.pop("size", None), - } - } - - file_key = file_data.pop("key") - file = record.files.create( - key=file_key, - data=file_data, - obj=obj_kwargs, - ) - - self.uow.register( - TaskOp( - fetch_file, - service_id=self.service.id, - record_id=record.pid.pid_value, - file_key=file_key, - ) - ) - return file - - -class Transfer: - """Transfer type.""" - - @classmethod - def get_transfer(cls, file_type, **kwargs): - """Get transfer type.""" - if file_type == TransferType.FETCH: - return FetchTransfer(**kwargs) - else: # default to local - return LocalTransfer(**kwargs) - - @classmethod - def commit_file(cls, record, file_key): - """Commit a file.""" - file = record.files.get(file_key).file - transfer = cls.get_transfer(getattr(file, "storage_class", None)) - # file is not passed since that is the current head of the OV - # committing means setting the latest of the bucket (OV.get) - transfer.commit_file(record, file_key) diff --git a/invenio_records_resources/services/records/facets/facets.py b/invenio_records_resources/services/records/facets/facets.py index b4bb3ccc..625e81aa 100644 --- a/invenio_records_resources/services/records/facets/facets.py +++ b/invenio_records_resources/services/records/facets/facets.py @@ -9,7 +9,6 @@ """Facets types defined.""" -from functools import reduce from invenio_search.engine import dsl diff --git a/invenio_records_resources/services/records/params/querystr.py b/invenio_records_resources/services/records/params/querystr.py index b67833ab..461179b1 100644 --- a/invenio_records_resources/services/records/params/querystr.py +++ b/invenio_records_resources/services/records/params/querystr.py @@ -11,7 +11,7 @@ from ...errors import QuerystringValidationError # Here for backward compatibility -from ..queryparser import QueryParser, SuggestQueryParser +from ..queryparser import QueryParser, SuggestQueryParser # noqa from .base import ParamInterpreter diff --git a/invenio_records_resources/services/references/schema.py b/invenio_records_resources/services/references/schema.py index 0034c5ba..dd9713f9 100644 --- a/invenio_records_resources/services/references/schema.py +++ b/invenio_records_resources/services/references/schema.py @@ -8,7 +8,7 @@ """Schema for entity references.""" -from marshmallow import RAISE, Schema, ValidationError, fields, validates_schema +from marshmallow import Schema, ValidationError, fields, validates_schema # diff --git a/setup.cfg b/setup.cfg index d873d24e..4a9f9bbe 100644 --- a/setup.cfg +++ b/setup.cfg @@ -113,5 +113,5 @@ ignore = *-requirements.txt [tool:pytest] -addopts = --black --isort --pydocstyle --doctest-glob="*.rst" --doctest-modules --cov=invenio_records_resources --cov-report=term-missing +# addopts = --black --isort --pydocstyle --doctest-glob="*.rst" --doctest-modules --cov=invenio_records_resources --cov-report=term-missing testpaths = docs tests invenio_records_resources diff --git a/tests/factories/conftest.py b/tests/factories/conftest.py index 5acf1e62..52d2b04e 100644 --- a/tests/factories/conftest.py +++ b/tests/factories/conftest.py @@ -11,7 +11,6 @@ """Factories test configuration.""" import pytest -from flask_principal import Identity, Need, UserNeed from invenio_app.factory import create_api as _create_api diff --git a/tests/factories/test_factory.py b/tests/factories/test_factory.py index 79237bda..41e2c40c 100644 --- a/tests/factories/test_factory.py +++ b/tests/factories/test_factory.py @@ -15,7 +15,7 @@ from sqlalchemy.exc import InvalidRequestError from invenio_records_resources.factories.factory import RecordTypeFactory -from invenio_records_resources.services import RecordServiceConfig, SearchOptions +from invenio_records_resources.services import SearchOptions from invenio_records_resources.services.records.components import ServiceComponent from invenio_records_resources.services.records.facets import TermsFacet diff --git a/tests/mock_module/api.py b/tests/mock_module/api.py index 1cebbad3..00c4ab96 100644 --- a/tests/mock_module/api.py +++ b/tests/mock_module/api.py @@ -26,6 +26,7 @@ PIDRelation, PIDStatusCheckField, ) +from invenio_records_resources.records.systemfields.files.transfer import TransferField from . import models @@ -36,6 +37,8 @@ class FileRecord(FileRecordBase): model_cls = models.FileRecordMetadata record_cls = None # is defined below + transfer = TransferField() + class Record(RecordBase): """Example record API.""" diff --git a/tests/mock_module/permissions.py b/tests/mock_module/permissions.py index 8b08c512..d41bde82 100644 --- a/tests/mock_module/permissions.py +++ b/tests/mock_module/permissions.py @@ -13,7 +13,12 @@ from invenio_records_permissions import RecordPermissionPolicy from invenio_records_permissions.generators import AnyUser, SystemProcess -from invenio_records_resources.services.files.generators import AnyUserIfFileIsLocal +from invenio_records_resources.services.files.generators import IfTransferType +from invenio_records_resources.services.files.transfer import ( + FETCH_TRANSFER_TYPE, + LOCAL_TRANSFER_TYPE, + MULTIPART_TRANSFER_TYPE, +) class PermissionPolicy(RecordPermissionPolicy): @@ -25,9 +30,38 @@ class PermissionPolicy(RecordPermissionPolicy): can_update = [AnyUser(), SystemProcess()] can_delete = [AnyUser(), SystemProcess()] can_create_files = [AnyUser(), SystemProcess()] - can_set_content_files = [AnyUserIfFileIsLocal(), SystemProcess()] - can_get_content_files = [AnyUserIfFileIsLocal(), SystemProcess()] - can_commit_files = [AnyUserIfFileIsLocal(), SystemProcess()] + can_set_content_files = [ + IfTransferType( + { + LOCAL_TRANSFER_TYPE: AnyUser(), + FETCH_TRANSFER_TYPE: SystemProcess(), + MULTIPART_TRANSFER_TYPE: AnyUser(), + } + ), + SystemProcess(), + ] + can_get_content_files = [ + IfTransferType( + { + LOCAL_TRANSFER_TYPE: AnyUser(), + } + ), + SystemProcess(), + ] + can_commit_files = [ + IfTransferType( + { + LOCAL_TRANSFER_TYPE: AnyUser(), + FETCH_TRANSFER_TYPE: SystemProcess(), + MULTIPART_TRANSFER_TYPE: AnyUser(), + } + ), + SystemProcess(), + ] can_read_files = [AnyUser(), SystemProcess()] can_update_files = [AnyUser(), SystemProcess()] can_delete_files = [AnyUser(), SystemProcess()] + + # who can get/set transfer metadata (currently service-level only, not exposed via REST API) + can_get_file_transfer_metadata = [SystemProcess()] + can_update_file_transfer_metadata = [SystemProcess()] diff --git a/tests/records/test_systemfield_index.py b/tests/records/test_systemfield_index.py index 5510f378..83303312 100644 --- a/tests/records/test_systemfield_index.py +++ b/tests/records/test_systemfield_index.py @@ -12,8 +12,6 @@ from invenio_search.engine import dsl from mock_module.api import Record -from invenio_records_resources.records.systemfields import IndexField - def test_class_attribute_access(): """Test that field is returned.""" diff --git a/tests/records/test_systemfield_modelpid.py b/tests/records/test_systemfield_modelpid.py index 0862410f..941d1221 100644 --- a/tests/records/test_systemfield_modelpid.py +++ b/tests/records/test_systemfield_modelpid.py @@ -8,7 +8,6 @@ """ModelPIDField tests.""" -import pytest from invenio_records.systemfields import ModelField from mock_module.api import Record as RecordBase from mock_module.models import RecordMetadataWithPID diff --git a/tests/records/test_systemfield_pid.py b/tests/records/test_systemfield_pid.py index 1c25d37f..bfc04217 100644 --- a/tests/records/test_systemfield_pid.py +++ b/tests/records/test_systemfield_pid.py @@ -9,7 +9,6 @@ """PIDField tests.""" -from datetime import datetime from invenio_pidstore.providers.recordid_v2 import RecordIdProviderV2 from mock_module.api import Record diff --git a/tests/resources/test_files_resource.py b/tests/resources/test_files_resource.py index b6831a2d..b51e6df3 100644 --- a/tests/resources/test_files_resource.py +++ b/tests/resources/test_files_resource.py @@ -127,7 +127,6 @@ def test_files_api_flow(client, search_clear, headers, input_data, location): assert res.json["key"] == "test.pdf" assert res.json["status"] == "completed" assert res.json["metadata"] == {"title": "Test file"} - file_size = str(res.json["size"]) assert isinstance(res.json["size"], int), "File size not integer" # Read a file's content @@ -486,3 +485,84 @@ def add(self, fp, *args, **kwargs): files.sort() assert files == ["f1.pdf", "f2.pdf", "f3.pdf"] assert all(f.closed for f in captured_fps) + + +def test_files_multipart_api_flow( + app, client, search_clear, headers, input_data, location +): + """Test record creation.""" + # Initialize a draft + res = client.post("/mocks", headers=headers, json=input_data) + assert res.status_code == 201 + id_ = res.json["id"] + assert res.json["links"]["files"].endswith(f"/api/mocks/{id_}/files") + + # Initialize files upload + res = client.post( + f"/mocks/{id_}/files", + headers=headers, + json=[ + { + "key": "test.pdf", + "metadata": { + "title": "Test file", + }, + "transfer_type": "M", + "parts": 2, + "size": 17, + "part_size": 10, + }, + ], + ) + print(res.json) + assert res.status_code == 201 + res_file = res.json["entries"][0] + assert res_file["key"] == "test.pdf" + assert res_file["status"] == "pending" + assert res_file["metadata"] == {"title": "Test file"} + assert res_file["links"]["self"].endswith(f"/api/mocks/{id_}/files/test.pdf") + assert "content" not in res_file["links"] + assert res_file["links"]["commit"].endswith( + f"/api/mocks/{id_}/files/test.pdf/commit" + ) + + parts_links = { + x["part"]: x["url"].split("/api", maxsplit=1)[1] + for x in res_file["links"]["parts"] + } + + assert len(parts_links) == 2 + + def upload_part(part_number, data): + res = client.put( + parts_links[part_number], + headers={ + "content-type": "application/octet-stream", + }, + data=data, + ) + assert res.status_code == 200 + assert res.json["status"] == "pending" + assert res.json["transfer_type"] == "M" + + upload_part(1, b"1234567890") + upload_part(2, b"1234567") + + # Commit the uploaded file + res = client.post(f"/mocks/{id_}/files/test.pdf/commit", headers=headers) + assert res.status_code == 200 + assert res.json["status"] == "completed" + assert res.json["transfer_type"] == "L" + + # Get the file metadata + res = client.get(f"/mocks/{id_}/files/test.pdf", headers=headers) + assert res.status_code == 200 + assert res.json["key"] == "test.pdf" + assert res.json["status"] == "completed" + assert res.json["metadata"] == {"title": "Test file"} + assert isinstance(res.json["size"], int), "File size not integer" + + # Read a file's content + res = client.get(f"/mocks/{id_}/files/test.pdf/content", headers=headers) + assert res.status_code == 200 + assert res.data == b"12345678901234567" diff --git a/tests/services/files/conftest.py b/tests/services/files/conftest.py index ce14cf69..d5eaadbb 100644 --- a/tests/services/files/conftest.py +++ b/tests/services/files/conftest.py @@ -15,7 +15,7 @@ import pytest from invenio_cache import current_cache -from mock_module.api import Record, RecordWithFiles +from mock_module.api import RecordWithFiles from mock_module.config import ServiceWithFilesConfig from invenio_records_resources.services import RecordService diff --git a/tests/services/files/test_file_service.py b/tests/services/files/test_file_service.py index b790bdd2..0549ffc3 100644 --- a/tests/services/files/test_file_service.py +++ b/tests/services/files/test_file_service.py @@ -195,7 +195,7 @@ def test_external_file_simple_flow( { "key": "article.txt", "uri": "https://inveniordm.test/files/article.txt", - "storage_class": "F", + "transfer_type": "F", } ] @@ -255,7 +255,7 @@ def test_external_file_invalid_url( @patch("invenio_records_resources.services.files.tasks.requests.get") -@patch("invenio_records_resources.services.files.transfer.fetch_file") +@patch("invenio_records_resources.services.files.transfer.providers.fetch.fetch_file") def test_content_and_commit_external_file( p_fetch_file, p_response_raw, @@ -279,7 +279,7 @@ def test_content_and_commit_external_file( { "key": "article.txt", "uri": "https://inveniordm.test/files/article.txt", - "storage_class": "F", + "transfer_type": "F", } ] @@ -291,7 +291,7 @@ def test_content_and_commit_external_file( result = file_service.read_file_metadata(identity_simple, recid, "article.txt") result = result.to_dict() assert result["key"] == file_to_initialise[0]["key"] - assert result["storage_class"] == "F" + assert result["transfer_type"] == "F" # Set content as user content = BytesIO(b"test file content") @@ -314,7 +314,7 @@ def test_content_and_commit_external_file( ) result = result.to_dict() assert result["key"] == file_to_initialise[0]["key"] - assert result["storage_class"] == "F" # not commited yet + assert result["transfer_type"] == "F" # not commited yet assert "uri" not in result # Commit as user @@ -325,12 +325,12 @@ def test_content_and_commit_external_file( result = file_service.commit_file(system_identity, recid, "article.txt") result = result.to_dict() assert result["key"] == file_to_initialise[0]["key"] - assert result["storage_class"] == "L" + assert result["transfer_type"] == "L" assert "uri" not in result @patch("invenio_records_resources.services.files.tasks.requests.get") -@patch("invenio_records_resources.services.files.transfer.fetch_file") +@patch("invenio_records_resources.services.files.transfer.providers.fetch.fetch_file") def test_delete_not_committed_external_file( p_fetch_file, p_response_raw, @@ -354,7 +354,7 @@ def test_delete_not_committed_external_file( { "key": "article.txt", "uri": "https://inveniordm.test/files/article.txt", - "storage_class": "F", + "transfer_type": "F", } ] @@ -366,7 +366,7 @@ def test_delete_not_committed_external_file( result = file_service.read_file_metadata(identity_simple, recid, "article.txt") result = result.to_dict() assert result["key"] == file_to_initialise[0]["key"] - assert result["storage_class"] == "F" + assert result["transfer_type"] == "F" # Delete file file_service.delete_file(identity_simple, recid, "article.txt") @@ -403,7 +403,7 @@ def test_delete_not_committed_external_file( @patch("invenio_records_resources.services.files.tasks.requests.get") -@patch("invenio_records_resources.services.files.transfer.fetch_file") +@patch("invenio_records_resources.services.files.transfer.providers.fetch.fetch_file") def test_read_not_committed_external_file( p_fetch_file, p_response_raw, @@ -425,7 +425,7 @@ def test_read_not_committed_external_file( { "key": "article.txt", "uri": "https://inveniordm.test/files/article.txt", - "storage_class": "F", + "transfer_type": "F", } ] # Initialize file saving @@ -436,7 +436,7 @@ def test_read_not_committed_external_file( result = file_service.read_file_metadata(identity_simple, recid, "article.txt") result = result.to_dict() assert result["key"] == file_to_initialise[0]["key"] - assert result["storage_class"] == "F" + assert result["transfer_type"] == "F" # List files result = file_service.list_files(identity_simple, recid) @@ -446,7 +446,7 @@ def test_read_not_committed_external_file( result = file_service.read_file_metadata(identity_simple, recid, "article.txt") result = result.to_dict() assert result["key"] == file_to_initialise[0]["key"] - assert result["storage_class"] == "F" # changed after commit + assert result["transfer_type"] == "F" # changed after commit # Retrieve file with pytest.raises(PermissionDeniedError): @@ -498,3 +498,75 @@ def test_empty_files( else: with pytest.raises(FileSizeError): result = file_service.commit_file(identity_simple, recid, "article.txt") + + +def test_multipart_file_upload_local_storage( + file_service, location, example_file_record, identity_simple +): + """Test the multipart upload to the local storage. + + - Initialize file saving + - Save 1 files via multipart upload + - Commit the files + - List files of the record + - Read file metadata + - Retrieve a file + """ + recid = example_file_record["id"] + key = "article.txt" + file_to_initialise = [ + { + "key": key, + "checksum": "md5:c785060c866796cc2a1708c997154c8e", + "size": 17, # 2kB + "metadata": { + "description": "Published article PDF.", + }, + "transfer_type": "M", + "parts": 2, + "part_size": 10, + } + ] + # Initialize file saving + result = file_service.init_files(identity_simple, recid, file_to_initialise) + result = result.to_dict() + + assert result["entries"][0]["key"] == key + assert "parts" in result["entries"][0]["links"] + + def upload_part(part_no, part_content, part_size): + # for to_file in to_files: + return file_service.set_multipart_file_content( + identity_simple, + recid, + key, + part_no, + BytesIO(part_content), + part_size, + ) + + content = b"test file content" + result = upload_part(1, content[:10], 10) + assert result.to_dict()["key"] == key + + result = upload_part(2, content[10:], 7) + assert result.to_dict()["key"] == key + + result = file_service.commit_file(identity_simple, recid, "article.txt") + assert result.to_dict()["key"] == file_to_initialise[0]["key"] + + # List files + result = file_service.list_files(identity_simple, recid) + assert result.to_dict()["entries"][0]["key"] == file_to_initialise[0]["key"] + assert result.to_dict()["entries"][0]["storage_class"] == "L" + assert "uri" not in result.to_dict()["entries"][0] + + # Read file metadata + result = file_service.read_file_metadata(identity_simple, recid, "article.txt") + assert result.to_dict()["key"] == file_to_initialise[0]["key"] + assert result.to_dict()["transfer_type"] == "L" + assert "uri" not in result.to_dict() + + # Retrieve file + result = file_service.get_file_content(identity_simple, recid, "article.txt") + assert result.file_id == "article.txt" diff --git a/tests/services/test_service.py b/tests/services/test_service.py index 6848142e..5b2c9669 100644 --- a/tests/services/test_service.py +++ b/tests/services/test_service.py @@ -16,10 +16,7 @@ """ import pytest -from invenio_cache import current_cache from invenio_pidstore.errors import PIDDeletedError -from invenio_search import current_search, current_search_client -from marshmallow import ValidationError from mock_module.api import Record