Open
Conversation
ghost
added
the
ghost
requested a review
from 
Dae-Hwa
reviewed
Jun 17, 2021
Collaborator
Dae-Hwa
left a comment
Dae-Hwa
left a comment
There was a problem hiding this comment.
어노테이션으로 검증하게 만드셨군요
고생많으셨습니다!
절차적으로 작성 된 부분이 많은데,
한 번에 다 고치기는 힘들수도 있을 것 같아요
힘든 부분은 말씀해주시면 나눠서 고쳐보겠습니다.
be/issue-tracker-be/build.gradle
Outdated
| implementation 'org.springframework.boot:spring-boot-starter-hateoas' | ||
| implementation 'org.springframework.boot:spring-boot-starter-validation' | ||
| implementation 'org.springframework.boot:spring-boot-starter-web' | ||
| ㄹ implementation 'org.projectlombok:lombok:1.18.16' |
Collaborator
There was a problem hiding this comment.
문제 생기지 않았나요? 그리고 굳이 필요 없어 보입니다.
| import static java.lang.annotation.ElementType.METHOD; | ||
| import static java.lang.annotation.RetentionPolicy.RUNTIME; | ||
|
|
||
| // NOTE: Login이 필요한 API일 경우 사용 |
Collaborator
There was a problem hiding this comment.
멋지네요. 어노테이션을 만드신거군요 ㅋㅋ
이런 주석은 java docs로 메모해두는게 더 좋을 것 같습니다.
| //Fixme : front로 변경 | ||
| @GetMapping("/callback") | ||
| public void callback(@RequestParam(value = "code") String code) throws AuthException { | ||
| auth(code); |
Collaborator
There was a problem hiding this comment.
핸들러에서 핸들러를 호출하는 구조는 좀 어색해보이는데 이유가 있나요?
Author
There was a problem hiding this comment.
이 부분이 저도 절대 하면 안되는 걸 알지만... callback쪽은 프론트 단에서 구현해야하는 부분이라 생각해서 저렇게 구현을 했습니다
Comment on lines
35
to
45
| GitHubAccessTokenResponse token = authService.getAccessToken(code); | ||
| String accessToken = token.getAccessToken(); | ||
|
|
||
| UserDto userDto = authService.getUserFromGitHub(accessToken); | ||
|
|
||
| AuthResponse authResponse = new AuthResponse(JwtUtil.createJwt(userDto)); | ||
|
|
||
| ResponseEntity<AuthResponse> authResponseResponseEntity = ResponseEntity.status(HttpStatus.CREATED). | ||
| body(new AuthResponse(JwtUtil.createJwt(userDto))); | ||
|
|
||
| return authResponseResponseEntity; |
Collaborator
There was a problem hiding this comment.
서비스 로직으로 옮겨야 할 것 같습니다.
그리고 더 나아가면 도메인처럼 생각해볼 수도 있겠죠?
Comment on lines
+3
to
+13
| public class AuthRequest { | ||
| private String code; | ||
|
|
||
| public AuthRequest(String code) { | ||
| this.code = code; | ||
| } | ||
|
|
||
| public String getCode() { | ||
| return code; | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
이런 부분은 롬복으로 처리해도 충분하다고 봅니다
| public UserDto getUserFromGitHub(String accessToken) throws AuthException { | ||
| RequestEntity<Void> request = RequestEntity | ||
| .get(gitHubUserUri) | ||
| .header("Accept", "application/json") |
Collaborator
There was a problem hiding this comment.
이런 부분은 중복 제거 시도해볼 수도 있겠네요
Comment on lines
+42
to
+43
| return Optional.ofNullable(response.getBody()) | ||
| .orElseThrow(() -> new AuthException("Access Token 획득 실패")); |
Collaborator
There was a problem hiding this comment.
null 체크를 위한 용도면 Optional을 사용할 필요는 없는 것 같아요
| import lombok.Data; | ||
|
|
||
| @Data | ||
| public class JwtUtil { |
Collaborator
There was a problem hiding this comment.
위에서도 말했지만, 객체로 만들 수 있을 것 같네요
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
|
||
| public class UserDto { |
| } | ||
| } | ||
|
|
||
| public static UserDto decodeJwt(String token) throws JwtException { |
FE, IOS타입에 따라서 clientId, secret의 값을 다르게 리턴
Dae-Hwa
requested changes
Jun 28, 2021
| implementation 'org.springframework.boot:spring-boot-starter-hateoas' | ||
| implementation 'org.springframework.boot:spring-boot-starter-validation' | ||
| implementation 'org.springframework.boot:spring-boot-starter-web' | ||
| implementation 'org.projectlombok:lombok:1.18.16' |
Collaborator
There was a problem hiding this comment.
Suggested change
| implementation 'org.projectlombok:lombok:1.18.16' |
없애주세요
Comment on lines
30
to
31
| annotationProcessor 'org.projectlombok:lombok' | ||
| compileOnly 'org.projectlombok:lombok' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
IOS쪽 App등록과
code를 받아오기, redirectUri 구현을 프론트 서버로 수정 해야 합니다.
token을 통해 GitHub Api로 userName, login, avatar정보를 받아옵니다.