model: Update role_id column

 * fix role instantiation
 * closes inveniosoftware/invenio-app-rdm#2186

Co-authored-by: jrcastro2 <[email protected]>

invenio_access/alembic/842a62b56e60_change_fk_accountsrole_to_string.py

@@ -0,0 +1,64 @@
+# This file is part of Invenio.
+# Copyright (C) 2016-2018 CERN.
+#
+# Invenio is free software; you can redistribute it and/or modify it
+# under the terms of the MIT License; see LICENSE file for more details.
+
+"""Change FK AccountsRole to string."""
+
+import sqlalchemy as sa
+from alembic import op
+from invenio_accounts.alembic.f2522cdd5fcd_change_accountsrole_primary_key_to_ import (
+    downgrade as accounts_downgrade,
+)
+
+# revision identifiers, used by Alembic.
+revision = "842a62b56e60"
+down_revision = "04480be1593e"
+branch_labels = ()
+depends_on = "f2522cdd5fcd"  # This is a revision from invenio-accounts, it's needed because in this recipe we re-create
+# a foreign key that uses a primary key (role_id) which type is modified in the pointed revision.
+
+
+def upgrade():
+    """Upgrade database."""
+    # ### commands auto generated by Alembic - please adjust! ###
+
+    op.alter_column(
+        "access_actionsroles",
+        "role_id",
+        existing_type=sa.Integer,
+        type_=sa.String(80),
+        postgresql_using="role_id::integer",
+    )
+    op.create_foreign_key(
+        op.f("fk_access_actionsroles_role_id_accounts_role"),
+        "access_actionsroles",
+        "accounts_role",
+        ["role_id"],
+        ["id"],
+        ondelete="CASCADE",
+    )
+
+
+def downgrade():
+    """Downgrade database."""
+    # When downgrading we need to make sure that first the downgrade of invenio-accounts is executed
+    # Since this depends on that revision, when downgrading it will execute this one first but this is wrong
+    # because in order to alter any foreign_key we first need to modify the primary key used (role_id)
+    accounts_downgrade()
+    op.alter_column(
+        "access_actionsroles",
+        "role_id",
+        existing_type=sa.String(80),
+        type_=sa.Integer,
+        postgresql_using="role_id::integer",
+    )
+    op.create_foreign_key(
+        op.f("fk_access_actionsroles_role_id_accounts_role"),
+        "access_actionsroles",
+        "accounts_role",
+        ["role_id"],
+        ["id"],
+        ondelete="CASCADE",
+    )

invenio_access/models.py

@@ -156,7 +156,7 @@ class ActionRoles(ActionNeedMixin, db.Model):
     )
 
     role_id = db.Column(
-        db.Integer(),
+        db.String(80),
         db.ForeignKey(Role.id, ondelete="CASCADE"),
         nullable=False,
         index=True,

invenio_access/translations/messages.pot

@@ -2,21 +2,21 @@
 # Copyright (C) 2022 CERN
 # This file is distributed under the same license as the invenio-access
 # project.
-# FIRST AUTHOR <EMAIL@ADDRESS>, 2022.
+# FIRST AUTHOR <EMAIL@ADDRESS>, 2023.
 #
 #, fuzzy
 msgid ""
 msgstr ""
-"Project-Id-Version: invenio-access 1.4.4\n"
+"Project-Id-Version: invenio-access 1.4.5\n"
 "Report-Msgid-Bugs-To: [email protected]\n"
-"POT-Creation-Date: 2022-10-12 09:03+0000\n"
+"POT-Creation-Date: 2023-04-28 15:49+0200\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[email protected]>\n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=utf-8\n"
 "Content-Transfer-Encoding: 8bit\n"
-"Generated-By: Babel 2.10.3\n"
+"Generated-By: Babel 2.12.1\n"
 
 #: invenio_access/admin.py:39
 msgid "Email"

run-tests.sh

@@ -18,12 +18,35 @@ set -o nounset
 function cleanup() {
     eval "$(docker-services-cli down --env)"
 }
-trap cleanup EXIT
+
+# Check for arguments
+# Note: "-k" would clash with "pytest"
+keep_services=0
+pytest_args=()
+for arg in $@; do
+	# from the CLI args, filter out some known values and forward the rest to "pytest"
+	# note: we don't use "getopts" here b/c of some limitations (e.g. long options),
+	#       which means that we can't combine short options (e.g. "./run-tests -Kk pattern")
+	case ${arg} in
+		-K|--keep-services)
+			keep_services=1
+			;;
+		*)
+			pytest_args+=( ${arg} )
+			;;
+	esac
+done
+
+if [[ ${keep_services} -eq 0 ]]; then
+	trap cleanup EXIT
+fi
 
 python -m check_manifest
 python -m setup extract_messages --dry-run
 python -m sphinx.cmd.build -qnNW docs docs/_build/html
 eval "$(docker-services-cli up --db ${DB:-postgresql} --cache ${CACHE:-redis} --env)"
