diff options
-rw-r--r-- | app/models/incoming_message.rb | 23 | ||||
-rw-r--r-- | spec/models/foi_attachment_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/incoming_message_spec.rb | 43 |
3 files changed, 60 insertions, 8 deletions
diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index 252f81bb7..f959a8799 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -568,9 +568,11 @@ class IncomingMessage < ActiveRecord::Base text end - # Returns part which contains main body text, or nil if there isn't one - def get_main_body_text_part - leaves = self.foi_attachments + # Returns part which contains main body text, or nil if there isn't one, + # from a set of foi_attachments. If the leaves parameter is empty or not + # supplied, uses its own foi_attachments. + def get_main_body_text_part(leaves=[]) + leaves = self.foi_attachments if leaves.empty? # Find first part which is text/plain or text/html # (We have to include HTML, as increasingly there are mail clients that @@ -604,6 +606,7 @@ class IncomingMessage < ActiveRecord::Base # nil in this case) return p end + # Returns attachments that are uuencoded in main body part def _uudecode_and_save_attachments(text) # Find any uudecoded things buried in it, yeuchly @@ -657,12 +660,16 @@ class IncomingMessage < ActiveRecord::Base attachment = self.foi_attachments.find_or_create_by_hexdigest(attrs[:hexdigest]) attachment.update_attributes(attrs) attachment.save! - attachments << attachment.id + attachments << attachment end + # Reload to refresh newly created foi_attachments self.reload - main_part = get_main_body_text_part + # get the main body part from the set of attachments we just created, + # not from the self.foi_attachments association - some of the total set of + # self.foi_attachments may now be obsolete + main_part = get_main_body_text_part(attachments) # we don't use get_main_body_text_internal, as we want to avoid charset # conversions, since /usr/bin/uudecode needs to deal with those. # e.g. for https://secure.mysociety.org/admin/foi/request/show_raw_email/24550 @@ -673,12 +680,14 @@ class IncomingMessage < ActiveRecord::Base c += 1 uudecode_attachment.url_part_number = c uudecode_attachment.save! - attachments << uudecode_attachment.id + attachments << uudecode_attachment end end + attachment_ids = attachments.map{ |attachment| attachment.id } # now get rid of any attachments we no longer have - FoiAttachment.destroy_all("id NOT IN (#{attachments.join(',')}) AND incoming_message_id = #{self.id}") + FoiAttachment.destroy_all(["id NOT IN (?) AND incoming_message_id = ?", + attachment_ids, self.id]) end # Returns body text as HTML with quotes flattened, and emails removed. diff --git a/spec/models/foi_attachment_spec.rb b/spec/models/foi_attachment_spec.rb index 537a3363c..9b0115c44 100644 --- a/spec/models/foi_attachment_spec.rb +++ b/spec/models/foi_attachment_spec.rb @@ -1,6 +1,6 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') -describe FoiAttachment, " when calculating due date" do +describe FoiAttachment do before(:each) do load_raw_emails_data diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index 1d86c26ad..03152f5ff 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -542,3 +542,46 @@ describe IncomingMessage, "when TNEF attachments are attached to messages" do end end +describe IncomingMessage, "when extracting attachments" do + + it 'handles the case where reparsing changes the body of the main part + and the cached attachment has been deleted' do + # original set of attachment attributes + attachment_attributes = { :url_part_number => 1, + :within_rfc822_subject => nil, + :content_type => "text/plain", + :charset => nil, + :body => "No way!\n", + :hexdigest => "0c8b1b0f5cb9c94ed15a180e73b5c7d1", + :filename => nil } + + # Make a small change in the body returned for the attachment + new_attachment_attributes = attachment_attributes.merge(:body => "No way!", + :hexdigest => "74d2c0a41e074f9cebe49324d5b47414") + + + # Simulate parsing with the original attachments + MailHandler.stub!(:get_attachment_attributes).and_return([attachment_attributes]) + incoming_message = incoming_messages(:useless_incoming_message) + + # Extract the attachments + incoming_message.extract_attachments! + + # delete the cached file for the main body part + main = incoming_message.get_main_body_text_part + main.delete_cached_file! + + # Simulate reparsing with the slightly changed body + MailHandler.stub!(:get_attachment_attributes).and_return([new_attachment_attributes]) + + # Re-extract the attachments + incoming_message.extract_attachments! + + attachments = incoming_message.foi_attachments + attachments.size.should == 1 + attachments.first.hexdigest.should == "74d2c0a41e074f9cebe49324d5b47414" + attachments.first.body.should == 'No way!' + end + +end + |