Rev 5760: Merge config-abstract-store into config-concrete-stores resolving conflicts in file:///home/vila/src/bzr/experimental/config/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Wed Apr 6 17:32:18 UTC 2011
At file:///home/vila/src/bzr/experimental/config/
------------------------------------------------------------
revno: 5760 [merge]
revision-id: v.ladeuil+lp at free.fr-20110406173218-fmlzozjyn5ru5v0b
parent: v.ladeuil+lp at free.fr-20110406091248-0fzx9x2g30s7eaxt
parent: v.ladeuil+lp at free.fr-20110406173110-6aezj2dnr9q8bvwm
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-concrete-stores
timestamp: Wed 2011-04-06 19:32:18 +0200
message:
Merge config-abstract-store into config-concrete-stores 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-06 09:12:48 +0000
+++ b/bzrlib/config.py 2011-04-06 17:32:18 +0000
@@ -848,7 +848,7 @@
# LockableConfig for other kind of transports, we will need to reuse
# whatever connection is already established -- vila 20100929
self.transport = transport.get_transport(self.dir)
- self._lock = lockdir.LockDir(self.transport, 'lock')
+ self._lock = lockdir.LockDir(self.transport, self.lock_name)
def _create_from_string(self, unicode_bytes, save):
super(LockableConfig, self)._create_from_string(unicode_bytes, False)
@@ -2117,7 +2117,7 @@
# 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 wawing).
+ # object </hand wawe>.
file_path = os.path.join(self.transport.external_url(),
self.file_name)
raise errors.ParseConfigError(e.errors, file_path)
@@ -2153,6 +2153,44 @@
return MutableSection(section_name, section)
+class LockableConfigObjStore(ConfigObjStore):
+ """A ConfigObjStore using locks on save to ensure store integrity."""
+
+ def __init__(self, transport, file_name, lock_dir_name=None):
+ """A config Store using ConfigObj for storage.
+
+ :param transport: The transport object where the config file is located.
+
+ :param file_name: The config file basename in the transport directory.
+ """
+ if lock_dir_name is None:
+ lock_dir_name = 'lock'
+ self.lock_dir_name = lock_dir_name
+ super(LockableConfigObjStore, self).__init__(transport, file_name)
+ self._lock = lockdir.LockDir(self.transport, self.lock_dir_name)
+
+ def lock_write(self, token=None):
+ """Takes a write lock in the directory containing the config file.
+
+ If the directory doesn't exist it is created.
+ """
+ # FIXME: This doesn't check the ownership of the created directories as
+ # ensure_config_dir_exists does. It should if the transport is local
+ # -- vila 2011-04-06
+ self.transport.create_prefix()
+ return self._lock.lock_write(token)
+
+ def unlock(self):
+ self._lock.unlock()
+
+ def break_lock(self):
+ self._lock.break_lock()
+
+ @needs_write_lock
+ def save(self):
+ super(LockableConfigObjStore, self).save()
+
+
# FIXME: global, bazaar, shouldn't that be 'user' instead or even
# 'user_defaults' as opposed to 'user_overrides', 'system_defaults'
# (/etc/bzr/bazaar.conf) and 'system_overrides' ? -- vila 2011-04-05
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2011-04-05 17:39:51 +0000
+++ b/bzrlib/tests/test_config.py 2011-04-06 17:32:18 +0000
@@ -831,7 +831,7 @@
def c1_write_config_file():
before_writing.set()
c1_orig()
- # The lock is held we wait for the main thread to decide when to
+ # The lock is held. We wait for the main thread to decide when to
# continue
after_writing.wait()
c1._write_config_file = c1_write_config_file
@@ -864,7 +864,7 @@
c1_orig = c1._write_config_file
def c1_write_config_file():
ready_to_write.set()
- # The lock is held we wait for the main thread to decide when to
+ # The lock is held. We wait for the main thread to decide when to
# continue
do_writing.wait()
c1_orig()
@@ -1889,19 +1889,45 @@
self.assertEquals(config._Created, section.orig['foo'])
+def get_ConfigObjStore(transport, file_name, content=None):
+ """Build a ConfigObjStore.
+
+ :param transport: The transport where the store lives.
+
+ :param file_name: The name of the store.
+
+ :param content: A provided content to inject into the built store.
+
+ 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.
+ """
+ if content is None:
+ store = config.ConfigObjStore(transport, file_name)
+ else:
+ store = config.ConfigObjStore.from_string(content, transport, file_name)
+ return store
+
+
class TestStore(tests.TestCaseWithTransport):
- # FIXME: parametrize against all valid (store, transport) combinations
-
- def get_store(self, name=None, content=None):
- if name is None:
- name = 'foo.conf'
- if content is None:
- store = config.ConfigObjStore(self.get_transport(), name)
- else:
- store = config.ConfigObjStore.from_string(
- content, self.get_transport(), name)
- return store
+ def assertSectionContent(self, expected, section):
+ """Assert that some options have the proper values in a section."""
+ expected_name, expected_options = expected
+ self.assertEquals(expected_name, section.id)
+ self.assertEquals(
+ expected_options,
+ dict([(k, section.get(k)) for k in expected_options.keys()]))
+
+
+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', '')])
@@ -1914,68 +1940,105 @@
store = self.get_store('foo.conf', '')
self.assertEquals(False, store.loaded)
store.load()
+ # We loaded the store from the provided content
self.assertEquals(True, store.loaded)
- # We use from_string and don't save, so the file shouldn't be created
- self.failIfExists('foo.conf')
+
+ def test_get_no_sections_for_empty(self):
+ store = self.get_store('foo.conf', '')
+ store.load()
+ self.assertEquals([], list(store.get_sections()))
+
+ def test_get_default_section(self):
+ store = self.get_store('foo.conf', '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')
+ sections = list(store.get_sections())
+ self.assertLength(1, sections)
+ self.assertSectionContent(('baz', {'foo': 'bar'}), sections[0])
+
+
+class TestMutableStore(TestStore):
+
+ scenarios = [('configobj', {'_get_store': get_ConfigObjStore})]
+
+ def get_store(self, file_name, content=None):
+ 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'))
+
+ 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'))
+ store.save()
+ self.assertEquals(True, self.get_transport().has('foo.conf'))
+ modified_store = self.get_store('foo.conf')
+ 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')
+ section = store.get_mutable_section(None)
+ section.set('foo', 'bar')
+ store.save()
+ modified_store = self.get_store('foo.conf')
+ 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', '')
+ section = store.get_mutable_section(None)
+ section.set('foo', 'bar')
+ store.save()
+ modified_store = self.get_store('foo.conf')
+ 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', '')
+ section = store.get_mutable_section('baz')
+ section.set('foo', 'bar')
+ store.save()
+ modified_store = self.get_store('foo.conf')
+ sections = list(modified_store.get_sections())
+ self.assertLength(1, sections)
+ self.assertSectionContent(('baz', {'foo': 'bar'}), sections[0])
+
+
+class TestConfigObjStore(TestStore):
def test_loading_unknown_file_fails(self):
store = config.ConfigObjStore(self.get_transport(), 'I-do-not-exist')
self.assertRaises(errors.NoSuchFile, store.load)
def test_invalid_content(self):
- store = self.get_store('foo.conf', 'this is invalid !')
+ store = config.ConfigObjStore.from_string(
+ 'this is invalid !', self.get_transport(), 'foo.conf', )
self.assertEquals(False, store.loaded)
exc = self.assertRaises(errors.ParseConfigError, store.load)
self.assertEndsWith(exc.filename, 'foo.conf')
# And the load failed
self.assertEquals(False, store.loaded)
- def test_save_empty_succeeds(self):
- store = self.get_store('foo.conf', '')
- store.load()
- self.failIfExists('foo.conf')
- store.save()
- self.failUnlessExists('foo.conf')
-
- def test_save_with_content_succeeds(self):
- store = self.get_store('foo.conf', 'foo=bar\n')
- store.load()
- self.failIfExists('foo.conf')
- store.save()
- self.failUnlessExists('foo.conf')
- # FIXME: Far too ConfigObj specific
- self.assertFileEqual('foo = bar\n', 'foo.conf')
-
- def test_get_no_sections_for_empty(self):
- store = self.get_store('foo.conf', '')
- store.load()
- self.assertEquals([], list(store.get_sections()))
-
- def assertSectionContent(self, expected, section):
- """Assert that some options have the proper values in a section."""
- expected_name, expected_options = expected
- self.assertEquals(expected_name, section.id)
- self.assertEquals(
- expected_options,
- dict([(k, section.get(k)) for k in expected_options.keys()]))
-
- def test_get_default_section(self):
- store = self.get_store('foo.conf', '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')
- sections = list(store.get_sections())
- self.assertLength(1, sections)
- self.assertSectionContent(('baz', {'foo': 'bar'}), sections[0])
-
def test_get_embedded_sections(self):
# A more complicated example (which also shows that section names and
# option names share the same name space...)
- # FIXME: This is really specific to ConfigObjStore -- vila 2011-04-05
- store = self.get_store('foo.conf', '''
+ # FIXME: This should be fixed by forbidding dicts as values ?
+ # -- vila 2011-04-05
+ store = config.ConfigObjStore.from_string('''
foo=bar
l=1,2
[DEFAULT]
@@ -1986,7 +2049,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.
@@ -2002,26 +2065,18 @@
('baz', {'foo_in_baz': 'barbaz', 'qux': {'foo_in_qux': 'quux'}}),
sections[3])
- def test_set_option_in_empty_file(self):
- store = self.get_store('foo.conf')
- section = store.get_mutable_section(None)
- section.set('foo', 'bar')
- store.save()
- self.assertFileEqual('foo = bar\n', 'foo.conf')
-
- def test_set_option_in_default_section(self):
- store = self.get_store('foo.conf', '')
- section = store.get_mutable_section(None)
- section.set('foo', 'bar')
- store.save()
- self.assertFileEqual('foo = bar\n', 'foo.conf')
-
- def test_set_option_in_named_section(self):
- store = self.get_store('foo.conf', '')
- section = store.get_mutable_section('baz')
- section.set('foo', 'bar')
- store.save()
- self.assertFileEqual('[baz]\nfoo = bar\n', 'foo.conf')
+
+class TestLockableConfigObjStore(TestStore):
+
+ def test_create_store_in_created_dir(self):
+ t = self.get_transport('dir/subdir')
+ store = config.LockableConfigObjStore(t, 'foo.conf')
+ store.get_mutable_section(None).set('foo', 'bar')
+ store.save()
+
+ # FIXME: We should adapt the tests in TestLockableConfig about concurrent
+ # writes, for now, I'll just rely on using the same code (copied, but
+ # pretty trivial) -- vila 20110-04-06
class TestConfigObjStore(tests.TestCaseWithTransport):
More information about the bazaar-commits
mailing list