Rev 5775: Merge config-concrete-stores into config-section-matchers resolving conflicts in file:///home/vila/src/bzr/experimental/config/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Tue Apr 12 11:22:50 UTC 2011
At file:///home/vila/src/bzr/experimental/config/
------------------------------------------------------------
revno: 5775 [merge]
revision-id: v.ladeuil+lp at free.fr-20110412112250-2b88a6qa3rh1d9n4
parent: v.ladeuil+lp at free.fr-20110409201738-mw0cn3nri8769s9i
parent: v.ladeuil+lp at free.fr-20110412112107-8b1m8nlhjjep7xet
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-section-matchers
timestamp: Tue 2011-04-12 13:22:50 +0200
message:
Merge config-concrete-stores into config-section-matchers resolving conflicts
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:17:38 +0000
+++ b/bzrlib/config.py 2011-04-12 11:22:50 +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:13:59 +0000
+++ b/bzrlib/tests/test_config.py 2011-04-12 11:22:50 +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