-
Notifications
You must be signed in to change notification settings - Fork 8
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
[BE] feat: 축제 조회 필터링 구현(#602) #603
Changes from 7 commits
65cd018
a6d4d8e
c8fe2ba
de59bb0
415a1b7
62e3998
7596ca0
0bead6a
5840f26
9bb33fc
9ff03dd
3b3aa77
91ae246
858672a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,40 @@ | ||||||||||||||||||||||||||||||||
package com.festago.festival.repository; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
import static com.festago.festival.repository.FestivalSpecification.afterEndDate; | ||||||||||||||||||||||||||||||||
import static com.festago.festival.repository.FestivalSpecification.afterStartDate; | ||||||||||||||||||||||||||||||||
import static com.festago.festival.repository.FestivalSpecification.all; | ||||||||||||||||||||||||||||||||
import static com.festago.festival.repository.FestivalSpecification.beforeEndDate; | ||||||||||||||||||||||||||||||||
import static com.festago.festival.repository.FestivalSpecification.beforeStartDate; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
import com.festago.common.exception.BadRequestException; | ||||||||||||||||||||||||||||||||
import com.festago.common.exception.ErrorCode; | ||||||||||||||||||||||||||||||||
import com.festago.festival.domain.Festival; | ||||||||||||||||||||||||||||||||
import java.time.LocalDate; | ||||||||||||||||||||||||||||||||
import java.util.Arrays; | ||||||||||||||||||||||||||||||||
import java.util.function.Function; | ||||||||||||||||||||||||||||||||
import org.springframework.data.jpa.domain.Specification; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
public enum FestivalFilter { | ||||||||||||||||||||||||||||||||
ALL(currentTime -> all()), | ||||||||||||||||||||||||||||||||
PROGRESS(currentTime -> afterStartDate(currentTime) | ||||||||||||||||||||||||||||||||
.and(beforeEndDate(currentTime))), | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 진행중인 축제의 스펙이 다음과 같이 되어있네요. public static Specification<Festival> afterStartDate(LocalDate currentTime) {
return (root, query, criteriaBuilder) -> criteriaBuilder.lessThan(root.get(START_DATE), currentTime);
}
public static Specification<Festival> beforeEndDate(LocalDate currentTime) {
return (root, query, criteriaBuilder) -> criteriaBuilder.greaterThan(root.get(END_DATE), currentTime);
} 이때 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
글렌이 말씀해준것 처럼 이 테스트 코드들이 통과하기 위해선 eqaulsTo 조건이 들어가야겠네요 !! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 헉 수정했습니다 |
||||||||||||||||||||||||||||||||
PLANNED(currentTime -> beforeStartDate(currentTime)), | ||||||||||||||||||||||||||||||||
END(currentTime -> afterEndDate(currentTime)); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enum에서 스펙을 조합하여 조건을 맞추기 보단,
Suggested change
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class FestivalSpecification {
private static final String START_DATE = "startDate";
private static final String END_DATE = "endDate";
public static Specification<Festival> all(LocalDate currentTime) { // 메서드 참조를 위해 파라미터 추가
return (root, query, criteriaBuilder) -> criteriaBuilder.conjunction();
}
public static Specification<Festival> progress(LocalDate currentTime) {
return (root, query, criteriaBuilder) -> criteriaBuilder.and(
criteriaBuilder.lessThan(root.get(START_DATE), currentTime), // lessThanOrEqualTo 고려할 것
criteriaBuilder.greaterThan(root.get(END_DATE), currentTime) // greaterThanOrEqualTo 고려할 것
);
}
public static Specification<Festival> beforeStartDate(LocalDate currentTime) {
return (root, query, criteriaBuilder) -> criteriaBuilder.greaterThan(root.get(START_DATE), currentTime);
}
public static Specification<Festival> afterEndDate(LocalDate currentTime) {
return (root, query, criteriaBuilder) -> criteriaBuilder.lessThan(root.get(END_DATE), currentTime);
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 글렌이 말씀하신 부분에 동의합니다. Specification method가 Progress 같은 경우 외부에서 and 조건으로 조합해주는 형식이 아니라 명확하게 완성된 조건을 뱉어주는게 더 이해하기 쉽고, 테스트하기 좋은 코드라고 여겨져요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반영했습니다! |
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
private final Function<LocalDate, Specification<Festival>> filter; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
FestivalFilter(Function<LocalDate, Specification<Festival>> filter) { | ||||||||||||||||||||||||||||||||
this.filter = filter; | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
Comment on lines
+17
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이녀석은 왜 접근제어자가 없나요?! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enum 생성자 접근제어자에 뭘 썼는지 기억이 안나요 이걸 따로 명시해줬나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enum은 접근제어자가 필요하지 않습니다..! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아이고 이놈 enum이었네 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 라임 디졌다 ;; |
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
public static FestivalFilter from(String filterName) { | ||||||||||||||||||||||||||||||||
return Arrays.stream(FestivalFilter.values()) | ||||||||||||||||||||||||||||||||
.filter(festivalFilter -> festivalFilter.name().equalsIgnoreCase(filterName)) | ||||||||||||||||||||||||||||||||
.findAny() | ||||||||||||||||||||||||||||||||
.orElseThrow(() -> new BadRequestException(ErrorCode.INVALID_FESTIVAL_FILTER)); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 크게 고려해야 할 상황은 아니지만...
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 연산마다 Stream 이 생성되는 것이 비효율적이라 생각했지만 |
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
public Specification<Festival> getSpecification() { | ||||||||||||||||||||||||||||||||
return filter.apply(LocalDate.now()); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,34 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
package com.festago.festival.repository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.festago.festival.domain.Festival; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.time.LocalDate; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import lombok.AccessLevel; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import lombok.NoArgsConstructor; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import org.springframework.data.jpa.domain.Specification; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@NoArgsConstructor(access = AccessLevel.PRIVATE) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public class FestivalSpecification { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private static final String START_DATE = "startDate"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private static final String END_DATE = "endDate"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public static Specification<Festival> all() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return (root, query, criteriaBuilder) -> criteriaBuilder.conjunction(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public static Specification<Festival> afterStartDate(LocalDate currentTime) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return (root, query, criteriaBuilder) -> criteriaBuilder.lessThan(root.get(START_DATE), currentTime); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public static Specification<Festival> beforeStartDate(LocalDate currentTime) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return (root, query, criteriaBuilder) -> criteriaBuilder.greaterThan(root.get(START_DATE), currentTime); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public static Specification<Festival> afterEndDate(LocalDate currentTime) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return (root, query, criteriaBuilder) -> criteriaBuilder.lessThan(root.get(END_DATE), currentTime); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public static Specification<Festival> beforeEndDate(LocalDate currentTime) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return (root, query, criteriaBuilder) -> criteriaBuilder.greaterThan(root.get(END_DATE), currentTime); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 위의 커멘트에도 남기긴 했지만...
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,13 +3,15 @@ | |
import com.festago.festival.application.FestivalService; | ||
import com.festago.festival.dto.FestivalDetailResponse; | ||
import com.festago.festival.dto.FestivalsResponse; | ||
import com.festago.festival.repository.FestivalFilter; | ||
import io.swagger.v3.oas.annotations.Operation; | ||
import io.swagger.v3.oas.annotations.tags.Tag; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RequestParam; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
||
@RestController | ||
|
@@ -21,9 +23,10 @@ public class FestivalController { | |
private final FestivalService festivalService; | ||
|
||
@GetMapping | ||
@Operation(description = "모든 축제들을 조회한다.", summary = "축제 목록 조회") | ||
public ResponseEntity<FestivalsResponse> findAll() { | ||
FestivalsResponse response = festivalService.findAll(); | ||
@Operation(description = "축제를 조건별로 조회한다. ALL: 전체, PROGRESS: 진행 중, PLANNED: 진행 예정, END: 종료", summary = "축제 목록 조회") | ||
public ResponseEntity<FestivalsResponse> findFestivals( | ||
@RequestParam(defaultValue = "ALL") String festivalFilter) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 카톡에서 이야기 했던대로 전체가 없어진다면 default는 PROGRESS로 처리하는게 맞죠?_? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네네 맞습니다! |
||
FestivalsResponse response = festivalService.findFestivals(FestivalFilter.from(festivalFilter)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. default ALL 👍 |
||
return ResponseEntity.ok() | ||
.body(response); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package com.festago.festival.domain; | ||
|
||
import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
||
import com.festago.common.exception.BadRequestException; | ||
import com.festago.festival.repository.FestivalFilter; | ||
import org.junit.jupiter.api.DisplayNameGeneration; | ||
import org.junit.jupiter.api.DisplayNameGenerator.ReplaceUnderscores; | ||
import org.junit.jupiter.api.Test; | ||
|
||
@DisplayNameGeneration(ReplaceUnderscores.class) | ||
@SuppressWarnings("NonAsciiCharacters") | ||
class FestivalFilterTest { | ||
|
||
@Test | ||
void 유요하지_않은_name_이면_예외() { | ||
// given && when && then | ||
assertThatThrownBy(() -> FestivalFilter.from("unvalid")) | ||
.isInstanceOf(BadRequestException.class); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
package com.festago.festival.repository; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.assertj.core.api.SoftAssertions.assertSoftly; | ||
|
||
import com.festago.festival.domain.Festival; | ||
import com.festago.school.domain.School; | ||
import com.festago.school.repository.SchoolRepository; | ||
import java.time.LocalDate; | ||
import java.util.List; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.DisplayNameGeneration; | ||
import org.junit.jupiter.api.DisplayNameGenerator.ReplaceUnderscores; | ||
import org.junit.jupiter.api.Test; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; | ||
|
||
@DisplayNameGeneration(ReplaceUnderscores.class) | ||
@SuppressWarnings("NonAsciiCharacters") | ||
@DataJpaTest | ||
class FestivalRepositoryTest { | ||
|
||
private static final String CURRENT_FESTIVAL = "현재 축제"; | ||
private static final String PAST_FESTIVAL = "과거 축제"; | ||
private static final String FUTURE_FESTIVAL = "미래 축제"; | ||
|
||
@Autowired | ||
FestivalRepository festivalRepository; | ||
|
||
@Autowired | ||
SchoolRepository schoolRepository; | ||
|
||
@BeforeEach | ||
public void setting() { | ||
School school = schoolRepository.save(new School("domain", "name")); | ||
LocalDate now = LocalDate.now(); | ||
festivalRepository.save(new Festival(PAST_FESTIVAL, now.minusDays(10L), now.minusDays(8L), school)); | ||
festivalRepository.save(new Festival(CURRENT_FESTIVAL, now.minusDays(1L), now.plusDays(1L), school)); | ||
festivalRepository.save(new Festival(FUTURE_FESTIVAL, now.plusDays(1L), now.plusDays(3L), school)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 시간에 대한 테스트는 "2023-11-10" 과 같이 고정된 값을 사용하는 것은 어떤가요? |
||
} | ||
|
||
@Test | ||
void 모든_축제_반환() { | ||
// given | ||
FestivalFilter filter = FestivalFilter.ALL; | ||
|
||
// when | ||
List<Festival> actual = festivalRepository.findAll(filter.getSpecification()); | ||
|
||
// then | ||
assertThat(actual).hasSize(3); | ||
} | ||
|
||
@Test | ||
void 진행_예정_축제_반환() { | ||
// given | ||
FestivalFilter filter = FestivalFilter.PLANNED; | ||
|
||
// when | ||
List<Festival> actual = festivalRepository.findAll(filter.getSpecification()); | ||
|
||
// then | ||
assertSoftly(softAssertions -> { | ||
assertThat(actual).hasSize(1); | ||
assertThat(actual.get(0)).matches(festival -> festival.getName().equals(FUTURE_FESTIVAL)); | ||
}); | ||
} | ||
|
||
@Test | ||
void 진행_중_축제_반환() { | ||
// given | ||
FestivalFilter filter = FestivalFilter.PROGRESS; | ||
|
||
// when | ||
List<Festival> actual = festivalRepository.findAll(filter.getSpecification()); | ||
|
||
// then | ||
assertSoftly(softAssertions -> { | ||
assertThat(actual).hasSize(1); | ||
assertThat(actual.get(0)).matches(festival -> festival.getName().equals(CURRENT_FESTIVAL)); | ||
}); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 당일에 해당하는 축제에 대한 테스트 코드도 필요할 것 같네요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반영했습니다! |
||
|
||
@Test | ||
void 종료_축제_반환() { | ||
// given | ||
FestivalFilter filter = FestivalFilter.END; | ||
|
||
// when | ||
List<Festival> actual = festivalRepository.findAll(filter.getSpecification()); | ||
|
||
// then | ||
assertSoftly(softAssertions -> { | ||
assertThat(actual).hasSize(1); | ||
assertThat(actual.get(0)).matches(festival -> festival.getName().equals(PAST_FESTIVAL)); | ||
}); | ||
} | ||
} |
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.
축제 라는 단어가 들어가도 좋겠네요.
ex) 올바르지 않은 축제의 필터 값 입니다.