Skip to content

Commit

Permalink
refactor: SlackClient 로직 개선 및 로깅 추가 (#637)
Browse files Browse the repository at this point in the history 
* feat: 슬랙 API 예외 처리 중복 코드 제거

* feat: 슬랙 API 예외 로깅

* refactor: request 값 로깅

* refactor: 누락된 로직 재생성

* test: builder 대신 DTO를 이용해 모킹이 제대로 되지 않던 현상 개선
  • Loading branch information
@yeon-06
yeon-06 authored Oct 20, 2022
1 parent ab94bc8 commit 63eb3b6
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 66 deletions.
116 changes: 59 additions & 57 deletions backend/src/main/java/com/pickpick/support/SlackClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
import com.pickpick.workspace.domain.Workspace;
import com.slack.api.methods.MethodsClient;
import com.slack.api.methods.SlackApiException;
import com.slack.api.methods.SlackApiRequest;
import com.slack.api.methods.SlackApiTextResponse;
import com.slack.api.methods.request.chat.ChatPostMessageRequest;
import com.slack.api.methods.request.conversations.ConversationsInviteRequest;
import com.slack.api.methods.request.conversations.ConversationsListRequest;
import com.slack.api.methods.request.oauth.OAuthV2AccessRequest;
import com.slack.api.methods.request.users.UsersIdentityRequest;
import com.slack.api.methods.response.chat.ChatPostMessageResponse;
import com.slack.api.methods.request.users.UsersListRequest;
import com.slack.api.methods.response.conversations.ConversationsInviteResponse;
import com.slack.api.methods.response.conversations.ConversationsListResponse;
import com.slack.api.methods.response.oauth.OAuthV2AccessResponse;
Expand All @@ -28,15 +30,17 @@
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;

@Slf4j
@Component
public class SlackClient implements ExternalClient {

private static final String LOGGING_INFO = "[Request] Slack Api Request - {}";
private static final String OAUTH_ACCESS_METHOD_NAME = "oauthV2Access";
private static final String USERS_IDENTITY_METHOD_NAME = "usersIdentity";
private static final String USER_LIST_METHOD_NAME = "usersList";
private static final String CHANNEL_INFO_METHOD_NAME = "conversationsInfo";
private static final String CHANNEL_LIST_METHOD_NAME = "conversationsList";
private static final String CHANNEL_INVITE_METHOD_NAME = "conversationsInvite";
private static final String CHAT_POST_METHOD_NAME = "chatPostMessage";
Expand All @@ -55,33 +59,20 @@ public SlackClient(final SlackProperties slackProperties, final MethodsClient me

@Override
public String callUserToken(final String code) {
String loginRedirectUrl = slackProperties.getLoginRedirectUrl();
OAuthV2AccessResponse response = callOAuth2(code, loginRedirectUrl);
validateResponse(OAUTH_ACCESS_METHOD_NAME, response);
OAuthV2AccessResponse response = callOAuth2(code, slackProperties.getLoginRedirectUrl());
return response.getAuthedUser().getAccessToken();
}

@Override
public WorkspaceInfoDto callWorkspaceInfo(final String code) {
String workspaceRedirectUrl = slackProperties.getWorkspaceRedirectUrl();
OAuthV2AccessResponse response = callOAuth2(code, workspaceRedirectUrl);

OAuthV2AccessResponse response = callOAuth2(code, slackProperties.getWorkspaceRedirectUrl());
return new WorkspaceInfoDto(response.getTeam().getId(), response.getAccessToken(), response.getBotUserId(),
response.getAuthedUser().getAccessToken());
}

private OAuthV2AccessResponse callOAuth2(final String code, final String redirectUrl) {
OAuthV2AccessRequest request = generateOAuthRequest(code, redirectUrl);

try {
OAuthV2AccessResponse response = methodsClient
.oauthV2Access(request);
validateResponse(OAUTH_ACCESS_METHOD_NAME, response);
return response;

} catch (IOException | SlackApiException e) {
throw new SlackApiCallException(OAUTH_ACCESS_METHOD_NAME);
}
return execute(methodsClient::oauthV2Access, OAUTH_ACCESS_METHOD_NAME, request);
}

private OAuthV2AccessRequest generateOAuthRequest(final String code, final String redirectUrl) {
Expand All @@ -99,24 +90,26 @@ public String callMemberSlackId(final String accessToken) {
.token(accessToken)
.build();

try {
UsersIdentityResponse response = methodsClient.usersIdentity(request);
validateResponse(USERS_IDENTITY_METHOD_NAME, response);
return response.getUser().getId();
} catch (IOException | SlackApiException e) {
throw new SlackApiCallException(USERS_IDENTITY_METHOD_NAME);
}
UsersIdentityResponse response = execute(
methodsClient::usersIdentity,
USERS_IDENTITY_METHOD_NAME,
request);

return response.getUser().getId();
}

@Override
public List<Member> findMembersByWorkspace(final Workspace workspace) {
try {
UsersListResponse response = methodsClient.usersList(request -> request.token(workspace.getBotToken()));
validateResponse(USER_LIST_METHOD_NAME, response);
return toMembers(response.getMembers(), workspace);
} catch (IOException | SlackApiException e) {
throw new SlackApiCallException(USER_LIST_METHOD_NAME);
}
UsersListRequest request = UsersListRequest.builder()
.token(workspace.getBotToken())
.build();

UsersListResponse response = execute(
methodsClient::usersList,
USER_LIST_METHOD_NAME,
request);

return toMembers(response.getMembers(), workspace);
}

private List<Member> toMembers(final List<User> users, final Workspace workspace) {
Expand All @@ -141,14 +134,8 @@ private Member toMember(final User user, final Workspace workspace) {

@Override
public List<Channel> findChannelsByWorkspace(final Workspace workspace) {
try {
ConversationsListResponse response = methodsClient.conversationsList(
request -> request.token(workspace.getBotToken()));
validateResponse(CHANNEL_LIST_METHOD_NAME, response);
return toChannels(response.getChannels(), workspace);
} catch (IOException | SlackApiException e) {
throw new SlackApiCallException(CHANNEL_LIST_METHOD_NAME);
}
ConversationsListResponse response = findChannels(workspace.getBotToken());
return toChannels(response.getChannels(), workspace);
}

private List<Channel> toChannels(final List<Conversation> channels, final Workspace workspace) {
Expand All @@ -163,20 +150,24 @@ private Channel toChannel(final Conversation channel, final Workspace workspace)

@Override
public Participation findChannelParticipation(final String userToken) {
try {
ConversationsListResponse response = methodsClient.conversationsList(
request -> request.token(userToken));
validateResponse(CHANNEL_LIST_METHOD_NAME, response);
ConversationsListResponse response = findChannels(userToken);

Map<String, Boolean> participation = response.getChannels()
.stream()
.collect(Collectors.toMap(Conversation::getId, Conversation::isMember));
Map<String, Boolean> participation = response.getChannels()
.stream()
.collect(Collectors.toMap(Conversation::getId, Conversation::isMember));

return new Participation(participation);
return new Participation(participation);
}

} catch (IOException | SlackApiException e) {
throw new SlackApiCallException(CHANNEL_LIST_METHOD_NAME);
}
private ConversationsListResponse findChannels(String accessToken) {
ConversationsListRequest request = ConversationsListRequest.builder()
.token(accessToken)
.build();

return execute(
methodsClient::conversationsList,
CHANNEL_LIST_METHOD_NAME,
request);
}

@Override
Expand All @@ -188,12 +179,7 @@ public void sendMessage(final Reminder reminder) {
.token(member.getWorkspace().getBotToken())
.build();

try {
ChatPostMessageResponse response = methodsClient.chatPostMessage(request);
validateResponse(CHAT_POST_METHOD_NAME, response);
} catch (IOException | SlackApiException e) {
throw new SlackApiCallException(CHAT_POST_METHOD_NAME);
}
execute(methodsClient::chatPostMessage, CHAT_POST_METHOD_NAME, request);
}

@Override
Expand All @@ -203,6 +189,7 @@ public void inviteBotToChannel(final Member member, final Channel channel) {
.token(member.getToken())
.users(List.of(member.getWorkspace().getBotSlackId()))
.build();
log.info(LOGGING_INFO, request);

try {
ConversationsInviteResponse response = methodsClient.conversationsInvite(request);
Expand All @@ -211,10 +198,25 @@ public void inviteBotToChannel(final Member member, final Channel channel) {
}
validateResponse(CHANNEL_INVITE_METHOD_NAME, response);
} catch (IOException | SlackApiException e) {
log.error(CHANNEL_INVITE_METHOD_NAME, e);
throw new SlackApiCallException(CHANNEL_INVITE_METHOD_NAME);
}
}

private <T extends SlackApiTextResponse, K extends SlackApiRequest> T execute(
final SlackFunction<T, K> slackFunction, final String methodName, final K request) {
log.info(LOGGING_INFO, request);

try {
T result = slackFunction.execute(request);
validateResponse(methodName, result);
return result;
} catch (IOException | SlackApiException e) {
log.error(methodName, e);
throw new SlackApiCallException(methodName);
}
}

private boolean isBotAlreadyInChannel(final ConversationsInviteResponse response) {
return !response.isOk() && "already_in_channel".equals(response.getError());
}
Expand Down
12 changes: 12 additions & 0 deletions backend/src/main/java/com/pickpick/support/SlackFunction.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.pickpick.support;

import com.slack.api.methods.SlackApiException;
import com.slack.api.methods.SlackApiRequest;
import com.slack.api.methods.SlackApiTextResponse;
import java.io.IOException;

@FunctionalInterface
public interface SlackFunction<T extends SlackApiTextResponse, K extends SlackApiRequest> {

T execute(K request) throws IOException, SlackApiException;
}
9 changes: 4 additions & 5 deletions backend/src/test/java/com/pickpick/auth/application/AuthServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,12 @@
import com.pickpick.support.DatabaseCleaner;
import com.pickpick.workspace.domain.Workspace;
import com.pickpick.workspace.domain.WorkspaceRepository;
import com.slack.api.RequestConfigurator;
import com.slack.api.methods.MethodsClient;
import com.slack.api.methods.SlackApiException;
import com.slack.api.methods.request.conversations.ConversationsListRequest.ConversationsListRequestBuilder;
import com.slack.api.methods.request.conversations.ConversationsListRequest;
import com.slack.api.methods.request.oauth.OAuthV2AccessRequest;
import com.slack.api.methods.request.users.UsersIdentityRequest;
import com.slack.api.methods.request.users.UsersListRequest.UsersListRequestBuilder;
import com.slack.api.methods.request.users.UsersListRequest;
import com.slack.api.methods.response.conversations.ConversationsListResponse;
import com.slack.api.methods.response.oauth.OAuthV2AccessResponse;
import com.slack.api.methods.response.oauth.OAuthV2AccessResponse.AuthedUser;
Expand Down Expand Up @@ -126,9 +125,9 @@ void registerWorkspace() throws SlackApiException, IOException {

given(slackClient.oauthV2Access(any(OAuthV2AccessRequest.class)))
.willReturn(generateOAuthV2AccessResponse(workspaceSlackId));
given(slackClient.conversationsList((RequestConfigurator<ConversationsListRequestBuilder>) any()))
given(slackClient.conversationsList(any(ConversationsListRequest.class)))
.willReturn(generateConversationsListResponse());
given(slackClient.usersList((RequestConfigurator<UsersListRequestBuilder>) any()))
given(slackClient.usersList(any(UsersListRequest.class)))
.willReturn(generateUsersListResponse(memberSlackId));
given(slackClient.usersIdentity(any(UsersIdentityRequest.class)))
.willReturn(generateUsersIdentityResponse(memberSlackId));
Expand Down
7 changes: 3 additions & 4 deletions backend/src/test/java/com/pickpick/channel/application/ChannelServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@
import com.pickpick.support.DatabaseCleaner;
import com.pickpick.workspace.domain.Workspace;
import com.pickpick.workspace.domain.WorkspaceRepository;
import com.slack.api.RequestConfigurator;
import com.slack.api.methods.MethodsClient;
import com.slack.api.methods.SlackApiException;
import com.slack.api.methods.request.conversations.ConversationsListRequest.ConversationsListRequestBuilder;
import com.slack.api.methods.request.conversations.ConversationsListRequest;
import com.slack.api.methods.response.conversations.ConversationsListResponse;
import com.slack.api.model.Conversation;
import java.io.IOException;
Expand Down Expand Up @@ -79,7 +78,7 @@ void findAll() throws SlackApiException, IOException {

channelSubscriptions.save(new ChannelSubscription(freeChat, yeonLog, 1));

given(methodsClient.conversationsList((RequestConfigurator<ConversationsListRequestBuilder>) any()))
given(methodsClient.conversationsList(any(ConversationsListRequest.class)))
.willReturn(generateConversationsListResponse(notice, freeChat, qna));

// when
Expand Down Expand Up @@ -107,7 +106,7 @@ void findChannelsHasUser() throws SlackApiException, IOException {

channelSubscriptions.save(new ChannelSubscription(freeChat, yeonLog, 1));

given(methodsClient.conversationsList((RequestConfigurator<ConversationsListRequestBuilder>) any()))
given(methodsClient.conversationsList(any(ConversationsListRequest.class)))
.willReturn(generateConversationsListResponse(notice));

// when
Expand Down

0 comments on commit 63eb3b6

Please sign in to comment.