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

#317 [feat] 엔티티 변경으로 인한 종합 일정 시간표 로직 리팩토링 #324

Merged
merged 14 commits into from
Aug 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ public interface UserRepository extends Repository<User, Long>, UserRepositoryCu

Optional<User> findById(final Long id);

List<User> findAllByIdIn(final List<Long> ids);

List<User> findByMeeting(final Meeting meeting);

int countByMeeting(final Meeting meeting);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ public SuccessResponse<TimeTableResponseDto> getTimeTable(
@MeetingPathVariable final Long meetingId,
@UserId final Long userId
) {
return SuccessResponse.success(Success.FIND_TIME_TABLE_SUCCESS, meetingService.getTimeTable(userId, meetingId));
return SuccessResponse.success(
Success.FIND_TIME_TABLE_SUCCESS,
TimeTableResponseDto.of(meetingRetrieveService.getTimeTable(userId, meetingId))
);
}

@GetMapping("/{meetingId}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,22 @@
import com.asap.server.service.meeting.dto.UserDto;
import com.asap.server.service.time.MeetingTimeRecommendService;
import com.asap.server.service.time.UserMeetingScheduleService;
import com.asap.server.service.time.dto.retrieve.AvailableDatesRetrieveDto;
import com.asap.server.service.time.dto.retrieve.TimeSlotRetrieveDto;
import com.asap.server.service.time.dto.retrieve.TimeTableRetrieveDto;
import com.asap.server.service.time.vo.BestMeetingTimeVo;
import com.asap.server.service.time.vo.BestMeetingTimeWithUsers;
import com.asap.server.service.time.vo.TimeBlockVo;
import com.asap.server.service.user.UserRetrieveService;

import java.time.LocalDate;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

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

@Service
@RequiredArgsConstructor
Expand Down Expand Up @@ -81,4 +89,67 @@ private BestMeetingTimeWithUsers mapToBestMeetingTimeWithUsers(
userDtos
);
}

@Transactional(readOnly = true)
public TimeTableRetrieveDto getTimeTable(final Long meetingId, final Long userId) {
Meeting meeting = meetingRepository.findById(meetingId)
.orElseThrow(() -> new NotFoundException(Error.MEETING_NOT_FOUND_EXCEPTION));

if (!meeting.authenticateHost(userId)) {
throw new UnauthorizedException(Error.INVALID_MEETING_HOST_EXCEPTION);
}
if (meeting.isConfirmedMeeting()) {
throw new ConflictException(MEETING_VALIDATION_FAILED_EXCEPTION);
}

List<String> userNames = userRetrieveService.getUsersFromMeetingId(meetingId).stream().map(User::getName).toList();

return TimeTableRetrieveDto.of(userNames, getAvailableDatesDto(meetingId, userNames.size()));

}

private List<AvailableDatesRetrieveDto> getAvailableDatesDto(final Long meetingId, final int totalUserCount) {
List<TimeBlockVo> timeBlockVos = userMeetingScheduleService.getTimeBlocks(meetingId);

Map<LocalDate, List<TimeSlotRetrieveDto>> timeSlotDtoMappedByDate = getTimeTableMapFromTimeBlockVo(timeBlockVos, totalUserCount);

return timeSlotDtoMappedByDate.keySet().stream().map(
date -> AvailableDatesRetrieveDto.of(
date,
timeSlotDtoMappedByDate.get(date)
)
).toList();
}

private Map<LocalDate, List<TimeSlotRetrieveDto>> getTimeTableMapFromTimeBlockVo(final List<TimeBlockVo> timeBlockVo, final int totalUserCount) {
return timeBlockVo.stream()
.collect(Collectors.groupingBy(
TimeBlockVo::availableDate,
Collectors.mapping(t -> new TimeSlotRetrieveDto(
t.timeSlot().getTime(),
userRetrieveService.getUserNamesFromId(t.userIds()),
setColorLevel(totalUserCount, t.userIds().size())
),
Collectors.toList()
)
Copy link
Member

Choose a reason for hiding this comment

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

P3

timeBlock 별로 userRetrieveService.getUserNamesFromId(t.userIds()),을 한다면,
timeBlock 별로 쿼리가 발생해서
최대 266개의 쿼리가 발생하지 않을까 우려가 됩니다..!

그래서 저는 일전에 최적의 회의 시간 로직 리팩토링할 때, 유저를 전체 조회한 후 Map<Long, User>을 통해서 user정보를 가지고 왔었습니다..!

혹시 쿼리가 얼마나 생기는지 알 수 있을까요??

userNames 를 구할 때, 영속성의 도움을 받아서 해당 쿼리가 발생하지 않는 것인가? 란 추측도 하는 중인데.. 실행해보지 않아서 모르겠네요..!

Copy link
Member Author

Choose a reason for hiding this comment

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

꼼꼼히 봐주셔서 감사합니다!! 쿼리문이 최대 252개가 나갈 수 있는 상황이었더라고요! 저도 원용님 방식으로 수정하였습니다!
그런데, Map을 사용할경우 정렬이 보장이 되지 않아 테스트를 통과하지 않아서 이를 받아와 sorted하는 방식을 사용했습니다! 리뷰 부탁드립니다!

));
}

public int setColorLevel(final int memberCount, final int availableUserCount) {
double ratio = (double) availableUserCount / memberCount;
Copy link
Member

Choose a reason for hiding this comment

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

p2

접근자가 public입니다! 해당 클래스에서만 사용하는 것 같으니 private으로 바꿔주세요!


if (ratio <= 0.2) {
return 1;
} else if (ratio <= 0.4) {
return 2;
} else if (ratio <= 0.6) {
return 3;
} else if (ratio <= 0.8) {
return 4;
} else if (ratio <= 1.0) {
return 5;
} else {
return 0;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
import com.asap.server.persistence.domain.Meeting;
import com.asap.server.persistence.domain.user.User;
import com.asap.server.persistence.repository.user.UserRepository;

import java.util.Map;
import java.util.List;
import java.util.stream.Collectors;

import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;

Expand All @@ -22,4 +25,12 @@ public Map<Long, User> getUserIdToUserMap(final Long meetingId) {
.findAllByMeetingId(meetingId).stream()
.collect(Collectors.toMap(User::getId, user -> user));
}

public List<String> getUserNamesFromId(final List<Long> userIds) {
return userRepository.findAllByIdIn(userIds).stream().map(User::getName).toList();
}

public List<User> getUsersFromMeetingId(final Long meetingId) {
return userRepository.findAllByMeetingId(meetingId);
}
}