From c990b05c211c4cdd18f3d013febbab7615e0b3bb Mon Sep 17 00:00:00 2001 From: Umputun Date: Sat, 23 Mar 2019 14:54:45 -0500 Subject: [PATCH] add image service with delayed commit. Move cleanup loop to service --- backend/app/cmd/server.go | 4 +- backend/app/rest/api/rest.go | 2 +- backend/app/store/image/fs_store.go | 161 +++++++++++++++++ backend/app/store/image/fs_store_test.go | 212 +++++++++++++++++++++++ backend/app/store/image/image.go | 201 ++++----------------- backend/app/store/image/image_mock.go | 92 ++++++++++ backend/app/store/image/image_test.go | 209 ++-------------------- backend/app/store/service/service.go | 8 +- backend/go.mod | 1 + backend/go.sum | 3 + backend/vendor/modules.txt | 2 + 11 files changed, 526 insertions(+), 369 deletions(-) create mode 100644 backend/app/store/image/fs_store.go create mode 100644 backend/app/store/image/fs_store_test.go create mode 100644 backend/app/store/image/image_mock.go diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index 3180f0a8..f5292400 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -178,7 +178,7 @@ type serverApp struct { dataService *service.DataStore avatarStore avatar.Store notifyService *notify.Service - imageService image.Interface + imageService image.Store terminated chan struct{} } @@ -434,7 +434,7 @@ func (s *ServerCommand) makeAvatarStore() (avatar.Store, error) { return nil, errors.Errorf("unsupported avatar store type %s", s.Avatar.Type) } -func (s *ServerCommand) makePicturesStore() (image.Interface, error) { +func (s *ServerCommand) makePicturesStore() (image.Store, error) { switch s.Image.Type { case "fs": if err := makeDirs(s.Image.FS.Path); err != nil { diff --git a/backend/app/rest/api/rest.go b/backend/app/rest/api/rest.go index c7349f84..8625d504 100644 --- a/backend/app/rest/api/rest.go +++ b/backend/app/rest/api/rest.go @@ -45,7 +45,7 @@ type Rest struct { CommentFormatter *store.CommentFormatter Migrator *Migrator NotifyService *notify.Service - ImageService image.Interface + ImageService image.Store WebRoot string RemarkURL string diff --git a/backend/app/store/image/fs_store.go b/backend/app/store/image/fs_store.go new file mode 100644 index 00000000..ae6c4c64 --- /dev/null +++ b/backend/app/store/image/fs_store.go @@ -0,0 +1,161 @@ +package image + +import ( + "context" + "fmt" + "hash/crc64" + "io" + "log" + "math" + "os" + "path" + "path/filepath" + "strconv" + "strings" + "sync" + "time" + + "github.com/google/uuid" + "github.com/pkg/errors" +) + +// FileSystem provides image Store for local files. Saves and loads files from Location, restricts max size. +type FileSystem struct { + Location string + Staging string + MaxSize int + Partitions int + + crc struct { + *crc64.Table + sync.Once + mask string + divider uint64 + } +} + +// Save data from reader for given file name to local FS, staging directory. Returns id as user/uuid.ext +// Files partitioned across multiple subdirectories and the final path includes part, i.e. /location/user1/03/123-4567.png +func (f *FileSystem) Save(fileName string, userID string, r io.Reader) (id string, err error) { + + uid, err := uuid.NewUUID() + if err != nil { + return "", errors.Wrap(err, "can't make image uuid") + } + + id = path.Join(userID, uid.String()) + filepath.Ext(fileName) // make id as user/uuid.ext + dst := f.location(f.Staging, id) + + if err = os.MkdirAll(path.Dir(dst), 0700); err != nil { + return "", errors.Wrap(err, "can't make image directory") + } + + fh, err := os.Create(dst) + if err != nil { + return "", errors.Wrapf(err, "can't make image file %s", dst) + } + lr := io.LimitReader(r, int64(f.MaxSize)+1) + written, err := io.Copy(fh, lr) + if err != nil { + return "", errors.Wrapf(err, "can't write image file %s", dst) + } + if err = fh.Close(); err != nil { + return "", errors.Wrapf(err, "can't close image file %s", dst) + } + if written > int64(f.MaxSize) { + if err = os.Remove(dst); err != nil { + log.Printf("[WARN] can't remove image file %s, %v", dst, err) + } + return "", errors.Errorf("file %s is too large", fileName) + } + log.Printf("[DEBUG] file %s saved for image %s", fh.Name(), fileName) + return id, nil +} + +// Commit file stored in staging location by moving it to permanent location +func (f *FileSystem) Commit(id string) error { + stagingImage, permImage := f.location(f.Staging, id), f.location(f.Location, id) + + if err := os.MkdirAll(path.Dir(permImage), 0700); err != nil { + return errors.Wrap(err, "can't make image directory") + } + + err := os.Rename(stagingImage, permImage) + return errors.Wrapf(err, "failed to commit image %s", id) +} + +// Load image from FS. Uses id to get partition subdirectory. +// returns ReadCloser and caller should call close after processing completed. +func (f *FileSystem) Load(id string) (io.ReadCloser, int64, error) { + + // get image file by id. first try permanent location and if not found - staging + img := func(id string) (file string, st os.FileInfo, err error) { + file = f.location(f.Location, id) + st, err = os.Stat(file) + if err != nil { + file = f.location(f.Staging, id) + st, err = os.Stat(file) + } + return file, st, errors.Wrapf(err, "can't get image stats for %s", id) + } + + imgFile, st, err := img(id) + if err != nil { + return nil, 0, errors.Wrapf(err, "can't get image file for %s", id) + } + + fh, err := os.Open(imgFile) + if err != nil { + return nil, 0, errors.Wrapf(err, "can't load image %s", id) + } + return fh, st.Size(), nil +} + +// Cleanup runs scan of staging and removes old files based on ttl +func (f *FileSystem) Cleanup(ctx context.Context, ttl time.Duration) error { + err := filepath.Walk(f.Staging, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + age := time.Since(info.ModTime()) + if age > ttl { + log.Printf("[INFO] remove staging image %s, age %v", path, age) + return os.Remove(path) + } + return nil + }) + return errors.Wrap(err, "failed to cleanup images") +} + +// 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. +// Number of partitions defined by FileSystem.Partitions +func (f *FileSystem) location(base string, id string) string { + + partition := func(id string) string { + f.crc.Do(func() { + f.crc.Table = crc64.MakeTable(crc64.ECMA) + p := int(math.Round(math.Log10(float64(f.Partitions)))) + f.crc.mask = "%0" + strconv.Itoa(p) + "d" + f.crc.divider = uint64(math.Pow(10, float64(p))) + }) + checksum64 := crc64.Checksum([]byte(id), f.crc.Table) + partition := checksum64 % f.crc.divider + return fmt.Sprintf(f.crc.mask, partition) + } + + user, file := "unknown", id // default if no user in id + if elems := strings.Split(id, "/"); len(elems) == 2 { + user, file = elems[0], elems[1] // user in id + } + + if f.Partitions == 0 { + return path.Join(base, user, file) // avoid partition directory if 0 Partitions + } + + return path.Join(base, user, partition(id), file) +} diff --git a/backend/app/store/image/fs_store_test.go b/backend/app/store/image/fs_store_test.go new file mode 100644 index 00000000..5906d814 --- /dev/null +++ b/backend/app/store/image/fs_store_test.go @@ -0,0 +1,212 @@ +package image + +import ( + "context" + "io/ioutil" + "math/rand" + "os" + "strconv" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFsStore_Save(t *testing.T) { + svc, teardown := prepareImageTest(t) + defer teardown() + + id, err := svc.Save("file1.png", "user1", strings.NewReader("blah blah")) + assert.NoError(t, err) + assert.Contains(t, id, "user1/") + assert.Contains(t, id, ".png") + t.Log(id) + + img := svc.location(svc.Staging, id) + t.Log(img) + data, err := ioutil.ReadFile(img) + assert.NoError(t, err) + assert.Equal(t, "blah blah", string(data)) +} + +func TestFsStore_SaveAndCommit(t *testing.T) { + svc, teardown := prepareImageTest(t) + defer teardown() + + id, err := svc.Save("file1.png", "user1", strings.NewReader("blah blah")) + require.NoError(t, err) + err = svc.Commit(id) + require.NoError(t, err) + + imgStaging := svc.location(svc.Staging, id) + _, err = os.Stat(imgStaging) + assert.NotNil(t, err, "no file on staging anymore") + + img := svc.location(svc.Location, id) + t.Log(img) + data, err := ioutil.ReadFile(img) + assert.NoError(t, err) + assert.Equal(t, "blah blah", string(data)) +} + +func TestFsStore_SaveTooLarge(t *testing.T) { + svc, teardown := prepareImageTest(t) + defer teardown() + svc.MaxSize = 5 + _, err := svc.Save("blah_ff1.png", "user2", strings.NewReader("blah blah")) + 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("blah_ff1.png", "user1", strings.NewReader("blah blah")) + assert.NoError(t, err) + t.Log(id) + + r, sz, err := svc.Load(id) + assert.NoError(t, err) + defer func() { assert.NoError(t, r.Close()) }() + data, err := ioutil.ReadAll(r) + assert.NoError(t, err) + assert.Equal(t, "blah blah", string(data)) + assert.Equal(t, int64(9), sz) + _, _, err = svc.Load("abcd") + assert.NotNil(t, err) +} + +func TestFsStore_LoadAfterCommit(t *testing.T) { + + svc, teardown := prepareImageTest(t) + defer teardown() + + id, err := svc.Save("blah_ff1.png", "user1", strings.NewReader("blah blah")) + assert.NoError(t, err) + t.Log(id) + err = svc.Commit(id) + require.NoError(t, err) + + r, sz, err := svc.Load(id) + assert.NoError(t, err) + defer func() { assert.NoError(t, r.Close()) }() + data, err := ioutil.ReadAll(r) + assert.NoError(t, err) + assert.Equal(t, "blah blah", string(data)) + assert.Equal(t, int64(9), sz) + _, _, err = svc.Load("abcd") + assert.NotNil(t, err) +} + +func TestFsStore_location(t *testing.T) { + tbl := []struct { + partitions int + id, res string + }{ + {10, "u1/abcdefg.png", "/tmp/u1/4/abcdefg.png"}, + {10, "abcdefe", "/tmp/unknown/1/abcdefe"}, + {10, "12345", "/tmp/unknown/9/12345"}, + {100, "12345", "/tmp/unknown/69/12345"}, + {100, "xyzz", "/tmp/unknown/58/xyzz"}, + {100, "6851dcde6024e03258a66705f29e14b506048c74.png", "/tmp/unknown/02/6851dcde6024e03258a66705f29e14b506048c74.png"}, + {5, "6851dcde6024e03258a66705f29e14b506048c74.png", "/tmp/unknown/2/6851dcde6024e03258a66705f29e14b506048c74.png"}, + {5, "xxxyz.png", "/tmp/unknown/0/xxxyz.png"}, + {0, "12345", "/tmp/unknown/12345"}, + } + for n, tt := range tbl { + t.Run(strconv.Itoa(n), func(t *testing.T) { + svc := FileSystem{Location: "/tmp", Partitions: tt.partitions} + assert.Equal(t, tt.res, svc.location("/tmp", tt.id)) + }) + } + + // generate random names and make sure partition never runs out of allowed + letterRunes := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") + randomID := func(n int) string { + b := make([]rune, n) + for i := range b { + b[i] = letterRunes[rand.Intn(len(letterRunes))] + } + return "user1" + "/" + string(b) + } + + svc := FileSystem{Location: "/tmp", Partitions: 10} + for i := 0; i < 1000; i++ { + v := randomID(rand.Intn(64)) + location := svc.location("/tmp", v) + elems := strings.Split(location, "/") + p, err := strconv.Atoi(elems[3]) + require.NoError(t, err, location) + assert.True(t, p >= 0 && p < 10) + } +} + +func TestFsStore_Cleanup(t *testing.T) { + svc, teardown := prepareImageTest(t) + defer teardown() + + save := func(file string, user string, content string) (path string) { + id, err := svc.Save(file, user, strings.NewReader(content)) + require.NoError(t, err) + img := svc.location(svc.Staging, id) + data, err := ioutil.ReadFile(img) + require.NoError(t, err) + require.Equal(t, content, string(data)) + return img + } + + // save 3 images to staging + img1 := save("blah_ff1.png", "user1", "blah blah1") + time.Sleep(100 * time.Millisecond) + img2 := save("blah_ff2.png", "user1", "blah blah2") + time.Sleep(100 * time.Millisecond) + img3 := save("blah_ff3.png", "user2", "blah blah3") + + time.Sleep(100 * time.Millisecond) // make first image expired + err := svc.Cleanup(context.Background(), time.Millisecond*300) + assert.NoError(t, err) + + _, err = os.Stat(img1) + assert.NotNil(t, err, "no file on staging anymore") + _, err = os.Stat(img2) + assert.NoError(t, err, "file on staging") + _, err = os.Stat(img3) + assert.NoError(t, err, "file on staging") + + time.Sleep(200 * time.Millisecond) // make all images expired + err = svc.Cleanup(context.Background(), time.Millisecond*300) + assert.NoError(t, err) + + _, err = os.Stat(img2) + assert.NotNil(t, err, "no file on staging anymore") + _, err = os.Stat(img3) + assert.NotNil(t, err, "no file on staging anymore") +} + +func prepareImageTest(t *testing.T) (svc FileSystem, teardown func()) { + loc, err := ioutil.TempDir("", "test_image_r42") + require.NoError(t, err, "failed to make temp dir") + + staging, err := ioutil.TempDir("", "test_image_r42.staging") + require.NoError(t, err, "failed to make temp staging dir") + + svc = FileSystem{ + Location: loc, + Staging: staging, + Partitions: 100, + MaxSize: 50, + } + + teardown = func() { + defer func() { + assert.NoError(t, os.RemoveAll(loc)) + assert.NoError(t, os.RemoveAll(staging)) + }() + } + + return svc, teardown +} diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index 16fa144d..e0c95224 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -1,195 +1,49 @@ // Package image handles storing, resizing and retrieval of images -// Provides Interface with Save and Load and one implementation on top of local file system. +// Provides Store with Save and Load and one implementation on top of local file system. +// Service object encloses Store and add common methods, this is the one consumer should use package image +//go:generate sh -c "mockgen -source=image.go -package=image > image_mock.go" + import ( "context" - "fmt" - "hash/crc64" "io" - "math" - "os" - "path" - "path/filepath" - "strconv" + "log" "strings" - "sync" "time" "github.com/PuerkitoBio/goquery" - log "github.com/go-pkgz/lgr" - "github.com/google/uuid" "github.com/pkg/errors" ) -// Interface defines Save and Load methods -type Interface interface { +// Store defines interface for saving and loading pictures. +// Declares two-stage save with commit +type Store interface { Save(fileName string, userID string, r io.Reader) (id string, err error) // get name and reader and returns ID of stored image Commit(id string) error // move image from staging to permanent Load(id string) (io.ReadCloser, int64, error) // load image by ID. Caller has to close the reader. Cleanup(ctx context.Context) // run removal loop for old images on staging } -// FileSystem provides image Interface for local files. Saves and loads files from Location, restricts max size -type FileSystem struct { - Location string - Staging string - MaxSize int - Partitions int - TTL time.Duration // for how long file allowed on staging - - crc struct { - *crc64.Table - sync.Once - mask string - divider uint64 - } +// Service extends Store with common functions needed for any store implementation +type Service struct { + Store + TTL time.Duration // for how long file allowed on staging } -// Save data from reader for given file name to local FS, staging directory. Returns id as user/uuid.ext -// Files partitioned across multiple subdirectories and the final path includes part, i.e. /location/user1/03/123-4567.png -func (f *FileSystem) Save(fileName string, userID string, r io.Reader) (id string, err error) { - - uid, err := uuid.NewUUID() - if err != nil { - return "", errors.Wrap(err, "can't make image uuid") - } - - id = path.Join(userID, uid.String()) + filepath.Ext(fileName) // make id as user/uuid.ext - dst := f.location(f.Staging, id) - - if err = os.MkdirAll(path.Dir(dst), 0700); err != nil { - return "", errors.Wrap(err, "can't make image directory") - } - - fh, err := os.Create(dst) - if err != nil { - return "", errors.Wrapf(err, "can't make image file %s", dst) - } - lr := io.LimitReader(r, int64(f.MaxSize)+1) - written, err := io.Copy(fh, lr) - if err != nil { - return "", errors.Wrapf(err, "can't write image file %s", dst) - } - if err = fh.Close(); err != nil { - return "", errors.Wrapf(err, "can't close image file %s", dst) - } - if written > int64(f.MaxSize) { - if err = os.Remove(dst); err != nil { - log.Printf("[WARN] can't remove image file %s, %v", dst, err) - } - return "", errors.Errorf("file %s is too large", fileName) - } - log.Printf("[DEBUG] file %s saved for image %s", fh.Name(), fileName) - return id, nil -} - -// Commit file stored in staging location by moving it to permanent location -func (f *FileSystem) Commit(id string) error { - stagingImage, permImage := f.location(f.Staging, id), f.location(f.Location, id) - - if err := os.MkdirAll(path.Dir(permImage), 0700); err != nil { - return errors.Wrap(err, "can't make image directory") - } - - err := os.Rename(stagingImage, permImage) - return errors.Wrapf(err, "failed to commit image %s", id) -} - -// Load image from FS. Uses id to get partition subdirectory. -// returns ReadCloser and caller should call close after processing completed. -func (f *FileSystem) Load(id string) (io.ReadCloser, int64, error) { - - // get image file by id. first try permanent location and if not found - staging - img := func(id string) (file string, st os.FileInfo, err error) { - file = f.location(f.Location, id) - st, err = os.Stat(file) - if err != nil { - file = f.location(f.Staging, id) - st, err = os.Stat(file) - } - return file, st, errors.Wrapf(err, "can't get image stats for %s", id) - } - - imgFile, st, err := img(id) - if err != nil { - return nil, 0, errors.Wrapf(err, "can't get image file for %s", id) - } - - fh, err := os.Open(imgFile) - if err != nil { - return nil, 0, errors.Wrapf(err, "can't load image %s", id) - } - return fh, st.Size(), nil -} - -// Cleanup runs periodic scan of staging and removes old files based on TTL -func (f *FileSystem) Cleanup(ctx context.Context) { - log.Printf("[INFO] start pictures cleanup, staging ttl=%v", f.TTL) - - cleanup := func() { - err := filepath.Walk(f.Staging, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err +// Submit multiple ids for delayed commit +func (s *Service) Submit(ids []string, delay time.Duration) { + time.AfterFunc(delay, func() { + for _, id := range ids { + if err := s.Commit(id); err != nil { + log.Printf("[WARN] failed to commit image %s", id) } - if info.IsDir() { - return nil - } - age := time.Since(info.ModTime()) - if age > f.TTL { - log.Printf("[INFO] remove staging image %s, age %v", path, age) - return os.Remove(path) - } - return nil - }) - if err != nil { - log.Printf("[WARN] failed to cleanup images, %v", err) } - } - - for { - select { - case <-ctx.Done(): - log.Printf("[INFO] cleanup terminated, %v", ctx.Err()) - return - case <-time.After(f.TTL / 2): - cleanup() - } - } -} - -// 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. -// Number of partitions defined by FileSystem.Partitions -func (f *FileSystem) location(base string, id string) string { - - partition := func(id string) string { - f.crc.Do(func() { - f.crc.Table = crc64.MakeTable(crc64.ECMA) - p := int(math.Round(math.Log10(float64(f.Partitions)))) - f.crc.mask = "%0" + strconv.Itoa(p) + "d" - f.crc.divider = uint64(math.Pow(10, float64(p))) - }) - checksum64 := crc64.Checksum([]byte(id), f.crc.Table) - partition := checksum64 % f.crc.divider - return fmt.Sprintf(f.crc.mask, partition) - } - - user, file := "unknown", id // default if no user in id - if elems := strings.Split(id, "/"); len(elems) == 2 { - user, file = elems[0], elems[1] // user in id - } - - if f.Partitions == 0 { - return path.Join(base, user, file) // avoid partition directory if 0 Partitions - } - - return path.Join(base, user, partition(id), file) + }) } // ExtractPictures gets list of images from the doc html and convert from urls to ids, i.e. user/pic.png -func ExtractPictures(commentHTML string, match string) (ids []string, err error) { +func (s *Service) ExtractPictures(commentHTML string, match string) (ids []string, err error) { doc, err := goquery.NewDocumentFromReader(strings.NewReader(commentHTML)) if err != nil { @@ -210,3 +64,18 @@ func ExtractPictures(commentHTML string, match string) (ids []string, err error) return result, nil } + +// Cleanup runs periodic cleanup with TTL. Blocking loop, should be called inside of goroutine by consumer +func (s *Service) Cleanup(ctx context.Context) { + log.Printf("[INFO] start pictures cleanup, staging ttl=%v", s.TTL) + + for { + select { + case <-ctx.Done(): + log.Printf("[INFO] cleanup terminated, %v", ctx.Err()) + return + case <-time.After(s.TTL / 2): + s.Store.Cleanup(ctx) + } + } +} diff --git a/backend/app/store/image/image_mock.go b/backend/app/store/image/image_mock.go new file mode 100644 index 00000000..5dc04453 --- /dev/null +++ b/backend/app/store/image/image_mock.go @@ -0,0 +1,92 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: image.go + +// Package image is a generated GoMock package. +package image + +import ( + context "context" + gomock "github.com/golang/mock/gomock" + io "io" + reflect "reflect" +) + +// MockStore is a mock of Store interface +type MockStore struct { + ctrl *gomock.Controller + recorder *MockStoreMockRecorder +} + +// MockStoreMockRecorder is the mock recorder for MockStore +type MockStoreMockRecorder struct { + mock *MockStore +} + +// NewMockStore creates a new mock instance +func NewMockStore(ctrl *gomock.Controller) *MockStore { + mock := &MockStore{ctrl: ctrl} + mock.recorder = &MockStoreMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockStore) EXPECT() *MockStoreMockRecorder { + return m.recorder +} + +// Save mocks base method +func (m *MockStore) Save(fileName, userID string, r io.Reader) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Save", fileName, userID, r) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Save indicates an expected call of Save +func (mr *MockStoreMockRecorder) Save(fileName, userID, r interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Save", reflect.TypeOf((*MockStore)(nil).Save), fileName, userID, r) +} + +// Commit mocks base method +func (m *MockStore) Commit(id string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Commit", id) + ret0, _ := ret[0].(error) + return ret0 +} + +// Commit indicates an expected call of Commit +func (mr *MockStoreMockRecorder) Commit(id interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Commit", reflect.TypeOf((*MockStore)(nil).Commit), id) +} + +// Load mocks base method +func (m *MockStore) Load(id string) (io.ReadCloser, int64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Load", id) + ret0, _ := ret[0].(io.ReadCloser) + ret1, _ := ret[1].(int64) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// Load indicates an expected call of Load +func (mr *MockStoreMockRecorder) Load(id interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Load", reflect.TypeOf((*MockStore)(nil).Load), id) +} + +// Cleanup mocks base method +func (m *MockStore) Cleanup(ctx context.Context) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Cleanup", ctx) +} + +// Cleanup indicates an expected call of Cleanup +func (mr *MockStoreMockRecorder) Cleanup(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Cleanup", reflect.TypeOf((*MockStore)(nil).Cleanup), ctx) +} diff --git a/backend/app/store/image/image_test.go b/backend/app/store/image/image_test.go index ef3ec135..44ced538 100644 --- a/backend/app/store/image/image_test.go +++ b/backend/app/store/image/image_test.go @@ -2,217 +2,34 @@ package image import ( "context" - "io/ioutil" - "math/rand" - "os" - "strconv" - "strings" "testing" "time" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestImage_Save(t *testing.T) { - svc, teardown := prepareImageTest(t) - defer teardown() - - id, err := svc.Save("file1.png", "user1", strings.NewReader("blah blah")) - assert.NoError(t, err) - assert.Contains(t, id, "user1/") - assert.Contains(t, id, ".png") - t.Log(id) - - img := svc.location(svc.Staging, id) - t.Log(img) - data, err := ioutil.ReadFile(img) - assert.NoError(t, err) - assert.Equal(t, "blah blah", string(data)) -} - -func TestImage_SaveAndCommit(t *testing.T) { - svc, teardown := prepareImageTest(t) - defer teardown() - - id, err := svc.Save("file1.png", "user1", strings.NewReader("blah blah")) - require.NoError(t, err) - err = svc.Commit(id) - require.NoError(t, err) - - imgStaging := svc.location(svc.Staging, id) - _, err = os.Stat(imgStaging) - assert.NotNil(t, err, "no file on staging anymore") - - img := svc.location(svc.Location, id) - t.Log(img) - data, err := ioutil.ReadFile(img) - assert.NoError(t, err) - assert.Equal(t, "blah blah", string(data)) -} - -func TestImage_SaveTooLarge(t *testing.T) { - svc, teardown := prepareImageTest(t) - defer teardown() - svc.MaxSize = 5 - _, err := svc.Save("blah_ff1.png", "user2", strings.NewReader("blah blah")) - assert.Error(t, err) - assert.Contains(t, err.Error(), "is too large") -} - -func TestImage_LoadAfterSave(t *testing.T) { - - svc, teardown := prepareImageTest(t) - defer teardown() - - id, err := svc.Save("blah_ff1.png", "user1", strings.NewReader("blah blah")) - assert.NoError(t, err) - t.Log(id) - - r, sz, err := svc.Load(id) - assert.NoError(t, err) - defer func() { assert.NoError(t, r.Close()) }() - data, err := ioutil.ReadAll(r) - assert.NoError(t, err) - assert.Equal(t, "blah blah", string(data)) - assert.Equal(t, int64(9), sz) - _, _, err = svc.Load("abcd") - assert.NotNil(t, err) -} - -func TestImage_LoadAfterCommit(t *testing.T) { - - svc, teardown := prepareImageTest(t) - defer teardown() - - id, err := svc.Save("blah_ff1.png", "user1", strings.NewReader("blah blah")) - assert.NoError(t, err) - t.Log(id) - err = svc.Commit(id) - require.NoError(t, err) - - r, sz, err := svc.Load(id) - assert.NoError(t, err) - defer func() { assert.NoError(t, r.Close()) }() - data, err := ioutil.ReadAll(r) - assert.NoError(t, err) - assert.Equal(t, "blah blah", string(data)) - assert.Equal(t, int64(9), sz) - _, _, err = svc.Load("abcd") - assert.NotNil(t, err) -} - -func TestImage_location(t *testing.T) { - tbl := []struct { - partitions int - id, res string - }{ - {10, "u1/abcdefg.png", "/tmp/u1/4/abcdefg.png"}, - {10, "abcdefe", "/tmp/unknown/1/abcdefe"}, - {10, "12345", "/tmp/unknown/9/12345"}, - {100, "12345", "/tmp/unknown/69/12345"}, - {100, "xyzz", "/tmp/unknown/58/xyzz"}, - {100, "6851dcde6024e03258a66705f29e14b506048c74.png", "/tmp/unknown/02/6851dcde6024e03258a66705f29e14b506048c74.png"}, - {5, "6851dcde6024e03258a66705f29e14b506048c74.png", "/tmp/unknown/2/6851dcde6024e03258a66705f29e14b506048c74.png"}, - {5, "xxxyz.png", "/tmp/unknown/0/xxxyz.png"}, - {0, "12345", "/tmp/unknown/12345"}, - } - for n, tt := range tbl { - t.Run(strconv.Itoa(n), func(t *testing.T) { - svc := FileSystem{Location: "/tmp", Partitions: tt.partitions} - assert.Equal(t, tt.res, svc.location("/tmp", tt.id)) - }) - } - - // generate random names and make sure partition never runs out of allowed - letterRunes := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") - randomID := func(n int) string { - b := make([]rune, n) - for i := range b { - b[i] = letterRunes[rand.Intn(len(letterRunes))] - } - return "user1" + "/" + string(b) - } - - svc := FileSystem{Location: "/tmp", Partitions: 10} - for i := 0; i < 1000; i++ { - v := randomID(rand.Intn(64)) - location := svc.location("/tmp", v) - elems := strings.Split(location, "/") - p, err := strconv.Atoi(elems[3]) - require.NoError(t, err, location) - assert.True(t, p >= 0 && p < 10) - } -} - -func TestImage_Cleanup(t *testing.T) { - svc, teardown := prepareImageTest(t) - defer teardown() - - save := func(file string, user string, content string) (path string) { - id, err := svc.Save(file, user, strings.NewReader(content)) - require.NoError(t, err) - img := svc.location(svc.Staging, id) - data, err := ioutil.ReadFile(img) - require.NoError(t, err) - require.Equal(t, content, string(data)) - return img - } - - // save 3 images to staging - img1 := save("blah_ff1.png", "user1", "blah blah1") - time.Sleep(100 * time.Millisecond) - img2 := save("blah_ff2.png", "user1", "blah blah2") - time.Sleep(100 * time.Millisecond) - img3 := save("blah_ff3.png", "user2", "blah blah3") - - svc.TTL = time.Millisecond * 300 - ctx, cancel := context.WithCancel(context.Background()) - go func() { - time.Sleep(1000 * time.Millisecond) - cancel() - }() - - svc.Cleanup(ctx) - - _, err := os.Stat(img1) - assert.NotNil(t, err, "no file on staging anymore") - _, err = os.Stat(img2) - assert.NotNil(t, err, "no file on staging anymore") - _, err = os.Stat(img3) - assert.NotNil(t, err, "no file on staging anymore") -} - -func TestExtractPictures(t *testing.T) { +func TestService_ExtractPictures(t *testing.T) { + svc := Service{} html := `blah foo xyz

123

` - ids, err := ExtractPictures(html, "/blah/") + ids, err := svc.ExtractPictures(html, "/blah/") require.NoError(t, err) assert.Equal(t, 2, len(ids), "two images") assert.Equal(t, "user1/pic1.png", ids[0]) assert.Equal(t, "user2/pic3.png", ids[1]) } -func prepareImageTest(t *testing.T) (svc FileSystem, teardown func()) { - loc, err := ioutil.TempDir("", "test_image_r42") - require.NoError(t, err, "failed to make temp dir") +func TestService_Cleanup(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() - staging, err := ioutil.TempDir("", "test_image_r42.staging") - require.NoError(t, err, "failed to make temp staging dir") + store := NewMockStore(ctrl) + store.EXPECT().Cleanup(gomock.Any()).Times(10) - svc = FileSystem{ - Location: loc, - Staging: staging, - Partitions: 100, - MaxSize: 50, - } - - teardown = func() { - defer func() { - assert.NoError(t, os.RemoveAll(loc)) - assert.NoError(t, os.RemoveAll(staging)) - }() - } - - return svc, teardown + svc := Service{Store: store, TTL: 100 * time.Millisecond} + ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*550) + defer cancel() + svc.Cleanup(ctx) } diff --git a/backend/app/store/service/service.go b/backend/app/store/service/service.go index 4878dae9..9bc355e2 100644 --- a/backend/app/store/service/service.go +++ b/backend/app/store/service/service.go @@ -129,8 +129,8 @@ func (s *DataStore) SetPin(locator store.Locator, commentID string, status bool) // Vote for comment by id and locator func (s *DataStore) Vote(locator store.Locator, commentID string, userID string, val bool) (comment store.Comment, err error) { - cLock := s.getsScopedLocks(locator.URL) // get lock for URL scope - cLock.Lock() // prevents race on voting + cLock := s.getScopedLocks(locator.URL) // get lock for URL scope + cLock.Lock() // prevents race on voting defer cLock.Unlock() comment, err = s.Get(locator, commentID) @@ -455,8 +455,8 @@ func (s *DataStore) upsAndDowns(c store.Comment) (ups, downs int) { return ups, downs } -// getsScopedLocks pull lock from the map if found or create a new one -func (s *DataStore) getsScopedLocks(id string) (lock sync.Locker) { +// getScopedLocks pull lock from the map if found or create a new one +func (s *DataStore) getScopedLocks(id string) (lock sync.Locker) { s.scopedLocks.Do(func() { s.scopedLocks.locks = map[string]sync.Locker{} }) s.scopedLocks.Lock() diff --git a/backend/go.mod b/backend/go.mod index 65c95d3d..55c2b513 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -20,6 +20,7 @@ require ( github.com/go-pkgz/repeater v1.1.1 github.com/go-pkgz/rest v1.4.0 github.com/go-pkgz/syncs v1.1.0 + github.com/golang/mock v1.2.0 github.com/google/uuid v0.0.0-20161128191214-064e2069ce9c github.com/gorilla/feeds v1.1.0 github.com/hashicorp/errwrap v1.0.0 // indirect diff --git a/backend/go.sum b/backend/go.sum index 97fddbf8..b7de4812 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -43,6 +43,8 @@ github.com/go-pkgz/rest v1.4.0 h1:xNkdMjEL2rNZSHouWjFTH22ncaZ77fopm34RN+eXAwk= github.com/go-pkgz/rest v1.4.0/go.mod h1:COazNj35u3RXAgQNBr6neR599tYP3URiOpsu9p0rOtk= github.com/go-pkgz/syncs v1.1.0 h1:k+dTyUZs1JHsYzo2tuUNrnW0OCwuGuS6ozfXHVspjSY= github.com/go-pkgz/syncs v1.1.0/go.mod h1:bt9lxWRRJ9vOCMGc8Big8ttjYHLKP88ofj1y38UlaHE= +github.com/golang/mock v1.2.0 h1:28o5sBqPkBsMGnC6b4MvE2TzSr5/AT4c/1fLqVGIwlk= +github.com/golang/mock v1.2.0/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/google/uuid v0.0.0-20161128191214-064e2069ce9c h1:jWtZjFEUE/Bz0IeIhqCnyZ3HG6KRXSntXe4SjtuTH7c= @@ -78,6 +80,7 @@ github.com/rakyll/statik v0.1.3/go.mod h1:OEi9wJV/fMUAGx1eNjq75DKDsJVuEv1U0oYdX6 github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo= github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= diff --git a/backend/vendor/modules.txt b/backend/vendor/modules.txt index d4dad4b1..2a3a7886 100644 --- a/backend/vendor/modules.txt +++ b/backend/vendor/modules.txt @@ -52,6 +52,8 @@ github.com/go-pkgz/rest github.com/go-pkgz/rest/logger # github.com/go-pkgz/syncs v1.1.0 github.com/go-pkgz/syncs +# github.com/golang/mock v1.2.0 +github.com/golang/mock/gomock # github.com/golang/protobuf v1.2.0 github.com/golang/protobuf/proto # github.com/google/uuid v0.0.0-20161128191214-064e2069ce9c