From cea403dce084a82c0bff017e7ca23526ff43d1c4 Mon Sep 17 00:00:00 2001 From: mmd-osm Date: Tue, 13 Feb 2018 23:07:30 +0100 Subject: [PATCH 1/1] Remove BBOX expansion performance optimization Closes #1749 Fixes #1742 --- app/models/changeset.rb | 13 +++---------- test/controllers/changeset_controller_test.rb | 8 +++----- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/app/models/changeset.rb b/app/models/changeset.rb index 2ec26ecee..502637c2a 100644 --- a/app/models/changeset.rb +++ b/app/models/changeset.rb @@ -55,9 +55,6 @@ class Changeset < ActiveRecord::Base before_save :update_closed_at - # over-expansion factor to use when updating the bounding box - EXPAND = 0.1 - # maximum number of elements allowed in a changeset MAX_ELEMENTS = 10000 @@ -127,12 +124,9 @@ class Changeset < ActiveRecord::Base end ## - # expand the bounding box to include the given bounding box. also, - # expand a little bit more in the direction of the expansion, so that - # further expansions may be unnecessary. this is an optimisation - # suggested on the wiki page by kleptog. + # expand the bounding box to include the given bounding box. def update_bbox!(bbox_update) - bbox.expand!(bbox_update, EXPAND) + bbox.expand!(bbox_update) # update active record. rails 2.1's dirty handling should take care of # whether this object needs saving or not. @@ -141,8 +135,7 @@ class Changeset < ActiveRecord::Base ## # the number of elements is also passed in so that we can ensure that - # a single changeset doesn't contain too many elements. this, of course, - # destroys the optimisation described in the bbox method above. + # a single changeset doesn't contain too many elements. def add_changes!(elements) self.num_changes += elements end diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index cb584759a..71fb71ca2 100644 --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@ -581,7 +581,6 @@ CHANGESET changeset_id = @response.body.to_i # upload some widely-spaced nodes, spiralling positive and negative to cause - # largest bbox over-expansion possible. diff = < @@ -619,7 +618,7 @@ CHANGESET assert cs.min_lon >= -180 * GeoRecord::SCALE, "Minimum longitude (#{cs.min_lon / GeoRecord::SCALE}) should be >= -180 to be valid." assert cs.max_lon <= 180 * GeoRecord::SCALE, "Maximum longitude (#{cs.max_lon / GeoRecord::SCALE}) should be <= 180 to be valid." assert cs.min_lat >= -90 * GeoRecord::SCALE, "Minimum latitude (#{cs.min_lat / GeoRecord::SCALE}) should be >= -90 to be valid." - assert cs.max_lat >= 90 * GeoRecord::SCALE, "Maximum latitude (#{cs.max_lat / GeoRecord::SCALE}) should be <= 90 to be valid." + assert cs.max_lat <= 90 * GeoRecord::SCALE, "Maximum latitude (#{cs.max_lat / GeoRecord::SCALE}) should be <= 90 to be valid." end ## @@ -1568,11 +1567,10 @@ CHANGESET # get the bounding box back from the changeset get :read, :params => { :id => changeset_id } assert_response :success, "Couldn't read back changeset for the third time." - # note that the 3.1 here is because of the bbox overexpansion assert_select "osm>changeset[min_lon='1.0000000']", 1 - assert_select "osm>changeset[max_lon='3.1000000']", 1 + assert_select "osm>changeset[max_lon='3.0000000']", 1 assert_select "osm>changeset[min_lat='1.0000000']", 1 - assert_select "osm>changeset[max_lat='3.1000000']", 1 + assert_select "osm>changeset[max_lat='3.0000000']", 1 end ## -- 2.39.5