2023-01-23

Danger のコマンドインジェクション問題を発見して修正した

ジモティーでサーバサイドとインフラを担当している吉田です。

この前、初めて献血に行ってきました。直前の検査で手汗に気づいたスタッフから「初めてで緊張されていますか？今ならまだ止められますよ。」と声をかけられたのが、今年一番緊張した出来事です。2023年はいろんな新しい事にチャレンジする年にしたいと思います。

さて、今回は Danger の問題を発見し、報告および修正した事例を紹介します。Danger には JavaScript 版や Swift 版もあるのですが、ここでは Ruby 版について記載します。また、Danger のメンテナである Orta Therox 氏には、公開の許可を頂いています。

概要
Danger とは？
問題
影響
原因
報告と修正
最後に

概要

バージョン 8.6.0 より前の Danger では、細工された名前のブランチで実行されると、任意のコマンドを実行できる問題がありました。すでに 8.6.0 以降では修正されています。

Danger は主にリモートの CI 環境で実行されるので RCE (Remote Code Execution) もしくはコマンドインジェクションの脆弱性に該当する可能性があると考えますが、Security Advisory の作成や CVE の申請はありませんでした。

Danger とは？

Danger は CI 上で、特定のルールにしたがったコードレビューを自動化するためのソフトウェアです。

例えば、プラグインと連携することで Rubocop や Breakman といった静的解析ツールによる指摘事項を GitHub Pull Request にコメントする事が可能です。一部の制限はあるものの、GitHub だけでなく GitLab や Bitbucket などの SCM (Source Control Management) でも動作しますし、JavaScript や Swift など他のプログラミング言語にも対応しています。

問題

環境

ここでは次の環境を想定しています。

Danger のバージョン
- 8.5.0
SCM
- GitHub（非 Enterprise）
CI 環境
- GitHub Actions (GitHub-hosted runner)
- Runner Image は ubuntu-22.04

準備

適当なリポジトリを準備し、名前を ;PATH=$PWD;evil.sh; としてブランチを作成します。

$ git switch -c ';PATH=$PWD;evil.sh;'

次に、任意のコマンドを実行するシェルスクリプト evil.sh を作成します。

#!/bin/bash

export PATH=$PATH:/sbin:/bin:/usr/bin

# 任意のコマンドの例
echo "evil" > evil.txt

GitHub Actions のワークフローファイルに対して、Pull Request 作成時に次のコマンドを実行する指定を行います。その他の設定については説明を省略します。

bundle exec danger
cat evil.txt

最後に evil.sh およびワークフローファイルをコミットして、Pull Request を作成します。

実行と確認

GitHub Actions で Danger の実行が完了すると、ログに evil が出力されます。これにより evil.sh で実行したコマンドの成功が確認できます。

影響

前提として、公開設定がプライベートになっているリポジトリは外部のユーザがアクセスできないため、影響を受ける可能性は非常に低いと思います。

そうでない場合、具体な影響の例として次が考えられます。

Danger を実行する環境の破壊
リポジトリのコードの追加、変更、削除
- Danger の実行ユーザに権限があった場合
シークレットを外部に送信
Danger の実行環境からアクセス可能な外部環境へのアクセス
- 例えば、CI からテストデータ参照のため社内 AWS 環境へアクセスを許可しているケース

実際は外部のユーザがそのまま CI を実行できないようにブロックされているケースが多いです。Danger 自体の GitHub Actions ワークフローも、承認が行われてから実行される運用になっていました。

しかし、たとえ承認フローが設定されていても、レビューで細工されたコードを見落とすと実行が成功してしまいます。今回の例ではシェルスクリプトでしたが、バイナリファイルだったり、別の Pull Request で入った（入る）コードを実行する処理が難読化されていると、見落としてしまう可能性は残ります。

原因

修正前の 8.5.0 を例に、原因となったコードについて解説します。

まず、Danger の実行の過程で Danger::GitRepo クラスの git_fetch_branch_to_depth メソッドが呼び出されます。

def git_fetch_branch_to_depth(branch, depth)
  exec("fetch --depth=#{depth} --prune origin +refs/heads/#{branch}:refs/remotes/origin/#{branch}")
end

このメソッドでは、第一引数の branch と第二引数の depth から作成される文字列が同じクラスの exec メソッドに string として渡されます。

def exec(string)
  require "open3"
  Dir.chdir(self.folder || ".") do
    Open3.popen2(default_env, "git #{string}") do |_stdin, stdout, _wait_thr|

  # （省略）

上記コードから、string は git コマンドのオプションとして期待された文字列になる事が分かります。

git_fetch_branch_to_depth メソッドの branch はその名の通りブランチ名です。ブランチ名には git で許可された任意の文字列を指定できます。参考までに、ブランチ名に関する制限は Git - git-check-ref-format Documentation から確認できます。

もし depth が 20 だと仮定すると、branch の値は ;PATH=$PWD;evil.sh; なので、次の文字列

"fetch --depth=#{depth} --prune origin +refs/heads/#{branch}:refs/remotes/origin/#{branch}"

を文字列展開すると次になります。

fetch --depth=20 --prune origin +refs/heads/;PATH=$PWD;evil.sh;:refs/remotes/origin/;PATH=$PWD;evil.sh;

これは Danger::GitRepo#exec 内の Open3.popen2 によって、次のコマンドが連続して実行される事になります。

git fetch --depth=20 --prune origin +refs/heads/
PATH=$PWD
evil.sh
:refs/remotes/origin/
PATH=$PWD
evil.sh

問題となるのは3と6のコマンドです。

PATH=$PWD は環境変数の PATH にカレントディレクトリを指定しています。ここには evil.sh が存在するため、evil.sh に指定した任意のコマンドが実行できてしまいます。

また、別のファイルを実行しなくても、例えばブランチ名を ;echo${IFS}"hello"; とすると結果的に echo hello を実行する事が出来ます。これは次のドキュメントを読んで知りました。

GitHub Actions のセキュリティ強化 - スクリプトインジェクションのリスクを理解する - GitHub Docs

