Move node/way/relation/old_* controllers into the api namespace

.rubocop_todo.yml

@@ -178,7 +178,7 @@ Style/FrozenStringLiteralComment:
 # Cop supports --auto-correct.
 Style/IfUnlessModifier:
   Exclude:
-    - 'app/controllers/ways_controller.rb'
+    - 'app/controllers/api/ways_controller.rb'
 
 # Offense count: 70
 # Cop supports --auto-correct.

app/controllers/api/nodes_controller.rb

@@ -0,0 +1,83 @@
+# The NodeController is the RESTful interface to Node objects
+
+module Api
+  class NodesController < ApplicationController
+    require "xml/libxml"
+
+    skip_before_action :verify_authenticity_token
+    before_action :authorize, :only => [:create, :update, :delete]
+    before_action :api_deny_access_handler
+
+    authorize_resource
+
+    before_action :require_public_data, :only => [:create, :update, :delete]
+    before_action :check_api_writable, :only => [:create, :update, :delete]
+    before_action :check_api_readable, :except => [:create, :update, :delete]
+    around_action :api_call_handle_error, :api_call_timeout
+
+    # Create a node from XML.
+    def create
+      assert_method :put
+
+      node = Node.from_xml(request.raw_post, true)
+
+      # Assume that Node.from_xml has thrown an exception if there is an error parsing the xml
+      node.create_with_history current_user
+      render :plain => node.id.to_s
+    end
+
+    # Dump the details on a node given in params[:id]
+    def show
+      node = Node.find(params[:id])
+
+      response.last_modified = node.timestamp
+
+      if node.visible
+        render :xml => node.to_xml.to_s
+      else
+        head :gone
+      end
+    end
+
+    # Update a node from given XML
+    def update
+      node = Node.find(params[:id])
+      new_node = Node.from_xml(request.raw_post)
+
+      raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" unless new_node && new_node.id == node.id
+
+      node.update_from(new_node, current_user)
+      render :plain => node.version.to_s
+    end
+
+    # Delete a node. Doesn't actually delete it, but retains its history
+    # in a wiki-like way. We therefore treat it like an update, so the delete
+    # method returns the new version number.
+    def delete
+      node = Node.find(params[:id])
+      new_node = Node.from_xml(request.raw_post)
+
+      raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" unless new_node && new_node.id == node.id
+
+      node.delete_with_history!(new_node, current_user)
+      render :plain => node.version.to_s
+    end
+
+    # Dump the details on many nodes whose ids are given in the "nodes" parameter.
+    def index
+      raise OSM::APIBadUserInput, "The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]" unless params["nodes"]
+
+      ids = params["nodes"].split(",").collect(&:to_i)
+
+      raise OSM::APIBadUserInput, "No nodes were given to search for" if ids.empty?
+
+      doc = OSM::API.new.get_xml_doc
+
+      Node.find(ids).each do |node|
+        doc.root << node.to_xml_node
+      end
+
+      render :xml => doc.to_s
+    end
+  end
+end

app/controllers/api/old_controller.rb

@@ -0,0 +1,79 @@
+# this class pulls together the logic for all the old_* controllers
+# into one place. as it turns out, the API methods for historical
+# nodes, ways and relations are basically identical.
+module Api
+  class OldController < ApplicationController
+    require "xml/libxml"
+
+    skip_before_action :verify_authenticity_token
+    before_action :setup_user_auth, :only => [:history, :version]
+    before_action :api_deny_access_handler
+    before_action :authorize, :only => [:redact]
+
+    authorize_resource
+
+    before_action :check_api_readable
+    before_action :check_api_writable, :only => [:redact]
+    around_action :api_call_handle_error, :api_call_timeout
+    before_action :lookup_old_element, :except => [:history]
+    before_action :lookup_old_element_versions, :only => [:history]
+
+    def history
+      # the .where() method used in the lookup_old_element_versions
+      # call won't throw an error if no records are found, so we have
+      # to do that ourselves.
+      raise OSM::APINotFoundError if @elements.empty?
+
+      doc = OSM::API.new.get_xml_doc
+
+      visible_elements = if show_redactions?
+                           @elements
+                         else
+                           @elements.unredacted
+                         end
+
+      visible_elements.each do |element|
+        doc.root << element.to_xml_node
+      end
+
+      render :xml => doc.to_s
+    end
+
+    def version
+      if @old_element.redacted? && !show_redactions?
+        head :forbidden
+
+      else
+        response.last_modified = @old_element.timestamp
+
+        doc = OSM::API.new.get_xml_doc
+        doc.root << @old_element.to_xml_node
+
+        render :xml => doc.to_s
+      end
+    end
+
+    def redact
+      redaction_id = params["redaction"]
+      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)
+      end
+
+      # just return an empty 200 OK for success
+      head :ok
+    end
+
+    private
+
+    def show_redactions?
+      current_user&.moderator? && params[:show_redactions] == "true"
+    end
+  end
+end

app/controllers/api/old_nodes_controller.rb

@@ -0,0 +1,13 @@
+module Api
+  class OldNodesController < OldController
+    private
+
+    def lookup_old_element
+      @old_element = OldNode.find([params[:id], params[:version]])
+    end
+
+    def lookup_old_element_versions
+      @elements = OldNode.where(:node_id => params[:id]).order(:version)
+    end
+  end
+end

app/controllers/api/old_relations_controller.rb

@@ -0,0 +1,13 @@
+module Api
+  class OldRelationsController < OldController
+    private
+
+    def lookup_old_element
+      @old_element = OldRelation.find([params[:id], params[:version]])
+    end
+
+    def lookup_old_element_versions
+      @elements = OldRelation.where(:relation_id => params[:id]).order(:version)
+    end
+  end
+end

app/controllers/api/old_ways_controller.rb

@@ -0,0 +1,13 @@
+module Api
+  class OldWaysController < OldController
+    private
+
+    def lookup_old_element
+      @old_element = OldWay.find([params[:id], params[:version]])
+    end
+
+    def lookup_old_element_versions
+      @elements = OldWay.where(:way_id => params[:id]).order(:version)
+    end
+  end
+end

app/controllers/api/relations_controller.rb

@@ -0,0 +1,169 @@
+module Api
+  class RelationsController < ApplicationController
+    require "xml/libxml"
+
+    skip_before_action :verify_authenticity_token
+    before_action :authorize, :only => [:create, :update, :delete]
+    before_action :api_deny_access_handler
+
+    authorize_resource
+
+    before_action :require_public_data, :only => [:create, :update, :delete]
+    before_action :check_api_writable, :only => [:create, :update, :delete]
+    before_action :check_api_readable, :except => [:create, :update, :delete]
+    around_action :api_call_handle_error, :api_call_timeout
+
+    def create
+      assert_method :put
+
+      relation = Relation.from_xml(request.raw_post, true)
+
+      # Assume that Relation.from_xml has thrown an exception if there is an error parsing the xml
+      relation.create_with_history current_user
+      render :plain => relation.id.to_s
+    end
+
+    def show
+      relation = Relation.find(params[:id])
+      response.last_modified = relation.timestamp
+      if relation.visible
+        render :xml => relation.to_xml.to_s
+      else
+        head :gone
+      end
+    end
+
+    def update
+      logger.debug request.raw_post
+
+      relation = Relation.find(params[:id])
+      new_relation = Relation.from_xml(request.raw_post)
+
+      raise OSM::APIBadUserInput, "The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})" unless new_relation && new_relation.id == relation.id
+
+      relation.update_from new_relation, current_user
+      render :plain => relation.version.to_s
+    end
+
+    def delete
+      relation = Relation.find(params[:id])
+      new_relation = Relation.from_xml(request.raw_post)
+      if new_relation && new_relation.id == relation.id
+        relation.delete_with_history!(new_relation, current_user)
+        render :plain => relation.version.to_s
+      else
+        head :bad_request
+      end
+    end
+
+    # -----------------------------------------------------------------
+    # full
+    #
+    # input parameters: id
+    #
+    # returns XML representation of one relation object plus all its
+    # members, plus all nodes part of member ways
+    # -----------------------------------------------------------------
+    def full
+      relation = Relation.find(params[:id])
+
+      if relation.visible
+
+        # first find the ids of nodes, ways and relations referenced by this
+        # relation - note that we exclude this relation just in case.
+
+        node_ids = relation.members.select { |m| m[0] == "Node" }.map { |m| m[1] }
+        way_ids = relation.members.select { |m| m[0] == "Way" }.map { |m| m[1] }
+        relation_ids = relation.members.select { |m| m[0] == "Relation" && m[1] != relation.id }.map { |m| m[1] }
+
+        # next load the relations and the ways.
+
+        relations = Relation.where(:id => relation_ids).includes(:relation_tags)
+        ways = Way.where(:id => way_ids).includes(:way_nodes, :way_tags)
+
+        # now additionally collect nodes referenced by ways. Note how we
+        # recursively evaluate ways but NOT relations.
+
+        way_node_ids = ways.collect do |way|
+          way.way_nodes.collect(&:node_id)
+        end
+        node_ids += way_node_ids.flatten
+        nodes = Node.where(:id => node_ids.uniq).includes(:node_tags)
+
+        # create XML.
+        doc = OSM::API.new.get_xml_doc
+        visible_nodes = {}
+        changeset_cache = {}
+        user_display_name_cache = {}
+
+        nodes.each do |node|
+          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
+        end
+
+        ways.each do |way|
+          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)
+        end
+
+        relations.each do |rel|
+          next unless rel.visible? # should be unnecessary if data is consistent.
+
+          doc.root << rel.to_xml_node(changeset_cache, user_display_name_cache)
+        end
+
+        # finally add self and output
+        doc.root << relation.to_xml_node(changeset_cache, user_display_name_cache)
+        render :xml => doc.to_s
+
+      else
+        head :gone
+      end
+    end
+
+    def index
+      raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]" unless params["relations"]
+
+      ids = params["relations"].split(",").collect(&:to_i)
+
+      raise OSM::APIBadUserInput, "No relations were given to search for" if ids.empty?
+
+      doc = OSM::API.new.get_xml_doc
+
+      Relation.find(ids).each do |relation|
+        doc.root << relation.to_xml_node
+      end
+
+      render :xml => doc.to_s
+    end
+
+    def relations_for_way
+      relations_for_object("Way")
+    end
+
+    def relations_for_node
+      relations_for_object("Node")
+    end
+
+    def relations_for_relation
+      relations_for_object("Relation")
+    end
+
+    private
+
+    def relations_for_object(objtype)
+      relationids = RelationMember.where(:member_type => objtype, :member_id => params[:id]).collect(&:relation_id).uniq
+
+      doc = OSM::API.new.get_xml_doc
+
+      Relation.find(relationids).each do |relation|
+        doc.root << relation.to_xml_node if relation.visible
+      end
+
+      render :xml => doc.to_s
+    end
+  end
+end

app/controllers/api/ways_controller.rb

@@ -0,0 +1,120 @@
+module Api
+  class WaysController < ApplicationController
+    require "xml/libxml"
+
+    skip_before_action :verify_authenticity_token
+    before_action :authorize, :only => [:create, :update, :delete]
+    before_action :api_deny_access_handler
+
+    authorize_resource
+
+    before_action :require_public_data, :only => [:create, :update, :delete]
+    before_action :check_api_writable, :only => [:create, :update, :delete]
+    before_action :check_api_readable, :except => [:create, :update, :delete]
+    around_action :api_call_handle_error, :api_call_timeout
+
+    def create
+      assert_method :put
+
+      way = Way.from_xml(request.raw_post, true)
+
+      # Assume that Way.from_xml has thrown an exception if there is an error parsing the xml
+      way.create_with_history current_user
+      render :plain => way.id.to_s
+    end
+
+    def show
+      way = Way.find(params[:id])
+
+      response.last_modified = way.timestamp
+
+      if way.visible
+        render :xml => way.to_xml.to_s
+      else
+        head :gone
+      end
+    end
+
+    def update
+      way = Way.find(params[:id])
+      new_way = Way.from_xml(request.raw_post)
+
+      unless new_way && new_way.id == way.id
+        raise OSM::APIBadUserInput, "The id in the url (#{way.id}) is not the same as provided in the xml (#{new_way.id})"
+      end
+
+      way.update_from(new_way, current_user)
+      render :plain => way.version.to_s
+    end
+
+    # This is the API call to delete a way
+    def delete
+      way = Way.find(params[:id])
+      new_way = Way.from_xml(request.raw_post)
+
+      if new_way && new_way.id == way.id
+        way.delete_with_history!(new_way, current_user)
+        render :plain => way.version.to_s
+      else
+        head :bad_request
+      end
+    end
+
+    def full
+      way = Way.includes(:nodes => :node_tags).find(params[:id])
+
+      if way.visible
+        visible_nodes = {}
+        changeset_cache = {}
+        user_display_name_cache = {}
+
+        doc = OSM::API.new.get_xml_doc
+        way.nodes.uniq.each do |node|
+          if node.visible
+            doc.root << node.to_xml_node(changeset_cache, user_display_name_cache)
+            visible_nodes[node.id] = node
+          end
+        end
+        doc.root << way.to_xml_node(visible_nodes, changeset_cache, user_display_name_cache)
+
+        render :xml => doc.to_s
+      else
+        head :gone
+      end
+    end
+
+    def index
+      unless params["ways"]
+        raise OSM::APIBadUserInput, "The parameter ways is required, and must be of the form ways=id[,id[,id...]]"
+      end
+
+      ids = params["ways"].split(",").collect(&:to_i)
+
+      raise OSM::APIBadUserInput, "No ways were given to search for" if ids.empty?
+
+      doc = OSM::API.new.get_xml_doc
+
+      Way.find(ids).each do |way|
+        doc.root << way.to_xml_node
+      end
+
+      render :xml => doc.to_s
+    end
+
+    ##
+    # returns all the ways which are currently using the node given in the
+    # :id parameter. note that this used to return deleted ways as well, but
+    # this seemed not to be the expected behaviour, so it was removed.
+    def ways_for_node
+      wayids = WayNode.where(:node_id => params[:id]).collect { |ws| ws.id[0] }.uniq
+
+      doc = OSM::API.new.get_xml_doc
+
+      Way.find(wayids).each do |way|
+        doc.root << way.to_xml_node if way.visible
+      end
+
+      render :xml => doc.to_s
+    end
+  end
+end

app/controllers/nodes_controller.rb

@@ -1,81 +0,0 @@
-# The NodeController is the RESTful interface to Node objects
-
-class NodesController < ApplicationController
-  require "xml/libxml"
-
-  skip_before_action :verify_authenticity_token
-  before_action :authorize, :only => [:create, :update, :delete]
-  before_action :api_deny_access_handler
-
-  authorize_resource
-
-  before_action :require_public_data, :only => [:create, :update, :delete]
-  before_action :check_api_writable, :only => [:create, :update, :delete]
-  before_action :check_api_readable, :except => [:create, :update, :delete]
-  around_action :api_call_handle_error, :api_call_timeout
-
-  # Create a node from XML.
-  def create
-    assert_method :put
-
-    node = Node.from_xml(request.raw_post, true)
-
-    # Assume that Node.from_xml has thrown an exception if there is an error parsing the xml
-    node.create_with_history current_user
-    render :plain => node.id.to_s
-  end
-
-  # Dump the details on a node given in params[:id]
-  def show
-    node = Node.find(params[:id])
-
-    response.last_modified = node.timestamp
-
-    if node.visible
-      render :xml => node.to_xml.to_s
-    else
-      head :gone
-    end
-  end
-
-  # Update a node from given XML
-  def update
-    node = Node.find(params[:id])
-    new_node = Node.from_xml(request.raw_post)
-
-    raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" unless new_node && new_node.id == node.id
-
-    node.update_from(new_node, current_user)
-    render :plain => node.version.to_s
-  end
-
-  # Delete a node. Doesn't actually delete it, but retains its history
-  # in a wiki-like way. We therefore treat it like an update, so the delete
-  # method returns the new version number.
-  def delete
-    node = Node.find(params[:id])
-    new_node = Node.from_xml(request.raw_post)
-
-    raise OSM::APIBadUserInput, "The id in the url (#{node.id}) is not the same as provided in the xml (#{new_node.id})" unless new_node && new_node.id == node.id
-
-    node.delete_with_history!(new_node, current_user)
-    render :plain => node.version.to_s
-  end
-
-  # Dump the details on many nodes whose ids are given in the "nodes" parameter.
-  def index
-    raise OSM::APIBadUserInput, "The parameter nodes is required, and must be of the form nodes=id[,id[,id...]]" unless params["nodes"]
-
-    ids = params["nodes"].split(",").collect(&:to_i)
-
-    raise OSM::APIBadUserInput, "No nodes were given to search for" if ids.empty?
-
-    doc = OSM::API.new.get_xml_doc
-
-    Node.find(ids).each do |node|
-      doc.root << node.to_xml_node
-    end
-
-    render :xml => doc.to_s
-  end
-end

app/controllers/old_controller.rb

@@ -1,77 +0,0 @@
-# this class pulls together the logic for all the old_* controllers
-# into one place. as it turns out, the API methods for historical
-# nodes, ways and relations are basically identical.
-class OldController < ApplicationController
-  require "xml/libxml"
-
-  skip_before_action :verify_authenticity_token
-  before_action :setup_user_auth, :only => [:history, :version]
-  before_action :api_deny_access_handler
-  before_action :authorize, :only => [:redact]
-
-  authorize_resource
-
-  before_action :check_api_readable
-  before_action :check_api_writable, :only => [:redact]
-  around_action :api_call_handle_error, :api_call_timeout
-  before_action :lookup_old_element, :except => [:history]
-  before_action :lookup_old_element_versions, :only => [:history]
-
-  def history
-    # the .where() method used in the lookup_old_element_versions
-    # call won't throw an error if no records are found, so we have
-    # to do that ourselves.
-    raise OSM::APINotFoundError if @elements.empty?
-
-    doc = OSM::API.new.get_xml_doc
-
-    visible_elements = if show_redactions?
-                         @elements
-                       else
-                         @elements.unredacted
-                       end
-
-    visible_elements.each do |element|
-      doc.root << element.to_xml_node
-    end
-
-    render :xml => doc.to_s
-  end
-
-  def version
-    if @old_element.redacted? && !show_redactions?
-      head :forbidden
-
-    else
-      response.last_modified = @old_element.timestamp
-
-      doc = OSM::API.new.get_xml_doc
-      doc.root << @old_element.to_xml_node
-
-      render :xml => doc.to_s
-    end
-  end
-
-  def redact
-    redaction_id = params["redaction"]
-    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)
-    end
-
-    # just return an empty 200 OK for success
-    head :ok
-  end
-
-  private
-
-  def show_redactions?
-    current_user&.moderator? && params[:show_redactions] == "true"
-  end
-end

app/controllers/old_nodes_controller.rb

@@ -1,11 +0,0 @@
-class OldNodesController < OldController
-  private
-
-  def lookup_old_element
-    @old_element = OldNode.find([params[:id], params[:version]])
-  end
-
-  def lookup_old_element_versions
-    @elements = OldNode.where(:node_id => params[:id]).order(:version)
-  end
-end

app/controllers/old_relations_controller.rb

@@ -1,11 +0,0 @@
-class OldRelationsController < OldController
-  private
-
-  def lookup_old_element
-    @old_element = OldRelation.find([params[:id], params[:version]])
-  end
-
-  def lookup_old_element_versions
-    @elements = OldRelation.where(:relation_id => params[:id]).order(:version)
-  end
-end

app/controllers/old_ways_controller.rb

@@ -1,11 +0,0 @@
-class OldWaysController < OldController
-  private
-
-  def lookup_old_element
-    @old_element = OldWay.find([params[:id], params[:version]])
-  end
-
-  def lookup_old_element_versions
-    @elements = OldWay.where(:way_id => params[:id]).order(:version)
-  end
-end

app/controllers/relations_controller.rb

@@ -1,167 +0,0 @@
-class RelationsController < ApplicationController
-  require "xml/libxml"
-
-  skip_before_action :verify_authenticity_token
-  before_action :authorize, :only => [:create, :update, :delete]
-  before_action :api_deny_access_handler
-
-  authorize_resource
-
-  before_action :require_public_data, :only => [:create, :update, :delete]
-  before_action :check_api_writable, :only => [:create, :update, :delete]
-  before_action :check_api_readable, :except => [:create, :update, :delete]
-  around_action :api_call_handle_error, :api_call_timeout
-
-  def create
-    assert_method :put
-
-    relation = Relation.from_xml(request.raw_post, true)
-
-    # Assume that Relation.from_xml has thrown an exception if there is an error parsing the xml
-    relation.create_with_history current_user
-    render :plain => relation.id.to_s
-  end
-
-  def show
-    relation = Relation.find(params[:id])
-    response.last_modified = relation.timestamp
-    if relation.visible
-      render :xml => relation.to_xml.to_s
-    else
-      head :gone
-    end
-  end
-
-  def update
-    logger.debug request.raw_post
-
-    relation = Relation.find(params[:id])
-    new_relation = Relation.from_xml(request.raw_post)
-
-    raise OSM::APIBadUserInput, "The id in the url (#{relation.id}) is not the same as provided in the xml (#{new_relation.id})" unless new_relation && new_relation.id == relation.id
-
-    relation.update_from new_relation, current_user
-    render :plain => relation.version.to_s
-  end
-
-  def delete
-    relation = Relation.find(params[:id])
-    new_relation = Relation.from_xml(request.raw_post)
-    if new_relation && new_relation.id == relation.id
-      relation.delete_with_history!(new_relation, current_user)
-      render :plain => relation.version.to_s
-    else
-      head :bad_request
-    end
-  end
-
-  # -----------------------------------------------------------------
-  # full
-  #
-  # input parameters: id
-  #
-  # returns XML representation of one relation object plus all its
-  # members, plus all nodes part of member ways
-  # -----------------------------------------------------------------
-  def full
-    relation = Relation.find(params[:id])
-
-    if relation.visible
-
-      # first find the ids of nodes, ways and relations referenced by this
-      # relation - note that we exclude this relation just in case.
-
-      node_ids = relation.members.select { |m| m[0] == "Node" }.map { |m| m[1] }
-      way_ids = relation.members.select { |m| m[0] == "Way" }.map { |m| m[1] }
-      relation_ids = relation.members.select { |m| m[0] == "Relation" && m[1] != relation.id }.map { |m| m[1] }
-
-      # next load the relations and the ways.
-
-      relations = Relation.where(:id => relation_ids).includes(:relation_tags)
-      ways = Way.where(:id => way_ids).includes(:way_nodes, :way_tags)
-
-      # now additionally collect nodes referenced by ways. Note how we
-      # recursively evaluate ways but NOT relations.
-
-      way_node_ids = ways.collect do |way|
-        way.way_nodes.collect(&:node_id)
-      end
-      node_ids += way_node_ids.flatten
-      nodes = Node.where(:id => node_ids.uniq).includes(:node_tags)
-
-      # create XML.
-      doc = OSM::API.new.get_xml_doc
-      visible_nodes = {}
-      changeset_cache = {}
-      user_display_name_cache = {}
-
-      nodes.each do |node|
-        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
-      end
-
-      ways.each do |way|
-        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)
-      end
-
-      relations.each do |rel|
-        next unless rel.visible? # should be unnecessary if data is consistent.
-
-        doc.root << rel.to_xml_node(changeset_cache, user_display_name_cache)
-      end
-
-      # finally add self and output
-      doc.root << relation.to_xml_node(changeset_cache, user_display_name_cache)
-      render :xml => doc.to_s
-
-    else
-      head :gone
-    end
-  end
-
-  def index
-    raise OSM::APIBadUserInput, "The parameter relations is required, and must be of the form relations=id[,id[,id...]]" unless params["relations"]
-
-    ids = params["relations"].split(",").collect(&:to_i)
-
-    raise OSM::APIBadUserInput, "No relations were given to search for" if ids.empty?
-
-    doc = OSM::API.new.get_xml_doc
-
-    Relation.find(ids).each do |relation|
-      doc.root << relation.to_xml_node
-    end
-
-    render :xml => doc.to_s
-  end
-
-  def relations_for_way
-    relations_for_object("Way")
-  end
-
-  def relations_for_node
-    relations_for_object("Node")
-  end
-
-  def relations_for_relation
-    relations_for_object("Relation")
-  end
-
-  private
-
-  def relations_for_object(objtype)
-    relationids = RelationMember.where(:member_type => objtype, :member_id => params[:id]).collect(&:relation_id).uniq
-
-    doc = OSM::API.new.get_xml_doc
-
-    Relation.find(relationids).each do |relation|
-      doc.root << relation.to_xml_node if relation.visible
-    end
-
-    render :xml => doc.to_s
-  end
-end

app/controllers/ways_controller.rb

