From 2b1240b88b460f82a23854598e10f7bf6d86d7f9 Mon Sep 17 00:00:00 2001 From: Kostiantyn <32730812+kkovaletp@users.noreply.github.com> Date: Wed, 18 Jun 2025 20:38:19 +0300 Subject: [PATCH] Refactor API video route (#1202) * Refactored code; temp disable workflows on push * remove printf leftovers * A better sanitizing code * extend auth routes tests * more tests, fix 2 bugs in token processing * more refactoring and fixes * optimize video mediaURLs DB query and make testCachePath in utils thread-safe * fix always passing test * and more refactoring; 1 more test, but cleanup still has to be fixed * more debug code in the test * fix the test * Final commit * Adding a defer function just in case of panic * Address some of review comments * Protect the shared var by mutex; implement correct context-aware scanning * Better error; better initial func wrap/capture; better log messages in `photos.go`; * Addressing a few more review comments * Order the query results; a better error type catch; setting the MP4 content type header explicitly * try to fix the cancelation detection condition * Extract a function and call it by name --------- Co-authored-by: Konstantin Koval --- api/routes/authenticate_routes.go | 40 ++- api/routes/authenticate_routes_test.go | 130 ++++++- api/routes/photos.go | 16 +- api/routes/photos_test.go | 3 +- api/routes/videos.go | 144 +++++--- api/routes/videos_test.go | 471 +++++++++++++++++++++++++ api/scanner/scanner_media.go | 4 +- api/utils/media_cache.go | 21 +- 8 files changed, 767 insertions(+), 62 deletions(-) create mode 100644 api/routes/videos_test.go diff --git a/api/routes/authenticate_routes.go b/api/routes/authenticate_routes.go index 49ed8a37..67a6f58a 100644 --- a/api/routes/authenticate_routes.go +++ b/api/routes/authenticate_routes.go @@ -3,9 +3,12 @@ package routes import ( "fmt" "net/http" + "time" "github.com/photoview/photoview/api/graphql/auth" "github.com/photoview/photoview/api/graphql/models" + + // "github.com/photoview/photoview/api/log" "github.com/pkg/errors" "golang.org/x/crypto/bcrypt" "gorm.io/gorm" @@ -19,22 +22,24 @@ func authenticateMedia(media *models.Media, db *gorm.DB, r *http.Request) (succe if user != nil { var album models.Album if err := db.First(&album, media.AlbumID).Error; err != nil { + // log.Debug(nil, "Failed to find album for media %d: %v", media.ID, err) return false, internalServerError, http.StatusInternalServerError, err } ownsAlbum, err := user.OwnsAlbum(db, &album) if err != nil { + // log.Debug(nil, "Failed to check if user owns album %d for media %d: %v", media.AlbumID, media.ID, err) return false, internalServerError, http.StatusInternalServerError, err } if !ownsAlbum { + // log.Debug(nil, "User does not own album %d for media %d", media.AlbumID, media.ID) return false, "invalid credentials", http.StatusForbidden, nil } } else { if success, respMsg, respStatus, err := shareTokenFromRequest(db, r, &media.ID, &media.AlbumID); !success { return success, respMsg, respStatus, err } - } return true, "success", http.StatusAccepted, nil @@ -46,17 +51,18 @@ func authenticateAlbum(album *models.Album, db *gorm.DB, r *http.Request) (succe if user != nil { ownsAlbum, err := user.OwnsAlbum(db, album) if err != nil { + // log.Debug(nil, "Failed to check if user owns album %d: %v", album.ID, err) return false, internalServerError, http.StatusInternalServerError, err } if !ownsAlbum { + // log.Debug(nil, "User does not own album %d", album.ID) return false, "invalid credentials", http.StatusForbidden, nil } } else { if success, respMsg, respStatus, err := shareTokenFromRequest(db, r, nil, &album.ID); !success { return success, respMsg, respStatus, err } - } return true, "success", http.StatusAccepted, nil @@ -72,33 +78,48 @@ func shareTokenFromRequest(db *gorm.DB, r *http.Request, mediaID *int, albumID * var shareToken models.ShareToken if err := db.Where("value = ?", token).First(&shareToken).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + // log.Debug(nil, "Share token not found: %s", token) + return false, "unauthorized", http.StatusForbidden, errors.New("invalid share token") + } + // log.Debug(nil, "Error fetching share token: %s, error: %v", token, err) return false, internalServerError, http.StatusInternalServerError, err } + if shareToken.Expire != nil && time.Now().UTC().After(shareToken.Expire.UTC()) { + // log.Debug(nil, "Share token expired: %s", token) + return false, "unauthorized", http.StatusForbidden, errors.New("invalid share token") + } + // Validate share token password, if set if shareToken.Password != nil { tokenPasswordCookie, err := r.Cookie(fmt.Sprintf("share-token-pw-%s", shareToken.Value)) if err != nil { - return false, "unauthorized", http.StatusForbidden, errors.Wrap(err, "get share token password cookie") + // log.Debug(nil, "Error getting share token password cookie: %v", err) + return false, "unauthorized", http.StatusForbidden, errors.Wrap(err, "share token password invalid") } // tokenPassword := r.Header.Get("TokenPassword") tokenPassword := tokenPasswordCookie.Value if err := bcrypt.CompareHashAndPassword([]byte(*shareToken.Password), []byte(tokenPassword)); err != nil { if err == bcrypt.ErrMismatchedHashAndPassword { - return false, "unauthorized", http.StatusForbidden, errors.New("incorrect password for share token") + // log.Debug(nil, "Incorrect password for share token: %s", token) + return false, "unauthorized", http.StatusForbidden, errors.New("share token password invalid") } else { + // log.Debug(nil, "Error comparing share token password: %s, error: %v", token, err) return false, internalServerError, http.StatusInternalServerError, err } } } if shareToken.AlbumID != nil && albumID == nil { - return false, "unauthorized", http.StatusForbidden, errors.New("share token is of type album, but no albumID was provided to function") + // log.Debug(nil, "Share token is of type album, but no albumID was provided to function") + return false, "unauthorized", http.StatusForbidden, errors.New("invalid share token") } if shareToken.MediaID != nil && mediaID == nil { - return false, "unauthorized", http.StatusForbidden, errors.New("share token is of type media, but no mediaID was provided to function") + // log.Debug(nil, "Share token is of type media, but no mediaID was provided to function") + return false, "unauthorized", http.StatusForbidden, errors.New("invalid share token") } if shareToken.AlbumID != nil && *albumID != *shareToken.AlbumID { @@ -115,16 +136,19 @@ func shareTokenFromRequest(db *gorm.DB, r *http.Request, mediaID *int, albumID * `, *shareToken.AlbumID, albumID).Find(&count).Error if err != nil { + // log.Debug(nil, "Error checking child albums for share token: %s, error: %v", token, err) return false, internalServerError, http.StatusInternalServerError, err } if count == 0 { - return false, "unauthorized", http.StatusForbidden, errors.New("no child albums found for share token") + // log.Debug(nil, "No child albums found for share token: %s", token) + return false, "unauthorized", http.StatusForbidden, errors.New("invalid share token") } } if shareToken.MediaID != nil && *mediaID != *shareToken.MediaID { - return false, "unauthorized", http.StatusForbidden, errors.New("media share token does not match mediaID") + // log.Debug(nil, "Media share token does not match mediaID: %d != %d", *mediaID, *shareToken.MediaID) + return false, "unauthorized", http.StatusForbidden, errors.New("invalid share token") } return true, "", 0, nil diff --git a/api/routes/authenticate_routes_test.go b/api/routes/authenticate_routes_test.go index e74bac35..e77effee 100644 --- a/api/routes/authenticate_routes_test.go +++ b/api/routes/authenticate_routes_test.go @@ -94,6 +94,70 @@ func TestAuthenticateRoute(t *testing.T) { assert.Equal(t, "success", responseMessage) assert.Equal(t, http.StatusAccepted, responseStatus) }) + + t.Run("Request with invalid share token", func(t *testing.T) { + url := fmt.Sprintf("/photo/image.jpg?token=%s", "invalid-token") + req := httptest.NewRequest("GET", url, strings.NewReader(imageData)) + // Even if a cookie is sent, the token is invalid + cookie := http.Cookie{ + Name: fmt.Sprintf("share-token-pw-%s", "invalid-token"), + Value: "whatever", + } + req.AddCookie(&cookie) + success, responseMessage, responseStatus, err := authenticateMedia(&media, db, req) + assert.Error(t, err) + assert.False(t, success) + assert.Equal(t, "unauthorized", responseMessage) + assert.Equal(t, http.StatusForbidden, responseStatus) + }) + + t.Run("Request with share token but no password cookie", func(t *testing.T) { + shareToken, err := actions.AddMediaShare(db, user, media.ID, &expire, &tokenPassword) + assert.NoError(t, err) + url := fmt.Sprintf("/photo/image.jpg?token=%s", shareToken.Value) + req := httptest.NewRequest("GET", url, strings.NewReader(imageData)) + // No cookie provided + success, responseMessage, responseStatus, err := authenticateMedia(&media, db, req) + assert.Error(t, err) + assert.False(t, success) + assert.Equal(t, "unauthorized", responseMessage) + assert.Equal(t, http.StatusForbidden, responseStatus) + }) + + t.Run("Request with share token and wrong password", func(t *testing.T) { + shareToken, err := actions.AddMediaShare(db, user, media.ID, &expire, &tokenPassword) + assert.NoError(t, err) + url := fmt.Sprintf("/photo/image.jpg?token=%s", shareToken.Value) + req := httptest.NewRequest("GET", url, strings.NewReader(imageData)) + cookie := http.Cookie{ + Name: fmt.Sprintf("share-token-pw-%s", shareToken.Value), + Value: "incorrect-password", + } + req.AddCookie(&cookie) + success, responseMessage, responseStatus, err := authenticateMedia(&media, db, req) + assert.Error(t, err) + assert.False(t, success) + assert.Equal(t, "unauthorized", responseMessage) + assert.Equal(t, http.StatusForbidden, responseStatus) + }) + + t.Run("Request with expired share token", func(t *testing.T) { + expired := time.Now().Add(-time.Hour) + shareToken, err := actions.AddMediaShare(db, user, media.ID, &expired, &tokenPassword) + assert.NoError(t, err) + url := fmt.Sprintf("/photo/image.jpg?token=%s", shareToken.Value) + req := httptest.NewRequest("GET", url, strings.NewReader(imageData)) + cookie := http.Cookie{ + Name: fmt.Sprintf("share-token-pw-%s", shareToken.Value), + Value: tokenPassword, + } + req.AddCookie(&cookie) + success, responseMessage, responseStatus, err := authenticateMedia(&media, db, req) + assert.Error(t, err) + assert.False(t, success) + assert.Equal(t, "unauthorized", responseMessage) + assert.Equal(t, http.StatusForbidden, responseStatus) + }) }) t.Run("Authenticate Album", func(t *testing.T) { @@ -138,13 +202,75 @@ func TestAuthenticateRoute(t *testing.T) { } req.AddCookie(&cookie) - success, responseMessage, responseStatus, err := authenticateMedia(&media, db, req) + success, responseMessage, responseStatus, err := authenticateAlbum(&album, db, req) assert.NoError(t, err) assert.True(t, success) assert.Equal(t, "success", responseMessage) assert.Equal(t, http.StatusAccepted, responseStatus) }) - }) + t.Run("Request with invalid album share token", func(t *testing.T) { + url := fmt.Sprintf("/download/album/1?token=%s", "invalid-token") + req := httptest.NewRequest("GET", url, strings.NewReader(albumData)) + cookie := http.Cookie{ + Name: fmt.Sprintf("share-token-pw-%s", "invalid-token"), + Value: "whatever", + } + req.AddCookie(&cookie) + success, responseMessage, responseStatus, err := authenticateAlbum(&album, db, req) + assert.Error(t, err) + assert.False(t, success) + assert.Equal(t, "unauthorized", responseMessage) + assert.Equal(t, http.StatusForbidden, responseStatus) + }) + + t.Run("Request with album share token but no password cookie", func(t *testing.T) { + shareToken, err := actions.AddAlbumShare(db, user, album.ID, &expire, &tokenPassword) + assert.NoError(t, err) + url := fmt.Sprintf("/download/album/1?token=%s", shareToken.Value) + req := httptest.NewRequest("GET", url, strings.NewReader(albumData)) + // No cookie provided + success, responseMessage, responseStatus, err := authenticateAlbum(&album, db, req) + assert.Error(t, err) + assert.False(t, success) + assert.Equal(t, "unauthorized", responseMessage) + assert.Equal(t, http.StatusForbidden, responseStatus) + }) + + t.Run("Request with album share token and wrong password", func(t *testing.T) { + shareToken, err := actions.AddAlbumShare(db, user, album.ID, &expire, &tokenPassword) + assert.NoError(t, err) + url := fmt.Sprintf("/download/album/1?token=%s", shareToken.Value) + req := httptest.NewRequest("GET", url, strings.NewReader(albumData)) + cookie := http.Cookie{ + Name: fmt.Sprintf("share-token-pw-%s", shareToken.Value), + Value: "incorrect-password", + } + req.AddCookie(&cookie) + success, responseMessage, responseStatus, err := authenticateAlbum(&album, db, req) + assert.Error(t, err) + assert.False(t, success) + assert.Equal(t, "unauthorized", responseMessage) + assert.Equal(t, http.StatusForbidden, responseStatus) + }) + + t.Run("Request with expired album share token", func(t *testing.T) { + expired := time.Now().Add(-time.Hour) + shareToken, err := actions.AddAlbumShare(db, user, album.ID, &expired, &tokenPassword) + assert.NoError(t, err) + url := fmt.Sprintf("/download/album/1?token=%s", shareToken.Value) + req := httptest.NewRequest("GET", url, strings.NewReader(albumData)) + cookie := http.Cookie{ + Name: fmt.Sprintf("share-token-pw-%s", shareToken.Value), + Value: tokenPassword, + } + req.AddCookie(&cookie) + success, responseMessage, responseStatus, err := authenticateAlbum(&album, db, req) + assert.Error(t, err) + assert.False(t, success) + assert.Equal(t, "unauthorized", responseMessage) + assert.Equal(t, http.StatusForbidden, responseStatus) + }) + }) } diff --git a/api/routes/photos.go b/api/routes/photos.go index 9a9e928c..e1b1fab8 100644 --- a/api/routes/photos.go +++ b/api/routes/photos.go @@ -1,7 +1,6 @@ package routes import ( - "log" "net/http" "os" @@ -9,6 +8,7 @@ import ( "gorm.io/gorm" "github.com/photoview/photoview/api/graphql/models" + "github.com/photoview/photoview/api/log" "github.com/photoview/photoview/api/scanner" ) @@ -34,7 +34,7 @@ func RegisterPhotoRoutes(db *gorm.DB, router *mux.Router) { if success, response, status, err := authenticateMedia(media, db, r); !success { if err != nil { - log.Printf("WARN: error authenticating photo: %s\n", err) + log.Warn(r.Context(), "error authenticating photo", "error", err) } w.WriteHeader(status) w.Write([]byte(response)) @@ -43,7 +43,7 @@ func RegisterPhotoRoutes(db *gorm.DB, router *mux.Router) { cachedPath, err := mediaURL.CachedPath() if err != nil { - log.Printf("ERROR: %s\n", err) + log.Error(r.Context(), "error getting cached path for media URL", "error", err) w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(internalServerError)) return @@ -51,15 +51,19 @@ func RegisterPhotoRoutes(db *gorm.DB, router *mux.Router) { if _, err := os.Stat(cachedPath); os.IsNotExist((err)) { // err := db.Transaction(func(tx *gorm.DB) error { - if err = scanner.ProcessSingleMediaFunc(db, media); err != nil { - log.Printf("ERROR: processing image not found in cache (%s): %s\n", cachedPath, err) + if err = scanner.ProcessSingleMediaFunc(r.Context(), db, media); err != nil { + log.Error(r.Context(), "processing image not found in cache", + "media_cache_path", cachedPath, + "error", err) w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(internalServerError)) return } if _, err = os.Stat(cachedPath); err != nil { - log.Printf("ERROR: after reprocessing image not found in cache (%s): %s\n", cachedPath, err) + log.Error(r.Context(), "after reprocessing image not found in cache", + "media_cache_path", cachedPath, + "error", err) w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(internalServerError)) return diff --git a/api/routes/photos_test.go b/api/routes/photos_test.go index c943b8cb..6b800131 100644 --- a/api/routes/photos_test.go +++ b/api/routes/photos_test.go @@ -1,6 +1,7 @@ package routes import ( + "context" "fmt" "net/http" "net/http/httptest" @@ -89,7 +90,7 @@ func TestPhotoRoutes(t *testing.T) { // mock scan to fail origScan := scanner.ProcessSingleMediaFunc - scanner.ProcessSingleMediaFunc = func(db *gorm.DB, m *models.Media) error { + scanner.ProcessSingleMediaFunc = func(ctx context.Context, db *gorm.DB, m *models.Media) error { return fmt.Errorf("scan error") } defer func() { scanner.ProcessSingleMediaFunc = origScan }() diff --git a/api/routes/videos.go b/api/routes/videos.go index 6048a89b..f8ac4b73 100644 --- a/api/routes/videos.go +++ b/api/routes/videos.go @@ -1,7 +1,7 @@ package routes import ( - "log" + "context" "net/http" "os" "path" @@ -9,64 +9,128 @@ import ( "github.com/gorilla/mux" "github.com/photoview/photoview/api/graphql/models" + "github.com/photoview/photoview/api/log" "github.com/photoview/photoview/api/scanner" "github.com/photoview/photoview/api/utils" + "github.com/pkg/errors" "gorm.io/gorm" ) -func RegisterVideoRoutes(db *gorm.DB, router *mux.Router) { +var processSingleMediaFn = func(ctx context.Context, db *gorm.DB, media *models.Media) error { + return scanner.ProcessSingleMedia(ctx, db, media) +} - router.HandleFunc("/{name}", func(w http.ResponseWriter, r *http.Request) { - mediaName := mux.Vars(r)["name"] +func handleVideoRequest( + w http.ResponseWriter, + r *http.Request, + db *gorm.DB, + mediaName string, + authenticateFn func(*models.Media, *gorm.DB, *http.Request) (bool, string, int, error), + getCachePathFn func(albumID, mediaID int, filename string) string, +) { + var mediaURLs []models.MediaURL + if err := db.Model(&models.MediaURL{}). + Preload("Media"). + Where("media_urls.media_name = ? AND media_urls.purpose = ?", mediaName, models.VideoWeb). + Order("created_at DESC"). + Find(&mediaURLs). + Error; err != nil || len(mediaURLs) == 0 || mediaURLs[0].Media == nil { - var mediaURL models.MediaURL - result := db.Model(&models.MediaURL{}).Select("media_urls.*").Joins("Media").Where("media_urls.media_name = ?", mediaName).Find(&mediaURL) - if err := result.Error; err != nil { - w.WriteHeader(http.StatusNotFound) - w.Write([]byte("404")) + w.WriteHeader(http.StatusNotFound) + w.Write([]byte("not found")) + return + } + + if len(mediaURLs) > 1 { + log.Warn(r.Context(), "Multiple video web URLs found", + "name", mediaName, + "count", len(mediaURLs), + "using", mediaURLs[0], + ) + } + + mediaURL := mediaURLs[0] + var media = mediaURL.Media + + if success, response, status, err := authenticateFn(media, db, r); !success { + if err != nil { + log.Warn(r.Context(), "got error authenticating video", + "error", err, + "media ID", media.ID, + "media path", media.Path) + } + w.WriteHeader(status) + w.Write([]byte(response)) + return + } + + var cachedPath string + + if mediaURL.Purpose == models.VideoWeb { + // Use the provided cache path function + cachedPath = getCachePathFn(int(media.AlbumID), int(mediaURL.MediaID), mediaURL.MediaName) + } else { + log.Error(r.Context(), "Can not handle media_purpose for video", + "purpose", mediaURL.Purpose, + "expected", models.VideoWeb) + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(internalServerError)) + return + } + + if _, err := os.Stat(cachedPath); err != nil { + if !os.IsNotExist(err) { + log.Error(r.Context(), "cached video access error", + "error", err, + "media ID", media.ID, + "media path", media.Path) + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(internalServerError)) return } - var media = mediaURL.Media - - if success, response, status, err := authenticateMedia(media, db, r); !success { - if err != nil { - log.Printf("WARN: error authenticating video: %s\n", err) + if err := processSingleMediaFn(r.Context(), db, media); err != nil { + // Check if error was due to context cancellation + if r.Context().Err() != nil && errors.Is(r.Context().Err(), context.Canceled) { + log.Warn(r.Context(), "video processing cancelled due to client disconnect", + "mediaID", media.ID, + "reason", r.Context().Err()) + return // Don't send response if client disconnected } - w.WriteHeader(status) - w.Write([]byte(response)) - return - } - var cachedPath string - - if mediaURL.Purpose == models.VideoWeb { - cachedPath = path.Join(utils.MediaCachePath(), strconv.Itoa(int(media.AlbumID)), strconv.Itoa(int(mediaURL.MediaID)), mediaURL.MediaName) - } else { - log.Printf("ERROR: Can not handle media_purpose for video: %s\n", mediaURL.Purpose) + log.Error(r.Context(), "processing video not found in cache", + "error", err, + "media ID", media.ID, + "media path", media.Path) w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(internalServerError)) return } if _, err := os.Stat(cachedPath); err != nil { - if os.IsNotExist(err) { - if err := scanner.ProcessSingleMedia(db, media); err != nil { - log.Printf("ERROR: processing video not found in cache: %s\n", err) - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(internalServerError)) - return - } - - if _, err := os.Stat(cachedPath); err != nil { - log.Printf("ERROR: after reprocessing video not found in cache: %s\n", err) - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(internalServerError)) - return - } - } + log.Error(r.Context(), "video not found in cache after reprocessing", + "error", err, + "media ID", media.ID, + "media path", media.Path) + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(internalServerError)) + return } + } - http.ServeFile(w, r, cachedPath) + w.Header().Set("Cache-Control", "private, max-age=86400, immutable") + w.Header().Set("Content-Type", mediaURL.ContentType) + http.ServeFile(w, r, cachedPath) +} + +func generateCacheFilename(albumID, mediaID int, filename string) string { + return path.Join(utils.MediaCachePath(), strconv.Itoa(albumID), strconv.Itoa(mediaID), filename) +} + +func RegisterVideoRoutes(db *gorm.DB, router *mux.Router) { + + router.HandleFunc("/{name}", func(w http.ResponseWriter, r *http.Request) { + mediaName := mux.Vars(r)["name"] + handleVideoRequest(w, r, db, mediaName, authenticateMedia, generateCacheFilename) }) } diff --git a/api/routes/videos_test.go b/api/routes/videos_test.go new file mode 100644 index 00000000..f89a6cb1 --- /dev/null +++ b/api/routes/videos_test.go @@ -0,0 +1,471 @@ +package routes + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path" + "path/filepath" + "strconv" + "testing" + "time" + + "github.com/gorilla/mux" + "github.com/photoview/photoview/api/graphql/models" + "github.com/photoview/photoview/api/graphql/models/actions" + "github.com/photoview/photoview/api/test_utils" + "github.com/photoview/photoview/api/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" +) + +// setTestCachePath temporarily sets a different media cache path for testing +// and returns a function to restore the original state +func setTestCachePath(tempPath string) func() { + original := utils.GetTestCachePath() + utils.ConfigureTestCache(tempPath) + return func() { + utils.ConfigureTestCache(original) + } +} + +// mockProcessSingleMedia replaces scanner.ProcessSingleMedia with a mock function during tests +// and returns a function to restore the original implementation +var originalProcessSingleMedia = processSingleMediaFn + +func mockProcessSingleMedia(t *testing.T, shouldSucceed bool, mediaID int, albumID int) func() { + // Save original implementation + savedFn := processSingleMediaFn + + // Replace with mock implementation + processSingleMediaFn = func(ctx context.Context, db *gorm.DB, media *models.Media) error { + // Check if context is already cancelled before starting work + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + + if shouldSucceed { + // On success: create the expected video file in cache + var mediaURLs []models.MediaURL + if err := db.Where("media_id = ? AND purpose = ?", media.ID, models.VideoWeb). + Find(&mediaURLs).Error; err != nil { + return err + } + + if len(mediaURLs) == 0 { + return fmt.Errorf("no media URLs found") + } + + // Get the cache path + tempCachePath := utils.GetTestCachePath() + albumDir := filepath.Join(tempCachePath, strconv.Itoa(albumID)) + mediaDir := filepath.Join(albumDir, strconv.Itoa(mediaID)) + if err := os.MkdirAll(mediaDir, 0755); err != nil { + return err + } + + videoPath := filepath.Join(mediaDir, mediaURLs[0].MediaName) + if err := os.WriteFile(videoPath, []byte("mocked processed video content"), 0644); err != nil { + return fmt.Errorf("failed to write mock video file: %w", err) + } + return nil + } + + // On failure: return an error + return fmt.Errorf("mock processing error") + } + + // Return cleanup function + return func() { + processSingleMediaFn = savedFn + } +} + +func registerMockVideoRoutesForTesting(db *gorm.DB, router *mux.Router, tempCachePath string) { + router.HandleFunc("/{name}", func(w http.ResponseWriter, r *http.Request) { + mediaName := mux.Vars(r)["name"] + + // Use no-op auth and test cache path + handleVideoRequest( + w, r, db, mediaName, + // Skip authentication for tests + func(media *models.Media, db *gorm.DB, r *http.Request) (bool, string, int, error) { + return true, "success", http.StatusOK, nil + }, + // Use test cache path + func(albumID, mediaID int, filename string) string { + return path.Join(tempCachePath, strconv.Itoa(albumID), strconv.Itoa(mediaID), filename) + }, + ) + }) +} + +// createTestResources creates all the necessary test resources for a single test case +// and returns cleanup functions to be called with t.Cleanup() +func createTestResources(t *testing.T, db *gorm.DB, testID string) ( + *models.User, + *models.Album, + *models.Media, + *models.MediaURL, + string, // mediaName + string, // cachePath + string, // shareToken + string, // tokenPassword +) { + // Create test user with unique username + user := &models.User{ + Username: fmt.Sprintf("testuser-%s", testID), + } + require.NoError(t, db.Create(user).Error) + t.Cleanup(func() { + db.Unscoped().Delete(user) + }) + + // Create test album with unique title and path + album := &models.Album{ + Title: fmt.Sprintf("Test Album %s", testID), + Path: fmt.Sprintf("/test/album/path/%s", testID), + } + require.NoError(t, db.Create(album).Error) + t.Cleanup(func() { + db.Unscoped().Delete(album) + }) + + // Establish ownership via many-to-many relationship + require.NoError(t, db.Model(album).Association("Owners").Append(user)) + t.Cleanup(func() { + db.Model(album).Association("Owners").Clear() + }) + + // Create unique media name for this test + mediaName := fmt.Sprintf("video-%s.mp4", testID) + + // Create media with VideoWeb purpose + media := &models.Media{ + Title: fmt.Sprintf("Test Video %s", testID), + Path: filepath.Join(t.TempDir(), mediaName), + PathHash: fmt.Sprintf("testhash-%s", testID), + AlbumID: album.ID, + Album: *album, + DateShot: time.Now(), + Type: "video", + } + require.NoError(t, db.Create(media).Error) + t.Cleanup(func() { + db.Unscoped().Delete(media) + }) + + // Create media URL entry + mediaURL := &models.MediaURL{ + MediaID: media.ID, + Media: media, + MediaName: mediaName, + Width: 1920, + Height: 1080, + Purpose: models.VideoWeb, + ContentType: "video/mp4", + FileSize: 1024, + } + require.NoError(t, db.Create(mediaURL).Error) + t.Cleanup(func() { + db.Unscoped().Delete(mediaURL) + }) + + // Create a unique cache path for this test + cachePath := filepath.Join(t.TempDir(), fmt.Sprintf("cache-%s", testID)) + require.NoError(t, os.MkdirAll(cachePath, 0755)) + t.Cleanup(func() { + os.RemoveAll(cachePath) + }) + + // Prepare share token for auth tests + tokenPassword := fmt.Sprintf("secret-password-%s", testID) + expiry := time.Now().Add(24 * time.Hour) + shareToken, err := actions.AddMediaShare(db, user, media.ID, &expiry, &tokenPassword) + require.NoError(t, err) + t.Cleanup(func() { + db.Unscoped().Delete(shareToken) + }) + + return user, album, media, mediaURL, mediaName, cachePath, shareToken.Value, tokenPassword +} + +func TestVideoRoutes(t *testing.T) { + // Ensure original function is always restored + defer func() { + processSingleMediaFn = originalProcessSingleMedia + }() + + // Setup test database + db := test_utils.DatabaseTest(t) + + // Define test cases + testCases := []struct { + name string + testFunc func(*testing.T, *gorm.DB) + }{ + { + name: "Valid video retrieval", + testFunc: func(t *testing.T, db *gorm.DB) { + // Create unique resources for this test + _, album, media, _, mediaName, cachePath, _, _ := createTestResources(t, db, "valid") + + // Setup cache path for this test + restorePath := setTestCachePath(cachePath) + t.Cleanup(restorePath) + + // Create cache directory and file + albumDir := filepath.Join(cachePath, strconv.Itoa(int(album.ID))) + mediaDir := filepath.Join(albumDir, strconv.Itoa(int(media.ID))) + require.NoError(t, os.MkdirAll(mediaDir, 0755)) + + videoPath := filepath.Join(mediaDir, mediaName) + require.NoError(t, os.WriteFile(videoPath, []byte("test video content"), 0644)) + + // Create mock router without auth for this test + router := mux.NewRouter() + registerMockVideoRoutesForTesting(db, router, cachePath) + + // Make request + req := httptest.NewRequest("GET", "/"+mediaName, nil) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Validate response + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, "test video content", rr.Body.String()) + }, + }, + { + name: "Video not found", + testFunc: func(t *testing.T, db *gorm.DB) { + // Create unique resources for this test + _, _, _, _, _, cachePath, _, _ := createTestResources(t, db, "notfound") + + // Setup cache path for this test + restorePath := setTestCachePath(cachePath) + t.Cleanup(restorePath) + + // Create mock router without auth for this test + router := mux.NewRouter() + registerMockVideoRoutesForTesting(db, router, cachePath) + + // Make request with nonexistent video name + req := httptest.NewRequest("GET", "/nonexistent.mp4", nil) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Validate response + assert.Equal(t, http.StatusNotFound, rr.Code) + assert.Equal(t, "not found", rr.Body.String()) + }, + }, + { + name: "Authentication with share token", + testFunc: func(t *testing.T, db *gorm.DB) { + // Create unique resources for this test + _, album, media, _, mediaName, cachePath, tokenValue, tokenPassword := createTestResources(t, db, "auth") + + // Setup cache path for this test + restorePath := setTestCachePath(cachePath) + t.Cleanup(restorePath) + + // Create the file in cache + albumDir := filepath.Join(cachePath, strconv.Itoa(int(album.ID))) + mediaDir := filepath.Join(albumDir, strconv.Itoa(int(media.ID))) + require.NoError(t, os.MkdirAll(mediaDir, 0755)) + + videoPath := filepath.Join(mediaDir, mediaName) + require.NoError(t, os.WriteFile(videoPath, []byte("test video content"), 0644)) + + // Create real router with auth for this test + router := mux.NewRouter() + RegisterVideoRoutes(db, router) + + // Make request with token + req := httptest.NewRequest("GET", "/"+mediaName+"?token="+tokenValue, nil) + cookie := http.Cookie{ + Name: fmt.Sprintf("share-token-pw-%s", tokenValue), + Value: tokenPassword, + } + req.AddCookie(&cookie) + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Validate response + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, "test video content", rr.Body.String()) + }, + }, + { + name: "Multiple media URLs with same name", + testFunc: func(t *testing.T, db *gorm.DB) { + // Create unique resources for this test + _, album, media, _, mediaName, cachePath, _, _ := createTestResources(t, db, "multiple") + + // Setup cache path for this test + restorePath := setTestCachePath(cachePath) + t.Cleanup(restorePath) + + // Create second mediaURL with same name + mediaURL2 := &models.MediaURL{ + MediaID: media.ID, + Media: media, + MediaName: mediaName, // Same name + Width: 1280, + Height: 720, + Purpose: models.VideoWeb, + ContentType: "video/mp4", + FileSize: 512, + } + require.NoError(t, db.Create(mediaURL2).Error) + t.Cleanup(func() { + db.Unscoped().Delete(mediaURL2) + }) + + // Create cache directory and file + albumDir := filepath.Join(cachePath, strconv.Itoa(int(album.ID))) + mediaDir := filepath.Join(albumDir, strconv.Itoa(int(media.ID))) + require.NoError(t, os.MkdirAll(mediaDir, 0755)) + + videoPath := filepath.Join(mediaDir, mediaName) + require.NoError(t, os.WriteFile(videoPath, []byte("test video content"), 0644)) + + // Create mock router without auth for this test + router := mux.NewRouter() + registerMockVideoRoutesForTesting(db, router, cachePath) + + // Make request + req := httptest.NewRequest("GET", "/"+mediaName, nil) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Validate response + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, "test video content", rr.Body.String()) + }, + }, + { + name: "Video file not in cache, processing succeeds", + testFunc: func(t *testing.T, db *gorm.DB) { + // Create unique resources for this test + _, album, media, _, mediaName, cachePath, _, _ := createTestResources(t, db, "process-success") + + // Setup cache path for this test + restorePath := setTestCachePath(cachePath) + t.Cleanup(restorePath) + + // Ensure cache directory exists but file doesn't exist + albumDir := filepath.Join(cachePath, strconv.Itoa(int(album.ID))) + mediaDir := filepath.Join(albumDir, strconv.Itoa(int(media.ID))) + require.NoError(t, os.MkdirAll(mediaDir, 0755)) + + // Mock processing to succeed + restoreProcessingFn := mockProcessSingleMedia(t, true, int(media.ID), int(album.ID)) + t.Cleanup(restoreProcessingFn) + + // Create mock router without auth for this test + router := mux.NewRouter() + registerMockVideoRoutesForTesting(db, router, cachePath) + + // Make request + req := httptest.NewRequest("GET", "/"+mediaName, nil) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Validate response + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, "mocked processed video content", rr.Body.String()) + }, + }, + { + name: "Video file not in cache, processing fails", + testFunc: func(t *testing.T, db *gorm.DB) { + // Create unique resources for this test + _, album, media, _, mediaName, cachePath, _, _ := createTestResources(t, db, "process-fail") + + // Setup cache path for this test + restorePath := setTestCachePath(cachePath) + t.Cleanup(restorePath) + + // Ensure cache directory exists but file doesn't exist + albumDir := filepath.Join(cachePath, strconv.Itoa(int(album.ID))) + mediaDir := filepath.Join(albumDir, strconv.Itoa(int(media.ID))) + require.NoError(t, os.MkdirAll(mediaDir, 0755)) + + // Mock processing to fail + restoreProcessingFn := mockProcessSingleMedia(t, false, int(media.ID), int(album.ID)) + t.Cleanup(restoreProcessingFn) + + // Create mock router without auth for this test + router := mux.NewRouter() + registerMockVideoRoutesForTesting(db, router, cachePath) + + // Make request + req := httptest.NewRequest("GET", "/"+mediaName, nil) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Validate response + assert.Equal(t, http.StatusInternalServerError, rr.Code) + }, + }, + { + name: "Context cancellation during processing", + testFunc: func(t *testing.T, db *gorm.DB) { + _, album, media, _, mediaName, cachePath, _, _ := createTestResources(t, db, "cancellation") + + // Setup cache path for this test + restorePath := setTestCachePath(cachePath) + t.Cleanup(restorePath) + + // Ensure cache directory exists but file doesn't exist to trigger processing + albumDir := filepath.Join(cachePath, strconv.Itoa(int(album.ID))) + mediaDir := filepath.Join(albumDir, strconv.Itoa(int(media.ID))) + require.NoError(t, os.MkdirAll(mediaDir, 0755)) + + // Create cancellable context + ctx, cancel := context.WithCancel(context.Background()) + + // Mock processing that simulates context cancellation + savedFn := processSingleMediaFn + processSingleMediaFn = func(reqCtx context.Context, db *gorm.DB, media *models.Media) error { + cancel() + return fmt.Errorf("processing interrupted by cancellation") + } + t.Cleanup(func() { processSingleMediaFn = savedFn }) + + // Create request with cancelled context + req := httptest.NewRequest("GET", "/video/"+mediaName, nil) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + + // Use testing router without auth + mockRouter := mux.NewRouter().PathPrefix("/video").Subrouter() + registerMockVideoRoutesForTesting(db, mockRouter, cachePath) + + mockRouter.ServeHTTP(w, req) + + // When context is cancelled, processing should be cancelled + // 1. Status remains default 200 (no explicit status written) + assert.Equal(t, http.StatusOK, w.Code, "Status should remain default when context cancelled") + + // 2. Response body should be empty (no video content served) + assert.Empty(t, w.Body.String(), "Response body should be empty when context cancelled") + }, + }, + } + + // Run test cases + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.testFunc(t, db) + }) + } +} diff --git a/api/scanner/scanner_media.go b/api/scanner/scanner_media.go index 98c600eb..dfddf403 100644 --- a/api/scanner/scanner_media.go +++ b/api/scanner/scanner_media.go @@ -72,7 +72,7 @@ func ScanMedia(tx *gorm.DB, mediaPath string, albumId int, cache *scanner_cache. // ProcessSingleMedia processes a single media, might be used to reprocess media with corrupted cache // Function waits for processing to finish before returning. -func ProcessSingleMedia(db *gorm.DB, media *models.Media) error { +func ProcessSingleMedia(ctx context.Context, db *gorm.DB, media *models.Media) error { albumCache := scanner_cache.MakeAlbumCache() var album models.Album @@ -82,7 +82,7 @@ func ProcessSingleMedia(db *gorm.DB, media *models.Media) error { mediaData := media_encoding.NewEncodeMediaData(media) - taskContext := scanner_task.NewTaskContext(context.Background(), db, &album, albumCache) + taskContext := scanner_task.NewTaskContext(ctx, db, &album, albumCache) if err := scanMedia(taskContext, media, &mediaData, 0, 1); err != nil { return errors.Wrap(err, "single media scan") } diff --git a/api/utils/media_cache.go b/api/utils/media_cache.go index 82ed8995..135612c2 100644 --- a/api/utils/media_cache.go +++ b/api/utils/media_cache.go @@ -4,6 +4,7 @@ import ( "os" "path" "strconv" + "sync" "github.com/pkg/errors" ) @@ -37,16 +38,30 @@ func CachePathForMedia(albumID int, mediaID int) (string, error) { return photoCachePath, nil } -var testCachePath string = "" +var ( + testCachePath string = "" + testCachePathLocker sync.RWMutex +) + +func GetTestCachePath() string { + testCachePathLocker.RLock() + defer testCachePathLocker.RUnlock() + return testCachePath +} func ConfigureTestCache(tmpDir string) { + testCachePathLocker.Lock() + defer testCachePathLocker.Unlock() testCachePath = tmpDir } // MediaCachePath returns the path for where the media cache is located on the file system func MediaCachePath() string { - if testCachePath != "" { - return testCachePath + testCachePathLocker.RLock() + cachedPath := testCachePath + testCachePathLocker.RUnlock() + if cachedPath != "" { + return cachedPath } photoCache := EnvMediaCachePath.GetValue()