Skip to content

Commit 56064d1

Browse files
Nostalgianostalumecodex
authored
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 <nostalucent@gmail.com> Co-authored-by: Codex <267193182+codex@users.noreply.github.com>
1 parent d894a39 commit 56064d1

7 files changed

Lines changed: 223 additions & 16 deletions

File tree

‎drivers/alias/driver.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,6 @@ func (d *Alias) Link(ctx context.Context, file model.Obj, args model.LinkArgs) (
328328
return nil, err
329329
}
330330
resultLink := link.Clone() // 复制一份,避免修改到原始link
331-
resultLink.Expiration = nil
332331
if args.Redirect {
333332
return resultLink, nil
334333
}

‎internal/model/args.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type Link struct {
3030
Header http.Header `json:"header"` // needed header (for url)
3131
RangeReader RangeReaderIF `json:"-"` // recommended way if can't use URL
3232

33-
Expiration *time.Duration // local cache expire Duration
33+
Expiration *time.Duration // local cache expiration; not transferred by Clone
3434

3535
//for accelerating request, use multi-thread downloading
3636
Concurrency int `json:"concurrency"`
@@ -42,12 +42,12 @@ type Link struct {
4242
RequireReference bool `json:"-"`
4343
}
4444

45+
// Clone transfers ownership of l without inheriting its cache expiration.
4546
func (l *Link) Clone() *Link {
4647
return &Link{
4748
URL: l.URL,
4849
Header: l.Header,
4950
RangeReader: l.RangeReader,
50-
Expiration: l.Expiration,
5151
Concurrency: l.Concurrency,
5252
PartSize: l.PartSize,
5353
ContentLength: l.ContentLength,

‎internal/model/args_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package model
2+
3+
import (
4+
"testing"
5+
"time"
6+
)
7+
8+
func TestLinkCloneTransfersOwnershipWithoutCachePolicy(t *testing.T) {
9+
ttl := time.Minute
10+
source := &Link{URL: "https://example.test/file", Expiration: &ttl}
11+
12+
clone := source.Clone()
13+
if clone.URL != source.URL {
14+
t.Fatal("clone did not preserve transport data")
15+
}
16+
if clone.Expiration != nil {
17+
t.Fatal("clone inherited source cache policy")
18+
}
19+
if err := clone.Close(); err != nil {
20+
t.Fatal(err)
21+
}
22+
if !source.Expired() {
23+
t.Fatal("closing clone did not release its source")
24+
}
25+
}

‎internal/op/archive.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -390,8 +390,9 @@ func ArchiveGet(ctx context.Context, storage driver.Driver, path string, args mo
390390
}
391391

392392
type objWithLink struct {
393-
link *model.Link
394-
obj model.Obj
393+
link *model.Link
394+
obj model.Obj
395+
policy linkCachePolicy
395396
}
396397

397398
var (
@@ -405,7 +406,7 @@ func DriverExtract(ctx context.Context, storage driver.Driver, path string, args
405406
}
406407
key := stdpath.Join(Key(storage, path), args.InnerPath)
407408
if ol, ok := extractCache.Get(key); ok {
408-
if ol.link.Expiration != nil || ol.link.SyncClosers.AcquireReference() || !ol.link.RequireReference {
409+
if ol.acquire() {
409410
return ol.link, ol.obj, nil
410411
}
411412
}
@@ -415,8 +416,8 @@ func DriverExtract(ctx context.Context, storage driver.Driver, path string, args
415416
if err != nil {
416417
return nil, errors.Wrapf(err, "failed extract archive")
417418
}
418-
if ol.link.Expiration != nil {
419-
extractCache.SetWithTTL(key, ol, *ol.link.Expiration)
419+
if ol.policy.expiration != nil {
420+
extractCache.SetWithTTL(key, ol, *ol.policy.expiration)
420421
} else {
421422
extractCache.SetWithExpirable(key, ol, &ol.link.SyncClosers)
422423
}
@@ -428,7 +429,7 @@ func DriverExtract(ctx context.Context, storage driver.Driver, path string, args
428429
if err != nil {
429430
return nil, nil, err
430431
}
431-
if ol.link.SyncClosers.AcquireReference() || !ol.link.RequireReference {
432+
if ol.acquire() {
432433
return ol.link, ol.obj, nil
433434
}
434435
}
@@ -450,7 +451,10 @@ func driverExtract(ctx context.Context, storage driver.Driver, path string, args
450451
return nil, errors.WithStack(errs.NotFile)
451452
}
452453
link, err := storageAr.Extract(ctx, archiveFile, args)
453-
return &objWithLink{link: link, obj: extracted}, err
454+
if err != nil {
455+
return nil, err
456+
}
457+
return admitLink(link, extracted)
454458
}
455459

456460
type streamWithParent struct {

‎internal/op/fs.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,7 @@ func Link(ctx context.Context, storage driver.Driver, path string, args model.Li
242242
}
243243
key := Key(storage, path)
244244
if ol, exists := Cache.linkCache.GetType(key, typeKey); exists {
245-
if ol.link.Expiration != nil ||
246-
ol.link.SyncClosers.AcquireReference() || !ol.link.RequireReference {
245+
if ol.acquire() {
247246
return ol.link, ol.obj, nil
248247
}
249248
}
@@ -261,9 +260,12 @@ func Link(ctx context.Context, storage driver.Driver, path string, args model.Li
261260
if err != nil {
262261
return nil, errors.Wrapf(err, "failed get link")
263262
}
264-
ol := &objWithLink{link: link, obj: file}
265-
if link.Expiration != nil {
266-
Cache.linkCache.SetTypeWithTTL(key, typeKey, ol, *link.Expiration)
263+
ol, err := admitLink(link, file)
264+
if err != nil {
265+
return nil, err
266+
}
267+
if ol.policy.expiration != nil {
268+
Cache.linkCache.SetTypeWithTTL(key, typeKey, ol, *ol.policy.expiration)
267269
} else {
268270
Cache.linkCache.SetTypeWithExpirable(key, typeKey, ol, &link.SyncClosers)
269271
}
@@ -274,7 +276,7 @@ func Link(ctx context.Context, storage driver.Driver, path string, args model.Li
274276
if err != nil {
275277
return nil, nil, err
276278
}
277-
if ol.link.SyncClosers.AcquireReference() || !ol.link.RequireReference {
279+
if ol.acquire() {
278280
return ol.link, ol.obj, nil
279281
}
280282
}

‎internal/op/link_lifecycle.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package op
2+
3+
import (
4+
"errors"
5+
"time"
6+
7+
"github.com/OpenListTeam/OpenList/v4/internal/model"
8+
)
9+
10+
var errConflictingLinkLifecycle = errors.New("invalid link lifecycle: expiration cannot be combined with owned closers or RequireReference")
11+
12+
type linkCachePolicy struct {
13+
expiration *time.Duration
14+
requireReference bool
15+
}
16+
17+
func admitLink(link *model.Link, obj model.Obj) (*objWithLink, error) {
18+
if link.Expiration != nil && (link.RequireReference || link.SyncClosers.Length() > 0) {
19+
return nil, errors.Join(errConflictingLinkLifecycle, link.Close())
20+
}
21+
return &objWithLink{
22+
link: link,
23+
obj: obj,
24+
policy: linkCachePolicy{
25+
expiration: link.Expiration,
26+
requireReference: link.RequireReference,
27+
},
28+
}, nil
29+
}
30+
31+
func (ol *objWithLink) acquire() bool {
32+
return ol.policy.expiration != nil ||
33+
ol.link.SyncClosers.AcquireReference() || !ol.policy.requireReference
34+
}

‎internal/op/link_lifecycle_test.go‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
package op
2+
3+
import (
4+
"context"
5+
"strings"
6+
"sync/atomic"
7+
"testing"
8+
"time"
9+
10+
"github.com/OpenListTeam/OpenList/v4/internal/driver"
11+
"github.com/OpenListTeam/OpenList/v4/internal/model"
12+
"github.com/OpenListTeam/OpenList/v4/pkg/singleflight"
13+
"github.com/OpenListTeam/OpenList/v4/pkg/utils"
14+
)
15+
16+
type linkLifecycleDriver struct {
17+
model.Storage
18+
links func() *model.Link
19+
calls atomic.Int32
20+
}
21+
22+
func (d *linkLifecycleDriver) Config() driver.Config { return driver.Config{} }
23+
func (d *linkLifecycleDriver) GetAddition() driver.Additional { return nil }
24+
func (d *linkLifecycleDriver) Init(context.Context) error { return nil }
25+
func (d *linkLifecycleDriver) Drop(context.Context) error { return nil }
26+
func (d *linkLifecycleDriver) List(context.Context, model.Obj, model.ListArgs) ([]model.Obj, error) {
27+
return nil, nil
28+
}
29+
func (d *linkLifecycleDriver) Get(context.Context, string) (model.Obj, error) {
30+
return &model.Object{Name: "file", Path: "/file"}, nil
31+
}
32+
func (d *linkLifecycleDriver) Link(context.Context, model.Obj, model.LinkArgs) (*model.Link, error) {
33+
d.calls.Add(1)
34+
return d.links(), nil
35+
}
36+
37+
func resetLinkLifecycleState(t *testing.T) {
38+
t.Helper()
39+
oldCache := Cache
40+
Cache, linkG = NewCacheManager(), singleflight.Group[*objWithLink]{}
41+
t.Cleanup(func() { Cache, linkG = oldCache, singleflight.Group[*objWithLink]{} })
42+
}
43+
44+
func acquireTestLink(t *testing.T, d *linkLifecycleDriver) *model.Link {
45+
t.Helper()
46+
link, _, err := Link(context.Background(), d, "/file", model.LinkArgs{})
47+
if err != nil {
48+
t.Fatal(err)
49+
}
50+
return link
51+
}
52+
53+
func TestLinkLifecycleModes(t *testing.T) {
54+
t.Run("TTL descriptor remains reusable after close", func(t *testing.T) {
55+
resetLinkLifecycleState(t)
56+
ttl := time.Minute
57+
d := &linkLifecycleDriver{
58+
Storage: model.Storage{MountPath: "/ttl"},
59+
links: func() *model.Link { return &model.Link{URL: "https://example.test/file", Expiration: &ttl} },
60+
}
61+
62+
first := acquireTestLink(t, d)
63+
_ = first.Close()
64+
second := acquireTestLink(t, d)
65+
if second.URL != first.URL || d.calls.Load() != 1 {
66+
t.Fatalf("TTL link was not reused: calls=%d", d.calls.Load())
67+
}
68+
_ = second.Close()
69+
})
70+
71+
t.Run("references keep shared resources alive until final close", func(t *testing.T) {
72+
resetLinkLifecycleState(t)
73+
var closes atomic.Int32
74+
d := &linkLifecycleDriver{
75+
Storage: model.Storage{MountPath: "/reference"},
76+
links: func() *model.Link {
77+
return &model.Link{
78+
URL: "https://example.test/file",
79+
SyncClosers: utils.NewSyncClosers(utils.CloseFunc(func() error { closes.Add(1); return nil })),
80+
RequireReference: true,
81+
}
82+
},
83+
}
84+
85+
first := acquireTestLink(t, d)
86+
second := acquireTestLink(t, d)
87+
_ = first.Close()
88+
if closes.Load() != 0 {
89+
t.Fatal("shared resource closed while another reference was active")
90+
}
91+
_ = second.Close()
92+
if closes.Load() != 1 {
93+
t.Fatalf("final close count = %d, want 1", closes.Load())
94+
}
95+
third := acquireTestLink(t, d)
96+
_ = third.Close()
97+
if d.calls.Load() != 2 || closes.Load() != 2 {
98+
t.Fatalf("stale link was not replaced: calls=%d closes=%d", d.calls.Load(), closes.Load())
99+
}
100+
})
101+
102+
t.Run("close-invalidated link is reacquired", func(t *testing.T) {
103+
resetLinkLifecycleState(t)
104+
d := &linkLifecycleDriver{
105+
Storage: model.Storage{MountPath: "/close-invalidated"},
106+
links: func() *model.Link {
107+
return &model.Link{SyncClosers: utils.NewSyncClosers(utils.CloseFunc(func() error { return nil }))}
108+
},
109+
}
110+
111+
first := acquireTestLink(t, d)
112+
_ = first.Close()
113+
second := acquireTestLink(t, d)
114+
_ = second.Close()
115+
if d.calls.Load() != 2 {
116+
t.Fatalf("driver calls = %d, want 2", d.calls.Load())
117+
}
118+
})
119+
120+
t.Run("TTL with owned resources is rejected and released", func(t *testing.T) {
121+
resetLinkLifecycleState(t)
122+
ttl := time.Minute
123+
var closes atomic.Int32
124+
d := &linkLifecycleDriver{
125+
Storage: model.Storage{MountPath: "/conflict"},
126+
links: func() *model.Link {
127+
return &model.Link{
128+
Expiration: &ttl,
129+
SyncClosers: utils.NewSyncClosers(utils.CloseFunc(func() error { closes.Add(1); return nil })),
130+
RequireReference: true,
131+
}
132+
},
133+
}
134+
135+
_, _, err := Link(context.Background(), d, "/file", model.LinkArgs{})
136+
if err == nil || !strings.Contains(err.Error(), "expiration cannot be combined") {
137+
t.Fatalf("unexpected error: %v", err)
138+
}
139+
if closes.Load() != 1 {
140+
t.Fatalf("rejected link close count = %d, want 1", closes.Load())
141+
}
142+
})
143+
}

0 commit comments

Comments
 (0)