From: Tom Hughes Date: Sun, 8 Mar 2009 13:02:37 +0000 (+0000) Subject: Merge 12304:14009 from trunk. X-Git-Tag: live~7604^2~61 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/c8ee1351049ef1bb4d7b50d071b2a96154266d1d Merge 12304:14009 from trunk. --- c8ee1351049ef1bb4d7b50d071b2a96154266d1d diff --cc app/controllers/user_preference_controller.rb index 3b56c257b,3a48ee65e..68ea88eea --- a/app/controllers/user_preference_controller.rb +++ b/app/controllers/user_preference_controller.rb @@@ -52,43 -52,48 +52,42 @@@ class UserPreferenceController < Applic # update the entire set of preferences def update - p = XML::Parser.new - p.string = request.raw_post - begin - p = XML::Parser.string(request.raw_post) - doc = p.parse - - prefs = [] - - keyhash = {} ++ p = XML::Parser.string(request.raw_post) + doc = p.parse - doc.find('//preferences/preference').each do |pt| - pref = UserPreference.new + prefs = [] - unless keyhash[pt['k']].nil? # already have that key - render :text => 'OH NOES! CAN HAS UNIQUE KEYS?', :status => :not_acceptable - return - end + keyhash = {} - keyhash[pt['k']] = 1 - - pref.k = pt['k'] - pref.v = pt['v'] - pref.user_id = @user.id - prefs << pref - end + doc.find('//preferences/preference').each do |pt| + pref = UserPreference.new - if prefs.size > 150 - render :text => 'Too many preferences', :status => :request_entity_too_large - return + unless keyhash[pt['k']].nil? # already have that key + render :text => 'OH NOES! CAN HAS UNIQUE KEYS?', :status => :not_acceptable end - # kill the existing ones - UserPreference.delete_all(['user_id = ?', @user.id]) + keyhash[pt['k']] = 1 - # save the new ones - prefs.each do |pref| - pref.save! - end + pref.k = pt['k'] + pref.v = pt['v'] + pref.user_id = @user.id + prefs << pref + end - rescue Exception => ex - render :text => 'OH NOES! FAIL!: ' + ex.to_s, :status => :internal_server_error - return + if prefs.size > 150 + render :text => 'Too many preferences', :status => :request_entity_too_large end + # kill the existing ones + UserPreference.delete_all(['user_id = ?', @user.id]) + + # save the new ones + prefs.each do |pref| + pref.save! + end render :nothing => true + + rescue Exception => ex + render :text => 'OH NOES! FAIL!: ' + ex.to_s, :status => :internal_server_error end end diff --cc app/models/changeset.rb index 31d927e9a,000000000..d420f537a mode 100644,000000..100644 --- a/app/models/changeset.rb +++ b/app/models/changeset.rb @@@ -1,238 -1,0 +1,237 @@@ +class Changeset < ActiveRecord::Base + require 'xml/libxml' + + belongs_to :user + + has_many :changeset_tags, :foreign_key => 'id' + + has_many :nodes + has_many :ways + has_many :relations + has_many :old_nodes + has_many :old_ways + has_many :old_relations + + validates_presence_of :id, :on => :update + validates_presence_of :user_id, :created_at, :closed_at, :num_changes + validates_uniqueness_of :id + validates_numericality_of :id, :on => :update, :integer_only => true + validates_numericality_of :min_lat, :max_lat, :min_lon, :max_lat, :allow_nil => true, :integer_only => true + validates_numericality_of :user_id, :integer_only => true + validates_numericality_of :num_changes, :integer_only => true, :greater_than_or_equal_to => 0 + validates_associated :user + + # over-expansion factor to use when updating the bounding box + EXPAND = 0.1 + + # maximum number of elements allowed in a changeset + MAX_ELEMENTS = 50000 + + # maximum time a changeset is allowed to be open for. + MAX_TIME_OPEN = 1.day + + # idle timeout increment, one hour seems reasonable. + IDLE_TIMEOUT = 1.hour + + # Use a method like this, so that we can easily change how we + # determine whether a changeset is open, without breaking code in at + # least 6 controllers + def is_open? + # a changeset is open (that is, it will accept further changes) when + # it has not yet run out of time and its capacity is small enough. + # note that this may not be a hard limit - due to timing changes and + # concurrency it is possible that some changesets may be slightly + # longer than strictly allowed or have slightly more changes in them. + return ((closed_at > Time.now) and (num_changes <= MAX_ELEMENTS)) + end + + def set_closed_time_now + if is_open? + self.closed_at = Time.now + end + end + + def self.from_xml(xml, create=false) + begin - p = XML::Parser.new - p.string = xml ++ p = XML::Parser.string(xml) + doc = p.parse + + cs = Changeset.new + + doc.find('//osm/changeset').each do |pt| + if create + cs.created_at = Time.now + # initial close time is 1h ahead, but will be increased on each + # modification. + cs.closed_at = Time.now + IDLE_TIMEOUT + # initially we have no changes in a changeset + cs.num_changes = 0 + end + + pt.find('tag').each do |tag| + cs.add_tag_keyval(tag['k'], tag['v']) + end + end + rescue Exception => ex + cs = nil + end + + return cs + end + + ## + # returns the bounding box of the changeset. it is possible that some + # or all of the values will be nil, indicating that they are undefined. + def bbox + @bbox ||= [ min_lon, min_lat, max_lon, max_lat ] + end + + ## + # expand the bounding box to include the given bounding box. also, + # expand a little bit more in the direction of the expansion, so that + # further expansions may be unnecessary. this is an optimisation + # suggested on the wiki page by kleptog. + def update_bbox!(array) + # ensure that bbox is cached and has no nils in it. if there are any + # nils, just use the bounding box update to write over them. + @bbox = bbox.zip(array).collect { |a, b| a.nil? ? b : a } + + # FIXME - this looks nasty and violates DRY... is there any prettier + # way to do this? + @bbox[0] = array[0] + EXPAND * (@bbox[0] - @bbox[2]) if array[0] < @bbox[0] + @bbox[1] = array[1] + EXPAND * (@bbox[1] - @bbox[3]) if array[1] < @bbox[1] + @bbox[2] = array[2] + EXPAND * (@bbox[2] - @bbox[0]) if array[2] > @bbox[2] + @bbox[3] = array[3] + EXPAND * (@bbox[3] - @bbox[1]) if array[3] > @bbox[3] + + # update active record. rails 2.1's dirty handling should take care of + # whether this object needs saving or not. + self.min_lon, self.min_lat, self.max_lon, self.max_lat = @bbox + end + + ## + # the number of elements is also passed in so that we can ensure that + # a single changeset doesn't contain too many elements. this, of course, + # destroys the optimisation described in the bbox method above. + def add_changes!(elements) + self.num_changes += elements + end + + def tags_as_hash + return tags + end + + def tags + unless @tags + @tags = {} + self.changeset_tags.each do |tag| + @tags[tag.k] = tag.v + end + end + @tags + end + + def tags=(t) + @tags = t + end + + def add_tag_keyval(k, v) + @tags = Hash.new unless @tags + @tags[k] = v + end + + def save_with_tags! + t = Time.now + + # do the changeset update and the changeset tags update in the + # same transaction to ensure consistency. + Changeset.transaction do + # set the auto-close time to be one hour in the future unless + # that would make it more than 24h long, in which case clip to + # 24h, as this has been decided is a reasonable time limit. + if (closed_at - created_at) > (MAX_TIME_OPEN - IDLE_TIMEOUT) + self.closed_at = created_at + MAX_TIME_OPEN + else + self.closed_at = Time.now + IDLE_TIMEOUT + end + self.save! + + tags = self.tags + ChangesetTag.delete_all(['id = ?', self.id]) + + tags.each do |k,v| + tag = ChangesetTag.new + tag.k = k + tag.v = v + tag.id = self.id + tag.save! + end + end + end + + def to_xml + doc = OSM::API.new.get_xml_doc + doc.root << to_xml_node() + return doc + end + + def to_xml_node(user_display_name_cache = nil) + el1 = XML::Node.new 'changeset' + el1['id'] = self.id.to_s + + user_display_name_cache = {} if user_display_name_cache.nil? + + if user_display_name_cache and user_display_name_cache.key?(self.user_id) + # use the cache if available + elsif self.user.data_public? + user_display_name_cache[self.user_id] = self.user.display_name + else + user_display_name_cache[self.user_id] = nil + end + + el1['user'] = user_display_name_cache[self.user_id] unless user_display_name_cache[self.user_id].nil? + el1['uid'] = self.user_id.to_s if self.user.data_public? + + self.tags.each do |k,v| + el2 = XML::Node.new('tag') + el2['k'] = k.to_s + el2['v'] = v.to_s + el1 << el2 + end + + el1['created_at'] = self.created_at.xmlschema + el1['closed_at'] = self.closed_at.xmlschema unless is_open? + el1['open'] = is_open?.to_s + + el1['min_lon'] = (bbox[0].to_f / GeoRecord::SCALE).to_s unless bbox[0].nil? + el1['min_lat'] = (bbox[1].to_f / GeoRecord::SCALE).to_s unless bbox[1].nil? + el1['max_lon'] = (bbox[2].to_f / GeoRecord::SCALE).to_s unless bbox[2].nil? + el1['max_lat'] = (bbox[3].to_f / GeoRecord::SCALE).to_s unless bbox[3].nil? + + # NOTE: changesets don't include the XML of the changes within them, + # they are just structures for tagging. to get the osmChange of a + # changeset, see the download method of the controller. + + return el1 + end + + ## + # update this instance from another instance given and the user who is + # doing the updating. note that this method is not for updating the + # bounding box, only the tags of the changeset. + def update_from(other, user) + # ensure that only the user who opened the changeset may modify it. + unless user.id == self.user_id + raise OSM::APIUserChangesetMismatchError + end + + # can't change a closed changeset + unless is_open? + raise OSM::APIChangesetAlreadyClosedError.new(self) + end + + # copy the other's tags + self.tags = other.tags + + save_with_tags! + end +end diff --cc app/models/node.rb index f2ad3a78a,af88a117d..d3e0a7e8d --- a/app/models/node.rb +++ b/app/models/node.rb @@@ -64,51 -57,43 +64,50 @@@ class Node < ActiveRecord::Bas # Read in xml as text and return it's Node object representation def self.from_xml(xml, create=false) begin - p = XML::Parser.new - p.string = xml + p = XML::Parser.string(xml) doc = p.parse - - node = Node.new doc.find('//osm/node').each do |pt| - node.lat = pt['lat'].to_f - node.lon = pt['lon'].to_f + return Node.from_xml_node(pt, create) + end + rescue LibXML::XML::Error => ex + raise OSM::APIBadXMLError.new("node", xml, ex.message) + end + end - return nil unless node.in_world? + def self.from_xml_node(pt, create=false) + node = Node.new + + raise OSM::APIBadXMLError.new("node", pt, "lat missing") if pt['lat'].nil? + raise OSM::APIBadXMLError.new("node", pt, "lon missing") if pt['lon'].nil? + node.lat = pt['lat'].to_f + node.lon = pt['lon'].to_f + raise OSM::APIBadXMLError.new("node", pt, "changeset id missing") if pt['changeset'].nil? + node.changeset_id = pt['changeset'].to_i - unless create - if pt['id'] != '0' - node.id = pt['id'].to_i - end - end + raise OSM::APIBadUserInput.new("The node is outside this world") unless node.in_world? - node.visible = pt['visible'] and pt['visible'] == 'true' + # version must be present unless creating + raise OSM::APIBadXMLError.new("node", pt, "Version is required when updating") unless create or not pt['version'].nil? + node.version = create ? 0 : pt['version'].to_i - if create - node.timestamp = Time.now - else - if pt['timestamp'] - node.timestamp = Time.parse(pt['timestamp']) - end - end + unless create + if pt['id'] != '0' + node.id = pt['id'].to_i + end + end - tags = [] + # visible if it says it is, or as the default if the attribute + # is missing. + # Don't need to set the visibility, when it is set explicitly in the create/update/delete + #node.visible = pt['visible'].nil? or pt['visible'] == 'true' - pt.find('tag').each do |tag| - tags << [tag['k'],tag['v']] - end + # We don't care about the time, as it is explicitly set on create/update/delete - node.tags = Tags.join(tags) - end - rescue - node = nil + tags = [] + + pt.find('tag').each do |tag| + node.add_tag_key_val(tag['k'],tag['v']) end return node diff --cc app/models/relation.rb index 6be106159,d9dba303f..64af4ecc1 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@@ -15,70 -13,39 +15,69 @@@ class Relation < ActiveRecord::Bas has_many :containing_relation_members, :class_name => "RelationMember", :as => :member has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation, :extend => ObjectFinder + validates_presence_of :id, :on => :update + validates_presence_of :timestamp,:version, :changeset_id + validates_uniqueness_of :id + validates_inclusion_of :visible, :in => [ true, false ] + validates_numericality_of :id, :on => :update, :integer_only => true + validates_numericality_of :changeset_id, :version, :integer_only => true + validates_associated :changeset + + TYPES = ["node", "way", "relation"] + def self.from_xml(xml, create=false) begin - p = XML::Parser.new - p.string = xml + p = XML::Parser.string(xml) doc = p.parse - relation = Relation.new - doc.find('//osm/relation').each do |pt| - if !create and pt['id'] != '0' - relation.id = pt['id'].to_i - end + return Relation.from_xml_node(pt, create) + end + rescue LibXML::XML::Error => ex + raise OSM::APIBadXMLError.new("relation", xml, ex.message) + end + end - if create - relation.timestamp = Time.now - relation.visible = true - else - if pt['timestamp'] - relation.timestamp = Time.parse(pt['timestamp']) - end - end + def self.from_xml_node(pt, create=false) + relation = Relation.new - pt.find('tag').each do |tag| - relation.add_tag_keyval(tag['k'], tag['v']) - end + if !create and pt['id'] != '0' + relation.id = pt['id'].to_i + end - pt.find('member').each do |member| - relation.add_member(member['type'], member['ref'], member['role']) - end + raise OSM::APIBadXMLError.new("relation", pt, "You are missing the required changeset in the relation") if pt['changeset'].nil? + relation.changeset_id = pt['changeset'] + + # The follow block does not need to be executed because they are dealt with + # in create_with_history, update_from and delete_with_history + if create + relation.timestamp = Time.now + relation.visible = true + relation.version = 0 + else + if pt['timestamp'] + relation.timestamp = Time.parse(pt['timestamp']) end - rescue - relation = nil + relation.version = pt['version'] + end + + pt.find('tag').each do |tag| + relation.add_tag_keyval(tag['k'], tag['v']) end + pt.find('member').each do |member| + #member_type = + logger.debug "each member" + raise OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member['type'] + logger.debug "after raise" + #member_ref = member['ref'] + #member_role + member['role'] ||= "" # Allow the upload to not include this, in which case we default to an empty string. + logger.debug member['role'] + relation.add_member(member['type'], member['ref'], member['role']) + end + raise OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil? + return relation end diff --cc app/models/way.rb index 86b25e08e,6c3ea9e46..dbc1197a9 --- a/app/models/way.rb +++ b/app/models/way.rb @@@ -17,57 -15,37 +17,56 @@@ class Way < ActiveRecord::Bas has_many :containing_relation_members, :class_name => "RelationMember", :as => :member has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation, :extend => ObjectFinder + validates_presence_of :id, :on => :update + validates_presence_of :changeset_id,:version, :timestamp + validates_uniqueness_of :id + validates_inclusion_of :visible, :in => [ true, false ] + validates_numericality_of :changeset_id, :version, :integer_only => true + validates_numericality_of :id, :on => :update, :integer_only => true + validates_associated :changeset + def self.from_xml(xml, create=false) begin - p = XML::Parser.new - p.string = xml + p = XML::Parser.string(xml) doc = p.parse - way = Way.new - doc.find('//osm/way').each do |pt| - if !create and pt['id'] != '0' - way.id = pt['id'].to_i - end - - if create - way.timestamp = Time.now - way.visible = true - else - if pt['timestamp'] - way.timestamp = Time.parse(pt['timestamp']) - end - end + return Way.from_xml_node(pt, create) + end + rescue LibXML::XML::Error => ex + raise OSM::APIBadXMLError.new("way", xml, ex.message) + end + end - pt.find('tag').each do |tag| - way.add_tag_keyval(tag['k'], tag['v']) - end + def self.from_xml_node(pt, create=false) + way = Way.new - pt.find('nd').each do |nd| - way.add_nd_num(nd['ref']) - end + if !create and pt['id'] != '0' + way.id = pt['id'].to_i + end + + way.version = pt['version'] + raise OSM::APIBadXMLError.new("node", pt, "Changeset is required") if pt['changeset'].nil? + way.changeset_id = pt['changeset'] + + # This next section isn't required for the create, update, or delete of ways + if create + way.timestamp = Time.now + way.visible = true + else + if pt['timestamp'] + way.timestamp = Time.parse(pt['timestamp']) end - rescue - way = nil + # if visible isn't present then it defaults to true + way.visible = (pt['visible'] or true) + end + + pt.find('tag').each do |tag| + way.add_tag_keyval(tag['k'], tag['v']) + end + + pt.find('nd').each do |nd| + way.add_nd_num(nd['ref']) end return way diff --cc app/views/diary_entry/list.rhtml index 9852313bb,3d1f0bbcb..38157b183 --- a/app/views/diary_entry/list.rhtml +++ b/app/views/diary_entry/list.rhtml @@@ -15,29 -16,20 +15,25 @@@ <% end %> <% end %> -

