Yet another fix to the mail gateway...

...messages got *all* files of an issue, not just the new ones. Thanks
to Rafal Bisingier for reporting and proposing a fix. The regression
test was updated.  Fix version numbers in upgrade documentation, the
file-unlink defect was in 1.4.17 not 1.4.16. Thanks to Rafal
Bisingier.

Author: Ralf Schlatterbeck

CHANGES.txt

@@ -2,11 +2,22 @@ This file contains the changes to the Roundup system over time. The entries
 are given with the most recent entry first. If no other name is given,
 Richard Jones did the change.
 
+2011-XX-XX 1.4.19 (r46XX)
+
+Fixed:
+
+- Yet another fix to the mail gateway, messages got *all* files of
+  an issue, not just the new ones. Thanks to Rafal Bisingier for
+  reporting and proposing a fix. The regression test was updated.
+  (Ralf)
+- Fix version numbers in upgrade documentation, the file-unlink defect
+  was in 1.4.17 not 1.4.16. Thanks to Rafal Bisingier.
+
 2011-05-29 1.4.18 (r4610)
 
 Features:
 
-- Norwegian Bokmal translation by Christian Aastorp (Ralf)
+- Norwegian Bokmal translation by Christian Aastorp
 - Allow to specify additional cc and bcc emails (not roundup users) for
   nosymessage used by the nosyreaction reactor. (Ralf)
 

doc/upgrading.txt

@@ -13,14 +13,14 @@ steps.
 
 .. contents::
 
-Migrating from 1.4.16 to 1.4.17
+Migrating from 1.4.17 to 1.4.18
 ===============================
 
-There was a bug in 1.4.16 where files were unlinked from issues if a
+There was a bug in 1.4.17 where files were unlinked from issues if a
 mail without attachment was received via the mail interface. The
 following script will list likely issues being affected by the bug.
-The date in the script is the date of the 1.4.16 release. If you have
-installed 1.4.16 later than this date, you can change the date
+The date in the script is the date of the 1.4.17 release. If you have
+installed 1.4.17 later than this date, you can change the date
 appropriately to your installation date. Run the script in the directory
 of your tracker.
 

roundup/mailgw.py

@@ -549,6 +549,7 @@ def __init__(self, mailgw, message):
         self.nodeid = None
         self.author = None
         self.recipients = None
+        self.msg_props = {}
         self.props = None
         self.content = None
         self.attachments = None
@@ -1052,6 +1053,7 @@ def create_files(self):
                     'You are not permitted to add files to %(classname)s.'
                     ) % self.__dict__
 
+            self.msg_props['files'] = files
             if self.nodeid:
                 # extend the existing files list
                 fileprop = self.cl.get(self.nodeid, 'files')
@@ -1066,6 +1068,7 @@ def create_msg(self):
         if not self.properties.has_key('messages'):
             return
         msg_props = self.mailgw.get_class_arguments('msg')
+        self.msg_props.update (msg_props)
 
         # Get the message ids
         inreplyto = self.message.getheader('in-reply-to') or ''
