Close a number of holes in the API by making it validate changes

more carefully.
This commit is contained in:
Tom Hughes 2007-06-19 23:20:39 +00:00
parent 7f2756fe20
commit c11d961f62
9 changed files with 250 additions and 194 deletions

View file

@ -4,16 +4,10 @@ class NodeController < ApplicationController
before_filter :authorize
after_filter :compress_output
def create
def create
response.headers["Content-Type"] = 'text/xml'
if request.put?
node = nil
begin
node = Node.from_xml(request.raw_post, true)
rescue
render :text => "XML didn't parse", :status => 400 # if we got here the doc didnt parse
return
end
node = Node.from_xml(request.raw_post, true)
if node
node.user_id = @user.id
@ -61,9 +55,13 @@ class NodeController < ApplicationController
when :delete
if node.visible
node.visible = 0
node.save_with_history
render :nothing => true
if Segment.find(:first, :conditions => [ "visible = 1 and (node_a = ? or node_b = ?)", node.id, node.id])
render :nothing => true, :status => HTTP_PRECONDITION_FAILED
else
node.visible = 0
node.save_with_history
render :nothing => true
end
else
render :nothing => true, :status => 410
end
@ -71,17 +69,21 @@ class NodeController < ApplicationController
when :put
new_node = Node.from_xml(request.raw_post)
node.timestamp = Time.now
node.user_id = @user.id
if new_node
node.timestamp = Time.now
node.user_id = @user.id
node.latitude = new_node.latitude
node.longitude = new_node.longitude
node.tags = new_node.tags
node.latitude = new_node.latitude
node.longitude = new_node.longitude
node.tags = new_node.tags
if node.id == new_node.id and node.save_with_history
render :nothing => true, :status => 200
if node.id == new_node.id and node.save_with_history
render :nothing => true
else
render :nothing => true, :status => 500
end
else
render :nothing => true, :status => 500
render :nothing => true, :status => 400 # if we got here the doc didnt parse
end
return
end

View file

@ -31,7 +31,6 @@ class SegmentController < ApplicationController
render :nothing => true, :status => 500
end
return
else
render :nothing => true, :status => 400 # if we got here the doc didnt parse
return
@ -58,9 +57,13 @@ class SegmentController < ApplicationController
when :delete
if segment.visible
segment.visible = 0
segment.save_with_history
render :nothing => true
if WaySegment.find(:first, :joins => "INNER JOIN current_ways ON current_ways.id = current_way_segments.id", :conditions => [ "current_ways.visible = 1 AND current_way_segments.segment_id = ?", segment.id ])
render :nothing => true, :status => HTTP_PRECONDITION_FAILED
else
segment.visible = 0
segment.save_with_history
render :nothing => true
end
else
render :nothing => true, :status => 410
end
@ -68,26 +71,32 @@ class SegmentController < ApplicationController
when :put
new_segment = Segment.from_xml(request.raw_post)
segment.timestamp = Time.now
segment.user_id = @user.id
if new_segment
if new_segment.node_a == new_segment.node_b
render :nothing => true, :status => HTTP_EXPECTATION_FAILED
return
end
unless new_segment.preconditions_ok? # are the nodes visible?
render :nothing => true, :status => HTTP_PRECONDITION_FAILED
return
end
if new_segment.node_a == new_segment.node_b
render :nothing => true, :status => HTTP_EXPECTATION_FAILED
return
end
segment.timestamp = Time.now
segment.user_id = @user.id
segment.node_a = new_segment.node_a
segment.node_b = new_segment.node_b
segment.tags = new_segment.tags
segment.visible = new_segment.visible
segment.node_a = new_segment.node_a
segment.node_b = new_segment.node_b
segment.tags = new_segment.tags
segment.visible = new_segment.visible
if segment.id == new_segment.id and segment.save_with_history
render :nothing => true, :status => HTTP_OK
if segment.id == new_segment.id and segment.save_with_history
render :nothing => true
else
render :nothing => true, :status => 500
end
else
render :nothing => true, :status => 500
render :nothing => true, :status => 400 # if we got here the doc didnt parse
end
return
end
end