Recent diary entries:

-<%= render :partial => 'diary_entry', :collection => @entries %> +<% if @entries.empty? %> -

No diary entries

- ++

No diary entries

+<% else %> ++

Recent diary entries:

+ -<%= link_to "Older Entries", { :page => @entry_pages.current.next } if @entry_pages.current.next %> -<% if @entry_pages.current.next and @entry_pages.current.previous %> -| -<% end %> -<%= link_to "Newer Entries", { :page => @entry_pages.current.previous } if @entry_pages.current.previous %> ++
+ -
++ <%= render :partial => 'diary_entry', :collection => @entries %> + -

Recent diary entries:

-
- <%= render :partial => 'diary_entry', :collection => @entries %> - - <%= link_to "Older Entries", { :page => @entry_pages.current.next } if @entry_pages.current.next %> - <% if @entry_pages.current.next and @entry_pages.current.previous %> - | - <% end %> - <%= link_to "Newer Entries", { :page => @entry_pages.current.previous } if @entry_pages.current.previous %> - -
- ++ <%= link_to "Older Entries", { :page => @entry_pages.current.next } if @entry_pages.current.next %> ++ <% if @entry_pages.current.next and @entry_pages.current.previous %>|<% end %> ++ <%= link_to "Newer Entries", { :page => @entry_pages.current.previous } if @entry_pages.current.previous %> ++ ++
+<% end %> <%= rss_link_to :action => 'rss' %> - <%= auto_discovery_link_tag :atom, :action => 'rss' %> - -
-
+ <% content_for :head do %> + <%= auto_discovery_link_tag :atom, :action => 'rss' %> + <% end %> diff --cc app/views/layouts/site.rhtml index cf7ad9fc8,34d2ecb30..cc8f7192d --- a/app/views/layouts/site.rhtml +++ b/app/views/layouts/site.rhtml @@@ -7,7 -7,7 +7,8 @@@ <%= stylesheet_link_tag 'site' %> <%= stylesheet_link_tag 'print', :media => "print" %> <%= tag("link", { :rel => "search", :type => "application/opensearchdescription+xml", :title => "OpenStreetMap Search", :href => "/opensearch/osm.xml" }) %> + <%= tag("meta", { :name => "description", :content => "OpenStreetMap is the free wiki world map." }) %> + <%= yield :head %> OpenStreetMap<%= ' | '+ h(@title) if @title %> @@@ -97,21 -113,16 +114,16 @@@ <%= yield :optionals %> - +
+
+ Make a Donation +
+ +
diff --cc app/views/user/login.rhtml index 770ad8873,ff988f070..3c40906ca --- a/app/views/user/login.rhtml +++ b/app/views/user/login.rhtml @@@ -1,15 -1,13 +1,13 @@@ --

