From 5ef3ef42cd7ad79701e6774693c028b3592706d6 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Mon, 1 Mar 2021 08:50:07 +0000 Subject: [PATCH] Fixes issue #3276 - Software error when Elasticsearch is not configured and `rake searchindex:rebuild` --- app/models/ticket/search_index.rb | 89 ++++++--- ...5000001_setting_es_total_max_size_in_mb.rb | 13 ++ db/seeds/settings.rb | 9 + spec/models/ticket_spec.rb | 185 ++++++++++++++++++ 4 files changed, 269 insertions(+), 27 deletions(-) create mode 100644 db/migrate/20210215000001_setting_es_total_max_size_in_mb.rb diff --git a/app/models/ticket/search_index.rb b/app/models/ticket/search_index.rb index 96cc4ad78..9aa0b0055 100644 --- a/app/models/ticket/search_index.rb +++ b/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 diff --git a/db/migrate/20210215000001_setting_es_total_max_size_in_mb.rb b/db/migrate/20210215000001_setting_es_total_max_size_in_mb.rb new file mode 100644 index 000000000..fb1fa255d --- /dev/null +++ b/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 diff --git a/db/seeds/settings.rb b/db/seeds/settings.rb index 929fbb9ad..0156000be 100644 --- a/db/seeds/settings.rb +++ b/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', diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index c7aa35472..1db0be18b 100644 --- a/spec/models/ticket_spec.rb +++ b/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