Move the getting of attachment leaves to the mail handler.

2 files changed, 89 insertions, 90 deletions

diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb
index 02dfbd934..bb6c9dc21 100644
--- a/app/models/incoming_message.rb
+++ b/app/models/incoming_message.rb
@@ -395,95 +395,6 @@ class IncomingMessage < ActiveRecord::Base
         return text
     end
 
-    # (This risks losing info if the unchosen alternative is the only one to contain
-    # useful info, but let's worry about that another time)
-    def get_attachment_leaves(mail)
-        return _get_attachment_leaves_recursive(mail, mail)
-    end
-    def _get_attachment_leaves_recursive(curr_mail, parent_mail, within_rfc822_attachment = nil)
-        leaves_found = []
-        if curr_mail.multipart?
-            if curr_mail.parts.size == 0
-                raise "no parts on multipart mail"
-            end
-
-            if curr_mail.sub_type == 'alternative'
-                # Choose best part from alternatives
-                best_part = nil
-                # Take the last text/plain one, or else the first one
-                curr_mail.parts.each do |m|
-                    if not best_part
-                        best_part = m
-                    elsif m.content_type == 'text/plain'
-                        best_part = m
-                    end
-                end
-                # Take an HTML one as even higher priority. (They tend
-                # to render better than text/plain, e.g. don't wrap links here:
-                # http://www.whatdotheyknow.com/request/amount_and_cost_of_freedom_of_in#incoming-72238 )
-                curr_mail.parts.each do |m|
-                    if m.content_type == 'text/html'
-                        best_part = m
-                    end
-                end
-                leaves_found += _get_attachment_leaves_recursive(best_part, parent_mail, within_rfc822_attachment)
-            else
-                # Add all parts
-                curr_mail.parts.each do |m|
-                    leaves_found += _get_attachment_leaves_recursive(m, parent_mail, within_rfc822_attachment)
-                end
-            end
-        else
-            # XXX Yuck. this section alters various content_type's. That puts
-            # it into conflict with MailHandler.ensure_parts_counted which it has to be
-            # called both before and after.  It will fail with cases of
-            # attachments of attachments etc.
-            charset = curr_mail.charset # save this, because overwriting content_type also resets charset
-            # Don't allow nil content_types
-            if curr_mail.content_type.nil?
-                curr_mail.content_type = 'application/octet-stream'
-            end
-            # PDFs often come with this mime type, fix it up for view code
-            if curr_mail.content_type == 'application/octet-stream'
-                part_file_name = MailHandler.get_part_file_name(curr_mail)
-                part_body = MailHandler.get_part_body(curr_mail)
-                calc_mime = AlaveteliFileTypes.filename_and_content_to_mimetype(part_file_name, part_body)
-                if calc_mime
-                    curr_mail.content_type = calc_mime
-                end
-            end
-
-            # Use standard content types for Word documents etc.
-            curr_mail.content_type = MailHandler.normalise_content_type(curr_mail.content_type)
-            if curr_mail.content_type == 'message/rfc822'
-                MailHandler.ensure_parts_counted(parent_mail) # fills in rfc822_attachment variable
-                if curr_mail.rfc822_attachment.nil?
-                    # Attached mail didn't parse, so treat as text
-                    curr_mail.content_type = 'text/plain'
-                end
-            end
-            if curr_mail.content_type == 'application/vnd.ms-outlook' || curr_mail.content_type == 'application/ms-tnef'
-                MailHandler.ensure_parts_counted(parent_mail) # fills in rfc822_attachment variable
-                if curr_mail.rfc822_attachment.nil?
-                    # Attached mail didn't parse, so treat as binary
-                    curr_mail.content_type = 'application/octet-stream'
-                end
-            end
-            # If the part is an attachment of email
-            if curr_mail.content_type == 'message/rfc822' || curr_mail.content_type == 'application/vnd.ms-outlook' || curr_mail.content_type == 'application/ms-tnef'
-                MailHandler.ensure_parts_counted(parent_mail) # fills in rfc822_attachment variable
-                leaves_found += _get_attachment_leaves_recursive(curr_mail.rfc822_attachment, parent_mail, curr_mail.rfc822_attachment)
-            else
-                # Store leaf
-                curr_mail.within_rfc822_attachment = within_rfc822_attachment
-                leaves_found += [curr_mail]
-            end
-            # restore original charset
-            curr_mail.charset = charset
-        end
-        return leaves_found
-    end
-
     # Removes anything cached about the object in the database, and saves
     def clear_in_database_caches!
         self.cached_attachment_text_clipped = nil
@@ -682,7 +593,7 @@ class IncomingMessage < ActiveRecord::Base
 
     def extract_attachments!
         force = true
-        leaves = get_attachment_leaves(self.mail(force)) # XXX check where else this is called from
+        leaves = MailHandler.get_attachment_leaves(self.mail(force)) # XXX check where else this is called from
         # XXX we have to call MailHandler.ensure_parts_counted after get_attachment_leaves
         # which is really messy.
         MailHandler.ensure_parts_counted(self.mail)
