diff --git a/pkg/gotenberg/mocks.go b/pkg/gotenberg/mocks.go index 380fab1a..49154c32 100644 --- a/pkg/gotenberg/mocks.go +++ b/pkg/gotenberg/mocks.go @@ -37,8 +37,8 @@ func (mod *ValidatorMock) Validate() error { type PdfEngineMock struct { MergeMock func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error ConvertMock func(ctx context.Context, logger *zap.Logger, formats PdfFormats, inputPath, outputPath string) error - ReadMetadataMock func(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error - WriteMetadataMock func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error + ReadMetadataMock func(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) + WriteMetadataMock func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error } func (engine *PdfEngineMock) Merge(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { @@ -49,12 +49,12 @@ func (engine *PdfEngineMock) Convert(ctx context.Context, logger *zap.Logger, fo return engine.ConvertMock(ctx, logger, formats, inputPath, outputPath) } -func (engine *PdfEngineMock) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error { - return engine.ReadMetadataMock(ctx, logger, inputPath, metadata) +func (engine *PdfEngineMock) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return engine.ReadMetadataMock(ctx, logger, inputPath) } -func (engine *PdfEngineMock) WriteMetadata(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { - return engine.WriteMetadataMock(ctx, logger, inputPath, newMetadata) +func (engine *PdfEngineMock) WriteMetadata(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { + return engine.WriteMetadataMock(ctx, logger, metadata, inputPath) } // PdfEngineProviderMock is a mock for the [PdfEngineProvider] interface. diff --git a/pkg/gotenberg/mocks_test.go b/pkg/gotenberg/mocks_test.go index fa67c3a1..548f330d 100644 --- a/pkg/gotenberg/mocks_test.go +++ b/pkg/gotenberg/mocks_test.go @@ -55,10 +55,10 @@ func TestPDFEngineMock(t *testing.T) { ConvertMock: func(ctx context.Context, logger *zap.Logger, formats PdfFormats, inputPath, outputPath string) error { return nil }, - ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error { - return nil + ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return nil, nil }, - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return nil }, } @@ -73,12 +73,12 @@ func TestPDFEngineMock(t *testing.T) { t.Errorf("expected no error from PdfEngineMock.Convert, but got: %v", err) } - err = mock.ReadMetadataMock(context.Background(), zap.NewNop(), "", map[string]interface{}{}) + _, err = mock.ReadMetadataMock(context.Background(), zap.NewNop(), "") if err != nil { t.Errorf("expected no error from PdfEngineMock.ReadMetadata, but got: %v", err) } - err = mock.WriteMetadataMock(context.Background(), zap.NewNop(), "", map[string]interface{}{}) + err = mock.WriteMetadataMock(context.Background(), zap.NewNop(), map[string]interface{}{}, "") if err != nil { t.Errorf("expected no error from PdfEngineMock.WriteMetadata but got: %v", err) } diff --git a/pkg/gotenberg/pdfengine.go b/pkg/gotenberg/pdfengine.go index f70d7d2d..87c32c15 100644 --- a/pkg/gotenberg/pdfengine.go +++ b/pkg/gotenberg/pdfengine.go @@ -15,6 +15,10 @@ var ( // ErrPdfFormatNotSupported is returned when the Convert method of the // PdfEngine interface does not support a requested PDF format conversion. ErrPdfFormatNotSupported = errors.New("PDF format not supported") + + // ErrPdfEngineMetadataValueNotSupported is returned when a metadata value + // is not supported. + ErrPdfEngineMetadataValueNotSupported = errors.New("metadata value not supported") ) const ( @@ -65,11 +69,11 @@ type PdfEngine interface { // PdfFormats. If no format, it does nothing. Convert(ctx context.Context, logger *zap.Logger, formats PdfFormats, inputPath, outputPath string) error - // ReadMetadata extracts the metadata of a given PDF file and load them into the provided metadata object. - ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error + // ReadMetadata extracts the metadata of a given PDF file. + ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) // WriteMetadata writes the metadata into a given PDF file. - WriteMetadata(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error + WriteMetadata(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error } // PdfEngineProvider offers an interface to instantiate a [PdfEngine]. diff --git a/pkg/gotenberg/sort.go b/pkg/gotenberg/sort.go index e8e2ea70..bd18da69 100644 --- a/pkg/gotenberg/sort.go +++ b/pkg/gotenberg/sort.go @@ -9,7 +9,7 @@ import ( // AlphanumericSort implements sort.Interface and helps to sort strings // alphanumerically. // -// See https://github.com/gotenberg/gotenberg/issues/805. +// See: https://github.com/gotenberg/gotenberg/issues/805. type AlphanumericSort []string func (s AlphanumericSort) Len() int { diff --git a/pkg/modules/api/middlewares.go b/pkg/modules/api/middlewares.go index 3854e8bc..1c5c6097 100644 --- a/pkg/modules/api/middlewares.go +++ b/pkg/modules/api/middlewares.go @@ -44,6 +44,10 @@ func ParseError(err error) (int, string) { return http.StatusBadRequest, "At least one PDF engine cannot process the requested PDF format, while others may have failed to convert due to different issues" } + if errors.Is(err, gotenberg.ErrPdfEngineMetadataValueNotSupported) { + return http.StatusBadRequest, "At least one PDF engine cannot process the requested metadata, while others may have failed to convert due to different issues" + } + var httpErr HttpError if errors.As(err, &httpErr) { return httpErr.HttpError() diff --git a/pkg/modules/api/middlewares_test.go b/pkg/modules/api/middlewares_test.go index d3c90030..1a9eb207 100644 --- a/pkg/modules/api/middlewares_test.go +++ b/pkg/modules/api/middlewares_test.go @@ -43,6 +43,11 @@ func TestParseError(t *testing.T) { expectStatus: http.StatusBadRequest, expectMessage: "At least one PDF engine cannot process the requested PDF format, while others may have failed to convert due to different issues", }, + { + err: gotenberg.ErrPdfEngineMetadataValueNotSupported, + expectStatus: http.StatusBadRequest, + expectMessage: "At least one PDF engine cannot process the requested metadata, while others may have failed to convert due to different issues", + }, { err: WrapError( errors.New("foo"), diff --git a/pkg/modules/chromium/routes.go b/pkg/modules/chromium/routes.go index 39ee13bb..51409e93 100644 --- a/pkg/modules/chromium/routes.go +++ b/pkg/modules/chromium/routes.go @@ -78,7 +78,7 @@ func FormDataChromiumOptions(ctx *api.Context) (*api.FormData, Options) { } if value != "screen" && value != "print" { - return fmt.Errorf("wrong value, expected either 'screen', 'print' or empty") + return errors.New("wrong value, expected either 'screen', 'print' or empty") } emulatedMediaType = value @@ -233,16 +233,14 @@ func FormDataChromiumPdfFormats(form *api.FormData) gotenberg.PdfFormats { } } -// FormDataMetadata creates metadata object from the form data. -func FormDataMetadata(form *api.FormData) map[string]interface{} { +// FormDataPdfMetadata creates metadata object from the form data. +func FormDataPdfMetadata(form *api.FormData) map[string]interface{} { var metadata map[string]interface{} - form.Custom("metadata", func(value string) error { - metadata = map[string]interface{}{} if len(value) > 0 { err := json.Unmarshal([]byte(value), &metadata) if err != nil { - return err + return fmt.Errorf("unmarshal metadata: %w", err) } } return nil @@ -260,7 +258,7 @@ func convertUrlRoute(chromium Api, engine gotenberg.PdfEngine) api.Route { ctx := c.Get("context").(*api.Context) form, options := FormDataChromiumPdfOptions(ctx) pdfFormats := FormDataChromiumPdfFormats(form) - metadata := FormDataMetadata(form) + metadata := FormDataPdfMetadata(form) var url string err := form. @@ -270,7 +268,7 @@ func convertUrlRoute(chromium Api, engine gotenberg.PdfEngine) api.Route { return fmt.Errorf("validate form data: %w", err) } - err = convertUrl(ctx, chromium, engine, url, pdfFormats, options, metadata) + err = convertUrl(ctx, chromium, engine, url, options, pdfFormats, metadata) if err != nil { return fmt.Errorf("convert URL to PDF: %w", err) } @@ -320,7 +318,7 @@ func convertHtmlRoute(chromium Api, engine gotenberg.PdfEngine) api.Route { ctx := c.Get("context").(*api.Context) form, options := FormDataChromiumPdfOptions(ctx) pdfFormats := FormDataChromiumPdfFormats(form) - metadata := FormDataMetadata(form) + metadata := FormDataPdfMetadata(form) var inputPath string err := form. @@ -331,7 +329,7 @@ func convertHtmlRoute(chromium Api, engine gotenberg.PdfEngine) api.Route { } url := fmt.Sprintf("file://%s", inputPath) - err = convertUrl(ctx, chromium, engine, url, pdfFormats, options, metadata) + err = convertUrl(ctx, chromium, engine, url, options, pdfFormats, metadata) if err != nil { return fmt.Errorf("convert HTML to PDF: %w", err) } @@ -382,7 +380,7 @@ func convertMarkdownRoute(chromium Api, engine gotenberg.PdfEngine) api.Route { ctx := c.Get("context").(*api.Context) form, options := FormDataChromiumPdfOptions(ctx) pdfFormats := FormDataChromiumPdfFormats(form) - metadata := FormDataMetadata(form) + metadata := FormDataPdfMetadata(form) var ( inputPath string @@ -402,7 +400,7 @@ func convertMarkdownRoute(chromium Api, engine gotenberg.PdfEngine) api.Route { return fmt.Errorf("transform markdown file(s) to HTML: %w", err) } - err = convertUrl(ctx, chromium, engine, url, pdfFormats, options, metadata) + err = convertUrl(ctx, chromium, engine, url, options, pdfFormats, metadata) if err != nil { return fmt.Errorf("convert markdown to PDF: %w", err) } @@ -526,7 +524,7 @@ func markdownToHtml(ctx *api.Context, inputPath string, markdownPaths []string) return fmt.Sprintf("file://%s", inputPath), nil } -func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url string, pdfFormats gotenberg.PdfFormats, options PdfOptions, metadata map[string]interface{}) error { +func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url string, options PdfOptions, pdfFormats gotenberg.PdfFormats, metadata map[string]interface{}) error { outputPath := ctx.GeneratePath(".pdf") err := chromium.Pdf(ctx, ctx.Log(), url, outputPath, options) @@ -584,7 +582,7 @@ func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url // Writes and potentially overrides metadata entries, if any. if len(metadata) > 0 { - err = engine.WriteMetadata(ctx, ctx.Log(), outputPath, metadata) + err = engine.WriteMetadata(ctx, ctx.Log(), metadata, outputPath) if err != nil { return fmt.Errorf("write metadata: %w", err) } diff --git a/pkg/modules/chromium/routes_test.go b/pkg/modules/chromium/routes_test.go index b67bccd4..56e1a69d 100644 --- a/pkg/modules/chromium/routes_test.go +++ b/pkg/modules/chromium/routes_test.go @@ -399,6 +399,57 @@ func TestFormDataChromiumPdfFormats(t *testing.T) { } } +func TestFormDataPdfMetadata(t *testing.T) { + for _, tc := range []struct { + scenario string + ctx *api.ContextMock + expectedMetadata map[string]interface{} + }{ + { + scenario: "no metadata form field", + ctx: &api.ContextMock{Context: new(api.Context)}, + expectedMetadata: nil, + }, + { + scenario: "invalid metadata form field", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetValues(map[string][]string{ + "metadata": { + "foo", + }, + }) + return ctx + }(), + expectedMetadata: nil, + }, + { + scenario: "valid metadata form field", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetValues(map[string][]string{ + "metadata": { + "{\"foo\":\"bar\"}", + }, + }) + return ctx + }(), + expectedMetadata: map[string]interface{}{ + "foo": "bar", + }, + }, + } { + t.Run(tc.scenario, func(t *testing.T) { + tc.ctx.SetLogger(zap.NewNop()) + actual := FormDataPdfMetadata(tc.ctx.Context.FormData()) + + if !reflect.DeepEqual(actual, tc.expectedMetadata) { + t.Fatalf("expected %+v but got: %+v", tc.expectedMetadata, actual) + } + }) + } +} + func TestConvertUrlRoute(t *testing.T) { for _, tc := range []struct { scenario string @@ -1223,8 +1274,8 @@ func TestConvertUrl(t *testing.T) { ctx *api.ContextMock api Api engine gotenberg.PdfEngine - pdfFormats gotenberg.PdfFormats options PdfOptions + pdfFormats gotenberg.PdfFormats metadata map[string]interface{} expectError bool expectHttpError bool @@ -1331,7 +1382,7 @@ func TestConvertUrl(t *testing.T) { expectOutputPathsCount: 0, }, { - scenario: "error from PDF engine", + scenario: "PDF engine convert error", ctx: &api.ContextMock{Context: new(api.Context)}, api: &ApiMock{PdfMock: func(ctx context.Context, logger *zap.Logger, url, outputPath string, options PdfOptions) error { return nil @@ -1339,14 +1390,14 @@ func TestConvertUrl(t *testing.T) { engine: &gotenberg.PdfEngineMock{ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { return errors.New("foo") }}, - pdfFormats: gotenberg.PdfFormats{PdfA: "foo"}, options: DefaultPdfOptions(), + pdfFormats: gotenberg.PdfFormats{PdfA: "foo"}, expectError: true, expectHttpError: false, expectOutputPathsCount: 0, }, { - scenario: "success with pdfa form field", + scenario: "success with PDF formats", ctx: &api.ContextMock{Context: new(api.Context)}, api: &ApiMock{PdfMock: func(ctx context.Context, logger *zap.Logger, url, outputPath string, options PdfOptions) error { return nil @@ -1354,12 +1405,29 @@ func TestConvertUrl(t *testing.T) { engine: &gotenberg.PdfEngineMock{ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { return nil }}, - pdfFormats: gotenberg.PdfFormats{PdfA: gotenberg.PdfA1b}, options: DefaultPdfOptions(), + pdfFormats: gotenberg.PdfFormats{PdfA: gotenberg.PdfA1b}, expectError: false, expectHttpError: false, expectOutputPathsCount: 1, }, + { + scenario: "PDF engine write metadata error", + ctx: &api.ContextMock{Context: new(api.Context)}, + api: &ApiMock{PdfMock: func(ctx context.Context, logger *zap.Logger, url, outputPath string, options PdfOptions) error { + return nil + }}, + engine: &gotenberg.PdfEngineMock{WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { + return errors.New("foo") + }}, + options: DefaultPdfOptions(), + metadata: map[string]interface{}{ + "Creator": "foo", + "Producer": "bar", + }, + expectError: true, + expectHttpError: false, + }, { scenario: "cannot add output paths", ctx: func() *api.ContextMock { @@ -1381,38 +1449,16 @@ func TestConvertUrl(t *testing.T) { api: &ApiMock{PdfMock: func(ctx context.Context, logger *zap.Logger, url, outputPath string, options PdfOptions) error { return nil }}, - options: DefaultPdfOptions(), - expectError: false, - expectHttpError: false, - expectOutputPathsCount: 1, - }, - { - scenario: "error with metadata write", - ctx: &api.ContextMock{Context: new(api.Context)}, - api: &ApiMock{PdfMock: func(ctx context.Context, logger *zap.Logger, url, outputPath string, options PdfOptions) error { - return nil - }}, - engine: &gotenberg.PdfEngineMock{WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { - return fmt.Errorf("error writing metadata to %s: %w", "foo.pdf", errors.New("foo")) - }}, - options: DefaultPdfOptions(), - metadata: map[string]interface{}{ - "Creator": "foo", - "Producer": "bar", + engine: &gotenberg.PdfEngineMock{ + ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { + return nil + }, + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { + return nil + }, }, - expectError: true, - expectHttpError: false, - }, - { - scenario: "success with metadata write", - ctx: &api.ContextMock{Context: new(api.Context)}, - api: &ApiMock{PdfMock: func(ctx context.Context, logger *zap.Logger, url, outputPath string, options PdfOptions) error { - return nil - }}, - engine: &gotenberg.PdfEngineMock{WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { - return nil - }}, - options: DefaultPdfOptions(), + options: DefaultPdfOptions(), + pdfFormats: gotenberg.PdfFormats{PdfA: gotenberg.PdfA1b}, metadata: map[string]interface{}{ "Creator": "foo", "Producer": "bar", @@ -1424,7 +1470,7 @@ func TestConvertUrl(t *testing.T) { } { t.Run(tc.scenario, func(t *testing.T) { tc.ctx.SetLogger(zap.NewNop()) - err := convertUrl(tc.ctx.Context, tc.api, tc.engine, "", tc.pdfFormats, tc.options, tc.metadata) + err := convertUrl(tc.ctx.Context, tc.api, tc.engine, "", tc.options, tc.pdfFormats, tc.metadata) if tc.expectError && err == nil { t.Fatal("expected error but got none", err) diff --git a/pkg/modules/exiftool/exiftool.go b/pkg/modules/exiftool/exiftool.go index 6958f659..2191e7c8 100644 --- a/pkg/modules/exiftool/exiftool.go +++ b/pkg/modules/exiftool/exiftool.go @@ -4,59 +4,25 @@ import ( "context" "errors" "fmt" - "net/http" "os" "github.com/barasher/go-exiftool" "go.uber.org/zap" "github.com/gotenberg/gotenberg/v8/pkg/gotenberg" - "github.com/gotenberg/gotenberg/v8/pkg/modules/api" ) func init() { gotenberg.MustRegisterModule(new(ExifTool)) } -// MetadataValueTypeError is constructed when metadata value types cannot be processed. -// The underlying library used in this implementation supports the writing of a limited -// number of metadata value types. -// -// For example, according to https://exiftool.org/TagNames/PDF.html metadata can be a boolean, -// i.e. "value format... may be string, date, integer, real, boolean or name". -// Furthermore, a native boolean type is also supported by JSON and Go. However, the -// underlying library does not currently support writing native Go boolean (bool) types. -// Therefore, an instance of this struct is created when a boolean metadata entry is supplied. -// -// The struct contains a key/value map corresponding to individual invalid metadata entries supplied by a consumer. -// This allows a helpful error message to be produced for API consumers. -// See API.WriteMetadata for more information on valid metadata value types. -type MetadataValueTypeError struct { - Entries map[string]interface{} -} - -// Error returns a helpful error message. -func (e *MetadataValueTypeError) Error() string { - return fmt.Sprintf("invalid metadata value types supplied - identified by Entries: %s", e.Entries) -} - -// GetKeys returns an array of keys with corresponding invalid value types, -func (e *MetadataValueTypeError) GetKeys() []string { - keys := make([]string, len(e.Entries)) - i := 0 - for key := range e.Entries { - keys[i] = key - i++ - } - return keys -} - -// ExifTool abstracts the CLI tool ExifTool and implements the [gotenberg.PdfEngine] interface . +// ExifTool abstracts the CLI tool ExifTool and implements the +// [gotenberg.PdfEngine] interface. type ExifTool struct { binPath string } -// Descriptor returns ExifTool's module descriptor. +// Descriptor returns [ExifTool]'s module descriptor. func (engine *ExifTool) Descriptor() gotenberg.ModuleDescriptor { return gotenberg.ModuleDescriptor{ ID: "exiftool", @@ -64,9 +30,7 @@ func (engine *ExifTool) Descriptor() gotenberg.ModuleDescriptor { } } -// Provision sets the module properties. It returns an error if -// - the environment variable EXIFTOOL_BIN_PATH is not set -// - there is an error creating an instance of exiftool.ExifTool +// Provision sets the module properties. func (engine *ExifTool) Provision(ctx *gotenberg.Context) error { binPath, ok := os.LookupEnv("EXIFTOOL_BIN_PATH") if !ok { @@ -90,99 +54,82 @@ func (engine *ExifTool) Validate() error { // Merge is not available in this implementation. func (engine *ExifTool) Merge(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { - return fmt.Errorf("merge PDFs with LibreOffice: %w", gotenberg.ErrPdfEngineMethodNotSupported) + return fmt.Errorf("merge PDFs with ExifTool: %w", gotenberg.ErrPdfEngineMethodNotSupported) } // Convert is not available in this implementation. func (engine *ExifTool) Convert(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return fmt.Errorf("convert PDF to '%+v' with PDFtk: %w", formats, gotenberg.ErrPdfEngineMethodNotSupported) + return fmt.Errorf("convert PDF to '%+v' with ExifTool: %w", formats, gotenberg.ErrPdfEngineMethodNotSupported) } -// ReadMetadata reads the metadata of the given PDF files. -func (engine *ExifTool) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error { - logger.Debug(fmt.Sprintf("reading metadata of file: %s", inputPath)) - - exifTool, err := exiftool.NewExiftool() +// ReadMetadata extracts the metadata of a given PDF file. +func (engine *ExifTool) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + exifTool, err := exiftool.NewExiftool(exiftool.SetExiftoolBinaryPath(engine.binPath)) if err != nil { - fmt.Printf("Error intializing ExifTool: %v\n", err) - return err + return nil, fmt.Errorf("new ExifTool: %w", err) } - fileMetadataInfos := exifTool.ExtractMetadata([]string{inputPath}...) + defer func(exifTool *exiftool.Exiftool) { + err := exifTool.Close() + if err != nil { + logger.Error(fmt.Sprintf("close ExifTool: %v", err)) + } + }(exifTool) - if fileMetadataInfos[0].Err != nil { - return fmt.Errorf("error reading metadata to following file: %+v", fileMetadataInfos[0]) + fileMetadata := exifTool.ExtractMetadata(inputPath) + if fileMetadata[0].Err != nil { + return nil, fmt.Errorf("read metadata with ExitfTool: %w", fileMetadata[0].Err) } - // load into metadata - for k, v := range fileMetadataInfos[0].Fields { - metadata[k] = v - } - - return exifTool.Close() + return fileMetadata[0].Fields, nil } -// WriteMetadata write the metadata to the given PDF files. -func (engine *ExifTool) WriteMetadata(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { - logger.Debug(fmt.Sprintf("writing new metadata %s to %s", newMetadata, inputPath)) - - exifTool, err := exiftool.NewExiftool() +// WriteMetadata writes the metadata into a given PDF file. +func (engine *ExifTool) WriteMetadata(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { + exifTool, err := exiftool.NewExiftool(exiftool.SetExiftoolBinaryPath(engine.binPath)) if err != nil { - fmt.Printf("Error intializing ExifTool: %v\n", err) - return err + return fmt.Errorf("new ExifTool: %w", err) } - fileMetadataInfos := exifTool.ExtractMetadata([]string{inputPath}...) + defer func(exifTool *exiftool.Exiftool) { + err := exifTool.Close() + if err != nil { + logger.Error(fmt.Sprintf("close ExifTool: %v", err)) + } + }(exifTool) - // there is only file metadata info - if fileMetadataInfos[0].Err != nil { - return fmt.Errorf("error reading metadata to following file: %+v", fileMetadataInfos[0]) + fileMetadata := exifTool.ExtractMetadata(inputPath) + if fileMetadata[0].Err != nil { + return fmt.Errorf("read metadata with ExitfTool: %w", fileMetadata[0].Err) } - // Metadata values can only be specific value types. - // An error is returned if metadata with an invalid type is requested. - metadataValueErrors := MetadataValueTypeError{ - Entries: make(map[string]interface{}), - } - - // transform metadata - for key, value := range newMetadata { + for key, value := range metadata { switch val := value.(type) { case string: - fileMetadataInfos[0].SetString(key, val) + fileMetadata[0].SetString(key, val) case int: - fileMetadataInfos[0].SetInt(key, int64(val)) + fileMetadata[0].SetInt(key, int64(val)) case int64: - fileMetadataInfos[0].SetInt(key, val) + fileMetadata[0].SetInt(key, val) case float32: - fileMetadataInfos[0].SetFloat(key, float64(val)) + fileMetadata[0].SetFloat(key, float64(val)) case float64: - fileMetadataInfos[0].SetFloat(key, val) + fileMetadata[0].SetFloat(key, val) case []string: - fileMetadataInfos[0].SetStrings(key, val) - // TODO: support more complex cases, e.g. arrays and nested objects (limitations in underlying library) + fileMetadata[0].SetStrings(key, val) + // TODO: support more complex cases, e.g., arrays and nested objects + // (limitations in underlying library). default: - metadataValueErrors.Entries[key] = value + return fmt.Errorf("write PDF metadata with ExifTool: %w", gotenberg.ErrPdfEngineMetadataValueNotSupported) } } - logger.Debug(fmt.Sprintf("writing metadata %s to %s", fileMetadataInfos[0].Fields, fileMetadataInfos[0].File)) - if len(metadataValueErrors.Entries) > 0 { - return api.WrapError( - fmt.Errorf("write metadata: %w", err), - api.NewSentinelHttpError( - http.StatusBadRequest, - fmt.Sprintf("Invalid metdata value types supplied by keys '%s'", metadataValueErrors.GetKeys())), - ) + exifTool.WriteMetadata(fileMetadata) + if fileMetadata[0].Err != nil { + return fmt.Errorf("write PDF metadata with ExifTool: %w", fileMetadata[0].Err) } - exifTool.WriteMetadata(fileMetadataInfos) - - if fileMetadataInfos[0].Err != nil { - return fmt.Errorf("error writing metadata to following file: %+v", fileMetadataInfos[0]) - } - - return exifTool.Close() + return nil } // Interface guards. diff --git a/pkg/modules/exiftool/exiftool_test.go b/pkg/modules/exiftool/exiftool_test.go index eefa3b63..3bc4ba59 100644 --- a/pkg/modules/exiftool/exiftool_test.go +++ b/pkg/modules/exiftool/exiftool_test.go @@ -9,62 +9,11 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" "go.uber.org/zap" "github.com/gotenberg/gotenberg/v8/pkg/gotenberg" ) -func TestMetadataValueTypeError_Error(t *testing.T) { - instance := MetadataValueTypeError{ - Entries: map[string]interface{}{ - "foo": "foo", - }, - } - - assert.True(t, len(instance.Error()) > 0) -} - -func TestMetadataValueTypeError_GetKeys(t *testing.T) { - for i, tc := range []struct { - instance MetadataValueTypeError - expect []string - }{ - { - instance: MetadataValueTypeError{ - Entries: map[string]interface{}{}, - }, - expect: []string{}, - }, - { - instance: MetadataValueTypeError{ - Entries: map[string]interface{}{ - "foo": "foo", - }, - }, - expect: []string{"foo"}, - }, - { - instance: MetadataValueTypeError{ - Entries: map[string]interface{}{ - "foo": "foo", - "bar": float64(123), - "baz": 4.56, - "qux": true, - "quux": nil, - }, - }, - expect: []string{"foo", "bar", "baz", "qux", "quux"}, - }, - } { - actual := tc.instance.GetKeys() - - if !assert.ElementsMatch(t, actual, tc.expect) { - t.Errorf("test %d: expected %+v but got: %+v", i, tc.expect, actual) - } - } -} - func TestExifTool_Descriptor(t *testing.T) { descriptor := new(ExifTool).Descriptor() @@ -144,59 +93,28 @@ func TestExiftool_Convert(t *testing.T) { func TestExiftool_ReadMetadata(t *testing.T) { for _, tc := range []struct { - scenario string - ctx context.Context - inputPath string - subset map[string]interface{} - expectError bool - expectDiff bool + scenario string + inputPath string + expectMetadata map[string]interface{} + expectError bool }{ { - scenario: "invalid input path", - ctx: context.TODO(), - inputPath: "foo", - expectError: true, + scenario: "invalid input path", + inputPath: "foo", + expectMetadata: nil, + expectError: true, }, { - scenario: "single file success", - ctx: context.TODO(), + scenario: "success", inputPath: "/tests/test/testdata/pdfengines/sample1.pdf", - subset: map[string]interface{}{ + expectMetadata: map[string]interface{}{ "FileName": "sample1.pdf", "FileTypeExtension": "pdf", "MIMEType": "application/pdf", "PDFVersion": 1.4, "PageCount": float64(3), - "CreateDate": "2018:12:06 17:50:06+00:00", - "ModifyDate": "2018:12:06 17:50:06+00:00", - "Directory": "/tests/test/testdata/pdfengines", - "FileType": "PDF", - "Linearized": "No", - "Creator": "Chromium", - "Producer": "Skia/PDF m70", - "SourceFile": "/tests/test/testdata/pdfengines/sample1.pdf", }, - }, - { - scenario: "single file incorrect metadata", - ctx: context.TODO(), - inputPath: "/tests/test/testdata/pdfengines/sample1.pdf", - subset: map[string]interface{}{ - "FileName": "sample1.pdf", - "FileTypeExtension": "pdf", - "MIMEType": "application/pdf", - "PDFVersion": 1.4, - "PageCount": float64(3), - "CreateDate": "2018:12:06 17:50:06+00:00", - "ModifyDate": "2018:12:06 17:50:06+00:00", - "Directory": "/tests/test/testdata/pdfengines", - "FileType": "PDF", - "Linearized": "No", - "Creator": "INVALID", - "Producer": "Skia/PDF m70", - "SourceFile": "/tests/test/testdata/pdfengines/sample1.pdf", - }, - expectDiff: true, + expectError: false, }, } { t.Run(tc.scenario, func(t *testing.T) { @@ -206,8 +124,8 @@ func TestExiftool_ReadMetadata(t *testing.T) { t.Fatalf("expected error but got: %v", err) } - actualMetadata := map[string]interface{}{} - err = engine.ReadMetadata(tc.ctx, zap.NewNop(), tc.inputPath, actualMetadata) + metadata, err := engine.ReadMetadata(context.Background(), zap.NewNop(), tc.inputPath) + if !tc.expectError && err != nil { t.Fatalf("expected no error but got: %v", err) } @@ -216,13 +134,11 @@ func TestExiftool_ReadMetadata(t *testing.T) { t.Fatal("expected error but got none") } - if tc.subset != nil && err == nil { - if !tc.expectDiff && !isMapSubset(actualMetadata, tc.subset) { - t.Errorf("test: %s: expected: %+v to be a subset of: %+v at path: %s", - tc.scenario, tc.subset, actualMetadata, tc.inputPath) - } else if tc.expectDiff && isMapSubset(actualMetadata, tc.subset) { - t.Errorf("test: %s: expected: %+v to be not be a subset of: %+v at path: %s", - tc.scenario, tc.subset, actualMetadata, tc.inputPath) + if tc.expectMetadata != nil && err == nil { + for k, v := range tc.expectMetadata { + if v2, ok := metadata[k]; !ok || v != v2 { + t.Errorf("expected entry %s with value %v to exists", k, v) + } } } }) @@ -231,101 +147,133 @@ func TestExiftool_ReadMetadata(t *testing.T) { func TestExiftool_WriteMetadata(t *testing.T) { for _, tc := range []struct { - scenario string - ctx context.Context - inputPath string - newMetadata map[string]interface{} - contains map[string]interface{} - expectError bool - expectDiff bool + scenario string + createCopy bool + inputPath string + metadata map[string]interface{} + expectMetadata map[string]interface{} + expectError bool + expectedError error }{ { - scenario: "single file success", - ctx: context.TODO(), - inputPath: "/tests/test/testdata/pdfengines/sample1.pdf", - newMetadata: map[string]interface{}{ - "Producer": "foo", - }, - contains: map[string]interface{}{ - "Producer": "foo", - }, - expectError: false, - expectDiff: false, - }, - { - scenario: "single file not same metadata", - ctx: context.TODO(), - inputPath: "/tests/test/testdata/pdfengines/sample1.pdf", - newMetadata: map[string]interface{}{ - "Producer": "foo", - }, - contains: map[string]interface{}{ - "Producer": "foobar", - }, - expectError: false, - expectDiff: true, - }, - { - scenario: "single file unknown type", - ctx: context.TODO(), - inputPath: "/tests/test/testdata/pdfengines/sample1.pdf", - newMetadata: map[string]interface{}{ - "foo": map[string]string{}, - }, + scenario: "invalid input path", + createCopy: false, + inputPath: "foo", expectError: true, - expectDiff: false, + }, + { + scenario: "gotenberg.ErrPdfEngineMetadataValueNotSupported", + createCopy: true, + inputPath: "/tests/test/testdata/pdfengines/sample1.pdf", + metadata: map[string]interface{}{ + "Unsupported": map[string]interface{}{}, + }, + expectError: true, + expectedError: gotenberg.ErrPdfEngineMetadataValueNotSupported, + }, + { + scenario: "success", + createCopy: true, + inputPath: "/tests/test/testdata/pdfengines/sample1.pdf", + metadata: map[string]interface{}{ + "Author": "Julien Neuhart", + "Copyright": "Julien Neuhart", + "CreationDate": "2006-09-18T16:27:50-04:00", + "Creator": "Gotenberg", + "Keywords": []string{ + "first", + "second", + }, + "Marked": "true", + "ModDate": "2006-09-18T16:27:50-04:00", + "PDFVersion": 1.7, + "Producer": "Gotenberg", + "Subject": "Sample", + "Title": "Sample", + "Trapped": "Unknown", + // Those are not valid PDF metadata. + "int": 1, + "int64": int64(2), + "float32": float32(2.2), + "float64": 3.3, + }, + expectMetadata: map[string]interface{}{ + "Author": "Julien Neuhart", + "Copyright": "Julien Neuhart", + "CreationDate": "2006:09:18 16:27:50-04:00", + "Creator": "Gotenberg", + "Keywords": []interface{}{ + "first", + "second", + }, + "Marked": true, + "ModDate": "2006:09:18 16:27:50-04:00", + "PDFVersion": 1.7, + "Producer": "Gotenberg", + "Subject": "Sample", + "Title": "Sample", + "Trapped": "Unknown", + }, + expectError: false, }, } { t.Run(tc.scenario, func(t *testing.T) { engine := new(ExifTool) err := engine.Provision(nil) if err != nil { - t.Fatalf("expected error but got: %v", err) + t.Fatalf("expected no error but got: %v", err) } - fs := gotenberg.NewFileSystem() - outputDir, err := fs.MkdirAll() - if err != nil { - t.Fatalf("expected error but got: %v", err) - } - - defer func() { - err = os.RemoveAll(fs.WorkingDirPath()) + var destinationPath string + if tc.createCopy { + fs := gotenberg.NewFileSystem() + outputDir, err := fs.MkdirAll() if err != nil { - t.Fatalf("expected no error while cleaning up but got: %v", err) + t.Fatalf("expected error no but got: %v", err) } - }() - copyPath := fmt.Sprintf("%s/copy_temp.pdf", outputDir) - // open the source file - source, err := os.Open(tc.inputPath) - if err != nil { - t.Fatalf("error in opening file: %v", err) + defer func() { + err = os.RemoveAll(fs.WorkingDirPath()) + if err != nil { + t.Fatalf("expected no error while cleaning up but got: %v", err) + } + }() + + destinationPath = fmt.Sprintf("%s/copy_temp.pdf", outputDir) + source, err := os.Open(tc.inputPath) + if err != nil { + t.Fatalf("open source file: %v", err) + } + + defer func(source *os.File) { + err := source.Close() + if err != nil { + t.Fatalf("close file: %v", err) + } + }(source) + + destination, err := os.Create(destinationPath) + if err != nil { + t.Fatalf("create destination file: %v", err) + } + + defer func(destination *os.File) { + err := destination.Close() + if err != nil { + t.Fatalf("close file: %v", err) + } + }(destination) + + _, err = io.Copy(destination, source) + if err != nil { + t.Fatalf("copy source into destination: %v", err) + } + } else { + destinationPath = tc.inputPath } - // create the destination file - destination, err := os.Create(copyPath) - if err != nil { - t.Fatalf("error in creating file: %v", err) - } + err = engine.WriteMetadata(context.Background(), zap.NewNop(), tc.metadata, destinationPath) - // copy the contents of source to destination file - _, err = io.Copy(destination, source) - if err != nil { - t.Fatalf("error in copying file: %v", err) - } - - err = source.Close() - if err != nil { - t.Fatalf("error in source file close: %v", err) - } - err = destination.Close() - if err != nil { - t.Fatalf("error in destination file close: %v", err) - } - - // write metadata to new copy files - err = engine.WriteMetadata(tc.ctx, zap.NewNop(), copyPath, tc.newMetadata) if !tc.expectError && err != nil { t.Fatalf("expected no error but got: %v", err) } @@ -334,85 +282,41 @@ func TestExiftool_WriteMetadata(t *testing.T) { t.Fatal("expected error but got none") } - if err == nil { - readMetadata := map[string]interface{}{} - readErr := engine.ReadMetadata(tc.ctx, zap.NewNop(), copyPath, readMetadata) - if tc.contains != nil && readErr == nil { - // match metadata - if !tc.expectDiff && !isMapSubset(readMetadata, tc.contains) { - t.Errorf("test: %s: expected: %+v to be a subset of: %+v at path: %s", - tc.scenario, tc.contains, readMetadata, copyPath) - } else if tc.expectDiff && isMapSubset(readMetadata, tc.contains) { - t.Errorf("test: %s: expected: %+v to be not be a subset of: %+v at path: %s", - tc.scenario, tc.contains, readMetadata, copyPath) + if tc.expectError { + return + } + + if tc.expectedError != nil && !errors.Is(err, tc.expectedError) { + t.Fatalf("expected error %v but got: %v", tc.expectedError, err) + } + + metadata, err := engine.ReadMetadata(context.Background(), zap.NewNop(), destinationPath) + if err != nil { + t.Fatalf("expected no error but got: %v", err) + } + + if tc.expectMetadata != nil && err == nil { + for k, v := range tc.expectMetadata { + v2, ok := metadata[k] + if !ok { + t.Errorf("expected entry %s with value %v to exists, but got none", k, v) + continue + } + + switch v2.(type) { + case []interface{}: + for i, entry := range v.([]interface{}) { + if entry != v2.([]interface{})[i] { + t.Errorf("expected entry %s to contain value %v, but got %v", k, entry, v2.([]interface{})[i]) + } + } + default: + if v != v2 { + t.Errorf("expected entry %s with value %v to exists, but got %v", k, v, v2) + } } } } }) } } - -func isMapSubset(mapSet interface{}, mapSubset interface{}) bool { - mapSetValue := reflect.ValueOf(mapSet) - mapSubsetValue := reflect.ValueOf(mapSubset) - - if mapSetValue.Kind() != reflect.Map || mapSubsetValue.Kind() != reflect.Map { - return false - } - if reflect.TypeOf(mapSetValue) != reflect.TypeOf(mapSubsetValue) { - return false - } - if len(mapSubsetValue.MapKeys()) == 0 { - return true - } - - iterMapSubset := mapSubsetValue.MapRange() - - for iterMapSubset.Next() { - k := iterMapSubset.Key() - v := iterMapSubset.Value() - - v2 := mapSetValue.MapIndex(k) - - if !v2.IsValid() { - return false - } - if isValueKind(v, reflect.Slice) && isValueKind(v2, reflect.Slice) { - vSlice := convertSlice(v) - v2Slice := convertSlice(v2) - if !equal(vSlice, v2Slice) { - return false - } - } else if v.Interface() != v2.Interface() { - return false - } - } - - return true -} - -func isValueKind(value reflect.Value, kind reflect.Kind) bool { - return reflect.TypeOf(value.Interface()).Kind() == kind -} - -func convertSlice(value reflect.Value) []interface{} { - slice := make([]interface{}, reflect.ValueOf(value.Interface()).Len()) - for i := range slice { - slice = append(slice, reflect.ValueOf(value.Interface()).Index(i).Interface()) - } - return slice -} - -// equal tells whether a and b contain the same elements. -// A nil argument is equivalent to an empty slice. -func equal(a, b []interface{}) bool { - if len(a) != len(b) { - return false - } - for i, v := range a { - if v != b[i] { - return false - } - } - return true -} diff --git a/pkg/modules/libreoffice/pdfengine/pdfengine.go b/pkg/modules/libreoffice/pdfengine/pdfengine.go index 43776f4d..a1b77e45 100644 --- a/pkg/modules/libreoffice/pdfengine/pdfengine.go +++ b/pkg/modules/libreoffice/pdfengine/pdfengine.go @@ -72,12 +72,12 @@ func (engine *LibreOfficePdfEngine) Convert(ctx context.Context, logger *zap.Log } // ReadMetadata is not available in this implementation. -func (engine *LibreOfficePdfEngine) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPaths string, metadata map[string]interface{}) error { - return fmt.Errorf("read PDF metadata with LibreOffice: %w", gotenberg.ErrPdfEngineMethodNotSupported) +func (engine *LibreOfficePdfEngine) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return nil, fmt.Errorf("read PDF metadata with LibreOffice: %w", gotenberg.ErrPdfEngineMethodNotSupported) } // WriteMetadata is not available in this implementation. -func (engine *LibreOfficePdfEngine) WriteMetadata(ctx context.Context, logger *zap.Logger, inputPaths string, newMetadata map[string]interface{}) error { +func (engine *LibreOfficePdfEngine) WriteMetadata(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return fmt.Errorf("write PDF metadata with LibreOffice: %w", gotenberg.ErrPdfEngineMethodNotSupported) } diff --git a/pkg/modules/libreoffice/pdfengine/pdfengine_test.go b/pkg/modules/libreoffice/pdfengine/pdfengine_test.go index 1b0740da..8353954d 100644 --- a/pkg/modules/libreoffice/pdfengine/pdfengine_test.go +++ b/pkg/modules/libreoffice/pdfengine/pdfengine_test.go @@ -169,7 +169,7 @@ func TestLibreOfficePdfEngine_Convert(t *testing.T) { func TestLibreOfficePdfEngine_ReadMetadata(t *testing.T) { engine := new(LibreOfficePdfEngine) - err := engine.ReadMetadata(context.Background(), zap.NewNop(), "", nil) + _, err := engine.ReadMetadata(context.Background(), zap.NewNop(), "") if !errors.Is(err, gotenberg.ErrPdfEngineMethodNotSupported) { t.Errorf("expected error %v, but got: %v", gotenberg.ErrPdfEngineMethodNotSupported, err) @@ -178,7 +178,7 @@ func TestLibreOfficePdfEngine_ReadMetadata(t *testing.T) { func TestLibreOfficePdfEngine_WriteMetadata(t *testing.T) { engine := new(LibreOfficePdfEngine) - err := engine.WriteMetadata(context.Background(), zap.NewNop(), "", nil) + err := engine.WriteMetadata(context.Background(), zap.NewNop(), nil, "") if !errors.Is(err, gotenberg.ErrPdfEngineMethodNotSupported) { t.Errorf("expected error %v, but got: %v", gotenberg.ErrPdfEngineMethodNotSupported, err) diff --git a/pkg/modules/libreoffice/routes.go b/pkg/modules/libreoffice/routes.go index f16d0cdc..73b1f68f 100644 --- a/pkg/modules/libreoffice/routes.go +++ b/pkg/modules/libreoffice/routes.go @@ -46,11 +46,10 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap Bool("nativePdfFormats", &nativePdfFormats, true). Bool("merge", &merge, false). Custom("metadata", func(value string) error { - metadata = map[string]interface{}{} if len(value) > 0 { err := json.Unmarshal([]byte(value), &metadata) if err != nil { - return err + return fmt.Errorf("unmarshal metadata: %w", err) } } return nil @@ -102,8 +101,7 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap } } - // So far so good, let's check if we have to merge the PDFs. Quick - // win: if there is only one PDF, skip this step. + // So far so good, let's check if we have to merge the PDFs. if len(outputPaths) > 1 && merge { outputPath := ctx.GeneratePath(".pdf") @@ -112,42 +110,12 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap return fmt.Errorf("merge PDFs: %w", err) } - // Now, let's check if the client want to convert this - // resulting PDF to specific PDF formats. - zeroValued := gotenberg.PdfFormats{} - if !nativePdfFormats && pdfFormats != zeroValued { - convertInputPath := outputPath - convertOutputPath := ctx.GeneratePath(".pdf") - - err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPath) - if err != nil { - return fmt.Errorf("convert PDF: %w", err) - } - - // Important: the output path is now the converted file. - outputPath = convertOutputPath - } - - // Writes and potentially overrides metadata entries, if any. - if len(metadata) > 0 { - err = engine.WriteMetadata(ctx, ctx.Log(), outputPath, metadata) - if err != nil { - return fmt.Errorf("write metadata failure: %w", err) - } - } - - // Last but not least, add the output path to the context so that - // the API is able to send it as a response to the client. - err = ctx.AddOutputPaths(outputPath) - if err != nil { - return fmt.Errorf("add output path: %w", err) - } - - return nil + // Only one output path. + outputPaths = []string{outputPath} } - // Ok, we don't have to merge the PDFs. Let's check if the client - // want to convert each PDF to a specific PDF format. + // Let's check if the client want to convert each PDF to a specific + // PDF format. zeroValued := gotenberg.PdfFormats{} if !nativePdfFormats && pdfFormats != zeroValued { convertOutputPaths := make([]string, len(outputPaths)) @@ -166,6 +134,16 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap outputPaths = convertOutputPaths } + // Writes and potentially overrides metadata entries, if any. + if len(metadata) > 0 { + for _, outputPath := range outputPaths { + err = engine.WriteMetadata(ctx, ctx.Log(), metadata, outputPath) + if err != nil { + return fmt.Errorf("write metadata: %w", err) + } + } + } + if len(outputPaths) > 1 { // If .zip archive, document.docx -> document.docx.pdf. for i, inputPath := range inputPaths { @@ -180,16 +158,6 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap } } - // Writes and potentially overrides metadata entries, if any. - if len(metadata) > 0 { - for _, outputPath := range outputPaths { - err = engine.WriteMetadata(ctx, ctx.Log(), outputPath, metadata) - if err != nil { - return fmt.Errorf("write metadata: %w", err) - } - } - } - // Last but not least, add the output paths to the context so that // the API is able to send them as a response to the client. err = ctx.AddOutputPaths(outputPaths...) diff --git a/pkg/modules/libreoffice/routes_test.go b/pkg/modules/libreoffice/routes_test.go index 6b514780..1455569a 100644 --- a/pkg/modules/libreoffice/routes_test.go +++ b/pkg/modules/libreoffice/routes_test.go @@ -39,6 +39,28 @@ func TestConvertRoute(t *testing.T) { expectHttpStatus: http.StatusBadRequest, expectOutputPathsCount: 0, }, + { + scenario: "invalid metadata form field", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetFiles(map[string]string{ + "document.docx": "/document.docx", + }) + ctx.SetValues(map[string][]string{ + "metadata": { + "foo", + }, + }) + return ctx + }(), + libreOffice: &libreofficeapi.ApiMock{ExtensionsMock: func() []string { + return []string{".docx"} + }}, + expectError: true, + expectHttpError: true, + expectHttpStatus: http.StatusBadRequest, + expectOutputPathsCount: 0, + }, { scenario: "ErrPdfFormatNotSupported (nativePdfFormats)", ctx: func() *api.ContextMock { @@ -110,7 +132,39 @@ func TestConvertRoute(t *testing.T) { expectOutputPathsCount: 0, }, { - scenario: "PDF engine convert error (single file)", + scenario: "PDF engine merge error", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetFiles(map[string]string{ + "document.docx": "/document.docx", + "document2.docx": "/document2.docx", + }) + ctx.SetValues(map[string][]string{ + "merge": { + "true", + }, + }) + return ctx + }(), + libreOffice: &libreofficeapi.ApiMock{ + PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { + return nil + }, + ExtensionsMock: func() []string { + return []string{".docx"} + }, + }, + engine: &gotenberg.PdfEngineMock{ + MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { + return errors.New("foo") + }, + }, + expectError: true, + expectHttpError: false, + expectOutputPathsCount: 0, + }, + { + scenario: "PDF engine convert error", ctx: func() *api.ContextMock { ctx := &api.ContextMock{Context: new(api.Context)} ctx.SetFiles(map[string]string{ @@ -144,13 +198,17 @@ func TestConvertRoute(t *testing.T) { expectOutputPathsCount: 0, }, { - scenario: "cannot add output paths (single file)", + scenario: "PDF engine write metadata error", ctx: func() *api.ContextMock { ctx := &api.ContextMock{Context: new(api.Context)} ctx.SetFiles(map[string]string{ "document.docx": "/document.docx", }) - ctx.SetCancelled(true) + ctx.SetValues(map[string][]string{ + "metadata": { + "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", + }, + }) return ctx }(), libreOffice: &libreofficeapi.ApiMock{ @@ -161,31 +219,15 @@ func TestConvertRoute(t *testing.T) { return []string{".docx"} }, }, + engine: &gotenberg.PdfEngineMock{ + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { + return errors.New("foo") + }, + }, expectError: true, expectHttpError: false, expectOutputPathsCount: 0, }, - { - scenario: "success (single file)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - expectError: false, - expectHttpError: false, - expectOutputPathsCount: 1, - }, { scenario: "cannot rename many files", ctx: func() *api.ContextMock { @@ -212,6 +254,78 @@ func TestConvertRoute(t *testing.T) { expectHttpError: false, expectOutputPathsCount: 0, }, + { + scenario: "cannot add output paths", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetFiles(map[string]string{ + "document.docx": "/document.docx", + }) + ctx.SetCancelled(true) + return ctx + }(), + libreOffice: &libreofficeapi.ApiMock{ + PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { + return nil + }, + ExtensionsMock: func() []string { + return []string{".docx"} + }, + }, + expectError: true, + expectHttpError: false, + expectOutputPathsCount: 0, + }, + { + scenario: "success (single file)", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetFiles(map[string]string{ + "document.docx": "/document.docx", + "document2.docx": "/document2.docx", + }) + ctx.SetValues(map[string][]string{ + "merge": { + "true", + }, + "pdfa": { + gotenberg.PdfA1b, + }, + "pdfua": { + "true", + }, + "nativePdfFormats": { + "false", + }, + "metadata": { + "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", + }, + }) + return ctx + }(), + libreOffice: &libreofficeapi.ApiMock{ + PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { + return nil + }, + ExtensionsMock: func() []string { + return []string{".docx"} + }, + }, + engine: &gotenberg.PdfEngineMock{ + ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { + return nil + }, + MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { + return nil + }, + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { + return nil + }, + }, + expectError: false, + expectHttpError: false, + expectOutputPathsCount: 1, + }, { scenario: "success (many files)", ctx: func() *api.ContextMock { @@ -221,6 +335,20 @@ func TestConvertRoute(t *testing.T) { "document2.docx": "/document2.docx", "document2.doc": "/document2.doc", }) + ctx.SetValues(map[string][]string{ + "pdfa": { + gotenberg.PdfA1b, + }, + "pdfua": { + "true", + }, + "nativePdfFormats": { + "false", + }, + "metadata": { + "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", + }, + }) ctx.SetPathRename(&gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error { return nil }}) @@ -234,328 +362,35 @@ func TestConvertRoute(t *testing.T) { return []string{".docx", ".doc"} }, }, + engine: &gotenberg.PdfEngineMock{ + ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { + return nil + }, + MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { + return nil + }, + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { + return nil + }, + }, expectError: false, expectHttpError: false, expectOutputPathsCount: 3, expectOutputPaths: []string{"/document.docx.pdf", "/document2.docx.pdf", "/document2.doc.pdf"}, }, { - scenario: "success with non-native PDF/A & PDF/UA (many files)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "pdfa": { - gotenberg.PdfA1b, - }, - "pdfua": { - "true", - }, - "nativePdfFormats": { - "false", - }, - }) - ctx.SetPathRename(&gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error { - return nil - }}) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return nil - }, - }, - expectError: false, - expectHttpError: false, - expectOutputPathsCount: 2, - expectOutputPaths: []string{"/document.docx.pdf", "/document2.docx.pdf"}, - }, - { - scenario: "success with native PDF/A & PDF/UA (many files)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "pdfa": { - gotenberg.PdfA1b, - }, - "pdfua": { - "true", - }, - }) - ctx.SetPathRename(&gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error { - return nil - }}) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return nil - }, - }, - expectError: false, - expectHttpError: false, - expectOutputPathsCount: 2, - expectOutputPaths: []string{"/document.docx.pdf", "/document2.docx.pdf"}, - }, - { - scenario: "merge error", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "merge": { - "true", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { - return errors.New("foo") - }, - }, - expectError: true, - expectHttpError: false, - expectOutputPathsCount: 0, - }, - { - scenario: "PDF engine convert error (merge)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "merge": { - "true", - }, - "pdfa": { - gotenberg.PdfA1b, - }, - "nativePdfFormats": { - "false", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { - return nil - }, - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return errors.New("foo") - }, - }, - expectError: true, - expectHttpError: false, - expectOutputPathsCount: 0, - }, - { - scenario: "cannot add output paths (merge)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "merge": { - "true", - }, - }) - ctx.SetCancelled(true) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { - return nil - }, - }, - expectError: true, - expectHttpError: false, - expectOutputPathsCount: 0, - }, - { - scenario: "success (merge)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "merge": { - "true", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { - return nil - }, - }, - expectError: false, - expectHttpError: false, - expectOutputPathsCount: 1, - }, - { - scenario: "success with non-native PDF/A & PDF/UA (merge)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "merge": { - "true", - }, - "pdfa": { - gotenberg.PdfA1b, - }, - "pdfua": { - "true", - }, - "nativePdfFormats": { - "false", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { - return nil - }, - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return nil - }, - }, - expectError: false, - expectHttpError: false, - expectOutputPathsCount: 1, - }, - { - scenario: "success with non-native PDF/A & PDF/UA (merge)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "merge": { - "true", - }, - "pdfa": { - gotenberg.PdfA1b, - }, - "pdfua": { - "true", - }, - "nativePdfFormats": { - "false", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { - return nil - }, - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return nil - }, - }, - expectError: false, - expectHttpError: false, - expectOutputPathsCount: 1, - }, - { - scenario: "success with metadata (single file)", + scenario: "success with native PDF/A & PDF/UA", ctx: func() *api.ContextMock { ctx := &api.ContextMock{Context: new(api.Context)} ctx.SetFiles(map[string]string{ "document.docx": "/document.docx", }) ctx.SetValues(map[string][]string{ - "metadata": { - "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", + "pdfa": { + gotenberg.PdfA1b, + }, + "pdfua": { + "true", }, }) return ctx @@ -568,293 +403,10 @@ func TestConvertRoute(t *testing.T) { return []string{".docx"} }, }, - engine: &gotenberg.PdfEngineMock{ - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return nil - }, - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { - return nil - }, - }, expectError: false, expectHttpError: false, expectOutputPathsCount: 1, }, - { - scenario: "success with metadata (many files)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "metadata": { - "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return nil - }, - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { - return nil - }, - }, - expectError: false, - expectHttpError: false, - expectOutputPathsCount: 2, - }, - { - scenario: "error with metadata (single file)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - }) - ctx.SetValues(map[string][]string{ - "metadata": { - // invalid json - "{\"Creator\"", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - expectError: true, - expectHttpError: true, - expectHttpStatus: http.StatusBadRequest, - expectOutputPathsCount: 0, - }, - { - scenario: "error with metadata (many files)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "metadata": { - // invalid json - "{\"Creator\"", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - expectError: true, - expectHttpError: true, - expectHttpStatus: http.StatusBadRequest, - expectOutputPathsCount: 0, - }, - { - scenario: "error with metadata write failure (single file)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - }) - ctx.SetValues(map[string][]string{ - "metadata": { - "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return nil - }, - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { - return errors.New("foo") - }, - }, - expectError: true, - expectHttpError: false, - expectOutputPathsCount: 0, - }, - { - scenario: "error with metadata write failure (many files)", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "metadata": { - "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return nil - }, - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { - return errors.New("foo") - }, - }, - expectError: true, - expectHttpError: false, - expectOutputPathsCount: 0, - }, - { - scenario: "success merge with metadata", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "merge": { - "true", - }, - "metadata": { - "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { - return nil - }, - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return nil - }, - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { - return nil - }, - }, - expectError: false, - expectHttpError: false, - expectOutputPathsCount: 1, - }, - { - scenario: "error merge with metadata", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "merge": { - "true", - }, - "metadata": { - // invalid json - "{\"Creator", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - expectError: true, - expectHttpError: true, - expectHttpStatus: http.StatusBadRequest, - expectOutputPathsCount: 0, - }, - { - scenario: "error merge with metadata write failure", - ctx: func() *api.ContextMock { - ctx := &api.ContextMock{Context: new(api.Context)} - ctx.SetFiles(map[string]string{ - "document.docx": "/document.docx", - "document2.docx": "/document2.docx", - }) - ctx.SetValues(map[string][]string{ - "merge": { - "true", - }, - "metadata": { - "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", - }, - }) - return ctx - }(), - libreOffice: &libreofficeapi.ApiMock{ - PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return nil - }, - ExtensionsMock: func() []string { - return []string{".docx"} - }, - }, - engine: &gotenberg.PdfEngineMock{ - MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error { - return nil - }, - ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error { - return nil - }, - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { - return errors.New("foo") - }, - }, - expectError: true, - expectHttpError: false, - expectOutputPathsCount: 0, - }, } { t.Run(tc.scenario, func(t *testing.T) { tc.ctx.SetLogger(zap.NewNop()) diff --git a/pkg/modules/pdfcpu/pdfcpu.go b/pkg/modules/pdfcpu/pdfcpu.go index 1e2bf1a7..b07f9f08 100644 --- a/pkg/modules/pdfcpu/pdfcpu.go +++ b/pkg/modules/pdfcpu/pdfcpu.go @@ -55,12 +55,12 @@ func (engine *PdfCpu) Convert(ctx context.Context, logger *zap.Logger, formats g } // ReadMetadata is not available in this implementation. -func (engine *PdfCpu) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPaths string, metadata map[string]interface{}) error { - return fmt.Errorf("read PDF metadata with PDFcpu: %w", gotenberg.ErrPdfEngineMethodNotSupported) +func (engine *PdfCpu) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return nil, fmt.Errorf("read PDF metadata with PDFcpu: %w", gotenberg.ErrPdfEngineMethodNotSupported) } // WriteMetadata is not available in this implementation. -func (engine *PdfCpu) WriteMetadata(ctx context.Context, logger *zap.Logger, inputPaths string, newMetadata map[string]interface{}) error { +func (engine *PdfCpu) WriteMetadata(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return fmt.Errorf("write PDF metadata with PDFcpu: %w", gotenberg.ErrPdfEngineMethodNotSupported) } diff --git a/pkg/modules/pdfcpu/pdfcpu_test.go b/pkg/modules/pdfcpu/pdfcpu_test.go index a189521c..6172fe2f 100644 --- a/pkg/modules/pdfcpu/pdfcpu_test.go +++ b/pkg/modules/pdfcpu/pdfcpu_test.go @@ -105,7 +105,7 @@ func TestPdfCpu_Convert(t *testing.T) { func TestLibreOfficePdfEngine_ReadMetadata(t *testing.T) { engine := new(PdfCpu) - err := engine.ReadMetadata(context.Background(), zap.NewNop(), "", nil) + _, err := engine.ReadMetadata(context.Background(), zap.NewNop(), "") if !errors.Is(err, gotenberg.ErrPdfEngineMethodNotSupported) { t.Errorf("expected error %v, but got: %v", gotenberg.ErrPdfEngineMethodNotSupported, err) @@ -114,7 +114,7 @@ func TestLibreOfficePdfEngine_ReadMetadata(t *testing.T) { func TestLibreOfficePdfEngine_WriteMetadata(t *testing.T) { engine := new(PdfCpu) - err := engine.WriteMetadata(context.Background(), zap.NewNop(), "", nil) + err := engine.WriteMetadata(context.Background(), zap.NewNop(), nil, "") if !errors.Is(err, gotenberg.ErrPdfEngineMethodNotSupported) { t.Errorf("expected error %v, but got: %v", gotenberg.ErrPdfEngineMethodNotSupported, err) diff --git a/pkg/modules/pdfengines/multi.go b/pkg/modules/pdfengines/multi.go index 81c9b67a..d948b413 100644 --- a/pkg/modules/pdfengines/multi.go +++ b/pkg/modules/pdfengines/multi.go @@ -3,6 +3,7 @@ package pdfengines import ( "context" "fmt" + "sync" "go.uber.org/multierr" "go.uber.org/zap" @@ -70,36 +71,49 @@ func (multi *multiPdfEngines) Convert(ctx context.Context, logger *zap.Logger, f return fmt.Errorf("convert PDF to '%+v' with multi PDF engines: %w", formats, err) } -func (multi *multiPdfEngines) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPaths string, metadata map[string]interface{}) error { +type readMetadataResult struct { + metadata map[string]interface{} + err error +} + +func (multi *multiPdfEngines) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { var err error - errChan := make(chan error, 1) + var mu sync.Mutex // to safely append errors. + + resultChan := make(chan readMetadataResult, len(multi.engines)) for _, engine := range multi.engines { go func(engine gotenberg.PdfEngine) { - errChan <- engine.ReadMetadata(ctx, logger, inputPaths, metadata) + metadata, err := engine.ReadMetadata(ctx, logger, inputPath) + resultChan <- readMetadataResult{metadata: metadata, err: err} }(engine) + } + for range multi.engines { select { - case readMetadataErr := <-errChan: - errored := multierr.AppendInto(&err, readMetadataErr) - if !errored { - return nil + case result := <-resultChan: + if result.err != nil { + mu.Lock() + err = multierr.Append(err, result.err) + mu.Unlock() + } else { + return result.metadata, nil } case <-ctx.Done(): - return ctx.Err() + return nil, ctx.Err() } } - return fmt.Errorf("read PDF metadata with multi PDF engines: %w", err) + return nil, fmt.Errorf("read PDF metadata with multi PDF engines: %w", err) } -func (multi *multiPdfEngines) WriteMetadata(ctx context.Context, logger *zap.Logger, inputPaths string, newMetadata map[string]interface{}) error { +func (multi *multiPdfEngines) WriteMetadata(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { var err error errChan := make(chan error, 1) for _, engine := range multi.engines { go func(engine gotenberg.PdfEngine) { - errChan <- engine.WriteMetadata(ctx, logger, inputPaths, newMetadata) + errChan <- engine.WriteMetadata(ctx, logger, metadata, inputPath) }(engine) select { diff --git a/pkg/modules/pdfengines/multi_test.go b/pkg/modules/pdfengines/multi_test.go index 038bb5ea..3a204b70 100644 --- a/pkg/modules/pdfengines/multi_test.go +++ b/pkg/modules/pdfengines/multi_test.go @@ -189,8 +189,8 @@ func TestMultiPdfEngines_ReadMetadata(t *testing.T) { scenario: "nominal behavior", engine: newMultiPdfEngines( &gotenberg.PdfEngineMock{ - ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error { - return nil + ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return make(map[string]interface{}), nil }, }, ), @@ -200,13 +200,13 @@ func TestMultiPdfEngines_ReadMetadata(t *testing.T) { scenario: "at least one engine does not return an error", engine: newMultiPdfEngines( &gotenberg.PdfEngineMock{ - ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error { - return errors.New("foo") + ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return nil, errors.New("foo") }, }, &gotenberg.PdfEngineMock{ - ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error { - return nil + ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return make(map[string]interface{}), nil }, }, ), @@ -216,13 +216,13 @@ func TestMultiPdfEngines_ReadMetadata(t *testing.T) { scenario: "all engines return an error", engine: newMultiPdfEngines( &gotenberg.PdfEngineMock{ - ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error { - return errors.New("foo") + ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return nil, errors.New("foo") }, }, &gotenberg.PdfEngineMock{ - ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error { - return errors.New("foo") + ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return nil, errors.New("foo") }, }, ), @@ -233,8 +233,8 @@ func TestMultiPdfEngines_ReadMetadata(t *testing.T) { scenario: "context expired", engine: newMultiPdfEngines( &gotenberg.PdfEngineMock{ - ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, metadata map[string]interface{}) error { - return nil + ReadMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return make(map[string]interface{}), nil }, }, ), @@ -248,7 +248,7 @@ func TestMultiPdfEngines_ReadMetadata(t *testing.T) { }, } { t.Run(tc.scenario, func(t *testing.T) { - err := tc.engine.ReadMetadata(tc.ctx, zap.NewNop(), "", nil) + _, err := tc.engine.ReadMetadata(tc.ctx, zap.NewNop(), "") if !tc.expectError && err != nil { t.Fatalf("expected no error but got: %v", err) @@ -272,7 +272,7 @@ func TestMultiPdfEngines_WriteMetadata(t *testing.T) { scenario: "nominal behavior", engine: newMultiPdfEngines( &gotenberg.PdfEngineMock{ - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return nil }, }, @@ -283,12 +283,12 @@ func TestMultiPdfEngines_WriteMetadata(t *testing.T) { scenario: "at least one engine does not return an error", engine: newMultiPdfEngines( &gotenberg.PdfEngineMock{ - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return errors.New("foo") }, }, &gotenberg.PdfEngineMock{ - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return nil }, }, @@ -299,12 +299,12 @@ func TestMultiPdfEngines_WriteMetadata(t *testing.T) { scenario: "all engines return an error", engine: newMultiPdfEngines( &gotenberg.PdfEngineMock{ - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return errors.New("foo") }, }, &gotenberg.PdfEngineMock{ - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return errors.New("foo") }, }, @@ -316,7 +316,7 @@ func TestMultiPdfEngines_WriteMetadata(t *testing.T) { scenario: "context expired", engine: newMultiPdfEngines( &gotenberg.PdfEngineMock{ - WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, inputPath string, newMetadata map[string]interface{}) error { + WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return nil }, }, @@ -331,7 +331,7 @@ func TestMultiPdfEngines_WriteMetadata(t *testing.T) { }, } { t.Run(tc.scenario, func(t *testing.T) { - err := tc.engine.WriteMetadata(tc.ctx, zap.NewNop(), "", nil) + err := tc.engine.WriteMetadata(tc.ctx, zap.NewNop(), nil, "") if !tc.expectError && err != nil { t.Fatalf("expected no error but got: %v", err) diff --git a/pkg/modules/pdftk/pdftk.go b/pkg/modules/pdftk/pdftk.go index a9aaf0db..571b8016 100644 --- a/pkg/modules/pdftk/pdftk.go +++ b/pkg/modules/pdftk/pdftk.go @@ -76,12 +76,12 @@ func (engine *PdfTk) Convert(ctx context.Context, logger *zap.Logger, formats go } // ReadMetadata is not available in this implementation. -func (engine *PdfTk) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPaths string, metadata map[string]interface{}) error { - return fmt.Errorf("read PDF metadata with PDFtk: %w", gotenberg.ErrPdfEngineMethodNotSupported) +func (engine *PdfTk) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return nil, fmt.Errorf("read PDF metadata with PDFtk: %w", gotenberg.ErrPdfEngineMethodNotSupported) } // WriteMetadata is not available in this implementation. -func (engine *PdfTk) WriteMetadata(ctx context.Context, logger *zap.Logger, inputPaths string, newMetadata map[string]interface{}) error { +func (engine *PdfTk) WriteMetadata(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return fmt.Errorf("write PDF metadata with PDFtk: %w", gotenberg.ErrPdfEngineMethodNotSupported) } diff --git a/pkg/modules/pdftk/pdftk_test.go b/pkg/modules/pdftk/pdftk_test.go index 9d3b86e0..bdc0a508 100644 --- a/pkg/modules/pdftk/pdftk_test.go +++ b/pkg/modules/pdftk/pdftk_test.go @@ -151,7 +151,7 @@ func TestPdfTk_Convert(t *testing.T) { func TestLibreOfficePdfEngine_ReadMetadata(t *testing.T) { engine := new(PdfTk) - err := engine.ReadMetadata(context.Background(), zap.NewNop(), "", nil) + _, err := engine.ReadMetadata(context.Background(), zap.NewNop(), "") if !errors.Is(err, gotenberg.ErrPdfEngineMethodNotSupported) { t.Errorf("expected error %v, but got: %v", gotenberg.ErrPdfEngineMethodNotSupported, err) @@ -160,7 +160,7 @@ func TestLibreOfficePdfEngine_ReadMetadata(t *testing.T) { func TestLibreOfficePdfEngine_WriteMetadata(t *testing.T) { engine := new(PdfTk) - err := engine.WriteMetadata(context.Background(), zap.NewNop(), "", nil) + err := engine.WriteMetadata(context.Background(), zap.NewNop(), nil, "") if !errors.Is(err, gotenberg.ErrPdfEngineMethodNotSupported) { t.Errorf("expected error %v, but got: %v", gotenberg.ErrPdfEngineMethodNotSupported, err) diff --git a/pkg/modules/qpdf/qpdf.go b/pkg/modules/qpdf/qpdf.go index c65af8d9..57698281 100644 --- a/pkg/modules/qpdf/qpdf.go +++ b/pkg/modules/qpdf/qpdf.go @@ -78,12 +78,12 @@ func (engine *QPdf) Convert(ctx context.Context, logger *zap.Logger, formats got } // ReadMetadata is not available in this implementation. -func (engine *QPdf) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPaths string, metadata map[string]interface{}) error { - return fmt.Errorf("read PDF metadata with QPDF: %w", gotenberg.ErrPdfEngineMethodNotSupported) +func (engine *QPdf) ReadMetadata(ctx context.Context, logger *zap.Logger, inputPath string) (map[string]interface{}, error) { + return nil, fmt.Errorf("read PDF metadata with QPDF: %w", gotenberg.ErrPdfEngineMethodNotSupported) } // WriteMetadata is not available in this implementation. -func (engine *QPdf) WriteMetadata(ctx context.Context, logger *zap.Logger, inputPaths string, newMetadata map[string]interface{}) error { +func (engine *QPdf) WriteMetadata(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return fmt.Errorf("write PDF metadata with QPDF: %w", gotenberg.ErrPdfEngineMethodNotSupported) } diff --git a/pkg/modules/qpdf/qpdf_test.go b/pkg/modules/qpdf/qpdf_test.go index ef0fe174..a17bafb5 100644 --- a/pkg/modules/qpdf/qpdf_test.go +++ b/pkg/modules/qpdf/qpdf_test.go @@ -151,7 +151,7 @@ func TestQPdf_Convert(t *testing.T) { func TestLibreOfficePdfEngine_ReadMetadata(t *testing.T) { engine := new(QPdf) - err := engine.ReadMetadata(context.Background(), zap.NewNop(), "", nil) + _, err := engine.ReadMetadata(context.Background(), zap.NewNop(), "") if !errors.Is(err, gotenberg.ErrPdfEngineMethodNotSupported) { t.Errorf("expected error %v, but got: %v", gotenberg.ErrPdfEngineMethodNotSupported, err) @@ -160,7 +160,7 @@ func TestLibreOfficePdfEngine_ReadMetadata(t *testing.T) { func TestLibreOfficePdfEngine_WriteMetadata(t *testing.T) { engine := new(QPdf) - err := engine.WriteMetadata(context.Background(), zap.NewNop(), "", nil) + err := engine.WriteMetadata(context.Background(), zap.NewNop(), nil, "") if !errors.Is(err, gotenberg.ErrPdfEngineMethodNotSupported) { t.Errorf("expected error %v, but got: %v", gotenberg.ErrPdfEngineMethodNotSupported, err)