SqlLinkMetadataStore migrates to plain errors (#14674)

Automatic Merge

i18n/en.json

@@ -6578,14 +6578,6 @@
     "id": "store.sql_license.save.app_error",
     "translation": "We encountered an error saving the license."
   },
-  {
-    "id": "store.sql_link_metadata.get.app_error",
-    "translation": "Unable to get the link metadata."
-  },
-  {
-    "id": "store.sql_link_metadata.save.app_error",
-    "translation": "Unable to save the link metadata."
-  },
   {
     "id": "store.sql_oauth.delete.commit_transaction.app_error",
     "translation": "Unable to commit transaction."

store/opentracing_layer.go

@@ -3900,7 +3900,7 @@ func (s *OpenTracingLayerLicenseStore) Save(license *model.LicenseRecord) (*mode
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerLinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetadata, *model.AppError) {
+func (s *OpenTracingLayerLinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetadata, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "LinkMetadataStore.Get")
 	s.Root.Store.SetContext(newCtx)
@@ -3918,7 +3918,7 @@ func (s *OpenTracingLayerLinkMetadataStore) Get(url string, timestamp int64) (*m
 	return resultVar0, resultVar1
 }
 
-func (s *OpenTracingLayerLinkMetadataStore) Save(linkMetadata *model.LinkMetadata) (*model.LinkMetadata, *model.AppError) {
+func (s *OpenTracingLayerLinkMetadataStore) Save(linkMetadata *model.LinkMetadata) (*model.LinkMetadata, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "LinkMetadataStore.Save")
 	s.Root.Store.SetContext(newCtx)

store/sqlstore/link_metadata_store.go

@@ -5,12 +5,12 @@ package sqlstore
 
 import (
 	"database/sql"
-	"net/http"
-
-	sq "github.com/Masterminds/squirrel"
 
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/store"
+
+	sq "github.com/Masterminds/squirrel"
+	"github.com/pkg/errors"
 )
 
 type SqlLinkMetadataStore struct {
@@ -38,7 +38,7 @@ func (s SqlLinkMetadataStore) createIndexesIfNotExists() {
 	}
 }
 
-func (s SqlLinkMetadataStore) Save(metadata *model.LinkMetadata) (*model.LinkMetadata, *model.AppError) {
+func (s SqlLinkMetadataStore) Save(metadata *model.LinkMetadata) (*model.LinkMetadata, error) {
 	if err := metadata.IsValid(); err != nil {
 		return nil, err
 	}
@@ -47,13 +47,13 @@ func (s SqlLinkMetadataStore) Save(metadata *model.LinkMetadata) (*model.LinkMet
 
 	err := s.GetMaster().Insert(metadata)
 	if err != nil && !IsUniqueConstraintError(err, []string{"PRIMARY", "linkmetadata_pkey"}) {
-		return nil, model.NewAppError("SqlLinkMetadataStore.Save", "store.sql_link_metadata.save.app_error", nil, "url="+metadata.URL+", "+err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "could not save link metadata")
 	}
 
 	return metadata, nil
 }
 
-func (s SqlLinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetadata, *model.AppError) {
+func (s SqlLinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetadata, error) {
 	var metadata *model.LinkMetadata
 	query, args, err := s.getQueryBuilder().
 		Select("*").
@@ -61,19 +61,19 @@ func (s SqlLinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetad
 		Where(sq.Eq{"URL": url, "Timestamp": timestamp}).
 		ToSql()
 	if err != nil {
-		return nil, model.NewAppError("SqlLinkMetadataStore.Get", "store.sql.build_query.app_error", nil, err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrap(err, "could not create query with querybuilder")
 	}
 	err = s.GetReplica().SelectOne(&metadata, query, args...)
 	if err != nil {
 		if err == sql.ErrNoRows {
-			return nil, model.NewAppError("SqlLinkMetadataStore.Get", "store.sql_link_metadata.get.app_error", nil, "url="+url+", "+err.Error(), http.StatusNotFound)
+			return nil, store.NewErrNotFound("LinkMetadata", "url="+url)
 		}
-		return nil, model.NewAppError("SqlLinkMetadataStore.Get", "store.sql_link_metadata.get.app_error", nil, "url="+url+", "+err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "could not get metadata with selectone: url=%s", url)
 	}
 
 	err = metadata.DeserializeDataToConcreteType()
 	if err != nil {
-		return nil, model.NewAppError("SqlLinkMetadataStore.Get", "store.sql_link_metadata.get.app_error", nil, "url="+url+", "+err.Error(), http.StatusInternalServerError)
+		return nil, errors.Wrapf(err, "could not deserialize metadata to concrete type for url=%s", url)
 	}
 
 	return metadata, nil

store/store.go

@@ -713,8 +713,8 @@ type GroupStore interface {
 }
 
 type LinkMetadataStore interface {
-	Save(linkMetadata *model.LinkMetadata) (*model.LinkMetadata, *model.AppError)
-	Get(url string, timestamp int64) (*model.LinkMetadata, *model.AppError)
+	Save(linkMetadata *model.LinkMetadata) (*model.LinkMetadata, error)
+	Get(url string, timestamp int64) (*model.LinkMetadata, error)
 }
 
 // ChannelSearchOpts contains options for searching channels.

store/storetest/link_metadata_store.go

@@ -4,7 +4,7 @@
 package storetest
 
 import (
-	"net/http"
+	"errors"
 	"testing"
 	"time"
 
@@ -153,7 +153,8 @@ func testLinkMetadataStoreGet(t *testing.T, ss store.Store) {
 		_, err = ss.LinkMetadata().Get("http://example.com/another_page", metadata.Timestamp)
 
 		require.NotNil(t, err)
-		assert.Equal(t, http.StatusNotFound, err.StatusCode)
+		var nfErr *store.ErrNotFound
+		assert.True(t, errors.As(err, &nfErr))
 	})
 
 	t.Run("should return not found with incorrect timestamp", func(t *testing.T) {
@@ -170,7 +171,8 @@ func testLinkMetadataStoreGet(t *testing.T, ss store.Store) {
 		_, err = ss.LinkMetadata().Get(metadata.URL, getNextLinkMetadataTimestamp())
 
 		require.NotNil(t, err)
-		assert.Equal(t, http.StatusNotFound, err.StatusCode)
+		var nfErr *store.ErrNotFound
+		assert.True(t, errors.As(err, &nfErr))
 	})
 }
 

store/storetest/mocks/LinkMetadataStore.go

@@ -15,7 +15,7 @@ type LinkMetadataStore struct {
 }
 
 // Get provides a mock function with given fields: url, timestamp
-func (_m *LinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetadata, *model.AppError) {
+func (_m *LinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetadata, error) {
 	ret := _m.Called(url, timestamp)
 
 	var r0 *model.LinkMetadata
@@ -27,20 +27,18 @@ func (_m *LinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetada
 		}
 	}
 
-	var r1 *model.AppError
-	if rf, ok := ret.Get(1).(func(string, int64) *model.AppError); ok {
+	var r1 error
+	if rf, ok := ret.Get(1).(func(string, int64) error); ok {
 		r1 = rf(url, timestamp)
 	} 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: linkMetadata
-func (_m *LinkMetadataStore) Save(linkMetadata *model.LinkMetadata) (*model.LinkMetadata, *model.AppError) {
+func (_m *LinkMetadataStore) Save(linkMetadata *model.LinkMetadata) (*model.LinkMetadata, error) {
 	ret := _m.Called(linkMetadata)
 
 	var r0 *model.LinkMetadata
@@ -52,13 +50,11 @@ func (_m *LinkMetadataStore) Save(linkMetadata *model.LinkMetadata) (*model.Link
 		}
 	}
 
-	var r1 *model.AppError
-	if rf, ok := ret.Get(1).(func(*model.LinkMetadata) *model.AppError); ok {
+	var r1 error
+	if rf, ok := ret.Get(1).(func(*model.LinkMetadata) error); ok {
 		r1 = rf(linkMetadata)
 	} else {
-		if ret.Get(1) != nil {
-			r1 = ret.Get(1).(*model.AppError)
-		}
+		r1 = ret.Error(1)
 	}
 
 	return r0, r1

store/timer_layer.go

@@ -3550,7 +3550,7 @@ func (s *TimerLayerLicenseStore) Save(license *model.LicenseRecord) (*model.Lice
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerLinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetadata, *model.AppError) {
+func (s *TimerLayerLinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetadata, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.LinkMetadataStore.Get(url, timestamp)
@@ -3566,7 +3566,7 @@ func (s *TimerLayerLinkMetadataStore) Get(url string, timestamp int64) (*model.L
 	return resultVar0, resultVar1
 }
 
-func (s *TimerLayerLinkMetadataStore) Save(linkMetadata *model.LinkMetadata) (*model.LinkMetadata, *model.AppError) {
+func (s *TimerLayerLinkMetadataStore) Save(linkMetadata *model.LinkMetadata) (*model.LinkMetadata, error) {
 	start := timemodule.Now()
 
 	resultVar0, resultVar1 := s.LinkMetadataStore.Save(linkMetadata)