Skip to content

Commit

Permalink
[api] optimize the usage of activerecord to avoid tons of useless SQL…
Browse files Browse the repository at this point in the history
… queries
  • Loading branch information
coolo committed Mar 14, 2012
1 parent 051cb11 commit 8814702
Show file tree
Hide file tree
Showing 11 changed files with 91 additions and 53 deletions.
6 changes: 3 additions & 3 deletions src/api/app/controllers/attribute_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def index
valid_http_methods :get

if params[:namespace]
if not AttribNamespace.find_by_name( params[:namespace] )
if not AttribNamespace.find_by_name( params[:namespace], :select => "id,name" )
render_error :status => 400, :errorcode => 'unknown_namespace',
:message => "Attribute namespace does not exist: #{params[:namespace]}"
return
Expand Down Expand Up @@ -46,7 +46,7 @@ def namespace_definition
namespace = params[:namespace]

if request.get?
an = AttribNamespace.find_by_name( namespace )
an = AttribNamespace.find_by_name( namespace, :select => "id,name" )
if an
render :text => an.render_axml, :content_type => 'text/xml'
else
Expand Down Expand Up @@ -121,7 +121,7 @@ def attribute_definition
end

if request.get?
at = AttribType.find( :first, :joins => ans, :conditions=>{:name=>name} )
at = ans.attrib_types.find( :first, :conditions=>{:name=>name} )
if at
render :text => at.render_axml, :content_type => 'text/xml'
else
Expand Down
6 changes: 3 additions & 3 deletions src/api/app/controllers/request_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ def create_create
unless e and DbPackage.exists_by_project_and_name( tprj, tpkg, follow_project_links=true, allow_remote_packages=false)
if action.value("type") == "maintenance_release"
newPackages << pkg
pkg.db_project.repositories.each do |repo|
pkg.db_project.repositories.find(:all, :include => [:release_targets]).each do |repo|
repo.release_targets.each do |rt|
newTargets << rt.target_repository.db_project.name
end
Expand All @@ -459,7 +459,7 @@ def create_create
# is this package source going to a project which is specified as release target ?
if action.value("type") == "maintenance_release"
found = nil
pkg.db_project.repositories.each do |repo|
pkg.db_project.repositories.find(:all, :include => [:release_targets]).each do |repo|
repo.release_targets.each do |rt|
if rt.target_repository.db_project.name == tprj
found = 1
Expand Down Expand Up @@ -656,7 +656,7 @@ def create_create
if action.value("type") == "maintenance_release"
# get sure that the releasetarget definition exists or we release without binaries
prj = DbProject.get_by_name(action.source.project)
prj.repositories.each do |repo|
prj.repositories.find(:all, :include => [:release_targets]).each do |repo|
unless repo.release_targets.size > 0
render_error :status => 400, :errorcode => "repository_without_releasetarget",
:message => "Release target definition is missing in #{prj.name} / #{repo.name}"
Expand Down
15 changes: 7 additions & 8 deletions src/api/app/controllers/source_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ def projectlist
output << dir.map { |item| " <entry name=\"#{item.fast_xs}\"/>\n" }.join
output << "</directory>\n"
render :text => output, :content_type => "text/xml"
return
end

# /source/:project
Expand Down Expand Up @@ -95,21 +94,21 @@ def index_project
end
pass_to_backend
else
if DbProject.is_remote_project? project_name
# for access check
pro = DbProject.get_by_name(project_name, :select => "id,name")
if (!pro || !pro.kind_of?(DbProject)) && DbProject.is_remote_project?(project_name)
pass_to_backend
else
# for access check
pro = DbProject.get_by_name project_name
else pro
raise DbProject::UnknownObjectError(project_name) unless pro
# we let the backend list the packages after we verified the project is visible
if params.has_key? :view
if params["view"] == "issues"
render :text => pro.render_issues_axml(params), :content_type => 'text/xml'
return
end
pass_to_backend
else
prj = DbProject.find_by_name project_name
dir = prj.db_packages.map { |i| i.name }.sort
elsif pro.kind_of? DbProject
dir = pro.db_packages.find(:all, :select => "db_packages.name").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
Expand Down
7 changes: 4 additions & 3 deletions src/api/app/models/attrib_namespace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class AttribNamespace < ActiveRecord::Base