@@ -1,118 +0,0 @@
-class WaysController < ApplicationController
-  require "xml/libxml"
-
-  skip_before_action :verify_authenticity_token
-  before_action :authorize, :only => [:create, :update, :delete]
-  before_action :api_deny_access_handler
-
-  authorize_resource
-
-  before_action :require_public_data, :only => [:create, :update, :delete]
-  before_action :check_api_writable, :only => [:create, :update, :delete]
-  before_action :check_api_readable, :except => [:create, :update, :delete]
-  around_action :api_call_handle_error, :api_call_timeout
-
-  def create
-    assert_method :put
-
-    way = Way.from_xml(request.raw_post, true)
-
-    # Assume that Way.from_xml has thrown an exception if there is an error parsing the xml
-    way.create_with_history current_user
-    render :plain => way.id.to_s
-  end
-
-  def show
-    way = Way.find(params[:id])
-
-    response.last_modified = way.timestamp
-
-    if way.visible
-      render :xml => way.to_xml.to_s
-    else
-      head :gone
-    end
-  end
-
-  def update
-    way = Way.find(params[:id])
-    new_way = Way.from_xml(request.raw_post)
-
-    unless new_way && new_way.id == way.id
-      raise OSM::APIBadUserInput, "The id in the url (#{way.id}) is not the same as provided in the xml (#{new_way.id})"
-    end
-
-    way.update_from(new_way, current_user)
-    render :plain => way.version.to_s
-  end
-
-  # This is the API call to delete a way
-  def delete
-    way = Way.find(params[:id])
-    new_way = Way.from_xml(request.raw_post)
-
-    if new_way && new_way.id == way.id
-      way.delete_with_history!(new_way, current_user)
-      render :plain => way.version.to_s
-    else
-      head :bad_request
-    end
-  end
-
-  def full
-    way = Way.includes(:nodes => :node_tags).find(params[:id])
-
-    if way.visible
-      visible_nodes = {}
-      changeset_cache = {}
-      user_display_name_cache = {}
-
-      doc = OSM::API.new.get_xml_doc
-      way.nodes.uniq.each do |node|
-        if node.visible
-          doc.root << node.to_xml_node(changeset_cache, user_display_name_cache)
-          visible_nodes[node.id] = node
-        end
-      end
-      doc.root << way.to_xml_node(visible_nodes, changeset_cache, user_display_name_cache)
-
-      render :xml => doc.to_s
-    else
-      head :gone
-    end
-  end
-
-  def index
-    unless params["ways"]
-      raise OSM::APIBadUserInput, "The parameter ways is required, and must be of the form ways=id[,id[,id...]]"
-    end
-
-    ids = params["ways"].split(",").collect(&:to_i)
-
-    raise OSM::APIBadUserInput, "No ways were given to search for" if ids.empty?
-
-    doc = OSM::API.new.get_xml_doc
-
-    Way.find(ids).each do |way|
-      doc.root << way.to_xml_node
-    end
-
-    render :xml => doc.to_s
-  end
-
-  ##
-  # returns all the ways which are currently using the node given in the
-  # :id parameter. note that this used to return deleted ways as well, but
-  # this seemed not to be the expected behaviour, so it was removed.
-  def ways_for_node
-    wayids = WayNode.where(:node_id => params[:id]).collect { |ws| ws.id[0] }.uniq
-
-    doc = OSM::API.new.get_xml_doc
-
-    Way.find(wayids).each do |way|
-      doc.root << way.to_xml_node if way.visible
-    end
-
-    render :xml => doc.to_s
-  end
-end

app/views/browse/feature.html.erb

@@ -8,7 +8,7 @@
 <%= render :partial => @type, :object => @feature %>
 
 <div class='secondary-actions'>
-  <%= link_to(t('browse.download_xml'), :controller => @type.pluralize, :action => :show) %>
+  <%= link_to(t('browse.download_xml'), :controller => "api/#{@type.pluralize}", :action => :show) %>
   &middot;
   <%= link_to(t('browse.view_history'), :action => "#{@type}_history") %>
 </div>

app/views/browse/history.html.erb

@@ -8,7 +8,7 @@
 <%= render :partial => @type, :collection => @feature.send("old_#{@type}s").reverse %>
 
 <div class='secondary-actions'>
-  <%= link_to(t('browse.download_xml'), :controller => "old_#{@type.pluralize}", :action => "history") %>
+  <%= link_to(t('browse.download_xml'), :controller => "api/old_#{@type.pluralize}", :action => "history") %>
   &middot;
   <%= link_to(t('browse.view_details'), :action => @type) %>
 </div>

config/routes.rb

@@ -22,38 +22,38 @@ OpenStreetMap::Application.routes.draw do
     post "changeset/comment/:id/hide" => "api/changeset_comments#destroy", :as => :changeset_comment_hide, :id => /\d+/
     post "changeset/comment/:id/unhide" => "api/changeset_comments#restore", :as => :changeset_comment_unhide, :id => /\d+/
 
-    put "node/create" => "nodes#create"
-    get "node/:id/ways" => "ways#ways_for_node", :id => /\d+/
-    get "node/:id/relations" => "relations#relations_for_node", :id => /\d+/
-    get "node/:id/history" => "old_nodes#history", :id => /\d+/
-    post "node/:id/:version/redact" => "old_nodes#redact", :version => /\d+/, :id => /\d+/
-    get "node/:id/:version" => "old_nodes#version", :id => /\d+/, :version => /\d+/
-    get "node/:id" => "nodes#show", :id => /\d+/
-    put "node/:id" => "nodes#update", :id => /\d+/
-    delete "node/:id" => "nodes#delete", :id => /\d+/
-    get "nodes" => "nodes#index"
+    put "node/create" => "api/nodes#create"
+    get "node/:id/ways" => "api/ways#ways_for_node", :id => /\d+/
+    get "node/:id/relations" => "api/relations#relations_for_node", :id => /\d+/
+    get "node/:id/history" => "api/old_nodes#history", :id => /\d+/
+    post "node/:id/:version/redact" => "api/old_nodes#redact", :version => /\d+/, :id => /\d+/
+    get "node/:id/:version" => "api/old_nodes#version", :id => /\d+/, :version => /\d+/
+    get "node/:id" => "api/nodes#show", :id => /\d+/
+    put "node/:id" => "api/nodes#update", :id => /\d+/
+    delete "node/:id" => "api/nodes#delete", :id => /\d+/
+    get "nodes" => "api/nodes#index"
 
-    put "way/create" => "ways#create"
-    get "way/:id/history" => "old_ways#history", :id => /\d+/
-    get "way/:id/full" => "ways#full", :id => /\d+/
-    get "way/:id/relations" => "relations#relations_for_way", :id => /\d+/
-    post "way/:id/:version/redact" => "old_ways#redact", :version => /\d+/, :id => /\d+/
-    get "way/:id/:version" => "old_ways#version", :id => /\d+/, :version => /\d+/
-    get "way/:id" => "ways#show", :id => /\d+/
-    put "way/:id" => "ways#update", :id => /\d+/
-    delete "way/:id" => "ways#delete", :id => /\d+/
-    get "ways" => "ways#index"
+    put "way/create" => "api/ways#create"
+    get "way/:id/history" => "api/old_ways#history", :id => /\d+/
+    get "way/:id/full" => "api/ways#full", :id => /\d+/
+    get "way/:id/relations" => "api/relations#relations_for_way", :id => /\d+/
+    post "way/:id/:version/redact" => "api/old_ways#redact", :version => /\d+/, :id => /\d+/
+    get "way/:id/:version" => "api/old_ways#version", :id => /\d+/, :version => /\d+/
+    get "way/:id" => "api/ways#show", :id => /\d+/
+    put "way/:id" => "api/ways#update", :id => /\d+/
+    delete "way/:id" => "api/ways#delete", :id => /\d+/
+    get "ways" => "api/ways#index"
 
-    put "relation/create" => "relations#create"
-    get "relation/:id/relations" => "relations#relations_for_relation", :id => /\d+/
-    get "relation/:id/history" => "old_relations#history", :id => /\d+/
-    get "relation/:id/full" => "relations#full", :id => /\d+/
-    post "relation/:id/:version/redact" => "old_relations#redact", :version => /\d+/, :id => /\d+/
-    get "relation/:id/:version" => "old_relations#version", :id => /\d+/, :version => /\d+/
-    get "relation/:id" => "relations#show", :id => /\d+/
-    put "relation/:id" => "relations#update", :id => /\d+/
-    delete "relation/:id" => "relations#delete", :id => /\d+/
-    get "relations" => "relations#index"
+    put "relation/create" => "api/relations#create"
+    get "relation/:id/relations" => "api/relations#relations_for_relation", :id => /\d+/
+    get "relation/:id/history" => "api/old_relations#history", :id => /\d+/
+    get "relation/:id/full" => "api/relations#full", :id => /\d+/
+    post "relation/:id/:version/redact" => "api/old_relations#redact", :version => /\d+/, :id => /\d+/
+    get "relation/:id/:version" => "api/old_relations#version", :id => /\d+/, :version => /\d+/
+    get "relation/:id" => "api/relations#show", :id => /\d+/
+    put "relation/:id" => "api/relations#update", :id => /\d+/
+    delete "relation/:id" => "api/relations#delete", :id => /\d+/
+    get "relations" => "api/relations#index"
 
     get "map" => "api/map#index"
 

test/controllers/api/nodes_controller_test.rb

