From 6e4934a93d9bf3e424161bfdca07c8622a6c244d Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Thu, 21 Aug 2025 22:32:29 +0200 Subject: [PATCH 1/4] Introduce points counter cache to optimize performance --- .../v1/countries/visited_cities_controller.rb | 2 +- app/controllers/api/v1/points_controller.rb | 6 ++-- app/controllers/home_controller.rb | 2 +- app/controllers/map_controller.rb | 2 +- app/controllers/points_controller.rb | 4 +-- app/helpers/application_helper.rb | 2 +- app/jobs/bulk_visits_suggesting_job.rb | 2 +- .../migrate_points_latlon_job.rb | 2 +- .../prefill_points_counter_cache_job.rb | 28 +++++++++++++++++++ app/jobs/tracks/cleanup_job.rb | 6 ++-- app/models/point.rb | 2 +- app/models/stat.rb | 2 +- app/models/trip.rb | 2 +- app/models/user.rb | 13 ++++----- app/serializers/stats_serializer.rb | 4 +-- app/services/exports/create.rb | 2 +- app/services/jobs/create.rb | 4 +-- app/services/points_limit_exceeded.rb | 2 +- app/services/stats/calculate_month.rb | 4 +-- app/services/tracks/generator.rb | 12 ++++---- app/services/tracks/incremental_processor.rb | 2 +- app/services/users/export_data.rb | 2 +- app/services/visits/smart_detect.rb | 2 +- app/services/visits/suggest.rb | 2 +- .../registrations/_points_usage.html.erb | 4 +-- app/views/settings/users/index.html.erb | 2 +- ...0250704185707_create_tracks_from_points.rb | 2 +- ...0250821192219_add_points_count_to_users.rb | 16 +++++++++++ db/schema.rb | 5 ++-- spec/models/user_spec.rb | 5 ++-- spec/services/points_limit_exceeded_spec.rb | 8 +++--- spec/services/users/export_data_spec.rb | 8 +++--- .../users/export_import_integration_spec.rb | 18 ++++++------ .../services/users/import_data/points_spec.rb | 10 +++---- spec/services/visits/smart_detect_spec.rb | 2 +- 35 files changed, 117 insertions(+), 74 deletions(-) create mode 100644 app/jobs/data_migrations/prefill_points_counter_cache_job.rb create mode 100644 db/migrate/20250821192219_add_points_count_to_users.rb diff --git a/app/controllers/api/v1/countries/visited_cities_controller.rb b/app/controllers/api/v1/countries/visited_cities_controller.rb index 85e53f7d..5af80348 100644 --- a/app/controllers/api/v1/countries/visited_cities_controller.rb +++ b/app/controllers/api/v1/countries/visited_cities_controller.rb @@ -8,7 +8,7 @@ class Api::V1::Countries::VisitedCitiesController < ApiController end_at = DateTime.parse(params[:end_at]).to_i points = current_api_user - .tracked_points + .points .where(timestamp: start_at..end_at) render json: { data: CountriesAndCities.new(points).call } diff --git a/app/controllers/api/v1/points_controller.rb b/app/controllers/api/v1/points_controller.rb index 505bb123..6dd2cf93 100644 --- a/app/controllers/api/v1/points_controller.rb +++ b/app/controllers/api/v1/points_controller.rb @@ -10,7 +10,7 @@ class Api::V1::PointsController < ApiController order = params[:order] || 'desc' points = current_api_user - .tracked_points + .points .where(timestamp: start_at..end_at) .order(timestamp: order) .page(params[:page]) @@ -31,7 +31,7 @@ class Api::V1::PointsController < ApiController end def update - point = current_api_user.tracked_points.find(params[:id]) + point = current_api_user.points.find(params[:id]) point.update(lonlat: "POINT(#{point_params[:longitude]} #{point_params[:latitude]})") @@ -39,7 +39,7 @@ class Api::V1::PointsController < ApiController end def destroy - point = current_api_user.tracked_points.find(params[:id]) + point = current_api_user.points.find(params[:id]) point.destroy render json: { message: 'Point deleted successfully' } diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index f77c7a54..27453c76 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -6,6 +6,6 @@ class HomeController < ApplicationController redirect_to map_url if current_user - @points = current_user.tracked_points.without_raw_data if current_user + @points = current_user.points.without_raw_data if current_user end end diff --git a/app/controllers/map_controller.rb b/app/controllers/map_controller.rb index 82d9435f..bbb308bb 100644 --- a/app/controllers/map_controller.rb +++ b/app/controllers/map_controller.rb @@ -88,6 +88,6 @@ class MapController < ApplicationController end def points_from_user - current_user.tracked_points.without_raw_data.order(timestamp: :asc) + current_user.points.without_raw_data.order(timestamp: :asc) end end diff --git a/app/controllers/points_controller.rb b/app/controllers/points_controller.rb index a78c97c4..65d99698 100644 --- a/app/controllers/points_controller.rb +++ b/app/controllers/points_controller.rb @@ -24,7 +24,7 @@ class PointsController < ApplicationController alert: 'No points selected.', status: :see_other and return if point_ids.blank? - current_user.tracked_points.where(id: point_ids).destroy_all + current_user.points.where(id: point_ids).destroy_all redirect_to points_url(preserved_params), notice: 'Points were successfully destroyed.', @@ -58,7 +58,7 @@ class PointsController < ApplicationController end def user_points - current_user.tracked_points + current_user.points end def order_by diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 5fdcd917..2fb02162 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -86,7 +86,7 @@ module ApplicationHelper end def points_exist?(year, month, user) - user.tracked_points.where( + user.points.where( timestamp: DateTime.new(year, month).beginning_of_month..DateTime.new(year, month).end_of_month ).exists? end diff --git a/app/jobs/bulk_visits_suggesting_job.rb b/app/jobs/bulk_visits_suggesting_job.rb index 4384be6a..e52b06da 100644 --- a/app/jobs/bulk_visits_suggesting_job.rb +++ b/app/jobs/bulk_visits_suggesting_job.rb @@ -18,7 +18,7 @@ class BulkVisitsSuggestingJob < ApplicationJob users.active.find_each do |user| next unless user.safe_settings.visits_suggestions_enabled? - next if user.tracked_points.empty? + next unless user.points_count.positive? schedule_chunked_jobs(user, time_chunks) end diff --git a/app/jobs/data_migrations/migrate_points_latlon_job.rb b/app/jobs/data_migrations/migrate_points_latlon_job.rb index 90dfa096..a74be609 100644 --- a/app/jobs/data_migrations/migrate_points_latlon_job.rb +++ b/app/jobs/data_migrations/migrate_points_latlon_job.rb @@ -7,7 +7,7 @@ class DataMigrations::MigratePointsLatlonJob < ApplicationJob user = User.find(user_id) # rubocop:disable Rails/SkipsModelValidations - user.tracked_points.update_all('lonlat = ST_SetSRID(ST_MakePoint(longitude, latitude), 4326)') + user.points.update_all('lonlat = ST_SetSRID(ST_MakePoint(longitude, latitude), 4326)') # rubocop:enable Rails/SkipsModelValidations end end diff --git a/app/jobs/data_migrations/prefill_points_counter_cache_job.rb b/app/jobs/data_migrations/prefill_points_counter_cache_job.rb new file mode 100644 index 00000000..2a860c58 --- /dev/null +++ b/app/jobs/data_migrations/prefill_points_counter_cache_job.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class DataMigrations::PrefillPointsCounterCacheJob < ApplicationJob + queue_as :data_migrations + + def perform(user_id = nil) + if user_id + prefill_counter_for_user(user_id) + else + User.find_each(batch_size: 100) do |user| + prefill_counter_for_user(user.id) + end + end + end + + private + + def prefill_counter_for_user(user_id) + user = User.find(user_id) + points_count = user.points.count + + User.where(id: user_id).update_all(points_count: points_count) + + Rails.logger.info "Updated points_count for user #{user_id}: #{points_count}" + rescue ActiveRecord::RecordNotFound + Rails.logger.warn "User #{user_id} not found, skipping counter cache update" + end +end diff --git a/app/jobs/tracks/cleanup_job.rb b/app/jobs/tracks/cleanup_job.rb index 82eae62d..ad1a8e29 100644 --- a/app/jobs/tracks/cleanup_job.rb +++ b/app/jobs/tracks/cleanup_job.rb @@ -23,9 +23,9 @@ class Tracks::CleanupJob < ApplicationJob 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 + User.active.joins(:points) + .where(points: { track_id: nil, timestamp: ..older_than.to_i }) + .having('COUNT(points.id) >= 2') # Only users with enough points for tracks .group(:id) end end diff --git a/app/models/point.rb b/app/models/point.rb index ef00e99b..69e87681 100644 --- a/app/models/point.rb +++ b/app/models/point.rb @@ -6,7 +6,7 @@ class Point < ApplicationRecord belongs_to :import, optional: true, counter_cache: true belongs_to :visit, optional: true - belongs_to :user + belongs_to :user, counter_cache: true belongs_to :country, optional: true belongs_to :track, optional: true diff --git a/app/models/stat.rb b/app/models/stat.rb index 2cf26d04..c69be6d0 100644 --- a/app/models/stat.rb +++ b/app/models/stat.rb @@ -24,7 +24,7 @@ class Stat < ApplicationRecord end def points - user.tracked_points + user.points .without_raw_data .where(timestamp: timespan) .order(timestamp: :asc) diff --git a/app/models/trip.rb b/app/models/trip.rb index e409a47b..fca5e1e2 100644 --- a/app/models/trip.rb +++ b/app/models/trip.rb @@ -18,7 +18,7 @@ class Trip < ApplicationRecord end def points - user.tracked_points.where(timestamp: started_at.to_i..ended_at.to_i).order(:timestamp) + user.points.where(timestamp: started_at.to_i..ended_at.to_i).order(:timestamp) end def photo_previews diff --git a/app/models/user.rb b/app/models/user.rb index 3f4046a0..a6cdc146 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,14 +4,13 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength devise :database_authenticatable, :registerable, :recoverable, :rememberable, :validatable, :trackable - has_many :tracked_points, class_name: 'Point', dependent: :destroy + has_many :points, dependent: :destroy, counter_cache: true has_many :imports, dependent: :destroy has_many :stats, dependent: :destroy has_many :exports, dependent: :destroy has_many :notifications, dependent: :destroy has_many :areas, dependent: :destroy has_many :visits, dependent: :destroy - has_many :points, through: :imports has_many :places, through: :visits has_many :trips, dependent: :destroy has_many :tracks, dependent: :destroy @@ -35,7 +34,7 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength end def countries_visited - tracked_points + points .where.not(country_name: [nil, '']) .distinct .pluck(:country_name) @@ -43,7 +42,7 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength end def cities_visited - tracked_points.where.not(city: [nil, '']).distinct.pluck(:city).compact + points.where.not(city: [nil, '']).distinct.pluck(:city).compact end def total_distance @@ -60,11 +59,11 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength end def total_reverse_geocoded_points - tracked_points.where.not(reverse_geocoded_at: nil).count + points.where.not(reverse_geocoded_at: nil).count end def total_reverse_geocoded_points_without_data - tracked_points.where(geodata: {}).count + points.where(geodata: {}).count end def immich_integration_configured? @@ -118,7 +117,7 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength end def trial_state? - tracked_points.none? && trial? + points_count.zero? && trial? end private diff --git a/app/serializers/stats_serializer.rb b/app/serializers/stats_serializer.rb index 3a35f157..ae66afbf 100644 --- a/app/serializers/stats_serializer.rb +++ b/app/serializers/stats_serializer.rb @@ -10,7 +10,7 @@ class StatsSerializer def call { totalDistanceKm: total_distance_km, - totalPointsTracked: user.tracked_points.count, + totalPointsTracked: user.points_count, totalReverseGeocodedPoints: reverse_geocoded_points, totalCountriesVisited: user.countries_visited.count, totalCitiesVisited: user.cities_visited.count, @@ -27,7 +27,7 @@ class StatsSerializer end def reverse_geocoded_points - user.tracked_points.reverse_geocoded.count + user.points.reverse_geocoded.count end def yearly_stats diff --git a/app/services/exports/create.rb b/app/services/exports/create.rb index d885afb8..0c2feb76 100644 --- a/app/services/exports/create.rb +++ b/app/services/exports/create.rb @@ -35,7 +35,7 @@ class Exports::Create def time_framed_points user - .tracked_points + .points .where(timestamp: start_at.to_i..end_at.to_i) .order(timestamp: :asc) end diff --git a/app/services/jobs/create.rb b/app/services/jobs/create.rb index d4176652..114a6cf4 100644 --- a/app/services/jobs/create.rb +++ b/app/services/jobs/create.rb @@ -14,9 +14,9 @@ class Jobs::Create points = case job_name when 'start_reverse_geocoding' - user.tracked_points + user.points when 'continue_reverse_geocoding' - user.tracked_points.not_reverse_geocoded + user.points.not_reverse_geocoded else raise InvalidJobName, 'Invalid job name' end diff --git a/app/services/points_limit_exceeded.rb b/app/services/points_limit_exceeded.rb index 2bf8de8a..21cb802a 100644 --- a/app/services/points_limit_exceeded.rb +++ b/app/services/points_limit_exceeded.rb @@ -9,7 +9,7 @@ class PointsLimitExceeded return false if DawarichSettings.self_hosted? Rails.cache.fetch(cache_key, expires_in: 1.day) do - @user.tracked_points.count >= points_limit + @user.points_count >= points_limit end end diff --git a/app/services/stats/calculate_month.rb b/app/services/stats/calculate_month.rb index 91c2a1d7..d05bafb3 100644 --- a/app/services/stats/calculate_month.rb +++ b/app/services/stats/calculate_month.rb @@ -47,7 +47,7 @@ class Stats::CalculateMonth return @points if defined?(@points) @points = user - .tracked_points + .points .without_raw_data .where(timestamp: start_timestamp..end_timestamp) .select(:lonlat, :timestamp) @@ -60,7 +60,7 @@ class Stats::CalculateMonth def toponyms toponym_points = user - .tracked_points + .points .without_raw_data .where(timestamp: start_timestamp..end_timestamp) .select(:city, :country_name) diff --git a/app/services/tracks/generator.rb b/app/services/tracks/generator.rb index be22d021..1993477f 100644 --- a/app/services/tracks/generator.rb +++ b/app/services/tracks/generator.rb @@ -86,7 +86,7 @@ class Tracks::Generator end def load_bulk_points - scope = user.tracked_points.order(:timestamp) + scope = user.points.order(:timestamp) scope = scope.where(timestamp: timestamp_range) if time_range_defined? scope @@ -95,7 +95,7 @@ class Tracks::Generator 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 = user.points.where(track_id: nil).order(:timestamp) scope = scope.where(timestamp: ..end_at.to_i) if end_at.present? scope @@ -104,7 +104,7 @@ class Tracks::Generator def load_daily_points day_range = daily_time_range - user.tracked_points.where(timestamp: day_range).order(:timestamp) + user.points.where(timestamp: day_range).order(:timestamp) end def create_track_from_segment(segment_data) @@ -195,8 +195,8 @@ class Tracks::Generator 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 = user.points.order(:timestamp).first + last_point = user.points.order(:timestamp).last [first_point&.timestamp || 0, last_point&.timestamp || Time.current.to_i] end @@ -207,7 +207,7 @@ class Tracks::Generator end def incremental_timestamp_range - first_point = user.tracked_points.where(track_id: nil).order(:timestamp).first + first_point = user.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] diff --git a/app/services/tracks/incremental_processor.rb b/app/services/tracks/incremental_processor.rb index 62c1faed..f02305a8 100644 --- a/app/services/tracks/incremental_processor.rb +++ b/app/services/tracks/incremental_processor.rb @@ -50,7 +50,7 @@ class Tracks::IncrementalProcessor def find_previous_point @previous_point ||= - user.tracked_points + user.points .where('timestamp < ?', new_point.timestamp) .order(:timestamp) .last diff --git a/app/services/users/export_data.rb b/app/services/users/export_data.rb index dbe4f33b..7ebbf0a1 100644 --- a/app/services/users/export_data.rb +++ b/app/services/users/export_data.rb @@ -331,7 +331,7 @@ class Users::ExportData trips: user.trips.count, stats: user.stats.count, notifications: user.notifications.count, - points: user.tracked_points.count, + points: user.points_count, visits: user.visits.count, places: user.places.count } diff --git a/app/services/visits/smart_detect.rb b/app/services/visits/smart_detect.rb index 64d66440..828c364d 100644 --- a/app/services/visits/smart_detect.rb +++ b/app/services/visits/smart_detect.rb @@ -13,7 +13,7 @@ module Visits @user = user @start_at = start_at.to_i @end_at = end_at.to_i - @points = user.tracked_points.not_visited + @points = user.points.not_visited .order(timestamp: :asc) .where(timestamp: start_at..end_at) end diff --git a/app/services/visits/suggest.rb b/app/services/visits/suggest.rb index 7aab6b93..b40853fb 100644 --- a/app/services/visits/suggest.rb +++ b/app/services/visits/suggest.rb @@ -6,7 +6,7 @@ class Visits::Suggest def initialize(user, start_at:, end_at:) @start_at = start_at.to_i @end_at = end_at.to_i - @points = user.tracked_points.not_visited.order(timestamp: :asc).where(timestamp: start_at..end_at) + @points = user.points.not_visited.order(timestamp: :asc).where(timestamp: start_at..end_at) @user = user end diff --git a/app/views/devise/registrations/_points_usage.html.erb b/app/views/devise/registrations/_points_usage.html.erb index c079b93a..68880d0d 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.tracked_points.count) %> points of <%= number_with_delimiter(DawarichSettings::BASIC_PAID_PLAN_LIMIT) %> available. + You have used <%= number_with_delimiter(current_user.points_count) %> points of <%= number_with_delimiter(DawarichSettings::BASIC_PAID_PLAN_LIMIT) %> available.

- +

diff --git a/app/views/settings/users/index.html.erb b/app/views/settings/users/index.html.erb index 90cb21e8..c4c6aea0 100644 --- a/app/views/settings/users/index.html.erb +++ b/app/views/settings/users/index.html.erb @@ -24,7 +24,7 @@ - <%= number_with_delimiter user.tracked_points.count %> + <%= number_with_delimiter user.points_count %> <%= human_datetime(user.created_at) %> diff --git a/db/data/20250704185707_create_tracks_from_points.rb b/db/data/20250704185707_create_tracks_from_points.rb index 7d5cffb5..2972eac4 100644 --- a/db/data/20250704185707_create_tracks_from_points.rb +++ b/db/data/20250704185707_create_tracks_from_points.rb @@ -8,7 +8,7 @@ class CreateTracksFromPoints < ActiveRecord::Migration[8.0] processed_users = 0 User.find_each do |user| - points_count = user.tracked_points.count + points_count = user.points.count if points_count > 0 puts "Enqueuing track creation for user #{user.id} (#{points_count} points)" diff --git a/db/migrate/20250821192219_add_points_count_to_users.rb b/db/migrate/20250821192219_add_points_count_to_users.rb new file mode 100644 index 00000000..b7a9aaa7 --- /dev/null +++ b/db/migrate/20250821192219_add_points_count_to_users.rb @@ -0,0 +1,16 @@ +class AddPointsCountToUsers < ActiveRecord::Migration[8.0] + def change + add_column :users, :points_count, :integer, default: 0, null: false + + # Initialize counter cache for existing users using background job + reversible do |dir| + dir.up do + # Enqueue job to prefill counter cache in background + # This prevents the migration from blocking on large datasets + say_with_time "Enqueueing job to prefill points counter cache" do + DataMigrations::PrefillPointsCounterCacheJob.perform_later + end + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index feac06e4..836137d2 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_28_191359) do +ActiveRecord::Schema[8.0].define(version: 2025_08_21_192219) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "postgis" @@ -230,7 +230,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_07_28_191359) do t.datetime "end_at", null: false t.bigint "user_id", null: false t.geometry "original_path", limit: {srid: 0, type: "line_string"}, null: false - t.integer "distance" + t.decimal "distance", precision: 8, scale: 2 t.float "avg_speed" t.integer "duration" t.integer "elevation_gain" @@ -274,6 +274,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_07_28_191359) do t.string "last_sign_in_ip" t.integer "status", default: 0 t.datetime "active_until" + t.integer "points_count", default: 0, null: false t.index ["email"], name: "index_users_on_email", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2c07580a..852d6141 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5,9 +5,8 @@ require 'rails_helper' RSpec.describe User, type: :model do describe 'associations' do it { is_expected.to have_many(:imports).dependent(:destroy) } - it { is_expected.to have_many(:points).through(:imports) } it { is_expected.to have_many(:stats) } - it { is_expected.to have_many(:tracked_points).class_name('Point').dependent(:destroy) } + it { is_expected.to have_many(:points).class_name('Point').dependent(:destroy) } it { is_expected.to have_many(:exports).dependent(:destroy) } it { is_expected.to have_many(:notifications).dependent(:destroy) } it { is_expected.to have_many(:areas).dependent(:destroy) } @@ -124,7 +123,7 @@ RSpec.describe User, type: :model do end it 'returns true' do - user.tracked_points.destroy_all + user.points.destroy_all expect(user.trial_state?).to be_truthy end diff --git a/spec/services/points_limit_exceeded_spec.rb b/spec/services/points_limit_exceeded_spec.rb index fed8a880..534192ee 100644 --- a/spec/services/points_limit_exceeded_spec.rb +++ b/spec/services/points_limit_exceeded_spec.rb @@ -24,20 +24,20 @@ RSpec.describe PointsLimitExceeded do context 'when user points count is equal to the limit' do before do - allow(user.tracked_points).to receive(:count).and_return(10) + allow(user.points).to receive(:count).and_return(10) end it { is_expected.to be true } it 'caches the result' do - expect(user.tracked_points).to receive(:count).once + expect(user.points).to receive(:count).once 2.times { described_class.new(user).call } end end context 'when user points count exceeds the limit' do before do - allow(user.tracked_points).to receive(:count).and_return(11) + allow(user.points).to receive(:count).and_return(11) end it { is_expected.to be true } @@ -45,7 +45,7 @@ RSpec.describe PointsLimitExceeded do context 'when user points count is below the limit' do before do - allow(user.tracked_points).to receive(:count).and_return(9) + allow(user.points).to receive(:count).and_return(9) end it { is_expected.to be false } diff --git a/spec/services/users/export_data_spec.rb b/spec/services/users/export_data_spec.rb index cc603d75..4db0acd0 100644 --- a/spec/services/users/export_data_spec.rb +++ b/spec/services/users/export_data_spec.rb @@ -45,7 +45,7 @@ RSpec.describe Users::ExportData, type: :service do allow(user).to receive(:trips).and_return(double(count: 8)) allow(user).to receive(:stats).and_return(double(count: 24)) allow(user).to receive(:notifications).and_return(double(count: 10)) - allow(user).to receive(:tracked_points).and_return(double(count: 15000)) + allow(user).to receive(:points).and_return(double(count: 15000)) allow(user).to receive(:visits).and_return(double(count: 45)) allow(user).to receive(:places).and_return(double(count: 20)) @@ -187,7 +187,7 @@ RSpec.describe Users::ExportData, type: :service do allow(user).to receive(:trips).and_return(double(count: 8)) allow(user).to receive(:stats).and_return(double(count: 24)) allow(user).to receive(:notifications).and_return(double(count: 10)) - allow(user).to receive(:tracked_points).and_return(double(count: 15000)) + allow(user).to receive(:points).and_return(double(count: 15000)) allow(user).to receive(:visits).and_return(double(count: 45)) allow(user).to receive(:places).and_return(double(count: 20)) @@ -267,7 +267,7 @@ RSpec.describe Users::ExportData, type: :service do allow(user).to receive(:trips).and_return(double(count: 8)) allow(user).to receive(:stats).and_return(double(count: 24)) allow(user).to receive(:notifications).and_return(double(count: 10)) - allow(user).to receive(:tracked_points).and_return(double(count: 15000)) + allow(user).to receive(:points).and_return(double(count: 15000)) allow(user).to receive(:visits).and_return(double(count: 45)) allow(user).to receive(:places).and_return(double(count: 20)) @@ -374,7 +374,7 @@ RSpec.describe Users::ExportData, type: :service do allow(user).to receive(:trips).and_return(double(count: 8)) allow(user).to receive(:stats).and_return(double(count: 24)) allow(user).to receive(:notifications).and_return(double(count: 10)) - allow(user).to receive(:tracked_points).and_return(double(count: 15000)) + allow(user).to receive(:points).and_return(double(count: 15000)) allow(user).to receive(:visits).and_return(double(count: 45)) allow(user).to receive(:places).and_return(double(count: 20)) allow(Rails.logger).to receive(:info) diff --git a/spec/services/users/export_import_integration_spec.rb b/spec/services/users/export_import_integration_spec.rb index ed959048..66e3c6a0 100644 --- a/spec/services/users/export_import_integration_spec.rb +++ b/spec/services/users/export_import_integration_spec.rb @@ -312,33 +312,33 @@ RSpec.describe 'Users Export-Import Integration', type: :service do trips: user.trips.count, stats: user.stats.count, notifications: user.notifications.count, - points: user.tracked_points.count, + points: user.points.count, visits: user.visits.count, places: user.places.count } end def verify_relationships_preserved(original_user, target_user) - original_points_with_imports = original_user.tracked_points.where.not(import_id: nil).count - target_points_with_imports = target_user.tracked_points.where.not(import_id: nil).count + original_points_with_imports = original_user.points.where.not(import_id: nil).count + target_points_with_imports = target_user.points.where.not(import_id: nil).count expect(target_points_with_imports).to eq(original_points_with_imports) - original_points_with_countries = original_user.tracked_points.where.not(country_id: nil).count - target_points_with_countries = target_user.tracked_points.where.not(country_id: nil).count + original_points_with_countries = original_user.points.where.not(country_id: nil).count + target_points_with_countries = target_user.points.where.not(country_id: nil).count expect(target_points_with_countries).to eq(original_points_with_countries) - original_points_with_visits = original_user.tracked_points.where.not(visit_id: nil).count - target_points_with_visits = target_user.tracked_points.where.not(visit_id: nil).count + original_points_with_visits = original_user.points.where.not(visit_id: nil).count + target_points_with_visits = target_user.points.where.not(visit_id: nil).count expect(target_points_with_visits).to eq(original_points_with_visits) original_visits_with_places = original_user.visits.where.not(place_id: nil).count target_visits_with_places = target_user.visits.where.not(place_id: nil).count expect(target_visits_with_places).to eq(original_visits_with_places) - original_office_points = original_user.tracked_points.where( + original_office_points = original_user.points.where( latitude: 40.7589, longitude: -73.9851 ).first - target_office_points = target_user.tracked_points.where( + target_office_points = target_user.points.where( latitude: 40.7589, longitude: -73.9851 ).first diff --git a/spec/services/users/import_data/points_spec.rb b/spec/services/users/import_data/points_spec.rb index cfb81c28..aa23e316 100644 --- a/spec/services/users/import_data/points_spec.rb +++ b/spec/services/users/import_data/points_spec.rb @@ -35,13 +35,13 @@ RSpec.describe Users::ImportData::Points, type: :service do it 'assigns the correct country association' do service.call - point = user.tracked_points.last + point = user.points.last expect(point.country).to eq(country) end it 'excludes the string country field from attributes' do service.call - point = user.tracked_points.last + point = user.points.last # The country association should be set, not the string attribute expect(point.read_attribute(:country)).to be_nil expect(point.country).to eq(country) @@ -68,7 +68,7 @@ RSpec.describe Users::ImportData::Points, type: :service do it 'does not create country and leaves country_id nil' do expect { service.call }.not_to change(Country, :count) - point = user.tracked_points.last + point = user.points.last expect(point.country_id).to be_nil expect(point.city).to eq('Berlin') end @@ -126,10 +126,10 @@ RSpec.describe Users::ImportData::Points, type: :service do it 'imports valid points and reconstructs lonlat when needed' do expect(service.call).to eq(2) # Two valid points (original + reconstructed) - expect(user.tracked_points.count).to eq(2) + expect(user.points.count).to eq(2) # Check that lonlat was reconstructed properly - munich_point = user.tracked_points.find_by(city: 'Munich') + munich_point = user.points.find_by(city: 'Munich') expect(munich_point).to be_present expect(munich_point.lonlat.to_s).to match(/POINT\s*\(11\.582\s+48\.1351\)/) end diff --git a/spec/services/visits/smart_detect_spec.rb b/spec/services/visits/smart_detect_spec.rb index 37ea7638..006770ae 100644 --- a/spec/services/visits/smart_detect_spec.rb +++ b/spec/services/visits/smart_detect_spec.rb @@ -27,7 +27,7 @@ RSpec.describe Visits::SmartDetect do let(:created_visits) { [instance_double(Visit)] } before do - allow(user).to receive_message_chain(:tracked_points, :not_visited, :order, :where).and_return(points) + allow(user).to receive_message_chain(:points, :not_visited, :order, :where).and_return(points) allow(Visits::Detector).to receive(:new).with(points).and_return(visit_detector) allow(Visits::Merger).to receive(:new).with(points).and_return(visit_merger) allow(Visits::Creator).to receive(:new).with(user).and_return(visit_creator) From 76a7c12133b30b5250bd02335bf5da79e2de7162 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Fri, 22 Aug 2025 21:27:50 +0200 Subject: [PATCH 2/4] Fix range issue --- app/services/tracks/track_builder.rb | 7 +++++-- spec/serializers/track_serializer_spec.rb | 4 ++-- spec/services/points_limit_exceeded_spec.rb | 8 ++++---- spec/services/users/export_data_spec.rb | 8 ++++---- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/app/services/tracks/track_builder.rb b/app/services/tracks/track_builder.rb index 0ccd82b0..a988f3bf 100644 --- a/app/services/tracks/track_builder.rb +++ b/app/services/tracks/track_builder.rb @@ -59,7 +59,8 @@ module Tracks::TrackBuilder original_path: build_path(points) ) - track.distance = pre_calculated_distance.round + # TODO: Move trips attrs to columns with more precision and range + track.distance = [[pre_calculated_distance.round, 999999.99].min, 0].max track.duration = calculate_duration(points) track.avg_speed = calculate_average_speed(track.distance, track.duration) @@ -99,8 +100,10 @@ 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_kmh = (speed_mps * 3.6).round(2) # m/s to km/h - (speed_mps * 3.6).round(2) # m/s to km/h + # Cap the speed to prevent database precision overflow (max 999999.99) + [speed_kmh, 999999.99].min end def calculate_elevation_stats(points) diff --git a/spec/serializers/track_serializer_spec.rb b/spec/serializers/track_serializer_spec.rb index 6622b23d..cfda4e5c 100644 --- a/spec/serializers/track_serializer_spec.rb +++ b/spec/serializers/track_serializer_spec.rb @@ -123,7 +123,7 @@ RSpec.describe TrackSerializer do context 'with very large values' do let(:track) do create(:track, user: user, - distance: 1_000_000.0, + distance: 999_999.99, avg_speed: 999.99, duration: 86_400, # 24 hours in seconds elevation_gain: 10_000, @@ -133,7 +133,7 @@ RSpec.describe TrackSerializer do end it 'handles large values correctly' do - expect(serialized_track[:distance]).to eq(1_000_000) + expect(serialized_track[:distance]).to eq(999_999) 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) diff --git a/spec/services/points_limit_exceeded_spec.rb b/spec/services/points_limit_exceeded_spec.rb index 534192ee..293c7a27 100644 --- a/spec/services/points_limit_exceeded_spec.rb +++ b/spec/services/points_limit_exceeded_spec.rb @@ -24,20 +24,20 @@ 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).to receive(:points_count).and_return(10) end it { is_expected.to be true } it 'caches the result' do - expect(user.points).to receive(:count).once + expect(user).to receive(:points_count).once 2.times { described_class.new(user).call } end end context 'when user points count exceeds the limit' do before do - allow(user.points).to receive(:count).and_return(11) + allow(user).to receive(:points_count).and_return(11) end it { is_expected.to be true } @@ -45,7 +45,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).to receive(:points_count).and_return(9) end it { is_expected.to be false } diff --git a/spec/services/users/export_data_spec.rb b/spec/services/users/export_data_spec.rb index 4db0acd0..b2c600d2 100644 --- a/spec/services/users/export_data_spec.rb +++ b/spec/services/users/export_data_spec.rb @@ -45,7 +45,7 @@ RSpec.describe Users::ExportData, type: :service do allow(user).to receive(:trips).and_return(double(count: 8)) allow(user).to receive(:stats).and_return(double(count: 24)) allow(user).to receive(:notifications).and_return(double(count: 10)) - allow(user).to receive(:points).and_return(double(count: 15000)) + allow(user).to receive(:points_count).and_return(15000) allow(user).to receive(:visits).and_return(double(count: 45)) allow(user).to receive(:places).and_return(double(count: 20)) @@ -187,7 +187,7 @@ RSpec.describe Users::ExportData, type: :service do allow(user).to receive(:trips).and_return(double(count: 8)) allow(user).to receive(:stats).and_return(double(count: 24)) allow(user).to receive(:notifications).and_return(double(count: 10)) - allow(user).to receive(:points).and_return(double(count: 15000)) + allow(user).to receive(:points_count).and_return(15000) allow(user).to receive(:visits).and_return(double(count: 45)) allow(user).to receive(:places).and_return(double(count: 20)) @@ -267,7 +267,7 @@ RSpec.describe Users::ExportData, type: :service do allow(user).to receive(:trips).and_return(double(count: 8)) allow(user).to receive(:stats).and_return(double(count: 24)) allow(user).to receive(:notifications).and_return(double(count: 10)) - allow(user).to receive(:points).and_return(double(count: 15000)) + allow(user).to receive(:points_count).and_return(15000) allow(user).to receive(:visits).and_return(double(count: 45)) allow(user).to receive(:places).and_return(double(count: 20)) @@ -374,7 +374,7 @@ RSpec.describe Users::ExportData, type: :service do allow(user).to receive(:trips).and_return(double(count: 8)) allow(user).to receive(:stats).and_return(double(count: 24)) allow(user).to receive(:notifications).and_return(double(count: 10)) - allow(user).to receive(:points).and_return(double(count: 15000)) + allow(user).to receive(:points_count).and_return(15000) allow(user).to receive(:visits).and_return(double(count: 45)) allow(user).to receive(:places).and_return(double(count: 20)) allow(Rails.logger).to receive(:info) From b8d69a67972d8003c93be1495b3b8b0404e8e1e6 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 26 Aug 2025 15:26:05 +0200 Subject: [PATCH 3/4] Reset counters for points counter cache --- .../data_migrations/prefill_points_counter_cache_job.rb | 7 +------ app/services/imports/create.rb | 1 + db/migrate/20250821192219_add_points_count_to_users.rb | 8 ++------ db/schema.rb | 1 - spec/services/imports/create_spec.rb | 9 +++++++++ 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/jobs/data_migrations/prefill_points_counter_cache_job.rb b/app/jobs/data_migrations/prefill_points_counter_cache_job.rb index 2a860c58..fe3715ab 100644 --- a/app/jobs/data_migrations/prefill_points_counter_cache_job.rb +++ b/app/jobs/data_migrations/prefill_points_counter_cache_job.rb @@ -16,12 +16,7 @@ class DataMigrations::PrefillPointsCounterCacheJob < ApplicationJob private def prefill_counter_for_user(user_id) - user = User.find(user_id) - points_count = user.points.count - - User.where(id: user_id).update_all(points_count: points_count) - - Rails.logger.info "Updated points_count for user #{user_id}: #{points_count}" + User.reset_counters(user_id, :points) rescue ActiveRecord::RecordNotFound Rails.logger.warn "User #{user_id} not found, skipping counter cache update" end diff --git a/app/services/imports/create.rb b/app/services/imports/create.rb index 67d05abb..58079188 100644 --- a/app/services/imports/create.rb +++ b/app/services/imports/create.rb @@ -28,6 +28,7 @@ class Imports::Create schedule_stats_creating(user.id) schedule_visit_suggesting(user.id, import) update_import_points_count(import) + User.reset_counters(user.id, :points) rescue StandardError => e import.update!(status: :failed) broadcast_status_update diff --git a/db/migrate/20250821192219_add_points_count_to_users.rb b/db/migrate/20250821192219_add_points_count_to_users.rb index b7a9aaa7..86f056a6 100644 --- a/db/migrate/20250821192219_add_points_count_to_users.rb +++ b/db/migrate/20250821192219_add_points_count_to_users.rb @@ -1,15 +1,11 @@ class AddPointsCountToUsers < ActiveRecord::Migration[8.0] def change add_column :users, :points_count, :integer, default: 0, null: false - + # Initialize counter cache for existing users using background job reversible do |dir| dir.up do - # Enqueue job to prefill counter cache in background - # This prevents the migration from blocking on large datasets - say_with_time "Enqueueing job to prefill points counter cache" do - DataMigrations::PrefillPointsCounterCacheJob.perform_later - end + DataMigrations::PrefillPointsCounterCacheJob.perform_later end end end diff --git a/db/schema.rb b/db/schema.rb index 3283db0a..6cb87072 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,6 @@ # # It's strongly recommended that you check this file into your version control system. - ActiveRecord::Schema[8.0].define(version: 2025_08_23_125940) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" diff --git a/spec/services/imports/create_spec.rb b/spec/services/imports/create_spec.rb index 756268f9..1410b2ee 100644 --- a/spec/services/imports/create_spec.rb +++ b/spec/services/imports/create_spec.rb @@ -27,6 +27,15 @@ RSpec.describe Imports::Create do expect(import.reload.source).to eq('owntracks') end + it 'resets points counter cache' do + allow(User).to receive(:reset_counters) + + service.call + + expect(User).to have_received(:reset_counters).with(user.id, :points) + end + + context 'when import succeeds' do it 'sets status to completed' do service.call From e9b8d9a673a41b457b6a2ba27dfbd9ecef044829 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 26 Aug 2025 15:28:10 +0200 Subject: [PATCH 4/4] Update changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d7cca45..03744e1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ 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/). +# [UNRELEASED] - 2025-08-26 + +## Fixed + +- Number of user points is not being cached resulting in performance boost on certain pages and operations. + + # [0.30.11] - 2025-08-23 ## Changed