ブランチ名のみにインジェクションする方法は、文字やその長さに制限がある一方で、レビュー時に見落とされる可能性が比較的上がると思います。

報告と修正

Danger のソースコードを読んでいて、偶然この問題に気づきました。発見した当初は具体的に問題となる方法が思いつかず、違和感がありつつもそのままにしていました。しばらくして、今となっては何がきっかけになったか忘れたのですが、その方法が思いついた次第です。

Danger の GitHub リポジトリにはセキュリティレポートの窓口がなかったため、メンテナの Orta Therox 氏に Twitter の DM で報告しました。

その結果、「Issue に該当する」と返答をもらいましたが、数ヶ月経過しても特に動きがありませんでした。念の為再度連絡して Issue の作成により問題が公開される事を説明し、その許可をもらったため、GitHub のリポジトリ上に Issue を作成しました。

Remote Code Execution Vulnerability · Issue #1359 · danger/danger

また、あわせて修正する PR を作成しました。

Fix Remote Code Execution vulnerability by kyoshidajp · Pull Request #1360 · danger/danger
- git のオプション文字列をいったんで分割した配列にして、展開したものを Open3.popen2 に渡すようにした

すぐにマージされ、8.6.0 としてリリースされました。

最後に

Danger に存在していた問題について解説し、報告および修正した事例を紹介しました。

CI 環境は、プロダクトコードと同様に常にセキュアに保つ必要があります。次は GitHub Actions に関する資料ですが、攻撃の例や、もし被害を受けた時の影響を軽減するベストプラクティスなどが記載されており、他の CI 環境でも非常に参考になると思いますので紹介しておきます。

GitHub Actions のセキュリティ強化 - GitHub Docs

また、ジモティーではエンジニアを積極的に採用しています。

ご興味のある方はこちらをご覧ください。

https://jmty.co.jp/recruit_top/

弊社では一緒にプロダクトを改善していただける仲間を探しています！

こちらでお気軽にお声がけください！

ネイティブアプリエンジニアの採用って難しいですよね。。。

ジモティーのウェブチームについてお話ししたいです

2022-11-18

iOSアプリのスクロールヒッチとハングに関して

こんにちは、ジモティーでiOSエンジニアをしている加藤です。

今回はジモティーiOSアプリのスクロールヒッチとハングに関して書きたいと思います。

前提

スクロールヒッチとはアプリをスクロールする際に、なめらかなアニメーションにならず飛び飛びになってしまう現象のこと
ハングとはユーザーが何かしらの挙動（ボタンをタップなど）をした後、Viewの更新に250ミリ秒程度以上の遅れが生じること。その遅れの間ユーザーはUIを操作できない。
背景ジモティーのiOSアプリを触ってみると少しばかりアニメーションがカクツいてしまうということがありました。その原因を探るため、実際に調査してみる運びとなりました。

お手軽おすすめ調査方法

スクロールヒッチを XcodeOrganizerで調べる方法 XcodeOrganizerを使用する方法です。これを使用すればリリース済みのアプリであれば即座に自分のアプリのスクロールヒッチ率を確認できます。

XcodeからWindow -> Organizer からXcodeOrganizerを起動し、Scrollingをタップします。

Organizerで集計されて表示されている値はヒッチした時間/スクロールする時間です。

Appleが公式に発表している閾値も存在しており、下記の通りです。

5.0ms/s以下 => 良い
5.0ms/s ~ 10.0ms/s => あまり良くない
10.0ms/s以上 => 良くない

これを一つの指標としてパフォーマンス改善に役立てるのは良さそうですね！

ハングをデベロッパーモードで調べる方法

端末のデベロッパーモードを使用する方法です。

iOS16から新しく設定アプリにデベロッパーモードが出来ました。

これを利用して簡単にハングに関して調査することができます。

端末の設定アプリからデベロッパーをタップします

HangDetectionをタップします

Enable Hang Detectionをオンにします

これをOnの状態でアプリを起動すると自動でハングを記録してくれて、画像3枚目の画面の下の方にログとして出てきます。

もちろん、InstrumentやMetricsKitなどを使用するパフォーマンス計測の方法もございますが、今回はお手軽な二つの方法を紹介させていただきました。

調査結果とこれから

実際に調査したところ、やはり改善の余地ありという感じでした！

今後は、Instrumentsを使用してボトルネックを具体的に調査していきたいと思います。

ハング、スクロールヒッチの主な原因は下記の通りが挙げられるので重点的にここら辺を調べていくつもりです！

