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] Refactor/#540 지연로딩의 경우, soft delete을 반영할 수 없는 문제 해결 #553

Merged
merged 16 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.mapbefine.mapbefine.admin.application;

import static com.mapbefine.mapbefine.pin.exception.PinErrorCode.PIN_NOT_FOUND;
import static com.mapbefine.mapbefine.topic.exception.TopicErrorCode.TOPIC_NOT_FOUND;

import com.mapbefine.mapbefine.atlas.domain.AtlasRepository;
Expand All @@ -11,14 +12,14 @@
import com.mapbefine.mapbefine.pin.domain.Pin;
import com.mapbefine.mapbefine.pin.domain.PinImageRepository;
import com.mapbefine.mapbefine.pin.domain.PinRepository;
import com.mapbefine.mapbefine.pin.exception.PinException.PinNotFoundException;
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicRepository;
import com.mapbefine.mapbefine.topic.exception.TopicException;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;
import java.util.NoSuchElementException;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional
Expand Down Expand Up @@ -61,12 +62,15 @@ private void deleteAllRelatedMember(Member member) {
List<Long> pinIds = extractPinIdsByMember(member);
Long memberId = member.getId();

pinImageRepository.deleteAllByPinIds(pinIds);
topicRepository.deleteAllByMemberId(memberId);
pinRepository.deleteAllByMemberId(memberId);
permissionRepository.deleteAllByMemberId(memberId);
permissionRepository.flush();
atlasRepository.deleteAllByMemberId(memberId);
atlasRepository.flush();
bookmarkRepository.deleteAllByMemberId(memberId);
bookmarkRepository.flush();
pinImageRepository.deleteAllByPinIds(pinIds);
pinRepository.deleteAllByMemberId(memberId);
topicRepository.deleteAllByMemberId(memberId);
}

