## Description Reverts [#14726](https://github.com/chatwoot/chatwoot/pull/14726) (\"feat: Add sidebar unread counts for filters (CW-7262)\"), which shipped in 4.15.0. After 4.15.0 rolled out to prod the unread-counts-for-filters code path caused a cascading incident: - `Counter#ensure_filters_cache!` fires on every `/unread_counts/index` and `update_last_seen` request. - On cache miss it calls `Builder#build_filters_for!`, which: - invokes `store.clear_user_filters!` -> `delete_matching` -> a Redis `SCAN_each` over a per-user pattern keyspace, and - runs 4 fresh SQL passes per user (mentions, participating, unattended, and per-folder `Conversations::FilterService` queries). - Threads blocked in the SCAN held their DB connections, the connection pool exhausted, Sidekiq jobs were discarded with `ActiveJob::DeserializationError: could not obtain a connection from the pool`, and the enqueued queue blew past 200K. Related: [CW-7262](https://linear.app/chatwoot/issue/CW-7262/unread-counts-for-filters-folders) ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality not to work as expected) - [ ] This change requires a documentation update ## How Has This Been Tested? ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules
201 lines
7.4 KiB
Ruby
201 lines
7.4 KiB
Ruby
require 'rails_helper'
|
|
|
|
RSpec.describe Conversations::UnreadCounts::Store do
|
|
let(:account_id) { 1 }
|
|
let(:inbox_id) { 2 }
|
|
let(:label_id) { 3 }
|
|
let(:user_id) { 4 }
|
|
let(:conversation_id) { 5 }
|
|
let(:team_id) { 6 }
|
|
|
|
after do
|
|
described_class.clear_account!(account_id)
|
|
end
|
|
|
|
describe 'key builders' do
|
|
it 'builds base keys using the Redis key naming convention' do
|
|
expect(described_class.inbox_key(account_id, inbox_id)).to eq(
|
|
'UNREAD_CONVERSATIONS::V1::ACCOUNT::1::INBOX::2'
|
|
)
|
|
expect(described_class.label_inbox_key(account_id, label_id, inbox_id)).to eq(
|
|
'UNREAD_CONVERSATIONS::V1::ACCOUNT::1::LABEL::3::INBOX::2'
|
|
)
|
|
expect(described_class.team_inbox_key(account_id, team_id, inbox_id)).to eq(
|
|
'UNREAD_CONVERSATIONS::V1::ACCOUNT::1::TEAM::6::INBOX::2'
|
|
)
|
|
end
|
|
|
|
it 'builds assignment-aware keys using the Redis key naming convention' do
|
|
expect(described_class.inbox_unassigned_key(account_id, inbox_id)).to eq(
|
|
'UNREAD_CONVERSATIONS::V1::ACCOUNT::1::INBOX::2::UNASSIGNED'
|
|
)
|
|
expect(described_class.inbox_assignee_key(account_id, inbox_id, user_id)).to eq(
|
|
'UNREAD_CONVERSATIONS::V1::ACCOUNT::1::INBOX::2::ASSIGNEE::4'
|
|
)
|
|
expect(described_class.label_inbox_unassigned_key(account_id, label_id, inbox_id)).to eq(
|
|
'UNREAD_CONVERSATIONS::V1::ACCOUNT::1::LABEL::3::INBOX::2::UNASSIGNED'
|
|
)
|
|
expect(described_class.label_inbox_assignee_key(account_id, label_id, inbox_id, user_id)).to eq(
|
|
'UNREAD_CONVERSATIONS::V1::ACCOUNT::1::LABEL::3::INBOX::2::ASSIGNEE::4'
|
|
)
|
|
expect(described_class.team_inbox_unassigned_key(account_id, team_id, inbox_id)).to eq(
|
|
'UNREAD_CONVERSATIONS::V1::ACCOUNT::1::TEAM::6::INBOX::2::UNASSIGNED'
|
|
)
|
|
expect(described_class.team_inbox_assignee_key(account_id, team_id, inbox_id, user_id)).to eq(
|
|
'UNREAD_CONVERSATIONS::V1::ACCOUNT::1::TEAM::6::INBOX::2::ASSIGNEE::4'
|
|
)
|
|
end
|
|
end
|
|
|
|
describe 'ready markers' do
|
|
it 'tracks base and assignment readiness independently' do
|
|
expect(described_class.base_ready?(account_id)).to be(false)
|
|
expect(described_class.assignment_ready?(account_id)).to be(false)
|
|
|
|
described_class.mark_base_ready!(account_id)
|
|
described_class.mark_assignment_ready!(account_id)
|
|
|
|
expect(described_class.base_ready?(account_id)).to be(true)
|
|
expect(described_class.assignment_ready?(account_id)).to be(true)
|
|
expect(ttl_for('UNREAD_CONVERSATIONS::V1::ACCOUNT::1::READY::BASE')).to be_within(5).of(Conversations::UnreadCounts::READY_TTL)
|
|
expect(ttl_for('UNREAD_CONVERSATIONS::V1::ACCOUNT::1::READY::ASSIGNMENT')).to be_within(5).of(Conversations::UnreadCounts::READY_TTL)
|
|
end
|
|
end
|
|
|
|
describe 'set operations' do
|
|
it 'adds, counts, and removes base memberships' do
|
|
described_class.add_base_membership(
|
|
account_id: account_id,
|
|
inbox_id: inbox_id,
|
|
label_ids: [label_id],
|
|
team_id: team_id,
|
|
conversation_id: conversation_id
|
|
)
|
|
|
|
expect(described_class.counts_for_keys(base_keys)).to eq(
|
|
described_class.inbox_key(account_id, inbox_id) => 1,
|
|
described_class.label_inbox_key(account_id, label_id, inbox_id) => 1,
|
|
described_class.team_inbox_key(account_id, team_id, inbox_id) => 1
|
|
)
|
|
expect(base_keys.map { |key| ttl_for(key) }).to all(be_within(5).of(Conversations::UnreadCounts::SET_TTL))
|
|
|
|
described_class.remove_base_membership(
|
|
account_id: account_id,
|
|
inbox_ids: [inbox_id],
|
|
label_ids: [label_id],
|
|
team_ids: [team_id],
|
|
conversation_id: conversation_id
|
|
)
|
|
|
|
expect(described_class.counts_for_keys(base_keys).values).to all(eq(0))
|
|
end
|
|
|
|
it 'checks memberships for a conversation across keys' do
|
|
described_class.add_base_membership(
|
|
account_id: account_id,
|
|
inbox_id: inbox_id,
|
|
label_ids: [label_id],
|
|
team_id: team_id,
|
|
conversation_id: conversation_id
|
|
)
|
|
|
|
expect(described_class.memberships_for_keys(base_keys, conversation_id)).to eq(
|
|
described_class.inbox_key(account_id, inbox_id) => true,
|
|
described_class.label_inbox_key(account_id, label_id, inbox_id) => true,
|
|
described_class.team_inbox_key(account_id, team_id, inbox_id) => true
|
|
)
|
|
expect(described_class.memberships_for_keys(base_keys, 999).values).to all(be(false))
|
|
end
|
|
|
|
it 'adds, counts, and removes assignment-aware memberships' do
|
|
described_class.add_assignment_membership(
|
|
account_id: account_id,
|
|
inbox_id: inbox_id,
|
|
label_ids: [label_id],
|
|
assignee_id: user_id,
|
|
team_id: team_id,
|
|
conversation_id: conversation_id
|
|
)
|
|
|
|
expect(described_class.counts_for_keys(assignment_keys)).to eq(
|
|
described_class.inbox_assignee_key(account_id, inbox_id, user_id) => 1,
|
|
described_class.label_inbox_assignee_key(account_id, label_id, inbox_id, user_id) => 1,
|
|
described_class.team_inbox_assignee_key(account_id, team_id, inbox_id, user_id) => 1
|
|
)
|
|
expect(assignment_keys.map { |key| ttl_for(key) }).to all(be_within(5).of(Conversations::UnreadCounts::SET_TTL))
|
|
|
|
described_class.remove_assignment_membership(
|
|
account_id: account_id,
|
|
inbox_ids: [inbox_id],
|
|
label_ids: [label_id],
|
|
assignee_ids: [user_id],
|
|
team_ids: [team_id],
|
|
conversation_id: conversation_id
|
|
)
|
|
|
|
expect(described_class.counts_for_keys(assignment_keys).values).to all(eq(0))
|
|
end
|
|
|
|
it 'sets expiry on bulk membership writes' do
|
|
described_class.add_memberships(
|
|
account_id: account_id,
|
|
memberships: [{
|
|
inbox_id: inbox_id,
|
|
label_ids: [label_id],
|
|
team_id: team_id,
|
|
conversation_id: conversation_id
|
|
}]
|
|
)
|
|
|
|
expect(base_keys.map { |key| ttl_for(key) }).to all(be_within(5).of(Conversations::UnreadCounts::SET_TTL))
|
|
end
|
|
|
|
it 'clears all account memberships' do
|
|
described_class.mark_base_ready!(account_id)
|
|
described_class.mark_assignment_ready!(account_id)
|
|
described_class.add_base_membership(
|
|
account_id: account_id,
|
|
inbox_id: inbox_id,
|
|
label_ids: [label_id],
|
|
team_id: team_id,
|
|
conversation_id: conversation_id
|
|
)
|
|
described_class.add_assignment_membership(
|
|
account_id: account_id,
|
|
inbox_id: inbox_id,
|
|
label_ids: [label_id],
|
|
assignee_id: user_id,
|
|
team_id: team_id,
|
|
conversation_id: conversation_id
|
|
)
|
|
|
|
described_class.clear_account!(account_id)
|
|
|
|
expect(described_class.base_ready?(account_id)).to be(false)
|
|
expect(described_class.assignment_ready?(account_id)).to be(false)
|
|
expect(described_class.counts_for_keys(base_keys).values).to all(eq(0))
|
|
expect(described_class.counts_for_keys(assignment_keys).values).to all(eq(0))
|
|
end
|
|
end
|
|
|
|
def base_keys
|
|
[
|
|
described_class.inbox_key(account_id, inbox_id),
|
|
described_class.label_inbox_key(account_id, label_id, inbox_id),
|
|
described_class.team_inbox_key(account_id, team_id, inbox_id)
|
|
]
|
|
end
|
|
|
|
def assignment_keys
|
|
[
|
|
described_class.inbox_assignee_key(account_id, inbox_id, user_id),
|
|
described_class.label_inbox_assignee_key(account_id, label_id, inbox_id, user_id),
|
|
described_class.team_inbox_assignee_key(account_id, team_id, inbox_id, user_id)
|
|
]
|
|
end
|
|
|
|
def ttl_for(key)
|
|
Redis::Alfred.ttl(key)
|
|
end
|
|
end
|