Merge lp:~thumper/launchpad/permission-fail into lp:launchpad/db-devel

[Tim Penhey]

[Tim Penhey (thumper)]

Fix a permission bug.

[Tim Penhey (thumper)]

Added the test now too.

[Michael Hudson-Doyle (mwhudson)]

Looks fine.

review: Approve

[Francis J. Lacoste (flacoste)]

Since this is actually a DB change and the re-roll won't update the DB, and the permission has been manually granted, I don't think we should land this change "release-critical".

review: Disapprove (release-critical)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2009-10-01 01:18:55 +0000
+++ database/schema/security.cfg	2009-10-01 11:45:25 +0000
@@ -1482,6 +1482,7 @@
 public.distroseries                     = SELECT
 public.job                              = SELECT, INSERT, UPDATE
 public.mergedirectivejob                = SELECT, INSERT
+public.previewdiff                      = SELECT
 public.staticdiff                       = SELECT, INSERT, UPDATE
 public.sourcepackagename                = SELECT
 public.seriessourcepackagebranch        = SELECT

=== modified file 'lib/canonical/librarian/ftests/test_gc.py'
--- lib/canonical/librarian/ftests/test_gc.py	2009-09-26 09:06:13 +0000
+++ lib/canonical/librarian/ftests/test_gc.py	2009-10-01 11:45:25 +0000
@@ -5,6 +5,7 @@
 
 __metaclass__ = type
 
+import shutil
 import sys
 import os
 from subprocess import Popen, PIPE, STDOUT
@@ -15,6 +16,7 @@
 from pytz import utc
 from sqlobject import SQLObjectNotFound
 from storm.locals import SQL, AutoReload
+import transaction
 from zope.component import getUtility
 
 from canonical.config import config
@@ -529,37 +531,59 @@
         # Long non-hexadecimal number
         noisedir3_path = os.path.join(config.librarian_server.root, '11.bak')
 
-        os.mkdir(noisedir1_path)
-        os.mkdir(noisedir2_path)
-        os.mkdir(noisedir3_path)
-
-        # Files in the noise directories.
-        noisefile1_path = os.path.join(noisedir1_path, 'abc')
-        noisefile2_path = os.path.join(noisedir2_path, 'def')
-        noisefile3_path = os.path.join(noisedir2_path, 'ghi')
-        open(noisefile1_path, 'w').write('hello')
-        open(noisefile2_path, 'w').write('there')
-        open(noisefile3_path, 'w').write('testsuite')
-
-        # Pretend it is tomorrow to ensure the files don't count as
-        # recently created, and run the delete_unwanted_files process.
-        org_time = librariangc.time
-        def tomorrow_time():
-            return org_time() + 24 * 60 * 60 + 1
         try:
-            librariangc.time = tomorrow_time
-            librariangc.delete_unwanted_files(self.con)
+            os.mkdir(noisedir1_path)
+            os.mkdir(noisedir2_path)
+            os.mkdir(noisedir3_path)
+
+            # Files in the noise directories.
+            noisefile1_path = os.path.join(noisedir1_path, 'abc')
+            noisefile2_path = os.path.join(noisedir2_path, 'def')
+            noisefile3_path = os.path.join(noisedir2_path, 'ghi')
+            open(noisefile1_path, 'w').write('hello')
+            open(noisefile2_path, 'w').write('there')
+            open(noisefile3_path, 'w').write('testsuite')
+
+            # Pretend it is tomorrow to ensure the files don't count as
+            # recently created, and run the delete_unwanted_files process.
+            org_time = librariangc.time
+            def tomorrow_time():
+                return org_time() + 24 * 60 * 60 + 1
+            try:
+                librariangc.time = tomorrow_time
+                librariangc.delete_unwanted_files(self.con)
+            finally:
+                librariangc.time = org_time
+
+            # None of the rubbish we created has been touched.
+            self.assert_(os.path.isdir(noisedir1_path))
+            self.assert_(os.path.isdir(noisedir2_path))
+            self.assert_(os.path.isdir(noisedir3_path))
+            self.assert_(os.path.exists(noisefile1_path))
+            self.assert_(os.path.exists(noisefile2_path))
+            self.assert_(os.path.exists(noisefile3_path))
         finally:
