refactoring: now detecting file type using filename from form data. Also, only one accepted content type

app/converter/file/file.go

@@ -5,8 +5,7 @@ import (
 	"fmt"
 	"io"
 	"os"
-
-	ghttp "github.com/thecodingmachine/gotenberg/app/http"
+	"path/filepath"
 
 	"github.com/satori/go.uuid"
 )
@@ -32,11 +31,42 @@ const (
 	OfficeType
 )
 
+// filesTypes associates a file extension with its file kind counterpart.
+var filesTypes = map[string]Type{
+	".pdf":  PDFType,
+	".html": HTMLType,
+	".doc":  OfficeType,
+	".docx": OfficeType,
+	".odt":  OfficeType,
+	".xls":  OfficeType,
+	".xlsx": OfficeType,
+	".ods":  OfficeType,
+	".ppt":  OfficeType,
+	".pptx": OfficeType,
+	".odp":  OfficeType,
+}
+
+type fileTypeNotFoundError struct {
+	fileName string
+}
+
+func (e *fileTypeNotFoundError) Error() string {
+	return fmt.Sprintf("File type was not found for '%s'", e.fileName)
+}
+
 // NewFile creates a file in the considered directory.
 // Returns a *File instance or an error if something bad happened.
-func NewFile(workingDir string, r io.Reader) (*File, error) {
+func NewFile(workingDir string, r io.Reader, fileName string) (*File, error) {
+	ext := filepath.Ext(fileName)
+
+	t, ok := filesTypes[ext]
+	if !ok {
+		return nil, &fileTypeNotFoundError{fileName: fileName}
+	}
+
 	f := &File{
-		Path: MakeFilePath(workingDir),
+		Path: MakeFilePath(workingDir, ext),
+		Type: t,
 	}
 
 	file, err := os.Create(f.Path)
@@ -54,103 +84,11 @@ func NewFile(workingDir string, r io.Reader) (*File, error) {
 	// resets the read pointer.
 	file.Seek(0, 0)
 
-	t, err := findFileType(file)
-	if err != nil {
-		return nil, err
-	}
-
-	f.Type = t
-
-	f, err = reworkFilePath(workingDir, f)
-	if err != nil {
-		return nil, err
-	}
-
 	return f, nil
 }
 
 // MakeFilePath is a simple helper which generates a random file name
 // and associates it with the considered directory to make a path.
-func MakeFilePath(workingDir string) string {
-	return fmt.Sprintf("%s%s", workingDir, uuid.NewV4().String())
-}
-
-// filesTypes associates a content type with its file kind counterpart.
-var filesTypes = map[ghttp.ContentType]Type{
-	ghttp.PDFContentType:         PDFType,
-	ghttp.HTMLContentType:        HTMLType,
-	ghttp.OctetStreamContentType: OfficeType,
-	ghttp.ZipContentType:         OfficeType,
-}
-
-type fileTypeNotFoundError struct{}
-
-const fileTypeNotFoundErrorMessage = "The file type was not found for the given 'Content-Type'"
-
-func (e *fileTypeNotFoundError) Error() string {
-	return fileTypeNotFoundErrorMessage
-}
-
-// findFileType tries to detect what kind of file is the given file.
-func findFileType(f *os.File) (Type, error) {
-	ct, err := ghttp.SniffContentType(f)
-	if err != nil {
-		return 999, err
-	}
-
-	t, ok := filesTypes[ct]
-	if !ok {
-		return 999, &fileTypeNotFoundError{}
-	}
-
-	return t, nil
-}
-
-// Ext represents a file extension.
-type Ext string
-
-const (
-	// PDFExt represents a... PDF extension.
-	PDFExt Ext = ".pdf"
-	// HTMLExt represents an... HTML extension.
-	HTMLExt Ext = ".html"
-	// OfficeExt is a empty string, as Office documents
-	// have a lot of different extensions (.docx, .doc and so on).
-	OfficeExt Ext = ""
-)
-
-// filesExtensions associates a kind of file with its extension.
-var filesExtensions = map[Type]Ext{
-	PDFType:    PDFExt,
-	HTMLType:   HTMLExt,
-	OfficeType: OfficeExt,
-}
-
-type fileExtNotFoundError struct{}
-
-const fileExtNotFoundErrorMessage = "The file extension was not found for the given file type"
-
-func (e *fileExtNotFoundError) Error() string {
-	return fileExtNotFoundErrorMessage
-}
-
-// reworkFilePath renames a file in the considered directory and adds its extension.
-func reworkFilePath(workingDir string, f *File) (*File, error) {
-	ext, ok := filesExtensions[f.Type]
-	if !ok {
-		return nil, &fileExtNotFoundError{}
-	}
-
-	if ext != OfficeExt {
-		newPath := fmt.Sprintf("%s%s", MakeFilePath(workingDir), ext)
-
-		err := os.Rename(f.Path, newPath)
-		if err != nil {
-			return nil, err
-		}
-
-		f.Path = newPath
-	}
-
-	return f, nil
+func MakeFilePath(workingDir string, ext string) string {
+	return fmt.Sprintf("%s%s%s", workingDir, uuid.NewV4().String(), ext)
 }

app/converter/file/file_test.go

@@ -2,6 +2,7 @@ package file
 
 import (
 	"bytes"
+	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
@@ -11,56 +12,27 @@ func TestNewFile(t *testing.T) {
 	workingDir := "test"
 	os.Mkdir(workingDir, 0666)
 
-	// case 1: uses an empty reader.
-	if _, err := NewFile(workingDir, new(bytes.Buffer)); err == nil {
-		t.Error("File should not have been instantiated!")
-	}
-
-	// case 2: uses a reader from a wrong file type.
-	path, _ := filepath.Abs("../../../_tests/configurations/gotenberg.yml")
-	r, _ := os.Open(path)
-	defer r.Close()
-	if _, err := NewFile(workingDir, r); err == nil {
+	// case 2: uses a wrong file name.
+	if _, err := NewFile(workingDir, new(bytes.Buffer), "file.yml"); err == nil {
 		t.Error("File should not have been instantiated!")
 	}
 
 	// case 3: uses a reader from a correct file type.
-	path, _ = filepath.Abs("../../../_tests/file.pdf")
-	r, _ = os.Open(path)
+	filePath, _ := filepath.Abs("../../../_tests/file.pdf")
+	r, _ := os.Open(filePath)
 	defer r.Close()
-	if _, err := NewFile(workingDir, r); err != nil {
+	if _, err := NewFile(workingDir, r, "file.pdf"); err != nil {
 		t.Error("File should have been instantiated!")
 	}
 
 	os.RemoveAll(workingDir)
 }
-
-func TestReworkFilePath(t *testing.T) {
-	workingDir := "test"
-	os.Mkdir(workingDir, 0666)
-
-	f := &File{
-		Path: MakeFilePath(workingDir),
-		Type: 999,
-	}
-
-	if _, err := reworkFilePath(workingDir, f); err == nil {
-		t.Error("It should not have been able to found the file extension!")
-	}
-
-	os.RemoveAll(workingDir)
-}
-
 func TestFileTypeNotFoundError(t *testing.T) {
-	err := &fileTypeNotFoundError{}
-	if err.Error() != fileTypeNotFoundErrorMessage {
-		t.Errorf("Error returned a wrong message: got %s want %s", err.Error(), fileTypeNotFoundErrorMessage)
-	}
-}
+	fileName := "file.wp"
+	err := &fileTypeNotFoundError{fileName: fileName}
+	expected := fmt.Sprintf("File type was not found for '%s'", fileName)
 
-func TestFileExtNotFoundError(t *testing.T) {
-	err := &fileExtNotFoundError{}
-	if err.Error() != fileExtNotFoundErrorMessage {
-		t.Errorf("Error returned a wrong message: got %s want %s", err.Error(), fileExtNotFoundErrorMessage)
+	if err.Error() != expected {
+		t.Errorf("Error returned a wrong message: got %s want %s", err.Error(), expected)
 	}
 }