@@ -0,0 +1,552 @@
+require "test_helper"
+
+module Api
+  class NodesControllerTest < ActionController::TestCase
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/api/0.6/node/create", :method => :put },
+        { :controller => "api/nodes", :action => "create" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/node/1", :method => :get },
+        { :controller => "api/nodes", :action => "show", :id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/node/1", :method => :put },
+        { :controller => "api/nodes", :action => "update", :id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/node/1", :method => :delete },
+        { :controller => "api/nodes", :action => "delete", :id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/nodes", :method => :get },
+        { :controller => "api/nodes", :action => "index" }
+      )
+    end
+
+    def test_create
+      private_user = create(:user, :data_public => false)
+      private_changeset = create(:changeset, :user => private_user)
+      user = create(:user)
+      changeset = create(:changeset, :user => user)
+
+      # create a node with random lat/lon
+      lat = rand(-50..50) + rand
+      lon = rand(-50..50) + rand
+
+      ## First try with no auth
+      # create a minimal xml file
+      xml = "<osm><node lat='#{lat}' lon='#{lon}' changeset='#{changeset.id}'/></osm>"
+      assert_difference("OldNode.count", 0) do
+        put :create, :body => xml
+      end
+      # hope for unauthorized
+      assert_response :unauthorized, "node upload did not return unauthorized status"
+
+      ## Now try with the user which doesn't have their data public
+      basic_authorization private_user.email, "test"
+
+      # create a minimal xml file
+      xml = "<osm><node lat='#{lat}' lon='#{lon}' changeset='#{private_changeset.id}'/></osm>"
+      assert_difference("Node.count", 0) do
+        put :create, :body => xml
+      end
+      # hope for success
+      assert_require_public_data "node create did not return forbidden status"
+
+      ## Now try with the user that has the public data
+      basic_authorization user.email, "test"
+
+      # create a minimal xml file
+      xml = "<osm><node lat='#{lat}' lon='#{lon}' changeset='#{changeset.id}'/></osm>"
+      put :create, :body => xml
+      # hope for success
+      assert_response :success, "node upload did not return success status"
+
+      # read id of created node and search for it
+      nodeid = @response.body
+      checknode = Node.find(nodeid)
+      assert_not_nil checknode, "uploaded node not found in data base after upload"
+      # compare values
+      assert_in_delta lat * 10000000, checknode.latitude, 1, "saved node does not match requested latitude"
+      assert_in_delta lon * 10000000, checknode.longitude, 1, "saved node does not match requested longitude"
+      assert_equal changeset.id, checknode.changeset_id, "saved node does not belong to changeset that it was created in"
+      assert_equal true, checknode.visible, "saved node is not visible"
+    end
+
+    def test_create_invalid_xml
+      ## Only test public user here, as test_create should cover what's the forbiddens
+      ## that would occur here
+
+      user = create(:user)
+      changeset = create(:changeset, :user => user)
+
+      basic_authorization user.email, "test"
+      lat = 3.434
+      lon = 3.23
+
+      # test that the upload is rejected when xml is valid, but osm doc isn't
+      xml = "<create/>"
+      put :create, :body => xml
+      assert_response :bad_request, "node upload did not return bad_request status"
+      assert_equal "Cannot parse valid node from xml string <create/>. XML doesn't contain an osm/node element.", @response.body
+
+      # test that the upload is rejected when no lat is supplied
+      # create a minimal xml file
+      xml = "<osm><node lon='#{lon}' changeset='#{changeset.id}'/></osm>"
+      put :create, :body => xml
+      # hope for success
+      assert_response :bad_request, "node upload did not return bad_request status"
+      assert_equal "Cannot parse valid node from xml string <node lon=\"3.23\" changeset=\"#{changeset.id}\"/>. lat missing", @response.body
+
+      # test that the upload is rejected when no lon is supplied
+      # create a minimal xml file
+      xml = "<osm><node lat='#{lat}' changeset='#{changeset.id}'/></osm>"
+      put :create, :body => xml
+      # hope for success
+      assert_response :bad_request, "node upload did not return bad_request status"
+      assert_equal "Cannot parse valid node from xml string <node lat=\"3.434\" changeset=\"#{changeset.id}\"/>. lon missing", @response.body
+
+      # test that the upload is rejected when lat is non-numeric
+      # create a minimal xml file
+      xml = "<osm><node lat='abc' lon='#{lon}' changeset='#{changeset.id}'/></osm>"
+      put :create, :body => xml
+      # hope for success
+      assert_response :bad_request, "node upload did not return bad_request status"
+      assert_equal "Cannot parse valid node from xml string <node lat=\"abc\" lon=\"#{lon}\" changeset=\"#{changeset.id}\"/>. lat not a number", @response.body
+
+      # test that the upload is rejected when lon is non-numeric
+      # create a minimal xml file
+      xml = "<osm><node lat='#{lat}' lon='abc' changeset='#{changeset.id}'/></osm>"
+      put :create, :body => xml
+      # hope for success
+      assert_response :bad_request, "node upload did not return bad_request status"
+      assert_equal "Cannot parse valid node from xml string <node lat=\"#{lat}\" lon=\"abc\" changeset=\"#{changeset.id}\"/>. lon not a number", @response.body
+
+      # test that the upload is rejected when we have a tag which is too long
+      xml = "<osm><node lat='#{lat}' lon='#{lon}' changeset='#{changeset.id}'><tag k='foo' v='#{'x' * 256}'/></node></osm>"
+      put :create, :body => xml
+      assert_response :bad_request, "node upload did not return bad_request status"
+      assert_equal ["NodeTag ", " v: is too long (maximum is 255 characters) (\"#{'x' * 256}\")"], @response.body.split(/[0-9]+,foo:/)
+    end
+
+    def test_show
+      # check that a visible node is returned properly
+      get :show, :params => { :id => create(:node).id }
+      assert_response :success
+
+      # check that an deleted node is not returned
+      get :show, :params => { :id => create(:node, :deleted).id }
+      assert_response :gone
+
+      # check chat a non-existent node is not returned
+      get :show, :params => { :id => 0 }
+      assert_response :not_found
+    end
+
+    # this tests deletion restrictions - basic deletion is tested in the unit
+    # tests for node!
+    def test_delete
+      private_user = create(:user, :data_public => false)
+      private_user_changeset = create(:changeset, :user => private_user)
+      private_user_closed_changeset = create(:changeset, :closed, :user => private_user)
+      private_node = create(:node, :changeset => private_user_changeset)
+      private_deleted_node = create(:node, :deleted, :changeset => private_user_changeset)
+
+      ## first try to delete node without auth
+      delete :delete, :params => { :id => private_node.id }
+      assert_response :unauthorized
+
+      ## now set auth for the non-data public user
+      basic_authorization private_user.email, "test"
+
+      # try to delete with an invalid (closed) changeset
+      xml = update_changeset(private_node.to_xml, private_user_closed_changeset.id)
+      delete :delete, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data("non-public user shouldn't be able to delete node")
+
+      # try to delete with an invalid (non-existent) changeset
+      xml = update_changeset(private_node.to_xml, 0)
+      delete :delete, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data("shouldn't be able to delete node, when user's data is private")
+
+      # valid delete now takes a payload
+      xml = private_node.to_xml
+      delete :delete, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data("shouldn't be able to delete node when user's data isn't public'")
+
+      # this won't work since the node is already deleted
+      xml = private_deleted_node.to_xml
+      delete :delete, :params => { :id => private_deleted_node.id }, :body => xml.to_s
+      assert_require_public_data
+
+      # this won't work since the node never existed
+      delete :delete, :params => { :id => 0 }
+      assert_require_public_data
+
+      ## these test whether nodes which are in-use can be deleted:
+      # in a way...
+      private_used_node = create(:node, :changeset => private_user_changeset)
+      create(:way_node, :node => private_used_node)
+
+      xml = private_used_node.to_xml
+      delete :delete, :params => { :id => private_used_node.id }, :body => xml.to_s
+      assert_require_public_data "shouldn't be able to delete a node used in a way (#{@response.body})"
+
+      # in a relation...
+      private_used_node2 = create(:node, :changeset => private_user_changeset)
+      create(:relation_member, :member => private_used_node2)
+
+      xml = private_used_node2.to_xml
+      delete :delete, :params => { :id => private_used_node2.id }, :body => xml.to_s
+      assert_require_public_data "shouldn't be able to delete a node used in a relation (#{@response.body})"
+
+      ## now setup for the public data user
+      user = create(:user, :data_public => true)
+      changeset = create(:changeset, :user => user)
+      closed_changeset = create(:changeset, :closed, :user => user)
+      node = create(:node, :changeset => changeset)
+      basic_authorization user.email, "test"
+
+      # try to delete with an invalid (closed) changeset
+      xml = update_changeset(node.to_xml, closed_changeset.id)
+      delete :delete, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :conflict
+
+      # try to delete with an invalid (non-existent) changeset
+      xml = update_changeset(node.to_xml, 0)
+      delete :delete, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :conflict
+
+      # try to delete a node with a different ID
+      other_node = create(:node)
+      xml = other_node.to_xml
+      delete :delete, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :bad_request,
+                      "should not be able to delete a node with a different ID from the XML"
+
+      # try to delete a node rubbish in the payloads
+      xml = "<delete/>"
+      delete :delete, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :bad_request,
+                      "should not be able to delete a node without a valid XML payload"
+
+      # valid delete now takes a payload
+      xml = node.to_xml
+      delete :delete, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :success
+
+      # valid delete should return the new version number, which should
+      # be greater than the old version number
+      assert @response.body.to_i > node.version,
+             "delete request should return a new version number for node"
+
+      # deleting the same node twice doesn't work
+      xml = node.to_xml
+      delete :delete, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :gone
+
+      # this won't work since the node never existed
+      delete :delete, :params => { :id => 0 }
+      assert_response :not_found
+
+      ## these test whether nodes which are in-use can be deleted:
+      # in a way...
+      used_node = create(:node, :changeset => create(:changeset, :user => user))
+      way_node = create(:way_node, :node => used_node)
+      way_node2 = create(:way_node, :node => used_node)
+
+      xml = used_node.to_xml
+      delete :delete, :params => { :id => used_node.id }, :body => xml.to_s
+      assert_response :precondition_failed,
+                      "shouldn't be able to delete a node used in a way (#{@response.body})"
+      assert_equal "Precondition failed: Node #{used_node.id} is still used by ways #{way_node.way.id},#{way_node2.way.id}.", @response.body
+
+      # in a relation...
+      used_node2 = create(:node, :changeset => create(:changeset, :user => user))
+      relation_member = create(:relation_member, :member => used_node2)
+      relation_member2 = create(:relation_member, :member => used_node2)
+
+      xml = used_node2.to_xml
+      delete :delete, :params => { :id => used_node2.id }, :body => xml.to_s
+      assert_response :precondition_failed,
+                      "shouldn't be able to delete a node used in a relation (#{@response.body})"
+      assert_equal "Precondition failed: Node #{used_node2.id} is still used by relations #{relation_member.relation.id},#{relation_member2.relation.id}.", @response.body
+    end
+
+    ##
+    # tests whether the API works and prevents incorrect use while trying
+    # to update nodes.
+    def test_update
+      ## First test with no user credentials
+      # try and update a node without authorisation
+      # first try to delete node without auth
+      private_user = create(:user, :data_public => false)
+      private_node = create(:node, :changeset => create(:changeset, :user => private_user))
+      user = create(:user)
+      node = create(:node, :changeset => create(:changeset, :user => user))
+
+      xml = node.to_xml
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :unauthorized
+
+      ## Second test with the private user
+
+      # setup auth
+      basic_authorization private_user.email, "test"
+
+      ## trying to break changesets
+
+      # try and update in someone else's changeset
+      xml = update_changeset(private_node.to_xml,
+                             create(:changeset).id)
+      put :update, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data "update with other user's changeset should be forbidden when data isn't public"
+
+      # try and update in a closed changeset
+      xml = update_changeset(private_node.to_xml,
+                             create(:changeset, :closed, :user => private_user).id)
+      put :update, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data "update with closed changeset should be forbidden, when data isn't public"
+
+      # try and update in a non-existant changeset
+      xml = update_changeset(private_node.to_xml, 0)
+      put :update, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data "update with changeset=0 should be forbidden, when data isn't public"
+
+      ## try and submit invalid updates
+      xml = xml_attr_rewrite(private_node.to_xml, "lat", 91.0)
+      put :update, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data "node at lat=91 should be forbidden, when data isn't public"
+
+      xml = xml_attr_rewrite(private_node.to_xml, "lat", -91.0)
+      put :update, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data "node at lat=-91 should be forbidden, when data isn't public"
+
+      xml = xml_attr_rewrite(private_node.to_xml, "lon", 181.0)
+      put :update, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data "node at lon=181 should be forbidden, when data isn't public"
+
+      xml = xml_attr_rewrite(private_node.to_xml, "lon", -181.0)
+      put :update, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data "node at lon=-181 should be forbidden, when data isn't public"
+
+      ## finally, produce a good request which still won't work
+      xml = private_node.to_xml
+      put :update, :params => { :id => private_node.id }, :body => xml.to_s
+      assert_require_public_data "should have failed with a forbidden when data isn't public"
+
+      ## Finally test with the public user
+
+      # try and update a node without authorisation
+      # first try to update node without auth
+      xml = node.to_xml
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :forbidden
+
+      # setup auth
+      basic_authorization user.email, "test"
+
+      ## trying to break changesets
+
+      # try and update in someone else's changeset
+      xml = update_changeset(node.to_xml,
+                             create(:changeset).id)
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :conflict, "update with other user's changeset should be rejected"
+
+      # try and update in a closed changeset
+      xml = update_changeset(node.to_xml,
+                             create(:changeset, :closed, :user => user).id)
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :conflict, "update with closed changeset should be rejected"
+
+      # try and update in a non-existant changeset
+      xml = update_changeset(node.to_xml, 0)
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :conflict, "update with changeset=0 should be rejected"
+
+      ## try and submit invalid updates
+      xml = xml_attr_rewrite(node.to_xml, "lat", 91.0)
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :bad_request, "node at lat=91 should be rejected"
+
+      xml = xml_attr_rewrite(node.to_xml, "lat", -91.0)
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :bad_request, "node at lat=-91 should be rejected"
+
+      xml = xml_attr_rewrite(node.to_xml, "lon", 181.0)
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :bad_request, "node at lon=181 should be rejected"
+
+      xml = xml_attr_rewrite(node.to_xml, "lon", -181.0)
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :bad_request, "node at lon=-181 should be rejected"
+
+      ## next, attack the versioning
+      current_node_version = node.version
+
+      # try and submit a version behind
+      xml = xml_attr_rewrite(node.to_xml,
+                             "version", current_node_version - 1)
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :conflict, "should have failed on old version number"
+
+      # try and submit a version ahead
+      xml = xml_attr_rewrite(node.to_xml,
+                             "version", current_node_version + 1)
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :conflict, "should have failed on skipped version number"
+
+      # try and submit total crap in the version field
+      xml = xml_attr_rewrite(node.to_xml,
+                             "version", "p1r4t3s!")
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :conflict,
+                      "should not be able to put 'p1r4at3s!' in the version field"
+
+      ## try an update with the wrong ID
+      xml = create(:node).to_xml
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :bad_request,
+                      "should not be able to update a node with a different ID from the XML"
+
+      ## try an update with a minimal valid XML doc which isn't a well-formed OSM doc.
+      xml = "<update/>"
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :bad_request,
+                      "should not be able to update a node with non-OSM XML doc."
+
+      ## finally, produce a good request which should work
+      xml = node.to_xml
+      put :update, :params => { :id => node.id }, :body => xml.to_s
+      assert_response :success, "a valid update request failed"
+    end
+
+    ##
+    # test fetching multiple nodes
+    def test_index
+      node1 = create(:node)
+      node2 = create(:node, :deleted)
+      node3 = create(:node)
+      node4 = create(:node, :with_history, :version => 2)
+      node5 = create(:node, :deleted, :with_history, :version => 2)
+
+      # check error when no parameter provided
+      get :index
+      assert_response :bad_request
+
+      # check error when no parameter value provided
+      get :index, :params => { :nodes => "" }
+      assert_response :bad_request
+
+      # test a working call
+      get :index, :params => { :nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id}" }
+      assert_response :success
+      assert_select "osm" do
+        assert_select "node", :count => 5
+        assert_select "node[id='#{node1.id}'][visible='true']", :count => 1
+        assert_select "node[id='#{node2.id}'][visible='false']", :count => 1
+        assert_select "node[id='#{node3.id}'][visible='true']", :count => 1
+        assert_select "node[id='#{node4.id}'][visible='true']", :count => 1
+        assert_select "node[id='#{node5.id}'][visible='false']", :count => 1
+      end
+
+      # check error when a non-existent node is included
+      get :index, :params => { :nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id},0" }
+      assert_response :not_found
+    end
+
+    ##
+    # test adding tags to a node
+    def test_duplicate_tags
+      existing_tag = create(:node_tag)
+      assert_equal true, existing_tag.node.changeset.user.data_public
+      # setup auth
+      basic_authorization existing_tag.node.changeset.user.email, "test"
+
+      # add an identical tag to the node
+      tag_xml = XML::Node.new("tag")
+      tag_xml["k"] = existing_tag.k
+      tag_xml["v"] = existing_tag.v
+
+      # add the tag into the existing xml
+      node_xml = existing_tag.node.to_xml
+      node_xml.find("//osm/node").first << tag_xml
+
+      # try and upload it
+      put :update, :params => { :id => existing_tag.node.id }, :body => node_xml.to_s
+      assert_response :bad_request,
+                      "adding duplicate tags to a node should fail with 'bad request'"
+      assert_equal "Element node/#{existing_tag.node.id} has duplicate tags with key #{existing_tag.k}", @response.body
+    end
+
+    # test whether string injection is possible
+    def test_string_injection
+      private_user = create(:user, :data_public => false)
+      private_changeset = create(:changeset, :user => private_user)
+      user = create(:user)
+      changeset = create(:changeset, :user => user)
+
+      ## First try with the non-data public user
+      basic_authorization private_user.email, "test"
+
+      # try and put something into a string that the API might
+      # use unquoted and therefore allow code injection...
+      xml = "<osm><node lat='0' lon='0' changeset='#{private_changeset.id}'>" \
+            '<tag k="#{@user.inspect}" v="0"/>' \
+            "</node></osm>"
+      put :create, :body => xml
+      assert_require_public_data "Shouldn't be able to create with non-public user"
+
+      ## Then try with the public data user
+      basic_authorization user.email, "test"
+
+      # try and put something into a string that the API might
+      # use unquoted and therefore allow code injection...
+      xml = "<osm><node lat='0' lon='0' changeset='#{changeset.id}'>" \
+            '<tag k="#{@user.inspect}" v="0"/>' \
+            "</node></osm>"
+      put :create, :body => xml
+      assert_response :success
+      nodeid = @response.body
+
+      # find the node in the database
+      checknode = Node.find(nodeid)
+      assert_not_nil checknode, "node not found in data base after upload"
+
+      # and grab it using the api
+      get :show, :params => { :id => nodeid }
+      assert_response :success
+      apinode = Node.from_xml(@response.body)
+      assert_not_nil apinode, "downloaded node is nil, but shouldn't be"
+
+      # check the tags are not corrupted
+      assert_equal checknode.tags, apinode.tags
+      assert apinode.tags.include?("\#{@user.inspect}")
+    end
+
+    ##
+    # update the changeset_id of a node element
+    def update_changeset(xml, changeset_id)
+      xml_attr_rewrite(xml, "changeset", changeset_id)
+    end
+
+    ##
+    # update an attribute in the node element
+    def xml_attr_rewrite(xml, name, value)
+      xml.find("//osm/node").first[name] = value.to_s
+      xml
+    end
+
+    ##
+    # parse some xml
+    def xml_parse(xml)
+      parser = XML::Parser.string(xml)
+      parser.parse
+    end
+  end
+end

test/controllers/api/old_nodes_controller_test.rb

@@ -0,0 +1,430 @@
+require "test_helper"
+
+module Api
+  class OldNodesControllerTest < ActionController::TestCase
+    #
+    # TODO: test history
+    #
+
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/api/0.6/node/1/history", :method => :get },
+        { :controller => "api/old_nodes", :action => "history", :id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/node/1/2", :method => :get },
+        { :controller => "api/old_nodes", :action => "version", :id => "1", :version => "2" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/node/1/2/redact", :method => :post },
+        { :controller => "api/old_nodes", :action => "redact", :id => "1", :version => "2" }
+      )
+    end
+
+    ##
+    # test the version call by submitting several revisions of a new node
+    # to the API and ensuring that later calls to version return the
+    # matching versions of the object.
+    #
+    ##
+    # FIXME: Move this test to being an integration test since it spans multiple controllers
+    def test_version
+      private_user = create(:user, :data_public => false)
+      private_node = create(:node, :with_history, :version => 4, :changeset => create(:changeset, :user => private_user))
+      user = create(:user)
+      node = create(:node, :with_history, :version => 4, :changeset => create(:changeset, :user => user))
+      create_list(:node_tag, 2, :node => node)
+      # Ensure that the current tags are propagated to the history too
+      propagate_tags(node, node.old_nodes.last)
+
+      ## First try this with a non-public user
+      basic_authorization private_user.email, "test"
+
+      # setup a simple XML node
+      xml_doc = private_node.to_xml
+      xml_node = xml_doc.find("//osm/node").first
+      nodeid = private_node.id
+
+      # keep a hash of the versions => string, as we'll need something
+      # to test against later
+      versions = {}
+
+      # save a version for later checking
+      versions[xml_node["version"]] = xml_doc.to_s
+
+      # randomly move the node about
+      3.times do
+        # move the node somewhere else
+        xml_node["lat"] = precision(rand * 180 - 90).to_s
+        xml_node["lon"] = precision(rand * 360 - 180).to_s
+        with_controller(NodesController.new) do
+          put :update, :params => { :id => nodeid }, :body => xml_doc.to_s
+          assert_response :forbidden, "Should have rejected node update"
+          xml_node["version"] = @response.body.to_s
+        end
+        # save a version for later checking
+        versions[xml_node["version"]] = xml_doc.to_s
+      end
+
+      # add a bunch of random tags
+      3.times do
+        xml_tag = XML::Node.new("tag")
+        xml_tag["k"] = random_string
+        xml_tag["v"] = random_string
+        xml_node << xml_tag
+        with_controller(NodesController.new) do
+          put :update, :params => { :id => nodeid }, :body => xml_doc.to_s
+          assert_response :forbidden,
+                          "should have rejected node #{nodeid} (#{@response.body}) with forbidden"
+          xml_node["version"] = @response.body.to_s
+        end
+        # save a version for later checking
+        versions[xml_node["version"]] = xml_doc.to_s
+      end
+
+      # probably should check that they didn't get written to the database
+
+      ## Now do it with the public user
+      basic_authorization user.email, "test"
+
+      # setup a simple XML node
+
+      xml_doc = node.to_xml
+      xml_node = xml_doc.find("//osm/node").first
+      nodeid = node.id
+
+      # keep a hash of the versions => string, as we'll need something
+      # to test against later
+      versions = {}
+
+      # save a version for later checking
+      versions[xml_node["version"]] = xml_doc.to_s
+
+      # randomly move the node about
+      3.times do
+        # move the node somewhere else
+        xml_node["lat"] = precision(rand * 180 - 90).to_s
+        xml_node["lon"] = precision(rand * 360 - 180).to_s
+        with_controller(NodesController.new) do
+          put :update, :params => { :id => nodeid }, :body => xml_doc.to_s
+          assert_response :success
+          xml_node["version"] = @response.body.to_s
+        end
+        # save a version for later checking
+        versions[xml_node["version"]] = xml_doc.to_s
+      end
+
+      # add a bunch of random tags
+      3.times do
+        xml_tag = XML::Node.new("tag")
+        xml_tag["k"] = random_string
+        xml_tag["v"] = random_string
+        xml_node << xml_tag
+        with_controller(NodesController.new) do
+          put :update, :params => { :id => nodeid }, :body => xml_doc.to_s
+          assert_response :success,
+                          "couldn't update node #{nodeid} (#{@response.body})"
+          xml_node["version"] = @response.body.to_s
+        end
+        # save a version for later checking
+        versions[xml_node["version"]] = xml_doc.to_s
+      end
+
+      # check all the versions
+      versions.each_key do |key|
+        get :version, :params => { :id => nodeid, :version => key.to_i }
+
+        assert_response :success,
+                        "couldn't get version #{key.to_i} of node #{nodeid}"
+
+        check_node = Node.from_xml(versions[key])
+        api_node = Node.from_xml(@response.body.to_s)
+
+        assert_nodes_are_equal check_node, api_node
+      end
+    end
+
+    def test_not_found_version
+      check_not_found_id_version(70000, 312344)
+      check_not_found_id_version(-1, -13)
+      check_not_found_id_version(create(:node).id, 24354)
+      check_not_found_id_version(24356, create(:node).version)
+    end
+
+    def check_not_found_id_version(id, version)
+      get :version, :params => { :id => id, :version => version }
+      assert_response :not_found
+    rescue ActionController::UrlGenerationError => ex
+      assert_match(/No route matches/, ex.to_s)
+    end
+
+    ##
+    # Test that getting the current version is identical to picking
+    # that version with the version URI call.
+    def test_current_version
+      node = create(:node, :with_history)
+      used_node = create(:node, :with_history)
+      create(:way_node, :node => used_node)
+      node_used_by_relationship = create(:node, :with_history)
+      create(:relation_member, :member => node_used_by_relationship)
+      node_with_versions = create(:node, :with_history, :version => 4)
+
+      create(:node_tag, :node => node)
+      create(:node_tag, :node => used_node)
+      create(:node_tag, :node => node_used_by_relationship)
+      create(:node_tag, :node => node_with_versions)
+      propagate_tags(node, node.old_nodes.last)
+      propagate_tags(used_node, used_node.old_nodes.last)
+      propagate_tags(node_used_by_relationship, node_used_by_relationship.old_nodes.last)
+      propagate_tags(node_with_versions, node_with_versions.old_nodes.last)
+
+      check_current_version(node)
+      check_current_version(used_node)
+      check_current_version(node_used_by_relationship)
+      check_current_version(node_with_versions)
+    end
+
+    ##
+    # test the redaction of an old version of a node, while not being
+    # authorised.
+    def test_redact_node_unauthorised
+      node = create(:node, :with_history, :version => 4)
+      node_v3 = node.old_nodes.find_by(:version => 3)
+
+      do_redact_node(node_v3,
+                     create(:redaction))
+      assert_response :unauthorized, "should need to be authenticated to redact."
+    end
+
+    ##
+    # test the redaction of an old version of a node, while being
+    # authorised as a normal user.
+    def test_redact_node_normal_user
+      basic_authorization create(:user).email, "test"
+
+      node = create(:node, :with_history, :version => 4)
+      node_v3 = node.old_nodes.find_by(:version => 3)
+
+      do_redact_node(node_v3,
+                     create(:redaction))
+      assert_response :forbidden, "should need to be moderator to redact."
+    end
+
+    ##
+    # test that, even as moderator, the current version of a node
+    # can't be redacted.
+    def test_redact_node_current_version
+      basic_authorization create(:moderator_user).email, "test"
+
+      node = create(:node, :with_history, :version => 4)
+      node_v4 = node.old_nodes.find_by(:version => 4)
+
+      do_redact_node(node_v4,
+                     create(:redaction))
+      assert_response :bad_request, "shouldn't be OK to redact current version as moderator."
+    end
+
+    ##
+    # test that redacted nodes aren't visible, regardless of
+    # authorisation except as moderator...
+    def test_version_redacted
+      node = create(:node, :with_history, :version => 2)
+      node_v1 = node.old_nodes.find_by(:version => 1)
+      node_v1.redact!(create(:redaction))
+
+      get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }
+      assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
+
+      # not even to a logged-in user
+      basic_authorization create(:user).email, "test"
+      get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }
+      assert_response :forbidden, "Redacted node shouldn't be visible via the version API, even when logged in."
+    end
+
+    ##
+    # test that redacted nodes aren't visible in the history
+    def test_history_redacted
+      node = create(:node, :with_history, :version => 2)
+      node_v1 = node.old_nodes.find_by(:version => 1)
+      node_v1.redact!(create(:redaction))
+
+      get :history, :params => { :id => node_v1.node_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 0, "redacted node #{node_v1.node_id} version #{node_v1.version} shouldn't be present in the history."
+
+      # not even to a logged-in user
+      basic_authorization create(:user).email, "test"
+      get :history, :params => { :id => node_v1.node_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 0, "redacted node #{node_v1.node_id} version #{node_v1.version} shouldn't be present in the history, even when logged in."
+    end
+
+    ##
+    # test the redaction of an old version of a node, while being
+    # authorised as a moderator.
+    def test_redact_node_moderator
+      node = create(:node, :with_history, :version => 4)
+      node_v3 = node.old_nodes.find_by(:version => 3)
+      basic_authorization create(:moderator_user).email, "test"
+
+      do_redact_node(node_v3, create(:redaction))
+      assert_response :success, "should be OK to redact old version as moderator."
+
+      # check moderator can still see the redacted data, when passing
+      # the appropriate flag
+      get :version, :params => { :id => node_v3.node_id, :version => node_v3.version }
+      assert_response :forbidden, "After redaction, node should be gone for moderator, when flag not passed."
+      get :version, :params => { :id => node_v3.node_id, :version => node_v3.version, :show_redactions => "true" }
+      assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
+
+      # and when accessed via history
+      get :history, :params => { :id => node_v3.node_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 0, "node #{node_v3.node_id} version #{node_v3.version} should not be present in the history for moderators when not passing flag."
+      get :history, :params => { :id => node_v3.node_id, :show_redactions => "true" }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 1, "node #{node_v3.node_id} version #{node_v3.version} should still be present in the history for moderators when passing flag."
+    end
+
+    # testing that if the moderator drops auth, he can't see the
+    # redacted stuff any more.
+    def test_redact_node_is_redacted
+      node = create(:node, :with_history, :version => 4)
+      node_v3 = node.old_nodes.find_by(:version => 3)
+      basic_authorization create(:moderator_user).email, "test"
+
+      do_redact_node(node_v3, create(:redaction))
+      assert_response :success, "should be OK to redact old version as moderator."
+
+      # re-auth as non-moderator
+      basic_authorization create(:user).email, "test"
+
+      # check can't see the redacted data
+      get :version, :params => { :id => node_v3.node_id, :version => node_v3.version }
+      assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
+
+      # and when accessed via history
+      get :history, :params => { :id => node_v3.node_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 0, "redacted node #{node_v3.node_id} version #{node_v3.version} shouldn't be present in the history."
+    end
+
+    ##
+    # test the unredaction of an old version of a node, while not being
+    # authorised.
+    def test_unredact_node_unauthorised
+      node = create(:node, :with_history, :version => 2)
+      node_v1 = node.old_nodes.find_by(:version => 1)
+      node_v1.redact!(create(:redaction))
+
+      post :redact, :params => { :id => node_v1.node_id, :version => node_v1.version }
+      assert_response :unauthorized, "should need to be authenticated to unredact."
+    end
+
+    ##
+    # test the unredaction of an old version of a node, while being
+    # authorised as a normal user.
+    def test_unredact_node_normal_user
+      user = create(:user)
+      node = create(:node, :with_history, :version => 2)
+      node_v1 = node.old_nodes.find_by(:version => 1)
+      node_v1.redact!(create(:redaction))
+
+      basic_authorization user.email, "test"
+
+      post :redact, :params => { :id => node_v1.node_id, :version => node_v1.version }
+      assert_response :forbidden, "should need to be moderator to unredact."
+    end
+
+    ##
+    # test the unredaction of an old version of a node, while being
+    # authorised as a moderator.
+    def test_unredact_node_moderator
+      moderator_user = create(:moderator_user)
+      node = create(:node, :with_history, :version => 2)
+      node_v1 = node.old_nodes.find_by(:version => 1)
+      node_v1.redact!(create(:redaction))
+
+      basic_authorization moderator_user.email, "test"
+
+      post :redact, :params => { :id => node_v1.node_id, :version => node_v1.version }
+      assert_response :success, "should be OK to unredact old version as moderator."
+
+      # check moderator can now see the redacted data, when not
+      # passing the aspecial flag
+      get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }
+      assert_response :success, "After unredaction, node should not be gone for moderator."
+
+      # and when accessed via history
+      get :history, :params => { :id => node_v1.node_id }
+      assert_response :success, "Unredaction shouldn't have stopped history working."
+      assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 1, "node #{node_v1.node_id} version #{node_v1.version} should now be present in the history for moderators without passing flag."
+
+      basic_authorization create(:user).email, "test"
+
+      # check normal user can now see the redacted data
+      get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }
+      assert_response :success, "After unredaction, node should be visible to normal users."
+
+      # and when accessed via history
+      get :history, :params => { :id => node_v1.node_id }
+      assert_response :success, "Unredaction shouldn't have stopped history working."
+      assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 1, "node #{node_v1.node_id} version #{node_v1.version} should now be present in the history for normal users without passing flag."
+    end
+
+    private
+
+    def do_redact_node(node, redaction)
+      get :version, :params => { :id => node.node_id, :version => node.version }
+      assert_response :success, "should be able to get version #{node.version} of node #{node.node_id}."
+
+      # now redact it
+      post :redact, :params => { :id => node.node_id, :version => node.version, :redaction => redaction.id }
+    end
+
+    def check_current_version(node_id)
+      # get the current version of the node
+      current_node = with_controller(NodesController.new) do
+        get :show, :params => { :id => node_id }
+        assert_response :success, "cant get current node #{node_id}"
+        Node.from_xml(@response.body)
+      end
+      assert_not_nil current_node, "getting node #{node_id} returned nil"
+
+      # get the "old" version of the node from the old_node interface
+      get :version, :params => { :id => node_id, :version => current_node.version }
+      assert_response :success, "cant get old node #{node_id}, v#{current_node.version}"
+      old_node = Node.from_xml(@response.body)
+
+      # check the nodes are the same
+      assert_nodes_are_equal current_node, old_node
+    end
+
+    ##
+    # returns a 16 character long string with some nasty characters in it.
+    # this ought to stress-test the tag handling as well as the versioning.
+    def random_string
+      letters = [["!", '"', "$", "&", ";", "@"],
+                 ("a".."z").to_a,
+                 ("A".."Z").to_a,
+                 ("0".."9").to_a].flatten
+      (1..16).map { |_i| letters[rand(letters.length)] }.join
+    end
+
+    ##
+    # truncate a floating point number to the scale that it is stored in
+    # the database. otherwise rounding errors can produce failing unit
+    # tests when they shouldn't.
+    def precision(f)
+      (f * GeoRecord::SCALE).round.to_f / GeoRecord::SCALE
+    end
+
+    def propagate_tags(node, old_node)
+      node.tags.each do |k, v|
+        create(:old_node_tag, :old_node => old_node, :k => k, :v => v)
+      end
+    end
+  end
+end

test/controllers/api/old_relations_controller_test.rb

@@ -0,0 +1,274 @@
+require "test_helper"
+
+module Api
+  class OldRelationsControllerTest < ActionController::TestCase
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/api/0.6/relation/1/history", :method => :get },
+        { :controller => "api/old_relations", :action => "history", :id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/relation/1/2", :method => :get },
+        { :controller => "api/old_relations", :action => "version", :id => "1", :version => "2" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/relation/1/2/redact", :method => :post },
+        { :controller => "api/old_relations", :action => "redact", :id => "1", :version => "2" }
+      )
+    end
+
+    # -------------------------------------
+    # Test reading old relations.
+    # -------------------------------------
+    def test_history
+      # check that a visible relations is returned properly
+      get :history, :params => { :id => create(:relation, :with_history).id }
+      assert_response :success
+
+      # check chat a non-existent relations is not returned
+      get :history, :params => { :id => 0 }
+      assert_response :not_found
+    end
+
+    ##
+    # test the redaction of an old version of a relation, while not being
+    # authorised.
+    def test_redact_relation_unauthorised
+      relation = create(:relation, :with_history, :version => 4)
+      relation_v3 = relation.old_relations.find_by(:version => 3)
+
+      do_redact_relation(relation_v3, create(:redaction))
+      assert_response :unauthorized, "should need to be authenticated to redact."
+    end
+
+    ##
+    # test the redaction of an old version of a relation, while being
+    # authorised as a normal user.
+    def test_redact_relation_normal_user
+      relation = create(:relation, :with_history, :version => 4)
+      relation_v3 = relation.old_relations.find_by(:version => 3)
+
+      basic_authorization create(:user).email, "test"
+
+      do_redact_relation(relation_v3, create(:redaction))
+      assert_response :forbidden, "should need to be moderator to redact."
+    end
+
+    ##
+    # test that, even as moderator, the current version of a relation
+    # can't be redacted.
+    def test_redact_relation_current_version
+      relation = create(:relation, :with_history, :version => 4)
+      relation_latest = relation.old_relations.last
+
+      basic_authorization create(:moderator_user).email, "test"
+
+      do_redact_relation(relation_latest, create(:redaction))
+      assert_response :bad_request, "shouldn't be OK to redact current version as moderator."
+    end
+
+    ##
+    # test that redacted relations aren't visible, regardless of
+    # authorisation except as moderator...
+    def test_version_redacted
+      relation = create(:relation, :with_history, :version => 2)
+      relation_v1 = relation.old_relations.find_by(:version => 1)
+      relation_v1.redact!(create(:redaction))
+
+      get :version, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
+      assert_response :forbidden, "Redacted relation shouldn't be visible via the version API."
+
+      # not even to a logged-in user
+      basic_authorization create(:user).email, "test"
+      get :version, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
+      assert_response :forbidden, "Redacted relation shouldn't be visible via the version API, even when logged in."
+    end
+
+    ##
+    # test that redacted relations aren't visible in the history
+    def test_history_redacted
+      relation = create(:relation, :with_history, :version => 2)
+      relation_v1 = relation.old_relations.find_by(:version => 1)
+      relation_v1.redact!(create(:redaction))
+
+      get :history, :params => { :id => relation_v1.relation_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 0, "redacted relation #{relation_v1.relation_id} version #{relation_v1.version} shouldn't be present in the history."
+
+      # not even to a logged-in user
+      basic_authorization create(:user).email, "test"
+      get :version, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
+      get :history, :params => { :id => relation_v1.relation_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 0, "redacted relation #{relation_v1.relation_id} version #{relation_v1.version} shouldn't be present in the history, even when logged in."
+    end
+
+    ##
+    # test the redaction of an old version of a relation, while being
+    # authorised as a moderator.
+    def test_redact_relation_moderator
+      relation = create(:relation, :with_history, :version => 4)
+      relation_v3 = relation.old_relations.find_by(:version => 3)
+
+      basic_authorization create(:moderator_user).email, "test"
+
+      do_redact_relation(relation_v3, create(:redaction))
+      assert_response :success, "should be OK to redact old version as moderator."
+
+      # check moderator can still see the redacted data, when passing
+      # the appropriate flag
+      get :version, :params => { :id => relation_v3.relation_id, :version => relation_v3.version }
+      assert_response :forbidden, "After redaction, relation should be gone for moderator, when flag not passed."
+      get :version, :params => { :id => relation_v3.relation_id, :version => relation_v3.version, :show_redactions => "true" }
+      assert_response :success, "After redaction, relation should not be gone for moderator, when flag passed."
+
+      # and when accessed via history
+      get :history, :params => { :id => relation_v3.relation_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 0, "relation #{relation_v3.relation_id} version #{relation_v3.version} should not be present in the history for moderators when not passing flag."
+      get :history, :params => { :id => relation_v3.relation_id, :show_redactions => "true" }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 1, "relation #{relation_v3.relation_id} version #{relation_v3.version} should still be present in the history for moderators when passing flag."
+    end
+
+    # testing that if the moderator drops auth, he can't see the
+    # redacted stuff any more.
+    def test_redact_relation_is_redacted
+      relation = create(:relation, :with_history, :version => 4)
+      relation_v3 = relation.old_relations.find_by(:version => 3)
+
+      basic_authorization create(:moderator_user).email, "test"
+
+      do_redact_relation(relation_v3, create(:redaction))
+      assert_response :success, "should be OK to redact old version as moderator."
+
+      # re-auth as non-moderator
+      basic_authorization create(:user).email, "test"
+
+      # check can't see the redacted data
+      get :version, :params => { :id => relation_v3.relation_id, :version => relation_v3.version }
+      assert_response :forbidden, "Redacted relation shouldn't be visible via the version API."
+
+      # and when accessed via history
+      get :history, :params => { :id => relation_v3.relation_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 0, "redacted relation #{relation_v3.relation_id} version #{relation_v3.version} shouldn't be present in the history."
+    end
+
+    ##
+    # test the unredaction of an old version of a relation, while not being
+    # authorised.
+    def test_unredact_relation_unauthorised
+      relation = create(:relation, :with_history, :version => 2)
+      relation_v1 = relation.old_relations.find_by(:version => 1)
+      relation_v1.redact!(create(:redaction))
+
+      post :redact, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
+      assert_response :unauthorized, "should need to be authenticated to unredact."
+    end
+
+    ##
+    # test the unredaction of an old version of a relation, while being
+    # authorised as a normal user.
+    def test_unredact_relation_normal_user
+      relation = create(:relation, :with_history, :version => 2)
+      relation_v1 = relation.old_relations.find_by(:version => 1)
+      relation_v1.redact!(create(:redaction))
+
+      basic_authorization create(:user).email, "test"
+
+      post :redact, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
+      assert_response :forbidden, "should need to be moderator to unredact."
+    end
+
+    ##
+    # test the unredaction of an old version of a relation, while being
+    # authorised as a moderator.
+    def test_unredact_relation_moderator
+      relation = create(:relation, :with_history, :version => 2)
+      relation_v1 = relation.old_relations.find_by(:version => 1)
+      relation_v1.redact!(create(:redaction))
+
+      basic_authorization create(:moderator_user).email, "test"
+
+      post :redact, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
+      assert_response :success, "should be OK to unredact old version as moderator."
+
+      # check moderator can still see the redacted data, without passing
+      # the appropriate flag
+      get :version, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
+      assert_response :success, "After unredaction, relation should not be gone for moderator."
+
+      # and when accessed via history
+      get :history, :params => { :id => relation_v1.relation_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 1, "relation #{relation_v1.relation_id} version #{relation_v1.version} should still be present in the history for moderators."
+
+      basic_authorization create(:user).email, "test"
+
+      # check normal user can now see the redacted data
+      get :version, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
+      assert_response :success, "After redaction, node should not be gone for normal user."
+
+      # and when accessed via history
+      get :history, :params => { :id => relation_v1.relation_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 1, "relation #{relation_v1.relation_id} version #{relation_v1.version} should still be present in the history for normal users."
+    end
+
+    private
+
+    ##
+    # check that the current version of a relation is equivalent to the
+    # version which we're getting from the versions call.
+    def check_current_version(relation_id)
+      # get the current version
+      current_relation = with_controller(RelationsController.new) do
+        get :show, :params => { :id => relation_id }
+        assert_response :success, "can't get current relation #{relation_id}"
+        Relation.from_xml(@response.body)
+      end
+      assert_not_nil current_relation, "getting relation #{relation_id} returned nil"
+
+      # get the "old" version of the relation from the version method
+      get :version, :params => { :id => relation_id, :version => current_relation.version }
+      assert_response :success, "can't get old relation #{relation_id}, v#{current_relation.version}"
+      old_relation = Relation.from_xml(@response.body)
+
+      # check that the relations are identical
+      assert_relations_are_equal current_relation, old_relation
+    end
+
+    ##
+    # look at all the versions of the relation in the history and get each version from
+    # the versions call. check that they're the same.
+    def check_history_equals_versions(relation_id)
+      get :history, :params => { :id => relation_id }
+      assert_response :success, "can't get relation #{relation_id} from API"
+      history_doc = XML::Parser.string(@response.body).parse
+      assert_not_nil history_doc, "parsing relation #{relation_id} history failed"
+
+      history_doc.find("//osm/relation").each do |relation_doc|
+        history_relation = Relation.from_xml_node(relation_doc)
+        assert_not_nil history_relation, "parsing relation #{relation_id} version failed"
+
+        get :version, :params => { :id => relation_id, :version => history_relation.version }
+        assert_response :success, "couldn't get relation #{relation_id}, v#{history_relation.version}"
+        version_relation = Relation.from_xml(@response.body)
+        assert_not_nil version_relation, "failed to parse #{relation_id}, v#{history_relation.version}"
+
+        assert_relations_are_equal history_relation, version_relation
+      end
+    end
+
+    def do_redact_relation(relation, redaction)
+      get :version, :params => { :id => relation.relation_id, :version => relation.version }
+      assert_response :success, "should be able to get version #{relation.version} of relation #{relation.relation_id}."
+
+      # now redact it
+      post :redact, :params => { :id => relation.relation_id, :version => relation.version, :redaction => redaction.id }
+    end
+  end
+end

test/controllers/api/old_ways_controller_test.rb

@@ -0,0 +1,320 @@
+require "test_helper"
+
+module Api
+  class OldWaysControllerTest < ActionController::TestCase
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/api/0.6/way/1/history", :method => :get },
+        { :controller => "api/old_ways", :action => "history", :id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/way/1/2", :method => :get },
+        { :controller => "api/old_ways", :action => "version", :id => "1", :version => "2" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/way/1/2/redact", :method => :post },
+        { :controller => "api/old_ways", :action => "redact", :id => "1", :version => "2" }
+      )
+    end
+
+    # -------------------------------------
+    # Test reading old ways.
+    # -------------------------------------
+
+    def test_history_visible
+      # check that a visible way is returned properly
+      get :history, :params => { :id => create(:way, :with_history).id }
+      assert_response :success
+    end
+
+    def test_history_invisible
+      # check that an invisible way's history is returned properly
+      get :history, :params => { :id => create(:way, :with_history, :deleted).id }
+      assert_response :success
+    end
+
+    def test_history_invalid
+      # check chat a non-existent way is not returned
+      get :history, :params => { :id => 0 }
+      assert_response :not_found
+    end
+
+    ##
+    # check that we can retrieve versions of a way
+    def test_version
+      way = create(:way, :with_history)
+      used_way = create(:way, :with_history)
+      create(:relation_member, :member => used_way)
+      way_with_versions = create(:way, :with_history, :version => 4)
+
+      create(:way_tag, :way => way)
+      create(:way_tag, :way => used_way)
+      create(:way_tag, :way => way_with_versions)
+      propagate_tags(way, way.old_ways.last)
+      propagate_tags(used_way, used_way.old_ways.last)
+      propagate_tags(way_with_versions, way_with_versions.old_ways.last)
+
+      check_current_version(way.id)
+      check_current_version(used_way.id)
+      check_current_version(way_with_versions.id)
+    end
+
+    ##
+    # check that returned history is the same as getting all
+    # versions of a way from the api.
+    def test_history_equals_versions
+      way = create(:way, :with_history)
+      used_way = create(:way, :with_history)
+      create(:relation_member, :member => used_way)
+      way_with_versions = create(:way, :with_history, :version => 4)
+
+      check_history_equals_versions(way.id)
+      check_history_equals_versions(used_way.id)
+      check_history_equals_versions(way_with_versions.id)
+    end
+
+    ##
+    # test the redaction of an old version of a way, while not being
+    # authorised.
+    def test_redact_way_unauthorised
+      way = create(:way, :with_history, :version => 4)
+      way_v3 = way.old_ways.find_by(:version => 3)
+
+      do_redact_way(way_v3, create(:redaction))
+      assert_response :unauthorized, "should need to be authenticated to redact."
+    end
+
+    ##
+    # test the redaction of an old version of a way, while being
+    # authorised as a normal user.
+    def test_redact_way_normal_user
+      basic_authorization create(:user).email, "test"
+      way = create(:way, :with_history, :version => 4)
+      way_v3 = way.old_ways.find_by(:version => 3)
+
+      do_redact_way(way_v3, create(:redaction))
+      assert_response :forbidden, "should need to be moderator to redact."
+    end
+
+    ##
+    # test that, even as moderator, the current version of a way
+    # can't be redacted.
+    def test_redact_way_current_version
+      basic_authorization create(:moderator_user).email, "test"
+      way = create(:way, :with_history, :version => 4)
+      way_latest = way.old_ways.last
+
+      do_redact_way(way_latest, create(:redaction))
+      assert_response :bad_request, "shouldn't be OK to redact current version as moderator."
+    end
+
+    ##
+    # test that redacted ways aren't visible, regardless of
+    # authorisation except as moderator...
+    def test_version_redacted
+      way = create(:way, :with_history, :version => 2)
+      way_v1 = way.old_ways.find_by(:version => 1)
+      way_v1.redact!(create(:redaction))
+
+      get :version, :params => { :id => way_v1.way_id, :version => way_v1.version }
+      assert_response :forbidden, "Redacted way shouldn't be visible via the version API."
+
+      # not even to a logged-in user
+      basic_authorization create(:user).email, "test"
+      get :version, :params => { :id => way_v1.way_id, :version => way_v1.version }
+      assert_response :forbidden, "Redacted way shouldn't be visible via the version API, even when logged in."
+    end
+
+    ##
+    # test that redacted ways aren't visible in the history
+    def test_history_redacted
+      way = create(:way, :with_history, :version => 2)
+      way_v1 = way.old_ways.find_by(:version => 1)
+      way_v1.redact!(create(:redaction))
+
+      get :history, :params => { :id => way_v1.way_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 0, "redacted way #{way_v1.way_id} version #{way_v1.version} shouldn't be present in the history."
+
+      # not even to a logged-in user
+      basic_authorization create(:user).email, "test"
+      get :version, :params => { :id => way_v1.way_id, :version => way_v1.version }
+      get :history, :params => { :id => way_v1.way_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 0, "redacted node #{way_v1.way_id} version #{way_v1.version} shouldn't be present in the history, even when logged in."
+    end
+
+    ##
+    # test the redaction of an old version of a way, while being
+    # authorised as a moderator.
+    def test_redact_way_moderator
+      way = create(:way, :with_history, :version => 4)
+      way_v3 = way.old_ways.find_by(:version => 3)
+      basic_authorization create(:moderator_user).email, "test"
+
+      do_redact_way(way_v3, create(:redaction))
+      assert_response :success, "should be OK to redact old version as moderator."
+
+      # check moderator can still see the redacted data, when passing
+      # the appropriate flag
+      get :version, :params => { :id => way_v3.way_id, :version => way_v3.version }
+      assert_response :forbidden, "After redaction, node should be gone for moderator, when flag not passed."
+      get :version, :params => { :id => way_v3.way_id, :version => way_v3.version, :show_redactions => "true" }
+      assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
+
+      # and when accessed via history
+      get :history, :params => { :id => way_v3.way_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 0, "way #{way_v3.way_id} version #{way_v3.version} should not be present in the history for moderators when not passing flag."
+      get :history, :params => { :id => way_v3.way_id, :show_redactions => "true" }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 1, "way #{way_v3.way_id} version #{way_v3.version} should still be present in the history for moderators when passing flag."
+    end
+
+    # testing that if the moderator drops auth, he can't see the
+    # redacted stuff any more.
+    def test_redact_way_is_redacted
+      way = create(:way, :with_history, :version => 4)
+      way_v3 = way.old_ways.find_by(:version => 3)
+      basic_authorization create(:moderator_user).email, "test"
+
+      do_redact_way(way_v3, create(:redaction))
+      assert_response :success, "should be OK to redact old version as moderator."
+
+      # re-auth as non-moderator
+      basic_authorization create(:user).email, "test"
+
+      # check can't see the redacted data
+      get :version, :params => { :id => way_v3.way_id, :version => way_v3.version }
+      assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
+
+      # and when accessed via history
+      get :history, :params => { :id => way_v3.way_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 0, "redacted way #{way_v3.way_id} version #{way_v3.version} shouldn't be present in the history."
+    end
+
+    ##
+    # test the unredaction of an old version of a way, while not being
+    # authorised.
+    def test_unredact_way_unauthorised
+      way = create(:way, :with_history, :version => 2)
+      way_v1 = way.old_ways.find_by(:version => 1)
+      way_v1.redact!(create(:redaction))
+
+      post :redact, :params => { :id => way_v1.way_id, :version => way_v1.version }
+      assert_response :unauthorized, "should need to be authenticated to unredact."
+    end
+
+    ##
+    # test the unredaction of an old version of a way, while being
+    # authorised as a normal user.
+    def test_unredact_way_normal_user
+      way = create(:way, :with_history, :version => 2)
+      way_v1 = way.old_ways.find_by(:version => 1)
+      way_v1.redact!(create(:redaction))
+
+      basic_authorization create(:user).email, "test"
+
+      post :redact, :params => { :id => way_v1.way_id, :version => way_v1.version }
+      assert_response :forbidden, "should need to be moderator to unredact."
+    end
+
+    ##
+    # test the unredaction of an old version of a way, while being
+    # authorised as a moderator.
+    def test_unredact_way_moderator
+      moderator_user = create(:moderator_user)
+      way = create(:way, :with_history, :version => 2)
+      way_v1 = way.old_ways.find_by(:version => 1)
+      way_v1.redact!(create(:redaction))
+
+      basic_authorization moderator_user.email, "test"
+
+      post :redact, :params => { :id => way_v1.way_id, :version => way_v1.version }
+      assert_response :success, "should be OK to unredact old version as moderator."
+
+      # check moderator can still see the unredacted data, without passing
+      # the appropriate flag
+      get :version, :params => { :id => way_v1.way_id, :version => way_v1.version }
+      assert_response :success, "After unredaction, node should not be gone for moderator."
+
+      # and when accessed via history
+      get :history, :params => { :id => way_v1.way_id }
+      assert_response :success, "Unredaction shouldn't have stopped history working."
+      assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 1, "way #{way_v1.way_id} version #{way_v1.version} should still be present in the history for moderators."
+
+      basic_authorization create(:user).email, "test"
+
+      # check normal user can now see the unredacted data
+      get :version, :params => { :id => way_v1.way_id, :version => way_v1.version }
+      assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
+
+      # and when accessed via history
+      get :history, :params => { :id => way_v1.way_id }
+      assert_response :success, "Redaction shouldn't have stopped history working."
+      assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 1, "way #{way_v1.way_id} version #{way_v1.version} should still be present in the history for normal users."
+    end
+
+    private
+
+    ##
+    # check that the current version of a way is equivalent to the
+    # version which we're getting from the versions call.
+    def check_current_version(way_id)
+      # get the current version
+      current_way = with_controller(WaysController.new) do
+        get :show, :params => { :id => way_id }
+        assert_response :success, "can't get current way #{way_id}"
+        Way.from_xml(@response.body)
+      end
+      assert_not_nil current_way, "getting way #{way_id} returned nil"
+
+      # get the "old" version of the way from the version method
+      get :version, :params => { :id => way_id, :version => current_way.version }
+      assert_response :success, "can't get old way #{way_id}, v#{current_way.version}"
+      old_way = Way.from_xml(@response.body)
+
+      # check that the ways are identical
+      assert_ways_are_equal current_way, old_way
+    end
+
+    ##
+    # look at all the versions of the way in the history and get each version from
+    # the versions call. check that they're the same.
+    def check_history_equals_versions(way_id)
+      get :history, :params => { :id => way_id }
+      assert_response :success, "can't get way #{way_id} from API"
+      history_doc = XML::Parser.string(@response.body).parse
+      assert_not_nil history_doc, "parsing way #{way_id} history failed"
+
+      history_doc.find("//osm/way").each do |way_doc|
+        history_way = Way.from_xml_node(way_doc)
+        assert_not_nil history_way, "parsing way #{way_id} version failed"
+
+        get :version, :params => { :id => way_id, :version => history_way.version }
+        assert_response :success, "couldn't get way #{way_id}, v#{history_way.version}"
+        version_way = Way.from_xml(@response.body)
+        assert_not_nil version_way, "failed to parse #{way_id}, v#{history_way.version}"
+
+        assert_ways_are_equal history_way, version_way
+      end
+    end
+
+    def do_redact_way(way, redaction)
+      get :version, :params => { :id => way.way_id, :version => way.version }
+      assert_response :success, "should be able to get version #{way.version} of way #{way.way_id}."
+
+      # now redact it
+      post :redact, :params => { :id => way.way_id, :version => way.version, :redaction => redaction.id }
+    end
+
+    def propagate_tags(way, old_way)
+      way.tags.each do |k, v|
+        create(:old_way_tag, :old_way => old_way, :k => k, :v => v)
+      end
+    end
+  end
+end

test/controllers/api/ways_controller_test.rb

@@ -0,0 +1,754 @@
+require "test_helper"
+
+module Api
+  class WaysControllerTest < ActionController::TestCase
+    ##
+    # test all routes which lead to this controller
+    def test_routes
+      assert_routing(
+        { :path => "/api/0.6/way/create", :method => :put },
+        { :controller => "api/ways", :action => "create" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/way/1/full", :method => :get },
+        { :controller => "api/ways", :action => "full", :id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/way/1", :method => :get },
+        { :controller => "api/ways", :action => "show", :id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/way/1", :method => :put },
+        { :controller => "api/ways", :action => "update", :id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/way/1", :method => :delete },
+        { :controller => "api/ways", :action => "delete", :id => "1" }
+      )
+      assert_routing(
+        { :path => "/api/0.6/ways", :method => :get },
+        { :controller => "api/ways", :action => "index" }
+      )
+    end
+
+    # -------------------------------------
+    # Test showing ways.
+    # -------------------------------------
+
+    def test_show
+      # check that a visible way is returned properly
+      get :show, :params => { :id => create(:way).id }
+      assert_response :success
+
+      # check that an invisible way is not returned
+      get :show, :params => { :id => create(:way, :deleted).id }
+      assert_response :gone
+
+      # check chat a non-existent way is not returned
+      get :show, :params => { :id => 0 }
+      assert_response :not_found
+    end
+
+    ##
+    # check the "full" mode
+    def test_full
+      Way.all.each do |way|
+        get :full, :params => { :id => way.id }
+
+        # full call should say "gone" for non-visible ways...
+        unless way.visible
+          assert_response :gone
+          next
+        end
+
+        # otherwise it should say success
+        assert_response :success
+
+        # Check the way is correctly returned
+        assert_select "osm way[id='#{way.id}'][version='#{way.version}'][visible='#{way.visible}']", 1
+
+        # check that each node in the way appears once in the output as a
+        # reference and as the node element.
+        way.nodes.each do |n|
+          count = (way.nodes - (way.nodes - [n])).length
+          assert_select "osm way nd[ref='#{n.id}']", count
+          assert_select "osm node[id='#{n.id}'][version='#{n.version}'][lat='#{format('%.7f', n.lat)}'][lon='#{format('%.7f', n.lon)}']", 1
+        end
+      end
+    end
+
+    ##
+    # test fetching multiple ways
+    def test_index
+      way1 = create(:way)
+      way2 = create(:way, :deleted)
+      way3 = create(:way)
+      way4 = create(:way)
+
+      # check error when no parameter provided
+      get :index
+      assert_response :bad_request
+
+      # check error when no parameter value provided
+      get :index, :params => { :ways => "" }
+      assert_response :bad_request
+
+      # test a working call
+      get :index, :params => { :ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id}" }
+      assert_response :success
+      assert_select "osm" do
+        assert_select "way", :count => 4
+        assert_select "way[id='#{way1.id}'][visible='true']", :count => 1
+        assert_select "way[id='#{way2.id}'][visible='false']", :count => 1
+        assert_select "way[id='#{way3.id}'][visible='true']", :count => 1
+        assert_select "way[id='#{way4.id}'][visible='true']", :count => 1
+      end
+
+      # check error when a non-existent way is included
+      get :index, :params => { :ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id},0" }
+      assert_response :not_found
+    end
+
+    # -------------------------------------
+    # Test simple way creation.
+    # -------------------------------------
+
+    def test_create
+      node1 = create(:node)
+      node2 = create(:node)
+      private_user = create(:user, :data_public => false)
+      private_changeset = create(:changeset, :user => private_user)
+      user = create(:user)
+      changeset = create(:changeset, :user => user)
+
+      ## First check that it fails when creating a way using a non-public user
+      basic_authorization private_user.email, "test"
+
+      # use the first user's open changeset
+      changeset_id = private_changeset.id
+
+      # create a way with pre-existing nodes
+      xml = "<osm><way changeset='#{changeset_id}'>" \
+            "<nd ref='#{node1.id}'/><nd ref='#{node2.id}'/>" \
+            "<tag k='test' v='yes' /></way></osm>"
+      put :create, :body => xml
+      # hope for failure
+      assert_response :forbidden,
+                      "way upload did not return forbidden status"
+
+      ## Now use a public user
+      basic_authorization user.email, "test"
+
+      # use the first user's open changeset
+      changeset_id = changeset.id
+
+      # create a way with pre-existing nodes
+      xml = "<osm><way changeset='#{changeset_id}'>" \
+            "<nd ref='#{node1.id}'/><nd ref='#{node2.id}'/>" \
+            "<tag k='test' v='yes' /></way></osm>"
+      put :create, :body => xml
+      # hope for success
+      assert_response :success,
+                      "way upload did not return success status"
+      # read id of created way and search for it
+      wayid = @response.body
+      checkway = Way.find(wayid)
+      assert_not_nil checkway,
+                     "uploaded way not found in data base after upload"
+      # compare values
+      assert_equal checkway.nds.length, 2,
+                   "saved way does not contain exactly one node"
+      assert_equal checkway.nds[0], node1.id,
+                   "saved way does not contain the right node on pos 0"
+      assert_equal checkway.nds[1], node2.id,
+                   "saved way does not contain the right node on pos 1"
+      assert_equal checkway.changeset_id, changeset_id,
+                   "saved way does not belong to the correct changeset"
+      assert_equal user.id, checkway.changeset.user_id,
+                   "saved way does not belong to user that created it"
+      assert_equal true, checkway.visible,
+                   "saved way is not visible"
+    end
+
+    # -------------------------------------
+    # Test creating some invalid ways.
+    # -------------------------------------
+
+    def test_create_invalid
+      node = create(:node)
+      private_user = create(:user, :data_public => false)
+      private_open_changeset = create(:changeset, :user => private_user)
+      private_closed_changeset = create(:changeset, :closed, :user => private_user)
+      user = create(:user)
+      open_changeset = create(:changeset, :user => user)
+      closed_changeset = create(:changeset, :closed, :user => user)
+
+      ## First test with a private user to make sure that they are not authorized
+      basic_authorization private_user.email, "test"
+
+      # use the first user's open changeset
+      # create a way with non-existing node
+      xml = "<osm><way changeset='#{private_open_changeset.id}'>" \
+            "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
+      put :create, :body => xml
+      # expect failure
+      assert_response :forbidden,
+                      "way upload with invalid node using a private user did not return 'forbidden'"
+
+      # create a way with no nodes
+      xml = "<osm><way changeset='#{private_open_changeset.id}'>" \
+            "<tag k='test' v='yes' /></way></osm>"
+      put :create, :body => xml
+      # expect failure
+      assert_response :forbidden,
+                      "way upload with no node using a private userdid not return 'forbidden'"
+
+      # create a way inside a closed changeset
+      xml = "<osm><way changeset='#{private_closed_changeset.id}'>" \
+            "<nd ref='#{node.id}'/></way></osm>"
+      put :create, :body => xml
+      # expect failure
+      assert_response :forbidden,
+                      "way upload to closed changeset with a private user did not return 'forbidden'"
+
+      ## Now test with a public user
+      basic_authorization user.email, "test"
+
+      # use the first user's open changeset
+      # create a way with non-existing node
+      xml = "<osm><way changeset='#{open_changeset.id}'>" \
+            "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
+      put :create, :body => xml
+      # expect failure
+      assert_response :precondition_failed,
+                      "way upload with invalid node did not return 'precondition failed'"
+      assert_equal "Precondition failed: Way  requires the nodes with id in (0), which either do not exist, or are not visible.", @response.body
+
+      # create a way with no nodes
+      xml = "<osm><way changeset='#{open_changeset.id}'>" \
+            "<tag k='test' v='yes' /></way></osm>"
+      put :create, :body => xml
+      # expect failure
+      assert_response :precondition_failed,
+                      "way upload with no node did not return 'precondition failed'"
+      assert_equal "Precondition failed: Cannot create way: data is invalid.", @response.body
+
+      # create a way inside a closed changeset
+      xml = "<osm><way changeset='#{closed_changeset.id}'>" \
+            "<nd ref='#{node.id}'/></way></osm>"
+      put :create, :body => xml
+      # expect failure
+      assert_response :conflict,
+                      "way upload to closed changeset did not return 'conflict'"
+
+      # create a way with a tag which is too long
+      xml = "<osm><way changeset='#{open_changeset.id}'>" \
+            "<nd ref='#{node.id}'/>" \
+            "<tag k='foo' v='#{'x' * 256}'/>" \
+            "</way></osm>"
+      put :create, :body => xml
+      # expect failure
+      assert_response :bad_request,
+                      "way upload to with too long tag did not return 'bad_request'"
+    end
+
+    # -------------------------------------
+    # Test deleting ways.
+    # -------------------------------------
+
+    def test_delete
+      private_user = create(:user, :data_public => false)
+      private_open_changeset = create(:changeset, :user => private_user)
+      private_closed_changeset = create(:changeset, :closed, :user => private_user)
+      private_way = create(:way, :changeset => private_open_changeset)
+      private_deleted_way = create(:way, :deleted, :changeset => private_open_changeset)
+      private_used_way = create(:way, :changeset => private_open_changeset)
+      create(:relation_member, :member => private_used_way)
+      user = create(:user)
+      open_changeset = create(:changeset, :user => user)
+      closed_changeset = create(:changeset, :closed, :user => user)
+      way = create(:way, :changeset => open_changeset)
+      deleted_way = create(:way, :deleted, :changeset => open_changeset)
+      used_way = create(:way, :changeset => open_changeset)
+      relation_member = create(:relation_member, :member => used_way)
+      relation = relation_member.relation
+
+      # first try to delete way without auth
+      delete :delete, :params => { :id => way.id }
+      assert_response :unauthorized
+
+      # now set auth using the private user
+      basic_authorization private_user.email, "test"
+
+      # this shouldn't work as with the 0.6 api we need pay load to delete
+      delete :delete, :params => { :id => private_way.id }
+      assert_response :forbidden
+
+      # Now try without having a changeset
+      xml = "<osm><way id='#{private_way.id}'/></osm>"
+      delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
+      assert_response :forbidden
+
+      # try to delete with an invalid (closed) changeset
+      xml = update_changeset(private_way.to_xml, private_closed_changeset.id)
+      delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
+      assert_response :forbidden
+
+      # try to delete with an invalid (non-existent) changeset
+      xml = update_changeset(private_way.to_xml, 0)
+      delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
+      assert_response :forbidden
+
+      # Now try with a valid changeset
+      xml = private_way.to_xml
+      delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
+      assert_response :forbidden
+
+      # check the returned value - should be the new version number
+      # valid delete should return the new version number, which should
+      # be greater than the old version number
+      # assert @response.body.to_i > current_ways(:visible_way).version,
+      #   "delete request should return a new version number for way"
+
+      # this won't work since the way is already deleted
+      xml = private_deleted_way.to_xml
+      delete :delete, :params => { :id => private_deleted_way.id }, :body => xml.to_s
+      assert_response :forbidden
+
+      # this shouldn't work as the way is used in a relation
+      xml = private_used_way.to_xml
+      delete :delete, :params => { :id => private_used_way.id }, :body => xml.to_s
+      assert_response :forbidden,
+                      "shouldn't be able to delete a way used in a relation (#{@response.body}), when done by a private user"
+
+      # this won't work since the way never existed
+      delete :delete, :params => { :id => 0 }
+      assert_response :forbidden
+
+      ### Now check with a public user
+      # now set auth
+      basic_authorization user.email, "test"
+
+      # this shouldn't work as with the 0.6 api we need pay load to delete
+      delete :delete, :params => { :id => way.id }
+      assert_response :bad_request
+
+      # Now try without having a changeset
+      xml = "<osm><way id='#{way.id}'/></osm>"
+      delete :delete, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :bad_request
+
+      # try to delete with an invalid (closed) changeset
+      xml = update_changeset(way.to_xml, closed_changeset.id)
+      delete :delete, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :conflict
+
+      # try to delete with an invalid (non-existent) changeset
+      xml = update_changeset(way.to_xml, 0)
+      delete :delete, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :conflict
+
+      # Now try with a valid changeset
+      xml = way.to_xml
+      delete :delete, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :success
+
+      # check the returned value - should be the new version number
+      # valid delete should return the new version number, which should
+      # be greater than the old version number
+      assert @response.body.to_i > way.version,
+             "delete request should return a new version number for way"
+
+      # this won't work since the way is already deleted
+      xml = deleted_way.to_xml
+      delete :delete, :params => { :id => deleted_way.id }, :body => xml.to_s
+      assert_response :gone
+
+      # this shouldn't work as the way is used in a relation
+      xml = used_way.to_xml
+      delete :delete, :params => { :id => used_way.id }, :body => xml.to_s
+      assert_response :precondition_failed,
+                      "shouldn't be able to delete a way used in a relation (#{@response.body})"
+      assert_equal "Precondition failed: Way #{used_way.id} is still used by relations #{relation.id}.", @response.body
+
+      # this won't work since the way never existed
+      delete :delete, :params => { :id => 0 }
+      assert_response :not_found
+    end
+
+    ##
+    # tests whether the API works and prevents incorrect use while trying
+    # to update ways.
+    def test_update
+      private_user = create(:user, :data_public => false)
+      private_way = create(:way, :changeset => create(:changeset, :user => private_user))
+      user = create(:user)
+      way = create(:way, :changeset => create(:changeset, :user => user))
+      node = create(:node)
+      create(:way_node, :way => private_way, :node => node)
+      create(:way_node, :way => way, :node => node)
+
+      ## First test with no user credentials
+      # try and update a way without authorisation
+      xml = way.to_xml
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :unauthorized
+
+      ## Second test with the private user
+
+      # setup auth
+      basic_authorization private_user.email, "test"
+
+      ## trying to break changesets
+
+      # try and update in someone else's changeset
+      xml = update_changeset(private_way.to_xml,
+                             create(:changeset).id)
+      put :update, :params => { :id => private_way.id }, :body => xml.to_s
+      assert_require_public_data "update with other user's changeset should be forbidden when date isn't public"
+
+      # try and update in a closed changeset
+      xml = update_changeset(private_way.to_xml,
+                             create(:changeset, :closed, :user => private_user).id)
+      put :update, :params => { :id => private_way.id }, :body => xml.to_s
+      assert_require_public_data "update with closed changeset should be forbidden, when data isn't public"
+
+      # try and update in a non-existant changeset
+      xml = update_changeset(private_way.to_xml, 0)
+      put :update, :params => { :id => private_way.id }, :body => xml.to_s
+      assert_require_public_data("update with changeset=0 should be forbidden, when data isn't public")
+
+      ## try and submit invalid updates
+      xml = xml_replace_node(private_way.to_xml, node.id, 9999)
+      put :update, :params => { :id => private_way.id }, :body => xml.to_s
+      assert_require_public_data "way with non-existent node should be forbidden, when data isn't public"
+
+      xml = xml_replace_node(private_way.to_xml, node.id, create(:node, :deleted).id)
+      put :update, :params => { :id => private_way.id }, :body => xml.to_s
+      assert_require_public_data "way with deleted node should be forbidden, when data isn't public"
+
+      ## finally, produce a good request which will still not work
+      xml = private_way.to_xml
+      put :update, :params => { :id => private_way.id }, :body => xml.to_s
+      assert_require_public_data "should have failed with a forbidden when data isn't public"
+
+      ## Finally test with the public user
+
+      # setup auth
+      basic_authorization user.email, "test"
+
+      ## trying to break changesets
+
+      # try and update in someone else's changeset
+      xml = update_changeset(way.to_xml,
+                             create(:changeset).id)
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :conflict, "update with other user's changeset should be rejected"
+
+      # try and update in a closed changeset
+      xml = update_changeset(way.to_xml,
+                             create(:changeset, :closed, :user => user).id)
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :conflict, "update with closed changeset should be rejected"
+
+      # try and update in a non-existant changeset
+      xml = update_changeset(way.to_xml, 0)
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :conflict, "update with changeset=0 should be rejected"
+
+      ## try and submit invalid updates
+      xml = xml_replace_node(way.to_xml, node.id, 9999)
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :precondition_failed, "way with non-existent node should be rejected"
+
+      xml = xml_replace_node(way.to_xml, node.id, create(:node, :deleted).id)
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :precondition_failed, "way with deleted node should be rejected"
+
+      ## next, attack the versioning
+      current_way_version = way.version
+
+      # try and submit a version behind
+      xml = xml_attr_rewrite(way.to_xml,
+                             "version", current_way_version - 1)
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :conflict, "should have failed on old version number"
+
+      # try and submit a version ahead
+      xml = xml_attr_rewrite(way.to_xml,
+                             "version", current_way_version + 1)
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :conflict, "should have failed on skipped version number"
+
+      # try and submit total crap in the version field
+      xml = xml_attr_rewrite(way.to_xml,
+                             "version", "p1r4t3s!")
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :conflict,
+                      "should not be able to put 'p1r4at3s!' in the version field"
+
+      ## try an update with the wrong ID
+      xml = create(:way).to_xml
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :bad_request,
+                      "should not be able to update a way with a different ID from the XML"
+
+      ## try an update with a minimal valid XML doc which isn't a well-formed OSM doc.
+      xml = "<update/>"
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :bad_request,
+                      "should not be able to update a way with non-OSM XML doc."
+
+      ## finally, produce a good request which should work
+      xml = way.to_xml
+      put :update, :params => { :id => way.id }, :body => xml.to_s
+      assert_response :success, "a valid update request failed"
+    end
+
+    # ------------------------------------------------------------
+    # test tags handling
+    # ------------------------------------------------------------
+
+    ##
+    # Try adding a new tag to a way
+    def test_add_tags
+      private_user = create(:user, :data_public => false)
+      private_way = create(:way_with_nodes, :nodes_count => 2, :changeset => create(:changeset, :user => private_user))
+      user = create(:user)
+      way = create(:way_with_nodes, :nodes_count => 2, :changeset => create(:changeset, :user => user))
+
+      ## Try with the non-public user
+      # setup auth
+      basic_authorization private_user.email, "test"
+
+      # add an identical tag to the way
+      tag_xml = XML::Node.new("tag")
+      tag_xml["k"] = "new"
+      tag_xml["v"] = "yes"
+
+      # add the tag into the existing xml
+      way_xml = private_way.to_xml
+      way_xml.find("//osm/way").first << tag_xml
+
+      # try and upload it
+      put :update, :params => { :id => private_way.id }, :body => way_xml.to_s
+      assert_response :forbidden,
+                      "adding a duplicate tag to a way for a non-public should fail with 'forbidden'"
+
+      ## Now try with the public user
+      # setup auth
+      basic_authorization user.email, "test"
+
+      # add an identical tag to the way
+      tag_xml = XML::Node.new("tag")
+      tag_xml["k"] = "new"
+      tag_xml["v"] = "yes"
+
+      # add the tag into the existing xml
+      way_xml = way.to_xml
+      way_xml.find("//osm/way").first << tag_xml
+
+      # try and upload it
+      put :update, :params => { :id => way.id }, :body => way_xml.to_s
+      assert_response :success,
+                      "adding a new tag to a way should succeed"
+      assert_equal way.version + 1, @response.body.to_i
+    end
+
+    ##
+    # Try adding a duplicate of an existing tag to a way
+    def test_add_duplicate_tags
+      private_user = create(:user, :data_public => false)
+      private_way = create(:way, :changeset => create(:changeset, :user => private_user))
+      private_existing_tag = create(:way_tag, :way => private_way)
+      user = create(:user)
+      way = create(:way, :changeset => create(:changeset, :user => user))
+      existing_tag = create(:way_tag, :way => way)
+
+      ## Try with the non-public user
+      # setup auth
+      basic_authorization private_user.email, "test"
+
+      # add an identical tag to the way
+      tag_xml = XML::Node.new("tag")
+      tag_xml["k"] = private_existing_tag.k
+      tag_xml["v"] = private_existing_tag.v
+
+      # add the tag into the existing xml
+      way_xml = private_way.to_xml
+      way_xml.find("//osm/way").first << tag_xml
+
+      # try and upload it
+      put :update, :params => { :id => private_way.id }, :body => way_xml.to_s
+      assert_response :forbidden,
+                      "adding a duplicate tag to a way for a non-public should fail with 'forbidden'"
+
+      ## Now try with the public user
+      # setup auth
+      basic_authorization user.email, "test"
+
+      # add an identical tag to the way
+      tag_xml = XML::Node.new("tag")
+      tag_xml["k"] = existing_tag.k
+      tag_xml["v"] = existing_tag.v
+
+      # add the tag into the existing xml
+      way_xml = way.to_xml
+      way_xml.find("//osm/way").first << tag_xml
+
+      # try and upload it
+      put :update, :params => { :id => way.id }, :body => way_xml.to_s
+      assert_response :bad_request,
+                      "adding a duplicate tag to a way should fail with 'bad request'"
+      assert_equal "Element way/#{way.id} has duplicate tags with key #{existing_tag.k}", @response.body
+    end
+
+    ##
+    # Try adding a new duplicate tags to a way
+    def test_new_duplicate_tags
+      private_user = create(:user, :data_public => false)
+      private_way = create(:way, :changeset => create(:changeset, :user => private_user))
+      user = create(:user)
+      way = create(:way, :changeset => create(:changeset, :user => user))
+
+      ## First test with the non-public user so should be rejected
+      # setup auth
+      basic_authorization private_user.email, "test"
+
+      # create duplicate tag
+      tag_xml = XML::Node.new("tag")
+      tag_xml["k"] = "i_am_a_duplicate"
+      tag_xml["v"] = "foobar"
+
+      # add the tag into the existing xml
+      way_xml = private_way.to_xml
+
+      # add two copies of the tag
+      way_xml.find("//osm/way").first << tag_xml.copy(true) << tag_xml
+
+      # try and upload it
+      put :update, :params => { :id => private_way.id }, :body => way_xml.to_s
+      assert_response :forbidden,
+                      "adding new duplicate tags to a way using a non-public user should fail with 'forbidden'"
+
+      ## Now test with the public user
+      # setup auth
+      basic_authorization user.email, "test"
+
+      # create duplicate tag
+      tag_xml = XML::Node.new("tag")
+      tag_xml["k"] = "i_am_a_duplicate"
+      tag_xml["v"] = "foobar"
+
+      # add the tag into the existing xml
+      way_xml = way.to_xml
+
+      # add two copies of the tag
+      way_xml.find("//osm/way").first << tag_xml.copy(true) << tag_xml
+
+      # try and upload it
+      put :update, :params => { :id => way.id }, :body => way_xml.to_s
+      assert_response :bad_request,
+                      "adding new duplicate tags to a way should fail with 'bad request'"
+      assert_equal "Element way/#{way.id} has duplicate tags with key i_am_a_duplicate", @response.body
+    end
+
+    ##
+    # Try adding a new duplicate tags to a way.
+    # But be a bit subtle - use unicode decoding ambiguities to use different
+    # binary strings which have the same decoding.
+    def test_invalid_duplicate_tags
+      private_user = create(:user, :data_public => false)
+      private_changeset = create(:changeset, :user => private_user)
+      user = create(:user)
+      changeset = create(:changeset, :user => user)
+
+      ## First make sure that you can't with a non-public user
+      # setup auth
+      basic_authorization private_user.email, "test"
+
+      # add the tag into the existing xml
+      way_str = "<osm><way changeset='#{private_changeset.id}'>"
+      way_str << "<tag k='addr:housenumber' v='1'/>"
+      way_str << "<tag k='addr:housenumber' v='2'/>"
+      way_str << "</way></osm>"
+
+      # try and upload it
+      put :create, :body => way_str
+      assert_response :forbidden,
+                      "adding new duplicate tags to a way with a non-public user should fail with 'forbidden'"
+
+      ## Now do it with a public user
+      # setup auth
+      basic_authorization user.email, "test"
+
+      # add the tag into the existing xml
+      way_str = "<osm><way changeset='#{changeset.id}'>"
+      way_str << "<tag k='addr:housenumber' v='1'/>"
+      way_str << "<tag k='addr:housenumber' v='2'/>"
+      way_str << "</way></osm>"
+
+      # try and upload it
+      put :create, :body => way_str
+      assert_response :bad_request,
+                      "adding new duplicate tags to a way should fail with 'bad request'"
+      assert_equal "Element way/ has duplicate tags with key addr:housenumber", @response.body
+    end
+
+    ##
+    # test that a call to ways_for_node returns all ways that contain the node
+    # and none that don't.
+    def test_ways_for_node
+      node = create(:node)
+      way1 = create(:way)
+      way2 = create(:way)
+      create(:way_node, :way => way1, :node => node)
+      create(:way_node, :way => way2, :node => node)
+      # create an unrelated way
+      create(:way_with_nodes, :nodes_count => 2)
+      # create a way which used to use the node
+      way3_v1 = create(:old_way, :version => 1)
+      _way3_v2 = create(:old_way, :current_way => way3_v1.current_way, :version => 2)
+      create(:old_way_node, :old_way => way3_v1, :node => node)
+
+      get :ways_for_node, :params => { :id => node.id }
+      assert_response :success
+      ways_xml = XML::Parser.string(@response.body).parse
+      assert_not_nil ways_xml, "failed to parse ways_for_node response"
+
+      # check that the set of IDs match expectations
+      expected_way_ids = [way1.id,
+                          way2.id]
+      found_way_ids = ways_xml.find("//osm/way").collect { |w| w["id"].to_i }
+      assert_equal expected_way_ids.sort, found_way_ids.sort,
+                   "expected ways for node #{node.id} did not match found"
+
+      # check the full ways to ensure we're not missing anything
+      expected_way_ids.each do |id|
+        way_xml = ways_xml.find("//osm/way[@id='#{id}']").first
+        assert_ways_are_equal(Way.find(id),
+                              Way.from_xml_node(way_xml))
+      end
+    end
+
+    ##
+    # update the changeset_id of a way element
+    def update_changeset(xml, changeset_id)
+      xml_attr_rewrite(xml, "changeset", changeset_id)
+    end
+
+    ##
+    # update an attribute in the way element
+    def xml_attr_rewrite(xml, name, value)
+      xml.find("//osm/way").first[name] = value.to_s
+      xml
+    end
+
+    ##
+    # replace a node in a way element
+    def xml_replace_node(xml, old_node, new_node)
+      xml.find("//osm/way/nd[@ref='#{old_node}']").first["ref"] = new_node.to_s
+      xml
+    end
+  end
+end

test/controllers/nodes_controller_test.rb

@@ -1,550 +0,0 @@
-require "test_helper"
-
-class NodesControllerTest < ActionController::TestCase
-  ##
-  # test all routes which lead to this controller
-  def test_routes
-    assert_routing(
-      { :path => "/api/0.6/node/create", :method => :put },
-      { :controller => "nodes", :action => "create" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/node/1", :method => :get },
-      { :controller => "nodes", :action => "show", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/node/1", :method => :put },
-      { :controller => "nodes", :action => "update", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/node/1", :method => :delete },
-      { :controller => "nodes", :action => "delete", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/nodes", :method => :get },
-      { :controller => "nodes", :action => "index" }
-    )
-  end
-
-  def test_create
-    private_user = create(:user, :data_public => false)
-    private_changeset = create(:changeset, :user => private_user)
-    user = create(:user)
-    changeset = create(:changeset, :user => user)
-
-    # create a node with random lat/lon
-    lat = rand(-50..50) + rand
-    lon = rand(-50..50) + rand
-
-    ## First try with no auth
-    # create a minimal xml file
-    xml = "<osm><node lat='#{lat}' lon='#{lon}' changeset='#{changeset.id}'/></osm>"
-    assert_difference("OldNode.count", 0) do
-      put :create, :body => xml
-    end
-    # hope for unauthorized
-    assert_response :unauthorized, "node upload did not return unauthorized status"
-
-    ## Now try with the user which doesn't have their data public
-    basic_authorization private_user.email, "test"
-
-    # create a minimal xml file
-    xml = "<osm><node lat='#{lat}' lon='#{lon}' changeset='#{private_changeset.id}'/></osm>"
-    assert_difference("Node.count", 0) do
-      put :create, :body => xml
-    end
-    # hope for success
-    assert_require_public_data "node create did not return forbidden status"
-
-    ## Now try with the user that has the public data
-    basic_authorization user.email, "test"
-
-    # create a minimal xml file
-    xml = "<osm><node lat='#{lat}' lon='#{lon}' changeset='#{changeset.id}'/></osm>"
-    put :create, :body => xml
-    # hope for success
-    assert_response :success, "node upload did not return success status"
-
-    # read id of created node and search for it
-    nodeid = @response.body
-    checknode = Node.find(nodeid)
-    assert_not_nil checknode, "uploaded node not found in data base after upload"
-    # compare values
-    assert_in_delta lat * 10000000, checknode.latitude, 1, "saved node does not match requested latitude"
-    assert_in_delta lon * 10000000, checknode.longitude, 1, "saved node does not match requested longitude"
-    assert_equal changeset.id, checknode.changeset_id, "saved node does not belong to changeset that it was created in"
-    assert_equal true, checknode.visible, "saved node is not visible"
-  end
-
-  def test_create_invalid_xml
-    ## Only test public user here, as test_create should cover what's the forbiddens
-    ## that would occur here
-
-    user = create(:user)
-    changeset = create(:changeset, :user => user)
-
-    basic_authorization user.email, "test"
-    lat = 3.434
-    lon = 3.23
-
-    # test that the upload is rejected when xml is valid, but osm doc isn't
-    xml = "<create/>"
-    put :create, :body => xml
-    assert_response :bad_request, "node upload did not return bad_request status"
-    assert_equal "Cannot parse valid node from xml string <create/>. XML doesn't contain an osm/node element.", @response.body
-
-    # test that the upload is rejected when no lat is supplied
-    # create a minimal xml file
-    xml = "<osm><node lon='#{lon}' changeset='#{changeset.id}'/></osm>"
-    put :create, :body => xml
-    # hope for success
-    assert_response :bad_request, "node upload did not return bad_request status"
-    assert_equal "Cannot parse valid node from xml string <node lon=\"3.23\" changeset=\"#{changeset.id}\"/>. lat missing", @response.body
-
-    # test that the upload is rejected when no lon is supplied
-    # create a minimal xml file
-    xml = "<osm><node lat='#{lat}' changeset='#{changeset.id}'/></osm>"
-    put :create, :body => xml
-    # hope for success
-    assert_response :bad_request, "node upload did not return bad_request status"
-    assert_equal "Cannot parse valid node from xml string <node lat=\"3.434\" changeset=\"#{changeset.id}\"/>. lon missing", @response.body
-
-    # test that the upload is rejected when lat is non-numeric
-    # create a minimal xml file
-    xml = "<osm><node lat='abc' lon='#{lon}' changeset='#{changeset.id}'/></osm>"
-    put :create, :body => xml
-    # hope for success
-    assert_response :bad_request, "node upload did not return bad_request status"
-    assert_equal "Cannot parse valid node from xml string <node lat=\"abc\" lon=\"#{lon}\" changeset=\"#{changeset.id}\"/>. lat not a number", @response.body
-
-    # test that the upload is rejected when lon is non-numeric
-    # create a minimal xml file
-    xml = "<osm><node lat='#{lat}' lon='abc' changeset='#{changeset.id}'/></osm>"
-    put :create, :body => xml
-    # hope for success
-    assert_response :bad_request, "node upload did not return bad_request status"
-    assert_equal "Cannot parse valid node from xml string <node lat=\"#{lat}\" lon=\"abc\" changeset=\"#{changeset.id}\"/>. lon not a number", @response.body
-
-    # test that the upload is rejected when we have a tag which is too long
-    xml = "<osm><node lat='#{lat}' lon='#{lon}' changeset='#{changeset.id}'><tag k='foo' v='#{'x' * 256}'/></node></osm>"
-    put :create, :body => xml
-    assert_response :bad_request, "node upload did not return bad_request status"
-    assert_equal ["NodeTag ", " v: is too long (maximum is 255 characters) (\"#{'x' * 256}\")"], @response.body.split(/[0-9]+,foo:/)
-  end
-
-  def test_show
-    # check that a visible node is returned properly
-    get :show, :params => { :id => create(:node).id }
-    assert_response :success
-
-    # check that an deleted node is not returned
-    get :show, :params => { :id => create(:node, :deleted).id }
-    assert_response :gone
-
-    # check chat a non-existent node is not returned
-    get :show, :params => { :id => 0 }
-    assert_response :not_found
-  end
-
-  # this tests deletion restrictions - basic deletion is tested in the unit
-  # tests for node!
-  def test_delete
-    private_user = create(:user, :data_public => false)
-    private_user_changeset = create(:changeset, :user => private_user)
-    private_user_closed_changeset = create(:changeset, :closed, :user => private_user)
-    private_node = create(:node, :changeset => private_user_changeset)
-    private_deleted_node = create(:node, :deleted, :changeset => private_user_changeset)
-
-    ## first try to delete node without auth
-    delete :delete, :params => { :id => private_node.id }
-    assert_response :unauthorized
-
-    ## now set auth for the non-data public user
-    basic_authorization private_user.email, "test"
-
-    # try to delete with an invalid (closed) changeset
-    xml = update_changeset(private_node.to_xml, private_user_closed_changeset.id)
-    delete :delete, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data("non-public user shouldn't be able to delete node")
-
-    # try to delete with an invalid (non-existent) changeset
-    xml = update_changeset(private_node.to_xml, 0)
-    delete :delete, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data("shouldn't be able to delete node, when user's data is private")
-
-    # valid delete now takes a payload
-    xml = private_node.to_xml
-    delete :delete, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data("shouldn't be able to delete node when user's data isn't public'")
-
-    # this won't work since the node is already deleted
-    xml = private_deleted_node.to_xml
-    delete :delete, :params => { :id => private_deleted_node.id }, :body => xml.to_s
-    assert_require_public_data
-
-    # this won't work since the node never existed
-    delete :delete, :params => { :id => 0 }
-    assert_require_public_data
-
-    ## these test whether nodes which are in-use can be deleted:
-    # in a way...
-    private_used_node = create(:node, :changeset => private_user_changeset)
-    create(:way_node, :node => private_used_node)
-
-    xml = private_used_node.to_xml
-    delete :delete, :params => { :id => private_used_node.id }, :body => xml.to_s
-    assert_require_public_data "shouldn't be able to delete a node used in a way (#{@response.body})"
-
-    # in a relation...
-    private_used_node2 = create(:node, :changeset => private_user_changeset)
-    create(:relation_member, :member => private_used_node2)
-
-    xml = private_used_node2.to_xml
-    delete :delete, :params => { :id => private_used_node2.id }, :body => xml.to_s
-    assert_require_public_data "shouldn't be able to delete a node used in a relation (#{@response.body})"
-
-    ## now setup for the public data user
-    user = create(:user, :data_public => true)
-    changeset = create(:changeset, :user => user)
-    closed_changeset = create(:changeset, :closed, :user => user)
-    node = create(:node, :changeset => changeset)
-    basic_authorization user.email, "test"
-
-    # try to delete with an invalid (closed) changeset
-    xml = update_changeset(node.to_xml, closed_changeset.id)
-    delete :delete, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :conflict
-
-    # try to delete with an invalid (non-existent) changeset
-    xml = update_changeset(node.to_xml, 0)
-    delete :delete, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :conflict
-
-    # try to delete a node with a different ID
-    other_node = create(:node)
-    xml = other_node.to_xml
-    delete :delete, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :bad_request,
-                    "should not be able to delete a node with a different ID from the XML"
-
-    # try to delete a node rubbish in the payloads
-    xml = "<delete/>"
-    delete :delete, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :bad_request,
-                    "should not be able to delete a node without a valid XML payload"
-
-    # valid delete now takes a payload
-    xml = node.to_xml
-    delete :delete, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :success
-
-    # valid delete should return the new version number, which should
-    # be greater than the old version number
-    assert @response.body.to_i > node.version,
-           "delete request should return a new version number for node"
-
-    # deleting the same node twice doesn't work
-    xml = node.to_xml
-    delete :delete, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :gone
-
-    # this won't work since the node never existed
-    delete :delete, :params => { :id => 0 }
-    assert_response :not_found
-
-    ## these test whether nodes which are in-use can be deleted:
-    # in a way...
-    used_node = create(:node, :changeset => create(:changeset, :user => user))
-    way_node = create(:way_node, :node => used_node)
-    way_node2 = create(:way_node, :node => used_node)
-
-    xml = used_node.to_xml
-    delete :delete, :params => { :id => used_node.id }, :body => xml.to_s
-    assert_response :precondition_failed,
-                    "shouldn't be able to delete a node used in a way (#{@response.body})"
-    assert_equal "Precondition failed: Node #{used_node.id} is still used by ways #{way_node.way.id},#{way_node2.way.id}.", @response.body
-
-    # in a relation...
-    used_node2 = create(:node, :changeset => create(:changeset, :user => user))
-    relation_member = create(:relation_member, :member => used_node2)
-    relation_member2 = create(:relation_member, :member => used_node2)
-
-    xml = used_node2.to_xml
-    delete :delete, :params => { :id => used_node2.id }, :body => xml.to_s
-    assert_response :precondition_failed,
-                    "shouldn't be able to delete a node used in a relation (#{@response.body})"
-    assert_equal "Precondition failed: Node #{used_node2.id} is still used by relations #{relation_member.relation.id},#{relation_member2.relation.id}.", @response.body
-  end
-
-  ##
-  # tests whether the API works and prevents incorrect use while trying
-  # to update nodes.
-  def test_update
-    ## First test with no user credentials
-    # try and update a node without authorisation
-    # first try to delete node without auth
-    private_user = create(:user, :data_public => false)
-    private_node = create(:node, :changeset => create(:changeset, :user => private_user))
-    user = create(:user)
-    node = create(:node, :changeset => create(:changeset, :user => user))
-
-    xml = node.to_xml
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :unauthorized
-
-    ## Second test with the private user
-
-    # setup auth
-    basic_authorization private_user.email, "test"
-
-    ## trying to break changesets
-
-    # try and update in someone else's changeset
-    xml = update_changeset(private_node.to_xml,
-                           create(:changeset).id)
-    put :update, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data "update with other user's changeset should be forbidden when data isn't public"
-
-    # try and update in a closed changeset
-    xml = update_changeset(private_node.to_xml,
-                           create(:changeset, :closed, :user => private_user).id)
-    put :update, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data "update with closed changeset should be forbidden, when data isn't public"
-
-    # try and update in a non-existant changeset
-    xml = update_changeset(private_node.to_xml, 0)
-    put :update, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data "update with changeset=0 should be forbidden, when data isn't public"
-
-    ## try and submit invalid updates
-    xml = xml_attr_rewrite(private_node.to_xml, "lat", 91.0)
-    put :update, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data "node at lat=91 should be forbidden, when data isn't public"
-
-    xml = xml_attr_rewrite(private_node.to_xml, "lat", -91.0)
-    put :update, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data "node at lat=-91 should be forbidden, when data isn't public"
-
-    xml = xml_attr_rewrite(private_node.to_xml, "lon", 181.0)
-    put :update, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data "node at lon=181 should be forbidden, when data isn't public"
-
-    xml = xml_attr_rewrite(private_node.to_xml, "lon", -181.0)
-    put :update, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data "node at lon=-181 should be forbidden, when data isn't public"
-
-    ## finally, produce a good request which still won't work
-    xml = private_node.to_xml
-    put :update, :params => { :id => private_node.id }, :body => xml.to_s
-    assert_require_public_data "should have failed with a forbidden when data isn't public"
-
-    ## Finally test with the public user
-
-    # try and update a node without authorisation
-    # first try to update node without auth
-    xml = node.to_xml
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :forbidden
-
-    # setup auth
-    basic_authorization user.email, "test"
-
-    ## trying to break changesets
-
-    # try and update in someone else's changeset
-    xml = update_changeset(node.to_xml,
-                           create(:changeset).id)
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :conflict, "update with other user's changeset should be rejected"
-
-    # try and update in a closed changeset
-    xml = update_changeset(node.to_xml,
-                           create(:changeset, :closed, :user => user).id)
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :conflict, "update with closed changeset should be rejected"
-
-    # try and update in a non-existant changeset
-    xml = update_changeset(node.to_xml, 0)
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :conflict, "update with changeset=0 should be rejected"
-
-    ## try and submit invalid updates
-    xml = xml_attr_rewrite(node.to_xml, "lat", 91.0)
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :bad_request, "node at lat=91 should be rejected"
-
-    xml = xml_attr_rewrite(node.to_xml, "lat", -91.0)
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :bad_request, "node at lat=-91 should be rejected"
-
-    xml = xml_attr_rewrite(node.to_xml, "lon", 181.0)
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :bad_request, "node at lon=181 should be rejected"
-
-    xml = xml_attr_rewrite(node.to_xml, "lon", -181.0)
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :bad_request, "node at lon=-181 should be rejected"
-
-    ## next, attack the versioning
-    current_node_version = node.version
-
-    # try and submit a version behind
-    xml = xml_attr_rewrite(node.to_xml,
-                           "version", current_node_version - 1)
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :conflict, "should have failed on old version number"
-
-    # try and submit a version ahead
-    xml = xml_attr_rewrite(node.to_xml,
-                           "version", current_node_version + 1)
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :conflict, "should have failed on skipped version number"
-
-    # try and submit total crap in the version field
-    xml = xml_attr_rewrite(node.to_xml,
-                           "version", "p1r4t3s!")
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :conflict,
-                    "should not be able to put 'p1r4at3s!' in the version field"
-
-    ## try an update with the wrong ID
-    xml = create(:node).to_xml
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :bad_request,
-                    "should not be able to update a node with a different ID from the XML"
-
-    ## try an update with a minimal valid XML doc which isn't a well-formed OSM doc.
-    xml = "<update/>"
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :bad_request,
-                    "should not be able to update a node with non-OSM XML doc."
-
-    ## finally, produce a good request which should work
-    xml = node.to_xml
-    put :update, :params => { :id => node.id }, :body => xml.to_s
-    assert_response :success, "a valid update request failed"
-  end
-
-  ##
-  # test fetching multiple nodes
-  def test_index
-    node1 = create(:node)
-    node2 = create(:node, :deleted)
-    node3 = create(:node)
-    node4 = create(:node, :with_history, :version => 2)
-    node5 = create(:node, :deleted, :with_history, :version => 2)
-
-    # check error when no parameter provided
-    get :index
-    assert_response :bad_request
-
-    # check error when no parameter value provided
-    get :index, :params => { :nodes => "" }
-    assert_response :bad_request
-
-    # test a working call
-    get :index, :params => { :nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id}" }
-    assert_response :success
-    assert_select "osm" do
-      assert_select "node", :count => 5
-      assert_select "node[id='#{node1.id}'][visible='true']", :count => 1
-      assert_select "node[id='#{node2.id}'][visible='false']", :count => 1
-      assert_select "node[id='#{node3.id}'][visible='true']", :count => 1
-      assert_select "node[id='#{node4.id}'][visible='true']", :count => 1
-      assert_select "node[id='#{node5.id}'][visible='false']", :count => 1
-    end
-
-    # check error when a non-existent node is included
-    get :index, :params => { :nodes => "#{node1.id},#{node2.id},#{node3.id},#{node4.id},#{node5.id},0" }
-    assert_response :not_found
-  end
-
-  ##
-  # test adding tags to a node
-  def test_duplicate_tags
-    existing_tag = create(:node_tag)
-    assert_equal true, existing_tag.node.changeset.user.data_public
-    # setup auth
-    basic_authorization existing_tag.node.changeset.user.email, "test"
-
-    # add an identical tag to the node
-    tag_xml = XML::Node.new("tag")
-    tag_xml["k"] = existing_tag.k
-    tag_xml["v"] = existing_tag.v
-
-    # add the tag into the existing xml
-    node_xml = existing_tag.node.to_xml
-    node_xml.find("//osm/node").first << tag_xml
-
-    # try and upload it
-    put :update, :params => { :id => existing_tag.node.id }, :body => node_xml.to_s
-    assert_response :bad_request,
-                    "adding duplicate tags to a node should fail with 'bad request'"
-    assert_equal "Element node/#{existing_tag.node.id} has duplicate tags with key #{existing_tag.k}", @response.body
-  end
-
-  # test whether string injection is possible
-  def test_string_injection
-    private_user = create(:user, :data_public => false)
-    private_changeset = create(:changeset, :user => private_user)
-    user = create(:user)
-    changeset = create(:changeset, :user => user)
-
-    ## First try with the non-data public user
-    basic_authorization private_user.email, "test"
-
-    # try and put something into a string that the API might
-    # use unquoted and therefore allow code injection...
-    xml = "<osm><node lat='0' lon='0' changeset='#{private_changeset.id}'>" \
-          '<tag k="#{@user.inspect}" v="0"/>' \
-          "</node></osm>"
-    put :create, :body => xml
-    assert_require_public_data "Shouldn't be able to create with non-public user"
-
-    ## Then try with the public data user
-    basic_authorization user.email, "test"
-
-    # try and put something into a string that the API might
-    # use unquoted and therefore allow code injection...
-    xml = "<osm><node lat='0' lon='0' changeset='#{changeset.id}'>" \
-          '<tag k="#{@user.inspect}" v="0"/>' \
-          "</node></osm>"
-    put :create, :body => xml
-    assert_response :success
-    nodeid = @response.body
-
-    # find the node in the database
-    checknode = Node.find(nodeid)
-    assert_not_nil checknode, "node not found in data base after upload"
-
-    # and grab it using the api
-    get :show, :params => { :id => nodeid }
-    assert_response :success
-    apinode = Node.from_xml(@response.body)
-    assert_not_nil apinode, "downloaded node is nil, but shouldn't be"
-
-    # check the tags are not corrupted
-    assert_equal checknode.tags, apinode.tags
-    assert apinode.tags.include?("\#{@user.inspect}")
-  end
-
-  ##
-  # update the changeset_id of a node element
-  def update_changeset(xml, changeset_id)
-    xml_attr_rewrite(xml, "changeset", changeset_id)
-  end
-
-  ##
-  # update an attribute in the node element
-  def xml_attr_rewrite(xml, name, value)
-    xml.find("//osm/node").first[name] = value.to_s
-    xml
-  end
-
-  ##
-  # parse some xml
-  def xml_parse(xml)
-    parser = XML::Parser.string(xml)
-    parser.parse
-  end
-end

test/controllers/old_nodes_controller_test.rb

@@ -1,428 +0,0 @@
-require "test_helper"
-
-class OldNodesControllerTest < ActionController::TestCase
-  #
-  # TODO: test history
-  #
-
-  ##
-  # test all routes which lead to this controller
-  def test_routes
-    assert_routing(
-      { :path => "/api/0.6/node/1/history", :method => :get },
-      { :controller => "old_nodes", :action => "history", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/node/1/2", :method => :get },
-      { :controller => "old_nodes", :action => "version", :id => "1", :version => "2" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/node/1/2/redact", :method => :post },
-      { :controller => "old_nodes", :action => "redact", :id => "1", :version => "2" }
-    )
-  end
-
-  ##
-  # test the version call by submitting several revisions of a new node
-  # to the API and ensuring that later calls to version return the
-  # matching versions of the object.
-  #
-  ##
-  # FIXME: Move this test to being an integration test since it spans multiple controllers
-  def test_version
-    private_user = create(:user, :data_public => false)
-    private_node = create(:node, :with_history, :version => 4, :changeset => create(:changeset, :user => private_user))
-    user = create(:user)
-    node = create(:node, :with_history, :version => 4, :changeset => create(:changeset, :user => user))
-    create_list(:node_tag, 2, :node => node)
-    # Ensure that the current tags are propagated to the history too
-    propagate_tags(node, node.old_nodes.last)
-
-    ## First try this with a non-public user
-    basic_authorization private_user.email, "test"
-
-    # setup a simple XML node
-    xml_doc = private_node.to_xml
-    xml_node = xml_doc.find("//osm/node").first
-    nodeid = private_node.id
-
-    # keep a hash of the versions => string, as we'll need something
-    # to test against later
-    versions = {}
-
-    # save a version for later checking
-    versions[xml_node["version"]] = xml_doc.to_s
-
-    # randomly move the node about
-    3.times do
-      # move the node somewhere else
-      xml_node["lat"] = precision(rand * 180 - 90).to_s
-      xml_node["lon"] = precision(rand * 360 - 180).to_s
-      with_controller(NodesController.new) do
-        put :update, :params => { :id => nodeid }, :body => xml_doc.to_s
-        assert_response :forbidden, "Should have rejected node update"
-        xml_node["version"] = @response.body.to_s
-      end
-      # save a version for later checking
-      versions[xml_node["version"]] = xml_doc.to_s
-    end
-
-    # add a bunch of random tags
-    3.times do
-      xml_tag = XML::Node.new("tag")
-      xml_tag["k"] = random_string
-      xml_tag["v"] = random_string
-      xml_node << xml_tag
-      with_controller(NodesController.new) do
-        put :update, :params => { :id => nodeid }, :body => xml_doc.to_s
-        assert_response :forbidden,
-                        "should have rejected node #{nodeid} (#{@response.body}) with forbidden"
-        xml_node["version"] = @response.body.to_s
-      end
-      # save a version for later checking
-      versions[xml_node["version"]] = xml_doc.to_s
-    end
-
-    # probably should check that they didn't get written to the database
-
-    ## Now do it with the public user
-    basic_authorization user.email, "test"
-
-    # setup a simple XML node
-
-    xml_doc = node.to_xml
-    xml_node = xml_doc.find("//osm/node").first
-    nodeid = node.id
-
-    # keep a hash of the versions => string, as we'll need something
-    # to test against later
-    versions = {}
-
-    # save a version for later checking
-    versions[xml_node["version"]] = xml_doc.to_s
-
-    # randomly move the node about
-    3.times do
-      # move the node somewhere else
-      xml_node["lat"] = precision(rand * 180 - 90).to_s
-      xml_node["lon"] = precision(rand * 360 - 180).to_s
-      with_controller(NodesController.new) do
-        put :update, :params => { :id => nodeid }, :body => xml_doc.to_s
-        assert_response :success
-        xml_node["version"] = @response.body.to_s
-      end
-      # save a version for later checking
-      versions[xml_node["version"]] = xml_doc.to_s
-    end
-
-    # add a bunch of random tags
-    3.times do
-      xml_tag = XML::Node.new("tag")
-      xml_tag["k"] = random_string
-      xml_tag["v"] = random_string
-      xml_node << xml_tag
-      with_controller(NodesController.new) do
-        put :update, :params => { :id => nodeid }, :body => xml_doc.to_s
-        assert_response :success,
-                        "couldn't update node #{nodeid} (#{@response.body})"
-        xml_node["version"] = @response.body.to_s
-      end
-      # save a version for later checking
-      versions[xml_node["version"]] = xml_doc.to_s
-    end
-
-    # check all the versions
-    versions.each_key do |key|
-      get :version, :params => { :id => nodeid, :version => key.to_i }
-
-      assert_response :success,
-                      "couldn't get version #{key.to_i} of node #{nodeid}"
-
-      check_node = Node.from_xml(versions[key])
-      api_node = Node.from_xml(@response.body.to_s)
-
-      assert_nodes_are_equal check_node, api_node
-    end
-  end
-
-  def test_not_found_version
-    check_not_found_id_version(70000, 312344)
-    check_not_found_id_version(-1, -13)
-    check_not_found_id_version(create(:node).id, 24354)
-    check_not_found_id_version(24356, create(:node).version)
-  end
-
-  def check_not_found_id_version(id, version)
-    get :version, :params => { :id => id, :version => version }
-    assert_response :not_found
-  rescue ActionController::UrlGenerationError => ex
-    assert_match(/No route matches/, ex.to_s)
-  end
-
-  ##
-  # Test that getting the current version is identical to picking
-  # that version with the version URI call.
-  def test_current_version
-    node = create(:node, :with_history)
-    used_node = create(:node, :with_history)
-    create(:way_node, :node => used_node)
-    node_used_by_relationship = create(:node, :with_history)
-    create(:relation_member, :member => node_used_by_relationship)
-    node_with_versions = create(:node, :with_history, :version => 4)
-
-    create(:node_tag, :node => node)
-    create(:node_tag, :node => used_node)
-    create(:node_tag, :node => node_used_by_relationship)
-    create(:node_tag, :node => node_with_versions)
-    propagate_tags(node, node.old_nodes.last)
-    propagate_tags(used_node, used_node.old_nodes.last)
-    propagate_tags(node_used_by_relationship, node_used_by_relationship.old_nodes.last)
-    propagate_tags(node_with_versions, node_with_versions.old_nodes.last)
-
-    check_current_version(node)
-    check_current_version(used_node)
-    check_current_version(node_used_by_relationship)
-    check_current_version(node_with_versions)
-  end
-
-  ##
-  # test the redaction of an old version of a node, while not being
-  # authorised.
-  def test_redact_node_unauthorised
-    node = create(:node, :with_history, :version => 4)
-    node_v3 = node.old_nodes.find_by(:version => 3)
-
-    do_redact_node(node_v3,
-                   create(:redaction))
-    assert_response :unauthorized, "should need to be authenticated to redact."
-  end
-
-  ##
-  # test the redaction of an old version of a node, while being
-  # authorised as a normal user.
-  def test_redact_node_normal_user
-    basic_authorization create(:user).email, "test"
-
-    node = create(:node, :with_history, :version => 4)
-    node_v3 = node.old_nodes.find_by(:version => 3)
-
-    do_redact_node(node_v3,
-                   create(:redaction))
-    assert_response :forbidden, "should need to be moderator to redact."
-  end
-
-  ##
-  # test that, even as moderator, the current version of a node
-  # can't be redacted.
-  def test_redact_node_current_version
-    basic_authorization create(:moderator_user).email, "test"
-
-    node = create(:node, :with_history, :version => 4)
-    node_v4 = node.old_nodes.find_by(:version => 4)
-
-    do_redact_node(node_v4,
-                   create(:redaction))
-    assert_response :bad_request, "shouldn't be OK to redact current version as moderator."
-  end
-
-  ##
-  # test that redacted nodes aren't visible, regardless of
-  # authorisation except as moderator...
-  def test_version_redacted
-    node = create(:node, :with_history, :version => 2)
-    node_v1 = node.old_nodes.find_by(:version => 1)
-    node_v1.redact!(create(:redaction))
-
-    get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }
-    assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
-
-    # not even to a logged-in user
-    basic_authorization create(:user).email, "test"
-    get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }
-    assert_response :forbidden, "Redacted node shouldn't be visible via the version API, even when logged in."
-  end
-
-  ##
-  # test that redacted nodes aren't visible in the history
-  def test_history_redacted
-    node = create(:node, :with_history, :version => 2)
-    node_v1 = node.old_nodes.find_by(:version => 1)
-    node_v1.redact!(create(:redaction))
-
-    get :history, :params => { :id => node_v1.node_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 0, "redacted node #{node_v1.node_id} version #{node_v1.version} shouldn't be present in the history."
-
-    # not even to a logged-in user
-    basic_authorization create(:user).email, "test"
-    get :history, :params => { :id => node_v1.node_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 0, "redacted node #{node_v1.node_id} version #{node_v1.version} shouldn't be present in the history, even when logged in."
-  end
-
-  ##
-  # test the redaction of an old version of a node, while being
-  # authorised as a moderator.
-  def test_redact_node_moderator
-    node = create(:node, :with_history, :version => 4)
-    node_v3 = node.old_nodes.find_by(:version => 3)
-    basic_authorization create(:moderator_user).email, "test"
-
-    do_redact_node(node_v3, create(:redaction))
-    assert_response :success, "should be OK to redact old version as moderator."
-
-    # check moderator can still see the redacted data, when passing
-    # the appropriate flag
-    get :version, :params => { :id => node_v3.node_id, :version => node_v3.version }
-    assert_response :forbidden, "After redaction, node should be gone for moderator, when flag not passed."
-    get :version, :params => { :id => node_v3.node_id, :version => node_v3.version, :show_redactions => "true" }
-    assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
-
-    # and when accessed via history
-    get :history, :params => { :id => node_v3.node_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 0, "node #{node_v3.node_id} version #{node_v3.version} should not be present in the history for moderators when not passing flag."
-    get :history, :params => { :id => node_v3.node_id, :show_redactions => "true" }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 1, "node #{node_v3.node_id} version #{node_v3.version} should still be present in the history for moderators when passing flag."
-  end
-
-  # testing that if the moderator drops auth, he can't see the
-  # redacted stuff any more.
-  def test_redact_node_is_redacted
-    node = create(:node, :with_history, :version => 4)
-    node_v3 = node.old_nodes.find_by(:version => 3)
-    basic_authorization create(:moderator_user).email, "test"
-
-    do_redact_node(node_v3, create(:redaction))
-    assert_response :success, "should be OK to redact old version as moderator."
-
-    # re-auth as non-moderator
-    basic_authorization create(:user).email, "test"
-
-    # check can't see the redacted data
-    get :version, :params => { :id => node_v3.node_id, :version => node_v3.version }
-    assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
-
-    # and when accessed via history
-    get :history, :params => { :id => node_v3.node_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm node[id='#{node_v3.node_id}'][version='#{node_v3.version}']", 0, "redacted node #{node_v3.node_id} version #{node_v3.version} shouldn't be present in the history."
-  end
-
-  ##
-  # test the unredaction of an old version of a node, while not being
-  # authorised.
-  def test_unredact_node_unauthorised
-    node = create(:node, :with_history, :version => 2)
-    node_v1 = node.old_nodes.find_by(:version => 1)
-    node_v1.redact!(create(:redaction))
-
-    post :redact, :params => { :id => node_v1.node_id, :version => node_v1.version }
-    assert_response :unauthorized, "should need to be authenticated to unredact."
-  end
-
-  ##
-  # test the unredaction of an old version of a node, while being
-  # authorised as a normal user.
-  def test_unredact_node_normal_user
-    user = create(:user)
-    node = create(:node, :with_history, :version => 2)
-    node_v1 = node.old_nodes.find_by(:version => 1)
-    node_v1.redact!(create(:redaction))
-
-    basic_authorization user.email, "test"
-
-    post :redact, :params => { :id => node_v1.node_id, :version => node_v1.version }
-    assert_response :forbidden, "should need to be moderator to unredact."
-  end
-
-  ##
-  # test the unredaction of an old version of a node, while being
-  # authorised as a moderator.
-  def test_unredact_node_moderator
-    moderator_user = create(:moderator_user)
-    node = create(:node, :with_history, :version => 2)
-    node_v1 = node.old_nodes.find_by(:version => 1)
-    node_v1.redact!(create(:redaction))
-
-    basic_authorization moderator_user.email, "test"
-
-    post :redact, :params => { :id => node_v1.node_id, :version => node_v1.version }
-    assert_response :success, "should be OK to unredact old version as moderator."
-
-    # check moderator can now see the redacted data, when not
-    # passing the aspecial flag
-    get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }
-    assert_response :success, "After unredaction, node should not be gone for moderator."
-
-    # and when accessed via history
-    get :history, :params => { :id => node_v1.node_id }
-    assert_response :success, "Unredaction shouldn't have stopped history working."
-    assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 1, "node #{node_v1.node_id} version #{node_v1.version} should now be present in the history for moderators without passing flag."
-
-    basic_authorization create(:user).email, "test"
-
-    # check normal user can now see the redacted data
-    get :version, :params => { :id => node_v1.node_id, :version => node_v1.version }
-    assert_response :success, "After unredaction, node should be visible to normal users."
-
-    # and when accessed via history
-    get :history, :params => { :id => node_v1.node_id }
-    assert_response :success, "Unredaction shouldn't have stopped history working."
-    assert_select "osm node[id='#{node_v1.node_id}'][version='#{node_v1.version}']", 1, "node #{node_v1.node_id} version #{node_v1.version} should now be present in the history for normal users without passing flag."
-  end
-
-  private
-
-  def do_redact_node(node, redaction)
-    get :version, :params => { :id => node.node_id, :version => node.version }
-    assert_response :success, "should be able to get version #{node.version} of node #{node.node_id}."
-
-    # now redact it
-    post :redact, :params => { :id => node.node_id, :version => node.version, :redaction => redaction.id }
-  end
-
-  def check_current_version(node_id)
-    # get the current version of the node
-    current_node = with_controller(NodesController.new) do
-      get :show, :params => { :id => node_id }
-      assert_response :success, "cant get current node #{node_id}"
-      Node.from_xml(@response.body)
-    end
-    assert_not_nil current_node, "getting node #{node_id} returned nil"
-
-    # get the "old" version of the node from the old_node interface
-    get :version, :params => { :id => node_id, :version => current_node.version }
-    assert_response :success, "cant get old node #{node_id}, v#{current_node.version}"
-    old_node = Node.from_xml(@response.body)
-
-    # check the nodes are the same
-    assert_nodes_are_equal current_node, old_node
-  end
-
-  ##
-  # returns a 16 character long string with some nasty characters in it.
-  # this ought to stress-test the tag handling as well as the versioning.
-  def random_string
-    letters = [["!", '"', "$", "&", ";", "@"],
-               ("a".."z").to_a,
-               ("A".."Z").to_a,
-               ("0".."9").to_a].flatten
-    (1..16).map { |_i| letters[rand(letters.length)] }.join
-  end
-
-  ##
-  # truncate a floating point number to the scale that it is stored in
-  # the database. otherwise rounding errors can produce failing unit
-  # tests when they shouldn't.
-  def precision(f)
-    (f * GeoRecord::SCALE).round.to_f / GeoRecord::SCALE
-  end
-
-  def propagate_tags(node, old_node)
-    node.tags.each do |k, v|
-      create(:old_node_tag, :old_node => old_node, :k => k, :v => v)
-    end
-  end
-end

test/controllers/old_relations_controller_test.rb

@@ -1,272 +0,0 @@
-require "test_helper"
-
-class OldRelationsControllerTest < ActionController::TestCase
-  ##
-  # test all routes which lead to this controller
-  def test_routes
-    assert_routing(
-      { :path => "/api/0.6/relation/1/history", :method => :get },
-      { :controller => "old_relations", :action => "history", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/relation/1/2", :method => :get },
-      { :controller => "old_relations", :action => "version", :id => "1", :version => "2" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/relation/1/2/redact", :method => :post },
-      { :controller => "old_relations", :action => "redact", :id => "1", :version => "2" }
-    )
-  end
-
-  # -------------------------------------
-  # Test reading old relations.
-  # -------------------------------------
-  def test_history
-    # check that a visible relations is returned properly
-    get :history, :params => { :id => create(:relation, :with_history).id }
-    assert_response :success
-
-    # check chat a non-existent relations is not returned
-    get :history, :params => { :id => 0 }
-    assert_response :not_found
-  end
-
-  ##
-  # test the redaction of an old version of a relation, while not being
-  # authorised.
-  def test_redact_relation_unauthorised
-    relation = create(:relation, :with_history, :version => 4)
-    relation_v3 = relation.old_relations.find_by(:version => 3)
-
-    do_redact_relation(relation_v3, create(:redaction))
-    assert_response :unauthorized, "should need to be authenticated to redact."
-  end
-
-  ##
-  # test the redaction of an old version of a relation, while being
-  # authorised as a normal user.
-  def test_redact_relation_normal_user
-    relation = create(:relation, :with_history, :version => 4)
-    relation_v3 = relation.old_relations.find_by(:version => 3)
-
-    basic_authorization create(:user).email, "test"
-
-    do_redact_relation(relation_v3, create(:redaction))
-    assert_response :forbidden, "should need to be moderator to redact."
-  end
-
-  ##
-  # test that, even as moderator, the current version of a relation
-  # can't be redacted.
-  def test_redact_relation_current_version
-    relation = create(:relation, :with_history, :version => 4)
-    relation_latest = relation.old_relations.last
-
-    basic_authorization create(:moderator_user).email, "test"
-
-    do_redact_relation(relation_latest, create(:redaction))
-    assert_response :bad_request, "shouldn't be OK to redact current version as moderator."
-  end
-
-  ##
-  # test that redacted relations aren't visible, regardless of
-  # authorisation except as moderator...
-  def test_version_redacted
-    relation = create(:relation, :with_history, :version => 2)
-    relation_v1 = relation.old_relations.find_by(:version => 1)
-    relation_v1.redact!(create(:redaction))
-
-    get :version, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
-    assert_response :forbidden, "Redacted relation shouldn't be visible via the version API."
-
-    # not even to a logged-in user
-    basic_authorization create(:user).email, "test"
-    get :version, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
-    assert_response :forbidden, "Redacted relation shouldn't be visible via the version API, even when logged in."
-  end
-
-  ##
-  # test that redacted relations aren't visible in the history
-  def test_history_redacted
-    relation = create(:relation, :with_history, :version => 2)
-    relation_v1 = relation.old_relations.find_by(:version => 1)
-    relation_v1.redact!(create(:redaction))
-
-    get :history, :params => { :id => relation_v1.relation_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 0, "redacted relation #{relation_v1.relation_id} version #{relation_v1.version} shouldn't be present in the history."
-
-    # not even to a logged-in user
-    basic_authorization create(:user).email, "test"
-    get :version, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
-    get :history, :params => { :id => relation_v1.relation_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 0, "redacted relation #{relation_v1.relation_id} version #{relation_v1.version} shouldn't be present in the history, even when logged in."
-  end
-
-  ##
-  # test the redaction of an old version of a relation, while being
-  # authorised as a moderator.
-  def test_redact_relation_moderator
-    relation = create(:relation, :with_history, :version => 4)
-    relation_v3 = relation.old_relations.find_by(:version => 3)
-
-    basic_authorization create(:moderator_user).email, "test"
-
-    do_redact_relation(relation_v3, create(:redaction))
-    assert_response :success, "should be OK to redact old version as moderator."
-
-    # check moderator can still see the redacted data, when passing
-    # the appropriate flag
-    get :version, :params => { :id => relation_v3.relation_id, :version => relation_v3.version }
-    assert_response :forbidden, "After redaction, relation should be gone for moderator, when flag not passed."
-    get :version, :params => { :id => relation_v3.relation_id, :version => relation_v3.version, :show_redactions => "true" }
-    assert_response :success, "After redaction, relation should not be gone for moderator, when flag passed."
-
-    # and when accessed via history
-    get :history, :params => { :id => relation_v3.relation_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 0, "relation #{relation_v3.relation_id} version #{relation_v3.version} should not be present in the history for moderators when not passing flag."
-    get :history, :params => { :id => relation_v3.relation_id, :show_redactions => "true" }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 1, "relation #{relation_v3.relation_id} version #{relation_v3.version} should still be present in the history for moderators when passing flag."
-  end
-
-  # testing that if the moderator drops auth, he can't see the
-  # redacted stuff any more.
-  def test_redact_relation_is_redacted
-    relation = create(:relation, :with_history, :version => 4)
-    relation_v3 = relation.old_relations.find_by(:version => 3)
-
-    basic_authorization create(:moderator_user).email, "test"
-
-    do_redact_relation(relation_v3, create(:redaction))
-    assert_response :success, "should be OK to redact old version as moderator."
-
-    # re-auth as non-moderator
-    basic_authorization create(:user).email, "test"
-
-    # check can't see the redacted data
-    get :version, :params => { :id => relation_v3.relation_id, :version => relation_v3.version }
-    assert_response :forbidden, "Redacted relation shouldn't be visible via the version API."
-
-    # and when accessed via history
-    get :history, :params => { :id => relation_v3.relation_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm relation[id='#{relation_v3.relation_id}'][version='#{relation_v3.version}']", 0, "redacted relation #{relation_v3.relation_id} version #{relation_v3.version} shouldn't be present in the history."
-  end
-
-  ##
-  # test the unredaction of an old version of a relation, while not being
-  # authorised.
-  def test_unredact_relation_unauthorised
-    relation = create(:relation, :with_history, :version => 2)
-    relation_v1 = relation.old_relations.find_by(:version => 1)
-    relation_v1.redact!(create(:redaction))
-
-    post :redact, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
-    assert_response :unauthorized, "should need to be authenticated to unredact."
-  end
-
-  ##
-  # test the unredaction of an old version of a relation, while being
-  # authorised as a normal user.
-  def test_unredact_relation_normal_user
-    relation = create(:relation, :with_history, :version => 2)
-    relation_v1 = relation.old_relations.find_by(:version => 1)
-    relation_v1.redact!(create(:redaction))
-
-    basic_authorization create(:user).email, "test"
-
-    post :redact, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
-    assert_response :forbidden, "should need to be moderator to unredact."
-  end
-
-  ##
-  # test the unredaction of an old version of a relation, while being
-  # authorised as a moderator.
-  def test_unredact_relation_moderator
-    relation = create(:relation, :with_history, :version => 2)
-    relation_v1 = relation.old_relations.find_by(:version => 1)
-    relation_v1.redact!(create(:redaction))
-
-    basic_authorization create(:moderator_user).email, "test"
-
-    post :redact, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
-    assert_response :success, "should be OK to unredact old version as moderator."
-
-    # check moderator can still see the redacted data, without passing
-    # the appropriate flag
-    get :version, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
-    assert_response :success, "After unredaction, relation should not be gone for moderator."
-
-    # and when accessed via history
-    get :history, :params => { :id => relation_v1.relation_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 1, "relation #{relation_v1.relation_id} version #{relation_v1.version} should still be present in the history for moderators."
-
-    basic_authorization create(:user).email, "test"
-
-    # check normal user can now see the redacted data
-    get :version, :params => { :id => relation_v1.relation_id, :version => relation_v1.version }
-    assert_response :success, "After redaction, node should not be gone for normal user."
-
-    # and when accessed via history
-    get :history, :params => { :id => relation_v1.relation_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm relation[id='#{relation_v1.relation_id}'][version='#{relation_v1.version}']", 1, "relation #{relation_v1.relation_id} version #{relation_v1.version} should still be present in the history for normal users."
-  end
-
-  private
-
-  ##
-  # check that the current version of a relation is equivalent to the
-  # version which we're getting from the versions call.
-  def check_current_version(relation_id)
-    # get the current version
-    current_relation = with_controller(RelationsController.new) do
-      get :show, :params => { :id => relation_id }
-      assert_response :success, "can't get current relation #{relation_id}"
-      Relation.from_xml(@response.body)
-    end
-    assert_not_nil current_relation, "getting relation #{relation_id} returned nil"
-
-    # get the "old" version of the relation from the version method
-    get :version, :params => { :id => relation_id, :version => current_relation.version }
-    assert_response :success, "can't get old relation #{relation_id}, v#{current_relation.version}"
-    old_relation = Relation.from_xml(@response.body)
-
-    # check that the relations are identical
-    assert_relations_are_equal current_relation, old_relation
-  end
-
-  ##
-  # look at all the versions of the relation in the history and get each version from
-  # the versions call. check that they're the same.
-  def check_history_equals_versions(relation_id)
-    get :history, :params => { :id => relation_id }
-    assert_response :success, "can't get relation #{relation_id} from API"
-    history_doc = XML::Parser.string(@response.body).parse
-    assert_not_nil history_doc, "parsing relation #{relation_id} history failed"
-
-    history_doc.find("//osm/relation").each do |relation_doc|
-      history_relation = Relation.from_xml_node(relation_doc)
-      assert_not_nil history_relation, "parsing relation #{relation_id} version failed"
-
-      get :version, :params => { :id => relation_id, :version => history_relation.version }
-      assert_response :success, "couldn't get relation #{relation_id}, v#{history_relation.version}"
-      version_relation = Relation.from_xml(@response.body)
-      assert_not_nil version_relation, "failed to parse #{relation_id}, v#{history_relation.version}"
-
-      assert_relations_are_equal history_relation, version_relation
-    end
-  end
-
-  def do_redact_relation(relation, redaction)
-    get :version, :params => { :id => relation.relation_id, :version => relation.version }
-    assert_response :success, "should be able to get version #{relation.version} of relation #{relation.relation_id}."
-
-    # now redact it
-    post :redact, :params => { :id => relation.relation_id, :version => relation.version, :redaction => redaction.id }
-  end
-end

test/controllers/old_ways_controller_test.rb

@@ -1,318 +0,0 @@
-require "test_helper"
-
-class OldWaysControllerTest < ActionController::TestCase
-  ##
-  # test all routes which lead to this controller
-  def test_routes
-    assert_routing(
-      { :path => "/api/0.6/way/1/history", :method => :get },
-      { :controller => "old_ways", :action => "history", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/way/1/2", :method => :get },
-      { :controller => "old_ways", :action => "version", :id => "1", :version => "2" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/way/1/2/redact", :method => :post },
-      { :controller => "old_ways", :action => "redact", :id => "1", :version => "2" }
-    )
-  end
-
-  # -------------------------------------
-  # Test reading old ways.
-  # -------------------------------------
-
-  def test_history_visible
-    # check that a visible way is returned properly
-    get :history, :params => { :id => create(:way, :with_history).id }
-    assert_response :success
-  end
-
-  def test_history_invisible
-    # check that an invisible way's history is returned properly
-    get :history, :params => { :id => create(:way, :with_history, :deleted).id }
-    assert_response :success
-  end
-
-  def test_history_invalid
-    # check chat a non-existent way is not returned
-    get :history, :params => { :id => 0 }
-    assert_response :not_found
-  end
-
-  ##
-  # check that we can retrieve versions of a way
-  def test_version
-    way = create(:way, :with_history)
-    used_way = create(:way, :with_history)
-    create(:relation_member, :member => used_way)
-    way_with_versions = create(:way, :with_history, :version => 4)
-
-    create(:way_tag, :way => way)
-    create(:way_tag, :way => used_way)
-    create(:way_tag, :way => way_with_versions)
-    propagate_tags(way, way.old_ways.last)
-    propagate_tags(used_way, used_way.old_ways.last)
-    propagate_tags(way_with_versions, way_with_versions.old_ways.last)
-
-    check_current_version(way.id)
-    check_current_version(used_way.id)
-    check_current_version(way_with_versions.id)
-  end
-
-  ##
-  # check that returned history is the same as getting all
-  # versions of a way from the api.
-  def test_history_equals_versions
-    way = create(:way, :with_history)
-    used_way = create(:way, :with_history)
-    create(:relation_member, :member => used_way)
-    way_with_versions = create(:way, :with_history, :version => 4)
-
-    check_history_equals_versions(way.id)
-    check_history_equals_versions(used_way.id)
-    check_history_equals_versions(way_with_versions.id)
-  end
-
-  ##
-  # test the redaction of an old version of a way, while not being
-  # authorised.
-  def test_redact_way_unauthorised
-    way = create(:way, :with_history, :version => 4)
-    way_v3 = way.old_ways.find_by(:version => 3)
-
-    do_redact_way(way_v3, create(:redaction))
-    assert_response :unauthorized, "should need to be authenticated to redact."
-  end
-
-  ##
-  # test the redaction of an old version of a way, while being
-  # authorised as a normal user.
-  def test_redact_way_normal_user
-    basic_authorization create(:user).email, "test"
-    way = create(:way, :with_history, :version => 4)
-    way_v3 = way.old_ways.find_by(:version => 3)
-
-    do_redact_way(way_v3, create(:redaction))
-    assert_response :forbidden, "should need to be moderator to redact."
-  end
-
-  ##
-  # test that, even as moderator, the current version of a way
-  # can't be redacted.
-  def test_redact_way_current_version
-    basic_authorization create(:moderator_user).email, "test"
-    way = create(:way, :with_history, :version => 4)
-    way_latest = way.old_ways.last
-
-    do_redact_way(way_latest, create(:redaction))
-    assert_response :bad_request, "shouldn't be OK to redact current version as moderator."
-  end
-
-  ##
-  # test that redacted ways aren't visible, regardless of
-  # authorisation except as moderator...
-  def test_version_redacted
-    way = create(:way, :with_history, :version => 2)
-    way_v1 = way.old_ways.find_by(:version => 1)
-    way_v1.redact!(create(:redaction))
-
-    get :version, :params => { :id => way_v1.way_id, :version => way_v1.version }
-    assert_response :forbidden, "Redacted way shouldn't be visible via the version API."
-
-    # not even to a logged-in user
-    basic_authorization create(:user).email, "test"
-    get :version, :params => { :id => way_v1.way_id, :version => way_v1.version }
-    assert_response :forbidden, "Redacted way shouldn't be visible via the version API, even when logged in."
-  end
-
-  ##
-  # test that redacted ways aren't visible in the history
-  def test_history_redacted
-    way = create(:way, :with_history, :version => 2)
-    way_v1 = way.old_ways.find_by(:version => 1)
-    way_v1.redact!(create(:redaction))
-
-    get :history, :params => { :id => way_v1.way_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 0, "redacted way #{way_v1.way_id} version #{way_v1.version} shouldn't be present in the history."
-
-    # not even to a logged-in user
-    basic_authorization create(:user).email, "test"
-    get :version, :params => { :id => way_v1.way_id, :version => way_v1.version }
-    get :history, :params => { :id => way_v1.way_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 0, "redacted node #{way_v1.way_id} version #{way_v1.version} shouldn't be present in the history, even when logged in."
-  end
-
-  ##
-  # test the redaction of an old version of a way, while being
-  # authorised as a moderator.
-  def test_redact_way_moderator
-    way = create(:way, :with_history, :version => 4)
-    way_v3 = way.old_ways.find_by(:version => 3)
-    basic_authorization create(:moderator_user).email, "test"
-
-    do_redact_way(way_v3, create(:redaction))
-    assert_response :success, "should be OK to redact old version as moderator."
-
-    # check moderator can still see the redacted data, when passing
-    # the appropriate flag
-    get :version, :params => { :id => way_v3.way_id, :version => way_v3.version }
-    assert_response :forbidden, "After redaction, node should be gone for moderator, when flag not passed."
-    get :version, :params => { :id => way_v3.way_id, :version => way_v3.version, :show_redactions => "true" }
-    assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
-
-    # and when accessed via history
-    get :history, :params => { :id => way_v3.way_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 0, "way #{way_v3.way_id} version #{way_v3.version} should not be present in the history for moderators when not passing flag."
-    get :history, :params => { :id => way_v3.way_id, :show_redactions => "true" }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 1, "way #{way_v3.way_id} version #{way_v3.version} should still be present in the history for moderators when passing flag."
-  end
-
-  # testing that if the moderator drops auth, he can't see the
-  # redacted stuff any more.
-  def test_redact_way_is_redacted
-    way = create(:way, :with_history, :version => 4)
-    way_v3 = way.old_ways.find_by(:version => 3)
-    basic_authorization create(:moderator_user).email, "test"
-
-    do_redact_way(way_v3, create(:redaction))
-    assert_response :success, "should be OK to redact old version as moderator."
-
-    # re-auth as non-moderator
-    basic_authorization create(:user).email, "test"
-
-    # check can't see the redacted data
-    get :version, :params => { :id => way_v3.way_id, :version => way_v3.version }
-    assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
-
-    # and when accessed via history
-    get :history, :params => { :id => way_v3.way_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm way[id='#{way_v3.way_id}'][version='#{way_v3.version}']", 0, "redacted way #{way_v3.way_id} version #{way_v3.version} shouldn't be present in the history."
-  end
-
-  ##
-  # test the unredaction of an old version of a way, while not being
-  # authorised.
-  def test_unredact_way_unauthorised
-    way = create(:way, :with_history, :version => 2)
-    way_v1 = way.old_ways.find_by(:version => 1)
-    way_v1.redact!(create(:redaction))
-
-    post :redact, :params => { :id => way_v1.way_id, :version => way_v1.version }
-    assert_response :unauthorized, "should need to be authenticated to unredact."
-  end
-
-  ##
-  # test the unredaction of an old version of a way, while being
-  # authorised as a normal user.
-  def test_unredact_way_normal_user
-    way = create(:way, :with_history, :version => 2)
-    way_v1 = way.old_ways.find_by(:version => 1)
-    way_v1.redact!(create(:redaction))
-
-    basic_authorization create(:user).email, "test"
-
-    post :redact, :params => { :id => way_v1.way_id, :version => way_v1.version }
-    assert_response :forbidden, "should need to be moderator to unredact."
-  end
-
-  ##
-  # test the unredaction of an old version of a way, while being
-  # authorised as a moderator.
-  def test_unredact_way_moderator
-    moderator_user = create(:moderator_user)
-    way = create(:way, :with_history, :version => 2)
-    way_v1 = way.old_ways.find_by(:version => 1)
-    way_v1.redact!(create(:redaction))
-
-    basic_authorization moderator_user.email, "test"
-
-    post :redact, :params => { :id => way_v1.way_id, :version => way_v1.version }
-    assert_response :success, "should be OK to unredact old version as moderator."
-
-    # check moderator can still see the unredacted data, without passing
-    # the appropriate flag
-    get :version, :params => { :id => way_v1.way_id, :version => way_v1.version }
-    assert_response :success, "After unredaction, node should not be gone for moderator."
-
-    # and when accessed via history
-    get :history, :params => { :id => way_v1.way_id }
-    assert_response :success, "Unredaction shouldn't have stopped history working."
-    assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 1, "way #{way_v1.way_id} version #{way_v1.version} should still be present in the history for moderators."
-
-    basic_authorization create(:user).email, "test"
-
-    # check normal user can now see the unredacted data
-    get :version, :params => { :id => way_v1.way_id, :version => way_v1.version }
-    assert_response :success, "After redaction, node should not be gone for moderator, when flag passed."
-
-    # and when accessed via history
-    get :history, :params => { :id => way_v1.way_id }
-    assert_response :success, "Redaction shouldn't have stopped history working."
-    assert_select "osm way[id='#{way_v1.way_id}'][version='#{way_v1.version}']", 1, "way #{way_v1.way_id} version #{way_v1.version} should still be present in the history for normal users."
-  end
-
-  private
-
-  ##
-  # check that the current version of a way is equivalent to the
-  # version which we're getting from the versions call.
-  def check_current_version(way_id)
-    # get the current version
-    current_way = with_controller(WaysController.new) do
-      get :show, :params => { :id => way_id }
-      assert_response :success, "can't get current way #{way_id}"
-      Way.from_xml(@response.body)
-    end
-    assert_not_nil current_way, "getting way #{way_id} returned nil"
-
-    # get the "old" version of the way from the version method
-    get :version, :params => { :id => way_id, :version => current_way.version }
-    assert_response :success, "can't get old way #{way_id}, v#{current_way.version}"
-    old_way = Way.from_xml(@response.body)
-
-    # check that the ways are identical
-    assert_ways_are_equal current_way, old_way
-  end
-
-  ##
-  # look at all the versions of the way in the history and get each version from
-  # the versions call. check that they're the same.
-  def check_history_equals_versions(way_id)
-    get :history, :params => { :id => way_id }
-    assert_response :success, "can't get way #{way_id} from API"
-    history_doc = XML::Parser.string(@response.body).parse
-    assert_not_nil history_doc, "parsing way #{way_id} history failed"
-
-    history_doc.find("//osm/way").each do |way_doc|
-      history_way = Way.from_xml_node(way_doc)
-      assert_not_nil history_way, "parsing way #{way_id} version failed"
-
-      get :version, :params => { :id => way_id, :version => history_way.version }
-      assert_response :success, "couldn't get way #{way_id}, v#{history_way.version}"
-      version_way = Way.from_xml(@response.body)
-      assert_not_nil version_way, "failed to parse #{way_id}, v#{history_way.version}"
-
-      assert_ways_are_equal history_way, version_way
-    end
-  end
-
-  def do_redact_way(way, redaction)
-    get :version, :params => { :id => way.way_id, :version => way.version }
-    assert_response :success, "should be able to get version #{way.version} of way #{way.way_id}."
-
-    # now redact it
-    post :redact, :params => { :id => way.way_id, :version => way.version, :redaction => redaction.id }
-  end
-
-  def propagate_tags(way, old_way)
-    way.tags.each do |k, v|
-      create(:old_way_tag, :old_way => old_way, :k => k, :v => v)
-    end
-  end
-end

test/controllers/ways_controller_test.rb

@@ -1,752 +0,0 @@
-require "test_helper"
-
-class WaysControllerTest < ActionController::TestCase
-  ##
-  # test all routes which lead to this controller
-  def test_routes
-    assert_routing(
-      { :path => "/api/0.6/way/create", :method => :put },
-      { :controller => "ways", :action => "create" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/way/1/full", :method => :get },
-      { :controller => "ways", :action => "full", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/way/1", :method => :get },
-      { :controller => "ways", :action => "show", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/way/1", :method => :put },
-      { :controller => "ways", :action => "update", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/way/1", :method => :delete },
-      { :controller => "ways", :action => "delete", :id => "1" }
-    )
-    assert_routing(
-      { :path => "/api/0.6/ways", :method => :get },
-      { :controller => "ways", :action => "index" }
-    )
-  end
-
-  # -------------------------------------
-  # Test showing ways.
-  # -------------------------------------
-
-  def test_show
-    # check that a visible way is returned properly
-    get :show, :params => { :id => create(:way).id }
-    assert_response :success
-
-    # check that an invisible way is not returned
-    get :show, :params => { :id => create(:way, :deleted).id }
-    assert_response :gone
-
-    # check chat a non-existent way is not returned
-    get :show, :params => { :id => 0 }
-    assert_response :not_found
-  end
-
-  ##
-  # check the "full" mode
-  def test_full
-    Way.all.each do |way|
-      get :full, :params => { :id => way.id }
-
-      # full call should say "gone" for non-visible ways...
-      unless way.visible
-        assert_response :gone
-        next
-      end
-
-      # otherwise it should say success
-      assert_response :success
-
-      # Check the way is correctly returned
-      assert_select "osm way[id='#{way.id}'][version='#{way.version}'][visible='#{way.visible}']", 1
-
-      # check that each node in the way appears once in the output as a
-      # reference and as the node element.
-      way.nodes.each do |n|
-        count = (way.nodes - (way.nodes - [n])).length
-        assert_select "osm way nd[ref='#{n.id}']", count
-        assert_select "osm node[id='#{n.id}'][version='#{n.version}'][lat='#{format('%.7f', n.lat)}'][lon='#{format('%.7f', n.lon)}']", 1
-      end
-    end
-  end
-
-  ##
-  # test fetching multiple ways
-  def test_index
-    way1 = create(:way)
-    way2 = create(:way, :deleted)
-    way3 = create(:way)
-    way4 = create(:way)
-
-    # check error when no parameter provided
-    get :index
-    assert_response :bad_request
-
-    # check error when no parameter value provided
-    get :index, :params => { :ways => "" }
-    assert_response :bad_request
-
-    # test a working call
-    get :index, :params => { :ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id}" }
-    assert_response :success
-    assert_select "osm" do
-      assert_select "way", :count => 4
-      assert_select "way[id='#{way1.id}'][visible='true']", :count => 1
-      assert_select "way[id='#{way2.id}'][visible='false']", :count => 1
-      assert_select "way[id='#{way3.id}'][visible='true']", :count => 1
-      assert_select "way[id='#{way4.id}'][visible='true']", :count => 1
-    end
-
-    # check error when a non-existent way is included
-    get :index, :params => { :ways => "#{way1.id},#{way2.id},#{way3.id},#{way4.id},0" }
-    assert_response :not_found
-  end
-
-  # -------------------------------------
-  # Test simple way creation.
-  # -------------------------------------
-
-  def test_create
-    node1 = create(:node)
-    node2 = create(:node)
-    private_user = create(:user, :data_public => false)
-    private_changeset = create(:changeset, :user => private_user)
-    user = create(:user)
-    changeset = create(:changeset, :user => user)
-
-    ## First check that it fails when creating a way using a non-public user
-    basic_authorization private_user.email, "test"
-
-    # use the first user's open changeset
-    changeset_id = private_changeset.id
-
-    # create a way with pre-existing nodes
-    xml = "<osm><way changeset='#{changeset_id}'>" \
-          "<nd ref='#{node1.id}'/><nd ref='#{node2.id}'/>" \
-          "<tag k='test' v='yes' /></way></osm>"
-    put :create, :body => xml
-    # hope for failure
-    assert_response :forbidden,
-                    "way upload did not return forbidden status"
-
-    ## Now use a public user
-    basic_authorization user.email, "test"
-
-    # use the first user's open changeset
-    changeset_id = changeset.id
-
-    # create a way with pre-existing nodes
-    xml = "<osm><way changeset='#{changeset_id}'>" \
-          "<nd ref='#{node1.id}'/><nd ref='#{node2.id}'/>" \
-          "<tag k='test' v='yes' /></way></osm>"
-    put :create, :body => xml
-    # hope for success
-    assert_response :success,
-                    "way upload did not return success status"
-    # read id of created way and search for it
-    wayid = @response.body
-    checkway = Way.find(wayid)
-    assert_not_nil checkway,
-                   "uploaded way not found in data base after upload"
-    # compare values
-    assert_equal checkway.nds.length, 2,
-                 "saved way does not contain exactly one node"
-    assert_equal checkway.nds[0], node1.id,
-                 "saved way does not contain the right node on pos 0"
-    assert_equal checkway.nds[1], node2.id,
-                 "saved way does not contain the right node on pos 1"
-    assert_equal checkway.changeset_id, changeset_id,
-                 "saved way does not belong to the correct changeset"
-    assert_equal user.id, checkway.changeset.user_id,
-                 "saved way does not belong to user that created it"
-    assert_equal true, checkway.visible,
-                 "saved way is not visible"
-  end
-
-  # -------------------------------------
-  # Test creating some invalid ways.
-  # -------------------------------------
-
-  def test_create_invalid
-    node = create(:node)
-    private_user = create(:user, :data_public => false)
-    private_open_changeset = create(:changeset, :user => private_user)
-    private_closed_changeset = create(:changeset, :closed, :user => private_user)
-    user = create(:user)
-    open_changeset = create(:changeset, :user => user)
-    closed_changeset = create(:changeset, :closed, :user => user)
-
-    ## First test with a private user to make sure that they are not authorized
-    basic_authorization private_user.email, "test"
-
-    # use the first user's open changeset
-    # create a way with non-existing node
-    xml = "<osm><way changeset='#{private_open_changeset.id}'>" \
-          "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
-    put :create, :body => xml
-    # expect failure
-    assert_response :forbidden,
-                    "way upload with invalid node using a private user did not return 'forbidden'"
-
-    # create a way with no nodes
-    xml = "<osm><way changeset='#{private_open_changeset.id}'>" \
-          "<tag k='test' v='yes' /></way></osm>"
-    put :create, :body => xml
-    # expect failure
-    assert_response :forbidden,
-                    "way upload with no node using a private userdid not return 'forbidden'"
-
-    # create a way inside a closed changeset
-    xml = "<osm><way changeset='#{private_closed_changeset.id}'>" \
-          "<nd ref='#{node.id}'/></way></osm>"
-    put :create, :body => xml
-    # expect failure
-    assert_response :forbidden,
-                    "way upload to closed changeset with a private user did not return 'forbidden'"
-
-    ## Now test with a public user
-    basic_authorization user.email, "test"
-
-    # use the first user's open changeset
-    # create a way with non-existing node
-    xml = "<osm><way changeset='#{open_changeset.id}'>" \
-          "<nd ref='0'/><tag k='test' v='yes' /></way></osm>"
-    put :create, :body => xml
-    # expect failure
-    assert_response :precondition_failed,
-                    "way upload with invalid node did not return 'precondition failed'"
-    assert_equal "Precondition failed: Way  requires the nodes with id in (0), which either do not exist, or are not visible.", @response.body
-
-    # create a way with no nodes
-    xml = "<osm><way changeset='#{open_changeset.id}'>" \
-          "<tag k='test' v='yes' /></way></osm>"
-    put :create, :body => xml
-    # expect failure
-    assert_response :precondition_failed,
-                    "way upload with no node did not return 'precondition failed'"
-    assert_equal "Precondition failed: Cannot create way: data is invalid.", @response.body
-
-    # create a way inside a closed changeset
-    xml = "<osm><way changeset='#{closed_changeset.id}'>" \
-          "<nd ref='#{node.id}'/></way></osm>"
-    put :create, :body => xml
-    # expect failure
-    assert_response :conflict,
-                    "way upload to closed changeset did not return 'conflict'"
-
-    # create a way with a tag which is too long
-    xml = "<osm><way changeset='#{open_changeset.id}'>" \
-          "<nd ref='#{node.id}'/>" \
-          "<tag k='foo' v='#{'x' * 256}'/>" \
-          "</way></osm>"
-    put :create, :body => xml
-    # expect failure
-    assert_response :bad_request,
-                    "way upload to with too long tag did not return 'bad_request'"
-  end
-
-  # -------------------------------------
-  # Test deleting ways.
-  # -------------------------------------
-
-  def test_delete
-    private_user = create(:user, :data_public => false)
-    private_open_changeset = create(:changeset, :user => private_user)
-    private_closed_changeset = create(:changeset, :closed, :user => private_user)
-    private_way = create(:way, :changeset => private_open_changeset)
-    private_deleted_way = create(:way, :deleted, :changeset => private_open_changeset)
-    private_used_way = create(:way, :changeset => private_open_changeset)
-    create(:relation_member, :member => private_used_way)
-    user = create(:user)
-    open_changeset = create(:changeset, :user => user)
-    closed_changeset = create(:changeset, :closed, :user => user)
-    way = create(:way, :changeset => open_changeset)
-    deleted_way = create(:way, :deleted, :changeset => open_changeset)
-    used_way = create(:way, :changeset => open_changeset)
-    relation_member = create(:relation_member, :member => used_way)
-    relation = relation_member.relation
-
-    # first try to delete way without auth
-    delete :delete, :params => { :id => way.id }
-    assert_response :unauthorized
-
-    # now set auth using the private user
-    basic_authorization private_user.email, "test"
-
-    # this shouldn't work as with the 0.6 api we need pay load to delete
-    delete :delete, :params => { :id => private_way.id }
-    assert_response :forbidden
-
-    # Now try without having a changeset
-    xml = "<osm><way id='#{private_way.id}'/></osm>"
-    delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
-    assert_response :forbidden
-
-    # try to delete with an invalid (closed) changeset
-    xml = update_changeset(private_way.to_xml, private_closed_changeset.id)
-    delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
-    assert_response :forbidden
-
-    # try to delete with an invalid (non-existent) changeset
-    xml = update_changeset(private_way.to_xml, 0)
-    delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
-    assert_response :forbidden
-
-    # Now try with a valid changeset
-    xml = private_way.to_xml
-    delete :delete, :params => { :id => private_way.id }, :body => xml.to_s
-    assert_response :forbidden
-
-    # check the returned value - should be the new version number
-    # valid delete should return the new version number, which should
-    # be greater than the old version number
-    # assert @response.body.to_i > current_ways(:visible_way).version,
-    #   "delete request should return a new version number for way"
-
-    # this won't work since the way is already deleted
-    xml = private_deleted_way.to_xml
-    delete :delete, :params => { :id => private_deleted_way.id }, :body => xml.to_s
-    assert_response :forbidden
-
-    # this shouldn't work as the way is used in a relation
-    xml = private_used_way.to_xml
-    delete :delete, :params => { :id => private_used_way.id }, :body => xml.to_s
-    assert_response :forbidden,
-                    "shouldn't be able to delete a way used in a relation (#{@response.body}), when done by a private user"
-
-    # this won't work since the way never existed
-    delete :delete, :params => { :id => 0 }
-    assert_response :forbidden
-
-    ### Now check with a public user
-    # now set auth
-    basic_authorization user.email, "test"
-
-    # this shouldn't work as with the 0.6 api we need pay load to delete
-    delete :delete, :params => { :id => way.id }
-    assert_response :bad_request
-
-    # Now try without having a changeset
-    xml = "<osm><way id='#{way.id}'/></osm>"
-    delete :delete, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :bad_request
-
-    # try to delete with an invalid (closed) changeset
-    xml = update_changeset(way.to_xml, closed_changeset.id)
-    delete :delete, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :conflict
-
-    # try to delete with an invalid (non-existent) changeset
-    xml = update_changeset(way.to_xml, 0)
-    delete :delete, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :conflict
-
-    # Now try with a valid changeset
-    xml = way.to_xml
-    delete :delete, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :success
-
-    # check the returned value - should be the new version number
-    # valid delete should return the new version number, which should
-    # be greater than the old version number
-    assert @response.body.to_i > way.version,
-           "delete request should return a new version number for way"
-
-    # this won't work since the way is already deleted
-    xml = deleted_way.to_xml
-    delete :delete, :params => { :id => deleted_way.id }, :body => xml.to_s
-    assert_response :gone
-
-    # this shouldn't work as the way is used in a relation
-    xml = used_way.to_xml
-    delete :delete, :params => { :id => used_way.id }, :body => xml.to_s
-    assert_response :precondition_failed,
-                    "shouldn't be able to delete a way used in a relation (#{@response.body})"
-    assert_equal "Precondition failed: Way #{used_way.id} is still used by relations #{relation.id}.", @response.body
-
-    # this won't work since the way never existed
-    delete :delete, :params => { :id => 0 }
-    assert_response :not_found
-  end
-
-  ##
-  # tests whether the API works and prevents incorrect use while trying
-  # to update ways.
-  def test_update
-    private_user = create(:user, :data_public => false)
-    private_way = create(:way, :changeset => create(:changeset, :user => private_user))
-    user = create(:user)
-    way = create(:way, :changeset => create(:changeset, :user => user))
-    node = create(:node)
-    create(:way_node, :way => private_way, :node => node)
-    create(:way_node, :way => way, :node => node)
-
-    ## First test with no user credentials
-    # try and update a way without authorisation
-    xml = way.to_xml
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :unauthorized
-
-    ## Second test with the private user
-
-    # setup auth
-    basic_authorization private_user.email, "test"
-
-    ## trying to break changesets
-
-    # try and update in someone else's changeset
-    xml = update_changeset(private_way.to_xml,
-                           create(:changeset).id)
-    put :update, :params => { :id => private_way.id }, :body => xml.to_s
-    assert_require_public_data "update with other user's changeset should be forbidden when date isn't public"
-
-    # try and update in a closed changeset
-    xml = update_changeset(private_way.to_xml,
-                           create(:changeset, :closed, :user => private_user).id)
-    put :update, :params => { :id => private_way.id }, :body => xml.to_s
-    assert_require_public_data "update with closed changeset should be forbidden, when data isn't public"
-
-    # try and update in a non-existant changeset
-    xml = update_changeset(private_way.to_xml, 0)
-    put :update, :params => { :id => private_way.id }, :body => xml.to_s
-    assert_require_public_data("update with changeset=0 should be forbidden, when data isn't public")
-
-    ## try and submit invalid updates
-    xml = xml_replace_node(private_way.to_xml, node.id, 9999)
-    put :update, :params => { :id => private_way.id }, :body => xml.to_s
-    assert_require_public_data "way with non-existent node should be forbidden, when data isn't public"
-
-    xml = xml_replace_node(private_way.to_xml, node.id, create(:node, :deleted).id)
-    put :update, :params => { :id => private_way.id }, :body => xml.to_s
-    assert_require_public_data "way with deleted node should be forbidden, when data isn't public"
-
-    ## finally, produce a good request which will still not work
-    xml = private_way.to_xml
-    put :update, :params => { :id => private_way.id }, :body => xml.to_s
-    assert_require_public_data "should have failed with a forbidden when data isn't public"
-
-    ## Finally test with the public user
-
-    # setup auth
-    basic_authorization user.email, "test"
-
-    ## trying to break changesets
-
-    # try and update in someone else's changeset
-    xml = update_changeset(way.to_xml,
-                           create(:changeset).id)
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :conflict, "update with other user's changeset should be rejected"
-
-    # try and update in a closed changeset
-    xml = update_changeset(way.to_xml,
-                           create(:changeset, :closed, :user => user).id)
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :conflict, "update with closed changeset should be rejected"
-
-    # try and update in a non-existant changeset
-    xml = update_changeset(way.to_xml, 0)
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :conflict, "update with changeset=0 should be rejected"
-
-    ## try and submit invalid updates
-    xml = xml_replace_node(way.to_xml, node.id, 9999)
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :precondition_failed, "way with non-existent node should be rejected"
-
-    xml = xml_replace_node(way.to_xml, node.id, create(:node, :deleted).id)
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :precondition_failed, "way with deleted node should be rejected"
-
-    ## next, attack the versioning
-    current_way_version = way.version
-
-    # try and submit a version behind
-    xml = xml_attr_rewrite(way.to_xml,
-                           "version", current_way_version - 1)
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :conflict, "should have failed on old version number"
-
-    # try and submit a version ahead
-    xml = xml_attr_rewrite(way.to_xml,
-                           "version", current_way_version + 1)
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :conflict, "should have failed on skipped version number"
-
-    # try and submit total crap in the version field
-    xml = xml_attr_rewrite(way.to_xml,
-                           "version", "p1r4t3s!")
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :conflict,
-                    "should not be able to put 'p1r4at3s!' in the version field"
-
-    ## try an update with the wrong ID
-    xml = create(:way).to_xml
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :bad_request,
-                    "should not be able to update a way with a different ID from the XML"
-
-    ## try an update with a minimal valid XML doc which isn't a well-formed OSM doc.
-    xml = "<update/>"
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :bad_request,
-                    "should not be able to update a way with non-OSM XML doc."
-
-    ## finally, produce a good request which should work
-    xml = way.to_xml
-    put :update, :params => { :id => way.id }, :body => xml.to_s
-    assert_response :success, "a valid update request failed"
-  end
-
-  # ------------------------------------------------------------
-  # test tags handling
-  # ------------------------------------------------------------
-
-  ##
-  # Try adding a new tag to a way
-  def test_add_tags
-    private_user = create(:user, :data_public => false)
-    private_way = create(:way_with_nodes, :nodes_count => 2, :changeset => create(:changeset, :user => private_user))
-    user = create(:user)
-    way = create(:way_with_nodes, :nodes_count => 2, :changeset => create(:changeset, :user => user))
-
-    ## Try with the non-public user
-    # setup auth
-    basic_authorization private_user.email, "test"
-
-    # add an identical tag to the way
-    tag_xml = XML::Node.new("tag")
-    tag_xml["k"] = "new"
-    tag_xml["v"] = "yes"
-
-    # add the tag into the existing xml
-    way_xml = private_way.to_xml
-    way_xml.find("//osm/way").first << tag_xml
-
-    # try and upload it
-    put :update, :params => { :id => private_way.id }, :body => way_xml.to_s
-    assert_response :forbidden,
-                    "adding a duplicate tag to a way for a non-public should fail with 'forbidden'"
-
-    ## Now try with the public user
-    # setup auth
-    basic_authorization user.email, "test"
-
-    # add an identical tag to the way
-    tag_xml = XML::Node.new("tag")
-    tag_xml["k"] = "new"
-    tag_xml["v"] = "yes"
-
-    # add the tag into the existing xml
-    way_xml = way.to_xml
-    way_xml.find("//osm/way").first << tag_xml
-
-    # try and upload it
-    put :update, :params => { :id => way.id }, :body => way_xml.to_s
-    assert_response :success,
-                    "adding a new tag to a way should succeed"
-    assert_equal way.version + 1, @response.body.to_i
-  end
-
-  ##
-  # Try adding a duplicate of an existing tag to a way
-  def test_add_duplicate_tags
-    private_user = create(:user, :data_public => false)
-    private_way = create(:way, :changeset => create(:changeset, :user => private_user))
-    private_existing_tag = create(:way_tag, :way => private_way)
-    user = create(:user)
-    way = create(:way, :changeset => create(:changeset, :user => user))
-    existing_tag = create(:way_tag, :way => way)
-
-    ## Try with the non-public user
-    # setup auth
-    basic_authorization private_user.email, "test"
-
-    # add an identical tag to the way
-    tag_xml = XML::Node.new("tag")
-    tag_xml["k"] = private_existing_tag.k
-    tag_xml["v"] = private_existing_tag.v
-
-    # add the tag into the existing xml
-    way_xml = private_way.to_xml
-    way_xml.find("//osm/way").first << tag_xml
-
-    # try and upload it
-    put :update, :params => { :id => private_way.id }, :body => way_xml.to_s
-    assert_response :forbidden,
-                    "adding a duplicate tag to a way for a non-public should fail with 'forbidden'"
-
-    ## Now try with the public user
-    # setup auth
-    basic_authorization user.email, "test"
-
-    # add an identical tag to the way
-    tag_xml = XML::Node.new("tag")
-    tag_xml["k"] = existing_tag.k
-    tag_xml["v"] = existing_tag.v
-
-    # add the tag into the existing xml
-    way_xml = way.to_xml
-    way_xml.find("//osm/way").first << tag_xml
-
-    # try and upload it
-    put :update, :params => { :id => way.id }, :body => way_xml.to_s
-    assert_response :bad_request,
-                    "adding a duplicate tag to a way should fail with 'bad request'"
-    assert_equal "Element way/#{way.id} has duplicate tags with key #{existing_tag.k}", @response.body
-  end
-
-  ##
-  # Try adding a new duplicate tags to a way
-  def test_new_duplicate_tags
-    private_user = create(:user, :data_public => false)
-    private_way = create(:way, :changeset => create(:changeset, :user => private_user))
-    user = create(:user)
-    way = create(:way, :changeset => create(:changeset, :user => user))
-
-    ## First test with the non-public user so should be rejected
-    # setup auth
-    basic_authorization private_user.email, "test"
-
-    # create duplicate tag
-    tag_xml = XML::Node.new("tag")
-    tag_xml["k"] = "i_am_a_duplicate"
-    tag_xml["v"] = "foobar"
-
-    # add the tag into the existing xml
-    way_xml = private_way.to_xml
-
-    # add two copies of the tag
-    way_xml.find("//osm/way").first << tag_xml.copy(true) << tag_xml
-
-    # try and upload it
-    put :update, :params => { :id => private_way.id }, :body => way_xml.to_s
-    assert_response :forbidden,
-                    "adding new duplicate tags to a way using a non-public user should fail with 'forbidden'"
-
-    ## Now test with the public user
-    # setup auth
-    basic_authorization user.email, "test"
-
-    # create duplicate tag
-    tag_xml = XML::Node.new("tag")
-    tag_xml["k"] = "i_am_a_duplicate"
-    tag_xml["v"] = "foobar"
-
-    # add the tag into the existing xml
-    way_xml = way.to_xml
-
-    # add two copies of the tag
-    way_xml.find("//osm/way").first << tag_xml.copy(true) << tag_xml
-
-    # try and upload it
-    put :update, :params => { :id => way.id }, :body => way_xml.to_s
-    assert_response :bad_request,
-                    "adding new duplicate tags to a way should fail with 'bad request'"
-    assert_equal "Element way/#{way.id} has duplicate tags with key i_am_a_duplicate", @response.body
-  end
-
-  ##
-  # Try adding a new duplicate tags to a way.
-  # But be a bit subtle - use unicode decoding ambiguities to use different
-  # binary strings which have the same decoding.
-  def test_invalid_duplicate_tags
-    private_user = create(:user, :data_public => false)
-    private_changeset = create(:changeset, :user => private_user)
-    user = create(:user)
-    changeset = create(:changeset, :user => user)
-
-    ## First make sure that you can't with a non-public user
-    # setup auth
-    basic_authorization private_user.email, "test"
-
-    # add the tag into the existing xml
-    way_str = "<osm><way changeset='#{private_changeset.id}'>"
-    way_str << "<tag k='addr:housenumber' v='1'/>"
-    way_str << "<tag k='addr:housenumber' v='2'/>"
-    way_str << "</way></osm>"
-
-    # try and upload it
-    put :create, :body => way_str
-    assert_response :forbidden,
-                    "adding new duplicate tags to a way with a non-public user should fail with 'forbidden'"
-
-    ## Now do it with a public user
-    # setup auth
-    basic_authorization user.email, "test"
-
-    # add the tag into the existing xml
-    way_str = "<osm><way changeset='#{changeset.id}'>"
-    way_str << "<tag k='addr:housenumber' v='1'/>"
-    way_str << "<tag k='addr:housenumber' v='2'/>"
-    way_str << "</way></osm>"
-
-    # try and upload it
-    put :create, :body => way_str
-    assert_response :bad_request,
-                    "adding new duplicate tags to a way should fail with 'bad request'"
-    assert_equal "Element way/ has duplicate tags with key addr:housenumber", @response.body
-  end
-
-  ##
-  # test that a call to ways_for_node returns all ways that contain the node
-  # and none that don't.
-  def test_ways_for_node
-    node = create(:node)
-    way1 = create(:way)
-    way2 = create(:way)
-    create(:way_node, :way => way1, :node => node)
-    create(:way_node, :way => way2, :node => node)
-    # create an unrelated way
-    create(:way_with_nodes, :nodes_count => 2)
-    # create a way which used to use the node
-    way3_v1 = create(:old_way, :version => 1)
-    _way3_v2 = create(:old_way, :current_way => way3_v1.current_way, :version => 2)
-    create(:old_way_node, :old_way => way3_v1, :node => node)
-
-    get :ways_for_node, :params => { :id => node.id }
-    assert_response :success
-    ways_xml = XML::Parser.string(@response.body).parse
-    assert_not_nil ways_xml, "failed to parse ways_for_node response"
-
-    # check that the set of IDs match expectations
-    expected_way_ids = [way1.id,
-                        way2.id]
-    found_way_ids = ways_xml.find("//osm/way").collect { |w| w["id"].to_i }
-    assert_equal expected_way_ids.sort, found_way_ids.sort,
-                 "expected ways for node #{node.id} did not match found"
-
-    # check the full ways to ensure we're not missing anything
-    expected_way_ids.each do |id|
-      way_xml = ways_xml.find("//osm/way[@id='#{id}']").first
-      assert_ways_are_equal(Way.find(id),
-                            Way.from_xml_node(way_xml))
-    end
-  end
-
-  ##
-  # update the changeset_id of a way element
-  def update_changeset(xml, changeset_id)
-    xml_attr_rewrite(xml, "changeset", changeset_id)
-  end
-
-  ##
-  # update an attribute in the way element
-  def xml_attr_rewrite(xml, name, value)
-    xml.find("//osm/way").first[name] = value.to_s
-    xml
-  end
-
-  ##
-  # replace a node in a way element
-  def xml_replace_node(xml, old_node, new_node)
-    xml.find("//osm/way/nd[@ref='#{old_node}']").first["ref"] = new_node.to_s
-    xml
-  end
-end