Hi. This is a proposal to add extra field support in the API.
I am not sure if this fits the project's policy, as the content was changed for personal purposes. If it doesn't fit, you can reject it. There are two changes. 01-api update extra field by update_repo.patch This is a change to accept updates to extra fields at the updat_repo endpoint. I recently found that the model already supports updates. However, the API interface was not accepted, so this is to add parameters for it. A point for a little consideration is the method of specifying key names for extra fields. For example, the get_repo API uses a flat hierarchy of properties with an 'ex_' prefix to retrieve information. This proposed change is not symmetrical with it. However, I believe this format is more convenient. 02-api add endpoint for extra fields.patch It adds endpoints for retrieving, creating, and deleting extra field definitions in the repository. I am not sure if the way db is referenced here, etc., is appropriate for the policy. In both of the above, I included methods for testing. However, we do not know if this way of creating the test is also appropriate. Thanks -- toras9000
# HG changeset patch # User toras9000 <[email protected]> # Date 1691408559 -32400 # Mon Aug 07 20:42:39 2023 +0900 # Branch stable # Node ID 6f59d9f02044092bac1993879dcea4eece45d049 # Parent 36a36ebdf4bbc4da77c41cabdbdf4a688e8fbeea api: update extra field by update_repo diff --git a/kallithea/controllers/api/api.py b/kallithea/controllers/api/api.py --- a/kallithea/controllers/api/api.py +++ b/kallithea/controllers/api/api.py @@ -1017,7 +1017,8 @@ description=None, private=None, clone_uri=None, landing_rev=None, enable_statistics=None, - enable_downloads=None): + enable_downloads=None, + extra_fields=None): """ Updates repo """ @@ -1037,6 +1038,11 @@ 'Only Kallithea admin can specify `owner` param' ) + if extra_fields is not None: + ex_field_setting = db.Setting.get_by_name('repository_fields') + if (ex_field_setting is None) or (not ex_field_setting.app_settings_value): + raise JSONRPCError('Extra field setting is disabled.') + updates = {} repo_group = group if repo_group is not None: @@ -1056,6 +1062,11 @@ store_update(updates, enable_statistics, 'repo_enable_statistics') store_update(updates, enable_downloads, 'repo_enable_downloads') + if isinstance(extra_fields, dict): + for key, val in extra_fields.items(): + if (key is not None) and (val is not None): + store_update(updates, val, db.RepositoryField.PREFIX + key) + RepoModel().update(repo, **updates) meta.Session().commit() return dict( diff --git a/kallithea/tests/api/api_base.py b/kallithea/tests/api/api_base.py --- a/kallithea/tests/api/api_base.py +++ b/kallithea/tests/api/api_base.py @@ -1243,6 +1243,72 @@ finally: fixture.destroy_repo(repo_name) + def test_api_update_repo_extra_field_change(self): + repo_name = 'admin_owned' + repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE) + try: + RepoModel().grant_user_permission(repo=repo_name, user=self.TEST_USER_LOGIN, perm='repository.admin') + db.Setting.create_or_update('repository_fields', True, 'bool') # extra_fields enabled + fixture.create_repo_extra_field(repo, field_key='testkey1', field_value='testval1') + fixture.create_repo_extra_field(repo, field_key='testkey2', field_value='testval2') + + expected = { + 'msg': 'updated repo ID:%s %s' % (repo.repo_id, repo_name), + 'repository': repo.get_api_data() + } + expected['repository']['ex_testkey1'] = 'testval1' + expected['repository']['ex_testkey2'] = 'changeval' + + updates = { 'extra_fields': { 'testkey2': 'changeval', }, } + id_, params = _build_data(self.apikey_regular, 'update_repo', repoid=repo_name, **updates) + response = api_call(self, params) + + self._compare_ok(id_, expected, given=response.body) + finally: + fixture.destroy_repo(repo_name) + + def test_api_update_repo_extra_field_missing(self): + repo_name = 'admin_owned' + repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE) + try: + RepoModel().grant_user_permission(repo=repo_name, user=self.TEST_USER_LOGIN, perm='repository.admin') + db.Setting.create_or_update('repository_fields', True, 'bool') # extra_fields enabled + fixture.create_repo_extra_field(repo, field_key='testkey1', field_value='testval1') + fixture.create_repo_extra_field(repo, field_key='testkey2', field_value='testval2') + + expected = { + 'msg': 'updated repo ID:%s %s' % (repo.repo_id, repo_name), + 'repository': repo.get_api_data() + } + expected['repository']['ex_testkey1'] = 'testval1' + expected['repository']['ex_testkey2'] = 'testval2' + + updates = { 'extra_fields': { 'testkey3': 'otherval', }, } + id_, params = _build_data(self.apikey_regular, 'update_repo', repoid=repo_name, **updates) + response = api_call(self, params) + + self._compare_ok(id_, expected, given=response.body) + finally: + fixture.destroy_repo(repo_name) + + def test_api_update_repo_extra_field_disabled(self): + repo_name = 'admin_owned' + repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE) + try: + RepoModel().grant_user_permission(repo=repo_name, user=self.TEST_USER_LOGIN, perm='repository.admin') + db.Setting.create_or_update('repository_fields', False, 'bool') # extra_fields disabled + fixture.create_repo_extra_field(repo, field_key='testkey1', field_value='testval1') + fixture.create_repo_extra_field(repo, field_key='testkey2', field_value='testval2') + + updates = { 'extra_fields': { 'testkey2': 'changeval', }, } + id_, params = _build_data(self.apikey_regular, 'update_repo', repoid=repo_name, **updates) + response = api_call(self, params) + + expected = 'Extra field setting is disabled.' + self._compare_error(id_, expected, given=response.body) + finally: + fixture.destroy_repo(repo_name) + def test_api_delete_repo(self): repo_name = 'api_delete_me' fixture.create_repo(repo_name, repo_type=self.REPO_TYPE) diff --git a/kallithea/tests/fixture.py b/kallithea/tests/fixture.py --- a/kallithea/tests/fixture.py +++ b/kallithea/tests/fixture.py @@ -192,6 +192,19 @@ RepoModel().delete(repo_name, **kwargs) meta.Session().commit() + def create_repo_extra_field(self, repo, field_key, field_value, **kwargs): + field = db.RepositoryField() + field.repository = repo + field.field_type = 'str' + field.field_key = field_key + field.field_value = field_value + field.field_label = kwargs.get('field_label', '') + field.field_desc = kwargs.get('field_desc', '') + meta.Session().add(field) + meta.Session().commit() + + return field + def create_repo_group(self, name, parent_group_id=None, cur_user=TEST_USER_ADMIN_LOGIN, **kwargs): assert '/' not in name, (name, kwargs) # use group_parent_id to make nested groups if 'skip_if_exists' in kwargs:
# HG changeset patch # User toras9000 <[email protected]> # Date 1691408559 -32400 # Mon Aug 07 20:42:39 2023 +0900 # Branch stable # Node ID 20f782d5b6ce2e2228151d99bcb45762ac6f188a # Parent 6f59d9f02044092bac1993879dcea4eece45d049 api: add endpoint for extra fields diff --git a/kallithea/controllers/api/api.py b/kallithea/controllers/api/api.py --- a/kallithea/controllers/api/api.py +++ b/kallithea/controllers/api/api.py @@ -1995,3 +1995,126 @@ # NOTE: no explicit check that removed reviewers were actually present. 'removed': [x.username for x in remove_objs], } + + # permission check inside + def get_repo_extra_fields(self, repoid): + """ + OUTPUT:: + + id : <id_given_in_input> + result : [ + { + "key" : "<field_key>", + "value" : "<field_value>", + "label" : "<field_label>", + "desc" : "<field_desc>", + "type" : "<field_type>" + }, + … + ] + error : null + """ + repo = get_repo_or_error(repoid) + if not HasRepoPermissionLevel('admin')(repo.repo_name): + raise JSONRPCError('Access denied to repo `%s`' % repo.repo_name) + + # Even if the extra field setting is disabled, reading may still be allowed. + + def field_api_data(field): + return { + 'key': db.RepositoryField.un_prefix_key(field.field_key), + 'value': field.field_value, + 'label': field.field_label, + 'desc': field.field_desc, + 'type': field.field_type, + } + + data = [field_api_data(field) for field in repo.extra_fields] + + return data + + # permission check inside + def create_repo_extra_field(self, repoid, field_key, field_label=None, field_desc=None, field_value=None): + """ + OUTPUT:: + + id : <id_given_in_input> + result : { + "msg": "created extra field key:<field_key>", + "extra_field": { + "key" : "<field_key>", + "value" : "<field_value>", + "label" : "<field_label>", + "desc" : "<field_desc>", + "type" : "<field_type>" + } + } + """ + repo = get_repo_or_error(repoid) + if not HasRepoPermissionLevel('admin')(repo.repo_name): + raise JSONRPCError('Access denied to repo `%s`' % repo.repo_name) + + ex_field_setting = db.Setting.get_by_name('repository_fields') + if (ex_field_setting is None) or (not ex_field_setting.app_settings_value): + raise JSONRPCError('Extra field setting is disabled.') + + try: + field = db.RepositoryField() + field.repository = repo + field.field_type = 'str' + field.field_key = field_key + field.field_desc = field_desc if field_desc is not None else '' + field.field_label = field_label if field_label is not None else '' + field.field_value = field_value if field_value is not None else '' + meta.Session().add(field) + meta.Session().commit() + + return { + 'msg': 'created extra field key:%s' % field_key, + 'extra_field': + { + 'type': field.field_type, + 'key': db.RepositoryField.un_prefix_key(field.field_key), + 'label': field.field_label, + 'desc': field.field_desc, + 'value': field.field_value, + }, + } + + except Exception: + log.error(traceback.format_exc()) + raise JSONRPCError('failed to create extra field key:%s' % field_key) + + # permission check inside + def delete_repo_extra_field(self, repoid, field_key): + """ + OUTPUT:: + + id : <id_given_in_input> + result : { + "msg": "created extra field key:<field_key>" + } + """ + repo = get_repo_or_error(repoid) + if not HasRepoPermissionLevel('admin')(repo.repo_name): + raise JSONRPCError('Access denied to repo `%s`' % repo.repo_name) + + ex_field_setting = db.Setting.get_by_name('repository_fields') + if (ex_field_setting is None) or (not ex_field_setting.app_settings_value): + raise JSONRPCError('Extra field setting is disabled.') + + field = db.RepositoryField.get_by_key_name(field_key, repo) + if field is None: + raise JSONRPCError('Extra field `%s` does not exist' % field_key) + + try: + meta.Session().delete(field) + meta.Session().commit() + + return { + 'msg': 'deleted extra field key:%s' % field_key, + } + + except Exception: + log.error(traceback.format_exc()) + raise JSONRPCError('failed to delete extra field key:%s' % field_ke) diff --git a/kallithea/tests/api/api_base.py b/kallithea/tests/api/api_base.py --- a/kallithea/tests/api/api_base.py +++ b/kallithea/tests/api/api_base.py @@ -3033,3 +3033,134 @@ self._compare_error(random_id, "Invalid request. Neither 'add' nor 'remove' is specified.", given=response.body) assert ext_json.loads(response.body)['result'] is None + + def test_api_get_repo_extra_fields(self): + repo_name = 'admin_owned' + repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE) + try: + RepoModel().grant_user_permission(repo=repo_name, user=self.TEST_USER_LOGIN, perm='repository.admin') + db.Setting.create_or_update('repository_fields', True, 'bool') # extra_fields enabled + fixture.create_repo_extra_field(repo, field_key='testkey1', field_value='testval1', + field_label='testlabel1', field_desc='testdesc1') + fixture.create_repo_extra_field(repo, field_key='testkey2', field_value='testval2', + field_label='testlabel2', field_desc='testdesc2') + + id_, params = _build_data(self.apikey_regular, 'get_repo_extra_fields', repoid=repo_name) + response = api_call(self, params) + + expected = [ + { 'key': 'testkey1', 'value': 'testval1', 'label': 'testlabel1', 'desc': 'testdesc1', 'type': 'str', }, + { 'key': 'testkey2', 'value': 'testval2', 'label': 'testlabel2', 'desc': 'testdesc2', 'type': 'str', }, + ] + + self._compare_ok(id_, expected, given=response.body) + finally: + fixture.destroy_repo(repo_name) + + def test_api_create_repo_extra_field(self): + repo_name = 'admin_owned' + repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE) + try: + RepoModel().grant_user_permission(repo=repo_name, user=self.TEST_USER_LOGIN, perm='repository.admin') + db.Setting.create_or_update('repository_fields', True, 'bool') # extra_fields enabled + + field_key = 'testkey1' + assert db.RepositoryField.get_by_key_name(field_key, repo) is None + + args = { 'field_key': field_key, 'field_label': 'testlabel1', 'field_desc': 'testdesc1', 'field_value': 'testval1', } + id_, params = _build_data(self.apikey_regular, 'create_repo_extra_field', repoid=repo_name, **args) + response = api_call(self, params) + + expected = { + 'msg': 'created extra field key:%s' % field_key, + 'extra_field': { 'type': 'str', 'key': field_key, 'label': 'testlabel1', 'desc': 'testdesc1', 'value': 'testval1', }, + } + self._compare_ok(id_, expected, given=response.body) + + field = db.RepositoryField.get_by_key_name(field_key, repo) + assert field is not None + assert field.field_type == 'str' + assert field.field_key == field_key + assert field.field_label == 'testlabel1' + assert field.field_desc == 'testdesc1' + assert field.field_value == 'testval1' + finally: + fixture.destroy_repo(repo_name) + + def test_api_create_repo_extra_field_minimum(self): + repo_name = 'admin_owned' + repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE) + try: + RepoModel().grant_user_permission(repo=repo_name, user=self.TEST_USER_LOGIN, perm='repository.admin') + db.Setting.create_or_update('repository_fields', True, 'bool') # extra_fields enabled + + field_key = 'testkey1' + args = { 'repoid': repo_name, 'field_key': field_key, } + id_, params = _build_data(self.apikey_regular, 'create_repo_extra_field', **args) + response = api_call(self, params) + + expected = { + 'msg': 'created extra field key:%s' % field_key, + 'extra_field': { 'type': 'str', 'key': field_key, 'label': '', 'desc': '', 'value': '', }, + } + self._compare_ok(id_, expected, given=response.body) + finally: + fixture.destroy_repo(repo_name) + + def test_api_create_repo_extra_field_dpplicate(self): + repo_name = 'admin_owned' + repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE) + try: + RepoModel().grant_user_permission(repo=repo_name, user=self.TEST_USER_LOGIN, perm='repository.admin') + db.Setting.create_or_update('repository_fields', True, 'bool') # extra_fields enabled + fixture.create_repo_extra_field(repo, field_key='testkey1', field_value='testval1') + + field_key = 'testkey1' + args = { 'repoid': repo_name, 'field_key': field_key, } + id_, params = _build_data(self.apikey_regular, 'create_repo_extra_field', **args) + response = api_call(self, params) + + expected = 'failed to create extra field key:%s' % field_key + self._compare_error(id_, expected, given=response.body) + finally: + fixture.destroy_repo(repo_name) + + def test_api_create_repo_extra_field_disabled(self): + repo_name = 'admin_owned' + repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE) + try: + RepoModel().grant_user_permission(repo=repo_name, user=self.TEST_USER_LOGIN, perm='repository.admin') + db.Setting.create_or_update('repository_fields', False, 'bool') # extra_fields disabled + + field_key = 'testkey' + args = { 'repoid': repo_name, 'field_key': field_key, } + id_, params = _build_data(self.apikey_regular, 'create_repo_extra_field', **args) + response = api_call(self, params) + + expected = 'Extra field setting is disabled.' + self._compare_error(id_, expected, given=response.body) + finally: + fixture.destroy_repo(repo_name) + + def test_api_delete_repo_extra_field(self): + repo_name = 'admin_owned' + repo = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE) + try: + RepoModel().grant_user_permission(repo=repo_name, user=self.TEST_USER_LOGIN, perm='repository.admin') + db.Setting.create_or_update('repository_fields', True, 'bool') # extra_fields enabled + fixture.create_repo_extra_field(repo, field_key='testkey1', field_value='testval1') + fixture.create_repo_extra_field(repo, field_key='testkey2', field_value='testval2') + + args = { 'repoid': repo_name, 'field_key': 'testkey2', } + id_, params = _build_data(self.apikey_regular, 'delete_repo_extra_field', **args) + response = api_call(self, params) + + expected = { + 'msg': 'deleted extra field key:testkey2', + } + self._compare_ok(id_, expected, given=response.body) + + assert db.RepositoryField.get_by_key_name('testkey2', repo) is None + assert db.RepositoryField.get_by_key_name('testkey1', repo) is not None + finally: + fixture.destroy_repo(repo_name)
_______________________________________________ kallithea-general mailing list [email protected] https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
