fix: replace imagemagick with libvips to increase performance (#60)

.golangci.yaml

@@ -10,6 +10,7 @@ linters:
       # - paralleltest
       # - testpackage
       # - forbidigo
+      - varnamelen
       - gomoddirectives
       - nlreturn
       - wsl
@@ -25,15 +26,6 @@ issues:
       linters:
         - gochecknoinits
         - gochecknoglobals
-    - text: "variable name 'ok' is too short for the scope of its usage"
-      linters:
-        - varnamelen
-    - text: "variable name 'b' is too short for the scope of its usage"
-      linters:
-        - varnamelen
-    - text: "parameter name 'wr' is too short for the scope of its usage"
-      linters:
-        - varnamelen
     - linters:
         - lll
       source: "^//go:generate "
@@ -44,4 +36,3 @@ issues:
       linters:
         - funlen
         - ireturn
-        - varnamelen

Makefile

@@ -37,7 +37,7 @@ integration-tests: ## Run go test with integration flags
 	 TEST_S3_ACCESS_KEY=$(shell make dev-s3-access-key) \
 	 TEST_S3_SECRET_KEY=$(shell make dev-s3-secret-key) \
 	 GIN_MODE=release \
-		richgo test -tags=integration $(GOTEST_OPTIONS) ./... # -run=TestGetFilePresignedURL
+		richgo test -tags=integration $(GOTEST_OPTIONS) ./...   # -run=TestGetFileByID
 
 
 .PHONY: build

build/dev/docker/docker-compose.yaml

@@ -59,4 +59,3 @@ services:
       S3_BUCKET: "default"
       S3_ROOT_FOLDER: "f215cf48-7458-4596-9aa5-2159fc6a3caf"
     command: serve --postgres-migrations --postgres-migrations-source ${HASURA_GRAPHQL_DATABASE_URL:-postgres://postgres:hejsan@postgres:5432/postgres?sslmode=disable} --hasura-metadata --hasura-admin-secret ${HASURA_ADMIN_SECRET:-hello123} 
-

client/get_file_information_test.go

@@ -187,9 +187,9 @@ func TestGetFileInformation(t *testing.T) {
 			id:   testFiles.ProcessedFiles[1].ID,
 			expected: &client.FileInformationHeader{
 				CacheControl:  "max-age=3600",
-				ContentLength: 25860,
+				ContentLength: 17827,
 				ContentType:   "image/jpeg",
-				Etag:          `"1c09c3c99b93297f4ea53174d4fb1f5e493faae59e1a106ace855ae316bf7d79"`,
+				Etag:          `"ae9bc3a8aa740376ec6a6aece16e3af31acbb4748f48d4fc246b783a91458f3c"`,
 				LastModified:  "",
 				Error:         "",
 				StatusCode:    200,
@@ -204,9 +204,9 @@ func TestGetFileInformation(t *testing.T) {
 			id:   testFiles.ProcessedFiles[1].ID,
 			expected: &client.FileInformationHeader{
 				CacheControl:  "max-age=3600",
-				ContentLength: 11306,
+				ContentLength: 10387,
 				ContentType:   "image/jpeg",
-				Etag:          `"1baa94b38cd0cc66cf4289c823c25ff03680d7f9408974bcd6145253e2007dfa"`,
+				Etag:          `"f7da209253132e5a78a3b696da48fd50fb75de6e8476c6c06b629ed794157840"`,
 				LastModified:  "",
 				Error:         "",
 				StatusCode:    200,

client/get_file_test.go

@@ -218,7 +218,7 @@ func TestGetFile(t *testing.T) {
 			name: "get image manipulated, if-match==etag",
 			id:   testFiles.ProcessedFiles[1].ID,
 			opts: []client.GetFileInformationOpt{
-				client.WithIfMatch(`"64ebc77afdaa1c37e1bfe8517903091c440de200c44b771d19c7949157dede85"`),
+				client.WithIfMatch(`"ee282701153963a2b74d0f19ff5b5a30d509a2543a4510e5febbaa2ea4c627ab"`),
 				client.WithImageSize(600, 200),
 				client.WithImageQuality(50),
 				client.WithImageBlur(5),
@@ -227,14 +227,14 @@ func TestGetFile(t *testing.T) {
 				Filename: "nhost.jpg",
 				FileInformationHeader: &client.FileInformationHeader{
 					CacheControl:  "max-age=3600",
-					ContentLength: 12284,
+					ContentLength: 15176,
 					ContentType:   "image/jpeg",
-					Etag:          `"64ebc77afdaa1c37e1bfe8517903091c440de200c44b771d19c7949157dede85"`,
+					Etag:          `"ee282701153963a2b74d0f19ff5b5a30d509a2543a4510e5febbaa2ea4c627ab"`,
 					LastModified:  "Tue, 18 Jan 2022 13:18:04 UTC",
 					StatusCode:    200,
 				},
 			},
-			expectedSha: "64ebc77afdaa1c37e1bfe8517903091c440de200c44b771d19c7949157dede85",
+			expectedSha: "ee282701153963a2b74d0f19ff5b5a30d509a2543a4510e5febbaa2ea4c627ab",
 		},
 		{
 			name: "get image manipulated, if-match!=etag",
@@ -248,9 +248,9 @@ func TestGetFile(t *testing.T) {
 			expected: &client.FileInformationHeaderWithReader{
 				FileInformationHeader: &client.FileInformationHeader{
 					CacheControl:  "max-age=3600",
-					ContentLength: 12284,
+					ContentLength: 15176,
 					ContentType:   "image/jpeg",
-					Etag:          `"64ebc77afdaa1c37e1bfe8517903091c440de200c44b771d19c7949157dede85"`,
+					Etag:          `"ee282701153963a2b74d0f19ff5b5a30d509a2543a4510e5febbaa2ea4c627ab"`,
 					LastModified:  "Tue, 18 Jan 2022 13:18:04 UTC",
 					StatusCode:    412,
 				},

cmd/serve.go

@@ -8,6 +8,7 @@ import (
 	"github.com/aws/aws-sdk-go/aws/credentials"
 	"github.com/gin-gonic/gin"
 	"github.com/nhost/hasura-storage/controller"
+	"github.com/nhost/hasura-storage/image"
 	"github.com/nhost/hasura-storage/metadata"
 	"github.com/nhost/hasura-storage/migrations"
 	"github.com/nhost/hasura-storage/storage"
@@ -69,6 +70,7 @@ func getGin(
 	hasuraAdminSecret string,
 	metadataStorage controller.MetadataStorage,
 	contentStorage controller.ContentStorage,
+	imageTransformer *image.Transformer,
 	trustedProxies []string,
 	logger *logrus.Logger,
 	debug bool,
@@ -77,7 +79,7 @@ func getGin(
 		gin.SetMode(gin.ReleaseMode)
 	}
 
-	ctrl := controller.New(publicURL, hasuraAdminSecret, metadataStorage, contentStorage, logger)
+	ctrl := controller.New(publicURL, hasuraAdminSecret, metadataStorage, contentStorage, imageTransformer, logger)
 
 	return ctrl.SetupRouter(trustedProxies, ginLogger(logger)) // nolint: wrapcheck
 }
@@ -198,6 +200,9 @@ var serveCmd = &cobra.Command{
 			gin.SetMode(gin.ReleaseMode)
 		}
 
+		imageTransformer := image.NewTransformer()
+		defer imageTransformer.Shutdown()
+
 		logger.WithFields(
 			logrus.Fields{
 				"debug":               viper.GetBool(debugFlag),
@@ -240,6 +245,7 @@ var serveCmd = &cobra.Command{
 			viper.GetString(hasuraAdminSecretFlag),
 			metadataStorage,
 			contentStorage,
+			imageTransformer,
 			viper.GetStringSlice(trustedProxiesFlag),
 			logger,
 			viper.GetBool(debugFlag),

controller/controller.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/gin-contrib/cors"
 	"github.com/gin-gonic/gin"
+	"github.com/nhost/hasura-storage/image"
 	"github.com/sirupsen/logrus"
 )
 
@@ -80,6 +81,7 @@ type Controller struct {
 	hasuraAdminSecret string
 	metadataStorage   MetadataStorage
 	contentStorage    ContentStorage
+	imageTransformer  *image.Transformer
 	logger            *logrus.Logger
 }
 
@@ -88,13 +90,15 @@ func New(
 	hasuraAdminSecret string,
 	metadataStorage MetadataStorage,
 	contentStorage ContentStorage,
+	imageTransformer *image.Transformer,
 	logger *logrus.Logger,
 ) *Controller {
 	return &Controller{
 		publicURL,
 		hasuraAdminSecret,
 		metadataStorage,
 		contentStorage,
+		imageTransformer,
 		logger,
 	}
 }

controller/delete_broken_metadata_test.go

@@ -101,7 +101,7 @@ func TestDeleteBrokenMetadata(t *testing.T) {
 				gomock.Any(), "e6aad336-ad79-4df7-a09b-5782f71948f4", gomock.Any(),
 			).Return(controller.FileMetadataWithBucket{}, nil)
 
-			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, logger)
+			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, nil, logger)
 
 			router, _ := ctrl.SetupRouter(nil, ginLogger(logger))
 

controller/delete_file_test.go

@@ -75,7 +75,7 @@ func TestDeleteFile(t *testing.T) {
 				nil,
 			)
 
-			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, logger)
+			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, nil, logger)
 
 			router, _ := ctrl.SetupRouter(nil, ginLogger(logger))
 

controller/delete_orphans_test.go

@@ -75,7 +75,7 @@ func TestDeleteOrphans(t *testing.T) {
 			contentStorage.EXPECT().DeleteFile("default/garbage").Return(nil)
 			contentStorage.EXPECT().DeleteFile("bucket2/7dc0b0d0-b100-4667-89f1-0434942d9c15").Return(nil)
 
-			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, logger)
+			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, nil, logger)
 
 			router, _ := ctrl.SetupRouter(nil, ginLogger(logger))
 

controller/get_file.go

@@ -2,7 +2,6 @@ package controller
 
 import (
 	"bytes"
-	"context"
 	"crypto/sha256"
 	"fmt"
 	"io"
@@ -19,17 +18,30 @@ type GetFileResponse struct {
 	Error *ErrorResponse `json:"error"`
 }
 
-func getQueryInt(ctx *gin.Context, param string) (int, bool, *APIError) {
+func getQueryInt(ctx *gin.Context, param string) (int, *APIError) {
 	s, ok := ctx.GetQuery(param)
 	if !ok {
-		return 0, false, nil
+		return 0, nil
 	}
 	x, err := strconv.Atoi(s)
 	if err != nil {
-		return 0, false, BadDataError(err, fmt.Sprintf("query parameter %s must be an int", param))
+		return 0, BadDataError(err, fmt.Sprintf("query parameter %s must be an int", param))
 	}
 
-	return x, true, nil
+	return x, nil
+}
+
+func getQueryFloat(ctx *gin.Context, param string) (float64, *APIError) {
+	s, ok := ctx.GetQuery(param)
+	if !ok {
+		return 0, nil
+	}
+	x, err := strconv.ParseFloat(s, 32) // nolint: gomnd
+	if err != nil {
+		return 0, BadDataError(err, fmt.Sprintf("query parameter %s must be an int", param))
+	}
+
+	return x, nil
 }
 
 func isImage(mimeType string) bool {
@@ -41,40 +53,34 @@ func isImage(mimeType string) bool {
 	return false
 }
 
-func getImageManipulationOptions(ctx *gin.Context, mimeType string) ([]image.Options, *APIError) { // nolint: cyclop
-	opts := make([]image.Options, 0, 3) // nolint: gomnd
-	// newSizeX, y, q, b
-	newSizeX, okX, err := getQueryInt(ctx, "w")
+func getImageManipulationOptions(ctx *gin.Context, mimeType string) (image.Options, *APIError) {
+	w, err := getQueryInt(ctx, "w")
 	if err != nil {
-		return nil, err
+		return image.Options{}, err
 	}
-	newSizeY, okY, err := getQueryInt(ctx, "h")
+	h, err := getQueryInt(ctx, "h")
 	if err != nil {
-		return nil, err
-	}
-
-	if okX || okY {
-		opts = append(opts, image.WithNewSize(newSizeX, newSizeY))
+		return image.Options{}, err
 	}
 
-	q, ok, err := getQueryInt(ctx, "q")
+	q, err := getQueryInt(ctx, "q")
 	if err != nil {
-		return nil, err
-	}
-	if ok {
-		opts = append(opts, image.WithQuality(q))
+		return image.Options{}, err
 	}
 
-	b, ok, err := getQueryInt(ctx, "b")
+	b, err := getQueryFloat(ctx, "b")
 	if err != nil {
-		return nil, err
-	}
-	if ok {
-		opts = append(opts, image.WithBlur(b))
+		return image.Options{}, err
 	}
 
-	if len(opts) > 0 && !isImage(mimeType) {
-		return nil, BadDataError(
+	opts := image.Options{
+		Height:  h,
+		Width:   w,
+		Blur:    b,
+		Quality: q,
+	}
+	if !opts.IsEmpty() && !isImage(mimeType) {
+		return image.Options{}, BadDataError(
 			fmt.Errorf("image manipulation features are not supported for '%s'", mimeType), // nolint: goerr113
 			fmt.Sprintf("image manipulation features are not supported for '%s'", mimeType),
 		)
@@ -96,12 +102,12 @@ func (p *FakeReadCloserWrapper) Close() error {
 }
 
 func (ctrl *Controller) manipulateImage(
-	ctx context.Context, object io.ReadCloser, opts ...image.Options,
+	object io.ReadCloser, opts image.Options,
 ) (io.ReadCloser, int64, string, *APIError) {
 	defer object.Close()
 
 	buf := &bytes.Buffer{}
-	if err := image.Manipulate(ctx, object, buf, opts...); err != nil {
+	if err := ctrl.imageTransformer.Run(object, buf, opts); err != nil {
 		return nil, 0, "", InternalServerError(err)
 	}
 
@@ -136,8 +142,8 @@ func (ctrl *Controller) processFileToDownload(
 		return nil, apiErr
 	}
 
-	if len(opts) > 0 {
-		object, fileMetadata.Size, fileMetadata.ETag, apiErr = ctrl.manipulateImage(ctx.Request.Context(), object, opts...)
+	if !opts.IsEmpty() {
+		object, fileMetadata.Size, fileMetadata.ETag, apiErr = ctrl.manipulateImage(object, opts)
 		if apiErr != nil {
 			return nil, apiErr
 		}

controller/get_file_information.go

@@ -141,14 +141,14 @@ func (ctrl *Controller) getFileInformationProcess(ctx *gin.Context) (*FileRespon
 		return nil, apiErr
 	}
 
-	if len(opts) > 0 {
+	if !opts.IsEmpty() {
 		object, apiErr := ctrl.contentStorage.GetFile(fileMetadata.ID)
 		if apiErr != nil {
 			return nil, apiErr
 		}
 		defer object.Close()
 
-		object, fileMetadata.Size, fileMetadata.ETag, apiErr = ctrl.manipulateImage(ctx.Request.Context(), object, opts...)
+		object, fileMetadata.Size, fileMetadata.ETag, apiErr = ctrl.manipulateImage(object, opts)
 		if apiErr != nil {
 			return nil, apiErr
 		}

controller/get_file_information_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/nhost/hasura-storage/controller"
 	"github.com/nhost/hasura-storage/controller/mock_controller"
+	"github.com/nhost/hasura-storage/image"
 	"github.com/sirupsen/logrus"
 )
 
@@ -135,7 +136,7 @@ func TestGetFileInfo(t *testing.T) {
 				CacheControl:         "max-age=3600",
 			}, nil)
 
-			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, logger)
+			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, image.NewTransformer(), logger)
 
 			router, _ := ctrl.SetupRouter(nil, ginLogger(logger))
 

controller/get_file_presigned_url_test.go

@@ -90,7 +90,7 @@ func TestGetFilePresignedURL(t *testing.T) {
 					controller.ErrFileNotFound)
 			}
 
-			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, logger)
+			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, nil, logger)
 
 			router, _ := ctrl.SetupRouter(nil, ginLogger(logger))
 

controller/get_file_test.go

@@ -69,7 +69,7 @@ func TestGetFile(t *testing.T) {
 				nil,
 			)
 
-			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, logger)
+			ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, nil, logger)
 
 			router, _ := ctrl.SetupRouter(nil, ginLogger(logger))