[FEATURE] : [경매 종료가 임박한 진행중인 경매 목록 조회] by na0th · Pull Request #37 · TaskSprints/auction

na0th · 2024-09-05T08:58:27Z

경매 종료가 임박한 진행중인 경매 목록 조회 추가

미흡 : 성능 개선 못함,
/auction으로 엔드포인트 합쳐야 하는데 못했음,
테스트쪽 리뷰 받아야 함.

name: Pull Request
about: '풀 리퀘스트를 제출합니다.'
title: "[FEATURE] - [기능명] 또는 [FIX] - [버그 설명]"
labels: ''
assignees: ''

[✨ FEATURE] 마감이 임박한 경매들을 메인에 게시합니다. #9 에 대한 Issue 을 중점으로 구현

🆕 기능 추가 / 🔧 버그 수정

경매 종료가 임박했고, 진행 중인 경매 목록을 조회합니다.

📋 변경 사항

🔍 테스트 사항

Controller 테스트 미구현

Repository, Service 테스트 미흡, 꼼꼼한 리뷰 필수

📄 관련 문서

변경 사항과 관련된 문서나 링크가 있다면 여기에 추가해주세요.

📝 추가 사항

/auction으로 엔드포인트 합쳐야 하는데 못했습니다.

polyglot-k

일단 전체적으로 작업 진행해주셔서 감사합니다! 일단 현재 Service 내부 로직은 의도에 맞게 잘 설계된 것 같습니다.

아쉬운 부분은 CriteriaRepository 는 전체 조회에서의 모든 filter 를 포함할 수 있도록 하려고 구성해 놓은 것이라서, 이를 좀 더 활용했으면 좋았을 것 같습니다.

인자를 추가하는 형태로 진행했어도 괜찮았을 것 같습니다.

Controller 를 새롭게 추가하는 것은, 지금 고민해봤을 때, 마감임박 요소만을 빠르게 불러오는 것을 만들어야하는지에 대한 고민을 가지고 있습니다. 일단 만들게 되었을 때는, 해당 기능을 통해서 fit하게 가지고 올 수 있다는 장점이 있는데, 단점은 restful 하지 못할 수도 있다라는 점이 있긴 합니다. 그래서 해당 부분에 대한 유지보수가 조금 필요해 보입니다!

polyglot-k · 2024-09-05T09:10:38Z

+        List<AuctionResponse> auctions = auctionService.getAuctionsByFilter(productCategory,auctionCategory);
+        return ResponseEntity.ok(ApiResult.success(ApiResponseMessages.ALL_AUCTIONS_RETRIEVED, auctions));
+    }
+    @GetMapping("/ending-soon")


엔드 포인트 이름이 조금 애매한거 같습니다.

/acution 으로 합치는 작업 필요하다고 한거 같아서 이부분 보류 하겠습니다

polyglot-k · 2024-09-05T09:12:53Z


    List<Auction> findAuctionsByAuctionCategory(AuctionCategory auctionCategory);
    List<Auction> findAuctionByProduct_Category(ProductCategory productCategory);
+    List<Auction> findAuctionsByEndTimeBetweenAndAuctionStatusOrderByEndTimeAsc(LocalDateTime now, LocalDateTime next24Hours, AuctionStatus auctionStatus);


얘도 어떻게보면, 필터 계열로 쓰일 수 있는 것이라고 생각이 듭니다. 기존 함수에서 인자 값을 추가 해주는 것은 어떨까라는 생각이 들어서 조금 아쉽습니다!!

polyglot-k · 2024-09-05T09:14:06Z

 public interface AuctionCriteriaRepository {
    public List<Auction> getAuctionsByFilters(ProductCategory productCategory,
                                              AuctionCategory category);
+    public List<Auction> getAuctionsEndWith24Hours(LocalDateTime now, LocalDateTime next24Hours, AuctionStatus auctionStatus);


기존에 있던 것을 활용해서 되었을 것 같습니다. 인자가 많다면, 묶어서 Condition 이라는 DTO 를 통해서 연결 하면 더 좋을 듯 합니다.

polyglot-k · 2024-09-05T09:15:23Z

    }

