[theimpulson]

What is it?

• Bugfix (user facing)
• Feature (user facing)
• Codebase improvement (dev facing)
• Meta improvement to the project (dev facing)

Description of the changes in your PR

• Fixes OOM error when importing a large number of subscriptions as seen here
• Fixes error when no video is found on channel as seen here
• Fixes error due to service not going foreground in allowed time of 10s
• Avoids crashing whole import if a channel is not found (banned for example noticed in the OOM import)

Fixes the following issue(s)

• Fixes [YouTube] Subscriptions import fails under certain scenarios #13442

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

• I read the contribution guidelines.
• The proposed changes follow the AI policy.
• I tested the changes using an emulator or a physical device.

[theimpulson]

This should fix most of the issues raised by @Stypox in #13347 (comment)

[theimpulson]

I am not super happy with the notification being shown before channels are inserted into database but I assume there shouldn't a failure to insert the subscriptions into database in any case.

[Idadelveloper]

Not sure if i missed it but is it deliberate that the user is not notified once import begins? Tried importing a csv and for a second i thought i did not work or my phone froze especially as there were so many items in the file. Will be nice if once the user uploads the file for youtube/soundcloud it leaves that import screen and return back to the subscriptions tab while the import happens. Remaining in the import screen plus the fact that there's no notification to tell the user the import is in progress does not do justice for the UX... or let me know what you think.

[theimpulson]

Not sure if i missed it but is it deliberate that the user is not notified once import begins? Tried importing a csv and for a second i thought i did not work or my phone froze especially as there were so many items in the file. Will be nice if once the user uploads the file for youtube/soundcloud it leaves that import screen and return back to the subscriptions tab while the import happens. Remaining in the import screen plus the fact that there's no notification to tell the user the import is in progress does not do justice for the UX... or let me know what you think.

The UI part is indeed handled poorly overall in general. For example, we have a worker trying to update UI by toast, which in my opinion should be done by a notification instead. I will do another PR for changing UI behavior and would like this one to be focused on fixing bugs.

[Isira-Seneviratne]

This looks good to me.

[Stypox]

Thank you! I tested the APK from CI using the same data as in #13347 (comment).

I still got the following after importing a .csv file as a YouTube export:

21:13:35.512 26202-26317 SubscriptionImport      org...ous.subscriptions  E  Error while loading subscriptions from path
                                                                             org.schabi.newpipe.extractor.subscription.SubscriptionExtractor$InvalidSourceException: Not a valid source (Unsupported content type: application/octet-stream)
                                                                             	at org.schabi.newpipe.extractor.services.youtube.extractors.YoutubeSubscriptionExtractor.fromInputStream(r8-map-id-1f32333c5f9fccc65df0ec96b41e3917e6af81f7281dcd0c469e8838186c1f83:103)
                                                                             	at org.schabi.newpipe.local.subscription.workers.SubscriptionImportWorker$loadSubscriptionsFromInput$2.invokeSuspend(r8-map-id-1f32333c5f9fccc65df0ec96b41e3917e6af81f7281dcd0c469e8838186c1f83:156)
                                                                             	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(r8-map-id-1f32333c5f9fccc65df0ec96b41e3917e6af81f7281dcd0c469e8838186c1f83:8)
                                                                             	at kotlinx.coroutines.DispatchedTask.run(r8-map-id-1f32333c5f9fccc65df0ec96b41e3917e6af81f7281dcd0c469e8838186c1f83:114)
                                                                             	at androidx.core.app.ActivityRecreator$1.run(r8-map-id-1f32333c5f9fccc65df0ec96b41e3917e6af81f7281dcd0c469e8838186c1f83:13)
                                                                             	at kotlinx.coroutines.scheduling.TaskImpl.run(r8-map-id-1f32333c5f9fccc65df0ec96b41e3917e6af81f7281dcd0c469e8838186c1f83:3)
                                                                             	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(r8-map-id-1f32333c5f9fccc65df0ec96b41e3917e6af81f7281dcd0c469e8838186c1f83:88)

The same happens also with importing a .zip takeout file, although I did not really have a takeout file at my fingertips to test, so what I did was just zip the csv (which might not be representative). I guess you could switch back to StoredFileHelper to fix these issues as mentioned in #13347 (comment).

Importing a huge "previously exported" JSON works though (which is one of the big things that wasn't working before), so the memory issues seems fixed indeed.

Do you think it would be possible to write some Android tests to ensure we don't introduce regressions again in the future?

[theimpulson]

Zip part should be fixed now thanks to @djchittoor's commit. I will take a look at tests this week.

[sanket143]

It imports subscriptions, but not the channel groups. Is this expected?

[theimpulson]

It imports subscriptions, but not the channel groups. Is this expected?

Was that working before?

[sanket143]

I don't know, but that was my expectation while importing.
This was the first time I used import, because of circumstances. Import failed in the released version, but I was able to import using this PR.

Added commented just in case it is a miss, because I was expecting it to import groups too but it's fine if it was there from the beginning.

Thanks.

[Stypox]

Thank you! I tested again with the previously mentioned .csv, .json and .zip of 342 subscriptions, and it worked well even under averse network conditions. I did not test whether everything still works well with non-SAF, so someone could test that.