From: Tom Hughes <tom@compton.nu>
Date: Tue, 17 Feb 2015 22:33:21 +0000 (+0000)
Subject: Fix rubocop style issues
X-Git-Tag: live~4812
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/5cbd4038edb32b0304bd766e70fc680ea447b52b

Fix rubocop style issues
---

diff --git a/.rubocop.yml b/.rubocop.yml
index 8187b2696..c05a1ca79 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -2,3 +2,9 @@ inherit_from: .rubocop_todo.yml
 
 Style/BracesAroundHashParameters:
   EnforcedStyle: context_dependent
+
+Style/FileName:
+  Exclude:
+    - 'script/deliver-message'
+    - 'script/locale/reload-languages'
+    - 'script/update-spam-blocks'
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index b857f5495..2817faea4 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -70,16 +70,6 @@ Style/AccessorMethodName:
 Style/AsciiComments:
   Enabled: false
 
-# Offense count: 21
-# Configuration parameters: IndentWhenRelativeTo, SupportedStyles, IndentOneStep.
-Style/CaseIndentation:
-  Enabled: false
-
-# Offense count: 3
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/ClassAndModuleChildren:
-  Enabled: false
-
 # Offense count: 9
 Style/ClassVars:
   Enabled: false
@@ -89,32 +79,10 @@ Style/ClassVars:
 Style/CommentAnnotation:
   Enabled: false
 
-# Offense count: 9
-Style/ConstantName:
-  Enabled: false
-
 # Offense count: 306
 Style/Documentation:
   Enabled: false
 
-# Offense count: 2
-Style/EachWithObject:
-  Enabled: false
-
-# Offense count: 3
-Style/EmptyElse:
-  Enabled: false
-
-# Offense count: 3
-# Configuration parameters: Exclude.
-Style/FileName:
-  Enabled: false
-
-# Offense count: 3
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/For:
-  Enabled: false
-
 # Offense count: 8
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 Style/FormatString:
@@ -136,34 +104,11 @@ Style/GuardClause:
 Style/HashSyntax:
   Enabled: false
 
-# Offense count: 41
-# Configuration parameters: MaxLineLength.
-Style/IfUnlessModifier:
-  Enabled: false
-
 # Offense count: 60
 # Cop supports --auto-correct.
 Style/LineEndConcatenation:
   Enabled: false
 
-# Offense count: 12
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/MethodName:
-  Enabled: false
-
-# Offense count: 3
-Style/MultilineTernaryOperator:
-  Enabled: false
-
-# Offense count: 1
-Style/NestedTernaryOperator:
-  Enabled: false
-
-# Offense count: 11
-# Configuration parameters: EnforcedStyle, MinBodyLength, SupportedStyles.
-Style/Next:
-  Enabled: false
-
 # Offense count: 53
 # Cop supports --auto-correct.
 Style/NumericLiterals:
@@ -173,10 +118,6 @@ Style/NumericLiterals:
 Style/OneLineConditional:
   Enabled: false
 
-# Offense count: 2
-Style/OpMethod:
-  Enabled: false
-
 # Offense count: 44
 # Cop supports --auto-correct.
 Style/PerlBackrefs:
@@ -200,16 +141,6 @@ Style/RegexpLiteral:
 Style/RescueModifier:
   Enabled: false
 
-# Offense count: 25
-# Configuration parameters: AllowAsExpressionSeparator.
-Style/Semicolon:
-  Enabled: false
-
-# Offense count: 4
-# Configuration parameters: Methods.
-Style/SingleLineBlockParams:
-  Enabled: false
-
 # Offense count: 6639
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles.
@@ -230,17 +161,3 @@ Style/StructInheritance:
 # Configuration parameters: ExactNameMatch, AllowPredicates, AllowDSLWriters, Whitelist.
 Style/TrivialAccessors:
   Enabled: false
-
-# Offense count: 10
-Style/UnlessElse:
-  Enabled: false
-
-# Offense count: 2
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-Style/VariableName:
-  Enabled: false
-
-# Offense count: 1
-# Configuration parameters: MaxLineLength.
-Style/WhileUntilModifier:
-  Enabled: false
diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb
index aa2156e8e..6f84313be 100644
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -51,17 +51,17 @@ class AmfController < ApplicationController
       logger.info("Executing AMF #{message}(#{args.join(',')})")
 
       case message
-        when 'getpresets' then        result = getpresets(*args)
-        when 'whichways' then         result = whichways(*args)
-        when 'whichways_deleted' then result = whichways_deleted(*args)
-        when 'getway' then            result = getway(args[0].to_i)
-        when 'getrelation' then       result = getrelation(args[0].to_i)
-        when 'getway_old' then        result = getway_old(args[0].to_i, args[1])
-        when 'getway_history' then    result = getway_history(args[0].to_i)
-        when 'getnode_history' then   result = getnode_history(args[0].to_i)
-        when 'findgpx' then           result = findgpx(*args)
-        when 'findrelations' then     result = findrelations(*args)
-        when 'getpoi' then            result = getpoi(*args)
+      when 'getpresets' then        result = getpresets(*args)
+      when 'whichways' then         result = whichways(*args)
+      when 'whichways_deleted' then result = whichways_deleted(*args)
+      when 'getway' then            result = getway(args[0].to_i)
+      when 'getrelation' then       result = getrelation(args[0].to_i)
+      when 'getway_old' then        result = getway_old(args[0].to_i, args[1])
+      when 'getway_history' then    result = getway_history(args[0].to_i)
+      when 'getnode_history' then   result = getnode_history(args[0].to_i)
+      when 'findgpx' then           result = findgpx(*args)
+      when 'findrelations' then     result = findrelations(*args)
+      when 'getpoi' then            result = getpoi(*args)
       end
 
       result
@@ -82,15 +82,20 @@ class AmfController < ApplicationController
         result = [-5, nil]
       else
         case message
-          when 'putway' then         orn = renumberednodes.dup
-                                     result = putway(renumberednodes, *args)
-                                     result[4] = renumberednodes.reject { |k, _v| orn.key?(k) }
-                                     if result[0] == 0 && result[2] != result[3] then renumberedways[result[2]] = result[3] end
-          when 'putrelation' then    result = putrelation(renumberednodes, renumberedways, *args)
-          when 'deleteway' then      result = deleteway(*args)
-          when 'putpoi' then         result = putpoi(*args)
-                                     if result[0] == 0 && result[2] != result[3] then renumberednodes[result[2]] = result[3] end
-          when 'startchangeset' then result = startchangeset(*args)
+        when 'putway' then
+          orn = renumberednodes.dup
+          result = putway(renumberednodes, *args)
+          result[4] = renumberednodes.reject { |k, _v| orn.key?(k) }
+          if result[0] == 0 && result[2] != result[3] then renumberedways[result[2]] = result[3] end
+        when 'putrelation' then
+          result = putrelation(renumberednodes, renumberedways, *args)
+        when 'deleteway' then
+          result = deleteway(*args)
+        when 'putpoi' then
+          result = putpoi(*args)
+          if result[0] == 0 && result[2] != result[3] then renumberednodes[result[2]] = result[3] end
+        when 'startchangeset' then
+          result = startchangeset(*args)
         end
 
         err = true if result[0] == -3  # If a conflict is detected, don't execute any more writes
@@ -249,8 +254,10 @@ class AmfController < ApplicationController
   def whichways(xmin, ymin, xmax, ymax) #:doc:
     amf_handle_error_with_timeout("'whichways'", nil, nil) do
       enlarge = [(xmax - xmin) / 8, 0.01].min
-      xmin -= enlarge; ymin -= enlarge
-      xmax += enlarge; ymax += enlarge
+      xmin -= enlarge
+      ymin -= enlarge
+      xmax += enlarge
+      ymax += enlarge
 
       # check boundary is sane and area within defined
       # see /config/application.yml
@@ -291,8 +298,10 @@ class AmfController < ApplicationController
   def whichways_deleted(xmin, ymin, xmax, ymax) #:doc:
     amf_handle_error_with_timeout("'whichways_deleted'", nil, nil) do
       enlarge = [(xmax - xmin) / 8, 0.01].min
-      xmin -= enlarge; ymin -= enlarge
-      xmax += enlarge; ymax += enlarge
+      xmin -= enlarge
+      ymin -= enlarge
+      xmax += enlarge
+      ymax += enlarge
 
       # check boundary is sane and area within defined
       # see /config/application.yml
@@ -454,8 +463,9 @@ class AmfController < ApplicationController
   def findgpx(searchterm, usertoken)
     amf_handle_error_with_timeout("'findgpx'", nil, nil) do
       user = getuser(usertoken)
-      unless user then return -1, "You must be logged in to search for GPX traces." end
-      if user.blocks.active.exists? then return -1, t('application.setup_user_auth.blocked') end
+
+      return -1, "You must be logged in to search for GPX traces." unless user
+      return -1, t('application.setup_user_auth.blocked') if user.blocks.active.exists?
 
       query = Trace.visible_to(user)
       if searchterm.to_i > 0
@@ -532,9 +542,7 @@ class AmfController < ApplicationController
       relation = nil
       Relation.transaction do
         # create a new relation, or find the existing one
-        if relid > 0
-          relation = Relation.find(relid)
-        end
+        relation = Relation.find(relid) if relid > 0
         # We always need a new node, based on the data that has been sent to us
         new_relation = Relation.new
 
@@ -633,7 +641,8 @@ class AmfController < ApplicationController
 
           if id == 0  then return -2, "Server error - node with id 0 found in way #{originalway}." end
           if lat == 90 then return -2, "Server error - node with latitude -90 found in way #{originalway}." end
-          if renumberednodes[id] then id = renumberednodes[id] end
+
+          id = renumberednodes[id]  if renumberednodes[id]
 
           node = Node.new
           node.changeset_id = changeset_id
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 5ef50eb3f..3b706966a 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -255,9 +255,7 @@ class ApplicationController < ActionController::Base
 
   def gpx_status
     status = database_status
-    if status == :online
-      status = :offline if STATUS == :gpx_offline
-    end
+    status = :offline if status == :online && STATUS == :gpx_offline
     status
   end
 
diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb
index 3f693a996..941e752f9 100644
--- a/app/controllers/changeset_controller.rb
+++ b/app/controllers/changeset_controller.rb
@@ -185,14 +185,14 @@ class ChangesetController < ApplicationController
           created = XML::Node.new "create"
           created << elt.to_xml_node(changeset_cache, user_display_name_cache)
         else
