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

[refactor] #221 - 메인페이지에서 캐러셀 응답 시 캐러셀 번호로 정렬해서 주도록 변경 및 메인 페이지 조회 코드 최적화 #222

Merged
merged 12 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from 11 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
10 changes: 4 additions & 6 deletions src/main/java/com/beat/admin/adapter/in/api/AdminApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

import com.beat.admin.application.dto.response.CarouselFindAllResponse;
import com.beat.admin.application.dto.response.UserFindAllResponse;
import com.beat.admin.application.dto.request.CarouselProcessRequest;
import com.beat.admin.application.dto.response.CarouselProcessAllResponse;
import com.beat.admin.application.dto.request.CarouselHandleRequest;
import com.beat.admin.application.dto.response.CarouselHandleAllResponse;
import com.beat.global.auth.annotation.CurrentMember;
import com.beat.global.common.dto.ErrorResponse;
import com.beat.global.common.dto.SuccessResponse;
Expand All @@ -16,7 +16,6 @@
import io.swagger.v3.oas.annotations.responses.ApiResponses;
import io.swagger.v3.oas.annotations.tags.Tag;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestParam;

Expand Down Expand Up @@ -128,9 +127,8 @@ ResponseEntity<SuccessResponse<CarouselFindAllResponse>> readAllCarouselImages(
)
}
)
@PutMapping("/carousels")
ResponseEntity<SuccessResponse<CarouselProcessAllResponse>> processCarouselImages(
ResponseEntity<SuccessResponse<CarouselHandleAllResponse>> processCarouselImages(
@CurrentMember Long memberId,
@RequestBody CarouselProcessRequest request
@RequestBody CarouselHandleRequest request
);
}
10 changes: 5 additions & 5 deletions src/main/java/com/beat/admin/adapter/in/api/AdminController.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package com.beat.admin.adapter.in.api;

import com.beat.admin.application.dto.response.CarouselFindAllResponse;
import com.beat.admin.application.dto.request.CarouselProcessRequest;
import com.beat.admin.application.dto.response.CarouselProcessAllResponse;
import com.beat.admin.application.dto.request.CarouselHandleRequest;
import com.beat.admin.application.dto.response.CarouselHandleAllResponse;
import com.beat.admin.exception.AdminSuccessCode;
import com.beat.admin.application.dto.response.UserFindAllResponse;
import com.beat.admin.facade.AdminFacade;
Expand Down Expand Up @@ -69,10 +69,10 @@ public ResponseEntity<SuccessResponse<CarouselFindAllResponse>> readAllCarouselI

