-
Notifications
You must be signed in to change notification settings - Fork 6
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
[BE] feature/#264 토픽에 대한 즐겨찾기 API #275
Changes from 23 commits
46e5107
0ea8213
a3d8629
47a52e7
f101ed3
4421edb
e3530d6
19b3f0c
81dffcb
a4d8897
5c9bfcb
9793347
e3684a2
4db58a9
3e8bb0b
d788f43
fe6691d
dbc8b12
a345b43
f97d032
f414e40
61b1ae4
770ab50
398f1e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
== 즐겨찾기 | ||
|
||
=== 토픽을 유저의 즐겨찾기에 추가 | ||
|
||
operation::bookmark-controller-test/add-topic-in-bookmark[snippets='http-request,http-response'] | ||
|
||
=== 유저의 토픽 즐겨찾기 목록 조회 | ||
|
||
operation::bookmark-controller-test/find-topics-in-bookmark[snippets='http-request,http-response'] | ||
|
||
=== 유저의 토픽 즐겨찾기 단일 삭제 | ||
operation::bookmark-controller-test/delete-topic-in-bookmark[snippets='http-request,http-response'] | ||
|
||
=== 유저의 토픽 즐겨찾기 전체 삭제 | ||
operation::bookmark-controller-test/delete-all-topics-in-bookmark[snippets='http-request,http-response'] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,4 @@ toc::[] | |
include::topic.adoc[] | ||
include::pin.adoc[] | ||
include::member.adoc[] | ||
include::bookmark.adoc[] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
package com.mapbefine.mapbefine.bookmark.application; | ||
|
||
import com.mapbefine.mapbefine.auth.domain.AuthMember; | ||
import com.mapbefine.mapbefine.bookmark.domain.Bookmark; | ||
import com.mapbefine.mapbefine.bookmark.domain.BookmarkRepository; | ||
import com.mapbefine.mapbefine.member.domain.Member; | ||
import com.mapbefine.mapbefine.member.domain.MemberRepository; | ||
import com.mapbefine.mapbefine.topic.domain.Topic; | ||
import com.mapbefine.mapbefine.topic.domain.TopicRepository; | ||
import java.util.NoSuchElementException; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
@Service | ||
@Transactional(readOnly = true) | ||
public class BookmarkCommandService { | ||
|
||
private final BookmarkRepository bookmarkRepository; | ||
|
||
private final MemberRepository memberRepository; | ||
|
||
private final TopicRepository topicRepository; | ||
|
||
public BookmarkCommandService( | ||
BookmarkRepository bookmarkRepository, | ||
MemberRepository memberRepository, | ||
TopicRepository topicRepository | ||
) { | ||
this.bookmarkRepository = bookmarkRepository; | ||
this.memberRepository = memberRepository; | ||
this.topicRepository = topicRepository; | ||
} | ||
|
||
public Long addTopicInBookmark(AuthMember authMember, Long topicId) { | ||
validateBookmarkDuplication(authMember, topicId); | ||
Topic topic = getTopicById(topicId); | ||
validateBookmarkingPermission(authMember, topic); | ||
Member member = getMemberById(authMember); | ||
|
||
Bookmark bookmark = Bookmark.createWithAssociatedTopicAndMember(topic, member); | ||
bookmarkRepository.save(bookmark); | ||
|
||
return bookmark.getId(); | ||
} | ||
|
||
private Topic getTopicById(Long topicId) { | ||
return topicRepository.findById(topicId) | ||
.orElseThrow(() -> new NoSuchElementException("존재하지 않는 토픽입니다.")); | ||
} | ||
|
||
private void validateBookmarkDuplication(AuthMember authMember, Long topicId) { | ||
if (isExistBookmark(authMember, topicId)) { | ||
throw new IllegalArgumentException("이미 즐겨찾기로 등록된 토픽입니다."); | ||
} | ||
} | ||
|
||
private boolean isExistBookmark(AuthMember authMember, Long topicId) { | ||
return bookmarkRepository.existsByMemberIdAndTopicId(authMember.getMemberId(), topicId); | ||
} | ||
|
||
private void validateBookmarkingPermission(AuthMember authMember, Topic topic) { | ||
if (authMember.canRead(topic)) { | ||
return; | ||
} | ||
|
||
throw new IllegalArgumentException("토픽에 대한 권한이 없어서 즐겨찾기에 추가할 수 없습니다."); | ||
} | ||
|
||
private Member getMemberById(AuthMember authMember) { | ||
return memberRepository.findById(authMember.getMemberId()) | ||
.orElseThrow(() -> new NoSuchElementException("존재하지 않는 멤버입니다.")); | ||
} | ||
|
||
public void deleteTopicInBookmark(AuthMember authMember, Long topicId) { | ||
validateBookmarkDeletingPermission(authMember, topicId); | ||
|
||
bookmarkRepository.deleteByMemberIdAndTopicId(authMember.getMemberId(), topicId); | ||
} | ||
|
||
private void validateBookmarkDeletingPermission(AuthMember authMember, Long topicId) { | ||
if (isExistBookmark(authMember, topicId)) { | ||
return; | ||
} | ||
|
||
throw new IllegalArgumentException("즐겨찾기 삭제에 대한 권한이 없습니다."); | ||
} | ||
|
||
public void deleteAllBookmarks(AuthMember authMember) { | ||
bookmarkRepository.deleteAllByMemberId(authMember.getMemberId()); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package com.mapbefine.mapbefine.bookmark.application; | ||
|
||
import com.mapbefine.mapbefine.auth.domain.AuthMember; | ||
import com.mapbefine.mapbefine.bookmark.domain.Bookmark; | ||
import com.mapbefine.mapbefine.bookmark.domain.BookmarkRepository; | ||
import com.mapbefine.mapbefine.topic.dto.response.TopicResponse; | ||
import java.util.List; | ||
import java.util.Objects; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
@Service | ||
@Transactional(readOnly = true) | ||
public class BookmarkQueryService { | ||
|
||
private final BookmarkRepository bookmarkRepository; | ||
|
||
public BookmarkQueryService(BookmarkRepository bookmarkRepository) { | ||
this.bookmarkRepository = bookmarkRepository; | ||
} | ||
|
||
public List<TopicResponse> findAllTopicsInBookmark(AuthMember authMember) { | ||
validateNonExistsMember(authMember); | ||
List<Bookmark> bookmarks = bookmarkRepository.findAllByMemberId(authMember.getMemberId()); | ||
|
||
return bookmarks.stream() | ||
.map(bookmark -> TopicResponse.from(bookmark.getTopic(), Boolean.TRUE)) | ||
.toList(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P4. 당장 중요한 부분은 아니고, 반영하지 않아도 좋은데 원분들의 의견이 궁금해져서 코멘트 답니다!! public static List<BookmarkResponse> from(List<Bookmark> bookmarks) {
return bookmarks.stream()
.map(BookmarkResponse::from)
.toList();
}
// 서비스에서는 아래와 같이 간략화할 수 있음
return BookmarkResponse.from(bookmarks); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 대부분의 변환 로직이 굉장히 간략화 될 것 같아서 좋을 것 같습니다! 킹치만, 아주 조금 우려가 되는 부분은 public Bookmark toBookmark(Topic topic) {
...
}
return topic.stream()
.map(this::toBookmark)
.map(BookmarkResponse::from)
.toList();
->
List<Bookmark> bookmarks = topic.stream()
.map(this::toBookmark)
.toList();
return BookmarkResponse.from(bookmarks) (toBookmark 는 예시를 들기 위해서 사용한 임의의 변환 메서드입니다.) 위와 같은 경우 원래 한번의 스트림으로 해결할 수 있었지만, 해당 정팩매를 둠으로써 중간에 toList() 를 통해 추출해내야 한다는 단점도 생길 수 있을 것 같아요. 이런 경우에서만 정팩메를 사용하지 않는다는 저희만의 컨벤션을 만들거나, 혹은 더 나아가 이런 경우는 발생하지 않을 것 같다고 판단되면 추가해도 너무 좋을 것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 개인적으로 List 자체를 넘겨주는 것 좋습니다 ㅎㅎ 매튜가 착각한 것 같은데, List로 추출하지 않고 stream에서 한 번에 처리할 수 있지 않나요 !? |
||
} | ||
|
||
public void validateNonExistsMember(AuthMember authMember) { | ||
if (Objects.isNull(authMember.getMemberId())) { | ||
throw new IllegalArgumentException("존재하지 않는 유저입니다."); | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
package com.mapbefine.mapbefine.bookmark.domain; | ||
|
||
import com.mapbefine.mapbefine.member.domain.Member; | ||
import com.mapbefine.mapbefine.topic.domain.Topic; | ||
import jakarta.persistence.Entity; | ||
import jakarta.persistence.GeneratedValue; | ||
import jakarta.persistence.GenerationType; | ||
import jakarta.persistence.Id; | ||
import jakarta.persistence.JoinColumn; | ||
import jakarta.persistence.ManyToOne; | ||
import lombok.AccessLevel; | ||
import lombok.Getter; | ||
import lombok.NoArgsConstructor; | ||
|
||
@Entity | ||
@NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
@Getter | ||
public class Bookmark { | ||
|
||
@Id | ||
@GeneratedValue(strategy = GenerationType.IDENTITY) | ||
private Long id; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P4. Bookmark가 id를 가지도록 하신 이유가 궁금합니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 복합키를 기본키로 사용하는 구현 방식이 복잡하여 (Serializable, equals&hashCode 재정의 등) 기본키 자동 생성 전략을 택했는데요 ! 즐겨찾기를 삭제할 때, 각 id 값을 통해 삭제를 요청해야 올바른 흐름이라고 생각했어요. 하지만 이럴 경우, 토픽의 데이터를 내려줄 때 즐겨찾기 목록인지 아닌지에 따라 response body가 달라진다는 점에서 프론트에서 관리해야하는 포인트들이 더 생기는 것 같네요. 준팍과 도이의 의견을 일부 반영하여, 내부적으로는 기본키를 유지하되, 비즈니스 로직(즐겿자기 삭제 등)에서는 복합키로 처리하도록 하겠습니다. |
||
|
||
@ManyToOne | ||
@JoinColumn(name = "topic_id", nullable = false) | ||
private Topic topic; | ||
|
||
@ManyToOne | ||
@JoinColumn(name = "member_id", nullable = false) | ||
private Member member; | ||
|
||
private Bookmark(Topic topic, Member member) { | ||
this.topic = topic; | ||
this.member = member; | ||
} | ||
|
||
// TODO: 2023/08/11 필요한 검증이 무엇이 있을까.. 현재로썬 외부에서 검증하는 방법 밖에 ? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 쥬니짱 제가 위에서 코멘트 드렸던 부분
을 여기서 해결할 수 있지 않을까? 싶어요!
중복으로 저장되는 경우도 쉽게 막을 수 있을 것 같긴한데 쥬니짱짱 생각은 어떠세요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 물론 위 방식을 기용하면 equals, hashcode 를 추가해줘야 할 것 같긴하네요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋은 방법인 것 같아요 ㅎㅎ 즐겨찾기는 매우 가벼운 비즈니스 로직인데, 매번 모든 정보를 가져와야 한다는 점이 걱정되네요 ㅎㅎ 이에따라, Service 계층에서 아래와 같이 검증 로직을 추가하였습니다 ! private void validateBookmarkDuplication(AuthMember authMember, Long topicId) {
if (isExistBookmark(authMember, topicId)) {
throw new IllegalArgumentException("이미 즐겨찾기로 등록된 토픽입니다.");
}
}
private boolean isExistBookmark(AuthMember authMember, Long topicId) {
return bookmarkRepository.existsByMemberIdAndTopicId(authMember.getMemberId(), topicId);
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네네 제 생각에도 Repository 에서 확인하는 것이 성능상 이점이 있을 것 같지만, 뭔가 조금 더 객체지향적으로 해결하려면 위와 같은 방법을 채택하는 것이 어떨까 해서 말씀드렸었습니당 홍홍홍 |
||
public static Bookmark createWithAssociatedTopicAndMember(Topic topic, Member member) { | ||
Bookmark bookmark = new Bookmark(topic, member); | ||
|
||
topic.addBookmark(bookmark); | ||
member.addBookmark(bookmark); | ||
|
||
return bookmark; | ||
} | ||
Comment on lines
+38
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 연관관계를 꼭 맺어줘야할까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 생각해보니 Members엔 넣어주는게 더편할 수 있겠네요 ㅎㅎ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 다시 생각해보니 topic이 북마크된 숫자도 반환해야하니 필요하겠군요 |
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package com.mapbefine.mapbefine.bookmark.domain; | ||
|
||
import java.util.List; | ||
import org.springframework.data.jpa.repository.JpaRepository; | ||
|
||
public interface BookmarkRepository extends JpaRepository<Bookmark, Long> { | ||
|
||
List<Bookmark> findAllByMemberId(Long memberId); | ||
|
||
void deleteAllByMemberId(Long memberId); | ||
|
||
boolean existsByMemberIdAndTopicId(Long memberId, Long topicId); | ||
|
||
void deleteByMemberIdAndTopicId(Long memberId, Long topicId); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
package com.mapbefine.mapbefine.bookmark.presentation; | ||
|
||
import com.mapbefine.mapbefine.auth.domain.AuthMember; | ||
import com.mapbefine.mapbefine.bookmark.application.BookmarkCommandService; | ||
import com.mapbefine.mapbefine.bookmark.application.BookmarkQueryService; | ||
import com.mapbefine.mapbefine.common.interceptor.LoginRequired; | ||
import com.mapbefine.mapbefine.topic.dto.response.TopicResponse; | ||
import java.net.URI; | ||
import java.util.List; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.DeleteMapping; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.PostMapping; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RequestParam; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
||
@RestController | ||
@RequestMapping("/bookmarks") | ||
public class BookmarkController { | ||
|
||
private final BookmarkCommandService bookmarkCommandService; | ||
|
||
private final BookmarkQueryService bookmarkQueryService; | ||
|
||
public BookmarkController( | ||
BookmarkCommandService bookmarkCommandService, | ||
BookmarkQueryService bookmarkQueryService | ||
) { | ||
this.bookmarkCommandService = bookmarkCommandService; | ||
this.bookmarkQueryService = bookmarkQueryService; | ||
} | ||
|
||
@LoginRequired | ||
@PostMapping | ||
public ResponseEntity<Void> addTopicInBookmark( | ||
AuthMember authMember, | ||
@RequestParam Long topicId | ||
) { | ||
Long bookmarkId = bookmarkCommandService.addTopicInBookmark(authMember, topicId); | ||
|
||
return ResponseEntity.created(URI.create("/bookmarks/" + bookmarkId)).build(); | ||
} | ||
|
||
@LoginRequired | ||
@GetMapping | ||
public ResponseEntity<List<TopicResponse>> findAllTopicsInBookmark(AuthMember authMember) { | ||
List<TopicResponse> responses = bookmarkQueryService.findAllTopicsInBookmark(authMember); | ||
|
||
return ResponseEntity.ok(responses); | ||
} | ||
|
||
@LoginRequired | ||
@DeleteMapping | ||
public ResponseEntity<Void> deleteTopicInBookmark( | ||
AuthMember authMember, | ||
@RequestParam Long topicId | ||
) { | ||
bookmarkCommandService.deleteTopicInBookmark(authMember, topicId); | ||
|
||
return ResponseEntity.noContent().build(); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3. canDelete boolean을 반환하는 부분을 canDelete() 메서드로 분리해줘도 좋겠네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 사항 반영했습니다 ㅏ!