Conversation
Context の stripe を Stripe から { sdk: Stripe } に変え、プロシージャが
SDK を取り出す経路を 1 つにする。sdk プロパティで Stripe を返すオブジェクト
なら何でも代入できるので、プロシージャを変えずに実体を差し替えられる。
あわせて、変更系ガードのテスト 3 件を強めた。これまでは拒否されたことしか
見ておらず、エラーのコードも、Stripe への書き込みが起きなかったことも
検査していなかった。ガードを書き込みの後ろへ動かす変更や、投げるエラーを
別のものに替える変更を検出できない状態だったので、FORBIDDEN のコードと
スタブが呼ばれなかったことの両方を表明する形にした。
apps/web/server/utils/stripe.ts は残しているので、apiVersion の明示的な
指定と SDK の遅延生成・キャッシュは変わらない。
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ContextのstripeをStripeから{ sdk: Stripe }に変え、プロシージャが SDK を取り出す経路を 1 つにする。あわせて、変更系ガードのテストが検出できる変異を増やす。なぜ
sdkプロパティで Stripe を返すオブジェクトなら何でも代入できる形にしておくと、プロシージャを変えずに実体を差し替えられる。この PR はContextの型と呼び出し側をその形に合わせるところまでで、実体の差し替えは行わない。実装している決定
Stripe SDK への到達経路を 1 つにする
Contextに SDK を直接置く形(変更前のstripe: Stripe)と、sdkプロパティを持つオブジェクトを置く形を比べ、後者を採った。前者のままだと、実体をsdkを持つアダプタに差し替えるときにプロシージャ 11 箇所の書き換えが要る。後者なら、本番のContextを組み立てるapps/web/server/routes/rpc/[...].tsの 1 箇所だけで差し替えが済み、プロシージャの側もContextの型も変えずに済む。stripeを残したまま SDK を別フィールドで併置する形は採らなかった。SDK への経路が 2 つになり、stripe.sdkを通らない呼び出しが型の上で許されたままになる。経路を 1 つにするというこの PR の目的が達成されない。apiVersion の明示的な指定を保つ
apps/web/server/utils/stripe.tsのuseStripe()を残し、この PR では削除しない。このファイルがapiVersionを明示的に指定しており、省略するとアカウントの既定の API バージョンに従って SDK の型と実行時のレスポンスの形がずれることがある。SDK を更新するとLatestApiVersion型が変わり、この文字列が型エラーになって気付ける。変更系ガードのテストを強める
prices.update・products.update・invoices.issueはmutationsEnabledが false のときFORBIDDENを投げる。既存のテストは拒否されたことしか見ておらず、次の 2 つを検査していなかった。FORBIDDENであること前者が無いと、ガードが別のエラーを投げるようになっても通る。後者が無いと、ガードを書き込みの後ろへ動かしても通る。どちらも、認可を持たない書き込みが Stripe に到達する状態を、CI がグリーンのまま許す形である。
変更をこの範囲で切る
実体の差し替え(
useStripe()を削除し、sdkを持つアダプタに寄せる)は、この PR には含めず後続の PR で行う。新しいアダプタのファイルもこの PR では作らない。{ sdk: useStripe() }は既存のuseStripe()をその場で包むだけなので、後でアダプタを採るときに置き換える対象が 1 つで済む。まとめて 1 つの PR にする案は採らなかった。
sdk経路が正しく当たっているかという判断と、どのアダプタを採るかという判断が同じ差分に混ざり、レビュアーが失敗を見たときにどちらの軸の失敗かを切り分けられなくなるためである。この PR 単体で lint・型チェック・テスト・ビルドが通るので、分けても途中で壊れた状態は残らない。apps/web/package.jsonのstripeの依存とapps/web/server/plugins/validate-config.tsも変更していない。この PR が変えるのは SDK の取り出し方だけで、SDK の生成も鍵の存在チェックも変えていないためである。確かめたこと
pnpm lint・pnpm knip・pnpm typecheck・pnpm test・pnpm buildがすべて通る。SDK への参照がすべてsdkを経由することは、git grep -n 'context\.stripe\.' -- packages/api/src | grep -v 'context\.stripe\.sdk\.'が何も返さないことで確かめられる。強めたテストについて、主張が成り立たなくなる変異を当てて、どの変異でもテストが失敗することを確かめた。ガードそのものへの変異は
assertMutationsEnabledに、プロシージャ側への変異はprices.updateに当てた。「変更前のテスト」の列は、この PR の直前のコミットの実装とテストに同じ変異を当てた結果である。throwを削る変更前のテストを通ってしまう 2 つは、検査していなかった 2 つにそのまま対応する。エラーのコードを見ていなかったので「ガードが投げるコードを別のものに変える」が、書き込みが起きなかったことを見ていなかったので「ガードを Stripe への書き込みの直後へ移す」が、それぞれすり抜ける。