Merge 15165:15373 from trunk.

app/controllers/amf_controller.rb

@@ -127,8 +127,10 @@ class AmfController < ApplicationController
           results[index]=[-5,nil]
         else
           case message
-            when 'putway';			  r=putway(renumberednodes,*args)
-                      						renumberednodes=r[4]
+            when 'putway';			  orn=renumberednodes.dup
+                                  r=putway(renumberednodes,*args)
+                      						renumberednodes=r[4].dup
+                      						r[4].delete_if { |k,v| orn.has_key?(k) }
                       						if r[2] != r[3] then renumberedways[r[2]] = r[3] end
                       						results[index]=AMF.putdata(index,r)
             when 'putrelation';		results[index]=AMF.putdata(index,putrelation(renumberednodes, renumberedways, *args))
@@ -176,6 +178,7 @@ class AmfController < ApplicationController
       cs = Changeset.new
       cs.tags = cstags
       cs.user_id = user.id
+      if !closecomment.empty? then cs.tags['comment']=closecomment end
       # smsm1 doesn't like the next two lines and thinks they need to be abstracted to the model more/better
       cs.created_at = Time.now.getutc
       cs.closed_at = cs.created_at + Changeset::IDLE_TIMEOUT
@@ -477,7 +480,7 @@ class AmfController < ApplicationController
         rels.push([rel.id, rel.tags, rel.members, rel.version])
       end
     else
-      RelationTag.find(:all, :limit => 11, :conditions => ["match(v) against (?)", searchterm] ).each do |t|
+      RelationTag.find(:all, :limit => 11, :conditions => ["v like ?", "%#{searchterm}%"] ).each do |t|
       if t.relation.visible then
 	      rels.push([t.relation.id, t.relation.tags, t.relation.members, t.relation.version])
 	    end
@@ -592,9 +595,7 @@ class AmfController < ApplicationController
     if pointlist.length < 2 then return -2,"Server error - way is only #{points.length} points long." end
 
     originalway = originalway.to_i
-logger.info("received #{pointlist} for #{originalway}")
   	pointlist.collect! {|a| a.to_i }
-logger.info("converted to #{pointlist}")
 
     way=nil	# this is returned, so scope it outside the transaction
     nodeversions = {}
@@ -634,11 +635,9 @@ logger.info("converted to #{pointlist}")
 
       # -- Save revised way
 
-logger.info("renumberednodes is #{renumberednodes.inspect}")
   	  pointlist.collect! {|a|
 	      renumberednodes[a] ? renumberednodes[a]:a
   	  } # renumber nodes
-logger.info("saving with #{pointlist}")
       new_way = Way.new
       new_way.tags = attributes
       new_way.nds = pointlist

app/controllers/browse_controller.rb

@@ -27,7 +27,7 @@ class BrowseController < ApplicationController
   end
   
   def way
-    @way = Way.find(params[:id])
+    @way = Way.find(params[:id], :include => [:way_tags, {:changeset => :user}, {:nodes => [:node_tags, {:ways => :way_tags}]}, :containing_relation_members])
     @next = Way.find(:first, :order => "id ASC", :conditions => [ "visible = true AND id > :id", { :id => @way.id }] )
     @prev = Way.find(:first, :order => "id DESC", :conditions => [ "visible = true AND id < :id", { :id => @way.id }] )
   rescue ActiveRecord::RecordNotFound
@@ -36,7 +36,7 @@ class BrowseController < ApplicationController
   end
   
   def way_history
-    @way = Way.find(params[:id])
+    @way = Way.find(params[:id], :include => [:way_tags, {:old_ways => {:changeset => :user}}])
   rescue ActiveRecord::RecordNotFound
     @type = "way"
     render :action => "not_found", :status => :not_found

app/controllers/way_controller.rb

@@ -60,18 +60,19 @@ class WayController < ApplicationController
   end
 
   def full
-    way = Way.find(params[:id])
+    way = Way.find(params[:id], :include => {:nodes => :node_tags})
     
     if way.visible
-      nd_ids = way.nds + [-1]
-      nodes = Node.find(:all, :conditions => ["visible = ? AND id IN (#{nd_ids.join(',')})", true])
+      changeset_cache = {}
+      user_display_name_cache = {}
 
