作業ログ 機能実装その1 2019/09/07

※この記事に登場するソースコードは僕個人が開発しているプロジェクトのもので仕事とは無関係です

ざっくり説明

複数の画像を選択して、別のユーザーに送信できるという機能があります。

このアプリケーションにはフォルダという概念があって、フォルダ has_many 画像 という関係になっています。

複数の画像を選択するのが割とめんどくさいことに気付いたのでフォルダをまるごと送信できる機能を実装することにします。

方針を考える

現状画像の送信は POST /api/v1/image_transfer というエンドポイントを叩くと、 Api::V1::ImageTransferControllerのcreateアクションが呼ばれ、そのなかでImageTransferServiceというクラスが呼ばれて、受け取り側のユーザーに紐づく新しいフォルダが作られ、そのフォルダ内に選択した画像が作られます。

今回は新たに FolderTransferService というサービスクラスと

POST /api/v1/folder_transfer というエンドポイントを作って、

Api::V1::FolderTransferControllerのcreateアクションでFolderTransferServiceを呼ぶようにしようと思います。

ImageTransferServiceを使うようにしようかと考えたんですが、フォルダを受け取るように拡張するとクラスがごちゃごちゃになってしまいそうだったのでやめます。

代わりに共通化できそうなところがあればモジュールに切り出したいと思います。

作っていきます

JS側の実装も含めると結構長丁場になりそうなのでのんびりやっていきます

サービスクラスを作る

FolderTransferServiceのテストから書きます。

まずはインターフェースだけ

require 'test_helper'

class FolderTransferServiceTest < ActiveSupport::TestCase
  setup do
    @sender = users(:kentas)
    @recipient = users(:some_user1)

    @folder1 = folders(:kentas_folder1)
    @image1 = @folder1.images.create(name: 'foo1.png', thumbnail_url: 'foo1_thumbnail.png')
    @image2 = @folder1.images.create(name: 'foo2.png', thumbnail_url: 'foo2_thumbnail.png')
    @image3 = @folder1.images.create(name: 'foo3.png', thumbnail_url: 'foo3_thumbnail.png')
    blob1 = fixture_file_upload(Rails.root.join('test', 'fixtures', 'files', 'kokusai_man.png'), 'image/png')
    blob2 = fixture_file_upload(Rails.root.join('test', 'fixtures', 'files', 'cat.jpg'), 'image/jpeg')
    blob3 = fixture_file_upload(Rails.root.join('test', 'fixtures', 'files', 'fruit_lime.png'), 'image/png')
    @image1.file.attach(io: blob1, filename: @image1.name)
    @image2.file.attach(io: blob2, filename: @image2.name)
    @image3.file.attach(io: blob3, filename: @image3.name)
  end

  test "send a folder with 3 images to an existing user" do
    service = FolderTransferService.new(
      sender: @sender,
      folder: @folder1,
      recipient_usernames_or_emails: ['some_user1'],
    )
    assert(service.call)
  end
end

initializeのrecipient_usernames_or_emailsという長い名前の引数は、入力されたユーザーIDまたはEmailアドレスをそのまま受け取ります。

EmailがDBに登録されていない場合は、invitationメールを送信します。

使われ方として、同時に複数人に送ることが多いみたいなので、送信相手は複数入力可能にします。

callメソッドは成功するとtrue, エラーの場合はfalseを返します。

とりあえずインターフェースが決まったのでここまでのテストが通るようにします。

Run options: --seed 31795

# Running:

E

