cst1/openstreetmap-website
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix new rubocop warnings

Browse source
This commit is contained in:
Tom Hughes 2016-02-05 12:28:16 +00:00
parent c7061991e7
commit 5d3ecffa28
41 changed files with 252 additions and 250 deletions
Download patch file Download diff file Expand all files Collapse all files

3
.rubocop.yml
View file

@ -3,6 +3,9 @@ inherit_from: .rubocop_todo.yml
Rails: Rails:
Enabled: true Enabled: true
Performance/RedundantMerge:
Enabled: false
Style/BracesAroundHashParameters: Style/BracesAroundHashParameters:
EnforcedStyle: context_dependent EnforcedStyle: context_dependent

4
app/controllers/amf_controller.rb
View file

@ -151,7 +151,7 @@ class AmfController < ApplicationController
cs = Changeset.find(closeid.to_i) cs = Changeset.find(closeid.to_i)
cs.set_closed_time_now cs.set_closed_time_now
if cs.user_id != user.id if cs.user_id != user.id
fail OSM::APIUserChangesetMismatchError.new raise OSM::APIUserChangesetMismatchError.new
elsif closecomment.empty? elsif closecomment.empty?
cs.save! cs.save!
else else
@ -677,7 +677,7 @@ class AmfController < ApplicationController
# -- Save revised way # -- Save revised way
pointlist.collect! do|a| pointlist.collect! do |a|
renumberednodes[a] ? renumberednodes[a] : a renumberednodes[a] ? renumberednodes[a] : a
end # renumber nodes end # renumber nodes
new_way = Way.new new_way = Way.new

9
app/controllers/api_controller.rb
View file

@ -141,7 +141,9 @@ class ApiController < ApplicationController
# get ways # get ways
# find which ways are needed # find which ways are needed
ways = [] ways = []
if node_ids.length > 0 if node_ids.empty?
list_of_way_nodes = []
else
way_nodes = WayNode.where(:node_id => node_ids) way_nodes = WayNode.where(:node_id => node_ids)
way_ids = way_nodes.collect { |way_node| way_node.id[0] } way_ids = way_nodes.collect { |way_node| way_node.id[0] }
ways = Way.preload(:way_nodes, :way_tags).find(way_ids) ways = Way.preload(:way_nodes, :way_tags).find(way_ids)
@ -150,15 +152,12 @@ class ApiController < ApplicationController
way.way_nodes.collect(&:node_id) way.way_nodes.collect(&:node_id)
end end
list_of_way_nodes.flatten! list_of_way_nodes.flatten!
else
list_of_way_nodes = []
end end
# - [0] in case some thing links to node 0 which doesn't exist. Shouldn't actually ever happen but it does. FIXME: file a ticket for this # - [0] in case some thing links to node 0 which doesn't exist. Shouldn't actually ever happen but it does. FIXME: file a ticket for this
nodes_to_fetch = (list_of_way_nodes.uniq - node_ids) - [0] nodes_to_fetch = (list_of_way_nodes.uniq - node_ids) - [0]
if nodes_to_fetch.length > 0 unless nodes_to_fetch.empty?
nodes += Node.includes(:node_tags).find(nodes_to_fetch) nodes += Node.includes(:node_tags).find(nodes_to_fetch)
end end

2
app/controllers/application_controller.rb
View file

@ -337,7 +337,7 @@ class ApplicationController < ActionController::Base
# or raises a suitable error. +method+ should be a symbol, e.g: :put or :get. # or raises a suitable error. +method+ should be a symbol, e.g: :put or :get.
def assert_method(method) def assert_method(method)
ok = request.send((method.to_s.downcase + "?").to_sym) ok = request.send((method.to_s.downcase + "?").to_sym)
fail OSM::APIBadMethodError.new(method) unless ok raise OSM::APIBadMethodError.new(method) unless ok
end end
## ##

36
app/controllers/changeset_controller.rb
View file

@ -308,8 +308,8 @@ class ChangesetController < ApplicationController
# Add a comment to a changeset # Add a comment to a changeset
def comment def comment
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No id was given") unless params[:id] raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
fail OSM::APIBadUserInput.new("No text was given") if params[:text].blank? raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
# Extract the arguments # Extract the arguments
id = params[:id].to_i id = params[:id].to_i
@ -317,7 +317,7 @@ class ChangesetController < ApplicationController
# Find the changeset and check it is valid # Find the changeset and check it is valid
changeset = Changeset.find(id) changeset = Changeset.find(id)
fail OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open? raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
# Add a comment to the changeset # Add a comment to the changeset
comment = changeset.comments.create(:changeset => changeset, comment = changeset.comments.create(:changeset => changeset,
@ -342,15 +342,15 @@ class ChangesetController < ApplicationController
# Adds a subscriber to the changeset # Adds a subscriber to the changeset
def subscribe def subscribe
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No id was given") unless params[:id] raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
# Extract the arguments # Extract the arguments
id = params[:id].to_i id = params[:id].to_i
# Find the changeset and check it is valid # Find the changeset and check it is valid
changeset = Changeset.find(id) changeset = Changeset.find(id)
fail OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open? raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
fail OSM::APIChangesetAlreadySubscribedError.new(changeset) if changeset.subscribers.exists?(@user.id) raise OSM::APIChangesetAlreadySubscribedError.new(changeset) if changeset.subscribers.exists?(@user.id)
# Add the subscriber # Add the subscriber
changeset.subscribers << @user changeset.subscribers << @user
@ -363,15 +363,15 @@ class ChangesetController < ApplicationController
# Removes a subscriber from the changeset # Removes a subscriber from the changeset
def unsubscribe def unsubscribe
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No id was given") unless params[:id] raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
# Extract the arguments # Extract the arguments
id = params[:id].to_i id = params[:id].to_i
# Find the changeset and check it is valid # Find the changeset and check it is valid
changeset = Changeset.find(id) changeset = Changeset.find(id)
fail OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open? raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
fail OSM::APIChangesetNotSubscribedError.new(changeset) unless changeset.subscribers.exists?(@user.id) raise OSM::APIChangesetNotSubscribedError.new(changeset) unless changeset.subscribers.exists?(@user.id)
# Remove the subscriber # Remove the subscriber
changeset.subscribers.delete(@user) changeset.subscribers.delete(@user)
@ -384,7 +384,7 @@ class ChangesetController < ApplicationController
# Sets visible flag on comment to false # Sets visible flag on comment to false
def hide_comment def hide_comment
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No id was given") unless params[:id] raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
# Extract the arguments # Extract the arguments
id = params[:id].to_i id = params[:id].to_i
@ -403,7 +403,7 @@ class ChangesetController < ApplicationController
# Sets visible flag on comment to true # Sets visible flag on comment to true
def unhide_comment def unhide_comment
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No id was given") unless params[:id] raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
# Extract the arguments # Extract the arguments
id = params[:id].to_i id = params[:id].to_i
@ -473,19 +473,19 @@ class ChangesetController < ApplicationController
changesets changesets
else else
# shouldn't provide both name and UID # shouldn't provide both name and UID
fail OSM::APIBadUserInput.new("provide either the user ID or display name, but not both") if user && name raise OSM::APIBadUserInput.new("provide either the user ID or display name, but not both") if user && name
# use either the name or the UID to find the user which we're selecting on. # use either the name or the UID to find the user which we're selecting on.
u = if name.nil? u = if name.nil?
# user input checking, we don't have any UIDs < 1 # user input checking, we don't have any UIDs < 1
fail OSM::APIBadUserInput.new("invalid user ID") if user.to_i < 1 raise OSM::APIBadUserInput.new("invalid user ID") if user.to_i < 1
u = User.find(user.to_i) u = User.find(user.to_i)
else else
u = User.find_by_display_name(name) u = User.find_by_display_name(name)
end end
# make sure we found a user # make sure we found a user
fail OSM::APINotFoundError.new if u.nil? raise OSM::APINotFoundError.new if u.nil?
# should be able to get changesets of public users only, or # should be able to get changesets of public users only, or
# our own changesets regardless of public-ness. # our own changesets regardless of public-ness.
@ -494,7 +494,7 @@ class ChangesetController < ApplicationController
# changesets if they're non-public # changesets if they're non-public
setup_user_auth setup_user_auth
fail OSM::APINotFoundError if @user.nil? || @user.id != u.id raise OSM::APINotFoundError if @user.nil? || @user.id != u.id
end end
changesets.where(:user_id => u.id) changesets.where(:user_id => u.id)
@ -512,7 +512,7 @@ class ChangesetController < ApplicationController
# check that we actually have 2 elements in the array # check that we actually have 2 elements in the array
times = time.split(/,/) times = time.split(/,/)
fail OSM::APIBadUserInput.new("bad time range") if times.size != 2 raise OSM::APIBadUserInput.new("bad time range") if times.size != 2
from, to = times.collect { |t| DateTime.parse(t) } from, to = times.collect { |t| DateTime.parse(t) }
return changesets.where("closed_at >= ? and created_at <= ?", from, to) return changesets.where("closed_at >= ? and created_at <= ?", from, to)
@ -561,7 +561,7 @@ class ChangesetController < ApplicationController
if ids.nil? if ids.nil?
return changesets return changesets
elsif ids.empty? elsif ids.empty?
fail OSM::APIBadUserInput.new("No changesets were given to search for") raise OSM::APIBadUserInput.new("No changesets were given to search for")
else else
ids = ids.split(",").collect(&:to_i) ids = ids.split(",").collect(&:to_i)
return changesets.where(:id => ids) return changesets.where(:id => ids)
@ -582,7 +582,7 @@ class ChangesetController < ApplicationController
if params[:limit].to_i > 0 && params[:limit].to_i <= 10000 if params[:limit].to_i > 0 && params[:limit].to_i <= 10000
params[:limit].to_i params[:limit].to_i
else else
fail OSM::APIBadUserInput.new("Comments limit must be between 1 and 10000") raise OSM::APIBadUserInput.new("Comments limit must be between 1 and 10000")
end end
else else
100 100

26
app/controllers/geocoder_controller.rb
View file

@ -298,7 +298,7 @@ class GeocoderController < ApplicationController
if response.success? if response.success?
response.body response.body
else else
fail response.status.to_s raise response.status.to_s
end end
end end
@ -339,11 +339,11 @@ class GeocoderController < ApplicationController
def nsew_to_decdeg(captures) def nsew_to_decdeg(captures)
begin begin
Float(captures[0]) Float(captures[0])
lat = captures[2].downcase != "s" ? captures[0].to_f : -captures[0].to_f lat = !captures[2].casecmp("s").zero? ? captures[0].to_f : -captures[0].to_f
lon = captures[5].downcase != "w" ? captures[3].to_f : -captures[3].to_f lon = !captures[5].casecmp("w").zero? ? captures[3].to_f : -captures[3].to_f
rescue rescue
lat = captures[0].downcase != "s" ? captures[1].to_f : -captures[1].to_f lat = !captures[0].casecmp("s").zero? ? captures[1].to_f : -captures[1].to_f
lon = captures[3].downcase != "w" ? captures[4].to_f : -captures[4].to_f lon = !captures[3].casecmp("w").zero? ? captures[4].to_f : -captures[4].to_f
end end
{ :lat => lat, :lon => lon } { :lat => lat, :lon => lon }
end end
@ -351,11 +351,11 @@ class GeocoderController < ApplicationController
def ddm_to_decdeg(captures) def ddm_to_decdeg(captures)
begin begin
Float(captures[0]) Float(captures[0])
lat = captures[3].downcase != "s" ? captures[0].to_f + captures[1].to_f / 60 : -(captures[0].to_f + captures[1].to_f / 60) lat = !captures[3].casecmp("s").zero? ? captures[0].to_f + captures[1].to_f / 60 : -(captures[0].to_f + captures[1].to_f / 60)
lon = captures[7].downcase != "w" ? captures[4].to_f + captures[5].to_f / 60 : -(captures[4].to_f + captures[5].to_f / 60) lon = !captures[7].casecmp("w").zero? ? captures[4].to_f + captures[5].to_f / 60 : -(captures[4].to_f + captures[5].to_f / 60)
rescue rescue
lat = captures[0].downcase != "s" ? captures[1].to_f + captures[2].to_f / 60 : -(captures[1].to_f + captures[2].to_f / 60) lat = !captures[0].casecmp("s").zero? ? captures[1].to_f + captures[2].to_f / 60 : -(captures[1].to_f + captures[2].to_f / 60)
lon = captures[4].downcase != "w" ? captures[5].to_f + captures[6].to_f / 60 : -(captures[5].to_f + captures[6].to_f / 60) lon = !captures[4].casecmp("w").zero? ? captures[5].to_f + captures[6].to_f / 60 : -(captures[5].to_f + captures[6].to_f / 60)
end end
{ :lat => lat, :lon => lon } { :lat => lat, :lon => lon }
end end
@ -363,11 +363,11 @@ class GeocoderController < ApplicationController
def dms_to_decdeg(captures) def dms_to_decdeg(captures)
begin begin
Float(captures[0]) Float(captures[0])
lat = captures[4].downcase != "s" ? captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60 : -(captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60) lat = !captures[4].casecmp("s").zero? ? captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60 : -(captures[0].to_f + (captures[1].to_f + captures[2].to_f / 60) / 60)
lon = captures[9].downcase != "w" ? captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60 : -(captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60) lon = !captures[9].casecmp("w").zero? ? captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60 : -(captures[5].to_f + (captures[6].to_f + captures[7].to_f / 60) / 60)
rescue rescue
lat = captures[0].downcase != "s" ? captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60 : -(captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60) lat = !captures[0].casecmp("s").zero? ? captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60 : -(captures[1].to_f + (captures[2].to_f + captures[3].to_f / 60) / 60)
lon = captures[5].downcase != "w" ? captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60 : -(captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60) lon = !captures[5].casecmp("w").zero? ? captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60 : -(captures[6].to_f + (captures[7].to_f + captures[8].to_f / 60) / 60)
end end
{ :lat => lat, :lon => lon } { :lat => lat, :lon => lon }
end end

10
app/controllers/node_controller.rb
View file

@ -41,7 +41,7 @@ class NodeController < ApplicationController
new_node = Node.from_xml(request.raw_post) new_node = Node.from_xml(request.raw_post)
unless new_node && new_node.id == node.id unless new_node && new_node.id == node.id
fail OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})") raise OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})")
end end
node.update_from(new_node, @user) node.update_from(new_node, @user)
@ -56,7 +56,7 @@ class NodeController < ApplicationController
new_node = Node.from_xml(request.raw_post) new_node = Node.from_xml(request.raw_post)
unless new_node && new_node.id == node.id unless new_node && new_node.id == node.id
fail OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})") raise OSM::APIBadUserInput.new("The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})")
end end
node.delete_with_history!(new_node, @user) node.delete_with_history!(new_node, @user)
render :text => node.version.to_s, :content_type => "text/plain" render :text => node.version.to_s, :content_type => "text/plain"
@ -65,13 +65,13 @@ class NodeController < ApplicationController
# Dump the details on many nodes whose ids are given in the "nodes" parameter. # Dump the details on many nodes whose ids are given in the "nodes" parameter.
def nodes def nodes
unless params["nodes"] unless params["nodes"]
fail OSM::APIBadUserInput.new("The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]") raise OSM::APIBadUserInput.new("The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]")
end end
ids = params["nodes"].split(",").collect(&:to_i) ids = params["nodes"].split(",").collect(&:to_i)
if ids.length == 0 if ids.empty?
fail OSM::APIBadUserInput.new("No nodes were given to search for") raise OSM::APIBadUserInput.new("No nodes were given to search for")
end end
doc = OSM::API.new.get_xml_doc doc = OSM::API.new.get_xml_doc

