Remove global variables and fix racy test (#18733)

https://community.mattermost.com/boards/workspace/zyoahc9uapdn3xdptac6jb69ic/285b80a3-257d-41f6-8cf4-ed80ca9d92e5/495cdb4d-c13a-4992-8eb9-80cfee2819a4?c=65b44de0-1a9c-4643-84c1-5d803593f314

This PR improves on two things at once:
- It fixes the race condition by removing global variables.
- But instead of moving them to Server, it moves them inside Channels,
thereby making one more step towards the multi-product architecture.

```release-note
NONE
```
Этот коммит содержится в:
Agniva De Sarker
2021-10-18 19:38:16 +05:30
коммит произвёл GitHub
родитель c5811939c4
Коммит 2a07d4641a
2 изменённых файлов: 20 добавлений и 20 удалений

Просмотреть файл

@@ -4,6 +4,7 @@
package app package app
import ( import (
"github.com/mattermost/mattermost-server/v6/model"
"github.com/mattermost/mattermost-server/v6/services/imageproxy" "github.com/mattermost/mattermost-server/v6/services/imageproxy"
) )
@@ -12,6 +13,13 @@ type Channels struct {
srv *Server srv *Server
imageProxy *imageproxy.ImageProxy imageProxy *imageproxy.ImageProxy
// cached counts that are used during notice condition validation
cachedPostCount int64
cachedUserCount int64
cachedDBMSVersion string
// previously fetched notices
cachedNotices model.ProductNotices
} }
func init() { func init() {

Просмотреть файл

@@ -29,14 +29,6 @@ const MinSecondsBetweenRepeatViewings = 60 * 60
// http request cache // http request cache
var noticesCache = utils.RequestCache{} var noticesCache = utils.RequestCache{}
// cached counts that are used during notice condition validation
var cachedPostCount int64
var cachedUserCount int64
var cachedDBMSVersion string
// previously fetched notices
var cachedNotices model.ProductNotices
var rcStripRegexp = regexp.MustCompile(`(.*?)(-rc\d+)(.*?)`) var rcStripRegexp = regexp.MustCompile(`(.*?)(-rc\d+)(.*?)`)
func cleanupVersion(originalVersion string) string { func cleanupVersion(originalVersion string) string {
@@ -269,7 +261,7 @@ func (a *App) GetProductNotices(c *request.Context, userID, teamID string, clien
filteredNotices := make([]model.NoticeMessage, 0) filteredNotices := make([]model.NoticeMessage, 0)
for noticeIndex, notice := range cachedNotices { for noticeIndex, notice := range a.ch.cachedNotices {
// check if the notice has been viewed already // check if the notice has been viewed already
var view *model.ProductNoticeViewState var view *model.ProductNoticeViewState
for viewIndex, v := range views { for viewIndex, v := range views {
@@ -297,17 +289,17 @@ func (a *App) GetProductNotices(c *request.Context, userID, teamID string, clien
userID, userID,
client, client,
clientVersion, clientVersion,
cachedPostCount, a.ch.cachedPostCount,
cachedUserCount, a.ch.cachedUserCount,
isSystemAdmin, isSystemAdmin,
isTeamAdmin, isTeamAdmin,
isCloud, isCloud,
sku, sku,
dbName, dbName,
cachedDBMSVersion, a.ch.cachedDBMSVersion,
searchEngineName, searchEngineName,
searchEngineVersion, searchEngineVersion,
&cachedNotices[noticeIndex]) &a.ch.cachedNotices[noticeIndex])
if err != nil { if err != nil {
return nil, model.NewAppError("GetProductNotices", "api.system.update_notices.validating_failed", nil, err.Error(), http.StatusBadRequest) return nil, model.NewAppError("GetProductNotices", "api.system.update_notices.validating_failed", nil, err.Error(), http.StatusBadRequest)
} }
@@ -337,7 +329,7 @@ func (a *App) UpdateViewedProductNotices(userID string, noticeIds []string) *mod
// user as viewed in order to avoid showing them imminently on first login // user as viewed in order to avoid showing them imminently on first login
func (a *App) UpdateViewedProductNoticesForNewUser(userID string) { func (a *App) UpdateViewedProductNoticesForNewUser(userID string) {
var noticeIds []string var noticeIds []string
for _, notice := range cachedNotices { for _, notice := range a.ch.cachedNotices {
noticeIds = append(noticeIds, notice.ID) noticeIds = append(noticeIds, notice.ID)
} }
if err := a.Srv().Store.ProductNotices().View(userID, noticeIds); err != nil { if err := a.Srv().Store.ProductNotices().View(userID, noticeIds); err != nil {
@@ -351,33 +343,33 @@ func (a *App) UpdateProductNotices() *model.AppError {
skip := *a.Srv().Config().AnnouncementSettings.NoticesSkipCache skip := *a.Srv().Config().AnnouncementSettings.NoticesSkipCache
mlog.Debug("Will fetch notices from", mlog.String("url", url), mlog.Bool("skip_cache", skip)) mlog.Debug("Will fetch notices from", mlog.String("url", url), mlog.Bool("skip_cache", skip))
var err error var err error
cachedPostCount, err = a.Srv().Store.Post().AnalyticsPostCount("", false, false) a.ch.cachedPostCount, err = a.Srv().Store.Post().AnalyticsPostCount("", false, false)
if err != nil { if err != nil {
mlog.Warn("Failed to fetch post count", mlog.String("error", err.Error())) mlog.Warn("Failed to fetch post count", mlog.String("error", err.Error()))
} }
cachedUserCount, err = a.Srv().Store.User().Count(model.UserCountOptions{IncludeDeleted: true}) a.ch.cachedUserCount, err = a.Srv().Store.User().Count(model.UserCountOptions{IncludeDeleted: true})
if err != nil { if err != nil {
mlog.Warn("Failed to fetch user count", mlog.String("error", err.Error())) mlog.Warn("Failed to fetch user count", mlog.String("error", err.Error()))
} }
cachedDBMSVersion, err = a.Srv().Store.GetDbVersion(false) a.ch.cachedDBMSVersion, err = a.Srv().Store.GetDbVersion(false)
if err != nil { if err != nil {
mlog.Warn("Failed to get DBMS version", mlog.String("error", err.Error())) mlog.Warn("Failed to get DBMS version", mlog.String("error", err.Error()))
} }
cachedDBMSVersion = strings.Split(cachedDBMSVersion, " ")[0] // get rid of trailing strings attached to the version a.ch.cachedDBMSVersion = strings.Split(a.ch.cachedDBMSVersion, " ")[0] // get rid of trailing strings attached to the version
data, err := utils.GetURLWithCache(url, &noticesCache, skip) data, err := utils.GetURLWithCache(url, &noticesCache, skip)
if err != nil { if err != nil {
return model.NewAppError("UpdateProductNotices", "api.system.update_notices.fetch_failed", nil, err.Error(), http.StatusBadRequest) return model.NewAppError("UpdateProductNotices", "api.system.update_notices.fetch_failed", nil, err.Error(), http.StatusBadRequest)
} }
cachedNotices, err = model.UnmarshalProductNotices(data) a.ch.cachedNotices, err = model.UnmarshalProductNotices(data)
if err != nil { if err != nil {
return model.NewAppError("UpdateProductNotices", "api.system.update_notices.parse_failed", nil, err.Error(), http.StatusBadRequest) return model.NewAppError("UpdateProductNotices", "api.system.update_notices.parse_failed", nil, err.Error(), http.StatusBadRequest)
} }
if err := a.Srv().Store.ProductNotices().ClearOldNotices(cachedNotices); err != nil { if err := a.Srv().Store.ProductNotices().ClearOldNotices(a.ch.cachedNotices); err != nil {
return model.NewAppError("UpdateProductNotices", "api.system.update_notices.clear_failed", nil, err.Error(), http.StatusBadRequest) return model.NewAppError("UpdateProductNotices", "api.system.update_notices.clear_failed", nil, err.Error(), http.StatusBadRequest)
} }
return nil return nil