Skip to content

Commit b31c303

Browse files
Dongjin0224Dongjin0224
authored
[Refactor] 북마크 리팩터링 진행 (#240)
* refactor: BookmarkService 리팩터링 진행 - 함수 추출 - 변수 인라인 * test: 테스트코드 추가 - 시퀀스 스왑 메서드 기존 로직에서 수정이 조금 있어서 테스트 코드를 추가하였습니다.
1 parent f50a82a commit b31c303

3 files changed

Lines changed: 99 additions & 66 deletions

File tree

‎src/main/java/org/programmers/signalbuddyfinal/domain/bookmark/repository/BookmarkRepository.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.programmers.signalbuddyfinal.domain.bookmark.repository;
22

3+
import java.util.Collection;
34
import java.util.List;
45
import java.util.Optional;
56
import org.programmers.signalbuddyfinal.domain.bookmark.entity.Bookmark;
@@ -17,4 +18,5 @@ public interface BookmarkRepository extends JpaRepository<Bookmark, Long>,
1718

1819
List<Bookmark> findAllBySequenceInAndMemberMemberId(List<Integer> targetSequences, Long id);
1920

21+
List<Bookmark> findAllByMemberMemberIdAndBookmarkIdInOrSequenceIn(Long memberMemberId, Collection<Long> bookmarkIds, Collection<Integer> sequences);
2022
}

‎src/main/java/org/programmers/signalbuddyfinal/domain/bookmark/service/BookmarkService.java‎

