Skip to content

Commit

Permalink
FIX: refactor calling of timed backup deletion
Browse files Browse the repository at this point in the history
refactor calling of timed backup deletion so it runs regardless of SiteSetting.automatic_backups_enabled value
  • Loading branch information
marstall authored Jan 8, 2024
1 parent c62d119 commit 3837657
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 17 deletions.
1 change: 1 addition & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,7 @@ PLATFORMS
x86_64-darwin-18
x86_64-darwin-19
x86_64-darwin-20
x86_64-darwin-22
x86_64-linux

DEPENDENCIES
Expand Down
5 changes: 5 additions & 0 deletions app/jobs/scheduled/schedule_backup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class ScheduleBackup < ::Jobs::Scheduled
sidekiq_options retry: false

def execute(args)
delete_prior_to_n_days
return unless SiteSetting.enable_backups? && SiteSetting.automatic_backups_enabled?

store = BackupRestore::BackupStore.create
Expand All @@ -25,6 +26,10 @@ def execute(args)
raise
end

def delete_prior_to_n_days
BackupRestore::Backuper.new(Discourse.system_user.id).delete_prior_to_n_days
end

def notify_user(ex)
SystemMessage.create_from_system_user(
Discourse.system_user,
Expand Down
17 changes: 7 additions & 10 deletions lib/backup_restore/backuper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,19 @@ def run
@backup_filename
ensure
delete_old
delete_prior_to_n_days
clean_up
notify_user
log "Finished!"
publish_completion
end

def delete_prior_to_n_days
return if Rails.env.development?
store.delete_prior_to_n_days
rescue => ex
log "Something went wrong while deleting backups prior to n days....", ex
end

protected

def ensure_no_operation_is_running
Expand Down Expand Up @@ -359,15 +365,6 @@ def delete_old
log "Something went wrong while deleting old backups.", ex
end

def delete_prior_to_n_days
return if Rails.env.development?

log "Deleting backups prior to n days..."
store.delete_prior_to_n_days
rescue => ex
log "Something went wrong while deleting backups prior to n days....", ex
end

def notify_user
return if success && @user.id == Discourse::SYSTEM_USER_ID

Expand Down
10 changes: 7 additions & 3 deletions lib/backup_restore/s3_backup_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,13 @@ def object_from_path(path)
def unsorted_files
objects = []

s3_helper.list.each do |obj|
objects << create_file_from_object(obj) if obj.key.match?(file_regex)
end
begin
s3_helper.list.each do |obj|
objects << create_file_from_object(obj) if obj.key.match?(file_regex)
end
rescue StandardError
NoMethodError
end #fired when s3_helper.list is nil - wont respond to .nil?

objects
rescue Aws::Errors::ServiceError => e
Expand Down
4 changes: 0 additions & 4 deletions spec/lib/backup_restore/backuper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,6 @@
store.expects(:reset_cache).at_least_once
run
end
it "deletes any old backups" do
store.expects(:delete_prior_to_n_days)
run
end
end
end
end
18 changes: 18 additions & 0 deletions spec/lib/backup_restore/shared_examples_for_backup_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,24 @@
store.delete_prior_to_n_days
expect(store.files).to eq([backup1])
end

it "runs if SiteSetting.automatic_backups_enabled? is true" do
stub_request(
:get,
"https://s3-backup-bucket.s3.amazonaws.com/?list-type=2&prefix=default/",
).to_return(status: 200, body: "", headers: {})
stub_request(:head, "https://s3-backup-bucket.s3.amazonaws.com/").to_return(
status: 200,
body: "",
headers: {
},
)

SiteSetting.automatic_backups_enabled = true
scheduleBackup = Jobs::ScheduleBackup.new
scheduleBackup.expects(:delete_prior_to_n_days)
scheduleBackup.perform
end
end

describe "#file" do
Expand Down

0 comments on commit 3837657

Please sign in to comment.