From 2aaad3cf30409da2d87cfc61d040feea12fcfa69 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Mon, 5 Jan 2026 21:13:57 +0100 Subject: [PATCH 1/2] Implement some performance improvements and caching for various features. --- CHANGELOG.md | 2 + app/controllers/api/v1/places_controller.rb | 22 +++- app/controllers/api/v1/visits_controller.rb | 11 ++ app/controllers/map/leaflet_controller.rb | 32 ++++-- app/controllers/stats_controller.rb | 12 +- .../maps_maplibre/services/api_client.js | 106 ++++++++++++++++-- app/jobs/cache/preheating_job.rb | 8 ++ app/models/user.rb | 6 +- app/services/cache/clean.rb | 7 ++ app/services/cache/invalidate_user_caches.rb | 5 + app/services/stats/calculate_month.rb | 14 +-- app/services/visits/find_in_time.rb | 2 +- .../visits/find_within_bounding_box.rb | 2 +- 13 files changed, 188 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45b4e351..21145b46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Time spent in a country and city is now calculated correctly for the year-end digest email. #2104 - Updated Trix to fix a XSS vulnerability. #2102 - Map v2 UI no longer blocks when Immich/Photoprism integration has a bad URL or is unreachable. Added 10-second timeout to photo API requests and improved error handling to prevent UI freezing during initial load. #2085 + +## Added - In Map v2 settings, you can now enable map to be rendered as a globe. # [0.37.1] - 2025-12-30 diff --git a/app/controllers/api/v1/places_controller.rb b/app/controllers/api/v1/places_controller.rb index 97035526..41d4dac8 100644 --- a/app/controllers/api/v1/places_controller.rb +++ b/app/controllers/api/v1/places_controller.rb @@ -16,11 +16,11 @@ module Api include_untagged = tag_ids.include?('untagged') if numeric_tag_ids.any? && include_untagged - # Both tagged and untagged: return union (OR logic) - tagged = current_api_user.places.includes(:tags, :visits).with_tags(numeric_tag_ids) - untagged = current_api_user.places.includes(:tags, :visits).without_tags - @places = Place.from("(#{tagged.to_sql} UNION #{untagged.to_sql}) AS places") - .includes(:tags, :visits) + # Both tagged and untagged: use OR logic to preserve eager loading + tagged_ids = current_api_user.places.with_tags(numeric_tag_ids).pluck(:id) + untagged_ids = current_api_user.places.without_tags.pluck(:id) + combined_ids = (tagged_ids + untagged_ids).uniq + @places = current_api_user.places.includes(:tags, :visits).where(id: combined_ids) elsif numeric_tag_ids.any? # Only tagged places with ANY of the selected tags (OR logic) @places = @places.with_tags(numeric_tag_ids) @@ -30,6 +30,16 @@ module Api end end + # Support optional pagination (backward compatible - returns all if no page param) + if params[:page].present? + per_page = [params[:per_page]&.to_i || 100, 500].min + @places = @places.page(params[:page]).per(per_page) + + response.set_header('X-Current-Page', @places.current_page.to_s) + response.set_header('X-Total-Pages', @places.total_pages.to_s) + response.set_header('X-Total-Count', @places.total_count.to_s) + end + render json: @places.map { |place| serialize_place(place) } end @@ -120,7 +130,7 @@ module Api note: place.note, icon: place.tags.first&.icon, color: place.tags.first&.color, - visits_count: place.visits.count, + visits_count: place.visits.size, created_at: place.created_at, tags: place.tags.map do |tag| { diff --git a/app/controllers/api/v1/visits_controller.rb b/app/controllers/api/v1/visits_controller.rb index 1002536d..0110a97f 100644 --- a/app/controllers/api/v1/visits_controller.rb +++ b/app/controllers/api/v1/visits_controller.rb @@ -3,6 +3,17 @@ class Api::V1::VisitsController < ApiController def index visits = Visits::Finder.new(current_api_user, params).call + + # Support optional pagination (backward compatible - returns all if no page param) + if params[:page].present? + per_page = [params[:per_page]&.to_i || 100, 500].min + visits = visits.page(params[:page]).per(per_page) + + response.set_header('X-Current-Page', visits.current_page.to_s) + response.set_header('X-Total-Pages', visits.total_pages.to_s) + response.set_header('X-Total-Count', visits.total_count.to_s) + end + serialized_visits = visits.map do |visit| Api::VisitSerializer.new(visit).call end diff --git a/app/controllers/map/leaflet_controller.rb b/app/controllers/map/leaflet_controller.rb index 660b9615..07431bfb 100644 --- a/app/controllers/map/leaflet_controller.rb +++ b/app/controllers/map/leaflet_controller.rb @@ -41,19 +41,31 @@ class Map::LeafletController < ApplicationController end def calculate_distance - return 0 if @coordinates.size < 2 + return 0 if @points.count < 2 - total_distance = 0 + # Use PostGIS window function for efficient distance calculation + # This is O(1) database operation vs O(n) Ruby iteration + sql = <<~SQL.squish + SELECT COALESCE(SUM(distance_m) / 1000.0, 0) as total_km FROM ( + SELECT ST_Distance( + lonlat::geography, + LAG(lonlat::geography) OVER (ORDER BY timestamp) + ) as distance_m + FROM points + WHERE user_id = :user_id + AND timestamp >= :start_at + AND timestamp <= :end_at + ) distances + SQL - @coordinates.each_cons(2) do - distance_km = Geocoder::Calculations.distance_between( - [_1[0], _1[1]], [_2[0], _2[1]], units: :km - ) + result = Point.connection.select_value( + ActiveRecord::Base.sanitize_sql_array([ + sql, + { user_id: current_user.id, start_at: start_at, end_at: end_at } + ]) + ) - total_distance += distance_km - end - - total_distance.round + result&.to_f&.round || 0 end def parsed_start_at diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index 8d735acf..e12c7e9d 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -80,8 +80,14 @@ class StatsController < ApplicationController end def build_stats - current_user.stats.group_by(&:year).transform_values do |stats| - stats.sort_by(&:updated_at).reverse - end.sort.reverse + # Select only needed columns - avoid loading large JSONB fields + # daily_distance and h3_hex_ids are never needed on index page + columns = [:id, :year, :month, :distance, :updated_at, :user_id] + columns << :toponyms if DawarichSettings.reverse_geocoding_enabled? + + current_user.stats + .select(columns) + .order(year: :desc, updated_at: :desc) + .group_by(&:year) end end diff --git a/app/javascript/maps_maplibre/services/api_client.js b/app/javascript/maps_maplibre/services/api_client.js index 1ef9e871..f69563cb 100644 --- a/app/javascript/maps_maplibre/services/api_client.js +++ b/app/javascript/maps_maplibre/services/api_client.js @@ -73,10 +73,17 @@ export class ApiClient { } /** - * Fetch visits for date range + * Fetch visits for date range (paginated) + * @param {Object} options - { start_at, end_at, page, per_page } + * @returns {Promise} { visits, currentPage, totalPages } */ - async fetchVisits({ start_at, end_at }) { - const params = new URLSearchParams({ start_at, end_at }) + async fetchVisitsPage({ start_at, end_at, page = 1, per_page = 500 }) { + const params = new URLSearchParams({ + start_at, + end_at, + page: page.toString(), + per_page: per_page.toString() + }) const response = await fetch(`${this.baseURL}/visits?${params}`, { headers: this.getHeaders() @@ -86,20 +93,63 @@ export class ApiClient { throw new Error(`Failed to fetch visits: ${response.statusText}`) } - return response.json() + const visits = await response.json() + + return { + visits, + currentPage: parseInt(response.headers.get('X-Current-Page') || '1'), + totalPages: parseInt(response.headers.get('X-Total-Pages') || '1') + } } /** - * Fetch places optionally filtered by tags + * Fetch all visits for date range (handles pagination) + * @param {Object} options - { start_at, end_at, onProgress } + * @returns {Promise} All visits */ - async fetchPlaces({ tag_ids = [] } = {}) { - const params = new URLSearchParams() + async fetchVisits({ start_at, end_at, onProgress = null }) { + const allVisits = [] + let page = 1 + let totalPages = 1 + + do { + const { visits, currentPage, totalPages: total } = + await this.fetchVisitsPage({ start_at, end_at, page, per_page: 500 }) + + allVisits.push(...visits) + totalPages = total + page++ + + if (onProgress) { + const progress = totalPages > 0 ? currentPage / totalPages : 1.0 + onProgress({ + loaded: allVisits.length, + currentPage, + totalPages, + progress + }) + } + } while (page <= totalPages) + + return allVisits + } + + /** + * Fetch places (paginated) + * @param {Object} options - { tag_ids, page, per_page } + * @returns {Promise} { places, currentPage, totalPages } + */ + async fetchPlacesPage({ tag_ids = [], page = 1, per_page = 500 } = {}) { + const params = new URLSearchParams({ + page: page.toString(), + per_page: per_page.toString() + }) if (tag_ids && tag_ids.length > 0) { tag_ids.forEach(id => params.append('tag_ids[]', id)) } - const url = `${this.baseURL}/places${params.toString() ? '?' + params.toString() : ''}` + const url = `${this.baseURL}/places?${params.toString()}` const response = await fetch(url, { headers: this.getHeaders() @@ -109,7 +159,45 @@ export class ApiClient { throw new Error(`Failed to fetch places: ${response.statusText}`) } - return response.json() + const places = await response.json() + + return { + places, + currentPage: parseInt(response.headers.get('X-Current-Page') || '1'), + totalPages: parseInt(response.headers.get('X-Total-Pages') || '1') + } + } + + /** + * Fetch all places optionally filtered by tags (handles pagination) + * @param {Object} options - { tag_ids, onProgress } + * @returns {Promise} All places + */ + async fetchPlaces({ tag_ids = [], onProgress = null } = {}) { + const allPlaces = [] + let page = 1 + let totalPages = 1 + + do { + const { places, currentPage, totalPages: total } = + await this.fetchPlacesPage({ tag_ids, page, per_page: 500 }) + + allPlaces.push(...places) + totalPages = total + page++ + + if (onProgress) { + const progress = totalPages > 0 ? currentPage / totalPages : 1.0 + onProgress({ + loaded: allPlaces.length, + currentPage, + totalPages, + progress + }) + } + } while (page <= totalPages) + + return allPlaces } /** diff --git a/app/jobs/cache/preheating_job.rb b/app/jobs/cache/preheating_job.rb index c8002fdf..90a8013f 100644 --- a/app/jobs/cache/preheating_job.rb +++ b/app/jobs/cache/preheating_job.rb @@ -28,6 +28,14 @@ class Cache::PreheatingJob < ApplicationJob user.cities_visited_uncached, expires_in: 1.day ) + + # Preheat total_distance cache + total_distance_meters = user.stats.sum(:distance) + Rails.cache.write( + "dawarich/user_#{user.id}_total_distance", + Stat.convert_distance(total_distance_meters, user.safe_settings.distance_unit), + expires_in: 1.day + ) end end end diff --git a/app/models/user.rb b/app/models/user.rb index 4b35390e..4279d494 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -56,8 +56,10 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength end def total_distance - total_distance_meters = stats.sum(:distance) - Stat.convert_distance(total_distance_meters, safe_settings.distance_unit) + Rails.cache.fetch("dawarich/user_#{id}_total_distance", expires_in: 1.day) do + total_distance_meters = stats.sum(:distance) + Stat.convert_distance(total_distance_meters, safe_settings.distance_unit) + end end def total_countries diff --git a/app/services/cache/clean.rb b/app/services/cache/clean.rb index e555e6a4..af8354b7 100644 --- a/app/services/cache/clean.rb +++ b/app/services/cache/clean.rb @@ -9,6 +9,7 @@ class Cache::Clean delete_years_tracked_cache delete_points_geocoded_stats_cache delete_countries_cities_cache + delete_total_distance_cache Rails.logger.info('Cache cleaned') end @@ -40,5 +41,11 @@ class Cache::Clean Rails.cache.delete("dawarich/user_#{user.id}_cities_visited") end end + + def delete_total_distance_cache + User.find_each do |user| + Rails.cache.delete("dawarich/user_#{user.id}_total_distance") + end + end end end diff --git a/app/services/cache/invalidate_user_caches.rb b/app/services/cache/invalidate_user_caches.rb index 839efdae..fdcd3642 100644 --- a/app/services/cache/invalidate_user_caches.rb +++ b/app/services/cache/invalidate_user_caches.rb @@ -14,6 +14,7 @@ class Cache::InvalidateUserCaches invalidate_countries_visited invalidate_cities_visited invalidate_points_geocoded_stats + invalidate_total_distance end def invalidate_countries_visited @@ -28,6 +29,10 @@ class Cache::InvalidateUserCaches Rails.cache.delete("dawarich/user_#{user_id}_points_geocoded_stats") end + def invalidate_total_distance + Rails.cache.delete("dawarich/user_#{user_id}_total_distance") + end + private attr_reader :user_id diff --git a/app/services/stats/calculate_month.rb b/app/services/stats/calculate_month.rb index 0a87b803..d5cda5f2 100644 --- a/app/services/stats/calculate_month.rb +++ b/app/services/stats/calculate_month.rb @@ -50,11 +50,13 @@ class Stats::CalculateMonth def points return @points if defined?(@points) + # Select all needed columns to avoid duplicate queries + # Used for both distance calculation and toponyms extraction @points = user .points .without_raw_data .where(timestamp: start_timestamp..end_timestamp) - .select(:lonlat, :timestamp) + .select(:lonlat, :timestamp, :city, :country_name) .order(timestamp: :asc) end @@ -63,14 +65,8 @@ class Stats::CalculateMonth end def toponyms - toponym_points = - user - .points - .without_raw_data - .where(timestamp: start_timestamp..end_timestamp) - .select(:city, :country_name, :timestamp) - - CountriesAndCities.new(toponym_points).call + # Reuse already-loaded points instead of making a duplicate query + CountriesAndCities.new(points).call end def create_stats_update_failed_notification(user, error) diff --git a/app/services/visits/find_in_time.rb b/app/services/visits/find_in_time.rb index e486382a..84e381c3 100644 --- a/app/services/visits/find_in_time.rb +++ b/app/services/visits/find_in_time.rb @@ -10,7 +10,7 @@ module Visits def call Visit - .includes(:place) + .includes(:place, :area) .where(user:) .where('started_at >= ? AND ended_at <= ?', start_at, end_at) .order(started_at: :desc) diff --git a/app/services/visits/find_within_bounding_box.rb b/app/services/visits/find_within_bounding_box.rb index d5bdb74a..b73913df 100644 --- a/app/services/visits/find_within_bounding_box.rb +++ b/app/services/visits/find_within_bounding_box.rb @@ -13,7 +13,7 @@ module Visits def call Visit - .includes(:place) + .includes(:place, :area) .where(user:) .joins(:place) .where( From 1b504ff757cfb3daf2fc5a3a860c5db85e8441b4 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Tue, 6 Jan 2026 00:25:36 +0100 Subject: [PATCH 2/2] Fix failing tests --- app/controllers/map/leaflet_controller.rb | 2 +- spec/requests/api/v1/users_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/map/leaflet_controller.rb b/app/controllers/map/leaflet_controller.rb index 07431bfb..0c425a57 100644 --- a/app/controllers/map/leaflet_controller.rb +++ b/app/controllers/map/leaflet_controller.rb @@ -41,7 +41,7 @@ class Map::LeafletController < ApplicationController end def calculate_distance - return 0 if @points.count < 2 + return 0 if @points.count(:id) < 2 # Use PostGIS window function for efficient distance calculation # This is O(1) database operation vs O(n) Ruby iteration diff --git a/spec/requests/api/v1/users_spec.rb b/spec/requests/api/v1/users_spec.rb index b1669b39..3af357f3 100644 --- a/spec/requests/api/v1/users_spec.rb +++ b/spec/requests/api/v1/users_spec.rb @@ -27,7 +27,7 @@ RSpec.describe 'Api::V1::Users', type: :request do 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 + speed_color_scale fog_of_war_threshold globe_projection ]) end end