62
app/controllers/notes_controller.rb
View file

@ -19,10 +19,10 @@ class NotesController < ApplicationController
if params[:bbox] if params[:bbox]
bbox = BoundingBox.from_bbox_params(params) bbox = BoundingBox.from_bbox_params(params)
else else
fail OSM::APIBadUserInput.new("No l was given") unless params[:l] raise OSM::APIBadUserInput.new("No l was given") unless params[:l]
fail OSM::APIBadUserInput.new("No r was given") unless params[:r] raise OSM::APIBadUserInput.new("No r was given") unless params[:r]
fail OSM::APIBadUserInput.new("No b was given") unless params[:b] raise OSM::APIBadUserInput.new("No b was given") unless params[:b]
fail OSM::APIBadUserInput.new("No t was given") unless params[:t] raise OSM::APIBadUserInput.new("No t was given") unless params[:t]
bbox = BoundingBox.from_lrbt_params(params) bbox = BoundingBox.from_lrbt_params(params)
end end
@ -52,12 +52,12 @@ class NotesController < ApplicationController
# Create a new note # Create a new note
def create def create
# Check the ACLs # Check the ACLs
fail OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip) raise OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip)
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No lat was given") unless params[:lat] raise OSM::APIBadUserInput.new("No lat was given") unless params[:lat]
fail OSM::APIBadUserInput.new("No lon was given") unless params[:lon] raise OSM::APIBadUserInput.new("No lon was given") unless params[:lon]
fail OSM::APIBadUserInput.new("No text was given") if params[:text].blank? raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
# Extract the arguments # Extract the arguments
lon = OSM.parse_float(params[:lon], OSM::APIBadUserInput, "lon was not a number") lon = OSM.parse_float(params[:lon], OSM::APIBadUserInput, "lon was not a number")
@ -68,7 +68,7 @@ class NotesController < ApplicationController
Note.transaction do Note.transaction do
# Create the note # Create the note
@note = Note.create(:lat => lat, :lon => lon) @note = Note.create(:lat => lat, :lon => lon)
fail OSM::APIBadUserInput.new("The note is outside this world") unless @note.in_world? raise OSM::APIBadUserInput.new("The note is outside this world") unless @note.in_world?
# Save the note # Save the note
@note.save! @note.save!
@ -88,11 +88,11 @@ class NotesController < ApplicationController
# Add a comment to an existing note # Add a comment to an existing note
def comment def comment
# Check the ACLs # Check the ACLs
fail OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip) raise OSM::APIAccessDenied if Acl.no_note_comment(request.remote_ip)
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No id was given") unless params[:id] raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
fail OSM::APIBadUserInput.new("No text was given") if params[:text].blank? raise OSM::APIBadUserInput.new("No text was given") if params[:text].blank?
# Extract the arguments # Extract the arguments
id = params[:id].to_i id = params[:id].to_i
@ -100,9 +100,9 @@ class NotesController < ApplicationController
# Find the note and check it is valid # Find the note and check it is valid
@note = Note.find(id) @note = Note.find(id)
fail OSM::APINotFoundError unless @note raise OSM::APINotFoundError unless @note
fail OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
fail OSM::APINoteAlreadyClosedError.new(@note) if @note.closed? raise OSM::APINoteAlreadyClosedError.new(@note) if @note.closed?
# Add a comment to the note # Add a comment to the note
Note.transaction do Note.transaction do
@ -120,7 +120,7 @@ class NotesController < ApplicationController
# Close a note # Close a note
def close def close
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No id was given") unless params[:id] raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
# Extract the arguments # Extract the arguments
id = params[:id].to_i id = params[:id].to_i
@ -128,9 +128,9 @@ class NotesController < ApplicationController
# Find the note and check it is valid # Find the note and check it is valid
@note = Note.find_by_id(id) @note = Note.find_by_id(id)
fail OSM::APINotFoundError unless @note raise OSM::APINotFoundError unless @note
fail OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
fail OSM::APINoteAlreadyClosedError.new(@note) if @note.closed? raise OSM::APINoteAlreadyClosedError.new(@note) if @note.closed?
# Close the note and add a comment # Close the note and add a comment
Note.transaction do Note.transaction do
@ -150,7 +150,7 @@ class NotesController < ApplicationController
# Reopen a note # Reopen a note
def reopen def reopen
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No id was given") unless params[:id] raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
# Extract the arguments # Extract the arguments
id = params[:id].to_i id = params[:id].to_i
@ -158,9 +158,9 @@ class NotesController < ApplicationController
# Find the note and check it is valid # Find the note and check it is valid
@note = Note.find_by_id(id) @note = Note.find_by_id(id)
fail OSM::APINotFoundError unless @note raise OSM::APINotFoundError unless @note
fail OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? || @user.moderator? raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? || @user.moderator?
fail OSM::APINoteAlreadyOpenError.new(@note) unless @note.closed? || !@note.visible? raise OSM::APINoteAlreadyOpenError.new(@note) unless @note.closed? || !@note.visible?
# Reopen the note and add a comment # Reopen the note and add a comment
Note.transaction do Note.transaction do
@ -205,12 +205,12 @@ class NotesController < ApplicationController
# Read a note # Read a note
def show def show
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No id was given") unless params[:id] raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
# Find the note and check it is valid # Find the note and check it is valid
@note = Note.find(params[:id]) @note = Note.find(params[:id])
fail OSM::APINotFoundError unless @note raise OSM::APINotFoundError unless @note
fail OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
# Render the result # Render the result
respond_to do |format| respond_to do |format|
@ -225,7 +225,7 @@ class NotesController < ApplicationController
# Delete (hide) a note # Delete (hide) a note
def destroy def destroy
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No id was given") unless params[:id] raise OSM::APIBadUserInput.new("No id was given") unless params[:id]
# Extract the arguments # Extract the arguments
id = params[:id].to_i id = params[:id].to_i
@ -233,8 +233,8 @@ class NotesController < ApplicationController
# Find the note and check it is valid # Find the note and check it is valid
@note = Note.find(id) @note = Note.find(id)
fail OSM::APINotFoundError unless @note raise OSM::APINotFoundError unless @note
fail OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible? raise OSM::APIAlreadyDeletedError.new("note", @note.id) unless @note.visible?
# Mark the note as hidden # Mark the note as hidden
Note.transaction do Note.transaction do
@ -255,7 +255,7 @@ class NotesController < ApplicationController
# Return a list of notes matching a given string # Return a list of notes matching a given string
def search def search
# Check the arguments are sane # Check the arguments are sane
fail OSM::APIBadUserInput.new("No query string was given") unless params[:q] raise OSM::APIBadUserInput.new("No query string was given") unless params[:q]
# Get any conditions that need to be applied # Get any conditions that need to be applied
@notes = closed_condition(Note.all) @notes = closed_condition(Note.all)
@ -308,7 +308,7 @@ class NotesController < ApplicationController
if params[:limit].to_i > 0 && params[:limit].to_i <= 10000 if params[:limit].to_i > 0 && params[:limit].to_i <= 10000
params[:limit].to_i params[:limit].to_i
else else
fail OSM::APIBadUserInput.new("Note limit must be between 1 and 10000") raise OSM::APIBadUserInput.new("Note limit must be between 1 and 10000")
end end
else else
100 100

