Merge pull request #114 from Freika/background_jobs

Background jobs
This commit is contained in:
Evgenii Burmakin 2024-07-12 22:15:52 +02:00 committed by GitHub
commit 64cc0dd3b5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
30 changed files with 477 additions and 58 deletions

View file

@ -1 +1 @@
0.8.7
0.9.0

View file

@ -5,6 +5,57 @@ 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.9.0] — 2024-07-12
### Added
- Background jobs page. You can find it in Settings -> Backgroun Jobs.
- Queue clearing buttons. You can clear all jobs in the queue.
- Reverse geocoding restart button. You can restart the reverse geocoding process for all of your points.
- Reverse geocoding continue button. Click on this button will start reverse geocoding process only for points that were not processed yet.
- A lot more data is now being saved in terms of reverse geocoding process. It will be used in the future to create more insights about your data.
### Changed
- Point reference to a user is no longer optional. It should not cause any problems, but if you see any issues, please let me know.
- ⚠️ Calculation of total reverse geocoded points was changed. ⚠️ Previously, the reverse geocoding process was recording only country and city for each point. Now, it records all the data that was received from the reverse geocoding service. This means that the total number of reverse geocoded points will be different from the previous one. It is recommended to restart the reverse geocoding process to get this data for all your existing points. Below you can find an example of what kind of data is being saved to your Dawarich database:
```json
{
"place_id": 127850637,
"licence": "Data © OpenStreetMap contributors, ODbL 1.0. http://osm.org/copyright",
"osm_type": "way",
"osm_id": 718035022,
"lat": "52.51450815",
"lon": "13.350110811262352",
"class": "historic",
"type": "monument",
"place_rank": 30,
"importance": 0.4155071896625501,
"addresstype": "historic",
"name": "Victory Column",
"display_name": "Victory Column, Großer Stern, Botschaftsviertel, Tiergarten, Mitte, Berlin, 10785, Germany",
"address": {
"historic": "Victory Column",
"road": "Großer Stern",
"neighbourhood": "Botschaftsviertel",
"suburb": "Tiergarten",
"borough": "Mitte",
"city": "Berlin",
"ISO3166-2-lvl4": "DE-BE",
"postcode": "10785",
"country": "Germany",
"country_code": "de"
},
"boundingbox": [
"52.5142449",
"52.5147775",
"13.3496725",
"13.3505485"
]
}
```
## [0.8.7] — 2024-07-09
### Changed

View file

