-
Notifications
You must be signed in to change notification settings - Fork 0
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
DailyDefense 컨트롤러 작성 및 시험 타이머 기능 개발 #36
Conversation
* ✨ Google OAuth 적용 및 커스텀 예외 추가 * ✏️ 패키지 구조 일부 변경 * ✏️ 출력 관련 코드 제거 * ✏️ MemberAdapter 코드 수정 * ✏️ OAuth Login과 관련한 Cookie 발급과 관련한 작업을 Service 단에서 처리하도록 수정 * ✏️ OAuth AccessToken 발급과 관련한 메서드 분리 * ✏️ GoogleUserDto에 @Setter, @AllArgsConstructor 제거 * ✏️ SetDomain 관련 url, path를 yml에 저장하여 관리 * ✏️ ErrorCode 관련 코드 수정 및 RestControllerAdvice 코드 수정 * ✏️ OAuth domain 관련 패키지 구조 변경 * ✏️ LoginUseCase를 목적에 맞게 AuthenticationUseCase로 변경 (인증과 관련한 작업) * ✏️ JwtToken 및 publicKey와 PrivateKey를 발급하는 코드 수정 * ✏️ LoginMember에서 Repository를 호출하는 형태 변경 및 패키지 구조 일부 변경 * ✏️ Filter를 통과하는 url 리스트를 정규 표현식으로 검사하도록 수정 * ✏️ Spring Security Filter와 관련한 코드 수정 * ✏️ CORS 설정을 시큐리티 기본값에서 직접 정의한 내용으로 수정 * ✨ RefreshToken 관리를 위한 Redis 환경 구성 * ✨ RefreshToken을 검증하여 accessToken을 재발급하도록 코드 추가 * ✏️ Redis에 저장하는 refreshToken에 대한 key 값을 명확하게 하기 위해 코드 수정 * ✏️ RefreshToken을 Redis에 저장하는 로직 수정 * ✏️ OAuth 정보와 관련된 DTO를 OAuthUserInfo, GoogleOAuthUserInfo 형태로 변경 * ✏️ RefreshToken을 구하는 로직 수정 * ✏️ JwtAuthenticationFilter 구조 변경 (jwtProvider, authenticationProvider, isIgnoredURIManager로 분리) * ✏️ JwtAuthenticationFilter에서 else if를 if로 수정 * ✏️ AccessToken, RefreshToken을 검사하는 필터에서 주석 추가 * ✏️ Cookie 발급 로직을 CookieUtils에서 처리하도록 수정 * ✏️ Security, OAuth 부분 패키지 구조 변경 * ✏️ ErrorCode 및 일부 패키지 구조 수정 * ✏️ 사용하지 않는 ErrorCode 삭제 * ✏️ RestControllerAdvice에서 쿠키를 받는 로직을 cookieUtils 를 사용하도록 수정 * ✏️ OAuth 관련 패키지 구조 일부 수정 * ✏️ Jwt 검증과 관련한 메서드를 JwtValidator로 분리 * ✏️ Securty, Jwt, OAuth 관련 패키지 분리 * ✏️ Domain security 관련 패키지를 Application로 이동 * ✏️ 패키지 구조 일부 변경 * 🎨 RefreshToken 오류 수정 및 임시 coverage 하향 * 🎨 Google oauth 예외처리 * 🐛 Google oauth 예외처리 --------- Co-authored-by: miiiinju1 <[email protected]>
@NoArgsConstructor(access = AccessLevel.PRIVATE) | ||
public class DailyDefenseInfoMapper { | ||
|
||
public static DailyDefenseInfoResponse fromNonAttempted(DailyDefense dailyDefense) { | ||
return DailyDefenseInfoResponse.builder() | ||
.defenseName(dailyDefense.getContentName()) | ||
.problemCount(dailyDefense.getProblemCount()) | ||
.attemptCount(dailyDefense.getAttemptCount()) | ||
.problems(DailyDefenseProblemInfoMapper.ofNonAttempted(dailyDefense.getDailyDefenseProblems())) | ||
.build(); | ||
} | ||
|
||
public static DailyDefenseInfoResponse ofAttempted(DailyDefense dailyDefense, DailyRecord dailyRecord) { | ||
return DailyDefenseInfoResponse.builder() | ||
.defenseName(dailyDefense.getContentName()) | ||
.problemCount(dailyDefense.getProblemCount()) | ||
.attemptCount(dailyDefense.getAttemptCount()) | ||
.problems(DailyDefenseProblemInfoMapper.ofAttempted( | ||
dailyDefense.getDailyDefenseProblems(), | ||
dailyRecord.getSolvedProblemNumbers()) | ||
) | ||
.build(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mapper 클래스를 따로 분리하는 것보다 DailyDefenseInfoResponse 객체 안에 두 개의 메서드를 만들어서 관리하는 것은 어떻게 생각하실까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SRP 원칙에 따라 하나의 클래스가 하나의 역할을 하도록 Mapper로 구성하는 것도 좋아보여요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀하신 부분도 제가 정말 많이 고민해봤던 부분 중 하나인데
"시험 시작 시 응답" dto의 경우에는 변환 로직이 복잡했던 관계로 mapper를 도입했었지만
"오늘의 문제 정보 응답" dto는 비교적 간단하여 정적 팩토리 메소드로 구현할까 생각도 했었습니다. 하지만 비즈니스 로직에 따라 응답 로직이 달라지는 점이 존재하여 응답 dto에 여러 변환 로직이 들어가는 것이 적절하지 않을 것으로 생각하여 mapper도입을 결정했습니다.
p.s. 변환 메소드에서 단순히 필드를 get만 하는 비즈니스 로직일 경우 mapper를 도입하더라도 큰 효과를 보지 못할 것으로 생각합니다!
// 등수 계산 | ||
// TODO 동점자 처리 필요 | ||
AtomicLong initialRank = new AtomicLong((long) page * size + 1); | ||
|
||
final List<DailyRecordRankResponse> dailyRecordRanks = dailyRecords.stream() | ||
.map(dr -> { | ||
String member = dr.getMember().getNickname(); | ||
Long rank = initialRank.getAndIncrement(); | ||
List<DailyDetailRankResponse> details = DailyDetailRankResponse.of(dr.getDetails()); | ||
Long totalSolvedTime = dr.getDetails().stream() | ||
.mapToLong(DailyDetail::getSolvedTime) | ||
.sum(); | ||
Long solvedCount = dr.getDetails().stream() | ||
.filter(DailyDetail::getIsSolved) | ||
.count(); | ||
|
||
return DailyRecordRankResponse.of(member, rank, requestTime, totalSolvedTime, solvedCount, details); | ||
}) | ||
.toList(); | ||
|
||
return DailyDefenseRankPageResponse.of(dailyRecordRanks, dailyRecords.getTotalPages(), page); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
만약 앞의 기록과 동일한 기록이라면 같은 순위를 부여하고, 다른 기록이라면 현재 번째 수를 순위로 등록하는 로직은 어떻게 생각하실까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분의 구현을 여태껏 미뤄왔는데
페이지가 달라졌지만 점수가 같은 경우에 대한 계산 부분을 해결해야 했습니다
ex) 점수가
90 80 70 70 70 70 70 70 70 60
위와 같을 때 page size가 5인 경우의 문제가 있었습니다
이 부분에 대해 @aj4941 님이 한 번 구현 해주시면 좋을 것 같아요! 알고리즘 장인이시니 ㅋㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재의 쿼리로는 한계가 있을 것 같아서 추가적인 정보를 함께 쿼리를 넣어서 해결해야 할 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 알아보니 rank함수를 쿼리에서 사용하면 해결 될 거 같아요!!
|
||
if (authentication == null || !authentication.isAuthenticated() || | ||
authentication instanceof AnonymousAuthenticationToken) { | ||
return null; | ||
} | ||
|
||
Object principal = authentication.getPrincipal(); | ||
|
||
/* | ||
* 일반적인 JWT 토큰을 사용할 때 발생. | ||
* */ | ||
if (principal instanceof Long) { | ||
return (Long) principal; | ||
} | ||
/* | ||
* @WithMockUser를 포함한 테스트나 기타 UserDetails 서비스를 사용할 때 발생한다. | ||
* */ | ||
if (principal instanceof UserDetails) { | ||
/* | ||
* principal이 UserDetails 타입인 경우, getUsername()에서 사용자 ID를 추출 | ||
* 따라서 @WithMockUser를 사용할 때는 getUsername에 Member ID를 넣어줘야 한다. | ||
*/ | ||
UserDetails userDetails = (UserDetails) principal; | ||
try { | ||
return Long.valueOf(userDetails.getUsername()); | ||
} catch (NumberFormatException e) { | ||
return null; | ||
} | ||
} | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분에서 Long 타입이 아니라 UserDetails로 타입이 나올 수 있는 것일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 주석에 쓰여있듯이 테스트 코드에서
'@WithMockUser' 를 쓰면 userDetails가 context에 설정되기 때문에 위와 같이 구성해 두었습니다!!
|
DailyDefense 비즈니스 로직 작성 #30
오늘의 문제 정보 반환 컨트롤러
요구 사항
1. 사용자가 로그아웃 상태일 경우 -> 오늘의 문제 정보만 반환
2. 사용자가 로그인 상태 & 오늘의 문제 시도 기록이 없으면 -> 오늘의 문제 정보만 반환
3. 사용자가 로그인 상태 & 오늘의 문제 시도 기록이 있으면 -> 오늘의 문제 정보 + 문제별 정답여부 필드 추가
구현
오늘의 문제를 검색하기 위한 조건은 현재 시간 뿐입니다.
현재 시간은 서버 기준으로 오늘의 문제를 반환하게 되므로, 파라미터를 받지 않고, Controller 내에서
LocalDateTime.now()
를 호출하여 현재 시각을 Service에 호출하여 가져올 수 있도록 구현했습니다.Service에서는 사용자의 로그인 여부에 따라 다른 로직을 구현하게 했습니다
사용자가 null일 경우 (이 경우 controller에서 securitycontext를 찾지 못했을 경우 null을 넘기는게 맞는 선택인지, 다른 방법을 취해야하는지 고민 중입니다.)
또는 사용자가 존재하지만, DailyDefense 응시 기록이 없는 경우
로 나누어 처리했습니다.
각 경우에 따라 ofAttempted, fromNonAttempted Mapper 메소드를 실행하여 반환했는데, 이 부분의 설계가 최선일지 궁금합니다.
DTO Mapper
DailyDefenseInfoMapper

DailyDefenseProblemInfoMapper

사용자가 풀었는지 여부의 isSolved 필드는

DailyRecord에서 푼 번호의 집합을 반환하는

getSolvedProblemNumbers()
메소드를 통해 구했습니다.만약 로그인하지 않았거나, 시도하지 않았다면 isSolved 필드는 null이 들어가고

@JsonInclude(JsonInclude.Include.NON_NULL)
어노테이션을 통해 NULL 필드는 노출하지 않게 만들었습니다.응답 Json
오늘의 문제 정보만 반환할 경우
오늘의 문제 정보 + 문제별 정답여부 함께 반환할 경우
수정사항 - Client 요청으로 문제 내용을 함께 응답해주어야함
기존 문제 데이터는 정적이라 S3에 관리하고 있었고, Lambda를 통해 bulk로 가져올 수 있게 구현해뒀습니다.
기존 Service에서 문제 내용도 가져올 수 있게 하여 Mapper에서 문제 내용도 함께 mapping하도록 수정했습니다.
수정된 Service 코드
제한 시간 후 시험을 종료하는 로직 추가
트랜잭션 롤백 시 타이머가 제거되지 않는 문제
트랜잭션 내에서 타이머가 설정되었다가 롤백되더라도, 타이머는 작동해버리는 현상이 발생합니다.
따라서 이를 해결하기 위해 AOP나, 이벤트를 사용하는 방법을 알아보았고, 이벤트를 발행한 후, @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)를 통해 커밋된 이후 이벤트를 실행하도록 하여 타이머가 정상적으로 트랜잭션에 참여할 수 있게 구현했습니다.
이벤트 발행 테스트 관련
이벤트 발행을 Mockbean으로 테스트하려 했지만, ApplicationEventPublisher는 MockBean으로 처리할 수 없는 점을 다음 블로그에 정리해보았습니다.
https://miiiinju.tistory.com/21