Lines changed: 87 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import java.util.stream.Collectors;
88
import lombok.RequiredArgsConstructor;
99
import lombok.extern.slf4j.Slf4j;
10-
import org.locationtech.jts.geom.GeometryFactory;
1110
import org.locationtech.jts.geom.Point;
1211
import org.programmers.signalbuddyfinal.domain.bookmark.dto.BookmarkRequest;
1312
import org.programmers.signalbuddyfinal.domain.bookmark.dto.BookmarkResponse;
@@ -35,10 +34,10 @@
3534
public class BookmarkService {
3635

3736
private final BookmarkRepository bookmarkRepository;
38-
private final GeometryFactory geometryFactory;
3937
private final MemberRepository memberRepository;
4038
private final RecentPathRepository recentPathRepository;
4139

40+
4241
public PageResponse<BookmarkResponse> findPagedBookmarks(Pageable pageable, Long memberId) {
4342
final Page<BookmarkResponse> page = bookmarkRepository.findPagedByMember(pageable,
4443
memberId);
@@ -51,76 +50,43 @@ public BookmarkResponse createBookmark(BookmarkRequest request, Long memberId) {
5150

5251
final Point point = PointUtils.toPoint(request.getLat(), request.getLng());
5352

54-
bookmarkRepository.findByCoordinateAndMemberIdNotDeleted(point, memberId)
55-
.ifPresent(bookmark -> {
56-
throw new BusinessException(BookmarkErrorCode.ALREADY_EXIST_BOOKMARK);
57-
});
53+
validateDuplicate(memberId, point);
5854

59-
final int nextSequence =
60-
bookmarkRepository.findTopByMemberOrderBySequenceDesc(member).map(Bookmark::getSequence)
61-
.orElse(0) + 1;
62-
63-
final Bookmark bookmark = BookmarkMapper.INSTANCE.toEntity(request, point, member);
64-
bookmark.updateSequence(nextSequence);
55+
final Bookmark bookmark = createBookmarkEntity(request, member, point);
6556

66-
// 북마크 저장하는 좌표가 최근경로에 있다면 연관관계 생성
67-
recentPathRepository.findByEndPointAndMemberMemberId(point, memberId)
68-
.ifPresent(recentPath -> recentPath.linkBookmark(bookmark));
57+
linkRecentPathIfExists(memberId, point, bookmark);
6958

70-
final Bookmark save = bookmarkRepository.save(bookmark);
71-
return BookmarkMapper.INSTANCE.toDto(save);
59+
return BookmarkMapper.INSTANCE.toDto(bookmarkRepository.save(bookmark));
7260
}
7361

7462
@Transactional
75-
public BookmarkResponse updateBookmark(BookmarkRequest request, Long id, Long memberId) {
63+
public BookmarkResponse updateBookmark(BookmarkRequest request, Long bookmarkId,
64+
Long memberId) {
7665
final Member member = getMember(memberId);
66+
final Bookmark bookmark = getAuthorizedBookmark(bookmarkId, member);
7767

78-
final Bookmark bookmark = bookmarkRepository.findById(id)
79-
.orElseThrow(() -> new BusinessException(BookmarkErrorCode.NOT_FOUND_BOOKMARK));
80-
81-
if (bookmark.isNotOwnedBy(member)) {
82-
throw new BusinessException(BookmarkErrorCode.UNAUTHORIZED_MEMBER_ACCESS);
83-
}
84-
85-
final Point point = PointUtils.toPoint(request.getLat(), request.getLng());
86-
87-
bookmark.update(point, request.getAddress(), request.getName());
68+
final Point updatedPoint = PointUtils.toPoint(request.getLat(), request.getLng());
69+
bookmark.update(updatedPoint, request.getAddress(), request.getName());
8870

89-
// 최근경로 <-> 북마크 연관관계 맺어진게 있다면 수정 진행.
90-
recentPathRepository.findByEndPointAndMemberMemberId(point, memberId).ifPresent(
91-
recentPath -> recentPath.updateNameAndAddress(request.getName(), request.getAddress()));
71+
updateLinkedRecentPath(updatedPoint, memberId, request);
9272

9373
return BookmarkMapper.INSTANCE.toDto(bookmark);
9474
}
9575

76+
9677
@Transactional
9778
public void deleteBookmark(List<Long> bookmarkIds, Long memberId) {
98-
final List<Bookmark> bookmarkList = bookmarkRepository.findAllByBookmarkIdInAndMemberMemberId(
99-
bookmarkIds, memberId);
100-
101-
final List<RecentPath> recentPaths = recentPathRepository.findAllByBookmarkIn(bookmarkList);
102-
103-
bookmarkList.forEach(Bookmark::delete);
104-
105-
// 북마크 삭제 시 최근경로에서 북마크 연관관계 해제
106-
recentPaths.forEach(RecentPath::unlinkBookmark);
79+
final List<Bookmark> bookmarks = findMemberBookmarks(bookmarkIds, memberId);
80+
bookmarks.forEach(Bookmark::delete);
10781

82+
unlinkBookmarksFromRecentPaths(bookmarks);
10883
log.info("Bookmark deleted: {}", bookmarkIds);
10984
}
11085

111-
private Member getMember(Long id) {
112-
return memberRepository.findById(id)
113-
.orElseThrow(() -> new BusinessException(MemberErrorCode.NOT_FOUND_MEMBER));
114-
}
115-
11686
@Transactional(readOnly = true)
117-
public BookmarkResponse getBookmark(Long id, Long bookmarkId) {
118-
final Member member = getMember(id);
119-
final Bookmark bookmark = bookmarkRepository.findById(bookmarkId)
120-
.orElseThrow(() -> new BusinessException(BookmarkErrorCode.NOT_FOUND_BOOKMARK));
121-
if (bookmark.isNotOwnedBy(member)) {
122-
throw new BusinessException(BookmarkErrorCode.UNAUTHORIZED_MEMBER_ACCESS);
123-
}
87+
public BookmarkResponse getBookmark(Long memberId, Long bookmarkId) {
88+
final Member member = getMember(memberId);
89+
final Bookmark bookmark = getAuthorizedBookmark(bookmarkId, member);
12490
return BookmarkMapper.INSTANCE.toDto(bookmark);
12591
}
12692

@@ -129,24 +95,82 @@ public List<BookmarkResponse> updateBookmarkSequences(Long id,
12995
@Valid List<BookmarkSequenceUpdateRequest> requests) {
13096
final List<Long> bookmarkIds = requests.stream().map(BookmarkSequenceUpdateRequest::id)
13197
.toList();
98+
13299
final List<Integer> targetSequences = requests.stream()
133100
.map(BookmarkSequenceUpdateRequest::targetSequence).toList();
134101

135-
// 북마크 ID로 목록 조회
136-
final List<Bookmark> bookmarks = bookmarkRepository.findAllByBookmarkIdInAndMemberMemberId(
137-
bookmarkIds, id);
102+
final List<Bookmark> all = bookmarkRepository.findAllByMemberMemberIdAndBookmarkIdInOrSequenceIn(
103+
id, bookmarkIds, targetSequences);
138104

139-
// Target Sequence 로 목록 조회
140-
final List<Bookmark> targetBookmarks = bookmarkRepository.findAllBySequenceInAndMemberMemberId(
141-
targetSequences, id);
142-
143-
// id -> Bookmark 매핑
144-
final Map<Long, Bookmark> bookmarkMap = bookmarks.stream()
105+
final Map<Long, Bookmark> bookmarkMap = all.stream()
106+
.filter(b -> bookmarkIds.contains(b.getBookmarkId()))
145107
.collect(Collectors.toMap(Bookmark::getBookmarkId, Function.identity()));
146-
// sequence -> bookmark 매핑
147-
final Map<Integer, Bookmark> sequenceMap = targetBookmarks.stream()
108+
109+
final Map<Integer, Bookmark> sequenceMap = all.stream()
110+
.filter(b -> targetSequences.contains(b.getSequence()))
148111
.collect(Collectors.toMap(Bookmark::getSequence, Function.identity()));
149112

113+
swapSequence(requests, bookmarkMap, sequenceMap);
114+
115+
return all.stream().filter(b -> bookmarkIds.contains(b.getBookmarkId()))
116+
.map(BookmarkMapper.INSTANCE::toDto).toList();
117+
}
118+
119+
120+
private void linkRecentPathIfExists(Long memberId, Point point, Bookmark bookmark) {
121+
// 북마크 저장하는 좌표가 최근경로에 있다면 연관관계 생성
122+
recentPathRepository.findByEndPointAndMemberMemberId(point, memberId)
123+
.ifPresent(recentPath -> recentPath.linkBookmark(bookmark));
124+
}
125+
126+
private Bookmark createBookmarkEntity(BookmarkRequest request, Member member, Point point) {
127+
final int nextSequence =
128+
bookmarkRepository.findTopByMemberOrderBySequenceDesc(member).map(Bookmark::getSequence)
129+
.orElse(0) + 1;
130+
131+
final Bookmark bookmark = BookmarkMapper.INSTANCE.toEntity(request, point, member);
132+
bookmark.updateSequence(nextSequence);
133+
return bookmark;
134+
}
135+
136+
private void validateDuplicate(Long memberId, Point point) {
137+
bookmarkRepository.findByCoordinateAndMemberIdNotDeleted(point, memberId)
138+
.ifPresent(bookmark -> {
139+
throw new BusinessException(BookmarkErrorCode.ALREADY_EXIST_BOOKMARK);
140+
});
141+
}
142+
143+
private Bookmark getAuthorizedBookmark(Long bookmarkId, Member member) {
144+
Bookmark bookmark = bookmarkRepository.findById(bookmarkId)
145+
.orElseThrow(() -> new BusinessException(BookmarkErrorCode.NOT_FOUND_BOOKMARK));
146+
147+
if (bookmark.isNotOwnedBy(member)) {
148+
throw new BusinessException(BookmarkErrorCode.UNAUTHORIZED_MEMBER_ACCESS);
149+
}
150+
return bookmark;
151+
}
152+
153+
private void updateLinkedRecentPath(Point point, Long memberId, BookmarkRequest request) {
154+
recentPathRepository.findByEndPointAndMemberMemberId(point, memberId)
155+
.ifPresent(path -> path.updateNameAndAddress(request.getName(), request.getAddress()));
156+
}
157+
158+
private List<Bookmark> findMemberBookmarks(List<Long> bookmarkIds, Long memberId) {
159+
return bookmarkRepository.findAllByBookmarkIdInAndMemberMemberId(bookmarkIds, memberId);
160+
}
161+
162+
private void unlinkBookmarksFromRecentPaths(List<Bookmark> bookmarks) {
163+
final List<RecentPath> recentPaths = recentPathRepository.findAllByBookmarkIn(bookmarks);
164+
recentPaths.forEach(RecentPath::unlinkBookmark);
165+
}
166+
167+
private Member getMember(Long id) {
168+
return memberRepository.findById(id)
169+
.orElseThrow(() -> new BusinessException(MemberErrorCode.NOT_FOUND_MEMBER));
170+
}
171+
172+
private void swapSequence(List<BookmarkSequenceUpdateRequest> requests,
173+
Map<Long, Bookmark> bookmarkMap, Map<Integer, Bookmark> sequenceMap) {
150174
for (BookmarkSequenceUpdateRequest request : requests) {
151175
final Bookmark bookmark = bookmarkMap.get(request.id());
152176
final Bookmark targetBookmark = sequenceMap.get(request.targetSequence());
@@ -165,9 +189,6 @@ public List<BookmarkResponse> updateBookmarkSequences(Long id,
165189
// sequenceMap 업데이트
166190
sequenceMap.put(targetSequence, bookmark);
167191
sequenceMap.put(originalSequence, targetBookmark);
168-
169192
}
170-
171-
return bookmarks.stream().map(BookmarkMapper.INSTANCE::toDto).toList();
172193
}
173194
}

‎src/test/java/org/programmers/signalbuddyfinal/domain/bookmark/service/BookmarkServiceTest.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,18 @@ void updateBookmarkSequences() {
205205
final List<BookmarkResponse> responses = bookmarkService.updateBookmarkSequences(
206206
member.getMemberId(), requests);
207207

208+
final Map<Long, Integer> expectedMap = requests.stream()
209+
.collect(Collectors.toMap(BookmarkSequenceUpdateRequest::id,
210+
BookmarkSequenceUpdateRequest::targetSequence));
211+
212+
final Map<Long, Integer> actualMap = responses.stream()
213+
.collect(Collectors.toMap(BookmarkResponse::getBookmarkId, BookmarkResponse::getSequence));
214+
215+
assertThat(actualMap).isEqualTo(expectedMap);
208216
assertThat(responses).isNotEmpty().allSatisfy(e -> {
209217
assertThat(map).doesNotContainEntry(e.getBookmarkId(), e.getSequence());
218+
// 기대한 값과 완전히 일치하는지 검증
219+
assertThat(expectedMap).containsEntry(e.getBookmarkId(), e.getSequence());
210220
});
211221
}
212222
}

0 commit comments

Comments
 (0)