From fa4e779fa0d196c06f830a0a2b29425b2f991991 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Thu, 26 Mar 2020 23:02:08 +0100 Subject: [PATCH 1/6] bump go modules in memory_store --- backend/_example/memory_store/go.mod | 8 ++++---- backend/_example/memory_store/go.sum | 8 ++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/backend/_example/memory_store/go.mod b/backend/_example/memory_store/go.mod index 45046f26..466edd4d 100644 --- a/backend/_example/memory_store/go.mod +++ b/backend/_example/memory_store/go.mod @@ -4,11 +4,11 @@ go 1.12 require ( github.com/go-pkgz/jrpc v0.1.0 - github.com/go-pkgz/lgr v0.6.3 + github.com/go-pkgz/lgr v0.7.0 github.com/jessevdk/go-flags v1.4.0 - github.com/pkg/errors v0.8.1 - github.com/stretchr/testify v1.4.0 - github.com/umputun/remark/backend v1.4.0 + github.com/pkg/errors v0.9.1 + github.com/stretchr/testify v1.5.1 + github.com/umputun/remark/backend v1.5.0 ) replace github.com/umputun/remark/backend => ../../ diff --git a/backend/_example/memory_store/go.sum b/backend/_example/memory_store/go.sum index 1c2a3f90..ff3a466d 100644 --- a/backend/_example/memory_store/go.sum +++ b/backend/_example/memory_store/go.sum @@ -71,6 +71,8 @@ github.com/go-pkgz/jrpc v0.1.0/go.mod h1:JxZsvoBklA50DNhELVJnJ567Rt+KrMH9rR3u515 github.com/go-pkgz/lcw v0.5.0/go.mod h1:CSdQRQthxJQ4iDD4wTPPuWFbFdknJzwJ8WXu1nfxb10= github.com/go-pkgz/lgr v0.6.3 h1:n9pGk2paBV8w/Y/FVEq5MkwDmP33dnUPKbY4CyyygwM= github.com/go-pkgz/lgr v0.6.3/go.mod h1:hBM1NM/SoYdlrykgdgJWGrZ/TM/XaZIjRbJfx7NkMm8= +github.com/go-pkgz/lgr v0.7.0 h1:S/AAPwt/RE9a5mNJskA7dGVp+Dq6SMIW6LYjG3ITxY8= +github.com/go-pkgz/lgr v0.7.0/go.mod h1:yMgxU+GobMRJgIEbSzDKy/67W18S7qmGx/7BVL5AB8Q= github.com/go-pkgz/repeater v1.1.3/go.mod h1:hVTavuO5x3Gxnu8zW7d6sQBfAneKV8X2FjU48kGfpKw= github.com/go-pkgz/rest v1.4.1 h1:DmaVLPH2O7yLehrWOW0uz01d2mVHz9fBR/iuTiPRzaw= github.com/go-pkgz/rest v1.4.1/go.mod h1:COazNj35u3RXAgQNBr6neR599tYP3URiOpsu9p0rOtk= @@ -153,6 +155,8 @@ github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTK github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= 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/rakyll/statik v0.1.6/go.mod h1:OEi9wJV/fMUAGx1eNjq75DKDsJVuEv1U0oYdX6GX8Zs= @@ -177,6 +181,8 @@ github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4= +github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/tidwall/btree v0.0.0-20170113224114-9876f1454cf0/go.mod h1:huei1BkDWJ3/sLXmO+bsCNELL+Bp2Kks9OLyQFkzvA8= github.com/tidwall/buntdb v1.0.0/go.mod h1:Y39xhcDW10WlyYXeLgGftXVbjtM0QP+/kpz8xl9cbzE= github.com/tidwall/buntdb v1.1.0/go.mod h1:Y39xhcDW10WlyYXeLgGftXVbjtM0QP+/kpz8xl9cbzE= @@ -280,6 +286,7 @@ golang.org/x/sys v0.0.0-20190624142023-c5567b49c5d0/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037 h1:YyJpGZS1sBuBCzLAR1VEpK193GlqGZbnPFnPV/5Rsb4= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= 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= golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs= @@ -329,6 +336,7 @@ google.golang.org/genproto v0.0.0-20191009194640-548a555dbc03/go.mod h1:n3cpQtvx google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= From d714e32668499cf4f0c9133734b103f312e7ac0b Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Thu, 26 Mar 2020 23:04:31 +0100 Subject: [PATCH 2/6] explicitly ignore error in test fmt.Fprint --- backend/app/store/admin/remote_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/store/admin/remote_test.go b/backend/app/store/admin/remote_test.go index f9d2722f..ae431ebe 100644 --- a/backend/app/store/admin/remote_test.go +++ b/backend/app/store/admin/remote_test.go @@ -96,6 +96,6 @@ func testServer(t *testing.T, req, resp string) *httptest.Server { require.NoError(t, err) assert.Equal(t, req, string(body)) t.Logf("req: %s", string(body)) - fmt.Fprintf(w, resp) + _, _ = fmt.Fprint(w, resp) })) } From ef6634e9b34f7cb2a3416530a8eab03059faa969 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Thu, 26 Mar 2020 23:04:49 +0100 Subject: [PATCH 3/6] exclude unused filename from image.Store.Save() parameters --- backend/app/rest/api/rest_private.go | 4 ++-- backend/app/store/image/bolt_store.go | 2 +- backend/app/store/image/bolt_store_test.go | 6 +++--- backend/app/store/image/fs_store.go | 10 +++------- backend/app/store/image/fs_store_test.go | 20 ++++++++++---------- backend/app/store/image/image.go | 8 ++++---- backend/app/store/image/image_mock.go | 14 +++++++------- 7 files changed, 30 insertions(+), 34 deletions(-) diff --git a/backend/app/rest/api/rest_private.go b/backend/app/rest/api/rest_private.go index 83036242..05e22d8e 100644 --- a/backend/app/rest/api/rest_private.go +++ b/backend/app/rest/api/rest_private.go @@ -568,14 +568,14 @@ func (s *private) savePictureCtrl(w http.ResponseWriter, r *http.Request) { return } - file, header, err := r.FormFile("file") + file, _, err := r.FormFile("file") if err != nil { rest.SendErrorJSON(w, r, http.StatusInternalServerError, err, "can't get image file from the request", rest.ErrInternal) return } defer func() { _ = file.Close() }() - id, err := s.imageService.Save(header.Filename, user.ID, file) + id, err := s.imageService.Save(user.ID, file) if err != nil { rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't save image", rest.ErrInternal) return diff --git a/backend/app/store/image/bolt_store.go b/backend/app/store/image/bolt_store.go index f1d13a1c..209907fd 100644 --- a/backend/app/store/image/bolt_store.go +++ b/backend/app/store/image/bolt_store.go @@ -86,7 +86,7 @@ func (b *Bolt) SaveWithID(id string, r io.Reader) (string, error) { } // Save data from reader to staging bucket in DB -func (b *Bolt) Save(_ string, userID string, r io.Reader) (id string, err error) { +func (b *Bolt) Save(userID string, r io.Reader) (id string, err error) { id = path.Join(userID, guid()) return b.SaveWithID(id, r) } diff --git a/backend/app/store/image/bolt_store_test.go b/backend/app/store/image/bolt_store_test.go index 27e01fed..5f1a8210 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("file1.png", "user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNG()) 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("file1.png", "user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNG()) 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(file, user, gopherPNG()) + id, err := svc.Save(user, gopherPNG()) require.NoError(t, err) checkBoltImgData(t, svc.db, imagesStagedBktName, id, func(data []byte) error { diff --git a/backend/app/store/image/fs_store.go b/backend/app/store/image/fs_store.go index 80377d6b..878ee892 100644 --- a/backend/app/store/image/fs_store.go +++ b/backend/app/store/image/fs_store.go @@ -58,15 +58,11 @@ func (f *FileSystem) SaveWithID(id string, r io.Reader) (string, error) { return id, nil } -// Save data from a reader for given file name to local FS, staging directory. Returns id as user/uuid +// 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(fileName string, userID string, r io.Reader) (id string, err error) { +func (f *FileSystem) Save(userID string, r io.Reader) (id string, err error) { tempId := path.Join(userID, guid()) // make id as user/uuid - id, err = f.SaveWithID(tempId, r) - if err != nil { - err = errors.Wrapf(err, "can't save image file %s", fileName) - } - return id, err + return f.SaveWithID(tempId, r) } // Commit file stored in staging location by moving it to permanent location diff --git a/backend/app/store/image/fs_store_test.go b/backend/app/store/image/fs_store_test.go index d2077d81..8b7908bc 100644 --- a/backend/app/store/image/fs_store_test.go +++ b/backend/app/store/image/fs_store_test.go @@ -44,7 +44,7 @@ func TestFsStore_Save(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - id, err := svc.Save("file1.png", "user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNG()) assert.NoError(t, err) assert.Contains(t, id, "user1/") t.Log(id) @@ -61,7 +61,7 @@ func TestFsStore_SaveWithResize(t *testing.T) { defer teardown() svc.MaxWidth, svc.MaxHeight = 32, 32 - id, err := svc.Save("file1.png", "user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNG()) assert.NoError(t, err) assert.Contains(t, id, "user1/") t.Log(id) @@ -82,7 +82,7 @@ func TestFsStore_SaveWithResizeJpeg(t *testing.T) { fh, err := os.Open("testdata/circles.jpg") defer func() { assert.NoError(t, fh.Close()) }() assert.NoError(t, err) - id, err := svc.Save("circles.jpg", "user1", fh) + id, err := svc.Save("user1", fh) assert.NoError(t, err) assert.Contains(t, id, "user1/") t.Log(id) @@ -103,7 +103,7 @@ func TestFsStore_SaveNoResizeJpeg(t *testing.T) { fh, err := os.Open("testdata/circles.jpg") defer func() { assert.NoError(t, fh.Close()) }() assert.NoError(t, err) - id, err := svc.Save("circles.jpg", "user1", fh) + id, err := svc.Save("user1", fh) assert.NoError(t, err) assert.Contains(t, id, "user1/") t.Log(id) @@ -119,7 +119,7 @@ func TestFsStore_WrongFormat(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - _, err := svc.Save("file1.png", "user1", strings.NewReader("blah blah bad image")) + _, err := svc.Save("user1", strings.NewReader("blah blah bad image")) assert.Error(t, err) } @@ -127,7 +127,7 @@ func TestFsStore_SaveAndCommit(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - id, err := svc.Save("file1.png", "user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNG()) require.NoError(t, err) err = svc.Commit(id) require.NoError(t, err) @@ -147,7 +147,7 @@ func TestFsStore_SaveTooLarge(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() svc.MaxSize = 2000 - _, err := svc.Save("blah_ff1.png", "user2", io.MultiReader(gopherPNG(), gopherPNG())) + _, err := svc.Save("user2", io.MultiReader(gopherPNG(), gopherPNG())) assert.Error(t, err) assert.Contains(t, err.Error(), "is too large") } @@ -157,7 +157,7 @@ func TestFsStore_LoadAfterSave(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - id, err := svc.Save("blah_ff1.png", "user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNG()) assert.NoError(t, err) t.Log(id) @@ -173,7 +173,7 @@ func TestFsStore_LoadAfterCommit(t *testing.T) { svc, teardown := prepareImageTest(t) defer teardown() - id, err := svc.Save("blah_ff1.png", "user1", gopherPNG()) + id, err := svc.Save("user1", gopherPNG()) assert.NoError(t, err) t.Log(id) err = svc.Commit(id) @@ -235,7 +235,7 @@ func TestFsStore_Cleanup(t *testing.T) { defer teardown() save := func(file string, user string) (path string) { - id, err := svc.Save(file, user, gopherPNG()) + id, err := svc.Save(user, gopherPNG()) require.NoError(t, err) img := svc.location(svc.Staging, id) data, err := ioutil.ReadFile(img) diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index 7a55e5f6..7cb2e7ae 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -47,10 +47,10 @@ 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(fileName string, 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, 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 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 diff --git a/backend/app/store/image/image_mock.go b/backend/app/store/image/image_mock.go index 8c23293c..9c3090fd 100644 --- a/backend/app/store/image/image_mock.go +++ b/backend/app/store/image/image_mock.go @@ -63,20 +63,20 @@ func (_m *MockStore) Load(id string) ([]byte, error) { return r0, r1 } -// Save provides a mock function with given fields: fileName, userID, r -func (_m *MockStore) Save(fileName string, userID string, r io.Reader) (string, error) { - ret := _m.Called(fileName, userID, r) +// 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) var r0 string - if rf, ok := ret.Get(0).(func(string, string, io.Reader) string); ok { - r0 = rf(fileName, userID, r) + if rf, ok := ret.Get(0).(func(string, io.Reader) string); ok { + r0 = rf(userID, r) } else { r0 = ret.Get(0).(string) } var r1 error - if rf, ok := ret.Get(1).(func(string, string, io.Reader) error); ok { - r1 = rf(fileName, userID, r) + if rf, ok := ret.Get(1).(func(string, io.Reader) error); ok { + r1 = rf(userID, r) } else { r1 = ret.Error(1) } From 33dde7d4dd785b92ffcd4eae2029e1825b8362ac Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Fri, 27 Mar 2020 01:53:17 +0100 Subject: [PATCH 4/6] 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 From e1d502ecf8273073902cafad2e2cddbcf485371e Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Fri, 27 Mar 2020 02:24:44 +0100 Subject: [PATCH 5/6] add clarifying comment to image.Store interface --- backend/app/store/image/image.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index 61a0fa26..32037b81 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -48,7 +48,9 @@ type Service struct { // sh -c "mockery -inpkg -name Store -print > /tmp/image-mock.tmp && mv /tmp/image-mock.tmp image_mock.go" // 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 +// Declares two-stage save with Commit. Save stores to staging area and Commit moves to the final location. +// Two-stage commit scheme is used for not storing images which are uploaded but later never used in the comments, +// e.g. when somebody uploaded a picture but did not sent the comment. type Store interface { 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 From 4539b8ffb4d5c97cbd7f078782b64f12ce411ab6 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Fri, 27 Mar 2020 20:57:53 +0100 Subject: [PATCH 6/6] hide image.Store from image.Service consumers --- backend/app/cmd/server.go | 10 +++--- backend/app/rest/api/rest.go | 2 +- backend/app/rest/api/rest_private_test.go | 10 +++--- backend/app/rest/api/rest_test.go | 15 ++++----- backend/app/rest/proxy/image_test.go | 4 +-- backend/app/store/image/image.go | 39 ++++++++++++++--------- backend/app/store/image/image_test.go | 34 +++++++++----------- backend/app/store/service/service_test.go | 2 +- 8 files changed, 59 insertions(+), 57 deletions(-) diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index 0345f921..bfd85fdc 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -567,7 +567,7 @@ func (s *ServerCommand) makeAvatarStore() (avatar.Store, error) { } func (s *ServerCommand) makePicturesStore() (*image.Service, error) { - imageService := &image.Service{ + imageServiceParams := image.ServiceParams{ ImageAPI: s.RemarkURL + "/api/v1/picture/", TTL: 5 * s.EditDuration, // add extra time to image TTL for staging MaxSize: s.Image.MaxSize, @@ -580,18 +580,16 @@ func (s *ServerCommand) makePicturesStore() (*image.Service, error) { if err != nil { return nil, err } - imageService.Store = boltImageStore - return imageService, nil + return image.NewService(boltImageStore, imageServiceParams), nil case "fs": if err := makeDirs(s.Image.FS.Path); err != nil { return nil, err } - imageService.Store = &image.FileSystem{ + return image.NewService(&image.FileSystem{ Location: s.Image.FS.Path, Staging: s.Image.FS.Staging, Partitions: s.Image.FS.Partitions, - } - return imageService, nil + }, imageServiceParams), nil } return nil, errors.Errorf("unsupported pictures store type %s", s.Image.Type) } diff --git a/backend/app/rest/api/rest.go b/backend/app/rest/api/rest.go index 1d3841b5..580d9acd 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.SizeLimit(), + MaxImageSize: s.ImageService.MaxSize, 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 fac95f26..fc19eb81 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -901,13 +901,13 @@ func TestRest_CreateWithPictures(t *testing.T) { }() lgr.Setup(lgr.Debug, lgr.CallerFile, lgr.CallerFunc) - imageService := svc.ImageService - imageService.Store = &image.FileSystem{ + imageService := image.NewService(&image.FileSystem{ Staging: "/tmp/remark42/images.staging", Location: "/tmp/remark42/images", - } - imageService.TTL = 100 * time.Millisecond - imageService.MaxSize = 2000 + }, image.ServiceParams{ + TTL: 100 * time.Millisecond, + 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 ebf61ae5..b55a74e9 100644 --- a/backend/app/rest/api/rest_test.go +++ b/backend/app/rest/api/rest_test.go @@ -371,15 +371,14 @@ func startupT(t *testing.T) (ts *httptest.Server, srv *Rest, teardown func()) { Cache: memCache, WebRoot: tmp, RemarkURL: "https://demo.remark42.com", - ImageService: &image.Service{ - Store: &image.FileSystem{ - Location: tmp + "/pics-remark42", - Partitions: 100, - Staging: tmp + "/pics-remark42/staging", - }, - TTL: time.Millisecond * 100, + ImageService: image.NewService(&image.FileSystem{ + Location: tmp + "/pics-remark42", + Partitions: 100, + Staging: tmp + "/pics-remark42/staging", + }, image.ServiceParams{ + TTL: 100 * time.Millisecond, MaxSize: 10000, - }, + }), ImageProxy: &proxy.Image{}, ReadOnlyAge: 10, CommentFormatter: store.NewCommentFormatter(&proxy.Image{}), diff --git a/backend/app/rest/proxy/image_test.go b/backend/app/rest/proxy/image_test.go index 301acf64..9702c8e5 100644 --- a/backend/app/rest/proxy/image_test.go +++ b/backend/app/rest/proxy/image_test.go @@ -128,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, MaxSize: 1500}, + ImageService: image.NewService(&imageStore, image.ServiceParams{MaxSize: 1500}), } ts := httptest.NewServer(http.HandlerFunc(img.Handler)) @@ -160,7 +160,7 @@ func TestImage_RoutesUsingCachedImage(t *testing.T) { CacheExternal: true, RemarkURL: "https://demo.remark42.com", RoutePath: "/api/v1/proxy", - ImageService: &image.Service{Store: &imageStore}, + ImageService: image.NewService(&imageStore, image.ServiceParams{}), } ts := httptest.NewServer(http.HandlerFunc(img.Handler)) diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index 32037b81..5767c4fa 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -31,17 +31,22 @@ import ( // It also provides async Submit with func param retrieving all submitting ids. // Submitted ids committed (i.e. moved from staging to final) on TTL expiration. type Service struct { - Store + ServiceParams + + store Store + wg sync.WaitGroup + submitCh chan submitReq + once sync.Once + term int32 // term value used atomically to detect emergency termination +} + +// ServiceParams contains externally adjustable parameters of Service +type ServiceParams struct { 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 - once sync.Once - term int32 // term value used atomically to detect emergency termination } // To regenerate mock run from this directory: @@ -67,6 +72,10 @@ type submitReq struct { TS time.Time } +func NewService(s Store, p ServiceParams) *Service { + return &Service{ServiceParams: p, store: s} +} + // Submit multiple ids via function for delayed commit func (s *Service) Submit(idsFn func() []string) { if idsFn == nil || s == nil { @@ -85,7 +94,7 @@ func (s *Service) Submit(idsFn func() []string) { time.Sleep(time.Millisecond * 10) // small sleep to relive busy wait but keep reactive for term (close) } for _, id := range req.idsFn() { - if err := s.Commit(id); err != nil { + if err := s.store.Commit(id); err != nil { log.Printf("[WARN] failed to commit image %s", id) } } @@ -131,7 +140,7 @@ func (s *Service) Cleanup(ctx context.Context) { log.Printf("[INFO] cleanup terminated, %v", ctx.Err()) return case <-time.After(s.TTL / 2): // cleanup call on every 1/2 TTL - if err := s.Store.Cleanup(ctx, s.TTL); err != nil { + if err := s.store.Cleanup(ctx, s.TTL); err != nil { log.Printf("[WARN] failed to cleanup, %v", err) } } @@ -155,13 +164,18 @@ func (s *Service) Close() { s.wg.Wait() } +// Load wraps storage Load function. +func (s *Service) Load(id string) ([]byte, error) { + return s.store.Load(id) +} + // 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) + return s.store.Save(userID, img) } // SaveWithID wraps storage SaveWithID function, validating and resizing the image before calling it. @@ -170,12 +184,7 @@ func (s *Service) SaveWithID(id string, r io.Reader) (string, error) { 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 + return s.store.SaveWithID(id, img) } // prepareImage calls readAndValidateImage and resize on provided image. diff --git a/backend/app/store/image/image_test.go b/backend/app/store/image/image_test.go index 7f2a4306..27ed6451 100644 --- a/backend/app/store/image/image_test.go +++ b/backend/app/store/image/image_test.go @@ -17,11 +17,9 @@ import ( "github.com/stretchr/testify/require" ) -func TestService_Save(t *testing.T) { +func TestService_SaveAndLoad(t *testing.T) { store := MockStore{} - svc := Service{Store: &store} - svc.MaxSize = 1500 - svc.MaxWidth, svc.MaxHeight = 32, 32 + svc := NewService(&store, ServiceParams{MaxSize: 1500, MaxWidth: 32, MaxHeight: 32}) store.On("Save", "user1", mock.Anything).Return("user1/test_id", nil) id, err := svc.Save("user1", gopherPNG()) @@ -32,6 +30,11 @@ func TestService_Save(t *testing.T) { id, err = svc.SaveWithID("test_id", gopherPNG()) assert.NoError(t, err) assert.Equal(t, "test_id", id) + + store.On("Load", "test_id", mock.Anything).Return(nil, nil) + img, err := svc.Load("test_id") + assert.NoError(t, err) + assert.Nil(t, img) } func TestService_Resize(t *testing.T) { @@ -57,7 +60,7 @@ func TestService_ResizeJpeg(t *testing.T) { } func TestService_SaveTooLarge(t *testing.T) { - svc := Service{ImageAPI: "/blah/"} + svc := Service{ServiceParams: ServiceParams{ImageAPI: "/blah/"}} svc.MaxSize = 2000 _, err := svc.Save("user2", io.MultiReader(gopherPNG(), gopherPNG())) assert.Error(t, err) @@ -68,21 +71,14 @@ func TestService_SaveTooLarge(t *testing.T) { } func TestService_WrongFormat(t *testing.T) { - svc := Service{ImageAPI: "/blah/"} + svc := Service{ServiceParams: ServiceParams{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/"} + svc := Service{ServiceParams: ServiceParams{ImageAPI: "/blah/"}} html := `blah foo xyz

123

` ids, err := svc.ExtractPictures(html) @@ -93,7 +89,7 @@ func TestService_ExtractPictures(t *testing.T) { } func TestService_ExtractPictures2(t *testing.T) { - svc := Service{ImageAPI: "https://remark42.radio-t.com/api/v1/picture/"} + svc := Service{ServiceParams: ServiceParams{ImageAPI: "https://remark42.radio-t.com/api/v1/picture/"}} html := "

TLDR: такое в go пока правильно посчитать трудно. То, что они считают это общее количество go packages в коде." + "

\n\n

Пакеты в го это средство организации кода, они могут быть связанны друг с другом в рамках одной библиотеки (модуля). Например одна из моих вот так выглядит на libraries.io:

\n\n

\"bjtr0-201906-08110846-i324c.png\"/

\n\n

По форме все верно, это все packages, но по сути это все одна библиотека организованная таким образом. При ее импорте, например посредством go mod, она выглядит как один модуль, т.е. github.com/go-pkgz/auth v0.5.2.

\n" ids, err := svc.ExtractPictures(html) @@ -106,7 +102,7 @@ func TestService_Cleanup(t *testing.T) { store := MockStore{} store.On("Cleanup", mock.Anything, mock.Anything).Times(10).Return(nil) - svc := Service{Store: &store, TTL: 100 * time.Millisecond} + svc := Service{store: &store, ServiceParams: ServiceParams{TTL: 100 * time.Millisecond}} ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*549) defer cancel() svc.Cleanup(ctx) @@ -116,7 +112,7 @@ func TestService_Cleanup(t *testing.T) { func TestService_Submit(t *testing.T) { store := MockStore{} store.On("Commit", mock.Anything, mock.Anything).Times(5).Return(nil) - svc := Service{Store: &store, ImageAPI: "/blah/", TTL: time.Millisecond * 100} + svc := Service{store: &store, ServiceParams: ServiceParams{ImageAPI: "/blah/", TTL: time.Millisecond * 100}} svc.Submit(func() []string { return []string{"id1", "id2", "id3"} }) svc.Submit(func() []string { return []string{"id4", "id5"} }) svc.Submit(nil) @@ -128,7 +124,7 @@ func TestService_Submit(t *testing.T) { func TestService_Close(t *testing.T) { store := MockStore{} store.On("Commit", mock.Anything, mock.Anything).Times(5).Return(nil) - svc := Service{Store: &store, ImageAPI: "/blah/", TTL: time.Millisecond * 500} + svc := Service{store: &store, ServiceParams: ServiceParams{ImageAPI: "/blah/", TTL: time.Millisecond * 500}} svc.Submit(func() []string { return []string{"id1", "id2", "id3"} }) svc.Submit(func() []string { return []string{"id4", "id5"} }) svc.Submit(nil) @@ -139,7 +135,7 @@ func TestService_Close(t *testing.T) { func TestService_SubmitDelay(t *testing.T) { store := MockStore{} store.On("Commit", mock.Anything, mock.Anything).Times(5).Return(nil) - svc := Service{Store: &store, ImageAPI: "/blah/", TTL: time.Millisecond * 100} + svc := Service{store: &store, ServiceParams: ServiceParams{ImageAPI: "/blah/", TTL: time.Millisecond * 100}} svc.Submit(func() []string { return []string{"id1", "id2", "id3"} }) time.Sleep(150 * time.Millisecond) // let first batch to pass TTL svc.Submit(func() []string { return []string{"id4", "id5"} }) diff --git a/backend/app/store/service/service_test.go b/backend/app/store/service/service_test.go index bf15b6dc..b237f600 100644 --- a/backend/app/store/service/service_test.go +++ b/backend/app/store/service/service_test.go @@ -1278,7 +1278,7 @@ func TestService_submitImages(t *testing.T) { mockStore := image.MockStore{} mockStore.On("Commit", mock.Anything, mock.Anything).Times(2).Return(nil) - imgSvc := &image.Service{Store: &mockStore, TTL: time.Millisecond * 50} + imgSvc := image.NewService(&mockStore, image.ServiceParams{TTL: 50 * time.Millisecond * 50}) // two comments for https://radio-t.com eng, teardown := prepStoreEngine(t)