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

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

Merged
merged 19 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ public Member resolveArgument(
NativeWebRequest webRequest,
WebDataBinderFactory binderFactory
) {
AuthenticationPrinciple parameterAnnotation = parameter.getParameterAnnotation(AuthenticationPrinciple.class);
HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest();
if(!parameterAnnotation.required() && !credentialManager.hasCredential(request)) {
zeus6768 marked this conversation as resolved.
Show resolved Hide resolved
return null;
}
String credential = credentialManager.getCredential(request);
return credentialProvider.extractMember(credential);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@
@Target(ElementType.PARAMETER)
@Retention(RetentionPolicy.RUNTIME)
public @interface AuthenticationPrinciple {
boolean required() default true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
@Component
public class CookieCredentialManager implements CredentialManager {

private static final String CREDENTIAL_COOKIE_NAME = "credential";
public static final String CREDENTIAL_COOKIE_NAME = "credential";
zangsu marked this conversation as resolved.
Show resolved Hide resolved

@Override
public String getCredential(final HttpServletRequest httpServletRequest) {
Expand All @@ -33,6 +33,16 @@ private void checkCookieExist(final Cookie[] cookies) {
}
}

@Override
public boolean hasCredential(final HttpServletRequest httpServletRequest) {
Cookie[] cookies = httpServletRequest.getCookies();
if (cookies == null) {
return false;
}
return Arrays.stream(cookies)
.anyMatch(cookie -> cookie.getName().equals(CREDENTIAL_COOKIE_NAME));
}

private Cookie extractTokenCookie(final Cookie[] cookies) {
return Arrays.stream(cookies)
.filter(cookie -> cookie.getName().equals(CREDENTIAL_COOKIE_NAME))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ public interface CredentialManager {

String getCredential(HttpServletRequest httpServletRequest);

boolean hasCredential(final HttpServletRequest httpServletRequest);
zeus6768 marked this conversation as resolved.
Show resolved Hide resolved

void setCredential(HttpServletResponse httpServletResponse, String credential);

void removeCredential(HttpServletResponse httpServletResponse);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,47 +55,8 @@ public interface SpringDocTemplateController {
})
ResponseEntity<Void> createTemplate(Member member, CreateTemplateRequest createTemplateRequest);

@Operation(summary = "템플릿 조회 (비회원)", description = """
조건에 맞는 모든 템플릿을 조회합니다. \n
- 조건 \n
- 멤버 ID
- 검색 키워드 (템플릿명, 템플릿 설명, 파일명, 소스 코드)
- 카테고리 ID
- 태그 ID들 \n

페이징 조건을 줄 수 있습니다. 페이지 번호는 1, 템플릿 개수는 20, 정렬 방식은 최신순이 기본 값입니다. \n
- 페이징 조건 \n
- 페이지 번호(pageNumber)
- 한 페이지에 템플릿 개수(pageSize)
- 페이지 정렬 방식(sort) \n

- 정렬 방식 \n
- 최신순 (modifiedAt,asc)
- 오래된순 (modifiedAt,desc)
- 좋아요 순 (likesCount, desc) \n
""")
@ApiResponse(responseCode = "200", description = "템플릿 검색 성공")
@ApiErrorResponse(status = HttpStatus.BAD_REQUEST,
instance = "/templates?memberId=1&keyword=\"java\"&tagIds=", errorCases = {
@ErrorCase(description = "태그 ID가 0개인 경우", exampleMessage = "태그 ID가 0개입니다. 필터링 하지 않을 경우 null로 전달해주세요."),
@ErrorCase(description = "페이지 번호가 1보다 작을 경우", exampleMessage = "페이지 번호는 1 이상이어야 합니다."),
})
@ApiErrorResponse(status = HttpStatus.NOT_FOUND,
instance = "/templates?memberId=1&keyword=\"java\"&categoryId=1&tagIds=1,2", errorCases = {
@ErrorCase(description = "멤버가 없는 경우", exampleMessage = "식별자 1에 해당하는 멤버가 존재하지 않습니다."),
@ErrorCase(description = "카테고리가 없는 경우", exampleMessage = "식별자 1에 해당하는 카테고리가 존재하지 않습니다."),
@ErrorCase(description = "태그가 없는 경우", exampleMessage = "식별자 1에 해당하는 태그가 존재하지 않습니다."),
})
ResponseEntity<FindAllTemplatesResponse> findAllTemplates(
Long memberId,
String keyword,
Long categoryId,
List<Long> tagIds,
Pageable pageable
);

@SecurityRequirement(name = "쿠키 인증 토큰")
@Operation(summary = "템플릿 조회 (회원)", description = """
@Operation(summary = "템플릿 조회", description = """
조건에 맞는 모든 템플릿을 조회합니다. \n
- 조건 \n
- 멤버 ID
Expand Down Expand Up @@ -126,7 +87,7 @@ ResponseEntity<FindAllTemplatesResponse> findAllTemplates(
@ErrorCase(description = "카테고리가 없는 경우", exampleMessage = "식별자 1에 해당하는 카테고리가 존재하지 않습니다."),
@ErrorCase(description = "태그가 없는 경우", exampleMessage = "식별자 1에 해당하는 태그가 존재하지 않습니다."),
})
ResponseEntity<FindAllTemplatesResponse> getTemplatesWithMember(
ResponseEntity<FindAllTemplatesResponse> findAllTemplates(
Member member,
Long memberId,
String keyword,
Expand All @@ -135,20 +96,13 @@ ResponseEntity<FindAllTemplatesResponse> getTemplatesWithMember(
Pageable pageable
);

@Operation(summary = "템플릿 단건 조회 (비회원)", description = "해당하는 식별자의 템플릿을 조회합니다.")
@ApiResponse(responseCode = "200", description = "템플릿 단건 조회 성공")
@ApiErrorResponse(status = HttpStatus.BAD_REQUEST, instance = "/templates/1", errorCases = {
@ErrorCase(description = "해당하는 ID 값인 템플릿이 없는 경우", exampleMessage = "식별자 1에 해당하는 템플릿이 존재하지 않습니다."),
})
ResponseEntity<FindTemplateResponse> findTemplateById(Long id);

@SecurityRequirement(name = "쿠키 인증 토큰 (회원)")
@SecurityRequirement(name = "쿠키 인증 토큰")
@Operation(summary = "템플릿 단건 조회", description = "해당하는 식별자의 템플릿을 조회합니다.")
@ApiResponse(responseCode = "200", description = "템플릿 단건 조회 성공")
@ApiErrorResponse(status = HttpStatus.BAD_REQUEST, instance = "/templates/1/login", errorCases = {
@ErrorCase(description = "해당하는 ID 값인 템플릿이 없는 경우", exampleMessage = "식별자 1에 해당하는 템플릿이 존재하지 않습니다."),
})
ResponseEntity<FindTemplateResponse> getTemplateByIdWithMember(Member member, Long id);
ResponseEntity<FindTemplateResponse> findTemplateById(Member member, Long id);

@SecurityRequirement(name = "쿠키 인증 토큰")
@Operation(summary = "템플릿 수정", description = "해당하는 식별자의 템플릿을 수정합니다.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,50 +44,54 @@ public ResponseEntity<Void> createTemplate(

@GetMapping
public ResponseEntity<FindAllTemplatesResponse> findAllTemplates(
@AuthenticationPrinciple(required = false) Member member,
@RequestParam(required = false) Long memberId,
@RequestParam(required = false) String keyword,
@RequestParam(required = false) Long categoryId,
@RequestParam(required = false) List<Long> tagIds,
@PageableDefault(size = 20) Pageable pageable
) {
FindAllTemplatesResponse response = templateApplicationService.findAllBy(
memberId,
keyword,
categoryId,
tagIds,
pageable);
return ResponseEntity.ok(response);
if (member == null) {
return ResponseEntity.ok(
templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable)
);
}
return ResponseEntity.ok(
templateApplicationService.findAllBy(memberId, keyword, categoryId, tagIds, pageable, member)
);
kyum-q marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +54 to +61
Copy link
Contributor

@zeus6768 zeus6768 Oct 8, 2024

Choose a reason for hiding this comment

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

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

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

TO-BE

스크린샷 2024-10-09 오전 8 47 15
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);

Copy link
Contributor Author

@zangsu zangsu Oct 9, 2024

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

저도 동의합니답~~

Copy link
Contributor

Choose a reason for hiding this comment

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

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

}

@Deprecated
@GetMapping("/login")
public ResponseEntity<FindAllTemplatesResponse> getTemplatesWithMember(
public ResponseEntity<FindAllTemplatesResponse> findAllTemplatesWithMember(
@AuthenticationPrinciple Member member,
@RequestParam(required = false) Long memberId,
@RequestParam(required = false) String keyword,
@RequestParam(required = false) Long categoryId,
@RequestParam(required = false) List<Long> tagIds,
@PageableDefault(size = 20) Pageable pageable
) {
return ResponseEntity.ok(templateApplicationService.findAllByWithMember(
memberId,
keyword,
categoryId,
tagIds,
pageable,
member));
return findAllTemplates(member, memberId, keyword, categoryId, tagIds, pageable);
}

@GetMapping("/{id}")
public ResponseEntity<FindTemplateResponse> findTemplateById(@PathVariable Long id) {
return ResponseEntity.ok(templateApplicationService.findById(id));
public ResponseEntity<FindTemplateResponse> findTemplateById(
@AuthenticationPrinciple(required = false) Member member,
@PathVariable Long id
) {
if (member == null) {
return ResponseEntity.ok(templateApplicationService.findById(id));
}
return ResponseEntity.ok(templateApplicationService.findById(id, member));
Comment on lines +82 to +85
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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

}

@Deprecated
zangsu marked this conversation as resolved.
Show resolved Hide resolved
@GetMapping("/{id}/login")
public ResponseEntity<FindTemplateResponse> getTemplateByIdWithMember(
public ResponseEntity<FindTemplateResponse> findTemplateByIdWithMember(
@AuthenticationPrinciple Member member,
@PathVariable Long id
) {
return ResponseEntity.ok(templateApplicationService.findByIdWithMember(id, member));
return findTemplateById(member, id);
}

@PostMapping("/{id}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ public FindTemplateResponse findById(Long id) {
return FindTemplateResponse.of(template, sourceCodes, tags, false);
}

public FindTemplateResponse findByIdWithMember(Long id, Member member) {
public FindTemplateResponse findById(Long id, Member loginMember) {
Template template = templateService.getById(id);
List<Tag> tags = tagService.findAllByTemplate(template);
List<SourceCode> sourceCodes = sourceCodeService.findAllByTemplate(template);
boolean isLiked = likesService.isLiked(member, template);
boolean isLiked = likesService.isLiked(loginMember, template);
return FindTemplateResponse.of(template, sourceCodes, tags, isLiked);
}

Expand All @@ -79,16 +79,16 @@ public FindAllTemplatesResponse findAllBy(
return makeResponse(templates, (template) -> false);
}

public FindAllTemplatesResponse findAllByWithMember(
public FindAllTemplatesResponse findAllBy(
Long memberId,
String keyword,
Long categoryId,
List<Long> tagIds,
Pageable pageable,
Member member
Member loginMember
) {
Page<Template> templates = templateService.findAllBy(memberId, keyword, categoryId, tagIds, pageable);
return makeResponse(templates, (template -> likesService.isLiked(member, template)));
return makeResponse(templates, (template -> likesService.isLiked(loginMember, template)));
}

private FindAllTemplatesResponse makeResponse(Page<Template> page, LikedChecker likedChecker) {
Expand Down
Loading