fixes and cleanups related to ongoing DownloaderService work
This includes a fix for bug that @mvdan found in the processing of `Apk.maxSdkVersion`, as well as some cleanups related to the ongoing work in !248 . Indeed a couple of these commits were pulled out of that MR.
See merge request !253
Since Downloader's outputFile variable is final, it can safely be used
as a public property variable. This makes it simple to use in
subclasses. Making it a public final variable rather than a getter
also communicates that the value does not change since there is no
getter method that could potentially change it.
http://binkley.blogspot.com/2005/01/read-only-properties-in-java.html
Having 0 mean max makes the logic confusing when maxSdkValue is used in
variable. This sanitizes the data so that maxSdkValue is always just a
plain int value that can be used to test against. It does this by
defaulting to Byte.MAX_VALUE (127) if it is not explicitly set. At the rate
of 24 SDK numbers in 8 years, that gives us about 24 years before we have
to think about setting it to Short.MAX_VALUE.
This fixes an issue created by e021eb5ca7e8f05dbce7c1b87833722542138302
gitlab-ci: use android-17 emulator for `gradle connectedCheck`
The android-10 emulator does not report test failures so it is pretty useless at the moment. After lots and lots of trying, the most recent emulator that I could get running on gitlab-ci was 17, so let's hope that turns out to be more useful. I also had to reduce the RAM that was used, it seems that gitlab-ci does not let the docker images use much RAM. This might be able to be improved by creating an pre-setup AVD image in the docker image used by this.
As you can see from the history, I tried lots of things to see if it is was possible to get a more recent emulator running on gitlab-ci.
See merge request !241
The android-10 emulator does not report test failures so it is pretty
useless at the moment. After lots and lots of trying, the most recent
emulator that I could get running on gitlab-ci was 17, so let's hope that
turns out to be more useful. I also had to reduce the RAM that was used,
it seems that gitlab-ci does not let the docker images use much RAM.
This might be able to be improved by creating an pre-setup AVD image in the
docker image used by this.
I find that the logs dumped into the gitlab-ci screens are generally
unreadable, so here, only the errors are dumped into the build log, then
the rest are uploaded to clbin, a paste bin, where the whole text can be
viewed and downloaded in a clean, raw format.
Add non-emulator tests and simplify internal API
A lot of the purely internal API is using constructs which are not needed for internal APIs. The internal API can be viewed and changed by any contributor, so its better to not cover all possible future uses. Indeed to keep the codebase simple, it should be the opposite: the app's code should reflect what is actually happening now, not what might happen in the future.
This also adds tests that run on the JVM rather than the emulator.
These commits where originally in !248 but I'm submitting them separately since !248 is too big.
See merge request !250
Since the DownloaderService's events are all based on the complete download
URLs, and RepoUpdater is where the update URLs are built, this makes the
full download URL into a read-only property of RepoUpdater so it can be
used wherever there is an instance of RepoUpdater
This is also important because having the `final` property highlights
the lifecycle of that variable: it does not change during the entire
life of a RepoUpdater Instance.
Instead of duplicate APIs, standardize on a single API, and use that
everywhere via the Downloader.LOCAL_ACTION_PROGRESS event that is already
wired in.
This is needed so that downloads can be canceled from within an
IntentService. Since the Downloader classes do not have any Thread logic in
them, they shouldn't use Thread logic within them anyway.
This also removes the unused argument to AsyncDownloader.attemptCancel().
SharedPreferences.Editor.apply() for android 8
Turns out that `SharedPreferences.Editor.apply()` was not added until `android-9`, so this is a little trick to support `android-8` still after the changes in c3b47ecd5a380678dd2df3dc2549155429d28514
See merge request !249
This is not pretty, but its the best I could think of.
Fixes this lint error:
Call requires API level 9 (current min is 8): android.content.SharedPreferences.Editor#apply [NewApi]
'src' works because we're only doing java files under that directory.
But it would be slower than needed, and in the case of PMD it would also
use the test files which wasn't intended at all.
It seems like having it as a compile dependency already works for the
tests. Having it duplicated seems to sometimes trigger errors (e.g. a
user reported a duplicate zip entry due to the duplication) and might
also be problematic if we don't keep the two versions in sync.
Random fixes from DownloaderService hacking
Here are some random fixes that I did in the process of the DownloaderService refactoring. I don't think anything should be controversial. Thanks for your rapid code reviews recently @mvdan :)
See merge request !245
"Consider using apply() instead; commit writes its data to persistent
storage immediately, whereas apply will handle it in the background"
commit() is only useful if the code actually checks the return value.
* Use a % sign that String.format() recognizes, apparently there are more
than one % signs, in Chinese, its big: ％
* a string in lithuanian forgot the %s
More DownloaderService progress
As part of the incremental approach of moving downloading to an IntentService, here are a few more commits refactoring things into events instead of listerners/callbacks/etc.
ping @pserwylo
See merge request !240
An AsyncTask ties into the UI thread for things like onPostExecute(). If it
is run within an AsyncTask, then it freaks out because it can't tie into
the UI thread. We don't need it to do that here anyway, so just use a
plain Thread, and set the priority to background.
As part of the process of moving the APK downloading to an
IntentService, I'm removing and incrementally reorganizing the
existing events so that the code continues to be functional as it is
reorganized. We might want to include more detail in a download error
to expose to the user, but I think instead what will be more fruitful
is to hide details on errors where there is nothing the user can do
except retry. Then if there are errors where the user can do
something about it, then F-Droid should instead offer them the option
of doing that, and not just show an error message and walk away.
This is part of the move to standardizing all internal broadcasts to use
the Intent's Uri field as the standard place for a download URL, and then
using that in IntentFilters to do matching.