class << self
def list_all
find :all
find :all, :select => "id,name"
end
end

Expand All @@ -33,9 +33,10 @@ def update_from_xml(node)
end

def render_axml(node = Builder::XmlMarkup.new(:indent=>2))
if attrib_namespace_modifiable_bies.length > 0
abies = attrib_namespace_modifiable_bies.find(:all, :include => [:user, :group])
if abies.length > 0
node.namespace(:name => self.name) do |an|
attrib_namespace_modifiable_bies.each do |mod_rule|
abies.each do |mod_rule|
p={}
p[:user] = mod_rule.user.login if mod_rule.user
p[:group] = mod_rule.group.title if mod_rule.group
Expand Down
9 changes: 5 additions & 4 deletions src/api/app/models/attrib_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ class AttribType < ActiveRecord::Base
class << self
def list_all(namespace=nil)
if namespace
find :all, :joins => "JOIN attrib_namespaces an ON attrib_types.attrib_namespace_id = an.id", :conditions => ["an.name = BINARY ?", namespace]
find :all, :joins => "JOIN attrib_namespaces an ON attrib_types.attrib_namespace_id = an.id", :conditions => ["an.name = BINARY ?", namespace], :select => "attrib_types.id,attrib_types.name"
else
find :all
find :all, :select => "id,name"
end
end

Expand Down Expand Up @@ -69,8 +69,9 @@ def render_axml
attr.count self.value_count
end

if attrib_type_modifiable_bies.length > 0
attrib_type_modifiable_bies.each do |mod_rule|
abies = attrib_type_modifiable_bies.find(:all, :include => [:user, :group, :role])
if abies.length > 0
abies.each do |mod_rule|
p={}
p[:user] = mod_rule.user.login if mod_rule.user
p[:group] = mod_rule.group.title if mod_rule.group
Expand Down
31 changes: 12 additions & 19 deletions src/api/app/models/db_project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,17 +118,8 @@ def check_access?(dbp=self)
return false if dbp.nil?
# check for 'access' flag

# same cache as in public controller
key = "public_project:" + dbp.name
public_allowed = Rails.cache.fetch(key, :expires_in => 30.minutes) do
flags_set = dbp.flags.count :conditions =>
["db_project_id = ? and flag = 'access' and status = 'disable'", dbp.id]
flags_set == 0
end

# Do we need a per user check ?
return true if public_allowed
return true if User.currentAdmin
return true unless ProjectUserRoleRelationship.forbidden_project_ids.include? dbp.id

# simple check for involvement --> involved users can access
# dbp.id, User.currentID
Expand Down Expand Up @@ -233,8 +224,9 @@ def securedfind_initial(options)
# should be always used when a project is required
# The return value is either a DbProject for local project or an xml
# array for a remote project
def get_by_name(name)
dbp = find :first, :conditions => ["name = BINARY ?", name]
def get_by_name(name, opts = {})
opts[:conditions] = ["name = BINARY ?", name]
dbp = find :first, opts
if dbp.nil?
dbp, remote_name = find_remote_project(name)
return dbp.name + ":" + remote_name if dbp
Expand All @@ -261,8 +253,9 @@ def exists_by_name(name)

# to be obsoleted, this function is not throwing exceptions on problems
# use get_by_name or exists_by_name instead
def find_by_name(name)
dbp = find :first, :conditions => ["name = BINARY ?", name]
def find_by_name(name, opts = {})
opts[:conditions] = ["name = BINARY ?", name]
dbp = find :first, opts
return if dbp.nil?
return unless check_access?(dbp)
return dbp
Expand Down Expand Up @@ -312,9 +305,9 @@ def find_remote_project(name, skip_access=false)
logger.debug "checking local project #{local_project}, remote_project #{remote_project}"
if skip_access
# hmm calling a private class method is not the best idea..
lpro = find_initial :conditions => ["name = BINARY ?", local_project]
lpro = find_initial :conditions => ["name = BINARY ?", local_project], :select => "id,name,remoteurl"
else
lpro = DbProject.find_by_name local_project
lpro = DbProject.find_by_name(local_project, :select => "id,name,remoteurl")
end
return lpro, remote_project unless lpro.nil? or lpro.remoteurl.nil?
end
Expand Down Expand Up @@ -1062,7 +1055,7 @@ def render_axml(view = nil)
end
end

