Skip to content
Open
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
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ build/
!**/src/main/**/build/
!**/src/test/**/build/
application*.yml
**/src/main/resources/static/swagger-ui/openapi3.yaml
src/main/resources/static/swagger-ui/openapi3.yaml

### STS ###
.apt_generated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
import jakarta.validation.constraints.Max;
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.NotBlank;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.Arrays;
import java.util.Objects;
import java.util.Collections;
import lombok.RequiredArgsConstructor;
import org.sopt.confeti.api.performance.dto.request.GetExpectedPerformanceRequest;
import org.sopt.confeti.api.performance.dto.response.ArtistPerformancesResponse;
Expand Down Expand Up @@ -144,9 +147,14 @@ public ResponseEntity<BaseResponse<?>> searchAutoComplete(
public ResponseEntity<BaseResponse<?>> getRecommendPerformanceId(
@UserId(require = false) Long userId
) {
RecommendMusicsPerformanceDTO recommendMusicsDTO = performanceFacade.getRecommendPerformanceId(userId);
return ApiResponseUtil.success(SuccessMessage.SUCCESS,
RecommendMusicsPerformanceResponse.from(recommendMusicsDTO));
Optional<RecommendMusicsPerformanceDTO> performanceDto = Objects.isNull(userId)
? performanceFacade.getRecommendPerformanceIdByRand()
: performanceFacade.getRecommendPerformanceIdByUserId(userId);
Comment on lines +150 to +152
Copy link
Member

Choose a reason for hiding this comment

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

삼항 연산자를 통해 직관적으로 파악할 수 있네요! 좋습니다


return performanceDto
.map(performance ->
ApiResponseUtil.success(SuccessMessage.SUCCESS, RecommendMusicsPerformanceResponse.from(performance)))
.orElseGet(()-> ApiResponseUtil.success(SuccessMessage.SUCCESS, Collections.emptyMap()));
Comment on lines +154 to +157
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return performanceDto
.map(performance ->
ApiResponseUtil.success(SuccessMessage.SUCCESS, RecommendMusicsPerformanceResponse.from(performance)))
.orElseGet(()-> ApiResponseUtil.success(SuccessMessage.SUCCESS, Collections.emptyMap()));
return ApiResponseUtil.success(SuccessMessage.SUCCESS,
performanceDto.map(RecommendMusicsPerformanceResponse::from)
.orElseGet(Collections.emptyMap())
);

이렇게 작성하는건 어떠신가요?
중복된 코드를 줄일 수 있을 것 같아요!

Comment on lines +156 to +157
Copy link
Contributor

Choose a reason for hiding this comment

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

orElse와 orElseGet의 차이를 인지하고 사용하신 부분이 좋네요 :)

}

@Permission(role = {Role.GENERAL})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
package org.sopt.confeti.api.performance.facade;

import java.time.LocalDate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Arrays;
import java.util.ArrayList;
import java.util.Optional;
import java.util.stream.Collectors;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -26,6 +27,7 @@
import org.sopt.confeti.api.performance.facade.dto.response.RecommendPerformancesDTO;
import org.sopt.confeti.api.performance.facade.dto.response.SearchACPerformancesDTO;
import org.sopt.confeti.api.performance.facade.dto.response.PerformanceIdsDTO;
import org.sopt.confeti.api.performance.vo.UserPerformanceRecordVO;
import org.sopt.confeti.domain.artist_favorite.ArtistFavorite;
import org.sopt.confeti.domain.artist_favorite.application.ArtistFavoriteService;
import org.sopt.confeti.domain.concert.Concert;
Expand All @@ -36,6 +38,7 @@
import org.sopt.confeti.domain.festival.Festival;
import org.sopt.confeti.domain.festival.application.FestivalService;
import org.sopt.confeti.domain.festival_favorite.application.FestivalFavoriteService;
import org.sopt.confeti.domain.setlist.SetlistType;
import org.sopt.confeti.domain.setlist.application.SetlistService;
import org.sopt.confeti.domain.timetable_festival.application.TimetableFestivalService;
import org.sopt.confeti.domain.user.application.UserService;
Expand All @@ -61,6 +64,8 @@ public class PerformanceFacade {

private static final int RECENT_PERFORMANCES_SIZE = 7;
private static final int RECOMMEND_MUSIC_SIZE = 3;
private static final int MUSIC_FETCH_SIZE = 5;
private static final int SELECTED_ARTISTS_SIZE = 3;
private static final boolean PERSONALIZED = true;
private static final boolean UNPERSONALIZED = false;

Expand Down Expand Up @@ -261,98 +266,89 @@ public SearchACPerformancesDTO searchACPerformances(String term, int limit, Perf
);
}

