From 1c9f6159cd6a03a4dc74b0912f475fc73c39d613 Mon Sep 17 00:00:00 2001 From: riking Date: Tue, 26 Aug 2014 17:08:53 -0700 Subject: [PATCH] Update the Receiver and PollMailbox specs for the changes Tests are both added, moved, and deleted. Add test for topic not being created Move html_only.eml to parse_body testing section --- spec/components/email/receiver_spec.rb | 271 ++++++++++++++------- spec/fixtures/emails/boundary.eml | 4 +- spec/fixtures/emails/multiline_wrote.eml | 23 -- spec/fixtures/emails/multipart.eml | 67 ----- spec/fixtures/emails/paragraphs.cooked | 7 + spec/fixtures/emails/too_many_mentions.eml | 31 +++ spec/fixtures/emails/too_short.eml | 21 ++ spec/jobs/poll_mailbox_spec.rb | 4 +- spec/spec_helper.rb | 1 + 9 files changed, 247 insertions(+), 182 deletions(-) delete mode 100644 spec/fixtures/emails/multiline_wrote.eml delete mode 100644 spec/fixtures/emails/multipart.eml create mode 100644 spec/fixtures/emails/paragraphs.cooked create mode 100644 spec/fixtures/emails/too_many_mentions.eml create mode 100644 spec/fixtures/emails/too_short.eml diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 71bc6a4d504..c5ef25543ec 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -8,124 +8,225 @@ describe Email::Receiver do before do SiteSetting.reply_by_email_address = "reply+%{reply_key}@appmail.adventuretime.ooo" SiteSetting.email_in = false + SiteSetting.title = "Discourse" end - describe 'invalid emails' do + describe 'parse_body' do + def test_parse_body(mail_string) + Email::Receiver.new(nil).parse_body(Mail::Message.new mail_string) + end + it "raises EmptyEmailError if the message is blank" do - expect { Email::Receiver.new("").process }.to raise_error(Email::Receiver::EmptyEmailError) + expect { test_parse_body("") }.to raise_error(Email::Receiver::EmptyEmailError) end it "raises EmptyEmailError if the message is not an email" do - expect { Email::Receiver.new("asdf" * 30).process}.to raise_error(Email::Receiver::EmptyEmailError) + expect { test_parse_body("asdf" * 30) }.to raise_error(Email::Receiver::EmptyEmailError) end - it "raises EmailUnparsableError if there is no reply content" do - expect { Email::Receiver.new(fixture_file("emails/no_content_reply.eml")).process}.to raise_error(Email::Receiver::EmailUnparsableError) + it "raises EmptyEmailError if there is no reply content" do + expect { test_parse_body(fixture_file("emails/no_content_reply.eml")) }.to raise_error(Email::Receiver::EmptyEmailError) end - end - describe "with multipart" do - let(:reply_below) { fixture_file("emails/multipart.eml") } - let(:receiver) { Email::Receiver.new(reply_below) } - - it "processes correctly" do - expect { receiver.process}.to raise_error(Email::Receiver::EmailLogNotFound) - expect(receiver.body).to eq( -"So presumably all the quoted garbage and my (proper) signature will get -stripped from my reply?") + pending "raises EmailUnparsableError if the headers are corrupted" do + expect { ; }.to raise_error(Email::Receiver::EmailUnparsableError) end - end - describe "html only" do - let(:reply_below) { fixture_file("emails/html_only.eml") } - let(:receiver) { Email::Receiver.new(reply_below) } - - it "processes correctly" do - expect { receiver.process}.to raise_error(Email::Receiver::EmailLogNotFound) - expect(receiver.body).to eq("The EC2 instance - I've seen that there tends to be odd and " + - "unrecommended settings on the Bitnami installs that I've checked out.") + it "can parse the html section" do + test_parse_body(fixture_file("emails/html_only.eml")).should == "The EC2 instance - I've seen that there tends to be odd and " + + "unrecommended settings on the Bitnami installs that I've checked out." end - end - describe "it supports a dutch reply" do - let(:dutch) { fixture_file("emails/dutch.eml") } - let(:receiver) { Email::Receiver.new(dutch) } - - it "processes correctly" do - expect { receiver.process}.to raise_error(Email::Receiver::EmailLogNotFound) - expect(receiver.body).to eq("Dit is een antwoord in het Nederlands.") + it "supports a Dutch reply" do + test_parse_body(fixture_file("emails/dutch.eml")).should == "Dit is een antwoord in het Nederlands." end - end - describe "It supports a non english reply" do - let(:hebrew) { fixture_file("emails/hebrew.eml") } - let(:receiver) { Email::Receiver.new(hebrew) } - - it "processes correctly" do + it "supports a Hebrew reply" do I18n.expects(:t).with('user_notifications.previous_discussion').returns('כלטוב') - expect { receiver.process}.to raise_error(Email::Receiver::EmailLogNotFound) - expect(receiver.body).to eq("שלום") + + # The force_encoding call is only needed for the test - it is passed on fine to the cooked post + test_parse_body(fixture_file("emails/hebrew.eml")).force_encoding("UTF-8").should == "שלום" end - end - describe "It supports a non UTF-8 reply" do - let(:big5) { fixture_file("emails/big5.eml") } - let(:receiver) { Email::Receiver.new(big5) } - - it "processes correctly" do + it "supports a BIG5-encoded reply" do I18n.expects(:t).with('user_notifications.previous_discussion').returns('媽！我上電視了！') - expect { receiver.process}.to raise_error(Email::Receiver::EmailLogNotFound) - expect(receiver.body).to eq("媽！我上電視了！") + + # The force_encoding call is only needed for the test - it is passed on fine to the cooked post + test_parse_body(fixture_file("emails/big5.eml")).force_encoding("UTF-8").should == "媽！我上電視了！" end - end - describe "via" do - let(:wrote) { fixture_file("emails/via_line.eml") } - let(:receiver) { Email::Receiver.new(wrote) } + it "removes 'via' lines if they match the site title" do + SiteSetting.title = "Discourse" - it "removes via lines if we know them" do - expect { receiver.process}.to raise_error(Email::Receiver::EmailLogNotFound) - expect(receiver.body).to eq("Hello this email has content!") + test_parse_body(fixture_file("emails/via_line.eml")).should == "Hello this email has content!" end - end - describe "if wrote is on a second line" do - let(:wrote) { fixture_file("emails/multiline_wrote.eml") } - let(:receiver) { Email::Receiver.new(wrote) } - - it "processes correctly" do - expect { receiver.process}.to raise_error(Email::Receiver::EmailLogNotFound) - expect(receiver.body).to eq("Thanks!") + it "removes the 'Previous Discussion' marker" do + test_parse_body(fixture_file("emails/previous.eml")).should == "This will not include the previous discussion that is present in this email." end - end - describe "remove previous discussion" do - let(:previous) { fixture_file("emails/previous.eml") } - let(:receiver) { Email::Receiver.new(previous) } - - it "processes correctly" do - expect { receiver.process}.to raise_error(Email::Receiver::EmailLogNotFound) - expect(receiver.body).to eq("This will not include the previous discussion that is present in this email.") - end - end - - describe "multiple paragraphs" do - let(:paragraphs) { fixture_file("emails/paragraphs.eml") } - let(:receiver) { Email::Receiver.new(paragraphs) } - - it "processes correctly" do - expect { receiver.process}.to raise_error(Email::Receiver::EmailLogNotFound) - expect(receiver.body).to eq( + it "handles multiple paragraphs" do + test_parse_body(fixture_file("emails/paragraphs.eml")). + should == ( "Is there any reason the *old* candy can't be be kept in silos while the new candy is imported into *new* silos? The thing about candy is it stays delicious for a long time -- we can just keep it there without worrying about it too much, imo. -Thanks for listening.") +Thanks for listening." + ) end end + describe "posting replies" do + let(:reply_key) { raise "Override this in a lower describe block" } + let(:email_raw) { raise "Override this in a lower describe block" } + # ---- + let(:receiver) { Email::Receiver.new(email_raw) } + let(:post) { create_post } + let(:topic) { post.topic } + let(:posting_user) { post.user } + let(:replying_user_email) { 'jake@adventuretime.ooo' } + let(:replying_user) { Fabricate(:user, email: replying_user_email, trust_level: 2)} + let(:email_log) { EmailLog.new(reply_key: reply_key, + post: post, + post_id: post.id, + topic_id: post.topic_id, + email_type: 'user_posted', + user: replying_user, + user_id: replying_user.id, + to_address: replying_user_email + ) } + + before do + email_log.save + end + + # === Success Posting === + + describe "valid_reply.eml" do + let!(:reply_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' } + let!(:email_raw) { fixture_file("emails/valid_reply.eml") } + + it "creates a post with the correct content" do + start_count = topic.posts.count + + receiver.process + + topic.posts.count.should == (start_count + 1) + topic.posts.last.cooked.strip.should == fixture_file("emails/valid_reply.cooked").strip + end + end + + describe "paragraphs.eml" do + let!(:reply_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' } + let!(:email_raw) { fixture_file("emails/paragraphs.eml") } + + it "cooks multiple paragraphs with traditional Markdown linebreaks" do + start_count = topic.posts.count + + receiver.process + + topic.posts.count.should == (start_count + 1) + topic.posts.last.cooked.strip.should == fixture_file("emails/paragraphs.cooked").strip + topic.posts.last.cooked.should_not match /
/ + Upload.find_by(sha1: upload_sha).should_not be_nil + end + + end + + # === Failure Conditions === + + describe "too_short.eml" do + let!(:reply_key) { '636ca428858779856c226bb145ef4fad' } + let!(:email_raw) { + fixture_file("emails/too_short.eml") + .gsub("TO", "reply+#{reply_key}@appmail.adventuretime.ooo") + .gsub("FROM", replying_user_email) + .gsub("SUBJECT", "re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'") + } + + it "raises an InvalidPost error" do + SiteSetting.min_post_length = 5 + expect { receiver.process }.to raise_error(Email::Receiver::InvalidPost) + end + end + + describe "too_many_mentions.eml" do + let!(:reply_key) { '636ca428858779856c226bb145ef4fad' } + let!(:email_raw) { fixture_file("emails/too_many_mentions.eml") } + + it "raises an InvalidPost error" do + SiteSetting.max_mentions_per_post = 10 + (1..11).each do |i| + Fabricate(:user, username: "user#{i}").save + end + + expect { receiver.process }.to raise_error(Email::Receiver::InvalidPost) + end + end + + end + + describe "posting a new topic" do + let(:category_destination) { raise "Override this in a lower describe block" } + let(:email_raw) { raise "Override this in a lower describe block" } + let(:allow_strangers) { false } + # ---- + let(:receiver) { Email::Receiver.new(email_raw) } + let(:user_email) { 'jake@adventuretime.ooo' } + let(:user) { Fabricate(:user, email: user_email, trust_level: 2)} + let(:category) { Fabricate(:category, email_in: category_destination, email_in_allow_strangers: allow_strangers) } + + before do + SiteSetting.email_in = true + user.save + category.save + end + + describe "too_short.eml" do + let!(:category_destination) { 'incoming+amazing@appmail.adventuretime.ooo' } + let(:email_raw) { + fixture_file("emails/too_short.eml") + .gsub("TO", category_destination) + .gsub("FROM", user_email) + .gsub("SUBJECT", "A long subject that passes the checks") + } + + it "does not create a topic if the post fails" do + before_topic_count = Topic.count + + expect { receiver.process }.to raise_error(Email::Receiver::InvalidPost) + + Topic.count.should == before_topic_count + end + + end + + end + def fill_email(mail, from, to, body = nil, subject = nil) result = mail.gsub("FROM", from).gsub("TO", to) if body @@ -181,12 +282,6 @@ greatest show ever created. Everyone should watch it. expect(receiver.body).to eq(reply_body) expect(receiver.email_log).to eq(email_log) - - attachment_email = fixture_file("emails/attachment.eml") - attachment_email = fill_email(attachment_email, "test@test.com", to) - r = Email::Receiver.new(attachment_email) - expect { r.process }.to_not raise_error - expect(r.body).to match(/here is an image attachment\n\n/) end end diff --git a/spec/fixtures/emails/boundary.eml b/spec/fixtures/emails/boundary.eml index 92eb4347f9c..1250fe498b0 100644 --- a/spec/fixtures/emails/boundary.eml +++ b/spec/fixtures/emails/boundary.eml @@ -18,7 +18,7 @@ Content-Type: text/plain; charset=ISO-8859-1 I'll look into it, thanks! -On Wednesday, June 19, 2013, jake via Adventure Time wrote: +On Wednesday, June 19, 2013, jake via Discourse wrote: > jake mentioned you in 'peppermint butler is missing' on Adventure > Time: @@ -58,4 +58,4 @@ p> ime.ooo/user_preferences" target=3D"_blank">user preferences.

