Move the code to a helper and add a spec

src/api/app/controllers/webui/package_controller.rb

@@ -385,7 +385,7 @@ def remove_file
 
   def view_file
     @filename = params[:filename] || params[:file] || ''
-    if Package.is_binary_file?(@filename) # We don't want to display binary files
+    if binary_file?(@filename) # We don't want to display binary files
       flash[:error] = "Unable to display binary file #{@filename}"
       redirect_back(fallback_location: { action: :show, project: @project, package: @package })
       return
@@ -695,7 +695,7 @@ def package_files(rev = nil, expand = nil)
     files = []
     dir.elements('entry') do |entry|
       file = Hash[*[:name, :size, :mtime, :md5].map! { |x| [x, entry.value(x.to_s)] }.flatten]
-      file[:viewable] = !Package.is_binary_file?(file[:name]) && file[:size].to_i < 2**20 # max. 1 MB
+      file[:viewable] = !binary_file?(file[:name]) && file[:size].to_i < 2**20 # max. 1 MB
       file[:editable] = file[:viewable] && !file[:name].match?(/^_service[_:]/)
       file[:srcmd5] = dir.value('srcmd5')
       files << file

src/api/app/helpers/webui/package_helper.rb

@@ -76,7 +76,13 @@ def expand_diff?(filename, state)
   end
 
   def viewable_file?(filename)
-    !Package.is_binary_file?(filename) && filename.exclude?('/')
+    !binary_file?(filename) && filename.exclude?('/')
+  end
+
+  def binary_file?(filename)
+    return false unless (mime = Marcel::Magic.by_path(filename))
+
+    !mime.text?
   end
 
   def calculate_revision_on_state(revision, state)

src/api/app/models/package.rb

@@ -1186,12 +1186,6 @@ def enable_for_repository(repo_name)
     store if update_needed
   end
 
-  def self.is_binary_file?(filename)
-    return false unless (mime = Marcel::Magic.by_path(filename))
-
-    !mime.text?
-  end
-
   def serviceinfo
     begin
       dir = Directory.hashed(project: project.name, package: name)

src/api/spec/helpers/webui/package_helper_spec.rb

@@ -260,4 +260,34 @@
     it { expect(viewable_file?('some_file.rb')).to be(true) }
     it { expect(viewable_file?('some_file.bin')).to be(false) }
   end
+
+  describe '#binary_file?' do
+    it { expect(binary_file?('anNhbG.exe')).to be(true) }
+    it { expect(binary_file?('ZramE7b2.bin')).to be(true) }
+    it { expect(binary_file?('g3cWhqem.tar')).to be(true) }
+    it { expect(binary_file?('x3ZXVqaDg.tar.bz')).to be(true) }
+    it { expect(binary_file?('zNDd1NDU5O.tar.bz2')).to be(true) }
+    it { expect(binary_file?('Dd5MnE0Ly.tar.zst')).to be(true) }
+    it { expect(binary_file?('4vLi8uNTMvN.gem')).to be(true) }
+    it { expect(binary_file?('S40LjUzOV.gif')).to be(true) }
+    it { expect(binary_file?('BISTs7U.jpg')).to be(true) }
+    it { expect(binary_file?('RGVUpF.jpeg')).to be(true) }
+    it { expect(binary_file?('U0hLSl.ttf')).to be(true) }
+    it { expect(binary_file?('ERgo.zip')).to be(true) }
+    it { expect(binary_file?('NDg3Mj/UyMDg3U.tar.gz')).to be(true) }
+    it { expect(binary_file?('kZJVUhWSk.png')).to be(true) }
+    it { expect(binary_file?('tIRkdJREta.gem')).to be(true) }
+    it { expect(binary_file?('WFk5V1dXV.cert')).to be(true) }
+    it { expect(binary_file?('MzQ1OHI3OGZ5ZW.svgz')).to be(true) }
+    it { expect(binary_file?('hzeml5OTc2e.taz')).to be(true) }
+    it { expect(binary_file?('WZ6OTdneXJqa.tlz')).to be(true) }
+    it { expect(binary_file?('3c0ZWdha.diff')).to be(false) }
+    it { expect(binary_file?('GpyNWpoc.txt')).to be(false) }
+    it { expect(binary_file?('0ZLREpoCg.csv')).to be(false) }
+    it { expect(binary_file?('YWprd2Fqb.pm')).to be(false) }
+    it { expect(binary_file?('GtqbWttLG.c')).to be(false) }
+    it { expect(binary_file?('ZuYW5tIGJ0.rb')).to be(false) }
+    it { expect(binary_file?('NGkzNzV5.h')).to be(false) }
+    it { expect(binary_file?('Jmc2hiCg.spec')).to be(false) }
+  end
 end

src/api/test/unit/package_test.rb

@@ -312,52 +312,10 @@ def test_activity
     end
   end
 
-  test 'is_binary_file?' do
-    file_paths = [
-      '/tmp/some/file',
-      '/srv/www/another_file_',
-      '/var/lib/cache/file with spaces'
-    ]
-
-    filename = ''
-
-    # binary files
-    generate_suffixes(['exe', 'bin', 'bz', 'bz2', 'gem', 'gif', 'jpg', 'jpeg', 'ttf', 'zip', 'gz', 'png']).each do |suffix|
-      file_paths.each do |file_path|
-        filename = file_path + '.' + suffix
-        assert Package.is_binary_file?(filename), "File #{filename} should be treated as binary"
-      end
-    end
-
-    # these aren't binary
-    generate_suffixes(['diff', 'txt', 'csv', 'pm', 'c', 'rb', 'h']).each do |suffix|
-      file_paths.each do |file_path|
-        filename = file_path + '.' + suffix
-        assert_not Package.is_binary_file?(filename), "File #{filename} should not be treated as binary"
-      end
-    end
-  end
-
   test 'fixtures name' do
     assert_equal 'home_Iggy_TestPack', packages(:home_Iggy_TestPack).fixtures_name
   end
 
-  private
-
-  # gets list of strings and tries to generate another longer list
-  # with some letters up/down-cased, based on the original list
-  def generate_suffixes(suffixes_in)
-    suffixes_out = suffixes_in.dup
-    # some lower-cased suffixes
-    suffixes_out.collect!(&:downcase)
-    # the same ones capitalized
-    suffixes_out.concat(suffixes_in.collect(&:capitalize))
-    # the same ones upper-cased
-    suffixes_out.concat(suffixes_in.collect(&:upcase))
-    # the same ones swap-cased
-    suffixes_out.concat(suffixes_in.collect { |i| i.capitalize.swapcase })
-  end
-
   test 'default scope does not include forbidden projects' do
     # assert that unscoped the forbidden projects are included
     assert Package.unscoped.all.where(project_id: Relationship.forbidden_project_ids).any?