AuthenticationPrinciple 어노테이션에 required 속성 추가 #747

zangsu · 2024-10-06T16:28:22Z

⚡️ 관련 이슈

close #745

📍주요 변경 사항

AuthenticationPrinciple 어노테이션에 required 속성 추가
AuthArgumentResolver 로직 변경 및 테스트 추가

🎸기타

기존의 /login 을 사용하는 분기는 @Deprecated 처리해 두었습니다.
- 아직 프론트에 작업 요청을 하지 않았기 때문에 그 동안의 하위 호환성을 제공하기 위함입니다.
기존의 XXXWithMember 를 사용하는 서비스 메서드 이름을 오버로딩으로 변경하였습니다.
- 파라미터로 Member loginMember 라고만 적어 두어도 간결한 메서드 이름으로 더 명확하게 동작을 나타낼 수 있을 것으로 판단하였습니다.
로직 처리를 Controller 에서 분기하였습니다.
- 파라미터가 null 로 들어오는 것을 예상할 수 있는 가장 빠른 곳에서 분기를 해 주는 것이 인지 부조화가 가장 적다고 판단하였습니다.

jminkkk

고생했어요 짱수~!! 🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥
리뷰 남겼으니 확인 부탁드려욥

backend/src/main/java/codezap/auth/manager/CookieCredentialManager.java

backend/src/test/java/codezap/auth/configuration/AuthArgumentResolverTest.java

backend/src/test/java/codezap/auth/provider/PlainCredentialProvider.java

backend/src/test/java/codezap/auth/configuration/AuthArgumentResolverTest.java

kyum-q

짱수 수고하셨어요 !!!! 짱 ~! 몇가지 의견 남겨뒀어요 ~ 확인 부탁드려요 ~~

backend/src/main/java/codezap/auth/manager/CookieCredentialManager.java

backend/src/main/java/codezap/template/controller/TemplateController.java

backend/src/test/java/codezap/auth/configuration/AuthArgumentResolverTest.java

zeus6768

required 옵션을 사용해서 문제를 해결하려고는 아무도 생각하지 못했는데 창의적이고 멋지네요 👍👍👍

코드잽에서 처음으로 하위 호완성을 고려한 API 설계가 등장했네요.

@Deprecated 도입한 짱수 멋져요~~~!

backend/src/main/java/codezap/auth/configuration/AuthArgumentResolver.java

backend/src/main/java/codezap/auth/manager/CookieCredentialManager.java

backend/src/main/java/codezap/auth/manager/CredentialManager.java

zeus6768 · 2024-10-08T23:49:17Z

backend/src/main/java/codezap/template/controller/TemplateController.java

+        if (member == null) {
+            return ResponseEntity.ok(
+                    templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable)
+            );
+        }
+        return ResponseEntity.ok(
+                templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable, member)
+        );


여기선 삼항연산자를 사용하는게 더 보기 좋을 것 같은데, 짱수 생각은 다를 수 있으니 자유롭게 반영해주세요.

삼항연산자는 컨벤션에 반하는 문법이지만 컨벤션이라는 것도 코드를 더 잘 관리하기 위해 존재하는 것이라, 이렇게 코드가 긴 경우에는 예외적으로 사용해도 좋을 것 같아 제안해요~

TO-BE

Suggested change

if (member == null) {

return ResponseEntity.ok(

templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable)

);

}

return ResponseEntity.ok(

templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable, member)

);

FindAllTemplatesResponse response = (member == null)

? templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable)

: templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable, member);

return ResponseEntity.ok(response);

오! 저는 꽤나 만족스러운 결과로 보이는데요!!!
다만 이 부분은 컨벤션과 관련된 부분이라서 다른 크루들의 의견도 묻고 싶어요. (괜히 수정했다가 롤백 하게 될까봐... 😅 )
@jminkkk @kyum-q @HoeSeong123 다들 어떻게 생각하시나용??

