[Review] 2022/05/27 2차 PR (Aribnb 서비스)

이번 리뷰는 Dion 께서 해주셨다! 리뷰해주셔서 감사합니다 !!

https://github.com/codesquad-members-2022/airbnb/pull/88

[Team - 06][BE] 에어비앤비 프로젝트 1주차 2회차 PR by zbqmgldjfh · Pull Request #88 · codesquad-members-2022/airb

 

1. 질문

Native Query 질문

다음 두 메서드 searchByCondition 과 searchByConditionQueryDsl 은 동일한 일을 수행하고 있습니다.

@Override
public List<House> searchByCondition(Point position, Integer minFee, Integer maxFee) {
  double baseLatitude = position.getX();
  double baseLongitude = position.getY();
  double distance = 1000; // m 단위

  // TODO : *를 필드 명으로 변경해주기
  Query query = em.createNativeQuery("" +
                                     "SELECT * \n" +
                                     "FROM house AS h \n" +
                                     "WHERE (ST_Distance_Sphere(h.point, point(?1, ?2)) < ?3) AND h.price >= ?4 AND h.price <= ?5"
                                     , House.class);

  query.setParameter(1, baseLatitude);
  query.setParameter(2, baseLongitude);
  query.setParameter(3, distance);
  query.setParameter(4, minFee);
  query.setParameter(5, maxFee);

  return query.getResultList();
}

다음은 queryDsl 을 통해 조건을 동적으로 받고 싶어 만들어본 함수 입니다.

다만 queryDsl에서 mysql의 함수를 직접 이용하려다 보니 간단하게 사용할수가 없었습니다...

@Override
public List<House> searchByConditionQueryDsl(Point position, Integer minFee, Integer maxFee) {
double baseLatitude = position.getX(); // 내 위치 y
double baseLongitude = position.getY(); // 내 위치 x
double distance = 1000; // m 단위

QHouse subHouse = new QHouse("subHouse");

List<House> results = queryFactory
          .selectFrom(house)
          .where(
                          Expressions.stringTemplate(
                          "function('ST_Distance_Sphere', {0}, {1})", 
                          subHouse.point, GeometryUtils.toPoint(baseLatitude, baseLongitude))
                          .as("subDistance")
                          .loe(String.valueOf(distance)
                      )
           ).fetch();

     return results;
}

루시드와 거의 5/26일 하루를 바쳤지만.... queryDsl 상에서 mysql의 함수 사용이 잘 작동하지 않았습니다...

여러 글들을 읽어보니 Expressions.stringTemplate를 사용하라고 하던데.... 인자가 잘 전달되지 않는것 같습니다..

이부분 해결책을 알려주신다면 감사하겠습니다...

 

답변

QueryDsl 을 포기하고 JPQL로 코드를 변경하였다.

위에서 보이는 entityManager를 사용하는 코드를 다음과 같이 간단하게 @Query를 사용하여 변경하였다.

public interface HouseRepository extends JpaRepository<House, Long> {

    @Query("SELECT h " +
            "FROM House h " +
            "WHERE FUNCTION('ST_Distance_Sphere', h.point, :point) < :distance AND h.price >= :min AND h.price <= :max")
    List<House> searchByCondition(@Param("point") Point point, @Param("distance") int distance, @Param("min") int min, @Param("max") int max);
}

@Query 로 간단하게 해결하였다. JPQL 에서 DB의 함수를 불러오는 방법을 알고나니 쉬웠다.

 

2. 코드 리뷰

2-1) 타입과 일치하는 변수명

음 사실 이부분은 꼭 일치해야 하나? 란 생각이 들었다.

startLocalDateTime 과 endLocalDateTiem으로 변경하기는 했는데, Java Beans Properties 조약 때문인건 알겠는데...

뭐랄까? 직관적이지 않은 이름인것 같달까? 

 

하지만 규약은 규약이지 변경하였다.

 

2-2) 역참조에 대한 고민

우리 ERD 에서 User - WishList 가 1 : N 으로, WishList - House 가 N : 1 의 상황이였다.

이러한 상황에서 구현의 편의를 위해 원래는 모두 양방향으로 매핑을 해줬었다.

 

하지만 House 에서 wishList에 대한 참조는 진짜 일어날 일이 거의 없었다.

찜 목록에 해당하는 wishList는 User가 참조하지, House 가 참조할일이 거의 없는것이 사실 이였다.

 

따라서 House 에서는 이에대한 참조를 제거 하였다!

 

2-3) Native Query 에 대한 의문

이부분은 질문 1번과 연관된 피드백 이다.

JPA에서 DB의 함수를 직접 호출하는 방법을 사용하고 나니 Dion의 말처럼 NativeQuery를 사용할 필요가 없게 되었다!

 

두번째 질문이 가장 어려웠다.

 

"우리팀에 정말 최선인가?"

 

Dion 께서는 다음과 같은 답변을 주셨다.

최선인지 판단하는 기준은 "'우리 팀'이 '제한된 기간'동안 '요구사항을 충족'할 수 있는가"가 아닐까요?

제한된 기간동안 요구사항의 충족이라...

그렇다면 Native 보다는 훨 간편한 @Query의 방식이 더 적합한것 같다.

 

2-4) Transactional 주의

까먹고 Service 계층에 Transactional을 추가해주지 않았다.

위 서비스와 같이 Repositroy에 접근하는 계층은 항상 Transaction 범위 안에서 진행해야 무결성이 보장된다.

