mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-10 00:16:59 +00:00
Merge pull request #3827 from davotoula/fix/location-foreground-gate
Listen only while foregrounded, and fix the meter that measures it
This commit is contained in:
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,808 @@
|
||||
# Location: foreground-gate the listener, trim the request, fix the meter
|
||||
|
||||
Date: 2026-07-29
|
||||
Module: `amethyst`
|
||||
Origin: Finding 1 of `2026-07-29-resource-report-1.13.0-analysis.md` (1.13.0-PLAY,
|
||||
Pixel 9a / Android 17)
|
||||
Revision: 2 — incorporates spec review of 2026-07-29
|
||||
|
||||
## Context
|
||||
|
||||
The 1.13.0 resource report showed **7.13 h of location listening against 9.1
|
||||
seconds of app foreground**, and the accompanying analysis called it the leading
|
||||
suspect for that day's 11 pp of background battery drain. Root cause given: 30
|
||||
`SharingStarted.Eagerly` top-nav filter states on the account scope
|
||||
(`Account.kt:843-933`), one of which is always `TopFilter.AroundMe` because
|
||||
`AccountSettings.kt:256` ships that as the Products default. The `AroundMe`
|
||||
branch collects `locationFlow()`, and an `Eagerly`-shared subscriber holds
|
||||
`LocationState`'s `WhileSubscribed(5000)` open for the life of the process.
|
||||
|
||||
That chain is real. **The battery conclusion drawn from it is not**, and this
|
||||
spec is written against the measurement rather than the inference.
|
||||
|
||||
### What the device reports
|
||||
|
||||
`amethyst/src/main/AndroidManifest.xml:80` declares **only**
|
||||
`ACCESS_COARSE_LOCATION` — no `ACCESS_FINE_LOCATION`, no
|
||||
`ACCESS_BACKGROUND_LOCATION` — and no service declares
|
||||
`foregroundServiceType="location"` (the declared types are `mediaPlayback`,
|
||||
`microphone`, `camera`, `phoneCall`, `shortService`, `dataSync`, `specialUse`).
|
||||
`targetSdk = 37`.
|
||||
|
||||
`adb shell dumpsys location`, Pixel 9a, 21 d 7 h of uptime. **These are the
|
||||
`com.vitorpamplona.amethyst` rows** of the per-provider *Historical Aggregate
|
||||
Location Provider Data* block — not system-wide totals:
|
||||
|
||||
| provider | registration held | **active** | **foreground** | fixes |
|
||||
|---|---|---|---|---|
|
||||
| passive | 9 d 14 h 20 m | 8 h 26 m 14 s | 8 h 14 m 50 s | 107 |
|
||||
| network | 9 d 14 h 20 m | 8 h 26 m 13 s | 8 h 14 m 49 s | 95 |
|
||||
| fused | 9 d 14 h 20 m | 8 h 26 m 12 s | 8 h 14 m 48 s | 95 |
|
||||
| gps | 9 d 14 h 20 m | 8 h 26 m 11 s | 8 h 14 m 47 s | 98 |
|
||||
|
||||
Roughly **11 minutes of background-active location in three weeks**, and about
|
||||
100 delivered fixes per provider. With the app backgrounded at the time of the
|
||||
dump: `gps provider: service: ProviderRequest[OFF]`, `gps_hardware:
|
||||
mStarted=false`.
|
||||
|
||||
The cleanest assumption-free comparison: the ledger's **two-day** `location.ms`
|
||||
total (3.57 h + 7.13 h = 10.70 h) **exceeds the OS's three-week active total**
|
||||
(8 h 26 m) by 27 %. `location.ms` is not measuring what its label implies.
|
||||
|
||||
**One figure does not reconcile, and is left open.** Registration-held is
|
||||
9 d 14 h over 21 d 7 h ≈ 10.8 h/day, whereas `location.ms` averages 5.35 h/day
|
||||
across the two ledger days. If `location.ms` measured subscription existence
|
||||
these should agree; they are ~2× apart. Candidates: segments open at process
|
||||
death are lost (the pre-flush hook does not run on a kill, and the report shows
|
||||
6 process starts across the two days); the ledger covers 2 days while dumpsys
|
||||
spans 21 with different usage. Not investigated. It does not affect the
|
||||
conclusion below, which rests on `location.ms` versus OS-*active* time, not
|
||||
versus registration-held.
|
||||
|
||||
### How far the "no background location" claim generalises
|
||||
|
||||
This matters because the "no behaviour change" argument rests on it, so it is
|
||||
scoped rather than asserted universally:
|
||||
|
||||
- **API 29+ (Android 10 and up):** `ACCESS_BACKGROUND_LOCATION` gates background
|
||||
access, and for `targetSdk ≥ 29` an FGS additionally needs
|
||||
`foregroundServiceType="location"`. Amethyst has neither, so background
|
||||
registrations are suspended. This is the case the Pixel 9a measurement above
|
||||
covers.
|
||||
- **API 26–28 (Android 8–9):** `ACCESS_BACKGROUND_LOCATION` does not exist.
|
||||
Amethyst *can* receive background location there, throttled by the platform to
|
||||
a few updates per hour. The gate is a genuine, if small, improvement on these
|
||||
releases rather than a no-op.
|
||||
|
||||
So "structural" applies to API 29+; on 26–28 the change has real effect.
|
||||
|
||||
### What is actually wrong
|
||||
|
||||
1. **The meter lies.** `location.ms` measures how long a *subscription* existed,
|
||||
not how long anything listened. That is what made Finding 1 read as the
|
||||
top-priority battery bug.
|
||||
2. **The request is 4× redundant.** `LocationFlow.kt:55` iterates
|
||||
`locationManager.allProviders` and calls `requestLocationUpdates` on each —
|
||||
passive, network, fused **and** gps (the last tagged `HIGH_ACCURACY`) — every
|
||||
one at `@+10s0ms, minUpdateDistance=100.0`, to produce a **5 km** geohash.
|
||||
This burns during the 8 h 14 m the app genuinely is foreground.
|
||||
3. **The subscription is held for 45 % of device uptime** doing nothing, because
|
||||
`Eagerly` never lets go.
|
||||
4. **Every user is exposed**, since Products defaults to `AroundMe` and needs no
|
||||
opt-in.
|
||||
5. **Location may be entirely broken on Android 8–11** — see Hypothesis H1.
|
||||
|
||||
## Hypothesis H1 — location is dead below API 31 (unverified)
|
||||
|
||||
Through Android 11, AOSP's `getMinimumPermissionForProvider` required
|
||||
`ACCESS_FINE_LOCATION` for the `gps`, `passive` and `fused` providers; only
|
||||
`network` accepted `ACCESS_COARSE_LOCATION`. Approximate-location, which lets a
|
||||
coarse-only app request any provider and receive a fuzzed result, is an Android
|
||||
12 (API 31) change.
|
||||
|
||||
Amethyst holds coarse only. So on API 26–30 today's `allProviders` loop should
|
||||
throw `SecurityException` on three of the four providers. Two consequences:
|
||||
|
||||
- The throw escapes the `callbackFlow` builder → `.catch` in `LocationState` →
|
||||
`LackPermission`. Location would be **non-functional** on Android 8–11.
|
||||
- The builder aborting means `awaitClose` never runs, so `removeUpdates` is
|
||||
never called and any registration made before the throw **leaks** for the life
|
||||
of the process.
|
||||
|
||||
**The leak is iteration-order dependent, and may not occur at all.**
|
||||
`getLastKnownLocation` is permission-checked per provider too, and
|
||||
`LocationFlow.kt:56-68` calls it *before* `requestLocationUpdates` on each
|
||||
iteration. If a fine-only provider comes first in `allProviders` — `passive`
|
||||
does, in the common AOSP ordering — the throw lands before any registration
|
||||
exists: dead, but not leaking. A leak requires `network` to precede a fine-only
|
||||
provider.
|
||||
|
||||
`@SuppressLint("MissingPermission")` at `LocationFlow.kt:40` suppresses the lint
|
||||
warning, not the runtime check, so this would not have been caught statically.
|
||||
|
||||
**Not reproduced.** Verify before implementing, on the existing
|
||||
`Medium_Phone_API_26_8_` AVD: grant coarse only, open a screen that subscribes,
|
||||
and capture **both** the `SecurityException` *and* the actual
|
||||
`locationManager.allProviders` order (log it). The PR should claim only what that
|
||||
run observed — "location is dead on Android 8–11" and "registrations leak" are
|
||||
separate claims and the second may not hold.
|
||||
|
||||
The design below is written to be correct either way (§B excludes
|
||||
permission-incompatible providers by API level, and catches `SecurityException`
|
||||
per provider). If H1 holds, this change also **fixes location on Android 8–11**,
|
||||
which should be called out in the PR.
|
||||
|
||||
### H1 verification result (2026-07-30, Pixel 9a, API 37)
|
||||
|
||||
Partial. The API-level claim could **not** be tested on this hardware: API 31+
|
||||
grants coarse-only apps access to every provider, so no `SecurityException` can
|
||||
appear regardless of whether H1 is true. Only an API ≤ 30 image can settle it.
|
||||
|
||||
What *was* settled is the ordering, which decides the leak sub-claim.
|
||||
`dumpsys location` recent-events shows the same iteration order on every
|
||||
registration cycle across two days, all four sharing one registration id
|
||||
(`88A8E679`), confirming a single `LocationFlow` subscription:
|
||||
|
||||
```
|
||||
07-30 07:09:58.282: passive provider +registration .../88A8E679
|
||||
07-30 07:09:58.291: network provider +registration .../88A8E679
|
||||
07-30 07:09:58.293: fused provider +registration .../88A8E679
|
||||
07-30 07:09:58.299: gps provider +registration .../88A8E679
|
||||
```
|
||||
|
||||
`allProviders` yields **passive first**, and `passive` is one of the fine-only
|
||||
providers below API 31. So on Android 8–11 the throw would land on the first
|
||||
iteration, before any registration exists:
|
||||
|
||||
- "location is dead on Android 8–11" — **still unverified**, needs API ≤ 30.
|
||||
- "registrations leak" — **disproved for this ordering**. Dead, but not leaking.
|
||||
|
||||
The PR must not claim the leak. Caveat: the ordering is observed on API 37 and
|
||||
`getAllProviders()` could order differently on API 26.
|
||||
|
||||
**Unrelated but decisive "before" datum, same session:** with
|
||||
`mWakefulness=Dozing` (screen off, device dozing) and `MainActivity` sitting in
|
||||
`mLastPausedActivity`, Amethyst held **four** live registrations at
|
||||
`@+10s0ms / minUpdateDistance=100.0`. That is the state §A's gate exists to
|
||||
eliminate, captured on the owner's daily-driver device rather than an emulator.
|
||||
|
||||
## Goals
|
||||
|
||||
- Release the location registration whenever no activity is started.
|
||||
- Register on one appropriate, permission-compatible provider at an interval
|
||||
matched to the precision actually needed.
|
||||
- Make `location.ms` reflect real listening time, correctly, under concurrency.
|
||||
- No user-visible behaviour change to the "Around Me" feed or geohash chats.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Changing the Products `AroundMe` default (`AccountSettings.kt:256`). With the
|
||||
gate in place its cost is bounded to foreground use. Worth revisiting
|
||||
separately as a product decision.
|
||||
- Requesting `ACCESS_FINE_LOCATION` or `ACCESS_BACKGROUND_LOCATION`.
|
||||
- Findings 2–6 of the source analysis. Finding 2 (the relay reconnect storm,
|
||||
1.65 GB/day at a 75 % dial-failure rate) is the more likely explanation for
|
||||
the background battery drain and should be taken next.
|
||||
|
||||
## Design
|
||||
|
||||
### A. The gate
|
||||
|
||||
`LocationState` gains an `isForeground: StateFlow<Boolean>` parameter, wired in
|
||||
`AppModules.kt:251` from the existing `foregroundTracker` (`AppModules.kt:333`,
|
||||
registered at `Amethyst.kt:122`). `locationManager` is `by lazy`, so
|
||||
initialisation order is safe.
|
||||
|
||||
Today's `hasLocationPermission.transformLatest { … }` becomes a three-state gate
|
||||
over *permission × foreground*, applied identically to `geohashStateFlow` and
|
||||
`preciseGeohashStateFlow`:
|
||||
|
||||
| gate state | behaviour |
|
||||
|---|---|
|
||||
| no permission | emit `LackPermission` (unchanged) |
|
||||
| permitted, foreground | **R1**: emit `Loading` *only if* no `Success` is cached; then `emitAll(locationSource(…))` |
|
||||
| permitted, backgrounded | emit nothing; the registration is released and the `StateFlow` retains its last value |
|
||||
|
||||
**R1 is a requirement, not an improvement.** `AroundMeFeedFlow.convert` collapses
|
||||
to `geotags = emptySet()` for anything that is not `Success`. Without R1 the gate
|
||||
would make the "Around Me" feed flash empty on **every** return to foreground — a
|
||||
new, frequent, user-visible regression introduced by this change. (It also fixes
|
||||
the same flash on permission grant, which exists today.)
|
||||
|
||||
**R1 corollary: the `NoPermission` branch must not clear the cache.** Today's
|
||||
code emits `LackPermission` without touching `latestLocation`
|
||||
(`LocationState.kt:94-96`), and that stays. Clearing it is superficially
|
||||
attractive — a revoked permission arguably should not leave a fix readable — but
|
||||
consumers already see `LackPermission` from the `StateFlow`; `latestLocation` is
|
||||
private and its only jobs are seeding `stateIn` and deciding whether `Loading` is
|
||||
emitted. Clearing it would therefore buy no privacy and would cost an
|
||||
empty-feed flash on every permission flap, which is precisely what R1 exists to
|
||||
prevent. The cache is in-memory and dies with the process regardless.
|
||||
|
||||
The `.catch` branch **does** clear the cache, and keeps doing so. That asymmetry
|
||||
looks arbitrary next to the paragraph above, so to be explicit: it is inherited,
|
||||
not introduced. Both branches preserve today's behaviour exactly
|
||||
(`LocationState.kt:87-91` clears on failure, `:94-96` does not clear on missing
|
||||
permission). This corollary argues against *adding* a clear, not for removing
|
||||
the existing one — changing it would be an unmotivated behaviour change. The
|
||||
asymmetry is also defensible on its own terms: a source that failed mid-stream
|
||||
says something about the fix's provenance, whereas a permission known to be
|
||||
absent says nothing about a fix already taken.
|
||||
|
||||
**R2 — grace period on the background edge.** The gate must delay the
|
||||
`foreground → background` transition by **5 s** before tearing down. Without it a
|
||||
one-second app switch destroys and rebuilds the registration, including a full
|
||||
`getLastKnownLocation` sweep, so a user flipping between apps pays more than the
|
||||
steady state. 5 s matches the existing `WhileSubscribed(5000)` and is the same
|
||||
intent. The `background → foreground` edge is **not** delayed.
|
||||
|
||||
Mechanism, stated because the obvious operator is the wrong one: `debounce(5000)`
|
||||
delays both edges, and the duration-selector overload that would allow an
|
||||
asymmetric delay is `@FlowPreview`. Use `transformLatest`, already in this file
|
||||
and already opted into via `@OptIn(ExperimentalCoroutinesApi::class)`:
|
||||
|
||||
```kotlin
|
||||
isForeground.transformLatest { fg ->
|
||||
if (!fg) delay(BACKGROUND_GRACE_MS)
|
||||
emit(fg)
|
||||
}
|
||||
```
|
||||
|
||||
`transformLatest` cancels the pending `delay` if foreground returns first, which
|
||||
is exactly the stated semantics, with no preview opt-in.
|
||||
|
||||
**R3 — the retained-value contract.** The "emit nothing" branch is what keeps
|
||||
this behaviour-neutral: `stateIn` holds the last `Success`, so the ~60
|
||||
synchronous `.value` reads across the feed filters
|
||||
(`HomeNewThreadFeedFilter.kt`, `VideoFeedFilter.kt`,
|
||||
`DiscoverLongFormFeedFilter.kt`, …) keep seeing the last known geohash. A 5 km
|
||||
cell does not meaningfully decay while backgrounded.
|
||||
|
||||
**R4 — memory visibility.** `latestLocation` and `latestPreciseLocation`
|
||||
(`LocationState.kt:63-64`) are plain `var`s today, used only as `stateIn` initial
|
||||
values. R1 promotes them to control flow, read from a different coroutine than
|
||||
the `onEach` that writes them. They must become `@Volatile` (or
|
||||
`MutableStateFlow`).
|
||||
|
||||
**Rejected alternative:** switching `FeedTopNavFilterState.flow` from `Eagerly`
|
||||
to `WhileSubscribed`. Roughly 60 call sites read
|
||||
`account.live*FollowLists.value` synchronously rather than collecting; under
|
||||
`WhileSubscribed` those reads would silently serve a stale or initial value
|
||||
whenever no collector happened to be active. That is a correctness regression,
|
||||
not a battery fix.
|
||||
|
||||
**Rejected alternative:** gating only at the `AppModules` wiring point
|
||||
(`geolocationFlow = { … }`). Smaller diff, but it leaves the raw
|
||||
`geohashStateFlow` as a loaded gun for the next eager consumer, does nothing for
|
||||
`preciseGeohashStateFlow`, and introduces a second `StateFlow` layer over the
|
||||
same data.
|
||||
|
||||
### B. Request shape
|
||||
|
||||
`LocationFlow.get` registers on **one** provider, chosen by a ladder over
|
||||
**provider existence and permission compatibility** — both static facts:
|
||||
|
||||
```
|
||||
chooseProviders(sdkInt, hasFine, exists) -> List<String>:
|
||||
API 31+ or hasFine → [FUSED, NETWORK, GPS, PASSIVE] filtered by exists
|
||||
API < 31, coarse → [NETWORK] filtered by exists (see H1)
|
||||
```
|
||||
|
||||
It returns the **ordered candidate list**, not a single choice, because the
|
||||
per-provider `SecurityException` fall-through below needs somewhere to fall to.
|
||||
An empty list means no compatible provider exists.
|
||||
|
||||
**The ladder deliberately does not consult `isProviderEnabled`.** Today's code
|
||||
registers regardless of enabled state, and such a registration goes live by
|
||||
itself when the user enables location — including from the quick-settings shade
|
||||
without leaving the app, which is exactly what someone does after seeing "Around
|
||||
Me" empty. A guard evaluated once at subscription start would lose that, and the
|
||||
foreground-transition restart does not cover the in-app path. Selecting on
|
||||
existence keeps the property with no `PROVIDERS_CHANGED_ACTION` receiver. If
|
||||
field reports show dead feeds on devices where the chosen provider exists but is
|
||||
disabled while another is enabled, adding that receiver is the follow-up.
|
||||
|
||||
`requestLocationUpdates` is wrapped in a per-provider `SecurityException` catch
|
||||
that falls through to the next rung, so H1 cannot abort the builder and leak
|
||||
registrations regardless of how the AOSP check actually behaves.
|
||||
|
||||
**When no provider can be registered** — the candidate list was empty, or every
|
||||
rung threw — `LocationFlow` **throws** `SecurityException`. It cannot emit
|
||||
`LackPermission`: the seam is `(Long, Float) -> Flow<Location>`, and
|
||||
`LackPermission` is a `LocationState.LocationResult`, which `LocationFlow` has no
|
||||
way to express. Throwing routes it through the `.catch` already present in
|
||||
`LocationState` (`LocationState.kt:87-91`, `:126-130`), which sets
|
||||
`latestLocation = LackPermission` and emits it — the existing, unchanged path.
|
||||
|
||||
Throwing covers **both** failure cases, and it subsumes R5's `registered`-flag
|
||||
guard: the throw happens before the acquire, so `onListening(true)` cannot fire
|
||||
without a live registration and no separate flag is needed. That is only half of
|
||||
R5's pairing, though — see R5 for the release half, which the throw does **not**
|
||||
cover and which needs `try`/`finally`.
|
||||
|
||||
**Stated decision: `LackPermission` stays conflated with "no usable provider".**
|
||||
That value renders `R.string.lack_location_permissions` — "No Location
|
||||
Permissions" — at `DisplayLocationObserver.kt:49` and `FeedFilterSpinner.kt:224`,
|
||||
which is wrong for a coarse-only pre-31 device that has no `network` provider.
|
||||
The conflation is accepted rather than introduced: if H1 holds, today's
|
||||
`SecurityException` already lands in the same `.catch` and shows the same wrong
|
||||
message. Adding an `Unavailable` state would ripple through four UI `when`s plus
|
||||
`LocationState` (10 references across 5 files) and belongs with the H1 fix
|
||||
messaging, not here. Recorded as a follow-up.
|
||||
|
||||
The `getLastKnownLocation` seed stays a sweep across all providers, taking the
|
||||
freshest result. It requires no registration and is what makes the first geohash
|
||||
appear immediately rather than after a fix.
|
||||
|
||||
`MIN_TIME` / `MIN_DISTANCE` split into two profiles, passed per call:
|
||||
|
||||
| flow | precision | interval / distance | provider set |
|
||||
|---|---|---|---|
|
||||
| `geohashStateFlow` | `KM_5_X_5` | 10 s / 100 m → **60 s / 500 m** | 4 → 1 |
|
||||
| `preciseGeohashStateFlow` | `BUILDING` (8 chars) | 10 s / 100 m (kept) | 4 → 1 |
|
||||
|
||||
Both rows change: the ladder narrows the precise flow's provider set too, and
|
||||
below API 31 that means `network` only, no GPS. Academic while the app holds
|
||||
coarse only (see Follow-ups), but it is not "unchanged".
|
||||
|
||||
At 120 km/h a 5 km cell takes 2.5 minutes to cross, so 60 s / 500 m has no
|
||||
observable effect on the feed.
|
||||
|
||||
**Rejected alternative — one shared source at the fine profile,** deriving the
|
||||
coarse geohash by prefix truncation. It halves registrations and removes the need
|
||||
for `RefCountedSession` entirely, but it upgrades the **common** case — the
|
||||
"Around Me" feed alone, which is always on via the Products default — from
|
||||
60 s/500 m to 10 s/100 m. That trades the change's main win for a rarer one.
|
||||
|
||||
**Rejected alternative — one shared source whose profile tracks the finest
|
||||
active subscriber.** Recovers the above and is the best of the three on both
|
||||
axes, but it is refcounting with the counter moved from the meter into the
|
||||
request path, for a benefit bounded by how often the two flows overlap. They
|
||||
overlap only while one of three composable-scoped, foreground-only screens is
|
||||
open (`GeohashChatScreen`, `NewGeohashChatScreen`,
|
||||
`GeohashLocationPickerDialog`). Not worth the machinery; revisit if that changes.
|
||||
|
||||
### C. The meter
|
||||
|
||||
`AppModules.kt:251` hands both flows the same non-refcounted
|
||||
`SessionTimeIntegrator`, so `setActive(false)` from either closes the segment
|
||||
while the other is still listening. Both can be live at once — the "Around Me"
|
||||
feed plus an open geohash chat.
|
||||
|
||||
**R5 — the hook moves inside `LocationFlow`, and both edges are paired.** Today
|
||||
`onListening(true)` is an `onStart` on the flow returned by `LocationFlow.get`,
|
||||
so it fires on *collection* whether or not anything was registered — meaning a
|
||||
device with no usable provider accrues `location.ms` with nothing listening,
|
||||
reintroducing the exact defect this section exists to fix. The hook must instead
|
||||
fire from inside the `callbackFlow`, after `requestLocationUpdates` returns
|
||||
without throwing, and again on the way out.
|
||||
|
||||
The obvious "way out" is `awaitClose`, and that would introduce a worse bug than
|
||||
it fixes — twice over. First, `awaitClose` runs on every normal
|
||||
completion, including one where no rung ever registered, so it would fire an
|
||||
**unpaired** `onListening(false)`. With R6 that does not merely under-count — it
|
||||
decrements a holder it never acquired, stealing another flow's. Concretely:
|
||||
`geohashStateFlow` registers (`holders = 1`), `preciseGeohashStateFlow` fails to
|
||||
register and closes (`holders = 0`), and the session latches off while the coarse
|
||||
flow is still listening. `coerceAtLeast(0)` does not help; the count never went
|
||||
negative.
|
||||
|
||||
Second — and this is the one that survives fixing the first — `awaitClose` also
|
||||
fails to run at all on some paths that *did* register. See below.
|
||||
|
||||
The pair therefore has to be guaranteed from **both** ends, and the two ends need
|
||||
different mechanisms.
|
||||
|
||||
*No acquire without a registration* is §B's **throw**: if no rung registers, the
|
||||
builder throws before reaching the acquire at all.
|
||||
|
||||
*No acquire without a release* needs `try`/`finally`, **not** `awaitClose`. This
|
||||
is the subtlest point in the document, so the justification below is the one that
|
||||
was **demonstrated**, not the one that sounds most obvious.
|
||||
|
||||
Anything between the acquire and `awaitClose` that unwinds skips cleanup parked
|
||||
inside `awaitClose`, because `awaitClose` is never reached to register it. The
|
||||
registration then leaks and the refcount sticks at ≥ 1 for the life of the
|
||||
process, so `location.ms` accrues forever with nothing listening — this exact
|
||||
defect, arrived at from the other direction, and unrecoverable once hit.
|
||||
|
||||
The **proven** path is the seed throwing a non-cancellation exception:
|
||||
`getLastKnownLocation` is a binder call and can fail. The regression test
|
||||
`releasesTheRegistrationWhenTheSeedThrows` provokes exactly this and was watched
|
||||
failing against an `awaitClose`-only implementation
|
||||
(`expected:<[true, false]> but was:<[true]>`).
|
||||
|
||||
A cancellation during the seed is *in principle* a second such path, since `send`
|
||||
is a suspending call. Recorded honestly: **this one could not be reproduced.**
|
||||
Two attempts during implementation both produced tests that passed against a
|
||||
deliberately broken implementation, because `callbackFlow`'s channel is buffered,
|
||||
so `send` returns without suspending and never observes the cancel. Do not treat
|
||||
the cancellation story as the reason for the `try`/`finally`; a future reader who
|
||||
tries to reproduce it, fails, and concludes the guard is unnecessary would
|
||||
reintroduce the leak.
|
||||
|
||||
```kotlin
|
||||
var registered: String? = null
|
||||
for (provider in candidates) {
|
||||
try {
|
||||
locationManager.requestLocationUpdates(provider, minTimeMs, minDistanceM, callback, Looper.getMainLooper())
|
||||
registered = provider
|
||||
break
|
||||
} catch (e: SecurityException) { /* next rung */ }
|
||||
}
|
||||
if (registered == null) throw SecurityException("no usable location provider")
|
||||
|
||||
onListening?.invoke(true) // cannot fire without a registration
|
||||
try {
|
||||
freshestLastKnownLocation(providers)?.let { send(it) } // suspends — cancellable
|
||||
awaitClose { } // only to satisfy callbackFlow's contract
|
||||
} finally {
|
||||
locationManager.removeUpdates(callback)
|
||||
onListening?.invoke(false) // cannot be skipped
|
||||
}
|
||||
```
|
||||
|
||||
`onListening?.invoke(true)` sits immediately before the `try`, with no suspension
|
||||
between them, so the acquire cannot happen outside the block that guarantees its
|
||||
release.
|
||||
|
||||
`trySend` for the seed would also close this particular hole, being
|
||||
non-suspending. It is rejected because it leaves the invariant resting on nobody
|
||||
adding a suspending call to that block later — vigilance rather than
|
||||
impossibility, which is the standard the rest of R5 is held to.
|
||||
|
||||
**R6 — refcounting.** An `AtomicInteger` beside the `setActive` call is not
|
||||
sufficient: two threads can leave the counter at 1 while the last
|
||||
`setActive(false)` lands after the `setActive(true)`, latching the session off.
|
||||
The count and the transition must move under one lock. New class in
|
||||
`service/resourceusage/`:
|
||||
|
||||
```kotlin
|
||||
class RefCountedSession(private val setSessionActive: (Boolean) -> Unit) {
|
||||
private val lock = Any()
|
||||
private var holders = 0
|
||||
|
||||
fun setActive(active: Boolean) =
|
||||
synchronized(lock) {
|
||||
holders = if (active) holders + 1 else (holders - 1).coerceAtLeast(0)
|
||||
setSessionActive(holders > 0)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
It takes the setter as a lambda rather than a `SessionTimeIntegrator` because
|
||||
that is all it needs, and because constructing a real integrator in a unit test
|
||||
would drag in a `ResourceUsageAccountant`, a `ResourceUsageStore` and a temp
|
||||
file to observe one boolean.
|
||||
|
||||
`AppModules` wires `RefCountedSession(locationSession::setActive)` and passes
|
||||
`onListening = { locationRefCount.setActive(it) }`. The outer
|
||||
lock serialises entry into `SessionTimeIntegrator.setActive`, whose own lock is
|
||||
then nested but never acquired in the reverse order, so there is no deadlock.
|
||||
`coerceAtLeast(0)` guards an unmatched release.
|
||||
|
||||
### What `location.ms` means after this change
|
||||
|
||||
Stated plainly, because the finding that opened this spec is "the meter lies"
|
||||
and the next reader should not over-trust the fixed number the way the last one
|
||||
over-trusted the broken one:
|
||||
|
||||
> `location.ms` measures **how long a location registration was held while the
|
||||
> app was in the foreground**. It is not radio-on time and not an energy
|
||||
> figure. A `network`-provider registration at 60 s costs close to nothing; a
|
||||
> `gps` registration at 10 s costs a great deal. The counter cannot tell them
|
||||
> apart.
|
||||
|
||||
Reading it as a battery signal requires knowing which provider was chosen —
|
||||
which the ledger does not record. Recording the chosen provider as a separate
|
||||
counter is a possible follow-up.
|
||||
|
||||
## Testing
|
||||
|
||||
JVM unit tests — JUnit + MockK + `kotlinx-coroutines-test`, no Robolectric,
|
||||
alongside the existing `service/resourceusage/ResourceUsageLedgerTest.kt`.
|
||||
|
||||
**Gate.** `LocationState` gains `locationSource: (Long, Float) -> Flow<Location>`,
|
||||
defaulting to `LocationFlow(context.getSystemService(…) as LocationManager)::get`
|
||||
(see *Registration pairing* for why `LocationFlow` now takes the manager rather
|
||||
than the `Context`). That is the seam:
|
||||
`Location.toGeoHash` is `GeoHash.encode(lat, lon, chars)` from quartz — pure
|
||||
Kotlin — and `unitTests.isReturnDefaultValues = true` is already set, so a
|
||||
`mockk<Location>` with stubbed `latitude`/`longitude` suffices. Against a
|
||||
counting fake source:
|
||||
|
||||
- backgrounded + permitted → source never subscribed
|
||||
- foreground + permitted → subscribed exactly once
|
||||
- foreground → background → subscription released after the R2 grace period,
|
||||
last `Success` still readable via `.value`
|
||||
- background edge shorter than the grace period → subscription **not** torn down
|
||||
- return to foreground with a cached `Success` → **no** `Loading` emission (R1)
|
||||
- return to foreground with no cached fix → `Loading` first
|
||||
- permission revoked → `LackPermission` regardless of foreground state
|
||||
|
||||
**Provider ladder.** Extracted as a pure function
|
||||
`chooseProviders(sdkInt: Int, hasFine: Boolean, exists: (String) -> Boolean):
|
||||
List<String>` so §B is covered rather than sitting below the seam. Cases: rungs
|
||||
returned in order; missing rungs filtered out; API < 31 coarse-only yields
|
||||
`[network]`; API < 31 with fine yields the full ladder; no compatible provider
|
||||
yields an empty list.
|
||||
|
||||
`hasFine` is **always `false` in production** — the non-goals rule out ever
|
||||
requesting `ACCESS_FINE_LOCATION`. It is a parameter rather than a constant so
|
||||
that the function is total over the permission axis and the API < 31 branch can
|
||||
be tested from both sides, not because fine access is anticipated. If that
|
||||
changes, the ladder is already correct.
|
||||
|
||||
R5 sits below the `locationSource` seam, so a fake source never fires it. It gets
|
||||
its own tests against a mocked `LocationManager` — see *Registration pairing*
|
||||
below.
|
||||
|
||||
**Meter.** `RefCountedSession`: overlapping holders keep the session open;
|
||||
balanced pairs close it; an unmatched release does not drive the count negative.
|
||||
|
||||
Note what this class **cannot** do: it cannot distinguish an unpaired release
|
||||
from a legitimate one, so `acquire → unpaired release` closes the session even
|
||||
while another holder is listening. That is precisely the R5 bug, and
|
||||
`coerceAtLeast(0)` is no defence against it. **The pairing guarantee belongs to
|
||||
`LocationFlow`, not here** — which is why it needs its own test below.
|
||||
|
||||
**Registration pairing (R5).** To make this testable rather than device-only,
|
||||
`LocationFlow` takes a `LocationManager` instead of a `Context`
|
||||
(`LocationFlow(context)` → `LocationFlow(locationManager)`; the caller in
|
||||
`LocationState` does the `getSystemService` lookup). A `mockk<LocationManager>`
|
||||
then covers:
|
||||
|
||||
- every rung throws `SecurityException` → the flow throws and `onListening` fires
|
||||
**neither** edge
|
||||
- `chooseProviders` returns an empty list → same: throws, neither edge
|
||||
- an earlier rung throws and a later one succeeds → registration falls through,
|
||||
exactly one `true`
|
||||
- a successful registration → exactly one `true`, and exactly one `false` plus
|
||||
`removeUpdates` on cancellation
|
||||
- **cancellation mid-seed**, while the `getLastKnownLocation` sweep is in flight
|
||||
→ both edges still fire and `removeUpdates` is still called. This is the case
|
||||
the `try`/`finally` exists for; without it the test fails by hanging the
|
||||
refcount at 1 rather than by throwing, so assert on the edges, not on the
|
||||
absence of an exception.
|
||||
These cover the *semantics* only — the two-thread interleaving that motivates the
|
||||
lock is made unobservable by the lock itself and is not reproduced by any test
|
||||
here.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
On device, re-running the measurement above:
|
||||
|
||||
- **Backgrounded:** after the 5 s grace period, `adb shell dumpsys location`
|
||||
shows no `com.vitorpamplona.amethyst` entry under any provider's `listeners:`,
|
||||
and a `-registration` in the recent-events log.
|
||||
- **Foregrounded:** **one registration per actively-collected flow — at most
|
||||
two**, and one in the steady state where only the "Around Me" feed is live
|
||||
(§C exists precisely because the two flows may overlap). Not four. The coarse
|
||||
registration reads `@+60s0ms` / `minUpdateDistance=500.0` rather than
|
||||
`@+10s0ms` / `100.0`.
|
||||
- The historical aggregates are cumulative since boot; compare deltas across a
|
||||
foreground/background cycle, not absolute totals.
|
||||
- **Invariant:** a subsequent in-app Resource Usage Report shows
|
||||
|
||||
```
|
||||
location.ms ≤ app.fgms + 5 s × (background transitions)
|
||||
```
|
||||
|
||||
Both counters are driven by the same `foregroundTracker.isForeground` flow, so
|
||||
without R2 this would hold exactly. R2 is deliberately the error term: the
|
||||
registration really *is* live during the grace period, so counting it is the
|
||||
honest reading, and a stated fudge factor beats an invariant quietly known to
|
||||
be false. The term is not negligible — the source report shows 6 process
|
||||
starts across two days, and app switches are far more frequent than that — so
|
||||
writing `location.ms ≤ app.fgms` would guarantee that the first person to
|
||||
check it files a bug against this change.
|
||||
|
||||
Second caveat: Finding 4 of the source analysis suspects `app.fgms` of
|
||||
under-reporting, so a violation beyond the grace term indicts that counter
|
||||
rather than this one.
|
||||
- If H1 holds: location works on the API 26 AVD after the change and did not
|
||||
before.
|
||||
|
||||
### Verified on device (2026-07-30, Pixel 9a / Android 17, API 37)
|
||||
|
||||
Measured against the `benchmark` variant — `initWith(release)`, so R8-minified with
|
||||
the shipping proguard rules, installed as `com.vitorpamplona.amethyst.benchmark`
|
||||
beside the untouched Play install. The Play install was force-stopped for the
|
||||
duration so its own (unfixed) registrations could not be mistaken for these.
|
||||
|
||||
| criterion | before | after |
|
||||
|---|---|---|
|
||||
| registrations, foreground | **4** (passive, network, fused, gps) | **1** (fused) |
|
||||
| request profile | `@+10s0ms HIGH_ACCURACY`, `minUpdateDistance=100.0` | `@+1m0s0ms BALANCED`, `minUpdateDistance=500.0` |
|
||||
| registrations, backgrounded | **4**, held while `mWakefulness=Dozing` | **0** |
|
||||
|
||||
Event trace for one full cycle, process alive throughout (pid 28682):
|
||||
|
||||
```
|
||||
17:57:00.117 +registration fused …/40F5A6D7 @+1m0s0ms BALANCED, minUpdateDistance=500.0
|
||||
17:57:33.894 -registration fused …/40F5A6D7 ← HOME pressed, released after the grace
|
||||
17:58:05.798 +registration fused …/091EDA96 @+1m0s0ms BALANCED, minUpdateDistance=500.0
|
||||
(HOME then reopen within 2 s — no -/+ pair; 091EDA96 survives)
|
||||
```
|
||||
|
||||
- **§A gate** — zero registrations while backgrounded, with the process still
|
||||
alive. That is the state the change exists to create; before, four
|
||||
registrations survived screen-off and doze.
|
||||
- **§B request shape** — one provider, top of the ladder (`fused`), at exactly
|
||||
`COARSE_MIN_TIME` / `COARSE_MIN_DISTANCE`. The OS tags it `(COARSE)` and
|
||||
coalesces the effective service request to `@+10m0s0ms LOW_POWER`.
|
||||
- **R2 grace period** — a sub-grace app switch produced **no** teardown/rebuild
|
||||
pair, so a brief switch no longer costs a re-registration and a fresh
|
||||
`getLastKnownLocation` sweep.
|
||||
|
||||
**The OS aggregate after four foreground/background cycles is the headline
|
||||
result**, because it is the same counter shape `location.ms` measures:
|
||||
|
||||
```
|
||||
com.vitorpamplona.amethyst.benchmark:
|
||||
min/max interval = 60s/60s
|
||||
total/active/foreground duration = +2m33s542ms / +2m33s456ms / +2m33s531ms
|
||||
locations = 4
|
||||
```
|
||||
|
||||
Total ≈ active ≈ foreground, all three within 90 ms — against the Play install's
|
||||
`9d14h20m / 8h26m / 8h14m`, where registration was held for 45 % of uptime while
|
||||
only 1.7 % was active. The four foreground windows sum to 153.6 s, matching the
|
||||
aggregate exactly, so nothing is held outside them. Registration-held time now
|
||||
*equals* foreground time, which is precisely what makes `location.ms` honest: the
|
||||
counter measures registration lifetime, and that quantity is no longer divorced
|
||||
from reality.
|
||||
|
||||
**A fix arrives within milliseconds of every re-registration**, which bounds the
|
||||
staleness the coarser profile was feared to introduce:
|
||||
|
||||
```
|
||||
17:57:00.117 +registration → 17:57:00.127 delivered location[1] (10 ms)
|
||||
17:58:05.798 +registration → 17:58:05.802 delivered location[1] ( 4 ms)
|
||||
17:59:09.965 +registration → 17:59:09.967 delivered location[1] ( 2 ms)
|
||||
18:00:09.206 +registration → 18:00:09.213 delivered location[1] ( 7 ms)
|
||||
```
|
||||
|
||||
The `fused` provider hands over its cached fix on registration, so the window in
|
||||
which a returning user could act on a stale geohash is milliseconds, not the 60 s
|
||||
poll interval. Caveat: that cache is warm on this device because Maps and GMS
|
||||
keep it fresh; on a device with no other location consumer it could be colder,
|
||||
which is what `freshestLastKnownLocation` exists to cover.
|
||||
|
||||
**Side-by-side A/B, same device, same instant, both clients backgrounded and
|
||||
running.** The unmodified release client (1.13.1, installed via Obtainium, pid
|
||||
5552) and the benchmark build of this branch (pid 28682) were sampled together:
|
||||
|
||||
```
|
||||
com.vitorpamplona.amethyst/B7B299BE {bg, na} (COARSE) Request[PASSIVE, minUpdateDistance=100.0] (inactive)
|
||||
com.vitorpamplona.amethyst/B7B299BE {bg, na} (COARSE) Request[@+10m LOW_POWER, minUpdateDistance=100.0] (inactive)
|
||||
com.vitorpamplona.amethyst/B7B299BE {bg, na} (COARSE) Request[@+10m LOW_POWER, minUpdateDistance=100.0] (inactive)
|
||||
com.vitorpamplona.amethyst/B7B299BE {bg, na} (COARSE) Request[@+10m LOW_POWER, minUpdateDistance=100.0] (inactive)
|
||||
← com.vitorpamplona.amethyst.benchmark: no rows at all
|
||||
```
|
||||
|
||||
Four held registrations versus zero. Note the release client's rows are all
|
||||
`{bg, na} … (inactive)`: the OS has throttled the effective interval to 10
|
||||
minutes and suspended delivery, exactly as §"What the device reports" describes —
|
||||
but the **registration is still held**, and registration-held time is precisely
|
||||
what `location.ms` counts. That is the inflation, visible in one frame.
|
||||
|
||||
Naming note for anyone re-reading the numbers above: both artifacts are `play`
|
||||
**flavor** builds and differ only by buildType, so "the Play install" is an
|
||||
ambiguous label. The unmodified client here is the *release* build, and on this
|
||||
device it came from Obtainium rather than Google Play.
|
||||
|
||||
### Ledger invariant confirmed (2026-07-31, benchmark client, in-app report)
|
||||
|
||||
The acceptance criterion `location.ms ≤ app.fgms + 5 s × transitions` now checks
|
||||
out against accumulated data:
|
||||
|
||||
| | `location.ms` | `app.fgms` | ratio |
|
||||
|---|---|---|---|
|
||||
| release client 1.13.0, day 20663 (before) | 25,660,172 | 9,147 | **2,805×** |
|
||||
| benchmark, day 20664 (permission granted mid-day) | 3m7.3s | 11m31.0s | 0.27× |
|
||||
| benchmark, **day 20665** (granted all day) | **2m10.8s** | **1m56.9s** | **1.12×** |
|
||||
|
||||
Day 20665 is the clean case: `location.ms` exceeds `app.fgms` by 13.9 s, which
|
||||
requires ≥ 3 background transitions to fall inside the grace allowance — met by
|
||||
the report navigation plus an `am start`. Day 20664 independently reconciles with
|
||||
the `dumpsys` measurement: 2m33.5s of OS registration-held + 4 × 5 s grace =
|
||||
~2m53.5s predicted, 3m7.3s actual, the residual being foreground use after the
|
||||
measurement ended. **The ledger and the OS now agree**, where before they were
|
||||
irreconcilable (10.7 h ledger vs 8h26m OS-active over three weeks).
|
||||
|
||||
**Limitation:** this is not a within-package before/after. `location.ms` is
|
||||
absent from days 20648–20663 because the benchmark client had location permission
|
||||
*denied* until 2026-07-30; the "before" is no data, not inflated data.
|
||||
|
||||
### The same data closes the battery question
|
||||
|
||||
Over days 20659–20665 on this device: **5m18s** of location listening against
|
||||
**596 pp** of background battery drain (~85 pp/day). Location cannot be a
|
||||
meaningful contributor at that ratio — Finding 1 is settled, and not in the
|
||||
direction the original analysis assumed.
|
||||
|
||||
Three consumers visible in the same report, none of them location:
|
||||
|
||||
- `service.alwayson.ms` ≈ **23.9 h/day** (148.9 h over 7 days) — an always-on
|
||||
foreground service running essentially continuously. Largest structural
|
||||
difference from a stock client; worth confirming it is deliberately enabled.
|
||||
- **Finding 2, unchanged.** 3,732 relay-hours over 7 days. Day 20664 alone:
|
||||
9,831 successful dials against 25,133 failures = **71.9 %**, matching the
|
||||
original report's 75 %.
|
||||
- **Finding 4, now on cellular.** Day 20664 `net.other.mobile.bg.activems =
|
||||
52,392,613` — **14.6 h** of background mobile active time with **0 requests and
|
||||
0 bytes**. The three-moment `isForeground()` sampling, exactly as diagnosed, so
|
||||
the fg/bg split in this report still cannot be trusted.
|
||||
|
||||
Caveat: the benchmark client's round-the-clock always-on service makes its
|
||||
battery figures non-comparable to a stock install. The relay and `net.other`
|
||||
figures do match the release client's original report closely.
|
||||
|
||||
### Smoke test on a clean install (2026-07-31, benchmark client)
|
||||
|
||||
Uninstalled and reinstalled from branch HEAD so the account, ledger and
|
||||
permission grant all started empty — which is what makes the first-grant and
|
||||
empty-cache paths reachable. UID changed 10805 → 10806, cleanly separating the
|
||||
new data.
|
||||
|
||||
- **R1 — no `Loading` flash on return to foreground: PASS.** Observed directly:
|
||||
granted the permission, set a feed to Around Me, backgrounded for 10 s,
|
||||
reopened — the geohash was present immediately with no blank feed. This was the
|
||||
last open acceptance criterion on the branch and the only one no test could
|
||||
cover. `dumpsys` shows why it works: the fix is delivered 1–6 ms after each
|
||||
re-registration.
|
||||
- **Precise profile live and distinct: PASS.** Geohash screens produce
|
||||
`@+10s0ms BALANCED, minUpdateDistance=100.0`, and the aggregate's `min/max
|
||||
interval` moved from `60s/60s` to `10s/60s`. Both profiles are real in
|
||||
production, not just in the unit tests.
|
||||
- **Refcount under concurrent flows: PASS.** The coarse registration `766C58A4`
|
||||
came up at 15:49:10 and survived **four complete precise-flow cycles**
|
||||
untouched (15:49:41–46, 15:50:47–52, 15:52:07–15:53:05, 15:53:17–22), with
|
||||
both live simultaneously during the first. Opening and closing geohash chats
|
||||
never closes the "Around Me" registration — `RefCountedSession` doing on a real
|
||||
device what `LocationLedgerCompositionTest` asserts.
|
||||
- **No hang in the location picker.** Every precise registration received a fix
|
||||
within ~1 ms; none stuck open. That path does
|
||||
`preciseGeohashStateFlow.first { it is Success }`, which would hang rather than
|
||||
crash if the gate failed to yield.
|
||||
- **Aggregate:** total 6m7.553s / active 6m7.401s (152 ms apart) / foreground
|
||||
5m49.441s. Foreground trails total by 18.1 s across ~7 background transitions,
|
||||
≈ 2.6 s each — the grace period, visible at a scale where it is easy to check.
|
||||
|
||||
**Measurement note:** counting listeners with `grep -c` on the package name is
|
||||
unreliable — the `service: ProviderRequest[…]` summary line also names the
|
||||
package when it is the only requester, inflating the count by one. List the rows
|
||||
and exclude that line instead. A count of zero is unambiguous either way.
|
||||
|
||||
Still not verified: nothing on the gate itself. The `location.ms ≤ app.fgms +
|
||||
5 s × transitions` invariant was separately confirmed from accumulated ledger
|
||||
data (see above); the clean install reset that counter, so it will need another
|
||||
day of use to re-check at scale.
|
||||
|
||||
## Follow-ups (not in this change)
|
||||
|
||||
- **`preciseGeohashStateFlow` is not actually building-level.** With only
|
||||
`ACCESS_COARSE_LOCATION`, Android fuzzes every fix to roughly a 3 km grid, so
|
||||
the 8-char geohash and the location chat channels built on it are far coarser
|
||||
than they claim (`LocationState.kt:104-141`, `GeohashChatScreen.kt:165`,
|
||||
`NewGeohashChatScreen.kt:309`). The profile is kept intact here so the intent
|
||||
survives if the app ever requests `ACCESS_FINE_LOCATION`; whether to request
|
||||
it, or to stop advertising building-level precision, is a separate decision.
|
||||
- **`GeohashChatScreen.kt:161-163` is a one-way permission latch** — it calls
|
||||
`setLocationPermission(true)` inside an `if (isGranted)` rather than passing
|
||||
the boolean, as every other caller does (`LoggedInPage.kt:144`,
|
||||
`LocationAsHash.kt:64`, `NewGeohashChatScreen.kt:285`,
|
||||
`GeohashLocationPickerDialog.kt:270`). Once set, a revoked permission is never
|
||||
reflected back into the shared `LocationState`. Small, in the blast radius,
|
||||
and cheap.
|
||||
- **An `Unavailable` `LocationResult`**, distinct from `LackPermission`, so a
|
||||
device with no usable provider stops being told "No Location Permissions" when
|
||||
it has them. Ten references across five files
|
||||
(`DisplayLocationObserver.kt`, `FeedFilterSpinner.kt`, `HomeScreen.kt`,
|
||||
`NewGeohashChatScreen.kt`, `LocationState.kt`). Belongs with the H1 fix
|
||||
messaging — see the stated decision in §B.
|
||||
- Recording the chosen provider as a ledger counter, so `location.ms` can be
|
||||
read as a cost signal.
|
||||
- Whether Products should still default to `TopFilter.AroundMe`.
|
||||
- Finding 2 — the relay reconnect storm.
|
||||
@@ -105,6 +105,7 @@ import com.vitorpamplona.amethyst.service.resourceusage.HttpUsageMeter
|
||||
import com.vitorpamplona.amethyst.service.resourceusage.MeteringNostrSigner
|
||||
import com.vitorpamplona.amethyst.service.resourceusage.ProcessCpuSampler
|
||||
import com.vitorpamplona.amethyst.service.resourceusage.RadioBurstEstimator
|
||||
import com.vitorpamplona.amethyst.service.resourceusage.RefCountedSession
|
||||
import com.vitorpamplona.amethyst.service.resourceusage.RelayConnectionTimeIntegrator
|
||||
import com.vitorpamplona.amethyst.service.resourceusage.RelayUsageListener
|
||||
import com.vitorpamplona.amethyst.service.resourceusage.ResourceUsageAccountant
|
||||
@@ -246,10 +247,17 @@ class AppModules(
|
||||
OtsSharedPreferences(appContext, applicationIOScope)
|
||||
}
|
||||
|
||||
// App services that should be run as soon as there are subscribers to their flows
|
||||
// App services that should be run as soon as there are subscribers to their
|
||||
// flows. Location additionally releases its OS registration whenever no
|
||||
// activity is started — see the foreground gate inside LocationState.
|
||||
val locationManager by lazy {
|
||||
Log.d("AppModules", "LocationManager Init")
|
||||
LocationState(appContext, applicationIOScope, onListening = { locationSession.setActive(it) })
|
||||
LocationState(
|
||||
appContext,
|
||||
applicationIOScope,
|
||||
isForeground = foregroundTracker.isForeground,
|
||||
onListening = { locationRefCount.setActive(it) },
|
||||
)
|
||||
}
|
||||
val connManager = ConnectivityManager(appContext, applicationIOScope)
|
||||
|
||||
@@ -374,6 +382,11 @@ class AppModules(
|
||||
private val torSession = SessionTimeIntegrator(resourceUsage, UsageKeys.TOR_MS, UsageKeys.TOR_STARTS).also { it.registerFlushHook() }
|
||||
private val locationSession = SessionTimeIntegrator(resourceUsage, UsageKeys.LOCATION_MS).also { it.registerFlushHook() }
|
||||
|
||||
// LocationState exposes two independent flows that can both be listening at
|
||||
// once (the "Around Me" feed plus an open geohash chat). Refcounting keeps
|
||||
// either one stopping from closing the other's segment.
|
||||
private val locationRefCount = RefCountedSession(locationSession::setActive)
|
||||
|
||||
// Time-per-screen (route base names only — arguments never reach the
|
||||
// ledger). Fed by the navigation listener in AppNavigation; foreground
|
||||
// gating means backgrounding on a screen closes its segment.
|
||||
|
||||
+107
-26
@@ -21,56 +21,137 @@
|
||||
package com.vitorpamplona.amethyst.service.location
|
||||
|
||||
import android.annotation.SuppressLint
|
||||
import android.content.Context
|
||||
import android.location.Location
|
||||
import android.location.LocationListener
|
||||
import android.location.LocationManager
|
||||
import android.os.Build
|
||||
import android.os.Looper
|
||||
import com.vitorpamplona.amethyst.service.location.LocationState.Companion.MIN_DISTANCE
|
||||
import com.vitorpamplona.amethyst.service.location.LocationState.Companion.MIN_TIME
|
||||
import com.vitorpamplona.quartz.utils.Log
|
||||
import kotlinx.coroutines.channels.awaitClose
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.callbackFlow
|
||||
import kotlinx.coroutines.launch
|
||||
|
||||
/**
|
||||
* Wraps [LocationManager] update registration as a cold [Flow].
|
||||
*
|
||||
* Registers on **one** provider, chosen by [LocationProviderLadder], rather than
|
||||
* on every provider the device reports. The previous shotgun cost four
|
||||
* simultaneous registrations — passive, network, fused and gps, the last at
|
||||
* HIGH_ACCURACY — to produce a 5 km geohash.
|
||||
*
|
||||
* Takes a [LocationManager] rather than a `Context` so the registration
|
||||
* behaviour is unit-testable; the caller does the `getSystemService` lookup.
|
||||
*
|
||||
* [onListening] is fired from inside the flow, after a registration succeeds, and
|
||||
* released again from the `try`/`finally` that wraps everything after it, never
|
||||
* as an `onStart`/`onCompletion` pair on the returned flow. The distinction
|
||||
* matters: an `onStart` fires on collection even when nothing registered, so a
|
||||
* device with no usable provider would accrue location time with no location
|
||||
* running, and — because the ledger refcounts the two [LocationState] flows
|
||||
* together — the unpaired close would steal the other flow's holder.
|
||||
*
|
||||
* The pair is kept honest from both ends. The acquire cannot fire without a
|
||||
* registration, because a failure to register throws before reaching it. The
|
||||
* release cannot be skipped, because everything after the acquire runs inside a
|
||||
* `try`/`finally` rather than inside `awaitClose` — [freshestLastKnownLocation]
|
||||
* (the seed sweep below) can throw a non-cancellation exception and unwind
|
||||
* before `awaitClose` is ever reached, and `try`/`finally` is what still runs
|
||||
* the release on that path; see `releasesTheRegistrationWhenTheSeedThrows` in
|
||||
* `LocationFlowTest`. (In principle a collector cancelling mid-seed would also
|
||||
* unwind past `awaitClose` the same way, but that path could not be
|
||||
* reproduced — `callbackFlow`'s buffer is empty at this point, so the single
|
||||
* seed send returns without suspending and never observes the cancellation.)
|
||||
*/
|
||||
class LocationFlow(
|
||||
private val context: Context,
|
||||
private val locationManager: LocationManager,
|
||||
private val sdkInt: Int = Build.VERSION.SDK_INT,
|
||||
) {
|
||||
@SuppressLint("MissingPermission")
|
||||
fun get(
|
||||
minTimeMs: Long = MIN_TIME,
|
||||
minDistanceM: Float = MIN_DISTANCE,
|
||||
minTimeMs: Long,
|
||||
minDistanceM: Float,
|
||||
onListening: ((Boolean) -> Unit)? = null,
|
||||
): Flow<Location> =
|
||||
callbackFlow {
|
||||
Log.i("LocationFlow", "Start")
|
||||
val locationManager = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager
|
||||
|
||||
val locationCallback =
|
||||
LocationListener { location ->
|
||||
Log.d("LocationFlow") { "onLocationChanged $location" }
|
||||
launch { send(location) }
|
||||
}
|
||||
|
||||
locationManager.allProviders.forEach {
|
||||
val location = locationManager.getLastKnownLocation(it)
|
||||
Log.d("LocationFlow") { "Last Known location is $location" }
|
||||
if (location != null) {
|
||||
send(location)
|
||||
}
|
||||
Log.d("LocationFlow", "Requesting Updates")
|
||||
locationManager.requestLocationUpdates(
|
||||
it,
|
||||
minTimeMs,
|
||||
minDistanceM,
|
||||
locationCallback,
|
||||
Looper.getMainLooper(),
|
||||
)
|
||||
}
|
||||
// One binder call, reused for both the ladder filter and the seed.
|
||||
val providers = locationManager.allProviders
|
||||
|
||||
awaitClose {
|
||||
Log.i("LocationFlow", "Stop")
|
||||
val candidates = LocationProviderLadder.chooseProviders(sdkInt) { it in providers }
|
||||
|
||||
val registered =
|
||||
candidates.firstOrNull { requestUpdates(it, minTimeMs, minDistanceM, locationCallback) }
|
||||
?: throw SecurityException("No usable location provider. Candidates: $candidates")
|
||||
|
||||
Log.i("LocationFlow") { "Listening on $registered every ${minTimeMs}ms / ${minDistanceM}m" }
|
||||
onListening?.invoke(true)
|
||||
|
||||
// Cleanup lives in this finally, not in awaitClose — see the class
|
||||
// KDoc, and `releasesTheRegistrationWhenTheSeedThrows` in
|
||||
// LocationFlowTest, which fails if it moves.
|
||||
try {
|
||||
// Seeded after registration so the no-provider path throws
|
||||
// without having emitted anything; seeding first would show the
|
||||
// consumer Success -> LackPermission on a device with no
|
||||
// compatible provider.
|
||||
freshestLastKnownLocation(candidates)?.let {
|
||||
Log.d("LocationFlow") { "Last known location is $it" }
|
||||
send(it)
|
||||
}
|
||||
|
||||
awaitClose { }
|
||||
} finally {
|
||||
Log.i("LocationFlow") { "Stopped listening on $registered" }
|
||||
locationManager.removeUpdates(locationCallback)
|
||||
onListening?.invoke(false)
|
||||
}
|
||||
}
|
||||
|
||||
/** True when the registration was accepted; false when the provider refused it. */
|
||||
@SuppressLint("MissingPermission")
|
||||
private fun requestUpdates(
|
||||
provider: String,
|
||||
minTimeMs: Long,
|
||||
minDistanceM: Float,
|
||||
locationCallback: LocationListener,
|
||||
): Boolean =
|
||||
try {
|
||||
locationManager.requestLocationUpdates(
|
||||
provider,
|
||||
minTimeMs,
|
||||
minDistanceM,
|
||||
locationCallback,
|
||||
Looper.getMainLooper(),
|
||||
)
|
||||
true
|
||||
} catch (e: SecurityException) {
|
||||
Log.w("LocationFlow", "Provider $provider refused the update request", e)
|
||||
false
|
||||
}
|
||||
|
||||
/**
|
||||
* The freshest cached fix across the providers the ladder deemed usable.
|
||||
* Sweeping every provider the device reports instead would, on the
|
||||
* coarse-only legacy path, pay a guaranteed-to-throw binder call per
|
||||
* fine-only provider on every flow start. Still guarded per provider, like
|
||||
* the update request is — on a device where one refuses us, the others
|
||||
* should still seed.
|
||||
*/
|
||||
@SuppressLint("MissingPermission")
|
||||
private fun freshestLastKnownLocation(providers: List<String>): Location? =
|
||||
providers
|
||||
.mapNotNull { provider ->
|
||||
try {
|
||||
locationManager.getLastKnownLocation(provider)
|
||||
} catch (e: SecurityException) {
|
||||
Log.w("LocationFlow", "No permission to read the last known location of $provider", e)
|
||||
null
|
||||
}
|
||||
}.maxByOrNull { it.time }
|
||||
}
|
||||
|
||||
+72
@@ -0,0 +1,72 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.location
|
||||
|
||||
import android.location.LocationManager
|
||||
import android.os.Build
|
||||
|
||||
/**
|
||||
* Picks which location providers to try, in order.
|
||||
*
|
||||
* Deliberately selects on **provider existence**, never on
|
||||
* [LocationManager.isProviderEnabled]. A registration on a disabled provider
|
||||
* goes live by itself when the user enables location — including from the
|
||||
* quick-settings shade without leaving the app, which is exactly what someone
|
||||
* does after seeing an empty "Around Me" feed. An enabled-state guard evaluated
|
||||
* once at subscription start would lose that.
|
||||
*
|
||||
* Below API 31, `gps`, `passive` and `fused` required `ACCESS_FINE_LOCATION`;
|
||||
* only `network` accepted `ACCESS_COARSE_LOCATION`. Approximate location, which
|
||||
* lets a coarse-only app request any provider and receive a fuzzed result, is an
|
||||
* Android 12 change. Amethyst declares coarse only, so the legacy branch is
|
||||
* unconditional below API 31.
|
||||
*
|
||||
* Adding `ACCESS_FINE_LOCATION` later would **not** widen this on its own — the
|
||||
* branch below has no permission input, so pre-31 devices would keep getting
|
||||
* `network` alone and silently lose the precision the new permission was granted
|
||||
* for. Whoever adds it must widen the condition here too.
|
||||
*
|
||||
* Returns the ordered candidate list rather than a single choice so the caller
|
||||
* can fall through to the next rung if a registration is refused. An empty list
|
||||
* means no compatible provider exists.
|
||||
*/
|
||||
object LocationProviderLadder {
|
||||
// Compile-time String constants, inlined by the compiler, so naming
|
||||
// FUSED_PROVIDER (added in API 31) is safe on older runtimes.
|
||||
private val FULL_LADDER =
|
||||
listOf(
|
||||
LocationManager.FUSED_PROVIDER,
|
||||
LocationManager.NETWORK_PROVIDER,
|
||||
LocationManager.GPS_PROVIDER,
|
||||
LocationManager.PASSIVE_PROVIDER,
|
||||
)
|
||||
|
||||
private val COARSE_ONLY_LEGACY_LADDER = listOf(LocationManager.NETWORK_PROVIDER)
|
||||
|
||||
fun chooseProviders(
|
||||
sdkInt: Int,
|
||||
exists: (String) -> Boolean,
|
||||
): List<String> {
|
||||
val ladder = if (sdkInt >= Build.VERSION_CODES.S) FULL_LADDER else COARSE_ONLY_LEGACY_LADDER
|
||||
|
||||
return ladder.filter(exists)
|
||||
}
|
||||
}
|
||||
+162
-65
@@ -21,32 +21,83 @@
|
||||
package com.vitorpamplona.amethyst.service.location
|
||||
|
||||
import android.content.Context
|
||||
import android.location.Location
|
||||
import android.location.LocationManager
|
||||
import com.vitorpamplona.quartz.experimental.bitchat.geohash.GeohashChannelLevel
|
||||
import com.vitorpamplona.quartz.nip01Core.tags.geohash.GeoHash
|
||||
import com.vitorpamplona.quartz.nip01Core.tags.geohash.GeohashPrecision
|
||||
import com.vitorpamplona.quartz.utils.Log
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.SharingStarted
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.catch
|
||||
import kotlinx.coroutines.flow.combine
|
||||
import kotlinx.coroutines.flow.distinctUntilChanged
|
||||
import kotlinx.coroutines.flow.emitAll
|
||||
import kotlinx.coroutines.flow.map
|
||||
import kotlinx.coroutines.flow.onCompletion
|
||||
import kotlinx.coroutines.flow.onEach
|
||||
import kotlinx.coroutines.flow.onStart
|
||||
import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.flow.transformLatest
|
||||
|
||||
// `toGeoHash` is an extension on Location declared in LocationGeoHash.kt, same
|
||||
// package, so it needs no import.
|
||||
|
||||
/**
|
||||
* Turns the device's location into geohashes, listening **only while the app is
|
||||
* in the foreground**.
|
||||
*
|
||||
* The gate is not an optimisation of last resort: `Account` builds 30
|
||||
* `SharingStarted.Eagerly` top-nav filter states on the account scope, and
|
||||
* `AccountSettings.defaultProductsFollowList` ships as `TopFilter.AroundMe`, so
|
||||
* without it every user with location permission holds a registration for the
|
||||
* life of the process. See `amethyst/plans/2026-07-29-location-foreground-gate.md`.
|
||||
*
|
||||
* Switching the *consumers* to `WhileSubscribed` is not an option: roughly 60
|
||||
* call sites read `account.live*FollowLists.value` synchronously rather than
|
||||
* collecting, and would silently serve a stale or initial value.
|
||||
*/
|
||||
class LocationState(
|
||||
context: Context,
|
||||
scope: CoroutineScope,
|
||||
/** Resource-ledger hook: true while GPS/location updates are actively requested. */
|
||||
private val scope: CoroutineScope,
|
||||
private val isForeground: StateFlow<Boolean>,
|
||||
/**
|
||||
* Resource-ledger hook: true while location updates are actively
|
||||
* registered. Reaches the OS only through the default [locationSource],
|
||||
* which hands it to [LocationFlow] — a caller that overrides
|
||||
* [locationSource] (the tests do) is responsible for firing it, or not.
|
||||
*/
|
||||
private val onListening: ((Boolean) -> Unit)? = null,
|
||||
private val locationSource: (Long, Float) -> Flow<Location> = { minTimeMs, minDistanceM ->
|
||||
LocationFlow(context.getSystemService(Context.LOCATION_SERVICE) as LocationManager)
|
||||
.get(minTimeMs, minDistanceM, onListening)
|
||||
},
|
||||
) {
|
||||
companion object {
|
||||
const val MIN_TIME: Long = 10000L
|
||||
const val MIN_DISTANCE: Float = 100.0f
|
||||
/** A 5 km cell takes 2.5 minutes to cross at 120 km/h; 60s/500m is ample. */
|
||||
const val COARSE_MIN_TIME: Long = 60_000L
|
||||
const val COARSE_MIN_DISTANCE: Float = 500.0f
|
||||
|
||||
/** Building-level geohashes need the tighter profile. */
|
||||
const val PRECISE_MIN_TIME: Long = 10_000L
|
||||
const val PRECISE_MIN_DISTANCE: Float = 100.0f
|
||||
|
||||
/**
|
||||
* How long to keep listening after the last activity stops, so a
|
||||
* one-second app switch doesn't destroy and rebuild the registration.
|
||||
* Same intent as [SUBSCRIPTION_STOP_TIMEOUT_MS], on the other axis.
|
||||
*/
|
||||
const val BACKGROUND_GRACE_MS: Long = 5_000L
|
||||
|
||||
/**
|
||||
* How long `stateIn` keeps the upstream alive after the last collector
|
||||
* leaves, so a screen rotation or a tab switch doesn't rebuild the
|
||||
* registration either.
|
||||
*/
|
||||
const val SUBSCRIPTION_STOP_TIMEOUT_MS: Long = 5_000L
|
||||
}
|
||||
|
||||
sealed class LocationResult {
|
||||
@@ -59,9 +110,16 @@ class LocationState(
|
||||
object Loading : LocationResult()
|
||||
}
|
||||
|
||||
private enum class Gate { NoPermission, Paused, Listen }
|
||||
|
||||
private var hasLocationPermission = MutableStateFlow(false)
|
||||
private var latestLocation: LocationResult = LocationResult.Loading
|
||||
private var latestPreciseLocation: LocationResult = LocationResult.Loading
|
||||
|
||||
// Read by R1 below to decide whether to emit Loading, from a different
|
||||
// coroutine than the onEach that writes it — hence a StateFlow rather than
|
||||
// a plain field.
|
||||
private val latestLocation = MutableStateFlow<LocationResult>(LocationResult.Loading)
|
||||
|
||||
private val latestPreciseLocation = MutableStateFlow<LocationResult>(LocationResult.Loading)
|
||||
|
||||
fun setLocationPermission(newValue: Boolean) {
|
||||
if (newValue != hasLocationPermission.value) {
|
||||
@@ -69,36 +127,92 @@ class LocationState(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Foreground with an asymmetric delay: leaving the foreground waits out
|
||||
* [BACKGROUND_GRACE_MS], returning to it is immediate.
|
||||
*
|
||||
* `debounce(5000)` would delay both edges, and the duration-selector
|
||||
* overload that allows an asymmetric delay is `@FlowPreview`.
|
||||
* `transformLatest` cancels the pending `delay` when foreground returns
|
||||
* first, which is exactly the semantics wanted, with no preview opt-in.
|
||||
*
|
||||
* Known and harmless: [ForegroundTracker] starts at `false`, so on a
|
||||
* process that starts backgrounded the first emission — and therefore the
|
||||
* first gate verdict, including `LackPermission` — is delayed by
|
||||
* [BACKGROUND_GRACE_MS]. Nothing renders while backgrounded, and a process
|
||||
* that starts into the foreground emits immediately, because the activity's
|
||||
* `onStart` cancels the pending delay.
|
||||
*/
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
val geohashStateFlow by lazy {
|
||||
hasLocationPermission
|
||||
.transformLatest {
|
||||
if (it) {
|
||||
emit(LocationResult.Loading)
|
||||
val result =
|
||||
LocationFlow(context)
|
||||
.get(MIN_TIME, MIN_DISTANCE)
|
||||
.onStart { onListening?.invoke(true) }
|
||||
.onCompletion { onListening?.invoke(false) }
|
||||
.map {
|
||||
LocationResult.Success(it.toGeoHash(GeohashPrecision.KM_5_X_5.digits)) as LocationResult
|
||||
}.onEach {
|
||||
latestLocation = it
|
||||
}.catch { e ->
|
||||
Log.w("GeohashStateFlow", "Exception in the flow", e)
|
||||
latestLocation = LocationResult.LackPermission
|
||||
emit(LocationResult.LackPermission)
|
||||
}
|
||||
private val settledForeground: Flow<Boolean> =
|
||||
isForeground.transformLatest { foreground ->
|
||||
if (!foreground) delay(BACKGROUND_GRACE_MS)
|
||||
emit(foreground)
|
||||
}
|
||||
|
||||
emitAll(result)
|
||||
} else {
|
||||
emit(LocationResult.LackPermission)
|
||||
private val gate: Flow<Gate> =
|
||||
combine(hasLocationPermission, settledForeground) { permitted, foreground ->
|
||||
when {
|
||||
!permitted -> Gate.NoPermission
|
||||
foreground -> Gate.Listen
|
||||
else -> Gate.Paused
|
||||
}
|
||||
}.distinctUntilChanged()
|
||||
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
private fun buildGeohashStateFlow(
|
||||
tag: String,
|
||||
charsCount: Int,
|
||||
minTimeMs: Long,
|
||||
minDistanceM: Float,
|
||||
cache: MutableStateFlow<LocationResult>,
|
||||
): StateFlow<LocationResult> =
|
||||
gate
|
||||
.transformLatest { state ->
|
||||
when (state) {
|
||||
// Deliberately does NOT write to the cache. Today's code emits
|
||||
// LackPermission without touching the cache, and wiping it
|
||||
// here would cost a Loading emission — and so an empty-feed
|
||||
// flash — on every permission flap, which is the regression R1
|
||||
// exists to prevent. Consumers already see LackPermission from
|
||||
// the StateFlow; the cache is internal and only decides whether
|
||||
// Loading is emitted.
|
||||
Gate.NoPermission -> emit(LocationResult.LackPermission)
|
||||
|
||||
// Emit nothing: stateIn keeps the last value, so every
|
||||
// synchronous .value reader still sees the last known geohash
|
||||
// while the OS registration is released.
|
||||
Gate.Paused -> Unit
|
||||
|
||||
Gate.Listen -> {
|
||||
// Only when there is nothing cached. Emitting Loading on
|
||||
// every foreground return would flash the "Around Me" feed
|
||||
// empty, because AroundMeFeedFlow.convert maps anything
|
||||
// that is not Success to an empty geotag set.
|
||||
if (cache.value !is LocationResult.Success) emit(LocationResult.Loading)
|
||||
|
||||
emitAll(
|
||||
locationSource(minTimeMs, minDistanceM)
|
||||
.map { LocationResult.Success(it.toGeoHash(charsCount)) as LocationResult }
|
||||
.onEach { cache.value = it }
|
||||
.catch { e ->
|
||||
Log.w(tag, "Exception in the flow", e)
|
||||
cache.value = LocationResult.LackPermission
|
||||
emit(LocationResult.LackPermission)
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
}.stateIn(
|
||||
scope,
|
||||
SharingStarted.WhileSubscribed(5000),
|
||||
latestLocation,
|
||||
)
|
||||
}.stateIn(scope, SharingStarted.WhileSubscribed(SUBSCRIPTION_STOP_TIMEOUT_MS), cache.value)
|
||||
|
||||
val geohashStateFlow: StateFlow<LocationResult> by lazy {
|
||||
buildGeohashStateFlow(
|
||||
tag = "GeohashStateFlow",
|
||||
charsCount = GeohashPrecision.KM_5_X_5.digits,
|
||||
minTimeMs = COARSE_MIN_TIME,
|
||||
minDistanceM = COARSE_MIN_DISTANCE,
|
||||
cache = latestLocation,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -107,36 +221,19 @@ class LocationState(
|
||||
* to every coarser level (a geohash is a prefix code), so one fix yields the
|
||||
* whole region→building ladder. Kept separate so the coarser
|
||||
* [geohashStateFlow] the "around me" feed relies on is unchanged.
|
||||
*
|
||||
* Note that Amethyst declares only `ACCESS_COARSE_LOCATION`, so Android
|
||||
* fuzzes every fix to roughly a 3 km grid and this is not in fact
|
||||
* building-level today. The profile is kept so the intent survives if the
|
||||
* app ever requests `ACCESS_FINE_LOCATION`.
|
||||
*/
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
val preciseGeohashStateFlow by lazy {
|
||||
hasLocationPermission
|
||||
.transformLatest {
|
||||
if (it) {
|
||||
emit(LocationResult.Loading)
|
||||
val result =
|
||||
LocationFlow(context)
|
||||
.get(MIN_TIME, MIN_DISTANCE)
|
||||
.onStart { onListening?.invoke(true) }
|
||||
.onCompletion { onListening?.invoke(false) }
|
||||
.map {
|
||||
LocationResult.Success(it.toGeoHash(GeohashChannelLevel.BUILDING.chars)) as LocationResult
|
||||
}.onEach {
|
||||
latestPreciseLocation = it
|
||||
}.catch { e ->
|
||||
Log.w("GeohashStateFlow", "Exception in the precise flow", e)
|
||||
latestPreciseLocation = LocationResult.LackPermission
|
||||
emit(LocationResult.LackPermission)
|
||||
}
|
||||
|
||||
emitAll(result)
|
||||
} else {
|
||||
emit(LocationResult.LackPermission)
|
||||
}
|
||||
}.stateIn(
|
||||
scope,
|
||||
SharingStarted.WhileSubscribed(5000),
|
||||
latestPreciseLocation,
|
||||
)
|
||||
val preciseGeohashStateFlow: StateFlow<LocationResult> by lazy {
|
||||
buildGeohashStateFlow(
|
||||
tag = "PreciseGeohashStateFlow",
|
||||
charsCount = GeohashChannelLevel.BUILDING.chars,
|
||||
minTimeMs = PRECISE_MIN_TIME,
|
||||
minDistanceM = PRECISE_MIN_DISTANCE,
|
||||
cache = latestPreciseLocation,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
+64
@@ -0,0 +1,64 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.resourceusage
|
||||
|
||||
/**
|
||||
* Refcounts a boolean session so overlapping holders don't close each other's
|
||||
* segment. [LocationState][com.vitorpamplona.amethyst.service.location.LocationState]
|
||||
* exposes two independent location flows that can both be listening at once —
|
||||
* the "Around Me" feed plus an open geohash chat — and a bare
|
||||
* [SessionTimeIntegrator] would close the segment when either one stops.
|
||||
*
|
||||
* The count and the transition it drives are taken under one lock. An
|
||||
* [java.util.concurrent.atomic.AtomicInteger] beside an unsynchronised call is
|
||||
* not enough: two threads can leave the counter at 1 while the last
|
||||
* `setActive(false)` lands after the `setActive(true)`, latching the session
|
||||
* off with a holder still active.
|
||||
*
|
||||
* Takes the setter as a lambda rather than a [SessionTimeIntegrator] because
|
||||
* that is all it needs, and so tests need no accountant or store file.
|
||||
*
|
||||
* Reports **transitions only**, not every call — the contract the name implies,
|
||||
* and what keeps the class honest for a future caller that reacts to the
|
||||
* callback rather than integrating it. (For [SessionTimeIntegrator] specifically
|
||||
* a 1 -> 2 re-entry would have been harmless: it splits one segment into two
|
||||
* contiguous pieces that `account()` re-adds, and its starts increment is
|
||||
* already guarded on `prev == null`.)
|
||||
*
|
||||
* Releases must be paired with acquires. This class cannot tell an unpaired
|
||||
* release from a real one, so callers guarantee the pairing; see `LocationFlow`,
|
||||
* which throws rather than reaching `awaitClose` when nothing registered.
|
||||
*/
|
||||
class RefCountedSession(
|
||||
private val setSessionActive: (Boolean) -> Unit,
|
||||
) {
|
||||
private val lock = Any()
|
||||
private var holders = 0
|
||||
|
||||
fun setActive(active: Boolean) {
|
||||
synchronized(lock) {
|
||||
val wasActive = holders > 0
|
||||
holders = if (active) holders + 1 else (holders - 1).coerceAtLeast(0)
|
||||
val isActive = holders > 0
|
||||
if (isActive != wasActive) setSessionActive(isActive)
|
||||
}
|
||||
}
|
||||
}
|
||||
+201
@@ -0,0 +1,201 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.location
|
||||
|
||||
import android.location.Location
|
||||
import android.location.LocationListener
|
||||
import android.location.LocationManager
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import io.mockk.verify
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.cancelAndJoin
|
||||
import kotlinx.coroutines.flow.collect
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.test.runCurrent
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
class LocationFlowTest {
|
||||
private fun manager(
|
||||
providers: List<String>,
|
||||
denied: Set<String> = emptySet(),
|
||||
) = mockLocationManager(providers, denied)
|
||||
|
||||
@Test
|
||||
fun firesNeitherEdgeWhenNoProviderExists() =
|
||||
runTest {
|
||||
val edges = mutableListOf<Boolean>()
|
||||
val flow = LocationFlow(manager(providers = emptyList()), sdkInt = 37).get(60_000L, 500f) { edges.add(it) }
|
||||
|
||||
val failure = runCatching { flow.collect { } }.exceptionOrNull()
|
||||
|
||||
assertTrue("expected SecurityException, got $failure", failure is SecurityException)
|
||||
assertEquals(emptyList<Boolean>(), edges)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun firesNeitherEdgeWhenEveryRungIsDenied() =
|
||||
runTest {
|
||||
val edges = mutableListOf<Boolean>()
|
||||
val lm = manager(providers = listOf("fused", "network"), denied = setOf("fused", "network"))
|
||||
val flow = LocationFlow(lm, sdkInt = 37).get(60_000L, 500f) { edges.add(it) }
|
||||
|
||||
val failure = runCatching { flow.collect { } }.exceptionOrNull()
|
||||
|
||||
assertTrue("expected SecurityException, got $failure", failure is SecurityException)
|
||||
assertEquals(emptyList<Boolean>(), edges)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun fallsThroughToTheNextRungWhenOneIsDenied() =
|
||||
runTest {
|
||||
val edges = mutableListOf<Boolean>()
|
||||
val lm = manager(providers = listOf("fused", "network"), denied = setOf("fused"))
|
||||
val job = launch { LocationFlow(lm, sdkInt = 37).get(60_000L, 500f) { edges.add(it) }.collect { } }
|
||||
|
||||
runCurrent()
|
||||
|
||||
assertEquals(listOf(true), edges)
|
||||
verify { lm.requestLocationUpdates("network", 60_000L, 500f, any<LocationListener>(), any()) }
|
||||
|
||||
job.cancelAndJoin()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun pairsTheListeningEdgesAroundASuccessfulRegistration() =
|
||||
runTest {
|
||||
val edges = mutableListOf<Boolean>()
|
||||
val lm = manager(providers = listOf("network"))
|
||||
val job = launch { LocationFlow(lm, sdkInt = 30).get(60_000L, 500f) { edges.add(it) }.collect { } }
|
||||
|
||||
runCurrent()
|
||||
assertEquals(listOf(true), edges)
|
||||
|
||||
job.cancelAndJoin()
|
||||
|
||||
assertEquals(listOf(true, false), edges)
|
||||
verify { lm.removeUpdates(any<LocationListener>()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun releasesTheRegistrationWhenTheSeedThrows() =
|
||||
runTest {
|
||||
// Deterministic discriminator for the try/finally: make the seed
|
||||
// sweep throw. A plain RuntimeException is used rather than
|
||||
// SecurityException, because freshestLastKnownLocation() catches
|
||||
// SecurityException internally per-provider and it would never
|
||||
// propagate out of the try block at all. With cleanup living in
|
||||
// try/finally, onListening(false)/removeUpdates still run even
|
||||
// though the block exits via an exception; if cleanup lived in
|
||||
// awaitClose instead, the throw would unwind past it before
|
||||
// awaitClose is ever reached and the release would be skipped.
|
||||
val edges = mutableListOf<Boolean>()
|
||||
val lm = mockk<LocationManager>(relaxed = true)
|
||||
every { lm.allProviders } returns listOf("network")
|
||||
every { lm.getLastKnownLocation(any()) } throws RuntimeException("boom")
|
||||
|
||||
val failure =
|
||||
runCatching {
|
||||
LocationFlow(lm, sdkInt = 30).get(60_000L, 500f) { edges.add(it) }.collect { }
|
||||
}.exceptionOrNull()
|
||||
|
||||
assertTrue("expected the seed's RuntimeException to surface, got $failure", failure is RuntimeException)
|
||||
assertEquals("the acquire must be released even when the seed throws", listOf(true, false), edges)
|
||||
verify { lm.removeUpdates(any<LocationListener>()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun emitsNothingBeforeThrowingWhenNoProviderRegisters() =
|
||||
runTest {
|
||||
// Discriminates the seed-after-registration ordering: every
|
||||
// provider is denied, so the registration loop never succeeds and
|
||||
// the flow must throw before ever reaching the seed. A cached fix
|
||||
// is deliberately made available (non-null, with a real `time`)
|
||||
// so that a seed-first implementation — which would emit it before
|
||||
// discovering no provider registers — fails this test.
|
||||
//
|
||||
// The `values` assertion below is necessary but, on its own, is
|
||||
// not sufficient to catch a seed-first regression here: a value
|
||||
// `send`-ed into the callbackFlow channel immediately before the
|
||||
// producer coroutine throws can be dropped by structured-
|
||||
// concurrency teardown before this collector's suspended
|
||||
// `receive` is ever resumed, so the collector may see zero
|
||||
// values purely as a scheduling artifact, independent of
|
||||
// ordering. The `getLastKnownLocation` verify is what actually
|
||||
// discriminates: it fails deterministically whenever the seed is
|
||||
// attempted at all, regardless of whether registration
|
||||
// eventually succeeds — which is exactly what a seed-first
|
||||
// implementation does and what the current try/finally-after-
|
||||
// registration implementation must never do on this path.
|
||||
val values = mutableListOf<Location>()
|
||||
val lm = manager(providers = listOf("fused", "network"), denied = setOf("fused", "network"))
|
||||
val cached = mockk<Location>()
|
||||
every { cached.time } returns 1_000L
|
||||
every { lm.getLastKnownLocation(any()) } returns cached
|
||||
|
||||
val failure =
|
||||
runCatching {
|
||||
LocationFlow(lm, sdkInt = 37).get(60_000L, 500f).collect { values.add(it) }
|
||||
}.exceptionOrNull()
|
||||
|
||||
assertTrue("expected SecurityException, got $failure", failure is SecurityException)
|
||||
assertEquals("no value should be emitted before the throw", emptyList<Location>(), values)
|
||||
verify(exactly = 0) { lm.getLastKnownLocation(any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun seedsOnlyFromTheProvidersTheLadderDeemedUsable() =
|
||||
runTest {
|
||||
// On the coarse-only legacy path the ladder narrows to `network`,
|
||||
// and gps/passive/fused would each throw SecurityException. Sweeping
|
||||
// every provider the device reports would pay those guaranteed
|
||||
// throws — stack trace plus an eagerly-interpolated Log.w — on every
|
||||
// flow start, i.e. on every foreground return.
|
||||
val lm = manager(providers = listOf("fused", "network", "gps", "passive"))
|
||||
val job = launch { LocationFlow(lm, sdkInt = 30).get(60_000L, 500f).collect { } }
|
||||
|
||||
runCurrent()
|
||||
|
||||
verify(exactly = 1) { lm.getLastKnownLocation("network") }
|
||||
verify(exactly = 0) { lm.getLastKnownLocation(neq("network")) }
|
||||
|
||||
job.cancelAndJoin()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun registersOnExactlyOneProvider() =
|
||||
runTest {
|
||||
val lm = manager(providers = listOf("fused", "network", "gps", "passive"))
|
||||
val job = launch { LocationFlow(lm, sdkInt = 37).get(60_000L, 500f).collect { } }
|
||||
|
||||
runCurrent()
|
||||
|
||||
verify(exactly = 1) {
|
||||
lm.requestLocationUpdates(any<String>(), any<Long>(), any<Float>(), any<LocationListener>(), any())
|
||||
}
|
||||
|
||||
job.cancelAndJoin()
|
||||
}
|
||||
}
|
||||
+243
@@ -0,0 +1,243 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.location
|
||||
|
||||
import android.content.Context
|
||||
import android.location.LocationListener
|
||||
import android.location.LocationManager
|
||||
import com.vitorpamplona.amethyst.service.resourceusage.RefCountedSession
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import io.mockk.verify
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.cancelAndJoin
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.test.UnconfinedTestDispatcher
|
||||
import kotlinx.coroutines.test.advanceTimeBy
|
||||
import kotlinx.coroutines.test.advanceUntilIdle
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* Wires the *real* [LocationState] -> [LocationFlow] -> [RefCountedSession]
|
||||
* seam together, exercising [LocationState]'s **default** `locationSource`
|
||||
* rather than overriding it the way [LocationStateTest] and [LocationFlowTest]
|
||||
* do. Each of those files mocks the other layer out, so neither can catch a
|
||||
* regression in the composition itself — e.g. a refcount that gets pinned
|
||||
* open (or closed) because `onListening` fires in an order or multiplicity
|
||||
* the two independent [LocationState] flows didn't anticipate. That failure
|
||||
* mode is unrecoverable at runtime: a pinned-open refcount means
|
||||
* `location.ms` accrues in the resource-usage ledger forever, with nothing
|
||||
* actually listening.
|
||||
*
|
||||
* Same dispatcher note as [LocationStateTest]: `runTest {}`'s default
|
||||
* `StandardTestDispatcher` does not drive this `combine` + `transformLatest` +
|
||||
* `stateIn(WhileSubscribed)` + `backgroundScope.launch { collect }` chain to
|
||||
* completion via `advanceUntilIdle()` alone, so every test here runs on
|
||||
* `UnconfinedTestDispatcher`.
|
||||
*/
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
class LocationLedgerCompositionTest {
|
||||
/** A [LocationManager] that reports two providers and never denies registration. */
|
||||
private fun locationManager(): LocationManager = mockLocationManager(providers = listOf("fused", "network"))
|
||||
|
||||
/** A [Context] whose `LOCATION_SERVICE` lookup returns [locationManager]. */
|
||||
private fun contextWithLocationService(locationManager: LocationManager): Context {
|
||||
val context = mockk<Context>(relaxed = true)
|
||||
every { context.getSystemService(Context.LOCATION_SERVICE) } returns locationManager
|
||||
return context
|
||||
}
|
||||
|
||||
/**
|
||||
* Wires a real [LocationState] to a real [RefCountedSession], standing in
|
||||
* for the resource-usage ledger session. Deliberately does NOT override
|
||||
* `locationSource` — that is the whole point of this file.
|
||||
*
|
||||
* Exposes [locationManager] so a test can `verify` against it directly —
|
||||
* e.g. that exactly one of two concurrent registrations was torn down —
|
||||
* rather than inferring release from the ledger alone, which cannot tell
|
||||
* "released" from "never released" on its own (see
|
||||
* `bothFlowsActiveThenOneStopsKeepsTheLedgerOpen`).
|
||||
*/
|
||||
private class Harness(
|
||||
scope: CoroutineScope,
|
||||
context: Context,
|
||||
val locationManager: LocationManager,
|
||||
) {
|
||||
/** Stand-in for `SessionTimeIntegrator.setActive`, i.e. the ledger. */
|
||||
val ledger = mutableListOf<Boolean>()
|
||||
private val refCount = RefCountedSession { ledger.add(it) }
|
||||
val foreground = MutableStateFlow(true)
|
||||
val state =
|
||||
LocationState(
|
||||
context = context,
|
||||
scope = scope,
|
||||
isForeground = foreground,
|
||||
onListening = { refCount.setActive(it) },
|
||||
)
|
||||
}
|
||||
|
||||
private fun harness(scope: CoroutineScope): Harness {
|
||||
val locationManager = locationManager()
|
||||
return Harness(scope, contextWithLocationService(locationManager), locationManager)
|
||||
}
|
||||
|
||||
/**
|
||||
* The interleaving [RefCountedSession] exists for. Two independent
|
||||
* [LocationState] flows both register with the OS; the ledger must open
|
||||
* once, not twice. Stopping one of the two must NOT close the ledger,
|
||||
* because the other is still listening — this half is exactly what a
|
||||
* per-flow (rather than refcounted) `onListening` -> ledger wiring would
|
||||
* get wrong, and what a per-layer test (mocking `locationSource`, or
|
||||
* driving `RefCountedSession` directly with synthetic booleans) cannot
|
||||
* see, because it never lets two real [LocationFlow] registrations race
|
||||
* each other through the shared hook.
|
||||
*
|
||||
* Asserts its premise, not just its consequence. The ledger staying at
|
||||
* `[true]` after the precise flow stops is also what you'd see if that
|
||||
* flow's registration never released at all — `onListening` deleted from
|
||||
* [LocationFlow]'s `finally`, cleanup moved somewhere unreached, or the
|
||||
* [LocationState.SUBSCRIPTION_STOP_TIMEOUT_MS] teardown simply hadn't
|
||||
* elapsed. The
|
||||
* `removeUpdates` verify below rules out the OS-registration side of
|
||||
* that; cancelling the coarse flow too and checking the full `[true,
|
||||
* false]` sequence at the end rules out the `onListening` side, since
|
||||
* `removeUpdates` alone fires unconditionally in the `finally` and would
|
||||
* not by itself notice `onListening` going missing.
|
||||
*/
|
||||
@Test
|
||||
fun bothFlowsActiveThenOneStopsKeepsTheLedgerOpen() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val harness = harness(backgroundScope)
|
||||
harness.state.setLocationPermission(true)
|
||||
|
||||
val preciseJob = backgroundScope.launch { harness.state.preciseGeohashStateFlow.collect { } }
|
||||
val coarseJob = backgroundScope.launch { harness.state.geohashStateFlow.collect { } }
|
||||
advanceUntilIdle()
|
||||
|
||||
assertEquals(
|
||||
"both flows registering with the OS must open the ledger exactly once, not twice",
|
||||
listOf(true),
|
||||
harness.ledger,
|
||||
)
|
||||
|
||||
preciseJob.cancelAndJoin()
|
||||
// The cancelled collector alone doesn't tear down the upstream
|
||||
// registration — WhileSubscribed keeps it alive for a grace
|
||||
// period first. Advance past it so the release below is
|
||||
// actually observable, not just "not yet due".
|
||||
advanceTimeBy(LocationState.SUBSCRIPTION_STOP_TIMEOUT_MS + 1)
|
||||
advanceUntilIdle()
|
||||
|
||||
// Exactly one of the two OS registrations should have been torn
|
||||
// down at this point — the precise one — while the coarse one is
|
||||
// still live.
|
||||
verify(exactly = 1) { harness.locationManager.removeUpdates(any<LocationListener>()) }
|
||||
|
||||
assertEquals(
|
||||
"the coarse flow is still listening; stopping the precise one alone must not close the ledger",
|
||||
listOf(true),
|
||||
harness.ledger,
|
||||
)
|
||||
|
||||
// Stop the coarse flow too, so the ledger closing here proves
|
||||
// the release path genuinely works end-to-end for this test's
|
||||
// own harness — not merely that the assertions above never
|
||||
// exercised it.
|
||||
coarseJob.cancelAndJoin()
|
||||
advanceTimeBy(LocationState.SUBSCRIPTION_STOP_TIMEOUT_MS + 1)
|
||||
advanceUntilIdle()
|
||||
|
||||
verify(exactly = 2) { harness.locationManager.removeUpdates(any<LocationListener>()) }
|
||||
|
||||
assertEquals(
|
||||
"both flows stopping must close the ledger exactly once, proving the precise flow's earlier release was real",
|
||||
listOf(true, false),
|
||||
harness.ledger,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The full foreground -> background cycle with both flows registered.
|
||||
* The ledger must see exactly `[true, false]`: one open when the first
|
||||
* flow registers (the second joining must not re-open it), one close
|
||||
* once BOTH flows have released (not one close per flow, and not before
|
||||
* the background grace period each flow independently waits out).
|
||||
*/
|
||||
@Test
|
||||
fun fullCycleWithBothFlowsOpensAndClosesTheLedgerExactlyOnce() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val harness = harness(backgroundScope)
|
||||
harness.state.setLocationPermission(true)
|
||||
|
||||
backgroundScope.launch { harness.state.geohashStateFlow.collect { } }
|
||||
backgroundScope.launch { harness.state.preciseGeohashStateFlow.collect { } }
|
||||
advanceUntilIdle()
|
||||
|
||||
assertEquals(listOf(true), harness.ledger)
|
||||
|
||||
harness.foreground.value = false
|
||||
// Real grace-period wait, not a test artefact: settledForeground
|
||||
// delays BACKGROUND_GRACE_MS before the gate reaches Paused for
|
||||
// either flow. See LocationStateTest for the worked example this
|
||||
// follows.
|
||||
advanceTimeBy(LocationState.BACKGROUND_GRACE_MS + 1)
|
||||
advanceUntilIdle()
|
||||
|
||||
assertEquals(
|
||||
"backgrounding must close the ledger exactly once, not once per flow, and not before the grace period",
|
||||
listOf(true, false),
|
||||
harness.ledger,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* After a full open/close cycle, returning to the foreground must reopen
|
||||
* the ledger exactly once — not once per flow, and not a second time on
|
||||
* top of a close that never actually happened.
|
||||
*/
|
||||
@Test
|
||||
fun returningToForegroundReopensTheLedgerExactlyOnce() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val harness = harness(backgroundScope)
|
||||
harness.state.setLocationPermission(true)
|
||||
|
||||
backgroundScope.launch { harness.state.geohashStateFlow.collect { } }
|
||||
backgroundScope.launch { harness.state.preciseGeohashStateFlow.collect { } }
|
||||
advanceUntilIdle()
|
||||
|
||||
harness.foreground.value = false
|
||||
advanceTimeBy(LocationState.BACKGROUND_GRACE_MS + 1)
|
||||
advanceUntilIdle()
|
||||
|
||||
harness.foreground.value = true
|
||||
advanceUntilIdle()
|
||||
|
||||
assertEquals(
|
||||
"a return to foreground after a full close must reopen the ledger exactly once",
|
||||
listOf(true, false, true),
|
||||
harness.ledger,
|
||||
)
|
||||
}
|
||||
}
|
||||
+75
@@ -0,0 +1,75 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.location
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
|
||||
class LocationProviderLadderTest {
|
||||
private val all = setOf("fused", "network", "gps", "passive")
|
||||
|
||||
@Test
|
||||
fun modernDevicePrefersFusedThenFallsBackInOrder() {
|
||||
assertEquals(
|
||||
listOf("fused", "network", "gps", "passive"),
|
||||
LocationProviderLadder.chooseProviders(sdkInt = 31) { it in all },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun missingProvidersAreFilteredOutButOrderIsKept() {
|
||||
val present = setOf("network", "passive")
|
||||
|
||||
assertEquals(
|
||||
listOf("network", "passive"),
|
||||
LocationProviderLadder.chooseProviders(sdkInt = 37) { it in present },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun coarseOnlyBelowApi31GetsNetworkOnly() {
|
||||
// gps, passive and fused all required ACCESS_FINE_LOCATION before
|
||||
// Android 12 (see Hypothesis H1 in the design spec), and Amethyst
|
||||
// declares ACCESS_COARSE_LOCATION only.
|
||||
assertEquals(
|
||||
listOf("network"),
|
||||
LocationProviderLadder.chooseProviders(sdkInt = 30) { it in all },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun coarseOnlyBelowApi31WithNoNetworkProviderGetsNothing() {
|
||||
val present = setOf("gps", "passive")
|
||||
|
||||
assertEquals(
|
||||
emptyList<String>(),
|
||||
LocationProviderLadder.chooseProviders(sdkInt = 28) { it in present },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun noProvidersAtAllGetsNothing() {
|
||||
assertEquals(
|
||||
emptyList<String>(),
|
||||
LocationProviderLadder.chooseProviders(sdkInt = 37) { false },
|
||||
)
|
||||
}
|
||||
}
|
||||
+306
@@ -0,0 +1,306 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.location
|
||||
|
||||
import android.content.Context
|
||||
import android.location.Location
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.awaitCancellation
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.FlowCollector
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.flow
|
||||
import kotlinx.coroutines.flow.onCompletion
|
||||
import kotlinx.coroutines.flow.onStart
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.test.UnconfinedTestDispatcher
|
||||
import kotlinx.coroutines.test.advanceTimeBy
|
||||
import kotlinx.coroutines.test.advanceUntilIdle
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlinx.coroutines.yield
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* All tests run on [UnconfinedTestDispatcher] rather than the `runTest {}` default
|
||||
* ([StandardTestDispatcher]): this suite's `combine` + `transformLatest` +
|
||||
* `stateIn(WhileSubscribed)` + `backgroundScope.launch { collect }` chain does not
|
||||
* get driven to completion by `advanceUntilIdle()` alone under the standard
|
||||
* dispatcher — the producer side never runs. Same root cause, same fix as
|
||||
* `amethyst/src/test/.../ui/tor/TorManagerTest.kt`, which hits it for the same
|
||||
* reason (see its `ioDispatcher = UnconfinedTestDispatcher(...)` comment).
|
||||
*/
|
||||
@OptIn(ExperimentalCoroutinesApi::class)
|
||||
class LocationStateTest {
|
||||
private fun locationAt(
|
||||
lat: Double,
|
||||
lon: Double,
|
||||
): Location =
|
||||
mockk<Location> {
|
||||
every { latitude } returns lat
|
||||
every { longitude } returns lon
|
||||
}
|
||||
|
||||
/** Counts subscriptions and completions of the underlying location source. */
|
||||
private class SourceProbe(
|
||||
private val body: suspend FlowCollector<Location>.() -> Unit,
|
||||
) {
|
||||
var subscriptions = 0
|
||||
private set
|
||||
var completions = 0
|
||||
private set
|
||||
|
||||
/** The (minTimeMs, minDistanceM) pair the source function was called with. */
|
||||
var requestedProfile: Pair<Long, Float>? = null
|
||||
private set
|
||||
|
||||
val live: Int get() = subscriptions - completions
|
||||
|
||||
fun source(): (Long, Float) -> Flow<Location> =
|
||||
{ minTimeMs, minDistanceM ->
|
||||
requestedProfile = minTimeMs to minDistanceM
|
||||
flow(body)
|
||||
.onStart { subscriptions++ }
|
||||
.onCompletion { completions++ }
|
||||
}
|
||||
}
|
||||
|
||||
private fun neverEmits() = SourceProbe { awaitCancellation() }
|
||||
|
||||
private fun emitsOnceThenHangs(
|
||||
lat: Double,
|
||||
lon: Double,
|
||||
) = SourceProbe {
|
||||
// Yield before emitting so the fix arrives as a genuine suspension
|
||||
// point (as a real LocationManager callback would) instead of a
|
||||
// synchronous burst. Without this, under UnconfinedTestDispatcher a
|
||||
// fix that lands in the same undispatched execution as the
|
||||
// immediately-preceding Loading emission can be conflated away by
|
||||
// the downstream StateFlow before a collector observes either value.
|
||||
yield()
|
||||
emit(locationAt(lat, lon))
|
||||
awaitCancellation()
|
||||
}
|
||||
|
||||
private fun stateWith(
|
||||
scope: CoroutineScope,
|
||||
foreground: MutableStateFlow<Boolean>,
|
||||
probe: SourceProbe,
|
||||
) = LocationState(
|
||||
context = mockk<Context>(relaxed = true),
|
||||
scope = scope,
|
||||
isForeground = foreground,
|
||||
locationSource = probe.source(),
|
||||
)
|
||||
|
||||
@Test
|
||||
fun doesNotListenWhileBackgrounded() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val probe = neverEmits()
|
||||
val foreground = MutableStateFlow(false)
|
||||
val state = stateWith(backgroundScope, foreground, probe)
|
||||
state.setLocationPermission(true)
|
||||
|
||||
backgroundScope.launch { state.geohashStateFlow.collect { } }
|
||||
// Not a test artefact: `gate` is a `combine`, so it produces nothing
|
||||
// until `settledForeground` first emits, and this test starts
|
||||
// backgrounded — `settledForeground` waits out BACKGROUND_GRACE_MS
|
||||
// before its first emission (ForegroundTracker starts at `false`).
|
||||
// LocationState's own KDoc calls this "known and harmless"; without
|
||||
// advancing past it here, the gate never resolves and the assertion
|
||||
// below would pass vacuously regardless of the implementation.
|
||||
advanceTimeBy(LocationState.BACKGROUND_GRACE_MS + 1)
|
||||
advanceUntilIdle()
|
||||
|
||||
assertEquals(0, probe.subscriptions)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun listensOnceWhileForegrounded() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val probe = neverEmits()
|
||||
val foreground = MutableStateFlow(true)
|
||||
val state = stateWith(backgroundScope, foreground, probe)
|
||||
state.setLocationPermission(true)
|
||||
|
||||
backgroundScope.launch { state.geohashStateFlow.collect { } }
|
||||
advanceUntilIdle()
|
||||
|
||||
assertEquals(1, probe.subscriptions)
|
||||
assertEquals(1, probe.live)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun releasesTheSourceAfterTheGracePeriodAndKeepsTheLastFix() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val probe = emitsOnceThenHangs(56.048839, 12.721029)
|
||||
val foreground = MutableStateFlow(true)
|
||||
val state = stateWith(backgroundScope, foreground, probe)
|
||||
state.setLocationPermission(true)
|
||||
|
||||
backgroundScope.launch { state.geohashStateFlow.collect { } }
|
||||
advanceUntilIdle()
|
||||
|
||||
val fixWhileForeground = state.geohashStateFlow.value
|
||||
assertTrue("expected a Success, got $fixWhileForeground", fixWhileForeground is LocationState.LocationResult.Success)
|
||||
|
||||
foreground.value = false
|
||||
// Real grace-period wait, not a test artefact: `settledForeground`
|
||||
// delays BACKGROUND_GRACE_MS before it settles to `false`, so the
|
||||
// gate has genuinely not transitioned to Paused until this elapses.
|
||||
advanceTimeBy(LocationState.BACKGROUND_GRACE_MS + 1)
|
||||
advanceUntilIdle()
|
||||
|
||||
assertEquals("source must be released once backgrounded", 0, probe.live)
|
||||
assertEquals(
|
||||
"the last geohash must survive the release for the ~60 synchronous .value readers",
|
||||
fixWhileForeground,
|
||||
state.geohashStateFlow.value,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun keepsListeningAcrossABackgroundEdgeShorterThanTheGracePeriod() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val probe = neverEmits()
|
||||
val foreground = MutableStateFlow(true)
|
||||
val state = stateWith(backgroundScope, foreground, probe)
|
||||
state.setLocationPermission(true)
|
||||
|
||||
backgroundScope.launch { state.geohashStateFlow.collect { } }
|
||||
advanceUntilIdle()
|
||||
assertEquals(1, probe.subscriptions)
|
||||
|
||||
foreground.value = false
|
||||
advanceTimeBy(LocationState.BACKGROUND_GRACE_MS / 2)
|
||||
foreground.value = true
|
||||
advanceUntilIdle()
|
||||
|
||||
assertEquals("a brief app switch must not tear down the registration", 1, probe.subscriptions)
|
||||
assertEquals(1, probe.live)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun doesNotReemitLoadingWhenReturningToForegroundWithACachedFix() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val probe = emitsOnceThenHangs(56.048839, 12.721029)
|
||||
val foreground = MutableStateFlow(true)
|
||||
val state = stateWith(backgroundScope, foreground, probe)
|
||||
state.setLocationPermission(true)
|
||||
|
||||
val seen = mutableListOf<LocationState.LocationResult>()
|
||||
backgroundScope.launch { state.geohashStateFlow.collect { seen.add(it) } }
|
||||
advanceUntilIdle()
|
||||
|
||||
foreground.value = false
|
||||
// Same real grace-period wait as above — needed so the gate actually
|
||||
// reaches Paused before we flip back to foreground and look for a
|
||||
// stray Loading.
|
||||
advanceTimeBy(LocationState.BACKGROUND_GRACE_MS + 1)
|
||||
advanceUntilIdle()
|
||||
val afterBackground = seen.size
|
||||
|
||||
foreground.value = true
|
||||
advanceUntilIdle()
|
||||
|
||||
assertTrue(
|
||||
"returning to foreground must not flash Loading — AroundMeFeedFlow renders an empty feed for it. Saw: ${seen.drop(afterBackground)}",
|
||||
seen.drop(afterBackground).none { it is LocationState.LocationResult.Loading },
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* NOTE — does not discriminate the `Gate.Listen` Loading-guard it might sound
|
||||
* like it covers: `geohashStateFlow` is seeded via
|
||||
* `stateIn(..., latestLocation)`, and `latestLocation` defaults to `Loading`
|
||||
* at construction, so *every* fresh collector on a brand-new [LocationState]
|
||||
* sees `Loading` first purely from that seed — regardless of whether
|
||||
* `Gate.Listen`'s own `if (latest() !is Success) emit(Loading)` line exists,
|
||||
* is guarded, or is deleted outright. Verified: deleting that emit still
|
||||
* leaves this test green. The guard's "only when nothing is cached" half IS
|
||||
* covered, by [doesNotReemitLoadingWhenReturningToForegroundWithACachedFix].
|
||||
* The "emits Loading when nothing is cached" half is not protected by any
|
||||
* test in this file. What this test does verify — honestly — is that a
|
||||
* fresh [LocationState] surfaces `Loading` before any fix arrives.
|
||||
*/
|
||||
@Test
|
||||
fun freshStateSurfacesLoadingBeforeAnyFix() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val probe = neverEmits()
|
||||
val foreground = MutableStateFlow(true)
|
||||
val state = stateWith(backgroundScope, foreground, probe)
|
||||
state.setLocationPermission(true)
|
||||
|
||||
val seen = mutableListOf<LocationState.LocationResult>()
|
||||
backgroundScope.launch { state.geohashStateFlow.collect { seen.add(it) } }
|
||||
advanceUntilIdle()
|
||||
|
||||
assertTrue("expected Loading, saw $seen", seen.any { it is LocationState.LocationResult.Loading })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun reportsLackPermissionRegardlessOfForeground() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val probe = neverEmits()
|
||||
val foreground = MutableStateFlow(true)
|
||||
val state = stateWith(backgroundScope, foreground, probe)
|
||||
state.setLocationPermission(false)
|
||||
|
||||
backgroundScope.launch { state.geohashStateFlow.collect { } }
|
||||
advanceUntilIdle()
|
||||
|
||||
assertEquals(LocationState.LocationResult.LackPermission, state.geohashStateFlow.value)
|
||||
assertEquals(0, probe.subscriptions)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun requestsTheDocumentedPollingProfilePerFlow() =
|
||||
runTest(UnconfinedTestDispatcher()) {
|
||||
val foreground = MutableStateFlow(true)
|
||||
|
||||
val coarseProbe = neverEmits()
|
||||
val coarseState = stateWith(backgroundScope, foreground, coarseProbe)
|
||||
coarseState.setLocationPermission(true)
|
||||
backgroundScope.launch { coarseState.geohashStateFlow.collect { } }
|
||||
|
||||
val preciseProbe = neverEmits()
|
||||
val preciseState = stateWith(backgroundScope, foreground, preciseProbe)
|
||||
preciseState.setLocationPermission(true)
|
||||
backgroundScope.launch { preciseState.preciseGeohashStateFlow.collect { } }
|
||||
|
||||
advanceUntilIdle()
|
||||
|
||||
assertEquals(
|
||||
"geohashStateFlow must poll at the coarse profile",
|
||||
LocationState.COARSE_MIN_TIME to LocationState.COARSE_MIN_DISTANCE,
|
||||
coarseProbe.requestedProfile,
|
||||
)
|
||||
assertEquals(
|
||||
"preciseGeohashStateFlow must poll at the precise profile",
|
||||
LocationState.PRECISE_MIN_TIME to LocationState.PRECISE_MIN_DISTANCE,
|
||||
preciseProbe.requestedProfile,
|
||||
)
|
||||
}
|
||||
}
|
||||
+50
@@ -0,0 +1,50 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.location
|
||||
|
||||
import android.location.LocationListener
|
||||
import android.location.LocationManager
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
|
||||
/**
|
||||
* A [LocationManager] that reports [providers], has no cached fix, and refuses
|
||||
* [denied] with a `SecurityException` — mimicking the pre-API-31 fine-location
|
||||
* requirement.
|
||||
*
|
||||
* Shared by [LocationFlowTest] and [LocationLedgerCompositionTest] so the mock's
|
||||
* surface tracks the binder calls [LocationFlow] actually makes in one place.
|
||||
*/
|
||||
internal fun mockLocationManager(
|
||||
providers: List<String>,
|
||||
denied: Set<String> = emptySet(),
|
||||
): LocationManager {
|
||||
val lm = mockk<LocationManager>(relaxed = true)
|
||||
every { lm.allProviders } returns providers
|
||||
every { lm.getLastKnownLocation(any()) } returns null
|
||||
every {
|
||||
lm.requestLocationUpdates(any<String>(), any<Long>(), any<Float>(), any<LocationListener>(), any())
|
||||
} answers {
|
||||
val provider = firstArg<String>()
|
||||
if (provider in denied) throw SecurityException("denied: $provider")
|
||||
}
|
||||
return lm
|
||||
}
|
||||
+70
@@ -0,0 +1,70 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.amethyst.service.resourceusage
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
|
||||
class RefCountedSessionTest {
|
||||
@Test
|
||||
fun overlappingHoldersKeepTheSessionOpenAndReportOnlyTransitions() {
|
||||
val calls = mutableListOf<Boolean>()
|
||||
val session = RefCountedSession { calls.add(it) }
|
||||
|
||||
session.setActive(true) // holders 1 — inactive -> active
|
||||
session.setActive(true) // holders 2 — a second listener joins, no transition
|
||||
session.setActive(false) // holders 1 — the first one leaves, still active
|
||||
|
||||
assertEquals("only the 0 -> 1 edge is a transition", listOf(true), calls)
|
||||
|
||||
session.setActive(false) // holders 0 — the last one leaves
|
||||
|
||||
assertEquals(listOf(true, false), calls)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun unmatchedReleaseDoesNotDriveTheCountNegative() {
|
||||
val calls = mutableListOf<Boolean>()
|
||||
val session = RefCountedSession { calls.add(it) }
|
||||
|
||||
session.setActive(false)
|
||||
session.setActive(false)
|
||||
|
||||
assertEquals("releasing an idle session is a no-op", emptyList<Boolean>(), calls)
|
||||
|
||||
// If the count had gone to -2, one acquire would leave it at -1 and
|
||||
// report inactive. It must open the session instead.
|
||||
session.setActive(true)
|
||||
|
||||
assertEquals(listOf(true), calls)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aSingleHolderOpensAndClosesTheSession() {
|
||||
val calls = mutableListOf<Boolean>()
|
||||
val session = RefCountedSession { calls.add(it) }
|
||||
|
||||
session.setActive(true)
|
||||
session.setActive(false)
|
||||
|
||||
assertEquals(listOf(true, false), calls)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user