Rewrite import and export subscriptions functionality using coroutines #11759
Conversation
ShareASmile
added
dependency
|
| try { | ||
| @OptIn(ExperimentalSerializationApi::class) | ||
| return json.decodeFromStream<SubscriptionData>(`in`).subscriptions | ||
| } catch (e: Throwable) { | ||
| throw InvalidSourceException("Couldn't parse json", e) | ||
| } | ||
| } |
There was a problem hiding this comment.
From a cursory look, this seems to be a lot stricter than the previous json parser.
e.g. the serviceId would default to 0, and items in the list would simply be skipped if they don’t conform to the schema.
Do we want to change that behavior?
| if (mode == 0 && value == null && serviceId == 0) { | ||
| throw new IllegalStateException("Input data not provided"); | ||
| } |
There was a problem hiding this comment.
This seems to crash when rotating the screen while the confirmation dialog is visible
There was a problem hiding this comment.
At that point the module is not yet restored, I don’t know what this is supposed to do?
There was a problem hiding this comment.
Should be fixed in the commit I added, I left out the check since it will always use the @Parcelize decoders from the SubscriptionImportInput subclasses.
Profpatsch
force-pushed
the
Import-export-worker
branch
from
e99c0bc to
dbd11a6
Compare
|
|
I rebased on current Unfortunately, there is no |
| val subscriptions = withContext(Dispatchers.IO) { | ||
| when (input) { | ||
| is SubscriptionImportInput.ChannelUrlMode -> | ||
| NewPipe.getService(input.serviceId).subscriptionExtractor | ||
| .fromChannelUrl(input.url) | ||
| .map { SubscriptionItem(it.serviceId, it.url, it.name) } | ||
|
|
||
| is SubscriptionImportInput.InputStreamMode -> | ||
| applicationContext.contentResolver.openInputStream(input.url.toUri())?.use { | ||
| val contentType = | ||
| MimeTypeMap.getFileExtensionFromUrl(input.url).ifEmpty { DEFAULT_MIME } | ||
| NewPipe.getService(input.serviceId).subscriptionExtractor | ||
| .fromInputStream(it, contentType) | ||
| .map { SubscriptionItem(it.serviceId, it.url, it.name) } | ||
| } | ||
|
|
||
| is SubscriptionImportInput.PreviousExportMode -> | ||
| applicationContext.contentResolver.openInputStream(input.url.toUri())?.use { | ||
| ImportExportJsonHelper.readFrom(it) | ||
| } |
There was a problem hiding this comment.
This is missing a bunch of error handling, for example:
- File that was selected might not exist
- File might not be able to be parsed as json, or have the wrong contents
- Channel might not exist
- ServiceId might not exist
All of these should probably be shown to the user, right now the worker crashes silently in the background and nothing happens.
|
Once the missing error handling is added on import & the question about how leniently to parse the json is resolved, I’d say LGTM! |
What is it?
Description of the changes in your PR
CoroutineWorkerfor better performance and readability of the code.Before/After Screenshots/Screen Record
Export
Screen_recording_20241129_071419.webm
Android 6.0
Screen_recording_20241129_064527.webm
Android 6.0
Screen_recording_20241129_064829.mp4
Android 15
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.
Due diligence