From 33dde7d4dd785b92ffcd4eae2029e1825b8362ac Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Fri, 27 Mar 2020 01:53:17 +0100 Subject: [PATCH] move MaxSize, MaxHeight and MaxWidth from image Storage to Service --- backend/app/cmd/server.go | 42 +++++------ backend/app/rest/api/rest.go | 2 +- backend/app/rest/api/rest_private_test.go | 2 +- backend/app/rest/api/rest_test.go | 4 +- backend/app/rest/proxy/image_test.go | 39 ++++++++-- backend/app/store/image/bolt_store.go | 47 ++++--------- backend/app/store/image/bolt_store_test.go | 8 +-- backend/app/store/image/fs_store.go | 27 ++----- backend/app/store/image/fs_store_test.go | 82 ++++------------------ backend/app/store/image/image.go | 48 +++++++++++-- backend/app/store/image/image_mock.go | 43 ++++-------- backend/app/store/image/image_test.go | 67 ++++++++++++++++++ 12 files changed, 216 insertions(+), 195 deletions(-) diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index 034271c3..0345f921 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -567,39 +567,31 @@ func (s *ServerCommand) makeAvatarStore() (avatar.Store, error) { } func (s *ServerCommand) makePicturesStore() (*image.Service, error) { + imageService := &image.Service{ + ImageAPI: s.RemarkURL + "/api/v1/picture/", + TTL: 5 * s.EditDuration, // add extra time to image TTL for staging + MaxSize: s.Image.MaxSize, + MaxHeight: s.Image.ResizeHeight, + MaxWidth: s.Image.ResizeWidth, + } switch s.Image.Type { case "bolt": - boltImageStore, err := image.NewBoltStorage( - s.Image.Bolt.File, - s.Image.MaxSize, - s.Image.ResizeHeight, - s.Image.ResizeWidth, - bolt.Options{}, - ) + boltImageStore, err := image.NewBoltStorage(s.Image.Bolt.File, bolt.Options{}) if err != nil { return nil, err } - return &image.Service{ - Store: boltImageStore, - ImageAPI: s.RemarkURL + "/api/v1/picture/", - TTL: 5 * s.EditDuration, // add extra time to image TTL for staging - }, nil + imageService.Store = boltImageStore + return imageService, nil case "fs": if err := makeDirs(s.Image.FS.Path); err != nil { return nil, err } - return &image.Service{ - Store: &image.FileSystem{ - Location: s.Image.FS.Path, - Staging: s.Image.FS.Staging, - Partitions: s.Image.FS.Partitions, - MaxSize: s.Image.MaxSize, - MaxHeight: s.Image.ResizeHeight, - MaxWidth: s.Image.ResizeWidth, - }, - ImageAPI: s.RemarkURL + "/api/v1/picture/", - TTL: 5 * s.EditDuration, // add extra time to image TTL for staging - }, nil + imageService.Store = &image.FileSystem{ + Location: s.Image.FS.Path, + Staging: s.Image.FS.Staging, + Partitions: s.Image.FS.Partitions, + } + return imageService, nil } return nil, errors.Errorf("unsupported pictures store type %s", s.Image.Type) } @@ -769,7 +761,7 @@ func (s *ServerCommand) makeNotify(dataStore *service.DataStore, authenticator * VerificationSubject: s.Notify.Email.VerificationSubject, UnsubscribeURL: s.RemarkURL + "/email/unsubscribe.html", // TODO: uncomment after #560 frontend part is ready and URL is known - //SubscribeURL: s.RemarkURL + "/subscribe.html?token=", + // SubscribeURL: s.RemarkURL + "/subscribe.html?token=", TokenGenFn: func(userID, email, site string) (string, error) { claims := token.Claims{ Handshake: &token.Handshake{ID: userID + "::" + email}, diff --git a/backend/app/rest/api/rest.go b/backend/app/rest/api/rest.go index c5939729..1d3841b5 100644 --- a/backend/app/rest/api/rest.go +++ b/backend/app/rest/api/rest.go @@ -425,7 +425,7 @@ func (s *Rest) configCtrl(w http.ResponseWriter, r *http.Request) { CriticalScore: s.ScoreThresholds.Critical, PositiveScore: s.DataService.PositiveScore, ReadOnlyAge: s.ReadOnlyAge, - MaxImageSize: s.ImageService.Store.SizeLimit(), + MaxImageSize: s.ImageService.SizeLimit(), EmailNotifications: s.EmailNotifications, EmojiEnabled: s.EmojiEnabled, AnonVote: s.AnonVote, diff --git a/backend/app/rest/api/rest_private_test.go b/backend/app/rest/api/rest_private_test.go index 9ef42a93..fac95f26 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -905,9 +905,9 @@ func TestRest_CreateWithPictures(t *testing.T) { imageService.Store = &image.FileSystem{ Staging: "/tmp/remark42/images.staging", Location: "/tmp/remark42/images", - MaxSize: 2000, } imageService.TTL = 100 * time.Millisecond + imageService.MaxSize = 2000 svc.privRest.imageService = imageService svc.ImageService = imageService diff --git a/backend/app/rest/api/rest_test.go b/backend/app/rest/api/rest_test.go index c7092800..ebf61ae5 100644 --- a/backend/app/rest/api/rest_test.go +++ b/backend/app/rest/api/rest_test.go @@ -375,10 +375,10 @@ func startupT(t *testing.T) (ts *httptest.Server, srv *Rest, teardown func()) { Store: &image.FileSystem{ Location: tmp + "/pics-remark42", Partitions: 100, - MaxSize: 10000, Staging: tmp + "/pics-remark42/staging", }, - TTL: time.Millisecond * 100, + TTL: time.Millisecond * 100, + MaxSize: 10000, }, ImageProxy: &proxy.Image{}, ReadOnlyAge: 10, diff --git a/backend/app/rest/proxy/image_test.go b/backend/app/rest/proxy/image_test.go index 8b1ce808..301acf64 100644 --- a/backend/app/rest/proxy/image_test.go +++ b/backend/app/rest/proxy/image_test.go @@ -19,6 +19,33 @@ import ( "github.com/umputun/remark/backend/app/store/image" ) +// gopher png for test, from https://golang.org/src/image/png/example_test.go +const gopher = "iVBORw0KGgoAAAANSUhEUgAAAEsAAAA8CAAAAAALAhhPAAAFfUlEQVRYw62XeWwUVRzHf2" + + "+OPbo9d7tsWyiyaZti6eWGAhISoIGKECEKCAiJJkYTiUgTMYSIosYYBBIUIxoSPIINEBDi2VhwkQrVsj1ESgu9doHWdrul7ba" + + "73WNm3vOPtsseM9MdwvvrzTs+8/t95ze/33sI5BqiabU6m9En8oNjduLnAEDLUsQXFF8tQ5oxK3vmnNmDSMtrncks9Hhtt" + + "/qeWZapHb1ha3UqYSWVl2ZmpWgaXMXGohQAvmeop3bjTRtv6SgaK/Pb9/bFzUrYslbFAmHPp+3WhAYdr+7GN/YnpN46Opv55VDs" + + "JkoEpMrY/vO2BIYQ6LLvm0ThY3MzDzzeSJeeWNyTkgnIE5ePKsvKlcg/0T9QMzXalwXMlj54z4c0rh/mzEfr+FgWEz2w6uk" + + "8dkzFAgcARAgNp1ZYef8bH2AgvuStbc2/i6CiWGj98y2tw2l4FAXKkQBIf+exyRnteY83LfEwDQAYCoK+P6bxkZm/0966LxcAA" + + "ILHB56kgD95PPxltuYcMtFTWw/FKkY/6Opf3GGd9ZF+Qp6mzJxzuRSractOmJrH1u8XTvWFHINNkLQLMR+XHXvfPPHw967raE1xxwtA36I" + + "MRfkAAG29/7mLuQcb2WOnsJReZGfpiHsSBX81cvMKywYZHhX5hFPtOqPGWZCXnhWGAu6lX91ElKXSalcLXu3UaOXVay57ZSe5f6Gpx7J2" + + "MXAsi7EqSp09b/MirKSyJfnfEEgeDjl8FgDAfvewP03zZ+AJ0m9aFRM8eEHBDRKjfcreDXnZdQuAxXpT2NRJ7xl3UkLBhuVGU16gZiGOgZm" + + "rSbRdqkILuL/yYoSXHHkl9KXgqNu3PB8oRg0geC5vFmLjad6mUyTKLmF3OtraWDIfACyXqmephaDABawfpi6tqqBZytfQMqOz6S09iWXhkt" + + "rRaB8Xz4Yi/8gyABDm5NVe6qq/3VzPrcjELWrebVuyY2T7ar4zQyybUCtsQ5Es1FGaZVrRVQwAgHGW2ZCRZshI5bGQi7HesyE972pOSeMM0" + + "dSktlzxRdrlqb3Osa6CCS8IJoQQQgBAbTAa5l5epO34rJszibJI8rxLfGzcp1dRosutGeb2VDNgqYrwTiPNsLxXiPi3dz7LiS1WBRBDBOnqEj" + + "yy3aQb+/bLiJzz9dIkscVBBLxMfSEac7kO4Fpkngi0ruNBeSOal+u8jgOuqPz12nryMLCniEjtOOOmpt+KEIqsEdocJjYXwrh9OZqWJQyPCTo67" + + "LNS/TdxLAv6R5ZNK9npEjbYdT33gRo4o5oTqR34R+OmaSzDBWsAIPhuRcgyoteNi9gF0KzNYWVItPf2TLoXEg+7isNC7uJkgo1iQWOfRSP9NR" + + "11RtbZZ3OMG/VhL6jvx+J1m87+RCfJChAtEBQkSBX2PnSiihc/Twh3j0h7qdYQAoRVsRGmq7HU2QRbaxVGa1D6nIOqaIWRjyRZpHMQKWKpZM5fe" + + "A+lzC4ZFultV8S6T0mzQGhQohi5I8iw+CsqBSxhFMuwyLgSwbghGb0AiIKkSDmGZVmJSiKihsiyOAUs70UkywooYP0bii9GdH4sfr1UNysd3fU" + + "yLLMQN+rsmo3grHl9VNJHbbwxoa47Vw5gupIqrZcjPh9R4Nye3nRDk199V+aetmvVtDRE8/+cbgAAgMIWGb3UA0MGLE9SCbWX670TDy" + + "1y98c3D27eppUjsZ6fql3jcd5rUe7+ZIlLNQny3Rd+E5Tct3WVhTM5RBCEdiEK0b6B+/ca2gYU393nFj/n1AygRQxPIUA043M42u85+z2S" + + "nssKrPl8Mx76NL3E6eXc3be7OD+H4WHbJkKI8AU8irbITQjZ+0hQcPEgId/Fn/pl9crKH02+5o2b9T/eMx7pKoskYgAAAABJRU5ErkJggg==" + +func gopherPNG() io.Reader { return base64.NewDecoder(base64.StdEncoding, strings.NewReader(gopher)) } +func gopherPNGBytes() []byte { + img, _ := ioutil.ReadAll(gopherPNG()) + return img +} + func TestImage_Extract(t *testing.T) { tbl := []struct { @@ -81,7 +108,7 @@ func TestImage_Routes(t *testing.T) { resp, err := http.Get(ts.URL + "/?src=" + encodedImgURL) require.NoError(t, err) assert.Equal(t, 200, resp.StatusCode) - assert.Equal(t, "123", resp.Header["Content-Length"][0]) + assert.Equal(t, "1462", resp.Header["Content-Length"][0]) assert.Equal(t, "image/*", resp.Header["Content-Type"][0]) encodedImgURL = base64.URLEncoding.EncodeToString([]byte(httpSrv.URL + "/image/no-such-image.png")) @@ -101,7 +128,7 @@ func TestImage_RoutesCachingImage(t *testing.T) { CacheExternal: true, RemarkURL: "https://demo.remark42.com", RoutePath: "/api/v1/proxy", - ImageService: &image.Service{Store: &imageStore}, + ImageService: &image.Service{Store: &imageStore, MaxSize: 1500}, } ts := httptest.NewServer(http.HandlerFunc(img.Handler)) @@ -113,13 +140,13 @@ func TestImage_RoutesCachingImage(t *testing.T) { encodedImgURL := base64.URLEncoding.EncodeToString([]byte(imgURL)) imageStore.On("Load", mock.Anything).Once().Return(nil, nil) - imageStore.On("SaveWithID", mock.Anything, mock.Anything).Once().Run(func(args mock.Arguments) { _, _ = ioutil.ReadAll(args.Get(1).(io.Reader)) }).Return("", nil) + imageStore.On("SaveWithID", mock.Anything, mock.Anything).Once().Return("", nil) imageStore.On("Commit", mock.Anything).Once().Return(nil) resp, err := http.Get(ts.URL + "/?src=" + encodedImgURL) require.Nil(t, err) assert.Equal(t, 200, resp.StatusCode) - assert.Equal(t, "123", resp.Header["Content-Length"][0]) + assert.Equal(t, "1462", resp.Header["Content-Length"][0]) assert.Equal(t, "image/*", resp.Header["Content-Type"][0]) imageStore.AssertCalled(t, "Load", mock.Anything) @@ -216,9 +243,9 @@ func imgHTTPTestsServer(t *testing.T) *httptest.Server { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/image/img1.png" { t.Log("http img request", r.URL) - w.Header().Add("Content-Length", "123") + w.Header().Add("Content-Length", "1462") w.Header().Add("Content-Type", "image/png") - _, err := w.Write([]byte(fmt.Sprintf("%123s", "X"))) + _, err := w.Write(gopherPNGBytes()) assert.NoError(t, err) return } diff --git a/backend/app/store/image/bolt_store.go b/backend/app/store/image/bolt_store.go index 209907fd..5767f724 100644 --- a/backend/app/store/image/bolt_store.go +++ b/backend/app/store/image/bolt_store.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/binary" - "io" "path" "time" @@ -21,15 +20,12 @@ const insertTimeBktName = "insertTimestamps" // It uses 3 buckets to manage images data. // Two buckets contains image data (staged and committed images). Third bucket holds insertion timestamps. type Bolt struct { - fileName string - db *bolt.DB - MaxSize int - MaxHeight int - MaxWidth int + fileName string + db *bolt.DB } // NewBoltStorage create bolt image store -func NewBoltStorage(fileName string, maxSize int, maxHeight int, maxWidth int, options bolt.Options) (*Bolt, error) { +func NewBoltStorage(fileName string, options bolt.Options) (*Bolt, error) { db, err := bolt.Open(fileName, 0600, &options) if err != nil { return nil, errors.Wrapf(err, "failed to make boltdb for %s", fileName) @@ -51,44 +47,34 @@ func NewBoltStorage(fileName string, maxSize int, maxHeight int, maxWidth int, o return nil, errors.Wrapf(err, "failed to initialize boltdb db %q buckets", fileName) } return &Bolt{ - db: db, - fileName: fileName, - MaxSize: maxSize, - MaxHeight: maxHeight, - MaxWidth: maxWidth, + db: db, + fileName: fileName, }, nil } // SaveWithID saves data from a reader, for given id -func (b *Bolt) SaveWithID(id string, r io.Reader) (string, error) { - data, err := readAndValidateImage(r, b.MaxSize) - if err != nil { - return "", errors.Wrapf(err, "can't load image with ID %s", id) - } - - data = resize(data, b.MaxWidth, b.MaxHeight) - - err = b.db.Update(func(tx *bolt.Tx) error { - if err = tx.Bucket([]byte(imagesStagedBktName)).Put([]byte(id), data); err != nil { +func (b *Bolt) SaveWithID(id string, img []byte) (string, error) { + err := b.db.Update(func(tx *bolt.Tx) error { + if err := tx.Bucket([]byte(imagesStagedBktName)).Put([]byte(id), img); err != nil { return errors.Wrapf(err, "can't put to bucket with %s", id) } tsBuf := &bytes.Buffer{} - if err = binary.Write(tsBuf, binary.LittleEndian, time.Now().UnixNano()); err != nil { + if err := binary.Write(tsBuf, binary.LittleEndian, time.Now().UnixNano()); err != nil { return errors.Wrapf(err, "can't serialize timestamp for %s", id) } - if err = tx.Bucket([]byte(insertTimeBktName)).Put([]byte(id), tsBuf.Bytes()); err != nil { + if err := tx.Bucket([]byte(insertTimeBktName)).Put([]byte(id), tsBuf.Bytes()); err != nil { return errors.Wrapf(err, "can't put to bucket with %s", id) } - return err + return nil }) return id, err } // Save data from reader to staging bucket in DB -func (b *Bolt) Save(userID string, r io.Reader) (id string, err error) { +func (b *Bolt) Save(userID string, img []byte) (id string, err error) { id = path.Join(userID, guid()) - return b.SaveWithID(id, r) + return b.SaveWithID(id, img) } // Commit file stored in staging bucket by copying it to permanent bucket @@ -130,7 +116,7 @@ func (b *Bolt) Cleanup(_ context.Context, ttl time.Duration) error { err := b.db.Update(func(tx *bolt.Tx) error { c := tx.Bucket([]byte(insertTimeBktName)).Cursor() - idsToRemove := [][]byte{} + var idsToRemove [][]byte for id, tsData := c.First(); id != nil; id, tsData = c.Next() { var ts int64 @@ -161,8 +147,3 @@ func (b *Bolt) Cleanup(_ context.Context, ttl time.Duration) error { }) return err } - -// SizeLimit returns max size of allowed image -func (b *Bolt) SizeLimit() int { - return b.MaxSize -} diff --git a/backend/app/store/image/bolt_store_test.go b/backend/app/store/image/bolt_store_test.go index 5f1a8210..e7bd7cd3 100644 --- a/backend/app/store/image/bolt_store_test.go +++ b/backend/app/store/image/bolt_store_test.go @@ -18,7 +18,7 @@ func TestBoltStore_SaveCommit(t *testing.T) { svc, teardown := prepareBoltImageStorageTest(t) defer teardown() - id, err := svc.Save("user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNGBytes()) assert.NoError(t, err) assert.Contains(t, id, "user1") t.Log(id) @@ -48,7 +48,7 @@ func TestBoltStore_LoadAfterSave(t *testing.T) { svc, teardown := prepareBoltImageStorageTest(t) defer teardown() - id, err := svc.Save("user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNGBytes()) assert.NoError(t, err) assert.Contains(t, id, "user1") t.Log(id) @@ -66,7 +66,7 @@ func TestBoltStore_Cleanup(t *testing.T) { defer teardown() save := func(file string, user string) (id string) { - id, err := svc.Save(user, gopherPNG()) + id, err := svc.Save(user, gopherPNGBytes()) require.NoError(t, err) checkBoltImgData(t, svc.db, imagesStagedBktName, id, func(data []byte) error { @@ -133,7 +133,7 @@ func prepareBoltImageStorageTest(t *testing.T) (svc *Bolt, teardown func()) { loc, err := ioutil.TempDir("", "test_image_r42") require.NoError(t, err, "failed to make temp dir") - svc, err = NewBoltStorage(path.Join(loc, "picture.db"), 1500, 0, 0, bolt.Options{}) + svc, err = NewBoltStorage(path.Join(loc, "picture.db"), bolt.Options{}) assert.NoError(t, err, "new bolt storage") teardown = func() { diff --git a/backend/app/store/image/fs_store.go b/backend/app/store/image/fs_store.go index 878ee892..a7177f00 100644 --- a/backend/app/store/image/fs_store.go +++ b/backend/app/store/image/fs_store.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "hash/crc64" - "io" "io/ioutil" "math" "os" @@ -23,10 +22,7 @@ import ( type FileSystem struct { Location string Staging string - MaxSize int Partitions int - MaxHeight int - MaxWidth int crc struct { *crc64.Table @@ -37,32 +33,26 @@ type FileSystem struct { } // SaveWithID saves data from a reader, with given id -func (f *FileSystem) SaveWithID(id string, r io.Reader) (string, error) { - data, err := readAndValidateImage(r, f.MaxSize) - if err != nil { - return "", errors.Wrapf(err, "can't load image with ID %s", id) - } - - data = resize(data, f.MaxWidth, f.MaxHeight) +func (f *FileSystem) SaveWithID(id string, img []byte) (string, error) { dst := f.location(f.Staging, id) - if err = os.MkdirAll(path.Dir(dst), 0700); err != nil { + if err := os.MkdirAll(path.Dir(dst), 0700); err != nil { return "", errors.Wrap(err, "can't make image directory") } - if err = ioutil.WriteFile(dst, data, 0600); err != nil { + if err := ioutil.WriteFile(dst, img, 0600); err != nil { return "", errors.Wrapf(err, "can't write image file with id %s", id) } - log.Printf("[DEBUG] file %s saved for image %s, size=%d", dst, id, len(data)) + log.Printf("[DEBUG] file %s saved for image %s, size=%d", dst, id, len(img)) return id, nil } // Save data from a reader to local FS, staging directory. Returns id as user/uuid // Files partitioned across multiple subdirectories, and the final path includes part, i.e. /location/user1/03/123-4567 -func (f *FileSystem) Save(userID string, r io.Reader) (id string, err error) { +func (f *FileSystem) Save(userID string, img []byte) (id string, err error) { tempId := path.Join(userID, guid()) // make id as user/uuid - return f.SaveWithID(tempId, r) + return f.SaveWithID(tempId, img) } // Commit file stored in staging location by moving it to permanent location @@ -131,11 +121,6 @@ func (f *FileSystem) Cleanup(_ context.Context, ttl time.Duration) error { return errors.Wrap(err, "failed to cleanup images") } -// SizeLimit returns max size of allowed image -func (f *FileSystem) SizeLimit() int { - return f.MaxSize -} - // location gets full path for id by adding partition to the final path in order to keep files in different subdirectories // and avoid too many files in a single place. // the end result is a full path like this - /tmp/images/user1/92/xxx-yyy.png. diff --git a/backend/app/store/image/fs_store_test.go b/backend/app/store/image/fs_store_test.go index 8b7908bc..4c24d23e 100644 --- a/backend/app/store/image/fs_store_test.go +++ b/backend/app/store/image/fs_store_test.go @@ -39,12 +39,16 @@ const gopher = "iVBORw0KGgoAAAANSUhEUgAAAEsAAAA8CAAAAAALAhhPAAAFfUlEQVRYw62XeWwU "nssKrPl8Mx76NL3E6eXc3be7OD+H4WHbJkKI8AU8irbITQjZ+0hQcPEgId/Fn/pl9crKH02+5o2b9T/eMx7pKoskYgAAAABJRU5ErkJggg==" func gopherPNG() io.Reader { return base64.NewDecoder(base64.StdEncoding, strings.NewReader(gopher)) } +func gopherPNGBytes() []byte { + img, _ := ioutil.ReadAll(gopherPNG()) + return img +} func TestFsStore_Save(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - id, err := svc.Save("user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNGBytes()) assert.NoError(t, err) assert.Contains(t, id, "user1/") t.Log(id) @@ -56,78 +60,32 @@ func TestFsStore_Save(t *testing.T) { assert.Equal(t, 1462, len(data)) } -func TestFsStore_SaveWithResize(t *testing.T) { - svc, teardown := prepareImageTest(t) - defer teardown() - svc.MaxWidth, svc.MaxHeight = 32, 32 - - id, err := svc.Save("user1", gopherPNG()) - assert.NoError(t, err) - assert.Contains(t, id, "user1/") - t.Log(id) - - img := svc.location(svc.Staging, id) - t.Log(img) - data, err := ioutil.ReadFile(img) - assert.NoError(t, err) - assert.Equal(t, 1135, len(data)) -} - -func TestFsStore_SaveWithResizeJpeg(t *testing.T) { - svc, teardown := prepareImageTest(t) - defer teardown() - svc.MaxWidth, svc.MaxHeight = 400, 300 - svc.MaxSize = 32000 - - fh, err := os.Open("testdata/circles.jpg") - defer func() { assert.NoError(t, fh.Close()) }() - assert.NoError(t, err) - id, err := svc.Save("user1", fh) - assert.NoError(t, err) - assert.Contains(t, id, "user1/") - t.Log(id) - - img := svc.location(svc.Staging, id) - t.Log(img) - data, err := ioutil.ReadFile(img) - assert.NoError(t, err) - assert.Equal(t, 10918, len(data)) -} - func TestFsStore_SaveNoResizeJpeg(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - svc.MaxWidth, svc.MaxHeight = 1400, 1300 - svc.MaxSize = 32000 fh, err := os.Open("testdata/circles.jpg") defer func() { assert.NoError(t, fh.Close()) }() assert.NoError(t, err) - id, err := svc.Save("user1", fh) + img, err := ioutil.ReadAll(fh) + assert.NoError(t, err) + id, err := svc.Save("user1", img) assert.NoError(t, err) assert.Contains(t, id, "user1/") t.Log(id) - img := svc.location(svc.Staging, id) - t.Log(img) - data, err := ioutil.ReadFile(img) + imgPath := svc.location(svc.Staging, id) + t.Log(imgPath) + data, err := ioutil.ReadFile(imgPath) assert.NoError(t, err) assert.Equal(t, 23983, len(data)) } -func TestFsStore_WrongFormat(t *testing.T) { - svc, teardown := prepareImageTest(t) - defer teardown() - - _, err := svc.Save("user1", strings.NewReader("blah blah bad image")) - assert.Error(t, err) -} - func TestFsStore_SaveAndCommit(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - id, err := svc.Save("user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNGBytes()) require.NoError(t, err) err = svc.Commit(id) require.NoError(t, err) @@ -143,21 +101,12 @@ func TestFsStore_SaveAndCommit(t *testing.T) { assert.Equal(t, 1462, len(data)) } -func TestFsStore_SaveTooLarge(t *testing.T) { - svc, teardown := prepareImageTest(t) - defer teardown() - svc.MaxSize = 2000 - _, err := svc.Save("user2", io.MultiReader(gopherPNG(), gopherPNG())) - assert.Error(t, err) - assert.Contains(t, err.Error(), "is too large") -} - func TestFsStore_LoadAfterSave(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - id, err := svc.Save("user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNGBytes()) assert.NoError(t, err) t.Log(id) @@ -173,7 +122,7 @@ func TestFsStore_LoadAfterCommit(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - id, err := svc.Save("user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNGBytes()) assert.NoError(t, err) t.Log(id) err = svc.Commit(id) @@ -235,7 +184,7 @@ func TestFsStore_Cleanup(t *testing.T) { defer teardown() save := func(file string, user string) (path string) { - id, err := svc.Save(user, gopherPNG()) + id, err := svc.Save(user, gopherPNGBytes()) require.NoError(t, err) img := svc.location(svc.Staging, id) data, err := ioutil.ReadFile(img) @@ -292,7 +241,6 @@ func prepareImageTest(t *testing.T) (svc *FileSystem, teardown func()) { Location: loc, Staging: staging, Partitions: 100, - MaxSize: 1500, } teardown = func() { diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index 7cb2e7ae..61a0fa26 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -32,8 +32,11 @@ import ( // Submitted ids committed (i.e. moved from staging to final) on TTL expiration. type Service struct { Store - TTL time.Duration // for how long file allowed on staging - ImageAPI string // image api matching path + TTL time.Duration // for how long file allowed on staging + ImageAPI string // image api matching path + MaxSize int + MaxHeight int + MaxWidth int wg sync.WaitGroup submitCh chan submitReq @@ -47,10 +50,9 @@ type Service struct { // Store defines interface for saving and loading pictures. // Declares two-stage save with Commit. Save stores to staging area and Commit moves to the final location type Store interface { - Save(userID string, r io.Reader) (id string, err error) // get name and reader and returns ID of stored (staging) image - SaveWithID(id string, r io.Reader) (string, error) // store image for passed id to staging - Load(id string) ([]byte, error) // load image by ID. Caller has to close the reader. - SizeLimit() int // max image size + Save(userID string, img []byte) (id string, err error) // get name and reader and returns ID of stored (staging) image + SaveWithID(id string, img []byte) (string, error) // store image for passed id to staging + Load(id string) ([]byte, error) // load image by ID. Caller has to close the reader. Commit(id string) error // move image from staging to permanent Cleanup(ctx context.Context, ttl time.Duration) error // run removal loop for old images on staging @@ -151,6 +153,40 @@ func (s *Service) Close() { s.wg.Wait() } +// Save wraps storage Save function, validating and resizing the image before calling it. +func (s *Service) Save(userID string, r io.Reader) (id string, err error) { + img, err := s.prepareImage(r) + if err != nil { + return "", err + } + return s.Store.Save(userID, img) +} + +// SaveWithID wraps storage SaveWithID function, validating and resizing the image before calling it. +func (s *Service) SaveWithID(id string, r io.Reader) (string, error) { + img, err := s.prepareImage(r) + if err != nil { + return "", err + } + return s.Store.SaveWithID(id, img) +} + +// SizeLimit returns max size of allowed image +func (s *Service) SizeLimit() int { + return s.MaxSize +} + +// prepareImage calls readAndValidateImage and resize on provided image. +func (s *Service) prepareImage(r io.Reader) ([]byte, error) { + data, err := readAndValidateImage(r, s.MaxSize) + if err != nil { + return nil, errors.Wrapf(err, "can't load image") + } + + data = resize(data, s.MaxWidth, s.MaxHeight) + return data, nil +} + // resize an image of supported format (PNG, JPG, GIF) to the size of "limit" px of // the biggest side (width or height) preserving aspect ratio. // Returns original data if resizing is not needed or failed. diff --git a/backend/app/store/image/image_mock.go b/backend/app/store/image/image_mock.go index 9c3090fd..e32fc262 100644 --- a/backend/app/store/image/image_mock.go +++ b/backend/app/store/image/image_mock.go @@ -3,7 +3,6 @@ package image import context "context" -import io "io" import mock "github.com/stretchr/testify/mock" import time "time" @@ -63,20 +62,20 @@ func (_m *MockStore) Load(id string) ([]byte, error) { return r0, r1 } -// Save provides a mock function with given fields: userID, r -func (_m *MockStore) Save(userID string, r io.Reader) (string, error) { - ret := _m.Called(userID, r) +// Save provides a mock function with given fields: userID, img +func (_m *MockStore) Save(userID string, img []byte) (string, error) { + ret := _m.Called(userID, img) var r0 string - if rf, ok := ret.Get(0).(func(string, io.Reader) string); ok { - r0 = rf(userID, r) + if rf, ok := ret.Get(0).(func(string, []byte) string); ok { + r0 = rf(userID, img) } else { r0 = ret.Get(0).(string) } var r1 error - if rf, ok := ret.Get(1).(func(string, io.Reader) error); ok { - r1 = rf(userID, r) + if rf, ok := ret.Get(1).(func(string, []byte) error); ok { + r1 = rf(userID, img) } else { r1 = ret.Error(1) } @@ -84,37 +83,23 @@ func (_m *MockStore) Save(userID string, r io.Reader) (string, error) { return r0, r1 } -// SaveWithID provides a mock function with given fields: id, r -func (_m *MockStore) SaveWithID(id string, r io.Reader) (string, error) { - ret := _m.Called(id, r) +// SaveWithID provides a mock function with given fields: id, img +func (_m *MockStore) SaveWithID(id string, img []byte) (string, error) { + ret := _m.Called(id, img) var r0 string - if rf, ok := ret.Get(0).(func(string, io.Reader) string); ok { - r0 = rf(id, r) + if rf, ok := ret.Get(0).(func(string, []byte) string); ok { + r0 = rf(id, img) } else { r0 = ret.Get(0).(string) } var r1 error - if rf, ok := ret.Get(1).(func(string, io.Reader) error); ok { - r1 = rf(id, r) + if rf, ok := ret.Get(1).(func(string, []byte) error); ok { + r1 = rf(id, img) } else { r1 = ret.Error(1) } return r0, r1 } - -// SizeLimit provides a mock function with given fields: -func (_m *MockStore) SizeLimit() int { - ret := _m.Called() - - var r0 int - if rf, ok := ret.Get(0).(func() int); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(int) - } - - return r0 -} diff --git a/backend/app/store/image/image_test.go b/backend/app/store/image/image_test.go index 5d55789c..7f2a4306 100644 --- a/backend/app/store/image/image_test.go +++ b/backend/app/store/image/image_test.go @@ -4,8 +4,11 @@ import ( "bytes" "context" "image" + "io" "io/ioutil" + "os" "strconv" + "strings" "testing" "time" @@ -14,6 +17,70 @@ import ( "github.com/stretchr/testify/require" ) +func TestService_Save(t *testing.T) { + store := MockStore{} + svc := Service{Store: &store} + svc.MaxSize = 1500 + svc.MaxWidth, svc.MaxHeight = 32, 32 + + store.On("Save", "user1", mock.Anything).Return("user1/test_id", nil) + id, err := svc.Save("user1", gopherPNG()) + assert.NoError(t, err) + assert.Equal(t, "user1/test_id", id) + + store.On("SaveWithID", "test_id", mock.Anything).Return("test_id", nil) + id, err = svc.SaveWithID("test_id", gopherPNG()) + assert.NoError(t, err) + assert.Equal(t, "test_id", id) +} + +func TestService_Resize(t *testing.T) { + img, err := readAndValidateImage(gopherPNG(), 1500) + assert.NoError(t, err) + assert.Equal(t, 1462, len(img)) + + img = resize(img, 32, 32) + assert.Equal(t, 1135, len(img)) +} + +func TestService_ResizeJpeg(t *testing.T) { + fh, err := os.Open("testdata/circles.jpg") + defer func() { assert.NoError(t, fh.Close()) }() + assert.NoError(t, err) + + img, err := readAndValidateImage(fh, 32000) + assert.NoError(t, err) + assert.Equal(t, 23983, len(img)) + + img = resize(img, 400, 300) + assert.Equal(t, 10918, len(img)) +} + +func TestService_SaveTooLarge(t *testing.T) { + svc := Service{ImageAPI: "/blah/"} + svc.MaxSize = 2000 + _, err := svc.Save("user2", io.MultiReader(gopherPNG(), gopherPNG())) + assert.Error(t, err) + assert.Contains(t, err.Error(), "is too large") + _, err = svc.SaveWithID("test_id", io.MultiReader(gopherPNG(), gopherPNG())) + assert.Error(t, err) + assert.Contains(t, err.Error(), "is too large") +} + +func TestService_WrongFormat(t *testing.T) { + svc := Service{ImageAPI: "/blah/"} + + _, err := svc.Save("user1", strings.NewReader("blah blah bad image")) + assert.Error(t, err) +} + +func TestService_SizeLimit(t *testing.T) { + svc := Service{MaxSize: 666} + + size := svc.SizeLimit() + assert.Equal(t, 666, size) +} + func TestService_ExtractPictures(t *testing.T) { svc := Service{ImageAPI: "/blah/"} html := `blah foo