Login:


--Please login or <%= link_to 'create an account', :controller => 'user', :action => 'new' %>.
++

Login

++ ++

Please login or <%= link_to 'create an account', :controller => 'user', :action => 'new' %>.

<% form_tag :action => 'login' do %> <%= hidden_field_tag('referer', h(params[:referer])) %> -
- - - ++ + + +
Email Address or username:<%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %>
Email Address or username:<%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %>
Password:<%= password_field('user', 'password',{:size => 50, :maxlength => 255}) %>
Email Address or Username:<%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %>
Password:<%= password_field('user', 'password',{:size => 28, :maxlength => 255}) %> (<%= link_to 'Lost your password?', :controller => 'user', :action => 'lost_password' %>)
 
<%= submit_tag 'Login' %>
-- --
-<%= submit_tag 'Login' %> -<% end %> (<%= link_to 'Lost your password?', :controller => 'user', :action => 'lost_password' %>) +<% end %> diff --cc app/views/user/new.rhtml index 1b7f6e9b4,d0b5a9667..823dccf52 --- a/app/views/user/new.rhtml +++ b/app/views/user/new.rhtml @@@ -1,28 -1,43 +1,45 @@@ -

Create a user account


- Fill in the form and we'll send you a quick email to activate your account. -

-

Create a user account