저는 좋다고 생각해요. 삼항연산자를 안 쓰는 이유는 코드의 가독성이 떨어지기 때문이라고 생각하는데, 이 경우는 오히려 올라가지 않았나...라고 생각합니닷

저도 동의합니답~~

모두 동의한다면 반박할 이유가 없어용~!~~! 굿 쟞~~!

zeus6768 · 2024-10-09T00:18:24Z

backend/src/main/java/codezap/template/controller/TemplateController.java

+        if (member == null) {
+            return ResponseEntity.ok(templateApplicationService.findById(id));
+        }
+        return ResponseEntity.ok(templateApplicationService.findById(id, member));


여기도 위와 같은 내용입니다~

Suggested change

if (member == null) {

return ResponseEntity.ok(templateApplicationService.findById(id));

}

return ResponseEntity.ok(templateApplicationService.findById(id, member));

FindTemplateResponse response = (member == null)

? templateApplicationService.findById(id)

: templateApplicationService.findById(id, member);

return ResponseEntity.ok(response);

backend/src/main/java/codezap/template/controller/TemplateController.java

backend/src/test/java/codezap/auth/provider/PlainCredentialProvider.java

…ecoder 사용

HoeSeong123

열심히 읽어봤는데 이미 폭풍 리뷰가 많이 달려서 제가 추가할만한 부분이 거의 없는 것 같아요.
삼항연산자 관련해서는 정말 많이 고민해봤는데 아무리 생각해도 저게 최선일 것 같네요....

고생하셨습니다 짱수!!

HoeSeong123 · 2024-10-10T03:43:18Z

backend/src/main/java/codezap/template/controller/TemplateController.java

+        if (member == null) {
+            return ResponseEntity.ok(
+                    templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable)
+            );
+        }
+        return ResponseEntity.ok(
+                templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable, member)
+        );


저는 좋다고 생각해요. 삼항연산자를 안 쓰는 이유는 코드의 가독성이 떨어지기 때문이라고 생각하는데, 이 경우는 오히려 올라가지 않았나...라고 생각합니닷

…ired

jminkkk

required 아이디어 너무 좋았습니다 짱수 굳~
수고했어요

jminkkk · 2024-10-10T08:19:46Z

backend/src/main/java/codezap/template/controller/TemplateController.java

+        if (member == null) {
+            return ResponseEntity.ok(
+                    templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable)
+            );
+        }
+        return ResponseEntity.ok(
+                templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable, member)
+        );


저도 동의합니답~~

zeus6768

대 짱 수

kyum-q

귯쟙

zangsu added 6 commits October 6, 2024 22:52

refactor: required 속성 추가

51b6db5

refactor: ArgumentResolver 에서 null 반환하도록 변경

8312373

test: AuthArgumentResolver 기능을 테스트

82d27a3

refactor: 파라미터 이름을 로그인 한 멤버를 나타내도록 변경

da864d9

refactor: 메서드 이름을 오버로딩으로 변경

c566ce6

refactor: 로그인 분기 메서드를 Deprecated 처리

55ca8c1

zangsu added refactor 요구사항이 바뀌지 않은 변경사항 BE 백엔드 labels Oct 6, 2024

zangsu added this to the 6차 스프린트 🦴 milestone Oct 6, 2024

zangsu requested review from zeus6768, jminkkk, kyum-q and HoeSeong123 October 6, 2024 16:28

zangsu self-assigned this Oct 6, 2024

jminkkk requested changes Oct 7, 2024

View reviewed changes

zangsu added 6 commits October 7, 2024 19:05

docs: DisplayName 컨벤션 적용

a250725

refactor: 불필요한 지역변수 추출 제거

ce98d64

style: 줄바꿈 추가

984ff46

refactor: 불필요한 지역변수 추출 제거

41dae36

refactor: 접근제한자 추가

f7fee28

refactor: Optional.get() 개선

02d083a

