github/dawarich
1
1
Fork 0
mirror of https://github.com/Freika/dawarich.git synced 2026-01-10 14:58:07 -05:00
Code Issues Packages Projects Releases Wiki Activity

Remove provider detection

Browse Source
This commit is contained in:
Eugene Burmakin
2025-09-03 19:28:26 +02:00
parent 7ca488802e
commit b276828af3
6 changed files with 108 additions and 289 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/controllers/api/v1/locations_controller.rb
View File

@@ -19,7 +19,7 @@ class Api::V1::LocationsController < ApiController
def suggestions
if search_query.present? && search_query.length >= 2
suggestions = LocationSearch::GeocodingService.new.search(search_query)
suggestions = LocationSearch::GeocodingService.new(search_query).search
# Format suggestions for the frontend
formatted_suggestions = suggestions.map do |suggestion|

153
app/services/location_search/geocoding_service.rb
View File

@@ -5,11 +5,12 @@ module LocationSearch
MAX_RESULTS = 10
CACHE_TTL = 1.hour
def initialize
def initialize(query)
@query = query
@cache_key_prefix = 'location_search:geocoding'
end
def search(query)
def search
return [] if query.blank?
cache_key = "#{@cache_key_prefix}:#{Digest::SHA256.hexdigest(query.downcase)}"
@@ -28,60 +29,38 @@ module LocationSearch
private
attr_reader :query
def perform_geocoding_search(query)
Rails.logger.info "LocationSearch::GeocodingService: Searching for '#{query}' using #{provider_name}"
# Try original query first
results = Geocoder.search(query, limit: MAX_RESULTS)
Rails.logger.info "LocationSearch::GeocodingService: Raw geocoder returned #{results.length} results"
# If we got results but they seem too generic (common chain names),
# also try with location context
if results.length > 1 && looks_like_chain_store?(query)
Rails.logger.info 'LocationSearch::GeocodingService: Query looks like chain store, trying with Berlin context'
berlin_results = Geocoder.search("#{query} Berlin", limit: MAX_RESULTS)
Rails.logger.info "LocationSearch::GeocodingService: Berlin-specific search returned #{berlin_results.length} results"
# Prioritize Berlin results
results = (berlin_results + results).uniq
end
return [] if results.blank?
normalized = normalize_geocoding_results(results)
Rails.logger.info "LocationSearch::GeocodingService: After normalization: #{normalized.length} results"
normalized
normalize_geocoding_results(results)
end
def normalize_geocoding_results(results)
normalized_results = []
results.each_with_index do |result, idx|
unless valid_result?(result)
Rails.logger.warn "LocationSearch::GeocodingService: Result #{idx} is invalid: lat=#{result.latitude}, lon=#{result.longitude}"
next
end
results.each do |result|
next unless valid_result?(result)
normalized_result = {
lat: result.latitude.to_f,
lon: result.longitude.to_f,
name: extract_name(result),
address: extract_address(result),
type: extract_type(result),
provider_data: extract_provider_data(result)
name: result.address&.split(',')&.first || 'Unknown location',
address: result.address || '',
type: result.data&.dig('type') || result.data&.dig('class') || 'unknown',
provider_data: {
osm_id: result.data&.dig('osm_id'),
place_rank: result.data&.dig('place_rank'),
importance: result.data&.dig('importance')
}
}
Rails.logger.info "LocationSearch::GeocodingService: Result #{idx}: '#{normalized_result[:name]}' at [#{normalized_result[:lat]}, #{normalized_result[:lon]}]"
normalized_results << normalized_result
end
# Remove duplicates based on coordinates (within 100m)
deduplicated = deduplicate_results(normalized_results)
Rails.logger.info "LocationSearch::GeocodingService: After deduplication: #{deduplicated.length} results"
deduplicated
deduplicate_results(normalized_results)
end
def valid_result?(result)
@@ -92,93 +71,6 @@ module LocationSearch
result.longitude.to_f.abs <= 180
end
def extract_name(result)
case provider_name.downcase
when 'photon'
extract_photon_name(result)
when 'nominatim'
extract_nominatim_name(result)
when 'geoapify'
extract_geoapify_name(result)
else
result.address || result.data&.dig('display_name') || 'Unknown location'
end
end
def extract_address(result)
case provider_name.downcase
when 'photon'
extract_photon_address(result)
when 'nominatim'
extract_nominatim_address(result)
when 'geoapify'
extract_geoapify_address(result)
else
result.address || result.data&.dig('display_name') || ''
end
end
def extract_type(result)
data = result.data || {}
case provider_name.downcase
when 'photon'
data.dig('properties', 'osm_key') || data.dig('properties', 'type') || 'unknown'
when 'nominatim'
data['type'] || data['class'] || 'unknown'
when 'geoapify'
data.dig('properties', 'datasource', 'sourcename') || data.dig('properties', 'place_type') || 'unknown'
else
'unknown'
end
end
def extract_provider_data(result)
{
osm_id: result.data&.dig('properties', 'osm_id'),
osm_type: result.data&.dig('properties', 'osm_type'),
place_rank: result.data&.dig('place_rank'),
importance: result.data&.dig('importance')
}
end
# Provider-specific extractors
def extract_photon_name(result)
properties = result.data&.dig('properties') || {}
properties['name'] || properties['street'] || properties['city'] || 'Unknown location'
end
def extract_photon_address(result)
properties = result.data&.dig('properties') || {}
parts = []
parts << properties['street'] if properties['street'].present?
parts << properties['housenumber'] if properties['housenumber'].present?
parts << properties['city'] if properties['city'].present?
parts << properties['state'] if properties['state'].present?
parts << properties['country'] if properties['country'].present?
parts.join(', ')
end
def extract_nominatim_name(result)
data = result.data || {}
data['display_name']&.split(',')&.first || 'Unknown location'
end
def extract_nominatim_address(result)
result.data&.dig('display_name') || ''
end
def extract_geoapify_name(result)
properties = result.data&.dig('properties') || {}
properties['name'] || properties['street'] || properties['city'] || 'Unknown location'
end
def extract_geoapify_address(result)
properties = result.data&.dig('properties') || {}
properties['formatted'] || ''
end
def deduplicate_results(results)
deduplicated = []
@@ -216,16 +108,5 @@ module LocationSearch
rkm * c
end
def looks_like_chain_store?(query)
chain_patterns = [
/\b(netto|kaufland|rewe|edeka|aldi|lidl|penny|real)\b/i,
/\b(mcdonalds?|burger king|kfc|subway)\b/i,
/\b(shell|aral|esso|bp|total)\b/i,
/\b(dm|rossmann|müller)\b/i,
/\b(h&m|c&a|zara|primark)\b/i
]
chain_patterns.any? { |pattern| query.match?(pattern) }
end
end
end

