Skip to content
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
Expand Up @@ -2,8 +2,7 @@

import life.mosu.mosuserver.application.auth.PrincipalDetails;
import life.mosu.mosuserver.domain.user.UserJpaEntity;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorCode;
import life.mosu.mosuserver.global.cookie.TokenCookies;
import life.mosu.mosuserver.presentation.auth.dto.Token;
import lombok.RequiredArgsConstructor;
import org.springframework.security.core.Authentication;
Expand All @@ -24,16 +23,12 @@ public Token generateAuthToken(final UserJpaEntity user) {
accessTokenProvider.expireTime, refreshTokenProvider.expireTime);
}

public Token reissueAccessToken(final String refreshTokenString) {
if (refreshTokenString == null) {
throw new CustomRuntimeException(ErrorCode.NOT_FOUND_REFRESH_TOKEN);
}
public Token reissueToken(final TokenCookies tokenCookies) {

final Authentication authentication = refreshTokenProvider.getAuthentication(
refreshTokenString);
tokenCookies.refreshToken());
final PrincipalDetails principalDetails = (PrincipalDetails) authentication.getPrincipal();
final UserJpaEntity user = principalDetails.user();

refreshTokenProvider.invalidateToken(user.getId());

return generateAuthToken(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@
@Service
public class RefreshTokenProvider extends JwtTokenProvider {

private final RefreshTokenRepository refreshTokenRepository;
private static final String TOKEN_TYPE = "Refresh";
private static final String HEADER = "Refresh-Token";
private final RefreshTokenRepository refreshTokenRepository;

public RefreshTokenProvider(
@Value("${jwt.refresh-token.expire-time}") final Long expireTime,
@Value("${jwt.secret}") final String secretKey,
final UserJpaRepository userJpaRepository,
final RefreshTokenRepository refreshTokenRepository
@Value("${jwt.refresh-token.expire-time}") final Long expireTime,
@Value("${jwt.secret}") final String secretKey,
final UserJpaRepository userJpaRepository,
final RefreshTokenRepository refreshTokenRepository
) {
super(expireTime, secretKey, TOKEN_TYPE, HEADER, userJpaRepository);
this.refreshTokenRepository = refreshTokenRepository;
Expand All @@ -36,18 +36,18 @@ protected Claims validateAndParseToken(final String token) {

public void invalidateToken(final Long id) {
if (!refreshTokenRepository.existsByUserId(id)) {
throw new CustomRuntimeException(ErrorCode.NOT_EXIST_REFRESH_TOKEN);
throw new CustomRuntimeException(ErrorCode.NOT_FOUND_REFRESH_TOKEN);
}
refreshTokenRepository.deleteByUserId(id);
}

public void cacheToken(final Long userId, final String refreshToken) {
refreshTokenRepository.save(
RefreshToken.of(
userId,
refreshToken,
expireTime
)
RefreshToken.of(
userId,
refreshToken,
expireTime
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@
import life.mosu.mosuserver.domain.inquiryAnswer.InquiryAnswerJpaRepository;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorCode;
import life.mosu.mosuserver.infra.notify.NotifyEventPublisher;
import life.mosu.mosuserver.infra.notify.dto.NotificationEvent;
import life.mosu.mosuserver.infra.notify.dto.NotificationStatus;
import life.mosu.mosuserver.presentation.inquiry.dto.InquiryAnswerRequest;
import life.mosu.mosuserver.presentation.inquiry.dto.InquiryAnswerUpdateRequest;
import life.mosu.mosuserver.presentation.inquiry.dto.InquiryDetailResponse;
Expand All @@ -27,21 +24,29 @@ public class InquiryAnswerService {
private final InquiryAnswerAttachmentService answerAttachmentService;
private final InquiryAnswerJpaRepository inquiryAnswerJpaRepository;
private final InquiryJpaRepository inquiryJpaRepository;
private final NotifyEventPublisher notifier;

private final InquiryAnswerTxService eventTxService;

@Transactional
public void createInquiryAnswer(Long postId, InquiryAnswerRequest request) {
isAnswerAlreadyRegister(postId);
InquiryJpaEntity inquiryEntity = getInquiry(postId);
Long userId = inquiryEntity.getUserId();

InquiryAnswerJpaEntity answerEntity = inquiryAnswerJpaRepository.save(
request.toEntity(postId));
try {
InquiryAnswerJpaEntity answerEntity = inquiryAnswerJpaRepository.save(
request.toEntity(postId));

answerAttachmentService.createAttachment(request.attachments(), answerEntity);
inquiryEntity.updateStatusToComplete();
answerAttachmentService.createAttachment(request.attachments(), answerEntity);
inquiryEntity.updateStatusToComplete();

eventTxService.publishSuccessEvent(userId, postId);

} catch (Exception ex) {
log.error("문의 답변 등록 실패: {}", ex.getMessage(), ex);
throw ex;
}
Comment on lines +45 to +48

Choose a reason for hiding this comment

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

medium

Catching the generic Exception is risky here. If a checked exception were thrown inside the try block, Spring's default transactional behavior would lead to a commit of the transaction upon re-throwing the checked exception. This is likely not the intended behavior, as any failure should probably result in a rollback.

To ensure correctness, it's safer to catch RuntimeException or wrap the caught exception in a new RuntimeException.

Suggested change
} catch (Exception ex) {
log.error("문의 답변 등록 실패: {}", ex.getMessage(), ex);
throw ex;
}
} catch (RuntimeException ex) {
log.error("문의 답변 등록 실패: {}", ex.getMessage(), ex);
throw ex;
}


sendNotification(userId, postId);

}

Expand Down Expand Up @@ -92,11 +97,5 @@ private void isAnswerAlreadyRegister(Long postId) {
}
}

private void sendNotification(Long userId, Long postId) {
NotificationEvent event = NotificationEvent.create(
NotificationStatus.INQUIRY_ANSWER_SUCCESS, userId, postId);
notifier.notify(event);
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package life.mosu.mosuserver.application.inquiry;

import life.mosu.mosuserver.application.inquiry.tx.InquiryAnswerContext;
import life.mosu.mosuserver.application.inquiry.tx.InquiryAnswerTxEventFactory;
import life.mosu.mosuserver.global.tx.TxEvent;
import life.mosu.mosuserver.global.tx.TxEventPublisher;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Slf4j
@Service
@RequiredArgsConstructor
public class InquiryAnswerTxService {

private final TxEventPublisher txEventPublisher;
private final InquiryAnswerTxEventFactory eventFactory;

@Transactional
public void publishSuccessEvent(Long userId, Long inquiryId) {
TxEvent<?> event = eventFactory.create(InquiryAnswerContext.ofSuccess(userId, inquiryId));
txEventPublisher.publish(event);
}
Comment on lines +20 to +24
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add parameter validation for business logic integrity.

The method accepts userId and inquiryId without validation, which could lead to invalid events being published if null or invalid IDs are passed.

Apply this diff to add validation:

 @Transactional
 public void publishSuccessEvent(Long userId, Long inquiryId) {
+    if (userId == null || userId <= 0) {
+        throw new IllegalArgumentException("UserId must be a positive number");
+    }
+    if (inquiryId == null || inquiryId <= 0) {
+        throw new IllegalArgumentException("InquiryId must be a positive number");
+    }
+    
     TxEvent<?> event = eventFactory.create(InquiryAnswerContext.ofSuccess(userId, inquiryId));
     txEventPublisher.publish(event);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Transactional
public void publishSuccessEvent(Long userId, Long inquiryId) {
TxEvent<?> event = eventFactory.create(InquiryAnswerContext.ofSuccess(userId, inquiryId));
txEventPublisher.publish(event);
}
@Transactional
public void publishSuccessEvent(Long userId, Long inquiryId) {
if (userId == null || userId <= 0) {
throw new IllegalArgumentException("UserId must be a positive number");
}
if (inquiryId == null || inquiryId <= 0) {
throw new IllegalArgumentException("InquiryId must be a positive number");
}
TxEvent<?> event = eventFactory.create(InquiryAnswerContext.ofSuccess(userId, inquiryId));
txEventPublisher.publish(event);
}
🤖 Prompt for AI Agents
In
src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerTxService.java
around lines 20 to 24, the method publishSuccessEvent accepts userId and
inquiryId without validation, risking invalid events. Add checks to ensure
neither userId nor inquiryId is null before proceeding. If either is null, throw
an IllegalArgumentException with a clear message to prevent publishing invalid
events and maintain business logic integrity.


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package life.mosu.mosuserver.application.inquiry.tx;

public record InquiryAnswerContext(
Long userId,
Long inquiryId,
Boolean isSuccess
) {

public static final InquiryAnswerContext ofSuccess(Long userId, Long inquiryId) {
return new InquiryAnswerContext(userId, inquiryId, true);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package life.mosu.mosuserver.application.inquiry.tx;

import life.mosu.mosuserver.global.tx.TxEvent;

public class InquiryAnswerTxEvent extends TxEvent<InquiryAnswerContext> {

public InquiryAnswerTxEvent(boolean isSuccess, InquiryAnswerContext context) {
super(isSuccess, context);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package life.mosu.mosuserver.application.inquiry.tx;

import life.mosu.mosuserver.global.tx.TxEvent;
import life.mosu.mosuserver.global.tx.TxEventFactory;
import org.springframework.stereotype.Component;

@Component
public class InquiryAnswerTxEventFactory implements TxEventFactory<InquiryAnswerContext> {

@Override
public TxEvent<?> create(InquiryAnswerContext context) {
return new InquiryAnswerTxEvent(context.isSuccess(), context);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package life.mosu.mosuserver.application.inquiry.tx;

import life.mosu.mosuserver.infra.notify.NotifyEventPublisher;
import life.mosu.mosuserver.infra.notify.dto.NotificationEvent;
import life.mosu.mosuserver.infra.notify.dto.NotificationStatus;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.event.TransactionPhase;
import org.springframework.transaction.event.TransactionalEventListener;

@Slf4j
@Component
@RequiredArgsConstructor
@Transactional(propagation = Propagation.NOT_SUPPORTED)
public class InquiryAnswerTxEventListener {

private final NotifyEventPublisher notifier;

@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void afterCommitHandler(InquiryAnswerTxEvent event) {
InquiryAnswerContext ctx = event.getContext();
log.debug("[AFTER_COMMIT] 문의 답변 등록 후 알림톡 발송 시작: userId={}, inquiryId={}", ctx.userId(),
ctx.inquiryId());

sendNotification(ctx.userId(), ctx.inquiryId());
}

private void sendNotification(Long userId, Long inquiryId) {
NotificationEvent notificationEvent = NotificationEvent.create(
NotificationStatus.INQUIRY_ANSWER_SUCCESS, userId, inquiryId);
notifier.notify(notificationEvent);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package life.mosu.mosuserver.application.profile;

import life.mosu.mosuserver.application.profile.tx.ProfileContext;
import life.mosu.mosuserver.application.profile.tx.ProfileTxEventFactory;
import life.mosu.mosuserver.global.tx.TxEvent;
import life.mosu.mosuserver.global.tx.TxEventPublisher;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Slf4j
@Service
@RequiredArgsConstructor
public class ProfileEventTxService {

private final TxEventPublisher txEventPublisher;
private final ProfileTxEventFactory eventFactory;

@Transactional
public void publishSuccessEvent(Long userId) {
TxEvent<?> event = eventFactory.create(ProfileContext.ofSuccess(userId));
txEventPublisher.publish(event);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,24 @@
import life.mosu.mosuserver.domain.user.UserRole;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorCode;
import life.mosu.mosuserver.infra.notify.NotifyEventPublisher;
import life.mosu.mosuserver.infra.notify.dto.NotificationEvent;
import life.mosu.mosuserver.infra.notify.dto.NotificationStatus;
import life.mosu.mosuserver.presentation.profile.dto.EditProfileRequest;
import life.mosu.mosuserver.presentation.profile.dto.ProfileDetailResponse;
import life.mosu.mosuserver.presentation.profile.dto.SignUpProfileRequest;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;

@Slf4j
@Service
@RequiredArgsConstructor
public class ProfileService {

private final UserJpaRepository userRepository;
private final ProfileJpaRepository profileJpaRepository;
private final NotifyEventPublisher notifier;

private final ProfileEventTxService eventTxService;

@Transactional
public void registerProfile(Long userId, SignUpProfileRequest request) {
Expand All @@ -33,13 +33,20 @@ public void registerProfile(Long userId, SignUpProfileRequest request) {
);
checkIfProfileExistsForUser(user);

ProfileJpaEntity profile = request.toEntity(userId);
profileJpaRepository.save(profile);
try {
ProfileJpaEntity profile = request.toEntity(userId);
profileJpaRepository.save(profile);

user.grantUserRole();
syncUserInfoFromProfile(user, request);

eventTxService.publishSuccessEvent(userId);

user.grantUserRole();
syncUserInfoFromProfile(user, request);
} catch (Exception ex) {
log.error("프로필 등록 실패: {}", ex.getMessage(), ex);
throw ex;
}
Comment on lines +45 to +48

Choose a reason for hiding this comment

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

medium

Similar to the InquiryAnswerService, this catch (Exception ex) block is too broad and could lead to incorrect transactional behavior. If a checked exception is thrown, the transaction might commit instead of rolling back.

To ensure the transaction correctly rolls back on any failure, please catch RuntimeException specifically.

Suggested change
} catch (Exception ex) {
log.error("프로필 등록 실패: {}", ex.getMessage(), ex);
throw ex;
}
} catch (RuntimeException ex) {
log.error("프로필 등록 실패: {}", ex.getMessage(), ex);
throw ex;
}


sendNotification();
}

@Transactional
Expand Down Expand Up @@ -71,10 +78,5 @@ private void syncUserInfoFromProfile(UserJpaEntity user, SignUpProfileRequest re
request.phoneNumber(), request.birth());
}
}

private void sendNotification() {
NotificationEvent event = NotificationEvent.create(NotificationStatus.SIGN_UP_SUCCESS);
notifier.notify(event);
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package life.mosu.mosuserver.application.profile.tx;

public record ProfileContext(
Long userId,
Boolean isSuccess
) {

public static ProfileContext ofSuccess(Long userId) {
return new ProfileContext(userId, true);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package life.mosu.mosuserver.application.profile.tx;

import life.mosu.mosuserver.global.tx.TxEvent;

public class ProfileTxEvent extends TxEvent<ProfileContext> {

public ProfileTxEvent(boolean isSuccess, ProfileContext context) {
super(isSuccess, context);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package life.mosu.mosuserver.application.profile.tx;

import life.mosu.mosuserver.global.tx.TxEvent;
import life.mosu.mosuserver.global.tx.TxEventFactory;
import org.springframework.stereotype.Component;

@Component
public class ProfileTxEventFactory implements TxEventFactory<ProfileContext> {

@Override
public TxEvent<?> create(ProfileContext context) {
return new ProfileTxEvent(context.isSuccess(), context);
}

}
Loading