* 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>
* MM-23264 Add api endpoint for get groups with members in channel
Add store tests
Add tests for api func
Gofmt
Apply changes from code review
* MM-23264 Make store layers
* MM-23264 Check read permission on channel member counts
* Trigger CI
* MM-23800: remove goroutineID and stack printing
Each hub has a goroutineID which is calculated with a known hack.
The FAQ clearly explains why goroutines don't have an id:
https://golang.org/doc/faq#no_goroutine_id.
We only added that because sometimes the hub would be deadlocked and
having the goroutineID would be useful when getting the stack trace.
This is also problematic in stress tests because the hubs would
frequently get overloaded and the logs would unnecessarily have stack traces.
But that was in the past, and we have done extensive testing with
load tests and fuzz testing to smooth any rough edges remaining.
Including adding additional metrics for hub buffer size.
Monitoring the metrics is a better way to approach this problem.
Therefore, we remove these kludges from the code.
* Also remove deadlock checking code
There is no need for that anymore since
we are getting rid of the stack printing anyways.
Let's do a wholesale refactor and clean up the codebase.
* MM-23805: Refactor web_hub
This is a beginning of the refactoring of the websocket code.
To start off with, we unexport some methods and constants which did not
need to be exported. There are more remaining but some are out of scope for this PR.
The main chunk of refactor is to unexport the webconn send channel
which was the main cause of panics. Since we were directly sending
to the connection from various parts of the codebase, it would be possible
that the send channel would be closed and we could still send a message.
This would crash the server.
To fix this, we refactor the code to centralize all sending from the main
hub goroutine. This means we can leverage the connections map to check
if the connection exists or not, and only then send the message.
We also move the cluster calls to cluster.go.
* bring back cluster code inside hub
* Incorporate review comments
* Address review comments
* rename index
* MM-23807: Refactor web_conn
- Unexport some struct fields and constants which are not necessary
to be accessed from outside the package. This will help us moving
the entire websocket handling code to a separate package later.
- Change some empty string checks to check for empty string rather
than doing a len check which is more idiomatic. Both of them compile
to the same code. So it doesn't make a difference performance-wise.
- Remove redundant ToJson calls to get the length.
- Incorporate review comments
- Unexport some more methods
* Fix field name
* Run make app-layers
* Add note on hub check
The current code path for `CreatePostAsUser` tries to update the `LastViewedAt` for bots, which logs a warning message if the bot isn't actually in the channel.
This pull request changes the semantics of bot posting to not update the bot's `LastViewedAt` timestamp, avoiding this log altogether. It matches the semantics of `from_webhook`, but notably makes bots slightly less like users in that they no longer "read" channels when they post. This seems reasonable, but I'm both looking for validation of this semantic change in addition to the code review.
Fixes: https://mattermost.atlassian.net/browse/MM-23926
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* MM-23800: remove goroutineID and stack printing
Each hub has a goroutineID which is calculated with a known hack.
The FAQ clearly explains why goroutines don't have an id:
https://golang.org/doc/faq#no_goroutine_id.
We only added that because sometimes the hub would be deadlocked and
having the goroutineID would be useful when getting the stack trace.
This is also problematic in stress tests because the hubs would
frequently get overloaded and the logs would unnecessarily have stack traces.
But that was in the past, and we have done extensive testing with
load tests and fuzz testing to smooth any rough edges remaining.
Including adding additional metrics for hub buffer size.
Monitoring the metrics is a better way to approach this problem.
Therefore, we remove these kludges from the code.
* Also remove deadlock checking code
There is no need for that anymore since
we are getting rid of the stack printing anyways.
Let's do a wholesale refactor and clean up the codebase.
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* Remove unnecessary check for PERMISSION_LIST_TEAM_CHANNELS
In the autocompleteChannelsForTeamForSearch method we're checking for
the PERMISSION_LIST_TEAM_CHANNELS permission in order to avoid filtering
channels in the autocomplete search but this check is not necessary.
Now we're going directly to the database to search for those channels
in this specific method and we're filtering by channel membership and
team so there is no chance that we are going to filter undesired
channels to the user.
[Here](https://github.com/mattermost/mattermost-server/blob/v5.22.0/store/sqlstore/channel_store.go#L2014)
is the query where you can see the filtering we're making
* MM-23017 Check group mentions as part of notification logic
* Add nil groups to existing test cases
* MM-23017 Add tests for insertGroupMention and addGroupMention
* MM-23017 Add tests for getExplicitMentions that have groups
* Add tests for group store GetMemberUsersNotInChannel
* MM-23017 Add tests for AllowGroupMentions
* MM-23017 Fix error message name
* MM-23017 Swap Checks to Name
* MM-23017 Code review fixes
* Rename var and fix allowGroupMentions test
* MM-23017 Use GetMemberUsersInTeam inside of insertGroupMentions
* MM-23017 use group mentions permission
* Actually call GetMemberUsersInTeam
* Remove unnecessary new line
* Uncomment filter allow reference
* MM-23017 Fix group channel notifications
* Update store layer
* MM-23017 Improve test coverage for group channels
* Trigger CI
* Trigger CI
* MM-20934: Fixing int overflow in 32 bits on MaxImageSize check
* Adding comments explaining the casting and the bug fixed there
* Apply suggestions from code review
Co-Authored-By: Juho Nurminen <juhonurm@gmail.com>
* Fixing store layers
Co-authored-by: Juho Nurminen <juhonurm@gmail.com>
* ResponseWriter wrapper to get status code
For our metrics, we need the status code returned by a request
so this wrapper includes a new method StatusCode() that includes
the desired code
* Shadow the responsewriter variable in the handlers
In order to avoid confusion to people deciding what variable to use.
I've also changed the tests to reflect this change and added a new
one that checks the Flush method works
* MM-23244: Validate that either both or neither AuthData and AuthService fields are set.
* MM-23244: Readability improvement.
* MM-23244: Adds translation. Tests for error id.
* MM-23244: Fix test.
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* Add database server version to telemetry
Also added a new query in the store to retrieve the database version
* Add test for the GetDbVersion function
* More drivers in the tests
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
* Jenkins: Make mysql and postgres tests run in parallel
* Trying again
* Start docker containers individually for each stage
This is because the parallel block does not allow
a common step to be specified.
* Giving a random name to each docker-compose
* Fixing order of the param
* Giving unique project names for tests
* Setting the right datasource and drivername
* Trying to use different copies of the src directory for test the databas3
* moving workspace copy to the beginning of postgres tests
* Fixing some missed out patches
* Added missing LDAP population command
* Upload all test results
And set allowEmptyResults to false.
* Pruning remaining docker networks
* Remove unneeded settings
Co-authored-by: mattermod <mattermod@users.noreply.github.com>
Co-authored-by: Jesús Espino <jespinog@gmail.com>
* MM-23666: rewrite TestHTTPClient
The existing `TestHTTPClient` actually tries to reach google.com, making it brittle to network hiccups (as unlikely as that might be). Rewrite the test to rely on a local server instead.
I found the existing implementation of `TestHTTPClient` extremely hard to understand, so I've rewritten it more explicitly, expanded coverage somewhat. In doing so, I found myself somewhat surprised by the current behaviour. If `allowHost` returns `true`, we ignore `allowIP` altogether. If `allowHost` returns `false`, we check with `allowIP`. There's a lack of symmetry here that the current filter names don't make clear. Perhaps `allowHost` should be renamed `skipIPCheckForHost`?
Fixes: https://mattermost.atlassian.net/browse/MM-23666
* more compact assertions
