3
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

[Ruby on Rails] Form オブジェクトの運用が辛くなったため改善案を

3
Posted at

はじめに

本記事では Form オブジェクトを運用していて大変になってきたポイントをまとめ、今後の改善案を提示します。
想定読者としてはこれから Form オブジェクトを採用しようと検討している方や、既に採用していて運用に悩んでいる方を考えています。
そのため Form オブジェクトとは何か、そのメリット・デメリットについては言及しません。これらの点について知りたい方は「Ruby on Rails Form オブジェクト」と検索すると多くの参考となる記事があるため、そちらを参照いただければと思います。

前提

本章では Form オブジェクトに関して、明文化されているものされていないものも含めどのような実態で運用されているかを整理します。

  1. Model 層のクラスを継承している Form オブジェクトを採用している
  • Model 層のクラスを継承せず ActiveModel::Model などをインクルードしているクラスも存在
  • Model 層の副作用に関するロジックは基本的にバリデーションに限定
    • バリデーション内容は DB の制約と整合性を保つことを重視
  1. Form オブジェクト内で別の Form オブジェクトを利用しているロジックが存在
  2. Form オブジェクトを継承する Form オブジェクトが存在
  3. ロジック実行は基本的に Rails Way となるように callback を利用

上記のようなルールで運用していて課題点となったのは 「2. Form オブジェクト内で別の Form オブジェクトを利用しているロジックが存在」(= 課題点 1) と「3. Form オブジェクトを継承する Form オブジェクトが存在」 (= 課題点 2) です。

課題点とその改善案

本章では前章で挙げた課題点について詳述し、改善案を提示します。

課題点 1: Form オブジェクト内で別の Form オブジェクトを利用している

Form オブジェクト内で別の Form オブジェクトを利用する、そのそもそもの動機については以下のような経緯で発生しました。
それは 単一レコードの保存処理を行う Form が元からあり、後から複数レコードを一括で保存処理をする機能の追加 です。
なぜこれが課題となるかというと、単一レコードのある処理が複数レコードの保存処理で利用する場合 N + 1 問題が発生する可能性があるからです。
また、通知処理など単一レコードの処理中のものではなく複数処理が完了した後に実行すべき処理が混在する可能性もあります。
もちろんこれを回避する方法は存在しますが、単一レコード処理の Form オブジェクトに複数レコード処理のロジックが混在することになり、責務が曖昧になることが多いです。

例として User モデルの保存処理を行う UserCreateForm が存在していて、そこに複数 User レコードを一括で保存処理を行う UserBulkCreateForm を追加する場合を考えます。
User に関する要件として以下のようなものがあるとします。

  • User は first_name, last_name という属性を持つ
  • User は複数の Tag を持つことができる (has_many :tags, through: :user_tags)

以下は UserCreateForm の例です。

# @example
# user = UserForm.new(first_name: 'Taro', last_name: 'Yamada', tag_ids: [1, 2, 3])
# if user.save
#  # 保存成功時の処理
# else
#  # 保存失敗時の処理
# end
class UserCreateForm < User
  # Tag#id の配列
  attribute :tag_ids, default: -> { [] }

  before_validation :set_tags
  validate :validate_to_tags_existence

  private

    # tags を設定
    def set_tags
      tags.build(Tag.where(id: tag_params))
    end

    # リクエストで渡された tag_params の Tag が存在するか検証
    def validate_to_tags_existence
      unexist_tags = tag_params - tags.pluck(:id)
      return if unexist_tags.blank?

      errors.add(:tag_params, :unexist_tags)
    end
end

この UserCreateForm を利用して複数 User レコードを一括で作成処理する UserBulkCreateForm は以下のようになります。

