From 49bb9ad8c153460ed2c9d13fa45e269e35079a18 Mon Sep 17 00:00:00 2001 From: tommydeboer Date: Thu, 18 Apr 2019 15:12:55 +0200 Subject: [PATCH 1/7] Add MolgenisOfflineError --- mcmd/client/auth.py | 4 ++-- mcmd/utils/errors.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/mcmd/client/auth.py b/mcmd/client/auth.py index d77705af..79eb0f8c 100644 --- a/mcmd/client/auth.py +++ b/mcmd/client/auth.py @@ -11,7 +11,7 @@ from mcmd import io from mcmd.config import config -from mcmd.utils.errors import McmdError +from mcmd.utils.errors import McmdError, MolgenisOfflineError _username = None _password = None @@ -49,7 +49,7 @@ def check_token(): else: raise McmdError(str(e)) except requests.exceptions.ConnectionError: - raise McmdError("Can't connect to {}".format(config.url())) + raise MolgenisOfflineError() def _login(): diff --git a/mcmd/utils/errors.py b/mcmd/utils/errors.py index d202270c..d63a50d0 100644 --- a/mcmd/utils/errors.py +++ b/mcmd/utils/errors.py @@ -1,3 +1,6 @@ +from mcmd.config import config + + class McmdError(Exception): def __init__(self, message): self.message = message @@ -12,3 +15,11 @@ def __init__(self, message): def __str__(self): return repr(self.message) + + +class MolgenisOfflineError(McmdError): + def __init__(self): + self.message = "Can't connect to {}".format(config.url()) + + def __str__(self): + return repr(self.message) From ecd5745b6193ce057e46f5f9b4de46f854d4234e Mon Sep 17 00:00:00 2001 From: tommydeboer Date: Thu, 18 Apr 2019 15:16:06 +0200 Subject: [PATCH 2/7] Remove endpoint information in client and simplify @request --- mcmd/client/molgenis_client.py | 43 +++++++++++++-------------------- mcmd/client/request_handler.py | 44 +++++++++++++++------------------- 2 files changed, 35 insertions(+), 52 deletions(-) diff --git a/mcmd/client/molgenis_client.py b/mcmd/client/molgenis_client.py index 1194c3c4..a4ce74ef 100644 --- a/mcmd/client/molgenis_client.py +++ b/mcmd/client/molgenis_client.py @@ -1,27 +1,29 @@ import json -from urllib.parse import urljoin import requests from mcmd.client import auth from mcmd.client.request_handler import request -from mcmd.config import config -@request() +@request def get(url): return requests.get(url, headers=_get_default_headers()) -@request() -def post(url, data): - return requests.post(url, - headers=_get_default_headers(), - data=json.dumps(data)) +@request +def post(url, data=None, params=None): + kwargs = {'headers': _get_default_headers()} + if data: + kwargs['data'] = json.dumps(data) + if params: + kwargs['params'] = params + + return requests.post(url, **kwargs) -@request() +@request def post_file(url, file_path, params): return requests.post(url, headers={'x-molgenis-token': auth.get_token()}, @@ -29,14 +31,14 @@ def post_file(url, file_path, params): params=params) -@request() +@request def post_files(files, url): return requests.post(url, headers={'x-molgenis-token': auth.get_token()}, files=files) -@request() +@request def post_form(url, data): return requests.post(url, headers={ @@ -45,39 +47,26 @@ def post_form(url, data): data=data) -@request() +@request def delete(url): return requests.delete(url, headers=_get_default_headers()) -@request() +@request def delete_data(url, data): return requests.delete(url, headers=_get_default_headers(), data=json.dumps({"entityIds": data})) -@request() +@request def put(url, data): return requests.put(url=url, headers=_get_default_headers(), data=data) -@request() -def import_by_url(params): - return requests.post(config.api('import_url'), - headers=_get_default_headers(), - params=params) - - -@request(login=False) -def get_version(): - return requests.get(urljoin(config.api('rest2'), 'version'), - headers={'Content-Type': 'application/json'}) - - def _get_default_headers(): headers = {'Content-Type': 'application/json', 'x-molgenis-token': auth.get_token()} diff --git a/mcmd/client/request_handler.py b/mcmd/client/request_handler.py index 8b13638f..01cfd5d1 100644 --- a/mcmd/client/request_handler.py +++ b/mcmd/client/request_handler.py @@ -7,37 +7,31 @@ import requests from mcmd.client import auth -from mcmd.config import config -from mcmd.utils.errors import McmdError +from mcmd.utils.errors import McmdError, MolgenisOfflineError -def request(login=True): +def request(func): """Request decorator.""" - def decorator(func): - - def handle_request(*args, **kwargs): - if login: - auth.check_token() - - response = str() - try: - response = func(*args, **kwargs) - response.raise_for_status() - return response - except requests.HTTPError as e: - if _is_json(response): - _handle_json_error(response.json()) - else: - raise McmdError(str(e)) - except requests.exceptions.ConnectionError: - raise McmdError("Can't connect to {}".format(config.url())) - except requests.RequestException as e: + def handle_request(*args, **kwargs): + auth.check_token() + + response = str() + try: + response = func(*args, **kwargs) + response.raise_for_status() + return response + except requests.HTTPError as e: + if _is_json(response): + _handle_json_error(response.json()) + else: raise McmdError(str(e)) + except requests.exceptions.ConnectionError: + raise MolgenisOfflineError() + except requests.RequestException as e: + raise McmdError(str(e)) - return handle_request - - return decorator + return handle_request def _handle_json_error(response_json): From fbf3ac3a8e9c5808b54ed6e9390c1785e3531d3b Mon Sep 17 00:00:00 2001 From: tommydeboer Date: Thu, 18 Apr 2019 15:18:59 +0200 Subject: [PATCH 3/7] Add api module and remove api section from config defaults --- mcmd/client/api.py | 70 ++++++++++++++++++++++++++++++++ mcmd/config/defaults.yaml | 13 ------ tests/integration/loader_mock.py | 13 ------ 3 files changed, 70 insertions(+), 26 deletions(-) create mode 100644 mcmd/client/api.py diff --git a/mcmd/client/api.py b/mcmd/client/api.py new file mode 100644 index 00000000..fcc5db56 --- /dev/null +++ b/mcmd/client/api.py @@ -0,0 +1,70 @@ +from urllib.parse import urljoin + +from mcmd.config import config + + +def endpoint(func): + def wrapper(): + return urljoin(config.get('host', 'selected'), func()) + + return wrapper + + +@endpoint +def rest1(): + return 'api/v1/' + + +@endpoint +def rest2(): + return 'api/v2/' + + +@endpoint +def login(): + return 'api/v1/login/' + + +@endpoint +def group(): + return 'api/plugin/security/group/' + + +@endpoint +def member(group_name): + return 'api/plugin/security/group/{}/member/'.format(group_name) + + +@endpoint +def import_(): + return 'plugin/importwizard/importFile/' + + +@endpoint +def import_by_url(): + return 'plugin/importwizard/importByUrl/' + + +@endpoint +def permissions(): + return 'menu/admin/permissionmanager/update/' + + +@endpoint +def rls(): + return 'menu/admin/permissionmanager/update/entityclass/rls' + + +@endpoint +def add_theme(): + return 'plugin/thememanager/add-bootstrap-theme' + + +@endpoint +def set_theme(): + return 'plugin/thememanager/set-bootstrap-theme' + + +@endpoint +def logo(): + return 'plugin/menumanager/upload-logo' diff --git a/mcmd/config/defaults.yaml b/mcmd/config/defaults.yaml index 33d0ae47..36ca2882 100644 --- a/mcmd/config/defaults.yaml +++ b/mcmd/config/defaults.yaml @@ -14,16 +14,3 @@ host: username: admin settings: import_action: add_update_existing -api: - rest1: api/v1/ - rest2: api/v2/ - login: api/v1/login/ - group: api/plugin/security/group/ - member: api/plugin/security/group/{}/member - import: plugin/importwizard/importFile/ - import_url: plugin/importwizard/importByUrl - perm: menu/admin/permissionmanager/update/ - rls: menu/admin/permissionmanager/update/entityclass/rls - add_theme: plugin/thememanager/add-bootstrap-theme - logo: plugin/menumanager/upload-logo - set_theme: plugin/thememanager/set-bootstrap-theme diff --git a/tests/integration/loader_mock.py b/tests/integration/loader_mock.py index cfee4881..9f3447f4 100644 --- a/tests/integration/loader_mock.py +++ b/tests/integration/loader_mock.py @@ -22,19 +22,6 @@ password: {password} settings: import_action: add_update_existing -api: - rest1: api/v1/ - rest2: api/v2/ - login: api/v1/login/ - group: api/plugin/security/group/ - member: api/plugin/security/group/{{}}/member - import: plugin/importwizard/importFile/ - import_url: plugin/importwizard/importByUrl - perm: menu/admin/permissionmanager/update/ - rls: menu/admin/permissionmanager/update/entityclass/rls - add_theme: plugin/thememanager/add-bootstrap-theme - logo: plugin/menumanager/upload-logo - set_theme: plugin/thememanager/set-bootstrap-theme """ _url: str = None From c174e5b972d76be960aa415800866f538074b780 Mon Sep 17 00:00:00 2001 From: tommydeboer Date: Thu, 18 Apr 2019 15:23:07 +0200 Subject: [PATCH 4/7] Add molgenis_version module --- mcmd/commands/ping.py | 6 +-- mcmd/version/__init__.py | 0 mcmd/version/molgenis_version.py | 55 +++++++++++++++++++++ tests/integration/commands/test_ping.py | 7 +-- tests/unit/version/molgenis_version_test.py | 34 +++++++++++++ 5 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 mcmd/version/__init__.py create mode 100644 mcmd/version/molgenis_version.py create mode 100644 tests/unit/version/molgenis_version_test.py diff --git a/mcmd/commands/ping.py b/mcmd/commands/ping.py index ba5eca17..5428ade0 100644 --- a/mcmd/commands/ping.py +++ b/mcmd/commands/ping.py @@ -1,11 +1,11 @@ from colorama import Fore import mcmd.config.config as config -from mcmd.commands._registry import arguments -from mcmd.client.molgenis_client import get_version from mcmd.command import command +from mcmd.commands._registry import arguments from mcmd.io import highlight from mcmd.utils.errors import McmdError +from mcmd.version import molgenis_version # ========= @@ -31,7 +31,7 @@ def ping(args): user = config.username() status = Fore.LIGHTGREEN_EX + 'Online' + Fore.RESET try: - version = get_version().json()['molgenisVersion'] + version = molgenis_version.get_version() except McmdError: status = Fore.LIGHTRED_EX + 'Offline' + Fore.RESET version = None diff --git a/mcmd/version/__init__.py b/mcmd/version/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/mcmd/version/molgenis_version.py b/mcmd/version/molgenis_version.py new file mode 100644 index 00000000..1a5cf769 --- /dev/null +++ b/mcmd/version/molgenis_version.py @@ -0,0 +1,55 @@ +"""Contains the MOLGENIS version of the current host.""" +import re +from urllib.parse import urljoin + +import requests +from requests import HTTPError + +from mcmd.config import config +from mcmd.utils.errors import McmdError, MolgenisOfflineError + +_version_number = None +_version = None + + +def get_version(): + """ + Gets the MOLGENIS version lazily. + """ + global _version + if not _version: + _get_version() + return _version + + +def get_version_number(): + """ + Gets the MOLGENIS version lazily and only returns the version number: '8.0.0-SNAPSHOT' will become '8.0.0' + """ + global _version_number + if not _version_number: + _get_version() + return _version_number + + +def _get_version(): + try: + response = requests.get(urljoin(config.get('host', 'selected'), 'api/v2/version'), + headers={'Content-Type': 'application/json'}) + response.raise_for_status() + + global _version + global _version_number + _version = response.json()['molgenisVersion'] + _version_number = _extract_version_number(_version) + except HTTPError as e: + raise McmdError(str(e)) + except requests.exceptions.ConnectionError: + raise MolgenisOfflineError() + + +def _extract_version_number(_version): + match = re.match(r"\d+.\d+.\d+", _version) + if not match: + raise McmdError('Unparsable MOLGENIS version: {}'.format(_version)) + return match.group() diff --git a/tests/integration/commands/test_ping.py b/tests/integration/commands/test_ping.py index b7e27ec6..15fbff5c 100644 --- a/tests/integration/commands/test_ping.py +++ b/tests/integration/commands/test_ping.py @@ -2,6 +2,7 @@ import pytest +from mcmd.utils.errors import McmdError from tests.integration.loader_mock import get_host from tests.integration.utils import run_commander @@ -18,9 +19,9 @@ def test_ping_online(capsys): @pytest.mark.integration -@mock.patch('mcmd.config.config.api') -def test_ping_offline(api_mock, capsys): - api_mock.return_value = 'https://nonexisting.url' +@mock.patch('mcmd.version.molgenis_version.get_version') +def test_ping_offline(get_version_mock, capsys): + get_version_mock.side_effect = McmdError('') run_commander('ping') captured = capsys.readouterr().out diff --git a/tests/unit/version/molgenis_version_test.py b/tests/unit/version/molgenis_version_test.py new file mode 100644 index 00000000..3738e714 --- /dev/null +++ b/tests/unit/version/molgenis_version_test.py @@ -0,0 +1,34 @@ +import unittest + +import pytest + +from mcmd.utils.errors import McmdError +from mcmd.version import molgenis_version + + +@pytest.mark.unit +class MolgenisVersionTest(unittest.TestCase): + + @staticmethod + def test_extract_version_number(): + version = molgenis_version._extract_version_number('7.0.0') + assert version == '7.0.0' + + @staticmethod + def test_extract_version_number_snapshot(): + version = molgenis_version._extract_version_number('8.0.1-SNAPSHOT') + assert version == '8.0.1' + + @staticmethod + def test_extract_version_number_preview(): + version = molgenis_version._extract_version_number('1.2.3-SNAPSHOT-PR-1234-2') + assert version == '1.2.3' + + @staticmethod + def test_extract_version_number_multi(): + version = molgenis_version._extract_version_number('7.2.3-APP-1.0.0') + assert version == '7.2.3' + + def test_extract_version_number_invalid(self): + with self.assertRaises(McmdError): + molgenis_version._extract_version_number('4.0-TESTING') From f82a586473b34c73c6462b92cc94dea1ae7773dd Mon Sep 17 00:00:00 2001 From: tommydeboer Date: Thu, 18 Apr 2019 15:24:50 +0200 Subject: [PATCH 5/7] Refactor all places where api endpoints are used --- mcmd/client/auth.py | 5 +++-- mcmd/commands/add.py | 33 +++++++++++++++++---------------- mcmd/commands/delete.py | 12 ++++++------ mcmd/commands/disable.py | 8 ++++---- mcmd/commands/enable.py | 14 +++++++------- mcmd/commands/give.py | 4 ++-- mcmd/commands/import_.py | 8 ++++---- mcmd/commands/make.py | 10 +++++----- mcmd/commands/script.py | 3 +-- mcmd/commands/set.py | 6 +++--- mcmd/config/config.py | 19 ++++++------------- mcmd/utils/principals.py | 6 +++--- mcmd/utils/resources.py | 6 +++--- 13 files changed, 64 insertions(+), 70 deletions(-) diff --git a/mcmd/client/auth.py b/mcmd/client/auth.py index 79eb0f8c..7bfb2a0f 100644 --- a/mcmd/client/auth.py +++ b/mcmd/client/auth.py @@ -10,6 +10,7 @@ from requests import HTTPError from mcmd import io +from mcmd.client import api from mcmd.config import config from mcmd.utils.errors import McmdError, MolgenisOfflineError @@ -40,7 +41,7 @@ def check_token(): return try: - response = requests.get(urljoin(config.api('rest2'), 'sys_sec_Token?q=token=={}'.format(_token)), + response = requests.get(urljoin(api.rest2(), 'sys_sec_Token?q=token=={}'.format(_token)), headers={'Content-Type': 'application/json', 'x-molgenis-token': _token}) response.raise_for_status() except HTTPError as e: @@ -61,7 +62,7 @@ def _login(): try: io.debug('Logging in as user {}'.format(_username)) - response = requests.post(config.api('login'), + response = requests.post(api.login(), headers={'Content-Type': 'application/json'}, data=json.dumps({"username": _username, "password": _password})) response.raise_for_status() diff --git a/mcmd/commands/add.py b/mcmd/commands/add.py index 1fdd0de2..ea0cb805 100644 --- a/mcmd/commands/add.py +++ b/mcmd/commands/add.py @@ -4,6 +4,7 @@ import mcmd.config.config as config from mcmd import io +from mcmd.client import api from mcmd.client.molgenis_client import post, get, post_files from mcmd.command import command from mcmd.commands._registry import arguments @@ -131,20 +132,20 @@ def add_user(args): superuser = args.is_superuser ch_pwd = args.change_password - post(config.api('rest1') + 'sys_sec_User', - {'username': args.username, - 'password_': password, - 'changePassword': ch_pwd, - 'Email': email, - 'active': active, - 'superuser': superuser - }) + post(api.rest1() + 'sys_sec_User', + data={'username': args.username, + 'password_': password, + 'changePassword': ch_pwd, + 'Email': email, + 'active': active, + 'superuser': superuser + }) @command def add_group(args): io.start('Adding group %s' % highlight(args.name)) - post(config.api('group'), {'name': args.name.lower(), 'label': args.name}) + post(api.group(), data={'name': args.name.lower(), 'label': args.name}) @command @@ -157,14 +158,14 @@ def add_package(args): if args.parent: data['parent'] = args.parent - post(config.api('rest1') + 'sys_md_Package', data) + post(api.rest1() + 'sys_md_Package', data=data) @command def add_token(args): io.start('Adding token %s for user %s' % (highlight(args.token), highlight(args.user))) - user = get(config.api('rest2') + 'sys_sec_User?attrs=id&q=username==%s' % args.user) + user = get(api.rest2() + 'sys_sec_User?attrs=id&q=username==%s' % args.user) if user.json()['total'] == 0: raise McmdError('Unknown user %s' % args.user) @@ -173,7 +174,7 @@ def add_token(args): data = {'User': user_id, 'token': args.token} - post(config.api('rest1') + 'sys_sec_Token', data) + post(api.rest1() + 'sys_sec_Token', data=data) @command @@ -185,7 +186,7 @@ def add_theme(args): """ _validate_args(args) valid_types = {'text/css'} - api = config.api('add_theme') + api_ = api.add_theme() bs3_name = args.bootstrap3 bs4 = args.bootstrap4 paths = [bs3_name] @@ -205,7 +206,7 @@ def add_theme(args): if not args.from_path: paths = [_get_path_from_quick_folders(theme) for theme in paths] files = _prepare_files_for_upload(paths, names, valid_types) - post_files(files, api) + post_files(files, api_) @command @@ -215,7 +216,7 @@ def add_logo(args): :param args: commandline arguments containing path to logo :return: None """ - api = config.api('logo') + api_ = api.logo() valid_types = {'image/jpeg', 'image/png', 'image/gif'} logo = [args.logo] if not args.from_path: @@ -224,7 +225,7 @@ def add_logo(args): else: io.start('Adding logo {}'.format(highlight(args.logo))) files = _prepare_files_for_upload(logo, ['logo'], valid_types) - post_files(files, api) + post_files(files, api_) def _prepare_files_for_upload(paths, names, valid_content_types): diff --git a/mcmd/commands/delete.py b/mcmd/commands/delete.py index 3bc0bc21..d98e10ae 100644 --- a/mcmd/commands/delete.py +++ b/mcmd/commands/delete.py @@ -2,9 +2,9 @@ import mcmd.client.molgenis_client as client from mcmd import io +from mcmd.client import api from mcmd.command import command from mcmd.commands._registry import arguments -from mcmd.config import config from mcmd.io import highlight from mcmd.utils.resources import detect_resource_type, ensure_resource_exists, ResourceType @@ -77,7 +77,7 @@ def _delete_entity_type_data(args): if args.force or (not args.force and io.confirm( 'Are you sure you want to delete all data in entity type {}?'.format(args.resource))): io.start('Deleting all data from entity {}'.format(highlight(args.resource))) - url = urljoin(config.api('rest1'), args.resource) + url = urljoin(api.rest1(), args.resource) client.delete(url) @@ -98,7 +98,7 @@ def _delete_package_contents(args): def _delete_entity_types_in_package(package_id): response = client.get( - config.api('rest2') + ResourceType.ENTITY_TYPE.get_entity_id() + '?attrs=id&q=package==' + package_id) + api.rest2() + ResourceType.ENTITY_TYPE.get_entity_id() + '?attrs=id&q=package==' + package_id) entity_ids = [entity_type['id'] for entity_type in response.json()['items']] if len(entity_ids) > 0: _delete_rows(ResourceType.ENTITY_TYPE.get_entity_id(), entity_ids) @@ -106,7 +106,7 @@ def _delete_entity_types_in_package(package_id): def _delete_packages_in_package(package_id): response = client.get( - config.api('rest2') + ResourceType.PACKAGE.get_entity_id() + '?attrs=id&q=parent==' + package_id) + api.rest2() + ResourceType.PACKAGE.get_entity_id() + '?attrs=id&q=parent==' + package_id) package_ids = [entity_type['id'] for entity_type in response.json()['items']] if len(package_ids) > 0: _delete_rows(ResourceType.PACKAGE.get_entity_id(), package_ids) @@ -116,11 +116,11 @@ def _delete_group(args): if args.force or (not args.force and io.confirm( 'Are you sure you want to delete group {}?'.format(args.resource))): io.start('Deleting group {}'.format(highlight(args.resource))) - client.delete(urljoin(config.api('group'), args.resource)) + client.delete(urljoin(api.group(), args.resource)) def _delete_rows(entity_type, rows): - url = '{}{}'.format(config.api('rest2'), entity_type) + url = '{}{}'.format(api.rest2(), entity_type) client.delete_data(url, rows) diff --git a/mcmd/commands/disable.py b/mcmd/commands/disable.py index 88bcc38c..e3e6405b 100644 --- a/mcmd/commands/disable.py +++ b/mcmd/commands/disable.py @@ -1,10 +1,10 @@ -import mcmd.config.config as config from mcmd import io +from mcmd.client import api from mcmd.client.molgenis_client import post -from mcmd.utils.resources import ensure_resource_exists, ResourceType from mcmd.command import command from mcmd.commands._registry import arguments from mcmd.io import highlight +from mcmd.utils.resources import ensure_resource_exists, ResourceType # ========= @@ -39,5 +39,5 @@ def disable_rls(args): io.start('Disabling row level security on entity type %s' % highlight(args.entity)) ensure_resource_exists(args.entity, ResourceType.ENTITY_TYPE) - post(config.api('rls'), data={'id': args.entity, - 'rlsEnabled': False}) + post(api.rls(), data={'id': args.entity, + 'rlsEnabled': False}) diff --git a/mcmd/commands/enable.py b/mcmd/commands/enable.py index 9eb87835..89d1ef4e 100644 --- a/mcmd/commands/enable.py +++ b/mcmd/commands/enable.py @@ -1,11 +1,11 @@ -import mcmd.config.config as config from mcmd import io -from mcmd.commands._registry import arguments +from mcmd.client import api from mcmd.client.molgenis_client import post -from mcmd.utils.resources import one_resource_exists, ensure_resource_exists, ResourceType from mcmd.command import command +from mcmd.commands._registry import arguments from mcmd.io import highlight from mcmd.utils.errors import McmdError +from mcmd.utils.resources import one_resource_exists, ensure_resource_exists, ResourceType # ========= @@ -47,8 +47,8 @@ def enable_rls(args): io.start('Enabling row level security on entity type %s' % highlight(args.entity)) ensure_resource_exists(args.entity, ResourceType.ENTITY_TYPE) - post(config.api('rls'), data={'id': args.entity, - 'rlsEnabled': True}) + post(api.rls(), data={'id': args.entity, + 'rlsEnabled': True}) @command @@ -66,9 +66,9 @@ def enable_theme(args): if one_resource_exists([theme + '.min.css', theme + '.css', 'bootstrap-' + theme + '.min.css'], ResourceType.THEME): # Molgenis themes start with bootstrap- but this is stripped from the name in the theme manager try: - post(config.api('set_theme'), theme) + post(api.set_theme(), data=theme) except: - post(config.api('set_theme'), theme.split('bootstrap-')[1]) + post(api.set_theme(), data=theme.split('bootstrap-')[1]) else: raise McmdError( 'Applying theme failed. No themes found containing {} in the name'.format(args.theme, 'sys_set_StyleSheet')) diff --git a/mcmd/commands/give.py b/mcmd/commands/give.py index 9c3c6dae..43bee2e6 100644 --- a/mcmd/commands/give.py +++ b/mcmd/commands/give.py @@ -6,10 +6,10 @@ from urllib.parse import urljoin from mcmd import io +from mcmd.client import api from mcmd.client.molgenis_client import post_form from mcmd.command import command from mcmd.commands._registry import arguments -from mcmd.config import config from mcmd.io import highlight from mcmd.utils.errors import McmdError from mcmd.utils.principals import ensure_principal_exists, detect_principal_type, PrincipalType @@ -94,7 +94,7 @@ def _grant(principal_type, principal_name, resource_type, identifier, permission else: raise McmdError('Unknown principal type: %s' % principal_type) - url = urljoin(config.api('perm'), '{}/{}'.format(resource_type.get_resource_name(), principal_type.value)) + url = urljoin(api.permissions(), '{}/{}'.format(resource_type.get_resource_name(), principal_type.value)) post_form(url, data) diff --git a/mcmd/commands/import_.py b/mcmd/commands/import_.py index 5e1c5326..655f6c65 100644 --- a/mcmd/commands/import_.py +++ b/mcmd/commands/import_.py @@ -7,8 +7,8 @@ import mcmd.config.config as config from mcmd import io -from mcmd.client import github_client as github -from mcmd.client.molgenis_client import post_file, get, import_by_url +from mcmd.client import github_client as github, api +from mcmd.client.molgenis_client import post_file, get, post from mcmd.command import command from mcmd.commands._registry import arguments from mcmd.config.home import get_issues_folder @@ -110,7 +110,7 @@ def _import_from_url(args): params['url'] = file_url - response = import_by_url(params) + response = post(api.import_by_url(), params=params) import_run_url = urljoin(config.get('host', 'selected'), response.text) status, message = _poll_for_completion(import_run_url) if status == 'FAILED': @@ -218,7 +218,7 @@ def _do_import(file_path, package, entity_type_id): if entity_type_id: params['entityTypeId'] = entity_type_id - response = post_file(config.api('import'), file_path.resolve(), params) + response = post_file(api.import_(), file_path.resolve(), params) import_run_url = urljoin(config.get('host', 'selected'), response.text) status, message = _poll_for_completion(import_run_url) if status == 'FAILED': diff --git a/mcmd/commands/make.py b/mcmd/commands/make.py index 845675f6..acd863a2 100644 --- a/mcmd/commands/make.py +++ b/mcmd/commands/make.py @@ -1,8 +1,8 @@ -import mcmd.config.config as config from mcmd import io -from mcmd.commands._registry import arguments +from mcmd.client import api from mcmd.client.molgenis_client import post, get from mcmd.command import command +from mcmd.commands._registry import arguments from mcmd.io import highlight from mcmd.utils.errors import McmdError from mcmd.utils.utils import lower_kebab, upper_snake @@ -36,13 +36,13 @@ def make(args): group_name = _find_group(args.role) - url = config.api('member').format(group_name) - post(url, {'username': args.user, 'roleName': args.role.upper()}) + url = api.member(group_name) + post(url, data={'username': args.user, 'roleName': args.role.upper()}) def _find_group(role): io.debug('Fetching groups') - groups = get(config.api('rest2') + 'sys_sec_Group?attrs=name') + groups = get(api.rest2() + 'sys_sec_Group?attrs=name') role = lower_kebab(role) matches = {len(group['name']): group['name'] for group in groups.json()['items'] if role.startswith(group['name'])} diff --git a/mcmd/commands/script.py b/mcmd/commands/script.py index 58a8bca8..ab07665c 100644 --- a/mcmd/commands/script.py +++ b/mcmd/commands/script.py @@ -1,8 +1,7 @@ from mcmd import history, io -from mcmd.commands._registry import arguments from mcmd.command import command +from mcmd.commands._registry import arguments from mcmd.config.home import get_scripts_folder -from mcmd.command import command from mcmd.io import confirm, highlight from mcmd.logging import get_logger from mcmd.utils.errors import McmdError diff --git a/mcmd/commands/set.py b/mcmd/commands/set.py index 854c9714..bc3b6c60 100644 --- a/mcmd/commands/set.py +++ b/mcmd/commands/set.py @@ -3,8 +3,8 @@ """ import json -import mcmd.config.config as config from mcmd import io +from mcmd.client import api from mcmd.client.molgenis_client import get, put from mcmd.command import command from mcmd.commands._registry import arguments @@ -66,7 +66,7 @@ def set_(args): io.start( 'Updating {} of {} settings to {}'.format(highlight(args.setting), highlight(args.type), highlight(args.value))) row = _get_row_id(entity) - url = '{}{}/{}/{}'.format(config.api('rest1'), entity, row, args.setting) + url = '{}{}/{}/{}'.format(api.rest1(), entity, row, args.setting) put(url, json.dumps(args.value)) @@ -91,7 +91,7 @@ def _get_settings_entity(setting): def _quick_get(entity, q): - rest = config.api('rest2') + rest = api.rest2() url = '{}{}?{}'.format(rest, entity, q) return get(url).json()['items'] diff --git a/mcmd/config/config.py b/mcmd/config/config.py index 3aa22a53..071a3a35 100644 --- a/mcmd/config/config.py +++ b/mcmd/config/config.py @@ -5,11 +5,10 @@ import operator from functools import reduce from pathlib import Path -from urllib.parse import urljoin from ruamel.yaml import YAML -from mcmd.utils.errors import ConfigError +import mcmd.utils.errors as errors _config = None _properties_file: Path = None @@ -43,21 +42,21 @@ def get(*args): prop = prop[at] return prop except KeyError as e: - raise ConfigError('missing property: {}'.format(_key_error_string(e))) + raise errors.ConfigError('missing property: {}'.format(_key_error_string(e))) def url(): try: return _get_selected_host_auth()['url'] except KeyError as e: - raise ConfigError('missing property: {}'.format(_key_error_string(e))) + raise errors.ConfigError('missing property: {}'.format(_key_error_string(e))) def username(): try: return _get_selected_host_auth()['username'] except KeyError as e: - raise ConfigError('missing property: {}'.format(_key_error_string(e))) + raise errors.ConfigError('missing property: {}'.format(_key_error_string(e))) def token(): @@ -78,12 +77,6 @@ def git_paths(): return [root_path.joinpath(path) for path in paths] -def api(endpoint): - """Returns the combination of the host's url and the API endpoint.""" - url_ = get('host', 'selected') - return urljoin(url_, get('api', endpoint)) - - def has_option(*args): try: reduce(operator.getitem, list(args), _config) @@ -97,7 +90,7 @@ def set_host(url_): if url_ in [host_['url'] for host_ in hosts]: _config['host']['selected'] = url_ else: - raise ConfigError("There is no host with url {}".format(url_)) + raise errors.ConfigError("There is no host with url {}".format(url_)) _persist() @@ -123,7 +116,7 @@ def _get_selected_host_auth(): if host_['url'] == selected: return host_ - raise ConfigError("The selected host doesn't exist.") + raise errors.ConfigError("The selected host doesn't exist.") def set_token(token_): diff --git a/mcmd/utils/principals.py b/mcmd/utils/principals.py index d1d46c19..ab3f6d6b 100644 --- a/mcmd/utils/principals.py +++ b/mcmd/utils/principals.py @@ -1,7 +1,7 @@ from enum import Enum +from mcmd.client import api from mcmd.client.molgenis_client import get -from mcmd.config import config from mcmd.io import multi_choice from mcmd.logging import get_logger from mcmd.utils.errors import McmdError @@ -30,13 +30,13 @@ def principal_exists(principal_name, principal_type): def user_exists(username): log.debug('Checking if user %s exists' % username) - response = get(config.api('rest2') + 'sys_sec_User?q=username==' + username) + response = get(api.rest2() + 'sys_sec_User?q=username==' + username) return int(response.json()['total']) > 0 def role_exists(rolename): log.debug('Checking if role %s exists' % rolename) - response = get(config.api('rest2') + 'sys_sec_Role?q=name==' + rolename.upper()) + response = get(api.rest2() + 'sys_sec_Role?q=name==' + rolename.upper()) return int(response.json()['total']) > 0 diff --git a/mcmd/utils/resources.py b/mcmd/utils/resources.py index 6b95e60a..60d730de 100644 --- a/mcmd/utils/resources.py +++ b/mcmd/utils/resources.py @@ -1,8 +1,8 @@ from enum import Enum from typing import List +from mcmd.client import api from mcmd.client.molgenis_client import get -from mcmd.config import config from mcmd.io import multi_choice from mcmd.logging import get_logger from mcmd.utils.errors import McmdError @@ -53,14 +53,14 @@ def detect_resource_type(resource_id, types: List[ResourceType]): def resource_exists(resource_id, resource_type): log.debug('Checking if %s %s exists' % (resource_type.get_label(), resource_id)) query = '?q={}=={}'.format(resource_type.get_identifying_attribute(), resource_id) - response = get(config.api('rest2') + resource_type.get_entity_id() + query) + response = get(api.rest2() + resource_type.get_entity_id() + query) return int(response.json()['total']) > 0 def one_resource_exists(resources, resource_type): log.debug('Checking if one of [{}] exists in [{}]'.format(','.join(resources), resource_type.get_label())) query = '?q={}=in=({})'.format(resource_type.get_identifying_attribute(), ','.join(resources)) - response = get(config.api('rest2') + resource_type.get_entity_id() + query) + response = get(api.rest2() + resource_type.get_entity_id() + query) return int(response.json()['total']) > 0 From 0d8f4c5375f8fa0d76fea8dadb5fb1e8499dbb2d Mon Sep 17 00:00:00 2001 From: tommydeboer Date: Fri, 19 Apr 2019 12:44:35 +0200 Subject: [PATCH 6/7] Fix @endpoint decorator not passing arguments --- mcmd/client/api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mcmd/client/api.py b/mcmd/client/api.py index fcc5db56..08d3bc7d 100644 --- a/mcmd/client/api.py +++ b/mcmd/client/api.py @@ -4,8 +4,8 @@ def endpoint(func): - def wrapper(): - return urljoin(config.get('host', 'selected'), func()) + def wrapper(*args, **kwargs): + return urljoin(config.get('host', 'selected'), func(*args, **kwargs)) return wrapper From 3db7fd3d0993146ddd70eb5943b06aebef1d894a Mon Sep 17 00:00:00 2001 From: tommydeboer Date: Thu, 9 May 2019 17:03:17 +0200 Subject: [PATCH 7/7] URL encoding --- mcmd/client/api.py | 12 ++++++------ mcmd/client/auth.py | 6 ++++-- mcmd/client/molgenis_client.py | 3 ++- mcmd/commands/add.py | 12 ++++++++---- mcmd/commands/delete.py | 20 ++++++++++++-------- mcmd/commands/make.py | 5 ++++- mcmd/commands/set.py | 26 ++++++++++++-------------- mcmd/utils/principals.py | 11 +++++++++-- mcmd/utils/resources.py | 15 +++++++++++---- 9 files changed, 68 insertions(+), 42 deletions(-) diff --git a/mcmd/client/api.py b/mcmd/client/api.py index 08d3bc7d..b4f1c01b 100644 --- a/mcmd/client/api.py +++ b/mcmd/client/api.py @@ -1,23 +1,23 @@ -from urllib.parse import urljoin +from urllib.parse import urljoin, quote from mcmd.config import config def endpoint(func): def wrapper(*args, **kwargs): - return urljoin(config.get('host', 'selected'), func(*args, **kwargs)) + return urljoin(config.get('host', 'selected'), quote(func(*args, **kwargs))) return wrapper @endpoint -def rest1(): - return 'api/v1/' +def rest1(path: str): + return urljoin('api/v1/', path) @endpoint -def rest2(): - return 'api/v2/' +def rest2(path: str): + return urljoin('api/v2/', path) @endpoint diff --git a/mcmd/client/auth.py b/mcmd/client/auth.py index 7bfb2a0f..e1937258 100644 --- a/mcmd/client/auth.py +++ b/mcmd/client/auth.py @@ -4,7 +4,6 @@ """ import json -from urllib.parse import urljoin import requests from requests import HTTPError @@ -41,7 +40,10 @@ def check_token(): return try: - response = requests.get(urljoin(api.rest2(), 'sys_sec_Token?q=token=={}'.format(_token)), + response = requests.get(api.rest2('sys_sec_Token'), + params={ + 'q': 'token=={}'.format(_token) + }, headers={'Content-Type': 'application/json', 'x-molgenis-token': _token}) response.raise_for_status() except HTTPError as e: diff --git a/mcmd/client/molgenis_client.py b/mcmd/client/molgenis_client.py index a4ce74ef..b2efa39a 100644 --- a/mcmd/client/molgenis_client.py +++ b/mcmd/client/molgenis_client.py @@ -7,8 +7,9 @@ @request -def get(url): +def get(url, params=None): return requests.get(url, + params=params, headers=_get_default_headers()) diff --git a/mcmd/commands/add.py b/mcmd/commands/add.py index ea0cb805..1124ee01 100644 --- a/mcmd/commands/add.py +++ b/mcmd/commands/add.py @@ -132,7 +132,7 @@ def add_user(args): superuser = args.is_superuser ch_pwd = args.change_password - post(api.rest1() + 'sys_sec_User', + post(api.rest1('sys_sec_User'), data={'username': args.username, 'password_': password, 'changePassword': ch_pwd, @@ -158,14 +158,18 @@ def add_package(args): if args.parent: data['parent'] = args.parent - post(api.rest1() + 'sys_md_Package', data=data) + post(api.rest1('sys_md_Package'), data=data) @command def add_token(args): io.start('Adding token %s for user %s' % (highlight(args.token), highlight(args.user))) - user = get(api.rest2() + 'sys_sec_User?attrs=id&q=username==%s' % args.user) + user = get(api.rest2('sys_sec_User'), + params={ + 'attrs': 'id', + 'q': 'username=={}'.format(args.user) + }) if user.json()['total'] == 0: raise McmdError('Unknown user %s' % args.user) @@ -174,7 +178,7 @@ def add_token(args): data = {'User': user_id, 'token': args.token} - post(api.rest1() + 'sys_sec_Token', data=data) + post(api.rest1('sys_sec_Token'), data=data) @command diff --git a/mcmd/commands/delete.py b/mcmd/commands/delete.py index d98e10ae..8386a552 100644 --- a/mcmd/commands/delete.py +++ b/mcmd/commands/delete.py @@ -77,8 +77,7 @@ def _delete_entity_type_data(args): if args.force or (not args.force and io.confirm( 'Are you sure you want to delete all data in entity type {}?'.format(args.resource))): io.start('Deleting all data from entity {}'.format(highlight(args.resource))) - url = urljoin(api.rest1(), args.resource) - client.delete(url) + client.delete(api.rest1(args.resource)) def _delete_package(args): @@ -97,16 +96,22 @@ def _delete_package_contents(args): def _delete_entity_types_in_package(package_id): - response = client.get( - api.rest2() + ResourceType.ENTITY_TYPE.get_entity_id() + '?attrs=id&q=package==' + package_id) + response = client.get(api.rest2(ResourceType.ENTITY_TYPE.get_entity_id()), + params={ + 'attrs': 'id', + 'q': 'package==' + package_id + }) entity_ids = [entity_type['id'] for entity_type in response.json()['items']] if len(entity_ids) > 0: _delete_rows(ResourceType.ENTITY_TYPE.get_entity_id(), entity_ids) def _delete_packages_in_package(package_id): - response = client.get( - api.rest2() + ResourceType.PACKAGE.get_entity_id() + '?attrs=id&q=parent==' + package_id) + response = client.get(api.rest2(ResourceType.PACKAGE.get_entity_id()), + params={ + 'attrs': 'id', + 'q': 'parent==' + package_id + }) package_ids = [entity_type['id'] for entity_type in response.json()['items']] if len(package_ids) > 0: _delete_rows(ResourceType.PACKAGE.get_entity_id(), package_ids) @@ -120,8 +125,7 @@ def _delete_group(args): def _delete_rows(entity_type, rows): - url = '{}{}'.format(api.rest2(), entity_type) - client.delete_data(url, rows) + client.delete_data(api.rest2(entity_type), rows) def _get_resource_type(args): diff --git a/mcmd/commands/make.py b/mcmd/commands/make.py index acd863a2..7bce0337 100644 --- a/mcmd/commands/make.py +++ b/mcmd/commands/make.py @@ -42,7 +42,10 @@ def make(args): def _find_group(role): io.debug('Fetching groups') - groups = get(api.rest2() + 'sys_sec_Group?attrs=name') + groups = get(api.rest2('sys_sec_Group'), + params={ + 'attrs': 'name' + }) role = lower_kebab(role) matches = {len(group['name']): group['name'] for group in groups.json()['items'] if role.startswith(group['name'])} diff --git a/mcmd/commands/set.py b/mcmd/commands/set.py index bc3b6c60..f3a72002 100644 --- a/mcmd/commands/set.py +++ b/mcmd/commands/set.py @@ -65,15 +65,17 @@ def set_(args): entity = _get_settings_entity(args.type) io.start( 'Updating {} of {} settings to {}'.format(highlight(args.setting), highlight(args.type), highlight(args.value))) - row = _get_row_id(entity) - url = '{}{}/{}/{}'.format(api.rest1(), entity, row, args.setting) + row = _get_first_row_id(entity) + url = api.rest1('{}/{}/{}'.format(entity, row, args.setting)) put(url, json.dumps(args.value)) def _get_settings(): - entity = 'sys_md_EntityType' - query = 'q=extends==sys_set_settings&attrs=~id' - molgenis_settings = _quick_get(entity, query) + molgenis_settings = get(api.rest2('sys_md_EntityType'), + params={ + 'q': 'extends==sys_set_settings', + 'attrs': '~id' + }).json()['items'] return [setting['id'] for setting in molgenis_settings] @@ -90,13 +92,9 @@ def _get_settings_entity(setting): raise McmdError('Setting [{}] is not a valid settings entity'.format(setting)) -def _quick_get(entity, q): - rest = api.rest2() - url = '{}{}?{}'.format(rest, entity, q) - return get(url).json()['items'] - - -def _get_row_id(entity): - query = 'attrs=~id' - settings = _quick_get(entity, query) +def _get_first_row_id(entity): + settings = get(api.rest2(entity), + params={ + 'attrs': '~id' + }).json()['items'] return settings[0]['id'] diff --git a/mcmd/utils/principals.py b/mcmd/utils/principals.py index ab3f6d6b..12a01657 100644 --- a/mcmd/utils/principals.py +++ b/mcmd/utils/principals.py @@ -30,13 +30,20 @@ def principal_exists(principal_name, principal_type): def user_exists(username): log.debug('Checking if user %s exists' % username) - response = get(api.rest2() + 'sys_sec_User?q=username==' + username) + response = get(api.rest2('sys_sec_User'), + params={ + 'q': 'username==' + username + }) + return int(response.json()['total']) > 0 def role_exists(rolename): log.debug('Checking if role %s exists' % rolename) - response = get(api.rest2() + 'sys_sec_Role?q=name==' + rolename.upper()) + response = get(api.rest2('sys_sec_Role'), + params={ + 'q': 'name==' + rolename.upper() + }) return int(response.json()['total']) > 0 diff --git a/mcmd/utils/resources.py b/mcmd/utils/resources.py index 60d730de..16769959 100644 --- a/mcmd/utils/resources.py +++ b/mcmd/utils/resources.py @@ -52,15 +52,22 @@ def detect_resource_type(resource_id, types: List[ResourceType]): def resource_exists(resource_id, resource_type): log.debug('Checking if %s %s exists' % (resource_type.get_label(), resource_id)) - query = '?q={}=={}'.format(resource_type.get_identifying_attribute(), resource_id) - response = get(api.rest2() + resource_type.get_entity_id() + query) + query = '{}=={}'.format(resource_type.get_identifying_attribute(), resource_id) + response = get(api.rest2(resource_type.get_entity_id()), + params={ + 'q': query + }) return int(response.json()['total']) > 0 def one_resource_exists(resources, resource_type): log.debug('Checking if one of [{}] exists in [{}]'.format(','.join(resources), resource_type.get_label())) - query = '?q={}=in=({})'.format(resource_type.get_identifying_attribute(), ','.join(resources)) - response = get(api.rest2() + resource_type.get_entity_id() + query) + query = '{}=in=({})'.format(resource_type.get_identifying_attribute(), ','.join(resources)) + response = get(api.rest2(resource_type.get_entity_id()), + params={ + 'q': query + }) + return int(response.json()['total']) > 0