@@ -1094,8 +1097,7 @@ def create_msg(self):
                 message_id = self.db.msg.create(author=self.author,
                     recipients=self.recipients, date=date.Date('.'),
                     summary=summary, content=content,
-                    files=self.props.get('files',[]),
-                    messageid=messageid, inreplyto=inreplyto, **msg_props)
+                    messageid=messageid, inreplyto=inreplyto, **self.msg_props)
             except exceptions.Reject, error:
                 raise MailUsageError, _("""
 Mail message was rejected by a detector.

test/test_mailgw.py

@@ -625,15 +625,47 @@ def testMultipartKeepAlternatives(self):
                 self.assertEqual(f.content, content [n])
         self.assertEqual(msg.content, 'test attachment second text/plain')
 
+    def testMultipartSeveralAttachmentMessages(self):
+        self.doNewIssue()
+        self._handle_mail(self.multipart_msg)
+        messages = self.db.issue.get('1', 'messages')
+        messages.sort()
+        self.assertEqual(messages[-1], '2')
+        msg = self.db.msg.getnode (messages[-1])
+        self.assertEqual(len(msg.files), 5)
+        issue = self.db.issue.getnode ('1')
+        self.assertEqual(len(issue.files), 5)
+        names = {0 : 'first.dvi', 4 : 'second.dvi'}
+        content = {3 : 'test attachment third text/plain\n',
+                   4 : 'Just a test\n'}
+        for n, id in enumerate (msg.files):
+            f = self.db.file.getnode (id)
+            self.assertEqual(f.name, names.get (n, 'unnamed'))
+            if n in content :
+                self.assertEqual(f.content, content [n])
+        self.assertEqual(msg.content, 'test attachment second text/plain')
+        self.assertEqual(msg.files, ['1', '2', '3', '4', '5'])
+        self.assertEqual(issue.files, ['1', '2', '3', '4', '5'])
+
+        self._handle_mail(self.multipart_msg)
+        issue = self.db.issue.getnode ('1')
+        self.assertEqual(len(issue.files), 10)
+        messages = self.db.issue.get('1', 'messages')
+        messages.sort()
+        self.assertEqual(messages[-1], '3')
+        msg = self.db.msg.getnode (messages[-1])
+        self.assertEqual(issue.files, [str(i+1) for i in range(10)])
+        self.assertEqual(msg.files, ['6', '7', '8', '9', '10'])
+
     def testMultipartKeepFiles(self):
         self.doNewIssue()
         self._handle_mail(self.multipart_msg)
         messages = self.db.issue.get('1', 'messages')
         messages.sort()
         msg = self.db.msg.getnode (messages[-1])
-        assert(len(msg.files) == 5)
+        self.assertEqual(len(msg.files), 5)
         issue = self.db.issue.getnode ('1')
-        assert(len(issue.files) == 5)
+        self.assertEqual(len(issue.files), 5)
         names = {0 : 'first.dvi', 4 : 'second.dvi'}
         content = {3 : 'test attachment third text/plain\n',
                    4 : 'Just a test\n'}
@@ -652,7 +684,7 @@ def testMultipartKeepFiles(self):
 This ist a message without attachment
 ''')
         issue = self.db.issue.getnode ('1')
-        assert(len(issue.files) == 5)
+        self.assertEqual(len(issue.files), 5)
         self.assertEqual(issue.files, ['1', '2', '3', '4', '5'])
 
     def testMultipartDropAlternatives(self):
@@ -662,7 +694,7 @@ def testMultipartDropAlternatives(self):
         messages = self.db.issue.get('1', 'messages')
         messages.sort()
         msg = self.db.msg.getnode (messages[-1])
-        assert(len(msg.files) == 2)
+        self.assertEqual(len(msg.files), 2)
         names = {1 : 'second.dvi'}
         content = {0 : 'test attachment third text/plain\n',
                    1 : 'Just a test\n'}
@@ -681,7 +713,7 @@ def testMultipartCharsetUTF8NoAttach(self):
         messages = self.db.issue.get('1', 'messages')
         messages.sort()
         msg = self.db.msg.getnode (messages[-1])
-        assert(len(msg.files) == 1)
+        self.assertEqual(len(msg.files), 1)
         name = 'unnamed'
         content = '<html>' + c + '</html>\n'
         for n, id in enumerate (msg.files):
@@ -732,7 +764,7 @@ def testMultipartCharsetLatin1NoAttach(self):
         messages = self.db.issue.get('1', 'messages')
         messages.sort()
         msg = self.db.msg.getnode (messages[-1])
-        assert(len(msg.files) == 1)
+        self.assertEqual(len(msg.files), 1)
         name = 'unnamed'
         content = '<html>' + c + '</html>\n'
         for n, id in enumerate (msg.files):
@@ -781,7 +813,7 @@ def testMultipartCharsetUTF8AttachFile(self):
         messages = self.db.issue.get('1', 'messages')
         messages.sort()
         msg = self.db.msg.getnode (messages[-1])
-        assert(len(msg.files) == 1)
+        self.assertEqual(len(msg.files), 1)
         name = 'unnamed'
         content = '<html>' + c + '</html>\n'
         for n, id in enumerate (msg.files):
@@ -845,7 +877,7 @@ def testMultipartCharsetLatin1AttachFile(self):
         messages = self.db.issue.get('1', 'messages')
         messages.sort()
         msg = self.db.msg.getnode (messages[-1])
-        assert(len(msg.files) == 1)
+        self.assertEqual(len(msg.files), 1)
         name = 'unnamed'
         content = '<html>' + c + '</html>\n'
         for n, id in enumerate (msg.files):
@@ -907,7 +939,7 @@ def testMultipartRFC822(self):
         messages = self.db.issue.get('1', 'messages')
         messages.sort()
         msg = self.db.msg.getnode (messages[-1])
-        assert(len(msg.files) == 1)
+        self.assertEqual(len(msg.files), 1)
         name = "Fwd: Original email subject.eml"
         for n, id in enumerate (msg.files):
             f = self.db.file.getnode (id)