4
app/services/location_search/point_finder.rb
View File

@@ -49,7 +49,7 @@ module LocationSearch
def text_based_search
return empty_result if @query.blank?
geocoded_locations = geocoding_service.search(@query)
geocoded_locations = LocationSearch::GeocodingService.new(@query).search
# Debug: Log geocoding results
Rails.logger.info "LocationSearch: Geocoding '#{@query}' returned #{geocoded_locations.length} locations"
@@ -63,7 +63,7 @@ module LocationSearch
end
def geocoding_service
@geocoding_service ||= LocationSearch::GeocodingService.new
@geocoding_service ||= LocationSearch::GeocodingService.new(@query || '')
end
def find_matching_points(geocoded_locations)

197
spec/services/location_search/geocoding_service_spec.rb
View File

@@ -3,11 +3,11 @@
require 'rails_helper'
RSpec.describe LocationSearch::GeocodingService do
let(:service) { described_class.new }
let(:query) { 'Kaufland Berlin' }
let(:service) { described_class.new(query) }
describe '#search' do
context 'with valid query' do
let(:query) { 'Kaufland Berlin' }
let(:mock_geocoder_result) do
double(
'Geocoder::Result',
@@ -15,15 +15,10 @@ RSpec.describe LocationSearch::GeocodingService do
longitude: 13.4050,
address: 'Kaufland, Alexanderplatz 1, Berlin',
data: {
'properties' => {
'name' => 'Kaufland Mitte',
'street' => 'Alexanderplatz',
'housenumber' => '1',
'city' => 'Berlin',
'country' => 'Germany',
'osm_key' => 'shop',
'osm_id' => '12345'
}
'type' => 'shop',
'osm_id' => '12345',
'place_rank' => 30,
'importance' => 0.8
}
)
end
@@ -34,37 +29,38 @@ RSpec.describe LocationSearch::GeocodingService do
end
it 'returns normalized geocoding results' do
results = service.search(query)
results = service.search
expect(results).to be_an(Array)
expect(results.first).to include(
lat: 52.5200,
lon: 13.4050,
name: 'Kaufland Mitte',
address: 'Alexanderplatz, 1, Berlin, Germany',
name: 'Kaufland',
address: 'Kaufland, Alexanderplatz 1, Berlin',
type: 'shop'
)
end
it 'includes provider data' do
results = service.search(query)
results = service.search
expect(results.first[:provider_data]).to include(
osm_id: '12345',
osm_type: nil
place_rank: 30,
importance: 0.8
)
end
it 'caches results' do
expect(Rails.cache).to receive(:fetch).and_call_original
service.search(query)
service.search
end
it 'limits results to MAX_RESULTS' do
expect(Geocoder).to receive(:search).with(query, limit: 10)
service.search(query)
service.search
end
context 'with cached results' do
@@ -77,19 +73,17 @@ RSpec.describe LocationSearch::GeocodingService do
it 'returns cached results without calling Geocoder' do
expect(Geocoder).not_to receive(:search)
results = service.search(query)
results = service.search
expect(results).to eq(cached_results)
end
end
end
context 'with blank query' do
let(:service) { described_class.new('') }
it 'returns empty array' do
results = service.search('')
expect(results).to eq([])
results = service.search(nil)
expect(results).to eq([])
expect(service.search).to eq([])
end
end
@@ -99,21 +93,17 @@ RSpec.describe LocationSearch::GeocodingService do
end
it 'returns empty array' do
results = service.search('nonexistent place')
expect(results).to eq([])
expect(service.search).to eq([])
end
end
context 'when Geocoder raises an error' do
before do
allow(Geocoder).to receive(:search).and_raise(StandardError.new('API error'))
allow(Geocoder).to receive(:search).and_raise(StandardError.new('Geocoding error'))
end
it 'handles error gracefully and returns empty array' do
expect(Rails.logger).to receive(:error).with(/Geocoding search failed/)
results = service.search('test query')
expect(results).to eq([])
expect(service.search).to eq([])
end
end
@@ -121,20 +111,32 @@ RSpec.describe LocationSearch::GeocodingService do
let(:invalid_result) do
double(
'Geocoder::Result',
latitude: 91.0, # Invalid latitude
longitude: 181.0, # Invalid longitude
address: 'Invalid Location',
latitude: 91.0, # Invalid latitude
longitude: 13.4050,
address: 'Invalid location',
data: {}
)
end
let(:valid_result) do
double(
'Geocoder::Result',
latitude: 52.5200,
longitude: 13.4050,
address: 'Valid location',
data: {}
)
end
before do
allow(Geocoder).to receive(:search).and_return([invalid_result])
allow(Geocoder).to receive(:search).and_return([invalid_result, valid_result])
end
it 'filters out results with invalid coordinates' do
results = service.search('test')
expect(results).to be_empty
results = service.search
expect(results.length).to eq(1)
expect(results.first[:lat]).to eq(52.5200)
end
end
@@ -144,124 +146,55 @@ RSpec.describe LocationSearch::GeocodingService do
{
lat: 52.5200,
lon: 13.4050,
name: 'Location A',
address: 'Address A',
type: 'shop'
name: 'Location 1',
address: 'Address 1',
type: 'shop',
provider_data: {}
},
{
lat: 52.5201, # Very close to first location (~11 meters)
lat: 52.5201, # Within 100m of first location
lon: 13.4051,
name: 'Location B',
address: 'Address B',
type: 'shop'
},
{
lat: 52.5300, # Far from others
lon: 13.4150,
name: 'Location C',
address: 'Address C',
type: 'restaurant'
name: 'Location 2',
address: 'Address 2',
type: 'shop',
provider_data: {}
}
]
end
before do
# Create mock geocoder results that will be normalized and deduplicated
mock_geocoder_results = duplicate_results.map do |result|
let(:mock_results) do
duplicate_results.map do |result|
double(
'Geocoder::Result',
latitude: result[:lat],
longitude: result[:lon],
address: result[:address],
data: {
'display_name' => result[:name],
'type' => result[:type],
'properties' => {
'name' => result[:name],
'osm_key' => result[:type]
}
}
data: { 'type' => result[:type] }
)
end
allow(Geocoder).to receive(:search).and_return(mock_geocoder_results)
end
before do
allow(Geocoder).to receive(:search).and_return(mock_results)
end
it 'removes locations within 100m of each other' do
results = service.search('test')
service = described_class.new('test')
results = service.search
expect(results.length).to eq(2)
expect(results.map { |r| r[:name] }).to include('Location A', 'Location C')
expect(results.length).to eq(1)
expect(results.first[:name]).to eq('Address 1')
end
end
end
describe '#provider_name' do
before do
allow(Geocoder.config).to receive(:lookup).and_return(:nominatim)
end
it 'returns the current geocoding provider name' do
allow(Geocoder.config).to receive(:lookup).and_return(:photon)
expect(service.provider_name).to eq('Photon')
end
end
describe 'provider-specific extraction' do
context 'with Photon provider' do
let(:photon_result) do
double(
'Geocoder::Result',
latitude: 52.5200,
longitude: 13.4050,
data: {
'properties' => {
'name' => 'Kaufland',
'street' => 'Alexanderplatz',
'housenumber' => '1',
'city' => 'Berlin',
'state' => 'Berlin',
'country' => 'Germany'
}
}
)
end
before do
allow(Geocoder).to receive(:search).and_return([photon_result])
allow(Geocoder.config).to receive(:lookup).and_return(:photon)
end
it 'extracts Photon-specific data correctly' do
results = service.search('test')
expect(results.first[:name]).to eq('Kaufland')
expect(results.first[:address]).to eq('Alexanderplatz, 1, Berlin, Berlin, Germany')
end
end
context 'with Nominatim provider' do
let(:nominatim_result) do
double(
'Geocoder::Result',
latitude: 52.5200,
longitude: 13.4050,
data: {
'display_name' => 'Kaufland, Alexanderplatz 1, Berlin, Germany',
'type' => 'shop',
'class' => 'amenity'
}
)
end
before do
allow(Geocoder).to receive(:search).and_return([nominatim_result])
allow(Geocoder.config).to receive(:lookup).and_return(:nominatim)
end
it 'extracts Nominatim-specific data correctly' do
results = service.search('test')
expect(results.first[:name]).to eq('Kaufland')
expect(results.first[:address]).to eq('Kaufland, Alexanderplatz 1, Berlin, Germany')
expect(results.first[:type]).to eq('shop')
end
expect(service.provider_name).to eq('Nominatim')
end
end
end