+    @Override
+    public List<Auction> getAuctionsEndWith24Hours(LocalDateTime now, LocalDateTime next24Hours, AuctionStatus auctionStatus) {


기존에 사용하는 함수의 역할과 크게 차이가 나지 않는다고 생각합니다.
위의 함수를 활용해도 동일 기능을 재현할 수 있을거 같습니다.

thornscript

자주 참여하지 못해서 죄송합니다 😢

수고하셨습니다 👍

thornscript · 2024-09-06T18:21:35Z

+    @Override
+    public List<AuctionResponse> getAuctionsByEndTimeBetweenOrderByEndTimeAsc() {
+        // auction의 endTime까지 24시간 이하로 남은 진행중인 경매 목록 조회
+        LocalDateTime now = LocalDateTime.now();


LocalDateTime.now()가 환경에 따라서 깨지기 쉬운 메서드인 것 같아서(예: 배포 환경의 TZ에 따라서 예상하지 못한 결과를 얻을 수 있는 등) 이 부분은 주의가 필요할 것 같네요! 🤔

https://code-run.tistory.com/m/82

해당 부분은 문서화하여 공유하면 좋을 것 같습니다

1. 경매 종료가 임박한 진행중인 경매 목록 조회 추가 미흡 : 성능 개선 못함, /auction으로 엔드포인트 합쳐야 하는데 못했음, 테스트쪽 리뷰 받아야 함.

코드 포맷터 빌드

1. 마감이 얼마 안 남은 경매 목록 조회 메서드의 파라미터가 없던 것에서 now, next24Hours 추가

1. 테스트 케이스의 LocalDateTime을 고정 2. 테스트 대상이 아닌 파라미터는 테스트 픽스처에 넣음 3. assertAll로 컬렉션인 경우 끝까지 assertion하고 결과를 보여주도록 함

polyglot-k · 2024-09-08T14:40:06Z

+        return queryFactory
+                .selectFrom(auction)
+                .where(endTimeBetween.and(statusEquals))
+                .orderBy(auction.endTime.asc())


order by가 성능 저하의 병목지점이 될 수 있음에 유의 해야합니다! 이에 대한 방안을 고려해보고, 최선책으로 order by를 사용해야하는 근거를 가지고 오시면 좋을 것 같습니다.!

제가 이전에 만든 filter 함수에 합치는게 더 낫지않을까요? 충분히 합칠 수 있다고 생각이 됩니다.

넵 노력하겠습니다

1. createAuction 중복 코드 리스트로 수정 2. LocalDateTime.now()를 고정 시간으로 수정 3. AuctionServiceImplTest 중복 코드 삭제

1: /auction에서 검색 조건별로 경매 목록을 조회한다. 미흡 : 파라미터를 SearchCondition이라는 DTO로 받아서 기존 테스트 다 깨져서 수정해야 함, 변경점에 대한 테스트 코드 없음.

na0th added 기능 추가 도움 필요 labels Sep 5, 2024

na0th added this to the 경매 도메인 milestone Sep 5, 2024

na0th self-assigned this Sep 5, 2024

na0th linked an issue Sep 5, 2024 that may be closed by this pull request

[✨ FEATURE] 마감이 임박한 경매들을 메인에 게시합니다. #9

Closed

na0th commented Sep 5, 2024

View reviewed changes

Comment thread src/main/java/com/tasksprints/auction/api/auction/AuctionController.java

na0th commented Sep 5, 2024

View reviewed changes

Comment thread src/main/java/com/tasksprints/auction/api/auction/AuctionController.java Outdated

na0th commented Sep 5, 2024

View reviewed changes

Comment thread src/test/java/com/tasksprints/auction/api/AuctionControllerTest.java

na0th commented Sep 5, 2024

View reviewed changes

Comment thread src/test/java/com/tasksprints/auction/domain/auction/service/AuctionServiceImplTest.java Outdated

polyglot-k requested changes Sep 5, 2024

View reviewed changes

thornscript reviewed Sep 6, 2024

View reviewed changes

na0th closed this Sep 7, 2024

na0th force-pushed the feat-pr branch from 21274c9 to 5ec5554 Compare September 7, 2024 15:20

na0th added 3 commits September 8, 2024 00:46

Feat : 경매 종료가 임박한 진행중인 경매 목록 조회

b3f344c

1. 경매 종료가 임박한 진행중인 경매 목록 조회 추가 미흡 : 성능 개선 못함, /auction으로 엔드포인트 합쳐야 하는데 못했음, 테스트쪽 리뷰 받아야 함.

Merge branch 'feat-pr' of https://github.com/na0th/auction into feat-pr

3581808

style : 코드 포맷터 빌드

1edfd44

코드 포맷터 빌드

polyglot-k reopened this Sep 7, 2024

na0th added 2 commits September 8, 2024 22:45

Refactor : 마감이 얼마 안남은 경매 조회 메서드에 파라미터 추가

f3f95cc

1. 마감이 얼마 안 남은 경매 목록 조회 메서드의 파라미터가 없던 것에서 now, next24Hours 추가

polyglot-k requested changes Sep 8, 2024

View reviewed changes

na0th added 2 commits September 9, 2024 00:45

test : 테스트 코드 수정

2499cc3

1. createAuction 중복 코드 리스트로 수정 2. LocalDateTime.now()를 고정 시간으로 수정 3. AuctionServiceImplTest 중복 코드 삭제

polyglot-k changed the base branch from develop to patch-auction-v1 September 10, 2024 08:18

polyglot-k merged commit 1015a0f into TaskSprints:patch-auction-v1 Sep 10, 2024

na0th deleted the feat-pr branch May 15, 2025 08:48

Conversation

na0th commented Sep 5, 2024 • edited by polyglot-k Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🆕 기능 추가 / 🔧 버그 수정

📋 변경 사항

🔍 테스트 사항

📄 관련 문서

📝 추가 사항

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

polyglot-k left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thornscript left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

na0th commented Sep 5, 2024 •

edited by polyglot-k

Loading

thornscript left a comment •

edited

Loading