Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(server): アンテナ再有効の手段にアンテナ設定の更新を追加 #11036

Merged
merged 2 commits into from
Jul 21, 2023

Conversation

meronmks
Copy link
Contributor

@meronmks meronmks commented Jun 19, 2023

What

一度無効になったアンテナTLを表示またはアンテナの設定を変更した際にも有効化するように

Why

#10476 の修正では該当するノート受信時のみとなっていると思われるため一度無効になってしまうと
アクセスした時点でRedisのアンテナTLが空っぽになってしまった場合
ユーザの操作では二度と有効にすることができない。

Additional info (optional)

Checklist

  • 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

@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Jun 19, 2023
@codecov
Copy link

codecov bot commented Jun 19, 2023

Codecov Report

Merging #11036 (55d4fef) into develop (8c7bcdf) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #11036      +/-   ##
===========================================
- Coverage    77.47%   77.36%   -0.12%     
===========================================
  Files          909      908       -1     
  Lines        91516    91664     +148     
  Branches      6891     7550     +659     
===========================================
+ Hits         70906    70916      +10     
- Misses       20610    20748     +138     
Impacted Files Coverage Δ
...ackend/src/server/api/endpoints/antennas/update.ts 98.38% <100.00%> (+0.02%) ⬆️

... and 54 files with indirect coverage changes

@syuilo
Copy link
Member

syuilo commented Jun 20, 2023

#10476 の修正では該当するノート受信時のみとなっていると思われるため一度無効になってしまうと
ユーザの操作では二度と有効にすることができない。

そんなことない気がする

@syuilo
Copy link
Member

syuilo commented Jun 20, 2023

アンテナのノートを取得したときに再度有効化される

@meronmks
Copy link
Contributor Author

アンテナのノートを取得したときに再度有効化される

私の理解が間違ってなければですがこれはおそらくこの意図通りには動いてなくて
受信されたノートは
NoteCreateService.tscreate()->postNoteCreated()->AntennaService.tsaddNoteToAntennas()->...
と流されていくと思うのですが
https://github.com/misskey-dev/misskey/blob/develop/packages/backend/src/core/AntennaService.ts#LL90C3-L90C3
上記で呼び出してるconst antennas = await this.getAntennas();で帰ってくるのは
https://github.com/misskey-dev/misskey/blob/develop/packages/backend/src/core/AntennaService.ts#L187
上記の通りisActive: trueのものだけとなっており無効となっているアンテナにはノートが送られない、つまりantennas/notes.tsがこの後呼ばれることはないと思っております。

@syuilo
Copy link
Member

syuilo commented Jun 20, 2023

無効となっているアンテナにはノートが送られない

アンテナにノートが送られているかどうかはantennas/notesを叩くかどうかに関係しないと思った

@meronmks
Copy link
Contributor Author

あー!!!今ものすごい根本的な勘違いに気が付いたantennas/notesが叩かれてないじゃなくて、
update()が走る前に空配列を返す条件に引っかかって有効にするロジックに入ってないが正解かも
image
https://github.com/misskey-dev/misskey/blob/develop/packages/backend/src/server/api/endpoints/antennas/notes.ts#L86-L94

@meronmks
Copy link
Contributor Author

アンテナが0件になってしまった後の再有効化をどこでやるのが相応しそうか考えてたのですが、
0件になってしまうということはアンテナの条件に問題があることが多そう(0件で動いてなさそうならユーザーは自然と設定項目を再度弄るという予想)ということでこのプルリクのantennas/updateの変更だけにしてantennas/showの方は元に戻そうと考えていますがどうでしょうか?

@meronmks meronmks changed the title fix(server): アンテナ再有効の手段にアンテナの表示とアンテナ設定の更新を追加 fix(server): アンテナ再有効の手段にアンテナ設定の更新を追加 Jul 7, 2023
@syuilo syuilo merged commit 2801946 into misskey-dev:develop Jul 21, 2023
@syuilo
Copy link
Member

syuilo commented Jul 21, 2023

👍🏻

slofp pushed a commit to Secineralyr/misskey.dream that referenced this pull request Jul 21, 2023
* fix(server): アンテナ再有効の手段にアンテナの表示とアンテナ設定の更新を追加

* 無効+Redisも空なアンテナの再有効化手段を antennas/update だけに
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants