Skip to content

Commit c0fb411

Browse files
authored
Merge pull request #1174 from Freika/fix/small-fixes
Skip points without lonlat and timestamp from Owntracks
2 parents e2cc8d2 + f571d1e commit c0fb411

File tree

10 files changed

+180
-22
lines changed

10 files changed

+180
-22
lines changed

CHANGELOG.md

+4-2
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@ All notable changes to this project will be documented in this file.
44
The format is based on [Keep a Changelog](http://keepachangelog.com/)
55
and this project adheres to [Semantic Versioning](http://semver.org/).
66

7-
# 0.26.1 - UNRELEASED
7+
8+
# 0.26.1 - 2025-05-12
89

910
## Fixed
1011

12+
- Fixed a bug with an attempt to write points with same lonlat and timestamp from iOS app. #1170
1113
- Importing GeoJSON files now saves velocity if it was stored in either `velocity` or `speed` property.
1214

15+
1316
# 0.26.0 - 2025-05-08
1417

1518
⚠️ This release includes a breaking change. ⚠️
@@ -25,7 +28,6 @@ If you have encountered problems with moving to a PostGIS image while still on P
2528
- Dawarich now uses PostgreSQL 17 with PostGIS 3.5 by default.
2629

2730

28-
2931
# 0.25.10 - 2025-05-08
3032

3133
## Added

Gemfile

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ gem 'rswag-api'
3434
gem 'rswag-ui'
3535
gem 'sentry-ruby'
3636
gem 'sentry-rails'
37+
gem 'stackprof'
3738
gem 'sidekiq'
3839
gem 'sidekiq-cron'
3940
gem 'sidekiq-limit_fetch'

Gemfile.lock

+2
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,7 @@ GEM
423423
actionpack (>= 6.1)
424424
activesupport (>= 6.1)
425425
sprockets (>= 3.0.0)
426+
stackprof (0.2.27)
426427
stimulus-rails (1.3.4)
427428
railties (>= 6.0.0)
428429
stringio (3.1.7)
@@ -525,6 +526,7 @@ DEPENDENCIES
525526
sidekiq-limit_fetch
526527
simplecov
527528
sprockets-rails
529+
stackprof
528530
stimulus-rails
529531
strong_migrations
530532
super_diff

app/jobs/owntracks/point_creating_job.rb

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ class Owntracks::PointCreatingJob < ApplicationJob
88
def perform(point_params, user_id)
99
parsed_params = OwnTracks::Params.new(point_params).call
1010

11+
return if parsed_params[:timestamp].nil? || parsed_params[:lonlat].nil?
1112
return if point_exists?(parsed_params, user_id)
1213

1314
Point.create!(parsed_params.merge(user_id:))

app/services/points/create.rb

+4-1
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@ def initialize(user, params)
1111
def call
1212
data = Points::Params.new(params, user.id).call
1313

14+
# Deduplicate points based on unique constraint
15+
deduplicated_data = data.uniq { |point| [point[:lonlat], point[:timestamp], point[:user_id]] }
16+
1417
created_points = []
1518

16-
data.each_slice(1000) do |location_batch|
19+
deduplicated_data.each_slice(1000) do |location_batch|
1720
# rubocop:disable Rails/SkipsModelValidations
1821
result = Point.upsert_all(
1922
location_batch,

app/views/home/index.html.erb

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
</h1>
99
<p class="py-6 text-3xl">The only location history tracker you'll ever need.</p>
1010

11-
<%#= 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" %>
11+
<%= 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" %>
12+
<div class="divider">or</div>
1213
<%= link_to 'Sign in', new_user_session_path, class: "rounded-lg py-3 px-5 bg-neutral text-neutral-content block font-medium" %>
1314
</div>
1415
</div>

app/views/imports/index.html.erb

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

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

config/initializers/sentry.rb

+1
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,5 @@
66
config.breadcrumbs_logger = [:active_support_logger]
77
config.dsn = SENTRY_DSN
88
config.traces_sample_rate = 1.0
9+
config.profiles_sample_rate = 1.0
910
end

spec/requests/home_spec.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
.to_return(status: 200, body: '[{"name": "1.0.0"}]', headers: {})
1010
end
1111

12-
it 'returns http success' do
12+
xit 'returns http success' do
1313
get '/'
1414

1515
expect(response).to have_http_status(:success)

spec/services/points/create_spec.rb

+163-16
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,15 @@
2323
lonlat: 'POINT(-0.1278 51.5074)',
2424
timestamp: timestamp,
2525
user_id: user.id,
26-
created_at: anything,
27-
updated_at: anything
26+
created_at: Time.current,
27+
updated_at: Time.current
2828
},
2929
{
3030
lonlat: 'POINT(-74.006 40.7128)',
3131
timestamp: timestamp + 1.hour,
3232
user_id: user.id,
33-
created_at: anything,
34-
updated_at: anything
33+
created_at: Time.current,
34+
updated_at: Time.current
3535
}
3636
]
3737
end
@@ -43,20 +43,167 @@
4343
]
4444
end
4545