2
app/controllers/old_controller.rb
View file

@ -19,7 +19,7 @@ class OldController < ApplicationController
# the .where() method used in the lookup_old_element_versions # the .where() method used in the lookup_old_element_versions
# call won't throw an error if no records are found, so we have # call won't throw an error if no records are found, so we have
# to do that ourselves. # to do that ourselves.
fail OSM::APINotFoundError.new if @elements.empty? raise OSM::APINotFoundError.new if @elements.empty?
doc = OSM::API.new.get_xml_doc doc = OSM::API.new.get_xml_doc

8
app/controllers/relation_controller.rb
View file

@ -36,7 +36,7 @@ class RelationController < ApplicationController
new_relation = Relation.from_xml(request.raw_post) new_relation = Relation.from_xml(request.raw_post)
unless new_relation && new_relation.id == relation.id unless new_relation && new_relation.id == relation.id
fail OSM::APIBadUserInput.new("The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})") raise OSM::APIBadUserInput.new("The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})")
end end
relation.update_from new_relation, @user relation.update_from new_relation, @user
@ -128,13 +128,13 @@ class RelationController < ApplicationController
def relations def relations
unless params["relations"] unless params["relations"]
fail OSM::APIBadUserInput.new("The parameter relations is required, and must be of the form relations=id[,id[,id...]]") raise OSM::APIBadUserInput.new("The parameter relations is required, and must be of the form relations=id[,id[,id...]]")
end end
ids = params["relations"].split(",").collect(&:to_i) ids = params["relations"].split(",").collect(&:to_i)
if ids.length == 0 if ids.empty?
fail OSM::APIBadUserInput.new("No relations were given to search for") raise OSM::APIBadUserInput.new("No relations were given to search for")
end end
doc = OSM::API.new.get_xml_doc doc = OSM::API.new.get_xml_doc

2
app/controllers/trace_controller.rb
View file

@ -264,7 +264,7 @@ class TraceController < ApplicationController
new_trace = Trace.from_xml(request.raw_post) new_trace = Trace.from_xml(request.raw_post)
unless new_trace && new_trace.id == trace.id unless new_trace && new_trace.id == trace.id
fail OSM::APIBadUserInput.new("The id in the url (#{trace.id}) is not the same as provided in the xml (#{new_trace.id})") raise OSM::APIBadUserInput.new("The id in the url (#{trace.id}) is not the same as provided in the xml (#{new_trace.id})")
end end
trace.description = new_trace.description trace.description = new_trace.description

2
app/controllers/user_controller.rb
View file

@ -653,7 +653,7 @@ class UserController < ApplicationController
user.display_name = params[:user][:display_name] user.display_name = params[:user][:display_name]
user.new_email = params[:user][:new_email] user.new_email = params[:user][:new_email]
if params[:user][:pass_crypt].length > 0 || params[:user][:pass_crypt_confirmation].length > 0 unless params[:user][:pass_crypt].empty? && params[:user][:pass_crypt_confirmation].empty?
user.pass_crypt = params[:user][:pass_crypt] user.pass_crypt = params[:user][:pass_crypt]
user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation] user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation]
end end

2
app/controllers/user_preference_controller.rb
View file

@ -45,7 +45,7 @@ class UserPreferenceController < ApplicationController
if preference = old_preferences.delete(pt["k"]) if preference = old_preferences.delete(pt["k"])
preference.v = pt["v"] preference.v = pt["v"]
elsif new_preferences.include?(pt["k"]) elsif new_preferences.include?(pt["k"])
fail OSM::APIDuplicatePreferenceError.new(pt["k"]) raise OSM::APIDuplicatePreferenceError.new(pt["k"])
else else
preference = @user.preferences.build(:k => pt["k"], :v => pt["v"]) preference = @user.preferences.build(:k => pt["k"], :v => pt["v"])
end end

8
app/controllers/way_controller.rb
View file

@ -36,7 +36,7 @@ class WayController < ApplicationController
new_way = Way.from_xml(request.raw_post) new_way = Way.from_xml(request.raw_post)
unless new_way && new_way.id == way.id unless new_way && new_way.id == way.id
fail OSM::APIBadUserInput.new("The id in the url (#{way.id}) is not the same as provided in the xml (#{new_way.id})") raise OSM::APIBadUserInput.new("The id in the url (#{way.id}) is not the same as provided in the xml (#{new_way.id})")
end end
way.update_from(new_way, @user) way.update_from(new_way, @user)
@ -81,13 +81,13 @@ class WayController < ApplicationController
def ways def ways
unless params["ways"] unless params["ways"]
fail OSM::APIBadUserInput.new("The parameter ways is required, and must be of the form ways=id[,id[,id...]]") raise OSM::APIBadUserInput.new("The parameter ways is required, and must be of the form ways=id[,id[,id...]]")
end end
ids = params["ways"].split(",").collect(&:to_i) ids = params["ways"].split(",").collect(&:to_i)
if ids.length == 0 if ids.empty?
fail OSM::APIBadUserInput.new("No ways were given to search for") raise OSM::APIBadUserInput.new("No ways were given to search for")
end end
doc = OSM::API.new.get_xml_doc doc = OSM::API.new.get_xml_doc

12
app/models/changeset.rb
View file

@ -63,7 +63,7 @@ class Changeset < ActiveRecord::Base
doc.find("//osm/changeset").each do |pt| doc.find("//osm/changeset").each do |pt|
return Changeset.from_xml_node(pt, create) return Changeset.from_xml_node(pt, create)
end end
fail OSM::APIBadXMLError.new("changeset", xml, "XML doesn't contain an osm/changeset element.") raise OSM::APIBadXMLError.new("changeset", xml, "XML doesn't contain an osm/changeset element.")
rescue LibXML::XML::Error, ArgumentError => ex rescue LibXML::XML::Error, ArgumentError => ex
raise OSM::APIBadXMLError.new("changeset", xml, ex.message) raise OSM::APIBadXMLError.new("changeset", xml, ex.message)
end end
@ -80,8 +80,8 @@ class Changeset < ActiveRecord::Base
end end
pt.find("tag").each do |tag| pt.find("tag").each do |tag|
fail OSM::APIBadXMLError.new("changeset", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("changeset", pt, "tag is missing key") if tag["k"].nil?
fail OSM::APIBadXMLError.new("changeset", pt, "tag is missing value") if tag["v"].nil? raise OSM::APIBadXMLError.new("changeset", pt, "tag is missing value") if tag["v"].nil?
cs.add_tag_keyval(tag["k"], tag["v"]) cs.add_tag_keyval(tag["k"], tag["v"])
end end
@ -137,7 +137,7 @@ class Changeset < ActiveRecord::Base
# duplicate tags are now forbidden, so we can't allow values # duplicate tags are now forbidden, so we can't allow values
# in the hash to be overwritten. # in the hash to be overwritten.
fail OSM::APIDuplicateTagsError.new("changeset", id, k) if @tags.include? k raise OSM::APIDuplicateTagsError.new("changeset", id, k) if @tags.include? k
@tags[k] = v @tags[k] = v
end end
@ -241,10 +241,10 @@ class Changeset < ActiveRecord::Base
# bounding box, only the tags of the changeset. # bounding box, only the tags of the changeset.
def update_from(other, user) def update_from(other, user)
# ensure that only the user who opened the changeset may modify it. # ensure that only the user who opened the changeset may modify it.
fail OSM::APIUserChangesetMismatchError.new unless user.id == user_id raise OSM::APIUserChangesetMismatchError.new unless user.id == user_id
# can't change a closed changeset # can't change a closed changeset
fail OSM::APIChangesetAlreadyClosedError.new(self) unless is_open? raise OSM::APIChangesetAlreadyClosedError.new(self) unless is_open?
# copy the other's tags # copy the other's tags
self.tags = other.tags self.tags = other.tags

28
app/models/node.rb
View file

@ -55,7 +55,7 @@ class Node < ActiveRecord::Base
doc.find("//osm/node").each do |pt| doc.find("//osm/node").each do |pt|
return Node.from_xml_node(pt, create) return Node.from_xml_node(pt, create)
end end
fail OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/node element.") raise OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/node element.")
rescue LibXML::XML::Error, ArgumentError => ex rescue LibXML::XML::Error, ArgumentError => ex
raise OSM::APIBadXMLError.new("node", xml, ex.message) raise OSM::APIBadXMLError.new("node", xml, ex.message)
end end
@ -63,25 +63,25 @@ class Node < ActiveRecord::Base
def self.from_xml_node(pt, create = false) def self.from_xml_node(pt, create = false)
node = Node.new node = Node.new
fail OSM::APIBadXMLError.new("node", pt, "lat missing") if pt["lat"].nil? raise OSM::APIBadXMLError.new("node", pt, "lat missing") if pt["lat"].nil?
fail OSM::APIBadXMLError.new("node", pt, "lon missing") if pt["lon"].nil? raise OSM::APIBadXMLError.new("node", pt, "lon missing") if pt["lon"].nil?
node.lat = OSM.parse_float(pt["lat"], OSM::APIBadXMLError, "node", pt, "lat not a number") node.lat = OSM.parse_float(pt["lat"], OSM::APIBadXMLError, "node", pt, "lat not a number")
node.lon = OSM.parse_float(pt["lon"], OSM::APIBadXMLError, "node", pt, "lon not a number") node.lon = OSM.parse_float(pt["lon"], OSM::APIBadXMLError, "node", pt, "lon not a number")
fail OSM::APIBadXMLError.new("node", pt, "Changeset id is missing") if pt["changeset"].nil? raise OSM::APIBadXMLError.new("node", pt, "Changeset id is missing") if pt["changeset"].nil?
node.changeset_id = pt["changeset"].to_i node.changeset_id = pt["changeset"].to_i
fail OSM::APIBadUserInput.new("The node is outside this world") unless node.in_world? raise OSM::APIBadUserInput.new("The node is outside this world") unless node.in_world?
# version must be present unless creating # version must be present unless creating
fail OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create || !pt["version"].nil? raise OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create || !pt["version"].nil?
node.version = create ? 0 : pt["version"].to_i node.version = create ? 0 : pt["version"].to_i
unless create unless create
fail OSM::APIBadXMLError.new("node", pt, "ID is required when updating.") if pt["id"].nil? raise OSM::APIBadXMLError.new("node", pt, "ID is required when updating.") if pt["id"].nil?
node.id = pt["id"].to_i node.id = pt["id"].to_i
# .to_i will return 0 if there is no number that can be parsed. # .to_i will return 0 if there is no number that can be parsed.
# We want to make sure that there is no id with zero anyway # We want to make sure that there is no id with zero anyway
fail OSM::APIBadUserInput.new("ID of node cannot be zero when updating.") if node.id == 0 raise OSM::APIBadUserInput.new("ID of node cannot be zero when updating.") if node.id == 0
end end
# We don't care about the time, as it is explicitly set on create/update/delete # We don't care about the time, as it is explicitly set on create/update/delete
@ -94,8 +94,8 @@ class Node < ActiveRecord::Base
# Add in any tags from the XML # Add in any tags from the XML
pt.find("tag").each do |tag| pt.find("tag").each do |tag|
fail OSM::APIBadXMLError.new("node", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("node", pt, "tag is missing key") if tag["k"].nil?
fail OSM::APIBadXMLError.new("node", pt, "tag is missing value") if tag["v"].nil? raise OSM::APIBadXMLError.new("node", pt, "tag is missing value") if tag["v"].nil?
node.add_tag_key_val(tag["k"], tag["v"]) node.add_tag_key_val(tag["k"], tag["v"])
end end
@ -111,7 +111,7 @@ class Node < ActiveRecord::Base
# Should probably be renamed delete_from to come in line with update # Should probably be renamed delete_from to come in line with update
def delete_with_history!(new_node, user) def delete_with_history!(new_node, user)
fail OSM::APIAlreadyDeletedError.new("node", new_node.id) unless visible raise OSM::APIAlreadyDeletedError.new("node", new_node.id) unless visible
# need to start the transaction here, so that the database can # need to start the transaction here, so that the database can
# provide repeatable reads for the used-by checks. this means it # provide repeatable reads for the used-by checks. this means it
@ -120,10 +120,10 @@ class Node < ActiveRecord::Base
lock! lock!
check_consistency(self, new_node, user) check_consistency(self, new_node, user)
ways = Way.joins(:way_nodes).where(:visible => true, :current_way_nodes => { :node_id => id }).order(:id) ways = Way.joins(:way_nodes).where(:visible => true, :current_way_nodes => { :node_id => id }).order(:id)
fail OSM::APIPreconditionFailedError.new("Node #{id} is still used by ways #{ways.collect(&:id).join(",")}.") unless ways.empty? raise OSM::APIPreconditionFailedError.new("Node #{id} is still used by ways #{ways.collect(&:id).join(",")}.") unless ways.empty?
rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Node", :member_id => id }).order(:id) rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Node", :member_id => id }).order(:id)
fail OSM::APIPreconditionFailedError.new("Node #{id} is still used by relations #{rels.collect(&:id).join(",")}.") unless rels.empty? raise OSM::APIPreconditionFailedError.new("Node #{id} is still used by relations #{rels.collect(&:id).join(",")}.") unless rels.empty?
self.changeset_id = new_node.changeset_id self.changeset_id = new_node.changeset_id
self.tags = {} self.tags = {}
@ -209,7 +209,7 @@ class Node < ActiveRecord::Base
# duplicate tags are now forbidden, so we can't allow values # duplicate tags are now forbidden, so we can't allow values
# in the hash to be overwritten. # in the hash to be overwritten.
fail OSM::APIDuplicateTagsError.new("node", id, k) if @tags.include? k raise OSM::APIDuplicateTagsError.new("node", id, k) if @tags.include? k
@tags[k] = v @tags[k] = v
end end