View file

@ -11,17 +11,16 @@ class WayController < ApplicationController
if way
way.user_id = @user.id
unless way.preconditions_ok? # are the segments (and their nodes) visible?
render :nothing => true, :status => 412
render :nothing => true, :status => HTTP_PRECONDITION_FAILED
return
end
if way.save_with_history
render :text => way.id.to_s
return
else
render :nothing => true, :status => 500
return
end
return
else
@ -87,38 +86,36 @@ class WayController < ApplicationController
render :text => way.to_xml.to_s
when :delete
unless way.visible
if way.visible
way.visible = false
way.save_with_history
render :nothing => true
else
render :nothing => true, :status => 410
return
end
way.visible = false
way.save_with_history
render :nothing => true
return
when :put
way = Way.from_xml(request.raw_post)
new_way = Way.from_xml(request.raw_post)
if way
way_in_db = Way.find(way.id)
if way_in_db
way_in_db.user_id = @user.id
way_in_db.tags = way.tags
way_in_db.segs = way.segs
way_in_db.timestamp = way.timestamp
way_in_db.visible = true
if way_in_db.save_with_history
render :text => way.id
else
render :nothing => true, :status => 500
end
if new_way
unless new_way.preconditions_ok? # are the segments (and their nodes) visible?
render :nothing => true, :status => HTTP_PRECONDITION_FAILED
return
end
way.user_id = @user.id
way.tags = new_way.tags
way.segs = new_way.segs
way.timestamp = new_way.timestamp
way.visible = true
if way.id == new_way.id and way.save_with_history
render :nothing => true
else
render :nothing => true, :status => 404 # way doesn't exist yet
render :nothing => true, :status => 500
end
else
render :nothing => true, :status => 400 # if we got here the doc didnt parse
return
end
end
end

View file

@ -2,59 +2,69 @@ class Node < ActiveRecord::Base
require 'xml/libxml'
set_table_name 'current_nodes'
validates_numericality_of :latitude
validates_numericality_of :longitude
# FIXME validate lat and lon within the world
validates_presence_of :user_id, :timestamp
validates_inclusion_of :visible, :in => [ true, false ]
validates_numericality_of :latitude, :longitude
validate :validate_position
has_many :old_nodes, :foreign_key => :id
belongs_to :user
def validate_position
errors.add_to_base("Node is not in the world") unless in_world?
end
def in_world?
return false if self.latitude < -90 or self.latitude > 90
return false if self.longitude < -180 or self.longitude > 180
return true
end
def self.from_xml(xml, create=false)
p = XML::Parser.new
p.string = xml
doc = p.parse
begin
p = XML::Parser.new
p.string = xml
doc = p.parse
node = Node.new
node = Node.new
doc.find('//osm/node').each do |pt|
node.latitude = pt['lat'].to_f
node.longitude = pt['lon'].to_f
doc.find('//osm/node').each do |pt|
return nil unless node.in_world?
node.latitude = pt['lat'].to_f
node.longitude = pt['lon'].to_f
if node.latitude > 90 or node.latitude < -90 or node.longitude > 180 or node.longitude < -180
return nil
end
unless create
if pt['id'] != '0'
node.id = pt['id'].to_i
unless create
if pt['id'] != '0'
node.id = pt['id'].to_i
end
end
end
node.visible = pt['visible'] and pt['visible'] == 'true'
node.visible = pt['visible'] and pt['visible'] == 'true'
if create
node.timestamp = Time.now
else
if pt['timestamp']
node.timestamp = Time.parse(pt['timestamp'])
if create
node.timestamp = Time.now
else
if pt['timestamp']
node.timestamp = Time.parse(pt['timestamp'])
end
end
tags = []
pt.find('tag').each do |tag|
tags << [tag['k'],tag['v']]
end
tags = tags.collect { |k,v| "#{k}=#{v}" }.join(';')
tags = '' if tags.nil?
node.tags = tags
end
tags = []
pt.find('tag').each do |tag|
tags << [tag['k'],tag['v']]
end
tags = tags.collect { |k,v| "#{k}=#{v}" }.join(';')
tags = '' if tags.nil?
node.tags = tags
rescue
node = nil
end
return node
end
@ -62,12 +72,13 @@ class Node < ActiveRecord::Base
begin
Node.transaction do
self.timestamp = Time.now
self.save
self.save!
old_node = OldNode.from_node(self)
old_node.save
old_node.save!
end
return true
rescue Exception => ex
rescue
return nil
end
end

