diff --git a/app/jobs/overland/batch_creating_job.rb b/app/jobs/overland/batch_creating_job.rb index 944a72e2..7d1f0832 100644 --- a/app/jobs/overland/batch_creating_job.rb +++ b/app/jobs/overland/batch_creating_job.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Overland::BatchCreatingJob < ApplicationJob + include PointValidation + queue_as :default def perform(params, user_id) @@ -12,14 +14,4 @@ class Overland::BatchCreatingJob < ApplicationJob Point.create!(location.merge(user_id:)) end end - - private - - def point_exists?(params, user_id) - Point.exists?( - lonlat: "POINT(#{params[:longitude]} #{params[:latitude]})", - timestamp: params[:timestamp], - user_id: - ) - end end diff --git a/app/jobs/owntracks/point_creating_job.rb b/app/jobs/owntracks/point_creating_job.rb index 86f6670d..9dfcc83e 100644 --- a/app/jobs/owntracks/point_creating_job.rb +++ b/app/jobs/owntracks/point_creating_job.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Owntracks::PointCreatingJob < ApplicationJob + include PointValidation + queue_as :default def perform(point_params, user_id) @@ -10,12 +12,4 @@ class Owntracks::PointCreatingJob < ApplicationJob Point.create!(parsed_params.merge(user_id:)) end - - def point_exists?(params, user_id) - Point.exists?( - lonlat: "POINT(#{params[:longitude]} #{params[:latitude]})", - timestamp: params[:timestamp], - user_id: - ) - end end diff --git a/app/models/concerns/point_validation.rb b/app/models/concerns/point_validation.rb new file mode 100644 index 00000000..09cda18e --- /dev/null +++ b/app/models/concerns/point_validation.rb @@ -0,0 +1,20 @@ +module PointValidation + extend ActiveSupport::Concern + + # Check if a point with the same coordinates, timestamp, and user_id already exists + def point_exists?(params, user_id) + # Ensure the coordinates are valid + longitude = params[:longitude].to_f + latitude = params[:latitude].to_f + + # Check if longitude and latitude are valid values + return false if longitude.zero? && latitude.zero? + return false if longitude.abs > 180 || latitude.abs > 90 + + # Use where with parameter binding and then exists? + Point.where( + 'ST_SetSRID(ST_MakePoint(?, ?), 4326) = lonlat AND timestamp = ? AND user_id = ?', + longitude, latitude, params[:timestamp].to_i, user_id + ).exists? + end +end diff --git a/app/services/points/params.rb b/app/services/points/params.rb index fc2348dc..7a9c81f6 100644 --- a/app/services/points/params.rb +++ b/app/services/points/params.rb @@ -14,7 +14,7 @@ class Points::Params next unless params_valid?(point) { - lonlat: "POINT(#{point[:geometry][:coordinates][0]} #{point[:geometry][:coordinates][1]})", + lonlat: lonlat(point), battery_status: point[:properties][:battery_state], battery: battery_level(point[:properties][:battery_level]), timestamp: DateTime.parse(point[:properties][:timestamp]), @@ -45,4 +45,8 @@ class Points::Params point[:geometry][:coordinates].present? && point.dig(:properties, :timestamp).present? end + + def lonlat(point) + "POINT(#{point[:geometry][:coordinates][0]} #{point[:geometry][:coordinates][1]})" + end end diff --git a/app/services/visits/calculate.rb b/app/services/visits/calculate.rb deleted file mode 100644 index 0f0a4c06..00000000 --- a/app/services/visits/calculate.rb +++ /dev/null @@ -1,92 +0,0 @@ -# frozen_string_literal: true - -class Visits::Calculate - def initialize(points) - @points = points - end - - def call - # Only one visit per city per day - normalized_visits.flat_map do |country| - { - country: country[:country], - cities: country[:cities].uniq { [_1[:city], Time.zone.at(_1[:timestamp]).to_date] } - } - end - end - - def normalized_visits - normalize_result(city_visits) - end - - private - - attr_reader :points - - def group_points - points.sort_by(&:timestamp).reject { _1.city.nil? }.group_by(&:country) - end - - def city_visits - group_points.transform_values do |grouped_points| - grouped_points - .group_by(&:city) - .transform_values { |city_points| identify_consecutive_visits(city_points) } - end - end - - def identify_consecutive_visits(city_points) - visits = [] - current_visit = [] - - city_points.each_cons(2) do |point1, point2| - time_diff = (point2.timestamp - point1.timestamp) / 60 - - if time_diff <= MIN_MINUTES_SPENT_IN_CITY - current_visit << point1 unless current_visit.include?(point1) - current_visit << point2 - else - visits << create_visit(current_visit) if current_visit.size > 1 - current_visit = [] - end - end - - visits << create_visit(current_visit) if current_visit.size > 1 - visits - end - - def create_visit(points) - { - city: points.first.city, - points:, - stayed_for: calculate_stayed_time(points), - last_timestamp: points.last.timestamp - } - end - - def calculate_stayed_time(points) - return 0 if points.empty? - - min_time = points.first.timestamp - max_time = points.last.timestamp - ((max_time - min_time) / 60).round - end - - def normalize_result(hash) - hash.map do |country, cities| - { - country:, - cities: cities.values.flatten - .select { |visit| visit[:stayed_for] >= MIN_MINUTES_SPENT_IN_CITY } - .map do |visit| - { - city: visit[:city], - points: visit[:points].count, - timestamp: visit[:last_timestamp], - stayed_for: visit[:stayed_for] - } - end - } - end.reject { |entry| entry[:cities].empty? } - end -end diff --git a/app/services/visits/group_points.rb b/app/services/visits/group_points.rb deleted file mode 100644 index 00961c16..00000000 --- a/app/services/visits/group_points.rb +++ /dev/null @@ -1,58 +0,0 @@ -# frozen_string_literal: true - -class Visits::GroupPoints - INITIAL_RADIUS = 30 # meters - MAX_RADIUS = 100 # meters - RADIUS_STEP = 10 # meters - MIN_VISIT_DURATION = 3 * 60 # 3 minutes in seconds - - attr_reader :day_points, :initial_radius, :max_radius, :step - - def initialize(day_points, initial_radius = INITIAL_RADIUS, max_radius = MAX_RADIUS, step = RADIUS_STEP) - @day_points = day_points - @initial_radius = initial_radius - @max_radius = max_radius - @step = step - end - - def group_points_by_radius - grouped = [] - remaining_points = day_points.dup - - while remaining_points.any? - point = remaining_points.shift - radius = initial_radius - - while radius <= max_radius - new_group = [point] - - remaining_points.each do |next_point| - break unless within_radius?(new_group.first, next_point, radius) - - new_group << next_point - end - - if new_group.size > 1 - group_duration = new_group.last.timestamp - new_group.first.timestamp - - if group_duration >= MIN_VISIT_DURATION - remaining_points -= new_group - grouped << new_group - end - - break - else - radius += step - end - end - end - - grouped - end - - private - - def within_radius?(point1, point2, radius) - point1.distance_to(point2) * 1000 <= radius - end -end diff --git a/app/services/visits/prepare.rb b/app/services/visits/prepare.rb deleted file mode 100644 index 4e4c56ce..00000000 --- a/app/services/visits/prepare.rb +++ /dev/null @@ -1,79 +0,0 @@ -# frozen_string_literal: true - -class Visits::Prepare - attr_reader :points - - def initialize(points) - @points = points - end - - def call - points_by_day = points.group_by { |point| point_date(point) } - - points_by_day.map do |day, day_points| - day_points.sort_by!(&:timestamp) - - grouped_points = Visits::GroupPoints.new(day_points).group_points_by_radius - day_result = prepare_day_result(grouped_points) - - # Iterate through the day_result, check if there are any points outside - # of visits that are between two consecutive visits. If there are none, - # merge the visits. - - day_result.each_cons(2) do |visit1, visit2| - next if visit1[:points].last == visit2[:points].first - - points_between_visits = day_points.select do |point| - point.timestamp > visit1[:points].last.timestamp && - point.timestamp < visit2[:points].first.timestamp - end - - if points_between_visits.any? - # If there are points between the visits, we need to check if they are close enough to the visits to be considered part of them. - - points_between_visits.each do |point| - next unless visit1[:points].last.distance_to(point) < visit1[:radius] || - visit2[:points].first.distance_to(point) < visit2[:radius] || - (point.timestamp - visit1[:points].last.timestamp).to_i < 600 - - visit1[:points] << point - end - end - - visit1[:points] += visit2[:points] - visit1[:duration] = (visit1[:points].last.timestamp - visit1[:points].first.timestamp).to_i / 60 - visit1[:ended_at] = Time.zone.at(visit1[:points].last.timestamp) - day_result.delete(visit2) - end - - next if day_result.blank? - - { date: day, visits: day_result } - end.compact - end - - private - - def point_date(point) = Time.zone.at(point.timestamp).to_date.to_s - - def calculate_radius(center_point, group) - max_distance = group.map { |point| center_point.distance_to(point) }.max - - (max_distance / 10.0).ceil * 10 - end - - def prepare_day_result(grouped_points) - grouped_points.map do |group| - center_point = group.first - - { - lonlat: "POINT(#{center_point.lon} #{center_point.lat})", - radius: calculate_radius(center_point, group), - points: group, - duration: (group.last.timestamp - group.first.timestamp).to_i / 60, - started_at: Time.zone.at(group.first.timestamp).to_s, - ended_at: Time.zone.at(group.last.timestamp).to_s - } - end - end -end diff --git a/spec/factories/places.rb b/spec/factories/places.rb index df790846..cdef9618 100644 --- a/spec/factories/places.rb +++ b/spec/factories/places.rb @@ -5,5 +5,6 @@ FactoryBot.define do name { 'MyString' } latitude { 1.5 } longitude { 1.5 } + lonlat { "POINT(#{longitude} #{latitude})" } end end diff --git a/spec/models/place_spec.rb b/spec/models/place_spec.rb index 48722d9c..3fdf3cac 100644 --- a/spec/models/place_spec.rb +++ b/spec/models/place_spec.rb @@ -11,8 +11,7 @@ RSpec.describe Place, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:name) } - it { is_expected.to validate_presence_of(:latitude) } - it { is_expected.to validate_presence_of(:longitude) } + it { is_expected.to validate_presence_of(:lonlat) } end describe 'enums' do diff --git a/spec/services/points/params_spec.rb b/spec/services/points/params_spec.rb index 62f9b82b..43e09829 100644 --- a/spec/services/points/params_spec.rb +++ b/spec/services/points/params_spec.rb @@ -10,8 +10,7 @@ RSpec.describe Points::Params do let(:json) { JSON.parse(file.read) } let(:expected_json) do { - latitude: 37.74430413, - longitude: -122.40530871, + lonlat: 'POINT(-122.40530871 37.74430413)', battery_status: nil, battery: nil, timestamp: DateTime.parse('2025-01-17T21:03:01Z'), diff --git a/spec/services/visits/calculate_spec.rb b/spec/services/visits/calculate_spec.rb deleted file mode 100644 index 23da0b41..00000000 --- a/spec/services/visits/calculate_spec.rb +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Visits::Calculate do - describe '#call' do - end -end diff --git a/spec/services/visits/group_points_spec.rb b/spec/services/visits/group_points_spec.rb deleted file mode 100644 index f672ddee..00000000 --- a/spec/services/visits/group_points_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Visits::GroupPoints do - describe '#group_points_by_radius' do - it 'groups points by radius' do - day_points = [ - build(:point, lonlat: 'POINT(0 0)', timestamp: 1.day.ago), - build(:point, lonlat: 'POINT(0.00001 0.00001)', timestamp: 1.day.ago + 1.minute), - build(:point, lonlat: 'POINT(0.00002 0.00002)', timestamp: 1.day.ago + 2.minutes), - build(:point, lonlat: 'POINT(0.00003 0.00003)', timestamp: 1.day.ago + 3.minutes), - build(:point, lonlat: 'POINT(0.00004 0.00004)', timestamp: 1.day.ago + 4.minutes), - build(:point, lonlat: 'POINT(0.00005 0.00005)', timestamp: 1.day.ago + 5.minutes), - build(:point, lonlat: 'POINT(0.00006 0.00006)', timestamp: 1.day.ago + 6.minutes), - build(:point, lonlat: 'POINT(0.00007 0.00007)', timestamp: 1.day.ago + 7.minutes), - build(:point, lonlat: 'POINT(0.00008 0.00008)', timestamp: 1.day.ago + 8.minutes), - build(:point, lonlat: 'POINT(0.00009 0.00009)', timestamp: 1.day.ago + 9.minutes), - build(:point, lonlat: 'POINT(0.001 0.001)', timestamp: 1.day.ago + 10.minutes) - ] - - grouped_points = described_class.new(day_points).group_points_by_radius - - expect(grouped_points.size).to eq(1) - expect(grouped_points.first.size).to eq(10) - # The last point is too far from the first point - expect(grouped_points.first).not_to include(day_points.last) - end - end -end diff --git a/spec/services/visits/prepare_spec.rb b/spec/services/visits/prepare_spec.rb deleted file mode 100644 index 39935bd1..00000000 --- a/spec/services/visits/prepare_spec.rb +++ /dev/null @@ -1,49 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Visits::Prepare do - describe '#call' do - let(:static_time) { Time.zone.local(2021, 1, 1, 0, 0, 0) } - let(:points) do - [ - build(:point, lonlat: 'POINT(0 0)', timestamp: static_time), - build(:point, lonlat: 'POINT(0.00001 0.00001)', timestamp: static_time + 5.minutes), - build(:point, lonlat: 'POINT(0.00002 0.00002)', timestamp: static_time + 10.minutes), - build(:point, lonlat: 'POINT(0.00003 0.00003)', timestamp: static_time + 15.minutes), - build(:point, lonlat: 'POINT(0.00004 0.00004)', timestamp: static_time + 20.minutes), - build(:point, lonlat: 'POINT(0.00005 0.00005)', timestamp: static_time + 25.minutes), - build(:point, lonlat: 'POINT(0.00006 0.00006)', timestamp: static_time + 30.minutes), - build(:point, lonlat: 'POINT(0.00007 0.00007)', timestamp: static_time + 35.minutes), - build(:point, lonlat: 'POINT(0.00008 0.00008)', timestamp: static_time + 40.minutes), - build(:point, lonlat: 'POINT(0.00009 0.00009)', timestamp: static_time + 45.minutes), - build(:point, lonlat: 'POINT(0.0001 0.0001)', timestamp: static_time + 50.minutes), - build(:point, lonlat: 'POINT(0.00011 0.00011)', timestamp: static_time + 55.minutes), - build(:point, lonlat: 'POINT(0.00011 0.00011)', timestamp: static_time + 95.minutes), - build(:point, lonlat: 'POINT(0.00011 0.00011)', timestamp: static_time + 100.minutes), - build(:point, lonlat: 'POINT(0.00011 0.00011)', timestamp: static_time + 105.minutes) - ] - end - - subject { described_class.new(points).call } - - it 'returns correct visits' do - expect(subject).to eq [ - { - date: static_time.to_date.to_s, - visits: [ - { - latitude: 0.0, - longitude: 0.0, - radius: 10, - points:, - duration: 105, - started_at: static_time.to_s, - ended_at: (static_time + 105.minutes).to_s - } - ] - } - ] - end - end -end diff --git a/spec/services/visits/smart_detect_spec.rb b/spec/services/visits/smart_detect_spec.rb index f8bf2f5c..23d7f018 100644 --- a/spec/services/visits/smart_detect_spec.rb +++ b/spec/services/visits/smart_detect_spec.rb @@ -4,8 +4,8 @@ require 'rails_helper' RSpec.describe Visits::SmartDetect do let(:user) { create(:user) } - let(:start_at) { 1.day.ago } - let(:end_at) { Time.current } + let(:start_at) { DateTime.new(2025, 3, 1, 12, 0, 0) } + let(:end_at) { DateTime.new(2025, 3, 1, 13, 0, 0) } subject(:detector) { described_class.new(user, start_at:, end_at:) } @@ -19,9 +19,9 @@ RSpec.describe Visits::SmartDetect do context 'with a simple visit' do let!(:points) do [ - create(:point, user:, lonlat: 'POINT(0 0)', timestamp: 1.hour.ago), - create(:point, user:, lonlat: 'POINT(0.00001 0.00001)', timestamp: 50.minutes.ago), - create(:point, user:, lonlat: 'POINT(0.00002 0.00002)', timestamp: 40.minutes.ago) + create(:point, user:, lonlat: 'POINT(0 0)', timestamp: start_at), + create(:point, user:, lonlat: 'POINT(0.00001 0.00001)', timestamp: start_at + 5.minutes), + create(:point, user:, lonlat: 'POINT(0.00002 0.00002)', timestamp: start_at + 10.minutes) ] end diff --git a/spec/services/visits/suggest_spec.rb b/spec/services/visits/suggest_spec.rb index 31d76b2e..14e7c1dd 100644 --- a/spec/services/visits/suggest_spec.rb +++ b/spec/services/visits/suggest_spec.rb @@ -5,11 +5,12 @@ require 'rails_helper' RSpec.describe Visits::Suggest do describe '#call' do let!(:user) { create(:user) } - let(:start_at) { Time.new(2020, 1, 1, 0, 0, 0) } - let(:end_at) { Time.new(2020, 1, 1, 2, 0, 0) } + let(:start_at) { Time.zone.local(2020, 1, 1, 0, 0, 0) } + let(:end_at) { Time.zone.local(2020, 1, 1, 2, 0, 0) } let!(:points) do [ + # first visit create(:point, :with_known_location, user:, timestamp: start_at), create(:point, :with_known_location, user:, timestamp: start_at + 5.minutes), create(:point, :with_known_location, user:, timestamp: start_at + 10.minutes), @@ -22,20 +23,73 @@ RSpec.describe Visits::Suggest do create(:point, :with_known_location, user:, timestamp: start_at + 45.minutes), create(:point, :with_known_location, user:, timestamp: start_at + 50.minutes), create(:point, :with_known_location, user:, timestamp: start_at + 55.minutes), + # end of first visit + # second visit create(:point, :with_known_location, user:, timestamp: start_at + 95.minutes), create(:point, :with_known_location, user:, timestamp: start_at + 100.minutes), create(:point, :with_known_location, user:, timestamp: start_at + 105.minutes) + # end of second visit ] end + let(:geocoder_struct) do + Struct.new(:data) do + def data + { + "features": [ + { + "geometry": { + "coordinates": [ + 37.6175406, + 55.7559395 + ], + "type": 'Point' + }, + "type": 'Feature', + "properties": { + "osm_id": 681_354_082, + "extent": [ + 37.6175406, + 55.7559395, + 37.6177036, + 55.755847 + ], + "country": 'Russia', + "city": 'Moscow', + "countrycode": 'RU', + "postcode": '103265', + "type": 'street', + "osm_type": 'W', + "osm_key": 'highway', + "district": 'Tverskoy', + "osm_value": 'pedestrian', + "name": 'проезд Воскресенские Ворота', + "state": 'Moscow' + } + } + ], + "type": 'FeatureCollection' + } + end + end + end + + let(:geocoder_response) do + [geocoder_struct.new] + end + subject { described_class.new(user, start_at:, end_at:).call } + before do + allow(Geocoder).to receive(:search).and_return(geocoder_response) + end + it 'creates places' do expect { subject }.to change(Place, :count).by(1) end it 'creates visits' do - expect { subject }.to change(Visit, :count).by(1) + expect { subject }.to change(Visit, :count).by(2) end it 'creates visits notification' do @@ -48,9 +102,7 @@ RSpec.describe Visits::Suggest do end it 'reverse geocodes visits' do - expect_any_instance_of(Visit).to receive(:async_reverse_geocode).and_call_original - - subject + expect { subject }.to have_enqueued_job(ReverseGeocodingJob).exactly(2).times end end