delete aproved user from the persistent storage too

app/bot/spam.go

@@ -19,11 +19,13 @@ import (
 )
 
 //go:generate moq --out mocks/detector.go --pkg mocks --skip-ensure --with-resets . Detector
+//go:generate moq --out mocks/approved_store.go --pkg mocks --skip-ensure --with-resets . ApprovedUsersStore
 
 // SpamFilter bot checks if a user is a spammer using lib.Detector
 // Reloads spam samples, stop words and excluded tokens on file change.
 type SpamFilter struct {
 	Detector
+	ApprovedUsersStore
 	params SpamConfig
 }
 
@@ -58,9 +60,14 @@ type Detector interface {
 	ApprovedUsers() (res []string)
 }
 
+// ApprovedUsersStore is a storage interface for approved users.
+type ApprovedUsersStore interface {
+	Delete(id string) error
+}
+
 // NewSpamFilter creates new spam filter
-func NewSpamFilter(ctx context.Context, detector Detector, params SpamConfig) *SpamFilter {
-	res := &SpamFilter{Detector: detector, params: params}
+func NewSpamFilter(ctx context.Context, detector Detector, aStore ApprovedUsersStore, params SpamConfig) *SpamFilter {
+	res := &SpamFilter{Detector: detector, params: params, ApprovedUsersStore: aStore}
 	go func() {
 		if err := res.watch(ctx, params.WatchDelay); err != nil {
 			log.Printf("[WARN] samples file watcher failed: %v", err)
@@ -134,6 +141,9 @@ func (s *SpamFilter) RemoveApprovedUsers(id int64, ids ...int64) {
 	sids := make([]string, len(combinedIDs))
 	for i, id := range combinedIDs {
 		sids[i] = strconv.FormatInt(id, 10)
+		if err := s.ApprovedUsersStore.Delete(sids[i]); err != nil {
+			log.Printf("[WARN] failed to delete approved user from storage %q: %v", sids[i], err)
+		}
 	}
 	s.Detector.RemoveApprovedUsers(sids...)
 }

app/bot/spam_test.go

@@ -31,7 +31,7 @@ func TestSpamFilter_OnMessage(t *testing.T) {
 	}
 
 	t.Run("spam detected", func(t *testing.T) {
-		s := NewSpamFilter(ctx, det, SpamConfig{SpamMsg: "detected", SpamDryMsg: "detected dry"})
+		s := NewSpamFilter(ctx, det, nil, SpamConfig{SpamMsg: "detected", SpamDryMsg: "detected dry"})
 		resp := s.OnMessage(Message{Text: "spam", From: User{ID: 1, Username: "john"}})
 		assert.Equal(t, Response{Text: `detected: "john" (1)`, Send: true, BanInterval: PermanentBanDuration,
 			User: User{ID: 1, Username: "john"}, DeleteReplyTo: true,
@@ -40,15 +40,15 @@ func TestSpamFilter_OnMessage(t *testing.T) {
 	})
 
 	t.Run("spam detected, dry", func(t *testing.T) {
-		s := NewSpamFilter(ctx, det, SpamConfig{SpamMsg: "detected", SpamDryMsg: "detected dry", Dry: true})
+		s := NewSpamFilter(ctx, det, nil, SpamConfig{SpamMsg: "detected", SpamDryMsg: "detected dry", Dry: true})
 		resp := s.OnMessage(Message{Text: "spam", From: User{ID: 1, Username: "john"}})
 		assert.Equal(t, `detected dry: "john" (1)`, resp.Text)
 		assert.True(t, resp.Send)
 		assert.Equal(t, []lib.CheckResult{{Name: "something", Spam: true, Details: "some spam"}}, resp.CheckResults)
 	})
 
 	t.Run("ham detected", func(t *testing.T) {
-		s := NewSpamFilter(ctx, det, SpamConfig{SpamMsg: "detected", SpamDryMsg: "detected dry"})
+		s := NewSpamFilter(ctx, det, nil, SpamConfig{SpamMsg: "detected", SpamDryMsg: "detected dry"})
 		resp := s.OnMessage(Message{Text: "good", From: User{ID: 1, Username: "john"}})
 		assert.Equal(t, Response{CheckResults: []lib.CheckResult{{Name: "already approved", Spam: false, Details: "some ham"}}}, resp)
 	})
@@ -151,7 +151,7 @@ func TestSpamFilter_reloadSamples(t *testing.T) {
 				HamDynamicFile:     "optional",
 			}
 			tc.modify(&params)
-			s := NewSpamFilter(ctx, mockDirector, params)
+			s := NewSpamFilter(ctx, mockDirector, nil, params)
 
 			err = s.ReloadSamples()
 
@@ -201,7 +201,7 @@ func TestSpamFilter_watch(t *testing.T) {
 	_, err = os.Create(stopWordsFile)
 	require.NoError(t, err)
 
-	NewSpamFilter(ctx, mockDetector, SpamConfig{
+	NewSpamFilter(ctx, mockDetector, nil, SpamConfig{
 		ExcludedTokensFile: excludedTokensFile,
 		SpamSamplesFile:    spamSamplesFile,
 		HamSamplesFile:     hamSamplesFile,
@@ -270,7 +270,7 @@ func TestSpamFilter_WatchMultipleUpdates(t *testing.T) {
 	_, err = os.Create(stopWordsFile)
 	require.NoError(t, err)
 
-	NewSpamFilter(ctx, mockDetector, SpamConfig{
+	NewSpamFilter(ctx, mockDetector, nil, SpamConfig{
 		ExcludedTokensFile: excludedTokensFile,
 		SpamSamplesFile:    spamSamplesFile,
 		HamSamplesFile:     hamSamplesFile,
@@ -318,7 +318,7 @@ func TestSpamFilter_Update(t *testing.T) {
 		},
 	}
 
-	sf := NewSpamFilter(ctx, mockDetector, SpamConfig{})
+	sf := NewSpamFilter(ctx, mockDetector, nil, SpamConfig{})
 
 	t.Run("good update", func(t *testing.T) {
 		mockDetector.ResetCalls()
@@ -385,26 +385,27 @@ func TestSpamFilter_AddApprovedUsers(t *testing.T) {
 
 func TestSpamFilter_RemoveApprovedUsers(t *testing.T) {
 	mockDirector := &mocks.DetectorMock{RemoveApprovedUsersFunc: func(ids ...string) {}}
+	mockApprovedStore := &mocks.ApprovedUsersStoreMock{DeleteFunc: func(id string) error { return nil }}
 
 	t.Run("remove single approved user", func(t *testing.T) {
 		mockDirector.ResetCalls()
-		sf := SpamFilter{Detector: mockDirector}
+		sf := SpamFilter{Detector: mockDirector, ApprovedUsersStore: mockApprovedStore}
 		sf.RemoveApprovedUsers(1)
 		require.Equal(t, 1, len(mockDirector.RemoveApprovedUsersCalls()))
 		assert.Equal(t, []string{"1"}, mockDirector.RemoveApprovedUsersCalls()[0].Ids)
 	})
 
 	t.Run("remove multiple approved users", func(t *testing.T) {
 		mockDirector.ResetCalls()
-		sf := SpamFilter{Detector: mockDirector}
+		sf := SpamFilter{Detector: mockDirector, ApprovedUsersStore: mockApprovedStore}
 		sf.RemoveApprovedUsers(1, 2, 3)
 		require.Equal(t, 1, len(mockDirector.RemoveApprovedUsersCalls()))
 		assert.Equal(t, []string{"1", "2", "3"}, mockDirector.RemoveApprovedUsersCalls()[0].Ids)
 	})
 
 	t.Run("remove empty list of approved users", func(t *testing.T) {
 		mockDirector.ResetCalls()
-		sf := SpamFilter{Detector: mockDirector}
+		sf := SpamFilter{Detector: mockDirector, ApprovedUsersStore: mockApprovedStore}
 		sf.RemoveApprovedUsers(1, 2, 3)
 		require.Equal(t, 1, len(mockDirector.RemoveApprovedUsersCalls()))
 		assert.Equal(t, []string{"1", "2", "3"}, mockDirector.RemoveApprovedUsersCalls()[0].Ids)
@@ -428,7 +429,7 @@ func TestSpamFilter_DynamicSamples(t *testing.T) {
 	spamFile.Close()
 	hamFile.Close()
 
-	sf := NewSpamFilter(context.Background(), &mocks.DetectorMock{}, SpamConfig{
+	sf := NewSpamFilter(context.Background(), &mocks.DetectorMock{}, nil, SpamConfig{
 		SpamDynamicFile: spamFile.Name(),
 		HamDynamicFile:  hamFile.Name(),
 	})
@@ -495,7 +496,7 @@ func TestSpamFilter_RemoveDynamiSample(t *testing.T) {
 		_, err = hamFile.WriteString("ham1\nham2\n")
 		require.NoError(t, err)
 
-		return NewSpamFilter(context.Background(), mockDirector, SpamConfig{
+		return NewSpamFilter(context.Background(), mockDirector, nil, SpamConfig{
 				SpamDynamicFile:    spamFile.Name(),
 				HamDynamicFile:     hamFile.Name(),
 				SpamSamplesFile:    spamSamplesFile,

app/main.go

@@ -201,7 +201,7 @@ func execute(ctx context.Context, opts options) error {
 	}()
 
 	// make spam bot
-	spamBot, err := makeSpamBot(ctx, opts, detector)
+	spamBot, err := makeSpamBot(ctx, opts, detector, approvedUsersStore)
 	if err != nil {
 		return fmt.Errorf("can't make spam bot, %w", err)
 	}
@@ -391,7 +391,7 @@ func makeDetector(opts options) *lib.Detector {
 	return detector
 }
 
-func makeSpamBot(ctx context.Context, opts options, detector *lib.Detector) (*bot.SpamFilter, error) {
+func makeSpamBot(ctx context.Context, opts options, detector *lib.Detector, aStore *storage.ApprovedUsers) (*bot.SpamFilter, error) {
 	spamBotParams := bot.SpamConfig{
 		SpamSamplesFile:    filepath.Join(opts.Files.SamplesDataPath, samplesSpamFile),
 		HamSamplesFile:     filepath.Join(opts.Files.SamplesDataPath, samplesHamFile),
@@ -404,7 +404,7 @@ func makeSpamBot(ctx context.Context, opts options, detector *lib.Detector) (*bo
 		SpamDryMsg:         opts.Message.Dry,
 		Dry:                opts.Dry,
 	}
-	spamBot := bot.NewSpamFilter(ctx, detector, spamBotParams)
+	spamBot := bot.NewSpamFilter(ctx, detector, aStore, spamBotParams)
 	log.Printf("[DEBUG] spam bot config: %+v", spamBotParams)
 
 	if err := spamBot.ReloadSamples(); err != nil {

app/main_test.go

@@ -189,7 +189,7 @@ func Test_makeSpamBot(t *testing.T) {
 
 	t.Run("no options", func(t *testing.T) {
 		var opts options
-		_, err := makeSpamBot(ctx, opts, nil)
+		_, err := makeSpamBot(ctx, opts, nil, nil)
 		assert.Error(t, err)
 	})
 
@@ -207,8 +207,12 @@ func Test_makeSpamBot(t *testing.T) {
 		require.NoError(t, err)
 
 		opts.Files.SamplesDataPath = tmpDir
-
-		res, err := makeSpamBot(ctx, opts, makeDetector(opts))
+		detector := makeDetector(opts)
+		db, err := storage.NewSqliteDB(":memory:")
+		require.NoError(t, err)
+		store, err := loadApprovedUsers(db, detector)
+		require.NoError(t, err)
+		res, err := makeSpamBot(ctx, opts, detector, store)
 		assert.NoError(t, err)
 		assert.NotNil(t, res)
 	})

app/storage/approved_users.go

@@ -98,3 +98,17 @@ func (au *ApprovedUsers) Timestamp(id string) (time.Time, error) {
 	}
 	return timestamp, nil
 }
+
+// Delete deletes the given id from the storage
+func (au *ApprovedUsers) Delete(id string) error {
+	idVal, err := strconv.ParseInt(id, 10, 64)
+	if err != nil {
+		return fmt.Errorf("failed to parse id %s: %w", id, err)
+	}
+
+	_, err = au.db.Exec("DELETE FROM approved_users WHERE id = ?", idVal)
+	if err != nil {
+		return fmt.Errorf("failed to delete id %s: %w", id, err)
+	}
+	return nil
+}

app/storage/approved_users_test.go

@@ -134,3 +134,57 @@ func TestApprovedUsers_Timestamp(t *testing.T) {
 		assert.Error(t, err, "Should return error for non-existing ID")
 	})
 }
+
+func TestApprovedUsers_Delete(t *testing.T) {
+	db, err := sqlx.Open("sqlite", ":memory:")
+	require.NoError(t, err)
+	defer db.Close()
+
+	au, err := NewApprovedUsers(db)
+	require.NoError(t, err)
+
+	t.Run("existing ID", func(t *testing.T) {
+		err = au.Store([]string{"123"})
+		require.NoError(t, err)
+		err = au.Store([]string{"456"})
+		require.NoError(t, err)
+
+		err = au.Delete("123")
+		require.NoError(t, err)
+
+		var count int
+		err = db.Get(&count, "SELECT COUNT(*) FROM approved_users")
+		require.NoError(t, err)
+		assert.Equal(t, 1, count, "There should be only one record left")
+	})
+
+	t.Run("non-existing ID", func(t *testing.T) {
+		err = au.Store([]string{"123"})
+		require.NoError(t, err)
+		err = au.Store([]string{"456"})
+		require.NoError(t, err)
+
+		err = au.Delete("789")
+		require.NoError(t, err)
+
+		var count int
+		err = db.Get(&count, "SELECT COUNT(*) FROM approved_users")
+		require.NoError(t, err)
+		assert.Equal(t, 2, count, "There should be two records left")
+	})
+
+	t.Run("invalid ID", func(t *testing.T) {
+		err = au.Store([]string{"123"})
+		require.NoError(t, err)
+		err = au.Store([]string{"456"})
+		require.NoError(t, err)
+
+		err = au.Delete("aaa789")
+		require.Error(t, err)
+
+		var count int
+		err = db.Get(&count, "SELECT COUNT(*) FROM approved_users")
+		require.NoError(t, err)
+		assert.Equal(t, 2, count, "There should be two records left")
+	})
+}