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

refactor(backend): core/activitypub #11247

Merged
merged 21 commits into from
Jul 13, 2023
Merged

refactor(backend): core/activitypub #11247

merged 21 commits into from
Jul 13, 2023

Conversation

okayurisotto
Copy link
Contributor

What

このPRでは/packages/backend/src/core/activitypub/*.tsを中心にリファクタリングを行いました。

Why

今後の円滑な開発のため。

Additional info (optional)

ActivityPubについてまだ詳しくないため、すべてのanyをなくすなどの改善はできていません。現時点の私にもできそうな推論をしました。それぞれの編集の意図についてはコミットメッセージを参照していただければと思います。

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

okayurisotto added 21 commits July 11, 2023 20:23
- `LdSignature`の`normalize`メソッドでの使われ方から、
	- `data`引数の型定義を`any`から`JsonLdDocument`へ修正
	- `getLoader`メソッドの返り値の型定義の一部を`any`から`RemoteDocument`へ修正
		- `contextUrl`が不正な値(`null`)となっていたことが判明したため`undefined`へ修正
		- `document`の型と合わせるために`CONTEXTS`の型定義の一部を`unknown`から`JsonLd`へ修正
			- とりあえず`satisfies`を使用
		- `document`の型と合わせるために`fetchDocument`メソッドの返り値の型定義の一部を`unknown`から`JsonLd`へ修正
			- どうしようもなく`as`を使用
`.filter()`で行っている型ガードなどの文脈から、より適しているだろうと思われる書き方に変更した。
- `id`は`null`とのunionになっていたが、`null`を渡している場面はなかった
	- またおそらくこのメソッドは`IOrderedCollection`を返すため、そちらに合わせて`null`とのunionをやめた
		- `IOrderedCollection`とはまだ型に相違がある
- `totalItems`をコメントや使われ方を元に`number`へ推論
@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Jul 12, 2023
@github-actions github-actions bot requested review from syuilo and tamaina July 12, 2023 01:57
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #11247 (819204d) into develop (9845cce) will decrease coverage by 0.05%.
The diff coverage is 55.21%.

@@             Coverage Diff             @@
##           develop   #11247      +/-   ##
===========================================
- Coverage    77.69%   77.64%   -0.05%     
===========================================
  Files          910      908       -2     
  Lines        92033    91878     -155     
  Branches      7671     7674       +3     
===========================================
- Hits         71504    71340     -164     
- Misses       20529    20538       +9     
Impacted Files Coverage Δ
...ackend/src/core/activitypub/ApDbResolverService.ts 61.62% <0.00%> (-1.47%) ⬇️
...s/backend/src/core/activitypub/ApRequestService.ts 78.12% <0.00%> (ø)
.../backend/src/core/activitypub/ApResolverService.ts 46.07% <0.00%> (+0.88%) ⬆️
...ckend/src/core/activitypub/models/ApNoteService.ts 65.01% <0.00%> (ø)
packages/backend/src/core/activitypub/type.ts 95.04% <ø> (+0.29%) ⬆️
...backend/src/core/activitypub/LdSignatureService.ts 12.96% <7.40%> (+0.86%) ⬆️
...ges/backend/src/core/activitypub/ApInboxService.ts 18.62% <11.76%> (-0.08%) ⬇️
...ges/backend/src/server/ActivityPubServerService.ts 53.95% <76.47%> (+0.43%) ⬆️
...nd/src/core/activitypub/ApDeliverManagerService.ts 96.69% <82.60%> (-1.92%) ⬇️
.../backend/src/core/activitypub/ApRendererService.ts 81.80% <85.36%> (-0.30%) ⬇️
... and 3 more

... and 5 files with indirect coverage changes

@syuilo syuilo merged commit e35a370 into misskey-dev:develop Jul 13, 2023
@syuilo
Copy link
Member

syuilo commented Jul 13, 2023

👍🏻👍🏻

@okayurisotto okayurisotto deleted the refactor-backend-apservice branch July 13, 2023 03:56
slofp pushed a commit to Secineralyr/misskey.dream that referenced this pull request Jul 21, 2023
* eslint: `explicit-function-return-type`

* eslint: `no-unnecessary-condition`

* eslint: `eslint-disable-next-line`

* eslint: `no-unused-vars`

* eslint: `comma-dangle`

* eslint: `import/order`

* cleanup: unnecessary non-null assertion

* cleanup: `IActivity`に`actor`は常に存在するようなので

* cleanup: unnecessary `as`

* cleanup: unnecessary `Promise.resolve`

* cleanup

* refactor: `String.prototype.match()`である必要がない部分をよりシンプルな書き方に変更

* refactor: よりよい型定義

* refactor: よりよい型定義

- `LdSignature`の`normalize`メソッドでの使われ方から、
	- `data`引数の型定義を`any`から`JsonLdDocument`へ修正
	- `getLoader`メソッドの返り値の型定義の一部を`any`から`RemoteDocument`へ修正
		- `contextUrl`が不正な値(`null`)となっていたことが判明したため`undefined`へ修正
		- `document`の型と合わせるために`CONTEXTS`の型定義の一部を`unknown`から`JsonLd`へ修正
			- とりあえず`satisfies`を使用
		- `document`の型と合わせるために`fetchDocument`メソッドの返り値の型定義の一部を`unknown`から`JsonLd`へ修正
			- どうしようもなく`as`を使用

* refactor: 型ガードを使うことでnon-null assertionをやめた

* refactor: non-null assertionをやめた

`.filter()`で行っている型ガードなどの文脈から、より適しているだろうと思われる書き方に変更した。

* refactor: 型ガードを使うことで`as`をやめた

* refactor: `as`をやめた

* refactor: よりよい型定義

- `id`は`null`とのunionになっていたが、`null`を渡している場面はなかった
	- またおそらくこのメソッドは`IOrderedCollection`を返すため、そちらに合わせて`null`とのunionをやめた
		- `IOrderedCollection`とはまだ型に相違がある
- `totalItems`をコメントや使われ方を元に`number`へ推論

* refactor: `for-of` -> `Array.prototype.map`

* refactor: `delete`演算子を使わない形に
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