Feature/kariskan step3

[kariskan]

리뷰 작성

• 해당 상품에 대한 구매 이력이 있는 경우, 리뷰를 작성하지 않은 경우만 리뷰 작성 가능
• 리뷰가 작성되면, 리뷰를 작성한 상품의 avgScore가 변경됩니다.

상품 검색

• SortType(NEWEST, PRICE_ASC, PRICE_DESC, POPULARITY, TOP_RATED) 기준으로 5개의 쿼리를 직접 작성했습니다.
• Querydsl을 사용할까도 고민했는데 제 경험상으로는 동적 쿼리에 대한 코드가 딱히 엄청나게 줄어든다는 느낌은 받지 못해서 사용하지 않았습니다.
• 각 쿼리는 pagination이 적용돼 있고, 모든 검색 조건 컬럼은 중복이 가능하기 때문에 product_id 컬럼과 함께 multi column pagination을 적용했습니다.

product 10만개, review 150만개, order_product 600만개, pageSize = 100 기준으로,

• 가격 낮은/높은 순과 최신 순은 Product의 컬럼으로만 조회가 가능해서 수십 ms 이내로 조회가 가능하지만, 구매 많은 순과 별점 높은 순은 조인을 하거나, 쿼리를 여러 번 수행해야 해서 수백 ms ~ 수 초가 걸렸습니다.
• 구매 많은 순은 count 쿼리를 잘 이용하면 100ms 대에 처리가 가능했지만 특히, 별점 높은 순은 review_tbl에 대해서 집계 쿼리를 사용해야만 해서 너무 오래 걸렸습니다.
• 커버링 인덱스를 적절히 넣어도 1초 이내로 끊을 수 없었습니다.

• 그래서 조회 성능을 끌어 올리기 위해서 product_tbl에 avg_score와 order_count 컬럼을 추가로 둬서 리뷰 추가 시, 구매 확정 시에 해당 컬럼들을 수정하도록 했습니다.
• 결론적으로 모든 쿼리가 수십 ms 이내로 조회가 가능해졌습니다.

[seungh1024]

5개 메소드가 특정 컬럼을 기준으로 정렬한 값을 리턴하는 것 같은데 이걸 하나의 메소드로 사용하는건 어떻게 생각하시나요?
해당 파라미터가 null이면 처리하지 않고, 입력값이 들어오면 처리하는 방식으로요.
MyBatis가 이런 작업 하기 좋았던 것으로 기억합니다.

[VSFe]

마 바 조 아

[kariskan]

MyBatis 적용했는데 이런 상황에선 굉장히 좋은것 같네요 굿굿

[seungh1024]

위의 쿼리를 동적 쿼리로 바꾸면 여기서도 switch case가 아닌 입력값만 전부 받아서 하나의 메소드만 호출하는 것도 좋을 것 같아요.
음.. 근데 어떤 값을 기준으로 정렬이 되는지 확인하려면 동적 쿼리를 생성하는 코드를 봐야하는 불편함은 있겠네요.

[kochungcheon]

이번 스텝 고생많으셨습니다!

코드 보며 많이 배워서 리뷰 하면서도 즐거웠습니다! 😃

[kochungcheon]

오호.. Factory 쓰는 것 배워 갑니다!

[kariskan]

저번에 본 것 중에 줍줍했습니다.. ㅋㅋㅋ

[kochungcheon]

NOT_FOUND 가 아닌 BAD_REQUEST 를 보내는 이유가 궁금합니다

[kariskan]

그렇네요. 수정했습니다!

[kochungcheon]

다 계산 하지 말고 조건에 만족하는 게 1개 있으면 true 반환하는 건 어떤가요!?

OrderProductReadService 의

@Transactional(readOnly = true)
	public boolean existsByConsumerIdAndProductId(Long consumerId, Long productId) {
		List<Long> orderIds = orderProductRepository.findOrderIdByProductId(productId);
		return orderProductRepository.countByIdsAndConsumerId(orderIds, consumerId) > 0;
	}

보면 결국 있는 지 여부만 찾는 거 같아서요

[kariskan]

exists로 수정했씁니다!