diff --git a/lib/mail_handler/backends/tmail_backend.rb b/lib/mail_handler/backends/tmail_backend.rb
index 3f77f9f8b..4df4780a3 100644
--- a/lib/mail_handler/backends/tmail_backend.rb
+++ b/lib/mail_handler/backends/tmail_backend.rb
@@ -130,6 +130,94 @@ module MailHandler
                 end
             end
 
+            # (This risks losing info if the unchosen alternative is the only one to contain
+            # useful info, but let's worry about that another time)
+            def get_attachment_leaves(mail)
+                return _get_attachment_leaves_recursive(mail, mail)
+            end
+            def _get_attachment_leaves_recursive(curr_mail, parent_mail, within_rfc822_attachment = nil)
+                leaves_found = []
+                if curr_mail.multipart?
+                    if curr_mail.parts.size == 0
+                        raise "no parts on multipart mail"
+                    end
+
+                    if curr_mail.sub_type == 'alternative'
+                        # Choose best part from alternatives
+                        best_part = nil
+                        # Take the last text/plain one, or else the first one
+                        curr_mail.parts.each do |m|
+                            if not best_part
+                                best_part = m
+                            elsif m.content_type == 'text/plain'
+                                best_part = m
+                            end
+                        end
+                        # Take an HTML one as even higher priority. (They tend
+                        # to render better than text/plain, e.g. don't wrap links here:
+                        # http://www.whatdotheyknow.com/request/amount_and_cost_of_freedom_of_in#incoming-72238 )
+                        curr_mail.parts.each do |m|
+                            if m.content_type == 'text/html'
+                                best_part = m
+                            end
+                        end
+                        leaves_found += _get_attachment_leaves_recursive(best_part, parent_mail, within_rfc822_attachment)
+                    else
+                        # Add all parts
+                        curr_mail.parts.each do |m|
+                            leaves_found += _get_attachment_leaves_recursive(m, parent_mail, within_rfc822_attachment)
+                        end
+                    end
+                else
+                    # XXX Yuck. this section alters various content_type's. That puts
+                    # it into conflict with MailHandler.ensure_parts_counted which it has to be
+                    # called both before and after.  It will fail with cases of
+                    # attachments of attachments etc.
+                    charset = curr_mail.charset # save this, because overwriting content_type also resets charset
+                    # Don't allow nil content_types
+                    if curr_mail.content_type.nil?
+                        curr_mail.content_type = 'application/octet-stream'
+                    end
+                    # PDFs often come with this mime type, fix it up for view code
+                    if curr_mail.content_type == 'application/octet-stream'
+                        part_file_name = MailHandler.get_part_file_name(curr_mail)
+                        part_body = MailHandler.get_part_body(curr_mail)
+                        calc_mime = AlaveteliFileTypes.filename_and_content_to_mimetype(part_file_name, part_body)
+                        if calc_mime
+                            curr_mail.content_type = calc_mime
+                        end
+                    end
+
+                    # Use standard content types for Word documents etc.
+                    curr_mail.content_type = MailHandler.normalise_content_type(curr_mail.content_type)
+                    if curr_mail.content_type == 'message/rfc822'
+                        MailHandler.ensure_parts_counted(parent_mail) # fills in rfc822_attachment variable
+                        if curr_mail.rfc822_attachment.nil?
+                            # Attached mail didn't parse, so treat as text
+                            curr_mail.content_type = 'text/plain'
+                        end
+                    end
+                    if curr_mail.content_type == 'application/vnd.ms-outlook' || curr_mail.content_type == 'application/ms-tnef'
+                        MailHandler.ensure_parts_counted(parent_mail) # fills in rfc822_attachment variable
+                        if curr_mail.rfc822_attachment.nil?
+                            # Attached mail didn't parse, so treat as binary
+                            curr_mail.content_type = 'application/octet-stream'
+                        end
+                    end
+                    # If the part is an attachment of email
+                    if curr_mail.content_type == 'message/rfc822' || curr_mail.content_type == 'application/vnd.ms-outlook' || curr_mail.content_type == 'application/ms-tnef'
+                        MailHandler.ensure_parts_counted(parent_mail) # fills in rfc822_attachment variable
+                        leaves_found += _get_attachment_leaves_recursive(curr_mail.rfc822_attachment, parent_mail, curr_mail.rfc822_attachment)
+                    else
+                        # Store leaf
+                        curr_mail.within_rfc822_attachment = within_rfc822_attachment
+                        leaves_found += [curr_mail]
+                    end
+                    # restore original charset
+                    curr_mail.charset = charset
+                end
+                return leaves_found
+            end
 
             def address_from_name_and_email(name, email)
                 if !MySociety::Validate.is_valid_email(email)