46-
it 'processes the points and upserts them to the database' do
47-
expect(Points::Params).to receive(:new).with(point_params, user.id).and_return(params_service)
48-
expect(params_service).to receive(:call).and_return(processed_data)
49-
expect(Point).to receive(:upsert_all)
50-
.with(
51-
processed_data,
52-
unique_by: %i[lonlat timestamp user_id],
53-
returning: Arel.sql('id, timestamp, ST_X(lonlat::geometry) AS longitude, ST_Y(lonlat::geometry) AS latitude')
54-
)
55-
.and_return(upsert_result)
46+
describe 'basic point creation' do
47+
before do
48+
allow(Points::Params).to receive(:new).with(point_params, user.id).and_return(params_service)
49+
allow(params_service).to receive(:call).and_return(processed_data)
50+
end
51+
52+
it 'initializes the params service with correct arguments' do
53+
expect(Points::Params).to receive(:new).with(point_params, user.id)
54+
described_class.new(user, point_params).call
55+
end
56+
57+
it 'calls the params service' do
58+
expect(params_service).to receive(:call)
59+
described_class.new(user, point_params).call
60+
end
5661

57-
result = described_class.new(user, point_params).call
62+
it 'upserts the processed data' do
63+
expect(Point).to receive(:upsert_all)
64+
.with(
65+
processed_data,
66+
unique_by: %i[lonlat timestamp user_id],
67+
returning: Arel.sql(
68+
'id, timestamp, ST_X(lonlat::geometry) AS longitude, ST_Y(lonlat::geometry) AS latitude'
69+
)
70+
)
71+
.and_return(upsert_result)
5872

