mirror of
https://github.com/Belphemur/CBZOptimizer.git
synced 2026-07-22 11:25:40 +02:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
868347cc55 | ||
|
|
bf21d3842f | ||
|
|
b58999c6ec | ||
|
|
4cb5d10d3f | ||
|
|
21f92bc6f5 | ||
|
|
f3ba585c13 | ||
|
|
e56b70b038 | ||
|
|
650623b340 | ||
|
|
4b961f18e3 | ||
|
|
2dfc39505c | ||
|
|
e596a026dd | ||
|
|
9423cde9f7 |
@@ -1 +0,0 @@
|
||||
testdata/large/*.cbz filter=lfs diff=lfs merge=lfs -text
|
||||
@@ -95,15 +95,6 @@ go test -v ./pkg/converter/...
|
||||
go test -v ./internal/utils/...
|
||||
```
|
||||
|
||||
A large-file integration test (`TestOptimizeIntegration_LargeFile`) exercises the
|
||||
optimize pipeline against a ~1GB CBZ fixture stored via Git LFS
|
||||
(`testdata/large/large_chapter.cbz`, tracked in `.gitattributes`). It is skipped
|
||||
by default; fetch the fixture with `git lfs pull` and opt in with:
|
||||
|
||||
```bash
|
||||
CBZ_RUN_LARGE_FILE_TEST=1 go test -v ./internal/utils/... -run TestOptimizeIntegration_LargeFile
|
||||
```
|
||||
|
||||
### Linting
|
||||
|
||||
```bash
|
||||
|
||||
@@ -14,7 +14,7 @@ jobs:
|
||||
uses: actions/checkout@v7
|
||||
|
||||
- name: Set up Go
|
||||
uses: actions/setup-go@v6
|
||||
uses: actions/setup-go@v7
|
||||
with:
|
||||
go-version-file: go.mod
|
||||
cache: true
|
||||
|
||||
@@ -24,7 +24,7 @@ jobs:
|
||||
with:
|
||||
fetch-depth: 0 # this is important, otherwise it won't checkout the full tree (i.e. no previous tags)
|
||||
- name: Set up Go
|
||||
uses: actions/setup-go@v6
|
||||
uses: actions/setup-go@v7
|
||||
with:
|
||||
go-version-file: go.mod
|
||||
cache: true
|
||||
|
||||
@@ -12,7 +12,7 @@ jobs:
|
||||
- uses: actions/checkout@v7
|
||||
|
||||
- name: Set up Go
|
||||
uses: actions/setup-go@v6
|
||||
uses: actions/setup-go@v7
|
||||
with:
|
||||
go-version-file: go.mod
|
||||
cache: true
|
||||
|
||||
@@ -11,6 +11,10 @@
|
||||
# Test binary, built with `go test -c`
|
||||
*.test
|
||||
|
||||
# Build artifacts
|
||||
/cbzoptimizer
|
||||
/encoder-setup
|
||||
|
||||
test/
|
||||
|
||||
# Output of the go coverage tool, specifically when used with LiteIDE
|
||||
|
||||
@@ -58,6 +58,12 @@ cbzconverter optimize [folder] --format=webp
|
||||
cbzconverter optimize [folder] --format WEBP
|
||||
```
|
||||
|
||||
Preserve original page filenames in the output CBZ instead of sequential renumbering:
|
||||
|
||||
```sh
|
||||
cbzconverter optimize [folder] --keep-filenames --quality 85 --format webp
|
||||
```
|
||||
|
||||
With timeout to avoid hanging on problematic chapters:
|
||||
|
||||
```sh
|
||||
@@ -103,6 +109,7 @@ docker run -v /path/to/comics:/comics ghcr.io/belphemur/cbzoptimizer:latest watc
|
||||
- `--format`, `-f`: Format to convert the images to (currently supports: webp). Default is webp.
|
||||
- Can be specified as: `--format webp`, `-f webp`, or `--format=webp`
|
||||
- Case-insensitive: `webp`, `WEBP`, and `WebP` are all valid
|
||||
- `--keep-filenames`: Preserve each page's original base filename in the output CBZ (with the extension swapped for format conversion) instead of renumbering pages to `0000`, `0001`, ... Default is false. When two source pages share the same base filename, the second one falls back to the indexed form so the output stays a valid zip.
|
||||
- `--timeout`, `-t`: Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout. Default is 0.
|
||||
- `--backfill`: *(`watch` only)* Optimize CBZ/CBR files that already exist in the watched folder at startup, before watching for new changes. Default is false.
|
||||
- `--log`, `-l`: Set log level; can be 'panic', 'fatal', 'error', 'warn', 'info', 'debug', or 'trace'. Default is info.
|
||||
|
||||
Binary file not shown.
@@ -70,6 +70,19 @@ func setupSplitFlag(cmd *cobra.Command, defaultValue bool, bindViper bool) {
|
||||
}
|
||||
}
|
||||
|
||||
// setupKeepFilenamesFlag sets up the keep-filenames flag for a command.
|
||||
//
|
||||
// Parameters:
|
||||
// - cmd: The Cobra command to add the keep-filenames flag to
|
||||
// - defaultValue: The default keep-filenames value
|
||||
// - bindViper: If true, binds the flag to viper for configuration file support
|
||||
func setupKeepFilenamesFlag(cmd *cobra.Command, defaultValue bool, bindViper bool) {
|
||||
cmd.Flags().Bool("keep-filenames", defaultValue, "Preserve original page filenames instead of renumbering to sequential indices")
|
||||
if bindViper {
|
||||
_ = viper.BindPFlag("keep-filenames", cmd.Flags().Lookup("keep-filenames"))
|
||||
}
|
||||
}
|
||||
|
||||
// setupTimeoutFlag sets up the timeout flag for a command.
|
||||
//
|
||||
// Parameters:
|
||||
@@ -96,5 +109,6 @@ func setupCommonFlags(cmd *cobra.Command, converterType *constant.ConversionForm
|
||||
setupQualityFlag(cmd, qualityDefault, bindViper)
|
||||
setupOverrideFlag(cmd, overrideDefault, bindViper)
|
||||
setupSplitFlag(cmd, splitDefault, bindViper)
|
||||
setupKeepFilenamesFlag(cmd, false, bindViper)
|
||||
setupTimeoutFlag(cmd, bindViper)
|
||||
}
|
||||
|
||||
@@ -73,6 +73,13 @@ func ConvertCbzCommand(cmd *cobra.Command, args []string) error {
|
||||
}
|
||||
log.Debug().Bool("split", split).Msg("Split parameter parsed")
|
||||
|
||||
keepFilenames, err := cmd.Flags().GetBool("keep-filenames")
|
||||
if err != nil {
|
||||
log.Error().Err(err).Msg("Failed to parse keep-filenames flag")
|
||||
return fmt.Errorf("invalid keep-filenames value")
|
||||
}
|
||||
log.Debug().Bool("keep-filenames", keepFilenames).Msg("Keep-filenames parameter parsed")
|
||||
|
||||
timeout, err := cmd.Flags().GetDuration("timeout")
|
||||
if err != nil {
|
||||
log.Error().Err(err).Msg("Failed to parse timeout flag")
|
||||
@@ -127,6 +134,7 @@ func ConvertCbzCommand(cmd *cobra.Command, args []string) error {
|
||||
Quality: quality,
|
||||
Override: override,
|
||||
Split: split,
|
||||
KeepFilenames: keepFilenames,
|
||||
Timeout: timeout,
|
||||
})
|
||||
if err != nil {
|
||||
|
||||
@@ -86,6 +86,7 @@ func TestConvertCbzCommand(t *testing.T) {
|
||||
cmd.Flags().IntP("parallelism", "n", 2, "Number of chapters to convert in parallel")
|
||||
cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files")
|
||||
cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks")
|
||||
cmd.Flags().Bool("keep-filenames", false, "Preserve original page filenames instead of renumbering to sequential indices")
|
||||
cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter (e.g., 30s, 5m, 1h). 0 means no timeout")
|
||||
|
||||
// Execute the command
|
||||
@@ -197,6 +198,7 @@ func setupTestCommand(t *testing.T) (*cobra.Command, func()) {
|
||||
cmd.Flags().IntP("parallelism", "n", 1, "Number of chapters to convert in parallel")
|
||||
cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files")
|
||||
cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks")
|
||||
cmd.Flags().Bool("keep-filenames", false, "Preserve original page filenames instead of renumbering to sequential indices")
|
||||
cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter")
|
||||
|
||||
// Reset converterType to default before test for consistency
|
||||
@@ -433,6 +435,7 @@ func TestConvertCbzCommand_ManyFiles_NoDeadlock(t *testing.T) {
|
||||
cmd.Flags().IntP("parallelism", "n", 2, "Number of chapters to convert in parallel")
|
||||
cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files")
|
||||
cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks")
|
||||
cmd.Flags().Bool("keep-filenames", false, "Preserve original page filenames instead of renumbering to sequential indices")
|
||||
cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter")
|
||||
|
||||
converterType = constant.DefaultConversion
|
||||
@@ -535,6 +538,7 @@ func TestConvertCbzCommand_HighParallelism_NoDeadlock(t *testing.T) {
|
||||
cmd.Flags().IntP("parallelism", "n", 8, "Number of chapters to convert in parallel")
|
||||
cmd.Flags().BoolP("override", "o", false, "Override the original CBZ/CBR files")
|
||||
cmd.Flags().BoolP("split", "s", false, "Split long pages into smaller chunks")
|
||||
cmd.Flags().Bool("keep-filenames", false, "Preserve original page filenames instead of renumbering to sequential indices")
|
||||
cmd.Flags().DurationP("timeout", "t", 0, "Maximum time allowed for converting a single chapter")
|
||||
|
||||
converterType = constant.DefaultConversion
|
||||
|
||||
@@ -65,6 +65,8 @@ func WatchCommand(_ *cobra.Command, args []string) error {
|
||||
|
||||
split := viper.GetBool("split")
|
||||
|
||||
keepFilenames := viper.GetBool("keep-filenames")
|
||||
|
||||
timeout := viper.GetDuration("timeout")
|
||||
|
||||
backfill := viper.GetBool("backfill")
|
||||
@@ -79,7 +81,7 @@ func WatchCommand(_ *cobra.Command, args []string) error {
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to prepare converter: %w", err)
|
||||
}
|
||||
log.Info().Str("path", path).Bool("override", override).Uint8("quality", quality).Str("format", converterType.String()).Bool("split", split).Bool("backfill", backfill).Msg("Watching directory")
|
||||
log.Info().Str("path", path).Bool("override", override).Uint8("quality", quality).Str("format", converterType.String()).Bool("split", split).Bool("keep_filenames", keepFilenames).Bool("backfill", backfill).Msg("Watching directory")
|
||||
|
||||
watcher, err := fsnotify.NewWatcher()
|
||||
if err != nil {
|
||||
@@ -98,6 +100,7 @@ func WatchCommand(_ *cobra.Command, args []string) error {
|
||||
Quality: quality,
|
||||
Override: override,
|
||||
Split: split,
|
||||
KeepFilenames: keepFilenames,
|
||||
Timeout: timeout,
|
||||
})
|
||||
defer queue.Stop()
|
||||
|
||||
Binary file not shown.
@@ -13,7 +13,7 @@ require (
|
||||
github.com/spf13/viper v1.21.0
|
||||
github.com/stretchr/testify v1.11.1
|
||||
github.com/thediveo/enumflag/v2 v2.2.1
|
||||
golang.org/x/image v0.43.0
|
||||
golang.org/x/image v0.44.0
|
||||
)
|
||||
|
||||
require (
|
||||
@@ -50,7 +50,7 @@ require (
|
||||
go4.org v0.0.0-20230225012048-214862532bf5 // indirect
|
||||
golang.org/x/net v0.56.0 // indirect
|
||||
golang.org/x/sys v0.46.0 // indirect
|
||||
golang.org/x/text v0.38.0 // indirect
|
||||
golang.org/x/tools v0.46.0 // indirect
|
||||
golang.org/x/text v0.40.0 // indirect
|
||||
golang.org/x/tools v0.47.0 // indirect
|
||||
gopkg.in/yaml.v3 v3.0.1 // indirect
|
||||
)
|
||||
|
||||
@@ -217,6 +217,8 @@ golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMx
|
||||
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
|
||||
golang.org/x/image v0.43.0 h1:FLxcP4ec2350nTfOC8ysKtqYSIFbk/QGjw1ZHNP4tsY=
|
||||
golang.org/x/image v0.43.0/go.mod h1:rrpelvGFt+kLPAjPM4HeWPgrl0FtafueU//e5N0qk/Q=
|
||||
golang.org/x/image v0.44.0 h1:+tDekMZED9+LrtB3G5xzRggpVh9CARjZqROla3R3R+I=
|
||||
golang.org/x/image v0.44.0/go.mod h1:V8K3KE9KKKE+pLpQDOeN18w9oacNSvy1tDOirTu4xtY=
|
||||
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
|
||||
golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
|
||||
golang.org/x/lint v0.0.0-20190301231843-5614ed5bae6f/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
|
||||
@@ -299,6 +301,8 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
|
||||
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
|
||||
golang.org/x/text v0.38.0 h1:sXmwo9DwP3OK9EZ7PqAdaooSGozfl/3a6/xJcbzPRhE=
|
||||
golang.org/x/text v0.38.0/go.mod h1:YXZt3QhHUKYT53r2lLKFIVi6Ao1jdzrTR/KQ09qyxF4=
|
||||
golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs=
|
||||
golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY=
|
||||
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
|
||||
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
|
||||
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
|
||||
@@ -328,6 +332,7 @@ golang.org/x/tools v0.0.0-20200212150539-ea181f53ac56/go.mod h1:TB2adYChydJhpapK
|
||||
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
|
||||
golang.org/x/tools v0.46.0 h1:7jTurBkPZu4moS/Uy4OQT1M+QBlsj3wejyZwsT8Z7rk=
|
||||
golang.org/x/tools v0.46.0/go.mod h1:FrD85F8l+NWL+9XWBSyVSHO6Ne4jutsfIFba7AWQ5Ys=
|
||||
golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA=
|
||||
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
|
||||
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
|
||||
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
|
||||
|
||||
@@ -5,6 +5,8 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/belphemur/CBZOptimizer/v2/internal/manga"
|
||||
@@ -12,6 +14,56 @@ import (
|
||||
"github.com/rs/zerolog/log"
|
||||
)
|
||||
|
||||
// resolvePageName picks the final archive entry name for a page.
|
||||
//
|
||||
// Order of resolution:
|
||||
// 1. When page.OriginalName is set (keep-filenames mode), use its stem with
|
||||
// the current page.Extension. Split pages append the -NN suffix so all
|
||||
// parts of the same source still land in the archive.
|
||||
// 2. If that final name has already been used in this archive, fall back to
|
||||
// the indexed form (stem + "_%04d" + extension) so the zip stays valid.
|
||||
// 3. Otherwise (OriginalName is empty), use the historical %04d / %04d-NN
|
||||
// sequential naming.
|
||||
//
|
||||
// usedNames tracks every name already chosen for this archive and is mutated
|
||||
// in place so the caller can keep a single map across the whole chapter.
|
||||
func resolvePageName(page *manga.PageFile, usedNames map[string]struct{}) string {
|
||||
if page.OriginalName != "" {
|
||||
stem := strings.TrimSuffix(page.OriginalName, filepath.Ext(page.OriginalName))
|
||||
var candidate string
|
||||
if page.IsSplitted {
|
||||
candidate = fmt.Sprintf("%s-%02d%s", stem, page.SplitPartIndex, page.Extension)
|
||||
} else {
|
||||
candidate = stem + page.Extension
|
||||
}
|
||||
if _, taken := usedNames[candidate]; !taken {
|
||||
usedNames[candidate] = struct{}{}
|
||||
return candidate
|
||||
}
|
||||
// Collision: fall back to the indexed form so the entry still gets
|
||||
// written, but make it visibly distinct from the preserved one.
|
||||
// Loop in case the fallback name is itself already taken (e.g. a
|
||||
// source file literally named "cover_0001.png" colliding with a
|
||||
// page-index 1 fallback "cover_0001.webp").
|
||||
for suffix := page.Index; ; suffix++ {
|
||||
fallback := fmt.Sprintf("%s_%04d%s", stem, suffix, page.Extension)
|
||||
if _, taken := usedNames[fallback]; !taken {
|
||||
usedNames[fallback] = struct{}{}
|
||||
return fallback
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if page.IsSplitted {
|
||||
name := fmt.Sprintf("%04d-%02d%s", page.Index, page.SplitPartIndex, page.Extension)
|
||||
usedNames[name] = struct{}{}
|
||||
return name
|
||||
}
|
||||
name := fmt.Sprintf("%04d%s", page.Index, page.Extension)
|
||||
usedNames[name] = struct{}{}
|
||||
return name
|
||||
}
|
||||
|
||||
// WriteChapterToCBZ creates a CBZ file from a Chapter by streaming page files
|
||||
// from disk directly into the zip archive. No image data is held in memory.
|
||||
func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) (err error) {
|
||||
@@ -34,14 +86,14 @@ func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) (err error
|
||||
zipWriter := zip.NewWriter(zipFile)
|
||||
defer errs.Capture(&err, zipWriter.Close, "failed to close .cbz writer")
|
||||
|
||||
// Write each page to the archive by streaming from disk
|
||||
// Write each page to the archive by streaming from disk.
|
||||
// Final name resolution: when a page carries an OriginalName (recorded by
|
||||
// ExtractChapter when --keep-filenames is on), preserve its stem and only
|
||||
// swap the extension to the current page.Extension. Duplicates in the
|
||||
// archive fall back to the indexed naming so the output stays a valid zip.
|
||||
usedNames := make(map[string]struct{}, len(chapter.Pages))
|
||||
for _, page := range chapter.Pages {
|
||||
var fileName string
|
||||
if page.IsSplitted {
|
||||
fileName = fmt.Sprintf("%04d-%02d%s", page.Index, page.SplitPartIndex, page.Extension)
|
||||
} else {
|
||||
fileName = fmt.Sprintf("%04d%s", page.Index, page.Extension)
|
||||
}
|
||||
fileName := resolvePageName(page, usedNames)
|
||||
|
||||
log.Debug().
|
||||
Str("output_path", outputFilePath).
|
||||
|
||||
@@ -101,6 +101,70 @@ func TestWriteChapterToCBZ(t *testing.T) {
|
||||
},
|
||||
expectedFiles: []string{"0000-01.jpg"},
|
||||
},
|
||||
{
|
||||
// --keep-filenames: the page stem is reused as-is with the
|
||||
// current Extension, regardless of the source extension.
|
||||
name: "Preserve original filename, extension swap",
|
||||
chapter: func(t *testing.T, dir string) *manga.Chapter {
|
||||
return &manga.Chapter{
|
||||
Pages: []*manga.PageFile{
|
||||
{
|
||||
Index: 0,
|
||||
Extension: ".webp",
|
||||
FilePath: createTempPage(t, dir, "image data", ".webp"),
|
||||
OriginalName: "page01.png",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
expectedFiles: []string{"page01.webp"},
|
||||
},
|
||||
{
|
||||
// --keep-filenames for a split page: stem is preserved, the
|
||||
// split part index still gets appended to keep parts distinct.
|
||||
name: "Split page with OriginalName",
|
||||
chapter: func(t *testing.T, dir string) *manga.Chapter {
|
||||
return &manga.Chapter{
|
||||
Pages: []*manga.PageFile{
|
||||
{
|
||||
Index: 0,
|
||||
Extension: ".webp",
|
||||
FilePath: createTempPage(t, dir, "split data", ".webp"),
|
||||
IsSplitted: true,
|
||||
SplitPartIndex: 2,
|
||||
OriginalName: "tall.png",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
expectedFiles: []string{"tall-02.webp"},
|
||||
},
|
||||
{
|
||||
// Two pages share the same OriginalName (rare but possible
|
||||
// with subdirectories): the first keeps the stem, the second
|
||||
// falls back to the indexed form to avoid duplicate zip
|
||||
// entries.
|
||||
name: "Duplicate OriginalName falls back to indexed form",
|
||||
chapter: func(t *testing.T, dir string) *manga.Chapter {
|
||||
return &manga.Chapter{
|
||||
Pages: []*manga.PageFile{
|
||||
{
|
||||
Index: 0,
|
||||
Extension: ".webp",
|
||||
FilePath: createTempPage(t, dir, "first", ".webp"),
|
||||
OriginalName: "cover.png",
|
||||
},
|
||||
{
|
||||
Index: 1,
|
||||
Extension: ".webp",
|
||||
FilePath: createTempPage(t, dir, "second", ".webp"),
|
||||
OriginalName: "cover.png",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
expectedFiles: []string{"cover.webp", "cover_0001.webp"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
|
||||
@@ -142,7 +142,13 @@ func IsAlreadyConverted(ctx context.Context, filePath string) (converted bool, e
|
||||
// ExtractChapter extracts an archive (CBZ/CBR) to a temp directory on disk.
|
||||
// Pages are streamed directly to files — no image data is held in memory.
|
||||
// Returns a Chapter with PageFile entries pointing to extracted files.
|
||||
func ExtractChapter(ctx context.Context, filePath string) (*manga.Chapter, error) {
|
||||
//
|
||||
// When keepFilenames is true, each PageFile has its OriginalName set to the
|
||||
// base filename of the entry inside the archive. Downstream code uses that
|
||||
// name to preserve the original page identity in the output CBZ (with the
|
||||
// extension swapped for format conversion). When false, OriginalName stays
|
||||
// empty and the sequential %04d naming convention is used instead.
|
||||
func ExtractChapter(ctx context.Context, filePath string, keepFilenames bool) (*manga.Chapter, error) {
|
||||
log.Debug().Str("file_path", filePath).Msg("Extracting chapter to disk")
|
||||
|
||||
// Create temp directory for extraction
|
||||
@@ -162,6 +168,22 @@ func ExtractChapter(ctx context.Context, filePath string) (*manga.Chapter, error
|
||||
TempDir: tempDir,
|
||||
}
|
||||
|
||||
// usedOriginalStems tracks the STEM (filename without extension) of every
|
||||
// OriginalName handed out in this chapter so stems stay unique across
|
||||
// pages. Tracking stems (not full names) prevents a downstream race in
|
||||
// pkg/converter/webp: the converter strips the OriginalName extension
|
||||
// and appends a target-format suffix (e.g. ".webp"), so two pages that
|
||||
// share a stem but differ in extension — e.g. a/page.png and b/page.jpg
|
||||
// (legal in zip) — would otherwise race on a single shared intermediate
|
||||
// output path. The map covers both same-stem-same-ext collisions (e.g.
|
||||
// a/page.png + b/page.png) and same-stem-different-ext collisions (e.g.
|
||||
// a/page.png + b/page.jpg). Allocated lazily so the keepFilenames=false
|
||||
// path stays allocation-free.
|
||||
var usedOriginalStems map[string]struct{}
|
||||
if keepFilenames {
|
||||
usedOriginalStems = make(map[string]struct{})
|
||||
}
|
||||
|
||||
// For CBZ files, read metadata from zip comment
|
||||
pathLower := strings.ToLower(filepath.Ext(filePath))
|
||||
if pathLower == ".cbz" {
|
||||
@@ -279,6 +301,9 @@ func ExtractChapter(ctx context.Context, filePath string) (*manga.Chapter, error
|
||||
Extension: ext,
|
||||
FilePath: outputPath,
|
||||
}
|
||||
if keepFilenames {
|
||||
page.OriginalName = allocateUniqueBaseName(archiveBaseName(path), pageIndex, usedOriginalStems)
|
||||
}
|
||||
chapter.Pages = append(chapter.Pages, page)
|
||||
|
||||
log.Debug().
|
||||
@@ -319,9 +344,76 @@ func isJunkFile(path string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// LoadChapter extracts the chapter from a CBZ/CBR file to disk.
|
||||
// It delegates to ExtractChapter and always extracts all pages.
|
||||
// Use IsAlreadyConverted for a fast conversion status check without extraction.
|
||||
func LoadChapter(filePath string) (*manga.Chapter, error) {
|
||||
return ExtractChapter(context.Background(), filePath)
|
||||
// archiveBaseName returns the bare base name of an archive entry, with
|
||||
// Windows-style backslash separators normalized to forward slashes before
|
||||
// the last-segment split. The archives library surfaces zip entry names
|
||||
// verbatim, so a name like "..\evil.png" (common from Windows-created
|
||||
// archives) would otherwise pass through filepath.Base unchanged on non-
|
||||
// Windows hosts and be written into the output CBZ as a path-traversal
|
||||
// shape. Normalizing the separators first and then taking the segment
|
||||
// after the final one guarantees the returned name is a bare base name
|
||||
// with no directory components, no backslashes, and no forward slashes —
|
||||
// which is what the downstream writer expects as a safe ZIP entry name.
|
||||
//
|
||||
// This intentionally does NOT touch filepath.Base calls in isJunkFile or
|
||||
// the ComicInfo.xml / converted.txt detection: those comparisons just
|
||||
// fail to match, and the entry then falls through to the non-image
|
||||
// extension filter, so no traversal-shaped data escapes the loader.
|
||||
func archiveBaseName(path string) string {
|
||||
normalized := strings.ReplaceAll(path, "\\", "/")
|
||||
if idx := strings.LastIndex(normalized, "/"); idx >= 0 {
|
||||
return normalized[idx+1:]
|
||||
}
|
||||
return normalized
|
||||
}
|
||||
|
||||
// allocateUniqueBaseName returns baseName when its stem is not already taken
|
||||
// by an earlier page in this chapter, or a collision-resolved variant
|
||||
// otherwise.
|
||||
//
|
||||
// Stem-uniqueness contract: the function guarantees that the STEM
|
||||
// (filename without extension) of the returned OriginalName is unique
|
||||
// among all names previously handed out from this chapter. That contract
|
||||
// matches what downstream code relies on —
|
||||
// pkg/converter/webp.intermediatePageName strips the OriginalName's
|
||||
// extension and appends a target-format suffix (e.g. ".webp"), so two
|
||||
// OriginalNames that share a stem would otherwise race on a single
|
||||
// intermediate output path. The stem is computed the same way downstream
|
||||
// does it (strings.TrimSuffix(name, filepath.Ext(name))) to keep both
|
||||
// definitions in lock-step.
|
||||
//
|
||||
// On collision the resolved name matches the existing fallback style used
|
||||
// by cbz_creator's resolvePageName: stem + "_%04d" + extension. The loop
|
||||
// checks the CANDIDATE's stem (not the full generated name) so two files
|
||||
// that share a stem but differ in extension — e.g. "page.png" and
|
||||
// "page.jpg" — resolve to, e.g., "page.png" and "page_0001.jpg", each
|
||||
// preserving its original extension. The starting suffix is pageIndex so
|
||||
// the resolved name stays in the same neighborhood as the page's archive
|
||||
// position. If the candidate's stem is itself already taken
|
||||
// (astronomically rare: the source archive would need both the original
|
||||
// name and a matching indexed name), the suffix is incremented until a
|
||||
// free stem is found. The chosen name's stem is recorded in usedStems so
|
||||
// subsequent calls cannot pick it again.
|
||||
func allocateUniqueBaseName(baseName string, pageIndex uint16, usedStems map[string]struct{}) string {
|
||||
ext := filepath.Ext(baseName)
|
||||
stem := strings.TrimSuffix(baseName, ext)
|
||||
if _, taken := usedStems[stem]; !taken {
|
||||
usedStems[stem] = struct{}{}
|
||||
return baseName
|
||||
}
|
||||
for suffix := int(pageIndex); ; suffix++ {
|
||||
candidateStem := fmt.Sprintf("%s_%04d", stem, suffix)
|
||||
if _, taken := usedStems[candidateStem]; !taken {
|
||||
usedStems[candidateStem] = struct{}{}
|
||||
return candidateStem + ext
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// LoadChapter extracts the chapter from a CBZ/CBR file to disk.
|
||||
// It delegates to ExtractChapter with keepFilenames=false and always
|
||||
// extracts all pages. Use IsAlreadyConverted for a fast conversion status
|
||||
// check without extraction.
|
||||
func LoadChapter(filePath string) (*manga.Chapter, error) {
|
||||
return ExtractChapter(context.Background(), filePath, false)
|
||||
}
|
||||
|
||||
@@ -6,6 +6,8 @@ import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -157,12 +159,12 @@ func TestIsAlreadyConverted_CBZWithNonDateComment(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestExtractChapter_NonexistentFile(t *testing.T) {
|
||||
_, err := ExtractChapter(context.Background(), "/nonexistent/file.cbz")
|
||||
_, err := ExtractChapter(context.Background(), "/nonexistent/file.cbz", false)
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
func TestExtractChapter_PageExtensions(t *testing.T) {
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz")
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", false)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = chapter.Cleanup() }()
|
||||
|
||||
@@ -174,17 +176,260 @@ func TestExtractChapter_PageExtensions(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestExtractChapter_PagesHaveSequentialIndices(t *testing.T) {
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz")
|
||||
t.Run("default sequential naming", func(t *testing.T) {
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", false)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = chapter.Cleanup() }()
|
||||
|
||||
for i, page := range chapter.Pages {
|
||||
assert.Equal(t, uint16(i), page.Index)
|
||||
assert.Empty(t, page.OriginalName, "keep-filenames disabled: OriginalName must stay empty")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("keep-filenames records OriginalName", func(t *testing.T) {
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", true)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = chapter.Cleanup() }()
|
||||
|
||||
require.NotEmpty(t, chapter.Pages)
|
||||
for i, page := range chapter.Pages {
|
||||
assert.Equal(t, uint16(i), page.Index)
|
||||
assert.NotEmpty(t, page.OriginalName, "keep-filenames enabled: OriginalName must be set for every page")
|
||||
// OriginalName must be a bare base name (no directory prefix), even
|
||||
// when the source archive nests pages in subdirectories.
|
||||
assert.Equal(t, filepath.Base(page.OriginalName), page.OriginalName,
|
||||
"OriginalName should be a base filename with no path components")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// writeCollisionCBZ builds a CBZ containing the same image base name in two
|
||||
// different subdirectories plus a normal unique page. The test fixture is
|
||||
// specifically crafted to reproduce the race fixed in ExtractChapter: when
|
||||
// keep-filenames naively copied filepath.Base(path) into OriginalName, both
|
||||
// a/page.png and b/page.png would have ended up with the same stem and the
|
||||
// WebP converter would have raced on a single shared intermediate path.
|
||||
func writeCollisionCBZ(t *testing.T, dir string) string {
|
||||
t.Helper()
|
||||
cbzPath := filepath.Join(dir, "collisions.cbz")
|
||||
f, err := os.Create(cbzPath)
|
||||
require.NoError(t, err)
|
||||
w := zip.NewWriter(f)
|
||||
|
||||
for _, entry := range []string{"a/page.png", "b/page.png", "cover.png"} {
|
||||
fw, err := w.Create(entry)
|
||||
require.NoError(t, err)
|
||||
_, err = fw.Write([]byte("dummy image bytes for " + entry))
|
||||
require.NoError(t, err)
|
||||
}
|
||||
require.NoError(t, w.Close())
|
||||
require.NoError(t, f.Close())
|
||||
return cbzPath
|
||||
}
|
||||
|
||||
func TestExtractChapter_KeepFilenames_DeduplicatesCollidingNames(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
cbzPath := writeCollisionCBZ(t, tmpDir)
|
||||
|
||||
t.Run("keep-filenames resolves colliding base names", func(t *testing.T) {
|
||||
chapter, err := ExtractChapter(context.Background(), cbzPath, true)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = chapter.Cleanup() }()
|
||||
|
||||
require.Len(t, chapter.Pages, 3, "expected 3 pages: two colliding + one unique")
|
||||
|
||||
// All OriginalNames must be non-empty bare base names and unique.
|
||||
seen := make(map[string]struct{}, len(chapter.Pages))
|
||||
for _, page := range chapter.Pages {
|
||||
assert.NotEmpty(t, page.OriginalName, "page %d must have OriginalName set", page.Index)
|
||||
assert.Equal(t, filepath.Base(page.OriginalName), page.OriginalName,
|
||||
"page %d OriginalName should be a bare base filename", page.Index)
|
||||
if _, dup := seen[page.OriginalName]; dup {
|
||||
t.Errorf("duplicate OriginalName across pages: %q", page.OriginalName)
|
||||
}
|
||||
seen[page.OriginalName] = struct{}{}
|
||||
}
|
||||
|
||||
// The unique page keeps its original name; the two colliding pages
|
||||
// must come out as one bare "page.png" and one indexed variant.
|
||||
assert.Contains(t, seen, "page.png", "one colliding page should keep the bare page.png name")
|
||||
assert.Contains(t, seen, "cover.png", "the unique page should keep its original name")
|
||||
|
||||
indexedPattern := regexp.MustCompile(`^page_\d{4}\.png$`)
|
||||
indexedCount := 0
|
||||
for name := range seen {
|
||||
if indexedPattern.MatchString(name) {
|
||||
indexedCount++
|
||||
}
|
||||
}
|
||||
assert.Equal(t, 1, indexedCount,
|
||||
"exactly one colliding page should be resolved to the page_NNNN.png pattern, got %v", seen)
|
||||
})
|
||||
|
||||
t.Run("keep-filenames off leaves OriginalName empty (regression guard)", func(t *testing.T) {
|
||||
// Same collision-prone archive, but with keep-filenames off: the
|
||||
// dedup logic must not run, so OriginalName stays empty for every
|
||||
// page. This is the historical default and must not regress.
|
||||
chapter, err := ExtractChapter(context.Background(), cbzPath, false)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = chapter.Cleanup() }()
|
||||
|
||||
require.Len(t, chapter.Pages, 3)
|
||||
for _, page := range chapter.Pages {
|
||||
assert.Empty(t, page.OriginalName, "page %d must have empty OriginalName when keep-filenames is off", page.Index)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// writeBackslashCBZ builds a CBZ whose entry names mix Windows-style
|
||||
// backslash separators with a normal forward-slash page. The fixture
|
||||
// reproduces the CodeRabbit finding for PR #217: archive entries like
|
||||
// "subdir\page.png" and "..\evil.png" must not surface verbatim as the
|
||||
// OriginalName written into the output CBZ, since Windows ZIP consumers
|
||||
// can interpret backslashes as path traversal.
|
||||
func writeBackslashCBZ(t *testing.T, dir string) string {
|
||||
t.Helper()
|
||||
cbzPath := filepath.Join(dir, "backslash.cbz")
|
||||
f, err := os.Create(cbzPath)
|
||||
require.NoError(t, err)
|
||||
w := zip.NewWriter(f)
|
||||
|
||||
// archive/zip stores entry names verbatim, so the bytes on the wire
|
||||
// carry the backslashes through to fs.WalkDir in ExtractChapter.
|
||||
for _, entry := range []string{`subdir\page.png`, `..\evil.png`, "cover.png"} {
|
||||
fw, err := w.Create(entry)
|
||||
require.NoError(t, err)
|
||||
_, err = fw.Write([]byte("dummy image bytes for " + entry))
|
||||
require.NoError(t, err)
|
||||
}
|
||||
require.NoError(t, w.Close())
|
||||
require.NoError(t, f.Close())
|
||||
return cbzPath
|
||||
}
|
||||
|
||||
func TestExtractChapter_KeepFilenames_NormalizesWindowsSeparators(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
cbzPath := writeBackslashCBZ(t, tmpDir)
|
||||
|
||||
chapter, err := ExtractChapter(context.Background(), cbzPath, true)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = chapter.Cleanup() }()
|
||||
|
||||
for i, page := range chapter.Pages {
|
||||
assert.Equal(t, uint16(i), page.Index)
|
||||
require.Len(t, chapter.Pages, 3, "expected 3 pages: two backslash entries + one normal")
|
||||
|
||||
// Every OriginalName must be a bare base name: no backslashes, no
|
||||
// forward slashes, and unique across the chapter.
|
||||
seen := make(map[string]struct{}, len(chapter.Pages))
|
||||
for _, page := range chapter.Pages {
|
||||
assert.NotEmpty(t, page.OriginalName, "page %d must have OriginalName set", page.Index)
|
||||
assert.NotContains(t, page.OriginalName, `\`, "page %d OriginalName must not contain backslash, got %q", page.Index, page.OriginalName)
|
||||
assert.NotContains(t, page.OriginalName, "/", "page %d OriginalName must not contain forward slash, got %q", page.Index, page.OriginalName)
|
||||
assert.Equal(t, filepath.Base(page.OriginalName), page.OriginalName,
|
||||
"page %d OriginalName should be a bare base filename", page.Index)
|
||||
if _, dup := seen[page.OriginalName]; dup {
|
||||
t.Errorf("duplicate OriginalName across pages: %q", page.OriginalName)
|
||||
}
|
||||
seen[page.OriginalName] = struct{}{}
|
||||
}
|
||||
|
||||
// The two backslash entries must collapse to their bare bases; the
|
||||
// "..\evil.png" entry's leading "..\evil" must not be carried through.
|
||||
assert.Contains(t, seen, "page.png", "subdir\\page.png should normalize to bare page.png")
|
||||
assert.Contains(t, seen, "evil.png", "..\\evil.png should normalize to bare evil.png")
|
||||
assert.Contains(t, seen, "cover.png", "cover.png should pass through unchanged")
|
||||
}
|
||||
|
||||
// writeSameStemDifferentExtCBZ builds a CBZ containing two same-stem pages
|
||||
// with different extensions plus a distinct unique page. The fixture
|
||||
// reproduces the CodeRabbit finding for PR #217: a/page.png and b/page.jpg
|
||||
// share a stem but do NOT share a full filename, so naive full-name
|
||||
// deduplication lets both OriginalNames through and the WebP converter then
|
||||
// races on a single shared intermediate output path (outputDir/page.webp).
|
||||
// The fix flips the dedup tracking to STEM-uniqueness, so one of the
|
||||
// colliding pair must come out as a bare "page.<ext>" and the other as
|
||||
// "page_NNNN.<ext>" — each preserving its original extension.
|
||||
func writeSameStemDifferentExtCBZ(t *testing.T, dir string) string {
|
||||
t.Helper()
|
||||
cbzPath := filepath.Join(dir, "same_stem.cbz")
|
||||
f, err := os.Create(cbzPath)
|
||||
require.NoError(t, err)
|
||||
w := zip.NewWriter(f)
|
||||
|
||||
for _, entry := range []string{"a/page.png", "b/page.jpg", "cover.png"} {
|
||||
fw, err := w.Create(entry)
|
||||
require.NoError(t, err)
|
||||
_, err = fw.Write([]byte("dummy image bytes for " + entry))
|
||||
require.NoError(t, err)
|
||||
}
|
||||
require.NoError(t, w.Close())
|
||||
require.NoError(t, f.Close())
|
||||
return cbzPath
|
||||
}
|
||||
|
||||
func TestExtractChapter_KeepFilenames_DeduplicatesSameStemDifferentExtensions(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
cbzPath := writeSameStemDifferentExtCBZ(t, tmpDir)
|
||||
|
||||
chapter, err := ExtractChapter(context.Background(), cbzPath, true)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = chapter.Cleanup() }()
|
||||
|
||||
require.Len(t, chapter.Pages, 3, "expected 3 pages: two same-stem + one unique")
|
||||
|
||||
// Every OriginalName must be a bare base name; full names AND stems
|
||||
// must each be unique across the chapter. The stem-uniqueness check is
|
||||
// the contract the WebP converter relies on (it strips the extension
|
||||
// and appends ".webp" when computing intermediatePageName).
|
||||
seenNames := make(map[string]struct{}, len(chapter.Pages))
|
||||
seenStems := make(map[string]struct{}, len(chapter.Pages))
|
||||
for _, page := range chapter.Pages {
|
||||
assert.NotEmpty(t, page.OriginalName, "page %d must have OriginalName set", page.Index)
|
||||
assert.Equal(t, filepath.Base(page.OriginalName), page.OriginalName,
|
||||
"page %d OriginalName should be a bare base filename", page.Index)
|
||||
if _, dup := seenNames[page.OriginalName]; dup {
|
||||
t.Errorf("duplicate OriginalName full name across pages: %q", page.OriginalName)
|
||||
}
|
||||
seenNames[page.OriginalName] = struct{}{}
|
||||
|
||||
stem := strings.TrimSuffix(page.OriginalName, filepath.Ext(page.OriginalName))
|
||||
if _, dup := seenStems[stem]; dup {
|
||||
t.Errorf("duplicate OriginalName stem across pages: %q (from %q)", stem, page.OriginalName)
|
||||
}
|
||||
seenStems[stem] = struct{}{}
|
||||
}
|
||||
|
||||
// The unique page keeps its original name untouched.
|
||||
assert.Contains(t, seenNames, "cover.png", "the unique page should keep its original name")
|
||||
|
||||
// The colliding pair must split into one bare "page.<ext>" and one
|
||||
// "page_NNNN.<ext>". The extensions must differ — each must keep the
|
||||
// extension of the archive entry it came from. Walk order is not
|
||||
// guaranteed, so the test checks both shapes regardless of which
|
||||
// entry comes first.
|
||||
barePattern := regexp.MustCompile(`^page\.(png|jpg)$`)
|
||||
indexedPattern := regexp.MustCompile(`^page_\d{4}\.(png|jpg)$`)
|
||||
|
||||
var bareExt, indexedExt string
|
||||
for name := range seenNames {
|
||||
if barePattern.MatchString(name) {
|
||||
bareExt = filepath.Ext(name)
|
||||
}
|
||||
if indexedPattern.MatchString(name) {
|
||||
indexedExt = filepath.Ext(name)
|
||||
}
|
||||
}
|
||||
assert.NotEmpty(t, bareExt,
|
||||
"expected one bare page.<ext> OriginalName from the colliding pair, got %v", seenNames)
|
||||
assert.NotEmpty(t, indexedExt,
|
||||
"expected one page_NNNN.<ext> OriginalName from the colliding pair, got %v", seenNames)
|
||||
assert.NotEqual(t, bareExt, indexedExt,
|
||||
"the bare and indexed variants must keep their original different extensions, got bare=%q indexed=%q",
|
||||
bareExt, indexedExt)
|
||||
}
|
||||
|
||||
func TestExtractChapter_Cleanup(t *testing.T) {
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz")
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", false)
|
||||
require.NoError(t, err)
|
||||
|
||||
tempDir := chapter.TempDir
|
||||
@@ -199,7 +444,7 @@ func TestExtractChapter_Cleanup(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestExtractChapter_CBR(t *testing.T) {
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 1.cbr")
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 1.cbr", false)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = chapter.Cleanup() }()
|
||||
|
||||
@@ -212,7 +457,7 @@ func TestExtractChapter_CBR(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestExtractChapter_ConvertedStatus(t *testing.T) {
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 10_converted.cbz")
|
||||
chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 10_converted.cbz", false)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = chapter.Cleanup() }()
|
||||
|
||||
@@ -356,7 +601,7 @@ func TestExtractChapter_WithConvertedTxt(t *testing.T) {
|
||||
_ = w.Close()
|
||||
_ = f.Close()
|
||||
|
||||
chapter, err := ExtractChapter(context.Background(), cbzPath)
|
||||
chapter, err := ExtractChapter(context.Background(), cbzPath, false)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = chapter.Cleanup() }()
|
||||
|
||||
|
||||
@@ -13,4 +13,10 @@ type PageFile struct {
|
||||
IsSplitted bool
|
||||
// SplitPartIndex is the part index when the page was split.
|
||||
SplitPartIndex uint16
|
||||
// OriginalName is the base filename (e.g. "page01.png") of the page as
|
||||
// it appeared in the source archive, recorded when the --keep-filenames
|
||||
// flag is enabled. Empty when the flag is off or the source name is
|
||||
// unknown. When set, downstream code uses this stem (with the final
|
||||
// Extension swapped in) instead of the default %04d sequential name.
|
||||
OriginalName string
|
||||
}
|
||||
|
||||
@@ -21,7 +21,12 @@ type OptimizeOptions struct {
|
||||
Quality uint8
|
||||
Override bool
|
||||
Split bool
|
||||
Timeout time.Duration
|
||||
// KeepFilenames preserves the original base filename of each page inside
|
||||
// the output CBZ (with the extension swapped for format conversion)
|
||||
// instead of the historical %04d sequential naming. Off by default so
|
||||
// existing behavior is unchanged.
|
||||
KeepFilenames bool
|
||||
Timeout time.Duration
|
||||
}
|
||||
|
||||
// Optimize optimizes a CBZ/CBR file using the specified converter.
|
||||
@@ -38,6 +43,7 @@ func Optimize(options *OptimizeOptions) error {
|
||||
Uint8("quality", options.Quality).
|
||||
Bool("override", options.Override).
|
||||
Bool("split", options.Split).
|
||||
Bool("keep_filenames", options.KeepFilenames).
|
||||
Msg("Optimization parameters")
|
||||
|
||||
// Step 1: Fast conversion check before extracting (new requirement)
|
||||
@@ -64,7 +70,7 @@ func Optimize(options *OptimizeOptions) error {
|
||||
extractCtx = context.Background()
|
||||
}
|
||||
|
||||
chapter, err := cbz.ExtractChapter(extractCtx, options.Path)
|
||||
chapter, err := cbz.ExtractChapter(extractCtx, options.Path, options.KeepFilenames)
|
||||
if err != nil {
|
||||
log.Error().Str("file", options.Path).Err(err).Msg("Failed to extract chapter")
|
||||
return fmt.Errorf("failed to extract chapter: %w", err)
|
||||
|
||||
@@ -51,13 +51,6 @@ func TestOptimizeIntegration(t *testing.T) {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// Skip the "large" fixtures directory: it holds the Git LFS-tracked
|
||||
// fixture used exclusively by TestOptimizeIntegration_LargeFile,
|
||||
// which may only be a small LFS pointer file if the content wasn't
|
||||
// fetched, and shouldn't be exercised by this generic test.
|
||||
if info.IsDir() && filepath.Base(path) == "large" {
|
||||
return filepath.SkipDir
|
||||
}
|
||||
if !info.IsDir() {
|
||||
fileName := strings.ToLower(info.Name())
|
||||
if (strings.HasSuffix(fileName, ".cbz") || strings.HasSuffix(fileName, ".cbr")) && !strings.Contains(fileName, "converted") {
|
||||
|
||||
@@ -1,121 +0,0 @@
|
||||
package utils
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/belphemur/CBZOptimizer/v2/internal/utils/errs"
|
||||
"github.com/belphemur/CBZOptimizer/v2/pkg/converter"
|
||||
"github.com/belphemur/CBZOptimizer/v2/pkg/converter/constant"
|
||||
"github.com/rs/zerolog/log"
|
||||
)
|
||||
|
||||
// largeTestFile is a ~1GB synthetic CBZ fixture stored via Git LFS (see
|
||||
// .gitattributes). It is used to exercise the optimize pipeline with a
|
||||
// chapter large enough to make in-memory-only handling impractical, and to
|
||||
// validate that converted pages are streamed to/from a staging temp folder
|
||||
// (see manga.Page.TempFilePath / manga.Chapter.TempDir) instead of blowing
|
||||
// up memory usage.
|
||||
const largeTestFile = "../../testdata/large/large_chapter.cbz"
|
||||
|
||||
// TestOptimizeIntegration_LargeFile is opt-in (set CBZ_RUN_LARGE_FILE_TEST=1)
|
||||
// since it processes a ~1GB fixture and can take a while to run. It is
|
||||
// automatically skipped if the fixture is unavailable (e.g. Git LFS content
|
||||
// wasn't fetched, leaving only a pointer file) or in short mode.
|
||||
func TestOptimizeIntegration_LargeFile(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("Skipping large file integration test in short mode")
|
||||
}
|
||||
if os.Getenv("CBZ_RUN_LARGE_FILE_TEST") == "" {
|
||||
t.Skip("Skipping large file integration test; set CBZ_RUN_LARGE_FILE_TEST=1 to run it")
|
||||
}
|
||||
|
||||
info, err := os.Stat(largeTestFile)
|
||||
if err != nil {
|
||||
t.Skipf("large test fixture not found: %v", err)
|
||||
}
|
||||
// If Git LFS content wasn't fetched (e.g. `actions/checkout` without
|
||||
// `lfs: true`), the file on disk is just a small pointer text file
|
||||
// rather than the real ~1GB fixture. Detect and skip gracefully instead
|
||||
// of failing the whole suite.
|
||||
const minExpectedSize = 500 * 1024 * 1024 // 500MB
|
||||
if info.Size() < minExpectedSize {
|
||||
t.Skipf("large test fixture looks like a Git LFS pointer (size=%d), skipping; run `git lfs pull`", info.Size())
|
||||
}
|
||||
|
||||
tempDir, err := os.MkdirTemp("", "test_optimize_large_file")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer errs.CaptureGeneric(&err, os.RemoveAll, tempDir, "failed to remove temporary directory")
|
||||
|
||||
converterInstance, err := converter.Get(constant.WebP)
|
||||
if err != nil {
|
||||
t.Skip("WebP converter not available, skipping large file integration test")
|
||||
}
|
||||
if err := converterInstance.PrepareConverter(); err != nil {
|
||||
t.Skip("Failed to prepare WebP converter, skipping large file integration test")
|
||||
}
|
||||
|
||||
cbzFile := filepath.Join(tempDir, "large_chapter.cbz")
|
||||
if err := copyFile(largeTestFile, cbzFile); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
var memBefore, memAfter runtime.MemStats
|
||||
runtime.GC()
|
||||
runtime.ReadMemStats(&memBefore)
|
||||
|
||||
options := &OptimizeOptions{
|
||||
ChapterConverter: converterInstance,
|
||||
Path: cbzFile,
|
||||
Quality: 85,
|
||||
Override: false,
|
||||
Split: true,
|
||||
}
|
||||
|
||||
err = Optimize(options)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to optimize large chapter: %v", err)
|
||||
}
|
||||
|
||||
runtime.GC()
|
||||
runtime.ReadMemStats(&memAfter)
|
||||
log.Info().
|
||||
Uint64("heap_alloc_before", memBefore.HeapAlloc).
|
||||
Uint64("heap_alloc_after", memAfter.HeapAlloc).
|
||||
Int64("input_size", info.Size()).
|
||||
Msg("Large file integration test memory usage")
|
||||
|
||||
outputFile := strings.TrimSuffix(cbzFile, ".cbz") + "_converted.cbz"
|
||||
if _, err := os.Stat(outputFile); err != nil {
|
||||
t.Fatalf("expected converted output file %s to exist: %v", outputFile, err)
|
||||
}
|
||||
}
|
||||
|
||||
// copyFile copies src to dst using streaming file I/O so that the whole
|
||||
// file content is never held in memory at once, which matters for the
|
||||
// large fixture used by this test.
|
||||
func copyFile(src, dst string) (err error) {
|
||||
in, err := os.Open(src)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to open source file: %w", err)
|
||||
}
|
||||
defer errs.Capture(&err, in.Close, "failed to close source file")
|
||||
|
||||
out, err := os.Create(dst)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create destination file: %w", err)
|
||||
}
|
||||
defer errs.Capture(&err, out.Close, "failed to close destination file")
|
||||
|
||||
if _, err := io.Copy(out, in); err != nil {
|
||||
return fmt.Errorf("failed to copy file contents: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -25,6 +25,26 @@ import (
|
||||
|
||||
const webpMaxHeight = 16383
|
||||
|
||||
// intermediatePageName returns the on-disk filename used for a page's WebP
|
||||
// intermediate during conversion. When the page carries an OriginalName
|
||||
// (recorded by --keep-filenames), its stem is reused so the temp file line
|
||||
// up with the name the archive writer will pick. Otherwise the historical
|
||||
// %04d indexed naming is kept. The splitSuffix argument is appended verbatim
|
||||
// after the stem (e.g. "-00", "-01") for split parts and is empty for the
|
||||
// happy-path single output. A leading dash is only added when both the
|
||||
// split suffix and the original-name stem are present, so the indexed form
|
||||
// stays as %04d-%02d.
|
||||
func intermediatePageName(page *manga.PageFile, splitSuffix string) string {
|
||||
if page.OriginalName != "" {
|
||||
stem := strings.TrimSuffix(page.OriginalName, filepath.Ext(page.OriginalName))
|
||||
return stem + splitSuffix + ".webp"
|
||||
}
|
||||
if splitSuffix == "" {
|
||||
return fmt.Sprintf("%04d.webp", page.Index)
|
||||
}
|
||||
return fmt.Sprintf("%04d%s.webp", page.Index, splitSuffix)
|
||||
}
|
||||
|
||||
type Converter struct {
|
||||
maxHeight int
|
||||
cropHeight int
|
||||
@@ -218,22 +238,26 @@ func (converter *Converter) convertPageFile(ctx context.Context, page *manga.Pag
|
||||
Str("input", page.FilePath).
|
||||
Msg("Converting page file")
|
||||
|
||||
// If the page is already WebP, just return it as-is
|
||||
// If the page is already WebP, just return it as-is. The returned page
|
||||
// keeps any OriginalName set during extraction so the archive writer can
|
||||
// honor --keep-filenames for the final entry name.
|
||||
if strings.ToLower(page.Extension) == ".webp" {
|
||||
log.Debug().Uint16("page_index", page.Index).Msg("Page already WebP, skipping")
|
||||
return []*manga.PageFile{page}, nil
|
||||
}
|
||||
|
||||
// Try direct file-to-file conversion first (happy path — no memory allocation)
|
||||
outputPath := filepath.Join(outputDir, fmt.Sprintf("%04d.webp", page.Index))
|
||||
outputPath := filepath.Join(outputDir, intermediatePageName(page, ""))
|
||||
err := EncodeFile(page.FilePath, outputPath, uint(quality))
|
||||
|
||||
if err == nil {
|
||||
// Success! No image decoding needed.
|
||||
// Success! No image decoding needed. Preserve OriginalName so
|
||||
// --keep-filenames carries through to the final zip entry name.
|
||||
return []*manga.PageFile{{
|
||||
Index: page.Index,
|
||||
Extension: ".webp",
|
||||
FilePath: outputPath,
|
||||
Index: page.Index,
|
||||
Extension: ".webp",
|
||||
FilePath: outputPath,
|
||||
OriginalName: page.OriginalName,
|
||||
}}, nil
|
||||
}
|
||||
|
||||
@@ -316,7 +340,7 @@ func (converter *Converter) splitAndConvert(ctx context.Context, page *manga.Pag
|
||||
partHeight = height - yOffset
|
||||
}
|
||||
|
||||
outputPath := filepath.Join(outputDir, fmt.Sprintf("%04d-%02d.webp", page.Index, i))
|
||||
outputPath := filepath.Join(outputDir, intermediatePageName(page, fmt.Sprintf("-%02d", i)))
|
||||
err := EncodeFileWithCrop(page.FilePath, outputPath, uint(quality), 0, yOffset, width, partHeight)
|
||||
|
||||
if err != nil {
|
||||
@@ -334,6 +358,7 @@ func (converter *Converter) splitAndConvert(ctx context.Context, page *manga.Pag
|
||||
FilePath: outputPath,
|
||||
IsSplitted: true,
|
||||
SplitPartIndex: uint16(i),
|
||||
OriginalName: page.OriginalName,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -485,3 +485,36 @@ func TestEncodeFileWithCrop(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
assert.Greater(t, info.Size(), int64(0))
|
||||
}
|
||||
|
||||
// TestIntermediatePageName_StemsAreUnique pins the contract that
|
||||
// intermediatePageName must produce a different on-disk filename for two
|
||||
// pages whose OriginalNames share a stem via the original archive entry
|
||||
// name. ExtractChapter guarantees stem-unique OriginalNames per chapter
|
||||
// (so, e.g., a/page.png and b/page.jpg come out as "page.png" and
|
||||
// "page_0001.jpg"), and the converter must reflect that: stripping the
|
||||
// extension and appending ".webp" must not collapse two distinct stems
|
||||
// onto the same intermediate path.
|
||||
func TestIntermediatePageName_StemsAreUnique(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
originalName string
|
||||
splitSuffix string
|
||||
}{
|
||||
{name: "bare stem", originalName: "page.png", splitSuffix: ""},
|
||||
{name: "indexed stem (post-fix)", originalName: "page_0001.jpg", splitSuffix: ""},
|
||||
{name: "bare stem + split suffix", originalName: "page.png", splitSuffix: "-00"},
|
||||
{name: "indexed stem + split suffix", originalName: "page_0001.jpg", splitSuffix: "-00"},
|
||||
}
|
||||
|
||||
names := make(map[string]struct{}, len(tests))
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
page := &manga.PageFile{Index: 0, OriginalName: tt.originalName}
|
||||
got := intermediatePageName(page, tt.splitSuffix)
|
||||
if _, dup := names[got]; dup {
|
||||
t.Errorf("intermediate name %q collides with an earlier page's intermediate name", got)
|
||||
}
|
||||
names[got] = struct{}{}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Vendored
-3
@@ -1,3 +0,0 @@
|
||||
version https://git-lfs.github.com/spec/v1
|
||||
oid sha256:8880c8909521c3716a339a39a29a4bf1114921a8b4651ff8ca04da7ce3a54b6a
|
||||
size 1001668690
|
||||
Reference in New Issue
Block a user