Log search functionality and log browser refactoring

[ezaquarii]

• added log search in logs browser
• added logs browser view model test
• added universal async filtering utility
• refactored async runner and added manual runner
• migrated logs browser to Android DataBinding and Kotlin
• disabled imports ordering Ktlint rule as IDE does not
  support ktlint ordering
• added some missing tests around logger

Closes #4311

Signed-off-by: Chris Narkiewicz hello@ezaquarii.com

[ezaquarii]

@tobiasKaminsky @AndyScherzinger This is a follow-up on #4275 with discussed UI improvements.

[AndyScherzinger]

@ezaquarii I haven't had a chance to test it yet, first review looks good code-wise. Did you take care of themeing (search icon / search view)? If not @tobiasKaminsky might be able to help :)

[codecov]

Codecov Report

Merging #4309 into master will increase coverage by 0.19%.
The diff coverage is 62.74%.

@@             Coverage Diff              @@
##             master    #4309      +/-   ##
============================================
+ Coverage     16.63%   16.83%   +0.19%     
  Complexity        1        1              
============================================
  Files           351      354       +3     
  Lines         31408    31498      +90     
  Branches       4446     4471      +25     
============================================
+ Hits           5225     5302      +77     
  Misses        25308    25308              
- Partials        875      888      +13

Impacted Files	Coverage Δ	Complexity Δ
...java/com/nextcloud/client/di/ComponentsModule.java	0% <ø> (ø)	0 <0> (ø)	⬇️
...owncloud/android/ui/activity/SettingsActivity.java	38.44% <ø> (ø)	0 <0> (ø)	⬇️
...c/main/java/com/nextcloud/client/di/AppModule.java	72.72% <0%> (ø)	0 <0> (ø)	⬇️
...in/java/com/owncloud/android/utils/ThemeUtils.java	51.89% <0%> (-0.67%)	0 <0> (ø)
...ava/com/nextcloud/client/logger/ui/LogsActivity.kt	0% <0%> (ø)	0 <0> (?)
...java/com/nextcloud/client/logger/ui/LogsAdapter.kt	0% <0%> (ø)	0 <0> (ø)	⬇️
...java/com/nextcloud/client/logger/FileLogHandler.kt	92.45% <100%> (+0.45%)	0 <0> (ø)	⬇️
src/main/java/com/nextcloud/client/core/Task.kt	100% <100%> (ø)	0 <0> (?)
...com/nextcloud/client/core/ThreadPoolAsyncRunner.kt	100% <100%> (ø)	0 <0> (?)
...va/com/nextcloud/client/logger/ui/LogsViewModel.kt	60.78% <67.44%> (+60.78%)	0 <0> (ø)	⬇️
... and 10 more

[codecov]

Codecov Report

Merging #4309 into master will increase coverage by 0.19%.
The diff coverage is 62.33%.

@@             Coverage Diff             @@
##             master   #4309      +/-   ##
===========================================
+ Coverage      16.6%   16.8%   +0.19%     
  Complexity        1       1              
===========================================
  Files           351     354       +3     
  Lines         31410   31499      +89     
  Branches       4447    4472      +25     
===========================================
+ Hits           5215    5292      +77     
+ Misses        25317   25316       -1     
- Partials        878     891      +13

Impacted Files	Coverage Δ	Complexity Δ
...java/com/nextcloud/client/di/ComponentsModule.java	0% <ø> (ø)	0 <0> (ø)	⬇️
...owncloud/android/ui/activity/SettingsActivity.java	38.44% <ø> (ø)	0 <0> (ø)	⬇️
...c/main/java/com/nextcloud/client/di/AppModule.java	72.72% <0%> (ø)	0 <0> (ø)	⬇️
...ava/com/nextcloud/client/logger/ui/LogsActivity.kt	0% <0%> (ø)	0 <0> (?)
...in/java/com/owncloud/android/utils/ThemeUtils.java	51.89% <0%> (-0.67%)	0 <0> (ø)
...java/com/nextcloud/client/logger/ui/LogsAdapter.kt	0% <0%> (ø)	0 <0> (ø)	⬇️
src/main/java/com/nextcloud/client/core/Task.kt	100% <100%> (ø)	0 <0> (?)
...com/nextcloud/client/core/ThreadPoolAsyncRunner.kt	100% <100%> (ø)	0 <0> (?)
...java/com/nextcloud/client/logger/FileLogHandler.kt	92.45% <100%> (+0.45%)	0 <0> (ø)	⬇️
...va/com/nextcloud/client/logger/ui/LogsViewModel.kt	60.78% <67.44%> (+60.78%)	0 <0> (ø)	⬇️
... and 17 more

[nextcloud-android-bot]

IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/10354

[ezaquarii]

@AndyScherzinger @tobiasKaminsky I tried to copy the solution from ExtendedListFragment.onCreateOptionsMenu but the search view won't budge - it's still dimmed. I'm not sure how the theming is done there, but it looks like it depends on some hidden state set elsewhere.

[nextcloud-android-bot]

IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/10365

[AndyScherzinger]

@tobiasKaminsky can you provide @ezaquarii with the infos for search bar theming (which you already implemented elsewhere in the app)? :)

[tobiasKaminsky]

Regarding translation strings:
Please do not rename keys that are already in use, otherwise they will conflict with older version (we translate master and 1-2 older versions within the same transifex repository)
Just create a new string, even when it is with the same content. It causes overhead to the translation team, but this way we are bulletproof.

As transifex is using only values/strings.xml as source, we do not need to change the translated files. In best case they will be overwritten by next transifex update, and in worst case they result in a conflict.
Modifying only values/strings.xml makes review also a bit easier.

After that I am happy to review it (currently my browser refuses a bit to show that many changes…)

I will add both to our contributing.md

[tobiasKaminsky]

@AndyScherzinger @tobiasKaminsky I tried to copy the solution from ExtendedListFragment.onCreateOptionsMenu but the search view won't budge - it's still dimmed. I'm not sure how the theming is done there, but it looks like it depends on some hidden state set elsewhere.

@tobiasKaminsky can you provide @ezaquarii with the infos for search bar theming (which you already implement4ed elsewhere in the app)? :)

Sorry, I do not get the term "budge". What is wrong?
At a quick test, only three dots are tinted wrong on a light theme?
(not sure if this is even a problem with this PR, I doubt so…)

[AndyScherzinger]

@tobiasKaminsky - could you test if it works with (very) light Background and dark text/icons? Else it seems fine (except for the 3-dot icon)

[tobiasKaminsky]

I reverted translation files and rebased.
I'll review & test it today and then let's merge it.

After that I'll have a look again at the tinting problematic, as I think that the ThemeUtils.themeSearchView() is not working and thus it might be changed also in other places.

[tobiasKaminsky]

Only minor changes.

[nextcloud-android-bot]

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/10382.apk



To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

[nextcloud-android-bot]

Codacy

276

Lint

Type	master	PR
Warnings	59	58
Errors	0	0

SpotBugs (new)

Warning Type	Number
Bad practice Warnings	24
Correctness Warnings	69
Internationalization Warnings	12
Malicious code vulnerability Warnings	4
Multithreaded correctness Warnings	9
Performance Warnings	111
Security Warnings	46
Dodgy code Warnings	135
Total	410

SpotBugs (master)

Warning Type	Number
Bad practice Warnings	24
Correctness Warnings	69
Internationalization Warnings	12
Malicious code vulnerability Warnings	4
Multithreaded correctness Warnings	9
Performance Warnings	111
Security Warnings	46
Dodgy code Warnings	135
Total	410

[AndyScherzinger]

Ready, steady, merge 🌟