Like PMD, we also had to add a concession to allow static imports.
This time, it was achieved by moving the assertions to a more generally
named `Assert` class, and then allowing static imports from that.
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).
This makes testing of the function easier, as the method previously expected
a real file to exist on disk for which it could then hash. This instead allows
mock hash values to be inserted when under test.
Other than this, the semantics remain exactly the same as before, and the
expensive hashing is still done on a worker thread as part of the `IntentService`.
Translators:
Ab Arabic
Adrià García-Alzórriz Catalan
Adrià García-Alzórriz Spanish
ageru French
Ajeje Brazorf Sardinian
ezjerry liao Traditional Chinese
Francesco Giordano Italian
Frank Ludviksson Spanish
Helder Santana Portuguese (Brazil)
Kristoffer Grundström Swedish
Licaon Kter Romanian
Marian Hanzel Slovak
Massimiliano Caniparoli Italian
Mladen Pejaković Serbian
Mutante Citta Italian
naofum Japanese
Olexandr Nesterenko Ukrainian
Prasanna Venkadesh Tamil
Sérgio Marques Portuguese (Portugal)
Tobias Bannert German
Verdulo Esperanto
Verdulo Polish
Service crash fixes
3 relatively simple crash fixes. The two related to `WifiStateChangeService` have already be included in `stable-0.100`, 7385d320b42af960be63c9c179e1cbf186c1398a should be cherry-picked into `stable-0.100` after this is merged.
I already have 198ad843c1fabc8cf57ffe85c77230288cd6d7a4 ready in my stable-0.100 branch
See merge request !317
Historically the providers were responsible for notifying about inserts/deletes
for this table. However this is no longer the case with the new service responsible
for throttling the rate with which these notifications occur.
The `parseApp` method was previously accepting an `Intent`, which could
have been anything. Given it was only used once, this now pushed the
creation of that `Intent` into the `parseApp` method, and also reduced the
visibility of the method as it is only used once at time of writing.
This should not be a particularly expensive opperation,. Also, at time of
writing it is only used in a background thread, and only used once in that
thread (i.e. not in a loop or anything like that).
Now that we have RX as a dependency, it can be used as a nice concise way to
achieve certain tasks. Rate limiting is one thing it does well - via the
`debounce` mechanism:
http://reactivex.io/documentation/operators/debounce.html
The semantics of this code is the same as before, limiting content change notifications
to one per second.
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.
caught java.lang.StringIndexOutOfBoundsException:
at java.lang.String.substring(String.java:1651)
at java.lang.String.subSequence(String.java:2040)
at org.fdroid.fdroid.data.App.setFromPackageInfo(App.java:298)
at org.fdroid.fdroid.data.App.<init>(App.java:268)
at org.fdroid.fdroid.localrepo.CacheSwapAppsService.onHandleIntent(CacheSwapAppsService.java:78)
at android.app.IntentService$ServiceHandler.handleMessage(IntentService.java:59)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loop(Looper.java:130)
at android.os.HandlerThread.run(HandlerThread.java:60)
This adds a check of whether the database has the current APK in it, based
on PackageInfo's lastUpdateTime field. This avoids recalculating the hash
of the whole APK, which is quite time and resource intensive.
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.
InstallAppProviderService now processes install and delete events one at a
time, where InstalledAppCacheUpdater made a batch of changes which it ran
all at once. This means that InstallAppProviderService will send out a
flood of notifications when first initializing, since it will index every
single installed app and send a notification for each one. This makes the
GUI lock up. This commit puts a rate limit on those notifications if they
start coming fast. They are limited to one per second.
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.
The SwapService is the central container for all things swap. If anything
at all related to swap is active, then SwapService needs to be running.
That also means that stopping SwapService should stop all things related to
swapping, including any screens or notifications.
fixes#258https://gitlab.com/fdroid/fdroidclient/issues/258
Since it takes a chunk of time to generate and write the app index.jar when
swapping apps, this service starts running in the background immediately
when SwapService starts. It first indexes the installed apps that were not
cached, then caches apps based PACKAGE_ADDED broadcasts. It does not index
system apps, since there are many and they are rarely swapped.
If the install process is interrupted, then InstallManagerService is no
longer managing it. It will make the announcements and set the
notification, then forget about that APK.
InstallManagerService and DownloaderService both use the download URL as
the unique ID to represent a given APK install through the whole lifecycle
of the install and download process. This converts the installer stuff to
use the same semantics. A Uri instance is mostly used there because its
the most useful format, but ultimately, the String, Uri, and int all derive
from the exact same URL. This then removes the local APK URI from use in
the installer broadcasts.
While I normally think reusing terms from Android is the best thing to do,
"originating URI" drives me nuts because it is almost nonsense English.
"Originating" is a verb in the continuous form, meaning that it is an
action that is ongoing. A URI is a static thing, and in this case, a URI
that points to a file that is completely downloaded. I left the term in
place for DefaultInstaller because it wraps PackageManager, which is where
that term originates.
This handles "Use strings instead of Uris in InstallManagerService for
urlString" as listed in #680
IntentServices can get a null Intent if they are restarted after being
killed. So this should be properly handled.
"[The intent] may be null if the service is being restarted after its
process has gone away, and it had previously returned anything except
START_STICKY_COMPATIBILITY."
https://developer.android.com/reference/android/app/IntentService.html#onStartCommand(android.content.Intent,%20int,%20int)
ANDROID_VERSION=5.1.1
APP_VERSION_NAME=0.99.2
BRAND=samsung
PHONE_MODEL=SM-G901F
java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String android.content.Intent.getStringExtra(java.lang.String)' on a null object reference
at org.fdroid.fdroid.UpdateService.onHandleIntent(UpdateService.java:342)
at android.app.IntentService$ServiceHandler.handleMessage(IntentService.java:65)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:135)
at android.os.HandlerThread.run(HandlerThread.java:61)
Some of these devices do shitty things.
htc_europe HTC EVO 3D X515m
java.lang.RuntimeException: An error occured while executing doInBackground()
at android.os.AsyncTask$3.done(AsyncTask.java:278)
at java.util.concurrent.FutureTask$Sync.innerSetException(FutureTask.java:273)
at java.util.concurrent.FutureTask.setException(FutureTask.java:124)
at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:307)
at java.util.concurrent.FutureTask.run(FutureTask.java:137)
at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:208)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1076)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:569)
at java.lang.Thread.run(Thread.java:864)
Caused by: java.lang.NullPointerException
at java.net.NetworkInterface.getNetworkInterfacesList(NetworkInterface.java:286)
at java.net.NetworkInterface.getNetworkInterfaces(NetworkInterface.java:262)
at org.fdroid.fdroid.net.WifiStateChangeService.setIpInfoFromNetworkInterface(WifiStateChangeService.java:202)
at org.fdroid.fdroid.net.WifiStateChangeService.access$300(WifiStateChangeService.java:37)
at org.fdroid.fdroid.net.WifiStateChangeService$WaitForWifiAsyncTask.doInBackground(WifiStateChangeService.java:99)
at org.fdroid.fdroid.net.WifiStateChangeService$WaitForWifiAsyncTask.doInBackground(WifiStateChangeService.java:71)
at android.os.AsyncTask$2.call(AsyncTask.java:264)
at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:305)
... 5 more
java.lang.NullPointerException
at java.net.NetworkInterface.getNetworkInterfacesList(NetworkInterface.java:286)
at java.net.NetworkInterface.getNetworkInterfaces(NetworkInterface.java:262)
at org.fdroid.fdroid.net.WifiStateChangeService.setIpInfoFromNetworkInterface(WifiStateChangeService.java:202)
at org.fdroid.fdroid.net.WifiStateChangeService.access$300(WifiStateChangeService.java:37)
at org.fdroid.fdroid.net.WifiStateChangeService$WaitForWifiAsyncTask.doInBackground(WifiStateChangeService.java:99)
at org.fdroid.fdroid.net.WifiStateChangeService$WaitForWifiAsyncTask.doInBackground(WifiStateChangeService.java:71)
at android.os.AsyncTask$2.call(AsyncTask.java:264)
at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:305)
at java.util.concurrent.FutureTask.run(FutureTask.java:137)
at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:208)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1076)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:569)
at java.lang.Thread.run(Thread.java:864)
java.lang.NullPointerException: Attempt to invoke virtual method
'android.os.Parcelable android.content.Intent.getParcelableExtra(java.lang.String)' on a null object reference
at org.fdroid.fdroid.net.WifiStateChangeService.onHandleIntent(WifiStateChangeService.java:56)
at android.app.IntentService$ServiceHandler.handleMessage(IntentService.java:65)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:135)
at android.os.HandlerThread.run(HandlerThread.java:61)
fixes#559
