From 15dbc78480b4d8676ba7c6afd5fb2b744cc5b5e6 Mon Sep 17 00:00:00 2001 From: Leonidas Triantafyllou Date: Wed, 25 Sep 2024 11:36:51 +0200 Subject: [PATCH 1/7] YDA-5892: delete revision avu for data objects in trash The job for revision creation now removes revision creation AVUs from data objects in trash. This makes it easier to monitor the number of data objects waiting for revision creation. --- revisions.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/revisions.py b/revisions.py index 07553652b..e1e3d4d8a 100644 --- a/revisions.py +++ b/revisions.py @@ -361,6 +361,10 @@ def rule_revision_batch(ctx, verbose, balance_id_min, balance_id_max, batch_size minimum_timestamp = int(time.time() - config.async_revision_delay_time) + # Remove revision creation AVUs from deleted data objects. + # This makes it easier to monitor the number of data objects waiting for revision creation. + remove_revision_creation_avu_from_deleted_data_objects(ctx, print_verbose) + # Get list of up to batch size limit of data objects (in research space) scheduled for revision, taking into account # modification time. log.write(ctx, "verbose = {}".format(verbose)) @@ -1054,3 +1058,28 @@ def memory_limit_exceeded(rss_limit): """ rss_limit = int(rss_limit) return rss_limit and memory_rss_usage() > rss_limit + + +def remove_revision_creation_avu_from_deleted_data_objects(ctx, print_verbose): + """ + Removes revision creation AVUs from deleted data objects [marked with 'org_revision_scheduled' metadata]. + + :param ctx: Combined type of a callback and rei struct + :param print_verbose: Whether to log verbose messages for troubleshooting (Boolean) + """ + revision_avu_name = constants.UUORGMETADATAPREFIX + "revision_scheduled" + + iter = genquery.row_iterator( + "COLL_NAME, DATA_NAME", + "COLL_NAME like '%{}/trash/home/%' AND META_DATA_ATTR_NAME = '{}'".format(user.zone(ctx), revision_avu_name), + genquery.AS_LIST, ctx + ) + + for coll_name, data_name in iter: + path = coll_name + '/' + data_name + try: + avu.rmw_from_data(ctx, path, revision_avu_name, "%") # use wildcard cause rm_from_data causes problems + if print_verbose: + log.write(ctx, 'Removed revision creation AVUs from data object: {}'.format(path)) + except Exception as e: + log.write(ctx, "Error processing data object {}: {}".format(path, str(e))) From d1bb8c8997394c198b68b0e4b7abfc5bfa2239df Mon Sep 17 00:00:00 2001 From: Leonidas Triantafyllou Date: Thu, 26 Sep 2024 09:45:19 +0200 Subject: [PATCH 2/7] YDA-5951 add transformation default-3 ISNI and Scopus ID The metadata schema transformation code from default-2 to default-3 transformed ORCID-IDs and Researcher IDs, but not Scopus IDs and ISNI IDs. Because of this, metadata files with the default-2 schema containing Scopus or ISNI IDs can usually not be converted automatically to default-3. Solution: Add a transformation function that handles transformation of an ISNI and Scopus ID that consists of a series of digits (with optional spaces) to the format that is specified in the default-3 schema --- schema_transformations.py | 89 +++++++++++++--------- schema_transformations_utils.py | 61 +++++++++++++++ unit-tests/test_schema_transformations.py | 93 +++++++++++++++++++++++ unit-tests/unit_tests.py | 4 + 4 files changed, 210 insertions(+), 37 deletions(-) create mode 100644 schema_transformations_utils.py create mode 100644 unit-tests/test_schema_transformations.py diff --git a/schema_transformations.py b/schema_transformations.py index af6d14ebb..5e6bd9ad9 100644 --- a/schema_transformations.py +++ b/schema_transformations.py @@ -6,6 +6,8 @@ import re +from schema_transformations_utils import correctify_isni, correctify_orcid, correctify_researcher_id, correctify_scopus + import meta from util import * @@ -128,21 +130,44 @@ def _default2_default3(ctx, m): person_identifiers = [] for person_identifier in creator.get('Person_Identifier', []): + # Check ORCID if person_identifier.get('Name_Identifier_Scheme', None) == 'ORCID': # Check for incorrect ORCID format. if not re.search("^(https://orcid.org/)[0-9]{4}-[0-9]{4}-[0-9]{4}-[0-9]{3}[0-9X]$", person_identifier.get('Name_Identifier', None)): corrected_orcid = correctify_orcid(person_identifier['Name_Identifier']) - # Only it an actual correction took place change the value and mark this data as 'changed'. + # Only if an actual correction took place change the value and mark this data as 'changed'. if corrected_orcid is None: log.write(ctx, "Warning: could not correct ORCID %s during schema transformation. It needs to be fixed manually." % (person_identifier['Name_Identifier'])) elif corrected_orcid != person_identifier['Name_Identifier']: person_identifier['Name_Identifier'] = corrected_orcid + # Check Scopus + elif person_identifier.get('Name_Identifier_Scheme', None) == 'Author identifier (Scopus)': + # Check for incorrect Scopus format. + if not re.search("^\d{1,11}$", person_identifier.get('Name_Identifier', None)): + corrected_scopus = correctify_scopus(person_identifier['Name_Identifier']) + # Only if an actual correction took place change the value and mark this data as 'changed'. + if corrected_scopus is None: + log.write(ctx, "Warning: could not correct Scopus %s during schema transformation. It needs to be fixed manually." + % (person_identifier['Name_Identifier'])) + elif corrected_scopus != person_identifier['Name_Identifier']: + person_identifier['Name_Identifier'] = corrected_scopus + # Check ISNI + elif person_identifier.get('Name_Identifier_Scheme', None) == 'ISNI': + # Check for incorrect ISNI format. + if not re.search("^(https://isni.org/isni/)[0-9]{15}[0-9X]$", person_identifier.get('Name_Identifier', None)): + corrected_isni = correctify_isni(person_identifier['Name_Identifier']) + # Only if an actual correction took place change the value and mark this data as 'changed'. + if corrected_isni is None: + log.write(ctx, "Warning: could not correct ISNI %s during schema transformation. It needs to be fixed manually." + % (person_identifier['Name_Identifier'])) + elif corrected_isni != person_identifier['Name_Identifier']: + person_identifier['Name_Identifier'] = corrected_isni elif person_identifier.get('Name_Identifier_Scheme', None) == 'ResearcherID (Web of Science)': # Check for incorrect ResearcherID format. if not re.search("^(https://www.researcherid.com/rid/)[A-Z]-[0-9]{4}-[0-9]{4}$", person_identifier.get('Name_Identifier', None)): corrected_researcher_id = correctify_researcher_id(person_identifier['Name_Identifier']) - # Only it an actual correction took place change the value and mark this data as 'changed'. + # Only if an actual correction took place change the value and mark this data as 'changed'. if corrected_researcher_id != person_identifier['Name_Identifier']: person_identifier['Name_Identifier'] = corrected_researcher_id elif 'Name_Identifier_Scheme' not in person_identifier: @@ -164,21 +189,44 @@ def _default2_default3(ctx, m): person_identifiers = [] for person_identifier in contributor.get('Person_Identifier', []): + # Check ORCID if person_identifier.get('Name_Identifier_Scheme', None) == 'ORCID': # Check for incorrect ORCID format. if not re.search("^(https://orcid.org/)[0-9]{4}-[0-9]{4}-[0-9]{4}-[0-9]{3}[0-9X]$", person_identifier.get('Name_Identifier', None)): corrected_orcid = correctify_orcid(person_identifier['Name_Identifier']) - # Only it an actual correction took place change the value and mark this data as 'changed'. + # Only if an actual correction took place change the value and mark this data as 'changed'. if corrected_orcid is None: log.write(ctx, "Warning: could not correct ORCID %s during schema transformation. It needs to be fixed manually." % (person_identifier['Name_Identifier'])) elif corrected_orcid != person_identifier['Name_Identifier']: person_identifier['Name_Identifier'] = corrected_orcid + # Check Scopus + elif person_identifier.get('Name_Identifier_Scheme', None) == 'Author identifier (Scopus)': + # Check for incorrect Scopus format. + if not re.search("^\d{1,11}$", person_identifier.get('Name_Identifier', None)): + corrected_scopus = correctify_scopus(person_identifier['Name_Identifier']) + # Only if an actual correction took place change the value and mark this data as 'changed'. + if corrected_scopus is None: + log.write(ctx, "Warning: could not correct Scopus %s during schema transformation. It needs to be fixed manually." + % (person_identifier['Name_Identifier'])) + elif corrected_scopus != person_identifier['Name_Identifier']: + person_identifier['Name_Identifier'] = corrected_scopus + # Check ISNI + elif person_identifier.get('Name_Identifier_Scheme', None) == 'ISNI': + # Check for incorrect ISNI format. + if not re.search("^(https://isni.org/isni/)[0-9]{15}[0-9X]$", person_identifier.get('Name_Identifier', None)): + corrected_isni = correctify_isni(person_identifier['Name_Identifier']) + # Only if an actual correction took place change the value and mark this data as 'changed'. + if corrected_isni is None: + log.write(ctx, "Warning: could not correct ISNI %s during schema transformation. It needs to be fixed manually." + % (person_identifier['Name_Identifier'])) + elif corrected_isni != person_identifier['Name_Identifier']: + person_identifier['Name_Identifier'] = corrected_isni elif person_identifier.get('Name_Identifier_Scheme', None) == 'ResearcherID (Web of Science)': # Check for incorrect ResearcherID format. if not re.search("^(https://www.researcherid.com/rid/)[A-Z]-[0-9]{4}-[0-9]{4}$", person_identifier.get('Name_Identifier', None)): corrected_researcher_id = correctify_researcher_id(person_identifier['Name_Identifier']) - # Only it an actual correction took place change the value and mark this data as 'changed'. + # Only if an actual correction took place change the value and mark this data as 'changed'. if corrected_researcher_id != person_identifier['Name_Identifier']: person_identifier['Name_Identifier'] = corrected_researcher_id elif 'Name_Identifier_Scheme' not in person_identifier: @@ -702,36 +750,3 @@ def get(src_id, dst_id): x = transformations.get(src_id) return None if x is None else x.get(dst_id) - - -def correctify_orcid(org_orcid): - """Correct illformatted ORCID.""" - # Get rid of all spaces. - orcid = org_orcid.replace(' ', '') - - # Upper-case X. - orcid = org_orcid.replace('x', 'X') - - # The last part should hold a valid id like eg: 1234-1234-1234-123X. - # If not, it is impossible to correct it to the valid orcid format - orcs = orcid.split('/') - if not re.search("^[0-9]{4}-[0-9]{4}-[0-9]{4}-[0-9]{3}[0-9X]$", orcs[-1]): - # Return original value. - return org_orcid - - return "https://orcid.org/{}".format(orcs[-1]) - - -def correctify_researcher_id(org_researcher_id): - """Correct illformatted ResearcherID.""" - # Get rid of all spaces. - researcher_id = org_researcher_id.replace(' ', '') - - # The last part should hold a valid id like eg: A-1234-1234 - # If not, it is impossible to correct it to the valid ResearcherID format - orcs = researcher_id.split('/') - if not re.search("^[A-Z]-[0-9]{4}-[0-9]{4}$", orcs[-1]): - # Return original value. - return org_researcher_id - - return "https://www.researcherid.com/rid/{}".format(orcs[-1]) diff --git a/schema_transformations_utils.py b/schema_transformations_utils.py new file mode 100644 index 000000000..34904a39a --- /dev/null +++ b/schema_transformations_utils.py @@ -0,0 +1,61 @@ +import re + + +def correctify_orcid(org_orcid): + """Correct illformatted ORCID.""" + # Get rid of all spaces. + orcid = org_orcid.replace(' ', '') + + # Upper-case X. + orcid = orcid.replace('x', 'X') + + # The last part should hold a valid id like eg: 1234-1234-1234-123X. + # If not, it is impossible to correct it to the valid orcid format + orcs = orcid.split('/') + if not re.search("^[0-9]{4}-[0-9]{4}-[0-9]{4}-[0-9]{3}[0-9X]$", orcs[-1]): + return None + + return "https://orcid.org/{}".format(orcs[-1]) + + +def correctify_scopus(org_scopus): + """Correct illformatted Scopus.""" + # Get rid of all spaces. + new_scopus = org_scopus.replace(' ', '') + + if not re.search("^\d{1,11}$", new_scopus): + return None + + return new_scopus + + +def correctify_isni(org_isni): + """Correct ill-formatted ISNI.""" + # Remove all spaces. + new_isni = org_isni.replace(' ', '') + + # Upper-case X. + new_isni = new_isni.replace('x', 'X') + + # The last part should hold a valid id like eg: 123412341234123X. + # If not, it is impossible to correct it to the valid isni format + new_isni = new_isni.split('/') + if not re.search("^[0-9]{15}[0-9X]$", new_isni[-1]): + return None + + return "https://isni.org/isni/{}".format(new_isni[-1]) + + +def correctify_researcher_id(org_researcher_id): + """Correct illformatted ResearcherID.""" + # Get rid of all spaces. + researcher_id = org_researcher_id.replace(' ', '') + + # The last part should hold a valid id like eg: A-1234-1234 + # If not, it is impossible to correct it to the valid ResearcherID format + orcs = researcher_id.split('/') + if not re.search("^[A-Z]-[0-9]{4}-[0-9]{4}$", orcs[-1]): + # Return original value. + return org_researcher_id + + return "https://www.researcherid.com/rid/{}".format(orcs[-1]) diff --git a/unit-tests/test_schema_transformations.py b/unit-tests/test_schema_transformations.py new file mode 100644 index 000000000..d273365ca --- /dev/null +++ b/unit-tests/test_schema_transformations.py @@ -0,0 +1,93 @@ +# -*- coding: utf-8 -*- +"""Unit tests for the correctify functions in schema_transformations""" + +__copyright__ = 'Copyright (c) 2024, Utrecht University' +__license__ = 'GPLv3, see LICENSE' + +import sys +from unittest import TestCase + +sys.path.append('..') + +from schema_transformations_utils import correctify_isni, correctify_orcid, correctify_scopus + + +class CorrectifyIsniTest(TestCase): + def test_isni_correct_format(self): + """Test ISNI with correct format""" + isni = "https://isni.org/isni/1234123412341234" + self.assertEqual(correctify_isni(isni), isni) + + def test_isni_correct_format_containing_x(self): + """Test ISNI with correct format""" + isni = "https://isni.org/isni/123412341234123x" + correct_isni = "https://isni.org/isni/123412341234123X" + self.assertEqual(correctify_isni(isni), correct_isni) + + def test_isni_invalid_format(self): + """Test ISNI with invalid format (1 less number)""" + isni = "123412341234123" + self.assertIsNone(correctify_isni(isni)) + + def test_isni_malformed_format(self): + """Test ISNI with invalid format""" + isni = "foobar0123456789" + self.assertIsNone(correctify_isni(isni)) + + def test_isni_with_spaces(self): + """Test ISNI that contains spaces and should be corrected""" + isni = " https://isni.org/isni/123412341234123x " + corrected_isni = "https://isni.org/isni/123412341234123X" + self.assertEqual(correctify_isni(isni), corrected_isni) + + +class CorrectifyOrcidTest(TestCase): + def test_orcid_correct_format(self): + """Test ORCID with correct format""" + orcid = "https://orcid.org/1234-1234-1234-1234" + self.assertEqual(correctify_orcid(orcid), orcid) + + def test_orcid_correct_format_containing_x(self): + """Test ORCID with correct format""" + orcid = "https://orcid.org/1234-1234-1234-123x" + correct_orcid = "https://orcid.org/1234-1234-1234-123X" + self.assertEqual(correctify_orcid(orcid), correct_orcid) + + def test_orcid_invalid_format(self): + """Test ORCID with invalid format (1 less number)""" + orcid = "1234-1234-1234-123" + self.assertIsNone(correctify_orcid(orcid)) + + def test_orcid_malformed_format(self): + """Test ORCID with invalid format""" + orcid = "1234-foo-bar-1234" + self.assertIsNone(correctify_orcid(orcid)) + + def test_orcid_with_spaces(self): + """Test ORCID that contains spaces and should be corrected""" + orcid = " https://orcid.org/1234-1234-1234-123x " + corrected_orcid = "https://orcid.org/1234-1234-1234-123X" + self.assertEqual(correctify_orcid(orcid), corrected_orcid) + + +class CorrectifyScopusTest(TestCase): + def test_correctify_format(self): + """Test SCOPUS with correct format""" + scopus = "12345678901" + self.assertEqual(correctify_scopus(scopus), scopus) + + def test_correctify_invalid_format(self): + """Test SCOPUS with invalid format""" + scopus = "123456789012" + self.assertIsNone(correctify_scopus(scopus)) + + def test_malformed_format(self): + """Test SCOPUS with invalid format""" + scopus = "foobar1234" + self.assertIsNone(correctify_scopus(scopus)) + + def test_orcid_with_spaces(self): + """Test SCOPUS that contains spaces and should be corrected""" + scopus = " 01234567890 " + corrected_scopus = "01234567890" + self.assertEqual(correctify_scopus(scopus), corrected_scopus) diff --git a/unit-tests/unit_tests.py b/unit-tests/unit_tests.py index a008c8607..3bd9d873e 100644 --- a/unit-tests/unit_tests.py +++ b/unit-tests/unit_tests.py @@ -9,6 +9,7 @@ from test_intake import IntakeTest from test_policies import PoliciesTest from test_revisions import RevisionTest +from test_schema_transformations import CorrectifyIsniTest, CorrectifyOrcidTest, CorrectifyScopusTest from test_util_misc import UtilMiscTest from test_util_pathutil import UtilPathutilTest from test_util_yoda_names import UtilYodaNamesTest @@ -16,6 +17,9 @@ def suite(): test_suite = TestSuite() + test_suite.addTest(makeSuite(CorrectifyIsniTest)) + test_suite.addTest(makeSuite(CorrectifyOrcidTest)) + test_suite.addTest(makeSuite(CorrectifyScopusTest)) test_suite.addTest(makeSuite(GroupImportTest)) test_suite.addTest(makeSuite(IntakeTest)) test_suite.addTest(makeSuite(PoliciesTest)) From 2c46936d4bf2ba971ad16d1d839153cf6fb52562 Mon Sep 17 00:00:00 2001 From: Sietse Snel Date: Fri, 27 Sep 2024 16:53:47 +0200 Subject: [PATCH 3/7] CI: update jobs for Yoda v1.10 --- .github/workflows/api-and-integration-tests.yml | 3 +++ .github/workflows/build-push-image.yml | 1 + 2 files changed, 4 insertions(+) diff --git a/.github/workflows/api-and-integration-tests.yml b/.github/workflows/api-and-integration-tests.yml index 3c54adc84..bdedd570f 100644 --- a/.github/workflows/api-and-integration-tests.yml +++ b/.github/workflows/api-and-integration-tests.yml @@ -5,6 +5,7 @@ on: branches: - development - release-1.9 + - release-1.10 - "**-atr" # We can force an integration/API test run without opening a PR by pushing to a branch name that ends with "-atr" pull_request: @@ -38,6 +39,8 @@ jobs: run: | if [ "${{ steps.extract_branch.outputs.branch }}" = "release-1.9" ]; then echo "branch=release-1.9" >> $GITHUB_OUTPUT + elif [ "${{ steps.extract_branch.outputs.branch }}" = "release-1.10" ]; then + echo "branch=release-1.10" >> $GITHUB_OUTPUT else echo "branch=development" >> $GITHUB_OUTPUT fi diff --git a/.github/workflows/build-push-image.yml b/.github/workflows/build-push-image.yml index d0a692e42..8b987d1bc 100644 --- a/.github/workflows/build-push-image.yml +++ b/.github/workflows/build-push-image.yml @@ -6,6 +6,7 @@ on: branches: - 'development' - 'release-1.9' + - 'release-1.10' jobs: push-image: From 39a519fe1aa214cdb674bd24f2536fa4c6f9e53e Mon Sep 17 00:00:00 2001 From: Leonidas Triantafyllou Date: Tue, 1 Oct 2024 08:20:04 +0200 Subject: [PATCH 4/7] YDA-5942: improve messages CSV group import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem When modifying groups with a CSV, I use groups that are already created (to add users for example) . I check the “Allow updates” button and “Process CSV”. I then get an error for existing groups even though the users have been added Solution When import groups using a CSV file, now the user can be informed with a simple, but useful descriptive message of the actions taken: A message may consist of 1 or more of the following phrases: Group '' created. Group '' already exists. Users added (). Users removed (). --- groups.py | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/groups.py b/groups.py index f189bfe7a..bb49024cb 100644 --- a/groups.py +++ b/groups.py @@ -529,11 +529,11 @@ def api_group_process_csv(ctx, csv_header_and_data, allow_update, delete_users): return api.Error('errors', validation_errors) # Step 3: Create / update groups. - error = apply_data(ctx, data, allow_update, delete_users) - if len(error): - return api.Error('errors', [error]) + status_msg = apply_data(ctx, data, allow_update, delete_users) + if status_msg['status'] == 'error': + return api.Error('errors', [status_msg['message']]) - return api.Result.ok() + return api.Result.ok(info=[status_msg['message']]) def validate_data(ctx, data, allow_update): @@ -553,7 +553,7 @@ def validate_data(ctx, data, allow_update): for (category, subcategory, groupname, _managers, _members, _viewers, _schema_id, _expiration_date) in data: if group.exists(ctx, groupname) and not allow_update: - errors.append('Group "{}" already exists'.format(groupname)) + errors.append('Group "{}" already exists. It has not been updated.'.format(groupname)) # Is user admin or has category add privileges? if not (is_admin or can_add_category): @@ -575,11 +575,13 @@ def apply_data(ctx, data, allow_update, delete_users): :param allow_update: Allow updates in groups :param delete_users: Allow for deleting of users from groups - :returns: Errors if found any + :returns: Errors if found any, or message with actions if everything is successful """ for (category, subcategory, group_name, managers, members, viewers, schema_id, expiration_date) in data: new_group = False + users_added, users_removed = 0, 0 + message = '' log.write(ctx, 'CSV import - Adding and updating group: {}'.format(group_name)) @@ -590,10 +592,12 @@ def apply_data(ctx, data, allow_update, delete_users): if response: new_group = True + message += "Group '{}' created.".format(group_name) elif response.status == "error_group_exists" and allow_update: log.write(ctx, 'CSV import - WARNING: group "{}" not created, it already exists'.format(group_name)) + message += "Group '{}' already exists.".format(group_name) else: - return "Error while attempting to create group {}. Status/message: {} / {}".format(group_name, response.status, response.status_info) + return {status: 'error', message: "Error while attempting to create group {}. Status/message: {} / {}".format(group_name, response.status, response.status_info)} # Now add the users and set their role if other than member allusers = managers + members + viewers @@ -604,6 +608,7 @@ def apply_data(ctx, data, allow_update, delete_users): if response: currentrole = "normal" log.write(ctx, "CSV import - Notice: added user {} to group {}".format(username, group_name)) + users_added += 1 else: log.write(ctx, "CSV import - Warning: error occurred while attempting to add user {} to group {}".format(username, group_name)) log.write(ctx, "CSV import - Status: {} , Message: {}".format(response.status, response.status_info)) @@ -669,11 +674,21 @@ def apply_data(ctx, data, allow_update, delete_users): response = group_remove_user_from_group(ctx, username, usergroupname) if response: log.write(ctx, "CSV import - Removing user {} from group {}".format(username, usergroupname)) + users_removed += 1 else: log.write(ctx, "CSV import - Warning: error while attempting to remove user {} from group {}".format(username, usergroupname)) log.write(ctx, "CSV import - Status: {} , Message: {}".format(response.status, response.status_info)) - return '' + if users_added > 0: + message += ' Users added ({}).'.format(users_added) + if users_removed > 0: + message += ' Users removed ({}).'.format(users_removed) + + # If no users added, no users removed and not new group created. + if not users_added and not users_removed and not new_group: + message += ' No changes made.' + + return {"status": "ok", "message": message} def _are_roles_equivalent(a, b): @@ -973,12 +988,15 @@ def group_create(ctx, group_name, category, subcategory, schema_id, expiration_d if not sram.sram_connect_service_collaboration(ctx, short_name): return api.Error('sram_error', 'Something went wrong connecting service to group "{}" in SRAM'.format(group_name)) + if group.exists(ctx, group_name): + return api.Error('group_exists', "Group {} not created, it already exists".format(group_name)) + response = ctx.uuGroupAdd(group_name, category, subcategory, schema_id, expiration_date, description, data_classification, co_identifier, '', '')['arguments'] status = response[8] message = response[9] if status == '0': return api.Result.ok() - elif status == '-1089000' or status == '-809000': + elif status == '-1089000' or status == '-809000' or status == '-806000': return api.Error('group_exists', "Group {} not created, it already exists".format(group_name)) else: return api.Error('policy_error', message) From b42a8e833c43620bb95b92688f45a259f513f37e Mon Sep 17 00:00:00 2001 From: Lazlo Westerhof Date: Tue, 1 Oct 2024 13:16:03 +0200 Subject: [PATCH 5/7] Schema transformation utils: add missing description and copyright notice --- schema_transformations_utils.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/schema_transformations_utils.py b/schema_transformations_utils.py index 34904a39a..d5cf58f68 100644 --- a/schema_transformations_utils.py +++ b/schema_transformations_utils.py @@ -1,3 +1,9 @@ +# -*- coding: utf-8 -*- +"""JSON schema transformation utility functions.""" + +__copyright__ = 'Copyright (c) 2024, Utrecht University' +__license__ = 'GPLv3, see LICENSE' + import re From 02fef147edac6fba22835ed29c7d13065fb961e7 Mon Sep 17 00:00:00 2001 From: Lazlo Westerhof Date: Wed, 2 Oct 2024 14:36:03 +0200 Subject: [PATCH 6/7] Update pytest-bdd to 7.3.0 and remove workaround --- tests/features/api/api_deposit_open.feature | 2 +- tests/features/api/api_deposit_restricted.feature | 2 +- tests/requirements.txt | 2 +- tests/step_defs/api/test_api_deposit.py | 11 +++++------ 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/features/api/api_deposit_open.feature b/tests/features/api/api_deposit_open.feature index a120c5c6a..8782e7f2d 100644 --- a/tests/features/api/api_deposit_open.feature +++ b/tests/features/api/api_deposit_open.feature @@ -60,7 +60,7 @@ Feature: Deposit API (open) And deposit exists And deposit is archived And user viewer is authenticated - And as viewer the Yoda browse collections API is queried with # Workaround for https://github.com/pytest-dev/pytest-bdd/issues/689 + And the Yoda browse collections API is queried with Then the response status code is "200" And the browse result contains deposit diff --git a/tests/features/api/api_deposit_restricted.feature b/tests/features/api/api_deposit_restricted.feature index 3155de18e..573957e74 100644 --- a/tests/features/api/api_deposit_restricted.feature +++ b/tests/features/api/api_deposit_restricted.feature @@ -49,7 +49,7 @@ Feature: Deposit API (restricted) And deposit exists And deposit is archived And user viewer is authenticated - And as viewer the Yoda browse collections API is queried with # Workaround for https://github.com/pytest-dev/pytest-bdd/issues/689 + And the Yoda browse collections API is queried with Then the response status code is "200" And the browse result does not contain deposit diff --git a/tests/requirements.txt b/tests/requirements.txt index ca93ffea6..6caab6ffa 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -3,7 +3,7 @@ requests==2.32.2 selenium==4.21.0 splinter==0.21.0 pytest-splinter==3.3.2 -pytest_bdd==7.2.0 +pytest_bdd==7.3.0 pytest==8.2.2 deepdiff==6.6.1 pyperclip==1.9.0 diff --git a/tests/step_defs/api/test_api_deposit.py b/tests/step_defs/api/test_api_deposit.py index e4665cb6e..621dcc25c 100644 --- a/tests/step_defs/api/test_api_deposit.py +++ b/tests/step_defs/api/test_api_deposit.py @@ -143,13 +143,12 @@ def data_access_restriction_restricted(user, deposit_name): ) -# Workaround for https://github.com/pytest-dev/pytest-bdd/issues/689 -@given(parsers.parse("as viewer the Yoda browse collections API is queried with {collection}"), target_fixture="api_response") -def api_browse_folder(collection): +@given(parsers.parse("the Yoda browse collections API is queried with {collection}"), target_fixture="api_response") +def api_browse_collections(user, collection): return api_request( - "viewer", - "browse_folder", - {"coll": collection} + user, + "browse_collections", + {"coll": collection, "sort_order": "desc"} ) From 4536c699baf1c4ad3ac4268277e9338b1c11dd06 Mon Sep 17 00:00:00 2001 From: Lazlo Westerhof Date: Fri, 4 Oct 2024 09:25:10 +0200 Subject: [PATCH 7/7] YDA-5956: remove unnecessary try-except blocks --- groups.py | 48 ++++++++++++++++-------------------------------- 1 file changed, 16 insertions(+), 32 deletions(-) diff --git a/groups.py b/groups.py index bb49024cb..0a399db6a 100644 --- a/groups.py +++ b/groups.py @@ -56,18 +56,13 @@ def getGroupsData(ctx): attr = row[1] value = row[2] - # Create/update group with this information. - try: - group = groups[name] - except Exception: - group = { - "name": name, - "managers": [], - "members": [], - "read": [], - "invited": [] - } - groups[name] = group + group = groups.setdefault(name, { + "name": name, + "managers": [], + "members": [], + "read": [], + "invited": [] + }) if attr in ["schema_id", "data_classification", "category", "subcategory"]: group[attr] = value @@ -95,26 +90,17 @@ def getGroupsData(ctx): if name.startswith("read-"): # Match read-* group with research-* or initial-* group. name = name[5:] - try: - # Attempt to add to read list of research group. - group = groups["research-" + name] - group["read"].append(user) - except Exception: - try: - # Attempt to add to read list of initial group. - group = groups["initial-" + name] + for prefix in ("research-", "initial-"): + group = groups.get(prefix + name) + if group: group["read"].append(user) - except Exception: - pass + break elif not name.startswith("vault-"): - try: - # Ordinary group. - group = groups[name] + group = groups.get(name) + if group: group["members"].append(user) - except KeyError: - pass - # Third query: obtain list of invited SRAM users + # Third query: obtain list of invited SRAM users. if config.enable_sram: iter = genquery.row_iterator( "META_USER_ATTR_VALUE, USER_NAME, USER_ZONE", @@ -124,11 +110,9 @@ def getGroupsData(ctx): for row in iter: name = row[0] user = row[1] + "#" + row[2] - try: - group = groups[name] + group = groups.get(name) + if group: group["invited"].append(user) - except KeyError: - pass return groups.values()