@ -87,8 +87,6 @@ You can import your existing location history from:
- Your own GPX files
- Your photos' EXIF data
⚠️ **Note**: Import of huge Google Maps Timeline files may take a long time and consume a lot of memory. It also might temporarily consume a lot of disk space due to logs. Please make sure you have enough resources before starting the import. After import is completed, you can restart your docker container and logs will be removed.
## How to start the app locally
`docker-compose up` to start the app. The app will be available at `http://localhost:3000`.
@ -114,7 +112,7 @@ Feel free to change them both in the Account section.
|-----------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------|
| MIN_MINUTES_SPENT_IN_CITY | minimum minutes between two points to consider them as visited the same city, e.g. `60` |
| TIME_ZONE | time zone, e.g. `Europe/Berlin`, full list is [here](https://github.com/Freika/dawarich/issues/27#issuecomment-2094721396) |
| APPLICATION_HOST | host of the application, e.g. `localhost` or `dawarich.example.com` |
| APPLICATION_HOSTS | list of host of the application, e.g. `localhost,dawarich.example.com` |
| BACKGROUND_PROCESSING_CONCURRENCY (only for dawarich_sidekiq service) | Number of simultaneously processed background jobs, default is 10 |
## Star History

File diff suppressed because one or more lines are too long

View file

@ -13,6 +13,12 @@ class ApplicationController < ActionController::Base
@unread_notifications ||= Notification.where(user: current_user).unread
end
def authenticate_first_user!
return if current_user == User.first
redirect_to root_path, notice: 'You are not authorized to perform this action.', status: :unauthorized
end
def authenticate_api_key
return head :unauthorized unless current_api_user

View file

@ -0,0 +1,28 @@
# frozen_string_literal: true
class Settings::BackgroundJobsController < ApplicationController
before_action :authenticate_user!
before_action :authenticate_first_user!
def index
@queues = Sidekiq::Queue.all
end
def create
EnqueueReverseGeocodingJob.perform_later(params[:job_name], current_user.id)
flash.now[:notice] = 'Job was successfully created.'
redirect_to settings_background_jobs_path, notice: 'Job was successfully created.'
end
def destroy
# Clear all jobs in the queue, params[:id] contains queue name
queue = Sidekiq::Queue.new(params[:id])
queue.clear
flash.now[:notice] = 'Queue was successfully cleared.'
redirect_to settings_background_jobs_path, notice: 'Queue was successfully cleared.'
end
end

View file

@ -23,10 +23,4 @@ class Settings::UsersController < ApplicationController
def user_params
params.require(:user).permit(:email)
end
def authenticate_first_user!
return if current_user == User.first
redirect_to settings_users_url, notice: 'You are not authorized to perform this action.', status: :unauthorized
end
end

View file

@ -109,4 +109,8 @@ module ApplicationHelper
base_title = 'Dawarich'
page_title.empty? ? base_title : "#{page_title} | #{base_title}"
end
def active_tab?(link_path)
'tab-active' if current_page?(link_path)
end
end

View file

@ -0,0 +1,9 @@
# frozen_string_literal: true
class EnqueueReverseGeocodingJob < ApplicationJob
queue_as :reverse_geocoding
def perform(job_name, user_id)
Jobs::Create.new(job_name, user_id).call
end
end

View file

@ -6,14 +6,6 @@ class ReverseGeocodingJob < ApplicationJob
def perform(point_id)
return unless REVERSE_GEOCODING_ENABLED
point = Point.find(point_id)
return if point.city.present? && point.country.present?
result = Geocoder.search([point.latitude, point.longitude])
return if result.blank?
point.update!(city: result.first.city, country: result.first.country)
rescue ActiveRecord::RecordNotFound => e
Rails.logger.error("Point with id #{point_id} not found: #{e.message}")
ReverseGeocoding::FetchData.new(point_id).call
end
end

View file

@ -1,8 +1,10 @@
# frozen_string_literal: true
class Point < ApplicationRecord
reverse_geocoded_by :latitude, :longitude
belongs_to :import, optional: true
belongs_to :user, optional: true
belongs_to :user
validates :latitude, :longitude, :timestamp, presence: true
@ -14,6 +16,9 @@ class Point < ApplicationRecord
}, _suffix: true
enum connection: { mobile: 0, wifi: 1, offline: 2 }, _suffix: true
scope :reverse_geocoded, -> { where.not(city: nil, country: nil) }
scope :not_reverse_geocoded, -> { where(city: nil, country: nil) }
after_create :async_reverse_geocode
def self.without_raw_data
@ -24,8 +29,6 @@ class Point < ApplicationRecord
Time.zone.at(timestamp)
end
private
def async_reverse_geocode
return unless REVERSE_GEOCODING_ENABLED

View file

@ -45,7 +45,7 @@ class User < ApplicationRecord
end
def total_reverse_geocoded
points.select(:id).where.not(country: nil, city: nil).count
points.select(:id).where.not(geodata: {}).count
end
private

View file

@ -0,0 +1,25 @@
# frozen_string_literal: true
class Jobs::Create
class InvalidJobName < StandardError; end
attr_reader :job_name, :user
def initialize(job_name, user_id)
@job_name = job_name
@user = User.find(user_id)
end
def call
points =
case job_name
when 'start_reverse_geocoding'
user.tracked_points
when 'continue_reverse_geocoding'
user.tracked_points.not_reverse_geocoded
else
raise InvalidJobName, 'Invalid job name'
end
points.each(&:async_reverse_geocode)
end
end

