This wouldn'tve actually found the problem in the previous commit,
due to the null happening before checking permissions while logging perms.
However, still seems like a nice test to have so that the method itself
handles nulls correctly.
<uses-permissions/> tags can have min and max SDK to take effect. This is
not supported currently, and it necessary especially with the privileged
installer so it can properly represent the permissions that an APK is
requesting.
For example:
<uses-permission
android:name="android.permission.MANAGE_ACCOUNTS"
android:maxSdkVersion="22" />
<uses-permission-sdk-23
android:name="android.permission.CAMERA" />
<uses-permission-sdk-23
android:name="android.permission.CALL_PHONE"
android:maxSdkVersion="23" />
android.content.pm.PackageInfo is the Android class for representing data
about an APK/package. Since Apk.permission is the same thing, we should
use the same name.
There are oddities with the way that Android has implemented the network
stack, as compared to OpenJDK or Oracle JDK. So running the tests on the
local JVM, i.e. Robolectric, will not provide good test coverage for real
world use cases.
To appease PMD, we now have a three rulesets in `config/pmd/*.xml`:
* `rules.xml`: The bulk of the rules, used by both main and test code.
* `rules-main.xml`: Rules specific to the andoid client code.
* `rules-test.xml`: Rules specific to test code.
The rationale is because checkstyle by default checks for "too many static
imports", which is a fair call. However in JUnit4 code, it is common to
import many `assert*` static methods.
The tests pass, but there is a lingering message that gets logged:
```
Jun 08, 2016 7:31:13 AM com.almworks.sqlite4java.Internal log
WARNING: [sqlite] [DETACH DATABASE temp_update_db]DB[1][C]: exception when clearing
com.almworks.sqlite4java.SQLiteException: [1] DB[1] reset [no such database: temp_update_db]
at com.almworks.sqlite4java.SQLiteConnection.throwResult(SQLiteConnection.java:1309)
at com.almworks.sqlite4java.SQLiteConnection.throwResult(SQLiteConnection.java:1282)
at com.almworks.sqlite4java.SQLiteConnection.cacheStatementHandle(SQLiteConnection.java:1211)
at com.almworks.sqlite4java.SQLiteConnection.access$900(SQLiteConnection.java:54)
at com.almworks.sqlite4java.SQLiteConnection$CachedController.dispose(SQLiteConnection.java:1606)
at com.almworks.sqlite4java.SQLiteStatement.dispose(SQLiteStatement.java:187)
at org.robolectric.shadows.ShadowSQLiteConnection$Connections$4.call(ShadowSQLiteConnection.java:421)
at org.robolectric.shadows.ShadowSQLiteConnection$Connections$6.call(ShadowSQLiteConnection.java:449)
at org.robolectric.shadows.ShadowSQLiteConnection$Connections$6.call(ShadowSQLiteConnection.java:443)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
```
The `temp_update_db` is the one used for repo updates, but I thought that it
correctly gets dropped/detached by the `TempAppProvider` when required. In fact,
given the nature of the error message (no such database: temp_update_db), that
hints at the fact that it is indeed dropped. I'm struggling to figure out what
causes this, but it should not be harmful to the running of the tests. If a test
actually fails, then it is picked up correctly by JUnit.
Many of the `Mock*` classes are there to deal with idiosyncrosies of
the Android SDK, including `final`/package local/`@Hide` annotations/etc.
They are no longer required with robolectric tests.
Get around silly `final` methods in `ContentResolver` with Mockito and `delegatesTo`.
The Robolectric library presumes that people always want to test content providers by
manually invoking the `query`/`update`/`delete` methods on the `ShadowContentResolver`.
While that is a great feature for testing, we have helper methods that require testing,
and these methods accept either a _real_ `ContentResolver` or `Context`. Robolectric
did some cool magic in terms of intercepting runtime calls to content resolvers and
forwarding them to the "shadow" verison, to deal with final/package private/etc methods.
However, as a side effect, the `ShadowContentProvider` _is not a `ContentProvider` as
far as the Java compiler is concerned.
By utilising Mockito + `delegatesTo` method, we are able to achieve what is required:
* An actual `ContentProvider` instance.
* It forwards calls to the `ShadowContentProvider` provided by Robolectric.
Robolectric provides testing support for Android via the JVM, including testing
of content providers. In order to get these tests to work, we need to avoid
the default behaviour of starting up FDroidApp.onCreate(). This method has a lot
of static state which fails if set multiple times. Instead of trying to ensure
we correctly zero out that state each test, it is preferable to instead never
bother with that in the first place. Expecially when that is not what is under
test (as is the case with content provider tests).
Since refactoring the installed app cache stuff, these methods are no longer
required for testing purposes. This is because the tests directly ask the
content provider to insert relevant apps, rather than testing the broadcast
receiving functionality.
The APK hash is useful for comparing whether something is exactly the same
file as something else. For example, to compare whether the installed APK
matches something that f-droid.org hosts. The "last update time" is a fast
way to check whether the information is current.
InstalledAppCacheUpdater was a custom Service-like thing with some
threading issues. InstalledAppProviderService is an IntentService that
relies on the built-in queue and threading of the IntentService to make
sure that things are processed nicely in the background and one at a time.
This changes the announcing so that each app added/changed/deleted triggers
a new annoucement. This keeps the UI more updated, and makes the Installed
tab show something as soon as possible, rather than waiting for the all of
the install apps to be processed. This becomes more important as more
stuff is added to InstalledAppProvider, like the hash of the APK.
This also strips down and simplifies the related BroadcastReceivers.
BroadcastReceivers work on the UI thread, so they should do as little work
as possible. PackageManagerReceiver just rebadges the incoming Intent and
sends it off to InstalledAppProviderService for processing.
Apparently, the CREATOR field is not (yet?) needed in the tests, since
they work without it. This gets us closer to making lint errors fail
the CI builds.
closes#580
A hacked fdroid server could "replay" old index.jar files known to have
apps with vulnerabilities in it. That provides a long window of time for
exploiting that vulnerability. By checking that the timestamp of an update
is never older than the current index, this attack is prevented.
The Event class is no longer needed once there is specific ProgressListener
instances for each type of progress update. The sourceUrl serves as the
unique ID, like with DownloaderService and InstallManagerService.
fixes#633https://gitlab.com/fdroid/fdroidclient/issues/633
DownloaderServiceTest is just a stub of a test that doesn't really test
anything yet. It is now causing a NullPointerException, so its a problem:
java.lang.NullPointerException
at org.fdroid.fdroid.net.DownloaderService.notifyDownloadComplete(DownloaderService.java:313)
at org.fdroid.fdroid.net.DownloaderService.handleIntent(DownloaderService.java:287)
at org.fdroid.fdroid.net.DownloaderService$ServiceHandler.handleMessage(DownloaderService.java:104)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loop(Looper.java:137)
at android.os.HandlerThread.run(HandlerThread.java:60)
This moves the cache file deletion to a dedicated IntentService that runs
at the lowest possible priority. The cache cleanup does not need to happen
with any kind of priority, so it shouldn't delay the app start or take any
resources away from foreground processes.
This also changes the logic around the "Cache packages" preference. The
downloader always saves APKs, then if "Cache packages" is disabled, those
APKs are deleted when they are older than an hour.
This also simplifies Utils.deleteFiles() since the endswith arg is no
longer needed.
This used to be the case, which is why only minimal changes were
required to bring it back. This also makes it take the same files that
checkstyle does, which is more consistent.
DownloaderService is based on IntentService to provide queued requests that
run in a background thread via the Handler and the HandlerThread. It began
as the IntentService code, but it could not be a subclass because the
downloading needs to be cancelable. IntentServices cannot be canceled and
they provide no visibility into their queue.
DownloaderService then announces relevant events via LocalBroadcastManager
and Intents with custom "action" Strings.
https://gitlab.com/fdroid/fdroidclient/issues/601#601
Android recently switched from JUnit 3 to 4 for its base testing classes.
It doesn't seem to support the old JUnit3 methods with gradle and AS. So
all the tests need to be ported to JUnit4 to work again.
#607https://gitlab.com/fdroid/fdroidclient/issues/607
This makes it a lot easier to setup all the testing stuff. Mostly,
I'm tired of fighting Android Studio's fragility, so I want to remove
as much non-standardness as possible in the hopes of improving that
situation.
closes#534https://gitlab.com/fdroid/fdroidclient/issues/534
