From 8ae5d94b2f16d6f2cf1739e19ebc3793a18a0a4a Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Tue, 20 Sep 2011 23:22:11 +0100 Subject: [PATCH] Update some more queries to use AREL in place of deprecated methods --- app/controllers/amf_controller.rb | 30 +++--- app/controllers/browse_controller.rb | 20 ++-- app/controllers/changeset_controller.rb | 103 +++++++++----------- app/controllers/oauth_clients_controller.rb | 2 +- app/controllers/search_controller.rb | 74 ++++---------- app/controllers/trace_controller.rb | 6 +- app/models/access_token.rb | 10 +- app/models/client_application.rb | 8 +- app/models/oauth_token.rb | 4 + app/models/old_node.rb | 4 +- app/models/old_relation.rb | 10 +- app/models/old_way.rb | 28 +++--- app/models/relation.rb | 5 +- app/models/trace.rb | 15 +-- app/models/user.rb | 14 ++- app/models/way.rb | 2 +- 16 files changed, 146 insertions(+), 189 deletions(-) diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 96d3602a9..ccc1259f7 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -375,7 +375,7 @@ class AmfController < ApplicationController # Ideally we would do ":include => :nodes" here but if we do that # then rails only seems to return the first copy of a node when a # way includes a node more than once - way = Way.find(:first, :conditions => { :id => wayid }, :include => { :nodes => :node_tags }) + way = Way.where(:id => wayid).preload(:nodes => :node_tags).first # check case where way has been deleted or doesn't exist return [-4, 'way', wayid] if way.nil? or !way.visible @@ -413,13 +413,13 @@ class AmfController < ApplicationController amf_handle_error_with_timeout("'getway_old' #{id}, #{timestamp}", 'way',id) do if timestamp == '' # undelete - old_way = OldWay.find(:first, :conditions => ['visible = ? AND id = ?', true, id], :order => 'version DESC') + old_way = OldWay.where(:visible => true, :id => id).order("version DESC").first points = old_way.get_nodes_undelete unless old_way.nil? else begin # revert timestamp = DateTime.strptime(timestamp.to_s, "%d %b %Y, %H:%M:%S") - old_way = OldWay.find(:first, :conditions => ['id = ? AND timestamp <= ?', id, timestamp], :order => 'timestamp DESC') + old_way = OldWay.where("id = ? AND timestamp <= ?", id, timestamp).order("timestamp DESC").first unless old_way.nil? points = old_way.get_nodes_revert(timestamp) if !old_way.visible @@ -515,16 +515,14 @@ class AmfController < ApplicationController if !user then return -1,"You must be logged in to search for GPX traces." end unless user.active_blocks.empty? then return -1,t('application.setup_user_auth.blocked') end - gpxs = [] - if searchterm.to_i>0 then - gpx = Trace.find(searchterm.to_i, :conditions => ["visible=? AND (public=? OR user_id=?)",true,true,user.id] ) - if gpx then - gpxs.push([gpx.id, gpx.name, gpx.description]) - end + query = Trace.visible_to(user) + if searchterm.to_i > 0 then + query = query.where(:id => searchterm.to_i) else - Trace.find(:all, :limit => 21, :conditions => ["visible=? AND (public=? OR user_id=?) AND MATCH(name) AGAINST (?)",true,true,user.id,searchterm] ).each do |gpx| - gpxs.push([gpx.id, gpx.name, gpx.description]) - end + query = query.where("MATCH(name) AGAINST (?)", searchterm).limit(21) + end + gpxs = query.collect do |gpx| + [gpx.id, gpx.name, gpx.description] end [0,'',gpxs] end @@ -541,7 +539,7 @@ class AmfController < ApplicationController def getrelation(relid) #:doc: amf_handle_error("'getrelation' #{relid}" ,'relation',relid) do - rel = Relation.find(:first, :conditions => { :id => relid }) + rel = Relation.where(:id => relid).first return [-4, 'relation', relid] if rel.nil? or !rel.visible [0, '', relid, rel.tags, rel.members, rel.version] @@ -554,12 +552,12 @@ class AmfController < ApplicationController def findrelations(searchterm) rels = [] if searchterm.to_i>0 then - rel = Relation.find(searchterm.to_i) + rel = Relation.where(:id => searchterm.to_i).first if rel and rel.visible then rels.push([rel.id, rel.tags, rel.members, rel.version]) end else - RelationTag.find(:all, :limit => 11, :conditions => ["v like ?", "%#{searchterm}%"] ).each do |t| + RelationTag.where("v like ?", "%#{searchterm}%").limit(11).each do |t| if t.relation.visible then rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version]) end @@ -838,7 +836,7 @@ class AmfController < ApplicationController n = Node.find(id) v = n.version unless timestamp == '' - n = OldNode.find(:first, :conditions => ['id = ? AND timestamp <= ?', id, timestamp], :order => 'timestamp DESC') + n = OldNode.where("id = ? AND timestamp <= ?", id, timestamp).order("timestamp DESC").first end if n diff --git a/app/controllers/browse_controller.rb b/app/controllers/browse_controller.rb index a7dd5f5c9..2a7e2c5f7 100644 --- a/app/controllers/browse_controller.rb +++ b/app/controllers/browse_controller.rb @@ -12,8 +12,8 @@ class BrowseController < ApplicationController def relation @type = "relation" @relation = Relation.find(params[:id]) - @next = Relation.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @relation.id }] ) - @prev = Relation.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @relation.id }] ) + @next = Relation.visible.where("id > ?", @relation.id).order("id ASC").first + @prev = Relation.visible.where("id < ?", @relation.id).order("id DESC").first rescue ActiveRecord::RecordNotFound render :action => "not_found", :status => :not_found end @@ -28,8 +28,8 @@ class BrowseController < ApplicationController def way @type = "way" @way = Way.find(params[:id], :include => [:way_tags, {:changeset => :user}, {:nodes => [:node_tags, {:ways => :way_tags}]}, :containing_relation_members]) - @next = Way.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @way.id }] ) - @prev = Way.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @way.id }] ) + @next = Way.visible.where("id > ?", @way.id).order("id ASC").first + @prev = Way.visible.where("id < ?", @way.id).order("id DESC").first # Used for edit link, takes approx middle node of way @midnode = @way.nodes[@way.nodes.length/2] @@ -47,8 +47,8 @@ class BrowseController < ApplicationController def node @type = "node" @node = Node.find(params[:id]) - @next = Node.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @node.id }] ) - @prev = Node.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @node.id }] ) + @next = Node.visible.where("id > ?", @node.id).order("id ASC").first + @prev = Node.visible.where("id < ?", @node.id).order("id DESC").first rescue ActiveRecord::RecordNotFound render :action => "not_found", :status => :not_found end @@ -69,12 +69,12 @@ class BrowseController < ApplicationController @relation_pages, @relations = paginate(:old_relations, :conditions => {:changeset_id => @changeset.id}, :per_page => 20, :parameter => 'relation_page') @title = "#{I18n.t('browse.changeset.title')} | #{@changeset.id}" - @next = Changeset.find(:first, :order => "id ASC", :conditions => [ "id > :id", { :id => @changeset.id }] ) - @prev = Changeset.find(:first, :order => "id DESC", :conditions => [ "id < :id", { :id => @changeset.id }] ) + @next = Changeset.where("id > ?", @changeset.id).order("id ASC").first + @prev = Changeset.where("id < ?", @changeset.id).order("id DESC").first if @changeset.user.data_public? - @next_by_user = Changeset.find(:first, :order => "id ASC", :conditions => [ "id > :id AND user_id = :user_id", { :id => @changeset.id, :user_id => @changeset.user_id }] ) - @prev_by_user = Changeset.find(:first, :order => "id DESC", :conditions => [ "id < :id AND user_id = :user_id", { :id => @changeset.id, :user_id => @changeset.user_id }] ) + @next_by_user = Changeset.where("user_id = ? AND id > ?", @changeset.user_id, @changeset.id).order("id ASC").first + @prev_by_user = Changeset.where("user_id = ? AND id < ?", @changeset.user_id, @changeset.id).order("id DESC").first end rescue ActiveRecord::RecordNotFound render :action => "not_found", :status => :not_found diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index b580ea786..51d0049a7 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -200,20 +200,18 @@ class ChangesetController < ApplicationController def query # create the conditions that the user asked for. some or all of # these may be nil. - conditions = conditions_bbox(params['bbox']) - conditions = cond_merge conditions, conditions_user(params['user'], params['display_name']) - conditions = cond_merge conditions, conditions_time(params['time']) - conditions = cond_merge conditions, conditions_open(params['open']) - conditions = cond_merge conditions, conditions_closed(params['closed']) + changesets = Changeset.scoped + changesets = conditions_bbox(changesets, params['bbox']) + changesets = conditions_user(changesets, params['user'], params['display_name']) + changesets = conditions_time(changesets, params['time']) + changesets = conditions_open(changesets, params['open']) + changesets = conditions_closed(changesets, params['closed']) # create the results document results = OSM::API.new.get_xml_doc # add all matching changesets to the XML results document - Changeset.find(:all, - :conditions => conditions, - :limit => 100, - :order => 'created_at desc').each do |cs| + changesets.order("created_at DESC").limit(100).each do |cs| results.root << cs.to_xml_node end @@ -253,16 +251,16 @@ class ChangesetController < ApplicationController if request.format == :atom and params[:page] redirect_to params.merge({ :page => nil }), :status => :moved_permanently else - conditions = conditions_nonempty + changesets = conditions_nonempty(Changeset.scoped) if params[:display_name] - user = User.find_by_display_name(params[:display_name], :conditions => { :status => ["active", "confirmed"] }) + user = User.find_by_display_name(params[:display_name]) - if user + if user and user.active? if user.data_public? or user == @user - conditions = cond_merge conditions, ['user_id = ?', user.id] + changesets = changesets.where(:user_id => user.id) else - conditions = cond_merge conditions, ['false'] + changesets = changesets.where("false") end elsif request.format == :html @title = t 'user.no_such_user.title' @@ -278,7 +276,7 @@ class ChangesetController < ApplicationController end if bbox - conditions = cond_merge conditions, conditions_bbox(bbox) + changesets = conditions_bbox(changesets, bbox) bbox = BoundingBox.from_s(bbox) bbox_link = render_to_string :partial => "bbox", :object => bbox end @@ -310,12 +308,7 @@ class ChangesetController < ApplicationController @bbox = bbox - @edits = Changeset.find(:all, - :include => [:user, :changeset_tags], - :conditions => conditions, - :order => "changesets.created_at DESC", - :offset => (@page - 1) * @page_size, - :limit => @page_size) + @edits = changesets.order("changesets.created_at DESC").offset((@page - 1) * @page_size).limit(@page_size).preload(:user, :changeset_tags) end end @@ -324,43 +317,29 @@ private # utility functions below. #------------------------------------------------------------ - ## - # merge two conditions - def cond_merge(a, b) - if a and b - a_str = a.shift - b_str = b.shift - return [ a_str + " AND " + b_str ] + a + b - elsif a - return a - else b - return b - end - end - ## # if a bounding box was specified then parse it and do some sanity # checks. this is mostly the same as the map call, but without the # area restriction. - def conditions_bbox(bbox) + def conditions_bbox(changesets, bbox) unless bbox.nil? raise OSM::APIBadUserInput.new("Bounding box should be min_lon,min_lat,max_lon,max_lat") unless bbox.count(',') == 3 bbox = sanitise_boundaries(bbox.split(/,/)) raise OSM::APIBadUserInput.new("Minimum longitude should be less than maximum.") unless bbox[0] <= bbox[2] raise OSM::APIBadUserInput.new("Minimum latitude should be less than maximum.") unless bbox[1] <= bbox[3] - return ['min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?', - (bbox[2] * GeoRecord::SCALE).to_i, - (bbox[0] * GeoRecord::SCALE).to_i, - (bbox[3] * GeoRecord::SCALE).to_i, - (bbox[1] * GeoRecord::SCALE).to_i] + return changesets.where("min_lon < ? and max_lon > ? and min_lat < ? and max_lat > ?", + (bbox[2] * GeoRecord::SCALE).to_i, + (bbox[0] * GeoRecord::SCALE).to_i, + (bbox[3] * GeoRecord::SCALE).to_i, + (bbox[1] * GeoRecord::SCALE).to_i) else - return nil + return changesets end end ## # restrict changesets to those by a particular user - def conditions_user(user, name) + def conditions_user(changesets, user, name) unless user.nil? and name.nil? # shouldn't provide both name and UID raise OSM::APIBadUserInput.new("provide either the user ID or display name, but not both") if user and name @@ -386,15 +365,15 @@ private raise OSM::APINotFoundError if @user.nil? or @user.id != u.id end - return ['user_id = ?', u.id] + return changesets.where(:user_id => u.id) else - return nil + return changesets end end ## # restrict changes to those closed during a particular time period - def conditions_time(time) + def conditions_time(changesets, time) unless time.nil? # if there is a range, i.e: comma separated, then the first is # low, second is high - same as with bounding boxes. @@ -404,13 +383,13 @@ private raise OSM::APIBadUserInput.new("bad time range") if times.size != 2 from, to = times.collect { |t| DateTime.parse(t) } - return ['closed_at >= ? and created_at <= ?', from, to] + return changesets.where("closed_at >= ? and created_at <= ?", from, to) else # if there is no comma, assume its a lower limit on time - return ['closed_at >= ?', DateTime.parse(time)] + return changesets.where("closed_at >= ?", DateTime.parse(time)) end else - return nil + return changesets end # stupid DateTime seems to throw both of these for bad parsing, so # we have to catch both and ensure the correct code path is taken. @@ -424,25 +403,33 @@ private # return changesets which are open (haven't been closed yet) # we do this by seeing if the 'closed at' time is in the future. Also if we've # hit the maximum number of changes then it counts as no longer open. - # if parameter 'open' is nill then open and closed changsets are returned - def conditions_open(open) - return open.nil? ? nil : ['closed_at >= ? and num_changes <= ?', - Time.now.getutc, Changeset::MAX_ELEMENTS] + # if parameter 'open' is nill then open and closed changesets are returned + def conditions_open(changesets, open) + if open.nil? + return changesets + else + return changesets.where("closed_at >= ? and num_changes <= ?", + Time.now.getutc, Changeset::MAX_ELEMENTS) + end end ## # query changesets which are closed # ('closed at' time has passed or changes limit is hit) - def conditions_closed(closed) - return closed.nil? ? nil : ['(closed_at < ? or num_changes > ?)', - Time.now.getutc, Changeset::MAX_ELEMENTS] + def conditions_closed(changesets, closed) + if closed.nil? + return changesets + else + return changesets.where("closed_at < ? or num_changes > ?", + Time.now.getutc, Changeset::MAX_ELEMENTS) + end end ## # eliminate empty changesets (where the bbox has not been set) # this should be applied to all changeset list displays - def conditions_nonempty() - return ['num_changes > 0'] + def conditions_nonempty(changesets) + return changesets.where("num_changes > 0") end end diff --git a/app/controllers/oauth_clients_controller.rb b/app/controllers/oauth_clients_controller.rb index f90302894..56f19dbda 100644 --- a/app/controllers/oauth_clients_controller.rb +++ b/app/controllers/oauth_clients_controller.rb @@ -7,7 +7,7 @@ class OauthClientsController < ApplicationController def index @client_applications = @user.client_applications - @tokens = @user.oauth_tokens.find :all, :conditions => 'oauth_tokens.invalidated_at is null and oauth_tokens.authorized_at is not null' + @tokens = @user.oauth_tokens.authorized end def new diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 520aa5a6f..210e0811b 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -41,69 +41,28 @@ class SearchController < ApplicationController return false end - way_ids = Array.new - ways = Array.new - nodes = Array.new - relations = Array.new - # Matching for node tags table - cond_node = Array.new - sql = '1=1' - if type - sql += ' AND current_node_tags.k=?' - cond_node += [type] - end - if value - sql += ' AND current_node_tags.v=?' - cond_node += [value] + if do_nodes + nodes = Node.joins(:node_tags) + nodes = nodes.where(:current_node_tags => { :k => type }) if type + nodes = nodes.where(:current_node_tags => { :v => value }) if value + nodes = nodes.limit(100) end - cond_node = [sql] + cond_node # Matching for way tags table - cond_way = Array.new - sql = '1=1' - if type - sql += ' AND current_way_tags.k=?' - cond_way += [type] - end - if value - sql += ' AND current_way_tags.v=?' - cond_way += [value] + if do_ways + ways = Way.joins(:way_tags) + ways = ways.where(:current_way_tags => { :k => type }) if type + ways = ways.where(:current_way_tags => { :v => value }) if value + ways = ways.limit(100) end - cond_way = [sql] + cond_way # Matching for relation tags table - cond_rel = Array.new - sql = '1=1' - if type - sql += ' AND current_relation_tags.k=?' - cond_rel += [type] - end - if value - sql += ' AND current_relation_tags.v=?' - cond_rel += [value] - end - cond_rel = [sql] + cond_rel - - # First up, look for the relations we want if do_relations - relations = Relation.find(:all, - :joins => "INNER JOIN current_relation_tags ON current_relation_tags.id = current_relations.id", - :conditions => cond_rel, :limit => 100) - end - - # then ways - if do_ways - ways = Way.find(:all, - :joins => "INNER JOIN current_way_tags ON current_way_tags.id = current_ways.id", - :conditions => cond_way, :limit => 100) - end - - # Now, nodes - if do_nodes - nodes = Node.find(:all, - :joins => "INNER JOIN current_node_tags ON current_node_tags.id = current_nodes.id", - :conditions => cond_node, :limit => 2000) + relations = Relation.joins(:relation_tags) + relations = relations.where(:current_relation_tags => { :k => type }) if type + relations = relations.where(:current_relation_tags => { :v => value }) if value + relations = relations.limit(2000) end # Fetch any node needed for our ways (only have matching nodes so far) @@ -121,11 +80,12 @@ class SearchController < ApplicationController ways.each do |way| doc.root << way.to_xml_node(visible_nodes, changeset_cache, user_display_name_cache) - end + end relations.each do |rel| doc.root << rel.to_xml_node(nil, changeset_cache, user_display_name_cache) - end + end + render :text => doc.to_s, :content_type => "text/xml" end end diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb index f381b3243..944335a8f 100644 --- a/app/controllers/trace_controller.rb +++ b/app/controllers/trace_controller.rb @@ -272,7 +272,7 @@ class TraceController < ApplicationController end def api_read - trace = Trace.find(params[:id], :conditions => { :visible => true }) + trace = Trace.visible.find(params[:id]) if trace.public? or trace.user == @user render :text => trace.to_xml.to_s, :content_type => "text/xml" @@ -282,7 +282,7 @@ class TraceController < ApplicationController end def api_update - trace = Trace.find(params[:id], :conditions => { :visible => true }) + trace = Trace.visible.find(params[:id]) if trace.user == @user new_trace = Trace.from_xml(request.raw_post) @@ -303,7 +303,7 @@ class TraceController < ApplicationController end def api_delete - trace = Trace.find(params[:id], :conditions => { :visible => true }) + trace = Trace.visible.find(params[:id]) if trace.user == @user trace.visible = false diff --git a/app/models/access_token.rb b/app/models/access_token.rb index b773310ce..1f172c5f0 100644 --- a/app/models/access_token.rb +++ b/app/models/access_token.rb @@ -1,5 +1,11 @@ -class AccessToken nil) + validates_presence_of :user + before_create :set_authorized_at protected @@ -7,4 +13,4 @@ protected def set_authorized_at self.authorized_at = Time.now end -end \ No newline at end of file +end diff --git a/app/models/client_application.rb b/app/models/client_application.rb index c69502a0b..04f1c0c99 100644 --- a/app/models/client_application.rb +++ b/app/models/client_application.rb @@ -1,16 +1,18 @@ require 'oauth' + class ClientApplication < ActiveRecord::Base belongs_to :user has_many :tokens, :class_name => "OauthToken" has_many :access_tokens + validates_presence_of :name, :url, :key, :secret validates_uniqueness_of :key - before_validation :generate_keys, :on => :create - validates_format_of :url, :with => /\Ahttp(s?):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i validates_format_of :support_url, :with => /\Ahttp(s?):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i, :allow_blank=>true validates_format_of :callback_url, :with => /\A[a-z][a-z0-9.+-]*:\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i, :allow_blank=>true + before_validation :generate_keys, :on => :create + attr_accessor :token_callback_url def self.find_token(token_key) @@ -55,7 +57,7 @@ class ClientApplication < ActiveRecord::Base end def access_token_for_user(user) - unless token = access_tokens.find(:first, :conditions => { :user_id => user.id, :invalidated_at => nil }) + unless token = access_tokens.valid.where(:user_id => user).first params = { :user => user } permissions.each do |p| diff --git a/app/models/oauth_token.rb b/app/models/oauth_token.rb index c45a3d569..376ad7644 100644 --- a/app/models/oauth_token.rb +++ b/app/models/oauth_token.rb @@ -1,8 +1,12 @@ class OauthToken < ActiveRecord::Base belongs_to :client_application belongs_to :user + + scope :authorized, where("authorized_at IS NOT NULL and invalidated_at IS NULL") + validates_uniqueness_of :token validates_presence_of :client_application, :token, :secret + before_validation :generate_keys, :on => :create def self.find_token(token_key) diff --git a/app/models/old_node.rb b/app/models/old_node.rb index 397e83618..ca179e0e9 100644 --- a/app/models/old_node.rb +++ b/app/models/old_node.rb @@ -69,7 +69,7 @@ class OldNode < ActiveRecord::Base clear_aggregation_cache clear_association_cache #ok from here - @attributes.update(OldNode.find(:first, :conditions => ['id = ? AND timestamp = ? AND version = ?', self.id, self.timestamp, self.version]).instance_variable_get('@attributes')) + @attributes.update(OldNode.where('id = ? AND timestamp = ? AND version = ?', self.id, self.timestamp, self.version).first.instance_variable_get('@attributes')) self.tags.each do |k,v| tag = OldNodeTag.new @@ -84,7 +84,7 @@ class OldNode < ActiveRecord::Base def tags unless @tags @tags = Hash.new - OldNodeTag.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version]).each do |tag| + OldNodeTag.where("id = ? AND version = ?", self.id, self.version).each do |tag| @tags[tag.k] = tag.v end end diff --git a/app/models/old_relation.rb b/app/models/old_relation.rb index ca43b5912..c83453e25 100644 --- a/app/models/old_relation.rb +++ b/app/models/old_relation.rb @@ -25,7 +25,7 @@ class OldRelation < ActiveRecord::Base save! clear_aggregation_cache clear_association_cache - @attributes.update(OldRelation.find(:first, :conditions => ['id = ? AND timestamp = ?', self.id, self.timestamp], :order => "version desc").instance_variable_get('@attributes')) + @attributes.update(OldRelation.where('id = ? AND timestamp = ?', self.id, self.timestamp).order("version DESC").first.instance_variable_get('@attributes')) # ok, you can touch from here on @@ -51,7 +51,7 @@ class OldRelation < ActiveRecord::Base def members unless @members @members = Array.new - OldRelationMember.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version], :order => "sequence_id").each do |m| + OldRelationMember.where("id = ? AND version = ?", self.id, self.version).order(:sequence_id).each do |m| @members += [[m.type,m.id,m.role]] end end @@ -61,7 +61,7 @@ class OldRelation < ActiveRecord::Base def tags unless @tags @tags = Hash.new - OldRelationTag.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version]).each do |tag| + OldRelationTag.where("id = ? AND version = ?", self.id, self.version).each do |tag| @tags[tag.k] = tag.v end end @@ -81,11 +81,11 @@ class OldRelation < ActiveRecord::Base # has_many :relation_tags, :class_name => 'OldRelationTag', :foreign_key => 'id' def old_members - OldRelationMember.find(:all, :conditions => ['id = ? AND version = ?', self.id, self.version], :order => "sequence_id") + OldRelationMember.where('id = ? AND version = ?', self.id, self.version).order(:sequence_id) end def old_tags - OldRelationTag.find(:all, :conditions => ['id = ? AND version = ?', self.id, self.version]) + OldRelationTag.where('id = ? AND version = ?', self.id, self.version) end def to_xml diff --git a/app/models/old_way.rb b/app/models/old_way.rb index dc5715693..85cc64447 100644 --- a/app/models/old_way.rb +++ b/app/models/old_way.rb @@ -30,7 +30,7 @@ class OldWay < ActiveRecord::Base save! clear_aggregation_cache clear_association_cache - @attributes.update(OldWay.find(:first, :conditions => ['id = ? AND timestamp = ?', self.id, self.timestamp], :order => "version desc").instance_variable_get('@attributes')) + @attributes.update(OldWay.where('id = ? AND timestamp = ?', self.id, self.timestamp).order("version DESC").first.instance_variable_get('@attributes')) # ok, you can touch from here on @@ -56,7 +56,7 @@ class OldWay < ActiveRecord::Base def nds unless @nds @nds = Array.new - OldWayNode.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version], :order => "sequence_id").each do |nd| + OldWayNode.where("id = ? AND version = ?", self.id, self.version).order(:sequence_id).each do |nd| @nds += [nd.node_id] end end @@ -66,7 +66,7 @@ class OldWay < ActiveRecord::Base def tags unless @tags @tags = Hash.new - OldWayTag.find(:all, :conditions => ["id = ? AND version = ?", self.id, self.version]).each do |tag| + OldWayTag.where("id = ? AND version = ?", self.id, self.version).each do |tag| @tags[tag.k] = tag.v end end @@ -86,11 +86,11 @@ class OldWay < ActiveRecord::Base # has_many :way_tags, :class_name => 'OldWayTag', :foreign_key => 'id' def old_nodes - OldWayNode.find(:all, :conditions => ['id = ? AND version = ?', self.id, self.version]) + OldWayNode.where('id = ? AND version = ?', self.id, self.version) end def old_tags - OldWayTag.find(:all, :conditions => ['id = ? AND version = ?', self.id, self.version]) + OldWayTag.where('id = ? AND version = ?', self.id, self.version) end def to_xml_node @@ -128,21 +128,21 @@ class OldWay < ActiveRecord::Base # (i.e. is it visible? are we actually reverting to an earlier version?) def get_nodes_undelete - points = [] - self.nds.each do |n| - node=Node.find(n) - points << [node.lon, node.lat, n, node.version, node.tags_as_hash, node.visible] + points = [] + self.nds.each do |n| + node = Node.find(n) + points << [node.lon, node.lat, n, node.version, node.tags_as_hash, node.visible] end - points + points end def get_nodes_revert(timestamp) points=[] self.nds.each do |n| - oldnode=OldNode.find(:first, :conditions=>['id=? AND timestamp<=?',n,timestamp], :order=>"timestamp DESC") - curnode=Node.find(n) - id=n; reuse=curnode.visible - if oldnode.lat!=curnode.lat or oldnode.lon!=curnode.lon or oldnode.tags!=curnode.tags then + oldnode = OldNode.where('id = ? AND timestamp <= ?', n, timestamp).order("timestamp DESC").first + curnode = Node.find(n) + id = n; reuse = curnode.visible + if oldnode.lat != curnode.lat or oldnode.lon != curnode.lon or oldnode.tags != curnode.tags then # node has changed: if it's in other ways, give it a new id if curnode.ways-[self.id] then id=-1; reuse=false end end diff --git a/app/models/relation.rb b/app/models/relation.rb index dd4d49258..d5a6d5dc0 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -220,8 +220,7 @@ class Relation < ActiveRecord::Base self.lock! check_consistency(self, new_relation, user) # This will check to see if this relation is used by another relation - rel = RelationMember.find(:first, :joins => :relation, - :conditions => [ "visible = ? AND member_type='Relation' and member_id=? ", true, self.id ]) + rel = RelationMember.joins(:relation).where("visible = ? AND member_type = 'Relation' and member_id = ? ", true, self.id).first raise OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is used in relation #{rel.relation.id}.") unless rel.nil? self.changeset_id = new_relation.changeset_id @@ -279,7 +278,7 @@ class Relation < ActiveRecord::Base # use reflection to look up the appropriate class model = Kernel.const_get(m[0].capitalize) # get the element with that ID - element = model.find(:first, :conditions =>["id = ?", m[1]]) + element = model.where(:id => m[1]).first # and check that it is OK to use. unless element and element.visible? and element.preconditions_ok? diff --git a/app/models/trace.rb b/app/models/trace.rb index 582c7285c..45abdf474 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -1,6 +1,13 @@ class Trace < ActiveRecord::Base set_table_name 'gpx_files' + belongs_to :user + has_many :tags, :class_name => 'Tracetag', :foreign_key => 'gpx_id', :dependent => :delete_all + has_many :points, :class_name => 'Tracepoint', :foreign_key => 'gpx_id', :dependent => :delete_all + + scope :visible, where(:visible => true) + scope :visible_to, lambda { |u| visible.where("visibility IN ('public', 'identifiable') OR user_id = ?", u) } + validates_presence_of :user_id, :name, :timestamp validates_presence_of :description, :on => :create validates_length_of :name, :maximum => 255 @@ -9,10 +16,6 @@ class Trace < ActiveRecord::Base validates_inclusion_of :inserted, :in => [ true, false ] validates_inclusion_of :visibility, :in => ["private", "public", "trackable", "identifiable"] - belongs_to :user - has_many :tags, :class_name => 'Tracetag', :foreign_key => 'gpx_id', :dependent => :delete_all - has_many :points, :class_name => 'Tracepoint', :foreign_key => 'gpx_id', :dependent => :delete_all - def destroy super FileUtils.rm_f(trace_name) @@ -262,8 +265,8 @@ class Trace < ActiveRecord::Base # If there are any existing points for this trace then delete # them - we check for existing points first to avoid locking # the table in the common case where there aren't any. - if Tracepoint.find(:first, :conditions => ['gpx_id = ?', self.id]) - Tracepoint.delete_all(['gpx_id = ?', self.id]) + if Tracepoint.where(:gpx_id => self.id).exists? + Tracepoint.delete_all(:gpx_id => self.id) end gpx.points do |point| diff --git a/app/models/user.rb b/app/models/user.rb index b52bcefef..759c8d0b0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -43,10 +43,10 @@ class User < ActiveRecord::Base def self.authenticate(options) if options[:username] and options[:password] - user = find(:first, :conditions => ["email = ? OR display_name = ?", options[:username], options[:username]]) + user = where("email = ? OR display_name = ?", options[:username], options[:username]).first user = nil if user and user.pass_crypt != OSM::encrypt_password(options[:password], user.pass_salt) elsif options[:token] - token = UserToken.find(:first, :include => :user, :conditions => ["user_tokens.token = ?", options[:token]]) + token = UserToken.where(:token => options[:token]).preload(:user).first user = token.user if token end @@ -91,7 +91,7 @@ class User < ActiveRecord::Base end def preferred_language - languages.find { |l| Language.find(:first, :conditions => { :code => l }) } + languages.find { |l| Language.exists?(:code => l) } end def preferred_language_from(array) @@ -103,9 +103,7 @@ class User < ActiveRecord::Base gc = OSM::GreatCircle.new(self.home_lat, self.home_lon) bounds = gc.bounds(radius) sql_for_distance = gc.sql_for_distance("home_lat", "home_lon") - nearby = User.find(:all, - :conditions => ["id != ? AND status IN (\'active\', \'confirmed\') AND data_public = ? AND #{sql_for_distance} <= ?", id, true, radius], - :order => sql_for_distance, :limit => num) + nearby = User.where("id != ? AND status IN (\'active\', \'confirmed\') AND data_public = ? AND #{sql_for_distance} <= ?", id, true, radius).order(sql_for_distance).limit(num) else nearby = [] end @@ -181,8 +179,8 @@ class User < ActiveRecord::Base ## # return a spam score for a user def spam_score - changeset_score = self.changesets.find(:all, :limit => 10).length * 50 - trace_score = self.traces.find(:all, :limit => 10).length * 50 + changeset_score = self.changesets.limit(10).length * 50 + trace_score = self.traces.limit(10).length * 50 diary_entry_score = self.diary_entries.inject(0) { |s,e| s += OSM.spam_score(e.body) } diary_comment_score = self.diary_comments.inject(0) { |s,e| s += OSM.spam_score(e.body) } diff --git a/app/models/way.rb b/app/models/way.rb index 23475785c..74fdfed34 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -246,7 +246,7 @@ class Way < ActiveRecord::Base new_nds = (self.nds - old_nodes).sort.uniq unless new_nds.empty? - db_nds = Node.find(:all, :conditions => { :id => new_nds, :visible => true }) + db_nds = Node.where(:id => new_nds, :visible => true) if db_nds.length < new_nds.length missing = new_nds - db_nds.collect { |n| n.id } -- 2.43.2