이점을 까먹고 에노테이션을 추가해주지 않았다.

 

Service 계층에 @Transactional을 추가해 주었다.

 

2-5) Controller 에서는 Domain 객체를 숨기기

흠 이말은 즉슨, Service 에서 dto를 받고 dto를 반환해야 한다는 의미로 확장 해석이 된다.

Service가 도메인 모델을 직접 받게되면 Controller에서 domain 모델에 의존성이 생겨버리니 말이다.

 

하지만 이에 대해서 아직은 확신이 안선다.

 

과연 DTO는 어디까지 사용해야 하는가? Controller 인가? Service 인가?...

이는 앞으로 더 공부해가면서 직접 느껴봐야 알수있을것 같다.

 

https://tecoble.techcourse.co.kr/post/2021-04-25-dto-layer-scope/

DTO의 사용 범위에 대하여

 

2-6) Database Flatform 선택

구현 당시 참고했던 블로그 들에서 전부 MySQL56InnoDBSpatialDialect 를 사용하라 하여 사용하였다.

따라서 우리는 처음에 8버전에서는 지원을 아직 안하는 줄 알았다.

 

하지만 이상했다... Google 에 MySQL Spatial Function을 검색하면 공식 레퍼런스가 8버전으로 설명이 나온다.

이말은 즉, 8버전 에서도 사용이 가능하다는 것 이였다.

 

따라서 이를 MySQL8SpatialDialect 로 변경하니 정상적으로 작동하는것을 확인할 수 있었다.

레퍼런스를 더 참고하자!

 

2-7) 내가 하려는 Test의 범주가 어디인가?

생각해보니 나의 테스트 코드는 Service 계층을 검증하고 싶은것이 아니였다.

Repository에서 사용하고 있는 MySQL의 함수가 정상적으로 작동하는지가 궁금했었다.

 

다만 이를 서비스에서 Repository를 끌어다 사용하고 있다보니, 어느세 그냥 Service 테스트를 작성하고 있었다.

이를 Repository 테스트로 다음과 같이 변경하였다.

ExtendWith(SpringExtension.class)
@DataJpaTest
@AutoConfigureTestDatabase(replace = Replace.NONE)
class HouseRepositoryTest {

    @Autowired
    HouseRepository houseRepository;

    @Autowired
    UserRepository userRepository;

    Point nowPosition;

    @BeforeEach
    void init() throws ParseException {
        Double latitude = 37.549214;
        Double longitude = 126.914016;
        String pointWKT = String.format("POINT(%s %s)", longitude, latitude);

        // WKTReader를 통해 WKT를 실제 타입으로 변환합니다.
        nowPosition = (Point) new WKTReader().read(pointWKT);

        User host = new User("user1", "email", Role.HOST);
        userRepository.save(host);

        House house0 = new House("house0", 85000, new DetailInfo(10, "oneRoom0", "방입니다", 4.8, 10),
            GeometryUtils.toPoint(37.549, 126.928), host);

        House house1 = new House("house1", 55000, new DetailInfo(10, "oneRoom1", "방입니다", 4.8, 10),
            GeometryUtils.toPoint(37.5492, 126.9113), host);

        House house2 = new House("house2", 55000, new DetailInfo(10, "oneRoom2", "방입니다", 4.8, 10),
            GeometryUtils.toPoint(37.5492, 126.9113), host);

        House house3 = new House("house3", 55000, new DetailInfo(10, "oneRoom3", "방입니다", 4.8, 10),
            GeometryUtils.toPoint(37.5492, 126.9113), host);

        House house4 = new House("house4", 85000, new DetailInfo(10, "oneRoom4", "방입니다", 4.8, 10),
            GeometryUtils.toPoint(37.552469, 126.933644), host);

        House house5 = new House("house5", 95000, new DetailInfo(10, "oneRoom5", "방입니다", 4.8, 10),
            GeometryUtils.toPoint(37.552469, 126.933650), host);

        House house6 = new House("house6", 95000, new DetailInfo(10, "oneRoom6", "방입니다", 4.8, 10),
            GeometryUtils.toPoint(37.5493, 126.9199), host);

        houseRepository.saveAll(List.of(house0, house1, house2, house3, house4, house5, house6));
    }

    @DisplayName("반경 1000m 내 10 ~ 100000원의 가격을 가진 숙소를 찾는다.")
    @Test
    void search_condition_jpql_test() {
        // when
        List<House> result = houseRepository.searchByCondition(nowPosition, 1000, 10, 100000);

        // then
        assertThat(result.size()).isEqualTo(4);
        assertThat(result).extracting("name").containsExactly("house1", "house2", "house3", "house6");
    }

    @DisplayName("가격대 별 숙소의 개수를 10000원 단위 오름차순으로 반환한다.")
    @Test
    void number_of_house_test() {
        // when
        List<HouseCount> houseCounts = houseRepository.numberOfHousesInTheRange(nowPosition, 1000);

        // then
        assertThat(houseCounts.size()).isEqualTo(2);
        assertThat(houseCounts.get(0).getPrice()).isEqualTo(50000);
        assertThat(houseCounts.get(0).getCount()).isEqualTo(3);
        assertThat(houseCounts.get(1).getPrice()).isEqualTo(90000);
        assertThat(houseCounts.get(1).getCount()).isEqualTo(1);
    }
}