[Refactor] 보행등 코드 커버리지 높이기 #229

ByungilOh-Fillip · 2025-04-29T18:19:38Z

#️⃣ 연관된 이슈

#215

✅ 체크리스트

📝 작업 내용

Traffic 도메인 log 처리 추가
Traffic 도메인 Jacoco 커버리지 달성
Traffic 도메인 refactor 진행 및 작업 추가 사항 생성

service에서 repository를 직접적으로 접근해서 사용하지 않게 수정
역할 분리가 명확하지 않아 테스트 시 어려움이 있어 작업 필요

👀 리뷰어 가이드라인

로그랑 테스트 코드 커버리지 맞게 올렸는데 부족한 부분이나 내용 추가할 사항 있다면 말씀해주세요!
TrafficService TODO 내용도 보시고 알려주고 싶은 부분있으시면 편하게 부탁드립니다!

++ refactor 사항 추가 crossroad 리펙토링 진행하며 작업 진행

github-actions · 2025-04-29T18:31:15Z

Test Results

76 files 76 suites 5m 11s ⏱️
279 tests 278 ✅ 0 💤 1 ❌
280 runs 279 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 25000ea.

♻️ This comment has been updated with latest results.

src/main/java/org/programmers/signalbuddyfinal/domain/trafficSignal/service/TrafficService.java

DongminL · 2025-04-30T00:44:01Z

...org/programmers/signalbuddyfinal/domain/trafficSignal/repository/TrafficRedisRepository.java

+        // 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);


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을 상속받으면 돼요!

오...변경해서 다시올려볼게요! 감사합니다!

src/main/java/org/programmers/signalbuddyfinal/domain/trafficSignal/service/TrafficService.java

ByungilOh-Fillip · 2025-04-30T02:27:35Z

리뷰 수정사항

service log info->debug로 수정
불필요한 Map 객체 이용 -> dto로 바로 받게 변경
repository와 service에서 redis 중복 호출 -> repository에서만 호출하도록 분리

변경으로 생긴 테스트 취약 및 통과하지 못하는 테스트 수정

[TrafficRedisRepositoryTest] - 주변 보행등 redis 데이터 반환
[TrafficRedisRepositoryTest] - 보행등 저장 테스트
[TrafficServiceTest] - 주변 보행등 정보 redis 테스트

## 리뷰 수정사항 - service log info-> debug로 수정 - 불필요한 Map 객체 이용 -> dto로 바로 받게 변경 - repository와 service에서 redis 중복 호출 -> repository에서만 호출하도록 분리 - 테스트 코드 수정 -> 변경된 내용으로 mock 수정

sonarqubecloud · 2025-04-30T03:04:51Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
67.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

DongminL · 2025-04-30T03:21:46Z

src/main/java/org/programmers/signalbuddyfinal/domain/trafficSignal/service/TrafficService.java

@@ -22,53 +20,66 @@
 @Transactional(readOnly = true)
 public class TrafficService {

+    private static final String TRAFFIC_REDIS_KEY = "traffic:info";


사용하지 않는 변수는 이제 필요없으니 삭제하면 좋을 거 같아요

헉 그러게요..! 바로 삭제할게요!

DongminL · 2025-04-30T03:22:28Z

...t/java/org/programmers/signalbuddyfinal/domain/trafficSignal/service/TrafficServiceTest.java

+    /**
+     * 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();
+    }*/
+}


이 테스트는 왜 주석 처리를 하셨는지 알 수 있을까요?

레포지토리에서 반환된 엔티티를

이 테스트는 왜 주석 처리를 하셨는지 알 수 있을까요?

서비스에서 repository.findbyid()를 이용하면서 테스트에서 엔티티를 모킹해서 사용하는 경우가 생겼는데
엔티티를 모킹해서 사용하면 JPA에서 영속성 컨텍스트 문제나 프록시 충돌같은 문제가 생길 수 있다고 해서 dto객체를 이용하는걸로 서비스를 리펙토링 해야겠다고 판단했었어요!

그런데 테스트 코드 작업을 어느정도 해둔 상태였어서 리펙토링 작업을 새로운 이슈생성해서 진행하고 완료 후 테스트 코드도 다시 정리해서 올리려구 남겨뒀습니다!

따로 메모해둬도 되는데 우선 지워둘까요?

기존 코드가 JPA를 사용해서 findById로 Entity를 가져왔고, DTO로 변환하여 사용하지 않았다는 말씀이신거죠?

네네 맞습니다

아하 그럼 리팩터링 하신 다음에 테스트 해보시면 될 것 같습니다!

네 답변 감사합니다! ㅎㅎ

DongminL · 2025-04-30T03:30:49Z

...org/programmers/signalbuddyfinal/domain/trafficSignal/repository/TrafficRedisRepository.java

-        trafficData.put("address", trafficResponse.getAddress());
-
-        hashOperations.put(KEY_HASH, trafficId.toString(), trafficData);
+        hashOperations.put(KEY_HASH, trafficId.toString(), trafficResponse);