View file

@ -1,8 +1,23 @@
class OldNode < ActiveRecord::Base
set_table_name 'nodes'
validates_presence_of :user_id, :timestamp
validates_inclusion_of :visible, :in => [ true, false ]
validates_numericality_of :latitude, :longitude
validate :validate_position
belongs_to :user
def validate_position
errors.add_to_base("Node is not in the world") unless in_world?
end
def in_world?
return false if self.latitude < -90 or self.latitude > 90
return false if self.longitude < -180 or self.longitude > 180
return true
end
def self.from_node(node)
old_node = OldNode.new
old_node.latitude = node.latitude
@ -26,5 +41,4 @@ class OldNode < ActiveRecord::Base
el1['timestamp'] = self.timestamp.xmlschema
return el1
end
end

View file

@ -1,6 +1,10 @@
class OldSegment < ActiveRecord::Base
set_table_name 'segments'
validates_presence_of :user_id, :timestamp
validates_inclusion_of :visible, :in => [ true, false ]
validates_numericality_of :node_a, :node_b
belongs_to :user
def self.from_segment(segment)

View file

@ -13,7 +13,7 @@ class OldWay < ActiveRecord::Base
return old_way
end
def save_with_dependencies
def save_with_dependencies!
# dont touch this unless you really have figured out why it's called
# (Rails doesn't deal well with the old ways table (called 'ways') because
@ -21,7 +21,7 @@ class OldWay < ActiveRecord::Base
# id and get it back but we have that and we want to get the 'version' back
# we could add another column but thats a lot of data. No, set_primary_key
# doesn't work either.
save()
save!
clear_aggregation_cache
clear_association_cache
@attributes.update(OldWay.find(:first, :conditions => ['id = ? AND timestamp = ?', self.id, self.timestamp]).instance_variable_get('@attributes'))
@ -34,7 +34,7 @@ class OldWay < ActiveRecord::Base
tag.v = v
tag.id = self.id
tag.version = self.version
tag.save
tag.save!
end
i = 0
@ -43,7 +43,7 @@ class OldWay < ActiveRecord::Base
seg.id = self.id
seg.segment_id = n
seg.version = self.version
seg.save
seg.save!
end
end

View file

