Skip to content

Commit

Permalink
Maintenance: Improves email drivers.
Browse files Browse the repository at this point in the history
- Added POP3 driver test.
- Improved IMAP driver test.
- Unified repeating code into a base class.
- Structural code improvements, increasing maintainability.
  • Loading branch information
mantas committed Jan 23, 2025
1 parent fc3bd1c commit ef47d6c
Show file tree
Hide file tree
Showing 17 changed files with 1,471 additions and 877 deletions.
12 changes: 0 additions & 12 deletions .dev/rubocop/todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ Metrics/AbcSize:
- 'app/models/calendar.rb'
- 'app/models/channel.rb'
- 'app/models/channel/assets.rb'
- 'app/models/channel/driver/imap.rb'
- 'app/models/channel/driver/pop3.rb'
- 'app/models/channel/driver/smtp.rb'
- 'app/models/channel/driver/twitter.rb'
- 'app/models/channel/email_build.rb'
- 'app/models/channel/email_parser.rb'
Expand Down Expand Up @@ -178,8 +175,6 @@ Metrics/BlockLength:
- 'app/models/application_model/can_creates_and_updates.rb'
- 'app/models/application_model/can_lookup.rb'
- 'app/models/channel.rb'
- 'app/models/channel/driver/imap.rb'
- 'app/models/channel/driver/pop3.rb'
- 'app/models/channel/email_parser.rb'
- 'app/models/channel/filter/bounce_delivery_permanent_failed.rb'
- 'app/models/channel/filter/database.rb'
Expand Down Expand Up @@ -275,9 +270,6 @@ Metrics/CyclomaticComplexity:
- 'app/models/calendar.rb'
- 'app/models/channel.rb'
- 'app/models/channel/assets.rb'
- 'app/models/channel/driver/imap.rb'
- 'app/models/channel/driver/pop3.rb'
- 'app/models/channel/driver/smtp.rb'
- 'app/models/channel/driver/twitter.rb'
- 'app/models/channel/email_build.rb'
- 'app/models/channel/email_parser.rb'
Expand Down Expand Up @@ -405,9 +397,6 @@ Metrics/PerceivedComplexity:
- 'app/models/calendar.rb'
- 'app/models/channel.rb'
- 'app/models/channel/assets.rb'
- 'app/models/channel/driver/imap.rb'
- 'app/models/channel/driver/pop3.rb'
- 'app/models/channel/driver/smtp.rb'
- 'app/models/channel/driver/twitter.rb'
- 'app/models/channel/email_build.rb'
- 'app/models/channel/email_parser.rb'
Expand Down Expand Up @@ -504,7 +493,6 @@ Style/OptionalBooleanParameter:
- 'app/models/channel/driver/sms/massenversand.rb'
- 'app/models/channel/driver/sms/message_bird.rb'
- 'app/models/channel/driver/sms/twilio.rb'
- 'app/models/channel/driver/smtp.rb'
- 'app/models/channel/driver/telegram.rb'
- 'app/models/channel/driver/twitter.rb'
- 'app/models/channel/driver/whatsapp.rb'
Expand Down
197 changes: 128 additions & 69 deletions app/models/channel/driver/base_email_inbound.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
# Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/

class Channel::Driver::BaseEmailInbound < Channel::EmailParser
def fetch(_options, _channel)
raise 'not implemented'
end
ACTIVE_CHECK_INTERVAL = 20

def check(_options)
raise 'not implemented'
end

def verify(_options, _verify_string)
raise 'not implemented'
end
MessageResult = Struct.new(:success, :after_action, keyword_init: true)

def fetchable?(_channel)
true
Expand All @@ -35,86 +27,153 @@ def channel_has_changed?(channel)
true
end

# Checks if email is not too big for processing
# Fetches emails
#
# @param options [Hash]. See subclass for options
# @param channel [Channel]
#
# @param [Integer] size in bytes
# @return [Hash]
#
# This method is used by IMAP and MicrosoftGraphInbound only
# It may be possible to reuse them with POP3 too, but it needs further refactoring
def too_large?(message_meta_size)
max_message_size = Setting.get('postmaster_max_size').to_f
real_message_size = message_meta_size.to_f / 1024 / 1024
if real_message_size > max_message_size
return [real_message_size, max_message_size]
# {
# result: 'ok',
# fetched: 123,
# notice: 'e. g. message about to big emails in mailbox',
# }
def fetch(options, channel)
@channel = channel
@options = options
@keep_on_server = ActiveModel::Type::Boolean.new.cast(options[:keep_on_server])

