Rev 5769: Merge config-section-matchers into config-stack in file:///home/vila/src/bzr/experimental/config/

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Apr 12 11:23:00 UTC 2011


At file:///home/vila/src/bzr/experimental/config/

------------------------------------------------------------
revno: 5769 [merge]
revision-id: v.ladeuil+lp at free.fr-20110412112259-ixj458w8ehrgkqro
parent: v.ladeuil+lp at free.fr-20110409201906-6179mc48afl4u6hn
parent: v.ladeuil+lp at free.fr-20110412112250-2b88a6qa3rh1d9n4
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-stack
timestamp: Tue 2011-04-12 13:22:59 +0200
message:
  Merge config-section-matchers into config-stack
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-04-09 20:19:06 +0000
+++ b/bzrlib/config.py	2011-04-12 11:22:59 +0000
@@ -2138,15 +2138,33 @@
 class Store(object):
     """Abstract interface to persistent storage for configuration options."""
 
-    def __init__(self):
-        self.loaded = False
+    readonly_section_class = ReadOnlySection
+    mutable_section_class = MutableSection
+
+    @property
+    def loaded(self):
+        raise NotImplementedError(self.loaded)
 
     def load(self):
         raise NotImplementedError(self.load)
 
+    def _load_from_string(self, str_or_unicode):
+        """Create a store from a string in configobj syntax.
+
+        :param str_or_unicode: A string representing the file content. This will
+            be encoded to suit store needs internally.
+
+        This is for tests and should not be used in production unless a
+        convincing use case can be demonstrated :)
+        """
+        raise NotImplementedError(self._load_from_string)
+
     def save(self):
         raise NotImplementedError(self.save)
 
+    def external_url(self):
+        raise NotImplementedError(self.external_url)
+
     def get_sections(self):
         """Returns an ordered iterable of existing sections.
 
@@ -2174,67 +2192,53 @@
         super(ConfigObjStore, self).__init__()
         self.transport = transport
         self.file_name = file_name
-        # No transient content is known initially
-        self._content = None
-
-    @classmethod
-    def from_string(cls, str_or_unicode, transport, file_name):
+        self._config_obj = None
+
+    @property
+    def loaded(self):
+        return self._config_obj != None
+
+    def load(self):
+        """Load the store from the associated file."""
+        if self.loaded:
+            return
+        content = self.transport.get_bytes(self.file_name)
+        self._load_from_string(content)
+
+    def _load_from_string(self, str_or_unicode):
         """Create a config store from a string.
 
         :param str_or_unicode: A string representing the file content. This will
             be utf-8 encoded internally.
 
-        :param transport: The transport object where the config file is located.
-
-        :param file_name: The configuration file basename.
-        """
-        conf = cls(transport=transport, file_name=file_name)
-        conf._create_from_string(str_or_unicode)
-        return conf
-
-    def _create_from_string(self, str_or_unicode):
-        # We just keep the content waiting for load() to be called when needed
-        self._content = StringIO(str_or_unicode.encode('utf-8'))
-
-    def load(self, allow_no_such_file=False):
-        """Load the store from the associated file.
-
-        :param allow_no_such_file: Swallow the NoSuchFile exception if True.
-            This allows delayed loading when creating the first option ever.
+        This is for tests and should not be used in production unless a
+        convincing use case can be demonstrated :)
         """
         if self.loaded:
-            return
-        if self._content is not None:
-            co_input = self._content
-        else:
-            try:
-                content = self.transport.get_bytes(self.file_name)
-            except errors.NoSuchFile:
-                if allow_no_such_file:
-                    content = ''
-                else:
-                    raise
-            co_input =  StringIO(content)
+            raise AssertionError('Already loaded: %r' % (self._config_obj,))
+        co_input = StringIO(str_or_unicode.encode('utf-8'))
         try:
             # The config files are always stored utf8-encoded
             self._config_obj = ConfigObj(co_input, encoding='utf-8')
         except configobj.ConfigObjError, e:
-            # FIXME: external_url should really accepts an optional relpath
-            # parameter (bug #750169) :-/ -- vila 2011-04-04
-            # The following will do in the interim but maybe we don't want to
-            # expose a path here but rather a config ID and its associated
-            # object </hand wawe>.
-            file_path = os.path.join(self.transport.external_url(),
-                                     self.file_name)
-            raise errors.ParseConfigError(e.errors, file_path)
-        self.loaded = True
+            self._config_obj = None
+            raise errors.ParseConfigError(e.errors, self.external_url())
 
     def save(self):
+        if not self.loaded:
+            # Nothing to save
+            return
         out = StringIO()
         self._config_obj.write(out)
         self.transport.put_bytes(self.file_name, out.getvalue())
-        # We don't need the transient content anymore
-        self._content = None
+
+    def external_url(self):
+        # FIXME: external_url should really accepts an optional relpath
+        # parameter (bug #750169) :-/ -- vila 2011-04-04
+        # The following will do in the interim but maybe we don't want to
+        # expose a path here but rather a config ID and its associated
+        # object </hand wawe>.
+        return os.path.join(self.transport.external_url(), self.file_name)
 
     def get_sections(self):
         """Get the configobj section in the file order.