-          unless elt.visible
-            # if the element isn't visible then it must have been deleted
-            deleted = XML::Node.new "delete"
-            deleted << elt.to_xml_node(changeset_cache, user_display_name_cache)
-          else
+          if elt.visible
             # must be a modify
             modified = XML::Node.new "modify"
             modified << elt.to_xml_node(changeset_cache, user_display_name_cache)
+          else
+            # if the element isn't visible then it must have been deleted
+            deleted = XML::Node.new "delete"
+            deleted << elt.to_xml_node(changeset_cache, user_display_name_cache)
           end
         end
     end
@@ -204,9 +204,7 @@ class ChangesetController < ApplicationController
   # query changesets by bounding box, time, user or open/closed status.
   def query
     # find any bounding box
-    if params['bbox']
-      bbox = BoundingBox.from_bbox_params(params)
-    end
+    bbox = BoundingBox.from_bbox_params(params) if params['bbox']
 
     # create the conditions that the user asked for. some or all of
     # these may be nil.
@@ -244,13 +242,12 @@ class ChangesetController < ApplicationController
     changeset = Changeset.find(params[:id])
     new_changeset = Changeset.from_xml(request.raw_post)
 
-    unless new_changeset.nil?
+    if new_changeset.nil?
+      render :text => "", :status => :bad_request
+    else
       check_changeset_consistency(changeset, @user)
       changeset.update_from(new_changeset, @user)
       render :text => changeset.to_xml, :mime_type => "text/xml"
-    else
-
-      render :text => "", :status => :bad_request
     end
   end
 
@@ -473,7 +470,9 @@ class ChangesetController < ApplicationController
   ##
   # restrict changesets to those by a particular user
   def conditions_user(changesets, user, name)
-    unless user.nil? && name.nil?
+    if user.nil? && name.nil?
+      return changesets
+    else
       # 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
 
@@ -499,15 +498,15 @@ class ChangesetController < ApplicationController
         fail OSM::APINotFoundError if @user.nil? || @user.id != u.id
       end
       return changesets.where(:user_id => u.id)
-    else
-      return changesets
     end
   end
 
   ##
   # restrict changes to those closed during a particular time period
   def conditions_time(changesets, time)
-    unless time.nil?
+    if time.nil?
+      return changesets
+    else
       # if there is a range, i.e: comma separated, then the first is
       # low, second is high - same as with bounding boxes.
       if time.count(',') == 1
@@ -521,8 +520,6 @@ class ChangesetController < ApplicationController
         # if there is no comma, assume its a lower limit on time
         return changesets.where("closed_at >= ?", DateTime.parse(time))
       end
-    else
-      return changesets
     end
     # stupid DateTime seems to throw both of these for bad parsing, so
     # we have to catch both and ensure the correct code path is taken.
diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb
index 0d39cdf0a..37cdc6557 100644
--- a/app/controllers/geocoder_controller.rb
+++ b/app/controllers/geocoder_controller.rb
@@ -140,9 +140,7 @@ class GeocoderController < ApplicationController
     end
 
     # get objects to excude
-    if params[:exclude]
-      exclude = "&exclude_place_ids=#{params[:exclude]}"
-    end
+    exclude = "&exclude_place_ids=#{params[:exclude]}" if params[:exclude]
 
     # ask nominatim
     response = fetch_xml("http:#{NOMINATIM_URL}search?format=xml&q=#{escape_query(query)}#{viewbox}#{exclude}&accept-language=#{http_accept_language.user_preferred_languages.join(',')}")
diff --git a/app/controllers/oauth_controller.rb b/app/controllers/oauth_controller.rb
index 4f094b981..819f74dc1 100644
--- a/app/controllers/oauth_controller.rb
+++ b/app/controllers/oauth_controller.rb
@@ -43,7 +43,10 @@ class OauthController < ApplicationController
       return
     end
 
-    unless @token.invalidated?
+    if @token.invalidated?
+      @message = t "oauth.oauthorize_failure.invalid"
+      render :action => "authorize_failure"
+    else
       if request.post?
         if user_authorizes_token?
           @token.authorize!(current_user)
@@ -54,16 +57,21 @@ class OauthController < ApplicationController
           end
           @redirect_url = URI.parse(callback_url) unless callback_url.blank?
 
-          unless @redirect_url.to_s.blank?
-            @redirect_url.query = @redirect_url.query.blank? ?
-            "oauth_token=#{@token.token}" :
-              @redirect_url.query + "&oauth_token=#{@token.token}"
+          if @redirect_url.to_s.blank?
+            render :action => "authorize_success"
+          else
+            @redirect_url.query = if @redirect_url.query.blank?
+                                    "oauth_token=#{@token.token}"
+                                  else
+                                    @redirect_url.query +
+                                      "&oauth_token=#{@token.token}"
+                                  end
+
             unless @token.oauth10?
               @redirect_url.query += "&oauth_verifier=#{@token.verifier}"
             end
+
             redirect_to @redirect_url.to_s
-          else
-            render :action => "authorize_success"
           end
         else
           @token.invalidate!
@@ -71,9 +79,6 @@ class OauthController < ApplicationController
           render :action => "authorize_failure"
         end
       end
-    else
-      @message = t "oauth.oauthorize_failure.invalid"
-      render :action => "authorize_failure"
     end
   end
 end
diff --git a/app/controllers/old_controller.rb b/app/controllers/old_controller.rb
index 002da675c..9d7cf2113 100644
--- a/app/controllers/old_controller.rb
+++ b/app/controllers/old_controller.rb
@@ -53,16 +53,15 @@ class OldController < ApplicationController
 
   def redact
     redaction_id = params['redaction']
-    unless redaction_id.nil?
+    if redaction_id.nil?
+      # if no redaction ID was provided, then this is an unredact
+      # operation.
+      @old_element.redact!(nil)
+    else
       # if a redaction ID was specified, then set this element to
       # be redacted in that redaction.
       redaction = Redaction.find(redaction_id.to_i)
       @old_element.redact!(redaction)
-
-    else
-      # if no redaction ID was provided, then this is an unredact
-      # operation.
-      @old_element.redact!(nil)
     end
 
     # just return an empty 200 OK for success
diff --git a/app/controllers/redactions_controller.rb b/app/controllers/redactions_controller.rb
index b40c83e0d..8ba5c7d9d 100644
--- a/app/controllers/redactions_controller.rb
+++ b/app/controllers/redactions_controller.rb
@@ -52,12 +52,9 @@ class RedactionsController < ApplicationController
   end
 
   def destroy
-    unless @redaction.old_nodes.empty? &&
-           @redaction.old_ways.empty? &&
-           @redaction.old_relations.empty?
-      flash[:error] = t('redaction.destroy.not_empty')
-      redirect_to @redaction
-    else
+    if @redaction.old_nodes.empty? &&
+       @redaction.old_ways.empty? &&
+       @redaction.old_relations.empty?
       if @redaction.destroy
         flash[:notice] = t('redaction.destroy.flash')
         redirect_to :redactions
@@ -65,6 +62,9 @@ class RedactionsController < ApplicationController
         flash[:error] = t('redaction.destroy.error')
         redirect_to @redaction
       end
+    else
+      flash[:error] = t('redaction.destroy.not_empty')
+      redirect_to @redaction
     end
   end
 
diff --git a/app/controllers/relation_controller.rb b/app/controllers/relation_controller.rb
index f77f722f6..132b78e9b 100644
--- a/app/controllers/relation_controller.rb
+++ b/app/controllers/relation_controller.rb
@@ -102,24 +102,27 @@ class RelationController < ApplicationController
       user_display_name_cache = {}
 
       nodes.each do |node|
-        if node.visible? # should be unnecessary if data is consistent.
-          doc.root << node.to_xml_node(changeset_cache, user_display_name_cache)
-          visible_nodes[node.id] = node
-          visible_members["Node"][node.id] = true
-        end
+        next unless node.visible? # should be unnecessary if data is consistent.
+
+        doc.root << node.to_xml_node(changeset_cache, user_display_name_cache)
+        visible_nodes[node.id] = node
+        visible_members["Node"][node.id] = true
       end
+
       ways.each do |way|
-        if way.visible? # should be unnecessary if data is consistent.
-          doc.root << way.to_xml_node(visible_nodes, changeset_cache, user_display_name_cache)
-          visible_members["Way"][way.id] = true
-        end
+        next unless way.visible? # should be unnecessary if data is consistent.
+
+        doc.root << way.to_xml_node(visible_nodes, changeset_cache, user_display_name_cache)
+        visible_members["Way"][way.id] = true
       end
+
       relations.each do |rel|
-        if rel.visible? # should be unnecessary if data is consistent.
-          doc.root << rel.to_xml_node(nil, changeset_cache, user_display_name_cache)
-          visible_members["Relation"][rel.id] = true
-        end
+        next unless rel.visible? # should be unnecessary if data is consistent.
+
+        doc.root << rel.to_xml_node(nil, changeset_cache, user_display_name_cache)
+        visible_members["Relation"][rel.id] = true
       end
+
       # finally add self and output
       doc.root << relation.to_xml_node(visible_members, changeset_cache, user_display_name_cache)
       render :text => doc.to_s, :content_type => "text/xml"
diff --git a/app/controllers/site_controller.rb b/app/controllers/site_controller.rb
index a133e7b27..01cdcf23f 100644
--- a/app/controllers/site_controller.rb
+++ b/app/controllers/site_controller.rb
@@ -11,7 +11,7 @@ class SiteController < ApplicationController
 
   def index
     unless STATUS == :database_readonly || STATUS == :database_offline
-      session[:location] ||= OSM::IPLocation(request.env['REMOTE_ADDR'])
+      session[:location] ||= OSM.ip_location(request.env['REMOTE_ADDR'])
     end
   end
 
@@ -47,10 +47,7 @@ class SiteController < ApplicationController
     end
 
     new_params[:anchor] = "map=#{zoom}/#{lat}/#{lon}"
-
-    if params.key? :layers
-      new_params[:anchor] += "&layers=#{params[:layers]}"
-    end
+    new_params[:anchor] += "&layers=#{params[:layers]}" if params.key? :layers
 
     redirect_to Hash[new_params]
   end