-python -m pytest
+# Note: expansion of pytest_args looks like below to not cause an unbound
+# variable error when 1) "nounset" and 2) the array is empty.
+python -m pytest ${pytest_args[@]+"${pytest_args[@]}"}
 tests_exit_code=$?
 exit "$tests_exit_code"

setup.cfg

@@ -28,16 +28,16 @@ python_requires = >=3.7
 zip_safe = False
 install_requires =
     invenio-admin>=1.2.0
-    invenio-accounts>=1.4.13
+    invenio-accounts>=2.2.0
     invenio-base>=1.2.11
     invenio-i18n>=2.0.0
 
 [options.extras_require]
 tests =
     pytest-black>=0.3.0,<0.3.10
     cachelib>=0.1
-    pytest-invenio>=1.4.1
-    invenio-db[mysql, postgresql]>=1.0.14
+    pytest-invenio>=1.4.5
+    invenio-db[mysql, postgresql]>=1.1.2
     redis>=2.10.5
     sphinx>=4.5
 # Kept for backwards compatibility

tests/test_invenio_access.py

@@ -111,21 +111,6 @@ def test_entrypoints():
     assert SystemRoleNeed("authenticated_user") in ext.system_roles.values()
 
 
-def test_alembic(app):
+def test_alembic_recipes(app, test_alembic):
     """Test alembic recipes."""
-    ext = app.extensions["invenio-db"]
-
-    with app.app_context():
-        if db.engine.name == "sqlite":
-            raise pytest.skip("Upgrades are not supported on SQLite.")
-
-        assert not ext.alembic.compare_metadata()
-        db.drop_all()
-        drop_alembic_version_table()
-        ext.alembic.upgrade()
-
-        assert not ext.alembic.compare_metadata()
-        ext.alembic.downgrade(target="96e796392533")
-        ext.alembic.upgrade()
-
-        assert not ext.alembic.compare_metadata()
+    test_alembic(app, db, "invenio_access", downgrade_target="96e796392533")

tests/test_permissions.py

@@ -84,7 +84,7 @@ def create_roles(*names):
     """Helper to create roles."""
     roles = []
     for name in names:
-        role = Role(name=name)
+        role = Role(id=name, name=name)
         db.session.add(role)
         roles.append(role)
     db.session.commit()

tests/test_permissions_cache.py

@@ -147,7 +147,7 @@ def test_invenio_access_permission_cache_redis(app, dynamic_permission):
         )
 
 
-def test_intenio_access_cache_performance(app, dynamic_permission):
+def test_invenio_access_cache_performance(app, dynamic_permission):
     """Performance test simulating 1000 users."""
     InvenioAccess(app, cache=None)
     with app.test_request_context():
@@ -161,7 +161,7 @@ def test_intenio_access_cache_performance(app, dynamic_permission):
         roles = []
         actions = []
         for i in range(actions_roles_number):
-            role = Role(name="role{0}".format(i))
+            role = Role(id=str(i), name="role{0}".format(i))
             roles.append(role)
             db.session.add(role)
             db.session.flush()
@@ -472,12 +472,12 @@ def test_invenio_access_permission_cache_roles_updates(app, dynamic_permission):
     InvenioAccess(app, cache=cache)
     with app.test_request_context():
         # Creation of some data to test.
-        role_1 = Role(name="role_1")
-        role_2 = Role(name="role_2")
-        role_3 = Role(name="role_3")
-        role_4 = Role(name="role_4")
-        role_5 = Role(name="role_5")
-        role_6 = Role(name="role_6")
+        role_1 = Role(id="role_1", name="role_1")
+        role_2 = Role(id="role_2", name="role_2")
+        role_3 = Role(id="role_3", name="role_3")
+        role_4 = Role(id="role_4", name="role_4")
+        role_5 = Role(id="role_5", name="role_5")
+        role_6 = Role(id="role_6", name="role_6")
 
         db.session.add(role_1)
         db.session.add(role_2)