We would call SetStatusOnline in a goroutine before actually calling HubRegister.
This could cause the message not to be sent after all, because there's no guarantee
that Register would actually happen before it.
To fix it, we just change the order of things.
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
This test writes directly to a connection
which causes panics and more frustration in an already fragile CI.
Since this anyways checks an edge condition, and will anyways be
removed in v6, let's remove this for now and let CI be happy.
After registering the conn in the hub, we proceeded to send a
direct message to the user. We had changed it to send the direct message
in the same hub goroutine that handles the registration. This was the correct
behavior and fixes chances of having panics due to sending to closed channels.
However, often fixing something unearths some deeper underlying bug. This was
such a case :)
The issue was that register channel had a buffer size of 1. And we were sending
a direct message after registration. In the code to send direct message, we
were checking if the user has been registered or not, and if not, then skip it.
Therefore, since the register channel buffer was 1, it could very well be that
the select case would pick up the direct message send case first - in which
case it would not have been registered, and therefore no hello message would be sent.
The fix is to unbuffer the register and unregister channels. There does not seem
to be a valid reason to make these buffered channels. They are meant to be
synchronous operations, because the code following them assumes that the user
has been registered.
While here, we also remove all the time.Sleeps before waiting on the Response channel
because they are not required at all. Waiting on a channel is already blocking.
Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com>
* MM-24987 Dont call a.GetSchemeRolesForChannel and instead load the scheme using the channel directly
* Trigger CI
* MM-24987 Add a test for concurrent patch to channel moderation
* MM-24987 Cleanup
* MM-24828 Take into account group synced state of team and channel when getting groups for mentions
* Update app/notification_test.go
* i18n-extract
* Revert "i18n-extract"
This reverts commit dcb0426b98afa4646c26870c0e3a1236f99fda17.
* Trigger CI
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* add a since parameter to getGroups api
* update for lint error
* when using since, return deleted groups as well.
* update flaky test, groups have same create time
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* add warning count as return value
* add warning count as return value
* fix file name
* update mock
* add setting warning to db
* replace wrongly removed string
* add dummy function to see if it will build
* remove dummy function
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* add getGroupsByUserId to API layer
* update for lint errors
* add check for contextId = userId or ManageSystem Permission
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* Disable read/search db replicas in TE/E0
* fixing tests
* Removing unnecesary text.
* Updating without-license read-replicas config before store initialization
* Reconnecting to database after remove read replicas
* MM-23935 extend session expiry on user activity
- if user types anything before a session expires the session will be extended to now + session length
- ensures new session expiries are not written to DB too frequently
- new session store func for updating session ExpiresAt
- session length defaults for mobile and web/ldap changed from 180 days to 30 days
This corrects an issue found when running multiple clustered
Mattermost servers and using the optional `get_server_status`
check on the ping API endpoint. This is done by allowing each
server to write and read from its own health check key in the
system table.
* MM-24547: Fix writer leak when connection closes
When the connection is closed, the exit path does not
shut down the writer goroutine. In which case, it will keep spinning forever.
Since we already have the CAS mechanism now, we can move the closing
functionality into the main Close method and just call that in the defer block.
This makes closing the websocket client idempotent from both perspective -
- Explicitly closing.
- Closing due to connection tear down.
There are still 2 races left:
- Using the exported Conn to directly write messages. We cannot do anything about
that as long as clients directly using that.
- Setting the wsc.pingTimeoutTimer field in a separate goroutine when calling
.Connect(). This will need to be seen later.
* Fix ineffectual assignment
* Duplicate the closing of writer
The problem with refactoring the writer closing to a common
function was that we needed to wait for the reader to exit
before closing the EventChannel and ResponseChannel.
But then there is another problem that the API can be used in such
a way that the client is liable to call Close without even calling
Listen. In that case, we cannot wait for Listen to quit.
So from Close, we can only close the connection. And therefore
we need to duplicate the writer closing in the read loop's
defer block.
* Cleanup some comments
Given that this query is part of the top 5 most used queries
we want to move it to use raw queries instead of gorp so we
can get rid of the reflection overhead
* MM-24395: Set response header to be attachment for SVG images.
We check the file extension and appropriately set the response header.
SVG images without a file extension aren't proxied at all. So there's no
problem with that.
* Add test
* Improve test a bit
* Capture content type
* incorporate comments
* Set attachment mode in case of error too
* MM-24397: Reusing the read buffer while reading messages from websockets
The core problem was that conn.ReadMessage allocated a buffer every time it was read.
This created heavy amount of allocations every single time we read a message from the websocket.
To avoid this, we bypass the ReadMessage which was more of a helper method,
and actually call the NextReader which returns a reader object.
We can then reuse a single byte.Buffer instance to read the object unmarshal
into a WebSocketEvent object. This gets rid of the allocation in the read path completly
and allows GC more time to do other tasks.
* Incorporate review comments
* Move reset buffer to top of loop
* Cleanup further
* Fix test
* Final fix
* Fixing system messages about non-visible users
* Adding unit tests to verify the new behavior
* Regenerating app layers
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
We invert the skipSend condition and outdent the remaining block
to make the code a bit more idiomatic.
While here, we also change the dropping message level from info to
warn because that's what it should be.
On user activity, we were clearing the job.pendingNotifications map.
But we had already created a copy of the notifications slice while
iterating the map. Therefore, if we pass the copied slice, it would
still have the old notifications which were originally deleted.
The unit tests would not catch this because it was testing the
job.pendingNotifications map and not actually checking if the email
handler was being called or not. We fix that now.
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
