search

Exodus-Privacy / exodus-android-app

εxodus Android application
GNU General Public License v3.0
663 stars 56 forks source link

Lock device during refresh re-launch another refresh #464

Open Jean-BaptisteC opened 4 months ago

Jean-BaptisteC commented 4 months ago

During the first time, when the app refreshes data, if the device is locked and users unlock the device. The coroutines are re-launched. We need to find a way to not re-launch coroutines when the device is unlocked

Iamlooker commented 4 months ago

Hi, if this issue is unassigned, I would like to work on this.

I looked into the code for about 30 mins and I think this might be one of the root causes of the issue. But I am not sure, I think we can remove the updateReport from these two functions: 1, 2

The mutability of the the IS_RUNNING function can be causing the issue or it could be the LifecycleService I can do a thorough research on it once I get a green flag 👀

I also went through some other parts of the codebase and found a huge dependency on MutableList and other mutable variables (which are not using the full potential of the coroutines), as the project is leaning towards coroutines this could lead to race situations. Also the cancellation is not handled appropriately

I think the performance can be improved significantly, but thats for another issue to handle. But just for context one thing which can be improved is, the notification shows 60/130 fetched, but none are reflected to database immediately.

Jean-BaptisteC commented 4 months ago

Okay, do you think setupSwipeRefreshLayout method is called when we unlock the device? Don't ask to work on issue, open PR if you want :) It can be cool if you improve performance, some users complain refresh takes too much time

Iamlooker commented 4 months ago

Yes, I think it is been called when the fragment is resumed, but I need to test it personally

Iamlooker commented 3 months ago

In my recent tests I am unable to reproduce this exact issue in last few days. But I can help with improving performance in the app. I will work on this and create a PR asap

Iamlooker commented 3 months ago

I have requested API key via mail for testing. I have done some drastic changes and will push them once I am done testing :)

Iamlooker commented 3 months ago

Most changes are done, some testing is still needed

Jean-BaptisteC commented 3 months ago

FIY, api_key is integrated in build debug workflow

Iamlooker commented 3 months ago

I willtest and get be soon

codeurimpulsif commented 3 months ago

@Iamlooker Hey, do you still need an API key?

Iamlooker commented 3 months ago

@Iamlooker Hey, do you still need an API key?

Sorry I don't think I will be able to test for few hours. I will revert back once I am back to workstation 😃

Iamlooker commented 3 months ago

This is the error I am getting on my debug build

21:30:47.054 ExodusAPIRepository      D  Attempting download of app details on com.google.android.gm.
21:30:47.287 ExodusAPIRepository      W  Failed to get app details, response code: 401. Returning emptyList.

I have not made any changes to ExodusAPIRepository yet

Jean-BaptisteC commented 3 months ago

Yes, I have same behaviour, I don't know where is the problem, maybe the API (max limit requests API?) I suggest you to retry later

Jean-BaptisteC commented 3 months ago

I continue to have refresh starting when I unlock device with your changes :( FIY I use Pixel 6 with Android 14

Jean-BaptisteC commented 3 months ago

I have find a way to reproduce: