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

enhance(backend): check visibility of reactions of remote users #14383

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

tesaguri
Copy link
Contributor

@tesaguri tesaguri commented Aug 9, 2024

What

リモートユーザのプロフィールを取得する際にActivityPubアクターのlikedコレクションの取得を試み、その内容が取得できる場合は当該ユーザのリアクションの一覧が公開設定であるものとしてデータベースに登録するようにします。(#14375の一般化)

また、ローカルのActivityPubアクターにlikedコレクションを実装し、ユーザがリアクションを送った投稿の一覧とリアクションの公開設定を連合できるようにします。

Why

Resolves #12964.

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 Aug 9, 2024
Copy link
Contributor

github-actions bot commented Aug 9, 2024

このPRによるapi.jsonの差分

差分はこちら
--- base
+++ head
@@ -72197,15 +72197,6 @@
                       }
                     }
                   },
-                  "IS_REMOTE_USER": {
-                    "value": {
-                      "error": {
-                        "message": "Currently unavailable to display reactions of remote users. See https://github.com/misskey-dev/misskey/issues/12964",
-                        "code": "IS_REMOTE_USER",
-                        "id": "6b95fa98-8cf9-2350-e284-f0ffdb54a805"
-                      }
-                    }
-                  },
                   "INVALID_PARAM": {
                     "value": {
                       "error": {

Get diff files from Workflow Page

Copy link

codecov bot commented Aug 9, 2024

Codecov Report

Attention: Patch coverage is 9.44882% with 115 lines in your changes missing coverage. Please review.

Project coverage is 39.66%. Comparing base (d2175a9) to head (97f96ab).
Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
...ges/backend/src/server/ActivityPubServerService.ts 0.00% 98 Missing ⚠️
.../backend/src/core/activitypub/ApRendererService.ts 0.00% 9 Missing ⚠️
...end/src/core/activitypub/models/ApPersonService.ts 50.00% 8 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14383      +/-   ##
===========================================
- Coverage    41.50%   39.66%   -1.85%     
===========================================
  Files         1549     1545       -4     
  Lines       199644   194004    -5640     
  Branches      3676     2510    -1166     
===========================================
- Hits         82868    76943    -5925     
- Misses      116213   116496     +283     
- Partials       563      565       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

followersVisibility,
publicReactions,
followingVisibility: followingIsPublic ? 'public' : followingIsPublic === false ? 'private' : undefined,
followersVisibility: followersIsPublic ? 'public' : followersIsPublic === false ? 'private' : undefined,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

followingIsPublic / followersIsPublic の型ってbooleanではない?

Copy link
Contributor Author

@tesaguri tesaguri Aug 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updatePerson()では「"public"SETする」、「"private"SETする」、「UPDATEしない」(リモートからの500レスポンス等の一時的なエラーの場合)という3通りの選択肢を表現するのに{following,followers}IsPublicの型をboolean | undefinedとしています。

@tesaguri tesaguri marked this pull request as draft August 14, 2024 03:35
The `liked` collection is a list of objects liked by the actor, not the
associated `Like` activities.
@tesaguri tesaguri marked this pull request as ready for review August 14, 2024 05:17
@tesaguri
Copy link
Contributor Author

ユーザが行ったリアクションの総数の情報はAPIでも露出されていないように見えたので、それに合わせてlikedコレクションのtotalItemsを削除しておきました。

@tesaguri
Copy link
Contributor Author

CIの失敗はdevelopブランチのHEAD (cd21000)と同じものですね。

.limit(limit + 1)
.orderBy('reaction.id', 'DESC')
.innerJoinAndSelect('reaction.note', 'note')
.distinctOn(['note.id'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noteの公開範囲 (非公開やフォロワー限定ではないか)
NoteがlocalOnlyか
NoteのUserが有効か (サスペンドや削除中ではないか)
あたりのフィルタは必要そう。

他の/users/:id, /users/:id/* あたりのUserフィルタも結構ガバガバそうだけどだわ…

@mei23
Copy link
Contributor

mei23 commented Aug 19, 2024

でも私はliked生やして有無で判定するよりActorにプロパティ生やしちゃったほうがいいと思うのだわね。

@tesaguri
Copy link
Contributor Author

独自プロパティでなく既存の仕組みを流用する利点としては、Misskey以外の実装との相互運用をより自然に(i.e. 独自プロパティを普及させるための政治的コストを負うことなく)行えるであろうという点を想定していますが、現状としてlikedコレクション自体があまり広く実装されていない様子なので、これはあくまで将来の展望でしかないというのは否めません。

@anatawa12
Copy link
Member

(https://misskey.niri.la/notes/9x4nmu9qx7 でノートしたことをコピペ)

コレクションへのアクセスが有るとhttpsリクエストがコレクションごとに一つ増えちゃうので following / followers 周りも含めて actor へのプロパティをはやしてそれをみんな見て、ない場合にのみlikedへのフォールバックする形ほうがネットワーク負荷と互換性の両面でそれなりに良さそうだと思いました。

mastodon等他のものが絶対追加しないって方針であればまた話が変わってくる可能性はありますがmisskeyはmisskey同士の連合も多いのでmisskey同士でも良さそうと漠然と考えています

@tesaguri tesaguri marked this pull request as draft August 19, 2024 16:34
@tesaguri tesaguri marked this pull request as ready for review August 19, 2024 16:34
@tesaguri
Copy link
Contributor Author

tesaguri commented Aug 19, 2024

コレクションへのアクセスが有るとhttpsリクエストがコレクションごとに一つ増えちゃうので following / followers 周りも含めて actor へのプロパティをはやしてそれをみんな見て、ない場合にのみlikedへのフォールバックする形ほうがネットワーク負荷と互換性の両面でそれなりに良さそうだと思いました。

ネットワーク負荷の懸念については、発信側でコレクションオブジェクトがアクターのJSON-LD文書に埋め込まれている場合(e.g. "liked": { "id": "https://example.com/actor/liked", "type": "Collection", "items": [] })は追加のリクエストなしで判定が完結するように実装されているので、Misskeyのアクターオブジェクトにコレクションを埋め込むようにする(0949c0e)ことでMisskeyとの連合における負荷は軽減されるかと思います。Misskey以外の実装との連合の場合はそもそもMisskeyの独自プロパティがサポートされている見込みが薄いでしょうから、独自プロパティを採用するか否かにかかわらず負荷は変わらないと考えます。

ただ、following/followersに関しては独自プロパティを採用することで「フォロワー限定」の公開範囲をサポートできるようになるという利点も考えられますね。しかしリアクションについては今のところ「フォロワー限定」の公開範囲が存在しないので、仮にこのための仕組みを追加するならば別のPRに分けたほうが良いかと思います。

@tesaguri
Copy link
Contributor Author

Misskeyのアクターオブジェクトにコレクションを埋め込むようにする(0949c0e)ことでMisskeyとの連合における負荷は軽減されるかと思います。

ちなみに0949c0eではfollowing/followersには手を付けていませんが、というのも特にfollowersコレクションはlikedコレクションと比べて1JSON上の表現を変更することによる既存実装との互換性への影響が予測しづらく、本PRのスコープから外れている割に変更の非自明さが大きいと判断したためです。

Footnotes

  1. likedコレクションはあまり広く実装されていない上に、仮に実装されていたとしてもオブジェクトの公開範囲の判定に使われるfollowersコレクションと違ってそのURIが重要になるケースは少ないと考えられるため。実際、Misskey自身のApPersonServicefollowersのURIは記録しているがfollowingコレクションは無視している。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend:test packages/backend Server side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

リアクション一覧の公開設定をリモートサーバーに連合したい
5 participants