View file

@ -0,0 +1,26 @@
# frozen_string_literal: true
class ReverseGeocoding::FetchData
attr_reader :point
def initialize(point_id)
@point = Point.find(point_id)
rescue ActiveRecord::RecordNotFound => e
Rails.logger.error("Point with id #{point_id} not found: #{e.message}")
end
def call
return if reverse_geocoded?
response = Geocoder.search([point.latitude, point.longitude]).first
return if response.blank? || response.data['error'].present?
point.update!(city: response.city, country: response.country, geodata: response.data)
end
private
def reverse_geocoded?
point.city.present? && point.country.present? || point.geodata.present?
end
end

View file

@ -1,4 +1,4 @@
<div role="<%= notification.kind %>" class="<%= notification.kind %> shadow-lg p-5 flex justify-between items-center mb-4 rounded-lg bg-neutral" id="<%= dom_id notification %>">
<div role="<%= notification.kind %>" class="<%= notification.kind %> shadow-lg p-5 flex justify-between items-center mb-4 rounded-lg bg-base-200" id="<%= dom_id notification %>">
<div class="flex-1">
<h3 class="font-bold text-xl">
<%= link_to notification.title, notification, class: 'link hover:no-underline text-blue-600' %>

View file

@ -0,0 +1,4 @@
<div role="tablist" class="tabs tabs-lifted tabs-lg">
<%= link_to 'Main', settings_path, role: 'tab', class: "tab #{active_tab?(settings_path)}" %>
<%= link_to 'Background Jobs', settings_background_jobs_path, role: 'tab', class: "tab #{active_tab?(settings_background_jobs_path)}" %>
</div>

View file

@ -0,0 +1,58 @@
<% content_for :title, "Background jobs" %>
<div class="min-h-content w-full">
<%= render 'settings/navigation' %>
<div class="flex justify-between items-center mt-5">
<h1 class="font-bold text-4xl">Background jobs</h1>
</div>
<div role="alert" class="alert m-5">
<svg
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
class="stroke-info h-6 w-6 shrink-0">
<path
stroke-linecap="round"
stroke-linejoin="round"
stroke-width="2"
d="M13 16h-1v-4h-1m1-4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z"></path>
</svg>
<span>Spamming many new jobs at once is a bad idea. Let them work or clear the queue beforehand.</span>
</div>
<div class='flex'>
<div class="card bg-base-300 w-96 shadow-xl m-5">
<div class="card-body">
<h2 class="card-title">Start Reverse Geocoding</h2>
<p>This job will re-run reverse geocoding process for all the points in your database. Might take a few days or even weeks based on the amount of points you have!</p>
<div class="card-actions justify-end">
<%= link_to 'Start Job', settings_background_jobs_path(job_name: 'start_reverse_geocoding'), method: :post, data: { confirm: 'Are you sure?', turbo_confirm: 'Are you sure?', turbo_method: :post }, class: 'btn btn-primary' %>
</div>
</div>
</div>
<div class="card bg-base-300 w-96 shadow-xl m-5">
<div class="card-body">
<h2 class="card-title">Continue Reverse Geocoding</h2>
<p>This job will process reverse geocoding for all points that don't have geocoding data yet.</p>
<div class="card-actions justify-end">
<%= link_to 'Start Job', settings_background_jobs_path(job_name: 'continue_reverse_geocoding'), method: :post, data: { confirm: 'Are you sure?', turbo_confirm: 'Are you sure?', turbo_method: :post }, class: 'btn btn-primary' %>
</div>
</div>
</div>
</div>
<div id="settings_background_jobs" class="min-w-full">
<% @queues.each do |queue| %>
<div class="card shadow-2xl bg-base-300 p-5 m-5">
<h2 class="text-2xl font-bold"><%= queue.name %></h2>
<div class="flex justify-between items-center">
<p class="text-lg">Jobs in queue: <%= queue.size %></p>
<%= link_to 'Clear queue', settings_background_job_path(queue.name), method: :delete, data: { confirm: 'Are you sure?', turbo_confirm: 'Are you sure?', turbo_method: :delete }, class: 'btn btn-primary' %>
</div>
</div>
<% end %>
</div>
</div>

