Skip to content
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

fix: 필터링 API 파싱 오류 수정 #604 #605

Merged
merged 8 commits into from
Jan 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .github/workflows/backend-ci-cd-dev.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
name: Backend CI/CD dev

on:
push:
paths: [ 'backend/**', '.github/**' ]
branches: [ "develop-be", "develop" ]
workflow_dispatch:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용자가 필요할 때 수동으로 실행할 수 있도록 설정하는 기능이네요! 👍


jobs:
ci:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Backend CI/CD multi prod
name: Backend CI/CD multi dev

on:
push:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public record LoginRequest(
String nickname
) {
public LoginRequest {
if (!Objects.isNull(nickname)) {
if (Objects.nonNull(nickname)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 커밋달고 나서 Objects.isNull을 또 어디에서 사용한 곳이 없을까 찾다가, 이 부분에서 약간 어색함을 느꼈었습니다 ㅋㅋㅋ 👍 👍 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사람 생각하는거 다 똑같구만요~ ㅋㅋㅋㅋㅋㅋㅋㅋ

nickname = nickname.trim();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public static List<MemoryFilter> findAllByName(List<String> filters) {
.map(name -> MemoryFilter.findByName(name.trim()))
.filter(Optional::isPresent)
.map(Optional::get)
.distinct()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

똑같은 필터가 두 개 들어올 수도 있기 때문에, 불필요한 연산 방지를 위해서 중복 제거 작업이 꼭 필요하겠네요! 👍

.toList();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import com.staccato.memory.service.MemoryFilter;
import com.staccato.memory.service.MemorySort;
import io.swagger.v3.oas.annotations.media.Schema;
Expand All @@ -16,12 +17,19 @@ public record MemoryReadRequest(
private static final String DELIMITER = ",";

public List<MemoryFilter> getFilters() {
List<String> filters = Arrays.stream(this.filters.split(DELIMITER))
List<String> filters = parseFilters().stream()
.map(String::trim)
.toList();
return MemoryFilter.findAllByName(filters);
}

private List<String> parseFilters() {
if (Objects.isNull(filters)) {
return List.of();
}
return Arrays.stream(filters.split(DELIMITER)).toList();
}

public MemorySort getSort() {
return MemorySort.findByName(sort);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ void findByNameWithValidFilter() {
);
}

@DisplayName("필터링명이 주어졌을 때 유효한 MemoryFilter 목록 민 반환한다.")
@DisplayName("필터링명이 주어졌을 때 유효한 MemoryFilter 목록만 반환한다.")
@Test
void findAllByNameIfOnlyValid() {
// when
Expand All @@ -49,6 +49,19 @@ void findAllByNameIfOnlyValid() {
);
}

@DisplayName("필터링명이 중복으로 주어졌을 때 중복 없이 MemoryFilter 목록을 반환한다.")
@Test
void findAllByNameDistinct() {
// when
List<MemoryFilter> result = MemoryFilter.findAllByName(List.of("TERM", "term"));

// then
assertAll(
() -> assertThat(result).hasSize(1),
() -> assertThat(result.get(0)).isEqualTo(MemoryFilter.TERM)
);
}

@DisplayName("유효하지 않거나 null인 정렬명이 주어졌을 때 빈 값을 반환한다.")
@Test
void findByNameWithInvalid() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.NullSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.beans.factory.annotation.Autowired;
Expand All @@ -26,13 +27,21 @@ public class MemorySortTest extends ServiceSliceTest {
private MemoryRepository memoryRepository;

@DisplayName("정렬명이 주어졌을 때 대소문자 구분 없이 MemorySort을 반환한다.")
@Test
void findByNameWithValidSort() {
@ParameterizedTest
@CsvSource({
"UPDATED, UPDATED",
"NEWEST, NEWEST",
"OLDEST, OLDEST",
"updated, UPDATED",
"newest, NEWEST",
"oldest, OLDEST"
})
void findByNameWithValidSort(String name, MemorySort sort) {
// when
MemorySort result = MemorySort.findByName("newest");
MemorySort result = MemorySort.findByName(name);

// then
assertThat(result).isEqualTo(MemorySort.NEWEST);
assertThat(result).isEqualTo(sort);
}
Comment on lines 29 to 45
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CsvSource 대신 EnumSource 사용하면 괜찮지 않을까? 생각이 들어서 직접 아래와 같이 구현해봤는데, 오히려 CsvSource 쪽이 훨씬 직관적이어서 좋네요.

@DisplayName("정렬명이 주어졌을 때 대소문자 구분 없이 MemorySort을 반환한다.")
@ParameterizedTest
@EnumSource(MemorySort.class)
void findByNameWithValidSort(MemorySort sort) {
    // when
    MemorySort resultUpper = MemorySort.findByName(sort.name());
    MemorySort resultLower = MemorySort.findByName(sort.name().toLowerCase());

    // then
    assertAll(
            () -> assertThat(resultUpper).isEqualTo(sort),
            () -> assertThat(resultLower).isEqualTo(sort)
    );
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가로, Enum으로 테스트를 진행했을 때 검증 결과 값으로 검증을 하는 것이 어색하게 다가온 것도 있었어요 ㅎㅎ
물론 지금은 대소문자 구분하지 않는 것을 검증하는 것이 주 목적이긴 하지만요 ㅎㅎ


@DisplayName("유효하지 않거나 null인 정렬명이 주어졌을 때 기본값인 UPDATED를 반환한다.")
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DTO 클래스의 테스트를 만들어서, filterssort가 주어지지 않은 상황에 대한 테스트를 추가해주셨군요! 👍

Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package com.staccato.memory.service.dto.request;

import java.util.List;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.NullAndEmptySource;
import com.staccato.memory.service.MemoryFilter;
import com.staccato.memory.service.MemorySort;

import static org.assertj.core.api.Assertions.assertThat;

class MemoryReadRequestTest {
@DisplayName("필터가 주어졌을 때 올바른 필터 목록을 반환한다")
@Test
void getFiltersWithValidFilters() {
// given
MemoryReadRequest request = new MemoryReadRequest("TERM, term", "NEWEST");

// when
List<MemoryFilter> filters = request.getFilters();

// then
assertThat(filters).hasSize(1).containsOnly(MemoryFilter.TERM);
}

@DisplayName("필터가 주어지지 않았을 때 빈 필터 목록을 반환한다")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'필터 목록'이라고 하지 않으면, 정렬도 생각할 수 있기 때문에 확실히 좋은 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵! 한글이 좋아서 한그롤...바꿧습니다 ㅋㅋ

@ParameterizedTest
@NullAndEmptySource
void getFiltersWithNullOrEmptyFilters(String filters) {
// given
MemoryReadRequest request = new MemoryReadRequest(filters, "NEWEST");

// when
List<MemoryFilter> result = request.getFilters();

// then
assertThat(result).isEmpty();
}

@DisplayName("정렬이 주어지지 않았을 때 기본 정렬 기준을 반환한다")
@ParameterizedTest
@NullAndEmptySource
void getSortWithNullOrEmptyFilters(String sort) {
// given
MemoryReadRequest request = new MemoryReadRequest(null, sort);

// when
MemorySort result = request.getSort();

// then
assertThat(result).isEqualTo(MemorySort.UPDATED);
}
}
Loading