Bug#736260: pu: package lazr.restfulclient/0.12.0-2
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: pu
Hi. Paul Wise reported launchpadlib cache corruption in the Debian wiki
in #736259.
It looks like the concurrency improvements in lazr.restfulclient 0.13.1
helped with the issue, and he's now running a version with the patch
attached, and reports an improvement.
This seems a useful commit to backport for everyone.
SR
diff -Nru lazr.restfulclient-0.12.0/debian/changelog lazr.restfulclient-0.12.0/debian/changelog
--- lazr.restfulclient-0.12.0/debian/changelog 2012-01-23 16:59:18.000000000 +0200
+++ lazr.restfulclient-0.12.0/debian/changelog 2014-01-21 18:59:08.000000000 +0200
@@ -1,3 +1,11 @@
+lazr.restfulclient (0.12.0-2+deb7u1) stable; urgency=low
+
+ * debian/patches/concurrency_fixes_rev_122.patch: backport rev 122: Creates
+ AtomicFileCache as a parent class for MultipleRepresentationCache to
+ handle concurrent use issues. (Closes: #736259)
+
+ -- Stefano Rivera <stefanor@debian.org> Sun, 05 Jan 2014 09:02:52 +0200
+
lazr.restfulclient (0.12.0-2) unstable; urgency=low
* New maintainer.
diff -Nru lazr.restfulclient-0.12.0/debian/patches/concurrency_fixes_rev_122.patch lazr.restfulclient-0.12.0/debian/patches/concurrency_fixes_rev_122.patch
--- lazr.restfulclient-0.12.0/debian/patches/concurrency_fixes_rev_122.patch 1970-01-01 02:00:00.000000000 +0200
+++ lazr.restfulclient-0.12.0/debian/patches/concurrency_fixes_rev_122.patch 2014-01-21 18:59:46.000000000 +0200
@@ -0,0 +1,309 @@
+From: Jonathan Lange <jonathan.lange@gmail.com>
+Subject: Creates AtomicFileCache as a parent class for MultipleRepresentationCache to handle concurrent use issues.
+Bug-Ubuntu: http://bugs.launchpad.net/bugs/459418
+Bug-Debian: http://bugs.debian.org/736259
+
+--- a/src/lazr/restfulclient/_browser.py
++++ b/src/lazr/restfulclient/_browser.py
+@@ -1,4 +1,4 @@
+-# Copyright 2008 Canonical Ltd.
++# Copyright 2008,2012 Canonical Ltd.
+
+ # This file is part of lazr.restfulclient.
+ #
+@@ -29,19 +29,21 @@
+ 'RestfulHttp',
+ ]
+
+-
+ import atexit
+-import gzip
++import errno
++import os
+ import shutil
++import sys
+ import tempfile
+ # Import sleep directly into the module so we can monkey-patch it
+ # during a test.
+ from time import sleep
+ from httplib2 import (
+- FailedToDecompressContent, FileCache, Http, urlnorm)
++ Http,
++ urlnorm,
++ )
+ import simplejson
+ from cStringIO import StringIO
+-import zlib
+
+ from urllib import urlencode
+ from wadllib.application import Application
+@@ -49,6 +51,7 @@
+ from errors import error_for, HTTPError
+ from _json import DatetimeJSONEncoder
+
++
+ # A drop-in replacement for httplib2's safename.
+ from httplib2 import _md5, re_url_scheme, re_slash
+ def safename(filename):
+@@ -136,7 +139,99 @@
+ return None
+
+
+-class MultipleRepresentationCache(FileCache):
++class AtomicFileCache(object):
++ """A FileCache that can be shared by multiple processes.
++
++ Based on a patch found at
++ <http://code.google.com/p/httplib2/issues/detail?id=125>.
++ """
++
++ TEMPFILE_PREFIX = ".temp"
++
++ def __init__(self, cache, safe=safename):
++ """Construct an ``AtomicFileCache``.
++
++ :param cache: The directory to use as a cache.
++ :param safe: A function that takes a key and returns a name that's
++ safe to use as a filename. The key must never return a string
++ that begins with ``TEMPFILE_PREFIX``. By default uses
++ ``safename``.
++ """
++ self._cache_dir = os.path.normpath(cache)
++ self._get_safe_name = safe
++ try:
++ os.makedirs(self._cache_dir)
++ except OSError, e:
++ if e.errno != errno.EEXIST:
++ raise
++
++ def _get_key_path(self, key):
++ """Return the path on disk where ``key`` is stored."""
++ safe_key = self._get_safe_name(key)
++ if safe_key.startswith(self.TEMPFILE_PREFIX):
++ # If the cache key starts with the tempfile prefix, then it's
++ # possible that it will clash with a temporary file that we
++ # create.
++ raise ValueError(
++ "Cache key cannot start with '%s'" % self.TEMPFILE_PREFIX)
++ return os.path.join(self._cache_dir, safe_key)
++
++ def get(self, key):
++ """Get the value of ``key`` if set.
++
++ This behaves slightly differently to ``FileCache`` in that if
++ ``set()`` fails to store a key, this ``get()`` will behave as if that
++ key were never set whereas ``FileCache`` returns the empty string.
++
++ :param key: The key to retrieve. Must be either bytes or unicode
++ text.
++ :return: The value of ``key`` if set, None otherwise.
++ """
++ cache_full_path = self._get_key_path(key)
++ try:
++ f = open(cache_full_path, 'rb')
++ try:
++ return f.read()
++ finally:
++ f.close()
++ except (IOError, OSError), e:
++ if e.errno != errno.ENOENT:
++ raise
++
++ def set(self, key, value):
++ """Set ``key`` to ``value``.
++
++ :param key: The key to set. Must be either bytes or unicode text.
++ :param value: The value to set ``key`` to. Must be bytes.
++ """
++ # Open a temporary file
++ handle, path_name = tempfile.mkstemp(
++ prefix=self.TEMPFILE_PREFIX, dir=self._cache_dir)
++ f = os.fdopen(handle, 'wb')
++ f.write(value)
++ f.close()
++ cache_full_path = self._get_key_path(key)
++ # And rename atomically (on POSIX at least)
++ if sys.platform == 'win32' and os.path.exists(cache_full_path):
++ os.unlink(cache_full_path)
++ os.rename(path_name, cache_full_path)
++
++ def delete(self, key):
++ """Delete ``key`` from the cache.
++
++ If ``key`` has not already been set then has no effect.
++
++ :param key: The key to delete. Must be either bytes or unicode text.
++ """
++ cache_full_path = self._get_key_path(key)
++ try:
++ os.remove(cache_full_path)
++ except OSError, e:
++ if e.errno != errno.ENOENT:
++ raise
++
++
++class MultipleRepresentationCache(AtomicFileCache):
+ """A cache that can hold different representations of the same resource.
+
+ If a resource has two representations with two media types,
+--- /dev/null
++++ b/src/lazr/restfulclient/tests/test_atomicfilecache.py
+@@ -0,0 +1,158 @@
++# Copyright 2012 Canonical Ltd.
++
++# This file is part of lazr.restfulclient.
++#
++# lazr.restfulclient is free software: you can redistribute it and/or
++# modify it under the terms of the GNU Lesser General Public License
++# as published by the Free Software Foundation, version 3 of the
++# License.
++#
++# lazr.restfulclient is distributed in the hope that it will be
++# useful, but WITHOUT ANY WARRANTY; without even the implied warranty
++# of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
++# Lesser General Public License for more details.
++#
++# You should have received a copy of the GNU Lesser General Public License
++# along with lazr.restfulclient. If not, see <http://www.gnu.org/licenses/>.
++
++"""Tests for the atomic file cache."""
++
++__metaclass__ = type
++
++import shutil
++import tempfile
++import unittest
++
++import httplib2
++
++from lazr.restfulclient._browser import AtomicFileCache
++
++
++class TestFileCacheInterface(unittest.TestCase):
++ """Tests for ``AtomicFileCache``."""
++
++ file_cache_factory = httplib2.FileCache
++
++ unicode_text = u'pa\u026a\u03b8\u0259n'
++
++ def setUp(self):
++ super(TestFileCacheInterface, self).setUp()
++ self.cache_dir = tempfile.mkdtemp()
++
++ def tearDown(self):
++ shutil.rmtree(self.cache_dir)
++ super(TestFileCacheInterface, self).tearDown()
++
++ def make_file_cache(self):
++ """Make a FileCache-like object to be tested."""
++ return self.file_cache_factory(self.cache_dir)
++
++ def test_get_non_existent_key(self):
++ # get() returns None if the key does not exist.
++ cache = self.make_file_cache()
++ self.assertIs(None, cache.get('nonexistent'))
++
++ def test_set_key(self):
++ # A key set with set() can be got by get().
++ cache = self.make_file_cache()
++ cache.set('key', 'value')
++ self.assertEqual('value', cache.get('key'))
++
++ def test_set_twice_overrides(self):
++ # Setting a key again overrides the value.
++ cache = self.make_file_cache()
++ cache.set('key', 'value')
++ cache.set('key', 'new-value')
++ self.assertEqual('new-value', cache.get('key'))
++
++ def test_delete_absent_key(self):
++ # Deleting a key that's not there does nothing.
++ cache = self.make_file_cache()
++ cache.delete('nonexistent')
++ self.assertIs(None, cache.get('nonexistent'))
++
++ def test_delete_key(self):
++ # A key once set can be deleted. Further attempts to get that key
++ # return None.
++ cache = self.make_file_cache()
++ cache.set('key', 'value')
++ cache.delete('key')
++ self.assertIs(None, cache.get('key'))
++
++ def test_get_non_string_key(self):
++ # get() raises TypeError if asked to get a non-string key.
++ cache = self.make_file_cache()
++ self.assertRaises(TypeError, cache.get, 42)
++
++ def test_delete_non_string_key(self):
++ # delete() raises TypeError if asked to delete a non-string key.
++ cache = self.make_file_cache()
++ self.assertRaises(TypeError, cache.delete, 42)
++
++ def test_set_non_string_key(self):
++ # set() raises TypeError if asked to set a non-string key.
++ cache = self.make_file_cache()
++ self.assertRaises(TypeError, cache.set, 42, 'the answer')
++
++ def test_set_non_string_value(self):
++ # set() raises TypeError if asked to set a key to a non-string value.
++ # Attempts to retrieve that value return the empty string. This is
++ # probably a bug in httplib2.FileCache.
++ cache = self.make_file_cache()
++ self.assertRaises(TypeError, cache.set, 'answer', 42)
++ self.assertEqual('', cache.get('answer'))
++
++ def test_get_unicode(self):
++ # get() can retrieve unicode keys.
++ cache = self.make_file_cache()
++ self.assertIs(None, cache.get(self.unicode_text))
++
++ def test_set_unicode_keys(self):
++ cache = self.make_file_cache()
++ cache.set(self.unicode_text, 'value')
++ self.assertEqual('value', cache.get(self.unicode_text))
++
++ def test_set_unicode_value(self):
++ # set() cannot store unicode values. Values must be bytes.
++ cache = self.make_file_cache()
++ self.assertRaises(
++ UnicodeEncodeError, cache.set, 'key', self.unicode_text)
++
++ def test_delete_unicode(self):
++ # delete() can remove unicode keys.
++ cache = self.make_file_cache()
++ cache.set(self.unicode_text, 'value')
++ cache.delete(self.unicode_text)
++ self.assertIs(None, cache.get(self.unicode_text))
++
++
++class TestAtomicFileCache(TestFileCacheInterface):
++ """Tests for ``AtomicFileCache``."""
++
++ file_cache_factory = AtomicFileCache
++
++ def test_set_non_string_value(self):
++ # set() raises TypeError if asked to set a key to a non-string value.
++ # Attempts to retrieve that value act is if it were never set.
++ #
++ # Note: This behaviour differs from httplib2.FileCache.
++ cache = self.make_file_cache()
++ self.assertRaises(TypeError, cache.set, 'answer', 42)
++ self.assertIs(None, cache.get('answer'))
++
++ # Implementation-specific tests follow.
++
++ def test_bad_safename_get(self):
++ safename = lambda x: AtomicFileCache.TEMPFILE_PREFIX + x
++ cache = AtomicFileCache(self.cache_dir, safename)
++ self.assertRaises(ValueError, cache.get, 'key')
++
++ def test_bad_safename_set(self):
++ safename = lambda x: AtomicFileCache.TEMPFILE_PREFIX + x
++ cache = AtomicFileCache(self.cache_dir, safename)
++ self.assertRaises(ValueError, cache.set, 'key', 'value')
++
++ def test_bad_safename_delete(self):
++ safename = lambda x: AtomicFileCache.TEMPFILE_PREFIX + x
++ cache = AtomicFileCache(self.cache_dir, safename)
++ self.assertRaises(ValueError, cache.delete, 'key')
diff -Nru lazr.restfulclient-0.12.0/debian/patches/series lazr.restfulclient-0.12.0/debian/patches/series
--- lazr.restfulclient-0.12.0/debian/patches/series 2012-01-19 15:20:12.000000000 +0200
+++ lazr.restfulclient-0.12.0/debian/patches/series 2014-01-05 08:57:02.000000000 +0200
@@ -1,2 +1,3 @@
remove_test_requires
no_package_data.patch
+concurrency_fixes_rev_122.patch
Reply to: