Rev 6425: (vila) Provide Store.save_changes() to allow configuration store to be saved in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/
Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Jan 5 10:39:50 UTC 2012
At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 6425 [merge]
revision-id: pqm at pqm.ubuntu.com-20120105103949-xxe5dkbfiu16q4ch
parent: pqm at pqm.ubuntu.com-20120104175752-l7142x10qoczkr30
parent: v.ladeuil+lp at free.fr-20120105093929-aqwc487n1yisq3wi
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2012-01-05 10:39:49 +0000
message:
(vila) Provide Store.save_changes() to allow configuration store to be saved
incrementally. (Vincent Ladeuil)
modified:
bzrlib/config.py config.py-20051011043216-070c74f4e9e338e8
bzrlib/tests/test_config.py testconfig.py-20051011041908-742d0c15d8d8c8eb
doc/en/release-notes/bzr-2.5.txt bzr2.5.txt-20110708125756-587p0hpw7oke4h05-1
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2012-01-03 12:56:06 +0000
+++ b/bzrlib/config.py 2012-01-05 09:39:29 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005-2011 Canonical Ltd
+# Copyright (C) 2005-2012 Canonical Ltd
# Authors: Robert Collins <robert.collins at canonical.com>
# and others
#
@@ -2886,6 +2886,8 @@
_NewlyCreatedOption = object()
"""Was the option created during the MutableSection lifetime"""
+_DeletedOption = object()
+"""Was the option deleted during the MutableSection lifetime"""
class MutableSection(Section):
@@ -2893,7 +2895,7 @@
def __init__(self, section_id, options):
super(MutableSection, self).__init__(section_id, options)
- self.orig = {}
+ self.reset_changes()
def set(self, name, value):
if name not in self.options:
@@ -2908,6 +2910,48 @@
self.orig[name] = self.get(name, None)
del self.options[name]
+ def reset_changes(self):
+ self.orig = {}
+
+ def apply_changes(self, dirty, store):
+ """Apply option value changes.
+
+ ``self`` has been reloaded from the persistent storage. ``dirty``
+ contains the changes made since the previous loading.
+
+ :param dirty: the mutable section containing the changes.
+
+ :param store: the store containing the section
+ """
+ for k, expected in dirty.orig.iteritems():
+ actual = dirty.get(k, _DeletedOption)
+ reloaded = self.get(k, _NewlyCreatedOption)
+ if actual is _DeletedOption:
+ if k in self.options:
+ self.remove(k)
+ else:
+ self.set(k, actual)
+ # Report concurrent updates in an ad-hoc way. This should only
+ # occurs when different processes try to update the same option
+ # which is not supported (as in: the config framework is not meant
+ # to be used a sharing mechanism).
+ if expected != reloaded:
+ if actual is _DeletedOption:
+ actual = '<DELETED>'
+ if reloaded is _NewlyCreatedOption:
+ reloaded = '<CREATED>'
+ if expected is _NewlyCreatedOption:
+ expected = '<CREATED>'
+ # Someone changed the value since we get it from the persistent
+ # storage.
+ trace.warning(gettext(
+ "Option {0} in section {1} of {2} was changed"
+ " from {3} to {4}. The {5} value will be saved.".format(
+ k, self.id, store.external_url(), expected,
+ reloaded, actual)))
+ # No need to keep track of these changes
+ self.reset_changes()
+
class Store(object):
"""Abstract interface to persistent storage for configuration options."""
@@ -2915,6 +2959,10 @@
readonly_section_class = Section
mutable_section_class = MutableSection
+ def __init__(self):
+ # Which sections need to be saved
+ self.dirty_sections = []
+
def is_loaded(self):
"""Returns True if the Store has been loaded.
@@ -2961,6 +3009,41 @@
"""Saves the Store to persistent storage."""
raise NotImplementedError(self.save)
+ def _need_saving(self):
+ for s in self.dirty_sections:
+ if s.orig:
+ # At least one dirty section contains a modification
+ return True
+ return False
+
+ def apply_changes(self, dirty_sections):
+ """Apply changes from dirty sections while checking for coherency.
+
+ The Store content is discarded and reloaded from persistent storage to
+ acquire up-to-date values.
+
+ Dirty sections are MutableSection which kept track of the value they
+ are expected to update.
+ """
+ # We need an up-to-date version from the persistent storage, unload the
+ # store. The reload will occur when needed (triggered by the first
+ # get_mutable_section() call below.
+ self.unload()
+ # Apply the changes from the preserved dirty sections
+ for dirty in dirty_sections:
+ clean = self.get_mutable_section(dirty.id)
+ clean.apply_changes(dirty, self)
+ # Everything is clean now
+ self.dirty_sections = []
+
+ def save_changes(self):
+ """Saves the Store to persistent storage if changes occurred.
+
+ Apply the changes recorded in the mutable sections to a store content
+ refreshed from persistent storage.
+ """
+ raise NotImplementedError(self.save_changes)
+
def external_url(self):
raise NotImplementedError(self.external_url)
@@ -3041,6 +3124,7 @@
def unload(self):
self._config_obj = None
+ self.dirty_sections = []
def _load_content(self):
"""Load the config file bytes.
@@ -3087,6 +3171,19 @@
except UnicodeDecodeError:
raise errors.ConfigContentError(self.external_url())
+ def save_changes(self):
+ if not self.is_loaded():
+ # Nothing to save
+ return
+ if not self._need_saving():
+ return
+ # Preserve the current version
+ current = self._config_obj
+ dirty_sections = list(self.dirty_sections)
+ self.apply_changes(dirty_sections)
+ # Save to the persistent storage
+ self.save()
+
def save(self):
if not self.is_loaded():
# Nothing to save
@@ -3127,7 +3224,10 @@
section = self._config_obj
else:
section = self._config_obj.setdefault(section_id, {})
- return self.mutable_section_class(section_id, section)
+ mutable_section = self.mutable_section_class(section_id, section)
+ # All mutable sections can become dirty
+ self.dirty_sections.append(mutable_section)
+ return mutable_section
def quote(self, value):
try:
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2012-01-04 11:09:24 +0000
+++ b/bzrlib/tests/test_config.py 2012-01-05 10:39:49 +0000
@@ -3000,6 +3000,100 @@
self.assertLength(1, calls)
self.assertEquals((store,), calls[0])
+ def test_set_mark_dirty(self):
+ stack = config.MemoryStack('')
+ self.assertLength(0, stack.store.dirty_sections)
+ stack.set('foo', 'baz')
+ self.assertLength(1, stack.store.dirty_sections)
+ self.assertTrue(stack.store._need_saving())
+
+ def test_remove_mark_dirty(self):
+ stack = config.MemoryStack('foo=bar')
+ self.assertLength(0, stack.store.dirty_sections)
+ stack.remove('foo')
+ self.assertLength(1, stack.store.dirty_sections)
+ self.assertTrue(stack.store._need_saving())
+
+
+class TestStoreSaveChanges(tests.TestCaseWithTransport):
+ """Tests that config changes are kept in memory and saved on-demand."""
+
+ def setUp(self):
+ super(TestStoreSaveChanges, self).setUp()
+ self.transport = self.get_transport()
+ # Most of the tests involve two stores pointing to the same persistent
+ # storage to observe the effects of concurrent changes
+ self.st1 = config.TransportIniFileStore(self.transport, 'foo.conf')
+ self.st2 = config.TransportIniFileStore(self.transport, 'foo.conf')
+ self.warnings = []
+ def warning(*args):
+ self.warnings.append(args[0] % args[1:])
+ self.overrideAttr(trace, 'warning', warning)
+
+ def has_store(self, store):
+ store_basename = urlutils.relative_url(self.transport.external_url(),
+ store.external_url())
+ return self.transport.has(store_basename)
+
+ def get_stack(self, store):
+ # Any stack will do as long as it uses the right store, just a single
+ # no-name section is enough
+ return config.Stack([store.get_sections], store)
+
+ def test_no_changes_no_save(self):
+ s = self.get_stack(self.st1)
+ s.store.save_changes()
+ self.assertEquals(False, self.has_store(self.st1))
+
+ def test_unrelated_concurrent_update(self):
+ s1 = self.get_stack(self.st1)
+ s2 = self.get_stack(self.st2)
+ s1.set('foo', 'bar')
+ s2.set('baz', 'quux')
+ s1.store.save()
+ # Changes don't propagate magically
+ self.assertEquals(None, s1.get('baz'))
+ s2.store.save_changes()
+ self.assertEquals('quux', s2.get('baz'))
+ # Changes are acquired when saving
+ self.assertEquals('bar', s2.get('foo'))
+ # Since there is no overlap, no warnings are emitted
+ self.assertLength(0, self.warnings)
+
+ def test_concurrent_update_modified(self):
+ s1 = self.get_stack(self.st1)
+ s2 = self.get_stack(self.st2)
+ s1.set('foo', 'bar')
+ s2.set('foo', 'baz')
+ s1.store.save()
+ # Last speaker wins
+ s2.store.save_changes()
+ self.assertEquals('baz', s2.get('foo'))
+ # But the user get a warning
+ self.assertLength(1, self.warnings)
+ warning = self.warnings[0]
+ self.assertStartsWith(warning, 'Option foo in section None')
+ self.assertEndsWith(warning, 'was changed from <CREATED> to bar.'
+ ' The baz value will be saved.')
+
+ def test_concurrent_deletion(self):
+ self.st1._load_from_string('foo=bar')
+ self.st1.save()
+ s1 = self.get_stack(self.st1)
+ s2 = self.get_stack(self.st2)
+ s1.remove('foo')
+ s2.remove('foo')
+ s1.store.save_changes()
+ # No warning yet
+ self.assertLength(0, self.warnings)
+ s2.store.save_changes()
+ # Now we get one
+ self.assertLength(1, self.warnings)
+ warning = self.warnings[0]
+ self.assertStartsWith(warning, 'Option foo in section None')
+ self.assertEndsWith(warning, 'was changed from bar to <CREATED>.'
+ ' The <DELETED> value will be saved.')
+
class TestQuotingIniFileStore(tests.TestCaseWithTransport):
=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- a/doc/en/release-notes/bzr-2.5.txt 2012-01-04 16:02:14 +0000
+++ b/doc/en/release-notes/bzr-2.5.txt 2012-01-05 10:39:49 +0000
@@ -142,6 +142,11 @@
* Configuration options can be SI units by using ``int_SI_from_unicode`` as
their ``convert_from_unicode`` helper. (Vincent Ladeuil)
+* Configuration stores can now save incremental changes by using
+ ``save_changes()`` instead of ``save()``. This reduces the number or
+ required input/outputs and allows stores to be shared between
+ stacks. (Vincent Ladeuil)
+
* ControlDir now has a get_branches method that returns a dictionary
whose keys are the names of the branches and whose values are the
branches themselves. The active branch uses the key None.
More information about the bazaar-commits
mailing list