diff --git a/internal/pkg/printer/merge.go b/internal/pkg/printer/merge.go index fd350f38..d9fd87ad 100644 --- a/internal/pkg/printer/merge.go +++ b/internal/pkg/printer/merge.go @@ -34,13 +34,13 @@ func NewMergePrinter(logger xlog.Logger, fpaths []string, opts MergePrinterOptio func (p mergePrinter) Print(destination string) error { const op string = "printer.mergePrinter.Print" - logOptions(p.logger, p.opts) /* context.Context may be providen from an officePrinter which needs to merge its result files. */ if p.ctx == nil { + logOptions(p.logger, p.opts) ctx, cancel := xcontext.WithTimeout(p.logger, p.opts.WaitTimeout) defer cancel() p.ctx = ctx diff --git a/internal/pkg/printer/office.go b/internal/pkg/printer/office.go index 9ed66c0e..fb0d8f27 100644 --- a/internal/pkg/printer/office.go +++ b/internal/pkg/printer/office.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "sync" "github.com/thecodingmachine/gotenberg/internal/pkg/xcontext" "github.com/thecodingmachine/gotenberg/internal/pkg/xerror" @@ -63,6 +62,7 @@ func (p officePrinter) Print(destination string) error { return nil } m := mergePrinter{ + logger: p.logger, ctx: ctx, fpaths: fpaths, } @@ -78,15 +78,10 @@ func (p officePrinter) Print(destination string) error { } // nolint: gochecknoglobals -var mu sync.Mutex +var lock = make(chan struct{}, 1) func unoconv(ctx context.Context, logger xlog.Logger, fpath, destination string, opts OfficePrinterOptions) error { const op string = "printer.unoconv" - // TODO check if timeout while waiting for the lock. - logger.DebugOp(op, "waiting lock to be released...") - mu.Lock() - defer mu.Unlock() - logger.DebugOp(op, "lock released") resolver := func() error { args := []string{ "--format", @@ -108,10 +103,23 @@ func unoconv(ctx context.Context, logger xlog.Logger, fpath, destination string, xexec.LogBeforeExecute(logger, cmd) return cmd.Run() } - if err := resolver(); err != nil { - return xerror.New(op, err) + logger.DebugOp(op, "waiting lock to be acquired...") + select { + case lock <- struct{}{}: + // lock acquired. + logger.DebugOp(op, "lock acquired") + if err := resolver(); err != nil { + <-lock // we release the lock. + return xerror.New(op, err) + } + <-lock // we release the lock. + return nil + case <-ctx.Done(): + // failed to acquire lock before + // deadline. + logger.DebugOp(op, "failed to acquire lock before context.Context deadline") + return xerror.New(op, ctx.Err()) } - return nil } // Compile-time checks to ensure type implements desired interfaces. diff --git a/internal/pkg/printer/office_test.go b/internal/pkg/printer/office_test.go new file mode 100644 index 00000000..836fe0d2 --- /dev/null +++ b/internal/pkg/printer/office_test.go @@ -0,0 +1,59 @@ +package printer + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/thecodingmachine/gotenberg/internal/pkg/xerror" + "github.com/thecodingmachine/gotenberg/internal/pkg/xlog" + "github.com/thecodingmachine/gotenberg/test/internalpkg/printertest" + "github.com/thecodingmachine/gotenberg/test/internalpkg/xerrortest" + "github.com/thecodingmachine/gotenberg/test/internalpkg/xlogtest" +) + +func TestOfficePrinter(t *testing.T) { + var ( + logger xlog.Logger = xlogtest.DebugLogger() + fpaths []string = printertest.OfficeFpaths(t) + opts OfficePrinterOptions + dest string + p Printer + err error + ) + // default options. + opts = OfficePrinterOptions{ + WaitTimeout: 10.0, + Landscape: false, + } + p = NewOfficePrinter(logger, fpaths, opts) + dest = printertest.GenerateDestination() + err = p.Print(dest) + assert.Nil(t, err) + err = os.RemoveAll(dest) + assert.Nil(t, err) + // options with landscape. + opts = OfficePrinterOptions{ + WaitTimeout: 10.0, + Landscape: true, + } + p = NewOfficePrinter(logger, fpaths, opts) + dest = printertest.GenerateDestination() + err = p.Print(dest) + assert.Nil(t, err) + err = os.RemoveAll(dest) + assert.Nil(t, err) + // should not be OK as context.Context + // should timeout. + opts = OfficePrinterOptions{ + WaitTimeout: 1.0, + Landscape: true, + } + p = NewOfficePrinter(logger, fpaths, opts) + dest = printertest.GenerateDestination() + err = p.Print(dest) + xerrortest.AssertError(t, err) + assert.Equal(t, xerror.TimeoutCode, xerror.Code(err)) + err = os.RemoveAll(dest) + assert.Nil(t, err) +} diff --git a/test/internalpkg/printertest/doc.go b/test/internalpkg/printertest/doc.go new file mode 100644 index 00000000..8b10544a --- /dev/null +++ b/test/internalpkg/printertest/doc.go @@ -0,0 +1,6 @@ +/* +Package printertest contains useful +functions for tests related +to printer package. +*/ +package printertest diff --git a/test/internalpkg/printertest/printertest.go b/test/internalpkg/printertest/printertest.go new file mode 100644 index 00000000..9c909856 --- /dev/null +++ b/test/internalpkg/printertest/printertest.go @@ -0,0 +1,35 @@ +package printertest + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "github.com/thecodingmachine/gotenberg/internal/pkg/xrand" +) + +const testdataDirectoryPath string = "/gotenberg/tests/test/testdata" + +// GenerateDestination simply generates +// a path for a resulting PDF file. +func GenerateDestination() string { + return fmt.Sprintf("/tmp/%s.pdf", xrand.Get()) +} + +// OfficeFpaths return the paths +// of the Office documents used in tests. +func OfficeFpaths(t *testing.T) []string { + return []string{ + fpath(t, "office", "document.docx"), + fpath(t, "office", "document.rtf"), + fpath(t, "office", "document.txt"), + } +} + +func fpath(t *testing.T, kind, filename string) string { + require.NotEmpty(t, kind) + require.NotEmpty(t, filename) + fpath := fmt.Sprintf("%s/%s/%s", testdataDirectoryPath, kind, filename) + require.FileExists(t, fpath) + return fpath +}