repos = repositories.find( :all, :conditions => "ISNULL(remote_project_name)" )
repos = repositories.find( :all, :conditions => "ISNULL(remote_project_name)", :include => [:release_targets, :path_elements, :architectures] )
repos.each do |repo|
params = {}
params[:name] = repo.name
Expand All @@ -1077,7 +1070,7 @@ def render_axml(view = nil)
params[:trigger] = rt.trigger unless rt.trigger.blank?
r.releasetarget( params )
end
repo.path_elements.each do |pe|
repo.path_elements.find(:all, :include => [:link]).each do |pe|
if pe.link.remote_project_name
project_name = pe.link.db_project.name+":"+pe.link.remote_project_name
else
Expand Down Expand Up @@ -1108,7 +1101,7 @@ def render_axml(view = nil)
end

def to_axml_id
return "<project name='#{name.to_xs}'/>"
return "<project name='#{name.fast_xs}'/>"
end


Expand Down
22 changes: 17 additions & 5 deletions src/api/app/models/project_user_role_relationship.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ class ProjectUserRoleRelationship < ActiveRecord::Base
belongs_to :user, :foreign_key => 'bs_user_id'
belongs_to :role

@@project_user_cache = nil

def validate_on_create
unless self.user
errors.add "Can not assign role to nonexistent user"
Expand All @@ -15,17 +17,27 @@ def validate_on_create

# 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,
unless @@project_user_cache
@@project_user_cache = 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
@@project_user_cache[r.db_project_id.to_i] ||= Hash.new
@@project_user_cache[r.db_project_id][r.bs_user_id] = 1
end
@@project_user_cache
end
ret = Array.new
userid = User.current ? User.currentID : User.nobodyID
hash.each do |project_id, users|
@@project_user_cache.each do |project_id, users|
ret << project_id unless users[userid]
end
ret
end

def save
logger.debug "ProjectUserRoleRelationship saved!"
@@project_user_cache = nil
super
end

end
11 changes: 6 additions & 5 deletions src/api/app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def update_user_info_from_proxy_env(env)
#####################

def is_admin?
roles.include? Role.find_by_title("Admin")
!roles.find_by_title("Admin", :select => "roles.id").nil?
end

def is_in_group?(group)
Expand Down Expand Up @@ -264,9 +264,9 @@ def can_create_attribute_definition?(object)
end

return true if is_admin?
return false if object.attrib_namespace_modifiable_bies.length <= 0

object.attrib_namespace_modifiable_bies.each do |mod_rule|
abies = object.attrib_namespace_modifiable_bies.find(:all, :include => [:user, :group])
abies.each do |mod_rule|
next if mod_rule.user and mod_rule.user != self
next if mod_rule.group and not is_in_group? mod_rule.group
return true
Expand Down Expand Up @@ -294,8 +294,9 @@ def can_create_attribute_in?(object, opts)
return true if is_admin?

# check modifiable_by rules
if atype.attrib_type_modifiable_bies.length > 0
atype.attrib_type_modifiable_bies.each do |mod_rule|
abies = atype.attrib_type_modifiable_bies.find(:all, :include => [:user, :group, :role])
if abies.length > 0
abies.each do |mod_rule|
next if mod_rule.user and mod_rule.user != self
next if mod_rule.group and not is_in_group? mod_rule.group
next if mod_rule.role and not has_local_role?(mod_rule.role, object)
Expand Down
1 change: 1 addition & 0 deletions src/api/config/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
config.gem 'ci_reporter', :lib => false # ci_reporter generates XML reports for Test::Unit
config.gem 'rdoc'
config.gem 'xmlhash'
config.gem 'fast_xs'

