概要
この記事は、以下に投稿したソースをリファクタリングするという記事です。
テスト困難なコードを書く
https://qiita.com/oda-kazuki/items/bac33094e82b0f51da41
テスタブルなコードの結論
以下のようにコーディングしましょう。
- クラスはどんどん分割する
- 最終的にひとつのことだけに専念させるようにする
- 特にグローバルなリソースへのアクセスは一箇所にまとめる
- メンバ変数の初期化やSingletonの利用はDependency Injectionで行う
- 外部ライブラリの生成箇所は場合によってはラップするファクトリクラスを作る
- 読みやすさを重視 （処理を簡単に理解できるようにするため)
- 早期リターンを心がける
- 変数のスコープは最小限に
- メソッド名や変数名にはこだわる
- Mapはなるべくは利用しない
これらをやることで、テストは非常に容易になります。それでは、前回のコードをリファクタリングしていきます。
凝集度を高める
前回書いた通り、LoginServiceは複数の役割を担っていました。
まずは、その中の最もな害悪である「WebAPIの直接呼び出し」をやめます。そちらを外だししてしまいましょう。
モデルの作成
ただその前に、Mapで表現してたアカウント情報やユーザー情報のクラスを作成します。
本当は中にも色々と処理をいれたいところですが、今回のだと特に使わなそうなのでDTOっぽい感じになります。
@Data
public class AuthToken {
private String token;
}
@Data
public class AuthResult {
private String accountId;
}
@Data
public class Account {
// 今回のでは使わないけど、一応こういうのがあるという意味で入れときます
private String accountId;
}
@Data
public class User {
// 今回のでは使わないけど、一応こういうのがあるという意味で入れときます
private String name;
}
ファクトリメソッドの作成
また、今回は JsonMapper というライブラリを利用してる(という設定)なのですが、こいつがジェネリクスを指定する必要があるので、ちょっと使いづらいです。なので、それ用のファクトリメソッドも作成します。単純に対象のJsonMapperを作成するだけの簡単なクラスです。
public class JsonMapperFactory {
public JsonMapper<Account> createAccountMapper() {
return new JsonMapper<>();
}
public JsonMapper<AuthToken> createAuthMapper() {
return new JsonMapper<>();
}
public JsonMapper<AuthResult> createAuthResultMapper() {
return new JsonMapper<>();
}
public JsonMapper<User> createUserMapper() {
return new JsonMapper<>();
}
}
WebAPIを閉じ込める
では、まず HttpClient というものを作成することにします。その中にWebAPIを直接呼び出す部分を閉じ込めます。
public abstract class HttpClient {
protected final Client client;
protected final JsonMapperFactory mapperFactory;
public HttpClient(Client client, JsonMapperFactory factory) {
this.client = client;
this.mapperFactory = factory;
}
}
では、次は実際にWebAPIを呼び出す AuthClient と UserClient を作成します。
public class AuthClient extends HttpClient {
public AuthClient(Client client, JsonMapperFactory factory) {
super(client, factory);
}
public AuthResult authorize(String accessToken) throws HttpException {
JsonMapper<AuthToken> tokenMapper = factory.createAuthMapper();
AuthToken token = new AuthToken(accessToken);
Json json = client.post("/auth" , tokenMapper.serialize(token)).body();
JsonMapper<AuthResult> resultMapper = factory.createAuthResultMapper();
return resultMapper.deserialize(json);
}
public Account fetchAccount(String accountId) throws HttpException {
Json json = client.get("/accounts/" + accountId).body();
JsonMapper<Account> accountMapper = factory.createAccountMapper();
return accountMapper.deserialize(json);
}
}
public class UserClient extends HttpClient {
public UserClient(Client client, JsonMapperFactory factory) {
super(client, factory);
}
public User fetchUser(Account account) throws HttpException {
JsonMapper<Account> accountMapper = factory.createAccountMapper();
Json json = client.post("/users" , accountMapper.serialize(account)).body();
JsonMapper<User> userMapper = factory.createUserMapper();
return userMapper.deserialize(json);
}
public void logout(User user) throws HttpException {
JsonMapper<User> userMapper = factory.createUserMapper();
client.post("/logout", userMapper.serialize(user));
}
}
Cacheクラスの修正
次に、キャッシュも修正します。
最終的にはシングルトンとして利用はするのですが、一旦はSingletonパターンでの記述をなくします。
public class Cache {
private User loggedInUser;
@Getter @Setter
private String accessToken;
@Getter
private Account loggedInAccount;
public Cache() {
this.reset();
}
public boolean isLoggedIn() {
return loggedInUser != null;
}
public String cacheToken(String accessToken) {
if (accessToken != null) {
this.accessToken = accessToken;
}
return this.accessToken;
}
public void cacheAll(String accessToken, Account account, User user) {
this.accessToken = accessToken;
this.loggedInAccount = account;
this.loggedInUser = user;
}
public void reset() {
this.accessToken = null;
this.loggedInAccount = null;
this.loggedInUser = null;
}
}
こう見ると、もともとすごい無駄なことしてるなぁとも思いますが、リファクタリングなのでそのままとしましょう。
Dependency Injection
さて、これでリファクタリングの準備もできました。
ただその前に、テスタブルなコードを書く切り札であるDI(Dependency Injection)について軽く紹介します。
詳しく知りたい方は、以下を見るほうが良いと思います。
猿でも分かる! Dependency Injection: 依存性の注入
https://qiita.com/hshimo/items/1136087e1c6e5c5b0d9f
こちらは、日本語では「依存性の注入」と言います。その名の通り、外部から依存するファイルを入れてあげるというデザインパターンです。
一番簡単なDIは、コンストラクタでメンバ変数を渡すというものです。
たとえば上記の「AuthClient」や「UserClient」のやり方がそれになります。
public class UserClient extends HttpClient {
public UserClient(Client client, JsonMapperFactory factory) {
super(client, factory);
}
// ︙
}
それの何がいいのか
テストする際、依存してるファイルを簡単にMock化できます。
基本的に、クラスはメンバ変数に依存しています。それがデータであれば単なる「情報」として扱えばいいだけですが、メソッドに依存しているとテストの際にMock化する必要があるケースがでてきます。
※ 特にそれが外部リソースやグローバルなリソースに依存していると、テストがしづらいです。
Dependency Injectionパターンを用いることで、テストする際にそういった依存している部分をモックに容易に置き換えることができるようになります。
Google Guice
Javaの場合、Google Guice や Springなど、強力なDIライブラリが多数存在しています。
今回はGoogle Guiceを使うということにしましょう。
LoginServiceの修正
さて、いよいよ今回テストしたかったLoginServiceをなおしていきます。
その前に、 LogoutService という新たなサービスがあるものとします。
(もう面倒なので、インタフェースだけ書きます)
public interface LogoutService {
/** ログインに成功したらキャッシュを全削除してtrueを返す。ログアウトに失敗したらfalseを返す */
boolean logout();
}
ということで、LoginServiceです。
public class LoginService {
@Inject // Guiceでの依存性の注入
private Cache cache;
@Inject
private AuthClient authClient;
@Inject
private UserClient userClient;
@Inject
private LogoutService logoutService;
public User login(String authToken) {
if(cache.isLoggedIn()) {
if(!logoutService.logout()) {
return null;
}
}
String token = cache.cacheToken(authToken);
return this.executeLogin(token, 0);
}
private User executeLogin(String token, int retryCount) {
if (retryCount >= 3) {
return null;
}
try {
AuthResult auth = authClient.aurhotize(token);
Account account = cache.getAccount();
if (account == null) {
account = authClient.fetchAccount(auth.getAccountId());
}
User user = userClient.fetchUser(account);
if (user == null) {
return null;
}
cache.cacheAll(token, account, user);
return user;
} catch (HttpException e) {
if (e.getReason() == HttpException.Reason.TIMEOUT) {
return this.executeLogin(authToken, ++retryCount);
}
return null;
}
}
}
だいぶシンプルになりました。
なお、このクラスを使う際には、Guiceでは以下のようにしてインジェクトします。
Injector injector = Guice.createInjector(() -> {
bind(Cache.class).toProvider(() -> {
return new Cache();
}).asEagerSingleton();
bind(AuthClient.class).toProvider(() -> {
return new AuthClient(HttpClientFactory.createClient("https://account.example.com"), new JsonMapperFactory());
}).asEagerSingleton();
bind(UserClient.class).toProvider(() -> {
return new UserClient(HttpClientFactory.createClient("https://example.com"), new JsonMapperFactory());
}).asEagerSingleton();
bind(LogoutService.class).to(LogoutServiceImpl.class);
});
// インスタンス生成
LoginService service = injector.getInstance(LoginService.class);
Google Guice 使い方メモ
https://qiita.com/opengl-8080/items/6fb69cd2493e149cac3a
実際のテスト
では、実際にテストの例を書いてみます。
メンバ変数はインジェクトできるわけですので、テストでもMockをインジェクトしてあげることで簡単にテストできるようになります。
class LoginServiceTest {
@Mock
Cache cache;
@Mock
AuthClient authClient;
@Mock
UserClient userClient;
@Mock
LogoutService logoutService;
Injector injector;
@Before
public void setup() {
injector = Guice.createInjector(() -> {
bind(Cache.class).toInstance(this.cache);
bind(AuthClient.class).toInstance(this.authClient);
bind(UserClient.class).toInstance(this.userClient);
bind(LogoutService.class).toInstance(this.logoutService);
});
}
@Test
public void 正常にログインできる() {
Account account = new Account("accountId");
User expectedUser = new User("name001");
LoginService test = injector.getInstance(LoginService.class);
when(cache.isLoggedIn()).thenReturn(false);
when(cache.cacheToken("token01")).thenReturn("token01");
when(authClient.authorize("token01")).thenReturn(new AuthResult("account01"));
when(cache.getAccount()).thenReturn(null);
when(authClient.fetchAccount("account01")).thenReturn(account);
when(userClient.fetchUser(account)).thenReturn(expectedUser);
User actualUser = test.login("token01");
Assert.that(actualUser, is(expectedUser));
// 呼ばれた / 呼ばれていない の確認
verify(logoutService, never()).logout();
verify(cache, times(1)).cacheAll("token01", acount, expectedUser);
}
}
こんな感じです。最初のやつと比べても、断然テストはしやすくなってるんじゃないかとは思います。
その他にもクラスが増えたのでテストの総数はあまり変わらないかもしれませんが、ひとつひとつのクラスのテストの作りやすさは最初より断然やりやすくなってるはずです。
もっとテストしやすくするために
今回は割愛したのですが、実際にはこの executeLogin は更に簡単にすることができるでしょう。
たとえば、 アクセストークンを取得した際ってだいたいは最終的にユーザー情報などを取得するためになると思うので、以下の処理をするサービスクラスを外だしし、アクセストークンを渡して、アカウントとユーザー情報を取得するサービスクラスを作ると LoginServiceはリトライとキャッシュ管理のみに専念させることができるようになるでしょう。
AuthResult auth = authClient.aurhotize(token);
// ここでキャッシュ読んでるから、ここをどうやってLoginServiceに残しつつこの全体の処理を外だしするかは要検討だけど
Account account = cache.getAccount();
if (account == null) {
account = authClient.fetchAccount(auth.getAccountId());
}
User user = userClient.fetchUser(account);
if (user == null) {
return null;
}
「クラスを分割しすぎると何があるかわからなくなるんじゃないか」と心配する人もいるかもしれませんが、名前やパッケージ構成などを気にしていたら全然問題ありません。実際、ぼく自身クラス数が1000を超えるシステムを作ったことがありますが、どのパッケージに何を入れておくかさえちゃんとしていたらわからなくなるということもありませんでした。
ところで、JsonMapperFactoryは何だったのか
これも割愛をしていますが、このJsonMapperFactoryを作ることで、今度は UserClient と AuthClient のテストをするのが容易になります。なぜなら、その際のテストではClientとJsonMapperFactoryをMock化することができるからです。
このClientとJsonMapperは外部ライブラリ(という設定)なので、それらをテストする必要はありません。なので、容易にMock化できるととてもテストがしやすくなります。
このように、外部ライブラリの生成をラップ化するのはテスタブルなコードを書く第二の切り札です。
全部そうする必要はないですが、今回のJsonMapperのようにジェネリクスなものであれば、ファクトリクラスを作るのはとても有用です。
最後に
もうひとつ重要なこととして、コードの読みやすさがあります。
テスト困難なコードを書く でも書きましたが、正しくテストするためには正しく処理を理解できることがとても大事です。
そのため、最初に書いた結論の部分はまもっていくようにするのが良いかと思います。