25
spec/services/location_search/point_finder_spec.rb
View File

@@ -47,8 +47,9 @@ RSpec.describe LocationSearch::PointFinder do
end
before do
allow_any_instance_of(LocationSearch::GeocodingService)
.to receive(:search).and_return(mock_geocoded_locations)
allow(LocationSearch::GeocodingService).to receive(:new).and_return(
double('GeocodingService', search: mock_geocoded_locations, provider_name: 'Test Provider')
)
allow_any_instance_of(LocationSearch::SpatialMatcher)
.to receive(:find_points_near).and_return(mock_matching_points)
@@ -82,8 +83,9 @@ RSpec.describe LocationSearch::PointFinder do
end
it 'calls geocoding service with the query' do
expect_any_instance_of(LocationSearch::GeocodingService)
.to receive(:search).with('Kaufland')
expect(LocationSearch::GeocodingService)
.to receive(:new).with('Kaufland')
.and_return(double('GeocodingService', search: mock_geocoded_locations, provider_name: 'Test Provider'))
service.call
end
@@ -160,8 +162,9 @@ RSpec.describe LocationSearch::PointFinder do
context 'when no geocoding results found' do
before do
allow_any_instance_of(LocationSearch::GeocodingService)
.to receive(:search).and_return([])
allow(LocationSearch::GeocodingService).to receive(:new).and_return(
double('GeocodingService', search: [], provider_name: 'Test Provider')
)
end
it 'returns empty result' do
@@ -174,8 +177,9 @@ RSpec.describe LocationSearch::PointFinder do
context 'when no matching points found' do
before do
allow_any_instance_of(LocationSearch::GeocodingService)
.to receive(:search).and_return([{ lat: 52.5200, lon: 13.4050, name: 'Test' }])
allow(LocationSearch::GeocodingService).to receive(:new).and_return(
double('GeocodingService', search: [{ lat: 52.5200, lon: 13.4050, name: 'Test' }], provider_name: 'Test Provider')
)
allow_any_instance_of(LocationSearch::SpatialMatcher)
.to receive(:find_points_near).and_return([])
@@ -206,8 +210,9 @@ RSpec.describe LocationSearch::PointFinder do
let(:many_visits) { Array.new(15) { |i| { timestamp: i, date: "2024-01-#{i+1}T12:00:00Z" } } }
before do
allow_any_instance_of(LocationSearch::GeocodingService)
.to receive(:search).and_return([{ lat: 52.5200, lon: 13.4050, name: 'Test' }])
allow(LocationSearch::GeocodingService).to receive(:new).and_return(
double('GeocodingService', search: [{ lat: 52.5200, lon: 13.4050, name: 'Test' }], provider_name: 'Test Provider')
)
allow_any_instance_of(LocationSearch::SpatialMatcher)
.to receive(:find_points_near).and_return([{}])

