Skip points without lonlat and timestamp from Owntracks

CHANGELOG.md

@@ -4,6 +4,14 @@ 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.26.1 - 2025-05-12
+
+## Fixed
+
+- Fixed a bug with an attempt to write points with same lonlat and timestamp from iOS app. #1170
+
+
+
 # 0.26.0 - 2025-05-08
 
 ⚠️ This release includes a breaking change. ⚠️

Gemfile

@@ -34,6 +34,7 @@ gem 'rswag-api'
 gem 'rswag-ui'
 gem 'sentry-ruby'
 gem 'sentry-rails'
+gem 'stackprof'
 gem 'sidekiq'
 gem 'sidekiq-cron'
 gem 'sidekiq-limit_fetch'

Gemfile.lock

@@ -423,6 +423,7 @@ GEM
       actionpack (>= 6.1)
       activesupport (>= 6.1)
       sprockets (>= 3.0.0)
+    stackprof (0.2.27)
     stimulus-rails (1.3.4)
       railties (>= 6.0.0)
     stringio (3.1.7)
@@ -525,6 +526,7 @@ DEPENDENCIES
   sidekiq-limit_fetch
   simplecov
   sprockets-rails
+  stackprof
   stimulus-rails
   strong_migrations
   super_diff

app/jobs/owntracks/point_creating_job.rb

@@ -8,6 +8,7 @@ class Owntracks::PointCreatingJob < ApplicationJob
   def perform(point_params, user_id)
     parsed_params = OwnTracks::Params.new(point_params).call
 
+    return if parsed_params[:timestamp].nil? || parsed_params[:lonlat].nil?
     return if point_exists?(parsed_params, user_id)
 
     Point.create!(parsed_params.merge(user_id:))

app/services/points/create.rb

@@ -11,9 +11,12 @@ class Points::Create
   def call
     data = Points::Params.new(params, user.id).call
 
+    # Deduplicate points based on unique constraint
+    deduplicated_data = data.uniq { |point| [point[:lonlat], point[:timestamp], point[:user_id]] }
+
     created_points = []
 
-    data.each_slice(1000) do |location_batch|
+    deduplicated_data.each_slice(1000) do |location_batch|
       # rubocop:disable Rails/SkipsModelValidations
       result = Point.upsert_all(
         location_batch,

app/views/home/index.html.erb

@@ -8,7 +8,8 @@
           </h1>
           <p class="py-6 text-3xl">The only location history tracker you'll ever need.</p>
 
-          <%#= link_to 'Sign up', new_user_registration_path, class: "rounded-lg py-3 px-5 my-3 bg-blue-600 text-white block font-medium" %>
+          <%= link_to 'Sign up', new_user_registration_path, class: "rounded-lg py-3 px-5 my-3 bg-blue-600 text-white block font-medium" %>
+          <div class="divider">or</div>
           <%= link_to 'Sign in', new_user_session_path, class: "rounded-lg py-3 px-5 bg-neutral text-neutral-content block font-medium" %>
         </div>
       </div>

app/views/imports/index.html.erb

@@ -19,7 +19,7 @@
 
   <div id="imports" class="min-w-full">
     <% if @imports.empty? %>
-      <div class="hero min-h-80 bg-base-200">
+      <div class="hero min-h-80 bg-base-200 my-5">
         <div class="hero-content text-center">
           <div class="max-w-md">
             <h1 class="text-5xl font-bold">Hello there!</h1>

config/initializers/sentry.rb

@@ -6,4 +6,5 @@ Sentry.init do |config|
   config.breadcrumbs_logger = [:active_support_logger]
   config.dsn = SENTRY_DSN
   config.traces_sample_rate = 1.0
+  config.profiles_sample_rate = 1.0
 end

spec/requests/home_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe 'Homes', type: :request do
         .to_return(status: 200, body: '[{"name": "1.0.0"}]', headers: {})
     end
 
-    it 'returns http success' do
+    xit 'returns http success' do
       get '/'
 
       expect(response).to have_http_status(:success)

spec/services/points/create_spec.rb

@@ -23,15 +23,15 @@ RSpec.describe Points::Create do
           lonlat: 'POINT(-0.1278 51.5074)',
           timestamp: timestamp,
           user_id: user.id,
-          created_at: anything,
-          updated_at: anything
+          created_at: Time.current,
+          updated_at: Time.current
         },
         {
           lonlat: 'POINT(-74.006 40.7128)',
           timestamp: timestamp + 1.hour,
           user_id: user.id,
-          created_at: anything,
-          updated_at: anything
+          created_at: Time.current,
+          updated_at: Time.current
         }
       ]
     end
@@ -43,21 +43,168 @@ RSpec.describe Points::Create do
       ]
     end
 
-    it 'processes the points and upserts them to the database' do
-      expect(Points::Params).to receive(:new).with(point_params, user.id).and_return(params_service)
-      expect(params_service).to receive(:call).and_return(processed_data)
+    describe 'basic point creation' do
+      before do
+        allow(Points::Params).to receive(:new).with(point_params, user.id).and_return(params_service)
+        allow(params_service).to receive(:call).and_return(processed_data)
+      end
+
+      it 'initializes the params service with correct arguments' do
+        expect(Points::Params).to receive(:new).with(point_params, user.id)
+        described_class.new(user, point_params).call
+      end
+
+      it 'calls the params service' do
+        expect(params_service).to receive(:call)
+        described_class.new(user, point_params).call
+      end
+
+      it 'upserts the processed data' do
         expect(Point).to receive(:upsert_all)
           .with(
             processed_data,
             unique_by: %i[lonlat timestamp user_id],
-          returning: Arel.sql('id, timestamp, ST_X(lonlat::geometry) AS longitude, ST_Y(lonlat::geometry) AS latitude')
+            returning: Arel.sql(
+              'id, timestamp, ST_X(lonlat::geometry) AS longitude, ST_Y(lonlat::geometry) AS latitude'
+            )
           )
           .and_return(upsert_result)
 
