Merge pull request openSUSE#15753 from hellcp-work/files-controller-p1

Remove package_files method from Webui::PackageController

Author: hellcp-work

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

@@ -83,7 +83,7 @@ def show
       authorize @current_notification, :update?, policy_class: NotificationPolicy
     end
 
-    @services = @files.any? { |file| file[:name] == '_service' }
+    @services = @files.any? { |file| file['name'] == '_service' }
 
     @package.cache_revisions(@revision)
 
@@ -305,14 +305,10 @@ def view_file
     @expand = params[:expand]
     @addeditlink = false
     if User.possibly_nobody.can_modify?(@package) && @rev.blank? && @package.scmsync.blank?
-      begin
-        files = package_files(@rev, @expand)
-      rescue Backend::Error
-        files = []
-      end
+      files = @package.dir_hash({ rev: @rev, expand: @expand }.compact).elements('entry')
       files.each do |file|
-        if file[:name] == @filename
-          @addeditlink = file[:editable]
+        if file['name'] == @filename
+          @addeditlink = editable_file?(@filename, file['size'].to_i)
           break
         end
       end
@@ -499,27 +495,6 @@ def validate_xml
     render layout: false, status: :bad_request, partial: 'layouts/webui/flash', object: flash
   end
 
-  def package_files(rev = nil, expand = nil)
-    query = {}
-    query[:expand]  = expand  if expand
-    query[:rev]     = rev     if rev
-
-    dir_xml = @package.source_file(nil, query)
-    return [] if dir_xml.blank?
-
-    dir = Xmlhash.parse(dir_xml)
-    @serviceinfo = dir.elements('serviceinfo').first
-    files = []
-    dir.elements('entry') do |entry|
-      file = Hash[*%i[name size mtime md5].map! { |x| [x, entry.value(x.to_s)] }.flatten]
-      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
-    end
-    files
-  end
-
   def require_architecture
     @architecture = Architecture.archcache[params[:arch]]
     return if @architecture
@@ -549,7 +524,8 @@ def set_file_details
       @current_rev = @package.rev
       @revision = @current_rev if !@revision && !@srcmd5 # on very first page load only
 
-      @files = package_files(@srcmd5 || @revision, @expand)
+      files_xml = @package.source_file(nil, { rev: @srcmd5 || @revision, expand: @expand }.compact)
+      @files = Xmlhash.parse(files_xml).elements('entry')
     rescue Backend::Error => e
       # TODO: crudest hack ever!
       if e.summary == 'service in progress' && @expand == 1

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

@@ -75,10 +75,16 @@ def expand_diff?(filename, state)
     state != 'deleted' && filename.exclude?('/') && (filename == '_patchinfo' || filename.ends_with?('.spec', '.changes'))
   end
 
-  def viewable_file?(filename)
+  def viewable_file?(filename, size = nil)
+    return false if size && size > 1.megabyte
+
     !binary_file?(filename) && filename.exclude?('/')
   end
 
+  def editable_file?(filename, size = nil)
+    viewable_file?(filename, size) && !filename.match?(/^_service[_:]/)
+  end
+
   def binary_file?(filename)
     return false unless (mime = Marcel::Magic.by_path(filename))
 

src/api/app/views/webui/package/_file.html.haml

@@ -1,26 +1,26 @@
-- can_be_removed = removable_file?(file_name: file[:name], package: package) && can_modify
-%tr{ id: "file-#{valid_xml_id(file[:name])}" }
+- can_be_removed = removable_file?(file_name: file['name'], package: package) && can_modify
+%tr{ id: "file-#{valid_xml_id(file['name'])}" }
   %td.text-word-break-all
-    - link_opts = { action: :view_file, project: project, package: package, filename: file[:name], expand: expand }
+    - link_opts = { action: :view_file, project: project, package: package, filename: file['name'], expand: expand }
     - unless is_current_rev
-      - link_opts[:rev] = file[:srcmd5]
-    = link_to_if(file[:viewable], nbsp(file[:name]), link_opts)
+      - link_opts[:rev] = srcmd5
+    = link_to_if(viewable_file?(file['name'], file['size'].to_i), nbsp(file['name']), link_opts)
   %td.text-nowrap
-    %span.d-none= file[:size].rjust(10, '0')
-    = human_readable_fsize(file[:size])
-  %td.text-nowrap{ data: { order: "-#{file[:mtime]}" } }
-    = render TimeComponent.new(time: Time.zone.at(file[:mtime].to_i))
+    %span.d-none= file['size'].rjust(10, '0')
+    = human_readable_fsize(file['size'])
+  %td.text-nowrap{ data: { order: "-#{file['mtime']}" } }
+    = render TimeComponent.new(time: Time.zone.at(file['mtime'].to_i))
   / limit download for anonymous user to avoid getting killed by crawlers
   - unless nobody
     %td.text-center<
-      = link_to(file_url(project.name, package.name, file[:name], file[:srcmd5]), title: 'Download file', class: 'me-1') do
+      = link_to(file_url(project.name, package.name, file['name'], srcmd5), title: 'Download file', class: 'me-1') do
         %i.fas.fa-download.text-secondary
       - if can_be_removed
         = link_to('#', title: 'Delete file', data: { 'bs-toggle': 'modal',
                                                      'bs-target': '#delete-file-modal',
-                                                     confirmation_text: "Please confirm deletion of file '#{file[:name]}'",
+                                                     confirmation_text: "Please confirm deletion of file '#{file['name']}'",
                                                      action: url_for(action: :remove_file,
                                                                      project: project,
                                                                      package: package,
-                                                                     filename: file[:name]) }) do
+                                                                     filename: file['name']) }) do
           %i.fas.fa-times-circle.text-danger

src/api/app/views/webui/package/_files_view.html.haml

@@ -13,9 +13,9 @@
             %th Actions
       %tbody
         - file_locals = { package: package, project: project, expand: expand, is_current_rev: is_current_rev,
-        can_modify: user_can_modify_package, nobody: nobody }
+        can_modify: user_can_modify_package, nobody: nobody, srcmd5: srcmd5 }
         = render partial: 'file', collection: files,
-        cached: proc { |file| [file[:name], file[:mtime], file[:md5], file_locals].hash }, locals: file_locals
+        cached: proc { |file| [file['name'], file['mtime'], file['md5'], file_locals].hash }, locals: file_locals
   - else
     %i This package has no files yet
   - if user_can_modify_package