Redis에 Hash 구조로 저장된 데이터를 삭제할 때, 관련 데이터가 전부 삭제되는 것을 확인하셨을까요?

그리고 ValueOperations 를 사용하시면 객체가 통째로 직렬화가 되어 저장됩니다. 역직렬화도 마찬가지로 가능합니다.

삭제할 때가 TTL이 모두 소진됐을 때를 말씀하시는거 맞죠? 그럼 삭제는 잘 됩니다!
ValueOperations 그래서 쓰는거군여..! 저도 그럼 코드 변경해볼게요! 감사합니다 ㅎㅎ

zzuharchive · 2025-04-30T02:59:02Z

.../programmers/signalbuddyfinal/domain/trafficSignal/repository/CustomTrafficRepoImplTest.java

+public class CustomTrafficRepoImplTest extends RepositoryTest {
+
+    @Mock
+    private CustomTrafficRepository customTrafficRepository;


Repository 테스트는 실제로 쿼리동작을 검증하는 목적으로 작성되기 때문에, mock보다는 실제 Repository를 @ Autowired해서 테스트하는 방식이 더 적절할 것 같습니다!
mock을 사용하면 when-then으로 지정한 값이 그대로 리턴되기 때문에, 쿼리의 실제 작동 여부를 확인하기는 어려울 수 있을 것 같아요!

엇 그러게요..! 감사합니다! 테스트 수정해서 올려둘게요!

ByungilOh-Fillip · 2025-05-05T15:47:58Z

리뷰 사항 체크리스트

ValueOperation 적용
Repository 테스트 수정

## 리뷰 수정사항 - TrafficRedisRepository hash->value로 변경 - TrafficService 불필요한 코드제거 - CustomTrafficRepoImplTest - RepositoryTest로 변경

원인 KEY_INFO가 Redis 예약어로 설정되어있어 충돌이 나는 문제 해결 KEY_INFO->KEY_VALUE로 수정

ByungilOh-Fillip added 7 commits April 30, 2025 03:06

test: CustomTrafficRepoImpl 테스트 작성

f6d2d5b

test: TrafficController 테스트 수정

5cd28e2

test: TrafficCsvService 테스트 수정 및 작성

b50c8d3

refactor: Traffic 도메인 로그 추가 및 코드 리펙토링

ac3e9d3

test: TrafficRedisRepository 테스트 작성

586eed4

test: TrafficService 테스트 작성

1b941f0

++ refactor 사항 추가 crossroad 리펙토링 진행하며 작업 진행

docs: 테스트 진행하며 더미 데이터 재 정의

e87d422

ByungilOh-Fillip added Refactor Test labels Apr 29, 2025

ByungilOh-Fillip added this to the unifiedSignal 테스트 커버리지 높이기 milestone Apr 29, 2025

ByungilOh-Fillip self-assigned this Apr 29, 2025

ByungilOh-Fillip linked an issue Apr 29, 2025 that may be closed by this pull request

보행등 코드 커버리지 높이기 #215

Open

1 task

DongminL reviewed Apr 30, 2025

View reviewed changes

src/main/java/org/programmers/signalbuddyfinal/domain/trafficSignal/service/TrafficService.java Outdated Show resolved Hide resolved

DongminL reviewed Apr 30, 2025

View reviewed changes

src/main/java/org/programmers/signalbuddyfinal/domain/trafficSignal/service/TrafficService.java Outdated Show resolved Hide resolved

DongminL reviewed Apr 30, 2025

View reviewed changes

zzuharchive reviewed Apr 30, 2025

View reviewed changes

ByungilOh-Fillip added 2 commits May 6, 2025 02:49

refactor: pr 코드리뷰 적용

df847bf

## 리뷰 수정사항 - TrafficRedisRepository hash->value로 변경 - TrafficService 불필요한 코드제거 - CustomTrafficRepoImplTest - RepositoryTest로 변경

fix: build error 해결

25000ea

원인 KEY_INFO가 Redis 예약어로 설정되어있어 충돌이 나는 문제 해결 KEY_INFO->KEY_VALUE로 수정

[Refactor] 보행등 코드 커버리지 높이기 #229

Are you sure you want to change the base?

[Refactor] 보행등 코드 커버리지 높이기 #229

Uh oh!

Conversation

ByungilOh-Fillip commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

#️⃣ 연관된 이슈

✅ 체크리스트

📝 작업 내용

👀 리뷰어 가이드라인

Uh oh!

github-actions bot commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ByungilOh-Fillip commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

리뷰 수정사항

변경으로 생긴 테스트 취약 및 통과하지 못하는 테스트 수정

Uh oh!

sonarqubecloud bot commented Apr 30, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ByungilOh-Fillip commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

리뷰 사항 체크리스트

Uh oh!

Uh oh!

ByungilOh-Fillip commented Apr 29, 2025 •

edited

Loading

github-actions bot commented Apr 29, 2025 •

edited

Loading

ByungilOh-Fillip commented Apr 30, 2025 •

edited

Loading

ByungilOh-Fillip commented May 5, 2025 •

edited

Loading