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)