From 135ec35a91bcca15637b6436dd7e546468c49690 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Thu, 22 Aug 2019 10:36:03 +0100 Subject: [PATCH] Fix numericality validation to use correct integer only constraint Fixes #2355 --- app/controllers/api/amf_controller.rb | 14 +++++++------- app/controllers/api/changesets_controller.rb | 8 ++++---- app/models/changeset.rb | 10 +++++----- app/models/changeset_comment.rb | 2 +- app/models/node.rb | 10 +++++----- app/models/note.rb | 2 +- app/models/note_comment.rb | 2 +- app/models/old_node.rb | 4 ++-- app/models/relation.rb | 6 +++--- app/models/way.rb | 6 +++--- test/controllers/api/changesets_controller_test.rb | 4 ++-- test/models/node_test.rb | 8 ++++---- 12 files changed, 38 insertions(+), 38 deletions(-) diff --git a/app/controllers/api/amf_controller.rb b/app/controllers/api/amf_controller.rb index ca46726a4..e5648989b 100644 --- a/app/controllers/api/amf_controller.rb +++ b/app/controllers/api/amf_controller.rb @@ -569,7 +569,7 @@ module Api new_relation.members = typedmembers new_relation.tags = tags new_relation.visible = visible - new_relation.changeset_id = changeset_id + new_relation.changeset_id = changeset_id.to_i new_relation.version = version if relid <= 0 @@ -653,7 +653,7 @@ module Api id = renumberednodes[id] if renumberednodes[id] node = Node.new - node.changeset_id = changeset_id + node.changeset_id = changeset_id.to_i node.lat = lat node.lon = lon node.tags = a[4] @@ -687,7 +687,7 @@ module Api new_way = Way.new new_way.tags = attributes new_way.nds = pointlist - new_way.changeset_id = changeset_id + new_way.changeset_id = changeset_id.to_i new_way.version = wayversion if originalway <= 0 new_way.create_with_history(user) @@ -705,7 +705,7 @@ module Api deletednodes.each do |id, v| node = Node.find(id.to_i) new_node = Node.new - new_node.changeset_id = changeset_id + new_node.changeset_id = changeset_id.to_i new_node.version = v.to_i new_node.id = id.to_i begin @@ -758,7 +758,7 @@ module Api # We always need a new node, based on the data that has been sent to us new_node = Node.new - new_node.changeset_id = changeset_id + new_node.changeset_id = changeset_id.to_i new_node.version = version new_node.lat = lat new_node.lon = lon @@ -834,7 +834,7 @@ module Api old_way = Way.find(way_id) delete_way = Way.new delete_way.version = way_version - delete_way.changeset_id = changeset_id + delete_way.changeset_id = changeset_id.to_i delete_way.id = way_id old_way.delete_with_history!(delete_way, user) @@ -843,7 +843,7 @@ module Api deletednodes.each do |id, v| node = Node.find(id.to_i) new_node = Node.new - new_node.changeset_id = changeset_id + new_node.changeset_id = changeset_id.to_i new_node.version = v.to_i new_node.id = id.to_i begin diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb index ffd9a112b..5f87324e0 100644 --- a/app/controllers/api/changesets_controller.rb +++ b/app/controllers/api/changesets_controller.rb @@ -93,10 +93,10 @@ module Api lat << cs.max_lat unless cs.max_lat.nil? # collapse the arrays to minimum and maximum - cs.min_lon = lon.min - cs.min_lat = lat.min - cs.max_lon = lon.max - cs.max_lat = lat.max + cs.min_lon = lon.min.round + cs.min_lat = lat.min.round + cs.max_lon = lon.max.round + cs.max_lat = lat.max.round # save the larger bounding box and return the changeset, which # will include the bigger bounding box. diff --git a/app/models/changeset.rb b/app/models/changeset.rb index d57086a8e..47f03c795 100644 --- a/app/models/changeset.rb +++ b/app/models/changeset.rb @@ -43,15 +43,15 @@ class Changeset < ActiveRecord::Base has_and_belongs_to_many :subscribers, :class_name => "User", :join_table => "changesets_subscribers", :association_foreign_key => "subscriber_id" validates :id, :uniqueness => true, :presence => { :on => :update }, - :numericality => { :on => :update, :integer_only => true } + :numericality => { :on => :update, :only_integer => true } validates :user_id, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :num_changes, :presence => true, - :numericality => { :integer_only => true, + :numericality => { :only_integer => true, :greater_than_or_equal_to => 0 } validates :created_at, :closed_at, :presence => true validates :min_lat, :max_lat, :min_lon, :max_lat, :allow_nil => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } before_save :update_closed_at @@ -131,7 +131,7 @@ class Changeset < ActiveRecord::Base # update active record. rails 2.1's dirty handling should take care of # whether this object needs saving or not. - self.min_lon, self.min_lat, self.max_lon, self.max_lat = @bbox.to_a if bbox.complete? + self.min_lon, self.min_lat, self.max_lon, self.max_lat = @bbox.to_a.collect(&:round) if bbox.complete? end ## diff --git a/app/models/changeset_comment.rb b/app/models/changeset_comment.rb index 529641c7e..75b1a055b 100644 --- a/app/models/changeset_comment.rb +++ b/app/models/changeset_comment.rb @@ -24,7 +24,7 @@ class ChangesetComment < ActiveRecord::Base belongs_to :author, :class_name => "User" validates :id, :uniqueness => true, :presence => { :on => :update }, - :numericality => { :on => :update, :integer_only => true } + :numericality => { :on => :update, :only_integer => true } validates :changeset, :presence => true, :associated => true validates :author, :presence => true, :associated => true validates :visible, :inclusion => [true, false] diff --git a/app/models/node.rb b/app/models/node.rb index 91a1dbc41..c5df1f79e 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -47,15 +47,15 @@ class Node < ActiveRecord::Base has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation validates :id, :uniqueness => true, :presence => { :on => :update }, - :numericality => { :on => :update, :integer_only => true } + :numericality => { :on => :update, :only_integer => true } validates :version, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :changeset_id, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :latitude, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :longitude, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :timestamp, :presence => true validates :changeset, :associated => true validates :visible, :inclusion => [true, false] diff --git a/app/models/note.rb b/app/models/note.rb index d4f9a801f..fa9d0b1ae 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -24,7 +24,7 @@ class Note < ActiveRecord::Base has_many :comments, -> { left_joins(:author).where(:visible => true, :users => { :status => [nil, "active", "confirmed"] }).order(:created_at) }, :class_name => "NoteComment", :foreign_key => :note_id validates :id, :uniqueness => true, :presence => { :on => :update }, - :numericality => { :on => :update, :integer_only => true } + :numericality => { :on => :update, :only_integer => true } validates :latitude, :longitude, :numericality => { :only_integer => true } validates :closed_at, :presence => true, :if => proc { :status == "closed" } validates :status, :inclusion => %w[open closed hidden] diff --git a/app/models/note_comment.rb b/app/models/note_comment.rb index 388f890a6..448703ffa 100644 --- a/app/models/note_comment.rb +++ b/app/models/note_comment.rb @@ -28,7 +28,7 @@ class NoteComment < ActiveRecord::Base belongs_to :author, :class_name => "User", :foreign_key => :author_id validates :id, :uniqueness => true, :presence => { :on => :update }, - :numericality => { :on => :update, :integer_only => true } + :numericality => { :on => :update, :only_integer => true } validates :note, :presence => true, :associated => true validates :visible, :inclusion => [true, false] validates :author, :associated => true diff --git a/app/models/old_node.rb b/app/models/old_node.rb index cc2327d08..bdf8cb47e 100644 --- a/app/models/old_node.rb +++ b/app/models/old_node.rb @@ -38,9 +38,9 @@ class OldNode < ActiveRecord::Base validates :changeset, :presence => true, :associated => true validates :latitude, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :longitude, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :timestamp, :presence => true validates :visible, :inclusion => [true, false] diff --git a/app/models/relation.rb b/app/models/relation.rb index bcac9d04b..dc2b71ac4 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -37,11 +37,11 @@ class Relation < ActiveRecord::Base has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation validates :id, :uniqueness => true, :presence => { :on => :update }, - :numericality => { :on => :update, :integer_only => true } + :numericality => { :on => :update, :only_integer => true } validates :version, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :changeset_id, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :timestamp, :presence => true validates :changeset, :associated => true validates :visible, :inclusion => [true, false] diff --git a/app/models/way.rb b/app/models/way.rb index 6fcaf39cc..b3466213c 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -39,11 +39,11 @@ class Way < ActiveRecord::Base has_many :containing_relations, :class_name => "Relation", :through => :containing_relation_members, :source => :relation validates :id, :uniqueness => true, :presence => { :on => :update }, - :numericality => { :on => :update, :integer_only => true } + :numericality => { :on => :update, :only_integer => true } validates :version, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :changeset_id, :presence => true, - :numericality => { :integer_only => true } + :numericality => { :only_integer => true } validates :timestamp, :presence => true validates :changeset, :associated => true validates :visible, :inclusion => [true, false] diff --git a/test/controllers/api/changesets_controller_test.rb b/test/controllers/api/changesets_controller_test.rb index 9f2b64c0a..3716ae707 100644 --- a/test/controllers/api/changesets_controller_test.rb +++ b/test/controllers/api/changesets_controller_test.rb @@ -1561,8 +1561,8 @@ CHANGESET user = create(:user) changeset = create(:changeset, :user => user) closed_changeset = create(:changeset, :closed, :user => user, :created_at => Time.utc(2008, 1, 1, 0, 0, 0), :closed_at => Time.utc(2008, 1, 2, 0, 0, 0)) - changeset2 = create(:changeset, :min_lat => 5 * GeoRecord::SCALE, :min_lon => 5 * GeoRecord::SCALE, :max_lat => 15 * GeoRecord::SCALE, :max_lon => 15 * GeoRecord::SCALE) - changeset3 = create(:changeset, :min_lat => 4.5 * GeoRecord::SCALE, :min_lon => 4.5 * GeoRecord::SCALE, :max_lat => 5 * GeoRecord::SCALE, :max_lon => 5 * GeoRecord::SCALE) + changeset2 = create(:changeset, :min_lat => (5 * GeoRecord::SCALE).round, :min_lon => (5 * GeoRecord::SCALE).round, :max_lat => (15 * GeoRecord::SCALE).round, :max_lon => (15 * GeoRecord::SCALE).round) + changeset3 = create(:changeset, :min_lat => (4.5 * GeoRecord::SCALE).round, :min_lon => (4.5 * GeoRecord::SCALE).round, :max_lat => (5 * GeoRecord::SCALE).round, :max_lon => (5 * GeoRecord::SCALE).round) get :query, :params => { :bbox => "-10,-10, 10, 10" } assert_response :success, "can't get changesets in bbox" diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 5f63251ee..d6fa1cd8f 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -80,8 +80,8 @@ class NodeTest < ActiveSupport::TestCase def test_create changeset = create(:changeset) node_template = Node.new( - :latitude => 12.3456, - :longitude => 65.4321, + :lat => 12.3456, + :lon => 65.4321, :changeset_id => changeset.id, :visible => 1, :version => 1 @@ -116,8 +116,8 @@ class NodeTest < ActiveSupport::TestCase assert_equal OldNode.where(:node_id => node_template.id).count, 1 assert_not_nil node - node_template.latitude = 12.3456 - node_template.longitude = 65.4321 + node_template.lat = 12.3456 + node_template.lon = 65.4321 # node_template.tags = "updated=yes" assert node.update_from(node_template, node.changeset.user) -- 2.43.2