From ad345805d95a4dc4f0bdc9487af994baa5b0e57f Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Mon, 11 Apr 2022 11:53:21 +0200 Subject: [PATCH] UX: Skip RAW files by default when downloading albums #2234 Signed-off-by: Michael Mayer --- internal/api/album.go | 76 -------------------- internal/api/album_test.go | 19 ----- internal/api/download_album.go | 104 ++++++++++++++++++++++++++++ internal/api/download_album_test.go | 27 ++++++++ internal/api/download_zip.go | 4 +- 5 files changed, 134 insertions(+), 96 deletions(-) create mode 100644 internal/api/download_album.go create mode 100644 internal/api/download_album_test.go diff --git a/internal/api/album.go b/internal/api/album.go index f2f0ba767..b39473068 100644 --- a/internal/api/album.go +++ b/internal/api/album.go @@ -1,12 +1,9 @@ package api import ( - "archive/zip" "net/http" "path/filepath" - "strings" "sync" - "time" "github.com/gin-gonic/gin" @@ -15,12 +12,10 @@ import ( "github.com/photoprism/photoprism/internal/event" "github.com/photoprism/photoprism/internal/form" "github.com/photoprism/photoprism/internal/i18n" - "github.com/photoprism/photoprism/internal/photoprism" "github.com/photoprism/photoprism/internal/query" "github.com/photoprism/photoprism/internal/search" "github.com/photoprism/photoprism/internal/service" - "github.com/photoprism/photoprism/pkg/fs" "github.com/photoprism/photoprism/pkg/sanitize" ) @@ -463,74 +458,3 @@ func RemovePhotosFromAlbum(router *gin.RouterGroup) { c.JSON(http.StatusOK, gin.H{"code": http.StatusOK, "message": i18n.Msg(i18n.MsgChangesSaved), "album": a, "photos": f.Photos, "removed": removed}) }) } - -// DownloadAlbum streams the album contents as zip archive. -// -// GET /api/v1/albums/:uid/dl -func DownloadAlbum(router *gin.RouterGroup) { - router.GET("/albums/:uid/dl", func(c *gin.Context) { - if InvalidDownloadToken(c) { - AbortUnauthorized(c) - return - } - - start := time.Now() - a, err := query.AlbumByUID(sanitize.IdString(c.Param("uid"))) - - if err != nil { - Abort(c, http.StatusNotFound, i18n.ErrAlbumNotFound) - return - } - - files, err := search.AlbumPhotos(a, 10000, true) - - if err != nil { - AbortEntityNotFound(c) - return - } - - zipFileName := a.ZipName() - - AddDownloadHeader(c, zipFileName) - - zipWriter := zip.NewWriter(c.Writer) - defer func() { _ = zipWriter.Close() }() - - var aliases = make(map[string]int) - - for _, file := range files { - if file.FileHash == "" { - log.Warnf("download: empty file hash, skipped %s", sanitize.Log(file.FileName)) - continue - } - - if file.FileSidecar { - log.Debugf("download: skipped sidecar %s", sanitize.Log(file.FileName)) - continue - } - - fileName := photoprism.FileName(file.FileRoot, file.FileName) - alias := file.ShareBase(0) - key := strings.ToLower(alias) - - if seq := aliases[key]; seq > 0 { - alias = file.ShareBase(seq) - } - - aliases[key] += 1 - - if fs.FileExists(fileName) { - if err := addFileToZip(zipWriter, fileName, alias); err != nil { - log.Error(err) - Abort(c, http.StatusInternalServerError, i18n.ErrZipFailed) - return - } - log.Infof("download: added %s as %s", sanitize.Log(file.FileName), sanitize.Log(alias)) - } else { - log.Errorf("download: failed finding %s", sanitize.Log(file.FileName)) - } - } - - log.Infof("download: created %s [%s]", sanitize.Log(zipFileName), time.Since(start)) - }) -} diff --git a/internal/api/album_test.go b/internal/api/album_test.go index 7e2a6babf..2a54a04e3 100644 --- a/internal/api/album_test.go +++ b/internal/api/album_test.go @@ -235,25 +235,6 @@ func TestRemovePhotosFromAlbum(t *testing.T) { }) } -func TestDownloadAlbum(t *testing.T) { - t.Run("download not existing album", func(t *testing.T) { - app, router, conf := NewApiTest() - - DownloadAlbum(router) - - r := PerformRequest(app, "GET", "/api/v1/albums/5678/dl?t="+conf.DownloadToken()) - assert.Equal(t, http.StatusNotFound, r.Code) - }) - t.Run("download existing album", func(t *testing.T) { - app, router, conf := NewApiTest() - - DownloadAlbum(router) - - r := PerformRequest(app, "GET", "/api/v1/albums/at9lxuqxpogaaba8/dl?t="+conf.DownloadToken()) - assert.Equal(t, http.StatusOK, r.Code) - }) -} - func TestCloneAlbums(t *testing.T) { app, router, _ := NewApiTest() CreateAlbum(router) diff --git a/internal/api/download_album.go b/internal/api/download_album.go new file mode 100644 index 000000000..43f43a92b --- /dev/null +++ b/internal/api/download_album.go @@ -0,0 +1,104 @@ +package api + +import ( + "archive/zip" + "net/http" + "strings" + "time" + + "github.com/gin-gonic/gin" + + "github.com/photoprism/photoprism/internal/i18n" + "github.com/photoprism/photoprism/internal/photoprism" + "github.com/photoprism/photoprism/internal/query" + "github.com/photoprism/photoprism/internal/search" + "github.com/photoprism/photoprism/internal/service" + + "github.com/photoprism/photoprism/pkg/fs" + "github.com/photoprism/photoprism/pkg/sanitize" +) + +// DownloadAlbum streams the album contents as zip archive. +// +// GET /api/v1/albums/:uid/dl +func DownloadAlbum(router *gin.RouterGroup) { + router.GET("/albums/:uid/dl", func(c *gin.Context) { + if InvalidDownloadToken(c) { + AbortUnauthorized(c) + return + } + + conf := service.Config() + + if !conf.Settings().Features.Download { + AbortFeatureDisabled(c) + return + } + + start := time.Now() + a, err := query.AlbumByUID(sanitize.IdString(c.Param("uid"))) + + if err != nil { + Abort(c, http.StatusNotFound, i18n.ErrAlbumNotFound) + return + } + + files, err := search.AlbumPhotos(a, 10000, true) + + if err != nil { + AbortEntityNotFound(c) + return + } + + zipFileName := a.ZipName() + + AddDownloadHeader(c, zipFileName) + + zipWriter := zip.NewWriter(c.Writer) + defer zipWriter.Close() + + skipRaw := !conf.Settings().Download.Raw + + var aliases = make(map[string]int) + + for _, file := range files { + if file.FileHash == "" { + log.Warnf("download: empty file hash, skipped %s", sanitize.Log(file.FileName)) + continue + } + + if file.FileSidecar { + log.Debugf("download: skipped sidecar %s", sanitize.Log(file.FileName)) + continue + } + + if skipRaw && fs.FormatRaw.Is(file.FileType) { + log.Debugf("download: skipped raw %s", sanitize.Log(file.FileName)) + continue + } + + fileName := photoprism.FileName(file.FileRoot, file.FileName) + alias := file.ShareBase(0) + key := strings.ToLower(alias) + + if seq := aliases[key]; seq > 0 { + alias = file.ShareBase(seq) + } + + aliases[key] += 1 + + if fs.FileExists(fileName) { + if err := addFileToZip(zipWriter, fileName, alias); err != nil { + log.Error(err) + Abort(c, http.StatusInternalServerError, i18n.ErrZipFailed) + return + } + log.Infof("download: added %s as %s", sanitize.Log(file.FileName), sanitize.Log(alias)) + } else { + log.Errorf("download: failed finding %s", sanitize.Log(file.FileName)) + } + } + + log.Infof("download: created %s [%s]", sanitize.Log(zipFileName), time.Since(start)) + }) +} diff --git a/internal/api/download_album_test.go b/internal/api/download_album_test.go new file mode 100644 index 000000000..d4a838b39 --- /dev/null +++ b/internal/api/download_album_test.go @@ -0,0 +1,27 @@ +package api + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDownloadAlbum(t *testing.T) { + t.Run("download not existing album", func(t *testing.T) { + app, router, conf := NewApiTest() + + DownloadAlbum(router) + + r := PerformRequest(app, "GET", "/api/v1/albums/5678/dl?t="+conf.DownloadToken()) + assert.Equal(t, http.StatusNotFound, r.Code) + }) + t.Run("download existing album", func(t *testing.T) { + app, router, conf := NewApiTest() + + DownloadAlbum(router) + + r := PerformRequest(app, "GET", "/api/v1/albums/at9lxuqxpogaaba8/dl?t="+conf.DownloadToken()) + assert.Equal(t, http.StatusOK, r.Code) + }) +} diff --git a/internal/api/download_zip.go b/internal/api/download_zip.go index 61bb612e0..d65791940 100644 --- a/internal/api/download_zip.go +++ b/internal/api/download_zip.go @@ -92,6 +92,8 @@ func CreateZip(router *gin.RouterGroup) { dlName := DownloadName(c) + skipRaw := !conf.Settings().Download.Raw + var aliases = make(map[string]int) for _, file := range files { @@ -105,7 +107,7 @@ func CreateZip(router *gin.RouterGroup) { continue } - if !conf.Settings().Download.Raw && fs.FormatRaw.Is(file.FileType) { + if skipRaw && fs.FormatRaw.Is(file.FileType) { log.Debugf("download: skipped raw %s", sanitize.Log(file.FileName)) continue }