diff --git a/app/controllers/swf_controller.rb b/app/controllers/swf_controller.rb
index 92a70457e..af5afb7c9 100644
--- a/app/controllers/swf_controller.rb
+++ b/app/controllers/swf_controller.rb
@@ -31,7 +31,7 @@ class SwfController < ApplicationController
     bounds_top = 240 * 20
 
     m = ''
-    m += swfRecord(9, 255.chr + 155.chr + 155.chr)			# Background
+    m += swf_record(9, 255.chr + 155.chr + 155.chr)			# Background
     absx = 0
     absy = 0
     xl = yb = 9999999
@@ -53,42 +53,44 @@ class SwfController < ApplicationController
 
     # - Draw GPS trace lines
 
-    r = startShape
+    r = start_shape
     gpslist.each do |row|
       xs = (long2coord(row['lon'].to_f, baselong, masterscale) * 20).floor
       ys = (lat2coord(row['lat'].to_f, basey, masterscale) * 20).floor
-      xl = [xs, xl].min; xr = [xs, xr].max
-      yb = [ys, yb].min; yt = [ys, yt].max
+      xl = [xs, xl].min
+      xr = [xs, xr].max
+      yb = [ys, yb].min
+      yt = [ys, yt].max
       if row['ts'].to_i - lasttime > 180 || row['fileid'] != lastfile || row['trackid'] != lasttrack # or row['ts'].to_i==lasttime
-        b += startAndMove(xs, ys, '01')
-        absx = xs.floor; absy = ys.floor
+        b += start_and_move(xs, ys, '01')
+        absx = xs.floor
+        absy = ys.floor
       end
-      b += drawTo(absx, absy, xs, ys)
-      absx = xs.floor; absy = ys.floor
+      b += draw_to(absx, absy, xs, ys)
+      absx = xs.floor
+      absy = ys.floor
       lasttime = row['ts'].to_i
       lastfile = row['fileid']
       lasttrack = row['trackid']
-      while b.length > 80
-        r += [b.slice!(0...80)].pack("B*")
-      end
+      r += [b.slice!(0...80)].pack("B*") while b.length > 80
     end
 
     #   (Unwayed segments removed)
 
     # - Write shape
 
-    b += endShape
+    b += end_shape
     r += [b].pack("B*")
-    m += swfRecord(2, packUI16(1) + packRect(xl, xr, yb, yt) + r)
-    m += swfRecord(4, packUI16(1) + packUI16(1))
+    m += swf_record(2, pack_u16(1) + pack_rect(xl, xr, yb, yt) + r)
+    m += swf_record(4, pack_u16(1) + pack_u16(1))
 
     # -	Create Flash header and write to browser
 
-    m += swfRecord(1, '')									# Show frame
-    m += swfRecord(0, '')									# End
+    m += swf_record(1, '')									# Show frame
+    m += swf_record(0, '')									# End
 
-    m = packRect(bounds_left, bounds_right, bounds_bottom, bounds_top) + 0.chr + 12.chr + packUI16(1) + m
-    m = 'FWS' + 6.chr + packUI32(m.length + 8) + m
+    m = pack_rect(bounds_left, bounds_right, bounds_bottom, bounds_top) + 0.chr + 12.chr + pack_u16(1) + m
+    m = 'FWS' + 6.chr + pack_u32(m.length + 8) + m
 
     render :text => m, :content_type => "application/x-shockwave-flash"
   end
@@ -101,28 +103,28 @@ class SwfController < ApplicationController
   # -----------------------------------------------------------------------
   # Line-drawing
 
-  def startShape
+  def start_shape
     s = 0.chr						# No fill styles
     s += 2.chr						# Two line styles
-    s += packUI16(0) + 0.chr + 255.chr + 255.chr	# Width 5, RGB #00FFFF
-    s += packUI16(0) + 255.chr + 0.chr + 255.chr	# Width 5, RGB #FF00FF
+    s += pack_u16(0) + 0.chr + 255.chr + 255.chr	# Width 5, RGB #00FFFF
+    s += pack_u16(0) + 255.chr + 0.chr + 255.chr	# Width 5, RGB #FF00FF
     s += 34.chr										# 2 fill, 2 line index bits
     s
   end
 
-  def endShape
+  def end_shape
     '000000'
   end
 
-  def startAndMove(x, y, col)
+  def start_and_move(x, y, col)
     d = '001001'					# Line style change, moveTo
-    l = [lengthSB(x), lengthSB(y)].max
+    l = [length_sb(x), length_sb(y)].max
     d += sprintf("%05b%0#{l}b%0#{l}b", l, x, y)
     d += col						# Select line style
     d
   end
 
-  def drawTo(absx, absy, x, y)
+  def draw_to(absx, absy, x, y)
     dx = x - absx
     dy = y - absy
 
@@ -133,18 +135,18 @@ class SwfController < ApplicationController
     ystep = dy / mstep
     d = ''
     1.upto(mstep).each do
-      d += drawSection(x, y, x + xstep, y + ystep)
+      d += draw_section(x, y, x + xstep, y + ystep)
       x += xstep
       y += ystep
     end
     d
   end
 
-  def drawSection(x1, y1, x2, y2)
+  def draw_section(x1, y1, x2, y2)
     d = '11'											# TypeFlag, EdgeFlag
     dx = x2 - x1
     dy = y2 - y1
-    l = [lengthSB(dx), lengthSB(dy)].max
+    l = [length_sb(dx), length_sb(dy)].max
     d += sprintf("%04b", l - 2)
     d += '1'											# GeneralLine
     d += sprintf("%0#{l}b%0#{l}b", dx, dy)
@@ -156,23 +158,23 @@ class SwfController < ApplicationController
 
   # SWF data block type
 
-  def swfRecord(id, r)
+  def swf_record(id, r)
     if r.length > 62
       # Long header: tag id, 0x3F, length
-      return packUI16((id << 6) + 0x3F) + packUI32(r.length) + r
+      return pack_u16((id << 6) + 0x3F) + pack_u32(r.length) + r
     else
       # Short header: tag id, length
-      return packUI16((id << 6) + r.length) + r
+      return pack_u16((id << 6) + r.length) + r
     end
   end
 
   # SWF RECT type
 
-  def packRect(a, b, c, d)
-    l = [lengthSB(a),
-         lengthSB(b),
-         lengthSB(c),
-         lengthSB(d)].max
+  def pack_rect(a, b, c, d)
+    l = [length_sb(a),
+         length_sb(b),
+         length_sb(c),
+         length_sb(d)].max
     # create binary string (00111001 etc.) - 5-byte length, then bbox
     n = sprintf("%05b%0#{l}b%0#{l}b%0#{l}b%0#{l}b", l, a, b, c, d)
     # pack into byte string
@@ -182,17 +184,17 @@ class SwfController < ApplicationController
   # -----------------------------------------------------------------------
   # Generic pack functions
 
-  def packUI16(n)
+  def pack_u16(n)
     [n.floor].pack("v")
   end
 
-  def packUI32(n)
+  def pack_u32(n)
     [n.floor].pack("V")
   end
 
   # Find number of bits required to store arbitrary-length binary
 
-  def lengthSB(n)
+  def length_sb(n)
     Math.frexp(n + (n == 0 ? 1 : 0))[1] + 1
   end
 
diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb
index 1b3f6355c..538cf10d2 100644
--- a/app/controllers/trace_controller.rb
+++ b/app/controllers/trace_controller.rb
@@ -59,9 +59,7 @@ class TraceController < ApplicationController
       end
     end
 
-    if params[:tag]
-      @traces = @traces.tagged(params[:tag])
-    end
+    @traces = @traces.tagged(params[:tag]) if params[:tag]
 
     @page = (params[:page] || 1).to_i
     @page_size = 20
@@ -212,10 +210,7 @@ class TraceController < ApplicationController
       @traces = @traces.joins(:user).where(:users => { :display_name => params[:display_name] })
     end
 
-    if params[:tag]
-      @traces = @traces.tagged(params[:tag])
-    end
-
+    @traces = @traces.tagged(params[:tag]) if params[:tag]
     @traces = @traces.order("timestamp DESC")
     @traces = @traces.limit(20)
     @traces = @traces.includes(:user)
diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index a8bbdee8a..70fa4f7fc 100644
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -20,7 +20,7 @@ class UserController < ApplicationController
   before_filter :lookup_user_by_name, :only => [:set_status, :delete]
 
   def terms
-    @legale = params[:legale] || OSM.IPToCountry(request.remote_ip) || DEFAULT_LEGALE
+    @legale = params[:legale] || OSM.ip_to_country(request.remote_ip) || DEFAULT_LEGALE
     @text = OSM.legal_text_for_country(@legale)
 
     if request.xhr?
@@ -61,9 +61,8 @@ class UserController < ApplicationController
         @user.consider_pd = params[:user][:consider_pd]
         @user.terms_agreed = Time.now.getutc
         @user.terms_seen = true
-        if @user.save
-          flash[:notice] = t 'user.new.terms accepted'
-        end
+
+        flash[:notice] = t 'user.new.terms accepted' if @user.save
       end
 
       if params[:referer]
@@ -158,9 +157,7 @@ class UserController < ApplicationController
       if user.nil?
         users = User.visible.where("LOWER(email) = LOWER(?)", params[:user][:email])
 
-        if users.count == 1
-          user = users.first
-        end
+        user = users.first if users.count == 1
       end
 
       if user
@@ -294,9 +291,7 @@ class UserController < ApplicationController
     if params[:session] == request.session_options[:id]
       if session[:token]
         token = UserToken.find_by_token(session[:token])
-        if token
-          token.destroy
-        end
+        token.destroy if token
         session.delete(:token)
       end
       session.delete(:user)
@@ -346,9 +341,8 @@ class UserController < ApplicationController
       end
     else
       user = User.find_by_display_name(params[:display_name])
-      if !user || user.active?
-        redirect_to root_path
-      end
+
+      redirect_to root_path if !user || user.active?
     end
   end
 
@@ -422,15 +416,15 @@ class UserController < ApplicationController
         friend = Friend.new
         friend.user_id = @user.id
         friend.friend_user_id = @new_friend.id
-        unless @user.is_friends_with?(@new_friend)
+        if @user.is_friends_with?(@new_friend)
+          flash[:warning] = t 'user.make_friend.already_a_friend', :name => @new_friend.display_name
+        else
           if friend.save
             flash[:notice] = t 'user.make_friend.success', :name => @new_friend.display_name
             Notifier.friend_notification(friend).deliver_now
           else
             friend.add_error(t('user.make_friend.failed', :name => @new_friend.display_name))
           end