++

Create a User Account

- By creating an account, you agree that all work uploaded to openstreetmap.org and all data created by use of any tools which connect to openstreetmap.org is to be (non-exclusively) licensed under this Creative Commons license (by-sa).

+ <% if Acl.find_by_address(request.remote_ip, :conditions => {:k => "no_account_creation"}) %> + +

Unfortunately we are not currently able to create an account for + you automatically. +

+ +

Please contact the webmaster + to arrange for an account to be created - we will try and deal with + the request as quickly as possible. +

+ + <% else %> + +

Fill in the form and we'll send you a quick email to activate your + account. +

+ +

By creating an account, you agree that all work uploaded to + openstreetmap.org and all data created by use of any tools which + connect to openstreetmap.org is to be (non-exclusively) licensed under + this Creative + Commons license (by-sa). +

<%= error_messages_for 'user' %> <% form_tag :action => 'save' do %> - - - - - - +
Email Address<%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %>
Confirm Email Address<%= text_field('user', 'email_confirmation',{:size => 50, :maxlength => 255}) %>
Display Name<%= text_field('user', 'display_name',{:size => 50, :maxlength => 255}) %>
Password<%= password_field('user', 'pass_crypt',{:size => 50, :maxlength => 255}) %>
Confirm Password<%= password_field('user', 'pass_crypt_confirmation',{:size => 50, :maxlength => 255}) %>
+ + + + + + + + + + +
Email Address : <%= text_field('user', 'email',{:size => 50, :maxlength => 255}) %>
Confirm Email Address : <%= text_field('user', 'email_confirmation',{:size => 50, :maxlength => 255}) %>
Not displayed publicly (see privacy policy)
 