# default secret
secret = "ad9712p8349zqmowiefzhiuzgfp9s8f7qp83947p98weap98dfe7"
Expand Down
14 changes: 11 additions & 3 deletions src/api/db/development_structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ CREATE TABLE `attrib_namespace_modifiable_bies` (
UNIQUE KEY `attrib_namespace_user_role_all_index` (`attrib_namespace_id`,`bs_user_id`,`bs_group_id`),
KEY `bs_user_id` (`bs_user_id`),
KEY `bs_group_id` (`bs_group_id`),
KEY `index_attrib_namespace_modifiable_bies_on_attrib_namespace_id` (`attrib_namespace_id`),
CONSTRAINT `attrib_namespace_modifiable_bies_ibfk_1` FOREIGN KEY (`attrib_namespace_id`) REFERENCES `attrib_namespaces` (`id`),
CONSTRAINT `attrib_namespace_modifiable_bies_ibfk_2` FOREIGN KEY (`bs_user_id`) REFERENCES `users` (`id`),
CONSTRAINT `attrib_namespace_modifiable_bies_ibfk_3` FOREIGN KEY (`bs_group_id`) REFERENCES `groups` (`id`)
Expand Down Expand Up @@ -243,7 +244,9 @@ CREATE TABLE `downloads` (
`mtype` varchar(255) DEFAULT NULL,
`architecture_id` int(11) DEFAULT NULL,
`db_project_id` int(11) DEFAULT NULL,
PRIMARY KEY (`id`)
PRIMARY KEY (`id`),
KEY `index_downloads_on_db_project_id` (`db_project_id`),
KEY `index_downloads_on_architecture_id` (`architecture_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

CREATE TABLE `flags` (
Expand Down Expand Up @@ -351,7 +354,9 @@ CREATE TABLE `maintenance_incidents` (
`request` int(11) DEFAULT NULL,
`updateinfo_id` varchar(255) DEFAULT NULL,
`incident_id` int(11) DEFAULT NULL,
PRIMARY KEY (`id`)
PRIMARY KEY (`id`),
KEY `index_maintenance_incidents_on_db_project_id` (`db_project_id`),
KEY `index_maintenance_incidents_on_maintenance_db_project_id` (`maintenance_db_project_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

CREATE TABLE `messages` (
Expand Down Expand Up @@ -448,7 +453,8 @@ CREATE TABLE `release_targets` (
`target_repository_id` int(11) NOT NULL,
`trigger` enum('finished','allsucceeded','maintenance') DEFAULT NULL,
PRIMARY KEY (`id`),
KEY `repository_id_index` (`repository_id`)
KEY `repository_id_index` (`repository_id`),
KEY `index_release_targets_on_target_repository_id` (`target_repository_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

CREATE TABLE `repositories` (
Expand Down Expand Up @@ -853,6 +859,8 @@ INSERT INTO schema_migrations (version) VALUES ('20120220114304');

INSERT INTO schema_migrations (version) VALUES ('20120223105426');

INSERT INTO schema_migrations (version) VALUES ('20120304205014');

INSERT INTO schema_migrations (version) VALUES ('20120313113554');

INSERT INTO schema_migrations (version) VALUES ('20120313131909');
Expand Down
22 changes: 22 additions & 0 deletions src/api/db/migrate/20120304205014_addmoreindexes.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
## This was found by a script
class Addmoreindexes < ActiveRecord::Migration
def self.up

add_index :release_targets, :target_repository_id
add_index :attrib_namespace_modifiable_bies, :attrib_namespace_id
add_index :downloads, :db_project_id
add_index :downloads, :architecture_id
add_index :maintenance_incidents, :db_project_id
add_index :maintenance_incidents, :maintenance_db_project_id
end

def self.down
remove_index :release_targets, :target_repository_id
remove_index :attrib_namespace_modifiable_bies, :attrib_namespace_id
remove_index :downloads, :db_project_id
remove_index :downloads, :architecture_id
remove_index :maintenance_incidents, :db_project_id
remove_index :maintenance_incidents, :maintenance_db_project_id
end

end

0 comments on commit 8814702

Please sign in to comment.