Feature/slot domain by xxx-sj · Pull Request #17 · f-lab-edu/spring-reservation-core

xxx-sj · 2025-12-13T12:13:31Z

1. 작업 개요 (Summary)

Slot entity 생성 및 로직 API 구현
테스트 코드 추가

2. 상세 내용 (What & How)

2-1. 주요 변경 사항

도메인 / 엔티티
- Slot entity 추가
서비스 / 비즈니스 로직
- 슬롯 등록
- 슬롯 조회
- 슬롯 리스트 조회
API / 컨트롤러
- [POST] /api/slots : 슬롯 생성 api endpoint 추가
- [GET] /api/slots/:slotId : 특정 슬롯 조회
- [GET] /api/slots : 슬롯 리스트 조회
기타
- 공통 에러 템플릿 추가
- 비즈니스 에러 추가
- 시작 시간, 종료 시간 validation annotation 추가

2-2. 설계/의도 (Design decisions)

왜 이렇게 설계했는지, 다른 대안은 뭐였는지 간단히 적어 주세요.
도메인 로직 decreaseRemaining() 호출 시 멀티스레드 환경에서 race condition 이 발생한다는걸
test code를 통해 확인했습니다.

2-3. DB / 마이그레이션

CREATE TABLE slots (
    id BIGINT NOT NULL AUTO_INCREMENT,
    
    title VARCHAR(255) NOT NULL,

    start_at DATETIME NOT NULL,
    end_at DATETIME NOT NULL,

    capacity INT NOT NULL,
    remaining INT NOT NULL,

    status VARCHAR(50) NOT NULL,

    created_at DATETIME NOT NULL,
    updated_at DATETIME NOT NULL,

    PRIMARY KEY (id)
);

- slot 도메인 테스트 코드 추가 - slot 레포지토리 테스트 코드 추가 - map을 활용한 테스트.

if-else-f · 2025-12-16T12:07:44Z

src/main/java/com/example/reservation/reservationsystem/api/slot/SlotController.java

+                .fromCurrentRequest()
+                .path("/{id}")
+                .buildAndExpand(slotId)
+                .toUri();


오오 이런 방법은 처음 보는데요
왜 url 을 같이 제공하시나여??

RFC 9110: HTTP 15.3.2. 201 Created 부분에

The 201 (Created) status code indicates that the request has been fulfilled and has resulted in one or more new resources being created. The primary resource created by the request is identified by either a Location header field in the response or, if no Location header field is received, by the target URI.

https://www.rfc-editor.org/rfc/rfc9110.html#section-15.3.2

가 있어서 추가해보았습니다..!

json 응답 데이터 반환으로 변경되어 location 은 제거했습니다 !

if-else-f · 2025-12-16T12:08:08Z

src/main/java/com/example/reservation/reservationsystem/api/slot/SlotController.java