-      # Render
       doc = OSM::API.new.get_xml_doc
-      nodes.each do |node|
-        doc.root << node.to_xml_node()
+      way.nodes.each do |node|
+        if node.visible
+          doc.root << node.to_xml_node(changeset_cache, user_display_name_cache)
         end
-      doc.root << way.to_xml_node()
+      end
+      doc.root << way.to_xml_node(nil, changeset_cache, user_display_name_cache)
       
       render :text => doc.to_s, :content_type => "text/xml"
     else

app/models/old_relation.rb

@@ -25,7 +25,7 @@ class OldRelation < ActiveRecord::Base
     save!
     clear_aggregation_cache
     clear_association_cache
-    @attributes.update(OldRelation.find(:first, :conditions => ['id = ? AND timestamp = ?', self.id, self.timestamp]).instance_variable_get('@attributes'))
+    @attributes.update(OldRelation.find(:first, :conditions => ['id = ? AND timestamp = ?', self.id, self.timestamp], :order => "version desc").instance_variable_get('@attributes'))
 
     # ok, you can touch from here on
 
@@ -81,7 +81,7 @@ class OldRelation < ActiveRecord::Base
 #  has_many :relation_tags, :class_name => 'OldRelationTag', :foreign_key => 'id'
 
   def old_members
-    OldRelationMember.find(:all, :conditions => ['id = ? AND version = ?', self.id, self.version])    
+    OldRelationMember.find(:all, :conditions => ['id = ? AND version = ?', self.id, self.version], :order => "sequence_id")
   end
 
   def old_tags

app/models/old_way.rb

@@ -30,7 +30,7 @@ class OldWay < ActiveRecord::Base
     save!
     clear_aggregation_cache
     clear_association_cache
-    @attributes.update(OldWay.find(:first, :conditions => ['id = ? AND timestamp = ?', self.id, self.timestamp]).instance_variable_get('@attributes'))
+    @attributes.update(OldWay.find(:first, :conditions => ['id = ? AND timestamp = ?', self.id, self.timestamp], :order => "version desc").instance_variable_get('@attributes'))
 
     # ok, you can touch from here on
 

app/models/relation.rb

@@ -208,7 +208,7 @@ class Relation < ActiveRecord::Base
 
   def add_member(type,id,role)
     @members = Array.new unless @members
-    @members += [[type,id,role]]
+    @members += [[type,id.to_i,role]]
   end
 
   def add_tag_keyval(k, v)
@@ -385,21 +385,18 @@ class Relation < ActiveRecord::Base
       # changed members in an array, as the bounding box updates for
       # elements are per-element, not blanked on/off like for tags.
       changed_members = Array.new
-      members = Hash.new
-      self.members.each do |m|
-        # should be: h[[m.id, m.type]] = m.role, but someone prefers arrays
-        members[[m[1], m[0]]] = m[2]
-      end
-      relation_members.each do |old_member|
-        key = [old_member.member_id.to_s, old_member.member_type]
-        if members.has_key? key
-          members.delete key
-        else
+      members = self.members.clone
+      self.relation_members.each do |old_member|
+        key = [old_member.member_type, old_member.member_id, old_member.member_role]
+        i = members.index key
+        if i.nil?
           changed_members << key
+        else
+          members.delete_at i
         end
       end
       # any remaining members must be new additions
-      changed_members += members.keys
+      changed_members += members
 
       # update the members. first delete all the old members, as the new
       # members may be in a different order and i don't feel like implementing
@@ -433,23 +430,19 @@ class Relation < ActiveRecord::Base
         changed_members.collect { |id,type| type == "relation" }.
         inject(false) { |b,s| b or s }
 
-      if tags_changed or any_relations
+      update_members = if tags_changed or any_relations
                          # add all non-relation bounding boxes to the changeset
                          # FIXME: check for tag changes along with element deletions and
                          # make sure that the deleted element's bounding box is hit.
-        self.members.each do |type, id, role|
-          if type != "Relation"
-            update_changeset_element(type, id)
-          end
-        end
+                         self.members
                        else 
-        # add only changed members to the changeset
-        changed_members.each do |id, type|
+                         changed_members
+                       end
+      update_members.each do |type, id, role|
         if type != "Relation"
           update_changeset_element(type, id)
         end
       end
