feat: Downloader API proposal
Feature description
Downloader API proposal
A downloader must specify a download function and a strategy.
The downloader provides a DSL in which a strategy can be invoked. In this case we want two strategies. An api() and webview() strategy.
API strategy
The API strategy is to use an API just like the name suggests. The strategy has a find function to find a download link from the requested app and version. The function returns a final download link.
The download function is called once the patching process begins. The download link earlier returned by find is used here to download the requested app:
val downloader = downloader {
api {
find { app, version -> "https://example.com/$app/$version" }
}
download { link-> URL(link).inputStream() }
}
WebView strategy
The WebView strategy is helpful because some sites may be gated behind anti-scraping or captcha systems, or some sites would prefer not to be scraped and instead want to generate revenue
The solution is to find the download link by displaying the user a webview. The user navigates the site until the download button is pressed for example. The download function hooks requests, and once it sees the response for the download request, it can capture the download URL:
val downloader = downloader {
webView {
show { app, version -> "https://example.com/$app/$version" }
capture { request, response -> response.takeIf { it.url == "https://cdn.com/app.apk" }?.url }
}
download { url -> url.inputStream() }
}
As seen, the show function receives the requested app and version and, in turn, builds a link as a starting point for displaying the webview. The user navigates the page, triggering requests. All requests/ responses are hooked by the function capture. Once capture returns a string (the download link), Manager ends the web view and saves the download link. The download earlier returned by capture is used here to download the requested app.
The download function
The argument of download should be generic and specified by the strategies. For example this would bind the generic type to URL:
val downloader = downloader {
webView<URL> {
show { app, version -> "https://example.com/$app/$version" }
capture { request, response -> response.takeIf { it.url == "https://cdn.com/app.apk" }?.url }
}
download { url -> url.inputStream() }
}
This would bind the generic type to String:
val downloader = downloader {
api<String> {
find { app, version -> "https://example.com/$app/$version" }
}
download { link-> URL(link).inputStream() }
}
This is useful when the downloader may want to share a custom object to download.
Full example
Click me
suspend fun test() {
downloader {
api { find { app, version -> "$app/$version" } }
download { URI.create(it).toURL().openStream() to null }
}.get("app", null).download()
downloader {
webView {
show { app, version -> "$app/$version" }
capture { _, response -> response.url to response.header["size"]!!.toInt() }
}
download { result: Pair<String, Int> -> URL(result.first).openStream() to result.second }
}.get("app", "1.0").download()
}
interface Strategy<T> {
suspend fun get(app: String, version: String?): T
}
class ApiStrategy<T>(
val find: suspend (app: String, version: String?) -> T
) : Strategy<T> {
override suspend fun get(app: String, version: String?) = find(app, version)
}
class ApiStrategyBuilder<T> {
private var find: (suspend (app: String, version: String?) -> T)? = null
fun find(find: suspend (app: String, version: String?) -> T) {
this.find = find
}
fun build(): ApiStrategy<T> {
return ApiStrategy(find!!)
}
}
fun <T> DownloaderBuilder<T>.api(block: ApiStrategyBuilder<T>.() -> Unit) =
strategy(ApiStrategyBuilder<T>().apply(block).build())
class Request
class Response(val url: String, val header: Map<String, String>)
class WebViewStrategy<T>(
val show: (app: String, version: String?) -> String,
val capture: (request: Request, response: Response) -> T?
) : Strategy<T> {
override suspend fun get(app: String, version: String?): T {
val uri = show(app, version)
// Show a webview here and somehow capture requests/responses to get the result
return capture(Request(), Response(uri, mapOf("size" to "1024")))!!
}
}
class WebViewStrategyBuilder<T> {
private var show: ((app: String, version: String?) -> String)? = null
private var capture: ((request: Request, response: Response) -> T?)? = null
fun show(show: (app: String, version: String?) -> String) {
this.show = show
}
fun capture(capture: (request: Request, response: Response) -> T?) {
this.capture = capture
}
fun build(): WebViewStrategy<T> {
return WebViewStrategy(show!!, capture!!)
}
}
fun <T> DownloaderBuilder<T>.webView(block: WebViewStrategyBuilder<T>.() -> Unit) =
strategy(WebViewStrategyBuilder<T>().apply(block).build())
class Downloader<T>(
private val strategy: Strategy<T>,
private val download: suspend (T) -> Pair<InputStream, Int?>
) {
class Downloadable<T>(
private val downloader: Downloader<T>,
private val result: T
) {
suspend fun download() = downloader.download(result)
}
suspend fun get(app: String, version: String?) = Downloadable(this, strategy.get(app, version))
}
class DownloaderBuilder<T> {
private var strategy: Strategy<T>? = null
private var download: ((T) -> Pair<InputStream, Int?>)? = null
internal fun strategy(strategy: Strategy<T>) {
this.strategy = strategy
}
fun download(download: (T) -> Pair<InputStream, Int?>) {
this.download = download
}
fun build(): Downloader<T> {
return Downloader(strategy!!, download!!)
}
}
fun <T> downloader(block: DownloaderBuilder<T>.() -> Unit) = DownloaderBuilder<T>().apply(block).build()
Questions
- For the webview downloader, when do we show the webview? Once the downloader is selected (before the patching process begins) or once the patching process starts?
- We may want to configure downloaders. An API design isn't yet available and must be discussed. For example, some APIs need an API key, and we may want to allow the user to enter an API key.
An example API could look like this:
val downloader = downloader {
val api: String by option("API URL", "The API obtained from 'https://example.com/dashboard'")
webview<URL> {
show { app, version -> "https://example.com/$app/$version?api=$api" }
capture { request, response -> response.takeIf { it.url == "https://cdn.com/app.apk" }?.url }
}
download { url -> url.inputStream() }
}
Acknowledgements
- [X] This issue is not a duplicate of an existing feature request.
- [X] I have chosen an appropriate title.
- [X] The feature request is only related to ReVanced Manager
I don't think webviews need to be part of this API. A more flexible alternative would be for manager to provide a way to launch activities, which can contain webviews or other authentication mechanisms:
// API code
interface ActivityLaunchPermit {
// Launches an activity and returns the resulting intent on completion.
suspend fun launch(intent: Intent): Intent?
}
// Plugin code
val downloader = downloader {
get {
val permit = requestUserInteraction() // Suspends until the user allows interaction and returns an ActivityLaunchPermit
val result = permit.launch(launchWebViewActivity)
// The result intent can contains the data that the plugin needs
...
}
download { ... }
}
Utilities for webviews can be implemented in a separate library if it emerges as a common pattern
For the webview downloader, when do we show the webview? Once the downloader is selected (before the patching process begins) or once the patching process starts?
I believe the most appropriate place would be when it reaches the Download APK step in the patching process, a popup would be shown explaining to the user what they need to do
We may want to configure downloaders. An API design isn't yet available and must be discussed. For example, some APIs need an API key, and we may want to allow the user to enter an API key.
For the API design, we should be able to re-use patch options for this, code for it exists in both the API form and the frontend form so it should simplify the process
@Axelen123
I don't think webviews need to be part of this API.
I believe it would be more readable and expressive if you "installed" a strategy via the DSL. What you described as "get" here is the strategy interface. Here is the explanation:
interface Strategy<T> {
suspend fun get(app: String, version: String?): T
}
As you can see now, you can do the following, which is on par with what you wanted:
downloader<Pair<String, String>> {
strategy(
object : Strategy<Pair<String, String>> {
override suspend fun get(app: String, version: String?): Pair<String, String> {
val permit = requestUserInteraction() // Suspends until the user allows interaction and returns an ActivityLaunchPermit
val result = permit.launch(launchWebViewActivity)
// The result intent can contains the data that the plugin needs
return result
}
}
)
download { (url, size) -> URL(url).openStream() to size.toInt() }
}
Now this is the part you said we should leave out: This anonymous object is wrapped into a nice API:
class WebViewStrategy<T>(
val show: suspend (app: String, version: String?) -> String,
val capture: suspend (request: Request, response: Response) -> T?
) : DownloaderStrategy<T> {
override suspend fun get(app: String, version: String?): T {
// show & capture logic
}
}
We add a small DSL API:
class WebViewStrategyBuilder<T> {
private var show: ((app: String, version: String?) -> String)? = null
private var capture: ((request: Request, response: Response) -> T?)? = null
fun show(show: (app: String, version: String?) -> String) {
this.show = show
}
fun capture(capture: (request: Request, response: Response) -> T?) {
this.capture = capture
}
fun build(): WebViewStrategy<T> {
return WebViewStrategy(show!!, capture!!)
}
}
fun <T> DownloaderBuilder<T>.webView(block: WebViewStrategyBuilder<T>.() -> Unit) =
strategy(WebViewStrategyBuilder<T>().apply(block).build())
and now we can use it:
downloader {
webView {
show { app, version -> "$app/$version" }
capture { _, response -> response.url to response.header["size"]!!.toInt() }
}
download { result: Pair<String, Int> -> URL(result.first).openStream() to result.second }
}.get("app", "1.0").download()
I have added a full example to OP for reference and testing.
@Ushie
I believe the most appropriate place would be when it reaches the Download APK step in the patching process; a popup would be shown explaining to the user what they need to do
Manager is agnostic to the implementation of the strategy. All it sees is this:
downloader.get("pkg", "version").download()
This is all it has to work with. So now, if you say we call get, which in turn launches the webview strategy right before the patching process, this means we would also call get for the API strategy right before the patching process. That must be kept in mind.
Manager is agnostic to the implementation of the strategy. All it sees is this:
downloader.get("pkg", "version").download()This is all it has to work with. So now if you say we call get which in turn launches the webview strategy, right before the patching process, this means, we would also call get for the API strategy right before the patching process. That mus be kept in mind.
Yes, that is fine, it either continues the patching process undisrupted or requests user interaction, temporarily suspending the patch process until the result is obtained
The API should allow the plugin to customize what is displayed in the popup, as different implementations require different popups and currently there is no limit to what the implementations can be, although this is annoying for localization
Yes, that is fine, it either continues the patching process undisrupted or requests user interaction, temporarily suspending the patch process until the result is obtained
I don't understand. The get function is intended for Manager so that manager can show a "This downloader couldn't find a suitable download` before the user starts patching. Otherwise it would be annoying to go back do all the steps again.
Yes, that is fine, it either continues the patching process undisrupted or requests user interaction, temporarily suspending the patch process until the result is obtained
I don't understand. The
getfunction is intended for Manager so that manager can show a "This downloader couldn't find a suitable download` before the user starts patching. Otherwise it would be annoying to go back do all the steps again.
ReVanced Manager can loop over all available plugins until it finds a suitable download, if it fails to find any then it will request the user to supply an APK themselves and give them a shortcut to a Google search, similar to the one in ReVanced Manager v1
until it finds a suitable download
This implies making calls to get.
So I assume you are talking about get being called after the user selects a non installed app. In that case, sounds good, edit the OP so we can keep track of updates.
until it finds a suitable download
This implies making calls to
get.So I assume you are talking about
getbeing called after the user selects a non installed app. In that case, sounds good, edit the OP so we can keep track of updates.
No, I'm talking about calling get in the patch process, the user doesn't have to select what provider to download with, they can have multiple
When they reach the patch process, ReVanced Manager loops over the multiple providers installed and either it finds one that includes the app, then proceeds to do its strategy, or it loops through all of them unsuccessfully and offers the user to select an APK from storage
calling get in the patch process
I think its better to check if the downloader can get the download after selecting a non installed app. Once get returns non null, ReVanced Manager progresses to the next screen and in the patch process screen it calls download.
@oSumAtrIX what is the point of the strategy interface? We can still support webView blocks even if we just have get. Also, requestUserInteraction is implemented by manager so there needs to be some sort of receiver scope.
The strategy is just an abstraction of the get() api. As you can see the api strategy has a readable and to an api domain specific find function. The webview one has a domain specific show and capture function. If you were to remove those and instead just show get() you lose semantics, and make it slightly less readable. The DSL also reduces duplicate code.
Like explained before your suggested get() is the get() inside the strategy interface, just that the interface allows for proper domain specific abstraction
Strategies seem odd here, Manager shouldn't care about the implementation of your plugin and we shouldn't assume specifics about implementations either. Just using get is simpler and doesn't require you to define anonymous objects if whatever you are trying to do isn't served by the existing strategies.
The strategy is just an abstraction of the get() api. As you can see the api strategy has a readable and to an api domain specific find function.
api {
find { app, version -> Something() }
}
Is longer than:
get { app, version ->
Something()
}
The webview one has a domain specific show and capture function. If you were to remove those and instead just show get() you lose semantics, and make it slightly less readable. The DSL also reduces duplicate code.
Webview helpers can be implemented in a library that extends this plugin API, we don't need it here. It is already possible to implement without requiring special treatment because plugins can launch their own activities through the requestUserInteraction API and have the webview there.
Manager shouldn't care about the implementation of your plugin and we shouldn't assume specifics about implementations either
That is correct. Downloader only sees: downloader.get("pkg", "version").download(). As you can see, there are no details on the implementation of the strategy behind get.
Just using get is simpler and doesn't require you to define anonymous objects if whatever you are trying to do isn't served by the existing strategies.
In the case of the API strategy, find is basically get. But in the case of webview, we have show and capture instead of get. If you were to replace that with get, every downloader that needs a webview would have to implement the boilerplate which the webView() function already has.
Webview helpers can be implemented in a library that extends this plugin API
Why not ship it to the library? If you outsource it, the strategy setter must be public and thus visible via the API. Right now its internal which is hidden from the public API.
because plugins can launch their own activities through the requestUserInteraction API
I am not aware of such an API. I only know what I proposed so far. If you intend to implement a requestUserInteraction API you have wrong abstraction in the case of an API downloader. In my proposal this is not possible, because if you use the api function you don't have access to the show function and vice versa.
This is how you would use requestUserInteraction:
// These two interfaces are implemented by Manager.
interface GetScope {
suspend fun requestUserInteraction(): ActivityLaunchPermit
}
interface ActivityLaunchPermit {
// Launches an activity using [intent] and returns the result intent.
suspend fun launch(intent: Intent): Intent?
}
// Usage
val downloader = downloader {
get { packageName, version ->
// requestUserInteraction() suspends until the user allows or denies the request.
val permit = requestUserInteraction()
val result = permit.launch(Intent().apply {
// This activity shows the webview and returns the URL.
`package` = "my.downloader.plugin"
action = "LAUNCH_WEBVIEW"
putExtra("PACKAGE_NAME", packageName)
putExtra("VERSION", version)
})!!
val url = result.getStringExtra("URL")
..
}
}
A convenient extension function for the get() implementation and a base class for the Webview activity can be provided in another library. The extension function only needs the public API of GetScope so there is no problem there. A major advantage of having the webview APIs in another library is that plugins which don't use them don't have to bring in the webview dependency, which reduces APK size.
Can you explain why we give plugins low-level APIs, such as when launching an activity? To me, it sounds like they will never need that.
A small and concise API where you can only do a limited but controlled and overseeable amount of things sounds simpler and more reasonable to me,.
The webView function is an extension to the downloader builder. It is not a direct member:
fun <T> DownloaderBuilder<T>.webView(block: WebViewStrategyBuilder<T>.() -> Unit) =
strategy(WebViewStrategyBuilder<T>().apply(block).build())
Same for api:
fun <T> DownloaderBuilder<T>.api(block: ApiStrategyBuilder<T>.() -> Unit) =
strategy(ApiStrategyBuilder<T>().apply(block).build())
As you can see they merely call strategy which is a direct member of DownloaderBuilder.
Plugins which do not need a webview, simply don't call it. I don't think moving these to another library next to the existing plugin library makes sense. Having some readymade extension functions there such as webView and api directly through the plugin library which can be invoked inside the builder scope is minimal and intuitive. They are not too far fetched to be outsourced to a different module but also not directly related to the downloader API which is why they are extension functions and not direct members.
val downloader = downloader {
webView {
show { ... }
capture { ... }
}
}
The "strategy" API is not exposed to the user. It is code that is abstracted away but makes sense to keep in the plugin code for architectural reasons.
If you think a low-level API access such as launching intents is necessary, next to webView and api, a strategy for downloaders to get the download link via an Intent can be added, too. And similarly to downloaders that do not need a webview, those that do not need that low level access to launch activities, simply don't call this extension.
val downloader = downloader {
intent {
launch { app, version -> Intent(app, version) }
result { intent -> intent.getStringExtra("url") to intent.getLongExtra("size") }
}
download { link, size -> URL(link).inputStream() to size }
}
Can you explain why we give plugins low-level APIs, such as when launching an activity? To me, it sounds like they will never need that.
Activities and intents are pretty common on Android. All plugins that require interaction need to use them for UI.
A small and concise API where you can only do a limited but controlled and overseeable amount of things sounds simpler and more reasonable to me,.
The suggested get API is small and you don't need it if you are using webView.
Plugins which do not need a webview, simply don't call it. I don't think moving these to another library next to the existing plugin library makes sense. Having some readymade extension functions there such as
webViewandapidirectly through the plugin library which can be invoked inside the builder scope is minimal and intuitive. They are not too far fetched to be outsourced to a different module but also not directly related to the downloader API which is why they are extension functions and not direct members.val downloader = downloader { webView { show { ... } capture { ... } } }
I am fine with webView being an extension function, but the implementation of it needs to be in a non-compileOnly library because the activity needs to be present inside the plugin APK. Just to clarify, the reason why I believe webView should be an extension function that launches an activity is because in order to show UI, manager and plugins have to cooperate. Activities are a primitive on which basically any UI can be built upon, so manager implementing special support for it seems unnecessary.
If you think a low-level API access such as launching intents is necessary, next to
webViewandapi, a strategy for downloaders to get the download link via an Intent can be added, too. ...val downloader = downloader { intent { launch { app, version -> Intent(app, version) } result { intent -> intent.getStringExtra("url") to intent.getLongExtra("size") } } download { link, size -> URL(link).inputStream() to size } }
I guess I am open to the idea, but what is the benefit of this over using a suspend function? How would the strategy implementation access the required functions to launch activities?
Activities and intents are pretty common on Android. All plugins that require interaction need to use them for UI.
But they would be abstracted by the plugin API. The plugins wouldn't need to interact with activities if the downloader API can simply provide concise and domain specific APIs.
The suggested get API is small and you don't need it if you are using webView.
The get API does not convey any semantics about a webView each plugin would have to implement a function that calls get with an implementation that launches a webview and so on. So it is not just get which is small on its own, its get plus a whole implementation for webview. The plugin API providing this by default simplifies that and is thus smaller for the API user.
it needs to be in a non-compileOnly library
I am not well versed with the intrinsic of activities. I do not understand why it must not be a compileOnly dependency. I was imagining the plugin API would provide an extension function:
fun <T> DownloaderBuilder<T>.webView(block: WebViewStrategyBuilder<T>.() -> Unit) =
strategy(WebViewStrategyBuilder<T>().apply(block).build())
whereas WebViewStrategyBuilder returns a lambda in which your activity stuff would be implemented:
class WebViewStrategy<T>(
val show: (app: String, version: String?) -> String,
val capture: (request: Request, response: Response) -> T?
) : Strategy<T> {
override suspend fun get(app: String, version: String?): T {
val uri = show(app, version)
// Show a webview activity here and somehow capture requests/responses to get the result
return capture(Request(), Response(uri, mapOf("size" to "1024")))!!
}
}
so manager implementing special support for it seems unnecessary.
I did not understand that sentence. Can you reword it/clarify what you mean.
I guess I am open to the idea, but what is the benefit of this over using a suspend function? How would the strategy implementation access the required functions to launch activities?
If you are asking if launch & result are separate, there's no particular reason. Both can be suspendable (because the underlying strategy get function is also suspendable in which both launch and result are invoked. I just laid out an example for you with nice semantics. Of course an intent strategy builder could be implemented like this as well, i was just giving ideas:
val downloader = downloader {
intent {
result { intent ->
val intent = Intent(app, version)
// your suspend stuff goes here
intent.getStringExtra("url") to intent.getLongExtra("size")
}
}
download { link, size -> URL(link).inputStream() to size }
}
But they would be abstracted by the plugin API. The plugins wouldn't need to interact with activities if the downloader API can simply provide concise and domain specific APIs.
Yes, webView abstracts this away for webview plugins. Developers don't have to call requestUserInteraction() if the plugin does not require user input to function. Plugins only need it if they have to show custom UI and are not using webView.
The suggested get API is small and you don't need it if you are using webView.
The
getAPI does not convey any semantics about awebVieweach plugin would have to implement a function that callsgetwith an implementation that launches a webview and so on. So it is not justgetwhich is small on its own, itsgetplus a whole implementation for webview. The plugin API providing this by default simplifies that and is thus smaller for the API user.
Webview plugins don't need to define a get block because webView does it for them.
it needs to be in a non-compileOnly library
I am not well versed with the intrinsic of activities. I do not understand why it must not be a compileOnly dependency. I was imagining the plugin API would provide an extension function:
fun <T> DownloaderBuilder<T>.webView(block: WebViewStrategyBuilder<T>.() -> Unit) = strategy(WebViewStrategyBuilder<T>().apply(block).build())whereas
WebViewStrategyBuilderreturns a lambda in which your activity stuff would be implemented:class WebViewStrategy<T>( val show: (app: String, version: String?) -> String, val capture: (request: Request, response: Response) -> T? ) : Strategy<T> { override suspend fun get(app: String, version: String?): T { val uri = show(app, version) // Show a webview activity here and somehow capture requests/responses to get the result return capture(Request(), Response(uri, mapOf("size" to "1024")))!! } }
The webView library contains an activity class, which is final and the user does not directly interact with it. The library needs to be implementation instead of compileOnly because the activity can't be launched later if it is not included in the plugin APK. The usage of webView could be:
val dl = downloader {
webView {
show { URL() }
capture { .. }
}
download { .. }
}
Internally, webView defines a get block that launches the activity I mentioned earlier with the requestUserInteraction API.
so manager implementing special support for it seems unnecessary. I did not understand that sentence. Can you reword it/clarify what you mean.
A dialog is shown to the user asking them if they want to continue with required user interaction or skip the plugin. This is part of the design proposed by @Ushie. In order to implement this, Manager and the plugin need some sort of interface. Implementing it is easier if Manager only needs to worry about activities.
I guess I am open to the idea, but what is the benefit of this over using a suspend function? How would the strategy implementation access the required functions to launch activities?
If you are asking if launch & result are separate, there's no particular reason. Both can be suspendable (because the underlying strategy get function is also suspendable in which both launch and result are invoked. I just laid out an example for you with nice semantics. Of course an intent strategy builder could be implemented like this as well, i was just giving ideas:
val downloader = downloader { intent { result { intent -> val intent = Intent(app, version) // your suspend stuff goes here intent.getStringExtra("url") to intent.getLongExtra("size") } } download { link, size -> URL(link).inputStream() to size } }
I guess this could also be an extension function as well. I would suggest modifying the example at the top of the issue to add the GetScope from my earlier comment so manager can pass on the required APIs.
The webView library contains an activity class, which is final and the user does not directly interact with it. The library needs to be implementation instead of compileOnly because the activity can't be launched later if it is not included in the plugin APK. The usage of webView could be:
The webview API classes are in the plugin API module. ReVanced Manager depends on that module. The Android application plugin shades the module classes into the final APK which means the activity for the webview API is shaded into ReVanced Manager meaning plugins that use the webview API also can use the activity just fine.
Manager and the plugin need some sort of interface. Implementing it is easier if Manager only needs to worry about activities.
Now I understand. So for the webview API you add a WebView activity. For some other API you add another respective activity.
Both the webview and the other APIs would use the requestUserInteraction API of the internal plugin system then I assume?
So the only public API is webview() and co, whereas requestUserInteraction is internal to the plugin API module. Since webview() is implemented in the plugin API module it uses the requestUserInteraction API to display its webview. This sounds fine to me.
Can you give an example of how the internal API design looks like?
The webview API classes are in the plugin API module. ReVanced Manager depends on that module. The Android application plugin shades the module classes into the final APK which means the activity for the webview API is shaded into ReVanced Manager meaning plugins that use the webview API also can use the activity just fine.
I guess that would work. I never considered doing it like that.
Manager and the plugin need some sort of interface. Implementing it is easier if Manager only needs to worry about activities.
Now I understand. So for the webview API you add a WebView activity. For some other API you add another respective activity. Both the webview and the other APIs would use the
requestUserInteractionAPI of the internal plugin system then I assume?
Yes, webview and plugins that need custom UIs both use the requestUserInteraction API. Plugins that use webview don't call requestUserInteraction manually.
So the only public API is webview() and co, whereas
requestUserInteractionis internal to the plugin API module.
By "internal", do you mean completely inaccessible to plugins?
Can you give an example of how the internal API design looks like?
// API
interface GetScope {
suspend fun requestUserInteraction(): ActivityLaunchPermit
}
interface ActivityLaunchPermit {
suspend fun launch(intent: Intent): Intent?
}
// Usage
val downloader = downloader {
get {
// Receiver is "GetScope".
val result = requestUserInteraction().launch(MyIntent())!!
val url = result.getStringExtra("URL")!!
something(url)
}
}
Keep in mind that requestUserInteraction and the ActivityLaunchPermit need to be implemented by manager in some way if you are going to suggest a replacement.
By "internal", do you mean completely inaccessible to plugins?
Yes. I was thinking it was an internal only API so we would provide wrappers to that internal function. webView is public API but calls requestUserInteraction internally. Now, if we want plugins to allow that as well, then we would, just like webView, provide intent for example:
val downloader = downloader {
intent {
launch { Intent(app, version) }
result { it.getStringExtra("url") to it.getLongExtra("size") }
}
download { link, size -> URL(link).inputStream() to size }
}
How would cases where interaction is only required conditionally be handled? A plugin that requires login might not need to ask the user to login if they have already logged in before. intent does not handle this.
Also, please provide an idea for how intent would get access to requestUserInteraction() since it needs to be implemented by manager.
How would cases where interaction is only required conditionally be handled?
I don't quite get the interaction part. Is this just metadata provided by the plugin for ReVanced Manager so that ReVanced Manager can display a "This plugin needs interaction" dialog or similar? If so:
val downloader = downloader {
intent(requiresInteraction = false) {
launch { Intent(app, version) }
result { it.getStringExtra("url") to it.getLongExtra("size") }
}
download { link, size -> URL(link).inputStream() to size }
}
No, the dialog is displayed when requestUserInteraction() is called. The function returns if the user accepts and throws an exception if the user chooses to skip the plugin. In the example I was talking about, the interaction would be something like OAuth or asking the user for credentials. The plugin would use those credentials to do its job. Asking for credentials is unnecessary if the user has already provided them before.
Offtopic
On second thought, having a launch and result function is not really necessary. It is readable and allows for flexible transformation grouped by scope though. The intent scope is the strategy on how we can get a download link. Inside this strategy we launch an intent and transform its result into something consumable by the download scope. So inside the intent scope everything regarding the intent is handled and to the outer scope something is nicely digestable is returned (in the current example an URL and Size pair). So each domain has its own nice small purpose. An alternative would be:
val downloader = downloader {
intent(requiresInteraction = false) {
handle { Intent(app, version) }
}
download { -> URL(it.getStringExtra("url")).inputStream() to it.getLongExtra("size") }
}
Here we simplify the strategy but as a side effect breach the intent scope. The intent now leaks out of the intent scope to the downloader scope. There is no transformation like before using a result function. So I believe having a launch and result function in the strategy is good.
An alternative would be if we can make transformations inside handle:
val downloader = downloader {
intent {
handle {
val result = Intent(app, version).launch(requiresInteraction = false)
result .getStringExtra("url") to result.getLongExtra("size")
}
}
download { link, size -> URL(link).inputStream() to size }
}
Here we retain domain-specific logic in its designated scopes by grouping both the previous launch and handle function into one. Might be less readable though, so I would still tend to my initial proposal.
Ontopic
No, the dialog is displayed when
requestUserInteraction()is called. The function returns if the user accepts and throws an exception if the user chooses to skip the plugin. In the example I was talking about, the interaction would be something like OAuth or asking the user for credentials. The plugin would use those credentials to do its job. Asking for credentials is unnecessary if the user has already provided them before.
But why do we ask for permission? Just let the plugin show a view? Can requestUserInteraction() be called twice? Can you write a small API proposal of how it would look in your OAuth/Credentials example?
But why do we ask for permission? Just let the plugin show a view?
It is part of Ushies design. Also, if there is no prompt the user might be thrown into a bunch of activities for downloaders they don't want to use for the app.
Activity API
The API for launching activities can be simplified. The new API would look like this: launch { Intent() } instead of: requestUserInteraction().launch(Intent()).
The return value of download
The download function shouldn't be forced to return Pair<InputStream, Long>. It will be really awkward for plugin developers if the libraries they are working with don't use InputStream. HTTP client librariies might not give you the size until you have started downloading and I don't know of any libraries that both return the size and an InputStream for downloading the file. A different idea is necessary. Look at the current implementation in the branch if you want to suggest something else.
The return value of strategies (or whatever we are going to call them)
The return value of strategies must implement Parcelable. This makes downloaders easier to work with inside Manager and encourages plugin developers to write data classes that only contain the information needed to find the APK. In the current implementation, the return value is an instance of App, which looks like this:
open class App(open val packageName: String, open val version: String) : Parcelable
The type can be used as-is if the plugin only needs the package name and version for the download. Plugin developers can create a data class that extends App if more data is necessary.
Loading plugins
Plugins are managed as Android apps instead of being handled by manager.
Manager looks for installed applications that have this uses-feature tag in the manifest:
<uses-feature android:name="app.revanced.manager.plugin.downloader" />
The class containing the downloader plugin is declared using a meta-data tag. See the example plugin manifest for a full example. ReVanced Manager will only load plugins if the user marks them as trusted.
It is part of Ushies design. Also, if there is no prompt the user might be thrown into a bunch of activities for downloaders they don't want to use for the app.
So, is this an opt-in API for downloaders? Can they launch intents if they want to without requesting? Is this something we want to provide as an API at all?
API would look like this: launch { Intent() } instead of: requestUserInteraction().launch(Intent()).
I think a better way would be an extension function Intent().requestLaunch() to imply you are requesting something. Just launch doesn't imply you are requesting, which is the important part of this API.
The extension function would set the internal-only lambda similar to how launch { } would. On a related note, is "launch" terminology we invented or something Android uses for starting intents? I would use whatever Android uses, for example, "requestStart" if start is used.
The return value of download
It should not be Pair<InputStream, Long> but Pair<InputStream, Long?> (nullable). An InputStream is the only correct choice here. A file system API is unnecessary for the downloader API and for the downloader implementation. You should use the type that fits the current situation correctly. You wouldn't use String instead of Integer either for counting something. Every downloader will open a stream at some point in time. If you are working with a library that only allows you to download to a file, you will either need to use a different library, as it clearly decided that the File type is necessary instead of an input stream and thus unsuitable to be used for a downloader implementation, or fork the library to suit the situation.
That being said, an input stream should be requested from a downloader. A file would be the incorrect type for the purpose. Regardless, a file-like API may be useful to a downloader. Why? Because the downloader may want to cache something to avoid consuming too much memory, for example. Given this proposal, the downloader can even use inappropriate APIs, abuse the cache to write to a file, and then open an input stream, thus satisfying the return type of the downloader API.
The reason I said "file-like" is because even in the case of caching, the type File is still wrong in the current domain. A more appropriate solution to solve the requirement "Caching" would be to introduce a controlled DSL such as:
cache("name") { stream -> }
cache is an API that, given a name, opens a closeable stream. Downloaders can use it to store something on disk for the lifespan of the downloader. They can access it given the same name at any time during the lifespan.
This is a much cleaner, smaller, and simpler API that precisely fulfills the requirement for caching without breaching the small and controlled domain with big, mostly useless for the current domain APIs provided by File. Due to this, a cache cannot be abused anymore to use libraries that require a File API, as such a requirement makes them automatically unsuitable for use as a library for a downloader implementation (technically, they can create and abuse a temp file using the Java APIs to still use the library even when unfit).
The return value of strategies must implement
Parcelable.
What if I want to return a stream? I don't think that's parcelable. The constraint maybe makes sense outside of the downloader, somewhere deep in the internal code of ReVanced Manager, but that is a technicality that should not impair the implementation of a downloader. The API of a downloader should be predictable, minimal, and simple. Asking for Parcelable raises the question for the user, why the return type must be parcelable. The answer to that would be something the user should not even know about just to implement a downloader. A compromise would be to require an inner member of the type Parcelable. So the API would allow any type to be used but with an upper constraint that requires the type to implement Parcelable. This parcelable would be the "parcelizable" representation of the type it is inside and its purpose would be for the Manager. Like I said, this is a compromise and not a solution because it still does not solve the issue I mentioned regarding creating constraints on the downloader API when the constraint does not even remotely concern the implementor.
The class containing the downloader plugin is declared using a meta-data tag.
Can multiple downloaders be provided, each via one uses-feature tag?
Can they launch intents if they want to without requesting?
Broadcasts and services can be sent/bound with no problems using the provided Context. Starting an activity without the API is not useful because the plugin does not get to handle the result.
Is this something we want to provide as an API at all?
Yes, because custom UIs are useful and it serves as a base for implementing webView { }.
I think a better way would be an extension function Intent().requestLaunch() to imply you are requesting something. Just launch doesn't imply you are requesting, which is the important part of this API.
The extension function would set the internal-only lambda similar to how launch { } would. On a related note, is "launch" terminology we invented or something Android uses for starting intents? I would use whatever Android uses, for example, "requestStart" if start is used.
I have no problem with changing the usage/signature. The Android API function for launching activities using an intent is called startActivity.
It should not be Pair<InputStream, Long> but Pair<InputStream, Long?> (nullable). An InputStream is the only correct choice here.
Give me the name of an HTTP client library that returns an InputStream and the size of the file being downloaded. If no such library exists, everyone will be forced to implement InputStream manually or resort to workarounds such as saving to a temporary file (which causes the indicators in the UI to not accurately reflect the actual download progress). Neither outcome is desirable.
cache is an API that, given a name, opens a closeable stream. Downloaders can use it to store something on disk for the lifespan of the downloader. They can access it given the same name at any time during the lifespan.
Java provides an API for creating temporary files, so providing a dedicated API for it seems unnecessary.
What if I want to return a stream? I don't think that's parcelable.
You don't return a stream, you store whatever you use to get the stream instead. This could be a file path, URL or an ID for a remote resource.
The constraint maybe makes sense outside of the downloader, somewhere deep in the internal code of ReVanced Manager, but that is a technicality that should not impair the implementation of a downloader.
The root cause of the constraint comes from the fact that the lifecycle of the Jetpack Compose composition is tied to an Activity, so users need to save and restore state to avoid losing it. This constraint is propagated until it reaches the downloaders. We don't have any control over this.
The API of a downloader should be predictable, minimal, and simple. Asking for Parcelable raises the question for the user, why the return type must be parcelable. The answer to that would be something the user should not even know about just to implement a downloader.
Parcelable may have been a very annoying interface to implement 10 years ago, but this is not the case anymore. Parcelable is very common in the Android ecosystem and its appearance here would not be considered out of place by anyone who knows what the interface is for. ReVanced Manager is made for Android so asking for basic Android development knowledge is not wrong here.
A compromise would be to require an inner member of the type Parcelable. So the API would allow any type to be used but with an upper constraint that requires the type to implement Parcelable. This parcelable would be the "parcelizable" representation of the type it is inside and its purpose would be for the Manager. Like I said, this is a compromise and not a solution because it still does not solve the issue I mentioned regarding creating constraints on the downloader API when the constraint does not even remotely concern the implementor.
I don't understand this. What do you mean by "inner member"?
Can multiple downloaders be provided, each via one uses-feature tag?
You would not need to specify the manifest attributes multiple times. Manager could simply load multiple downloaders from a single class instead. Regardless, I don't think supporting multiple downloaders in a single APK is necessary right now. It can still be implemented in the future.
using the provided Context
Is it necessary to pass the entire context down? Context for the purpose of launching intents can already be done internally via a public facing API.
Is this something we want to provide as an API at all?
Yes, because custom UIs are useful and it serves as a base for implementing webView { }.
I meant is giving entire context to the API something we want? Creating some constrained APIs which make calls to context APIs would simplify the API because the API would not need to expose the downloader implementation to the entire context APIs.
The Android API function for launching activities using an intent is called startActivity
How about requestStartActivity? We are passing an intent though and not an activity, so i don't know why Android calls it activity. I guess the intent carries the activity we want to start.
Give me the name of an HTTP client library that returns an InputStream and the size of the file being downloaded
Ktor client provides an API to open a stream. It can also read the headers, pause the stream and pass down the same open stream for reading the body. Since the header carries the size, it can provide the size for the file being downloaded. Another alternative is Java's native URL. It has a default handler for the HTTP protocol to open a stream.
everyone will be forced to implement InputStream manually or resort to workarounds such as saving to a temporary file (which causes the indicators in the UI to not accurately reflect the actual download progress). Neither outcome is desirable
Nobody is forced to implement input stream. File is an abstraction of input stream not the other way round. You would have to use a File when just a stream is necessary, but not the other way round. If just a stream is necessary, you do not need a File. How downloaders handle providing a stream is effectively their responsibility. It only makes sense to ask for an input stream. The file API is wrong as depicted with the String/Integer example in my previous comment.
Java provides an API for creating temporary files, so providing a dedicated API for it seems unnecessary.
The API breaches the domain. It provides access to an entire File API as explained in my previous comment. I have followed with a proposal for this issue.
You don't return a stream, you store whatever you use to get the stream instead. This could be a file path, URL or an ID for a remote resource.
Well, what I return is not really your concern. I return it for my purpose.
The root cause of the constraint comes from the fact that the lifecycle of the Jetpack Compose composition is tied to an Activity, so users need to save and restore state to avoid losing it. This constraint is propagated until it reaches the downloaders. We don't have any control over this.
Thats detailed intrinsics and technicality which now burdes the implementor of the downloader. Like explained earlier, this is a black-box constraint for them and is anything but comprehensible, unless they themselves delve deep in this technicality. Can the activity not be reused or something shared?
Parcelable is very common in the Android ecosystem and its appearance here would not be considered out of place
Can you make Stream parcelable? Or a lambda?
I don't understand this. What do you mean by "inner member"?
abstract class Data<T: Parcelable> {
abstract val parcelableData: T
}
parcelableData is a member of Data, which is what I meant.
You would not need to specify the manifest attributes multiple times. Manager could simply load multiple downloaders from a single class instead
Downloaders are instance referenced by properties. I could declare them in multiple files so Kotlin would generate multiple classes. On a related note, since the outer class is generated, how would the user reference it in the manifest? They could guess the generated name, but that's kinda unintuitive. Why not load all public accessible properties of type downloader? The user wouldn't need to declare them in manifest and manager could load them from any class.
using the provided Context
Is it necessary to pass the entire context down? Context for the purpose of launching intents can already be done internally via a public facing API.
Is this something we want to provide as an API at all?
Yes, because custom UIs are useful and it serves as a base for implementing webView { }.
I meant is giving entire context to the API something we want? Creating some constrained APIs which make calls to context APIs would simplify the API because the API would not need to expose the downloader implementation to the entire context APIs.
Binding to services or retrieving localized string resources for toast/error messages are examples of valid use-cases for Context. I am sure there are more. Third-party libraries used by plugins could also require a Context.
The Android API function for launching activities using an intent is called startActivity
How about
requestStartActivity? We are passing an intent though and not an activity, so i don't know why Android calls it activity. I guess the intent carries the activity we want to start.
Maybe startActivityRequest or requestActivity sound better?
The Android function is called startActivity because it starts an activity using the provided Intent (references which activity to start and the "arguments" for that activity).
Give me the name of an HTTP client library that returns an InputStream and the size of the file being downloaded
Ktor client provides an API to open a stream. It can also read the headers, pause the stream and pass down the same open stream for reading the body. Since the header carries the size, it can provide the size for the file being downloaded. Another alternative is Java's native URL. It has a default handler for the HTTP protocol to open a stream.
In the case of Ktor, you cannot return a stream if you are using the prepareX and execute API. The other functions offered by Ktor store the response in memory so any streams created using them will not reflect the actual download. I am not familiar with the Java API. How do you set request headers and read response headers?
everyone will be forced to implement InputStream manually or resort to workarounds such as saving to a temporary file (which causes the indicators in the UI to not accurately reflect the actual download progress). Neither outcome is desirable
Nobody is forced to implement input stream. File is an abstraction of input stream not the other way round. You would have to use a File when just a stream is necessary, but not the other way round. If just a stream is necessary, you do not need a File. How downloaders handle providing a stream is effectively their responsibility. It only makes sense to ask for an input stream. The file API is wrong as depicted with the String/Integer example in my previous comment.
The signature would make sense for an extension function, but it does not seem flexible enough for real world use-cases. Look at the code for the play store plugin. It downloads splits, merges them and then writes the result to the output. The plugin would not be able to accurately report download progress since the UI indicators are tied to the InputStream which cannot exist until all the splits have been downloaded and merged. A more flexible API is required to support this. The play store plugin can and does correctly report download progress and must still be able to do so even after API changes. The new API does not necessarily have to use files, it can use OutputStream or something, but it needs to separate download progress tracking from the process of outputting the APK. The Pair<InputStream, Long?> API can still be provided using extension functions. We could even create an extension function where you just return a URL.
Java provides an API for creating temporary files, so providing a dedicated API for it seems unnecessary.
The API breaches the domain. It provides access to an entire File API as explained in my previous comment. I have followed with a proposal for this issue.
We don't get to decide which standard library APIs can and can't be used. That is up to plugin developers since they are the ones writing that code. We do not need to provide an API for caching because plugins can implement caching themselves through the Java API or a third-party library.
You don't return a stream, you store whatever you use to get the stream instead. This could be a file path, URL or an ID for a remote resource.
Well, what I return is not really your concern. I return it for my purpose.
Manager does not care about the implementation of your plugin. Manager only sees App. You can use the class as-is or extend it to store more data if needed.
The root cause of the constraint comes from the fact that the lifecycle of the Jetpack Compose composition is tied to an Activity, so users need to save and restore state to avoid losing it. This constraint is propagated until it reaches the downloaders. We don't have any control over this.
Thats detailed intrinsics and technicality which now burdes the implementor of the downloader. Like explained earlier, this is a black-box constraint for them and is anything but comprehensible, unless they themselves delve deep in this technicality.
Activity recreation and system-initiated process death are well documented and every Android app developer has to account for them. It is impossible to write a usable Android app if you are constantly losing your state.
Can the activity not be reused or something shared?
State stored inside ViewModels or global variables are not affected by activity recreation, but any state stored inside them will still be lost on system-initiated process death. Apps handle process death by reloading state from a database, the filesystem, etc (Manager does this with patch bundles for example) or by saving and restoring it with the parcel system. We cannot prevent the system from killing the app process.
Parcelable is very common in the Android ecosystem and its appearance here would not be considered out of place
Can you make Stream parcelable? Or a lambda?
Streams are not parcelable. Lambdas are if they implement java.io.Serializable through the @JvmSerializableLambda annotation or you use a fun interface with Serializable as a superinterface. You do not need to return those types from get/similar if you implement your plugin sensibly by storing URLs, files, etc instead. Regardless, your reply does not have anything to do with my original statement that using Parcelable here is not out of place.
The Parcelable requirement isn't there to force developers to write their plugins in a specific way, it is only present for technical reasons.
I don't understand this. What do you mean by "inner member"?
abstract class Data<T: Parcelable> { abstract val parcelableData: T }
parcelableDatais a member of Data, which is what I meant.
How does having parcelableData help here? You would not be able to restore the Data instance in case of process death since Data is not Parcelable.
You would not need to specify the manifest attributes multiple times. Manager could simply load multiple downloaders from a single class instead
Downloaders are instance referenced by properties. I could declare them in multiple files so Kotlin would generate multiple classes. On a related note, since the outer class is generated, how would the user reference it in the manifest? They could guess the generated name, but that's kinda unintuitive.
Generated class names are very easy to predict and can be fully controlled through annotations such as @JvmName and @JvmMultifileClass if desired. This is no different from referencing a main function. The plugin template also shows you how to predict the class name, so developers don't need to figure that out themselves.
Why not load all public accessible properties of type downloader? The user wouldn't need to declare them in manifest and manager could load them from any class.
Classloaders do not tell you which classes exist.
Talking about supporting multiple plugins inside a single app seems unnecessary since it is not something we are going to implement on release.