setup_connection(options)

collection, count_all = messages_iterator(@keep_on_server, options)
count_fetched = 0
too_large_messages = []
result = 'ok'
notice = ''

collection.each.with_index(1) do |message_id, count|
break if stop_fetching?(count)

Rails.logger.info " - message #{count}/#{count_all}"

message_result = fetch_single_message(message_id, count, count_all)

count_fetched += 1 if message_result.success

case message_result.after_action
in [:too_large_ignored, message]
notice += message
too_large_messages << message
in [:notice, message]
notice += message
in [:result_error, message]
result = 'error'
notice += message
else
end
end

false
end
fetch_wrap_up

# Checks if a message with the given headers is a Zammad verify message
#
# This method is used by IMAP and MicrosoftGraphInbound only
# It may be possible to reuse them with POP3 too, but it needs further refactoring
def messages_is_verify_message?(headers)
return true if headers['X-Zammad-Verify'] == 'true'
if count_all.zero?
Rails.logger.info ' - no message'
end

false
end
# Error is raised if one of the messages was too large AND postmaster_send_reject_if_mail_too_large is turned off.
# This effectivelly marks channels as stuck and gets highlighted for the admin.
# New emails are still processed! But large email is not touched, so error keeps being re-raised on every fetch.
if too_large_messages.present?
raise too_large_messages.join("\n")
end

# Checks if a message with the given headers marked to be ignored by Zammad
#
# This method is used by IMAP and MicrosoftGraphInbound only
# It may be possible to reuse them with POP3 too, but it needs further refactoring
def messages_is_ignore_message?(headers)
return true if headers['X-Zammad-Ignore'] == 'true'
{
result: result,
fetched: count_fetched,
notice: notice,
}
end

false
def stop_fetching?(count)
(count % ACTIVE_CHECK_INTERVAL).zero? && channel_has_changed?(@channel)
end

# Checks if a message is an old Zammad verify message
def fetch_wrap_up; end

# Checks if mailbox has anything besides Zammad verification emails.
# If any real messages exists, return the real count including messages to be ignored when importing.
# If only verification messages found, return 0.
#
# Returns false only if a verify message is less than 30 minutes old
# @param options [Hash] driver-specific server setup. See #fetch in the corresponding driver.
#
# This method is used by IMAP and MicrosoftGraphInbound only
# It may be possible to reuse them with POP3 too, but it needs further refactoring
def messages_is_too_old_verify?(headers, count, count_all)
return true if !messages_is_verify_message?(headers)
return true if headers['X-Zammad-Verify-Time'].blank?

begin
verify_time = Time.zone.parse(headers['X-Zammad-Verify-Time'])
rescue => e
Rails.logger.error e
return true
end
return true if verify_time < 30.minutes.ago
# @return [Hash]
#
# {
# result: 'ok'
# content_messages: 123 # or 0 if there're none
# }
def check_configuration(options)
setup_connection(options, check: true)

Rails.logger.info " - ignore message #{count}/#{count_all} - because message has a verify message"
Rails.logger.info 'check only mode, fetch no emails'

false
collection, count_all = messages_iterator(false, options)

has_content_messages = collection
.any? do |message_id|
validator = check_single_message(message_id)

next if !validator

!validator.verify_message? && !validator.ignore?
end

disconnect

{
result: 'ok',
content_messages: has_content_messages ? count_all : 0,
}
end

# Checks if a message is already imported in a given channel
# This check is skipped for channels which do not keep messages on the server
# Checks if probing email has arrived
#
# This method is used by IMAP and MicrosoftGraphInbound only
# It may be possible to reuse them with POP3 too, but it needs further refactoring
def already_imported?(headers, keep_on_server, channel)
return false if !keep_on_server
# This method is used for custom IMAP only.
# It is not used in conjunction with Micrsofot365 or Gogle OAuth channels.
#
# @param options [Hash] driver-specific server setup. See #fetch in the corresponding driver.
# @param verify_string [String] to check with
#
# @return [Hash]
#
# {
# result: 'ok' # or 'verify not ok' in case of failure
# }
def verify_transport(options, verify_string)
setup_connection(options)

collection, _count_all = messages_iterator(false, options, reverse: true)

return false if !headers
Rails.logger.info "verify mode, fetch no emails #{verify_string}"

