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] Feat/#586 핀 수정 시 변경 이력 저장 이벤트 구현 #591

Merged
merged 21 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0bd7c7c
refactor: 테스트 환경 콘솔 로그 출력 설정 추가
yoondgu Oct 16, 2023
e6a4f40
test: 핀 수정 인수 테스트 추가
yoondgu Oct 16, 2023
65ef31b
feat: 핀 수정 시 수정 이력 저장 구현 및 테스트
yoondgu Oct 16, 2023
0dd8fc7
refactor: 사용하지 않는 변수 삭제
yoondgu Oct 16, 2023
19bcc6f
feat: 새 핀 추가시에도 핀 이력 저장
yoondgu Oct 16, 2023
ae2bc06
feat: 핀 수정 이력이 참조하는 엔티티 삭제 시 soft delete 하도록 이벤트 구현
yoondgu Oct 16, 2023
57073b9
feat: 핀 수정 이력은 삭제 개념이 없는 것으로 복구 (이전 커밋 revert)
yoondgu Oct 16, 2023
2a7f4db
test: 핀 수정 이력 이벤트 테스트 실패 수정
yoondgu Oct 16, 2023
a9b5308
test: 빠른 테스트 조건문 확인을 위해 assertAll softly로 변경
yoondgu Oct 16, 2023
9ded478
refactor: hard delete 메서드 Query 작성해서 Modifying 적용
yoondgu Oct 16, 2023
ca92508
rename: PinUpdateHistory 에서 PinHistory로 네이밍 변경
yoondgu Oct 16, 2023
8ed8da4
test: 테스트 displayName 핀 정보 이력 관련 용어 통일
yoondgu Oct 16, 2023
0309e78
fix: 핀 변경 이력 엔티티 컬럼 수정
yoondgu Oct 16, 2023
8444f53
refactor: 불필요한 공백 제거
yoondgu Oct 17, 2023
02519c7
test: 일시 검증 메서드 수정, 핀 변경 일시 검증 추가
yoondgu Oct 17, 2023
20cb321
feat: 핀 변경 이력 엔티티에 핀 변경 일시 컬럼 추가
yoondgu Oct 17, 2023
b257b24
fix: PinHistoryCommandService 트랜잭션 어노테이션 추가
yoondgu Oct 17, 2023
5c1bbdb
fix: 핀 변경 이력 롤백 테스트 예외 수정, 실패 테스트 disabled 처리
yoondgu Oct 17, 2023
2fc368c
fix: 이벤트를 통한 롤백 테스트 통합 테스트로 이동
yoondgu Oct 17, 2023
3592ff0
chore: #548과 충돌해결을 위한 merge
yoondgu Oct 17, 2023
5bfbcfc
test: 새로운 테스트에 테스트 컨테이너 적용
yoondgu Oct 17, 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
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public AdminCommandService(
public void blockMember(Long memberId) {
Member member = findMemberById(memberId);
member.updateStatus(Status.BLOCKED);
memberRepository.flush();
Copy link
Collaborator

Choose a reason for hiding this comment

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

도이가 쓴 글에 의하면 hardDelete 가 진행되기 이전에 flush 가 발생하면 되고, 그렇다면 permissionRepository.deleteAllByMemberId() 를 호출하게 되면서 flush 가 발생하게 되니까명시적인 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.

맞아요. Repository에서 제공하는 flush를 통해 member의 update 쿼리도 같이 나갈 줄 알았는데,
그렇지 않아서 추가해주게 되었어요 ㅜㅜ
@Modifying으로 실행되는 flush는 해당 repository 메서드의 쿼리만 flush해주는 걸까요?
원인은 더 탐구해보는 것으로..


deleteAllRelatedMember(member);
}
Expand All @@ -63,11 +64,8 @@ private void deleteAllRelatedMember(Member member) {
Long memberId = member.getId();

permissionRepository.deleteAllByMemberId(memberId);
permissionRepository.flush();
atlasRepository.deleteAllByMemberId(memberId);
atlasRepository.flush();
bookmarkRepository.deleteAllByMemberId(memberId);
bookmarkRepository.flush();
pinImageRepository.deleteAllByPinIds(pinIds);
pinRepository.deleteAllByMemberId(memberId);
topicRepository.deleteAllByMemberId(memberId);
Expand All @@ -90,11 +88,8 @@ public void deleteTopic(Long 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.mapbefine.mapbefine.atlas.domain;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
import org.springframework.stereotype.Repository;

@Repository
Expand All @@ -10,7 +12,11 @@ public interface AtlasRepository extends JpaRepository<Atlas, Long> {

void deleteByMemberIdAndTopicId(Long memberId, Long topicId);

@Modifying(clearAutomatically = true)
@Query("delete from Atlas a where a.member.id = :memberId")
void deleteAllByMemberId(Long memberId);

@Modifying(clearAutomatically = true)
@Query("delete from Atlas a where a.topic.id = :topicId")
void deleteAllByTopicId(Long topicId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,21 @@

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

public interface BookmarkRepository extends JpaRepository<Bookmark, Long> {

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

boolean existsByMemberIdAndTopicId(Long memberId, Long topicId);

@Modifying(clearAutomatically = true)
@Query("delete from Bookmark b where b.member.id = :memberId")
Comment on lines +14 to +15
Copy link
Collaborator

Choose a reason for hiding this comment

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

flush 를 왜 다 지우셨나 했는데 @query 어노테이션도 추가적으로 붙여주시면서 @Modifying 어노테이션이 동작하게 해주셨군요! 굳굳

void deleteAllByMemberId(Long memberId);

@Modifying(clearAutomatically = true)
@Query("delete from Bookmark b where b.topic.id = :topicId")
void deleteAllByTopicId(Long topicId);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.mapbefine.mapbefine.history.application;

import com.mapbefine.mapbefine.history.domain.PinHistory;
import com.mapbefine.mapbefine.history.domain.PinHistoryRepository;
import com.mapbefine.mapbefine.pin.domain.Pin;
import com.mapbefine.mapbefine.pin.event.PinUpdateEvent;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Slf4j
@Transactional
@Service
public class PinHistoryCommandService {

private final PinHistoryRepository pinHistoryRepository;

public PinHistoryCommandService(PinHistoryRepository pinHistoryRepository) {
this.pinHistoryRepository = pinHistoryRepository;
}

@EventListener
public void saveHistory(PinUpdateEvent event) {
Pin pin = event.pin();
pinHistoryRepository.save(new PinHistory(pin, event.member()));

log.debug("pin history saved for update pin id =: {}", pin.getId());
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package com.mapbefine.mapbefine.history.domain;

import static lombok.AccessLevel.PROTECTED;

import com.mapbefine.mapbefine.common.entity.BaseTimeEntity;
import com.mapbefine.mapbefine.member.domain.Member;
import com.mapbefine.mapbefine.pin.domain.Pin;
import com.mapbefine.mapbefine.pin.domain.PinInfo;
import jakarta.persistence.Column;
import jakarta.persistence.Embedded;
import jakarta.persistence.Entity;
import jakarta.persistence.EntityListeners;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import java.time.LocalDateTime;
import lombok.Getter;
import lombok.NoArgsConstructor;
import org.springframework.data.jpa.domain.support.AuditingEntityListener;

@EntityListeners(AuditingEntityListener.class)
@Entity
@NoArgsConstructor(access = PROTECTED)
@Getter
public class PinHistory extends BaseTimeEntity {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@ManyToOne
@JoinColumn(name = "pin_id", nullable = false)
private Pin pin;

@ManyToOne
@JoinColumn(name = "member_id", nullable = false)
private Member member;

@Embedded
private PinInfo pinInfo;

@Column(name = "pin_updated_at", nullable = false)
private LocalDateTime pinUpdatedAt;
Comment on lines +44 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.

BaseEntitycreatedAt과 동일하지 않나요 ?

헷갈리네요..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

BaseEntity의 createdAt은 해당 엔티티가 영속화될때 저장되는데
변경 이력 저장 시점에 따라 pinUpdatedAt과 미세하게 다를 수 있습니다.
@PrePersist 에서 해주기에는 Pin이 PinHistory를 몰라야 한다고 생각하고요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Pin을 업데이트 할 때에도, pin.updatedAt은 영속화 시점에 결정되지않나요 ?
같은 트랜잭션으로 묶여있는 현 상황에서도 서로 불일치할 수 있는건가요 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

createdAt, updatedAt 모두 종합해서 말씀드린거였어요!!
JpaAuditing은 해당하는 엔티티에 대해 persist를 호출하기 전동작하기 때문에
Pin에 대한 persist가 호출되기 전PinUpdateEvent에 대한 이벤트리스너가 동작-> PinHistory에 대한 persist가 호출되기 전은 다르지 않나요..?! 혹시 제가 잘못 생각하고 있는 부분이 있을까요?

        pinRepository.save(pin);
        eventPublisher.publishEvent(new PinUpdateEvent(pin, member)); // pinHistoryRepository.save(...);

++ 그리고 같은 트랜잭션으로 묶여 있다 해도, 아래 코멘트에서 말씀해주신 것처럼 서로 다른 트랜잭션으로 가져가도록 변경할 여지가 있다면 별개로 관리하는 건 어떨까요? BaseEntity는 기본적으로 저장하는, '레코드'에 대한 시간 정보라는 생각도 들고요!


public PinHistory(Pin pin, Member member) {
this.pin = pin;
PinInfo history = pin.getPinInfo();
this.pinInfo = PinInfo.of(history.getName(), history.getDescription());
this.pinUpdatedAt = pin.getUpdatedAt();
this.member = member;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.mapbefine.mapbefine.history.domain;

import java.util.List;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;

@Repository
public interface PinHistoryRepository extends JpaRepository<PinHistory, Long> {
List<PinHistory> findAllByPinId(Long pinId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,21 @@

import java.util.List;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;

public interface PermissionRepository extends JpaRepository<Permission, Long> {

List<Permission> findAllByTopicId(Long topicId);

boolean existsByTopicIdAndMemberId(Long topicId, Long memberId);

@Modifying(clearAutomatically = true)
@Query("delete from Permission p where p.member.id = :memberId")
void deleteAllByMemberId(Long memberId);

@Modifying(clearAutomatically = true)
@Query("delete from Permission p where p.topic.id = :topicId")
void deleteAllByTopicId(Long topicId);

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.mapbefine.mapbefine.pin.dto.request.PinCreateRequest;
import com.mapbefine.mapbefine.pin.dto.request.PinImageCreateRequest;
import com.mapbefine.mapbefine.pin.dto.request.PinUpdateRequest;
import com.mapbefine.mapbefine.pin.event.PinUpdateEvent;
import com.mapbefine.mapbefine.pin.exception.PinException.PinBadRequestException;
import com.mapbefine.mapbefine.pin.exception.PinException.PinForbiddenException;
import com.mapbefine.mapbefine.topic.domain.Topic;
Expand All @@ -30,16 +31,20 @@
import java.util.List;
import java.util.NoSuchElementException;
import java.util.Objects;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.multipart.MultipartFile;

@Slf4j
@Transactional
@Service
public class PinCommandService {

private static final double DUPLICATE_LOCATION_DISTANCE_METERS = 10.0;

private final ApplicationEventPublisher eventPublisher;
private final PinRepository pinRepository;
private final LocationRepository locationRepository;
private final TopicRepository topicRepository;
Expand All @@ -48,13 +53,15 @@ public class PinCommandService {
private final ImageService imageService;

public PinCommandService(
ApplicationEventPublisher eventPublisher,
PinRepository pinRepository,
LocationRepository locationRepository,
TopicRepository topicRepository,
MemberRepository memberRepository,
PinImageRepository pinImageRepository,
ImageService imageService
) {
this.eventPublisher = eventPublisher;
this.pinRepository = pinRepository;
this.locationRepository = locationRepository;
this.topicRepository = topicRepository;
Expand All @@ -81,8 +88,8 @@ public long save(
);

addPinImagesToPin(images, pin);

pinRepository.save(pin);
eventPublisher.publishEvent(new PinUpdateEvent(pin, member));
Copy link
Collaborator

Choose a reason for hiding this comment

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

기가 막힙니다!


return pin.getId();
}
Expand Down Expand Up @@ -139,10 +146,12 @@ public void update(
Long pinId,
PinUpdateRequest request
) {
Member member = findMember(authMember.getMemberId());
Pin pin = findPin(pinId);
validatePinCreateOrUpdate(authMember, pin.getTopic());

pin.updatePinInfo(request.name(), request.description());
eventPublisher.publishEvent(new PinUpdateEvent(pin, member));
}

private Pin findPin(Long pinId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ public double getLongitude() {
return location.getLongitude();
}

public String getDescription() {
return pinInfo.getDescription();
}

public String getRoadBaseAddress() {
Address address = location.getAddress();
return address.getRoadBaseAddress();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.mapbefine.mapbefine.pin.event;

import com.mapbefine.mapbefine.member.domain.Member;
import com.mapbefine.mapbefine.pin.domain.Pin;

public record PinUpdateEvent(Pin pin, Member member) {
}
16 changes: 8 additions & 8 deletions backend/src/main/resources/logback-spring.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,25 @@
<property name="LOG_PATTERN_COLORED"
value="%green(%d{yyyy-MM-dd HH:mm:ss.SSS}) [%thread] ${PID} %highlight(%-5level) %cyan(%logger) - %msg%n"/>

<springProfile name="default">
<include resource="logback/console-appender.xml"/>
<include resource="logback/console-appender.xml"/>

<root level="INFO">
<root level="INFO">
<appender-ref ref="CONSOLE"/>
</root>

<springProfile name="test">
<logger name="com.mapbefine.mapbefine" level="DEBUG" additivity="false">
<appender-ref ref="CONSOLE"/>
</root>
</logger>
</springProfile>

<springProfile name="dev">
<include resource="logback/console-appender.xml"/>
<include resource="logback/file-hibernate-appender.xml"/>
<include resource="logback/file-debug-appender.xml"/>
<include resource="logback/file-warn-appender.xml"/>
<include resource="logback/file-error-appender.xml"/>

<root level="INFO">
<appender-ref ref="CONSOLE"/>
<appender-ref ref="FILE_WARN"/>
<appender-ref ref="FILE_ERROR"/>
</root>
Expand All @@ -38,14 +40,12 @@
</springProfile>

<springProfile name="prod">
<include resource="logback/console-appender.xml"/>
<include resource="logback/file-info-appender.xml"/>
<include resource="logback/file-warn-appender.xml"/>
<include resource="logback/file-error-appender.xml"/>
<include resource="logback/slack-error-appender.xml"/>

<root level="INFO">
<appender-ref ref="CONSOLE"/>
<appender-ref ref="FILE_WARN"/>
<appender-ref ref="FILE_ERROR"/>
<appender-ref ref="SLACK_ERROR"/>
Expand Down
Loading
Loading