Fixed bug #1816 - the timeout updating logic should have been in a before_save handler, not in save_with_tags.

app/controllers/amf_controller.rb

@@ -815,12 +815,10 @@ class AmfController < ApplicationController
     return user
   end
 
-  # Update changeset timeout
-  # i.e. one hour after current edit
-  
-  def updatetimeout(changeset_id) #:doc:
+  # save the changeset identified by +changeset_id+. this 
+  # automatically sets the close timeout appropriately.
+  def updatetimeout(changeset_id) 
     cs = Changeset.find(changeset_id)
-    cs.closed_at = Time.now.getutc + Changeset::IDLE_TIMEOUT
     cs.save!
   end
 

app/models/changeset.rb

@@ -144,19 +144,9 @@ class Changeset < ActiveRecord::Base
   end
 
   def save_with_tags!
-    t = Time.now.getutc
-
     # 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.getutc + IDLE_TIMEOUT
-      end
       self.save!
 
       tags = self.tags
@@ -171,6 +161,20 @@ class Changeset < ActiveRecord::Base
       end
     end
   end
+
+  ##
+  # 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.
+  def before_save
+    if self.is_open?
+      if (closed_at - created_at) > (MAX_TIME_OPEN - IDLE_TIMEOUT)
+        self.closed_at = created_at + MAX_TIME_OPEN
+      else
+        self.closed_at = Time.now.getutc + IDLE_TIMEOUT
+      end
+    end
+  end
   
   def to_xml
     doc = OSM::API.new.get_xml_doc

test/fixtures/changesets.yml

@@ -17,7 +17,7 @@ normal_user_first_change:
 public_user_first_change:
   id: 2
   user_id: 2
-  created_at: "2008-05-01 01:23:45"
+  created_at: <%= Time.now.getutc %>
   closed_at: <%= DateTime.now + Rational(1,24) %>
   num_changes: 0
 
@@ -38,7 +38,7 @@ public_user_closed_change:
 public_user_version_change:
   id: 4
   user_id: 2
-  created_at: "2008-01-01 00:00:00"
+  created_at: <%= Time.now.getutc %>
   closed_at: <%= DateTime.now + Rational(1,24) %>
   min_lon: <%= 1 * SCALE %>
   min_lat: <%= 1 * SCALE %>

test/functional/amf_controller_test.rb

@@ -325,7 +325,8 @@ class AmfControllerTest < ActionController::TestCase
   # AMF Write tests
   def test_putpoi_update_valid
     nd = current_nodes(:visible_node)
-    amf_content "putpoi", "/1", ["test@openstreetmap.org:test", nd.changeset_id, nd.version, nd.id, nd.lon, nd.lat, nd.tags, nd.visible]
+    cs_id = changesets(:public_user_first_change).id
+    amf_content "putpoi", "/1", ["test@example.com:test", cs_id, nd.version, nd.id, nd.lon, nd.lat, nd.tags, nd.visible]
     post :amf_write
     assert_response :success
     amf_parse_response
@@ -339,7 +340,7 @@ class AmfControllerTest < ActionController::TestCase
     # Now try to update again, with a different lat/lon, using the updated version number
     lat = nd.lat+0.1
     lon = nd.lon-0.1
-    amf_content "putpoi", "/2", ["test@openstreetmap.org:test", nd.changeset_id, nd.version+1, nd.id, lon, lat, nd.tags, nd.visible]
+    amf_content "putpoi", "/2", ["test@example.com:test", cs_id, nd.version+1, nd.id, lon, lat, nd.tags, nd.visible]
     post :amf_write
     assert_response :success
     amf_parse_response
@@ -360,9 +361,9 @@ class AmfControllerTest < ActionController::TestCase
     lat = rand(100)-50 + rand
     lon = rand(100)-50 + rand
     # normal user has a changeset open
-    changeset = changesets(:normal_user_first_change)
+    changeset = changesets(:public_user_first_change)
     
-    amf_content "putpoi", "/1", ["test@openstreetmap.org:test", changeset.id, nil, nil, lon, lat, {}, nil]
+    amf_content "putpoi", "/1", ["test@example.com:test", changeset.id, nil, nil, lon, lat, {}, nil]
     post :amf_write
     assert_response :success
     amf_parse_response
@@ -399,9 +400,9 @@ class AmfControllerTest < ActionController::TestCase
     lat = rand(100)-50 + rand
     lon = rand(100)-50 + rand
     # normal user has a changeset open
-    changeset = changesets(:normal_user_first_change)
+    changeset = changesets(:public_user_first_change)
     
-    amf_content "putpoi", "/2", ["test@openstreetmap.org:test", changeset.id, nil, nil, lon, lat, { "key" => "value", "ping" => "pong" }, nil]
+    amf_content "putpoi", "/2", ["test@example.com:test", changeset.id, nil, nil, lon, lat, { "key" => "value", "ping" => "pong" }, nil]
     post :amf_write
     assert_response :success
     amf_parse_response

test/functional/changeset_controller_test.rb

@@ -128,7 +128,7 @@ class ChangesetControllerTest < ActionController::TestCase
     # test that it really is closed now
     cs = Changeset.find(cs_id)
     assert(!cs.is_open?, 
-           "changeset should be closed now (#{cs.closed_at} > #{Time.now}.")
+           "changeset should be closed now (#{cs.closed_at} > #{Time.now.getutc}.")
   end
 
   ##
@@ -1295,7 +1295,7 @@ EOF
 
     get :query, :time => '2007-12-31T23:59Z,2008-01-01T00:01Z'
     assert_response :success, "can't get changesets by time-range"
-    assert_changesets [1,4,5,6]
+    assert_changesets [1,5,6]
 
     get :query, :open => 'true'
     assert_response :success, "can't get changesets by open-ness"