Browse Source

Fixes #3579 - Ensure Upload Cache files are removed after grace period.

Dominik Klein 3 years ago
parent
commit
4ab01e3638

+ 17 - 0
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

+ 1 - 17
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?

+ 36 - 0
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

+ 18 - 0
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

+ 9 - 0
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,
+)

+ 47 - 0
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

+ 60 - 0
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

+ 2 - 0
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