launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00271
[Merge] lp:~jtv/launchpad/fake-librarian into lp:launchpad/devel
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/fake-librarian into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
= Fake Librarian =
This implements a fake librarian that we could use in tests.
The real librarian runs as a separate daemon that needs to be started and managed during test runs, taken up and down in some places (which takes ages), and sometimes manually killed in order to get things running again. It also writes files to the filesystem. In other words, it's lots of stuff we'd rather not have too much of in test runs.
In this branch I propose a simple fake librarian that could replace the real thing in many tests. It runs purely in-process and stores non-persistently, so it has none of that stuff that we don't like. Of course that also means it won't work for tests that fire off sub-processes.
To splice the fake librarian into a test's environment, e.g. in a cut-down LaunchpadLayer, we would ideally:
* Create one global FakeLibrarian object.
* Override the ILibrarianClient utility with the fake librarian.
* Override the ILibraryFileAliasSet utility with the fake librarian.
* Register the fake librarian as a transaction synchronizer.
The test would then use the fake librarian instead of the real one.
URLs and remote addition of files are not supported for now, so the fake librarian would be of little use in browser tests for now.
You'll also note that the fake librarian keeps track of whether a file has been committed to storage. This is an idiosyncrasy of the real librarian: any newly added files remain inaccessible until you commit. Stuart noted that it would be best to simulate the real librarian's behaviour here so that we don't hide nasty transaction-boundary bugs. Unlike the real librarian the fake one gives a reasonably helpful error message in this case. You'll also have the option of simulating a commit without actually forcing one on the test database.
The fake librarian uses the real database objects, just like the original. This means that it should still work when application code goes straight to the database, e.g. by joining or prejoining LibraryFileAlias and/or LibraryFileContent.
At the time of writing I'm sure we'll still want to play with this a bit before considering it for use in our test suite. I'm filing this as a Work-in-Progress MP.
Test with:
{{{
./bin/test -vvc -m lp.testing.tests.test_fakelibrarian
}}}
No lint.
Jeroen
--
https://code.launchpad.net/~jtv/launchpad/fake-librarian/+merge/30907
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/fake-librarian into lp:launchpad/devel.
=== modified file 'lib/canonical/librarian/interfaces.py'
--- lib/canonical/librarian/interfaces.py 2010-04-20 14:27:26 +0000
+++ lib/canonical/librarian/interfaces.py 2010-07-26 00:01:46 +0000
@@ -1,9 +1,17 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# PyLint doesn't grok Zope interfaces.
# pylint: disable-msg=E0213
__metaclass__ = type
+__all__ = [
+ 'DownloadFailed',
+ 'ILibrarianClient',
+ 'IRestrictedLibrarianClient',
+ 'LibrarianServerError',
+ 'LIBRARIAN_SERVER_DEFAULT_TIMEOUT',
+ 'UploadFailed',
+ ]
import signal
@@ -35,14 +43,18 @@
# LibrarianServerError.
LIBRARIAN_SERVER_DEFAULT_TIMEOUT = 5
+
class IFileUploadClient(Interface):
+ """Upload API for the Librarian client."""
+
def addFile(name, size, file, contentType, expires=None):
"""Add a file to the librarian.
- :param name: Name to store the file as
- :param size: Size of the file
- :param file: File-like object with the content in it
- :param expires: Expiry time of file, or None to keep until unreferenced
+ :param name: Name to store the file as.
+ :param size: Size of the file.
+ :param file: File-like object with the content in it.
+ :param expires: Expiry time of file, or None to keep until
+ unreferenced.
:raises UploadFailed: If the server rejects the upload for some reason
@@ -74,6 +86,8 @@
class IFileDownloadClient(Interface):
+ """Download API for the Librarian client."""
+
def getURLForAlias(aliasID):
"""Returns the URL to the given file"""
@@ -87,7 +101,7 @@
LibrarianServerError is raised.
:return: A file-like object to read the file contents from.
:raises DownloadFailed: If the alias is not found.
- :raises LibrarianServerError: If the librarain server is
+ :raises LibrarianServerError: If the librarian server is
unreachable or returns an 5xx HTTPError.
"""
@@ -98,4 +112,3 @@
class IRestrictedLibrarianClient(ILibrarianClient):
"""A version of the client that connects to a restricted librarian."""
-
=== modified file 'lib/canonical/librarian/storage.py'
--- lib/canonical/librarian/storage.py 2010-02-09 00:17:40 +0000
+++ lib/canonical/librarian/storage.py 2010-07-26 00:01:46 +0000
@@ -15,11 +15,18 @@
IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
from canonical.librarian.db import write_transaction
-__all__ = ['DigestMismatchError', 'LibrarianStorage', 'LibraryFileUpload',
- 'DuplicateFileIDError', 'WrongDatabaseError',
- # _relFileLocation needed by other modules in this package.
- # Listed here to keep the import facist happy
- '_relFileLocation', '_sameFile']
+__all__ = [
+ 'DigestMismatchError',
+ 'LibrarianStorage',
+ 'LibraryFileUpload',
+ 'DuplicateFileIDError',
+ 'WrongDatabaseError',
+ # _relFileLocation needed by other modules in this package.
+ # Listed here to keep the import facist happy
+ '_relFileLocation',
+ '_sameFile',
+ ]
+
class DigestMismatchError(Exception):
"""The given digest doesn't match the SHA-1 digest of the file."""
@@ -31,6 +38,7 @@
class WrongDatabaseError(Exception):
"""The client's database name doesn't match our database."""
+
def __init__(self, clientDatabaseName, serverDatabaseName):
Exception.__init__(self, clientDatabaseName, serverDatabaseName)
self.clientDatabaseName = clientDatabaseName
@@ -40,8 +48,8 @@
class LibrarianStorage:
"""Blob storage.
- This manages the actual storage of files on disk and the record of those in
- the database; it has nothing to do with the network interface to those
+ This manages the actual storage of files on disk and the record of those
+ in the database; it has nothing to do with the network interface to those
files.
"""
@@ -109,11 +117,11 @@
# the file really is removed or renamed, and can't possibly be
# left in limbo
os.remove(self.tmpfilepath)
- raise DigestMismatchError, (self.srcDigest, dstDigest)
+ raise DigestMismatchError(self.srcDigest, dstDigest)
try:
- # If the client told us the name database of the database
- # its using, check that it matches
+ # If the client told us the name of the database it's using,
+ # check that it matches.
if self.databaseName is not None:
store = getUtility(IStoreSelector).get(
MAIN_STORE, DEFAULT_FLAVOR)
@@ -122,7 +130,8 @@
if self.databaseName != databaseName:
raise WrongDatabaseError(self.databaseName, databaseName)
- self.debugLog.append('database name %r ok' % (self.databaseName,))
+ self.debugLog.append(
+ 'database name %r ok' % (self.databaseName, ))
# If we haven't got a contentID, we need to create one and return
# it to the client.
if self.contentID is None:
@@ -135,7 +144,7 @@
else:
contentID = self.contentID
aliasID = None
- self.debugLog.append('received contentID: %r' % (contentID,))
+ self.debugLog.append('received contentID: %r' % (contentID, ))
except:
# Abort transaction and re-raise
=== added file 'lib/lp/testing/fakelibrarian.py'
--- lib/lp/testing/fakelibrarian.py 1970-01-01 00:00:00 +0000
+++ lib/lp/testing/fakelibrarian.py 2010-07-26 00:01:46 +0000
@@ -0,0 +1,139 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Fake, in-process implementation of the Librarian API."""
+
+__metaclass__ = type
+__all__ = [
+ 'FakeLibrarian',
+ ]
+
+import hashlib
+from StringIO import StringIO
+from transaction.interfaces import ISynchronizer
+
+from zope.interface import implements
+
+from canonical.launchpad.database.librarian import (
+ LibraryFileContent, LibraryFileAlias)
+from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
+from canonical.librarian.interfaces import (
+ DownloadFailed,
+ ILibrarianClient,
+ LIBRARIAN_SERVER_DEFAULT_TIMEOUT,
+ UploadFailed)
+
+
+class InstrumentedLibraryFileAlias(LibraryFileAlias):
+ """A `ILibraryFileAlias` implementation that fakes library access."""
+
+ file_committed = False
+
+ def checkCommitted(self):
+ """Raise an error if this file has not been committed yet."""
+ if not self.file_committed:
+ raise DownloadFailed(
+ "Attempting to retrieve file '%s' from the fake "
+ "librarian, but the file has not yet been committed to "
+ "storage." % self.filename)
+
+ def open(self, timeout=LIBRARIAN_SERVER_DEFAULT_TIMEOUT):
+ self.checkCommitted()
+ self._datafile = StringIO(self.content_string)
+
+ def read(self, chunksize=None, timeout=LIBRARIAN_SERVER_DEFAULT_TIMEOUT):
+ return self._datafile.read(chunksize)
+
+
+class FakeLibrarian(object):
+ """A fake, in-process Librarian.
+
+ This takes the role of both the librarian client and the LibraryFileAlias
+ utility.
+ """
+ implements(
+ ILibrarianClient, ILibraryFileAliasSet, ISynchronizer)
+
+ def __init__(self):
+ self.aliases = {}
+
+ def addFile(self, name, size, file, content_type, expires=None):
+ """See `IFileUploadClient`."""
+ content = file.read()
+ real_size = len(content)
+ if real_size != size:
+ raise UploadFailed(
+ "Uploading '%s' to the fake librarian with incorrect "
+ "size %d; actual size is %d." % (name, size, real_size))
+
+ file_ref = self._makeLibraryFileContent(content)
+ alias = self._makeAlias(file_ref.id, name, content, content_type)
+ self.aliases[alias.id] = alias
+
+ return alias.id
+
+ def remoteAddFile(self, name, size, file, contentType, expires=None):
+ """See `IFileUploadClient`."""
+ return NotImplementedError()
+
+ def getURLForAlias(self, aliasID):
+ """See `IFileDownloadClient`."""
+ raise NotImplementedError()
+
+ def getFileByAlias(self, aliasID,
+ timeout=LIBRARIAN_SERVER_DEFAULT_TIMEOUT):
+ """See `IFileDownloadClient`."""
+ alias = self[aliasID]
+ alias.checkCommitted()
+ return StringIO(alias.content_string)
+
+ def _makeAlias(self, file_id, name, content, content_type):
+ """Create a `LibraryFileAlias`."""
+ alias = InstrumentedLibraryFileAlias(
+ contentID=file_id, filename=name, mimetype=content_type)
+ alias.content_string = content
+ return alias
+
+ def _makeLibraryFileContent(self, content):
+ """Create a `LibraryFileContent`."""
+ size = len(content)
+ sha1 = hashlib.sha1(content).hexdigest()
+ md5 = hashlib.md5(content).hexdigest()
+
+ content_object = LibraryFileContent(filesize=size, sha1=sha1, md5=md5)
+ return content_object
+
+ def create(self, name, size, file, contentType, expires=None,
+ debugID=None, restricted=False):
+ "See `ILibraryFileAliasSet`."""
+ return self.addFile(
+ name, size, file, contentType, expires=expires, debugID=debugID)
+
+ def __getitem__(self, key):
+ "See `ILibraryFileAliasSet`."""
+ alias = self.aliases.get(key)
+ if alias is None:
+ raise DownloadFailed(
+ "Attempting to retrieve file alias %d from the fake "
+ "librarian, who has never heard of it." % key)
+ return alias
+
+ def findBySHA1(self, sha1):
+ "See `ILibraryFileAliasSet`."""
+ for alias in self.aliases.itervalues():
+ if alias.content.sha1 == sha1:
+ return alias
+
+ return None
+
+ def beforeCompletion(self, txn):
+ """See `ISynchronizer`."""
+
+ def afterCompletion(self, txn):
+ """See `ISynchronizer`."""
+ # Note that all files have been committed to storage.
+ for alias in self.aliases.itervalues():
+ alias.file_committed = True
+
+ def newTransaction(self, txn):
+ """See `ISynchronizer`."""
=== added file 'lib/lp/testing/tests/test_fakelibrarian.py'
--- lib/lp/testing/tests/test_fakelibrarian.py 1970-01-01 00:00:00 +0000
+++ lib/lp/testing/tests/test_fakelibrarian.py 2010-07-26 00:01:46 +0000
@@ -0,0 +1,86 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test the fake librarian."""
+
+__metaclass__ = type
+
+from StringIO import StringIO
+import transaction
+from transaction.interfaces import ISynchronizer
+
+from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
+from canonical.librarian.interfaces import (
+ DownloadFailed, ILibrarianClient, UploadFailed)
+from canonical.launchpad.webapp.testing import verifyObject
+from canonical.testing import DatabaseFunctionalLayer
+from lp.testing import TestCaseWithFactory
+from lp.testing.fakelibrarian import FakeLibrarian
+
+
+class TestFakeLibrarian(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestFakeLibrarian, self).setUp()
+ self.librarian = FakeLibrarian()
+ transaction.manager.registerSynch(self.librarian)
+
+ def tearDown(self):
+ super(TestFakeLibrarian, self).tearDown()
+ transaction.manager.unregisterSynch(self.librarian)
+
+ def _storeFile(self):
+ """Store a file in the `FakeLibrarian`.
+
+ :return: Tuple of filename, file contents, alias id.
+ """
+ name = self.factory.getUniqueString()
+ text = self.factory.getUniqueString()
+ alias_id = self.librarian.addFile(
+ name, len(text), StringIO(text), 'text/plain')
+ return name, text, alias_id
+
+ def test_baseline(self):
+ self.assertTrue(verifyObject(ILibrarianClient, self.librarian))
+ self.assertTrue(verifyObject(ILibraryFileAliasSet, self.librarian))
+ self.assertTrue(verifyObject(ISynchronizer, self.librarian))
+
+ def test_insert_retrieve(self):
+ name, text, alias_id = self._storeFile()
+ self.assertIsInstance(alias_id, (int, long))
+
+ transaction.commit()
+
+ self.assertEqual(text, self.librarian.getFileByAlias(alias_id).read())
+
+ def test_alias_set(self):
+ name, text, alias_id = self._storeFile()
+
+ retrieved_alias = self.librarian[alias_id]
+
+ self.assertEqual(alias_id, retrieved_alias.id)
+ self.assertEqual(name, retrieved_alias.filename)
+
+ def test_read(self):
+ name, text, alias_id = self._storeFile()
+ transaction.commit()
+
+ retrieved_alias = self.librarian[alias_id]
+ retrieved_alias.open()
+ self.assertEqual(text, retrieved_alias.read())
+
+ def test_uncommitted_file(self):
+ name, text, alias_id = self._storeFile()
+ retrieved_alias = self.librarian[alias_id]
+ self.assertRaises(DownloadFailed, retrieved_alias.open)
+
+ def test_incorrect_upload_size(self):
+ name = self.factory.getUniqueString()
+ text = self.factory.getUniqueString()
+ wrong_length = len(text) + 1
+ self.assertRaises(
+ UploadFailed,
+ self.librarian.addFile,
+ name, wrong_length, StringIO(text), 'text/plain')
