From 5447ecb07202c16b8d86d60c68266ac4e0053997 Mon Sep 17 00:00:00 2001 From: Nostalgia Date: Mon, 14 Sep 2026 20:30:17 +0800 Subject: [PATCH 1/6] fix(s3): encode multipart fixture paths (#3074) test(s3): encode multipart fixture paths - Serialize the Local storage addition instead of interpolating filesystem paths into JSON. - Keep multipart fixture behavior portable across Windows and Linux. Co-authored-by: nostalume Co-authored-by: Codex <267193182+codex@users.noreply.github.com> --- server/s3/multipart_test.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/server/s3/multipart_test.go b/server/s3/multipart_test.go index 7adc4c5e3..a7349583d 100644 --- a/server/s3/multipart_test.go +++ b/server/s3/multipart_test.go @@ -3,6 +3,7 @@ package s3 import ( "bytes" "context" + "encoding/json" "errors" "os" "path/filepath" @@ -67,11 +68,21 @@ func setupMultipartBackend(t *testing.T) (*s3Backend, string) { t.Fatalf("mkdir local root: %v", err) } t.Cleanup(func() { _ = os.RemoveAll(localRoot) }) + addition, err := json.Marshal(struct { + RootFolderPath string `json:"root_folder_path"` + Thumbnail bool `json:"thumbnail"` + }{ + RootFolderPath: localRoot, + Thumbnail: false, + }) + if err != nil { + t.Fatalf("marshal local storage addition: %v", err) + } _, err = op.CreateStorage(ctx, model.Storage{ Driver: "Local", MountPath: mount, - Addition: `{"root_folder_path":"` + localRoot + `","thumbnail":false}`, + Addition: string(addition), }) if err != nil { t.Fatalf("create local storage: %+v", err) From 4580c4db335aa4d163a64416a94731ec89f7b91d Mon Sep 17 00:00:00 2001 From: Fighting Date: Tue, 15 Sep 2026 23:13:10 +0800 Subject: [PATCH 2/6] fix(alist_v3): set child paths so nested directories resolve (#3019) * fix(alist_v3): set child paths so nested directories resolve - Set `Path` on every object returned by `List`, matching the OpenList driver. `op.Get` hands a child object straight back to `List`, so a child without a path made the driver request `""` from the upstream server, which answered with its own root: every directory below the mount point served the same listing back, endlessly. - Add tests covering the child paths and a three-level descent. Co-authored-by: Claude <81847+claude@users.noreply.github.com> * test(alist_v3): trim the child-path test to a single case Collapse the two tests into one two-level descent, drop the recorder type and the helper funcs, and inline the driver setup. 173 -> 65 lines. Co-authored-by: Claude <81847+claude@users.noreply.github.com> --------- Co-authored-by: Fighting <3899648+Elity@users.noreply.github.com> Co-authored-by: Claude <81847+claude@users.noreply.github.com> --- drivers/alist_v3/driver.go | 1 + drivers/alist_v3/driver_test.go | 65 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 drivers/alist_v3/driver_test.go diff --git a/drivers/alist_v3/driver.go b/drivers/alist_v3/driver.go index a9fc44fcb..ddfb0a962 100644 --- a/drivers/alist_v3/driver.go +++ b/drivers/alist_v3/driver.go @@ -95,6 +95,7 @@ func (d *AListV3) List(ctx context.Context, dir model.Obj, args model.ListArgs) file := model.ObjThumb{ Object: model.Object{ Name: f.Name, + Path: path.Join(dir.GetPath(), f.Name), Modified: f.Modified, Ctime: f.Created, Size: f.Size, diff --git a/drivers/alist_v3/driver_test.go b/drivers/alist_v3/driver_test.go new file mode 100644 index 000000000..bb8e009ea --- /dev/null +++ b/drivers/alist_v3/driver_test.go @@ -0,0 +1,65 @@ +package alist_v3 + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/OpenListTeam/OpenList/v4/drivers/base" + "github.com/OpenListTeam/OpenList/v4/internal/driver" + "github.com/OpenListTeam/OpenList/v4/internal/model" + "github.com/go-resty/resty/v2" +) + +// TestListSetsChildPaths descends two levels the way op.Get does, feeding an +// object from one listing back into List as dir. Without a Path on that object +// the driver asks upstream for "", which a real server answers with its own +// root -- hence the fake upstream's fallback, and the endless self-similar tree. +func TestListSetsChildPaths(t *testing.T) { + tree := map[string][]ObjResp{ + "/": {{Name: "root-marker", IsDir: true}}, + "/drive": {{Name: "concerts", IsDir: true}}, + "/drive/concerts": {{Name: "show.mkv"}}, + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var req ListReq + _ = json.NewDecoder(r.Body).Decode(&req) + content, ok := tree[req.Path] + if !ok { + content = tree["/"] + } + w.Header().Set("Content-Type", "application/json") // resty only unmarshals JSON + _ = json.NewEncoder(w).Encode(map[string]any{ + "code": 200, "message": "success", + "data": map[string]any{"content": content, "total": len(content)}, + }) + })) + t.Cleanup(srv.Close) + + // conf.Conf is nil outside a booted server, so base.InitClient() is unusable. + prev := base.RestyClient + base.RestyClient = resty.New().SetTimeout(5 * time.Second) + t.Cleanup(func() { base.RestyClient = prev }) + + d := &AListV3{Addition: Addition{ + RootPath: driver.RootPath{RootFolderPath: "/drive"}, + Address: srv.URL, + }} + dir := model.Obj(&model.Object{Path: "/drive", IsFolder: true}) + for _, want := range []string{"/drive/concerts", "/drive/concerts/show.mkv"} { + objs, err := d.List(context.Background(), dir, model.ListArgs{}) + if err != nil { + t.Fatalf("List(%q): %v", dir.GetPath(), err) + } + if len(objs) != 1 { + t.Fatalf("List(%q) returned %d objects, want 1", dir.GetPath(), len(objs)) + } + if got := objs[0].GetPath(); got != want { + t.Fatalf("child of %q has path %q, want %q", dir.GetPath(), got, want) + } + dir = objs[0] + } +} From 084c008102142b4188f3dc0bdc86d2d4976a4bf0 Mon Sep 17 00:00:00 2001 From: ShenLin <773933146@qq.com> Date: Sun, 20 Sep 2026 14:25:20 +0800 Subject: [PATCH 3/6] chore(build): replace uncontrolled email domain (#3103) - Use the controlled oplist.org domain for embedded build authors - Update shell and release workflow build metadata Signed-off-by: jyxjjj <16695261+jyxjjj@users.noreply.github.com> Co-authored-by: Codex <267193182+codex@users.noreply.github.com> --- .github/workflows/beta_release.yml | 2 +- .github/workflows/build.yml | 2 +- build.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/beta_release.yml b/.github/workflows/beta_release.yml index cb42fef5e..6ebf46462 100644 --- a/.github/workflows/beta_release.yml +++ b/.github/workflows/beta_release.yml @@ -136,7 +136,7 @@ jobs: musl-base-url: "https://github.com/OpenListTeam/musl-compilers/releases/latest/download/" x-flags: | github.com/OpenListTeam/OpenList/v4/internal/conf.BuiltAt=$built_at - github.com/OpenListTeam/OpenList/v4/internal/conf.GitAuthor=The OpenList Projects Contributors + github.com/OpenListTeam/OpenList/v4/internal/conf.GitAuthor=The OpenList Projects Contributors github.com/OpenListTeam/OpenList/v4/internal/conf.GitCommit=$git_commit github.com/OpenListTeam/OpenList/v4/internal/conf.Version=$tag github.com/OpenListTeam/OpenList/v4/internal/conf.WebVersion=rolling diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 47991393e..41a2b524e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -52,7 +52,7 @@ jobs: out-dir: build x-flags: | github.com/OpenListTeam/OpenList/v4/internal/conf.BuiltAt=$built_at - github.com/OpenListTeam/OpenList/v4/internal/conf.GitAuthor=The OpenList Projects Contributors + github.com/OpenListTeam/OpenList/v4/internal/conf.GitAuthor=The OpenList Projects Contributors github.com/OpenListTeam/OpenList/v4/internal/conf.GitCommit=$git_commit github.com/OpenListTeam/OpenList/v4/internal/conf.Version=$tag github.com/OpenListTeam/OpenList/v4/internal/conf.WebVersion=rolling diff --git a/build.sh b/build.sh index 7a7715ef2..a43918c0f 100644 --- a/build.sh +++ b/build.sh @@ -1,7 +1,7 @@ set -e appName="openlist" builtAt="$(date +'%F %T %z')" -gitAuthor="The OpenList Projects Contributors " +gitAuthor="The OpenList Projects Contributors " gitCommit=$(git log --pretty=format:"%h" -1) # Set frontend repository, default to OpenListTeam/OpenList-Frontend From d894a3983b329eab456561937b6f3383f466f216 Mon Sep 17 00:00:00 2001 From: Ziheng Mao Date: Mon, 21 Sep 2026 10:38:14 +0800 Subject: [PATCH 4/6] fix(115_open): refresh expired OSS credentials during upload (#3063) - Refresh OSS credentials and retry the rejected multipart operation - Preserve upload IDs, completed parts, callbacks, and outer part retries - Rewind parts before resending and pass the upload context to OSS requests Co-authored-by: Codex <267193182+codex@users.noreply.github.com> --- drivers/115_open/upload.go | 77 ++++++++++++++++++++++++++++++-------- 1 file changed, 61 insertions(+), 16 deletions(-) diff --git a/drivers/115_open/upload.go b/drivers/115_open/upload.go index d02640e2c..ac0aad38b 100644 --- a/drivers/115_open/upload.go +++ b/drivers/115_open/upload.go @@ -3,6 +3,7 @@ package _115_open import ( "context" "encoding/base64" + "errors" "io" "time" @@ -70,6 +71,19 @@ func (d *Open115) singleUpload(ctx context.Context, tempF model.File, tokenResp // } `json:"data"` // } +// retryExpiredToken retries only the rejected OSS operation, preserving the upload ID. +func retryExpiredToken(refresh func() error, operation func() error) error { + err := operation() + var serviceErr oss.ServiceError + if !errors.As(err, &serviceErr) || serviceErr.Code != "SecurityTokenExpired" { + return err + } + if err := refresh(); err != nil { + return err + } + return operation() +} + func (d *Open115) multpartUpload(ctx context.Context, stream model.FileStreamer, up driver.UpdateProgress, tokenResp *sdk.UploadGetTokenResp, initResp *sdk.UploadInitResp) error { ossClient, err := netutil.NewOSSClient(tokenResp.Endpoint, tokenResp.AccessKeyId, tokenResp.AccessKeySecret, oss.SecurityToken(tokenResp.SecurityToken)) if err != nil { @@ -80,7 +94,32 @@ func (d *Open115) multpartUpload(ctx context.Context, stream model.FileStreamer, return err } - imur, err := bucket.InitiateMultipartUpload(initResp.Object, oss.Sequential()) + refresh := func() error { + if err := d.WaitLimit(ctx); err != nil { + return err + } + token, err := d.client.UploadGetToken(ctx) + if err != nil { + return err + } + client, err := netutil.NewOSSClient(token.Endpoint, token.AccessKeyId, token.AccessKeySecret, oss.SecurityToken(token.SecurityToken)) + if err != nil { + return err + } + newBucket, err := client.Bucket(initResp.Bucket) + if err != nil { + return err + } + bucket = newBucket + return nil + } + + var imur oss.InitiateMultipartUploadResult + err = retryExpiredToken(refresh, func() error { + var err error + imur, err = bucket.InitiateMultipartUpload(initResp.Object, oss.Sequential(), oss.WithContext(ctx)) + return err + }) if err != nil { return err } @@ -109,13 +148,17 @@ func (d *Open115) multpartUpload(ctx context.Context, stream model.FileStreamer, return err } err = retry.Do(func() error { - rd.Seek(0, io.SeekStart) - part, err := bucket.UploadPart(imur, driver.NewLimitedUploadStream(ctx, rd), partSize, int(i)) - if err != nil { - return err - } - parts[i-1] = part - return nil + return retryExpiredToken(refresh, func() error { + if _, err := rd.Seek(0, io.SeekStart); err != nil { + return err + } + part, err := bucket.UploadPart(imur, driver.NewLimitedUploadStream(ctx, rd), partSize, int(i), oss.WithContext(ctx)) + if err != nil { + return err + } + parts[i-1] = part + return nil + }) }, retry.Context(ctx), retry.Attempts(3), @@ -134,14 +177,16 @@ func (d *Open115) multpartUpload(ctx context.Context, stream model.FileStreamer, up(float64(offset) * 100 / float64(fileSize)) } - // callbackRespBytes := make([]byte, 1024) - _, err = bucket.CompleteMultipartUpload( - imur, - parts, - oss.Callback(base64.StdEncoding.EncodeToString([]byte(initResp.Callback.Value.Callback))), - oss.CallbackVar(base64.StdEncoding.EncodeToString([]byte(initResp.Callback.Value.CallbackVar))), - // oss.CallbackResult(&callbackRespBytes), - ) + err = retryExpiredToken(refresh, func() error { + _, err := bucket.CompleteMultipartUpload( + imur, + parts, + oss.Callback(base64.StdEncoding.EncodeToString([]byte(initResp.Callback.Value.Callback))), + oss.CallbackVar(base64.StdEncoding.EncodeToString([]byte(initResp.Callback.Value.CallbackVar))), + oss.WithContext(ctx), + ) + return err + }) if err != nil { return err } From 56064d198165c58048c45944f59d53b710d964c8 Mon Sep 17 00:00:00 2001 From: Nostalgia Date: Mon, 21 Sep 2026 16:54:12 +0800 Subject: [PATCH 5/6] fix(op): enforce link cache lifecycle policy (#3101) - Share one admitted lifecycle policy between regular and archive links. - Reject and release links that combine TTL caching with owned resources. - Keep wrapper clones independent from the source cache expiration. - Cover reuse, reference, invalidation, conflict, and clone behavior. Co-authored-by: nostalume Co-authored-by: Codex <267193182+codex@users.noreply.github.com> --- drivers/alias/driver.go | 1 - internal/model/args.go | 4 +- internal/model/args_test.go | 25 +++++ internal/op/archive.go | 18 ++-- internal/op/fs.go | 14 +-- internal/op/link_lifecycle.go | 34 +++++++ internal/op/link_lifecycle_test.go | 143 +++++++++++++++++++++++++++++ 7 files changed, 223 insertions(+), 16 deletions(-) create mode 100644 internal/model/args_test.go create mode 100644 internal/op/link_lifecycle.go create mode 100644 internal/op/link_lifecycle_test.go diff --git a/drivers/alias/driver.go b/drivers/alias/driver.go index 7008495ce..d69d6cf50 100644 --- a/drivers/alias/driver.go +++ b/drivers/alias/driver.go @@ -328,7 +328,6 @@ func (d *Alias) Link(ctx context.Context, file model.Obj, args model.LinkArgs) ( return nil, err } resultLink := link.Clone() // 复制一份，避免修改到原始link - resultLink.Expiration = nil if args.Redirect { return resultLink, nil } diff --git a/internal/model/args.go b/internal/model/args.go index 393a716fa..b051106cc 100644 --- a/internal/model/args.go +++ b/internal/model/args.go @@ -30,7 +30,7 @@ type Link struct { Header http.Header `json:"header"` // needed header (for url) RangeReader RangeReaderIF `json:"-"` // recommended way if can't use URL - Expiration *time.Duration // local cache expire Duration + Expiration *time.Duration // local cache expiration; not transferred by Clone //for accelerating request, use multi-thread downloading Concurrency int `json:"concurrency"` @@ -42,12 +42,12 @@ type Link struct { RequireReference bool `json:"-"` } +// Clone transfers ownership of l without inheriting its cache expiration. func (l *Link) Clone() *Link { return &Link{ URL: l.URL, Header: l.Header, RangeReader: l.RangeReader, - Expiration: l.Expiration, Concurrency: l.Concurrency, PartSize: l.PartSize, ContentLength: l.ContentLength, diff --git a/internal/model/args_test.go b/internal/model/args_test.go new file mode 100644 index 000000000..a82ae44b6 --- /dev/null +++ b/internal/model/args_test.go @@ -0,0 +1,25 @@ +package model + +import ( + "testing" + "time" +) + +func TestLinkCloneTransfersOwnershipWithoutCachePolicy(t *testing.T) { + ttl := time.Minute + source := &Link{URL: "https://example.test/file", Expiration: &ttl} + + clone := source.Clone() + if clone.URL != source.URL { + t.Fatal("clone did not preserve transport data") + } + if clone.Expiration != nil { + t.Fatal("clone inherited source cache policy") + } + if err := clone.Close(); err != nil { + t.Fatal(err) + } + if !source.Expired() { + t.Fatal("closing clone did not release its source") + } +} diff --git a/internal/op/archive.go b/internal/op/archive.go index d0a53a919..bb3de11a1 100644 --- a/internal/op/archive.go +++ b/internal/op/archive.go @@ -390,8 +390,9 @@ func ArchiveGet(ctx context.Context, storage driver.Driver, path string, args mo } type objWithLink struct { - link *model.Link - obj model.Obj + link *model.Link + obj model.Obj + policy linkCachePolicy } var ( @@ -405,7 +406,7 @@ func DriverExtract(ctx context.Context, storage driver.Driver, path string, args } key := stdpath.Join(Key(storage, path), args.InnerPath) if ol, ok := extractCache.Get(key); ok { - if ol.link.Expiration != nil || ol.link.SyncClosers.AcquireReference() || !ol.link.RequireReference { + if ol.acquire() { return ol.link, ol.obj, nil } } @@ -415,8 +416,8 @@ func DriverExtract(ctx context.Context, storage driver.Driver, path string, args if err != nil { return nil, errors.Wrapf(err, "failed extract archive") } - if ol.link.Expiration != nil { - extractCache.SetWithTTL(key, ol, *ol.link.Expiration) + if ol.policy.expiration != nil { + extractCache.SetWithTTL(key, ol, *ol.policy.expiration) } else { extractCache.SetWithExpirable(key, ol, &ol.link.SyncClosers) } @@ -428,7 +429,7 @@ func DriverExtract(ctx context.Context, storage driver.Driver, path string, args if err != nil { return nil, nil, err } - if ol.link.SyncClosers.AcquireReference() || !ol.link.RequireReference { + if ol.acquire() { return ol.link, ol.obj, nil } } @@ -450,7 +451,10 @@ func driverExtract(ctx context.Context, storage driver.Driver, path string, args return nil, errors.WithStack(errs.NotFile) } link, err := storageAr.Extract(ctx, archiveFile, args) - return &objWithLink{link: link, obj: extracted}, err + if err != nil { + return nil, err + } + return admitLink(link, extracted) } type streamWithParent struct { diff --git a/internal/op/fs.go b/internal/op/fs.go index f82a3ca8f..033d89b96 100644 --- a/internal/op/fs.go +++ b/internal/op/fs.go @@ -242,8 +242,7 @@ func Link(ctx context.Context, storage driver.Driver, path string, args model.Li } key := Key(storage, path) if ol, exists := Cache.linkCache.GetType(key, typeKey); exists { - if ol.link.Expiration != nil || - ol.link.SyncClosers.AcquireReference() || !ol.link.RequireReference { + if ol.acquire() { return ol.link, ol.obj, nil } } @@ -261,9 +260,12 @@ func Link(ctx context.Context, storage driver.Driver, path string, args model.Li if err != nil { return nil, errors.Wrapf(err, "failed get link") } - ol := &objWithLink{link: link, obj: file} - if link.Expiration != nil { - Cache.linkCache.SetTypeWithTTL(key, typeKey, ol, *link.Expiration) + ol, err := admitLink(link, file) + if err != nil { + return nil, err + } + if ol.policy.expiration != nil { + Cache.linkCache.SetTypeWithTTL(key, typeKey, ol, *ol.policy.expiration) } else { Cache.linkCache.SetTypeWithExpirable(key, typeKey, ol, &link.SyncClosers) } @@ -274,7 +276,7 @@ func Link(ctx context.Context, storage driver.Driver, path string, args model.Li if err != nil { return nil, nil, err } - if ol.link.SyncClosers.AcquireReference() || !ol.link.RequireReference { + if ol.acquire() { return ol.link, ol.obj, nil } } diff --git a/internal/op/link_lifecycle.go b/internal/op/link_lifecycle.go new file mode 100644 index 000000000..71491c97b --- /dev/null +++ b/internal/op/link_lifecycle.go @@ -0,0 +1,34 @@ +package op + +import ( + "errors" + "time" + + "github.com/OpenListTeam/OpenList/v4/internal/model" +) + +var errConflictingLinkLifecycle = errors.New("invalid link lifecycle: expiration cannot be combined with owned closers or RequireReference") + +type linkCachePolicy struct { + expiration *time.Duration + requireReference bool +} + +func admitLink(link *model.Link, obj model.Obj) (*objWithLink, error) { + if link.Expiration != nil && (link.RequireReference || link.SyncClosers.Length() > 0) { + return nil, errors.Join(errConflictingLinkLifecycle, link.Close()) + } + return &objWithLink{ + link: link, + obj: obj, + policy: linkCachePolicy{ + expiration: link.Expiration, + requireReference: link.RequireReference, + }, + }, nil +} + +func (ol *objWithLink) acquire() bool { + return ol.policy.expiration != nil || + ol.link.SyncClosers.AcquireReference() || !ol.policy.requireReference +} diff --git a/internal/op/link_lifecycle_test.go b/internal/op/link_lifecycle_test.go new file mode 100644 index 000000000..53bb0baf7 --- /dev/null +++ b/internal/op/link_lifecycle_test.go @@ -0,0 +1,143 @@ +package op + +import ( + "context" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/OpenListTeam/OpenList/v4/internal/driver" + "github.com/OpenListTeam/OpenList/v4/internal/model" + "github.com/OpenListTeam/OpenList/v4/pkg/singleflight" + "github.com/OpenListTeam/OpenList/v4/pkg/utils" +) + +type linkLifecycleDriver struct { + model.Storage + links func() *model.Link + calls atomic.Int32 +} + +func (d *linkLifecycleDriver) Config() driver.Config { return driver.Config{} } +func (d *linkLifecycleDriver) GetAddition() driver.Additional { return nil } +func (d *linkLifecycleDriver) Init(context.Context) error { return nil } +func (d *linkLifecycleDriver) Drop(context.Context) error { return nil } +func (d *linkLifecycleDriver) List(context.Context, model.Obj, model.ListArgs) ([]model.Obj, error) { + return nil, nil +} +func (d *linkLifecycleDriver) Get(context.Context, string) (model.Obj, error) { + return &model.Object{Name: "file", Path: "/file"}, nil +} +func (d *linkLifecycleDriver) Link(context.Context, model.Obj, model.LinkArgs) (*model.Link, error) { + d.calls.Add(1) + return d.links(), nil +} + +func resetLinkLifecycleState(t *testing.T) { + t.Helper() + oldCache := Cache + Cache, linkG = NewCacheManager(), singleflight.Group[*objWithLink]{} + t.Cleanup(func() { Cache, linkG = oldCache, singleflight.Group[*objWithLink]{} }) +} + +func acquireTestLink(t *testing.T, d *linkLifecycleDriver) *model.Link { + t.Helper() + link, _, err := Link(context.Background(), d, "/file", model.LinkArgs{}) + if err != nil { + t.Fatal(err) + } + return link +} + +func TestLinkLifecycleModes(t *testing.T) { + t.Run("TTL descriptor remains reusable after close", func(t *testing.T) { + resetLinkLifecycleState(t) + ttl := time.Minute + d := &linkLifecycleDriver{ + Storage: model.Storage{MountPath: "/ttl"}, + links: func() *model.Link { return &model.Link{URL: "https://example.test/file", Expiration: &ttl} }, + } + + first := acquireTestLink(t, d) + _ = first.Close() + second := acquireTestLink(t, d) + if second.URL != first.URL || d.calls.Load() != 1 { + t.Fatalf("TTL link was not reused: calls=%d", d.calls.Load()) + } + _ = second.Close() + }) + + t.Run("references keep shared resources alive until final close", func(t *testing.T) { + resetLinkLifecycleState(t) + var closes atomic.Int32 + d := &linkLifecycleDriver{ + Storage: model.Storage{MountPath: "/reference"}, + links: func() *model.Link { + return &model.Link{ + URL: "https://example.test/file", + SyncClosers: utils.NewSyncClosers(utils.CloseFunc(func() error { closes.Add(1); return nil })), + RequireReference: true, + } + }, + } + + first := acquireTestLink(t, d) + second := acquireTestLink(t, d) + _ = first.Close() + if closes.Load() != 0 { + t.Fatal("shared resource closed while another reference was active") + } + _ = second.Close() + if closes.Load() != 1 { + t.Fatalf("final close count = %d, want 1", closes.Load()) + } + third := acquireTestLink(t, d) + _ = third.Close() + if d.calls.Load() != 2 || closes.Load() != 2 { + t.Fatalf("stale link was not replaced: calls=%d closes=%d", d.calls.Load(), closes.Load()) + } + }) + + t.Run("close-invalidated link is reacquired", func(t *testing.T) { + resetLinkLifecycleState(t) + d := &linkLifecycleDriver{ + Storage: model.Storage{MountPath: "/close-invalidated"}, + links: func() *model.Link { + return &model.Link{SyncClosers: utils.NewSyncClosers(utils.CloseFunc(func() error { return nil }))} + }, + } + + first := acquireTestLink(t, d) + _ = first.Close() + second := acquireTestLink(t, d) + _ = second.Close() + if d.calls.Load() != 2 { + t.Fatalf("driver calls = %d, want 2", d.calls.Load()) + } + }) + + t.Run("TTL with owned resources is rejected and released", func(t *testing.T) { + resetLinkLifecycleState(t) + ttl := time.Minute + var closes atomic.Int32 + d := &linkLifecycleDriver{ + Storage: model.Storage{MountPath: "/conflict"}, + links: func() *model.Link { + return &model.Link{ + Expiration: &ttl, + SyncClosers: utils.NewSyncClosers(utils.CloseFunc(func() error { closes.Add(1); return nil })), + RequireReference: true, + } + }, + } + + _, _, err := Link(context.Background(), d, "/file", model.LinkArgs{}) + if err == nil || !strings.Contains(err.Error(), "expiration cannot be combined") { + t.Fatalf("unexpected error: %v", err) + } + if closes.Load() != 1 { + t.Fatalf("rejected link close count = %d, want 1", closes.Load()) + } + }) +} From 3a31b438a94af2532608499b74251c630ddf0f6f Mon Sep 17 00:00:00 2001 From: Nostalgia Date: Mon, 21 Sep 2026 16:54:22 +0800 Subject: [PATCH 6/6] refactor(offline): centralize native tool setup (#3096) - Keep storage-to-tool identity in the offline tool package. - Share settings persistence, tool initialization, and storage admission across handlers. - Preserve endpoint payloads and unsupported-storage diagnostics with focused tests. Co-authored-by: nostalume Co-authored-by: Codex <267193182+codex@users.noreply.github.com> --- internal/offline_download/tool/add.go | 8 +- internal/offline_download/tool/add_test.go | 6 +- internal/offline_download/tool/tools.go | 2 +- server/handles/offline_download.go | 339 +++++---------------- server/handles/offline_download_test.go | 155 ++++++++++ 5 files changed, 233 insertions(+), 277 deletions(-) create mode 100644 server/handles/offline_download_test.go diff --git a/internal/offline_download/tool/add.go b/internal/offline_download/tool/add.go index 12160dcfe..4a963a51d 100644 --- a/internal/offline_download/tool/add.go +++ b/internal/offline_download/tool/add.go @@ -223,15 +223,17 @@ func isEd2kURL(urlStr string) bool { } func ed2kToolForStorage(storage driver.Driver) string { - switch toolNameForStorage(storage) { + name := NativeToolName(storage) + switch name { case "115 Cloud", "115 Open": - return toolNameForStorage(storage) + return name default: return "" } } -func toolNameForStorage(storage driver.Driver) string { +// NativeToolName returns the offline-download tool implemented by storage. +func NativeToolName(storage driver.Driver) string { switch storage.(type) { case *_115.Pan115: return "115 Cloud" diff --git a/internal/offline_download/tool/add_test.go b/internal/offline_download/tool/add_test.go index c2782130f..e538ea2f3 100644 --- a/internal/offline_download/tool/add_test.go +++ b/internal/offline_download/tool/add_test.go @@ -58,7 +58,7 @@ func TestEd2kToolForStorage(t *testing.T) { } } -func TestToolNameForStorage(t *testing.T) { +func TestNativeToolName(t *testing.T) { tests := []struct { name string storage driver.Driver @@ -78,8 +78,8 @@ func TestToolNameForStorage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := toolNameForStorage(tt.storage); got != tt.want { - t.Fatalf("toolNameForStorage(%T) = %q, want %q", tt.storage, got, tt.want) + if got := NativeToolName(tt.storage); got != tt.want { + t.Fatalf("NativeToolName(%T) = %q, want %q", tt.storage, got, tt.want) } }) } diff --git a/internal/offline_download/tool/tools.go b/internal/offline_download/tool/tools.go index fa7fd2608..57fccd069 100644 --- a/internal/offline_download/tool/tools.go +++ b/internal/offline_download/tool/tools.go @@ -45,7 +45,7 @@ func (t ToolsManager) NamesForPath(path string) []string { return names } - name := toolNameForStorage(storage) + name := NativeToolName(storage) if name == "" { return names } diff --git a/server/handles/offline_download.go b/server/handles/offline_download.go index 4d4167ba8..53f30662e 100644 --- a/server/handles/offline_download.go +++ b/server/handles/offline_download.go @@ -3,15 +3,6 @@ package handles import ( "strings" - _115 "github.com/OpenListTeam/OpenList/v4/drivers/115" - _115_open "github.com/OpenListTeam/OpenList/v4/drivers/115_open" - _123 "github.com/OpenListTeam/OpenList/v4/drivers/123" - _123_open "github.com/OpenListTeam/OpenList/v4/drivers/123_open" - "github.com/OpenListTeam/OpenList/v4/drivers/guangyapan" - "github.com/OpenListTeam/OpenList/v4/drivers/pikpak" - "github.com/OpenListTeam/OpenList/v4/drivers/thunder" - "github.com/OpenListTeam/OpenList/v4/drivers/thunder_browser" - "github.com/OpenListTeam/OpenList/v4/drivers/thunderx" "github.com/OpenListTeam/OpenList/v4/internal/conf" "github.com/OpenListTeam/OpenList/v4/internal/errs" "github.com/OpenListTeam/OpenList/v4/internal/model" @@ -23,6 +14,44 @@ import ( "github.com/pkg/errors" ) +func saveAndInitOfflineDownloadTool(c *gin.Context, name string, items []model.SettingItem) (string, bool) { + if err := op.SaveSettingItems(items); err != nil { + common.ErrorResp(c, err, 500) + return "", false + } + downloadTool, err := tool.Tools.Get(name) + if err != nil { + common.ErrorResp(c, err, 500) + return "", false + } + version, err := downloadTool.Init() + if err != nil { + common.ErrorResp(c, err, 500) + return "", false + } + return version, true +} + +func validateOfflineDownloadStorage(c *gin.Context, tempDir, nativeTool string) bool { + if tempDir == "" { + return true + } + storage, _, err := op.GetStorageAndActualPath(tempDir) + if err != nil { + common.ErrorStrResp(c, "storage does not exists", 400) + return false + } + if storage.Config().CheckStatus && storage.GetStorage().Status != op.WORK { + common.ErrorStrResp(c, "storage not init: "+storage.GetStorage().Status, 400) + return false + } + if tool.NativeToolName(storage) != nativeTool { + common.ErrorStrResp(c, "unsupported storage driver for offline download, only "+nativeTool+" is supported", 400) + return false + } + return true +} + type SetAria2Req struct { Uri string `json:"uri" form:"uri"` Secret string `json:"secret" form:"secret"` @@ -38,18 +67,8 @@ func SetAria2(c *gin.Context) { {Key: conf.Aria2Uri, Value: req.Uri, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, {Key: conf.Aria2Secret, Value: req.Secret, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("aria2") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - version, err := _tool.Init() - if err != nil { - common.ErrorResp(c, err, 500) + version, ok := saveAndInitOfflineDownloadTool(c, "aria2", items) + if !ok { return } common.SuccessResp(c, version) @@ -70,17 +89,7 @@ func SetQbittorrent(c *gin.Context) { {Key: conf.QbittorrentUrl, Value: req.Url, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, {Key: conf.QbittorrentSeedtime, Value: req.Seedtime, Type: conf.TypeNumber, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("qBittorrent") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "qBittorrent", items); !ok { return } common.SuccessResp(c, "ok") @@ -101,17 +110,7 @@ func SetTransmission(c *gin.Context) { {Key: conf.TransmissionUri, Value: req.Uri, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, {Key: conf.TransmissionSeedtime, Value: req.Seedtime, Type: conf.TypeNumber, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("Transmission") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "Transmission", items); !ok { return } common.SuccessResp(c, "ok") @@ -127,35 +126,13 @@ func Set115(c *gin.Context) { common.ErrorResp(c, err, 400) return } - if req.TempDir != "" { - storage, _, err := op.GetStorageAndActualPath(req.TempDir) - if err != nil { - common.ErrorStrResp(c, "storage does not exists", 400) - return - } - if storage.Config().CheckStatus && storage.GetStorage().Status != op.WORK { - common.ErrorStrResp(c, "storage not init: "+storage.GetStorage().Status, 400) - return - } - if _, ok := storage.(*_115.Pan115); !ok { - common.ErrorStrResp(c, "unsupported storage driver for offline download, only 115 Cloud is supported", 400) - return - } + if !validateOfflineDownloadStorage(c, req.TempDir, "115 Cloud") { + return } items := []model.SettingItem{ {Key: conf.Pan115TempDir, Value: req.TempDir, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("115 Cloud") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "115 Cloud", items); !ok { return } common.SuccessResp(c, "ok") @@ -171,35 +148,13 @@ func Set115Open(c *gin.Context) { common.ErrorResp(c, err, 400) return } - if req.TempDir != "" { - storage, _, err := op.GetStorageAndActualPath(req.TempDir) - if err != nil { - common.ErrorStrResp(c, "storage does not exists", 400) - return - } - if storage.Config().CheckStatus && storage.GetStorage().Status != op.WORK { - common.ErrorStrResp(c, "storage not init: "+storage.GetStorage().Status, 400) - return - } - if _, ok := storage.(*_115_open.Open115); !ok { - common.ErrorStrResp(c, "unsupported storage driver for offline download, only 115 Open is supported", 400) - return - } + if !validateOfflineDownloadStorage(c, req.TempDir, "115 Open") { + return } items := []model.SettingItem{ {Key: conf.Pan115OpenTempDir, Value: req.TempDir, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("115 Open") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "115 Open", items); !ok { return } common.SuccessResp(c, "ok") @@ -215,35 +170,13 @@ func Set123Pan(c *gin.Context) { common.ErrorResp(c, err, 400) return } - if req.TempDir != "" { - storage, _, err := op.GetStorageAndActualPath(req.TempDir) - if err != nil { - common.ErrorStrResp(c, "storage does not exists", 400) - return - } - if storage.Config().CheckStatus && storage.GetStorage().Status != op.WORK { - common.ErrorStrResp(c, "storage not init: "+storage.GetStorage().Status, 400) - return - } - if _, ok := storage.(*_123.Pan123); !ok { - common.ErrorStrResp(c, "unsupported storage driver for offline download, only 123Pan is supported", 400) - return - } + if !validateOfflineDownloadStorage(c, req.TempDir, "123Pan") { + return } items := []model.SettingItem{ {Key: conf.Pan123TempDir, Value: req.TempDir, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("123Pan") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "123Pan", items); !ok { return } common.SuccessResp(c, "ok") @@ -260,36 +193,14 @@ func Set123Open(c *gin.Context) { common.ErrorResp(c, err, 400) return } - if req.TempDir != "" { - storage, _, err := op.GetStorageAndActualPath(req.TempDir) - if err != nil { - common.ErrorStrResp(c, "storage does not exists", 400) - return - } - if storage.Config().CheckStatus && storage.GetStorage().Status != op.WORK { - common.ErrorStrResp(c, "storage not init: "+storage.GetStorage().Status, 400) - return - } - if _, ok := storage.(*_123_open.Open123); !ok { - common.ErrorStrResp(c, "unsupported storage driver for offline download, only 123 Open is supported", 400) - return - } + if !validateOfflineDownloadStorage(c, req.TempDir, "123 Open") { + return } items := []model.SettingItem{ {Key: conf.Pan123OpenTempDir, Value: req.TempDir, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, {Key: conf.Pan123OpenOfflineDownloadCallbackUrl, Value: req.CallbackUrl, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("123 Open") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "123 Open", items); !ok { return } common.SuccessResp(c, "ok") @@ -305,35 +216,13 @@ func SetPikPak(c *gin.Context) { common.ErrorResp(c, err, 400) return } - if req.TempDir != "" { - storage, _, err := op.GetStorageAndActualPath(req.TempDir) - if err != nil { - common.ErrorStrResp(c, "storage does not exists", 400) - return - } - if storage.Config().CheckStatus && storage.GetStorage().Status != op.WORK { - common.ErrorStrResp(c, "storage not init: "+storage.GetStorage().Status, 400) - return - } - if _, ok := storage.(*pikpak.PikPak); !ok { - common.ErrorStrResp(c, "unsupported storage driver for offline download, only PikPak is supported", 400) - return - } + if !validateOfflineDownloadStorage(c, req.TempDir, "PikPak") { + return } items := []model.SettingItem{ {Key: conf.PikPakTempDir, Value: req.TempDir, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("PikPak") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "PikPak", items); !ok { return } common.SuccessResp(c, "ok") @@ -349,35 +238,13 @@ func SetThunder(c *gin.Context) { common.ErrorResp(c, err, 400) return } - if req.TempDir != "" { - storage, _, err := op.GetStorageAndActualPath(req.TempDir) - if err != nil { - common.ErrorStrResp(c, "storage does not exists", 400) - return - } - if storage.Config().CheckStatus && storage.GetStorage().Status != op.WORK { - common.ErrorStrResp(c, "storage not init: "+storage.GetStorage().Status, 400) - return - } - if _, ok := storage.(*thunder.Thunder); !ok { - common.ErrorStrResp(c, "unsupported storage driver for offline download, only Thunder is supported", 400) - return - } + if !validateOfflineDownloadStorage(c, req.TempDir, "Thunder") { + return } items := []model.SettingItem{ {Key: conf.ThunderTempDir, Value: req.TempDir, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("Thunder") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "Thunder", items); !ok { return } common.SuccessResp(c, "ok") @@ -393,35 +260,13 @@ func SetThunderX(c *gin.Context) { common.ErrorResp(c, err, 400) return } - if req.TempDir != "" { - storage, _, err := op.GetStorageAndActualPath(req.TempDir) - if err != nil { - common.ErrorStrResp(c, "storage does not exists", 400) - return - } - if storage.Config().CheckStatus && storage.GetStorage().Status != op.WORK { - common.ErrorStrResp(c, "storage not init: "+storage.GetStorage().Status, 400) - return - } - if _, ok := storage.(*thunderx.ThunderX); !ok { - common.ErrorStrResp(c, "unsupported storage driver for offline download, only ThunderX is supported", 400) - return - } + if !validateOfflineDownloadStorage(c, req.TempDir, "ThunderX") { + return } items := []model.SettingItem{ {Key: conf.ThunderXTempDir, Value: req.TempDir, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("ThunderX") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "ThunderX", items); !ok { return } common.SuccessResp(c, "ok") @@ -437,37 +282,13 @@ func SetThunderBrowser(c *gin.Context) { common.ErrorResp(c, err, 400) return } - if req.TempDir != "" { - storage, _, err := op.GetStorageAndActualPath(req.TempDir) - if err != nil { - common.ErrorStrResp(c, "storage does not exists", 400) - return - } - if storage.Config().CheckStatus && storage.GetStorage().Status != op.WORK { - common.ErrorStrResp(c, "storage not init: "+storage.GetStorage().Status, 400) - return - } - switch storage.(type) { - case *thunder_browser.ThunderBrowser, *thunder_browser.ThunderBrowserExpert: - default: - common.ErrorStrResp(c, "unsupported storage driver for offline download, only ThunderBrowser is supported", 400) - return - } + if !validateOfflineDownloadStorage(c, req.TempDir, "ThunderBrowser") { + return } items := []model.SettingItem{ {Key: conf.ThunderBrowserTempDir, Value: req.TempDir, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("ThunderBrowser") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "ThunderBrowser", items); !ok { return } common.SuccessResp(c, "ok") @@ -483,35 +304,13 @@ func SetGuangYaPan(c *gin.Context) { common.ErrorResp(c, err, 400) return } - if req.TempDir != "" { - storage, _, err := op.GetStorageAndActualPath(req.TempDir) - if err != nil { - common.ErrorStrResp(c, "storage does not exists", 400) - return - } - if storage.Config().CheckStatus && storage.GetStorage().Status != op.WORK { - common.ErrorStrResp(c, "storage not init: "+storage.GetStorage().Status, 400) - return - } - if _, ok := storage.(*guangyapan.GuangYaPan); !ok { - common.ErrorStrResp(c, "unsupported storage driver for offline download, only GuangYaPan is supported", 400) - return - } + if !validateOfflineDownloadStorage(c, req.TempDir, "GuangYaPan") { + return } items := []model.SettingItem{ {Key: conf.GuangYaPanTempDir, Value: req.TempDir, Type: conf.TypeString, Group: model.OFFLINE_DOWNLOAD, Flag: model.PRIVATE}, } - if err := op.SaveSettingItems(items); err != nil { - common.ErrorResp(c, err, 500) - return - } - _tool, err := tool.Tools.Get("GuangYaPan") - if err != nil { - common.ErrorResp(c, err, 500) - return - } - if _, err := _tool.Init(); err != nil { - common.ErrorResp(c, err, 500) + if _, ok := saveAndInitOfflineDownloadTool(c, "GuangYaPan", items); !ok { return } common.SuccessResp(c, "ok") diff --git a/server/handles/offline_download_test.go b/server/handles/offline_download_test.go new file mode 100644 index 000000000..6abecbee4 --- /dev/null +++ b/server/handles/offline_download_test.go @@ -0,0 +1,155 @@ +package handles + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + _ "github.com/OpenListTeam/OpenList/v4/drivers/local" + "github.com/OpenListTeam/OpenList/v4/internal/conf" + "github.com/OpenListTeam/OpenList/v4/internal/db" + "github.com/OpenListTeam/OpenList/v4/internal/model" + "github.com/OpenListTeam/OpenList/v4/internal/offline_download/tool" + "github.com/OpenListTeam/OpenList/v4/internal/op" + "github.com/OpenListTeam/OpenList/v4/server/common" + "github.com/gin-gonic/gin" + "github.com/glebarez/sqlite" + "gorm.io/gorm" +) + +func init() { + dataDir, err := os.MkdirTemp("", "openlist-handles-*") + if err != nil { + panic(err) + } + conf.Conf = conf.DefaultConfig(dataDir) + database, err := gorm.Open(sqlite.Open("file:handles?mode=memory&cache=shared"), &gorm.Config{}) + if err != nil { + panic(err) + } + db.Init(database) +} + +type settingsTool struct { + name string + version string + initCalls int +} + +func (t *settingsTool) Name() string { return t.name } +func (*settingsTool) Items() []model.SettingItem { return nil } +func (t *settingsTool) Init() (string, error) { t.initCalls++; return t.version, nil } +func (*settingsTool) IsReady() bool { return true } +func (*settingsTool) AddURL(*tool.AddUrlArgs) (string, error) { return "", nil } +func (*settingsTool) Remove(*tool.DownloadTask) error { return nil } +func (*settingsTool) Status(*tool.DownloadTask) (*tool.Status, error) { + return &tool.Status{}, nil +} +func (*settingsTool) Run(*tool.DownloadTask) error { return nil } + +func TestOfflineDownloadSettingsPreserveSuccessPayloads(t *testing.T) { + tests := []struct { + name string + toolName string + version string + body string + handler gin.HandlerFunc + wantData string + settingKey string + }{ + { + name: "aria2 returns version", + toolName: "aria2", + version: "v-test", + body: `{"uri":"http://aria2","secret":"secret"}`, + handler: SetAria2, + wantData: "v-test", + settingKey: conf.Aria2Uri, + }, + { + name: "qBittorrent returns ok", + toolName: "qBittorrent", + version: "ignored", + body: `{"url":"http://qbit","seedtime":"1"}`, + handler: SetQbittorrent, + wantData: "ok", + settingKey: conf.QbittorrentUrl, + }, + } + gin.SetMode(gin.TestMode) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fake := &settingsTool{name: test.toolName, version: test.version} + previous, existed := tool.Tools[test.toolName] + tool.Tools[test.toolName] = fake + t.Cleanup(func() { + if existed { + tool.Tools[test.toolName] = previous + } else { + delete(tool.Tools, test.toolName) + } + _ = db.DeleteSettingItemByKey(test.settingKey) + op.SettingCacheUpdate() + }) + + response := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(response) + ctx.Request = httptest.NewRequest(http.MethodPost, "/", strings.NewReader(test.body)) + ctx.Request.Header.Set("Content-Type", "application/json") + test.handler(ctx) + + var result common.Resp[string] + if err := json.Unmarshal(response.Body.Bytes(), &result); err != nil { + t.Fatal(err) + } + if result.Code != 200 || result.Data != test.wantData { + t.Fatalf("response = %#v, want code 200 and data %q", result, test.wantData) + } + if fake.initCalls != 1 { + t.Fatalf("Init calls = %d, want 1", fake.initCalls) + } + }) + } +} + +func TestValidateOfflineDownloadStorageRejectsWrongNativeTool(t *testing.T) { + root := t.TempDir() + addition, err := json.Marshal(struct { + RootFolderPath string `json:"root_folder_path"` + }{RootFolderPath: root}) + if err != nil { + t.Fatal(err) + } + mount := "/" + strings.ReplaceAll(t.Name(), "/", "_") + storageID, err := op.CreateStorage(context.Background(), model.Storage{ + Driver: "Local", + MountPath: mount, + Addition: string(addition), + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := op.DeleteStorageById(context.Background(), storageID); err != nil { + t.Errorf("delete fixture storage: %v", err) + } + }) + + response := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(response) + if validateOfflineDownloadStorage(ctx, mount, "Thunder") { + t.Fatal("Local storage unexpectedly accepted as Thunder") + } + var result common.Resp[any] + if err := json.Unmarshal(response.Body.Bytes(), &result); err != nil { + t.Fatal(err) + } + want := "unsupported storage driver for offline download, only Thunder is supported" + if result.Code != 400 || result.Message != want { + t.Fatalf("response = %#v, want code 400 and message %q", result, want) + } +}