59-
expect(result).to eq(upsert_result)
73+
described_class.new(user, point_params).call
74+
end
75+
76+
it 'returns the upsert result' do
77+
allow(Point).to receive(:upsert_all).and_return(upsert_result)
78+
result = described_class.new(user, point_params).call
79+
expect(result).to eq(upsert_result)
80+
end
81+
end
82+
83+
context 'with duplicate points' do
84+
let(:duplicate_point_params) do
85+
{
86+
locations: [
87+
{ lat: 51.5074, lon: -0.1278, timestamp: timestamp.iso8601 },
88+
{ lat: 51.5074, lon: -0.1278, timestamp: timestamp.iso8601 }, # Duplicate
89+
{ lat: 40.7128, lon: -74.0060, timestamp: (timestamp + 1.hour).iso8601 }
90+
]
91+
}
92+
end
93+
94+
let(:duplicate_processed_data) do
95+
current_time = Time.current
96+
[
97+
{
98+
lonlat: 'POINT(-0.1278 51.5074)',
99+
timestamp: timestamp,
100+
user_id: user.id,
101+
created_at: current_time,
102+
updated_at: current_time
103+
},
104+
{
105+
lonlat: 'POINT(-0.1278 51.5074)', # Duplicate
106+
timestamp: timestamp,
107+
user_id: user.id,
108+
created_at: current_time,
109+
updated_at: current_time
110+
},
111+
{
112+
lonlat: 'POINT(-74.006 40.7128)',
113+
timestamp: timestamp + 1.hour,
114+
user_id: user.id,
115+
created_at: current_time,
116+
updated_at: current_time
117+
}
118+
]
119+
end
120+
121+
let(:deduplicated_upsert_result) do
122+
[
123+
Point.new(id: 1, lonlat: 'POINT(-0.1278 51.5074)', timestamp: timestamp),
124+
Point.new(id: 2, lonlat: 'POINT(-74.006 40.7128)', timestamp: timestamp + 1.hour)
125+
]
126+
end
127+
128+
before do
129+
allow_any_instance_of(Points::Params).to receive(:call).and_return(duplicate_processed_data)
130+
end
131+
132+
describe 'deduplication behavior' do
133+
it 'reduces the number of points to unique combinations' do
134+
expect(Point).to receive(:upsert_all) do |data, _options|
135+
expect(data.size).to eq(2)
136+
deduplicated_upsert_result
137+
end
138+
139+
described_class.new(user, duplicate_point_params).call
140+
end
141+
142+
it 'preserves the correct lonlat values' do
143+
expect(Point).to receive(:upsert_all) do |data, _options|
144+
expect(data.map { |d| d[:lonlat] }).to match_array(['POINT(-0.1278 51.5074)', 'POINT(-74.006 40.7128)'])
145+
deduplicated_upsert_result
146+
end
147+
148+
described_class.new(user, duplicate_point_params).call
149+
end
150+
151+
it 'preserves the correct timestamps' do
152+
expect(Point).to receive(:upsert_all) do |data, _options|
153+
expect(data.map { |d| d[:timestamp] }).to match_array([timestamp, timestamp + 1.hour])
154+
deduplicated_upsert_result
155+
end
156+
157+
described_class.new(user, duplicate_point_params).call
158+
end
159+
160+
it 'maintains the correct user_id for all points' do
161+
expect(Point).to receive(:upsert_all) do |data, _options|
162+
expect(data.map { |d| d[:user_id] }).to all(eq(user.id))
163+
deduplicated_upsert_result
164+
end
165+
166+
described_class.new(user, duplicate_point_params).call
167+
end
168+
169+
it 'uses the correct unique constraint' do
170+
expect(Point).to receive(:upsert_all) do |_data, options|
171+
expect(options[:unique_by]).to eq(%i[lonlat timestamp user_id])
172+
deduplicated_upsert_result
173+
end
174+
175+
described_class.new(user, duplicate_point_params).call
176+
end
177+
178+
it 'uses the correct returning clause' do
179+
expect(Point).to receive(:upsert_all) do |_data, options|
180+
expect(options[:returning]).to eq(
181+
Arel.sql('id, timestamp, ST_X(lonlat::geometry) AS longitude, ST_Y(lonlat::geometry) AS latitude')
182+
)
183+
deduplicated_upsert_result
184+
end
185+
186+
described_class.new(user, duplicate_point_params).call
187+
end
188+
end
189+
190+
describe 'database interaction' do
191+
it 'creates only unique points' do
192+
expect do
193+
described_class.new(user, duplicate_point_params).call
194+
end.to change(Point, :count).by(2)
195+
end
196+
197+
it 'creates points with correct coordinates' do
198+
described_class.new(user, duplicate_point_params).call
199+
points = Point.order(:timestamp).last(2)
200+
201+
expect(points[0].lonlat.x).to be_within(0.0001).of(-0.1278)
202+
expect(points[0].lonlat.y).to be_within(0.0001).of(51.5074)
203+
expect(points[1].lonlat.x).to be_within(0.0001).of(-74.006)
204+
expect(points[1].lonlat.y).to be_within(0.0001).of(40.7128)
205+
end
206+
end
60207
end
61208

62209
context 'with large datasets' do

0 commit comments

Comments
 (0)