Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
4 changes: 2 additions & 2 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ jobs:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up JDK 21
- name: Set up JDK 17
uses: actions/setup-java@v4
with:
java-version: '21'
java-version: '17'
distribution: 'temurin'
cache: 'gradle'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,7 @@

@Repository
public interface BlogRepository extends JpaRepository<BlogJpaEntity, Long> {
Optional<BlogJpaEntity> findByIdAndUserId(Long id, Long userId);

@Modifying
@Transactional
@Query("UPDATE BlogJpaEntity b SET b.comment = b.comment + 1 WHERE b.id = :blogId")
void incrementComment(Long blogId);

@Modifying
@Transactional
@Query("UPDATE BlogJpaEntity b SET b.comment = b.comment - 1 WHERE b.id = :blogId")
void decrementComment(Long blogId);
Optional<BlogJpaEntity> findByIdAndUser_Id(Long id, Long userId);

@Query("""
select b from BlogJpaEntity b
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ public BlogCommentResponse createComment(Long userId, BlogCommentRequest request
.content(request.content())
.createdAt(Instant.now())
.build();
blogCommentRepository.save(comment);
blog.increaseComment();
blogCommentRepository.save(comment);
return new BlogCommentResponse(
comment.getId(),
comment.getBlog().getId(),
Expand All @@ -60,8 +60,8 @@ public void deleteComment(Long userId, Long blogId, Long commentId) {
if (!comment.getUser().getId().equals(userId)) {
throw new ExpectedException(HttpStatus.FORBIDDEN, "댓글을 삭제할 권한이 없습니다.");
}
blogCommentRepository.delete(comment);
blog.decreaseComment();
blogCommentRepository.delete(comment);
}

public BlogCommentListResponse getComments(Long blogId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class BlogQueryService {
private final BlogRepository blogRepository;

public BlogResponse getForEdit(Long userId, Long blogId){
BlogJpaEntity blog = blogRepository.findByIdAndUserId(blogId, userId)
BlogJpaEntity blog = blogRepository.findByIdAndUser_Id(blogId, userId)
.orElseThrow(() -> new ExpectedException(HttpStatus.NOT_FOUND, "존재하지 않는 블로그입니다."));
return new BlogResponse(
blog.getId(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public BlogResponse write(Long userId, BlogRequest request) {


public BlogResponse edit(Long userId, Long blogId, BlogRequest request) {
BlogJpaEntity blog = blogRepository.findByIdAndUserId(blogId, userId)
BlogJpaEntity blog = blogRepository.findByIdAndUser_Id(blogId, userId)
.orElseThrow(() -> new ExpectedException(HttpStatus.NOT_FOUND, "존재하지 않는 블로그입니다."));

blog.update(request.title(), request.content(), request.introduce(), request.thumbnail());
Expand All @@ -94,17 +94,18 @@ public BlogResponse edit(Long userId, Long blogId, BlogRequest request) {
@Transactional
public void delete(Long userId, Long blogId) {
List<BlogImageJpaEntity> images =
blogImageRepository.findByBlogId(blogId);
blogImageRepository.findByBlog_Id(blogId);

for (BlogImageJpaEntity img : images) {
imageService.deleteImage(img.getImageUrl());
}

BlogJpaEntity blog = blogRepository.findByIdAndUserId(blogId, userId)
BlogJpaEntity blog = blogRepository.findByIdAndUser_Id(blogId, userId)
.orElseThrow(() -> new ExpectedException(HttpStatus.NOT_FOUND, "존재하지 않는 블로그입니다."));
if (blog.getThumbnail() != null && !blog.getThumbnail().isBlank()) {
imageService.deleteImage(blog.getThumbnail());
}

for (BlogImageJpaEntity img : images) {
imageService.deleteImage(img.getImageUrl());
}

blogRepository.delete(blog);
}
Comment on lines 95 to 110

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

delete 메서드는 @Transactional로 관리되고 있지만, 데이터베이스 트랜잭션 내에서 외부 시스템(Minio)의 파일을 삭제하는 로직이 있어 데이터 불일치 위험이 있습니다.

imageService.deleteImage() 호출이 반복문 중간에 실패할 경우,

  • 데이터베이스 트랜잭션은 롤백되어 블로그와 이미지 정보는 DB에서 삭제되지 않습니다.
  • 하지만, 예외 발생 이전에 imageService.deleteImage()로 성공적으로 삭제된 파일들은 외부 저장소에서 영구적으로 사라지며 복구되지 않습니다.

이로 인해 DB에는 존재하지만 실제 파일은 없는 '깨진 데이터'가 발생할 수 있습니다.

개선 제안:
데이터베이스 작업과 외부 시스템 호출을 분리하는 것을 고려해 보세요. 예를 들어, 트랜잭션이 성공적으로 커밋된 후에만 파일을 삭제하도록 @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)를 사용하는 방법이 있습니다. 이렇게 하면 DB 작업의 원자성을 보장하면서 외부 시스템과의 상호작용으로 인한 부작용을 최소화할 수 있습니다.


Expand Down Expand Up @@ -190,7 +191,7 @@ public void deleteBlogImage(Long userId,Long imageId) {
public List<ImageResponse> getBlogImages(Long blogId) {

List<BlogImageJpaEntity> images =
blogImageRepository.findByBlogId(blogId);
blogImageRepository.findByBlog_Id(blogId);

return images.stream()
.map(img -> new ImageResponse(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@

@Repository
public interface BlogImageRepository extends JpaRepository<BlogImageJpaEntity, Long> {
List<BlogImageJpaEntity> findByBlogId(Long blogId);
List<BlogImageJpaEntity> findByBlog_Id(Long blogId);

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@

@Repository
public interface PortfolioImageRepository extends JpaRepository<PortfolioImageJpaEntity, Long> {
List<PortfolioImageJpaEntity> findByPortfolioId(Long portfolioId);
List<PortfolioImageJpaEntity> findByPortfolio_Id(Long portfolioId);

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ public class ImageService {

private final MinioClient minioClient;

@Value("${minio.bucket}")
@Value("${spring.minio.bucket}")
private String bucket;

@Value("${minio.endpoint}")
@Value("${spring.minio.endpoint}")
private String endpoint;

@Value("${minio.presigned-url-duration-minutes}")
@Value("${spring.minio.presigned-url-duration-minutes}")
private long presignedUrlDuration;

private static final String THUMBNAIL_DIRECTORY = "thumbnail/";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,7 @@

@Repository
public interface PortfolioRepository extends JpaRepository<PortfolioJpaEntity, Long> {
Optional<PortfolioJpaEntity> findByIdAndUserId(Long id, Long userId);

@Modifying
@Transactional
@Query("UPDATE PortfolioJpaEntity p SET p.comment = p.comment + 1 WHERE p.id = :portfolioId")
void incrementComment(Long portfolioId);

@Modifying
@Transactional
@Query("UPDATE PortfolioJpaEntity p SET p.comment = p.comment - 1 WHERE p.id = :portfolioId")
void decrementComment(Long portfolioId);
Optional<PortfolioJpaEntity> findByIdAndUser_Id(Long id, Long userId);

@Query("""
select p from PortfolioJpaEntity p
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ public PortfolioCommentResponse createComment(Long userId, PortfolioCommentReque
.content(request.content())
.createdAt(Instant.now())
.build();
portfolioCommentRepository.save(comment);
portfolio.increaseComment();
portfolioCommentRepository.save(comment);
return new PortfolioCommentResponse(
comment.getId(),
comment.getPortfolio().getId(),
Expand All @@ -63,9 +63,8 @@ public void deleteComment(Long userId, Long portfolioId, Long commentId) {
if (!comment.getUser().getId().equals(userId)) {
throw new ExpectedException(HttpStatus.FORBIDDEN, "댓글을 삭제할 권한이 없습니다.");
}
portfolioCommentRepository.delete(comment);
portfolio.decreaseComment();

portfolioCommentRepository.delete(comment);
}

public PortfolioCommentListResponse getComments(Long portfolioId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class PortfolioQueryService {
private final PortfolioRepository portfolioRepository;

public PortfolioResponse getForEdit(Long userId, Long portfolioId){
PortfolioJpaEntity portfolio = portfolioRepository.findByIdAndUserId(portfolioId, userId)
PortfolioJpaEntity portfolio = portfolioRepository.findByIdAndUser_Id(portfolioId, userId)
.orElseThrow(() -> new ExpectedException(HttpStatus.NOT_FOUND, "존재하지 않는 포트폴리오입니다."));
return new PortfolioResponse(
portfolio.getId(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public PortfolioResponse write(Long userId, PortfolioRequest request) {


public PortfolioResponse edit(Long userId, Long portfolioId, PortfolioRequest request) {
PortfolioJpaEntity portfolio = portfolioRepository.findByIdAndUserId(portfolioId, userId)
PortfolioJpaEntity portfolio = portfolioRepository.findByIdAndUser_Id(portfolioId, userId)
.orElseThrow(() -> new ExpectedException(HttpStatus.NOT_FOUND, "존재하지 않는 포트폴리오입니다."));

portfolio.update(request.title(),request.content(),request.introduce());
Expand All @@ -92,12 +92,12 @@ public PortfolioResponse edit(Long userId, Long portfolioId, PortfolioRequest re

public void delete(Long userId, Long portfolioId) {
List<PortfolioImageJpaEntity> images =
portfolioImageRepository.findByPortfolioId(portfolioId);
portfolioImageRepository.findByPortfolio_Id(portfolioId);

for (PortfolioImageJpaEntity img : images) {
imageService.deleteImage(img.getImageUrl());
}
PortfolioJpaEntity portfolio = portfolioRepository.findByIdAndUserId(portfolioId, userId)
PortfolioJpaEntity portfolio = portfolioRepository.findByIdAndUser_Id(portfolioId, userId)
.orElseThrow(() -> new ExpectedException(HttpStatus.NOT_FOUND, "존재하지 않는 포트폴리오입니다."));

portfolioRepository.delete(portfolio);
Comment on lines 93 to 103

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

delete 메서드에는 몇 가지 심각한 문제가 있습니다.

  1. 트랜잭션 부재: 메서드에 @Transactional 어노테이션이 없어, 이미지 삭제와 포트폴리오 삭제 작업이 원자적으로 처리되지 않습니다. 중간에 오류가 발생하면 데이터가 불일치 상태에 빠질 수 있습니다.
  2. 잘못된 작업 순서 및 권한 검사: 포트폴리오 삭제 권한을 확인하기 전에 외부 저장소의 이미지를 먼저 삭제하고 있습니다. 만약 사용자가 권한이 없거나 포트폴리오가 존재하지 않아 findByIdAndUser_Id에서 예외가 발생하면, 이미지는 삭제되었지만 포트폴리오는 그대로 남아있는 심각한 데이터 불일치가 발생합니다.

아래와 같이 수정하여 문제를 해결하는 것을 강력히 권장합니다.

  1. @Transactional 어노테이션을 추가하여 모든 DB 작업을 하나의 트랜잭션으로 묶습니다.
  2. 가장 먼저 포트폴리오 존재 여부와 사용자 권한을 확인합니다.
Suggested change
public void delete(Long userId, Long portfolioId) {
List<PortfolioImageJpaEntity> images =
portfolioImageRepository.findByPortfolioId(portfolioId);
portfolioImageRepository.findByPortfolio_Id(portfolioId);
for (PortfolioImageJpaEntity img : images) {
imageService.deleteImage(img.getImageUrl());
}
PortfolioJpaEntity portfolio = portfolioRepository.findByIdAndUserId(portfolioId, userId)
PortfolioJpaEntity portfolio = portfolioRepository.findByIdAndUser_Id(portfolioId, userId)
.orElseThrow(() -> new ExpectedException(HttpStatus.NOT_FOUND, "존재하지 않는 포트폴리오입니다."));
portfolioRepository.delete(portfolio);
@Transactional
public void delete(Long userId, Long portfolioId) {
PortfolioJpaEntity portfolio = portfolioRepository.findByIdAndUser_Id(portfolioId, userId)
.orElseThrow(() -> new ExpectedException(HttpStatus.NOT_FOUND, "존재하지 않는 포트폴리오입니다."));
List<PortfolioImageJpaEntity> images =
portfolioImageRepository.findByPortfolio_Id(portfolioId);
for (PortfolioImageJpaEntity img : images) {
imageService.deleteImage(img.getImageUrl());
}
portfolioRepository.delete(portfolio);
}

Expand Down Expand Up @@ -209,7 +209,7 @@ public void deletePortfolioImage(Long userId, Long imageId) {
public List<ImageResponse> getPortfolioImages(Long portfolioId) {

List<PortfolioImageJpaEntity> images =
portfolioImageRepository.findByPortfolioId(portfolioId);
portfolioImageRepository.findByPortfolio_Id(portfolioId);

return images.stream()
.map(img -> new ImageResponse(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public TokenResDto execute(String email) {
.orElseGet(() -> userRepository.save(
UserJpaEntity.builder()
.email(email)
.role("VERIFIED")
.role("UNVERIFIED")
.build()
));
return new TokenResDto(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void execute(
null
), 10L
);
String verifyUrl = url + "/api/v1/auth/verify?token=" + token;
String verifyUrl = url + "?token=" + token;
try {
MimeMessage mimeMessage = mailSender.createMimeMessage();
MimeMessageHelper helper = new MimeMessageHelper(mimeMessage, true, "UTF-8");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ public class MinioConfig {

@Bean
public MinioClient minioClient(
@Value("${minio.endpoint}") String endpoint,
@Value("${minio.access-key}") String accessKey,
@Value("${minio.secret-key}") String secretKey
@Value("${spring.minio.endpoint}") String endpoint,
@Value("${spring.minio.access-key}") String accessKey,
@Value("${spring.minio.secret-key}") String secretKey
) {
return MinioClient.builder()
.endpoint(endpoint)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,31 @@ protected void doFilterInternal(HttpServletRequest request,
}
filterChain.doFilter(request, response);
}

@Override
protected boolean shouldNotFilter(HttpServletRequest request) {
String uri = request.getRequestURI();

if (uri.startsWith("/api/v1/auth/sign")
|| uri.startsWith("/api/v1/auth/login")
|| uri.startsWith("/api/v1/auth/verify")
|| uri.startsWith("/api/v1/auth/google")
|| uri.startsWith("/api/v1/auth/kakao")) {
return true;
} else if (request.getMethod().equals("GET") && (
uri.startsWith("/api/v1/main/")
|| uri.startsWith("/api/v1/profile/")
|| uri.startsWith("/api/v1/portfolio/view/")
|| uri.startsWith("/api/v1/portfolio/comment/")
|| uri.startsWith("/api/v1/blog/view/")
|| uri.startsWith("/api/v1/blog/comment/"))) {
return true;
}

if(request.getMethod().equals("OPTIONS")) {
return true;
}

return false;
}
Comment on lines +81 to +105

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

새로 추가된 shouldNotFilter 메서드는 JWT 필터를 건너뛸 경로를 지정하여 최적화에 도움이 됩니다. 하지만 현재 구현에는 유지보수성 측면에서 몇 가지 개선점이 있습니다.

  1. 하드코딩된 URL 패턴: URL 경로들이 문자열로 하드코딩되어 있어, 향후 경로 변경 시 여러 곳을 수정해야 할 수 있고 오류 발생 가능성이 높습니다.
  2. startsWith 사용의 위험성: startsWithAntPathMatcher (예: /**)에 비해 정교하지 않습니다. 예를 들어, uri.startsWith("/api/v1/auth/sign")는 의도치 않게 /api/v1/auth/signup과 같은 다른 경로와도 일치할 수 있습니다.
  3. SecurityConfig와의 중복: 필터링 예외 경로는 SecurityConfig에 정의된 permitAll 경로와 거의 동일합니다. 두 곳에서 경로를 관리하면 동기화가 누락될 위험이 있습니다.

개선 제안:

  • 예외 경로 목록을 별도의 static final 리스트나 배열로 추출하여 한 곳에서 관리하고, SecurityConfigJwtHeaderFilter에서 함께 사용하도록 리팩토링하는 것을 권장합니다.
  • startsWith 대신 Spring에서 널리 사용되는 AntPathMatcher를 사용하여 SecurityConfig와 동일한 방식으로 경로를 검사하면 일관성과 안정성을 높일 수 있습니다.

}
1 change: 1 addition & 0 deletions src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ spring:
access-key: ${MINIO_ACCESS_KEY}
secret-key: ${MINIO_SECRET_KEY}
bucket: story-images
presigned-url-duration-minutes: 10
task:
execution:
pool:
Expand Down