@Override
@PutMapping("/carousels")
public ResponseEntity<SuccessResponse<CarouselProcessAllResponse>> processCarouselImages(
public ResponseEntity<SuccessResponse<CarouselHandleAllResponse>> processCarouselImages(
@CurrentMember Long memberId,
@RequestBody CarouselProcessRequest request) {
CarouselProcessAllResponse response = adminFacade.checkMemberAndProcessAllPromotionsSortedByCarouselNumber(memberId, request);
@RequestBody CarouselHandleRequest request) {
CarouselHandleAllResponse response = adminFacade.checkMemberAndProcessAllPromotionsSortedByCarouselNumber(memberId, request);
return ResponseEntity.status(HttpStatus.OK)
.body(SuccessResponse.of(AdminSuccessCode.UPDATE_ALL_CAROUSEL_PROMOTIONS_SUCCESS, response));
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/beat/admin/application/AdminService.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.beat.admin.application;

import com.beat.admin.application.dto.request.CarouselProcessRequest.PromotionGenerateRequest;
import com.beat.admin.application.dto.request.CarouselProcessRequest.PromotionModifyRequest;
import com.beat.admin.application.dto.request.CarouselHandleRequest.PromotionGenerateRequest;
import com.beat.admin.application.dto.request.CarouselHandleRequest.PromotionModifyRequest;
import com.beat.admin.port.in.AdminUseCase;
import com.beat.domain.performance.domain.Performance;
import com.beat.domain.performance.port.in.PerformanceUseCase;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import com.beat.domain.promotion.domain.CarouselNumber;

public record CarouselProcessRequest(
public record CarouselHandleRequest(
List<PromotionHandleRequest> carousels
) {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.beat.admin.application.dto.request;

import static com.beat.admin.application.dto.request.CarouselProcessRequest.*;
import static com.beat.admin.application.dto.request.CarouselHandleRequest.*;

import com.beat.domain.promotion.domain.CarouselNumber;
import com.fasterxml.jackson.annotation.JsonSubTypes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@

import com.beat.domain.promotion.domain.Promotion;

public record CarouselProcessAllResponse(
public record CarouselHandleAllResponse(
List<PromotionResponse> modifiedPromotions
) {

public static CarouselProcessAllResponse from(List<Promotion> promotions) {
public static CarouselHandleAllResponse from(List<Promotion> promotions) {
List<PromotionResponse> modifiedPromotions = promotions.stream()
.map(PromotionResponse::from)
.collect(Collectors.toList());
return new CarouselProcessAllResponse(modifiedPromotions);
return new CarouselHandleAllResponse(modifiedPromotions);
}

public record PromotionResponse(
Expand Down
22 changes: 11 additions & 11 deletions src/main/java/com/beat/admin/facade/AdminFacade.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
import com.beat.admin.application.dto.request.PromotionHandleRequest;
import com.beat.admin.application.dto.response.CarouselFindAllResponse;
import com.beat.admin.application.dto.response.UserFindAllResponse;
import com.beat.admin.application.dto.request.CarouselProcessRequest;
import com.beat.admin.application.dto.request.CarouselProcessRequest.PromotionGenerateRequest;
import com.beat.admin.application.dto.request.CarouselProcessRequest.PromotionModifyRequest;
import com.beat.admin.application.dto.response.CarouselProcessAllResponse;
import com.beat.admin.application.dto.request.CarouselHandleRequest;
import com.beat.admin.application.dto.request.CarouselHandleRequest.PromotionGenerateRequest;
import com.beat.admin.application.dto.request.CarouselHandleRequest.PromotionModifyRequest;
import com.beat.admin.application.dto.response.CarouselHandleAllResponse;
import com.beat.admin.port.in.AdminUseCase;
import com.beat.domain.member.port.in.MemberUseCase;
import com.beat.domain.promotion.domain.CarouselNumber;
Expand Down Expand Up @@ -64,8 +64,8 @@ public CarouselFindAllResponse checkMemberAndFindAllPromotionsSortedByCarouselNu
return CarouselFindAllResponse.from(promotions);
}

public CarouselProcessAllResponse checkMemberAndProcessAllPromotionsSortedByCarouselNumber(Long memberId,
CarouselProcessRequest request) {
public CarouselHandleAllResponse checkMemberAndProcessAllPromotionsSortedByCarouselNumber(Long memberId,
CarouselHandleRequest request) {

memberUseCase.findMemberById(memberId);

Expand All @@ -85,10 +85,10 @@ public CarouselProcessAllResponse checkMemberAndProcessAllPromotionsSortedByCaro
List<Promotion> sortedPromotions = adminUsecase.processPromotionsAndSortByCarouselNumber(modifyRequests,
generateRequests, deleteCarouselNumbers, overlappingCarouselNumbers);

return CarouselProcessAllResponse.from(sortedPromotions);
return CarouselHandleAllResponse.from(sortedPromotions);
}

private void categorizePromotionRequests(CarouselProcessRequest request,
private void categorizePromotionRequests(CarouselHandleRequest request,
List<PromotionModifyRequest> modifyRequests, List<PromotionGenerateRequest> generateRequests,
Set<CarouselNumber> requestCarouselNumbers) {

Expand All @@ -111,15 +111,15 @@ private List<CarouselNumber> findDeleteCarouselNumbers(Set<CarouselNumber> reque
}

private List<CarouselNumber> findOverlappingCarouselNumbers(Set<CarouselNumber> requestCarouselNumbers,
List<CarouselNumber> allExistingCarouselNumbers, CarouselProcessRequest request) {
List<CarouselNumber> allExistingCarouselNumbers, CarouselHandleRequest request) {
return allExistingCarouselNumbers.stream()
.filter(requestCarouselNumbers::contains)
.filter(existingCarouselNumber -> {
Promotion existingPromotion = promotionUseCase.findPromotionByCarouselNumber(existingCarouselNumber);
return request.carousels()
.stream()
.filter(req -> req instanceof CarouselProcessRequest.PromotionModifyRequest)
.map(req -> (CarouselProcessRequest.PromotionModifyRequest)req)
.filter(req -> req instanceof CarouselHandleRequest.PromotionModifyRequest)
.map(req -> (CarouselHandleRequest.PromotionModifyRequest)req)
.noneMatch(req -> req.promotionId() != null && req.promotionId().equals(existingPromotion.getId()));
})
.toList();
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/beat/admin/port/in/AdminUseCase.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.beat.admin.port.in;

import com.beat.admin.application.dto.request.CarouselProcessRequest.PromotionGenerateRequest;
import com.beat.admin.application.dto.request.CarouselProcessRequest.PromotionModifyRequest;
import com.beat.admin.application.dto.request.CarouselHandleRequest.PromotionGenerateRequest;
import com.beat.admin.application.dto.request.CarouselHandleRequest.PromotionModifyRequest;
import com.beat.domain.promotion.domain.CarouselNumber;
import com.beat.domain.promotion.domain.Promotion;

Expand Down
33 changes: 33 additions & 0 deletions src/main/java/com/beat/domain/performance/api/HomeApi.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.beat.domain.performance.api;

import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.RequestParam;

import com.beat.domain.performance.application.dto.home.HomeFindAllResponse;
import com.beat.domain.performance.domain.Genre;
import com.beat.global.common.dto.ErrorResponse;
import com.beat.global.common.dto.SuccessResponse;

import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.media.Content;
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.responses.ApiResponses;
import io.swagger.v3.oas.annotations.tags.Tag;

@Tag(name = "Home", description = "홈 화면에서 공연 및 홍보목록 조회 API")
public interface HomeApi {

@Operation(summary = "전체 공연 및 홍보 목록 조회", description = "홈 화면에서 전체 공연 목록 및 홍보 목록을 조회하는 GET API")
@ApiResponses(
value = {
@ApiResponse(
responseCode = "200",
description = "홈 화면 공연 목록 조회가 성공적으로 완료되었습니다.",
content = @Content(schema = @Schema(implementation = SuccessResponse.class))
)
}
)
ResponseEntity<SuccessResponse<HomeFindAllResponse>> getHomePerformanceList(
@RequestParam(required = false) Genre genre);
}
39 changes: 20 additions & 19 deletions src/main/java/com/beat/domain/performance/api/HomeController.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package com.beat.domain.performance.api;

import com.beat.domain.performance.application.PerformanceService;
import com.beat.domain.performance.application.dto.home.HomeRequest;
import com.beat.domain.performance.application.dto.home.HomeResponse;
import com.beat.domain.performance.application.HomeService;
import com.beat.domain.performance.application.dto.home.HomeFindRequest;
import com.beat.domain.performance.application.dto.home.HomeFindAllResponse;
import com.beat.domain.performance.domain.Genre;
import com.beat.domain.performance.exception.PerformanceSuccessCode;
import com.beat.global.common.dto.SuccessResponse;
import io.swagger.v3.oas.annotations.Operation;

import lombok.RequiredArgsConstructor;

import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
Expand All @@ -17,19 +18,19 @@
@RestController
@RequestMapping("/api/main")
@RequiredArgsConstructor
public class HomeController {

private final PerformanceService performanceService;

@Operation(summary = "전체공연목록, 홍보목록 조회 API", description = "홈화면에서 전체공연목록, 홍보목록을 조회하는 GET API입니다.")
@GetMapping
public ResponseEntity<SuccessResponse<HomeResponse>> getHomePerformanceList(@RequestParam(required = false) String genre) {
HomeRequest homeRequest;
if (genre != null) {
homeRequest = new HomeRequest(Genre.valueOf(genre));
} else {
homeRequest = new HomeRequest(null);
} HomeResponse homeResponse = performanceService.getHomePerformanceList(homeRequest);
return ResponseEntity.ok(SuccessResponse.of(PerformanceSuccessCode.HOME_PERFORMANCE_RETRIEVE_SUCCESS, homeResponse));
}
public class HomeController implements HomeApi {

private final HomeService homeService;

@Override
@GetMapping
public ResponseEntity<SuccessResponse<HomeFindAllResponse>> getHomePerformanceList(
@RequestParam(required = false) Genre genre) {

HomeFindRequest request = new HomeFindRequest(genre);

HomeFindAllResponse response = homeService.findHomePerformanceList(request);
return ResponseEntity.ok(
SuccessResponse.of(PerformanceSuccessCode.HOME_PERFORMANCE_RETRIEVE_SUCCESS, response));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package com.beat.domain.performance.application;

import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;

import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import com.beat.domain.performance.application.dto.home.HomeFindAllResponse;
import com.beat.domain.performance.application.dto.home.HomeFindRequest;
import com.beat.domain.performance.application.dto.home.HomePerformanceDetail;
import com.beat.domain.performance.application.dto.home.HomePromotionDetail;
import com.beat.domain.performance.dao.PerformanceRepository;
import com.beat.domain.performance.domain.Genre;
import com.beat.domain.performance.domain.Performance;
import com.beat.domain.promotion.domain.Promotion;
import com.beat.domain.promotion.port.in.PromotionUseCase;
import com.beat.domain.schedule.application.ScheduleService;

import lombok.RequiredArgsConstructor;

@Service
@RequiredArgsConstructor
public class HomeService {

private final ScheduleService scheduleService;
private final PromotionUseCase promotionUseCase;

private final PerformanceRepository performanceRepository;

@Transactional(readOnly = true)
public HomeFindAllResponse findHomePerformanceList(HomeFindRequest homeFindRequest) {

List<Performance> performances = findPerformancesByGenre(homeFindRequest);
List<HomePromotionDetail> promotions = findAllPromotionsSortedByCarouselNumber();

if (performances.isEmpty()) {
return HomeFindAllResponse.of(promotions, new ArrayList<>());
}

List<HomePerformanceDetail> sortedPerformances = performances.stream()
.map(performance -> {
int minDueDate = scheduleService.getMinDueDateForPerformance(performance.getId());
return HomePerformanceDetail.of(performance, minDueDate);
})
.sorted(Comparator.comparingInt(HomePerformanceDetail::dueDate)
.reversed()
.thenComparingInt(detail -> detail.dueDate() >= 0 ? -1 : 1))
.toList();
Copy link
Collaborator

@hyerinhwang-sailin hyerinhwang-sailin Oct 2, 2024

Choose a reason for hiding this comment

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

홈화면에서 공연 정렬 기준은 duedate가 양수일 경우 오름차순, 이후 음수는 내림차순입니다!
예를 들어 0, 2, 4, 15, -1, -4, -10 이런식으로요. 지금 코드는 양수인 경우에도 내림차순으로 정렬하고 있는 것 같습니다ㅠㅠ

List<HomePerformanceDetail> sortedPerformances = performances.stream()
    .map(performance -> {
        int minDueDate = scheduleService.getMinDueDateForPerformance(performance.getId());
        return HomePerformanceDetail.of(performance, minDueDate);
    })
    .sorted(Comparator.<HomePerformanceDetail>comparingInt(detail -> detail.dueDate() >= 0 ? 0 : 1)  // 양수는 0, 음수는 1로 구분
        .thenComparingInt(detail -> detail.dueDate() >= 0 ? detail.dueDate() : detail.dueDate()))  // 양수는 오름차순, 음수는 내림차순
    .toList();

이렇게 바꾸면 어떨까요?

로컬 테스트하실 때 과거 공연과 미래 공연 섞어서 홈화면 잘 정렬되는지 확인해주시면 좋을 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

제가 놓쳤던 부분이였네요!!

그런데, 지금 보내주신 코드도 음수 정렬의 경우 오름차순 정렬을 해주고 있어서 다음과 같이 반영하겠습니다!

List<HomePerformanceDetail> sortedPerformances = performances.stream()
	.map(performance -> {
		int minDueDate = scheduleService.getMinDueDateForPerformance(performance.getId());
		return HomePerformanceDetail.of(performance, minDueDate);
	})
	.sorted(Comparator.<HomePerformanceDetail>comparingInt(detail -> detail.dueDate() < 0 ? 1 : 0)
		.thenComparingInt(detail -> Math.abs(detail.dueDate())))
	.toList();

절댓값을 비교해서 정렬을 해주는 로직을 도입하여 양수는 그대로 오름차순, 음수는 절댓값을 씌워서 정렬하면 내림차순으로 정렬되는 점을 이용했습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

pr 내용에도 추가했습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 처음엔 절댓값을 씌우는 방식을 고려했는데 더 알아본 결과 comparator가 원래 양수는 오름차순, 음수는 내림차순으로 정렬하더라구요!
https://bono039.tistory.com/847
링크 참고해주세요!

Copy link
Member Author

@hoonyworld hoonyworld Oct 2, 2024

Choose a reason for hiding this comment

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

comparator는 양수는 오름차순, 음수는 내림차순으로 정렬하는 것이 아닌 양수든 음수든 결국 같이 오름차순 or 내림차순으로 정렬하는 로직입니다.

Comparator.compare(a, b)에서:

  • a - b가 양수일 경우: b가 a보다 작기 때문에 b가 앞에 오고 a는 뒤로 갑니다.
  • a - b가 음수일 경우: a가 b보다 작기 때문에 a는 앞에 남고 b가 뒤로 갑니다.
  • b - a가 양수일 경우: b가 a보다 크기 때문에 b가 앞에 오고 a는 뒤로 갑니다.
  • b - a가 음수일 경우: a가 b보다 크기 때문에 a가 앞에 오고 b는 뒤로 갑니다.

예를 들어, a=-2, b=-4라 가정할 때 a-b 로직을 사용하면 -2 - (-4) = +2 이므로 양수기 때문에 -4가 앞에오고 -2는 뒤로 가게 됩니다.
이때는 오름차순 정렬이 되게됩니다.

반면, a=-2, b=-4라 가정할 때 b-a 로직을 사용하면 -4 - (-2) = -2 이므로 음수기 때문에 -2가 앞에오고 -4는 뒤로 가게 됩니다.
이때는 내림차순 졍렬이 되게됩니다.

따라서 a-b냐 b-a냐에 따라 양수든 음수든 오름차순으로 혹은 내림차순으로 정렬이 되게됩니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

엇 그렇군요! 제가 잘못 알고 있었나보네요 감사합니다~


return HomeFindAllResponse.of(promotions, sortedPerformances);
}

private List<Performance> findPerformancesByGenre(HomeFindRequest homeFindRequest) {
Genre genre = homeFindRequest.genre();

if (genre != null) {
return performanceRepository.findByGenre(genre);
}

return performanceRepository.findAll();
}

private List<HomePromotionDetail> findAllPromotionsSortedByCarouselNumber() {
return promotionUseCase.findAllPromotions().stream()
.sorted(Comparator.comparing(Promotion::getCarouselNumber, Comparator.comparingInt(Enum::ordinal)))
.map(HomePromotionDetail::from)
.toList();
}
}
Loading
Loading