@Transactional(readOnly = true)
public RecommendMusicsPerformanceDTO getRecommendPerformanceId(final Long userId) {

Performance performance = performanceService.getPerformanceByUserFavorites(userId);
if (userId == null || performance == null) {
performance = performanceService.getPerformanceByRand();
}

return RecommendMusicsPerformanceDTO.from(performance);
public Optional<RecommendMusicsPerformanceDTO> getRecommendPerformanceIdByRand() {
return performanceService.getPerformanceByRand()
.map(RecommendMusicsPerformanceDTO::from);
}

protected Set<String> setArtistsByRandom(Performance performance) {
List<PerformanceArtist> performanceArtists = performance.getArtists();

if (performanceArtists.isEmpty()) {
return Collections.emptySet();
}

List<String> artistList = performanceArtists.stream()
.map(PerformanceArtist::getArtistId).distinct().collect(Collectors.toList());
Collections.shuffle(artistList);

int artistCount = Math.min(artistList.size(), 3);
return new HashSet<>(artistList.subList(0, artistCount));
public Optional<RecommendMusicsPerformanceDTO> getRecommendPerformanceIdByUserId(final Long userId) {
return performanceService.getPerformanceByUserFavorites(userId)
.map(RecommendMusicsPerformanceDTO::from);
}

@Transactional(readOnly = true)
public RecommendMusicsDTO getNewRecommendMusics(long performanceId, List<String> musicIds) {
Performance performance = performanceService.getPerformanceById(performanceId);
Set<String> selectedArtistIds = setArtistsByRandom(performance);
Set<String> selectedArtistIds = selectRandomArtistIds(performance);
Copy link
Member

Choose a reason for hiding this comment

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

무작위 아티스트 아이디를 추출할 때 애플리케이션 레벨, 데이터베이스 레벨 각각 어떤 장단점이 있었고 애플리케이션 레벨 구현을 선택하신 이유가 궁금합니다!

Copy link
Member

Choose a reason for hiding this comment

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

추가로 아티스트 아이디를 가져올 때 서비스에 요청하지 않고 트랜잭션 외부에서 공연 엔티티에서 추출하신 이유가 궁금합니다!

Set<String> existingMusicIds = (musicIds == null || musicIds.isEmpty())
? Collections.emptySet()
: musicIds.stream().flatMap(ids -> Arrays.stream(ids.split(",")))
.map(String::trim).collect(Collectors.toSet());
: musicIds.stream()
.flatMap(ids -> Arrays.stream(ids.split(",")))
.map(String::trim)
.collect(Collectors.toSet());

List<String> artistIdList = new ArrayList<>(selectedArtistIds);
Copy link
Member

Choose a reason for hiding this comment

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

복수형 (-s, -List) 중 하나만 작성하시면 일관성 측면에서 더 좋을 것 같아요!

List<ConfetiMusic> recommendMusics = recommendMusicsByArtistCount(artistIdList, existingMusicIds);

return RecommendMusicsDTO.from(recommendMusics);
}

private List<ConfetiMusic> recommendMusicsByArtistCount(List<String> artistIdList, Set<String> existingMusicIds) {
int artistCount = artistIdList.size();
if (artistCount == 1) {
return recommendForSingleArtist(artistIdList, existingMusicIds);
}
if (artistCount == 2) {
return recommendForTwoArtists(artistIdList, existingMusicIds);
protected Set<String> selectRandomArtistIds(Performance performance) {
List<PerformanceArtist> performanceArtists = performance.getArtists();

Comment on lines +296 to +298
Copy link
Contributor

Choose a reason for hiding this comment

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

별도의 메서드로 분리하신 부분 좋은 것 같아요 :)

if (performanceArtists.isEmpty()) {
return Collections.emptySet();
}
return recommendForMultipleArtists(artistIdList, existingMusicIds);
}

private List<ConfetiMusic> recommendForSingleArtist(List<String> artistIdList, Set<String> existingMusicIds) {
return musicAPIHandler.getFilteredTopSongsByArtist(
artistIdList.getFirst(), RECOMMEND_MUSIC_SIZE, existingMusicIds
);
}
List<String> artistList = performanceArtists.stream()
.map(PerformanceArtist::getArtistId).distinct().collect(Collectors.toList());
Collections.shuffle(artistList);

private List<ConfetiMusic> recommendForTwoArtists(List<String> artistIdList, Set<String> existingMusicIds) {
List<ConfetiMusic> musics = new ArrayList<>();
musics.addAll(musicAPIHandler.getFilteredTopSongsByArtist(artistIdList.getFirst(), 2, existingMusicIds));
musics.addAll(musicAPIHandler.getFilteredTopSongsByArtist(artistIdList.getLast(), 1, existingMusicIds));
return musics;
int artistCount = Math.min(artistList.size(), SELECTED_ARTISTS_SIZE);
return new HashSet<>(artistList.subList(0, artistCount));
}

private List<ConfetiMusic> recommendForMultipleArtists(List<String> artistIdList, Set<String> existingMusicIds) {
private List<ConfetiMusic> recommendMusicsByArtistCount(List<String> artistIdList, Set<String> existingMusicIds) {
List<ConfetiMusic> musics = new ArrayList<>();
for (String artistId : artistIdList) {
if (musics.size() >= RECOMMEND_MUSIC_SIZE) {
break;
Set<String> seenMusicIds = new HashSet<>(existingMusicIds);
int round = 0;

while (musics.size() < RECOMMEND_MUSIC_SIZE && round < MUSIC_FETCH_SIZE) {
Copy link
Member

Choose a reason for hiding this comment

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

round의 limit 값이랑 음악 조회 개수의 값이 같은 상수를 사용하는건가요?
따로 작성하지 않으신 이유가 궁금합니다.

제 의견은 매직 넘버를 상수화하신 것이라면, 이름을 따로 두어야 한다고 생각해요.
매직 넘버를 상수화하는 것은 이름으로 해당 상수 값이 어떤걸 의미하는지 직관적으로 나타낼 때 사용하기 때문이에요.

Copy link
Contributor

Choose a reason for hiding this comment

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

동의합니다!!! MAX_FETCH_ATTEMPTS 같은 단어로 바꾸는 것도 좋을 것 같네요 ㅎㅎ

for (String artistId : artistIdList) {
if (musics.size() >= RECOMMEND_MUSIC_SIZE) break;

List<ConfetiMusic> topSongs = musicAPIHandler.getFilteredTopSongsByArtist(
artistId, MUSIC_FETCH_SIZE, seenMusicIds
);

if (round < topSongs.size()) {
ConfetiMusic song = topSongs.get(round);
if (!seenMusicIds.contains(song.getId())) {
musics.add(song);
seenMusicIds.add(song.getId());
}
}
}
musics.addAll(musicAPIHandler.getFilteredTopSongsByArtist(artistId, 1, existingMusicIds));
round++;
}

return musics;
}

@Transactional(readOnly = true)
public ConfetiRecordDTO getConfetiRecord(final long userId) {
validateExistUser(userId);

List<Long> timetableFestivalIds = timetableFestivalService.findFestivalIdsByUserId(userId);
List<Long> setListFestivalIds = setlistService.findFestivalIdsByUserId(userId);
List<Long> setListConcertIds = setlistService.findConcertIdsByUserId(userId);
List<Long> setListFestivalIds = setlistService.findMusicIdsByUserId(userId, SetlistType.FESTIVAL);
List<Long> setListConcertIds = setlistService.findMusicIdsByUserId(userId, SetlistType.CONCERT);

Set<Long> uniqueFestivalIds = new HashSet<>();
uniqueFestivalIds.addAll(timetableFestivalIds);
uniqueFestivalIds.addAll(setListFestivalIds);

int totalCount = uniqueFestivalIds.size() + setListConcertIds.size();
UserPerformanceRecordVO record = new UserPerformanceRecordVO(
timetableFestivalIds,
setListFestivalIds,
setListConcertIds
);

return ConfetiRecordDTO.of(totalCount, timetableFestivalIds.size(),
setListFestivalIds.size() + setListConcertIds.size());
return ConfetiRecordDTO.from(record);
}

protected void validateExistUser(final long userId) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package org.sopt.confeti.api.performance.facade.dto.response;

import org.sopt.confeti.api.performance.vo.UserPerformanceRecordVO;

public record ConfetiRecordDTO(
long totalCount,
long timetableCount,
long setlistCount
) {
public static ConfetiRecordDTO of(final long totalCount, final long timetableCount, final long setlistCount) {
public static ConfetiRecordDTO from(UserPerformanceRecordVO record) {
return new ConfetiRecordDTO(
totalCount, timetableCount, setlistCount
record.getTotalUniquePerformanceCount(),
record.getTimetableFestivalCount(),
record.getSetListPerformanceCount()
);
}
}
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.sopt.confeti.api.performance.vo;

import java.util.HashSet;
import java.util.List;
import java.util.Set;

public record UserPerformanceRecordVO(
List<Long> timetableFestivalIds,
List<Long> setListFestivalIds,
List<Long> setListConcertIds
) {
public int getTotalUniquePerformanceCount() {
Set<Long> uniqueFestivalIds = new HashSet<>(timetableFestivalIds);
uniqueFestivalIds.addAll(setListFestivalIds);
return uniqueFestivalIds.size() + setListConcertIds.size();
}

public int getTimetableFestivalCount() {
return timetableFestivalIds.size();
}

public int getSetListPerformanceCount() {
return setListFestivalIds.size() + setListConcertIds.size();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import jakarta.validation.constraints.Min;
import java.util.Collections;
import java.util.Optional;
import lombok.RequiredArgsConstructor;
import org.sopt.confeti.api.user.dto.response.UpcomingPerformanceResponse;
import org.sopt.confeti.api.user.dto.response.UserFavoriteArtistsPreviewResponse;
Expand Down Expand Up @@ -137,12 +138,11 @@ public ResponseEntity<BaseResponse<?>> getFavoritePerformancesAll(
public ResponseEntity<BaseResponse<?>> getUpcomingPerformance(
@UserId Long userId
) {
UpcomingPerformanceDTO upcomingPerformanceDTO = userFavoriteFacade.getUpcomingPerformance(userId);
if (upcomingPerformanceDTO == null) {
return ApiResponseUtil.success(SuccessMessage.SUCCESS, Collections.emptyMap());
}
return ApiResponseUtil.success(SuccessMessage.SUCCESS,
UpcomingPerformanceResponse.of(upcomingPerformanceDTO, s3FileHandler));
Optional<UpcomingPerformanceDTO> upcomingPerformanceDTO = userFavoriteFacade.getUpcomingPerformance(userId);
return upcomingPerformanceDTO
.map(performanceDTO ->
ApiResponseUtil.success(SuccessMessage.SUCCESS, UpcomingPerformanceResponse.of(performanceDTO, s3FileHandler)))
.orElseGet(() -> ApiResponseUtil.success(SuccessMessage.SUCCESS, Collections.emptyMap()));
}

@Permission(role = {Role.GENERAL})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.sopt.confeti.api.user.controller;

import org.sopt.confeti.global.exception.ConfetiException;
import org.sopt.confeti.global.message.ErrorMessage;

import java.util.Arrays;

public enum UserFavoriteSortType {
CREATED_AT("createdAt"),
ALPHABETICALLY("alphabetically");

private final String value;

UserFavoriteSortType(String value) {
this.value = value;
}

public static UserFavoriteSortType from(String value) {
return Arrays.stream(values())
.filter(type -> type.value.equalsIgnoreCase(value))
.findFirst()
.orElseThrow(() -> new ConfetiException(ErrorMessage.BAD_REQUEST));
}

public String getValue() {
return value;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.sopt.confeti.api.user.controller;

import org.sopt.confeti.global.exception.ConfetiException;
import org.sopt.confeti.global.message.ErrorMessage;

import java.util.Arrays;

public enum UserTimetableSortType {
OLDEST_FIRST("oldestFirst"),
CREATED_AT("createdAt");

private final String value;

UserTimetableSortType(String value) {
this.value = value;
}

public static UserTimetableSortType from(String value) {
return Arrays.stream(values())
.filter(type -> type.value.equalsIgnoreCase(value))
.findFirst()
.orElseThrow(() -> new ConfetiException(ErrorMessage.BAD_REQUEST));
}

public String getValue() {
return value;
}
}
Loading