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_downgrade.py

@@ -0,0 +1,40 @@
+# 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 (downgrade recipe)."""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "842a62b56e60"
+down_revision = "04480be1593e"
+branch_labels = ()
+depends_on = None
+
+
+def upgrade():
+    """Upgrade database."""
+    pass
+
+
+def downgrade():
+    """Downgrade database."""
+    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/alembic/f9843093f686_change_fk_accountsrole_to_string_upgrade.py

@@ -0,0 +1,47 @@
+#
+# 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 (upgrade recipe).
+
+This recipe only contains the upgrade because as it directly depends on invenio-accounts recipe. That recipe is in
+charge of deleting all the constraints on the role_id, including foreign keys using the role_id declared in this module.
+Therefore, when in order to downgrade we need to split the recipes to be able to first execute the recipe in
+invenio-accounts (f2522cdd5fcd) and after that we can execute the downgrade recipe (842a62b56e60).
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "f9843093f686"
+down_revision = ("f2522cdd5fcd", "842a62b56e60")  # Depends on invenio-accounts revision id (f2522cdd5fcd)
+branch_labels = ()
+depends_on = None
+
+
+def upgrade():
+    """Upgrade database."""
+    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."""
+    pass

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,

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,7 +28,7 @@ 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
 
@@ -37,7 +37,7 @@ tests =
     pytest-black>=0.3.0,<0.3.10
     cachelib>=0.1
     pytest-invenio>=1.4.1
-    invenio-db[mysql, postgresql]>=1.0.14
+    invenio-db[mysql, postgresql]>=1.1.2
     redis>=2.10.5
     sphinx>=4.5
 # Kept for backwards compatibility

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)