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

[BE] feature/#264 토픽에 대한 즐겨찾기 API #275

Merged
merged 24 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
46e5107
feat: Bookmark Entity 구현
cpot5620 Aug 11, 2023
0ea8213
feat: 즐겨찾기 추가 기능 구현
cpot5620 Aug 11, 2023
a3d8629
feat: 즐겨찾기 추가 API 구현
cpot5620 Aug 11, 2023
47a52e7
feat: 즐겨찾기 조회 API 구현
cpot5620 Aug 11, 2023
f101ed3
test: 즐겨찾기 토픽 추가 및 조회 API 테스트 구현
cpot5620 Aug 11, 2023
4421edb
feat: 즐겨찾기 토픽 단일 및 전체 삭제 기능 구현
cpot5620 Aug 11, 2023
e3530d6
test: 즐겨찾기 토픽 단일 및 전체 삭제 기능 테스트 구현
cpot5620 Aug 11, 2023
19b3f0c
feat: 즐겨찾기 토픽 단일 및 전체 삭제 API 구현
cpot5620 Aug 11, 2023
81dffcb
test: 즐겨찾기 토픽 단일 및 전체 삭제 API 테스트 구현
cpot5620 Aug 11, 2023
a4d8897
chore: Bookmark 패키지 분리
cpot5620 Aug 11, 2023
5c9bfcb
refactor: 메서드 순서 변경
cpot5620 Aug 11, 2023
9793347
test: 통합 테스트 @DisplayName 변경
cpot5620 Aug 12, 2023
e3684a2
feat: 토픽에 대한 즐겨찾기 여부 반환 JPQL 구현
cpot5620 Aug 12, 2023
4db58a9
feat: 토픽 조회시, 즐겨찾기 여부 반환 기능 구현
cpot5620 Aug 12, 2023
3e8bb0b
test: 토픽 조회시, 즐겨찾기 여부 반환 테스트 구현
cpot5620 Aug 12, 2023
d788f43
style: 개행 수정
cpot5620 Aug 12, 2023
fe6691d
refactor: 즐겨 찾기 삭제 로직 수정
cpot5620 Aug 14, 2023
dbc8b12
refactor: 즐겨 찾기 등록시, 검증 로직 추가
cpot5620 Aug 14, 2023
a345b43
refactor: TopicResponse 매개변수 통일
cpot5620 Aug 14, 2023
f97d032
refactor: 변수명 수정
cpot5620 Aug 14, 2023
f414e40
refactor: BookmarkResponse 삭제
cpot5620 Aug 14, 2023
61b1ae4
refactor: 사용하지 않는 변수 삭제
cpot5620 Aug 14, 2023
770ab50
style: restdocs 오탈자 수정
cpot5620 Aug 14, 2023
398f1e4
chore: conflict 해결
cpot5620 Aug 14, 2023
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
15 changes: 15 additions & 0 deletions backend/src/docs/asciidoc/bookmark.adoc
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']
1 change: 1 addition & 0 deletions backend/src/docs/asciidoc/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ toc::[]
include::topic.adoc[]
include::pin.adoc[]
include::member.adoc[]
include::bookmark.adoc[]
2 changes: 1 addition & 1 deletion backend/src/docs/asciidoc/member.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ operation::member-controller-test/find-pins-by-member[snippets='http-request,htt

=== 유저가 만든 토픽 조회

operation::member-controller-test/find-topics-by-member[snippets='http-request,http-response']
operation::member-controller-test/find-topics-by-member[snippets='http-request,http-response']
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
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) {
Topic topic = getTopicById(topicId);
validateBookmarkingPermission(authMember, topic);
Member member = getMemberById(authMember);

Bookmark bookmark = Bookmark.createWithAssociatedTopicAndMember(topic, member);
bookmarkRepository.save(bookmark);

return bookmark.getId();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이전 코드들과의 일관성을 위해서 bookmark 를 저장하신건가용?

저는 쥬니짱이 말씀하셨던 것처럼 member 혹은 topic 을 저장하여 cascade 로 bookmark 를 저장하는 것도 좋아보여서요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

음 그리고 사용자가 즐겨찾기를 중복으로 실행하는 경우도 고려할 수 있을 것 같아요 쥬니짱.

물론 화면딴에서는 쥬니짱이 추가해주신 isBookmarked 로 인해 중복으로 즐겨찾기 하는 경우는 막을 수 있겠지만, 악의적인 유저가 Postman 을 이용하여 중복으로 즐겨찾기 요청을 계속 보내게되면 동일한 정보들이 DB 에 저장될 수도 있지 않을까? 하는 우려가 들기도 하네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

물론 화면딴에서는 쥬니짱이 추가해주신 isBookmarked 로 인해 중복으로 즐겨찾기 하는 경우는 막을 수 있겠지만, 악의적인 유저가 Postman 을 이용하여 중복으로 즐겨찾기 요청을 계속 보내게되면 동일한 정보들이 DB 에 저장될 수도 있지 않을까? 하는 우려가 들기도 하네요

아앗.. 포스트맨이 있었군요.
저는 프론트 크루들을 전적으로 신뢰하고 있었기 떄문에, 별다른 검증을 하지 않았는데 ㅎㅎ

해당 사항 반영하였습니다 !


private Topic getTopicById(Long topicId) {
return topicRepository.findById(topicId)
.orElseThrow(() -> new NoSuchElementException("존재하지 않는 토픽입니다."));
}

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 bookmarkId) {
validateBookmarkDeletingPermission(authMember, bookmarkId);

bookmarkRepository.deleteById(bookmarkId);
}

