From fb5f39f19ae00dad9ec896e216e8e813de42c151 Mon Sep 17 00:00:00 2001 From: Shaun McDonald Date: Mon, 13 Oct 2008 15:39:21 +0000 Subject: [PATCH] Creating consistency check for creation of nodes, way and relations. Moving some creation code from the controller to the model, and adding error handling on create errors. --- app/controllers/node_controller.rb | 27 ++++++++++++-------------- app/controllers/relation_controller.rb | 23 ++++++++++------------ app/controllers/way_controller.rb | 26 ++++++++++--------------- app/models/node.rb | 7 +++++++ app/models/relation.rb | 10 ++++++++++ app/models/way.rb | 10 ++++++++++ lib/consistency_validations.rb | 11 +++++++++++ 7 files changed, 70 insertions(+), 44 deletions(-) diff --git a/app/controllers/node_controller.rb b/app/controllers/node_controller.rb index 62e680388..309b930b7 100644 --- a/app/controllers/node_controller.rb +++ b/app/controllers/node_controller.rb @@ -11,24 +11,21 @@ class NodeController < ApplicationController # Create a node from XML. def create - if request.put? - node = Node.from_xml(request.raw_post, true) - # FIXME remove debug - logger.debug request.raw_post - logger.debug node - - if node - node.version = 0 - #node.changeset_id = node.changeset - node.visible = true - node.save_with_history! + begin + if request.put? + node = Node.from_xml(request.raw_post, true) - render :text => node.id.to_s, :content_type => "text/plain" + if node + node.create_with_history @user + render :text => node.id.to_s, :content_type => "text/plain" + else + render :nothing => true, :status => :bad_request + end else - render :nothing => true, :status => :bad_request + render :nothing => true, :status => :method_not_allowed end - else - render :nothing => true, :status => :method_not_allowed + rescue OSM::APIError => ex + render ex.render_opts end end diff --git a/app/controllers/relation_controller.rb b/app/controllers/relation_controller.rb index 09c878325..4b3fdf34f 100644 --- a/app/controllers/relation_controller.rb +++ b/app/controllers/relation_controller.rb @@ -8,24 +8,21 @@ class RelationController < ApplicationController after_filter :compress_output def create - if request.put? - relation = Relation.from_xml(request.raw_post, true) - - if relation - if !relation.preconditions_ok? - render :text => "", :status => :precondition_failed - else - relation.version = 0 - #relation.user_id = @user.id - relation.save_with_history! + begin + if request.put? + relation = Relation.from_xml(request.raw_post, true) + if relation + relation.create_with_history @user render :text => relation.id.to_s, :content_type => "text/plain" + else + render :nothing => true, :status => :bad_request end else - render :nothing => true, :status => :bad_request + render :nothing => true, :status => :method_not_allowed end - else - render :nothing => true, :status => :method_not_allowed + rescue OSM::APIError => ex + render ex.render_opts end end diff --git a/app/controllers/way_controller.rb b/app/controllers/way_controller.rb index 08270094d..e7cf0f7f1 100644 --- a/app/controllers/way_controller.rb +++ b/app/controllers/way_controller.rb @@ -8,27 +8,21 @@ class WayController < ApplicationController after_filter :compress_output def create - if request.put? - way = Way.from_xml(request.raw_post, true) - - if way - # FIXME move some of this to the model. The controller shouldn't need to - # know about the fact that the first version number is 0 on creation - # it will also allow use to run a variation on the check_consistency - # so that we don't get exceptions thrown when the changesets are not right - unless way.preconditions_ok? - render :text => "", :status => :precondition_failed - else - way.version = 0 - way.save_with_history! + begin + if request.put? + way = Way.from_xml(request.raw_post, true) + if way + way.create_with_history @user render :text => way.id.to_s, :content_type => "text/plain" + else + render :nothing => true, :status => :bad_request end else - render :nothing => true, :status => :bad_request + render :nothing => true, :status => :method_not_allowed end - else - render :nothing => true, :status => :method_not_allowed + rescue OSM::APIError => ex + render ex.render_opts end end diff --git a/app/models/node.rb b/app/models/node.rb index e7058a5ad..d6a5143db 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -161,6 +161,13 @@ class Node < ActiveRecord::Base self.visible = true save_with_history! end + + def create_with_history(user) + check_create_consistency(self, user) + self.version = 0 + self.visible = true + save_with_history! + end def to_xml doc = OSM::API.new.get_xml_doc diff --git a/app/models/relation.rb b/app/models/relation.rb index dd2d1c7d5..195090e4b 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -251,6 +251,16 @@ class Relation < ActiveRecord::Base self.visible = true save_with_history! end + + def create_with_history(user) + check_create_consistency(self, user) + if !self.preconditions_ok? + raise OSM::APIPreconditionFailedError.new + end + self.version = 0 + self.visible = true + save_with_history! + end def preconditions_ok? # These are hastables that store an id in the index of all diff --git a/app/models/way.rb b/app/models/way.rb index 6a5ad58ab..b2bdfb39b 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -219,6 +219,16 @@ class Way < ActiveRecord::Base save_with_history! end + def create_with_history(user) + check_create_consistency(self, user) + if !self.preconditions_ok? + raise OSM::APIPreconditionsFailedError.new + end + self.version = 0 + self.visible = true + save_with_history! + end + def preconditions_ok? return false if self.nds.empty? self.nds.each do |n| diff --git a/lib/consistency_validations.rb b/lib/consistency_validations.rb index 4becce89c..8fd6c257d 100644 --- a/lib/consistency_validations.rb +++ b/lib/consistency_validations.rb @@ -16,4 +16,15 @@ module ConsistencyValidations raise OSM::APIChangesetAlreadyClosedError.new end end + + # This is similar to above, just some validations don't apply + def check_create_consistency(new, user) + if new.changeset.nil? + raise OSM::APIChangesetMissingError.new + elsif new.changeset.user_id != user.id + raise OSM::APIUserChangesetMismatchError.new + elsif not new.changeset.is_open? + raise OSM::APIChangesetAlreadyClosedError.new + end + end end -- 2.43.2