-            librariangc.time = org_time
-
-        # None of the rubbish we created has been touched.
-        self.assert_(os.path.isdir(noisedir1_path))
-        self.assert_(os.path.isdir(noisedir2_path))
-        self.assert_(os.path.isdir(noisedir3_path))
-        self.assert_(os.path.exists(noisefile1_path))
-        self.assert_(os.path.exists(noisefile2_path))
-        self.assert_(os.path.exists(noisefile3_path))
-
+            # We need to clean this up ourselves, as the standard librarian
+            # cleanup only removes files it knows where valid to avoid
+            # accidents.
+            shutil.rmtree(noisedir1_path)
+            shutil.rmtree(noisedir2_path)
+            shutil.rmtree(noisedir3_path)
+
+    def test_delete_unwanted_files_bug437084(self):
+        # There was a bug where delete_unwanted_files() would die
+        # if the last file found on disk was unwanted.
+        self.layer.switchDbUser(dbuser='testadmin')
+        content='foo'
+        self.client.addFile(
+            'foo.txt', len(content), StringIO(content), 'text/plain')
+        # Roll back the database changes, leaving the file on disk.
+        transaction.abort()
+
+        self.layer.switchDbUser(config.librarian_gc.dbuser)
+
+        # This should cope.
+        librariangc.delete_unwanted_files(self.con)
 
     def test_cronscript(self):
         script_path = os.path.join(

=== modified file 'lib/canonical/librarian/librariangc.py'
--- lib/canonical/librarian/librariangc.py	2009-09-26 08:43:07 +0000
+++ lib/canonical/librarian/librariangc.py	2009-10-01 11:45:25 +0000
@@ -578,6 +578,7 @@
                 next_wanted_content_id = get_next_wanted_content_id()
 
                 if (config.librarian_server.upstream_host is None
+                        and next_wanted_content_id is not None
                         and next_wanted_content_id < content_id):
                     log.error(
                         "LibraryFileContent %d exists in the database but "

=== modified file 'lib/lp/code/mail/tests/test_codehandler.py'
--- lib/lp/code/mail/tests/test_codehandler.py	2009-09-15 14:32:47 +0000
+++ lib/lp/code/mail/tests/test_codehandler.py	2009-10-01 11:45:24 +0000
@@ -5,6 +5,7 @@
 
 __metaclass__ = type
 
+from difflib import unified_diff
 from textwrap import dedent
 import transaction
 import unittest
@@ -30,6 +31,7 @@
 from canonical.launchpad.database import MessageSet
 from lp.code.model.branchmergeproposaljob import (
     CreateMergeProposalJob, MergeProposalCreatedJob)
+from lp.code.model.diff import PreviewDiff
 from canonical.launchpad.interfaces.mail import (
     EmailProcessingError, IWeaklyAuthenticatedPrincipal)
 from lp.code.mail.codehandler import (
@@ -590,6 +592,25 @@
         # Ensure the DB operations violate no constraints.
         transaction.commit()
 
+    def test_reviewer_with_diff(self):
+        """Requesting a review with a diff works."""
+        diff_text = ''.join(unified_diff('', 'Fake diff'))
+        preview_diff = PreviewDiff.create(
+            diff_text,
+            unicode(self.factory.getUniqueString('revid')),
+            unicode(self.factory.getUniqueString('revid')),
+            None, None)
+        # To record the diff in the librarian.
+        transaction.commit()
+        bmp = self.factory.makeBranchMergeProposal(preview_diff=preview_diff)
+        eric = self.factory.makePerson(name="eric", email="[e-mail]")
+        mail = self.factory.makeSignedMessage(body=' reviewer eric')
+        email_addr = bmp.address
+        self.switchDbUser(config.processmail.dbuser)
+        self.code_handler.process(mail, email_addr, None)
+        [vote] = bmp.votes
+        self.assertEqual(eric, vote.reviewer)
+
     def test_processMergeProposalDefaultReviewer(self):
         # If no reviewer was requested in the comment body, then the default
         # reviewer of the target branch is used.