djangoRESTFrameworkのリファクタリング ~レガシーなコードの寿命を延ばすために~

Pycon APAC

Transcript

1. djangoRESTFrameworkのリファクタリング ~レガシーなコードの寿命を延ばすために~ YUUKI NAKAJIMA - PyCon APAC 2023 -
2. who am I?
 Company:
3. who am I?
 Company: I am not employee
4. who am I?
 Company: I am not employee I am freerance
5. who am I?
 Company: I am not employee I am freerance Works:djangoRESTframework/AWS X(twitter):@ProletariatPro
6. who am I?
 Company: I am not employee I am freerance Works:DjangoRestFrameWork/AWS X:@ProletariatPro プログレッシブなプロレタリアート(労働者階級)という意 味です
7. 企業に所属していないので宣伝する 製品やサービスがありません！
8. 代わりに推しメンを紹介します

9. いい加減本編に入ります・・・・・
10. 空前のPythonブームから数年が経ちました.... ･機械学習やAIの分野がPythonだからwebフレームワークもPythonにするか...みたい な理由でDjangoが採用されていたりしませんか？ ･なんかPythonが流行だからバックエンドはPythonにしよう、Djangoならなんでもできる らしいぞ！ ･そんな理由でDjangoRESTframeworkが採用されて数年とかありませんか？
11. 空前のPythonブームから数年が経ちました.... ･機械学習やAIの分野がPythonだからwebフレームワークもPythonにするか...みたい な理由でDjangoが採用されていたりしませんか？ ･なんかPythonが流行だからバックエンドはPythonにしよう、Djangoならなんでもできる らしいぞ！ ･そんな理由でDjangoRESTframeworkが採用されて数年とかありませんか？ ･そろそろ保守メンテがきつい時期に入っていませんか？
12. ここで話すこと ･自分が携わってるプロジェクト (ファイルの先頭に # coding: utf-8と書かれている）の寿命を延ばすため にやったこと ･fat controller問題 ･swaggerの修正 ･request用のserializerの導入 ･unitテストの改善 ･その他意図不明の処理 ･blah-blah-blah….(等々） とチームで戦ってきた記録になります。
13. 話さないこと ･hogehogeアーキテクチャーとか ･全てを解決する魔法 ･リファクタリングに最も必要なことは怒りの力であること
14. 今回話すプロジェクトの概要 ･toC向けスマートフォンアプリのバックエンドのAPI ･DjangoRestFrameworkでできている ･8年ぐらい稼働しているのでもう当時を知るエンジニアがいない ･別のwebフレームワークに置き換える余力はない
15. レガシーコードって何?
16. レガシーコードって何?
17. レガシーコードって何? まず、テストで保護しろ、保護してから リファクタリングしろ！
18. テストがあれば何でもできる！

19. テストがあれば何でもできる！ ･With testing, anything is possible. ･Dengan ujian, segala-galanya mungkin. ･通過測試，一切皆有可能。 ･ดวยการทดสอบ ทุกสิ่งก็เปนไปได ･Dengan pengujian, segala sesuatu mungkin terjadi. ･테스트를 통해 무엇이든 가능합니다. ･ேசாதைனயால் எதுவும் சாத்தியம்
20. テストがないときはどうすればいい? ･まずは修正箇所から”ついでに”テストを書いて行くのが良いのかなと思います。 ･プロジェクトの規模にもよりますが一度に一気にテストだけを書いて行くのは心が折れ る仕事になりそう。 ･テストを書くことは短期的にはなんの機能追加でもないのでビジネスが許さない ･なので少しずつ書いていくしかない...?? ･この場合なによりも必要なのは怒りの力でテストを書くことです(でも当時の人には当時 の事情があったと察してgit logを見て責めないでください)
21. 構成イメージ APP views models serializer.py tests urls.py hogehoge.py foo.py test_hogehoge.py test_foo.py hogehoge.py foo.py
22. Simple!
23. Not SIMPLE!! class BreakfastView(APIView): def post(self, request, hogehoge): try: spam = request.data[“spam”] except KeyError: raise NotSpamError with transaction.atomic(): (A long code like a galaxy far, far away…) return Response(BreakfastSerializer(instance).data)
24. Not SIMPLE!! class BreakfastView(APIView): def post(self, request, hogehoge): try: spam = request.data[“spam”] except KeyError: raise NotSpamError with transaction.atomic(): (A long code like a galaxy far, far away…) return Response(BreakfastSerializer(instance).data) request_bodyのチェックをviewで行っている 宇宙みたいなコードでした
25. ではなかった...(宇宙が肥大化していく例) class BreakfastView(APIView): def post(self, request, hogehoge): try: spam = request.data[“spam”] if spam < 2: raise SpamLackError except KeyError: raise NotSpamError ham = request.data.get(“ham”, None) egg = request.data.get(“egg”, None) if ham and egg: raise HamEggError with transaction.atomic(): (A long code like a galaxy far, far away…) return Response(BreakfastSerializer(instance).data)
26. FAT!
27. 一応モデルにメソッドを大量に作ることで、まとまった処理を分けるようにはなっていた。 その結果、あちこちのモデルのメソッドで循環参照(circular reference)が発生してとても 気持ち悪い状態になっていた
28. fat controller問題との戦い ･そもそもDjangoではViewなんだからfat viewでは？ ･今のプロジェクトではApiViewを使ってviewにひたすらビジネスロジックを書いて行くス タイルだったのでviewが肥大化していき可読性が低くなっていた。 ･でもテストがあった
29. fat controller問題 ･そもそもDjangoではViewなんだからfat viewでは？ ･今のプロジェクトではApiViewを使ってviewにひたすらビジネスロジックを書いて行くス タイルだったのでviewが肥大化していき可読性が低くなっていた。 ･でもテストがあった ･でもテストがあった！
30. テストがあれば何でもできる！
31. テストがあれば何でもできる！ ･With testing, anything is possible. ･Dengan ujian, segala-galanya mungkin. ･通過測試，一切皆有可能。 ･ดวยการทดสอบ ทุกสิ่งก็เปนไปได ･Dengan pengujian, segala sesuatu mungkin terjadi. ･테스트를 통해 무엇이든 가능합니다. ･ேசாதைனயால் எதுவும் சாத்தியம்
32. リファクタリング①:service層の導入
33. 構成図 構成イメージ APP views models serializer.py tests urls.py hogehoge.py foo.py test_hogehoge.py test_foo.py hogehoge.py foo.py services hogehoge.py foo.py
34. Service層導入後 class BreakfastView(APIView): def post(self, request, hogehoge): try: spam = request.data[“spam”] except KeyError: raise NotSpamError with transaction.atomic(): 　　 s = BreakfastService(spam) s.method1() instance = s.method2() return Response(BreakfastSerializer(instance).data) メソッド名を見ればだいたい何をやってるか分かるように なった。 テストがしやすくなった モデルからも分離できたので循環参照も減らせる！
35. 🎉
36. 補足: ModelのメソッドとService層の使い分けってどうやるの？ 個人的には.... • 自分のモデルにしか触らないならモデルのメソッド • 複数のモデルに触るならService層に分離するのが良いのでは？？ • どのくらい使い回すか等状況次第ではない...
37. requestのチェック requestもserializerでチェックするようにしました。
38. RequestCheck用のSerializer導入 class BreakfastView(APIView): def post(self, request, hogehoge): serialiser = BreakfastRequestBodySerializer(request.data) seriarizer.is_valid(raise_exception=True) with transaction.atomic(): 　　 s = BreakfastService(spam) s.method1() res = s.method2() return Response(ResponseSerializer(instance).data) チェック処理は全部Serializerに押し込んだ 型やその他のチェックも全部ここでやるよ！
39. 🎉
40. このことによって･･･
41. swaggerも自動生成できるようになったよ @swagger_auto_schema( request_body=BreakfastRequestBodySerializer(), responses={200: BreakfastSerializer()} class BreakfastView(APIView): def post(self, request, hogehoge):
42. クライアントエンジニアとswaggerを通して仕様を共有できるようになった！ それまで手書きのswaggerファイルを通してクライアントエンジニアとAPI仕様を共有して いたが、実体とswagerファイルが合わない箇所が多く出ていたのでその問題も解消し た！
43. 🎉
44. swaggerの補足 今回はdrf_yasgというのを採用しましたが、drf-spectacularというのもありますがここで は触れません...
45. unitテストの改善 ･テストがないコードはレガシーなコードだ ･failした時にすぐにエラー箇所が分かるテストがunit テストだ
46. テストの現状 class BreakfastTest(TestCase): def test_ok(self): # prepare test c = client() res =c.post(path=”/api/breakfast”,data={“spam”:3, “ham”:1, “bacon”:2}) self.assertEqual(res.status_code, 200) self.assertEqual(レスポンスの確認)　　 # other check…. self.assertEqual(データベースの更新確認) self.assertEqual(その他諸々の更新確認)
47. テストの現状 class BreakfastTest(TestCase): def test_ok(self): # prepare test c = client() res =c.post(path=”/api/breakfast”,data={“spam”:3, “ham”:1, “bacon”:2}) self.assertEqual(res.status_code, 200) self.assertEqual(レスポンスの確認)　　 # other check…. self.assertEqual(データベースの更新確認) self.assertEqual(その他諸々の更新確認) ･1回のテストで確認していることがたくさんある ･落ちた時に原因究明に時間がかかる ･可読性が低いのでなんのテストか確認する必 要がある レコード作成とかをたくさんやっていた
48. unitテストの改善 ･テストがないコードはレガシーなコードだ ･failした時にすぐにエラー箇所が分かるテストがunit テストだ ･でもそうはなってなかったんだよ.... ･だからこの話はこれでおしまい...
49. ではなく！やっていきましょう

50. テストの現状 class BreakfastTest(TestCase): def test_ok(self): # prepare test c = client() res =c.post(path=”/api/breakfast”,data={“spam”:3, “ham”:1, “bacon”:2}) self.assertEqual(res.status_code, 200) self.assertEqual(レスポンスの確認)　　 # other check…. self.assertEqual(データベースの更新確認) self.assertEqual(その他諸々の更新確認) ･1回のテストで確認していることがたくさんある ･落ちた時に原因究明に時間がかかる ･可読性が低いのでなんのテストか確認する必 要がある レコード作成とかをたくさんやっていた
51. FAT!
52. テストの改善 ･確認する項目は減らせない ･分割はできる ･modelのメソッドのテスト。新規導入したservice層のテストなど ･とはいえ、それもできない場合
53. 簡単に直せて効果的(?)な方法 class BreakfastTest(TestCase): def test_正常パターン(self): BreakFastFactory() c = client() res =c.post(path=”/api/breakfast”,data={“spam”:3, “ham”:1, “bacon”:2}) with self.subTest(“レスポンスの確認”): self.assertEqual(res.status_code, 200) self.assertEqual(レスポンスの確認)　　 with self.subTest(“データベースの更新確認 ”) self.assertEqual(データベースの更新確認 ) with self.subTest(“その他のテスト”): self.assertEqual(その他諸々の更新確認) とりあえずまとまり毎に subTestにすることでリーダブ ルにする 日本語でOKなのでメソッド名でなんの確認か分かるように 事前にレコードを用意する必要がある箇所は FactoryBoyなどを使ってお手軽にレコード作成
54. まとめ テストがあればリファクタリングはできる！ テストがないなら少しずつでもいいから書きましょう FatControllerは自分のプロジェクトでは service層の導入で解決 テストはview全部ではなくクラスや method毎にしていきたい
55. Python/Djangoとあんまり関係ないこと ･コメントを書く(このコードを読むのは将来の誰か(自分かもしれない))その人に向けてメ タ情報を記入してほしい ･変数名/関数名/メソッド名はとても大事、自分が一番レビューで気にするところ ･レビューで決まらない問題はチームで話ながら決めて行こう ･リファクタリングはテストで保護してから ･リファクタリングに乗り出すには怒りの力が必要、でも当時の事情を察して責めないで ほしい ･コードがリーダブルになるととても気持ちいいぞ
56. 長く動くシステムを作っていきましょう！

57. ご静聴ありがとうございました！

58. 👏