server.conf is lazily loaded

pulp.server.config would load /etc/pulp/server.conf unconditionally at
import time. Converting this to lazily loading the config file allows
for the config object to be instantiated without immediately trying
to read the conf. This also allows changing the config files before
loading them, which makes pulp.server.config testable.

fixes #607

devel/pulp/devel/unit/server/base.py

@@ -49,14 +49,35 @@ def _load_test_config():
 
     config.config.set('database', 'name', 'pulp_unittest')
     config.config.set('server', 'storage_dir', '/tmp/pulp')
+    override_config_attrs()
 
+    start_logging()
+
+
+def override_config_attrs():
     # Prevent the tests from altering the config so that nobody accidentally makes global changes
-    config.config.set = _enforce_config
-    config.load_configuration = _enforce_config
+    if not hasattr(config, '_overridden_attrs'):
+        # only need to save these one
+        setattr(config, '_overridden_attrs', {
+            '__setattr__': config.__setattr__,
+            'load_configuration': config.load_configuration,
+            'config.set': config.config.set,
+        })
     config.__setattr__ = _enforce_config
-    config.config.__setattr__ = _enforce_config
+    config.load_configuration = _enforce_config
+    config.config.set = _enforce_config
 
-    start_logging()
+
+def restore_config_attrs():
+    # Restore values overridden by _override_config_attrs
+    if not hasattr(config, '_overridden_attrs'):
+        return
+
+    for attr in '__setattr__', 'load_configuration':
+        setattr(config, attr, config._overridden_attrs[attr])
+    setattr(config.config, attr, config._overridden_attrs['config.set'])
+
+    del(config._overridden_attrs)
 
 
 class PulpWebservicesTests(unittest.TestCase):

devel/test/unit/server/test_base.py

@@ -61,7 +61,6 @@ def test_load_test_config(self, start_logging, stop_logging, config_set):
         self.assertTrue(config.config.set is base._enforce_config)
         self.assertTrue(config.load_configuration is base._enforce_config)
         self.assertTrue(config.__setattr__ is base._enforce_config)
-        self.assertTrue(config.config.__setattr__ is base._enforce_config)
 
 
 class StartDatabaseConnectionTestCase(unittest.TestCase):

server/pulp/server/config.py

@@ -3,7 +3,40 @@
 from ConfigParser import SafeConfigParser
 
 