[VSFe]

고민 많이 하신 느낌이 나네요.
조금만 더 수정해봅시다!

[VSFe]

가능하면 해당 로직에 대해선 이후 성능 테스트를 꼭 진행해 주세요.
여러 유저가 동시에 리뷰를 날렸을 때 발생할 수 있는 이슈가 있을"수도" 있습니다.

[kariskan]

리뷰 동시에 날렸을 때 같은 avgScore를 읽고 update 하면 동시성 문제가 발생할 수도 있겠네요.
차후 스텝에서 꼭 진행해 보겠습니다!

[VSFe]

마 바 조 아

[VSFe]

static method로 만드는게 더 낫지 않을까요? + @Component 어노테이션 제거하고

[kariskan]

객체 생성 못하게 하고 static method로 변경했습니다

[VSFe]

orderProduct에 데이터가 상상이상으로 빠르게 찰 것 같은 생각이 좀 드는데...

1. 데이터 분산 방식에 대해 고민해보거나
2. 정기적으로 특정 기간 이전에 이뤄진 결재 내역을 없애버려야 할 수도 있을 것 같네요.

개인적으로 1번이 더 낫지만, 지금 당장 저희가 뭘 하기에는 어려우니까...

[kariskan]

1. OrderProduct가 만약 삭제되지 않고 모든 기간에 걸쳐 조회되게 한다면 range sharding 등으로 기간에 따라 나누면 좋을 것 같습니다. 특정 기간에 엄청난 데이터가 몰려서 삽입될 일도 적을 것 같고, 조회를 할 때도 특정 기간만 많은 트래픽이 오는 경우는 없으니까요.

그런데 일단 2번으로 진행해서 30일 이전 기록들은 모두 삭제되게 했습니다.

[VSFe]

리뷰 작성 가능 기간에 대한 제약은 없나요?

[kariskan]

OrderProduct가 존재할 수 있는 기간을 30일로 함으로써, 구매 이력이 없다면

• 아예 구매한 적이 없거나
• 리뷰 작성 가능 기간이 지난 것을 의미

하게 되었습니다. 그래서 error message만 변경하였습니다.

[VSFe]

가능하면 이건 validate로 해 주세요.
(어지간하면 JPA가 DDL을 날리는 일은 없어야 합니다.)

[kariskan]

데이터 엄청 많은 테이블에 인덱스 생성 쿼리 날리니까 블락이 오래 되더라구요.. 주의하겠습니다.

[VSFe]

그냥 exist query를 날릴 순 없을까요?

select 1
from ~

[kariskan]

그렇네요 굳이 셀 필요가 없네요. exists query로 수정했습니다.

[VSFe]

가능하면 @ModelAttribute 보다는 @RequestBody 를 사용하면 좋을 것 같아요.
참고: https://tecoble.techcourse.co.kr/post/2021-05-11-requestbody-modelattribute/

[kariskan]

이유가 왜일까요? 참고 문서에서는 @RequestBody는 http body가 java object로 변환되고, @ModelAttribute는 form or query param을 java object로 변환된다고 하는데 GetMapping에서는 body를 통한 데이터 전달을 지양하고, 보통 query string으로 요청을 하는 것으로 알고 있어서요.
차이가 있다면 RequestBody는 기본 생성자 + getter or setter이고 ModelAttribute는 생성자 or setter인 것 같습니다.
그런데 record로 정의하면 생성자, getter, 기타 등등 포함되기 때문에 어떤 형태로 오든 변환이 가능하지 않나요?

ProductSearchRequest.class

//
// Source code recreated from a .class file by IntelliJ IDEA
// (powered by FernFlower decompiler)
//

package org.c4marathon.assignment.domain.product.dto.request;

import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size;
import java.time.LocalDateTime;
import java.util.Objects;
import org.c4marathon.assignment.global.constant.SortType;

