Fixed some bugs in changeset query code. Added more test cases.

app/controllers/application.rb

@@ -22,7 +22,11 @@ class ApplicationController < ActionController::Base
     redirect_to :controller => 'user', :action => 'login', :referer => request.request_uri unless @user
   end
 
-  def authorize(realm='Web Password', errormessage="Couldn't authenticate you") 
+  ##
+  # sets up the @user object for use by other methods. this is mostly called
+  # from the authorize method, but can be called elsewhere if authorisation
+  # is optional.
+  def setup_user_auth
     username, passwd = get_auth_data # parse from headers
     # authenticate per-scheme
     if username.nil?
@@ -32,6 +36,11 @@ class ApplicationController < ActionController::Base
     else
       @user = User.authenticate(:username => username, :password => passwd) # basic auth
     end
+  end
+
+  def authorize(realm='Web Password', errormessage="Couldn't authenticate you") 
+    # make the @user object from any auth sources we have
+    setup_user_auth
 
     # handle authenticate pass/fail
     unless @user

app/controllers/changeset_controller.rb

@@ -236,9 +236,9 @@ class ChangesetController < ApplicationController
     # create the conditions that the user asked for. some or all of
     # these may be nil.
     conditions = conditions_bbox(params['bbox'])
-    cond_merge conditions, conditions_user(params['user'])
-    cond_merge conditions, conditions_time(params['time'])
-    cond_merge conditions, conditions_open(params['open'])
+    conditions = cond_merge conditions, conditions_user(params['user'])
+    conditions = cond_merge conditions, conditions_time(params['time'])
+    conditions = cond_merge conditions, conditions_open(params['open'])
 
     # create the results document
     results = OSM::API.new.get_xml_doc
@@ -333,7 +333,15 @@ class ChangesetController < ApplicationController
   def conditions_user(user)
     unless user.nil?
       u = User.find(user.to_i)
-      raise OSM::APINotFoundError unless u.data_public?
+      # should be able to get changesets of public users only, or 
+      # our own changesets regardless of public-ness.
+      unless u.data_public?
+        # get optional user auth stuff so that users can see their own
+        # changesets if they're non-public
+        setup_user_auth
+        
+        raise OSM::APINotFoundError if @user.nil? or @user.id != u.id
+      end
       return ['user_id = ?', u.id]
     else
       return nil
@@ -347,11 +355,11 @@ class ChangesetController < ApplicationController
       # if there is a range, i.e: comma separated, then the first is 
       # low, second is high - same as with bounding boxes.
       if time.count(',') == 1
-        from, to = time.split(/,/).collect { |t| Date.parse(t) }
+        from, to = time.split(/,/).collect { |t| DateTime.parse(t) }
         return ['created_at > ? and created_at < ?', from, to]
       else
         # if there is no comma, assume its a lower limit on time
-        return ['created_at > ?', Date.parse(time)]
+        return ['created_at > ?', DateTime.parse(time)]
       end
     else
       return nil

lib/osm.rb

@@ -17,6 +17,9 @@ module OSM
 
   # Raised when an API object is not found.
   class APINotFoundError < APIError
+    def render_opts
+      { :nothing => true, :status => :not_found }
+    end
   end
 
   # Raised when a precondition to an API action fails sanity check.

test/fixtures/changesets.yml

@@ -30,12 +30,17 @@ normal_user_version_change:
   user_id: 1
   created_at: "2008-01-01 00:00:00"
   open: true
+  min_lon: <%= 1 * SCALE %>
+  min_lat: <%= 1 * SCALE %>
+  max_lon: <%= 4 * SCALE %>
+  max_lat: <%= 4 * SCALE %>
 
 # changeset to contain all the invalid stuff that is in the
-# fixtures (nodes outside the world, etc...)
+# fixtures (nodes outside the world, etc...), but needs to have
+# a valid user...
 invalid_changeset:
   id: 5
-  user_id: 0
+  user_id: 3
   created_at: "2008-01-01 00:00:00"
   open: false
   

test/functional/changeset_controller_test.rb

@@ -595,7 +595,37 @@ EOF
   def test_changeset_by_bbox
     get :query, :bbox => "-10,-10, 10, 10"
     assert_response :success, "can't get changesets in bbox"
-    # FIXME: write the actual test bit after fixing the fixtures!
+    assert_changesets [1,4]
+
+    get :query, :bbox => "4.5,4.5,4.6,4.6"
+    assert_response :success, "can't get changesets in bbox"
+    assert_changesets [1]
+
+    # can't get changesets of user 1 without authenticating
+    get :query, :user => users(:normal_user).id
+    assert_response :not_found, "shouldn't be able to get changesets by non-public user"
+
+    # but this should work
+    basic_authorization "test@openstreetmap.org", "test"
+    get :query, :user => users(:normal_user).id
+    assert_response :success, "can't get changesets by user"
+    assert_changesets [1,3,4]
+
+    get :query, :user => users(:normal_user).id, :open => true
+    assert_response :success, "can't get changesets by user and open"
+    assert_changesets [1,4]
+
+    get :query, :time => '2007-12-31'
+    assert_response :success, "can't get changesets by time-since"
+    assert_changesets [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]
+
+    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]
   end
 
   ##
@@ -641,6 +671,16 @@ EOF
   # utility functions
   #------------------------------------------------------------
 
+  ##
+  # boilerplate for checking that certain changesets exist in the
+  # output.
+  def assert_changesets(ids)
+    assert_select "osm>changeset", ids.size
+    ids.each do |id|
+      assert_select "osm>changeset[id=#{id}]", 1
+    end
+  end
+
   ##
   # call the include method and assert properties of the bbox
   def check_after_include(changeset_id, lon, lat, bbox)