private Member findMemberById(Long id) {
Expand All @@ -82,21 +86,44 @@ private List<Long> extractPinIdsByMember(Member member) {
}

public void deleteTopic(Long topicId) {
Topic topic = findTopicById(topicId);
List<Long> pinIds = extractPinIdsByTopic(topic);

permissionRepository.deleteAllByTopicId(topicId);
permissionRepository.flush();
atlasRepository.deleteAllByTopicId(topicId);
atlasRepository.flush();
bookmarkRepository.deleteAllByTopicId(topicId);
bookmarkRepository.flush();
pinImageRepository.deleteAllByPinIds(pinIds);
pinRepository.deleteAllByTopicId(topicId);
Comment on lines +92 to +99
Copy link
Collaborator

Choose a reason for hiding this comment

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

하나의 repository 가 flush 의무를 갖는 것이 애매하기 때문에 모든 repository 가 flush 를 호출할 수 있도록 하셨군요! 좋습니다.

그리고, 이 부분은 추후에 개선할 수 있으면 개선하도록 해보죠! (명시적으로 flush 호출 안해보도록)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

앗 저 각각 해야 하는줄알았어요...ㅎㅎ 근데 매튜 말대로 곧 사라질 내용인걸로!

topicRepository.deleteById(topicId);
}

private List<Long> extractPinIdsByTopic(Topic topic) {
return topic.getPins()
.stream()
.map(Pin::getId)
.toList();
}

public void deleteTopicImage(Long topicId) {
Topic topic = findTopicById(topicId);
topic.removeImage();
}

private Topic findTopicById(Long topicId) {
return topicRepository.findByIdAndIsDeletedFalse(topicId)
return topicRepository.findById(topicId)
.orElseThrow(() -> new TopicException.TopicNotFoundException(TOPIC_NOT_FOUND, List.of(topicId)));
}

public void deletePin(Long pinId) {
pinRepository.deleteById(pinId);
Pin pin = pinRepository.findById(pinId)
.orElseThrow(() -> new PinNotFoundException(PIN_NOT_FOUND, pinId));

pin.decreaseTopicPinCount();
pinImageRepository.deleteAllByPinId(pinId);
pinRepository.deleteById(pin.getId());
}

public void deletePinImage(Long pinImageId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@
import com.mapbefine.mapbefine.member.domain.MemberRepository;
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicRepository;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.NoSuchElementException;
import java.util.Objects;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional
Expand Down Expand Up @@ -73,7 +72,7 @@ private boolean isTopicAlreadyAdded(Long topicId, Long memberId) {
}

private Topic findTopicById(Long topicId) {
return topicRepository.findByIdAndIsDeletedFalse(topicId)
return topicRepository.findById(topicId)
.orElseThrow(() -> new AtlasBadRequestException(ILLEGAL_TOPIC_ID));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
public interface AtlasRepository extends JpaRepository<Atlas, Long> {

boolean existsByMemberIdAndTopicId(Long memberId, Long topicId);

void deleteByMemberIdAndTopicId(Long memberId, Long topicId);

void deleteAllByMemberId(Long memberId);

void deleteAllByTopicId(Long topicId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import com.mapbefine.mapbefine.topic.domain.Topic;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

Expand All @@ -32,9 +31,12 @@ public void validateMember(Long memberId) {
}

public AuthMember findAuthMemberByMemberId(Long memberId) {
return memberRepository.findById(memberId)
.map(this::convertToAuthMember)
Member member = memberRepository.findById(memberId)
.orElseThrow(() -> new IllegalArgumentException("findAuthMemberByMemberId; memberId= " + memberId));
if (member.isNormalStatus()) {
return convertToAuthMember(member);
}
throw new AuthUnauthorizedException(AuthErrorCode.ILLEGAL_MEMBER_ID);
}

private AuthMember convertToAuthMember(Member member) {
Expand Down Expand Up @@ -63,15 +65,4 @@ private List<Long> getCreatedTopics(Member member) {
.toList();
}

public boolean isAdmin(Long memberId) {
if (Objects.isNull(memberId)) {
return false;
}

Optional<Member> member = memberRepository.findById(memberId);

return member.map(Member::isAdmin)
.orElse(false);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@
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;

import java.util.NoSuchElementException;

@Service
@Transactional
public class BookmarkCommandService {
Expand Down Expand Up @@ -53,7 +52,7 @@ public Long addTopicInBookmark(AuthMember authMember, Long topicId) {
}

private Topic getTopicById(Long topicId) {
return topicRepository.findByIdAndIsDeletedFalse(topicId)
return topicRepository.findById(topicId)
.orElseThrow(() -> new BookmarkBadRequestException(ILLEGAL_TOPIC_ID));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
package com.mapbefine.mapbefine.bookmark.domain;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;

import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;

public interface BookmarkRepository extends JpaRepository<Bookmark, Long> {

Optional<Bookmark> findByMemberIdAndTopicId(Long memberId, Long topicId);

boolean existsByMemberIdAndTopicId(Long memberId, Long topicId);

Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 쥬니 글에서 보았듯 @query 어노테이션이 동반되지 않으면 짜피 clearAutomatically 가 동작하지 않으니까 빼신게 맞나요 ?

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.

또잉또잉 클릭 잘못했어연 바로 아래줄일듯 허허허

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

맞아요!

@Modifying(clearAutomatically = true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 부분 JPQL로 해결해야하는데... 도이꺼 PR 머지되면 브랜치 따서 해결할게요.

잊지말자 !!!!!!!!!!!!

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

@yoondgu yoondgu Oct 5, 2023

Choose a reason for hiding this comment

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

Service에서 EntityManager 불러오는 대신 JPQL로 바꾸고 @Modifying 다시 적용한다는거죵 ?
아주 좋습니다아

void deleteAllByMemberId(Long memberId);

Optional<Bookmark> findByMemberIdAndTopicId(Long memberId, Long topicId);
void deleteAllByTopicId(Long topicId);

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public void updateInfoById(AuthMember authMember, MemberUpdateRequest request) {

validateNicknameDuplicated(nickName);

member.update(nickName);
member.updateNickName(nickName);
}

private Member findMemberById(Long memberId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public List<MemberResponse> findAll() {
public List<TopicResponse> findAllTopicsInBookmark(AuthMember authMember) {
Member member = findMemberById(authMember.getMemberId());

List<Topic> bookMarkedTopics = topicRepository.findTopicsByBookmarksMemberIdAndIsDeletedFalse(
List<Topic> bookMarkedTopics = topicRepository.findTopicsByBookmarksMemberId(
authMember.getMemberId());
return bookMarkedTopics.stream()
.map(topic -> TopicResponse.from(
Expand All @@ -90,7 +90,6 @@ private boolean isInAtlas(Long memberId, Long topicId) {

public List<TopicResponse> findAllTopicsInAtlas(AuthMember authMember) {
Member member = findMemberById(authMember.getMemberId());

List<Topic> topicsInAtlas = findTopicsInAtlas(member);

return topicsInAtlas.stream()
Expand All @@ -107,7 +106,6 @@ private boolean isInBookmark(Long memberId, Long topicId) {
}

public List<TopicResponse> findMyAllTopics(AuthMember authMember) {

Long memberId = authMember.getMemberId();
Member member = findMemberById(memberId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,16 @@ private static String createNicknameSuffix() {
.substring(0, DEFAULT_NICKNAME_SUFFIX_LENGTH);
}

public void update(
public void updateNickName(
String nickName
) {
memberInfo = memberInfo.createUpdatedMemberInfo(nickName);
}

public void updateStatus(Status status) {
memberInfo = memberInfo.createUpdatedMemberInfo(status);
}

public void addTopic(Topic topic) {
Comment on lines +113 to 117
Copy link
Collaborator

Choose a reason for hiding this comment

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

형태를 통일해주셨군요 굳

createdTopics.add(topic);
}
Expand Down Expand Up @@ -147,14 +151,4 @@ public List<Topic> getTopicsWithPermissions() {
public boolean isNormalStatus() {
return memberInfo.getStatus() == Status.NORMAL;
}

public void updateStatus(Status status) {
memberInfo = MemberInfo.of(
memberInfo.getNickName(),
memberInfo.getEmail(),
memberInfo.getImageUrl(),
memberInfo.getRole(),
status
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ public MemberInfo createUpdatedMemberInfo(String nickName) {
return MemberInfo.of(nickName, this.email, this.imageUrl.getImageUrl(), this.role, this.status);
}

public MemberInfo createUpdatedMemberInfo(Status status) {

return MemberInfo.of(this.nickName, this.email, this.imageUrl.getImageUrl(), this.role, status);
}

public String getImageUrl() {
return imageUrl.getImageUrl();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@
import com.mapbefine.mapbefine.permission.exception.PermissionException.PermissionForbiddenException;
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicRepository;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;
import java.util.Objects;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional
Expand Down Expand Up @@ -78,7 +77,7 @@ private boolean isNotSelfMember(AuthMember authMember, Member member) {

private Topic findTopic(PermissionRequest request) {
Long topicId = request.topicId();
return topicRepository.findByIdAndIsDeletedFalse(topicId)
return topicRepository.findById(topicId)
.orElseThrow(() -> new PermissionBadRequestException(ILLEGAL_TOPIC_ID, topicId));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@
import com.mapbefine.mapbefine.topic.domain.TopicStatus;
import com.mapbefine.mapbefine.topic.exception.TopicErrorCode;
import com.mapbefine.mapbefine.topic.exception.TopicException.TopicNotFoundException;
import java.util.List;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;

@Service
@Transactional(readOnly = true)
public class PermissionQueryService {
Expand All @@ -43,12 +42,13 @@ public TopicAccessDetailResponse findTopicAccessDetailById(Long topicId) {
}

private Publicity findTopicPublicityById(Long topicId) {
return topicRepository.findByIdAndIsDeletedFalse(topicId)
return topicRepository.findById(topicId)
.map(Topic::getTopicStatus)
.map(TopicStatus::getPublicity)
.orElseThrow(() -> new TopicNotFoundException(TopicErrorCode.TOPIC_NOT_FOUND, topicId));
}

@Deprecated(since = "2023.10.06")
public PermissionMemberDetailResponse findPermissionById(Long permissionId) {
Permission permission = permissionRepository.findById(permissionId)
.orElseThrow(() -> new PermissionNotFoundException(PERMISSION_NOT_FOUND, permissionId));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,7 @@ public interface PermissionRepository extends JpaRepository<Permission, Long> {
boolean existsByTopicIdAndMemberId(Long topicId, Long memberId);

void deleteAllByMemberId(Long memberId);

void deleteAllByTopicId(Long topicId);

}
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ public ResponseEntity<TopicAccessDetailResponse> findTopicAccessDetailByTopicId(
return ResponseEntity.ok(response);
}

// TODO 이 API를 쓰는 곳이 있나? + 결국 특정 회원을 조회하는 건데 어떤 API인지 알기 어렵다..
// 회원 정보 조회는 /members 에서 하는 걸로 충분하지 않나? 재사용성이 떨어진다. 테스트의 DisplayName도 매칭이 안된다.
@Deprecated(since = "2023.10.06")
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@LoginRequired
@GetMapping("/{permissionId}")
public ResponseEntity<PermissionMemberDetailResponse> findPermissionById(@PathVariable Long permissionId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,12 @@
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicRepository;
import com.mapbefine.mapbefine.topic.exception.TopicException.TopicBadRequestException;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.multipart.MultipartFile;

import java.util.List;
import java.util.NoSuchElementException;
import java.util.Objects;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.multipart.MultipartFile;

@Transactional
@Service
Expand Down Expand Up @@ -100,7 +99,7 @@ private Topic findTopic(Long topicId) {
if (Objects.isNull(topicId)) {
throw new TopicBadRequestException(ILLEGAL_TOPIC_ID);
}
return topicRepository.findByIdAndIsDeletedFalse(topicId)
return topicRepository.findById(topicId)
.orElseThrow(() -> new TopicBadRequestException(ILLEGAL_TOPIC_ID));
}

Expand Down Expand Up @@ -146,7 +145,7 @@ public void update(
}

private Pin findPin(Long pinId) {
return pinRepository.findByIdAndIsDeletedFalse(pinId)
return pinRepository.findById(pinId)
.orElseThrow(() -> new PinBadRequestException(ILLEGAL_PIN_ID));
}

Expand Down Expand Up @@ -182,7 +181,7 @@ public void removeImageById(AuthMember authMember, Long pinImageId) {
}

private PinImage findPinImage(Long pinImageId) {
return pinImageRepository.findByIdAndIsDeletedFalse(pinImageId)
return pinImageRepository.findById(pinImageId)
.orElseThrow(() -> new PinBadRequestException(ILLEGAL_PIN_IMAGE_ID));
}

Expand Down
Loading
Loading