From 35697c3b9a0d348addc8018e9daed4bfd75d91d1 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 25 Jun 2019 15:17:30 +0100 Subject: [PATCH 1/9] Util for parsing a directory of json/yaml OMERO config files --- components/tools/OmeroPy/src/omero/config.py | 63 ++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/components/tools/OmeroPy/src/omero/config.py b/components/tools/OmeroPy/src/omero/config.py index 247a567dba9..62214447d8b 100644 --- a/components/tools/OmeroPy/src/omero/config.py +++ b/components/tools/OmeroPy/src/omero/config.py @@ -30,6 +30,11 @@ ) from omero_ext import portalocker import json +try: + import yaml + YAML_ENABLED = True +except ImportError: + YAML_ENABLED = False class Environment(object): @@ -439,3 +444,61 @@ def __delitem__(self, key): to_remove.append(p) for x in to_remove: props.remove(x) + + +def load_json_config_dir(configdir): + """ + Load a directory of .json, .yaml or .yml configuration files. + """ + files = (f for f in os.listdir(configdir) if + not f.startswith('.') and + os.path.splitext(f)[1].lower() in ('.json', '.yaml', '.yml')) + config = load_json_configs(os.path.join(configdir, f) for f in sorted(files)) + return config + + +def load_json_configs(configfiles): + """ + Load a set of configuration files in the given order. Each file is a single + set of key-value pairs which will be merged as follows: + - a null value will unset the configuration key + - an empty list [] or dictionary {} will set the property to [] or {} + - a non-empty list will be appended to the existing property if set + - a non-empty dictionary will be merged non-recursively to the existing + property if set + - if none of the above are true the property will be set to the new value + """ + config = {} + + for f in configfiles: + with open(f) as fh: + try: + if f.lower().endswith('.yaml') or f.lower().endswith('.yml'): + if not YAML_ENABLED: + raise Exception( + 'PyYAML module required to load {}'.format(f)) + d = yaml.load(fh) + else: + d = json.load(fh) + except Exception as e: + raise Exception('Failed to load file {}'.format(f)) + for k, v in d.items(): + if v is None: + config.pop(k, None) + elif k not in config or v == [] or v == {}: + config[k] = v + elif isinstance(v, list): + if not isinstance(config[k], list): + raise Exception( + 'Incompatible types for key {}: {} {}'.format( + k, config[k], v)) + config[k].extend(v) + elif isinstance(v, dict): + if not isinstance(config[k], dict): + raise Exception( + 'Incompatible types for key {}: {} {}'.format( + k, config[k], v)) + config[k].update(v) + else: + config[k] = v + return config From 3f8a40b1cfdf4df0cc794983bd8cd35b917121e0 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 25 Jun 2019 15:18:03 +0100 Subject: [PATCH 2/9] OMERO.web settings load from $OMERO_WEB_CONFIG_DIR if set --- .../tools/OmeroWeb/omeroweb/settings.py | 58 +++++++++++-------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/settings.py b/components/tools/OmeroWeb/omeroweb/settings.py index a54aeb338ac..155387815ed 100644 --- a/components/tools/OmeroWeb/omeroweb/settings.py +++ b/components/tools/OmeroWeb/omeroweb/settings.py @@ -166,6 +166,7 @@ } +JSON_CONFIG_DIR = os.getenv('OMERO_WEB_CONFIG_DIR') CONFIG_XML = os.path.join(OMERO_HOME, 'etc', 'grid', 'config.xml') count = 10 event = get_event("websettings") @@ -173,7 +174,10 @@ while True: try: CUSTOM_SETTINGS = dict() - if os.path.exists(CONFIG_XML): + if JSON_CONFIG_DIR: + CUSTOM_SETTINGS = omero.config.load_json_config_dir( + JSON_CONFIG_DIR) + elif os.path.exists(CONFIG_XML): CONFIG_XML = omero.config.ConfigXml(CONFIG_XML, read_only=True) CUSTOM_SETTINGS = CONFIG_XML.as_map() CONFIG_XML.close() @@ -213,6 +217,8 @@ def parse_boolean(s): + if JSON_CONFIG_DIR: + return s s = s.strip().lower() if s in ('true', '1', 't'): return True @@ -220,7 +226,7 @@ def parse_boolean(s): def parse_paths(s): - return [os.path.normpath(path) for path in json.loads(s)] + return [os.path.normpath(path) for path in parse_json(s)] def check_server_type(s): @@ -243,6 +249,12 @@ def identity(x): return x +def parse_json(j): + if JSON_CONFIG_DIR: + return j + return json.loads(j) + + def str_slash(s): if s is not None: s = str(s) @@ -281,7 +293,7 @@ def leave_none_unset_int(s): # Allowed hosts: # https://docs.djangoproject.com/en/1.8/ref/settings/#allowed-hosts "omero.web.allowed_hosts": - ["ALLOWED_HOSTS", '["*"]', json.loads, None], + ["ALLOWED_HOSTS", '["*"]', parse_json, None], # Do not show WARNING (1_8.W001): The standalone TEMPLATE_* settings # were deprecated in Django 1.8 and the TEMPLATES dictionary takes @@ -289,7 +301,7 @@ def leave_none_unset_int(s): # into your default TEMPLATES dict: # TEMPLATE_DIRS, TEMPLATE_CONTEXT_PROCESSORS. "omero.web.system_checks": - ["SILENCED_SYSTEM_CHECKS", '["1_8.W001"]', json.loads, None], + ["SILENCED_SYSTEM_CHECKS", '["1_8.W001"]', parse_json, None], # Internal email notification for omero.web.admins, # loaded from config.xml directly @@ -351,7 +363,7 @@ def leave_none_unset_int(s): "omero.web.admins": ["ADMINS", '[]', - json.loads, + parse_json, ("A list of people who get code error notifications whenever the " "application identifies a broken link or raises an unhandled " "exception that results in an internal server error. This gives " @@ -393,7 +405,7 @@ def leave_none_unset_int(s): '{"index": 6, ' '"class": "django.middleware.clickjacking.XFrameOptionsMiddleware"}' ']'), - json.loads, + parse_json, ('Warning: Only system administrators should use this feature. ' 'List of Django middleware classes in the form ' '[{"class": "class.name", "index": FLOAT}]. ' @@ -448,7 +460,7 @@ def leave_none_unset_int(s): ["CACHES", ('{"default": {"BACKEND":' ' "django.core.cache.backends.dummy.DummyCache"}}'), - json.loads, + parse_json, ("OMERO.web offers alternative session backends to automatically" " delete stale data using the cache session store backend, see " ":djangodoc:`Django cached session documentation `. " @@ -752,11 +764,11 @@ def leave_none_unset_int(s): "omero.web.apps": ["ADDITIONAL_APPS", '[]', - json.loads, + parse_json, ("Add additional Django applications. For example, see" " :doc:`/developers/Web/CreateApp`")], "omero.web.databases": - ["DATABASES", '{}', json.loads, None], + ["DATABASES", '{}', parse_json, None], "omero.web.page_size": ["PAGE", 200, @@ -781,7 +793,7 @@ def leave_none_unset_int(s): '["Help", "https://help.openmicroscopy.org/",' '{"title":"Open OMERO user guide in a new tab", "target":"new"}]' ']'), - json.loads, + parse_json, ("Add links to the top header: links are ``['Link Text', " "'link|lookup_view', options]``, where the url is reverse('link'), " "simply 'link' (for external urls) or lookup_view is a detailed " @@ -803,7 +815,7 @@ def leave_none_unset_int(s): '{"name": "rating", "label": "Ratings", "index": 6},' '{"name": "other", "label": "Others", "index": 7}' ']'), - json.loads, + parse_json, ("Manage Metadata pane accordion. This functionality is limited to" " the existing sections.")], "omero.web.ui.right_plugins": @@ -815,7 +827,7 @@ def leave_none_unset_int(s): # "image_roi_tab"],' '["Preview", "webclient/data/includes/right_plugin.preview.js.html"' ', "preview_tab"]]'), - json.loads, + parse_json, ("Add plugins to the right-hand panel. " "Plugins are ``['Label', 'include.js', 'div_id']``. " "The javascript loads data into ``$('#div_id')``.")], @@ -826,7 +838,7 @@ def leave_none_unset_int(s): # "webtest/webclient_plugins/center_plugin.splitview.js.html", # "split_view_panel"],' ']'), - json.loads, + parse_json, ("Add plugins to the center panels. Plugins are " "``['Channel overlay'," " 'webtest/webclient_plugins/center_plugin.overlay.js.html'," @@ -837,7 +849,7 @@ def leave_none_unset_int(s): "omero.web.cors_origin_whitelist": ["CORS_ORIGIN_WHITELIST", '[]', - json.loads, + parse_json, ("A list of origin hostnames that are authorized to make cross-site " "HTTP requests. " "Used by the django-cors-headers app as described at " @@ -859,7 +871,7 @@ def leave_none_unset_int(s): "omero.web.django_additional_settings": ["DJANGO_ADDITIONAL_SETTINGS", "[]", - json.loads, + parse_json, ("Additional Django settings as list of key-value tuples. " "Use this to set or override Django settings that aren't managed by " "OMERO.web. E.g. ``[\"CUSTOM_KEY\", \"CUSTOM_VALUE\"]``")], From 8e8d735673ea5dae162cc93093e4fa794a809820 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Thu, 4 Jul 2019 16:31:11 +0100 Subject: [PATCH 3/9] Track source of web config values config.xml values are always strings so need to be mapped to the required types, JSON value are already type so only need to be mapped for limited values --- .../tools/OmeroWeb/omeroweb/settings.py | 138 +++++++++++------- 1 file changed, 83 insertions(+), 55 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/settings.py b/components/tools/OmeroWeb/omeroweb/settings.py index 155387815ed..53d4de6f092 100644 --- a/components/tools/OmeroWeb/omeroweb/settings.py +++ b/components/tools/OmeroWeb/omeroweb/settings.py @@ -174,10 +174,7 @@ while True: try: CUSTOM_SETTINGS = dict() - if JSON_CONFIG_DIR: - CUSTOM_SETTINGS = omero.config.load_json_config_dir( - JSON_CONFIG_DIR) - elif os.path.exists(CONFIG_XML): + if os.path.exists(CONFIG_XML): CONFIG_XML = omero.config.ConfigXml(CONFIG_XML, read_only=True) CUSTOM_SETTINGS = CONFIG_XML.as_map() CONFIG_XML.close() @@ -197,6 +194,10 @@ exctype, value = sys.exc_info()[:2] raise exctype(value) +CUSTOM_SETTINGS_JSON = dict() +if JSON_CONFIG_DIR: + CUSTOM_SETTINGS_JSON = omero.config.load_json_config_dir(JSON_CONFIG_DIR) + del event del count del get_event @@ -216,8 +217,8 @@ 'django.contrib.sessions.backends.cached_db') -def parse_boolean(s): - if JSON_CONFIG_DIR: +def parse_boolean(s, src=None): + if src == 'json': return s s = s.strip().lower() if s in ('true', '1', 't'): @@ -229,7 +230,7 @@ def parse_paths(s): return [os.path.normpath(path) for path in parse_json(s)] -def check_server_type(s): +def check_server_type(s, src=None): if s not in ALL_SERVER_TYPES: raise ValueError( "Unknown server type: %s. Valid values are: %s" @@ -237,7 +238,7 @@ def check_server_type(s): return s -def check_session_engine(s): +def check_session_engine(s, src=None): if s not in SESSION_ENGINE_VALUES: raise ValueError( "Unknown session engine: %s. Valid values are: %s" @@ -245,17 +246,17 @@ def check_session_engine(s): return s -def identity(x): +def identity(x, src=None): return x -def parse_json(j): - if JSON_CONFIG_DIR: +def parse_json(j, src=None): + if src == 'json': return j return json.loads(j) -def str_slash(s): +def str_slash(s, src=None): if s is not None: s = str(s) if s and not s.endswith("/"): @@ -263,17 +264,33 @@ def str_slash(s): return s +def str_plain(s, src=None): + return str(s) + + +def int_plain(i, src=None): + return int(i) + + +def normpath_plain(p, src=None): + return os.path.normpath(p) + + +def str_compile(s, src=None): + return re.compile(s) + + class LeaveUnset(Exception): pass -def leave_none_unset(s): +def leave_none_unset(s, src=None): if s is None: raise LeaveUnset() return s -def leave_none_unset_int(s): +def leave_none_unset_int(s, src=None): s = leave_none_unset(s) if s is not None: return int(s) @@ -284,7 +301,7 @@ def leave_none_unset_int(s): # DO NOT EDIT! INTERNAL_SETTINGS_MAPPING = { "omero.qa.feedback": - ["FEEDBACK_URL", "http://qa.openmicroscopy.org.uk", str, None], + ["FEEDBACK_URL", "http://qa.openmicroscopy.org.uk", str_plain, None], "omero.web.upgrades.url": ["UPGRADES_URL", None, leave_none_unset, None], "omero.web.check_version": @@ -335,7 +352,7 @@ def leave_none_unset_int(s): "omero.web.admins.email_subject_prefix": ["EMAIL_SUBJECT_PREFIX", "[OMERO.web - admin notification]", - str, + str_plain, "Subject-line prefix for email messages"], "omero.mail.smtp.starttls.enable": ["EMAIL_USE_TLS", @@ -381,12 +398,17 @@ def leave_none_unset_int(s): "omero.web.application_server.host": ["APPLICATION_SERVER_HOST", "127.0.0.1", - str, + str_plain, "Upstream application host"], "omero.web.application_server.port": - ["APPLICATION_SERVER_PORT", 4080, int, "Upstream application port"], + ["APPLICATION_SERVER_PORT", + 4080, + int_plain, + "Upstream application port"], "omero.web.application_server.max_requests": - ["APPLICATION_SERVER_MAX_REQUESTS", 0, int, + ["APPLICATION_SERVER_MAX_REQUESTS", + 0, + int_plain, ("The maximum number of requests a worker will process before " "restarting.")], "omero.web.middleware": @@ -435,7 +457,7 @@ def leave_none_unset_int(s): "omero.web.static_root": ["STATIC_ROOT", os.path.join(os.path.dirname(__file__), 'static').replace('\\', '/'), - os.path.normpath, + normpath_plain, ("The absolute path to the directory where collectstatic will" " collect static files for deployment. If the staticfiles contrib" " app is enabled (default) the collectstatic management command" @@ -473,7 +495,7 @@ def leave_none_unset_int(s): "omero.web.session_cookie_age": ["SESSION_COOKIE_AGE", 86400, - int, + int_plain, "The age of session cookies, in seconds."], "omero.web.session_cookie_domain": ["SESSION_COOKIE_DOMAIN", @@ -506,7 +528,7 @@ def leave_none_unset_int(s): "OMERO.web.")], "omero.web.logdir": - ["LOGDIR", LOGDIR, str, "A path to the custom log directory."], + ["LOGDIR", LOGDIR, str_plain, "A path to the custom log directory."], "omero.web.secure_proxy_ssl_header": ["SECURE_PROXY_SSL_HEADER", '[]', @@ -526,14 +548,14 @@ def leave_none_unset_int(s): "omero.web.wsgi_workers": ["WSGI_WORKERS", 5, - int, + int_plain, ("The number of worker processes for handling requests. " "Check Gunicorn Documentation " "https://docs.gunicorn.org/en/stable/settings.html#workers")], "omero.web.wsgi_timeout": ["WSGI_TIMEOUT", 60, - int, + int_plain, ("Workers silent for more than this many seconds are killed " "and restarted. Check Gunicorn Documentation " "https://docs.gunicorn.org/en/stable/settings.html#timeout")], @@ -547,7 +569,7 @@ def leave_none_unset_int(s): "omero.web.public.url_filter": ["PUBLIC_URL_FILTER", r'(?#This regular expression matches nothing)a^', - re.compile, + str_compile, ("Set a regular expression that matches URLs the public user is " "allowed to access. If this is not set, no URLs will be " "publicly available.")], @@ -557,7 +579,10 @@ def leave_none_unset_int(s): parse_boolean, "Restrict public users to GET requests only"], "omero.web.public.server_id": - ["PUBLIC_SERVER_ID", 1, int, "Server to authenticate against."], + ["PUBLIC_SERVER_ID", + 1, + int_plain, + "Server to authenticate against."], "omero.web.public.user": ["PUBLIC_USER", None, @@ -571,9 +596,9 @@ def leave_none_unset_int(s): "omero.web.public.cache.enabled": ["PUBLIC_CACHE_ENABLED", "false", parse_boolean, None], "omero.web.public.cache.key": - ["PUBLIC_CACHE_KEY", "omero.web.public.cache.key", str, None], + ["PUBLIC_CACHE_KEY", "omero.web.public.cache.key", str_plain, None], "omero.web.public.cache.timeout": - ["PUBLIC_CACHE_TIMEOUT", 60 * 60 * 24, int, None], + ["PUBLIC_CACHE_TIMEOUT", 60 * 60 * 24, int_plain, None], # Social media integration "omero.web.sharing.twitter": @@ -600,12 +625,12 @@ def leave_none_unset_int(s): "omero.web.ping_interval": ["PING_INTERVAL", 60000, - int, + int_plain, "Timeout interval between ping invocations in seconds"], "omero.web.chunk_size": ["CHUNK_SIZE", 1048576, - int, + int_plain, "Size, in bytes, of the “chunk”"], "omero.web.webgateway_cache": ["WEBGATEWAY_CACHE", None, leave_none_unset, None], @@ -614,7 +639,7 @@ def leave_none_unset_int(s): "omero.web.viewer.view": ["VIEWER_VIEW", 'omeroweb.webclient.views.image_viewer', - str, + str_plain, ("Django view which handles display of, or redirection to, the " "desired full image viewer.")], @@ -652,7 +677,7 @@ def leave_none_unset_int(s): "omero.web.pipeline_staticfile_storage": ["STATICFILES_STORAGE", "pipeline.storage.PipelineStorage", - str, + str_plain, ("The file storage engine to use when collecting static files with" " the collectstatic management command. See `the documentation " "`_" @@ -670,13 +695,13 @@ def leave_none_unset_int(s): "omero.web.login_view": ["LOGIN_VIEW", "weblogin", - str, + str_plain, ("The Django view name used for login. Use this to provide an " "alternative login workflow.")], "omero.web.login_incorrect_credentials_text": ["LOGIN_INCORRECT_CREDENTIALS_TEXT", "Connection not available, please check your user name and password.", - str, + str_plain, ("The error message shown to users who enter an incorrect username " "or password.")], @@ -758,7 +783,7 @@ def leave_none_unset_int(s): "omero.web.login.client_downloads_base": ["CLIENT_DOWNLOAD_GITHUB_REPO", 'ome/omero-insight', - str, + str_plain, ("GitHub repository containing the Desktop client downloads")], "omero.web.apps": @@ -772,13 +797,13 @@ def leave_none_unset_int(s): "omero.web.page_size": ["PAGE", 200, - int, + int_plain, ("Number of images displayed within a dataset or 'orphaned'" " container to prevent from loading them all at once.")], "omero.web.thumbnails_batch": ["THUMBNAILS_BATCH", 50, - int, + int_plain, ("Number of thumbnails retrieved to prevent from loading them" " all at once. Make sure the size is not too big, otherwise" " you may exceed limit request line, see" @@ -864,7 +889,7 @@ def leave_none_unset_int(s): "omero.web.x_frame_options": ["X_FRAME_OPTIONS", "SAMEORIGIN", - str, + str_plain, "Whether to allow OMERO.web to be loaded in a frame." ], @@ -912,7 +937,7 @@ def leave_none_unset_int(s): "omero.web.email_subject_prefix": ["EMAIL_SUBJECT_PREFIX", "[OMERO.web]", - str, + str_plain, ("Default email subject is no longer configurable.")], "omero.web.email_use_tls": ["EMAIL_USE_TLS", @@ -942,7 +967,7 @@ def leave_none_unset_int(s): del CUSTOM_HOST -def check_worker_class(c): +def check_worker_class(c, src=None): if c == "gevent": try: import gevent # NOQA @@ -952,7 +977,7 @@ def check_worker_class(c): return str(c) -def check_threading(t): +def check_threading(t, src=None): if t > 1: try: import concurrent.futures # NOQA @@ -976,7 +1001,7 @@ def check_threading(t): "omero.web.wsgi_worker_connections": ["WSGI_WORKER_CONNECTIONS", 1000, - int, + int_plain, ("(ASYNC WORKERS only) The maximum number of simultaneous clients. " "Check Gunicorn Documentation https://docs.gunicorn.org" "/en/stable/settings.html#worker-connections")], @@ -1025,17 +1050,21 @@ def process_custom_settings( global_name, default_value, mapping, description = values try: - global_value = CUSTOM_SETTINGS[key] - values.append(False) + global_value = CUSTOM_SETTINGS_JSON[key] + src = 'json' except KeyError: - global_value = default_value - values.append(True) + try: + global_value = CUSTOM_SETTINGS[key] + src = 'xml' + except KeyError: + global_value = default_value + src = 'default' + values.append(src) try: - using_default = values[-1] if global_name in deprecated_map: dep_value, dep_key = deprecated_map[global_name] - if using_default: + if src == 'default': logging.warning( 'Setting %s is deprecated, use %s', dep_key, key) global_value = dep_value @@ -1043,7 +1072,7 @@ def process_custom_settings( logging.error( '%s and its deprecated key %s are both set, using %s', key, dep_key, key) - setattr(module, global_name, mapping(global_value)) + setattr(module, global_name, mapping(global_value, src)) except ValueError, e: raise ValueError( "Invalid %s (%s = %r). %s. %s" % @@ -1072,9 +1101,9 @@ def report_settings(module): custom_settings_mappings = getattr(module, 'CUSTOM_SETTINGS_MAPPINGS', {}) for key in sorted(custom_settings_mappings): values = custom_settings_mappings[key] - global_name, default_value, mapping, description, using_default = \ - values - source = using_default and "default" or key + global_name, default_value, mapping, description, source = values + if source != 'default': + source = '%s:%s' % (source, key) global_value = getattr(module, global_name, None) if global_name.isupper(): logger.debug( @@ -1084,10 +1113,9 @@ def report_settings(module): deprecated_settings = getattr(module, 'DEPRECATED_SETTINGS_MAPPINGS', {}) for key in sorted(deprecated_settings): values = deprecated_settings[key] - global_name, default_value, mapping, description, using_default = \ - values + global_name, default_value, mapping, description, source = values global_value = getattr(module, global_name, None) - if global_name.isupper() and not using_default: + if global_name.isupper() and source != 'default': logger.debug( "%s = %r (deprecated:%s, %s)", global_name, cleanse_setting(global_name, global_value), key, description) From fda97e5b9e176374a403e35c12e75f5ee5347061 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 5 Jul 2019 17:36:22 +0100 Subject: [PATCH 4/9] web json config: Handle set and append separately This is necessary because the default values have to be loaded separately --- components/tools/OmeroPy/src/omero/config.py | 107 ++++++++++---- .../tools/OmeroPy/test/unit/test_config.py | 133 +++++++++++++++++- .../tools/OmeroWeb/omeroweb/settings.py | 19 ++- 3 files changed, 224 insertions(+), 35 deletions(-) diff --git a/components/tools/OmeroPy/src/omero/config.py b/components/tools/OmeroPy/src/omero/config.py index 62214447d8b..b7677b00135 100644 --- a/components/tools/OmeroPy/src/omero/config.py +++ b/components/tools/OmeroPy/src/omero/config.py @@ -453,22 +453,47 @@ def load_json_config_dir(configdir): files = (f for f in os.listdir(configdir) if not f.startswith('.') and os.path.splitext(f)[1].lower() in ('.json', '.yaml', '.yml')) - config = load_json_configs(os.path.join(configdir, f) for f in sorted(files)) - return config + cset, cappend = load_json_configs( + os.path.join(configdir, f) for f in sorted(files)) + return cset, cappend def load_json_configs(configfiles): """ - Load a set of configuration files in the given order. Each file is a single - set of key-value pairs which will be merged as follows: + Load a set of configuration files in the given order, and return two + dictionaries: + - values to be set + - values to be appended + + The caller should combine these two dictionaries appropriately. + Typically properties to be `set` will be processed first to allow existing + defaults to be unset if necessary, followed by processing of properties to + `append`. + + Each file is a single JSON or YAML dictionary (set of key-value pairs). + A special key `_mode` can take the value `"set"` (default) or `"append"` + which determines whether the properties will be added to the `set` or + `append` config dictionary. + + In `set` mode duplicate properties will overwrite existing ones, this is + equivalent to `omero config set` with the following rules: - a null value will unset the configuration key - - an empty list [] or dictionary {} will set the property to [] or {} - - a non-empty list will be appended to the existing property if set - - a non-empty dictionary will be merged non-recursively to the existing - property if set - - if none of the above are true the property will be set to the new value + - otherwise the configuration key will be set to the value + + In `append` mode the value of each field must be a list or dictionary, it + is not possible to append a scalar. + - If the value is a list all values in the list will be appended to the + current property value + - If the value is a dictionary it will be merged non-recursively to the + all values in the list will be appended to the + current property value + + :param configfiles [str]: List of JSON or YAML (if module is available) + filepaths to load + :return (dict, dict): Tuple of config-set and config-append properties """ - config = {} + cset = {} + cappend = {} for f in configfiles: with open(f) as fh: @@ -481,24 +506,48 @@ def load_json_configs(configfiles): else: d = json.load(fh) except Exception as e: - raise Exception('Failed to load file {}'.format(f)) + raise Exception('Failed to load file {}: {}'.format(f, e)) + mode = d.pop('_mode', 'set') for k, v in d.items(): - if v is None: - config.pop(k, None) - elif k not in config or v == [] or v == {}: - config[k] = v - elif isinstance(v, list): - if not isinstance(config[k], list): - raise Exception( - 'Incompatible types for key {}: {} {}'.format( - k, config[k], v)) - config[k].extend(v) - elif isinstance(v, dict): - if not isinstance(config[k], dict): - raise Exception( - 'Incompatible types for key {}: {} {}'.format( - k, config[k], v)) - config[k].update(v) + if mode == 'set': + _json_config_set(cset, k, v) + elif mode == 'append': + _json_config_append(cappend, k, v) else: - config[k] = v - return config + raise Exception( + 'Invalid configuration mode: {}'.format(mode)) + + return cset, cappend + + +def _json_config_set(config, k, v): + if v is None: + config.pop(k, None) + else: + config[k] = v + + +def _json_config_append(config, k, v): + if isinstance(v, list): + if k not in config: + config[k] = v + else: + try: + config[k].extend(v) + except AttributeError: + raise Exception( + 'Incompatible types for append key {}: {} {}'.format( + k, config[k], v)) + elif isinstance(v, dict): + if k not in config: + config[k] = v + else: + try: + config[k].update(v) + except AttributeError: + raise Exception( + 'Incompatible types for append key {}: {} {}'.format( + k, config[k], v)) + else: + raise Exception( + 'Append requires a list or dictionary value: {}: {}'.format(k, v)) diff --git a/components/tools/OmeroPy/test/unit/test_config.py b/components/tools/OmeroPy/test/unit/test_config.py index a868914c88b..acf33a95f97 100644 --- a/components/tools/OmeroPy/test/unit/test_config.py +++ b/components/tools/OmeroPy/test/unit/test_config.py @@ -12,10 +12,18 @@ import os import errno import pytest -from omero.config import ConfigXml, xml +from omero.config import ( + ConfigXml, + xml, + _json_config_append, + _json_config_set, + load_json_config_dir, + load_json_configs, +) from omero.util.temp_files import create_path from omero_ext import portalocker +import json from xml.etree.ElementTree import XML, Element, SubElement, tostring @@ -355,3 +363,126 @@ def testCannotRead(self): with pytest.raises(IOError) as excinfo: ConfigXml(str(p)).close() assert excinfo.value.errno == errno.EACCES + + +class TestJsonConfig(object): + + def test_json_config_set(self): + cfg = {} + _json_config_set(cfg, 'a', 'a1') + assert cfg == {'a': 'a1'} + _json_config_set(cfg, 'b', 'b2') + assert cfg == {'a': 'a1', 'b': 'b2'} + _json_config_set(cfg, 'a', 'a11') + assert cfg == {'a': 'a11', 'b': 'b2'} + + def test_json_config_append_invalid(self): + cfg = { + 'a': 1, + 'b': [2], + 'c': {'c3': 3}, + } + with pytest.raises(Exception): + _json_config_append(cfg, 'a', 'not-list-or-dict') + with pytest.raises(Exception): + _json_config_append(cfg, 'b', 'not-list-or-dict') + with pytest.raises(Exception): + _json_config_append(cfg, 'c', 'not-list-or-dict') + + with pytest.raises(Exception): + _json_config_append(cfg, 'b', {'not': 'list'}) + with pytest.raises(Exception): + _json_config_append(cfg, 'c', ['not', 'dict']) + + def test_json_config_append_list(self): + cfg = {} + _json_config_append(cfg, 'b', [2]) + assert cfg == {'b': [2]} + _json_config_append(cfg, 'b', [3]) + assert cfg == {'b': [2, 3]} + _json_config_append(cfg, 'b', [4, 5]) + assert cfg == {'b': [2, 3, 4, 5]} + + def test_json_config_append_dict(self): + cfg = {} + _json_config_append(cfg, 'c', {'c3': 3}) + assert cfg == {'c': {'c3': 3}} + _json_config_append(cfg, 'c', {'c4': 4}) + assert cfg == {'c': {'c3': 3, 'c4': 4}} + _json_config_append(cfg, 'c', {'c4': '44', 'c5': '55'}) + assert cfg == {'c': {'c3': 3, 'c4': '44', 'c5': '55'}} + + def test_load_json_configs_set(self, tmpdir): + cfgfile = tmpdir / 'test.json' + cfg = { + 'str': 'def', + 'int': 1, + 'list': ['a', 1], + 'dict': {'b': 2, 'c': {'d': None}}, + } + cfgfile.write(json.dumps(cfg)) + + cset, cappend = load_json_configs([str(cfgfile)]) + assert cset == cfg + assert cappend == {} + + def test_load_json_configs_append(self, tmpdir): + cfgfile = tmpdir / 'test.json' + cfg = { + '_mode': 'append', + 'list': ['a', 1], + 'dict': {'b': 2, 'c': {'d': None}}, + } + cfgfile.write(json.dumps(cfg)) + + cset, cappend = load_json_configs([str(cfgfile)]) + assert cset == {} + assert cappend == { + 'list': ['a', 1], + 'dict': {'b': 2, 'c': {'d': None}}, + } + + def test_load_json_config_dir(self, tmpdir): + cfgfile1 = tmpdir / 'test1.json' + cfg1 = { + '_mode': 'set', + 'str': 'def', + 'int': 1, + 'list': ['a', 1], + 'dict': {'b': 2, 'c': {'d': None}}, + } + cfgfile1.write(json.dumps(cfg1)) + + cfgfile2 = tmpdir / 'test2.json' + cfg2 = { + '_mode': 'append', + 'list': ['a', 1], + 'dict': {'b': 2, 'c': {'d': None}}, + } + cfgfile2.write(json.dumps(cfg2)) + + cfgfile3 = tmpdir / 'test3.json' + cfg3 = { + '_mode': 'append', + 'list': [None, {'1': 123}], + 'dict': {'e': [1, 2], 'c': {'f': 54321}}, + } + cfgfile3.write(json.dumps(cfg3)) + + cfgfile4 = tmpdir / 'test4.json' + cfg4 = { + 'int': None, + 'dict': {} + } + cfgfile4.write(json.dumps(cfg4)) + + cset, cappend = load_json_config_dir(str(tmpdir)) + assert cset == { + 'str': 'def', + 'list': ['a', 1], + 'dict': {}, + } + assert cappend == { + 'list': ['a', 1, None, {'1': 123}], + 'dict': {'b': 2, 'c': {'f': 54321}, 'e': [1, 2]}, + } diff --git a/components/tools/OmeroWeb/omeroweb/settings.py b/components/tools/OmeroWeb/omeroweb/settings.py index 53d4de6f092..d83539218d5 100644 --- a/components/tools/OmeroWeb/omeroweb/settings.py +++ b/components/tools/OmeroWeb/omeroweb/settings.py @@ -194,9 +194,11 @@ exctype, value = sys.exc_info()[:2] raise exctype(value) -CUSTOM_SETTINGS_JSON = dict() +CUSTOM_SETTINGS_JSON_SET = dict() +CUSTOM_SETTINGS_JSON_APPEND = dict() if JSON_CONFIG_DIR: - CUSTOM_SETTINGS_JSON = omero.config.load_json_config_dir(JSON_CONFIG_DIR) + CUSTOM_SETTINGS_JSON_SET, CUSTOM_SETTINGS_JSON_APPEND = \ + omero.config.load_json_config_dir(JSON_CONFIG_DIR) del event del count @@ -1049,10 +1051,17 @@ def process_custom_settings( global_name, default_value, mapping, description = values - try: - global_value = CUSTOM_SETTINGS_JSON[key] + if (key in CUSTOM_SETTINGS_JSON_SET or + key in CUSTOM_SETTINGS_JSON_APPEND): + global_value = CUSTOM_SETTINGS_JSON_SET.get( + key, mapping(default_value)) + if key in CUSTOM_SETTINGS_JSON_APPEND: + try: + global_value.extend(CUSTOM_SETTINGS_JSON_APPEND[key]) + except AttributeError: + global_value.update(CUSTOM_SETTINGS_JSON_APPEND[key]) src = 'json' - except KeyError: + else: try: global_value = CUSTOM_SETTINGS[key] src = 'xml' From 800991b0f7de023f9c399584654172f24a3b6842 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 5 Jul 2019 17:36:49 +0100 Subject: [PATCH 5/9] Util omero.settings.lookup_web_config to aid external lookups New method that can be used to lookup OMERO.web django and config.xml settings --- .../tools/OmeroWeb/omeroweb/settings.py | 86 +++++++++++++------ 1 file changed, 62 insertions(+), 24 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/settings.py b/components/tools/OmeroWeb/omeroweb/settings.py index d83539218d5..18873a6519e 100644 --- a/components/tools/OmeroWeb/omeroweb/settings.py +++ b/components/tools/OmeroWeb/omeroweb/settings.py @@ -1032,6 +1032,57 @@ def map_deprecated_settings(settings): return m +def lookup_web_config(key, default_value=None, mapping=identity): + """ + Lookup an omero config property as seen by OMERO.web, taking into account + the config.xml and and config JSON files. + + :param key str: The omero config property name + :param default_value: The default value of the property + :param mapping func: The mapping function(value, src) to convert the + property to the OMERO.web value + + :return (value, src, unset): + value: the value of the property taking into account the + default_value and mapping parameters + src: whether the property is the 'default', from the 'xml' config, + or from the 'json', config + unset: If True the property should be left unset, e.g. it should + inherit the default Django value + """ + unset = False + if (key in CUSTOM_SETTINGS_JSON_SET or + key in CUSTOM_SETTINGS_JSON_APPEND): + try: + global_value = CUSTOM_SETTINGS_JSON_SET[key] + except KeyError: + try: + global_value = mapping(default_value, src='default') + except LeaveUnset: + global_value = None + unset = True + src = 'json' + if key in CUSTOM_SETTINGS_JSON_APPEND: + try: + global_value.extend(CUSTOM_SETTINGS_JSON_APPEND[key]) + except AttributeError: + global_value.update(CUSTOM_SETTINGS_JSON_APPEND[key]) + global_value = mapping(global_value, src) + else: + try: + src = 'xml' + global_value = CUSTOM_SETTINGS[key] + except KeyError: + src = 'default' + global_value = default_value + try: + global_value = mapping(global_value, src) + except LeaveUnset: + global_value = None + unset = True + return global_value, src, unset + + def process_custom_settings( module, settings='CUSTOM_SETTINGS_MAPPINGS', deprecated=None): logging.info('Processing custom settings for module %s' % module.__name__) @@ -1050,38 +1101,27 @@ def process_custom_settings( continue global_name, default_value, mapping, description = values - - if (key in CUSTOM_SETTINGS_JSON_SET or - key in CUSTOM_SETTINGS_JSON_APPEND): - global_value = CUSTOM_SETTINGS_JSON_SET.get( - key, mapping(default_value)) - if key in CUSTOM_SETTINGS_JSON_APPEND: - try: - global_value.extend(CUSTOM_SETTINGS_JSON_APPEND[key]) - except AttributeError: - global_value.update(CUSTOM_SETTINGS_JSON_APPEND[key]) - src = 'json' - else: - try: - global_value = CUSTOM_SETTINGS[key] - src = 'xml' - except KeyError: - global_value = default_value - src = 'default' - values.append(src) - try: + global_value, src, unset = lookup_web_config( + key, default_value, mapping) + values.append(src) + if global_name in deprecated_map: dep_value, dep_key = deprecated_map[global_name] if src == 'default': logging.warning( 'Setting %s is deprecated, use %s', dep_key, key) - global_value = dep_value + try: + global_value = mapping(dep_value) + except LeaveUnset: + global_value = None + unset = True else: logging.error( '%s and its deprecated key %s are both set, using %s', key, dep_key, key) - setattr(module, global_name, mapping(global_value, src)) + if not unset: + setattr(module, global_name, global_value) except ValueError, e: raise ValueError( "Invalid %s (%s = %r). %s. %s" % @@ -1090,8 +1130,6 @@ def process_custom_settings( raise ImportError( "ImportError: %s. %s (%s = %r).\n%s" % (e.message, global_name, key, global_value, description)) - except LeaveUnset: - pass process_custom_settings(sys.modules[__name__], 'INTERNAL_SETTINGS_MAPPING') From fafa7efd2560a5452fefa8398f3e251a54d4cdb4 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 5 Jul 2019 17:49:50 +0100 Subject: [PATCH 6/9] Fix web mapping types after rebase --- components/tools/OmeroWeb/omeroweb/settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/settings.py b/components/tools/OmeroWeb/omeroweb/settings.py index 18873a6519e..0449b87a4ab 100644 --- a/components/tools/OmeroWeb/omeroweb/settings.py +++ b/components/tools/OmeroWeb/omeroweb/settings.py @@ -710,13 +710,13 @@ def leave_none_unset_int(s, src=None): "omero.web.top_logo": ["TOP_LOGO", "", - str, + str_plain, ("Customize the webclient top bar logo. The recommended image height " "is 23 pixels and it must be hosted outside of OMERO.web.")], "omero.web.top_logo_link": ["TOP_LOGO_LINK", "", - str, + str_plain, ("The target location of the webclient top logo, default unlinked.")], "omero.web.user_dropdown": From f189f2736f52987843ab3b62c8fca990a007f377 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 5 Jul 2019 19:07:21 +0100 Subject: [PATCH 7/9] omeroweb.api needs to handle json conf --- .../tools/OmeroWeb/omeroweb/api/api_settings.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/api/api_settings.py b/components/tools/OmeroWeb/omeroweb/api/api_settings.py index e458198de5e..b491da0d1ea 100644 --- a/components/tools/OmeroWeb/omeroweb/api/api_settings.py +++ b/components/tools/OmeroWeb/omeroweb/api/api_settings.py @@ -20,8 +20,12 @@ """Settings for the OMERO JSON api app.""" import sys -from omeroweb.settings import process_custom_settings, report_settings, \ - str_slash +from omeroweb.settings import ( + int_plain, + process_custom_settings, + report_settings, + str_slash, +) # load settings API_SETTINGS_MAPPING = { @@ -29,12 +33,12 @@ "omero.web.api.limit": ["API_LIMIT", 200, - int, + int_plain, "Default number of items returned from json api."], "omero.web.api.max_limit": ["API_MAX_LIMIT", 500, - int, + int_plain, "Maximum number of items returned from json api."], "omero.web.api.absolute_url": ["API_ABSOLUTE_URL", From 9442c7c83cc5f957071ded39c63121a2184563bf Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sat, 6 Jul 2019 13:03:34 +0100 Subject: [PATCH 8/9] omero.web json config: backwards compatibility with old mappings Inspect the web settings mapping function and log a deprecation warning for old functions instead of failing. --- .../tools/OmeroWeb/omeroweb/settings.py | 71 ++++++++++++++----- 1 file changed, 53 insertions(+), 18 deletions(-) diff --git a/components/tools/OmeroWeb/omeroweb/settings.py b/components/tools/OmeroWeb/omeroweb/settings.py index 0449b87a4ab..7068be0b147 100644 --- a/components/tools/OmeroWeb/omeroweb/settings.py +++ b/components/tools/OmeroWeb/omeroweb/settings.py @@ -36,6 +36,7 @@ import omero.clients import tempfile import re +import inspect import json import random import string @@ -1032,6 +1033,47 @@ def map_deprecated_settings(settings): return m +def _apply_mapping(value, src, mapping, key): + """ + Applies the mapping function for an OMERO.web setting. + This attempts to handle both new mapping functions (value, src) and old + (value). + Old mapping functions only work with config.xml settings, they do not + support new JSON settings. + + :param value: The raw config value + :param src str: The source of the value + :param mapping function: The mapping function + :param key str: The key name, used to log a deprecation message if an old + mapping function is found + :return (value, unset): + value: The mapped value + unset: If True the property should be left unset + """ + try: + argspec = inspect.getargspec(mapping) + isold = len(argspec.args) < 2 + except TypeError: + # E.g. int(), str() + isold = True + if isold: + logger.warn( + 'Setting %s uses a deprecated mapping function %s', + key, mapping.__name__) + if src not in ('default', 'xml'): + raise ValueError( + 'Deprecated mapping function cannot be used with JSON ' + 'configuration for key %s' % key) + try: + return mapping(value), False + except LeaveUnset: + return None, True + try: + return mapping(value, src), False + except LeaveUnset: + return None, True + + def lookup_web_config(key, default_value=None, mapping=identity): """ Lookup an omero config property as seen by OMERO.web, taking into account @@ -1050,24 +1092,23 @@ def lookup_web_config(key, default_value=None, mapping=identity): unset: If True the property should be left unset, e.g. it should inherit the default Django value """ - unset = False if (key in CUSTOM_SETTINGS_JSON_SET or key in CUSTOM_SETTINGS_JSON_APPEND): try: global_value = CUSTOM_SETTINGS_JSON_SET[key] + src = 'json' except KeyError: - try: - global_value = mapping(default_value, src='default') - except LeaveUnset: - global_value = None - unset = True - src = 'json' + global_value = default_value + src = 'default' + global_value, unset = _apply_mapping(global_value, src, mapping, key) if key in CUSTOM_SETTINGS_JSON_APPEND: + src = 'json' try: global_value.extend(CUSTOM_SETTINGS_JSON_APPEND[key]) except AttributeError: global_value.update(CUSTOM_SETTINGS_JSON_APPEND[key]) - global_value = mapping(global_value, src) + global_value, unset = _apply_mapping( + global_value, src, mapping, key) else: try: src = 'xml' @@ -1075,11 +1116,8 @@ def lookup_web_config(key, default_value=None, mapping=identity): except KeyError: src = 'default' global_value = default_value - try: - global_value = mapping(global_value, src) - except LeaveUnset: - global_value = None - unset = True + global_value, unset = _apply_mapping( + global_value, src, mapping, key) return global_value, src, unset @@ -1111,11 +1149,8 @@ def process_custom_settings( if src == 'default': logging.warning( 'Setting %s is deprecated, use %s', dep_key, key) - try: - global_value = mapping(dep_value) - except LeaveUnset: - global_value = None - unset = True + global_value, unset = _apply_mapping( + global_value, src, mapping, dep_key) else: logging.error( '%s and its deprecated key %s are both set, using %s', From 1dabb0b63f52bcccade6b9dde62e179653d58daa Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sat, 6 Jul 2019 14:46:40 +0100 Subject: [PATCH 9/9] web: Test new json and mapping methods --- .../tools/OmeroWeb/test/unit/test_settings.py | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/components/tools/OmeroWeb/test/unit/test_settings.py b/components/tools/OmeroWeb/test/unit/test_settings.py index 6f90c4a4bc1..675c5541cf0 100644 --- a/components/tools/OmeroWeb/test/unit/test_settings.py +++ b/components/tools/OmeroWeb/test/unit/test_settings.py @@ -23,6 +23,15 @@ """ from connector import Server +import settings +from settings import ( + _apply_mapping, + int_plain, + leave_none_unset, + lookup_web_config, + parse_json, + str_plain, +) # Test model @@ -81,3 +90,80 @@ def test_load_server_list(self): assert str(te) == 'No more instances allowed' Server(host=u'example1.com', port=4064) + + +class TestProperties(object): + + def test_apply_mapping_leaveunset(self): + assert (None, True) == _apply_mapping( + None, 'default', leave_none_unset, 'test.unset') + assert ('x', False) == _apply_mapping( + 'x', 'default', leave_none_unset, 'test.unset') + + def test_apply_mapping_oldmapping(self): + assert (123, False) == _apply_mapping( + '123', 'default', int, 'test.oldmapping') + + def test_apply_mapping_parsejson(self): + assert ({'a': 1}, False) == _apply_mapping( + '{"a": 1}', 'default', parse_json, 'test.parsejson') + assert ({'a': 1}, False) == _apply_mapping( + '{"a": 1}', 'xml', parse_json, 'test.parsejson') + assert ({'a': 1}, False) == _apply_mapping( + {'a': 1}, 'json', parse_json, 'test.parsejson') + + def test_lookup_web_config_default(self, monkeypatch): + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS_JSON_SET', {}) + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS_JSON_APPEND', {}) + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS', {}) + + assert (123, 'default', False) == lookup_web_config( + 'test.missing', 123) + assert ('123', 'default', False) == lookup_web_config( + 'test.missing', 123, str_plain) + + def test_lookup_web_config_jsonset(self, monkeypatch): + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS_JSON_SET', { + 'test.k1': 1, + 'test.k2': [2], + }) + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS_JSON_APPEND', {}) + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS', { + 'test.k1': 'should be ignored', + }) + + assert (1, 'json', False) == lookup_web_config('test.k1') + assert (123, 'default', False) == lookup_web_config( + 'test.missing', 123) + + assert (1, 'json', False) == lookup_web_config( + 'test.k1', '123', parse_json) + assert (123, 'default', False) == lookup_web_config( + 'test.missing', '123', parse_json) + + def test_lookup_web_config_jsonappend(self, monkeypatch): + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS_JSON_SET', { + 'test.k1': [1], + }) + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS_JSON_APPEND', { + 'test.k1': [2, 3], + 'test.k2': [4, 5], + }) + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS', { + 'test.k1': 'should be ignored', + }) + assert ([1, 2, 3], 'json', False) == lookup_web_config( + 'test.k1', '[]', parse_json) + assert ([4, 5], 'json', False) == lookup_web_config( + 'test.k2', '[]', parse_json) + + def test_lookup_web_config_xml(self, monkeypatch): + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS_JSON_SET', {}) + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS_JSON_APPEND', {}) + monkeypatch.setattr(settings, 'CUSTOM_SETTINGS', { + 'test.k1': '1', + }) + + assert ('1', 'xml', False) == lookup_web_config('test.k1', 123) + assert (1, 'xml', False) == lookup_web_config( + 'test.k1', 123, int_plain)