[Team02] 깃허브 로그인 구현, 라벨 수정/이슈 필터링 카운트 기능 수정 #91
Conversation
- Access token을 요청하여 사용자 정보를 가져온다.
- 이미지 URL을 byte 배열로 변환한 후 변환된 배열로 MultipartFile 객체를 생성한다.
- 회원가입을 하기위해 필요한 MemberCreateDto로 매핑한다.
- 처음 사용자가 깃허브 로그인을 하는 것이라면 회원가입 로직을 수행한다. - 사용자에게 유저 정보와 JWT 토큰을 전달한다.
- 프로필 이미지를 MultipartFile로 변환하여 메타데이터와 함께 저장한다.
- 라벨 개수와 마일스톤 개수를 구하여 반환한다.
- 필터링된 이슈의 개수를 구하는 것이 아니라 전체 이슈의 개수를 구하고 있어 수정하였다.
- Entity의 @Setter를 삭제하면서 구조가 변경됨에 따라 라벨이 수정되는 것이 아니라 생성되는 오류가 발생하여 수정용 Mapper를 추가하였다.
zzawang
changed the title
주신 질문들이 전반적으로 정답을 찾고 계신 느낌이었는데, 정해진 답이 있는 영역이 아니기 때문에 모호한 답변을 드리게된 점 양해 부탁드립니다. 매 PR때마다 말씀드리는 점이지만, 실무에서 하는 방식이 항상 정답이 아니기도 하고 처해진 상황마다 정답이 다를 수 있다고 생각합니다. 그리고 구체적인 방법론이나 코드/데이터 구조를 궁금해하시는 것 같았습니다. 자세하게 말씀드리는 것도 회사 내부의 코드나 데이터 구조를 알려드리는 느낌이라 너무 디테일하게 말씀드리기는 조심스러운 점 또한 양해 부탁드립니다 |
| * byte 배열을 MultipartFile 객체로 변환하기 위한 클래스 | ||
| */ | ||
| public class CustomMultipartFile implements MultipartFile { | ||
| private final Tika tika = new Tika(); |
There was a problem hiding this comment.
제가 Tika를 잘 모르긴하지만, 유틸성 느낌이 강하다고 한다면 static하게 설정해주는 것도 좋을 것 같습니다!
인스턴스 별로 (1) 다르게 가지고 있어야하는 필드와 (2) 모든 인스턴스들이 공유해도 괜찮은 필드를 구분해서 생각해주시면 좋습니다😃
| @Getter | ||
| @ToString | ||
| @Builder | ||
| @EqualsAndHashCode |
| long openedCount = filteredIssues.stream().filter(issue -> issue.getIsClosed().equals(false)).toList().size(); | ||
| long closedCount = filteredIssues.stream().filter(issue -> issue.getIsClosed().equals(true)).toList().size(); |
There was a problem hiding this comment.
로직이 너무 길어지고, 가독성을 챙기고 싶다면
데이터와 그 데이터와 관련된 로직이 응집될 수 있도록, 객체 내부로 로직을 안으로 숨기는 것도 방법일 것 같습니다!
issue.getIsClosed().equals(false)
-> issue.isNotClosed()
issue.getIsClosed().equals(true)
-> issue.isClosed()
| @@ -16,7 +16,6 @@ public class Label { | |||
| private final Long id; | |||
| private final String name; | |||
| private final String description; | |||
| @Column("text_color") | |||
| private final String textColor; | |||
| @Column("background_color") | |||
| private final String bgColor; | |||
There was a problem hiding this comment.
개인차가 있겠지만, 저는 길더라도 이름을 보고서 어떤 것을 의미하는지가 명확하게 드러나는 것을 선호하는 편입니다!
추후 1년뒤, 2년뒤 처음 코드를 접하는 사람이 볼 때도 최대한 알아볼 수 있도록 네이밍을 하는 것의 장점도 생각해주시면 좋을 것 같아요😃
| @@ -53,7 +55,7 @@ public Label createLabel(LabelDto labelDto) { | |||
| public Label modifyLabel(LabelDto labelDto, Long id) { | |||
| validateBgColor(labelDto); | |||
|
|
|||
| Label label = toLabel(labelDto); | |||
| Label label = LabelMapper.toModifiedLabel(id, labelDto); | |||
| private final GithubLoginService githubLoginService; | ||
| private final MemberService memberService; | ||
| private final LoginService loginService; | ||
| private final FileService fileService; |
There was a problem hiding this comment.
GithubLoginService과 LoginService 서비스명에서 기대하는 클래스의 역할과 실제 역할이 다른 느낌을 받았습니다. 네이밍을 좀 더 명확하게 해주시면 좋을 것 같아요 (처음 이름만 보고서는 일반 로그인 서비스와 깃헙 로그인 서비스를 구분하기 위해서라고 생각했어요!)
그리고 두 서비스 로직들이 컨트롤러에 있어야할지, 서비스에 있어야할지도 함께 고민해볼만한 요소라 생각합니다
추가로 (1) OAuth를 여러 개 사용했을 때 혹은 (2) OAuth 구현을 깃헙이 아닌 다른 것으로 교체되었을 때의 상황도 고려해보시면 좋습니다!
| @Builder | ||
| @ToString | ||
| @Getter | ||
| public class AccessTokenRequest { |
There was a problem hiding this comment.
Request만 스네이크 케이스군요!
JsonProperty 혹은 JsonNaming으로 어플리케이션 내부에서 사용하는 네이밍 전략의 일관성을 맞춰주는 방법도 있습니다👍
| OAuthInfo oAuthInfo = webClient.post() | ||
| .uri("https://github.com/login/oauth/access_token") | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .bodyValue(getAccessTokenRequest(code)) | ||
| .accept(MediaType.APPLICATION_JSON) | ||
| .retrieve() | ||
| .bodyToMono(OAuthInfo.class) | ||
| .block(); // 비동기 결과를 동기적으로 대기 |
There was a problem hiding this comment.
webClient가 기본적으로 비동기 방식으로 동작해서 동기 코드로 변경해주신거군요
4주동안 감사했습니다 우디 😊 👍🏻
배포된 저희의 사이트입니다☺️ 👉🏻 이슈트래커
✅ 구현 내용
(저번주 PR에 JWT, Redis를 적용했는데 PR 메시지에는 적지 못하여 혹시 못보셨을까봐 다시 적어봅니다..!)
🤔 고민과 🙋🏻 질문