class UserBulkCreateForm
  include ActiveModel::Model
  include ActiveModel::Attributes
  include ActiveModel::Validations::Callbacks

  # Array<{ first_name: String, last_name: String, tag_ids: Array<Integer> }>
  attribute :users_params

  validates :user_params, presence: true
  validate :validate_to_users_params

  # @example
  # user_bulk_form = UserBulkForm.new(users_params: [ { first_name: 'Taro', last_name: 'Yamada', tag_ids: [1,2,3] }, ... ])
  # if user_bulk_form.save
  #  # 保存成功時の処理
  # else
  #  # 保存失敗時の処理
  # end
  def save
    valid? &&
    save_users_forms &&
    errors.empty?
  end

  private

    # users_params のフォーマットを検証
    def validate_to_users_params
      return if users_params.is_a?(Array) && users_params.all? { |p| p.is_a?(Hash) && p.key?(:first_name) && p.key?(:last_name) && p.key?(:tag_ids) }

      errors.add(:users_params, :invalid_format)
    end

    # users_params の各要素に対して UserCreateForm を利用して保存
    def save_users_forms
      ActiveRecord::Base.transaction do
        users_params.map.with_index do |user_params, index|
          user = UserCreateForm.new(user_params)
          unless user.save
            user.errors.each do |attr, msg|
              errors.add("users_params.#{index}.#{attr}", msg)
            end
          end
        end
        raise ActiveRecord::Rollback if errors.present?
      end
    end
end

この処理方法の課題点としては users_params の数だけ UserCreateForm#set_tags が呼び出され N + 1 問題が発生する可能性があることです。
局所最適な改善案としては UserCreateForm の context に :bulk のようなフラグを追加し、set_tags 内で context を判定して bulk 保存時は一括で Tag を取得するようにする方法が考えられます。
なぜ局所最適かというと UserCreateForm の責務に単一での作成処理と複数での作成処理の双方が混在しているからです。
以下はその局所最適な改善案の例です。

class UserCreateForm < User
  # 省略
  before_validation :set_tags, if: -> { validation_context != :bulk } # context: :bulk 時は set_tags をスキップ
  # 省略
end
class UserBulkCreateForm
  # 省略
  private

    # users_params の各要素に対して UserCreateForm を利用して保存
    def save_users_forms
      all_tag_ids = users_params.flat_map { |p| p[:tag_ids] }.uniq
      tags_by_id = Tag.where(id: all_tag_ids).index_by(&:id)

      ActiveRecord::Base.transaction do
        users_params.map.with_index do |user_params, index|
          user = UserCreateForm.new(user_params)
          # UserCreateForm#set_tags 内で tags_by_id を利用するように修正が必要
          unless user.save(context: :bulk) # context を :bulk に設定
            user.errors.each do |attr, msg|
              errors.add("users_params.#{index}.#{attr}", msg)
            end
          end
        end
        raise ActiveRecord::Rollback if errors.present?
      end
    end
  # 省略
end

以下は UserCreateForm, UserBulkCreateForm の改善案です。
ポイントとしては共通の #validate_to_tags_existence メソッドをモジュールに切り出し、UserCreateFormUserBulkCreateForm の両方で利用するようにしています。
また UserBulkCreateForm 内でプライベートなクラス UserCreatePrivateForm を定義し、モジュールを検証ロジックをインクルードしています。

module UserCreatable
  include ActiveSupport::Concern

  # Tag#id の配列
  attribute :tag_ids, default: -> { [] }

  def validate_to_tags_existence(tags:)
    unexist_tags = tag_ids - tags.pluck(:id)
    return if unexist_tags.blank?

    errors.add(:tag_ids, :unexist_tags)
  end
end
class UserCreateForm < User
  include UserCreatable

  before_validation :set_tags
  validate -> { validate_to_tags_existence(tags: tags) } # UserCreatable#validate_to_tags_existence を利用

  private

    # tags を設定
    def set_tags
      tags.build(Tag.where(id: tag_params))
    end
