diff --git a/changelog/unreleased/change-thumbnails-library.md b/changelog/unreleased/change-thumbnails-library.md new file mode 100644 index 00000000000..7a4920e3927 --- /dev/null +++ b/changelog/unreleased/change-thumbnails-library.md @@ -0,0 +1,6 @@ +Change: replace the library which scales the images + +The library went out out support. +Also did some refactoring of the thumbnails service code. + +https://github.com/owncloud/ocis/pull/910 diff --git a/ocis/go.sum b/ocis/go.sum index e8cbb7ee4cd..e3480462859 100644 --- a/ocis/go.sum +++ b/ocis/go.sum @@ -125,6 +125,10 @@ github.com/aws/aws-sdk-go v1.28.2/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN github.com/aws/aws-sdk-go v1.33.19/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= github.com/aws/aws-sdk-go v1.35.9 h1:b1HiUpdkFLJyoOQ7zas36YHzjNHH0ivHx/G5lWBeg+U= github.com/aws/aws-sdk-go v1.35.9/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= +github.com/aws/aws-sdk-go v1.35.20 h1:Hs7x9Czh+MMPnZLQqHhsuZKeNFA3Vuf7pdy2r5QlVb0= +github.com/aws/aws-sdk-go v1.35.20/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= +github.com/aws/aws-sdk-go v1.35.23 h1:SCP0d0XvyJTDmfnHEQPvBaYi3kea1VNUo7uQmkVgFts= +github.com/aws/aws-sdk-go v1.35.23/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= github.com/aws/aws-xray-sdk-go v0.9.4/go.mod h1:XtMKdBQfpVut+tJEwI7+dJFRxxRdxHDyVNp2tHXRq04= github.com/baiyubin/aliyun-sts-go-sdk v0.0.0-20180326062324-cfa1a18b161f/go.mod h1:AuiFmCCPBSrqvVMvuqFuk0qogytodnVFVSN5CeJB8Gc= github.com/beevik/etree v1.1.0 h1:T0xke/WvNtMoCqgzPhkX2r4rjY3GDZFi+FjpRZY2Jbs= @@ -259,6 +263,7 @@ github.com/cs3org/cato v0.0.0-20200626150132-28a40e643719/go.mod h1:XJEZ3/EQuI3B github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4= github.com/cs3org/go-cs3apis v0.0.0-20191128165347-19746c015c83/go.mod h1:IsVGyZrOLUQD48JIhlM/xb3Vz6He5o2+W0ZTfUGY+IU= github.com/cs3org/go-cs3apis v0.0.0-20200730121022-c4f3d4f7ddfd/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= +github.com/cs3org/go-cs3apis v0.0.0-20200929101248-821df597ec8d/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00 h1:LVl25JaflluOchVvaHWtoCynm5OaM+VNai0IYkcCSe0= github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= github.com/cs3org/reva v0.0.2-0.20200115110931-4c7513415ec5/go.mod h1:Hk3eCcdhtv4eIhKvRK736fQuOyS1HuHnUcz0Dq6NK1A= @@ -399,6 +404,7 @@ github.com/go-playground/universal-translator v0.16.0/go.mod h1:1AnU7NaIRDWWzGEK github.com/go-playground/universal-translator v0.17.0/go.mod h1:UkSxE5sNxxRwHyU+Scu5vgOQjsIJAF8j9muTVoKLVtA= github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= +github.com/go-sql-driver/mysql v1.5.0 h1:ozyZYNQW3x3HtqT1jira07DN2PArx2v7/mN66gGcHOs= github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/go-telegram-bot-api/telegram-bot-api v4.6.4+incompatible/go.mod h1:qf9acutJ8cwBUhm1bqgz6Bei9/C/c93FPDljKWwsOgM= @@ -1050,6 +1056,8 @@ github.com/ory/dockertest v3.3.5+incompatible/go.mod h1:1vX4m9wsvi00u5bseYwXaSnh github.com/ory/fosite v0.29.0/go.mod h1:0atSZmXO7CAcs6NPMI/Qtot8tmZYj04Nddoold4S2h0= github.com/ory/fosite v0.30.2/go.mod h1:Lq9qQ9Sl6mcea2Tt8J7PU+wUeFYPZ+vg7N3zPVKGbN8= github.com/ory/fosite v0.32.2/go.mod h1:UeBhRgW6nAjTcd8S7kAo0IFsY/rTPyOXPq/t8N20Q8I= +github.com/ory/fosite v0.35.0 h1:ugOMHhlcfAjBmIwLOVCK0LQMnYMP/8J/reb2+VEHrfg= +github.com/ory/fosite v0.35.0/go.mod h1:h+ize9gk0GvRyGjabriqSEmTkMhny+O95cijb8DVqPE= github.com/ory/fosite v0.35.1 h1:mGPcwVGwHA7Yy9wr/7LDps6BEXyavL32NxizL9eH53Q= github.com/ory/fosite v0.35.1/go.mod h1:h+ize9gk0GvRyGjabriqSEmTkMhny+O95cijb8DVqPE= github.com/ory/go-acc v0.0.0-20181118080137-ddc355013f90/go.mod h1:sxnvPCxChFuSmTJGj8FdMupeq1BezCiEpDjTUXQ4hf4= @@ -1104,6 +1112,8 @@ github.com/pkg/sftp v1.10.1/go.mod h1:lYOWFsE0bwd1+KfKJaKeuokY15vzFx25BLbzYYoAxZ github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= github.com/pkg/xattr v0.4.1 h1:dhclzL6EqOXNaPDWqoeb9tIxATfBSmjqL0b4DpSjwRw= github.com/pkg/xattr v0.4.1/go.mod h1:W2cGD0TBEus7MkUgv0tNZ9JutLtVO3cXu+IBRuHqnFs= +github.com/pkg/xattr v0.4.2 h1:fbVxr9lvkToTGgPljVszvFsOdcbSv5BmGABneyxRgZM= +github.com/pkg/xattr v0.4.2/go.mod h1:sBD3RAqlr8Q+RC3FutZcikpT8nyDrIEEBw2J744gVWs= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI= @@ -1442,6 +1452,7 @@ golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EH golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= golang.org/x/exp v0.0.0-20200331195152-e8c3332aa8e5/go.mod h1:4M0jN8W1tt0AVLNr8HDosyJCDCDuyL9N9+3m7wDWgKw= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= +golang.org/x/image v0.0.0-20190802002840-cff245a6509b h1:+qEpEAPhDZ1o0x3tHzZTQDArnOixOzGD9HUJfcg0mb4= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/lint v0.0.0-20190301231843-5614ed5bae6f/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= @@ -1615,6 +1626,8 @@ golang.org/x/sys v0.0.0-20200918174421-af09f7315aff/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201022201747-fb209a7c41cd h1:WgqgiQvkiZWz7XLhphjt2GI2GcGCTIZs9jqXMWmH+oc= golang.org/x/sys v0.0.0-20201022201747-fb209a7c41cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201101102859-da207088b7d1 h1:a/mKvvZr9Jcc8oKfcmgzyp7OwF73JPWsQLvH1z2Kxck= +golang.org/x/sys v0.0.0-20201101102859-da207088b7d1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/thumbnails/go.mod b/thumbnails/go.mod index 21bfb4d653d..a5bea1c04cd 100644 --- a/thumbnails/go.mod +++ b/thumbnails/go.mod @@ -8,6 +8,7 @@ require ( contrib.go.opencensus.io/exporter/zipkin v0.1.1 github.com/UnnoTed/fileb0x v1.1.4 github.com/cespare/reflex v0.2.0 + github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00 github.com/go-test/deep v1.0.2-0.20181118220953-042da051cf31 // indirect github.com/gogo/protobuf v1.3.1 // indirect github.com/golang/protobuf v1.4.3 @@ -31,7 +32,9 @@ require ( github.com/stretchr/testify v1.6.1 go.opencensus.io v0.22.5 golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a // indirect + golang.org/x/image v0.0.0-20190802002840-cff245a6509b google.golang.org/genproto v0.0.0-20200918140846-d0d605568037 // indirect + google.golang.org/grpc v1.33.1 gopkg.in/square/go-jose.v2 v2.5.1 ) diff --git a/thumbnails/go.sum b/thumbnails/go.sum index cbea27d5b05..dbb3fa75883 100644 --- a/thumbnails/go.sum +++ b/thumbnails/go.sum @@ -1209,6 +1209,7 @@ golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EH golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= golang.org/x/exp v0.0.0-20200331195152-e8c3332aa8e5/go.mod h1:4M0jN8W1tt0AVLNr8HDosyJCDCDuyL9N9+3m7wDWgKw= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= +golang.org/x/image v0.0.0-20190802002840-cff245a6509b h1:+qEpEAPhDZ1o0x3tHzZTQDArnOixOzGD9HUJfcg0mb4= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/lint v0.0.0-20190301231843-5614ed5bae6f/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/thumbnails/pkg/config/config.go b/thumbnails/pkg/config/config.go index bff257fb291..60b92cf3b3f 100644 --- a/thumbnails/pkg/config/config.go +++ b/thumbnails/pkg/config/config.go @@ -53,6 +53,11 @@ type WebDavSource struct { Insecure bool } +// RevaImageSource defines the available configuration for the reva image source. +type RevaImageSource struct { + GatewayURL string +} + // FileSystemSource defines the available filesystem source configuration. type FileSystemSource struct { BasePath string diff --git a/thumbnails/pkg/service/v0/service.go b/thumbnails/pkg/service/v0/service.go index 0b4194fa1ac..eb8357effe9 100644 --- a/thumbnails/pkg/service/v0/service.go +++ b/thumbnails/pkg/service/v0/service.go @@ -2,6 +2,7 @@ package svc import ( "context" + "image" "gopkg.in/square/go-jose.v2/jwt" @@ -10,7 +11,6 @@ import ( v0proto "github.com/owncloud/ocis/thumbnails/pkg/proto/v0" "github.com/owncloud/ocis/thumbnails/pkg/thumbnail" "github.com/owncloud/ocis/thumbnails/pkg/thumbnail/imgsource" - "github.com/owncloud/ocis/thumbnails/pkg/thumbnail/resolution" "github.com/pkg/errors" ) @@ -18,19 +18,19 @@ import ( func NewService(opts ...Option) v0proto.ThumbnailServiceHandler { options := newOptions(opts...) logger := options.Logger - resolutions, err := resolution.New(options.Config.Thumbnail.Resolutions) + resolutions, err := thumbnail.ParseResolutions(options.Config.Thumbnail.Resolutions) if err != nil { logger.Fatal().Err(err).Msg("resolutions not configured correctly") } svc := Thumbnail{ serviceID: options.Config.Server.Namespace + "." + options.Config.Server.Name, manager: thumbnail.NewSimpleManager( + resolutions, options.ThumbnailStorage, logger, ), - resolutions: resolutions, - source: options.ImageSource, - logger: logger, + source: options.ImageSource, + logger: logger, } return svc @@ -38,11 +38,10 @@ func NewService(opts ...Option) v0proto.ThumbnailServiceHandler { // Thumbnail implements the GRPC handler. type Thumbnail struct { - serviceID string - manager thumbnail.Manager - resolutions resolution.Resolutions - source imgsource.Source - logger log.Logger + serviceID string + manager thumbnail.Manager + source imgsource.Source + logger log.Logger } // GetThumbnail retrieves a thumbnail for an image @@ -52,7 +51,6 @@ func (g Thumbnail) GetThumbnail(ctx context.Context, req *v0proto.GetRequest, rs g.logger.Debug().Str("filetype", req.Filetype.String()).Msg("unsupported filetype") return nil } - r := g.resolutions.ClosestMatch(int(req.Width), int(req.Height)) auth := req.Authorization if auth == "" { @@ -64,8 +62,7 @@ func (g Thumbnail) GetThumbnail(ctx context.Context, req *v0proto.GetRequest, rs } tr := thumbnail.Request{ - Resolution: r, - ImagePath: req.Filepath, + Resolution: image.Rect(0, 0, int(req.Width), int(req.Height)), Encoder: encoder, ETag: req.Etag, Username: username, @@ -78,8 +75,8 @@ func (g Thumbnail) GetThumbnail(ctx context.Context, req *v0proto.GetRequest, rs return nil } - sCtx := imgsource.WithAuthorization(ctx, auth) - img, err := g.source.Get(sCtx, tr.ImagePath) + sCtx := imgsource.ContextSetAuthorization(ctx, auth) + img, err := g.source.Get(sCtx, req.Filepath) if err != nil { return merrors.InternalServerError(g.serviceID, "could not get image from source: %v", err.Error()) } diff --git a/thumbnails/pkg/thumbnail/imgsource/filesystem.go b/thumbnails/pkg/thumbnail/imgsource/filesystem.go index 22710b1cd63..f809ba30b92 100644 --- a/thumbnails/pkg/thumbnail/imgsource/filesystem.go +++ b/thumbnails/pkg/thumbnail/imgsource/filesystem.go @@ -2,12 +2,12 @@ package imgsource import ( "context" - "fmt" "image" "os" "path/filepath" "github.com/owncloud/ocis/thumbnails/pkg/config" + "github.com/pkg/errors" ) // NewFileSystemSource return a new FileSystem instance @@ -27,12 +27,12 @@ func (s FileSystem) Get(ctx context.Context, file string) (image.Image, error) { imgPath := filepath.Join(s.basePath, file) f, err := os.Open(filepath.Clean(imgPath)) if err != nil { - return nil, fmt.Errorf("failed to load the file %s from %s error %s", file, imgPath, err.Error()) + return nil, errors.Wrapf(err, "failed to load the file %s from %s", file, imgPath) } img, _, err := image.Decode(f) if err != nil { - return nil, err + return nil, errors.Wrap(err, "Get: Decode:") } return img, nil diff --git a/thumbnails/pkg/thumbnail/imgsource/imgsource.go b/thumbnails/pkg/thumbnail/imgsource/imgsource.go index 3b4adfd1395..394836a8002 100644 --- a/thumbnails/pkg/thumbnail/imgsource/imgsource.go +++ b/thumbnails/pkg/thumbnail/imgsource/imgsource.go @@ -16,15 +16,16 @@ type Source interface { Get(ctx context.Context, path string) (image.Image, error) } -// WithAuthorization puts the authorization in the context. -func WithAuthorization(parent context.Context, authorization string) context.Context { +// ContextSetAuthorization puts the authorization in the context. +func ContextSetAuthorization(parent context.Context, authorization string) context.Context { return context.WithValue(parent, auth, authorization) } -func authorization(ctx context.Context) string { +// ContextGetAuthorization gets the authorization from the context. +func ContextGetAuthorization(ctx context.Context) (string, bool) { val := ctx.Value(auth) if val == nil { - return "" + return "", false } - return val.(string) + return val.(string), true } diff --git a/thumbnails/pkg/thumbnail/imgsource/webdav.go b/thumbnails/pkg/thumbnail/imgsource/webdav.go index 56503395aac..eedcbc7f7ce 100644 --- a/thumbnails/pkg/thumbnail/imgsource/webdav.go +++ b/thumbnails/pkg/thumbnail/imgsource/webdav.go @@ -10,6 +10,7 @@ import ( "path" "github.com/owncloud/ocis/thumbnails/pkg/config" + "github.com/pkg/errors" ) // NewWebDavSource creates a new webdav instance. @@ -32,13 +33,13 @@ func (s WebDav) Get(ctx context.Context, file string) (image.Image, error) { u.Path = path.Join(u.Path, file) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, fmt.Errorf("could not get the image \"%s\" error: %s", file, err.Error()) + return nil, errors.Wrapf(err, `could not get the image "%s"`, file) } http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: s.insecure} - auth := authorization(ctx) - if auth == "" { + auth, ok := ContextGetAuthorization(ctx) + if !ok { return nil, fmt.Errorf("could not get image \"%s\" error: authorization is missing", file) } req.Header.Add("Authorization", auth) @@ -46,7 +47,7 @@ func (s WebDav) Get(ctx context.Context, file string) (image.Image, error) { client := &http.Client{} resp, err := client.Do(req) if err != nil { - return nil, fmt.Errorf("could not get the image \"%s\" error: %s", file, err.Error()) + return nil, errors.Wrapf(err, `could not get the image "%s"`, file) } if resp.StatusCode != http.StatusOK { @@ -55,7 +56,7 @@ func (s WebDav) Get(ctx context.Context, file string) (image.Image, error) { img, _, err := image.Decode(resp.Body) if err != nil { - return nil, fmt.Errorf("could not decode the image \"%s\". error: %s", file, err.Error()) + return nil, errors.Wrapf(err, `could not decode the image "%s"`, file) } return img, nil } diff --git a/thumbnails/pkg/thumbnail/resolution/resolution.go b/thumbnails/pkg/thumbnail/resolution/resolution.go deleted file mode 100644 index 514fe857446..00000000000 --- a/thumbnails/pkg/thumbnail/resolution/resolution.go +++ /dev/null @@ -1,37 +0,0 @@ -package resolution - -import ( - "fmt" - "strconv" - "strings" -) - -// Parse parses a resolution string in the form x and returns a resolution instance. -func Parse(s string) (Resolution, error) { - parts := strings.Split(s, "x") - if len(parts) != 2 { - return Resolution{}, fmt.Errorf("failed to parse resolution: %s. Expected format x", s) - } - width, err := strconv.Atoi(parts[0]) - if err != nil { - return Resolution{}, fmt.Errorf("width: %s has an invalid value. Expected an integer", parts[0]) - } - height, err := strconv.Atoi(parts[1]) - if err != nil { - return Resolution{}, fmt.Errorf("height: %s has an invalid value. Expected an integer", parts[1]) - } - return Resolution{Width: width, Height: height}, nil -} - -// Resolution defines represents the width and height of a thumbnail. -type Resolution struct { - Width int - Height int -} - -// String returns the resolution in the format: -// -// x -func (r Resolution) String() string { - return strconv.Itoa(r.Width) + "x" + strconv.Itoa(r.Height) -} diff --git a/thumbnails/pkg/thumbnail/resolution/resolution_test.go b/thumbnails/pkg/thumbnail/resolution/resolution_test.go deleted file mode 100644 index b8c3e6c8832..00000000000 --- a/thumbnails/pkg/thumbnail/resolution/resolution_test.go +++ /dev/null @@ -1,40 +0,0 @@ -package resolution - -import "testing" - -func TestParseWithEmptyString(t *testing.T) { - _, err := Parse("") - if err == nil { - t.Error("Parse with empty string should return an error.") - } -} - -func TestParseWithInvalidWidth(t *testing.T) { - _, err := Parse("invalidx42") - if err == nil { - t.Error("Parse with invalid width should return an error.") - } -} - -func TestParseWithInvalidHeight(t *testing.T) { - _, err := Parse("42xinvalid") - if err == nil { - t.Error("Parse with invalid height should return an error.") - } -} - -func TestParse(t *testing.T) { - rStr := "42x23" - r, _ := Parse(rStr) - if r.Width != 42 || r.Height != 23 { - t.Errorf("Expected resolution %s got %s", rStr, r.String()) - } -} - -func TestString(t *testing.T) { - r := Resolution{Width: 42, Height: 23} - expected := "42x23" - if r.String() != expected { - t.Errorf("Expected string %s got %s", expected, r.String()) - } -} diff --git a/thumbnails/pkg/thumbnail/resolution/resolutions.go b/thumbnails/pkg/thumbnail/resolution/resolutions.go deleted file mode 100644 index 2a83d45e7df..00000000000 --- a/thumbnails/pkg/thumbnail/resolution/resolutions.go +++ /dev/null @@ -1,74 +0,0 @@ -package resolution - -import ( - "fmt" - "math" - "sort" -) - -// New creates an instance of Resolutions from resolution strings. -func New(rStrs []string) (Resolutions, error) { - var rs Resolutions - for _, rStr := range rStrs { - r, err := Parse(rStr) - if err != nil { - return nil, fmt.Errorf("failed to initialize resolutions: %s", err.Error()) - } - rs = append(rs, r) - } - sort.Slice(rs, func(i, j int) bool { - left := rs[i] - right := rs[j] - - leftSize := left.Width * left.Height - rightSize := right.Width * right.Height - - return leftSize < rightSize - }) - - return rs, nil -} - -// Resolutions represents the available thumbnail resolutions. -type Resolutions []Resolution - -// ClosestMatch returns the resolution which is closest to the provided resolution. -// If there is no exact match the resolution will be the next higher one. -// If the given resolution is bigger than all available resolutions the biggest available one is used. -func (r Resolutions) ClosestMatch(width, height int) Resolution { - if len(r) == 0 { - return Resolution{Width: width, Height: height} - } - - isLandscape := width > height - givenLen := int(math.Max(float64(width), float64(height))) - - // Initialize with the first resolution - var match Resolution - minDiff := math.MaxInt32 - - for _, current := range r { - len := dimensionLength(current, isLandscape) - diff := givenLen - len - if diff > 0 { - continue - } - absDiff := int(math.Abs(float64(diff))) - if absDiff < minDiff { - minDiff = absDiff - match = current - } - } - - if match == (Resolution{}) { - match = r[len(r)-1] - } - return match -} - -func dimensionLength(r Resolution, landscape bool) int { - if landscape { - return r.Width - } - return r.Height -} diff --git a/thumbnails/pkg/thumbnail/resolution/resolutions_test.go b/thumbnails/pkg/thumbnail/resolution/resolutions_test.go deleted file mode 100644 index 1190cc942aa..00000000000 --- a/thumbnails/pkg/thumbnail/resolution/resolutions_test.go +++ /dev/null @@ -1,111 +0,0 @@ -package resolution - -import ( - "testing" -) - -func TestInitWithEmptyArray(t *testing.T) { - rs, err := New([]string{}) - if err != nil { - t.Errorf("Init with an empty array should not fail. Error: %s.\n", err.Error()) - } - if len(rs) != 0 { - t.Error("Init with an empty array should return an empty Resolutions instance.\n") - } -} - -func TestInitWithNil(t *testing.T) { - rs, err := New(nil) - if err != nil { - t.Errorf("Init with nil parameter should not fail. Error: %s.\n", err.Error()) - } - if len(rs) != 0 { - t.Error("Init with nil parameter should return an empty Resolutions instance.\n") - } -} - -func TestInitWithInvalidValuesInArray(t *testing.T) { - _, err := New([]string{"invalid"}) - if err == nil { - t.Error("Init with invalid parameter should fail.\n") - } -} - -func TestInit(t *testing.T) { - rs, err := New([]string{"16x16"}) - if err != nil { - t.Errorf("Init with valid parameter should not fail. Error: %s.\n", err.Error()) - } - if len(rs) != 1 { - t.Errorf("resolutions has size %d, expected size %d.\n", len(rs), 1) - } -} - -func TestInitWithMultipleResolutions(t *testing.T) { - rStrs := []string{"16x16", "32x32", "64x64", "128x128"} - rs, err := New(rStrs) - if err != nil { - t.Errorf("Init with valid parameter should not fail. Error: %s.\n", err.Error()) - } - if len(rs) != len(rStrs) { - t.Errorf("resolutions has size %d, expected size %d.\n", len(rs), len(rStrs)) - } -} - -func TestInitWithMultipleResolutionsShouldBeSorted(t *testing.T) { - rStrs := []string{"32x32", "64x64", "16x16", "128x128"} - rs, err := New(rStrs) - if err != nil { - t.Errorf("Init with valid parameter should not fail. Error: %s.\n", err.Error()) - } - - for i := 0; i < len(rs)-1; i++ { - current := rs[i] - currentSize := current.Width * current.Height - next := rs[i] - nextSize := next.Width * next.Height - - if currentSize > nextSize { - t.Error("Resolutions are not sorted.") - } - - } -} -func TestClosestMatchWithEmptyResolutions(t *testing.T) { - rs, _ := New(nil) - width := 24 - height := 24 - - r := rs.ClosestMatch(width, height) - if r.Width != width || r.Height != height { - t.Errorf("ClosestMatch from empty resolutions should return the given resolution") - } -} - -func TestClosestMatch(t *testing.T) { - rs, _ := New([]string{"16x16", "24x24", "32x32", "64x64", "128x128"}) - table := [][]int{ - // width, height, expectedWidth, expectedHeight - []int{17, 17, 24, 24}, - []int{12, 17, 24, 24}, - []int{24, 24, 24, 24}, - []int{20, 20, 24, 24}, - []int{20, 80, 128, 128}, - []int{80, 20, 128, 128}, - []int{48, 48, 64, 64}, - []int{1024, 1024, 128, 128}, - } - - for _, row := range table { - width := row[0] - height := row[1] - expectedWidth := row[2] - expectedHeight := row[3] - - match := rs.ClosestMatch(width, height) - - if match.Width != expectedWidth || match.Height != expectedHeight { - t.Errorf("Expected resolution %dx%d got %s", expectedWidth, expectedHeight, match.String()) - } - } -} diff --git a/thumbnails/pkg/thumbnail/resolutions.go b/thumbnails/pkg/thumbnail/resolutions.go new file mode 100644 index 00000000000..19ed6984c6e --- /dev/null +++ b/thumbnails/pkg/thumbnail/resolutions.go @@ -0,0 +1,110 @@ +package thumbnail + +import ( + "fmt" + "image" + "math" + "strconv" + "strings" + + "github.com/pkg/errors" +) + +const ( + _resolutionSeperator = "x" +) + +// ParseResolution returns an image.Rectangle representing the resolution given as a string +func ParseResolution(s string) (image.Rectangle, error) { + parts := strings.Split(s, _resolutionSeperator) + if len(parts) != 2 { + return image.Rectangle{}, fmt.Errorf("failed to parse resolution: %s. Expected format x", s) + } + width, err := strconv.Atoi(parts[0]) + if err != nil { + return image.Rectangle{}, fmt.Errorf("width: %s has an invalid value. Expected an integer", parts[0]) + } + height, err := strconv.Atoi(parts[1]) + if err != nil { + return image.Rectangle{}, fmt.Errorf("height: %s has an invalid value. Expected an integer", parts[1]) + } + return image.Rect(0, 0, width, height), nil +} + +// Resolutions is a list of image.Rectangle representing resolutions. +type Resolutions []image.Rectangle + +// ParseResolutions creates an instance of Resolutions from resolution strings. +func ParseResolutions(strs []string) (Resolutions, error) { + var rs Resolutions + for _, s := range strs { + r, err := ParseResolution(s) + if err != nil { + return nil, errors.Wrap(err, "could not parse resolutions") + } + rs = append(rs, r) + } + return rs, nil +} + +// ClosestMatch returns the resolution which is closest to the provided resolution. +// If there is no exact match the resolution will be the next higher one. +// If the given resolution is bigger than all available resolutions the biggest available one is used. +func (rs Resolutions) ClosestMatch(requested image.Rectangle, sourceSize image.Rectangle) image.Rectangle { + isLandscape := sourceSize.Dx() > sourceSize.Dy() + sourceLen := dimensionLength(sourceSize, isLandscape) + requestedLen := dimensionLength(requested, isLandscape) + isSourceSmaller := sourceLen < requestedLen + + // We don't want to scale images up. + if isSourceSmaller { + return sourceSize + } + + if len(rs) == 0 { + return requested + } + + var match image.Rectangle + // Since we want to search for the smallest difference we start with the highest possible number + minDiff := math.MaxInt32 + + for _, current := range rs { + cLen := dimensionLength(current, isLandscape) + diff := requestedLen - cLen + if diff > 0 { + // current is smaller + continue + } + + // Convert diff to positive value + // Multiplying by -1 is safe since we aren't getting postive numbers here + // because of the check above + absDiff := diff * -1 + if absDiff < minDiff { + minDiff = absDiff + match = current + } + } + + if (match == image.Rectangle{}) { + match = rs[len(rs)-1] + } + return match +} + +func mapRatio(given image.Rectangle, other image.Rectangle) image.Rectangle { + isLandscape := given.Dx() > given.Dy() + ratio := float64(given.Dx()) / float64(given.Dy()) + if isLandscape { + return image.Rect(0, 0, other.Dx(), int(float64(other.Dx())/ratio)) + } + return image.Rect(0, 0, int(float64(other.Dy())*ratio), other.Dy()) +} + +func dimensionLength(rect image.Rectangle, isLandscape bool) int { + if isLandscape { + return rect.Dx() + } + return rect.Dy() +} diff --git a/thumbnails/pkg/thumbnail/resolutions_test.go b/thumbnails/pkg/thumbnail/resolutions_test.go new file mode 100644 index 00000000000..d184fbdc000 --- /dev/null +++ b/thumbnails/pkg/thumbnail/resolutions_test.go @@ -0,0 +1,139 @@ +package thumbnail + +import ( + "image" + "testing" +) + +func TestInitWithEmptyArray(t *testing.T) { + rs, err := ParseResolutions([]string{}) + if err != nil { + t.Errorf("Init with an empty array should not fail. Error: %s.\n", err.Error()) + } + if len(rs) != 0 { + t.Error("Init with an empty array should return an empty Resolutions instance.\n") + } +} + +func TestInitWithNil(t *testing.T) { + rs, err := ParseResolutions(nil) + if err != nil { + t.Errorf("Init with nil parameter should not fail. Error: %s.\n", err.Error()) + } + if len(rs) != 0 { + t.Error("Init with nil parameter should return an empty Resolutions instance.\n") + } +} + +func TestInitWithInvalidValuesInArray(t *testing.T) { + _, err := ParseResolutions([]string{"invalid"}) + if err == nil { + t.Error("Init with invalid parameter should fail.\n") + } +} + +func TestInit(t *testing.T) { + rs, err := ParseResolutions([]string{"16x16"}) + if err != nil { + t.Errorf("Init with valid parameter should not fail. Error: %s.\n", err.Error()) + } + if len(rs) != 1 { + t.Errorf("resolutions has size %d, expected size %d.\n", len(rs), 1) + } +} + +func TestInitWithMultipleResolutions(t *testing.T) { + rStrs := []string{"16x16", "32x32", "64x64", "128x128"} + rs, err := ParseResolutions(rStrs) + if err != nil { + t.Errorf("Init with valid parameter should not fail. Error: %s.\n", err.Error()) + } + if len(rs) != len(rStrs) { + t.Errorf("resolutions has size %d, expected size %d.\n", len(rs), len(rStrs)) + } +} + +func TestClosestMatchWithEmptyResolutions(t *testing.T) { + rs, _ := ParseResolutions(nil) + want := image.Rect(0, 0, 24, 24) + imgSize := image.Rect(0, 0, 24, 24) + + r := rs.ClosestMatch(want, imgSize) + if r.Dx() != want.Dx() || r.Dy() != want.Dy() { + t.Errorf("ClosestMatch from empty resolutions should return the given resolution") + } +} + +func TestClosestMatch(t *testing.T) { + rs, _ := ParseResolutions([]string{"16x16", "24x24", "32x32", "64x64", "128x128"}) + + testData := [][]image.Rectangle{ + {image.Rect(0, 0, 17, 17), image.Rect(0, 0, 1920, 1080), image.Rect(0, 0, 24, 24)}, + {image.Rect(0, 0, 12, 17), image.Rect(0, 0, 1080, 1920), image.Rect(0, 0, 24, 24)}, + {image.Rect(0, 0, 24, 24), image.Rect(0, 0, 1920, 1080), image.Rect(0, 0, 24, 24)}, + {image.Rect(0, 0, 20, 20), image.Rect(0, 0, 1920, 1080), image.Rect(0, 0, 24, 24)}, + {image.Rect(0, 0, 20, 80), image.Rect(0, 0, 1080, 1920), image.Rect(0, 0, 128, 128)}, + {image.Rect(0, 0, 80, 20), image.Rect(0, 0, 1920, 1080), image.Rect(0, 0, 128, 128)}, + {image.Rect(0, 0, 48, 48), image.Rect(0, 0, 1920, 1080), image.Rect(0, 0, 64, 64)}, + {image.Rect(0, 0, 1024, 1024), image.Rect(0, 0, 1920, 1080), image.Rect(0, 0, 128, 128)}, + {image.Rect(0, 0, 1920, 1080), image.Rect(0, 0, 256, 36), image.Rect(0, 0, 256, 36)}, + } + + for _, row := range testData { + given := row[0] + imgSize := row[1] + expected := row[2] + + match := rs.ClosestMatch(given, imgSize) + + if match != expected { + t.Errorf("Expected resolution %dx%d got %dx%d", expected.Dx(), expected.Dy(), match.Dx(), match.Dy()) + } + } +} + +func TestParseWithEmptyString(t *testing.T) { + _, err := ParseResolution("") + if err == nil { + t.Error("Parse with empty string should return an error.") + } +} + +func TestParseWithInvalidWidth(t *testing.T) { + _, err := ParseResolution("invalidx42") + if err == nil { + t.Error("Parse with invalid width should return an error.") + } +} + +func TestParseWithInvalidHeight(t *testing.T) { + _, err := ParseResolution("42xinvalid") + if err == nil { + t.Error("Parse with invalid height should return an error.") + } +} + +func TestParseResolution(t *testing.T) { + rStr := "42x23" + r, _ := ParseResolution(rStr) + if r.Dx() != 42 || r.Dy() != 23 { + t.Errorf("Expected resolution %s got %s", rStr, r.String()) + } +} + +func TestMapRatio(t *testing.T) { + testData := [][]image.Rectangle{ + {image.Rect(0, 0, 1920, 1080), image.Rect(0, 0, 32, 32), image.Rect(0, 0, 32, 18)}, + {image.Rect(0, 0, 1080, 1920), image.Rect(0, 0, 32, 32), image.Rect(0, 0, 18, 32)}, + {image.Rect(0, 0, 1024, 735), image.Rect(0, 0, 32, 32), image.Rect(0, 0, 32, 22)}, + } + for _, row := range testData { + given := row[0] + other := row[1] + expected := row[2] + mapped := mapRatio(given, other) + if mapped.Dx() != expected.Dx() || mapped.Dy() != expected.Dy() { + t.Errorf("Expected %dx%d got %dx%d", expected.Dx(), expected.Dy(), mapped.Dx(), mapped.Dy()) + } + } +} diff --git a/thumbnails/pkg/thumbnail/storage/filesystem.go b/thumbnails/pkg/thumbnail/storage/filesystem.go index bd6063c39d4..4bf2a297a10 100644 --- a/thumbnails/pkg/thumbnail/storage/filesystem.go +++ b/thumbnails/pkg/thumbnail/storage/filesystem.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strconv" "strings" "sync" @@ -70,7 +71,7 @@ func (s *FileSystem) Set(username string, key string, img []byte) error { func (s *FileSystem) BuildKey(r Request) string { etag := r.ETag filetype := r.Types[0] - filename := r.Resolution.String() + "." + filetype + filename := strconv.Itoa(r.Resolution.Dx()) + "x" + strconv.Itoa(r.Resolution.Dy()) + "." + filetype return filepath.Join(etag[:2], etag[2:4], etag[4:], filename) } diff --git a/thumbnails/pkg/thumbnail/storage/storage.go b/thumbnails/pkg/thumbnail/storage/storage.go index 5b17c25de4a..1ec71cc7616 100644 --- a/thumbnails/pkg/thumbnail/storage/storage.go +++ b/thumbnails/pkg/thumbnail/storage/storage.go @@ -1,12 +1,14 @@ package storage -import "github.com/owncloud/ocis/thumbnails/pkg/thumbnail/resolution" +import ( + "image" +) // Request combines different attributes needed for storage operations. type Request struct { ETag string Types []string - Resolution resolution.Resolution + Resolution image.Rectangle } // Storage defines the interface for a thumbnail store. diff --git a/thumbnails/pkg/thumbnail/thumbnails.go b/thumbnails/pkg/thumbnail/thumbnail.go similarity index 71% rename from thumbnails/pkg/thumbnail/thumbnails.go rename to thumbnails/pkg/thumbnail/thumbnail.go index 481e4b5564e..1f12d4f56bf 100644 --- a/thumbnails/pkg/thumbnail/thumbnails.go +++ b/thumbnails/pkg/thumbnail/thumbnail.go @@ -4,16 +4,14 @@ import ( "bytes" "image" - "github.com/nfnt/resize" "github.com/owncloud/ocis/ocis-pkg/log" - "github.com/owncloud/ocis/thumbnails/pkg/thumbnail/resolution" "github.com/owncloud/ocis/thumbnails/pkg/thumbnail/storage" + "golang.org/x/image/draw" ) // Request bundles information needed to generate a thumbnail for afile type Request struct { - Resolution resolution.Resolution - ImagePath string + Resolution image.Rectangle Encoder Encoder ETag string Username string @@ -29,22 +27,25 @@ type Manager interface { } // NewSimpleManager creates a new instance of SimpleManager -func NewSimpleManager(storage storage.Storage, logger log.Logger) SimpleManager { +func NewSimpleManager(resolutions Resolutions, storage storage.Storage, logger log.Logger) SimpleManager { return SimpleManager{ - storage: storage, - logger: logger, + storage: storage, + logger: logger, + resolutions: resolutions, } } // SimpleManager is a simple implementation of Manager type SimpleManager struct { - storage storage.Storage - logger log.Logger + storage storage.Storage + logger log.Logger + resolutions Resolutions } // Get implements the Get Method of Manager func (s SimpleManager) Get(r Request, img image.Image) ([]byte, error) { - thumbnail := s.generate(r, img) + match := s.resolutions.ClosestMatch(r.Resolution, img.Bounds()) + thumbnail := s.generate(match, img) key := s.storage.BuildKey(mapToStorageRequest(r)) @@ -69,8 +70,10 @@ func (s SimpleManager) GetStored(r Request) []byte { return stored } -func (s SimpleManager) generate(r Request, img image.Image) image.Image { - thumbnail := resize.Thumbnail(uint(r.Resolution.Width), uint(r.Resolution.Height), img, resize.Lanczos2) +func (s SimpleManager) generate(r image.Rectangle, img image.Image) image.Image { + targetResolution := mapRatio(img.Bounds(), r) + thumbnail := image.NewRGBA(targetResolution) + draw.ApproxBiLinear.Scale(thumbnail, targetResolution, img, img.Bounds(), draw.Over, nil) return thumbnail } diff --git a/thumbnails/pkg/thumbnail/thumbnail_test.go b/thumbnails/pkg/thumbnail/thumbnail_test.go new file mode 100644 index 00000000000..270630704b4 --- /dev/null +++ b/thumbnails/pkg/thumbnail/thumbnail_test.go @@ -0,0 +1,47 @@ +package thumbnail + +import ( + "image" + "os" + "path/filepath" + "testing" + + "github.com/owncloud/ocis/ocis-pkg/log" + "github.com/owncloud/ocis/thumbnails/pkg/thumbnail/storage" +) + +type NoOpManager struct { + storage.Storage +} + +func (m NoOpManager) BuildKey(r storage.Request) string { + return "" +} + +func (m NoOpManager) Set(username, key string, thumbnail []byte) error { + return nil +} + +func BenchmarkGet(b *testing.B) { + + sut := NewSimpleManager( + Resolutions{}, + NoOpManager{}, + log.NewLogger(), + ) + + res, _ := ParseResolution("32x32") + req := Request{ + Resolution: res, + ETag: "1872ade88f3013edeb33decd74a4f947", + } + cwd, _ := os.Getwd() + p := filepath.Join(cwd, "../../testdata/oc.png") + f, _ := os.Open(p) + defer f.Close() + img, ext, _ := image.Decode(f) + req.Encoder = EncoderForType(ext) + for i := 0; i < b.N; i++ { + sut.Get(req, img) + } +}