refactoring multipart from data handling from converter + tests for converter package

_tests/configurations/merge-timeout-gotenberg.yml

@@ -0,0 +1,14 @@
+port: 3000
+logs:
+  level: "DEBUG"
+  format: "text"
+commands:
+  html:
+    timeout: 30
+    template: "xvfb-run -e /dev/stdout wkhtmltopdf {{ .FilePath }} {{ .ResultFilePath }}"
+  office:
+    timeout: 30
+    template: "unoconv --format pdf --output \"{{ .ResultFilePath }}\" \"{{ .FilePath }}\""
+  merge:
+    timeout: 0
+    template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}"

app/converter/converter.go

@@ -3,6 +3,7 @@ package converter
 
 import (
 	"fmt"
+	"io"
 	"net/http"
 	"os"
 
@@ -19,22 +20,6 @@ type Converter struct {
 	workingDir string
 }
 
-// NoFileToConvertError is raised when the converter has no file
-// to convert.
-type NoFileToConvertError struct{}
-
-func (e *NoFileToConvertError) Error() string {
-	return "There is no file to convert"
-}
-
-// FilesKeyNotFoundError is raised when "files" key does not exist
-// in the form data
-type FilesKeyNotFoundError struct{}
-
-func (e *FilesKeyNotFoundError) Error() string {
-	return "\"files\" key was not found in the form data"
-}
-
 // NewConverter instantiates a converter by parsing a request.
 func NewConverter(r *http.Request, contentType ghttp.ContentType) (*Converter, error) {
 	c := &Converter{
@@ -47,26 +32,22 @@ func NewConverter(r *http.Request, contentType ghttp.ContentType) (*Converter, e
 
 	switch contentType {
 	case ghttp.MultipartFormDataContentType:
-		err := r.ParseMultipartForm(32 << 20)
+		reader, err := r.MultipartReader()
 		if err != nil {
 			return nil, err
 		}
 
-		formData := r.MultipartForm
-		files, ok := formData.File["files"]
-		if !ok {
-			return nil, &FilesKeyNotFoundError{}
-		}
-
-		for i := range files {
-			file, err := files[i].Open()
-			if err != nil {
-				return nil, err
+		for {
+			part, err := reader.NextPart()
+			if err == io.EOF {
+				break
 			}
 
-			defer file.Close()
+			if part.FileName() == "" {
+				continue
+			}
 
-			f, err := gfile.NewFile(c.workingDir, file)
+			f, err := gfile.NewFile(c.workingDir, part)
 			if err != nil {
 				return nil, err
 			}
@@ -83,10 +64,6 @@ func NewConverter(r *http.Request, contentType ghttp.ContentType) (*Converter, e
 		c.files = append(c.files, f)
 	}
 
-	if len(c.files) == 0 {
-		return nil, &NoFileToConvertError{}
-	}
-
 	return c, nil
 }
 

app/converter/converter_test.go

@@ -0,0 +1,132 @@
+package converter
+
+import (
+	"bytes"
+	"io"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/gulien/gotenberg/app/config"
+	"github.com/gulien/gotenberg/app/converter/process"
+	ghttp "github.com/gulien/gotenberg/app/http"
+)
+
+func makeRequest(filesPaths ...string) *http.Request {
+	if len(filesPaths) == 0 {
+		req := httptest.NewRequest(http.MethodPost, "/", new(bytes.Buffer))
+		req.Header.Set("Content-Type", string(ghttp.OctetStreamContentType))
+		return req
+	}
+
+	if len(filesPaths) == 1 {
+		file, _ := os.Open(filesPaths[0])
+		req := httptest.NewRequest(http.MethodPost, "/", file)
+		req.Header.Set("Content-Type", string(ghttp.OctetStreamContentType))
+		return req
+	}
+
+	r, w := io.Pipe()
+	mpw := multipart.NewWriter(w)
+
+	go func() {
+		var part io.Writer
+		defer w.Close()
+
+		for _, filePath := range filesPaths {
+			file, _ := os.Open(filePath)
+			defer file.Close()
+
+			fileInfo, _ := file.Stat()
+			part, _ = mpw.CreateFormFile("files", fileInfo.Name())
+			io.Copy(part, file)
+		}
+
+		mpw.Close()
+	}()
+
+	req := httptest.NewRequest(http.MethodPost, "/", r)
+	req.Header.Set("Content-Type", mpw.FormDataContentType())
+	return req
+}
+
+func TestNewConverter(t *testing.T) {
+	// case 1: uses a request with a single file.
+	path, _ := filepath.Abs("../../_tests/file.docx")
+	if _, err := NewConverter(makeRequest(path), ghttp.OctetStreamContentType); err != nil {
+		t.Error("Converter should have been instantiated!")
+	}
+
+	// case 2: uses a request with wrong file type.
+	path, _ = filepath.Abs("../../_tests/configurations/gotenberg.yml")
+	if _, err := NewConverter(makeRequest(path), ghttp.OctetStreamContentType); err == nil {
+		t.Error("Converter should not have been instantiated!")
+	}
+
+	// case 3: uses a request with two files.
+	oPath, _ := filepath.Abs("../../_tests/file.docx")
+	path, _ = filepath.Abs("../../_tests/file.pdf")
+	if _, err := NewConverter(makeRequest(oPath, path), ghttp.MultipartFormDataContentType); err != nil {
+		t.Error("Converter should have been instantiated!")
+	}
+
+	// case 4: uses a request with one Office file and one wrong file type.
+	oPath, _ = filepath.Abs("../../_tests/file.docx")
+	path, _ = filepath.Abs("../../_tests/configurations/gotenberg.yml")
+	if _, err := NewConverter(makeRequest(oPath, path), ghttp.MultipartFormDataContentType); err == nil {
+		t.Error("Converter should not have been instantiated!")
+	}
+}
+
+func TestConvert(t *testing.T) {
+	path, _ := filepath.Abs("../../_tests/configurations/gotenberg.yml")
+	appConfig, _ := config.NewAppConfig(path)
+	process.Load(appConfig.CommandsConfig)
+
+	// case 1: uses a request with a single file.
+	path, _ = filepath.Abs("../../_tests/file.docx")
+	c, _ := NewConverter(makeRequest(path), ghttp.OctetStreamContentType)
+	if _, err := c.Convert(); err != nil {
+		t.Error("Converter should have been able to convert an Office document to PDF!")
+	}
+
+	// case 2: uses a request with two files.
+	oPath, _ := filepath.Abs("../../_tests/file.docx")
+	path, _ = filepath.Abs("../../_tests/file.pdf")
+	c, _ = NewConverter(makeRequest(oPath, path), ghttp.MultipartFormDataContentType)
+	if _, err := c.Convert(); err != nil {
+		t.Error("Converter should have been able to convert two files to PDF and merge them!")
+	}
+
+	// case 3: uses a request with a single file and a configuration with an unsuitable timeout for the conversion commands.
+	path, _ = filepath.Abs("../../_tests/configurations/timeout-gotenberg.yml")
+	appConfig, _ = config.NewAppConfig(path)
+	process.Load(appConfig.CommandsConfig)
+	path, _ = filepath.Abs("../../_tests/file.docx")
+	c, _ = NewConverter(makeRequest(path), ghttp.OctetStreamContentType)
+	if _, err := c.Convert(); err == nil {
+		t.Error("Converter should not have been able to convert an Office document to PDF!")
+	}
+
+	// case 4: uses a request with two files and a configuration with an unsuitable timeout for the merge command.
+	path, _ = filepath.Abs("../../_tests/configurations/merge-timeout-gotenberg.yml")
+	appConfig, _ = config.NewAppConfig(path)
+	process.Load(appConfig.CommandsConfig)
+	oPath, _ = filepath.Abs("../../_tests/file.docx")
+	path, _ = filepath.Abs("../../_tests/file.pdf")
+	c, _ = NewConverter(makeRequest(oPath, path), ghttp.MultipartFormDataContentType)
+	if _, err := c.Convert(); err == nil {
+		t.Error("Converter should not have been able to merge PDF!")
+	}
+}
+
+func TestClear(t *testing.T) {
+	path, _ := filepath.Abs("../../_tests/file.docx")
+	c, _ := NewConverter(makeRequest(path), ghttp.OctetStreamContentType)
+	if err := c.Clear(); err != nil {
+		t.Error("Converter should have been able to clear itself!")
+	}
+}

app/handlers.go

@@ -70,14 +70,7 @@ func convertHandler(next http.Handler) http.Handler {
 
 		c, err := converter.NewConverter(r, ct)
 		if err != nil {
-			if noFileToConvertError, ok := err.(*converter.NoFileToConvertError); ok {
-				http.Error(w, noFileToConvertError.Error(), http.StatusBadRequest)
-			} else if filesKeyNotFoundError, ok := err.(*converter.FilesKeyNotFoundError); ok {
-				http.Error(w, filesKeyNotFoundError.Error(), http.StatusBadRequest)
-			} else {
-				http.Error(w, err.Error(), http.StatusInternalServerError)
-			}
-
+			http.Error(w, err.Error(), http.StatusInternalServerError)
 			logger.Error(err)
 			return
 		}

orbit.yml

@@ -28,4 +28,4 @@ tasks:
   - use: up
     short: Starts the {{ .Orbit.Version }} version of the Gotenberg image
     run:
-      - docker run --rm -p 3000:3000/tcp --name=gotenberg gotenberg:{{ .Orbit.Version }}
+      - docker run --rm -p 3000:3000/tcp gotenberg:{{ .Orbit.Version }}