Skip to content

Step02. 심화과제 - #4

Merged
loveAlakazam merged 27 commits into
mainfrom
step02
Mar 29, 2025
Merged

loveAlakazam merged 27 commits into
mainfrom
step02

Conversation

@loveAlakazam

@loveAlakazam loveAlakazam commented Mar 23, 2025

Copy link
Copy Markdown
Owner

커밋 링크


리뷰 포인트(질문)

[리뷰 포인트 1] 만일 DB까지 붙이게된다면, 테스트가 완료되면 테스트데이터를 지우게될텐데요. 보통 afterEach를 사용해서 truncate를 쓰는편인가요? 아니면 다른 방법도 있을까요?

하나의 테스트파일에서 통합테스트를 했을때는 발견하지 못했지만, 여러개의 통합테스트들을 한꺼번에 테스트를 했을 경우에는 아래와 같이 포인트내역 조회테스트에서 실패가 됐습니다.

image

로그를 확인해보니, 응답바디에 id =1 인 유저포인트에 대한 포인트내역이 앞에서 진행한 통합테스트의 데이터가 쌓여있기 때문입니다.

현재에는 DB가없이 다시 로딩을해서라도 데이터를 지워나갔습니다. DB의 경우에는 디스크에 저장되어있을텐데요. 더나아가서 만일 DB까지 붙이게된다면, 테스트가 완료되면 테스트데이터를 지우게될텐데요. 보통 afterEach를 사용해서 truncate를 쓰는편인가요? 아니면 다른 방법도 있을까요?


[리뷰포인트 2] 동시성제어에 대한 통합테스트를 API 엔드포인트를 호출하는 방식은 잘못된 통합테스트일까요?

저의 경우에는 API 엔드포인트를 호출해서 (코드1)처럼 동시성제어 테스트코드를 작성했습니다.

(코드1)

@SpringBootTest
@AutoConfigureMockMvc
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
public class ChargeConcurrencyTest {
	@Autowired
	private MockMvc mockMvc;

	@Autowired
	private PointService pointService;

	@Autowired
	private UserPointRepository userPointRepository;

	@Autowired
	private PointHistoryRepository pointHistoryRepository;

	@Autowired
	private ObjectMapper objectMapper;

	@Autowired
	private UserPointLockManager userPointLockManager;

	private static final Logger log = LoggerFactory.getLogger(ChargeConcurrencyTest.class);


	@BeforeEach
	void setUp() {
		// UserPoint 의 초기 포인트값을 0 으로한다.
		userPointRepository.save(1L, 0L);
	}


	@Test
	void 한명의_유저가_동시에_1000원_충전을_100번_요청했을때_정상적으로_합산에_성공해야한다() throws Exception {
		// given
		long id = 1L;
		int threadCount = 100;
		long chargeAmount = 1000L;

		ExecutorService executor = Executors.newFixedThreadPool(10); // 스레드풀 10개
		CountDownLatch latch = new CountDownLatch(threadCount); // 요청가능한 스레드개수
		ChargeRequestBody requestBody = new ChargeRequestBody(chargeAmount);
		String json = objectMapper.writeValueAsString(requestBody);

		// when
		for(int i = 0 ; i < threadCount ; i++) {
			executor.submit(()-> {
				try {
					// 포인트충전 API 호출
					mockMvc.perform(patch("/point/"+id+"/charge")
						.contentType(MediaType.APPLICATION_JSON)
						.content(json));
				} catch (Exception e) {
					e.printStackTrace();
					throw new RuntimeException(e);
				} finally {
					latch.countDown(); // 요청가능 스레드 개수 감소
				}
			});
		}

		latch.await(); // 다 끝날 때까지 대기
		Thread.sleep(1000); // 1초 정도 대기후 최종 포인트 확인

		// then
		long expectedPoint = chargeAmount * threadCount;
		long finalPoint = userPointRepository.findById(id).point();
		assertEquals(expectedPoint, finalPoint, "충전후 포인트값이 예상값("+expectedPoint+")과 실제값("+finalPoint+")이 서로다릅니다.");
	}
}

준규님과 학습메이트 예진님과 같이 제 코드를 화면공유하게 됐는데요. "락을 이용한 동시성 제어는 서비스 통합테스트 라고 생각한다" 라는 의견을 들었습니다.

제가 생각한 컨트롤러에서 테스트는

  • 단순히 호출해서 e2e 관점에서 처럼 정상이냐 에러냐를 확인이 필요할때
  • 단순히 호출해서 테스트대상과 테스트대상을 이루는 의존성의 연결이 적절한지 확인이 필요할때