private void validateBookmarkDeletingPermission(AuthMember authMember, Long bookmarkId) {
boolean canDelete = bookmarkRepository.existsByIdAndMemberId(
bookmarkId,
authMember.getMemberId()
);

if (canDelete) {
return;
}

throw new IllegalArgumentException("즐겨찾기 삭제에 대한 권한이 없습니다.");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3. canDelete boolean을 반환하는 부분을 canDelete() 메서드로 분리해줘도 좋겠네요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

해당 사항 반영했습니다 ㅏ!


public void deleteAllBookmarks(AuthMember authMember) {
bookmarkRepository.deleteAllByMemberId(authMember.getMemberId());
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
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.bookmark.dto.response.BookmarkResponse;
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<BookmarkResponse> findAllTopicsInBookmark(AuthMember authMember) {
validateNonExistsMember(authMember);
List<Bookmark> bookmark = bookmarkRepository.findAllByMemberId(authMember.getMemberId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2. 변수명에 s가 붙으면 좋겠네용~~


return bookmark.stream()
.map(BookmarkResponse::from)
.toList();
Copy link
Collaborator

Choose a reason for hiding this comment

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

P4. 당장 중요한 부분은 아니고, 반영하지 않아도 좋은데 원분들의 의견이 궁금해져서 코멘트 답니다!!
저는 도메인 List -> DTO List 로 변환하는 로직도 DTO의 정적 팩토리 메서드로 넣는 걸 선호하는데요
서비스에서 변환 로직의 코드를 간략화할 수 있기 때문입니다!
다들 어떠신가요!?

public static List<BookmarkResponse> from(List<Bookmark> bookmarks) {
        return bookmarks.stream()
                .map(BookmarkResponse::from)
                .toList();
}

// 서비스에서는 아래와 같이 간략화할 수 있음
return BookmarkResponse.from(bookmarks);

Copy link
Collaborator

Choose a reason for hiding this comment

The 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() 를 통해 추출해내야 한다는 단점도 생길 수 있을 것 같아요.

이런 경우에서만 정팩메를 사용하지 않는다는 저희만의 컨벤션을 만들거나, 혹은 더 나아가 이런 경우는 발생하지 않을 것 같다고 판단되면 추가해도 너무 좋을 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

P4. Bookmark가 id를 가지도록 하신 이유가 궁금합니다!
사실 Bookmark는 어떤 Member가 어떤 Topic을 즐겨찾기로 저장했는지 확인하기 위한 엔티티이고,
Response 클래스에 준팍의 코멘트처럼, memberId, topicId로도 식별이 가능하기 때문에
별도의 id 없이 복합키를 쓸 수도 있을 것 같아서요!
아니면 Bookmark의 Id를 사용해야 하는 비즈니스 로직이 있을까용?
제가 놓치고 있는 부분이 있을 것 같아 여쭤봅니다!!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 필요한 검증이 무엇이 있을까.. 현재로썬 외부에서 검증하는 방법 밖에 ?
Copy link
Collaborator

Choose a reason for hiding this comment

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

쥬니짱 제가 위에서 코멘트 드렸던 부분

음 그리고 사용자가 즐겨찾기를 중복으로 실행하는 경우도 고려할 수 있을 것 같아요 쥬니짱.

물론 화면딴에서는 쥬니짱이 추가해주신 isBookmarked 로 인해 중복으로 즐겨찾기 하는 경우는 막을 수 있겠지만, 악의적인 유저가 Postman 을 이용하여 중복으로 즐겨찾기 요청을 계속 보내게되면 동일한 정보들이 DB 에 저장될 수도 있지 않을까? 하는 우려가 들기도 하네요

을 여기서 해결할 수 있지 않을까? 싶어요!

(topic/member).alreadyContainsBookmark() 이런 메서드를 만들어 해당 토픽이나 멤버에 현재 추가하려는 bookmark 가 이미 존재하는지 확인한다면

중복으로 저장되는 경우도 쉽게 막을 수 있을 것 같긴한데

쥬니짱짱 생각은 어떠세요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

물론 위 방식을 기용하면 equals, hashcode 를 추가해줘야 할 것 같긴하네요

Copy link
Collaborator Author

@cpot5620 cpot5620 Aug 14, 2023

Choose a reason for hiding this comment

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

좋은 방법인 것 같아요 ㅎㅎ
다만, 한 가지 우려되는 부분은 member 혹은 topic에서 OneToMany로 가지고 있기 때문에, 이후에 지연 로딩을 적용할 것이잖아요 ?

즐겨찾기는 매우 가벼운 비즈니스 로직인데, 매번 모든 정보를 가져와야 한다는 점이 걱정되네요 ㅎㅎ

이에따라, 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);
    }

Copy link
Collaborator

Choose a reason for hiding this comment

The 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
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

Choose a reason for hiding this comment

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

생각해보니 Members엔 넣어주는게 더편할 수 있겠네요 ㅎㅎ

Copy link
Collaborator

Choose a reason for hiding this comment

The 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,14 @@
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 existsByIdAndMemberId(Long id, Long memberId);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.mapbefine.mapbefine.bookmark.dto.response;

import com.mapbefine.mapbefine.bookmark.domain.Bookmark;
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicInfo;
import java.time.LocalDateTime;

public record BookmarkResponse(
Copy link
Collaborator

Choose a reason for hiding this comment

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

BookMarkResponse 따로 만들어주기보다는 TopicResponse로 통합해서 처리 가능하지 않을까요?
TopicResponse만 있어도 BookMark를 찾는데 필요한 정보는 다 있다는 생각이 드네요 ㅎㅎ

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 동의합니다.
처음에는 DTO를 통합함으로써 결합도가 높아져 안좋다고 생각했는데,
사실 Bookmark에 대한 Response는 Member가 즐겨찾기를 한 Topic에 대한 응답이기에
도메인 논리로만 생각하더라도 TopicResponse를 반환해도 자연스럽다는 생각이 듭니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 현재 topicResponse 에 isBookmarked 가 포함되어 있어, 현재 방식도 괜찮아보이긴합니다.

만일 topicResponse 로 반환하게 된다면, TopicResponse.from(Topic topic) 을 호출해야할테고, 그렇다면 무조건 isBookmarked 가 false 로 설정되게 될테니까요.

Long id,
Long topicId,
String name,
String image,
Integer pinCount,
LocalDateTime updatedAt
) {

public static BookmarkResponse from(Bookmark bookmark) {
Topic topic = bookmark.getTopic();
TopicInfo topicInfo = topic.getTopicInfo();

return new BookmarkResponse(
bookmark.getId(),
topic.getId(),
topicInfo.getName(),
topicInfo.getImageUrl(),
topic.countPins(),
topic.getUpdatedAt()
);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
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.bookmark.dto.response.BookmarkResponse;
import com.mapbefine.mapbefine.common.interceptor.LoginRequired;
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.PathVariable;
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("/members/bookmarks")
public class BookmarkController {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

즐겨찾기라는게 아무래도, 유저와 관련된 내용이다 보니 위와 같이 URI를 작성하였습니다.

MemberController와 별도의 컨트롤러라는 점에서, 위 URI가 잘못된 구조인 것 같기도 합니다.

의견 부탁드립니다 !

Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 members 빼는 게 더 좋을 것 같습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 members 빼는 것에 동의합니다!

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 Author

Choose a reason for hiding this comment

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

반영했습니다 ~


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("/members/bookmarks/" + bookmarkId)).build();
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

즐겨찾기에 대한 Id를 반환해주어야 하는가 ? 라는 질문을 준팍이 하셨던 것 같아서 코멘트 남깁니다.

사용자가 자신의 즐겨찾기 목록에 존재하는 토픽을 삭제하려면, 해당 토픽의 id보다는 해당 즐겨찾기의 id를 요청하는 것이 맞다고 판단했습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

즐겨찾기에 같은 토픽이 여러개 들어갈 수 없으니, TopicId와 MemberId로 충분히 탐색할 수 있지 않을까요?
즐겨찾기 Id를 넣어주는 Response를 만들 경우 프론트에서 보여주는 내용은 동일한데 다양한 Response에 대해서 상태 관리를 해줘야하지 않을까하는 생각이 드네용ㅎㅎ

Copy link
Collaborator

Choose a reason for hiding this comment

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

topicId, memberId로 식별하고, 그렇게 되면 아예 id 대신 두가지를 복합키로 하는 방식도 괜찮아보입니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

유연성 제한: id 컬럼 없이 memberId와 topicId만 사용하면 테이블 내에서 각 행을 고유하게 식별할 수 있습니다. 하지만 id를 사용하는 경우, 나중에 유연하게 스키마를 변경하거나 다른 관련 정보를 추가하는 데 더 유리할 수 있습니다.

성능 이슈: 복합키로 인덱스를 생성할 경우, 검색 성능이 향상될 수 있지만, 조건에 따라 성능이 저하될 수도 있습니다. 특히 대량의 데이터에서 복합키를 사용하면 인덱스 크기가 커져서 일부 검색 작업에서 더 많은 리소스를 필요로 할 수 있습니다.

복잡성 증가: 복합키를 사용하면 데이터베이스 쿼리 및 조인 작업이 복잡해질 수 있습니다. 특히 다른 테이블과의 조인 작업에서 조금 더 복잡한 SQL문이 필요할 수 있습니다.

데이터 무결성 유지: 복합키를 사용하는 경우, 데이터 무결성을 유지하기 위해 추가적인 제약 조건이나 처리 로직이 필요할 수 있습니다. 이를 관리하는 것도 고려해야 합니다.

확장성 제한: 나중에 테이블에 다른 컬럼을 추가하거나 변경할 때, 복합키가 있는 경우 일부 작업이 더 복잡해질 수 있습니다.

gpt가 복합키를 사용했을 때의 단점은 이렇다고 하네용.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

id를 제거하는 방식이 아닌, 삭제 등의 요청에 대해 복합키를 사용하는 방식으로 결정하겠습니다 !


@LoginRequired
@GetMapping
public ResponseEntity<List<BookmarkResponse>> findAllTopicsInBookmark(AuthMember authMember) {
List<BookmarkResponse> responses = bookmarkQueryService.findAllTopicsInBookmark(authMember);

return ResponseEntity.ok(responses);
}

@LoginRequired
@DeleteMapping("{bookmarkId}")
public ResponseEntity<Void> deleteTopicInBookmark(
AuthMember authMember,
@PathVariable Long bookmarkId
) {
bookmarkCommandService.deleteTopicInBookmark(authMember, bookmarkId);

return ResponseEntity.noContent().build();
}

@LoginRequired
@DeleteMapping
public ResponseEntity<Void> deleteAllTopicsInBookmark(AuthMember authMember) {
bookmarkCommandService.deleteAllBookmarks(authMember);

return ResponseEntity.noContent().build();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ private void validateUniqueEmail(String email) {
}
}

public Long saveMemberTopicPermission(AuthMember authMember, MemberTopicPermissionCreateRequest request) {
public Long saveMemberTopicPermission(
AuthMember authMember,
MemberTopicPermissionCreateRequest request
) {
Member member = memberRepository.findById(request.memberId())
.orElseThrow(NoSuchElementException::new);
Topic topic = topicRepository.findById(request.topicId())
Expand Down Expand Up @@ -107,8 +110,9 @@ private void validateDuplicatePermission(Long topicId, Long memberId) {
}

public void deleteMemberTopicPermission(AuthMember authMember, Long permissionId) {
MemberTopicPermission memberTopicPermission = memberTopicPermissionRepository.findById(permissionId)
.orElseThrow(NoSuchElementException::new);
MemberTopicPermission memberTopicPermission =
memberTopicPermissionRepository.findById(permissionId)
.orElseThrow(NoSuchElementException::new);

validateMemberCanTopicUpdate(authMember, memberTopicPermission.getTopic());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public List<MemberResponse> findAll() {
}

public List<TopicResponse> findTopicsByMember(AuthMember authMember) {
validateNonExistsMember(authMember.getMemberId());
validateNonExistsMember(authMember);
List<Topic> topicsByCreator = topicRepository.findByCreatorId(authMember.getMemberId());

return topicsByCreator.stream()
Expand All @@ -66,16 +66,16 @@ public List<TopicResponse> findTopicsByMember(AuthMember authMember) {
}

public List<PinResponse> findPinsByMember(AuthMember authMember) {
validateNonExistsMember(authMember.getMemberId());
validateNonExistsMember(authMember);
List<Pin> pinsByCreator = pinRepository.findByCreatorId(authMember.getMemberId());

return pinsByCreator.stream()
.map(PinResponse::from)
.toList();
}
public void validateNonExistsMember(Long memberId) {
if (Objects.isNull(memberId)) {

public void validateNonExistsMember(AuthMember authMember) {
if (Objects.isNull(authMember.getMemberId())) {
throw new IllegalArgumentException("존재하지 않는 유저입니다.");
}
}
Expand All @@ -91,7 +91,8 @@ public List<MemberTopicPermissionResponse> findAllWithPermission(Long topicId) {
}

public MemberTopicPermissionDetailResponse findMemberTopicPermissionById(Long permissionId) {
MemberTopicPermission memberTopicPermission = memberTopicPermissionRepository.findById(permissionId)
MemberTopicPermission memberTopicPermission = memberTopicPermissionRepository.findById(
permissionId)
.orElseThrow(NoSuchElementException::new);

return MemberTopicPermissionDetailResponse.from(memberTopicPermission);
Expand Down
Loading
Loading