Move points creation logic from background jobs to service objects

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 +1,9 @@
 # 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
+    Overland::PointsCreator.new(params, user_id).call
   end
 end

app/jobs/owntracks/point_creating_job.rb

@@ -1,16 +1,9 @@
 # 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:))
+    OwnTracks::PointCreator.new(point_params, user_id).call
   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/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

@@ -2,34 +2,33 @@
 
 require 'rails_helper'
 
-RSpec.describe 'Api::V1::Owntracks::Points', type: :request do
+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