-      end
 
       # tell the changeset we updated one element only
       changeset.add_changes! 1

app/views/browse/_node_details.rhtml

@@ -2,6 +2,11 @@
 
   <%= render :partial => "common_details", :object => node_details %>
 
+  <tr>
+    <th>Coordinates:</th>
+    <td><div class="geo"><a href="/?lat=<%= h(node_details.lat) %>&lon=<%= h(node_details.lon) %>&zoom=18"><span class="latitude"><%= h(node_details.lat) %></span>, <span class="longitude"><%= h(node_details.lon) %></span></a></div></td>
+  </tr>
+
   <% unless node_details.ways.empty? and node_details.containing_relation_members.empty? %>
     <tr valign="top">
       <th><%= t 'browse.node_details.part_of' %></th>

app/views/browse/_tag.rhtml

@@ -1,3 +1,3 @@
 <tr>
-  <td><%= h(tag[0]) %> = <%= h(tag[1]) %></td>
+  <td><%= h(tag[0]) %> = <%= sanitize(auto_link(tag[1])) %></td>
 </tr> 

app/views/browse/_way_details.rhtml

@@ -7,7 +7,12 @@
     <td>
       <table cellpadding="0">
         <% way_details.way_nodes.each do |wn| %>
-          <tr><td><%= link_to h(printable_name(wn.node)), :action => "node", :id => wn.node_id.to_s %></td></tr>
+          <tr><td>
+            <%= link_to h(printable_name(wn.node)), :action => "node", :id => wn.node_id.to_s %>
+	    <% if wn.node.ways.size > 1 then %>
+	      (also part of <%= wn.node.ways.reject { |w| w.id == way_details.id }.map { |w| link_to(h(printable_name(w)), :action => "way", :id => w.id.to_s) }.join(", ") %>)
+            <% end %>
+	  </td></tr>
         <% end %>
       </table>
     </td>

app/views/changeset/_changeset.rhtml

@@ -5,14 +5,14 @@
     #<%= changeset.id %>
   </td>
 
-  <td class="<%= cl %>">
+  <td class="<%= cl %> date">
     <% if changeset.closed_at > DateTime.now %> <%= t'changeset.changeset.still_editing' %>
     <% else %><%= changeset.closed_at.strftime("%d %b %Y %H:%M") %><% end %>
   </td>
 
     
   <%if showusername %>  
-    <td class="<%= cl %>">
+    <td class="<%= cl %> user">
     <% if changeset.user.data_public? %>
       <%= link_to h(changeset.user.display_name), :controller => "changeset", :action => "list_user", :display_name => changeset.user.display_name %>
     <% else %>
@@ -29,7 +29,7 @@
     <% end %>
   </td>
 
-  <td class="<%= cl %>">
+  <td class="<%= cl %> area">
     <% if changeset.min_lat.nil? %>
       <%= t'changeset.changeset.no_edits' %>
     <% else 

app/views/export/start.rjs

@@ -327,7 +327,7 @@ page << <<EOJ
     var ymax = bounds.top * Math.PI / 180;
     var yzoom = Math.LOG2E * (Math.log(2000 * 2 * Math.PI) - Math.log(Math.log((Math.tan(ymax) + 1 / Math.cos(ymax)) / (Math.tan(ymin) + 1 / Math.cos(ymin)))))
 
