diff --git a/b2share/modules/communities/models.py b/b2share/modules/communities/models.py index 1fa9a28544..1736511687 100644 --- a/b2share/modules/communities/models.py +++ b/b2share/modules/communities/models.py @@ -34,6 +34,7 @@ from invenio_accounts.models import Role from invenio_access.models import ActionRoles from invenio_oaiserver.models import OAISet +from b2share.utils import add_to_db class Community(db.Model, Timestamp): @@ -89,7 +90,20 @@ def _community_member_role_name(community): @event.listens_for(Community, 'after_insert') def receive_before_insert(mapper, connection, target): - """Create community admin and member roles and add their permissions.""" + """Add the necessary rows in the database when a community is created.""" + create_roles_and_permissions(target) + create_community_oaiset(target) + + +def create_roles_and_permissions(community, fix=False): + """Create community admin and member roles and add their permissions. + + :param community: the community for which we add the roles and + permissions. + :param fix: Enable the fixing of the database. This function doesn't fail + if some roles or permissions already exist. It will just add the + missing ones. + """ from b2share.modules.deposit.permissions import ( create_deposit_need_factory, read_deposit_need_factory, update_deposit_metadata_need_factory, @@ -103,60 +117,74 @@ def receive_before_insert(mapper, connection, target): assign_role_need_factory, search_accounts_need, ) admin_role = Role( - name=_community_admin_role_name(target), - description='Admin role of the community "{}"'.format(target.name) + name=_community_admin_role_name(community), + description='Admin role of the community "{}"'.format(community.name) ) member_role = Role( - name=_community_member_role_name(target), - description='Member role of the community "{}"'.format(target.name) + name=_community_member_role_name(community), + description='Member role of the community "{}"'.format(community.name) ) - - db.session.add(admin_role) - db.session.add(member_role) + # use a nested session so that the ids are generated by the DB. + with db.session.begin_nested(): + admin_role = add_to_db(admin_role, skip_if_exists=fix) + member_role = add_to_db(member_role, skip_if_exists=fix) member_needs = [ - create_deposit_need_factory(str(target.id)), + create_deposit_need_factory(str(community.id)), ] admin_needs = [ read_deposit_need_factory( - community=str(target.id), + community=str(community.id), publication_state=PublicationStates.submitted.name ), read_deposit_need_factory( - community=str(target.id), + community=str(community.id), publication_state=PublicationStates.published.name ), update_deposit_metadata_need_factory( - community=str(target.id), + community=str(community.id), publication_state=PublicationStates.submitted.name ), # permission to publish a submission update_deposit_publication_state_need_factory( - community=str(target.id), + community=str(community.id), old_state=PublicationStates.submitted.name, new_state=PublicationStates.published.name ), # permission to ask the owners to fix a submission before resubmitting update_deposit_publication_state_need_factory( - community=str(target.id), + community=str(community.id), old_state=PublicationStates.submitted.name, new_state=PublicationStates.draft.name ), # permission to update the metadata of a published record update_record_metadata_need_factory( - community=str(target.id), + community=str(community.id), ), # allow to assign any role owned by this community - assign_role_need_factory(community=target.id), + assign_role_need_factory(community=community.id), # allow to list users' accounts search_accounts_need, ] for need in member_needs: - db.session.add(ActionRoles.allow(need, role=member_role)) + add_to_db(ActionRoles.allow(need, role=member_role), + skip_if_exists=fix, + role_id=member_role.id) for need in chain (member_needs, admin_needs): - db.session.add(ActionRoles.allow(need, role=admin_role)) + add_to_db(ActionRoles.allow(need, role=admin_role), + skip_if_exists=fix, + role_id=admin_role.id) + - oaiset = OAISet(spec=str(target.id), name=target.name, description=target.description) - db.session.add(oaiset) +def create_community_oaiset(community, fix=False): + """Create a community's OAI set. + + :param community: the community for which we add the OAI set. + :param fix: Enable the fixing of the database. This function doesn't fail + if the oai set already exists. It will just add it if it is missing. + """ + oaiset = OAISet(spec=str(community.id), name=community.name, + description=community.description) + add_to_db(oaiset, skip_if_exists=fix) __all__ = ( diff --git a/b2share/modules/upgrade/upgrades/common.py b/b2share/modules/upgrade/upgrades/common.py index ff13de4039..347374be6a 100644 --- a/b2share/modules/upgrade/upgrades/common.py +++ b/b2share/modules/upgrade/upgrades/common.py @@ -26,6 +26,7 @@ from __future__ import absolute_import, print_function import amqp +import click from flask import current_app from invenio_search import current_search, current_search_client from invenio_pidstore.models import PersistentIdentifier, PIDStatus @@ -34,6 +35,8 @@ from invenio_queues.proxies import current_queues from celery.messaging import establish_connection from b2share.modules.schemas.helpers import load_root_schemas +from b2share.modules.communities.models import create_roles_and_permissions, \ + create_community_oaiset, Community def elasticsearch_index_destroy(alembic, verbose): @@ -86,3 +89,12 @@ def queues_declare(alembic, verbose): def schemas_init(alembic, verbose): """Load root schemas.""" load_root_schemas(cli=True, verbose=verbose) + + +def fix_communities(alembic, verbose): + """Fix communities by creating the missing permissions, roles, etc.""" + for community in Community.query.all(): + if verbose: + click.secho('Fixing community {}.'.format(community.name)) + create_roles_and_permissions(community, fix=True) + create_community_oaiset(community, fix=True) diff --git a/b2share/modules/upgrade/upgrades/upgrade_2_0_0_to_2_1_0.py b/b2share/modules/upgrade/upgrades/upgrade_2_0_0_to_2_1_0.py index 6fc302f4cf..a18f3c896e 100644 --- a/b2share/modules/upgrade/upgrades/upgrade_2_0_0_to_2_1_0.py +++ b/b2share/modules/upgrade/upgrades/upgrade_2_0_0_to_2_1_0.py @@ -39,7 +39,7 @@ from ..api import UpgradeRecipe, alembic_stamp, alembic_upgrade from .common import elasticsearch_index_destroy, elasticsearch_index_init, \ - elasticsearch_index_reindex, queues_declare, schemas_init + elasticsearch_index_reindex, queues_declare, schemas_init, fix_communities from b2share.modules.records.providers import RecordUUIDProvider from b2share.modules.deposit.providers import DepositUUIDProvider from b2share.modules.deposit.api import Deposit @@ -205,7 +205,7 @@ def migrate_record_metadata(record, parent_pid): record.commit() - -for step in [elasticsearch_index_destroy, elasticsearch_index_init, - elasticsearch_index_reindex, queues_declare]: +for step in [fix_communities, elasticsearch_index_destroy, + elasticsearch_index_init, elasticsearch_index_reindex, + queues_declare]: migrate_2_0_0_to_2_1_0.step()(step) diff --git a/b2share/utils.py b/b2share/utils.py new file mode 100644 index 0000000000..c87a79f881 --- /dev/null +++ b/b2share/utils.py @@ -0,0 +1,71 @@ +# -*- coding: utf-8 -*- +# +# This file is part of EUDAT B2Share. +# Copyright (C) 2017 CERN. +# +# B2Share is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of the +# License, or (at your option) any later version. +# +# B2Share 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 +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with B2Share; if not, write to the Free Software Foundation, Inc., +# 59 Temple Place, Suite 330, Boston, MA 02111-1307, USA. +# +# In applying this license, CERN does not +# waive the privileges and immunities granted to it by virtue of its status +# as an Intergovernmental Organization or submit itself to any jurisdiction. + +"""B2Share utility functions.""" + +from sqlalchemy import UniqueConstraint, PrimaryKeyConstraint +from invenio_db import db + + +def add_to_db(instance, skip_if_exists=False, **fields): + """Add a row to the database, optionally skip if it already exist. + + :param instance: the row to add to the database. + :param skip_if_exists: if true, check if the row exists before + inserting it. + :param fields: override fields during comparison. Some fields might be null + if the session is not flushed yet. + """ + if not skip_if_exists: + db.session.add(instance) + return instance + # Add only if the row does not already exist + clazz = instance.__class__ + table = instance.__table__ + cols = None + # Try retrieving the row using the first unique constraint + unique_constraints = [ + cst for cst in table.constraints if cst.__class__ == UniqueConstraint + ] + if unique_constraints: + cols = unique_constraints[0].columns + else: + # Otherwise use the first primary key constraint + primary_constraints = [ + cst for cst in table.constraints + if cst.__class__ == PrimaryKeyConstraint + ] + cols = primary_constraints[0].columns + # Retrieve the row + existing = db.session.query(clazz).filter( + *[getattr(clazz, col.name)==(fields.get(col.name) or + getattr(instance, col.name)) + for col in cols] + ).one_or_none() + + if existing is not None: + return existing + + # Add the row if it does not already exist + db.session.add(instance) + return instance diff --git a/tests/b2share_unit_tests/b2share_db_load_data.sql b/tests/b2share_unit_tests/b2share_db_load_data.sql index 51fd72e56f..291dad5cd4 100644 --- a/tests/b2share_unit_tests/b2share_db_load_data.sql +++ b/tests/b2share_unit_tests/b2share_db_load_data.sql @@ -33,7 +33,8 @@ SET search_path = public, pg_catalog; INSERT INTO accounts_role (id, name, description) VALUES (1, 'com:c4234f93da964d2fa2c8fa83d0775212:admin', 'Admin role of the community "Aalto"'); INSERT INTO accounts_role (id, name, description) VALUES (2, 'com:c4234f93da964d2fa2c8fa83d0775212:member', 'Member role of the community "Aalto"'); INSERT INTO accounts_role (id, name, description) VALUES (3, 'com:99916f6f9a2c4feba3426552ac7f1529:admin', 'Admin role of the community "BBMRI"'); -INSERT INTO accounts_role (id, name, description) VALUES (4, 'com:99916f6f9a2c4feba3426552ac7f1529:member', 'Member role of the community "BBMRI"'); +-- This is commented on purpose so that we can check that the permissions are correctly fixed if they are missing +-- INSERT INTO accounts_role (id, name, description) VALUES (4, 'com:99916f6f9a2c4feba3426552ac7f1529:member', 'Member role of the community "BBMRI"'); INSERT INTO accounts_role (id, name, description) VALUES (5, 'com:0afede872bf24d89867ed2ee57251c62:admin', 'Admin role of the community "CLARIN"'); INSERT INTO accounts_role (id, name, description) VALUES (6, 'com:0afede872bf24d89867ed2ee57251c62:member', 'Member role of the community "CLARIN"'); INSERT INTO accounts_role (id, name, description) VALUES (7, 'com:94a9567e2fba46778fdea8b68bdb63e8:admin', 'Admin role of the community "DRIHM"'); @@ -68,8 +69,9 @@ INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES (8, 'update-record-metadata', false, '{"community":"c4234f93-da96-4d2f-a2c8-fa83d0775212"}', 1); INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES (9, 'assign_role', false, '{"community":"c4234f93-da96-4d2f-a2c8-fa83d0775212","role":"None"}', 1); INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES (10, 'accounts-search', false, NULL, 1); -INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES (11, 'create-deposit', false, '{"community":"99916f6f-9a2c-4feb-a342-6552ac7f1529","publication_state":"draft"}', 4); -INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES (12, 'create-deposit', false, '{"community":"99916f6f-9a2c-4feb-a342-6552ac7f1529","publication_state":"draft"}', 3); +-- This is commented on purpose so that we can check that the permissions are correctly fixed if they are missing +-- INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES (11, 'create-deposit', false, '{"community":"99916f6f-9a2c-4feb-a342-6552ac7f1529","publication_state":"draft"}', 4); +-- INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES (12, 'create-deposit', false, '{"community":"99916f6f-9a2c-4feb-a342-6552ac7f1529","publication_state":"draft"}', 3); INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES (13, 'read-deposit', false, '{"community":"99916f6f-9a2c-4feb-a342-6552ac7f1529","publication_state":"submitted"}', 3); INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES (14, 'read-deposit', false, '{"community":"99916f6f-9a2c-4feb-a342-6552ac7f1529","publication_state":"published"}', 3); INSERT INTO access_actionsroles (id, action, exclude, argument, role_id) VALUES (15, 'update-deposit-metadata', false, '{"community":"99916f6f-9a2c-4feb-a342-6552ac7f1529","publication_state":"submitted"}', 3); diff --git a/tests/b2share_unit_tests/upgrade/helpers.py b/tests/b2share_unit_tests/upgrade/helpers.py index de2b03f897..2093a9b481 100644 --- a/tests/b2share_unit_tests/upgrade/helpers.py +++ b/tests/b2share_unit_tests/upgrade/helpers.py @@ -43,6 +43,8 @@ b2share_record_uuid_fetcher from b2share.modules.records.providers import RecordUUIDProvider from invenio_pidstore.errors import PIDDoesNotExistError +from invenio_accounts.models import Role +from invenio_access.models import ActionRoles def upgrade_run(app): @@ -165,6 +167,24 @@ def check_pids_migration(): assert parent.get_redirect() == rec_pid +def check_communities(): + """Check that the missing community permissions have been fixed. + + See the commented rows of Role and ActionRole in b2share_db_load_data.sql. + These are recreated by the upgrade. + """ + role = Role.query.filter( + Role.name == 'com:99916f6f9a2c4feba3426552ac7f1529:member' + ).one_or_none() + assert role is not None + action = ActionRoles.query.filter( + ActionRoles.argument == '{"community":"99916f6f-9a2c-4feb-a342-6552ac7f1529","publication_state":"draft"}', + ActionRoles.action == 'create-deposit', + ActionRoles.role_id == role.id + ).one_or_none() + assert action is not None + + def validate_loaded_data(app, alembic): """Checks that the loaded data is still there and valid.""" with app.app_context(): @@ -175,6 +195,7 @@ def validate_loaded_data(app, alembic): check_records_migration(app) check_pids_migration() + check_communities() def validate_database_schema(app, alembic):