+    }
+
+    @GetMapping("/{slotId}")
+    public ResponseEntity<SlotResponse> getSlot(@PathVariable Long slotId) {


slotId 가 null 이면 어케 되나여??

제 생각에는 null 이라는 값이 들어올 수 없고, 선언한 path variable과는 다른 타입이 요청으로 들어오는 경우 missmatch exception 이 나는걸로 알고 있습니다.

if-else-f · 2025-12-16T12:08:28Z

src/main/java/com/example/reservation/reservationsystem/api/slot/SlotController.java

+    private final SlotService slotService;
+
+    @PostMapping("")
+    public ResponseEntity<Long> createSlot(@Valid @RequestBody SlotCreateRequest request, Locale locale) {


locale 은 어디에 사용하나요??

아래 location 넣으면서 들어간 것 같습니다 제거하겠습니다..!

if-else-f · 2025-12-16T12:09:03Z

src/main/java/com/example/reservation/reservationsystem/api/slot/SlotController.java

+    private final SlotService slotService;
+
+    @PostMapping("")
+    public ResponseEntity<Long> createSlot(@Valid @RequestBody SlotCreateRequest request, Locale locale) {


RestController 인데 왜 ResponseEntity 로 반환하나요??
응답은 json 으로 반환하는게 좋지 않을까요?

HTTP status code도 같이 반환하기 위해서 ResponseEntity에 body를 담아 리턴하도록 하였습니다.
DTO 객체를 그대로 리턴하게 된다면, HTTP status code 200으로만 내려가는걸로 알고있는데요, 그렇다면 따로 HTTP Status code는 사용하지 않는지 궁금합니다 !

dto 객체만 반환하도록 변경했습니다 !

if-else-f · 2025-12-16T12:09:23Z

src/main/java/com/example/reservation/reservationsystem/api/slot/SlotController.java

+
+    @GetMapping("/{slotId}")
+    public ResponseEntity<SlotResponse> getSlot(@PathVariable Long slotId) {
+        return ResponseEntity.ok(slotService.getSlot(slotId));


ResponseEntity 는 이전 PR에서 안쓰는걸로 논의되지 않았나요??

바로 response dto를 반환하도록 변경하였습니다 !

if-else-f · 2025-12-16T12:09:39Z

src/main/java/com/example/reservation/reservationsystem/api/slot/SlotController.java

+    }
+
+    @GetMapping
+    public ResponseEntity<Page<SlotResponse>> getSlots(@PageableDefault(size = 20) Pageable pageable) {


여기도 ResponseEntity 네요

위와 동일하게 변경하였습니다 !

if-else-f · 2025-12-16T12:10:12Z

src/main/java/com/example/reservation/reservationsystem/application/slot/SlotService.java

+                .build();
+
+        Slot savedSlot = slotRepository.save(slot);
+        return savedSlot.getId();


slotRepository.save(slot).getId();

이면 불필요한 변수를 제거할 수 있지 않을까요?

수정했습니다 !

if-else-f · 2025-12-16T12:10:42Z

src/main/java/com/example/reservation/reservationsystem/application/slot/SlotService.java

+                .startAt(request.startAt())
+                .endAt(request.endAt())
+                .capacity(request.capacity())
+                .remaining(request.capacity())


구현이 너무 외부로 드러나는것 같아요

Slot.of(request)

와 같이 작성하는게 OCP 원칙에도 더 알맞지 않을까요??

Slot domain 내부에 of static 메서드를 추가하여 수정하였습니다

if-else-f · 2025-12-16T12:12:05Z

src/main/java/com/example/reservation/reservationsystem/application/slot/SlotService.java

+    }
+
+    public Page<SlotResponse> getSlots(Pageable pageable) {
+        return slotRepository.findAll(pageable)


페이지를 사용할 경우 page의 숫자가 충분히 큰 값이면 DB에 어떤 문제가 발생할까요?

이와 같이 되면 created_at asc (정확히는 pk asc) 로 정렬 될 텐데 의도하신 정렬이 맞을까요?

page(offset), limit 를 사용한 페이지네이션 같은경우 DB에서 page만큼의 데이터를 읽고 나서 그 다음 limit 수 만큼을 읽은 후 앞에 읽은 page 의 데이터는 제외하고 응답하는 방식이라 read 비용이 증가하게 됩니다. 또한 조건 없는 조회같은경우 count에 대한 비용도 포함되게 됩니다.

이 부분에 있어서 전통적인 페이지네이션 화면을 구현하기 위해서는 윗 방법을 사용할 듯 한데요,
만약 row의 양이 많고 db의 데이터가 샤딩되지않은 상태라면 count 값을 배치를 통해 캐싱을 해둘 것 같습니다.
또는 전통적인 페이지네이션 화면 구성이 필요없다면, cursor를 통해 다음 조회 데이터가 있고 없고만을 판단하거나, slice를 통해 구현방법을 변경하는 방법도 있을 것 같습니다.

이 부분은 제가 놓친 것 같습니다 created_at desc, id desc 정렬 조건을 추가하겠습니다 !
정렬 같은 경우, 아직 정해놓은 부분이 없어 기본적으로 최신 순서 그 다음 아이디로 정렬 예정입니다

if-else-f · 2025-12-16T12:12:37Z

...n/java/com/example/reservation/reservationsystem/application/slot/dto/SlotCreateRequest.java

+
+        @NotNull(message = "종료 시간은 필수입니다.")
+        @Future(message = "종료 시간은 현재 시간 이후여야 합니다.")
+        LocalDateTime endAt,


endAt <= startAt

케이스는 어떻게 대응하나요?

ValidTimeRange 어노테이션 내부 구현에서 처리하고 있습니다

if-else-f · 2025-12-16T12:13:23Z

...om/example/reservation/reservationsystem/common/validation/timerange/TimeRangeValidator.java

+        if (value.startAt() == null || value.endAt() == null)
+            return true; // pass to not null validation
+
+        if (!value.endAt().isAfter(value.startAt())) {


여기서 대응하는군요

if-else-f · 2025-12-16T12:15:48Z

src/main/java/com/example/reservation/reservationsystem/domain/slot/Slot.java

+     * remaining 증가 (예약 취소 시)
+     * - 처음 설정한 capacity를 초과하면 예외 발생
+     */
+    public void increaseRemaining() {


원자적 설정이 필요해 보입니다.
만약 임의의 slot id = s0에 대해서 임의의 스레드 t0가 88라인에 접근하고, t1이 85를 막 통과한다면
write skew가 발생할 것 같습니다.

db 원자적 업데이트로 구현을 변경하였습니다. 위에 도메인 메서드인 decreaseRemaining 메서드도 원자적 업데이트로 변경하였으며, 두 메서드 테스트 코드는 삭제하였습니다 !

slotService.cancelSlotReservation();
slotService.reserveSlot();

if-else-f · 2025-12-16T12:16:42Z

src/main/java/com/example/reservation/reservationsystem/domain/slot/SlotStatus.java

+package com.example.reservation.reservationsystem.domain.slot;
+
+public enum SlotStatus {
+    OPEN,


OPEN("OPEN에 대한 설명"), CLOSED("CLOSED에 대한 설명"),

과 같이 변경되면 좋겠습니다.

if-else-f · 2025-12-16T12:17:21Z

src/main/java/com/example/reservation/reservationsystem/global/config/JpaConfig.java

+
+@Configuration
+@EnableJpaAuditing
+public class JpaConfig {


따로 JPA 설정이 없다면 @EnableJpaAuditing 는 스프링 application 에 들어가는게 좋지 않을까요?

ReservationSystemApplication으로 통합했습니다 !

통합하려 했으나, @WebMvcTest 테스트 관련해서 오류가 발생하고 있어 분리했습니다.

webMvcTest 어노테이션을 사용하여 테스트하면 controller layer 관련 빈만 올라오게 되면서 entity가 올라오지 않은 상황에서 enableJpaAuditing 어노테이션이 JPA 메타모델 을 필요로 하게 되면서 테스트에 오류가 발생하게 됩니다.

if-else-f · 2025-12-16T12:18:04Z

...main/java/com/example/reservation/reservationsystem/global/error/GlobalExceptionHandler.java

+    /**
+     * 도메인 오류 이외의 서버 오류
+     */
+    @ExceptionHandler(Exception.class)


우리는 checked exception과 unchecked exception에 대해 학습했습니다.
이 코드는 둘을 분리하지 않으니 둘을 분리하는게 좋지 않을까요??

기존 코드를 unchecked 로 변경하고, checked 관련하여 메서드를 추가하였습니다

if-else-f · 2025-12-16T12:18:30Z

...java/com/example/reservation/reservationsystem/global/error/exception/BusinessException.java

+@Getter
+public class BusinessException extends RuntimeException {
+
+    private final ErrorCode errorCode;


소나큐브의 메시지를 확인해주세요.

ErrorCode interface 에 Serializable 추가하였습니다

if-else-f · 2025-12-16T12:19:01Z

...main/java/com/example/reservation/reservationsystem/global/error/response/ErrorResponse.java

+                            error.getField(),
+                            error.getRejectedValue() == null ? "" : error.getRejectedValue().toString(),
+                            error.getDefaultMessage()))
+                    .collect(Collectors.toList());


소나큐브 메시지를 확인해주세요.

응답 이후 리스트는 수정되지 않아야 하므로 기존 .collect(Collectors.toList()); 에서 stream().toList() 로 변경하였습니다.

if-else-f · 2025-12-16T12:19:31Z

...n/java/com/example/reservation/reservationsystem/infrastructure/slot/SlotRepositoryImpl.java

+
+@Repository
+@RequiredArgsConstructor
+public class SlotRepositoryImpl implements SlotRepository {


그냥 jpa repository를 쓰면 될것같은데 왜 따로 구현체를 만드신건가요??

test에서 db 없이 메모리로 테스트 하기 위해 slotRepository interface를 정의했으나, 로컬 테스트 때 h2DB를 사용하게 된다면 불 필요한 interface라고 생각합니다. 삭제하고 service 에서 jpaRepository를 바로 사용하도록 수정하겠습니다.

if-else-f · 2025-12-16T12:20:11Z

src/main/resources/application.yml

+      on-profile: dev
+
+  datasource:
+    url: jdbc:mysql://${DB_HOST:localhost}:${DB_PORT:3306}/reservation_dev?serverTimezone=Asia/Seoul&characterEncoding=UTF-8


mysql로 변경하셨나요?

if-else-f · 2025-12-16T12:20:55Z

src/test/java/com/example/reservation/reservationsystem/api/slot/SlotControllerTest.java

+        @MockitoBean
+        private SlotService slotService;
+
+        private final String TEST_NAME = "test slot";


이건 소나큐브 메시지를 무시하는게 좋겠네요

if-else-f · 2025-12-16T12:21:33Z

src/test/java/com/example/reservation/reservationsystem/api/slot/SlotControllerTest.java

+                                .content(objectMapper.writeValueAsString(request)))
+                                .andDo(print())
+                                .andExpect(status().isCreated())
+                                .andExpect(jsonPath("$").value(1L));


이건 무엇을 테스트하는 건가요??
mock 을 하고 mock 의 결과를 왜 확인하나요??

create slot endpoint 를 테스트 하는 것이 목표였고, 테스트 하고자 한 항목은 요청으로 받은 request를 slotService.createSlot() 를 호출하여 저장하고 id를 반환하는 것을 목킹하고 controller에서 반환이 잘 되는지를 테스트하고자 했습니다 ..!

if-else-f · 2025-12-16T12:22:30Z

src/test/java/com/example/reservation/reservationsystem/api/slot/SlotControllerTest.java

+        }
+
+        @Test
+        @DisplayName("Slot 생성 실패")


"Slot 생성 실패시 G002 에러를 반환한다"
와 같이 display name을 정의하시면 더 좋지 않을까요?

수정했습니다 !

if-else-f · 2025-12-16T12:23:11Z

src/test/java/com/example/reservation/reservationsystem/api/slot/SlotControllerTest.java

+        }
+
+        @Test
+        @DisplayName("Slot 단건 조회")


"단건 조회시 TEST_NAME을 반환한다" 와 같이 수정하면 더 좋지 않을까요?

네 수정하였습니다 !

if-else-f · 2025-12-16T12:23:24Z

src/test/java/com/example/reservation/reservationsystem/api/slot/SlotControllerTest.java

+
+        @Test
+        @DisplayName("Slot 생성 실패")
+        void createSlot_ValidationError() throws Exception {


throws Exception 이건 왜 붙은건가요??

mockMvc.perform() 에서 throws Exception 을 하고 있습니다 ..!

if-else-f · 2025-12-16T12:24:06Z

src/test/java/com/example/reservation/reservationsystem/api/slot/SlotControllerTest.java

+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
+@WebMvcTest(SlotController.class)
+class SlotControllerTest {


상당한 범위의 통합 테스트를 작성하셨는데 어떠한 것을 중점으로 생각하셧나요?

컨트롤러의 validation, dto 바인딩, 전역 예외 처리와 에러 응답 포맷 테스트에 중점을 두었습니다.
service는 mock을 통해 테스트 코드 작성하였습니다

if-else-f · 2025-12-16T12:25:00Z

src/test/java/com/example/reservation/reservationsystem/application/slot/SlotServiceTest.java

+    private SlotService slotService;
+    private SlotRepository slotRepository;
+
+    @BeforeEach


beforeEach는 테스트 마다 실행합니다.
의도하신 내용이 맞을까요?

매 테스트마다 builder를 통해 Slot을 저장하고 있어서 매번 테스트 실행 전에 새로운 메모리디비를 사용하려는 의도였습니다

동시성 관련 테스트는 따로 SlotServiceConcurrencyTest로 만들었습니다

if-else-f · 2025-12-16T12:25:20Z

src/test/java/com/example/reservation/reservationsystem/application/slot/SlotServiceTest.java

+
+    @BeforeEach
+    void setUp() {
+        slotRepository = new SlotMemoryRepository(new ConcurrentHashMap<>());


어차피 h2를 사용하니 직접 jpa 를 사용해서 insert, select를 확인해보시는건 어떠신가요??

넵 h2를 이용한 테스트로 변경하겠습니다 !

if-else-f · 2025-12-16T12:26:44Z

src/test/java/com/example/reservation/reservationsystem/api/slot/SlotControllerTest.java

+
+        @Test
+        @DisplayName("Slot 생성")
+        void createSlot() throws Exception {


왜 throws Exception 가 붙은건가요?

mockMvc.perform() 에서 throws Exception 을 하고 있습니다 ..!

if-else-f · 2025-12-16T12:27:48Z

...java/com/example/reservation/reservationsystem/infrastructure/slot/SlotMemoryRepository.java

+import org.springframework.data.domain.Pageable;
+
+public class SlotMemoryRepository implements SlotRepository {
+    private final Map<Long, Slot> store;


이렇게 정의하시면 테스트에만 사용할 SlotMemoryRepository 가 운영 코드에서 반영됩니다.
그렇다면 이 컨텍스트를 모르는 개발자가 충분히 사용하여 장애를 일으킬 수 있습니다.
어떻게 해결해야 할까요?

해당 클래스는 현재 test 패키지 하위에 구현되어있는 repository로 실제 운영 코드인 src 하위 패키지 클래스에 import로 들어가지 못하는걸로 알고있습니다. 만약 해당 클래스가 src 하위에 구현되어 있다면
@profiles("test")를 통해 테스트 시에만 사용가능한 빈으로 정의할 것 같습니다 !

해당 클래스는 메모리 테스트 하기 위해 사용했었는데요, h2DB를 사용하게 됨으로서 필요하지 않는 클래스라 판단되어 삭제 하도록 하겠습니다 ..!

sonarqubecloud · 2025-12-19T02:05:34Z

Quality Gate passed

Issues
5 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

xxx-sj and others added 4 commits December 8, 2025 02:36

feat: slot domain 추가 (#1)

b4d6cbe

test: slot 도메인 테스트 코드 추가 (#1)

19245e4

- slot 도메인 테스트 코드 추가 - slot 레포지토리 테스트 코드 추가 - map을 활용한 테스트.

feat: slot service, repository 추가 (#2)

896f26d

feat: slot api 개발 (#3)

84e9985

xxx-sj requested a review from if-else-f December 14, 2025 18:10

if-else-f reviewed Dec 16, 2025

View reviewed changes

fix: 코드리뷰 수정사항 적용

28498dd

xxx-sj requested a review from if-else-f December 18, 2025 18:13

fix: 소나큐브 오류 수정

61578df

Conversation

xxx-sj commented Dec 13, 2025

1. 작업 개요 (Summary)

2. 상세 내용 (What & How)

2-1. 주요 변경 사항

2-2. 설계/의도 (Design decisions)

2-3. DB / 마이그레이션

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xxx-sj Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

xxx-sj Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

xxx-sj Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xxx-sj Dec 17, 2025 •

edited

Loading

xxx-sj Dec 17, 2025 •

edited

Loading

xxx-sj Dec 17, 2025 •

edited

Loading

if-else-f Dec 16, 2025 •

edited

Loading