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

jhon3242 · 2025-01-02T08:05:23Z

Issue Number

closed #444

As-Is

쿠키를 발급하는 로직을 RoomMemberCookieEncryptor와 같은 필드로 컨트롤러에 가지고 있고, 쿠키를 설정하고 지우는 작업을 컨트롤러가 가지고 있는 것은 책임 분리 측면에서 부적절하다고 판단하였다. 이에 따라, 쿠키 설정 및 삭제 로직을 컨트롤러에서 분리하여 관리할 필요성을 느끼게 되었다.

To-Be

AOP를 활용한 쿠키 인증 로직 분리

Check List

테스트가 전부 통과되었나요?
모든 commit이 push 되었나요?
merge할 branch를 확인했나요?
Assignee를 지정했나요?
Label을 지정했나요?

Test Screenshot

(Optional) Additional Description

기존의 Documentation 테스트가 @WebMvcTest 로 동작하고 있었는데 쿠키관련 로직이 AOP로 빠지게 되면서
빈을 조회하는 로직이 제대로 동작하지 않았습니다.
따라서 이를 해결하기 위해 TestCookieConfig를 추가하였고 BaseDocumentationTest에서 @Import 했습니다.
또한 실제로 쿠키를 사용하는 RoomDocumentationTest에서는 @WebMvcTest 대신 @SpringBootTest 로 바꾸어 빈을 활용하도록 구현했습니다~

memberId -> RoomMember

leegwichan

타칸 의문가는 점들이 있어 리뷰 추가했어요!
추가로 반영이나 코멘트 남긴 후에 알려주세요~ 바로 리뷰할께요!

leegwichan · 2025-01-03T08:26:36Z

backend/src/main/java/ddangkong/aop/cookie/RoomMemberCookieAspect.java

+    @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());
+    }


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

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

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

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

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

leegwichan · 2025-01-03T08:29:05Z

backend/src/test/java/ddangkong/controller/room/EncryptionUtilsTest.java

@@ -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;


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

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

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

leegwichan · 2025-01-03T08:29:16Z

backend/src/test/java/ddangkong/controller/room/RoomMemberCookieEncryptorTest.java

@@ -2,6 +2,7 @@

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

+import ddangkong.aop.cookie.RoomMemberCookieEncryptor;


leegwichan · 2025-01-03T08:32:12Z

backend/src/test/java/ddangkong/documentation/TestCookieConfig.java

+@Configuration
+public class TestCookieConfig {
+
+    @Bean
+    public RoomMemberCookieEncryptor roomMemberCookieEncryptor() {
+        return new RoomMemberCookieEncryptor(encryptionUtils(), "test_cookie");
+    }
+
+    @Bean
+    public EncryptionUtils encryptionUtils() {
+        return new EncryptionUtils("AES", "1234567890123456");
+    }
+}


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

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

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

만약에 어쩔 수 없이 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);

}

}

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

GIVEN53

고생하셨습니다 👍🏻 질문 하나만 확인부탁드려요!

GIVEN53 · 2025-01-05T10:45:57Z

backend/src/test/java/ddangkong/documentation/room/RoomDocumentationTest.java

 import org.springframework.http.MediaType;

-@WebMvcTest(value = RoomController.class)
-@Import(value = {RoomMemberCookieEncryptor.class, EncryptionUtils.class})
+@SpringBootTest


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

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

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

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

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

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

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

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

jhon3242

일본갔다가 어제 와서 이제야 리뷰 반영하네요 하핫..🙇‍♂️
커멘트 감사합니다! 더 나은 방법을 알게되어서 수정했슴돠! 확인 부탁드려요!

jhon3242 · 2025-01-17T04:20:28Z

backend/src/main/java/ddangkong/aop/cookie/RoomMemberCookieAspect.java

+    @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());
+    }


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

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

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

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

jhon3242 · 2025-01-17T04:23:09Z

backend/src/test/java/ddangkong/controller/room/EncryptionUtilsTest.java

@@ -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;


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

jhon3242 · 2025-01-17T04:50:07Z

backend/src/test/java/ddangkong/documentation/room/RoomDocumentationTest.java

 import org.springframework.http.MediaType;

-@WebMvcTest(value = RoomController.class)
-@Import(value = {RoomMemberCookieEncryptor.class, EncryptionUtils.class})
+@SpringBootTest


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

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

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

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

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

jhon3242 · 2025-01-17T05:39:40Z

backend/src/test/java/ddangkong/documentation/TestCookieConfig.java

+@Configuration
+public class TestCookieConfig {
+
+    @Bean
+    public RoomMemberCookieEncryptor roomMemberCookieEncryptor() {
+        return new RoomMemberCookieEncryptor(encryptionUtils(), "test_cookie");
+    }
+
+    @Bean
+    public EncryptionUtils encryptionUtils() {
+        return new EncryptionUtils("AES", "1234567890123456");
+    }
+}


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

jhon3242 · 2025-01-17T05:50:30Z

backend/src/test/java/ddangkong/documentation/room/RoomDocumentationTest.java

 import org.springframework.http.MediaType;

-@WebMvcTest(value = RoomController.class)
-@Import(value = {RoomMemberCookieEncryptor.class, EncryptionUtils.class})
+@SpringBootTest


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

jhon3242 · 2025-01-17T05:57:55Z

backend/src/test/java/ddangkong/documentation/BaseDocumentationTest.java

@@ -18,6 +22,7 @@
 import org.springframework.web.context.WebApplicationContext;

 @ExtendWith(RestDocumentationExtension.class)
+@Import(value = {RoomMemberCookieEncryptor.class, EncryptionUtils.class, RoomMemberCookieAspect.class})


BaseDocumentTest에 위치시킨 이유:
@WebMvcTest는 Resolver를 스프링 컨테이너에 등록합니다. 이 과정에서 RoomMemberCookieAspect와 그 하위 빈들이 필요합니다. 따라서 @WebMvcTest를 사용하는 모든 테스트에서 해당 빈들을 import해야 하므로, 이를 공통적으로 처리하기 위해 BaseDocumentTest에 위치시켰습니다.”

leegwichan

일본은 조심히 갔다 오셨나요~?
리뷰한 부분 다 확인했습니다! Approve 하겠습니다!

leegwichan · 2025-01-17T08:01:56Z

backend/src/test/java/ddangkong/documentation/room/RoomDocumentationTest.java

 import org.springframework.http.MediaType;

-@WebMvcTest(value = RoomController.class)
-@Import(value = {RoomMemberCookieEncryptor.class, EncryptionUtils.class})
+@SpringBootTest


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

jhon3242 added 6 commits December 26, 2024 16:21

feat: memberIdArgumentResolver 구현 #444

2a0b67a

feat: ArgumentResolver를 활용한 memberId 캐스팅 구현 #444

e5ade1c

feat: AOP를 활용한 쿠키 발급 및 삭제 구현 #444

1eeb886

feat: 커스텀 애노테이션 RetentionPolicy 값 CLASS로 변경 #444

0f647dc

feat: 네이밍 통일 #444

6e1c8fb

memberId -> RoomMember

test: AOP 등록된 쿠키 관련 빈 조회를 위해 Config 추가 #444

8406d6e

jhon3242 added ♻️ refactor 리팩토링 🍃 BE back end labels Jan 2, 2025

jhon3242 requested review from leegwichan, PgmJun and GIVEN53 January 2, 2025 08:05

jhon3242 self-assigned this Jan 2, 2025

feat: 통일성을 위해 롬복 추가 #444

09fbd57

leegwichan reviewed Jan 3, 2025

View reviewed changes

GIVEN53 approved these changes Jan 5, 2025

View reviewed changes

feat: SpringBootTest -> WebMvcTest 변경 #444

f5a5c62

jhon3242 commented Jan 17, 2025

View reviewed changes

leegwichan approved these changes Jan 17, 2025

View reviewed changes

jhon3242 merged commit e5e779e into develop Jan 20, 2025
1 check passed

jhon3242 deleted the refactor/#444 branch January 20, 2025 02:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jhon3242 commented Jan 2, 2025 •

edited

Loading

leegwichan left a comment

leegwichan Jan 3, 2025

jhon3242 Jan 17, 2025

leegwichan Jan 3, 2025

jhon3242 Jan 17, 2025

leegwichan Jan 17, 2025

leegwichan Jan 3, 2025

leegwichan Jan 3, 2025

leegwichan Jan 3, 2025

leegwichan Jan 3, 2025

jhon3242 Jan 17, 2025

GIVEN53 left a comment

GIVEN53 Jan 5, 2025

jhon3242 Jan 17, 2025

jhon3242 Jan 17, 2025

leegwichan Jan 17, 2025

jhon3242 left a comment •

edited

Loading

jhon3242 Jan 17, 2025

jhon3242 Jan 17, 2025

jhon3242 Jan 17, 2025

jhon3242 Jan 17, 2025

jhon3242 Jan 17, 2025

jhon3242 Jan 17, 2025

leegwichan left a comment

leegwichan Jan 17, 2025

		@@ -2,6 +2,7 @@

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

		import ddangkong.aop.cookie.RoomMemberCookieEncryptor;

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

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

Conversation

jhon3242 commented Jan 2, 2025 • edited Loading

Issue Number

As-Is

To-Be

Check List

Test Screenshot

(Optional) Additional Description

leegwichan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GIVEN53 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhon3242 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leegwichan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhon3242 commented Jan 2, 2025 •

edited

Loading

jhon3242 left a comment •

edited

Loading