feat: ミュートされたユーザーのリアクションをレスポンスに含めないようにする(#13456)
What
- ミュートされたユーザーのリアクションをノートに含めないようにする
-
note.reactionAndUserPairCacheに含まれるユーザーのみを対象とする
-
- ミュートされたユーザーをリアクション詳細に含めないようにする
Why
#13456
Additional info (optional)
- Whatの1について
-
note.reactionAndUserPairCacheに含まれるユーザーのみを対象とするため、現状では最初から数えてPER_NOTE_REACTION_USER_PAIR_CACHE_MAX(=16)のユーザーのみが処理される - 対象のユーザーがミュートされていた場合、リアクション数をデクリメントする(または消去する)
- したがって、リアクション数16以下のときには、ミュートは必ず実施される
- この処理ではミュートされたユーザーを検索する際、Redisのキャッシュを使用する(これによりパフォーマンスの問題は起きにくいと考える)
-
- Whatの2について
- 1で処理が漏れたユーザーについては、リアクション詳細(
MkReactionsViewer.details.vue等)に表示されないようミュートする - 注意されたいこととして、フロントエンドのAPI呼び出し方法を変更している
- 具体的には
misskeyApiGet('notes/reactions', {...})をmisskeyApi('notes/reactions', {...})に置き換えている - これは、
notes/reactionsの内部においてミュートの情報を参照するためにユーザー情報が必要であり、このためPOSTリクエストでCredentialsを含める必要があるからである(が、PR作成者はAPI呼び出しの仕様を詳しく把握できなかったので、この呼び出し方法について有識者にご判断いただきたい)
- 具体的には
- 1で処理が漏れたユーザーについては、リアクション詳細(
- テストについて
- ブラウザを手動で操作することによる動作確認は実施済み
- e2eテストを書いてみたが、手元ではテスト全体がうまく動作しないのでこのテストは
git revertしてある
Checklist
- [x] Read the contribution guide
- [ ] Test working in a local environment
- [ ] (If needed) Add story of storybook
- [ ] (If needed) Update CHANGELOG.md
- [ ] (If possible) Add tests
Codecov Report
Attention: Patch coverage is 23.33333% with 23 lines in your changes are missing coverage. Please review.
Project coverage is 63.55%. Comparing base (
f8261a1) to head (ebf10ff). Report is 557 commits behind head on develop.
| Files | Patch % | Lines |
|---|---|---|
| ...ges/backend/src/core/entities/NoteEntityService.ts | 19.23% | 20 Missing and 1 partial :warning: |
| ...ackend/src/server/api/endpoints/notes/reactions.ts | 0.00% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #13827 +/- ##
============================================
- Coverage 79.95% 63.55% -16.41%
============================================
Files 956 985 +29
Lines 108864 112166 +3302
Branches 8413 5521 -2892
============================================
- Hits 87045 71283 -15762
- Misses 21819 39213 +17394
- Partials 0 1670 +1670
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
パフォーマンスへの影響が気になる点
- GETが使えなくなる(キャッシュできなくなる)点
- ミュート情報がキャッシュされてない場合、追加でデータベースへのクエリが発生する点
おっしゃる通り、色々考慮が不足していました
- GETが使えなくなる(キャッシュできなくなる)点
やはりフロントエンド側でどうにかしたほうが良いかもしれません(バックエンドだとPOSTは免れない)
- 解決案:
-
notes/reactionsをGETで取得、それとは別にmute/listを取得、リアクション詳細でユーザーを非表示にする- (現在は
mute/listが毎回DBアクセスになるので、Redisに置き換えたほうがよさそう?)
- (現在は
-
- 懸念点:
- ミュート情報をブラウザ側でどのように記憶しておくか?
- ミュート情報がキャッシュされてない場合、追加でデータベースへのクエリが発生する点
Redisのキャッシュを極力使用しないとすると、DBにキャッシュする方法が考えられます
-
解決案A:
- userテーブルに
muteeIdCache character varying(32)[]のようなカラムを追加する - このカラムには、当該ユーザーにミュートされたユーザーのIDを格納する
- 配列の件数の最大値は何件にすべきか不明…
- ミュートの件数が配列の最大長を超える場合、無効であることを示す値を格納する
- NoteEntityServiceのミュート処理では、上記のキャッシュが使用可能なら上記を、無効である場合にはRedisを使用する
- これにより、(理想的には)ほとんどのユーザーを追加のDBアクセスの発生なしに処理できる
- userテーブルに
-
懸念点:
- DBのマイグレーションが必要
- 配列の最大長を何件にするか?
- これによりDBの容量がどうなるか?
-
解決案B:
- 仕様の把握が不完全ですが、タイムラインを取得する際にミュート情報がキャッシュされているようです
- タイムライン取得とリアクション詳細表示は時間的にそれほど乖離した操作ではないと思われますので、キャッシュが残っている可能性は高いのではないでしょうか?
-
懸念点:
- 上記の検証方法が不明…
解決案が不明瞭なので一旦クローズします
ミュート情報がキャッシュされてない場合、追加でデータベースへのクエリが発生する点
厳しめのレートリミットを掛けたり、キャッシュするにしても更新頻度を下げたりすればある程度マシになったりしますかね…?
微妙かも