Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SearchResultEntry: detect file results not from Files provider #762

Merged
merged 1 commit into from
Nov 25, 2021

Conversation

AlvaroBrey
Copy link
Member

@AlvaroBrey AlvaroBrey commented Nov 24, 2021

Some providers like FullTextSearch give file results without the fileId attribute.
This refactor allows for that, otherwise they would crash the UnifiedSearch UI.

Fixes nextcloud/android#9345

Some providers like FullTextSearch give file results without the fileId attribute.
This refactor allows for that, otherwise they would crash the UnifiedSearch UI.

Signed-off-by: Álvaro Brey Vilas <alvaro.brey@nextcloud.com>
sanitizer.parseUrl(resourceUrl)
URL(resourceUrl).query?.let { sanitizer.parseQuery(it) }
Copy link
Member Author

@AlvaroBrey AlvaroBrey Nov 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed because we don't need to do this. parseUrl already calls parseQuery if present. Additionally this crashes when trying to parse paths that are not valid URLs (like files paths without the server).

@AlvaroBrey
Copy link
Member Author

/backport to stable-2.8

@nextcloud-android-bot
Copy link
Collaborator

SpotBugs (new)

Warning Type Number
Bad practice Warnings 14
Correctness Warnings 38
Internationalization Warnings 6
Malicious code vulnerability Warnings 52
Multithreaded correctness Warnings 3
Performance Warnings 14
Security Warnings 1
Dodgy code Warnings 44
Total 172

SpotBugs (master)

Warning Type Number
Bad practice Warnings 14
Correctness Warnings 38
Internationalization Warnings 6
Malicious code vulnerability Warnings 52
Multithreaded correctness Warnings 3
Performance Warnings 14
Security Warnings 1
Dodgy code Warnings 44
Total 172

@codecov
Copy link

codecov bot commented Nov 24, 2021

Codecov Report

Merging #762 (9d8f309) into master (788d3eb) will increase coverage by 0.02%.
The diff coverage is 90.00%.

@@            Coverage Diff             @@
##           master     #762      +/-   ##
==========================================
+ Coverage   46.33%   46.35%   +0.02%     
==========================================
  Files         164      164              
  Lines        6354     6355       +1     
  Branches      834      833       -1     
==========================================
+ Hits         2944     2946       +2     
  Misses       2971     2971              
+ Partials      439      438       -1     
Impacted Files Coverage Δ
...m/owncloud/android/lib/common/SearchResultEntry.kt 95.65% <90.00%> (+4.74%) ⬆️

@tobiasKaminsky
Copy link
Member

Some providers like FullTextSearch give file results without the fileId attribute.

If we need this, please create an issue in these repositories, so this will then work automatically at some point.
👏 for having this as a fallback 👏

@AlvaroBrey
Copy link
Member Author

If we need this, please create an issue in these repositories, so this will then work automatically at some point. clap for having this as a fallback clap

Done: nextcloud/files_fulltextsearch#143

@AlvaroBrey AlvaroBrey merged commit 81ca432 into master Nov 25, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix/fulltext-files-crash branch November 25, 2021 09:33
@AlvaroBrey AlvaroBrey added this to the NC Android lib 2.9.0 milestone Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash clicking on a full text search result
3 participants