diff --git a/CHANGELOG.md b/CHANGELOG.md index 677335b2..0066e76e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Map V2 points loading is significantly sped up. - Points size on Map V2 was reduced to prevent overlapping. +- Points sent from Owntracks and Overland are now being created synchronously to instantly reflect success or failure of point creation. # [0.37.2] - 2026-01-04 diff --git a/app/controllers/api/v1/overland/batches_controller.rb b/app/controllers/api/v1/overland/batches_controller.rb index 18ea576a..9abd1679 100644 --- a/app/controllers/api/v1/overland/batches_controller.rb +++ b/app/controllers/api/v1/overland/batches_controller.rb @@ -5,7 +5,7 @@ class Api::V1::Overland::BatchesController < ApiController before_action :validate_points_limit, only: %i[create] def create - Overland::BatchCreatingJob.perform_later(batch_params, current_api_user.id) + Overland::PointsCreator.new(batch_params, current_api_user.id).call render json: { result: 'ok' }, status: :created end diff --git a/app/controllers/api/v1/owntracks/points_controller.rb b/app/controllers/api/v1/owntracks/points_controller.rb index 6f97cfe9..42ce9d7e 100644 --- a/app/controllers/api/v1/owntracks/points_controller.rb +++ b/app/controllers/api/v1/owntracks/points_controller.rb @@ -5,7 +5,7 @@ class Api::V1::Owntracks::PointsController < ApiController before_action :validate_points_limit, only: %i[create] def create - Owntracks::PointCreatingJob.perform_later(point_params, current_api_user.id) + OwnTracks::PointCreator.new(point_params, current_api_user.id).call render json: {}, status: :ok end diff --git a/app/jobs/overland/batch_creating_job.rb b/app/jobs/overland/batch_creating_job.rb deleted file mode 100644 index 2933e81b..00000000 --- a/app/jobs/overland/batch_creating_job.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -class Overland::BatchCreatingJob < ApplicationJob - include PointValidation - - queue_as :points - - def perform(params, user_id) - data = Overland::Params.new(params).call - - data.each do |location| - next if location[:lonlat].nil? - next if point_exists?(location, user_id) - - Point.create!(location.merge(user_id:)) - end - end -end diff --git a/app/jobs/owntracks/point_creating_job.rb b/app/jobs/owntracks/point_creating_job.rb deleted file mode 100644 index 63ff6c90..00000000 --- a/app/jobs/owntracks/point_creating_job.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -class Owntracks::PointCreatingJob < ApplicationJob - include PointValidation - - queue_as :points - - def perform(point_params, user_id) - parsed_params = OwnTracks::Params.new(point_params).call - - return if parsed_params.try(:[], :timestamp).nil? || parsed_params.try(:[], :lonlat).nil? - return if point_exists?(parsed_params, user_id) - - Point.create!(parsed_params.merge(user_id:)) - end -end diff --git a/app/services/overland/params.rb b/app/services/overland/params.rb index 40c33599..e140678a 100644 --- a/app/services/overland/params.rb +++ b/app/services/overland/params.rb @@ -4,16 +4,18 @@ class Overland::Params attr_reader :data, :points def initialize(json) - @data = json.with_indifferent_access - @points = @data[:locations] + @data = normalize(json) + @points = Array.wrap(@data[:locations]) end def call + return [] if points.blank? + points.map do |point| next if point[:geometry].nil? || point.dig(:properties, :timestamp).nil? { - lonlat: "POINT(#{point[:geometry][:coordinates][0]} #{point[:geometry][:coordinates][1]})", + lonlat: lonlat(point), battery_status: point[:properties][:battery_state], battery: battery_level(point[:properties][:battery_level]), timestamp: DateTime.parse(point[:properties][:timestamp]), @@ -35,4 +37,26 @@ class Overland::Params value.positive? ? value : nil end + + def lonlat(point) + coordinates = point.dig(:geometry, :coordinates) + return if coordinates.blank? + + "POINT(#{coordinates[0]} #{coordinates[1]})" + end + + def normalize(json) + payload = case json + when ActionController::Parameters + json.to_unsafe_h + when Hash + json + when Array + { locations: json } + else + json.respond_to?(:to_h) ? json.to_h : {} + end + + payload.with_indifferent_access + end end diff --git a/app/services/overland/points_creator.rb b/app/services/overland/points_creator.rb new file mode 100644 index 00000000..5c6b7814 --- /dev/null +++ b/app/services/overland/points_creator.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +class Overland::PointsCreator + RETURNING_COLUMNS = 'id, timestamp, ST_X(lonlat::geometry) AS longitude, ST_Y(lonlat::geometry) AS latitude' + + attr_reader :params, :user_id + + def initialize(params, user_id) + @params = params + @user_id = user_id + end + + def call + data = Overland::Params.new(params).call + return [] if data.blank? + + payload = data + .compact + .reject { |location| location[:lonlat].nil? || location[:timestamp].nil? } + .map { |location| location.merge(user_id:) } + + upsert_points(payload) + end + + private + + def upsert_points(locations) + created_points = [] + + locations.each_slice(1000) do |batch| + result = Point.upsert_all( + batch, + unique_by: %i[lonlat timestamp user_id], + returning: Arel.sql(RETURNING_COLUMNS) + ) + created_points.concat(result) if result + end + + created_points + end +end diff --git a/app/services/own_tracks/point_creator.rb b/app/services/own_tracks/point_creator.rb new file mode 100644 index 00000000..7d13862d --- /dev/null +++ b/app/services/own_tracks/point_creator.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +class OwnTracks::PointCreator + RETURNING_COLUMNS = 'id, timestamp, ST_X(lonlat::geometry) AS longitude, ST_Y(lonlat::geometry) AS latitude' + + attr_reader :params, :user_id + + def initialize(params, user_id) + @params = params + @user_id = user_id + end + + def call + parsed_params = OwnTracks::Params.new(params).call + return [] if parsed_params.blank? + + payload = parsed_params.merge(user_id:) + return [] if payload[:timestamp].nil? || payload[:lonlat].nil? + + upsert_points([payload]) + end + + private + + def upsert_points(locations) + created_points = [] + + locations.each_slice(1000) do |batch| + result = Point.upsert_all( + batch, + unique_by: %i[lonlat timestamp user_id], + returning: Arel.sql(RETURNING_COLUMNS) + ) + created_points.concat(result) if result + end + + created_points + end +end diff --git a/spec/jobs/overland/batch_creating_job_spec.rb b/spec/jobs/overland/batch_creating_job_spec.rb deleted file mode 100644 index 75446df1..00000000 --- a/spec/jobs/overland/batch_creating_job_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Overland::BatchCreatingJob, type: :job do - describe '#perform' do - subject(:perform) { described_class.new.perform(json, user.id) } - - let(:file_path) { 'spec/fixtures/files/overland/geodata.json' } - let(:file) { File.open(file_path) } - let(:json) { JSON.parse(file.read) } - let(:user) { create(:user) } - - it 'creates a location' do - expect { perform }.to change { Point.count }.by(1) - end - - it 'creates a point with the correct user_id' do - perform - - expect(Point.last.user_id).to eq(user.id) - end - - context 'when point already exists' do - it 'does not create a point' do - perform - - expect { perform }.not_to(change { Point.count }) - end - end - end -end diff --git a/spec/jobs/owntracks/point_creating_job_spec.rb b/spec/jobs/owntracks/point_creating_job_spec.rb deleted file mode 100644 index ae8d49fb..00000000 --- a/spec/jobs/owntracks/point_creating_job_spec.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Owntracks::PointCreatingJob, type: :job do - describe '#perform' do - subject(:perform) { described_class.new.perform(point_params, user.id) } - - let(:point_params) do - { lat: 1.0, lon: 1.0, tid: 'test', tst: Time.now.to_i, topic: 'iPhone 12 pro' } - end - let(:user) { create(:user) } - - it 'creates a point' do - expect { perform }.to change { Point.count }.by(1) - end - - it 'creates a point with the correct user_id' do - perform - - expect(Point.last.user_id).to eq(user.id) - end - - context 'when point already exists' do - it 'does not create a point' do - perform - - expect { perform }.not_to(change { Point.count }) - end - end - - context 'when point is invalid' do - let(:point_params) { { lat: 1.0, lon: 1.0, tid: 'test', tst: nil, topic: 'iPhone 12 pro' } } - - it 'does not create a point' do - expect { perform }.not_to(change { Point.count }) - end - end - end -end diff --git a/spec/requests/api/v1/overland/batches_spec.rb b/spec/requests/api/v1/overland/batches_spec.rb index df5f74e0..a79fdc7b 100644 --- a/spec/requests/api/v1/overland/batches_spec.rb +++ b/spec/requests/api/v1/overland/batches_spec.rb @@ -26,10 +26,10 @@ RSpec.describe 'Api::V1::Overland::Batches', type: :request do expect(response).to have_http_status(:created) end - it 'enqueues a job' do + it 'creates points immediately' do expect do post "/api/v1/overland/batches?api_key=#{user.api_key}", params: params - end.to have_enqueued_job(Overland::BatchCreatingJob) + end.to change(Point, :count).by(1) end context 'when user is inactive' do diff --git a/spec/requests/api/v1/owntracks/points_spec.rb b/spec/requests/api/v1/owntracks/points_spec.rb index f8e6128e..104ddd93 100644 --- a/spec/requests/api/v1/owntracks/points_spec.rb +++ b/spec/requests/api/v1/owntracks/points_spec.rb @@ -4,32 +4,31 @@ require 'rails_helper' RSpec.describe 'Api::V1::Owntracks::Points', type: :request do describe 'POST /api/v1/owntracks/points' do - context 'with valid params' do - let(:params) do - { lat: 1.0, lon: 1.0, tid: 'test', tst: Time.current.to_i, topic: 'iPhone 12 pro' } + let(:file_path) { 'spec/fixtures/files/owntracks/2024-03.rec' } + let(:json) { OwnTracks::RecParser.new(File.read(file_path)).call } + let(:point_params) { json.first } + + context 'with invalid api key' do + it 'returns http unauthorized' do + post '/api/v1/owntracks/points', params: point_params + + expect(response).to have_http_status(:unauthorized) end + end + + context 'with valid api key' do let(:user) { create(:user) } - context 'with invalid api key' do - it 'returns http unauthorized' do - post api_v1_owntracks_points_path, params: params + it 'returns ok' do + post "/api/v1/owntracks/points?api_key=#{user.api_key}", params: point_params - expect(response).to have_http_status(:unauthorized) - end + expect(response).to have_http_status(:ok) end - context 'with valid api key' do - it 'returns http success' do - post api_v1_owntracks_points_path(api_key: user.api_key), params: params - - expect(response).to have_http_status(:success) - end - - it 'enqueues a job' do - expect do - post api_v1_owntracks_points_path(api_key: user.api_key), params: params - end.to have_enqueued_job(Owntracks::PointCreatingJob) - end + it 'creates a point immediately' do + expect do + post "/api/v1/owntracks/points?api_key=#{user.api_key}", params: point_params + end.to change(Point, :count).by(1) end context 'when user is inactive' do @@ -38,7 +37,7 @@ RSpec.describe 'Api::V1::Owntracks::Points', type: :request do end it 'returns http unauthorized' do - post api_v1_owntracks_points_path(api_key: user.api_key), params: params + post "/api/v1/owntracks/points?api_key=#{user.api_key}", params: point_params expect(response).to have_http_status(:unauthorized) end diff --git a/spec/services/overland/points_creator_spec.rb b/spec/services/overland/points_creator_spec.rb new file mode 100644 index 00000000..80c98e83 --- /dev/null +++ b/spec/services/overland/points_creator_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Overland::PointsCreator do + subject(:call_service) { described_class.new(payload, user.id).call } + + let(:user) { create(:user) } + let(:file_path) { 'spec/fixtures/files/overland/geodata.json' } + let(:payload_hash) { JSON.parse(File.read(file_path)) } + + context 'with a hash payload' do + let(:payload) { payload_hash } + + it 'creates points synchronously' do + expect { call_service }.to change { Point.where(user:).count }.by(1) + end + + it 'returns the created points with coordinates' do + result = call_service + + expect(result.first).to include('id', 'timestamp', 'longitude', 'latitude') + end + + it 'does not duplicate existing points' do + call_service + + expect { call_service }.not_to change { Point.where(user:).count } + end + end + + context 'with a locations array payload' do + let(:payload) { payload_hash['locations'] } + + it 'processes the array successfully' do + expect { call_service }.to change { Point.where(user:).count }.by(1) + end + end + + context 'with invalid data' do + let(:payload) { { 'locations' => [{ 'properties' => { 'timestamp' => nil } }] } } + + it 'returns an empty array' do + expect(call_service).to eq([]) + end + end +end diff --git a/spec/services/own_tracks/point_creator_spec.rb b/spec/services/own_tracks/point_creator_spec.rb new file mode 100644 index 00000000..4221c685 --- /dev/null +++ b/spec/services/own_tracks/point_creator_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe OwnTracks::PointCreator do + subject(:call_service) { described_class.new(point_params, user.id).call } + + let(:user) { create(:user) } + let(:file_path) { 'spec/fixtures/files/owntracks/2024-03.rec' } + let(:point_params) { OwnTracks::RecParser.new(File.read(file_path)).call.first } + + it 'creates a point immediately' do + expect { call_service }.to change { Point.where(user:).count }.by(1) + end + + it 'returns created point coordinates' do + result = call_service + + expect(result.first).to include('id', 'timestamp', 'longitude', 'latitude') + end + + it 'avoids duplicate points' do + call_service + + expect { call_service }.not_to change { Point.where(user:).count } + end + + context 'when params are invalid' do + let(:point_params) { { lat: nil } } + + it 'returns an empty array' do + expect(call_service).to eq([]) + end + end +end