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] 일정 조율 기능을 제거한다. #630

Conversation

hyeonic
Copy link
Collaborator

@hyeonic hyeonic commented Sep 20, 2022

  • 🔀 PR 제목의 형식을 잘 작성했나요? e.g. [feat] PR을 등록한다.
  • 💯 테스트는 잘 통과했나요?
  • 🏗️ 빌드는 성공했나요?
  • 🧹 불필요한 코드는 제거했나요?
  • 💭 이슈는 등록했나요?
  • 🏷️ 라벨은 등록했나요?
  • 💻 git rebase를 사용했나요?
  • 🌈 알록달록한가요?

작업 내용

드디어 composition의 모든 서비스를 제거 하였습니다... 다만 워낙에 일정 조율 로직이 복잡하고 많은 쿼리와 네트워크 통신을 유발해서 우리 서비스에서 정상적으로 동작할 수 있을지 조금 걱정이 되네요. 테스트 코드 까지 꼼꼼하게 짜 보려 했는데 외부 리소스 관련 Stub 때문에 테스트가 매우 부실한 상태입니다. 이 점 인지해주시고, 우선적으로 코드 스타일 부터 확인해주시면 감사하겠습니다.

테스트는 추후 이야기 나눠보고 보충할 예정이고, 아직 쿼리 분석은 못했습니다. 시간이 좀 더 소요될 것 같아서 내일 오전 중으로 진행해야 할 것 같네요.

이슈 내용 중 일부

일정 조율 기능을 제거한다. 일정 조율 기능은 달록 내부에 저장된 일정과 구독한 구글 일정을 모두 조회하여 가능한 일정을 계산 해준다. 하지만 구글에 직접적인 요청을 통해 일정 정보를 조회해야 하기 때문에 구독한 구글 캘린더가 많을 수록 더 많은 네트워크 요청이 일어난다.

만약 100명의 사용자가 공통 일정을 구독 했다고 가장한다. 각각의 사용자는 구글 계정을 연동한 뒤 각각 2개의 구글 캘린더 정보를 구독하였다. 이때 공통 일정에 대한 일정 조율을 진행할 경우 대략 200번의 구글 요청이 들어간다. 하나의 기능에 너무 많은 네트워크 시간이 소요될 것이다. 우리 달록은 이러한 문제점을 인지하고 해당 기능을 개선하기 보단 제거한 뒤 다른 부분에 더 많은 힘을 쏟기로 결정하였다.

스크린샷

주의사항

Closes #615

@hyeonic hyeonic added backend 백엔드 feature 새로운 기능 refactor 리팩터링 labels Sep 20, 2022
@hyeonic hyeonic self-assigned this Sep 20, 2022
Copy link
Collaborator Author

@hyeonic hyeonic left a comment

Choose a reason for hiding this comment

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

간단히 메서드에 의도 적어두었습니다. 참고해주시면 감사하겠습니다.

일정 조율의 경우 너무 많은 경우의 수를 가지기 때문에 홀로 테스트하는데 많은 어려움이 있었습니다. 저희끼리 좀 더 다양한 시나리오를 구성한 뒤 함께 테스트 코드를 작성해보는 시간을 가져보는 건 어떨까요?? 의견 남겨 주시면 감사하겠습니다!

