From 98b15bef455de6fcf83fec1e5fdddc244dc1a914 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Mon, 17 Nov 2008 11:45:50 +0000 Subject: [PATCH] Implemented changeset tags updating via the update method. --- app/controllers/changeset_controller.rb | 36 ++++++++++++++++++ app/models/changeset.rb | 25 ++++++++++++- config/routes.rb | 3 +- test/functional/changeset_controller_test.rb | 39 ++++++++++++++++++++ 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb index 8c042ef8d..cd49176e6 100644 --- a/app/controllers/changeset_controller.rb +++ b/app/controllers/changeset_controller.rb @@ -257,6 +257,42 @@ class ChangesetController < ApplicationController rescue String => s render :text => s, :content_type => "text/plain", :status => :bad_request end + + ## + # updates a changeset's tags. none of the changeset's attributes are + # user-modifiable, so they will be ignored. + # + # changesets are not (yet?) versioned, so we don't have to deal with + # history tables here. changesets are locked to a single user, however. + # + # after succesful update, returns the XML of the changeset. + def update + # request *must* be a PUT. + unless request.put? + render :nothing => true, :status => :method_not_allowed + return + end + + changeset = Changeset.find(params[:id]) + new_changeset = Changeset.from_xml(request.raw_post) + + unless new_changeset.nil? + changeset.update_from(new_changeset, @user) + render :text => changeset.to_xml, :mime_type => "text/xml" + else + + render :nothing => true, :status => :bad_request + end + + rescue ActiveRecord::RecordNotFound + render :nothing => true, :status => :not_found + rescue OSM::APIError => ex + render ex.render_opts + end + + #------------------------------------------------------------ + # utility functions below. + #------------------------------------------------------------ ## # merge two conditions diff --git a/app/models/changeset.rb b/app/models/changeset.rb index 4a4d12124..047569dca 100644 --- a/app/models/changeset.rb +++ b/app/models/changeset.rb @@ -104,13 +104,13 @@ class Changeset < ActiveRecord::Base def save_with_tags! t = Time.now + # 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 self.save! - end - ChangesetTag.transaction do tags = self.tags ChangesetTag.delete_all(['id = ?', self.id]) @@ -168,4 +168,25 @@ class Changeset < ActiveRecord::Base return el1 end + + ## + # update this instance from another instance given and the user who is + # doing the updating. note that this method is not for updating the + # bounding box, only the tags of the changeset. + def update_from(other, user) + # ensure that only the user who opened the changeset may modify it. + unless user.id == self.user_id + raise OSM::APIUserChangesetMismatchError + end + + # can't change a closed changeset + unless open + raise OSM::APIChangesetAlreadyClosedError + end + + # copy the other's tags + self.tags = other.tags + + save_with_tags! + end end diff --git a/config/routes.rb b/config/routes.rb index a835dc552..88fa0a551 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -7,7 +7,8 @@ ActionController::Routing::Routes.draw do |map| map.connect "api/#{API_VERSION}/changeset/:id/upload", :controller => 'changeset', :action => 'upload', :id => /\d+/ map.connect "api/#{API_VERSION}/changeset/:id/download", :controller => 'changeset', :action => 'download', :id => /\d+/ map.connect "api/#{API_VERSION}/changeset/:id/include", :controller => 'changeset', :action => 'include', :id => /\d+/ - map.connect "api/#{API_VERSION}/changeset/:id", :controller => 'changeset', :action => 'read', :id => /\d+/ + map.connect "api/#{API_VERSION}/changeset/:id", :controller => 'changeset', :action => 'read', :id => /\d+/, :conditions => { :method => :get } + map.connect "api/#{API_VERSION}/changeset/:id", :controller => 'changeset', :action => 'update', :id => /\d+/, :conditions => { :method => :put } map.connect "api/#{API_VERSION}/changeset/:id/close", :controller => 'changeset', :action => 'close', :id =>/\d+/ map.connect "api/#{API_VERSION}/changesets", :controller => 'changeset', :action => 'query' diff --git a/test/functional/changeset_controller_test.rb b/test/functional/changeset_controller_test.rb index d81bf46b8..1b0c63e2d 100644 --- a/test/functional/changeset_controller_test.rb +++ b/test/functional/changeset_controller_test.rb @@ -584,6 +584,45 @@ EOF # FIXME: write the actual test bit after fixing the fixtures! end + ## + # check updating tags on a changeset + def test_changeset_update + basic_authorization "test@openstreetmap.org", "test" + + changeset = changesets(:normal_user_first_change) + new_changeset = changeset.to_xml + new_tag = XML::Node.new "tag" + new_tag['k'] = "testing" + new_tag['v'] = "testing" + new_changeset.find("//osm/changeset").first << new_tag + + content new_changeset + put :update, :id => changeset.id + assert_response :success + + assert_select "osm>changeset[id=#{changeset.id}]", 1 + assert_select "osm>changeset>tag", 2 + assert_select "osm>changeset>tag[k=testing][v=testing]", 1 + end + + ## + # check that a user different from the one who opened the changeset + # can't modify it. + def test_changeset_update_invalid + basic_authorization "test@example.com", "test" + + changeset = changesets(:normal_user_first_change) + new_changeset = changeset.to_xml + new_tag = XML::Node.new "tag" + new_tag['k'] = "testing" + new_tag['v'] = "testing" + new_changeset.find("//osm/changeset").first << new_tag + + content new_changeset + put :update, :id => changeset.id + assert_response :conflict + end + #------------------------------------------------------------ # utility functions #------------------------------------------------------------ -- 2.43.2