diff --git a/server/channels/app/post_metadata.go b/server/channels/app/post_metadata.go index cb0bca2cc14..5285c8641f5 100644 --- a/server/channels/app/post_metadata.go +++ b/server/channels/app/post_metadata.go @@ -130,7 +130,7 @@ func (a *App) populatePostListTranslations(rctx request.CTX, list *model.PostLis continue } - translationsMap, err := a.AutoTranslation().GetBatch(postIDs, userLang) + translationsMap, err := a.AutoTranslation().GetBatch(model.TranslationObjectTypePost, postIDs, userLang) if err != nil { var notAvailErr *model.ErrAutoTranslationNotAvailable if errors.As(err, ¬AvailErr) { diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index 98e80125065..b98968bda45 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -299,3 +299,5 @@ channels/db/migrations/postgres/000150_add_translation_state.down.sql channels/db/migrations/postgres/000150_add_translation_state.up.sql channels/db/migrations/postgres/000151_add_autotranslationdisabled_to_channelmembers.down.sql channels/db/migrations/postgres/000151_add_autotranslationdisabled_to_channelmembers.up.sql +channels/db/migrations/postgres/000152_translations_primary_key_change.down.sql +channels/db/migrations/postgres/000152_translations_primary_key_change.up.sql diff --git a/server/channels/db/migrations/postgres/000152_translations_primary_key_change.down.sql b/server/channels/db/migrations/postgres/000152_translations_primary_key_change.down.sql new file mode 100644 index 00000000000..7f9afcede6a --- /dev/null +++ b/server/channels/db/migrations/postgres/000152_translations_primary_key_change.down.sql @@ -0,0 +1,6 @@ +-- Revert primary key (WARNING: will fail if duplicate objectId+dstLang exist) +ALTER TABLE translations DROP CONSTRAINT translations_pkey; +ALTER TABLE translations ADD PRIMARY KEY (objectId, dstLang); + +-- Allow NULL objectType again +ALTER TABLE translations ALTER COLUMN objectType DROP NOT NULL; diff --git a/server/channels/db/migrations/postgres/000152_translations_primary_key_change.up.sql b/server/channels/db/migrations/postgres/000152_translations_primary_key_change.up.sql new file mode 100644 index 00000000000..d869e1a7583 --- /dev/null +++ b/server/channels/db/migrations/postgres/000152_translations_primary_key_change.up.sql @@ -0,0 +1,9 @@ +-- Safety: Set objectType to 'post' for any NULL rows (table should be empty) +UPDATE translations SET objectType = 'post' WHERE objectType IS NULL; + +-- Make objectType NOT NULL +ALTER TABLE translations ALTER COLUMN objectType SET NOT NULL; + +-- Change primary key to include objectType +ALTER TABLE translations DROP CONSTRAINT translations_pkey; +ALTER TABLE translations ADD PRIMARY KEY (objectId, objectType, dstLang); diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 95ac96d10b0..0924f1370d4 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -872,11 +872,11 @@ func (s *RetryLayerAutoTranslationStore) ClearCaches() { } -func (s *RetryLayerAutoTranslationStore) Get(objectID string, dstLang string) (*model.Translation, error) { +func (s *RetryLayerAutoTranslationStore) Get(objectType string, objectID string, dstLang string) (*model.Translation, error) { tries := 0 for { - result, err := s.AutoTranslationStore.Get(objectID, dstLang) + result, err := s.AutoTranslationStore.Get(objectType, objectID, dstLang) if err == nil { return result, nil } @@ -935,11 +935,11 @@ func (s *RetryLayerAutoTranslationStore) GetAllByStatePage(state model.Translati } -func (s *RetryLayerAutoTranslationStore) GetAllForObject(objectID string) ([]*model.Translation, error) { +func (s *RetryLayerAutoTranslationStore) GetAllForObject(objectType string, objectID string) ([]*model.Translation, error) { tries := 0 for { - result, err := s.AutoTranslationStore.GetAllForObject(objectID) + result, err := s.AutoTranslationStore.GetAllForObject(objectType, objectID) if err == nil { return result, nil } @@ -956,11 +956,11 @@ func (s *RetryLayerAutoTranslationStore) GetAllForObject(objectID string) ([]*mo } -func (s *RetryLayerAutoTranslationStore) GetBatch(objectIDs []string, dstLang string) (map[string]*model.Translation, error) { +func (s *RetryLayerAutoTranslationStore) GetBatch(objectType string, objectIDs []string, dstLang string) (map[string]*model.Translation, error) { tries := 0 for { - result, err := s.AutoTranslationStore.GetBatch(objectIDs, dstLang) + result, err := s.AutoTranslationStore.GetBatch(objectType, objectIDs, dstLang) if err == nil { return result, nil } diff --git a/server/channels/store/sqlstore/autotranslation_store.go b/server/channels/store/sqlstore/autotranslation_store.go index da46d456b33..3959e0901d2 100644 --- a/server/channels/store/sqlstore/autotranslation_store.go +++ b/server/channels/store/sqlstore/autotranslation_store.go @@ -172,16 +172,16 @@ func (s *SqlAutoTranslationStore) GetActiveDestinationLanguages(channelID, exclu return languages, nil } -func (s *SqlAutoTranslationStore) Get(objectID, dstLang string) (*model.Translation, error) { +func (s *SqlAutoTranslationStore) Get(objectType, objectID, dstLang string) (*model.Translation, error) { query := s.getQueryBuilder(). Select("ObjectType", "ObjectId", "DstLang", "ProviderId", "NormHash", "Text", "Confidence", "Meta", "State", "UpdateAt"). From("Translations"). - Where(sq.Eq{"ObjectId": objectID, "DstLang": dstLang}) + Where(sq.Eq{"ObjectType": objectType, "ObjectId": objectID, "DstLang": dstLang}) var translation Translation if err := s.GetReplica().GetBuilder(&translation, query); err != nil { if err == sql.ErrNoRows { - return nil, nil + return nil, store.NewErrNotFound("Translation", objectID) } return nil, errors.Wrapf(err, "failed to get translation for object_id=%s, dst_lang=%s", objectID, dstLang) } @@ -198,12 +198,6 @@ func (s *SqlAutoTranslationStore) Get(objectID, dstLang string) (*model.Translat } } - // Default objectType to "post" if not set - objectType := translation.ObjectType - if objectType == "" { - objectType = model.TranslationObjectTypePost - } - result := &model.Translation{ ObjectID: translation.ObjectID, ObjectType: objectType, @@ -224,7 +218,7 @@ func (s *SqlAutoTranslationStore) Get(objectID, dstLang string) (*model.Translat return result, nil } -func (s *SqlAutoTranslationStore) GetBatch(objectIDs []string, dstLang string) (map[string]*model.Translation, error) { +func (s *SqlAutoTranslationStore) GetBatch(objectType string, objectIDs []string, dstLang string) (map[string]*model.Translation, error) { if len(objectIDs) == 0 { return make(map[string]*model.Translation), nil } @@ -232,7 +226,7 @@ func (s *SqlAutoTranslationStore) GetBatch(objectIDs []string, dstLang string) ( query := s.getQueryBuilder(). Select("ObjectType", "ObjectId", "DstLang", "ProviderId", "NormHash", "Text", "Confidence", "Meta", "State", "UpdateAt"). From("Translations"). - Where(sq.Eq{"ObjectId": objectIDs, "DstLang": dstLang}) + Where(sq.Eq{"ObjectType": objectType, "ObjectId": objectIDs, "DstLang": dstLang}) var translations []Translation if err := s.GetReplica().SelectBuilder(&translations, query); err != nil { @@ -255,12 +249,6 @@ func (s *SqlAutoTranslationStore) GetBatch(objectIDs []string, dstLang string) ( } } - // Default objectType to "post" if not set - objectType := t.ObjectType - if objectType == "" { - objectType = model.TranslationObjectTypePost - } - modelT := &model.Translation{ ObjectID: t.ObjectID, ObjectType: objectType, @@ -285,11 +273,11 @@ func (s *SqlAutoTranslationStore) GetBatch(objectIDs []string, dstLang string) ( return result, nil } -func (s *SqlAutoTranslationStore) GetAllForObject(objectID string) ([]*model.Translation, error) { +func (s *SqlAutoTranslationStore) GetAllForObject(objectType, objectID string) ([]*model.Translation, error) { query := s.getQueryBuilder(). Select("ObjectType", "ObjectId", "DstLang", "ProviderId", "NormHash", "Text", "Confidence", "Meta", "State", "UpdateAt"). From("Translations"). - Where(sq.Eq{"ObjectId": objectID}) + Where(sq.Eq{"ObjectType": objectType, "ObjectId": objectID}) var translations []Translation if err := s.GetReplica().SelectBuilder(&translations, query); err != nil { @@ -312,12 +300,6 @@ func (s *SqlAutoTranslationStore) GetAllForObject(objectID string) ([]*model.Tra } } - // Default objectType to "post" if not set - objectType := t.ObjectType - if objectType == "" { - objectType = model.TranslationObjectTypePost - } - modelT := &model.Translation{ ObjectID: t.ObjectID, ObjectType: objectType, @@ -355,9 +337,9 @@ func (s *SqlAutoTranslationStore) Save(translation *model.Translation) error { text = string(translation.ObjectJSON) } - var objectType *string - if translation.ObjectType != "" { - objectType = &translation.ObjectType + objectType := translation.ObjectType + if objectType == "" { + objectType = model.TranslationObjectTypePost } objectID := translation.ObjectID @@ -389,9 +371,8 @@ func (s *SqlAutoTranslationStore) Save(translation *model.Translation) error { Insert("Translations"). Columns("ObjectId", "DstLang", "ObjectType", "ProviderId", "NormHash", "Text", "Confidence", "Meta", "State", "UpdateAt"). Values(objectID, dstLang, objectType, providerID, translation.NormHash, text, confidence, metaBytes, string(translation.State), now). - Suffix(`ON CONFLICT (ObjectId, dstLang) + Suffix(`ON CONFLICT (ObjectId, ObjectType, dstLang) DO UPDATE SET - ObjectType = EXCLUDED.ObjectType, ProviderId = EXCLUDED.ProviderId, NormHash = EXCLUDED.NormHash, Text = EXCLUDED.Text, diff --git a/server/channels/store/store.go b/server/channels/store/store.go index cf4a711c82b..7f2c6b06cf1 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -1164,9 +1164,9 @@ type AutoTranslationStore interface { GetUserLanguage(userID, channelID string) (string, error) // GetActiveDestinationLanguages returns distinct locales of users who have auto-translation enabled. GetActiveDestinationLanguages(channelID, excludeUserID string, filterUserIDs []string) ([]string, error) - Get(objectID, dstLang string) (*model.Translation, error) - GetBatch(objectIDs []string, dstLang string) (map[string]*model.Translation, error) - GetAllForObject(objectID string) ([]*model.Translation, error) + Get(objectType, objectID, dstLang string) (*model.Translation, error) + GetBatch(objectType string, objectIDs []string, dstLang string) (map[string]*model.Translation, error) + GetAllForObject(objectType, objectID string) ([]*model.Translation, error) Save(translation *model.Translation) error GetAllByStatePage(state model.TranslationState, offset, limit int) ([]*model.Translation, error) GetByStateOlderThan(state model.TranslationState, olderThanMillis int64, limit int) ([]*model.Translation, error) diff --git a/server/channels/store/storetest/mocks/AutoTranslationStore.go b/server/channels/store/storetest/mocks/AutoTranslationStore.go index a4ab3da90ff..20656267db3 100644 --- a/server/channels/store/storetest/mocks/AutoTranslationStore.go +++ b/server/channels/store/storetest/mocks/AutoTranslationStore.go @@ -19,9 +19,9 @@ func (_m *AutoTranslationStore) ClearCaches() { _m.Called() } -// Get provides a mock function with given fields: objectID, dstLang -func (_m *AutoTranslationStore) Get(objectID string, dstLang string) (*model.Translation, error) { - ret := _m.Called(objectID, dstLang) +// Get provides a mock function with given fields: objectType, objectID, dstLang +func (_m *AutoTranslationStore) Get(objectType string, objectID string, dstLang string) (*model.Translation, error) { + ret := _m.Called(objectType, objectID, dstLang) if len(ret) == 0 { panic("no return value specified for Get") @@ -29,19 +29,19 @@ func (_m *AutoTranslationStore) Get(objectID string, dstLang string) (*model.Tra var r0 *model.Translation var r1 error - if rf, ok := ret.Get(0).(func(string, string) (*model.Translation, error)); ok { - return rf(objectID, dstLang) + if rf, ok := ret.Get(0).(func(string, string, string) (*model.Translation, error)); ok { + return rf(objectType, objectID, dstLang) } - if rf, ok := ret.Get(0).(func(string, string) *model.Translation); ok { - r0 = rf(objectID, dstLang) + if rf, ok := ret.Get(0).(func(string, string, string) *model.Translation); ok { + r0 = rf(objectType, objectID, dstLang) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.Translation) } } - if rf, ok := ret.Get(1).(func(string, string) error); ok { - r1 = rf(objectID, dstLang) + if rf, ok := ret.Get(1).(func(string, string, string) error); ok { + r1 = rf(objectType, objectID, dstLang) } else { r1 = ret.Error(1) } @@ -109,9 +109,9 @@ func (_m *AutoTranslationStore) GetAllByStatePage(state model.TranslationState, return r0, r1 } -// GetAllForObject provides a mock function with given fields: objectID -func (_m *AutoTranslationStore) GetAllForObject(objectID string) ([]*model.Translation, error) { - ret := _m.Called(objectID) +// GetAllForObject provides a mock function with given fields: objectType, objectID +func (_m *AutoTranslationStore) GetAllForObject(objectType string, objectID string) ([]*model.Translation, error) { + ret := _m.Called(objectType, objectID) if len(ret) == 0 { panic("no return value specified for GetAllForObject") @@ -119,19 +119,19 @@ func (_m *AutoTranslationStore) GetAllForObject(objectID string) ([]*model.Trans var r0 []*model.Translation var r1 error - if rf, ok := ret.Get(0).(func(string) ([]*model.Translation, error)); ok { - return rf(objectID) + if rf, ok := ret.Get(0).(func(string, string) ([]*model.Translation, error)); ok { + return rf(objectType, objectID) } - if rf, ok := ret.Get(0).(func(string) []*model.Translation); ok { - r0 = rf(objectID) + if rf, ok := ret.Get(0).(func(string, string) []*model.Translation); ok { + r0 = rf(objectType, objectID) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.Translation) } } - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(objectID) + if rf, ok := ret.Get(1).(func(string, string) error); ok { + r1 = rf(objectType, objectID) } else { r1 = ret.Error(1) } @@ -139,9 +139,9 @@ func (_m *AutoTranslationStore) GetAllForObject(objectID string) ([]*model.Trans return r0, r1 } -// GetBatch provides a mock function with given fields: objectIDs, dstLang -func (_m *AutoTranslationStore) GetBatch(objectIDs []string, dstLang string) (map[string]*model.Translation, error) { - ret := _m.Called(objectIDs, dstLang) +// GetBatch provides a mock function with given fields: objectType, objectIDs, dstLang +func (_m *AutoTranslationStore) GetBatch(objectType string, objectIDs []string, dstLang string) (map[string]*model.Translation, error) { + ret := _m.Called(objectType, objectIDs, dstLang) if len(ret) == 0 { panic("no return value specified for GetBatch") @@ -149,19 +149,19 @@ func (_m *AutoTranslationStore) GetBatch(objectIDs []string, dstLang string) (ma var r0 map[string]*model.Translation var r1 error - if rf, ok := ret.Get(0).(func([]string, string) (map[string]*model.Translation, error)); ok { - return rf(objectIDs, dstLang) + if rf, ok := ret.Get(0).(func(string, []string, string) (map[string]*model.Translation, error)); ok { + return rf(objectType, objectIDs, dstLang) } - if rf, ok := ret.Get(0).(func([]string, string) map[string]*model.Translation); ok { - r0 = rf(objectIDs, dstLang) + if rf, ok := ret.Get(0).(func(string, []string, string) map[string]*model.Translation); ok { + r0 = rf(objectType, objectIDs, dstLang) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(map[string]*model.Translation) } } - if rf, ok := ret.Get(1).(func([]string, string) error); ok { - r1 = rf(objectIDs, dstLang) + if rf, ok := ret.Get(1).(func(string, []string, string) error); ok { + r1 = rf(objectType, objectIDs, dstLang) } else { r1 = ret.Error(1) } diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 744799fb840..e21022758e3 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -804,10 +804,10 @@ func (s *TimerLayerAutoTranslationStore) ClearCaches() { } } -func (s *TimerLayerAutoTranslationStore) Get(objectID string, dstLang string) (*model.Translation, error) { +func (s *TimerLayerAutoTranslationStore) Get(objectType string, objectID string, dstLang string) (*model.Translation, error) { start := time.Now() - result, err := s.AutoTranslationStore.Get(objectID, dstLang) + result, err := s.AutoTranslationStore.Get(objectType, objectID, dstLang) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { @@ -852,10 +852,10 @@ func (s *TimerLayerAutoTranslationStore) GetAllByStatePage(state model.Translati return result, err } -func (s *TimerLayerAutoTranslationStore) GetAllForObject(objectID string) ([]*model.Translation, error) { +func (s *TimerLayerAutoTranslationStore) GetAllForObject(objectType string, objectID string) ([]*model.Translation, error) { start := time.Now() - result, err := s.AutoTranslationStore.GetAllForObject(objectID) + result, err := s.AutoTranslationStore.GetAllForObject(objectType, objectID) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { @@ -868,10 +868,10 @@ func (s *TimerLayerAutoTranslationStore) GetAllForObject(objectID string) ([]*mo return result, err } -func (s *TimerLayerAutoTranslationStore) GetBatch(objectIDs []string, dstLang string) (map[string]*model.Translation, error) { +func (s *TimerLayerAutoTranslationStore) GetBatch(objectType string, objectIDs []string, dstLang string) (map[string]*model.Translation, error) { start := time.Now() - result, err := s.AutoTranslationStore.GetBatch(objectIDs, dstLang) + result, err := s.AutoTranslationStore.GetBatch(objectType, objectIDs, dstLang) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { diff --git a/server/einterfaces/autotranslation.go b/server/einterfaces/autotranslation.go index a6839ad102b..ad1504834c4 100644 --- a/server/einterfaces/autotranslation.go +++ b/server/einterfaces/autotranslation.go @@ -53,7 +53,7 @@ type AutoTranslationInterface interface { // GetBatch fetches a batch of translations for a list of object IDs and a destination language. // This is used for efficiently populating translations for list views (e.g., channel history). // Returns error if the feature is unavailable. - GetBatch(objectIDs []string, dstLang string) (map[string]*model.Translation, *model.AppError) + GetBatch(objectType string, objectIDs []string, dstLang string) (map[string]*model.Translation, *model.AppError) // GetUserLanguage returns the preferred language for a user in a channel if auto-translation is enabled. // Returns the language code or error if feature is unavailable. diff --git a/server/einterfaces/mocks/AutoTranslationInterface.go b/server/einterfaces/mocks/AutoTranslationInterface.go index 10c09d42b54..beecafb862a 100644 --- a/server/einterfaces/mocks/AutoTranslationInterface.go +++ b/server/einterfaces/mocks/AutoTranslationInterface.go @@ -76,9 +76,9 @@ func (_m *AutoTranslationInterface) DetectRemote(ctx context.Context, text strin return r0, r1, r2 } -// GetBatch provides a mock function with given fields: objectIDs, dstLang -func (_m *AutoTranslationInterface) GetBatch(objectIDs []string, dstLang string) (map[string]*model.Translation, *model.AppError) { - ret := _m.Called(objectIDs, dstLang) +// GetBatch provides a mock function with given fields: objectType, objectIDs, dstLang +func (_m *AutoTranslationInterface) GetBatch(objectType string, objectIDs []string, dstLang string) (map[string]*model.Translation, *model.AppError) { + ret := _m.Called(objectType, objectIDs, dstLang) if len(ret) == 0 { panic("no return value specified for GetBatch") @@ -86,19 +86,19 @@ func (_m *AutoTranslationInterface) GetBatch(objectIDs []string, dstLang string) var r0 map[string]*model.Translation var r1 *model.AppError - if rf, ok := ret.Get(0).(func([]string, string) (map[string]*model.Translation, *model.AppError)); ok { - return rf(objectIDs, dstLang) + if rf, ok := ret.Get(0).(func(string, []string, string) (map[string]*model.Translation, *model.AppError)); ok { + return rf(objectType, objectIDs, dstLang) } - if rf, ok := ret.Get(0).(func([]string, string) map[string]*model.Translation); ok { - r0 = rf(objectIDs, dstLang) + if rf, ok := ret.Get(0).(func(string, []string, string) map[string]*model.Translation); ok { + r0 = rf(objectType, objectIDs, dstLang) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(map[string]*model.Translation) } } - if rf, ok := ret.Get(1).(func([]string, string) *model.AppError); ok { - r1 = rf(objectIDs, dstLang) + if rf, ok := ret.Get(1).(func(string, []string, string) *model.AppError); ok { + r1 = rf(objectType, objectIDs, dstLang) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*model.AppError)