mapbox / mapbox/mapbox-navigation-android

Expose API to check if offline regions are expired

Open
#4,471 10 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

jira-sync-complete
Dominant language
Kotlin
Stars
651
Forks
321
PR merge metrics
No merged PRs in 30d

Description

`TileStore` has a method `tileRegionContainsDescriptors(...)` which can be used by a customer to check single regions expiration.
We need provide some extensions to make such checks easier
It might be:
```
fun TileStore.checkAllRegionsTilesExpired(
descriptorFactory: TilesetDescriptorFactory,
callback: RegionsTilesCheckExpiredCallback
)
```
and for a single region
```
fun TileStore.checkRegionTilesExpired(
regionId: String,
descriptorFactory: TilesetDescriptorFactory,
callback: RegionTilesCheckExpiredCallback
)
```
but in that case on android we can't cover it with unit tests, `TileStore` is a c++ class, it's methods can't be mocked without an additional dependency like `PowerMockito`.

also such methods should be implemented on each platform and we might get additional bugs.

Example (iOS has pretty close logic):
```kotlin
fun TileStore.checkAllRegionsTilesExpired(
descriptorFactory: TilesetDescriptorFactory,
callback: RegionsTilesCheckExpiredCallback
) {
val latestDescriptors = descriptorFactory.getLatest()
getAllTileRegions {
if (it.isValue) {
val regions = it.value!!
val expiredRegions = mutableListOf()
val countDownLatch = CountDownLatch(regions.size)
regions.forEach { region ->
tileRegionContainsDescriptors(region.id, listOf(latestDescriptors)) { result ->
if (result.isValue) {
val regionIsExpired = result.value!!.not()
if (regionIsExpired) {
expiredRegions.add(region.id)
}
}
countDownLatch.countDown()
}
}
countDownLatch.await()

if (expiredRegions.isEmpty()) {
callback.onTilesUpToDate()
} else {
callback.onTilesExpired(expiredRegions)
}
} else {
callback.onError("type: ${it.error?.type}, message: ${it.error?.message}")
}
}
}

interface RegionsTilesCheckExpiredCallback {
fun onTilesUpToDate()
fun onTilesExpired(regionIds: List)
fun onError(message: String)
}
```

Should we ask `common` team to expose such API? `Common` should receive `descriptors` to make a check, so
- it should be provided by a customer
- Platforms can implement short wrappers like
```kotlin
fun TileStore.checkAllRegionsTilesExpired(callback: RegionsTilesCheckExpiredCallback) {
val latestDescriptors = descriptorFactory.getLatest()
// pass `latestDescriptors` to `common`
}
```

it's a little confusing that platforms need to write the same extensions. It means that base API is not enough and it should be improved, doesn't it?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with TileStore.tileRegionContainsDescriptors(...) and the proposed checkAllRegionsTilesExpired and checkRegionTilesExpired extensions. Compare the Android proposal with the referenced iOS logic and determine whether the API belongs in common or platform wrappers; done means the ownership, descriptor flow, callbacks, and testability approach are resolved.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, kotlin
Domain
api, mobile
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.