cf6a5c17ee
The bug allows a newly-created element to refer to a deleted one if the transactions for both overlap. Precisely, the issue is that the check that an element exists does not prevent a concurrent transaction from altering that row. Because "deleting" an element in the OSM database does not remove the row, we cannot rely on FK constraints to ensure the correct behaviour. Instead, this fix relies on manually locking referenced elements. Note that this "fix" is suboptimal, as it does not allow any updates to the referenced elements. Updates which do not delete the row could safely be done, but will be prevented. Also, it's not clear what the negative performance impact of this change will be.
394 lines
14 KiB
Ruby
394 lines
14 KiB
Ruby
class Relation < ActiveRecord::Base
|
|
require "xml/libxml"
|
|
|
|
include ConsistencyValidations
|
|
include NotRedactable
|
|
include ObjectMetadata
|
|
|
|
self.table_name = "current_relations"
|
|
|
|
belongs_to :changeset
|
|
|
|
has_many :old_relations, -> { order(:version) }
|
|
|
|
has_many :relation_members, -> { order(:sequence_id) }
|
|
has_many :relation_tags
|
|
|
|
has_many :containing_relation_members, :class_name => "RelationMember", :as => :member
|
|
has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation
|
|
|
|
validates :id, :uniqueness => true, :presence => { :on => :update },
|
|
:numericality => { :on => :update, :integer_only => true }
|
|
validates :version, :presence => true,
|
|
:numericality => { :integer_only => true }
|
|
validates :changeset_id, :presence => true,
|
|
:numericality => { :integer_only => true }
|
|
validates :timestamp, :presence => true
|
|
validates :changeset, :associated => true
|
|
validates :visible, :inclusion => [true, false]
|
|
|
|
scope :visible, -> { where(:visible => true) }
|
|
scope :invisible, -> { where(:visible => false) }
|
|
scope :nodes, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Node", :member_id => ids.flatten }) }
|
|
scope :ways, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Way", :member_id => ids.flatten }) }
|
|
scope :relations, ->(*ids) { joins(:relation_members).where(:current_relation_members => { :member_type => "Relation", :member_id => ids.flatten }) }
|
|
|
|
TYPES = %w(node way relation)
|
|
|
|
def self.from_xml(xml, create = false)
|
|
p = XML::Parser.string(xml)
|
|
doc = p.parse
|
|
|
|
doc.find("//osm/relation").each do |pt|
|
|
return Relation.from_xml_node(pt, create)
|
|
end
|
|
fail OSM::APIBadXMLError.new("node", xml, "XML doesn't contain an osm/relation element.")
|
|
rescue LibXML::XML::Error, ArgumentError => ex
|
|
raise OSM::APIBadXMLError.new("relation", xml, ex.message)
|
|
end
|
|
|
|
def self.from_xml_node(pt, create = false)
|
|
relation = Relation.new
|
|
|
|
fail OSM::APIBadXMLError.new("relation", pt, "Version is required when updating") unless create || !pt["version"].nil?
|
|
relation.version = pt["version"]
|
|
fail OSM::APIBadXMLError.new("relation", pt, "Changeset id is missing") if pt["changeset"].nil?
|
|
relation.changeset_id = pt["changeset"]
|
|
|
|
unless create
|
|
fail OSM::APIBadXMLError.new("relation", pt, "ID is required when updating") if pt["id"].nil?
|
|
relation.id = pt["id"].to_i
|
|
# .to_i will return 0 if there is no number that can be parsed.
|
|
# We want to make sure that there is no id with zero anyway
|
|
fail OSM::APIBadUserInput.new("ID of relation cannot be zero when updating.") if relation.id == 0
|
|
end
|
|
|
|
# We don't care about the timestamp nor the visibility as these are either
|
|
# set explicitly or implicit in the action. The visibility is set to true,
|
|
# and manually set to false before the actual delete.
|
|
relation.visible = true
|
|
|
|
# Start with no tags
|
|
relation.tags = {}
|
|
|
|
# Add in any tags from the XML
|
|
pt.find("tag").each do |tag|
|
|
fail OSM::APIBadXMLError.new("relation", pt, "tag is missing key") if tag["k"].nil?
|
|
fail OSM::APIBadXMLError.new("relation", pt, "tag is missing value") if tag["v"].nil?
|
|
relation.add_tag_keyval(tag["k"], tag["v"])
|
|
end
|
|
|
|
# need to initialise the relation members array explicitly, as if this
|
|
# isn't done for a new relation then @members attribute will be nil,
|
|
# and the members will be loaded from the database instead of being
|
|
# empty, as intended.
|
|
relation.members = []
|
|
|
|
pt.find("member").each do |member|
|
|
# member_type =
|
|
fail OSM::APIBadXMLError.new("relation", pt, "The #{member['type']} is not allowed only, #{TYPES.inspect} allowed") unless TYPES.include? member["type"]
|
|
# member_ref = member['ref']
|
|
# member_role
|
|
member["role"] ||= "" # Allow the upload to not include this, in which case we default to an empty string.
|
|
relation.add_member(member["type"].classify, member["ref"], member["role"])
|
|
end
|
|
fail OSM::APIBadUserInput.new("Some bad xml in relation") if relation.nil?
|
|
|
|
relation
|
|
end
|
|
|
|
def to_xml
|
|
doc = OSM::API.new.get_xml_doc
|
|
doc.root << to_xml_node
|
|
doc
|
|
end
|
|
|
|
def to_xml_node(visible_members = nil, changeset_cache = {}, user_display_name_cache = {})
|
|
el = XML::Node.new "relation"
|
|
el["id"] = id.to_s
|
|
|
|
add_metadata_to_xml_node(el, self, changeset_cache, user_display_name_cache)
|
|
|
|
relation_members.each do |member|
|
|
p = 0
|
|
|
|
if visible_members
|
|
# if there is a list of visible members then use that to weed out deleted segments
|
|
p = 1 if visible_members[member.member_type][member.member_id]
|
|
else
|
|
# otherwise, manually go to the db to check things
|
|
p = 1 if member.member.visible?
|
|
end
|
|
|
|
next unless p
|
|
|
|
member_el = XML::Node.new "member"
|
|
member_el["type"] = member.member_type.downcase
|
|
member_el["ref"] = member.member_id.to_s
|
|
member_el["role"] = member.member_role
|
|
el << member_el
|
|
end
|
|
|
|
add_tags_to_xml_node(el, relation_tags)
|
|
|
|
el
|
|
end
|
|
|
|
# FIXME: is this really needed?
|
|
def members
|
|
@members ||= relation_members.map do |member|
|
|
[member.member_type, member.member_id, member.member_role]
|
|
end
|
|
end
|
|
|
|
def tags
|
|
@tags ||= Hash[relation_tags.collect { |t| [t.k, t.v] }]
|
|
end
|
|
|
|
attr_writer :members
|
|
|
|
attr_writer :tags
|
|
|
|
def add_member(type, id, role)
|
|
@members ||= []
|
|
@members << [type, id.to_i, role]
|
|
end
|
|
|
|
def add_tag_keyval(k, v)
|
|
@tags = {} unless @tags
|
|
|
|
# duplicate tags are now forbidden, so we can't allow values
|
|
# in the hash to be overwritten.
|
|
fail OSM::APIDuplicateTagsError.new("relation", id, k) if @tags.include? k
|
|
|
|
@tags[k] = v
|
|
end
|
|
|
|
##
|
|
# updates the changeset bounding box to contain the bounding box of
|
|
# the element with given +type+ and +id+. this only works with nodes
|
|
# and ways at the moment, as they're the only elements to respond to
|
|
# the :bbox call.
|
|
def update_changeset_element(type, id)
|
|
element = Kernel.const_get(type.capitalize).find(id)
|
|
changeset.update_bbox! element.bbox
|
|
end
|
|
|
|
def delete_with_history!(new_relation, user)
|
|
unless visible
|
|
fail OSM::APIAlreadyDeletedError.new("relation", new_relation.id)
|
|
end
|
|
|
|
# need to start the transaction here, so that the database can
|
|
# provide repeatable reads for the used-by checks. this means it
|
|
# shouldn't be possible to get race conditions.
|
|
Relation.transaction do
|
|
self.lock!
|
|
check_consistency(self, new_relation, user)
|
|
# This will check to see if this relation is used by another relation
|
|
rel = RelationMember.joins(:relation).find_by("visible = ? AND member_type = 'Relation' and member_id = ? ", true, id)
|
|
fail 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 = []
|
|
self.visible = false
|
|
save_with_history!
|
|
end
|
|
end
|
|
|
|
def update_from(new_relation, user)
|
|
Relation.transaction do
|
|
self.lock!
|
|
check_consistency(self, new_relation, user)
|
|
unless new_relation.preconditions_ok?(members)
|
|
fail OSM::APIPreconditionFailedError.new("Cannot update relation #{id}: data or member data is invalid.")
|
|
end
|
|
self.changeset_id = new_relation.changeset_id
|
|
self.changeset = new_relation.changeset
|
|
self.tags = new_relation.tags
|
|
self.members = new_relation.members
|
|
self.visible = true
|
|
save_with_history!
|
|
end
|
|
end
|
|
|
|
def create_with_history(user)
|
|
check_create_consistency(self, user)
|
|
unless self.preconditions_ok?
|
|
fail 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?(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
|
|
# relevant hash table, with the value true as a cache.
|
|
# Thus if you have nodes with the ids of 50 and 1 already in the
|
|
# relation, then the hash table nodes would contain:
|
|
# => {50=>true, 1=>true}
|
|
elements = { :node => {}, :way => {}, :relation => {} }
|
|
|
|
# pre-set all existing members to good
|
|
good_members.each { |m| elements[m[0].downcase.to_sym][m[1]] = true }
|
|
|
|
members.each do |m|
|
|
# find the hash for the element type or die
|
|
hash = elements[m[0].downcase.to_sym]
|
|
return false unless hash
|
|
|
|
# unless its in the cache already
|
|
next if 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. and, if found, lock the element to
|
|
# ensure it can't be deleted until after the current transaction
|
|
# commits.
|
|
element = model.lock("for share").find_by(:id => m[1])
|
|
|
|
# and check that it is OK to use.
|
|
unless element && element.visible? && element.preconditions_ok?
|
|
fail OSM::APIPreconditionFailedError.new("Relation with id #{id} cannot be saved due to #{m[0]} with id #{m[1]}")
|
|
end
|
|
hash[m[1]] = true
|
|
end
|
|
|
|
true
|
|
end
|
|
|
|
##
|
|
# 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, placeholder_id = nil)
|
|
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]
|
|
fail 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]
|
|
end
|
|
end
|
|
end
|
|
|
|
private
|
|
|
|
def save_with_history!
|
|
Relation.transaction do
|
|
# have to be a little bit clever here - to detect if any tags
|
|
# changed then we have to monitor their before and after state.
|
|
tags_changed = false
|
|
|
|
t = Time.now.getutc
|
|
self.version += 1
|
|
self.timestamp = t
|
|
self.save!
|
|
|
|
tags = self.tags.clone
|
|
relation_tags.each do |old_tag|
|
|
key = old_tag.k
|
|
# if we can match the tags we currently have to the list
|
|
# of old tags, then we never set the tags_changed flag. but
|
|
# if any are different then set the flag and do the DB
|
|
# update.
|
|
if tags.key? key
|
|
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
|
|
tags.delete key
|
|
|
|
else
|
|
# this means a tag was deleted
|
|
tags_changed = true
|
|
end
|
|
end
|
|
# if there are left-over tags then they are new and will have to
|
|
# be added.
|
|
tags_changed |= (!tags.empty?)
|
|
RelationTag.delete_all(:relation_id => id)
|
|
self.tags.each do |k, v|
|
|
tag = RelationTag.new
|
|
tag.relation_id = id
|
|
tag.k = k
|
|
tag.v = v
|
|
tag.save!
|
|
end
|
|
|
|
# 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.
|
|
changed_members = []
|
|
members = self.members.clone
|
|
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
|
|
|
|
# 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
|
|
# a longest common subsequence algorithm to optimise this.
|
|
members = self.members
|
|
RelationMember.delete_all(:relation_id => id)
|
|
members.each_with_index do |m, i|
|
|
mem = RelationMember.new
|
|
mem.relation_id = id
|
|
mem.sequence_id = i
|
|
mem.member_type = m[0]
|
|
mem.member_id = m[1]
|
|
mem.member_role = m[2]
|
|
mem.save!
|
|
end
|
|
|
|
old_relation = OldRelation.from_relation(self)
|
|
old_relation.timestamp = t
|
|
old_relation.save_with_dependencies!
|
|
|
|
# update the bbox of the changeset and save it too.
|
|
# discussion on the mailing list gave the following definition for
|
|
# the bounding box update procedure of a relation:
|
|
#
|
|
# adding or removing nodes or ways from a relation causes them to be
|
|
# added to the changeset bounding box. adding a relation member or
|
|
# changing tag values causes all node and way members to be added to the
|
|
# bounding box. this is similar to how the map call does things and is
|
|
# reasonable on the assumption that adding or removing members doesn't
|
|
# materially change the rest of the relation.
|
|
any_relations =
|
|
changed_members.collect { |_id, type| type == "relation" }
|
|
.inject(false) { |a, e| a || e }
|
|
|
|
update_members = if tags_changed || 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
|
|
else
|
|
changed_members
|
|
end
|
|
update_members.each do |type, id, _role|
|
|
update_changeset_element(type, id) if type != "Relation"
|
|
end
|
|
|
|
# tell the changeset we updated one element only
|
|
changeset.add_changes! 1
|
|
|
|
# save the (maybe updated) changeset bounding box
|
|
changeset.save!
|
|
end
|
|
end
|
|
end
|