]> git.openstreetmap.org Git - rails.git/blobdiff - app/controllers/changeset_controller.rb
tidier code
[rails.git] / app / controllers / changeset_controller.rb
index 3f693a99660d8293fd587dd4092f1614c68a2167..8a72d941d03752f3c826c4abd4f52ba2c233ae90 100644 (file)
@@ -1,22 +1,21 @@
 # The ChangesetController is the RESTful interface to Changeset objects
 
 class ChangesetController < ApplicationController
-  layout 'site'
-  require 'xml/libxml'
-
-  skip_before_filter :verify_authenticity_token, :except => [:list]
-  before_filter :authorize_web, :only => [:list, :feed, :comments_feed]
-  before_filter :set_locale, :only => [:list, :feed, :comments_feed]
-  before_filter :authorize, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
-  before_filter :require_moderator, :only => [:hide_comment, :unhide_comment]
-  before_filter :require_allow_write_api, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
-  before_filter :require_public_data, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe]
-  before_filter :check_api_writable, :only => [:create, :update, :delete, :upload, :include, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
-  before_filter :check_api_readable, :except => [:create, :update, :delete, :upload, :download, :query, :list, :feed, :comment, :subscribe, :unsubscribe, :comments_feed]
-  before_filter(:only => [:list, :feed, :comments_feed]) { |c| c.check_database_readable(true) }
-  after_filter :compress_output
-  around_filter :api_call_handle_error, :except => [:list, :feed, :comments_feed]
-  around_filter :web_timeout, :only => [:list, :feed, :comments_feed]
+  layout "site"
+  require "xml/libxml"
+
+  skip_before_action :verify_authenticity_token, :except => [:list]
+  before_action :authorize_web, :only => [:list, :feed, :comments_feed]
+  before_action :set_locale, :only => [:list, :feed, :comments_feed]
+  before_action :authorize, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
+  before_action :require_moderator, :only => [:hide_comment, :unhide_comment]
+  before_action :require_allow_write_api, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
+  before_action :require_public_data, :only => [:create, :update, :delete, :upload, :include, :close, :comment, :subscribe, :unsubscribe]
+  before_action :check_api_writable, :only => [:create, :update, :delete, :upload, :include, :comment, :subscribe, :unsubscribe, :hide_comment, :unhide_comment]
+  before_action :check_api_readable, :except => [:create, :update, :delete, :upload, :download, :query, :list, :feed, :comment, :subscribe, :unsubscribe, :comments_feed]
+  before_action(:only => [:list, :feed, :comments_feed]) { |c| c.check_database_readable(true) }
+  around_action :api_call_handle_error, :except => [:list, :feed, :comments_feed]
+  around_action :web_timeout, :only => [:list, :feed, :comments_feed]
 
   # Helper methods for checking consistency
   include ConsistencyValidations
@@ -85,8 +84,8 @@ class ChangesetController < ApplicationController
     # abuse, maybe should change to some other format?
     doc = XML::Parser.string(request.raw_post).parse
     doc.find("//osm/node").each do |n|
-      lon << n['lon'].to_f * GeoRecord::SCALE
-      lat << n['lat'].to_f * GeoRecord::SCALE
+      lon << n["lon"].to_f * GeoRecord::SCALE
+      lat << n["lat"].to_f * GeoRecord::SCALE
     end
 
     # add the existing bounding box to the lon-lat array
@@ -96,8 +95,10 @@ class ChangesetController < ApplicationController
     lat << cs.max_lat unless cs.max_lat.nil?
 
     # collapse the arrays to minimum and maximum
-    cs.min_lon, cs.min_lat, cs.max_lon, cs.max_lat =
-      lon.min, lat.min, lon.max, lat.max
+    cs.min_lon = lon.min
+    cs.min_lat = lat.min
+    cs.max_lon = lon.max
+    cs.max_lat = lat.max
 
     # save the larger bounding box and return the changeset, which
     # will include the bigger bounding box.
@@ -185,14 +186,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,19 +205,17 @@ 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.
     changesets = Changeset.all
     changesets = conditions_bbox(changesets, bbox)
-    changesets = conditions_user(changesets, params['user'], params['display_name'])
-    changesets = conditions_time(changesets, params['time'])
-    changesets = conditions_open(changesets, params['open'])
-    changesets = conditions_closed(changesets, params['closed'])
-    changesets = conditions_ids(changesets, params['changesets'])
+    changesets = conditions_user(changesets, params["user"], params["display_name"])
+    changesets = conditions_time(changesets, params["time"])
+    changesets = conditions_open(changesets, params["open"])
+    changesets = conditions_closed(changesets, params["closed"])
+    changesets = conditions_ids(changesets, params["changesets"])
 
     # create the results document
     results = OSM::API.new.get_xml_doc
@@ -244,21 +243,16 @@ class ChangesetController < ApplicationController
     changeset = Changeset.find(params[:id])
     new_changeset = Changeset.from_xml(request.raw_post)
 
-    unless new_changeset.nil?
-      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
+    check_changeset_consistency(changeset, @user)
+    changeset.update_from(new_changeset, @user)
+    render :text => changeset.to_xml, :mime_type => "text/xml"
   end
 
   ##
   # list edits (open changesets) in reverse chronological order
   def list
     if request.format == :atom && params[:max_id]
-      redirect_to params.merge(:max_id => nil), :status => :moved_permanently
+      redirect_to url_for(params.merge(:max_id => nil)), :status => :moved_permanently
       return
     end
 
@@ -270,7 +264,7 @@ class ChangesetController < ApplicationController
       end
     end
 
-    if (params[:friends] || params[:nearby]) && !@user && request.format == :html
+    if (params[:friends] || params[:nearby]) && !@user
       require_user
       return
     end
@@ -446,6 +440,8 @@ class ChangesetController < ApplicationController
     respond_to do |format|
       format.rss
     end
+  rescue OSM::APIBadUserInput
+    render :text => "", :status => :bad_request
   end
 
   private
@@ -473,7 +469,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,18 +497,18 @@ 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
+      if time.count(",") == 1
         # check that we actually have 2 elements in the array
         times = time.split(/,/)
         fail OSM::APIBadUserInput.new("bad time range") if times.size != 2
@@ -521,8 +519,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.
@@ -567,7 +563,7 @@ class ChangesetController < ApplicationController
     elsif ids.empty?
       fail OSM::APIBadUserInput.new("No changesets were given to search for")
     else
-      ids = ids.split(',').collect(&:to_i)
+      ids = ids.split(",").collect(&:to_i)
       return changesets.where(:id => ids)
     end
   end