SES1251 - App crash on non alphanumeric RE-FIX #1445
Conversation
| import java.io.IOException | ||
| import java.util.Locale | ||
|
|
There was a problem hiding this comment.
revert file?
Do you have a formatter that's adding linebreaks between import groups.
There was a problem hiding this comment.
Nah, I just tend to prefer lines between import groups, but I can revert :) / Done
| @@ -41,14 +48,19 @@ class GlobalSearchViewModel @Inject constructor(private val searchRepository: Se | |||
| _queryText | |||
| .buffer(onBufferOverflow = BufferOverflow.DROP_OLDEST) | |||
| .mapLatest { query -> | |||
| // Minimum search term is 2 characters | |||
There was a problem hiding this comment.
iOS lets us search 1 character.
It's not necessarily a good search, but you can give yourself a 1-char name, so you should be able to search that... and ideally a match of an entire word should be elevated.
There was a problem hiding this comment.
Yeah, fair enough - have allowed 1 char searches and it does prioritise contacts & groups over individual messages.
| val settableFuture = SettableFuture<SearchResult>() | ||
|
|
||
| // If we already have a search running then stop it | ||
| if (!settableFuture.isDone) { settableFuture.cancel(true) } |
There was a problem hiding this comment.
you don't need to check if the Future done, it's checked in cancel, the comment is not exactly true, as canceling this future would essentially stop listening for results, but won't stop any work being done.
Is it even necessary? mapLatest will prevent stale results clobbering newer ones.
There was a problem hiding this comment.
Cool, I didn't know mapLatest did that - have reverted settableFuture back to being a val defined at point of use
| if (TextUtils.isEmpty(query)) { | ||
| // If the sanitized search query is empty or less than 2 chars then abort the search | ||
| String cleanQuery = sanitizeQuery(query).trim(); | ||
| if (cleanQuery.isEmpty() || cleanQuery.length() < 2) { |
There was a problem hiding this comment.
a bit tautological, length < 2 would cover both cases, but maybe the isEmpty is preferred?
There was a problem hiding this comment.
Adjusted so only using sanitize -> trim -> isEmpty
Contributor checklist
Fixes #1234syntaxDescription
Searching messages and entering a non-alphanumeric character as the first character caused an crash & restart. This was due to the search being performed after each character entered, and any illegal character entered as the 1st character is replaced with a space - so in effect we are searching on the string
" "which was causing the error.Guards have been put in place to prevent search queries on strings of
" "which eliminates the error.Tested by entering a variety of non-alphanumeric characters as first search letter of message search both in regular user-to-user comms & in a group- all fine now.