From 1d1b4fb04d6c18be449865e8ed29b52dccebfe99 Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Tue, 28 Jan 2025 17:15:14 +0100 Subject: [PATCH] feat(pdfengines): add flatten form field to split route --- pkg/gotenberg/pdfengine.go | 7 ++- pkg/modules/pdfengines/multi.go | 5 +- pkg/modules/pdfengines/routes.go | 17 +++-- pkg/modules/pdfengines/routes_test.go | 91 ++++++++++++++++++++++++--- pkg/modules/qpdf/doc.go | 1 + pkg/modules/qpdf/qpdf.go | 3 +- 6 files changed, 102 insertions(+), 22 deletions(-) diff --git a/pkg/gotenberg/pdfengine.go b/pkg/gotenberg/pdfengine.go index dc4a4f7c..02c55a41 100644 --- a/pkg/gotenberg/pdfengine.go +++ b/pkg/gotenberg/pdfengine.go @@ -98,9 +98,10 @@ type PdfEngine interface { // Split splits a given PDF file. Split(ctx context.Context, logger *zap.Logger, mode SplitMode, inputPath, outputDirPath string) ([]string, error) - // Flatten merges existing annotation appearances with page content, effectively deleting the original annotations. - // This process can flatten forms as well, as forms share a relationship with annotations. - // Note that this operation is irreversible. + // Flatten merges existing annotation appearances with page content, + // effectively deleting the original annotations. This process can flatten + // forms as well, as forms share a relationship with annotations. Note that + // this operation is irreversible. Flatten(ctx context.Context, logger *zap.Logger, inputPath string) error // Convert transforms a given PDF to the specified formats defined in diff --git a/pkg/modules/pdfengines/multi.go b/pkg/modules/pdfengines/multi.go index 20ae2cdc..b56ecef3 100644 --- a/pkg/modules/pdfengines/multi.go +++ b/pkg/modules/pdfengines/multi.go @@ -101,9 +101,8 @@ func (multi *multiPdfEngines) Split(ctx context.Context, logger *zap.Logger, mod return nil, fmt.Errorf("split PDF with multi PDF engines: %w", err) } -// Flatten merges existing annotation appearances with page content, effectively deleting the original annotations. -// This process can flatten forms as well, as forms share a relationship with annotations. -// Note that this operation is irreversible. +// Flatten merges existing annotation appearances with page content, thanks to +// its children. If the context is done, it stops and returns an error func (multi *multiPdfEngines) Flatten(ctx context.Context, logger *zap.Logger, inputPath string) error { var err error errChan := make(chan error, 1) diff --git a/pkg/modules/pdfengines/routes.go b/pkg/modules/pdfengines/routes.go index d90dd65e..ac800a23 100644 --- a/pkg/modules/pdfengines/routes.go +++ b/pkg/modules/pdfengines/routes.go @@ -202,10 +202,8 @@ func SplitPdfStub(ctx *api.Context, engine gotenberg.PdfEngine, mode gotenberg.S return outputPaths, nil } -// FlattenStub merges annotation appearances with page content for each given PDF -// in the input paths, effectively deleting the original annotations. It generates -// new output paths for the flattened PDFs and returns them. If an error occurs -// during the flattening process, it returns the error. +// FlattenStub merges annotation appearances with page content for each given +// PDF, effectively deleting the original annotations. func FlattenStub(ctx *api.Context, engine gotenberg.PdfEngine, inputPaths []string) error { for _, inputPath := range inputPaths { err := engine.Flatten(ctx, ctx.Log(), inputPath) @@ -327,8 +325,10 @@ func splitRoute(engine gotenberg.PdfEngine) api.Route { metadata := FormDataPdfMetadata(form, false) var inputPaths []string + var flatten bool err := form. MandatoryPaths([]string{".pdf"}, &inputPaths). + Bool("flatten", &flatten, false). Validate() if err != nil { return fmt.Errorf("validate form data: %w", err) @@ -349,6 +349,13 @@ func splitRoute(engine gotenberg.PdfEngine) api.Route { return fmt.Errorf("write metadata: %w", err) } + if flatten { + err = FlattenStub(ctx, engine, convertOutputPaths) + if err != nil { + return fmt.Errorf("flatten PDFs: %w", err) + } + } + zeroValuedSplitMode := gotenberg.SplitMode{} zeroValuedPdfFormats := gotenberg.PdfFormats{} if mode != zeroValuedSplitMode && pdfFormats != zeroValuedPdfFormats { @@ -392,7 +399,7 @@ func flattenRoute(engine gotenberg.PdfEngine) api.Route { err = FlattenStub(ctx, engine, inputPaths) if err != nil { - return fmt.Errorf("convert PDFs: %w", err) + return fmt.Errorf("flatten PDFs: %w", err) } err = ctx.AddOutputPaths(inputPaths...) diff --git a/pkg/modules/pdfengines/routes_test.go b/pkg/modules/pdfengines/routes_test.go index 405b5d05..c50d470e 100644 --- a/pkg/modules/pdfengines/routes_test.go +++ b/pkg/modules/pdfengines/routes_test.go @@ -503,6 +503,45 @@ func TestSplitPdfStub(t *testing.T) { } } +func TestFlattenStub(t *testing.T) { + for _, tc := range []struct { + scenario string + engine gotenberg.PdfEngine + expectError bool + }{ + { + scenario: "flatten error", + engine: &gotenberg.PdfEngineMock{ + FlattenMock: func(ctx context.Context, logger *zap.Logger, inputPath string) error { + return errors.New("foo") + }, + }, + expectError: true, + }, + { + scenario: "flatten success", + engine: &gotenberg.PdfEngineMock{ + FlattenMock: func(ctx context.Context, logger *zap.Logger, inputPath string) error { + return nil + }, + }, + expectError: false, + }, + } { + t.Run(tc.scenario, func(t *testing.T) { + err := FlattenStub(new(api.Context), tc.engine, []string{"my.pdf", "my2.pdf"}) + + if tc.expectError && err == nil { + t.Fatal("expected error but got none", err) + } + + if !tc.expectError && err != nil { + t.Fatalf("expected no error but got: %v", err) + } + }) + } +} + func TestConvertStub(t *testing.T) { for _, tc := range []struct { scenario string @@ -647,7 +686,7 @@ func TestMergeHandler(t *testing.T) { expectOutputPathsCount: 0, }, { - scenario: "PDF engine merge error", + scenario: "error from PDF engine (merge)", ctx: func() *api.ContextMock { ctx := &api.ContextMock{Context: new(api.Context)} ctx.SetFiles(map[string]string{ @@ -666,7 +705,7 @@ func TestMergeHandler(t *testing.T) { expectOutputPathsCount: 0, }, { - scenario: "PDF engine convert error", + scenario: "error from PDF engine (convert)", ctx: func() *api.ContextMock { ctx := &api.ContextMock{Context: new(api.Context)} ctx.SetFiles(map[string]string{ @@ -693,7 +732,7 @@ func TestMergeHandler(t *testing.T) { expectOutputPathsCount: 0, }, { - scenario: "PDF engine write metadata error", + scenario: "error from PDF engine (write metadata)", ctx: func() *api.ContextMock { ctx := &api.ContextMock{Context: new(api.Context)} ctx.SetFiles(map[string]string{ @@ -720,7 +759,7 @@ func TestMergeHandler(t *testing.T) { expectOutputPathsCount: 0, }, { - scenario: "PDF engine flatten error", + scenario: "error from PDF engine (flatten)", ctx: func() *api.ContextMock { ctx := &api.ContextMock{Context: new(api.Context)} ctx.SetFiles(map[string]string{ @@ -728,9 +767,6 @@ func TestMergeHandler(t *testing.T) { "file2.pdf": "/file2.pdf", }) ctx.SetValues(map[string][]string{ - "metadata": { - "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", - }, "flatten": { "true", }, @@ -741,9 +777,6 @@ func TestMergeHandler(t *testing.T) { 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 - }, FlattenMock: func(ctx context.Context, logger *zap.Logger, inputPath string) error { return errors.New("foo") }, @@ -975,6 +1008,38 @@ func TestSplitHandler(t *testing.T) { expectHttpError: false, expectOutputPathsCount: 0, }, + { + scenario: "error from PDF engine (flatten)", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetFiles(map[string]string{ + "file.pdf": "/file.pdf", + }) + ctx.SetValues(map[string][]string{ + "splitMode": { + gotenberg.SplitModeIntervals, + }, + "splitSpan": { + "1", + }, + "flatten": { + "true", + }, + }) + return ctx + }(), + engine: &gotenberg.PdfEngineMock{ + SplitMock: func(ctx context.Context, logger *zap.Logger, mode gotenberg.SplitMode, inputPath, outputDirPath string) ([]string, error) { + return []string{inputPath}, nil + }, + FlattenMock: func(ctx context.Context, logger *zap.Logger, inputPath string) error { + return errors.New("foo") + }, + }, + expectError: true, + expectHttpError: false, + expectOutputPathsCount: 0, + }, { scenario: "cannot add output paths", ctx: func() *api.ContextMock { @@ -1022,6 +1087,9 @@ func TestSplitHandler(t *testing.T) { "metadata": { "{\"Creator\": \"foo\", \"Producer\": \"bar\" }", }, + "flatten": { + "true", + }, }) return ctx }(), @@ -1035,6 +1103,9 @@ func TestSplitHandler(t *testing.T) { WriteMetadataMock: func(ctx context.Context, logger *zap.Logger, metadata map[string]interface{}, inputPath string) error { return nil }, + FlattenMock: func(ctx context.Context, logger *zap.Logger, inputPath string) error { + return nil + }, }, expectError: false, expectHttpError: false, diff --git a/pkg/modules/qpdf/doc.go b/pkg/modules/qpdf/doc.go index f0d54a54..5f494a1f 100644 --- a/pkg/modules/qpdf/doc.go +++ b/pkg/modules/qpdf/doc.go @@ -3,6 +3,7 @@ // // 1. The merging of PDF files. // 2. The splitting of PDF files. +// 3. Flattening of PDF files // // The path to the QPDF binary must be specified using the QPDK_BIN_PATH // environment variable. diff --git a/pkg/modules/qpdf/qpdf.go b/pkg/modules/qpdf/qpdf.go index 760b6512..7271a2fd 100644 --- a/pkg/modules/qpdf/qpdf.go +++ b/pkg/modules/qpdf/qpdf.go @@ -101,7 +101,8 @@ func (engine *QPdf) Merge(ctx context.Context, logger *zap.Logger, inputPaths [] return fmt.Errorf("merge PDFs with QPDF: %w", err) } -// Flatten merges annotation appearances with page content, deleting the original annotations. +// Flatten merges annotation appearances with page content, deleting the +// original annotations. func (engine *QPdf) Flatten(ctx context.Context, logger *zap.Logger, inputPath string) error { var args []string args = append(args, "--generate-appearances")