Merge pull request #3658 from alphagov/migrate-hidden-files

Ensure hidden files are migrated to asset manager

lib/migrate_assets_to_asset_manager.rb

@@ -19,8 +19,9 @@ class Worker < WorkerBase
     sidekiq_options queue: :asset_migration
 
     def perform(file_path)
-      file = AssetFile.open(file_path)
-      create_whitehall_asset(file) unless asset_exists?(file)
+      AssetFile.open(file_path) do |file|
+        create_whitehall_asset(file) unless asset_exists?(file)
+      end
     end
 
   private
@@ -55,7 +56,7 @@ def file_paths
   private
 
     def all_paths_under_target_directory
-      Dir.glob(File.join(full_target_dir, '**', '*'))
+      Dir.glob(File.join(full_target_dir, '**', '*'), File::FNM_DOTMATCH)
     end
 
     def full_target_dir

test/unit/migrate_assets_to_asset_manager_test.rb

@@ -4,21 +4,19 @@ class MigrateAssetsToAssetManagerTest < ActiveSupport::TestCase
   setup do
     Services.asset_manager.stubs(:whitehall_asset).raises(GdsApi::HTTPNotFound.new(404))
 
-    organisation_logo_dir = File.join(Whitehall.clean_uploads_root, 'system', 'uploads', 'organisation', 'logo', '1')
     FileUtils.mkdir_p(organisation_logo_dir)
-
-    @organisation_logo_path = File.join(organisation_logo_dir, 'logo.jpg')
-    dummy_asset_path = Rails.root.join('test', 'fixtures', 'images', '960x640_jpeg.jpg')
-    FileUtils.cp(dummy_asset_path, @organisation_logo_path)
-
-    @organisation_logo_file = File.open(@organisation_logo_path)
+    FileUtils.cp(dummy_asset_path, organisation_logo_path)
 
     @subject = MigrateAssetsToAssetManager.new('system/uploads/organisation/logo')
   end
 
+  teardown do
+    FileUtils.rm_rf(organisation_logo_dir)
+  end
+
   test 'it calls create_whitehall_asset for each file in the list' do
     Services.asset_manager.expects(:create_whitehall_asset)
-      .with(has_entry(:file, responds_with(:read, File.read(@organisation_logo_path))))
+      .with(has_entry(:file, responds_with(:read, File.read(organisation_logo_path))))
 
     @subject.perform
   end
@@ -32,7 +30,7 @@ class MigrateAssetsToAssetManagerTest < ActiveSupport::TestCase
   end
 
   test 'it calls create_whitehall_asset with the legacy last modified time' do
