-
Notifications
You must be signed in to change notification settings - Fork 171
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
Fixed a few issues with the OpenGroupPoller #1241
Fixed a few issues with the OpenGroupPoller #1241
Conversation
mpretty-cyro
commented
Jun 7, 2023
- Fixed an issue where the admin/moderator status wasn't getting stored if set before joining a community
- Fixed an issue where multiple pollers for the same server could run at the same time when joining multiple rooms within the same app run (very noticeable when restoring/linking)
Fixed an issue where the admin/moderator status wasn't getting stored if set before joining a community Fixed an issue where multiple pollers for the same server could run at the same time when joining multiple rooms within the same app run (very noticeable when restoring/linking)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly needs synchronization on runId
.
app/src/main/java/org/thoughtcrime/securesms/groups/OpenGroupManager.kt
Outdated
Show resolved
Hide resolved
.../src/main/java/org/session/libsession/messaging/sending_receiving/pollers/OpenGroupPoller.kt
Outdated
Show resolved
Hide resolved
@@ -41,11 +41,13 @@ object OpenGroupManager { | |||
isPolling = true | |||
val storage = MessagingModuleConfiguration.shared.storage | |||
val servers = storage.getAllOpenGroups().values.map { it.server }.toSet() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless .toSet()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the conversion to Set<T>
is in order to remove duplicates from the collection - getAllOpenGroups()
returns every community you are a member of, so if you are in multiple rooms on the same server (eg. Session
, Oxen
, Session Updates
all on the getsession.org
server) the result would have 3 entries for getsession.org
The poll
request actually makes a single /batch
API call to the server which includes a call for each room on that server (so without the toSet()
the forEach
loop would end up redundantly initialising and triggering a poll
for each copy of the server)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohhh... cool.
.../src/main/java/org/session/libsession/messaging/sending_receiving/pollers/OpenGroupPoller.kt
Outdated
Show resolved
Hide resolved
}.fail { | ||
updateCapabilitiesIfNeeded(isPostCapabilitiesRetry, it) | ||
updateCapabilitiesIfNeeded(isPostCapabilitiesRetry, currentRunId, it) | ||
}.map { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless map{}
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The map was there to convert the return value from Promise<List<OpenGroupApi.BatchResponse<*>>, Exception>
to Promise<Unit, Exception>