From 64d12c08e9c290e1af1d6c6aa05cb4600a36443d Mon Sep 17 00:00:00 2001 From: Rodrigo Villablanca Date: Tue, 23 Jun 2020 23:56:35 -0400 Subject: [PATCH] LicenseStore migration to return plain errors (#14837) Automatic Merge --- app/license.go | 17 ++++++++++++----- i18n/en.json | 12 ------------ store/opentracing_layer.go | 4 ++-- store/sqlstore/license_store.go | 14 +++++++------- store/store.go | 4 ++-- store/storetest/mocks/LicenseStore.go | 20 ++++++++------------ store/timer_layer.go | 4 ++-- 7 files changed, 33 insertions(+), 42 deletions(-) diff --git a/app/license.go b/app/license.go index 314af4213a..20f36564ad 100644 --- a/app/license.go +++ b/app/license.go @@ -5,6 +5,7 @@ package app import ( "bytes" + "errors" "net/http" "strings" @@ -35,8 +36,8 @@ func (s *Server) LoadLicense() { } } - record, err := s.Store.License().Get(licenseId) - if err != nil { + record, nErr := s.Store.License().Get(licenseId) + if nErr != nil { mlog.Info("License key from https://mattermost.com required to unlock enterprise features.") s.SetLicense(nil) return @@ -74,10 +75,16 @@ func (s *Server) SaveLicense(licenseBytes []byte) (*model.License, *model.AppErr record.Id = license.Id record.Bytes = string(licenseBytes) - _, err = s.Store.License().Save(record) - if err != nil { + _, nErr := s.Store.License().Save(record) + if nErr != nil { s.RemoveLicense() - return nil, model.NewAppError("addLicense", "api.license.add_license.save.app_error", nil, "err="+err.Error(), http.StatusInternalServerError) + var appErr *model.AppError + switch { + case errors.As(nErr, &appErr): + return nil, appErr + default: + return nil, model.NewAppError("addLicense", "api.license.add_license.save.app_error", nil, err.Error(), http.StatusInternalServerError) + } } sysVar := &model.System{} diff --git a/i18n/en.json b/i18n/en.json index e38220c5ea..9818a65fbd 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -6590,18 +6590,6 @@ "id": "store.sql_job.update.app_error", "translation": "Unable to update the job." }, - { - "id": "store.sql_license.get.app_error", - "translation": "We encountered an error getting the license." - }, - { - "id": "store.sql_license.get.missing.app_error", - "translation": "A license with that ID was not found." - }, - { - "id": "store.sql_license.save.app_error", - "translation": "We encountered an error saving the license." - }, { "id": "store.sql_oauth.delete.commit_transaction.app_error", "translation": "Unable to commit transaction." diff --git a/store/opentracing_layer.go b/store/opentracing_layer.go index c7887d3e2b..1aabf62f26 100644 --- a/store/opentracing_layer.go +++ b/store/opentracing_layer.go @@ -3864,7 +3864,7 @@ func (s *OpenTracingLayerJobStore) UpdateStatusOptimistically(id string, current return resultVar0, resultVar1 } -func (s *OpenTracingLayerLicenseStore) Get(id string) (*model.LicenseRecord, *model.AppError) { +func (s *OpenTracingLayerLicenseStore) Get(id string) (*model.LicenseRecord, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "LicenseStore.Get") s.Root.Store.SetContext(newCtx) @@ -3882,7 +3882,7 @@ func (s *OpenTracingLayerLicenseStore) Get(id string) (*model.LicenseRecord, *mo return resultVar0, resultVar1 } -func (s *OpenTracingLayerLicenseStore) Save(license *model.LicenseRecord) (*model.LicenseRecord, *model.AppError) { +func (s *OpenTracingLayerLicenseStore) Save(license *model.LicenseRecord) (*model.LicenseRecord, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "LicenseStore.Save") s.Root.Store.SetContext(newCtx) diff --git a/store/sqlstore/license_store.go b/store/sqlstore/license_store.go index 2de5dbe753..f2f7a13a3e 100644 --- a/store/sqlstore/license_store.go +++ b/store/sqlstore/license_store.go @@ -4,7 +4,7 @@ package sqlstore import ( - "net/http" + "github.com/pkg/errors" sq "github.com/Masterminds/squirrel" @@ -39,7 +39,7 @@ func (ls SqlLicenseStore) createIndexesIfNotExists() { // database it returns the license stored in the database. If not, it saves the // new database and returns the created license with the CreateAt field // updated. -func (ls SqlLicenseStore) Save(license *model.LicenseRecord) (*model.LicenseRecord, *model.AppError) { +func (ls SqlLicenseStore) Save(license *model.LicenseRecord) (*model.LicenseRecord, error) { license.PreSave() if err := license.IsValid(); err != nil { return nil, err @@ -50,13 +50,13 @@ func (ls SqlLicenseStore) Save(license *model.LicenseRecord) (*model.LicenseReco Where(sq.Eq{"Id": license.Id}) queryString, args, err := query.ToSql() if err != nil { - return nil, model.NewAppError("SqlLicenseStore.Save", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "license_tosql") } var storedLicense model.LicenseRecord if err := ls.GetReplica().SelectOne(&storedLicense, queryString, args...); err != nil { // Only insert if not exists if err := ls.GetMaster().Insert(license); err != nil { - return nil, model.NewAppError("SqlLicenseStore.Save", "store.sql_license.save.app_error", nil, "license_id="+license.Id+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to get License with licenseId=%s", license.Id) } return license, nil } @@ -66,13 +66,13 @@ func (ls SqlLicenseStore) Save(license *model.LicenseRecord) (*model.LicenseReco // Get obtains the license with the provided id parameter from the database. // If the license doesn't exist it returns a model.AppError with // http.StatusNotFound in the StatusCode field. -func (ls SqlLicenseStore) Get(id string) (*model.LicenseRecord, *model.AppError) { +func (ls SqlLicenseStore) Get(id string) (*model.LicenseRecord, error) { obj, err := ls.GetReplica().Get(model.LicenseRecord{}, id) if err != nil { - return nil, model.NewAppError("SqlLicenseStore.Get", "store.sql_license.get.app_error", nil, "license_id="+id+", "+err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to get License with licenseId=%s", id) } if obj == nil { - return nil, model.NewAppError("SqlLicenseStore.Get", "store.sql_license.get.missing.app_error", nil, "license_id="+id, http.StatusNotFound) + return nil, store.NewErrNotFound("License", id) } return obj.(*model.LicenseRecord), nil } diff --git a/store/store.go b/store/store.go index 10f2a13fd3..169ced5d51 100644 --- a/store/store.go +++ b/store/store.go @@ -493,8 +493,8 @@ type PreferenceStore interface { } type LicenseStore interface { - Save(license *model.LicenseRecord) (*model.LicenseRecord, *model.AppError) - Get(id string) (*model.LicenseRecord, *model.AppError) + Save(license *model.LicenseRecord) (*model.LicenseRecord, error) + Get(id string) (*model.LicenseRecord, error) } type TokenStore interface { diff --git a/store/storetest/mocks/LicenseStore.go b/store/storetest/mocks/LicenseStore.go index f5acaea942..6ffc0f3aaf 100644 --- a/store/storetest/mocks/LicenseStore.go +++ b/store/storetest/mocks/LicenseStore.go @@ -15,7 +15,7 @@ type LicenseStore struct { } // Get provides a mock function with given fields: id -func (_m *LicenseStore) Get(id string) (*model.LicenseRecord, *model.AppError) { +func (_m *LicenseStore) Get(id string) (*model.LicenseRecord, error) { ret := _m.Called(id) var r0 *model.LicenseRecord @@ -27,20 +27,18 @@ func (_m *LicenseStore) Get(id string) (*model.LicenseRecord, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(id) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // Save provides a mock function with given fields: license -func (_m *LicenseStore) Save(license *model.LicenseRecord) (*model.LicenseRecord, *model.AppError) { +func (_m *LicenseStore) Save(license *model.LicenseRecord) (*model.LicenseRecord, error) { ret := _m.Called(license) var r0 *model.LicenseRecord @@ -52,13 +50,11 @@ func (_m *LicenseStore) Save(license *model.LicenseRecord) (*model.LicenseRecord } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.LicenseRecord) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.LicenseRecord) error); ok { r1 = rf(license) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 diff --git a/store/timer_layer.go b/store/timer_layer.go index c6c9e69bcb..be80666668 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -3518,7 +3518,7 @@ func (s *TimerLayerJobStore) UpdateStatusOptimistically(id string, currentStatus return resultVar0, resultVar1 } -func (s *TimerLayerLicenseStore) Get(id string) (*model.LicenseRecord, *model.AppError) { +func (s *TimerLayerLicenseStore) Get(id string) (*model.LicenseRecord, error) { start := timemodule.Now() resultVar0, resultVar1 := s.LicenseStore.Get(id) @@ -3534,7 +3534,7 @@ func (s *TimerLayerLicenseStore) Get(id string) (*model.LicenseRecord, *model.Ap return resultVar0, resultVar1 } -func (s *TimerLayerLicenseStore) Save(license *model.LicenseRecord) (*model.LicenseRecord, *model.AppError) { +func (s *TimerLayerLicenseStore) Save(license *model.LicenseRecord) (*model.LicenseRecord, error) { start := timemodule.Now() resultVar0, resultVar1 := s.LicenseStore.Save(license)