[Refactor] 보행등 코드 커버리지 높이기#229
Hidden character warning
Conversation
++ refactor 사항 추가 crossroad 리펙토링 진행하며 작업 진행
Test Results 76 files 76 suites 5m 11s ⏱️ For more details on these failures, see this check. Results for commit 25000ea. ♻️ This comment has been updated with latest results. |
| // HASH 데이터 저장 | ||
| Map<String, String> trafficData = new HashMap<>(); | ||
| trafficData.put("serialNumber", String.valueOf(trafficResponse.getSerialNumber())); | ||
| trafficData.put("district", trafficResponse.getDistrict()); | ||
| trafficData.put("signalType", trafficResponse.getSignalType()); | ||
| trafficData.put("address", trafficResponse.getAddress()); | ||
|
|
||
| // HASH 데이터 저장 | ||
| Map<String, String> trafficData = new HashMap<>(); | ||
| trafficData.put("serialNumber", String.valueOf(trafficResponse.getSerialNumber())); | ||
| trafficData.put("district", trafficResponse.getDistrict()); | ||
| trafficData.put("signalType", trafficResponse.getSignalType()); | ||
| trafficData.put("address", trafficResponse.getAddress()); | ||
| hashOperations.put(KEY_HASH, trafficId.toString(), trafficData); |
There was a problem hiding this comment.
public void putStateCache(Long crossroadId, CrossroadStateResponse response) {
ValueOperations<Object, Object> operations = redisTemplate.opsForValue();
int minTimeLeft = response.minTimeLeft();
minTimeLeft *= 100; // 1/10초 단위를 1/1000(ms)로 변환
operations.set(STATE_PREFIX + crossroadId, response, minTimeLeft, TimeUnit.MILLISECONDS);
}이 코드처럼 DTO 자체를 직렬화 시켜서 저장시킬 수 있습니다. 이때 저장하려는 DTO는 Serializable을 상속받으면 돼요!
There was a problem hiding this comment.
오...변경해서 다시올려볼게요! 감사합니다!
리뷰 수정사항
변경으로 생긴 테스트 취약 및 통과하지 못하는 테스트 수정
|
## 리뷰 수정사항 - service log info-> debug로 수정 - 불필요한 Map 객체 이용 -> dto로 바로 받게 변경 - repository와 service에서 redis 중복 호출 -> repository에서만 호출하도록 분리 - 테스트 코드 수정 -> 변경된 내용으로 mock 수정
|
| @Transactional(readOnly = true) | ||
| public class TrafficService { | ||
|
|
||
| private static final String TRAFFIC_REDIS_KEY = "traffic:info"; |
There was a problem hiding this comment.
사용하지 않는 변수는 이제 필요없으니 삭제하면 좋을 거 같아요
There was a problem hiding this comment.
헉 그러게요..! 바로 삭제할게요!
| /** | ||
| * TODO: TrafficService 코드 refactor 필요 | ||
| * -> findById() | ||
| * Servive 레이어에서 entity에 접근하는 repository를 직접 사용하지 않게 | ||
| */ | ||
| /* @Test | ||
| @DisplayName("보행등 ID 캐싱 테스트") | ||
| void testTrafficFindByIdRedisNotExists() { | ||
|
|
||
| // Given | ||
| Long id = expected.get(0).getTrafficSignalId(); | ||
|
|
||
| when(redisTemplate.hasKey(TRAFFIC_REDIS_KEY)).thenReturn(false); | ||
| when(trafficRepository.findById(id)).thenReturn(expected.get(0)); | ||
|
|
||
| // When | ||
| TrafficResponse result = trafficService.trafficFindById(id); | ||
|
|
||
| // Then | ||
| assertThat(result).isNotNull(); | ||
| }*/ | ||
| } |
There was a problem hiding this comment.
이 테스트는 왜 주석 처리를 하셨는지 알 수 있을까요?
There was a problem hiding this comment.
레포지토리에서 반환된 엔티티를
이 테스트는 왜 주석 처리를 하셨는지 알 수 있을까요?
서비스에서 repository.findbyid()를 이용하면서 테스트에서 엔티티를 모킹해서 사용하는 경우가 생겼는데
엔티티를 모킹해서 사용하면 JPA에서 영속성 컨텍스트 문제나 프록시 충돌같은 문제가 생길 수 있다고 해서 dto객체를 이용하는걸로 서비스를 리펙토링 해야겠다고 판단했었어요!
그런데 테스트 코드 작업을 어느정도 해둔 상태였어서 리펙토링 작업을 새로운 이슈생성해서 진행하고 완료 후 테스트 코드도 다시 정리해서 올리려구 남겨뒀습니다!
따로 메모해둬도 되는데 우선 지워둘까요?
There was a problem hiding this comment.
기존 코드가 JPA를 사용해서 findById로 Entity를 가져왔고, DTO로 변환하여 사용하지 않았다는 말씀이신거죠?
There was a problem hiding this comment.
아하 그럼 리팩터링 하신 다음에 테스트 해보시면 될 것 같습니다!
There was a problem hiding this comment.
네 답변 감사합니다! ㅎㅎ
| trafficData.put("address", trafficResponse.getAddress()); | ||
|
|
||
| hashOperations.put(KEY_HASH, trafficId.toString(), trafficData); | ||
| hashOperations.put(KEY_HASH, trafficId.toString(), trafficResponse); |
There was a problem hiding this comment.
Redis에 Hash 구조로 저장된 데이터를 삭제할 때, 관련 데이터가 전부 삭제되는 것을 확인하셨을까요?
그리고 ValueOperations 를 사용하시면 객체가 통째로 직렬화가 되어 저장됩니다. 역직렬화도 마찬가지로 가능합니다.
There was a problem hiding this comment.
삭제할 때가 TTL이 모두 소진됐을 때를 말씀하시는거 맞죠? 그럼 삭제는 잘 됩니다!
ValueOperations 그래서 쓰는거군여..! 저도 그럼 코드 변경해볼게요! 감사합니다 ㅎㅎ
| public class CustomTrafficRepoImplTest extends RepositoryTest { | ||
|
|
||
| @Mock | ||
| private CustomTrafficRepository customTrafficRepository; |
There was a problem hiding this comment.
Repository 테스트는 실제로 쿼리동작을 검증하는 목적으로 작성되기 때문에, mock보다는 실제 Repository를 @ Autowired해서 테스트하는 방식이 더 적절할 것 같습니다!
mock을 사용하면 when-then으로 지정한 값이 그대로 리턴되기 때문에, 쿼리의 실제 작동 여부를 확인하기는 어려울 수 있을 것 같아요!
There was a problem hiding this comment.
엇 그러게요..! 감사합니다! 테스트 수정해서 올려둘게요!
리뷰 사항 체크리스트
|
## 리뷰 수정사항
- TrafficRedisRepository hash->value로 변경
- TrafficService 불필요한 코드제거
- CustomTrafficRepoImplTest
- RepositoryTest로 변경
원인 KEY_INFO가 Redis 예약어로 설정되어있어 충돌이 나는 문제 해결 KEY_INFO->KEY_VALUE로 수정



#️⃣ 연관된 이슈
#215
✅ 체크리스트
📝 작업 내용
Traffic 도메인 log 처리 추가
Traffic 도메인 Jacoco 커버리지 달성
Traffic 도메인 refactor 진행 및 작업 추가 사항 생성
👀 리뷰어 가이드라인
로그랑 테스트 코드 커버리지 맞게 올렸는데 부족한 부분이나 내용 추가할 사항 있다면 말씀해주세요!
TrafficService TODO 내용도 보시고 알려주고 싶은 부분있으시면 편하게 부탁드립니다!