From e3366d0b628d9596ddb419987ae7e2bc4a38ef7f Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Wed, 16 May 2018 12:06:30 +0200 Subject: [PATCH] adding new attribute lock (#8) --- .ci/gotenberg.yml | 5 +++ _tests/configurations/broken-gotenberg.yml | 1 + .../duplicate-command-gotenberg.yml | 1 + _tests/configurations/gotenberg.yml | 1 + .../merge-timeout-gotenberg.yml | 1 + _tests/configurations/no-lock-gotenberg.yml | 35 +++++++++++++++++++ _tests/configurations/timeout-gotenberg.yml | 1 + .../wrong-command-template-gotenberg.yml | 1 + .../wrong-logging-formatter-gotenberg.yml | 1 + .../wrong-logging-level-gotenberg.yml | 1 + ...wrong-merge-command-template-gotenberg.yml | 1 + app/config/config.go | 11 ++++++ app/config/config_test.go | 18 ++++++++++ app/config/parser.go | 3 ++ app/converter/process/process.go | 8 +++-- app/converter/process/process_test.go | 10 ++++++ 16 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 _tests/configurations/no-lock-gotenberg.yml diff --git a/.ci/gotenberg.yml b/.ci/gotenberg.yml index 2980643c..5bf8f98b 100644 --- a/.ci/gotenberg.yml +++ b/.ci/gotenberg.yml @@ -14,6 +14,11 @@ logs: # You may provide here your own implementation! commands: + # Some libraries like unoconv cannot perform concurrent conversions. That's why the API does only one conversion at a time. + # If your current implementation uses libraries which are able to perform concurrent conversions, you may + # change this value to false. + lock: true + # Unlike others commands' templates, you have access to FilesPaths instead of FilePath: it gathers all PDF files which should be merged. merge: template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" diff --git a/_tests/configurations/broken-gotenberg.yml b/_tests/configurations/broken-gotenberg.yml index 93d01111..74218676 100644 --- a/_tests/configurations/broken-gotenberg.yml +++ b/_tests/configurations/broken-gotenberg.yml @@ -6,6 +6,7 @@ logs: ... } commands: + lock: true merge: template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" interpreter: "/bin/sh -c" diff --git a/_tests/configurations/duplicate-command-gotenberg.yml b/_tests/configurations/duplicate-command-gotenberg.yml index 2b9863c7..8631b4c9 100644 --- a/_tests/configurations/duplicate-command-gotenberg.yml +++ b/_tests/configurations/duplicate-command-gotenberg.yml @@ -3,6 +3,7 @@ logs: level: "DEBUG" formatter: "text" commands: + lock: true merge: template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" interpreter: "/bin/sh -c" diff --git a/_tests/configurations/gotenberg.yml b/_tests/configurations/gotenberg.yml index 66b4024b..1916cb89 100644 --- a/_tests/configurations/gotenberg.yml +++ b/_tests/configurations/gotenberg.yml @@ -3,6 +3,7 @@ logs: level: "DEBUG" formatter: "text" commands: + lock: true merge: template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" interpreter: "/bin/sh -c" diff --git a/_tests/configurations/merge-timeout-gotenberg.yml b/_tests/configurations/merge-timeout-gotenberg.yml index 94c42ccb..4cb2ccae 100644 --- a/_tests/configurations/merge-timeout-gotenberg.yml +++ b/_tests/configurations/merge-timeout-gotenberg.yml @@ -3,6 +3,7 @@ logs: level: "DEBUG" formatter: "text" commands: + lock: true merge: template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" interpreter: "/bin/sh -c" diff --git a/_tests/configurations/no-lock-gotenberg.yml b/_tests/configurations/no-lock-gotenberg.yml new file mode 100644 index 00000000..947574f1 --- /dev/null +++ b/_tests/configurations/no-lock-gotenberg.yml @@ -0,0 +1,35 @@ +port: 3000 +logs: + level: "DEBUG" + formatter: "text" +commands: + lock: false + merge: + template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" + interpreter: "/bin/sh -c" + timeout: 30 + conversions: + - template: "markdown-pdf {{ .FilePath }} -o {{ .ResultFilePath }}" + interpreter: "/bin/sh -c" + timeout: 30 + extensions: + - ".md" + - template: "xvfb-run -e /dev/stdout wkhtmltopdf {{ .FilePath }} {{ .ResultFilePath }}" + interpreter: "/bin/sh -c" + timeout: 30 + extensions: + - ".html" + - ".htm" + - template: "unoconv --format pdf --output \"{{ .ResultFilePath }}\" \"{{ .FilePath }}\"" + interpreter: "/bin/sh -c" + timeout: 30 + extensions: + - ".doc" + - ".docx" + - ".odt" + - ".xls" + - ".xlsx" + - ".ods" + - ".ppt" + - ".pptx" + - ".odp" diff --git a/_tests/configurations/timeout-gotenberg.yml b/_tests/configurations/timeout-gotenberg.yml index a9668d3a..98fdcaba 100644 --- a/_tests/configurations/timeout-gotenberg.yml +++ b/_tests/configurations/timeout-gotenberg.yml @@ -3,6 +3,7 @@ logs: level: "DEBUG" formatter: "text" commands: + lock: true merge: template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" interpreter: "/bin/sh -c" diff --git a/_tests/configurations/wrong-command-template-gotenberg.yml b/_tests/configurations/wrong-command-template-gotenberg.yml index 65cbe447..ad73422d 100644 --- a/_tests/configurations/wrong-command-template-gotenberg.yml +++ b/_tests/configurations/wrong-command-template-gotenberg.yml @@ -3,6 +3,7 @@ logs: level: "DEBUG" formatter: "text" commands: + lock: true merge: template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" interpreter: "/bin/sh -c" diff --git a/_tests/configurations/wrong-logging-formatter-gotenberg.yml b/_tests/configurations/wrong-logging-formatter-gotenberg.yml index 9bcea896..e4e57223 100644 --- a/_tests/configurations/wrong-logging-formatter-gotenberg.yml +++ b/_tests/configurations/wrong-logging-formatter-gotenberg.yml @@ -3,6 +3,7 @@ logs: level: "DEBUG" formatter: "DEBUG" commands: + lock: true merge: template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" interpreter: "/bin/sh -c" diff --git a/_tests/configurations/wrong-logging-level-gotenberg.yml b/_tests/configurations/wrong-logging-level-gotenberg.yml index 68065d53..70b1a1df 100644 --- a/_tests/configurations/wrong-logging-level-gotenberg.yml +++ b/_tests/configurations/wrong-logging-level-gotenberg.yml @@ -3,6 +3,7 @@ logs: level: "text" formatter: "text" commands: + lock: true merge: template: "pdftk {{ range $filePath := .FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" interpreter: "/bin/sh -c" diff --git a/_tests/configurations/wrong-merge-command-template-gotenberg.yml b/_tests/configurations/wrong-merge-command-template-gotenberg.yml index c5118c47..54745b42 100644 --- a/_tests/configurations/wrong-merge-command-template-gotenberg.yml +++ b/_tests/configurations/wrong-merge-command-template-gotenberg.yml @@ -3,6 +3,7 @@ logs: level: "DEBUG" formatter: "text" commands: + lock: true merge: template: "pdftk {{ range $filePath := FilesPaths }} {{ $filePath }} {{ end }} cat output {{ .ResultFilePath }}" interpreter: "/bin/sh -c" diff --git a/app/config/config.go b/app/config/config.go index 4081ae12..ce483f7d 100644 --- a/app/config/config.go +++ b/app/config/config.go @@ -20,6 +20,7 @@ type ( port string logsLevel logrus.Level logsFormatter logrus.Formatter + lock bool // commands associates a file extension with a Command instance. // Particular case: ".pdf" extension is used for the merge command. commands map[string]*Command @@ -126,6 +127,16 @@ func GetLogsFormatter() logrus.Formatter { return config.logsFormatter } +// WithLock sets the lock strategy. +func WithLock(lock bool) { + config.lock = lock +} + +// HasLock returns the current lock strategy. +func HasLock() bool { + return config.lock +} + type interpreterEmptyError struct { command string } diff --git a/app/config/config_test.go b/app/config/config_test.go index 243ec996..a5682722 100644 --- a/app/config/config_test.go +++ b/app/config/config_test.go @@ -99,6 +99,24 @@ func TestGetLogsFormatter(t *testing.T) { } } +func TestWithLock(t *testing.T) { + lock := true + WithLock(lock) + + if config.lock != lock { + t.Errorf("Configuration populated with a wrong lock strategy: got '%t' want '%t'", config.lock, lock) + } +} + +func TestHasLock(t *testing.T) { + lock := true + config.lock = true + + if HasLock() != lock { + t.Errorf("Configuration returned a wrong port: got '%t' want '%t'", HasLock(), lock) + } +} + func TestInterpreterEmptyError(t *testing.T) { err := &interpreterEmptyError{"echo hello world"} expected := fmt.Sprintf(interpreterEmptyErrorMessage, err.command) diff --git a/app/config/parser.go b/app/config/parser.go index 6bd1e0dc..249d6e07 100644 --- a/app/config/parser.go +++ b/app/config/parser.go @@ -23,6 +23,8 @@ func ParseFile(configurationFilePath string) error { return err } + WithLock(fileConfig.Commands.Lock) + // handles merge command first... cmd, err := NewCommand(fileConfig.Commands.Merge.Template, fileConfig.Commands.Merge.Interpreter, fileConfig.Commands.Merge.Timeout) if err != nil { @@ -57,6 +59,7 @@ type ( Formatter string `yaml:"formatter"` } `yaml:"logs"` Commands struct { + Lock bool `yaml:"lock"` Merge *mergeCommand `yaml:"merge"` Conversions []*conversionCommand `yaml:"conversions,omitempty"` } `yaml:"commands"` diff --git a/app/converter/process/process.go b/app/converter/process/process.go index 41cf1848..d6b5632e 100644 --- a/app/converter/process/process.go +++ b/app/converter/process/process.go @@ -33,8 +33,12 @@ func (e *commandTimeoutError) Error() string { // run runs the given command. If timeout is reached or // something bad happened, returns an error. func (r *runner) run(command string, interpreter []string, timeout int) error { - r.mu.Lock() - defer r.mu.Unlock() + if config.HasLock() { + r.mu.Lock() + defer r.mu.Unlock() + } else { + logger.Warn("lock disabled") + } binary := interpreter[0] parameters := append(interpreter[1:], command) diff --git a/app/converter/process/process_test.go b/app/converter/process/process_test.go index fa406a8d..6799876d 100644 --- a/app/converter/process/process_test.go +++ b/app/converter/process/process_test.go @@ -41,6 +41,8 @@ func TestCommandTimeoutError(t *testing.T) { func TestRun(t *testing.T) { var cmd string + load("../../../_tests/configurations/gotenberg.yml") + // case 1: uses a simple command. cmd = "echo Hello world" if err := forest.run(cmd, strings.Fields("/bin/sh -c"), 30); err != nil { @@ -58,6 +60,14 @@ func TestRun(t *testing.T) { if err := forest.run(cmd, strings.Fields("/bin/sh -c"), 30); err == nil { t.Errorf("Command '%s' should not have worked", cmd) } + + load("../../../_tests/configurations/no-lock-gotenberg.yml") + + // case 4: uses a configuration with a no lock strategy. + cmd = "echo Hello world" + if err := forest.run(cmd, strings.Fields("/bin/sh -c"), 30); err != nil { + t.Errorf("Command '%s' should have worked", cmd) + } } func TestUnconv(t *testing.T) {