From d58d7741ecf0e32c67db5803949322d054ca121a Mon Sep 17 00:00:00 2001 From: Stephan Kulow Date: Thu, 19 Jul 2012 10:36:46 +0200 Subject: [PATCH] [api] add options for get_by_* the old booleans were just too fragile and too non-verbose --- src/api/app/controllers/build_controller.rb | 8 ++--- src/api/app/controllers/public_controller.rb | 2 +- src/api/app/controllers/request_controller.rb | 12 +++---- src/api/app/controllers/source_controller.rb | 31 +++++++++---------- .../app/controllers/statistics_controller.rb | 8 ++--- src/api/app/controllers/tag_controller.rb | 6 ++-- src/api/app/helpers/maintenance_helper.rb | 20 ++++++------ src/api/app/models/db_package.rb | 20 ++++++++---- 8 files changed, 57 insertions(+), 50 deletions(-) diff --git a/src/api/app/controllers/build_controller.rb b/src/api/app/controllers/build_controller.rb index 44a9a4847c8..1ae1a9db645 100644 --- a/src/api/app/controllers/build_controller.rb +++ b/src/api/app/controllers/build_controller.rb @@ -5,7 +5,7 @@ def index # for read access and visibility permission check if params[:package] and not ["_repository", "_jobhistory"].include?(params[:package]) - DbPackage.get_by_project_and_name( params[:project], params[:package], false ) + DbPackage.get_by_project_and_name( params[:project], params[:package], use_source: false ) else DbProject.get_by_name params[:project] end @@ -17,7 +17,7 @@ def index if @http_user.is_admin? # check for a local package instance - DbPackage.get_by_project_and_name( params[:project], params[:package], false ) + DbPackage.get_by_project_and_name( params[:project], params[:package], use_source: false, follow_project_links: false ) pass_to_backend else render_error :status => 403, :errorcode => "execute_cmd_no_permission", @@ -119,7 +119,7 @@ def buildinfo # for osc local package build in this repository DbProject.get_by_name params[:project] else - DbPackage.get_by_project_and_name params[:project], params[:package], false + DbPackage.get_by_project_and_name params[:project], params[:package], use_source: false end path = "/build/#{params[:project]}/#{params[:repository]}/#{params[:arch]}/#{params[:package]}/_buildinfo" @@ -150,7 +150,7 @@ def file if params[:package] == "_repository" prj = DbProject.get_by_name params[:project] else - pkg = DbPackage.get_by_project_and_name params[:project], params[:package], false + pkg = DbPackage.get_by_project_and_name params[:project], params[:package], use_source: false prj = pkg.db_project if pkg.class == DbPackage end diff --git a/src/api/app/controllers/public_controller.rb b/src/api/app/controllers/public_controller.rb index ffe755fada8..7e2797f5e83 100644 --- a/src/api/app/controllers/public_controller.rb +++ b/src/api/app/controllers/public_controller.rb @@ -21,7 +21,7 @@ def check_package_access(project, package, use_source=true) key = "public_package:" + project + ":" + package allowed = Rails.cache.fetch(key, :expires_in => 30.minutes) do begin - DbPackage.get_by_project_and_name(project, package, use_source=false) + DbPackage.get_by_project_and_name(project, package, use_source: false) true rescue Exception false diff --git a/src/api/app/controllers/request_controller.rb b/src/api/app/controllers/request_controller.rb index bcf57218e85..4c72e5f7408 100644 --- a/src/api/app/controllers/request_controller.rb +++ b/src/api/app/controllers/request_controller.rb @@ -379,7 +379,7 @@ def create_expand_package(action, packages) end # Will this be a new package ? unless missing_ok_link - unless e and DbPackage.exists_by_project_and_name( tprj, tpkg, true, false) + unless e and DbPackage.exists_by_project_and_name( tprj, tpkg, follow_project_links: true, allow_remote_packages: false) if action.action_type == :maintenance_release newPackages << pkg pkg.db_project.repositories.includes(:release_targets).each do |repo| @@ -588,7 +588,7 @@ def check_action_permission(action) return false end if action.source_package - spkg = DbPackage.get_by_project_and_name(action.source_project, action.source_package, true, true) + spkg = DbPackage.get_by_project_and_name(action.source_project, action.source_package, use_source: true, follow_project_links: true) end end @@ -980,9 +980,9 @@ def command_diff else # for requests not yet accepted or accepted with OBS 2.0 and before tpkg = linked_tpkg = nil - if DbPackage.exists_by_project_and_name( target_project, target_package, false ) + if DbPackage.exists_by_project_and_name( target_project, target_package, follow_project_links: false ) tpkg = DbPackage.get_by_project_and_name( target_project, target_package ) - elsif DbPackage.exists_by_project_and_name( target_project, target_package, true ) + elsif DbPackage.exists_by_project_and_name( target_project, target_package, follow_project_links: true ) tpkg = linked_tpkg = DbPackage.get_by_project_and_name( target_project, target_package ) else DbProject.get_by_name( target_project ) @@ -1243,7 +1243,7 @@ def command_changestate end # accept also a remote source package if source_package.nil? and [ :submit ].include? action.action_type - unless DbPackage.exists_by_project_and_name( source_project.name, action.source_package, true, true) + unless DbPackage.exists_by_project_and_name( source_project.name, action.source_package, follow_project_links: true, allow_remote_packages: true) render_error :status => 404, :errorcode => "unknown_package", :message => "Source package is missing for request #{req.id} (type #{action.action_type})" return @@ -1634,7 +1634,7 @@ def command_changestate elsif action.action_type == :delete if action.target_package - package = DbPackage.get_by_project_and_name(action.target_project, action.target_package, true, false) + package = DbPackage.get_by_project_and_name(action.target_project, action.target_package, use_source: true, follow_project_links: false) package.destroy delete_path = "/source/#{action.target_project}/#{action.target_package}" else diff --git a/src/api/app/controllers/source_controller.rb b/src/api/app/controllers/source_controller.rb index 4e27957ba0b..2d1674c68a6 100644 --- a/src/api/app/controllers/source_controller.rb +++ b/src/api/app/controllers/source_controller.rb @@ -320,7 +320,7 @@ def index_package # The target must exist, except for following cases if (request.post? and command == 'undelete') or (request.get? and deleted_package) tprj = DbProject.get_by_name(target_project_name) - if DbPackage.exists_by_project_and_name(target_project_name, target_package_name, follow_project_links=false) + if DbPackage.exists_by_project_and_name(target_project_name, target_package_name, follow_project_links: false) render_error :status => 404, :errorcode => "package_exists", :message => "the package exists already #{tprj.name} #{target_package_name}" return @@ -337,9 +337,8 @@ def index_package # The branch command may be used just for simulation unless params[:dryrun] # we require a target, but are we allowed to modify the existing target ? - # FIXME2.4: this assignment of follow_project is bogus - if DbProject.exists_by_name(target_project_name) and DbPackage.exists_by_project_and_name(target_project_name, target_package_name, follow_project_links=false) - tpkg = DbPackage.get_by_project_and_name(target_project_name, target_package_name, follow_project_links=false) + if DbProject.exists_by_name(target_project_name) and DbPackage.exists_by_project_and_name(target_project_name, target_package_name, follow_project_links: false) + tpkg = DbPackage.get_by_project_and_name(target_project_name, target_package_name, use_source: false, follow_project_links: false) unless @http_user.can_modify_package?(tpkg) render_error :status => 403, :errorcode => "cmd_execution_no_permission", :message => "no permission to execute command '#{command}' for package #{tpkg.name} in project #{tpkg.db_project.name}" @@ -370,9 +369,9 @@ def index_package if [ '_project', '_pattern' ].include? target_package_name and not request.delete? tprj = DbProject.get_by_name target_project_name else - use_source = true - use_source = false if command == "showlinked" - tpkg = DbPackage.get_by_project_and_name(target_project_name, target_package_name, use_source, follow_project_links) + use_source = true + use_source = false if command == "showlinked" + tpkg = DbPackage.get_by_project_and_name(target_project_name, target_package_name, use_source: use_source, follow_project_links: follow_project_links) tprj = tpkg.db_project unless tpkg.nil? # for remote package case if request.delete? or (request.post? and not read_commands.include? command) # unlock @@ -499,7 +498,7 @@ def attribute_meta # valid post commands raise IllegalRequestError.new "invalid_project_name" unless valid_project_name?(params[:project]) if params[:package] and params[:package] != "_project" - @attribute_container = DbPackage.get_by_project_and_name(params[:project], params[:package], false) + @attribute_container = DbPackage.get_by_project_and_name(params[:project], params[:package], use_source: false) else # project if DbProject.is_remote_project?(params[:project]) @@ -938,7 +937,7 @@ def package_meta if request.get? # GET /source/:project/:package/_meta - pack = DbPackage.get_by_project_and_name( project_name, package_name, false ) + pack = DbPackage.get_by_project_and_name( project_name, package_name, use_source: false ) if params.has_key?(:rev) or pack.nil? # and not pro_name # check if this comes from a remote project, also true for _project package @@ -973,13 +972,13 @@ def package_meta end # check for project - if DbPackage.exists_by_project_and_name( project_name, package_name, false ) + if DbPackage.exists_by_project_and_name( project_name, package_name, follow_project_links: false ) # is lock explicit set to disable ? allow the un-freeze of the project in that case ... ignoreLock = nil # unlock only via command for now # ignoreLock = 1 if Xmlhash.parse(request.raw_post).get("lock")["disable"] - pkg = DbPackage.get_by_project_and_name( project_name, package_name, false ) + pkg = DbPackage.get_by_project_and_name( project_name, package_name, use_source: false ) unless @http_user.can_modify_package?(pkg, ignoreLock) render_error :status => 403, :errorcode => "change_package_no_permission", :message => "no permission to modify package '#{pkg.db_project.name}'/#{pkg.name}" @@ -1110,7 +1109,7 @@ def file tpackage_name = data.value("package") || package_name if data.has_attribute? 'missingok' DbProject.get_by_name(tproject_name) # permission check - if DbPackage.exists_by_project_and_name(tproject_name, tpackage_name, true, true) + if DbPackage.exists_by_project_and_name(tproject_name, tpackage_name, follow_project_links: true, allow_remote_packages: true) render_error :status => 400, :errorcode => 'not_missing', :message => "Link contains a missingok statement but link target (#{tproject_name}/#{tpackage_name}) exists." return @@ -1158,7 +1157,7 @@ def file # _pattern was not a real package in former OBS 2.0 and before, so we need to create the # package here implicit to stay api compatible. # FIXME3.0: to be revisited - if package_name == "_pattern" and not DbPackage.exists_by_project_and_name( project_name, package_name, false ) + if package_name == "_pattern" and not DbPackage.exists_by_project_and_name( project_name, package_name, follow_project_links: false ) pack = DbPackage.new(:name => "_pattern", :title => "Patterns", :description => "Package Patterns") prj.db_packages << pack pack.save @@ -1207,7 +1206,7 @@ def package_flags return end - pack = DbPackage.get_by_project_and_name( project_name, package_name, false ) + pack = DbPackage.get_by_project_and_name( project_name, package_name, use_source: false ) render :text => pack.expand_flags.to_json, :content_type => 'text/json' end @@ -1874,7 +1873,7 @@ def index_package_linktobranch pkg_rev = params[:rev] pkg_linkrev = params[:linkrev] - pkg = DbPackage.get_by_project_and_name prj_name, pkg_name, true, false + pkg = DbPackage.get_by_project_and_name prj_name, pkg_name, use_source: true, follow_project_links: false #convert link to branch rev = "" @@ -1914,7 +1913,7 @@ def index_package_set_flag prj_name = params[:project] pkg_name = params[:package] - pkg = DbPackage.get_by_project_and_name prj_name, pkg_name, true, false + pkg = DbPackage.get_by_project_and_name prj_name, pkg_name, use_source: true, follow_project_links: false # first remove former flags of the same class begin diff --git a/src/api/app/controllers/statistics_controller.rb b/src/api/app/controllers/statistics_controller.rb index 52a52d9489e..e5571a9e34f 100644 --- a/src/api/app/controllers/statistics_controller.rb +++ b/src/api/app/controllers/statistics_controller.rb @@ -37,7 +37,7 @@ def rating @package = params[:package] object = DbProject.get_by_name(@project) - object = DbPackage.get_by_project_and_name(@project, @package, false, false) if @package + object = DbPackage.get_by_project_and_name(@project, @package, use_source: false, follow_project_links: false) if @package if request.get? @@ -124,7 +124,7 @@ def most_active_packages def activity @project = DbProject.get_by_name(params[:project]) - @package = DbPackage.get_by_project_and_name(params[:project], params[:package], false, false) if params[:package] + @package = DbPackage.get_by_project_and_name(params[:project], params[:package], use_source: false, follow_project_links: false) if params[:package] end @@ -149,7 +149,7 @@ def latest_added def added_timestamp @project = DbProject.get_by_name(params[:project]) - @package = DbPackage.get_by_project_and_name(params[:project], params[:package], false, true) + @package = DbPackage.get_by_project_and_name(params[:project], params[:package], use_source: false, follow_project_links: true) # is it used at all ? end @@ -195,7 +195,7 @@ def latest_updated def updated_timestamp @project = DbProject.get_by_name(params[:project]) - @package = DbPackage.get_by_project_and_name(params[:project], params[:package], false, true) + @package = DbPackage.get_by_project_and_name(params[:project], params[:package], use_source: false, follow_project_links: true) end diff --git a/src/api/app/controllers/tag_controller.rb b/src/api/app/controllers/tag_controller.rb index d52a6802520..3800f0900cf 100644 --- a/src/api/app/controllers/tag_controller.rb +++ b/src/api/app/controllers/tag_controller.rb @@ -162,7 +162,7 @@ def get_tags_by_user_and_package( do_render=true ) @type = "package" @name = params[:package] - @package = DbPackage.get_by_project_and_name(params[:project], params[:package], false, false) + @package = DbPackage.get_by_project_and_name(params[:project], params[:package], use_source: false, follow_project_links: false) @project = @package.db_project @tags = @package.tags.where("taggings.user_id = ?",user.id).order(:name).all @@ -235,7 +235,7 @@ def tagcloud collection.each_package do |package| project = DbProject.get_by_name(package.project) - pack = DbPackage.get_by_project_and_name( project.name, package.name, false, false ) + pack = DbPackage.get_by_project_and_name( project.name, package.name, use_source: false, follow_project_links: false ) packages << pack end @@ -364,7 +364,7 @@ def update_tags_by_object_and_user if params[:package] logger.debug "[TAG:] Package selected" - @package = DbPackage.get_by_project_and_name(params[:project], params[:package], false, false) + @package = DbPackage.get_by_project_and_name(params[:project], params[:package], use_source: false, follow_project_links: false) old_tags = get_tags_by_user_and_package( false ) old_tags.each do |old_tag| diff --git a/src/api/app/helpers/maintenance_helper.rb b/src/api/app/helpers/maintenance_helper.rb index 22611f36b2c..66abdcb2cdc 100644 --- a/src/api/app/helpers/maintenance_helper.rb +++ b/src/api/app/helpers/maintenance_helper.rb @@ -118,8 +118,8 @@ def merge_into_maintenance_incident(incidentProject, base, releaseproject=nil, r end # patchinfos are handled as new packages if pkg.db_package_kinds.find_by_kind 'patchinfo' - if DbPackage.exists_by_project_and_name(incidentProject.name, pkg.name, follow_project_links=false) - new_pkg = DbPackage.get_by_project_and_name(incidentProject.name, pkg.name, follow_project_links=false) + if DbPackage.exists_by_project_and_name(incidentProject.name, pkg.name, follow_project_links: false) + new_pkg = DbPackage.get_by_project_and_name(incidentProject.name, pkg.name, use_source: false, follow_project_links: false) else new_pkg = incidentProject.db_packages.create(:name => pkg.name, :title => pkg.title, :description => pkg.description) new_pkg.flags.create(:status => "enable", :flag => "build") @@ -142,7 +142,7 @@ def merge_into_maintenance_incident(incidentProject, base, releaseproject=nil, r :project => releaseproject, :package => package_name } branch_params[:requestid] = request.id if request # it is fine to have new packages - unless DbPackage.exists_by_project_and_name(releaseproject, package_name, follow_project_links=true) + unless DbPackage.exists_by_project_and_name(releaseproject, package_name, follow_project_links: true) branch_params[:missingok]= 1 end ret = do_branch branch_params @@ -165,8 +165,8 @@ def merge_into_maintenance_incident(incidentProject, base, releaseproject=nil, r # a new package for all targets if e and e.attributes["package"] - if DbPackage.exists_by_project_and_name(incidentProject.name, pkg.name, follow_project_links=false) - new_pkg = DbPackage.get_by_project_and_name(incidentProject.name, pkg.name, follow_project_links=false) + if DbPackage.exists_by_project_and_name(incidentProject.name, pkg.name, follow_project_links: false) + new_pkg = DbPackage.get_by_project_and_name(incidentProject.name, pkg.name, use_source: false, follow_project_links: false) else new_pkg = DbPackage.new(:name => pkg.name, :title => pkg.title, :description => pkg.description) incidentProject.db_packages << new_pkg @@ -206,8 +206,8 @@ def release_package(sourcePackage, targetProjectName, targetPackageName, revisio # create package container, if missing tpkg = nil - if DbPackage.exists_by_project_and_name(targetProject.name, targetPackageName, follow_project_links=false) - tpkg = DbPackage.get_by_project_and_name(targetProject.name, targetPackageName, follow_project_links=false) + if DbPackage.exists_by_project_and_name(targetProject.name, targetPackageName, follow_project_links: false) + tpkg = DbPackage.get_by_project_and_name(targetProject.name, targetPackageName, use_source: false, follow_project_links: false) else tpkg = DbPackage.new(:name => targetPackageName, :title => sourcePackage.title, :description => sourcePackage.description) targetProject.db_packages << tpkg @@ -310,8 +310,8 @@ def release_package(sourcePackage, targetProjectName, targetPackageName, revisio # only if package does not contain a _patchinfo file lpkg = nil - if DbPackage.exists_by_project_and_name(targetProject.name, basePackageName, follow_project_links=false) - lpkg = DbPackage.get_by_project_and_name(targetProject.name, basePackageName, follow_project_links=false) + if DbPackage.exists_by_project_and_name(targetProject.name, basePackageName, follow_project_links: false) + lpkg = DbPackage.get_by_project_and_name(targetProject.name, basePackageName, use_source: false, follow_project_links: false) else lpkg = DbPackage.new(:name => basePackageName, :title => sourcePackage.title, :description => sourcePackage.description) targetProject.db_packages << lpkg @@ -432,7 +432,7 @@ def do_branch params pkg = nil prj = DbProject.get_by_name params[:project] if params[:missingok] - if DbPackage.exists_by_project_and_name(params[:project], params[:package], true, true) + if DbPackage.exists_by_project_and_name(params[:project], params[:package], follow_project_links: true, allow_remote_packages: true) return { :status => 400, :errorcode => 'not_missing', :message => "Branch call with missingok paramater but branch source (#{params[:project]}/#{params[:package]}) exists." } end diff --git a/src/api/app/models/db_package.rb b/src/api/app/models/db_package.rb index 760a7dc2dbb..a3d3ed94b47 100644 --- a/src/api/app/models/db_package.rb +++ b/src/api/app/models/db_package.rb @@ -77,9 +77,15 @@ def store_axml( package ) # returns an object of package or raises an exception # should be always used when a project is required # in case you don't access sources or build logs in any way use - # use_source=false to skip check for sourceaccess permissions + # use_source: false to skip check for sourceaccess permissions # function returns a nil object in case the package is on remote instance - def get_by_project_and_name( project, package, use_source=true, follow_project_links=true ) + def get_by_project_and_name( project, package, opts = {} ) + raise "get_by_project_and_name expects a hash as third arg" unless opts.kind_of? Hash + opts = { use_source: true, follow_project_links: true }.merge(opts) + use_source = opts.delete :use_source + follow_project_links = opts.delete :follow_project_links + raise "get_by_project_and_name passed unknown options #{opts.inspect}" unless opts.empty? + logger.debug "get_by_project_and_name #{opts.inspect}" if project.class == DbProject prj = project else @@ -113,9 +119,11 @@ def get_by_project_and_name( project, package, use_source=true, follow_project_l end # to check existens of a project (local or remote) - def exists_by_project_and_name( project, package, follow_project_links=true, allow_remote_packages=false ) + def exists_by_project_and_name( project, package, opts = {} ) + raise "get_by_project_and_name expects a hash as third arg" unless opts.kind_of? Hash + opts = { follow_project_links: true, allow_remote_packages: false}.merge(opts) if DbProject.is_remote_project?( project ) - if allow_remote_packages + if opts[:allow_remote_packages] begin answer = Suse::Backend.get("/source/#{URI.escape(project)}/#{URI.escape(package)}") return true if answer @@ -125,14 +133,14 @@ def exists_by_project_and_name( project, package, follow_project_links=true, all return false end prj = DbProject.get_by_name( project ) - if follow_project_links + if opts[:follow_project_links] pkg = prj.find_package(package) else pkg = prj.db_packages.find_by_name(package) end if pkg.nil? # local project, but package may be in a linked remote one - if allow_remote_packages + if opts[:allow_remote_packages] begin answer = Suse::Backend.get("/source/#{URI.escape(project)}/#{URI.escape(package)}") return true if answer