[Feature] - 여행기 제목 기준 검색 기능 구현 #302
Conversation
Test Results 27 files 27 suites 7s ⏱️ Results for commit dab43fb. ♻️ This comment has been updated with latest results. |
| // QueryDSL | ||
| implementation 'com.querydsl:querydsl-jpa:5.0.0:jakarta' | ||
| annotationProcessor 'com.querydsl:querydsl-apt:5.0.0:jakarta' | ||
| annotationProcessor "jakarta.annotation:jakarta.annotation-api" | ||
| annotationProcessor "jakarta.persistence:jakarta.persistence-api" | ||
| } |
| @ExceptionHandler(ConstraintViolationException.class) | ||
| public ResponseEntity<ExceptionResponse> handleConstraintViolationException( | ||
| ConstraintViolationException exception | ||
| ) { | ||
| log.warn("CONSTRAINT_VIOLATION_EXCEPTION :: message = {}", exception.getMessage()); | ||
|
|
||
| String message = exception.getConstraintViolations() | ||
| .stream() | ||
| .map(ConstraintViolation::getMessage) | ||
| .findFirst() | ||
| .orElseThrow(); | ||
| ExceptionResponse data = new ExceptionResponse(message); | ||
| return ResponseEntity.badRequest() | ||
| .body(data); | ||
| } |
There was a problem hiding this comment.
jakarta.validation에서 던지는 예외인 듯 하네요? 저희 서비스의 예외로 한번 싸서 던질 수 있으면 좋을 텐데 메서드 핸들러 argument에러 처럼 그럴 수 없는 상황인가보네요.
어떤 예외 잡고있는 것인지 관련해서 어떻게 처리되고 있는지 한 번 설명 듣고 싶은데 구두로든 아니면 코멘트로든 한 번 부탁드려도 될까욥?
There was a problem hiding this comment.
TravelogueController에서 클래스 단위 @Validated가 없다면 Bean validation 시 HandlerMethodValidationException이 발생합니다. 이 예외의 문제점이, bean validation에서 따로 지정해둔 메시지가 전부 Validation failure로 오버라이드 됩니다.
심지어, 400 Bad Request를 500 Internal Server Error로 치환까지 해버리기 때문에 다른 방법이 필요했고, 그러던 중 Baeldung에서 이런 글을 발견하여 @Validated를 클래스 단위에 붙여 메소드 파라미터의 bean validation이 ConstraintViolationException이 발생하도록 했습니다.
There was a problem hiding this comment.
밸덩 확인했습니다. 깔끔한 설명 감사합니다!
근데 밸덩 글이긴 해도 깔끔하지 못한 처리인 것 같긴 하네요 🤔
조금 더 좋은 생각있으면 아래 코멘트에 붙여보겠습니다.
| @RequiredArgsConstructor | ||
| @Validated | ||
| @RestController |
| @NotBlank(message = "검색어는 2글자 이상이어야 합니다.") | ||
| @Size(min = 2, message = "검색어는 2글자 이상이어야 합니다.") |
There was a problem hiding this comment.
Bean Validation 정말 헷갈리네요 (@Valid필요한지 아닌지) 저는 아무리 찾아봐도 동작과 적절한 사용 예시를 못찾겠음요..
어디까지 활용되고 어떻게 사용되는지 코치님들한테 한번 같이 물어보러 가시쥬..
There was a problem hiding this comment.
SearchKeyword에 validation이 많이 붙기도하고 이후에 searchKeyword 여러 개 될 수도 있으니 DTO로 빼고 @Valid 붙여보는건 어떨까용? 될지는 잘 모르겠지만 츄라이츄라이
There was a problem hiding this comment.
설명을 듣고나니 지금의 구현도 나름 합리적이라고 생각해서요 아래에서 제안드리는 내용은 리건이 반영여부를 판단해주시면 감사하겠습니다.
검색어는 2글자 이상이어야 한다를 비즈니스 로직으로 정의SearchConditionDTO 운용SearchCondition이라는 도메인 클래스를 만든다.- 비즈니스 로직을 도메인과 DTO에 적절히 응집
There was a problem hiding this comment.
Dto에서 검증하는 것도 괜찮아 보이네요.
한 번 고민해보시죵 😄
| private final TravelogueRepository travelogueRepository; | ||
| private final AwsS3Provider s3Provider; | ||
| private final TravelogueQueryRepositoryImpl travelogueQueryRepositoryImpl; | ||
|
|
| @Bean | ||
| public TravelogueQueryRepositoryImpl travelogueQueryRepositoryImpl() { | ||
| return new TravelogueQueryRepositoryImpl(jpaQueryFactory()); |
| @TestConfiguration | ||
| public class TestQueryDslConfig { | ||
|
|
||
| @PersistenceContext | ||
| private EntityManager entityManager; | ||
|
|
||
| @Bean | ||
| public JPAQueryFactory jpaQueryFactory() { | ||
| return new JPAQueryFactory(entityManager); | ||
| } | ||
|
|
There was a problem hiding this comment.
QueryDSL을 활용하는 TravelogueQueryRepositoryImpl은 JpaRepository의 구현체가 아니기 때문에 @DataJpaTest에서 빈으로 감지하지도 못하고 심지어 해당 리포지토리를 활용하는 서비스에서 자동 의존성 주입(Autowire) 후보(candidate)로 판단하지도 못합니다. 해결책을 찾던 도중 테스트 config를 작성해서 직접 빈으로 생성하면 자동 주입을 활용할 수 있다고 해서 활용했습니다.
| @NotBlank(message = "검색어는 2글자 이상이어야 합니다.") | ||
| @Size(min = 2, message = "검색어는 2글자 이상이어야 합니다.") |
There was a problem hiding this comment.
SearchKeyword에 validation이 많이 붙기도하고 이후에 searchKeyword 여러 개 될 수도 있으니 DTO로 빼고 @Valid 붙여보는건 어떨까용? 될지는 잘 모르겠지만 츄라이츄라이
| public Page<TravelogueSimpleResponse> findSimpleTraveloguesByKeyword(Pageable pageable, String keyword) { | ||
| Page<Travelogue> travelogues = travelogueService.findByKeyword(keyword, pageable); | ||
|
|
||
| return new PageImpl<>(travelogues.stream() |
There was a problem hiding this comment.
PageImpl이랑 Page<>.map이랑 별 차이 없다면 후자가 좀 더 코드가 깔끔해질 것 같슴다.
| @NotBlank(message = "검색어는 2글자 이상이어야 합니다.") | ||
| @Size(min = 2, message = "검색어는 2글자 이상이어야 합니다.") |
There was a problem hiding this comment.
Dto에서 검증하는 것도 괜찮아 보이네요.
한 번 고민해보시죵 😄
| public class TravelogueQueryRepositoryImpl implements TravelogueQueryRepository { | ||
|
|
||
| private final JPAQueryFactory jpaQueryFactory; | ||
|
|
||
| @Override | ||
| public Page<Travelogue> findByTitleContaining(String keyword, Pageable pageable) { | ||
| return new PageImpl<>(jpaQueryFactory.selectFrom(travelogue) | ||
| .where(Expressions.stringTemplate("replace({0}, ' ', '')", travelogue.title) | ||
| .containsIgnoreCase(keyword.replace(" ", ""))) | ||
| .fetch()); | ||
| } | ||
| } |
There was a problem hiding this comment.
저희가 동적 쿼리는 QueryDsl 사용을 컨벤션으로 정했던 것 같은데 추상화를 한 번 하신 이유는 그럼에도 JPQL이나 Native Query가 필요할 것 같아서일까요?
추상화를 한 근거가 궁금하네욤 🤔
There was a problem hiding this comment.
Repository 계층은 다른 계층으로부터 많이 의존됩니다. 이렇게 다른 계층을 의존하지 않고 의존만 당하는 객체를 구체화에 의존하게 된다면 변경에 매우 취약하다고 생각합니다. QueryDSL 외에 다른 기술이 사용될 여지가 많이 적지만, 그래도 미래를 위해서 대책을 세워두는 편이 낫다고 생각했습니다.
| @@ -59,8 +59,8 @@ void setUp() { | |||
| @Test | |||
| void readTravelogues() { | |||
| // given | |||
| travelogueTestHelper.initTravelogueTestDate(member); | |||
| travelogueTestHelper.initTravelogueTestDate(member); | |||
* feat: 여행기 제목 키워드 기준 검색 기능 구현 * feat: 여행기 검색 기능을 위한 키워드 검증 예외 처리 * feat: QueryDSL 의존성 추가 * refactor: 예외 메시지 추출 로직 변경 * refactor: API 문서 설명 수정 * test: 여행기 제목 키워드 검색 기능 테스트 작성 * feat: 여행기 제목 키워드 기준 검색 기능 구현 * refactor: 추상화에 의존하도록 변경 * refactor: 필드 final 추가 * refactor: DTO 변환 과정 개선 * refactor: 필요 없어진 예외 처리 로직 제거 * refactor: 검색 키워드 request parameter DTO로 분리 * refactor: 검색 메소드 시그니처 리팩토링 * refactor: 여행기 조회 테스트 검증 대상 수정 * chore: 오타 수정 * refactor: pagination 관련 테스트 fixture 수정 * fix: conflict 해결 * refactor: 조회 쿼리에 정렬 및 페이지네이션 정보 추가
✅ 작업 내용
🙈 참고 사항
gradle - clean작업 한번 수행 후,gradle - build해주셔야 정상 작동할 수도 있습니다.