View file

@ -1,6 +1,8 @@
<% content_for :title, 'Settings' %>
<div class="min-h-content bg-base-200 w-full">
<div class="min-h-content w-full">
<%= render 'settings/navigation' %>
<div class="flex flex-col lg:flex-row w-full my-10 space-x-4">
<div class="card flex-shrink-0 w-full max-w-sm shadow-2xl bg-base-100 px-5 py-5 mx-5">
<h2 class="text-2xl font-bold">Edit your Dawarich settings!</h1>

View file

@ -1,4 +1,5 @@
# config/initializers/geocoder.rb
# frozen_string_literal: true
Geocoder.configure(
# geocoding service request timeout, in seconds (default 3):
# timeout: 5,
@ -9,7 +10,7 @@ Geocoder.configure(
# caching (see Caching section below for details):
cache: Redis.new,
cache_options: {
expiration: 1.day, # Defaults to `nil`
expiration: 1.day # Defaults to `nil`
# prefix: "another_key:" # Defaults to `geocoder:`
}
)

View file

@ -9,6 +9,7 @@ Rails.application.routes.draw do
resources :settings, only: :index
namespace :settings do
resources :background_jobs, only: %i[index create destroy]
resources :users, only: :create
end

View file

@ -0,0 +1,8 @@
# frozen_string_literal: true
class AddGeodataToPoints < ActiveRecord::Migration[7.1]
def change
add_column :points, :geodata, :jsonb, null: false, default: {}
add_index :points, :geodata, using: :gin
end
end

4
db/schema.rb generated
View file

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.1].define(version: 2024_07_03_105734) do
ActiveRecord::Schema[7.1].define(version: 2024_07_12_141303) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -109,12 +109,14 @@ ActiveRecord::Schema[7.1].define(version: 2024_07_03_105734) do
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.bigint "user_id"
t.jsonb "geodata", default: {}, null: false
t.index ["altitude"], name: "index_points_on_altitude"
t.index ["battery"], name: "index_points_on_battery"
t.index ["battery_status"], name: "index_points_on_battery_status"
t.index ["city"], name: "index_points_on_city"
t.index ["connection"], name: "index_points_on_connection"
t.index ["country"], name: "index_points_on_country"
t.index ["geodata"], name: "index_points_on_geodata", using: :gin
t.index ["import_id"], name: "index_points_on_import_id"
t.index ["latitude", "longitude"], name: "index_points_on_latitude_and_longitude"
t.index ["timestamp"], name: "index_points_on_timestamp"

View file

@ -25,5 +25,6 @@ FactoryBot.define do
import_id { '' }
city { nil }
country { nil }
user
end
end

View file

@ -0,0 +1,14 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe EnqueueReverseGeocodingJob, type: :job do
let(:job_name) { 'start_reverse_geocoding' }
let(:user_id) { 1 }
it 'calls job creation service' do
expect(Jobs::Create).to receive(:new).with(job_name, user_id).and_return(double(call: nil))
EnqueueReverseGeocodingJob.perform_now(job_name, user_id)
end
end

View file