優先度の低いスレッドがメインスレッドをブロックしてしまっている
最適化されていない3rdパーティ-のUIライブラリをたくさん使っている。
GPUでレンダリングできる処理をCPUに任せてしまっている。(例 CoreAnimationでも良いところをUIBezierPathを使ってしまう等）
極端にViewの階層が多い。

まとめ

今回はiOSアプリのスクロールヒッチとハングに関して書いてみました。

良いUXを実現するためにも妥協したくない部分であるため、これから改善していきたいと思います👍

また、ジモティーではエンジニアを積極的に採用しています。

ご興味のある方はこちらを御覧ください。

https://jmty.co.jp/recruit_top/

弊社では一緒にプロダクトを改善していただける仲間を探しています！

こちらでお気軽にお声がけください！

ネイティブアプリエンジニアの採用って難しいですよね。。。

ジモティーのウェブチームについてお話ししたいです

2022-11-07

ジモティーのエンジニア組織の特徴

こんにちは。ジモティーのエンジニア部の執行役員をしている鈴木です。

今回の記事ではジモティーのエンジニア組織の特徴を紹介したいと思います。

下記に記載しているのは採用の面談のときにもよくお伝えしている内容で、我々がどういう考えで何を大切にしたいと思っているか、その結果生まれた今の組織の特徴です。

これをお伝えするのが弊社への入社を検討していただくうえで一番重要と考えています。

実際、弊社への入社を考えていただく過程でこのテックブログをお読みになっていただくこともあると思いましたので、この場にてアウトプットさせていただく運びになりました。

それでは行きましょう。

前提 -バリュー

いきなり各論に入るまえに全体を整理する必要があります。

今回、本題にてあげさせていただく特徴というのは、まず我々が大切にしたい価値観・行動指針（=バリュー）があって、それに基づいて生まれた「現在、具体的に行っていること」になります。

引用 : https://www.recruit-ms.co.jp/issue/column/0000000650/?theme=personnelsystem

ここのバリューの部分は別の記事にて詳細はあげさせていただこうと思いますが、簡単に話すと、行動指針は全社のものと、さらに具体度をあげたエンジニア組織版の行動指針があり、それぞれの観点ごとにひとつひとつ規定されていますがそれをサマったワンフレーズがあります。

「継続的に事業価値に直結する成果を出し続ける」

我々のバリューを一言で表現したものがこれになります。

今回お話するのは全社のビジョンをこのバリューに基づいて達成するために具体的にどういう風に普段の業務を設計しているのか、という部分になります。

1. ビジネスと近い距離で要件や仕様に関わっていきながらプロダクトを作る

ジモティーという社会貢献性が高いサービスのイチ社員であると考えたとき、エンジニアでもサービスの要件や仕様に大きく関わり、自らの創造的な働きによって得られたサービス成長を感じられるのが我々ジモティーエンジニアとしてのやり甲斐だと考えています。

弊社の開発フローは大きく 1. ビジネス要件定義 2. システム要件定義 3. 開発 4. テスト 5. リリース

と分けられています。

3〜5はイメージが付きやすいと思いますし、ビジネスと関わる部分というと1と2になりますのでここを説明していきます。

大雑把に言えば、ビジネス要件定義は「目的・課題はなにか」「何を作るか」のフェーズで、システム要件定義は「どう作るか」のフェーズになります。

フェーズ	役割	詳細 - 明確にする部分
ビジネス要件定義	・目的・課題は何か・何を作るか	・解決したい課題・ゴール・どこまでをシステムで実現するのか（スコープ）・粗いワイヤー、業務フロー・リスク
システム要件定義	どう作るか？	・工数・画面設計（モックアップ）・機能設計・細かい仕様（イレギュラー時など）・実装手段の実現可否や代替案の提示

弊社の場合、基本的にはビジネス要件定義の段階でエンジニアを投入します。

冒頭でお伝えした「やり甲斐」が目的でもありますが、生産性の観点でも合理的であると考えています。

まず「機能」を考えるとき良質なインプットがないといけません。そこからユーザーの本質的な課題は何か？ということを推察していきます。そのためにカスタマーにインタビューした内容やログデータを参照しますが、ログデータをどう分析するか？というところはエンジニアのほうが得意だったりします。どこに何のデータがあるか？どうデータを出せばよいか？そういったことは基本的にはエンジニアの得意領域です。

実際に、少し前まではシステム要件定義の段階でエンジニアをアサインしていましたが、ビジネス要件フェーズへの差し戻しが多く発生していました。

スライド1.png (83.0 kB)

現在ではビジネス要件定義フェーズからエンジニアを投入して「目的は何だっけ」「ユーザーの本質的な課題は何？」「じゃあ何作ったらよい？」というところから大きく関わっています。

こうした形で「言われたものを作る」ではなく、エンジニア自らがビジネスに深く関わっていき「事業に直結する成果」を出せるチームで有り続けたいと考えています。

2. 技術の幅を大切にしている

我々は技術は手段、と考えています。技術の幅が広がればそれだけ解決できる課題が増えると考えているのと20名弱のエンジニア組織でのリソース効率性を考えたときに複数領域にコミットできるエンジニアが多いほうがサービス成長に直接的に貢献できると考えています。そうしたエンジニアを増やすために、エンジニアが技術の幅を広げるのを支援する環境を用意しています。

具体的には各技術領域毎に技術評価項目という実務で使う技術項目がまとまっているものを用意しています。領域をコンバートした直後やジュニアのエンジニアが業務で必要な技術スキルを身につけるのに適しています。またチームとしてどの領域の技術が弱い、ということが明確になるので伸ばすべき弱点を明確にできます。

下記のイメージです。

引用 : https://udemy.benesse.co.jp/training/corporate_training/skill-map.html

実際にはもっと技術の具体度が高く、スキルを習得したいエンジニアが何をどの程度まで理解できていればよいか、というレベルを明確にしています。サンプルを記載します。

Module#include, extend, prependの違いの理解し、使い分けできる
クラスの組み合わせレベルでの設計の基本であるデザインパターン、特に、ジモティーで重要視するパターンについて説明でき、実装とレビューに応用できる
top/vmstat/ps/lsofなどのコマンドをつかって負荷・障害原因を調査できる

この粒度の技術知識習得について、まずメンバーがリーダーやマネージャー、テックリード（育成者）と学習とアウトプットの仕方を相談します。そのあとメンバーがアウトプットを出したら育成者がそれをレビュー・FBしています。

こうした形でチームと個人の技術力の向上、それによってプロダクトの成長に貢献できるように努めています。

3. リファクタリングや新技術導入による中長期視点での生産性の維持・向上を大事にしている

ジモティーは2011年から始まり、2022年現在12年目のサービスになります。12年の間にソースコードはサービスと共に進化を遂げ、新しい技術もどんどん生まれてきました。今後も5年、10年とサービスを発展させつづけるという前提に立ったときにリファクタリングや新技術導入を導入し開発を円滑に進められるようにするのが大切だと考えています。具体的には全開発予算の25%をリファクタリングや新技術導入に投下しています。

ただし、25%使えて内容は何でもよい、無鉄砲に新しい技術に手を出して良い、とは考えていません。

目安としては2年程度で投資回収できるぐらいのROIでやりましょう、としています。

例えば簡単な例だと

インフラのコンテナ化に3ヶ月の間エンジニアを一人投下する（20d/1mで20dx3の60d のリソース投下）
その結果インフラのメンテナンス工数が年間あたり30d程度削減される

この場合は2年で回収できます。（実際の数字ではなくイメージです）

また、このテックブログでも度々記事になっているフロントエンド分割なんかでは、単純な人日での工数削減ではなく、リードタイム削減が目的になります。

jmty-tech.hatenablog.com

これもざっとイメージですが、

フロント分割をして「リードタイムが半分になった」暁に、あるPRJに半年間投資しましょうとなった場合、

30d の6回リリースで（ユーザーから）6回フィードバックループを得られる

というのと

15d で12回リリースでき12回フィードバックループを得られる

というのだとプロダクトの成長スピードが全然違うというのはイメージしやすいと思います。

あとは、一回のフィードバックでプロダクトがN%改善する機会を得られる、としたときに、年間に走るPRJの数分だけ差がうまれるので、そこから大体フロントエンドの分割に2年間（1人）ぐらいはかけ続けても全然メリットのほうがでかいよね、というような議論のプロセスを経て意思決定をします。

ただ、実際はそこまで全てを数値化して天秤にかけているわけではなく、他にも、採用しやすくなるよね、とか中のエンジニアがキャリア形成しやすくなるよね、とかそういったことも含めながら感覚も含めて決めている感じです。

定量とある程度の定性的な観点も判断に加えつつ、ビジネスも含めプロダクトを作るチームとして皆が納得感をもって技術投資ができることが重要と考えています。

終わりに

長い記事に目を通していただきありがとうございます。

上記にあげさせていただいた内容も含め、バリューやそこから生まれる業務プロセスについては日々見直すことが大事と考えています。

そのためには会社としての目的・目標に対して、現場がどうワークしているか、それをきちんと観察・ヒアリングして課題を吸い上げて改善し続けていくことが重要と思います。

例えば開発フローの話もフローとしてエンジニアをビジネス要件からアサインしていこう、となったのも比較的最近の話です。ビジネスとエンジニアが阿吽の呼吸でワークするようになり、ビジネスの要件のアウトプットに対してエンジニアの納得感が高くなってきたりしたら、今度はエンジニアは逆に作ることに集中したほうが生産性が高い、という結論にもなりえます。

絶対的な解はなく、内外の変化に対して柔軟に対応していく、それが素敵な組織だと私は考えています。

We're hiring！

現在、ジモティーでは一緒に働く仲間を募集しています！ご興味のある方はこちらを御覧ください。

https://www.green-japan.com/company/2189

弊社では一緒にプロダクトを改善していただける仲間を探しています！

こちらでお気軽にお声がけください！

ネイティブアプリエンジニアの採用って難しいですよね。。。

ジモティーのウェブチームについてお話ししたいです

2022-11-07

社内アプリを Flutter で開発して感じたこと

iOS チームの池沢と申します。ちなみに鯉党です。来季から新井監督が指揮を執る事になりましたね！どんな野球をするのか、今から楽しみです！

野球の話はさておき、つい先日まで私は Flutter を用いた業務効率改善のタスクを行っていましたので、その時のお話を書こうと思います。

アプリエンジニアにとって Flutter はホットワードだと思いますので、実際の業務として扱った所感として皆様のお役に立てたらと思います。

背景

社内であるプロジェクトが立ち上がり、その時「簡単な入力だけでジモティーに投稿できるような、簡易投稿機能が欲しい」という話が出ました。

またお話を聞いていくと、その投稿ごとに出品情報とは別で管理していきたい情報もある、ということがわかりました。

この要望を解決するため、社内ツールとしてジモティーへの投稿と管理を楽にできる API とその専用アプリを作るプランが立ち上がりました。

Flutter で開発した理由

以下、私がアプリ開発の担当者としてアサインされていたためアプリのお話がメインとなります。

そして以下の理由から、 Flutter を用いて開発を行うことになりました。

UI に細かくこだわらずに、iOS と Android で共通の UX にしたい

社員の個人端末それぞれにアプリをインストールすることを想定していたため、 iOS と Android 両方で開発する必要がありました。

また、 iOS と Android で差が生まれることは極力避けたいと考えていました（マニュアルをそれぞれで作る必要が生まれるため）。

それらの要件を満たすためにも、細かい実装をせずにマテリアルデザインにしてくれる Flutter は心強かったです。

ジモティーとして、 Flutter を開発の選択肢として持ちたい

今後、ジモティーアプリ以外の開発も行っていくかもしれないという中で、 Flutter でサクッとつくる選択肢も持ちたい、というのもありました。

そのためのお試しステップとして、今回の社内ツールの作成はちょうどよかったというのも理由の一つでした。

実際に Flutter で開発してみて

実際に作ったアプリの一部のスクリーンショットを載せます。ご覧いただいて分かるように、完全に Flutter のデフォルトの UI を活用しまくっています。

QR コードを用いて投稿画面に遷移したり、その他にも新規の画面があり、約 5 画面ほど作成しました。

それぞれの画面を Clean Architecture で作成しており、行数的には計 8000 行くらいでした。

以下に、これから Flutter を業務に使用されようと検討されている方に向けて、実際に Flutter で開発出来るようになるまでの経緯などを記載していきます。

Flutter のキャッチアップ

個人でも Flutter を軽く触っていたので肌感になってしまうのですが、 Flutter を学習ハードルはかなり低そうと感じました（一週間くらいお試しで動かしていれば十分そう）。

Flutter には Hot Reload / Hot Restart という機能がありビルドし直すということがほぼ発生しないため、調べたことの動作確認がすぐでき、実装しながら覚えていく方針で十分だと感じています。

事前の期間が必要になるものとしては、実装したい機能が Flutter で実現できるか？を調べることだと思います。

Flutter が各プラットフォームの API （カメラや GPS など）を利用するためには、 Method Channel という機能を利用し各ネイティブコードを呼び出すことになります。

その実装が必要かどうかは、あらかじめ調査したほうがいいと思います。

ただ、多くの人がその部分をパッケージとして公開してくれているため、あまり必要になるケースは多くない、という気はしています。

開発期間

今回のアプリの開発期間は、約 2 ヶ月でした。

ただ、要件をまとめたり、仮実装した後のデザイン修正期間などにも多く時間を割いていたため、実際に開発していた時間としては 1 ヶ月半くらいだったと思います。

まとめ

私が実際に業務として Flutter を扱った際の結果や感想などを記載しました。

業務に Flutter の導入を検討されている方や、 Flutter でアプリを作ってみたいと考えている方の一つの参考になりましたら幸いです。

記載した感想の他に、 Flutter での開発体験についてとても素晴らしいと感じていまして、引き続き Flutter での開発タスクが増えていくと嬉しいなと感じました。

あと余談ですが、実はバックエンド担当者も私と同じく、普段は他領域を触っている人が新しく挑戦したいと表明し実装を行った人でした。

紆余曲折ありいろいろ相談しながらですが、リリースまで無事に達成することができ、大きな問題等発生しなかったことがとても嬉しかったです。

最後に

ジモティーでは、一緒に課題を解決してくれるエンジニアを募集中です！ iOS エンジニアであった私が Flutter に挑戦できたように、様々な技術的挑戦を支援してくださる環境だと思います。

大きな成長を遂げたいという強い想いをお持ちの方にジョインしていただけることを待ち望んでおります！

https://jmty.co.jp/recruit_top/

弊社では一緒にプロダクトを改善していただける仲間を探しています！

こちらでお気軽にお声がけください！

ネイティブアプリエンジニアの採用って難しいですよね。。。

ジモティーのウェブチームについてお話ししたいです

2022-08-31

Androidでテスト駆動開発

自己紹介

Androidエンジニアの坂本です。

Android未経験で3月末に入社して約半年になります。

入社前は、完全未経験の状態からiOSの勉強を独学で1年ほどやった程度。

そこから初めてジモティーのインターンでAndriodをすることになり今に至るといった感じです。

課題

ジモティーでは既存のテストコードはあるものの、工数だったりスピード感の兼ね合いでテストコードが書けていないこともあります。

今回はテストコードのメリットを確認しながら、簡単な機能をテスト駆動開発してみます。

テストコードのメリット

仕様書になる
- テストコードを見ると何をしているのか分かる
無駄な開発時間を短縮出来る
- コード修正のたびにビルドしなくてもいい
  - ロジックの実装はテストコードで担保して、最後に挙動確認でビルドする
設計を見直すきっかけになる
心理的安心が生まれる
- ここのロジックは間違いないという確信を思って開発が進められる
  - （自分のコードを疑わなくていいということではない）

テスト駆動開発でやってみる

同期的な処理

ViewModelを作成してメソッドだけ用意

class CountViewModel : ViewModel() {
    
    private val _totalCount: MutableLiveData<Int> = MutableLiveData()
    val totalCount: LiveData<Int> = _totalCount
    
    fun onClickPlusButton() {

    }
}

テストを作成

テスト実装

実装したいロジックをテストするコードを書く

ViewModelのメソッドを呼んだ時に、LiveDataが期待した値で更新されているかのテスト

@RunWith(AndroidJUnit4::class)
class CountViewModelTest {
    @get:Rule
    val rule: TestRule = InstantTaskExecutorRule()

    private lateinit var countViewModel: CountViewModel

    @Mock
    lateinit var mockTotalCountObserver: Observer<Int>

    @Before
    fun setUp() {
        countViewModel = CountViewModel()
        MockitoAnnotations.initMocks(this)
    }

    @Test
    fun onClickPlusButton_プラスボタンをクリックした時_渡された値の合計が通知できていること() {
        countViewModel.totalCount.observeForever(mockTotalCountObserver)

        countViewModel.onClickPlusButton(10, 20)

        verify(mockTotalCountObserver).onChanged(30)
    }
}

失敗することを確認する

正しい処理を実装

class CountViewModel : ViewModel() {
    
    private val _totalCount: MutableLiveData<Int> = MutableLiveData()
    val totalCount: LiveData<Int> = _totalCount
    
    fun onClickPlusButton() {
        _totalCount.value = firstNumber + secondNumber
    }
}

無駄な開発時間を短縮出来る

プロジェクトが大きくなってくると、1回ビルドするのに5分10分かかることもあると思います。

ロジックを変更する際に毎回ビルドしていると、ビルド待ちの時間も増えてきます。

テストの実行はビルドの時間に比べると短いので、テストでロジックが正しく実装されていることを確認してから最後にビルドで確認すると、ビルド回数が抑えられて効率良く開発できることにつながるかもしれません。

非同期の処理

次は非同期のテストを書いていきます。

サーバーから未読数を取得してLiveDataを更新するという想定でテスト書いてみます。

作成したテスト

fetchTotalUnreadCountUseCaseのモックを作成し、UnreadCount(1, 2)を返すようにします。
作成したUseCaseを使ってViewModelをインスタンス化
メソッドを叩いた時にLiveDataが未読数の合計で更新されているかを検証します。

@RunWith(AndroidJUnit4::class)
class CountViewModelTest {
    @get:Rule
    val rule: TestRule = InstantTaskExecutorRule()

    private val dispatcher = TestCoroutineDispatcher()

    @Mock
    lateinit var mockTotalUnreadCountObserver: Observer<Int>

    @Before
    fun setUp() {
        MockitoAnnotations.initMocks(this)
        Dispatchers.setMain(dispatcher)
    }

    @After
    fun tearDown() {
        Dispatchers.resetMain()
        dispatcher.cancel()
    }

    @Test
    fun 未読数の合計を取得して値を渡せていること() = dispatcher.runBlockingTest {
        val fetchTotalUnreadCountUseCase = mock<FetchTotalUnreadCountUseCase>()
        whenever(fetchTotalUnreadCountUseCase.invoke())
            .thenReturn(UnreadCount(1, 2))

        val countViewModel = CountViewModel(fetchTotalUnreadCountUseCase)
        countViewModel.fetchTotalUnreadCount()
        countViewModel.totalUnreadCount.observeForever(mockTotalUnreadCountObserver)
        val actual = countViewModel.totalUnreadCount.value
        assertEquals(3, actual)
    }
}

FetchTotalUnreadCountUseCaseは非同期想定なのでsuspendを付けておきます。

class FetchTotalUnreadCountUseCase {
    private val unreadCountRepository: UnreadCountRepository = UnreadCountRepositoryImpl()

    suspend fun invoke(): UnreadCount {
        return unreadCountRepository.fetchUnreadCount()
    }
}

失敗を確認する

テストコードでは未読数の合計３を期待して書く。
実装側ではfromUserCountだけで更新するような実装にしてみた。

失敗を確認できました。

実際に実装する

    fun fetchTotalUnreadCount() {
        viewModelScope.launch {
            val unreadCount = fetchTotalUnreadCountUseCase.invoke()
            _totalUnreadCount.value = unreadCount.fromUserCount + unreadCount.fromSystemCount
        }
    }

テスト成功

設計を見直す

テストコードを書こうとすると、テストが書きづらかったりして改めて実装を見直す機会になったりする時があります。

実際に先ほど作成してテストコードを見てみると、ViewModelでは合計を取得して値を渡せばよさそうですが、

    @Test
    fun 未読数の合計を取得して値を渡せていること() = dispatcher.runBlockingTest {
    }

実装側では取得した未読数の合計を求める処理も書いてあります。

    fun fetchTotalUnreadCount() {
        viewModelScope.launch {
            val unreadCount = fetchTotalUnreadCountUseCase.invoke()
            _totalUnreadCount.value = unreadCount.fromUserCount + unreadCount.fromSystemCount
        }
    }

合計の処理はモデルに移すなどの選択肢もあるかもしれません。

data class UnreadCount(
    val fromUserCount: Int,
    val fromSystemCount: Int,
) {
    val totalCount = fromUserCount + fromSystemCount
}

    fun fetchTotalUnreadCount() {
        viewModelScope.launch {
            val unreadCount = fetchTotalUnreadCountUseCase.invoke()
            _totalUnreadCount.value = unreadCount.totalCount
        }
    }

→サンプルコード

おわりに

今回はテストコードを実際に書いて、テスト駆動開発の流れとメリットを見ていきました。

毎回ビルドして確認する必要なかったり、設計の見直しができたり、良い面はかなりあると思いました。

ただ現状、テストコード書くことに慣れていないので時間がかかってしまうことの方が多いです。

全ての処理にテストを書けば良いというわけではないと思うので、テストを書く意味や目的を理解して、開発効率向上やバグの早期発見のためにうまく使っていけるようになりたいですね。

弊社では一緒にプロダクトを改善していただける仲間を探しています！

こちらでお気軽にお声がけください！

ネイティブアプリエンジニアの採用って難しいですよね。。。

ジモティーのウェブチームについてお話ししたいです

2022-05-24

Railsバージョンアップに学ぶフレームワークアップデートの進め方

自己紹介

お久しぶりです。ジモティーで2020年4月からサーバサイドエンジニアをしている水上と申します。早いもので入社3年目となり、日々案件開発と格闘する日々を送っております。

まえがき

入社3年目となり、大きめの案件開発に携わることも増えてきたので、少し前に実施したRailsのバージョンアップについて記載致します。

Rails バージョンアップ

今回のバージョンアップ前の弊社のRailsは5.2.6でした。最近Rails 7が発表され5.2系のサポートが今年6月までで切れてしまうとのことで今回のアップデートを実施した運びとなります。

作業内容

バージョンアップ先選定&バージョンアップ
アップデート後Railsが起動する状態にする
raill consoleの起動に成功したらCIを完走させる
CIでは賄いきれない箇所があるので、手動で動作テストを行う
レビュー
他部署連携テスト
リリース

以下、トピックごとに今回行った詳細を記載します。

1.バージョンアップ先選定

今回はバージョンアップ手順のドキュメントが充実していた5系から6および6系から6.1の最新である6.1.4をターゲットとしました。 RubyのバージョンがボトルネックとなりRails7へのアップデートは見送りになりました。 - Gemfileを修正しつつbundle update - bin/rails app:update 上記2点を実施

2.アップデート後Railsが起動する状態にする

今回の最初の山場がここでした。

Rails6からAutoload方式が変わって、新しくZeitwerkが推奨となりました。バージョンアップ前の記載はClassicモードとして利用可能ですが、非推奨になりました。そのため上記に対応するのか、非推奨警告を出したままclassicで行くのかの調査などに時間がかかりました。新Autoload方式に対応するために修正すべきファイルが多いため、今回は対応外とし、classicのままで次回のバージョンアップ時にZeitwerk対応を行う予定です。

3.rails consoleの起動に成功したらCIを完走させる

大きく時間がかかった箇所その２でした。

CIは開始するようになったものの初期完走時でエラー数は1800ほど発生しており、すべての修正に辟易しながらも中身を見てみると同じエラーで失敗しているものが多く、パターンを覚えて適宜対応しました。

その中でも、database cleanerとfeature testの相性が悪く、specファイル内で正しくcreateしているにも関わらず対象テストの実施時にdatabase cleanerが動作してしまい、対象テストに必要なデータまで削除されてしまう事態が発生していました。 factory_botのアップデートによりbuild及びcreateの挙動が変わり、以前までの記載だとdatabase cleanerが動き出してしまう状態に陥っていたようでしたので - buildの箇所をcreateで書き直し - 明示的に.saveを行う

等対処していたところ結果として、上記対応は不必要であり

FactoryBot.use_parent_strategy

の設定を行うことで諸々の挙動が改善しました。結果として、既存の書き方を踏襲したままアップデートすることができました。

アップデートにおける開発体験は極力変えたくなかったので上記対応にて体験を変えずにアップデートできたのは良かったです。

4.CIでは賄いきれない箇所があるので、手動で動作テストを行う

手動テストを行ったところ、Rails 5->6へのアップデートの際、デフォルトのままだとCookieの互換性が無く、ログイン状態がリセットされてしまう問題が露見しました。原因は２つ有り - ENV[“SECRET_KEY_BASE”]が未設定だった - Rails6からCookieにメタデータが埋め込まれるようになったため、後方互換性が失われた

ENV[“SECRET_KEY_BASE”]が未設定だった

に関してはRails5のメインブランチに対して、secret_key_baseをAWS Parameter Storeに保存しておき、そこから呼び出す様に変更を適応

Rails6からCookieにメタデータが埋め込まれるようになったため、後方互換性が失われた

に関しては

Rails.application.config.action_dispatch.use_cookies_with_metadata = false

を設定することで対応できました。

他部署へのテスト依頼のためのテストケース作成もこのタイミングで作成しました。ジモティーというサービス上テストすべき画面が非常に多く、自身の手動テストや、テストケースの作成が大変でした。同じ画面でも複数のパターン（ログイン済みか非ログイン状態か等）があるため、漏れがないようにしなければ障害発生は避けられません。

5.レビュー

他の作業に比べると大きな時間はかかりませんでしたが、レビュアーがレビューしやすいように工夫はしてくべきだったと悔やんでおります。コミットの区切り方やメッセージの内容等をきちんと精査し、レビュアーが見やすい状態にしておくことを今後意識します。また、メソッド名の変更等、使用箇所が多いものの命名変更はレビュー時のノイズとなるため、単なる命名変更は別PRとして切り出しておくとスムーズでした。

6.他部署連携テスト

部署ごとにテストケースの確認と実施したほうがよいテストは無いかを確認してもらいながらテストを実施しました。 CSチーム、アプリチー厶、それぞれの観点からテストケースを拡充して頂き、結果としては良いテストケースが作成できました。

7.リリース

本来の予定ではカナリアリリースを行う予定でありましたが、CSRFトークンの変更等の後方互換性の無い変更が含まれていた事により、Rails5<->Rails 6のPOSTができない問題等があり一斉リリースとなりました。そのため、テストケースの充実が更に重要でした。

おわりに

今回の様な大規模な開発を行うことで業務や、Railsについての知識がさらに充実しました。開発体験も大きく変えずにリリースまで完了できて非常に良かったです。次回以降にアップデートを行う際は今回学習した点を踏まえてよりスマートにアップデートを行いたいです。 Rails7へのアップデート担当が僕になるとは限らないため、今回の知見をドキュメントとして残しておくことで次回以降に役立てればいいなと思います。

また、弊社ではジモティーを更に成長させる為に一緒に取り組んでいただけるエンジニアを募集しています。頼りになる先輩方と一緒に開発できればと思います。（僕も頼りがいのある先輩の一員になれるよう精進中です）もし少しでも気になればお気軽にご応募ください。

弊社では一緒にプロダクトを改善していただける仲間を探しています！

こちらでお気軽にお声がけください！

ネイティブアプリエンジニアの採用って難しいですよね。。。

ジモティーのウェブチームについてお話ししたいです

2022-03-29

ANR調査とその対策のお話

はじめに

お久しぶりです。 Androidチームで活動している阿部です。前回投稿からおよそ１年、iOSでビルド速度の改善など様々な経験を積んで、Androidへコンバートしています！

今回は、Androidアプリのパフォーマンス指標としてしばしば話題に上がるANR(Application Not Responding) についてお話しできればと思っています。

弊社でもこのANRをAndroidチームが追うべきKPIの一つに設定しているのですが、昨年の10月あたりに上昇トレンドになっており問題になっていました。

調査方法と調査結果からの仮説

調査

Androidのdeveploper向けのドキュメントで紹介されているANRの要因としては以下のものが挙げられています。

メインスレッドのコードが遅い
メインスレッドの IO
ロックの競合
ブロードキャストレシーバが遅い

参考：https://developer.android.com/topic/performance/vitals/anr?hl=ja

しかし、どの要因がANRの起因になっているかはlogcatを使用したデバッグではなかなか見つけづらくなっています。さらに端末の性能の差によって発生する可能性があるANRもまちまちです。

そこで、今回はStrictMode(厳格モード)を使用した調査方法を採用していくことにしました。厳格モードはlog形式で警告を出してくれるため、問題があると思われる違反を容易に判別が可能です。

調査は以下の手順で行いました。

コンソールANRから発生件数が多い順に抽出
上位五件でActivityが特定可能なものを選択
StrictModeを適用し、上記画面で検証を行う

結果と仮説

まずは一部抜粋した結果をご覧ください。

SQLiteに関する違反が表示された

SharedPreferencesについても違反が表示
- これに関しては、1292msもかかっている様です

上記のようにandroid.os.strictmode.DiskReadViolationが多発しており、どうやらローカルDBへのアクセス中に問題がある様です。 AndroidのローカルDBをSQLiteからRoomに置き換えてみたの記事でも書かれているように、ローカルDBへのアクセスはUIスレッドで実行されるという問題点がありました。また、SharedPreferencesも意識しないとUIスレッドで実行できるようになっているため、ここにも問題がありそうです。

対策

上記で調査した結果、以下の二点の問題があることが判明しました。 1. SQLiteにUIスレッドでアクセスしてる 2. SharedPreferencesにUIスレッドでアクセスしている

1番目の問題に対しては、 AndroidのローカルDBをSQLiteからRoomに置き換えてみたを進めていくことで解決していきそうです。 (現時点で、移行は完了しています)

2番目の問題に関してはどうでしょう？たとえ、現存している全てのSharedPreferencesのアクセスをI/Oスレッドで行なったとしても、 SharedPreferencesがUIスレッドでアクセス可能なためメンバーの入れ替えが発生してしまった場合、また同じ状態になってしまう恐れがあります。

そこで、確実にスレッドを切り替えることができるJetpack DataStoreを導入していくことにしました。

Jetpack DataStoreの導入

Jetpack DataStoreはKotlin コルーチンとフローを使用して、データを非同期的に、一貫した形で、トランザクションとして保存できます。また、Preferences DataStore と Proto DataStore の2 種類がありどちらを導入するか検討する必要がありました。

Preferences DataStore では、キーを使用してデータの保存およびアクセスを行います。この実装では、定義済みのスキーマは必要ありませんが、タイプセーフではありません。

Proto DataStore では、カスタムデータ型のインスタンスとしてデータを保存します。この実装では、プロトコルバッファを使用してスキーマを定義する必要がありますが、タイプセーフです。

弊社では、カスタムデータ型のインスタンスとしてデータを保存できるメリットが大きいと判断しProto DataStoreを選択することになりました。

Jetpack DataStoreの実装例

app/src/main/protoの配下にprotoファイル作成

syntax = "proto3";

option java_package = "**.**.**.datastore.entity"; //<- 生成ファイル吐き出す階層を指定
option java_multiple_files = true;  //<- 生成ファイルを別で吐き出すか

message DataStoreExample {
  // 各フィールドの識別子として 1, 2... というフィールド番号(タグ)が必要。
  bool boolean = 1;
  string hoge = 2;
  int32 huga = 3;
}

protoファイルから生成したファイルを使える様にSerializerを追加
- androidx.datastore.core.Serializerを実装する

object ExampleSerializer : Serializer<DataStoreExample> {
    override val defaultValue: DataStoreExample
        get() = DataStoreExample.getDefaultInstance()

    override suspend fun readFrom(input: InputStream): DataStoreExample {
        try {
            return DataStoreExample.parseFrom(input)
        } catch (exception: InvalidProtocolBufferException) {
            throw CorruptionException("Cannot read proto.", exception)
        }
    }

    override suspend fun writeTo(t: DataStoreExample, output: OutputStream) {
        t.writeTo(output)
    }
}

DataStoreをラッパするManagerクラスを作成

class DataStoreExampleDataStoreManager @Inject constructor(
    context: Context,
) {
    private val Context.dataStoreExample: DataStore<DataStoreExample>
        by dataStore(
            fileName = "data_store_example.proto",
            serializer = DataStoreExampleSerializer,
        )

    private val dataStore = context.dataStoreExample

    suspend fun getString(): Flow<String> = dataStore.data
        .map { it.hoge }
        .catch { exception ->
            if (exception is IOException) {
                emit("")
            } else {
                throw exception
            }
        }
    }
    companion object {
        private var instance: DataStoreExampleDataStoreManager? = null

        fun getInstance(context: Context): DataStoreExampleDataStoreManager =
            DataStoreExampleDataStoreManager(context)
                .also { instance = it }
    }
}