-        else
-          flash[:warning] = t 'user.make_friend.already_a_friend', :name => @new_friend.display_name
         end
 
         if params[:referer]
@@ -542,14 +536,14 @@ class UserController < ApplicationController
         # provider do we know the unique address for the user.
         if user = User.find_by_openid_url(identity_url)
           case user.status
-            when "pending" then
-              unconfirmed_login(user)
-            when "active", "confirmed" then
-              successful_login(user)
-            when "suspended" then
-              failed_login t('user.login.account is suspended', :webmaster => "mailto:webmaster@openstreetmap.org")
-            else
-              failed_login t('user.login.auth failure')
+          when "pending" then
+            unconfirmed_login(user)
+          when "active", "confirmed" then
+            successful_login(user)
+          when "suspended" then
+            failed_login t('user.login.account is suspended', :webmaster => "mailto:webmaster@openstreetmap.org")
+          else
+            failed_login t('user.login.auth failure')
           end
         else
           # Guard against not getting any extension data
diff --git a/app/controllers/user_preference_controller.rb b/app/controllers/user_preference_controller.rb
index 03cb8f191..60463967c 100644
--- a/app/controllers/user_preference_controller.rb
+++ b/app/controllers/user_preference_controller.rb
@@ -33,9 +33,8 @@ class UserPreferenceController < ApplicationController
 
   # update the entire set of preferences
   def update
-    old_preferences = @user.preferences.reduce({}) do |preferences, preference|
+    old_preferences = @user.preferences.each_with_object({}) do |preference, preferences|
       preferences[preference.k] = preference
-      preferences
     end
 
     new_preferences = {}
diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb
index 45240e082..e9aa6e4e8 100644
--- a/app/helpers/browse_helper.rb
+++ b/app/helpers/browse_helper.rb
@@ -164,8 +164,8 @@ module BrowseHelper
 
     # remove all whitespace instead of encoding it http://tools.ietf.org/html/rfc3966#section-5.1.1
     # "+1 (234) 567-8901 " -> "+1(234)567-8901"
-    valueNoWhitespace = value.gsub(/\s+/, '')
+    value_no_whitespace = value.gsub(/\s+/, '')
 
-    "tel:#{valueNoWhitespace}"
+    "tel:#{value_no_whitespace}"
   end
 end
diff --git a/app/helpers/user_roles_helper.rb b/app/helpers/user_roles_helper.rb
index adc5b22e0..690cccdca 100644
--- a/app/helpers/user_roles_helper.rb
+++ b/app/helpers/user_roles_helper.rb
@@ -1,6 +1,6 @@
 module UserRolesHelper
   def role_icons(user)
-    UserRole::ALL_ROLES.reduce("".html_safe) { |s, r| s + " " + role_icon(user, r) }
+    UserRole::ALL_ROLES.reduce("".html_safe) { |a, e| a + " " + role_icon(user, e) }
   end
 
   def role_icon(user, role)
@@ -26,10 +26,7 @@ module UserRolesHelper
 
     if image
       icon = image_tag(image, :size => "20x20", :border => 0, :alt => alt, :title => title)
-
-      if url
-        icon = link_to(icon, url, :method => :post, :confirm => confirm)
-      end
+      icon = link_to(icon, url, :method => :post, :confirm => confirm) if url
     end
 
     icon
diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index 2001c70be..89aaa262d 100644
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -50,9 +50,7 @@ class Changeset < ActiveRecord::Base
   end
 
   def set_closed_time_now
-    if is_open?
-      self.closed_at = Time.now.getutc
-    end
+    self.closed_at = Time.now.getutc if is_open?
   end
 
   def self.from_xml(xml, create = false)
@@ -212,9 +210,7 @@ class Changeset < ActiveRecord::Base
     el1['closed_at'] = closed_at.xmlschema unless is_open?
     el1['open'] = is_open?.to_s
 
-    if bbox.complete?
-      bbox.to_unscaled.add_bounds_to(el1, '_')
-    end
+    bbox.to_unscaled.add_bounds_to(el1, '_') if bbox.complete?
 
     el1['comments_count'] = comments.count.to_s
 
@@ -246,14 +242,10 @@ class Changeset < ActiveRecord::Base
   # bounding box, only the tags of the changeset.
   def update_from(other, user)
     # ensure that only the user who opened the changeset may modify it.
-    unless user.id == user_id
-      fail OSM::APIUserChangesetMismatchError.new
-    end
+    fail OSM::APIUserChangesetMismatchError.new unless user.id == user_id
 
     # can't change a closed changeset
-    unless is_open?
-      fail OSM::APIChangesetAlreadyClosedError.new(self)
-    end
+    fail OSM::APIChangesetAlreadyClosedError.new(self) unless is_open?
 
     # copy the other's tags
     self.tags = other.tags
diff --git a/app/models/client_application.rb b/app/models/client_application.rb
index b1d3d945e..34856f82b 100644
--- a/app/models/client_application.rb
+++ b/app/models/client_application.rb
@@ -19,11 +19,7 @@ class ClientApplication < ActiveRecord::Base
 
   def self.find_token(token_key)
     token = OauthToken.find_by_token(token_key, :include => :client_application)
-    if token && token.authorized?
-      token
-    else
-      nil
-    end
+    token if token && token.authorized?
   end
 
   def self.verify_request(request, options = {}, &block)
diff --git a/app/models/node.rb b/app/models/node.rb
index 7d2219d75..19210b8ac 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -104,9 +104,7 @@ class Node < ActiveRecord::Base
 
   # Should probably be renamed delete_from to come in line with update
   def delete_with_history!(new_node, user)
-    unless visible
-      fail OSM::APIAlreadyDeletedError.new("node", new_node.id)
-    end
+    fail OSM::APIAlreadyDeletedError.new("node", new_node.id) unless visible
 
     # need to start the transaction here, so that the database can
     # provide repeatable reads for the used-by checks. this means it
diff --git a/app/models/old_way.rb b/app/models/old_way.rb
index c088ee129..3447cf6bb 100644
--- a/app/models/old_way.rb
+++ b/app/models/old_way.rb
@@ -100,10 +100,14 @@ class OldWay < ActiveRecord::Base
     nds.each do |n|
       oldnode = OldNode.where('node_id = ? AND timestamp <= ?', n, timestamp).unredacted.order("timestamp DESC").first
       curnode = Node.find(n)
-      id = n; reuse = curnode.visible
+      id = n
+      reuse = curnode.visible
       if oldnode.lat != curnode.lat || oldnode.lon != curnode.lon || oldnode.tags != curnode.tags
         # node has changed: if it's in other ways, give it a new id
-        if curnode.ways - [way_id] then id = -1; reuse = false end
+        if curnode.ways - [way_id]
+          id = -1
+          reuse = false
+        end
       end
       points << [oldnode.lon, oldnode.lat, id, curnode.version, oldnode.tags_as_hash, reuse]
     end
diff --git a/app/models/relation.rb b/app/models/relation.rb
index 8f2d8b241..39c1ad834 100644
--- a/app/models/relation.rb
+++ b/app/models/relation.rb
@@ -112,24 +112,22 @@ class Relation < ActiveRecord::Base
 
     relation_members.each do |member|
       p = 0
+
       if visible_members
         # if there is a list of visible members then use that to weed out deleted segments
-        if visible_members[member.member_type][member.member_id]
-          p = 1
-        end
+        p = 1 if visible_members[member.member_type][member.member_id]
       else
         # otherwise, manually go to the db to check things
-        if member.member.visible?
-          p = 1
-        end
-      end
-      if p
-        member_el = XML::Node.new 'member'
-        member_el['type'] = member.member_type.downcase
-        member_el['ref'] = member.member_id.to_s
-        member_el['role'] = member.member_role
-        el << member_el
+        p = 1 if member.member.visible?
       end
+
+      next unless p
+
+      member_el = XML::Node.new 'member'
+      member_el['type'] = member.member_type.downcase
+      member_el['ref'] = member.member_id.to_s
+      member_el['role'] = member.member_role
+      el << member_el
     end
 
     add_tags_to_xml_node(el, relation_tags)
@@ -243,19 +241,20 @@ class Relation < ActiveRecord::Base
       # find the hash for the element type or die
       hash = elements[m[0].downcase.to_sym]
       return false unless hash
+
       # unless its in the cache already
-      unless hash.key? m[1]
-        # use reflection to look up the appropriate class
-        model = Kernel.const_get(m[0].capitalize)
-        # get the element with that ID
-        element = model.where(:id => m[1]).first
-
-        # and check that it is OK to use.
-        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]}")
-        end
-        hash[m[1]] = true
+      next if hash.key? m[1]
+
+      # use reflection to look up the appropriate class
+      model = Kernel.const_get(m[0].capitalize)
+      # get the element with that ID
+      element = model.where(:id => m[1]).first
+
+      # and check that it is OK to use.
+      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]}")
       end
+      hash[m[1]] = true
     end
 
     true
@@ -375,7 +374,7 @@ class Relation < ActiveRecord::Base
       # materially change the rest of the relation.
       any_relations =
         changed_members.collect { |_id, type| type == "relation" }
-        .inject(false) { |b, s| b || s }
+        .inject(false) { |a, e| a || e }
 
       update_members = if tags_changed || any_relations
                          # add all non-relation bounding boxes to the changeset
@@ -386,9 +385,7 @@ class Relation < ActiveRecord::Base
                          changed_members
                        end
       update_members.each do |type, id, _role|
-        if type != "Relation"
-          update_changeset_element(type, id)
-        end
+        update_changeset_element(type, id) if type != "Relation"
       end
 
       # tell the changeset we updated one element only
diff --git a/app/models/user.rb b/app/models/user.rb
index f6d7ee78d..132f0de21 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -65,9 +65,7 @@ class User < ActiveRecord::Base
       if user.nil?
         users = where("LOWER(email) = LOWER(?) OR LOWER(display_name) = LOWER(?)", options[:username], options[:username])
 
-        if users.count == 1
-          user = users.first
-        end
+        user = users.first if users.count == 1
       end
 
       if user && PasswordHash.check(user.pass_crypt, user.pass_salt, options[:password])