32
app/models/relation.rb
View file

@ -42,7 +42,7 @@ class Relation < ActiveRecord::Base
doc.find("//osm/relation").each do |pt| doc.find("//osm/relation").each do |pt|
return Relation.from_xml_node(pt, create) return Relation.from_xml_node(pt, create)
end end
fail OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/relation element.") raise OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/relation element.")
rescue LibXML::XML::Error, ArgumentError => ex rescue LibXML::XML::Error, ArgumentError => ex
raise OSM::APIBadXMLError.new("relation", xml, ex.message) raise OSM::APIBadXMLError.new("relation", xml, ex.message)
end end
@ -50,17 +50,17 @@ class Relation < ActiveRecord::Base
def self.from_xml_node(pt, create = false) def self.from_xml_node(pt, create = false)
relation = Relation.new relation = Relation.new
fail OSM::APIBadXMLError.new("relation", pt, "Version is required when updating") unless create || !pt["version"].nil? raise OSM::APIBadXMLError.new("relation", pt, "Version is required when updating") unless create || !pt["version"].nil?
relation.version = pt["version"] relation.version = pt["version"]
fail OSM::APIBadXMLError.new("relation", pt, "Changeset id is missing") if pt["changeset"].nil? raise OSM::APIBadXMLError.new("relation", pt, "Changeset id is missing") if pt["changeset"].nil?
relation.changeset_id = pt["changeset"] relation.changeset_id = pt["changeset"]
unless create unless create
fail OSM::APIBadXMLError.new("relation", pt, "ID is required when updating") if pt["id"].nil? raise OSM::APIBadXMLError.new("relation", pt, "ID is required when updating") if pt["id"].nil?
relation.id = pt["id"].to_i relation.id = pt["id"].to_i
# .to_i will return 0 if there is no number that can be parsed. # .to_i will return 0 if there is no number that can be parsed.
# We want to make sure that there is no id with zero anyway # We want to make sure that there is no id with zero anyway
fail OSM::APIBadUserInput.new("ID of relation cannot be zero when updating.") if relation.id == 0 raise OSM::APIBadUserInput.new("ID of relation cannot be zero when updating.") if relation.id == 0
end end
# We don't care about the timestamp nor the visibility as these are either # We don't care about the timestamp nor the visibility as these are either
@ -73,8 +73,8 @@ class Relation < ActiveRecord::Base
# Add in any tags from the XML # Add in any tags from the XML
pt.find("tag").each do |tag| pt.find("tag").each do |tag|
fail OSM::APIBadXMLError.new("relation", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("relation", pt, "tag is missing key") if tag["k"].nil?
fail OSM::APIBadXMLError.new("relation", pt, "tag is missing value") if tag["v"].nil? raise OSM::APIBadXMLError.new("relation", pt, "tag is missing value") if tag["v"].nil?
relation.add_tag_keyval(tag["k"], tag["v"]) relation.add_tag_keyval(tag["k"], tag["v"])
end end
@ -86,13 +86,13 @@ class Relation < ActiveRecord::Base
pt.find("member").each do |member| pt.find("member").each do |member|
# member_type = # member_type =
fail OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member["type"] raise OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member["type"]
# member_ref = member['ref'] # member_ref = member['ref']
# member_role # member_role
member["role"] ||= "" # Allow the upload to not include this, in which case we default to an empty string. member["role"] ||= "" # Allow the upload to not include this, in which case we default to an empty string.
relation.add_member(member["type"].classify, member["ref"], member["role"]) relation.add_member(member["type"].classify, member["ref"], member["role"])
end end
fail OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil? raise OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil?
relation relation
end end
@ -159,7 +159,7 @@ class Relation < ActiveRecord::Base
# duplicate tags are now forbidden, so we can't allow values # duplicate tags are now forbidden, so we can't allow values
# in the hash to be overwritten. # in the hash to be overwritten.
fail OSM::APIDuplicateTagsError.new("relation", id, k) if @tags.include? k raise OSM::APIDuplicateTagsError.new("relation", id, k) if @tags.include? k
@tags[k] = v @tags[k] = v
end end
@ -176,7 +176,7 @@ class Relation < ActiveRecord::Base
def delete_with_history!(new_relation, user) def delete_with_history!(new_relation, user)
unless visible unless visible
fail OSM::APIAlreadyDeletedError.new("relation", new_relation.id) raise OSM::APIAlreadyDeletedError.new("relation", new_relation.id)
end end
# need to start the transaction here, so that the database can # need to start the transaction here, so that the database can
@ -187,7 +187,7 @@ class Relation < ActiveRecord::Base
check_consistency(self, new_relation, user) check_consistency(self, new_relation, user)
# This will check to see if this relation is used by another relation # This will check to see if this relation is used by another relation
rel = RelationMember.joins(:relation).find_by("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id) rel = RelationMember.joins(:relation).find_by("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id)
fail OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is used in relation #{rel.relation.id}.") unless rel.nil? 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 self.changeset_id = new_relation.changeset_id
self.tags = {} self.tags = {}
@ -202,7 +202,7 @@ class Relation < ActiveRecord::Base
lock! lock!
check_consistency(self, new_relation, user) check_consistency(self, new_relation, user)
unless new_relation.preconditions_ok?(members) unless new_relation.preconditions_ok?(members)
fail OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.") raise OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.")
end end
self.changeset_id = new_relation.changeset_id self.changeset_id = new_relation.changeset_id
self.changeset = new_relation.changeset self.changeset = new_relation.changeset
@ -216,7 +216,7 @@ class Relation < ActiveRecord::Base
def create_with_history(user) def create_with_history(user)
check_create_consistency(self, user) check_create_consistency(self, user)
unless preconditions_ok? unless preconditions_ok?
fail OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.") raise OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.")
end end
self.version = 0 self.version = 0
self.visible = true self.visible = true
@ -253,7 +253,7 @@ class Relation < ActiveRecord::Base
# and check that it is OK to use. # and check that it is OK to use.
unless element && element.visible? && element.preconditions_ok? unless element && element.visible? && element.preconditions_ok?
fail OSM::APIPreconditionFailedError.new("Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}") raise OSM::APIPreconditionFailedError.new("Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}")
end end
hash[m[1]] = true hash[m[1]] = true
end end
@ -270,7 +270,7 @@ class Relation < ActiveRecord::Base
old_id = id.to_i old_id = id.to_i
if old_id < 0 if old_id < 0
new_id = id_map[type.downcase.to_sym][old_id] new_id = id_map[type.downcase.to_sym][old_id]
fail OSM::APIBadUserInput.new("Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}.") if new_id.nil? raise OSM::APIBadUserInput.new("Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}.") if new_id.nil?
[type, new_id, role] [type, new_id, role]
else else
[type, id, role] [type, id, role]

14
app/models/trace.rb
View file

@ -29,14 +29,14 @@ class Trace < ActiveRecord::Base
def tagstring=(s) def tagstring=(s)
self.tags = if s.include? "," self.tags = if s.include? ","
s.split(/\s*,\s*/).select { |tag| tag !~ /^\s*$/ }.collect do|tag| s.split(/\s*,\s*/).select { |tag| tag !~ /^\s*$/ }.collect do |tag|
tt = Tracetag.new tt = Tracetag.new
tt.tag = tag tt.tag = tag
tt tt
end end
else else
# do as before for backwards compatibility: # do as before for backwards compatibility:
s.split.collect do|tag| s.split.collect do |tag|
tt = Tracetag.new tt = Tracetag.new
tt.tag = tag tt.tag = tag
tt tt
@ -181,7 +181,7 @@ class Trace < ActiveRecord::Base
return Trace.from_xml_node(pt, create) return Trace.from_xml_node(pt, create)
end end
fail OSM::APIBadXMLError.new("trace", xml, "XML doesn't contain an osm/gpx_file element.") raise OSM::APIBadXMLError.new("trace", xml, "XML doesn't contain an osm/gpx_file element.")
rescue LibXML::XML::Error, ArgumentError => ex rescue LibXML::XML::Error, ArgumentError => ex
raise OSM::APIBadXMLError.new("trace", xml, ex.message) raise OSM::APIBadXMLError.new("trace", xml, ex.message)
end end
@ -189,15 +189,15 @@ class Trace < ActiveRecord::Base
def self.from_xml_node(pt, create = false) def self.from_xml_node(pt, create = false)
trace = Trace.new trace = Trace.new
fail OSM::APIBadXMLError.new("trace", pt, "visibility missing") if pt["visibility"].nil? raise OSM::APIBadXMLError.new("trace", pt, "visibility missing") if pt["visibility"].nil?
trace.visibility = pt["visibility"] trace.visibility = pt["visibility"]
unless create unless create
fail OSM::APIBadXMLError.new("trace", pt, "ID is required when updating.") if pt["id"].nil? raise OSM::APIBadXMLError.new("trace", pt, "ID is required when updating.") if pt["id"].nil?
trace.id = pt["id"].to_i trace.id = pt["id"].to_i
# .to_i will return 0 if there is no number that can be parsed. # .to_i will return 0 if there is no number that can be parsed.
# We want to make sure that there is no id with zero anyway # We want to make sure that there is no id with zero anyway
fail OSM::APIBadUserInput.new("ID of trace cannot be zero when updating.") if trace.id == 0 raise OSM::APIBadUserInput.new("ID of trace cannot be zero when updating.") if trace.id == 0
end end
# We don't care about the time, as it is explicitly set on create/update/delete # We don't care about the time, as it is explicitly set on create/update/delete
@ -206,7 +206,7 @@ class Trace < ActiveRecord::Base
trace.visible = true trace.visible = true
description = pt.find("description").first description = pt.find("description").first
fail OSM::APIBadXMLError.new("trace", pt, "description missing") if description.nil? raise OSM::APIBadXMLError.new("trace", pt, "description missing") if description.nil?
trace.description = description.content trace.description = description.content
pt.find("tag").each do |tag| pt.find("tag").each do |tag|

