X-Git-Url: https://git.openstreetmap.org/rails.git/blobdiff_plain/fd6f00b069af5a7b87227ea4c3401dc461d7311a..7331f82bce6ed54959b9550aa110c68e75f58eb0:/app/models/relation.rb diff --git a/app/models/relation.rb b/app/models/relation.rb index 36d6943d9..95181ef05 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 @@ -212,6 +218,11 @@ class Relation < ActiveRecord::Base # in the hash to be overwritten. raise OSM::APIDuplicateTagsError.new("relation", self.id, k) if @tags.include? k + # check tag size here, as we don't create a RelationTag object until + # just before we save... + raise OSM::APIBadUserInput.new("Relation #{self.id} has a tag with too long a key, '#{k}'.") if k.length > 255 + raise OSM::APIBadUserInput.new("Relation #{self.id} has a tag with too long a value, '#{k}'='#{v}'.") if v.length > 255 + @tags[k] = v end @@ -227,7 +238,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 +247,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 +261,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 +274,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,19 +291,23 @@ 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] # use reflection to look up the appropriate class model = Kernel.const_get(m[0].capitalize) # get the element with that ID - element = model.find(m[1]) + element = model.find(:first, :conditions =>["id = ?", m[1]]) # 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 @@ -299,8 +315,6 @@ class Relation < ActiveRecord::Base end return true - rescue - return false end # Temporary method to match interface to nodes @@ -312,12 +326,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] @@ -338,7 +352,7 @@ class Relation < ActiveRecord::Base self.timestamp = t self.save! - tags = self.tags + tags = self.tags.clone self.relation_tags.each do |old_tag| key = old_tag.k # if we can match the tags we currently have to the list @@ -346,11 +360,7 @@ class Relation < ActiveRecord::Base # if any are different then set the flag and do the DB # update. if tags.has_key? key - # rails 2.1 dirty handling should take care of making this - # somewhat efficient... hopefully... - old_tag.v = tags[key] - tags_changed |= old_tag.changed? - old_tag.save! + tags_changed |= (old_tag.v != tags[key]) # remove from the map, so that we can expect an empty map # at the end if there are no new tags @@ -359,13 +369,13 @@ class Relation < ActiveRecord::Base else # this means a tag was deleted tags_changed = true - RelationTag.delete_all ['id = ? and k = ?', self.id, old_tag.k] end end # if there are left-over tags then they are new and will have to # be added. tags_changed |= (not tags.empty?) - tags.each do |k,v| + RelationTag.delete_all(:id => self.id) + self.tags.each do |k,v| tag = RelationTag.new tag.k = k tag.v = v @@ -373,10 +383,6 @@ class Relation < ActiveRecord::Base tag.save! end - # reload, so that all of the members are accessible in their - # new state. - self.reload - # same pattern as before, but this time we're collecting the # changed members in an array, as the bounding box updates for # elements are per-element, not blanked on/off like for tags.