@@ -2245,18 +2249,22 @@
         self.load()
         cobj = self._config_obj
         if cobj.scalars:
-            yield ReadOnlySection(None, cobj)
+            yield self.readonly_section_class(None, cobj)
         for section_name in cobj.sections:
-            yield ReadOnlySection(section_name, cobj[section_name])
+            yield self.readonly_section_class(section_name, cobj[section_name])
 
     def get_mutable_section(self, section_name=None):
         # We need a loaded store
-        self.load(allow_no_such_file=True)
+        try:
+            self.load()
+        except errors.NoSuchFile:
+            # The file doesn't exist, let's pretend it was empty
+            self._load_from_string('')
         if section_name is None:
             section = self._config_obj
         else:
             section = self._config_obj.setdefault(section_name, {})
-        return MutableSection(section_name, section)
+        return self.mutable_section_class(section_name, section)
 
 
 # Note that LockableConfigObjStore inherits from ConfigObjStore because we need
@@ -2319,7 +2327,7 @@
     def __init__(self, possible_transports=None):
         t = transport.get_transport(config_dir(),
                                     possible_transports=possible_transports)
-        super(LocationStore, self).__init__(transport, 'locations.conf')
+        super(LocationStore, self).__init__(t, 'locations.conf')
 
 
 class BranchStore(ConfigObjStore):

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-04-09 20:19:06 +0000
+++ b/bzrlib/tests/test_config.py	2011-04-12 11:22:59 +0000
@@ -36,6 +36,7 @@
     mergetools,
     ui,
     urlutils,
+    registry,
     tests,
     trace,
     transport,
@@ -62,6 +63,23 @@
 
 load_tests = scenarios.load_tests_apply_scenarios
 
+# We need adpaters that can build a config store in a test context. Test
+# classes, based on TestCaseWithTransport, can use the registry to parametrize
+# themselves. The builder will receive a test instance and should return a
+# ready-to-use store.  Plugins that defines new stores can also register
+# themselves here to be tested against the tests defined below.
+test_store_builder_registry = registry.Registry()
+test_store_builder_registry.register(
+    'configobj', lambda test: config.ConfigObjStore(test.get_transport(),
+                                                    'configobj.conf'))
+test_store_builder_registry.register(
+    'bazaar', lambda test: config.GlobalStore())
+test_store_builder_registry.register(
+    'location', lambda test: config.LocationStore())
+test_store_builder_registry.register(
+    'branch', lambda test: config.BranchStore(test.branch))
+
+
 
 sample_long_alias="log -r-15..-1 --line"
 sample_config_text = u"""
@@ -1826,19 +1844,19 @@
 
     def test_get_unkown_option(self):
         a_dict = dict()
-        section = config.ReadOnlySection('myID', a_dict)
+        section = config.ReadOnlySection(None, a_dict)
         self.assertEquals('out of thin air',
                           section.get('foo', 'out of thin air'))
 
     def test_options_is_shared(self):
         a_dict = dict()
-        section = config.ReadOnlySection('myID', a_dict)
+        section = config.ReadOnlySection(None, a_dict)
         self.assertIs(a_dict, section.options)
 
 
 class TestConfigMutableSection(tests.TestCase):
 
-    # FIXME: Parametrize so that all sections (includind os.envrion and the
+    # FIXME: Parametrize so that all sections (including os.environ and the
     # ones produced by Stores) run these tests -- vila 2011-04-01
 
     def test_set(self):
@@ -1898,12 +1916,13 @@
     If provided, the content is added to the store but without saving it on
     disk. It should be a string or a unicode string in the ConfigObj syntax.
     While this poses a constraint on other store implementations, it keeps a
-    simple syntax usable by test writers.
+    simple syntax usable by test writers. Note that the other store
+    implementations can rely on ConfigObj to parse the content and get the
+    option definitions and values from it.
     """