16
spec/services/points/raw_data_lonlat_extractor_spec.rb
View File

@@ -18,7 +18,7 @@ RSpec.describe Points::RawDataLonlatExtractor do
}
}
end
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: nil, latitude: nil) }
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: 0.0, latitude: 0.0) }
it 'extracts longitude and latitude correctly' do
expect { described_class.new(point).call }.to \
@@ -36,7 +36,7 @@ RSpec.describe Points::RawDataLonlatExtractor do
'latitudeE7' => 512_345_678
}
end
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: nil, latitude: nil) }
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: 0.0, latitude: 0.0) }
it 'extracts longitude and latitude correctly' do
expect { described_class.new(point).call }.to \
@@ -55,7 +55,7 @@ RSpec.describe Points::RawDataLonlatExtractor do
}
}
end
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: nil, latitude: nil) }
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: 0.0, latitude: 0.0) }
it 'extracts longitude and latitude correctly' do
expect { described_class.new(point).call }.to \
@@ -74,7 +74,7 @@ RSpec.describe Points::RawDataLonlatExtractor do
}
}
end
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: nil, latitude: nil) }
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: 0.0, latitude: 0.0) }
it 'extracts longitude and latitude correctly' do
expect { described_class.new(point).call }.to \
@@ -92,7 +92,7 @@ RSpec.describe Points::RawDataLonlatExtractor do
'lat' => 51.2345678
}
end
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: nil, latitude: nil) }
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: 0.0, latitude: 0.0) }
it 'extracts longitude and latitude correctly' do
expect { described_class.new(point).call }.to \
@@ -111,7 +111,7 @@ RSpec.describe Points::RawDataLonlatExtractor do
}
}
end
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: nil, latitude: nil) }
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: 0.0, latitude: 0.0) }
it 'extracts longitude and latitude correctly' do
expect { described_class.new(point).call }.to \
@@ -129,7 +129,7 @@ RSpec.describe Points::RawDataLonlatExtractor do
'latitude' => 51.2345678
}
end
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: nil, latitude: nil) }
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: 0.0, latitude: 0.0) }
it 'extracts longitude and latitude correctly' do
expect { described_class.new(point).call }.to \
@@ -148,7 +148,7 @@ RSpec.describe Points::RawDataLonlatExtractor do
}
}
end
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: nil, latitude: nil) }
let(:point) { create(:point, user: user, raw_data: raw_data, longitude: 0.0, latitude: 0.0) }
# Mock the entire call method since service doesn't have nil check
before do