X-Git-Url: https://git.openstreetmap.org./rails.git/blobdiff_plain/1654b3d407feefbabd2da99b754dd28698ca6948..30bbca0f3b05834dc9fd899e75e01b1994c0333b:/app/models/relation.rb diff --git a/app/models/relation.rb b/app/models/relation.rb index 464d1646c..b66d957b9 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -87,7 +87,7 @@ class Relation < ActiveRecord::Base return doc end - def to_xml_node(user_display_name_cache = nil) + def to_xml_node(changeset_cache = {}, user_display_name_cache = {}) el1 = XML::Node.new 'relation' el1['id'] = self.id.to_s el1['visible'] = self.visible.to_s @@ -95,19 +95,25 @@ class Relation < ActiveRecord::Base el1['version'] = self.version.to_s el1['changeset'] = self.changeset_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.changeset.user_id) + if changeset_cache.key?(self.changeset_id) + # use the cache if available + else + changeset_cache[self.changeset_id] = self.changeset.user_id + end + + user_id = changeset_cache[self.changeset_id] + + if user_display_name_cache.key?(user_id) # use the cache if available elsif self.changeset.user.data_public? - user_display_name_cache[self.changeset.user_id] = self.changeset.user.display_name + user_display_name_cache[user_id] = self.changeset.user.display_name else - user_display_name_cache[self.changeset.user_id] = nil + user_display_name_cache[user_id] = nil end - if not user_display_name_cache[self.changeset.user_id].nil? - el1['user'] = user_display_name_cache[self.changeset.user_id] - el1['uid'] = self.changeset.user_id.to_s + if not user_display_name_cache[user_id].nil? + el1['user'] = user_display_name_cache[user_id] + el1['uid'] = user_id.to_s end self.relation_members.each do |member| @@ -145,7 +151,7 @@ class Relation < ActiveRecord::Base if ids.empty? return [] else - self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'Node' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do + self.with_scope(:find => { :joins => "INNER JOIN current_relation_members AS crm ON crm.id = current_relations.id", :conditions => "crm.member_type = 'Node' AND crm.member_id IN (#{ids.join(',')})" }) do return self.find(:all, options) end end @@ -155,7 +161,7 @@ class Relation < ActiveRecord::Base if ids.empty? return [] else - self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'Way' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do + self.with_scope(:find => { :joins => "INNER JOIN current_relation_members AS crm ON crm.id = current_relations.id", :conditions => "crm.member_type = 'Way' AND crm.member_id IN (#{ids.join(',')})" }) do return self.find(:all, options) end end @@ -165,7 +171,7 @@ class Relation < ActiveRecord::Base if ids.empty? return [] else - self.with_scope(:find => { :joins => "INNER JOIN current_relation_members ON current_relation_members.id = current_relations.id", :conditions => "current_relation_members.member_type = 'Relation' AND current_relation_members.member_id IN (#{ids.join(',')})" }) do + self.with_scope(:find => { :joins => "INNER JOIN current_relation_members AS crm ON crm.id = current_relations.id", :conditions => "crm.member_type = 'Relation' AND crm.member_id IN (#{ids.join(',')})" }) do return self.find(:all, options) end end @@ -227,7 +233,7 @@ class Relation < ActiveRecord::Base def delete_with_history!(new_relation, user) unless self.visible - raise OSM::APIAlreadyDeletedError.new + raise OSM::APIAlreadyDeletedError.new("relation", new_relation.id) end # need to start the transaction here, so that the database can @@ -236,9 +242,10 @@ class Relation < ActiveRecord::Base Relation.transaction do check_consistency(self, new_relation, user) # This will check to see if this relation is used by another relation - if RelationMember.find(:first, :joins => "INNER JOIN current_relations ON current_relations.id=current_relation_members.id", :conditions => [ "visible = ? AND member_type='Relation' and member_id=? ", true, self.id ]) - raise OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is a used in another relation") - end + rel = RelationMember.find(:first, :joins => :relation, + :conditions => [ "visible = ? AND member_type='Relation' and member_id=? ", true, self.id ]) + raise OSM::APIPreconditionFailedError.new("The relation #{new_relation.id} is used in relation #{rel.relation.id}.") unless rel.nil? + self.changeset_id = new_relation.changeset_id self.tags = {} self.members = [] @@ -249,8 +256,8 @@ class Relation < ActiveRecord::Base def update_from(new_relation, user) check_consistency(self, new_relation, user) - if !new_relation.preconditions_ok? - raise OSM::APIPreconditionFailedError.new + unless new_relation.preconditions_ok?(self.members) + raise OSM::APIPreconditionFailedError.new("Cannot update relation #{self.id}: data or member data is invalid.") end self.changeset_id = new_relation.changeset_id self.changeset = new_relation.changeset @@ -262,15 +269,15 @@ class Relation < ActiveRecord::Base def create_with_history(user) check_create_consistency(self, user) - if !self.preconditions_ok? - raise OSM::APIPreconditionFailedError.new + unless self.preconditions_ok? + raise OSM::APIPreconditionFailedError.new("Cannot create relation: data or member data is invalid.") end self.version = 0 self.visible = true save_with_history! end - def preconditions_ok? + def preconditions_ok?(good_members = []) # These are hastables that store an id in the index of all # the nodes/way/relations that have already been added. # If the member is valid and visible then we add it to the @@ -279,9 +286,12 @@ class Relation < ActiveRecord::Base # relation, then the hash table nodes would contain: # => {50=>true, 1=>true} elements = { :node => Hash.new, :way => Hash.new, :relation => Hash.new } + + # pre-set all existing members to good + good_members.each { |m| elements[m[0].downcase.to_sym][m[1]] = true } + self.members.each do |m| # find the hash for the element type or die - logger.debug m[0] hash = elements[m[0].downcase.to_sym] or return false # unless its in the cache already unless hash.key? m[1] @@ -292,6 +302,7 @@ class Relation < ActiveRecord::Base # and check that it is OK to use. unless element and element.visible? and element.preconditions_ok? + raise OSM::APIPreconditionFailedError.new("Relation with id #{self.id} cannot be saved due to #{m[0]} with id #{element.id}") return false end hash[m[1]] = true @@ -312,12 +323,12 @@ class Relation < ActiveRecord::Base # if any members are referenced by placeholder IDs (i.e: negative) then # this calling this method will fix them using the map from placeholders # to IDs +id_map+. - def fix_placeholders!(id_map) + def fix_placeholders!(id_map, placeholder_id = nil) self.members.map! do |type, id, role| old_id = id.to_i if old_id < 0 new_id = id_map[type.downcase.to_sym][old_id] - raise "invalid placeholder" if new_id.nil? + raise OSM::APIBadUserInput.new("Placeholder #{type} not found for reference #{old_id} in relation #{self.id.nil? ? placeholder_id : self.id}.") if new_id.nil? [type, new_id, role] else [type, id, role] @@ -362,7 +373,6 @@ class Relation < ActiveRecord::Base tags_changed |= (not tags.empty?) RelationTag.delete_all(:id => self.id) self.tags.each do |k,v| - logger.info "TAG added: #{k} -> #{v}" tag = RelationTag.new tag.k = k tag.v = v