diff --git a/backend/_example/memory_store/accessor/image.go b/backend/_example/memory_store/accessor/image.go index a5757f5e..c3ce0b21 100644 --- a/backend/_example/memory_store/accessor/image.go +++ b/backend/_example/memory_store/accessor/image.go @@ -70,6 +70,17 @@ func (m *MemImage) Load(id string) ([]byte, error) { return img, nil } +// Delete image by ID +func (m *MemImage) Delete(id string) error { + m.mu.Lock() + // delete key from permanent and staging storage + delete(m.images, id) + delete(m.insertTime, id) + delete(m.imagesStaging, id) + m.mu.Unlock() + return nil +} + // Commit moves image from staging to permanent func (m *MemImage) Commit(id string) error { m.mu.RLock() diff --git a/backend/_example/memory_store/accessor/image_test.go b/backend/_example/memory_store/accessor/image_test.go index 42722754..f0df173b 100644 --- a/backend/_example/memory_store/accessor/image_test.go +++ b/backend/_example/memory_store/accessor/image_test.go @@ -18,7 +18,7 @@ import ( ) // gopher png for test, from https://golang.org/src/image/png/example_test.go -const gopher = "iVBORw0KGgoAAAANSUhEUgAAAEsAAAA8CAAAAAALAhhPAAAFfUlEQVRYw62XeWwUVRzHf2" + +const rawGopher = "iVBORw0KGgoAAAANSUhEUgAAAEsAAAA8CAAAAAALAhhPAAAFfUlEQVRYw62XeWwUVRzHf2" + "+OPbo9d7tsWyiyaZti6eWGAhISoIGKECEKCAiJJkYTiUgTMYSIosYYBBIUIxoSPIINEBDi2VhwkQrVsj1ESgu9doHWdrul7ba" + "73WNm3vOPtsseM9MdwvvrzTs+8/t95ze/33sI5BqiabU6m9En8oNjduLnAEDLUsQXFF8tQ5oxK3vmnNmDSMtrncks9Hhtt" + "/qeWZapHb1ha3UqYSWVl2ZmpWgaXMXGohQAvmeop3bjTRtv6SgaK/Pb9/bFzUrYslbFAmHPp+3WhAYdr+7GN/YnpN46Opv55VDs" + @@ -38,7 +38,9 @@ const gopher = "iVBORw0KGgoAAAANSUhEUgAAAEsAAAA8CAAAAAALAhhPAAAFfUlEQVRYw62XeWwU "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 gopherPNG() io.Reader { + return base64.NewDecoder(base64.StdEncoding, strings.NewReader(rawGopher)) +} func TestMemImage_LoadAfterSave(t *testing.T) { svc := NewMemImageStore() @@ -57,7 +59,8 @@ func TestMemImage_LoadAfterSave(t *testing.T) { assert.NoError(t, err) assert.Equal(t, gopher, img) - svc.ResetCleanupTimer(id) + err = svc.ResetCleanupTimer(id) + assert.NoError(t, err) err = svc.Commit(id) assert.NoError(t, err) @@ -70,6 +73,26 @@ func TestMemImage_LoadAfterSave(t *testing.T) { assert.Equal(t, gopher, img) } +func TestMemImage_LoadAfterDelete(t *testing.T) { + svc := NewMemImageStore() + gopher, err := io.ReadAll(gopherPNG()) + assert.NoError(t, err) + + id := "test_img" + err = svc.Save(id, gopher) + assert.NoError(t, err) + + err = svc.Delete(id) + assert.NoError(t, err) + + img, err := svc.Load(id) + assert.EqualError(t, err, "image test_img not found") + assert.Empty(t, img) + + err = svc.ResetCleanupTimer(id) + assert.EqualError(t, err, "image test_img not found") +} + func TestMemImage_CommitFail(t *testing.T) { svc := NewMemImageStore() err := svc.Commit("test_id") diff --git a/backend/_example/memory_store/server/image.go b/backend/_example/memory_store/server/image.go index f5e0f420..793deada 100644 --- a/backend/_example/memory_store/server/image.go +++ b/backend/_example/memory_store/server/image.go @@ -35,7 +35,6 @@ func (s *RPC) imgResetClnTimerHndl(id uint64, params json.RawMessage) (rr jrpc.R } err := s.img.ResetCleanupTimer(fileID) return jrpc.EncodeResponse(id, nil, err) - } func (s *RPC) imgLoadHndl(id uint64, params json.RawMessage) (rr jrpc.Response) { @@ -47,6 +46,16 @@ func (s *RPC) imgLoadHndl(id uint64, params json.RawMessage) (rr jrpc.Response) return jrpc.EncodeResponse(id, value, err) } +func (s *RPC) imgDeleteHndl(id uint64, params json.RawMessage) (rr jrpc.Response) { + var fileID string + if err := json.Unmarshal(params, &fileID); err != nil { + return jrpc.Response{Error: err.Error()} + } + err := s.img.Delete(fileID) + return jrpc.EncodeResponse(id, nil, err) + +} + func (s *RPC) imgCommitHndl(id uint64, params json.RawMessage) (rr jrpc.Response) { var fileID string if err := json.Unmarshal(params, &fileID); err != nil { diff --git a/backend/_example/memory_store/server/image_test.go b/backend/_example/memory_store/server/image_test.go index 0f70fd0b..79f133ea 100644 --- a/backend/_example/memory_store/server/image_test.go +++ b/backend/_example/memory_store/server/image_test.go @@ -158,4 +158,9 @@ func TestRPC_imgInfoHndl(t *testing.T) { info, err = ri.Info() assert.NoError(t, err) assert.False(t, info.FirstStagingImageTS.IsZero()) + + err = ri.Delete("test_img") + assert.NoError(t, err) + _, err = ri.Load("test_img") + assert.EqualError(t, err, "image test_img not found") } diff --git a/backend/_example/memory_store/server/rpc.go b/backend/_example/memory_store/server/rpc.go index 411c446b..97e6eca4 100644 --- a/backend/_example/memory_store/server/rpc.go +++ b/backend/_example/memory_store/server/rpc.go @@ -60,6 +60,7 @@ func (s *RPC) addHandlers() { "save_with_id": s.imgSaveWithIDHndl, "reset_cleanup_timer": s.imgResetClnTimerHndl, "load": s.imgLoadHndl, + "delete": s.imgDeleteHndl, "commit": s.imgCommitHndl, "cleanup": s.imgCleanupHndl, "info": s.imgInfoHndl, diff --git a/backend/app/store/image/bolt_store.go b/backend/app/store/image/bolt_store.go index 9d9fe9b2..7b0014de 100644 --- a/backend/app/store/image/bolt_store.go +++ b/backend/app/store/image/bolt_store.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/binary" + "errors" "fmt" "time" @@ -118,6 +119,18 @@ func (b *Bolt) Load(id string) ([]byte, error) { return data, nil } +// Delete image from storage +func (b *Bolt) Delete(id string) error { + return b.db.Update(func(tx *bolt.Tx) error { + // deleting a non-existing key doesn't return an error, so joining errors from deleting an image + // from both buckets is safe and will return nil if there are no errors on the real delete + // or image is absent in both buckets + err := tx.Bucket([]byte(imagesBktName)).Delete([]byte(id)) + err = errors.Join(err, tx.Bucket([]byte(imagesStagedBktName)).Delete([]byte(id))) + return err + }) +} + // Cleanup runs scan of staging and removes old data based on ttl func (b *Bolt) Cleanup(_ context.Context, ttl time.Duration) error { return b.db.Update(func(tx *bolt.Tx) error { diff --git a/backend/app/store/image/bolt_store_test.go b/backend/app/store/image/bolt_store_test.go index d9977b19..232db45d 100644 --- a/backend/app/store/image/bolt_store_test.go +++ b/backend/app/store/image/bolt_store_test.go @@ -58,6 +58,36 @@ func TestBoltStore_LoadAfterSave(t *testing.T) { assert.Error(t, err) } +func TestBoltStore_LoadAfterDelete(t *testing.T) { + svc, teardown := prepareBoltImageStorageTest(t) + defer teardown() + + // delete image from permanent storage + id := "test_img" + err := svc.Save(id, gopherPNGBytes()) + assert.NoError(t, err) + + err = svc.Commit(id) + require.NoError(t, err) + + err = svc.Delete(id) + assert.NoError(t, err) + + _, err = svc.Load(id) + assert.Error(t, err) + + // delete staging image + id = "staging_img" + err = svc.Save(id, gopherPNGBytes()) + assert.NoError(t, err) + + err = svc.Delete(id) + assert.NoError(t, err) + + _, err = svc.Load(id) + assert.Error(t, err) +} + func TestBoltStore_Cleanup(t *testing.T) { svc, teardown := prepareBoltImageStorageTest(t) defer teardown() diff --git a/backend/app/store/image/fs_store.go b/backend/app/store/image/fs_store.go index 09ca5e78..cf9a3907 100644 --- a/backend/app/store/image/fs_store.go +++ b/backend/app/store/image/fs_store.go @@ -23,7 +23,8 @@ type FileSystem struct { Staging string Partitions int - crc struct { + moveLock sync.Mutex // needed only for deleting images or moving them from staging to permanent storage + crc struct { *crc64.Table sync.Once mask string @@ -50,6 +51,8 @@ func (f *FileSystem) Save(id string, img []byte) error { // Commit file stored in staging location by moving it to permanent location func (f *FileSystem) Commit(id string) error { + f.moveLock.Lock() + defer f.moveLock.Unlock() log.Printf("[DEBUG] Commit image %s", id) stagingImage, permImage := f.location(f.Staging, id), f.location(f.Location, id) @@ -107,11 +110,31 @@ func (f *FileSystem) Load(id string) ([]byte, error) { return io.ReadAll(fh) } +// Delete image from storage +func (f *FileSystem) Delete(id string) error { + f.moveLock.Lock() + defer f.moveLock.Unlock() + staging := f.location(f.Staging, id) + // file doesn't exist on staging, delete from permanent location + if _, err := os.Stat(staging); os.IsNotExist(err) { + file := f.location(f.Location, id) + e := os.Remove(file) + _ = os.Remove(path.Dir(file)) // try to remove directory + return e + } + // delete file from staging + err := os.Remove(staging) + _ = os.Remove(path.Dir(staging)) // try to remove directory + return err +} + // Cleanup runs scan of staging and removes old files based on ttl func (f *FileSystem) Cleanup(_ context.Context, ttl time.Duration) error { if _, err := os.Stat(f.Staging); os.IsNotExist(err) { return nil } + f.moveLock.Lock() + defer f.moveLock.Unlock() // we can ignore context as on local FS remove is relatively fast operation err := filepath.Walk(f.Staging, func(fpath string, info os.FileInfo, err error) error { diff --git a/backend/app/store/image/fs_store_test.go b/backend/app/store/image/fs_store_test.go index dbc88953..1c5e564a 100644 --- a/backend/app/store/image/fs_store_test.go +++ b/backend/app/store/image/fs_store_test.go @@ -131,6 +131,31 @@ func TestFsStore_LoadAfterCommit(t *testing.T) { assert.Error(t, err) } +func TestFsStore_LoadAfterDelete(t *testing.T) { + svc, teardown := prepareImageTest(t) + defer teardown() + + id := "test_img" + err := svc.Save(id, gopherPNGBytes()) + assert.NoError(t, err) + err = svc.Commit(id) + require.NoError(t, err) + err = svc.Delete(id) + require.NoError(t, err) + + _, err = svc.Load(id) + assert.Error(t, err) + + // create file on staging + err = svc.Save(id, gopherPNGBytes()) + assert.NoError(t, err) + err = svc.Delete(id) + require.NoError(t, err) + + _, err = svc.Load(id) + assert.Error(t, err) +} + func TestFsStore_location(t *testing.T) { tbl := []struct { partitions int diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index 29420629..647594c5 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -70,6 +70,7 @@ type Store interface { Info() (StoreInfo, error) // get meta information about storage Save(id string, img []byte) error // store image with passed id to staging Load(id string) ([]byte, error) // load image by ID + Delete(id string) error // delete image by ID ResetCleanupTimer(id string) error // resets cleanup timer for the image, called on comment preview Commit(id string) error // move image from staging to permanent @@ -212,6 +213,11 @@ func (s *Service) Load(id string) ([]byte, error) { return s.store.Load(id) } +// Delete wraps storage Delete function. +func (s *Service) Delete(id string) error { + return s.store.Delete(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) { id = path.Join(userID, guid()) diff --git a/backend/app/store/image/image_mock.go b/backend/app/store/image/image_mock.go index 18562dff..106eeb77 100644 --- a/backend/app/store/image/image_mock.go +++ b/backend/app/store/image/image_mock.go @@ -4,9 +4,9 @@ package image import ( - context "context" + "context" "sync" - time "time" + "time" ) // Ensure, that StoreMock does implement Store. @@ -25,6 +25,9 @@ var _ Store = &StoreMock{} // CommitFunc: func(id string) error { // panic("mock out the Commit method") // }, +// DeleteFunc: func(id string) error { +// panic("mock out the Delete method") +// }, // InfoFunc: func() (StoreInfo, error) { // panic("mock out the Info method") // }, @@ -50,6 +53,9 @@ type StoreMock struct { // CommitFunc mocks the Commit method. CommitFunc func(id string) error + // DeleteFunc mocks the Delete method. + DeleteFunc func(id string) error + // InfoFunc mocks the Info method. InfoFunc func() (StoreInfo, error) @@ -76,6 +82,11 @@ type StoreMock struct { // ID is the id argument value. ID string } + // Delete holds details about calls to the Delete method. + Delete []struct { + // ID is the id argument value. + ID string + } // Info holds details about calls to the Info method. Info []struct { } @@ -99,6 +110,7 @@ type StoreMock struct { } lockCleanup sync.RWMutex lockCommit sync.RWMutex + lockDelete sync.RWMutex lockInfo sync.RWMutex lockLoad sync.RWMutex lockResetCleanupTimer sync.RWMutex @@ -173,6 +185,38 @@ func (mock *StoreMock) CommitCalls() []struct { return calls } +// Delete calls DeleteFunc. +func (mock *StoreMock) Delete(id string) error { + if mock.DeleteFunc == nil { + panic("StoreMock.DeleteFunc: method is nil but Store.Delete was just called") + } + callInfo := struct { + ID string + }{ + ID: id, + } + mock.lockDelete.Lock() + mock.calls.Delete = append(mock.calls.Delete, callInfo) + mock.lockDelete.Unlock() + return mock.DeleteFunc(id) +} + +// DeleteCalls gets all the calls that were made to Delete. +// Check the length with: +// +// len(mockedStore.DeleteCalls()) +func (mock *StoreMock) DeleteCalls() []struct { + ID string +} { + var calls []struct { + ID string + } + mock.lockDelete.RLock() + calls = mock.calls.Delete + mock.lockDelete.RUnlock() + return calls +} + // Info calls InfoFunc. func (mock *StoreMock) Info() (StoreInfo, error) { if mock.InfoFunc == nil { diff --git a/backend/app/store/image/remote_store.go b/backend/app/store/image/remote_store.go index e7cec50e..bdb096ca 100644 --- a/backend/app/store/image/remote_store.go +++ b/backend/app/store/image/remote_store.go @@ -41,6 +41,12 @@ func (r *RPC) Load(id string) ([]byte, error) { return io.ReadAll(base64.NewDecoder(base64.StdEncoding, strings.NewReader(rawImg))) } +// Delete image from storage +func (r *RPC) Delete(id string) error { + _, err := r.Call("image.delete", id) + return err +} + // Commit file stored in staging location by moving it to permanent location func (r *RPC) Commit(id string) error { _, err := r.Call("image.commit", id) diff --git a/backend/app/store/image/remote_store_test.go b/backend/app/store/image/remote_store_test.go index 8037b884..cd5d3de0 100644 --- a/backend/app/store/image/remote_store_test.go +++ b/backend/app/store/image/remote_store_test.go @@ -41,6 +41,18 @@ func TestRemote_Load(t *testing.T) { assert.Equal(t, gopherPNGBytes(), res) } +func TestRemote_Delete(t *testing.T) { + ts := testServer(t, `{"method":"image.delete","params":"54321","id":1}`, `{}`) + defer ts.Close() + c := RPC{Client: jrpc.Client{API: ts.URL, Client: http.Client{}}} + + var a Store = &c + _ = a + + err := c.Delete("54321") + assert.NoError(t, err) +} + func TestRemote_Commit(t *testing.T) { ts := testServer(t, `{"method":"image.commit","params":"gopher_id","id":1}`, `{"id":1}`) defer ts.Close() diff --git a/backend/app/store/service/service.go b/backend/app/store/service/service.go index b6a8a045..48e722c9 100644 --- a/backend/app/store/service/service.go +++ b/backend/app/store/service/service.go @@ -5,6 +5,7 @@ package service import ( "fmt" "math" + "slices" "sort" "strings" "sync" @@ -796,6 +797,34 @@ func (s *DataStore) Delete(locator store.Locator, commentID string, mode store.D s.repliesCache.Delete(commentID) s.repliesCache.Delete(comment.ParentID) } + + // delete images from the comment if they are not reused elsewhere in comments to the same page + idsFn := func() []string { // get IDs of all images from the same URL to verify if image from deleted comment was reused + comments, e := s.Engine.Find(engine.FindRequest{Locator: locator}) + if e != nil { + log.Printf("[WARN] can't get comments %s text for deleted comment image check, %v", comment.ID, err) + return nil + } + var imgIDs = []string{} + for _, cc := range comments { + // exclude the comment we are deleting + if cc.ID != commentID { + imgIDs = append(imgIDs, s.ImageService.ExtractPictures(cc.Text)...) + } + } + return imgIDs + } + commentImgIDs := s.ImageService.ExtractPictures(comment.Text) + pageImgIDs := idsFn() + for _, id := range commentImgIDs { + if !slices.Contains(pageImgIDs, id) { + if err := s.ImageService.Delete(id); err != nil { + log.Printf("[WARN] failed to delete image %s on comment %s deletion, %v", id, commentID, err) + } + } + } + log.Printf("[ERROR] commentImgIDs: %v, pageImgIDs: %v", commentImgIDs, pageImgIDs) + req := engine.DeleteRequest{Locator: locator, CommentID: commentID, DeleteMode: mode} return s.Engine.Delete(req) } diff --git a/backend/app/store/service/service_test.go b/backend/app/store/service/service_test.go index 6fe75b39..258953b8 100644 --- a/backend/app/store/service/service_test.go +++ b/backend/app/store/service/service_test.go @@ -1406,6 +1406,80 @@ func TestService_Delete(t *testing.T) { assert.NoError(t, err) } +func TestService_deleteImagesOnCommentDelete(t *testing.T) { + lgr.Setup(lgr.Debug, lgr.CallerFile, lgr.CallerFunc) + + mockStore := image.StoreMock{ + DeleteFunc: func(id string) error { return nil }, + CommitFunc: func(id string) error { return nil }, + ResetCleanupTimerFunc: func(id string) error { return nil }, + } + imgSvc := image.NewService(&mockStore, + image.ServiceParams{ + EditDuration: 50 * time.Millisecond, + ImageAPI: "/images/dev/", + ProxyAPI: "/non_existent", + }) + defer imgSvc.Close(context.TODO()) + + // two comments for https://radio-t.com + eng, teardown := prepStoreEngine(t) + defer teardown() + b := DataStore{Engine: eng, EditDuration: 50 * time.Millisecond, + AdminStore: admin.NewStaticKeyStore("secret 123"), ImageService: imgSvc} + + c := store.Comment{ + ID: "id-22", + Text: `some text xx `, + Timestamp: time.Date(2017, 12, 20, 15, 18, 22, 0, time.Local), + Locator: store.Locator{URL: "https://radio-t.com", SiteID: "radio-t"}, + User: store.User{ID: "user1", Name: "user name"}, + } + _, err := b.Engine.Create(c) // create directly with engine, doesn't call submitImages + assert.NoError(t, err) + b.submitImages(c) + // reply to the first comment with one new image and one existing one + c = store.Comment{ + ID: "id-23", + ParentID: "id-22", + Text: `some text xx `, + Locator: store.Locator{URL: "https://radio-t.com", SiteID: "radio-t"}, + User: store.User{ID: "user1", Name: "user name"}, + } + _, err = b.Engine.Create(c) // create directly with engine, doesn't call submitImages + assert.NoError(t, err) + b.submitImages(c) + + // verify that images are in staging store + assert.Equal(t, 4, len(mockStore.ResetCleanupTimerCalls())) + assert.Equal(t, "dev/pic1.png", mockStore.ResetCleanupTimerCalls()[0].ID) + assert.Equal(t, "dev/pic2.png", mockStore.ResetCleanupTimerCalls()[1].ID) + assert.Equal(t, "dev/pic2.png", mockStore.ResetCleanupTimerCalls()[2].ID) + assert.Equal(t, "dev/pic3.png", mockStore.ResetCleanupTimerCalls()[3].ID) + time.Sleep(b.EditDuration + 100*time.Millisecond) + // verify that they got into the main store + assert.Equal(t, 4, len(mockStore.CommitCalls())) + assert.Equal(t, "dev/pic1.png", mockStore.CommitCalls()[0].ID) + assert.Equal(t, "dev/pic2.png", mockStore.CommitCalls()[1].ID) + assert.Equal(t, "dev/pic2.png", mockStore.CommitCalls()[2].ID) + assert.Equal(t, "dev/pic3.png", mockStore.CommitCalls()[3].ID) + + // delete the first comment + err = b.Delete(store.Locator{URL: "https://radio-t.com", SiteID: "radio-t"}, "id-22", store.SoftDelete) + assert.NoError(t, err) + // verify that images are deleted from the main store + assert.Equal(t, 1, len(mockStore.DeleteCalls())) + assert.Equal(t, "dev/pic1.png", mockStore.DeleteCalls()[0].ID) + + // delete the second comment + err = b.Delete(store.Locator{URL: "https://radio-t.com", SiteID: "radio-t"}, "id-23", store.SoftDelete) + assert.NoError(t, err) + // verify that images are deleted from the main store + assert.Equal(t, 3, len(mockStore.DeleteCalls())) + assert.Equal(t, "dev/pic2.png", mockStore.DeleteCalls()[1].ID) + assert.Equal(t, "dev/pic3.png", mockStore.DeleteCalls()[2].ID) +} + // DeleteUser removes all comments from user func TestService_DeleteUser(t *testing.T) { // two comments for https://radio-t.com, no reply