다시 생각해보니까 충전서비스에서도 래포지토리와 락관리자라는 서비스외의 대상이 있기 때문에
포인트충전을 담당하는 charge 서비스 메서드를 호출목적으로한다면 통합테스트라는 것을 확인하여 코드를 하나더 만들었습니다.

(코드2)

public class ChargeConcurrencyServiceTest {

	private PointService pointService;

	private UserPointRepository userPointRepository;

	private PointHistoryRepository pointHistoryRepository;

	private UserPointLockManager userPointLockManager;

	private static final Logger log = LoggerFactory.getLogger(ChargeConcurrencyServiceTest.class);

	@BeforeEach
	void setUp() {
		// 직접 의존성 주입(SpringBootTest / Autowired 을 사용시 자동으로 의존성주입)
		userPointLockManager = new UserPointLockManager();
		pointHistoryRepository = new PointHistoryRepositoryImpl(new PointHistoryTable());
		userPointRepository = new UserPointRepositoryImpl(new UserPointTable());
		pointService = new PointServiceImpl(userPointRepository, pointHistoryRepository, userPointLockManager);

		// UserPoint 의 초기 포인트값을 0 으로한다.
		userPointRepository.save(1L, 0L);
	}

	@Test
	void 한명의_유저가_동시에_1000원_충전을_100번_요청했을때_정상적으로_합산에_성공해야한다() throws Exception {
		// given
		long id = 1L;
		int threadCount = 100;
		long chargeAmount = 1000L;

		ExecutorService executor = Executors.newFixedThreadPool(10); // 스레드풀 10개
		CountDownLatch latch = new CountDownLatch(threadCount); // 요청가능한 스레드개수
		ChargeRequest request = new ChargeRequest(id , chargeAmount);

		// when
		for(int i = 0 ; i < threadCount ; i++) {
			executor.submit(()-> {
				try {
					// 포인트충전 API 호출
					pointService.charge(request);
				} catch (Exception e) {
					e.printStackTrace();
					throw new RuntimeException(e);
				} finally {
					latch.countDown(); // 요청가능 스레드 개수 감소
				}
			});
		}

		latch.await(); // 다 끝날 때까지 대기
		Thread.sleep(1000); // 1초 정도 대기후 최종 포인트 확인

		// then
		long expectedPoint = chargeAmount * threadCount;
		long finalPoint = userPointRepository.findById(id).point();
		assertEquals(expectedPoint, finalPoint, "충전후 포인트값이 예상값("+expectedPoint+")과 실제값("+finalPoint+")이 서로다릅니다.");
	}
}

동시성제어를 할때는 서비스에 대한 통합테스트를 작성하는게 정답인가요?
반면에 동시성제어할때 컨트롤러로 호출하는 통합테스트는 오답인가요?
코치님을 비롯한 리뷰어들은 락을 활용한 통합테스트는 어떻게 생각하시나요? 이 PR을 보고 있는 독자님들의 솔직한 생각이 궁금하며 날카로운 지적을 해주셨으면 좋겠습니다.



[리뷰포인트 3] 서로다른 유저 10명이 동시에 포인트 1000원을 충전했을 때, synchornized와 ReentrantLock의 차이점

synchronized 예약어가 붙어있는 경우는 동시요청 처리를 할때 1개의 처리가 다할때까지 다른 스레드는 대기를 해야되고
반면에 ReentrantLock의 경우에는 synchronized의 단점인 다른 스레드들은 무한대기를 하지 않도록, 공정성부여 등을 synchronized보다 더 많은 옵션을 갖는걸로 학습하면서 알게됐습니다.

멘토링시간때 말씀하신 synchronized와 reentrantLock의 처리방식 차이에 대한 테스트를 작성을 시도해봤으나
실행했을때 두개의 방법 모두 테스트에 통과가 되었고, 실행시간도 약 1초~2초 정도 ReentrantLock이 상대적으로 실행속도가 빠르다는 결과를 도출했는데요. 그러나 synchronized 방식이 공유데이터에 1개의 스레드만 접근이 가능하다는 걸 입증하는 테스트로 나타냈다고 확답을 드리기가 어려울거같습니다. 그래도 여러고민끝에 케이스를 공유드립니다.

public class ChargeConcurrencyServiceTest {

	private PointService pointService;

	private UserPointRepository userPointRepository;

	private PointHistoryRepository pointHistoryRepository;

	private UserPointLockManager userPointLockManager;

	private static final Logger log = LoggerFactory.getLogger(ChargeConcurrencyServiceTest.class);