@@ -211,8 +209,8 @@ class User < ActiveRecord::Base
   def spam_score
     changeset_score = changesets.size * 50
     trace_score = traces.size * 50
-    diary_entry_score = diary_entries.inject(0) { |s, e| s + e.body.spam_score }
-    diary_comment_score = diary_comments.inject(0) { |s, c| s + c.body.spam_score }
+    diary_entry_score = diary_entries.inject(0) { |a, e| a + e.body.spam_score }
+    diary_comment_score = diary_comments.inject(0) { |a, e| a + e.body.spam_score }
 
     score = description.spam_score / 4.0
     score += diary_entries.where("created_at > ?", 1.day.ago).count * 10
diff --git a/app/models/way.rb b/app/models/way.rb
index e61661a70..2d427051c 100644
--- a/app/models/way.rb
+++ b/app/models/way.rb
@@ -121,11 +121,11 @@ class Way < ActiveRecord::Base
     end
 
     ordered_nodes.each do |nd_id|
-      if nd_id && nd_id != '0'
-        node_el = XML::Node.new 'nd'
-        node_el['ref'] = nd_id
-        el << node_el
-      end
+      next unless nd_id && nd_id != '0'
+
+      node_el = XML::Node.new 'nd'
+      node_el['ref'] = nd_id
+      el << node_el
     end
 
     add_tags_to_xml_node(el, way_tags)
@@ -219,9 +219,7 @@ class Way < ActiveRecord::Base
   end
 
   def delete_with_history!(new_way, user)
-    unless visible
-      fail OSM::APIAlreadyDeletedError.new("way", new_way.id)
-    end
+    fail OSM::APIAlreadyDeletedError.new("way", new_way.id) unless visible
 
     # need to start the transaction here, so that the database can
     # provide repeatable reads for the used-by checks. this means it
diff --git a/config/application.rb b/config/application.rb
index 652721e21..12d777c24 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -39,13 +39,9 @@ module OpenStreetMap
     # Use SQL instead of Active Record's schema dumper when creating the database.
     # This is necessary if your schema can't be completely dumped by the schema dumper,
     # like if you have constraints or database-specific column types
-    unless STATUS == :database_offline
-      config.active_record.schema_format = :sql
-    end
+    config.active_record.schema_format = :sql unless STATUS == :database_offline
 
     # Don't eager load models when the database is offline
-    if STATUS == :database_offline
-      config.paths["app/models"].skip_eager_load!
-    end
+    config.paths["app/models"].skip_eager_load! if STATUS == :database_offline
   end
 end
diff --git a/config/environments/production.rb b/config/environments/production.rb
index 1dab35563..3c6c21cef 100644
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@@ -52,9 +52,7 @@ OpenStreetMap::Application.configure do
   # config.log_tags = [ :subdomain, :uuid ]
 
   # Use a different log path in production.
-  if defined?(LOG_PATH)
-    config.paths["log"] = LOG_PATH
-  end
+  config.paths["log"] = LOG_PATH if defined?(LOG_PATH)
 
   # Use a different logger for distributed setups.
   # config.logger = ActiveSupport::TaggedLogging.new(SyslogLogger.new)
diff --git a/config/initializers/memory_limits.rb b/config/initializers/memory_limits.rb
index 199bda5c6..b58c1f28e 100644
--- a/config/initializers/memory_limits.rb
+++ b/config/initializers/memory_limits.rb
@@ -17,9 +17,7 @@ if defined?(SOFT_MEMORY_LIMIT) && defined?(PhusionPassenger)
       status, headers, body = @app.call(env)
 
       # Restart if we've hit our memory limit
-      if resident_size > SOFT_MEMORY_LIMIT
-        Process.kill("USR1", Process.pid)
-      end
+      Process.kill("USR1", Process.pid) if resident_size > SOFT_MEMORY_LIMIT
 
       # Return the result of this request
       [status, headers, body]
diff --git a/config/initializers/oauth.rb b/config/initializers/oauth.rb
index 56dd9ff25..fa9685d70 100644
--- a/config/initializers/oauth.rb
+++ b/config/initializers/oauth.rb
@@ -2,10 +2,12 @@ require 'oauth/rack/oauth_filter'
 
 Rails.configuration.middleware.use OAuth::Rack::OAuthFilter
 
-module OAuth::RequestProxy
-  class RackRequest
-    def method
-      request.request_method
+module OAuth
+  module RequestProxy
+    class RackRequest
+      def method
+        request.request_method
+      end
     end
   end
 end
diff --git a/config/preinitializer.rb b/config/preinitializer.rb
index 818146c21..34a9a3858 100644
--- a/config/preinitializer.rb
+++ b/config/preinitializer.rb
@@ -15,7 +15,5 @@ ENV.each do |key, value|
 end
 
 config[env].each do |key, value|
-  unless Object.const_defined?(key.upcase)
-    Object.const_set(key.upcase, value)
-  end
+  Object.const_set(key.upcase, value) unless Object.const_defined?(key.upcase)
 end
diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb
index e78434263..a8ae1d99c 100644
--- a/lib/bounding_box.rb
+++ b/lib/bounding_box.rb
@@ -16,11 +16,7 @@ class BoundingBox
   end
 
   def self.from_s(s)
-    if s.count(',') == 3
-      BoundingBox.new(*s.split(/,/))
-    else
-      nil
-    end
+    BoundingBox.new(*s.split(/,/)) if s.count(',') == 3
   end
 
   def self.from_bbox_params(params)
diff --git a/lib/classic_pagination/pagination.rb b/lib/classic_pagination/pagination.rb
index 7a7177984..3fa1109f0 100644
--- a/lib/classic_pagination/pagination.rb
+++ b/lib/classic_pagination/pagination.rb
@@ -57,7 +57,9 @@ module ActionController
   # Paginator#to_sql to retrieve <tt>@people</tt> from the model.
   #
   module Pagination
-    unless const_defined?(:OPTIONS)
+    if const_defined?(:OPTIONS)
+      DEFAULT_OPTIONS[:group] = nil
+    else
       # A hash holding options for controllers using macro-style pagination
       OPTIONS = {}
 
@@ -77,8 +79,6 @@ module ActionController
         :group      => nil,
         :parameter  => 'page'
       }
-    else
-      DEFAULT_OPTIONS[:group] = nil
     end
 
     def self.included(base) #:nodoc:
@@ -195,10 +195,7 @@ module ActionController
       collection = collection.order(options[:order_by] || options[:order])
       collection = collection.includes(options[:include])
       collection = collection.group(options[:group])
-
-      if options[:select]
-        collection = collection.select(options[:select])
-      end
+      collection = collection.select(options[:select]) if options[:select]
 
       collection.offset(paginator.current.offset).limit(options[:per_page])
     end
@@ -273,8 +270,12 @@ module ActionController
 
       # Returns the number of pages in this paginator.
       def page_count
-        @page_count ||= @item_count.zero? ? 1 :
-                          (q, r = @item_count.divmod(@items_per_page); r == 0 ? q : q + 1)
+        @page_count ||= if @item_count.zero?
+                          1
+                        else
+                          q, r = @item_count.divmod(@items_per_page)
+                          r == 0 ? q : q + 1
+                        end
       end
 
       alias_method :length, :page_count
@@ -315,19 +316,19 @@ module ActionController
         # Compares two Page objects and returns true when they represent the
         # same page (i.e., their paginators are the same and they have the
         # same page number).
-        def ==(page)
-          return false if page.nil?
-          @paginator == page.paginator &&
-            @number == page.number
+        def ==(other)
+          return false if other.nil?
+          @paginator == other.paginator &&
+            @number == other.number
         end
 
         # Compares two Page objects and returns -1 if the left-hand page comes
         # before the right-hand page, 0 if the pages are equal, and 1 if the
         # left-hand page comes after the right-hand page. Raises ArgumentError
         # if the pages do not belong to the same Paginator object.
-        def <=>(page)
-          fail ArgumentError unless @paginator == page.paginator
-          @number <=> page.number
+        def <=>(other)
+          fail ArgumentError unless @paginator == other.paginator
+          @number <=> other.number
         end
 
         # Returns the item offset for the first item in this page.
@@ -399,10 +400,16 @@ module ActionController
         def padding=(padding)
           @padding = padding < 0 ? 0 : padding
           # Find the beginning and end pages of the window
-          @first = @paginator.has_page_number?(@page.number - @padding) ?
-            @paginator[@page.number - @padding] : @paginator.first
-          @last =  @paginator.has_page_number?(@page.number + @padding) ?
-            @paginator[@page.number + @padding] : @paginator.last
+          @first = if @paginator.has_page_number?(@page.number - @padding)
+                     @paginator[@page.number - @padding]
+                   else
+                     @paginator.first
+                   end
+          @last = if @paginator.has_page_number?(@page.number + @padding)
+                    @paginator[@page.number + @padding]
+                  else
+                    @paginator.last
+                  end
         end
         attr_reader :padding, :first, :last
 
diff --git a/lib/daemons/gpx_import_ctl b/lib/daemons/gpx_import_ctl
index fd06863fa..fc81e7549 100755
--- a/lib/daemons/gpx_import_ctl
+++ b/lib/daemons/gpx_import_ctl
@@ -6,7 +6,8 @@ require 'erb'
 
 class Hash
   def with_symbols!
-    keys.each { |key| self[key.to_s.to_sym] = self[key] }; self
+    keys.each { |key| self[key.to_s.to_sym] = self[key] }
+    self
   end
 end
 
diff --git a/lib/gpx.rb b/lib/gpx.rb
index ae9af0d46..5a61b344a 100644
--- a/lib/gpx.rb
+++ b/lib/gpx.rb
@@ -94,9 +94,7 @@ module GPX
         end
 
         m += 1
-        if m > num_points.to_f / frames.to_f * (mm + 1)
-          mm += 1
-        end
+        mm += 1 if m > num_points.to_f / frames.to_f * (mm + 1)
 
         oldpy = py
         oldpx = px
diff --git a/lib/osm.rb b/lib/osm.rb
index 6a41a57b6..89333763f 100644
--- a/lib/osm.rb
+++ b/lib/osm.rb
@@ -493,11 +493,11 @@ module OSM
     end
   end
 
-  def self.IPToCountry(ip_address)
+  def self.ip_to_country(ip_address)
     Timer.timeout(4) do
       ipinfo = Quova::IpInfo.new(ip_address)
 
