From 3b9f7b28dfb6c0f73780a4916b9bf5af8892818c Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sat, 25 Jun 2016 08:54:25 +0200 Subject: [PATCH] Added tests for email retry to deliver or add a note it delivery was not possible. --- .gitlab-ci.yml | 11 + app/models/channel.rb | 1 + .../communicate_email/background_job.rb | 90 ++++-- test/browser/aaa_getting_started_test.rb | 1 - test/integration/email_deliver_test.rb | 270 ++++++++++++++++++ 5 files changed, 355 insertions(+), 18 deletions(-) create mode 100644 test/integration/email_deliver_test.rb diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 2127ab6fd..78a110e66 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -72,6 +72,17 @@ test:integration:email_helper: - ruby -I test/ test/integration/email_helper_test.rb - rake db:drop +test:integration:email_deliver: + stage: test + tags: + - core + script: + - export RAILS_ENV=test + - rake db:create + - rake db:migrate + - ruby -I test/ test/integration/email_deliver_test.rb + - rake db:drop + test:integration:twitter: stage: test tags: diff --git a/app/models/channel.rb b/app/models/channel.rb index df4ed9df5..457ce5d16 100644 --- a/app/models/channel.rb +++ b/app/models/channel.rb @@ -244,6 +244,7 @@ send via account self.status_out = 'error' self.last_log_out = error save + raise error end result end diff --git a/app/models/observer/ticket/article/communicate_email/background_job.rb b/app/models/observer/ticket/article/communicate_email/background_job.rb index 9c30050ce..44286e3f5 100644 --- a/app/models/observer/ticket/article/communicate_email/background_job.rb +++ b/app/models/observer/ticket/article/communicate_email/background_job.rb @@ -15,11 +15,17 @@ class Observer::Ticket::Article::CommunicateEmail::BackgroundJob ticket.subject_build(record.subject) end + # set retry count + if !record.preferences['delivery_retry'] + record.preferences['delivery_retry'] = 0 + end + record.preferences['delivery_retry'] += 1 + # send email if !ticket.group.email_address_id - raise "Can't send email, no email address definde for group id '#{ticket.group.id}'" + log_error(record, "Unable to send email, no email address definde for group id '#{ticket.group.id}'") elsif !ticket.group.email_address.channel_id - raise "Can't send email, no channel definde for email_address id '#{ticket.group.email_address_id}'" + log_error(record, "Unable to send email, no channel definde for email_address id '#{ticket.group.email_address_id}'") end channel = ticket.group.email_address.channel @@ -31,21 +37,36 @@ class Observer::Ticket::Article::CommunicateEmail::BackgroundJob end # get linked channel and send - message = channel.deliver( - { - message_id: record.message_id, - in_reply_to: record.in_reply_to, - references: ticket.get_references([record.message_id]), - from: record.from, - to: record.to, - cc: record.cc, - subject: subject, - content_type: record.content_type, - body: record.body, - attachments: record.attachments - }, - notification - ) + begin + message = channel.deliver( + { + message_id: record.message_id, + in_reply_to: record.in_reply_to, + references: ticket.get_references([record.message_id]), + from: record.from, + to: record.to, + cc: record.cc, + subject: subject, + content_type: record.content_type, + body: record.body, + attachments: record.attachments + }, + notification + ) + rescue => e + log_error(record, e.message) + return + end + if !message + log_error(record, 'Unable to send email') + return + end + + # set delivery status + record.preferences['delivery_status_message'] = nil + record.preferences['delivery_status'] = 'success' + record.preferences['delivery_status_date'] = Time.zone.now + record.save # store mail plain Store.add( @@ -83,4 +104,39 @@ class Observer::Ticket::Article::CommunicateEmail::BackgroundJob created_by_id: record.created_by_id, ) end + + def log_error(local_record, message) + local_record.preferences['delivery_status'] = 'fail' + local_record.preferences['delivery_status_message'] = message + local_record.preferences['delivery_status_date'] = Time.zone.now + local_record.save + Rails.logger.error message + + if local_record.preferences['delivery_retry'] > 3 + Ticket::Article.create( + ticket_id: local_record.ticket_id, + content_type: 'text/plain', + body: "Unable to send email: #{message}", + internal: true, + sender: Ticket::Article::Sender.find_by(name: 'System'), + type: Ticket::Article::Type.find_by(name: 'note'), + updated_by_id: 1, + created_by_id: 1, + ) + return + end + + raise message + end + + def max_attempts + 4 + end + + def reschedule_at(current_time, attempts) + if Rails.env.production? + return current_time + attempts * 20.seconds + end + current_time + 5.seconds + end end diff --git a/test/browser/aaa_getting_started_test.rb b/test/browser/aaa_getting_started_test.rb index 661c2fc5c..529d8f9f9 100644 --- a/test/browser/aaa_getting_started_test.rb +++ b/test/browser/aaa_getting_started_test.rb @@ -3,7 +3,6 @@ require 'browser_test_helper' class AaaGettingStartedTest < TestCase def test_a_getting_started - #return # TODO: temp disable if !ENV['MAILBOX_INIT'] #raise "Need MAILBOX_INIT as ENV variable like export MAILBOX_INIT='unittest01@znuny.com:somepass'" puts "NOTICE: Need MAILBOX_INIT as ENV variable like export MAILBOX_INIT='unittest01@znuny.com:somepass'" diff --git a/test/integration/email_deliver_test.rb b/test/integration/email_deliver_test.rb new file mode 100644 index 000000000..728004728 --- /dev/null +++ b/test/integration/email_deliver_test.rb @@ -0,0 +1,270 @@ +# encoding: utf-8 +require 'test_helper' + +class EmailDeliverTest < ActiveSupport::TestCase + test 'basic check' do + + if !ENV['MAIL_SERVER'] + raise "Need MAIL_SERVER as ENV variable like export MAIL_SERVER='mx.example.com'" + end + if !ENV['MAIL_SERVER_ACCOUNT'] + raise "Need MAIL_SERVER_ACCOUNT as ENV variable like export MAIL_SERVER_ACCOUNT='user:somepass'" + end + server_login = ENV['MAIL_SERVER_ACCOUNT'].split(':')[0] + server_password = ENV['MAIL_SERVER_ACCOUNT'].split(':')[1] + + email_address = EmailAddress.create( + realname: 'me Helpdesk', + email: "me#{rand(999_999_999)}@example.com", + updated_by_id: 1, + created_by_id: 1, + ) + + group = Group.create_or_update( + name: 'DeliverTest', + email_address_id: email_address.id, + updated_by_id: 1, + created_by_id: 1, + ) + + channel = Channel.create( + area: 'Email::Account', + group_id: group.id, + options: { + inbound: { + adapter: 'imap', + options: { + host: 'mx1.example.com', + user: 'example', + password: 'some_pw', + ssl: true, + } + }, + outbound: { + adapter: 'sendmail' + } + }, + active: true, + updated_by_id: 1, + created_by_id: 1, + ) + + email_address.channel_id = channel.id + email_address.save + + ticket1 = Ticket.create( + title: 'some delivery test', + group: group, + customer_id: 2, + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + updated_by_id: 1, + created_by_id: 1, + ) + assert(ticket1, 'ticket created') + + article1 = Ticket::Article.create( + ticket_id: ticket1.id, + to: 'some_recipient@example_not_existing_what_ever.com', + subject: 'some subject', + message_id: 'some@id', + body: 'some message delivery test', + internal: false, + sender: Ticket::Article::Sender.find_by(name: 'Agent'), + type: Ticket::Article::Type.find_by(name: 'email'), + updated_by_id: 1, + created_by_id: 1, + ) + + assert_equal(nil, article1.preferences['delivery_retry']) + assert_equal(nil, article1.preferences['delivery_status']) + assert_equal(nil, article1.preferences['delivery_status_date']) + assert_equal(nil, article1.preferences['delivery_status_message']) + + result = Observer::Ticket::Article::CommunicateEmail::BackgroundJob.new(article1.id) + assert(result.perform) + + article1_lookup = Ticket::Article.find(article1.id) + assert_equal(1, article1_lookup.preferences['delivery_retry']) + assert_equal('success', article1_lookup.preferences['delivery_status']) + assert(article1_lookup.preferences['delivery_status_date']) + assert_equal(nil, article1_lookup.preferences['delivery_status_message']) + + # send with invalid smtp settings + channel.update_attributes( + options: { + inbound: { + adapter: 'imap', + options: { + host: 'mx1.example.com', + user: 'example', + password: 'some_pw', + ssl: true, + } + }, + outbound: { + adapter: 'smtp', + options: { + host: 'mx1.example.com', + port: 25, + start_tls: true, + user: 'not_existing', + password: 'not_existing', + }, + }, + }, + ) + assert_raises(RuntimeError) { + result = Observer::Ticket::Article::CommunicateEmail::BackgroundJob.new(article1.id) + assert_not(result.perform) + } + article1_lookup = Ticket::Article.find(article1.id) + assert_equal(2, article1_lookup.preferences['delivery_retry']) + assert_equal('fail', article1_lookup.preferences['delivery_status']) + assert(article1_lookup.preferences['delivery_status_date']) + assert(article1_lookup.preferences['delivery_status_message']) + + # send with correct smtp settings + channel.update_attributes( + options: { + inbound: { + adapter: 'imap', + options: { + host: 'mx1.example.com', + user: 'example', + password: 'some_pw', + ssl: true, + } + }, + outbound: { + adapter: 'smtp', + options: { + host: ENV['MAIL_SERVER'], + port: 25, + start_tls: true, + user: server_login, + password: server_password, + }, + }, + }, + ) + + result = Observer::Ticket::Article::CommunicateEmail::BackgroundJob.new(article1.id) + assert(result.perform) + article1_lookup = Ticket::Article.find(article1.id) + assert_equal(3, article1_lookup.preferences['delivery_retry']) + assert_equal('success', article1_lookup.preferences['delivery_status']) + assert(article1_lookup.preferences['delivery_status_date']) + assert_nil(article1_lookup.preferences['delivery_status_message']) + + # check retry jobs + # remove background jobs + Delayed::Job.destroy_all + + # send with invalid smtp settings + channel.update_attributes( + options: { + inbound: { + adapter: 'imap', + options: { + host: 'mx1.example.com', + user: 'example', + password: 'some_pw', + ssl: true, + } + }, + outbound: { + adapter: 'smtp', + options: { + host: 'mx1.example.com', + port: 25, + start_tls: true, + user: 'not_existing', + password: 'not_existing', + }, + }, + }, + ) + + # remove background jobs + Delayed::Job.destroy_all + + article2 = Ticket::Article.create( + ticket_id: ticket1.id, + to: 'some_recipient@example_not_existing_what_ever.com', + subject: 'some subject2', + message_id: 'some@id', + body: 'some message delivery test2', + internal: false, + sender: Ticket::Article::Sender.find_by(name: 'Agent'), + type: Ticket::Article::Type.find_by(name: 'email'), + updated_by_id: 1, + created_by_id: 1, + ) + + assert_raises(RuntimeError) { + Scheduler.worker(true) + } + + article2_lookup = Ticket::Article.find(article2.id) + assert_equal(2, ticket1.articles.count) + assert_equal(1, article2_lookup.preferences['delivery_retry']) + assert_equal('fail', article2_lookup.preferences['delivery_status']) + assert(article2_lookup.preferences['delivery_status_date']) + assert(article2_lookup.preferences['delivery_status_message']) + + Scheduler.worker(true) + + article2_lookup = Ticket::Article.find(article2.id) + assert_equal(2, ticket1.articles.count) + assert_equal(1, article2_lookup.preferences['delivery_retry']) + assert_equal('fail', article2_lookup.preferences['delivery_status']) + assert(article2_lookup.preferences['delivery_status_date']) + assert(article2_lookup.preferences['delivery_status_message']) + + sleep 6 + assert_raises(RuntimeError) { + Scheduler.worker(true) + } + + article2_lookup = Ticket::Article.find(article2.id) + assert_equal(2, ticket1.articles.count) + assert_equal(2, article2_lookup.preferences['delivery_retry']) + assert_equal('fail', article2_lookup.preferences['delivery_status']) + assert(article2_lookup.preferences['delivery_status_date']) + assert(article2_lookup.preferences['delivery_status_message']) + + Scheduler.worker(true) + article2_lookup = Ticket::Article.find(article2.id) + assert_equal(2, ticket1.articles.count) + assert_equal(2, article2_lookup.preferences['delivery_retry']) + assert_equal('fail', article2_lookup.preferences['delivery_status']) + assert(article2_lookup.preferences['delivery_status_date']) + assert(article2_lookup.preferences['delivery_status_message']) + + sleep 11 + assert_raises(RuntimeError) { + Scheduler.worker(true) + } + + article2_lookup = Ticket::Article.find(article2.id) + assert_equal(2, ticket1.articles.count) + assert_equal(3, article2_lookup.preferences['delivery_retry']) + assert_equal('fail', article2_lookup.preferences['delivery_status']) + assert(article2_lookup.preferences['delivery_status_date']) + assert(article2_lookup.preferences['delivery_status_message']) + + sleep 16 + Scheduler.worker(true) + + article2_lookup = Ticket::Article.find(article2.id) + assert_equal(3, ticket1.articles.count) + assert_equal('System', ticket1.articles.last.sender.name) + assert_equal(4, article2_lookup.preferences['delivery_retry']) + assert_equal('fail', article2_lookup.preferences['delivery_status']) + assert(article2_lookup.preferences['delivery_status_date']) + assert(article2_lookup.preferences['delivery_status_message']) + + end + +end