diff --git a/CHANGELOG.md b/CHANGELOG.md index e61a3d6..69007b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,19 @@ +# 0.1.23 + +**Critical Bug Fix:** +- Fix cleanup_folder to respect 'only' filter and prevent unintended deletions + - When force=true was used with an 'only' filter, cleanup_folder was deleting all files in the destination that weren't explicitly ignored, including unrelated folders not being managed by the mapping + - This caused data loss for folders like cabal/ and ghc/ that weren't in the only list + - The fix ensures only paths matching the inclusion filter are cleaned up, leaving unmanaged content intact + +**Test Coverage:** +- Add comprehensive test coverage for the cleanup_folder bug fix +- Test preserving unrelated folders when using force + only +- Test only cleaning managed paths specified in only list +- Test edge case with empty source and unmanaged dest content +- Update existing test to reflect correct behavior +- Total: 326 examples, 0 failures, 2 pending with 96.41% line coverage + # 0.1.22 **Testing & Quality:** diff --git a/Gemfile.lock b/Gemfile.lock index 5ddd4aa..5f090ec 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - dotsync (0.1.22) + dotsync (0.1.23) fileutils (~> 1.7.3) find (~> 0.2.0) listen (~> 3.9.0) diff --git a/README.md b/README.md index 6e8e970..fa959a8 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # Dotsync [![Gem Version](https://badge.fury.io/rb/dotsync.svg)](https://rubygems.org/gems/dotsync) -[![Ruby Gem Test Status](https://github.com/dsaenztagarro/dotsync/actions/workflows/gem-push.yml/badge.svg)](https://github.com/dsaenztagarro/dotsync/actions) +[![CI](https://github.com/dsaenztagarro/dotsync/actions/workflows/gem-push.yml/badge.svg)](https://github.com/dsaenztagarro/dotsync/actions) [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) [![Ruby Version](https://img.shields.io/badge/ruby-%3E%3D%203.2-blue)](https://www.ruby-lang.org/) diff --git a/lib/dotsync/utils/file_transfer.rb b/lib/dotsync/utils/file_transfer.rb index 03a2bc1..f5e5dd0 100644 --- a/lib/dotsync/utils/file_transfer.rb +++ b/lib/dotsync/utils/file_transfer.rb @@ -144,10 +144,20 @@ def cleanup_folder(target_dir) Find.find(target_dir) do |path| next if path == target_dir abs_path = File.expand_path(path) + + # Skip if ignored if @mapping.ignore?(abs_path) Find.prune if File.directory?(path) next end + + # When inclusions (only) are specified, only clean up paths that match the inclusion filter + # This ensures we don't delete unrelated files/folders that aren't being managed + unless @mapping.include?(abs_path) + Find.prune if File.directory?(path) + next + end + if File.file?(path) FileUtils.rm(path) elsif File.directory?(path) && Dir.empty?(path) diff --git a/lib/dotsync/version.rb b/lib/dotsync/version.rb index 6c50300..fac3d1b 100644 --- a/lib/dotsync/version.rb +++ b/lib/dotsync/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Dotsync - VERSION = "0.1.22" + VERSION = "0.1.23" end diff --git a/spec/dotsync/utils/file_transfer_spec.rb b/spec/dotsync/utils/file_transfer_spec.rb index 8828706..2f4ae1f 100644 --- a/spec/dotsync/utils/file_transfer_spec.rb +++ b/spec/dotsync/utils/file_transfer_spec.rb @@ -241,15 +241,94 @@ expect(File.read(File.join(dest, "folder3", "subfolder2", "file4.txt"))).to eq("dest content") expect(File.read(File.join(dest, "file7.txt"))).to eq("dest content") - # Paths not included in "only" option - # expect(File.read(File.join(dest, "folder3", "subfolder3", "sub2folder2", "file6.txt"))).to eq("dest content") - expect(File.exist?(File.join(dest, "folder3", "subfolder3", "sub2folder2", "file6.txt"))).to be false + # Paths not included in "only" option should be preserved (not deleted) + expect(File.read(File.join(dest, "folder3", "subfolder3", "sub2folder2", "file6.txt"))).to eq("dest content") # Paths included in "only" option expect(File.read(File.join(dest, "folder1", "file1.txt"))).to eq("src content") expect(File.read(File.join(dest, "folder3", "subfolder1", "file3.txt"))).to eq("src content") expect(File.read(File.join(dest, "file8.txt"))).to eq("src content") end + + context "with unrelated folders in destination" do + let(:only) { ["folder1", "file8.txt"] } + let(:ignore) { [] } + + before do + # Create source files that match the 'only' filter + FileUtils.rm_rf(src) + FileUtils.mkdir_p(File.join(src, "folder1")) + File.write(File.join(src, "folder1", "file1.txt"), "new src content") + File.write(File.join(src, "file8.txt"), "new src content") + + # Create unrelated folders in destination that should NOT be deleted + FileUtils.mkdir_p(File.join(dest, "cabal")) + File.write(File.join(dest, "cabal", "config"), "cabal config content") + + FileUtils.mkdir_p(File.join(dest, "ghc")) + File.write(File.join(dest, "ghc", "ghci.conf"), "ghc config content") + + # Create additional files in managed folders that should be cleaned up + FileUtils.mkdir_p(File.join(dest, "folder1")) + File.write(File.join(dest, "folder1", "old_file.txt"), "old content to remove") + end + + it "does not delete unrelated folders that are not in the only list" do + subject.transfer + + # Unrelated folders should be preserved + expect(File.exist?(File.join(dest, "cabal", "config"))).to be true + expect(File.read(File.join(dest, "cabal", "config"))).to eq("cabal config content") + expect(File.exist?(File.join(dest, "ghc", "ghci.conf"))).to be true + expect(File.read(File.join(dest, "ghc", "ghci.conf"))).to eq("ghc config content") + + # Managed folders should be cleaned and synced + expect(File.exist?(File.join(dest, "folder1", "file1.txt"))).to be true + expect(File.read(File.join(dest, "folder1", "file1.txt"))).to eq("new src content") + expect(File.exist?(File.join(dest, "folder1", "old_file.txt"))).to be false + + # Only included files should be transferred + expect(File.exist?(File.join(dest, "file8.txt"))).to be true + expect(File.read(File.join(dest, "file8.txt"))).to eq("new src content") + end + + it "only cleans up files within the managed paths specified in only list" do + # Add more dest files not in the only list + FileUtils.mkdir_p(File.join(dest, "other_folder")) + File.write(File.join(dest, "other_folder", "other_file.txt"), "other content") + File.write(File.join(dest, "unmanaged.txt"), "unmanaged content") + + subject.transfer + + # Files not in the only list should be untouched + expect(File.exist?(File.join(dest, "other_folder", "other_file.txt"))).to be true + expect(File.read(File.join(dest, "other_folder", "other_file.txt"))).to eq("other content") + expect(File.exist?(File.join(dest, "unmanaged.txt"))).to be true + expect(File.read(File.join(dest, "unmanaged.txt"))).to eq("unmanaged content") + end + end + + context "when source is empty and only filter is specified" do + let(:only) { ["folder1"] } + let(:ignore) { [] } + + before do + FileUtils.rm_rf(src) + FileUtils.mkdir_p(src) + + # Create dest with files that shouldn't be touched since nothing is in src + FileUtils.mkdir_p(File.join(dest, "unmanaged_folder")) + File.write(File.join(dest, "unmanaged_folder", "important_file.txt"), "important content") + end + + it "does not delete unmanaged folders when source is empty" do + subject.transfer + + # Unmanaged folders should be preserved + expect(File.exist?(File.join(dest, "unmanaged_folder", "important_file.txt"))).to be true + expect(File.read(File.join(dest, "unmanaged_folder", "important_file.txt"))).to eq("important content") + end + end end end end