From 0c79506d92218e3446553a9c82e736f50c468378 Mon Sep 17 00:00:00 2001 From: Alexander Graul Date: Tue, 5 Jun 2018 16:02:44 +0200 Subject: [PATCH] Refactor: new obs_controller Loading distribution information logic from OBS is moved to a separate controller along with methods that depend on this kind of information. If the connection to OBS is down, searching is disabled and the user is notified. .rubocop_todo.yml was updated because offending code was moved --- .rubocop_todo.yml | 71 ++++++---------- app/controllers/application_controller.rb | 79 ----------------- app/controllers/distributions_controller.rb | 2 +- app/controllers/obs_controller.rb | 94 +++++++++++++++++++++ app/controllers/package_controller.rb | 79 +++++++++-------- app/controllers/search_controller.rb | 2 +- 6 files changed, 165 insertions(+), 162 deletions(-) create mode 100644 app/controllers/obs_controller.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index eba9a5f4b..44749ddd7 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2018-02-02 15:19:47 +0100 using RuboCop version 0.49.1. +# on 2018-06-02 15:57:33 +0200 using RuboCop version 0.49.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -23,14 +23,13 @@ Layout/CaseIndentation: - 'app/controllers/application_controller.rb' - 'app/models/screenshot.rb' -# Offense count: 86 +# Offense count: 81 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, EnforcedStyleForEmptyBraces, SupportedStylesForEmptyBraces, SpaceBeforeBlockParameters. # SupportedStyles: space, no_space # SupportedStylesForEmptyBraces: space, no_space Layout/SpaceInsideBlockBraces: Exclude: - - 'app/controllers/application_controller.rb' - 'app/controllers/download_controller.rb' - 'app/controllers/package_controller.rb' - 'app/helpers/application_helper.rb' @@ -88,10 +87,10 @@ Lint/InheritException: - 'app/models/seeker.rb' - 'lib/api_connect.rb' -# Offense count: 5 +# Offense count: 3 Lint/RescueException: Exclude: - - 'app/controllers/application_controller.rb' + - 'app/controllers/obs_controller.rb' - 'app/models/screenshot.rb' - 'lib/api_connect.rb' @@ -128,17 +127,17 @@ Lint/UselessAccessModifier: - 'app/models/appdata.rb' - 'lib/activexml/matcher.rb' -# Offense count: 4 +# Offense count: 3 Lint/UselessAssignment: Exclude: - 'app/models/screenshot.rb' - 'lib/activexml/node.rb' -# Offense count: 37 +# Offense count: 42 Metrics/AbcSize: Max: 108 -# Offense count: 1 +# Offense count: 2 # Configuration parameters: CountComments, ExcludedMethods. Metrics/BlockLength: Max: 26 @@ -151,19 +150,19 @@ Metrics/BlockNesting: # Offense count: 6 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 470 + Max: 469 -# Offense count: 23 +# Offense count: 27 Metrics/CyclomaticComplexity: Max: 55 -# Offense count: 155 +# Offense count: 200 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Metrics/LineLength: - Max: 157 + Max: 155 -# Offense count: 41 +# Offense count: 48 # Configuration parameters: CountComments. Metrics/MethodLength: Max: 105 @@ -171,9 +170,9 @@ Metrics/MethodLength: # Offense count: 1 # Configuration parameters: CountComments. Metrics/ModuleLength: - Max: 128 + Max: 127 -# Offense count: 17 +# Offense count: 19 Metrics/PerceivedComplexity: Max: 55 @@ -223,7 +222,7 @@ Style/AndOr: - 'lib/activexml/node.rb' - 'lib/activexml/transport.rb' -# Offense count: 4 +# Offense count: 1 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, ProceduralMethods, FunctionalMethods, IgnoredMethods. # SupportedStyles: line_count_based, semantic, braces_for_chaining @@ -232,9 +231,7 @@ Style/AndOr: # IgnoredMethods: lambda, proc, it Style/BlockDelimiters: Exclude: - - 'app/controllers/application_controller.rb' - 'app/controllers/download_controller.rb' - - 'app/controllers/search_controller.rb' # Offense count: 2 Style/CaseEquality: @@ -268,7 +265,6 @@ Style/ClassVars: # Cop supports --auto-correct. Style/ColonMethodCall: Exclude: - - 'app/controllers/application_controller.rb' - 'app/controllers/download_controller.rb' - 'lib/activexml/node.rb' @@ -291,20 +287,18 @@ Style/Documentation: - 'lib/activexml/transport.rb' - 'lib/api_connect.rb' -# Offense count: 4 +# Offense count: 3 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: empty, nil, both Style/EmptyElse: Exclude: - 'app/controllers/download_controller.rb' - - 'app/models/screenshot.rb' -# Offense count: 22 +# Offense count: 21 # Cop supports --auto-correct. Style/EmptyLiteral: Exclude: - - 'app/controllers/application_controller.rb' - 'app/controllers/download_controller.rb' - 'app/models/appdata.rb' - 'app/models/seeker.rb' @@ -312,14 +306,13 @@ Style/EmptyLiteral: - 'lib/activexml/node.rb' - 'lib/activexml/transport.rb' -# Offense count: 2 +# Offense count: 1 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: compact, expanded Style/EmptyMethod: Exclude: - 'app/controllers/download_controller.rb' - - 'app/controllers/package_controller.rb' # Offense count: 5 # Cop supports --auto-correct. @@ -341,7 +334,7 @@ Style/GuardClause: - 'lib/activexml/transport.rb' - 'lib/api_connect.rb' -# Offense count: 117 +# Offense count: 109 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, UseHashRocketsWithSymbolValues, PreferHashRocketsForNonAlnumEndingSymbols. # SupportedStyles: ruby19, hash_rockets, no_mixed_keys, ruby19_no_mixed_keys @@ -353,7 +346,6 @@ Style/HashSyntax: - 'app/controllers/search_controller.rb' - 'app/helpers/application_helper.rb' - 'app/helpers/package_helper.rb' - - 'app/models/screenshot.rb' - 'app/models/seeker.rb' - 'lib/activexml/matcher.rb' - 'lib/activexml/node.rb' @@ -383,7 +375,7 @@ Style/MethodCallWithoutArgsParentheses: Exclude: - 'lib/activexml/transport.rb' -# Offense count: 16 +# Offense count: 15 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: require_parentheses, require_no_parentheses, require_no_parentheses_except_multiline @@ -391,7 +383,6 @@ Style/MethodDefParentheses: Exclude: - 'app/controllers/application_controller.rb' - 'app/controllers/download_controller.rb' - - 'app/controllers/package_controller.rb' - 'app/helpers/application_helper.rb' - 'app/helpers/package_helper.rb' - 'lib/activexml/node.rb' @@ -402,11 +393,6 @@ Style/MethodMissing: - 'app/models/seeker.rb' - 'lib/activexml/node.rb' -# Offense count: 1 -Style/MultilineBlockChain: - Exclude: - - 'app/controllers/search_controller.rb' - # Offense count: 2 # Cop supports --auto-correct. Style/MultilineIfModifier: @@ -461,7 +447,7 @@ Style/ParallelAssignment: - 'app/controllers/application_controller.rb' - 'lib/activexml/transport.rb' -# Offense count: 11 +# Offense count: 12 # Cop supports --auto-correct. # Configuration parameters: AllowSafeAssignment. Style/ParenthesesAroundCondition: @@ -501,13 +487,12 @@ Style/PreferredHashMethods: - 'lib/activexml/node.rb' - 'lib/activexml/transport.rb' -# Offense count: 10 +# Offense count: 9 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: compact, exploded Style/RaiseArgs: Exclude: - - 'app/controllers/application_controller.rb' - 'app/models/seeker.rb' - 'lib/activexml/node.rb' - 'lib/activexml/transport.rb' @@ -519,7 +504,7 @@ Style/RedundantException: Exclude: - 'lib/activexml/transport.rb' -# Offense count: 11 +# Offense count: 12 # Cop supports --auto-correct. Style/RedundantParentheses: Exclude: @@ -528,12 +513,11 @@ Style/RedundantParentheses: - 'app/helpers/package_helper.rb' - 'app/models/seeker.rb' -# Offense count: 21 +# Offense count: 20 # Cop supports --auto-correct. # Configuration parameters: AllowMultipleReturnValues. Style/RedundantReturn: Exclude: - - 'app/controllers/application_controller.rb' - 'app/helpers/application_helper.rb' - 'app/models/seeker.rb' - 'lib/activexml/node.rb' @@ -547,14 +531,13 @@ Style/RedundantSelf: - 'lib/activexml/node.rb' - 'lib/activexml/transport.rb' -# Offense count: 4 +# Offense count: 3 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, AllowInnerSlashes. # SupportedStyles: slashes, percent_r, mixed Style/RegexpLiteral: Exclude: - 'app/helpers/package_helper.rb' - - 'app/models/screenshot.rb' - 'lib/activexml/transport.rb' # Offense count: 1 @@ -570,7 +553,7 @@ Style/Semicolon: Exclude: - 'lib/api_connect.rb' -# Offense count: 224 +# Offense count: 268 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, ConsistentQuotesInMultiline. # SupportedStyles: single_quotes, double_quotes @@ -586,7 +569,7 @@ Style/StringLiteralsInInterpolation: - 'app/models/seeker.rb' - 'lib/activexml/transport.rb' -# Offense count: 13 +# Offense count: 12 # Cop supports --auto-correct. # Configuration parameters: IgnoredMethods. # IgnoredMethods: respond_to, define_method diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1bdeaa1d8..86f89af30 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -8,7 +8,6 @@ class ApplicationController < ActionController::Base before_action :validate_configuration before_action :set_language before_action :set_external_urls - before_action :set_distributions before_action :set_releases_parameters # depends on releases before_action :set_baseproject @@ -58,18 +57,6 @@ def set_language @lang = FastGettext.locale end - def set_distributions - begin - @distributions = Rails.cache.fetch('distributions', - :expires_in => 120.minutes) do - load_distributions - end - rescue - @distributions = nil - end - raise ApiConnect::Error.new(_("OBS Backend not available")) if @distributions.nil? - end - RELEASES_FILE = Rails.root.join('config', 'releases.yml').freeze def load_releases @@ -123,28 +110,6 @@ def set_baseproject @baseproject = "openSUSE:Leap:#{@stable_version}" if @baseproject.blank? end - # load available distributions - def load_distributions - logger.debug "Loading distributions" - distributions = Array.new - begin - response = ApiConnect::get("public/distributions") - doc = REXML::Document.new response.body - doc.elements.each("distributions/distribution") { |element| - dist = Hash[:name => element.elements['name'].text, :project => element.elements['project'].text, - :reponame => element.elements['reponame'].text, :repository => element.elements['repository'].text, - :dist_id => element.attributes['id'].sub(".", "")] - distributions << dist - logger.debug "Added Distribution: #{dist[:name]}" - } - distributions.unshift(Hash[:name => "ALL Distributions", :project => 'ALL']) - rescue Exception => e - logger.error "Error while loading distributions: " + e.to_s - raise - end - return distributions - end - # special version of render json with JSONP capabilities (only needed for rails < 3.0) def render_json(json, options = {}) callback, variable = params[:callback], params[:variable] @@ -182,50 +147,6 @@ def valid_project_name? name name =~ /^[[:alnum:]][-+\w.:]+$/ end - def set_search_options - @search_term = params[:q] || "" - @baseproject = if !cookies[:baseproject].nil? && @distributions.select { |d| d[:project] == cookies[:baseproject] } - cookies[:baseproject] - else - "openSUSE:Factory" - end - @search_devel = (cookies[:search_devel] == "true" ? true : false) - @search_lang = (cookies[:search_lang] == "true" ? true : false) - @search_debug = (cookies[:search_debug] == "true" ? true : false) - end - - def filter_packages - # remove maintenance projects, they are not meant for end users - @packages.reject! { |p| p.project.match(/openSUSE\:Maintenance\:/) } - @packages.reject! { |p| p.project == "openSUSE:Factory:Rebuild" } - @packages.reject! { |p| p.project.start_with?("openSUSE:Factory:Staging") } - - # only show packages - @packages = @packages.reject { |p| p.first.type == 'ymp' } - - @packages.reject! { |p| p.name.end_with?("-devel") } unless @search_devel - - unless @search_lang - @packages.reject! { |p| p.name.end_with?("-lang") || p.name.include?("-translations-") || p.name.include?("-l10n-") } - end - - unless @search_debug - @packages.reject! { |p| p.name.end_with?("-buildsymbols", "-debuginfo", "-debugsource") } - end - - # filter out ports for different arch - if @baseproject.end_with?("ARM") - @packages.filter! { |p| p.project.include?("ARM") || p.repository.include?("ARM") } - elsif @baseproject.end_with?("PowerPC") - @packages.filter! { |p| p.project.include?("PowerPC") || p.repository.include?("PowerPC") } - else # x86 - @packages.reject! do |p| - p.repository.end_with?("_ARM", "_PowerPC", "_zSystems") || - p.project.include?("ARM") || p.project.include?("PowerPC") || p.project.include?("zSystems") - end - end - end - # TODO: atm obs only offers appdata for Factory def prepare_appdata @appdata = Rails.cache.fetch("appdata", :expires_in => 12.hours) do diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb index 09d918f70..a775fef3c 100644 --- a/app/controllers/distributions_controller.rb +++ b/app/controllers/distributions_controller.rb @@ -1,4 +1,4 @@ -class DistributionsController < ApplicationController +class DistributionsController < OBSController before_action :set_releases_parameters, only: %i[index testing leap] # GET /distributions diff --git a/app/controllers/obs_controller.rb b/app/controllers/obs_controller.rb new file mode 100644 index 000000000..ae77911a5 --- /dev/null +++ b/app/controllers/obs_controller.rb @@ -0,0 +1,94 @@ +require 'api_connect' + +class OBSError < StandardError; end + +# Handle connection to OBS +class OBSController < ApplicationController + before_action :set_distributions + before_action :set_releases_parameters + + def set_distributions + @distributions = Rails.cache.fetch('distributions', + expires_in: 120.minutes, + force: true) do + load_distributions + end + rescue OBSError + @distributions = nil + @hide_search_box = true + flash[:error] = "Connection to OBS is unavailable. Functionality of this site is limited." + end + + def load_distributions + logger.debug "Loading distributions" + loaded_distros = [] + begin + response = ApiConnect.get("public/distributions") + doc = REXML::Document.new response.body + doc.elements.each("distributions/distribution") do |element| + loaded_distros << parse_distribution(element) + end + loaded_distros.unshift(Hash[name: "ALL Distributions", project: 'ALL']) + rescue Exception => e + logger.error "Error while loading distributions: " + e.to_s + raise OBSError.new, _("OBS Backend not available") + end + loaded_distros + end + + def parse_distribution(element) + dist = { + name: element.elements['name'].text, + project: element.elements['project'].text, + reponame: element.elements['reponame'].text, + repository: element.elements['repository'].text, + dist_id: element.attributes['id'].sub(".", "") + } + logger.debug "Added Distribution: #{dist[:name]}" + dist + end + + def set_search_options + @search_term = params[:q] || "" + @baseproject = if !cookies[:baseproject].nil? && @distributions.select { |d| d[:project] == cookies[:baseproject] } + cookies[:baseproject] + else + "openSUSE:Factory" + end + @search_devel = (cookies[:search_devel] == "true" ? true : false) + @search_lang = (cookies[:search_lang] == "true" ? true : false) + @search_debug = (cookies[:search_debug] == "true" ? true : false) + end + + def filter_packages + # remove maintenance projects, they are not meant for end users + @packages.reject! { |p| p.project.match(/openSUSE\:Maintenance\:/) } + @packages.reject! { |p| p.project == "openSUSE:Factory:Rebuild" } + @packages.reject! { |p| p.project.start_with?("openSUSE:Factory:Staging") } + + # only show packages + @packages = @packages.reject { |p| p.first.type == 'ymp' } + + @packages.reject! { |p| p.name.end_with?("-devel") } unless @search_devel + + unless @search_lang + @packages.reject! { |p| p.name.end_with?("-lang") || p.name.include?("-translations-") || p.name.include?("-l10n-") } + end + + unless @search_debug + @packages.reject! { |p| p.name.end_with?("-buildsymbols", "-debuginfo", "-debugsource") } + end + + # filter out ports for different arch + if @baseproject.end_with?("ARM") + @packages.filter! { |p| p.project.include?("ARM") || p.repository.include?("ARM") } + elsif @baseproject.end_with?("PowerPC") + @packages.filter! { |p| p.project.include?("PowerPC") || p.repository.include?("PowerPC") } + else # x86 + @packages.reject! do |p| + p.repository.end_with?("_ARM", "_PowerPC", "_zSystems") || + p.project.include?("ARM") || p.project.include?("PowerPC") || p.project.include?("zSystems") + end + end + end +end diff --git a/app/controllers/package_controller.rb b/app/controllers/package_controller.rb index 48d0ebdec..a9b2d513e 100644 --- a/app/controllers/package_controller.rb +++ b/app/controllers/package_controller.rb @@ -1,5 +1,4 @@ -class PackageController < ApplicationController - # before_action :set_beta_warning, :only => [:category, :categories] +class PackageController < OBSController before_action :set_search_options, :only => %i[show categories] before_action :prepare_appdata, :set_categories, :only => %i[show explore category thumbnail screenshot] @@ -9,46 +8,52 @@ def show required_parameters :package @pkgname = params[:package] raise MissingParameterError, "Invalid parameter package" unless valid_package_name? @pkgname + begin + raise OBSError if @distributions.nil? + + @search_term = params[:search_term] + @base_appdata_project = "openSUSE:Factory" + + @packages = Seeker.prepare_result("\"#{@pkgname}\"", nil, nil, nil, nil) + # only show rpms + @packages = @packages.select {|p| p.first.type != 'ymp' && p.quality != "Private"} + @default_project = @baseproject + @default_project_name = @distributions.select {|d| d[:project] == @default_project}.first[:name] + @default_repo = @distributions.select {|d| d[:project] == @default_project}.first[:repository] + @default_package = if (!@packages.select {|s| s.project == "#{@default_project}:Update"}.empty?) + @packages.select {|s| s.project == "#{@default_project}:Update"}.first + else + @packages.select {|s| [@default_project, "#{@default_project}:NonFree"].include? s.project}.first + end + + pkg_appdata = @appdata[:apps].select {|app| app[:pkgname].downcase == @pkgname.downcase} + if (!pkg_appdata.first.blank?) + @name = pkg_appdata.first[:name] + @appcategories = pkg_appdata.first[:categories] + @homepage = pkg_appdata.first[:homepage] + end - @search_term = params[:search_term] - @base_appdata_project = "openSUSE:Factory" - - @packages = Seeker.prepare_result("\"#{@pkgname}\"", nil, nil, nil, nil) - # only show rpms - @packages = @packages.select {|p| p.first.type != 'ymp' && p.quality != "Private"} - @default_project = @baseproject - @default_project_name = @distributions.select {|d| d[:project] == @default_project}.first[:name] - @default_repo = @distributions.select {|d| d[:project] == @default_project}.first[:repository] - @default_package = if (!@packages.select {|s| s.project == "#{@default_project}:Update"}.empty?) - @packages.select {|s| s.project == "#{@default_project}:Update"}.first - else - @packages.select {|s| [@default_project, "#{@default_project}:NonFree"].include? s.project}.first - end - - pkg_appdata = @appdata[:apps].select {|app| app[:pkgname].downcase == @pkgname.downcase} - if (!pkg_appdata.first.blank?) - @name = pkg_appdata.first[:name] - @appcategories = pkg_appdata.first[:categories] - @homepage = pkg_appdata.first[:homepage] - end - - @screenshot = url_for :controller => :package, :action => :screenshot, :package => @pkgname, protocol: request.protocol - @thumbnail = url_for :controller => :package, :action => :thumbnail, :package => @pkgname, protocol: request.protocol + @screenshot = url_for :controller => :package, :action => :screenshot, :package => @pkgname, protocol: request.protocol + @thumbnail = url_for :controller => :package, :action => :thumbnail, :package => @pkgname, protocol: request.protocol - filter_packages + filter_packages - @packages.each do |package| - # Backports chains up to the toolchain module for newer GCC. - # So break the link immediately. - if (package.project.match(/^openSUSE:Backports:SLE-12/)) - package.baseproject = package.project + @packages.each do |package| + # Backports chains up to the toolchain module for newer GCC. + # So break the link immediately. + if (package.project.match(/^openSUSE:Backports:SLE-12/)) + package.baseproject = package.project + end end - end - @official_projects = @distributions.map {|d| d[:project]} - # get extra distributions that are not in the default distribution list - @extra_packages = @packages.reject {|p| @distributions.map {|d| d[:project]}.include? p.baseproject } - @extra_dists = @extra_packages.map {|p| p.baseproject}.reject {|d| d.nil?}.uniq.map {|d| { :project => d }} + @official_projects = @distributions.map {|d| d[:project]} + # get extra distributions that are not in the default distribution list + @extra_packages = @packages.reject {|p| @distributions.map {|d| d[:project]}.include? p.baseproject } + @extra_dists = @extra_packages.map {|p| p.baseproject}.reject {|d| d.nil?}.uniq.map {|d| { :project => d }} + rescue OBSError + @hide_search_box = true + flash.now[:error] = _("Connection to OBS is unavailable. Functionality of this site is limited.") + end end def explore diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 873e7c5d3..66ce4af27 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -1,4 +1,4 @@ -class SearchController < ApplicationController +class SearchController < OBSController before_action :set_search_options before_action :prepare_appdata