-      if ipinfo.status == Quova::Success
+      if ipinfo.status == Quova::SUCCESS
         country = ipinfo.country_code
       else
         Net::HTTP.start('api.hostip.info') do |http|
@@ -514,8 +514,8 @@ module OSM
     return nil
   end
 
-  def self.IPLocation(ip_address)
-    code = OSM.IPToCountry(ip_address)
+  def self.ip_location(ip_address)
+    code = OSM.ip_to_country(ip_address)
 
     if code && country = Country.find_by_code(code)
       return { :minlon => country.min_lon, :minlat => country.min_lat, :maxlon => country.max_lon, :maxlat => country.max_lat }
diff --git a/lib/output_compression/output_compression.rb b/lib/output_compression/output_compression.rb
index 2c5dd6ca5..df8e55cb6 100644
--- a/lib/output_compression/output_compression.rb
+++ b/lib/output_compression/output_compression.rb
@@ -61,6 +61,8 @@ module CompressionSystem
   end
 end
 
-class ActionController::Base
-  include CompressionSystem
+module ActionController
+  class Base
+    include CompressionSystem
+  end
 end
diff --git a/lib/potlatch.rb b/lib/potlatch.rb
index 80d4d7225..879972e03 100644
--- a/lib/potlatch.rb
+++ b/lib/potlatch.rb
@@ -30,19 +30,16 @@ module Potlatch
 
     # Return numeric array
     def self.getarray(s)
-      len = getlong(s)
-      arr = []
-      for i in (0..len - 1)
-        arr[i] = getvalue(s)
+      getlong(s).times.collect do
+        getvalue(s)
       end
-      arr
     end
 
     # Return object/hash
     def self.getobject(s)
       arr = {}
       while (key = getstring(s))
-        if (key == '') then break end
+        break if key == ''
         arr[key] = getvalue(s)
       end
       s.getbyte		# skip the 9 'end of object' value
@@ -52,16 +49,16 @@ module Potlatch
     # Parse and get value
     def self.getvalue(s)
       case s.getbyte
-      when 0 then	return getdouble(s)			# number
-      when 1 then	return s.getbyte			# boolean
-      when 2 then	return getstring(s)			# string
-      when 3 then	return getobject(s)			# object/hash
-      when 5 then	return nil				# null
-      when 6 then	return nil				# undefined
-      when 8 then	s.read(4)				# mixedArray
+      when 0 then return getdouble(s)			# number
+      when 1 then return s.getbyte			# boolean
+      when 2 then return getstring(s)			# string
+      when 3 then return getobject(s)			# object/hash
+      when 5 then return nil				# null
+      when 6 then return nil				# undefined
+      when 8 then s.read(4)				# mixedArray
                   return getobject(s)			#  |
-      when 10 then  return getarray(s)			# array
-      else;	return nil				# error
+      when 10 then return getarray(s)			# array
+      else         return nil				# error
       end
     end
 
@@ -95,12 +92,12 @@ module Potlatch
         0.chr + encodedouble(n)
       when 'NilClass'
         5.chr
-    when 'TrueClass'
-      0.chr + encodedouble(1)
-    when 'FalseClass'
-      0.chr + encodedouble(0)
-    else
-      Rails.logger.error("Unexpected Ruby type for AMF conversion: " + n.class.to_s)
+      when 'TrueClass'
+        0.chr + encodedouble(1)
+      when 'FalseClass'
+        0.chr + encodedouble(0)
+      else
+        Rails.logger.error("Unexpected Ruby type for AMF conversion: " + n.class.to_s)
       end
     end
 
@@ -178,8 +175,8 @@ module Potlatch
 
       # Read preset menus
       presets = {}
-      presetmenus = {}; presetmenus['point'] = []; presetmenus['way'] = []; presetmenus['POI'] = []
-      presetnames = {}; presetnames['point'] = {}; presetnames['way'] = {}; presetnames['POI'] = {}
+      presetmenus = { 'point' => [], 'way' => [], 'POI' => [] }
+      presetnames = { 'point' => {}, 'way' => {}, 'POI' => {} }
       presettype = ''
       presetcategory = ''
       #	StringIO.open(txt) do |file|
@@ -192,48 +189,52 @@ module Potlatch
             presetmenus[presettype].push(presetcategory)
             presetnames[presettype][presetcategory] = ["(no preset)"]
           elsif t =~ /^([\w\s]+):\s?(.+)$/
-            pre = $1; kv = $2
+            pre = $1
+            kv = $2
             presetnames[presettype][presetcategory].push(pre)
             presets[pre] = {}
             kv.split(',').each do|a|
-              if a =~ /^(.+)=(.*)$/ then presets[pre][$1] = $2 end
+              presets[pre][$1] = $2 if a =~ /^(.+)=(.*)$/
             end
           end
         end
       end
 
       # Read colours/styling
-      colours = {}; casing = {}; areas = {}
+      colours = {}
+      casing = {}
+      areas = {}
       File.open("#{Rails.root}/config/potlatch/colours.txt") do |file|
-        file.each_line do|line|
-          t = line.chomp
-          if t =~ /(\w+)\s+([^\s]+)\s+([^\s]+)\s+([^\s]+)/
-            tag = $1
-            if ($2 != '-') then colours[tag] = $2.hex end
-            if ($3 != '-') then casing[tag] = $3.hex end
-            if ($4 != '-') then areas[tag] = $4.hex end
-          end
+        file.each_line do |line|
+          next unless line.chomp =~ /(\w+)\s+([^\s]+)\s+([^\s]+)\s+([^\s]+)/
+
+          tag = $1
+          colours[tag] = $2.hex if $2 != '-'
+          casing[tag] = $3.hex if $3 != '-'
+          areas[tag] = $4.hex if $4 != '-'
         end
       end
 
       # Read relations colours/styling
-      relcolours = {}; relalphas = {}; relwidths = {}
+      relcolours = {}
+      relalphas = {}
+      relwidths = {}
       File.open("#{Rails.root}/config/potlatch/relation_colours.txt") do |file|
-        file.each_line do|line|
-          t = line.chomp
-          if t =~ /(\w+)\s+([^\s]+)\s+([^\s]+)\s+([^\s]+)/
-            tag = $1
-            if ($2 != '-') then relcolours[tag] = $2.hex end
-            if ($3 != '-') then relalphas[tag] = $3.to_i end
-            if ($4 != '-') then relwidths[tag] = $4.to_i end
-          end
+        file.each_line do |line|
+          next unless line.chomp =~ /(\w+)\s+([^\s]+)\s+([^\s]+)\s+([^\s]+)/
+
+          tag = $1
+          relcolours[tag] = $2.hex if $2 != '-'
+          relalphas[tag] = $3.to_i if $3 != '-'
+          relwidths[tag] = $4.to_i if $4 != '-'
         end
       end
 
       # Read POI presets
-      icon_list = []; icon_tags = {}
+      icon_list = []
+      icon_tags = {}
       File.open("#{Rails.root}/config/potlatch/icon_presets.txt") do |file|
-        file.each_line do|line|
+        file.each_line do |line|
           (icon, tags) = line.chomp.split("\t")
           icon_list.push(icon)
           icon_tags[icon] = Hash[*tags.scan(/([^;=]+)=([^;=]+)/).flatten]
@@ -242,17 +243,18 @@ module Potlatch
       icon_list.reverse!
 
       # Read auto-complete
-      autotags = {}; autotags['point'] = {}; autotags['way'] = {}; autotags['POI'] = {}
+      autotags = { 'point' => {}, 'way' => {}, 'POI' => {} }
       File.open("#{Rails.root}/config/potlatch/autocomplete.txt") do |file|
         file.each_line do|line|
-          t = line.chomp
-          if t =~ /^([\w:]+)\/(\w+)\s+(.+)$/
-            tag = $1; type = $2; values = $3
-            if values == '-'
-              autotags[type][tag] = []
-            else
-              autotags[type][tag] = values.split(',').sort.reverse
-            end
+          next unless line.chomp =~ /^([\w:]+)\/(\w+)\s+(.+)$/
+
+          tag = $1
+          type = $2
+          values = $3
+          if values == '-'
+            autotags[type][tag] = []
+          else
+            autotags[type][tag] = values.split(',').sort.reverse
           end
         end
       end
diff --git a/lib/quova.rb b/lib/quova.rb
index 9a4fb3bf8..a59f432b5 100644
--- a/lib/quova.rb
+++ b/lib/quova.rb
@@ -22,15 +22,15 @@ module Quova
 
   ##
   # Status codes
-  Success = 0
-  IPV6NoSupport = 1
-  InvalidCredentials = 2
-  NotMapped = 3
-  InvalidIPFormat = 4
-  IPAddressNull = 5
-  AccessDenied = 6
-  QueryLimit = 7
-  OutOfService = 10
+  SUCCESS = 0
+  IPV6_NO_SUPPORT = 1
+  INVALID_CREDENTIALS = 2
+  NOT_MAPPED = 3
+  INVALID_IP_FORMAT = 4
+  IP_ADDRESS_NULL = 5
+  ACCESS_DENIED = 6
+  QUERY_LIMIT = 7
+  OUT_OF_SERVICE = 10
 
   ##
   # Create SOAP endpoint
diff --git a/lib/rich_text.rb b/lib/rich_text.rb
index 503ecedbd..bb2baddc8 100644
--- a/lib/rich_text.rb
+++ b/lib/rich_text.rb
@@ -4,7 +4,6 @@ module RichText
     when "html" then HTML.new(text || "")
     when "markdown" then Markdown.new(text || "")
     when "text" then Text.new(text || "")
-    else; nil
     end
   end
 
diff --git a/lib/short_link.rb b/lib/short_link.rb
index 0d8f21cad..44b9d719d 100644
--- a/lib/short_link.rb
+++ b/lib/short_link.rb
@@ -30,8 +30,13 @@ module ShortLink
         z_offset -= 1
       else
         3.times do
-          x <<= 1; x |= 1 unless (t & 32).zero?; t <<= 1
-          y <<= 1; y |= 1 unless (t & 32).zero?; t <<= 1
+          x <<= 1
+          x |= 1 unless (t & 32).zero?
+          t <<= 1
+
+          y <<= 1
+          y |= 1 unless (t & 32).zero?
+          t <<= 1
         end
         z += 3
       end
