Feat/#23 학생회 공지 게시글 생성/조회/수정/삭제 기능 및 72시간 이내 행사 조회 기능 구현 #24
Conversation
Walkthrough학생회 공지(Notice) 도메인(엔티티·리포지토리·매퍼·서비스·컨트롤러·예외·응답코드) 신규 추가 및 게시글(Post) 도메인 DTO/매퍼/서비스/컨트롤러 명칭·시그니처 리네이밍, 이벤트 조회·OCI 이미지 삭제 관련 예외·코드 변경이 반영되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Controller as StudentCouncilNoticeController
participant Service as StudentCouncilNoticeService
participant Mapper as StudentCouncilNoticeMapper
participant RepoNotice as StudentCouncilNoticeRepository
participant RepoImage as NoticeImageRepository
participant OCI as PresignedUrlService/OCI
participant DB as Database
User->>Controller: POST /student-council/notices (NoticeRequest)
Controller->>Service: create(councilId, dto)
rect rgb(220,239,255)
Service->>Mapper: createStudentCouncilNotice(writer, dto)
Mapper-->>Service: StudentCouncilNotice
Service->>RepoNotice: save(notice)
RepoNotice->>DB: INSERT notice
DB-->>RepoNotice: persisted
end
rect rgb(230,255,238)
alt images present
loop each imageUrl
Service->>Mapper: createStudentCouncilNoticeImage(notice, imageUrl)
Mapper-->>Service: NoticeImage
Service->>RepoImage: save(image)
RepoImage->>DB: INSERT image
end
end
end
rect rgb(255,245,240)
Service->>OCI: cleanupUnusedImages(oldImages - newImages)
OCI-->>Service: success / error (logged)
end
Service->>Mapper: toNoticeResponse(notice, imageUrls, councilId)
Mapper-->>Service: NoticeResponse
Service-->>Controller: NoticeResponse
Controller-->>User: CommonResponse<NoticeResponse>
(색상은 변경/신규 흐름 강조용이며 접근성 고려하여 최소 사용) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java (1)
16-29: 작성자 확인 로직의 일관성 문제와 NPE 위험두 가지 문제가 있습니다:
NPE 위험 (Critical): Line 27에서
currentUserId가 null일 수 있는데equals()호출 시 NPE가 발생합니다.StudentCouncilPostController의 Line 136과 148에서@CurrentCouncilId(required = false)로 선언되어 null이 전달될 수 있습니다.일관성 부족: Line 27(
toListItem)은 직접 비교를 사용하고, Line 37(toDetail)은isWrittenByCouncil()메서드를 사용합니다. 동일한 방식으로 통일하는 것이 좋습니다.🔎 제안하는 수정안
public static PostListItemResponseDto toListItem(StudentCouncilPost post, Long currentUserId) { return new PostListItemResponseDto( post.getId(), post.getCategory(), post.getTitle(), post.getPlace(), post.isEvent() ? post.getStartDateTime() : post.getEndDateTime(), post.getThumbnailImageUrl(), post.getThumbnailIcon(), - post.getWriter().getId().equals(currentUserId) + currentUserId != null && post.isWrittenByCouncil(currentUserId) ); }Also applies to: 31-54
🧹 Nitpick comments (8)
src/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostController.java (1)
143-153: 72시간 이내 행사 조회 엔드포인트 구현 확인새로운 엔드포인트가 적절하게 구현되었습니다. 서비스 계층에 로직을 위임하고 표준 응답 형식을 사용하고 있습니다.
참고: 72시간 기준을 상수나 설정값으로 추출하면 향후 요구사항 변경 시 유연하게 대응할 수 있습니다.
src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java (1)
109-135: 72시간 이내 행사 조회 로직 구현 확인전체적인 로직 흐름이 적절합니다:
- 현재 시각부터 72시간 이내의 행사를 조회
startDateTime오름차순 정렬로 가장 가까운 행사가 먼저 표시됨- 페이징 처리가 올바르게 구현됨
시간 상수를 추출하면 가독성과 유지보수성이 향상됩니다:
private static final int UPCOMING_EVENT_HOURS = 72; // Line 122 LocalDateTime limit = now.plusHours(UPCOMING_EVENT_HOURS);src/main/java/com/campus/campus/domain/councilpost/domain/repository/StudentCouncilPostRepository.java (1)
27-38: 72시간 이내 행사 조회 쿼리 구현 확인 및 성능 최적화 제안쿼리 로직이 올바르게 구현되었습니다:
category와startDateTime범위로 필터링BETWEEN연산자로 시작/종료 시각이 모두 포함됨- 페이징 처리 지원
🔎 N+1 문제 방지를 위한 JOIN FETCH 추가 제안
StudentCouncilPostMapper.toListItem에서post.getWriter().getId()를 호출하므로, writer를 함께 조회하지 않으면 N+1 쿼리 문제가 발생합니다:@Query(""" - SELECT p + SELECT p FROM StudentCouncilPost p + JOIN FETCH p.writer WHERE p.category = :category AND p.startDateTime BETWEEN :now AND :limit """) Page<StudentCouncilPost> findUpcomingEvents( @Param("category") PostCategory category, @Param("now") LocalDateTime now, @Param("limit") LocalDateTime limit, Pageable pageable );이렇게 하면 페이지당 한 번의 쿼리로 모든 writer 정보를 가져올 수 있습니다.
src/main/java/com/campus/campus/domain/councilNotice/domain/repository/NoticeImageRepository.java (1)
12-14: 정렬 여부가 다른 중복 메서드 고려
findAllByNotice와findAllByNoticeOrderByIdAsc가 모두 존재합니다. 기존PostImageRepository에도 동일한 패턴이 있지만, 대부분의 사용처에서 정렬된 결과가 필요하다면findAllByNoticeOrderByIdAsc하나만 유지하는 것이 더 간결할 수 있습니다.src/main/java/com/campus/campus/domain/councilNotice/application/dto/request/NoticeRequestDto.java (1)
5-9: 입력 검증 어노테이션 추가 권장DTO에 validation 어노테이션이 없어 잘못된 입력이 서비스 레이어까지 전달될 수 있습니다. 다음과 같은 검증을 추가하는 것을 권장합니다:
title:@NotBlank,@Size(max=100)(StudentCouncilNotice 엔티티의 length 제약과 일치)content:@NotBlankimageUrls:@NotNull,@Size(max=10)(ErrorCode의 이미지 제한이 10개인 것으로 보임)🔎 제안하는 수정안
package com.campus.campus.domain.councilNotice.application.dto.request; import java.util.List; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; public record NoticeRequestDto( + @NotBlank(message = "제목은 필수입니다") + @Size(max = 100, message = "제목은 최대 100자까지 입력 가능합니다") String title, + @NotBlank(message = "내용은 필수입니다") String content, + @NotNull(message = "이미지 URL 목록은 null일 수 없습니다") + @Size(max = 10, message = "이미지는 최대 10개까지 등록 가능합니다") List<String> imageUrls ) { }src/main/java/com/campus/campus/domain/councilNotice/domain/entity/StudentCouncilNotice.java (1)
43-46:update메서드에서 null 값 검증 누락
title과content필드는@Column(nullable = false)로 정의되어 있지만,update메서드에서는 null 값을 허용합니다. 이로 인해 데이터베이스 제약 조건 위반 예외가 발생할 수 있습니다.🔎 제안된 수정
public void update(String title, String content) { + if (title == null || title.isBlank()) { + throw new IllegalArgumentException("Title cannot be null or blank"); + } + if (content == null || content.isBlank()) { + throw new IllegalArgumentException("Content cannot be null or blank"); + } this.title = title; this.content = content; }src/main/java/com/campus/campus/domain/councilNotice/application/mapper/StudentCouncilNoticeMapper.java (1)
12-51: 정적 메서드 대신 Spring 컴포넌트 고려현재 정적 메서드로 구현되어 있습니다. 기능적으로 문제는 없지만, Spring의 의존성 주입 패턴과 일관성을 위해
@Component로 등록하는 것을 고려해볼 수 있습니다. 이렇게 하면 향후 다른 서비스 의존성이 필요할 때 확장이 용이합니다.src/main/java/com/campus/campus/domain/councilNotice/application/service/StudentCouncilNoticeService.java (1)
64-68: 불필요한 데이터베이스 조회이미지를 저장한 직후에 다시 조회하고 있습니다.
dto.imageUrls()를 직접 사용하면 불필요한 쿼리를 줄일 수 있습니다.🔎 제안된 수정
- List<String> imageUrls = noticeImageRepository - .findAllByNoticeOrderByIdAsc(notice) - .stream() - .map(NoticeImage::getImageUrl) - .toList(); + List<String> imageUrls = dto.imageUrls() != null ? dto.imageUrls() : List.of();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/main/java/com/campus/campus/domain/councilNotice/application/dto/request/NoticeRequestDto.javasrc/main/java/com/campus/campus/domain/councilNotice/application/dto/response/NoticeListItemResponseDto.javasrc/main/java/com/campus/campus/domain/councilNotice/application/dto/response/NoticeResponseDto.javasrc/main/java/com/campus/campus/domain/councilNotice/application/exception/ErrorCode.javasrc/main/java/com/campus/campus/domain/councilNotice/application/exception/NotNoticeWriterException.javasrc/main/java/com/campus/campus/domain/councilNotice/application/exception/NoticeImageLimitExceededException.javasrc/main/java/com/campus/campus/domain/councilNotice/application/exception/NoticeNotFoundException.javasrc/main/java/com/campus/campus/domain/councilNotice/application/mapper/StudentCouncilNoticeMapper.javasrc/main/java/com/campus/campus/domain/councilNotice/application/service/StudentCouncilNoticeService.javasrc/main/java/com/campus/campus/domain/councilNotice/domain/entity/NoticeImage.javasrc/main/java/com/campus/campus/domain/councilNotice/domain/entity/StudentCouncilNotice.javasrc/main/java/com/campus/campus/domain/councilNotice/domain/repository/NoticeImageRepository.javasrc/main/java/com/campus/campus/domain/councilNotice/domain/repository/StudentCouncilNoticeRepository.javasrc/main/java/com/campus/campus/domain/councilNotice/presentation/NoticeResponseCode.javasrc/main/java/com/campus/campus/domain/councilNotice/presentation/StudentCouncilNoticeController.javasrc/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.javasrc/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.javasrc/main/java/com/campus/campus/domain/councilpost/domain/entity/StudentCouncilPost.javasrc/main/java/com/campus/campus/domain/councilpost/domain/repository/StudentCouncilPostRepository.javasrc/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostController.javasrc/main/java/com/campus/campus/global/oci/presentation/PresignedUrlController.java
🧰 Additional context used
🧬 Code graph analysis (6)
src/main/java/com/campus/campus/domain/councilNotice/domain/repository/NoticeImageRepository.java (2)
src/main/java/com/campus/campus/domain/councilpost/domain/repository/PostImageRepository.java (4)
PostImageRepository(10-17)deleteByPost(14-14)findAllByPostOrderByIdAsc(16-16)findAllByPost(12-12)src/main/java/com/campus/campus/domain/councilpost/domain/entity/PostImage.java (1)
Entity(18-36)
src/main/java/com/campus/campus/domain/councilNotice/domain/entity/StudentCouncilNotice.java (1)
src/main/java/com/campus/campus/domain/councilNotice/domain/entity/NoticeImage.java (1)
Entity(17-34)
src/main/java/com/campus/campus/domain/councilNotice/domain/entity/NoticeImage.java (1)
src/main/java/com/campus/campus/domain/councilNotice/domain/entity/StudentCouncilNotice.java (1)
Entity(21-51)
src/main/java/com/campus/campus/domain/councilNotice/presentation/StudentCouncilNoticeController.java (2)
src/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostController.java (1)
RestController(32-176)src/main/java/com/campus/campus/global/oci/presentation/PresignedUrlController.java (1)
RestController(18-61)
src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java (1)
src/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java (1)
StudentCouncilPostMapper(14-77)
src/main/java/com/campus/campus/domain/councilNotice/application/service/StudentCouncilNoticeService.java (5)
src/main/java/com/campus/campus/domain/council/application/exception/StudentCouncilNotFoundException.java (1)
StudentCouncilNotFoundException(5-9)src/main/java/com/campus/campus/domain/councilNotice/application/exception/NotNoticeWriterException.java (1)
NotNoticeWriterException(5-9)src/main/java/com/campus/campus/domain/councilNotice/application/exception/NoticeImageLimitExceededException.java (1)
NoticeImageLimitExceededException(5-9)src/main/java/com/campus/campus/domain/councilNotice/application/exception/NoticeNotFoundException.java (1)
NoticeNotFoundException(5-9)src/main/java/com/campus/campus/domain/councilNotice/application/mapper/StudentCouncilNoticeMapper.java (1)
StudentCouncilNoticeMapper(12-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (20)
src/main/java/com/campus/campus/domain/councilpost/domain/entity/StudentCouncilPost.java (1)
94-96: 메서드명 변경이 적절합니다.
isWrittenBy에서isWrittenByCouncil로 변경하여 학생회 게시글의 작성자 검증 의도가 더 명확해졌습니다.src/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java (1)
22-24: 조건부 날짜 로직의 의도를 명확히 해주세요.행사(EVENT)인 경우
startDateTime을 반환하고, 그렇지 않은 경우endDateTime을 반환하고 있습니다.PostListItemResponseDto의 해당 필드가 어떤 의미를 나타내는지 명확하지 않아 API 사용자에게 혼란을 줄 수 있습니다.행사의 경우 시작 시각을 표시하려는 의도라면, DTO 필드명이나 문서화를 통해 이를 명확히 하는 것이 좋습니다.
src/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostController.java (1)
122-141: 선택적 councilId 파라미터 변경 승인
@CurrentCouncilId(required = false)로 변경하여 학생회 회원이 아닌 사용자도 게시글 목록을 조회할 수 있도록 한 것은 적절한 접근성 개선입니다.src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java (1)
138-144: 삭제 시 사용하는 조회 메서드 변경 확인
findByIdWithWriter에서findByIdWithFullInfo로 변경되었습니다. 이는 삭제 작업에 필요한 것보다 더 많은 데이터(학교, 단과대학, 학과 정보)를 조회하지만, 코드베이스 전체에서 일관된 조회 메서드를 사용하여 유지보수성을 높입니다.참고: 삭제 작업에는 writer 정보만 필요하므로 약간의 성능 오버헤드가 있지만, 일관성의 이점이 더 큽니다.
src/main/java/com/campus/campus/domain/councilNotice/domain/repository/NoticeImageRepository.java (1)
10-17: LGTM! 일관된 리포지토리 패턴기존
PostImageRepository와 동일한 구조를 따르고 있어 코드베이스 전체의 일관성이 유지됩니다. 메서드 네이밍도 Spring Data JPA 규칙을 잘 따르고 있습니다.src/main/java/com/campus/campus/global/oci/presentation/PresignedUrlController.java (1)
49-60: LGTM! 일관된 엔드포인트 패턴기존 게시글 이미지 엔드포인트와 동일한 구조를 따르고 있어 일관성이 유지됩니다. Swagger 문서화도 적절하게 추가되었습니다.
src/main/java/com/campus/campus/domain/councilNotice/domain/entity/NoticeImage.java (1)
17-34: LGTM! 표준 JPA 엔티티 패턴기존
PostImage엔티티와 동일한 구조를 따르고 있으며, JPA 베스트 프랙티스를 잘 준수하고 있습니다:
- LAZY 페치 전략 사용으로 성능 최적화
optional=false로 필수 관계 명시- 적절한 Lombok 어노테이션 활용
src/main/java/com/campus/campus/domain/councilNotice/application/exception/NoticeNotFoundException.java (1)
5-9: LGTM! 표준 예외 처리 패턴프로젝트의 예외 처리 패턴을 잘 따르고 있습니다.
src/main/java/com/campus/campus/domain/councilNotice/application/dto/response/NoticeResponseDto.java (1)
8-20: LGTM! 잘 설계된 응답 DTO상세 조회에 필요한 모든 정보를 포함하고 있으며,
isWriter필드를 통해 권한 확인도 가능합니다.@Builder와 record의 조합도 적절합니다.src/main/java/com/campus/campus/domain/councilNotice/application/exception/NoticeImageLimitExceededException.java (1)
5-9: LGTM! 명확한 예외 클래스이미지 개수 제한을 위한 명확한 예외 클래스이며, 프로젝트의 예외 처리 패턴을 잘 따르고 있습니다.
src/main/java/com/campus/campus/domain/councilNotice/application/dto/response/NoticeListItemResponseDto.java (1)
7-15: LGTM! 목록 조회에 최적화된 DTO목록 조회에 필요한 최소한의 정보만 포함하여 적절하게 설계되었습니다. 페이지네이션을 고려한 간결한 구조입니다.
src/main/java/com/campus/campus/domain/councilNotice/application/exception/NotNoticeWriterException.java (1)
1-9: LGTM!예외 클래스가 기존 코드베이스의 패턴(NoticeNotFoundException 등)과 일관되게 구현되어 있습니다.
ApplicationException을 확장하고 적절한ErrorCode를 사용합니다.src/main/java/com/campus/campus/domain/councilNotice/domain/entity/StudentCouncilNotice.java (1)
48-50: LGTM!
isWrittenByCouncil메서드가 방어적으로 null 체크를 수행하고 있어 안전합니다.optional=false로 설정되어 있지만, 추가적인 null 체크는 좋은 방어적 프로그래밍 패턴입니다.src/main/java/com/campus/campus/domain/councilNotice/domain/repository/StudentCouncilNoticeRepository.java (1)
11-20: LGTM!
findByIdWithFullInfo쿼리가 N+1 문제를 방지하기 위해 적절하게JOIN FETCH를 사용하고 있습니다. 필수 관계(writer)에는JOIN FETCH, 선택적 관계(school,college,major)에는LEFT JOIN FETCH를 올바르게 적용했습니다.src/main/java/com/campus/campus/domain/councilNotice/application/exception/ErrorCode.java (1)
10-21: LGTM!에러 코드가 적절한 HTTP 상태 코드와 함께 잘 정의되어 있습니다. 코드 번호가 순차적이고, 각 에러에 대한 의미가 명확합니다.
src/main/java/com/campus/campus/domain/councilNotice/presentation/NoticeResponseCode.java (1)
10-23: LGTM!응답 코드가 CRUD 작업에 맞게 잘 정의되어 있습니다. 생성(201 CREATED), 조회/수정(200 OK), 삭제(204 NO_CONTENT) 등 RESTful 규칙을 따르고 있습니다.
src/main/java/com/campus/campus/domain/councilNotice/application/service/StudentCouncilNoticeService.java (2)
73-86: LGTM!단건 조회 로직이 깔끔하게 구현되어 있습니다.
findByIdWithFullInfo를 사용하여 N+1 문제를 방지하고 있습니다.
149-171: LGTM!삭제 로직이 적절하게 구현되어 있습니다. DB 삭제를 먼저 수행하고 OCI 파일 삭제를 best-effort로 처리하는 것은 올바른 패턴입니다. OCI 삭제 실패 시 로그를 남기되 트랜잭션은 성공시키는 것이 합리적입니다.
src/main/java/com/campus/campus/domain/councilNotice/presentation/StudentCouncilNoticeController.java (2)
46-67: LGTM!조회 엔드포인트들이 잘 구현되어 있습니다.
@CurrentCouncilId(required = false)를 사용하여 비로그인 사용자도 조회할 수 있도록 적절히 처리했습니다.
82-92: LGTM!삭제 엔드포인트가 올바르게 구현되어 있습니다.
@PreAuthorize로 권한 검증을 수행하고, 적절한 응답 코드를 반환합니다.
.../com/campus/campus/domain/councilnotice/application/service/StudentCouncilNoticeService.java
Show resolved
Hide resolved
| List<NoticeImage> oldImages = noticeImageRepository.findAllByNotice(notice); | ||
|
|
||
| notice.update(dto.title(), dto.content()); | ||
|
|
||
| noticeImageRepository.deleteByNotice(notice); | ||
|
|
||
| if (dto.imageUrls() != null && !dto.imageUrls().isEmpty()) { | ||
| List<NoticeImage> images = dto.imageUrls().stream() | ||
| .map(imageUrl -> StudentCouncilNoticeMapper.toEntity(notice, imageUrl)) | ||
| .toList(); | ||
|
|
||
| noticeImageRepository.saveAll(images); | ||
| } | ||
|
|
||
| for (NoticeImage image : oldImages) { | ||
| try { | ||
| presignedUrlService.deleteImage(image.getImageUrl()); | ||
| } catch (Exception e) { | ||
| log.warn("OCI 파일 삭제 실패 (파일이 없을 수 있음): {}", image.getImageUrl()); | ||
| } | ||
| } |
There was a problem hiding this comment.
기존 이미지와 새 이미지 URL이 동일할 경우 문제 발생 가능
업데이트 로직에서 기존 이미지를 모두 삭제하고 새 이미지를 저장한 후, OCI에서 기존 이미지 파일을 삭제합니다. 만약 사용자가 일부 이미지를 유지하려고 동일한 URL을 다시 전달하면, 해당 파일이 OCI에서 삭제되어 새 이미지 레코드가 깨진 링크를 참조하게 됩니다.
🔎 제안된 해결 방법
새 이미지 목록과 비교하여 실제로 제거된 이미지만 OCI에서 삭제하세요:
+ Set<String> newImageUrls = dto.imageUrls() != null
+ ? new HashSet<>(dto.imageUrls())
+ : Set.of();
+
for (NoticeImage image : oldImages) {
+ if (newImageUrls.contains(image.getImageUrl())) {
+ continue; // 유지되는 이미지는 삭제하지 않음
+ }
try {
presignedUrlService.deleteImage(image.getImageUrl());
} catch (Exception e) {
log.warn("OCI 파일 삭제 실패 (파일이 없을 수 있음): {}", image.getImageUrl());
}
}🤖 Prompt for AI Agents
In
src/main/java/com/campus/campus/domain/councilNotice/application/service/StudentCouncilNoticeService.java
around lines 118-138, the update flow deletes all existing image records and
then always deletes every old image file from OCI which breaks links when a user
re-supplies the same image URL; change the logic to compute which image URLs
were actually removed (old image URLs minus new dto.imageUrls, handling
null/empty new list) and only delete those removed images from OCI and remove
only those records from the repository (or use a repository method to delete by
notice and imageUrl list) while keeping records and files for images that remain
unchanged. Ensure null-safe comparison, use URL string equality, and perform
repository saves for new images as before.
...java/com/campus/campus/domain/councilNotice/presentation/StudentCouncilNoticeController.java
Outdated
Show resolved
Hide resolved
...java/com/campus/campus/domain/councilNotice/presentation/StudentCouncilNoticeController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/campus/campus/domain/councilNotice/application/service/StudentCouncilNoticeService.java (1)
89-103: N+1 쿼리 문제가 여전히 해결되지 않았습니다이전 리뷰에서 지적된 N+1 쿼리 문제가 그대로 남아있습니다.
noticeRepository.findAll(pageable)은writer를 페치하지 않지만,toListItem메서드에서isWrittenByCouncil(councilId)를 호출할 때writer.getId()에 접근하여 페이지당 N개의 추가 쿼리가 발생합니다.이전 리뷰의 제안대로
writer를 함께 로드하는 커스텀 쿼리 메서드를 추가해주세요.
🧹 Nitpick comments (2)
src/main/java/com/campus/campus/domain/councilNotice/application/service/StudentCouncilNoticeService.java (2)
57-69: 생성 후 즉시 재조회하는 패턴 개선 고려이미지를 저장한 직후(62번 라인) 동일한 이미지를 다시 조회하고 있습니다(65-69번 라인). 이미 저장된
images리스트를 재사용하면 불필요한 쿼리를 제거할 수 있습니다.다만, DB에서 생성된 ID나 기본값이 필요한 경우 현재 패턴이 적절할 수 있습니다. 해당 값들이 필요하지 않다면 아래와 같이 개선할 수 있습니다.
🔎 제안된 개선 방법
if (dto.imageUrls() != null && !dto.imageUrls().isEmpty()) { List<NoticeImage> images = dto.imageUrls().stream() .map(imageUrl -> StudentCouncilNoticeMapper.toEntity(notice, imageUrl)) .toList(); noticeImageRepository.saveAll(images); + + List<String> imageUrls = images.stream() + .map(NoticeImage::getImageUrl) + .toList(); + return StudentCouncilNoticeMapper.toDetail(notice, imageUrls, councilId); } - List<String> imageUrls = noticeImageRepository - .findAllByNoticeOrderByIdAsc(notice) - .stream() - .map(NoticeImage::getImageUrl) - .toList(); - - return StudentCouncilNoticeMapper.toDetail(notice, imageUrls, councilId); + return StudentCouncilNoticeMapper.toDetail(notice, List.of(), councilId);
156-159: 이미지 URL 수집 로직 간소화 가능불필요하게 중간 리스트를 생성하고
forEach로 추가하는 대신,stream().toList()를 직접 사용하여 코드를 간소화할 수 있습니다.🔎 제안된 개선 방법
- List<String> deleteTargets = new ArrayList<>(); - images.stream() - .map(NoticeImage::getImageUrl) - .forEach(deleteTargets::add); + List<String> deleteTargets = images.stream() + .map(NoticeImage::getImageUrl) + .toList();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/campus/campus/domain/councilNotice/application/service/StudentCouncilNoticeService.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/campus/campus/domain/councilNotice/application/service/StudentCouncilNoticeService.java (6)
src/main/java/com/campus/campus/domain/council/application/exception/StudentCouncilNotFoundException.java (1)
StudentCouncilNotFoundException(5-9)src/main/java/com/campus/campus/domain/councilNotice/application/exception/NotNoticeWriterException.java (1)
NotNoticeWriterException(5-9)src/main/java/com/campus/campus/domain/councilNotice/application/exception/NoticeImageLimitExceededException.java (1)
NoticeImageLimitExceededException(5-9)src/main/java/com/campus/campus/domain/councilNotice/application/exception/NoticeNotFoundException.java (1)
NoticeNotFoundException(5-9)src/main/java/com/campus/campus/domain/councilNotice/application/mapper/StudentCouncilNoticeMapper.java (1)
StudentCouncilNoticeMapper(12-52)src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java (1)
Slf4j(36-253)
🔇 Additional comments (2)
src/main/java/com/campus/campus/domain/councilNotice/application/service/StudentCouncilNoticeService.java (2)
74-87: LGTM!단일 공지 조회 로직이 명확하고 올바르게 구현되었습니다.
findByIdWithFullInfo를 사용하여 필요한 연관 데이터를 함께 가져오고 있습니다.
105-142: 이미지 업데이트 로직 개선 확인이전 리뷰에서 지적된 이미지 삭제 로직 문제가 개선되었습니다. 새로 추가된
cleanupUnusedImages헬퍼 메서드(177-200번 라인)가 유지되는 이미지는 OCI에서 삭제하지 않도록 필터링하고 있어, 동일한 URL이 재전달될 때 발생하던 문제가 해결되었습니다.
1winhyun
left a comment
1winhyun
left a comment
There was a problem hiding this comment.
수고하셨습니다!! 구현된 api들에 필요한 정보들이 다 담겨있는 것 같아요!! (제 눈에는...)
리뷰 남긴 것들 한번 한번 부탁드려요!!
추가로 현재 councilNotice 라는 패키지 명으로 되어있던데 councilnotice로 패키지명 수정 부탁드려요!!
There was a problem hiding this comment.
mapper에 관한 메서드 명명 규칙에 대한 통일이 필요해보여요!!
저의 경우 엔티티를 새로 생성해 저장하는 경우 createStudentCouncilNotice, createCouncil 등 "create + 엔티티명"의 형태로 구현하며 응답 dto 경우 toNoticeResponseDto, toUserResponse등으로 "to + 응답dto 이름"의 형태로 이름을 만듭니다.
재영님의 경우 toEntity, toDetail 등으로 메서드명을 통일하고 계시는 느낌인데 이는 모두가 동일한 명명법을 사용하는 방법을 생각해야할 것 같아요!!
저의 경우 재영님과 같이 메서드명을 통일시키지 않은 이유는 추후 service 내 코드를 확인할 때 이름이 겹쳐 코드를 확인하고 이해하는데 혼동이 올 수 있다고 생각하여 각각 한번에 알아볼 수 있도록 하려 했기 때문입니다!!
재영님은 어떤 방식으로 메서드명을 짓는 것이 좋다고 생각하시나요??
There was a problem hiding this comment.
저는 예를 들어 mapper의 toEntitiy가 꼭 create 하는 데에만 쓰이는 것이 아니라 update에도 쓰니까 create~이렇게 하게 되면 create할 때만 쓰이는 것인가?로 생각할 수도 있고 좀 더 뭐할 때에 쓰이는 지 명확하다고 생각을 해서 평소에는 그렇게 쓴 것이었습니다.
이 부분은 원래 먼저 올린 사람대로 규칙을 따라가자고 했어서 제가 고쳤어야 했는데, 승현님꺼 보고 나중에 고치자 하다가 깜빡했네요! 승현님 메서드명으로 바꾸겠습니다.
| return NoticeResponseDto.builder() | ||
| .id(notice.getId()) | ||
| .writerId(notice.getWriter().getId()) | ||
| .writerName(notice.getWriter().getFullCouncilName()) | ||
| .isWriter(notice.isWrittenByCouncil(councilId)) | ||
| .title(notice.getTitle()) | ||
| .content(notice.getContent()) | ||
| .images(imageUrls) | ||
| .createdAt(notice.getCreatedAt()) | ||
| .updatedAt(notice.getUpdatedAt()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
이 부분의 경우 이전과는 다르게 시간설정과 같은 로직이 추가되지 않았는데도 불구하고 builder()패턴을 사용하신 이유가 있을까요?
There was a problem hiding this comment.
승현님 이 부분 관련해서 의견 여쭤보고 싶은 것이 있었습니다.
조회용 응답 DTO는 new로 만드는 것에 동의합니다! 그런데, 구현하면서, 필드가 많아서, 순서나 가독성이 떨어질 수 있을 것 같아 의견을 구하고자, new로 작성하기 보다는 builder로 우선 작성해두었습니다. 5개 이상의 파라미터의 경우엔 builder를 쓰는 것이 어떨까하여 의견을 묻고 싶습니다.
There was a problem hiding this comment.
그래도 될 것 같습니다!! 재영님 의견대로 해도 좋을 것 같아요!!
| return NoticeListItemResponseDto.builder() | ||
| .id(notice.getId()) | ||
| .title(notice.getTitle()) | ||
| .isWriter(notice.isWrittenByCouncil(currentUserId)) | ||
| .createdAt(notice.getCreatedAt()) | ||
| .updatedAt(notice.getUpdatedAt()) | ||
| .build(); |
| try { | ||
| presignedUrlService.deleteImage(imageUrl); | ||
| } catch (Exception e) { | ||
| log.warn("OCI 파일 삭제 실패 (파일이 없을 수 있음): {}", imageUrl); | ||
| } | ||
| } |
There was a problem hiding this comment.
try catch 문으로 묶어주셨는데 어떤 예외상황의 가능성으로 인해 try-catch 문으로 묶으신건지 궁금합니다!!
추가적으로 Exception e로 끝나는 것보다 조금 더 자세하게 예외처리가 이루어지면 좋을 것 같아요!!
There was a problem hiding this comment.
저 부분은 버킷 내의 이미지 오브젝트 삭제를 하는 것인데, 만약에 에러가 생겨서 버킷 내의 오브젝트까지 삭제가 안됐을 경우 혹은 이미 삭제됐는데 또 해당 url을 호출하는 등의 경우 예외적인 상황이 생길 것이라고 생각해서 try-catch를 사용하였습니다!
후자의 말씀엔 동의합니다 수정하겠습니다!
| try { | ||
| presignedUrlService.deleteImage(imageUrl); | ||
| } catch (Exception e) { | ||
| log.warn("OCI 파일 삭제 실패 (파일이 없을 수 있음): {}", imageUrl); | ||
| } |
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @AllArgsConstructor | ||
| @Builder | ||
| public class NoticeImage { |
There was a problem hiding this comment.
이 부분의 경우 따로 extends BaseEntity를 붙이지 않은 이유가 있을까요?
There was a problem hiding this comment.
아 이 부분은 수정하겠습니다!!
발견해주셔서 감사해요
| NOTICE_READ_SUCCESS(200, HttpStatus.OK, "공지 조회에 성공했습니다."), | ||
| NOTICE_LIST_READ_SUCCESS(200, HttpStatus.OK, "공지 목록 조회에 성공했습니다."), | ||
| NOTICE_UPDATE_SUCCESS(200, HttpStatus.OK, "공지 수정에 성공했습니다."), | ||
| NOTICE_DELETE_SUCCESS(204, HttpStatus.NO_CONTENT, "공지 삭제에 성공했습니다."); |
There was a problem hiding this comment.
이 부분의 경우 "요청은 성공적으로 처리했으나, 클라이언트에게 돌려줄 데이터는 없다" 라는 의미를 가지고 있는 것으로 알고 있어요!! 굳이 이렇게 하는 것보다 다른 상태 코드와 동일하게 HttpStatus.OK로 해도 좋다는 생각이 드는게 어떻게 생각하세요??
There was a problem hiding this comment.
네 저는 하드삭제의 경우엔 204라고 생각을 해서 무의식적으로 저렇게 사용을 하였는데
저희가 공통성공응답을 사용하니까 200이 맞네요! 수정하겠습니다!
There was a problem hiding this comment.
해당 코드에 경우 역할별로 나누어서 api들을 정렬해주는 것이 개발자들끼리 보기 편할 것 같아요!! 예를 들어 @PreAuthorize("hasRole('COUNCIL')") 해당 어노테이션이 있는 api들을 우선 정렬한 후 모두가 사용가능한 api들을 정렬하는 것은 어떨까요?
There was a problem hiding this comment.
저는 컨트롤러의 경우 정렬 방식이 crud였어서 이렇게 배치를 하였는데 말씀하신 것처럼 어노테이션 기준으로 하는 것이 더 좋을 것 같습니다
| Page<StudentCouncilPost> posts = | ||
| postRepository.findUpcomingEvents( | ||
| PostCategory.EVENT, | ||
| now, | ||
| limit, | ||
| pageable | ||
| ); |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/campus/campus/domain/councilpost/domain/entity/PostCategory.java (1)
19-21: EVENT 카테고리의 endDateTime 검증 로직은 이미 문서화된 API 요구사항을 강제하는 것입니다.PostCategory의 validateAndNormalize() 메서드에서 EVENT 타입에 대해
endDateTime을 거부하는 검증은 이미 API 문서에 명시된 요구사항(StudentCouncilPostController 라인 54: "endDateTime은 허용되지 않습니다")과 일치합니다. 정의된 예외(EventEndDateTimeNotAllowedException) 및 에러 코드(2406)가 이미 존재하며, 검증 로직은 이 요구사항을 강제하는 것입니다.EVENT 카테고리는 startDateTime만 필요하고 endDateTime은 null로 정규화되므로(라인 22), 이 제약은 의도된 설계입니다. 다만 기존 데이터에서 API 계약 위반으로 EVENT 게시글에 endDateTime이 포함된 경우가 있다면, 해당 게시글 수정 시 예외가 발생할 수 있습니다. 필요시 레거시 데이터 검증을 수행하세요.
src/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostController.java (1)
135-142:getPost에서@CurrentCouncilId(required = false)설정이 필요합니다.
getPostList(Line 158)과getUpcomingEvents(Line 170)는required = false를 사용하지만,getPost는 그렇지 않습니다. 비로그인 사용자나 일반 학생도 게시글을 조회할 수 있어야 한다면 일관성을 맞춰야 합니다.🔎 수정 제안
@GetMapping("/{postId}") @Operation(summary = "학생회 게시글 단건 조회") -public CommonResponse<PostResponse> getPost(@PathVariable Long postId, @CurrentCouncilId Long councilId) { +public CommonResponse<PostResponse> getPost(@PathVariable Long postId, @CurrentCouncilId(required = false) Long councilId) { PostResponse responseDto = postService.findById(postId, councilId);
♻️ Duplicate comments (3)
src/main/java/com/campus/campus/domain/councilnotice/application/service/StudentCouncilNoticeService.java (3)
89-103: LGTM! N+1 쿼리 문제가 해결되었습니다.이전 리뷰에서 지적된 N+1 문제가
findAllWithWriter메서드 사용으로 해결되었습니다. Writer를 함께 페치하여 추가 쿼리를 방지합니다.
119-142: LGTM! 이미지 URL 비교 문제가 해결되었습니다.이전 리뷰에서 지적된 "동일 URL 재사용 시 OCI 파일 삭제" 문제가
cleanupUnusedImages에서 기존 URL과 새 URL을 비교하여 실제로 제거된 이미지만 OCI에서 삭제하도록 수정되었습니다.
163-173: LGTM! 예외 처리가 개선되었습니다.이전 리뷰에서 요청된 대로
Exception대신NoticeOciImageDeleteFailedException을 사용하여 더 구체적인 예외 처리가 적용되었습니다.
🧹 Nitpick comments (7)
src/main/java/com/campus/campus/domain/councilnotice/application/dto/response/NoticeListItemResponse.java (1)
7-14:isWriter타입 일관성 확인 필요
isWriter필드가 primitiveboolean으로 선언되어 있습니다. 관련 DTO인PostListItemResponseDto에서는Boolean(wrapper) 타입을 사용하고 있습니다.primitive 타입은 null을 표현할 수 없으므로, 의도적인 설계라면 괜찮지만, 코드베이스 전체의 일관성을 위해
Boolean사용을 고려해 보세요.🔎 제안된 수정
@Builder public record NoticeListItemResponse( Long id, String title, - boolean isWriter, + Boolean isWriter, LocalDateTime createdAt, LocalDateTime updatedAt ) { }src/main/java/com/campus/campus/domain/councilnotice/application/dto/request/NoticeRequest.java (1)
5-9: 유효성 검증 어노테이션 누락
PostRequestDto와 같은 다른 요청 DTO에서는title과content필드에@NotBlank어노테이션을 사용하고 있습니다. 일관성과 데이터 무결성을 위해 유효성 검증 어노테이션 추가를 권장합니다.🔎 제안된 수정
package com.campus.campus.domain.councilnotice.application.dto.request; import java.util.List; + +import jakarta.validation.constraints.NotBlank; public record NoticeRequest( + @NotBlank String title, + @NotBlank String content, List<String> imageUrls ) { }src/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostResponseCode.java (1)
18-18: DELETE 응답 코드 RESTful 관례 검토
POST_DELETE_SUCCESS가200 OK를 사용 중입니다. RESTful 표준에 따르면 DELETE 성공 시 응답 본문이 없을 때204 No Content가 더 적절합니다.다만 현재 애플리케이션의
CommonResponse패턴은 모든 응답을 래퍼 구조로 감싸기 때문에 기술적으로 응답 본문이 존재합니다. 이는 프레임워크 차원의 설계 선택이며, DELETE 외에도 다른 작업 완료 엔드포인트들(회원탈퇴, 이메일 변경 등)에서도 동일하게CommonResponse<Void>+ 200을 사용하고 있습니다.응답 의미론을 더 엄격히 하려면
204 No Content로 변경하는 것을 고려할 수 있지만, 이는 애플리케이션 전반의 설계 패턴 개선과 함께 진행해야 합니다.src/main/java/com/campus/campus/domain/councilnotice/domain/entity/StudentCouncilNotice.java (1)
21-26: @table 어노테이션으로 테이블 명을 명시적으로 지정하는 것을 권장합니다.프로젝트의 다른 엔티티들(예: StudentCouncil)은
@Table(name = "...")을 사용하여 테이블 명을 명시적으로 지정합니다. 일관성을 위해 이 엔티티에도 동일한 패턴을 적용하는 것이 좋습니다.🔎 제안하는 수정사항
@Entity +@Table(name = "student_council_notices") @Getter @NoArgsConstructor(access = AccessLevel.PROTECTED) @AllArgsConstructor @Builder public class StudentCouncilNotice extends BaseEntity {src/main/java/com/campus/campus/domain/councilnotice/application/mapper/StudentCouncilNoticeMapper.java (1)
12-52: @component 어노테이션을 추가하여 Spring 빈으로 등록하는 것을 권장합니다.프로젝트의 다른 매퍼 클래스들(예:
SchoolFindMapper,StudentCouncilLoginMapper)은@Component어노테이션을 사용하여 Spring 빈으로 등록하고 인스턴스 메서드를 사용합니다. 일관성을 위해 이 매퍼도 동일한 패턴을 따르는 것이 좋습니다.🔎 제안하는 수정사항
+import org.springframework.stereotype.Component; + +@Component public class StudentCouncilNoticeMapper { - public static StudentCouncilNotice createStudentCouncilNotice(StudentCouncil writer, NoticeRequest dto) { + public StudentCouncilNotice createStudentCouncilNotice(StudentCouncil writer, NoticeRequest dto) { return StudentCouncilNotice.builder() .title(dto.title()) .content(dto.content()) .writer(writer) .build(); } - public static NoticeImage createStudentCouncilNoticeImage(StudentCouncilNotice notice, String imageUrl) { + public NoticeImage createStudentCouncilNoticeImage(StudentCouncilNotice notice, String imageUrl) { return NoticeImage.builder() .notice(notice) .imageUrl(imageUrl) .build(); } - public static NoticeResponse toNoticeResponse(StudentCouncilNotice notice, List<String> imageUrls, Long councilId) { + public NoticeResponse toNoticeResponse(StudentCouncilNotice notice, List<String> imageUrls, Long councilId) { return NoticeResponse.builder() .id(notice.getId()) .writerId(notice.getWriter().getId()) .writerName(notice.getWriter().getFullCouncilName()) .isWriter(notice.isWrittenByCouncil(councilId)) .title(notice.getTitle()) .content(notice.getContent()) .images(imageUrls) .createdAt(notice.getCreatedAt()) .updatedAt(notice.getUpdatedAt()) .build(); } - public static NoticeListItemResponse toNoticeListItemResponse(StudentCouncilNotice notice, Long currentUserId) { + public NoticeListItemResponse toNoticeListItemResponse(StudentCouncilNotice notice, Long currentUserId) { return NoticeListItemResponse.builder() .id(notice.getId()) .title(notice.getTitle()) .isWriter(notice.isWrittenByCouncil(currentUserId)) .createdAt(notice.getCreatedAt()) .updatedAt(notice.getUpdatedAt()) .build(); } }Based on learnings from other mapper classes in the codebase.
src/main/java/com/campus/campus/domain/councilnotice/domain/entity/NoticeImage.java (1)
19-36: LGTM! 엔티티 구조가 적절합니다.BaseEntity 상속, LAZY 페치 전략, 보호된 기본 생성자 등 JPA 엔티티 모범 사례를 잘 따르고 있습니다.
선택적으로
imageUrl컬럼에 길이 제한을 추가하는 것을 고려해볼 수 있습니다:@Column(nullable = false, length = 500) private String imageUrl;src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java (1)
110-126: LGTM! 72시간 이내 행사 조회 로직이 올바르게 구현되었습니다.시간 범위 쿼리와
startDateTime기준 오름차순 정렬이 적절합니다.테스트 용이성을 위해
LocalDateTime.now()대신Clock을 주입받는 방식을 고려해볼 수 있습니다:private final Clock clock; // ... LocalDateTime now = LocalDateTime.now(clock);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
src/main/java/com/campus/campus/domain/councilnotice/application/dto/request/NoticeRequest.javasrc/main/java/com/campus/campus/domain/councilnotice/application/dto/response/NoticeListItemResponse.javasrc/main/java/com/campus/campus/domain/councilnotice/application/dto/response/NoticeResponse.javasrc/main/java/com/campus/campus/domain/councilnotice/application/exception/ErrorCode.javasrc/main/java/com/campus/campus/domain/councilnotice/application/exception/NotNoticeWriterException.javasrc/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeImageLimitExceededException.javasrc/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeNotFoundException.javasrc/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeOciImageDeleteFailedException.javasrc/main/java/com/campus/campus/domain/councilnotice/application/mapper/StudentCouncilNoticeMapper.javasrc/main/java/com/campus/campus/domain/councilnotice/application/service/StudentCouncilNoticeService.javasrc/main/java/com/campus/campus/domain/councilnotice/domain/entity/NoticeImage.javasrc/main/java/com/campus/campus/domain/councilnotice/domain/entity/StudentCouncilNotice.javasrc/main/java/com/campus/campus/domain/councilnotice/domain/repository/NoticeImageRepository.javasrc/main/java/com/campus/campus/domain/councilnotice/domain/repository/StudentCouncilNoticeRepository.javasrc/main/java/com/campus/campus/domain/councilnotice/presentation/NoticeResponseCode.javasrc/main/java/com/campus/campus/domain/councilnotice/presentation/StudentCouncilNoticeController.javasrc/main/java/com/campus/campus/domain/councilpost/application/dto/request/PostRequest.javasrc/main/java/com/campus/campus/domain/councilpost/application/dto/response/PostListItemResponse.javasrc/main/java/com/campus/campus/domain/councilpost/application/dto/response/PostResponse.javasrc/main/java/com/campus/campus/domain/councilpost/application/exception/ErrorCode.javasrc/main/java/com/campus/campus/domain/councilpost/application/exception/PostOciImageDeleteFailedException.javasrc/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.javasrc/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.javasrc/main/java/com/campus/campus/domain/councilpost/domain/entity/PostCategory.javasrc/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostController.javasrc/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostResponseCode.javasrc/main/java/com/campus/campus/global/oci/exception/ErrorCode.java
🧰 Additional context used
🧬 Code graph analysis (15)
src/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeNotFoundException.java (7)
src/main/java/com/campus/campus/domain/council/application/exception/StudentCouncilNotFoundException.java (2)
StudentCouncilNotFoundException(5-9)StudentCouncilNotFoundException(6-8)src/main/java/com/campus/campus/domain/school/application/exception/CollegeNotFoundException.java (2)
CollegeNotFoundException(5-9)CollegeNotFoundException(6-8)src/main/java/com/campus/campus/domain/councilpost/application/exception/PostNotFoundException.java (1)
PostNotFoundException(5-9)src/main/java/com/campus/campus/domain/school/application/exception/SchoolNotFoundException.java (1)
SchoolNotFoundException(5-9)src/main/java/com/campus/campus/domain/council/application/exception/InvalidCouncilScopeException.java (1)
InvalidCouncilScopeException(5-9)src/main/java/com/campus/campus/domain/council/application/exception/SignupEmailNotFoundException.java (2)
SignupEmailNotFoundException(6-8)SignupEmailNotFoundException(5-9)src/main/java/com/campus/campus/domain/council/application/exception/CouncilSignupForbiddenException.java (1)
CouncilSignupForbiddenException(5-9)
src/main/java/com/campus/campus/domain/councilnotice/application/mapper/StudentCouncilNoticeMapper.java (2)
src/main/java/com/campus/campus/domain/council/application/mapper/StudentCouncilLoginMapper.java (2)
Component(16-56)createStudentCouncil(21-34)src/main/java/com/campus/campus/domain/school/application/mapper/SchoolFindMapper.java (1)
Component(14-54)
src/main/java/com/campus/campus/domain/councilpost/domain/entity/PostCategory.java (1)
src/main/java/com/campus/campus/domain/councilpost/application/dto/request/PostRequestDto.java (1)
PostRequestDto(14-42)
src/main/java/com/campus/campus/domain/councilnotice/application/dto/response/NoticeListItemResponse.java (3)
src/main/java/com/campus/campus/domain/councilpost/application/dto/response/PostListItemResponseDto.java (1)
PostListItemResponseDto(8-18)src/main/java/com/campus/campus/domain/user/application/dto/response/UserFirstProfileResponse.java (1)
Builder(6-17)src/main/java/com/campus/campus/domain/councilpost/application/dto/response/PostResponseDto.java (1)
Builder(13-35)
src/main/java/com/campus/campus/domain/councilpost/application/dto/response/PostResponse.java (1)
src/main/java/com/campus/campus/domain/councilpost/application/dto/response/PostResponseDto.java (1)
Builder(13-35)
src/main/java/com/campus/campus/domain/councilnotice/application/dto/request/NoticeRequest.java (3)
src/main/java/com/campus/campus/domain/councilpost/application/dto/request/PostRequestDto.java (1)
PostRequestDto(14-42)src/main/java/com/campus/campus/domain/council/application/dto/request/StudentCouncilSignUpRequest.java (1)
StudentCouncilSignUpRequest(11-40)src/main/java/com/campus/campus/domain/user/application/dto/request/UserProfileRequest.java (1)
UserProfileRequest(5-12)
src/main/java/com/campus/campus/domain/councilpost/application/exception/ErrorCode.java (5)
src/main/java/com/campus/campus/global/oci/exception/OciObjectDeleteFailException.java (2)
OciObjectDeleteFailException(5-9)OciObjectDeleteFailException(6-8)src/main/java/com/campus/campus/domain/council/application/exception/ErrorCode.java (1)
Getter(10-28)src/main/java/com/campus/campus/global/oci/exception/OciPresignedUrlCreateFailException.java (1)
OciPresignedUrlCreateFailException(5-9)src/main/java/com/campus/campus/domain/school/application/exception/ErrorCode.java (1)
Getter(10-22)src/main/java/com/campus/campus/global/common/exception/ErrorCodeInterface.java (1)
ErrorCodeInterface(5-11)
src/main/java/com/campus/campus/domain/councilnotice/domain/entity/StudentCouncilNotice.java (1)
src/main/java/com/campus/campus/domain/council/domain/entity/StudentCouncil.java (2)
Entity(28-90)changeEmail(87-89)
src/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostController.java (2)
src/main/java/com/campus/campus/domain/councilpost/domain/entity/StudentCouncilPost.java (1)
Entity(25-98)src/main/java/com/campus/campus/domain/councilpost/application/dto/request/PostRequestDto.java (1)
PostRequestDto(14-42)
src/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeImageLimitExceededException.java (4)
src/main/java/com/campus/campus/domain/councilpost/application/exception/PostImageLimitExceededException.java (2)
PostImageLimitExceededException(5-9)PostImageLimitExceededException(6-8)src/main/java/com/campus/campus/domain/councilpost/application/exception/ThumbnailRequiredException.java (1)
ThumbnailRequiredException(5-9)src/main/java/com/campus/campus/domain/council/application/exception/EmailAlreadyExistsException.java (1)
EmailAlreadyExistsException(5-9)src/main/java/com/campus/campus/global/oci/exception/OciObjectDeleteFailException.java (1)
OciObjectDeleteFailException(5-9)
src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java (7)
src/main/java/com/campus/campus/domain/councilpost/application/exception/NotPostWriterException.java (1)
NotPostWriterException(5-9)src/main/java/com/campus/campus/domain/councilpost/application/exception/PostImageLimitExceededException.java (1)
PostImageLimitExceededException(5-9)src/main/java/com/campus/campus/domain/councilpost/application/exception/PostNotFoundException.java (1)
PostNotFoundException(5-9)src/main/java/com/campus/campus/domain/councilpost/application/exception/PostOciImageDeleteFailedException.java (1)
PostOciImageDeleteFailedException(6-10)src/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java (1)
StudentCouncilPostMapper(14-77)src/main/java/com/campus/campus/domain/councilpost/domain/entity/StudentCouncilPost.java (1)
Entity(25-98)src/main/java/com/campus/campus/domain/councilpost/application/dto/request/PostRequestDto.java (1)
PostRequestDto(14-42)
src/main/java/com/campus/campus/domain/councilpost/application/dto/response/PostListItemResponse.java (1)
src/main/java/com/campus/campus/domain/councilpost/application/dto/response/PostListItemResponseDto.java (1)
PostListItemResponseDto(8-18)
src/main/java/com/campus/campus/domain/councilnotice/domain/repository/NoticeImageRepository.java (2)
src/main/java/com/campus/campus/domain/councilpost/domain/repository/PostImageRepository.java (3)
PostImageRepository(10-17)deleteByPost(14-14)findAllByPostOrderByIdAsc(16-16)src/main/java/com/campus/campus/domain/councilpost/domain/entity/PostImage.java (1)
Entity(18-36)
src/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java (1)
src/main/java/com/campus/campus/domain/councilpost/domain/entity/StudentCouncilPost.java (1)
Entity(25-98)
src/main/java/com/campus/campus/domain/councilnotice/application/service/StudentCouncilNoticeService.java (6)
src/main/java/com/campus/campus/domain/council/application/exception/StudentCouncilNotFoundException.java (1)
StudentCouncilNotFoundException(5-9)src/main/java/com/campus/campus/domain/councilnotice/application/exception/NotNoticeWriterException.java (1)
NotNoticeWriterException(5-9)src/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeImageLimitExceededException.java (1)
NoticeImageLimitExceededException(5-9)src/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeNotFoundException.java (1)
NoticeNotFoundException(5-9)src/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeOciImageDeleteFailedException.java (1)
NoticeOciImageDeleteFailedException(5-9)src/main/java/com/campus/campus/domain/councilnotice/application/mapper/StudentCouncilNoticeMapper.java (1)
StudentCouncilNoticeMapper(12-52)
🔇 Additional comments (25)
src/main/java/com/campus/campus/global/oci/exception/ErrorCode.java (1)
16-18: 메시지 필드 추가가 적절합니다.
message필드를 추가하여 에러 코드를 더욱 명시적이고 설명적으로 만든 것은 좋은 개선사항입니다. 모든 필드가final로 선언되어 있고@RequiredArgsConstructor와 함께 사용되어 올바른 초기화가 보장됩니다.src/main/java/com/campus/campus/domain/councilnotice/presentation/NoticeResponseCode.java (1)
10-23: LGTM!응답 코드 enum이 적절하게 구성되어 있습니다. HTTP 상태 코드와 int code 값이 일관성 있게 매칭되어 있고,
ResponseCodeInterface구현 패턴을 잘 따르고 있습니다.src/main/java/com/campus/campus/domain/councilpost/application/dto/request/PostRequest.java (1)
14-41: LGTM!
PostRequestDto에서PostRequest로의 이름 변경이 적절합니다. 모든 필드, 유효성 검증 어노테이션, JSON 포맷 설정이 그대로 유지되어 있습니다.src/main/java/com/campus/campus/domain/councilpost/application/dto/response/PostResponse.java (1)
13-35: LGTM!
PostResponseDto에서PostResponse로의 이름 변경이 깔끔하게 이루어졌습니다. 모든 필드와 어노테이션이 유지되어 있습니다.src/main/java/com/campus/campus/domain/councilpost/application/dto/response/PostListItemResponse.java (1)
8-18: LGTM!
PostListItemResponseDto에서PostListItemResponse로의 이름 변경이 적절합니다. 원본 구조가 그대로 유지되어 있습니다.src/main/java/com/campus/campus/domain/councilnotice/application/exception/NotNoticeWriterException.java (1)
5-9: LGTM!기존 프로젝트의 예외 클래스 패턴(PostNotFoundException, SchoolNotFoundException 등)과 일관성 있게 구현되었습니다.
src/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeNotFoundException.java (1)
5-9: LGTM!기존 예외 클래스들(PostNotFoundException, SchoolNotFoundException 등)과 동일한 패턴으로 구현되어 일관성이 유지됩니다.
src/main/java/com/campus/campus/domain/councilnotice/domain/repository/NoticeImageRepository.java (2)
10-14: LGTM - 기존 패턴과 일관성 있는 구현
PostImageRepository와 동일한 패턴으로 구현되어 있습니다.
16-16:deleteByNotice메서드에@Modifying어노테이션은 필요하지 않습니다Spring Data JPA에서
@Modifying어노테이션은 명시적@Query메서드에만 필요합니다. 파생된 쿼리 메서드(메서드 이름 기반)인deleteByNotice는 엔티티를 로드한 후CrudRepository.delete(Iterable)을 호출하므로, 어노테이션 없이도 JPA 라이프사이클 콜백과 영속성 컨텍스트가 정상적으로 작동합니다.현재 구현은 올바릅니다.
StudentCouncilNoticeService.update()메서드의@Transactional어노테이션(105줄)이 트랜잭션 컨텍스트를 제공하며,PostImageRepository.deleteByPost()도 동일한 패턴으로 작동 중입니다.Likely an incorrect or invalid review comment.
src/main/java/com/campus/campus/domain/councilnotice/application/dto/response/NoticeResponse.java (1)
8-19: LGTM!Record와
@Builder조합으로 깔끔하게 구현되었습니다. 필요한 모든 필드가 포함되어 있습니다.src/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeOciImageDeleteFailedException.java (1)
5-9: LGTM!기존 예외 클래스 패턴을 따르며, 같은 도메인 내의
ErrorCode를 사용하여 도메인 분리 원칙을 준수합니다.src/main/java/com/campus/campus/domain/councilnotice/application/exception/ErrorCode.java (1)
10-23: 구현이 올바르고 명확합니다.에러 코드가 적절한 HTTP 상태와 명확한 메시지로 잘 정의되어 있습니다. ErrorCodeInterface를 올바르게 구현하고 있으며, 에러 코드 번호 체계도 일관성 있게 사용되고 있습니다.
src/main/java/com/campus/campus/domain/councilpost/domain/entity/PostCategory.java (1)
6-6: DTO 클래스명 변경이 일관되게 적용되었습니다.
PostRequestDto에서PostRequest로의 네이밍 변경이 모든 메서드 시그니처에 올바르게 적용되었습니다. PR 전체의 DTO 네이밍 규칙 통일과 일치합니다.Also applies to: 15-15, 28-28, 39-39
src/main/java/com/campus/campus/domain/councilnotice/application/mapper/StudentCouncilNoticeMapper.java (1)
29-41: 매핑 로직이 올바르고 명확합니다.엔티티를 DTO로 변환하는 로직이 잘 구현되어 있으며,
isWrittenByCouncil메서드를 활용한 권한 체크와getFullCouncilName을 통한 작성자 정보 조회가 적절합니다.src/main/java/com/campus/campus/domain/councilnotice/presentation/StudentCouncilNoticeController.java (2)
46-56: LGTM!페이지네이션과 선택적
councilId처리가 적절합니다. 1-based 페이지 파라미터가 사용자 친화적입니다.
83-92: LGTM!단건 조회 API가 적절하게 구현되어 있으며, 로그인하지 않은 사용자도 조회할 수 있도록
@CurrentCouncilId(required = false)가 올바르게 설정되어 있습니다.src/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostController.java (3)
113-124: LGTM!게시글 수정 API가 올바르게 구현되었습니다.
@PreAuthorize로 권한 검증,@Valid로 입력 검증이 적절히 적용되어 있습니다.
126-133: LGTM!게시글 삭제 API 구현이 적절합니다.
165-175: LGTM!72시간 이내 행사 조회 API가 잘 구현되어 있습니다.
startDateTime기준 오름차순 정렬과 선택적councilId처리가 적절합니다.src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java (2)
152-158: LGTM!OCI 이미지 삭제 실패 시 예외를 잡아 로깅하고 계속 진행하는 방식이 적절합니다. 이미지 삭제 실패가 전체 삭제 작업을 중단시키지 않습니다.
214-243: LGTM!
cleanupUnusedImages메서드가 기존 이미지 중 새 URL 목록에 없는 이미지만 필터링하여 삭제합니다. 이전 리뷰에서 지적된 "동일 URL 재사용 시 파일 삭제" 문제가 올바르게 해결되어 있습니다.src/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java (2)
31-54: LGTM!
toPostResponse메서드가 이벤트/제휴 카테고리에 따라 적절한 날짜 필드를 설정합니다. Builder 패턴 사용이 깔끔합니다.
56-76: LGTM!팩토리 메서드
createStudentCouncilPost와createPostImage가 명확한 네이밍과 함께 잘 구현되어 있습니다.src/main/java/com/campus/campus/domain/councilnotice/application/service/StudentCouncilNoticeService.java (2)
44-72: LGTM!
create메서드가 이미지 개수 검증, 작성자 조회, 공지 저장, 이미지 저장을 올바른 순서로 수행합니다. 트랜잭션 내에서 일관성이 유지됩니다.
144-174: LGTM!
delete메서드가 작성자 권한 검증 후 DB 레코드를 먼저 삭제하고, OCI 이미지 정리를 수행합니다. OCI 삭제 실패가 전체 삭제 작업에 영향을 주지 않도록 적절히 처리됩니다.
| public class NoticeImageLimitExceededException extends ApplicationException { | ||
| public NoticeImageLimitExceededException() { | ||
| super(ErrorCode.NOTICE_OCI_IMAGE_DELETE_FAILED); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the ErrorCode enum file
fd -e java -type f | xargs rg -l "enum ErrorCode"Repository: our-campUS/our-campUS-BE
Length of output: 974
🏁 Script executed:
# Search for all NOTICE-related error codes
rg "NOTICE_" --type java -B 2 -A 2 | head -100Repository: our-campUS/our-campUS-BE
Length of output: 9404
🏁 Script executed:
# Check the context of PostImageLimitExceededException to compare
rg "PostImageLimitExceededException" --type java -B 5 -A 5Repository: our-campUS/our-campUS-BE
Length of output: 6462
ErrorCode 불일치 - 심각한 버그
NoticeImageLimitExceededException이 NOTICE_OCI_IMAGE_DELETE_FAILED ErrorCode를 사용하고 있습니다. 예외 이름은 이미지 개수 초과를 나타내지만, 사용된 에러 코드는 OCI 이미지 삭제 실패를 나타내므로 의미가 맞지 않습니다.
PostImageLimitExceededException이 POST_IMAGE_LIMIT_EXCEEDED를 사용하는 것과 일관성 있게, 이 예외도 NOTICE_IMAGE_LIMIT_EXCEEDED와 같은 적절한 ErrorCode를 사용해야 합니다. 현재 설정으로는 사용자에게 잘못된 오류 메시지가 전달됩니다.
필수 조치:
src/main/java/com/campus/campus/domain/councilnotice/application/exception/ErrorCode.java에서NOTICE_IMAGE_LIMIT_EXCEEDEDErrorCode 추가NoticeImageLimitExceededException에서 새로 추가된 ErrorCode 사용
🤖 Prompt for AI Agents
In
src/main/java/com/campus/campus/domain/councilnotice/application/exception/NoticeImageLimitExceededException.java
around lines 5 to 8 and
src/main/java/com/campus/campus/domain/councilnotice/application/exception/ErrorCode.java,
the exception uses the wrong ErrorCode (NOTICE_OCI_IMAGE_DELETE_FAILED); add a
new enum entry NOTICE_IMAGE_LIMIT_EXCEEDED to ErrorCode.java and update
NoticeImageLimitExceededException to call
super(ErrorCode.NOTICE_IMAGE_LIMIT_EXCEEDED) so the error code matches the
exception name and message.
src/main/java/com/campus/campus/domain/councilnotice/domain/entity/StudentCouncilNotice.java
Outdated
Show resolved
Hide resolved
src/main/java/com/campus/campus/domain/councilnotice/domain/entity/StudentCouncilNotice.java
Show resolved
Hide resolved
...com/campus/campus/domain/councilnotice/domain/repository/StudentCouncilNoticeRepository.java
Show resolved
Hide resolved
...com/campus/campus/domain/councilnotice/domain/repository/StudentCouncilNoticeRepository.java
Outdated
Show resolved
Hide resolved
...java/com/campus/campus/domain/councilnotice/presentation/StudentCouncilNoticeController.java
Show resolved
Hide resolved
src/main/java/com/campus/campus/domain/councilpost/application/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
| import com.campus.campus.domain.councilnotice.application.exception.ErrorCode; | ||
| import com.campus.campus.global.common.exception.ApplicationException; | ||
|
|
||
| public class PostOciImageDeleteFailedException extends ApplicationException { | ||
| public PostOciImageDeleteFailedException() { | ||
| super(ErrorCode.NOTICE_OCI_IMAGE_DELETE_FAILED); |
There was a problem hiding this comment.
도메인 간 의존성 및 ErrorCode 불일치 문제
councilpost 도메인에서 councilnotice 도메인의 ErrorCode를 import하고 있으며, NOTICE_OCI_IMAGE_DELETE_FAILED라는 notice 전용 에러 코드를 post 예외에서 사용하고 있습니다.
이는 도메인 분리 원칙을 위반하고 불필요한 결합을 발생시킵니다.
🔎 권장 수정안
councilpost 도메인에 자체 ErrorCode를 정의하거나, 공통 에러 코드를 별도 모듈로 분리하세요:
-import com.campus.campus.domain.councilnotice.application.exception.ErrorCode;
+import com.campus.campus.domain.councilpost.application.exception.ErrorCode;
public class PostOciImageDeleteFailedException extends ApplicationException {
public PostOciImageDeleteFailedException() {
- super(ErrorCode.NOTICE_OCI_IMAGE_DELETE_FAILED);
+ super(ErrorCode.POST_OCI_IMAGE_DELETE_FAILED);
}
}그리고 councilpost의 ErrorCode에 다음을 추가:
POST_OCI_IMAGE_DELETE_FAILED(6102, HttpStatus.INTERNAL_SERVER_ERROR, "게시글 이미지 삭제에 실패했습니다.")Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/com/campus/campus/domain/councilpost/application/exception/PostOciImageDeleteFailedException.java
around lines 3 to 8, the class wrongly imports ErrorCode from the councilnotice
domain and uses a notice-specific code; replace this dependency by creating or
using a councilpost-scoped ErrorCode (or a common shared ErrorCode module), add
a POST_OCI_IMAGE_DELETE_FAILED entry (e.g. code 6102,
HttpStatus.INTERNAL_SERVER_ERROR, message "게시글 이미지 삭제에 실패했습니다.") to that enum,
and update this exception to reference the new councilpost ErrorCode constant
instead of NOTICE_OCI_IMAGE_DELETE_FAILED so the domain separation and error
code consistency are preserved.
...n/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java
Outdated
Show resolved
Hide resolved
src/main/java/com/campus/campus/global/oci/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
…eRespository의 findByIdWithWriter 쿼리 수정
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/campus/campus/global/oci/mapper/PresignedUrlMapper.java (1)
14-44: static 메서드와 @component 어노테이션 간의 심각한 불일치를 수정하세요.이 클래스는
@Component와@RequiredArgsConstructor로 어노테이션되어 있지만 모든 메서드가static으로 선언되어 있습니다. 이는 다음과 같은 문제를 야기합니다:
- Spring이 사용되지 않는 빈 인스턴스를 생성하여 불필요한 오버헤드가 발생합니다
- 의존성 주입이 필요한 경우 활용할 수 없습니다
- 코드베이스의 다른 매퍼들(
StudentCouncilNoticeMapper,StudentCouncilPostMapper)과 패턴이 일관되지 않습니다@RequiredArgsConstructor는 주입할 필드가 없어 불필요합니다관련 코드 스니펫을 보면
StudentCouncilNoticeMapper와StudentCouncilPostMapper는@Component를 사용하면서 인스턴스 메서드(non-static)를 가지고 있습니다. 이 클래스도 동일한 패턴을 따라야 합니다.🔎 제안하는 수정사항
옵션 1 (권장): Spring 컴포넌트로 변경 (다른 매퍼와 일관성 유지)
@Component @RequiredArgsConstructor public class PresignedUrlMapper { - public static CreatePreauthenticatedRequestRequest toPutObjectRequest(String bucketName, String namespaceName, + public CreatePreauthenticatedRequestRequest toPutObjectRequest(String bucketName, String namespaceName, String objectName, long expiresAtMillis) { CreatePreauthenticatedRequestDetails details = CreatePreauthenticatedRequestDetails.builder() .name("upload-" + UUID.randomUUID()) .objectName(objectName) .accessType(CreatePreauthenticatedRequestDetails.AccessType.ObjectWrite) .timeExpires(new Date(expiresAtMillis)) .build(); return CreatePreauthenticatedRequestRequest.builder() .bucketName(bucketName) .namespaceName(namespaceName) .createPreauthenticatedRequestDetails(details) .build(); } - public static DeleteObjectRequest toDeleteObjectRequest( + public DeleteObjectRequest toDeleteObjectRequest( String bucketName, String namespaceName, String objectName ) { return DeleteObjectRequest.builder() .bucketName(bucketName) .namespaceName(namespaceName) .objectName(objectName) .build(); } }옵션 2: 순수 유틸리티 클래스로 유지 (어노테이션 제거)
-@Component -@RequiredArgsConstructor public class PresignedUrlMapper { + private PresignedUrlMapper() { + // utility class + } + public static CreatePreauthenticatedRequestRequest toPutObjectRequest(String bucketName, String namespaceName, String objectName, long expiresAtMillis) {
🧹 Nitpick comments (5)
src/main/java/com/campus/campus/domain/councilnotice/application/service/StudentCouncilNoticeService.java (1)
106-144: 업데이트 로직이 올바르게 구현되었습니다.기존 이미지 처리 로직이 정확합니다.
cleanupUnusedImages는 새 URL 목록에 없는 이미지만 OCI에서 삭제하므로, 재사용되는 URL은 안전하게 보존됩니다.🔎 중복 코드 리팩터링 제안
이미지 개수 검증 로직(lines 48-50, 109-111)과 이미지 URL 조회 로직(lines 66-70, 81-85, 137-141)이 반복됩니다. 향후 유지보수성을 위해 헬퍼 메서드로 추출할 수 있습니다:
private void validateImageCount(NoticeRequest dto) { if (dto.imageUrls() != null && dto.imageUrls().size() > MAX_IMAGE_COUNT) { throw new NoticeImageLimitExceededException(); } } private List<String> getImageUrls(StudentCouncilNotice notice) { return noticeImageRepository .findAllByNoticeOrderByIdAsc(notice) .stream() .map(NoticeImage::getImageUrl) .toList(); }src/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java (1)
37-42:writer가 null일 경우 NPE 발생 가능Line 38에서
post.getWriter()를 직접 호출하고 Line 41-42에서writer.getId(),writer.getFullCouncilName()에 접근합니다.writer가 null인 경우 NPE가 발생할 수 있습니다.서비스 레이어에서 항상 writer가 포함된 post를 조회하도록 보장되어 있다면 현재 코드로도 문제없지만, 방어적 코딩을 위해 null 체크를 추가하거나 서비스에서의 보장을 문서화하는 것을 권장합니다.
src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java (3)
134-139: 권한 체크 시 null 안전성 및 일관성 개선 권장Line 137에서
post.getWriter().getId().equals(councilId)를 직접 호출하고 있습니다.writer가 null인 경우 NPE가 발생할 수 있습니다.매퍼에서는
post.isWrittenByCouncil(currentUserId)를 사용하여 null 체크를 포함한 엔티티 메서드를 활용하고 있으므로, 서비스에서도 동일한 패턴을 사용하는 것이 일관성 있습니다.🔎 수정 제안
StudentCouncilPost post = postRepository.findByIdWithFullInfo(postId) .orElseThrow(PostNotFoundException::new); - if (!post.getWriter().getId().equals(councilId)) { + if (!post.isWrittenBy(councilId)) { throw new NotPostWriterException(); }Line 174의
update메서드에서도 동일하게 수정해주세요.
114-130: 테스트 용이성을 위한Clock주입 고려
LocalDateTime.now()를 직접 호출하면 단위 테스트에서 시간을 제어하기 어렵습니다. 테스트 용이성을 높이려면Clock을 주입받아 사용하는 것을 권장합니다.현재 구현도 기능적으로 문제없지만, 향후 테스트 작성 시 참고해주세요.
🔎 Clock 주입 예시
// 필드 추가 private final Clock clock; // 메서드 내에서 사용 LocalDateTime now = LocalDateTime.now(clock);테스트에서는
Clock.fixed()를 사용하여 특정 시점을 고정할 수 있습니다.
171-176:update메서드에도 동일한 null 안전성 개선 필요Line 174에서도
post.getWriter().getId().equals(councilId)를 직접 호출하고 있습니다. Line 137과 동일하게 엔티티 메서드를 사용하도록 수정해주세요.🔎 수정 제안
- if (!post.getWriter().getId().equals(councilId)) { + if (!post.isWrittenBy(councilId)) { throw new NotPostWriterException(); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/campus/campus/domain/councilnotice/application/mapper/StudentCouncilNoticeMapper.javasrc/main/java/com/campus/campus/domain/councilnotice/application/service/StudentCouncilNoticeService.javasrc/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.javasrc/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.javasrc/main/java/com/campus/campus/global/oci/mapper/PresignedUrlMapper.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/campus/campus/domain/councilnotice/application/mapper/StudentCouncilNoticeMapper.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java (3)
src/main/java/com/campus/campus/domain/councilnotice/application/mapper/StudentCouncilNoticeMapper.java (1)
Component(16-58)src/main/java/com/campus/campus/global/oci/mapper/PresignedUrlMapper.java (1)
Component(14-45)src/main/java/com/campus/campus/domain/councilpost/domain/entity/StudentCouncilPost.java (1)
Entity(25-98)
src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java (4)
src/main/java/com/campus/campus/domain/councilpost/application/exception/NotPostWriterException.java (1)
NotPostWriterException(5-9)src/main/java/com/campus/campus/domain/councilpost/application/exception/PostImageLimitExceededException.java (1)
PostImageLimitExceededException(5-9)src/main/java/com/campus/campus/domain/councilpost/application/exception/PostNotFoundException.java (1)
PostNotFoundException(5-9)src/main/java/com/campus/campus/domain/councilpost/application/exception/PostOciImageDeleteFailedException.java (1)
PostOciImageDeleteFailedException(5-9)
src/main/java/com/campus/campus/global/oci/mapper/PresignedUrlMapper.java (2)
src/main/java/com/campus/campus/domain/councilnotice/application/mapper/StudentCouncilNoticeMapper.java (1)
Component(16-58)src/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java (1)
Component(18-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
src/main/java/com/campus/campus/domain/councilnotice/application/service/StudentCouncilNoticeService.java (6)
32-43: 서비스 클래스 구조가 잘 설계되었습니다.의존성 주입, 트랜잭션 관리, 상수 정의가 적절하게 구현되어 있습니다.
45-73: 생성 로직이 올바르게 구현되었습니다.이미지 개수 검증, 학생회 존재 확인, 공지 및 이미지 저장 순서가 적절합니다.
75-88: 조회 로직이 적절합니다.전체 정보를 포함한 공지 조회 및 이미지 URL 매핑이 올바르게 구현되어 있습니다.
90-104: 페이지네이션 조회 로직이 잘 구현되었습니다.N+1 문제가 해결되었고, 페이징 및 정렬 처리가 적절합니다.
146-176: 삭제 로직이 적절하게 구현되었습니다.작성자 검증, DB 삭제 순서(이미지 → 공지), OCI 삭제 실패에 대한 비치명적 처리가 올바릅니다.
178-198: 이미지 정리 헬퍼 메서드가 올바르게 구현되었습니다.새 URL 목록에 없는 이미지만 필터링하여 삭제하는 로직이 정확하며, OCI 삭제 실패를 적절히 처리하고 있습니다.
src/main/java/com/campus/campus/domain/councilpost/application/mapper/StudentCouncilPostMapper.java (1)
33-33: 문제 없습니다.isWrittenByCouncil메서드가StudentCouncilPost엔티티에 존재하며, 라인 33과 43에서 올바르게 사용되고 있습니다.
| for (String imageUrl : deleteTargets) { | ||
| try { | ||
| presignedUrlService.deleteImage(imageUrl); | ||
| } catch (Exception e) { | ||
| log.warn("OCI 파일 삭제 실패: {}", imageUrl); | ||
| } catch (PostOciImageDeleteFailedException e) { | ||
| log.warn("OCI 파일 삭제 실패: {}", imageUrl, e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find PresignedUrlService class
find . -type f -name "*.java" | xargs grep -l "class PresignedUrlService"Repository: our-campUS/our-campUS-BE
Length of output: 157
🏁 Script executed:
# Search for deleteImage method in the codebase
rg -n "deleteImage" --type java -A 5Repository: our-campUS/our-campUS-BE
Length of output: 4057
🏁 Script executed:
# Look for PostOciImageDeleteFailedException definition
fd -e java | xargs grep -l "PostOciImageDeleteFailedException"Repository: our-campUS/our-campUS-BE
Length of output: 281
🏁 Script executed:
# Read the full PresignedUrlService.java file
cat -n src/main/java/com/campus/campus/global/oci/application/service/PresignedUrlService.javaRepository: our-campUS/our-campUS-BE
Length of output: 4119
🏁 Script executed:
# Read the PostOciImageDeleteFailedException definition
cat -n src/main/java/com/campus/campus/domain/councilpost/application/exception/PostOciImageDeleteFailedException.javaRepository: our-campUS/our-campUS-BE
Length of output: 449
🏁 Script executed:
# Find OciObjectDeleteFailException definition
fd -e java | xargs grep -l "OciObjectDeleteFailException"Repository: our-campUS/our-campUS-BE
Length of output: 242
🏁 Script executed:
# Check the full definition of OciObjectDeleteFailException
find . -name "OciObjectDeleteFailException.java" -exec cat -n {} \;Repository: our-campUS/our-campUS-BE
Length of output: 415
🏁 Script executed:
# Check if there's any exception mapping or wrapping happening
rg -n "PostOciImageDeleteFailedException" --type java -B 2 -A 2Repository: our-campUS/our-campUS-BE
Length of output: 3397
🏁 Script executed:
# Check if there's any exception handler or aspect converting exceptions
rg -n "OciObjectDeleteFailException" --type java -B 2 -A 2Repository: our-campUS/our-campUS-BE
Length of output: 2265
🏁 Script executed:
# Check the exception hierarchy to see if there's any relationship
rg -n "extends.*Exception|implements.*Exception" src/main/java/com/campus/campus/domain/councilpost/application/exception/PostOciImageDeleteFailedException.java src/main/java/com/campus/campus/global/oci/exception/OciObjectDeleteFailException.javaRepository: our-campUS/our-campUS-BE
Length of output: 421
🏁 Script executed:
# Check if StudentCouncilNoticeService has similar issue
cat -n src/main/java/com/campus/campus/domain/councilnotice/application/service/StudentCouncilNoticeService.java | grep -A 5 -B 5 "deleteImage"Repository: our-campUS/our-campUS-BE
Length of output: 726
presignedUrlService.deleteImage 호출 시 잘못된 예외 타입을 catch하고 있습니다
PresignedUrlService.deleteImage 메서드는 내부적으로 OciObjectDeleteFailException을 던지지만, 현재 코드에서는 PostOciImageDeleteFailedException을 catch하고 있습니다. 이 두 예외는 상속 관계가 없으므로, catch 블록이 실행되지 않고 예외가 상위로 전파됩니다. OciObjectDeleteFailException을 catch하도록 수정이 필요합니다.
🤖 Prompt for AI Agents
In
src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java
around lines 156 to 162, the catch clause is catching
PostOciImageDeleteFailedException while presignedUrlService.deleteImage(...)
actually throws OciObjectDeleteFailException; update the catch to
OciObjectDeleteFailException (and add/import that exception if missing) so the
delete failures are handled and logged as intended, preserving the existing
log.warn call and message.
| if (dto.imageUrls() != null && dto.imageUrls().size() > 10) { | ||
| throw new PostImageLimitExceededException(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
상수 MAX_IMAGE_COUNT 사용으로 일관성 유지 필요
Line 53에서는 MAX_IMAGE_COUNT 상수를 사용하지만, Line 167에서는 하드코딩된 10을 사용하고 있습니다. 일관성을 위해 상수를 사용해주세요.
🔎 수정 제안
- if (dto.imageUrls() != null && dto.imageUrls().size() > 10) {
+ if (dto.imageUrls() != null && dto.imageUrls().size() > MAX_IMAGE_COUNT) {📝 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.
| if (dto.imageUrls() != null && dto.imageUrls().size() > 10) { | |
| throw new PostImageLimitExceededException(); | |
| if (dto.imageUrls() != null && dto.imageUrls().size() > MAX_IMAGE_COUNT) { | |
| throw new PostImageLimitExceededException(); |
🤖 Prompt for AI Agents
In
src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java
around lines 167-168, the image count check uses a hardcoded literal 10 instead
of the existing MAX_IMAGE_COUNT constant declared earlier (line 53); replace the
literal 10 with MAX_IMAGE_COUNT so the validation uses the same constant
everywhere and remove magic numbers to keep consistency.
🔀 변경 내용
✅ 작업 항목
📸 스크린샷 (선택)
공지 게시글
학생
학생회
72시간 행사 조회 API
72시간 지난 행사는 조회 되지 않음
📎 참고 이슈
관련 이슈 번호#23
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.