Merge pull request #1612 from nharraud/fix-communities

global: fix communities during upgrade

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__ = (

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)

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)

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

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);

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):