raise OSM::APIUserChangesetMismatchError
end
- changeset.open = false
+ # to close the changeset, we'll just set its closed_at time to
+ # now. this might not be enough if there are concurrency issues,
+ # but we'll have to wait and see.
+ changeset.closed_at = DateTime.now
+
changeset.save!
render :nothing => true
rescue ActiveRecord::RecordNotFound
raise OSM::APIBadUserInput.new("bad time range") if times.size != 2
from, to = times.collect { |t| DateTime.parse(t) }
- return ['created_at > ? and created_at < ?', from, to]
+ return ['closed_at >= ? and created_at <= ?', from, to]
else
# if there is no comma, assume its a lower limit on time
- return ['created_at > ?', DateTime.parse(time)]
+ return ['closed_at >= ?', DateTime.parse(time)]
end
else
return nil
##
# restrict changes to those which are open
def conditions_open(open)
- return open.nil? ? nil : ['open = ?', true]
+ return open.nil? ? nil : ['closed_at >= ?', DateTime.now]
end
end
has_many :old_ways
has_many :old_relations
- validates_presence_of :user_id, :created_at
- validates_inclusion_of :open, :in => [ true, false ]
+ validates_presence_of :user_id, :created_at, :closed_at
# 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 (note that this
+ # is in days - so one hour is Rational(1,24)).
+ MAX_TIME_OPEN = 1
+
+ # idle timeout increment, one hour as a rational number of days.
+ IDLE_TIMEOUT = Rational(1,24)
+
# 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?
- return 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 > DateTime.now) and (num_changes <= MAX_ELEMENTS))
end
def self.from_xml(xml, create=false)
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|
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
# do the changeset update and the changeset tags update in the
# same transaction to ensure consistency.
Changeset.transaction do
- # fixme update modified_at time?
- # FIXME there is no modified_at time, should it be added
+ # 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 = DateTime.now + IDLE_TIMEOUT
+ end
self.save!
tags = self.tags
end
el1['created_at'] = self.created_at.xmlschema
- el1['open'] = self.open.to_s
+ 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?
end
# can't change a closed changeset
- unless open
+ unless is_open?
raise OSM::APIChangesetAlreadyClosedError
end
old_node.timestamp = t
old_node.save_with_dependencies!
+ # tell the changeset we updated one element only
+ changeset.add_changes! 1
+
# save the changeset in case of bounding box updates
changeset.save!
end
end
end
+ # tell the changeset we updated one element only
+ changeset.add_changes! 1
+
# save the (maybe updated) changeset bounding box
changeset.save!
end
# update and commit the bounding box, now that way nodes
# have been updated and we're in a transaction.
changeset.update_bbox!(bbox) unless nodes.empty?
+
+ # tell the changeset we updated one element only
+ changeset.add_changes! 1
+
changeset.save!
end
end
--- /dev/null
+class AddEndTimeToChangesets < ActiveRecord::Migration
+ def self.up
+ # swap the boolean closed-or-not for a time when the changeset will
+ # close or has closed.
+ add_column(:changesets, :closed_at, :datetime, :null => false)
+
+ # it appears that execute will only accept string arguments, so
+ # this is an ugly, ugly hack to get some sort of mysql/postgres
+ # independence. now i have to go wash my brain with bleach.
+ execute("update changesets set closed_at=(now()-'1 hour') where open=(1=0)")
+ execute("update changesets set closed_at=(now()+'1 hour') where open=(1=1)")
+
+ # remove the open column as it is unnecessary now and denormalises
+ # the table.
+ remove_column :changesets, :open
+
+ # add a column to keep track of the number of changes in a changeset.
+ # could probably work out how many changes there are here, but i'm not
+ # sure its actually important.
+ add_column(:changesets, :num_changes, :integer,
+ :null => false, :default => 0)
+ end
+
+ def self.down
+ # in the reverse direction, we can look at the closed_at to figure out
+ # if changesets are closed or not.
+ add_column(:changesets, :open, :boolean, :null => false, :default => true)
+ execute("update changesets set open=(closed_at > now())")
+ remove_column :changesets, :closed_at
+
+ # remove the column for tracking number of changes
+ remove_column :changesets, :num_changes
+ end
+end
id: 1
user_id: 1
created_at: "2007-01-01 00:00:00"
- open: true
+ closed_at: <%= DateTime.now + Rational(1,24) %>
min_lon: <%= 1 * SCALE %>
min_lat: <%= 1 * SCALE %>
max_lon: <%= 5 * SCALE %>
max_lat: <%= 5 * SCALE %>
+ num_changes: 11
second_user_first_change:
id: 2
user_id: 2
created_at: "2008-05-01 01:23:45"
- open: true
+ closed_at: <%= DateTime.now + Rational(1,24) %>
+ num_changes: 0
normal_user_closed_change:
id: 3
user_id: 1
created_at: "2007-01-01 00:00:00"
- open: false
+ closed_at: "2007-01-02 00:00:00"
+ num_changes: 0
normal_user_version_change:
id: 4
user_id: 1
created_at: "2008-01-01 00:00:00"
- open: true
+ closed_at: <%= DateTime.now + Rational(1,24) %>
min_lon: <%= 1 * SCALE %>
min_lat: <%= 1 * SCALE %>
max_lon: <%= 4 * SCALE %>
max_lat: <%= 4 * SCALE %>
+ num_changes: 8
# changeset to contain all the invalid stuff that is in the
# fixtures (nodes outside the world, etc...), but needs to have
id: 5
user_id: 3
created_at: "2008-01-01 00:00:00"
- open: false
-
\ No newline at end of file
+ closed_at: "2008-01-02 00:00:00"
+ num_changes: 9
get :query, :time => '2007-12-31'
assert_response :success, "can't get changesets by time-since"
- assert_changesets [2,4,5]
+ assert_changesets [1,2,4,5]
get :query, :time => '2008-01-01T12:34Z'
assert_response :success, "can't get changesets by time-since with hour"
- assert_changesets [2]
+ assert_changesets [1,2,4,5]
get :query, :time => '2007-12-31T23:59Z,2008-01-01T00:01Z'
assert_response :success, "can't get changesets by time-range"
- assert_changesets [4,5]
+ assert_changesets [1,4,5]
+
+ get :query, :open => 'true'
+ assert_response :success, "can't get changesets by open-ness"
+ assert_changesets [1,2,4]
end
##
assert_response :conflict
end
+ ##
+ # check that a changeset can contain a certain max number of changes.
+ def test_changeset_limits
+ basic_authorization "test@openstreetmap.org", "test"
+
+ # open a new changeset
+ content "<osm><changeset/></osm>"
+ put :create
+ assert_response :success, "can't create a new changeset"
+ cs_id = @response.body.to_i
+
+ # start the counter just short of where the changeset should finish.
+ offset = 10
+ # alter the database to set the counter on the changeset directly,
+ # otherwise it takes about 6 minutes to fill all of them.
+ changeset = Changeset.find(cs_id)
+ changeset.num_changes = Changeset::MAX_ELEMENTS - offset
+ changeset.save!
+
+ with_controller(NodeController.new) do
+ # create a new node
+ content "<osm><node changeset='#{cs_id}' lat='0.0' lon='0.0'/></osm>"
+ put :create
+ assert_response :success, "can't create a new node"
+ node_id = @response.body.to_i
+
+ get :read, :id => node_id
+ assert_response :success, "can't read back new node"
+ node_doc = XML::Parser.string(@response.body).parse
+ node_xml = node_doc.find("//osm/node").first
+
+ # loop until we fill the changeset with nodes
+ offset.times do |i|
+ node_xml['lat'] = rand.to_s
+ node_xml['lon'] = rand.to_s
+ node_xml['version'] = (i+1).to_s
+
+ content node_doc
+ put :update, :id => node_id
+ assert_response :success, "attempt #{i} should have succeeded"
+ end
+
+ # trying again should fail
+ node_xml['lat'] = rand.to_s
+ node_xml['lon'] = rand.to_s
+ node_xml['version'] = offset.to_s
+
+ content node_doc
+ put :update, :id => node_id
+ assert_response :conflict, "final attempt should have failed"
+ end
+
+ changeset = Changeset.find(cs_id)
+ assert_equal Changeset::MAX_ELEMENTS + 1, changeset.num_changes
+ end
+
#------------------------------------------------------------
# utility functions
#------------------------------------------------------------
projects / rails.git / commitdiff