fix(pdfcpu): correct sorting for the output paths of the split method

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)

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)
-			}
-		})
-	}
-}

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])
+	numI, restI := extractNumber(s[i])
-	numJ, restJ := extractPrefix(s[j])
+	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
+	// If one contains a number and the other doesn't, the one with the number
-	// number comes first.
+	// 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
+// extractNumber attempts to extract a numeric portion from the filename.
-func extractPrefix(filename string) (int, string) {
+// It first checks for a numeric prefix (digits at the beginning).
-	matches := numPrefixRegexp.FindStringSubmatch(filename)
+// If none is found, it next attempts to match a number immediately before the
-	if len(matches) > 2 {
+// extension (for filenames such as "sample1_1.pdf").
-		prefix, err := strconv.Atoi(matches[1])
+// If that fails, it then attempts a trailing numeric pattern.
-		if err == nil {
+// If no number is found, it returns -1 and the original string.
-			return prefix, matches[2]
+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
+	// Check for a number immediately before an extension.
-	// as strings.
+	if matches := extensionSuffixRegexp.FindStringSubmatch(str); len(matches) > 3 {
-	return -1, filename
+		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 (
+var _ sort.Interface = (*AlphanumericSort)(nil)
-	_ sort.Interface = (*AlphanumericSort)(nil)
-)

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"},

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
 }
 

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,
 		},