From e2d7978f8e2153685f9afbc124fa67f2473eae3c Mon Sep 17 00:00:00 2001 From: Samuel Sciolla Date: Mon, 22 Jul 2024 10:17:45 -0400 Subject: [PATCH 01/10] Add failing test for bag.bag_files --- spec/bagit_spec.rb | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/bagit_spec.rb b/spec/bagit_spec.rb index 80569be..291abf2 100644 --- a/spec/bagit_spec.rb +++ b/spec/bagit_spec.rb @@ -165,4 +165,31 @@ end end end + + describe "bag with hidden files" do + before do + @sandbox = Sandbox.new + + # make the bag + @bag_path = File.join @sandbox.to_s, "the_bag" + @bag = described_class.new @bag_path, {}, false, false + + # add some files + @bag.add_file(".keep") { |io| io.puts "" } + @bag.add_file("test.txt") { |io| io.puts "testing testing" } + end + + after do + @sandbox.cleanup! + end + + describe "#bag_files" do + it "returns an array including non-hidden and hidden files" do + files = @bag.bag_files + expect(files).to be_a_kind_of(Array) + expect(files).not_to be_empty + expect(files).to be([".keep", "test.txt"]) + end + end + end end From dc4fcb723a457d5c698b86a874dadea2cfdae312 Mon Sep 17 00:00:00 2001 From: Samuel Sciolla Date: Mon, 22 Jul 2024 15:36:14 -0400 Subject: [PATCH 02/10] Extract FileFinder, add variants; add option for choosing variant --- lib/bagit/bag.rb | 26 ++++++++++++++++++++++++-- spec/bagit_spec.rb | 6 +++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/bagit/bag.rb b/lib/bagit/bag.rb index 84cd529..28ee257 100644 --- a/lib/bagit/bag.rb +++ b/lib/bagit/bag.rb @@ -8,9 +8,28 @@ require "bagit/valid" module BagIt + class FileFinder + def self.find(dir) + raise NotImplementedError + end + end + + class StandardFileFinder < FileFinder + def self.find(dir) + Dir[File.join(dir, "**", "*")].select { |f| File.file? f } + end + end + + class StandardWithHiddenFileFinder < FileFinder + def self.find(dir) + Dir.glob(File.join(dir, "**", "*"), File::FNM_DOTMATCH).select { |f| File.file? f } + end + end + # Represents the state of a bag on a filesystem class Bag attr_reader :bag_dir + attr_reader :detect_hidden include Validity # Validity functionality include Info # bagit & bag info functionality @@ -18,8 +37,11 @@ class Bag include Fetch # fetch related functionality # Make a new Bag based at path - def initialize(path, info = {}, _create = false) + def initialize(path, info = {}, _create = false, detect_hidden = false) @bag_dir = path + @detect_hidden = detect_hidden + @file_finder = @detect_hidden ? StandardWithHiddenFileFinder : StandardFileFinder + # make the dir structure if it doesn't exist FileUtils.mkdir bag_dir unless File.directory? bag_dir FileUtils.mkdir data_dir unless File.directory? data_dir @@ -37,7 +59,7 @@ def data_dir # Return the paths to each bag file relative to bag_dir def bag_files - Dir[File.join(data_dir, "**", "*")].select { |f| File.file? f } + @file_finder.find(data_dir) end # Return the paths to each tag file relative to bag_dir diff --git a/spec/bagit_spec.rb b/spec/bagit_spec.rb index 291abf2..74d384d 100644 --- a/spec/bagit_spec.rb +++ b/spec/bagit_spec.rb @@ -172,7 +172,7 @@ # make the bag @bag_path = File.join @sandbox.to_s, "the_bag" - @bag = described_class.new @bag_path, {}, false, false + @bag = described_class.new @bag_path, {}, false, true # add some files @bag.add_file(".keep") { |io| io.puts "" } @@ -185,10 +185,10 @@ describe "#bag_files" do it "returns an array including non-hidden and hidden files" do - files = @bag.bag_files + files = @bag.bag_files.map { |f| f.sub(File.join(@bag_path, "data", ""), "") } expect(files).to be_a_kind_of(Array) expect(files).not_to be_empty - expect(files).to be([".keep", "test.txt"]) + expect(files).to eq([".keep", "test.txt"]) end end end From 41dab5e9fd0706bb0e59fac55bd5346ab186fd7f Mon Sep 17 00:00:00 2001 From: Samuel Sciolla Date: Mon, 22 Jul 2024 15:36:49 -0400 Subject: [PATCH 03/10] Add spec for validation; modify message if detect_hidden is false --- lib/bagit/valid.rb | 4 +++- spec/validation_spec.rb | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/bagit/valid.rb b/lib/bagit/valid.rb index 2b036c4..740350e 100644 --- a/lib/bagit/valid.rb +++ b/lib/bagit/valid.rb @@ -33,7 +33,9 @@ def complete? empty_manifests.each do |file| logger.error("#{file} is manifested but not present".red) - errors.add :completeness, "#{file} is manifested but not present" + error_message = "#{file} is manifested but not present" + error_message += "; consider turning on hidden file detection" if !detect_hidden + errors.add :completeness, error_message end tag_empty_manifests.each do |file| logger.error("#{file} is a manifested tag but not present".red) diff --git a/spec/validation_spec.rb b/spec/validation_spec.rb index b286b8a..cf06f31 100644 --- a/spec/validation_spec.rb +++ b/spec/validation_spec.rb @@ -139,4 +139,38 @@ end end end + + describe "a bag with regular and hidden files" do + before do + @sandbox = Sandbox.new + + # make the bag + @orig_bag_path = File.join @sandbox.to_s, "the_bag" + @orig_bag = described_class.new @orig_bag_path, {}, false, true + + # add some files + @orig_bag.add_file(".keep") { |io| io.puts "" } + @orig_bag.add_file("test.txt") { |io| io.puts "testing testing" } + @orig_bag.manifest! + + @unaware_bag = described_class.new @orig_bag_path, {}, false # false + @aware_bag = described_class.new @orig_bag_path, {}, false, true + end + + after do + @sandbox.cleanup! + end + + it "passes validation when bag doing the validating is aware" do + expect(@aware_bag).to be_valid + end + + it "fails validation and gives suggestion to detect hidden files" do + expect(@unaware_bag).not_to be_valid + expect(@unaware_bag.errors.on(:completeness)).not_to be_empty + expect(@unaware_bag.errors.on(:completeness)).to include( + "data/.keep is manifested but not present; consider turning on hidden file detection" + ) + end + end end From 2657d0874e5bb02375460fa52d7916df4010c878 Mon Sep 17 00:00:00 2001 From: Samuel Sciolla Date: Mon, 22 Jul 2024 15:41:31 -0400 Subject: [PATCH 04/10] Rename some variables and tweak string descriptions --- spec/validation_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/validation_spec.rb b/spec/validation_spec.rb index cf06f31..569f74c 100644 --- a/spec/validation_spec.rb +++ b/spec/validation_spec.rb @@ -145,27 +145,27 @@ @sandbox = Sandbox.new # make the bag - @orig_bag_path = File.join @sandbox.to_s, "the_bag" - @orig_bag = described_class.new @orig_bag_path, {}, false, true + @source_bag_path = File.join @sandbox.to_s, "the_bag" + @source_bag = described_class.new @source_bag_path, {}, false, true # add some files - @orig_bag.add_file(".keep") { |io| io.puts "" } - @orig_bag.add_file("test.txt") { |io| io.puts "testing testing" } - @orig_bag.manifest! + @source_bag.add_file(".keep") { |io| io.puts "" } + @source_bag.add_file("test.txt") { |io| io.puts "testing testing" } + @source_bag.manifest! - @unaware_bag = described_class.new @orig_bag_path, {}, false # false - @aware_bag = described_class.new @orig_bag_path, {}, false, true + @unaware_bag = described_class.new @source_bag_path, {}, false # false + @aware_bag = described_class.new @source_bag_path, {}, false, true end after do @sandbox.cleanup! end - it "passes validation when bag doing the validating is aware" do + it "passes validation when bag is aware of hidden files" do expect(@aware_bag).to be_valid end - it "fails validation and gives suggestion to detect hidden files" do + it "fails validation when bag is unaware of hidden files and suggests option" do expect(@unaware_bag).not_to be_valid expect(@unaware_bag.errors.on(:completeness)).not_to be_empty expect(@unaware_bag.errors.on(:completeness)).to include( From 8328349f161b7746ace6e459cc67dd8e9af2d2b4 Mon Sep 17 00:00:00 2001 From: Samuel Sciolla Date: Tue, 23 Jul 2024 11:56:00 -0400 Subject: [PATCH 05/10] Tweak logic, check for hidden file --- lib/bagit/valid.rb | 4 +++- spec/validation_spec.rb | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/bagit/valid.rb b/lib/bagit/valid.rb index 740350e..7f33e1c 100644 --- a/lib/bagit/valid.rb +++ b/lib/bagit/valid.rb @@ -34,7 +34,9 @@ def complete? empty_manifests.each do |file| logger.error("#{file} is manifested but not present".red) error_message = "#{file} is manifested but not present" - error_message += "; consider turning on hidden file detection" if !detect_hidden + if !detect_hidden && file.start_with?("data/.") + error_message += "; consider turning on hidden file detection" + end errors.add :completeness, error_message end tag_empty_manifests.each do |file| diff --git a/spec/validation_spec.rb b/spec/validation_spec.rb index 569f74c..254f48e 100644 --- a/spec/validation_spec.rb +++ b/spec/validation_spec.rb @@ -44,13 +44,15 @@ it "is invalid if there are files that are in the manifest but not in the bag" do # add a file and then remove it through the back door - @bag.add_file("file-k") { |io| io.puts "time to go" } + file_name = "file-k" + @bag.add_file(file_name) { |io| io.puts "time to go" } @bag.manifest! - FileUtils.rm File.join(@bag.bag_dir, "data", "file-k") + FileUtils.rm File.join(@bag.bag_dir, "data", file_name) @bag.validate_only("true_for/completeness") expect(@bag.errors.on(:completeness)).not_to be_empty + expect(@bag.errors.on(:completeness)).to include("#{File.join("data", file_name)} is manifested but not present") expect(@bag).not_to be_valid end From 625a40e6f294c3dedf4c5b5ca4dd45923fb6763f Mon Sep 17 00:00:00 2001 From: Samuel Sciolla Date: Tue, 23 Jul 2024 14:30:02 -0400 Subject: [PATCH 06/10] Use File.join in one place --- lib/bagit/valid.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bagit/valid.rb b/lib/bagit/valid.rb index 7f33e1c..760e7c0 100644 --- a/lib/bagit/valid.rb +++ b/lib/bagit/valid.rb @@ -34,7 +34,7 @@ def complete? empty_manifests.each do |file| logger.error("#{file} is manifested but not present".red) error_message = "#{file} is manifested but not present" - if !detect_hidden && file.start_with?("data/.") + if !detect_hidden && file.start_with?(File.join("data", ".")) error_message += "; consider turning on hidden file detection" end errors.add :completeness, error_message From 104b6b33b9ed2af50a78800dd766d30cc087b9aa Mon Sep 17 00:00:00 2001 From: Samuel Sciolla Date: Wed, 24 Jul 2024 15:35:59 -0400 Subject: [PATCH 07/10] Improve structure and coverage of validation tests --- spec/validation_spec.rb | 42 ++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/spec/validation_spec.rb b/spec/validation_spec.rb index 254f48e..696d416 100644 --- a/spec/validation_spec.rb +++ b/spec/validation_spec.rb @@ -142,33 +142,57 @@ end end - describe "a bag with regular and hidden files" do + describe "a bag with unmanifested hidden files" do before do @sandbox = Sandbox.new - # make the bag + # make a bag with hidden files not manifested @source_bag_path = File.join @sandbox.to_s, "the_bag" - @source_bag = described_class.new @source_bag_path, {}, false, true - - # add some files + @source_bag = described_class.new @source_bag_path, {}, false, false @source_bag.add_file(".keep") { |io| io.puts "" } @source_bag.add_file("test.txt") { |io| io.puts "testing testing" } @source_bag.manifest! + end + + after do + @sandbox.cleanup! + end - @unaware_bag = described_class.new @source_bag_path, {}, false # false + it "fails validation when hidden file detection is on" do @aware_bag = described_class.new @source_bag_path, {}, false, true + expect(@aware_bag).to_not be_valid + end + + it "passes validation when hidden file detection is off" do + @unaware_bag = described_class.new @source_bag_path, {}, false, false + expect(@unaware_bag).to be_valid + end + end + + describe "a bag with manifested hidden files" do + before do + @sandbox = Sandbox.new + + # make a bag with hidden files manifested + @source_bag_path = File.join @sandbox.to_s, "the_bag" + @source_bag = described_class.new @source_bag_path, {}, false, true + @source_bag.add_file(".keep") { |io| io.puts "" } + @source_bag.add_file("test.txt") { |io| io.puts "testing testing" } + @source_bag.manifest! end after do @sandbox.cleanup! end - it "passes validation when bag is aware of hidden files" do + it "passes validation when hidden file detection is on" do + @aware_bag = described_class.new @source_bag_path, {}, false, true expect(@aware_bag).to be_valid end - it "fails validation when bag is unaware of hidden files and suggests option" do - expect(@unaware_bag).not_to be_valid + it "fails validation when hidden file detection is off" do + @unaware_bag = described_class.new @source_bag_path, {}, false, false + expect(@unaware_bag).to_not be_valid expect(@unaware_bag.errors.on(:completeness)).not_to be_empty expect(@unaware_bag.errors.on(:completeness)).to include( "data/.keep is manifested but not present; consider turning on hidden file detection" From 0195fef98969e0a426f4bab84296a5340c15a551 Mon Sep 17 00:00:00 2001 From: Samuel Sciolla Date: Wed, 24 Jul 2024 15:40:42 -0400 Subject: [PATCH 08/10] Add explicit completeness check --- spec/validation_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/validation_spec.rb b/spec/validation_spec.rb index 696d416..b552837 100644 --- a/spec/validation_spec.rb +++ b/spec/validation_spec.rb @@ -161,6 +161,7 @@ it "fails validation when hidden file detection is on" do @aware_bag = described_class.new @source_bag_path, {}, false, true expect(@aware_bag).to_not be_valid + expect(@aware_bag.errors.on(:completeness)).not_to be_empty end it "passes validation when hidden file detection is off" do From 92aecb6871e8855bc0585ef9747aaddc9ca37b75 Mon Sep 17 00:00:00 2001 From: Samuel Sciolla Date: Wed, 24 Jul 2024 15:49:10 -0400 Subject: [PATCH 09/10] Add bit to description about suggestion --- spec/validation_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/validation_spec.rb b/spec/validation_spec.rb index b552837..1b3f603 100644 --- a/spec/validation_spec.rb +++ b/spec/validation_spec.rb @@ -191,7 +191,7 @@ expect(@aware_bag).to be_valid end - it "fails validation when hidden file detection is off" do + it "fails validation when hidden file detection is off, with suggested fix offered" do @unaware_bag = described_class.new @source_bag_path, {}, false, false expect(@unaware_bag).to_not be_valid expect(@unaware_bag.errors.on(:completeness)).not_to be_empty From 16b3780b7efbe8354bc81488576c60ab4321c683 Mon Sep 17 00:00:00 2001 From: Samuel Sciolla Date: Wed, 24 Jul 2024 16:16:13 -0400 Subject: [PATCH 10/10] Break up line --- spec/validation_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/validation_spec.rb b/spec/validation_spec.rb index 1b3f603..aa0bf67 100644 --- a/spec/validation_spec.rb +++ b/spec/validation_spec.rb @@ -52,7 +52,9 @@ @bag.validate_only("true_for/completeness") expect(@bag.errors.on(:completeness)).not_to be_empty - expect(@bag.errors.on(:completeness)).to include("#{File.join("data", file_name)} is manifested but not present") + expect(@bag.errors.on(:completeness)).to include( + "#{File.join("data", file_name)} is manifested but not present" + ) expect(@bag).not_to be_valid end