[CHORE] 코드 리뷰용 PR #246
Open
[CHORE] 코드 리뷰용 PR #246
Conversation
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
- OAuth2.0을 이용한 소셜로그인에 필요한 설정 추가 (filterChain을 통해 구현) - 실제 객체가 생성되어있지 않아 작성할 수 없는 부분은 주석 처리 (추후 주석 해제 필요) - application-oauth.yml 파일에 소셜로그인에 대한 설정 후, 테스트 시 정상작동 확인
- 정적 팩토리 메서드를 통해 provider에 맞는 AuthProvider를 반환
- 생성시간, 수정시간, 삭제시간 필드를 가지고 있는 공통 Entity 추가 - createdDate, modifiedDate 자동 기록을 위해 @EnableJpaAuditing 어노테이션 추가
- JPA를 활용하여 User 엔티티 클래스 작성 - BaseTimeEntity 상속 - 사용자의 회원가입 여부/ 권한에 대한 정보를 담고 있는 Enum class 작성
- JpaRepository 인터페이스를 상속하여 구현 - 이메일 / 이메일+provider 를 통해 DB에서 사용자를 찾는 기능 구현 - 두 메서드에 대해 단위 테스트 코드 작성
Feat: 소셜로그인에 필요한 User Entity 및 Repository 개발
Feat: Spring Security, OAuth 2.0에서 필요한 설정 진행
- BusinessException: 비지니스 로직에서 발생한 예외를 처리할 때 해당 예외 클래스를 throw - BusinessExceptionHandler : 사용자가 처리한 예외(BusinessException)을 어떻게 처리할 것인지 구현 - GlobalExceptionHandler: 개발자가 처리하지 않은 예외가 던져졌을 때 처리되는 부분 -> 추가 처리 필요함을 날리는 곳 - ErrorCode: 예외 메시지를 저장하는 Enum 클래스
- 반환하는 값의 형식에 따른 Response DTO 타입 지정
- 서드파티로부터 access-token 받은 이후 실행되는 custom 서비스 클래스 로직 구현 - 사용자 정보(email)을 받아온 후 -> DB에 저장 여부 확인 -> 없다면 DB에 저장
- 소셜로그인을 통한 인증 성공 후, 실행되는 OAuth2SuccessHandler 클래스 로직 구현 - 소셜로그인 도중 모종의 이유로 인해 실패했을 경우, 실행되는 OAuth2FailureHandler 클래스 로직 구현
- 매직 넘버(magic number)에 대해 상수로 변경 - 인증 객체에 필요한 메서드 추가
- 기존에 OAuth2User 인터페이스 상속에서 DefaultOAuth2User 상속으로 변경 - getName() 메서드 오버라이드: 사용자의 이메일을 반환하도록 설정
- 기존에 무조건 ROLE_USER를 전달하던 로직에서, 생성자에 사용자의 역할을 전달하도록 수정
Feat: 소셜로그인에 필요한 DTO 개발, 클래스 커스터마이징 및 로직 구현
* fix: 실패한 챌린지에 대해 달성율이 나오지 않는 버그 픽스 - 끝까지 참여는 했지만, 달성율이 목표치에 도달하지 않아 실패한 챌린지에 대해 달성율이 나오지 않는 버그 픽스 - 응답 DTO에 해당 데이터 추가 * test: 버그가 발생한 상황에 대한 테스트 코드 추가
* fix: 아이템 구매가 안되는 버그 픽스 - LazyInitialization으로 인해 아이템 구매가 안되는 버그 픽스 - Transactional 어노테이션 적용 및 영속화된 엔티티 전달을 통해 버그 픿흐 * refactor: DTO에 정적 팩토리 메서드 적용 - PreActivityResponse에 정적 팩토리 메서드 적용 - ActivityResponse 정적 팩토리 메서드 네이밍 of로 변경 * refactor: MyChallenge 클래스에 Facade 패턴 적용 - MyChallengeService를 Facade 클래스로 변경 - Facade 인터페이스 선언 및 구현 * feat: 트랜잭션 어노테이션 추가 - OrderService에 트랜잭션 어노테이션 추가 * feat: myChallengeFacade 적용 및 코드 리팩토링 - Participant에 보상 컨디션 확인하는 메서드 추가 - Certification에 더미 데이터 생성하는 정적 팩토리 메서드 추가 - StoreFacade, MyChallengeFacade 코드 리팩토링 * refactor: Certification 생성 코드 리팩토링 - Certification이 없을 때 더미 데이터를 생성하는 코드를 CertificationProvider로 변경 * fix: 인증 조건을 검증하는 코드 누락 수정 * refactor: 보상 받는 로직 통일을 위해 리팩토링 - ParticipantProvider에 보상 받는 메서드를 추가하고, 이를 호출하도록 리팩토링 (로직의 통일성을 위해) * refactor: 클래스명 변경 - ParticipantProvider에서 ProviderService로 클래스명 변경 * test: 테스트 코드에 DCI 패턴 적용 * chore: 필요 없는 부분 삭제
…적용 (#239) * refactor: CertificationController에 파사드 패턴 적용 - 파사드 패턴 적용에 따라 인터페이스 추가 및 구현체 적용 - 이름 변경 * refactor: GithubController에 파사드 패턴 적용 - 파사드 패턴 적용에 따른 인터페이스 및 구현체 적용 - 이름 변경 * refactor: GithubProvider에서 GithubService로 이름 변경 * refactor: 파사드 패턴에 따라 의존성 변경 - InstanceProvider 의존에서 InstanceService 의존으로 변경 * feat: Certification & Github에 파사드 패턴 적용 - Facade에서 Service만 의존하도록 리팩토링 * test: 테스트코드에 DCI 패턴 적용 중 * test: DCI 패턴 적용 중 * test: CertificationFacade 테스트 코드에 DCI 패턴 적용 * refactor: CertificationFacade 코드 리팩토링 - 깃허브와 관련된 부분은 GithubService로 이동 - 도메인에 해당하는 비지니스 로직 이동 * test: GithubFacade 테스트 코드에 DCI 패턴 적용
- ProfileService 제거 - ProfileFacade & ProfileFacadeService 구현 - 테스트 코드 DCI 패턴 도입
- InstanceDetailFacade & InstanceDetailFacadeService 생성 - InstanceDetailService 기능 상실 -> 제거 - 테스트 코드 DCI 도입
- 파사드 패턴 적용에 따라 FileService와 FileManager의 이름을 서로 변경
VSFe
suggested changes
Aug 15, 2024
| InstancePreviewResponse instancePreviewResponse = certificationFacade.getInstancePreview(instanceId); | ||
|
|
||
| return ResponseEntity.ok().body( | ||
| new SingleResponse<>(SUCCESS.getStatus(), SUCCESS.getMessage(), instancePreviewResponse) |
There was a problem hiding this comment.
API 요청 성공에 굳이 메시지 래핑을 해야하나 싶은데요...
특히나 성공 메시지는 절대 참고하지 않는지라, 더더욱 의미가 없다 싶습니다.
|
|
||
| @PostMapping("/today") | ||
| public ResponseEntity<SingleResponse<CertificationResponse>> certificateByGithub( | ||
| @AuthenticationPrincipal UserPrincipal userPrincipal, |
There was a problem hiding this comment.
UserPrinciple 에서 user만 꺼낼거면, 별도의 어노테이션을 만들어서 이걸 Resolve 하는 방식이 낫겠네요.
import com.genius.gitget.challenge.certification.dto.CertificationRequest;
import com.genius.gitget.challenge.certification.dto.CertificationResponse;
import com.genius.gitget.global.util.response.dto.SingleResponse;
import org.springframework.http.ResponseEntity;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
@PostMapping("/today")
public ResponseEntity<SingleResponse<CertificationResponse>> certificateByGithub(
@GitGetUser User user,
@RequestBody CertificationRequest certificationRequest
) {
}| public ResponseEntity<SlicingResponse<WeekResponse>> getAllUserWeekCertification( | ||
| @AuthenticationPrincipal UserPrincipal userPrincipal, | ||
| @PathVariable Long instanceId, | ||
| @PageableDefault Pageable pageable |
| @RequiredArgsConstructor | ||
| public class ListResponse<T> extends CommonResponse { | ||
| private List<T> dataList; | ||
| private int count; |
There was a problem hiding this comment.
FE에서 배열의 길이를 세는건 어렵지 않을텐데요,
굳이 저희가 size를 넘겨줄 필요는 없어보입니다.
| import org.springframework.web.bind.annotation.RequestParam; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| @Slf4j |
| } | ||
|
|
||
| // 인스턴스 리스트 조회 | ||
| public Page<Instance> findAllInstances(Pageable pageable) { |
There was a problem hiding this comment.
- Cursor Based Paging, Offset Based Paging 의 차이에 대해 알아보셨으면 좋겠습니다.
- 과연 둘 중 어떤 방식을 사용하면 좋을까요? 특히 데이터가 많을때요.
| * 참가자 수 정보 수정 | ||
| * */ | ||
| public void updateParticipantCount(int amount) { | ||
| if (amount < 0 && this.participantCount + amount < 0) { |
There was a problem hiding this comment.
this.participantCount = Math.max(0, this.participantCount + amount);
| } | ||
|
|
||
| @Override | ||
| public Optional<Files> getFiles() { |
|
|
||
| @Builder | ||
| public record InstanceDetailResponse( | ||
| Long topicId, Long instanceId, |
There was a problem hiding this comment.
https://naver.github.io/hackday-conventions-java/
등의 컨벤션 도입을 적극 검토하셨으면 좋겠습니다.
(Intellij 의 기능들을 활용하면 자동으로 해당 컨벤션에 맞게 코드 포맷을 변경해 줍니다.)
| import lombok.NoArgsConstructor; | ||
|
|
||
| @NoArgsConstructor | ||
| @Data |
- JWT 발급 API에서 token-reissued의 값을 true로 설정
* feat: 커스텀 어노테이션 개발 - AuthenticationPrincipal 의 값을 받을 수 있는 커스텀 어노테이션 개발 * refactor: 커스텀 어노테이션 적용 - @AuthenticationPrincipal 어노테이션이 적용되어 있는 곳을 @gitgetuser 어노테이션으로 변경 * refactor: 커스텀 어노테이션 적용 - @AuthenticationPrincipal 어노테이션 있던 곳에 커스텀 어노테이션(@gitgetuser) 적용
* refactor: User 계층에 Facade pattern 적용 * chore: 클래스 네이밍 변경 * fix: identifier 자료형을 Boxing type으로 변경 - identifier의 자료형을 null을 받을 수 있는 Boxing type으로 변경 * fix: 사용 중인 프레임이 없을 때의 응답값 수정 - 사용 중인 프레임이 없을 때 identifier의 값으로 null을 전달하게끔 수정 * test: UserFacadeTest에 DCI패턴 적용 * test: 테스트 코드 수정
ProfileFacadeService, UserService의 Dirty checking이 필요한 메서드에 대해 @transactional 어노테이션 추가
* refactor: 파일 조회 응답 데이터 변경 - 파일 조회 시 Base64 인코딩 문자열을 보냈던 방식에서, 파일의 URI를 반환하는 방식으로 변경 * fix: local 버전에서 경로가 중복되어 응답하는 버그 픽스 - local 버전에서 이미지 경로가 중복되어 전달되는 버그 픽스 * chore: 인터페이스의 주석 수정
- 트랜잭션 미적용으로 인해 회원가입 처리가 이루어지지 않은 버그 핫픽스
* feat: Certification 테이블에 DB index 적용 * feat: CertificationRepository의 쿼리 수정 * fix: Certification 테이블의 인덱스 수정
* feat: guest 계정 로그인 API 개발 * fix: 게스트 계정 로그인 시, 아이디&비밀번호 확인하는 로직 추가 * feat: 게스트 계정 추가하는 SQL문 작성 - Users 테이블에 게스트 계정 정보가 없을 때, 게스트 계정을 추가하는 SQL문 작성 * test: data.sql 문 수정으로 인해 실패하는 테스트 수정 * feat: 게스트 로그인 시 전달하는 데이터 추가 - 게스트 로그인 응답 데이터에 identifier 정보 추가
* feat: 파일 버전 표시를 위한 클래스 추가 - yml 파일의 값을 읽은 후, 로컬/프로덕션 버전 정보를 저장하는 클래스 작성 * feat: 파일 응답 클래스 구조 변경 - accessURI를 source로 변경 - 파일 버전 정보(environment) 추가 - LOCAL / PROD * fix: 쿼리문 오류 수정 - PK 관련 에러 수정 * refactor: 버전 별 처리 방법 변경 - LOCAL 버전의 경우 파일을 다운로드 받아 BASE64로 인코딩한 문자열을 반환하도록 변경 - PROD 버전의 경우 파일에 접근할 수 있는 URI 반환
* refactor: 쿼리의 성능 개선 * chore: git submodule 갱신 * chore: 워크플로우 갱신 * chore: git submodule 갱신 * chore: 워크플로우 변경
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
코드 리뷰에 사용할 Pull Request입니다.
제일 첫 commit 내역부터 포함되어 있습니다.