Skip to content

Commit

Permalink
[api] instead of relying on ultra complex joins, just find out
Browse files Browse the repository at this point in the history
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
  • Loading branch information
coolo committed Mar 14, 2012
1 parent 9db02d4 commit 55a4702
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 19 deletions.
13 changes: 8 additions & 5 deletions src/api/app/controllers/source_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ def projectlist
output = String.new
output << "<?xml version='1.0' encoding='UTF-8'?>\n"
output << "<directory>\n"
dir.each do |item|
output << " <entry name=\"#{item.to_xs}\"/>\n"
end
output << dir.map { |item| " <entry name=\"#{item.fast_xs}\"/>\n" }.join
output << "</directory>\n"
render :text => output, :content_type => "text/xml"
return
Expand Down Expand Up @@ -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 << "<directory count='#{dir.length}'>\n"
output << dir.map { |item| " <entry name=\"#{item.fast_xs}\"/>\n" }.join
output << "</directory>\n"
render :text => output, :content_type => "text/xml"
end
end
end
Expand Down
9 changes: 4 additions & 5 deletions src/api/app/models/db_package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 5 additions & 9 deletions src/api/app/models/db_project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions src/api/app/models/project_user_role_relationship.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 55a4702

Please sign in to comment.