local_message_id = headers['Message-ID']
return false if local_message_id.blank?
verify_regexp = %r{#{verify_string}}

local_message_id_md5 = Digest::MD5.hexdigest(local_message_id)
article = Ticket::Article.where(message_id_md5: local_message_id_md5).reorder('created_at DESC, id DESC').limit(1).first
return false if !article
# check for verify message
verify_message_id = collection.find do |message_id|
verify_single_message(message_id, verify_regexp)
end

# verify if message is already imported via same channel, if not, import it again
ticket = article.ticket
return false if ticket&.preferences && ticket.preferences[:channel_id].present? && channel.present? && ticket.preferences[:channel_id] != channel[:id]
result = if verify_message_id
Rails.logger.info " - verify email #{verify_string} found"
verify_message_cleanup(verify_message_id)

true
'ok'
else
'verify not ok'
end

disconnect

{ result:, }
end
end
88 changes: 88 additions & 0 deletions app/models/channel/driver/base_email_inbound/message_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/

class Channel::Driver::BaseEmailInbound
class MessageValidator
attr_reader :headers, :size

# @param headers [Hash] in key-value format
# @param size [Integer] in bytes
def initialize(headers, size = nil)
@headers = headers
@size = size
end

# Checks if email is not too big for processing
#
# This method is used by IMAP and MicrosoftGraphInbound only
# It may be possible to reuse them with POP3 too, but it needs further refactoring
def too_large?
max_message_size = Setting.get('postmaster_max_size').to_f
real_message_size = size.to_f / 1024 / 1024
if real_message_size > max_message_size
return [real_message_size, max_message_size]
end

false
end

# Checks if a message with the given headers is a Zammad verify message
#
# This method is used by IMAP and MicrosoftGraphInbound only
# It may be possible to reuse them with POP3 too, but it needs further refactoring
def verify_message?
headers['X-Zammad-Verify'] == 'true'
end

# Checks if a message with the given headers marked to be ignored by Zammad
#
# This method is used by IMAP and MicrosoftGraphInbound only
# It may be possible to reuse them with POP3 too, but it needs further refactoring
def ignore?
headers['X-Zammad-Ignore'] == 'true'
end

# Checks if a message is a new Zammad verify message
#
# Returns false only if a verify message is less than 30 minutes old
#
# This method is used by IMAP and MicrosoftGraphInbound only
# It may be possible to reuse them with POP3 too, but it needs further refactoring
def fresh_verify_message?
return false if !verify_message?
return false if headers['X-Zammad-Verify-Time'].blank?

begin
verify_time = Time.zone.parse(headers['X-Zammad-Verify-Time'])
rescue => e
Rails.logger.error e
return false
end

verify_time > 30.minutes.ago
end

# Checks if a message is already imported in a given channel
# This check is skipped for channels which do not keep messages on the server
#
# This method is used by IMAP and MicrosoftGraphInbound only
# It may be possible to reuse them with POP3 too, but it needs further refactoring
def already_imported?(keep_on_server, channel)
return false if !keep_on_server

return false if !headers

local_message_id = headers['Message-ID']
return false if local_message_id.blank?

local_message_id_md5 = Digest::MD5.hexdigest(local_message_id)
article = Ticket::Article.where(message_id_md5: local_message_id_md5).reorder('created_at DESC, id DESC').limit(1).first
return false if !article

# verify if message is already imported via same channel, if not, import it again
ticket = article.ticket
return false if ticket&.preferences && ticket.preferences[:channel_id].present? && channel.present? && ticket.preferences[:channel_id] != channel[:id]

true
end
end
end
7 changes: 1 addition & 6 deletions app/models/channel/driver/base_email_outbound.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@
class Channel::Driver::BaseEmailOutbound
include Channel::EmailHelper

# We're using the same timeouts like in Net::SMTP gem
# but we would like to have the possibility to mock them for tests
DEFAULT_OPEN_TIMEOUT = 30.seconds
DEFAULT_READ_TIMEOUT = 60.seconds

def deliver(_options, _attr, _notification = false) # rubocop:disable Style/OptionalBooleanParameter
raise 'not implemented'
end
Expand All @@ -25,7 +20,7 @@ def prepare_message_attrs(attr)
prepare_idn_outbound(attr)
end

def deliver_mail(attr, notification, method, options)
def deliver_mail(attr, notification, method, options = nil)
mail = Channel::EmailBuild.build(attr, notification)
mail.delivery_method method, options
mail.deliver
Expand Down
Loading

0 comments on commit ef47d6c

Please sign in to comment.