※ラッパクラスは一貫性を保つためにシングルトン管理にしなければいけない。別のインスタンスからアクセスしようとするとException吐きます。

結果

上記のRoomへの移行とJetpack DataStoreの移行を小規模で行なってみた結果は以下の通りになっています

- Room移行後 -> 30日平均：0.04%減
- Jetpack DataStore移行後 -> 30日平均：0.01%増

Roomの方は効果がありましたが、Jetpack DataStoreの方は目に見えた成果が出ませんでした。この原因としては以下の二点が考えられます。

他のリリース物によるパフォーマンスの悪化によるもの
軽微な箇所の移行のみであったため、パフォーマンスの改善幅が少なかった

また、firebaseなどのsdkもSharedPreferencesを使用しているためにUIスレッドで行なっている箇所も考慮しなければいけません。

そのため、Jetpack DataStoreの移行の改善幅は小さかったと考えられます。今後は大きなSharedPreferencesも含め移行し、sdk使用箇所の使用スレッドの考慮していくことが重要になっていきそうです。

一方で、Jetpack DataStoreでの移行を行うことでcoroutines flowの考え方や使用スレッドの考え方を深めれるようになったのは大きな収穫です。また、MVVMでcoroutines flowとLiveData双方向への変換が可能になったためよりスムーズな実装が可能になったと考えています。