@ -2,8 +2,9 @@ class Segment < ActiveRecord::Base
require 'xml/libxml'
set_table_name 'current_segments'
validates_numericality_of :node_a
validates_numericality_of :node_b
validates_presence_of :user_id, :timestamp
validates_inclusion_of :visible, :in => [ true, false ]
validates_numericality_of :node_a, :node_b
has_many :old_segments, :foreign_key => :id
belongs_to :user
@ -13,43 +14,48 @@ class Segment < ActiveRecord::Base
belongs_to :to_node, :class_name => 'Node', :foreign_key => 'node_b'
def self.from_xml(xml, create=false)
p = XML::Parser.new
p.string = xml
doc = p.parse
begin
p = XML::Parser.new
p.string = xml
doc = p.parse
segment = Segment.new
segment = Segment.new
doc.find('//osm/segment').each do |pt|
doc.find('//osm/segment').each do |pt|
segment.node_a = pt['from'].to_i
segment.node_b = pt['to'].to_i
segment.node_a = pt['from'].to_i
segment.node_b = pt['to'].to_i
if pt['id'] != '0'
segment.id = pt['id'].to_i
end
segment.visible = true
if create
segment.timestamp = Time.now
else
if pt['timestamp']
segment.timestamp = Time.parse(pt['timestamp'])
unless create
if pt['id'] != '0'
segment.id = pt['id'].to_i
end
end
segment.visible = true
if create
segment.timestamp = Time.now
else
if pt['timestamp']
segment.timestamp = Time.parse(pt['timestamp'])
end
end
tags = []
pt.find('tag').each do |tag|
tags << [tag['k'],tag['v']]
end
tags = tags.collect { |k,v| "#{k}=#{v}" }.join(';')
tags = '' if tags.nil?
segment.tags = tags
end
tags = []
pt.find('tag').each do |tag|
tags << [tag['k'],tag['v']]
end
tags = tags.collect { |k,v| "#{k}=#{v}" }.join(';')
tags = '' if tags.nil?
segment.tags = tags
rescue
segment = nil
end
return segment
end
@ -57,12 +63,13 @@ class Segment < ActiveRecord::Base
begin
Segment.transaction do
self.timestamp = Time.now
self.save
self.save!
old_segment = OldSegment.from_segment(self)
old_segment.save
old_segment.save!
end
return true
rescue Exception => ex
rescue
return nil
end
end

View file

@ -11,34 +11,37 @@ class Way < ActiveRecord::Base
set_table_name 'current_ways'
def self.from_xml(xml, create=false)
p = XML::Parser.new
p.string = xml
doc = p.parse
begin
p = XML::Parser.new
p.string = xml
doc = p.parse
way = Way.new
way = Way.new
doc.find('//osm/way').each do |pt|
if !create and pt['id'] != '0'
way.id = pt['id'].to_i
end
doc.find('//osm/way').each do |pt|
if !create and pt['id'] != '0'
way.id = pt['id'].to_i
end
if create
way.timestamp = Time.now
way.visible = true
else
if pt['timestamp']
way.timestamp = Time.parse(pt['timestamp'])
if create
way.timestamp = Time.now
way.visible = true
else
if pt['timestamp']
way.timestamp = Time.parse(pt['timestamp'])
end
end
pt.find('tag').each do |tag|
way.add_tag_keyval(tag['k'], tag['v'])
end
pt.find('seg').each do |seg|
way.add_seg_num(seg['id'])
end
end
pt.find('tag').each do |tag|
way.add_tag_keyval(tag['k'], tag['v'])
end
pt.find('seg').each do |seg|
way.add_seg_num(seg['id'])
end
rescue
way = nil
end
return way
@ -140,38 +143,47 @@ class Way < ActiveRecord::Base
end
def save_with_history
t = Time.now
self.timestamp = t
self.save
begin
Way.transaction do
t = Time.now
self.timestamp = t
self.save!
WayTag.delete_all(['id = ?', self.id])
WayTag.delete_all(['id = ?', self.id])
self.tags.each do |k,v|
tag = WayTag.new
tag.k = k
tag.v = v
tag.id = self.id
tag.save
end
self.tags.each do |k,v|
tag = WayTag.new
tag.k = k
tag.v = v
tag.id = self.id
tag.save!
end
WaySegment.delete_all(['id = ?', self.id])
WaySegment.delete_all(['id = ?', self.id])
i = 0
self.segs.each do |n|
seg = WaySegment.new
seg.id = self.id
seg.segment_id = n
seg.sequence_id = i
seg.save
i += 1
end
i = 0
self.segs.each do |n|
seg = WaySegment.new
seg.id = self.id
seg.segment_id = n
seg.sequence_id = i
seg.save!
i += 1
end
old_way = OldWay.from_way(self)
old_way.timestamp = t
old_way.save_with_dependencies
old_way = OldWay.from_way(self)
old_way.timestamp = t
old_way.save_with_dependencies!
end
return true
rescue
return nil
end
end
def preconditions_ok?
return false if self.segs.empty?
self.segs.each do |n|
segment = Segment.find(n)
unless segment and segment.visible and segment.preconditions_ok?