-config = None  # ConfigParser.SafeConfigParser instance
+class LazyConfigParser(SafeConfigParser):
+    def __init__(self, *args, **kwargs):
+        self.reload()
+        SafeConfigParser.__init__(self, *args, **kwargs)
+        # The superclass _sections attr takes precedence over our property of
+        # the same name. Deleting it gives us the ability to hook the config
+        # load into access to that attr, which just about every ConfigParser
+        # method does, making for a reliable hook.
+        self._lazy_sections = self._sections
+        del self._sections
+
+    def reload(self):
+        self._loaded = False
+
+    @property
+    def _sections(self):
+        self._load_config()
+        return self._lazy_sections
+
+    def _load_config(self):
+        if self._loaded:
+            return
+        self._loaded = True
+
+        check_config_files()
+        # add the defaults first
+        self._sections.clear()
+        for section, settings in _default_values.items():
+            self.add_section(section)
+            for option, value in settings.items():
+                self.set(section, option, value)
+        self.read(_config_files)
+
+config = LazyConfigParser()
 
 # to guarantee that a section and/or setting exists, add a default value here
 _default_values = {
@@ -112,16 +145,10 @@ def load_configuration():
     """
     Check the configuration files and load the global 'config' object from them.
     """
-    global config
-    check_config_files()
-    config = SafeConfigParser()
-    # add the defaults first
-    for section, settings in _default_values.items():
-        config.add_section(section)
-        for option, value in settings.items():
-            config.set(section, option, value)
-    # read the config files
-    return config.read(_config_files)
+    # config is lazy, so this doesn't immediately trigger a load
+    # the actual load will occur the next time config is accessed
+    config.reload()
+    return config
 
 
 def add_config_file(file_path):
@@ -132,11 +159,10 @@ def add_config_file(file_path):
     @type file_path: str
     @param file_path: full path to the new file to add
     """
-    global _config_files
     if file_path in _config_files:
         raise RuntimeError('File, %s, already in configuration files' % file_path)
     _config_files.append(file_path)
-    load_configuration()
+    config.reload()
 
 
 def remove_config_file(file_path):
@@ -147,11 +173,7 @@ def remove_config_file(file_path):
     @type file_path: str
     @param file_path: full path to the file to remove
     """
-    global _config_files
     if file_path not in _config_files:
         raise RuntimeError('File, %s, not in configuration files' % file_path)
     _config_files.remove(file_path)
-    load_configuration()
-
-
-load_configuration()
+    config.reload()

server/test/unit/server/test_config.py

@@ -0,0 +1,211 @@
+import __builtin__
+import mock
+import os
+import shutil
+import tempfile
+from collections import defaultdict
+from functools import partial
+
+from pulp.common.compat import unittest
+from pulp.devel.unit.server.base import override_config_attrs, restore_config_attrs
+from pulp.server import config
+
+# a fake config with a new section and one that overrides a default
+FAKE_CONFIG_1 = '''\
+[section1]
+key1 = value1
+
+[server]
+test_key = test_value
+server_name = foo
+'''
+
+# another fake config
+FAKE_CONFIG_2 = '''\
+[section1]
+key2 = value2
+
+[section2]
+key1 = value3
+'''
+
+# save the existing config to restore when we're done messing with it
+initial_config_files = config._config_files
+initial_config_object = config.config
+
+
+class ConfigFileMock(object):
+    load_counts = defaultdict(int)
+
+    # this version of mock.mock_open doesn't support the readline(s) methods,
+    # so this is a custom mock designed just for tracking config file loads
+    # supporting only the file interface that ConfigParser actually uses
+    def __init__(self, name, *args, **kwargs):
+        # we're mocking open, so open doesn't work here
+        self.real_file = os.fdopen(os.open(name, os.O_RDWR), *args, **kwargs)
+        self.load_counts[name] += 1
+
+    def readline(self):
+        return self.real_file.readline()
+
+    def close(self):
+        self.real_file.close()
+
+
+class TestDefaultConfigFiles(unittest.TestCase):
+    @mock.patch.object(__builtin__, 'open')
+    def test_config_not_loaded(self, mock_open):
+        # We mess with the default config files *a lot* in following tests,
+        # so we ought to make sure the defaults are sane before messing around
+        self.assertEqual(config._config_files, ['/etc/pulp/server.conf'])
+
+        # Also make sure that our config loading code isn't run...
+        # - config is the right type
+        self.assertIsInstance(config.config, config.LazyConfigParser)
+
+        # - /etc/pulp/server.conf isn't opened on Instantiation
+        config.LazyConfigParser()
+        self.assertEqual(mock_open.call_count, 0)
+
+
+class TestLoadConfFromFiles(unittest.TestCase):
+    def setUp(self):
+        restore_config_attrs()
+        self.addCleanup(override_config_attrs)
+
+        self.tmpdir = tempfile.mkdtemp()
+        self.addCleanup(partial(shutil.rmtree, self.tmpdir))
+
+        config_1_fh, self.config_1_name = tempfile.mkstemp(dir=self.tmpdir)
+        config_1 = os.fdopen(config_1_fh, 'w')
+        config_1.write(FAKE_CONFIG_1)
+        config_1.close()
+
+        config_2_fh, self.config_2_name = tempfile.mkstemp(dir=self.tmpdir)
+        config_2 = os.fdopen(config_2_fh, 'w')
+        config_2.write(FAKE_CONFIG_2)
+        config_2.close()
+
+        # teh spoofs (spooves?)
+        config._config_files = [self.config_1_name, self.config_2_name]
+        config.config = config.LazyConfigParser()
+        ConfigFileMock.load_counts.clear()
+
+    @classmethod
+    def tearDownClass(self):
+        # we were never here...restore the saved config methods
+        config._config_files = initial_config_files
+        config.config = initial_config_object
+
+    @mock.patch.object(__builtin__, 'open', ConfigFileMock)
+    def test_file_load_counts(self):
+        self.assertEqual(open.load_counts[self.config_1_name], 0)
+        self.assertEqual(open.load_counts[self.config_2_name], 0)
+        self.assertFalse(config.config._loaded)
+
+        config.config.sections()
+        # open was called, config is loaded
+        self.assertEqual(open.load_counts[self.config_1_name], 1)
+        self.assertEqual(open.load_counts[self.config_2_name], 1)
+        self.assertTrue(config.config._loaded)
+
+        config.config.sections()
+        # open wasn't called again, config already loaded
+        self.assertEqual(open.load_counts[self.config_1_name], 1)
+        self.assertEqual(open.load_counts[self.config_2_name], 1)
+        self.assertTrue(config.config._loaded)
+
+        config.load_configuration()
+        config.config.sections()
+        # open was called again, config was reloaded
+        self.assertEqual(open.load_counts[self.config_1_name], 2)
+        self.assertEqual(open.load_counts[self.config_2_name], 2)
+
+        # Both config files were loaded
+        self.assertEqual(len(open.load_counts), 2)
+
+    @mock.patch.object(__builtin__, 'open', ConfigFileMock)
+    def test_file_contents(self):
+        default_sections = set(config._default_values)
+        loaded_sections = set(config.config.sections())
+
+        # All of the defaults sections are in the loaded config
+        self.assertTrue(set(default_sections).issubset(loaded_sections))
+
+        # The test sections and values from each config are loaded
+        self.assertEqual(config.config.get('section1', 'key1'), 'value1')
+        self.assertEqual(config.config.get('section1', 'key2'), 'value2')
+        self.assertEqual(config.config.get('section2', 'key1'), 'value3')
+
+        # A new value is added to an existing section
+        self.assertEqual(config.config.get('server', 'test_key'), 'test_value')
+
+        # Existing values are overridden in an existing section
+        self.assertEqual(config.config.get('server', 'server_name'), 'foo')
+
+    def test_set_before_load(self):
+        # Setting a config key triggers the lazy open
+        config.config.set('section1', 'key', 'value')
+
+    def test_file_missing(self):
+        missing = '/idontexist'
+        config._config_files = [missing]
+
+        with self.assertRaises(RuntimeError) as cm:
+            config.config.sections()
+        self.assertEqual(cm.exception.args[0],
+                         'Cannot find configuration file: {0}'.format(missing))
+
+    def test_file_read_denied(self):
+        r_denied = self.config_1_name
+        os.chmod(self.config_1_name, 0o0000)
+        config._config_files = [r_denied]
+
+        with self.assertRaises(RuntimeError) as cm:
+            config.config.sections()
+        self.assertEqual(cm.exception.args[0],
+                         'Cannot read configuration file: {0}'.format(r_denied))
+
+    def test_file_add_remove(self):
+        self.assertIn(self.config_1_name, config._config_files)
+        self.assertIn(self.config_2_name, config._config_files)
+        self.assertIn('section1', config.config.sections())
+        self.assertIn('section2', config.config.sections())
+
+        # remove a config, make sure the other one's still loaded
+        config.remove_config_file(self.config_2_name)
+        self.assertIn('section1', config.config.sections())
+        self.assertNotIn('section2', config.config.sections())
+
+        # remove both, make sure neither is loaded
+        config.remove_config_file(self.config_1_name)
+        self.assertNotIn('section1', config.config.sections())
+        self.assertNotIn('section2', config.config.sections())
+
+        # add a config back, make sure the other still isn't loaded
+        config.add_config_file(self.config_1_name)
+        self.assertIn('section1', config.config.sections())
+        self.assertNotIn('section2', config.config.sections())
+
+        # add the other config back, everything should be loaded again
+        config.add_config_file(self.config_2_name)
+        self.assertIn('section1', config.config.sections())
+        self.assertIn('section2', config.config.sections())
+
+    def test_file_add_remove_existing(self):
+        self.assertIn(self.config_1_name, config._config_files)
+
+        # add a file that's already in the config files list
+        with self.assertRaises(RuntimeError) as cm:
+            config.add_config_file(self.config_1_name)
+        self.assertIn(self.config_1_name, cm.exception.args[0])
+        self.assertIn('already in configuration files', cm.exception.args[0])
+
+        # remove a file that isn't in the config files list
+        config.remove_config_file(self.config_1_name)
+        self.assertNotIn(self.config_1_name, config._config_files)
+
+        with self.assertRaises(RuntimeError) as cm:
+            config.remove_config_file(self.config_1_name)
+        self.assertIn(self.config_1_name, cm.exception.args[0])
+        self.assertIn('not in configuration files', cm.exception.args[0])