4
app/models/user.rb
View file

@ -217,8 +217,8 @@ class User < ActiveRecord::Base
score = description.spam_score / 4.0 score = description.spam_score / 4.0
score += diary_entries.where("created_at > ?", 1.day.ago).count * 10 score += diary_entries.where("created_at > ?", 1.day.ago).count * 10
score += diary_entry_score / diary_entries.length if diary_entries.length > 0 score += diary_entry_score / diary_entries.length unless diary_entries.empty?
score += diary_comment_score / diary_comments.length if diary_comments.length > 0 score += diary_comment_score / diary_comments.length unless diary_comments.empty?
score -= changeset_score score -= changeset_score
score -= trace_score score -= trace_score

30
app/models/way.rb
View file

@ -40,7 +40,7 @@ class Way < ActiveRecord::Base
doc.find("//osm/way").each do |pt| doc.find("//osm/way").each do |pt|
return Way.from_xml_node(pt, create) return Way.from_xml_node(pt, create)
end end
fail OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/way element.") raise OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/way element.")
rescue LibXML::XML::Error, ArgumentError => ex rescue LibXML::XML::Error, ArgumentError => ex
raise OSM::APIBadXMLError.new("way", xml, ex.message) raise OSM::APIBadXMLError.new("way", xml, ex.message)
end end
@ -48,17 +48,17 @@ class Way < ActiveRecord::Base
def self.from_xml_node(pt, create = false) def self.from_xml_node(pt, create = false)
way = Way.new way = Way.new
fail OSM::APIBadXMLError.new("way", pt, "Version is required when updating") unless create || !pt["version"].nil? raise OSM::APIBadXMLError.new("way", pt, "Version is required when updating") unless create || !pt["version"].nil?
way.version = pt["version"] way.version = pt["version"]
fail OSM::APIBadXMLError.new("way", pt, "Changeset id is missing") if pt["changeset"].nil? raise OSM::APIBadXMLError.new("way", pt, "Changeset id is missing") if pt["changeset"].nil?
way.changeset_id = pt["changeset"] way.changeset_id = pt["changeset"]
unless create unless create
fail OSM::APIBadXMLError.new("way", pt, "ID is required when updating") if pt["id"].nil? raise OSM::APIBadXMLError.new("way", pt, "ID is required when updating") if pt["id"].nil?
way.id = pt["id"].to_i way.id = pt["id"].to_i
# .to_i will return 0 if there is no number that can be parsed. # .to_i will return 0 if there is no number that can be parsed.
# We want to make sure that there is no id with zero anyway # We want to make sure that there is no id with zero anyway
fail OSM::APIBadUserInput.new("ID of way cannot be zero when updating.") if way.id == 0 raise OSM::APIBadUserInput.new("ID of way cannot be zero when updating.") if way.id == 0
end end
# We don't care about the timestamp nor the visibility as these are either # We don't care about the timestamp nor the visibility as these are either
@ -71,8 +71,8 @@ class Way < ActiveRecord::Base
# Add in any tags from the XML # Add in any tags from the XML
pt.find("tag").each do |tag| pt.find("tag").each do |tag|
fail OSM::APIBadXMLError.new("way", pt, "tag is missing key") if tag["k"].nil? raise OSM::APIBadXMLError.new("way", pt, "tag is missing key") if tag["k"].nil?
fail OSM::APIBadXMLError.new("way", pt, "tag is missing value") if tag["v"].nil? raise OSM::APIBadXMLError.new("way", pt, "tag is missing value") if tag["v"].nil?
way.add_tag_keyval(tag["k"], tag["v"]) way.add_tag_keyval(tag["k"], tag["v"])
end end
@ -147,7 +147,7 @@ class Way < ActiveRecord::Base
# duplicate tags are now forbidden, so we can't allow values # duplicate tags are now forbidden, so we can't allow values
# in the hash to be overwritten. # in the hash to be overwritten.
fail OSM::APIDuplicateTagsError.new("way", id, k) if @tags.include? k raise OSM::APIDuplicateTagsError.new("way", id, k) if @tags.include? k
@tags[k] = v @tags[k] = v
end end
@ -166,7 +166,7 @@ class Way < ActiveRecord::Base
lock! lock!
check_consistency(self, new_way, user) check_consistency(self, new_way, user)
unless new_way.preconditions_ok?(nds) unless new_way.preconditions_ok?(nds)
fail OSM::APIPreconditionFailedError.new("Cannot update way #{id}: data is invalid.") raise OSM::APIPreconditionFailedError.new("Cannot update way #{id}: data is invalid.")
end end
self.changeset_id = new_way.changeset_id self.changeset_id = new_way.changeset_id
@ -181,7 +181,7 @@ class Way < ActiveRecord::Base
def create_with_history(user) def create_with_history(user)
check_create_consistency(self, user) check_create_consistency(self, user)
unless preconditions_ok? unless preconditions_ok?
fail OSM::APIPreconditionFailedError.new("Cannot create way: data is invalid.") raise OSM::APIPreconditionFailedError.new("Cannot create way: data is invalid.")
end end
self.version = 0 self.version = 0
self.visible = true self.visible = true
@ -191,7 +191,7 @@ class Way < ActiveRecord::Base
def preconditions_ok?(old_nodes = []) def preconditions_ok?(old_nodes = [])
return false if nds.empty? return false if nds.empty?
if nds.length > MAX_NUMBER_OF_WAY_NODES if nds.length > MAX_NUMBER_OF_WAY_NODES
fail OSM::APITooManyWayNodesError.new(id, nds.length, MAX_NUMBER_OF_WAY_NODES) raise OSM::APITooManyWayNodesError.new(id, nds.length, MAX_NUMBER_OF_WAY_NODES)
end end
# check only the new nodes, for efficiency - old nodes having been checked last time and can't # check only the new nodes, for efficiency - old nodes having been checked last time and can't
@ -205,7 +205,7 @@ class Way < ActiveRecord::Base
if db_nds.length < new_nds.length if db_nds.length < new_nds.length
missing = new_nds - db_nds.collect(&:id) missing = new_nds - db_nds.collect(&:id)
fail OSM::APIPreconditionFailedError.new("Way #{id} requires the nodes with id in (#{missing.join(',')}), which either do not exist, or are not visible.") raise OSM::APIPreconditionFailedError.new("Way #{id} requires the nodes with id in (#{missing.join(',')}), which either do not exist, or are not visible.")
end end
end end
@ -213,7 +213,7 @@ class Way < ActiveRecord::Base
end end
def delete_with_history!(new_way, user) def delete_with_history!(new_way, user)
fail OSM::APIAlreadyDeletedError.new("way", new_way.id) unless visible raise OSM::APIAlreadyDeletedError.new("way", new_way.id) unless visible
# need to start the transaction here, so that the database can # need to start the transaction here, so that the database can
# provide repeatable reads for the used-by checks. this means it # provide repeatable reads for the used-by checks. this means it
@ -222,7 +222,7 @@ class Way < ActiveRecord::Base
lock! lock!
check_consistency(self, new_way, user) check_consistency(self, new_way, user)
rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Way", :member_id => id }).order(:id) rels = Relation.joins(:relation_members).where(:visible => true, :current_relation_members => { :member_type => "Way", :member_id => id }).order(:id)
fail OSM::APIPreconditionFailedError.new("Way #{id} is still used by relations #{rels.collect(&:id).join(",")}.") unless rels.empty? raise OSM::APIPreconditionFailedError.new("Way #{id} is still used by relations #{rels.collect(&:id).join(",")}.") unless rels.empty?
self.changeset_id = new_way.changeset_id self.changeset_id = new_way.changeset_id
self.changeset = new_way.changeset self.changeset = new_way.changeset
@ -242,7 +242,7 @@ class Way < ActiveRecord::Base
nds.map! do |node_id| nds.map! do |node_id|
if node_id < 0 if node_id < 0
new_id = id_map[:node][node_id] new_id = id_map[:node][node_id]
fail OSM::APIBadUserInput.new("Placeholder node not found for reference #{node_id} in way #{id.nil? ? placeholder_id : id}") if new_id.nil? raise OSM::APIBadUserInput.new("Placeholder node not found for reference #{node_id} in way #{id.nil? ? placeholder_id : id}") if new_id.nil?
new_id new_id
else else
node_id node_id

2
config/initializers/libxml.rb
View file

@ -1,5 +1,5 @@
# This is required otherwise libxml writes out memory errors to # This is required otherwise libxml writes out memory errors to
# the standard output and exits uncleanly # the standard output and exits uncleanly
LibXML::XML::Error.set_handler do |message| LibXML::XML::Error.set_handler do |message|
fail message raise message
end end

6
db/migrate/008_remove_segments.rb
View file

@ -11,13 +11,13 @@ class RemoveSegments < ActiveRecord::Migration
src = "#{cmd}.cc" src = "#{cmd}.cc"
if !File.exist?(cmd) || File.mtime(cmd) < File.mtime(src) if !File.exist?(cmd) || File.mtime(cmd) < File.mtime(src)
system("c++ -O3 -Wall `mysql_config --cflags --libs` " + system("c++ -O3 -Wall `mysql_config --cflags --libs` " +
"#{src} -o #{cmd}") || fail "#{src} -o #{cmd}") || raise
end end
conn_opts = ActiveRecord::Base.connection conn_opts = ActiveRecord::Base.connection
.instance_eval { @connection_options } .instance_eval { @connection_options }
args = conn_opts.map(&:to_s) + [prefix] args = conn_opts.map(&:to_s) + [prefix]
fail "#{cmd} failed" unless system cmd, *args raise "#{cmd} failed" unless system cmd, *args
tempfiles = %w(ways way_nodes way_tags relations relation_members relation_tags) tempfiles = %w(ways way_nodes way_tags relations relation_members relation_tags)
.map { |base| prefix + base } .map { |base| prefix + base }
@ -81,6 +81,6 @@ class RemoveSegments < ActiveRecord::Migration
end end
def self.down def self.down
fail ActiveRecord::IrreversibleMigration raise ActiveRecord::IrreversibleMigration
end end
end end

6
db/migrate/020_populate_node_tags_and_remove.rb
View file

