mirror of
https://github.com/stashapp/stash.git
synced 2026-03-07 13:41:58 +01:00
Fix stale browser-cached thumbnails after file content changes during scan. (#6622)
* Fix stale thumbnails after file content changes When a file's content changed (e.g. after renaming files in a gallery), the scan handler updated fingerprints but did not bump the entity's updated_at timestamp. Since thumbnail URLs use updated_at as a cache buster and are served with immutable/1-year cache headers, browsers would indefinitely serve the old cached thumbnail. Update image, scene, and gallery scan handlers to call UpdatePartial (which sets updated_at to now) whenever file content changes, not only when a new file association is created.
This commit is contained in:
parent
b8dff73696
commit
52bd9392fb
7 changed files with 363 additions and 11 deletions
|
|
@ -135,13 +135,14 @@ func (h *ScanHandler) associateExisting(ctx context.Context, existing []*models.
|
|||
if err := h.CreatorUpdater.AddFileID(ctx, i.ID, f.Base().ID); err != nil {
|
||||
return fmt.Errorf("adding file to gallery: %w", err)
|
||||
}
|
||||
// update updated_at time
|
||||
if _, err := h.CreatorUpdater.UpdatePartial(ctx, i.ID, models.NewGalleryPartial()); err != nil {
|
||||
return fmt.Errorf("updating gallery: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
if !found || updateExisting {
|
||||
// update updated_at time when file association or content changes
|
||||
if _, err := h.CreatorUpdater.UpdatePartial(ctx, i.ID, models.NewGalleryPartial()); err != nil {
|
||||
return fmt.Errorf("updating gallery: %w", err)
|
||||
}
|
||||
|
||||
h.PluginCache.RegisterPostHooks(ctx, i.ID, hook.GalleryUpdatePost, nil, nil)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
108
pkg/gallery/scan_test.go
Normal file
108
pkg/gallery/scan_test.go
Normal file
|
|
@ -0,0 +1,108 @@
|
|||
package gallery
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stashapp/stash/pkg/models"
|
||||
"github.com/stashapp/stash/pkg/models/mocks"
|
||||
"github.com/stashapp/stash/pkg/plugin"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/mock"
|
||||
)
|
||||
|
||||
func TestAssociateExisting_UpdatePartialOnContentChange(t *testing.T) {
|
||||
const (
|
||||
testGalleryID = 1
|
||||
testFileID = 100
|
||||
)
|
||||
|
||||
existingFile := &models.BaseFile{ID: models.FileID(testFileID), Path: "test.zip"}
|
||||
|
||||
makeGallery := func() *models.Gallery {
|
||||
return &models.Gallery{
|
||||
ID: testGalleryID,
|
||||
Files: models.NewRelatedFiles([]models.File{existingFile}),
|
||||
}
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
updateExisting bool
|
||||
expectUpdate bool
|
||||
}{
|
||||
{
|
||||
name: "calls UpdatePartial when file content changed",
|
||||
updateExisting: true,
|
||||
expectUpdate: true,
|
||||
},
|
||||
{
|
||||
name: "skips UpdatePartial when file unchanged and already associated",
|
||||
updateExisting: false,
|
||||
expectUpdate: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
db := mocks.NewDatabase()
|
||||
db.Gallery.On("GetFiles", mock.Anything, testGalleryID).Return([]models.File{existingFile}, nil)
|
||||
|
||||
if tt.expectUpdate {
|
||||
db.Gallery.On("UpdatePartial", mock.Anything, testGalleryID, mock.Anything).
|
||||
Return(&models.Gallery{ID: testGalleryID}, nil)
|
||||
}
|
||||
|
||||
h := &ScanHandler{
|
||||
CreatorUpdater: db.Gallery,
|
||||
PluginCache: &plugin.Cache{},
|
||||
}
|
||||
|
||||
db.WithTxnCtx(func(ctx context.Context) {
|
||||
err := h.associateExisting(ctx, []*models.Gallery{makeGallery()}, existingFile, tt.updateExisting)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
if tt.expectUpdate {
|
||||
db.Gallery.AssertCalled(t, "UpdatePartial", mock.Anything, testGalleryID, mock.Anything)
|
||||
} else {
|
||||
db.Gallery.AssertNotCalled(t, "UpdatePartial", mock.Anything, mock.Anything, mock.Anything)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExisting_UpdatePartialOnNewFile(t *testing.T) {
|
||||
const (
|
||||
testGalleryID = 1
|
||||
existFileID = 100
|
||||
newFileID = 200
|
||||
)
|
||||
|
||||
existingFile := &models.BaseFile{ID: models.FileID(existFileID), Path: "existing.zip"}
|
||||
newFile := &models.BaseFile{ID: models.FileID(newFileID), Path: "new.zip"}
|
||||
|
||||
gallery := &models.Gallery{
|
||||
ID: testGalleryID,
|
||||
Files: models.NewRelatedFiles([]models.File{existingFile}),
|
||||
}
|
||||
|
||||
db := mocks.NewDatabase()
|
||||
db.Gallery.On("GetFiles", mock.Anything, testGalleryID).Return([]models.File{existingFile}, nil)
|
||||
db.Gallery.On("AddFileID", mock.Anything, testGalleryID, models.FileID(newFileID)).Return(nil)
|
||||
db.Gallery.On("UpdatePartial", mock.Anything, testGalleryID, mock.Anything).
|
||||
Return(&models.Gallery{ID: testGalleryID}, nil)
|
||||
|
||||
h := &ScanHandler{
|
||||
CreatorUpdater: db.Gallery,
|
||||
PluginCache: &plugin.Cache{},
|
||||
}
|
||||
|
||||
db.WithTxnCtx(func(ctx context.Context) {
|
||||
err := h.associateExisting(ctx, []*models.Gallery{gallery}, newFile, false)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
db.Gallery.AssertCalled(t, "AddFileID", mock.Anything, testGalleryID, models.FileID(newFileID))
|
||||
db.Gallery.AssertCalled(t, "UpdatePartial", mock.Anything, testGalleryID, mock.Anything)
|
||||
}
|
||||
|
|
@ -210,8 +210,8 @@ func (h *ScanHandler) associateExisting(ctx context.Context, existing []*models.
|
|||
changed = true
|
||||
}
|
||||
|
||||
if changed {
|
||||
// always update updated_at time
|
||||
if changed || updateExisting {
|
||||
// update updated_at time when file association or content changes
|
||||
imagePartial := models.NewImagePartial()
|
||||
imagePartial.GalleryIDs = galleryIDs
|
||||
|
||||
|
|
@ -229,9 +229,7 @@ func (h *ScanHandler) associateExisting(ctx context.Context, existing []*models.
|
|||
return fmt.Errorf("updating gallery updated at timestamp: %w", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if changed || updateExisting {
|
||||
h.PluginCache.RegisterPostHooks(ctx, i.ID, hook.ImageUpdatePost, nil, nil)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
120
pkg/image/scan_test.go
Normal file
120
pkg/image/scan_test.go
Normal file
|
|
@ -0,0 +1,120 @@
|
|||
package image
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stashapp/stash/pkg/models"
|
||||
"github.com/stashapp/stash/pkg/models/mocks"
|
||||
"github.com/stashapp/stash/pkg/plugin"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/mock"
|
||||
)
|
||||
|
||||
type mockScanConfig struct{}
|
||||
|
||||
func (m *mockScanConfig) GetCreateGalleriesFromFolders() bool { return false }
|
||||
|
||||
func TestAssociateExisting_UpdatePartialOnContentChange(t *testing.T) {
|
||||
const (
|
||||
testImageID = 1
|
||||
testFileID = 100
|
||||
)
|
||||
|
||||
existingFile := &models.BaseFile{ID: models.FileID(testFileID), Path: "/images/test.jpg"}
|
||||
|
||||
makeImage := func() *models.Image {
|
||||
return &models.Image{
|
||||
ID: testImageID,
|
||||
Files: models.NewRelatedFiles([]models.File{existingFile}),
|
||||
GalleryIDs: models.NewRelatedIDs([]int{}),
|
||||
}
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
updateExisting bool
|
||||
expectUpdate bool
|
||||
}{
|
||||
{
|
||||
name: "calls UpdatePartial when file content changed",
|
||||
updateExisting: true,
|
||||
expectUpdate: true,
|
||||
},
|
||||
{
|
||||
name: "skips UpdatePartial when file unchanged and already associated",
|
||||
updateExisting: false,
|
||||
expectUpdate: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
db := mocks.NewDatabase()
|
||||
db.Image.On("GetFiles", mock.Anything, testImageID).Return([]models.File{existingFile}, nil)
|
||||
db.Image.On("GetGalleryIDs", mock.Anything, testImageID).Return([]int{}, nil)
|
||||
|
||||
if tt.expectUpdate {
|
||||
db.Image.On("UpdatePartial", mock.Anything, testImageID, mock.Anything).
|
||||
Return(&models.Image{ID: testImageID}, nil)
|
||||
}
|
||||
|
||||
h := &ScanHandler{
|
||||
CreatorUpdater: db.Image,
|
||||
GalleryFinder: db.Gallery,
|
||||
ScanConfig: &mockScanConfig{},
|
||||
PluginCache: &plugin.Cache{},
|
||||
}
|
||||
|
||||
db.WithTxnCtx(func(ctx context.Context) {
|
||||
err := h.associateExisting(ctx, []*models.Image{makeImage()}, existingFile, tt.updateExisting)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
if tt.expectUpdate {
|
||||
db.Image.AssertCalled(t, "UpdatePartial", mock.Anything, testImageID, mock.Anything)
|
||||
} else {
|
||||
db.Image.AssertNotCalled(t, "UpdatePartial", mock.Anything, mock.Anything, mock.Anything)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExisting_UpdatePartialOnNewFile(t *testing.T) {
|
||||
const (
|
||||
testImageID = 1
|
||||
existFileID = 100
|
||||
newFileID = 200
|
||||
)
|
||||
|
||||
existingFile := &models.BaseFile{ID: models.FileID(existFileID), Path: "/images/existing.jpg"}
|
||||
newFile := &models.BaseFile{ID: models.FileID(newFileID), Path: "/images/new.jpg"}
|
||||
|
||||
image := &models.Image{
|
||||
ID: testImageID,
|
||||
Files: models.NewRelatedFiles([]models.File{existingFile}),
|
||||
GalleryIDs: models.NewRelatedIDs([]int{}),
|
||||
}
|
||||
|
||||
db := mocks.NewDatabase()
|
||||
db.Image.On("GetFiles", mock.Anything, testImageID).Return([]models.File{existingFile}, nil)
|
||||
db.Image.On("GetGalleryIDs", mock.Anything, testImageID).Return([]int{}, nil)
|
||||
db.Image.On("AddFileID", mock.Anything, testImageID, models.FileID(newFileID)).Return(nil)
|
||||
db.Image.On("UpdatePartial", mock.Anything, testImageID, mock.Anything).
|
||||
Return(&models.Image{ID: testImageID}, nil)
|
||||
|
||||
h := &ScanHandler{
|
||||
CreatorUpdater: db.Image,
|
||||
GalleryFinder: db.Gallery,
|
||||
ScanConfig: &mockScanConfig{},
|
||||
PluginCache: &plugin.Cache{},
|
||||
}
|
||||
|
||||
db.WithTxnCtx(func(ctx context.Context) {
|
||||
err := h.associateExisting(ctx, []*models.Image{image}, newFile, false)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
db.Image.AssertCalled(t, "AddFileID", mock.Anything, testImageID, models.FileID(newFileID))
|
||||
db.Image.AssertCalled(t, "UpdatePartial", mock.Anything, testImageID, mock.Anything)
|
||||
}
|
||||
|
|
@ -3,6 +3,7 @@ package mocks
|
|||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
|
||||
"github.com/stashapp/stash/pkg/models"
|
||||
"github.com/stashapp/stash/pkg/txn"
|
||||
|
|
@ -89,6 +90,16 @@ func (db *Database) AssertExpectations(t mock.TestingT) {
|
|||
db.SavedFilter.AssertExpectations(t)
|
||||
}
|
||||
|
||||
// WithTxnCtx runs fn with a context that has a transaction hook manager registered,
|
||||
// so code that calls txn.AddPostCommitHook (e.g. plugin cache) won't nil-panic.
|
||||
// Always rolls back to avoid executing the registered hooks.
|
||||
func (db *Database) WithTxnCtx(fn func(ctx context.Context)) {
|
||||
_ = txn.WithTxn(context.Background(), db, func(ctx context.Context) error {
|
||||
fn(ctx)
|
||||
return errors.New("rollback")
|
||||
})
|
||||
}
|
||||
|
||||
func (db *Database) Repository() models.Repository {
|
||||
return models.Repository{
|
||||
TxnManager: db,
|
||||
|
|
|
|||
|
|
@ -160,15 +160,15 @@ func (h *ScanHandler) associateExisting(ctx context.Context, existing []*models.
|
|||
if err := h.CreatorUpdater.AddFileID(ctx, s.ID, f.ID); err != nil {
|
||||
return fmt.Errorf("adding file to scene: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
// update updated_at time
|
||||
if !found || updateExisting {
|
||||
// update updated_at time when file association or content changes
|
||||
scenePartial := models.NewScenePartial()
|
||||
if _, err := h.CreatorUpdater.UpdatePartial(ctx, s.ID, scenePartial); err != nil {
|
||||
return fmt.Errorf("updating scene: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
if !found || updateExisting {
|
||||
h.PluginCache.RegisterPostHooks(ctx, s.ID, hook.SceneUpdatePost, nil, nil)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
114
pkg/scene/scan_test.go
Normal file
114
pkg/scene/scan_test.go
Normal file
|
|
@ -0,0 +1,114 @@
|
|||
package scene
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stashapp/stash/pkg/models"
|
||||
"github.com/stashapp/stash/pkg/models/mocks"
|
||||
"github.com/stashapp/stash/pkg/plugin"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/mock"
|
||||
)
|
||||
|
||||
func TestAssociateExisting_UpdatePartialOnContentChange(t *testing.T) {
|
||||
const (
|
||||
testSceneID = 1
|
||||
testFileID = 100
|
||||
)
|
||||
|
||||
existingFile := &models.VideoFile{
|
||||
BaseFile: &models.BaseFile{ID: models.FileID(testFileID), Path: "test.mp4"},
|
||||
}
|
||||
|
||||
makeScene := func() *models.Scene {
|
||||
return &models.Scene{
|
||||
ID: testSceneID,
|
||||
Files: models.NewRelatedVideoFiles([]*models.VideoFile{existingFile}),
|
||||
}
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
updateExisting bool
|
||||
expectUpdate bool
|
||||
}{
|
||||
{
|
||||
name: "calls UpdatePartial when file content changed",
|
||||
updateExisting: true,
|
||||
expectUpdate: true,
|
||||
},
|
||||
{
|
||||
name: "skips UpdatePartial when file unchanged and already associated",
|
||||
updateExisting: false,
|
||||
expectUpdate: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
db := mocks.NewDatabase()
|
||||
db.Scene.On("GetFiles", mock.Anything, testSceneID).Return([]*models.VideoFile{existingFile}, nil)
|
||||
|
||||
if tt.expectUpdate {
|
||||
db.Scene.On("UpdatePartial", mock.Anything, testSceneID, mock.Anything).
|
||||
Return(&models.Scene{ID: testSceneID}, nil)
|
||||
}
|
||||
|
||||
h := &ScanHandler{
|
||||
CreatorUpdater: db.Scene,
|
||||
PluginCache: &plugin.Cache{},
|
||||
}
|
||||
|
||||
db.WithTxnCtx(func(ctx context.Context) {
|
||||
err := h.associateExisting(ctx, []*models.Scene{makeScene()}, existingFile, tt.updateExisting)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
if tt.expectUpdate {
|
||||
db.Scene.AssertCalled(t, "UpdatePartial", mock.Anything, testSceneID, mock.Anything)
|
||||
} else {
|
||||
db.Scene.AssertNotCalled(t, "UpdatePartial", mock.Anything, mock.Anything, mock.Anything)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExisting_UpdatePartialOnNewFile(t *testing.T) {
|
||||
const (
|
||||
testSceneID = 1
|
||||
existFileID = 100
|
||||
newFileID = 200
|
||||
)
|
||||
|
||||
existingFile := &models.VideoFile{
|
||||
BaseFile: &models.BaseFile{ID: models.FileID(existFileID), Path: "existing.mp4"},
|
||||
}
|
||||
newFile := &models.VideoFile{
|
||||
BaseFile: &models.BaseFile{ID: models.FileID(newFileID), Path: "new.mp4"},
|
||||
}
|
||||
|
||||
scene := &models.Scene{
|
||||
ID: testSceneID,
|
||||
Files: models.NewRelatedVideoFiles([]*models.VideoFile{existingFile}),
|
||||
}
|
||||
|
||||
db := mocks.NewDatabase()
|
||||
db.Scene.On("GetFiles", mock.Anything, testSceneID).Return([]*models.VideoFile{existingFile}, nil)
|
||||
db.Scene.On("AddFileID", mock.Anything, testSceneID, models.FileID(newFileID)).Return(nil)
|
||||
db.Scene.On("UpdatePartial", mock.Anything, testSceneID, mock.Anything).
|
||||
Return(&models.Scene{ID: testSceneID}, nil)
|
||||
|
||||
h := &ScanHandler{
|
||||
CreatorUpdater: db.Scene,
|
||||
PluginCache: &plugin.Cache{},
|
||||
}
|
||||
|
||||
db.WithTxnCtx(func(ctx context.Context) {
|
||||
err := h.associateExisting(ctx, []*models.Scene{scene}, newFile, false)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
|
||||
db.Scene.AssertCalled(t, "AddFileID", mock.Anything, testSceneID, models.FileID(newFileID))
|
||||
db.Scene.AssertCalled(t, "UpdatePartial", mock.Anything, testSceneID, mock.Anything)
|
||||
}
|
||||
Loading…
Reference in a new issue