-    if content is None:
-        store = config.ConfigObjStore(transport, file_name)
-    else:
-        store = config.ConfigObjStore.from_string(content, transport, file_name)
+    store = config.ConfigObjStore(transport, file_name)
+    if content is not None:
+        store._load_from_string(content)
     return store
 
 
@@ -1920,97 +1939,120 @@
 
 class TestReadonlyStore(TestStore):
 
-    scenarios = [('configobj', {'_get_store': get_ConfigObjStore})]
-
-    def get_store(self, file_name, content=None):
-        return self._get_store(
-            self.get_readonly_transport(), file_name, content=content)
-
-    def test_delayed_load(self):
-        self.build_tree_contents([('foo.conf', '')])
-        store = self.get_store('foo.conf')
-        self.assertEquals(False, store.loaded)
-        store.load()
-        self.assertEquals(True, store.loaded)
-
-    def test_from_string_delayed_load(self):
-        store = self.get_store('foo.conf', '')
-        self.assertEquals(False, store.loaded)
-        store.load()
-        # We loaded the store from the provided content
+    scenarios = [(key, {'get_store': builder})
+                 for key, builder in test_store_builder_registry.iteritems()]
+
+    def setUp(self):
+        super(TestReadonlyStore, self).setUp()
+        self.branch = self.make_branch('branch')
+
+    def test_building_delays_load(self):
+        store = self.get_store(self)
+        self.assertEquals(False, store.loaded)
+        store._load_from_string('')
         self.assertEquals(True, store.loaded)
 
     def test_get_no_sections_for_empty(self):
-        store = self.get_store('foo.conf', '')
-        store.load()
+        store = self.get_store(self)
+        store._load_from_string('')
         self.assertEquals([], list(store.get_sections()))
 
     def test_get_default_section(self):
-        store = self.get_store('foo.conf', 'foo=bar')
+        store = self.get_store(self)
+        store._load_from_string('foo=bar')
         sections = list(store.get_sections())
         self.assertLength(1, sections)
         self.assertSectionContent((None, {'foo': 'bar'}), sections[0])
 
     def test_get_named_section(self):
-        store = self.get_store('foo.conf', '[baz]\nfoo=bar')
+        store = self.get_store(self)
+        store._load_from_string('[baz]\nfoo=bar')
         sections = list(store.get_sections())
         self.assertLength(1, sections)
         self.assertSectionContent(('baz', {'foo': 'bar'}), sections[0])
 
+    def test_load_from_string_fails_for_non_empty_store(self):
+        store = self.get_store(self)
+        store._load_from_string('foo=bar')
+        self.assertRaises(AssertionError, store._load_from_string, 'bar=baz')
+
 
 class TestMutableStore(TestStore):
 
-    scenarios = [('configobj', {'_get_store': get_ConfigObjStore})]
-
-    def get_store(self, file_name, content=None):
-        # Overriden to get a writable transport
-        return self._get_store(
-            self.get_transport(), file_name, content=content)
-
-    def test_save_empty_succeeds(self):
-        store = self.get_store('foo.conf', '')
-        store.load()
-        self.assertEquals(False, self.get_transport().has('foo.conf'))
-        store.save()
-        self.assertEquals(True, self.get_transport().has('foo.conf'))
+    scenarios = [(key, {'store_id': key, 'get_store': builder})
+                 for key, builder in test_store_builder_registry.iteritems()]
+
+    def setUp(self):
+        super(TestMutableStore, self).setUp()
+        self.transport = self.get_transport()
+        self.branch = self.make_branch('branch')
+
+    def has_store(self, store):
+        store_basename = urlutils.relative_url(self.transport.external_url(),
+                                               store.external_url())
+        return self.transport.has(store_basename)
+
+    def test_save_empty_creates_no_file(self):
+        if self.store_id == 'branch':
+            raise tests.TestNotApplicable(
+                'branch.conf is *always* created when a branch is initialized')
+        store = self.get_store(self)
+        store.save()
+        self.assertEquals(False, self.has_store(store))
+
+    def test_save_emptied_succeeds(self):
+        store = self.get_store(self)
+        store._load_from_string('foo=bar\n')
+        section = store.get_mutable_section(None)
+        section.remove('foo')
+        store.save()
+        self.assertEquals(True, self.has_store(store))
+        modified_store = self.get_store(self)
+        sections = list(modified_store.get_sections())
+        self.assertLength(0, sections)
 
     def test_save_with_content_succeeds(self):
