diff --git a/lib/rmt/mirror/base.rb b/lib/rmt/mirror/base.rb index 2b521cc00..b2a4d4ccd 100644 --- a/lib/rmt/mirror/base.rb +++ b/lib/rmt/mirror/base.rb @@ -155,8 +155,9 @@ def cleanup_stale_metadata logger.debug("Can not remove stale metadata directory: #{e}") end - def move_files(glob:, destination:) + def move_files(glob:, destination:, clean_before: false) FileUtils.mkpath(destination) unless Dir.exist?(destination) + FileUtils.rm_rf(Dir.glob(File.join(destination, '*'))) if clean_before FileUtils.mv(Dir.glob(glob), destination, force: true) rescue StandardError => e raise RMT::Mirror::Exception.new(_('Error while moving files %{glob} to %{dest}: %{error}') % { diff --git a/lib/rmt/mirror/repomd.rb b/lib/rmt/mirror/repomd.rb index 2374763cd..a3f0fda34 100644 --- a/lib/rmt/mirror/repomd.rb +++ b/lib/rmt/mirror/repomd.rb @@ -15,7 +15,7 @@ def mirror_implementation mirror_packages(metadata_files) glob_metadata = File.join(temp(:metadata), 'repodata', '*') - move_files(glob: glob_metadata, destination: repository_path('repodata')) + move_files(glob: glob_metadata, destination: repository_path('repodata'), clean_before: true) end protected diff --git a/package/obs/rmt-server.changes b/package/obs/rmt-server.changes index c84e3dbf6..71fdda942 100644 --- a/package/obs/rmt-server.changes +++ b/package/obs/rmt-server.changes @@ -13,6 +13,7 @@ Wed Oct 30 09:01:32 UTC 2024 - Natnael Getahun * Allow SLE Micro system to access SLES repositories (bsc#1230419) * Skip system token rotation in read-only APIs * Enable RMT to handle the new dl.suse.com CDN domain (bsc#1234641) + * Clean up RMT metadata after repo update (bsc#1233308) ------------------------------------------------------------------- Wed Aug 21 15:28:43 UTC 2024 - Jesús Bermúdez Velázquez diff --git a/spec/lib/rmt/mirror/base_spec.rb b/spec/lib/rmt/mirror/base_spec.rb index 1add79b21..f8cf947fc 100644 --- a/spec/lib/rmt/mirror/base_spec.rb +++ b/spec/lib/rmt/mirror/base_spec.rb @@ -230,6 +230,54 @@ expect { base.move_files(glob: src, destination: dest) }.to raise_exception(/Error while moving files/) end + + it 'removes existing files when destination directory is empty' do + allow(Dir).to receive(:exist?).with(dest).and_return(false) + allow(Dir).to receive(:glob).with(src).and_return(%w[/source/path/newfile1.txt /source/path/newfile2.txt]) + allow(Dir).to receive(:glob).with(File.join(dest, '*')).and_return([]) + expect(FileUtils).to receive(:mkpath).with(dest) + expect(FileUtils).to receive(:rm_rf).with([]) + expect(FileUtils).to receive(:mv).with(%w[/source/path/newfile1.txt /source/path/newfile2.txt], dest, force: true) + + base.move_files(glob: src, destination: dest, clean_before: true) + end + + it 'handles errors during removal of existing files' do + allow(Dir).to receive(:exist?).with(dest).and_return(true) + existing_files = ['/destination/path/file1.txt'] + + allow(Dir).to receive(:glob).with(File.join(dest, '*')).and_return(existing_files) + allow(FileUtils).to receive(:rm_rf).with(existing_files).and_raise(StandardError.new('Remove failed')) + + expect do + base.move_files(glob: src, destination: dest, clean_before: true) + end.to raise_exception(/Error while moving files/) + end + + it 'does not remove existing files when clean_before is false' do + allow(Dir).to receive(:exist?).with(dest).and_return(true) + allow(Dir).to receive(:glob).with(src).and_return(%w[/source/path/newfile1.txt /source/path/newfile2.txt]) + allow(Dir).to receive(:glob).with(File.join(dest, '*')).and_return(%w[/destination/path/existing1.txt /destination/path/existing2.txt]) + + expect(FileUtils).not_to receive(:rm_rf) + expect(FileUtils).to receive(:mv).with(%w[/source/path/newfile1.txt /source/path/newfile2.txt], dest, force: true) + + base.move_files(glob: src, destination: dest, clean_before: false) + end + + it 'removes existing files when clean_before is true' do + allow(Dir).to receive(:exist?).with(dest).and_return(true) + existing_files = %w[/destination/path/file1.txt /destination/path/file2.txt] + source_files = %w[/source/path/newfile1.txt /source/path/newfile2.txt] + + allow(Dir).to receive(:glob).with(File.join(dest, '*')).and_return(existing_files) + allow(Dir).to receive(:glob).with(src).and_return(source_files) + + expect(FileUtils).to receive(:rm_rf).with(existing_files) + expect(FileUtils).to receive(:mv).with(source_files, dest, force: true) + + base.move_files(glob: src, destination: dest, clean_before: true) + end end describe '#need_to_download?' do diff --git a/spec/lib/rmt/mirror/repomd_spec.rb b/spec/lib/rmt/mirror/repomd_spec.rb index 5a120ee79..b73dff1c0 100644 --- a/spec/lib/rmt/mirror/repomd_spec.rb +++ b/spec/lib/rmt/mirror/repomd_spec.rb @@ -52,7 +52,7 @@ expect(licenses).to receive(:mirror) expect(repomd).to receive(:mirror_metadata) expect(repomd).to receive(:mirror_packages) - expect(repomd).to receive(:move_files).with(glob: 'a/repodata/*', destination: repomd.repository_path('repodata')) + expect(repomd).to receive(:move_files).with(glob: 'a/repodata/*', destination: repomd.repository_path('repodata'), clean_before: true) repomd.mirror_implementation end @@ -65,7 +65,7 @@ expect(repomd).to receive(:create_temp_dir).with(:metadata) expect(repomd).to receive(:mirror_metadata) expect(repomd).to receive(:mirror_packages) - expect(repomd).to receive(:move_files).with(glob: 'a/repodata/*', destination: repomd.repository_path('repodata')) + expect(repomd).to receive(:move_files).with(glob: 'a/repodata/*', destination: repomd.repository_path('repodata'), clean_before: true) expect(licenses).not_to receive(:mirror)