From f2ac8d442a3ebadb6c4bbbb81ac8a3c1e53bcc66 Mon Sep 17 00:00:00 2001 From: Robert Swain Date: Thu, 8 Jun 2017 13:06:32 +0200 Subject: [PATCH] mediaapi/thumbnailer: Factor out isThumbnailExists Appease gocyclo^w^w simplify --- .../mediaapi/thumbnailer/thumbnailer.go | 20 ++++++++++ .../mediaapi/thumbnailer/thumbnailer_bimg.go | 38 +++++-------------- .../mediaapi/thumbnailer/thumbnailer_nfnt.go | 24 ++---------- 3 files changed, 33 insertions(+), 49 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer.go b/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer.go index 0a1e507e4..16268d13b 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer.go @@ -17,10 +17,12 @@ package thumbnailer import ( "fmt" "math" + "os" "path/filepath" "sync" log "github.com/Sirupsen/logrus" + "github.com/matrix-org/dendrite/mediaapi/storage" "github.com/matrix-org/dendrite/mediaapi/types" ) @@ -131,6 +133,24 @@ func broadcastGeneration(dst types.Path, activeThumbnailGeneration *types.Active delete(activeThumbnailGeneration.PathToResult, string(dst)) } +func isThumbnailExists(dst types.Path, config types.ThumbnailSize, mediaMetadata *types.MediaMetadata, db *storage.Database, logger *log.Entry) (bool, error) { + thumbnailMetadata, err := db.GetThumbnail(mediaMetadata.MediaID, mediaMetadata.Origin, config.Width, config.Height, config.ResizeMethod) + if err != nil { + logger.Error("Failed to query database for thumbnail.") + return false, err + } + if thumbnailMetadata != nil { + return true, nil + } + // Note: The double-negative is intentional as os.IsExist(err) != !os.IsNotExist(err). + // The functions are error checkers to be used in different cases. + if _, err = os.Stat(string(dst)); !os.IsNotExist(err) { + // Thumbnail exists + return true, nil + } + return false, nil +} + // init with worst values func newThumbnailFitness() thumbnailFitness { return thumbnailFitness{ diff --git a/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer_bimg.go b/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer_bimg.go index 5ba3994e1..d8400a641 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer_bimg.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer_bimg.go @@ -17,7 +17,6 @@ package thumbnailer import ( - "fmt" "os" "time" @@ -34,9 +33,10 @@ func GenerateThumbnails(src types.Path, configs []types.ThumbnailSize, mediaMeta logger.WithError(err).WithField("src", src).Error("Failed to read src file") return false, err } + img := bimg.NewImage(buffer) for _, config := range configs { // Note: createThumbnail does locking based on activeThumbnailGeneration - busy, err = createThumbnail(src, buffer, config, mediaMetadata, activeThumbnailGeneration, maxThumbnailGenerators, db, logger) + busy, err = createThumbnail(src, img, config, mediaMetadata, activeThumbnailGeneration, maxThumbnailGenerators, db, logger) if err != nil { logger.WithError(err).WithField("src", src).Error("Failed to generate thumbnails") return false, err @@ -57,8 +57,9 @@ func GenerateThumbnail(src types.Path, config types.ThumbnailSize, mediaMetadata }).Error("Failed to read src file") return false, err } + img := bimg.NewImage(buffer) // Note: createThumbnail does locking based on activeThumbnailGeneration - busy, err = createThumbnail(src, buffer, config, mediaMetadata, activeThumbnailGeneration, maxThumbnailGenerators, db, logger) + busy, err = createThumbnail(src, img, config, mediaMetadata, activeThumbnailGeneration, maxThumbnailGenerators, db, logger) if err != nil { logger.WithError(err).WithFields(log.Fields{ "src": src, @@ -73,7 +74,7 @@ func GenerateThumbnail(src types.Path, config types.ThumbnailSize, mediaMetadata // createThumbnail checks if the thumbnail exists, and if not, generates it // Thumbnail generation is only done once for each non-existing thumbnail. -func createThumbnail(src types.Path, buffer []byte, config types.ThumbnailSize, mediaMetadata *types.MediaMetadata, activeThumbnailGeneration *types.ActiveThumbnailGeneration, maxThumbnailGenerators int, db *storage.Database, logger *log.Entry) (busy bool, errorReturn error) { +func createThumbnail(src types.Path, img *bimg.Image, config types.ThumbnailSize, mediaMetadata *types.MediaMetadata, activeThumbnailGeneration *types.ActiveThumbnailGeneration, maxThumbnailGenerators int, db *storage.Database, logger *log.Entry) (busy bool, errorReturn error) { logger = logger.WithFields(log.Fields{ "Width": config.Width, "Height": config.Height, @@ -104,30 +105,13 @@ func createThumbnail(src types.Path, buffer []byte, config types.ThumbnailSize, }() } - // Check if the thumbnail exists. - thumbnailMetadata, err := db.GetThumbnail(mediaMetadata.MediaID, mediaMetadata.Origin, config.Width, config.Height, config.ResizeMethod) - if err != nil { - logger.Error("Failed to query database for thumbnail.") + exists, err := isThumbnailExists(dst, config, mediaMetadata, db, logger) + if err != nil || exists { return false, err } - if thumbnailMetadata != nil { - return false, nil - } - // Note: The double-negative is intentional as os.IsExist(err) != !os.IsNotExist(err). - // The functions are error checkers to be used in different cases. - if _, err = os.Stat(string(dst)); !os.IsNotExist(err) { - // Thumbnail exists - return false, nil - } - - if isActive == false { - // Note: This should not happen, but we check just in case. - logger.Error("Failed to stat file but this is not the active thumbnail generator. This should not happen.") - return false, fmt.Errorf("Not active thumbnail generator. Stat error: %q", err) - } start := time.Now() - width, height, err := resize(dst, buffer, config.Width, config.Height, config.ResizeMethod == "crop", logger) + width, height, err := resize(dst, img, config.Width, config.Height, config.ResizeMethod == "crop", logger) if err != nil { return false, err } @@ -142,7 +126,7 @@ func createThumbnail(src types.Path, buffer []byte, config types.ThumbnailSize, return false, err } - thumbnailMetadata = &types.ThumbnailMetadata{ + thumbnailMetadata := &types.ThumbnailMetadata{ MediaMetadata: &types.MediaMetadata{ MediaID: mediaMetadata.MediaID, Origin: mediaMetadata.Origin, @@ -172,9 +156,7 @@ func createThumbnail(src types.Path, buffer []byte, config types.ThumbnailSize, // resize scales an image to fit within the provided width and height // If the source aspect ratio is different to the target dimensions, one edge will be smaller than requested // If crop is set to true, the image will be scaled to fill the width and height with any excess being cropped off -func resize(dst types.Path, buffer []byte, w, h int, crop bool, logger *log.Entry) (int, int, error) { - inImage := bimg.NewImage(buffer) - +func resize(dst types.Path, inImage *bimg.Image, w, h int, crop bool, logger *log.Entry) (int, int, error) { inSize, err := inImage.Size() if err != nil { return -1, -1, err diff --git a/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer_nfnt.go b/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer_nfnt.go index 4517cda41..476e1cb35 100644 --- a/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer_nfnt.go +++ b/src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer_nfnt.go @@ -17,7 +17,6 @@ package thumbnailer import ( - "fmt" "image" "image/draw" // Imported for gif codec @@ -138,27 +137,10 @@ func createThumbnail(src types.Path, img image.Image, config types.ThumbnailSize }() } - // Check if the thumbnail exists. - thumbnailMetadata, err := db.GetThumbnail(mediaMetadata.MediaID, mediaMetadata.Origin, config.Width, config.Height, config.ResizeMethod) - if err != nil { - logger.Error("Failed to query database for thumbnail.") + exists, err := isThumbnailExists(dst, config, mediaMetadata, db, logger) + if err != nil || exists { return false, err } - if thumbnailMetadata != nil { - return false, nil - } - // Note: The double-negative is intentional as os.IsExist(err) != !os.IsNotExist(err). - // The functions are error checkers to be used in different cases. - if _, err = os.Stat(string(dst)); !os.IsNotExist(err) { - // Thumbnail exists - return false, nil - } - - if isActive == false { - // Note: This should not happen, but we check just in case. - logger.Error("Failed to stat file but this is not the active thumbnail generator. This should not happen.") - return false, fmt.Errorf("Not active thumbnail generator. Stat error: %q", err) - } start := time.Now() width, height, err := adjustSize(dst, img, config.Width, config.Height, config.ResizeMethod == "crop", logger) @@ -176,7 +158,7 @@ func createThumbnail(src types.Path, img image.Image, config types.ThumbnailSize return false, err } - thumbnailMetadata = &types.ThumbnailMetadata{ + thumbnailMetadata := &types.ThumbnailMetadata{ MediaMetadata: &types.MediaMetadata{ MediaID: mediaMetadata.MediaID, Origin: mediaMetadata.Origin,