-        store = self.get_store('foo.conf', 'foo=bar\n')
-        store.load()
-        self.assertEquals(False, self.get_transport().has('foo.conf'))
+        if self.store_id == 'branch':
+            raise tests.TestNotApplicable(
+                'branch.conf is *always* created when a branch is initialized')
+        store = self.get_store(self)
+        store._load_from_string('foo=bar\n')
+        self.assertEquals(False, self.has_store(store))
         store.save()
-        self.assertEquals(True, self.get_transport().has('foo.conf'))
-        modified_store = self.get_store('foo.conf')
+        self.assertEquals(True, self.has_store(store))
+        modified_store = self.get_store(self)
         sections = list(modified_store.get_sections())
         self.assertLength(1, sections)
         self.assertSectionContent((None, {'foo': 'bar'}), sections[0])
 
     def test_set_option_in_empty_store(self):
-        store = self.get_store('foo.conf')
+        store = self.get_store(self)
         section = store.get_mutable_section(None)
         section.set('foo', 'bar')
         store.save()
-        modified_store = self.get_store('foo.conf')
+        modified_store = self.get_store(self)
         sections = list(modified_store.get_sections())
         self.assertLength(1, sections)
         self.assertSectionContent((None, {'foo': 'bar'}), sections[0])
 
     def test_set_option_in_default_section(self):
-        store = self.get_store('foo.conf', '')
+        store = self.get_store(self)
+        store._load_from_string('')
         section = store.get_mutable_section(None)
         section.set('foo', 'bar')
         store.save()
-        modified_store = self.get_store('foo.conf')
+        modified_store = self.get_store(self)
         sections = list(modified_store.get_sections())
         self.assertLength(1, sections)
         self.assertSectionContent((None, {'foo': 'bar'}), sections[0])
 
     def test_set_option_in_named_section(self):
-        store = self.get_store('foo.conf', '')
+        store = self.get_store(self)
+        store._load_from_string('')
         section = store.get_mutable_section('baz')
         section.set('foo', 'bar')
         store.save()
-        modified_store = self.get_store('foo.conf')
+        modified_store = self.get_store(self)
         sections = list(modified_store.get_sections())
         self.assertLength(1, sections)
         self.assertSectionContent(('baz', {'foo': 'bar'}), sections[0])
@@ -2023,10 +2065,11 @@
         self.assertRaises(errors.NoSuchFile, store.load)
 
     def test_invalid_content(self):
-        store = config.ConfigObjStore.from_string(
-            'this is invalid !', self.get_transport(), 'foo.conf', )
+        store = config.ConfigObjStore(self.get_transport(), 'foo.conf', )
         self.assertEquals(False, store.loaded)
-        exc = self.assertRaises(errors.ParseConfigError, store.load)
+        exc = self.assertRaises(
+            errors.ParseConfigError, store._load_from_string,
+            'this is invalid !')
         self.assertEndsWith(exc.filename, 'foo.conf')
         # And the load failed
         self.assertEquals(False, store.loaded)
@@ -2036,7 +2079,8 @@
         # option names share the same name space...)
         # FIXME: This should be fixed by forbidding dicts as values ?
         # -- vila 2011-04-05
-        store = config.ConfigObjStore.from_string('''
+        store = config.ConfigObjStore(self.get_transport(), 'foo.conf', )
+        store._load_from_string('''
 foo=bar
 l=1,2
 [DEFAULT]
@@ -2047,7 +2091,7 @@
 foo_in_baz=barbaz
 [[qux]]
 foo_in_qux=quux
-''', self.get_transport(), 'foo.conf')
+''')
         sections = list(store.get_sections())
         self.assertLength(4, sections)
         # The default section has no name.



More information about the bazaar-commits mailing list