	@BeforeEach
	void setUp() {
		// 직접 의존성 주입(SpringBootTest / Autowired 을 사용시 자동으로 의존성주입)
		userPointLockManager = new UserPointLockManager();
		pointHistoryRepository = new PointHistoryRepositoryImpl(new PointHistoryTable());
		userPointRepository = new UserPointRepositoryImpl(new UserPointTable());
		pointService = new PointServiceImpl(userPointRepository, pointHistoryRepository, userPointLockManager);

		// UserPoint 의 초기 포인트값을 0 으로한다.
		userPointRepository.save(1L, 0L);
	}
	@Test
	void 서로_다른유저_10명이_동시에_1000원_충전을_요청했을때_정상적으로_합산에_성공한다() throws Exception {
		// given
		int numberOfUsers = 10; // 인원수
		int requestPerUser = 10; // 한사람당 요청개수

		// 포인트 초기화
		for(long id = 1L; id < numberOfUsers ; id++) {
			userPointRepository.save(id, 0L);
		}

		ExecutorService executor = Executors.newFixedThreadPool(5); // 스레드풀: 5개. (요청 5개 동시에 실행가능)
		CountDownLatch latch = new CountDownLatch(numberOfUsers * requestPerUser); // 10 * 10
		long chargeAmount = 1000L;

		// when
		for(long id = 1L; id <= numberOfUsers; id++) {
			ChargeRequest request = new ChargeRequest(id, chargeAmount);

			// 유저 1명당 10번을 요청한다.
			for(int i = 0 ; i < requestPerUser; i++) {
				long uid = id;
				executor.submit(() -> {
					try {
                                                  if(uid == 1L) Thread.sleep(1000); // id=1번 스레드 1초 대기
						pointService.charge(request);
					} catch(Exception e) {
						e.printStackTrace();
						throw new RuntimeException(e);
					} finally {
						latch.countDown();
					}
				});
			}
		}

		latch.await();
		Thread.sleep(1000);

		// then
		log.info("::: 테스트 종료후 유저별 보유포인트 조회 :::");
		for(long id = 1L; id <= numberOfUsers; id++){
			long actualPoint = userPointRepository.findById(id).point();
			log.info("유저 ID {} 의 보유포인트: {}", id, actualPoint);
		}

		for(long id =1L; id <= numberOfUsers; id++) {
			long actualPoint = userPointRepository.findById(id).point();
			assertEquals(requestPerUser * chargeAmount, actualPoint,"UserPoint id "+id+ "인 회원의 포인트는 예상금액과 다릅니다: "+ actualPoint + "원" );
		}
	}
}

다만 특정스레드에게 Thread.sleep(1000) 으로 1초를 sleep시켜준다면 syncrhonized가 reentrantlock보다 14초 가량 더 느렸습니다.

  • reentrantlock
    • 스레드1번에게 10초 슬립: 33초
    • 슬립없음: 31초
  • synchronized:
    • 스레드 1번에게 10초 슬립시킬경우: 47 초
    • 슬립없음: 43초

이번주 KPT 회고

Keep

  • 원래 평상대로라면 약속 이후 귀가하게되면 잠자거나 집중이 저하됐는데, 컴퓨터앞에 앉아서 과제를 수행하고있는 나자신이 놀랍네요ㅎㅎ 반강제성이 있어서 어찌저찌하게됩니다. 10주차동안 이 자세가 끝까지 유지됐으면 좋겠어요! 과제의 요구사항에 대한 나만의 답을 작성해놓고, 여유시간이 확보되면 주제에 대해 깊이 생각해보고 고민해보고 저만의 언어로 이해하되 설명하고 싶습니다.
  • 잠을 4시간정도 적게자는데, 그럼에도 불구하고 집중해서 부족한 지식을 채우고 학습하고 그걸 바탕으로 응용해서 문제 해결하는 과정이 쌓이다보니 개발에 대한 재미를 느꼈습니다. 더 잘하고 싶단 욕심이 생깁니다.

Problem

  • 낮밤이 뒤집혀졌습니다...
  • 시간관리 소홀함
    • 실험을 하고싶었는데 생각보다 결과가 잘 나오지 않았습니다ㅠㅠ 여기에 너무 고민하는데 시간을 많이 썼습니다....

Try

