search

kaiserbh / tachiyomi

Free and open source manga reader for Android.
https://tachiyomi.org/
Apache License 2.0
4 stars 3 forks source link

Added exceptions in Google Drive sync process so that it fails correctly and moved the redirect url to a constant #5

Closed undefiened closed 1 year ago

undefiened commented 1 year ago

I added an exception to the Google Drive sync process in case the user hasn't logged in to Google Drive.

Before my changes syncing via Google Drive didn't generate any errors if the user hadn't logged in. The bug is easy to reproduce:

  1. Make a clean installation of this branch
  2. Select Google Drive service for syncing
  3. Click "Sync Now" without signing in to the Google Drive

Then the sync happily reported that 0 errors were generated even though nothing actually was synchronized because Google Drive wasn't even signed in.

I also added exceptions in a couple of other places, since they are supposed to be treated as a failure anyway. As far as I understand, SyncDataJob reports that sync was successful if no exceptions were generated during it, so I think it probably makes sense to put exceptions everywhere. From a bigger point of view it would have probably made more sense to make SyncDataJob work the other way around: to assume failure by default, unless the syncing process reported success, but I am not sure how it can be done.

I also moved redirect url into a constant so that it is in one place.

undefiened commented 1 year ago

Thank you very much for fixing it and showing me how it should be done instead of simply fixing it! I am not very familiar with Kotlin, so I got scared when I attempted to use "const" and it said something about companion objects :) So I am very thankful for the learning opportunity.