From 418df71c53ae468575a4c13fb2618368086cb627 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sat, 12 Jul 2025 22:04:14 +0200 Subject: [PATCH 01/30] Fixes for bulk creating job --- app/jobs/tracks/bulk_creating_job.rb | 33 ++++- spec/jobs/tracks/bulk_creating_job_spec.rb | 151 ++++++++++++++++++--- 2 files changed, 156 insertions(+), 28 deletions(-) diff --git a/app/jobs/tracks/bulk_creating_job.rb b/app/jobs/tracks/bulk_creating_job.rb index f2bafdc8..cbeb0a55 100644 --- a/app/jobs/tracks/bulk_creating_job.rb +++ b/app/jobs/tracks/bulk_creating_job.rb @@ -1,27 +1,48 @@ # frozen_string_literal: true -# This job is being run on daily basis to create tracks for all users -# for the past 24 hours. +# This job is being run on daily basis to create tracks for all users. +# For each user, it starts from the end of their last track (or from their oldest point +# if no tracks exist) and processes points until the specified end_at time. # # To manually run for a specific time range: # Tracks::BulkCreatingJob.perform_later(start_at: 1.week.ago, end_at: Time.current) # # To run for specific users only: # Tracks::BulkCreatingJob.perform_later(user_ids: [1, 2, 3]) +# +# To let the job determine start times automatically (recommended): +# Tracks::BulkCreatingJob.perform_later(end_at: Time.current) class Tracks::BulkCreatingJob < ApplicationJob queue_as :tracks sidekiq_options retry: false - def perform(start_at: 1.day.ago.beginning_of_day, end_at: 1.day.ago.end_of_day, user_ids: []) + def perform(start_at: nil, end_at: 1.day.ago.end_of_day, user_ids: []) users = user_ids.any? ? User.active.where(id: user_ids) : User.active - start_at = start_at.to_datetime end_at = end_at.to_datetime users.find_each do |user| next if user.tracked_points.empty? - next unless user.tracked_points.where(timestamp: start_at.to_i..end_at.to_i).exists? - Tracks::CreateJob.perform_later(user.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + # Start from the end of the last track, or from the beginning if no tracks exist + user_start_at = start_at&.to_datetime || start_time(user) + + next unless user.tracked_points.where(timestamp: user_start_at.to_i..end_at.to_i).exists? + + Tracks::CreateJob.perform_later(user.id, start_at: user_start_at, end_at: end_at, cleaning_strategy: :daily) + end + end + + private + + def start_time(user) + # Find the latest track for this user + latest_track = user.tracks.order(end_at: :desc).first + + if latest_track + latest_track.end_at + else + oldest_point = user.tracked_points.order(:timestamp).first + oldest_point ? Time.zone.at(oldest_point.timestamp) : 1.day.ago.beginning_of_day end end end diff --git a/spec/jobs/tracks/bulk_creating_job_spec.rb b/spec/jobs/tracks/bulk_creating_job_spec.rb index b40f5d43..016146f8 100644 --- a/spec/jobs/tracks/bulk_creating_job_spec.rb +++ b/spec/jobs/tracks/bulk_creating_job_spec.rb @@ -20,26 +20,28 @@ RSpec.describe Tracks::BulkCreatingJob, type: :job do create(:point, user: inactive_user, timestamp: start_at.to_i + 1.hour.to_i) end - it 'schedules tracks creation jobs for active users with points in the timeframe' do - expect { - described_class.new.perform(start_at: start_at, end_at: end_at) - }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) - end + context 'when explicit start_at is provided' do + it 'schedules tracks creation jobs for active users with points in the timeframe' do + expect { + described_class.new.perform(start_at: start_at, end_at: end_at) + }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + end - it 'does not schedule jobs for users without tracked points' do - expect { - described_class.new.perform(start_at: start_at, end_at: end_at) - }.not_to have_enqueued_job(Tracks::CreateJob).with(user_without_points.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) - end + it 'does not schedule jobs for users without tracked points' do + expect { + described_class.new.perform(start_at: start_at, end_at: end_at) + }.not_to have_enqueued_job(Tracks::CreateJob).with(user_without_points.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + end - it 'does not schedule jobs for users without points in the specified timeframe' do - # Create a user with points outside the timeframe - user_with_old_points = create(:user) - create(:point, user: user_with_old_points, timestamp: 2.days.ago.to_i) + it 'does not schedule jobs for users without points in the specified timeframe' do + # Create a user with points outside the timeframe + user_with_old_points = create(:user) + create(:point, user: user_with_old_points, timestamp: 2.days.ago.to_i) - expect { - described_class.new.perform(start_at: start_at, end_at: end_at) - }.not_to have_enqueued_job(Tracks::CreateJob).with(user_with_old_points.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + expect { + described_class.new.perform(start_at: start_at, end_at: end_at) + }.not_to have_enqueued_job(Tracks::CreateJob).with(user_with_old_points.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) + end end context 'when specific user_ids are provided' do @@ -56,17 +58,122 @@ RSpec.describe Tracks::BulkCreatingJob, type: :job do end end - context 'with default parameters' do - it 'uses yesterday as the default timeframe' do + context 'with automatic start time determination' do + let(:user_with_tracks) { create(:user) } + let(:user_without_tracks) { create(:user) } + let(:current_time) { Time.current } + + before do + # Create some historical points and tracks for user_with_tracks + create(:point, user: user_with_tracks, timestamp: 3.days.ago.to_i) + create(:point, user: user_with_tracks, timestamp: 2.days.ago.to_i) + + # Create a track ending 1 day ago + create(:track, user: user_with_tracks, end_at: 1.day.ago) + + # Create newer points after the last track + create(:point, user: user_with_tracks, timestamp: 12.hours.ago.to_i) + create(:point, user: user_with_tracks, timestamp: 6.hours.ago.to_i) + + # Create points for user without tracks + create(:point, user: user_without_tracks, timestamp: 2.days.ago.to_i) + create(:point, user: user_without_tracks, timestamp: 1.day.ago.to_i) + end + + it 'starts from the end of the last track for users with existing tracks' do + track_end_time = user_with_tracks.tracks.order(end_at: :desc).first.end_at + expect { - described_class.new.perform + described_class.new.perform(end_at: current_time, user_ids: [user_with_tracks.id]) }.to have_enqueued_job(Tracks::CreateJob).with( - active_user.id, - start_at: 1.day.ago.beginning_of_day.to_datetime, + user_with_tracks.id, + start_at: track_end_time, + end_at: current_time.to_datetime, + cleaning_strategy: :daily + ) + end + + it 'starts from the oldest point for users without tracks' do + oldest_point_time = Time.zone.at(user_without_tracks.tracked_points.order(:timestamp).first.timestamp) + + expect { + described_class.new.perform(end_at: current_time, user_ids: [user_without_tracks.id]) + }.to have_enqueued_job(Tracks::CreateJob).with( + user_without_tracks.id, + start_at: oldest_point_time, + end_at: current_time.to_datetime, + cleaning_strategy: :daily + ) + end + + it 'falls back to 1 day ago for users with no points' do + expect { + described_class.new.perform(end_at: current_time) + }.not_to have_enqueued_job(Tracks::CreateJob).with( + user_without_points.id, + start_at: anything, + end_at: anything, + cleaning_strategy: :daily + ) + end + end + + context 'with default parameters' do + let(:user_with_recent_points) { create(:user) } + + before do + # Create points within yesterday's timeframe + create(:point, user: user_with_recent_points, timestamp: 1.day.ago.beginning_of_day.to_i + 2.hours.to_i) + create(:point, user: user_with_recent_points, timestamp: 1.day.ago.beginning_of_day.to_i + 6.hours.to_i) + end + + it 'uses automatic start time determination with yesterday as end_at' do + oldest_point_time = Time.zone.at(user_with_recent_points.tracked_points.order(:timestamp).first.timestamp) + + expect { + described_class.new.perform(user_ids: [user_with_recent_points.id]) + }.to have_enqueued_job(Tracks::CreateJob).with( + user_with_recent_points.id, + start_at: oldest_point_time, end_at: 1.day.ago.end_of_day.to_datetime, cleaning_strategy: :daily ) end end end + + describe '#start_time' do + let(:user) { create(:user) } + let(:job) { described_class.new } + + context 'when user has tracks' do + let!(:old_track) { create(:track, user: user, end_at: 3.days.ago) } + let!(:recent_track) { create(:track, user: user, end_at: 1.day.ago) } + + it 'returns the end time of the most recent track' do + result = job.send(:start_time, user) + + expect(result).to eq(recent_track.end_at) + end + end + + context 'when user has no tracks but has points' do + let!(:old_point) { create(:point, user: user, timestamp: 5.days.ago.to_i) } + let!(:recent_point) { create(:point, user: user, timestamp: 2.days.ago.to_i) } + + it 'returns the timestamp of the oldest point' do + result = job.send(:start_time, user) + + expect(result).to eq(Time.zone.at(old_point.timestamp)) + end + end + + context 'when user has no tracks and no points' do + it 'returns 1 day ago beginning of day' do + result = job.send(:start_time, user) + + expect(result).to eq(1.day.ago.beginning_of_day) + end + end + end end From 244fb2b192e6d5b1035d8eeb6bfc267760c9aab1 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sat, 12 Jul 2025 23:04:15 +0200 Subject: [PATCH 02/30] Move bulk track creation to service --- app/jobs/tracks/bulk_creating_job.rb | 28 +-- app/services/tracks/bulk_track_creator.rb | 39 ++++ spec/jobs/tracks/bulk_creating_job_spec.rb | 170 +---------------- .../tracks/bulk_track_creator_spec.rb | 176 ++++++++++++++++++ 4 files changed, 221 insertions(+), 192 deletions(-) create mode 100644 app/services/tracks/bulk_track_creator.rb create mode 100644 spec/services/tracks/bulk_track_creator_spec.rb diff --git a/app/jobs/tracks/bulk_creating_job.rb b/app/jobs/tracks/bulk_creating_job.rb index cbeb0a55..71ae15dc 100644 --- a/app/jobs/tracks/bulk_creating_job.rb +++ b/app/jobs/tracks/bulk_creating_job.rb @@ -17,32 +17,6 @@ class Tracks::BulkCreatingJob < ApplicationJob sidekiq_options retry: false def perform(start_at: nil, end_at: 1.day.ago.end_of_day, user_ids: []) - users = user_ids.any? ? User.active.where(id: user_ids) : User.active - end_at = end_at.to_datetime - - users.find_each do |user| - next if user.tracked_points.empty? - - # Start from the end of the last track, or from the beginning if no tracks exist - user_start_at = start_at&.to_datetime || start_time(user) - - next unless user.tracked_points.where(timestamp: user_start_at.to_i..end_at.to_i).exists? - - Tracks::CreateJob.perform_later(user.id, start_at: user_start_at, end_at: end_at, cleaning_strategy: :daily) - end - end - - private - - def start_time(user) - # Find the latest track for this user - latest_track = user.tracks.order(end_at: :desc).first - - if latest_track - latest_track.end_at - else - oldest_point = user.tracked_points.order(:timestamp).first - oldest_point ? Time.zone.at(oldest_point.timestamp) : 1.day.ago.beginning_of_day - end + Tracks::BulkTrackCreator.new(start_at:, end_at:, user_ids:).call end end diff --git a/app/services/tracks/bulk_track_creator.rb b/app/services/tracks/bulk_track_creator.rb new file mode 100644 index 00000000..f7eaf301 --- /dev/null +++ b/app/services/tracks/bulk_track_creator.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Tracks + class BulkTrackCreator + def initialize(start_at: nil, end_at: 1.day.ago.end_of_day, user_ids: []) + @start_at = start_at + @end_at = end_at.to_datetime + @user_ids = user_ids + end + + def call + users.find_each do |user| + next if user.tracked_points.empty? + + user_start_at = @start_at&.to_datetime || start_time(user) + + next unless user.tracked_points.where(timestamp: user_start_at.to_i..@end_at.to_i).exists? + + Tracks::CreateJob.perform_later(user.id, start_at: user_start_at, end_at: @end_at, cleaning_strategy: :daily) + end + end + + private + + def users + @user_ids.any? ? User.active.where(id: @user_ids) : User.active + end + + def start_time(user) + latest_track = user.tracks.order(end_at: :desc).first + if latest_track + latest_track.end_at + else + oldest_point = user.tracked_points.order(:timestamp).first + oldest_point ? Time.zone.at(oldest_point.timestamp) : 1.day.ago.beginning_of_day + end + end + end +end diff --git a/spec/jobs/tracks/bulk_creating_job_spec.rb b/spec/jobs/tracks/bulk_creating_job_spec.rb index 016146f8..47844452 100644 --- a/spec/jobs/tracks/bulk_creating_job_spec.rb +++ b/spec/jobs/tracks/bulk_creating_job_spec.rb @@ -4,176 +4,16 @@ require 'rails_helper' RSpec.describe Tracks::BulkCreatingJob, type: :job do describe '#perform' do - let!(:active_user) { create(:user) } - let!(:inactive_user) { create(:user, :inactive) } - let!(:user_without_points) { create(:user) } - - let(:start_at) { 1.day.ago.beginning_of_day } - let(:end_at) { 1.day.ago.end_of_day } + let(:service) { instance_double(Tracks::BulkTrackCreator) } before do - # Create points for active user in the target timeframe - create(:point, user: active_user, timestamp: start_at.to_i + 1.hour.to_i) - create(:point, user: active_user, timestamp: start_at.to_i + 2.hours.to_i) - - # Create points for inactive user in the target timeframe - create(:point, user: inactive_user, timestamp: start_at.to_i + 1.hour.to_i) + allow(Tracks::BulkTrackCreator).to receive(:new).with(start_at: 'foo', end_at: 'bar', user_ids: [1, 2]).and_return(service) end - context 'when explicit start_at is provided' do - it 'schedules tracks creation jobs for active users with points in the timeframe' do - expect { - described_class.new.perform(start_at: start_at, end_at: end_at) - }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) - end + it 'calls Tracks::BulkTrackCreator with the correct arguments' do + expect(service).to receive(:call) - it 'does not schedule jobs for users without tracked points' do - expect { - described_class.new.perform(start_at: start_at, end_at: end_at) - }.not_to have_enqueued_job(Tracks::CreateJob).with(user_without_points.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) - end - - it 'does not schedule jobs for users without points in the specified timeframe' do - # Create a user with points outside the timeframe - user_with_old_points = create(:user) - create(:point, user: user_with_old_points, timestamp: 2.days.ago.to_i) - - expect { - described_class.new.perform(start_at: start_at, end_at: end_at) - }.not_to have_enqueued_job(Tracks::CreateJob).with(user_with_old_points.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) - end - end - - context 'when specific user_ids are provided' do - it 'only processes the specified users' do - expect { - described_class.new.perform(start_at: start_at, end_at: end_at, user_ids: [active_user.id]) - }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) - end - - it 'does not process users not in the user_ids list' do - expect { - described_class.new.perform(start_at: start_at, end_at: end_at, user_ids: [active_user.id]) - }.not_to have_enqueued_job(Tracks::CreateJob).with(inactive_user.id, start_at: start_at, end_at: end_at, cleaning_strategy: :daily) - end - end - - context 'with automatic start time determination' do - let(:user_with_tracks) { create(:user) } - let(:user_without_tracks) { create(:user) } - let(:current_time) { Time.current } - - before do - # Create some historical points and tracks for user_with_tracks - create(:point, user: user_with_tracks, timestamp: 3.days.ago.to_i) - create(:point, user: user_with_tracks, timestamp: 2.days.ago.to_i) - - # Create a track ending 1 day ago - create(:track, user: user_with_tracks, end_at: 1.day.ago) - - # Create newer points after the last track - create(:point, user: user_with_tracks, timestamp: 12.hours.ago.to_i) - create(:point, user: user_with_tracks, timestamp: 6.hours.ago.to_i) - - # Create points for user without tracks - create(:point, user: user_without_tracks, timestamp: 2.days.ago.to_i) - create(:point, user: user_without_tracks, timestamp: 1.day.ago.to_i) - end - - it 'starts from the end of the last track for users with existing tracks' do - track_end_time = user_with_tracks.tracks.order(end_at: :desc).first.end_at - - expect { - described_class.new.perform(end_at: current_time, user_ids: [user_with_tracks.id]) - }.to have_enqueued_job(Tracks::CreateJob).with( - user_with_tracks.id, - start_at: track_end_time, - end_at: current_time.to_datetime, - cleaning_strategy: :daily - ) - end - - it 'starts from the oldest point for users without tracks' do - oldest_point_time = Time.zone.at(user_without_tracks.tracked_points.order(:timestamp).first.timestamp) - - expect { - described_class.new.perform(end_at: current_time, user_ids: [user_without_tracks.id]) - }.to have_enqueued_job(Tracks::CreateJob).with( - user_without_tracks.id, - start_at: oldest_point_time, - end_at: current_time.to_datetime, - cleaning_strategy: :daily - ) - end - - it 'falls back to 1 day ago for users with no points' do - expect { - described_class.new.perform(end_at: current_time) - }.not_to have_enqueued_job(Tracks::CreateJob).with( - user_without_points.id, - start_at: anything, - end_at: anything, - cleaning_strategy: :daily - ) - end - end - - context 'with default parameters' do - let(:user_with_recent_points) { create(:user) } - - before do - # Create points within yesterday's timeframe - create(:point, user: user_with_recent_points, timestamp: 1.day.ago.beginning_of_day.to_i + 2.hours.to_i) - create(:point, user: user_with_recent_points, timestamp: 1.day.ago.beginning_of_day.to_i + 6.hours.to_i) - end - - it 'uses automatic start time determination with yesterday as end_at' do - oldest_point_time = Time.zone.at(user_with_recent_points.tracked_points.order(:timestamp).first.timestamp) - - expect { - described_class.new.perform(user_ids: [user_with_recent_points.id]) - }.to have_enqueued_job(Tracks::CreateJob).with( - user_with_recent_points.id, - start_at: oldest_point_time, - end_at: 1.day.ago.end_of_day.to_datetime, - cleaning_strategy: :daily - ) - end - end - end - - describe '#start_time' do - let(:user) { create(:user) } - let(:job) { described_class.new } - - context 'when user has tracks' do - let!(:old_track) { create(:track, user: user, end_at: 3.days.ago) } - let!(:recent_track) { create(:track, user: user, end_at: 1.day.ago) } - - it 'returns the end time of the most recent track' do - result = job.send(:start_time, user) - - expect(result).to eq(recent_track.end_at) - end - end - - context 'when user has no tracks but has points' do - let!(:old_point) { create(:point, user: user, timestamp: 5.days.ago.to_i) } - let!(:recent_point) { create(:point, user: user, timestamp: 2.days.ago.to_i) } - - it 'returns the timestamp of the oldest point' do - result = job.send(:start_time, user) - - expect(result).to eq(Time.zone.at(old_point.timestamp)) - end - end - - context 'when user has no tracks and no points' do - it 'returns 1 day ago beginning of day' do - result = job.send(:start_time, user) - - expect(result).to eq(1.day.ago.beginning_of_day) - end + described_class.new.perform(start_at: 'foo', end_at: 'bar', user_ids: [1, 2]) end end end diff --git a/spec/services/tracks/bulk_track_creator_spec.rb b/spec/services/tracks/bulk_track_creator_spec.rb new file mode 100644 index 00000000..88594ee2 --- /dev/null +++ b/spec/services/tracks/bulk_track_creator_spec.rb @@ -0,0 +1,176 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Tracks::BulkTrackCreator do + describe '#call' do + let!(:active_user) { create(:user) } + let!(:inactive_user) { create(:user, :inactive) } + let!(:user_without_points) { create(:user) } + + let(:start_at) { 1.day.ago.beginning_of_day } + let(:end_at) { 1.day.ago.end_of_day } + + before do + # Create points for active user in the target timeframe + create(:point, user: active_user, timestamp: start_at.to_i + 1.hour.to_i) + create(:point, user: active_user, timestamp: start_at.to_i + 2.hours.to_i) + + # Create points for inactive user in the target timeframe + create(:point, user: inactive_user, timestamp: start_at.to_i + 1.hour.to_i) + end + + context 'when explicit start_at is provided' do + it 'schedules tracks creation jobs for active users with points in the timeframe' do + expect { + described_class.new(start_at:, end_at:).call + }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at:, end_at:, cleaning_strategy: :daily) + end + + it 'does not schedule jobs for users without tracked points' do + expect { + described_class.new(start_at:, end_at:).call + }.not_to have_enqueued_job(Tracks::CreateJob).with(user_without_points.id, start_at:, end_at:, cleaning_strategy: :daily) + end + + it 'does not schedule jobs for users without points in the specified timeframe' do + # Create a user with points outside the timeframe + user_with_old_points = create(:user) + create(:point, user: user_with_old_points, timestamp: 2.days.ago.to_i) + + expect { + described_class.new(start_at:, end_at:).call + }.not_to have_enqueued_job(Tracks::CreateJob).with(user_with_old_points.id, start_at:, end_at:, cleaning_strategy: :daily) + end + end + + context 'when specific user_ids are provided' do + it 'only processes the specified users' do + expect { + described_class.new(start_at:, end_at:, user_ids: [active_user.id]).call + }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at:, end_at:, cleaning_strategy: :daily) + end + + it 'does not process users not in the user_ids list' do + expect { + described_class.new(start_at:, end_at:, user_ids: [active_user.id]).call + }.not_to have_enqueued_job(Tracks::CreateJob).with(inactive_user.id, start_at:, end_at:, cleaning_strategy: :daily) + end + end + + context 'with automatic start time determination' do + let(:user_with_tracks) { create(:user) } + let(:user_without_tracks) { create(:user) } + let(:current_time) { Time.current } + + before do + # Create some historical points and tracks for user_with_tracks + create(:point, user: user_with_tracks, timestamp: 3.days.ago.to_i) + create(:point, user: user_with_tracks, timestamp: 2.days.ago.to_i) + + # Create a track ending 1 day ago + create(:track, user: user_with_tracks, end_at: 1.day.ago) + + # Create newer points after the last track + create(:point, user: user_with_tracks, timestamp: 12.hours.ago.to_i) + create(:point, user: user_with_tracks, timestamp: 6.hours.ago.to_i) + + # Create points for user without tracks + create(:point, user: user_without_tracks, timestamp: 2.days.ago.to_i) + create(:point, user: user_without_tracks, timestamp: 1.day.ago.to_i) + end + + it 'starts from the end of the last track for users with existing tracks' do + track_end_time = user_with_tracks.tracks.order(end_at: :desc).first.end_at + + expect { + described_class.new(end_at: current_time, user_ids: [user_with_tracks.id]).call + }.to have_enqueued_job(Tracks::CreateJob).with( + user_with_tracks.id, + start_at: track_end_time, + end_at: current_time.to_datetime, + cleaning_strategy: :daily + ) + end + + it 'starts from the oldest point for users without tracks' do + oldest_point_time = Time.zone.at(user_without_tracks.tracked_points.order(:timestamp).first.timestamp) + + expect { + described_class.new(end_at: current_time, user_ids: [user_without_tracks.id]).call + }.to have_enqueued_job(Tracks::CreateJob).with( + user_without_tracks.id, + start_at: oldest_point_time, + end_at: current_time.to_datetime, + cleaning_strategy: :daily + ) + end + + it 'falls back to 1 day ago for users with no points' do + expect { + described_class.new(end_at: current_time, user_ids: [user_without_points.id]).call + }.not_to have_enqueued_job(Tracks::CreateJob).with( + user_without_points.id, + start_at: anything, + end_at: anything, + cleaning_strategy: :daily + ) + end + end + + context 'with default parameters' do + let(:user_with_recent_points) { create(:user) } + + before do + # Create points within yesterday's timeframe + create(:point, user: user_with_recent_points, timestamp: 1.day.ago.beginning_of_day.to_i + 2.hours.to_i) + create(:point, user: user_with_recent_points, timestamp: 1.day.ago.beginning_of_day.to_i + 6.hours.to_i) + end + + it 'uses automatic start time determination with yesterday as end_at' do + oldest_point_time = Time.zone.at(user_with_recent_points.tracked_points.order(:timestamp).first.timestamp) + + expect { + described_class.new(user_ids: [user_with_recent_points.id]).call + }.to have_enqueued_job(Tracks::CreateJob).with( + user_with_recent_points.id, + start_at: oldest_point_time, + end_at: 1.day.ago.end_of_day.to_datetime, + cleaning_strategy: :daily + ) + end + end + end + + describe '#start_time' do + let(:user) { create(:user) } + let(:service) { described_class.new } + + context 'when user has tracks' do + let!(:old_track) { create(:track, user: user, end_at: 3.days.ago) } + let!(:recent_track) { create(:track, user: user, end_at: 1.day.ago) } + + it 'returns the end time of the most recent track' do + result = service.send(:start_time, user) + expect(result).to eq(recent_track.end_at) + end + end + + context 'when user has no tracks but has points' do + let!(:old_point) { create(:point, user: user, timestamp: 5.days.ago.to_i) } + let!(:recent_point) { create(:point, user: user, timestamp: 2.days.ago.to_i) } + + it 'returns the timestamp of the oldest point' do + result = service.send(:start_time, user) + expect(result).to eq(Time.zone.at(old_point.timestamp)) + end + end + + context 'when user has no tracks and no points' do + it 'returns 1 day ago beginning of day' do + result = service.send(:start_time, user) + expect(result).to eq(1.day.ago.beginning_of_day) + end + end + end +end From 788537499318bcd8c8440519e140e7cadb268906 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sat, 12 Jul 2025 23:45:43 +0200 Subject: [PATCH 03/30] Refactor Tracks::BulkTrackCreator to use start_at and end_at as datetime objects --- app/services/tracks/bulk_track_creator.rb | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/app/services/tracks/bulk_track_creator.rb b/app/services/tracks/bulk_track_creator.rb index f7eaf301..7dba8506 100644 --- a/app/services/tracks/bulk_track_creator.rb +++ b/app/services/tracks/bulk_track_creator.rb @@ -3,8 +3,8 @@ module Tracks class BulkTrackCreator def initialize(start_at: nil, end_at: 1.day.ago.end_of_day, user_ids: []) - @start_at = start_at - @end_at = end_at.to_datetime + @start_at = start_at&.to_datetime + @end_at = end_at&.to_datetime @user_ids = user_ids end @@ -12,22 +12,30 @@ module Tracks users.find_each do |user| next if user.tracked_points.empty? - user_start_at = @start_at&.to_datetime || start_time(user) + user_start_at = start_at || start_time(user) - next unless user.tracked_points.where(timestamp: user_start_at.to_i..@end_at.to_i).exists? + next unless user.tracked_points.where(timestamp: user_start_at.to_i..end_at.to_i).exists? - Tracks::CreateJob.perform_later(user.id, start_at: user_start_at, end_at: @end_at, cleaning_strategy: :daily) + Tracks::CreateJob.perform_later( + user.id, + start_at: user_start_at, + end_at:, + cleaning_strategy: :daily + ) end end private + attr_reader :start_at, :end_at, :user_ids + def users - @user_ids.any? ? User.active.where(id: @user_ids) : User.active + user_ids.any? ? User.active.where(id: user_ids) : User.active end def start_time(user) latest_track = user.tracks.order(end_at: :desc).first + if latest_track latest_track.end_at else From 24378b150d61d9b431ad7c986b9b914c50cb2bca Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 13 Jul 2025 12:50:24 +0200 Subject: [PATCH 04/30] Add user serializer and update CHANGELOG.md --- CHANGELOG.md | 33 ++++++++ app/controllers/api/v1/users_controller.rb | 2 +- app/serializers/api/user_serializer.rb | 44 ++++++++++ app/services/users/safe_settings.rb | 12 ++- spec/requests/api/v1/users_spec.rb | 22 ++++- spec/serializers/api/user_serializer_spec.rb | 85 ++++++++++++++++++++ 6 files changed, 193 insertions(+), 5 deletions(-) create mode 100644 app/serializers/api/user_serializer.rb create mode 100644 spec/serializers/api/user_serializer_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index a495db16..93771c6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,39 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Notification about Photon API load is now disabled. - All distance values are now stored in the database in meters. Conversion to user's preferred unit is done on the fly. - Every night, Dawarich will try to fetch names for places and visits that don't have them. #1281 #902 #583 #212 +- User settings are now being serialized in a more consistent way. `GET /api/v1/users/me` now returns the following data structure: +```json +{ + "user": { + "email": "test@example.com", + "theme": "light", + "created_at": "2025-01-01T00:00:00Z", + "updated_at": "2025-01-01T00:00:00Z", + "settings": { + "maps": { + "url": "https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png", + "name": "Custom OpenStreetMap", + "distance_unit": "km" + }, + "fog_of_war_meters": 51, + "meters_between_routes": 500, + "preferred_map_layer": "Light", + "speed_colored_routes": false, + "points_rendering_mode": "raw", + "minutes_between_routes": 30, + "time_threshold_minutes": 30, + "merge_threshold_minutes": 15, + "live_map_enabled": false, + "route_opacity": 0.3, + "immich_url": "https://persistence-test-1752264458724.com", + "photoprism_url": "", + "visits_suggestions_enabled": true, + "speed_color_scale": "0:#00ff00|15:#00ffff|30:#ff00ff|50:#ffff00|100:#ff3300", + "fog_of_war_threshold": 5 + } + } +} +``` ## Fixed diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index 4fbb3f60..810eb55a 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -2,6 +2,6 @@ class Api::V1::UsersController < ApiController def me - render json: { user: current_api_user } + render json: Api::UserSerializer.new(current_api_user).call end end diff --git a/app/serializers/api/user_serializer.rb b/app/serializers/api/user_serializer.rb new file mode 100644 index 00000000..d3e89dfe --- /dev/null +++ b/app/serializers/api/user_serializer.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +class Api::UserSerializer + def initialize(user) + @user = user + end + + def call + { + user: { + email: user.email, + theme: user.theme, + created_at: user.created_at, + updated_at: user.updated_at, + settings: settings, + } + } + end + + private + + attr_reader :user + + def settings + { + maps: user.safe_settings.maps, + fog_of_war_meters: user.safe_settings.fog_of_war_meters.to_i, + meters_between_routes: user.safe_settings.meters_between_routes.to_i, + preferred_map_layer: user.safe_settings.preferred_map_layer, + speed_colored_routes: user.safe_settings.speed_colored_routes, + points_rendering_mode: user.safe_settings.points_rendering_mode, + minutes_between_routes: user.safe_settings.minutes_between_routes.to_i, + time_threshold_minutes: user.safe_settings.time_threshold_minutes.to_i, + merge_threshold_minutes: user.safe_settings.merge_threshold_minutes.to_i, + live_map_enabled: user.safe_settings.live_map_enabled, + route_opacity: user.safe_settings.route_opacity.to_f, + immich_url: user.safe_settings.immich_url, + photoprism_url: user.safe_settings.photoprism_url, + visits_suggestions_enabled: user.safe_settings.visits_suggestions_enabled?, + speed_color_scale: user.safe_settings.speed_color_scale, + fog_of_war_threshold: user.safe_settings.fog_of_war_threshold + } + end +end diff --git a/app/services/users/safe_settings.rb b/app/services/users/safe_settings.rb index 47548983..308121e5 100644 --- a/app/services/users/safe_settings.rb +++ b/app/services/users/safe_settings.rb @@ -45,7 +45,9 @@ class Users::SafeSettings photoprism_api_key: photoprism_api_key, maps: maps, distance_unit: distance_unit, - visits_suggestions_enabled: visits_suggestions_enabled? + visits_suggestions_enabled: visits_suggestions_enabled?, + speed_color_scale: speed_color_scale, + fog_of_war_threshold: fog_of_war_threshold } end # rubocop:enable Metrics/MethodLength @@ -118,4 +120,12 @@ class Users::SafeSettings def visits_suggestions_enabled? settings['visits_suggestions_enabled'] == 'true' end + + def speed_color_scale + settings['speed_color_scale'] + end + + def fog_of_war_threshold + settings['fog_of_war_threshold'] + end end diff --git a/spec/requests/api/v1/users_spec.rb b/spec/requests/api/v1/users_spec.rb index 3075a94f..b1669b39 100644 --- a/spec/requests/api/v1/users_spec.rb +++ b/spec/requests/api/v1/users_spec.rb @@ -7,12 +7,28 @@ RSpec.describe 'Api::V1::Users', type: :request do let(:user) { create(:user) } let(:headers) { { 'Authorization' => "Bearer #{user.api_key}" } } - it 'returns http success' do + it 'returns success response' do get '/api/v1/users/me', headers: headers expect(response).to have_http_status(:success) - expect(response.body).to include(user.email) - expect(response.body).to include(user.id.to_s) + end + + it 'returns only the keys and values stated in the serializer' do + get '/api/v1/users/me', headers: headers + + json = JSON.parse(response.body, symbolize_names: true) + + expect(json.keys).to eq([:user]) + expect(json[:user].keys).to match_array( + %i[email theme created_at updated_at settings] + ) + expect(json[:user][:settings].keys).to match_array(%i[ + maps fog_of_war_meters meters_between_routes preferred_map_layer + speed_colored_routes points_rendering_mode minutes_between_routes + time_threshold_minutes merge_threshold_minutes live_map_enabled + route_opacity immich_url photoprism_url visits_suggestions_enabled + speed_color_scale fog_of_war_threshold + ]) end end end diff --git a/spec/serializers/api/user_serializer_spec.rb b/spec/serializers/api/user_serializer_spec.rb new file mode 100644 index 00000000..178c64e0 --- /dev/null +++ b/spec/serializers/api/user_serializer_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Api::UserSerializer do + describe '#call' do + subject(:serializer) { described_class.new(user).call } + + let(:user) { create(:user, email: 'test@example.com', theme: 'dark') } + + it 'returns JSON with correct user attributes' do + expect(serializer[:user][:email]).to eq(user.email) + expect(serializer[:user][:theme]).to eq(user.theme) + expect(serializer[:user][:created_at]).to eq(user.created_at) + expect(serializer[:user][:updated_at]).to eq(user.updated_at) + end + + it 'returns settings with expected keys and types' do + settings = serializer[:user][:settings] + expect(settings).to include( + :maps, + :fog_of_war_meters, + :meters_between_routes, + :preferred_map_layer, + :speed_colored_routes, + :points_rendering_mode, + :minutes_between_routes, + :time_threshold_minutes, + :merge_threshold_minutes, + :live_map_enabled, + :route_opacity, + :immich_url, + :photoprism_url, + :visits_suggestions_enabled, + :speed_color_scale, + :fog_of_war_threshold + ) + end + + context 'with custom settings' do + let(:custom_settings) do + { + 'fog_of_war_meters' => 123, + 'meters_between_routes' => 456, + 'preferred_map_layer' => 'Satellite', + 'speed_colored_routes' => true, + 'points_rendering_mode' => 'cluster', + 'minutes_between_routes' => 42, + 'time_threshold_minutes' => 99, + 'merge_threshold_minutes' => 77, + 'live_map_enabled' => false, + 'route_opacity' => 0.75, + 'immich_url' => 'https://immich.example.com', + 'photoprism_url' => 'https://photoprism.example.com', + 'visits_suggestions_enabled' => 'false', + 'speed_color_scale' => 'rainbow', + 'fog_of_war_threshold' => 5, + 'maps' => { 'distance_unit' => 'mi' } + } + end + + let(:user) { create(:user, settings: custom_settings) } + + it 'serializes custom settings correctly' do + settings = serializer[:user][:settings] + expect(settings[:fog_of_war_meters]).to eq(123) + expect(settings[:meters_between_routes]).to eq(456) + expect(settings[:preferred_map_layer]).to eq('Satellite') + expect(settings[:speed_colored_routes]).to eq(true) + expect(settings[:points_rendering_mode]).to eq('cluster') + expect(settings[:minutes_between_routes]).to eq(42) + expect(settings[:time_threshold_minutes]).to eq(99) + expect(settings[:merge_threshold_minutes]).to eq(77) + expect(settings[:live_map_enabled]).to eq(false) + expect(settings[:route_opacity]).to eq(0.75) + expect(settings[:immich_url]).to eq('https://immich.example.com') + expect(settings[:photoprism_url]).to eq('https://photoprism.example.com') + expect(settings[:visits_suggestions_enabled]).to eq(false) + expect(settings[:speed_color_scale]).to eq('rainbow') + expect(settings[:fog_of_war_threshold]).to eq(5) + expect(settings[:maps]).to eq({ 'distance_unit' => 'mi' }) + end + end + end +end From 878d86356958fa834fee0359bd0bc861cb37b878 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Mon, 14 Jul 2025 21:15:45 +0200 Subject: [PATCH 05/30] Fix some tests --- CHANGELOG.md | 2 +- app/services/visits/suggest.rb | 5 ++-- spec/services/users/safe_settings_spec.rb | 8 +++++-- spec/services/visits/suggest_spec.rb | 2 +- spec/swagger/api/v1/users_controller_spec.rb | 23 +++++++++++-------- test/application_system_test_case.rb | 5 ---- .../application_cable/connection_test.rb | 11 --------- test/controllers/.keep | 0 test/fixtures/files/.keep | 0 test/helpers/.keep | 0 test/integration/.keep | 0 test/mailers/.keep | 0 test/models/.keep | 0 test/system/.keep | 0 test/test_helper.rb | 13 ----------- 15 files changed, 23 insertions(+), 46 deletions(-) delete mode 100644 test/application_system_test_case.rb delete mode 100644 test/channels/application_cable/connection_test.rb delete mode 100644 test/controllers/.keep delete mode 100644 test/fixtures/files/.keep delete mode 100644 test/helpers/.keep delete mode 100644 test/integration/.keep delete mode 100644 test/mailers/.keep delete mode 100644 test/models/.keep delete mode 100644 test/system/.keep delete mode 100644 test/test_helper.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 93771c6f..6bdd02d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Notification about Photon API load is now disabled. - All distance values are now stored in the database in meters. Conversion to user's preferred unit is done on the fly. - Every night, Dawarich will try to fetch names for places and visits that don't have them. #1281 #902 #583 #212 -- User settings are now being serialized in a more consistent way. `GET /api/v1/users/me` now returns the following data structure: +- ⚠️ User settings are now being serialized in a more consistent way ⚠. `GET /api/v1/users/me` now returns the following data structure: ```json { "user": { diff --git a/app/services/visits/suggest.rb b/app/services/visits/suggest.rb index 39f0ef11..7aab6b93 100644 --- a/app/services/visits/suggest.rb +++ b/app/services/visits/suggest.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class Visits::Suggest - include Rails.application.routes.url_helpers - attr_reader :points, :user, :start_at, :end_at def initialize(user, start_at:, end_at:) @@ -14,6 +12,7 @@ class Visits::Suggest def call visits = Visits::SmartDetect.new(user, start_at:, end_at:).call + create_visits_notification(user) if visits.any? return nil unless DawarichSettings.reverse_geocoding_enabled? @@ -35,7 +34,7 @@ class Visits::Suggest def create_visits_notification(user) content = <<~CONTENT - New visits have been suggested based on your location data from #{Time.zone.at(start_at)} to #{Time.zone.at(end_at)}. You can review them on the Visits page. + New visits have been suggested based on your location data from #{Time.zone.at(start_at)} to #{Time.zone.at(end_at)}. You can review them on the Visits page. CONTENT user.notifications.create!( diff --git a/spec/services/users/safe_settings_spec.rb b/spec/services/users/safe_settings_spec.rb index 11079920..573009c9 100644 --- a/spec/services/users/safe_settings_spec.rb +++ b/spec/services/users/safe_settings_spec.rb @@ -27,7 +27,9 @@ RSpec.describe Users::SafeSettings do photoprism_api_key: nil, maps: { "distance_unit" => "km" }, distance_unit: 'km', - visits_suggestions_enabled: true + visits_suggestions_enabled: true, + speed_color_scale: nil, + fog_of_war_threshold: nil } ) end @@ -98,7 +100,9 @@ RSpec.describe Users::SafeSettings do photoprism_api_key: "photoprism-key", maps: { "name" => "custom", "url" => "https://custom.example.com" }, distance_unit: nil, - visits_suggestions_enabled: false + visits_suggestions_enabled: false, + speed_color_scale: nil, + fog_of_war_threshold: nil } ) end diff --git a/spec/services/visits/suggest_spec.rb b/spec/services/visits/suggest_spec.rb index 167b9ba9..be56338c 100644 --- a/spec/services/visits/suggest_spec.rb +++ b/spec/services/visits/suggest_spec.rb @@ -81,7 +81,7 @@ RSpec.describe Visits::Suggest do before do allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(true) - # Create points for reverse geocoding test in a separate time range + create_visit_points(user, reverse_geocoding_start_at) clear_enqueued_jobs end diff --git a/spec/swagger/api/v1/users_controller_spec.rb b/spec/swagger/api/v1/users_controller_spec.rb index 753f4f08..73ad274d 100644 --- a/spec/swagger/api/v1/users_controller_spec.rb +++ b/spec/swagger/api/v1/users_controller_spec.rb @@ -29,19 +29,22 @@ describe 'Users API', type: :request do settings: { type: :object, properties: { - immich_url: { type: :string }, - route_opacity: { type: :string }, - immich_api_key: { type: :string }, - live_map_enabled: { type: :boolean }, - fog_of_war_meters: { type: :string }, + maps: { type: :object }, + fog_of_war_meters: { type: :integer }, + meters_between_routes: { type: :integer }, preferred_map_layer: { type: :string }, speed_colored_routes: { type: :boolean }, - meters_between_routes: { type: :string }, points_rendering_mode: { type: :string }, - minutes_between_routes: { type: :string }, - time_threshold_minutes: { type: :string }, - merge_threshold_minutes: { type: :string }, - speed_colored_polylines: { type: :boolean } + minutes_between_routes: { type: :integer }, + time_threshold_minutes: { type: :integer }, + merge_threshold_minutes: { type: :integer }, + live_map_enabled: { type: :boolean }, + route_opacity: { type: :number }, + immich_url: { type: :string, nullable: true }, + photoprism_url: { type: :string, nullable: true }, + visits_suggestions_enabled: { type: :boolean }, + speed_color_scale: { type: :string, nullable: true }, + fog_of_war_threshold: { type: :string, nullable: true } } }, admin: { type: :boolean } diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb deleted file mode 100644 index d19212ab..00000000 --- a/test/application_system_test_case.rb +++ /dev/null @@ -1,5 +0,0 @@ -require "test_helper" - -class ApplicationSystemTestCase < ActionDispatch::SystemTestCase - driven_by :selenium, using: :chrome, screen_size: [1400, 1400] -end diff --git a/test/channels/application_cable/connection_test.rb b/test/channels/application_cable/connection_test.rb deleted file mode 100644 index 800405f1..00000000 --- a/test/channels/application_cable/connection_test.rb +++ /dev/null @@ -1,11 +0,0 @@ -require "test_helper" - -class ApplicationCable::ConnectionTest < ActionCable::Connection::TestCase - # test "connects with cookies" do - # cookies.signed[:user_id] = 42 - # - # connect - # - # assert_equal connection.user_id, "42" - # end -end diff --git a/test/controllers/.keep b/test/controllers/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/fixtures/files/.keep b/test/fixtures/files/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/helpers/.keep b/test/helpers/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/integration/.keep b/test/integration/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/mailers/.keep b/test/mailers/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/models/.keep b/test/models/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/system/.keep b/test/system/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/test/test_helper.rb b/test/test_helper.rb deleted file mode 100644 index d713e377..00000000 --- a/test/test_helper.rb +++ /dev/null @@ -1,13 +0,0 @@ -ENV["RAILS_ENV"] ||= "test" -require_relative "../config/environment" -require "rails/test_help" - -class ActiveSupport::TestCase - # Run tests in parallel with specified workers - parallelize(workers: :number_of_processors) - - # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. - fixtures :all - - # Add more helper methods to be used by all tests here... -end From 49d1e7014bc52738938648c3e3ba6d09af39d34f Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Mon, 14 Jul 2025 21:26:19 +0200 Subject: [PATCH 06/30] Add simple analytics --- app/views/layouts/application.html.erb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 2ef799ef..e7b97017 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -17,6 +17,9 @@ <%= render 'application/favicon' %> <%= Sentry.get_trace_propagation_meta.html_safe if Sentry.initialized? %> + <% if !DawarichSettings.self_hosted? %> + + <% end %> From c31d09e5c3c5ec3e2c3d744d5c864dcf9b812a7e Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Wed, 16 Jul 2025 22:22:33 +0200 Subject: [PATCH 07/30] Refactor tracks jobs and services --- app/jobs/tracks/bulk_creating_job.rb | 22 - app/jobs/tracks/bulk_generator_job.rb | 45 ++ app/jobs/tracks/cleanup_job.rb | 36 ++ app/jobs/tracks/create_job.rb | 25 +- app/jobs/tracks/incremental_check_job.rb | 12 + app/jobs/tracks/incremental_generator_job.rb | 30 -- app/models/point.rb | 5 +- app/services/points_limit_exceeded.rb | 2 +- app/services/tracks/bulk_track_creator.rb | 47 -- app/services/tracks/cleaners/daily_cleaner.rb | 116 ----- app/services/tracks/cleaners/no_op_cleaner.rb | 16 - .../tracks/cleaners/replace_cleaner.rb | 69 --- app/services/tracks/create_from_points.rb | 73 --- app/services/tracks/generator.rb | 230 +++++---- .../buffer_handler.rb | 36 -- .../ignore_handler.rb | 48 -- app/services/tracks/incremental_processor.rb | 97 ++++ .../tracks/point_loaders/bulk_loader.rb | 54 -- .../point_loaders/incremental_loader.rb | 72 --- app/services/tracks/redis_buffer.rb | 72 --- app/services/tracks/segmentation.rb | 29 +- app/services/tracks/track_builder.rb | 3 +- .../registrations/_points_usage.html.erb | 4 +- config/schedule.yml | 6 +- ...0250704185707_create_tracks_from_points.rb | 2 +- docs/TRACKS_OVERVIEW.md | 483 ++++++++++++++++++ spec/jobs/tracks/bulk_creating_job_spec.rb | 19 - spec/jobs/tracks/cleanup_job_spec.rb | 88 ++++ spec/jobs/tracks/create_job_spec.rb | 122 ++++- .../jobs/tracks/incremental_check_job_spec.rb | 39 ++ spec/models/point_spec.rb | 4 +- spec/services/points_limit_exceeded_spec.rb | 6 +- .../tracks/bulk_track_creator_spec.rb | 176 ------- .../tracks/cleaners/daily_cleaner_spec.rb | 95 ---- .../tracks/create_from_points_spec.rb | 357 ------------- spec/services/tracks/generator_spec.rb | 369 ++++++------- .../tracks/incremental_processor_spec.rb | 249 +++++++++ spec/services/tracks/redis_buffer_spec.rb | 238 --------- spec/services/tracks/track_builder_spec.rb | 4 +- spec/support/point_helpers.rb | 20 + 40 files changed, 1524 insertions(+), 1896 deletions(-) delete mode 100644 app/jobs/tracks/bulk_creating_job.rb create mode 100644 app/jobs/tracks/bulk_generator_job.rb create mode 100644 app/jobs/tracks/cleanup_job.rb create mode 100644 app/jobs/tracks/incremental_check_job.rb delete mode 100644 app/jobs/tracks/incremental_generator_job.rb delete mode 100644 app/services/tracks/bulk_track_creator.rb delete mode 100644 app/services/tracks/cleaners/daily_cleaner.rb delete mode 100644 app/services/tracks/cleaners/no_op_cleaner.rb delete mode 100644 app/services/tracks/cleaners/replace_cleaner.rb delete mode 100644 app/services/tracks/create_from_points.rb delete mode 100644 app/services/tracks/incomplete_segment_handlers/buffer_handler.rb delete mode 100644 app/services/tracks/incomplete_segment_handlers/ignore_handler.rb create mode 100644 app/services/tracks/incremental_processor.rb delete mode 100644 app/services/tracks/point_loaders/bulk_loader.rb delete mode 100644 app/services/tracks/point_loaders/incremental_loader.rb delete mode 100644 app/services/tracks/redis_buffer.rb create mode 100644 docs/TRACKS_OVERVIEW.md delete mode 100644 spec/jobs/tracks/bulk_creating_job_spec.rb create mode 100644 spec/jobs/tracks/cleanup_job_spec.rb create mode 100644 spec/jobs/tracks/incremental_check_job_spec.rb delete mode 100644 spec/services/tracks/bulk_track_creator_spec.rb delete mode 100644 spec/services/tracks/cleaners/daily_cleaner_spec.rb delete mode 100644 spec/services/tracks/create_from_points_spec.rb create mode 100644 spec/services/tracks/incremental_processor_spec.rb delete mode 100644 spec/services/tracks/redis_buffer_spec.rb create mode 100644 spec/support/point_helpers.rb diff --git a/app/jobs/tracks/bulk_creating_job.rb b/app/jobs/tracks/bulk_creating_job.rb deleted file mode 100644 index 71ae15dc..00000000 --- a/app/jobs/tracks/bulk_creating_job.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -# This job is being run on daily basis to create tracks for all users. -# For each user, it starts from the end of their last track (or from their oldest point -# if no tracks exist) and processes points until the specified end_at time. -# -# To manually run for a specific time range: -# Tracks::BulkCreatingJob.perform_later(start_at: 1.week.ago, end_at: Time.current) -# -# To run for specific users only: -# Tracks::BulkCreatingJob.perform_later(user_ids: [1, 2, 3]) -# -# To let the job determine start times automatically (recommended): -# Tracks::BulkCreatingJob.perform_later(end_at: Time.current) -class Tracks::BulkCreatingJob < ApplicationJob - queue_as :tracks - sidekiq_options retry: false - - def perform(start_at: nil, end_at: 1.day.ago.end_of_day, user_ids: []) - Tracks::BulkTrackCreator.new(start_at:, end_at:, user_ids:).call - end -end diff --git a/app/jobs/tracks/bulk_generator_job.rb b/app/jobs/tracks/bulk_generator_job.rb new file mode 100644 index 00000000..a76970c2 --- /dev/null +++ b/app/jobs/tracks/bulk_generator_job.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +# Background job for bulk track generation. +# +# This job regenerates all tracks for a user from scratch, typically used for: +# - Initial track generation after data import +# - Full recalculation when settings change +# - Manual track regeneration requested by user +# +# The job uses the new simplified Tracks::Generator service with bulk mode, +# which cleans existing tracks and regenerates everything from points. +# +# Parameters: +# - user_id: The user whose tracks should be generated +# - start_at: Optional start timestamp to limit processing +# - end_at: Optional end timestamp to limit processing +# +class Tracks::BulkGeneratorJob < ApplicationJob + queue_as :default + + def perform(user_id, start_at: nil, end_at: nil) + user = User.find(user_id) + + Rails.logger.info "Starting bulk track generation for user #{user_id}, " \ + "start_at: #{start_at}, end_at: #{end_at}" + + generator = Tracks::Generator.new( + user, + start_at: start_at, + end_at: end_at, + mode: :bulk + ) + + generator.call + + Rails.logger.info "Completed bulk track generation for user #{user_id}" + rescue ActiveRecord::RecordNotFound => e + Rails.logger.error "Record not found in bulk track generation: #{e.message}" + # Don't retry if records are missing + rescue StandardError => e + Rails.logger.error "Error in bulk track generation for user #{user_id}: #{e.message}" + Rails.logger.error e.backtrace.join("\n") + raise # Re-raise for job retry logic + end +end \ No newline at end of file diff --git a/app/jobs/tracks/cleanup_job.rb b/app/jobs/tracks/cleanup_job.rb new file mode 100644 index 00000000..f9dc9c4e --- /dev/null +++ b/app/jobs/tracks/cleanup_job.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +# Lightweight cleanup job that runs weekly to catch any missed track generation. +# This replaces the daily bulk creation job with a more targeted approach. +# +# Instead of processing all users daily, this job only processes users who have +# untracked points that are older than a threshold (e.g., 1 day), indicating +# they may have been missed by incremental processing. +# +# This provides a safety net while avoiding the overhead of daily bulk processing. +class Tracks::CleanupJob < ApplicationJob + queue_as :tracks + sidekiq_options retry: false + + def perform(older_than: 1.day.ago) + users_with_old_untracked_points(older_than).find_each do |user| + Rails.logger.info "Processing missed tracks for user #{user.id}" + + # Process only the old untracked points + Tracks::Generator.new( + user, + end_at: older_than, + mode: :incremental + ).call + end + end + + private + + def users_with_old_untracked_points(older_than) + User.active.joins(:tracked_points) + .where(tracked_points: { track_id: nil, timestamp: ..older_than.to_i }) + .having('COUNT(tracked_points.id) >= 2') # Only users with enough points for tracks + .group(:id) + end +end diff --git a/app/jobs/tracks/create_job.rb b/app/jobs/tracks/create_job.rb index 57bc5bb4..514a6ac9 100644 --- a/app/jobs/tracks/create_job.rb +++ b/app/jobs/tracks/create_job.rb @@ -1,11 +1,30 @@ # frozen_string_literal: true class Tracks::CreateJob < ApplicationJob - queue_as :default + queue_as :tracks - def perform(user_id, start_at: nil, end_at: nil, cleaning_strategy: :replace) + def perform(user_id, start_at: nil, end_at: nil, mode: :daily) user = User.find(user_id) - tracks_created = Tracks::CreateFromPoints.new(user, start_at:, end_at:, cleaning_strategy:).call + + # Translate mode parameter to Generator mode + generator_mode = case mode + when :daily then :daily + when :none then :incremental + else :bulk + end + + # Count tracks before generation + tracks_before = user.tracks.count + + Tracks::Generator.new( + user, + start_at: start_at, + end_at: end_at, + mode: generator_mode + ).call + + # Calculate tracks created + tracks_created = user.tracks.count - tracks_before create_success_notification(user, tracks_created) rescue StandardError => e diff --git a/app/jobs/tracks/incremental_check_job.rb b/app/jobs/tracks/incremental_check_job.rb new file mode 100644 index 00000000..738246d6 --- /dev/null +++ b/app/jobs/tracks/incremental_check_job.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Tracks::IncrementalCheckJob < ApplicationJob + queue_as :tracks + + def perform(user_id, point_id) + user = User.find(user_id) + point = Point.find(point_id) + + Tracks::IncrementalProcessor.new(user, point).call + end +end diff --git a/app/jobs/tracks/incremental_generator_job.rb b/app/jobs/tracks/incremental_generator_job.rb deleted file mode 100644 index 00f8a46f..00000000 --- a/app/jobs/tracks/incremental_generator_job.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -class Tracks::IncrementalGeneratorJob < ApplicationJob - queue_as :default - sidekiq_options retry: 3 - - def perform(user_id, day = nil, grace_period_minutes = 5) - user = User.find(user_id) - day = day ? Date.parse(day.to_s) : Date.current - - Rails.logger.info "Starting incremental track generation for user #{user.id}, day #{day}" - - generator(user, day, grace_period_minutes).call - rescue StandardError => e - ExceptionReporter.call(e, 'Incremental track generation failed') - - raise e - end - - private - - def generator(user, day, grace_period_minutes) - @generator ||= Tracks::Generator.new( - user, - point_loader: Tracks::PointLoaders::IncrementalLoader.new(user, day), - incomplete_segment_handler: Tracks::IncompleteSegmentHandlers::BufferHandler.new(user, day, grace_period_minutes), - track_cleaner: Tracks::Cleaners::NoOpCleaner.new(user) - ) - end -end diff --git a/app/models/point.rb b/app/models/point.rb index e8f0f9e3..f45607d7 100644 --- a/app/models/point.rb +++ b/app/models/point.rb @@ -105,9 +105,6 @@ class Point < ApplicationRecord end def trigger_incremental_track_generation - point_date = Time.zone.at(timestamp).to_date - return if point_date < 1.day.ago.to_date - - Tracks::IncrementalGeneratorJob.perform_later(user_id, point_date.to_s, 5) + Tracks::IncrementalCheckJob.perform_later(user.id, id) end end diff --git a/app/services/points_limit_exceeded.rb b/app/services/points_limit_exceeded.rb index 62f9b821..f47543d1 100644 --- a/app/services/points_limit_exceeded.rb +++ b/app/services/points_limit_exceeded.rb @@ -7,7 +7,7 @@ class PointsLimitExceeded def call return false if DawarichSettings.self_hosted? - return true if @user.points.count >= points_limit + return true if @user.tracked_points.count >= points_limit false end diff --git a/app/services/tracks/bulk_track_creator.rb b/app/services/tracks/bulk_track_creator.rb deleted file mode 100644 index 7dba8506..00000000 --- a/app/services/tracks/bulk_track_creator.rb +++ /dev/null @@ -1,47 +0,0 @@ -# frozen_string_literal: true - -module Tracks - class BulkTrackCreator - def initialize(start_at: nil, end_at: 1.day.ago.end_of_day, user_ids: []) - @start_at = start_at&.to_datetime - @end_at = end_at&.to_datetime - @user_ids = user_ids - end - - def call - users.find_each do |user| - next if user.tracked_points.empty? - - user_start_at = start_at || start_time(user) - - next unless user.tracked_points.where(timestamp: user_start_at.to_i..end_at.to_i).exists? - - Tracks::CreateJob.perform_later( - user.id, - start_at: user_start_at, - end_at:, - cleaning_strategy: :daily - ) - end - end - - private - - attr_reader :start_at, :end_at, :user_ids - - def users - user_ids.any? ? User.active.where(id: user_ids) : User.active - end - - def start_time(user) - latest_track = user.tracks.order(end_at: :desc).first - - if latest_track - latest_track.end_at - else - oldest_point = user.tracked_points.order(:timestamp).first - oldest_point ? Time.zone.at(oldest_point.timestamp) : 1.day.ago.beginning_of_day - end - end - end -end diff --git a/app/services/tracks/cleaners/daily_cleaner.rb b/app/services/tracks/cleaners/daily_cleaner.rb deleted file mode 100644 index 6991fdfc..00000000 --- a/app/services/tracks/cleaners/daily_cleaner.rb +++ /dev/null @@ -1,116 +0,0 @@ -# frozen_string_literal: true - -# Track cleaning strategy for daily track processing. -# -# This cleaner handles tracks that overlap with the specified time window, -# ensuring proper handling of cross-day tracks and preventing orphaned points. -# -# How it works: -# 1. Finds tracks that overlap with the time window (not just those completely contained) -# 2. For overlapping tracks, removes only points within the time window -# 3. Deletes tracks that become empty after point removal -# 4. Preserves tracks that extend beyond the time window with their remaining points -# -# Key differences from ReplaceCleaner: -# - Handles tracks that span multiple days correctly -# - Uses overlap logic instead of containment logic -# - Preserves track portions outside the processing window -# - Prevents orphaned points from cross-day tracks -# -# Used primarily for: -# - Daily track processing that handles 24-hour windows -# - Incremental processing that respects existing cross-day tracks -# - Scenarios where tracks may span the processing boundary -# -# Example usage: -# cleaner = Tracks::Cleaners::DailyCleaner.new(user, start_at: 1.day.ago.beginning_of_day, end_at: 1.day.ago.end_of_day) -# cleaner.cleanup -# -module Tracks - module Cleaners - class DailyCleaner - attr_reader :user, :start_at, :end_at - - def initialize(user, start_at: nil, end_at: nil) - @user = user - @start_at = start_at - @end_at = end_at - end - - def cleanup - return unless start_at.present? && end_at.present? - - overlapping_tracks = find_overlapping_tracks - - return if overlapping_tracks.empty? - - Rails.logger.info "Processing #{overlapping_tracks.count} overlapping tracks for user #{user.id} in time window #{start_at} to #{end_at}" - - overlapping_tracks.each do |track| - process_overlapping_track(track) - end - end - - private - - def find_overlapping_tracks - # Find tracks that overlap with our time window - # A track overlaps if: track_start < window_end AND track_end > window_start - user.tracks.where( - '(start_at < ? AND end_at > ?)', - Time.zone.at(end_at), - Time.zone.at(start_at) - ) - end - - def process_overlapping_track(track) - # Find points within our time window that belong to this track - points_in_window = track.points.where( - 'timestamp >= ? AND timestamp <= ?', - start_at.to_i, - end_at.to_i - ) - - if points_in_window.empty? - Rails.logger.debug "Track #{track.id} has no points in time window, skipping" - return - end - - # Remove these points from the track - points_in_window.update_all(track_id: nil) - - Rails.logger.debug "Removed #{points_in_window.count} points from track #{track.id}" - - # Check if the track has any remaining points - remaining_points_count = track.points.count - - if remaining_points_count == 0 - # Track is now empty, delete it - Rails.logger.debug "Track #{track.id} is now empty, deleting" - track.destroy! - elsif remaining_points_count < 2 - # Track has too few points to be valid, delete it and orphan remaining points - Rails.logger.debug "Track #{track.id} has insufficient points (#{remaining_points_count}), deleting" - track.points.update_all(track_id: nil) - track.destroy! - else - # Track still has valid points outside our window, update its boundaries - Rails.logger.debug "Track #{track.id} still has #{remaining_points_count} points, updating boundaries" - update_track_boundaries(track) - end - end - - def update_track_boundaries(track) - remaining_points = track.points.order(:timestamp) - - return if remaining_points.empty? - - # Update track start/end times based on remaining points - track.update!( - start_at: Time.zone.at(remaining_points.first.timestamp), - end_at: Time.zone.at(remaining_points.last.timestamp) - ) - end - end - end -end diff --git a/app/services/tracks/cleaners/no_op_cleaner.rb b/app/services/tracks/cleaners/no_op_cleaner.rb deleted file mode 100644 index 9d564b9d..00000000 --- a/app/services/tracks/cleaners/no_op_cleaner.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -module Tracks - module Cleaners - class NoOpCleaner - def initialize(user) - @user = user - end - - def cleanup - # No cleanup needed for incremental processing - # We only append new tracks, don't remove existing ones - end - end - end -end diff --git a/app/services/tracks/cleaners/replace_cleaner.rb b/app/services/tracks/cleaners/replace_cleaner.rb deleted file mode 100644 index 41eae76e..00000000 --- a/app/services/tracks/cleaners/replace_cleaner.rb +++ /dev/null @@ -1,69 +0,0 @@ -# frozen_string_literal: true - -# Track cleaning strategy for bulk track regeneration. -# -# This cleaner removes existing tracks before generating new ones, -# ensuring a clean slate for bulk processing without duplicate tracks. -# -# How it works: -# 1. Finds all existing tracks for the user within the specified time range -# 2. Detaches all points from these tracks (sets track_id to nil) -# 3. Destroys the existing track records -# 4. Allows the generator to create fresh tracks from the same points -# -# Used primarily for: -# - Bulk track regeneration after settings changes -# - Reprocessing historical data with updated algorithms -# - Ensuring consistency when tracks need to be rebuilt -# -# The cleaner respects optional time boundaries (start_at/end_at) to enable -# partial regeneration of tracks within specific time windows. -# -# This strategy is essential for bulk operations but should not be used -# for incremental processing where existing tracks should be preserved. -# -# Example usage: -# cleaner = Tracks::Cleaners::ReplaceCleaner.new(user, start_at: 1.week.ago, end_at: Time.current) -# cleaner.cleanup -# -module Tracks - module Cleaners - class ReplaceCleaner - attr_reader :user, :start_at, :end_at - - def initialize(user, start_at: nil, end_at: nil) - @user = user - @start_at = start_at - @end_at = end_at - end - - def cleanup - tracks_to_remove = find_tracks_to_remove - - if tracks_to_remove.any? - Rails.logger.info "Removing #{tracks_to_remove.count} existing tracks for user #{user.id}" - - Point.where(track_id: tracks_to_remove.ids).update_all(track_id: nil) - - tracks_to_remove.destroy_all - end - end - - private - - def find_tracks_to_remove - scope = user.tracks - - if start_at.present? - scope = scope.where('start_at >= ?', Time.zone.at(start_at)) - end - - if end_at.present? - scope = scope.where('end_at <= ?', Time.zone.at(end_at)) - end - - scope - end - end - end -end diff --git a/app/services/tracks/create_from_points.rb b/app/services/tracks/create_from_points.rb deleted file mode 100644 index 73c15f66..00000000 --- a/app/services/tracks/create_from_points.rb +++ /dev/null @@ -1,73 +0,0 @@ -# frozen_string_literal: true - -class Tracks::CreateFromPoints - include Tracks::Segmentation - include Tracks::TrackBuilder - - attr_reader :user, :start_at, :end_at, :cleaning_strategy - - def initialize(user, start_at: nil, end_at: nil, cleaning_strategy: :replace) - @user = user - @start_at = start_at - @end_at = end_at - @cleaning_strategy = cleaning_strategy - end - - def call - generator = Tracks::Generator.new( - user, - point_loader: point_loader, - incomplete_segment_handler: incomplete_segment_handler, - track_cleaner: track_cleaner - ) - - generator.call - end - - # Expose threshold properties for tests - def distance_threshold_meters - @distance_threshold_meters ||= user.safe_settings.meters_between_routes.to_i || 500 - end - - def time_threshold_minutes - @time_threshold_minutes ||= user.safe_settings.minutes_between_routes.to_i || 60 - end - - private - - def point_loader - @point_loader ||= - Tracks::PointLoaders::BulkLoader.new( - user, start_at: start_at, end_at: end_at - ) - end - - def incomplete_segment_handler - @incomplete_segment_handler ||= - Tracks::IncompleteSegmentHandlers::IgnoreHandler.new(user) - end - - def track_cleaner - @track_cleaner ||= - case cleaning_strategy - when :daily - Tracks::Cleaners::DailyCleaner.new(user, start_at: start_at, end_at: end_at) - when :none - Tracks::Cleaners::NoOpCleaner.new(user) - else # :replace (default) - Tracks::Cleaners::ReplaceCleaner.new(user, start_at: start_at, end_at: end_at) - end - end - - # Legacy method for backward compatibility with tests - # Delegates to segmentation module logic - def should_start_new_track?(current_point, previous_point) - should_start_new_segment?(current_point, previous_point) - end - - # Legacy method for backward compatibility with tests - # Delegates to segmentation module logic - def calculate_distance_kilometers(point1, point2) - calculate_distance_kilometers_between_points(point1, point2) - end -end diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index 9ac40ced..ac599b59 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -1,108 +1,162 @@ # frozen_string_literal: true -# The core track generation engine that orchestrates the entire process of creating tracks from GPS points. +# Simplified track generation service that replaces the complex strategy pattern. # -# This class uses a flexible strategy pattern to handle different track generation scenarios: -# - Bulk processing: Generate all tracks at once from existing points -# - Incremental processing: Generate tracks as new points arrive +# This service handles both bulk and incremental track generation using a unified +# approach with different modes: # -# How it works: -# 1. Uses a PointLoader strategy to load points from the database -# 2. Applies segmentation logic to split points into track segments based on time/distance gaps -# 3. Determines which segments should be finalized into tracks vs buffered for later -# 4. Creates Track records from finalized segments with calculated statistics -# 5. Manages cleanup of existing tracks based on the chosen strategy +# - :bulk - Regenerates all tracks from scratch (replaces existing) +# - :incremental - Processes untracked points up to a specified end time +# - :daily - Processes tracks on a daily basis # -# Strategy Components: -# - point_loader: Loads points from database (BulkLoader, IncrementalLoader) -# - incomplete_segment_handler: Handles segments that aren't ready to finalize (IgnoreHandler, BufferHandler) -# - track_cleaner: Manages existing tracks when regenerating (ReplaceCleaner, NoOpCleaner) +# The service maintains the same core logic as the original system but simplifies +# the architecture by removing the multiple strategy classes in favor of +# mode-based configuration. # -# The class includes Tracks::Segmentation for splitting logic and Tracks::TrackBuilder for track creation. -# Distance and time thresholds are configurable per user via their settings. +# Key features: +# - Deterministic results (same algorithm for all modes) +# - Simple incremental processing without buffering complexity +# - Configurable time and distance thresholds from user settings +# - Automatic track statistics calculation +# - Proper handling of edge cases (empty points, incomplete segments) # -# Example usage: -# generator = Tracks::Generator.new( -# user, -# point_loader: Tracks::PointLoaders::BulkLoader.new(user), -# incomplete_segment_handler: Tracks::IncompleteSegmentHandlers::IgnoreHandler.new(user), -# track_cleaner: Tracks::Cleaners::ReplaceCleaner.new(user) -# ) -# tracks_created = generator.call +# Usage: +# # Bulk regeneration +# Tracks::Generator.new(user, mode: :bulk).call # -module Tracks - class Generator - include Tracks::Segmentation - include Tracks::TrackBuilder +# # Incremental processing +# Tracks::Generator.new(user, mode: :incremental).call +# +# # Daily processing +# Tracks::Generator.new(user, start_at: Date.current, mode: :daily).call +# +class Tracks::Generator + include Tracks::Segmentation + include Tracks::TrackBuilder - attr_reader :user, :point_loader, :incomplete_segment_handler, :track_cleaner + attr_reader :user, :start_at, :end_at, :mode - def initialize(user, point_loader:, incomplete_segment_handler:, track_cleaner:) - @user = user - @point_loader = point_loader - @incomplete_segment_handler = incomplete_segment_handler - @track_cleaner = track_cleaner - end + def initialize(user, start_at: nil, end_at: nil, mode: :bulk) + @user = user + @start_at = start_at + @end_at = end_at + @mode = mode.to_sym + end - def call - Rails.logger.info "Starting track generation for user #{user.id}" + def call + clean_existing_tracks if should_clean_tracks? - tracks_created = 0 + points = load_points + Rails.logger.debug "Generator: loaded #{points.size} points for user #{user.id} in #{mode} mode" + return if points.empty? - Point.transaction do - # Clean up existing tracks if needed - track_cleaner.cleanup + segments = split_points_into_segments(points) + Rails.logger.debug "Generator: created #{segments.size} segments" - # Load points using the configured strategy - points = point_loader.load_points + segments.each { |segment| create_track_from_segment(segment) } - if points.empty? - Rails.logger.info "No points to process for user #{user.id}" - return 0 - end + Rails.logger.info "Generated #{segments.size} tracks for user #{user.id} in #{mode} mode" + end - Rails.logger.info "Processing #{points.size} points for user #{user.id}" + private - # Apply segmentation logic - segments = split_points_into_segments(points) - - Rails.logger.info "Created #{segments.size} segments for user #{user.id}" - - # Process each segment - segments.each do |segment_points| - next if segment_points.size < 2 - - if incomplete_segment_handler.should_finalize_segment?(segment_points) - # Create track from finalized segment - track = create_track_from_points(segment_points) - if track&.persisted? - tracks_created += 1 - Rails.logger.debug "Created track #{track.id} with #{segment_points.size} points" - end - else - # Handle incomplete segment according to strategy - incomplete_segment_handler.handle_incomplete_segment(segment_points) - Rails.logger.debug "Stored #{segment_points.size} points as incomplete segment" - end - end - - # Cleanup any processed buffered data - incomplete_segment_handler.cleanup_processed_data - end - - Rails.logger.info "Completed track generation for user #{user.id}: #{tracks_created} tracks created" - tracks_created - end - - private - - # Required by Tracks::Segmentation module - def distance_threshold_meters - @distance_threshold_meters ||= user.safe_settings.meters_between_routes.to_i || 500 - end - - def time_threshold_minutes - @time_threshold_minutes ||= user.safe_settings.minutes_between_routes.to_i || 60 + def should_clean_tracks? + case mode + when :bulk then true + when :daily then true + when :incremental then false + else false end end + + def load_points + case mode + when :bulk then load_bulk_points + when :incremental then load_incremental_points + when :daily then load_daily_points + else + raise ArgumentError, "Unknown mode: #{mode}" + end + end + + def load_bulk_points + scope = user.tracked_points.order(:timestamp) + scope = scope.where(timestamp: time_range) if time_range_defined? + scope + end + + def load_incremental_points + # For incremental mode, we process untracked points + # If end_at is specified, only process points up to that time + scope = user.tracked_points.where(track_id: nil).order(:timestamp) + scope = scope.where(timestamp: ..end_at.to_i) if end_at.present? + scope + end + + def load_daily_points + day_range = daily_time_range + user.tracked_points.where(timestamp: day_range).order(:timestamp) + end + + def create_track_from_segment(segment) + Rails.logger.debug "Generator: processing segment with #{segment.size} points" + return unless segment.size >= 2 + + track = create_track_from_points(segment) + Rails.logger.debug "Generator: created track #{track&.id}" + track + end + + def time_range_defined? + start_at.present? || end_at.present? + end + + def time_range + return nil unless time_range_defined? + + Time.at(start_at&.to_i)..Time.at(end_at&.to_i) + end + + def daily_time_range + day = start_at&.to_date || Date.current + day.beginning_of_day.to_i..day.end_of_day.to_i + end + + def incremental_mode? + mode == :incremental + end + + def clean_existing_tracks + case mode + when :bulk + clean_bulk_tracks + when :daily + clean_daily_tracks + end + end + + def clean_bulk_tracks + scope = user.tracks + scope = scope.where(start_at: time_range) if time_range_defined? + + deleted_count = scope.delete_all + Rails.logger.info "Deleted #{deleted_count} existing tracks for user #{user.id}" + end + + def clean_daily_tracks + day_range_times = daily_time_range.map { |timestamp| Time.at(timestamp) } + range = Range.new(day_range_times.first, day_range_times.last) + + deleted_count = user.tracks.where(start_at: range).delete_all + Rails.logger.info "Deleted #{deleted_count} daily tracks for user #{user.id}" + end + + # Threshold methods from safe_settings + def distance_threshold_meters + @distance_threshold_meters ||= user.safe_settings.meters_between_routes.to_i + end + + def time_threshold_minutes + @time_threshold_minutes ||= user.safe_settings.minutes_between_routes.to_i + end end diff --git a/app/services/tracks/incomplete_segment_handlers/buffer_handler.rb b/app/services/tracks/incomplete_segment_handlers/buffer_handler.rb deleted file mode 100644 index 78549085..00000000 --- a/app/services/tracks/incomplete_segment_handlers/buffer_handler.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -module Tracks - module IncompleteSegmentHandlers - class BufferHandler - attr_reader :user, :day, :grace_period_minutes, :redis_buffer - - def initialize(user, day = nil, grace_period_minutes = 5) - @user = user - @day = day || Date.current - @grace_period_minutes = grace_period_minutes - @redis_buffer = Tracks::RedisBuffer.new(user.id, @day) - end - - def should_finalize_segment?(segment_points) - return false if segment_points.empty? - - # Check if the last point is old enough (grace period) - last_point_time = Time.zone.at(segment_points.last.timestamp) - grace_period_cutoff = Time.current - grace_period_minutes.minutes - - last_point_time < grace_period_cutoff - end - - def handle_incomplete_segment(segment_points) - redis_buffer.store(segment_points) - Rails.logger.debug "Stored #{segment_points.size} points in buffer for user #{user.id}, day #{day}" - end - - def cleanup_processed_data - redis_buffer.clear - Rails.logger.debug "Cleared buffer for user #{user.id}, day #{day}" - end - end - end -end diff --git a/app/services/tracks/incomplete_segment_handlers/ignore_handler.rb b/app/services/tracks/incomplete_segment_handlers/ignore_handler.rb deleted file mode 100644 index 0bdb912a..00000000 --- a/app/services/tracks/incomplete_segment_handlers/ignore_handler.rb +++ /dev/null @@ -1,48 +0,0 @@ -# frozen_string_literal: true - -# Incomplete segment handling strategy for bulk track generation. -# -# This handler always finalizes segments immediately without buffering, -# making it suitable for bulk processing where all data is historical -# and no segments are expected to grow with new incoming points. -# -# How it works: -# 1. Always returns true for should_finalize_segment? - every segment becomes a track -# 2. Ignores any incomplete segments (logs them but takes no action) -# 3. Requires no cleanup since no data is buffered -# -# Used primarily for: -# - Bulk track generation from historical data -# - One-time processing where all points are already available -# - Scenarios where you want to create tracks from every valid segment -# -# This strategy is efficient for bulk operations but not suitable for -# real-time processing where segments may grow as new points arrive. -# -# Example usage: -# handler = Tracks::IncompleteSegmentHandlers::IgnoreHandler.new(user) -# should_create_track = handler.should_finalize_segment?(segment_points) -# -module Tracks - module IncompleteSegmentHandlers - class IgnoreHandler - def initialize(user) - @user = user - end - - def should_finalize_segment?(segment_points) - # Always finalize segments in bulk processing - true - end - - def handle_incomplete_segment(segment_points) - # Ignore incomplete segments in bulk processing - Rails.logger.debug "Ignoring incomplete segment with #{segment_points.size} points" - end - - def cleanup_processed_data - # No cleanup needed for ignore strategy - end - end - end -end diff --git a/app/services/tracks/incremental_processor.rb b/app/services/tracks/incremental_processor.rb new file mode 100644 index 00000000..1d714e9e --- /dev/null +++ b/app/services/tracks/incremental_processor.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +# This service analyzes new points as they're created and determines whether +# they should trigger incremental track generation based on time and distance +# thresholds defined in user settings. +# +# The key insight is that we should trigger track generation when there's a +# significant gap between the new point and the previous point, indicating +# the end of a journey and the start of a new one. +# +# Process: +# 1. Check if the new point should trigger processing (skip imported points) +# 2. Find the last point before the new point +# 3. Calculate time and distance differences +# 4. If thresholds are exceeded, trigger incremental generation +# 5. Set the end_at time to the previous point's timestamp for track finalization +# +# This ensures tracks are properly finalized when journeys end, not when they start. +# +# Usage: +# # In Point model after_create_commit callback +# Tracks::IncrementalProcessor.new(user, new_point).call +# +class Tracks::IncrementalProcessor + attr_reader :user, :new_point, :previous_point + + def initialize(user, new_point) + @user = user + @new_point = new_point + @previous_point = find_previous_point + end + + def call + return unless should_process? + + start_at = find_start_time + end_at = find_end_time + + Tracks::CreateJob.perform_later( + user.id, + start_at: start_at, + end_at: end_at, + mode: :none + ) + end + + private + + def should_process? + return false if new_point.import_id.present? + return true unless previous_point + + exceeds_thresholds?(previous_point, new_point) + end + + def find_previous_point + @previous_point ||= + user.tracked_points + .where('timestamp < ?', new_point.timestamp) + .order(:timestamp) + .last + end + + def find_start_time + user.tracks.order(:end_at).last&.end_at + end + + def find_end_time + previous_point ? Time.at(previous_point.timestamp) : nil + end + + def exceeds_thresholds?(previous_point, current_point) + time_gap = time_difference_minutes(previous_point, current_point) + distance_gap = distance_difference_meters(previous_point, current_point) + + time_exceeded = time_gap >= time_threshold_minutes + distance_exceeded = distance_gap >= distance_threshold_meters + + time_exceeded || distance_exceeded + end + + def time_difference_minutes(point1, point2) + (point2.timestamp - point1.timestamp) / 60.0 + end + + def distance_difference_meters(point1, point2) + point1.distance_to(point2) * 1000 + end + + def time_threshold_minutes + @time_threshold_minutes ||= user.safe_settings.minutes_between_routes.to_i + end + + def distance_threshold_meters + @distance_threshold_meters ||= user.safe_settings.meters_between_routes.to_i + end +end diff --git a/app/services/tracks/point_loaders/bulk_loader.rb b/app/services/tracks/point_loaders/bulk_loader.rb deleted file mode 100644 index 85fc18e4..00000000 --- a/app/services/tracks/point_loaders/bulk_loader.rb +++ /dev/null @@ -1,54 +0,0 @@ -# frozen_string_literal: true - -# Point loading strategy for bulk track generation from existing GPS points. -# -# This loader retrieves all valid points for a user within an optional time range, -# suitable for regenerating all tracks at once or processing historical data. -# -# How it works: -# 1. Queries all points belonging to the user -# 2. Filters out points without valid coordinates or timestamps -# 3. Optionally filters by start_at/end_at time range if provided -# 4. Returns points ordered by timestamp for sequential processing -# -# Used primarily for: -# - Initial track generation when a user first enables tracks -# - Bulk regeneration of all tracks after settings changes -# - Processing historical data imports -# -# The loader is designed to be efficient for large datasets while ensuring -# data integrity by filtering out invalid points upfront. -# -# Example usage: -# loader = Tracks::PointLoaders::BulkLoader.new(user, start_at: 1.week.ago, end_at: Time.current) -# points = loader.load_points -# -module Tracks - module PointLoaders - class BulkLoader - attr_reader :user, :start_at, :end_at - - def initialize(user, start_at: nil, end_at: nil) - @user = user - @start_at = start_at - @end_at = end_at - end - - def load_points - scope = Point.where(user: user) - .where.not(lonlat: nil) - .where.not(timestamp: nil) - - if start_at.present? - scope = scope.where('timestamp >= ?', start_at) - end - - if end_at.present? - scope = scope.where('timestamp <= ?', end_at) - end - - scope.order(:timestamp) - end - end - end -end diff --git a/app/services/tracks/point_loaders/incremental_loader.rb b/app/services/tracks/point_loaders/incremental_loader.rb deleted file mode 100644 index 44be09f6..00000000 --- a/app/services/tracks/point_loaders/incremental_loader.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -module Tracks - module PointLoaders - class IncrementalLoader - attr_reader :user, :day, :redis_buffer - - def initialize(user, day = nil) - @user = user - @day = day || Date.current - @redis_buffer = Tracks::RedisBuffer.new(user.id, @day) - end - - def load_points - # Get buffered points from Redis - buffered_points = redis_buffer.retrieve - - # Find the last track for this day to determine where to start - last_track = Track.last_for_day(user, day) - - # Load new points since last track - new_points = load_new_points_since_last_track(last_track) - - # Combine buffered points with new points - combined_points = merge_points(buffered_points, new_points) - - Rails.logger.debug "Loaded #{buffered_points.size} buffered points and #{new_points.size} new points for user #{user.id}" - - combined_points - end - - private - - def load_new_points_since_last_track(last_track) - scope = user.points - .where.not(lonlat: nil) - .where.not(timestamp: nil) - .where(track_id: nil) # Only process points not already assigned to tracks - - if last_track - scope = scope.where('timestamp > ?', last_track.end_at.to_i) - else - # If no last track, load all points for the day - day_start = day.beginning_of_day.to_i - day_end = day.end_of_day.to_i - scope = scope.where('timestamp >= ? AND timestamp <= ?', day_start, day_end) - end - - scope.order(:timestamp) - end - - def merge_points(buffered_points, new_points) - # Convert buffered point hashes back to Point objects if needed - buffered_point_objects = buffered_points.map do |point_data| - # If it's already a Point object, use it directly - if point_data.is_a?(Point) - point_data - else - # Create a Point-like object from the hash - Point.new(point_data.except('id').symbolize_keys) - end - end - - # Combine and sort by timestamp - all_points = (buffered_point_objects + new_points.to_a).sort_by(&:timestamp) - - # Remove duplicates based on timestamp and coordinates - all_points.uniq { |point| [point.timestamp, point.lat, point.lon] } - end - end - end -end diff --git a/app/services/tracks/redis_buffer.rb b/app/services/tracks/redis_buffer.rb deleted file mode 100644 index 2262c7a4..00000000 --- a/app/services/tracks/redis_buffer.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -class Tracks::RedisBuffer - BUFFER_PREFIX = 'track_buffer' - BUFFER_EXPIRY = 7.days - - attr_reader :user_id, :day - - def initialize(user_id, day) - @user_id = user_id - @day = day.is_a?(Date) ? day : Date.parse(day.to_s) - end - - def store(points) - return if points.empty? - - points_data = serialize_points(points) - redis_key = buffer_key - - Rails.cache.write(redis_key, points_data, expires_in: BUFFER_EXPIRY) - Rails.logger.debug "Stored #{points.size} points in buffer for user #{user_id}, day #{day}" - end - - def retrieve - redis_key = buffer_key - cached_data = Rails.cache.read(redis_key) - - return [] unless cached_data - - deserialize_points(cached_data) - rescue StandardError => e - Rails.logger.error "Failed to retrieve buffered points for user #{user_id}, day #{day}: #{e.message}" - [] - end - - # Clear the buffer for the user/day combination - def clear - redis_key = buffer_key - Rails.cache.delete(redis_key) - Rails.logger.debug "Cleared buffer for user #{user_id}, day #{day}" - end - - def exists? - Rails.cache.exist?(buffer_key) - end - - private - - def buffer_key - "#{BUFFER_PREFIX}:#{user_id}:#{day.strftime('%Y-%m-%d')}" - end - - def serialize_points(points) - points.map do |point| - { - id: point.id, - lonlat: point.lonlat.to_s, - timestamp: point.timestamp, - lat: point.lat, - lon: point.lon, - altitude: point.altitude, - velocity: point.velocity, - battery: point.battery, - user_id: point.user_id - } - end - end - - def deserialize_points(points_data) - points_data || [] - end -end diff --git a/app/services/tracks/segmentation.rb b/app/services/tracks/segmentation.rb index e52cc3d8..8b93dee4 100644 --- a/app/services/tracks/segmentation.rb +++ b/app/services/tracks/segmentation.rb @@ -68,8 +68,8 @@ module Tracks::Segmentation return false if previous_point.nil? # Check time threshold (convert minutes to seconds) - current_timestamp = point_timestamp(current_point) - previous_timestamp = point_timestamp(previous_point) + current_timestamp = current_point.timestamp + previous_timestamp = previous_point.timestamp time_diff_seconds = current_timestamp - previous_timestamp time_threshold_seconds = time_threshold_minutes.to_i * 60 @@ -79,6 +79,7 @@ module Tracks::Segmentation # Check distance threshold - convert km to meters to match frontend logic distance_km = calculate_distance_kilometers_between_points(previous_point, current_point) distance_meters = distance_km * 1000 # Convert km to meters + return true if distance_meters > distance_threshold_meters false @@ -96,7 +97,7 @@ module Tracks::Segmentation return false if segment_points.size < 2 last_point = segment_points.last - last_timestamp = point_timestamp(last_point) + last_timestamp = last_point.timestamp current_time = Time.current.to_i # Don't finalize if the last point is too recent (within grace period) @@ -106,30 +107,10 @@ module Tracks::Segmentation time_since_last_point > grace_period_seconds end - def point_timestamp(point) - if point.respond_to?(:timestamp) - # Point objects from database always have integer timestamps - point.timestamp - elsif point.is_a?(Hash) - # Hash might come from Redis buffer or test data - timestamp = point[:timestamp] || point['timestamp'] - timestamp.to_i - else - raise ArgumentError, "Invalid point type: #{point.class}" - end - end - def point_coordinates(point) - if point.respond_to?(:lat) && point.respond_to?(:lon) - [point.lat, point.lon] - elsif point.is_a?(Hash) - [point[:lat] || point['lat'], point[:lon] || point['lon']] - else - raise ArgumentError, "Invalid point type: #{point.class}" - end + [point.lat, point.lon] end - # These methods need to be implemented by the including class def distance_threshold_meters raise NotImplementedError, "Including class must implement distance_threshold_meters" end diff --git a/app/services/tracks/track_builder.rb b/app/services/tracks/track_builder.rb index 12735eb7..99830bc1 100644 --- a/app/services/tracks/track_builder.rb +++ b/app/services/tracks/track_builder.rb @@ -73,6 +73,7 @@ module Tracks::TrackBuilder if track.save Point.where(id: points.map(&:id)).update_all(track_id: track.id) + track else Rails.logger.error "Failed to create track for user #{user.id}: #{track.errors.full_messages.join(', ')}" @@ -82,7 +83,7 @@ module Tracks::TrackBuilder end def build_path(points) - Tracks::BuildPath.new(points.map(&:lonlat)).call + Tracks::BuildPath.new(points).call end def calculate_track_distance(points) diff --git a/app/views/devise/registrations/_points_usage.html.erb b/app/views/devise/registrations/_points_usage.html.erb index e31c13ec..c079b93a 100644 --- a/app/views/devise/registrations/_points_usage.html.erb +++ b/app/views/devise/registrations/_points_usage.html.erb @@ -1,6 +1,6 @@