@ -11,12 +11,12 @@ class PopulateNodeTagsAndRemove < ActiveRecord::Migration
src = "#{cmd}.c" src = "#{cmd}.c"
if !File.exist?(cmd) || File.mtime(cmd) < File.mtime(src) if !File.exist?(cmd) || File.mtime(cmd) < File.mtime(src)
system("cc -O3 -Wall `mysql_config --cflags --libs` " + system("cc -O3 -Wall `mysql_config --cflags --libs` " +
"#{src} -o #{cmd}") || fail "#{src} -o #{cmd}") || raise
end end
conn_opts = ActiveRecord::Base.connection.instance_eval { @connection_options } conn_opts = ActiveRecord::Base.connection.instance_eval { @connection_options }
args = conn_opts.map(&:to_s) + [prefix] args = conn_opts.map(&:to_s) + [prefix]
fail "#{cmd} failed" unless system cmd, *args raise "#{cmd} failed" unless system cmd, *args
tempfiles = %w(nodes node_tags current_nodes current_node_tags) tempfiles = %w(nodes node_tags current_nodes current_node_tags)
.map { |base| prefix + base } .map { |base| prefix + base }
@ -56,7 +56,7 @@ class PopulateNodeTagsAndRemove < ActiveRecord::Migration
end end
def self.down def self.down
fail ActiveRecord::IrreversibleMigration raise ActiveRecord::IrreversibleMigration
# add_column :nodes, "tags", :text, :default => "", :null => false # add_column :nodes, "tags", :text, :default => "", :null => false
# add_column :current_nodes, "tags", :text, :default => "", :null => false # add_column :current_nodes, "tags", :text, :default => "", :null => false
end end

2
db/migrate/021_move_to_innodb.rb
View file

@ -36,6 +36,6 @@ class MoveToInnodb < ActiveRecord::Migration
end end
def self.down def self.down
fail ActiveRecord::IrreversibleMigration raise ActiveRecord::IrreversibleMigration
end end
end end

2
db/migrate/022_key_constraints.rb
View file

@ -47,6 +47,6 @@ class KeyConstraints < ActiveRecord::Migration
end end
def self.down def self.down
fail ActiveRecord::IrreversibleMigration raise ActiveRecord::IrreversibleMigration
end end
end end

4
db/migrate/023_add_changesets.rb
View file

@ -29,7 +29,7 @@ class AddChangesets < ActiveRecord::Migration
# all the changesets will have the id of the user that made them. # all the changesets will have the id of the user that made them.
# We need to generate a changeset for each user in the database # We need to generate a changeset for each user in the database
execute "INSERT INTO changesets (id, user_id, created_at, open)" + execute "INSERT INTO changesets (id, user_id, created_at, open)" +
"SELECT id, id, creation_time, false from users;" "SELECT id, id, creation_time, false from users;"
@conv_user_tables.each do |tbl| @conv_user_tables.each do |tbl|
rename_column tbl, :user_id, :changeset_id rename_column tbl, :user_id, :changeset_id
@ -40,7 +40,7 @@ class AddChangesets < ActiveRecord::Migration
def self.down def self.down
# It's not easy to generate the user ids from the changesets # It's not easy to generate the user ids from the changesets
fail ActiveRecord::IrreversibleMigration raise ActiveRecord::IrreversibleMigration
# drop_table "changesets" # drop_table "changesets"
# drop_table "changeset_tags" # drop_table "changeset_tags"
end end

2
db/migrate/030_add_foreign_keys.rb
View file

@ -9,6 +9,6 @@ class AddForeignKeys < ActiveRecord::Migration
end end
def self.down def self.down
fail ActiveRecord::IrreversibleMigration raise ActiveRecord::IrreversibleMigration
end end
end end

10
lib/bounding_box.rb
View file

@ -58,15 +58,15 @@ class BoundingBox
def check_boundaries def check_boundaries
# check the bbox is sane # check the bbox is sane
if min_lon > max_lon if min_lon > max_lon
fail OSM::APIBadBoundingBox.new( raise OSM::APIBadBoundingBox.new(
"The minimum longitude must be less than the maximum longitude, but it wasn't") "The minimum longitude must be less than the maximum longitude, but it wasn't")
end end
if min_lat > max_lat if min_lat > max_lat
fail OSM::APIBadBoundingBox.new( raise OSM::APIBadBoundingBox.new(
"The minimum latitude must be less than the maximum latitude, but it wasn't") "The minimum latitude must be less than the maximum latitude, but it wasn't")
end end
if min_lon < -LON_LIMIT || min_lat < -LAT_LIMIT || max_lon > +LON_LIMIT || max_lat > +LAT_LIMIT if min_lon < -LON_LIMIT || min_lat < -LAT_LIMIT || max_lon > +LON_LIMIT || max_lat > +LAT_LIMIT
fail OSM::APIBadBoundingBox.new("The latitudes must be between #{-LAT_LIMIT} and #{LAT_LIMIT}," + raise OSM::APIBadBoundingBox.new("The latitudes must be between #{-LAT_LIMIT} and #{LAT_LIMIT}," +
" and longitudes between #{-LON_LIMIT} and #{LON_LIMIT}") " and longitudes between #{-LON_LIMIT} and #{LON_LIMIT}")
end end
self self
@ -75,7 +75,7 @@ class BoundingBox
def check_size(max_area = MAX_REQUEST_AREA) def check_size(max_area = MAX_REQUEST_AREA)
# check the bbox isn't too large # check the bbox isn't too large
if area > max_area if area > max_area
fail OSM::APIBadBoundingBox.new("The maximum bbox size is " + max_area.to_s + raise OSM::APIBadBoundingBox.new("The maximum bbox size is " + max_area.to_s +
", and your request was too large. Either request a smaller area, or use planet.osm") ", and your request was too large. Either request a smaller area, or use planet.osm")
end end
self self
@ -161,7 +161,7 @@ class BoundingBox
def from_bbox_array(bbox_array) def from_bbox_array(bbox_array)
unless bbox_array unless bbox_array
fail OSM::APIBadUserInput.new( raise OSM::APIBadUserInput.new(
"The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat") "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat")
end end
# Take an array of length 4, create a bounding box with min_lon, min_lat, max_lon and # Take an array of length 4, create a bounding box with min_lon, min_lat, max_lon and

10
lib/classic_pagination/pagination.rb
View file

@ -93,8 +93,8 @@ module ActionController
valid_options << :actions unless in_action valid_options << :actions unless in_action
unknown_option_keys = options.keys - valid_options unknown_option_keys = options.keys - valid_options
fail ActionController::ActionControllerError, raise ActionController::ActionControllerError,
"Unknown options: #{unknown_option_keys.join(', ')}" unless "Unknown options: #{unknown_option_keys.join(', ')}" unless
unknown_option_keys.empty? unknown_option_keys.empty?
options[:singular_name] ||= ActiveSupport::Inflector.singularize(collection_id.to_s) options[:singular_name] ||= ActiveSupport::Inflector.singularize(collection_id.to_s)
@ -225,7 +225,7 @@ module ActionController
# than or equal to zero). The page CGI parameter for links defaults to # than or equal to zero). The page CGI parameter for links defaults to
# "page" and can be overridden with +page_parameter+. # "page" and can be overridden with +page_parameter+.
def initialize(controller, item_count, items_per_page, current_page = 1) def initialize(controller, item_count, items_per_page, current_page = 1)
fail ArgumentError, "must have at least one item per page" if raise ArgumentError, "must have at least one item per page" if
items_per_page <= 0 items_per_page <= 0
@controller = controller @controller = controller
@ -242,7 +242,7 @@ module ActionController
# not belong to this Paginator, an ArgumentError is raised. # not belong to this Paginator, an ArgumentError is raised.
def current_page=(page) def current_page=(page)
if page.is_a? Page if page.is_a? Page
fail ArgumentError, "Page/Paginator mismatch" unless raise ArgumentError, "Page/Paginator mismatch" unless
page.paginator == self page.paginator == self
end end
page = page.to_i page = page.to_i
@ -326,7 +326,7 @@ module ActionController
# left-hand page comes after the right-hand page. Raises ArgumentError # left-hand page comes after the right-hand page. Raises ArgumentError
# if the pages do not belong to the same Paginator object. # if the pages do not belong to the same Paginator object.
def <=>(other) def <=>(other)
fail ArgumentError unless @paginator == other.paginator raise ArgumentError unless @paginator == other.paginator
@number <=> other.number @number <=> other.number
end end

22
lib/consistency_validations.rb
View file

@ -7,26 +7,26 @@ module ConsistencyValidations
# This will throw an exception if there is an inconsistency # This will throw an exception if there is an inconsistency
def check_consistency(old, new, user) def check_consistency(old, new, user)
if new.id != old.id || new.id.nil? || old.id.nil? if new.id != old.id || new.id.nil? || old.id.nil?
fail OSM::APIPreconditionFailedError.new("New and old IDs don't match on #{new.class}. #{new.id} != #{old.id}.") raise OSM::APIPreconditionFailedError.new("New and old IDs don't match on #{new.class}. #{new.id} != #{old.id}.")
elsif new.version != old.version elsif new.version != old.version
fail OSM::APIVersionMismatchError.new(new.id, new.class.to_s, new.version, old.version) raise OSM::APIVersionMismatchError.new(new.id, new.class.to_s, new.version, old.version)
elsif new.changeset.nil? elsif new.changeset.nil?
fail OSM::APIChangesetMissingError.new raise OSM::APIChangesetMissingError.new
elsif new.changeset.user_id != user.id elsif new.changeset.user_id != user.id
fail OSM::APIUserChangesetMismatchError.new raise OSM::APIUserChangesetMismatchError.new
elsif !new.changeset.is_open? elsif !new.changeset.is_open?
fail OSM::APIChangesetAlreadyClosedError.new(new.changeset) raise OSM::APIChangesetAlreadyClosedError.new(new.changeset)
end end
end end
# This is similar to above, just some validations don't apply # This is similar to above, just some validations don't apply
def check_create_consistency(new, user) def check_create_consistency(new, user)
if new.changeset.nil? if new.changeset.nil?
fail OSM::APIChangesetMissingError.new raise OSM::APIChangesetMissingError.new
elsif new.changeset.user_id != user.id elsif new.changeset.user_id != user.id
fail OSM::APIUserChangesetMismatchError.new raise OSM::APIUserChangesetMismatchError.new
elsif !new.changeset.is_open? elsif !new.changeset.is_open?
fail OSM::APIChangesetAlreadyClosedError.new(new.changeset) raise OSM::APIChangesetAlreadyClosedError.new(new.changeset)
end end
end end
@ -37,11 +37,11 @@ module ConsistencyValidations
# check user credentials - only the user who opened a changeset # check user credentials - only the user who opened a changeset
# may alter it. # may alter it.
if changeset.nil? if changeset.nil?
fail OSM::APIChangesetMissingError.new raise OSM::APIChangesetMissingError.new
elsif user.id != changeset.user_id elsif user.id != changeset.user_id
fail OSM::APIUserChangesetMismatchError.new raise OSM::APIUserChangesetMismatchError.new
elsif !changeset.is_open? elsif !changeset.is_open?
fail OSM::APIChangesetAlreadyClosedError.new(changeset) raise OSM::APIChangesetAlreadyClosedError.new(changeset)
end end
end end
end end

16
lib/diff_reader.rb
View file

