-
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
[fix] 더미 데이터 생성 및 주요 API 로컬에서 검증 (#54) #57
Conversation
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.
어드민 기능을 제외한 API 검증을 마쳤고, 노션에 Prod DB에 더미 데이터를 삽입하는 쿼리를 작성해두었습니다.
어드민 기능도 구현이 마무리되고 일부 테이블 필드가 재조정된 후 동일하게 API를 검증하고 더미 데이터를 삽입하겠습니다.
public ResponseEntity<Boolean> createComment(@EventUserAnnotation EventUserInfo userInfo, @PathVariable String eventFrameId, @RequestBody @Valid CreateCommentDto dto) { | ||
boolean isPositive = apiService.analyzeComment(dto.getContent()); | ||
return ResponseEntity.ok(commentService.createComment(userInfo.getUserId(), eventFrameId, dto, isPositive)); | ||
public ResponseEntity<Boolean> createComment(@Parameter(hidden = true) @EventUserAnnotation EventUserInfo userInfo, @PathVariable String eventFrameId, @RequestBody @Valid CreateCommentDto dto) { | ||
return ResponseEntity.ok(commentService.createComment(userInfo.getUserId(), eventFrameId, dto)); |
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.
기존 기능은 eventFrameId, eventUser의 유효성 검사 및 하루에 여러 개의 기대평을 작성하려는 경우를 감지하는 예외 처리 과정이CommentService의 createComment에 존재하기에 유효성 검사 여부와 관계없이 무조건 외부 API가 호출되는 상황이었습니다.
이러한 의미 없는 비용 부담을 막고자 관련 비즈니스 로직을 개편했습니다. 자세한 건 후술하겠습니다.
public ResponseEntity<Boolean> isCommentable(@EventUserAnnotation EventUserInfo userInfo) { | ||
public ResponseEntity<Boolean> isCommentable(@Parameter(hidden = true) @EventUserAnnotation EventUserInfo userInfo) { |
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.
어노테이션이 Swagger에 떠 혼란을 줄 수 있기에 hidden 처리했습니다.
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.
이벤트 유저는 외부에서 전달하는 데이터가 아니니까 hidden=true로 가리는 것이 좋은 것 같습니다
// 오늘 날짜 기준으로 이미 유저의 기대평이 등록되어 있는지 확인 | ||
@Query(value = "SELECT COUNT(*) FROM comment WHERE event_user_id = :eventUserId AND DATE(createdAt) = CURDATE()", nativeQuery = true) | ||
// 오늘 날짜 기준으로 이미 유저의 기대평이 등록되어 있는지 확인 (JPQL) | ||
@Query("SELECT (COUNT(c) > 0) FROM Comment c WHERE c.eventUser.id = :eventUserId AND FUNCTION('DATE', c.createdAt) = CURRENT_DATE") |
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.
기존 네이티브 쿼리가 원인을 알 수 없는 에러를 발생시키고 있었습니다. 네이티브 쿼리에서 JPQL로 쿼리를 바꾸어서 추상화를 유지하고, 구체적인 DB 및 쿼리에 메서드가 의존하는 것을 피하고자 했습니다.
다만 JPQL 특성상 DB별 내장 함수를 FUNCTION()으로 실행하고 있기 때문에 만약 특정 DB가 해당 함수를 지원하지 않는다면 여전히 오류를 마주하게 됩니다.
|
||
private EventUser getEventUser(String userId) { | ||
return eventUserRepository.findByUserId(userId) | ||
.orElseThrow(() -> new CommentException(ErrorCode.EVENT_USER_NOT_FOUND)); | ||
} | ||
|
||
private EventFrame getEventFrame(String eventFrameId) { | ||
return eventFrameRepository.findByFrameId(eventFrameId) | ||
.orElseThrow(() -> new CommentException(ErrorCode.EVENT_FRAME_NOT_FOUND)); | ||
} |
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.
조회 로직이 여러 메서드에서 반복되고 있어 private으로 별도로 분리했습니다.
@RequiredArgsConstructor | ||
@Service | ||
public class ApiService { | ||
public class CommentValidator { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(ApiService.class); | ||
private static final Logger log = LoggerFactory.getLogger(CommentValidator.class); | ||
private final RestTemplate restTemplate = new RestTemplate(); | ||
private final NaverApiConfig naverApiConfig; | ||
private final ObjectMapper objectMapper; | ||
|
||
public boolean analyzeComment(String content) { | ||
String responseBody = sendSentimentAnalysisRequest(content); | ||
return parseSentimentAnalysisResponse(responseBody, content); | ||
} | ||
|
||
private String sendSentimentAnalysisRequest(String content) { | ||
HttpHeaders headers = createHeaders(); | ||
String requestJson = createRequestBody(content); | ||
|
||
HttpEntity<String> requestEntity = new HttpEntity<>(requestJson, headers); | ||
log.info("comment <{}> sentiment analysis request to Naver API", content); | ||
|
||
ResponseEntity<String> responseEntity = restTemplate.postForEntity(naverApiConfig.getUrl(), requestEntity, String.class); | ||
return responseEntity.getBody(); | ||
} |
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.
초기에 ApiService 컴포넌트를 운영했지만, 메서드가 하나 뿐이라는 점과 앞서 언급드린 로직 상 허점으로 인한 무의미한 비용 지출을 막고자 유효성 검사를 통과한 경우에 한해서 API가 호출되도록 변경했습니다.
또한, 클래스 및 메서드의 역할이 모호하다고 판단해 부정적인 댓글을 검사하는 기능에 걸맞게 CommentValidator로 명칭을 변경했습니다.
@RequiredArgsConstructor | ||
@Service | ||
public class FcfsAnswerService { | ||
|
||
private final StringRedisTemplate stringRedisTemplate; | ||
|
||
public boolean judgeAnswer(Long eventSequence, String answer) { | ||
// 잘못된 이벤트 참여 시간 | ||
String startTime = stringRedisTemplate.opsForValue().get(FcfsUtil.startTimeFormatting(eventSequence.toString())); | ||
if(startTime == null) { | ||
throw new FcfsEventException(ErrorCode.FCFS_EVENT_NOT_FOUND); | ||
} | ||
if (LocalDateTime.now().isBefore(LocalDateTime.parse(startTime))){ | ||
throw new FcfsEventException(ErrorCode.INVALID_EVENT_TIME); | ||
} | ||
|
||
// 정답 비교 | ||
String correctAnswer = stringRedisTemplate.opsForValue().get(FcfsUtil.answerFormatting(eventSequence.toString())); |
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.
기존 선착순 API 테스트 중 이벤트 시작 시간 전일 때 제출한 답이 정답인 경우에는 이벤트 시각과 관련 예외를 발생시키는데, 오답인 경우 예외를 발생시키지 않는 문제가 있었습니다.
정답 여부와 별개로, 이벤트 시작 시각 전인 경우엔 중복을 감수하고 일관된 예외를 발생시키도록 수정했습니다.
this.userName = userName; | ||
this.createdAt = createdAt.toString(); | ||
} | ||
|
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.
큰 이유가 없다면 문자열로 변환하는 대신 LocalDateTime 형식으로 값을 관리하는 것이 좋을 것 같습니다. 현재 프로젝트는 하나의 ObjectMapper에서 JavaTimeModule을 로드해서 문자열 형식으로 json 직렬화를 해주고 있는데, 이 부분에서 의도하지 않은 차이가 발생할 수도 있다고 생각해요.
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.
함께 코드 검토해주신 덕분에 toString()없이 직렬화/역직렬화가 가능해졌습니다. 감사합니다:)
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.
고생하셨습니다. 오늘 도메인 연결, mock 데이터 삽입 등 너무 많은 일을 해주셔서, 좀 더 분발해야겠다는 생각이 드네요!
#️⃣ 연관 이슈
📝 작업 내용
참고 이미지 및 자료
💬 리뷰 요구사항