- You have used <%= number_with_delimiter(current_user.points.count) %> points of <%= number_with_delimiter(DawarichSettings::BASIC_PAID_PLAN_LIMIT) %> available. + You have used <%= number_with_delimiter(current_user.tracked_points.count) %> points of <%= number_with_delimiter(DawarichSettings::BASIC_PAID_PLAN_LIMIT) %> available.

- +

diff --git a/config/schedule.yml b/config/schedule.yml index aae74d6d..dee572ce 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -30,9 +30,9 @@ cache_preheating_job: class: "Cache::PreheatingJob" queue: default -tracks_bulk_creating_job: - cron: "10 0 * * *" # every day at 00:10 - class: "Tracks::BulkCreatingJob" +tracks_cleanup_job: + cron: "0 2 * * 0" # every Sunday at 02:00 + class: "Tracks::CleanupJob" queue: tracks place_name_fetching_job: diff --git a/db/data/20250704185707_create_tracks_from_points.rb b/db/data/20250704185707_create_tracks_from_points.rb index aae55296..fd744de9 100644 --- a/db/data/20250704185707_create_tracks_from_points.rb +++ b/db/data/20250704185707_create_tracks_from_points.rb @@ -20,7 +20,7 @@ class CreateTracksFromPoints < ActiveRecord::Migration[8.0] user.id, start_at: nil, end_at: nil, - cleaning_strategy: :replace + mode: :daily ) processed_users += 1 diff --git a/docs/TRACKS_OVERVIEW.md b/docs/TRACKS_OVERVIEW.md new file mode 100644 index 00000000..1874bc0e --- /dev/null +++ b/docs/TRACKS_OVERVIEW.md @@ -0,0 +1,483 @@ +# Dawarich Tracks Feature Overview + +## Table of Contents +- [Introduction](#introduction) +- [Architecture Overview](#architecture-overview) +- [Core Components](#core-components) +- [Data Flow](#data-flow) +- [Configuration](#configuration) +- [Usage Examples](#usage-examples) +- [API Reference](#api-reference) +- [Development Guidelines](#development-guidelines) + +## Introduction + +The Dawarich Tracks feature automatically converts raw GPS points into meaningful movement tracks. It analyzes sequences of location points to identify distinct journeys, providing users with structured visualizations of their movement patterns. + +### Key Features +- **Automatic Track Generation**: Converts GPS points into coherent movement tracks +- **Real-time Processing**: Incremental track generation as new points arrive +- **Configurable Thresholds**: User-customizable time and distance parameters +- **Multiple Generation Modes**: Bulk, incremental, and daily processing +- **Rich Statistics**: Distance, speed, elevation, and duration metrics +- **Live Updates**: Real-time track updates via WebSocket connections + +## Architecture Overview + +```mermaid +graph TB + A[GPS Points] --> B[Incremental Processor] + B --> C[Threshold Check] + C --> D{Exceeds Thresholds?} + D -->|Yes| E[Tracks Generator] + D -->|No| F[Skip Processing] + E --> G[Segmentation Engine] + G --> H[Track Builder] + H --> I[Database] + I --> J[Real-time Broadcasting] + J --> K[Frontend Updates] +``` + +## Core Components + +### 1. Models + +#### Track Model +```ruby +# app/models/track.rb +class Track < ApplicationRecord + belongs_to :user + has_many :points, dependent: :nullify + + # Attributes + # start_at, end_at (DateTime) + # distance (Integer, meters) + # avg_speed (Float, km/h) + # duration (Integer, seconds) + # elevation_gain/loss/max/min (Integer, meters) + # original_path (PostGIS LineString) +end +``` + +#### Point Model +```ruby +# app/models/point.rb +class Point < ApplicationRecord + belongs_to :track, optional: true + belongs_to :user + + # Triggers incremental track generation via background job + after_create_commit :trigger_incremental_track_generation + + private + + def trigger_incremental_track_generation + Tracks::IncrementalCheckJob.perform_later(user.id, id) + end +end +``` + +### 2. Services + +#### Tracks::Generator +**Purpose**: Unified track generation service with multiple modes + +```ruby +# Usage +Tracks::Generator.new(user, mode: :bulk).call +Tracks::Generator.new(user, mode: :incremental, end_at: Time.current).call +Tracks::Generator.new(user, mode: :daily, start_at: Date.current).call +``` + +**Modes**: +- `:bulk` - Regenerates all tracks from scratch (replaces existing) +- `:incremental` - Processes only untracked points up to specified time +- `:daily` - Processes tracks on daily basis with cleanup + +#### Tracks::IncrementalProcessor +**Purpose**: Analyzes new points and triggers track generation when thresholds are exceeded + +```ruby +# Automatically called when new points are created +Tracks::IncrementalProcessor.new(user, new_point).call +``` + +#### Tracks::Segmentation +**Purpose**: Core algorithm for splitting GPS points into meaningful segments + +**Criteria**: +- **Time threshold**: Configurable minutes gap (default: 30 minutes) +- **Distance threshold**: Configurable meters jump (default: 500 meters) +- **Minimum segment size**: 2 points required for valid track + +#### Tracks::TrackBuilder +**Purpose**: Converts point arrays into Track records with calculated statistics + +**Statistics Calculated**: +- **Distance**: Always stored in meters as integers +- **Duration**: Total time in seconds between first and last point +- **Average Speed**: Calculated in km/h regardless of user preference +- **Elevation Metrics**: Gain, loss, maximum, minimum in meters + +### 3. Background Jobs + +#### Tracks::IncrementalCheckJob +- **Purpose**: Lightweight job triggered by point creation +- **Queue**: `tracks` for dedicated processing +- **Trigger**: Automatically enqueued when non-import points are created +- **Function**: Checks thresholds and conditionally triggers track generation + +#### Tracks::CreateJob +- **Purpose**: Main orchestration job for track creation +- **Features**: User notifications on success/failure +- **Incremental Usage**: Enqueued by IncrementalCheckJob when thresholds are exceeded +- **Parameters**: `user_id`, `start_at`, `end_at`, `mode` + +#### Tracks::CleanupJob +- **Purpose**: Weekly cleanup of missed track generation +- **Schedule**: Runs weekly on Sunday at 02:00 via cron +- **Strategy**: Processes only users with old untracked points (1+ days old) + +### 4. Real-time Features + +#### TracksChannel (ActionCable) +```javascript +// Real-time track updates +consumer.subscriptions.create("TracksChannel", { + received(data) { + // Handle track created/updated/destroyed events + } +}); +``` + +## Data Flow + +### 1. Point Creation Flow +``` +New Point Created → IncrementalCheckJob → Incremental Processor → Threshold Check → +(if exceeded) → CreateJob → Track Generation → Database Update → +User Notification → Real-time Broadcast → Frontend Update +``` + +### 2. Bulk Processing Flow +``` +Scheduled Job → Load Historical Points → Segmentation → +Track Creation → Statistics Calculation → Database Batch Update +``` + +### 3. Incremental Processing Flow +``` +New Point → IncrementalCheckJob → Find Previous Point → Calculate Time/Distance Gaps → +(if thresholds exceeded) → CreateJob(start_at: last_track_end, end_at: previous_point_time) → +Process Untracked Points → Create Tracks → User Notification +``` + +## Configuration + +### User Settings +Tracks behavior is controlled by user-configurable settings in `Users::SafeSettings`: + +```ruby +# Default values +{ + 'meters_between_routes' => 500, # Distance threshold + 'minutes_between_routes' => 30, # Time threshold + 'route_opacity' => 60, # Visual opacity + 'distance_unit' => 'km' # Display unit (km/mi) +} +``` + +### Threshold Configuration +```ruby +# Time threshold: Gap longer than X minutes = new track +user.safe_settings.minutes_between_routes # default: 30 + +# Distance threshold: Jump larger than X meters = new track +user.safe_settings.meters_between_routes # default: 500 + +# Access in services +def time_threshold_minutes + user.safe_settings.minutes_between_routes.to_i +end +``` + +### Background Job Schedule +```yaml +# config/schedule.yml +tracks_cleanup_job: + cron: '0 2 * * 0' # Weekly on Sunday at 02:00 + class: Tracks::CleanupJob +``` + +## Usage Examples + +### 1. Manual Track Generation + +```ruby +# Bulk regeneration (replaces all existing tracks) +Tracks::Generator.new(user, mode: :bulk).call + +# Process specific date range +Tracks::Generator.new( + user, + start_at: 1.week.ago, + end_at: Time.current, + mode: :bulk +).call + +# Daily processing +Tracks::Generator.new( + user, + start_at: Date.current, + mode: :daily +).call +``` + +### 2. Incremental Processing + +```ruby +# Triggered automatically when points are created +point = Point.create!( + user: user, + timestamp: Time.current.to_i, + lonlat: 'POINT(-122.4194 37.7749)' +) +# → Automatically enqueues IncrementalCheckJob +# → Job checks thresholds and conditionally triggers track generation +``` + +### 3. Background Job Management + +```ruby +# Enqueue bulk processing +Tracks::BulkGeneratorJob.perform_later(user.id) + +# Enqueue incremental check (automatically triggered by point creation) +Tracks::IncrementalCheckJob.perform_later(user.id, point.id) + +# Enqueue incremental processing (triggered by IncrementalCheckJob) +Tracks::CreateJob.perform_later( + user.id, + start_at: last_track_end, + end_at: previous_point_timestamp, + mode: :none +) + +# Run cleanup for missed tracks +Tracks::CleanupJob.perform_later(older_than: 1.day.ago) + +# Create tracks with notifications +Tracks::CreateJob.perform_later(user.id, start_at: nil, end_at: nil, mode: :bulk) +``` + +### 4. Frontend Integration + +```javascript +// Initialize tracks on map +const tracksLayer = new TracksLayer(map, tracksData); + +// Handle real-time updates +consumer.subscriptions.create("TracksChannel", { + received(data) { + switch(data.event) { + case 'created': + tracksLayer.addTrack(data.track); + break; + case 'updated': + tracksLayer.updateTrack(data.track); + break; + case 'destroyed': + tracksLayer.removeTrack(data.track.id); + break; + } + } +}); +``` + +## API Reference + +### Track Model API + +```ruby +# Key methods +track.formatted_distance # Distance in user's preferred unit +track.distance_in_unit(unit) # Distance in specific unit +track.recalculate_path_and_distance! # Recalculate from points + +# Scopes +Track.for_user(user) +Track.between_dates(start_date, end_date) +Track.last_for_day(user, date) +``` + +### TrackSerializer Output +```json +{ + "id": 123, + "start_at": "2023-01-01T10:00:00Z", + "end_at": "2023-01-01T11:30:00Z", + "distance": 5000, + "avg_speed": 25.5, + "duration": 5400, + "elevation_gain": 150, + "elevation_loss": 100, + "elevation_max": 300, + "elevation_min": 200, + "path": "LINESTRING(...)" +} +``` + +### Service APIs + +```ruby +# Generator API +generator = Tracks::Generator.new(user, options) +generator.call # Returns nil, tracks saved to database + +# Processor API +processor = Tracks::IncrementalProcessor.new(user, point) +processor.call # May enqueue background job + +# Segmentation API (via inclusion) +segments = split_points_into_segments(points) +should_start_new_segment?(current_point, previous_point) +``` + +## Development Guidelines + +### 1. Adding New Generation Modes + +```ruby +# In Tracks::Generator +def load_points + case mode + when :bulk + load_bulk_points + when :incremental + load_incremental_points + when :daily + load_daily_points + when :custom_mode # New mode + load_custom_points + end +end + +def should_clean_tracks? + case mode + when :bulk, :daily then true + when :incremental, :custom_mode then false + end +end +``` + +### 2. Customizing Segmentation Logic + +```ruby +# Override in including class +def should_start_new_segment?(current_point, previous_point) + # Custom logic here + super || custom_condition?(current_point, previous_point) +end +``` + +### 3. Testing Patterns + +```ruby +# Test track generation +expect { generator.call }.to change(Track, :count).by(1) + +# Test point callback +expect { point.save! }.to have_enqueued_job(Tracks::IncrementalCheckJob) + .with(user.id, point.id) + +# Test incremental processing +expect(Tracks::CreateJob).to receive(:perform_later) + .with(user.id, start_at: anything, end_at: anything, mode: :none) +processor.call + +# Test segmentation +segments = generator.send(:segment_points, points) +expect(segments.size).to eq(2) +``` + +### 4. Performance Considerations + +- **Batch Processing**: Use `find_in_batches` for large datasets +- **Database Indexes**: Ensure proper indexing on `timestamp` and `track_id` +- **Memory Usage**: Process points in chunks for very large datasets +- **Asynchronous Processing**: Point creation is never blocked by track generation +- **Job Queue Management**: Monitor job queue performance for incremental processing + +### 5. Error Handling + +```ruby +# In services +begin + generator.call +rescue StandardError => e + Rails.logger.error "Track generation failed: #{e.message}" + # Handle gracefully +end + +# In jobs +def perform(*args) + # Main logic +rescue ActiveRecord::RecordNotFound + # Don't retry for missing records +rescue StandardError => e + Rails.logger.error "Job failed: #{e.message}" + raise # Re-raise for retry logic +end +``` + +### 6. Monitoring and Debugging + +```ruby +# Add logging +Rails.logger.info "Generated #{segments.size} tracks for user #{user.id}" + +# Performance monitoring +Rails.logger.info "Track generation took #{duration}ms" + +# Debug segmentation +Rails.logger.debug "Threshold check: time=#{time_gap}min, distance=#{distance_gap}m" +``` + +## Best Practices + +1. **Data Consistency**: Always store distances in meters, convert only for display +2. **Threshold Configuration**: Make thresholds user-configurable for flexibility +3. **Error Handling**: Gracefully handle missing data and network issues +4. **Performance**: Use database queries efficiently, avoid N+1 queries +5. **Testing**: Test all modes and edge cases thoroughly +6. **Real-time Updates**: Use ActionCable for responsive user experience +7. **Background Processing**: Use appropriate queues for different job priorities +8. **Asynchronous Design**: Never block point creation with track generation logic +9. **Job Monitoring**: Monitor background job performance and failure rates + +## Troubleshooting + +### Common Issues + +1. **Missing Tracks**: Check if points have `track_id: nil` for incremental processing +2. **Incorrect Thresholds**: Verify user settings configuration +3. **Job Failures**: Check background job logs for errors +4. **Real-time Updates**: Verify WebSocket connection and channel subscriptions +5. **Performance Issues**: Monitor database query performance and indexing + +### Debugging Tools + +```ruby +# Check track generation +user.tracked_points.where(track_id: nil).count # Untracked points + +# Verify thresholds +user.safe_settings.minutes_between_routes +user.safe_settings.meters_between_routes + +# Test segmentation +generator = Tracks::Generator.new(user, mode: :bulk) +segments = generator.send(:segment_points, points) +``` + +This overview provides a comprehensive understanding of the Dawarich Tracks feature, from high-level architecture to specific implementation details. diff --git a/spec/jobs/tracks/bulk_creating_job_spec.rb b/spec/jobs/tracks/bulk_creating_job_spec.rb deleted file mode 100644 index 47844452..00000000 --- a/spec/jobs/tracks/bulk_creating_job_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Tracks::BulkCreatingJob, type: :job do - describe '#perform' do - let(:service) { instance_double(Tracks::BulkTrackCreator) } - - before do - allow(Tracks::BulkTrackCreator).to receive(:new).with(start_at: 'foo', end_at: 'bar', user_ids: [1, 2]).and_return(service) - end - - it 'calls Tracks::BulkTrackCreator with the correct arguments' do - expect(service).to receive(:call) - - described_class.new.perform(start_at: 'foo', end_at: 'bar', user_ids: [1, 2]) - end - end -end diff --git a/spec/jobs/tracks/cleanup_job_spec.rb b/spec/jobs/tracks/cleanup_job_spec.rb new file mode 100644 index 00000000..d4823f86 --- /dev/null +++ b/spec/jobs/tracks/cleanup_job_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Tracks::CleanupJob, type: :job do + let(:user) { create(:user) } + + describe '#perform' do + context 'with old untracked points' do + let!(:old_points) do + create_points_around(user: user, count: 1, base_lat: 20.0, timestamp: 2.days.ago.to_i) + create_points_around(user: user, count: 1, base_lat: 20.0, timestamp: 1.day.ago.to_i) + end + let!(:recent_points) do + create_points_around(user: user, count: 2, base_lat: 20.0, timestamp: 1.hour.ago.to_i) + end + let(:generator) { instance_double(Tracks::Generator) } + + it 'processes only old untracked points' do + expect(Tracks::Generator).to receive(:new) + .and_return(generator) + + expect(generator).to receive(:call) + + described_class.new.perform(older_than: 1.day.ago) + end + + it 'logs processing information' do + allow(Tracks::Generator).to receive(:new).and_return(double(call: nil)) + + expect(Rails.logger).to receive(:info).with(/Processing missed tracks for user #{user.id}/) + + described_class.new.perform(older_than: 1.day.ago) + end + end + + context 'with users having insufficient points' do + let!(:single_point) do + create_points_around(user: user, count: 1, base_lat: 20.0, timestamp: 2.days.ago.to_i) + end + + it 'skips users with less than 2 points' do + expect(Tracks::Generator).not_to receive(:new) + + described_class.new.perform(older_than: 1.day.ago) + end + end + + context 'with no old untracked points' do + let(:track) { create(:track, user: user) } + let!(:tracked_points) do + create_points_around(user: user, count: 3, base_lat: 20.0, timestamp: 2.days.ago.to_i, track: track) + end + + it 'does not process any users' do + expect(Tracks::Generator).not_to receive(:new) + + described_class.new.perform(older_than: 1.day.ago) + end + end + + context 'with custom older_than parameter' do + let!(:points) do + create_points_around(user: user, count: 3, base_lat: 20.0, timestamp: 3.days.ago.to_i) + end + let(:generator) { instance_double(Tracks::Generator) } + + it 'uses custom threshold' do + expect(Tracks::Generator).to receive(:new) + .and_return(generator) + + expect(generator).to receive(:call) + + described_class.new.perform(older_than: 2.days.ago) + end + end + end + + describe 'job configuration' do + it 'uses tracks queue' do + expect(described_class.queue_name).to eq('tracks') + end + + it 'does not retry on failure' do + expect(described_class.sidekiq_options_hash['retry']).to be false + end + end +end diff --git a/spec/jobs/tracks/create_job_spec.rb b/spec/jobs/tracks/create_job_spec.rb index 2cbba7de..fd772609 100644 --- a/spec/jobs/tracks/create_job_spec.rb +++ b/spec/jobs/tracks/create_job_spec.rb @@ -6,26 +6,36 @@ RSpec.describe Tracks::CreateJob, type: :job do let(:user) { create(:user) } describe '#perform' do - let(:service_instance) { instance_double(Tracks::CreateFromPoints) } + let(:generator_instance) { instance_double(Tracks::Generator) } let(:notification_service) { instance_double(Notifications::Create) } before do - allow(Tracks::CreateFromPoints).to receive(:new).with(user, start_at: nil, end_at: nil, cleaning_strategy: :replace).and_return(service_instance) - allow(service_instance).to receive(:call).and_return(3) + allow(Tracks::Generator).to receive(:new).and_return(generator_instance) + allow(generator_instance).to receive(:call) allow(Notifications::Create).to receive(:new).and_return(notification_service) allow(notification_service).to receive(:call) end - it 'calls the service and creates a notification' do + it 'calls the generator and creates a notification' do + # Mock the generator to actually create tracks + allow(generator_instance).to receive(:call) do + create_list(:track, 2, user: user) + end + described_class.new.perform(user.id) - expect(Tracks::CreateFromPoints).to have_received(:new).with(user, start_at: nil, end_at: nil, cleaning_strategy: :replace) - expect(service_instance).to have_received(:call) + expect(Tracks::Generator).to have_received(:new).with( + user, + start_at: nil, + end_at: nil, + mode: :daily + ) + expect(generator_instance).to have_received(:call) expect(Notifications::Create).to have_received(:new).with( user: user, kind: :info, title: 'Tracks Generated', - content: 'Created 3 tracks from your location data. Check your tracks section to view them.' + content: 'Created 2 tracks from your location data. Check your tracks section to view them.' ) expect(notification_service).to have_received(:call) end @@ -33,38 +43,108 @@ RSpec.describe Tracks::CreateJob, type: :job do context 'with custom parameters' do let(:start_at) { 1.day.ago.beginning_of_day.to_i } let(:end_at) { 1.day.ago.end_of_day.to_i } - let(:cleaning_strategy) { :daily } + let(:mode) { :daily } before do - allow(Tracks::CreateFromPoints).to receive(:new).with(user, start_at: start_at, end_at: end_at, cleaning_strategy: cleaning_strategy).and_return(service_instance) - allow(service_instance).to receive(:call).and_return(2) + allow(Tracks::Generator).to receive(:new).and_return(generator_instance) + allow(generator_instance).to receive(:call) allow(Notifications::Create).to receive(:new).and_return(notification_service) allow(notification_service).to receive(:call) end - it 'passes custom parameters to the service' do - described_class.new.perform(user.id, start_at: start_at, end_at: end_at, cleaning_strategy: cleaning_strategy) + it 'passes custom parameters to the generator' do + # Create some existing tracks and mock generator to create 1 more + create_list(:track, 5, user: user) + allow(generator_instance).to receive(:call) do + create(:track, user: user) + end + + described_class.new.perform(user.id, start_at: start_at, end_at: end_at, mode: mode) - expect(Tracks::CreateFromPoints).to have_received(:new).with(user, start_at: start_at, end_at: end_at, cleaning_strategy: cleaning_strategy) - expect(service_instance).to have_received(:call) + expect(Tracks::Generator).to have_received(:new).with( + user, + start_at: start_at, + end_at: end_at, + mode: :daily + ) + expect(generator_instance).to have_received(:call) expect(Notifications::Create).to have_received(:new).with( user: user, kind: :info, title: 'Tracks Generated', - content: 'Created 2 tracks from your location data. Check your tracks section to view them.' + content: 'Created 1 tracks from your location data. Check your tracks section to view them.' ) expect(notification_service).to have_received(:call) end end - context 'when service raises an error' do + context 'with mode translation' do + before do + allow(Tracks::Generator).to receive(:new).and_return(generator_instance) + allow(generator_instance).to receive(:call) # No tracks created for mode tests + allow(Notifications::Create).to receive(:new).and_return(notification_service) + allow(notification_service).to receive(:call) + end + + it 'translates :none to :incremental' do + described_class.new.perform(user.id, mode: :none) + + expect(Tracks::Generator).to have_received(:new).with( + user, + start_at: nil, + end_at: nil, + mode: :incremental + ) + expect(Notifications::Create).to have_received(:new).with( + user: user, + kind: :info, + title: 'Tracks Generated', + content: 'Created 0 tracks from your location data. Check your tracks section to view them.' + ) + end + + it 'translates :daily to :daily' do + described_class.new.perform(user.id, mode: :daily) + + expect(Tracks::Generator).to have_received(:new).with( + user, + start_at: nil, + end_at: nil, + mode: :daily + ) + expect(Notifications::Create).to have_received(:new).with( + user: user, + kind: :info, + title: 'Tracks Generated', + content: 'Created 0 tracks from your location data. Check your tracks section to view them.' + ) + end + + it 'translates other modes to :bulk' do + described_class.new.perform(user.id, mode: :replace) + + expect(Tracks::Generator).to have_received(:new).with( + user, + start_at: nil, + end_at: nil, + mode: :bulk + ) + expect(Notifications::Create).to have_received(:new).with( + user: user, + kind: :info, + title: 'Tracks Generated', + content: 'Created 0 tracks from your location data. Check your tracks section to view them.' + ) + end + end + + context 'when generator raises an error' do let(:error_message) { 'Something went wrong' } - let(:service_instance) { instance_double(Tracks::CreateFromPoints) } let(:notification_service) { instance_double(Notifications::Create) } before do - allow(Tracks::CreateFromPoints).to receive(:new).with(user, start_at: nil, end_at: nil, cleaning_strategy: :replace).and_return(service_instance) - allow(service_instance).to receive(:call).and_raise(StandardError, error_message) + allow(Tracks::Generator).to receive(:new).and_return(generator_instance) + allow(generator_instance).to receive(:call).and_raise(StandardError, error_message) allow(Notifications::Create).to receive(:new).and_return(notification_service) allow(notification_service).to receive(:call) end @@ -108,8 +188,8 @@ RSpec.describe Tracks::CreateJob, type: :job do end describe 'queue' do - it 'is queued on default queue' do - expect(described_class.new.queue_name).to eq('default') + it 'is queued on tracks queue' do + expect(described_class.new.queue_name).to eq('tracks') end end end diff --git a/spec/jobs/tracks/incremental_check_job_spec.rb b/spec/jobs/tracks/incremental_check_job_spec.rb new file mode 100644 index 00000000..c25d1299 --- /dev/null +++ b/spec/jobs/tracks/incremental_check_job_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Tracks::IncrementalCheckJob, type: :job do + let(:user) { create(:user) } + let(:point) { create(:point, user: user) } + + describe '#perform' do + context 'with valid parameters' do + let(:processor) { instance_double(Tracks::IncrementalProcessor) } + + it 'calls the incremental processor' do + expect(Tracks::IncrementalProcessor).to receive(:new) + .with(user, point) + .and_return(processor) + + expect(processor).to receive(:call) + + described_class.new.perform(user.id, point.id) + end + end + end + + describe 'job configuration' do + it 'uses tracks queue' do + expect(described_class.queue_name).to eq('tracks') + end + end + + describe 'integration with ActiveJob' do + it 'enqueues the job' do + expect do + described_class.perform_later(user.id, point.id) + end.to have_enqueued_job(described_class) + .with(user.id, point.id) + end + end +end diff --git a/spec/models/point_spec.rb b/spec/models/point_spec.rb index eb56f84e..644f8003 100644 --- a/spec/models/point_spec.rb +++ b/spec/models/point_spec.rb @@ -127,8 +127,8 @@ RSpec.describe Point, type: :model do end let(:track) { create(:track) } - it 'enqueues Tracks::IncrementalGeneratorJob' do - expect { point.send(:trigger_incremental_track_generation) }.to have_enqueued_job(Tracks::IncrementalGeneratorJob).with(point.user_id, point.recorded_at.to_date.to_s, 5) + it 'enqueues Tracks::IncrementalCheckJob' do + expect { point.send(:trigger_incremental_track_generation) }.to have_enqueued_job(Tracks::IncrementalCheckJob).with(point.user_id, point.id) end end end diff --git a/spec/services/points_limit_exceeded_spec.rb b/spec/services/points_limit_exceeded_spec.rb index 8edfcad3..88cd6268 100644 --- a/spec/services/points_limit_exceeded_spec.rb +++ b/spec/services/points_limit_exceeded_spec.rb @@ -24,7 +24,7 @@ RSpec.describe PointsLimitExceeded do context 'when user points count is equal to the limit' do before do - allow(user.points).to receive(:count).and_return(10) + allow(user.tracked_points).to receive(:count).and_return(10) end it { is_expected.to be true } @@ -32,7 +32,7 @@ RSpec.describe PointsLimitExceeded do context 'when user points count exceeds the limit' do before do - allow(user.points).to receive(:count).and_return(11) + allow(user.tracked_points).to receive(:count).and_return(11) end it { is_expected.to be true } @@ -40,7 +40,7 @@ RSpec.describe PointsLimitExceeded do context 'when user points count is below the limit' do before do - allow(user.points).to receive(:count).and_return(9) + allow(user.tracked_points).to receive(:count).and_return(9) end it { is_expected.to be false } diff --git a/spec/services/tracks/bulk_track_creator_spec.rb b/spec/services/tracks/bulk_track_creator_spec.rb deleted file mode 100644 index 88594ee2..00000000 --- a/spec/services/tracks/bulk_track_creator_spec.rb +++ /dev/null @@ -1,176 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Tracks::BulkTrackCreator do - describe '#call' do - let!(:active_user) { create(:user) } - let!(:inactive_user) { create(:user, :inactive) } - let!(:user_without_points) { create(:user) } - - let(:start_at) { 1.day.ago.beginning_of_day } - let(:end_at) { 1.day.ago.end_of_day } - - before do - # Create points for active user in the target timeframe - create(:point, user: active_user, timestamp: start_at.to_i + 1.hour.to_i) - create(:point, user: active_user, timestamp: start_at.to_i + 2.hours.to_i) - - # Create points for inactive user in the target timeframe - create(:point, user: inactive_user, timestamp: start_at.to_i + 1.hour.to_i) - end - - context 'when explicit start_at is provided' do - it 'schedules tracks creation jobs for active users with points in the timeframe' do - expect { - described_class.new(start_at:, end_at:).call - }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at:, end_at:, cleaning_strategy: :daily) - end - - it 'does not schedule jobs for users without tracked points' do - expect { - described_class.new(start_at:, end_at:).call - }.not_to have_enqueued_job(Tracks::CreateJob).with(user_without_points.id, start_at:, end_at:, cleaning_strategy: :daily) - end - - it 'does not schedule jobs for users without points in the specified timeframe' do - # Create a user with points outside the timeframe - user_with_old_points = create(:user) - create(:point, user: user_with_old_points, timestamp: 2.days.ago.to_i) - - expect { - described_class.new(start_at:, end_at:).call - }.not_to have_enqueued_job(Tracks::CreateJob).with(user_with_old_points.id, start_at:, end_at:, cleaning_strategy: :daily) - end - end - - context 'when specific user_ids are provided' do - it 'only processes the specified users' do - expect { - described_class.new(start_at:, end_at:, user_ids: [active_user.id]).call - }.to have_enqueued_job(Tracks::CreateJob).with(active_user.id, start_at:, end_at:, cleaning_strategy: :daily) - end - - it 'does not process users not in the user_ids list' do - expect { - described_class.new(start_at:, end_at:, user_ids: [active_user.id]).call - }.not_to have_enqueued_job(Tracks::CreateJob).with(inactive_user.id, start_at:, end_at:, cleaning_strategy: :daily) - end - end - - context 'with automatic start time determination' do - let(:user_with_tracks) { create(:user) } - let(:user_without_tracks) { create(:user) } - let(:current_time) { Time.current } - - before do - # Create some historical points and tracks for user_with_tracks - create(:point, user: user_with_tracks, timestamp: 3.days.ago.to_i) - create(:point, user: user_with_tracks, timestamp: 2.days.ago.to_i) - - # Create a track ending 1 day ago - create(:track, user: user_with_tracks, end_at: 1.day.ago) - - # Create newer points after the last track - create(:point, user: user_with_tracks, timestamp: 12.hours.ago.to_i) - create(:point, user: user_with_tracks, timestamp: 6.hours.ago.to_i) - - # Create points for user without tracks - create(:point, user: user_without_tracks, timestamp: 2.days.ago.to_i) - create(:point, user: user_without_tracks, timestamp: 1.day.ago.to_i) - end - - it 'starts from the end of the last track for users with existing tracks' do - track_end_time = user_with_tracks.tracks.order(end_at: :desc).first.end_at - - expect { - described_class.new(end_at: current_time, user_ids: [user_with_tracks.id]).call - }.to have_enqueued_job(Tracks::CreateJob).with( - user_with_tracks.id, - start_at: track_end_time, - end_at: current_time.to_datetime, - cleaning_strategy: :daily - ) - end - - it 'starts from the oldest point for users without tracks' do - oldest_point_time = Time.zone.at(user_without_tracks.tracked_points.order(:timestamp).first.timestamp) - - expect { - described_class.new(end_at: current_time, user_ids: [user_without_tracks.id]).call - }.to have_enqueued_job(Tracks::CreateJob).with( - user_without_tracks.id, - start_at: oldest_point_time, - end_at: current_time.to_datetime, - cleaning_strategy: :daily - ) - end - - it 'falls back to 1 day ago for users with no points' do - expect { - described_class.new(end_at: current_time, user_ids: [user_without_points.id]).call - }.not_to have_enqueued_job(Tracks::CreateJob).with( - user_without_points.id, - start_at: anything, - end_at: anything, - cleaning_strategy: :daily - ) - end - end - - context 'with default parameters' do - let(:user_with_recent_points) { create(:user) } - - before do - # Create points within yesterday's timeframe - create(:point, user: user_with_recent_points, timestamp: 1.day.ago.beginning_of_day.to_i + 2.hours.to_i) - create(:point, user: user_with_recent_points, timestamp: 1.day.ago.beginning_of_day.to_i + 6.hours.to_i) - end - - it 'uses automatic start time determination with yesterday as end_at' do - oldest_point_time = Time.zone.at(user_with_recent_points.tracked_points.order(:timestamp).first.timestamp) - - expect { - described_class.new(user_ids: [user_with_recent_points.id]).call - }.to have_enqueued_job(Tracks::CreateJob).with( - user_with_recent_points.id, - start_at: oldest_point_time, - end_at: 1.day.ago.end_of_day.to_datetime, - cleaning_strategy: :daily - ) - end - end - end - - describe '#start_time' do - let(:user) { create(:user) } - let(:service) { described_class.new } - - context 'when user has tracks' do - let!(:old_track) { create(:track, user: user, end_at: 3.days.ago) } - let!(:recent_track) { create(:track, user: user, end_at: 1.day.ago) } - - it 'returns the end time of the most recent track' do - result = service.send(:start_time, user) - expect(result).to eq(recent_track.end_at) - end - end - - context 'when user has no tracks but has points' do - let!(:old_point) { create(:point, user: user, timestamp: 5.days.ago.to_i) } - let!(:recent_point) { create(:point, user: user, timestamp: 2.days.ago.to_i) } - - it 'returns the timestamp of the oldest point' do - result = service.send(:start_time, user) - expect(result).to eq(Time.zone.at(old_point.timestamp)) - end - end - - context 'when user has no tracks and no points' do - it 'returns 1 day ago beginning of day' do - result = service.send(:start_time, user) - expect(result).to eq(1.day.ago.beginning_of_day) - end - end - end -end diff --git a/spec/services/tracks/cleaners/daily_cleaner_spec.rb b/spec/services/tracks/cleaners/daily_cleaner_spec.rb deleted file mode 100644 index 06e64bf4..00000000 --- a/spec/services/tracks/cleaners/daily_cleaner_spec.rb +++ /dev/null @@ -1,95 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Tracks::Cleaners::DailyCleaner do - let(:user) { create(:user) } - let(:start_at) { 1.day.ago.beginning_of_day } - let(:end_at) { 1.day.ago.end_of_day } - let(:cleaner) { described_class.new(user, start_at: start_at.to_i, end_at: end_at.to_i) } - - describe '#cleanup' do - context 'when there are no overlapping tracks' do - before do - # Create a track that ends before our window - track = create(:track, user: user, start_at: 2.days.ago, end_at: 2.days.ago + 1.hour) - create(:point, user: user, track: track, timestamp: 2.days.ago.to_i) - end - - it 'does not remove any tracks' do - expect { cleaner.cleanup }.not_to change { user.tracks.count } - end - end - - context 'when a track is completely within the time window' do - let!(:track) { create(:track, user: user, start_at: start_at + 1.hour, end_at: end_at - 1.hour) } - let!(:point1) { create(:point, user: user, track: track, timestamp: (start_at + 1.hour).to_i) } - let!(:point2) { create(:point, user: user, track: track, timestamp: (start_at + 2.hours).to_i) } - - it 'removes all points from the track and deletes it' do - expect { cleaner.cleanup }.to change { user.tracks.count }.by(-1) - expect(point1.reload.track_id).to be_nil - expect(point2.reload.track_id).to be_nil - end - end - - context 'when a track spans across the time window' do - let!(:track) { create(:track, user: user, start_at: start_at - 1.hour, end_at: end_at + 1.hour) } - let!(:point_before) { create(:point, user: user, track: track, timestamp: (start_at - 30.minutes).to_i) } - let!(:point_during1) { create(:point, user: user, track: track, timestamp: (start_at + 1.hour).to_i) } - let!(:point_during2) { create(:point, user: user, track: track, timestamp: (start_at + 2.hours).to_i) } - let!(:point_after) { create(:point, user: user, track: track, timestamp: (end_at + 30.minutes).to_i) } - - it 'removes only points within the window and updates track boundaries' do - expect { cleaner.cleanup }.not_to change { user.tracks.count } - - # Points outside window should remain attached - expect(point_before.reload.track_id).to eq(track.id) - expect(point_after.reload.track_id).to eq(track.id) - - # Points inside window should be detached - expect(point_during1.reload.track_id).to be_nil - expect(point_during2.reload.track_id).to be_nil - - # Track boundaries should be updated - track.reload - expect(track.start_at).to be_within(1.second).of(Time.zone.at(point_before.timestamp)) - expect(track.end_at).to be_within(1.second).of(Time.zone.at(point_after.timestamp)) - end - end - - context 'when a track overlaps but has insufficient remaining points' do - let!(:track) { create(:track, user: user, start_at: start_at - 1.hour, end_at: end_at + 1.hour) } - let!(:point_before) { create(:point, user: user, track: track, timestamp: (start_at - 30.minutes).to_i) } - let!(:point_during) { create(:point, user: user, track: track, timestamp: (start_at + 1.hour).to_i) } - - it 'removes the track entirely and orphans remaining points' do - expect { cleaner.cleanup }.to change { user.tracks.count }.by(-1) - - expect(point_before.reload.track_id).to be_nil - expect(point_during.reload.track_id).to be_nil - end - end - - context 'when track has no points in the time window' do - let!(:track) { create(:track, user: user, start_at: start_at - 2.hours, end_at: end_at + 2.hours) } - let!(:point_before) { create(:point, user: user, track: track, timestamp: (start_at - 30.minutes).to_i) } - let!(:point_after) { create(:point, user: user, track: track, timestamp: (end_at + 30.minutes).to_i) } - - it 'does not modify the track' do - expect { cleaner.cleanup }.not_to change { user.tracks.count } - expect(track.reload.start_at).to be_within(1.second).of(track.start_at) - expect(track.reload.end_at).to be_within(1.second).of(track.end_at) - end - end - - context 'without start_at and end_at' do - let(:cleaner) { described_class.new(user) } - - it 'does not perform any cleanup' do - create(:track, user: user) - expect { cleaner.cleanup }.not_to change { user.tracks.count } - end - end - end -end diff --git a/spec/services/tracks/create_from_points_spec.rb b/spec/services/tracks/create_from_points_spec.rb deleted file mode 100644 index df64439d..00000000 --- a/spec/services/tracks/create_from_points_spec.rb +++ /dev/null @@ -1,357 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Tracks::CreateFromPoints do - let(:user) { create(:user) } - let(:service) { described_class.new(user) } - - describe '#initialize' do - it 'sets user and thresholds from user settings' do - expect(service.user).to eq(user) - expect(service.distance_threshold_meters).to eq(user.safe_settings.meters_between_routes.to_i) - expect(service.time_threshold_minutes).to eq(user.safe_settings.minutes_between_routes.to_i) - end - - it 'defaults to replace cleaning strategy' do - expect(service.cleaning_strategy).to eq(:replace) - end - - context 'with custom user settings' do - before do - user.update!(settings: user.settings.merge({ - 'meters_between_routes' => 1000, - 'minutes_between_routes' => 60 - })) - end - - it 'uses custom settings' do - service = described_class.new(user) - expect(service.distance_threshold_meters).to eq(1000) - expect(service.time_threshold_minutes).to eq(60) - end - end - - context 'with custom cleaning strategy' do - it 'accepts daily cleaning strategy' do - service = described_class.new(user, cleaning_strategy: :daily) - expect(service.cleaning_strategy).to eq(:daily) - end - - it 'accepts none cleaning strategy' do - service = described_class.new(user, cleaning_strategy: :none) - expect(service.cleaning_strategy).to eq(:none) - end - - it 'accepts custom date range with cleaning strategy' do - start_time = 1.day.ago.beginning_of_day.to_i - end_time = 1.day.ago.end_of_day.to_i - service = described_class.new(user, start_at: start_time, end_at: end_time, cleaning_strategy: :daily) - - expect(service.start_at).to eq(start_time) - expect(service.end_at).to eq(end_time) - expect(service.cleaning_strategy).to eq(:daily) - end - end - end - - describe '#call' do - context 'with no points' do - it 'returns 0 tracks created' do - expect(service.call).to eq(0) - end - end - - context 'with insufficient points' do - let!(:single_point) { create(:point, user: user, timestamp: 1.hour.ago.to_i) } - - it 'returns 0 tracks created' do - expect(service.call).to eq(0) - end - end - - context 'with points that form a single track' do - let(:base_time) { 1.hour.ago } - let!(:points) do - [ - create(:point, user: user, timestamp: base_time.to_i, - lonlat: 'POINT(-74.0060 40.7128)', altitude: 10), - create(:point, user: user, timestamp: (base_time + 5.minutes).to_i, - lonlat: 'POINT(-74.0070 40.7130)', altitude: 15), - create(:point, user: user, timestamp: (base_time + 10.minutes).to_i, - lonlat: 'POINT(-74.0080 40.7132)', altitude: 20) - ] - end - - it 'creates one track' do - expect { service.call }.to change(Track, :count).by(1) - end - - it 'returns 1 track created' do - expect(service.call).to eq(1) - end - - it 'sets track attributes correctly' do - service.call - track = Track.last - - expect(track.user).to eq(user) - expect(track.start_at).to be_within(1.second).of(base_time) - expect(track.end_at).to be_within(1.second).of(base_time + 10.minutes) - expect(track.duration).to eq(600) # 10 minutes in seconds - expect(track.original_path).to be_present - expect(track.distance).to be > 0 - expect(track.avg_speed).to be > 0 - expect(track.elevation_gain).to eq(10) # 20 - 10 - expect(track.elevation_loss).to eq(0) - expect(track.elevation_max).to eq(20) - expect(track.elevation_min).to eq(10) - end - - it 'associates points with the track' do - service.call - track = Track.last - expect(points.map(&:reload).map(&:track)).to all(eq(track)) - end - end - - context 'with points that should be split by time' do - let(:base_time) { 2.hours.ago } - let!(:points) do - [ - # First track - create(:point, user: user, timestamp: base_time.to_i, - lonlat: 'POINT(-74.0060 40.7128)'), - create(:point, user: user, timestamp: (base_time + 5.minutes).to_i, - lonlat: 'POINT(-74.0070 40.7130)'), - - # Gap > time threshold (default 30 minutes) - create(:point, user: user, timestamp: (base_time + 45.minutes).to_i, - lonlat: 'POINT(-74.0080 40.7132)'), - create(:point, user: user, timestamp: (base_time + 50.minutes).to_i, - lonlat: 'POINT(-74.0090 40.7134)') - ] - end - - it 'creates two tracks' do - expect { service.call }.to change(Track, :count).by(2) - end - - it 'returns 2 tracks created' do - expect(service.call).to eq(2) - end - end - - context 'with points that should be split by distance' do - let(:base_time) { 1.hour.ago } - let!(:points) do - [ - # First track - close points - create(:point, user: user, timestamp: base_time.to_i, - lonlat: 'POINT(-74.0060 40.7128)'), - create(:point, user: user, timestamp: (base_time + 1.minute).to_i, - lonlat: 'POINT(-74.0061 40.7129)'), - - # Far point (> distance threshold, but within time threshold) - create(:point, user: user, timestamp: (base_time + 2.minutes).to_i, - lonlat: 'POINT(-74.0500 40.7500)'), # ~5km away - create(:point, user: user, timestamp: (base_time + 3.minutes).to_i, - lonlat: 'POINT(-74.0501 40.7501)') - ] - end - - it 'creates two tracks' do - expect { service.call }.to change(Track, :count).by(2) - end - end - - context 'with existing tracks' do - let!(:existing_track) { create(:track, user: user) } - let!(:points) do - [ - create(:point, user: user, timestamp: 1.hour.ago.to_i, - lonlat: 'POINT(-74.0060 40.7128)'), - create(:point, user: user, timestamp: 50.minutes.ago.to_i, - lonlat: 'POINT(-74.0070 40.7130)') - ] - end - - it 'destroys existing tracks and creates new ones' do - expect { service.call }.to change(Track, :count).by(0) # -1 + 1 - expect(Track.exists?(existing_track.id)).to be false - end - - context 'with none cleaning strategy' do - let(:service) { described_class.new(user, cleaning_strategy: :none) } - - it 'preserves existing tracks and creates new ones' do - expect { service.call }.to change(Track, :count).by(1) # +1, existing preserved - expect(Track.exists?(existing_track.id)).to be true - end - end - end - - context 'with different cleaning strategies' do - let!(:points) do - [ - create(:point, user: user, timestamp: 1.hour.ago.to_i, - lonlat: 'POINT(-74.0060 40.7128)'), - create(:point, user: user, timestamp: 50.minutes.ago.to_i, - lonlat: 'POINT(-74.0070 40.7130)') - ] - end - - it 'works with replace strategy (default)' do - service = described_class.new(user, cleaning_strategy: :replace) - expect { service.call }.to change(Track, :count).by(1) - end - - it 'works with daily strategy' do - # Create points within the daily range we're testing - start_time = 1.day.ago.beginning_of_day.to_i - end_time = 1.day.ago.end_of_day.to_i - - # Create test points within the daily range - create(:point, user: user, timestamp: start_time + 1.hour.to_i, - lonlat: 'POINT(-74.0060 40.7128)') - create(:point, user: user, timestamp: start_time + 2.hours.to_i, - lonlat: 'POINT(-74.0070 40.7130)') - - # Create an existing track that overlaps with our time window - existing_track = create(:track, user: user, - start_at: Time.zone.at(start_time - 1.hour), - end_at: Time.zone.at(start_time + 30.minutes)) - - service = described_class.new(user, start_at: start_time, end_at: end_time, cleaning_strategy: :daily) - - # Daily cleaning should handle existing tracks properly and create new ones - expect { service.call }.to change(Track, :count).by(0) # existing cleaned and new created - end - - it 'works with none strategy' do - service = described_class.new(user, cleaning_strategy: :none) - expect { service.call }.to change(Track, :count).by(1) - end - end - - context 'with mixed elevation data' do - let!(:points) do - [ - create(:point, user: user, timestamp: 1.hour.ago.to_i, - lonlat: 'POINT(-74.0060 40.7128)', altitude: 100), - create(:point, user: user, timestamp: 50.minutes.ago.to_i, - lonlat: 'POINT(-74.0070 40.7130)', altitude: 150), - create(:point, user: user, timestamp: 40.minutes.ago.to_i, - lonlat: 'POINT(-74.0080 40.7132)', altitude: 120) - ] - end - - it 'calculates elevation correctly' do - service.call - track = Track.last - - expect(track.elevation_gain).to eq(50) # 150 - 100 - expect(track.elevation_loss).to eq(30) # 150 - 120 - expect(track.elevation_max).to eq(150) - expect(track.elevation_min).to eq(100) - end - end - - context 'with points missing altitude data' do - let!(:points) do - [ - create(:point, user: user, timestamp: 1.hour.ago.to_i, - lonlat: 'POINT(-74.0060 40.7128)', altitude: nil), - create(:point, user: user, timestamp: 50.minutes.ago.to_i, - lonlat: 'POINT(-74.0070 40.7130)', altitude: nil) - ] - end - - it 'uses default elevation values' do - service.call - track = Track.last - - expect(track.elevation_gain).to eq(0) - expect(track.elevation_loss).to eq(0) - expect(track.elevation_max).to eq(0) - expect(track.elevation_min).to eq(0) - end - end - end - - describe 'private methods' do - describe '#should_start_new_track?' do - let(:point1) { build(:point, timestamp: 1.hour.ago.to_i, lonlat: 'POINT(-74.0060 40.7128)') } - let(:point2) { build(:point, timestamp: 50.minutes.ago.to_i, lonlat: 'POINT(-74.0070 40.7130)') } - - it 'returns false when previous point is nil' do - result = service.send(:should_start_new_track?, point1, nil) - expect(result).to be false - end - - it 'returns true when time threshold is exceeded' do - # Create a point > 30 minutes later (default threshold) - later_point = build(:point, timestamp: 29.minutes.ago.to_i, lonlat: 'POINT(-74.0070 40.7130)') - - result = service.send(:should_start_new_track?, later_point, point1) - expect(result).to be true - end - - it 'returns true when distance threshold is exceeded' do - # Create a point far away (> 500m default threshold) - far_point = build(:point, timestamp: 59.minutes.ago.to_i, lonlat: 'POINT(-74.0500 40.7500)') - - result = service.send(:should_start_new_track?, far_point, point1) - expect(result).to be true - end - - it 'returns false when both thresholds are not exceeded' do - result = service.send(:should_start_new_track?, point2, point1) - expect(result).to be false - end - end - - describe '#calculate_distance_kilometers' do - let(:point1) { build(:point, lonlat: 'POINT(-74.0060 40.7128)') } - let(:point2) { build(:point, lonlat: 'POINT(-74.0070 40.7130)') } - - it 'calculates distance between two points in kilometers' do - distance = service.send(:calculate_distance_kilometers, point1, point2) - expect(distance).to be > 0 - expect(distance).to be < 0.2 # Should be small distance for close points (in km) - end - end - - describe '#calculate_average_speed' do - it 'calculates speed correctly' do - # 1000 meters in 100 seconds = 10 m/s = 36 km/h - speed = service.send(:calculate_average_speed, 1000, 100) - expect(speed).to eq(36.0) - end - - it 'returns 0 for zero duration' do - speed = service.send(:calculate_average_speed, 1000, 0) - expect(speed).to eq(0.0) - end - - it 'returns 0 for zero distance' do - speed = service.send(:calculate_average_speed, 0, 100) - expect(speed).to eq(0.0) - end - end - - describe '#calculate_track_distance' do - let(:points) do - [ - build(:point, lonlat: 'POINT(-74.0060 40.7128)'), - build(:point, lonlat: 'POINT(-74.0070 40.7130)') - ] - end - - it 'stores distance in meters by default' do - distance = service.send(:calculate_track_distance, points) - expect(distance).to eq(87) - end - end - end -end diff --git a/spec/services/tracks/generator_spec.rb b/spec/services/tracks/generator_spec.rb index 851508f8..0b53c5f5 100644 --- a/spec/services/tracks/generator_spec.rb +++ b/spec/services/tracks/generator_spec.rb @@ -4,253 +4,220 @@ require 'rails_helper' RSpec.describe Tracks::Generator do let(:user) { create(:user) } - let(:point_loader) { double('PointLoader') } - let(:incomplete_segment_handler) { double('IncompleteSegmentHandler') } - let(:track_cleaner) { double('Cleaner') } - - let(:generator) do - described_class.new( - user, - point_loader: point_loader, - incomplete_segment_handler: incomplete_segment_handler, - track_cleaner: track_cleaner - ) - end + let(:safe_settings) { user.safe_settings } before do - allow_any_instance_of(Users::SafeSettings).to receive(:meters_between_routes).and_return(500) - allow_any_instance_of(Users::SafeSettings).to receive(:minutes_between_routes).and_return(60) - allow_any_instance_of(Users::SafeSettings).to receive(:distance_unit).and_return('km') + allow(user).to receive(:safe_settings).and_return(safe_settings) end describe '#call' do - context 'with no points to process' do - before do - allow(track_cleaner).to receive(:cleanup) - allow(point_loader).to receive(:load_points).and_return([]) + context 'with bulk mode' do + let(:generator) { described_class.new(user, mode: :bulk) } + + context 'with sufficient points' do + let!(:points) { create_points_around(user: user, count: 5, base_lat: 20.0) } + + it 'generates tracks from all points' do + expect { generator.call }.to change(Track, :count).by(1) + end + + it 'cleans existing tracks' do + existing_track = create(:track, user: user) + generator.call + expect(Track.exists?(existing_track.id)).to be false + end + + it 'associates points with created tracks' do + generator.call + expect(points.map(&:reload).map(&:track)).to all(be_present) + end end - it 'returns 0 tracks created' do - result = generator.call - expect(result).to eq(0) + context 'with insufficient points' do + let!(:points) { create_points_around(user: user, count: 1, base_lat: 20.0) } + + it 'does not create tracks' do + expect { generator.call }.not_to change(Track, :count) + end end - it 'does not call incomplete segment handler' do - expect(incomplete_segment_handler).not_to receive(:should_finalize_segment?) - expect(incomplete_segment_handler).not_to receive(:handle_incomplete_segment) - expect(incomplete_segment_handler).not_to receive(:cleanup_processed_data) + context 'with time range' do + let!(:old_points) { create_points_around(user: user, count: 3, base_lat: 20.0, timestamp: 2.days.ago.to_i) } + let!(:new_points) { create_points_around(user: user, count: 3, base_lat: 21.0, timestamp: 1.day.ago.to_i) } - generator.call + it 'only processes points within range' do + generator = described_class.new( + user, + start_at: 1.day.ago.beginning_of_day, + end_at: 1.day.ago.end_of_day, + mode: :bulk + ) + + generator.call + track = Track.last + expect(track.points.count).to eq(3) + end end end - context 'with points that create tracks' do - let!(:points) do - [ - create(:point, user: user, lonlat: 'POINT(-74.0060 40.7128)', timestamp: 1.hour.ago.to_i, latitude: 40.7128, longitude: -74.0060), - create(:point, user: user, lonlat: 'POINT(-74.0050 40.7138)', timestamp: 30.minutes.ago.to_i, latitude: 40.7138, longitude: -74.0050), - create(:point, user: user, lonlat: 'POINT(-74.0040 40.7148)', timestamp: 10.minutes.ago.to_i, latitude: 40.7148, longitude: -74.0040) - ] - end + context 'with incremental mode' do + let(:generator) { described_class.new(user, mode: :incremental) } - before do - allow(track_cleaner).to receive(:cleanup) - allow(point_loader).to receive(:load_points).and_return(points) - allow(incomplete_segment_handler).to receive(:should_finalize_segment?).and_return(true) - allow(incomplete_segment_handler).to receive(:cleanup_processed_data) + context 'with untracked points' do + let!(:points) { create_points_around(user: user, count: 3, base_lat: 22.0, track_id: nil) } + + it 'processes untracked points' do + expect { generator.call }.to change(Track, :count).by(1) + end + + it 'associates points with created tracks' do + generator.call + expect(points.map(&:reload).map(&:track)).to all(be_present) + end end - it 'creates tracks from segments' do - expect { generator.call }.to change { Track.count }.by(1) + context 'with end_at specified' do + let!(:early_points) { create_points_around(user: user, count: 2, base_lat: 23.0, timestamp: 2.hours.ago.to_i) } + let!(:late_points) { create_points_around(user: user, count: 2, base_lat: 24.0, timestamp: 1.hour.ago.to_i) } + + it 'only processes points up to end_at' do + generator = described_class.new(user, end_at: 1.5.hours.ago, mode: :incremental) + generator.call + + expect(Track.count).to eq(1) + expect(Track.first.points.count).to eq(2) + end end - it 'returns the number of tracks created' do - result = generator.call - expect(result).to eq(1) - end + context 'without existing tracks' do + let!(:points) { create_points_around(user: user, count: 3, base_lat: 25.0) } - it 'calls cleanup on processed data' do - expect(incomplete_segment_handler).to receive(:cleanup_processed_data) - generator.call + it 'does not clean existing tracks' do + existing_track = create(:track, user: user) + generator.call + expect(Track.exists?(existing_track.id)).to be true + end end - - it 'assigns points to the created track' do - generator.call - points.each(&:reload) - track_ids = points.map(&:track_id).uniq.compact - expect(track_ids.size).to eq(1) - end end - context 'with incomplete segments' do - let!(:points) do - [ - create(:point, user: user, lonlat: 'POINT(-74.0060 40.7128)', timestamp: 5.minutes.ago.to_i, latitude: 40.7128, longitude: -74.0060), - create(:point, user: user, lonlat: 'POINT(-74.0050 40.7138)', timestamp: 4.minutes.ago.to_i, latitude: 40.7138, longitude: -74.0050) - ] - end + context 'with daily mode' do + let(:today) { Date.current } + let(:generator) { described_class.new(user, start_at: today, mode: :daily) } - before do - allow(track_cleaner).to receive(:cleanup) - allow(point_loader).to receive(:load_points).and_return(points) - allow(incomplete_segment_handler).to receive(:should_finalize_segment?).and_return(false) - allow(incomplete_segment_handler).to receive(:handle_incomplete_segment) - allow(incomplete_segment_handler).to receive(:cleanup_processed_data) + let!(:today_points) { create_points_around(user: user, count: 3, base_lat: 26.0, timestamp: today.beginning_of_day.to_i) } + let!(:yesterday_points) { create_points_around(user: user, count: 3, base_lat: 27.0, timestamp: 1.day.ago.to_i) } + + it 'only processes points from specified day' do + generator.call + track = Track.last + expect(track.points.count).to eq(3) end + it 'cleans existing tracks for the day' do + existing_track = create(:track, user: user, start_at: today.beginning_of_day) + generator.call + expect(Track.exists?(existing_track.id)).to be false + end + end + + context 'with empty points' do + let(:generator) { described_class.new(user, mode: :bulk) } + it 'does not create tracks' do - expect { generator.call }.not_to change { Track.count } - end - - it 'handles incomplete segments' do - expect(incomplete_segment_handler).to receive(:handle_incomplete_segment).with(points) - generator.call - end - - it 'returns 0 tracks created' do - result = generator.call - expect(result).to eq(0) + expect { generator.call }.not_to change(Track, :count) end end - context 'with mixed complete and incomplete segments' do - let!(:old_points) do - [ - create(:point, user: user, lonlat: 'POINT(-74.0060 40.7128)', timestamp: 2.hours.ago.to_i, latitude: 40.7128, longitude: -74.0060), - create(:point, user: user, lonlat: 'POINT(-74.0050 40.7138)', timestamp: 1.hour.ago.to_i, latitude: 40.7138, longitude: -74.0050) - ] - end - - let!(:recent_points) do - [ - create(:point, user: user, lonlat: 'POINT(-74.0040 40.7148)', timestamp: 3.minutes.ago.to_i, latitude: 40.7148, longitude: -74.0040), - create(:point, user: user, lonlat: 'POINT(-74.0030 40.7158)', timestamp: 2.minutes.ago.to_i, latitude: 40.7158, longitude: -74.0030) - ] - end - - before do - allow(track_cleaner).to receive(:cleanup) - allow(point_loader).to receive(:load_points).and_return(old_points + recent_points) - - # First segment (old points) should be finalized - # Second segment (recent points) should be incomplete - call_count = 0 - allow(incomplete_segment_handler).to receive(:should_finalize_segment?) do |segment_points| - call_count += 1 - call_count == 1 # Only finalize first segment - end - - allow(incomplete_segment_handler).to receive(:handle_incomplete_segment) - allow(incomplete_segment_handler).to receive(:cleanup_processed_data) - end - - it 'creates tracks for complete segments only' do - expect { generator.call }.to change { Track.count }.by(1) - end - - it 'handles incomplete segments' do - # Note: The exact behavior depends on segmentation logic - # The important thing is that the method can be called without errors - generator.call - # Test passes if no exceptions are raised - expect(true).to be_truthy - end - - it 'returns the correct number of tracks created' do - result = generator.call - expect(result).to eq(1) - end - end - - context 'with insufficient points for track creation' do - let!(:single_point) do - [create(:point, user: user, lonlat: 'POINT(-74.0060 40.7128)', timestamp: 1.hour.ago.to_i, latitude: 40.7128, longitude: -74.0060)] - end + context 'with threshold configuration' do + let(:generator) { described_class.new(user, mode: :bulk) } before do - allow(track_cleaner).to receive(:cleanup) - allow(point_loader).to receive(:load_points).and_return(single_point) - allow(incomplete_segment_handler).to receive(:should_finalize_segment?).and_return(true) - allow(incomplete_segment_handler).to receive(:cleanup_processed_data) + allow(safe_settings).to receive(:meters_between_routes).and_return(1000) + allow(safe_settings).to receive(:minutes_between_routes).and_return(90) end - it 'does not create tracks with less than 2 points' do - expect { generator.call }.not_to change { Track.count } - end - - it 'returns 0 tracks created' do - result = generator.call - expect(result).to eq(0) + it 'uses configured thresholds' do + expect(generator.send(:distance_threshold_meters)).to eq(1000) + expect(generator.send(:time_threshold_minutes)).to eq(90) end end - context 'error handling' do - before do - allow(track_cleaner).to receive(:cleanup) - allow(point_loader).to receive(:load_points).and_raise(StandardError, 'Point loading failed') - end - - it 'propagates errors from point loading' do - expect { generator.call }.to raise_error(StandardError, 'Point loading failed') + context 'with invalid mode' do + it 'raises argument error' do + expect do + described_class.new(user, mode: :invalid).call + end.to raise_error(ArgumentError, /Unknown mode/) end end end - describe 'strategy pattern integration' do - context 'with bulk processing strategies' do - let(:bulk_loader) { Tracks::PointLoaders::BulkLoader.new(user) } - let(:ignore_handler) { Tracks::IncompleteSegmentHandlers::IgnoreHandler.new(user) } - let(:replace_cleaner) { Tracks::Cleaners::ReplaceCleaner.new(user) } + describe 'segmentation behavior' do + let(:generator) { described_class.new(user, mode: :bulk) } - let(:bulk_generator) do - described_class.new( - user, - point_loader: bulk_loader, - incomplete_segment_handler: ignore_handler, - track_cleaner: replace_cleaner - ) + context 'with points exceeding time threshold' do + let!(:points) do + [ + create_points_around(user: user, count: 1, base_lat: 29.0, timestamp: 90.minutes.ago.to_i), + create_points_around(user: user, count: 1, base_lat: 29.0, timestamp: 60.minutes.ago.to_i), + # Gap exceeds threshold 👇👇👇 + create_points_around(user: user, count: 1, base_lat: 29.0, timestamp: 10.minutes.ago.to_i), + create_points_around(user: user, count: 1, base_lat: 29.0, timestamp: Time.current.to_i) + ] end - let!(:existing_track) { create(:track, user: user) } - let!(:points) do - [ - create(:point, user: user, lonlat: 'POINT(-74.0060 40.7128)', timestamp: 1.hour.ago.to_i, latitude: 40.7128, longitude: -74.0060), - create(:point, user: user, lonlat: 'POINT(-74.0050 40.7138)', timestamp: 30.minutes.ago.to_i, latitude: 40.7138, longitude: -74.0050) - ] - end - - it 'behaves like bulk processing' do - initial_count = Track.count - bulk_generator.call - # Bulk processing replaces existing tracks with new ones - # The final count depends on how many valid tracks can be created from the points - expect(Track.count).to be >= 0 - end - end - - context 'with incremental processing strategies' do - let(:incremental_loader) { Tracks::PointLoaders::IncrementalLoader.new(user) } - let(:buffer_handler) { Tracks::IncompleteSegmentHandlers::BufferHandler.new(user, Date.current, 5) } - let(:noop_cleaner) { Tracks::Cleaners::NoOpCleaner.new(user) } - - let(:incremental_generator) do - described_class.new( - user, - point_loader: incremental_loader, - incomplete_segment_handler: buffer_handler, - track_cleaner: noop_cleaner - ) - end - - let!(:existing_track) { create(:track, user: user) } - before do - # Mock the incremental loader to return some points - allow(incremental_loader).to receive(:load_points).and_return([]) + allow(safe_settings).to receive(:minutes_between_routes).and_return(45) end - it 'behaves like incremental processing' do - expect { incremental_generator.call }.not_to change { Track.count } + it 'creates separate tracks for segments' do + expect { generator.call }.to change(Track, :count).by(2) + end + end + + context 'with points exceeding distance threshold' do + let!(:points) do + [ + create_points_around(user: user, count: 2, base_lat: 29.0, timestamp: 20.minutes.ago.to_i), + create_points_around(user: user, count: 2, base_lat: 29.0, timestamp: 15.minutes.ago.to_i), + # Large distance jump 👇👇👇 + create_points_around(user: user, count: 2, base_lat: 28.0, timestamp: 10.minutes.ago.to_i), + create_points_around(user: user, count: 1, base_lat: 28.0, timestamp: Time.current.to_i) + ] + end + + before do + allow(safe_settings).to receive(:meters_between_routes).and_return(200) + end + + it 'creates separate tracks for segments' do + expect { generator.call }.to change(Track, :count).by(2) + end + end + end + + describe 'deterministic behavior' do + let!(:points) { create_points_around(user: user, count: 10, base_lat: 28.0) } + + it 'produces same results for bulk and incremental modes' do + # Generate tracks in bulk mode + bulk_generator = described_class.new(user, mode: :bulk) + bulk_generator.call + bulk_tracks = user.tracks.order(:start_at).to_a + + # Clear tracks and generate incrementally + user.tracks.destroy_all + incremental_generator = described_class.new(user, mode: :incremental) + incremental_generator.call + incremental_tracks = user.tracks.order(:start_at).to_a + + # Should have same number of tracks + expect(incremental_tracks.size).to eq(bulk_tracks.size) + + # Should have same track boundaries (allowing for small timing differences) + bulk_tracks.zip(incremental_tracks).each do |bulk_track, incremental_track| + expect(incremental_track.start_at).to be_within(1.second).of(bulk_track.start_at) + expect(incremental_track.end_at).to be_within(1.second).of(bulk_track.end_at) + expect(incremental_track.distance).to be_within(10).of(bulk_track.distance) end end end diff --git a/spec/services/tracks/incremental_processor_spec.rb b/spec/services/tracks/incremental_processor_spec.rb new file mode 100644 index 00000000..f3b66499 --- /dev/null +++ b/spec/services/tracks/incremental_processor_spec.rb @@ -0,0 +1,249 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Tracks::IncrementalProcessor do + let(:user) { create(:user) } + let(:safe_settings) { user.safe_settings } + + before do + allow(user).to receive(:safe_settings).and_return(safe_settings) + allow(safe_settings).to receive(:minutes_between_routes).and_return(30) + allow(safe_settings).to receive(:meters_between_routes).and_return(500) + end + + describe '#call' do + context 'with imported points' do + let(:imported_point) { create(:point, user: user, import: create(:import)) } + let(:processor) { described_class.new(user, imported_point) } + + it 'does not process imported points' do + expect(Tracks::CreateJob).not_to receive(:perform_later) + + processor.call + end + end + + context 'with first point for user' do + let(:new_point) { create(:point, user: user) } + let(:processor) { described_class.new(user, new_point) } + + it 'processes first point' do + expect(Tracks::CreateJob).to receive(:perform_later) + .with(user.id, start_at: nil, end_at: nil, mode: :none) + processor.call + end + end + + context 'with thresholds exceeded' do + let(:previous_point) { create(:point, user: user, timestamp: 1.hour.ago.to_i) } + let(:new_point) { create(:point, user: user, timestamp: Time.current.to_i) } + let(:processor) { described_class.new(user, new_point) } + + before do + # Create previous point first + previous_point + end + + it 'processes when time threshold exceeded' do + expect(Tracks::CreateJob).to receive(:perform_later) + .with(user.id, start_at: nil, end_at: Time.at(previous_point.timestamp), mode: :none) + processor.call + end + end + + context 'with existing tracks' do + let(:existing_track) { create(:track, user: user, end_at: 2.hours.ago) } + let(:previous_point) { create(:point, user: user, timestamp: 1.hour.ago.to_i) } + let(:new_point) { create(:point, user: user, timestamp: Time.current.to_i) } + let(:processor) { described_class.new(user, new_point) } + + before do + existing_track + previous_point + end + + it 'uses existing track end time as start_at' do + expect(Tracks::CreateJob).to receive(:perform_later) + .with(user.id, start_at: existing_track.end_at, end_at: Time.at(previous_point.timestamp), mode: :none) + processor.call + end + end + + context 'with distance threshold exceeded' do + let(:previous_point) do + create(:point, user: user, timestamp: 10.minutes.ago.to_i, lonlat: 'POINT(0 0)') + end + let(:new_point) do + create(:point, user: user, timestamp: Time.current.to_i, lonlat: 'POINT(1 1)') + end + let(:processor) { described_class.new(user, new_point) } + + before do + # Create previous point first + previous_point + # Mock distance calculation to exceed threshold + allow_any_instance_of(Point).to receive(:distance_to).and_return(1.0) # 1 km = 1000m + end + + it 'processes when distance threshold exceeded' do + expect(Tracks::CreateJob).to receive(:perform_later) + .with(user.id, start_at: nil, end_at: Time.at(previous_point.timestamp), mode: :none) + processor.call + end + end + + context 'with thresholds not exceeded' do + let(:previous_point) { create(:point, user: user, timestamp: 10.minutes.ago.to_i) } + let(:new_point) { create(:point, user: user, timestamp: Time.current.to_i) } + let(:processor) { described_class.new(user, new_point) } + + before do + # Create previous point first + previous_point + # Mock distance to be within threshold + allow_any_instance_of(Point).to receive(:distance_to).and_return(0.1) # 100m + end + + it 'does not process when thresholds not exceeded' do + expect(Tracks::CreateJob).not_to receive(:perform_later) + processor.call + end + end + end + + describe '#should_process?' do + let(:processor) { described_class.new(user, new_point) } + + context 'with imported point' do + let(:new_point) { create(:point, user: user, import: create(:import)) } + + it 'returns false' do + expect(processor.send(:should_process?)).to be false + end + end + + context 'with first point for user' do + let(:new_point) { create(:point, user: user) } + + it 'returns true' do + expect(processor.send(:should_process?)).to be true + end + end + + context 'with thresholds exceeded' do + let(:previous_point) { create(:point, user: user, timestamp: 1.hour.ago.to_i) } + let(:new_point) { create(:point, user: user, timestamp: Time.current.to_i) } + + before do + previous_point # Create previous point + end + + it 'returns true when time threshold exceeded' do + expect(processor.send(:should_process?)).to be true + end + end + + context 'with thresholds not exceeded' do + let(:previous_point) { create(:point, user: user, timestamp: 10.minutes.ago.to_i) } + let(:new_point) { create(:point, user: user, timestamp: Time.current.to_i) } + + before do + previous_point # Create previous point + allow_any_instance_of(Point).to receive(:distance_to).and_return(0.1) # 100m + end + + it 'returns false when thresholds not exceeded' do + expect(processor.send(:should_process?)).to be false + end + end + end + + describe '#exceeds_thresholds?' do + let(:processor) { described_class.new(user, new_point) } + let(:previous_point) { create(:point, user: user, timestamp: 1.hour.ago.to_i) } + let(:new_point) { create(:point, user: user, timestamp: Time.current.to_i) } + + context 'with time threshold exceeded' do + before do + allow(safe_settings).to receive(:minutes_between_routes).and_return(30) + end + + it 'returns true' do + result = processor.send(:exceeds_thresholds?, previous_point, new_point) + expect(result).to be true + end + end + + context 'with distance threshold exceeded' do + before do + allow(safe_settings).to receive(:minutes_between_routes).and_return(120) # 2 hours + allow(safe_settings).to receive(:meters_between_routes).and_return(400) + allow_any_instance_of(Point).to receive(:distance_to).and_return(0.5) # 500m + end + + it 'returns true' do + result = processor.send(:exceeds_thresholds?, previous_point, new_point) + expect(result).to be true + end + end + + context 'with neither threshold exceeded' do + before do + allow(safe_settings).to receive(:minutes_between_routes).and_return(120) # 2 hours + allow(safe_settings).to receive(:meters_between_routes).and_return(600) + allow_any_instance_of(Point).to receive(:distance_to).and_return(0.1) # 100m + end + + it 'returns false' do + result = processor.send(:exceeds_thresholds?, previous_point, new_point) + expect(result).to be false + end + end + end + + describe '#time_difference_minutes' do + let(:processor) { described_class.new(user, new_point) } + let(:point1) { create(:point, user: user, timestamp: 1.hour.ago.to_i) } + let(:point2) { create(:point, user: user, timestamp: Time.current.to_i) } + let(:new_point) { point2 } + + it 'calculates time difference in minutes' do + result = processor.send(:time_difference_minutes, point1, point2) + expect(result).to be_within(1).of(60) # Approximately 60 minutes + end + end + + describe '#distance_difference_meters' do + let(:processor) { described_class.new(user, new_point) } + let(:point1) { create(:point, user: user) } + let(:point2) { create(:point, user: user) } + let(:new_point) { point2 } + + before do + allow(point1).to receive(:distance_to).with(point2).and_return(1.5) # 1.5 km + end + + it 'calculates distance difference in meters' do + result = processor.send(:distance_difference_meters, point1, point2) + expect(result).to eq(1500) # 1.5 km = 1500 m + end + end + + describe 'threshold configuration' do + let(:processor) { described_class.new(user, create(:point, user: user)) } + + before do + allow(safe_settings).to receive(:minutes_between_routes).and_return(45) + allow(safe_settings).to receive(:meters_between_routes).and_return(750) + end + + it 'uses configured time threshold' do + expect(processor.send(:time_threshold_minutes)).to eq(45) + end + + it 'uses configured distance threshold' do + expect(processor.send(:distance_threshold_meters)).to eq(750) + end + end +end diff --git a/spec/services/tracks/redis_buffer_spec.rb b/spec/services/tracks/redis_buffer_spec.rb deleted file mode 100644 index e50ab4cc..00000000 --- a/spec/services/tracks/redis_buffer_spec.rb +++ /dev/null @@ -1,238 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Tracks::RedisBuffer do - let(:user_id) { 123 } - let(:day) { Date.current } - let(:buffer) { described_class.new(user_id, day) } - - describe '#initialize' do - it 'stores user_id and converts day to Date' do - expect(buffer.user_id).to eq(user_id) - expect(buffer.day).to eq(day) - expect(buffer.day).to be_a(Date) - end - - it 'handles string date input' do - buffer = described_class.new(user_id, '2024-01-15') - expect(buffer.day).to eq(Date.parse('2024-01-15')) - end - - it 'handles Time input' do - time = Time.current - buffer = described_class.new(user_id, time) - expect(buffer.day).to eq(time.to_date) - end - end - - describe '#store' do - let(:user) { create(:user) } - let!(:points) do - [ - create(:point, user: user, lonlat: 'POINT(-74.0060 40.7128)', timestamp: 1.hour.ago.to_i), - create(:point, user: user, lonlat: 'POINT(-74.0070 40.7130)', timestamp: 30.minutes.ago.to_i) - ] - end - - it 'stores points in Redis cache' do - expect(Rails.cache).to receive(:write).with( - "track_buffer:#{user_id}:#{day.strftime('%Y-%m-%d')}", - anything, - expires_in: 7.days - ) - - buffer.store(points) - end - - it 'serializes points correctly' do - buffer.store(points) - - stored_data = Rails.cache.read("track_buffer:#{user_id}:#{day.strftime('%Y-%m-%d')}") - - expect(stored_data).to be_an(Array) - expect(stored_data.size).to eq(2) - - first_point = stored_data.first - expect(first_point[:id]).to eq(points.first.id) - expect(first_point[:timestamp]).to eq(points.first.timestamp) - expect(first_point[:lat]).to eq(points.first.lat) - expect(first_point[:lon]).to eq(points.first.lon) - expect(first_point[:user_id]).to eq(points.first.user_id) - end - - it 'does nothing when given empty array' do - expect(Rails.cache).not_to receive(:write) - buffer.store([]) - end - - it 'logs debug message when storing points' do - expect(Rails.logger).to receive(:debug).with( - "Stored 2 points in buffer for user #{user_id}, day #{day}" - ) - - buffer.store(points) - end - end - - describe '#retrieve' do - context 'when buffer exists' do - let(:stored_data) do - [ - { - id: 1, - lonlat: 'POINT(-74.0060 40.7128)', - timestamp: 1.hour.ago.to_i, - lat: 40.7128, - lon: -74.0060, - altitude: 100, - velocity: 5.0, - battery: 80, - user_id: user_id - }, - { - id: 2, - lonlat: 'POINT(-74.0070 40.7130)', - timestamp: 30.minutes.ago.to_i, - lat: 40.7130, - lon: -74.0070, - altitude: 105, - velocity: 6.0, - battery: 75, - user_id: user_id - } - ] - end - - before do - Rails.cache.write( - "track_buffer:#{user_id}:#{day.strftime('%Y-%m-%d')}", - stored_data - ) - end - - it 'returns the stored point data' do - result = buffer.retrieve - - expect(result).to eq(stored_data) - expect(result.size).to eq(2) - end - end - - context 'when buffer does not exist' do - it 'returns empty array' do - result = buffer.retrieve - expect(result).to eq([]) - end - end - - context 'when Redis read fails' do - before do - allow(Rails.cache).to receive(:read).and_raise(StandardError.new('Redis error')) - end - - it 'returns empty array and logs error' do - expect(Rails.logger).to receive(:error).with( - "Failed to retrieve buffered points for user #{user_id}, day #{day}: Redis error" - ) - - result = buffer.retrieve - expect(result).to eq([]) - end - end - end - - describe '#clear' do - before do - Rails.cache.write( - "track_buffer:#{user_id}:#{day.strftime('%Y-%m-%d')}", - [{ id: 1, timestamp: 1.hour.ago.to_i }] - ) - end - - it 'deletes the buffer from cache' do - buffer.clear - - expect(Rails.cache.read("track_buffer:#{user_id}:#{day.strftime('%Y-%m-%d')}")).to be_nil - end - - it 'logs debug message' do - expect(Rails.logger).to receive(:debug).with( - "Cleared buffer for user #{user_id}, day #{day}" - ) - - buffer.clear - end - end - - describe '#exists?' do - context 'when buffer exists' do - before do - Rails.cache.write( - "track_buffer:#{user_id}:#{day.strftime('%Y-%m-%d')}", - [{ id: 1 }] - ) - end - - it 'returns true' do - expect(buffer.exists?).to be true - end - end - - context 'when buffer does not exist' do - it 'returns false' do - expect(buffer.exists?).to be false - end - end - end - - describe 'buffer key generation' do - it 'generates correct Redis key format' do - expected_key = "track_buffer:#{user_id}:#{day.strftime('%Y-%m-%d')}" - - # Access private method for testing - actual_key = buffer.send(:buffer_key) - - expect(actual_key).to eq(expected_key) - end - - it 'handles different date formats consistently' do - date_as_string = '2024-03-15' - date_as_date = Date.parse(date_as_string) - - buffer1 = described_class.new(user_id, date_as_string) - buffer2 = described_class.new(user_id, date_as_date) - - expect(buffer1.send(:buffer_key)).to eq(buffer2.send(:buffer_key)) - end - end - - describe 'integration test' do - let(:user) { create(:user) } - let!(:points) do - [ - create(:point, user: user, lonlat: 'POINT(-74.0060 40.7128)', timestamp: 2.hours.ago.to_i), - create(:point, user: user, lonlat: 'POINT(-74.0070 40.7130)', timestamp: 1.hour.ago.to_i) - ] - end - - it 'stores and retrieves points correctly' do - # Store points - buffer.store(points) - expect(buffer.exists?).to be true - - # Retrieve points - retrieved_points = buffer.retrieve - expect(retrieved_points.size).to eq(2) - - # Verify data integrity - expect(retrieved_points.first[:id]).to eq(points.first.id) - expect(retrieved_points.last[:id]).to eq(points.last.id) - - # Clear buffer - buffer.clear - expect(buffer.exists?).to be false - expect(buffer.retrieve).to eq([]) - end - end -end diff --git a/spec/services/tracks/track_builder_spec.rb b/spec/services/tracks/track_builder_spec.rb index 0c0b4d26..5046e60f 100644 --- a/spec/services/tracks/track_builder_spec.rb +++ b/spec/services/tracks/track_builder_spec.rb @@ -116,11 +116,11 @@ RSpec.describe Tracks::TrackBuilder do it 'builds path using Tracks::BuildPath service' do expect(Tracks::BuildPath).to receive(:new).with( - points.map(&:lonlat) + points ).and_call_original result = builder.build_path(points) - expect(result).to respond_to(:as_text) # RGeo geometry object + expect(result).to respond_to(:as_text) end end diff --git a/spec/support/point_helpers.rb b/spec/support/point_helpers.rb new file mode 100644 index 00000000..3e6b45c7 --- /dev/null +++ b/spec/support/point_helpers.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module PointHelpers + # Creates a list of points spaced ~100m apart northwards + def create_points_around(user:, count:, base_lat: 20.0, base_lon: 10.0, timestamp: nil, **attrs) + Array.new(count) do |i| + create( + :point, + user: user, + timestamp: (timestamp.respond_to?(:call) ? timestamp.call(i) : timestamp) || (Time.current - i.minutes).to_i, + lonlat: "POINT(#{base_lon} #{base_lat + i * 0.0009})", + **attrs + ) + end + end +end + +RSpec.configure do |config| + config.include PointHelpers +end From eca09ce3eb04c81a7d5a702882f0a6c52099c216 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Wed, 16 Jul 2025 22:25:50 +0200 Subject: [PATCH 08/30] Remove bulk generator job --- app/jobs/tracks/bulk_generator_job.rb | 45 --------------------------- docs/TRACKS_OVERVIEW.md | 3 -- 2 files changed, 48 deletions(-) delete mode 100644 app/jobs/tracks/bulk_generator_job.rb diff --git a/app/jobs/tracks/bulk_generator_job.rb b/app/jobs/tracks/bulk_generator_job.rb deleted file mode 100644 index a76970c2..00000000 --- a/app/jobs/tracks/bulk_generator_job.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -# Background job for bulk track generation. -# -# This job regenerates all tracks for a user from scratch, typically used for: -# - Initial track generation after data import -# - Full recalculation when settings change -# - Manual track regeneration requested by user -# -# The job uses the new simplified Tracks::Generator service with bulk mode, -# which cleans existing tracks and regenerates everything from points. -# -# Parameters: -# - user_id: The user whose tracks should be generated -# - start_at: Optional start timestamp to limit processing -# - end_at: Optional end timestamp to limit processing -# -class Tracks::BulkGeneratorJob < ApplicationJob - queue_as :default - - def perform(user_id, start_at: nil, end_at: nil) - user = User.find(user_id) - - Rails.logger.info "Starting bulk track generation for user #{user_id}, " \ - "start_at: #{start_at}, end_at: #{end_at}" - - generator = Tracks::Generator.new( - user, - start_at: start_at, - end_at: end_at, - mode: :bulk - ) - - generator.call - - Rails.logger.info "Completed bulk track generation for user #{user_id}" - rescue ActiveRecord::RecordNotFound => e - Rails.logger.error "Record not found in bulk track generation: #{e.message}" - # Don't retry if records are missing - rescue StandardError => e - Rails.logger.error "Error in bulk track generation for user #{user_id}: #{e.message}" - Rails.logger.error e.backtrace.join("\n") - raise # Re-raise for job retry logic - end -end \ No newline at end of file diff --git a/docs/TRACKS_OVERVIEW.md b/docs/TRACKS_OVERVIEW.md index 1874bc0e..5c4e5ca2 100644 --- a/docs/TRACKS_OVERVIEW.md +++ b/docs/TRACKS_OVERVIEW.md @@ -249,9 +249,6 @@ point = Point.create!( ### 3. Background Job Management ```ruby -# Enqueue bulk processing -Tracks::BulkGeneratorJob.perform_later(user.id) - # Enqueue incremental check (automatically triggered by point creation) Tracks::IncrementalCheckJob.perform_later(user.id, point.id) From 10777714b107f9abe8fc3ab5271128f3e5ab78c5 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 17 Jul 2025 19:19:50 +0200 Subject: [PATCH 09/30] Clean up a bit --- app/jobs/tracks/cleanup_job.rb | 5 ----- app/services/tracks/generator.rb | 21 +++++++------------ ...0250704185707_create_tracks_from_points.rb | 3 +-- 3 files changed, 9 insertions(+), 20 deletions(-) diff --git a/app/jobs/tracks/cleanup_job.rb b/app/jobs/tracks/cleanup_job.rb index f9dc9c4e..82eae62d 100644 --- a/app/jobs/tracks/cleanup_job.rb +++ b/app/jobs/tracks/cleanup_job.rb @@ -1,11 +1,6 @@ # frozen_string_literal: true # Lightweight cleanup job that runs weekly to catch any missed track generation. -# This replaces the daily bulk creation job with a more targeted approach. -# -# Instead of processing all users daily, this job only processes users who have -# untracked points that are older than a threshold (e.g., 1 day), indicating -# they may have been missed by incremental processing. # # This provides a safety net while avoiding the overhead of daily bulk processing. class Tracks::CleanupJob < ApplicationJob diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index ac599b59..765253a8 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -# Simplified track generation service that replaces the complex strategy pattern. -# # This service handles both bulk and incremental track generation using a unified # approach with different modes: # @@ -9,10 +7,6 @@ # - :incremental - Processes untracked points up to a specified end time # - :daily - Processes tracks on a daily basis # -# The service maintains the same core logic as the original system but simplifies -# the architecture by removing the multiple strategy classes in favor of -# mode-based configuration. -# # Key features: # - Deterministic results (same algorithm for all modes) # - Simple incremental processing without buffering complexity @@ -62,9 +56,7 @@ class Tracks::Generator def should_clean_tracks? case mode - when :bulk then true - when :daily then true - when :incremental then false + when :bulk, :daily then true else false end end @@ -82,6 +74,7 @@ class Tracks::Generator def load_bulk_points scope = user.tracked_points.order(:timestamp) scope = scope.where(timestamp: time_range) if time_range_defined? + scope end @@ -90,11 +83,13 @@ class Tracks::Generator # If end_at is specified, only process points up to that time scope = user.tracked_points.where(track_id: nil).order(:timestamp) scope = scope.where(timestamp: ..end_at.to_i) if end_at.present? + scope end def load_daily_points day_range = daily_time_range + user.tracked_points.where(timestamp: day_range).order(:timestamp) end @@ -128,10 +123,10 @@ class Tracks::Generator def clean_existing_tracks case mode - when :bulk - clean_bulk_tracks - when :daily - clean_daily_tracks + when :bulk then clean_bulk_tracks + when :daily then clean_daily_tracks + else + raise ArgumentError, "Unknown mode: #{mode}" end end diff --git a/db/data/20250704185707_create_tracks_from_points.rb b/db/data/20250704185707_create_tracks_from_points.rb index fd744de9..7d5cffb5 100644 --- a/db/data/20250704185707_create_tracks_from_points.rb +++ b/db/data/20250704185707_create_tracks_from_points.rb @@ -15,12 +15,11 @@ class CreateTracksFromPoints < ActiveRecord::Migration[8.0] # Use explicit parameters for bulk historical processing: # - No time limits (start_at: nil, end_at: nil) = process ALL historical data - # - Replace strategy = clean slate, removes any existing tracks first Tracks::CreateJob.perform_later( user.id, start_at: nil, end_at: nil, - mode: :daily + mode: :bulk ) processed_users += 1 From 1f5325d9bbb4b03a6f00b7a59ab7240788f514e1 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 17 Jul 2025 19:22:50 +0200 Subject: [PATCH 10/30] Remove doc file --- docs/TRACKS_OVERVIEW.md | 480 ---------------------------------------- 1 file changed, 480 deletions(-) delete mode 100644 docs/TRACKS_OVERVIEW.md diff --git a/docs/TRACKS_OVERVIEW.md b/docs/TRACKS_OVERVIEW.md deleted file mode 100644 index 5c4e5ca2..00000000 --- a/docs/TRACKS_OVERVIEW.md +++ /dev/null @@ -1,480 +0,0 @@ -# Dawarich Tracks Feature Overview - -## Table of Contents -- [Introduction](#introduction) -- [Architecture Overview](#architecture-overview) -- [Core Components](#core-components) -- [Data Flow](#data-flow) -- [Configuration](#configuration) -- [Usage Examples](#usage-examples) -- [API Reference](#api-reference) -- [Development Guidelines](#development-guidelines) - -## Introduction - -The Dawarich Tracks feature automatically converts raw GPS points into meaningful movement tracks. It analyzes sequences of location points to identify distinct journeys, providing users with structured visualizations of their movement patterns. - -### Key Features -- **Automatic Track Generation**: Converts GPS points into coherent movement tracks -- **Real-time Processing**: Incremental track generation as new points arrive -- **Configurable Thresholds**: User-customizable time and distance parameters -- **Multiple Generation Modes**: Bulk, incremental, and daily processing -- **Rich Statistics**: Distance, speed, elevation, and duration metrics -- **Live Updates**: Real-time track updates via WebSocket connections - -## Architecture Overview - -```mermaid -graph TB - A[GPS Points] --> B[Incremental Processor] - B --> C[Threshold Check] - C --> D{Exceeds Thresholds?} - D -->|Yes| E[Tracks Generator] - D -->|No| F[Skip Processing] - E --> G[Segmentation Engine] - G --> H[Track Builder] - H --> I[Database] - I --> J[Real-time Broadcasting] - J --> K[Frontend Updates] -``` - -## Core Components - -### 1. Models - -#### Track Model -```ruby -# app/models/track.rb -class Track < ApplicationRecord - belongs_to :user - has_many :points, dependent: :nullify - - # Attributes - # start_at, end_at (DateTime) - # distance (Integer, meters) - # avg_speed (Float, km/h) - # duration (Integer, seconds) - # elevation_gain/loss/max/min (Integer, meters) - # original_path (PostGIS LineString) -end -``` - -#### Point Model -```ruby -# app/models/point.rb -class Point < ApplicationRecord - belongs_to :track, optional: true - belongs_to :user - - # Triggers incremental track generation via background job - after_create_commit :trigger_incremental_track_generation - - private - - def trigger_incremental_track_generation - Tracks::IncrementalCheckJob.perform_later(user.id, id) - end -end -``` - -### 2. Services - -#### Tracks::Generator -**Purpose**: Unified track generation service with multiple modes - -```ruby -# Usage -Tracks::Generator.new(user, mode: :bulk).call -Tracks::Generator.new(user, mode: :incremental, end_at: Time.current).call -Tracks::Generator.new(user, mode: :daily, start_at: Date.current).call -``` - -**Modes**: -- `:bulk` - Regenerates all tracks from scratch (replaces existing) -- `:incremental` - Processes only untracked points up to specified time -- `:daily` - Processes tracks on daily basis with cleanup - -#### Tracks::IncrementalProcessor -**Purpose**: Analyzes new points and triggers track generation when thresholds are exceeded - -```ruby -# Automatically called when new points are created -Tracks::IncrementalProcessor.new(user, new_point).call -``` - -#### Tracks::Segmentation -**Purpose**: Core algorithm for splitting GPS points into meaningful segments - -**Criteria**: -- **Time threshold**: Configurable minutes gap (default: 30 minutes) -- **Distance threshold**: Configurable meters jump (default: 500 meters) -- **Minimum segment size**: 2 points required for valid track - -#### Tracks::TrackBuilder -**Purpose**: Converts point arrays into Track records with calculated statistics - -**Statistics Calculated**: -- **Distance**: Always stored in meters as integers -- **Duration**: Total time in seconds between first and last point -- **Average Speed**: Calculated in km/h regardless of user preference -- **Elevation Metrics**: Gain, loss, maximum, minimum in meters - -### 3. Background Jobs - -#### Tracks::IncrementalCheckJob -- **Purpose**: Lightweight job triggered by point creation -- **Queue**: `tracks` for dedicated processing -- **Trigger**: Automatically enqueued when non-import points are created -- **Function**: Checks thresholds and conditionally triggers track generation - -#### Tracks::CreateJob -- **Purpose**: Main orchestration job for track creation -- **Features**: User notifications on success/failure -- **Incremental Usage**: Enqueued by IncrementalCheckJob when thresholds are exceeded -- **Parameters**: `user_id`, `start_at`, `end_at`, `mode` - -#### Tracks::CleanupJob -- **Purpose**: Weekly cleanup of missed track generation -- **Schedule**: Runs weekly on Sunday at 02:00 via cron -- **Strategy**: Processes only users with old untracked points (1+ days old) - -### 4. Real-time Features - -#### TracksChannel (ActionCable) -```javascript -// Real-time track updates -consumer.subscriptions.create("TracksChannel", { - received(data) { - // Handle track created/updated/destroyed events - } -}); -``` - -## Data Flow - -### 1. Point Creation Flow -``` -New Point Created → IncrementalCheckJob → Incremental Processor → Threshold Check → -(if exceeded) → CreateJob → Track Generation → Database Update → -User Notification → Real-time Broadcast → Frontend Update -``` - -### 2. Bulk Processing Flow -``` -Scheduled Job → Load Historical Points → Segmentation → -Track Creation → Statistics Calculation → Database Batch Update -``` - -### 3. Incremental Processing Flow -``` -New Point → IncrementalCheckJob → Find Previous Point → Calculate Time/Distance Gaps → -(if thresholds exceeded) → CreateJob(start_at: last_track_end, end_at: previous_point_time) → -Process Untracked Points → Create Tracks → User Notification -``` - -## Configuration - -### User Settings -Tracks behavior is controlled by user-configurable settings in `Users::SafeSettings`: - -```ruby -# Default values -{ - 'meters_between_routes' => 500, # Distance threshold - 'minutes_between_routes' => 30, # Time threshold - 'route_opacity' => 60, # Visual opacity - 'distance_unit' => 'km' # Display unit (km/mi) -} -``` - -### Threshold Configuration -```ruby -# Time threshold: Gap longer than X minutes = new track -user.safe_settings.minutes_between_routes # default: 30 - -# Distance threshold: Jump larger than X meters = new track -user.safe_settings.meters_between_routes # default: 500 - -# Access in services -def time_threshold_minutes - user.safe_settings.minutes_between_routes.to_i -end -``` - -### Background Job Schedule -```yaml -# config/schedule.yml -tracks_cleanup_job: - cron: '0 2 * * 0' # Weekly on Sunday at 02:00 - class: Tracks::CleanupJob -``` - -## Usage Examples - -### 1. Manual Track Generation - -```ruby -# Bulk regeneration (replaces all existing tracks) -Tracks::Generator.new(user, mode: :bulk).call - -# Process specific date range -Tracks::Generator.new( - user, - start_at: 1.week.ago, - end_at: Time.current, - mode: :bulk -).call - -# Daily processing -Tracks::Generator.new( - user, - start_at: Date.current, - mode: :daily -).call -``` - -### 2. Incremental Processing - -```ruby -# Triggered automatically when points are created -point = Point.create!( - user: user, - timestamp: Time.current.to_i, - lonlat: 'POINT(-122.4194 37.7749)' -) -# → Automatically enqueues IncrementalCheckJob -# → Job checks thresholds and conditionally triggers track generation -``` - -### 3. Background Job Management - -```ruby -# Enqueue incremental check (automatically triggered by point creation) -Tracks::IncrementalCheckJob.perform_later(user.id, point.id) - -# Enqueue incremental processing (triggered by IncrementalCheckJob) -Tracks::CreateJob.perform_later( - user.id, - start_at: last_track_end, - end_at: previous_point_timestamp, - mode: :none -) - -# Run cleanup for missed tracks -Tracks::CleanupJob.perform_later(older_than: 1.day.ago) - -# Create tracks with notifications -Tracks::CreateJob.perform_later(user.id, start_at: nil, end_at: nil, mode: :bulk) -``` - -### 4. Frontend Integration - -```javascript -// Initialize tracks on map -const tracksLayer = new TracksLayer(map, tracksData); - -// Handle real-time updates -consumer.subscriptions.create("TracksChannel", { - received(data) { - switch(data.event) { - case 'created': - tracksLayer.addTrack(data.track); - break; - case 'updated': - tracksLayer.updateTrack(data.track); - break; - case 'destroyed': - tracksLayer.removeTrack(data.track.id); - break; - } - } -}); -``` - -## API Reference - -### Track Model API - -```ruby -# Key methods -track.formatted_distance # Distance in user's preferred unit -track.distance_in_unit(unit) # Distance in specific unit -track.recalculate_path_and_distance! # Recalculate from points - -# Scopes -Track.for_user(user) -Track.between_dates(start_date, end_date) -Track.last_for_day(user, date) -``` - -### TrackSerializer Output -```json -{ - "id": 123, - "start_at": "2023-01-01T10:00:00Z", - "end_at": "2023-01-01T11:30:00Z", - "distance": 5000, - "avg_speed": 25.5, - "duration": 5400, - "elevation_gain": 150, - "elevation_loss": 100, - "elevation_max": 300, - "elevation_min": 200, - "path": "LINESTRING(...)" -} -``` - -### Service APIs - -```ruby -# Generator API -generator = Tracks::Generator.new(user, options) -generator.call # Returns nil, tracks saved to database - -# Processor API -processor = Tracks::IncrementalProcessor.new(user, point) -processor.call # May enqueue background job - -# Segmentation API (via inclusion) -segments = split_points_into_segments(points) -should_start_new_segment?(current_point, previous_point) -``` - -## Development Guidelines - -### 1. Adding New Generation Modes - -```ruby -# In Tracks::Generator -def load_points - case mode - when :bulk - load_bulk_points - when :incremental - load_incremental_points - when :daily - load_daily_points - when :custom_mode # New mode - load_custom_points - end -end - -def should_clean_tracks? - case mode - when :bulk, :daily then true - when :incremental, :custom_mode then false - end -end -``` - -### 2. Customizing Segmentation Logic - -```ruby -# Override in including class -def should_start_new_segment?(current_point, previous_point) - # Custom logic here - super || custom_condition?(current_point, previous_point) -end -``` - -### 3. Testing Patterns - -```ruby -# Test track generation -expect { generator.call }.to change(Track, :count).by(1) - -# Test point callback -expect { point.save! }.to have_enqueued_job(Tracks::IncrementalCheckJob) - .with(user.id, point.id) - -# Test incremental processing -expect(Tracks::CreateJob).to receive(:perform_later) - .with(user.id, start_at: anything, end_at: anything, mode: :none) -processor.call - -# Test segmentation -segments = generator.send(:segment_points, points) -expect(segments.size).to eq(2) -``` - -### 4. Performance Considerations - -- **Batch Processing**: Use `find_in_batches` for large datasets -- **Database Indexes**: Ensure proper indexing on `timestamp` and `track_id` -- **Memory Usage**: Process points in chunks for very large datasets -- **Asynchronous Processing**: Point creation is never blocked by track generation -- **Job Queue Management**: Monitor job queue performance for incremental processing - -### 5. Error Handling - -```ruby -# In services -begin - generator.call -rescue StandardError => e - Rails.logger.error "Track generation failed: #{e.message}" - # Handle gracefully -end - -# In jobs -def perform(*args) - # Main logic -rescue ActiveRecord::RecordNotFound - # Don't retry for missing records -rescue StandardError => e - Rails.logger.error "Job failed: #{e.message}" - raise # Re-raise for retry logic -end -``` - -### 6. Monitoring and Debugging - -```ruby -# Add logging -Rails.logger.info "Generated #{segments.size} tracks for user #{user.id}" - -# Performance monitoring -Rails.logger.info "Track generation took #{duration}ms" - -# Debug segmentation -Rails.logger.debug "Threshold check: time=#{time_gap}min, distance=#{distance_gap}m" -``` - -## Best Practices - -1. **Data Consistency**: Always store distances in meters, convert only for display -2. **Threshold Configuration**: Make thresholds user-configurable for flexibility -3. **Error Handling**: Gracefully handle missing data and network issues -4. **Performance**: Use database queries efficiently, avoid N+1 queries -5. **Testing**: Test all modes and edge cases thoroughly -6. **Real-time Updates**: Use ActionCable for responsive user experience -7. **Background Processing**: Use appropriate queues for different job priorities -8. **Asynchronous Design**: Never block point creation with track generation logic -9. **Job Monitoring**: Monitor background job performance and failure rates - -## Troubleshooting - -### Common Issues - -1. **Missing Tracks**: Check if points have `track_id: nil` for incremental processing -2. **Incorrect Thresholds**: Verify user settings configuration -3. **Job Failures**: Check background job logs for errors -4. **Real-time Updates**: Verify WebSocket connection and channel subscriptions -5. **Performance Issues**: Monitor database query performance and indexing - -### Debugging Tools - -```ruby -# Check track generation -user.tracked_points.where(track_id: nil).count # Untracked points - -# Verify thresholds -user.safe_settings.minutes_between_routes -user.safe_settings.meters_between_routes - -# Test segmentation -generator = Tracks::Generator.new(user, mode: :bulk) -segments = generator.send(:segment_points, points) -``` - -This overview provides a comprehensive understanding of the Dawarich Tracks feature, from high-level architecture to specific implementation details. From f5ef2ab9ef835e2e7a9738428d4fbb5917e4b0d6 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 17 Jul 2025 20:20:14 +0200 Subject: [PATCH 11/30] Fix potential issue with time range data types --- app/services/tracks/generator.rb | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index 765253a8..9eb90122 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -73,7 +73,7 @@ class Tracks::Generator def load_bulk_points scope = user.tracked_points.order(:timestamp) - scope = scope.where(timestamp: time_range) if time_range_defined? + scope = scope.where(timestamp: timestamp_range) if time_range_defined? scope end @@ -109,7 +109,31 @@ class Tracks::Generator def time_range return nil unless time_range_defined? - Time.at(start_at&.to_i)..Time.at(end_at&.to_i) + start_time = start_at&.to_i + end_time = end_at&.to_i + + if start_time && end_time + Time.zone.at(start_time)..Time.zone.at(end_time) + elsif start_time + Time.zone.at(start_time).. + elsif end_time + ..Time.zone.at(end_time) + end + end + + def timestamp_range + return nil unless time_range_defined? + + start_time = start_at&.to_i + end_time = end_at&.to_i + + if start_time && end_time + start_time..end_time + elsif start_time + start_time.. + elsif end_time + ..end_time + end end def daily_time_range From 91f4cf7c7a7013e17b374ab60815b2b1c382163d Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 17 Jul 2025 20:36:21 +0200 Subject: [PATCH 12/30] Fix range objects in generator --- app/services/tracks/generator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index 9eb90122..ebb47873 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -163,8 +163,8 @@ class Tracks::Generator end def clean_daily_tracks - day_range_times = daily_time_range.map { |timestamp| Time.at(timestamp) } - range = Range.new(day_range_times.first, day_range_times.last) + day_range = daily_time_range + range = Time.zone.at(day_range.begin)..Time.zone.at(day_range.end) deleted_count = user.tracks.where(start_at: range).delete_all Rails.logger.info "Deleted #{deleted_count} daily tracks for user #{user.id}" From dc8460a948a2a13e800d29b8bf717750139f385c Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 17 Jul 2025 20:46:07 +0200 Subject: [PATCH 13/30] Fix tracks create job spec --- app/jobs/tracks/create_job.rb | 13 +++----- app/services/tracks/generator.rb | 11 +++++-- spec/jobs/tracks/create_job_spec.rb | 51 ++++++++++++++++++++++------- 3 files changed, 52 insertions(+), 23 deletions(-) diff --git a/app/jobs/tracks/create_job.rb b/app/jobs/tracks/create_job.rb index 514a6ac9..a57a3f79 100644 --- a/app/jobs/tracks/create_job.rb +++ b/app/jobs/tracks/create_job.rb @@ -5,26 +5,21 @@ class Tracks::CreateJob < ApplicationJob def perform(user_id, start_at: nil, end_at: nil, mode: :daily) user = User.find(user_id) - + # Translate mode parameter to Generator mode generator_mode = case mode when :daily then :daily when :none then :incremental else :bulk end - - # Count tracks before generation - tracks_before = user.tracks.count - - Tracks::Generator.new( + + # Generate tracks and get the count of tracks created + tracks_created = Tracks::Generator.new( user, start_at: start_at, end_at: end_at, mode: generator_mode ).call - - # Calculate tracks created - tracks_created = user.tracks.count - tracks_before create_success_notification(user, tracks_created) rescue StandardError => e diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index ebb47873..4191a286 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -42,14 +42,19 @@ class Tracks::Generator points = load_points Rails.logger.debug "Generator: loaded #{points.size} points for user #{user.id} in #{mode} mode" - return if points.empty? + return 0 if points.empty? segments = split_points_into_segments(points) Rails.logger.debug "Generator: created #{segments.size} segments" - segments.each { |segment| create_track_from_segment(segment) } + tracks_created = 0 + segments.each do |segment| + track = create_track_from_segment(segment) + tracks_created += 1 if track + end - Rails.logger.info "Generated #{segments.size} tracks for user #{user.id} in #{mode} mode" + Rails.logger.info "Generated #{tracks_created} tracks for user #{user.id} in #{mode} mode" + tracks_created end private diff --git a/spec/jobs/tracks/create_job_spec.rb b/spec/jobs/tracks/create_job_spec.rb index fd772609..69a47fa2 100644 --- a/spec/jobs/tracks/create_job_spec.rb +++ b/spec/jobs/tracks/create_job_spec.rb @@ -17,11 +17,9 @@ RSpec.describe Tracks::CreateJob, type: :job do end it 'calls the generator and creates a notification' do - # Mock the generator to actually create tracks - allow(generator_instance).to receive(:call) do - create_list(:track, 2, user: user) - end - + # Mock the generator to return the count of tracks created + allow(generator_instance).to receive(:call).and_return(2) + described_class.new.perform(user.id) expect(Tracks::Generator).to have_received(:new).with( @@ -53,12 +51,9 @@ RSpec.describe Tracks::CreateJob, type: :job do end it 'passes custom parameters to the generator' do - # Create some existing tracks and mock generator to create 1 more - create_list(:track, 5, user: user) - allow(generator_instance).to receive(:call) do - create(:track, user: user) - end - + # Mock generator to return the count of tracks created + allow(generator_instance).to receive(:call).and_return(1) + described_class.new.perform(user.id, start_at: start_at, end_at: end_at, mode: mode) expect(Tracks::Generator).to have_received(:new).with( @@ -87,6 +82,8 @@ RSpec.describe Tracks::CreateJob, type: :job do end it 'translates :none to :incremental' do + allow(generator_instance).to receive(:call).and_return(0) + described_class.new.perform(user.id, mode: :none) expect(Tracks::Generator).to have_received(:new).with( @@ -104,6 +101,8 @@ RSpec.describe Tracks::CreateJob, type: :job do end it 'translates :daily to :daily' do + allow(generator_instance).to receive(:call).and_return(0) + described_class.new.perform(user.id, mode: :daily) expect(Tracks::Generator).to have_received(:new).with( @@ -121,6 +120,8 @@ RSpec.describe Tracks::CreateJob, type: :job do end it 'translates other modes to :bulk' do + allow(generator_instance).to receive(:call).and_return(0) + described_class.new.perform(user.id, mode: :replace) expect(Tracks::Generator).to have_received(:new).with( @@ -185,6 +186,34 @@ RSpec.describe Tracks::CreateJob, type: :job do expect(ExceptionReporter).to have_received(:call) end end + + context 'when tracks are deleted and recreated' do + it 'returns the correct count of newly created tracks' do + # Create some existing tracks first + create_list(:track, 3, user: user) + + # Mock the generator to simulate deleting existing tracks and creating new ones + # This should return the count of newly created tracks, not the difference + allow(generator_instance).to receive(:call).and_return(2) + + described_class.new.perform(user.id, mode: :bulk) + + expect(Tracks::Generator).to have_received(:new).with( + user, + start_at: nil, + end_at: nil, + mode: :bulk + ) + expect(generator_instance).to have_received(:call) + expect(Notifications::Create).to have_received(:new).with( + user: user, + kind: :info, + title: 'Tracks Generated', + content: 'Created 2 tracks from your location data. Check your tracks section to view them.' + ) + expect(notification_service).to have_received(:call) + end + end end describe 'queue' do From 7cdb7d2f21cb2d67eb9c060dae04121856f38241 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 17 Jul 2025 20:57:55 +0200 Subject: [PATCH 14/30] Add some more tests to make sure points are properly cleaned up --- app/services/tracks/generator.rb | 9 +++-- app/services/tracks/incremental_processor.rb | 2 +- spec/services/tracks/generator_spec.rb | 36 +++++++++++++++++++ .../tracks/incremental_processor_spec.rb | 6 ++-- spec/services/users/import_data_spec.rb | 2 +- 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index 4191a286..45610a50 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -163,15 +163,18 @@ class Tracks::Generator scope = user.tracks scope = scope.where(start_at: time_range) if time_range_defined? - deleted_count = scope.delete_all - Rails.logger.info "Deleted #{deleted_count} existing tracks for user #{user.id}" + deleted_count = scope.destroy_all + + Rails.logger.info "Deleted #{deleted_count} existing tracks for user #{user.id}" end def clean_daily_tracks day_range = daily_time_range range = Time.zone.at(day_range.begin)..Time.zone.at(day_range.end) - deleted_count = user.tracks.where(start_at: range).delete_all + scope = user.tracks.where(start_at: range) + deleted_count = scope.destroy_all + Rails.logger.info "Deleted #{deleted_count} daily tracks for user #{user.id}" end diff --git a/app/services/tracks/incremental_processor.rb b/app/services/tracks/incremental_processor.rb index 1d714e9e..3f3bcd8b 100644 --- a/app/services/tracks/incremental_processor.rb +++ b/app/services/tracks/incremental_processor.rb @@ -66,7 +66,7 @@ class Tracks::IncrementalProcessor end def find_end_time - previous_point ? Time.at(previous_point.timestamp) : nil + previous_point ? Time.zone.at(previous_point.timestamp) : nil end def exceeds_thresholds?(previous_point, current_point) diff --git a/spec/services/tracks/generator_spec.rb b/spec/services/tracks/generator_spec.rb index 0b53c5f5..6f352b86 100644 --- a/spec/services/tracks/generator_spec.rb +++ b/spec/services/tracks/generator_spec.rb @@ -31,6 +31,24 @@ RSpec.describe Tracks::Generator do generator.call expect(points.map(&:reload).map(&:track)).to all(be_present) end + + it 'properly handles point associations when cleaning existing tracks' do + # Create existing tracks with associated points + existing_track = create(:track, user: user) + existing_points = create_list(:point, 3, user: user, track: existing_track) + + # Verify points are associated + expect(existing_points.map(&:reload).map(&:track_id)).to all(eq(existing_track.id)) + + # Run generator which should clean existing tracks and create new ones + generator.call + + # Verify the old track is deleted + expect(Track.exists?(existing_track.id)).to be false + + # Verify the points are no longer associated with the deleted track + expect(existing_points.map(&:reload).map(&:track_id)).to all(be_nil) + end end context 'with insufficient points' do @@ -118,6 +136,24 @@ RSpec.describe Tracks::Generator do generator.call expect(Track.exists?(existing_track.id)).to be false end + + it 'properly handles point associations when cleaning daily tracks' do + # Create existing tracks with associated points for today + existing_track = create(:track, user: user, start_at: today.beginning_of_day) + existing_points = create_list(:point, 3, user: user, track: existing_track) + + # Verify points are associated + expect(existing_points.map(&:reload).map(&:track_id)).to all(eq(existing_track.id)) + + # Run generator which should clean existing tracks for the day and create new ones + generator.call + + # Verify the old track is deleted + expect(Track.exists?(existing_track.id)).to be false + + # Verify the points are no longer associated with the deleted track + expect(existing_points.map(&:reload).map(&:track_id)).to all(be_nil) + end end context 'with empty points' do diff --git a/spec/services/tracks/incremental_processor_spec.rb b/spec/services/tracks/incremental_processor_spec.rb index f3b66499..a2d21bd5 100644 --- a/spec/services/tracks/incremental_processor_spec.rb +++ b/spec/services/tracks/incremental_processor_spec.rb @@ -47,7 +47,7 @@ RSpec.describe Tracks::IncrementalProcessor do it 'processes when time threshold exceeded' do expect(Tracks::CreateJob).to receive(:perform_later) - .with(user.id, start_at: nil, end_at: Time.at(previous_point.timestamp), mode: :none) + .with(user.id, start_at: nil, end_at: Time.zone.at(previous_point.timestamp), mode: :none) processor.call end end @@ -65,7 +65,7 @@ RSpec.describe Tracks::IncrementalProcessor do it 'uses existing track end time as start_at' do expect(Tracks::CreateJob).to receive(:perform_later) - .with(user.id, start_at: existing_track.end_at, end_at: Time.at(previous_point.timestamp), mode: :none) + .with(user.id, start_at: existing_track.end_at, end_at: Time.zone.at(previous_point.timestamp), mode: :none) processor.call end end @@ -88,7 +88,7 @@ RSpec.describe Tracks::IncrementalProcessor do it 'processes when distance threshold exceeded' do expect(Tracks::CreateJob).to receive(:perform_later) - .with(user.id, start_at: nil, end_at: Time.at(previous_point.timestamp), mode: :none) + .with(user.id, start_at: nil, end_at: Time.zone.at(previous_point.timestamp), mode: :none) processor.call end end diff --git a/spec/services/users/import_data_spec.rb b/spec/services/users/import_data_spec.rb index 5d57b97f..1fcf9cfd 100644 --- a/spec/services/users/import_data_spec.rb +++ b/spec/services/users/import_data_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Users::ImportData, type: :service do let(:import_directory) { Rails.root.join('tmp', "import_#{user.email.gsub(/[^0-9A-Za-z._-]/, '_')}_1234567890") } before do - allow(Time).to receive(:current).and_return(Time.at(1234567890)) + allow(Time).to receive(:current).and_return(Time.zone.at(1234567890)) allow(FileUtils).to receive(:mkdir_p) allow(FileUtils).to receive(:rm_rf) allow(File).to receive(:directory?).and_return(true) From 9d616c795722f8e77ce4178b7ed1480fd9e519fc Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 17 Jul 2025 21:02:45 +0200 Subject: [PATCH 15/30] Remove logging from tracks generator --- app/services/tracks/generator.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index 45610a50..be16b48f 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -163,9 +163,7 @@ class Tracks::Generator scope = user.tracks scope = scope.where(start_at: time_range) if time_range_defined? - deleted_count = scope.destroy_all - - Rails.logger.info "Deleted #{deleted_count} existing tracks for user #{user.id}" + scope.destroy_all end def clean_daily_tracks @@ -173,9 +171,7 @@ class Tracks::Generator range = Time.zone.at(day_range.begin)..Time.zone.at(day_range.end) scope = user.tracks.where(start_at: range) - deleted_count = scope.destroy_all - - Rails.logger.info "Deleted #{deleted_count} daily tracks for user #{user.id}" + scope.destroy_all end # Threshold methods from safe_settings From 002b3bd6350f8775f9abd46901c3781890e14556 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 20 Jul 2025 17:06:45 +0200 Subject: [PATCH 16/30] Fix settings controller spec and tracks popup --- app/controllers/api/v1/settings_controller.rb | 2 +- app/javascript/maps/tracks.js | 2 +- .../api/v1/settings_controller_spec.rb | 72 ++++++++----------- 3 files changed, 32 insertions(+), 44 deletions(-) diff --git a/app/controllers/api/v1/settings_controller.rb b/app/controllers/api/v1/settings_controller.rb index 0471f49b..10620730 100644 --- a/app/controllers/api/v1/settings_controller.rb +++ b/app/controllers/api/v1/settings_controller.rb @@ -5,7 +5,7 @@ class Api::V1::SettingsController < ApiController def index render json: { - settings: current_api_user.settings, + settings: current_api_user.safe_settings, status: 'success' }, status: :ok end diff --git a/app/javascript/maps/tracks.js b/app/javascript/maps/tracks.js index ffda6a35..2e30ca98 100644 --- a/app/javascript/maps/tracks.js +++ b/app/javascript/maps/tracks.js @@ -30,7 +30,7 @@ export function createTrackPopupContent(track, distanceUnit) { 🕐 Start: ${startTime}
🏁 End: ${endTime}
⏱️ Duration: ${durationFormatted}
- 📏 Distance: ${formatDistance(track.distance, distanceUnit)}
+ 📏 Distance: ${formatDistance(track.distance / 1000, distanceUnit)}
⚡ Avg Speed: ${formatSpeed(track.avg_speed, distanceUnit)}
⛰️ Elevation: +${track.elevation_gain || 0}m / -${track.elevation_loss || 0}m
📊 Max Alt: ${track.elevation_max || 0}m
diff --git a/spec/swagger/api/v1/settings_controller_spec.rb b/spec/swagger/api/v1/settings_controller_spec.rb index e9716d12..0f440b51 100644 --- a/spec/swagger/api/v1/settings_controller_spec.rb +++ b/spec/swagger/api/v1/settings_controller_spec.rb @@ -21,8 +21,8 @@ describe 'Settings API', type: :request do 'immich_api_key': 'your-immich-api-key', 'photoprism_url': 'https://photoprism.example.com', 'photoprism_api_key': 'your-photoprism-api-key', - 'maps': { 'distance_unit': 'km' }, - 'visits_suggestions_enabled': true + 'speed_color_scale': 'viridis', + 'fog_of_war_threshold': 100 } } tags 'Settings' @@ -100,21 +100,15 @@ describe 'Settings API', type: :request do example: 'your-photoprism-api-key', description: 'API key for PhotoPrism photo service' }, - maps: { - type: :object, - properties: { - distance_unit: { - type: :string, - example: 'km', - description: 'Distance unit preference (km or miles)' - } - }, - description: 'Map-related settings' + speed_color_scale: { + type: :string, + example: 'viridis', + description: 'Color scale for speed-colored routes' }, - visits_suggestions_enabled: { - type: :boolean, - example: true, - description: 'Whether visit suggestions are enabled' + fog_of_war_threshold: { + type: :number, + example: 100, + description: 'Fog of war threshold value' } } } @@ -138,33 +132,33 @@ describe 'Settings API', type: :request do type: :object, properties: { route_opacity: { - type: :string, - example: '60', + type: :number, + example: 60, description: 'Route opacity percentage (0-100)' }, meters_between_routes: { - type: :string, - example: '500', + type: :number, + example: 500, description: 'Minimum distance between routes in meters' }, minutes_between_routes: { - type: :string, - example: '30', + type: :number, + example: 30, description: 'Minimum time between routes in minutes' }, fog_of_war_meters: { - type: :string, - example: '50', + type: :number, + example: 50, description: 'Fog of war radius in meters' }, time_threshold_minutes: { - type: :string, - example: '30', + type: :number, + example: 30, description: 'Time threshold for grouping points in minutes' }, merge_threshold_minutes: { - type: :string, - example: '15', + type: :number, + example: 15, description: 'Threshold for merging nearby points in minutes' }, preferred_map_layer: { @@ -207,21 +201,15 @@ describe 'Settings API', type: :request do example: 'your-photoprism-api-key', description: 'API key for PhotoPrism photo service' }, - maps: { - type: :object, - properties: { - distance_unit: { - type: :string, - example: 'km', - description: 'Distance unit preference (km or miles)' - } - }, - description: 'Map-related settings' + speed_color_scale: { + type: :string, + example: 'viridis', + description: 'Color scale for speed-colored routes' }, - visits_suggestions_enabled: { - type: :boolean, - example: true, - description: 'Whether visit suggestions are enabled' + fog_of_war_threshold: { + type: :number, + example: 100, + description: 'Fog of war threshold value' } } } From f5c399a8cc8b02999325ab05940578f1ed665076 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 20 Jul 2025 17:11:11 +0200 Subject: [PATCH 17/30] Fix domain in development and production --- config/environments/development.rb | 2 +- config/environments/production.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/environments/development.rb b/config/environments/development.rb index 68c0aeaa..c940de0e 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -88,7 +88,7 @@ Rails.application.configure do hosts = ENV.fetch('APPLICATION_HOSTS', 'localhost').split(',') - config.action_mailer.default_url_options = { host: ENV['SMTP_DOMAIN'] || hosts.first, port: ENV.fetch('PORT', 3000) } + config.action_mailer.default_url_options = { host: ENV['DOMAIN'] || hosts.first, port: ENV.fetch('PORT', 3000) } config.hosts.concat(hosts) if hosts.present? diff --git a/config/environments/production.rb b/config/environments/production.rb index 7207e549..1e4b392a 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -103,7 +103,7 @@ Rails.application.configure do config.host_authorization = { exclude: ->(request) { request.path == "/api/v1/health" } } hosts = ENV.fetch('APPLICATION_HOSTS', 'localhost').split(',') - config.action_mailer.default_url_options = { host: ENV['SMTP_DOMAIN'] } + config.action_mailer.default_url_options = { host: ENV['DOMAIN'] } config.hosts.concat(hosts) if hosts.present? config.action_mailer.delivery_method = :smtp From 45713f46dc7debf23f2781285ce42c93d948cef0 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 20 Jul 2025 17:11:11 +0200 Subject: [PATCH 18/30] Fix domain in development and production --- config/environments/development.rb | 2 +- config/environments/production.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/environments/development.rb b/config/environments/development.rb index 68c0aeaa..c940de0e 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -88,7 +88,7 @@ Rails.application.configure do hosts = ENV.fetch('APPLICATION_HOSTS', 'localhost').split(',') - config.action_mailer.default_url_options = { host: ENV['SMTP_DOMAIN'] || hosts.first, port: ENV.fetch('PORT', 3000) } + config.action_mailer.default_url_options = { host: ENV['DOMAIN'] || hosts.first, port: ENV.fetch('PORT', 3000) } config.hosts.concat(hosts) if hosts.present? diff --git a/config/environments/production.rb b/config/environments/production.rb index 7207e549..1e4b392a 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -103,7 +103,7 @@ Rails.application.configure do config.host_authorization = { exclude: ->(request) { request.path == "/api/v1/health" } } hosts = ENV.fetch('APPLICATION_HOSTS', 'localhost').split(',') - config.action_mailer.default_url_options = { host: ENV['SMTP_DOMAIN'] } + config.action_mailer.default_url_options = { host: ENV['DOMAIN'] } config.hosts.concat(hosts) if hosts.present? config.action_mailer.delivery_method = :smtp From 708bca26eb1f4004b632e550392e13240e4e9188 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 20 Jul 2025 17:43:55 +0200 Subject: [PATCH 19/30] Fix owntracks point creation --- CHANGELOG.md | 1 + app/jobs/owntracks/point_creating_job.rb | 2 +- app/services/own_tracks/params.rb | 6 ++ .../jobs/owntracks/point_creating_job_spec.rb | 8 +++ spec/services/own_tracks/params_spec.rb | 8 +++ swagger/v1/swagger.yaml | 69 ++++++++----------- 6 files changed, 54 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bdd02d1..21a189d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Fixed - Swagger documentation is now valid again. +- Invalid owntracks points are now ignored. # [0.29.1] - 2025-07-02 diff --git a/app/jobs/owntracks/point_creating_job.rb b/app/jobs/owntracks/point_creating_job.rb index 5695894e..63ff6c90 100644 --- a/app/jobs/owntracks/point_creating_job.rb +++ b/app/jobs/owntracks/point_creating_job.rb @@ -8,7 +8,7 @@ class Owntracks::PointCreatingJob < ApplicationJob def perform(point_params, user_id) parsed_params = OwnTracks::Params.new(point_params).call - return if parsed_params[:timestamp].nil? || parsed_params[:lonlat].nil? + return if parsed_params.try(:[], :timestamp).nil? || parsed_params.try(:[], :lonlat).nil? return if point_exists?(parsed_params, user_id) Point.create!(parsed_params.merge(user_id:)) diff --git a/app/services/own_tracks/params.rb b/app/services/own_tracks/params.rb index 88533690..838af33a 100644 --- a/app/services/own_tracks/params.rb +++ b/app/services/own_tracks/params.rb @@ -10,6 +10,8 @@ class OwnTracks::Params # rubocop:disable Metrics/MethodLength # rubocop:disable Metrics/AbcSize def call + return unless valid_point? + { lonlat: "POINT(#{params[:lon]} #{params[:lat]})", battery: params[:batt], @@ -84,4 +86,8 @@ class OwnTracks::Params def owntracks_point? params[:topic].present? end + + def valid_point? + params[:lon].present? && params[:lat].present? && params[:tst].present? + end end diff --git a/spec/jobs/owntracks/point_creating_job_spec.rb b/spec/jobs/owntracks/point_creating_job_spec.rb index 3607bc7b..ae8d49fb 100644 --- a/spec/jobs/owntracks/point_creating_job_spec.rb +++ b/spec/jobs/owntracks/point_creating_job_spec.rb @@ -28,5 +28,13 @@ RSpec.describe Owntracks::PointCreatingJob, type: :job do expect { perform }.not_to(change { Point.count }) end end + + context 'when point is invalid' do + let(:point_params) { { lat: 1.0, lon: 1.0, tid: 'test', tst: nil, topic: 'iPhone 12 pro' } } + + it 'does not create a point' do + expect { perform }.not_to(change { Point.count }) + end + end end end diff --git a/spec/services/own_tracks/params_spec.rb b/spec/services/own_tracks/params_spec.rb index d08f5b30..9bea14cc 100644 --- a/spec/services/own_tracks/params_spec.rb +++ b/spec/services/own_tracks/params_spec.rb @@ -185,5 +185,13 @@ RSpec.describe OwnTracks::Params do expect(params[:trigger]).to eq('unknown') end end + + context 'when point is invalid' do + let(:raw_point_params) { super().merge(lon: nil, lat: nil, tst: nil) } + + it 'returns parsed params' do + expect(params).to eq(nil) + end + end end end diff --git a/swagger/v1/swagger.yaml b/swagger/v1/swagger.yaml index 7a65546f..bc25a57d 100644 --- a/swagger/v1/swagger.yaml +++ b/swagger/v1/swagger.yaml @@ -1059,18 +1059,14 @@ paths: type: string example: your-photoprism-api-key description: API key for PhotoPrism photo service - maps: - type: object - properties: - distance_unit: - type: string - example: km - description: Distance unit preference (km or miles) - description: Map-related settings - visits_suggestions_enabled: - type: boolean - example: true - description: Whether visit suggestions are enabled + speed_color_scale: + type: string + example: viridis + description: Color scale for speed-colored routes + fog_of_war_threshold: + type: number + example: 100 + description: Fog of war threshold value examples: '0': summary: Updates user settings @@ -1090,9 +1086,8 @@ paths: immich_api_key: your-immich-api-key photoprism_url: https://photoprism.example.com photoprism_api_key: your-photoprism-api-key - maps: - distance_unit: km - visits_suggestions_enabled: true + speed_color_scale: viridis + fog_of_war_threshold: 100 get: summary: Retrieves user settings tags: @@ -1116,28 +1111,28 @@ paths: type: object properties: route_opacity: - type: string - example: '60' + type: number + example: 60 description: Route opacity percentage (0-100) meters_between_routes: - type: string - example: '500' + type: number + example: 500 description: Minimum distance between routes in meters minutes_between_routes: - type: string - example: '30' + type: number + example: 30 description: Minimum time between routes in minutes fog_of_war_meters: - type: string - example: '50' + type: number + example: 50 description: Fog of war radius in meters time_threshold_minutes: - type: string - example: '30' + type: number + example: 30 description: Time threshold for grouping points in minutes merge_threshold_minutes: - type: string - example: '15' + type: number + example: 15 description: Threshold for merging nearby points in minutes preferred_map_layer: type: string @@ -1172,18 +1167,14 @@ paths: type: string example: your-photoprism-api-key description: API key for PhotoPrism photo service - maps: - type: object - properties: - distance_unit: - type: string - example: km - description: Distance unit preference (km or miles) - description: Map-related settings - visits_suggestions_enabled: - type: boolean - example: true - description: Whether visit suggestions are enabled + speed_color_scale: + type: string + example: viridis + description: Color scale for speed-colored routes + fog_of_war_threshold: + type: number + example: 100 + description: Fog of war threshold value "/api/v1/stats": get: summary: Retrieves all stats From f969d5d3e6cac0e5b624f6b453c750300be0a788 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 20 Jul 2025 18:57:53 +0200 Subject: [PATCH 20/30] Clean up some mess --- app/controllers/map_controller.rb | 3 +- app/jobs/tracks/create_job.rb | 15 +- app/models/concerns/distance_convertible.rb | 16 -- app/models/point.rb | 4 +- app/serializers/track_serializer.rb | 41 ++-- app/serializers/tracks_serializer.rb | 22 ++ app/services/places/name_fetcher.rb | 34 +-- app/services/tracks/generator.rb | 5 +- app/services/tracks/incremental_processor.rb | 7 +- app/services/tracks/segmentation.rb | 4 +- app/services/users/safe_settings.rb | 1 - .../settings/background_jobs/index.html.erb | 2 +- config/initializers/01_constants.rb | 8 +- ..._visits_calculation_scheduling_job_spec.rb | 8 +- spec/jobs/tracks/create_job_spec.rb | 90 +------- spec/models/trip_spec.rb | 2 +- spec/rails_helper.rb | 9 - spec/serializers/track_serializer_spec.rb | 205 ++++++++++++------ spec/serializers/tracks_serializer_spec.rb | 99 +++++++++ .../tracks/incremental_processor_spec.rb | 8 +- spec/services/visits/suggest_spec.rb | 1 - spec/support/redis.rb | 1 - 22 files changed, 325 insertions(+), 260 deletions(-) create mode 100644 app/serializers/tracks_serializer.rb create mode 100644 spec/serializers/tracks_serializer_spec.rb diff --git a/app/controllers/map_controller.rb b/app/controllers/map_controller.rb index cf9a67cd..f335d6ef 100644 --- a/app/controllers/map_controller.rb +++ b/app/controllers/map_controller.rb @@ -31,7 +31,8 @@ class MapController < ApplicationController def build_tracks track_ids = extract_track_ids - TrackSerializer.new(current_user, track_ids).call + + TracksSerializer.new(current_user, track_ids).call end def calculate_distance diff --git a/app/jobs/tracks/create_job.rb b/app/jobs/tracks/create_job.rb index a57a3f79..919e5f82 100644 --- a/app/jobs/tracks/create_job.rb +++ b/app/jobs/tracks/create_job.rb @@ -6,20 +6,7 @@ class Tracks::CreateJob < ApplicationJob def perform(user_id, start_at: nil, end_at: nil, mode: :daily) user = User.find(user_id) - # Translate mode parameter to Generator mode - generator_mode = case mode - when :daily then :daily - when :none then :incremental - else :bulk - end - - # Generate tracks and get the count of tracks created - tracks_created = Tracks::Generator.new( - user, - start_at: start_at, - end_at: end_at, - mode: generator_mode - ).call + tracks_created = Tracks::Generator.new(user, start_at:, end_at:, mode:).call create_success_notification(user, tracks_created) rescue StandardError => e diff --git a/app/models/concerns/distance_convertible.rb b/app/models/concerns/distance_convertible.rb index 52054b3d..2a757303 100644 --- a/app/models/concerns/distance_convertible.rb +++ b/app/models/concerns/distance_convertible.rb @@ -19,7 +19,6 @@ # track.distance # => 5000 (meters stored in DB) # track.distance_in_unit('km') # => 5.0 (converted to km) # track.distance_in_unit('mi') # => 3.11 (converted to miles) -# track.formatted_distance('km') # => "5.0 km" # module DistanceConvertible extend ActiveSupport::Concern @@ -38,21 +37,11 @@ module DistanceConvertible distance.to_f / conversion_factor end - def formatted_distance(unit, precision: 2) - converted_distance = distance_in_unit(unit) - "#{converted_distance.round(precision)} #{unit}" - end - def distance_for_user(user) user_unit = user.safe_settings.distance_unit distance_in_unit(user_unit) end - def formatted_distance_for_user(user, precision: 2) - user_unit = user.safe_settings.distance_unit - formatted_distance(user_unit, precision: precision) - end - module ClassMethods def convert_distance(distance_meters, unit) return 0.0 unless distance_meters.present? @@ -66,10 +55,5 @@ module DistanceConvertible distance_meters.to_f / conversion_factor end - - def format_distance(distance_meters, unit, precision: 2) - converted = convert_distance(distance_meters, unit) - "#{converted.round(precision)} #{unit}" - end end end diff --git a/app/models/point.rb b/app/models/point.rb index f45607d7..75566be3 100644 --- a/app/models/point.rb +++ b/app/models/point.rb @@ -34,7 +34,7 @@ class Point < ApplicationRecord after_create :set_country after_create_commit :broadcast_coordinates after_create_commit :trigger_incremental_track_generation, if: -> { import_id.nil? } - after_commit :recalculate_track, on: :update + after_commit :recalculate_track, on: :update, if: -> { track.present? } def self.without_raw_data select(column_names - ['raw_data']) @@ -99,8 +99,6 @@ class Point < ApplicationRecord end def recalculate_track - return unless track.present? - track.recalculate_path_and_distance! end diff --git a/app/serializers/track_serializer.rb b/app/serializers/track_serializer.rb index 1a67ccba..9674db0b 100644 --- a/app/serializers/track_serializer.rb +++ b/app/serializers/track_serializer.rb @@ -1,38 +1,23 @@ # frozen_string_literal: true class TrackSerializer - def initialize(user, track_ids) - @user = user - @track_ids = track_ids + def initialize(track) + @track = track end def call - return [] if track_ids.empty? - - tracks = user.tracks - .where(id: track_ids) - .order(start_at: :asc) - - tracks.map { |track| serialize_track_data(track) } - end - - private - - attr_reader :user, :track_ids - - def serialize_track_data(track) { - id: track.id, - start_at: track.start_at.iso8601, - end_at: track.end_at.iso8601, - distance: track.distance.to_i, - avg_speed: track.avg_speed.to_f, - duration: track.duration, - elevation_gain: track.elevation_gain, - elevation_loss: track.elevation_loss, - elevation_max: track.elevation_max, - elevation_min: track.elevation_min, - original_path: track.original_path.to_s + id: @track.id, + start_at: @track.start_at.iso8601, + end_at: @track.end_at.iso8601, + distance: @track.distance.to_i, + avg_speed: @track.avg_speed.to_f, + duration: @track.duration, + elevation_gain: @track.elevation_gain, + elevation_loss: @track.elevation_loss, + elevation_max: @track.elevation_max, + elevation_min: @track.elevation_min, + original_path: @track.original_path.to_s } end end diff --git a/app/serializers/tracks_serializer.rb b/app/serializers/tracks_serializer.rb new file mode 100644 index 00000000..79aeaddf --- /dev/null +++ b/app/serializers/tracks_serializer.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class TracksSerializer + def initialize(user, track_ids) + @user = user + @track_ids = track_ids + end + + def call + return [] if track_ids.empty? + + tracks = user.tracks + .where(id: track_ids) + .order(start_at: :asc) + + tracks.map { |track| TrackSerializer.new(track).call } + end + + private + + attr_reader :user, :track_ids +end diff --git a/app/services/places/name_fetcher.rb b/app/services/places/name_fetcher.rb index 3a817dda..d4e01b9e 100644 --- a/app/services/places/name_fetcher.rb +++ b/app/services/places/name_fetcher.rb @@ -7,7 +7,7 @@ module Places end def call - geodata = Geocoder.search([@place.lat, @place.lon], units: :km, limit: 1, distance_sort: true).first + geodata = Geocoder.search([place.lat, place.lon], units: :km, limit: 1, distance_sort: true).first return if geodata.blank? @@ -15,21 +15,29 @@ module Places return if properties.blank? ActiveRecord::Base.transaction do - @place.name = properties['name'] if properties['name'].present? - @place.city = properties['city'] if properties['city'].present? - @place.country = properties['country'] if properties['country'].present? - @place.geodata = geodata.data if DawarichSettings.store_geodata? - @place.save! + update_place_name(properties, geodata) - if properties['name'].present? - @place - .visits - .where(name: Place::DEFAULT_NAME) - .update_all(name: properties['name']) - end + update_visits_name(properties) if properties['name'].present? - @place + place end end + + private + + attr_reader :place + + def update_place_name(properties, geodata) + place.name = properties['name'] if properties['name'].present? + place.city = properties['city'] if properties['city'].present? + place.country = properties['country'] if properties['country'].present? + place.geodata = geodata.data if DawarichSettings.store_geodata? + + place.save! + end + + def update_visits_name(properties) + place.visits.where(name: Place::DEFAULT_NAME).update_all(name: properties['name']) + end end end diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index be16b48f..9ffcdbb7 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -48,6 +48,7 @@ class Tracks::Generator Rails.logger.debug "Generator: created #{segments.size} segments" tracks_created = 0 + segments.each do |segment| track = create_track_from_segment(segment) tracks_created += 1 if track @@ -146,10 +147,6 @@ class Tracks::Generator day.beginning_of_day.to_i..day.end_of_day.to_i end - def incremental_mode? - mode == :incremental - end - def clean_existing_tracks case mode when :bulk then clean_bulk_tracks diff --git a/app/services/tracks/incremental_processor.rb b/app/services/tracks/incremental_processor.rb index 3f3bcd8b..62c1faed 100644 --- a/app/services/tracks/incremental_processor.rb +++ b/app/services/tracks/incremental_processor.rb @@ -36,12 +36,7 @@ class Tracks::IncrementalProcessor start_at = find_start_time end_at = find_end_time - Tracks::CreateJob.perform_later( - user.id, - start_at: start_at, - end_at: end_at, - mode: :none - ) + Tracks::CreateJob.perform_later(user.id, start_at:, end_at:, mode: :incremental) end private diff --git a/app/services/tracks/segmentation.rb b/app/services/tracks/segmentation.rb index 8b93dee4..57ca3b03 100644 --- a/app/services/tracks/segmentation.rb +++ b/app/services/tracks/segmentation.rb @@ -77,7 +77,7 @@ module Tracks::Segmentation return true if time_diff_seconds > time_threshold_seconds # Check distance threshold - convert km to meters to match frontend logic - distance_km = calculate_distance_kilometers_between_points(previous_point, current_point) + distance_km = calculate_km_distance_between_points(previous_point, current_point) distance_meters = distance_km * 1000 # Convert km to meters return true if distance_meters > distance_threshold_meters @@ -85,7 +85,7 @@ module Tracks::Segmentation false end - def calculate_distance_kilometers_between_points(point1, point2) + def calculate_km_distance_between_points(point1, point2) lat1, lon1 = point_coordinates(point1) lat2, lon2 = point_coordinates(point2) diff --git a/app/services/users/safe_settings.rb b/app/services/users/safe_settings.rb index 308121e5..43b6fcac 100644 --- a/app/services/users/safe_settings.rb +++ b/app/services/users/safe_settings.rb @@ -113,7 +113,6 @@ class Users::SafeSettings end def distance_unit - # km or mi settings.dig('maps', 'distance_unit') end diff --git a/app/views/settings/background_jobs/index.html.erb b/app/views/settings/background_jobs/index.html.erb index b29c6e23..22813e2a 100644 --- a/app/views/settings/background_jobs/index.html.erb +++ b/app/views/settings/background_jobs/index.html.erb @@ -50,7 +50,7 @@ -
+

Visits suggestions

Enable or disable visits suggestions. It's a background task that runs every day at midnight. Disabling it might be useful if you don't want to receive visits suggestions or if you're using the Dawarich iOS app, which has its own visits suggestions.

diff --git a/config/initializers/01_constants.rb b/config/initializers/01_constants.rb index 494f1116..594cadb3 100644 --- a/config/initializers/01_constants.rb +++ b/config/initializers/01_constants.rb @@ -5,11 +5,11 @@ SELF_HOSTED = ENV.fetch('SELF_HOSTED', 'true') == 'true' MIN_MINUTES_SPENT_IN_CITY = ENV.fetch('MIN_MINUTES_SPENT_IN_CITY', 60).to_i DISTANCE_UNITS = { - km: 1000, # to meters + km: 1000, # to meters mi: 1609.34, # to meters - m: 1, # already in meters - ft: 0.3048, # to meters - yd: 0.9144 # to meters + m: 1, # already in meters + ft: 0.3048, # to meters + yd: 0.9144 # to meters }.freeze APP_VERSION = File.read('.app_version').strip diff --git a/spec/jobs/area_visits_calculation_scheduling_job_spec.rb b/spec/jobs/area_visits_calculation_scheduling_job_spec.rb index c2e1bbeb..b38ee551 100644 --- a/spec/jobs/area_visits_calculation_scheduling_job_spec.rb +++ b/spec/jobs/area_visits_calculation_scheduling_job_spec.rb @@ -4,13 +4,13 @@ require 'rails_helper' RSpec.describe AreaVisitsCalculationSchedulingJob, type: :job do describe '#perform' do - let!(:user) { create(:user) } - let!(:area) { create(:area, user: user) } + let(:user) { create(:user) } + let(:area) { create(:area, user: user) } it 'calls the AreaVisitsCalculationService' do - expect(AreaVisitsCalculatingJob).to receive(:perform_later).with(user.id) + expect(AreaVisitsCalculatingJob).to receive(:perform_later).with(user.id).and_call_original - described_class.new.perform_now + described_class.new.perform end end end diff --git a/spec/jobs/tracks/create_job_spec.rb b/spec/jobs/tracks/create_job_spec.rb index 69a47fa2..7303ed3d 100644 --- a/spec/jobs/tracks/create_job_spec.rb +++ b/spec/jobs/tracks/create_job_spec.rb @@ -14,12 +14,10 @@ RSpec.describe Tracks::CreateJob, type: :job do allow(generator_instance).to receive(:call) allow(Notifications::Create).to receive(:new).and_return(notification_service) allow(notification_service).to receive(:call) + allow(generator_instance).to receive(:call).and_return(2) end it 'calls the generator and creates a notification' do - # Mock the generator to return the count of tracks created - allow(generator_instance).to receive(:call).and_return(2) - described_class.new.perform(user.id) expect(Tracks::Generator).to have_received(:new).with( @@ -48,12 +46,10 @@ RSpec.describe Tracks::CreateJob, type: :job do allow(generator_instance).to receive(:call) allow(Notifications::Create).to receive(:new).and_return(notification_service) allow(notification_service).to receive(:call) + allow(generator_instance).to receive(:call).and_return(1) end it 'passes custom parameters to the generator' do - # Mock generator to return the count of tracks created - allow(generator_instance).to receive(:call).and_return(1) - described_class.new.perform(user.id, start_at: start_at, end_at: end_at, mode: mode) expect(Tracks::Generator).to have_received(:new).with( @@ -73,72 +69,6 @@ RSpec.describe Tracks::CreateJob, type: :job do end end - context 'with mode translation' do - before do - allow(Tracks::Generator).to receive(:new).and_return(generator_instance) - allow(generator_instance).to receive(:call) # No tracks created for mode tests - allow(Notifications::Create).to receive(:new).and_return(notification_service) - allow(notification_service).to receive(:call) - end - - it 'translates :none to :incremental' do - allow(generator_instance).to receive(:call).and_return(0) - - described_class.new.perform(user.id, mode: :none) - - expect(Tracks::Generator).to have_received(:new).with( - user, - start_at: nil, - end_at: nil, - mode: :incremental - ) - expect(Notifications::Create).to have_received(:new).with( - user: user, - kind: :info, - title: 'Tracks Generated', - content: 'Created 0 tracks from your location data. Check your tracks section to view them.' - ) - end - - it 'translates :daily to :daily' do - allow(generator_instance).to receive(:call).and_return(0) - - described_class.new.perform(user.id, mode: :daily) - - expect(Tracks::Generator).to have_received(:new).with( - user, - start_at: nil, - end_at: nil, - mode: :daily - ) - expect(Notifications::Create).to have_received(:new).with( - user: user, - kind: :info, - title: 'Tracks Generated', - content: 'Created 0 tracks from your location data. Check your tracks section to view them.' - ) - end - - it 'translates other modes to :bulk' do - allow(generator_instance).to receive(:call).and_return(0) - - described_class.new.perform(user.id, mode: :replace) - - expect(Tracks::Generator).to have_received(:new).with( - user, - start_at: nil, - end_at: nil, - mode: :bulk - ) - expect(Notifications::Create).to have_received(:new).with( - user: user, - kind: :info, - title: 'Tracks Generated', - content: 'Created 0 tracks from your location data. Check your tracks section to view them.' - ) - end - end - context 'when generator raises an error' do let(:error_message) { 'Something went wrong' } let(:notification_service) { instance_double(Notifications::Create) } @@ -175,12 +105,13 @@ RSpec.describe Tracks::CreateJob, type: :job do end context 'when user does not exist' do - it 'handles the error gracefully and creates error notification' do + before do allow(User).to receive(:find).with(999).and_raise(ActiveRecord::RecordNotFound) allow(ExceptionReporter).to receive(:call) allow(Notifications::Create).to receive(:new).and_return(instance_double(Notifications::Create, call: nil)) + end - # Should not raise an error because it's caught by the rescue block + it 'handles the error gracefully and creates error notification' do expect { described_class.new.perform(999) }.not_to raise_error expect(ExceptionReporter).to have_received(:call) @@ -188,15 +119,14 @@ RSpec.describe Tracks::CreateJob, type: :job do end context 'when tracks are deleted and recreated' do - it 'returns the correct count of newly created tracks' do - # Create some existing tracks first - create_list(:track, 3, user: user) + let(:existing_tracks) { create_list(:track, 3, user: user) } - # Mock the generator to simulate deleting existing tracks and creating new ones - # This should return the count of newly created tracks, not the difference + before do allow(generator_instance).to receive(:call).and_return(2) + end - described_class.new.perform(user.id, mode: :bulk) + it 'returns the correct count of newly created tracks' do + described_class.new.perform(user.id, mode: :incremental) expect(Tracks::Generator).to have_received(:new).with( user, diff --git a/spec/models/trip_spec.rb b/spec/models/trip_spec.rb index eecf3fb8..20bb5ba3 100644 --- a/spec/models/trip_spec.rb +++ b/spec/models/trip_spec.rb @@ -160,7 +160,7 @@ RSpec.describe Trip, type: :model do end end - describe '#recalculate_distance!' do + describe '#recalculate_distance!' do it 'recalculates and saves the distance' do original_distance = trip.distance diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 7275e402..0cd0f177 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -41,9 +41,6 @@ RSpec.configure do |config| config.before(:suite) do Rails.application.reload_routes! - - # DatabaseCleaner.strategy = :transaction - # DatabaseCleaner.clean_with(:truncation) end config.before do @@ -92,12 +89,6 @@ RSpec.configure do |config| config.after(:suite) do Rake::Task['rswag:generate'].invoke end - - # config.around(:each) do |example| - # DatabaseCleaner.cleaning do - # example.run - # end - # end end Shoulda::Matchers.configure do |config| diff --git a/spec/serializers/track_serializer_spec.rb b/spec/serializers/track_serializer_spec.rb index 42f99175..6213e2c9 100644 --- a/spec/serializers/track_serializer_spec.rb +++ b/spec/serializers/track_serializer_spec.rb @@ -5,95 +5,166 @@ require 'rails_helper' RSpec.describe TrackSerializer do describe '#call' do let(:user) { create(:user) } + let(:track) { create(:track, user: user) } + let(:serializer) { described_class.new(track) } - context 'when serializing user tracks with track IDs' do - subject(:serializer) { described_class.new(user, track_ids).call } + subject(:serialized_track) { serializer.call } - let!(:track1) { create(:track, user: user, start_at: 2.hours.ago, end_at: 1.hour.ago) } - let!(:track2) { create(:track, user: user, start_at: 4.hours.ago, end_at: 3.hours.ago) } - let!(:track3) { create(:track, user: user, start_at: 6.hours.ago, end_at: 5.hours.ago) } - let(:track_ids) { [track1.id, track2.id] } + it 'returns a hash with all required attributes' do + expect(serialized_track).to be_a(Hash) + expect(serialized_track.keys).to contain_exactly( + :id, :start_at, :end_at, :distance, :avg_speed, :duration, + :elevation_gain, :elevation_loss, :elevation_max, :elevation_min, :original_path + ) + end - it 'returns an array of serialized tracks' do - expect(serializer).to be_an(Array) - expect(serializer.length).to eq(2) - end + it 'serializes the track ID correctly' do + expect(serialized_track[:id]).to eq(track.id) + end - it 'serializes each track correctly' do - serialized_ids = serializer.map { |track| track[:id] } - expect(serialized_ids).to contain_exactly(track1.id, track2.id) - expect(serialized_ids).not_to include(track3.id) - end + it 'formats start_at as ISO8601 timestamp' do + expect(serialized_track[:start_at]).to eq(track.start_at.iso8601) + expect(serialized_track[:start_at]).to match(/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/) + end - it 'formats timestamps as ISO8601 for all tracks' do - serializer.each do |track| - expect(track[:start_at]).to match(/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/) - expect(track[:end_at]).to match(/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/) - end - end + it 'formats end_at as ISO8601 timestamp' do + expect(serialized_track[:end_at]).to eq(track.end_at.iso8601) + expect(serialized_track[:end_at]).to match(/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/) + end - it 'includes all required fields for each track' do - serializer.each do |track| - expect(track.keys).to contain_exactly( - :id, :start_at, :end_at, :distance, :avg_speed, :duration, - :elevation_gain, :elevation_loss, :elevation_max, :elevation_min, :original_path - ) - end - end + it 'converts distance to integer' do + expect(serialized_track[:distance]).to eq(track.distance.to_i) + expect(serialized_track[:distance]).to be_a(Integer) + end - it 'handles numeric values correctly' do - serializer.each do |track| - expect(track[:distance]).to be_a(Numeric) - expect(track[:avg_speed]).to be_a(Numeric) - expect(track[:duration]).to be_a(Numeric) - expect(track[:elevation_gain]).to be_a(Numeric) - expect(track[:elevation_loss]).to be_a(Numeric) - expect(track[:elevation_max]).to be_a(Numeric) - expect(track[:elevation_min]).to be_a(Numeric) - end - end + it 'converts avg_speed to float' do + expect(serialized_track[:avg_speed]).to eq(track.avg_speed.to_f) + expect(serialized_track[:avg_speed]).to be_a(Float) + end - it 'orders tracks by start_at in ascending order' do - serialized_tracks = serializer - expect(serialized_tracks.first[:id]).to eq(track2.id) # Started 4 hours ago - expect(serialized_tracks.second[:id]).to eq(track1.id) # Started 2 hours ago + it 'serializes duration as numeric value' do + expect(serialized_track[:duration]).to eq(track.duration) + expect(serialized_track[:duration]).to be_a(Numeric) + end + + it 'serializes elevation_gain as numeric value' do + expect(serialized_track[:elevation_gain]).to eq(track.elevation_gain) + expect(serialized_track[:elevation_gain]).to be_a(Numeric) + end + + it 'serializes elevation_loss as numeric value' do + expect(serialized_track[:elevation_loss]).to eq(track.elevation_loss) + expect(serialized_track[:elevation_loss]).to be_a(Numeric) + end + + it 'serializes elevation_max as numeric value' do + expect(serialized_track[:elevation_max]).to eq(track.elevation_max) + expect(serialized_track[:elevation_max]).to be_a(Numeric) + end + + it 'serializes elevation_min as numeric value' do + expect(serialized_track[:elevation_min]).to eq(track.elevation_min) + expect(serialized_track[:elevation_min]).to be_a(Numeric) + end + + it 'converts original_path to string' do + expect(serialized_track[:original_path]).to eq(track.original_path.to_s) + expect(serialized_track[:original_path]).to be_a(String) + end + + context 'with decimal distance values' do + let(:track) { create(:track, user: user, distance: 1234.56) } + + it 'truncates distance to integer' do + expect(serialized_track[:distance]).to eq(1234) end end - context 'when track IDs belong to different users' do - subject(:serializer) { described_class.new(user, track_ids).call } + context 'with decimal avg_speed values' do + let(:track) { create(:track, user: user, avg_speed: 25.75) } - let(:other_user) { create(:user) } - let!(:user_track) { create(:track, user: user) } - let!(:other_user_track) { create(:track, user: other_user) } - let(:track_ids) { [user_track.id, other_user_track.id] } - - it 'only returns tracks belonging to the specified user' do - serialized_ids = serializer.map { |track| track[:id] } - expect(serialized_ids).to contain_exactly(user_track.id) - expect(serialized_ids).not_to include(other_user_track.id) + it 'converts avg_speed to float' do + expect(serialized_track[:avg_speed]).to eq(25.75) end end - context 'when track IDs array is empty' do - subject(:serializer) { described_class.new(user, []).call } + context 'with different original_path formats' do + let(:track) { create(:track, user: user, original_path: 'LINESTRING(0 0, 1 1, 2 2)') } - it 'returns an empty array' do - expect(serializer).to eq([]) + it 'converts geometry to WKT string format' do + expect(serialized_track[:original_path]).to eq('LINESTRING (0 0, 1 1, 2 2)') + expect(serialized_track[:original_path]).to be_a(String) end end - context 'when track IDs contain non-existent IDs' do - subject(:serializer) { described_class.new(user, track_ids).call } + context 'with zero values' do + let(:track) do + create(:track, user: user, + distance: 0, + avg_speed: 0.0, + duration: 0, + elevation_gain: 0, + elevation_loss: 0, + elevation_max: 0, + elevation_min: 0) + end - let!(:existing_track) { create(:track, user: user) } - let(:track_ids) { [existing_track.id, 999999] } + it 'handles zero values correctly' do + expect(serialized_track[:distance]).to eq(0) + expect(serialized_track[:avg_speed]).to eq(0.0) + expect(serialized_track[:duration]).to eq(0) + expect(serialized_track[:elevation_gain]).to eq(0) + expect(serialized_track[:elevation_loss]).to eq(0) + expect(serialized_track[:elevation_max]).to eq(0) + expect(serialized_track[:elevation_min]).to eq(0) + end + end - it 'only returns existing tracks' do - serialized_ids = serializer.map { |track| track[:id] } - expect(serialized_ids).to contain_exactly(existing_track.id) - expect(serializer.length).to eq(1) + context 'with very large values' do + let(:track) do + create(:track, user: user, + distance: 1_000_000.0, + avg_speed: 999.99, + duration: 86_400, # 24 hours in seconds + elevation_gain: 10_000, + elevation_loss: 8_000, + elevation_max: 5_000, + elevation_min: 0) + end + + it 'handles large values correctly' do + expect(serialized_track[:distance]).to eq(1_000_000) + expect(serialized_track[:avg_speed]).to eq(999.99) + expect(serialized_track[:duration]).to eq(86_400) + expect(serialized_track[:elevation_gain]).to eq(10_000) + expect(serialized_track[:elevation_loss]).to eq(8_000) + expect(serialized_track[:elevation_max]).to eq(5_000) + expect(serialized_track[:elevation_min]).to eq(0) + end + end + + context 'with different timestamp formats' do + let(:start_time) { Time.current } + let(:end_time) { start_time + 1.hour } + let(:track) { create(:track, user: user, start_at: start_time, end_at: end_time) } + + it 'formats timestamps consistently' do + expect(serialized_track[:start_at]).to eq(start_time.iso8601) + expect(serialized_track[:end_at]).to eq(end_time.iso8601) end end end + + describe '#initialize' do + let(:track) { create(:track) } + + it 'accepts a track parameter' do + expect { described_class.new(track) }.not_to raise_error + end + + it 'stores the track instance' do + serializer = described_class.new(track) + expect(serializer.instance_variable_get(:@track)).to eq(track) + end + end end diff --git a/spec/serializers/tracks_serializer_spec.rb b/spec/serializers/tracks_serializer_spec.rb new file mode 100644 index 00000000..a4a536b7 --- /dev/null +++ b/spec/serializers/tracks_serializer_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe TracksSerializer do + describe '#call' do + let(:user) { create(:user) } + + context 'when serializing user tracks with track IDs' do + subject(:serializer) { described_class.new(user, track_ids).call } + + let!(:track1) { create(:track, user: user, start_at: 2.hours.ago, end_at: 1.hour.ago) } + let!(:track2) { create(:track, user: user, start_at: 4.hours.ago, end_at: 3.hours.ago) } + let!(:track3) { create(:track, user: user, start_at: 6.hours.ago, end_at: 5.hours.ago) } + let(:track_ids) { [track1.id, track2.id] } + + it 'returns an array of serialized tracks' do + expect(serializer).to be_an(Array) + expect(serializer.length).to eq(2) + end + + it 'serializes each track correctly' do + serialized_ids = serializer.map { |track| track[:id] } + expect(serialized_ids).to contain_exactly(track1.id, track2.id) + expect(serialized_ids).not_to include(track3.id) + end + + it 'formats timestamps as ISO8601 for all tracks' do + serializer.each do |track| + expect(track[:start_at]).to match(/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/) + expect(track[:end_at]).to match(/\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/) + end + end + + it 'includes all required fields for each track' do + serializer.each do |track| + expect(track.keys).to contain_exactly( + :id, :start_at, :end_at, :distance, :avg_speed, :duration, + :elevation_gain, :elevation_loss, :elevation_max, :elevation_min, :original_path + ) + end + end + + it 'handles numeric values correctly' do + serializer.each do |track| + expect(track[:distance]).to be_a(Numeric) + expect(track[:avg_speed]).to be_a(Numeric) + expect(track[:duration]).to be_a(Numeric) + expect(track[:elevation_gain]).to be_a(Numeric) + expect(track[:elevation_loss]).to be_a(Numeric) + expect(track[:elevation_max]).to be_a(Numeric) + expect(track[:elevation_min]).to be_a(Numeric) + end + end + + it 'orders tracks by start_at in ascending order' do + serialized_tracks = serializer + expect(serialized_tracks.first[:id]).to eq(track2.id) # Started 4 hours ago + expect(serialized_tracks.second[:id]).to eq(track1.id) # Started 2 hours ago + end + end + + context 'when track IDs belong to different users' do + subject(:serializer) { described_class.new(user, track_ids).call } + + let(:other_user) { create(:user) } + let!(:user_track) { create(:track, user: user) } + let!(:other_user_track) { create(:track, user: other_user) } + let(:track_ids) { [user_track.id, other_user_track.id] } + + it 'only returns tracks belonging to the specified user' do + serialized_ids = serializer.map { |track| track[:id] } + expect(serialized_ids).to contain_exactly(user_track.id) + expect(serialized_ids).not_to include(other_user_track.id) + end + end + + context 'when track IDs array is empty' do + subject(:serializer) { described_class.new(user, []).call } + + it 'returns an empty array' do + expect(serializer).to eq([]) + end + end + + context 'when track IDs contain non-existent IDs' do + subject(:serializer) { described_class.new(user, track_ids).call } + + let!(:existing_track) { create(:track, user: user) } + let(:track_ids) { [existing_track.id, 999999] } + + it 'only returns existing tracks' do + serialized_ids = serializer.map { |track| track[:id] } + expect(serialized_ids).to contain_exactly(existing_track.id) + expect(serializer.length).to eq(1) + end + end + end +end diff --git a/spec/services/tracks/incremental_processor_spec.rb b/spec/services/tracks/incremental_processor_spec.rb index a2d21bd5..165af52d 100644 --- a/spec/services/tracks/incremental_processor_spec.rb +++ b/spec/services/tracks/incremental_processor_spec.rb @@ -30,7 +30,7 @@ RSpec.describe Tracks::IncrementalProcessor do it 'processes first point' do expect(Tracks::CreateJob).to receive(:perform_later) - .with(user.id, start_at: nil, end_at: nil, mode: :none) + .with(user.id, start_at: nil, end_at: nil, mode: :incremental) processor.call end end @@ -47,7 +47,7 @@ RSpec.describe Tracks::IncrementalProcessor do it 'processes when time threshold exceeded' do expect(Tracks::CreateJob).to receive(:perform_later) - .with(user.id, start_at: nil, end_at: Time.zone.at(previous_point.timestamp), mode: :none) + .with(user.id, start_at: nil, end_at: Time.zone.at(previous_point.timestamp), mode: :incremental) processor.call end end @@ -65,7 +65,7 @@ RSpec.describe Tracks::IncrementalProcessor do it 'uses existing track end time as start_at' do expect(Tracks::CreateJob).to receive(:perform_later) - .with(user.id, start_at: existing_track.end_at, end_at: Time.zone.at(previous_point.timestamp), mode: :none) + .with(user.id, start_at: existing_track.end_at, end_at: Time.zone.at(previous_point.timestamp), mode: :incremental) processor.call end end @@ -88,7 +88,7 @@ RSpec.describe Tracks::IncrementalProcessor do it 'processes when distance threshold exceeded' do expect(Tracks::CreateJob).to receive(:perform_later) - .with(user.id, start_at: nil, end_at: Time.zone.at(previous_point.timestamp), mode: :none) + .with(user.id, start_at: nil, end_at: Time.zone.at(previous_point.timestamp), mode: :incremental) processor.call end end diff --git a/spec/services/visits/suggest_spec.rb b/spec/services/visits/suggest_spec.rb index be56338c..4a6df048 100644 --- a/spec/services/visits/suggest_spec.rb +++ b/spec/services/visits/suggest_spec.rb @@ -75,7 +75,6 @@ RSpec.describe Visits::Suggest do end context 'when reverse geocoding is enabled' do - # Use a different time range to avoid interference with main tests let(:reverse_geocoding_start_at) { Time.zone.local(2020, 6, 1, 0, 0, 0) } let(:reverse_geocoding_end_at) { Time.zone.local(2020, 6, 1, 2, 0, 0) } diff --git a/spec/support/redis.rb b/spec/support/redis.rb index 6ffa0528..d473b269 100644 --- a/spec/support/redis.rb +++ b/spec/support/redis.rb @@ -2,7 +2,6 @@ RSpec.configure do |config| config.before(:each) do - # Clear the cache before each test Rails.cache.clear end end From 8b03b0c7f55b62c7050e610503a8d0c76193e40a Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 20 Jul 2025 19:14:20 +0200 Subject: [PATCH 21/30] Recalculate stats after changing distance units --- CHANGELOG.md | 15 +++++++++++++++ app/jobs/bulk_stats_calculating_job.rb | 2 +- ...lculate_stats_after_changing_distance_units.rb | 11 +++++++++++ db/data_schema.rb | 2 +- 4 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 db/data/20250720171241_recalculate_stats_after_changing_distance_units.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 21a189d3..f2b2acf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # [0.29.2] - 2025-07-12 +⚠️ If you were using RC, please run the following commands in the console, otherwise read on. ⚠️ + +```ruby +# This will delete all tracks 👇 +Track.delete_all + +# This will remove all tracks relations from points 👇 +Point.update_all(track_id: nil) + +# This will create tracks for all users 👇 +User.find_each do |user| + Tracks::CreateJob.perform_later(user.id, start_at: nil, end_at: nil, mode: :bulk) +end +``` + ## Added - In the User Settings -> Background Jobs, you can now disable visits suggestions, which is enabled by default. It's a background task that runs every day around midnight. Disabling it might be useful if you don't want to receive visits suggestions or if you're using the Dawarich iOS app, which has its own visits suggestions. diff --git a/app/jobs/bulk_stats_calculating_job.rb b/app/jobs/bulk_stats_calculating_job.rb index 8cc2ba46..4311a361 100644 --- a/app/jobs/bulk_stats_calculating_job.rb +++ b/app/jobs/bulk_stats_calculating_job.rb @@ -4,7 +4,7 @@ class BulkStatsCalculatingJob < ApplicationJob queue_as :stats def perform - user_ids = User.pluck(:id) + user_ids = User.active.pluck(:id) user_ids.each do |user_id| Stats::BulkCalculator.new(user_id).call diff --git a/db/data/20250720171241_recalculate_stats_after_changing_distance_units.rb b/db/data/20250720171241_recalculate_stats_after_changing_distance_units.rb new file mode 100644 index 00000000..6b23deaf --- /dev/null +++ b/db/data/20250720171241_recalculate_stats_after_changing_distance_units.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class RecalculateStatsAfterChangingDistanceUnits < ActiveRecord::Migration[8.0] + def up + BulkStatsCalculatingJob.perform_later + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/data_schema.rb b/db/data_schema.rb index 0fac2063..bdbae245 100644 --- a/db/data_schema.rb +++ b/db/data_schema.rb @@ -1 +1 @@ -DataMigrate::Data.define(version: 20250709195003) +DataMigrate::Data.define(version: 20250720171241) From b7aa05f4ea97669f15bc132a9c07485766c3cc9e Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 20 Jul 2025 21:29:05 +0200 Subject: [PATCH 22/30] Fix specs --- spec/factories/users.rb | 2 +- spec/jobs/tracks/create_job_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/factories/users.rb b/spec/factories/users.rb index e74f97fd..9f1e0140 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -13,7 +13,7 @@ FactoryBot.define do settings do { - 'route_opacity' => '0.5', + 'route_opacity' => '50', 'meters_between_routes' => '500', 'minutes_between_routes' => '30', 'fog_of_war_meters' => '100', diff --git a/spec/jobs/tracks/create_job_spec.rb b/spec/jobs/tracks/create_job_spec.rb index 7303ed3d..bc2648d9 100644 --- a/spec/jobs/tracks/create_job_spec.rb +++ b/spec/jobs/tracks/create_job_spec.rb @@ -132,7 +132,7 @@ RSpec.describe Tracks::CreateJob, type: :job do user, start_at: nil, end_at: nil, - mode: :bulk + mode: :incremental ) expect(generator_instance).to have_received(:call) expect(Notifications::Create).to have_received(:new).with( From 6ec24ffc3dca98de0697753e60bb04605a19551a Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 20 Jul 2025 21:38:46 +0200 Subject: [PATCH 23/30] Optimize bulk visits suggesting job --- app/jobs/bulk_visits_suggesting_job.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/jobs/bulk_visits_suggesting_job.rb b/app/jobs/bulk_visits_suggesting_job.rb index 4384be6a..dbef2d04 100644 --- a/app/jobs/bulk_visits_suggesting_job.rb +++ b/app/jobs/bulk_visits_suggesting_job.rb @@ -11,13 +11,14 @@ class BulkVisitsSuggestingJob < ApplicationJob return unless DawarichSettings.reverse_geocoding_enabled? users = user_ids.any? ? User.active.where(id: user_ids) : User.active + users = users.select { _1.safe_settings.visits_suggestions_enabled? } + start_at = start_at.to_datetime end_at = end_at.to_datetime time_chunks = Visits::TimeChunks.new(start_at:, end_at:).call - users.active.find_each do |user| - next unless user.safe_settings.visits_suggestions_enabled? + users.find_each do |user| next if user.tracked_points.empty? schedule_chunked_jobs(user, time_chunks) From c74ba7d1fe99825ba85c2c31f2bd1ce4d8ef4888 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 20 Jul 2025 21:54:00 +0200 Subject: [PATCH 24/30] Revert "Optimize bulk visits suggesting job" --- app/jobs/bulk_visits_suggesting_job.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/jobs/bulk_visits_suggesting_job.rb b/app/jobs/bulk_visits_suggesting_job.rb index dbef2d04..4384be6a 100644 --- a/app/jobs/bulk_visits_suggesting_job.rb +++ b/app/jobs/bulk_visits_suggesting_job.rb @@ -11,14 +11,13 @@ class BulkVisitsSuggestingJob < ApplicationJob return unless DawarichSettings.reverse_geocoding_enabled? users = user_ids.any? ? User.active.where(id: user_ids) : User.active - users = users.select { _1.safe_settings.visits_suggestions_enabled? } - start_at = start_at.to_datetime end_at = end_at.to_datetime time_chunks = Visits::TimeChunks.new(start_at:, end_at:).call - users.find_each do |user| + users.active.find_each do |user| + next unless user.safe_settings.visits_suggestions_enabled? next if user.tracked_points.empty? schedule_chunked_jobs(user, time_chunks) From fbdf630502963a4820adde5feec83902b2fa6243 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sun, 20 Jul 2025 22:23:08 +0200 Subject: [PATCH 25/30] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2b2acf8..9452c761 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ end } } ``` +- Links in emails will be based on the `DOMAIN` environment variable instead of `SMTP_DOMAIN`. ## Fixed From 59a4d760bff4d6a070d66d1e845c49ac1614f7c4 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Mon, 21 Jul 2025 18:59:13 +0200 Subject: [PATCH 26/30] Fix owntracks points creation --- CHANGELOG.md | 2 ++ app/services/own_tracks/rec_parser.rb | 4 ++++ spec/fixtures/files/owntracks/2023-02_old.rec | 10 ++++++++++ spec/services/own_tracks/importer_spec.rb | 14 ++++++++++++++ 4 files changed, 30 insertions(+) create mode 100644 spec/fixtures/files/owntracks/2023-02_old.rec diff --git a/CHANGELOG.md b/CHANGELOG.md index 9452c761..0b9b9fd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,8 @@ end - Swagger documentation is now valid again. - Invalid owntracks points are now ignored. +- An older Owntrack's .rec format is now also supported. +- Course and course accuracy are now rounded to 8 decimal places to fix the issue with points creation. # [0.29.1] - 2025-07-02 diff --git a/app/services/own_tracks/rec_parser.rb b/app/services/own_tracks/rec_parser.rb index 7e3550af..74959460 100644 --- a/app/services/own_tracks/rec_parser.rb +++ b/app/services/own_tracks/rec_parser.rb @@ -9,8 +9,12 @@ class OwnTracks::RecParser def call file.split("\n").map do |line| + # Try tab-separated first, then fall back to whitespace-separated parts = line.split("\t") + # If tab splitting didn't work (only 1 part), try whitespace splitting + parts = line.split(/\s+/) if parts.size == 1 + Oj.load(parts[2]) if parts.size > 2 && parts[1].strip == '*' end.compact end diff --git a/spec/fixtures/files/owntracks/2023-02_old.rec b/spec/fixtures/files/owntracks/2023-02_old.rec new file mode 100644 index 00000000..a87c0aaf --- /dev/null +++ b/spec/fixtures/files/owntracks/2023-02_old.rec @@ -0,0 +1,10 @@ +2023-02-20T18:46:22Z * {"_type":"location","BSSID":"6c:c4:9f:e0:bb:b1","SSID":"WiFi","acc":14,"alt":136,"batt":38,"bs":1,"conn":"w","created_at":1676918783,"lat":22.0687934,"lon":24.7941786,"m":2,"tid":"l6","topic":"owntracks/pixel6/pixel99","tst":1676918782,"vac":0,"vel":0,"_http":true} +2023-02-20T18:46:25Z * {"_type":"location","BSSID":"6c:c4:9f:e0:bb:b1","SSID":"WiFi","acc":13,"alt":136,"batt":38,"bs":1,"conn":"w","created_at":1676918785,"lat":22.0687967,"lon":24.7941813,"m":2,"tid":"l6","topic":"owntracks/pixel6/pixel99","tst":1676918785,"vac":0,"vel":0,"_http":true} +2023-02-20T18:46:25Z * {"_type":"location","BSSID":"6c:c4:9f:e0:bb:b1","SSID":"WiFi","acc":13,"alt":136,"batt":38,"bs":1,"conn":"w","created_at":1676918790,"lat":22.0687967,"lon":24.7941813,"m":2,"tid":"l6","topic":"owntracks/pixel6/pixel99","tst":1676918785,"vac":0,"vel":0,"_http":true} +2023-02-20T18:46:35Z * {"_type":"location","BSSID":"6c:c4:9f:e0:bb:b1","SSID":"WiFi","acc":14,"alt":136,"batt":38,"bs":1,"conn":"w","created_at":1676918795,"lat":22.0687906,"lon":24.794195,"m":2,"tid":"l6","topic":"owntracks/pixel6/pixel99","tst":1676918795,"vac":0,"vel":0,"_http":true} +2023-02-20T18:46:40Z * {"_type":"location","BSSID":"6c:c4:9f:e0:bb:b1","SSID":"WiFi","acc":14,"alt":136,"batt":38,"bs":1,"conn":"w","created_at":1676918800,"lat":22.0687967,"lon":24.7941859,"m":2,"tid":"l6","topic":"owntracks/pixel6/pixel99","tst":1676918800,"vac":0,"vel":0,"_http":true} +2023-02-20T18:46:45Z * {"_type":"location","BSSID":"6c:c4:9f:e0:bb:b1","SSID":"WiFi","acc":14,"alt":136,"batt":38,"bs":1,"conn":"w","created_at":1676918805,"lat":22.0687946,"lon":24.7941883,"m":2,"tid":"l6","topic":"owntracks/pixel6/pixel99","tst":1676918805,"vac":0,"vel":0,"_http":true} +2023-02-20T18:46:50Z * {"_type":"location","BSSID":"6c:c4:9f:e0:bb:b1","SSID":"WiFi","acc":14,"alt":136,"batt":38,"bs":1,"conn":"w","created_at":1676918810,"lat":22.0687912,"lon":24.7941837,"m":2,"tid":"l6","topic":"owntracks/pixel6/pixel99","tst":1676918810,"vac":0,"vel":0,"_http":true} +2023-02-20T18:46:55Z * {"_type":"location","BSSID":"6c:c4:9f:e0:bb:b1","SSID":"WiFi","acc":14,"alt":136,"batt":38,"bs":1,"conn":"w","created_at":1676918815,"lat":22.0687927,"lon":24.794186,"m":2,"tid":"l6","topic":"owntracks/pixel6/pixel99","tst":1676918815,"vac":0,"vel":0,"_http":true} +2023-02-20T18:46:55Z * {"_type":"location","BSSID":"6c:c4:9f:e0:bb:b1","SSID":"WiFi","acc":14,"alt":136,"batt":38,"bs":1,"conn":"w","created_at":1676918815,"lat":22.0687937,"lon":24.794186,"m":2,"tid":"l6","topic":"owntracks/pixel6/pixel99","tst":1676918815,"vac":0,"vel":0,"_http":true} +2023-02-20T18:47:00Z * {"_type":"location","BSSID":"6c:c4:9f:e0:bb:b1","SSID":"WiFi","acc":14,"alt":136,"batt":38,"bs":1,"conn":"w","created_at":1676918820,"lat":22.0687937,"lon":24.794186,"m":2,"tid":"l6","topic":"owntracks/pixel6/pixel99","tst":1676918820,"vac":0,"vel":0,"_http":true} diff --git a/spec/services/own_tracks/importer_spec.rb b/spec/services/own_tracks/importer_spec.rb index 0800d0b8..842883f8 100644 --- a/spec/services/own_tracks/importer_spec.rb +++ b/spec/services/own_tracks/importer_spec.rb @@ -78,5 +78,19 @@ RSpec.describe OwnTracks::Importer do expect(Point.first.velocity).to eq('1.4') end end + + context 'when file is old' do + let(:file_path) { Rails.root.join('spec/fixtures/files/owntracks/2023-02_old.rec') } + + it 'creates points' do + expect { parser }.to change { Point.count }.by(9) + end + + it 'correctly writes attributes' do + parser + + point = Point.first + end + end end end From 6a6c3c938fc5b2dc888971204a387629619023e6 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Mon, 21 Jul 2025 19:00:28 +0200 Subject: [PATCH 27/30] Fix distance calculation --- app/controllers/map_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/map_controller.rb b/app/controllers/map_controller.rb index f335d6ef..5fcdabc1 100644 --- a/app/controllers/map_controller.rb +++ b/app/controllers/map_controller.rb @@ -36,17 +36,17 @@ class MapController < ApplicationController end def calculate_distance - total_distance_meters = 0 + total_distance = 0 @coordinates.each_cons(2) do distance_km = Geocoder::Calculations.distance_between( [_1[0], _1[1]], [_2[0], _2[1]], units: :km ) - total_distance_meters += distance_km + total_distance += distance_km end - total_distance_meters.round + total_distance.round end def parsed_start_at From 9bcd522e25d3687d42f8f71e6484065a57c4fdd5 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Mon, 21 Jul 2025 20:22:18 +0200 Subject: [PATCH 28/30] Update specs --- spec/factories/users.rb | 2 +- spec/serializers/track_serializer_spec.rb | 2 +- spec/system/map_interaction_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 9f1e0140..c9eb856e 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -13,7 +13,7 @@ FactoryBot.define do settings do { - 'route_opacity' => '50', + 'route_opacity' => 60, 'meters_between_routes' => '500', 'minutes_between_routes' => '30', 'fog_of_war_meters' => '100', diff --git a/spec/serializers/track_serializer_spec.rb b/spec/serializers/track_serializer_spec.rb index 6213e2c9..6622b23d 100644 --- a/spec/serializers/track_serializer_spec.rb +++ b/spec/serializers/track_serializer_spec.rb @@ -92,7 +92,7 @@ RSpec.describe TrackSerializer do let(:track) { create(:track, user: user, original_path: 'LINESTRING(0 0, 1 1, 2 2)') } it 'converts geometry to WKT string format' do - expect(serialized_track[:original_path]).to eq('LINESTRING (0 0, 1 1, 2 2)') + expect(serialized_track[:original_path]).to match(/LINESTRING \(0(\.0)? 0(\.0)?, 1(\.0)? 1(\.0)?, 2(\.0)? 2(\.0)?\)/) expect(serialized_track[:original_path]).to be_a(String) end end diff --git a/spec/system/map_interaction_spec.rb b/spec/system/map_interaction_spec.rb index d8116bba..234736ec 100644 --- a/spec/system/map_interaction_spec.rb +++ b/spec/system/map_interaction_spec.rb @@ -447,7 +447,7 @@ RSpec.describe 'Map Interaction', type: :system do # Find and update route opacity within('.leaflet-settings-panel') do opacity_input = find('#route-opacity') - expect(opacity_input.value).to eq('50') # Default value + expect(opacity_input.value).to eq('60') # Default value # Change opacity to 80% opacity_input.fill_in(with: '80') From 2206622726716e3e381ab29474697a34532bfcca Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Mon, 21 Jul 2025 20:35:43 +0200 Subject: [PATCH 29/30] Release 0.30.0 --- .app_version | 2 +- CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.app_version b/.app_version index 20f06870..c25c8e5b 100644 --- a/.app_version +++ b/.app_version @@ -1 +1 @@ -0.29.2 +0.30.0 diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b9b9fd7..89e12393 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -# [0.29.2] - 2025-07-12 +# [0.30.0] - 2025-07-21 ⚠️ If you were using RC, please run the following commands in the console, otherwise read on. ⚠️ From 7afc399724fa0599f2970afba94d5c34e68b13fd Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Mon, 21 Jul 2025 22:27:20 +0200 Subject: [PATCH 30/30] Add cache to points limit exceeded check --- .app_version | 2 +- CHANGELOG.md | 8 +++++++- app/services/points_limit_exceeded.rb | 9 +++++++-- spec/services/points_limit_exceeded_spec.rb | 5 +++++ 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/.app_version b/.app_version index c25c8e5b..1a44cad7 100644 --- a/.app_version +++ b/.app_version @@ -1 +1 @@ -0.30.0 +0.30.1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 89e12393..04d1a143 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,16 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +# [0.30.1] - 2025-07-21 + +## Fixed + +- Points limit exceeded check is now cached. + # [0.30.0] - 2025-07-21 -⚠️ If you were using RC, please run the following commands in the console, otherwise read on. ⚠️ +⚠️ If you were using 0.29.2 RC, please run the following commands in the console, otherwise read on. ⚠️ ```ruby # This will delete all tracks 👇 diff --git a/app/services/points_limit_exceeded.rb b/app/services/points_limit_exceeded.rb index f47543d1..2bf8de8a 100644 --- a/app/services/points_limit_exceeded.rb +++ b/app/services/points_limit_exceeded.rb @@ -7,13 +7,18 @@ class PointsLimitExceeded def call return false if DawarichSettings.self_hosted? - return true if @user.tracked_points.count >= points_limit - false + Rails.cache.fetch(cache_key, expires_in: 1.day) do + @user.tracked_points.count >= points_limit + end end private + def cache_key + "points_limit_exceeded/#{@user.id}" + end + def points_limit DawarichSettings::BASIC_PAID_PLAN_LIMIT end diff --git a/spec/services/points_limit_exceeded_spec.rb b/spec/services/points_limit_exceeded_spec.rb index 88cd6268..fed8a880 100644 --- a/spec/services/points_limit_exceeded_spec.rb +++ b/spec/services/points_limit_exceeded_spec.rb @@ -28,6 +28,11 @@ RSpec.describe PointsLimitExceeded do end it { is_expected.to be true } + + it 'caches the result' do + expect(user.tracked_points).to receive(:count).once + 2.times { described_class.new(user).call } + end end context 'when user points count exceeds the limit' do