From 5b252e87366c22df73e335d4623e5a1ef3eeb117 Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Mon, 13 May 2019 10:53:46 -0400 Subject: [PATCH] Tracking Push Notifications in a structured logger (notifications.log) (#10823) * Remove NotificationRegistry table and use structured logging * Fix ackId for notification sent * Notification logger at server level * Remove unused i18n strings --- api4/system.go | 7 -- app/app.go | 3 +- app/notification.go | 34 +++--- app/notification_push.go | 107 ++++++++---------- app/options.go | 1 + app/server.go | 16 ++- config/default.json | 9 ++ i18n/en.json | 40 ------- model/config.go | 106 +++++++++++------ model/push_notification.go | 56 +-------- store/layered_store.go | 4 - store/sqlstore/notification_registry_store.go | 81 ------------- .../notification_registry_store_test.go | 14 --- store/sqlstore/store.go | 1 - store/sqlstore/supplier.go | 6 - store/store.go | 7 -- .../mocks/LayeredStoreDatabaseLayer.go | 16 --- .../mocks/NotificationRegistryStore.go | 70 ------------ store/storetest/mocks/SqlStore.go | 16 --- store/storetest/mocks/Store.go | 16 --- .../storetest/notification_registry_store.go | 106 ----------------- store/storetest/store.go | 37 +++--- utils/logger.go | 31 ++++- 23 files changed, 206 insertions(+), 578 deletions(-) delete mode 100644 store/sqlstore/notification_registry_store.go delete mode 100644 store/sqlstore/notification_registry_store_test.go delete mode 100644 store/storetest/mocks/NotificationRegistryStore.go delete mode 100644 store/storetest/notification_registry_store.go diff --git a/api4/system.go b/api4/system.go index c41ae82859..dc8ab3febf 100644 --- a/api4/system.go +++ b/api4/system.go @@ -334,13 +334,6 @@ func pushNotificationAck(c *Context, w http.ResponseWriter, r *http.Request) { return } - mlog.Debug("Push notification ack received", - mlog.String("ackId", ack.Id), - mlog.String("platform", ack.ClientPlatform), - mlog.String("type", ack.NotificationType), - mlog.Int64("at", ack.ClientReceivedAt), - ) - ReturnStatusOK(w) return } diff --git a/app/app.go b/app/app.go index e3624187c7..406efa75b1 100644 --- a/app/app.go +++ b/app/app.go @@ -23,7 +23,8 @@ import ( type App struct { Srv *Server - Log *mlog.Logger + Log *mlog.Logger + NotificationsLog *mlog.Logger T goi18n.TranslateFunc Session model.Session diff --git a/app/notification.go b/app/notification.go index 7b5c97aa1e..421464e083 100644 --- a/app/notification.go +++ b/app/notification.go @@ -308,16 +308,13 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod ) } else { // register that a notification was not sent - notificationRegistry := model.NotificationRegistry{ - UserId: id, - PostId: post.Id, - SendStatus: model.PUSH_NOT_SENT, - Type: model.PUSH_TYPE_MESSAGE, - } - _, appErr := a.Srv.Store.NotificationRegistry().Save(¬ificationRegistry) - if appErr != nil { - mlog.Debug(appErr.Error()) - } + a.NotificationsLog.Warn("Notification not sent", + mlog.String("ackId", ""), + mlog.String("type", model.PUSH_TYPE_MESSAGE), + mlog.String("userId", id), + mlog.String("postId", post.Id), + mlog.String("status", model.PUSH_NOT_SENT), + ) } } @@ -343,16 +340,13 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod ) } else { // register that a notification was not sent - notificationRegistry := model.NotificationRegistry{ - UserId: id, - PostId: post.Id, - SendStatus: model.PUSH_NOT_SENT, - Type: model.PUSH_TYPE_MESSAGE, - } - _, appErr := a.Srv.Store.NotificationRegistry().Save(¬ificationRegistry) - if appErr != nil { - mlog.Debug(appErr.Error()) - } + a.NotificationsLog.Warn("Notification not sent", + mlog.String("ackId", ""), + mlog.String("type", model.PUSH_TYPE_MESSAGE), + mlog.String("userId", id), + mlog.String("postId", post.Id), + mlog.String("status", model.PUSH_NOT_SENT), + ) } } } diff --git a/app/notification_push.go b/app/notification_push.go index 22f79c5519..97060d317b 100644 --- a/app/notification_push.go +++ b/app/notification_push.go @@ -112,34 +112,30 @@ func (a *App) sendPushNotificationSync(post *model.Post, user *model.User, chann tmpMessage.SetDeviceIdAndPlatform(session.DeviceId) tmpMessage.AckId = model.NewId() - mlog.Debug( - "Sending push notification", - mlog.String("ackId", tmpMessage.AckId), - mlog.String("deviceId", tmpMessage.DeviceId), - mlog.String("userId", user.Id), - ) - err := a.sendToPushProxy(*tmpMessage, session) if err != nil { - mlog.Error( - "Failed to send Push Notification:", - mlog.String("error", err.Error()), + a.NotificationsLog.Error("Notification error", + mlog.String("ackId", tmpMessage.AckId), + mlog.String("type", tmpMessage.Type), mlog.String("userId", session.UserId), - mlog.String("sessionId", session.Id), - mlog.String("deviceId", msg.DeviceId), - mlog.String("ackId", msg.AckId), + mlog.String("postId", tmpMessage.PostId), + mlog.String("channelId", tmpMessage.ChannelId), + mlog.String("deviceId", tmpMessage.DeviceId), + mlog.String("status", err.Error()), ) - appErr := a.Srv.Store.NotificationRegistry().UpdateSendStatus(tmpMessage.AckId, model.PUSH_SEND_ERROR+": "+err.Error()) - if appErr != nil { - mlog.Debug(appErr.Error()) - } + continue } - appErr := a.Srv.Store.NotificationRegistry().UpdateSendStatus(tmpMessage.AckId, model.PUSH_SEND_SUCCESS) - if appErr != nil { - mlog.Debug(appErr.Error()) - } + a.NotificationsLog.Info("Notification sent", + mlog.String("ackId", tmpMessage.AckId), + mlog.String("type", tmpMessage.Type), + mlog.String("userId", session.UserId), + mlog.String("postId", tmpMessage.PostId), + mlog.String("channelId", tmpMessage.ChannelId), + mlog.String("deviceId", tmpMessage.DeviceId), + mlog.String("status", model.PUSH_SEND_SUCCESS), + ) if a.Metrics != nil { a.Metrics.IncrementPostSentPush() @@ -248,35 +244,30 @@ func (a *App) ClearPushNotificationSync(currentSessionId, userId, channelId stri tmpMessage.SetDeviceIdAndPlatform(session.DeviceId) tmpMessage.AckId = model.NewId() - mlog.Debug( - "Sending clear push notification", - mlog.String("ackId", tmpMessage.AckId), - mlog.String("deviceId", tmpMessage.DeviceId), - mlog.String("userId", session.UserId), - mlog.String("channelId", channelId), //should we remove the message from the logs? - ) - err := a.sendToPushProxy(*tmpMessage, session) if err != nil { - mlog.Error( - "Failed to send Push Notification:", - mlog.String("error", err.Error()), + a.NotificationsLog.Error("Notification error", + mlog.String("ackId", tmpMessage.AckId), + mlog.String("type", tmpMessage.Type), mlog.String("userId", session.UserId), - mlog.String("sessionId", session.Id), - mlog.String("deviceId", msg.DeviceId), - mlog.String("ackId", msg.AckId), + mlog.String("postId", tmpMessage.PostId), + mlog.String("channelId", tmpMessage.ChannelId), + mlog.String("deviceId", tmpMessage.DeviceId), + mlog.String("status", err.Error()), ) - appErr := a.Srv.Store.NotificationRegistry().UpdateSendStatus(tmpMessage.AckId, model.PUSH_SEND_ERROR+": "+err.Error()) - if appErr != nil { - mlog.Debug(appErr.Error()) - } + continue } - appErr := a.Srv.Store.NotificationRegistry().UpdateSendStatus(tmpMessage.AckId, model.PUSH_SEND_SUCCESS) - if appErr != nil { - mlog.Debug(appErr.Error()) - } + a.NotificationsLog.Info("Notification sent", + mlog.String("ackId", tmpMessage.AckId), + mlog.String("type", tmpMessage.Type), + mlog.String("userId", session.UserId), + mlog.String("postId", tmpMessage.PostId), + mlog.String("channelId", tmpMessage.ChannelId), + mlog.String("deviceId", tmpMessage.DeviceId), + mlog.String("status", model.PUSH_SEND_SUCCESS), + ) if a.Metrics != nil { a.Metrics.IncrementPostSentPush() @@ -343,18 +334,13 @@ func (a *App) StopPushNotificationsHubWorkers() { func (a *App) sendToPushProxy(msg model.PushNotification, session *model.Session) error { msg.ServerId = a.DiagnosticId() - notificationRegistry := model.NotificationRegistry{ - AckId: msg.AckId, - DeviceId: msg.DeviceId, - UserId: session.UserId, - PostId: msg.PostId, - Type: msg.Type, - } - - _, appErr := a.Srv.Store.NotificationRegistry().Save(¬ificationRegistry) - if appErr != nil { - return appErr - } + a.NotificationsLog.Info("Notification will be sent", + mlog.String("ackId", msg.AckId), + mlog.String("type", msg.Type), + mlog.String("userId", session.UserId), + mlog.String("postId", msg.PostId), + mlog.String("status", model.PUSH_SEND_PREPARE), + ) request, err := http.NewRequest("POST", strings.TrimRight(*a.Config().EmailSettings.PushNotificationServer, "/")+model.API_URL_SUFFIX_V1+"/send_push", strings.NewReader(msg.ToJson())) if err != nil { @@ -388,10 +374,13 @@ func (a *App) SendAckToPushProxy(ack *model.PushNotificationAck) error { return nil } - appErr := a.Srv.Store.NotificationRegistry().MarkAsReceived(ack.Id, ack.ClientReceivedAt) - if appErr != nil { - return appErr - } + a.NotificationsLog.Info("Notification received", + mlog.String("ackId", ack.Id), + mlog.String("type", ack.NotificationType), + mlog.String("deviceType", ack.ClientPlatform), + mlog.Int64("receivedAt", ack.ClientReceivedAt), + mlog.String("status", model.PUSH_RECEIVED), + ) request, err := http.NewRequest( "POST", diff --git a/app/options.go b/app/options.go index 7ad54cc3a7..4409b5aedf 100644 --- a/app/options.go +++ b/app/options.go @@ -99,6 +99,7 @@ func ServerConnector(s *Server) AppOption { a.Srv = s a.Log = s.Log + a.NotificationsLog = s.NotificationsLog a.AccountMigration = s.AccountMigration a.Cluster = s.Cluster diff --git a/app/server.go b/app/server.go index dd403b37bf..7d033a6d5f 100644 --- a/app/server.go +++ b/app/server.go @@ -108,7 +108,8 @@ type Server struct { ImageProxy *imageproxy.ImageProxy - Log *mlog.Logger + Log *mlog.Logger + NotificationsLog *mlog.Logger joinCluster bool startMetrics bool @@ -152,7 +153,13 @@ func NewServer(options ...Option) (*Server, error) { } if s.Log == nil { - s.Log = mlog.NewLogger(utils.MloggerConfigFromLoggerConfig(&s.Config().LogSettings)) + s.Log = mlog.NewLogger(utils.MloggerConfigFromLoggerConfig(&s.Config().LogSettings, utils.GetLogFileLocation)) + } + + if s.NotificationsLog == nil { + notificationLogSettings := utils.GetLogSettingsFromNotificationsLogSettings(&s.Config().NotificationLogSettings) + s.NotificationsLog = mlog.NewLogger(utils.MloggerConfigFromLoggerConfig(notificationLogSettings, utils.GetNotificationsLogFileLocation)). + WithCallerSkip(1).With(mlog.String("logSource", "notifications")) } // Redirect default golang logger to this logger @@ -162,7 +169,10 @@ func NewServer(options ...Option) (*Server, error) { mlog.InitGlobalLogger(s.Log) s.logListenerId = s.AddConfigListener(func(_, after *model.Config) { - s.Log.ChangeLevels(utils.MloggerConfigFromLoggerConfig(&after.LogSettings)) + s.Log.ChangeLevels(utils.MloggerConfigFromLoggerConfig(&after.LogSettings, utils.GetLogFileLocation)) + + notificationLogSettings := utils.GetLogSettingsFromNotificationsLogSettings(&after.NotificationLogSettings) + s.NotificationsLog.ChangeLevels(utils.MloggerConfigFromLoggerConfig(notificationLogSettings, utils.GetNotificationsLogFileLocation)) }) s.HTTPService = httpservice.MakeHTTPService(s.FakeApp()) diff --git a/config/default.json b/config/default.json index dce2188f2a..b693daf2a2 100644 --- a/config/default.json +++ b/config/default.json @@ -147,6 +147,15 @@ "EnableWebhookDebugging": true, "EnableDiagnostics": true }, + "NotificationLogSettings": { + "EnableConsole": true, + "ConsoleLevel": "DEBUG", + "ConsoleJson": true, + "EnableFile": true, + "FileLevel": "INFO", + "FileJson": true, + "FileLocation": "" + }, "PasswordSettings": { "MinimumLength": 5, "Lowercase": false, diff --git a/i18n/en.json b/i18n/en.json index 008ed45f88..10075abb67 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -4790,34 +4790,6 @@ "id": "model.link_metadata.is_valid.url.app_error", "translation": "Link metadata URL must be set" }, - { - "id": "model.notification_registry.ack_id.app_error", - "translation": "Invalid Acknowledge Id" - }, - { - "id": "model.notification_registry.create_at.app_error", - "translation": "Create at must be a valid time" - }, - { - "id": "model.notification_registry.device_id.app_error", - "translation": "Invalid Device Id" - }, - { - "id": "model.notification_registry.post_id.app_error", - "translation": "Invalid Post Id" - }, - { - "id": "model.notification_registry.status.app_error", - "translation": "Invalid Send Status" - }, - { - "id": "model.notification_registry.type.app_error", - "translation": "Invalid push type, must be \"clear\" or \"message\"" - }, - { - "id": "model.notification_registry.user_id.app_error", - "translation": "Invalid User Id" - }, { "id": "model.oauth.is_valid.app_id.app_error", "translation": "Invalid app id" @@ -5890,18 +5862,6 @@ "id": "store.sql_link_metadata.save.app_error", "translation": "Unable to save the link metadata" }, - { - "id": "store.sql_notification.mark_as_received.app_error", - "translation": "An error occurred marking the notification as received" - }, - { - "id": "store.sql_notification.save.app_error", - "translation": "An error occurred while saving the notification registry" - }, - { - "id": "store.sql_notification.update_status.app_error", - "translation": "An error occurred while updating the notification status" - }, { "id": "store.sql_oauth.delete.commit_transaction.app_error", "translation": "Unable to commit transaction" diff --git a/model/config.go b/model/config.go index 877c836ee4..6dec9ddcf2 100644 --- a/model/config.go +++ b/model/config.go @@ -912,6 +912,46 @@ func (s *LogSettings) SetDefaults() { } } +type NotificationLogSettings struct { + EnableConsole *bool `restricted:"true"` + ConsoleLevel *string `restricted:"true"` + ConsoleJson *bool `restricted:"true"` + EnableFile *bool `restricted:"true"` + FileLevel *string `restricted:"true"` + FileJson *bool `restricted:"true"` + FileLocation *string `restricted:"true"` +} + +func (s *NotificationLogSettings) SetDefaults() { + if s.EnableConsole == nil { + s.EnableConsole = NewBool(true) + } + + if s.ConsoleLevel == nil { + s.ConsoleLevel = NewString("DEBUG") + } + + if s.EnableFile == nil { + s.EnableFile = NewBool(true) + } + + if s.FileLevel == nil { + s.FileLevel = NewString("INFO") + } + + if s.FileLocation == nil { + s.FileLocation = NewString("") + } + + if s.ConsoleJson == nil { + s.ConsoleJson = NewBool(true) + } + + if s.FileJson == nil { + s.FileJson = NewBool(true) + } +} + type PasswordSettings struct { MinimumLength *int Lowercase *bool @@ -2241,38 +2281,39 @@ func (ips *ImageProxySettings) SetDefaults(ss ServiceSettings) { type ConfigFunc func() *Config type Config struct { - ServiceSettings ServiceSettings - TeamSettings TeamSettings - ClientRequirements ClientRequirements - SqlSettings SqlSettings - LogSettings LogSettings - PasswordSettings PasswordSettings - FileSettings FileSettings - EmailSettings EmailSettings - RateLimitSettings RateLimitSettings - PrivacySettings PrivacySettings - SupportSettings SupportSettings - AnnouncementSettings AnnouncementSettings - ThemeSettings ThemeSettings - GitLabSettings SSOSettings - GoogleSettings SSOSettings - Office365Settings SSOSettings - LdapSettings LdapSettings - ComplianceSettings ComplianceSettings - LocalizationSettings LocalizationSettings - SamlSettings SamlSettings - NativeAppSettings NativeAppSettings - ClusterSettings ClusterSettings - MetricsSettings MetricsSettings - ExperimentalSettings ExperimentalSettings - AnalyticsSettings AnalyticsSettings - ElasticsearchSettings ElasticsearchSettings - DataRetentionSettings DataRetentionSettings - MessageExportSettings MessageExportSettings - JobSettings JobSettings - PluginSettings PluginSettings - DisplaySettings DisplaySettings - ImageProxySettings ImageProxySettings + ServiceSettings ServiceSettings + TeamSettings TeamSettings + ClientRequirements ClientRequirements + SqlSettings SqlSettings + LogSettings LogSettings + NotificationLogSettings NotificationLogSettings + PasswordSettings PasswordSettings + FileSettings FileSettings + EmailSettings EmailSettings + RateLimitSettings RateLimitSettings + PrivacySettings PrivacySettings + SupportSettings SupportSettings + AnnouncementSettings AnnouncementSettings + ThemeSettings ThemeSettings + GitLabSettings SSOSettings + GoogleSettings SSOSettings + Office365Settings SSOSettings + LdapSettings LdapSettings + ComplianceSettings ComplianceSettings + LocalizationSettings LocalizationSettings + SamlSettings SamlSettings + NativeAppSettings NativeAppSettings + ClusterSettings ClusterSettings + MetricsSettings MetricsSettings + ExperimentalSettings ExperimentalSettings + AnalyticsSettings AnalyticsSettings + ElasticsearchSettings ElasticsearchSettings + DataRetentionSettings DataRetentionSettings + MessageExportSettings MessageExportSettings + JobSettings JobSettings + PluginSettings PluginSettings + DisplaySettings DisplaySettings + ImageProxySettings ImageProxySettings } func (o *Config) Clone() *Config { @@ -2344,6 +2385,7 @@ func (o *Config) SetDefaults() { o.DataRetentionSettings.SetDefaults() o.RateLimitSettings.SetDefaults() o.LogSettings.SetDefaults() + o.NotificationLogSettings.SetDefaults() o.JobSettings.SetDefaults() o.MessageExportSettings.SetDefaults() o.DisplaySettings.SetDefaults() diff --git a/model/push_notification.go b/model/push_notification.go index e375fef40a..03b49bc897 100644 --- a/model/push_notification.go +++ b/model/push_notification.go @@ -6,7 +6,6 @@ package model import ( "encoding/json" "io" - "net/http" "strings" ) @@ -26,22 +25,12 @@ const ( MHPNS = "https://push.mattermost.com" + PUSH_SEND_PREPARE = "Prepared to send" PUSH_SEND_SUCCESS = "Successful" - PUSH_SEND_ERROR = "Error" PUSH_NOT_SENT = "Not Sent due to preferences" + PUSH_RECEIVED = "Received by device" ) -type NotificationRegistry struct { - AckId string - CreateAt int64 - UserId string - DeviceId string - PostId string - SendStatus string - Type string - ReceiveAt int64 -} - type PushNotificationAck struct { Id string `json:"id"` ClientReceivedAt int64 `json:"received_at"` @@ -104,44 +93,3 @@ func (ack *PushNotificationAck) ToJson() string { b, _ := json.Marshal(ack) return string(b) } - -func (o *NotificationRegistry) IsValid() *AppError { - - if len(o.AckId) != 26 { - return NewAppError("NotificationRegistry.IsValid", "model.notification_registry.ack_id.app_error", nil, "", http.StatusBadRequest) - } - - if o.CreateAt == 0 { - return NewAppError("NotificationRegistry.IsValid", "model.notification_registry.create_at.app_error", nil, "AckId="+o.AckId, http.StatusBadRequest) - } - - if len(o.UserId) != 26 { - return NewAppError("NotificationRegistry.IsValid", "model.notification_registry.user_id.app_error", nil, "", http.StatusBadRequest) - } - - if len(o.PostId) > 0 && len(o.PostId) != 26 { - return NewAppError("NotificationRegistry.IsValid", "model.notification_registry.post_id.app_error", nil, "", http.StatusBadRequest) - } - - if len(o.DeviceId) > 512 { - return NewAppError("NotificationRegistry.IsValid", "model.notification_registry.device_id.app_error", nil, "", http.StatusBadRequest) - } - - if len(o.SendStatus) > 4096 { - return NewAppError("NotificationRegistry.IsValid", "model.notification_registry.status.app_error", nil, "", http.StatusBadRequest) - } - - if o.Type != PUSH_TYPE_CLEAR && o.Type != PUSH_TYPE_MESSAGE { - return NewAppError("NotificationRegistry.IsValid", "model.notification_registry.type.app_error", nil, "", http.StatusBadRequest) - } - - return nil -} - -func (o *NotificationRegistry) PreSave() { - if o.AckId == "" { - o.AckId = NewId() - } - - o.CreateAt = GetMillis() -} diff --git a/store/layered_store.go b/store/layered_store.go index 046aaa463f..be50ba5894 100644 --- a/store/layered_store.go +++ b/store/layered_store.go @@ -195,10 +195,6 @@ func (s *LayeredStore) LinkMetadata() LinkMetadataStore { return s.DatabaseLayer.LinkMetadata() } -func (s *LayeredStore) NotificationRegistry() NotificationRegistryStore { - return s.DatabaseLayer.NotificationRegistry() -} - func (s *LayeredStore) MarkSystemRanUnitTests() { s.DatabaseLayer.MarkSystemRanUnitTests() } diff --git a/store/sqlstore/notification_registry_store.go b/store/sqlstore/notification_registry_store.go deleted file mode 100644 index 8384d269e4..0000000000 --- a/store/sqlstore/notification_registry_store.go +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See License.txt for license information. - -package sqlstore - -import ( - "net/http" - - "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/store" -) - -type SqlNotificationRegistryStore struct { - SqlStore -} - -func NewSqlNotificationRegistryStore(sqlStore SqlStore) store.NotificationRegistryStore { - s := &SqlNotificationRegistryStore{sqlStore} - - for _, db := range sqlStore.GetAllConns() { - table := db.AddTableWithName(model.NotificationRegistry{}, "NotificationRegistry").SetKeys(false, "AckId") - table.ColMap("AckId").SetMaxSize(26) - table.ColMap("UserId").SetMaxSize(26) - table.ColMap("PostId").SetMaxSize(26) - table.ColMap("DeviceId").SetMaxSize(512) - table.ColMap("Type").SetMaxSize(26) - table.ColMap("SendStatus").SetMaxSize(4096) - } - - return s -} - -func (s *SqlNotificationRegistryStore) CreateIndexesIfNotExists() { - s.CreateIndexIfNotExists("idx_notification_ack_id", "NotificationRegistry", "AckId") - s.CreateIndexIfNotExists("idx_notification_create_at", "NotificationRegistry", "CreateAt") - s.CreateIndexIfNotExists("idx_notification_receive_at", "NotificationRegistry", "ReceiveAt") - s.CreateIndexIfNotExists("idx_notification_post_id", "NotificationRegistry", "PostId") - s.CreateIndexIfNotExists("idx_notification_user_id", "NotificationRegistry", "UserId") - s.CreateIndexIfNotExists("idx_notification_type", "NotificationRegistry", "Type") -} - -func (s *SqlNotificationRegistryStore) Save(notification *model.NotificationRegistry) (*model.NotificationRegistry, *model.AppError) { - notification.PreSave() - - appErr := notification.IsValid() - if appErr != nil { - return nil, appErr - } - - err := s.GetMaster().Insert(notification) - if err != nil { - appErr = model.NewAppError("SqlNotificationRegistryStore.Save", "store.sql_notification.save.app_error", nil, "id="+notification.AckId+", "+err.Error(), http.StatusInternalServerError) - return nil, appErr - } - - return notification, nil -} - -func (s *SqlNotificationRegistryStore) MarkAsReceived(ackId string, time int64) *model.AppError { - result, err := s.GetMaster().Exec("UPDATE NotificationRegistry SET ReceiveAt = :ReceiveAt WHERE AckId = :AckId AND ReceiveAt = 0", map[string]interface{}{"AckId": ackId, "ReceiveAt": time}) - if err != nil { - return model.NewAppError("SqlNotificationRegistryStore.Save", "store.sql_notification.mark_as_received.app_error", nil, "id="+ackId+", "+err.Error(), http.StatusInternalServerError) - } - - affected, err := result.RowsAffected() - if err != nil { - return model.NewAppError("SqlNotificationRegistryStore.Save", "store.sql_notification.mark_as_received.app_error", nil, "id="+ackId+", "+err.Error(), http.StatusInternalServerError) - } else if affected != 1 { - return model.NewAppError("SqlNotificationRegistryStore.Save", "store.sql_notification.mark_as_received.app_error", nil, "id="+ackId+", Message already received", http.StatusInternalServerError) - } - - return nil -} - -func (s *SqlNotificationRegistryStore) UpdateSendStatus(ackId, status string) *model.AppError { - _, err := s.GetMaster().Exec("UPDATE NotificationRegistry SET SendStatus = :Status WHERE AckId = :AckId", map[string]interface{}{"AckId": ackId, "Status": status}) - if err != nil { - return model.NewAppError("SqlNotificationRegistryStore.Save", "store.sql_notification.update_status.app_error", nil, "id="+ackId+", "+err.Error(), http.StatusInternalServerError) - } - return nil -} diff --git a/store/sqlstore/notification_registry_store_test.go b/store/sqlstore/notification_registry_store_test.go deleted file mode 100644 index 56306444f9..0000000000 --- a/store/sqlstore/notification_registry_store_test.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See License.txt for license information. - -package sqlstore - -import ( - "testing" - - "github.com/mattermost/mattermost-server/store/storetest" -) - -func TestNotificationRegistryStore(t *testing.T) { - StoreTest(t, storetest.TestNotificationRegistryStore) -} diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index 695aa9f064..76b38627fb 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -99,5 +99,4 @@ type SqlStore interface { UserTermsOfService() store.UserTermsOfServiceStore LinkMetadata() store.LinkMetadataStore getQueryBuilder() sq.StatementBuilderType - NotificationRegistry() store.NotificationRegistryStore } diff --git a/store/sqlstore/supplier.go b/store/sqlstore/supplier.go index 09aa39851a..35b576dd7d 100644 --- a/store/sqlstore/supplier.go +++ b/store/sqlstore/supplier.go @@ -99,7 +99,6 @@ type SqlSupplierOldStores struct { group store.GroupStore UserTermsOfService store.UserTermsOfServiceStore linkMetadata store.LinkMetadataStore - notificationRegistry store.NotificationRegistryStore } type SqlSupplier struct { @@ -152,7 +151,6 @@ func NewSqlSupplier(settings model.SqlSettings, metrics einterfaces.MetricsInter supplier.oldStores.TermsOfService = NewSqlTermsOfServiceStore(supplier, metrics) supplier.oldStores.UserTermsOfService = NewSqlUserTermsOfServiceStore(supplier) supplier.oldStores.linkMetadata = NewSqlLinkMetadataStore(supplier) - supplier.oldStores.notificationRegistry = NewSqlNotificationRegistryStore(supplier) initSqlSupplierReactions(supplier) initSqlSupplierRoles(supplier) @@ -1055,10 +1053,6 @@ func (ss *SqlSupplier) LinkMetadata() store.LinkMetadataStore { return ss.oldStores.linkMetadata } -func (ss *SqlSupplier) NotificationRegistry() store.NotificationRegistryStore { - return ss.oldStores.notificationRegistry -} - func (ss *SqlSupplier) DropAllTables() { ss.master.TruncateTables() } diff --git a/store/store.go b/store/store.go index cfb4a5934e..b7148ee8ff 100644 --- a/store/store.go +++ b/store/store.go @@ -78,7 +78,6 @@ type Store interface { TotalMasterDbConnections() int TotalReadDbConnections() int TotalSearchDbConnections() int - NotificationRegistry() NotificationRegistryStore } type TeamStore interface { @@ -605,9 +604,3 @@ type LinkMetadataStore interface { Save(linkMetadata *model.LinkMetadata) StoreChannel Get(url string, timestamp int64) StoreChannel } - -type NotificationRegistryStore interface { - Save(notification *model.NotificationRegistry) (*model.NotificationRegistry, *model.AppError) - MarkAsReceived(ackId string, time int64) *model.AppError - UpdateSendStatus(ackId, status string) *model.AppError -} diff --git a/store/storetest/mocks/LayeredStoreDatabaseLayer.go b/store/storetest/mocks/LayeredStoreDatabaseLayer.go index e61bc05a22..236802106e 100644 --- a/store/storetest/mocks/LayeredStoreDatabaseLayer.go +++ b/store/storetest/mocks/LayeredStoreDatabaseLayer.go @@ -780,22 +780,6 @@ func (_m *LayeredStoreDatabaseLayer) Next() store.LayeredStoreSupplier { return r0 } -// NotificationRegistry provides a mock function with given fields: -func (_m *LayeredStoreDatabaseLayer) NotificationRegistry() store.NotificationRegistryStore { - ret := _m.Called() - - var r0 store.NotificationRegistryStore - if rf, ok := ret.Get(0).(func() store.NotificationRegistryStore); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(store.NotificationRegistryStore) - } - } - - return r0 -} - // OAuth provides a mock function with given fields: func (_m *LayeredStoreDatabaseLayer) OAuth() store.OAuthStore { ret := _m.Called() diff --git a/store/storetest/mocks/NotificationRegistryStore.go b/store/storetest/mocks/NotificationRegistryStore.go deleted file mode 100644 index bce923c807..0000000000 --- a/store/storetest/mocks/NotificationRegistryStore.go +++ /dev/null @@ -1,70 +0,0 @@ -// Code generated by mockery v1.0.0. DO NOT EDIT. - -// Regenerate this file using `make store-mocks`. - -package mocks - -import mock "github.com/stretchr/testify/mock" -import model "github.com/mattermost/mattermost-server/model" - -// NotificationRegistryStore is an autogenerated mock type for the NotificationRegistryStore type -type NotificationRegistryStore struct { - mock.Mock -} - -// MarkAsReceived provides a mock function with given fields: ackId, time -func (_m *NotificationRegistryStore) MarkAsReceived(ackId string, time int64) *model.AppError { - ret := _m.Called(ackId, time) - - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(string, int64) *model.AppError); ok { - r0 = rf(ackId, time) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } - } - - return r0 -} - -// Save provides a mock function with given fields: notification -func (_m *NotificationRegistryStore) Save(notification *model.NotificationRegistry) (*model.NotificationRegistry, *model.AppError) { - ret := _m.Called(notification) - - var r0 *model.NotificationRegistry - if rf, ok := ret.Get(0).(func(*model.NotificationRegistry) *model.NotificationRegistry); ok { - r0 = rf(notification) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.NotificationRegistry) - } - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.NotificationRegistry) *model.AppError); ok { - r1 = rf(notification) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - -// UpdateSendStatus provides a mock function with given fields: ackId, status -func (_m *NotificationRegistryStore) UpdateSendStatus(ackId string, status string) *model.AppError { - ret := _m.Called(ackId, status) - - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(string, string) *model.AppError); ok { - r0 = rf(ackId, status) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } - } - - return r0 -} diff --git a/store/storetest/mocks/SqlStore.go b/store/storetest/mocks/SqlStore.go index d16767b493..3c7c29c3b1 100644 --- a/store/storetest/mocks/SqlStore.go +++ b/store/storetest/mocks/SqlStore.go @@ -482,22 +482,6 @@ func (_m *SqlStore) MarkSystemRanUnitTests() { _m.Called() } -// NotificationRegistry provides a mock function with given fields: -func (_m *SqlStore) NotificationRegistry() store.NotificationRegistryStore { - ret := _m.Called() - - var r0 store.NotificationRegistryStore - if rf, ok := ret.Get(0).(func() store.NotificationRegistryStore); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(store.NotificationRegistryStore) - } - } - - return r0 -} - // OAuth provides a mock function with given fields: func (_m *SqlStore) OAuth() store.OAuthStore { ret := _m.Called() diff --git a/store/storetest/mocks/Store.go b/store/storetest/mocks/Store.go index ce95bd94a7..21746dd211 100644 --- a/store/storetest/mocks/Store.go +++ b/store/storetest/mocks/Store.go @@ -256,22 +256,6 @@ func (_m *Store) MarkSystemRanUnitTests() { _m.Called() } -// NotificationRegistry provides a mock function with given fields: -func (_m *Store) NotificationRegistry() store.NotificationRegistryStore { - ret := _m.Called() - - var r0 store.NotificationRegistryStore - if rf, ok := ret.Get(0).(func() store.NotificationRegistryStore); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(store.NotificationRegistryStore) - } - } - - return r0 -} - // OAuth provides a mock function with given fields: func (_m *Store) OAuth() store.OAuthStore { ret := _m.Called() diff --git a/store/storetest/notification_registry_store.go b/store/storetest/notification_registry_store.go deleted file mode 100644 index d6574c8439..0000000000 --- a/store/storetest/notification_registry_store.go +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See License.txt for license information. - -package storetest - -import ( - "testing" - - "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/store" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestNotificationRegistryStore(t *testing.T, ss store.Store) { - t.Run("Save", func(t *testing.T) { testNotificationRegistryStoreSave(t, ss) }) - t.Run("MarkAsReceived", func(t *testing.T) { testNotificationRegistryStoreMarkAsReceived(t, ss) }) - t.Run("UpdateSendStatus", func(t *testing.T) { testNotificationRegistryStoreUpdateSendStatus(t, ss) }) -} - -func testNotificationRegistryStoreSave(t *testing.T, ss store.Store) { - t.Run("should save notification registry", func(t *testing.T) { - notificationRegistry := &model.NotificationRegistry{ - AckId: model.NewId(), - DeviceId: "apple_or_android_device_identifier", - PostId: model.NewId(), - UserId: model.NewId(), - Type: model.PUSH_TYPE_MESSAGE, - } - - result, err := ss.NotificationRegistry().Save(notificationRegistry) - - require.Nil(t, err) - require.IsType(t, notificationRegistry, result) - assert.NotZero(t, result.CreateAt) - }) - - t.Run("should save notification registry without AckId", func(t *testing.T) { - notificationRegistry := &model.NotificationRegistry{ - DeviceId: "apple_or_android_device_identifier", - PostId: model.NewId(), - UserId: model.NewId(), - Type: model.PUSH_TYPE_MESSAGE, - } - - result, err := ss.NotificationRegistry().Save(notificationRegistry) - - require.Nil(t, err) - require.IsType(t, notificationRegistry, result) - assert.NotEmpty(t, result.AckId) - assert.NotZero(t, result.CreateAt) - }) - - t.Run("should fail to save invalid notification registry", func(t *testing.T) { - notificationRegistry := &model.NotificationRegistry{ - DeviceId: "apple_or_android_device_identifier", - PostId: model.NewId(), - UserId: model.NewId(), - Type: "garbage", - } - - _, err := ss.NotificationRegistry().Save(notificationRegistry) - - assert.NotNil(t, err) - }) -} - -func testNotificationRegistryStoreMarkAsReceived(t *testing.T, ss store.Store) { - notificationRegistry := &model.NotificationRegistry{ - DeviceId: "apple_or_android_device_identifier", - PostId: model.NewId(), - UserId: model.NewId(), - Type: model.PUSH_TYPE_MESSAGE, - } - - t.Run("should update when the notification was received", func(t *testing.T) { - result, err := ss.NotificationRegistry().Save(notificationRegistry) - if err != nil { - t.Fatal("Notification should have been saved") - } - - err = ss.NotificationRegistry().MarkAsReceived(result.AckId, model.GetMillis()) - - assert.Nil(t, err) - }) -} - -func testNotificationRegistryStoreUpdateSendStatus(t *testing.T, ss store.Store) { - notificationRegistry := &model.NotificationRegistry{ - DeviceId: "apple_or_android_device_identifier", - UserId: model.NewId(), - Type: model.PUSH_TYPE_CLEAR, - } - - t.Run("should update when the notification send status", func(t *testing.T) { - result, err := ss.NotificationRegistry().Save(notificationRegistry) - if err != nil { - t.Fatal("Notification should have been saved") - } - - err = ss.NotificationRegistry().UpdateSendStatus(result.AckId, "Some Status") - - assert.Nil(t, err) - }) -} diff --git a/store/storetest/store.go b/store/storetest/store.go index 572641528b..6c29491219 100644 --- a/store/storetest/store.go +++ b/store/storetest/store.go @@ -50,28 +50,24 @@ type Store struct { GroupStore mocks.GroupStore UserTermsOfServiceStore mocks.UserTermsOfServiceStore LinkMetadataStore mocks.LinkMetadataStore - NotificationRegistryStore mocks.NotificationRegistryStore } -func (s *Store) Team() store.TeamStore { return &s.TeamStore } -func (s *Store) Channel() store.ChannelStore { return &s.ChannelStore } -func (s *Store) Post() store.PostStore { return &s.PostStore } -func (s *Store) User() store.UserStore { return &s.UserStore } -func (s *Store) Bot() store.BotStore { return &s.BotStore } -func (s *Store) Audit() store.AuditStore { return &s.AuditStore } -func (s *Store) ClusterDiscovery() store.ClusterDiscoveryStore { return &s.ClusterDiscoveryStore } -func (s *Store) Compliance() store.ComplianceStore { return &s.ComplianceStore } -func (s *Store) Session() store.SessionStore { return &s.SessionStore } -func (s *Store) OAuth() store.OAuthStore { return &s.OAuthStore } -func (s *Store) System() store.SystemStore { return &s.SystemStore } -func (s *Store) Webhook() store.WebhookStore { return &s.WebhookStore } -func (s *Store) Command() store.CommandStore { return &s.CommandStore } -func (s *Store) CommandWebhook() store.CommandWebhookStore { return &s.CommandWebhookStore } -func (s *Store) Preference() store.PreferenceStore { return &s.PreferenceStore } -func (s *Store) License() store.LicenseStore { return &s.LicenseStore } -func (s *Store) NotificationRegistry() store.NotificationRegistryStore { - return &s.NotificationRegistryStore -} +func (s *Store) Team() store.TeamStore { return &s.TeamStore } +func (s *Store) Channel() store.ChannelStore { return &s.ChannelStore } +func (s *Store) Post() store.PostStore { return &s.PostStore } +func (s *Store) User() store.UserStore { return &s.UserStore } +func (s *Store) Bot() store.BotStore { return &s.BotStore } +func (s *Store) Audit() store.AuditStore { return &s.AuditStore } +func (s *Store) ClusterDiscovery() store.ClusterDiscoveryStore { return &s.ClusterDiscoveryStore } +func (s *Store) Compliance() store.ComplianceStore { return &s.ComplianceStore } +func (s *Store) Session() store.SessionStore { return &s.SessionStore } +func (s *Store) OAuth() store.OAuthStore { return &s.OAuthStore } +func (s *Store) System() store.SystemStore { return &s.SystemStore } +func (s *Store) Webhook() store.WebhookStore { return &s.WebhookStore } +func (s *Store) Command() store.CommandStore { return &s.CommandStore } +func (s *Store) CommandWebhook() store.CommandWebhookStore { return &s.CommandWebhookStore } +func (s *Store) Preference() store.PreferenceStore { return &s.PreferenceStore } +func (s *Store) License() store.LicenseStore { return &s.LicenseStore } func (s *Store) Token() store.TokenStore { return &s.TokenStore } func (s *Store) Emoji() store.EmojiStore { return &s.EmojiStore } func (s *Store) Status() store.StatusStore { return &s.StatusStore } @@ -116,7 +112,6 @@ func (s *Store) AssertExpectations(t mock.TestingT) bool { &s.CommandWebhookStore, &s.PreferenceStore, &s.LicenseStore, - &s.NotificationRegistryStore, &s.TokenStore, &s.EmojiStore, &s.StatusStore, diff --git a/utils/logger.go b/utils/logger.go index 9b071b0841..49b560b0d2 100644 --- a/utils/logger.go +++ b/utils/logger.go @@ -13,11 +13,14 @@ import ( ) const ( - LOG_ROTATE_SIZE = 10000 - LOG_FILENAME = "mattermost.log" + LOG_ROTATE_SIZE = 10000 + LOG_FILENAME = "mattermost.log" + LOG_NOTIFICATION_FILENAME = "notifications.log" ) -func MloggerConfigFromLoggerConfig(s *model.LogSettings) *mlog.LoggerConfiguration { +type fileLocationFunc func(string) string + +func MloggerConfigFromLoggerConfig(s *model.LogSettings, getFileFunc fileLocationFunc) *mlog.LoggerConfiguration { return &mlog.LoggerConfiguration{ EnableConsole: *s.EnableConsole, ConsoleJson: *s.ConsoleJson, @@ -25,7 +28,7 @@ func MloggerConfigFromLoggerConfig(s *model.LogSettings) *mlog.LoggerConfigurati EnableFile: *s.EnableFile, FileJson: *s.FileJson, FileLevel: strings.ToLower(*s.FileLevel), - FileLocation: GetLogFileLocation(*s.FileLocation), + FileLocation: getFileFunc(*s.FileLocation), } } @@ -37,6 +40,26 @@ func GetLogFileLocation(fileLocation string) string { return filepath.Join(fileLocation, LOG_FILENAME) } +func GetNotificationsLogFileLocation(fileLocation string) string { + if fileLocation == "" { + fileLocation, _ = fileutils.FindDir("logs") + } + + return filepath.Join(fileLocation, LOG_NOTIFICATION_FILENAME) +} + +func GetLogSettingsFromNotificationsLogSettings(notificationLogSettings *model.NotificationLogSettings) *model.LogSettings { + return &model.LogSettings{ + ConsoleJson: notificationLogSettings.ConsoleJson, + ConsoleLevel: notificationLogSettings.ConsoleLevel, + EnableConsole: notificationLogSettings.EnableConsole, + EnableFile: notificationLogSettings.EnableFile, + FileJson: notificationLogSettings.FileJson, + FileLevel: notificationLogSettings.FileLevel, + FileLocation: notificationLogSettings.FileLocation, + } +} + // DON'T USE THIS Modify the level on the app logger func DisableDebugLogForTest() { mlog.GloballyDisableDebugLogForTest()