From b476aeebe48316ef0c848e898f3d023da6a47050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jovan=20Kruni=C4=87?= Date: Wed, 7 Oct 2026 14:30:10 +0200 Subject: [PATCH] fix: make gelocation work reliably Closes #249 --- frontend/app/__mocks__/@capacitor/app.ts | 41 ++ .../app/android/capacitor.settings.gradle | 14 +- .../list/food-data-list.component.spec.ts | 127 ++++++ .../data/list/food-data-list.component.ts | 53 +-- .../controls/geolocate-control.component.ts | 113 +++++- .../app/modules/map/position.service.spec.ts | 362 +++++++++++++++++- .../src/app/modules/map/position.service.ts | 257 ++++++++++--- .../util/rxjs/distinct-until-moved.spec.ts | 82 ++++ .../src/app/util/rxjs/distinct-until-moved.ts | 31 ++ 9 files changed, 986 insertions(+), 94 deletions(-) create mode 100644 frontend/app/__mocks__/@capacitor/app.ts create mode 100644 frontend/app/src/app/modules/data/list/food-data-list.component.spec.ts create mode 100644 frontend/app/src/app/util/rxjs/distinct-until-moved.spec.ts create mode 100644 frontend/app/src/app/util/rxjs/distinct-until-moved.ts diff --git a/frontend/app/__mocks__/@capacitor/app.ts b/frontend/app/__mocks__/@capacitor/app.ts new file mode 100644 index 00000000..22d4a82d --- /dev/null +++ b/frontend/app/__mocks__/@capacitor/app.ts @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2026 StApps + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ + +export interface URLOpenListenerEvent { + url: string; +} + +export interface AppInfo { + name: string; + id: string; + build: string; + version: string; +} + +export interface PluginListenerHandle { + remove: () => Promise; +} + +export class AppMock { + addListener(_eventName: string, _listener: (event: T) => void): Promise { + return Promise.resolve({remove: () => Promise.resolve()}); + } + + getInfo(): Promise { + return Promise.resolve({name: 'StApps', id: 'de.anyschool.app', build: '1', version: '1.2.3'}); + } +} + +export const App = new AppMock(); diff --git a/frontend/app/android/capacitor.settings.gradle b/frontend/app/android/capacitor.settings.gradle index ed17b8b0..af9a5408 100644 --- a/frontend/app/android/capacitor.settings.gradle +++ b/frontend/app/android/capacitor.settings.gradle @@ -12,28 +12,28 @@ include ':capacitor-app' project(':capacitor-app').projectDir = new File('../../../node_modules/.pnpm/@capacitor+app@8.0.1_@capacitor+core@8.2.0/node_modules/@capacitor/app/android') include ':capacitor-browser' -project(':capacitor-browser').projectDir = new File('../../../node_modules/.pnpm/@capacitor+browser@8.0.2_@capacitor+core@8.2.0/node_modules/@capacitor/browser/android') +project(':capacitor-browser').projectDir = new File('../../../node_modules/.pnpm/@capacitor+browser@8.0.4_@capacitor+core@8.2.0/node_modules/@capacitor/browser/android') include ':capacitor-clipboard' project(':capacitor-clipboard').projectDir = new File('../../../node_modules/.pnpm/@capacitor+clipboard@8.0.1_@capacitor+core@8.2.0/node_modules/@capacitor/clipboard/android') include ':capacitor-device' -project(':capacitor-device').projectDir = new File('../../../node_modules/.pnpm/@capacitor+device@8.0.1_@capacitor+core@8.2.0/node_modules/@capacitor/device/android') +project(':capacitor-device').projectDir = new File('../../../node_modules/.pnpm/@capacitor+device@8.0.3_@capacitor+core@8.2.0/node_modules/@capacitor/device/android') include ':capacitor-dialog' project(':capacitor-dialog').projectDir = new File('../../../node_modules/.pnpm/@capacitor+dialog@8.0.1_@capacitor+core@8.2.0/node_modules/@capacitor/dialog/android') include ':capacitor-filesystem' -project(':capacitor-filesystem').projectDir = new File('../../../node_modules/.pnpm/@capacitor+filesystem@8.1.2_@capacitor+core@8.2.0/node_modules/@capacitor/filesystem/android') +project(':capacitor-filesystem').projectDir = new File('../../../node_modules/.pnpm/@capacitor+filesystem@8.1.3_@capacitor+core@8.2.0/node_modules/@capacitor/filesystem/android') include ':capacitor-geolocation' project(':capacitor-geolocation').projectDir = new File('../../../node_modules/.pnpm/@capacitor+geolocation@8.1.0_@capacitor+core@8.2.0/node_modules/@capacitor/geolocation/android') include ':capacitor-haptics' -project(':capacitor-haptics').projectDir = new File('../../../node_modules/.pnpm/@capacitor+haptics@8.0.1_@capacitor+core@8.2.0/node_modules/@capacitor/haptics/android') +project(':capacitor-haptics').projectDir = new File('../../../node_modules/.pnpm/@capacitor+haptics@8.0.2_@capacitor+core@8.2.0/node_modules/@capacitor/haptics/android') include ':capacitor-keyboard' -project(':capacitor-keyboard').projectDir = new File('../../../node_modules/.pnpm/@capacitor+keyboard@8.0.1_@capacitor+core@8.2.0/node_modules/@capacitor/keyboard/android') +project(':capacitor-keyboard').projectDir = new File('../../../node_modules/.pnpm/@capacitor+keyboard@8.0.5_@capacitor+core@8.2.0/node_modules/@capacitor/keyboard/android') include ':capacitor-local-notifications' project(':capacitor-local-notifications').projectDir = new File('../../../node_modules/.pnpm/@capacitor+local-notifications@8.0.2_@capacitor+core@8.2.0/node_modules/@capacitor/local-notifications/android') @@ -51,10 +51,10 @@ include ':capacitor-share' project(':capacitor-share').projectDir = new File('../../../node_modules/.pnpm/@capacitor+share@8.0.1_@capacitor+core@8.2.0/node_modules/@capacitor/share/android') include ':capacitor-splash-screen' -project(':capacitor-splash-screen').projectDir = new File('../../../node_modules/.pnpm/@capacitor+splash-screen@8.0.1_@capacitor+core@8.2.0/node_modules/@capacitor/splash-screen/android') +project(':capacitor-splash-screen').projectDir = new File('../../../node_modules/.pnpm/@capacitor+splash-screen@8.0.2_@capacitor+core@8.2.0/node_modules/@capacitor/splash-screen/android') include ':transistorsoft-capacitor-background-fetch' project(':transistorsoft-capacitor-background-fetch').projectDir = new File('../../../node_modules/.pnpm/@transistorsoft+capacitor-background-fetch@8.0.0_@capacitor+core@8.2.0/node_modules/@transistorsoft/capacitor-background-fetch/android') include ':capacitor-secure-storage-plugin' -project(':capacitor-secure-storage-plugin').projectDir = new File('../../../node_modules/.pnpm/capacitor-secure-storage-plugin@0.12.0_@capacitor+core@8.2.0/node_modules/capacitor-secure-storage-plugin/android') +project(':capacitor-secure-storage-plugin').projectDir = new File('../../../node_modules/.pnpm/capacitor-secure-storage-plugin@0.13.0_@capacitor+core@8.2.0/node_modules/capacitor-secure-storage-plugin/android') diff --git a/frontend/app/src/app/modules/data/list/food-data-list.component.spec.ts b/frontend/app/src/app/modules/data/list/food-data-list.component.spec.ts new file mode 100644 index 00000000..0c20daed --- /dev/null +++ b/frontend/app/src/app/modules/data/list/food-data-list.component.spec.ts @@ -0,0 +1,127 @@ +/* + * Copyright (C) 2023 StApps + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ +import {TestBed} from '@angular/core/testing'; +import {Position} from '@capacitor/geolocation'; +import {SCSearchSort} from '@openstapps/core'; +import {BehaviorSubject, ReplaySubject} from 'rxjs'; +import {PositionService} from '../../map/position.service'; +import {ContextMenuService} from '../../menu/context/context-menu.service'; +import {FoodDataListComponent, RESORT_AFTER_METERS} from './food-data-list.component'; + +describe('FoodDataListComponent', () => { + // keep well clear of the threshold: the helper below uses a flat-earth approximation, + // so distances right at the boundary could round either way + const BELOW_THRESHOLD = RESORT_AFTER_METERS * 0.6; + const ABOVE_THRESHOLD = RESORT_AFTER_METERS * 1.5; + + let geoLocation: ReplaySubject; + let positionService: { + geoLocation: ReplaySubject; + requestPermission: jasmine.Spy; + position?: {latitude: number; longitude: number}; + }; + let sortQuery: BehaviorSubject; + let sortQueryNext: jasmine.Spy; + let component: FoodDataListComponent; + + /** A Capacitor position, `metersNorth` north of the start point (only the fields the component reads) */ + const position = (metersNorth = 0): Position => + ({ + timestamp: 0, + coords: {latitude: 50.1278 + metersNorth / 111_195, longitude: 8.6674, accuracy: 20}, + }) as Position; + + const sortedBy = (p: Position): SCSearchSort[] => [ + { + type: 'distance', + order: 'asc', + arguments: {field: 'geo', position: [p.coords.longitude, p.coords.latitude]}, + }, + ]; + + beforeEach(() => { + // the real geoLocation replays the last position to new subscribers, so does this stand-in + geoLocation = new ReplaySubject(1); + positionService = { + geoLocation, + requestPermission: jasmine.createSpy('requestPermission').and.resolveTo('granted'), + }; + sortQuery = new BehaviorSubject(undefined); + sortQueryNext = spyOn(sortQuery, 'next').and.callThrough(); + + TestBed.configureTestingModule({ + providers: [ + {provide: PositionService, useValue: positionService}, + {provide: ContextMenuService, useValue: {sortQuery}}, + ], + }); + // only the class logic is under test, so the template is not rendered + component = TestBed.runInInjectionContext(() => new FoodDataListComponent()); + }); + + it('should ask for location permission when the page is entered', () => { + component.ionViewWillEnter(); + expect(positionService.requestPermission).toHaveBeenCalledTimes(1); + }); + + it('should not sort while the page is not in view', () => { + geoLocation.next(position()); + expect(sortQueryNext).not.toHaveBeenCalled(); + }); + + it('should sort by distance to the current position when the page is entered', () => { + geoLocation.next(position()); + component.ionViewWillEnter(); + + expect(sortQueryNext).toHaveBeenCalledOnceWith(sortedBy(position())); + expect(positionService.position).toEqual({ + latitude: position().coords.latitude, + longitude: position().coords.longitude, + }); + }); + + it(`should not re-sort on movements below ${RESORT_AFTER_METERS} m (GPS jitter)`, () => { + component.ionViewWillEnter(); + geoLocation.next(position()); + geoLocation.next(position(RESORT_AFTER_METERS * 0.1)); + geoLocation.next(position(BELOW_THRESHOLD)); + + expect(sortQueryNext).toHaveBeenCalledTimes(1); + }); + + it(`should re-sort after moving more than ${RESORT_AFTER_METERS} m`, () => { + component.ionViewWillEnter(); + geoLocation.next(position()); + geoLocation.next(position(ABOVE_THRESHOLD)); + + expect(sortQueryNext).toHaveBeenCalledTimes(2); + expect(sortQueryNext).toHaveBeenCalledWith(sortedBy(position(ABOVE_THRESHOLD))); + }); + + it('should stop sorting when the page is left and sort again with the latest position on return', () => { + const movedWhileAway = RESORT_AFTER_METERS * 5; + + component.ionViewWillEnter(); + geoLocation.next(position()); + component.ionViewWillLeave(); + + geoLocation.next(position(movedWhileAway)); + expect(sortQueryNext).toHaveBeenCalledTimes(1); + + component.ionViewWillEnter(); + expect(sortQueryNext).toHaveBeenCalledTimes(2); + expect(sortQueryNext).toHaveBeenCalledWith(sortedBy(position(movedWhileAway))); + }); +}); diff --git a/frontend/app/src/app/modules/data/list/food-data-list.component.ts b/frontend/app/src/app/modules/data/list/food-data-list.component.ts index 546d686b..7372d139 100644 --- a/frontend/app/src/app/modules/data/list/food-data-list.component.ts +++ b/frontend/app/src/app/modules/data/list/food-data-list.component.ts @@ -14,13 +14,18 @@ */ import {Component, inject} from '@angular/core'; import {PositionService} from '../../map/position.service'; -import {Geolocation} from '@capacitor/geolocation'; -import {BehaviorSubject, catchError} from 'rxjs'; +import {BehaviorSubject, map} from 'rxjs'; import {pauseWhen} from '../../../util/rxjs/pause-when'; +import {distinctUntilMoved} from '../../../util/rxjs/distinct-until-moved'; import {SCSearchFilter} from '@openstapps/core'; import {ContextMenuService} from '../../menu/context/context-menu.service'; import {takeUntilDestroyed} from '@angular/core/rxjs-interop'; +/** + * Re-sort the list (= new search) only after the user moved this far + */ +export const RESORT_AFTER_METERS = 100; + /** * Presents a list of places for eating/drinking */ @@ -68,43 +73,39 @@ export class FoodDataListComponent { type: 'boolean', }; + private positionService = inject(PositionService); + constructor() { - const positionService = inject(PositionService); + const positionService = this.positionService; const contextMenuService = inject(ContextMenuService); - positionService - .watchCurrentLocation({enableHighAccuracy: false, maximumAge: 1000}) + // Uses the app-wide shared location watch instead of starting its own + positionService.geoLocation .pipe( + map(({coords}) => ({latitude: coords.latitude, longitude: coords.longitude})), + distinctUntilMoved(RESORT_AFTER_METERS), pauseWhen(this.isNotInView$), takeUntilDestroyed(), - catchError(async _error => { - await Geolocation.checkPermissions(); - }), ) - .subscribe({ - next(position) { - if (!position) return; - positionService.position = position; - contextMenuService.sortQuery.next([ - { - type: 'distance', - order: 'asc', - arguments: { - field: 'geo', - position: [position.longitude, position.latitude], - }, + .subscribe(position => { + positionService.position = position; + contextMenuService.sortQuery.next([ + { + type: 'distance', + order: 'asc', + arguments: { + field: 'geo', + position: [position.longitude, position.latitude], }, - ]); - }, - async error() { - positionService.position = undefined; - await Geolocation.checkPermissions(); - }, + }, + ]); }); } ionViewWillEnter() { this.isNotInView$.next(false); + // the shared watch never asks by itself; on this page, location is the point, so ask here + void this.positionService.requestPermission(); } ionViewWillLeave() { diff --git a/frontend/app/src/app/modules/map/controls/geolocate-control.component.ts b/frontend/app/src/app/modules/map/controls/geolocate-control.component.ts index ac3f5acc..0d141126 100644 --- a/frontend/app/src/app/modules/map/controls/geolocate-control.component.ts +++ b/frontend/app/src/app/modules/map/controls/geolocate-control.component.ts @@ -13,16 +13,86 @@ import {IonFabButton} from '@ionic/angular/standalone'; import {MapService} from '@maplibre/ngx-maplibre-gl'; import {FitBoundsOptions, GeolocateControl, GeolocateControlOptions} from 'maplibre-gl'; import {Map as MapLibre} from 'maplibre-gl'; -import {BehaviorSubject} from 'rxjs'; +import {BehaviorSubject, Subscription} from 'rxjs'; import {IonIconDirective} from 'src/app/util/ion-icon/ion-icon.directive'; +import {PERMISSION_GRANTED, PositionService} from '../position.service'; type WatchState = InstanceType['_watchState']; +type GeolocationApi = Pick; + +/** + * A `navigator.geolocation` look-alike that serves positions from the app's PositionService. + * MapLibre has no option for a custom position source and always calls `navigator.geolocation`, + * so this lets the map share the app's single location watch instead of starting its own. + */ +class PositionServiceGeolocation implements GeolocationApi { + private nextId = 1; + + private readonly watches = new Map(); + + constructor(private readonly positionService: PositionService) {} + + watchPosition(success: PositionCallback, error?: PositionErrorCallback | null): number { + const id = this.nextId++; + const watch = new Subscription(); + this.watches.set(id, watch); + void this.positionService.requestPermission().then(permission => { + if (watch.closed) return; + if (permission != PERMISSION_GRANTED) { + error?.({code: 1, message: 'User denied Geolocation'} as GeolocationPositionError); + return; + } + // Capacitor's Position has the same shape as the browser's GeolocationPosition + watch.add( + // precise: while the user tracks their location on the map, the shared watch uses GPS + this.positionService.preciseLocation.subscribe(position => + success(position as unknown as GeolocationPosition), + ), + ); + }); + return id; + } + + clearWatch(id: number): void { + this.watches.get(id)?.unsubscribe(); + this.watches.delete(id); + } + + getCurrentPosition(success: PositionCallback, error?: PositionErrorCallback | null): void { + const id = this.watchPosition( + position => { + this.clearWatch(id); + success(position); + }, + positionError => { + this.clearWatch(id); + error?.(positionError); + }, + ); + } +} + +/** + * Runs `run` while `navigator.geolocation` is replaced by `geolocation`. + * MapLibre only uses it synchronously inside the wrapped calls, so no other code sees the swap. + */ +function withGeolocation(geolocation: GeolocationApi, run: () => T): T { + Object.defineProperty(globalThis.navigator, 'geolocation', {value: geolocation, configurable: true}); + try { + return run(); + } finally { + // removing the own property brings back the browser's original + delete (globalThis.navigator as {geolocation?: unknown}).geolocation; + } +} + class CustomGeolocateControl extends GeolocateControl { constructor( public _container: HTMLElement, watchState: BehaviorSubject, options: GeolocateControlOptions, + private readonly geolocation: GeolocationApi, ) { super(options); Object.defineProperty(this, '_watchState', { @@ -45,7 +115,27 @@ class CustomGeolocateControl extends GeolocateControl { return this._container; } - override onRemove() {} + // MapLibre uses navigator.geolocation only in these three methods: point it to the PositionService + + override trigger(): boolean { + return withGeolocation(this.geolocation, () => super.trigger()); + } + + override _clearWatch(): void { + withGeolocation(this.geolocation, () => super._clearWatch()); + } + + /** + * Run MapLibre's cleanup (stops the watch, removes the dot, resets its global watch counter), + * but not letting it remove the Angular-owned container + */ + override onRemove() { + if (!this._map) return; + const container = this._container; + this._container = document.createElement('div'); + withGeolocation(this.geolocation, () => super.onRemove()); + this._container = container; + } } @Component({ @@ -58,6 +148,8 @@ class CustomGeolocateControl extends GeolocateControl { export class GeolocateControlComponent implements AfterContentInit, OnDestroy { private mapService = inject(MapService); + private positionService = inject(PositionService); + @Input() position?: 'top-left' | 'top-right' | 'bottom-left' | 'bottom-right'; @Input() positionOptions?: PositionOptions; @@ -75,12 +167,17 @@ export class GeolocateControlComponent implements AfterContentInit, OnDestroy { control: CustomGeolocateControl; ngAfterContentInit() { - this.control = new CustomGeolocateControl(this.content.nativeElement, this.watchState, { - positionOptions: this.positionOptions, - fitBoundsOptions: this.fitBoundsOptions, - trackUserLocation: this.trackUserLocation, - showUserLocation: this.showUserLocation, - }); + this.control = new CustomGeolocateControl( + this.content.nativeElement, + this.watchState, + { + positionOptions: this.positionOptions, + fitBoundsOptions: this.fitBoundsOptions, + trackUserLocation: this.trackUserLocation, + showUserLocation: this.showUserLocation, + }, + new PositionServiceGeolocation(this.positionService), + ); this.mapService.mapCreated$.subscribe(() => { this.mapService.addControl(this.control, this.position); }); diff --git a/frontend/app/src/app/modules/map/position.service.spec.ts b/frontend/app/src/app/modules/map/position.service.spec.ts index 0c050b2d..45dcac2a 100644 --- a/frontend/app/src/app/modules/map/position.service.spec.ts +++ b/frontend/app/src/app/modules/map/position.service.spec.ts @@ -12,15 +12,25 @@ * You should have received a copy of the GNU General Public License along with * this program. If not, see . */ -import {TestBed} from '@angular/core/testing'; +import {discardPeriodicTasks, fakeAsync, TestBed, tick} from '@angular/core/testing'; import {MapModule} from './map.module'; import {provideHttpClient, withInterceptorsFromDi} from '@angular/common/http'; import {StorageModule} from '../storage/storage.module'; -import {MapPosition, PositionService} from './position.service'; +import { + MapPosition, + PERMISSION_DENIED, + PERMISSION_GRANTED, + PERMISSION_PROMPT, + PositionService, + WATCH_KEEP_ALIVE_MS, + WATCH_RETRY_DELAY_MS, +} from './position.service'; import {ConfigProvider} from '../config/config.provider'; import {LoggerTestingModule} from 'ngx-logger/testing'; -import {firstValueFrom} from 'rxjs'; -import {Geolocation} from '@capacitor/geolocation'; +import {firstValueFrom, Subscription} from 'rxjs'; +import {Geolocation, PermissionStatus, Position} from '@capacitor/geolocation'; +import {App} from '@capacitor/app'; +import {Capacitor} from '@capacitor/core'; describe('PositionService', () => { let positionService: PositionService; @@ -70,4 +80,348 @@ describe('PositionService', () => { await new Promise(resolve => setTimeout(resolve, 100)); expect(clearWatch).toHaveBeenCalledWith({id: 'abc'}); }); + + /** Helpers for the shared watch */ + + /** Some longer period of time, for "nothing happens meanwhile" checks */ + const A_WHILE_MS = 10_000; + + /** A permission status as Capacitor reports it */ + const permission = ( + location: PermissionStatus['location'], + coarseLocation: PermissionStatus['coarseLocation'] = location, + ): PermissionStatus => ({location, coarseLocation}); + + /** A position (only the fields the service reads) */ + const position = (latitude: number): Position => + ({timestamp: 0, coords: {latitude, longitude: 8.6674, accuracy: 20}}) as Position; + + /** Every native watch the service started, with its options and callback */ + let watches: Array<{ + id: string; + options: PositionOptions; + emit: (position: Position | undefined, error?: unknown) => void; + }>; + + /** Replaces the native watch, so tests can see how many watches exist and feed them positions */ + const mockNativeWatch = () => { + watches = []; + spyOn(Geolocation, 'watchPosition').and.callFake((options, callback) => { + const id = `watch-${watches.length + 1}`; + watches.push({ + id, + options, + emit: callback as (position: Position | undefined, error?: unknown) => void, + }); + return Promise.resolve(id); + }); + return spyOn(Geolocation, 'clearWatch').and.resolveTo(); + }; + + describe('geoLocation (shared watch)', () => { + let clearWatch: jasmine.Spy; + let subscriptions: Subscription[]; + + beforeEach(() => { + clearWatch = mockNativeWatch(); + subscriptions = []; + }); + + afterEach(() => { + for (const subscription of subscriptions) subscription.unsubscribe(); + }); + + it('should start only one native watch for many subscribers', fakeAsync(() => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('granted')); + for (let index = 0; index < 10; index++) { + subscriptions.push(positionService.geoLocation.subscribe()); + } + tick(); + + expect(watches.length).toBe(1); + discardPeriodicTasks(); + })); + + it('should use coarse location by default', fakeAsync(() => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('granted')); + subscriptions.push(positionService.geoLocation.subscribe()); + tick(); + + expect(watches[0].options.enableHighAccuracy).toBeFalse(); + discardPeriodicTasks(); + })); + + it('should give the positions of the one watch to all subscribers', fakeAsync(() => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('granted')); + const first: number[] = []; + const second: number[] = []; + subscriptions.push( + positionService.geoLocation.subscribe(p => first.push(p.coords.latitude)), + positionService.geoLocation.subscribe(p => second.push(p.coords.latitude)), + ); + tick(); + watches[0].emit(position(50.1)); + + expect(first).toEqual([50.1]); + expect(second).toEqual([50.1]); + discardPeriodicTasks(); + })); + + it('should give new subscribers the last position right away', fakeAsync(() => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('granted')); + subscriptions.push(positionService.geoLocation.subscribe()); + tick(); + watches[0].emit(position(50.1)); + + let latecomer: number | undefined; + subscriptions.push(positionService.geoLocation.subscribe(p => (latecomer = p.coords.latitude))); + + expect(latecomer).toBe(50.1); + expect(watches.length).toBe(1); + discardPeriodicTasks(); + })); + + it('should accept approximate location (Android 12+)', fakeAsync(() => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('prompt', 'granted')); + subscriptions.push(positionService.geoLocation.subscribe()); + tick(); + + expect(watches.length).toBe(1); + discardPeriodicTasks(); + })); + + it('should not start a watch (and not ask) while permission is missing', fakeAsync(() => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('prompt')); + const getCurrentPosition = spyOn(Geolocation, 'getCurrentPosition'); + subscriptions.push(positionService.geoLocation.subscribe()); + tick(A_WHILE_MS); + + expect(watches.length).toBe(0); + expect(getCurrentPosition).not.toHaveBeenCalled(); + discardPeriodicTasks(); + })); + + it('should start the watch as soon as permission is granted later (web: re-checks every 2 s)', fakeAsync(() => { + const checkPermissions = spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('prompt')); + subscriptions.push(positionService.geoLocation.subscribe()); + tick(); + expect(watches.length).toBe(0); + + // the user clicks "Allow" in the browser prompt + checkPermissions.and.resolveTo(permission('granted')); + tick(2000); + + expect(watches.length).toBe(1); + discardPeriodicTasks(); + })); + + it('should keep the watch alive for a while without subscribers (e.g. list re-render)', fakeAsync(() => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('granted')); + positionService.geoLocation.subscribe().unsubscribe(); + // just before the keep-alive time is over + tick(WATCH_KEEP_ALIVE_MS - 1000); + subscriptions.push(positionService.geoLocation.subscribe()); + tick(); + + expect(watches.length).toBe(1); + expect(clearWatch).not.toHaveBeenCalled(); + discardPeriodicTasks(); + })); + + it('should clear the native watch when the keep-alive time after the last subscriber is over', fakeAsync(() => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('granted')); + const subscription = positionService.geoLocation.subscribe(); + tick(); + subscription.unsubscribe(); + tick(WATCH_KEEP_ALIVE_MS); + + expect(clearWatch).toHaveBeenCalledOnceWith({id: 'watch-1'}); + discardPeriodicTasks(); + })); + + it('should retry after a delay instead of breaking the subscribers on an error', fakeAsync(() => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('granted')); + let failed = false; + subscriptions.push(positionService.geoLocation.subscribe({error: () => (failed = true)})); + tick(); + + watches[0].emit(undefined, {code: 'OS-PLUG-GLOC-0010', message: 'timeout'}); + tick(WATCH_RETRY_DELAY_MS); + + expect(failed).toBeFalse(); + expect(watches.length).toBe(2); + // the plugin removes a failed watch by itself, so it must not be cleared again + expect(clearWatch).not.toHaveBeenCalled(); + discardPeriodicTasks(); + })); + }); + + describe('preciseLocation', () => { + beforeEach(() => { + mockNativeWatch(); + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('granted')); + }); + + it('should switch the shared watch to GPS while subscribed, and back to coarse afterwards', fakeAsync(() => { + const list = positionService.geoLocation.subscribe(); + tick(); + expect(watches.map(watch => watch.options.enableHighAccuracy)).toEqual([false]); + + const map = positionService.preciseLocation.subscribe(); + tick(); + expect(watches.map(watch => watch.options.enableHighAccuracy)).toEqual([false, true]); + + map.unsubscribe(); + tick(); + expect(watches.map(watch => watch.options.enableHighAccuracy)).toEqual([false, true, false]); + + list.unsubscribe(); + tick(WATCH_KEEP_ALIVE_MS); + discardPeriodicTasks(); + })); + + it('should not restart the watch for a second precise subscriber', fakeAsync(() => { + const first = positionService.preciseLocation.subscribe(); + tick(); + const second = positionService.preciseLocation.subscribe(); + tick(); + second.unsubscribe(); + tick(); + + expect(watches.length).toBe(1); + expect(watches[0].options.enableHighAccuracy).toBeTrue(); + + first.unsubscribe(); + tick(WATCH_KEEP_ALIVE_MS); + discardPeriodicTasks(); + })); + }); + + describe('requestPermission', () => { + let getCurrentPosition: jasmine.Spy; + + beforeEach(() => { + getCurrentPosition = spyOn(Geolocation, 'getCurrentPosition').and.resolveTo(position(50.1)); + }); + + it('should not ask again if location is already allowed (also approximate only)', async () => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('prompt', 'granted')); + + expect(await positionService.requestPermission()).toBe(PERMISSION_GRANTED); + expect(getCurrentPosition).not.toHaveBeenCalled(); + }); + + it('should not ask again if location was denied', async () => { + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('denied')); + + expect(await positionService.requestPermission()).toBe(PERMISSION_DENIED); + expect(getCurrentPosition).not.toHaveBeenCalled(); + }); + + it('should show the prompt and report "granted" if the user allows', async () => { + spyOn(Geolocation, 'checkPermissions').and.returnValues( + Promise.resolve(permission('prompt')), + Promise.resolve(permission('granted')), + ); + + expect(await positionService.requestPermission()).toBe(PERMISSION_GRANTED); + expect(getCurrentPosition).toHaveBeenCalledTimes(1); + }); + + it('should report "denied" if the user denies in the prompt', async () => { + getCurrentPosition.and.rejectWith({code: 1}); + spyOn(Geolocation, 'checkPermissions').and.returnValues( + Promise.resolve(permission('prompt')), + Promise.resolve(permission('denied')), + ); + + expect(await positionService.requestPermission()).toBe(PERMISSION_DENIED); + }); + + it('should report "prompt" if the user closes the prompt without deciding', async () => { + getCurrentPosition.and.rejectWith({code: 1}); + spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('prompt')); + + expect(await positionService.requestPermission()).toBe(PERMISSION_PROMPT); + }); + + it('should report "granted" if location is allowed but there is no fix yet (timeout)', async () => { + getCurrentPosition.and.rejectWith({code: 'OS-PLUG-GLOC-0010'}); + spyOn(Geolocation, 'checkPermissions').and.returnValues( + Promise.resolve(permission('prompt')), + Promise.resolve(permission('granted')), + ); + + expect(await positionService.requestPermission()).toBe(PERMISSION_GRANTED); + }); + + it('should judge by the position if the browser has no Permissions API', async () => { + spyOn(Geolocation, 'checkPermissions').and.rejectWith(new Error('Permissions API not available')); + + expect(await positionService.requestPermission()).toBe(PERMISSION_GRANTED); + + getCurrentPosition.and.rejectWith({code: 1}); + expect(await positionService.requestPermission()).toBe(PERMISSION_DENIED); + }); + }); + + describe('on devices (no permission polling)', () => { + let checkPermissions: jasmine.Spy; + let resumeApp: () => void; + let service: PositionService; + + beforeEach(() => { + mockNativeWatch(); + spyOn(Capacitor, 'isNativePlatform').and.returnValue(true); + spyOn(App, 'addListener').and.callFake((_event, callback) => { + resumeApp = callback as () => void; + return Promise.resolve({remove: () => Promise.resolve()}); + }); + spyOn(Geolocation, 'getCurrentPosition').and.resolveTo(position(50.1)); + checkPermissions = spyOn(Geolocation, 'checkPermissions').and.resolveTo(permission('prompt')); + // created after the spies, because the service decides at creation whether it runs on a device + service = TestBed.runInInjectionContext(() => new PositionService()); + }); + + it('should check the permission once instead of polling', fakeAsync(() => { + const subscription = service.geoLocation.subscribe(); + tick(A_WHILE_MS); + + expect(checkPermissions).toHaveBeenCalledTimes(1); + subscription.unsubscribe(); + tick(WATCH_KEEP_ALIVE_MS); + })); + + it('should start the watch right after the user allowed location in our prompt', fakeAsync(() => { + const subscription = service.geoLocation.subscribe(); + tick(); + expect(watches.length).toBe(0); + + // the user allows location in the prompt shown by requestPermission() + (Geolocation.getCurrentPosition as jasmine.Spy).and.callFake(() => { + checkPermissions.and.resolveTo(permission('granted')); + return Promise.resolve(position(50.1)); + }); + void service.requestPermission(); + tick(); + + expect(watches.length).toBe(1); + subscription.unsubscribe(); + tick(WATCH_KEEP_ALIVE_MS); + })); + + it('should check again when the app comes back from the system settings', fakeAsync(() => { + const subscription = service.geoLocation.subscribe(); + tick(); + expect(watches.length).toBe(0); + + checkPermissions.and.resolveTo(permission('granted')); + resumeApp(); + tick(); + + expect(watches.length).toBe(1); + subscription.unsubscribe(); + tick(WATCH_KEEP_ALIVE_MS); + })); + }); }); diff --git a/frontend/app/src/app/modules/map/position.service.ts b/frontend/app/src/app/modules/map/position.service.ts index 418145f2..0aa7f89e 100644 --- a/frontend/app/src/app/modules/map/position.service.ts +++ b/frontend/app/src/app/modules/map/position.service.ts @@ -14,8 +14,113 @@ */ import {Injectable} from '@angular/core'; import {Point} from 'geojson'; -import {Observable} from 'rxjs'; -import {Geolocation, Position} from '@capacitor/geolocation'; +import { + BehaviorSubject, + catchError, + defer, + distinctUntilChanged, + finalize, + first, + from, + map, + merge, + Observable, + of, + ReplaySubject, + retry, + share, + Subject, + switchMap, + timer, +} from 'rxjs'; +import {Geolocation, PermissionStatus, Position} from '@capacitor/geolocation'; +import {App} from '@capacitor/app'; +import {Capacitor, PermissionState} from '@capacitor/core'; + +export const PERMISSION_PROMPT = 'prompt' satisfies PermissionState; + +/** + * Result of requestPermission(): granted, denied, or still undecided (prompt dismissed) + */ +export type LocationPermission = + | typeof PERMISSION_GRANTED + | typeof PERMISSION_DENIED + | typeof PERMISSION_PROMPT; + +/** + * How long the shared location watch keeps running after the last subscriber left, + * so a short gap (e.g. a list re-render or quick page switch) doesn't restart it + */ +export const WATCH_KEEP_ALIVE_MS = 30_000; + +/** + * How long to wait before starting a new watch after an error (e.g. no fix indoors) + */ +export const WATCH_RETRY_DELAY_MS = 10_000; + +/** + * Coarse (network / Wi-Fi) location: enough for distances in lists, fast indoors, saves battery + */ +const COARSE_OPTIONS: PositionOptions = {enableHighAccuracy: false, maximumAge: 30_000, timeout: 30_000}; + +/** + * Precise (GPS) location: for following the user on the map + */ +const PRECISE_OPTIONS: PositionOptions = {enableHighAccuracy: true, maximumAge: 0, timeout: 30_000}; + +/** + * Permission states as defined by Capacitor (`PermissionState` is a type only, it has no runtime constants) + */ +export const PERMISSION_GRANTED = 'granted' satisfies PermissionState; +export const PERMISSION_DENIED = 'denied' satisfies PermissionState; + +/** + * Location may be used. On Android, `location` is only granted for *precise* location; + * if the user chose "approximate", only `coarseLocation` is granted, which is enough for us + * (Android then just ignores enableHighAccuracy). + */ +function isGranted(status: PermissionStatus): boolean { + return status.location === PERMISSION_GRANTED || status.coarseLocation === PERMISSION_GRANTED; +} + +/** + * The user refused both precise and approximate location + */ +function isDenied(status: PermissionStatus): boolean { + return status.location === PERMISSION_DENIED && status.coarseLocation === PERMISSION_DENIED; +} + +/** + * Emits when the app comes back to the foreground (e.g. from the system settings) + */ +const appResumed$ = new Observable(subscriber => { + const listener = App.addListener('resume', () => subscriber.next()); + return () => void listener.then(handle => handle.remove()); +}); + +/** + * Wraps a Capacitor position watch in an Observable. + * Passes the real error on, and only clears the watch if it is still alive + * (the plugin removes a watch by itself after an error). + */ +function nativeWatch(options: PositionOptions): Observable { + return new Observable(subscriber => { + let active = true; + const watcherID = Geolocation.watchPosition(options, (position, error) => { + if (error) { + active = false; + subscriber.error(error); + } else if (position) { + subscriber.next(position); + } + }); + return () => { + const wasActive = active; + active = false; + void watcherID.then(id => (wasActive ? Geolocation.clearWatch({id}) : undefined)).catch(() => {}); + }; + }); +} export interface Coordinates { /** @@ -39,29 +144,95 @@ export interface MapPosition extends Coordinates { providedIn: 'root', }) export class PositionService { - geoLocation = new Observable(subscriber => { - const watcherID = Geolocation.checkPermissions().then(permissions => { - if (permissions.location === 'granted') { - return Geolocation.watchPosition({}, (position, error) => { - if (error) { - subscriber.error(position); - } else if (position) { - subscriber.next(position); - } - }); - } - return; - }); - return { - unsubscribe() { - watcherID.then(id => { - if (id) { - Geolocation.clearWatch({id}); - } - }); - }, - }; - }); + /** + * How many subscribers currently need precise (GPS) location, see preciseLocation + */ + private readonly preciseSubscribers$ = new BehaviorSubject(0); + + /** + * Emits after requestPermission() showed the permission prompt + */ + private readonly permissionRequested$ = new Subject(); + + /** + * When to (re-)check whether location permission has been granted. + * Web: every 2 s, browsers don't tell us when the user clicks "Allow". + * Device: permission only changes via our prompt or the system settings (and coming back + * from there resumes the app), so no polling is needed. + */ + private readonly permissionChecks$: Observable = Capacitor.isNativePlatform() + ? merge(of(true), this.permissionRequested$, appResumed$) + : timer(0, 2000); + + /** + * The device position, shared by everything that needs it (list distances, map, ...). + * + * - Only one native watch, however many subscribers (before: one per list item) + * - Waits until location permission is granted, also if that happens later (e.g. via the prompt) + * - New subscribers immediately get the last position + * - Keeps running for 30 s without subscribers, so a list re-render doesn't restart it + * - Retries after errors (e.g. timeouts indoors) instead of breaking all subscribers + * - Coarse by default; switches the same watch to GPS while someone uses preciseLocation + */ + readonly geoLocation: Observable = defer(() => + this.permissionChecks$.pipe( + switchMap(() => + from(Geolocation.checkPermissions()).pipe( + // no Permissions API (some browsers): just start, watchPosition prompts / fails by itself + catchError(() => + of({location: PERMISSION_GRANTED, coarseLocation: PERMISSION_GRANTED}), + ), + ), + ), + first(isGranted), + // restart the one watch with other options whenever the need for precision changes + switchMap(() => + this.preciseSubscribers$.pipe( + map(count => count > 0), + distinctUntilChanged(), + switchMap(precise => nativeWatch(precise ? PRECISE_OPTIONS : COARSE_OPTIONS)), + ), + ), + retry({delay: () => timer(WATCH_RETRY_DELAY_MS)}), + ), + ).pipe( + share({ + connector: () => new ReplaySubject(1), + resetOnRefCountZero: () => timer(WATCH_KEEP_ALIVE_MS), + }), + ); + + /** + * Same positions as geoLocation, but while subscribed, the shared watch uses GPS. + * For actively following the user (map); falls back to coarse when the last one unsubscribes. + */ + readonly preciseLocation: Observable = defer(() => { + this.preciseSubscribers$.next(this.preciseSubscribers$.value + 1); + return this.geoLocation; + }).pipe(finalize(() => this.preciseSubscribers$.next(this.preciseSubscribers$.value - 1))); + + /** + * Shows the permission prompt if permission hasn't been decided yet + * @returns granted, denied, or prompt if the user closed the dialog without deciding + */ + async requestPermission(): Promise { + const status = await Geolocation.checkPermissions().catch(() => {}); + if (status && isGranted(status)) return PERMISSION_GRANTED; + if (status && isDenied(status)) return PERMISSION_DENIED; + // getCurrentPosition shows the prompt on web, Android and iOS + const located = await Geolocation.getCurrentPosition(COARSE_OPTIONS).then( + () => true, + () => false, + ); + this.permissionRequested$.next(); + const after = await Geolocation.checkPermissions().catch(() => {}); + // no Permissions API (some browsers): judge by whether a position came back + if (!after) return located ? PERMISSION_GRANTED : PERMISSION_DENIED; + // granted also covers "allowed, but no fix yet" (timeout): the shared watch keeps trying + if (isGranted(after)) return PERMISSION_GRANTED; + if (isDenied(after)) return PERMISSION_DENIED; + return PERMISSION_PROMPT; + } /** * Current position @@ -108,29 +279,17 @@ export class PositionService { * @param options Options which define which data should be provided (e.g., how accurate or how old) */ watchCurrentLocation(options: PositionOptions = {}): Observable { - return new Observable(subscriber => { - const watcherID = Geolocation.watchPosition(options, (position, error) => { - if (error) { - subscriber.error(position); - } else { - this.position = { - // TODO use native compass heading instead - // waiting for https://github.com/ionic-team/capacitor-plugins/issues/1192 - heading: undefined, - latitude: position?.coords.latitude ?? 0, - longitude: position?.coords.longitude ?? 0, // TODO: handle null position - }; - - subscriber.next(this.position); - } - }); - return { - unsubscribe() { - watcherID.then(id => { - void Geolocation.clearWatch({id}); - }); - }, - }; - }); + return nativeWatch(options).pipe( + map(position => { + this.position = { + // TODO use native compass heading instead + // waiting for https://github.com/ionic-team/capacitor-plugins/issues/1192 + heading: undefined, + latitude: position.coords.latitude, + longitude: position.coords.longitude, + }; + return this.position; + }), + ); } } diff --git a/frontend/app/src/app/util/rxjs/distinct-until-moved.spec.ts b/frontend/app/src/app/util/rxjs/distinct-until-moved.spec.ts new file mode 100644 index 00000000..1889cc26 --- /dev/null +++ b/frontend/app/src/app/util/rxjs/distinct-until-moved.spec.ts @@ -0,0 +1,82 @@ +/* + * Copyright (C) 2026 StApps + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ +import {LngLat, LngLatBounds} from 'maplibre-gl'; +import {of, toArray, firstValueFrom} from 'rxjs'; +import {distinctUntilMoved} from './distinct-until-moved'; + +describe('distinctUntilMoved', () => { + // Frankfurt (Campus Westend), 50° N + const start = {latitude: 50.1278, longitude: 8.6674}; + + /** A box `meters` around `start` (MapLibre): its north / east edge is `meters` north / east of `start` */ + const around = (meters: number) => + LngLatBounds.fromLngLat(new LngLat(start.longitude, start.latitude), meters); + + /** `start` moved `meters` north */ + const north = (meters: number) => ({latitude: around(meters).getNorth(), longitude: start.longitude}); + + /** `start` moved `meters` east */ + const east = (meters: number) => ({latitude: start.latitude, longitude: around(meters).getEast()}); + + /** `start` moved `northMeters` north and `eastMeters` east */ + const moved = (northMeters: number, eastMeters: number) => ({ + latitude: around(northMeters).getNorth(), + longitude: around(eastMeters).getEast(), + }); + + /** `start` moved `meters` north-east (diagonally, as the crow flies) */ + const northEast = (meters: number) => moved(meters / Math.SQRT2, meters / Math.SQRT2); + + const run = (positions: T[], meters = 100) => + firstValueFrom(of(...positions).pipe(distinctUntilMoved(meters), toArray())); + + it('should always let the first position through', async () => { + expect(await run([start])).toEqual([start]); + }); + + it('should drop GPS jitter below the threshold', async () => { + expect(await run([start, north(5), north(30), north(99)])).toEqual([start]); + }); + + it('should let a position through once the threshold is reached', async () => { + const far = north(120); + expect(await run([start, north(50), far])).toEqual([start, far]); + }); + + it('should measure real meters in every direction (not degrees)', async () => { + // at 50° N, 100 m east is a bigger change in degrees than 100 m north + expect(await run([start, east(90)])).toEqual([start]); + expect(await run([start, east(110)])).toEqual([start, east(110)]); + }); + + it('should compare with the last position that was let through, so slow movement adds up', async () => { + // walk 240 m north-east in 40 m steps + const steps = [ + start, + northEast(40), + northEast(80), + northEast(120), // 120 m from start: let through, new reference + northEast(160), + northEast(200), // 200 m from start, but only 80 m from the reference + northEast(240), // 120 m from the reference + ]; + expect(await run(steps)).toEqual([start, northEast(120), northEast(240)]); + }); + + it('should use the given threshold', async () => { + expect(await run([start, north(30)], 25)).toEqual([start, north(30)]); + expect(await run([start, north(30)], 50)).toEqual([start]); + }); +}); diff --git a/frontend/app/src/app/util/rxjs/distinct-until-moved.ts b/frontend/app/src/app/util/rxjs/distinct-until-moved.ts new file mode 100644 index 00000000..80ff6b77 --- /dev/null +++ b/frontend/app/src/app/util/rxjs/distinct-until-moved.ts @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2026 StApps + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ +import {LngLat} from 'maplibre-gl'; +import {distinctUntilChanged, MonoTypeOperatorFunction} from 'rxjs'; + +/** + * Lets a position through only after the device has moved at least `meters` + * since the last position that was let through (ignores GPS jitter) + */ +export function distinctUntilMoved( + meters: number, +): MonoTypeOperatorFunction { + return distinctUntilChanged( + (previous, current) => + new LngLat(previous.longitude, previous.latitude).distanceTo( + new LngLat(current.longitude, current.latitude), + ) < meters, + ); +}