From d7e0bd552953a0bae0b3500ef8a1d51df3069add Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johnny=20Marie=CC=81thoz?= Date: Wed, 19 May 2021 12:27:56 +0200 Subject: [PATCH] operation logs: use an elasticsearch only resource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Renames virtua command cli name. * Fixes monitor view to compute elasticsearch and db count diff when the index does not exists. * Creates an operation logs elasticsearch record class. It creates one index per year. * Denies to all to read one record. * Adds a cli to dumps the operation logs in a JSON file for backup. * Closes: #1725. Co-Authored-By: Johnny MarieĢthoz --- data/operation_logs.json | 11 +- rero_ils/config.py | 6 +- rero_ils/modules/cli.py | 5 +- rero_ils/modules/monitoring.py | 2 + rero_ils/modules/operation_logs/api.py | 164 ++++++++++++------ rero_ils/modules/operation_logs/cli.py | 68 ++++---- .../{mappings/v7 => es_templates}/__init__.py | 11 +- .../{mappings => es_templates/v7}/__init__.py | 6 +- .../v7/operation_logs.json} | 6 + rero_ils/modules/operation_logs/extensions.py | 61 +++++++ .../modules/operation_logs/jsonresolver.py | 28 --- rero_ils/modules/operation_logs/listener.py | 24 +-- rero_ils/modules/operation_logs/models.py | 49 ------ .../modules/operation_logs/permissions.py | 2 +- scripts/setup | 2 +- setup.py | 13 +- .../test_operation_logs_permissions.py | 129 -------------- .../test_operation_logs_rest.py | 89 +++++++--- tests/fixtures/metadata.py | 9 +- .../operation_logs/test_operation_logs_api.py | 69 +++++--- .../test_operation_logs_jsonresolver.py | 47 ----- .../test_operation_logs_mapping.py | 32 +--- tests/ui/test_monitoring.py | 10 +- tests/unit/test_operation_logs_jsonschema.py | 8 +- 24 files changed, 393 insertions(+), 458 deletions(-) rename rero_ils/modules/operation_logs/{mappings/v7 => es_templates}/__init__.py (74%) rename rero_ils/modules/operation_logs/{mappings => es_templates/v7}/__init__.py (85%) rename rero_ils/modules/operation_logs/{mappings/v7/operation_logs/operation_log-v0.0.1.json => es_templates/v7/operation_logs.json} (92%) create mode 100644 rero_ils/modules/operation_logs/extensions.py delete mode 100644 rero_ils/modules/operation_logs/jsonresolver.py delete mode 100644 rero_ils/modules/operation_logs/models.py delete mode 100644 tests/api/operation_logs/test_operation_logs_permissions.py delete mode 100644 tests/ui/operation_logs/test_operation_logs_jsonresolver.py diff --git a/data/operation_logs.json b/data/operation_logs.json index fe51488c70..be8802efb7 100644 --- a/data/operation_logs.json +++ b/data/operation_logs.json @@ -1 +1,10 @@ -[] +[ + { + "record": { + "$ref": "https://ils.rero.ch/api/documents/1" + }, + "operation": "update", + "user_name": "system", + "date": "2021-01-21T09:51:52.879533+00:00" + } +] diff --git a/rero_ils/config.py b/rero_ils/config.py index 7b9fdda069..948af6858e 100644 --- a/rero_ils/config.py +++ b/rero_ils/config.py @@ -1613,13 +1613,12 @@ def _(x): action='delete', record=record, cls=TemplatePermission) ), oplg=dict( + # TODO: useless, but required pid_type='oplg', - pid_minter='operation_log_id', + pid_minter='recid', pid_fetcher='operation_log_id', - search_class='rero_ils.modules.operation_logs.api:OperationLogsSearch', search_index='operation_logs', search_type=None, - indexer_class='rero_ils.modules.operation_logs.api:OperationLogsIndexer', record_serializers={ 'application/json': ( 'rero_ils.modules.serializers:json_v1_response' @@ -1638,6 +1637,7 @@ def _(x): }, record_class='rero_ils.modules.operation_logs.api:OperationLog', list_route='/operation_logs/', + # TODO: create a converter for es id, not used for the moment. item_route='/operation_logs/', default_media_type='application/json', diff --git a/rero_ils/modules/cli.py b/rero_ils/modules/cli.py index 65e0c452a7..2c72f0aece 100644 --- a/rero_ils/modules/cli.py +++ b/rero_ils/modules/cli.py @@ -76,7 +76,7 @@ from .items.cli import create_items, reindex_items from .loans.cli import create_loans, load_virtua_transactions from .monitoring import Monitoring -from .operation_logs.cli import migrate_virtua_operation_logs +from .operation_logs.cli import create_operation_logs, dump_operation_logs from .patrons.cli import import_users, users_validate from .tasks import process_bulk_queue from .utils import bulk_load_metadata, bulk_load_pids, bulk_load_pidstore, \ @@ -110,7 +110,8 @@ def fixtures(): fixtures.add_command(create_patterns) fixtures.add_command(create_ill_requests) fixtures.add_command(create_collections) -fixtures.add_command(migrate_virtua_operation_logs) +fixtures.add_command(create_operation_logs) +fixtures.add_command(dump_operation_logs) @users.command('confirm') diff --git a/rero_ils/modules/monitoring.py b/rero_ils/modules/monitoring.py index 1881782724..0e44a33df2 100644 --- a/rero_ils/modules/monitoring.py +++ b/rero_ils/modules/monitoring.py @@ -446,10 +446,12 @@ def info(cls, with_deleted=False, difference_db_es=False): ).items(): info[doc_type] = {} count_db = cls.get_db_count(doc_type, with_deleted=with_deleted) + count_db = count_db if isinstance(count_db, int) else 0 info[doc_type]['db'] = count_db index = endpoint.get('search_index', '') if index: count_es = cls.get_es_count(index) + count_es = count_es if isinstance(count_es, int) else 0 db_es = count_db - count_es info[doc_type]['index'] = index info[doc_type]['es'] = count_es diff --git a/rero_ils/modules/operation_logs/api.py b/rero_ils/modules/operation_logs/api.py index 9d0ab67259..e79ba86d86 100644 --- a/rero_ils/modules/operation_logs/api.py +++ b/rero_ils/modules/operation_logs/api.py @@ -17,76 +17,132 @@ """API for manipulating operation_logs.""" -from functools import partial +from elasticsearch.helpers import bulk +from elasticsearch_dsl import Search +from invenio_records.api import RecordBase +from invenio_search import current_search_client -from .models import OperationLogIdentifier, OperationLogMetadata, \ - OperationLogOperation -from ..api import IlsRecord, IlsRecordsIndexer, IlsRecordsSearch -from ..fetchers import id_fetcher -from ..minters import id_minter -from ..providers import Provider +from .extensions import ResolveRefsExension +from ..fetchers import FetchedPID -# provider -OperationLogProvider = type( - 'OperationLogProvider', - (Provider,), - dict(identifier=OperationLogIdentifier, pid_type='oplg') -) -# minter -operation_log_id_minter = partial(id_minter, provider=OperationLogProvider) -# fetcher -operation_log_id_fetcher = partial(id_fetcher, provider=OperationLogProvider) +def operation_log_id_fetcher(record_uuid, data): + """Fetch a Organisation record's identifiers. -class OperationLogsSearch(IlsRecordsSearch): - """Operation log Search.""" + :param record_uuid: The record UUID. + :param data: The record metadata. + :return: A :data:`rero_ils.modules.fetchers.FetchedPID` instance. + """ + return FetchedPID( + provider=None, + pid_type='oplg', + pid_value=record_uuid + ) - class Meta: - """Search only on operation_log index.""" - index = 'operation_logs' - doc_types = None - fields = ('*', ) - facets = {} +LONG_INDEX_NAME = 'operation_logs-operation_log-v0.0.1' - default_filter = None - -class OperationLog(IlsRecord): +class OperationLog(RecordBase): """OperationLog class.""" - minter = operation_log_id_minter - fetcher = operation_log_id_fetcher - provider = OperationLogProvider - model_cls = OperationLogMetadata + index_name = 'operation_logs' - @classmethod - def get_create_operation_log_by_resource_pid(cls, pid_type, record_pid): - """Return a create operation log for a given resource and pid. + _extensions = [ResolveRefsExension()] - :param pid_type: resource pid type. - :param record_pid: record pid. + @classmethod + def create(cls, data, id_=None, index_refresh='false', **kwargs): + r"""Create a new record instance and store it in the database. + + :param data: Dict with the record metadata. + :param id_: Specify a UUID to use for the new record, instead of + automatically generated. + :param refresh: If `true` then refresh the affected shards to make + this operation visible to search, if `wait_for` then wait for a + refresh to make this operation visible to search, if `false` + (the default) then do nothing with refreshes. + Valid choices: true, false, wait_for + :returns: A new :class:`Record` instance. """ - search = OperationLogsSearch() - search = search.filter('term', record__pid=record_pid)\ - .filter('term', record__type=pid_type)\ - .filter('term', operation=OperationLogOperation.CREATE) - oplgs = search.source(['pid']).scan() - try: - return OperationLog.get_record_by_pid(next(oplgs).pid) - except StopIteration: - return None + record = cls( + data, + model=None, + **kwargs + ) + + # TODO: enable this whe a invenio-records is updated. + # # Run pre create extensions + # for e in cls._extensions: + # e.pre_create(record) + + res = current_search_client.index( + cls.get_index(record), + record.dumps(), + refresh=index_refresh) + record['id'] = res.get('_id') + + # Run post create extensions + for e in cls._extensions: + e.post_create(record) + return record + @classmethod + def get_index(cls, data): + """Get the index name given the data. + + One index per year is created based on the data date field. + + :param data: Dict with the record metadata. + :returns: str, the corresponding index name. + """ + suffix = '-'.join(data.get('date', '').split('-')[0:1]) + return f'{cls.index_name}-{suffix}' -class OperationLogsIndexer(IlsRecordsIndexer): - """Operation log indexing class.""" + @classmethod + def bulk(cls, data): + """Bulk indexing. - record_cls = OperationLog + :params data: list of dicts with the record metadata. + """ + actions = [] + for d in data: + d = OperationLog(d) + action = { + '_op_type': 'index', + '_index': cls.get_index(d), + '_source': d.dumps() + } + actions.append(action) + res = bulk(current_search_client, actions) + # TODO: check errors + print(res) - def bulk_index(self, record_id_iterator): - """Bulk index records. + @classmethod + def get_record(cls, id_): + """Retrieve the record by id. - :param record_id_iterator: Iterator yielding record UUIDs. + Raise a database exception if the record does not exist. + :param id_: record ID. + :returns: The :class:`Record` instance. """ - super(OperationLogsIndexer, self).bulk_index( - record_id_iterator, doc_type='oplg') + source = current_search_client.get(id=id_, index=cls.index_name) + return cls(source.get('_source', {})) + + @classmethod + def get_indices(cls): + """Get all index names present in the elasticsearch server.""" + return set([ + v['_index'] for v in current_search_client.search( + index=cls.index_name)['hits']['hits'] + ]) + + @classmethod + def delete_indices(cls): + """Remove all index names present in the elasticsearch server.""" + current_search_client.indices.delete(f'{cls.index_name}*') + return True + + @property + def id(self): + """Get model identifier.""" + return self.get('id') diff --git a/rero_ils/modules/operation_logs/cli.py b/rero_ils/modules/operation_logs/cli.py index 082a5aba86..945e9d199a 100644 --- a/rero_ils/modules/operation_logs/cli.py +++ b/rero_ils/modules/operation_logs/cli.py @@ -24,28 +24,26 @@ import click from flask import current_app from flask.cli import with_appcontext +from invenio_search.api import RecordsSearch from rero_ils.modules.operation_logs.api import OperationLog -from rero_ils.modules.operation_logs.models import OperationLogOperation -from rero_ils.modules.utils import extracted_data_from_ref from ..utils import read_json_record -@click.command('migrate_virtua_operation_logs') -@click.option('-v', '--verbose', 'verbose', is_flag=True, default=False) -@click.option('-d', '--debug', 'debug', is_flag=True, default=False) +@click.command('create_operation_logs') @click.option('-l', '--lazy', 'lazy', is_flag=True, default=False) +@click.option('-s', '--batch-size', 'size', type=int, default=10000) @click.argument('infile', type=click.File('r')) @with_appcontext -def migrate_virtua_operation_logs(infile, verbose, debug, lazy): - """Migrate Virtua operation log records in reroils. +def create_operation_logs(infile, lazy, size): + """Load operation log records in reroils. :param infile: Json operation log file. :param lazy: lazy reads file """ enabled_logs = current_app.config.get('RERO_ILS_ENABLE_OPERATION_LOG') - click.secho('Migrate Virtua operation log records:', fg='green') + click.secho('Load operation log records:', fg='green') if lazy: # try to lazy read json file (slower, better memory management) data = read_json_record(infile) @@ -54,28 +52,38 @@ def migrate_virtua_operation_logs(infile, verbose, debug, lazy): data = json.load(infile) index_count = 0 with click.progressbar(data) as bar: + records = [] for oplg in bar: - try: - operation = oplg.get('operation') - resource = extracted_data_from_ref( - oplg.get('record').get('$ref'), data='resource') - pid_type = enabled_logs.get(resource) - if pid_type and operation == OperationLogOperation.CREATE: - # The virtua create operation log overrides the reroils - # create operation log, the method to use is UPDATE - record_pid = extracted_data_from_ref( - oplg.get('record').get('$ref'), data='pid') + if not (index_count + 1) % size: + OperationLog.bulk(records) + records = [] + records.append(oplg) + index_count += 1 + # the rest of the records + if records: + OperationLog.bulk(records) + index_count += len(records) + click.echo(f'created {index_count} operation logs.') + - create_rec = \ - OperationLog.get_create_operation_log_by_resource_pid( - pid_type, record_pid) - if create_rec: - create_rec.update(oplg, dbcommit=True, reindex=True) - elif pid_type and operation == OperationLogOperation.UPDATE: - # The virtua update operation log is a new entry in the - # reroils operation log, the method to use is CREATE - OperationLog.create(data=oplg, dbcommit=True, reindex=True) - except Exception: - pass - index_count += len(data) +@click.command('dump_operation_logs') +@click.argument('outfile', type=click.File('w')) +@with_appcontext +def dump_operation_logs(outfile): + """Dumps operation log records in a given file. + + :param outfile: Json operation log output file. + """ + enabled_logs = current_app.config.get('RERO_ILS_ENABLE_OPERATION_LOG') + click.secho('Dumps operation log records:', fg='green') + search = RecordsSearch(index=OperationLog.index_name) + + index_count = 0 + outfile.write('[\n') + with click.progressbar(search.scan()) as bar: + for oplg in bar: + outfile.write(str(oplg.to_dict())) + outfile.write(',\n') + index_count += 1 + outfile.write('\n]') click.echo(f'created {index_count} operation logs.') diff --git a/rero_ils/modules/operation_logs/mappings/v7/__init__.py b/rero_ils/modules/operation_logs/es_templates/__init__.py similarity index 74% rename from rero_ils/modules/operation_logs/mappings/v7/__init__.py rename to rero_ils/modules/operation_logs/es_templates/__init__.py index e9f3dd93af..565801510b 100644 --- a/rero_ils/modules/operation_logs/mappings/v7/__init__.py +++ b/rero_ils/modules/operation_logs/es_templates/__init__.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # # RERO ILS -# Copyright (C) 2021 RERO +# Copyright (C) 2019 RERO # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as published by @@ -15,6 +15,11 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -"""Elasticsearch mappings.""" +"""Operation logs records elasticsarch templates.""" -from __future__ import absolute_import, print_function + +def list_es_templates(): + """Elasticsearch Templates path.""" + return [ + 'rero_ils.modules.operation_logs.es_templates' + ] diff --git a/rero_ils/modules/operation_logs/mappings/__init__.py b/rero_ils/modules/operation_logs/es_templates/v7/__init__.py similarity index 85% rename from rero_ils/modules/operation_logs/mappings/__init__.py rename to rero_ils/modules/operation_logs/es_templates/v7/__init__.py index e9f3dd93af..7aed4434dc 100644 --- a/rero_ils/modules/operation_logs/mappings/__init__.py +++ b/rero_ils/modules/operation_logs/es_templates/v7/__init__.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # # RERO ILS -# Copyright (C) 2021 RERO +# Copyright (C) 2020 RERO # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as published by @@ -15,6 +15,4 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -"""Elasticsearch mappings.""" - -from __future__ import absolute_import, print_function +"""ES Templates module for the operation logs.""" diff --git a/rero_ils/modules/operation_logs/mappings/v7/operation_logs/operation_log-v0.0.1.json b/rero_ils/modules/operation_logs/es_templates/v7/operation_logs.json similarity index 92% rename from rero_ils/modules/operation_logs/mappings/v7/operation_logs/operation_log-v0.0.1.json rename to rero_ils/modules/operation_logs/es_templates/v7/operation_logs.json index 2dac7f2459..3bfcd7853f 100644 --- a/rero_ils/modules/operation_logs/mappings/v7/operation_logs/operation_log-v0.0.1.json +++ b/rero_ils/modules/operation_logs/es_templates/v7/operation_logs.json @@ -1,4 +1,7 @@ { + "index_patterns": [ + "operation_logs*" + ], "settings": { "number_of_shards": 8, "number_of_replicas": 1, @@ -60,5 +63,8 @@ "type": "date" } } + }, + "aliases": { + "operation_logs": {} } } diff --git a/rero_ils/modules/operation_logs/extensions.py b/rero_ils/modules/operation_logs/extensions.py new file mode 100644 index 0000000000..8e6e7248a1 --- /dev/null +++ b/rero_ils/modules/operation_logs/extensions.py @@ -0,0 +1,61 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2021 RERO +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""Operation logs Record extensions.""" + +from invenio_records.extensions import RecordExtension + +from ..utils import extracted_data_from_ref + + +class ResolveRefsExension(RecordExtension): + """Defines the methods needed by an extension.""" + + mod_type = { + 'documents': 'doc', + 'items': 'item', + 'holdings': 'hold' + } + + def pre_dump(self, record, dumper=None): + """Called before a record is dumped. + + :param record: the record metadata. + :param dumper: the record dumper. + """ + self._resolve_refs(record) + + def _resolve_refs(self, record): + """Recursively replace the $refs. + + Replace in place all $ref to a dict of pid, type values. + + :param record: the record metadata. + """ + for k, v in record.items(): + if isinstance(v, dict): + if v.get('$ref'): + _type = self.mod_type.get( + extracted_data_from_ref(v, data='resource')) + if _type: + resolved = dict( + pid=extracted_data_from_ref(v), + type=_type + ) + record[k] = resolved + else: + self.resolve_refs(v) diff --git a/rero_ils/modules/operation_logs/jsonresolver.py b/rero_ils/modules/operation_logs/jsonresolver.py deleted file mode 100644 index c15108396c..0000000000 --- a/rero_ils/modules/operation_logs/jsonresolver.py +++ /dev/null @@ -1,28 +0,0 @@ -# -*- coding: utf-8 -*- -# -# RERO ILS -# Copyright (C) 2021 RERO -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published by -# the Free Software Foundation, version 3 of the License. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see . - -"""OperationLog resolver.""" - -import jsonresolver - -from ..jsonresolver import resolve_json_refs - - -@jsonresolver.route('/api/operation_logs/', host='ils.rero.ch') -def operation_log_resolver(pid): - """Resolver for operation_log record.""" - return resolve_json_refs('oplg', pid) diff --git a/rero_ils/modules/operation_logs/listener.py b/rero_ils/modules/operation_logs/listener.py index b792408d0f..eb282a88e0 100644 --- a/rero_ils/modules/operation_logs/listener.py +++ b/rero_ils/modules/operation_logs/listener.py @@ -22,7 +22,6 @@ from flask import current_app from .api import OperationLog -from .models import OperationLogOperation from ..patrons.api import current_librarian from ..utils import extracted_data_from_ref, get_ref_for_pid @@ -36,8 +35,7 @@ def operation_log_record_create(sender, record=None, *args, **kwargs): :param record: the record being created. """ - build_operation_log_record( - record=record, operation=OperationLogOperation.CREATE) + build_operation_log_record(record=record, operation='create') def operation_log_record_update(sender, record=None, *args, **kwargs): @@ -49,14 +47,13 @@ def operation_log_record_update(sender, record=None, *args, **kwargs): :param record: the record being updated. """ - build_operation_log_record( - record=record, operation=OperationLogOperation.UPDATE) + build_operation_log_record(record=record, operation='update') def build_operation_log_record(record=None, operation=None): """Build an operation_log record to load. - :param record: the record being created or updated. + :param record: the record being created. """ if record.get('$schema'): resource_name = extracted_data_from_ref( @@ -65,18 +62,21 @@ def build_operation_log_record(record=None, operation=None): 'RERO_ILS_ENABLE_OPERATION_LOG'): oplg = { 'date': datetime.now(timezone.utc).isoformat(), - 'record': {'$ref': get_ref_for_pid( - record.provider.pid_type, record.get('pid'))}, + 'record': { + '$ref': get_ref_for_pid( + record.provider.pid_type, record.get('pid')) + }, 'operation': operation } if current_librarian: oplg['user'] = { - '$ref': get_ref_for_pid('ptrn', current_librarian.pid)} + '$ref': get_ref_for_pid('ptrn', current_librarian.pid) + } oplg['user_name'] = current_librarian.formatted_name oplg['organisation'] = { '$ref': get_ref_for_pid( - 'org', current_librarian.organisation_pid)} + 'org', current_librarian.organisation_pid) + } else: oplg['user_name'] = 'system' - oplg = OperationLog(oplg) - oplg.create(oplg, dbcommit=True, reindex=True) + OperationLog.create(oplg) diff --git a/rero_ils/modules/operation_logs/models.py b/rero_ils/modules/operation_logs/models.py deleted file mode 100644 index b45d584ed3..0000000000 --- a/rero_ils/modules/operation_logs/models.py +++ /dev/null @@ -1,49 +0,0 @@ -# -*- coding: utf-8 -*- -# -# RERO ILS -# Copyright (C) 2021 RERO -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published by -# the Free Software Foundation, version 3 of the License. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see . - -"""Define relation between records and buckets.""" - -from __future__ import absolute_import - -from invenio_db import db -from invenio_pidstore.models import RecordIdentifier -from invenio_records.models import RecordMetadataBase - - -class OperationLogIdentifier(RecordIdentifier): - """Sequence generator for OperationLog identifiers.""" - - __tablename__ = 'operation_log_id' - __mapper_args__ = {'concrete': True} - - recid = db.Column( - db.BigInteger().with_variant(db.Integer, 'sqlite'), - primary_key=True, autoincrement=True, - ) - - -class OperationLogMetadata(db.Model, RecordMetadataBase): - """Operation log record metadata.""" - - __tablename__ = 'operation_log_metadata' - - -class OperationLogOperation: - """Enum class to list all possible operations.""" - - CREATE = 'create' - UPDATE = 'update' diff --git a/rero_ils/modules/operation_logs/permissions.py b/rero_ils/modules/operation_logs/permissions.py index 2cc6460963..a25a7bc6b6 100644 --- a/rero_ils/modules/operation_logs/permissions.py +++ b/rero_ils/modules/operation_logs/permissions.py @@ -44,7 +44,7 @@ def read(cls, user, record): :return: "True" if action can be done. """ # all users (lib, sys_lib) can read operation_log records. - return bool(current_librarian) + return False @classmethod def create(cls, user, record=None): diff --git a/scripts/setup b/scripts/setup index d89cc8be88..bd6c262bb3 100755 --- a/scripts/setup +++ b/scripts/setup @@ -511,7 +511,7 @@ then fi info_msg "- Load Virtua operation logs: ${DATA_PATH}/operation_logs.json" -eval ${PREFIX} invenio fixtures migrate_virtua_operation_logs ${DATA_PATH}/operation_logs.json ${CREATE_LAZY} ${DONT_STOP} +eval ${PREFIX} invenio fixtures create_operation_logs ${DATA_PATH}/operation_logs.json ${CREATE_LAZY} ${DONT_STOP} # load legacy circulation transactions from Virtua info_msg "Checkout transactions: ${DATA_PATH}/checkouts.json ${CREATE_LAZY} ${DONT_STOP}" diff --git a/setup.py b/setup.py index cdc7091426..7839f65605 100644 --- a/setup.py +++ b/setup.py @@ -174,7 +174,6 @@ def run(self): 'patrons = rero_ils.modules.patrons.models', 'templates = rero_ils.modules.templates.models', 'vendors = rero_ils.modules.vendors.models', - 'operation_logs = rero_ils.modules.operation_logs.models', 'selfcheck = rero_ils.modules.selfcheck.models', ], 'invenio_pidstore.minters': [ @@ -202,7 +201,6 @@ def run(self): 'patron_type_id = rero_ils.modules.patron_types.api:patron_type_id_minter', 'template_id = rero_ils.modules.templates.api:template_id_minter', 'vendor_id = rero_ils.modules.vendors.api:vendor_id_minter', - 'operation_log_id = rero_ils.modules.operation_logs.api:operation_log_id_minter', ], 'invenio_pidstore.fetchers': [ 'acq_account_id = rero_ils.modules.acq_accounts.api:acq_account_id_fetcher', @@ -258,7 +256,6 @@ def run(self): 'patrons = rero_ils.modules.patrons.jsonschemas', 'templates = rero_ils.modules.templates.jsonschemas', 'vendors = rero_ils.modules.vendors.jsonschemas', - 'operation_logs = rero_ils.modules.operation_logs.jsonschemas', 'users = rero_ils.modules.users.jsonschemas', ], 'invenio_search.mappings': [ @@ -286,11 +283,12 @@ def run(self): 'patron_types = rero_ils.modules.patron_types.mappings', 'patrons = rero_ils.modules.patrons.mappings', 'templates = rero_ils.modules.templates.mappings', - 'vendors = rero_ils.modules.vendors.mappings', - 'operation_logs = rero_ils.modules.operation_logs.mappings', + 'vendors = rero_ils.modules.vendors.mappings' ], 'invenio_search.templates': [ - 'base-record = rero_ils.es_templates:list_es_templates', + 'rero_ils = rero_ils.es_templates:list_es_templates', + 'operation_logs = rero_ils.modules.operation_logs' + '.es_templates:list_es_templates', ], 'invenio_celery.tasks': [ 'apiharvester = rero_ils.modules.apiharvester.tasks', @@ -326,8 +324,7 @@ def run(self): 'patron_types = rero_ils.modules.patron_types.jsonresolver', 'patrons = rero_ils.modules.patrons.jsonresolver', 'templates = rero_ils.modules.templates.jsonresolver', - 'vendors = rero_ils.modules.vendors.jsonresolver', - 'operation_logs = rero_ils.modules.operation_logs.jsonresolver', + 'vendors = rero_ils.modules.vendors.jsonresolver' ] }, classifiers=[ diff --git a/tests/api/operation_logs/test_operation_logs_permissions.py b/tests/api/operation_logs/test_operation_logs_permissions.py deleted file mode 100644 index cc571df3db..0000000000 --- a/tests/api/operation_logs/test_operation_logs_permissions.py +++ /dev/null @@ -1,129 +0,0 @@ -# -*- coding: utf-8 -*- -# -# RERO ILS -# Copyright (C) 2021 RERO -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published by -# the Free Software Foundation, version 3 of the License. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see . - -from copy import deepcopy - -import mock -from flask import url_for -from invenio_accounts.testutils import login_user_via_session -from utils import get_json - -from rero_ils.modules.operation_logs.api import OperationLog -from rero_ils.modules.operation_logs.permissions import OperationLogPermission -from rero_ils.modules.utils import get_ref_for_pid - - -def test_operation_logs_permissions_api( - client, document, patron_sion, - librarian_martigny): - """Test operation log permissions api.""" - oplg = OperationLog.get_record_by_pid('1') - - operation_log_permissions_url = url_for( - 'api_blueprint.permissions', - route_name='operation_logs' - ) - operation_log_martigny_permission_url = url_for( - 'api_blueprint.permissions', - route_name='operation_logs', - record_pid=oplg.pid - ) - - # Not logged - res = client.get(operation_log_permissions_url) - assert res.status_code == 401 - - # Logged as patron - login_user_via_session(client, patron_sion.user) - res = client.get(operation_log_permissions_url) - assert res.status_code == 403 - - # Logged as - login_user_via_session(client, librarian_martigny.user) - res = client.get(operation_log_martigny_permission_url) - assert res.status_code == 200 - data = get_json(res) - assert not data['create']['can'] - assert not data['delete']['can'] - assert data['list']['can'] - assert data['read']['can'] - assert not data['update']['can'] - - -def test_operation_logs_permissions(patron_martigny, org_martigny, - librarian_martigny, org_sion, - item_lib_martigny, - system_librarian_martigny): - """Test operation log permissions class.""" - - oplg = OperationLog.get_record_by_pid('1') - - # Anonymous user - assert not OperationLogPermission.list(None, {}) - assert not OperationLogPermission.read(None, {}) - assert not OperationLogPermission.create(None, {}) - assert not OperationLogPermission.update(None, {}) - assert not OperationLogPermission.delete(None, {}) - - # As non Librarian - assert not OperationLogPermission.list(None, oplg) - assert not OperationLogPermission.read(None, oplg) - assert not OperationLogPermission.create(None, oplg) - assert not OperationLogPermission.update(None, oplg) - assert not OperationLogPermission.delete(None, oplg) - - oplg_sion = deepcopy(oplg) - oplg_sion['organisation'] = {'$ref': get_ref_for_pid('org', org_sion.pid)} - oplg_sion = OperationLog.create( - oplg, dbcommit=True, reindex=True, delete_pid=True) - - # As Librarian - with mock.patch( - 'rero_ils.modules.operation_logs.permissions.current_librarian', - librarian_martigny - ): - assert OperationLogPermission.list(None, oplg) - assert OperationLogPermission.read(None, oplg) - assert not OperationLogPermission.create(None, oplg) - assert not OperationLogPermission.update(None, oplg) - assert not OperationLogPermission.delete(None, oplg) - - assert OperationLogPermission.read(None, oplg) - assert not OperationLogPermission.create(None, oplg) - assert not OperationLogPermission.update(None, oplg) - assert not OperationLogPermission.delete(None, oplg) - - assert OperationLogPermission.read(None, oplg_sion) - assert not OperationLogPermission.create(None, oplg_sion) - assert not OperationLogPermission.update(None, oplg_sion) - assert not OperationLogPermission.delete(None, oplg_sion) - - # # As System-librarian - # with mock.patch( - # 'rero_ils.modules.operation_logs.permissions.current_librarian', - # system_librarian_martigny - # ): - # assert not OperationLogPermission.list(None, oplg) - # assert not OperationLogPermission.read(None, oplg) - # assert not OperationLogPermission.create(None, oplg) - # assert not OperationLogPermission.update(None, oplg) - # assert not OperationLogPermission.delete(None, oplg) - - # assert not OperationLogPermission.read(None, oplg_sion) - # assert not OperationLogPermission.create(None, oplg_sion) - # assert not OperationLogPermission.update(None, oplg_sion) - # assert not OperationLogPermission.delete(None, oplg_sion) diff --git a/tests/api/operation_logs/test_operation_logs_rest.py b/tests/api/operation_logs/test_operation_logs_rest.py index df3d95e2a9..c33f13659e 100644 --- a/tests/api/operation_logs/test_operation_logs_rest.py +++ b/tests/api/operation_logs/test_operation_logs_rest.py @@ -17,29 +17,70 @@ """Tests REST API operation logs.""" -import mock +from flask import url_for from invenio_accounts.testutils import login_user_via_session +from utils import get_json, postdata -from rero_ils.modules.operation_logs.api import OperationLogsSearch -from rero_ils.modules.operation_logs.models import OperationLogOperation - - -def test_operation_log_entries(client, librarian_martigny, document): - """Test operation log entries after record update.""" - with mock.patch( - 'rero_ils.modules.operation_logs.listener.current_librarian', - librarian_martigny - ): - login_user_via_session(client, librarian_martigny.user) - print('_start_here') - document.update( - document, dbcommit=True, reindex=True) - search = OperationLogsSearch() - results = search.filter( - 'term', - operation=OperationLogOperation.UPDATE).filter( - 'term', record__pid=document.pid).filter( - 'term', user_name=librarian_martigny.formatted_name - ).source().count() - - assert results == 1 + +def test_operation_logs_permissions(client, operation_log, patron_sion, + librarian_martigny, json_header): + """Test record retrieval.""" + item_url = url_for('invenio_records_rest.oplg_item', pid_value='1') + item_list = url_for('invenio_records_rest.oplg_list') + + res = client.get(item_url) + assert res.status_code == 404 + + res = client.get(item_list) + assert res.status_code == 401 + + res, _ = postdata( + client, + 'invenio_records_rest.oplg_list', + {} + ) + assert res.status_code == 401 + + res = client.put( + url_for('invenio_records_rest.oplg_item', pid_value='1'), + data={}, + headers=json_header + ) + assert res.status_code == 404 + + res = client.delete(item_url) + assert res.status_code == 404 + + +def test_operation_logs_rest(client, loan_pending_martigny, + librarian_martigny, + json_header): + """Test record retrieval.""" + login_user_via_session(client, librarian_martigny.user) + item_url = url_for('invenio_records_rest.oplg_item', pid_value='1') + item_list = url_for('invenio_records_rest.oplg_list') + + res = client.get(item_url) + assert res.status_code == 404 + + res = client.get(item_list) + assert res.status_code == 200 + data = get_json(res) + assert data['hits']['total']['value'] > 0 + + res, _ = postdata( + client, + 'invenio_records_rest.oplg_list', + {} + ) + assert res.status_code == 403 + + res = client.put( + url_for('invenio_records_rest.oplg_item', pid_value='1'), + data={}, + headers=json_header + ) + assert res.status_code == 404 + + res = client.delete(item_url) + assert res.status_code == 404 diff --git a/tests/fixtures/metadata.py b/tests/fixtures/metadata.py index 3097909933..beff4a9aa1 100644 --- a/tests/fixtures/metadata.py +++ b/tests/fixtures/metadata.py @@ -31,6 +31,7 @@ from rero_ils.modules.holdings.api import Holding, HoldingsSearch from rero_ils.modules.items.api import Item, ItemsSearch from rero_ils.modules.local_fields.api import LocalField, LocalFieldsSearch +from rero_ils.modules.operation_logs.api import OperationLog from rero_ils.modules.templates.api import Template, TemplatesSearch @@ -1059,6 +1060,12 @@ def local_field_sion(app, org_martigny, document, local_field_sion_data): # --- OPERATION LOGS @pytest.fixture(scope="module") -def operation_log_1_data(data): +def operation_log_data(data): """Load operation log record.""" return deepcopy(data.get('oplg1')) + + +@pytest.fixture(scope="module") +def operation_log(operation_log_data, item_lib_sion): + """Load operation log record.""" + return OperationLog.create(operation_log_data) diff --git a/tests/ui/operation_logs/test_operation_logs_api.py b/tests/ui/operation_logs/test_operation_logs_api.py index 68fb034d27..e187d87c5a 100644 --- a/tests/ui/operation_logs/test_operation_logs_api.py +++ b/tests/ui/operation_logs/test_operation_logs_api.py @@ -17,35 +17,52 @@ """Operation logs Record tests.""" -from __future__ import absolute_import, print_function +from copy import deepcopy -from utils import flush_index, get_mapping +from invenio_search import current_search -from rero_ils.modules.operation_logs.api import OperationLog, \ - OperationLogsSearch -from rero_ils.modules.operation_logs.api import \ - operation_log_id_fetcher as fetcher -from rero_ils.modules.operation_logs.models import OperationLogOperation +from rero_ils.modules.operation_logs.api import OperationLog -def test_operation_logs_es_mapping(db, item_lib_sion, operation_log_1_data): - """Test operation logs elasticsearch mapping.""" - search = OperationLogsSearch() - mapping = get_mapping(search.Meta.index) - assert mapping - oplg = OperationLog.create(operation_log_1_data, dbcommit=True, - reindex=True, delete_pid=True) - flush_index(OperationLogsSearch.Meta.index) - assert mapping == get_mapping(search.Meta.index) +def test_operation_create(client, es_clear, operation_log_data): + """Test operation logs creation.""" + oplg = OperationLog.create(operation_log_data, index_refresh='wait_for') + assert oplg + assert oplg.id + # need to compare with dumps as it has resolve $refs + assert OperationLog.get_record(oplg.id) == \ + OperationLog(operation_log_data).dumps() + tmp = deepcopy(operation_log_data) + tmp['date'] = '2020-01-21T09:51:52.879533+00:00' + OperationLog.create(tmp, index_refresh='wait_for') + assert OperationLog.get_indices() == set(( + 'operation_logs-2020', + 'operation_logs-2021' + )) + # clean up the index + assert OperationLog.delete_indices() - assert oplg == operation_log_1_data - assert oplg.get('pid') == '7' - oplg = OperationLog.get_record_by_pid('7') - assert oplg == operation_log_1_data - - fetched_pid = fetcher(oplg.id, oplg) - assert fetched_pid.pid_value == '7' - assert fetched_pid.pid_type == 'oplg' - - assert oplg.get('operation') == OperationLogOperation.UPDATE +def test_operation_bulk_create(client, es_clear, operation_log_data): + """Test operation logs bulk creation.""" + data = [] + for date in [ + '2020-01-21T09:51:52.879533+00:00', + '2020-02-21T09:51:52.879533+00:00', + '2020-03-21T09:51:52.879533+00:00', + '2020-04-21T09:51:52.879533+00:00', + '2021-01-21T09:51:52.879533+00:00', + '2021-02-21T09:51:52.879533+00:00' + ]: + tmp = deepcopy(operation_log_data) + tmp['date'] = date + data.append(tmp) + OperationLog.bulk(data) + # flush the index for the test + current_search.flush_and_refresh(OperationLog.index_name) + assert OperationLog.get_indices() == set(( + 'operation_logs-2020', + 'operation_logs-2021' + )) + # clean up the index + assert OperationLog.delete_indices() diff --git a/tests/ui/operation_logs/test_operation_logs_jsonresolver.py b/tests/ui/operation_logs/test_operation_logs_jsonresolver.py deleted file mode 100644 index 50c720b51b..0000000000 --- a/tests/ui/operation_logs/test_operation_logs_jsonresolver.py +++ /dev/null @@ -1,47 +0,0 @@ -# -*- coding: utf-8 -*- -# -# RERO ILS -# Copyright (C) 2021 RERO -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as published by -# the Free Software Foundation, version 3 of the License. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU Affero General Public License for more details. -# -# You should have received a copy of the GNU Affero General Public License -# along with this program. If not, see . - -"""Operation Logs JSONResolver tests.""" - -import pytest -from invenio_records.api import Record -from jsonref import JsonRefError - -from rero_ils.modules.operation_logs.api import OperationLog - - -def test_operation_log_jsonresolver(item_lib_martigny): - """Test operation logs json resolver.""" - oplg = OperationLog.get_record_by_pid('1') - rec = Record.create({ - 'operation_log': {'$ref': 'https://ils.rero.ch/api/operation_logs/1'} - }) - assert rec.replace_refs().get('operation_log') == \ - {'pid': '1', 'type': 'oplg'} - - # deleted record - oplg.delete() - with pytest.raises(JsonRefError): - rec.replace_refs().dumps() - - # non existing record - rec = Record.create({ - 'operation_logs': { - '$ref': 'https://ils.rero.ch/api/operation_logs/n_e'} - }) - with pytest.raises(JsonRefError): - rec.replace_refs().dumps() diff --git a/tests/ui/operation_logs/test_operation_logs_mapping.py b/tests/ui/operation_logs/test_operation_logs_mapping.py index fe6c9900d5..ddd2f8c128 100644 --- a/tests/ui/operation_logs/test_operation_logs_mapping.py +++ b/tests/ui/operation_logs/test_operation_logs_mapping.py @@ -19,35 +19,13 @@ from utils import get_mapping -from rero_ils.modules.operation_logs.api import OperationLog, \ - OperationLogsSearch -from rero_ils.modules.operation_logs.models import OperationLogOperation +from rero_ils.modules.operation_logs.api import OperationLog -def test_operation_log_es_mapping(item_lib_sion, operation_log_1_data): +def test_operation_log_es_mapping(item_lib_sion, operation_log_data): """Test operation log elasticsearch mapping.""" - search = OperationLogsSearch() - mapping = get_mapping(search.Meta.index) + mapping = get_mapping(OperationLog.index_name) assert mapping OperationLog.create( - operation_log_1_data, - dbcommit=True, - reindex=True, - delete_pid=True) - assert mapping == get_mapping(search.Meta.index) - - count = search.query( - 'query_string', query=OperationLogOperation.CREATE - ).count() - assert count == 3 - - count = search.query( - 'query_string', query=OperationLogOperation.UPDATE - ).count() - assert count == 4 - - count = search.query( - 'match', - **{'user_name': 'updated_user'}).\ - count() - assert count == 1 + operation_log_data) + assert mapping == get_mapping(OperationLog.index_name) diff --git a/tests/ui/test_monitoring.py b/tests/ui/test_monitoring.py index c290e66820..a2bd73dfa4 100644 --- a/tests/ui/test_monitoring.py +++ b/tests/ui/test_monitoring.py @@ -48,7 +48,7 @@ def test_monitoring(app, document_sion_items_data, script_info): ' 0 loc 0 locations 0', ' 0 lofi 0 local_fields 0', ' 0 notif 0 notifications 0', - ' 0 oplg 2 operation_logs 2', + ' -2 oplg 0 operation_logs 2', ' 0 org 0 organisations 0', ' 0 ptre 0 patron_transaction_events 0', ' 0 ptrn 0 patrons 0', @@ -72,6 +72,8 @@ def test_monitoring(app, document_sion_items_data, script_info): assert mon.get_es_count('documents') == 0 assert mon.check() == {'doc': {'db_es': 1}} assert mon.missing('doc') == {'DB': [], 'ES': ['doc3'], 'ES duplicate': []} + # not flushed by default + flush_index('operation_logs') assert mon.info() == { 'acac': {'db': 0, 'db-es': 0, 'es': 0, 'index': 'acq_accounts'}, 'acin': {'db': 0, 'db-es': 0, 'es': 0, 'index': 'acq_invoices'}, @@ -91,7 +93,7 @@ def test_monitoring(app, document_sion_items_data, script_info): 'loc': {'db': 0, 'db-es': 0, 'es': 0, 'index': 'locations'}, 'lofi': {'db': 0, 'db-es': 0, 'es': 0, 'index': 'local_fields'}, 'notif': {'db': 0, 'db-es': 0, 'es': 0, 'index': 'notifications'}, - 'oplg': {'db': 2, 'db-es': 0, 'es': 2, 'index': 'operation_logs'}, + 'oplg': {'db': 0, 'db-es': -2, 'es': 2, 'index': 'operation_logs'}, 'org': {'db': 0, 'db-es': 0, 'es': 0, 'index': 'organisations'}, 'ptre': {'db': 0, 'db-es': 0, 'es': 0, 'index': 'patron_transaction_events'}, @@ -121,10 +123,10 @@ def test_monitoring(app, document_sion_items_data, script_info): doc.reindex() flush_index(DocumentsSearch.Meta.index) assert mon.get_es_count('documents') == 1 - assert mon.check() == {} + assert mon.check() == {'oplg': {'db_es': -2}} assert mon.missing('doc') == {'DB': [], 'ES': [], 'ES duplicate': []} doc.delete(dbcommit=True) assert mon.get_db_count('doc') == 0 assert mon.get_es_count('documents') == 1 - assert mon.check() == {'doc': {'db_es': -1}} + assert mon.check() == {'doc': {'db_es': -1}, 'oplg': {'db_es': -2}} assert mon.missing('doc') == {'DB': ['doc3'], 'ES': [], 'ES duplicate': []} diff --git a/tests/unit/test_operation_logs_jsonschema.py b/tests/unit/test_operation_logs_jsonschema.py index c999e07399..9b99fe6e6c 100644 --- a/tests/unit/test_operation_logs_jsonschema.py +++ b/tests/unit/test_operation_logs_jsonschema.py @@ -24,18 +24,18 @@ from jsonschema.exceptions import ValidationError -def test_required(operation_log_schema, operation_log_1_data): +def test_required(operation_log_schema, operation_log_data): """Test required for operation log jsonschemas.""" - validate(operation_log_1_data, operation_log_schema) + validate(operation_log_data, operation_log_schema) with pytest.raises(ValidationError): validate({}, operation_log_schema) def test_operation_log_all_jsonschema_keys_values( - operation_log_schema, operation_log_1_data): + operation_log_schema, operation_log_data): """Test all keys and values for operation log jsonschema.""" - record = operation_log_1_data + record = operation_log_data validate(record, operation_log_schema) validator = [ {'key': 'pid', 'value': 25},