Fixes issue #3276 - Software error when Elasticsearch is not configured and rake searchindex:rebuild

app/models/ticket/search_index.rb

@@ -13,18 +13,12 @@ module Ticket::SearchIndex
       attributes[:tags] = tags
     end
 
-    # list ignored file extensions
-    attachments_ignore = Setting.get('es_attachment_ignore') || [ '.png', '.jpg', '.jpeg', '.mpeg', '.mpg', '.mov', '.bin', '.exe' ]
-
-    # max attachment size
-    attachment_max_size_in_mb = Setting.get('es_attachment_max_size_in_mb') || 10
-    attachment_total_max_size_in_kb = 314_572
-    attachment_total_max_size_in_kb_current = 0
+    # current payload size
+    total_size_current = 0
 
     # collect article data
-    articles = Ticket::Article.where(ticket_id: id).limit(1000)
     attributes['article'] = []
-    articles.each do |article|
+    Ticket::Article.where(ticket_id: id).order(:id).limit(1000).find_each(batch_size: 50).each do |article|
 
       # lookup attributes of ref. objects (normally name and note)
       article_attributes = article.search_index_attribute_lookup(include_references: false)
@@ -40,37 +34,78 @@ module Ticket::SearchIndex
         article_attributes['body'] = article_attributes['body'].html2text
       end
 
+      article_attributes_payload_size = article_attributes.to_json.bytes.size
+
+      next if search_index_attribute_lookup_oversized?(total_size_current + article_attributes_payload_size)
+
+      # add body size to totel payload size
+      total_size_current += article_attributes_payload_size
+
       # lookup attachments
       article_attributes['attachment'] = []
-      if attachment_total_max_size_in_kb_current < attachment_total_max_size_in_kb
-        article.attachments.each do |attachment|
 
-          # check file size
-          next if !attachment.content
-          next if attachment.content.size / 1024 > attachment_max_size_in_mb * 1024
+      article.attachments.each do |attachment|
 
-          # check ignored files
-          next if !attachment.filename
+        next if search_index_attribute_lookup_file_ignored?(attachment)
 
-          filename_extention = attachment.filename.downcase
-          filename_extention.gsub!(/^.*(\..+?)$/, '\\1')
+        next if search_index_attribute_lookup_file_oversized?(attachment, total_size_current)
 
-          next if attachments_ignore.include?(filename_extention.downcase)
+        next if search_index_attribute_lookup_oversized?(total_size_current + attachment.content.bytes.size)
 
-          attachment_total_max_size_in_kb_current += (attachment.content.size / 1024).to_i
-          next if attachment_total_max_size_in_kb_current > attachment_total_max_size_in_kb
+        # add attachment size to totel payload size
+        total_size_current += attachment.content.bytes.size
 
-          data = {
-            '_name'    => attachment.filename,
-            '_content' => Base64.encode64(attachment.content).delete("\n")
-          }
-          article_attributes['attachment'].push data
-        end
+        data = {
+          'size'     => attachment.size,
+          '_name'    => attachment.filename,
+          '_content' => Base64.encode64(attachment.content).delete("\n"),
+        }
+
+        article_attributes['attachment'].push data
       end
+
       attributes['article'].push article_attributes
     end
 
     attributes
   end
 
+  private
+
+  def search_index_attribute_lookup_oversized?(total_size_current)
+
+    # if complete payload is to high
+    total_max_size_in_kb = (Setting.get('es_total_max_size_in_mb') || 300).megabyte
+    return true if total_size_current >= total_max_size_in_kb
+
+    false
+  end
+
+  def search_index_attribute_lookup_file_oversized?(attachment, total_size_current)
+    return true if attachment.content.blank?
+
+    # if attachment size is bigger as configured
+    attachment_max_size = (Setting.get('es_attachment_max_size_in_mb') || 10).megabyte
+    return true if attachment.content.bytes.size > attachment_max_size
+
+    # if complete size is bigger as configured
+    return true if search_index_attribute_lookup_oversized?(total_size_current + attachment.content.bytes.size)
+
+    false
+  end
+
+  def search_index_attribute_lookup_file_ignored?(attachment)
+    return true if attachment.filename.blank?
+
+    filename_extention = attachment.filename.downcase
+    filename_extention.gsub!(/^.*(\..+?)$/, '\\1')
+
+    # list ignored file extensions
+    attachments_ignore = Setting.get('es_attachment_ignore') || [ '.png', '.jpg', '.jpeg', '.mpeg', '.mpg', '.mov', '.bin', '.exe' ]
+
+    return true if attachments_ignore.include?(filename_extention.downcase)
+
+    false
+  end
+
 end