kyum-q requested changes Oct 8, 2024

View reviewed changes

zangsu added 2 commits October 8, 2024 20:04

docs: 사용하지 않는 given 삭제

287691c

docs: DisplayName 변경

3b12062

zeus6768 requested changes Oct 9, 2024

View reviewed changes

zangsu added 4 commits October 9, 2024 17:18

refactor: 파라미터의 final 키워드 제거

035a5e5

refactor: RFC2616 에 따라 쿠키에 들어갈 수 있는 ASCII 값으로 변형을 위해 URLEncoder, URLD…

225430c

…ecoder 사용

refactor: 쿠키 값 세팅 방식을 변경

c112462

style: 포매팅 적용

506f9dd

HoeSeong123 previously approved these changes Oct 10, 2024

View reviewed changes

Merge branch 'dev/be' into refactor/745-authentication-principle-requ…

b49e6d4

…ired

zangsu dismissed HoeSeong123’s stale review via b49e6d4 October 10, 2024 08:19

jminkkk approved these changes Oct 10, 2024

View reviewed changes

zeus6768 approved these changes Oct 10, 2024

View reviewed changes

HoeSeong123 approved these changes Oct 10, 2024

View reviewed changes

kyum-q approved these changes Oct 10, 2024

View reviewed changes

kyum-q merged commit ab1f658 into dev/be Oct 10, 2024
6 checks passed

kyum-q deleted the refactor/745-authentication-principle-required branch October 10, 2024 11:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AuthenticationPrinciple 어노테이션에 required 속성 추가 #747

AuthenticationPrinciple 어노테이션에 required 속성 추가 #747

zangsu commented Oct 6, 2024

jminkkk left a comment

kyum-q left a comment

zeus6768 left a comment

zeus6768 Oct 8, 2024 •

edited

Loading

zangsu Oct 9, 2024 •

edited

Loading

HoeSeong123 Oct 10, 2024

jminkkk Oct 10, 2024

kyum-q Oct 10, 2024

zeus6768 Oct 9, 2024

HoeSeong123 left a comment

HoeSeong123 Oct 10, 2024

jminkkk left a comment

jminkkk Oct 10, 2024

zeus6768 left a comment

kyum-q left a comment

-        if (member == null) {
-            return ResponseEntity.ok(templateApplicationService.findById(id));
-        }
-        return ResponseEntity.ok(templateApplicationService.findById(id, member));
+        FindTemplateResponse response = (member == null)
+                ? templateApplicationService.findById(id)
+                : templateApplicationService.findById(id, member);
+        return ResponseEntity.ok(response);

AuthenticationPrinciple 어노테이션에 required 속성 추가 #747

AuthenticationPrinciple 어노테이션에 required 속성 추가 #747

Conversation

zangsu commented Oct 6, 2024

⚡️ 관련 이슈

📍주요 변경 사항

🎸기타

jminkkk left a comment

Choose a reason for hiding this comment

kyum-q left a comment

Choose a reason for hiding this comment

zeus6768 left a comment

Choose a reason for hiding this comment

zeus6768 Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

TO-BE

zangsu Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

HoeSeong123 Oct 10, 2024

Choose a reason for hiding this comment

jminkkk Oct 10, 2024

Choose a reason for hiding this comment

kyum-q Oct 10, 2024

Choose a reason for hiding this comment

zeus6768 Oct 9, 2024

Choose a reason for hiding this comment

HoeSeong123 left a comment

Choose a reason for hiding this comment

HoeSeong123 Oct 10, 2024

Choose a reason for hiding this comment

jminkkk left a comment

Choose a reason for hiding this comment

jminkkk Oct 10, 2024

Choose a reason for hiding this comment

zeus6768 left a comment

Choose a reason for hiding this comment

kyum-q left a comment

Choose a reason for hiding this comment

zeus6768 Oct 8, 2024 •

edited

Loading

zangsu Oct 9, 2024 •

edited

Loading