Display Name : <%= text_field('user', 'display_name',{:size => 30, :maxlength => 255}) %>
 
Password : <%= password_field('user', 'pass_crypt',{:size => 30, :maxlength => 255}) %>
Confirm Password : <%= password_field('user', 'pass_crypt_confirmation',{:size => 30, :maxlength => 255}) %>
 
--
--
- - - + <% end %> + <% end %> diff --cc config/environment.rb index ffed548bd,e6af619eb..b3481c024 --- a/config/environment.rb +++ b/config/environment.rb @@@ -40,16 -37,6 +40,16 @@@ Rails::Initializer.run do |config config.frameworks -= [ :active_record ] end + # Specify gems that this application depends on. + # They can then be installed with "rake gems:install" on new installations. + # config.gem "bj" + # config.gem "hpricot", :version => '0.6', :source => "http://code.whytheluckystiff.net" + # config.gem "aws-s3", :lib => "aws/s3" + config.gem 'composite_primary_keys', :version => '1.1.0' - config.gem 'libxml-ruby', :version => '0.9.4', :lib => 'libxml' ++ config.gem 'libxml-ruby', :version => '>= 1.0.0', :lib => 'libxml' + config.gem 'rmagick', :lib => 'RMagick' + config.gem 'mysql' + # Only load the plugins named here, in the order given. By default, all plugins # in vendor/plugins are loaded in alphabetical order. # :all can be used as a placeholder for all plugins not explicitly named diff --cc config/initializers/libxml.rb index ae636a9a3,f783cda1e..07f79660f --- a/config/initializers/libxml.rb +++ b/config/initializers/libxml.rb @@@ -1,9 -1,9 +1,5 @@@ -require 'rubygems' -gem 'libxml-ruby', '>= 1.0.0' -require 'libxml' - # This is required otherwise libxml writes out memory errors to - # the standard output and exits uncleanly - # Changed method due to deprecation of the old register_error_handler - # http://libxml.rubyforge.org/rdoc/classes/LibXML/XML/Parser.html#M000076 - # So set_handler is used instead - # http://libxml.rubyforge.org/rdoc/classes/LibXML/XML/Error.html#M000334 + # the standard output and exits uncleanly LibXML::XML::Error.set_handler do |message| raise message end diff --cc db/migrate/019_add_timestamp_indexes.rb index c6b3bc7c2,000000000..c6b3bc7c2 mode 100644,000000..100644 --- a/db/migrate/019_add_timestamp_indexes.rb +++ b/db/migrate/019_add_timestamp_indexes.rb diff --cc db/migrate/020_populate_node_tags_and_remove.rb index 860358646,000000000..dc048f190 mode 100644,000000..100644 --- a/db/migrate/020_populate_node_tags_and_remove.rb +++ b/db/migrate/020_populate_node_tags_and_remove.rb @@@ -1,60 -1,0 +1,60 @@@ +class PopulateNodeTagsAndRemove < ActiveRecord::Migration + def self.up + have_nodes = select_value("SELECT count(*) FROM current_nodes").to_i != 0 + + if have_nodes - prefix = File.join Dir.tmpdir, "019_populate_node_tags_and_remove.#{$$}." ++ prefix = File.join Dir.tmpdir, "020_populate_node_tags_and_remove.#{$$}." + - cmd = "db/migrate/019_populate_node_tags_and_remove_helper" ++ cmd = "db/migrate/020_populate_node_tags_and_remove_helper" + src = "#{cmd}.c" + if not File.exists? cmd or File.mtime(cmd) < File.mtime(src) then + system 'cc -O3 -Wall `mysql_config --cflags --libs` ' + + "#{src} -o #{cmd}" or fail + end + + conn_opts = ActiveRecord::Base.connection.instance_eval { @connection_options } + args = conn_opts.map { |arg| arg.to_s } + [prefix] + fail "#{cmd} failed" unless system cmd, *args + + tempfiles = ['nodes', 'node_tags', 'current_nodes', 'current_node_tags']. + map { |base| prefix + base } + nodes, node_tags, current_nodes, current_node_tags = tempfiles + end + + execute "TRUNCATE nodes" + remove_column :nodes, :tags + remove_column :current_nodes, :tags + + add_column :nodes, :version, :bigint, :limit => 20, :null => false + + create_table :current_node_tags, innodb_table do |t| + t.column :id, :bigint, :limit => 64, :null => false + t.column :k, :string, :default => "", :null => false + t.column :v, :string, :default => "", :null => false + end + + create_table :node_tags, innodb_table do |t| + t.column :id, :bigint, :limit => 64, :null => false + t.column :version, :bigint, :limit => 20, :null => false + t.column :k, :string, :default => "", :null => false + t.column :v, :string, :default => "", :null => false + end + + # now get the data back + csvopts = "FIELDS TERMINATED BY ',' ENCLOSED BY '\"' ESCAPED BY '\"' LINES TERMINATED BY '\\n'" + + if have_nodes + execute "LOAD DATA INFILE '#{nodes}' INTO TABLE nodes #{csvopts} (id, latitude, longitude, user_id, visible, timestamp, tile, version)"; + execute "LOAD DATA INFILE '#{node_tags}' INTO TABLE node_tags #{csvopts} (id, version, k, v)" + execute "LOAD DATA INFILE '#{current_node_tags}' INTO TABLE current_node_tags #{csvopts} (id, k, v)" + end + + tempfiles.each { |fn| File.unlink fn } if have_nodes + end + + def self.down + raise IrreversibleMigration.new +# add_column :nodes, "tags", :text, :default => "", :null => false +# add_column :current_nodes, "tags", :text, :default => "", :null => false + end +end diff --cc db/migrate/020_populate_node_tags_and_remove_helper.c index c41ea33da,000000000..c41ea33da mode 100644,000000..100644 --- a/db/migrate/020_populate_node_tags_and_remove_helper.c +++ b/db/migrate/020_populate_node_tags_and_remove_helper.c diff --cc db/migrate/021_move_to_innodb.rb index da0488ca5,000000000..da0488ca5 mode 100644,000000..100644 --- a/db/migrate/021_move_to_innodb.rb +++ b/db/migrate/021_move_to_innodb.rb diff --cc db/migrate/022_key_constraints.rb index 40f98be02,000000000..40f98be02 mode 100644,000000..100644 --- a/db/migrate/022_key_constraints.rb +++ b/db/migrate/022_key_constraints.rb diff --cc db/migrate/023_add_changesets.rb index e0cf3904a,000000000..e0cf3904a mode 100644,000000..100644 --- a/db/migrate/023_add_changesets.rb +++ b/db/migrate/023_add_changesets.rb diff --cc db/migrate/024_order_relation_members.rb index 5500edfcf,000000000..5500edfcf mode 100644,000000..100644 --- a/db/migrate/024_order_relation_members.rb +++ b/db/migrate/024_order_relation_members.rb diff --cc db/migrate/025_add_end_time_to_changesets.rb index b87ce3fde,000000000..b87ce3fde mode 100644,000000..100644 --- a/db/migrate/025_add_end_time_to_changesets.rb +++ b/db/migrate/025_add_end_time_to_changesets.rb diff --cc lib/diff_reader.rb index 37de8ea59,000000000..217e9309d mode 100644,000000..100644 --- a/lib/diff_reader.rb +++ b/lib/diff_reader.rb @@@ -1,223 -1,0 +1,224 @@@ +## +# DiffReader reads OSM diffs and applies them to the database. +# +# Uses the streaming LibXML "Reader" interface to cut down on memory +# usage, so hopefully we can process fairly large diffs. +class DiffReader + include ConsistencyValidations + + # maps each element type to the model class which handles it + MODELS = { + "node" => Node, + "way" => Way, + "relation" => Relation + } + + ## + # Construct a diff reader by giving it a bunch of XML +data+ to parse + # in OsmChange format. All diffs must be limited to a single changeset + # given in +changeset+. + def initialize(data, changeset) - @reader = XML::Reader.new data ++ @reader = XML::Reader.string(data) + @changeset = changeset + end + + ## + # Reads the next element from the XML document. Checks the return value + # and throws an exception if an error occurred. + def read_or_die - # NOTE: XML::Reader#read returns 0 for EOF and -1 for error. - # we allow an EOF because we are expecting this to always happen - # at the end of a document. - if @reader.read < 0 - raise APIBadUserInput.new("Unexpected end of XML document.") ++ # NOTE: XML::Reader#read returns false for EOF and raises an ++ # exception if an error occurs. ++ begin ++ @reader.read ++ rescue LibXML::XML::Error => ex ++ raise OSM::APIBadXMLError.new("changeset", xml, ex.message) + end + end + + ## + # An element-block mapping for using the LibXML reader interface. + # + # Since a lot of LibXML reader usage is boilerplate iteration through + # elements, it would be better to DRY and do this in a block. This + # could also help with error handling...? + def with_element + # if the start element is empty then don't do any processing, as + # there won't be any child elements to process! + unless @reader.empty_element? + # read the first element + read_or_die + + while @reader.node_type != 15 do # end element + # because we read elements in DOM-style to reuse their DOM + # parsing code, we don't always read an element on each pass + # as the call to @reader.next in the innermost loop will take + # care of that for us. + if @reader.node_type == 1 # element + yield @reader.name + else + read_or_die + end + end + end + read_or_die + end + + ## + # An element-block mapping for using the LibXML reader interface. + # + # Since a lot of LibXML reader usage is boilerplate iteration through + # elements, it would be better to DRY and do this in a block. This + # could also help with error handling...? + def with_model + with_element do |model_name| + model = MODELS[model_name] + raise "Unexpected element type #{model_name}, " + + "expected node, way, relation." if model.nil? + yield model, @reader.expand + @reader.next + end + end + + ## + # Checks a few invariants. Others are checked in the model methods + # such as save_ and delete_with_history. + def check(model, xml, new) + raise OSM::APIBadXMLError.new(model, xml) if new.nil? + unless new.changeset_id == @changeset.id + raise OSM::APIChangesetMismatchError.new(new.changeset_id, @changeset.id) + end + end + + ## + # Consume the XML diff and try to commit it to the database. This code + # is *not* transactional, so code which calls it should ensure that the + # appropriate transaction block is in place. + # + # On a failure to meet preconditions (e.g: optimistic locking fails) + # an exception subclassing OSM::APIError will be thrown. + def commit + + # data structure used for mapping placeholder IDs to real IDs + node_ids, way_ids, rel_ids = {}, {}, {} + ids = { :node => node_ids, :way => way_ids, :relation => rel_ids} + + # take the first element and check that it is an osmChange element + @reader.read + raise APIBadUserInput.new("Document element should be 'osmChange'.") if @reader.name != 'osmChange' + + result = OSM::API.new.get_xml_doc + result.root.name = "diffResult" + + # loop at the top level, within the element + with_element do |action_name| + if action_name == 'create' + # create a new element. this code is agnostic of the element type + # because all the elements support the methods that we're using. + with_model do |model, xml| + new = model.from_xml_node(xml, true) + check(model, xml, new) + + # when this element is saved it will get a new ID, so we save it + # to produce the mapping which is sent to other elements. + placeholder_id = xml['id'].to_i + raise OSM::APIBadXMLError.new(model, xml) if placeholder_id.nil? + + # check if the placeholder ID has been given before and throw + # an exception if it has - we can't create the same element twice. + model_sym = model.to_s.downcase.to_sym + raise OSM::APIBadUserInput.new("Placeholder IDs must be unique for created elements.") if ids[model_sym].include? placeholder_id + + # some elements may have placeholders for other elements in the + # diff, so we must fix these before saving the element. + new.fix_placeholders!(ids) + + # create element given user + new.create_with_history(@changeset.user) + + # save placeholder => allocated ID map + ids[model_sym][placeholder_id] = new.id + + # add the result to the document we're building for return. + xml_result = XML::Node.new model.to_s.downcase + xml_result["old_id"] = placeholder_id.to_s + xml_result["new_id"] = new.id.to_s + xml_result["new_version"] = new.version.to_s + result.root << xml_result + end + + elsif action_name == 'modify' + # modify an existing element. again, this code doesn't directly deal + # with types, but uses duck typing to handle them transparently. + with_model do |model, xml| + # get the new element from the XML payload + new = model.from_xml_node(xml, false) + check(model, xml, new) + + # if the ID is a placeholder then map it to the real ID + model_sym = model.to_s.downcase.to_sym + is_placeholder = ids[model_sym].include? new.id + id = is_placeholder ? ids[model_sym][new.id] : new.id + + # and the old one from the database + old = model.find(id) + + new.fix_placeholders!(ids) + old.update_from(new, @changeset.user) + + xml_result = XML::Node.new model.to_s.downcase + # oh, the irony... the "new" element actually contains the "old" ID + # a better name would have been client/server, but anyway... + xml_result["old_id"] = new.id.to_s + xml_result["new_id"] = id.to_s + # version is updated in "old" through the update, so we must not + # return new.version here but old.version! + xml_result["new_version"] = old.version.to_s + result.root << xml_result + end + + elsif action_name == 'delete' + # delete action. this takes a payload in API 0.6, so we need to do + # most of the same checks that are done for the modify. + with_model do |model, xml| + # delete doesn't have to contain a full payload, according to + # the wiki docs, so we just extract the things we need. + new_id = xml['id'].to_i + raise API::APIBadXMLError.new(model, xml, "ID attribute is required") if new_id.nil? + + # if the ID is a placeholder then map it to the real ID + model_sym = model.to_s.downcase.to_sym + is_placeholder = ids[model_sym].include? new_id + id = is_placeholder ? ids[model_sym][new_id] : new_id + + # build the "new" element by modifying the existing one + new = model.find(id) + new.changeset_id = xml['changeset'].to_i + new.version = xml['version'].to_i + check(model, xml, new) + + # fetch the matching old element from the DB + old = model.find(id) + + # can a delete have placeholders under any circumstances? + # if a way is modified, then deleted is that a valid diff? + new.fix_placeholders!(ids) + old.delete_with_history!(new, @changeset.user) + + xml_result = XML::Node.new model.to_s.downcase + # oh, the irony... the "new" element actually contains the "old" ID + # a better name would have been client/server, but anyway... + xml_result["old_id"] = new_id.to_s + result.root << xml_result + end + + else + # no other actions to choose from, so it must be the users fault! + raise OSM::APIChangesetActionInvalid.new(action_name) + end + end + + # return the XML document to be rendered back to the client + return result + end + +end diff --cc lib/osm.rb index f6646503d,365ddf68e..f372979a8 --- a/lib/osm.rb +++ b/lib/osm.rb @@@ -326,10 -187,10 +326,10 @@@ module OS class API def get_xml_doc doc = XML::Document.new - doc.encoding = 'UTF-8' + doc.encoding = XML::Encoding::UTF_8 root = XML::Node.new 'osm' root['version'] = API_VERSION - root['generator'] = 'OpenStreetMap server' + root['generator'] = GENERATOR doc.root = root return doc end diff --cc test/functional/node_controller_test.rb index bc9ffa489,a380eeb20..8d019bf79 --- a/test/functional/node_controller_test.rb +++ b/test/functional/node_controller_test.rb @@@ -257,27 -82,6 +257,26 @@@ class NodeControllerTest < ActionContro end def content(c) - @request.env["RAW_POST_DATA"] = c + @request.env["RAW_POST_DATA"] = c.to_s + 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 + return xml + end + + ## + # parse some xml + def xml_parse(xml) - parser = XML::Parser.new - parser.string = xml ++ parser = XML::Parser.string(xml) + parser.parse end end diff --cc test/functional/relation_controller_test.rb index 4ace316a4,202a015a8..f52981233 --- a/test/functional/relation_controller_test.rb +++ b/test/functional/relation_controller_test.rb @@@ -323,225 -205,4 +323,224 @@@ class RelationControllerTest < ActionCo assert_response :not_found end + ## + # when a relation's tag is modified then it should put the bounding + # box of all its members into the changeset. + def test_tag_modify_bounding_box + # in current fixtures, relation 5 contains nodes 3 and 5 (node 3 + # indirectly via way 3), so the bbox should be [3,3,5,5]. + check_changeset_modify([3,3,5,5]) do |changeset_id| + # add a tag to an existing relation + relation_xml = current_relations(:visible_relation).to_xml + relation_element = relation_xml.find("//osm/relation").first + new_tag = XML::Node.new("tag") + new_tag['k'] = "some_new_tag" + new_tag['v'] = "some_new_value" + relation_element << new_tag + + # update changeset ID to point to new changeset + update_changeset(relation_xml, changeset_id) + + # upload the change + content relation_xml + put :update, :id => current_relations(:visible_relation).id + assert_response :success, "can't update relation for tag/bbox test" + end + end + + ## + # 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_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['role'] = "some_role" + relation_element << new_member + + # update changeset ID to point to new changeset + update_changeset(relation_xml, changeset_id) + + # 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" + end + end + + ## + # remove a member from a relation and check the bounding box is + # only that element. + def test_remove_member_bounding_box + check_changeset_modify([5,5,5,5]) do |changeset_id| + # remove node 5 (5,5) from an existing relation + relation_xml = current_relations(:visible_relation).to_xml + relation_xml. + find("//osm/relation/member[@type='node'][@ref='5']"). + first.remove! + + # update changeset ID to point to new changeset + update_changeset(relation_xml, changeset_id) + + # upload the change + content relation_xml + put :update, :id => current_relations(:visible_relation).id + assert_response :success, "can't update relation for remove node/bbox test" + end + end + + ## + # check that relations are ordered + def test_relation_member_ordering + basic_authorization("test@openstreetmap.org", "test"); + + doc_str = < + + + + + + + +OSM + doc = XML::Parser.string(doc_str).parse + + content doc + put :create + assert_response :success, "can't create a relation: #{@response.body}" + relation_id = @response.body.to_i + + # 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(doc, @response.body) + + # insert a member at the front + new_member = XML::Node.new "member" + new_member['ref'] = 5.to_s + new_member['type'] = 'node' + new_member['role'] = 'new first' + doc.find("//osm/relation").first.child.prev = new_member + # update the version, should be 1? + doc.find("//osm/relation").first['id'] = relation_id.to_s + doc.find("//osm/relation").first['version'] = 1.to_s + + # upload the next version of the relation + content doc + put :update, :id => relation_id + assert_response :success, "can't update relation: #{@response.body}" + new_version = @response.body.to_i + + # get it back again and check the ordering again + get :read, :id => relation_id + assert_response :success, "can't read back the relation: #{@response.body}" + check_ordering(doc, @response.body) + end + + ## + # check that relations can contain duplicate members + def test_relation_member_duplicates + basic_authorization("test@openstreetmap.org", "test"); + + doc_str = < + + + + + + + +OSM + doc = XML::Parser.string(doc_str).parse + + content doc + put :create + assert_response :success, "can't create a relation: #{@response.body}" + relation_id = @response.body.to_i + + # 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(doc, @response.body) + end + + # ============================================================ + # utility functions + # ============================================================ + + ## + # checks that the XML document and the string arguments have + # members in the same order. + def check_ordering(doc, xml) + new_doc = XML::Parser.string(xml).parse + + doc_members = doc.find("//osm/relation/member").collect do |m| + [m['ref'].to_i, m['type'].to_sym, m['role']] + end + + new_members = new_doc.find("//osm/relation/member").collect do |m| + [m['ref'].to_i, m['type'].to_sym, m['role']] + end + + doc_members.zip(new_members).each do |d, n| + assert_equal d, n, "members are not equal - ordering is wrong? (#{doc}, #{xml})" + end + end + + ## + # create a changeset and yield to the caller to set it up, then assert + # that the changeset bounding box is +bbox+. + def check_changeset_modify(bbox) + basic_authorization("test@openstreetmap.org", "test"); + + # create a new changeset for this operation, so we are assured + # that the bounding box will be newly-generated. + changeset_id = with_controller(ChangesetController.new) do + content "" + put :create + assert_response :success, "couldn't create changeset for modify test" + @response.body.to_i + end + + # go back to the block to do the actual modifies + yield changeset_id + + # now download the changeset to check its bounding box + 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 + end + 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/relation").first[name] = value.to_s + return xml + end + + ## + # parse some xml + def xml_parse(xml) - parser = XML::Parser.new - parser.string = xml ++ parser = XML::Parser.string(xml) + parser.parse + end end