@ -85,7 +85,7 @@ class DiffReader
def with_model def with_model
with_element do |model_name, _model_attributes| with_element do |model_name, _model_attributes|
model = MODELS[model_name] model = MODELS[model_name]
fail OSM::APIBadUserInput.new("Unexpected element type #{model_name}, " + raise OSM::APIBadUserInput.new("Unexpected element type #{model_name}, " +
"expected node, way or relation.") if model.nil? "expected node, way or relation.") if model.nil?
# new in libxml-ruby >= 2, expand returns an element not associated # new in libxml-ruby >= 2, expand returns an element not associated
# with a document. this means that there's no encoding parameter, # with a document. this means that there's no encoding parameter,
@ -109,9 +109,9 @@ class DiffReader
# Checks a few invariants. Others are checked in the model methods # Checks a few invariants. Others are checked in the model methods
# such as save_ and delete_with_history. # such as save_ and delete_with_history.
def check(model, xml, new) def check(model, xml, new)
fail OSM::APIBadXMLError.new(model, xml) if new.nil? raise OSM::APIBadXMLError.new(model, xml) if new.nil?
unless new.changeset_id == @changeset.id unless new.changeset_id == @changeset.id
fail OSM::APIChangesetMismatchError.new(new.changeset_id, @changeset.id) raise OSM::APIChangesetMismatchError.new(new.changeset_id, @changeset.id)
end end
end end
@ -128,7 +128,7 @@ class DiffReader
# take the first element and check that it is an osmChange element # take the first element and check that it is an osmChange element
@reader.read @reader.read
fail OSM::APIBadUserInput.new("Document element should be 'osmChange'.") if @reader.name != "osmChange" raise OSM::APIBadUserInput.new("Document element should be 'osmChange'.") if @reader.name != "osmChange"
result = OSM::API.new.get_xml_doc result = OSM::API.new.get_xml_doc
result.root.name = "diffResult" result.root.name = "diffResult"
@ -145,12 +145,12 @@ class DiffReader
# when this element is saved it will get a new ID, so we save it # when this element is saved it will get a new ID, so we save it
# to produce the mapping which is sent to other elements. # to produce the mapping which is sent to other elements.
placeholder_id = xml["id"].to_i placeholder_id = xml["id"].to_i
fail OSM::APIBadXMLError.new(model, xml) if placeholder_id.nil? raise OSM::APIBadXMLError.new(model, xml) if placeholder_id.nil?
# check if the placeholder ID has been given before and throw # check if the placeholder ID has been given before and throw
# an exception if it has - we can't create the same element twice. # an exception if it has - we can't create the same element twice.
model_sym = model.to_s.downcase.to_sym model_sym = model.to_s.downcase.to_sym
fail OSM::APIBadUserInput.new("Placeholder IDs must be unique for created elements.") if ids[model_sym].include? placeholder_id raise OSM::APIBadUserInput.new("Placeholder IDs must be unique for created elements.") if ids[model_sym].include? placeholder_id
# some elements may have placeholders for other elements in the # some elements may have placeholders for other elements in the
# diff, so we must fix these before saving the element. # diff, so we must fix these before saving the element.
@ -209,7 +209,7 @@ class DiffReader
# delete doesn't have to contain a full payload, according to # delete doesn't have to contain a full payload, according to
# the wiki docs, so we just extract the things we need. # the wiki docs, so we just extract the things we need.
new_id = xml["id"].to_i new_id = xml["id"].to_i
fail OSM::APIBadXMLError.new(model, xml, "ID attribute is required") if new_id.nil? raise OSM::APIBadXMLError.new(model, xml, "ID attribute is required") if new_id.nil?
# if the ID is a placeholder then map it to the real ID # if the ID is a placeholder then map it to the real ID
model_sym = model.to_s.downcase.to_sym model_sym = model.to_s.downcase.to_sym
@ -250,7 +250,7 @@ class DiffReader
else else
# no other actions to choose from, so it must be the users fault! # no other actions to choose from, so it must be the users fault!
fail OSM::APIChangesetActionInvalid.new(action_name) raise OSM::APIChangesetActionInvalid.new(action_name)
end end
end end

2
lib/not_redactable.rb
View file

@ -6,6 +6,6 @@ module NotRedactable
end end
def redact!(_r) def redact!(_r)
fail OSM::APICannotRedactError.new raise OSM::APICannotRedactError.new
end end
end end

6
lib/potlatch.rb
View file

@ -179,7 +179,7 @@ module Potlatch
presetcategory = "" presetcategory = ""
# StringIO.open(txt) do |file| # StringIO.open(txt) do |file|
File.open("#{Rails.root}/config/potlatch/presets.txt") do |file| File.open("#{Rails.root}/config/potlatch/presets.txt") do |file|
file.each_line do|line| file.each_line do |line|
t = line.chomp t = line.chomp
if t =~ %r{(\w+)/(\w+)} if t =~ %r{(\w+)/(\w+)}
presettype = $1 presettype = $1
@ -191,7 +191,7 @@ module Potlatch
kv = $2 kv = $2
presetnames[presettype][presetcategory].push(pre) presetnames[presettype][presetcategory].push(pre)
presets[pre] = {} presets[pre] = {}
kv.split(",").each do|a| kv.split(",").each do |a|
presets[pre][$1] = $2 if a =~ /^(.+)=(.*)$/ presets[pre][$1] = $2 if a =~ /^(.+)=(.*)$/
end end
end end
@ -243,7 +243,7 @@ module Potlatch
# Read auto-complete # Read auto-complete
autotags = { "point" => {}, "way" => {}, "POI" => {} } autotags = { "point" => {}, "way" => {}, "POI" => {} }
File.open("#{Rails.root}/config/potlatch/autocomplete.txt") do |file| File.open("#{Rails.root}/config/potlatch/autocomplete.txt") do |file|
file.each_line do|line| file.each_line do |line|
next unless line.chomp =~ %r{^([\w:]+)/(\w+)\s+(.+)$} next unless line.chomp =~ %r{^([\w:]+)/(\w+)\s+(.+)$}
tag = $1 tag = $1

2
lib/quad_tile.rb
View file

@ -72,7 +72,7 @@ module QuadTile
end end
end end
sql.push("#{prefix}tile IN (#{single.join(',')})") if single.size > 0 sql.push("#{prefix}tile IN (#{single.join(',')})") unless single.empty?
"( " + sql.join(" OR ") + " )" "( " + sql.join(" OR ") + " )"
end end

2
lib/redactable.rb
View file

@ -13,7 +13,7 @@ module Redactable
def redact!(redaction) def redact!(redaction)
# check that this version isn't the current version # check that this version isn't the current version
fail OSM::APICannotRedactError.new if is_latest_version? raise OSM::APICannotRedactError.new if is_latest_version?
# make the change # make the change
self.redaction = redaction self.redaction = redaction

6
lib/rich_text.rb
View file

@ -25,15 +25,15 @@ module RichText
doc = Nokogiri::HTML(to_html) doc = Nokogiri::HTML(to_html)
if doc.content.length > 0 if doc.content.empty?
link_proportion = 0
else
doc.xpath("//a").each do |link| doc.xpath("//a").each do |link|
link_count += 1 link_count += 1
link_size += link.content.length link_size += link.content.length
end end
link_proportion = link_size.to_f / doc.content.length.to_f link_proportion = link_size.to_f / doc.content.length.to_f
else
link_proportion = 0
end end
[link_proportion - 0.2, 0.0].max * 200 + link_count * 40 [link_proportion - 0.2, 0.0].max * 200 + link_count * 40

2
lib/tasks/add_version_to_nodes.rake
View file

@ -28,7 +28,7 @@ namespace "db" do
temp_old_node.timestamp = node.timestamp temp_old_node.timestamp = node.timestamp
temp_old_node.tile = node.tile temp_old_node.tile = node.tile
temp_old_node.version = n temp_old_node.version = n
temp_old_node.save! || fail temp_old_node.save! || raise
n += 1 n += 1
end end
end end

36
test/controllers/changeset_controller_test.rb
View file

@ -102,16 +102,16 @@ class ChangesetControllerTest < ActionController::TestCase
basic_authorization users(:normal_user).email, "test" basic_authorization users(:normal_user).email, "test"
# Create the first user's changeset # Create the first user's changeset
content "<osm><changeset>" + content "<osm><changeset>" +
"<tag k='created_by' v='osm test suite checking changesets'/>" + "<tag k='created_by' v='osm test suite checking changesets'/>" +
"</changeset></osm>" "</changeset></osm>"
put :create put :create
assert_require_public_data assert_require_public_data
basic_authorization users(:public_user).email, "test" basic_authorization users(:public_user).email, "test"
# Create the first user's changeset # Create the first user's changeset
content "<osm><changeset>" + content "<osm><changeset>" +
"<tag k='created_by' v='osm test suite checking changesets'/>" + "<tag k='created_by' v='osm test suite checking changesets'/>" +
"</changeset></osm>" "</changeset></osm>"
put :create put :create
assert_response :success, "Creation of changeset did not return sucess status" assert_response :success, "Creation of changeset did not return sucess status"
@ -527,8 +527,8 @@ EOF
# create a temporary changeset # create a temporary changeset
content "<osm><changeset>" + content "<osm><changeset>" +
"<tag k='created_by' v='osm test suite checking changesets'/>" + "<tag k='created_by' v='osm test suite checking changesets'/>" +
"</changeset></osm>" "</changeset></osm>"
assert_difference "Changeset.count", 1 do assert_difference "Changeset.count", 1 do
put :create put :create
end end
@ -1102,8 +1102,8 @@ EOF
basic_authorization users(:public_user).email, "test" basic_authorization users(:public_user).email, "test"
content "<osm><changeset>" + content "<osm><changeset>" +
"<tag k='created_by' v='osm test suite checking changesets'/>" + "<tag k='created_by' v='osm test suite checking changesets'/>" +
"</changeset></osm>" "</changeset></osm>"
put :create put :create
assert_response :success assert_response :success
changeset_id = @response.body.to_i changeset_id = @response.body.to_i
@ -1140,8 +1140,8 @@ EOF
basic_authorization users(:public_user).email, "test" basic_authorization users(:public_user).email, "test"
content "<osm><changeset>" + content "<osm><changeset>" +
"<tag k='created_by' v='osm test suite checking changesets'/>" + "<tag k='created_by' v='osm test suite checking changesets'/>" +
"</changeset></osm>" "</changeset></osm>"
put :create put :create
assert_response :success assert_response :success
changeset_id = @response.body.to_i changeset_id = @response.body.to_i
@ -1225,8 +1225,8 @@ EOF
# create a temporary changeset # create a temporary changeset
content "<osm><changeset>" + content "<osm><changeset>" +
"<tag k='created_by' v='osm test suite checking changesets'/>" + "<tag k='created_by' v='osm test suite checking changesets'/>" +
"</changeset></osm>" "</changeset></osm>"
put :create put :create
assert_response :forbidden assert_response :forbidden
@ -1235,8 +1235,8 @@ EOF
# create a temporary changeset # create a temporary changeset
content "<osm><changeset>" + content "<osm><changeset>" +
"<tag k='created_by' v='osm test suite checking changesets'/>" + "<tag k='created_by' v='osm test suite checking changesets'/>" +
"</changeset></osm>" "</changeset></osm>"
put :create put :create
assert_response :success assert_response :success
changeset_id = @response.body.to_i changeset_id = @response.body.to_i
@ -1281,8 +1281,8 @@ EOF
# create a temporary changeset # create a temporary changeset
content "<osm><changeset>" + content "<osm><changeset>" +
"<tag k='created_by' v='osm test suite checking changesets'/>" + "<tag k='created_by' v='osm test suite checking changesets'/>" +
"</changeset></osm>" "</changeset></osm>"
put :create put :create
assert_response :success assert_response :success
changeset_id = @response.body.to_i changeset_id = @response.body.to_i
@ -1340,8 +1340,8 @@ OSMFILE
# create a temporary changeset # create a temporary changeset
content "<osm><changeset>" + content "<osm><changeset>" +
"<tag k='created_by' v='osm test suite checking changesets'/>" + "<tag k='created_by' v='osm test suite checking changesets'/>" +
"</changeset></osm>" "</changeset></osm>"
put :create put :create
assert_response :success assert_response :success
changeset_id = @response.body.to_i changeset_id = @response.body.to_i

8
test/controllers/node_controller_test.rb
View file