- 리뷰를 위한 PR 탬플릿 마크다운파일 추가
- 포인트 충전 API 내부 비즈니스 로직 작성
- 포인트 사용 API 내부 비즈니스 로직 작성
- 포인트 조회 API 내부 비즈니스 로직 작성
- 포인트 내역조회 API 내부 비즈니스 로직 작성
@loveAlakazam loveAlakazam self-assigned this Mar 23, 2025
@loveAlakazam loveAlakazam linked an issue Mar 23, 2025 that may be closed by this pull request
3 tasks
@loveAlakazam loveAlakazam changed the title Step02 Step02. 심화과제 Mar 23, 2025
- 포인트 충전/사용/조회/내역조회 서비스 로직 UnitTest 케이스작성
- 동일한 사용자가 동시에 포인트충전을 요청할때 정상적으로 처리하도록 테스트케이스 작성.
- 포인트서비스 - 포인트 충전 - 단위테스트 실패케이스
- 포인트서비스 - 포인트 사용 - 단위테스트 실패케이스
- UserPoint에게 충전 객체책임부여
- UserPoint에게 사용 객체책임부여
- 포인트충전량/사용량 으로 값이 잘못저장되는 오해 소지가 있음.
- 변경될 포인트잔액 을 저장해야함.
- UserPoint에게 충전/사용 책임을 부여후 코드 개선
- 단순히 '충전성공' 과 같이 애매하게 하지 않고 목적을 구체화시켜서 테스트네이밍 변경
- 객체의 책임을 활용하여 테스트코드 수정
@loveAlakazam
loveAlakazam merged commit 0212a15 into main Mar 29, 2025
@loveAlakazam

loveAlakazam commented Apr 5, 2025

Copy link
Copy Markdown
Owner Author

[1-2주차 과제 코멘트]

  • 좋은 접근이고 고민인 것 같아요. 개선해보면 좋을 점을 적어볼게요.
  • 비즈니스 로직을 담당하고 있는 서비스가 "어떻게 HTTP 요청/응답할지" 알고 있는 것은 다소 어색해 보입니다. ( Request 를 인자로 받고 Response 를 인자로 받는 것보단 Request 에 검증의 책임은 두고, charge 가 필요한 값을 꽂아주는 게 더 좋을 것 같아요. )

  • 최대 포인트 충전/사용금액 <- 비즈니스 정책인데, 요청에서 검증하기보다 비즈니스 로직에 책임을 맡기는 게 더 좋아보입니다.
    ChargeRequest 에서는 id, amount 가 0보다 큰지 정도 ( 유효성 검증 ) 의 책임을 갖고, UserPoint 는 돈을 충전할 때 결과가 최대금액을 넘지는 않은지 등을 검증하게끔 책임을 분리시키면 좋을 것 같아요.

  • 수십 수백개의 비즈니스 역할을 갖고 있다면, 그렇게 될 수 있습니다. 다만, 그렇지 않도록 모델링하는게 개발자의 역할이 되겠죠 :)

  • 저는 포인트 내역을 "충전, 사용" 이 amount 이고, 만약 필요하다면 result 필드를 확장할 것 같아요 :)
    ( 네이밍을 보았을 때, 수량 이기 때문에 )

  • 네 저는 DB 에 대해서 truncate 를 활용하는 편이고 ( DatabaseCleanUp 이라는 객체를 만들어 활용합니다 ) 항상 깨끗한 환경에서 테스트를 수행하는게 ( 개입 없는 것 ) 이 더 중요하다고 판단하여 활용합니다. 실제 DB 를 활용한다면, 서로 겹치지 않는 데이터를 활용하도록 식별자를 테스트마다 지정하는 것 외엔 없는 것 같네요.

  • 동시성 제어 로직이 제일 가깝게 있는 곳에서 하는 게 가장 좋은 방식인 것 같습니다. ( 현재로 보면 서비스 ) 오답은 아닙니다. 다만, 동시성 제어로직과 멀어져있는 곳에서 테스트를 하게 되면, 동시성 제어로직 이외의 개입들도 생길 수 있기 때문에 명확하게 동시성 제어 주체가 잘 수행하고 있는지 확인하기 어렵습니다.

  • synchronized 와 ReentrantLock 을 활용해보고, 각각의 차이에 따라 적절하게 선정해주신 점 좋았습니다. + 서로 다른 유저간 접근에서 동시에 수행되느냐 안되느냐는 두개의 Executor 를 두고, 실행시간/ 종료시간을 기록한 후에 overlap 되는지 검증하여 자동화할 수도 있습니다.

  • 동시성 관련해서 상세히 적어주셔서 좋았습니다. 다만 "DB" 에 관한 내용은 이번 주차에서 관심사가 아니므로, 불필요한 정보일 것 같아요 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[STEP 02] TDD & 클린아키텍쳐 심화과제

1 participant