misskey-dev
fix(backend): チャンネルフォロー一覧のsinceId/untilIdによる絞り込みが上手く動いていないのを修正
What
チャンネルのフォロー一覧を返すAPIにて、sinceId/untilIdの比較条件として与えているIDに間違いがあるのを直します（与えられているIDはchannel.idだが、比較先のIDはchannel_following.id） これにより、チャンネルフォロー一覧から結果が抜け落ちる現象が改善されます。
Why
fix https://github.com/misskey-dev/misskey/issues/12175
Additional info (optional)
実際にチャンネルを大量に作成＆お気に入り登録し、歯抜けにならないことを確認。
Checklist
- [x] Read the contribution guide
- [x] Test working in a local environment
- [ ] (If needed) Add story of storybook
- [x] (If needed) Update CHANGELOG.md
- [ ] (If possible) Add tests
Codecov Report
Attention: Patch coverage is 3.12500% with 31 lines in your changes missing coverage. Please review.
Project coverage is 41.91%. Comparing base (
abc1e91) to head (4c8b4c4).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| packages/backend/src/core/QueryService.ts | 4.34% | 22 Missing :warning: |
| ...kend/src/server/api/endpoints/channels/followed.ts | 0.00% | 9 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #13698 +/- ##
============================================
+ Coverage 15.28% 41.91% +26.62%
============================================
Files 784 1622 +838
Lines 72028 165713 +93685
Branches 1268 4098 +2830
============================================
+ Hits 11008 69452 +58444
- Misses 60596 95784 +35188
- Partials 424 477 +53
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
🚀 New features to boost your workflow:
- ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
①フロントエンド側のMkPaginationはidというプロパティを期待しており、これをページネーションのsinceId/untilIdに使用している
https://github.com/misskey-dev/misskey/blob/c1514ce91dc08481a092a789ee37da546cdb4583/packages/frontend/src/components/MkPagination.vue#L83-L85 https://github.com/misskey-dev/misskey/blob/c1514ce91dc08481a092a789ee37da546cdb4583/packages/frontend/src/components/MkPagination.vue#L239-L328
②バックエンド側のQueryService.makePaginationQuery()はidというプロパティを期待しており、これをページネーションのsinceId/untilIdに使用している
https://github.com/misskey-dev/misskey/blob/c1514ce91dc08481a092a789ee37da546cdb4583/packages/backend/src/server/api/endpoints/channels/followed.ts#L51-L52 https://github.com/misskey-dev/misskey/blob/c1514ce91dc08481a092a789ee37da546cdb4583/packages/backend/src/core/QueryService.ts#L43-L68
ここで渡しているのはchannelFollowingsRepositoryのクエリビルダーなので、channel_followings.idがsinceId/untilIdとして渡されることが想定されている
③バックエンドから返される時にpackされているが、この時のIDはchannelのものになる
https://github.com/misskey-dev/misskey/blob/c1514ce91dc08481a092a789ee37da546cdb4583/packages/backend/src/server/api/endpoints/channels/followed.ts#L58 https://github.com/misskey-dev/misskey/blob/c1514ce91dc08481a092a789ee37da546cdb4583/packages/backend/src/core/entities/ChannelEntityService.ts#L75
①＋②＋③
フロントエンドにはchannel.idをキーとした一覧が返却され、MkPaginationによるページネーションもこのIDが使用される。channels/followedのsinceId/untilIdにはchannel.idが渡ってくることになるが、channels/followedそのものはchannel_followings.idを使って絞り込みをしており、不正確な条件で絞り込みをしてしまう（IDの種類が違うので）
@syuilo この流れ、1回やった気がするのでもう一度 https://github.com/misskey-dev/misskey/issues/12175 を確認いただけますと… :pray: そのうえでこのブランチの対応がよく無さそうであれば、別の方法を考えます
https://github.com/misskey-dev/misskey/blob/514a65e45330f09ad58cac3cab16bd888be80866/packages/backend/src/core/entities/FollowingEntityService.ts#L90-L101
followingのIDでした
個人的な考えとして…
- 本来返すべきもの(ChannelFollowing)を返せていない(Channelを返している)問題
- v2なエンドポイントの作成が必要で、v2が生え次第Misskeyのフロントエンドもこっちを使うように変えるべき
- v1(現行)とv2で互換性は無い
- 現状返しているものを前提とした上での挙動が正しくない(現行Misskeyのフロントエンドと、v1を使っているサードパーティクライアントが正常に動いていない)問題
- v1(現行)のエンドポイントを使っているサードパーティクライアント(更新未追従)が壊れないまま修正でき、互換性も失われない(このPR)
という2つの問題があると捉えていて、前者と後者は方向性が違う(ので両方やって良さそう)と思っているのですが、syuiloさんとしては両方やるのは冗長…という感覚でしょうか?
- 前者のみの対応だと、サードパーティクライアントは追従するまで壊れた挙動のままになる
- 後者の対応を行ったとしても、本来返すべきものを返すようにv2を作成する理由は依然ある(v2が生えればMisskeyのフロントエンドもv2に切り替える。v1は互換性のために残す)
v1(現行)のエンドポイントを使っているサードパーティクライアント(更新未追従)が壊れないまま修正でき、互換性も失われない(このPR)
自分の理解が間違ってるかもしれないけど、このPRで修正されるとは思えない(違う種類のデータ同士を比較しているから、期待したデータが返って来たとしても偶然と思われる)
v1(現行)のエンドポイントを使っているサードパーティクライアント(更新未追従)が壊れないまま修正でき、互換性も失われない(このPR)
自分の理解が間違ってるかもしれないけど、このPRで修正されるとは思えない(違う種類のデータ同士を比較しているから、期待したデータが返って来たとしても偶然と思われる)
sinceId/untilIdをフォローした時間ベースのものであると捉えると正にそうなのですが、しかし「"v1(現行)は返ってきているものがChannelであり、ChannelFollowingではない"ので、v1(現行)におけるsinceId/untilIdがチャンネルが作成された時間ベースのものであってもおかしくはない」と思うのですが、いかがでしょうか…? (もちろん、v2が生えればそれが返すものはChannelFollowingになるので、v2のsinceId/untilIdはフォローした時間ベースになるべき)
makePaginationQueryを少し弄ったコピペ版を作成し（ただし、安易に複製しないでほしい旨と経緯、このissue/prをコメントで添える）、それを使ってページング処理をするようにすれば、すべてクリアになりますかね…？
ページネーションが壊れているのを直すという目的を考えると、offsetを追加してsinceId/untilIdは非推奨にするのがシンプルなのではないかと思います
変更に追従しないサードパーティは壊れたままになりますがレスポンスが異なる別のエンドポイントが増える場合よりも追従は容易になります
確かに、offset式に変えるとページネーションは直せるかもしれません…? しかし、正しいものが返ってきていないということが発覚した時点で、"このエンドポイントに手を付けるならv2を作る"ということ自体はほぼ確定しているのでは、と思っています。 そのため、v1に対して"変更に追従しないと壊れたまま"な変更を加えるぐらいなら、直さずにv2作るだけで良いじゃんとなりそうな気がします? (このPRは、とりあえず"変更に追従されなくても挙動が直る"ようにすることを目的にしていると思うので)
Channelの代わりにChannelFollowingを返すようにすることにどのような意味があるのでしょうか（フロントエンドでフォローした日時を表示する？） 返すものを変えるためにほとんど同じ機能の別のエンドポイントを作るぐらいなら元のエンドポイントに（互換性を保ったまま）変更を入れるだけでいいのではと思いました
Channelの代わりにChannelFollowingを返すようにすることにどのような意味があるのでしょうか（フロントエンドでフォローした日時を表示する？）
一般的にフォロー中の何かを一覧で返す場合は、フォローした順に一覧したいことが多そう