@ -466,8 +466,8 @@ class NodeControllerTest < ActionController::TestCase
# try and put something into a string that the API might # try and put something into a string that the API might
# use unquoted and therefore allow code injection... # use unquoted and therefore allow code injection...
content "<osm><node lat='0' lon='0' changeset='#{changeset_id}'>" + content "<osm><node lat='0' lon='0' changeset='#{changeset_id}'>" +
'<tag k="#{@user.inspect}" v="0"/>' + '<tag k="#{@user.inspect}" v="0"/>' +
"</node></osm>" "</node></osm>"
put :create put :create
assert_require_public_data "Shouldn't be able to create with non-public user" assert_require_public_data "Shouldn't be able to create with non-public user"
@ -478,8 +478,8 @@ class NodeControllerTest < ActionController::TestCase
# try and put something into a string that the API might # try and put something into a string that the API might
# use unquoted and therefore allow code injection... # use unquoted and therefore allow code injection...
content "<osm><node lat='0' lon='0' changeset='#{changeset_id}'>" + content "<osm><node lat='0' lon='0' changeset='#{changeset_id}'>" +
'<tag k="#{@user.inspect}" v="0"/>' + '<tag k="#{@user.inspect}" v="0"/>' +
"</node></osm>" "</node></osm>"
put :create put :create
assert_response :success assert_response :success
nodeid = @response.body nodeid = @response.body

34
test/controllers/relation_controller_test.rb
View file

@ -165,8 +165,8 @@ class RelationControllerTest < ActionController::TestCase
# This time try with a role attribute in the relation # This time try with a role attribute in the relation
nid = current_nodes(:used_node_1).id nid = current_nodes(:used_node_1).id
content "<osm><relation changeset='#{changeset_id}'>" + content "<osm><relation changeset='#{changeset_id}'>" +
"<member ref='#{nid}' type='node' role='some'/>" + "<member ref='#{nid}' type='node' role='some'/>" +
"<tag k='test' v='yes' /></relation></osm>" "<tag k='test' v='yes' /></relation></osm>"
put :create put :create
# hope for forbidden due to user # hope for forbidden due to user
assert_response :forbidden, assert_response :forbidden,
@ -177,7 +177,7 @@ class RelationControllerTest < ActionController::TestCase
# need a role attribute to be included # need a role attribute to be included
nid = current_nodes(:used_node_1).id nid = current_nodes(:used_node_1).id
content "<osm><relation changeset='#{changeset_id}'>" + content "<osm><relation changeset='#{changeset_id}'>" +
"<member ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>" "<member ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>"
put :create put :create
# hope for forbidden due to user # hope for forbidden due to user
assert_response :forbidden, assert_response :forbidden,
@ -188,9 +188,9 @@ class RelationControllerTest < ActionController::TestCase
nid = current_nodes(:used_node_1).id nid = current_nodes(:used_node_1).id
wid = current_ways(:used_way).id wid = current_ways(:used_way).id
content "<osm><relation changeset='#{changeset_id}'>" + content "<osm><relation changeset='#{changeset_id}'>" +
"<member type='node' ref='#{nid}' role='some'/>" + "<member type='node' ref='#{nid}' role='some'/>" +
"<member type='way' ref='#{wid}' role='other'/>" + "<member type='way' ref='#{wid}' role='other'/>" +
"<tag k='test' v='yes' /></relation></osm>" "<tag k='test' v='yes' /></relation></osm>"
put :create put :create
# hope for forbidden, due to user # hope for forbidden, due to user
assert_response :forbidden, assert_response :forbidden,
@ -233,8 +233,8 @@ class RelationControllerTest < ActionController::TestCase
# This time try with a role attribute in the relation # This time try with a role attribute in the relation
nid = current_nodes(:used_node_1).id nid = current_nodes(:used_node_1).id
content "<osm><relation changeset='#{changeset_id}'>" + content "<osm><relation changeset='#{changeset_id}'>" +
"<member ref='#{nid}' type='node' role='some'/>" + "<member ref='#{nid}' type='node' role='some'/>" +
"<tag k='test' v='yes' /></relation></osm>" "<tag k='test' v='yes' /></relation></osm>"
put :create put :create
# hope for success # hope for success
assert_response :success, assert_response :success,
@ -265,7 +265,7 @@ class RelationControllerTest < ActionController::TestCase
# need a role attribute to be included # need a role attribute to be included
nid = current_nodes(:used_node_1).id nid = current_nodes(:used_node_1).id
content "<osm><relation changeset='#{changeset_id}'>" + content "<osm><relation changeset='#{changeset_id}'>" +
"<member ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>" "<member ref='#{nid}' type='node'/>" + "<tag k='test' v='yes' /></relation></osm>"
put :create put :create
# hope for success # hope for success
assert_response :success, assert_response :success,
@ -296,9 +296,9 @@ class RelationControllerTest < ActionController::TestCase
nid = current_nodes(:used_node_1).id nid = current_nodes(:used_node_1).id
wid = current_ways(:used_way).id wid = current_ways(:used_way).id
content "<osm><relation changeset='#{changeset_id}'>" + content "<osm><relation changeset='#{changeset_id}'>" +
"<member type='node' ref='#{nid}' role='some'/>" + "<member type='node' ref='#{nid}' role='some'/>" +
"<member type='way' ref='#{wid}' role='other'/>" + "<member type='way' ref='#{wid}' role='other'/>" +
"<tag k='test' v='yes' /></relation></osm>" "<tag k='test' v='yes' /></relation></osm>"
put :create put :create
# hope for success # hope for success
assert_response :success, assert_response :success,
@ -412,8 +412,8 @@ class RelationControllerTest < ActionController::TestCase
# create a relation with non-existing node as member # create a relation with non-existing node as member
content "<osm><relation changeset='#{changeset_id}'>" + content "<osm><relation changeset='#{changeset_id}'>" +
"<member type='node' ref='0'/><tag k='test' v='yes' />" + "<member type='node' ref='0'/><tag k='test' v='yes' />" +
"</relation></osm>" "</relation></osm>"
put :create put :create
# expect failure # expect failure
assert_response :precondition_failed, assert_response :precondition_failed,
@ -432,8 +432,8 @@ class RelationControllerTest < ActionController::TestCase
# create some xml that should return an error # create some xml that should return an error
content "<osm><relation changeset='#{changeset_id}'>" + content "<osm><relation changeset='#{changeset_id}'>" +
"<member type='type' ref='#{current_nodes(:used_node_1).id}' role=''/>" + "<member type='type' ref='#{current_nodes(:used_node_1).id}' role=''/>" +
"<tag k='tester' v='yep'/></relation></osm>" "<tag k='tester' v='yep'/></relation></osm>"
put :create put :create
# expect failure # expect failure
assert_response :bad_request assert_response :bad_request
@ -963,7 +963,7 @@ OSM
a_tags.each do |k, v| a_tags.each do |k, v|
assert_equal v, b_tags[k], assert_equal v, b_tags[k],
"Tags which were not altered should be the same. " + "Tags which were not altered should be the same. " +
"#{a_tags.inspect} != #{b_tags.inspect}" "#{a_tags.inspect} != #{b_tags.inspect}"
end end
end end

26
test/controllers/way_controller_test.rb
View file

@ -121,8 +121,8 @@ class WayControllerTest < ActionController::TestCase
# create a way with pre-existing nodes # create a way with pre-existing nodes
content "<osm><way changeset='#{changeset_id}'>" + content "<osm><way changeset='#{changeset_id}'>" +
"<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" + "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" +
"<tag k='test' v='yes' /></way></osm>" "<tag k='test' v='yes' /></way></osm>"
put :create put :create
# hope for failure # hope for failure
assert_response :forbidden, assert_response :forbidden,
@ -138,8 +138,8 @@ class WayControllerTest < ActionController::TestCase
# create a way with pre-existing nodes # create a way with pre-existing nodes
content "<osm><way changeset='#{changeset_id}'>" + content "<osm><way changeset='#{changeset_id}'>" +
"<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" + "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" +
"<tag k='test' v='yes' /></way></osm>" "<tag k='test' v='yes' /></way></osm>"
put :create put :create
# hope for success # hope for success
assert_response :success, assert_response :success,
@ -179,7 +179,7 @@ class WayControllerTest < ActionController::TestCase
# create a way with non-existing node # create a way with non-existing node
content "<osm><way changeset='#{open_changeset_id}'>" + content "<osm><way changeset='#{open_changeset_id}'>" +
"<nd ref='0'/><tag k='test' v='yes' /></way></osm>" "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
put :create put :create
# expect failure # expect failure
assert_response :forbidden, assert_response :forbidden,
@ -187,7 +187,7 @@ class WayControllerTest < ActionController::TestCase
# create a way with no nodes # create a way with no nodes
content "<osm><way changeset='#{open_changeset_id}'>" + content "<osm><way changeset='#{open_changeset_id}'>" +
"<tag k='test' v='yes' /></way></osm>" "<tag k='test' v='yes' /></way></osm>"
put :create put :create
# expect failure # expect failure
assert_response :forbidden, assert_response :forbidden,
@ -195,7 +195,7 @@ class WayControllerTest < ActionController::TestCase
# create a way inside a closed changeset # create a way inside a closed changeset
content "<osm><way changeset='#{closed_changeset_id}'>" + content "<osm><way changeset='#{closed_changeset_id}'>" +
"<nd ref='#{nid1}'/></way></osm>" "<nd ref='#{nid1}'/></way></osm>"
put :create put :create
# expect failure # expect failure
assert_response :forbidden, assert_response :forbidden,
@ -211,7 +211,7 @@ class WayControllerTest < ActionController::TestCase
# create a way with non-existing node # create a way with non-existing node
content "<osm><way changeset='#{open_changeset_id}'>" + content "<osm><way changeset='#{open_changeset_id}'>" +
"<nd ref='0'/><tag k='test' v='yes' /></way></osm>" "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
put :create put :create
# expect failure # expect failure
assert_response :precondition_failed, assert_response :precondition_failed,
@ -220,7 +220,7 @@ class WayControllerTest < ActionController::TestCase
# create a way with no nodes # create a way with no nodes
content "<osm><way changeset='#{open_changeset_id}'>" + content "<osm><way changeset='#{open_changeset_id}'>" +
"<tag k='test' v='yes' /></way></osm>" "<tag k='test' v='yes' /></way></osm>"
put :create put :create
# expect failure # expect failure
assert_response :precondition_failed, assert_response :precondition_failed,
@ -229,7 +229,7 @@ class WayControllerTest < ActionController::TestCase
# create a way inside a closed changeset # create a way inside a closed changeset
content "<osm><way changeset='#{closed_changeset_id}'>" + content "<osm><way changeset='#{closed_changeset_id}'>" +
"<nd ref='#{nid1}'/></way></osm>" "<nd ref='#{nid1}'/></way></osm>"
put :create put :create
# expect failure # expect failure
assert_response :conflict, assert_response :conflict,
@ -237,9 +237,9 @@ class WayControllerTest < ActionController::TestCase
# create a way with a tag which is too long # create a way with a tag which is too long
content "<osm><way changeset='#{open_changeset_id}'>" + content "<osm><way changeset='#{open_changeset_id}'>" +
"<nd ref='#{nid1}'/>" + "<nd ref='#{nid1}'/>" +
"<tag k='foo' v='#{'x' * 256}'/>" + "<tag k='foo' v='#{'x' * 256}'/>" +
"</way></osm>" "</way></osm>"
put :create put :create
# expect failure # expect failure
assert_response :bad_request, assert_response :bad_request,