Merge pull request #138 from phac-nml/fix_config_boolean_eval

Improve boolean evaluation from config files

CHANGELOG.md

@@ -4,6 +4,7 @@ Changes
 Beta 0.8.2
 ---------
 Bug Fixes:
+* config file now evaluates any capitalization of True/False and displays errors to the user if unable to parse.
 * Fixed command line help text inconsistency
 
 Beta 0.8.1

iridauploader/config/config.py

@@ -254,7 +254,7 @@ def read_config_option(key, expected_type=None, default_value=None):
             if type(res) is bool:
                 return res
             elif type(res) is str:
-                return eval(res)
+                return _eval_boolean(res, key)
             else:
                 raise NameError
     except (ValueError, NameError, NoOptionError):
@@ -264,6 +264,27 @@ def read_config_option(key, expected_type=None, default_value=None):
             raise
 
 
+def _eval_boolean(string, field):
+    """
+    Converts string to bool value, accepts any case permutation.
+    If non true/false is given, displays error to user and raises to exit
+    :param string: string to evaluate as a boolean
+    :param field: config field the key originated from for logging and error message
+    :return: Boolean
+    """
+    caps = string.upper()
+    if caps == "TRUE":
+        logging.debug("Evaluated as True")
+        return True
+    elif caps == "FALSE":
+        logging.debug("Evaluated as False")
+        return False
+    else:
+        error_msg = "Config file field '{}' expected 'True' or 'False' but instead got '{}'".format(field, string)
+        logging.error(error_msg)
+        raise NameError(error_msg)
+
+
 def _update_config_option(field_name, field_value):
     """
     Sets an option in the config parser (Does not write to file)

iridauploader/tests/config/test_config.conf

@@ -5,3 +5,4 @@ username = admin
 password = password1
 base_url = http://localhost:8080/irida-latest/api/
 parser = miseq
+readonly = False

iridauploader/tests/config/test_config.py

@@ -104,7 +104,7 @@ def test_dont_create_new_file_if_override_file_exists(self, mock_path_exists, mo
 
     def test_read_config_option(self):
         """
-        Test writing to config file, make sure writen values are written correctly
+        Test reading from well-defined config file
         :return:
         """
         # set up config
@@ -119,6 +119,61 @@ def test_read_config_option(self):
         self.assertEqual(config.read_config_option('parser'), 'miseq')
         self.assertEqual(config.read_config_option('readonly', bool), False)
 
+    def test_read_config_option_case_false(self):
+        """
+        Test reading from well-defined config file with all lowercase boolean (false)
+        :return:
+        """
+        # set up config
+        config.set_config_file(os.path.join(path_to_module, "test_config_case_false.conf"))
+        config.setup()
+        # Test that all the parameters loaded from file are correct
+        self.assertEqual(config.read_config_option('client_id'), 'uploader')
+        self.assertEqual(config.read_config_option('client_secret'), 'secret')
+        self.assertEqual(config.read_config_option('username'), 'admin')
+        self.assertEqual(config.read_config_option('password'), 'password1')
+        self.assertEqual(config.read_config_option('base_url'), 'http://localhost:8080/irida-latest/api/')
+        self.assertEqual(config.read_config_option('parser'), 'miseq')
+        self.assertEqual(config.read_config_option('readonly', bool), False)
+
+    def test_read_config_option_case_true(self):
+        """
+        Test reading from well-defined config file with strange casing boolean (tRuE)
+        :return:
+        """
+        # set up config
+        config.set_config_file(os.path.join(path_to_module, "test_config_case_true.conf"))
+        config.setup()
+        # Test that all the parameters loaded from file are correct
+        self.assertEqual(config.read_config_option('client_id'), 'uploader')
+        self.assertEqual(config.read_config_option('client_secret'), 'secret')
+        self.assertEqual(config.read_config_option('username'), 'admin')
+        self.assertEqual(config.read_config_option('password'), 'password1')
+        self.assertEqual(config.read_config_option('base_url'), 'http://localhost:8080/irida-latest/api/')
+        self.assertEqual(config.read_config_option('parser'), 'miseq')
+        self.assertEqual(config.read_config_option('readonly', bool), True)
+
+    def test_read_config_option_case_error(self):
+        """
+        Test reading from ill-defined config file with a non-boolean entry
+        :return:
+        """
+        # set up config
+        config.set_config_file(os.path.join(path_to_module, "test_config_case_error.conf"))
+        config.setup()
+        # Test that all the parameters loaded from file are correct
+        self.assertEqual(config.read_config_option('client_id'), 'uploader')
+        self.assertEqual(config.read_config_option('client_secret'), 'secret')
+        self.assertEqual(config.read_config_option('username'), 'admin')
+        self.assertEqual(config.read_config_option('password'), 'password1')
+        self.assertEqual(config.read_config_option('base_url'), 'http://localhost:8080/irida-latest/api/')
+        self.assertEqual(config.read_config_option('parser'), 'miseq')
+        with self.assertRaises(NameError) as context:
+            self.assertEqual(config.read_config_option('readonly', bool), False)
+        self.assertTrue(
+            "Config file field 'readonly' expected 'True' or 'False' but instead got 'this_is_not_a_bool'" in
+            str(context.exception))
+
     def test_set_config_options(self):
         """
         Test writing to config file, make sure writen values are written correctly

iridauploader/tests/config/test_config_case_error.conf

@@ -0,0 +1,8 @@
+[Settings]
+client_id = uploader
+client_secret = secret
+username = admin
+password = password1
+base_url = http://localhost:8080/irida-latest/api/
+parser = miseq
+readonly = this_is_not_a_bool

iridauploader/tests/config/test_config_case_false.conf

@@ -0,0 +1,8 @@
+[Settings]
+client_id = uploader
+client_secret = secret
+username = admin
+password = password1
+base_url = http://localhost:8080/irida-latest/api/
+parser = miseq
+readonly = false

iridauploader/tests/config/test_config_case_true.conf

@@ -0,0 +1,8 @@
+[Settings]
+client_id = uploader
+client_secret = secret
+username = admin
+password = password1
+base_url = http://localhost:8080/irida-latest/api/
+parser = miseq
+readonly = tRuE