-    expected_last_modified = File.stat(@organisation_logo_file.path).mtime
+    expected_last_modified = File.stat(organisation_logo_path).mtime
 
     Services.asset_manager.expects(:create_whitehall_asset).with(
       has_entry(:legacy_last_modified, expected_last_modified)
@@ -43,8 +41,8 @@ class MigrateAssetsToAssetManagerTest < ActiveSupport::TestCase
 
   test 'it calls create_whitehall_asset with the legacy etag' do
     expected_etag = [
-      File.stat(@organisation_logo_path).mtime.to_i.to_s(16),
-      File.stat(@organisation_logo_path).size.to_i.to_s(16),
+      File.stat(organisation_logo_path).mtime.to_i.to_s(16),
+      File.stat(organisation_logo_path).size.to_i.to_s(16),
     ].join('-')
 
     Services.asset_manager.expects(:create_whitehall_asset).with(
@@ -64,29 +62,38 @@ class MigrateAssetsToAssetManagerTest < ActiveSupport::TestCase
   test 'to_s is a count of the number of files to be migrated' do
     assert_equal 'Migrating 1 file', @subject.to_s
   end
+
+private
+
+  def organisation_logo_dir
+    File.join(Whitehall.clean_uploads_root, 'system', 'uploads', 'organisation', 'logo', '1')
+  end
+
+  def organisation_logo_path
+    File.join(organisation_logo_dir, 'logo.jpg')
+  end
+
+  def dummy_asset_path
+    Rails.root.join('test', 'fixtures', 'images', '960x640_jpeg.jpg')
+  end
 end
 
 class AssetFilePathsTest < ActiveSupport::TestCase
   setup do
-    organisation_logo_dir = File.join(Whitehall.clean_uploads_root, 'system', 'uploads', 'organisation', 'logo', '1')
-    other_asset_dir = File.join(Whitehall.clean_uploads_root, 'system', 'uploads', 'other')
-
     FileUtils.mkdir_p(organisation_logo_dir)
     FileUtils.mkdir_p(other_asset_dir)
 
-    organisation_logo_path = File.join(organisation_logo_dir, 'logo.jpg')
-    other_asset_path = File.join(other_asset_dir, 'other_asset.png')
-    dummy_asset_path = Rails.root.join('test', 'fixtures', 'images', '960x640_jpeg.jpg')
-
     FileUtils.cp(dummy_asset_path, organisation_logo_path)
     FileUtils.cp(dummy_asset_path, other_asset_path)
 
-    @organisation_logo = File.open(organisation_logo_path)
-    @other_asset = File.open(other_asset_path)
-
     @subject = MigrateAssetsToAssetManager::AssetFilePaths.new('system/uploads/organisation/logo')
   end
 
+  teardown do
+    FileUtils.rm_rf(organisation_logo_dir)
+    FileUtils.rm_rf(other_asset_dir)
+  end
+
   test 'delegates each to file_paths' do
     assert @subject.respond_to?(:each)
   end
@@ -96,7 +103,7 @@ class AssetFilePathsTest < ActiveSupport::TestCase
   end
 
   test '#files includes only organisation logos' do
-    assert_equal 1, @subject.file_paths.size
+    assert_same_elements [organisation_logo_path], @subject.file_paths
   end
 
   test '#files does not includes directories' do
@@ -107,15 +114,45 @@ class AssetFilePathsTest < ActiveSupport::TestCase
 
   test '#files includes all files when initialised with a top level target directory' do
     subject = MigrateAssetsToAssetManager::AssetFilePaths.new('system/uploads')
-    assert_equal 2, subject.file_paths.size
+    assert_same_elements [organisation_logo_path, other_asset_path], subject.file_paths
+  end
+
+  test '#files includes hidden files' do
+    hidden_path = File.join(organisation_logo_dir, '.hidden.jpg')
+    FileUtils.cp(dummy_asset_path, hidden_path)
+
+    assert_same_elements [organisation_logo_path, hidden_path], @subject.file_paths
+  end
+
+private
+
+  def organisation_logo_dir
+    File.join(Whitehall.clean_uploads_root, 'system', 'uploads', 'organisation', 'logo', '1')
+  end
+
+  def other_asset_dir
+    File.join(Whitehall.clean_uploads_root, 'system', 'uploads', 'other')
+  end
+
+  def organisation_logo_path
+    File.join(organisation_logo_dir, 'logo.jpg')
+  end
+
+  def other_asset_path
+    File.join(other_asset_dir, 'other_asset.png')
+  end
+
+  def dummy_asset_path
+    Rails.root.join('test', 'fixtures', 'images', '960x640_jpeg.jpg')
   end
 end
 
 class AssetFileTest < ActiveSupport::TestCase
   setup do
     @path = Rails.root.join('test/fixtures/logo.png')
-    file = MigrateAssetsToAssetManager::AssetFile.open(@path)
-    @parts = file.legacy_etag.split('-')
+    MigrateAssetsToAssetManager::AssetFile.open(@path) do |file|
+      @parts = file.legacy_etag.split('-')
+    end
   end
 
   test 'returns string made up of 2 parts separated by a hyphen' do