---001a11c206a073876a04df81d2a9-- \ No newline at end of file +--001a11c206a073876a04df81d2a9-- diff --git a/spec/fixtures/emails/multiline_wrote.eml b/spec/fixtures/emails/multiline_wrote.eml deleted file mode 100644 index 0829990dca5..00000000000 --- a/spec/fixtures/emails/multiline_wrote.eml +++ /dev/null @@ -1,23 +0,0 @@ - -Delivered-To: discourse-reply+cd480e301683c9902891f15968bf07a5@discourse.org -Received: by 10.194.216.104 with SMTP id op8csp80593wjc; - Wed, 24 Jul 2013 07:59:14 -0700 (PDT) -Return-Path: -References: <51efeb9b36c34_66dc2dfce6811866@discourse.mail> -From: Walter White -In-Reply-To: <51efeb9b36c34_66dc2dfce6811866@discourse.mail> -Mime-Version: 1.0 (1.0) -Date: Wed, 24 Jul 2013 15:59:10 +0100 -Message-ID: <4597127794206131679@unknownmsgid> -Subject: Re: [Discourse] new reply to your post in 'Crystal Blue' -To: walter via Discourse -Content-Type: multipart/alternative; boundary=001a11c20edc15a39304e2432790 - -Thanks! - -On 24 Jul 2013, at 15:58, walter via Discourse -wrote: - - walter July 24 - -You look great today Walter. diff --git a/spec/fixtures/emails/multipart.eml b/spec/fixtures/emails/multipart.eml deleted file mode 100644 index b61f9fa4848..00000000000 --- a/spec/fixtures/emails/multipart.eml +++ /dev/null @@ -1,67 +0,0 @@ -Message-ID: <51C22E52.1030509@darthvader.ca> -Date: Wed, 19 Jun 2013 18:18:58 -0400 -From: Anakin Skywalker -User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 -MIME-Version: 1.0 -To: Han Solo via Death Star -Subject: Re: [Death Star] [PM] re: Regarding your post in "Site Customization - not working" -References: <51d23d33f41fb_5f4e4b35d7d60798@xwing.mail> -In-Reply-To: <51d23d33f41fb_5f4e4b35d7d60798@xwing.mail> -Content-Type: multipart/alternative; - boundary="------------070503080300090900010604" - -This is a multi-part message in MIME format. ---------------070503080300090900010604 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 7bit - -So presumably all the quoted garbage and my (proper) signature will get -stripped from my reply? - --- -Anakin Skywalker | `One of the main causes of the fall of -evildad@darthvader.ca | the Roman Empire was that, lacking zero, - | they had no way to indicate successful - | termination of their C programs.' - Firth - - ---------------070503080300090900010604 -Content-Type: text/html; charset=UTF-8 -Content-Transfer-Encoding: 7bit - - - - - - -
On 13-06-19 06:14 PM, Han Solo via - Death Star wrote:
-
-
-

Han Solo just sent you a private message

-
-

I got it here! Yay it worked!

-
-

Please visit this link to respond: http://darthvader.ca/t/regarding-your-post-in-site-customization-not-working/7641/2

-

To unsubscribe from these emails, visit your user - preferences.

-
- So presumably all the quoted garbage and my (proper) signature will - get stripped from my reply?
-
- 
--
-Anakin Skywalker               | `One of the main causes of the fall of
-evildad@darthvader.ca       | the Roman Empire was that, lacking zero,
-                            | they had no way to indicate successful
-                            | termination of their C programs.' - Firth
-
- - - ---------------070503080300090900010604-- diff --git a/spec/fixtures/emails/paragraphs.cooked b/spec/fixtures/emails/paragraphs.cooked new file mode 100644 index 00000000000..da83260e09c --- /dev/null +++ b/spec/fixtures/emails/paragraphs.cooked @@ -0,0 +1,7 @@ +

