diff --git a/app/post_metadata.go b/app/post_metadata.go index 941aa9f85a..1bc4ff73bf 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -450,9 +450,9 @@ func (a *App) saveLinkMetadataToDatabase(requestURL string, timestamp int64, og metadata.Type = model.LINK_METADATA_TYPE_NONE } - result := <-a.Srv.Store.LinkMetadata().Save(metadata) - if result.Err != nil { - mlog.Warn("Failed to write link metadata", mlog.String("request_url", requestURL), mlog.Err(result.Err)) + _, err := a.Srv.Store.LinkMetadata().Save(metadata) + if err != nil { + mlog.Warn("Failed to write link metadata", mlog.String("request_url", requestURL), mlog.Err(err)) } } diff --git a/store/sqlstore/link_metadata_store.go b/store/sqlstore/link_metadata_store.go index 96ce3715bf..25df3b676d 100644 --- a/store/sqlstore/link_metadata_store.go +++ b/store/sqlstore/link_metadata_store.go @@ -36,22 +36,19 @@ func (s SqlLinkMetadataStore) CreateIndexesIfNotExists() { } } -func (s SqlLinkMetadataStore) Save(metadata *model.LinkMetadata) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if result.Err = metadata.IsValid(); result.Err != nil { - return - } +func (s SqlLinkMetadataStore) Save(metadata *model.LinkMetadata) (*model.LinkMetadata, *model.AppError) { + if err := metadata.IsValid(); err != nil { + return nil, err + } - metadata.PreSave() + metadata.PreSave() - err := s.GetMaster().Insert(metadata) - if err != nil && !IsUniqueConstraintError(err, []string{"PRIMARY", "linkmetadata_pkey"}) { - result.Err = model.NewAppError("SqlLinkMetadataStore.Save", "store.sql_link_metadata.save.app_error", nil, "url="+metadata.URL+", "+err.Error(), http.StatusInternalServerError) - return - } + 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) + } - result.Data = metadata - }) + return metadata, nil } func (s SqlLinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetadata, *model.AppError) { diff --git a/store/store.go b/store/store.go index dd80986f55..69c64c62d1 100644 --- a/store/store.go +++ b/store/store.go @@ -613,7 +613,7 @@ type GroupStore interface { } type LinkMetadataStore interface { - Save(linkMetadata *model.LinkMetadata) StoreChannel + Save(linkMetadata *model.LinkMetadata) (*model.LinkMetadata, *model.AppError) Get(url string, timestamp int64) (*model.LinkMetadata, *model.AppError) } diff --git a/store/storetest/link_metadata_store.go b/store/storetest/link_metadata_store.go index b8a4e302e6..25b1c98e4e 100644 --- a/store/storetest/link_metadata_store.go +++ b/store/storetest/link_metadata_store.go @@ -38,11 +38,10 @@ func testLinkMetadataStoreSave(t *testing.T, ss store.Store) { Data: &model.PostImage{}, } - result := <-ss.LinkMetadata().Save(metadata) + linkMetadata, err := ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) - require.IsType(t, metadata, result.Data) - assert.Equal(t, *metadata, *result.Data.(*model.LinkMetadata)) + require.Nil(t, err) + assert.Equal(t, *metadata, *linkMetadata) }) t.Run("should fail to save invalid item", func(t *testing.T) { @@ -53,9 +52,9 @@ func testLinkMetadataStoreSave(t *testing.T, ss store.Store) { Data: nil, } - result := <-ss.LinkMetadata().Save(metadata) + _, err := ss.LinkMetadata().Save(metadata) - assert.NotNil(t, result.Err) + assert.NotNil(t, err) }) t.Run("should save with duplicate URL and different timestamp", func(t *testing.T) { @@ -66,16 +65,15 @@ func testLinkMetadataStoreSave(t *testing.T, ss store.Store) { Data: &model.PostImage{}, } - result := <-ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) + _, err := ss.LinkMetadata().Save(metadata) + require.Nil(t, err) metadata.Timestamp = getNextLinkMetadataTimestamp() - result = <-ss.LinkMetadata().Save(metadata) + linkMetadata, err := ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) - require.IsType(t, metadata, result.Data) - assert.Equal(t, *metadata, *result.Data.(*model.LinkMetadata)) + require.Nil(t, err) + assert.Equal(t, *metadata, *linkMetadata) }) t.Run("should save with duplicate timestamp and different URL", func(t *testing.T) { @@ -86,16 +84,15 @@ func testLinkMetadataStoreSave(t *testing.T, ss store.Store) { Data: &model.PostImage{}, } - result := <-ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) + _, err := ss.LinkMetadata().Save(metadata) + require.Nil(t, err) metadata.URL = "http://example.com/another/page" - result = <-ss.LinkMetadata().Save(metadata) + linkMetadata, err := ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) - require.IsType(t, metadata, result.Data) - assert.Equal(t, *metadata, *result.Data.(*model.LinkMetadata)) + require.Nil(t, err) + assert.Equal(t, *metadata, *linkMetadata) }) t.Run("should not save with duplicate URL and timestamp, but should not return an error", func(t *testing.T) { @@ -106,20 +103,20 @@ func testLinkMetadataStoreSave(t *testing.T, ss store.Store) { Data: &model.PostImage{}, } - result := <-ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) - assert.Equal(t, &model.PostImage{}, result.Data.(*model.LinkMetadata).Data) + linkMetadata, err := ss.LinkMetadata().Save(metadata) + require.Nil(t, err) + assert.Equal(t, &model.PostImage{}, linkMetadata.Data) metadata.Data = &model.PostImage{Height: 10, Width: 20} - result = <-ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) - assert.Equal(t, result.Data.(*model.LinkMetadata).Data, &model.PostImage{Height: 10, Width: 20}) + linkMetadata, err = ss.LinkMetadata().Save(metadata) + require.Nil(t, err) + assert.Equal(t, linkMetadata.Data, &model.PostImage{Height: 10, Width: 20}) // Should return the original result, not the duplicate one - metadata, err := ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) + linkMetadata, err = ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) require.Nil(t, err) - assert.Equal(t, &model.PostImage{}, metadata.Data) + assert.Equal(t, &model.PostImage{}, linkMetadata.Data) }) } @@ -132,8 +129,8 @@ func testLinkMetadataStoreGet(t *testing.T, ss store.Store) { Data: &model.PostImage{}, } - result := <-ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) + _, err := ss.LinkMetadata().Save(metadata) + require.Nil(t, err) linkMetadata, err := ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) @@ -150,10 +147,10 @@ func testLinkMetadataStoreGet(t *testing.T, ss store.Store) { Data: &model.PostImage{}, } - result := <-ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) + _, err := ss.LinkMetadata().Save(metadata) + require.Nil(t, err) - _, err := ss.LinkMetadata().Get("http://example.com/another_page", metadata.Timestamp) + _, err = ss.LinkMetadata().Get("http://example.com/another_page", metadata.Timestamp) require.NotNil(t, err) assert.Equal(t, http.StatusNotFound, err.StatusCode) @@ -167,10 +164,10 @@ func testLinkMetadataStoreGet(t *testing.T, ss store.Store) { Data: &model.PostImage{}, } - result := <-ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) + _, err := ss.LinkMetadata().Save(metadata) + require.Nil(t, err) - _, err := ss.LinkMetadata().Get(metadata.URL, getNextLinkMetadataTimestamp()) + _, err = ss.LinkMetadata().Get(metadata.URL, getNextLinkMetadataTimestamp()) require.NotNil(t, err) assert.Equal(t, http.StatusNotFound, err.StatusCode) @@ -189,14 +186,13 @@ func testLinkMetadataStoreTypes(t *testing.T, ss store.Store) { }, } - result := <-ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) + received, err := ss.LinkMetadata().Save(metadata) + require.Nil(t, err) - received := result.Data.(*model.LinkMetadata) require.IsType(t, &model.PostImage{}, received.Data) assert.Equal(t, *(metadata.Data.(*model.PostImage)), *(received.Data.(*model.PostImage))) - received, err := ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) + received, err = ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) require.Nil(t, err) require.IsType(t, &model.PostImage{}, received.Data) @@ -220,14 +216,13 @@ func testLinkMetadataStoreTypes(t *testing.T, ss store.Store) { Data: og, } - result := <-ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) + received, err := ss.LinkMetadata().Save(metadata) + require.Nil(t, err) - received := result.Data.(*model.LinkMetadata) require.IsType(t, &opengraph.OpenGraph{}, received.Data) assert.Equal(t, *(metadata.Data.(*opengraph.OpenGraph)), *(received.Data.(*opengraph.OpenGraph))) - received, err := ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) + received, err = ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) require.Nil(t, err) require.IsType(t, &opengraph.OpenGraph{}, received.Data) @@ -242,13 +237,11 @@ func testLinkMetadataStoreTypes(t *testing.T, ss store.Store) { Data: nil, } - result := <-ss.LinkMetadata().Save(metadata) - require.Nil(t, result.Err) - - received := result.Data.(*model.LinkMetadata) + received, err := ss.LinkMetadata().Save(metadata) + require.Nil(t, err) assert.Nil(t, received.Data) - received, err := ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) + received, err = ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) require.Nil(t, err) require.Nil(t, received.Data) diff --git a/store/storetest/mocks/LinkMetadataStore.go b/store/storetest/mocks/LinkMetadataStore.go index f1a56fde8c..5de2575969 100644 --- a/store/storetest/mocks/LinkMetadataStore.go +++ b/store/storetest/mocks/LinkMetadataStore.go @@ -6,7 +6,6 @@ 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" // LinkMetadataStore is an autogenerated mock type for the LinkMetadataStore type type LinkMetadataStore struct { @@ -39,17 +38,26 @@ func (_m *LinkMetadataStore) Get(url string, timestamp int64) (*model.LinkMetada } // Save provides a mock function with given fields: linkMetadata -func (_m *LinkMetadataStore) Save(linkMetadata *model.LinkMetadata) store.StoreChannel { +func (_m *LinkMetadataStore) Save(linkMetadata *model.LinkMetadata) (*model.LinkMetadata, *model.AppError) { ret := _m.Called(linkMetadata) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.LinkMetadata) store.StoreChannel); ok { + var r0 *model.LinkMetadata + if rf, ok := ret.Get(0).(func(*model.LinkMetadata) *model.LinkMetadata); ok { r0 = rf(linkMetadata) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.LinkMetadata) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(*model.LinkMetadata) *model.AppError); ok { + r1 = rf(linkMetadata) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 }