From 6f884cb6f5ecfdb74243d8700baa54edc0022e74 Mon Sep 17 00:00:00 2001 From: Nikita Shoshin Date: Mon, 8 Apr 2024 00:55:33 +0400 Subject: [PATCH] init thumbnail service with the `rview.OpenFileFn`, don't pass it for every task --- cmd/rview.go | 21 +++-- rview/rview.go | 80 +++++++++---------- tests/thumbnails_test.go | 15 ++-- thumbnails/noop.go | 2 +- thumbnails/service.go | 15 ++-- thumbnails/service_test.go | 158 ++++++++++++++++++++++--------------- web/web.go | 6 +- web/web_test.go | 4 +- 8 files changed, 164 insertions(+), 137 deletions(-) diff --git a/cmd/rview.go b/cmd/rview.go index a34656a..1f7e00f 100644 --- a/cmd/rview.go +++ b/cmd/rview.go @@ -3,6 +3,7 @@ package cmd import ( "context" "fmt" + "io" "path/filepath" "reflect" "strings" @@ -44,6 +45,12 @@ func (r *Rview) Prepare() (err error) { return fmt.Errorf("couldn't prepare disk cache for service needs: %w", err) } + // Rclone Instance + r.rcloneInstance, err = rclone.NewRclone(r.cfg.RclonePort, r.cfg.RcloneTarget, r.cfg.RcloneDirCacheTime) + if err != nil { + return fmt.Errorf("couldn't prepare rclone: %w", err) + } + // Thumbnail Service if r.cfg.Thumbnails { err := thumbnails.CheckVips() @@ -61,7 +68,13 @@ func (r *Rview) Prepare() (err error) { maxTotalFileSize := int64(r.cfg.ThumbnailsMaxTotalSizeInMB * 1 << 20) r.thumbnailCleaner = cache.NewCleaner(thumbnailsCacheDir, maxFileAge, maxTotalFileSize) - r.thumbnailService = thumbnails.NewThumbnailService(thumbnailsCache, r.cfg.ThumbnailsWorkersCount, false) + openFileFn := func(ctx context.Context, id rview.FileID) (io.ReadCloser, error) { + rc, _, err := r.rcloneInstance.GetFile(ctx, id) + return rc, err + } + r.thumbnailService = thumbnails.NewThumbnailService( + openFileFn, thumbnailsCache, r.cfg.ThumbnailsWorkersCount, false, + ) } else { rlog.Info("thumbnail service is disabled") @@ -70,12 +83,6 @@ func (r *Rview) Prepare() (err error) { r.thumbnailCleaner = cache.NewNoopCleaner() } - // Rclone Instance - r.rcloneInstance, err = rclone.NewRclone(r.cfg.RclonePort, r.cfg.RcloneTarget, r.cfg.RcloneDirCacheTime) - if err != nil { - return fmt.Errorf("couldn't prepare rclone: %w", err) - } - // Search Service r.searchService = search.NewService(r.rcloneInstance, serviceCache) diff --git a/rview/rview.go b/rview/rview.go index 14412ba..8fc5f8d 100644 --- a/rview/rview.go +++ b/rview/rview.go @@ -68,31 +68,29 @@ func IsRcloneNotFoundError(err error) bool { return errors.As(err, &rcloneErr) && rcloneErr.StatusCode == http.StatusNotFound } -type ( - Rclone interface { - GetFile(ctx context.Context, id FileID) (io.ReadCloser, http.Header, error) - GetDirInfo(ctx context.Context, path string, sort, order string) (*RcloneDirInfo, error) - } +type Rclone interface { + GetFile(ctx context.Context, id FileID) (io.ReadCloser, http.Header, error) + GetDirInfo(ctx context.Context, path string, sort, order string) (*RcloneDirInfo, error) +} - RcloneDirInfo struct { - Sort string `json:"sort"` - Order string `json:"order"` +type RcloneDirInfo struct { + Sort string `json:"sort"` + Order string `json:"order"` - Breadcrumbs []RcloneDirBreadcrumb `json:"breadcrumbs"` - Entries []RcloneDirEntry `json:"entries"` - } + Breadcrumbs []RcloneDirBreadcrumb `json:"breadcrumbs"` + Entries []RcloneDirEntry `json:"entries"` +} - RcloneDirBreadcrumb struct { - Text string `json:"text"` - } +type RcloneDirBreadcrumb struct { + Text string `json:"text"` +} - RcloneDirEntry struct { - URL string `json:"url"` - IsDir bool `json:"is_dir"` - Size int64 `json:"size"` - ModTime int64 `json:"mod_time"` - } -) +type RcloneDirEntry struct { + URL string `json:"url"` + IsDir bool `json:"is_dir"` + Size int64 `json:"size"` + ModTime int64 `json:"mod_time"` +} var ( ErrCacheMiss = errors.New("cache miss") @@ -114,28 +112,24 @@ type ThumbnailID struct { FileID } -type ( - ThumbnailService interface { - NewThumbnailID(FileID) ThumbnailID - CanGenerateThumbnail(FileID) bool - IsThumbnailReady(ThumbnailID) bool - OpenThumbnail(context.Context, ThumbnailID) (io.ReadCloser, error) - SendTask(id FileID, openFileFn OpenFileFn) error - Shutdown(context.Context) error - } +type ThumbnailService interface { + NewThumbnailID(FileID) ThumbnailID + CanGenerateThumbnail(FileID) bool + IsThumbnailReady(ThumbnailID) bool + OpenThumbnail(context.Context, ThumbnailID) (io.ReadCloser, error) + SendTask(id FileID) error + Shutdown(context.Context) error +} - OpenFileFn func(ctx context.Context, id FileID) (io.ReadCloser, error) -) +type OpenFileFn func(ctx context.Context, id FileID) (io.ReadCloser, error) -type ( - SearchService interface { - GetMinSearchLength() int - Search(ctx context.Context, search string, dirLimit, fileLimit int) (dirs, files []SearchHit, err error) - RefreshIndexes(ctx context.Context) error - } +type SearchService interface { + GetMinSearchLength() int + Search(ctx context.Context, search string, dirLimit, fileLimit int) (dirs, files []SearchHit, err error) + RefreshIndexes(ctx context.Context) error +} - SearchHit struct { - Path string - Score float64 - } -) +type SearchHit struct { + Path string + Score float64 +} diff --git a/tests/thumbnails_test.go b/tests/thumbnails_test.go index 7230db5..16db593 100644 --- a/tests/thumbnails_test.go +++ b/tests/thumbnails_test.go @@ -22,8 +22,6 @@ func TestThumbnailGeneration(t *testing.T) { cache, err := cache.NewDiskCache(t.TempDir()) require.NoError(t, err) - thumbnailService := thumbnails.NewThumbnailService(cache, 1, true) - for _, tt := range []struct { imageType string file string @@ -38,15 +36,18 @@ func TestThumbnailGeneration(t *testing.T) { t.Run(tt.imageType, func(t *testing.T) { r := require.New(t) - fileID := rview.NewFileID(tt.file, 0) - thumbnailID := thumbnailService.NewThumbnailID(fileID) - originalImage, err := os.ReadFile(filepath.Join("testdata", tt.file)) r.NoError(err) - err = thumbnailService.SendTask(fileID, func(context.Context, rview.FileID) (io.ReadCloser, error) { + openFileFn := func(context.Context, rview.FileID) (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(originalImage)), nil - }) + } + thumbnailService := thumbnails.NewThumbnailService(openFileFn, cache, 1, true) + + fileID := rview.NewFileID(tt.file, 0) + thumbnailID := thumbnailService.NewThumbnailID(fileID) + + err = thumbnailService.SendTask(fileID) r.NoError(err) ctx, cancel := context.WithTimeout(context.Background(), time.Second) diff --git a/thumbnails/noop.go b/thumbnails/noop.go index d0d3282..efa7975 100644 --- a/thumbnails/noop.go +++ b/thumbnails/noop.go @@ -28,7 +28,7 @@ func (NoopThumbnailService) IsThumbnailReady(rview.ThumbnailID) bool { return false } -func (NoopThumbnailService) SendTask(rview.FileID, rview.OpenFileFn) error { +func (NoopThumbnailService) SendTask(rview.FileID) error { return ErrNoopThumbnailService } diff --git a/thumbnails/service.go b/thumbnails/service.go index ade5eae..ce4ea9b 100644 --- a/thumbnails/service.go +++ b/thumbnails/service.go @@ -35,8 +35,9 @@ var ( ) type ThumbnailService struct { - cache rview.Cache - resizeFn func(originalFile, cacheFile string, id rview.ThumbnailID) error + cache rview.Cache + openFileFn rview.OpenFileFn + resizeFn func(originalFile, cacheFile string, id rview.ThumbnailID) error // useOriginalImageThresholdSize defines the maximum size of an original image that should be // used without resizing. The main purpose of resizing is to reduce image size, and with small // files it is not always possible - after resizing they become just larger. @@ -55,8 +56,6 @@ type ThumbnailService struct { type generateThumbnailTask struct { fileID rview.FileID thumbnailID rview.ThumbnailID - - openFileFn rview.OpenFileFn } func CheckVips() error { @@ -75,8 +74,9 @@ func CheckVips() error { // // For some images we can generate thumbnails of different formats. For example, // for .heic images we generate .jpeg thumbnails. -func NewThumbnailService(cache rview.Cache, workersCount int, generateThumbnailsForSmallFiles bool) *ThumbnailService { +func NewThumbnailService(openFileFn rview.OpenFileFn, cache rview.Cache, workersCount int, generateThumbnailsForSmallFiles bool) *ThumbnailService { r := &ThumbnailService{ + openFileFn: openFileFn, cache: cache, resizeFn: resizeWithVips, useOriginalImageThresholdSize: 200 << 10, // 200 KiB @@ -167,7 +167,7 @@ type stats struct { } func (s *ThumbnailService) processTask(ctx context.Context, task generateThumbnailTask) (finalStats stats, err error) { - rc, err := task.openFileFn(ctx, task.fileID) + rc, err := s.openFileFn(ctx, task.fileID) if err != nil { return stats{}, fmt.Errorf("couldn't get image reader: %w", err) } @@ -400,7 +400,7 @@ func (s *ThumbnailService) OpenThumbnail(ctx context.Context, id rview.Thumbnail // must be absolute. // // SendTask ignores duplicate tasks. However, it doesn't check files on disk. -func (s *ThumbnailService) SendTask(id rview.FileID, openFileFn rview.OpenFileFn) error { +func (s *ThumbnailService) SendTask(id rview.FileID) error { if s.stopped.Load() { return errors.New("can't send tasks after Shutdown call") } @@ -428,7 +428,6 @@ func (s *ThumbnailService) SendTask(id rview.FileID, openFileFn rview.OpenFileFn s.tasksCh <- generateThumbnailTask{ fileID: id, thumbnailID: thumbnailID, - openFileFn: openFileFn, } return nil } diff --git a/thumbnails/service_test.go b/thumbnails/service_test.go index e29e751..61771c8 100644 --- a/thumbnails/service_test.go +++ b/thumbnails/service_test.go @@ -23,77 +23,105 @@ func TestThumbnailService(t *testing.T) { cache, err := cache.NewDiskCache(t.TempDir()) r.NoError(err) - service := NewThumbnailService(cache, 2, false) - service.useOriginalImageThresholdSize = 10 + newService := func( + t *testing.T, + openFileFn rview.OpenFileFn, + resizeFn func(originalFile, cacheFile string, id rview.ThumbnailID) error, + ) *ThumbnailService { + + service := NewThumbnailService(nil, cache, 2, false) + service.useOriginalImageThresholdSize = 10 + service.openFileFn = openFileFn + service.resizeFn = resizeFn + + t.Cleanup(func() { + err = service.Shutdown(context.Background()) + require.NoError(t, err) + }) - var resizedCount int - service.resizeFn = func(_, cacheFile string, id rview.ThumbnailID) error { - resizedCount++ - return os.WriteFile(cacheFile, []byte("resized-content-"+id.GetName()), 0o600) + return service } - fileID := rview.NewFileID("1.jpg", time.Now().Unix()) - thumbnailID := service.NewThumbnailID(fileID) + t.Run("resize", func(t *testing.T) { + var openFileFnCount, resizedCount int + service := newService( + t, + func(_ context.Context, id rview.FileID) (io.ReadCloser, error) { + openFileFnCount++ + time.Sleep(110 * time.Millisecond) + return io.NopCloser(bytes.NewReader([]byte("original-content-" + id.String()))), nil + }, + func(_, cacheFile string, id rview.ThumbnailID) error { + resizedCount++ + return os.WriteFile(cacheFile, []byte("resized-content-"+id.GetName()), 0o600) + }, + ) + + fileID := rview.NewFileID("1.jpg", time.Now().Unix()) + thumbnailID := service.NewThumbnailID(fileID) - r.False(service.IsThumbnailReady(thumbnailID)) + r.False(service.IsThumbnailReady(thumbnailID)) - resizeStart := time.Now() + resizeStart := time.Now() - err = service.SendTask(fileID, func(_ context.Context, id rview.FileID) (io.ReadCloser, error) { - time.Sleep(110 * time.Millisecond) - return io.NopCloser(bytes.NewReader([]byte("original-content-" + id.String()))), nil - }) - r.NoError(err) + err = service.SendTask(fileID) + r.NoError(err) - // Must take into account in-progress tasks. - r.True(service.IsThumbnailReady(thumbnailID)) + // Must take into account in-progress tasks. + r.True(service.IsThumbnailReady(thumbnailID)) - // Must ignore duplicate tasks. - for i := 0; i < 3; i++ { - err = service.SendTask(fileID, nil) - r.NoError(err) - } + // Must ignore duplicate tasks. + for i := 0; i < 3; i++ { + err = service.SendTask(fileID) + r.NoError(err) + } - rc, err := service.OpenThumbnail(context.Background(), thumbnailID) - r.NoError(err) + rc, err := service.OpenThumbnail(context.Background(), thumbnailID) + r.NoError(err) - data, err := io.ReadAll(rc) - r.NoError(err) - r.Equal("resized-content-1.thumbnail.jpg", string(data)) + data, err := io.ReadAll(rc) + r.NoError(err) + r.Equal("resized-content-1.thumbnail.jpg", string(data)) - dur := time.Since(resizeStart) - if dur < 200*time.Millisecond { - t.Fatalf("image must be opened in >=200ms, got: %s", dur) - } + dur := time.Since(resizeStart) + if dur < 200*time.Millisecond { + t.Fatalf("image must be opened in >=200ms, got: %s", dur) + } - r.Equal(1, resizedCount) - r.True(service.IsThumbnailReady(thumbnailID)) + r.Equal(1, openFileFnCount) + r.Equal(1, resizedCount) + r.True(service.IsThumbnailReady(thumbnailID)) - // Same path, but different mod time. - newFileID := rview.NewFileID(fileID.GetPath(), time.Now().Unix()+5) - newThumbnailID := service.NewThumbnailID(newFileID) - r.False(service.IsThumbnailReady(newThumbnailID)) + // Same path, but different mod time. + newFileID := rview.NewFileID(fileID.GetPath(), time.Now().Unix()+5) + newThumbnailID := service.NewThumbnailID(newFileID) + r.False(service.IsThumbnailReady(newThumbnailID)) + }) t.Run("remove resized file after error", func(t *testing.T) { r := require.New(t) - fileID := rview.NewFileID("2.jpg", time.Now().Unix()) - thumbnailID := service.NewThumbnailID(fileID) + service := newService( + t, + func(context.Context, rview.FileID) (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader([]byte("long phrase to exceed threshold"))), nil + }, + func(_, cacheFile string, thumbnailID rview.ThumbnailID) error { + // File must be created by vips, emulate it. + f, err := os.Create(cacheFile) + r.NoError(err) + r.NoError(f.Close()) - service.resizeFn = func(_, cacheFile string, _ rview.ThumbnailID) error { - // File must be created by vips, emulate it. - f, err := os.Create(cacheFile) - r.NoError(err) - r.NoError(f.Close()) + r.NoError(cache.Check(thumbnailID.FileID)) - r.NoError(service.cache.Check(thumbnailID.FileID)) + return errors.New("some error") + }, + ) - return errors.New("some error") - } + fileID := rview.NewFileID("2.jpg", time.Now().Unix()) + thumbnailID := service.NewThumbnailID(fileID) - err = service.SendTask(fileID, func(context.Context, rview.FileID) (io.ReadCloser, error) { - return io.NopCloser(bytes.NewReader([]byte("long phrase to exceed threshold"))), nil - }) + err = service.SendTask(fileID) r.NoError(err) _, err = service.OpenThumbnail(context.Background(), thumbnailID) @@ -106,29 +134,31 @@ func TestThumbnailService(t *testing.T) { t.Run("use original file", func(t *testing.T) { r := require.New(t) + var resizeCalled bool + service := newService( + t, + func(context.Context, rview.FileID) (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader([]byte("x"))), nil + }, + func(_, _ string, _ rview.ThumbnailID) error { + resizeCalled = true + return errors.New("should not be called") + }, + ) + fileID := rview.NewFileID("3.jpg", time.Now().Unix()) thumbnailID := service.NewThumbnailID(fileID) - var resizeCalled bool - service.resizeFn = func(_, _ string, _ rview.ThumbnailID) error { - resizeCalled = true - return errors.New("should not be called") - } - - err = service.SendTask(fileID, func(context.Context, rview.FileID) (io.ReadCloser, error) { - return io.NopCloser(bytes.NewReader([]byte("x"))), nil - }) + err = service.SendTask(fileID) r.NoError(err) - rc, err = service.OpenThumbnail(context.Background(), thumbnailID) + rc, err := service.OpenThumbnail(context.Background(), thumbnailID) r.NoError(err) data, err := io.ReadAll(rc) r.NoError(err) r.Equal("x", string(data)) r.False(resizeCalled) }) - - r.NoError(service.Shutdown(context.Background())) } func TestThumbnailService_CanGenerateThumbnail(t *testing.T) { @@ -138,7 +168,7 @@ func TestThumbnailService_CanGenerateThumbnail(t *testing.T) { now := time.Now().Unix() - canGenerate := NewThumbnailService(nil, 0, false).CanGenerateThumbnail + canGenerate := NewThumbnailService(nil, nil, 0, false).CanGenerateThumbnail r.True(canGenerate(rview.NewFileID("/home/users/test.png", now))) r.True(canGenerate(rview.NewFileID("/home/users/test.pNg", now))) @@ -151,7 +181,7 @@ func TestThumbnailService_CanGenerateThumbnail(t *testing.T) { func TestThumbnailService_NewThumbnailID(t *testing.T) { t.Parallel() - service := NewThumbnailService(nil, 0, false) + service := NewThumbnailService(nil, nil, 0, false) for path, wantThumbnail := range map[string]string{ "/home/cat.jpeg": "/home/cat.thumbnail.jpeg", diff --git a/web/web.go b/web/web.go index ccb15be..cd9f134 100644 --- a/web/web.go +++ b/web/web.go @@ -434,11 +434,7 @@ func (s *Server) sendGenerateThumbnailTasks(info DirInfo) DirInfo { continue } - openFile := func(ctx context.Context, id rview.FileID) (io.ReadCloser, error) { - rc, _, err := s.rclone.GetFile(ctx, id) - return rc, err - } - err := s.thumbnailService.SendTask(id, openFile) + err := s.thumbnailService.SendTask(id) if err != nil { rlog.Errorf("couldn't start resizing for file %q: %s", entry.filepath, err) continue diff --git a/web/web_test.go b/web/web_test.go index 1891dc8..8895243 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -59,7 +59,7 @@ type thumbnailServiceStub struct { func newThumbnailServiceStub() *thumbnailServiceStub { return &thumbnailServiceStub{ - s: thumbnails.NewThumbnailService(cache.NewInMemoryCache(), 0, false), + s: thumbnails.NewThumbnailService(nil, cache.NewInMemoryCache(), 0, false), } } @@ -71,7 +71,7 @@ func (s *thumbnailServiceStub) IsThumbnailReady(id rview.ThumbnailID) bool { return id.GetName() == "resized.thumbnail.jpg" } -func (s *thumbnailServiceStub) SendTask(id rview.FileID, _ rview.OpenFileFn) error { +func (s *thumbnailServiceStub) SendTask(id rview.FileID) error { s.taskCount++ if id.GetName() == "error.jpg" {