end
class UserBulkCreateForm
  include ActiveModel::Model
  include ActiveModel::Attributes
  include ActiveModel::Validations::Callbacks

  class UserCreatePrivateForm < User
    include UserCreatable
  end
  # 外部からの呼び出しを防ぐため private_constant に設定
  private_constant :UserCreatePrivateForm

  # Array<{ first_name: String, last_name: String, tag_ids: Array<Integer> }>
  attribute :users_params
  attribute :tag_by_id, default: -> { {} }

  before_validation :set_tag_by_id

  validates :user_params, presence: true
  validate :validate_to_users_params

  # @example
  # user_bulk_form = UserBulkForm.new(users_params: [ { first_name: 'Taro', last_name: 'Yamada', tag_ids: [1,2,3] }, ... ])
  # if user_bulk_form.save
  #  # 保存成功時の処理
  # else
  #  # 保存失敗時の処理
  # end
  def save
    valid? &&
    save_users_forms &&
    errors.empty?
  end

  private

    # Tag を一括で取得して tag_by_id にセット
    def set_tag_by_id
      all_tag_ids = users_params.flat_map { |p| p[:tag_ids] }.uniq
      self.tag_by_id = Tag.where(id: all_tag_ids).index_by(&:id)
    end

    # users_params のフォーマットを検証
    def validate_to_users_params
      return if users_params.is_a?(Array) && users_params.all? { |p| p.is_a?(Hash) && p.key?(:first_name) && p.key?(:last_name) && p.key?(:tag_ids) }

      errors.add(:users_params, :invalid_format)
    end

    # users_params の各要素に対して UserCreateForm を利用して保存
    def save_users_forms
      ActiveRecord::Base.transaction do
        users_params.map.with_index do |user_params, index|
          tag_ids = user_params[:tag_ids].map { |id| tag_by_id[id] }
          user = UserCreatePrivateForm.new(user_params.merge(tag_ids: tag_ids))
          unless user.save
            user.errors.each do |attr, msg|
              errors.add("users_params.#{index}.#{attr}", msg)
            end
          end
        end
        raise ActiveRecord::Rollback if errors.present?
      end
    end
end

課題点 2: Form オブジェクトを継承する Form オブジェクトが存在

Form オブジェクトを継承する Form オブジェクトが存在する動機としては 共通の属性やバリデーション、コールバックを持つ複数の Form オブジェクトが存在する ことが挙げられます。
例えば作成処理を行う Form オブジェクトと更新処理を行う Form オブジェクトが存在し、両者で共通の属性やバリデーション、コールバックを持つ場合です。
その他には Form オブジェクトを利用するユースケースが増えた場合も考えられます。
さて、ここで取り上げる課題は課題点 1 と同様に責務が曖昧になったり、可読性が低下したりすることにあると考えています。

例として CSV による User モデルの一括作成処理を行う UserBulkCreateForm が存在していて、そこに後から要件として更新処理を行う UserBulkUpdateForm を追加する場合を考えます。
作成要件は課題点 1 と同様に以下とします。

  • User は first_name, last_name という属性を持つ
  • User は複数の Tag を持つことができる (has_many :tags, through: :user_tags)

この要件をベースに CSV による一括作成の要件を以下とします。

  • CSV の各行は first_name, last_name, tag_names (カンマ区切り) を持つ
  • tag_names に存在しない Tag 名が含まれている場合は新規作成する
  • 成功した場合に一括作成を行なったことを通知する

以下は UserBulkCreateForm の例です。

class UserBulkCreateForm
  include ActiveModel::Model
  include ActiveModel::Attributes
  include ActiveModel::Validations::Callbacks

  # CSV データの文字列 (controller から渡される)
  attribute :csv_data
  # パース済みのユーザーデータの配列
  attribute :user_rows, default: -> { [] }
  # 保存対象の User レコードの配列
  attribute :users, default: -> { [] }

  before_validation :set_user_rows # CSV データをパースして user_rows にセット
  before_validation :set_users # user_rows を元に User インスタンスを build して users にセット
  before_validation :set_tag_users # Tag レコードを取得・作成して User に紐付け
  validate :validate_to_user_rows # user_rows のフォーマットを検証
  after_validation :save_users, if: -> { errors.blank? } # User レコードを保存 (バリデーション成功時のみ)
  after_validation :notify_success, if: -> { errors.blank? } # 一括作成成功の通知 (バリデーション成功時のみ)

  def save
    valid? && errors.empty?
  end

  private

    def set_user_rows
    end

    def set_users
    end

    def set_tag_users
    end

    def validate_to_user_rows
    end

    def save_users
    end

    def notify_success
    end
