다음 학기 lectures sync 시 semester tag 자동 update#141
Conversation
eastshine2741
left a comment
There was a problem hiding this comment.
첫 작업 고생 많으셨습니다!!
이미 깔끔하게 잘 짜주셨지만 어떤 맥락에서 결정하신 건지를 코멘트나 description으로 남겨주시면 더 리뷰하기 쉬울 것 같아요!
| open var id: Long? = null, | ||
| @Column(name = "created_at", nullable = false, updatable = false) | ||
| open var createdAt: LocalDateTime = LocalDateTime.now(), | ||
| @field:UpdateTimestamp | ||
| @Column(name = "updated_at", nullable = false) | ||
| @OptimisticLock(excluded = true) | ||
| open val updatedAt: LocalDateTime? = LocalDateTime.now(), | ||
| open var updatedAt: LocalDateTime? = LocalDateTime.now(), |
There was a problem hiding this comment.
BaseEntity가 수정된 이유가 뭔가용?
There was a problem hiding this comment.
Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Handler dispatch failed: java.lang.NoSuchMethodError: 'void com.wafflestudio.snuttev.core.common.model.BaseEntity.$$_hibernate_write_id(java.lang.Long)'] with root cause
처음에 이렇게 에러 떠서 찾아보니까 BaseEntity id가 val이라 write method를 작성할 수가 없는게 원인이더라고요. 그래서 var로 바꾸니까 제대로 동작했습니다.
createdAt이랑 updatedAt도 원래는 val이었는데 충돌 가능성을 낮추기 위해 var로 교체하고, 대신 createdAt은 처음 db에 추가한 이후에 바꾸지 못하도록 updatable=false로 설정했습니다.
There was a problem hiding this comment.
제가 잘 이해를 못 했는데, 이번에 작성해주신 코드에서 Tag 객체가 생성된 후 id, createdAt, updatedAt을 바꾸는 코드는 없었지만 hibernate에서 에러를 낸 건가용...?
그렇다면 저희가 기존에 엔티티를 수정하고 save하는 경우에도 같은 에러가 나야 했을 것 같은데 이번에만 난 이유도 궁금합니다!
| private fun tagSaveStepListener(targetYearSemester: TargetYearSemester? = null): StepExecutionListener { | ||
| return object : StepExecutionListener { | ||
| override fun afterStep(stepExecution: StepExecution): ExitStatus { | ||
| if (stepExecution.exitStatus == ExitStatus.COMPLETED && targetYearSemester != null) { | ||
| saveTagIfNotExists(targetYearSemester) |
There was a problem hiding this comment.
SemesterLecture 싱크가 성공하고 난 후에만 yearsemester 를 저장하시고, yearsemester 저장 성공 여부에는 관계없이 다음 step으로 넘어가시는군요 전 좋습니다!
그리고 제가 spring batch는 잘 모르지만.. 이 상태라면 앞선 트랜잭션이 종료된 상태에서 실행되는 것 같은데 앞선 step에서처럼 JpaTransactionManager를 지정해주지 않아도 잘 동작하나요?
생각해보니 세미나에서 jpa를 안 다뤘군요 죄송.. 그래도 찾아보시면 금방 아실 거에요...!
There was a problem hiding this comment.
로컬에서 돌렸을 때는 별 문제 없이 동작하기는 했는데, 그래도 리스너 부분도 별도의 트랜잭션으로 묶어주면 좋을 것 같긴 하네요! saveTagIfNotExists가 지금은 config 내부에 있는데 이걸 TagService로 빼서 @transactional로 묶어주겠습니다
There was a problem hiding this comment.
아하 쿼리는 잘 가는군요! 저도 찾아봤는데 JPA repository 함수들이 내부적으로 트랜잭션 여는 동작을 해서 잘 되나 보네용(JpaRepository의 기본 구현체인 SimpleJpaRepository에 class-level에 @Transactional이 달려 있음) 좋습니다!
| private fun saveTagIfNotExists(targetYearSemester: TargetYearSemester) { | ||
| val stringValue = targetYearSemester.year.toString() + "," + targetYearSemester.semester.toString() | ||
| if (tagRepository.searchTagByStringValue(stringValue) == null) { | ||
| val tagGroup = tagGroupRepository.findByName(name = "학기") ?: throw TagGroupNotFoundException |
There was a problem hiding this comment.
spring batch에서 exception을 던지면 어떻게 동작하나요? (이후 step은 계속해서 실행되는지, 나중에 재시도 가능하도록 데이터가 남는지, ...)
There was a problem hiding this comment.
다시 보니까 tagRepository에 학기 태그가 하나라도 들어있으면 foreign key 때문에 tagGroupRepository에도 학기 태그가 있을 수 밖에 없어서, 이 부분은 예외 처리를 안 하고 그냥 findByName 그대로 쓰면 되지 않나 싶어요..!
There was a problem hiding this comment.
findByName을 하시더라도 findByName이 null을 반환한 경우 대응은 필요할 것 같아요! 물론 이미 찾아주신 것처럼 TagGroup이 존재하긴 하니 자주 발생할 상황은 절대 아니지만용
가장 간단하게 대응하면 !! 붙이고 NullPointerException 낼 것 같은데, 그런 상황에 spring batch가 어떻게 동작하는지 알고 계신지가 질문이었습니다! 사실 저도 잘 몰라서 순수한 질문입니다,,
| Tag( | ||
| tagGroup = tagGroup, | ||
| name = name, | ||
| ordering = ordering, | ||
| stringValue = stringValue, | ||
| description = null, | ||
| ), |
There was a problem hiding this comment.
TagGroup.valueType으로 STRING, INT, LOGIC 중 나누어서 stringValue, intValue에 저장하도록 유연하게 되어있었군요
| WRONG_MAIN_TAG(20002, HttpStatus.BAD_REQUEST), | ||
| WRONG_SEARCH_TAG(20003, HttpStatus.BAD_REQUEST), | ||
| EVALUATION_CONTENT_BLANK(20004, HttpStatus.BAD_REQUEST), | ||
| INVALID_SEMESTER_VALUE(20005, HttpStatus.BAD_REQUEST), |
There was a problem hiding this comment.
http API에서 사용되는 에러가 아니라서 // 400 주석에 함께 있기에는 애매한 것 같은데 어떻게 생각하시나용?
| } | ||
| } | ||
|
|
||
| private fun saveTagIfNotExists(targetYearSemester: TargetYearSemester) { |
There was a problem hiding this comment.
저번 스프린트 구두) 최근 한 학기만 저장하므로 기존 누락된 학기들은 DB에 수동 INSERT
| return object : StepExecutionListener { | ||
| override fun afterStep(stepExecution: StepExecution): ExitStatus { | ||
| if (stepExecution.exitStatus == ExitStatus.COMPLETED && targetYearSemester != null) { | ||
| tagService.saveYearSemesterTagIfNotExists(targetYearSemester.year, targetYearSemester.semester) |
There was a problem hiding this comment.
save tag 로직을 tagSerivce로 옮겨서 @transcactional로 묶었습니다
| ; | ||
|
|
||
| companion object { | ||
| fun labelOfOrNull(value: Int): String? = entries.firstOrNull { it.value == value }?.label |
There was a problem hiding this comment.
invalid semester value일 경우(학기 숫자가 1~4가 아닌 값이 들어올 때) 기존에는 exception을 띄웠었는데, 그냥 로그만 남기고, 리스너에서 tag 저장 작업 중에 리턴하는거로 수정했습니다
| description = tag.description, | ||
| ordering = tag.ordering, | ||
| ) | ||
|
|
There was a problem hiding this comment.
SnuttLectureSyncJob Config에서 TagService로 옮겨온 로직
변경사항
-"학기"가 tag group db에 없거나, 입력된 학기 정보가 잘못된 경우에 대한 대응
기존: 각각 custom exception / 변경: log 남기고 tag 저장 작업 중단 후 리턴 (앞서 진행된 sync 작업은 정상 진행됨)
NEXT_SEMESTER_SYNC_JOB 수행 시 다음 학기에 해당하는 태그를 tagRepository에 저장하는 로직 추가