diff --git a/pkg/gotenberg/fs.go b/pkg/gotenberg/fs.go index d1e14a7d..ec58c829 100644 --- a/pkg/gotenberg/fs.go +++ b/pkg/gotenberg/fs.go @@ -3,10 +3,6 @@ package gotenberg import ( "fmt" "os" - "path/filepath" - "sort" - "strings" - "time" "github.com/google/uuid" ) @@ -88,46 +84,6 @@ func (fs *FileSystem) MkdirAll() (string, error) { return path, nil } -type fileWithModTime struct { - path string - modTime time.Time -} - -// WalkDir walks through the root level of a directory and returns a list of -// files paths that match the specified file extension. -func WalkDir(dir, ext string) ([]string, error) { - var files []fileWithModTime - err := filepath.Walk(dir, func(path string, info os.FileInfo, pathErr error) error { - if pathErr != nil { - return pathErr - } - if info.IsDir() { - return nil - } - if strings.EqualFold(filepath.Ext(info.Name()), ext) { - files = append(files, fileWithModTime{ - path: path, - modTime: info.ModTime(), - }) - } - return nil - }) - if err != nil { - return nil, err - } - - sort.Slice(files, func(i, j int) bool { - return files[i].modTime.Before(files[j].modTime) - }) - - sortedPaths := make([]string, len(files)) - for i, f := range files { - sortedPaths[i] = f.path - } - - return sortedPaths, nil -} - // Interface guards. var ( _ MkdirAll = (*OsMkdirAll)(nil) diff --git a/pkg/gotenberg/fs_test.go b/pkg/gotenberg/fs_test.go index f6c15a35..ee57d1b9 100644 --- a/pkg/gotenberg/fs_test.go +++ b/pkg/gotenberg/fs_test.go @@ -6,7 +6,6 @@ import ( "io" "os" "path/filepath" - "reflect" "strings" "testing" @@ -145,88 +144,3 @@ func TestFileSystem_MkdirAll(t *testing.T) { }) } } - -func TestWalkDir(t *testing.T) { - for _, tc := range []struct { - scenario string - dir string - ext string - expectError bool - expectFiles []string - }{ - { - scenario: "directory does not exist", - dir: uuid.NewString(), - ext: ".pdf", - expectError: true, - }, - { - scenario: "find PDF files, sorted by mod time", - dir: func() string { - path := fmt.Sprintf("%s/a_directory", os.TempDir()) - - err := os.MkdirAll(path, 0o755) - if err != nil { - t.Fatalf(fmt.Sprintf("expected no error but got: %v", err)) - } - - err = os.WriteFile(fmt.Sprintf("%s/2.pdf", path), []byte{1}, 0o755) - if err != nil { - t.Fatalf("expected no error but got: %v", err) - } - - err = os.WriteFile(fmt.Sprintf("%s/1.PDF", path), []byte{1}, 0o755) - if err != nil { - t.Fatalf("expected no error but got: %v", err) - } - - err = os.WriteFile(fmt.Sprintf("%s/3.txt", path), []byte{1}, 0o755) - if err != nil { - t.Fatalf("expected no error but got: %v", err) - } - - err = os.WriteFile(fmt.Sprintf("%s/10.pdf", path), []byte{1}, 0o755) - if err != nil { - t.Fatalf("expected no error but got: %v", err) - } - - err = os.WriteFile(fmt.Sprintf("%s/11.pdf", path), []byte{1}, 0o755) - if err != nil { - t.Fatalf("expected no error but got: %v", err) - } - - return path - }(), - ext: ".pdf", - expectError: false, - expectFiles: []string{"/tmp/a_directory/2.pdf", "/tmp/a_directory/1.PDF", "/tmp/a_directory/10.pdf", "/tmp/a_directory/11.pdf"}, - }, - } { - t.Run(tc.scenario, func(t *testing.T) { - defer func() { - err := os.RemoveAll(tc.dir) - if err != nil { - t.Fatalf("expected no error while cleaning up but got: %v", err) - } - }() - - files, err := WalkDir(tc.dir, tc.ext) - - if !tc.expectError && err != nil { - t.Fatalf("expected no error but got: %v", err) - } - - if tc.expectError && err == nil { - t.Fatal("expected error but got none") - } - - if tc.expectError && err != nil { - return - } - - if !reflect.DeepEqual(files, tc.expectFiles) { - t.Errorf("expected files %+v, but got %+v", tc.expectFiles, files) - } - }) - } -} diff --git a/pkg/gotenberg/sort.go b/pkg/gotenberg/sort.go index bd18da69..0a64ec2f 100644 --- a/pkg/gotenberg/sort.go +++ b/pkg/gotenberg/sort.go @@ -7,7 +7,8 @@ import ( ) // AlphanumericSort implements sort.Interface and helps to sort strings -// alphanumerically. +// alphanumerically by either a numeric prefix or, if missing, a numeric +// suffix. // // See: https://github.com/gotenberg/gotenberg/issues/805. type AlphanumericSort []string @@ -21,20 +22,20 @@ func (s AlphanumericSort) Swap(i, j int) { } func (s AlphanumericSort) Less(i, j int) bool { - numI, restI := extractPrefix(s[i]) - numJ, restJ := extractPrefix(s[j]) + numI, restI := extractNumber(s[i]) + numJ, restJ := extractNumber(s[j]) - // Compares numerical prefixes if they exist. + // If both strings contain a number, compare them numerically. if numI != -1 && numJ != -1 { if numI != numJ { return numI < numJ } - // If numbers are equal, falls back to string comparison of the rest. + // If the numbers are equal, compare the "rest" strings. return restI < restJ } - // If one has a numerical prefix and the other doesn't, the one with the - // number comes first. + // If one contains a number and the other doesn't, the one with the number + // comes first. if numI != -1 { return true } @@ -42,28 +43,52 @@ func (s AlphanumericSort) Less(i, j int) bool { return false } - // If neither has a numerical prefix, compare as strings + // Neither has a number; fall back to lexicographical order. return s[i] < s[j] } -// extractPrefix attempts to extract a numerical prefix and the rest of the filename -func extractPrefix(filename string) (int, string) { - matches := numPrefixRegexp.FindStringSubmatch(filename) - if len(matches) > 2 { - prefix, err := strconv.Atoi(matches[1]) - if err == nil { - return prefix, matches[2] +// extractNumber attempts to extract a numeric portion from the filename. +// It first checks for a numeric prefix (digits at the beginning). +// If none is found, it next attempts to match a number immediately before the +// extension (for filenames such as "sample1_1.pdf"). +// If that fails, it then attempts a trailing numeric pattern. +// If no number is found, it returns -1 and the original string. +func extractNumber(str string) (int, string) { + // Check for a numeric prefix. + if matches := prefixRegexp.FindStringSubmatch(str); len(matches) > 2 { + if num, err := strconv.Atoi(matches[1]); err == nil { + return num, matches[2] } } - // Returns -1 if no numerical prefix is found, indicating to just compare - // as strings. - return -1, filename + // Check for a number immediately before an extension. + if matches := extensionSuffixRegexp.FindStringSubmatch(str); len(matches) > 3 { + if num, err := strconv.Atoi(matches[2]); err == nil { + // Remove the numeric block but keep the extension. + return num, matches[1] + matches[3] + } + } + + // Check for a trailing number (with no extension following). + if matches := suffixRegexp.FindStringSubmatch(str); len(matches) > 2 { + if num, err := strconv.Atoi(matches[2]); err == nil { + return num, matches[1] + } + } + + // No numeric portion found. + return -1, str } -var numPrefixRegexp = regexp.MustCompile(`^(\d+)(.*)$`) +// Regular expressions used by extractNumber. +var ( + // Matches a numeric prefix: one or more digits at the start. + prefixRegexp = regexp.MustCompile(`^(\d+)(.*)$`) + // Matches a numeric block immediately before a file extension. + extensionSuffixRegexp = regexp.MustCompile(`^(.*?)(\d+)(\.[^.]+)$`) + // Matches a trailing numeric sequence when there is no extension. + suffixRegexp = regexp.MustCompile(`^(.*?)(\d+)$`) +) // Interface guard. -var ( - _ sort.Interface = (*AlphanumericSort)(nil) -) +var _ sort.Interface = (*AlphanumericSort)(nil) diff --git a/pkg/gotenberg/sort_test.go b/pkg/gotenberg/sort_test.go index 94b29166..c797ce7f 100644 --- a/pkg/gotenberg/sort_test.go +++ b/pkg/gotenberg/sort_test.go @@ -17,6 +17,16 @@ func TestAlphanumericSort(t *testing.T) { values: []string{"10qux.pdf", "2_baz.txt", "2_aza.txt", "1bar.pdf", "Afoo.txt", "Bbar.docx", "25zeta.txt", "3.pdf", "4_foo.pdf"}, expectedSort: []string{"1bar.pdf", "2_aza.txt", "2_baz.txt", "3.pdf", "4_foo.pdf", "10qux.pdf", "25zeta.txt", "Afoo.txt", "Bbar.docx"}, }, + { + scenario: "numeric suffixes with extensions", + values: []string{"sample1_10.pdf", "sample1_11.pdf", "sample1_4.pdf", "sample1_3.pdf", "sample1_1.pdf", "sample1_2.pdf"}, + expectedSort: []string{"sample1_1.pdf", "sample1_2.pdf", "sample1_3.pdf", "sample1_4.pdf", "sample1_10.pdf", "sample1_11.pdf"}, + }, + { + scenario: "numeric suffixes", + values: []string{"sample1_10", "sample1_11", "sample1_4", "sample1_3", "sample1_1", "sample1_2"}, + expectedSort: []string{"sample1_1", "sample1_2", "sample1_3", "sample1_4", "sample1_10", "sample1_11"}, + }, { scenario: "hrtime (PHP library)", values: []string{"245654773395259", "245654773395039", "245654773395149", "245654773394919", "245654773394369"}, diff --git a/pkg/modules/pdfcpu/pdfcpu.go b/pkg/modules/pdfcpu/pdfcpu.go index 4a468c1a..609e955d 100644 --- a/pkg/modules/pdfcpu/pdfcpu.go +++ b/pkg/modules/pdfcpu/pdfcpu.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "sort" "strings" "syscall" @@ -128,11 +129,25 @@ func (engine *PdfCpu) Split(ctx context.Context, logger *zap.Logger, mode gotenb return nil, fmt.Errorf("split PDFs with pdfcpu: %w", err) } - outputPaths, err := gotenberg.WalkDir(outputDirPath, ".pdf") + var outputPaths []string + err = filepath.Walk(outputDirPath, func(path string, info os.FileInfo, pathErr error) error { + if pathErr != nil { + return pathErr + } + if info.IsDir() { + return nil + } + if strings.EqualFold(filepath.Ext(info.Name()), ".pdf") { + outputPaths = append(outputPaths, path) + } + return nil + }) if err != nil { return nil, fmt.Errorf("walk directory to find resulting PDFs from split with pdfcpu: %w", err) } + sort.Sort(gotenberg.AlphanumericSort(outputPaths)) + return outputPaths, nil } diff --git a/pkg/modules/pdfcpu/pdfcpu_test.go b/pkg/modules/pdfcpu/pdfcpu_test.go index b44003c3..b1253cfd 100644 --- a/pkg/modules/pdfcpu/pdfcpu_test.go +++ b/pkg/modules/pdfcpu/pdfcpu_test.go @@ -230,15 +230,15 @@ func TestPdfCpu_Split(t *testing.T) { scenario: "success (intervals)", ctx: context.TODO(), mode: gotenberg.SplitMode{Mode: gotenberg.SplitModeIntervals, Span: "1"}, - inputPath: "/tests/test/testdata/pdfengines/sample1.pdf", + inputPath: "/tests/test/testdata/pdfengines/sample4.pdf", expectError: false, - expectOutputPathsCount: 3, + expectOutputPathsCount: 20, }, { scenario: "success (pages)", ctx: context.TODO(), mode: gotenberg.SplitMode{Mode: gotenberg.SplitModePages, Span: "1"}, - inputPath: "/tests/test/testdata/pdfengines/sample1.pdf", + inputPath: "/tests/test/testdata/pdfengines/sample4.pdf", expectError: false, expectOutputPathsCount: 1, }, @@ -246,7 +246,7 @@ func TestPdfCpu_Split(t *testing.T) { scenario: "success (pages & unify)", ctx: context.TODO(), mode: gotenberg.SplitMode{Mode: gotenberg.SplitModePages, Span: "1-2", Unify: true}, - inputPath: "/tests/test/testdata/pdfengines/sample1.pdf", + inputPath: "/tests/test/testdata/pdfengines/sample4.pdf", expectError: false, expectOutputPathsCount: 1, }, diff --git a/test/testdata/pdfengines/sample4.pdf b/test/testdata/pdfengines/sample4.pdf new file mode 100644 index 00000000..5fc57f14 Binary files /dev/null and b/test/testdata/pdfengines/sample4.pdf differ