[team-32][BE][산토리 & 포키] IssueTracker 5차 PR #237
Conversation
- 해당 이슈 id들을 리스트로 받습니다.
- issues에 빈 리스트로 초기화 - progressRate 소수점 3째자리까지 반환하도록 변경
There was a problem hiding this comment.
IssueService 를 비롯한 다양한 Service에서 findxxx()와 같은 Optional 처리를 한 다음 엔티티를 찾아오는 메서드가 중복됩니다.
이런 중복되는 코드들을 효과적으로 관리할 방법이 있을까요? 별도의 조회용 메서드만 모아놓은 서비스를 주입받는 방법도 있을 것 같긴한데
적절한 방법일지 궁금합니다.
서비스레이어에서 여러 서비스간에 엔티티 조회시 중복되는 로직이 있다면 엔티티를 기준으로 adapter Layer를 하나더 만드셔도 괜찮습니다.
예를 들어 Reader Layer를 만들어서 IssueReader 빈을 만들고 해당 클래스에 findIssue를 구현한뒤 다른 서비스에서 이 reader를 주입받아 사용하면 중복코드를 줄일 수 있겠죠?
| @Column(columnDefinition = "BOOLEAN", nullable = false) | ||
| private boolean isDeleted; |
| public class GithubProperty { | ||
|
|
||
| private final String clientId; | ||
| private final String clientSecret; | ||
| private final String redirectUrl; | ||
| private final String accessTokenUrl; | ||
| private final String resourceUrl; | ||
|
|
||
| public GithubProperty(String clientId, String clientSecret, String redirectUrl, String accessTokenUrl, String resourceUrl) { | ||
| this.clientId = clientId; | ||
| this.clientSecret = clientSecret; | ||
| this.redirectUrl = redirectUrl; | ||
| this.accessTokenUrl = accessTokenUrl; | ||
| this.resourceUrl = resourceUrl; | ||
| } | ||
| } |
| @Bean | ||
| public EnumMap<LoginType, OAuthProvider> oAuthProviderEnumMap(List<OAuthProvider> oAuthProviders) { | ||
| return new EnumMap<>(oAuthProviders.stream() | ||
| .collect(Collectors.toMap(OAuthProvider::getOAuthType, u -> u))); |
There was a problem hiding this comment.
꼭 EnumMap으로 만들필요가 있을까요? enummap이 가져다주는 어떤 장점이 있는지 고려해보시면 좋을 것 같습니다 ㅎㅎ
| @Query("select distinct m from Milestone m join fetch m.issues where m.isDeleted=false") | ||
| List<Milestone> findAll(); |
There was a problem hiding this comment.
soft-delete를 사용하면 모든 쿼리에 is_deleted = false인 조건을 추가해줘야하는 불편함이 생깁니다.
테이블에 인덱스를 설정해줘야하는 것도 필요하겠죠?
현재는 deleted된 리소스를 조회할 필요성은 보이지 않기에 엔티티에 @where 어노테이션을 사용하면 default로 isDeleted 조건을 추가할 수 있습니다.!
| } | ||
|
|
||
| @GetMapping("/count") | ||
| @Operation(summary = "마일스톤 개수 조회하기", description = "현재 마일스톤의 개수를 출력합니다.") | ||
| public MilestoneCountDto getCount() { | ||
| return null; | ||
| return milestoneService.count(); |
There was a problem hiding this comment.
마일스톤의 카운트만 따로 조회하는 API가 꼭 필요할까요?
목록조회와 같은 화면에서 사용되는 API라면 목록의 갯수를 카운트로 활용하는 방법이 불필요한 호출을 줄이는 방법이 될 수 있을 것 같네요.
| public MilestonesResponseDto findAll() { | ||
|
|
||
| return new MilestonesResponseDto(milestoneRepository.findAll() | ||
| .stream() | ||
| .map(milestone -> | ||
| new MilestoneResponseDto( | ||
| milestone.getName(), | ||
| milestone.getDueDate(), | ||
| milestone.getDescription(), | ||
| milestone.getInformation()) | ||
| ).collect(Collectors.toList())); | ||
| } |
| if (issueSaveRequestDto.getAssignees() != null) { | ||
| List<IssueAssignee> issueAssignees = createIssueAssignees(issueSaveRequestDto.getAssignees(), issue); | ||
| issue.updateAssignees(issueAssignees); | ||
| } | ||
|
|
||
| if (issueSaveRequestDto.getLabelNames() != null) { | ||
| List<IssueLabel> labels = createIssueLabels(issueSaveRequestDto.getLabelNames(), issue); | ||
| issue.updateLabels(labels); | ||
| } |
There was a problem hiding this comment.
request로 assignee와 label의 id값을 받을 수 있지 않을까요?
| List<IssueLabel> labels = createIssueLabels(issueLabelEditRequestDto.getLabelNames(), issue); | ||
|
|
||
| issue.updateLabels(labels); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void editMilestone(long issueId, IssueMilestoneEditRequestDto issueMilestoneEditRequestDto) { | ||
| Issue issue = findIssue(issueId); | ||
|
|
||
| Milestone milestone = findMilestone(issueMilestoneEditRequestDto.getMilestoneName()); | ||
|
|
There was a problem hiding this comment.
label과 milestone의 이름으로 찾기보다 id로 받을 수 있다면 id 찾는게 좋지 않을까요?
| protected void changeDeleted(boolean isDeleted) { | ||
| this.isDeleted = isDeleted; | ||
| } |
| } | ||
|
|
||
| @Operation(summary = "이슈의 제목을 편집하기", description = "기존 이슈의 제목을 편집합니다.") | ||
| @PutMapping("/title/{id}") | ||
| public IssueDetailDto editTitle(@PathVariable(value = "id") long issueId, @RequestBody IssueTitleEditRequestDto issueTitleEditRequestDto) { | ||
| return null; | ||
| public ResponseEntity<Void> editTitle(@Auth Long userId, |
There was a problem hiding this comment.
저번에 리뷰드린바와 같이 @Auth를 통해 ArgumentResolver실행시 userId를 통해 User를 조회해서 넘겨준다면,
추후 추가적인 user조회 로직의 중복을 피할 수 있지 않을까요?
안녕하세요 피터! 5번째 pr 제출합니다~
이번 PR에서 진행한 사항
질문드릴 점
findxxx()와 같은 Optional 처리를 한 다음 엔티티를 찾아오는 메서드가 중복됩니다.이런 중복되는 코드들을 효과적으로 관리할 방법이 있을까요? 별도의 조회용 메서드만 모아놓은 서비스를 주입받는 방법도 있을 것 같긴한데
적절한 방법일지 궁금합니다.