Fixed bug in changeset idle timeout. Fixed another with a spurious require.

diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb
index ca19fba308f067733822619f95835bb23c03a993..5e538c721c09527737d9578b3b33feea5990ab23 100644 (file)
--- a/app/controllers/changeset_controller.rb
+++ b/app/controllers/changeset_controller.rb
@@ -2,7 +2,6 @@
 
 class ChangesetController < ApplicationController
   require 'xml/libxml'
-  require 'diff_reader'
 
   before_filter :authorize, :only => [:create, :update, :delete, :upload, :include, :close]
   before_filter :check_write_availability, :only => [:create, :update, :delete, :upload, :include]

diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index a65e3aebcfa4635dca8f257dc7d3f67c6596a0cc..38cd8014f734efe2d27629127609cd7813532288 100644 (file)
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -25,7 +25,10 @@ class Changeset < ActiveRecord::Base
   MAX_TIME_OPEN = 1
 
   # idle timeout increment, one hour as a rational number of days.
-  IDLE_TIMEOUT = 1.hour #Rational(1,24)
+  # NOTE: DO NOT CHANGE THIS TO 1.hour! when this was done the idle
+  # timeout changed to 1 second, which meant all changesets closed 
+  # almost immediately.
+  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 
@@ -219,7 +222,7 @@ class Changeset < ActiveRecord::Base
     
     # can't change a closed changeset
     unless is_open?
-      raise OSM::APIChangesetAlreadyClosedError
+      raise OSM::APIChangesetAlreadyClosedError.new(self)
     end
 
     # copy the other's tags

diff --git a/lib/consistency_validations.rb b/lib/consistency_validations.rb
index 6e214f90271a066b77a50873c19caddf6840cacd..46fb3c06e2ad771a9fd00db2ab618d4e0c453b80 100644 (file)
--- a/lib/consistency_validations.rb
+++ b/lib/consistency_validations.rb
@@ -13,7 +13,7 @@ module ConsistencyValidations
     elsif new.changeset.user_id != user.id
       raise OSM::APIUserChangesetMismatchError.new
     elsif not new.changeset.is_open?
-      raise OSM::APIChangesetAlreadyClosedError.new
+      raise OSM::APIChangesetAlreadyClosedError.new(new.changeset)
     end
   end
   
@@ -24,7 +24,7 @@ module ConsistencyValidations
     elsif new.changeset.user_id != user.id
       raise OSM::APIUserChangesetMismatchError.new
     elsif not new.changeset.is_open?
-      raise OSM::APIChangesetAlreadyClosedError.new
+      raise OSM::APIChangesetAlreadyClosedError.new(new.changeset)
     end
   end
 end

diff --git a/lib/osm.rb b/lib/osm.rb
index 00215c6773e74f2e5895e407fd1ef9a809a3f094..09ded2bd231806c43ae116057bf04c014389a5bc 100644 (file)
--- a/lib/osm.rb
+++ b/lib/osm.rb
@@ -45,8 +45,14 @@ module OSM
 
   # Raised when the changeset provided is already closed
   class APIChangesetAlreadyClosedError < APIError
+    def initialize(changeset)
+      @changeset = changeset
+    end
+
+    attr_reader :changeset
+    
     def render_opts
-      { :text => "The supplied changeset has already been closed", :status => :conflict }
+      { :text => "The changeset #{@changeset.id} was closed at #{@changeset.closed_at}.", :status => :conflict }
     end
   end
   

diff --git a/test/functional/changeset_controller_test.rb b/test/functional/changeset_controller_test.rb
index 3d7531cfba0c79c87a885379292af531af2b61a6..47cd95c241a9915dbf53df19546dfa3a73425c8d 100644 (file)
--- a/test/functional/changeset_controller_test.rb
+++ b/test/functional/changeset_controller_test.rb
@@ -26,7 +26,19 @@ class ChangesetControllerTest < ActionController::TestCase
     put :create
     
     assert_response :success, "Creation of changeset did not return sucess status"