diff --git a/lib/tasks/add_version_to_nodes.rake b/lib/tasks/add_version_to_nodes.rake
index c0fac49b7..f86961904 100644
--- a/lib/tasks/add_version_to_nodes.rake
+++ b/lib/tasks/add_version_to_nodes.rake
@@ -12,9 +12,7 @@ namespace 'db' do
 
       # should be offsetting not selecting
       OldNode.find(:all, :limit => increment, :offset => offset, :order => 'timestamp').each do |node|
-        if hash[node.id].nil?
-          hash[node.id] = []
-        end
+        hash[node.id] ||= []
         hash[node.id] << node
       end
 
diff --git a/script/locale/po2yaml b/script/locale/po2yaml
index ea78ce2c6..a4b8ac8b7 100755
--- a/script/locale/po2yaml
+++ b/script/locale/po2yaml
@@ -11,9 +11,8 @@ def add_translation(hash, keys, value)
   if keys.empty?
     hash[key] = value
   else
-    unless hash.key? key
-      hash[key] = {}
-    end
+    hash[key] ||= {}
+
     add_translation(hash[key], keys, value)
   end
   hash
@@ -31,11 +30,11 @@ def po2hash(f)
       msgstr = line[8..-2]
     end
 
-    if !path.empty? && !msgstr.empty?
-      add_translation(trs, path, msgstr)
-      path = []
-      msgstr = ''
-    end
+    next if path.empty? || msgstr.empty?
+
+    add_translation(trs, path, msgstr)
+    path = []
+    msgstr = ''
   end
   trs
 end
diff --git a/script/update-spam-blocks b/script/update-spam-blocks
index 89bccf1c9..772417774 100755
--- a/script/update-spam-blocks
+++ b/script/update-spam-blocks
@@ -12,21 +12,16 @@ addresses = User.count(
 )
 
 addresses.each do |address, count|