Error:
FolderTransferServiceTest#test_send_a_folder_with_3_images_to_an_existing_user:
NameError: uninitialized constant FolderTransferServiceTest::FolderTransferService
    test/services/folder_transfer_service_test.rb:21:in `block in <class:FolderTransferServiceTest>'


rails test test/services/folder_transfer_service_test.rb:20

現時点では当然落ちます。

class FolderTransferService < BaseService
  def call
    true
  end
end

BaseServiceのinitializeメソッドでいい感じに引数をattrに変換するように書いているのでこれだけで上で書いたテストはパスします。

Run options: --seed 2812

# Running:

.

Finished in 0.524547s, 1.9064 runs/s, 1.9064 assertions/s.
1 runs, 1 assertions, 0 failures, 0 errors, 0 skips

次は受け取り側にフォルダが作成されることを確認するテストを追加します

  test "send a folder with 3 images to an existing user" do
    service = FolderTransferService.new(
      sender: @sender,
      folder: @folder1,
      recipient_usernames_or_emails: ['some_user1'],
    )
    assert(service.call)
+    folder = @recipient.folders.find_by(name: 'kentasさんから')
+    assert_not_nil(folder)
  end

フォルダ名についてはどうするか画像の送信の場合もそうなんですが、どうしようか迷っていて、とりあえず受け取った側では「〇〇さんから」という名前になるようにしています。

ちなみに名前が被った場合は「〇〇さんから(1)」のように末尾に情報が追加されるようにFolderモデルを作っています。

UI的に誰から送られてきたのかわかるようになっていないという問題があって、とりあえずの回避策です。

きっちり考えてから作りこむのが理想なんでしょうけど、そのへんは今回はスルーでお願いします。

※あとデフォルトのロケールは:jaです

次はこれを通るようにしていきます

# Running:

F

Failure:
FolderTransferServiceTest#test_send_a_folder_with_3_images_to_an_existing_user [/home/kenta-s/Repositories/oak/test/services/folder_transfer_service_test.rb:28]:
Expected nil to not be nil.


rails test test/services/folder_transfer_service_test.rb:20

既存のImageTransferServiceに、find_or_create_recipientという、user_idかemailを表す文字列からユーザーを返すメソッドがあるので、これを今回のクラスからも使えるように切り出します。

汚い上に不要なロジックまで入っていてここに貼りたくなかったんですが、自分を戒めるためにそのまま貼ります。

常に誰に見られても恥ずかしくないコードを書くようにしていきたいですね（キリッ）

  def find_or_create_recipient(recipient_username_or_email)
    user = User.find_by(email: recipient_username_or_email) || User.find_by(username: recipient_username_or_email)
    return user if user

    unless recipient_username_or_email =~ URI::MailTo::EMAIL_REGEXP
      error_messages << I18n.t('devise.messages.user_does_not_exist', username_or_email: recipient_username_or_email)
      return
    end

    generated_password = Devise.friendly_token(8)
    user = User.create_new_user_from_email_skip_confirmation_notification(recipient_username_or_email, generated_password)
    if user.errors.present?
      error_messages = error_messages + user.errors.full_messages
      nil
    else
      InvitationMailer.with(user: user, sender: sender, tmp_password: generated_password).welcome.deliver_later
      user
    end
  end

ユーザーが見つかればそのまま返し、そうでなければ作成してinvitationメールを送信しています。

当初はinvitationメールに仮パスワードを表示するつもりだったけど、結局それをやめたのにコードが残っていました。

したがって仮パスワードにDevise.friendly_tokenを使う必要はなくSecureRandomを使うように変更できそうです。

少し寄り道になりますがこれをUserクラスに移す作業を先にやることにします。

既存のImageTransferServiceのテストを壊さないように確認しながらやっていきます

$ bundle exec rails test test/services/image_transfer_service_test.rb
Run options: --seed 17002

# Running:

.......

Finished in 1.205590s, 5.8063 runs/s, 111.1489 assertions/s.
7 runs, 134 assertions, 0 failures, 0 errors, 0 skips

ちなみにfind_or_create_recipientを削除して走らせると見事に落ちます。

Finished in 0.809332s, 8.6491 runs/s, 13.5915 assertions/s.
7 runs, 11 assertions, 0 failures, 7 errors, 0 skips

余談ですがMiniTestを書き始めたのは最近なんですが、RSpecに比べるとテストの数に対してアサーションが多くなりがちな気がします。

Userモデルに移して余計な処理を取り除く

  def self.find_or_create_recipient(sender, recipient_username_or_email)
    user = self.find_by(email: recipient_username_or_email) || self.find_by(username: recipient_username_or_email)
    return user if user

    return nil unless recipient_username_or_email =~ URI::MailTo::EMAIL_REGEXP

    user = self.create_new_user_from_email_skip_confirmation_notification(recipient_username_or_email)
    if user.save
      InvitationMailer.with(user: user, sender: sender).welcome.deliver_later
      user
    else
      nil
    end
  end

ImageTransferServiceのfind_or_create_recipient

  def find_or_create_recipient(recipient_username_or_email)
    user = User.find_or_create_recipient(sender, recipient_username_or_email)
    if user.present?
      user
    else
      error_messages << I18n.t('devise.messages.user_does_not_exist', username_or_email: recipient_username_or_email)
      nil
    end
  end

これで既存のテストは通るようになりました。

Run options: --seed 63892

# Running:

.......

Finished in 1.223707s, 5.7203 runs/s, 109.5034 assertions/s.
7 runs, 134 assertions, 0 failures, 0 errors, 0 skips

これでFolderTransferServiceからも使えるようになったので同じように書きます。

  def find_or_create_recipient(recipient_username_or_email)
    user = User.find_or_create_recipient(sender, recipient_username_or_email)
    if user.present?
      user
    else
      error_messages << I18n.t('devise.messages.user_does_not_exist', username_or_email: recipient_username_or_email)
      nil
    end
  end

initializeが受け取るrecipient_usernames_or_emailsはArrayなのでcallメソッドもそれに対応します。

def call
  recipient_usernames_or_emails.each do |recipient_username_or_email|
    recipient = find_or_create_recipient(recipient_username_or_email)
    # おっと、ここはImageTransferServiceに似た処理があるんでした。
  end

  true
end

と思ったんですがImageTransferServiceに似た処理を書いた記憶があるので見てみます

  def transfer_images(recipient_username_or_email)
    recipient = find_or_create_recipient(recipient_username_or_email)
    return false unless recipient

    folder = if photo_date.present?
      taken_at = recipient.photo_dates.find_or_create_by(taken_at: photo_date)
      taken_at.folders.create(name: folder_name)
    else
      recipient.folders.create(name: folder_name)
    end

    images.each do |image|
      next if image.thumbnail_url.nil?
      copied = folder.images.create(
        name: image.name,
        thumbnail_url: image.thumbnail_url,
      )
      copied.file.attach(image.file.blob)
    end
    sender.sent_histories.create(recipient: recipient, folder: folder)
    messages << I18n.t('services.image_transfer_service.sent_images', username_or_email: recipient_username_or_email, image_count: images.size)
  end

photo_dateって概念があったのを忘れていました。クソっ

これは撮影日という情報を持つモデルで、Folderに紐づきます。 nilでもOKなやつなのでとりあえずこのまま進めます。

このtransfer_imagesはモジュールに切り出そうと思います。

find_or_create_recipientのときと同じように、まずは例のメソッドを削除した状態でテストを走らせて失敗することを確認します

Finished in 0.769953s, 9.0915 runs/s, 14.2866 assertions/s.
7 runs, 11 assertions, 0 failures, 7 errors, 0 skips

モジュールを作成

module ImageTransferable
  def transfer_images(recipient_username_or_email)
    recipient = find_or_create_recipient(recipient_username_or_email)
    return false unless recipient

    folder = if photo_date.present?
      taken_at = recipient.photo_dates.find_or_create_by(taken_at: photo_date)
      taken_at.folders.create(name: folder_name)
    else
      recipient.folders.create(name: folder_name)
    end

    images.each do |image|
      next if image.thumbnail_url.nil?
      copied = folder.images.create(
        name: image.name,
        thumbnail_url: image.thumbnail_url,
      )
      copied.file.attach(image.file.blob)
    end
    sender.sent_histories.create(recipient: recipient, folder: folder)
    messages << I18n.t('services.image_transfer_service.sent_images', username_or_email: recipient_username_or_email, image_count: images.size)
  end
end

class ImageTransferService < BaseService
+  include ImageTransferable
  ... 省略
end

テスト通るようになりました

# Running:

.......

Finished in 1.131116s, 6.1886 runs/s, 118.4671 assertions/s.
7 runs, 134 assertions, 0 failures, 0 errors, 0 skips

このtransfer_imagesが参照しているfolder_nameというメソッドがあったのでそれもモジュールに移します。

あとさっきFolderTransferServiceにコピペしたfind_or_create_recipientも移します。

ここまでやると既存のImageTransferServiceは毛刈り後の羊のごとくやせ細ってしまいました。

class ImageTransferService < BaseService
  include ImageTransferable

  def call
    recipient_usernames_or_emails.each do |recipient_username_or_email|
      transfer_images(recipient_username_or_email)
    end
  end
end

テストももちろん通ります。

Run options: --seed 60688

# Running:

.......

Finished in 1.167899s, 5.9937 runs/s, 114.7360 assertions/s.
7 runs, 134 assertions, 0 failures, 0 errors, 0 skips

ではFolderTransferServiceに戻ります。

imagesだけ定義すれば動きそうな気配があったのでやってみます

class FolderTransferService < BaseService
  include ImageTransferable

  def call
    recipient_usernames_or_emails.each do |recipient_username_or_email|
      transfer_images(recipient_username_or_email)
    end

    true
  end

  private

  def images
    folder.images
  end
end

ようやく通りました。

Run options: --seed 22586

# Running:

.

Finished in 0.615516s, 1.6247 runs/s, 3.2493 assertions/s.
1 runs, 2 assertions, 0 failures, 0 errors, 0 skips

なんかもはや動きそうなのでテストに一気にアサーションを追加します

  test "send a folder with 3 images to an existing user" do
    service = FolderTransferService.new(
      sender: @sender,
      folder: @folder1,
      recipient_usernames_or_emails: ['some_user1'],
    )
    assert(service.call)
    folder = @recipient.folders.find_by(name: 'kentasさんから')
    assert_not_nil(folder)
    assert_equal(3, folder.images.count)
    assert_equal('foo1.png', folder.images[0].name)
    assert_equal(@image1.file.blob, folder.images[0].file.blob)
    assert_equal('foo2.png', folder.images[1].name)
    assert_equal(@image2.file.blob, folder.images[1].file.blob)
    assert_equal('foo3.png', folder.images[2].name)
    assert_equal(@image3.file.blob, folder.images[2].file.blob)
    assert_equal(["some_user1に3個の画像を送信しました"], service.messages)
  end

通りました

Run options: --seed 13537

# Running:

.

Finished in 0.575353s, 1.7381 runs/s, 17.3806 assertions/s.
1 runs, 10 assertions, 0 failures, 0 errors, 0 skips

これでとりあえずサービスクラスは完成とします（全体が動くようになったらいくつかテストケースを追加するために戻ってきます）。

長くなってきたので続きは別の記事でやります。