STORES Product Blog

こだわりを持ったお商売を支える「STORES」のテクノロジー部門のメンバーによるブログです。

STORES 予約 のリファクタリング(後編)

STORES 予約 で Webアプリケーションエンジニアをしています。ykpythemindです。

STORES 予約 では機能開発する際に積極的にリファクタリングを行っています。 前回 は支払い方法API部分のリファクタリングを途中まで紹介しました。後編ではより踏み込んだリファクタリングを実施し、凝集度の低いクラスを削除します。

tech.hey.jp

内部実装を知らなくて済むように分岐を減らす

前回、予約確定後に使った支払い方法を記録する部分には問題がありました。

# 予約に対して支払いを実施し、結果を記録します
def update_payment!(reservation, payment_request)
  payment_service = Reservation::PaymentService.new reservation.merchant

  # 支払い方法によって分岐
  case payment_request.type
  when :subscription
    subscription =
      current_user!.end_user_customer_subscriptions.find_by(
        public_id: payment_request.customer_subscription_id,
      )
    payment_service.pay_with_subscription!(reservation, subscription)

  when :ticket
    product = Product.find_by(public_id: payment_request.product_id)
    ticket_book_ids =
      current_user!.end_user_ticket_books.where(product: product).available.pluck(:id)

    payment_service.pay_with_tickets!(reservation, current_user!.end_user_tickets.valid.where(ticket_book_id: ticket_book_ids)

  when :credit_card
    payment_service.pay_with_credit_card!(reservation, remote_ip, payment_request.ctok, credit_card)
  end
end

パラメータとして予約者から与えられた支払い方法が実際に正しいのか(月謝の期限が切れていないか、不正なパラメータが与えられていないかどうかなど)の検証するクラスでも、同様の分岐が発生していました。

class PaymentMethodValidator
  def initialize(reservation, payment_request:, user:)
    @reservation = reservation
    @payment_request = payment_request
    @user = user
  end

  def validate_payment_method
    case @payment_request.type
    when :subscription
      return validate_payment_method_subscription(@reservation, payment_request.customer_subscription_id)
    when :ticket
      return validate_payment_method_ticket(@reservation, payment_request.product_id)
    when :credit_card
      return validate_payment_method_credit_card(@reservation)
    end

    return { success: true }
  end

  def validate_payment_method_credit_card(reservation)
    # クレジットカード支払いを使用できるかのバリデーション...
  end

  # ...

支払い方法の種類によってどのようなメソッドを呼ばなければならないのかを常に実装者が把握していなければならないという点や、PaymentService / PaymentMethodValidator class自体の凝集度が低いという点で好ましくありません。

例えば今回実装したい機能である「クーポン支払い」を実装する際に、PaymentRequestのtypeによっての分岐を漏れなく書く必要がありました。

ここではリファクタリング第2版『ポリモーフィズムによる条件記述の置き換え』を行い、分岐を削除しましょう。*1

ポリモーフィックに振る舞うため、PaymentRequestに実装するインターフェースを決めましょう。コードを読むと支払い方法を検証する部分(PaymentMethodValidator)と支払いを実施する部分(PaymentService) の2つのクラスがあり、どちらも予約を表すmodel Reservationを受け取っています。したがって以下のメソッドを各PaymentRequest側に実装します。

  • #pay(reservation: Reservation, current_user: User | nil) -> boolean
  • #validate(reservation: Reservation, current_user: User | nil) -> PaymentRequest::ValidationResult

新しい支払い方法が追加されたときは、この2つのメソッドを実装したPaymentRequestを実装すれば良くなり、使用者は内部実装を知らずにメソッドを呼び出せばいいだけになります。

class PaymentRequest
  # ValidationResultは支払い方法のバリデーションの結果を保持するクラスです。
  ValidationResult = Struct.new(:message, :success, keyword_init: true) do
                       def valid?
                         success
                       end

                       def invalid?
                         !valid?
                       end
                     end

  class Base
    # 結果オブジェクトを生成するヘルパーメソッドです。お好みで定義してください。
    def validate_error(message)
      ValidationResult.new(message: message, success: false)
    end
    
    def validate_success
      ValidationResult.new(success: true)
    end
  end

  # (前記事参照) クレジットカード払い
  class CreditCard < Base
    # ...
    
    def validate(reservation:, current_user:)
      # 注釈) 該当の予約に紐づく予約ページの設定で、実際にクレジットカード払いを要求しているかを調べる
      unless valid_method_for_reservation?(reservation)
        return validate_error('not_supported')
      end

      # ... 中略 ...
      # (ここの内容は PaymentMethodValidatorのクレジットカード払いの場合に書かれていた検証を持ってくる。)

      validate_success
    end

    def pay(reservation:, current_user:)
      # (ここの内容は PaymentService#pay_with_credit_card! を呼んでいる部分を持ってくる )
    end
  end
end

PaymentMethodValidatorには validate_payment_method_ticket / validate_payment_method_credit_card のような検証用メソッドがありましたが、それをすべて validateインターフェースの実装として各PaymentRequestに移植しましょう。

支払いを行わない場合も、なにもしない pay メソッドを定義するようにします。

  # (前記事参照) 支払いを行わない場合
  class Noop < Base
    # ...
    
    def validate(reservation:, current_user:)
      # 注釈) 該当の予約に紐づく予約ページの設定で、支払いを行わなくても良いかどうか確認
      unless valid_method_for_reservation?(reservation)
        return validate_error('not_supported')
      end

      validate_success
    end

    def pay(reservation:, current_user:)
      # なにもしないで成功扱い
      true
    end
  end
end

支払い方法ごとのPaymentRequestにpay/validateメソッドを定義できたら、あとは使用箇所を置き換えます。

前回の記事で初めに載せたリクエストの処理部分を変更しています。

diff --git a/r3.rb b/r2.rb
index fb70541..2752d11 100644
--- a/r3.rb
+++ b/r2.rb
@@ -5,17 +5,11 @@ post '/payment/validate' do
   # 特定の支払い方法の場合はログインが必須
   user = current_user! if payment_request.need_user_login?

-  # すべての処理をValidation classに委譲
-  payment_method_validator =
-    ::Reservations::PaymentMethodValidator.new(
-      @reservation,
-      payment_request: payment_request,
-      user: user,
-    )
+  validation_result = payment_request.validate(reservation: @reservation, current_user: user)

   # 選択した支払い方法が正しくなければ構造化したエラーレスポンスを返す。
-  unless payment_method_validator.valid?
-    error!(payment_method_validator.payment_method_error, 400)
+  if validation_result.invalid?
+    error!(validation_result.message, 400)
   end

   status :ok
@@ -45,23 +39,17 @@ post '/finalize' do
   user = current_user! if payment_request.need_user_login?

   # このエンドポイントでも同様に支払い方法パラメータを検証する
-  payment_method_validator =
-    ::Reservations::PaymentMethodValidator.new(
-      @reservation,
-      payment_request: payment_request,
-      user: user,
-    )
-
-  unless payment_method_validator.valid?
-    error!(payment_method_validator.payment_method_error, 400)
+  validation_result = payment_request.validate(reservation: @reservation, current_user: user)
+
+  # 選択した支払い方法が正しくなければ構造化したエラーレスポンスを返す。
+  if validation_result.invalid?
+    error!(validation_result.message, 400)
   end

   # -- 中略 --

-  # 支払い方法が設定されているページでは支払いを実施し、レコードを作成する
-  if @booking_page.require_payment?
-    update_payment! @reservation, payment_request
-  end
+  # 支払いを実施し、レコードを作成する
+  payment_request.pay(reservation: @reservation, current_user: user)
 end

置き換えができたら、 既存の PaymentMethodValidator やupdate_payment!メソッドを削除できます。

最後に、シーケンス図のbefore/afterを掲載しておきます。

before

before

after

after

エラーレスポンスを返す箇所も少なくなり、使われるクラスも少なくなって実装としてシンプルになりました。

まとめ

いかがでしたでしょうか。

実際3回ほどのPull Requestに分けて改修を実施しましたが、だいたい2日ほどですべての作業が終わっており、すぐにリターンが得られそうです。*2

エンジニア間でも「もし決済手段にコンビニ決済を追加したかったら新しいPaymentRequestを実装すればいいね」みたいな会話ができるようになりました。

STORES 予約 では日々機能の開発しながら、メンテナンス性も重要視しています。10年、20年先も使われるプロダクトでありたいと思っています。

*1: 本著にもあるのですが、大抵はif分岐で十分で、ある特定のラインで複雑度を超えたときにリファクタリングを行えば良いと考えています。

*2: PaymentRequest単体である程度テストも書きやすくなったため、エッジケースのテストを書くことができるようになり、既に嬉しいことも多いです。