まとめ

今回は、ANRの調査からその対策までを簡単にご紹介していきました。結果としては、大きな改善には至らなかったのですが、Jetpack DataStoreを導入したことによりUIスレッドをブロックしないように作れるためパフォーマンスも良いアプリになっていくだろうと思っております。

またANR自体も減少したとはいえ、ロックの競合やブロードキャストレシーバが遅いなどの警告も存在していたため、まだまだ改善していく必要がありそうです。

最後に

ジモティーでは一緒に課題を解決してくれるエンジニアを募集中です！
jmty.co.jp

弊社では一緒にプロダクトを改善していただける仲間を探しています！

こちらでお気軽にお声がけください！

ネイティブアプリエンジニアの採用って難しいですよね。。。

ジモティーのウェブチームについてお話ししたいです

概要

Danger とは？

問題

環境

準備

実行と確認

影響

原因

報告と修正

最後に

目次

前提

お手軽おすすめ調査方法

ハングをデベロッパーモードで調べる方法

調査結果とこれから

まとめ

前提 -バリュー

「継続的に事業価値に直結する成果を出し続ける」

1. ビジネスと近い距離で要件や仕様に関わっていきながらプロダクトを作る

2. 技術の幅を大切にしている

3. リファクタリングや新技術導入による中長期視点での生産性の維持・向上を大事にしている

終わりに

We're hiring！

背景

Flutter で開発した理由

UI に細かくこだわらずに、iOS と Android で共通の UX にしたい

ジモティーとして、 Flutter を開発の選択肢として持ちたい

実際に Flutter で開発してみて

Flutter のキャッチアップ

開発期間

まとめ

最後に

自己紹介

課題

テストコードのメリット

テスト駆動開発でやってみる

同期的な処理

ViewModelを作成してメソッドだけ用意

テストを作成

テスト実装

失敗することを確認する

正しい処理を実装

無駄な開発時間を短縮出来る

非同期の処理

作成したテスト

失敗を確認する

実際に実装する

設計を見直す

おわりに

自己紹介

まえがき

Rails バージョンアップ

作業内容

1.バージョンアップ先選定

2.アップデート後Railsが起動する状態にする

3.rails consoleの起動に成功したらCIを完走させる

4.CIでは賄いきれない箇所があるので、手動で動作テストを行う

5.レビュー

6.他部署連携テスト

7.リリース

おわりに

はじめに

調査方法と調査結果からの仮説

調査

結果と仮説

対策

Jetpack DataStoreの導入

Jetpack DataStoreの実装例

結果

まとめ

最後に