end

上記のような処理を行う UserBulkCreateForm が既にある状態で、更新処理を行う UserBulkUpdateForm を追加する場合を考えます。
一括作成処理と一括更新処理の要件は同様として、更新処理の場合は既存の User レコードを取得して更新する必要があります。
この要件を元に UserBulkBaseForm を共通の継承元 Form オブジェクトとして切り出し、UserBulkCreateForm, UserBulkUpdateForm を実装すると以下のようになります。

共通処理を切り出した継承元 Form オブジェクト: UserBulkBaseForm

この Form オブジェクトは元々の UserBulkCreateForm と通知処理を除いたほぼ同様のロジックを持ちます。

class UserBulkBaseForm
  include ActiveModel::Model
  include ActiveModel::Attributes
  include ActiveModel::Validations::Callbacks

  # CSV データの文字列 (controller から渡される)
  attribute :csv_data
  # パース済みのユーザーデータの配列
  attribute :user_rows, default: -> { [] }
  # 保存対象の User レコードの配列
  attribute :users, default: -> { [] }

  before_validation :set_user_rows # CSV データをパースして user_rows にセット
  before_validation :set_users # user_rows を元に User インスタンスを build して users にセット
  before_validation :set_tag_users # Tag レコードを取得・作成して User に紐付け
  validate :validate_to_user_rows # user_rows のフォーマットを検証
  after_validation :save_users, if: -> { errors.blank? } # User レコードを保存 (バリデーション成功時のみ)

  def save
    valid? && errors.empty?
  end

  private

    def set_user_rows
    end

    def set_users
    end

    def set_tag_users
    end

    def validate_to_user_rows
    end

    def save_users
    end
end

一括作成処理を行う Form オブジェクト: UserBulkCreateForm

継承元 Form オブジェクトのロジックをそのまま利用します。
通知処理は一括作成用に実装します。

class UserBulkCreateForm < UserBulkBaseForm
  after_validation :notify_success_create, if: -> { errors.blank? } # 一括作成成功の通知 (バリデーション成功時のみ)

  private

    def notify_success_create
    end
end

一括更新処理を行う Form オブジェクト: UserBulkUpdateForm

継承元の Form オブジェクトの set_users メソッドをオーバーライドして、既存の User レコードを取得してセットするロジックに変更します。
また通知処理は一括更新用に実装します。

class UserBulkUpdateForm < UserBulkBaseForm
  after_validation :notify_success_update, if: -> { errors.blank? } # 一括更新成功の通知 (バリデーション成功時のみ)

  private

    def set_users
      # user_rows の id 列を元に既存の User レコードを取得して users にセットするロジック
      # 継承元の UserBulkBaseForm#set_users をオーバーライド
    end

    def notify_success_update
    end
end

上記のように Form オブジェクトを継承して実装するとコールバックメソッドをオーバーライドしたり、同じ種類のコールバック (今回のケースでは after_validation) を継承元でも継承先でも定義したりすることになります。
結果としてこの具体例では可読性が低下して、要件の追加に伴う修正が難しくなると考えています。
解決策としては課題点と同様にモジュールに共通ロジックを切り出し、Form オブジェクト間で共有する方法が考えられます。

おわりに

本記事では Form オブジェクトの運用が辛くなった点を 2 つ挙げ、それぞれに対して改善案を提示しました。
Form オブジェクトは責務を明確にすることができる一方で、適切に運用しないとかえって責務が曖昧になることがあります。
また実装のスピードを優先するあまり、可読性の低下が起こり後から運用が辛くなることもあります。
Form オブジェクトの運用に悩んでいる方の参考になれば幸いです。

3
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
3
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?