From df695115be82e09c27d9b54de754889599b0bf20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Fri, 30 Apr 2021 23:21:26 +0200 Subject: [PATCH] Removing FilesSearch feature flag (#17548) * Removing FilesSearch feature flag * Fixing tests * Adding an improvement on plain text extraction * Adding tests for plain text extraction * Removed unneeded conversion * Adding missed license * Remove the feature flag from the migration * Fixing some tests * Updating i18n/en.json file --- api4/file.go | 5 -- api4/file_test.go | 18 ------- app/file.go | 4 +- app/migrations.go | 3 -- app/post_metadata_test.go | 1 + app/upload.go | 2 +- cmd/mattermost/commands/extract_content.go | 2 +- i18n/en.json | 4 -- model/feature_flags.go | 4 -- services/docextractor/plain.go | 4 +- services/docextractor/plain_test.go | 53 +++++++++++++++++++ .../bleveengine/indexer/indexing_job.go | 4 -- testlib/store.go | 1 + 13 files changed, 61 insertions(+), 44 deletions(-) create mode 100644 services/docextractor/plain_test.go diff --git a/api4/file.go b/api4/file.go index 31c5d10c3a..0203695c51 100644 --- a/api4/file.go +++ b/api4/file.go @@ -734,11 +734,6 @@ func searchFiles(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !c.App.Config().FeatureFlags.FilesSearch { - c.Err = model.NewAppError("searchFiles", "api.post.search_files.not_implemented.app_error", nil, "", http.StatusNotImplemented) - return - } - if !c.App.SessionHasPermissionToTeam(*c.App.Session(), c.Params.TeamId, model.PERMISSION_VIEW_TEAM) { c.SetPermissionError(model.PERMISSION_VIEW_TEAM) return diff --git a/api4/file_test.go b/api4/file_test.go index 9751c8fe5c..d5164dfb67 100644 --- a/api4/file_test.go +++ b/api4/file_test.go @@ -1048,33 +1048,15 @@ func TestGetPublicFile(t *testing.T) { require.Equal(t, http.StatusNotFound, resp.StatusCode, "should've failed to get file after it is deleted") } -func TestSearchFilesOnFeatureFlagDisabled(t *testing.T) { - th := Setup(t).InitBasic() - defer th.TearDown() - - terms := "search" - isOrSearch := false - timezoneOffset := 5 - searchParams := model.SearchParameter{ - Terms: &terms, - IsOrSearch: &isOrSearch, - TimeZoneOffset: &timezoneOffset, - } - _, resp := th.Client.SearchFilesWithParams(th.BasicTeam.Id, &searchParams) - require.NotNil(t, resp.Error) -} - func TestSearchFiles(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() experimentalViewArchivedChannels := *th.App.Config().TeamSettings.ExperimentalViewArchivedChannels defer func() { - os.Unsetenv("MM_FEATUREFLAGS_FILESSEARCH") th.App.UpdateConfig(func(cfg *model.Config) { cfg.TeamSettings.ExperimentalViewArchivedChannels = &experimentalViewArchivedChannels }) }() - os.Setenv("MM_FEATUREFLAGS_FILESSEARCH", "true") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.ExperimentalViewArchivedChannels = true }) diff --git a/app/file.go b/app/file.go index bdff81ff7a..f7b4b05195 100644 --- a/app/file.go +++ b/app/file.go @@ -782,7 +782,7 @@ func (a *App) UploadFileX(channelID, name string, input io.Reader, } } - if *a.Config().FileSettings.ExtractContent && a.Config().FeatureFlags.FilesSearch { + if *a.Config().FileSettings.ExtractContent { infoCopy := *t.fileinfo a.Srv().Go(func() { err := a.ExtractContentFromFileInfo(&infoCopy) @@ -1040,7 +1040,7 @@ func (a *App) DoUploadFileExpectModification(now time.Time, rawTeamId string, ra } } - if *a.Config().FileSettings.ExtractContent && a.Config().FeatureFlags.FilesSearch { + if *a.Config().FileSettings.ExtractContent { infoCopy := *info a.Srv().Go(func() { err := a.ExtractContentFromFileInfo(&infoCopy) diff --git a/app/migrations.go b/app/migrations.go index 1920cde63d..f06c0946e0 100644 --- a/app/migrations.go +++ b/app/migrations.go @@ -288,9 +288,6 @@ func (a *App) DoSystemConsoleRolesCreationMigration() { } func (a *App) doContentExtractionConfigDefaultTrueMigration() { - if !a.Config().FeatureFlags.FilesSearch { - return - } // If the migration is already marked as completed, don't do it again. if _, err := a.Srv().Store.System().GetByName(ContentExtractionConfigDefaultTrueMigrationKey); err == nil { return diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index 7b97abfece..af16d3727d 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -177,6 +177,7 @@ func TestPreparePostForClient(t *testing.T) { defer th.TearDown() fileInfo, err := th.App.DoUploadFile(time.Now(), th.BasicTeam.Id, th.BasicChannel.Id, th.BasicUser.Id, "test.txt", []byte("test")) + fileInfo.Content = "test" require.Nil(t, err) post, err := th.App.CreatePost(&model.Post{ diff --git a/app/upload.go b/app/upload.go index f239f02b68..39cd004228 100644 --- a/app/upload.go +++ b/app/upload.go @@ -299,7 +299,7 @@ func (a *App) UploadData(us *model.UploadSession, rd io.Reader) (*model.FileInfo } } - if *a.Config().FileSettings.ExtractContent && a.Config().FeatureFlags.FilesSearch { + if *a.Config().FileSettings.ExtractContent { infoCopy := *info a.Srv().Go(func() { err := a.ExtractContentFromFileInfo(&infoCopy) diff --git a/cmd/mattermost/commands/extract_content.go b/cmd/mattermost/commands/extract_content.go index 87915dbcbb..1a12165e91 100644 --- a/cmd/mattermost/commands/extract_content.go +++ b/cmd/mattermost/commands/extract_content.go @@ -42,7 +42,7 @@ func extractContentCmdF(command *cobra.Command, args []string) error { } defer a.Srv().Shutdown() - if !*a.Config().FileSettings.ExtractContent || !a.Config().FeatureFlags.FilesSearch { + if !*a.Config().FileSettings.ExtractContent { return errors.New("ERROR: Document extraction is not enabled") } diff --git a/i18n/en.json b/i18n/en.json index b375f0d7c2..7d8df04708 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2246,10 +2246,6 @@ "id": "api.post.search_files.invalid_body.app_error", "translation": "Unable to parse the request body." }, - { - "id": "api.post.search_files.not_implemented.app_error", - "translation": "This feature is in development, and is only available using a feature flag." - }, { "id": "api.post.search_posts.invalid_body.app_error", "translation": "Unable to parse the request body." diff --git a/model/feature_flags.go b/model/feature_flags.go index d8722333ca..7a853e34f2 100644 --- a/model/feature_flags.go +++ b/model/feature_flags.go @@ -35,9 +35,6 @@ type FeatureFlags struct { PluginIncidentManagement string `plugin_id:"com.mattermost.plugin-incident-management"` PluginApps string `plugin_id:"com.mattermost.apps"` - // Toggle on and off support for Files search - FilesSearch bool - // Control support for custom data retention policies CustomDataRetentionEnabled bool } @@ -48,7 +45,6 @@ func (f *FeatureFlags) SetDefaults() { f.CloudDelinquentEmailJobsEnabled = false f.CollapsedThreads = false f.EnableRemoteClusterService = false - f.FilesSearch = false f.AppsEnabled = false f.PluginIncidentManagement = "1.7.0" diff --git a/services/docextractor/plain.go b/services/docextractor/plain.go index d2dc22d77a..4d3b084502 100644 --- a/services/docextractor/plain.go +++ b/services/docextractor/plain.go @@ -42,11 +42,11 @@ func (pe *plainExtractor) Extract(filename string, r io.ReadSeeker) (string, err count += size // subtract the max rune size to prevent accidentally splitted runes at the end of first 1024 bytes - if count > total-utf8.UTFMax || count > len(runes)-utf8.UTFMax { + if count > total-utf8.UTFMax { break } } text, _ := ioutil.ReadAll(r) - return string(runes) + string(text), nil + return string(runes[0:total]) + string(text), nil } diff --git a/services/docextractor/plain_test.go b/services/docextractor/plain_test.go new file mode 100644 index 0000000000..4108f22050 --- /dev/null +++ b/services/docextractor/plain_test.go @@ -0,0 +1,53 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package docextractor + +import ( + "bytes" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPlainEmptyFile(t *testing.T) { + extractor := plainExtractor{} + extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte{})) + require.NoError(t, err) + require.Equal(t, "", extractedText) +} + +func TestPlainTextSmallFile(t *testing.T) { + extractor := plainExtractor{} + content := strings.Repeat("test \n", 5) + extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte(content))) + require.NoError(t, err) + require.Equal(t, content, extractedText) +} + +func TestPlainBigFile(t *testing.T) { + extractor := plainExtractor{} + content := strings.Repeat("test \n", 1000) + extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte(content))) + require.NoError(t, err) + require.Equal(t, content, extractedText) +} + +func TestSmallBinaryFile(t *testing.T) { + extractor := plainExtractor{} + notUTF8Char := byte(0x7) + content := bytes.Repeat([]byte{notUTF8Char}, 1000) + extractedText, err := extractor.Extract("test.bin", bytes.NewReader(content)) + require.NoError(t, err) + require.Equal(t, "", extractedText) +} + +func TestBigBinaryFile(t *testing.T) { + extractor := plainExtractor{} + notUTF8Char := byte(0x7) + content := bytes.Repeat([]byte{notUTF8Char}, 10000) + extractedText, err := extractor.Extract("test.bin", bytes.NewReader(content)) + require.NoError(t, err) + require.Equal(t, "", extractedText) +} diff --git a/services/searchengine/bleveengine/indexer/indexing_job.go b/services/searchengine/bleveengine/indexer/indexing_job.go index 29bd090579..55eb0ddce4 100644 --- a/services/searchengine/bleveengine/indexer/indexing_job.go +++ b/services/searchengine/bleveengine/indexer/indexing_job.go @@ -148,10 +148,6 @@ func (worker *BleveIndexerWorker) DoJob(job *model.Job) { EndAtTime: model.GetMillis(), } - if !worker.jobServer.Config().FeatureFlags.FilesSearch { - progress.DoneFiles = true - } - // Extract the start and end times, if they are set. if startString, ok := job.Data["start_time"]; ok { startInt, err := strconv.ParseInt(startString, 10, 64) diff --git a/testlib/store.go b/testlib/store.go index e2c7b29dc2..a18ea90347 100644 --- a/testlib/store.go +++ b/testlib/store.go @@ -24,6 +24,7 @@ func (s *TestStore) Close() { func GetMockStoreForSetupFunctions() *mocks.Store { mockStore := mocks.Store{} systemStore := mocks.SystemStore{} + systemStore.On("GetByName", "ContentExtractionConfigDefaultTrueMigrationComplete").Return(&model.System{Name: "ContentExtractionConfigDefaultTrueMigrationComplete", Value: "true"}, nil) systemStore.On("GetByName", "UpgradedFromTE").Return(nil, model.NewAppError("FakeError", "app.system.get_by_name.app_error", nil, "", http.StatusInternalServerError)) systemStore.On("GetByName", "ContentExtractionConfigMigrationComplete").Return(&model.System{Name: "ContentExtractionConfigMigrationComplete", Value: "true"}, nil) systemStore.On("GetByName", "AsymmetricSigningKey").Return(nil, model.NewAppError("FakeError", "app.system.get_by_name.app_error", nil, "", http.StatusInternalServerError))