Move points creation logic from background jobs to service objects (#2145)

* Move points creation logic from background jobs to service objects

* Remove unused point creation jobs

* Update changelog

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
 

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

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

app/jobs/overland/batch_creating_job.rb

@@ -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

app/jobs/owntracks/point_creating_job.rb

@@ -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

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

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

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

spec/jobs/overland/batch_creating_job_spec.rb

@@ -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

spec/jobs/owntracks/point_creating_job_spec.rb

@@ -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

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

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

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

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