Merge lp:~jtv/launchpad/bug-629442 into lp:launchpad

[Jeroen T. Vermeulen]

FakeLibrarian.create returns LibraryFileAlias.

= Bug 629442 =

Fixes the bug where FakeLibrarian.create returns a LibraryFileAlias id instead of a LibraryFileAlias.

Unlike the real librarian (and I mean, why!?) this does not involve creating a LibraryFileAlias, returning an id, then querying the database for the LibraryFileAlias with that id. I just made addFile and create both use the same private method that returns the LibraryFileAlias. (This seemed a bit cleaner than having the call relationship inverted compared to the real librarian and risking problems with future changes).

To test,
{{{
./bin/test -vvc lp.testing.tests.test_fakelibrarian
}}}

No lint.

Jeroen

[Brad Crittenden (bac)]

Looks quite nice Jeroen.

review: Approve (code)

=== modified file 'lib/lp/testing/fakelibrarian.py'
--- lib/lp/testing/fakelibrarian.py	2010-08-27 11:08:27 +0000
+++ lib/lp/testing/fakelibrarian.py	2010-09-03 13:19:43 +0000
@@ -129,6 +129,11 @@
 
     def addFile(self, name, size, file, contentType, expires=None):
         """See `IFileUploadClient`."""
+        return self._storeFile(
+            name, size, file, contentType, expires=expires).id
+
+    def _storeFile(self, name, size, file, contentType, expires=None):
+        """Like `addFile`, but returns the `LibraryFileAlias`."""
         content = file.read()
         real_size = len(content)
         if real_size != size:
@@ -139,8 +144,7 @@
         file_ref = self._makeLibraryFileContent(content)
         alias = self._makeAlias(file_ref.id, name, content, contentType)
         self.aliases[alias.id] = alias
-
-        return alias.id
+        return alias
 
     def remoteAddFile(self, name, size, file, contentType, expires=None):
         """See `IFileUploadClient`."""
@@ -191,7 +195,7 @@
     def create(self, name, size, file, contentType, expires=None,
                debugID=None, restricted=False):
         "See `ILibraryFileAliasSet`."""
-        return self.addFile(name, size, file, contentType, expires=expires)
+        return self._storeFile(name, size, file, contentType, expires=expires)
 
     def __getitem__(self, key):
         "See `ILibraryFileAliasSet`."""

=== modified file 'lib/lp/testing/tests/test_fakelibrarian.py'
--- lib/lp/testing/tests/test_fakelibrarian.py	2010-08-27 11:16:39 +0000
+++ lib/lp/testing/tests/test_fakelibrarian.py	2010-09-03 13:19:43 +0000
@@ -11,7 +11,10 @@
 from transaction.interfaces import ISynchronizer
 from zope.component import getUtility
 
-from canonical.launchpad.database.librarian import LibraryFileAliasSet
+from canonical.launchpad.database.librarian import (
+    LibraryFileAlias,
+    LibraryFileAliasSet,
+    )
 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
 from canonical.launchpad.webapp.testing import verifyObject
 from canonical.librarian.client import LibrarianClient
@@ -90,12 +93,17 @@
             getUtility(ILibrarianClient).addFile,
             name, wrong_length, StringIO(text), 'text/plain')
 
+    def test_create_returns_alias(self):
+        alias = getUtility(ILibraryFileAliasSet).create(
+            'foo.txt', 3, StringIO('foo'), 'text/plain', debugID='txt')
+        self.assertIsInstance(alias, LibraryFileAlias)
+
     def test_debugID_is_harmless(self):
         # addFile takes an argument debugID that doesn't do anything
         # observable.  We get a LibraryFileAlias regardless.
-        alias_id = getUtility(ILibraryFileAliasSet).create(
+        alias = getUtility(ILibraryFileAliasSet).create(
             'txt.txt', 3, StringIO('txt'), 'text/plain', debugID='txt')
-        self.assertNotEqual(None, alias_id)
+        self.assertNotEqual(None, alias)
 
 
 class TestFakeLibrarian(LibraryAccessScenarioMixin, TestCaseWithFactory):