fixes for site lat/long obfuscation closes #91

app/models/site.rb

@@ -15,6 +15,12 @@ class Site < ActiveRecord::Base
                     styles: {span4: '300x300#', span3: '220x220#', span2: '140x140#', span1: '60x60#', spanhalf: '30x30#'},
                     default_url: '/images/site/site_:style.png'
 
+  LATITUDE_MIN = -90
+  LATITUDE_MAX = 90
+  LONGITUDE_MIN = -180
+  LONGITUDE_MAX = 180
+
+  JITTER_RANGE = 0.0002
 
   # acts_as_paranoid
   # userstamp
@@ -25,9 +31,13 @@ class Site < ActiveRecord::Base
   # validations
   validates :name, presence: true, length: {minimum: 2}
   # between -90 and 90 degrees
-  validates :latitude, numericality: {only_integer: false, greater_than_or_equal_to: -90, less_than_or_equal_to: 90}, allow_nil: true
+  validates :latitude, numericality: {only_integer: false, greater_than_or_equal_to: Site::LATITUDE_MIN, less_than_or_equal_to: Site::LATITUDE_MAX,
+                                      message: "%{value} must be greater than or equal to #{Site::LATITUDE_MIN} and less than or equal to #{Site::LATITUDE_MAX}"}, allow_nil: true
+
   # -180 and 180 degrees
-  validates :longitude, numericality: {only_integer: false, greater_than_or_equal_to: -180, less_than_or_equal_to: 180}, allow_nil: true
+  validates :longitude, numericality: {only_integer: false, greater_than_or_equal_to: Site::LONGITUDE_MIN, less_than_or_equal_to: Site::LONGITUDE_MAX,
+                                       message: "%{value} must be greater than or equal to #{Site::LONGITUDE_MIN} and less than or equal to #{Site::LONGITUDE_MAX}"}, allow_nil: true
+
   #validates_as_paranoid
   validates_attachment_content_type :image, content_type: /^image\/(jpg|jpeg|pjpeg|png|x-png|gif)$/, message: 'file type %{value} is not allowed (only jpeg/png/gif images)'
 
@@ -43,7 +53,7 @@ def project_ids
   def latitude
     value = read_attribute(:latitude)
     if self.location_obfuscated && !value.blank?
-      add_jitter(value, -90, 90)
+      Site.add_location_jitter(value, Site::LATITUDE_MIN, Site::LATITUDE_MAX)
     else
       value
     end
@@ -52,7 +62,7 @@ def latitude
   def longitude
     value = read_attribute(:longitude)
     if self.location_obfuscated && !value.blank?
-      add_jitter(value, -180, 180)
+      Site.add_location_jitter(value, Site::LONGITUDE_MIN, Site::LONGITUDE_MAX)
     else
       value
     end
@@ -62,4 +72,43 @@ def update_location_obfuscated(current_user)
     highest_permission = current_user.highest_permission_any(self.projects)
     @location_obfuscated = highest_permission < AccessLevel::OWNER
   end
+
+  def self.add_location_jitter(value, min, max)
+    # truncate to 4 decimal places, then add random jitter
+    # that has been truncated to 5 decimal places
+    # http://en.wikipedia.org/wiki/Decimal_degrees#Precision
+    # add or subtract between ~4m - ~20m jitter
+
+    truncate_decimals_4 = 10000.0
+    truncated_value = (value * truncate_decimals_4).floor / truncate_decimals_4
+
+    truncate_decimals_5 = 100000.0
+    random_jitter = rand(-Site::JITTER_RANGE..Site::JITTER_RANGE)
+    truncated_jitter = (random_jitter * truncate_decimals_5).floor / truncate_decimals_5
+
+    modified_value = truncated_value + truncated_jitter
+
+    # ensure range is maintained (damn floating point in-exactness)
+    modified_value = modified_value.round(5)
+
+    # ensure range is maintained (damn floating point in-exactness)
+    if modified_value > (value + Site::JITTER_RANGE)
+      modified_value = value + Site::JITTER_RANGE
+    end
+
+    if modified_value < (value - Site::JITTER_RANGE)
+      modified_value = value - Site::JITTER_RANGE
+    end
+
+    # ensure modified value stays within lat/long valid ranges
+    if modified_value > max
+      modified_value = max
+    end
+
+    if modified_value < min
+      modified_value = min
+    end
+
+    modified_value
+  end
 end

spec/models/site_spec.rb

@@ -38,13 +38,11 @@
   end
 
   it 'should obfuscate lat/longs properly' do
-    min = -90
-    max = 90
 
-    1000.times {
-      result = add_jitter((rand * (max - -min)) + min, min, max)
-      expect(result).to be <= max
-      expect(result).to be >= min
+    10.times {
+      s = FactoryGirl.build(:site_with_lat_long)
+      expect(Site.add_location_jitter(s.longitude, Site::LONGITUDE_MIN, Site::LONGITUDE_MAX)).to be_within(Site::JITTER_RANGE).of(s.longitude)
+      expect(Site.add_location_jitter(s.latitude, Site::LATITUDE_MIN, Site::LATITUDE_MAX)).to be_within(Site::JITTER_RANGE).of(s.latitude)
     }
   end