Skip to content

Commit

Permalink
[BE] Refactor/#540 지연로딩의 경우, soft delete을 반영할 수 없는 문제 해결 (#553)
Browse files Browse the repository at this point in the history
* fix: Pin @where 적용으로 지연 로딩 시 soft delete 반영

* fix: Pin 삭제 시 pinCount 반영하도록 수정

- 검토할 TODO 다수 존재

* fix: Pin @where 적용에 따라 JPQL 원복

* refactor: Member update 메서드 형식 통일

* fix: Topic @where 적용으로 지연 로딩 시 soft delete 반영

* fix: PinImage @where 적용으로 지연 로딩 시 soft delete 반영

* refactor: 메서드명 수정

* refactor: AdminCommandService 의 불필요한 EntityManager 의존성 제거

* refactor: 누락된 삭제 로직 TODO 작성

* refactor: 개행 수정

* refactor: 개행 수정

* fix: 토픽 삭제 중복 로직이나 사용되지 않는 API 코드 @deprecated 처리

* fix: 토픽 삭제 시 연관 관계 지우지 않는 부분 수정

* fix: AuthMember 생성 시 정상 사용자만 조회하도록 filter 추가, 검증 추가

* test: AuthServiceTest 작성

* chore: /permissions/id API @deprecated 추가
  • Loading branch information
yoondgu authored Oct 6, 2023
1 parent e11eab0 commit 9776f6e
Show file tree
Hide file tree
Showing 38 changed files with 389 additions and 215 deletions.
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);
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);

@Modifying(clearAutomatically = true)
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) {
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")
@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

0 comments on commit 9776f6e

Please sign in to comment.