-    newid = @response.body
+    newid = @response.body.to_i
+
+    # check end time, should be an hour ahead of creation time
+    cs = Changeset.find(newid)
+    duration = cs.closed_at - cs.created_at
+    # the difference can either be a rational, or a floating point number
+    # of seconds, depending on the code path taken :-(
+    if duration.class == Rational
+      assert_equal Rational(1,24), duration , "initial idle timeout should be an hour (#{cs.created_at} -> #{cs.closed_at})"
+    else
+      # must be number of seconds...
+      assert_equal 3600.0, duration , "initial idle timeout should be an hour (#{cs.created_at} -> #{cs.closed_at})"
+    end
   end
   
   def test_create_invalid
@@ -455,6 +467,67 @@ EOF
     assert_select "osmChange>modify>node", 8
   end
   
+  ##
+  # culled this from josm to ensure that nothing in the way that josm
+  # is formatting the request is causing it to fail.
+  #
+  # NOTE: the error turned out to be something else completely!
+  def test_josm_upload
+    basic_authorization(users(:normal_user).email, "test")
+
+    # create a temporary changeset
+    content "<osm><changeset>" +
+      "<tag k='created_by' v='osm test suite checking changesets'/>" + 
+      "</changeset></osm>"
+    put :create
+    assert_response :success
+    changeset_id = @response.body.to_i
+
+    diff = <<OSM
+<osmChange version="0.6" generator="JOSM">
+<create version="0.6" generator="JOSM">
+  <node id='-1' visible='true' changeset='#{changeset_id}' lat='51.49619982187321' lon='-0.18722061869438314' />
+  <node id='-2' visible='true' changeset='#{changeset_id}' lat='51.496359883909605' lon='-0.18653093576241928' />
+  <node id='-3' visible='true' changeset='#{changeset_id}' lat='51.49598132358285' lon='-0.18719613290981638' />
+  <node id='-4' visible='true' changeset='#{changeset_id}' lat='51.4961591711078' lon='-0.18629015888084607' />
+  <node id='-5' visible='true' changeset='#{changeset_id}' lat='51.49582126021711' lon='-0.18708186591517145' />
+  <node id='-6' visible='true' changeset='#{changeset_id}' lat='51.49591018437858' lon='-0.1861432441734455' />
+  <node id='-7' visible='true' changeset='#{changeset_id}' lat='51.49560784152179' lon='-0.18694719410005425' />
+  <node id='-8' visible='true' changeset='#{changeset_id}' lat='51.49567389979617' lon='-0.1860289771788006' />
+  <node id='-9' visible='true' changeset='#{changeset_id}' lat='51.49543761398892' lon='-0.186820684213126' />
+  <way id='-10' action='modiy' visible='true' changeset='#{changeset_id}'>
+    <nd ref='-1' />
+    <nd ref='-2' />
+    <nd ref='-3' />
+    <nd ref='-4' />
+    <nd ref='-5' />
+    <nd ref='-6' />
+    <nd ref='-7' />
+    <nd ref='-8' />
+    <nd ref='-9' />
+    <tag k='highway' v='residential' />
+    <tag k='name' v='Foobar Street' />
+  </way>
+</create>
+</osmChange>
+OSM
+
+    # upload it
+    content diff
+    post :upload, :id => changeset_id
+    assert_response :success, 
+      "can't upload a diff from JOSM: #{@response.body}"
+    
+    get :download, :id => changeset_id
+    assert_response :success
+
+    assert_select "osmChange", 1
+    assert_select "osmChange>create>node", 9
+    assert_select "osmChange>create>way", 1
+    assert_select "osmChange>create>way>nd", 9
+    assert_select "osmChange>create>way>tag", 2
+  end
+
   ##
   # when we make some complex changes we get the same changes back from the 
   # diff download.