@ -1,3 +1,5 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe ReverseGeocodingJob, type: :job do
@ -17,44 +19,27 @@ RSpec.describe ReverseGeocodingJob, type: :job do
expect { perform }.not_to(change { point.reload.city })
end
it 'does not call Geocoder' do
it 'does not call ReverseGeocoding::FetchData' do
allow(ReverseGeocoding::FetchData).to receive(:new).and_call_original
perform
expect(Geocoder).not_to have_received(:search)
expect(ReverseGeocoding::FetchData).not_to have_received(:new)
end
end
context 'when REVERSE_GEOCODING_ENABLED is true' do
before { stub_const('REVERSE_GEOCODING_ENABLED', true) }
it 'updates point with city and country' do
expect { perform }.to change { point.reload.city }.from(nil)
end
let(:stubbed_geocoder) { OpenStruct.new(data: { city: 'City', country: 'Country' }) }
it 'calls Geocoder' do
allow(Geocoder).to receive(:search).and_return([stubbed_geocoder])
allow(ReverseGeocoding::FetchData).to receive(:new).and_call_original
perform
expect(Geocoder).to have_received(:search).with([point.latitude, point.longitude])
end
context 'when point has city and country' do
let(:point) { create(:point, city: 'City', country: 'Country') }
before do
allow(Geocoder).to receive(:search).and_return(
[double(city: 'Another city', country: 'Some country')]
)
end
it 'does not update point' do
expect { perform }.not_to change { point.reload.city }
end
it 'does not call Geocoder' do
perform
expect(Geocoder).not_to have_received(:search)
end
expect(ReverseGeocoding::FetchData).to have_received(:new).with(point.id)
end
end
end

View file

@ -5,7 +5,7 @@ require 'rails_helper'
RSpec.describe Point, type: :model do
describe 'associations' do
it { is_expected.to belong_to(:import).optional }
it { is_expected.to belong_to(:user).optional }
it { is_expected.to belong_to(:user) }
end
describe 'validations' do
@ -13,4 +13,24 @@ RSpec.describe Point, type: :model do
it { is_expected.to validate_presence_of(:longitude) }
it { is_expected.to validate_presence_of(:timestamp) }
end
describe 'scopes' do
describe '.reverse_geocoded' do
let(:point) { create(:point, country: 'Country', city: 'City') }
let(:point_without_address) { create(:point, city: nil, country: nil) }
it 'returns points with reverse geocoded address' do
expect(described_class.reverse_geocoded).to eq([point])
end
end
describe '.not_reverse_geocoded' do
let(:point) { create(:point, country: 'Country', city: 'City') }
let(:point_without_address) { create(:point, city: nil, country: nil) }
it 'returns points without reverse geocoded address' do
expect(described_class.not_reverse_geocoded).to eq([point_without_address])
end
end
end
end

View file

@ -77,7 +77,8 @@ RSpec.describe User, type: :model do
user:,
toponyms: [
{ 'cities' => [], 'country' => nil },
{ 'cities' => [{ 'city' => 'Berlin', 'points' => 64, 'timestamp' => 1710446806, 'stayed_for' => 8772 }], 'country' => 'Germany' }
{ 'cities' => [{ 'city' => 'Berlin', 'points' => 64, 'timestamp' => 1_710_446_806, 'stayed_for' => 8772 }],
'country' => 'Germany' }
]
)
end
@ -91,7 +92,10 @@ RSpec.describe User, type: :model do
subject { user.total_reverse_geocoded }
let(:import) { create(:import, user:) }
let!(:point) { create(:point, country: 'Country', city: 'City', import:) }
let!(:reverse_geocoded_point) do
create(:point, country: 'Country', city: 'City', geodata: { some: 'data' }, import:)
end
let!(:not_reverse_geocoded_point) { create(:point, country: 'Country', city: 'City', import:) }
it 'returns number of reverse geocoded points' do
expect(subject).to eq(1)

View file