Is there any reason the old candy can't be be kept in silos while the new candy +is imported into new silos?

+ +

The thing about candy is it stays delicious for a long time -- we can just keep +it there without worrying about it too much, imo.

+ +

Thanks for listening.

\ No newline at end of file diff --git a/spec/fixtures/emails/too_many_mentions.eml b/spec/fixtures/emails/too_many_mentions.eml new file mode 100644 index 00000000000..9cc7b75c94f --- /dev/null +++ b/spec/fixtures/emails/too_many_mentions.eml @@ -0,0 +1,31 @@ +Return-Path: +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog +To: reply+636ca428858779856c226bb145ef4fad@appmail.adventuretime.ooo +Message-ID: +Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 + + +@user1 +@user2 +@user3 +@user4 +@user5 +@user6 +@user7 +@user8 +@user9 +@user10 +@user11 \ No newline at end of file diff --git a/spec/fixtures/emails/too_short.eml b/spec/fixtures/emails/too_short.eml new file mode 100644 index 00000000000..54fed0f98c5 --- /dev/null +++ b/spec/fixtures/emails/too_short.eml @@ -0,0 +1,21 @@ +Return-Path: +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog +To: TO +Message-ID: +Subject: SUBJECT +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 + + ++1 \ No newline at end of file diff --git a/spec/jobs/poll_mailbox_spec.rb b/spec/jobs/poll_mailbox_spec.rb index b10dcc09ede..3084b2b2fee 100644 --- a/spec/jobs/poll_mailbox_spec.rb +++ b/spec/jobs/poll_mailbox_spec.rb @@ -202,9 +202,9 @@ describe Jobs::PollMailbox do email.should be_deleted end - it "a no content reply raises an EmailUnparsableError" do + it "a no content reply raises an EmptyEmailError" do email = MockPop3EmailObject.new fixture_file('emails/no_content_reply.eml') - expect_exception Email::Receiver::EmailUnparsableError + expect_exception Email::Receiver::EmptyEmailError poller.handle_mail(email) email.should be_deleted diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 23c827e7db4..f90de100e8b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -30,6 +30,7 @@ Spork.prefork do # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. Dir[Rails.root.join("spec/support/**/*.rb")].each {|f| require f} + Dir[Rails.root.join("spec/fabricators/*.rb")].each {|f| require f} # let's not run seed_fu every test SeedFu.quiet = true if SeedFu.respond_to? :quiet