userManager.currentUser was called too often. I was not able to prove that a bug is related to it but i think it may fix some hidden bugs.
CurrentUserProviderImpl is now used throughout the code to access the current user.
userManager.currentUser is only used from CurrentUserProviderImpl whenever the _currentUser was null (should only happen on app startup)
To avoid multiple initialization of CurrentUserProviderImpl it was changed to be a @Singleton
The handling should soon be replaced with coroutine flows. However for the v21.0.0 release it's still done with RxJava to avoid bugs.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
- Uncommented out fetchOpenConversations
- Added fetchUsers to conversationList
- Made the messages appear first
Signed-off-by: rapterjet2004 <juliuslinus1@gmail.com>
It could happen that when sending a message it was received on server but if the servers response is not received, the retry was triggered in the talk app.
This sometimes happened when internet connection was not the best.
Best would be that messages with the same referenceId would be refused on server side, but this won't be the case for now. So messages with the same referenceId are NOT refused as this would be too much overhead as there would be additional queries on server for every received message)
For now, the automatic retry logic is just removed so duplicated messages won't be created automatically.
However it's still possible to manually trigger the retry via button. In this case it is not guaranteed that there won't be duplicates.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
usersDao.getActiveUser() will return only one current users even if multiple exist for whatever reason.
By executing setUserAsActiveWithId(it.id) with the same user, it is made sure all other users current status is set to 0.
This change won't fix the root cause if multiple users are set to current, but it will make sure the state is fixed as soon as there is a query to get the active user.
In fact, this change might also make it harder to find the root cause because debugging may be more difficult! When searching for the root cause, always keep this in mind and maybe revert the change.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
To make sure getAnyUserAndSetAsActive() is invoked lazily and avoid any side effects, it's explicitly wrapped it in a lambda.
Maybe.defer ensures that getAnyUserAndSetAsActive() is not invoked until switchIfEmpty decides it’s needed.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Before, getAnyUserAndSetAsActive did only set a random first user to current=true without to check if there is any other current user.
So it was up to the calling function of getAnyUserAndSetAsActive to check this.
I did not identify a scenario where the getAnyUserAndSetAsActive could cause to set multiple users to current, but anyway the new implementation might fix some scenario that i could not reproduce.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
- First commit - Made options yellow
- Now possible to archive/unarchive conversations from dialog
- Now possible to archive and unarchive conversations in settings without leaving the screen
- Better UX
- Unread message bubble fix - no longer shows up when archive filter is set
Signed-off-by: rapterjet2004 <juliuslinus1@gmail.com>
when being offline and no messages are set -> hide shimmer
when messages are received -> hide shimmer and "offlineNoMessages"-info
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Because long polling may have to be informed that the scopep was cancelled, the isActive checks are added so it can't happen that messages are added when they shouldn't.
I could not reproduce the scenario, anyway the checks should make sense in my opinion.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
This is necessary especially to cancel the long polling when configuration change was made, e.g. screen was rotated. Otherwise multiple long polling requests would be running after configuration changes.
Because it not possible to launch a new coroutine in a scope that was canceled, it is necessary to re-initialize the scope.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
if chatMessageList is empty after handleSystemMessages it makes no sense to call the following methods.
Also processMessagesFromTheFuture was executed which caused that the popup was shown.
A better solution for the future should be to handle(remove) the "to-hide" system messages already in the repo or viewmodel
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
The idea to show the progress bar (aka shimmer animation) was that it show not be shown at all if loading messages happens in less than a second (this was a better UX in my opinion).
The idea was to hide the progress bar when
ChatMessageStartState was triggered.
However there can be moment when adapter is still empty after ChatMessageStartState and if in this moment the DELAY_TO_SHOW_PROGRESS_BAR is reached before the adapter is actually filled, the overlay happens.
It could be a solution to move the hiding of the progress bar, however then special cases might have to be handled.
For simplicity, the logic for DELAY_TO_SHOW_PROGRESS_BAR is removed. Progress bar is always shown without a delay so it wont be triggered on a later moment and can't overlay the chat.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
By using
networkMonitor.isOnline.first()
the function
unregisterNetworkCallback
was triggered, which sometimes causes the ConnectivityManager$TooManyRequestsException.
So each time isOnline.first() was called, the callbackFlow would:
- Register a new NetworkCallback.
- Emit a value and cancel the flow.
- Unregister the NetworkCallback.
The exception was:
Exception android.net.ConnectivityManager$TooManyRequestsException:
at android.net.ConnectivityManager.convertServiceException (ConnectivityManager.java:3771)
at android.net.ConnectivityManager.sendRequestForNetwork (ConnectivityManager.java:3960)
at android.net.ConnectivityManager.sendRequestForNetwork (ConnectivityManager.java:3967)
at android.net.ConnectivityManager.registerNetworkCallback (ConnectivityManager.java:4349)
at android.net.ConnectivityManager.registerNetworkCallback (ConnectivityManager.java:4319)
at com.nextcloud.talk.data.network.NetworkMonitorImpl$isOnline$1.invokeSuspend (NetworkMonitorImpl.kt:61)
To fix this, the cold flow from callbackFlow is converted to a StateFlow.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
It seems there could have been a race condition because currentUser was initialized too late.
I was not able to reproduce but hopefully this fixes it:
- initialize currentUser earlier (moved from onResume to onCreate)
- use currentUserProvider instead userManager
NPEs were reported via gplay:
1)
Exception java.lang.NullPointerException:
at com.nextcloud.talk.conversationlist.ConversationsListActivity.addToConversationItems (ConversationsListActivity.kt:851)
at com.nextcloud.talk.conversationlist.ConversationsListActivity.access$addToConversationItems (ConversationsListActivity.kt:151)
at com.nextcloud.talk.conversationlist.ConversationsListActivity$initObservers$5$1.invokeSuspend (ConversationsListActivity.kt:394)
at com.nextcloud.talk.conversationlist.ConversationsListActivity$initObservers$5$1.invoke (Unknown Source:8)
at com.nextcloud.talk.conversationlist.ConversationsListActivity$initObservers$5$1.invoke (Unknown Source:4)
2)
Exception java.lang.RuntimeException:
at android.app.ActivityThread.performResumeActivity (ActivityThread.java:5427)
at android.app.ActivityThread.handleResumeActivity (ActivityThread.java:5508)
...
Caused by java.lang.NullPointerException:
at com.nextcloud.talk.conversationlist.ConversationsListActivity.shouldShowNotificationWarning (ConversationsListActivity.kt:1557)
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
by moving networkMonitor.isOnline to separate check and by setting
binding.checkMark.visibility = View.INVISIBLE
binding.sendingProgress.visibility = View.GONE
before setting the status icons
to to handle recyclerview behavior
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
If user sent a message as a first message in today's chat, the temp message will be deleted when messages are retrieved from server, but also the date has to be deleted as it will be added again when the chat messages are added from server. Otherwise date "Today" would be shown twice.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
reason was that the UI was not yet loaded but isScrolledToBottom was already called, so findFirstVisibleItemPosition returned -1.
Fix for now is to return true for isScrolledToBottom when position is -1
They does not solve the root cause for now. It should be made sure the code is not executed until UI is ready.
A quick try with
repeatOnLifecycle(Lifecycle.State.STARTED)
when collecting getMessageFlow did not help.
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
TODO:
check id type --> see TODO "currentTimeMillies fails as id because later on in the model it's not Long but Int!!!!" in OfflineFirstChatRepository.kt
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
(as datasources should be only used in repositories)
use coroutines instead RxJava for api calls triggered by MessageInputViewModel
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
The speaking state is still sent only through data channels, as it is
not currently handled by other clients when sent through signaling
messages.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Note that this implicitly send the current state to remote participants
when the local participant joins, as in that case all the remote
participants already in the call join from the point of view of the
local participant
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This is not possible when Janus is used, as Janus only allows
broadcasting data channel messages to all the subscribers of the
publisher connection.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The LocalStateBroadcaster observes changes in the
LocalCallParticipantModel and notifies other participants in the call as
needed. Although it is created right before joining the call there is a
slim chance of the state changing before the local participant is
actually in the call, but even in that case other participants would not
be notified about the state due to the MessageSender depending on the
list of call participants / peer connections passed to it, which should
not be initialized before the local participant is actually in the call.
There is, however, a race condition that could cause participants to not
be added to the participant list if they join at the same time as the
local participant and a signaling message listing them but not the local
participant as in the call is received once the CallParticipantList was
created, but that is unrelated to the broadcaster and will be fixed
in another commit.
Currently only changes in the audio, speaking and video state are
notified, although in the future it should also notify about the nick,
the raised hand or any other state (but not one-time events, like
reactions). The notifications right now are sent only through data
channels, but at a later point they will be sent also through signaling
messages as needed.
Similarly, although right now it only notifies of changes in the state
it will also take care of notifying other participants about the current
state when they join the call (or the local participant joins).
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This is the counterpart of CallParticipantModel for the local
participant. For now it just stores whether audio and video are enabled
or not, and whether the local participant is speaking or not, but it
will be eventually extended with further properties.
It is also expected that the views, like the button with the microphone
state, will update themselves based on the model. Similarly the model
should be moved from the CallActivity to a class similar to
CallParticipant but for the local participant. In any case, all that is
something for the future; the immediate use of the model will be to know
when the local state changes to notify other participants.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Data channel messages are expected to be sent only to peer connections
with "video" type, which provide the audio and video tracks of the
participant (and, in fact, peer connections for screen shares do not
even have data channels enabled in the WebUI).
Note that this could change if at some point several audio/video tracks
are sent in the same peer connection, or if "speaking" messages are
added to screen shares, but that will be addressed if/when that happens.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
For now it just provides support for sending a data channel message to
all participants, so notifying all participants when the media is
toggled or the speaking status change can be directly refactored to use
it.
While it would have been fine to use a single class for both MCU and no
MCU they were split for easier and cleaner unit testing in future
stages.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
"hasMCU" (which has always been the wrong name, because it is an SFU
rather than an MCU, but it is wrong even in the signaling server so for
now the legacy name is kept) was set again and again whenever the call
participant list changed. Now it is set instead once its value is known,
that is, when it is known that the internal signaling server is used (as
no "MCU" is used in that case), or when the connection with the external
signaling server is established, as its supported features are not known
until then.
This change should have no effect in the usages of "hasMCU", as it is
used when the call participant list change, which will happen only after
joining the call in the signaling server, or when sending "isSpeaking"
and toggling media, in both cases guarded by "isConnectionEstablished",
which will be true only once "performCall" was called or if the call is
active with other participants.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The SDP constraints for publisher connections when the MCU is used were
set for all connections. Those constraints set "OfferToReceiveAudio" and
"OfferToReceiveVideo" to false, which disables receiving audio and video
when the local participant is the one sending the offer. Therefore,
audio and video was not received when the MCU was not used and the local
participant was the one initiating the connection.
The "OfferToReceiveXXX" configurations have no effect when set on an
answer (and thus are not even set, an empty MediaConstraints is used in
that case). However, when "OfferToReceiveVideo = false" is set the video
transceiver is explicitly stopped (which is used to avoid receiving
video when joining a call with audio only). Therefore, as
"OfferToReceiveVideo = false" was always set, video was never received
in subscriber connections when the MCU is used, or connections initiated
by the other peer when the MCU is not used.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
