diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index d387c2ed..68cde53b 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -398,17 +398,15 @@ def merge_patch( :param enforce_dict: Boolean, set True enfoces that the target and patch are both dictionaries. """ - target_is_dict = isinstance(target, dict) - patch_is_dict = isinstance(patch, dict) - if enforce_dict and not (target_is_dict and patch_is_dict): + if enforce_dict and not (isinstance(target, dict) and isinstance(patch, dict)): raise ValueError("Both target and patch should be dictionaries.") # If patch is not a dict (object), it replaces target entirely: - if not patch_is_dict: + if not isinstance(patch, dict): return patch # If target is not an object, replace it with an empty object - if not target_is_dict: + if not isinstance(target, dict): target = {} result = {} @@ -456,6 +454,10 @@ def load_patched_config(config_path: str) -> dict: config = json.load(f) except IOError as e: raise type(e)(f"Couldn't load configuration file {config_path}") from e + except json.JSONDecodeError as e: + raise json.JSONDecodeError( + f"Invalid JSON in configuration file {config_path}: {e}", e.doc, e.pos + ) from e # If base_config_file not specified then this is a normal LabThings config file # Just return it. @@ -471,7 +473,11 @@ def load_patched_config(config_path: str) -> dict: with open(base_conf_path, "r", encoding="utf-8") as f: base_config = json.load(f) except IOError as e: - raise type(e)(f"Couldn't load configuration file {base_conf_path}") from e + raise type(e)(f"Couldn't load base configuration file {base_conf_path}") from e + except json.JSONDecodeError as e: + raise json.JSONDecodeError( + f"Invalid JSON in base configuration file {config_path}: {e}", e.doc, e.pos + ) from e if "patch" in config: return merge_patch(base_config, config["patch"], enforce_dict=True) diff --git a/tests/test_config_utilities.py b/tests/test_config_utilities.py new file mode 100644 index 00000000..dd1b212f --- /dev/null +++ b/tests/test_config_utilities.py @@ -0,0 +1,124 @@ +"""Test the configuration handling functions in utilities.""" + +import pytest +import os +import json +from json import JSONDecodeError + +from openflexure_microscope_server import utilities + + +@pytest.mark.parametrize( + ("target", "patch", "outcome", "err_if_enforce"), + [ + ({"a": "b"}, {"a": "c"}, {"a": "c"}, False), + ({"a": "b"}, {"b": "c"}, {"a": "b", "b": "c"}, False), + ({"a": "b"}, {"a": None}, {}, False), + ({"a": "b", "b": "c"}, {"a": None}, {"b": "c"}, False), + ({"a": ["b"]}, {"a": "c"}, {"a": "c"}, False), + ({"a": "c"}, {"a": ["b"]}, {"a": ["b"]}, False), + ( + {"a": {"b": "c"}}, + {"a": {"b": "d", "c": None}}, + {"a": {"b": "d"}}, + False, + ), + ({"a": [{"b": "c"}]}, {"a": [1]}, {"a": [1]}, False), + (["a", "b"], ["c", "d"], ["c", "d"], True), + ({"a": "b"}, ["c"], ["c"], True), + ({"a": "foo"}, None, None, True), + ({"a": "foo"}, "bar", "bar", True), + ({"e": None}, {"a": 1}, {"e": None, "a": 1}, False), + ([1, 2], {"a": "b", "c": None}, {"a": "b"}, True), + ({}, {"a": {"bb": {"ccc": None}}}, {"a": {"bb": {}}}, False), + ], +) +def test_merge_patch(target, patch, outcome, err_if_enforce): + """Check that merge_patch returns the expected outcome for each target and patch. + + These test cases are specified in Appedix A of IETF RFC 7396. + """ + assert utilities.merge_patch(target, patch) == outcome + + # If expected to error when enforce_dict is True then check: + if err_if_enforce: + with pytest.raises(ValueError, match="Both target and patch"): + utilities.merge_patch(target, patch, enforce_dict=True) + else: + # Else should run without error with same expected value + assert utilities.merge_patch(target, patch, enforce_dict=True) == outcome + + +@pytest.mark.parametrize( + ("base_conf", "resolves_to"), + [ + ("/var/base.json", "/var/base.json"), + ("base.json", "/var/openflexure/settings/base.json"), + ("./base.json", "/var/openflexure/settings/base.json"), + ("../base.json", "/var/openflexure/base.json"), + ( + "$OFM_LIB/base.json", + "/var/openflexure/application/openflexure-microscope-server/base.json", + ), + ], +) +def test_resolve_path_from_dir(base_conf, resolves_to, mocker): + """Test that resolve path handles, absolute and relative paths and expands env vars.""" + mocker.patch.dict( + os.environ, + {"OFM_LIB": "/var/openflexure/application/openflexure-microscope-server"}, + ) + conf_dir = "/var/openflexure/settings" + resolved_path = utilities.resolve_path_from_dir(base_conf, directory=conf_dir) + assert resolved_path == resolves_to + + +def test_patching_config(tmpdir): + """Test each situation in load_patched_config.""" + conf_file = os.path.join(tmpdir, "ofm_config.json") + base_conf_file = os.path.join(tmpdir, "ofm_base_config.json") + + simple_config = {"things": {"thing1": "python.class"}} + config_patch = {"things": {"thing2": "python.class2"}} + patched_config = {"things": {"thing1": "python.class", "thing2": "python.class2"}} + broken_json = "{{invalid{Json" + + # Error if config file doesn't exist + with pytest.raises(IOError, match="Couldn't load configuration"): + utilities.load_patched_config(conf_file) + + # Error if config file contains bad json + with open(conf_file, "w", encoding="utf-8") as file_obj: + file_obj.write(broken_json) + with pytest.raises(JSONDecodeError, match="Invalid JSON in configuration"): + utilities.load_patched_config(conf_file) + + # If base_config_file is not set, just return the configuration + with open(conf_file, "w", encoding="utf-8") as file_obj: + json.dump(simple_config, file_obj) + assert utilities.load_patched_config(conf_file) == simple_config + + # Set the "base_config_file" instead, error as there is no base config file + config = {"base_config_file": "./ofm_base_config.json"} + with open(conf_file, "w", encoding="utf-8") as file_obj: + json.dump(config, file_obj) + with pytest.raises(IOError, match="Couldn't load base configuration"): + utilities.load_patched_config(conf_file) + + # Error if base config file contains bad json + with open(base_conf_file, "w", encoding="utf-8") as file_obj: + file_obj.write(broken_json) + with pytest.raises(JSONDecodeError, match="Invalid JSON in base configuration"): + utilities.load_patched_config(conf_file) + + # Loads the simple configuration from the base config file. + with open(base_conf_file, "w", encoding="utf-8") as file_obj: + json.dump(simple_config, file_obj) + assert utilities.load_patched_config(conf_file) == simple_config + + # Set the outer config to have a patch. + # It now reads the simple_config from the base condif file and patches it + config["patch"] = config_patch + with open(conf_file, "w", encoding="utf-8") as file_obj: + json.dump(config, file_obj) + assert utilities.load_patched_config(conf_file) == patched_config diff --git a/tests/test_utilities.py b/tests/test_utilities.py deleted file mode 100644 index a995689e..00000000 --- a/tests/test_utilities.py +++ /dev/null @@ -1,46 +0,0 @@ -"""Test the utilities functions.""" - -import pytest - -from openflexure_microscope_server import utilities - - -@pytest.mark.parametrize( - ("target", "patch", "outcome", "err_if_enforce"), - [ - ({"a": "b"}, {"a": "c"}, {"a": "c"}, False), - ({"a": "b"}, {"b": "c"}, {"a": "b", "b": "c"}, False), - ({"a": "b"}, {"a": None}, {}, False), - ({"a": "b", "b": "c"}, {"a": None}, {"b": "c"}, False), - ({"a": ["b"]}, {"a": "c"}, {"a": "c"}, False), - ({"a": "c"}, {"a": ["b"]}, {"a": ["b"]}, False), - ( - {"a": {"b": "c"}}, - {"a": {"b": "d", "c": None}}, - {"a": {"b": "d"}}, - False, - ), - ({"a": [{"b": "c"}]}, {"a": [1]}, {"a": [1]}, False), - (["a", "b"], ["c", "d"], ["c", "d"], True), - ({"a": "b"}, ["c"], ["c"], True), - ({"a": "foo"}, None, None, True), - ({"a": "foo"}, "bar", "bar", True), - ({"e": None}, {"a": 1}, {"e": None, "a": 1}, False), - ([1, 2], {"a": "b", "c": None}, {"a": "b"}, True), - ({}, {"a": {"bb": {"ccc": None}}}, {"a": {"bb": {}}}, False), - ], -) -def test_merge_patch(target, patch, outcome, err_if_enforce): - """Check that merge_patch returns the expected outcome for each target and patch. - - These test cases are specified in Appedix A of IETF RFC 7396. - """ - assert utilities.merge_patch(target, patch) == outcome - - # If expected to error when enforce_dict is True then check: - if err_if_enforce: - with pytest.raises(ValueError, match="Both target and patch"): - utilities.merge_patch(target, patch, enforce_dict=True) - else: - # Else should run without error with same expected value - assert utilities.merge_patch(target, patch, enforce_dict=True) == outcome