diff --git a/.app_version b/.app_version index 0f721773..e8262eb5 100644 --- a/.app_version +++ b/.app_version @@ -1 +1 @@ -0.30.2 +0.30.3 diff --git a/CHANGELOG.md b/CHANGELOG.md index e7c5a882..2046bacf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ 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.3] - 2025-07-23 + +## Changed + +- Track generation is now significantly faster and less resource intensive. + +## Fixed + +- Distance on the stats page is now rounded. #1548 +- Non-selfhosted users can now export and import their account data. + + # [0.30.2] - 2025-07-22 ## Fixed diff --git a/app/controllers/settings/users_controller.rb b/app/controllers/settings/users_controller.rb index d8696617..f00a28ce 100644 --- a/app/controllers/settings/users_controller.rb +++ b/app/controllers/settings/users_controller.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class Settings::UsersController < ApplicationController - before_action :authenticate_self_hosted! + before_action :authenticate_self_hosted!, except: [:export, :import] before_action :authenticate_admin!, except: [:export, :import] - before_action :authenticate_user!, only: [:export, :import] + before_action :authenticate_user! def index @users = User.order(created_at: :desc) diff --git a/app/models/concerns/distanceable.rb b/app/models/concerns/distanceable.rb index 7ddc190d..532cfa7f 100644 --- a/app/models/concerns/distanceable.rb +++ b/app/models/concerns/distanceable.rb @@ -5,7 +5,6 @@ module Distanceable module ClassMethods def total_distance(points = nil, unit = :km) - # Handle method being called directly on relation vs with array if points.nil? calculate_distance_for_relation(unit) else @@ -50,16 +49,48 @@ module Distanceable return 0 if points.length < 2 - total_meters = points.each_cons(2).sum do |point1, point2| - connection.select_value( - 'SELECT ST_Distance(ST_GeomFromEWKT($1)::geography, ST_GeomFromEWKT($2)::geography)', - nil, - [point1.lonlat, point2.lonlat] - ) - end + total_meters = calculate_batch_distances(points).sum total_meters.to_f / ::DISTANCE_UNITS[unit.to_sym] end + + def calculate_batch_distances(points) + return [] if points.length < 2 + + point_pairs = points.each_cons(2).to_a + return [] if point_pairs.empty? + + # Create parameterized placeholders for VALUES clause using ? placeholders + values_placeholders = point_pairs.map do |_| + "(?, ST_GeomFromEWKT(?)::geography, ST_GeomFromEWKT(?)::geography)" + end.join(', ') + + # Flatten parameters: [pair_id, lonlat1, lonlat2, pair_id, lonlat1, lonlat2, ...] + params = point_pairs.flat_map.with_index do |(p1, p2), index| + [index, p1.lonlat, p2.lonlat] + end + + # Single query to calculate all distances using parameterized query + sql_with_params = ActiveRecord::Base.sanitize_sql_array([<<-SQL.squish] + params) + WITH point_pairs AS ( + SELECT + pair_id, + point1, + point2 + FROM (VALUES #{values_placeholders}) AS t(pair_id, point1, point2) + ) + SELECT + pair_id, + ST_Distance(point1, point2) as distance_meters + FROM point_pairs + ORDER BY pair_id + SQL + + results = connection.select_all(sql_with_params) + + # Return array of distances in meters + results.map { |row| row['distance_meters'].to_f } + end end def distance_to(other_point, unit = :km) @@ -67,7 +98,6 @@ module Distanceable raise ArgumentError, "Invalid unit. Supported units are: #{::DISTANCE_UNITS.keys.join(', ')}" end - # Extract coordinates based on what type other_point is other_lonlat = extract_point(other_point) return nil if other_lonlat.nil? diff --git a/app/models/track.rb b/app/models/track.rb index 9e9724a7..f02071d2 100644 --- a/app/models/track.rb +++ b/app/models/track.rb @@ -25,6 +25,114 @@ class Track < ApplicationRecord .first end + def self.segment_points_in_sql(user_id, start_timestamp, end_timestamp, time_threshold_minutes, distance_threshold_meters, untracked_only: false) + time_threshold_seconds = time_threshold_minutes * 60 + + where_clause = if untracked_only + "WHERE user_id = $1 AND timestamp BETWEEN $2 AND $3 AND track_id IS NULL" + else + "WHERE user_id = $1 AND timestamp BETWEEN $2 AND $3" + end + + sql = <<~SQL + WITH points_with_gaps AS ( + SELECT + id, + timestamp, + lonlat, + LAG(lonlat) OVER (ORDER BY timestamp) as prev_lonlat, + LAG(timestamp) OVER (ORDER BY timestamp) as prev_timestamp, + ST_Distance( + lonlat::geography, + LAG(lonlat) OVER (ORDER BY timestamp)::geography + ) as distance_meters, + (timestamp - LAG(timestamp) OVER (ORDER BY timestamp)) as time_diff_seconds + FROM points + #{where_clause} + ORDER BY timestamp + ), + segment_breaks AS ( + SELECT *, + CASE + WHEN prev_lonlat IS NULL THEN 1 + WHEN time_diff_seconds > $4 THEN 1 + WHEN distance_meters > $5 THEN 1 + ELSE 0 + END as is_break + FROM points_with_gaps + ), + segments AS ( + SELECT *, + SUM(is_break) OVER (ORDER BY timestamp ROWS UNBOUNDED PRECEDING) as segment_id + FROM segment_breaks + ) + SELECT + segment_id, + array_agg(id ORDER BY timestamp) as point_ids, + count(*) as point_count, + min(timestamp) as start_timestamp, + max(timestamp) as end_timestamp, + sum(COALESCE(distance_meters, 0)) as total_distance_meters + FROM segments + GROUP BY segment_id + HAVING count(*) >= 2 + ORDER BY segment_id + SQL + + results = Point.connection.exec_query( + sql, + 'segment_points_in_sql', + [user_id, start_timestamp, end_timestamp, time_threshold_seconds, distance_threshold_meters] + ) + + # Convert results to segment data + segments_data = [] + results.each do |row| + segments_data << { + segment_id: row['segment_id'].to_i, + point_ids: parse_postgres_array(row['point_ids']), + point_count: row['point_count'].to_i, + start_timestamp: row['start_timestamp'].to_i, + end_timestamp: row['end_timestamp'].to_i, + total_distance_meters: row['total_distance_meters'].to_f + } + end + + segments_data + end + + # Get actual Point objects for each segment with pre-calculated distances + def self.get_segments_with_points(user_id, start_timestamp, end_timestamp, time_threshold_minutes, distance_threshold_meters, untracked_only: false) + segments_data = segment_points_in_sql( + user_id, + start_timestamp, + end_timestamp, + time_threshold_minutes, + distance_threshold_meters, + untracked_only: untracked_only + ) + + point_ids = segments_data.flat_map { |seg| seg[:point_ids] } + points_by_id = Point.where(id: point_ids).index_by(&:id) + + segments_data.map do |seg_data| + { + points: seg_data[:point_ids].map { |id| points_by_id[id] }.compact, + pre_calculated_distance: seg_data[:total_distance_meters], + start_timestamp: seg_data[:start_timestamp], + end_timestamp: seg_data[:end_timestamp] + } + end + end + + # Parse PostgreSQL array format like "{1,2,3}" into Ruby array + def self.parse_postgres_array(pg_array_string) + return [] if pg_array_string.nil? || pg_array_string.empty? + + # Remove curly braces and split by comma + pg_array_string.gsub(/[{}]/, '').split(',').map(&:to_i) + end + private def broadcast_track_created @@ -45,23 +153,7 @@ class Track < ApplicationRecord def broadcast_track_update(action) TracksChannel.broadcast_to(user, { action: action, - track: serialize_track_data + track: TrackSerializer.new(self).call }) end - - def serialize_track_data - { - id: id, - start_at: start_at.iso8601, - end_at: end_at.iso8601, - distance: distance.to_i, - avg_speed: avg_speed.to_f, - duration: duration, - elevation_gain: elevation_gain, - elevation_loss: elevation_loss, - elevation_max: elevation_max, - elevation_min: elevation_min, - original_path: original_path.to_s - } - end end diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index 9ffcdbb7..be22d021 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -40,12 +40,20 @@ class Tracks::Generator def call clean_existing_tracks if should_clean_tracks? - points = load_points - Rails.logger.debug "Generator: loaded #{points.size} points for user #{user.id} in #{mode} mode" - return 0 if points.empty? + start_timestamp, end_timestamp = get_timestamp_range - segments = split_points_into_segments(points) - Rails.logger.debug "Generator: created #{segments.size} segments" + Rails.logger.debug "Generator: querying points for user #{user.id} in #{mode} mode" + + segments = Track.get_segments_with_points( + user.id, + start_timestamp, + end_timestamp, + time_threshold_minutes, + distance_threshold_meters, + untracked_only: mode == :incremental + ) + + Rails.logger.debug "Generator: created #{segments.size} segments via SQL" tracks_created = 0 @@ -99,11 +107,14 @@ class Tracks::Generator 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 + def create_track_from_segment(segment_data) + points = segment_data[:points] + pre_calculated_distance = segment_data[:pre_calculated_distance] - track = create_track_from_points(segment) + Rails.logger.debug "Generator: processing segment with #{points.size} points" + return unless points.size >= 2 + + track = create_track_from_points(points, pre_calculated_distance) Rails.logger.debug "Generator: created track #{track&.id}" track end @@ -171,7 +182,37 @@ class Tracks::Generator scope.destroy_all end - # Threshold methods from safe_settings + def get_timestamp_range + case mode + when :bulk then bulk_timestamp_range + when :daily then daily_timestamp_range + when :incremental then incremental_timestamp_range + else + raise ArgumentError, "Unknown mode: #{mode}" + end + end + + def bulk_timestamp_range + return [start_at.to_i, end_at.to_i] if start_at && end_at + + first_point = user.tracked_points.order(:timestamp).first + last_point = user.tracked_points.order(:timestamp).last + + [first_point&.timestamp || 0, last_point&.timestamp || Time.current.to_i] + end + + def daily_timestamp_range + day = start_at&.to_date || Date.current + [day.beginning_of_day.to_i, day.end_of_day.to_i] + end + + def incremental_timestamp_range + first_point = user.tracked_points.where(track_id: nil).order(:timestamp).first + end_timestamp = end_at ? end_at.to_i : Time.current.to_i + + [first_point&.timestamp || 0, end_timestamp] + end + def distance_threshold_meters @distance_threshold_meters ||= user.safe_settings.meters_between_routes.to_i end diff --git a/app/services/tracks/segmentation.rb b/app/services/tracks/segmentation.rb index 57ca3b03..e5b4477d 100644 --- a/app/services/tracks/segmentation.rb +++ b/app/services/tracks/segmentation.rb @@ -86,11 +86,13 @@ module Tracks::Segmentation end def calculate_km_distance_between_points(point1, point2) - lat1, lon1 = point_coordinates(point1) - lat2, lon2 = point_coordinates(point2) + distance_meters = Point.connection.select_value( + 'SELECT ST_Distance(ST_GeomFromEWKT($1)::geography, ST_GeomFromEWKT($2)::geography)', + nil, + [point1.lonlat, point2.lonlat] + ) - # Use Geocoder to match behavior with frontend (same library used elsewhere in app) - Geocoder::Calculations.distance_between([lat1, lon1], [lat2, lon2], units: :km) + distance_meters.to_f / 1000.0 # Convert meters to kilometers end def should_finalize_segment?(segment_points, grace_period_minutes = 5) diff --git a/app/services/tracks/track_builder.rb b/app/services/tracks/track_builder.rb index 99830bc1..0ccd82b0 100644 --- a/app/services/tracks/track_builder.rb +++ b/app/services/tracks/track_builder.rb @@ -49,7 +49,7 @@ module Tracks::TrackBuilder extend ActiveSupport::Concern - def create_track_from_points(points) + def create_track_from_points(points, pre_calculated_distance) return nil if points.size < 2 track = Track.new( @@ -59,17 +59,16 @@ module Tracks::TrackBuilder original_path: build_path(points) ) - # Calculate track statistics - track.distance = calculate_track_distance(points) - track.duration = calculate_duration(points) + track.distance = pre_calculated_distance.round + track.duration = calculate_duration(points) track.avg_speed = calculate_average_speed(track.distance, track.duration) - # Calculate elevation statistics + # Calculate elevation statistics (no DB queries needed) elevation_stats = calculate_elevation_stats(points) track.elevation_gain = elevation_stats[:gain] track.elevation_loss = elevation_stats[:loss] - track.elevation_max = elevation_stats[:max] - track.elevation_min = elevation_stats[:min] + track.elevation_max = elevation_stats[:max] + track.elevation_min = elevation_stats[:min] if track.save Point.where(id: points.map(&:id)).update_all(track_id: track.id) @@ -77,7 +76,6 @@ module Tracks::TrackBuilder track else Rails.logger.error "Failed to create track for user #{user.id}: #{track.errors.full_messages.join(', ')}" - nil end end @@ -101,6 +99,7 @@ module Tracks::TrackBuilder # Speed in meters per second, then convert to km/h for storage speed_mps = distance_in_meters.to_f / duration_seconds + (speed_mps * 3.6).round(2) # m/s to km/h end diff --git a/app/views/stats/index.html.erb b/app/views/stats/index.html.erb index a5ab367b..bd06de8e 100644 --- a/app/views/stats/index.html.erb +++ b/app/views/stats/index.html.erb @@ -21,7 +21,9 @@ <% end %> - <%= link_to 'Update stats', update_all_stats_path, data: { turbo_method: :put }, class: 'btn btn-primary mt-5' %> + <% if current_user.active? %> + <%= link_to 'Update stats', update_all_stats_path, data: { turbo_method: :put }, class: 'btn btn-primary mt-5' %> + <% end %>
<% cache [current_user, 'year_distance_stat', year], skip_digest: true do %> - <%= number_with_delimiter year_distance_stat(year, current_user) %><%= current_user.safe_settings.distance_unit %> + <%= number_with_delimiter year_distance_stat(year, current_user).round %> <%= current_user.safe_settings.distance_unit %> <% end %>
<% if DawarichSettings.reverse_geocoding_enabled? %> diff --git a/db/migrate/20250723164055_add_track_generation_composite_index.rb b/db/migrate/20250723164055_add_track_generation_composite_index.rb new file mode 100644 index 00000000..1685011a --- /dev/null +++ b/db/migrate/20250723164055_add_track_generation_composite_index.rb @@ -0,0 +1,9 @@ +class AddTrackGenerationCompositeIndex < ActiveRecord::Migration[8.0] + disable_ddl_transaction! + + def change + add_index :points, [:user_id, :timestamp, :track_id], + algorithm: :concurrently, + name: 'idx_points_track_generation', if_not_exists: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 402729b9..25efe3c2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_07_21_204404) do +ActiveRecord::Schema[8.0].define(version: 2025_07_23_164055) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "postgis" @@ -203,6 +203,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_07_21_204404) do t.index ["timestamp"], name: "index_points_on_timestamp" t.index ["track_id"], name: "index_points_on_track_id" t.index ["trigger"], name: "index_points_on_trigger" + t.index ["user_id", "timestamp", "track_id"], name: "idx_points_track_generation" t.index ["user_id"], name: "index_points_on_user_id" t.index ["visit_id"], name: "index_points_on_visit_id" end diff --git a/lib/timestamps.rb b/lib/timestamps.rb index ea7358cc..2154a3ef 100644 --- a/lib/timestamps.rb +++ b/lib/timestamps.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true module Timestamps - def self.parse_timestamp(timestamp) begin # if the timestamp is in ISO 8601 format, try to parse it diff --git a/spec/services/tracks/track_builder_spec.rb b/spec/services/tracks/track_builder_spec.rb index 5046e60f..2213f5a2 100644 --- a/spec/services/tracks/track_builder_spec.rb +++ b/spec/services/tracks/track_builder_spec.rb @@ -39,21 +39,23 @@ RSpec.describe Tracks::TrackBuilder do ] end + let(:pre_calculated_distance) { 1500 } # 1500 meters + it 'creates a track with correct attributes' do - track = builder.create_track_from_points(points) + track = builder.create_track_from_points(points, pre_calculated_distance) expect(track).to be_persisted expect(track.user).to eq(user) expect(track.start_at).to be_within(1.second).of(Time.zone.at(points.first.timestamp)) expect(track.end_at).to be_within(1.second).of(Time.zone.at(points.last.timestamp)) - expect(track.distance).to be > 0 + expect(track.distance).to eq(1500) expect(track.duration).to eq(90.minutes.to_i) expect(track.avg_speed).to be > 0 expect(track.original_path).to be_present end it 'calculates elevation statistics correctly' do - track = builder.create_track_from_points(points) + track = builder.create_track_from_points(points, pre_calculated_distance) expect(track.elevation_gain).to eq(10) # 110 - 100 expect(track.elevation_loss).to eq(5) # 110 - 105 @@ -62,7 +64,7 @@ RSpec.describe Tracks::TrackBuilder do end it 'associates points with the track' do - track = builder.create_track_from_points(points) + track = builder.create_track_from_points(points, pre_calculated_distance) points.each(&:reload) expect(points.map(&:track)).to all(eq(track)) @@ -73,12 +75,12 @@ RSpec.describe Tracks::TrackBuilder do let(:single_point) { [create(:point, user: user)] } it 'returns nil for single point' do - result = builder.create_track_from_points(single_point) + result = builder.create_track_from_points(single_point, 1000) expect(result).to be_nil end it 'returns nil for empty array' do - result = builder.create_track_from_points([]) + result = builder.create_track_from_points([], 1000) expect(result).to be_nil end end @@ -100,7 +102,7 @@ RSpec.describe Tracks::TrackBuilder do /Failed to create track for user #{user.id}/ ) - result = builder.create_track_from_points(points) + result = builder.create_track_from_points(points, 1000) expect(result).to be_nil end end @@ -120,7 +122,7 @@ RSpec.describe Tracks::TrackBuilder do ).and_call_original result = builder.build_path(points) - expect(result).to respond_to(:as_text) + expect(result).to be_a(RGeo::Geographic::SphericalLineStringImpl) end end @@ -134,7 +136,7 @@ RSpec.describe Tracks::TrackBuilder do before do # Mock Point.total_distance to return distance in meters - allow(Point).to receive(:total_distance).and_return(1500) # 1500 meters + allow(Point).to receive(:total_distance).with(points, :m).and_return(1500) # 1500 meters end it 'stores distance in meters regardless of user unit preference' do @@ -143,7 +145,7 @@ RSpec.describe Tracks::TrackBuilder do end it 'rounds distance to nearest meter' do - allow(Point).to receive(:total_distance).and_return(1500.7) + allow(Point).to receive(:total_distance).with(points, :m).and_return(1500.7) result = builder.calculate_track_distance(points) expect(result).to eq(1501) # Rounded to nearest meter end @@ -312,13 +314,15 @@ RSpec.describe Tracks::TrackBuilder do ] end + let(:pre_calculated_distance) { 2000 } + it 'creates a complete track end-to-end' do - expect { builder.create_track_from_points(points) }.to change(Track, :count).by(1) + expect { builder.create_track_from_points(points, pre_calculated_distance) }.to change(Track, :count).by(1) track = Track.last expect(track.user).to eq(user) expect(track.points).to match_array(points) - expect(track.distance).to be > 0 + expect(track.distance).to eq(2000) expect(track.duration).to eq(1.hour.to_i) expect(track.elevation_gain).to eq(20) end