Skip to content

Commit

Permalink
Add new data policy fields
Browse files Browse the repository at this point in the history
- enable_forwarding allows forwarding to be enabled or disabled per
  device

- precise_location allows users to choose whether precise (5dp) or
  approxmite (3dp + 2dp of noise) locations are stored.

On deploying we will need to run migrations, then run rake
devices:truncate_and_fuzz_locations in order to truncate the existing
locations to 5dp.
  • Loading branch information
timcowlishaw committed Jun 24, 2024
1 parent 8249a4d commit e171952
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 8 deletions.
26 changes: 25 additions & 1 deletion app/models/device.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class Device < ActiveRecord::Base

default_scope { with_active_state }

include ActiveModel::Dirty
include Workflow
include WorkflowActiverecord
include ArchiveWorkflow
Expand Down Expand Up @@ -37,6 +38,7 @@ class Device < ActiveRecord::Base
with: /\A([0-9a-fA-F]{2}[:-]){5}[0-9a-fA-F]{2}\z/, allow_nil: true

before_save :nullify_other_mac_addresses, if: :mac_address
before_save :truncate_and_fuzz_location!, if: :location_changed?
before_save :calculate_geohash
after_validation :do_geocoding

Expand Down Expand Up @@ -245,6 +247,14 @@ def update_component_timestamps(timestamp, sensor_ids)
end
end

def data_policy(authorized=false)
{
is_private: authorized ? is_private : "[FILTERED]",
enable_forwarding: authorized ? enable_forwarding : "[FILTERED]",
precise_location: authorized ? precise_location : "[FILTERED]"
}
end