-  if count > 1
-    acl = Acl.find(:first, :conditions => {
-                     :address => address
-                   })
-
-    unless acl
-      Acl.create({
-                   :address => address,
-                   :k => "no_account_creation",
-                   :v => "auto_spam_block"
-                 }, { :without_protection => true })
-
-      puts "Blocked #{address}"
-    end
-  end
+  next unless count > 1
+  next if Acl.where(:address => address).exists?
+
+  Acl.create({
+               :address => address,
+               :k => "no_account_creation",
+               :v => "auto_spam_block"
+             }, { :without_protection => true })
+
+  puts "Blocked #{address}"
 end
 
 acls = Acl.find(:all, :conditions => {
@@ -35,11 +30,11 @@ acls = Acl.find(:all, :conditions => {
                 })
 
 acls.each do |acl|
-  unless addresses[acl.address]
-    acl.delete
+  next if addresses[acl.address]
+
+  acl.delete
 
-    puts "Unblocked #{acl.address}"
-  end
+  puts "Unblocked #{acl.address}"
 end
 
 exit 0
diff --git a/test/controllers/redactions_controller_test.rb b/test/controllers/redactions_controller_test.rb
index 44a760694..214b54972 100644
--- a/test/controllers/redactions_controller_test.rb
+++ b/test/controllers/redactions_controller_test.rb
@@ -57,9 +57,9 @@ class RedactionsControllerTest < ActionController::TestCase
 
     # remove all elements from the redaction
     redaction = redactions(:example)
-    redaction.old_nodes.each     { |n| n.redaction = nil; n.save! }
-    redaction.old_ways.each      { |w| w.redaction = nil; w.save! }
-    redaction.old_relations.each { |r| r.redaction = nil; r.save! }
+    redaction.old_nodes.each     { |n| n.update!(:redaction => nil) }
+    redaction.old_ways.each      { |w| w.update!(:redaction => nil) }
+    redaction.old_relations.each { |r| r.update!(:redaction => nil) }
 
     delete :destroy, :id => redaction.id
     assert_response :redirect
diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb
index c8ca07a38..fb50ebc5b 100644
--- a/test/controllers/relation_controller_test.rb
+++ b/test/controllers/relation_controller_test.rb
@@ -108,9 +108,7 @@ class RelationControllerTest < ActionController::TestCase
     get :full, :id => current_relations(:visible_relation).id
     assert_response :success
     # FIXME check whether this contains the stuff we want!
-    if $VERBOSE
-      print @response.body
-    end
+    print @response.body if $VERBOSE
   end
 
   ##
@@ -930,9 +928,8 @@ OSM
   ##
   # returns a k->v hash of tags from an xml doc
   def get_tags_as_hash(a)
-    a.find("//osm/relation/tag").sort_by { |v| v['k'] }.inject({}) do |h, v|
+    a.find("//osm/relation/tag").sort_by { |v| v['k'] }.each_with_object({}) do |v, h|
       h[v['k']] = v['v']
-      h
     end
   end
 
diff --git a/test/lib/i18n_test.rb b/test/lib/i18n_test.rb
index e8fcb687e..0509da156 100644
--- a/test/lib/i18n_test.rb
+++ b/test/lib/i18n_test.rb
@@ -34,10 +34,10 @@ class I18nTest < ActiveSupport::TestCase
           value.each do |subkey, subvalue|
             # assert plural_keys.include?(subkey), "#{key}.#{subkey} is not a valid plural key"
 
-            unless subvalue.nil?
-              subvalue.scan(/%\{(\w+)\}/) do
-                assert variables.include?($1), "#{key}.#{subkey} uses unknown interpolation variable #{$1}"
-              end
+            next if subvalue.nil?
+
+            subvalue.scan(/%\{(\w+)\}/) do
+              assert variables.include?($1), "#{key}.#{subkey} uses unknown interpolation variable #{$1}"
             end
           end
         else
diff --git a/test/models/user_preference_test.rb b/test/models/user_preference_test.rb
index de35193f5..8d9d0da4e 100644
--- a/test/models/user_preference_test.rb
+++ b/test/models/user_preference_test.rb
@@ -15,12 +15,12 @@ class UserPreferenceTest < ActiveSupport::TestCase
   # Checks that you cannot add a new preference, that is a duplicate
   def test_add_duplicate_preference
     up = user_preferences(:a)
-    newUP = UserPreference.new
-    newUP.user = users(:normal_user)
-    newUP.k = up.k
-    newUP.v = "some other value"
-    assert_not_equal newUP.v, up.v
-    assert_raise (ActiveRecord::RecordNotUnique) { newUP.save }
+    new_up = UserPreference.new
+    new_up.user = users(:normal_user)
+    new_up.k = up.k
+    new_up.v = "some other value"
+    assert_not_equal new_up.v, up.v
+    assert_raise (ActiveRecord::RecordNotUnique) { new_up.save }
   end
 
   def test_check_valid_length
diff --git a/test/test_helper.rb b/test/test_helper.rb
index f59bed648..b1219b765 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -3,179 +3,181 @@ require File.expand_path('../../config/environment', __FILE__)
 require 'rails/test_help'
 load 'composite_primary_keys/fixtures.rb'
 
-class ActiveSupport::TestCase
-  # Load standard fixtures needed to test API methods
-  def self.api_fixtures
-    # print "setting up the api_fixtures"
-    fixtures :users, :user_roles, :changesets, :changeset_tags
+module ActiveSupport
+  class TestCase
+    # Load standard fixtures needed to test API methods
+    def self.api_fixtures
+      # print "setting up the api_fixtures"
+      fixtures :users, :user_roles, :changesets, :changeset_tags
 
-    fixtures :current_nodes, :nodes
-    set_fixture_class :current_nodes => Node
-    set_fixture_class :nodes => OldNode
+      fixtures :current_nodes, :nodes
+      set_fixture_class :current_nodes => Node
+      set_fixture_class :nodes => OldNode
 
-    fixtures :current_node_tags, :node_tags
-    set_fixture_class :current_node_tags => NodeTag
-    set_fixture_class :node_tags => OldNodeTag
+      fixtures :current_node_tags, :node_tags
+      set_fixture_class :current_node_tags => NodeTag
+      set_fixture_class :node_tags => OldNodeTag
 
-    fixtures :current_ways
-    set_fixture_class :current_ways => Way
+      fixtures :current_ways
+      set_fixture_class :current_ways => Way
 
-    fixtures :current_way_nodes, :current_way_tags
-    set_fixture_class :current_way_nodes => WayNode
-    set_fixture_class :current_way_tags => WayTag
+      fixtures :current_way_nodes, :current_way_tags
+      set_fixture_class :current_way_nodes => WayNode
+      set_fixture_class :current_way_tags => WayTag
 
-    fixtures :ways
-    set_fixture_class :ways => OldWay
+      fixtures :ways
+      set_fixture_class :ways => OldWay
 
-    fixtures :way_nodes, :way_tags
-    set_fixture_class :way_nodes => OldWayNode
-    set_fixture_class :way_tags => OldWayTag
+      fixtures :way_nodes, :way_tags
+      set_fixture_class :way_nodes => OldWayNode
+      set_fixture_class :way_tags => OldWayTag
 
-    fixtures :current_relations
-    set_fixture_class :current_relations => Relation
+      fixtures :current_relations
+      set_fixture_class :current_relations => Relation
 
-    fixtures :current_relation_members, :current_relation_tags
-    set_fixture_class :current_relation_members => RelationMember
-    set_fixture_class :current_relation_tags => RelationTag
+      fixtures :current_relation_members, :current_relation_tags
+      set_fixture_class :current_relation_members => RelationMember
+      set_fixture_class :current_relation_tags => RelationTag
 
-    fixtures :relations
-    set_fixture_class :relations => OldRelation
+      fixtures :relations
+      set_fixture_class :relations => OldRelation
 
-    fixtures :relation_members, :relation_tags
-    set_fixture_class :relation_members => OldRelationMember
-    set_fixture_class :relation_tags => OldRelationTag
+      fixtures :relation_members, :relation_tags
+      set_fixture_class :relation_members => OldRelationMember
+      set_fixture_class :relation_tags => OldRelationTag
 
-    fixtures :gpx_files, :gps_points, :gpx_file_tags
-    set_fixture_class :gpx_files => Trace
-    set_fixture_class :gps_points => Tracepoint
-    set_fixture_class :gpx_file_tags => Tracetag
+      fixtures :gpx_files, :gps_points, :gpx_file_tags
+      set_fixture_class :gpx_files => Trace
+      set_fixture_class :gps_points => Tracepoint
+      set_fixture_class :gpx_file_tags => Tracetag
 
-    fixtures :client_applications
+      fixtures :client_applications
 
-    fixtures :redactions
+      fixtures :redactions
 
-    fixtures :notes, :note_comments
-  end
+      fixtures :notes, :note_comments
+    end
 
-  ##
-  # takes a block which is executed in the context of a different
-  # ActionController instance. this is used so that code can call methods
-  # on the node controller whilst testing the old_node controller.
-  def with_controller(new_controller)
-    controller_save = @controller
-    begin
-      @controller = new_controller
-      yield
-    ensure
-      @controller = controller_save
+    ##
+    # takes a block which is executed in the context of a different
+    # ActionController instance. this is used so that code can call methods
+    # on the node controller whilst testing the old_node controller.
+    def with_controller(new_controller)
+      controller_save = @controller
+      begin
+        @controller = new_controller
+        yield
+      ensure
+        @controller = controller_save
+      end
     end
-  end
 
-  ##
-  # for some reason assert_equal a, b fails when the relations are
-  # actually equal, so this method manually checks the fields...
-  def assert_relations_are_equal(a, b)
-    assert_not_nil a, "first relation is not allowed to be nil"
-    assert_not_nil b, "second relation #{a.id} is not allowed to be nil"
-    assert_equal a.id, b.id, "relation IDs"
-    assert_equal a.changeset_id, b.changeset_id, "changeset ID on relation #{a.id}"
-    assert_equal a.visible, b.visible, "visible on relation #{a.id}, #{a.visible.inspect} != #{b.visible.inspect}"
-    assert_equal a.version, b.version, "version on relation #{a.id}"
-    assert_equal a.tags, b.tags, "tags on relation #{a.id}"
-    assert_equal a.members, b.members, "member references on relation #{a.id}"
-  end
+    ##
+    # for some reason assert_equal a, b fails when the relations are
+    # actually equal, so this method manually checks the fields...
+    def assert_relations_are_equal(a, b)
+      assert_not_nil a, "first relation is not allowed to be nil"
+      assert_not_nil b, "second relation #{a.id} is not allowed to be nil"
+      assert_equal a.id, b.id, "relation IDs"
+      assert_equal a.changeset_id, b.changeset_id, "changeset ID on relation #{a.id}"
+      assert_equal a.visible, b.visible, "visible on relation #{a.id}, #{a.visible.inspect} != #{b.visible.inspect}"
+      assert_equal a.version, b.version, "version on relation #{a.id}"
+      assert_equal a.tags, b.tags, "tags on relation #{a.id}"
+      assert_equal a.members, b.members, "member references on relation #{a.id}"
+    end
 
-  ##
-  # for some reason assert_equal a, b fails when the ways are actually
-  # equal, so this method manually checks the fields...
-  def assert_ways_are_equal(a, b)
-    assert_not_nil a, "first way is not allowed to be nil"
-    assert_not_nil b, "second way #{a.id} is not allowed to be nil"
-    assert_equal a.id, b.id, "way IDs"
-    assert_equal a.changeset_id, b.changeset_id, "changeset ID on way #{a.id}"
-    assert_equal a.visible, b.visible, "visible on way #{a.id}, #{a.visible.inspect} != #{b.visible.inspect}"
-    assert_equal a.version, b.version, "version on way #{a.id}"
-    assert_equal a.tags, b.tags, "tags on way #{a.id}"
-    assert_equal a.nds, b.nds, "node references on way #{a.id}"
-  end
+    ##
+    # for some reason assert_equal a, b fails when the ways are actually
+    # equal, so this method manually checks the fields...
+    def assert_ways_are_equal(a, b)
+      assert_not_nil a, "first way is not allowed to be nil"
+      assert_not_nil b, "second way #{a.id} is not allowed to be nil"
+      assert_equal a.id, b.id, "way IDs"
+      assert_equal a.changeset_id, b.changeset_id, "changeset ID on way #{a.id}"
+      assert_equal a.visible, b.visible, "visible on way #{a.id}, #{a.visible.inspect} != #{b.visible.inspect}"
+      assert_equal a.version, b.version, "version on way #{a.id}"
+      assert_equal a.tags, b.tags, "tags on way #{a.id}"
+      assert_equal a.nds, b.nds, "node references on way #{a.id}"
+    end
 
-  ##
-  # for some reason a==b is false, but there doesn't seem to be any
-  # difference between the nodes, so i'm checking all the attributes
-  # manually and blaming it on ActiveRecord
-  def assert_nodes_are_equal(a, b)
-    assert_equal a.id, b.id, "node IDs"
-    assert_equal a.latitude, b.latitude, "latitude on node #{a.id}"
-    assert_equal a.longitude, b.longitude, "longitude on node #{a.id}"
-    assert_equal a.changeset_id, b.changeset_id, "changeset ID on node #{a.id}"
-    assert_equal a.visible, b.visible, "visible on node #{a.id}"
-    assert_equal a.version, b.version, "version on node #{a.id}"
-    assert_equal a.tags, b.tags, "tags on node #{a.id}"
-  end
+    ##
+    # for some reason a==b is false, but there doesn't seem to be any
+    # difference between the nodes, so i'm checking all the attributes
+    # manually and blaming it on ActiveRecord
+    def assert_nodes_are_equal(a, b)
+      assert_equal a.id, b.id, "node IDs"
+      assert_equal a.latitude, b.latitude, "latitude on node #{a.id}"
+      assert_equal a.longitude, b.longitude, "longitude on node #{a.id}"
+      assert_equal a.changeset_id, b.changeset_id, "changeset ID on node #{a.id}"
+      assert_equal a.visible, b.visible, "visible on node #{a.id}"
+      assert_equal a.version, b.version, "version on node #{a.id}"
+      assert_equal a.tags, b.tags, "tags on node #{a.id}"
+    end
 
-  def basic_authorization(user, pass)
-    @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}")
-  end
+    def basic_authorization(user, pass)
+      @request.env["HTTP_AUTHORIZATION"] = "Basic %s" % Base64.encode64("#{user}:#{pass}")
+    end
 
-  def error_format(format)
-    @request.env["HTTP_X_ERROR_FORMAT"] = format
-  end
+    def error_format(format)
+      @request.env["HTTP_X_ERROR_FORMAT"] = format
+    end
 
-  def content(c)
-    @request.env["RAW_POST_DATA"] = c.to_s
-  end
+    def content(c)
+      @request.env["RAW_POST_DATA"] = c.to_s
+    end
 
-  # Used to check that the error header and the forbidden responses are given
-  # when the owner of the changset has their data not marked as public
-  def assert_require_public_data(msg = "Shouldn't be able to use API when the user's data is not public")
-    assert_response :forbidden, msg
-    assert_equal @response.headers['Error'], "You must make your edits public to upload new data", "Wrong error message"
-  end
+    # Used to check that the error header and the forbidden responses are given
+    # when the owner of the changset has their data not marked as public
+    def assert_require_public_data(msg = "Shouldn't be able to use API when the user's data is not public")
+      assert_response :forbidden, msg
+      assert_equal @response.headers['Error'], "You must make your edits public to upload new data", "Wrong error message"
+    end
 
-  # Not sure this is the best response we could give
-  def assert_inactive_user(msg = "an inactive user shouldn't be able to access the API")
-    assert_response :unauthorized, msg
-    # assert_equal @response.headers['Error'], ""
-  end
+    # Not sure this is the best response we could give
+    def assert_inactive_user(msg = "an inactive user shouldn't be able to access the API")
+      assert_response :unauthorized, msg
+      # assert_equal @response.headers['Error'], ""
+    end
 
-  def assert_no_missing_translations(msg = "")
-    assert_select "span[class=translation_missing]", false, "Missing translation #{msg}"
-  end
+    def assert_no_missing_translations(msg = "")
+      assert_select "span[class=translation_missing]", false, "Missing translation #{msg}"
+    end
 
-  # Set things up for OpenID testing
-  def openid_setup
-    Net::HTTP.get_response(URI.parse("http://localhost:1123/"))
-  rescue
-    # It isn't, so start a new instance.
-    rots = IO.popen("#{Rails.root}/vendor/gems/rots-0.2.1/bin/rots --silent")
+    # Set things up for OpenID testing
+    def openid_setup
+      Net::HTTP.get_response(URI.parse("http://localhost:1123/"))
+    rescue
+      # It isn't, so start a new instance.
+      rots = IO.popen("#{Rails.root}/vendor/gems/rots-0.2.1/bin/rots --silent")
+
+      # Wait for up to 30 seconds for the server to start and respond before continuing
+      1.upto(30).each do
+        begin
+          sleep 1
+          Net::HTTP.get_response(URI.parse("http://localhost:1123/"))
+          # If the rescue block doesn't fire, ROTS is up and running and we can continue
+          break
+        rescue
+          # If the connection failed, do nothing and repeat the loop
+          next
+        end
+      end
 
-    # Wait for up to 30 seconds for the server to start and respond before continuing
-    1.upto(30).each do
-      begin
-        sleep 1
-        Net::HTTP.get_response(URI.parse("http://localhost:1123/"))
-        # If the rescue block doesn't fire, ROTS is up and running and we can continue
-        break
-      rescue
-        # If the connection failed, do nothing and repeat the loop
-        next
+      # Arrange to kill the process when we exit - note that we need
+      # to kill it really har due to a bug in ROTS
+      Kernel.at_exit do
+        Process.kill("KILL", rots.pid)
       end
     end
 
-    # Arrange to kill the process when we exit - note that we need
-    # to kill it really har due to a bug in ROTS
-    Kernel.at_exit do
-      Process.kill("KILL", rots.pid)
-    end
-  end
+    def openid_request(openid_request_uri)
+      openid_response = Net::HTTP.get_response(URI.parse(openid_request_uri))
+      openid_response_uri = URI(openid_response['Location'])
+      openid_response_qs = Rack::Utils.parse_query(openid_response_uri.query)
 
-  def openid_request(openid_request_uri)
-    openid_response = Net::HTTP.get_response(URI.parse(openid_request_uri))
-    openid_response_uri = URI(openid_response['Location'])
-    openid_response_qs = Rack::Utils.parse_query(openid_response_uri.query)
+      openid_response_qs
+    end
 
-    openid_response_qs
+    # Add more helper methods to be used by all tests here...
   end
-
-  # Add more helper methods to be used by all tests here...
 end