From eb5ad7d9faf658b262ce892ecd74be48f38b6572 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Fri, 17 Nov 2017 11:41:44 +0100 Subject: [PATCH] Fixed issue #1649 - Tickets are deleted but database is still the same size. --- app/models/store.rb | 13 +++-- app/models/ticket/article.rb | 13 +++++ test/unit/store_test.rb | 43 +++++++------- test/unit/ticket_article_store_empty.rb | 75 +++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 27 deletions(-) create mode 100644 test/unit/ticket_article_store_empty.rb diff --git a/app/models/store.rb b/app/models/store.rb index de3548358..76fea4961 100644 --- a/app/models/store.rb +++ b/app/models/store.rb @@ -47,7 +47,7 @@ returns data.delete('object') # store meta data - store = Store.create(data) + store = Store.create!(data) store end @@ -123,17 +123,18 @@ remove one attachment from storage =end def self.remove_item(store_id) - store = Store.find(store_id) file_id = store.store_file_id - store.destroy # check backend for references files = Store.where(store_file_id: file_id) - return if files.count != 1 - return if files.first.id != store.id + if files.count > 1 || files.first.id != store.id + store.destroy! + return true + end - Store::File.find(file_id).destroy + store.destroy! + Store::File.find(file_id).destroy! end =begin diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index 6a5e2cf5d..96213f2d8 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -18,6 +18,7 @@ class Ticket::Article < ApplicationModel store :preferences before_create :check_subject, :check_body, :check_message_id_md5 before_update :check_subject, :check_body, :check_message_id_md5 + after_destroy :store_delete sanitized_html :body @@ -316,6 +317,18 @@ returns } end + # delete attachments and mails of article + def store_delete + Store.remove( + object: 'Ticket::Article', + o_id: id, + ) + Store.remove( + object: 'Ticket::Article::Mail', + o_id: id, + ) + end + class Flag < ApplicationModel end diff --git a/test/unit/store_test.rb b/test/unit/store_test.rb index 1213768f2..134c6ae76 100644 --- a/test/unit/store_test.rb +++ b/test/unit/store_test.rb @@ -29,7 +29,7 @@ class StoreTest < ActiveSupport::TestCase assert(exists) end - test 'store attachment' do + test 'store attachment and move it between backends' do files = [ { data: 'hello world', @@ -54,7 +54,7 @@ class StoreTest < ActiveSupport::TestCase ] files.each do |file| - sha = Digest::SHA256.hexdigest( file[:data] ) + sha = Digest::SHA256.hexdigest(file[:data]) # add attachments store = Store.add( @@ -75,23 +75,23 @@ class StoreTest < ActiveSupport::TestCase assert attachments # sha check - sha_new = Digest::SHA256.hexdigest( attachments[0].content ) - assert_equal( sha, sha_new, "check file #{file[:filename]}") + sha_new = Digest::SHA256.hexdigest(attachments[0].content) + assert_equal(sha, sha_new, "check file #{file[:filename]}") # filename check - assert_equal( file[:filename], attachments[0].filename ) + assert_equal(file[:filename], attachments[0].filename) # provider check - assert_equal( 'DB', attachments[0].provider ) + assert_equal('DB', attachments[0].provider) end success = Store::File.verify assert success, 'verify ok' - Store::File.move( 'DB', 'File' ) + Store::File.move('DB', 'File') files.each do |file| - sha = Digest::SHA256.hexdigest( file[:data] ) + sha = Digest::SHA256.hexdigest(file[:data]) # get list of attachments attachments = Store.list( @@ -101,54 +101,55 @@ class StoreTest < ActiveSupport::TestCase assert attachments # sha check - sha_new = Digest::SHA256.hexdigest( attachments[0].content ) - assert_equal( sha, sha_new, "check file #{file[:filename]}") + sha_new = Digest::SHA256.hexdigest(attachments[0].content) + assert_equal(sha, sha_new, "check file #{file[:filename]}") # filename check - assert_equal( file[:filename], attachments[0].filename ) + assert_equal(file[:filename], attachments[0].filename) # provider check - assert_equal( 'File', attachments[0].provider ) + assert_equal('File', attachments[0].provider) end success = Store::File.verify assert success, 'verify ok' - Store::File.move( 'File', 'DB' ) + Store::File.move('File', 'DB') files.each do |file| - sha = Digest::SHA256.hexdigest( file[:data] ) + sha = Digest::SHA256.hexdigest(file[:data]) # get list of attachments attachments = Store.list( object: 'Test', o_id: file[:o_id], ) - assert attachments + assert(attachments) + assert_equal(attachments.count, 1) # sha check - sha_new = Digest::SHA256.hexdigest( attachments[0].content ) - assert_equal( sha, sha_new, "check file #{file[:filename]}") + sha_new = Digest::SHA256.hexdigest(attachments[0].content) + assert_equal(sha, sha_new, "check file #{file[:filename]}") # filename check - assert_equal( file[:filename], attachments[0].filename ) + assert_equal(file[:filename], attachments[0].filename) # provider check - assert_equal( 'DB', attachments[0].provider ) + assert_equal('DB', attachments[0].provider) # delete attachments success = Store.remove( object: 'Test', o_id: file[:o_id], ) - assert success + assert(success) # check attachments again attachments = Store.list( object: 'Test', o_id: file[:o_id], ) - assert !attachments[0] + assert_not(attachments[0]) end end end diff --git a/test/unit/ticket_article_store_empty.rb b/test/unit/ticket_article_store_empty.rb new file mode 100644 index 000000000..ad3cd5049 --- /dev/null +++ b/test/unit/ticket_article_store_empty.rb @@ -0,0 +1,75 @@ +# encoding: utf-8 +require 'test_helper' + +class TicketArticleStoreEmpty < ActiveSupport::TestCase + + test 'check if attachments are deleted after ticket is deleted' do + + current_count = Store.count + current_file_count = Store::File.count + current_backend_count = Store::Provider::DB.count + + email_raw_string = IO.binread('test/fixtures/mail1.box') + ticket, article, user, mail = Channel::EmailParser.new.process({}, email_raw_string) + + next_count = Store.count + next_file_count = Store::File.count + next_backend_count = Store::Provider::DB.count + + assert_equal(current_count, next_count - 2) + assert_equal(current_file_count, next_file_count - 2) + assert_equal(current_backend_count, next_backend_count - 2) + + ticket.destroy! + + after_count = Store.count + after_file_count = Store::File.count + after_backend_count = Store::Provider::DB.count + + assert_equal(current_count, after_count) + assert_equal(current_file_count, after_file_count) + assert_equal(current_backend_count, after_backend_count) + + end + + test 'check if attachments are deleted after ticket same ticket 2 times is deleted' do + + current_count = Store.count + current_file_count = Store::File.count + current_backend_count = Store::Provider::DB.count + + email_raw_string = IO.binread('test/fixtures/mail1.box') + ticket1, article1, user1, mail1 = Channel::EmailParser.new.process({}, email_raw_string) + ticket2, article2, user2, mail2 = Channel::EmailParser.new.process({}, email_raw_string) + + next_count = Store.count + next_file_count = Store::File.count + next_backend_count = Store::Provider::DB.count + + assert_equal(current_count, next_count - 4) + assert_equal(current_file_count, next_file_count - 2) + assert_equal(current_backend_count, next_backend_count - 2) + + ticket1.destroy! + + next_count = Store.count + next_file_count = Store::File.count + next_backend_count = Store::Provider::DB.count + + assert_equal(current_count, next_count - 2) + assert_equal(current_file_count, next_file_count - 2) + assert_equal(current_backend_count, next_backend_count - 2) + + ticket2.destroy! + + after_count = Store.count + after_file_count = Store::File.count + after_backend_count = Store::Provider::DB.count + + assert_equal(current_count, after_count) + assert_equal(current_file_count, after_file_count) + assert_equal(current_backend_count, after_backend_count) + + end + +end