Comment on lines 38 to 51
public List<PeriodResponse> getAvailablePeriods(final Long categoryId, final DateRangeRequest request) {
AvailablePeriodMaterial material = scheduleService.findInMembersByCategoryIdAndDateRange(categoryId, request);

String startDateTime = request.getStartDateTime().format(DateTimeFormatter.ofPattern(DATE_FORMAT));
String endDateTime = request.getEndDateTime().format(DateTimeFormatter.ofPattern(DATE_FORMAT));

List<IntegrationSchedule> schedules = material.getInternalSchedules();
List<IntegrationSchedule> externalSchedules = toExternalSchedules(startDateTime, endDateTime, material);

schedules.addAll(externalSchedules);
Scheduler scheduler = new Scheduler(schedules, request.getStartDateTime(), request.getEndDateTime());

return toPeriodResponses(scheduler);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

가능한 구간을 조회하기 위한 메서드입니다.

AvailablePeriodMaterial material = scheduleService.findInMembersByCategoryIdAndDateRange(categoryId, request);

해당 부분에서 트랜잭션이 마무리 되고, 이후에는 트랜잭션 외부에서 진행됨을 인지해주세요!

Comment on lines 53 to 61
private List<IntegrationSchedule> toExternalSchedules(final String startDateTime, final String endDateTime,
final AvailablePeriodMaterial material) {
return material.getTokenByExternalIds()
.entrySet()
.stream()
.map(entry -> toExternalSchedules(entry, startDateTime, endDateTime))
.flatMap(Collection::stream)
.collect(Collectors.toList());
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

외부 access token 조회를 위한 refresh token 별 외부 캘린더 id를 기반으로 외부 캘린더 리스트를 가져온 뒤 합체합니다.

Comment on lines 63 to 75
private List<IntegrationSchedule> toExternalSchedules(final Entry<String, List<ExternalRequestMaterial>> entry,
final String startDateTime, final String endDateTime) {
String refreshToken = entry.getKey();
List<ExternalRequestMaterial> externalRequestMaterials = entry.getValue();

String accessToken = oAuthClient.getAccessToken(refreshToken).getAccessToken();

return externalRequestMaterials.stream()
.map(externalRequestMaterial ->
toExternalSchedules(accessToken, externalRequestMaterial, startDateTime, endDateTime))
.flatMap(Collection::stream)
.collect(Collectors.toList());
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

토큰을 조회하고 실제 외부 서버에게 조회된 일정 리스트를 합체하여 반환합니다.

Comment on lines 77 to 85
private List<IntegrationSchedule> toExternalSchedules(final String accessToken,
final ExternalRequestMaterial externalRequestMaterial,
final String startDateTime, final String endDateTime) {
Long internalCategoryId = externalRequestMaterial.getInternalCategoryId();
String externalCalendarId = externalRequestMaterial.getExternalCalendarId();

return externalCalendarClient.getExternalCalendarSchedules(
accessToken, internalCategoryId, externalCalendarId, startDateTime, endDateTime);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

실제 외부 서버에게 일정 리스트를 조회하는 부분입니다.

Comment on lines 94 to 108
public AvailablePeriodMaterial findInMembersByCategoryIdAndDateRange(final Long categoryId,
final DateRangeRequest request) {
List<Long> memberIds = toMemberIds(categoryId);
List<Subscription> subscriptions = subscriptionRepository.findByMemberIdIn(memberIds);

List<IntegrationSchedule> internalSchedules = toInternalSchedules(subscriptions, request);
Map<String, List<ExternalRequestMaterial>> tokenByExternalIds = toTokenByExternalIds(subscriptions);

return new AvailablePeriodMaterial(internalSchedules, tokenByExternalIds);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

특정 카테고리에 대한 내부 일정 리스트와 외부 캘린더 일정 조회를 위한 정보를 dto로 만들어 반환합니다.

Comment on lines 113 to 125
private List<IntegrationSchedule> toInternalSchedules(final List<Subscription> subscriptions,
final DateRangeRequest request) {
List<Category> internalCategories = toInternalCategories(subscriptions);

LocalDateTime startDate = request.getStartDateTime();
LocalDateTime endDate = request.getEndDateTime();
return scheduleRepository.getByCategoriesAndBetween(internalCategories, startDate, endDate);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

구독 정보에서 내부 카테고리 리스트 정보를 조회하여 일정 리스트를 반환합니다.

Comment on lines 129 to 139
private Map<String, List<ExternalRequestMaterial>> toTokenByExternalIds(final List<Subscription> subscriptions) {
return subscriptions.stream()
.filter(Subscription::hasExternalCategory)
.map(this::toExternalRequestMaterial)
.collect(Collectors.groupingBy(ExternalRequestMaterial::getRefreshToken));
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

refresh token 별 외부 캘린더 조회를 위한 메타 데이터를 그룹핑합니다.

간단한 예시로 제가 구글 개인 일정 및 우테코 일정을 구글에서 등록한 뒤 구독 했다고 가정합니다. 제 refresh token 하나 당 두 개의 외부 캘린더 id를 기반으로 모두 조회해야 하기 때문에 그룹핑을 활용 했습니다.

Comment on lines 9 to 27
public class AvailablePeriodMaterial {

private final List<IntegrationSchedule> internalSchedules;
private final Map<String, List<ExternalRequestMaterial>> tokenByExternalIds;

public AvailablePeriodMaterial(final List<IntegrationSchedule> internalSchedules,
final Map<String, List<ExternalRequestMaterial>> tokenByExternalIds) {
this.internalSchedules = new ArrayList<>(internalSchedules);
this.tokenByExternalIds = new HashMap<>(tokenByExternalIds);
}

public List<IntegrationSchedule> getInternalSchedules() {
return new ArrayList<>(internalSchedules);
}

public Map<String, List<ExternalRequestMaterial>> getTokenByExternalIds() {
return new HashMap<>(tokenByExternalIds);
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

앞서 언급한 것 처럼 가능한 일정 구간 조회를 위한 정보를 담은 dto입니다. 이렇게 추가적인 dto를 생성한 이유는 트랜잭션 구간 내에서 최대한 많은 DB 정보를 조회한 뒤 이를 기반으로 외부 리소스에 관련된 처리를 진행하기 위함입니다.

@hyeonic hyeonic force-pushed the feature/615-precedence-service-refactoring branch 2 times, most recently from bc6ec30 to 769f4be Compare September 22, 2022 06:32
@hyeonic hyeonic changed the title [refactor] 상위 서비스 로직을 개선한다. [refactor] 일정 조율 기능을 제거한다. Sep 22, 2022
@hyeonic hyeonic force-pushed the feature/615-precedence-service-refactoring branch from f149bc0 to 1818e09 Compare September 22, 2022 06:45
Copy link
Collaborator

@devHudi devHudi left a comment

Choose a reason for hiding this comment

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

안녕... 조율.. 😭
아직 조율 기능을 덜 보내준 곳이 있어서 코멘트 달았습니다!

Comment on lines 9 to 27
public class AvailablePeriodMaterial {

private final List<IntegrationSchedule> internalSchedules;
private final Map<String, List<ExternalRequestMaterial>> tokenByExternalIds;

public AvailablePeriodMaterial(final List<IntegrationSchedule> internalSchedules,
final Map<String, List<ExternalRequestMaterial>> tokenByExternalIds) {
this.internalSchedules = new ArrayList<>(internalSchedules);
this.tokenByExternalIds = new HashMap<>(tokenByExternalIds);
}

public List<IntegrationSchedule> getInternalSchedules() {
return new ArrayList<>(internalSchedules);
}

public Map<String, List<ExternalRequestMaterial>> getTokenByExternalIds() {
return new HashMap<>(tokenByExternalIds);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

일정 조율 기능이 제거되어도 이 클래스는 사용되나요? 현재는 사용하고 있는 곳이 없는 것 같아요!

Copy link
Collaborator

@devHudi devHudi left a comment

Choose a reason for hiding this comment

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

LGTM!!

Copy link
Collaborator

@summerlunaa summerlunaa left a comment

Choose a reason for hiding this comment

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

유리야... 잘가... 행복했다😥

@hyeonic hyeonic force-pushed the feature/615-precedence-service-refactoring branch from 9bc197f to fac6d23 Compare September 22, 2022 10:20
Copy link
Collaborator

@gudonghee2000 gudonghee2000 left a comment

Choose a reason for hiding this comment

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

매트 안녕하세요! 작성 해주신 코드 잘보았습니다~
조유리가 사라지면서 많은 코드들이 사라졌군요..!
코드가 많이 사라져서 좋으면서도 조유리와 이별해서 아쉽네요😔

바로 approve하겠습니다!

@hyeonic hyeonic merged commit fa17b77 into woowacourse-teams:develop Sep 22, 2022
@hyeonic hyeonic deleted the feature/615-precedence-service-refactoring branch September 22, 2022 10:22
@hyeonic hyeonic mentioned this pull request Sep 22, 2022
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend 백엔드 feature 새로운 기능 refactor 리팩터링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants