Improve document extraction and including a document extraction command (#17183)

* Add extract documents content command

* Adding the extraction command and making the pure go pdf library as secondary option

* Improving the memory usage and docextractor interface

* Enable content extraction by default in all the instances

* Tiny improvement on archive indexing

* Adding App interface generation and the opentracing layer

* Fixing linter errors

* Addressing PR review comments

* Addressing PR review comments

app/app_iface.go

@@ -515,6 +515,7 @@ type AppIface interface {
 	EnableUserAccessToken(token *model.UserAccessToken) *model.AppError
 	EnvironmentConfig(filter func(reflect.StructField) bool) map[string]interface{}
 	ExportPermissions(w io.Writer) error
+	ExtractContentFromFileInfo(fileInfo *model.FileInfo) error
 	FetchSamlMetadataFromIdp(url string) ([]byte, *model.AppError)
 	FileBackend() (filestore.FileBackend, *model.AppError)
 	FileExists(path string) (bool, *model.AppError)

app/file.go

@@ -9,7 +9,6 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/base64"
-	"errors"
 	"fmt"
 	"image"
 	"image/color"
@@ -30,6 +29,7 @@ import (
 
 	"github.com/disintegration/imaging"
 	_ "github.com/oov/psd"
+	"github.com/pkg/errors"
 	"github.com/rwcarlsen/goexif/exif"
 	_ "golang.org/x/image/bmp"
 	_ "golang.org/x/image/tiff"
@@ -75,6 +75,7 @@ const (
 	ImageThumbnailPixelWidth  = 120
 	ImageThumbnailPixelHeight = 100
 	ImagePreviewPixelWidth    = 1920
+	MaxContentExtractionSize  = 1024 * 1024 // 1Mb
 )
 
 func (a *App) FileBackend() (filestore.FileBackend, *model.AppError) {
@@ -773,21 +774,9 @@ func (a *App) UploadFileX(channelID, name string, input io.Reader,
 	if *a.Config().FileSettings.ExtractContent && a.Config().FeatureFlags.FilesSearch {
 		infoCopy := *t.fileinfo
 		a.Srv().Go(func() {
-			file, aerr := a.FileReader(t.fileinfo.Path)
-			if aerr != nil {
-				mlog.Error("Failed to open file for extract file content", mlog.Err(aerr))
-				return
-			}
-			defer file.Close()
-			text, err := docextractor.Extract(infoCopy.Name, file, docextractor.ExtractSettings{
-				ArchiveRecursion: *a.Config().FileSettings.ArchiveRecursion,
-			})
+			err := a.ExtractContentFromFileInfo(&infoCopy)
 			if err != nil {
-				mlog.Error("Failed to extract file content", mlog.Err(err))
-				return
-			}
-			if storeErr := a.Srv().Store.FileInfo().SetContent(infoCopy.Id, text); storeErr != nil {
-				mlog.Error("Failed to save the extracted file content", mlog.Err(storeErr))
+				mlog.Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id))
 			}
 		})
 	}
@@ -1043,21 +1032,9 @@ func (a *App) DoUploadFileExpectModification(now time.Time, rawTeamId string, ra
 	if *a.Config().FileSettings.ExtractContent && a.Config().FeatureFlags.FilesSearch {
 		infoCopy := *info
 		a.Srv().Go(func() {
-			file, aerr := a.FileReader(infoCopy.Path)
-			if aerr != nil {
-				mlog.Error("Failed to open file for extract file content", mlog.Err(aerr))
-				return
-			}
-			defer file.Close()
-			text, err := docextractor.Extract(infoCopy.Name, file, docextractor.ExtractSettings{
-				ArchiveRecursion: *a.Config().FileSettings.ArchiveRecursion,
-			})
+			err := a.ExtractContentFromFileInfo(&infoCopy)
 			if err != nil {
-				mlog.Error("Failed to extract file content", mlog.Err(err))
-				return
-			}
-			if storeErr := a.Srv().Store.FileInfo().SetContent(infoCopy.Id, text); storeErr != nil {
-				mlog.Error("Failed to save the extracted file content", mlog.Err(storeErr))
+				mlog.Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id))
 			}
 		})
 	}
@@ -1396,3 +1373,26 @@ func (a *App) SearchFilesInTeamForUser(terms string, userId string, teamId strin
 
 	return fileInfoSearchResults, nil
 }
+
+func (a *App) ExtractContentFromFileInfo(fileInfo *model.FileInfo) error {
+	file, aerr := a.FileReader(fileInfo.Path)
+	if aerr != nil {
+		return errors.Wrap(aerr, "failed to open file for extract file content")
+	}
+	defer file.Close()
+	text, err := docextractor.Extract(fileInfo.Name, file, docextractor.ExtractSettings{
+		ArchiveRecursion: *a.Config().FileSettings.ArchiveRecursion,
+	})
+	if err != nil {
+		return errors.Wrap(err, "failed to extract file content")
+	}
+	if text != "" {
+		if len(text) > MaxContentExtractionSize {
+			text = text[0:MaxContentExtractionSize]
+		}
+		if storeErr := a.Srv().Store.FileInfo().SetContent(fileInfo.Id, text); storeErr != nil {
+			return errors.Wrap(storeErr, "failed to save the extracted file content")
+		}
+	}
+	return nil
+}

app/migrations.go

@@ -16,7 +16,6 @@ const EmojisPermissionsMigrationKey = "EmojisPermissionsMigrationComplete"
 const GuestRolesCreationMigrationKey = "GuestRolesCreationMigrationComplete"
 const SystemConsoleRolesCreationMigrationKey = "SystemConsoleRolesCreationMigrationComplete"
 const ContentExtractionConfigMigrationKey = "ContentExtractionConfigMigrationComplete"
-const usersLimitToAutoEnableContentExtraction = 500
 
 // This function migrates the default built in roles from code/config to the database.
 func (a *App) DoAdvancedPermissionsMigration() {
@@ -285,35 +284,6 @@ func (a *App) DoSystemConsoleRolesCreationMigration() {
 	}
 }
 
-func (a *App) doContentExtractionConfigMigration() {
-	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(ContentExtractionConfigMigrationKey); err == nil {
-		return
-	}
-
-	if usersCount, err := a.Srv().Store.User().Count(model.UserCountOptions{}); err != nil {
-		mlog.Critical("Failed to get the users count for migrating the content extraction, using default value", mlog.Err(err))
-	} else {
-		if usersCount < usersLimitToAutoEnableContentExtraction {
-			a.UpdateConfig(func(config *model.Config) {
-				config.FileSettings.ExtractContent = model.NewBool(true)
-			})
-		}
-	}
-
-	system := model.System{
-		Name:  ContentExtractionConfigMigrationKey,
-		Value: "true",
-	}
-
-	if err := a.Srv().Store.System().Save(&system); err != nil {
-		mlog.Critical("Failed to mark content extraction config migration as completed.", mlog.Err(err))
-	}
-}
-
 func (a *App) DoAppMigrations() {
 	a.DoAdvancedPermissionsMigration()
 	a.DoEmojisPermissionsMigration()
@@ -325,5 +295,4 @@ func (a *App) DoAppMigrations() {
 	if err != nil {
 		mlog.Critical("(app.App).DoPermissionsMigrations failed", mlog.Err(err))
 	}
-	a.doContentExtractionConfigMigration()
 }

app/opentracing/opentracing_layer.go

@@ -3795,6 +3795,28 @@ func (a *OpenTracingAppLayer) ExtendSessionExpiryIfNeeded(session *model.Session
 	return resultVar0
 }
 
+func (a *OpenTracingAppLayer) ExtractContentFromFileInfo(fileInfo *model.FileInfo) error {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.ExtractContentFromFileInfo")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0 := a.app.ExtractContentFromFileInfo(fileInfo)
+
+	if resultVar0 != nil {
+		span.LogFields(spanlog.Error(resultVar0))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0
+}
+
 func (a *OpenTracingAppLayer) FetchSamlMetadataFromIdp(url string) ([]byte, *model.AppError) {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.FetchSamlMetadataFromIdp")

app/upload.go

@@ -14,7 +14,6 @@ import (
 
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/plugin"
-	"github.com/mattermost/mattermost-server/v5/services/docextractor"
 	"github.com/mattermost/mattermost-server/v5/shared/mlog"
 	"github.com/mattermost/mattermost-server/v5/store"
 )
@@ -303,15 +302,9 @@ func (a *App) UploadData(us *model.UploadSession, rd io.Reader) (*model.FileInfo
 	if *a.Config().FileSettings.ExtractContent && a.Config().FeatureFlags.FilesSearch {
 		infoCopy := *info
 		a.Srv().Go(func() {
-			text, err := docextractor.Extract(infoCopy.Name, file, docextractor.ExtractSettings{
-				ArchiveRecursion: *a.Config().FileSettings.ArchiveRecursion,
-			})
+			err := a.ExtractContentFromFileInfo(&infoCopy)
 			if err != nil {
-				mlog.Error("Failed to extract file content", mlog.Err(err))
-				return
-			}
-			if storeErr := a.Srv().Store.FileInfo().SetContent(infoCopy.Id, text); storeErr != nil {
-				mlog.Error("Failed to save the extracted file content", mlog.Err(storeErr))
+				mlog.Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id))
 			}
 		})
 	}

cmd/mattermost/commands/extract_content.go

@@ -0,0 +1,86 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package commands
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+	"github.com/spf13/cobra"
+
+	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/shared/mlog"
+)
+
+var ExtractContentCmd = &cobra.Command{
+	Use:     "extract-documents-content",
+	Short:   "Extracts the documents content",
+	Long:    "Extracts the documents content and stores it in the database for document searche",
+	Example: "extract-documents-content --from=12345",
+	RunE:    extractContentCmdF,
+}
+
+func init() {
+	ExtractContentCmd.Flags().Int64("from", 0, "The timestamp of the earliest file to extract, expressed in seconds since the unix epoch.")
+	ExtractContentCmd.Flags().Int64("to", model.GetMillis(), "The timestamp of the latest file to extract, expressed in seconds since the unix epoch.")
+	RootCmd.AddCommand(ExtractContentCmd)
+}
+
+func extractContentCmdF(command *cobra.Command, args []string) error {
+	a, err := InitDBCommandContextCobra(command)
+	if err != nil {
+		return err
+	}
+	defer a.Srv().Shutdown()
+
+	if !*a.Config().FileSettings.ExtractContent || !a.Config().FeatureFlags.FilesSearch {
+		return errors.New("ERROR: Document extraction is not enabled")
+	}
+
+	startTime, err := command.Flags().GetInt64("from")
+	if err != nil {
+		return errors.New("\"from\" flag error")
+	}
+	if startTime < 0 {
+		return errors.New("\"from\" must be a positive integer")
+	}
+
+	endTime, err := command.Flags().GetInt64("to")
+	if err != nil {
+		return errors.New("\"to\" flag error")
+	}
+	if endTime < startTime {
+		return errors.New("\"to\" must be greater than from")
+	}
+
+	since := startTime
+	for {
+		opts := model.GetFileInfosOptions{
+			Since:          since,
+			SortBy:         model.FILEINFO_SORT_BY_CREATED,
+			IncludeDeleted: false,
+		}
+		fileInfos, err := a.Srv().Store.FileInfo().GetWithOptions(0, 1000, &opts)
+		if err != nil {
+			return fmt.Errorf("ERROR: Document extraction failed %v", err.Error())
+		}
+		if len(fileInfos) == 0 {
+			break
+		}
+		for _, fileInfo := range fileInfos {
+			fmt.Println("extracting file", fileInfo.Name, fileInfo.Path)
+			err = a.ExtractContentFromFileInfo(fileInfo)
+			if err != nil {
+				mlog.Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", fileInfo.Id))
+			}
+		}
+		lastFileInfo := fileInfos[len(fileInfos)-1]
+		if lastFileInfo.CreateAt > endTime {
+			break
+		}
+		since = lastFileInfo.CreateAt + 1
+	}
+
+	return nil
+}

model/config.go

@@ -1430,7 +1430,7 @@ func (s *FileSettings) SetDefaults(isUpdate bool) {
 	}
 
 	if s.ExtractContent == nil {
-		s.ExtractContent = NewBool(false)
+		s.ExtractContent = NewBool(true)
 	}
 
 	if s.ArchiveRecursion == nil {

services/docextractor/archive.go

@@ -4,6 +4,7 @@
 package docextractor
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -23,7 +24,7 @@ func (ae *archiveExtractor) Match(filename string) bool {
 	return err == nil
 }
 
-func (ae *archiveExtractor) Extract(name string, r io.Reader) (string, error) {
+func (ae *archiveExtractor) Extract(name string, r io.ReadSeeker) (string, error) {
 	dir, err := ioutil.TempDir(os.TempDir(), "archiver")
 	if err != nil {
 		return "", fmt.Errorf("error creating temporary file: %v", err)
@@ -45,7 +46,14 @@ func (ae *archiveExtractor) Extract(name string, r io.Reader) (string, error) {
 		text.WriteString(file.Name() + " ")
 		if ae.SubExtractor != nil {
 			filename := filepath.Base(file.Name())
-			subtext, extractErr := ae.SubExtractor.Extract(filename, file)
+			filename = strings.ReplaceAll(filename, "-", " ")
+			filename = strings.ReplaceAll(filename, ".", " ")
+			filename = strings.ReplaceAll(filename, ",", " ")
+			data, err2 := ioutil.ReadAll(file)
+			if err2 != nil {
+				return err2
+			}
+			subtext, extractErr := ae.SubExtractor.Extract(filename, bytes.NewReader(data))
 			if extractErr == nil {
 				text.WriteString(subtext + " ")
 			}

services/docextractor/combine.go

@@ -26,9 +26,10 @@ func (ce *combineExtractor) Match(filename string) bool {
 	return false
 }
 
-func (ce *combineExtractor) Extract(filename string, r io.Reader) (string, error) {
+func (ce *combineExtractor) Extract(filename string, r io.ReadSeeker) (string, error) {
 	for _, extractor := range ce.SubExtractors {
 		if extractor.Match(filename) {
+			r.Seek(0, io.SeekStart)
 			text, err := extractor.Extract(filename, r)
 			if err != nil {
 				mlog.Warn("unable to extract file content", mlog.Err(err))

services/docextractor/docextractor.go

@@ -15,18 +15,18 @@ type ExtractSettings struct {
 }
 
 // Extract extract the text from a document using the system default extractors
-func Extract(filename string, r io.Reader, settings ExtractSettings) (string, error) {
+func Extract(filename string, r io.ReadSeeker, settings ExtractSettings) (string, error) {
 	return ExtractWithExtraExtractors(filename, r, settings, []Extractor{})
 }
 
 // ExtractWithExtraExtractors extract the text from a document using the provided extractors beside the system default extractors.
-func ExtractWithExtraExtractors(filename string, r io.Reader, settings ExtractSettings, extraExtractors []Extractor) (string, error) {
+func ExtractWithExtraExtractors(filename string, r io.ReadSeeker, settings ExtractSettings, extraExtractors []Extractor) (string, error) {
 	enabledExtractors := &combineExtractor{}
 	for _, extraExtractor := range extraExtractors {
 		enabledExtractors.Add(extraExtractor)
 	}
-	enabledExtractors.Add(&pdfExtractor{})
 	enabledExtractors.Add(&documentExtractor{})
+	enabledExtractors.Add(&pdfExtractor{})
 
 	if settings.ArchiveRecursion {
 		enabledExtractors.Add(&archiveExtractor{SubExtractor: enabledExtractors})

services/docextractor/docextractor_test.go

@@ -149,7 +149,7 @@ func (te *customTestPdfExtractor) Match(filename string) bool {
 	return strings.HasSuffix(filename, ".pdf")
 }
 
-func (te *customTestPdfExtractor) Extract(filename string, r io.Reader) (string, error) {
+func (te *customTestPdfExtractor) Extract(filename string, r io.ReadSeeker) (string, error) {
 	return "this is a text generated content", nil
 }
 
@@ -159,7 +159,7 @@ func (te *failingExtractor) Match(filename string) bool {
 	return true
 }
 
-func (te *failingExtractor) Extract(filename string, r io.Reader) (string, error) {
+func (te *failingExtractor) Extract(filename string, r io.ReadSeeker) (string, error) {
 	return "", errors.New("this always fail")
 }
 

services/docextractor/documents.go

@@ -25,6 +25,7 @@ var doconvConverterByExtensions = map[string]func(io.Reader) (string, map[string
 	"html":  func(r io.Reader) (string, map[string]string, error) { return docconv.ConvertHTML(r, true) },
 	"pages": docconv.ConvertPages,
 	"rtf":   docconv.ConvertRTF,
+	"pdf":   docconv.ConvertPDF,
 }
 
 func (de *documentExtractor) Match(filename string) bool {
@@ -33,7 +34,7 @@ func (de *documentExtractor) Match(filename string) bool {
 	return ok
 }
 
-func (de *documentExtractor) Extract(filename string, r io.Reader) (string, error) {
+func (de *documentExtractor) Extract(filename string, r io.ReadSeeker) (string, error) {
 	extension := strings.TrimPrefix(path.Ext(filename), ".")
 	converter, ok := doconvConverterByExtensions[extension]
 	if !ok {

services/docextractor/interface.go

@@ -10,5 +10,5 @@ import (
 // Extractors define the interface needed to extract file content
 type Extractor interface {
 	Match(filename string) bool
-	Extract(filename string, file io.Reader) (string, error)
+	Extract(filename string, file io.ReadSeeker) (string, error)
 }

services/docextractor/mmpreview.go

@@ -10,6 +10,7 @@ package docextractor
 import (
 	"bytes"
 	"io"
+	"io/ioutil"
 	"mime/multipart"
 	"net/http"
 	"path"
@@ -41,7 +42,7 @@ func (mpe *mmPreviewExtractor) Match(filename string) bool {
 	return mmpreviewSupportedExtensions[extension]
 }
 
-func (mpe *mmPreviewExtractor) Extract(filename string, file io.Reader) (string, error) {
+func (mpe *mmPreviewExtractor) Extract(filename string, file io.ReadSeeker) (string, error) {
 	b, w, err := createMultipartFormData("file", filename, file)
 	if err != nil {
 		return "", errors.Wrap(err, "Unable to generate file preview using mmpreview.")
@@ -62,10 +63,14 @@ func (mpe *mmPreviewExtractor) Extract(filename string, file io.Reader) (string,
 	if resp.StatusCode != 200 {
 		return "", errors.New("Unable to generate file preview using mmpreview (The server has replied with an error)")
 	}
-	return mpe.pdfExtractor.Extract(filename, resp.Body)
+	data, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return "", errors.Wrap(err, "unable to read the response from mmpreview")
+	}
+	return mpe.pdfExtractor.Extract(filename, bytes.NewReader(data))
 }
 
-func createMultipartFormData(fieldName, fileName string, fileData io.Reader) (bytes.Buffer, *multipart.Writer, error) {
+func createMultipartFormData(fieldName, fileName string, fileData io.ReadSeeker) (bytes.Buffer, *multipart.Writer, error) {
 	var b bytes.Buffer
 	var err error
 	w := multipart.NewWriter(&b)

services/docextractor/pdf.go

@@ -25,7 +25,7 @@ func (pe *pdfExtractor) Match(filename string) bool {
 	return supportedExtensions[extension]
 }
 
-func (pe *pdfExtractor) Extract(filename string, r io.Reader) (string, error) {
+func (pe *pdfExtractor) Extract(filename string, r io.ReadSeeker) (string, error) {
 	f, err := ioutil.TempFile(os.TempDir(), "pdflib")
 	if err != nil {
 		return "", fmt.Errorf("error creating temporary file: %v", err)

services/docextractor/plain.go

@@ -16,7 +16,7 @@ func (pe *plainExtractor) Match(filename string) bool {
 	return true
 }
 
-func (pe *plainExtractor) Extract(filename string, r io.Reader) (string, error) {
+func (pe *plainExtractor) Extract(filename string, r io.ReadSeeker) (string, error) {
 	// This detects any visible character plus any whitespace
 	validRanges := append(unicode.GraphicRanges, unicode.White_Space)
 

store/sqlstore/file_info_store.go

@@ -350,19 +350,11 @@ func (fs SqlFileInfoStore) SetContent(fileId, content string) error {
 		return errors.Wrap(err, "file_info_tosql")
 	}
 
-	sqlResult, err := fs.GetMaster().Exec(queryString, args...)
+	_, err = fs.GetMaster().Exec(queryString, args...)
 	if err != nil {
 		return errors.Wrapf(err, "failed to update FileInfo content with id=%s", fileId)
 	}
 
-	count, err := sqlResult.RowsAffected()
-	if err != nil {
-		// RowsAffected should never fail with the MySQL or Postgres drivers
-		return errors.Wrap(err, "unable to retrieve rows affected")
-	} else if count == 0 {
-		// Could not attach the file to the post
-		return store.NewErrInvalidInput("FileInfo", "<id>", fmt.Sprintf("<%s>", fileId))
-	}
 	return nil
 }