diff --git a/enterprise/suppress-user/handler.go b/enterprise/suppress-user/handler.go index b571c98b9f..37fa0662ad 100644 --- a/enterprise/suppress-user/handler.go +++ b/enterprise/suppress-user/handler.go @@ -46,7 +46,7 @@ type handler struct { func (h *handler) IsSuppressedUser(workspaceID, userID, sourceID string) bool { h.log.Debugf("IsSuppressedUser called for workspace: %s, user %s, source %s", workspaceID, userID, sourceID) - suppressed, err := h.r.Suppress(workspaceID, userID, sourceID) + suppressed, err := h.r.Suppressed(workspaceID, userID, sourceID) if err != nil { h.errLog.errMu.Lock() h.errLog.err = fmt.Errorf("suppression check failed for workspace: %s, user: %s, source: %s: %w", workspaceID, userID, sourceID, err) diff --git a/enterprise/suppress-user/handler_test.go b/enterprise/suppress-user/handler_test.go index 431427cedc..4fd812674f 100644 --- a/enterprise/suppress-user/handler_test.go +++ b/enterprise/suppress-user/handler_test.go @@ -31,7 +31,7 @@ type fakeSuppresser struct { Repository } -func (*fakeSuppresser) Suppress(_, _, _ string) (bool, error) { +func (*fakeSuppresser) Suppressed(_, _, _ string) (bool, error) { return false, fmt.Errorf("some error") } diff --git a/enterprise/suppress-user/internal/badgerdb/badgerdb.go b/enterprise/suppress-user/internal/badgerdb/badgerdb.go index 8593cbe671..c476de2383 100644 --- a/enterprise/suppress-user/internal/badgerdb/badgerdb.go +++ b/enterprise/suppress-user/internal/badgerdb/badgerdb.go @@ -105,8 +105,8 @@ func (b *Repository) GetToken() ([]byte, error) { return token, nil } -// Suppress returns true if the given user is suppressed, false otherwise -func (b *Repository) Suppress(workspaceID, userID, sourceID string) (bool, error) { +// Suppressed returns true if the given user is suppressed, false otherwise +func (b *Repository) Suppressed(workspaceID, userID, sourceID string) (bool, error) { b.restoringLock.RLock() defer b.restoringLock.RUnlock() if b.restoring { @@ -116,7 +116,7 @@ func (b *Repository) Suppress(workspaceID, userID, sourceID string) (bool, error return false, badger.ErrDBClosed } - keyPrefix := fmt.Sprintf("%s:%s:", workspaceID, userID) + keyPrefix := keyPrefix(workspaceID, userID) err := b.db.View(func(txn *badger.Txn) error { wildcardKey := keyPrefix + model.Wildcard _, err := txn.Get([]byte(wildcardKey)) @@ -156,7 +156,7 @@ func (b *Repository) Add(suppressions []model.Suppression, token []byte) error { for i := range suppressions { suppression := suppressions[i] - keyPrefix := fmt.Sprintf("%s:%s:", suppression.WorkspaceID, suppression.UserID) + keyPrefix := keyPrefix(suppression.WorkspaceID, suppression.UserID) var keys []string if len(suppression.SourceIDs) == 0 { keys = []string{keyPrefix + model.Wildcard} @@ -305,3 +305,7 @@ type blogger struct { func (l blogger) Warningf(fmt string, args ...interface{}) { l.Warnf(fmt, args...) } + +func keyPrefix(workspaceID, userID string) string { + return fmt.Sprintf("%s:%s:", workspaceID, userID) +} diff --git a/enterprise/suppress-user/internal/badgerdb/badgerdb_test.go b/enterprise/suppress-user/internal/badgerdb/badgerdb_test.go index 0f95187429..fc45fd85ba 100644 --- a/enterprise/suppress-user/internal/badgerdb/badgerdb_test.go +++ b/enterprise/suppress-user/internal/badgerdb/badgerdb_test.go @@ -75,7 +75,7 @@ func TestBadgerRepository(t *testing.T) { require.Error(t, err, "it should return an error when trying to get the token from a repository that is restoring") require.ErrorIs(t, model.ErrRestoring, err) - _, err = repo.Suppress("workspace2", "user2", "source2") + _, err = repo.Suppressed("workspace2", "user2", "source2") require.Error(t, err, "it should return an error when trying to suppress a user from a repository that is restoring") require.ErrorIs(t, model.ErrRestoring, err) @@ -116,7 +116,7 @@ func TestBadgerRepository(t *testing.T) { t.Run("badgerdb errors", func(t *testing.T) { require.NoError(t, repo.Stop(), "it should be able to stop the badgerdb instance without an error") - _, err := repo.Suppress("workspace1", "user1", "") + _, err := repo.Suppressed("workspace1", "user1", "") require.Error(t, err) _, err = repo.GetToken() @@ -138,7 +138,7 @@ func TestBadgerRepository(t *testing.T) { require.Equal(t, repo.Add(nil, nil), badger.ErrDBClosed) - s, err := repo.Suppress("", "", "") + s, err := repo.Suppressed("", "", "") require.False(t, s) require.Equal(t, err, badger.ErrDBClosed) diff --git a/enterprise/suppress-user/internal/memory/memory.go b/enterprise/suppress-user/internal/memory/memory.go index e3e7356fac..466780858c 100644 --- a/enterprise/suppress-user/internal/memory/memory.go +++ b/enterprise/suppress-user/internal/memory/memory.go @@ -30,8 +30,8 @@ func (m *Repository) GetToken() ([]byte, error) { return m.token, nil } -// Suppress returns true if the given user is suppressed, false otherwise -func (m *Repository) Suppress(workspaceID, userID, sourceID string) (bool, error) { +// Suppressed returns true if the given user is suppressed, false otherwise +func (m *Repository) Suppressed(workspaceID, userID, sourceID string) (bool, error) { m.suppressionsMu.RLock() defer m.suppressionsMu.RUnlock() workspace, ok := m.suppressions[workspaceID] diff --git a/enterprise/suppress-user/internal/repotest/repotest.go b/enterprise/suppress-user/internal/repotest/repotest.go index 26267f53e9..5fcc274cdf 100644 --- a/enterprise/suppress-user/internal/repotest/repotest.go +++ b/enterprise/suppress-user/internal/repotest/repotest.go @@ -40,35 +40,35 @@ func RunRepositoryTestSuite(t *testing.T, repo suppression.Repository) { }) t.Run("wildcard suppression", func(t *testing.T) { - suppressed, err := repo.Suppress("workspace1", "user1", "source1") + suppressed, err := repo.Suppressed("workspace1", "user1", "source1") require.NoError(t, err) require.True(t, suppressed, "it should return true when trying to suppress a user that is suppressed by a wildcard suppression") - suppressed, err = repo.Suppress("workspace1", "user1", "source2") + suppressed, err = repo.Suppressed("workspace1", "user1", "source2") require.NoError(t, err) require.True(t, suppressed, "it should return true when trying to suppress a user that is suppressed by a wildcard suppression") }) t.Run("exact suppression", func(t *testing.T) { - suppressed, err := repo.Suppress("workspace2", "user2", "source1") + suppressed, err := repo.Suppressed("workspace2", "user2", "source1") require.NoError(t, err) require.True(t, suppressed, "it should return true when trying to suppress a user that is suppressed by an exact suppression") }) t.Run("non matching key", func(t *testing.T) { - suppressed, err := repo.Suppress("workspace3", "user3", "source2") + suppressed, err := repo.Suppressed("workspace3", "user3", "source2") require.NoError(t, err) require.False(t, suppressed, "it should return false when trying to suppress a user that is not suppressed") }) t.Run("non matching suppression", func(t *testing.T) { - suppressed, err := repo.Suppress("workspace2", "user2", "source2") + suppressed, err := repo.Suppressed("workspace2", "user2", "source2") require.NoError(t, err) require.False(t, suppressed, "it should return false when trying to suppress a user that is suppressed for a different sourceID") }) t.Run("canceling a suppression", func(t *testing.T) { - suppressed, err := repo.Suppress("workspace1", "user1", "source1") + suppressed, err := repo.Suppressed("workspace1", "user1", "source1") require.NoError(t, err) require.True(t, suppressed, "it should return true when trying to suppress a user that is suppressed by a wildcard suppression") @@ -86,7 +86,7 @@ func RunRepositoryTestSuite(t *testing.T, repo suppression.Repository) { require.NoError(t, err) require.Equal(t, token2, rtoken) - suppressed, err = repo.Suppress("workspace1", "user1", "source1") + suppressed, err = repo.Suppressed("workspace1", "user1", "source1") require.NoError(t, err) require.False(t, suppressed, "it should return false when trying to suppress a user that was suppressed by a wildcard suppression after the suppression has been canceled") }) @@ -111,7 +111,7 @@ func RunRepositoryTestSuite(t *testing.T, repo suppression.Repository) { }, token) require.NoError(t, err, "it should be able to add some suppressions without an error") - suppressed, err := repo.Suppress("workspaceX", "userX", "sourceX") + suppressed, err := repo.Suppressed("workspaceX", "userX", "sourceX") require.NoError(t, err) require.True(t, suppressed, "it should return true when trying to suppress a user that is suppressed by a wildcard suppression") @@ -123,11 +123,11 @@ func RunRepositoryTestSuite(t *testing.T, repo suppression.Repository) { SourceIDs: []string{}, }, }, token)) - suppressed, err = repo.Suppress("workspaceX", "userX", "sourceX") + suppressed, err = repo.Suppressed("workspaceX", "userX", "sourceX") require.NoError(t, err) require.False(t, suppressed, "it should return false when trying to suppress a user that is no longer suppressed by a wildcard suppression") - suppressed, err = repo.Suppress("workspaceX", "userX", "source1") + suppressed, err = repo.Suppressed("workspaceX", "userX", "source1") require.NoError(t, err) require.True(t, suppressed, "it should return true when trying to suppress a user that is still suppressed by an exact match suppression") @@ -139,11 +139,11 @@ func RunRepositoryTestSuite(t *testing.T, repo suppression.Repository) { SourceIDs: []string{"source1"}, }, }, token)) - suppressed, err = repo.Suppress("workspaceX", "userX", "source1") + suppressed, err = repo.Suppressed("workspaceX", "userX", "source1") require.NoError(t, err) require.False(t, suppressed, "it should return false when trying to suppress a user that is no longer suppressed by an exact match suppression") - suppressed, err = repo.Suppress("workspaceX", "userX", "source2") + suppressed, err = repo.Suppressed("workspaceX", "userX", "source2") require.NoError(t, err) require.True(t, suppressed, "it should return true when trying to suppress a user that is still suppressed by an exact match suppression") @@ -155,7 +155,7 @@ func RunRepositoryTestSuite(t *testing.T, repo suppression.Repository) { SourceIDs: []string{"source2"}, }, }, token)) - suppressed, err = repo.Suppress("workspaceX", "userX", "source2") + suppressed, err = repo.Suppressed("workspaceX", "userX", "source2") require.NoError(t, err) require.False(t, suppressed, "it should return false when trying to suppress a user that is no longer suppressed by an exact match suppression") }) diff --git a/enterprise/suppress-user/repository.go b/enterprise/suppress-user/repository.go index 36cbafdfd8..293c5ff08d 100644 --- a/enterprise/suppress-user/repository.go +++ b/enterprise/suppress-user/repository.go @@ -20,8 +20,8 @@ type Repository interface { // Add adds the given suppressions to the repository Add(suppressions []model.Suppression, token []byte) error - // Suppress returns true if the given user is suppressed, false otherwise - Suppress(workspaceID, userID, sourceID string) (bool, error) + // Suppressed returns true if the given user is suppressed, false otherwise + Suppressed(workspaceID, userID, sourceID string) (bool, error) // Backup writes a backup of the repository to the given writer Backup(w io.Writer) error diff --git a/enterprise/suppress-user/repository_bench_test.go b/enterprise/suppress-user/repository_bench_test.go index 294946fae3..3da0f67b06 100644 --- a/enterprise/suppress-user/repository_bench_test.go +++ b/enterprise/suppress-user/repository_bench_test.go @@ -38,7 +38,7 @@ func BenchmarkAddAndSuppress(b *testing.B) { for i := 0; i < totalReads; i++ { start := time.Now() idx := randomInt(totalSuppressions * 2) // multiply by 2 to include non-existing keys suppressions - _, err := repo.Suppress(fmt.Sprintf("workspace%d", idx), fmt.Sprintf("user%d", idx), fmt.Sprintf("source%d", idx)) + _, err := repo.Suppressed(fmt.Sprintf("workspace%d", idx), fmt.Sprintf("user%d", idx), fmt.Sprintf("source%d", idx)) require.NoError(b, err) totalTime += time.Since(start) }