This article is about refactoring the sources posted below.
Write code that is difficult to test https://qiita.com/oda-kazuki/items/bac33094e82b0f51da41
Let's code as follows.
Doing these makes testing a lot easier. Now let's refactor the previous code.
Last time As I wrote, Login Service played multiple roles. First of all, stop the most harmful of them, "direct call of WebAPI". Let's take it out.
But before that, create a class of account information and user information expressed in Map. Actually, I would like to add various processing inside, but this time it seems that it will not be used in particular, so it will be like a DTO.
@Data
public class AuthToken {
private String token;
}
@Data
public class AuthResult {
private String accountId;
}
@Data
public class Account {
//I will not use it this time, but I will put it in the sense that there is something like this
private String accountId;
}
@Data
public class User {
//I will not use it this time, but I will put it in the sense that there is something like this
private String name;
}
Also, this time I'm using a library called JsonMapper (setting), but it's a little difficult to use because it needs to specify the generics. So we will also create a factory method for it. It is a simple class that simply creates the target 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<>();
}
}
So, let's first create something called HttpClient. Confine the part that directly calls the Web API in it.
public abstract class HttpClient {
protected final Client client;
protected final JsonMapperFactory mapperFactory;
public HttpClient(Client client, JsonMapperFactory factory) {
this.client = client;
this.mapperFactory = factory;
}
}
Now, let's create ʻAuthClient and ʻUserClient that actually call the Web API.
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));
}
}
Then modify the cache as well. Eventually, it will be used as a singleton, but once the description in the Singleton pattern is lost.
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;
}
}
Looking at it like this, I think it's a lot of waste, but it's a refactoring, so let's leave it as it is.
Dependency Injection Now you're ready to refactor. But before that, I will briefly introduce DI (Dependency Injection), which is the trump card for writing testable code.
If you want to know more, I think you should see the following.
Even monkeys can understand! Dependency Injection: Dependency Injection https://qiita.com/hshimo/items/1136087e1c6e5c5b0d9f
This is called "dependency injection" in Japanese. As the name suggests, it is a design pattern that puts in dependent files from the outside.
The simplest DI is to pass a member variable in the constructor. For example, the above "AuthClient" and "UserClient" methods are used.
Repost
public class UserClient extends HttpClient {
public UserClient(Client client, JsonMapperFactory factory) {
super(client, factory);
}
// ︙
}
When testing, you can easily mock dependent files. Basically, ** classes depend on member variables **. If it is data, you can just treat it as "information", but if it depends on the method, there are cases where it needs to be Mocked at the time of testing.
The Dependency Injection pattern makes it easy to replace those dependent parts with mocking when testing.
Google Guice For Java, there are many powerful DI libraries such as Google Guice and Spring. Let's use Google Guice this time.
Now, let's fix the Login Service that we wanted to test this time.
Before that, suppose there is a new service called LogoutService.
(Since it is already troublesome, I will write only the interface)
public interface LogoutService {
/**If the login is successful, the cache is completely deleted and true is returned. Returns false if logout fails*/
boolean logout();
}
So it's Login Service.
public class LoginService {
@Inject //Dependency injection on 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;
}
}
}
It's much simpler. In addition, when using this class, Guice injects as follows.
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);
});
//Instance generation
LoginService service = injector.getInstance(LoginService.class);
Google Guice Usage Notes https://qiita.com/opengl-8080/items/6fb69cd2493e149cac3a
Now, let's actually write a test example. Since member variables can be injected, you can easily test them by injecting Mock even in the test.
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 You can log in normally() {
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));
//Called/Confirmation of not being called
verify(logoutService, never()).logout();
verify(cache, times(1)).cacheAll("token01", acount, expectedUser);
}
}
It is like this. I think it's much easier to test than the first one. The total number of tests may not change much as there are more classes, but ** the ease of creating tests for each class should be much easier than it was at the beginning **.
I've omitted this time, but in reality this ʻexecuteLogin` could be made even easier. For example, when you get an access token, you will usually get user information etc. in the end, so remove the service class that performs the following processing, pass the access token, and get the account and user information Creating a class of service to get ** will allow LoginService to focus solely on retries and cache management **.
AuthResult auth = authClient.aurhotize(token);
//I'm reading the cache here, so I need to consider how to leave this in the Login Service and remove this entire process.
Account account = cache.getAccount();
if (account == null) {
account = authClient.fetchAccount(auth.getAccountId());
}
User user = userClient.fetchUser(account);
if (user == null) {
return null;
}
Some people may be worried that "** If you divide the class too much, you won't know what's going on **", but if you care about the name and package structure, there is no problem. In fact, I've created a system with more than 1000 classes myself, but I didn't even know what to put in which package.
This is also omitted, but by creating this JsonMapperFactory, it will be easier to test ʻUserClient and ʻAuthClient this time. This is because ** Client and JsonMapperFactory can be Mocked ** in the test at that time.
This Client and JsonMapper are external libraries (with a setting called) so you don't need to test them. So, if you can easily convert it to Mock, it will be very easy to test.
Thus, wrapping the generation of external libraries is a ** second trump card ** for writing testable code. You don't have to do it all, but if it's generic like JsonMapper this time, it's very useful to create a factory class.
Another important thing is the readability of the code. As I wrote in Writing code that is difficult to test, it is very important to understand the process correctly in order to test correctly. Therefore, I think it is better to keep the conclusion part that I wrote at the beginning.