-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: Implement API fallback system #3343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
mostafaNazari702
wants to merge
7
commits into
ReVanced:dev
Choose a base branch
from
mostafaNazari702:feat/api-resilience-fallback-system
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
a146676
feat: API reselience and fallback system
mostafaNazari702 19c0712
Changes per receeved feedback.
mostafaNazari702 afc2898
removed broken build and added a clarifying comment
mostafaNazari702 c4bb7f1
Addressed some of Axel's requests to change and removed remains
mostafaNazari702 0b0633d
use apply over also for api_cache dir
mostafaNazari702 0cd3608
restore primary endpoint silently via periodic reconnect
mostafaNazari702 c2a87e8
support chained API fallback with prioritized recovery
mostafaNazari702 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
77 changes: 77 additions & 0 deletions
77
app/src/main/java/app/revanced/manager/network/api/EndpointState.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| package app.revanced.manager.network.api | ||
|
|
||
| import android.util.Log | ||
| import app.revanced.manager.domain.manager.PreferencesManager | ||
| import app.revanced.manager.util.tag | ||
| import kotlinx.coroutines.flow.MutableStateFlow | ||
| import kotlinx.coroutines.flow.StateFlow | ||
| import kotlinx.coroutines.flow.asStateFlow | ||
|
|
||
| class EndpointState( | ||
| private val prefs: PreferencesManager, | ||
| ) { | ||
|
|
||
| // A node in the prioritised API endpoint chain. | ||
| // [fallback] is the next lower-priority endpoint to try while this one is unreachable so the chain reads from the primary/head downward. | ||
| // Restoration to a higher-priority endpoint is derived by walking from the head (Ctrl + F("restoreCandidates")., the chain is single-direction because a doubly-linked immutable chain is not constructible. | ||
|
|
||
| data class ApiEndpoint( | ||
| val url: String, | ||
| val fallback: ApiEndpoint?, | ||
| ) { | ||
| fun asSequence(): Sequence<ApiEndpoint> = generateSequence(this) { it.fallback } | ||
| } | ||
|
|
||
| // URL of the endpoint currently in use, null means the primary (chain head). | ||
| private val _activeUrl = MutableStateFlow<String?>(null) | ||
| val activeUrl: StateFlow<String?> = _activeUrl.asStateFlow() | ||
|
|
||
| suspend fun primaryUrl(): String = prefs.api.get().trimEnd('/') | ||
|
|
||
| // the persisted endpoints, ordered from primary downward. The API currently advertises a single backup but the chain is built from a list so additional tiers need no structural changes. | ||
|
|
||
| private suspend fun endpointUrls(): List<String> = | ||
| listOf(primaryUrl(), prefs.apiFallback.get().trimEnd('/')).distinct() | ||
|
|
||
| // The endpoint chain built from persisted configuration, head (primary) first. | ||
| suspend fun chain(): ApiEndpoint { | ||
| var node: ApiEndpoint? = null | ||
| endpointUrls().asReversed().forEach { url -> node = ApiEndpoint(url, node) } | ||
| return node!! | ||
| } | ||
|
|
||
| // The endpoint currently in use or the chain head when on the primary. | ||
| suspend fun activeEndpoint(): ApiEndpoint { | ||
| val chain = chain() | ||
| val active = _activeUrl.value ?: return chain | ||
| return chain.asSequence().firstOrNull { it.url == active } ?: chain | ||
| } | ||
|
|
||
| // Higher-priority endpoints to probe for restoration, ordered from the primary downward. | ||
| suspend fun restoreCandidates(): List<ApiEndpoint> { | ||
| val active = _activeUrl.value ?: return emptyList() | ||
| return chain().asSequence().takeWhile { it.url != active }.toList() | ||
| } | ||
|
|
||
| // Records [endpoint] as the active endpoint. Resets to the primary when the head is selected. | ||
| suspend fun setActive(endpoint: ApiEndpoint) { | ||
| _activeUrl.value = endpoint.url.takeIf { it != primaryUrl() } | ||
| } | ||
|
|
||
| suspend fun updateFallbackFromAbout(advertised: String?) { | ||
| val normalized = advertised?.trim()?.trimEnd('/').orEmpty() | ||
| if (normalized.isEmpty()) return | ||
| if (!normalized.startsWith("https://")) { | ||
| Log.w(tag, "EndpointState: ignoring non-HTTPS fallback URL from /about: $normalized") | ||
| return | ||
| } | ||
| if (normalized == prefs.apiFallback.get().trimEnd('/')) return | ||
| Log.i(tag, "EndpointState: updating persisted fallback endpoint to $normalized") | ||
| prefs.apiFallback.update(normalized) | ||
| } | ||
|
|
||
| companion object { | ||
| const val DEFAULT_PRIMARY_API_URL = "https://api.revanced.app" | ||
| const val DEFAULT_FALLBACK_API_URL = "https://backup-api.revanced.app" | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this class works for chained fallbacks and it is also overcomplicated. You simply need one persistence for a schema:
API(url: String, fallback: String?, restore: API?)
If current API is down -> fallback
If restore != null, in an interval probe all restore APIs recursively, the earliest wins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reworked EndpointState into the priority-chain setup you described. Endpoints are now represented as "ApiEndpoint(url, fallback)" and the behavior matches what we discussed on Discord, if i rememeber correctly.
Requests always try the active endpoint first. if that one is down we walk the chain from the top and pick the first reachable endpoint, so higher-priority endpoints always win. for example if we are currently on C and it dies while A or B are available we will move back up to A/B instead of falling further down to D.
I also added the recovery logic for backup endpoints. while we are running on a backup, every 5 minutes we check whether any higher-priority endpoints have come back. If they have, we switch to the highest priority one that's reachable. over time that means we naturally climb back up the chain (e.g C then to B tehn to A) until we're back on the primary then the recovery loop stops doing anything.
I didnt keep an explicit "restore" link on the endpoint node. A doubly-linked immutable structure (A.fallback = B and B.restore = A) is awkward to construct with vals so the node only stores the fallback direction. when we need restore candidates we derive them by walking the chain from the head. Same behavior, just simpler to build and maintain ig.
Also worth noting that /about only exposes a single fallback today, so in practice we are still running with two tiers. The code just no longer assumes that's the limit, so if we add more fallback levels later, it should work without any further changes.
the literal "restore" field can be added back if you'd rather keep the structure closer to the original proposal, since we kinda....shifted a lot during the...journey?