db/migrate/20210215000001_setting_es_total_max_size_in_mb.rb

@@ -0,0 +1,13 @@
+class SettingEsTotalMaxSizeInMb < ActiveRecord::Migration[5.2]
+  def up
+    Setting.create_if_not_exists(
+      title:       'Elasticsearch Total Payload Size',
+      name:        'es_total_max_size_in_mb',
+      area:        'SearchIndex::Elasticsearch',
+      description: 'Define max. payload size for Elasticsearch.',
+      state:       300,
+      preferences: { online_service_disable: true },
+      frontend:    false
+    )
+  end
+end

db/seeds/settings.rb

@@ -2971,6 +2971,15 @@ Setting.create_if_not_exists(
   preferences: { online_service_disable: true },
   frontend:    false
 )
+Setting.create_if_not_exists(
+  title:       'Elasticsearch Total Payload Size',
+  name:        'es_total_max_size_in_mb',
+  area:        'SearchIndex::Elasticsearch',
+  description: 'Define max. payload size for Elasticsearch.',
+  state:       300,
+  preferences: { online_service_disable: true },
+  frontend:    false
+)
 Setting.create_if_not_exists(
   title:       'Elasticsearch Pipeline Name',
   name:        'es_pipeline',

spec/models/ticket_spec.rb

@@ -1508,4 +1508,189 @@ RSpec.describe Ticket, type: :model do
       end
     end
   end
+
+  describe '.search_index_attribute_lookup_oversized?' do
+    subject!(:ticket) { create(:ticket) }
+
+    context 'when payload is ok' do
+      let(:current_payload_size) { 3.megabyte }
+
+      it 'return false' do
+        expect(ticket.send(:search_index_attribute_lookup_oversized?, current_payload_size)).to eq false
+      end
+    end
+
+    context 'when payload is bigger' do
+      let(:current_payload_size) { 350.megabyte }
+
+      it 'return true' do
+        expect(ticket.send(:search_index_attribute_lookup_oversized?, current_payload_size)).to eq true
+      end
+    end
+  end
+
+  describe '.search_index_attribute_lookup_file_oversized?' do
+    subject!(:store) do
+      Store.add(
+        object:        'SomeObject',
+        o_id:          1,
+        data:          (1024**800_000).to_s, # with 2.4 mb
+        filename:      'test.TXT',
+        created_by_id: 1,
+      )
+    end
+
+    context 'when total payload is ok' do
+      let(:current_payload_size) { 200.megabyte }
+
+      it 'return false' do
+        expect(ticket.send(:search_index_attribute_lookup_file_oversized?, store, current_payload_size)).to eq false
+      end
+    end
+
+    context 'when total payload is oversized' do
+      let(:current_payload_size) { 299.megabyte }
+
+      it 'return true' do
+        expect(ticket.send(:search_index_attribute_lookup_file_oversized?, store, current_payload_size)).to eq true
+      end
+    end
+  end
+
+  describe '.search_index_attribute_lookup_file_ignored?' do
+    context 'when attachment is indexable' do
+      subject!(:store_with_indexable_extention) do
+        Store.add(
+          object:        'SomeObject',
+          o_id:          1,
+          data:          'some content',
+          filename:      'test.TXT',
+          created_by_id: 1,
+        )
+      end
+
+      it 'return false' do
+        expect(ticket.send(:search_index_attribute_lookup_file_ignored?, store_with_indexable_extention)).to eq false
+      end
+    end
+
+    context 'when attachment is no indexable' do
+      subject!(:store_without_indexable_extention) do
+        Store.add(
+          object:        'SomeObject',
+          o_id:          1,
+          data:          'some content',
+          filename:      'test.BIN',
+          created_by_id: 1,
+        )
+      end
+
+      it 'return true' do
+        expect(ticket.send(:search_index_attribute_lookup_file_ignored?, store_without_indexable_extention)).to eq true
+      end
+    end
+  end
+
+  describe '.search_index_attribute_lookup' do
+    subject!(:ticket) { create(:ticket) }
+
+    let(:search_index_attribute_lookup) do
+      article1 = create(:ticket_article, ticket: ticket)
+      Store.add(
+        object:        'Ticket::Article',
+        o_id:          article1.id,
+        data:          'some content',
+        filename:      'some_file.bin',
+        preferences:   {
+          'Content-Type' => 'text/plain',
+        },
+        created_by_id: 1,
+      )
+      Store.add(
+        object:        'Ticket::Article',
+        o_id:          article1.id,
+        data:          (1024**800_000).to_s, # with 2.4 mb
+        filename:      'some_file.pdf',
+        preferences:   {
+          'Content-Type' => 'image/pdf',
+        },
+        created_by_id: 1,
+      )
+      Store.add(
+        object:        'Ticket::Article',
+        o_id:          article1.id,
+        data:          (1024**2_000_000).to_s, # with 5,8 mb
+        filename:      'some_file.txt',
+        preferences:   {
+          'Content-Type' => 'text/plain',
+        },
+        created_by_id: 1,
+      )
+      create(:ticket_article, ticket: ticket, body: (1024**400_000).to_s.split(/(.{100})/).join(' ')) # body with 1,2 mb
+      create(:ticket_article, ticket: ticket)
+      ticket.search_index_attribute_lookup
+    end
+
+    context 'when es_attachment_max_size_in_mb takes all attachments' do
+      before { Setting.set('es_attachment_max_size_in_mb', 15) }
+
+      it 'verify count of articles' do
+        expect(search_index_attribute_lookup['article'].count).to eq 3
+      end
+
+      it 'verify count of attachments' do
+        expect(search_index_attribute_lookup['article'][0]['attachment'].count).to eq 2
+      end
+
+      it 'verify if pdf exists' do
+        expect(search_index_attribute_lookup['article'][0]['attachment'][0]['_name']).to eq 'some_file.pdf'
+      end
+
+      it 'verify if txt exists' do
+        expect(search_index_attribute_lookup['article'][0]['attachment'][1]['_name']).to eq 'some_file.txt'
+      end
+    end
+
+    context 'when es_attachment_max_size_in_mb takes only one attachment' do
+      before { Setting.set('es_attachment_max_size_in_mb', 4) }
+
+      it 'verify count of articles' do
+        expect(search_index_attribute_lookup['article'].count).to eq 3
+      end
+
+      it 'verify count of attachments' do
+        expect(search_index_attribute_lookup['article'][0]['attachment'].count).to eq 1
+      end
+
+      it 'verify if pdf exists' do
+        expect(search_index_attribute_lookup['article'][0]['attachment'][0]['_name']).to eq 'some_file.pdf'
+      end
+    end
+
+    context 'when es_attachment_max_size_in_mb takes no attachment' do
+      before { Setting.set('es_attachment_max_size_in_mb', 2) }
+
+      it 'verify count of articles' do
+        expect(search_index_attribute_lookup['article'].count).to eq 3
+      end
+
+      it 'verify count of attachments' do
+        expect(search_index_attribute_lookup['article'][0]['attachment'].count).to eq 0
+      end
+    end
+
+    context 'when es_total_max_size_in_mb takes no attachment and no oversized article' do
+      before { Setting.set('es_total_max_size_in_mb', 1) }
+
+      it 'verify count of articles' do
+        expect(search_index_attribute_lookup['article'].count).to eq 2
+      end
+
+      it 'verify count of attachments' do
+        expect(search_index_attribute_lookup['article'][0]['attachment'].count).to eq 0
+      end
+    end
+
+  end
+
 end