-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feat] 관리자 이벤트 조회 기능 구현 (#32) #36
Conversation
현재 프로젝트는 이벤트 수정 - 이벤트 상세에서 사용하는 데이터가 거의 동일하므로, 굳이 상세 이벤트 / 수정 이벤트 객체로 구분할 필요가 없다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생많으셨습니다:)
@Operation(summary = "이벤트 리스트 획득", description = "관리자가 이벤트 목록을 검색한다. 검색어, sort 기준 등을 정의할 수 있다.", responses = { | ||
@ApiResponse(responseCode = "200", description = "성공적으로 이벤트 목록을 반환한다"), | ||
@ApiResponse(responseCode = "5xx", description = "서버 내부적 에러"), | ||
@ApiResponse(responseCode = "4xx", description = "클라이언트 에러 (보통 page / size 값을 잘못 지정. 숫자가 아닌 경우 등) ") | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추후에 응답 객체까지 지정해주시면 감사하겠습니다:)
/** | ||
* Fcfs 이벤트의 id. 서버 db 측에서 사용하기 위한 값 | ||
*/ | ||
private Long id; | ||
|
||
/** | ||
* 시작 시간 | ||
*/ | ||
@NotNull | ||
private LocalDateTime startTime; | ||
|
||
/** | ||
* 종료 시간 | ||
*/ | ||
@NotNull | ||
private LocalDateTime endTime; | ||
|
||
/** | ||
* 당첨 인원 | ||
*/ | ||
@NotNull | ||
private Long participantCount; | ||
|
||
/** | ||
* 상품 관련된 정보를 저장하는 영역 | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필드와 메서드마다 꼼꼼한 주석처리 저도 본받아야겠네요.
emRepo.save(em1); | ||
emRepo.save(em2); | ||
emRepo.save(em3); | ||
emRepo.save(em4); | ||
emRepo.save(em5); | ||
emRepo.save(em6); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List로 만든 다음에 saveAll() 해도 괜찮을 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 것 같습니다. 이런 부분도 나중에 수정해보겠습니다.
@@ -23,6 +25,9 @@ | |||
import static org.mockito.ArgumentMatchers.anyString; | |||
import static org.mockito.Mockito.*; | |||
|
|||
|
|||
@DataJpaTest | |||
@TestPropertySource(locations = "classpath:application-test.yml") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
찾아보니까 Spring 자체의 Test Container가 있어서, 추후 필수 요구사항 다 끝난 뒤 적용해 보면 좋을 듯 합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redis 테스트의 경우 test container을 이용하는 것이 환경을 안타고 테스트할 수 있을 것 같더라고요. mocking으로 한계가 있다면 redis도 test container을 도입해봐야 할 것 같습니다.
@@ -15,6 +16,9 @@ public ObjectMapper objectMapper() { | |||
objectMapper.registerModule(new JavaTimeModule()); | |||
// timestamp를 문자열로 전달 | |||
objectMapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS); | |||
// serialization 시 값 없는 필드 = null을 노출하지 않도록 제외. 문제가 되는 경우 구체적인 dto로 이동할 예정. | |||
// 참고: https://www.baeldung.com/jackson-ignore-null-fields | |||
objectMapper.setSerializationInclusion(JsonInclude.Include.NON_NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 EventDto는 이벤트 종류에 따라 다른 객체를 추가적으로 포함하는 형태로 구현되어 있습니다. 예를 들어 선착순 이벤트라면 fcfs를, 추첨 이벤트라면 draw 객체를 포함합니다. 이때, EventDto를 보낼 때 지정되지 않은 필드가 null로 표현되어 클라이언트 측에서는 필요 없는 필드가 null로 지정되어 페이로드를 분석하기 불편한 상황입니다. 따라서 지정되지 않은 필드를 직렬화 과정에서 생략하도록 만들어 클라이언트 입장에서 필요 없는 필드를 보지 않게 만들었습니다.
// 검색 기능 관련 상수들 | ||
public static final int EVENT_DEFAULT_PAGE = 0; | ||
public static final int EVENT_DEFAULT_SIZE = 5; | ||
public static final Set<String> sortableFields = Set.of("eventId", "name", "startTime", "endTime", "eventType"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이벤트와 관련된 상수를 분리하여 여러 곳에서 사용되더라도 한번에 수정할 수 있도록 했습니다.
import java.util.Map; | ||
|
||
public class EventSearchQueryParser { | ||
public static Map<String, String> parse(String searchQuery) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 프로젝트에서 이벤트 검색 시 정렬 쿼리는 field1:(asc|desc)?,field(asc|desc) 형식으로 표현되고 있습니다. 이때, 정렬 쿼리가 프로젝트 내 다른 api에서도 사용될 수 있다고 생각하여 별도의 클래스로 분리했습니다. 정렬 이외의 기능에도 사용할 수 있도록 asc / desc를 직접 검사하는 대신 순수하게 key - value 분리하여 제공하도록 구현했습니다.
@@ -19,7 +19,7 @@ public class EventFrame { | |||
@GeneratedValue(strategy = GenerationType.IDENTITY) | |||
private Long id; | |||
|
|||
@Column | |||
@Column(unique = true, nullable = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동일한 이름의 이벤트 프레임이 생성되는 문제가 있어 name을 유니크 필드로 지정했습니다.
import jakarta.persistence.criteria.Predicate; | ||
import org.springframework.data.jpa.domain.Specification; | ||
|
||
public class EventSpecification { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spring jpa specification을 이용하여 동적 쿼리를 구현하기 위해 Specification 코드를 작성했습니다. eventId 검색 부분은 댓글 검색 기능에도 거의 동일한 로직으로 사용되므로, 추후 서로 다른 Specification으로 분리하는 것이 나을 것 같네요.
orders.add(Sort.Order.desc(field)); | ||
break; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sort를 만드는 객체 자체를 만드는 것도 좋을 것 같다는 생각이 드네요. 나중에 분리해서 리팩토링해봐야겠습니다.
@Builder | ||
@NoArgsConstructor | ||
@AllArgsConstructor | ||
@Getter | ||
public class EventDto { | ||
@NotNull(groups = {EventEditGroup.class}) | ||
Long id; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어느 경우에도 EventDto가 클라이언트 측에서 사용될 일이 없으므로 제거했습니다. 현재 생성되고 있는 eventId 자체가 redis에 의해 요일 순서대로 구현되기 때문에, 클러스터링 인덱스에서 정렬 순서가 바뀔 가능성이 드물다고 생각합니다. 이런 측면에서 생각해볼 때 아예 eventId 자체를 primary key로 만드는 것도 고민해봐야 할 것 같네요
@NotNull | ||
private EventType eventType; | ||
|
||
/** | ||
* 이벤트에 대한 태그 ex) 2024 현대 여름 이벤트 | ||
* 이벤트 프레임 정보. 추후 변경될 수 있음. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
프레임 부분은 추후 협의가 필요할 것 같습니다. 현재 어떤 부분에서는 id 값으로, 어떤 부분은 name 부분으로 사용되고 있어서, 이걸 프로젝트 전반에서 통합할 필요가 있다고 생각합니다.
#️⃣ 연관 이슈
📝 작업 내용
상세 검색 기능 구현
상세 검색 기능을 구현할 때 초기에는 @ Query 어노테이션으로 구성하고자 했습니다. 하지만, @ Query로는 검색어 유무에 따른 동적 쿼리를 처리하기 어려워 동적 쿼리를 처리할 수 있는 다른 방안을 찾게 되었습니다.
현재 검색어(search) 가 존재할 때는 db에서 필터링을 통해 검색하고, 존재하지 않을 때는 필터링을 처리하지 않는 로직을 수행하고 있습니다. 페이징 기능은 최대한 JPA의 Pageable을 기반으로 처리하는 것을 원했습니다.
1, 2번의 경우 JPA이외의 별도 라이브러리를 설치할 필요가 없습니다. 현재 프로젝트 기획 수준에서는 동적 쿼리가 매우 복잡해질 가능성이 높지 않고, queryDSL을 도입하더라도 현재 api 정도에만 적용될 가능성이 높습니다. 나중에 동적 쿼리 자체가 많아진다면 queryDSL 등을 도입하는 것이 좋겠지만, 현재 시점에서는 JPA 자체 기능으로 충분할 것이라고 생각했습니다.
1번의 경우 별도 라이브러리는 필요 없지만 보일러 플레이트 코드가 많습니다. 개발이 불편합니다.
3번을 선택하는 경우 Pageable 객체를 파싱하여 직접 페이징 기능을 구현해야 합니다. 또한, JPA 영속성이 동작하지 않는 것으로 알고 있어 다른 JPA 코드와 유연하게 연결되지 않을 수 있습니다.
2번은 유일하게 Pageable 객체를 그대로 사용할 수 있습니다. 또한 동적인 where 절을 추가할 수 있습니다. 현재 프로젝트에서 요구하는 동적 쿼리 수준은 like 기반 where 절의 등록 / 제거 정도이므로, 별도 라이브러리를 도입하지 않고 간단한 동적 쿼리를 처리할 수 있는 jpa specification이 적합하다고 생각했습니다.
사용법
JpaSpecificationExecutor을 상속합니다.
Specification을 구현합니다.
사용 코드
상세 페이지 보기 기능
상세 페이지와 수정 페이지에서 요구하는 데이터가 완전히 일치하므로, 따로 구현하는 대신 api 경로를 변경하여 함께 사용할 수 있도록 수정했습니다.