From 55a470200b47fba5304b4385cf49578fa7b8b6d8 Mon Sep 17 00:00:00 2001 From: Stephan Kulow Date: Sun, 4 Mar 2012 21:12:50 +0100 Subject: [PATCH] [api] instead of relying on ultra complex joins, just find out what projects the user can't see (which can be reused/cached) and give that as block list in SQL calls This also makes it easy to later on filter for roles of the user, because we only have to do it in one call --- src/api/app/controllers/source_controller.rb | 13 ++++++++----- src/api/app/models/db_package.rb | 9 ++++----- src/api/app/models/db_project.rb | 14 +++++--------- .../app/models/project_user_role_relationship.rb | 16 ++++++++++++++++ 4 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/api/app/controllers/source_controller.rb b/src/api/app/controllers/source_controller.rb index 51b150b6a1f..800341a0ed9 100644 --- a/src/api/app/controllers/source_controller.rb +++ b/src/api/app/controllers/source_controller.rb @@ -63,9 +63,7 @@ def projectlist output = String.new output << "\n" output << "\n" - dir.each do |item| - output << " \n" - end + output << dir.map { |item| " \n" }.join output << "\n" render :text => output, :content_type => "text/xml" return @@ -110,8 +108,13 @@ def index_project end pass_to_backend else - @dir = Package.find :all, :project => project_name - render :text => @dir.dump_xml, :content_type => "text/xml" + prj = DbProject.find_by_name project_name + dir = prj.db_packages.map { |i| i.name }.sort + output = String.new + output << "\n" + output << dir.map { |item| " \n" }.join + output << "\n" + render :text => output, :content_type => "text/xml" end end end diff --git a/src/api/app/models/db_package.rb b/src/api/app/models/db_package.rb index 10831c13160..f6882424e18 100644 --- a/src/api/app/models/db_package.rb +++ b/src/api/app/models/db_package.rb @@ -80,17 +80,16 @@ def securedfind_every(options) # limit to projects which have no "access" flag, except user has any role inside # FIXME3.0: we should limit this to maintainer and reader role only ? # - options[:joins] += " LEFT JOIN flags f ON f.db_project_id = prj.id AND (ISNULL(f.flag) OR flag = 'access')" # filter projects with or without access flag - options[:joins] += " LEFT JOIN project_user_role_relationships ur ON ur.db_project_id = prj.id" - cond = "((f.flag = 'access' AND ur.bs_user_id = #{User.current ? User.currentID : User.nobodyID}) OR ISNULL(f.flag))" + fprjs = ProjectUserRoleRelationship.forbidden_project_ids + cond = "(prj.id not in (#{fprjs.join(',')}))" unless fprjs.blank? if options[:conditions].nil? - options[:conditions] = cond + options[:conditions] = cond if cond else if options[:conditions].class == String options[:conditions] = options[:conditions] else - options[:conditions][0] = cond + "AND (" + options[:conditions][0] + ")" + options[:conditions][0] = cond + "AND (" + options[:conditions][0] + ")" if cond end end end diff --git a/src/api/app/models/db_project.rb b/src/api/app/models/db_project.rb index a5a4541349a..507e004c2f7 100644 --- a/src/api/app/models/db_project.rb +++ b/src/api/app/models/db_project.rb @@ -188,23 +188,19 @@ def securedfind_every(options) # limit to projects which have no "access" flag, except user has any role inside # FIXME3.0: we should limit this to maintainer and reader role only ? # - options[:joins] = "" if options[:joins].nil? - options[:joins] += " LEFT JOIN flags f ON f.db_project_id = db_projects.id AND (ISNULL(f.flag) OR flag = 'access')" # filter projects with or without access flag - options[:joins] += " LEFT OUTER JOIN project_user_role_relationships ur ON ur.db_project_id = db_projects.id" - options[:group] = "db_projects.id" unless options[:group] # is creating a DISTINCT select to have uniq results - - cond = "((f.flag = 'access' AND ur.bs_user_id = #{User.current ? User.currentID : User.nobodyID}) OR ISNULL(f.flag))" + fprjs = ProjectUserRoleRelationship.forbidden_project_ids + cond = "(db_projects.id not in (#{fprjs.join(',')}))" if options[:conditions].nil? - options[:conditions] = cond + options[:conditions] = cond if cond else if options[:conditions].class == String options[:conditions] = options[:conditions] # TDWTF ?!? - elsif options[:conditions].class == Hash + elsif options[:conditions].class == Hash and cond wacky = '' options[:conditions].keys.each {|k| wacky += ' AND (' + k.to_s + ')'} options[:conditions] = cond + wacky else - options[:conditions][0] = cond + "AND (" + options[:conditions][0] + ")" + options[:conditions][0] = cond + "AND (" + options[:conditions][0] + ")" if cond end end end diff --git a/src/api/app/models/project_user_role_relationship.rb b/src/api/app/models/project_user_role_relationship.rb index 55a6c40e8e0..7e1d71c76b4 100644 --- a/src/api/app/models/project_user_role_relationship.rb +++ b/src/api/app/models/project_user_role_relationship.rb @@ -12,4 +12,20 @@ def validate_on_create errors.add "User already has this role" end end + + # this is to speed up secure DbProject.find + def self.forbidden_project_ids + hash = Hash.new + ProjectUserRoleRelationship.find_by_sql("SELECT ur.db_project_id, ur.bs_user_id from flags f, + project_user_role_relationships ur where f.flag = 'access' and ur.db_project_id = f.db_project_id").each do |r| + hash[r.db_project_id.to_i] ||= Hash.new + hash[r.db_project_id][r.bs_user_id] = 1 + end + ret = Array.new + userid = User.current ? User.currentID : User.nobodyID + hash.each do |project_id, users| + ret << project_id unless users[userid] + end + ret + end end