From d1d37c83555870b30f109dcb42c98c0b66b3dd9d Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Wed, 1 Oct 2025 22:05:51 +0200 Subject: [PATCH] Revert some changes --- app/services/tracks/boundary_detector.rb | 37 +++++------------------ app/services/users/import_data/imports.rb | 6 ---- spec/factories/points.rb | 36 ++++------------------ 3 files changed, 14 insertions(+), 65 deletions(-) diff --git a/app/services/tracks/boundary_detector.rb b/app/services/tracks/boundary_detector.rb index f39467f4..6f88f4a8 100644 --- a/app/services/tracks/boundary_detector.rb +++ b/app/services/tracks/boundary_detector.rb @@ -92,25 +92,13 @@ class Tracks::BoundaryDetector # Check if two tracks are spatially connected (endpoints are close) def tracks_spatially_connected?(track1, track2) - # Use preloaded points to avoid N+1 queries when available - if track1.points.loaded? && track2.points.loaded? - track1_points = track1.points.sort_by(&:timestamp) - track2_points = track2.points.sort_by(&:timestamp) - else - # Prosopite pause for direct method calls without preloading (e.g., tests) - Prosopite.pause if defined?(Prosopite) - track1_points = track1.points.order(:timestamp).to_a - track2_points = track2.points.order(:timestamp).to_a - Prosopite.resume if defined?(Prosopite) - end + return false unless track1.points.exists? && track2.points.exists? - return false if track1_points.empty? || track2_points.empty? - - # Get endpoints of both tracks from preloaded/sorted arrays - track1_start = track1_points.first - track1_end = track1_points.last - track2_start = track2_points.first - track2_end = track2_points.last + # Get endpoints of both tracks + track1_start = track1.points.order(:timestamp).first + track1_end = track1.points.order(:timestamp).last + track2_start = track2.points.order(:timestamp).first + track2_end = track2.points.order(:timestamp).last # Check various connection scenarios connection_threshold = distance_threshold_meters @@ -161,22 +149,13 @@ class Tracks::BoundaryDetector # Sort tracks by start time sorted_tracks = track_group.sort_by(&:start_at) - # Collect all points from all tracks using preloaded data + # Collect all points from all tracks all_points = [] - - # Check if any track doesn't have preloaded points - needs_query = sorted_tracks.any? { |track| !track.points.loaded? } - - Prosopite.pause if defined?(Prosopite) && needs_query - sorted_tracks.each do |track| - # Use preloaded points if available, otherwise query - track_points = track.points.loaded? ? track.points.sort_by(&:timestamp) : track.points.order(:timestamp).to_a + track_points = track.points.order(:timestamp).to_a all_points.concat(track_points) end - Prosopite.resume if defined?(Prosopite) && needs_query - # Remove duplicates and sort by timestamp unique_points = all_points.uniq(&:id).sort_by(&:timestamp) diff --git a/app/services/users/import_data/imports.rb b/app/services/users/import_data/imports.rb index be73e9ca..3eaf3b07 100644 --- a/app/services/users/import_data/imports.rb +++ b/app/services/users/import_data/imports.rb @@ -135,10 +135,6 @@ class Users::ImportData::Imports end begin - # Prosopite detects N+1 queries in ActiveStorage's internal operations - # These are unavoidable and part of ActiveStorage's design - Prosopite.pause if defined?(Prosopite) - import_record.file.attach( io: File.open(file_path), filename: import_data['original_filename'] || import_data['file_name'], @@ -152,8 +148,6 @@ class Users::ImportData::Imports ExceptionReporter.call(e, 'Import file restoration failed') false - ensure - Prosopite.resume if defined?(Prosopite) end end end diff --git a/spec/factories/points.rb b/spec/factories/points.rb index ba46092e..779f18f0 100644 --- a/spec/factories/points.rb +++ b/spec/factories/points.rb @@ -1,31 +1,5 @@ # frozen_string_literal: true -# Module to cache countries during factory creation to avoid N+1 queries -module CountriesCache - def self.get_or_create(country_name) - @cache ||= {} - @cache[country_name] ||= begin - # Pause Prosopite as this is test data setup, not application code - Prosopite.pause if defined?(Prosopite) - - country = Country.find_or_create_by(name: country_name) do |c| - iso_a2, iso_a3 = Countries::IsoCodeMapper.fallback_codes_from_country_name(country_name) - c.iso_a2 = iso_a2 - c.iso_a3 = iso_a3 - c.geom = "MULTIPOLYGON (((0 0, 1 0, 1 1, 0 1, 0 0)))" - end - - country - ensure - Prosopite.resume if defined?(Prosopite) - end - end - - def self.clear - @cache = {} - end -end - FactoryBot.define do factory :point do battery_status { 1 } @@ -123,10 +97,12 @@ FactoryBot.define do # Only set country if not already set by transient attribute unless point.read_attribute(:country) country_name = FFaker::Address.country - - # Use module-level cache to avoid N+1 queries during factory creation - country_obj = CountriesCache.get_or_create(country_name) - + country_obj = Country.find_or_create_by(name: country_name) do |country| + iso_a2, iso_a3 = Countries::IsoCodeMapper.fallback_codes_from_country_name(country_name) + country.iso_a2 = iso_a2 + country.iso_a3 = iso_a3 + country.geom = "MULTIPOLYGON (((0 0, 1 0, 1 1, 0 1, 0 0)))" + end point.write_attribute(:country, country_name) # Set the legacy string attribute point.write_attribute(:country_name, country_name) # Set the new string attribute point.country_id = country_obj.id # Set the association