From ed7b6d6d2475a7bf4f226bc0f0bfd3a7f57fb826 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Mon, 12 May 2025 22:33:47 +0200 Subject: [PATCH] Add a `STORE_GEODATA` environment variable to control whether to store geodata in the database. --- CHANGELOG.md | 12 ++++++++++++ app/models/place.rb | 6 ------ app/models/point.rb | 2 +- app/services/jobs/create.rb | 4 +--- config/initializers/01_constants.rb | 1 + config/initializers/03_dawarich_settings.rb | 4 ++++ spec/models/place_spec.rb | 9 --------- spec/models/point_spec.rb | 15 ++++++++++++++- spec/services/jobs/create_spec.rb | 5 ++++- 9 files changed, 37 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5983d2ab..b150176a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # 0.26.1 - 2025-05-12 +Geodata on demand + +- [x] Introduce a `STORE_GEODATA` environment variable to control whether to store geodata in the database. +- [ ] When `STORE_GEODATA` is disabled, each feature that uses geodata will now make a direct request to the geocoding service to calculate required data. +- [ ] When `STORE_GEODATA` is disabled, points are not being reverse geocoded on creation. +- [ ] When `STORE_GEODATA` is enabled, points are being reverse geocoded upon creation and stored in the database. +- [ ] Each feature that uses geodata will check if an entity (point, place, etc.) has geodata stored in the database and use it if available. If not, it will make a direct request to the geocoding service to calculate required data. + +## Changed + +- Reverse geocoding is now working as on-demand job instead of storing the result in the database. + ## Fixed - Fixed a bug with an attempt to write points with same lonlat and timestamp from iOS app. #1170 diff --git a/app/models/place.rb b/app/models/place.rb index 836358c8..99f57756 100644 --- a/app/models/place.rb +++ b/app/models/place.rb @@ -22,12 +22,6 @@ class Place < ApplicationRecord lonlat.y end - def async_reverse_geocode - return unless DawarichSettings.reverse_geocoding_enabled? - - ReverseGeocodingJob.perform_later(self.class.to_s, id) - end - def reverse_geocoded? geodata.present? end diff --git a/app/models/point.rb b/app/models/point.rb index 38970d91..68ed47c9 100644 --- a/app/models/point.rb +++ b/app/models/point.rb @@ -28,7 +28,7 @@ class Point < ApplicationRecord scope :visited, -> { where.not(visit_id: nil) } scope :not_visited, -> { where(visit_id: nil) } - after_create :async_reverse_geocode + after_create :async_reverse_geocode, if: -> { DawarichSettings.store_geodata? } after_create_commit :broadcast_coordinates def self.without_raw_data diff --git a/app/services/jobs/create.rb b/app/services/jobs/create.rb index bbbcb15c..0d46b60f 100644 --- a/app/services/jobs/create.rb +++ b/app/services/jobs/create.rb @@ -21,8 +21,6 @@ class Jobs::Create raise InvalidJobName, 'Invalid job name' end - points.find_each(batch_size: 1_000) do |point| - point.async_reverse_geocode - end + points.find_each(&:async_reverse_geocode) end end diff --git a/config/initializers/01_constants.rb b/config/initializers/01_constants.rb index 6a475aa2..1f3203da 100644 --- a/config/initializers/01_constants.rb +++ b/config/initializers/01_constants.rb @@ -17,6 +17,7 @@ NOMINATIM_API_KEY = ENV.fetch('NOMINATIM_API_KEY', nil) NOMINATIM_API_USE_HTTPS = ENV.fetch('NOMINATIM_API_USE_HTTPS', 'true') == 'true' GEOAPIFY_API_KEY = ENV.fetch('GEOAPIFY_API_KEY', nil) +STORE_GEODATA = ENV.fetch('STORE_GEODATA', 'false') == 'true' # /Reverse geocoding settings SENTRY_DSN = ENV.fetch('SENTRY_DSN', nil) diff --git a/config/initializers/03_dawarich_settings.rb b/config/initializers/03_dawarich_settings.rb index 96ee30ee..19f7394e 100644 --- a/config/initializers/03_dawarich_settings.rb +++ b/config/initializers/03_dawarich_settings.rb @@ -32,5 +32,9 @@ class DawarichSettings def nominatim_enabled? @nominatim_enabled ||= NOMINATIM_API_HOST.present? end + + def store_geodata? + @store_geodata ||= STORE_GEODATA + end end end diff --git a/spec/models/place_spec.rb b/spec/models/place_spec.rb index 6787c3f9..cf6852c4 100644 --- a/spec/models/place_spec.rb +++ b/spec/models/place_spec.rb @@ -21,15 +21,6 @@ RSpec.describe Place, type: :model do describe 'methods' do let(:place) { create(:place, :with_geodata) } - describe '#async_reverse_geocode' do - before { allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(true) } - before { allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(true) } - - it 'updates address' do - expect { place.async_reverse_geocode }.to have_enqueued_job(ReverseGeocodingJob).with('Place', place.id) - end - end - describe '#osm_id' do it 'returns the osm_id' do expect(place.osm_id).to eq(5_762_449_774) diff --git a/spec/models/point_spec.rb b/spec/models/point_spec.rb index 7f5f03e9..92f36f04 100644 --- a/spec/models/point_spec.rb +++ b/spec/models/point_spec.rb @@ -45,7 +45,10 @@ RSpec.describe Point, type: :model do describe '#async_reverse_geocode' do let(:point) { build(:point) } - before { allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(true) } + before do + allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(true) + allow(DawarichSettings).to receive(:store_geodata?).and_return(true) + end it 'enqueues ReverseGeocodeJob with correct arguments' do point.save @@ -61,6 +64,16 @@ RSpec.describe Point, type: :model do expect { point.async_reverse_geocode }.to have_enqueued_job(ReverseGeocodingJob) end end + + context 'when reverse geocoding is disabled' do + before do + allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(false) + end + + it 'does not enqueue ReverseGeocodeJob' do + expect { point.save }.not_to have_enqueued_job(ReverseGeocodingJob) + end + end end describe '#lon' do diff --git a/spec/services/jobs/create_spec.rb b/spec/services/jobs/create_spec.rb index 84988ff3..ad28f498 100644 --- a/spec/services/jobs/create_spec.rb +++ b/spec/services/jobs/create_spec.rb @@ -4,7 +4,10 @@ require 'rails_helper' RSpec.describe Jobs::Create do describe '#call' do - before { allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(true) } + before do + allow(DawarichSettings).to receive(:reverse_geocoding_enabled?).and_return(true) + allow(DawarichSettings).to receive(:store_geodata?).and_return(true) + end context 'when job_name is start_reverse_geocoding' do let(:user) { create(:user) }