def hardware(authorized=false)
{
name: hardware_name,
Expand Down Expand Up @@ -272,19 +282,33 @@ def hardware_slug
end

def forward_readings?
owner.forward_device_readings?
enable_forwarding && owner.forward_device_readings?
end

def forwarding_token
owner.forwarding_token
end

def truncate_and_fuzz_location!
if latitude && longitude
decimal_places = self.precise_location ? 5 : 3
lat_fuzz = self.precise_location ? 0.0 : (Random.rand * 1/10.0**decimal_places).truncate(5)
long_fuzz = self.precise_location ? 0.0 : (Random.rand * 1/10.0**decimal_places).truncate(5)
self.latitude = self.latitude.truncate(decimal_places) + lat_fuzz
self.longitude = self.longitude.truncate(decimal_places) + long_fuzz
end
end

private

def set_state
self.state = self.soft_state
end

def location_changed?
latitude_changed? || longitude_changed? || precise_location_changed?
end

def calculate_geohash
# include ActiveModel::Dirty
# if latitude.changed? or longitude.changed?
Expand Down
2 changes: 1 addition & 1 deletion app/views/v0/devices/_device.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ json.(
:state,
:system_tags,
:user_tags,
:is_private,
:last_reading_at,
:created_at,
:updated_at
Expand All @@ -35,6 +34,7 @@ else
end
json.merge!(postprocessing: device.postprocessing) if local_assigns[:with_postprocessing]
json.merge!(location: device.formatted_location) if local_assigns[:with_location]
json.merge!(data_policy: device.data_policy(authorized))
json.merge!(hardware: device.hardware(authorized))

if local_assigns[:with_owner] && device.owner
Expand Down
8 changes: 8 additions & 0 deletions db/migrate/20240624161155_add_data_policy_fields_to_device.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class AddDataPolicyFieldsToDevice < ActiveRecord::Migration[6.1]
def change
add_column :devices, :precise_location, :boolean, null: false, default: false
add_column :devices, :enable_forwarding, :boolean, null: false, default: false
# Existing devices have precise locations, despite the default for all new ones.
execute "UPDATE devices SET precise_location = true"
end
end
4 changes: 3 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2024_05_12_132257) do
ActiveRecord::Schema.define(version: 2024_06_24_161155) do

# These are extensions that must be enabled in order to support this database
enable_extension "adminpack"
Expand Down Expand Up @@ -105,6 +105,8 @@
t.string "hardware_name_override"
t.string "hardware_version_override"
t.string "hardware_slug_override"
t.boolean "precise_location", default: false, null: false
t.boolean "enable_forwarding", default: false, null: false
t.index ["device_token"], name: "index_devices_on_device_token", unique: true
t.index ["geohash"], name: "index_devices_on_geohash"
t.index ["last_reading_at"], name: "index_devices_on_last_reading_at"
Expand Down
8 changes: 8 additions & 0 deletions lib/tasks/devices.rake
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace :devices do
task :truncate_and_fuzz_locations => :environment do
Device.all.each do |device|
device.truncate_and_fuzz_location!
device.save!(validate: false)
end
end
end
103 changes: 99 additions & 4 deletions spec/models/device_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@

it "calculates geohash on save" do
barcelona = create(:device)
expect(barcelona.geohash).to match('sp3e9bh31y')
expect(barcelona.geohash).to match(/^sp3e/)
end

skip "calculates elevation on save", :vcr do
Expand All @@ -203,6 +203,60 @@

end

describe "location truncation and fuzzing" do
context "when the location is changed" do
context "when the device has precise location set" do
let(:device) {
create(:device, precise_location: true)

}

it "truncates the location to 5 decimal places" do
device.update(latitude: 0.1234567, longitude: 10.1234567)
device.save
expect(device.latitude).to eq(0.12345)
expect(device.longitude).to eq(10.12345)
end
end

context "when the device has no precise location set" do
let(:device) {
create(:device, precise_location: false)

}


it "truncates the location to 3 decimal places and adds 2 extra dp of randomness" do
device.update(latitude: 0.12345, longitude: 10.12345)
device.save
expect(device.latitude).not_to eq(0.12345)
expect(device.longitude).not_to eq(10.12345)
expect((device.latitude - 0.12345).abs).to be <= 0.001
expect((device.longitude - 10.12345).abs).to be <= 0.001
end
end
end

context "when the location is not changed" do
let(:device) {
create(:device, precise_location: false, latitude: 0.12345, longitude: 0.12345)

}

it "does not re-fuzz the device location" do
lat_before = device.latitude
long_before = device.longitude

device.update(name: "not updating location")
device.save!

expect(device.reload.latitude).to eq(lat_before)
expect(device.reload.longitude).to eq(long_before)
end
end
end


it "has to_s" do
device = create(:device, name: 'cool device')
expect(device.to_s).to eq('cool device')
Expand Down Expand Up @@ -524,10 +578,51 @@

describe "forwarding" do
describe "#forward_readings?" do
context "when the device has forwarding enabled" do
let(:device) {
create(:device, enable_forwarding: true)
}

context "when forward_device_readings is true on the owning user" do
it "forwardds readings" do
expect(device.owner).to receive(:forward_device_readings?).and_return(true)
expect(device.forward_readings?).to be(true)
end
end

context "when forward_device_readings is not true on the owning user" do
it "does not forward readings" do
expect(device.owner).to receive(:forward_device_readings?).and_return(false)
expect(device.forward_readings?).to be(false)
end
end
end

context "when the device does not have forwarding enabled" do
let(:device) {
create(:device, enable_forwarding: false)
}

context "when forward_device_readings is true on the owning user" do
it "does not forward readings" do
allow(device.owner).to receive(:forward_device_readings?).and_return(true)
expect(device.forward_readings?).to be(false)
expect(device.owner).not_to have_received(:forward_device_readings?)
end
end

context "when forward_device_readings is not true on the owning user" do
it "does not forward readings" do
allow(device.owner).to receive(:forward_device_readings?).and_return(false)
expect(device.forward_readings?).to be(false)
expect(device.owner).not_to have_received(:forward_device_readings?)
end
end


end

it "delegates to the forward_device_readings? method on the device owner" do
forward_readings = double(:forward_readings)
expect(device.owner).to receive(:forward_device_readings?).and_return(forward_readings)
expect(device.forward_readings?).to eq(forward_readings)
end
end

Expand Down
24 changes: 23 additions & 1 deletion spec/requests/v0/devices_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
expect(json.length).to eq(2)
# expect(json[0]['name']).to eq(first.name)
# expect(json[1]['name']).to eq(second.name)
expect(json[0].keys).to eq(%w(id uuid name description state system_tags user_tags is_private last_reading_at created_at updated_at notify device_token postprocessing location hardware owner data))
expect(json[0].keys).to eq(%w(id uuid name description state system_tags user_tags last_reading_at created_at updated_at notify device_token postprocessing location data_policy hardware owner data))
end

describe "when not logged in" do
Expand All @@ -46,6 +46,13 @@
json = api_get 'devices'
expect(json[0]['hardware']['last_status_message']).to eq("[FILTERED]")
end

it "does not show data policies" do
first = create(:device)
second = create(:device)
json = api_get 'devices'
expect(json[0]['data_policy']['enable_forwarding']).to eq("[FILTERED]")
end
end

describe "when logged in as a normal user" do
Expand All @@ -67,6 +74,14 @@
json = api_get 'devices', { access_token: token.token }
expect(json[0]['hardware']['last_status_message']).to eq("[FILTERED]")
end

it "only shows device policies for the owning user" do
first = create(:device)
second = create(:device, owner: user)
json = api_get 'devices', { access_token: token.token}
expect(json[0]['data_policy']['enable_forwarding']).to eq('[FILTERED]')
expect(json[1]['data_policy']['enable_forwarding']).not_to eq('[FILTERED]')
end
end

describe "when logged in as an admin" do
Expand All @@ -88,6 +103,13 @@
json = api_get 'devices', { access_token: admin_token.token}
expect(json[0]['hardware']['last_status_message']).not_to eq('[FILTERED]')
end

it "shows device policies" do
first = create(:device)
second = create(:device)
json = api_get 'devices', { access_token: admin_token.token}
expect(json[0]['data_policy']['enable_forwarding']).not_to eq('[FILTERED]')
end
end

describe "world map" do
Expand Down

0 comments on commit e171952

Please sign in to comment.