Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REFACTOR] AOP를 통한 쿠키 관리 로직 분리 #456

Merged
merged 8 commits into from
Jan 20, 2025
11 changes: 11 additions & 0 deletions backend/src/main/java/ddangkong/aop/cookie/DeleteCookie.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package ddangkong.aop.cookie;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.CLASS)
public @interface DeleteCookie {
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package ddangkong.controller.room;
package ddangkong.aop.cookie;

import ddangkong.exception.room.CipherException;
import ddangkong.exception.room.InvalidCookieException;
Expand Down
11 changes: 11 additions & 0 deletions backend/src/main/java/ddangkong/aop/cookie/IssueCookie.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package ddangkong.aop.cookie;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.CLASS)
public @interface IssueCookie {
}
11 changes: 11 additions & 0 deletions backend/src/main/java/ddangkong/aop/cookie/MemberId.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package ddangkong.aop.cookie;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target(ElementType.PARAMETER)
@Retention(RetentionPolicy.RUNTIME)
public @interface MemberId {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package ddangkong.aop.cookie;

import ddangkong.facade.room.dto.RoomJoinResponse;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.extern.slf4j.Slf4j;
import org.aspectj.lang.annotation.After;
import org.aspectj.lang.annotation.AfterReturning;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Pointcut;
import org.springframework.http.HttpHeaders;
import org.springframework.http.ResponseCookie;
import org.springframework.stereotype.Component;
import org.springframework.web.context.request.RequestContextHolder;
import org.springframework.web.context.request.ServletRequestAttributes;

@Slf4j
@Aspect
@Component
public class RoomMemberCookieAspect {

private final RoomMemberCookieEncryptor roomMemberCookieEncryptor;

public RoomMemberCookieAspect(RoomMemberCookieEncryptor roomMemberCookieEncryptor) {
this.roomMemberCookieEncryptor = roomMemberCookieEncryptor;
}

@Pointcut("@annotation(ddangkong.aop.cookie.IssueCookie)")
public void issueCookie() {
}

@Pointcut("@annotation(ddangkong.aop.cookie.DeleteCookie)")
public void deleteCookie() {
}

@AfterReturning(value = "issueCookie()", returning = "roomJoinResponse")
public void handleIssueCookie(RoomJoinResponse roomJoinResponse) {
HttpServletRequest request = getHttpServletRequest();
HttpServletResponse response = getHttpServletResponse();
String origin = request.getHeader(HttpHeaders.ORIGIN);

ResponseCookie encodedCookie = roomMemberCookieEncryptor.getEncodedCookie(roomJoinResponse.member().memberId(), origin);
response.addHeader(HttpHeaders.SET_COOKIE, encodedCookie.toString());
}

@After("deleteCookie()")
public void handleDeleteCookie() {
HttpServletRequest request = getHttpServletRequest();
HttpServletResponse response = getHttpServletResponse();
String origin = request.getHeader(HttpHeaders.ORIGIN);

ResponseCookie deleteCookie = roomMemberCookieEncryptor.deleteCookie(origin);
response.addHeader(HttpHeaders.SET_COOKIE, deleteCookie.toString());
}
Comment on lines +36 to +54
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleIssueCookie는 정상 작동을 완료했을 때 작동하는데,
handleDeleteCookie는 정장 작동 했을 뿐만 아니라 예외가 던져졌을 때도 작동하는 이유가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리턴값이 필요했기 때문에 AfterReturning를 사용했었는데, After가 '정상 작동' + '예외 발생 시'를 포함하는줄은 몰랐네요! 커찬 덕분에 학습하게 되었어요!

@Around -> @Before -> proceed(예외 발생) -> @AfterThrowing-> @After -> @Around(catch)

직접 테스트해보니까 위 같은 프로세스로 진행되더라고요!

쿠키 삭제 로직은 정상 작동을 하든, 예외가 발생하든 삭제되어야 하는 로직흐름이 맞을 것 같아요! 방을 나가고 더 이상 사용하지 못하도록 해야하기 때문이죠! 그렇기에 After로 두는게 맞을 것 같습니다!


private HttpServletRequest getHttpServletRequest() {
ServletRequestAttributes servletRequestAttributes = (ServletRequestAttributes) RequestContextHolder.currentRequestAttributes();
return servletRequestAttributes.getRequest();
}

private HttpServletResponse getHttpServletResponse() {
ServletRequestAttributes servletRequestAttributes = (ServletRequestAttributes) RequestContextHolder.currentRequestAttributes();
return servletRequestAttributes.getResponse();
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package ddangkong.controller.room;
package ddangkong.aop.cookie;

import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.web.server.Cookie.SameSite;
Expand Down
20 changes: 20 additions & 0 deletions backend/src/main/java/ddangkong/config/CookieConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package ddangkong.config;

import ddangkong.resolver.RoomMemberArgumentResolver;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;

@Configuration
@RequiredArgsConstructor
public class CookieConfig implements WebMvcConfigurer {

private final RoomMemberArgumentResolver roomMemberArgumentResolver;

@Override
public void addArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) {
resolvers.add(roomMemberArgumentResolver);
}
}
52 changes: 13 additions & 39 deletions backend/src/main/java/ddangkong/controller/room/RoomController.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package ddangkong.controller.room;

import ddangkong.aop.cookie.DeleteCookie;
import ddangkong.aop.cookie.IssueCookie;
import ddangkong.aop.cookie.MemberId;
import ddangkong.aop.logging.Polling;
import ddangkong.facade.room.RoomFacade;
import ddangkong.facade.room.dto.InitialRoomResponse;
Expand All @@ -10,17 +13,12 @@
import ddangkong.facade.room.dto.RoomSettingRequest;
import ddangkong.facade.room.dto.RoomStatusResponse;
import ddangkong.facade.room.dto.RoundFinishedResponse;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.validation.Valid;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.Positive;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseCookie;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.CookieValue;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PatchMapping;
Expand All @@ -39,21 +37,17 @@
public class RoomController {

private final RoomFacade roomFacade;
private final RoomMemberCookieEncryptor roomMemberCookieEncryptor;

@IssueCookie
@ResponseStatus(HttpStatus.CREATED)
@PostMapping("/balances/rooms")
public RoomJoinResponse createRoom(@Valid @RequestBody RoomJoinRequest request,
HttpServletRequest httpRequest,
HttpServletResponse httpResponse) {
RoomJoinResponse roomJoinResponse = roomFacade.createRoom(request.nickname());
setEncryptCookie(httpRequest, httpResponse, roomJoinResponse.member().memberId());
return roomJoinResponse;
public RoomJoinResponse createRoom(@Valid @RequestBody RoomJoinRequest request) {
return roomFacade.createRoom(request.nickname());
}

@GetMapping("/balances/rooms/member")
public RoomMemberResponse getRoomMemberInfo(@CookieValue(name = "${cookie.rejoin-key}") String cookieValue) {
return roomFacade.getRoomMemberInfo(roomMemberCookieEncryptor.getDecodedCookieValue(cookieValue));
public RoomMemberResponse getRoomMemberInfo(@MemberId Long memberId) {
return roomFacade.getRoomMemberInfo(memberId);
}

@Polling
Expand All @@ -69,25 +63,20 @@ public void updateRoomSetting(@PathVariable @Positive Long roomId,
roomFacade.updateRoomSetting(roomId, request);
}

@IssueCookie
@ResponseStatus(HttpStatus.CREATED)
@PostMapping("/balances/rooms/{uuid}/members")
public RoomJoinResponse joinRoom(@PathVariable String uuid,
@Valid @RequestBody RoomJoinRequest request,
HttpServletRequest httpRequest,
HttpServletResponse httpResponse) {
RoomJoinResponse roomJoinResponse = roomFacade.joinRoom(request.nickname(), uuid);
setEncryptCookie(httpRequest, httpResponse, roomJoinResponse.member().memberId());
return roomJoinResponse;
@Valid @RequestBody RoomJoinRequest request) {
return roomFacade.joinRoom(request.nickname(), uuid);
}

@DeleteCookie
@ResponseStatus(HttpStatus.NO_CONTENT)
@DeleteMapping("/balances/rooms/{roomId}/members/{memberId}")
public void leaveRoom(@PathVariable @Positive Long roomId,
@PathVariable @Positive Long memberId,
HttpServletRequest request,
HttpServletResponse response) {
@PathVariable @Positive Long memberId) {
roomFacade.leaveRoom(roomId, memberId);
deleteCookie(request, response);
}

@ResponseStatus(HttpStatus.NO_CONTENT)
Expand Down Expand Up @@ -125,19 +114,4 @@ public RoomStatusResponse getRoomStatus(@NotBlank @PathVariable String uuid) {
public InitialRoomResponse isInitialRoom(@PathVariable @Positive Long roomId) {
return roomFacade.isInitialRoom(roomId);
}

private void setEncryptCookie(HttpServletRequest request,
HttpServletResponse response,
Object cookieValue) {
String origin = request.getHeader(HttpHeaders.ORIGIN);
ResponseCookie encodedCookie = roomMemberCookieEncryptor.getEncodedCookie(cookieValue, origin);
response.addHeader(HttpHeaders.SET_COOKIE, encodedCookie.toString());
}

private void deleteCookie(HttpServletRequest request,
HttpServletResponse response) {
String origin = request.getHeader(HttpHeaders.ORIGIN);
ResponseCookie deleteCookie = roomMemberCookieEncryptor.deleteCookie(origin);
response.addHeader(HttpHeaders.SET_COOKIE, deleteCookie.toString());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package ddangkong.resolver;

import ddangkong.aop.cookie.MemberId;
import ddangkong.aop.cookie.RoomMemberCookieEncryptor;
import ddangkong.exception.room.NotFoundCookieException;
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import java.util.Arrays;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.core.MethodParameter;
import org.springframework.stereotype.Component;
import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.method.support.ModelAndViewContainer;

@Component
public class RoomMemberArgumentResolver implements HandlerMethodArgumentResolver {

private final RoomMemberCookieEncryptor roomMemberCookieEncryptor;

private final String cookieKey;

public RoomMemberArgumentResolver(RoomMemberCookieEncryptor roomMemberCookieEncryptor, @Value("${cookie.rejoin-key}") String cookieKey) {
this.roomMemberCookieEncryptor = roomMemberCookieEncryptor;
this.cookieKey = cookieKey;
}

@Override
public boolean supportsParameter(MethodParameter parameter) {
boolean isLongType = parameter.getParameterType().equals(Long.class);
boolean hasParameterAnnotation = parameter.hasParameterAnnotation(MemberId.class);
return isLongType && hasParameterAnnotation;
}

@Override
public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer,
NativeWebRequest webRequest, WebDataBinderFactory binderFactory) {
HttpServletRequest request = webRequest.getNativeRequest(HttpServletRequest.class);
Cookie[] cookies = request.getCookies();
if (cookies == null) {
throw new NotFoundCookieException();
}
Cookie cookie = Arrays.stream(request.getCookies())
.filter(curCookie -> curCookie.getName().equals(cookieKey))
.findAny()
.orElseThrow(NotFoundCookieException::new);

String cookieValue = cookie.getValue();
return roomMemberCookieEncryptor.getDecodedCookieValue(cookieValue);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import ddangkong.aop.cookie.EncryptionUtils;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 아래서 사용되는 import 문 맞을까요? 확인 부탁드릴께요~

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어떤 의도에 질문인지 잘 이해가 안됐어요. 좀 더 자세히 설명이 가능할까요??
EncryptionUtils 객체에 대한 테스트니까 EncryptionUtils를 import 하는게 맞지 않나요??

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅇ? 이 때 안쓰는 import로 봤던 것 같은데... 아니었나?
죄송합니다. 이게 2주 넘은 코멘트라 기억이 안나네요. 넘어가죠.

import ddangkong.controller.BaseControllerTest;
import ddangkong.exception.room.InvalidCookieException;
import org.junit.jupiter.api.Nested;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.assertj.core.api.Assertions.assertThat;

import ddangkong.aop.cookie.RoomMemberCookieEncryptor;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

import ddangkong.controller.BaseControllerTest;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Import;
import org.springframework.http.HttpHeaders;
import org.springframework.restdocs.RestDocumentationContextProvider;
import org.springframework.restdocs.RestDocumentationExtension;
Expand All @@ -18,6 +19,7 @@
import org.springframework.web.context.WebApplicationContext;

@ExtendWith(RestDocumentationExtension.class)
@Import(TestCookieConfig.class)
public abstract class BaseDocumentationTest {

@Autowired
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package ddangkong.documentation;

import ddangkong.aop.cookie.EncryptionUtils;
import ddangkong.aop.cookie.RoomMemberCookieEncryptor;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
public class TestCookieConfig {

@Bean
public RoomMemberCookieEncryptor roomMemberCookieEncryptor() {
return new RoomMemberCookieEncryptor(encryptionUtils(), "test_cookie");
}

@Bean
public EncryptionUtils encryptionUtils() {
return new EncryptionUtils("AES", "1234567890123456");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 그냥 테스트에 있는 application.yml을 사용할 것 같은데, 따로 Config로 지정해주신 이유가 있나요? (정확히 어떤 에러가 있어 도입하게 되었는지 설명해주세요~)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기존의 Documentation 테스트가 @WebMvcTest 로 동작하고 있었는데 쿠키관련 로직이 AOP로 빠지게 되면서
빈을 조회하는 로직이 제대로 동작하지 않았습니다.
따라서 이를 해결하기 위해 TestCookieConfig를 추가하였고 BaseDocumentationTest에서 @import 했습니다.

해당 테스트에만 @Import(RoomMemberCookieAspect.class)를 추가해도 안되나요? (실험은 안해보긴 했는데, 뭔가 더 타칸이 잘 알지 않을까 싶어서 물어봤습니다~)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

만약에 어쩔 수 없이 TestCookieConfig를 유지해야 한다면 application-test.yml에 있는 값을 가져다 쓰는 것이 좋을 것 같아요. 나중에 yml 값을 바꾸었는데 실제 테스트 환경에 적용이 안될 수 있을 것 같아요.

Suggested change
@Configuration
public class TestCookieConfig {
@Bean
public RoomMemberCookieEncryptor roomMemberCookieEncryptor() {
return new RoomMemberCookieEncryptor(encryptionUtils(), "test_cookie");
}
@Bean
public EncryptionUtils encryptionUtils() {
return new EncryptionUtils("AES", "1234567890123456");
}
}
@Configuration
public class TestCookieConfig {
@Value("${cookie.rejoin-key}")
private String rejoinKey;
@Value("${encrypt.algorithm}")
private String encryptionAlgorithm;
@Value("${encrypt.secret-key}")
private String encryptionSecretKey;
@Bean
public RoomMemberCookieEncryptor roomMemberCookieEncryptor() {
return new RoomMemberCookieEncryptor(encryptionUtils(), rejoinKey);
}
@Bean
public EncryptionUtils encryptionUtils() {
return new EncryptionUtils(encryptionAlgorithm, encryptionSecretKey);
}
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

당시에 @Import(RoomMemberCookieAspect.class) 를 활용해보긴 했는데 의도한대로 동작하지 않더라고요. 테스트가 실패했습니다. 그래서 TestCookieConfig를 정의해 직접 빈을 추가해주었습니다. 하지만 오늘 해결책을 찾아서 TestCookieConfig를 사용하지 않아도 될 것 같아요! 이 부분은 수정해서 커밋할게요!

Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@
import static org.springframework.restdocs.request.RequestDocumentation.queryParameters;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import ddangkong.controller.room.RoomMemberCookieEncryptor;
import ddangkong.controller.room.EncryptionUtils;
import ddangkong.controller.room.RoomController;
import ddangkong.documentation.BaseDocumentationTest;
import ddangkong.domain.balance.content.Category;
import ddangkong.facade.balance.content.BalanceCategoryResponse;
Expand All @@ -48,13 +45,11 @@
import java.util.List;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.context.annotation.Import;
import org.springframework.http.MediaType;

@WebMvcTest(value = RoomController.class)
@Import(value = {RoomMemberCookieEncryptor.class, EncryptionUtils.class})
@SpringBootTest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

질문)
BaseDocumentationTest에서 TestCookieConfig를 import했다면 @SpringBootTest를 사용하지 않아도 TestCookieConfig에서 등록한 빈이 올라가지 않나요?
문서 테스트는 빠르게 문서를 생성하려는 목적인데, @SpringBootTest때문에 컨텍스트 새로 띄우기 + 모든 빈 등록으로 시간이 지체되지 않을까 싶어요 타칸의 생각은 어떠신가요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

문서 테스트는 빠르게 문서를 생성하려는 목적인데, @SpringBootTest때문에 컨텍스트 새로 띄우기 + 모든 빈 등록으로 시간이 지체되지 않을까 싶어요

이 부분에 대해선 저도 공감합니다! 하지만 WebMvcTest를 하게 되면 AOP를 활용할 수 없어서 SpringBootTest로 하게 되었습니다.
실제로 WebMvcTest로 돌려보면 응답 객체에 쿠키가 포함되지 않습니다ㅠㅠ
image

관련 문제를 해결할 수 없나 찾아보았는데 스택오버플로우에도 다음과 같은 답변이 있더라고요.

In order to test Aspectj you need to load whole application context using @SpringBootTest annotation (링크)

물론 다른 방법이 있을 수도 있지만 아직까지는 딱히 뾰족한 수가 보이지 않아 SpringBootTest로 두었습니다!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해결할 수 있는 방법을 찾았습니다! 간단하게 @EnableAspectJAutoProxy를 추가해주면 Aspectj를 활용할 수 있더라고요! 해당 부분은 수정해서 커밋해두었습니다!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 이번에 어드민 만들 때 같은 고민을 했었는데, 타칸 코드 참고해서 @EnableAspectJAutoProxy 도입해볼께요!

class RoomDocumentationTest extends BaseDocumentationTest {

@MockBean
Expand Down
Loading