public record ProductSearchRequest(@NotNull @Size(
    min = 2,
    message = "keyword length less than 2"
) String keyword, @NotNull SortType sortType, LocalDateTime createdAt, Long productId, Long amount, Long orderCount, Double score, int pageSize) {
    public ProductSearchRequest(String keyword, SortType sortType, LocalDateTime createdAt, Long productId, Long amount, Long orderCount, Double score, int pageSize) {
        this.keyword = keyword;
        this.sortType = sortType;
        this.createdAt = (LocalDateTime)Objects.requireNonNullElse(createdAt, LocalDateTime.now());
        this.productId = (Long)Objects.requireNonNullElse(productId, Long.MIN_VALUE);
        this.amount = (Long)Objects.requireNonNullElse(amount, this.getDefaultAmount(sortType));
        this.orderCount = (Long)Objects.requireNonNullElse(orderCount, Long.MAX_VALUE);
        this.score = (Double)Objects.requireNonNullElse(score, Double.MAX_VALUE);
        this.pageSize = pageSize;
    }

    private Long getDefaultAmount(SortType sortType) {
        return sortType == SortType.PRICE_ASC ? Long.MIN_VALUE : Long.MAX_VALUE;
    }

    public static ProductSearchRequestBuilder builder() {
        return new ProductSearchRequestBuilder();
    }

    public @NotNull @Size(
    min = 2,
    message = "keyword length less than 2"
) String keyword() {
        return this.keyword;
    }

    public @NotNull SortType sortType() {
        return this.sortType;
    }

    public LocalDateTime createdAt() {
        return this.createdAt;
    }

    public Long productId() {
        return this.productId;
    }

    public Long amount() {
        return this.amount;
    }

    public Long orderCount() {
        return this.orderCount;
    }

    public Double score() {
        return this.score;
    }

    public int pageSize() {
        return this.pageSize;
    }

    public static class ProductSearchRequestBuilder {
        private String keyword;
        private SortType sortType;
        private LocalDateTime createdAt;
        private Long productId;
        private Long amount;
        private Long orderCount;
        private Double score;
        private int pageSize;

        ProductSearchRequestBuilder() {
        }

        public ProductSearchRequestBuilder keyword(final String keyword) {
            this.keyword = keyword;
            return this;
        }

        public ProductSearchRequestBuilder sortType(final SortType sortType) {
            this.sortType = sortType;
            return this;
        }

        public ProductSearchRequestBuilder createdAt(final LocalDateTime createdAt) {
            this.createdAt = createdAt;
            return this;
        }

        public ProductSearchRequestBuilder productId(final Long productId) {
            this.productId = productId;
            return this;
        }

        public ProductSearchRequestBuilder amount(final Long amount) {
            this.amount = amount;
            return this;
        }

        public ProductSearchRequestBuilder orderCount(final Long orderCount) {
            this.orderCount = orderCount;
            return this;
        }

        public ProductSearchRequestBuilder score(final Double score) {
            this.score = score;
            return this;
        }

        public ProductSearchRequestBuilder pageSize(final int pageSize) {
            this.pageSize = pageSize;
            return this;
        }

        public ProductSearchRequest build() {
            return new ProductSearchRequest(this.keyword, this.sortType, this.createdAt, this.productId, this.amount, this.orderCount, this.score, this.pageSize);
        }

        public String toString() {
            return "ProductSearchRequest.ProductSearchRequestBuilder(keyword=" + this.keyword + ", sortType=" + this.sortType + ", createdAt=" + this.createdAt + ", productId=" + this.productId + ", amount=" + this.amount + ", orderCount=" + this.orderCount + ", score=" + this.score + ", pageSize=" + this.pageSize + ")";
        }
    }
}

[VSFe]

아 Get이었네요 ㅋㅋ
제가 잘못봄

[VSFe]

Objects.requireNonNullElse(value, defaultValue) 로 대체 가능합니다.

[kariskan]

이 메서드가 이런곳에서 쓰는 것이었군요..

[VSFe]

형식 일치가 필요합니다. 여기에만 asc가 있네요.

[kariskan]

세세한 리뷰 감사합니다.
개인적으로 asc를 적는 것과 안 적는 것 중에 어떤거 선호하시나요?
datagrip에서는 asc 쓰면 채찍질 엄청 하더라구요

[VSFe]

기왕이면 적는게 좋습니다.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
83.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud