From 58ec50e61348c3e4ccae455c3f80d16ef2c77646 Mon Sep 17 00:00:00 2001 From: Umputun Date: Sat, 23 Mar 2019 18:33:19 -0500 Subject: [PATCH] wire image submit to store.Service --- backend/app/cmd/server.go | 32 +++++++++++++++------------ backend/app/rest/api/rest.go | 2 +- backend/app/store/image/image.go | 25 +++++++++++++-------- backend/app/store/image/image_mock.go | 11 +++++---- backend/app/store/image/image_test.go | 32 ++++++++++++++++++++++++--- backend/app/store/service/service.go | 7 ++++++ 6 files changed, 78 insertions(+), 31 deletions(-) diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index f5292400..8fa4ef8a 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.Store + imageService *image.Service terminated chan struct{} } @@ -232,6 +232,11 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) { return nil, errors.Wrap(err, "failed to make admin store") } + imageService, err := s.makePicturesStore() + if err != nil { + return nil, errors.Wrap(err, "failed to make pictures store") + } + dataService := &service.DataStore{ Interface: storeEngine, EditDuration: s.EditDuration, @@ -239,6 +244,7 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) { MaxCommentSize: s.MaxCommentSize, MaxVotes: s.MaxVotes, PositiveScore: s.PositiveScore, + ImageService: imageService, TitleExtractor: service.NewTitleExtractor(http.Client{Timeout: time.Second * 5}), RestrictedWordsMatcher: service.NewRestrictedWordsMatcher(service.StaticRestrictedWordsLister{Words: s.RestrictedWords}), } @@ -274,11 +280,6 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) { imgProxy := &proxy.Image{Enabled: s.ImageProxy, RoutePath: "/api/v1/img", RemarkURL: s.RemarkURL} commentFormatter := store.NewCommentFormatter(imgProxy) - pictStore, err := s.makePicturesStore() - if err != nil { - return nil, errors.Wrap(err, "failed to make pictures store") - } - sslConfig, err := s.makeSSLConfig() if err != nil { return nil, errors.Wrap(err, "failed to make config of ssl server params") @@ -299,7 +300,7 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) { NotifyService: notifyService, SSLConfig: sslConfig, UpdateLimiter: s.UpdateLimit, - ImageService: pictStore, + ImageService: imageService, } srv.ScoreThresholds.Low, srv.ScoreThresholds.Critical = s.LowScore, s.CriticalScore @@ -322,7 +323,7 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) { dataService: dataService, avatarStore: avatarStore, notifyService: notifyService, - imageService: pictStore, + imageService: imageService, terminated: make(chan struct{}), }, nil } @@ -434,17 +435,20 @@ func (s *ServerCommand) makeAvatarStore() (avatar.Store, error) { return nil, errors.Errorf("unsupported avatar store type %s", s.Avatar.Type) } -func (s *ServerCommand) makePicturesStore() (image.Store, error) { +func (s *ServerCommand) makePicturesStore() (*image.Service, error) { switch s.Image.Type { case "fs": if err := makeDirs(s.Image.FS.Path); err != nil { return nil, err } - return &image.FileSystem{ - Location: s.Image.FS.Path, - Staging: s.Image.FS.Staging, - Partitions: s.Image.FS.Partitions, - MaxSize: s.Image.MaxSize, + 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, + }, + TTL: s.EditDuration + time.Second, // add extra second to image TTL for staging }, 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 8625d504..7844217e 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.Store + ImageService *image.Service WebRoot string RemarkURL string diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index e0c95224..08097f00 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -22,18 +22,23 @@ 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 + Cleanup(ctx context.Context, ttl time.Duration) error // run removal loop for old images on staging } // 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 + TTL time.Duration // for how long file allowed on staging + ImageAPI string // image api matching path } // Submit multiple ids for delayed commit -func (s *Service) Submit(ids []string, delay time.Duration) { - time.AfterFunc(delay, func() { +func (s *Service) Submit(ids []string) { + if len(ids) == 0 { + return + } + + time.AfterFunc(s.TTL, func() { for _, id := range ids { if err := s.Commit(id); err != nil { log.Printf("[WARN] failed to commit image %s", id) @@ -43,16 +48,16 @@ func (s *Service) Submit(ids []string, delay time.Duration) { } // ExtractPictures gets list of images from the doc html and convert from urls to ids, i.e. user/pic.png -func (s *Service) ExtractPictures(commentHTML string, match string) (ids []string, err error) { +func (s *Service) ExtractPictures(commentHTML string) (ids []string, err error) { doc, err := goquery.NewDocumentFromReader(strings.NewReader(commentHTML)) if err != nil { return nil, errors.Wrap(err, "can't create document") } result := []string{} - doc.Find("img").Each(func(i int, s *goquery.Selection) { - if im, ok := s.Attr("src"); ok { - if strings.Contains(im, match) { + doc.Find("img").Each(func(i int, sl *goquery.Selection) { + if im, ok := sl.Attr("src"); ok { + if strings.Contains(im, s.ImageAPI) { elems := strings.Split(im, "/") if len(elems) >= 2 { id := elems[len(elems)-2] + "/" + elems[len(elems)-1] @@ -75,7 +80,9 @@ func (s *Service) Cleanup(ctx context.Context) { log.Printf("[INFO] cleanup terminated, %v", ctx.Err()) return case <-time.After(s.TTL / 2): - s.Store.Cleanup(ctx) + if err := s.Store.Cleanup(ctx, s.TTL); err != nil { + log.Printf("[WARN] failed to cleanup, %v", err) + } } } } diff --git a/backend/app/store/image/image_mock.go b/backend/app/store/image/image_mock.go index 5dc04453..0049e558 100644 --- a/backend/app/store/image/image_mock.go +++ b/backend/app/store/image/image_mock.go @@ -9,6 +9,7 @@ import ( gomock "github.com/golang/mock/gomock" io "io" reflect "reflect" + time "time" ) // MockStore is a mock of Store interface @@ -80,13 +81,15 @@ func (mr *MockStoreMockRecorder) Load(id interface{}) *gomock.Call { } // Cleanup mocks base method -func (m *MockStore) Cleanup(ctx context.Context) { +func (m *MockStore) Cleanup(ctx context.Context, ttl time.Duration) error { m.ctrl.T.Helper() - m.ctrl.Call(m, "Cleanup", ctx) + ret := m.ctrl.Call(m, "Cleanup", ctx, ttl) + ret0, _ := ret[0].(error) + return ret0 } // Cleanup indicates an expected call of Cleanup -func (mr *MockStoreMockRecorder) Cleanup(ctx interface{}) *gomock.Call { +func (mr *MockStoreMockRecorder) Cleanup(ctx, ttl interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Cleanup", reflect.TypeOf((*MockStore)(nil).Cleanup), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Cleanup", reflect.TypeOf((*MockStore)(nil).Cleanup), ctx, ttl) } diff --git a/backend/app/store/image/image_test.go b/backend/app/store/image/image_test.go index 44ced538..ad12dae5 100644 --- a/backend/app/store/image/image_test.go +++ b/backend/app/store/image/image_test.go @@ -11,22 +11,48 @@ import ( ) func TestService_ExtractPictures(t *testing.T) { - svc := Service{} + svc := Service{ImageAPI: "/blah/"} html := `blah foo xyz

123

` - ids, err := svc.ExtractPictures(html, "/blah/") + ids, err := svc.ExtractPictures(html) 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 TestService_Submit(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + store := NewMockStore(ctrl) + + store.EXPECT().Commit(gomock.Any()).Times(5) // all 5 should be committed + svc := Service{Store: store, ImageAPI: "/blah/", TTL: time.Millisecond * 100} + svc.Submit([]string{"id1", "id2", "id3"}) + svc.Submit([]string{"id4", "id5"}) + svc.Submit(nil) + time.Sleep(time.Millisecond * 500) +} + +func TestService_SubmitDelay(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + store := NewMockStore(ctrl) + + store.EXPECT().Commit(gomock.Any()).Times(3) // first batch should be committed + svc := Service{Store: store, ImageAPI: "/blah/", TTL: time.Millisecond * 100} + svc.Submit([]string{"id1", "id2", "id3"}) + time.Sleep(150 * time.Millisecond) // let first batch to pass TTL + svc.Submit([]string{"id4", "id5"}) + svc.Submit(nil) +} + func TestService_Cleanup(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() store := NewMockStore(ctrl) - store.EXPECT().Cleanup(gomock.Any()).Times(10) + store.EXPECT().Cleanup(gomock.Any(), gomock.Any()).Times(10) svc := Service{Store: store, TTL: 100 * time.Millisecond} ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*550) diff --git a/backend/app/store/service/service.go b/backend/app/store/service/service.go index 9bc355e2..d65f5cbc 100644 --- a/backend/app/store/service/service.go +++ b/backend/app/store/service/service.go @@ -12,6 +12,7 @@ import ( multierror "github.com/hashicorp/go-multierror" cache "github.com/patrickmn/go-cache" "github.com/pkg/errors" + "github.com/umputun/remark/backend/app/store/image" "github.com/umputun/remark/backend/app/store" "github.com/umputun/remark/backend/app/store/admin" @@ -28,6 +29,7 @@ type DataStore struct { PositiveScore bool TitleExtractor *TitleExtractor RestrictedWordsMatcher *RestrictedWordsMatcher + ImageService *image.Service // granular locks scopedLocks struct { @@ -90,6 +92,11 @@ func (s *DataStore) Create(comment store.Comment) (commentID string, err error) comment.PostTitle = title }() + imgIds, err := s.ImageService.ExtractPictures(comment.Text) + if err != nil { + return "", errors.Wrap(err, "failed to prepare extract pictures") + } + s.ImageService.Submit(imgIds) // submit images commit, delayed by EditDuration return s.Interface.Create(comment) }