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でのOpenAPI関連の処理の整理 #10625

Closed
wants to merge 11 commits into from
Closed

Refactor: backendでのOpenAPI関連の処理の整理 #10625

wants to merge 11 commits into from

Conversation

okayurisotto
Copy link
Contributor

@okayurisotto okayurisotto commented Apr 13, 2023

What

このPRはbackendにおけるOpenAPIのSpecの生成に関連した処理のリファクタリングを目的としたものです。

具体的には次のことを行う予定です。

  • ESLintのWarningなどの修正
  • よりよい型を定義する

Why

Misskeyでは/api.jsonへアクセスすることでOpenAPIのSpecを得ることができますが、このSpecに書かれた内容には不正確なものがあるようです。(まだ規模や詳細を把握しておらず、Issueを作成できる段階ではありません。)

その規模や詳細を把握するためにまずbackendにおいてどのようにOpenAPIのSpecが生成されているのかを知っておこうと思い、ソースコードを読んだのですが、生成するOpenAPIのSpecの型が定義されていないことなどによって、関数の返り値の型定義がないことやanyの使用などの問題が起きてしまっていることが確認されました。

そこで、まずはこれらの問題をリファクタリングによって取り除こうと考え、このドラフトPRを作成しました。

Additional info (optional)

OpenAPIのSpecの型を定義するにあたり、OAI/OpenAPI-Specificationにあるv3.0のJSON Schemaを参照しました。JSON SchemaからTypeScriptの型定義への変換は手動で行いました。ツールによって自動生成しようかとも思ったのですが、まだ他の方々の意見を聞いていない段階でそのようなことをすべきではないのではないかという判断をし、とりあえず手動で書くことにした次第です。

そのため、type FIXME = never;という型を多用してしまっています。ただ、現時点のMisskeyが生成するSpecはそこまで複雑なものではないため、必要な部分の型定義についてはすでに完了しています。しかし将来的によりしっかりとしたSpecを作るかもしれないことを考えると、現時点の型定義は不十分です。


convertSchemaToOpenApiSchema関数内でのanyの使用をやめたことで、多くの型エラーが発生しています。できる限りのことはしますが、この問題の根本的な原因はそもそもの現時点のこの関数の実装に問題があることです。そしてこの問題は、現時点のMisskeyが出力するSpecをVS Code上で前述のJSON Schemaで検証したときに表示される多くの警告の原因でもあります。この関数の型エラーをなくそうとすると、最終的に/api.jsonで出力されるJSONの構造にも影響があるということです。


各エンドポイントのリクエスト・レスポンスのschemaについて、responsesschemaではSchema型の値をconvertSchemaToOpenApiSchema関数を通すことでOpenAPI形式の型(OpenAPISchema)へ変換しているのですが、requestBodyではこの関数を使っていません。よりよい型を定義した関係でエラーになってしまっています。この問題の修正でも、出力されるJSONの構造に影響が出るものと思われます。


現時点のMisskeyが出力するJSONでは、components.securitySchemes.ApiKeyAuth.inの値が"body"となっていますが、これは前述のJSON Schemaには定義されていない値です。私はOpenAPIについて詳しくないため、誰か何かご存知でしたら教えていただけるとうれしいです。

コミットログなどを遡ってみたのですが、どうやら #4351 から"body"となっているようでした。

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

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

codecov bot commented Apr 13, 2023

Codecov Report

Merging #10625 (403c1a8) into develop (dffefda) will decrease coverage by 2.49%.
The diff coverage is 8.36%.

@@             Coverage Diff             @@
##           develop   #10625      +/-   ##
===========================================
- Coverage    78.33%   75.85%   -2.49%     
===========================================
  Files          164      897     +733     
  Lines        20445    88756   +68311     
  Branches       363     6249    +5886     
===========================================
+ Hits         16016    67323   +51307     
- Misses        4429    21433   +17004     
Impacted Files Coverage Δ
packages/backend/src/server/api/openapi/types.ts 0.00% <0.00%> (ø)
...end/src/server/api/openapi/OpenApiServerService.ts 100.00% <100.00%> (ø)
packages/backend/src/server/api/openapi/errors.ts 100.00% <100.00%> (ø)
...ackages/backend/src/server/api/openapi/gen-spec.ts 100.00% <100.00%> (ø)
packages/backend/src/server/api/openapi/schemas.ts 100.00% <100.00%> (ø)

... and 732 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@okayurisotto
Copy link
Contributor Author

VS CodeでのJSON Schemaを使ったバリデーションではいくつかのエラーが見逃がされていたようだったため、別のツールを使ってバリデーションをし直しました。

14f30af のMisskeyが出力するapi.jsonswagger-cliでバリデーションしてみたところ、6034行のエラーメッセージが出力されました。この多くは許可されていないadditional propertiesによるものだと思われます。そしてこれはconvertSchemaToOpenApiSchema関数の実装に問題があることが原因です。

このPRでの修正を試みます。とりあえずこの問題についてのIssueを作成することで、この問題とこのPRを関連付けておきます。

@okayurisotto
Copy link
Contributor Author

convertSchemaToOpenApiSchema関数の正しい実装においてどのような処理が必要になるか考えるために、まずは従来のSchema型とOpenAPIのSchema型を比較しました。

#10632 (comment)

これらの相違点は実際のエラーメッセージの内容と関係したものでした。まずはこれらの相違点をうまく解消するような実装をしてみて、その上でまたバリデーションを実行して様子を見てみることにします。

@Nanashia
Copy link
Contributor

現時点のMisskeyが出力するJSONでは、components.securitySchemes.ApiKeyAuth.inの値が"body"となっていますが、これは前述のJSON Schemaには定義されていない値です。私はOpenAPIについて詳しくないため、誰か何かご存知でしたら教えていただけるとうれしいです。

Related to misskey-dev/misskey-hub#222 (comment)

@okayurisotto
Copy link
Contributor Author

#10632 (comment) とのことでしたので、抜本的な改善の具体的な実装1を、ソースコードを読んで模索しようと思います。

現在の状況は当初想定していたものから大きく変わってしまっているため、このPRはすでにout-of-dateです。適切なタイミングでcloseしようと思います。失礼しました。

Footnotes

  1. 私が当初考えていた「末端のわかりやすく実装しやすい部分から定義していく」というものはベストではないようなので考え直します。

@okayurisotto
Copy link
Contributor Author

閉じます。失礼しました!

@okayurisotto okayurisotto deleted the refactor-openapi branch April 17, 2023 09:23
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