fix(libreoffice): keep original extension all the time for preserved filenames in zipped output

This commit is contained in:
Julien Neuhart
2024-02-16 17:21:48 +01:00
parent 089f161d1e
commit 9091ffd1b3
7 changed files with 43 additions and 118 deletions

View File

@@ -75,7 +75,6 @@ func Run() {
go func(app gotenberg.App) { go func(app gotenberg.App) {
id := app.(gotenberg.Module).Descriptor().ID id := app.(gotenberg.Module).Descriptor().ID
err = app.Start() err = app.Start()
if err != nil { if err != nil {
fmt.Printf("[FATAL] starting %s: %s\n", id, err) fmt.Printf("[FATAL] starting %s: %s\n", id, err)
os.Exit(1) os.Exit(1)

View File

@@ -164,7 +164,6 @@ func newContext(echoCtx echo.Context, logger *zap.Logger, fs *gotenberg.FileSyst
for _, files := range form.File { for _, files := range form.File {
for _, fh := range files { for _, fh := range files {
err = copyToDisk(fh) err = copyToDisk(fh)
if err != nil { if err != nil {
return ctx, cancel, fmt.Errorf("copy to disk: %w", err) return ctx, cancel, fmt.Errorf("copy to disk: %w", err)
} }
@@ -191,14 +190,11 @@ func (ctx *Context) FormData() *FormData {
} }
} }
// GeneratePath generates a path within the context's working directory. It does not create a file. // GeneratePath generates a path within the context's working directory.
// It either generates a new UUID-based filename or uses the provided filename. // It either generates a new UUID-based filename or uses the provided filename.
func (ctx *Context) GeneratePath(extension string, optionalFilename ...string) string { // It does not create a file.
var filename string func (ctx *Context) GeneratePath(filename, extension string) string {
if len(optionalFilename) > 0 { if filename == "" {
// Use the provided filename
filename = optionalFilename[0]
} else {
// Generate a new UUID-based filename // Generate a new UUID-based filename
filename = uuid.New().String() filename = uuid.New().String()
} }
@@ -254,7 +250,7 @@ func (ctx *Context) BuildOutputFile() (string, error) {
ImplicitTopLevelFolder: false, ImplicitTopLevelFolder: false,
} }
archivePath := ctx.GeneratePath(".zip") archivePath := ctx.GeneratePath(".zip", "")
err := z.Archive(ctx.outputPaths, archivePath) err := z.Archive(ctx.outputPaths, archivePath)
if err != nil { if err != nil {

View File

@@ -190,11 +190,15 @@ func TestContext_GeneratePath(t *testing.T) {
dirPath: "/foo", dirPath: "/foo",
} }
path := ctx.GeneratePath(".pdf") path := ctx.GeneratePath("", ".pdf")
if !strings.HasPrefix(path, ctx.dirPath) { if !strings.HasPrefix(path, ctx.dirPath) {
t.Errorf("expected '%s' to start with '%s'", path, ctx.dirPath) t.Errorf("expected '%s' to start with '%s'", path, ctx.dirPath)
} }
path = ctx.GeneratePath("foo.txt", ".pdf")
if !strings.Contains(path, "foo.txt.pdf") {
t.Errorf("expected '%s' to start with '%s'", path, ctx.dirPath)
}
} }
func TestContext_AddOutputPaths(t *testing.T) { func TestContext_AddOutputPaths(t *testing.T) {

View File

@@ -494,7 +494,7 @@ func markdownToHtml(ctx *api.Context, inputPath string, markdownPaths []string)
) )
} }
inputPath = ctx.GeneratePath(".html") inputPath = ctx.GeneratePath("", ".html")
err = os.WriteFile(inputPath, buffer.Bytes(), 0o600) err = os.WriteFile(inputPath, buffer.Bytes(), 0o600)
if err != nil { if err != nil {
@@ -505,7 +505,7 @@ func markdownToHtml(ctx *api.Context, inputPath string, markdownPaths []string)
} }
func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url string, pdfFormats gotenberg.PdfFormats, options PdfOptions) error { func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url string, pdfFormats gotenberg.PdfFormats, options PdfOptions) error {
outputPath := ctx.GeneratePath(".pdf") outputPath := ctx.GeneratePath("", ".pdf")
err := chromium.Pdf(ctx, ctx.Log(), url, outputPath, options) err := chromium.Pdf(ctx, ctx.Log(), url, outputPath, options)
err = handleChromiumError(err, url, options.Options) err = handleChromiumError(err, url, options.Options)
@@ -549,10 +549,9 @@ func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url
zeroValued := gotenberg.PdfFormats{} zeroValued := gotenberg.PdfFormats{}
if pdfFormats != zeroValued { if pdfFormats != zeroValued {
convertInputPath := outputPath convertInputPath := outputPath
convertOutputPath := ctx.GeneratePath(".pdf") convertOutputPath := ctx.GeneratePath("", ".pdf")
err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPath) err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPath)
if err != nil { if err != nil {
if errors.Is(err, gotenberg.ErrPdfFormatNotSupported) { if errors.Is(err, gotenberg.ErrPdfFormatNotSupported) {
return api.WrapError( return api.WrapError(
@@ -581,7 +580,7 @@ func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url
func screenshotUrl(ctx *api.Context, chromium Api, url string, options ScreenshotOptions) error { func screenshotUrl(ctx *api.Context, chromium Api, url string, options ScreenshotOptions) error {
ext := fmt.Sprintf(".%s", options.Format) ext := fmt.Sprintf(".%s", options.Format)
outputPath := ctx.GeneratePath(ext) outputPath := ctx.GeneratePath("", ext)
err := chromium.Screenshot(ctx, ctx.Log(), url, outputPath, options) err := chromium.Screenshot(ctx, ctx.Log(), url, outputPath, options)
err = handleChromiumError(err, url, options.Options) err = handleChromiumError(err, url, options.Options)

View File

@@ -5,7 +5,6 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"path/filepath" "path/filepath"
"strings"
"github.com/labstack/echo/v4" "github.com/labstack/echo/v4"
@@ -53,25 +52,11 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap
PdfUa: pdfua, PdfUa: pdfua,
} }
// We need to check and see if there are any duplicate filenames in inputPaths.
filenameCounts := make(map[string]int)
for _, path := range inputPaths {
filename := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path))
filenameCounts[filename]++
}
// Alright, let's convert each document to PDF. // Alright, let's convert each document to PDF.
outputPaths := make([]string, len(inputPaths)) outputPaths := make([]string, len(inputPaths))
for i, inputPath := range inputPaths { for i, inputPath := range inputPaths {
filename := strings.TrimSuffix(filepath.Base(inputPath), filepath.Ext(inputPath)) // document.docx -> document.docx.pdf.
extension := filepath.Ext(inputPath) outputPaths[i] = ctx.GeneratePath(filepath.Base(inputPath), ".pdf")
// Ex: `document.docx`, `document.doc` -> `document.docx.pdf`, `document.doc.pdf`
if filenameCounts[filename] > 1 {
outputPaths[i] = ctx.GeneratePath(".pdf", filename+extension)
} else {
outputPaths[i] = ctx.GeneratePath(".pdf", filename)
}
options := libreofficeapi.Options{ options := libreofficeapi.Options{
Landscape: landscape, Landscape: landscape,
PageRanges: nativePageRanges, PageRanges: nativePageRanges,
@@ -108,7 +93,7 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap
// win: if there is only one PDF, skip this step. // win: if there is only one PDF, skip this step.
if len(outputPaths) > 1 && merge { if len(outputPaths) > 1 && merge {
outputPath := ctx.GeneratePath(".pdf") outputPath := ctx.GeneratePath("", ".pdf")
err = engine.Merge(ctx, ctx.Log(), outputPaths, outputPath) err = engine.Merge(ctx, ctx.Log(), outputPaths, outputPath)
if err != nil { if err != nil {
@@ -120,7 +105,7 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap
zeroValued := gotenberg.PdfFormats{} zeroValued := gotenberg.PdfFormats{}
if !nativePdfFormats && pdfFormats != zeroValued { if !nativePdfFormats && pdfFormats != zeroValued {
convertInputPath := outputPath convertInputPath := outputPath
convertOutputPath := ctx.GeneratePath(".pdf") convertOutputPath := ctx.GeneratePath("", ".pdf")
err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPath) err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPath)
if err != nil { if err != nil {
@@ -160,7 +145,8 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap
for i, outputPath := range outputPaths { for i, outputPath := range outputPaths {
convertInputPath := outputPath convertInputPath := outputPath
convertOutputPaths[i] = ctx.GeneratePath(".pdf") // document.docx -> document.docx.pdf.
convertOutputPaths[i] = ctx.GeneratePath(filepath.Base(inputPaths[i]), ".pdf")
err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPaths[i]) err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPaths[i])
if err != nil { if err != nil {

View File

@@ -220,6 +220,7 @@ func TestConvertRoute(t *testing.T) {
expectError: false, expectError: false,
expectHttpError: false, expectHttpError: false,
expectOutputPathsCount: 1, expectOutputPathsCount: 1,
expectFileNames: []string{"/document.docx.pdf"},
}, },
{ {
scenario: "success (many files)", scenario: "success (many files)",
@@ -228,6 +229,7 @@ func TestConvertRoute(t *testing.T) {
ctx.SetFiles(map[string]string{ ctx.SetFiles(map[string]string{
"document.docx": "/document.docx", "document.docx": "/document.docx",
"document2.docx": "/document2.docx", "document2.docx": "/document2.docx",
"document2.doc": "/document2.doc",
}) })
return ctx return ctx
}(), }(),
@@ -236,19 +238,21 @@ func TestConvertRoute(t *testing.T) {
return nil return nil
}, },
ExtensionsMock: func() []string { ExtensionsMock: func() []string {
return []string{".docx"} return []string{".docx", ".doc"}
}, },
}, },
expectError: false, expectError: false,
expectHttpError: false, expectHttpError: false,
expectOutputPathsCount: 2, expectOutputPathsCount: 3,
expectFileNames: []string{"/document.docx.pdf", "/document2.docx.pdf", "/document2.doc.pdf"},
}, },
{ {
scenario: "success with non-native PDF/A & PDF/UA (single file)", scenario: "success with non-native PDF/A & PDF/UA (many files)",
ctx: func() *api.ContextMock { ctx: func() *api.ContextMock {
ctx := &api.ContextMock{Context: new(api.Context)} ctx := &api.ContextMock{Context: new(api.Context)}
ctx.SetFiles(map[string]string{ ctx.SetFiles(map[string]string{
"document.docx": "/document.docx", "document.docx": "/document.docx",
"document2.docx": "/document2.docx",
}) })
ctx.SetValues(map[string][]string{ ctx.SetValues(map[string][]string{
"pdfa": { "pdfa": {
@@ -278,14 +282,16 @@ func TestConvertRoute(t *testing.T) {
}, },
expectError: false, expectError: false,
expectHttpError: false, expectHttpError: false,
expectOutputPathsCount: 1, expectOutputPathsCount: 2,
expectFileNames: []string{"/document.docx.pdf", "/document2.docx.pdf"},
}, },
{ {
scenario: "success with native PDF/A & PDF/UA (single file)", scenario: "success with native PDF/A & PDF/UA (many files)",
ctx: func() *api.ContextMock { ctx: func() *api.ContextMock {
ctx := &api.ContextMock{Context: new(api.Context)} ctx := &api.ContextMock{Context: new(api.Context)}
ctx.SetFiles(map[string]string{ ctx.SetFiles(map[string]string{
"document.docx": "/document.docx", "document.docx": "/document.docx",
"document2.docx": "/document2.docx",
}) })
ctx.SetValues(map[string][]string{ ctx.SetValues(map[string][]string{
"pdfa": { "pdfa": {
@@ -312,7 +318,8 @@ func TestConvertRoute(t *testing.T) {
}, },
expectError: false, expectError: false,
expectHttpError: false, expectHttpError: false,
expectOutputPathsCount: 1, expectOutputPathsCount: 2,
expectFileNames: []string{"/document.docx.pdf", "/document2.docx.pdf"},
}, },
{ {
scenario: "merge error", scenario: "merge error",
@@ -494,72 +501,6 @@ func TestConvertRoute(t *testing.T) {
expectHttpError: false, expectHttpError: false,
expectOutputPathsCount: 1, expectOutputPathsCount: 1,
}, },
{
scenario: "success (not merged, unique filenames with different extensions)",
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": {
"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", ".doc"}
},
},
engine: &gotenberg.PdfEngineMock{
MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error {
return nil
},
},
expectError: false,
expectHttpError: false,
expectOutputPathsCount: 2,
},
{
scenario: "success (not merged, duplicate filenames with different extensions)",
ctx: func() *api.ContextMock {
ctx := &api.ContextMock{Context: new(api.Context)}
ctx.SetFiles(map[string]string{
"document.docx": "/document.docx",
"document2.docx": "/document2.docx",
"document2.doc": "/document2.doc",
})
ctx.SetValues(map[string][]string{
"merge": {
"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", ".doc"}
},
},
engine: &gotenberg.PdfEngineMock{
MergeMock: func(ctx context.Context, logger *zap.Logger, inputPaths []string, outputPath string) error {
return nil
},
},
expectError: false,
expectHttpError: false,
expectOutputPathsCount: 3,
expectFileNames: []string{"document.pdf", "document2.docx.pdf", "document2.doc.pdf"},
},
{ {
scenario: "success with non-native PDF/A & PDF/UA (merge)", scenario: "success with non-native PDF/A & PDF/UA (merge)",
ctx: func() *api.ContextMock { ctx: func() *api.ContextMock {
@@ -686,8 +627,10 @@ func TestConvertRoute(t *testing.T) {
t.Errorf("expected %d output paths but got %d", tc.expectOutputPathsCount, len(tc.ctx.OutputPaths())) t.Errorf("expected %d output paths but got %d", tc.expectOutputPathsCount, len(tc.ctx.OutputPaths()))
} }
if !reflect.DeepEqual(tc.ctx.OutputPaths(), tc.expectFileNames) { if len(tc.expectFileNames) != 0 {
t.Errorf("expected output paths %v, got %v", tc.expectFileNames, tc.ctx.OutputPaths()) if !reflect.DeepEqual(tc.ctx.OutputPaths(), tc.expectFileNames) {
t.Errorf("expected output paths %v, got %v", tc.expectFileNames, tc.ctx.OutputPaths())
}
} }
}) })
} }

View File

@@ -43,7 +43,7 @@ func mergeRoute(engine gotenberg.PdfEngine) api.Route {
// Alright, let's merge the PDFs. // Alright, let's merge the PDFs.
outputPath := ctx.GeneratePath(".pdf") outputPath := ctx.GeneratePath("", ".pdf")
err = engine.Merge(ctx, ctx.Log(), inputPaths, outputPath) err = engine.Merge(ctx, ctx.Log(), inputPaths, outputPath)
if err != nil { if err != nil {
@@ -56,10 +56,9 @@ func mergeRoute(engine gotenberg.PdfEngine) api.Route {
zeroValued := gotenberg.PdfFormats{} zeroValued := gotenberg.PdfFormats{}
if pdfFormats != zeroValued { if pdfFormats != zeroValued {
convertInputPath := outputPath convertInputPath := outputPath
convertOutputPath := ctx.GeneratePath(".pdf") convertOutputPath := ctx.GeneratePath("", ".pdf")
err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPath) err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPath)
if err != nil { if err != nil {
if errors.Is(err, gotenberg.ErrPdfFormatNotSupported) { if errors.Is(err, gotenberg.ErrPdfFormatNotSupported) {
return api.WrapError( return api.WrapError(
@@ -137,10 +136,9 @@ func convertRoute(engine gotenberg.PdfEngine) api.Route {
outputPaths := make([]string, len(inputPaths)) outputPaths := make([]string, len(inputPaths))
for i, inputPath := range inputPaths { for i, inputPath := range inputPaths {
outputPaths[i] = ctx.GeneratePath(".pdf") outputPaths[i] = ctx.GeneratePath("", ".pdf")
err = engine.Convert(ctx, ctx.Log(), pdfFormats, inputPath, outputPaths[i]) err = engine.Convert(ctx, ctx.Log(), pdfFormats, inputPath, outputPaths[i])
if err != nil { if err != nil {
if errors.Is(err, gotenberg.ErrPdfFormatNotSupported) { if errors.Is(err, gotenberg.ErrPdfFormatNotSupported) {
return api.WrapError( return api.WrapError(