-      result = described_class.new(user, point_params).call
+        described_class.new(user, point_params).call
+      end
 
+      it 'returns the upsert result' do
+        allow(Point).to receive(:upsert_all).and_return(upsert_result)
+        result = described_class.new(user, point_params).call
         expect(result).to eq(upsert_result)
       end
+    end
+
+    context 'with duplicate points' do
+      let(:duplicate_point_params) do
+        {
+          locations: [
+            { lat: 51.5074, lon: -0.1278, timestamp: timestamp.iso8601 },
+            { lat: 51.5074, lon: -0.1278, timestamp: timestamp.iso8601 }, # Duplicate
+            { lat: 40.7128, lon: -74.0060, timestamp: (timestamp + 1.hour).iso8601 }
+          ]
+        }
+      end
+
+      let(:duplicate_processed_data) do
+        current_time = Time.current
+        [
+          {
+            lonlat: 'POINT(-0.1278 51.5074)',
+            timestamp: timestamp,
+            user_id: user.id,
+            created_at: current_time,
+            updated_at: current_time
+          },
+          {
+            lonlat: 'POINT(-0.1278 51.5074)', # Duplicate
+            timestamp: timestamp,
+            user_id: user.id,
+            created_at: current_time,
+            updated_at: current_time
+          },
+          {
+            lonlat: 'POINT(-74.006 40.7128)',
+            timestamp: timestamp + 1.hour,
+            user_id: user.id,
+            created_at: current_time,
+            updated_at: current_time
+          }
+        ]
+      end
+
+      let(:deduplicated_upsert_result) do
+        [
+          Point.new(id: 1, lonlat: 'POINT(-0.1278 51.5074)', timestamp: timestamp),
+          Point.new(id: 2, lonlat: 'POINT(-74.006 40.7128)', timestamp: timestamp + 1.hour)
+        ]
+      end
+
+      before do
+        allow_any_instance_of(Points::Params).to receive(:call).and_return(duplicate_processed_data)
+      end
+
+      describe 'deduplication behavior' do
+        it 'reduces the number of points to unique combinations' do
+          expect(Point).to receive(:upsert_all) do |data, _options|
+            expect(data.size).to eq(2)
+            deduplicated_upsert_result
+          end
+
+          described_class.new(user, duplicate_point_params).call
+        end
+
+        it 'preserves the correct lonlat values' do
+          expect(Point).to receive(:upsert_all) do |data, _options|
+            expect(data.map { |d| d[:lonlat] }).to match_array(['POINT(-0.1278 51.5074)', 'POINT(-74.006 40.7128)'])
+            deduplicated_upsert_result
+          end
+
+          described_class.new(user, duplicate_point_params).call
+        end
+
+        it 'preserves the correct timestamps' do
+          expect(Point).to receive(:upsert_all) do |data, _options|
+            expect(data.map { |d| d[:timestamp] }).to match_array([timestamp, timestamp + 1.hour])
+            deduplicated_upsert_result
+          end
+
+          described_class.new(user, duplicate_point_params).call
+        end
+
+        it 'maintains the correct user_id for all points' do
+          expect(Point).to receive(:upsert_all) do |data, _options|
+            expect(data.map { |d| d[:user_id] }).to all(eq(user.id))
+            deduplicated_upsert_result
+          end
+
+          described_class.new(user, duplicate_point_params).call
+        end
+
+        it 'uses the correct unique constraint' do
+          expect(Point).to receive(:upsert_all) do |_data, options|
+            expect(options[:unique_by]).to eq(%i[lonlat timestamp user_id])
+            deduplicated_upsert_result
+          end
+
+          described_class.new(user, duplicate_point_params).call
+        end
+
+        it 'uses the correct returning clause' do
+          expect(Point).to receive(:upsert_all) do |_data, options|
+            expect(options[:returning]).to eq(
+              Arel.sql('id, timestamp, ST_X(lonlat::geometry) AS longitude, ST_Y(lonlat::geometry) AS latitude')
+            )
+            deduplicated_upsert_result
+          end
+
+          described_class.new(user, duplicate_point_params).call
+        end
+      end
+
+      describe 'database interaction' do
+        it 'creates only unique points' do
+          expect do
+            described_class.new(user, duplicate_point_params).call
+          end.to change(Point, :count).by(2)
+        end
+
+        it 'creates points with correct coordinates' do
+          described_class.new(user, duplicate_point_params).call
+          points = Point.order(:timestamp).last(2)
+
+          expect(points[0].lonlat.x).to be_within(0.0001).of(-0.1278)
+          expect(points[0].lonlat.y).to be_within(0.0001).of(51.5074)
+          expect(points[1].lonlat.x).to be_within(0.0001).of(-74.006)
+          expect(points[1].lonlat.y).to be_within(0.0001).of(40.7128)
+        end
+      end
+    end
 
     context 'with large datasets' do
       let(:many_locations) do