From 4ab01e36381df0fbc9e1ee892e31edeecaf9e51d Mon Sep 17 00:00:00 2001 From: Dominik Klein Date: Thu, 22 Jul 2021 12:35:13 +0000 Subject: [PATCH] Fixes #3579 - Ensure Upload Cache files are removed after grace period. --- app/jobs/upload_cache_cleanup_job.rb | 17 ++++++ app/models/taskbar.rb | 18 +----- app/models/taskbar/has_attachments.rb | 36 +++++++++++ ...20210701131549_issue_3579_new_scheduler.rb | 18 ++++++ db/seeds/schedulers.rb | 9 +++ spec/jobs/upload_cache_cleanup_job_spec.rb | 47 +++++++++++++++ .../taskbar/has_attachments_examples.rb | 60 +++++++++++++++++++ spec/models/taskbar_spec.rb | 2 + 8 files changed, 190 insertions(+), 17 deletions(-) create mode 100644 app/jobs/upload_cache_cleanup_job.rb create mode 100644 app/models/taskbar/has_attachments.rb create mode 100644 db/migrate/20210701131549_issue_3579_new_scheduler.rb create mode 100644 spec/jobs/upload_cache_cleanup_job_spec.rb create mode 100644 spec/models/taskbar/has_attachments_examples.rb diff --git a/app/jobs/upload_cache_cleanup_job.rb b/app/jobs/upload_cache_cleanup_job.rb new file mode 100644 index 000000000..b4c9431b7 --- /dev/null +++ b/app/jobs/upload_cache_cleanup_job.rb @@ -0,0 +1,17 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class UploadCacheCleanupJob < ApplicationJob + def perform + taskbar_form_ids = Taskbar.with_form_id.filter_map(&:persisted_form_id) + + Store.where(store_object_id: store_object_id).where('created_at < ?', 1.month.ago).where.not(o_id: taskbar_form_ids).find_each do |store| + Store.remove_item(store.id) + end + end + + private + + def store_object_id + Store::Object.lookup(name: 'UploadCache').id + end +end diff --git a/app/models/taskbar.rb b/app/models/taskbar.rb index d7e96cf06..025e7c8cf 100644 --- a/app/models/taskbar.rb +++ b/app/models/taskbar.rb @@ -2,6 +2,7 @@ class Taskbar < ApplicationModel include ChecksClientNotification + include ::Taskbar::HasAttachments store :state store :params @@ -55,25 +56,8 @@ class Taskbar < ApplicationModel add_attachments_to_attributes(super) end - # form_id is saved directly in a new ticket, but inside of the article when updating an existing ticket - def persisted_form_id - state&.dig(:form_id) || state&.dig(:article, :form_id) - end - private - def attachments - return [] if persisted_form_id.blank? - - UploadCache.new(persisted_form_id).attachments - end - - def add_attachments_to_attributes(attributes) - attributes.tap do |result| - result['attachments'] = attachments.map(&:attributes_for_display) - end - end - def update_last_contact return true if local_update return true if changes.blank? diff --git a/app/models/taskbar/has_attachments.rb b/app/models/taskbar/has_attachments.rb new file mode 100644 index 000000000..408164dfc --- /dev/null +++ b/app/models/taskbar/has_attachments.rb @@ -0,0 +1,36 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +module Taskbar::HasAttachments + extend ActiveSupport::Concern + + included do + scope :with_form_id, -> { where("state LIKE '%form_id%'") } + + after_destroy :clear_attachments + end + + # form_id is saved directly in a new ticket, but inside of the article when updating an existing ticket + def persisted_form_id + state&.dig(:form_id) || state&.dig(:article, :form_id) + end + + private + + def attachments + return [] if persisted_form_id.blank? + + UploadCache.new(persisted_form_id).attachments + end + + def add_attachments_to_attributes(attributes) + attributes.tap do |result| + result['attachments'] = attachments.map(&:attributes_for_display) + end + end + + def clear_attachments + return if persisted_form_id.blank? + + UploadCache.new(persisted_form_id).destroy + end +end diff --git a/db/migrate/20210701131549_issue_3579_new_scheduler.rb b/db/migrate/20210701131549_issue_3579_new_scheduler.rb new file mode 100644 index 000000000..1c254f5ba --- /dev/null +++ b/db/migrate/20210701131549_issue_3579_new_scheduler.rb @@ -0,0 +1,18 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class Issue3579NewScheduler < ActiveRecord::Migration[6.0] + def change + return if !Setting.exists?(name: 'system_init_done') + + Scheduler.create_if_not_exists( + name: 'Delete old upload cache entries.', + method: 'UploadCacheCleanupJob.perform_now', + period: 1.month, + prio: 2, + active: true, + updated_by_id: 1, + created_by_id: 1, + last_run: Time.zone.now, + ) + end +end diff --git a/db/seeds/schedulers.rb b/db/seeds/schedulers.rb index 606e38270..b75fd595a 100644 --- a/db/seeds/schedulers.rb +++ b/db/seeds/schedulers.rb @@ -220,3 +220,12 @@ Scheduler.create_if_not_exists( updated_by_id: 1, created_by_id: 1, ) +Scheduler.create_if_not_exists( + name: 'Delete old upload cache entries.', + method: 'UploadCacheCleanupJob.perform_now', + period: 1.month, + prio: 2, + active: true, + updated_by_id: 1, + created_by_id: 1, +) diff --git a/spec/jobs/upload_cache_cleanup_job_spec.rb b/spec/jobs/upload_cache_cleanup_job_spec.rb new file mode 100644 index 000000000..5b701e8e7 --- /dev/null +++ b/spec/jobs/upload_cache_cleanup_job_spec.rb @@ -0,0 +1,47 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe UploadCacheCleanupJob, type: :job do + let(:upload_cache) { UploadCache.new(1337) } + + before do + UserInfo.current_user_id = 1 + + upload_cache.add( + data: 'current example', + filename: 'current.txt', + preferences: { + 'Content-Type' => 'text/plain', + }, + ) + + travel_to 1.month.ago + + # create one taskbar and related upload cache entry, which should not be deleted + create(:taskbar, state: { form_id: 9999 }) + UploadCache.new(9999).add( + data: 'Some Example with related Taskbar', + filename: 'another_example_with_taskbar.txt', + preferences: { + 'Content-Type' => 'text/plain', + } + ) + + 3.times do + upload_cache.add( + data: 'hello world', + filename: 'some.txt', + preferences: { + 'Content-Type' => 'text/plain', + }, + ) + end + + travel_back + end + + it 'cleanup the store items which are expired with job' do + expect { described_class.perform_now }.to change(Store, :count).by(-3) + end +end diff --git a/spec/models/taskbar/has_attachments_examples.rb b/spec/models/taskbar/has_attachments_examples.rb new file mode 100644 index 000000000..5aeee1820 --- /dev/null +++ b/spec/models/taskbar/has_attachments_examples.rb @@ -0,0 +1,60 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.shared_examples 'Taskbar::HasAttachments' do + describe '.with_form_id' do + before do + create(:taskbar) + create_list(:taskbar, 2, state: { form_id: 1337 }) + end + + it 'get list of all form ids' do + expect(described_class.with_form_id.filter_map(&:persisted_form_id)).to eq([1337, 1337]) + end + end + + describe 'delete attachments in upload cache' do + let(:state) { nil } + + let(:taskbar) do + taskbar = create(:taskbar, state: state) + UploadCache.new(1337).add( + data: 'Some Example', + filename: 'another_example.txt', + preferences: { + 'Content-Type' => 'text/plain', + } + ) + taskbar + end + + # required for adding items to the Store + before do + UserInfo.current_user_id = 1 + + # initialize taskbar to have different store counts in expect test + taskbar + end + + context 'when ticket create' do + let(:state) do + { form_id: 1337 } + end + + it 'delete attachments in upload cache after destroy' do + expect { taskbar.destroy }.to change(Store, :count).by(-1) + end + end + + context 'when ticket zoom' do + let(:state) do + { ticket: {}, article: { form_id: 1337 } } + end + + it 'delete attachments in upload cache after destroy' do + expect { taskbar.destroy }.to change(Store, :count).by(-1) + end + end + end +end diff --git a/spec/models/taskbar_spec.rb b/spec/models/taskbar_spec.rb index 3c189fcc6..f141028f0 100644 --- a/spec/models/taskbar_spec.rb +++ b/spec/models/taskbar_spec.rb @@ -1,8 +1,10 @@ # Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ require 'rails_helper' +require 'models/taskbar/has_attachments_examples' RSpec.describe Taskbar do + it_behaves_like 'Taskbar::HasAttachments' context 'key = Search' do