@ -0,0 +1,69 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe '/settings/background_jobs', type: :request do
before do
stub_request(:any, 'https://api.github.com/repos/Freika/dawarich/tags')
.to_return(status: 200, body: '[{"name": "1.0.0"}]', headers: {})
end
context 'when user is not authenticated' do
it 'redirects to sign in page' do
get settings_background_jobs_url
expect(response).to redirect_to(new_user_session_url)
end
end
context 'when user is authenticated' do
let(:user) { create(:user) }
before do
sign_in user
end
describe 'GET /index' do
it 'renders a successful response' do
get settings_background_jobs_url
expect(response).to be_successful
end
end
describe 'POST /create' do
let(:params) { { job_name: 'start_reverse_geocoding' } }
context 'with valid parameters' do
it 'enqueues a new job' do
expect do
post settings_background_jobs_url, params:
end.to have_enqueued_job(EnqueueReverseGeocodingJob)
end
it 'redirects to the created settings_background_job' do
post(settings_background_jobs_url, params:)
expect(response).to redirect_to(settings_background_jobs_url)
end
end
end
describe 'DELETE /destroy' do
it 'clears the Sidekiq queue' do
queue = instance_double(Sidekiq::Queue)
allow(Sidekiq::Queue).to receive(:new).and_return(queue)
expect(queue).to receive(:clear)
delete settings_background_job_url('queue_name')
end
it 'redirects to the settings_background_jobs list' do
delete settings_background_job_url('queue_name')
expect(response).to redirect_to(settings_background_jobs_url)
end
end
end
end

View file

@ -0,0 +1,50 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe Jobs::Create do
describe '#call' do
context 'when job_name is start_reverse_geocoding' do
let(:user) { create(:user) }
let(:points) { create_list(:point, 4, user:) }
let(:job_name) { 'start_reverse_geocoding' }
it 'enqueues reverse geocoding for all user points' do
allow(ReverseGeocodingJob).to receive(:perform_later).and_return(nil)
described_class.new(job_name, user.id).call
points.each do |point|
expect(ReverseGeocodingJob).to have_received(:perform_later).with(point.id)
end
end
end
context 'when job_name is continue_reverse_geocoding' do
let(:user) { create(:user) }
let(:points_without_address) { create_list(:point, 4, user:, country: nil, city: nil) }
let(:points_with_address) { create_list(:point, 5, user:, country: 'Country', city: 'City') }
let(:job_name) { 'continue_reverse_geocoding' }
it 'enqueues reverse geocoding for all user points without address' do
allow(ReverseGeocodingJob).to receive(:perform_later).and_return(nil)
described_class.new(job_name, user.id).call
points_without_address.each do |point|
expect(ReverseGeocodingJob).to have_received(:perform_later).with(point.id)
end
end
end
context 'when job_name is invalid' do
let(:user) { create(:user) }
let(:job_name) { 'invalid_job_name' }
it 'raises an error' do
expect { described_class.new(job_name, user.id).call }.to raise_error(Jobs::Create::InvalidJobName)
end
end
end
end

View file

@ -0,0 +1,64 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe ReverseGeocoding::FetchData do
subject(:fetch_data) { described_class.new(point.id).call }
let(:point) { create(:point) }
context 'when Geocoder returns city and country' do
before do
allow(Geocoder).to receive(:search).and_return([double(city: 'City', country: 'Country',
data: { 'address' => 'Address' })])
end
context 'when point does not have city and country' do
it 'updates point with city and country' do
expect { fetch_data }.to change { point.reload.city }
.from(nil).to('City')
.and change { point.reload.country }.from(nil).to('Country')
end
it 'updates point with geodata' do
expect { fetch_data }.to change { point.reload.geodata }.from({}).to('address' => 'Address')
end
it 'calls Geocoder' do
fetch_data
expect(Geocoder).to have_received(:search).with([point.latitude, point.longitude])
end
end
context 'when point has city and country' do
let(:point) { create(:point, city: 'City', country: 'Country') }
before do
allow(Geocoder).to receive(:search).and_return(
[double(city: 'Another city', country: 'Some country')]
)
end
it 'does not update point' do
expect { fetch_data }.not_to(change { point.reload.city })
end
it 'does not call Geocoder' do
fetch_data
expect(Geocoder).not_to have_received(:search)
end
end
end
context 'when Geocoder returns an error' do
before do
allow(Geocoder).to receive(:search).and_return([double(data: { 'error' => 'Error' })])
end
it 'does not update point' do
expect { fetch_data }.not_to(change { point.reload.city })
end
end
end