-    return Math.floor(Math.min(xzoom, yzoom));
+    return Math.min(Math.floor(Math.min(xzoom, yzoom)), 17);
   }
 
   function roundScale(scale) {

app/views/layouts/site.rhtml

@@ -110,7 +110,7 @@
       </div>
 
       <div id="sotm" class="notice">
-        <%= t 'layouts.sotm' %>
+        <%= link_to image_tag("sotm.png", :alt => "State of the Map", :border => "0"), "http://www.stateofthemap.org/" %>
       </div>
 
       <%= yield :optionals %>

config/initializers/action_mailer.rb

@@ -1,4 +1,4 @@
-# Configure ActionMailer
+# Configure ActionMailer SMTP settings
 ActionMailer::Base.smtp_settings = {
   :address => 'localhost',
   :port => 25, 

config/initializers/load_configs.rb

@@ -2,9 +2,3 @@
 
 # Load application config
 APP_CONFIG = YAML.load(File.read(RAILS_ROOT + "/config/application.yml"))[RAILS_ENV]
-# This will let you more easily use helpers based on url_for in your mailers.
-ActionMailer::Base.default_url_options[:host] = APP_CONFIG['host']
-
-# Load texts in a particular language
-TEXT = YAML.load(File.read(RAILS_ROOT + "/config/text_outputs/en.yml"))
-

config/lighttpd.conf

@@ -110,6 +110,19 @@ url.redirect = (
   "^/wiki/(.*)$" => "http://wiki.openstreetmap.org/$1"
 )
 
+#
+# Redirect everything except www.openstreetmap.org and
+# api.openstreetmap.org to www.openstreetmap.org
+#
+$HTTP["host"] =~ "^api\." {
+  $HTTP["host"] != "api.openstreetmap.org" {
+    url.redirect = ( "^(.*)$" => "http://api.openstreetmap.org$1" )
+  }
+}
+else $HTTP["host"] != "www.openstreetmap.org" {
+  url.redirect = ( "^(.*)$" => "http://www.openstreetmap.org$1" )
+} 
+
 #
 # Run anything with a .pl iextension as a CGI script
 #

config/text_outputs/en.yml

@@ -1,3 +0,0 @@
-boundary_parameter_required: "The parameter bbox is required, and must be of the form min_lon,min_lat,max_lon,max_lat"
-
-

db/migrate/005_tile_tracepoints.rb

@@ -6,9 +6,9 @@ class TileTracepoints < ActiveRecord::Migration
     add_index "gps_points", ["tile"], :name => "points_tile_idx"
     remove_index "gps_points", :name => "points_idx"
 
-    begin
+    if ENV["USE_DB_FUNCTIONS"]
       Tracepoint.update_all("latitude = latitude * 10, longitude = longitude * 10, tile = tile_for_point(latitude * 10, longitude * 10)")
-    rescue ActiveRecord::StatementInvalid => ex
+    else
       Tracepoint.find(:all).each do |tp|
         tp.latitude = tp.latitude * 10
         tp.longitude = tp.longitude * 10

db/migrate/006_tile_nodes.rb

@@ -2,7 +2,7 @@ require 'lib/migrate'
 
 class TileNodes < ActiveRecord::Migration
   def self.upgrade_table(from_table, to_table, model)
-    begin
+    if ENV["USE_DB_FUNCTIONS"]
       execute <<-END_SQL
       INSERT INTO #{to_table} (id, latitude, longitude, user_id, visible, tags, timestamp, tile)
       SELECT id, ROUND(latitude * 10000000), ROUND(longitude * 10000000),
@@ -11,7 +11,7 @@ class TileNodes < ActiveRecord::Migration
                             CAST(ROUND(longitude * 10000000) AS INTEGER))
       FROM #{from_table}
       END_SQL
-    rescue ActiveRecord::StatementInvalid => ex
+    else
       execute <<-END_SQL
       INSERT INTO #{to_table} (id, latitude, longitude, user_id, visible, tags, timestamp, tile)
       SELECT id, ROUND(latitude * 10000000), ROUND(longitude * 10000000),

lib/map_boundary.rb

@@ -12,20 +12,20 @@ module MapBoundary
   def check_boundaries(min_lon, min_lat, max_lon, max_lat)
     # check the bbox is sane
     unless min_lon <= max_lon
-      raise("The minimum longitude must be less than the maximum longitude, but it wasn't")
+      raise OSM::APIBadBoundingBox.new("The minimum longitude must be less than the maximum longitude, but it wasn't")
     end
     unless min_lat <= max_lat
-      raise("The minimum latitude must be less than the maximum latitude, but it wasn't")
+      raise OSM::APIBadBoundingBox.new("The minimum latitude must be less than the maximum latitude, but it wasn't")
     end
     unless min_lon >= -180 && min_lat >= -90 && max_lon <= 180 && max_lat <= 90
       # Due to sanitize_boundaries, it is highly unlikely we'll actually get here
-      raise("The latitudes must be between -90 and 90, and longitudes between -180 and 180")
+      raise OSM::APIBadBoundingBox.new("The latitudes must be between -90 and 90, and longitudes between -180 and 180")
     end
 
     # check the bbox isn't too large
     requested_area = (max_lat-min_lat)*(max_lon-min_lon)
     if requested_area > APP_CONFIG['max_request_area']
-      raise("The maximum bbox size is " + APP_CONFIG['max_request_area'].to_s + 
+      raise OSM::APIBadBoundingBox.new("The maximum bbox size is " + APP_CONFIG['max_request_area'].to_s + 
         ", and your request was too large. Either request a smaller area, or use planet.osm")
     end
   end

lib/osm.rb

@@ -218,6 +218,22 @@ module OSM
     end
   end
 
+  ##
+  # raised when bounding box is invalid
+  class APIBadBoundingBox < APIError
+    def initialize(message)
+      @message = message
+    end
+
+    def status
+      :bad_request
+    end
+
+    def to_s
+      @message
+    end
+  end
+
   ##
   # raised when an API call is made using a method not supported on that URI
   class APIBadMethodError < APIError

public/javascripts/site.js

@@ -45,7 +45,7 @@ function updatelinks(lon,lat,zoom,layers,minlon,minlat,maxlon,maxlat) {
 
   node = document.getElementById("editanchor");
   if (node) {
-    if (zoom >= 11) {
+    if (zoom >= 13) {
       var args = new Object();
       args.lat = lat;
       args.lon = lon;

public/stylesheets/site.css

@@ -83,6 +83,11 @@ body {
   font-size: 14px;
 }
 
+#sotm {
+  width: 170px;
+  padding: 0px;
+}
+
 .notice {
   width: 150px;
   margin: 10px;
@@ -194,6 +199,18 @@ body {
   width: 100%;
 }
 
+#changeset_list .date {
+  white-space: nowrap;
+}
+
+#changeset_list .user {
+  white-space: nowrap;
+}
+
+#changeset_list .area {
+  white-space: nowrap;
+}
+
 #changeset_list.th {
   font-weight: bold;
 }

test/functional/relation_controller_test.rb

@@ -529,13 +529,20 @@ class RelationControllerTest < ActionController::TestCase
   # add a member to a relation and check the bounding box is only that
   # element.
   def test_add_member_bounding_box
-    check_changeset_modify([4,4,4,4]) do |changeset_id|
-      # add node 4 (4,4) to an existing relation
-      relation_xml = current_relations(:visible_relation).to_xml
+    relation_id = current_relations(:visible_relation).id
+
+    [current_nodes(:used_node_1),
+     current_nodes(:used_node_2),
+     current_ways(:used_way),
+     current_ways(:way_with_versions)
+    ].each_with_index do |element, version|
+      bbox = element.bbox.collect { |x| x / SCALE }
+      check_changeset_modify(bbox) do |changeset_id|
+        relation_xml = Relation.find(relation_id).to_xml
         relation_element = relation_xml.find("//osm/relation").first
         new_member = XML::Node.new("member")
-      new_member['ref'] = current_nodes(:used_node_2).id.to_s
-      new_member['type'] = "node"
+        new_member['ref'] = element.id.to_s
+        new_member['type'] = element.class.to_s.downcase
         new_member['role'] = "some_role"
         relation_element << new_member
       
@@ -545,7 +552,13 @@ class RelationControllerTest < ActionController::TestCase
         # upload the change
         content relation_xml
         put :update, :id => current_relations(:visible_relation).id
-      assert_response :success, "can't update relation for add node/bbox test"
+        assert_response :success, "can't update relation for add #{element.class}/bbox test: #{@response.body}"
+
+        # get it back and check the ordering 
+        get :read, :id => relation_id
+        assert_response :success, "can't read back the relation: #{@response.body}"
+        check_ordering(relation_xml, @response.body)
+      end
     end
   end
   
@@ -617,14 +630,18 @@ OSM
     get :read, :id => relation_id
     assert_response :success, "can't read back the relation: #{@response.body}"
     check_ordering(doc, @response.body)
+
+    # check the ordering in the history tables:
+    with_controller(OldRelationController.new) do
+      get :version, :id => relation_id, :version => 2
+      assert_response :success, "can't read back version 2 of the relation #{relation_id}"
+      check_ordering(doc, @response.body)
+    end
   end
 
   ## 
   # check that relations can contain duplicate members
   def test_relation_member_duplicates
-    ## First try with the private user
-    basic_authorization(users(:normal_user).email, "test");  
-
     doc_str = <<OSM
 <osm>
  <relation changeset='4'>
@@ -637,26 +654,16 @@ OSM
 OSM
     doc = XML::Parser.string(doc_str).parse
 
+    ## First try with the private user
+    basic_authorization(users(:normal_user).email, "test");  
+
     content doc
     put :create
     assert_response :forbidden
 
-
     ## Now try with the public user
     basic_authorization(users(:public_user).email, "test");  
 
-    doc_str = <<OSM
-<osm>
- <relation changeset='4'>
-  <member ref='1' type='node' role='forward'/>
-  <member ref='3' type='node' role='forward'/>
-  <member ref='1' type='node' role='forward'/>
-  <member ref='3' type='node' role='forward'/>
- </relation>
-</osm>
-OSM
-    doc = XML::Parser.string(doc_str).parse
-
     content doc
     put :create
     assert_response :success, "can't create a relation: #{@response.body}"
@@ -664,8 +671,42 @@ OSM
 
     # get it back and check the ordering
     get :read, :id => relation_id
+    assert_response :success, "can't read back the relation: #{relation_id}"
+    check_ordering(doc, @response.body)
+  end
+
+  ##
+  # test that the ordering of elements in the history is the same as in current.
+  def test_history_ordering
+    doc_str = <<OSM
+<osm>
+ <relation changeset='4'>
+  <member ref='1' type='node' role='forward'/>
+  <member ref='5' type='node' role='forward'/>
+  <member ref='4' type='node' role='forward'/>
+  <member ref='3' type='node' role='forward'/>
+ </relation>
+</osm>
+OSM
+    doc = XML::Parser.string(doc_str).parse
+    basic_authorization(users(:public_user).email, "test");  
+
+    content doc
+    put :create
+    assert_response :success, "can't create a relation: #{@response.body}"
+    relation_id = @response.body.to_i
+
+    # check the ordering in the current tables:
+    get :read, :id => relation_id
     assert_response :success, "can't read back the relation: #{@response.body}"
     check_ordering(doc, @response.body)
+
+    # check the ordering in the history tables:
+    with_controller(OldRelationController.new) do
+      get :version, :id => relation_id, :version => 1
+      assert_response :success, "can't read back version 1 of the relation: #{@response.body}"
+      check_ordering(doc, @response.body)
+    end
   end
 
   # ============================================================
@@ -726,12 +767,12 @@ OSM
     with_controller(ChangesetController.new) do
       get :read, :id => changeset_id
       assert_response :success, "can't re-read changeset for modify test"
-      assert_select "osm>changeset", 1
-      assert_select "osm>changeset[id=#{changeset_id}]", 1
-      assert_select "osm>changeset[min_lon=#{bbox[0].to_f}]", 1
-      assert_select "osm>changeset[min_lat=#{bbox[1].to_f}]", 1
-      assert_select "osm>changeset[max_lon=#{bbox[2].to_f}]", 1
-      assert_select "osm>changeset[max_lat=#{bbox[3].to_f}]", 1
+      assert_select "osm>changeset", 1, "Changeset element doesn't exist in #{@response.body}"
+      assert_select "osm>changeset[id=#{changeset_id}]", 1, "Changeset id=#{changeset_id} doesn't exist in #{@response.body}"
+      assert_select "osm>changeset[min_lon=#{bbox[0].to_f}]", 1, "Changeset min_lon wrong in #{@response.body}"
+      assert_select "osm>changeset[min_lat=#{bbox[1].to_f}]", 1, "Changeset min_lat wrong in #{@response.body}"
+      assert_select "osm>changeset[max_lon=#{bbox[2].to_f}]", 1, "Changeset max_lon wrong in #{@response.body}"
+      assert_select "osm>changeset[max_lat=#{bbox[3].to_f}]", 1, "Changeset max_lat wrong in #{@response.body}"
     end
   end