From af275fe9242303581192258ef4f6457fa45a58e4 Mon Sep 17 00:00:00 2001 From: Harshil Sharma Date: Wed, 26 Sep 2018 20:49:22 +0000 Subject: #MM-12130 changes for custom service terms (#9450) * #MM-12130 changes for custom service terms * Fixed styling * Added getServiceTerms API * removed unnecessary panic * removed custom service terms text from flat config * reverted user sql store as those changes are no longer needed * added tests * Updated a config key to be more standard * Added copyright info * Loading service terms only if the feature is enabled * Loading service terms only if the feature is enabled * removed unused index * added createservice termns API * made a param to bool instead of string * added createservice termns API * review fixes * fixed styling * Minor refactoring * removed saveConfig and loadConfig magic * added empty service terms text check to createServiceTerms API * refactoed some urls to be terms_of_service instead of service_terms * removed check for support settings * changed URLs in tests * removed unused code * fixed a bug * added service termd id in conif * fixed a test * review fixes * minor fixes * Fixed TestCreateServiceTerms --- store/layered_store.go | 4 + store/sqlstore/service_terms_store.go | 143 +++++++++++++++++++++ store/sqlstore/service_terms_store_test.go | 10 ++ store/sqlstore/store.go | 1 + store/sqlstore/supplier.go | 7 + store/sqlstore/upgrade.go | 2 +- store/store.go | 7 + store/storetest/mocks/LayeredStoreDatabaseLayer.go | 16 +++ store/storetest/mocks/ServiceTermsStore.go | 62 +++++++++ store/storetest/mocks/SqlStore.go | 16 +++ store/storetest/mocks/Store.go | 16 +++ store/storetest/service_terms_store.go | 82 ++++++++++++ store/storetest/store.go | 2 + store/storetest/user_store.go | 1 + 14 files changed, 368 insertions(+), 1 deletion(-) create mode 100644 store/sqlstore/service_terms_store.go create mode 100644 store/sqlstore/service_terms_store_test.go create mode 100644 store/storetest/mocks/ServiceTermsStore.go create mode 100644 store/storetest/service_terms_store.go (limited to 'store') diff --git a/store/layered_store.go b/store/layered_store.go index 6587868d6..f5f1f9b54 100644 --- a/store/layered_store.go +++ b/store/layered_store.go @@ -169,6 +169,10 @@ func (s *LayeredStore) Role() RoleStore { return s.RoleStore } +func (s *LayeredStore) ServiceTerms() ServiceTermsStore { + return s.DatabaseLayer.ServiceTerms() +} + func (s *LayeredStore) Scheme() SchemeStore { return s.SchemeStore } diff --git a/store/sqlstore/service_terms_store.go b/store/sqlstore/service_terms_store.go new file mode 100644 index 000000000..43a1189f6 --- /dev/null +++ b/store/sqlstore/service_terms_store.go @@ -0,0 +1,143 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package sqlstore + +import ( + "database/sql" + "github.com/mattermost/mattermost-server/einterfaces" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" + "github.com/mattermost/mattermost-server/utils" + "net/http" +) + +type SqlServiceTermsStore struct { + SqlStore + metrics einterfaces.MetricsInterface +} + +var serviceTermsCache = utils.NewLru(model.SERVICE_TERMS_CACHE_SIZE) + +const serviceTermsCacheName = "ServiceTerms" + +func NewSqlTermStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface) store.ServiceTermsStore { + s := SqlServiceTermsStore{sqlStore, metrics} + + for _, db := range sqlStore.GetAllConns() { + table := db.AddTableWithName(model.ServiceTerms{}, "ServiceTerms").SetKeys(false, "Id") + table.ColMap("Id").SetMaxSize(26) + table.ColMap("UserId").SetMaxSize(26) + table.ColMap("Text").SetMaxSize(model.POST_MESSAGE_MAX_BYTES_V2) + } + + return s +} + +func (s SqlServiceTermsStore) CreateIndexesIfNotExists() { +} + +func (s SqlServiceTermsStore) Save(serviceTerms *model.ServiceTerms) store.StoreChannel { + return store.Do(func(result *store.StoreResult) { + if len(serviceTerms.Id) > 0 { + result.Err = model.NewAppError( + "SqlServiceTermsStore.Save", + "store.sql_service_terms_store.save.existing.app_error", + nil, + "id="+serviceTerms.Id, http.StatusBadRequest, + ) + return + } + + serviceTerms.PreSave() + + if result.Err = serviceTerms.IsValid(); result.Err != nil { + return + } + + if err := s.GetMaster().Insert(serviceTerms); err != nil { + result.Err = model.NewAppError( + "SqlServiceTermsStore.Save", + "store.sql_service_terms.save.app_error", + nil, + "service_term_id="+serviceTerms.Id+",err="+err.Error(), + http.StatusInternalServerError, + ) + } + + result.Data = serviceTerms + + serviceTermsCache.AddWithDefaultExpires(serviceTerms.Id, serviceTerms) + }) +} + +func (s SqlServiceTermsStore) GetLatest(allowFromCache bool) store.StoreChannel { + return store.Do(func(result *store.StoreResult) { + if allowFromCache { + if serviceTermsCache.Len() == 0 { + if s.metrics != nil { + s.metrics.IncrementMemCacheMissCounter(serviceTermsCacheName) + } + } else { + if cacheItem, ok := serviceTermsCache.Get(serviceTermsCache.Keys()[0]); ok { + if s.metrics != nil { + s.metrics.IncrementMemCacheHitCounter(serviceTermsCacheName) + } + + result.Data = cacheItem.(*model.ServiceTerms) + return + } else if s.metrics != nil { + s.metrics.IncrementMemCacheMissCounter(serviceTermsCacheName) + } + } + } + + var serviceTerms *model.ServiceTerms + + err := s.GetReplica().SelectOne(&serviceTerms, "SELECT * FROM ServiceTerms ORDER BY CreateAt DESC LIMIT 1") + if err != nil { + if err == sql.ErrNoRows { + result.Err = model.NewAppError("SqlServiceTermsStore.GetLatest", "store.sql_service_terms_store.get.no_rows.app_error", nil, "err="+err.Error(), http.StatusNotFound) + } else { + result.Err = model.NewAppError("SqlServiceTermsStore.GetLatest", "store.sql_service_terms_store.get.app_error", nil, "err="+err.Error(), http.StatusInternalServerError) + } + } else { + result.Data = serviceTerms + + if allowFromCache { + serviceTermsCache.AddWithDefaultExpires(serviceTerms.Id, serviceTerms) + } + } + }) +} + +func (s SqlServiceTermsStore) Get(id string, allowFromCache bool) store.StoreChannel { + return store.Do(func(result *store.StoreResult) { + if allowFromCache { + if serviceTermsCache.Len() == 0 { + if s.metrics != nil { + s.metrics.IncrementMemCacheMissCounter(serviceTermsCacheName) + } + } else { + if cacheItem, ok := serviceTermsCache.Get(id); ok { + if s.metrics != nil { + s.metrics.IncrementMemCacheHitCounter(serviceTermsCacheName) + } + + result.Data = cacheItem.(*model.ServiceTerms) + return + } else if s.metrics != nil { + s.metrics.IncrementMemCacheMissCounter(serviceTermsCacheName) + } + } + } + + if obj, err := s.GetReplica().Get(model.ServiceTerms{}, id); err != nil { + result.Err = model.NewAppError("SqlServiceTermsStore.Get", "store.sql_service_terms_store.get.app_error", nil, "err="+err.Error(), http.StatusInternalServerError) + } else if obj == nil { + result.Err = model.NewAppError("SqlServiceTermsStore.GetLatest", "store.sql_service_terms_store.get.no_rows.app_error", nil, "", http.StatusNotFound) + } else { + result.Data = obj.(*model.ServiceTerms) + } + }) +} diff --git a/store/sqlstore/service_terms_store_test.go b/store/sqlstore/service_terms_store_test.go new file mode 100644 index 000000000..030d0d7ae --- /dev/null +++ b/store/sqlstore/service_terms_store_test.go @@ -0,0 +1,10 @@ +package sqlstore + +import ( + "github.com/mattermost/mattermost-server/store/storetest" + "testing" +) + +func TestServiceTermsStore(t *testing.T) { + StoreTest(t, storetest.TestServiceTermsStore) +} diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index df912028b..b6f0fa84e 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -93,4 +93,5 @@ type SqlStore interface { UserAccessToken() store.UserAccessTokenStore Role() store.RoleStore Scheme() store.SchemeStore + ServiceTerms() store.ServiceTermsStore } diff --git a/store/sqlstore/supplier.go b/store/sqlstore/supplier.go index 11216dd25..62c1102ca 100644 --- a/store/sqlstore/supplier.go +++ b/store/sqlstore/supplier.go @@ -92,6 +92,7 @@ type SqlSupplierOldStores struct { channelMemberHistory store.ChannelMemberHistoryStore role store.RoleStore scheme store.SchemeStore + serviceTerms store.ServiceTermsStore } type SqlSupplier struct { @@ -145,6 +146,7 @@ func NewSqlSupplier(settings model.SqlSettings, metrics einterfaces.MetricsInter supplier.oldStores.userAccessToken = NewSqlUserAccessTokenStore(supplier) supplier.oldStores.channelMemberHistory = NewSqlChannelMemberHistoryStore(supplier) supplier.oldStores.plugin = NewSqlPluginStore(supplier) + supplier.oldStores.serviceTerms = NewSqlTermStore(supplier, metrics) initSqlSupplierReactions(supplier) initSqlSupplierRoles(supplier) @@ -180,6 +182,7 @@ func NewSqlSupplier(settings model.SqlSettings, metrics einterfaces.MetricsInter supplier.oldStores.job.(*SqlJobStore).CreateIndexesIfNotExists() supplier.oldStores.userAccessToken.(*SqlUserAccessTokenStore).CreateIndexesIfNotExists() supplier.oldStores.plugin.(*SqlPluginStore).CreateIndexesIfNotExists() + supplier.oldStores.serviceTerms.(SqlServiceTermsStore).CreateIndexesIfNotExists() supplier.oldStores.preference.(*SqlPreferenceStore).DeleteUnusedFeatures() @@ -961,6 +964,10 @@ func (ss *SqlSupplier) Role() store.RoleStore { return ss.oldStores.role } +func (ss *SqlSupplier) ServiceTerms() store.ServiceTermsStore { + return ss.oldStores.serviceTerms +} + func (ss *SqlSupplier) Scheme() store.SchemeStore { return ss.oldStores.scheme } diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index a8be96172..05e10d266 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -496,11 +496,11 @@ func UpgradeDatabaseToVersion54(sqlStore SqlStore) { // if shouldPerformUpgrade(sqlStore, VERSION_5_3_0, VERSION_5_4_0) { sqlStore.AlterColumnTypeIfExists("OutgoingWebhooks", "Description", "varchar(500)", "varchar(500)") sqlStore.AlterColumnTypeIfExists("IncomingWebhooks", "Description", "varchar(500)", "varchar(500)") - if err := sqlStore.Channel().MigratePublicChannels(); err != nil { mlog.Critical("Failed to migrate PublicChannels table", mlog.Err(err)) time.Sleep(time.Second) os.Exit(EXIT_GENERIC_FAILURE) } + sqlStore.CreateColumnIfNotExists("Users", "AcceptedServiceTermsId", "varchar(64)", "varchar(64)", "") // saveSchemaVersion(sqlStore, VERSION_5_4_0) } diff --git a/store/store.go b/store/store.go index 608d501c0..f375bfb0f 100644 --- a/store/store.go +++ b/store/store.go @@ -65,6 +65,7 @@ type Store interface { UserAccessToken() UserAccessTokenStore ChannelMemberHistory() ChannelMemberHistoryStore Plugin() PluginStore + ServiceTerms() ServiceTermsStore MarkSystemRanUnitTests() Close() LockToMaster() @@ -518,3 +519,9 @@ type SchemeStore interface { Delete(schemeId string) StoreChannel PermanentDeleteAll() StoreChannel } + +type ServiceTermsStore interface { + Save(serviceTerms *model.ServiceTerms) StoreChannel + GetLatest(allowFromCache bool) StoreChannel + Get(id string, allowFromCache bool) StoreChannel +} diff --git a/store/storetest/mocks/LayeredStoreDatabaseLayer.go b/store/storetest/mocks/LayeredStoreDatabaseLayer.go index 8e82e9494..7f653fc2f 100644 --- a/store/storetest/mocks/LayeredStoreDatabaseLayer.go +++ b/store/storetest/mocks/LayeredStoreDatabaseLayer.go @@ -729,6 +729,22 @@ func (_m *LayeredStoreDatabaseLayer) SchemeSave(ctx context.Context, scheme *mod return r0 } +// ServiceTerms provides a mock function with given fields: +func (_m *LayeredStoreDatabaseLayer) ServiceTerms() store.ServiceTermsStore { + ret := _m.Called() + + var r0 store.ServiceTermsStore + if rf, ok := ret.Get(0).(func() store.ServiceTermsStore); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.ServiceTermsStore) + } + } + + return r0 +} + // Session provides a mock function with given fields: func (_m *LayeredStoreDatabaseLayer) Session() store.SessionStore { ret := _m.Called() diff --git a/store/storetest/mocks/ServiceTermsStore.go b/store/storetest/mocks/ServiceTermsStore.go new file mode 100644 index 000000000..9115e6093 --- /dev/null +++ b/store/storetest/mocks/ServiceTermsStore.go @@ -0,0 +1,62 @@ +// 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" +import store "github.com/mattermost/mattermost-server/store" + +// ServiceTermsStore is an autogenerated mock type for the ServiceTermsStore type +type ServiceTermsStore struct { + mock.Mock +} + +// Get provides a mock function with given fields: id, allowFromCache +func (_m *ServiceTermsStore) Get(id string, allowFromCache bool) store.StoreChannel { + ret := _m.Called(id, allowFromCache) + + var r0 store.StoreChannel + if rf, ok := ret.Get(0).(func(string, bool) store.StoreChannel); ok { + r0 = rf(id, allowFromCache) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.StoreChannel) + } + } + + return r0 +} + +// GetLatest provides a mock function with given fields: allowFromCache +func (_m *ServiceTermsStore) GetLatest(allowFromCache bool) store.StoreChannel { + ret := _m.Called(allowFromCache) + + var r0 store.StoreChannel + if rf, ok := ret.Get(0).(func(bool) store.StoreChannel); ok { + r0 = rf(allowFromCache) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.StoreChannel) + } + } + + return r0 +} + +// Save provides a mock function with given fields: serviceTerms +func (_m *ServiceTermsStore) Save(serviceTerms *model.ServiceTerms) store.StoreChannel { + ret := _m.Called(serviceTerms) + + var r0 store.StoreChannel + if rf, ok := ret.Get(0).(func(*model.ServiceTerms) store.StoreChannel); ok { + r0 = rf(serviceTerms) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.StoreChannel) + } + } + + return r0 +} diff --git a/store/storetest/mocks/SqlStore.go b/store/storetest/mocks/SqlStore.go index 38cdc0a1b..c2852f3a1 100644 --- a/store/storetest/mocks/SqlStore.go +++ b/store/storetest/mocks/SqlStore.go @@ -603,6 +603,22 @@ func (_m *SqlStore) Scheme() store.SchemeStore { return r0 } +// ServiceTerms provides a mock function with given fields: +func (_m *SqlStore) ServiceTerms() store.ServiceTermsStore { + ret := _m.Called() + + var r0 store.ServiceTermsStore + if rf, ok := ret.Get(0).(func() store.ServiceTermsStore); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.ServiceTermsStore) + } + } + + return r0 +} + // Session provides a mock function with given fields: func (_m *SqlStore) Session() store.SessionStore { ret := _m.Called() diff --git a/store/storetest/mocks/Store.go b/store/storetest/mocks/Store.go index e5d0c4290..8f15650e8 100644 --- a/store/storetest/mocks/Store.go +++ b/store/storetest/mocks/Store.go @@ -320,6 +320,22 @@ func (_m *Store) Scheme() store.SchemeStore { return r0 } +// ServiceTerms provides a mock function with given fields: +func (_m *Store) ServiceTerms() store.ServiceTermsStore { + ret := _m.Called() + + var r0 store.ServiceTermsStore + if rf, ok := ret.Get(0).(func() store.ServiceTermsStore); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.ServiceTermsStore) + } + } + + return r0 +} + // Session provides a mock function with given fields: func (_m *Store) Session() store.SessionStore { ret := _m.Called() diff --git a/store/storetest/service_terms_store.go b/store/storetest/service_terms_store.go new file mode 100644 index 000000000..fcb209934 --- /dev/null +++ b/store/storetest/service_terms_store.go @@ -0,0 +1,82 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package storetest + +import ( + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestServiceTermsStore(t *testing.T, ss store.Store) { + t.Run("TestSaveServiceTerms", func(t *testing.T) { testSaveServiceTerms(t, ss) }) + t.Run("TestGetLatestServiceTerms", func(t *testing.T) { testGetLatestServiceTerms(t, ss) }) + t.Run("TestGetServiceTerms", func(t *testing.T) { testGetServiceTerms(t, ss) }) +} + +func testSaveServiceTerms(t *testing.T, ss store.Store) { + u1 := model.User{} + u1.Username = model.NewId() + u1.Email = MakeEmail() + u1.Nickname = model.NewId() + store.Must(ss.User().Save(&u1)) + + serviceTerms := &model.ServiceTerms{Text: "service terms", UserId: u1.Id} + r1 := <-ss.ServiceTerms().Save(serviceTerms) + + if r1.Err != nil { + t.Fatal(r1.Err) + } + + savedServiceTerms := r1.Data.(*model.ServiceTerms) + if len(savedServiceTerms.Id) != 26 { + t.Fatal("Id should have been populated") + } + + if savedServiceTerms.CreateAt == 0 { + t.Fatal("Create at should have been populated") + } +} + +func testGetLatestServiceTerms(t *testing.T, ss store.Store) { + u1 := model.User{} + u1.Username = model.NewId() + u1.Email = MakeEmail() + u1.Nickname = model.NewId() + store.Must(ss.User().Save(&u1)) + + serviceTerms := &model.ServiceTerms{Text: "service terms", UserId: u1.Id} + store.Must(ss.ServiceTerms().Save(serviceTerms)) + + r1 := <-ss.ServiceTerms().GetLatest(true) + if r1.Err != nil { + t.Fatal(r1.Err) + } + + fetchedServiceTerms := r1.Data.(*model.ServiceTerms) + assert.Equal(t, serviceTerms.Text, fetchedServiceTerms.Text) + assert.Equal(t, serviceTerms.UserId, fetchedServiceTerms.UserId) +} + +func testGetServiceTerms(t *testing.T, ss store.Store) { + u1 := model.User{} + u1.Username = model.NewId() + u1.Email = MakeEmail() + u1.Nickname = model.NewId() + store.Must(ss.User().Save(&u1)) + + serviceTerms := &model.ServiceTerms{Text: "service terms", UserId: u1.Id} + store.Must(ss.ServiceTerms().Save(serviceTerms)) + + r1 := <-ss.ServiceTerms().Get("an_invalid_id", true) + assert.NotNil(t, r1.Err) + assert.Nil(t, r1.Data) + + r1 = <-ss.ServiceTerms().Get(serviceTerms.Id, true) + assert.Nil(t, r1.Err) + + receivedServiceTerms := r1.Data.(*model.ServiceTerms) + assert.Equal(t, "service terms", receivedServiceTerms.Text) +} diff --git a/store/storetest/store.go b/store/storetest/store.go index e73596ec4..e7086a3a5 100644 --- a/store/storetest/store.go +++ b/store/storetest/store.go @@ -45,6 +45,7 @@ type Store struct { ChannelMemberHistoryStore mocks.ChannelMemberHistoryStore RoleStore mocks.RoleStore SchemeStore mocks.SchemeStore + ServiceTermsStore mocks.ServiceTermsStore } func (s *Store) Team() store.TeamStore { return &s.TeamStore } @@ -72,6 +73,7 @@ func (s *Store) UserAccessToken() store.UserAccessTokenStore { return &s.UserA func (s *Store) Plugin() store.PluginStore { return &s.PluginStore } func (s *Store) Role() store.RoleStore { return &s.RoleStore } func (s *Store) Scheme() store.SchemeStore { return &s.SchemeStore } +func (s *Store) ServiceTerms() store.ServiceTermsStore { return &s.ServiceTermsStore } func (s *Store) ChannelMemberHistory() store.ChannelMemberHistoryStore { return &s.ChannelMemberHistoryStore } diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index f3cc59946..533e376b2 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -2181,6 +2181,7 @@ func testUserStoreGetAllAfter(t *testing.T, ss store.Store) { found := false for _, u := range d1 { + if u.Id == u1.Id { found = true assert.Equal(t, u1.Id, u.Id) -- cgit v1.2.3-1-g7c22