Refactor: 이미지 정합성 개선#46
Conversation
Walkthrough이미지 참조(ImageRef) 기반 이미지 관리 추가: Image/ImageRef 엔티티·레포지토리·서비스·정리 스케줄러 도입, 그룹·유저 DTO/서비스에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant ImageUploadController as API
participant ImageService as ImageService
participant ImageRepository as ImgRepo
participant ImageRefService as RefSvc
participant S3 as S3
Client->>API: POST /images/presign {fileName,type}
API->>ImageService: getPresignedUrl(fileName)
ImageService->>ImgRepo: findByHash(hash(fileName))
alt 기존 이미지 존재
ImageService->>RefSvc: save(new ImageRef(image))
ImageService->>ImageService: generateUrl(image.s3Key)
ImageService-->>API: url + imageRefId
else 신규 업로드
ImageService->>S3: Presign PUT (5m)
ImageService->>ImgRepo: save(Image)
ImageService->>RefSvc: save(ImageRef)
ImageService-->>API: presigned url + imageRefId
end
API-->>Client: {url,imageRefId}
sequenceDiagram
autonumber
actor Client
participant GroupController as Group API
participant GroupService as GroupService
participant ImageService as ImageService
participant GroupPolicyService as Policy
participant GroupRepo as Repo
Client->>GroupController: POST /groups {imageRefId,...}
GroupController->>GroupService: create(req)
alt imageRefId 제공
GroupService->>ImageService: changeImage(GROUP,newGroupId?,imageRefId)
ImageService-->>GroupService: imageUrl
else 없음
GroupService->>ImageService: changeImage(..., null)
ImageService-->>GroupService: default group imageUrl
end
GroupService->>Repo: save(group with imageUrl)
GroupService-->>Client: groupId
Client->>GroupController: PUT /groups/{id} {imageRefId?,...}
GroupController->>GroupService: changeGroup(id, req)
GroupService->>ImageService: changeImage(GROUP,id, imageRefId?)
ImageService-->>GroupService: newUrl
GroupService->>Policy: changeGroup(id, req, newUrl)
Policy->>Repo: update(group with newUrl)
Policy-->>GroupService: group
GroupService-->>Client: 200 OK
sequenceDiagram
autonumber
participant Scheduler as SchedulerConfig
participant ImageCleanService as CleanSvc
participant ImageRefRepo as RefRepo
participant ImageRepo as ImgRepo
participant S3 as S3
Scheduler->>CleanSvc: daily cleanImageRef()
loop batches
CleanSvc->>RefRepo: findExpiredPending(...)
CleanSvc->>RefRepo: deleteAllInBatch
end
Scheduler->>CleanSvc: weekly cleanImage()
loop batches
CleanSvc->>ImgRepo: findOrphanCandidates(...)
alt has ref
CleanSvc-->>ImgRepo: skip
else no ref
CleanSvc->>S3: deleteObject
CleanSvc->>ImgRepo: hard delete if still no ref
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ 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)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 15
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/project/flipnote/group/repository/GroupRepositoryImpl.java (1)
40-48: 중요: ImageRef 중복으로 인한 cursor 페이징 깨짐 — ImageStatus.USING 필터 및 DISTINCT 적용 필요findAllByCursor 및 findAllByCursorAndUserId에서 LEFT JOIN imageRef에 status 필터가 없어 동일 그룹에 다수 ImageRef가 있으면 중복 행으로 cursor 기반 페이징이 깨집니다. ImageStatus.USING으로 조인 조건을 제한하고 방어적으로 distinct()를 적용하세요.
파일: src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java (관련 쿼리 약 라인 40–56, 72–90)
+import project.flipnote.image.entity.ImageStatus; ... - return queryFactory.select(Projections.constructor( + return queryFactory.select(Projections.constructor( GroupInfo.class, group.id, group.name, group.description, group.category, group.imageUrl, imageRef.id )) + .distinct() .from(group) .where(where) .leftJoin(imageRef) - .on(imageRef.referenceType.eq(ReferenceType.GROUP) - .and(imageRef.referenceId.eq(group.id))) + .on(imageRef.referenceType.eq(ReferenceType.GROUP) + .and(imageRef.referenceId.eq(group.id)) + .and(imageRef.status.eq(ImageStatus.USING))) .orderBy(group.id.desc()) .limit(pageSize+1) .fetch();- return queryFactory.select(Projections.constructor( + return queryFactory.select(Projections.constructor( GroupInfo.class, group.id, group.name, group.description, group.category, group.imageUrl, imageRef.id )) + .distinct() .from(groupMember) .join(groupMember.group, group) .on(groupMember.user.id.eq(userId)) .where(where) .leftJoin(imageRef) - .on(imageRef.referenceType.eq(ReferenceType.GROUP) - .and(imageRef.referenceId.eq(group.id))) + .on(imageRef.referenceType.eq(ReferenceType.GROUP) + .and(imageRef.referenceId.eq(group.id)) + .and(imageRef.status.eq(ImageStatus.USING))) .orderBy(group.id.desc()) .limit(pageSize+1) .fetch();src/main/java/project/flipnote/group/service/GroupPolicyService.java (1)
25-35: Redisson tryLock leaseTime(3s)로 인한 임계구역 조기 해제 위험
tryLock(2, 3, SECONDS)는 3초 후 자동 해제됩니다. 비즈니스 로직이 3초를 넘으면 잠금이 풀려 동시 수정이 가능합니다. Redisson watchdog을 활용하는 오버로드 사용을 권장합니다.- isLocked = lock.tryLock(2, 3, TimeUnit.SECONDS); + // leaseTime 제거 -> watchdog이 자동 연장 + isLocked = lock.tryLock(2, TimeUnit.SECONDS);
🧹 Nitpick comments (50)
src/main/java/project/flipnote/user/model/UserUpdateRequest.java (2)
13-15: 전화번호 NPE 예방 및 imageRefId 유효값 보장
getNormalizedPhone()호출 전에 null/blank 방지용으로@NotBlank를 추가하는 편이 안전합니다(커스텀@ValidPhone이 null 허용 시 NPE 위험).imageRefId는 선택 입력이면@Positive로 양수만 허용해 두세요(미입력은 null 허용).package project.flipnote.user.model; import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Positive; import project.flipnote.common.util.PhoneUtil; import project.flipnote.common.validation.annotation.ValidPhone; public record UserUpdateRequest( @NotEmpty String nickname, - @ValidPhone + @NotBlank + @ValidPhone String phone, @NotNull Boolean smsAgree, - Long imageRefId + @Positive(message = "이미지 참조 id는 양수여야 합니다.") + Long imageRefId ) {Also applies to: 19-20
22-24: (대안) null‑safe 정규화위의
@NotBlank를 추가하지 않는다면, 아래처럼 null‑safe 처리도 가능합니다.public String getNormalizedPhone() { - return PhoneUtil.normalize(phone); + return phone == null ? null : PhoneUtil.normalize(phone); }src/main/java/project/flipnote/group/model/GroupPutRequest.java (1)
3-3: 미사용 import 제거 및 imageRefId 양수 제약 추가
@URL은 더 이상 사용되지 않습니다. import 제거하세요.imageRefId는 선택값이라도 유효 범위를 명확히 하기 위해@Positive권장합니다.package project.flipnote.group.model; -import org.hibernate.validator.constraints.URL; +import jakarta.validation.constraints.Positive; import jakarta.validation.constraints.Max; import jakarta.validation.constraints.Min; import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Size; import project.flipnote.group.entity.Category; public record GroupPutRequest( ... - Long imageRefId + @Positive(message = "이미지 참조 id는 양수여야 합니다.") + Long imageRefId ) {Also applies to: 35-36
src/main/java/project/flipnote/image/model/ImageIdKey.java (1)
3-4: 타입/이름 적절함, 단 이름 범용화는 고려해볼 만
record투영용으로 적절합니다. 스토리지 벤더 종속성 노출을 줄이려면s3Key→storageKey로의 리네이밍을 검토해 보세요(후속 전역 치환 필요).src/main/java/project/flipnote/group/model/GroupCreateRequest.java (2)
3-3: 미사용 import 제거 및 주석 코드 삭제 + 유효성 명확화
- 사용하지 않는
@URLimport 제거.- 주석 처리된
@NotNull은 제거하고, 선택 입력이라면@Positive로 범위만 제한하세요. 필수 입력이라면@NotNull도 함께 선언해 주세요(제품 정책에 맞춰 선택).package project.flipnote.group.model; -import org.hibernate.validator.constraints.URL; +// (unused import 제거) import jakarta.validation.constraints.*; import project.flipnote.group.entity.Category; public record GroupCreateRequest( ... - // @NotNull(message = "이미지 참조 id를 입력해주세요.") - Long imageRefId + @Positive(message = "이미지 참조 id는 양수여야 합니다.") + Long imageRefId ) {Also applies to: 31-33
31-33: 생성 시 기본 이미지 정책 확인 요청
imageRefId미입력(null) 시 기본 이미지가 적용되는지GroupService#createGroup흐름을 확인해 주세요. 문서/Swagger 예시도 일치하도록 업데이트 권장.src/main/java/project/flipnote/auth/model/UserRegisterRequest.java (1)
27-29: 전화번호 null/blank 방지 어노테이션 보강 권장
toCommand()에서 정규화 호출 전 null/blank를 방지하기 위해@NotBlank추가를 권장합니다(커스텀@ValidPhone이 null을 금지하지 않을 수 있음).public record UserRegisterRequest( ... - @ValidPhone - String phone + @NotBlank + @ValidPhone + String phone ) { public UserCreateCommand toCommand() { return new UserCreateCommand(email, name, nickname, smsAgree, getNormalizedPhone()); }Also applies to: 35-37
src/main/java/project/flipnote/image/entity/Image.java (2)
3-6: 불필요한 import 정리사용되지 않는 import들이 있습니다:
org.checkerframework.checker.units.qual.C,SQLDelete,SQLRestriction,SoftDeletableEntity. 제거해 주세요.적용 예시:
-import org.checkerframework.checker.units.qual.C; -import org.hibernate.annotations.SQLDelete; -import org.hibernate.annotations.SQLRestriction; ... -import project.flipnote.common.entity.SoftDeletableEntity;Also applies to: 21-22
23-31: 엔티티 동등성 정의 보완 제안(id 기반 equals/hashCode)영속성 컨텍스트에서 동등성 혼선을 줄이기 위해 id 기반 equals/hashCode를 명시하는 것을 권장합니다.
예시(Lombok):
import lombok.AccessLevel; import lombok.Builder; import lombok.Getter; +import lombok.EqualsAndHashCode; import lombok.NoArgsConstructor; ... -@NoArgsConstructor(access = AccessLevel.PROTECTED) -public class Image extends BaseEntity { +@NoArgsConstructor(access = AccessLevel.PROTECTED) +@EqualsAndHashCode(onlyExplicitlyIncluded = true, callSuper = false) +public class Image extends BaseEntity { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) - private Long id; + @EqualsAndHashCode.Include + private Long id;src/main/java/project/flipnote/image/model/ImageUploadResponseDto.java (1)
6-11: DTO에서 URL 타입 사용에 대한 직렬화 안정성 검토Jackson 직렬화/역직렬화 관점에서
java.net.URL보다String(또는URI)가 호환성이 좋습니다. 외부 API 스펙 고정 목적이면 String 전환을 고려해 주세요.가능한 방향:
-public record ImageUploadResponseDto( - URL url, - Long imageRefId -) { - public static ImageUploadResponseDto from(URL url, Long imageRefId) { - return new ImageUploadResponseDto(url, imageRefId); - } +public record ImageUploadResponseDto( + String url, + Long imageRefId +) { + public static ImageUploadResponseDto from(String url, Long imageRefId) { + return new ImageUploadResponseDto(url, imageRefId); + } }src/main/java/project/flipnote/group/entity/Group.java (1)
110-118: 메서드 파라미터 명확화 및 도메인 규칙 확인
String url→String imageUrl로 명확성 향상 제안.- 최대 인원 검증/잠금 등 도메인 규칙은 서비스 계층에서 보장되는지 확인 바랍니다.
예시:
- public void changeGroup(GroupPutRequest req, String url) { + public void changeGroup(GroupPutRequest req, String imageUrl) { ... - this.imageUrl = url; + this.imageUrl = imageUrl; }src/main/java/project/flipnote/user/model/UserUpdateResponse.java (1)
10-16: 오버로드 추가 권장 — UserUpdateResponse.from(UserProfile) 구현레포지토리 검색 결과 호출은 src/main/java/project/flipnote/user/service/UserService.java:85에서만 발견되며 from(user, req.imageRefId())(두 인자)로 호출됩니다. 구 시그니처 사용 흔적은 없습니다. 오버로드(from(UserProfile user))는 필수는 아니지만 외부(타 모듈/테스트) 호환성 확보를 위해 선택적으로 추가할 것을 권장합니다.
src/main/resources/application.yml (1)
104-111: 기본 이미지 URL을 환경변수로 오버라이드하도록 변경 권장
image.default.* 및 image-clean.*는 @value로 주입되어 사용 중 — 위치:
- src/main/java/project/flipnote/user/entity/UserProfile.java — @value("${image.default.user}")
- src/main/java/project/flipnote/image/service/ImageService.java — @value("${image.default.user}"), @value("${image.default.group}")
- src/main/java/project/flipnote/group/service/GroupService.java — @value("${image.default.group}")
- src/main/java/project/flipnote/image/service/ImageCleanService.java — @value("${image-clean.batch-size}"), @value("${image-clean.orphan-grace-minutes}")
application.yml 하드코딩 완화 제안 (예):
image: default: - user: https://flipnote-bucket.s3.ap-northeast-2.amazonaws.com/image/default/user.png - group: https://flipnote-bucket.s3.ap-northeast-2.amazonaws.com/image/default/group.png + user: ${IMAGE_DEFAULT_USER_URL:https://flipnote-bucket.s3.ap-northeast-2.amazonaws.com/image/default/user.png} + group: ${IMAGE_DEFAULT_GROUP_URL:https://flipnote-bucket.s3.ap-northeast-2.amazonaws.com/image/default/group.png}src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java (1)
81-84: userId 필터는 where 절로 옮기는 것이 가독성과 의도 표현에 적합INNER JOIN의 ON 조건에
groupMember.user.id.eq(userId)를 두기보다, where에 포함하면 의도가 명확합니다(조인 조건은 관계, where는 필터).src/main/java/project/flipnote/group/controller/docs/GroupControllerDocs.java (1)
80-90: 응답 예시의 필드 불일치 가능성(imageUrl vs imageRefId)이미지 참조 기반으로 전환되었다면 응답 예시에도
imageRefId가 반영되어야 합니다. 실제GroupPutResponse스키마와 일치하도록 수정해 주세요(둘 다 제공한다면 둘 다 명시).예시 수정(스키마에 따라 한 가지 선택):
- "imageUrl": "https://cdn.example.com/group/cover_v2.png", + "imageRefId": 1,또는
"maxMember": 30, - "imageUrl": "https://cdn.example.com/group/cover_v2.png", + "imageUrl": "https://cdn.example.com/group/cover_v2.png", + "imageRefId": 1,src/main/java/project/flipnote/image/repository/ImageRefRepositoryCustom.java (1)
8-10: interface 메서드의 public 한정자 제거 및 동작 계약 명시 제안
- interface 메서드는 기본이 public이므로 중복 표기 불필요.
- 정렬/포함 조건(예: id ASC, USING 제외)을 자바독에 명시하면 호출부 이해가 쉬워집니다.
public interface ImageRefRepositoryCustom { - public List<ImageRef> findExpiredPending(Long lastId, LocalDateTime cutoffTime, int batchSize); + List<ImageRef> findExpiredPending(Long lastId, LocalDateTime cutoffTime, int batchSize); }src/main/java/project/flipnote/image/controller/ImageUploadController.java (1)
20-27: 필드명 정합성: fileService → imageService로 교정타입과 일치하도록 필드명을 변경하면 가독성이 개선됩니다.
- private final ImageService fileService; + private final ImageService imageService; ... - ImageUploadResponseDto res = fileService.getPresignedUrl(req.fileName()); + ImageUploadResponseDto res = imageService.getPresignedUrl(req.fileName());또한, DTO가
ReferenceType을 포함한다면 서비스 시그니처가 이를 받는지 확인하고 반영 필요합니다.src/main/java/project/flipnote/image/repository/ImageRepository.java (1)
12-13: 파라미터명 명확화 제안
findByHash(String fileName)는 인자명이 혼동을 줄 수 있습니다. 의미에 맞게hash로 변경을 권장합니다.- Optional<Image> findByHash(String fileName); + Optional<Image> findByHash(String hash);또한 DB에 해시 컬럼 인덱스가 존재하는지 확인해 주세요(조회 경로 상 빈번히 사용될 가능성 큼).
src/main/java/project/flipnote/image/repository/ImageRefRepository.java (2)
11-12: @repository는 생략 가능Spring Data JPA 스캔을 쓰면 @repository 없이도 빈 등록됩니다. 팀 컨벤션에 맞춰 일관성 유지하세요.
15-15: 네스티드 프로퍼티 네이밍 컨벤션 확인
existsByImage_Id는 Spring Data의 중첩 프로퍼티 탐색으로 동작하지만(association.id), 언더스코어 사용을 팀 컨벤션에서 허용하는지 확인 바랍니다.src/main/java/project/flipnote/group/model/GroupDetailResponse.java (1)
22-24: imageRefId 추가는 적절. imageUrl과의 우선순위/이관 계획 명시 필요클라이언트 기준에서 imageRefId vs imageUrl 중 어떤 값을 우선 사용할지 API 스펙에 명확히 해주세요. 향후 imageUrl 제거/Deprecated 계획도 공유 바랍니다.
원하시면 Swagger/문서 업데이트 패치 제안드리겠습니다.
src/main/java/project/flipnote/image/repository/ImageRepositoryCustom.java (1)
10-14: 배치 스캔 시 정렬/경계 정의 명확화 + 메서드 네이밍 제안오프셋 대신 시크 페이징이라면 정렬(예: id ASC)과 경계(
lastId초과 여부)를 계약으로 명확히 해주세요. 메서드 명도 의도를 드러내면 가독성이 좋아집니다.- List<ImageIdKey> findOrphanCandidates(Long lastId, int batchSize); + // lastSeenId 기준 초과(>) id ASC로 batchSize만큼 반환 + List<ImageIdKey> findOrphanCandidatesAfterId(Long lastSeenId, int batchSize);운영 관점: 다중 인스턴스 환경에서 중복 처리 방지를 위해 구현부에서
SKIP LOCKED(지원 DB 한정) 또는 락 토큰을 고려하세요.src/main/java/project/flipnote/user/model/MyInfoResponse.java (2)
26-36: 오타: imageRedId → imageRefId파라미터/전달 변수명이 잘못되어 혼란을 유발합니다. 아래와 같이 정정하세요.
- public static MyInfoResponse from(UserProfile user, Long imageRedId) { + public static MyInfoResponse from(UserProfile user, Long imageRefId) { return new MyInfoResponse( user.getId(), user.getEmail(), user.getNickname(), user.getName(), user.getPhone(), user.isSmsAgree(), user.getProfileImageUrl(), - imageRedId, + imageRefId, user.getCreatedAt(), user.getModifiedAt() ); }
17-18: NULL 출력 억제 권장
imageRefId가 없는 경우 응답에서 필드를 숨기려면@JsonInclude(Include.NON_NULL)적용을 고려하세요.예)
- 타입 레벨:
@JsonInclude(Include.NON_NULL)(레코드 상단, import 필요)- 또는 컴포넌트 레벨:
@JsonInclude(Include.NON_NULL) Long imageRefId,src/main/java/project/flipnote/image/model/ImageUploadRequestDto.java (2)
13-16: type 필드 @NotNull 추가요청 DTO에서 참조 타입은 필수로 보입니다. 널 검증 추가를 권장합니다.
- String fileName, - - ReferenceType type + String fileName, + + @NotNull(message = "참조 타입을 입력해주세요.") + ReferenceType type
8-13: 확장자 대소문자 허용(선택)현재 소문자 확장자만 허용합니다. 업로드 파이프라인에서 대소문자 정규화가 보장되지 않는다면 정규식을 다음처럼 완화하세요.
- regexp = "^[a-fA-F0-9]{32}\\.(jpg|jpeg|png|gif)$", + regexp = "^[a-fA-F0-9]{32}\\.(?i:jpg|jpeg|png|gif)$",확장자 스펙(WebP/HEIC 등) 확대 계획이 있으면 함께 반영해 주세요.
src/main/java/project/flipnote/user/model/UserInfoResponse.java (1)
8-13: URL+참조 ID 병행 노출 의도 확인 및 null 필드 억제 제안imageRefId가 null일 수 있는 응답이라면, 클라이언트 혼동을 줄이기 위해 null 필드 비노출을 권장합니다.
아래와 같이 Jackson 설정을 추가하면 됩니다.
+import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonInclude.Include; -public record UserInfoResponse( +@JsonInclude(Include.NON_NULL) +public record UserInfoResponse( Long userId, String nickname, String profileImageUrl, Long imageRefId ) {src/main/java/project/flipnote/image/repository/ImageRefRepositoryImpl.java (2)
20-20: @OverRide 누락구현 클래스에서 시그니처 이탈을 컴파일 타임에 잡기 위해 @OverRide를 붙이세요.
- public List<ImageRef> findExpiredPending(Long lastId, LocalDateTime cutoffTime, int batchSize) { + @Override + public List<ImageRef> findExpiredPending(Long lastId, LocalDateTime cutoffTime, int batchSize) {
20-31: 쿼리 패턴에 맞는 인덱스 제안
status, createdAt, id접근이 잦다면 복합 인덱스를 고려하세요. 또한 조회/활성화 경로에는(referenceType, referenceId)인덱스가 유용합니다. 엔티티 수준 @table(indexes=...) 또는 마이그레이션 스크립트로 적용 검토 바랍니다.src/main/java/project/flipnote/image/entity/ImageRef.java (2)
51-55: 상태 전이 가드 추가 제안PENDING 외 상태에서 재활성화되는 것을 방지하세요.
public void activateFor(ReferenceType referenceType, Long referenceId) { + if (this.status != ImageStatus.PENDING) { + throw new IllegalStateException("ImageRef can be activated only from PENDING state"); + } this.referenceType = referenceType; this.referenceId = referenceId; this.status = ImageStatus.USING; }
42-44: 낙관적 락 추가 권장동시 상태 변경 충돌을 막기 위해 @Version 필드를 두길 권장합니다.
import jakarta.persistence.Table; +import jakarta.persistence.Version; ... @Column(nullable = false) private ImageStatus status = ImageStatus.PENDING; +@Version +private Long version;src/main/java/project/flipnote/user/service/UserService.java (2)
91-96: 오타: imageRedId → imageRefId가독성/검색성 저하를 초래합니다. 변수명 정정 바랍니다.
- Long imageRedId = imageRef.isPresent() ? imageRef.get().getId() : null; - return MyInfoResponse.from(user, imageRedId); + Long imageRefId = imageRef.isPresent() ? imageRef.get().getId() : null; + return MyInfoResponse.from(user, imageRefId);
101-107: 오타: imageRedId → imageRefId (중복 위치)동일 오타 재발. 함께 수정해 주세요.
- Long imageRedId = imageRef.isPresent() ? imageRef.get().getId() : null; - return UserInfoResponse.from(user, imageRedId); + Long imageRefId = imageRef.isPresent() ? imageRef.get().getId() : null; + return UserInfoResponse.from(user, imageRefId);src/main/java/project/flipnote/image/repository/ImageRepositoryImpl.java (4)
28-44: limit(1)+fetchOne 대신 fetchFirst 사용으로 단순화.불필요한 limit(1) 제거 및 NonUnique 예외 리스크 감소.
.orderBy(imageRef.id.desc()) - .limit(1) - .fetchOne(); + .fetchFirst();
47-66: 고아 후보 조회 시 s3Key null 방어 필요.s3Key가 null인 이미지가 포함되면 S3 삭제 시 NPE/검사 누락이 날 수 있습니다. where 절에 isNotNull을 추가하세요.
.from(image) .where( lastId != null ? image.id.gt(lastId) : null, + image.s3Key.isNotNull(), JPAExpressions.selectOne()
6-9: 미사용 import 정리.
Expression,Expressions는 사용되지 않습니다.-import com.querydsl.core.types.Expression; import com.querydsl.core.types.Projections; -import com.querydsl.core.types.dsl.Expressions; import com.querydsl.jpa.JPAExpressions;
23-41: 조회 성능 인덱스 권고.다음 인덱스를 권장합니다: ImageRef(referenceType, referenceId, status), ImageRef(image_id), Image(id), Image(s3Key).
src/main/java/project/flipnote/group/service/GroupService.java (2)
312-313: REFERENCE_TYPE 상수 일관 사용.하드코딩 대신 상수를 사용해 의미를 통일하세요.
- String url = imageService.changeImage(ReferenceType.GROUP, groupId, imageRefId); + String url = imageService.changeImage(REFERENCE_TYPE, groupId, imageRefId);
301-303: 주석 오탈자 수정(권환 → 권한).가독성 개선.
- //유저 권환 조회 + //유저 권한 조회Also applies to: 371-374
src/main/java/project/flipnote/image/service/ImageCleanService.java (5)
64-112: 장시간 트랜잭션 지양: S3 삭제는 트랜잭션 밖에서 처리하세요.현재 cleanImage 전체가 @transactional로 묶여 있어 S3 I/O 동안 트랜잭션이 유지됩니다. 메서드의 @transactional을 제거하고, DB 삭제는 REQUIRES_NEW로 짧게 분리하세요.
-import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.annotation.Propagation; @@ - @Transactional public void cleanImage() { @@ - // 2) DB 하드 삭제 (짧은 트랜잭션) + // 2) DB 하드 삭제 (짧은 트랜잭션) try { hardDeleteImage(imageId);- @Transactional - protected void hardDeleteImage(Long imageId) { + @Transactional(propagation = Propagation.REQUIRES_NEW) + protected void hardDeleteImage(Long imageId) {
30-32: 미사용 필드(region) 제거.불필요한 설정 주입은 혼란을 줍니다.
- @Value("${cloud.aws.region}") - private String region;
91-95: 로그에 예외 전체 스택을 남기세요.
e.toString()대신 예외 객체를 전달하세요.- } catch (Exception e) { - log.warn("S3 delete failed, keep DB for retry. imageId={}, key={}, err={}", - imageId, s3Key, e.toString()); + } catch (Exception e) { + log.warn("S3 delete failed, keep DB for retry. imageId={}, key={}", imageId, s3Key, e); lastId = imageId; continue; // 다음 항목으로 (DB는 남겨서 재시도) } @@ - } catch (Exception e) { - log.warn("DB delete failed after S3 deletion. imageId={}, err={}", imageId, e.toString()); + } catch (Exception e) { + log.warn("DB delete failed after S3 deletion. imageId={}", imageId, e); }Also applies to: 102-104
70-88: 희귀 레이스에 대한 안전장치 고려.S3 삭제 후 DB 삭제 전에 참조가 생기면 S3 객체는 이미 삭제된 상태로 남습니다. 상태 플래그(예: DELETING)로 선점 후 삭제하거나 S3 버저닝 활성화 검토를 권장합니다.
54-58: 대량 삭제를 배치로 변경 (성능 최적화)ImageRefRepository가 JpaRepository를 상속하므로 개별 delete 반복 대신 deleteAllInBatch(refs)를 사용하세요. 엔티티 라이프사이클 콜백/연관삭제가 필요하면 deleteAll(refs)를 사용하십시오.
- refs.forEach(imageRefRepository::delete); + imageRefRepository.deleteAllInBatch(refs);src/main/java/project/flipnote/image/service/ImageRefService.java (2)
31-39: 상태 전이(activate)는 트랜잭션으로 보장하세요.동시성·일관성 보장을 위해 @transactional 추가를 권장합니다.
public void imageActivate(Long imageRefId, ReferenceType type, Long referenceId) { + @org.springframework.transaction.annotation.Transactional + public void imageActivate(Long imageRefId, ReferenceType type, Long referenceId) {
45-51: 삭제 오퍼레이션도 트랜잭션으로 래핑.부분 실패 방지를 위해 트랜잭션 적용을 권장합니다.
- public void deleteByReferenceAndId(ReferenceType type, Long id) { + @org.springframework.transaction.annotation.Transactional + public void deleteByReferenceAndId(ReferenceType type, Long id) {src/main/java/project/flipnote/image/service/ImageService.java (4)
157-157: URL 구성 방식 개선 필요S3 URL을 문자열 연결로 직접 구성하는 것은 오류가 발생하기 쉽습니다. AWS SDK의 내장 메서드나
UriComponentsBuilder를 사용하는 것이 더 안전합니다.다음과 같이 개선할 수 있습니다:
- URL url = new URL("https://" + bucket + ".s3." + region + ".amazonaws.com/" + key); + String urlString = String.format("https://%s.s3.%s.amazonaws.com/%s", + bucket, region, key.replace(" ", "+")); + URL url = new URL(urlString);또는 Spring의
UriComponentsBuilder를 사용하세요:String urlString = UriComponentsBuilder.newInstance() .scheme("https") .host(bucket + ".s3." + region + ".amazonaws.com") .path(key) .build() .toUriString();
137-142: S3 호출 시 예외 처리 필요S3
headObject호출 시 네트워크 오류나 객체가 존재하지 않는 경우에 대한 예외 처리가 없습니다.다음과 같이 예외 처리를 추가하세요:
+ try { HeadObjectResponse headResponse = s3Client.headObject( HeadObjectRequest.builder() .bucket(bucket) .key(image.getS3Key()) .build() ); + } catch (NoSuchKeyException e) { + log.error("S3 object not found: {}", image.getS3Key()); + throw new BizException(ImageErrorCode.IMAGE_NOT_FOUND); + } catch (S3Exception e) { + log.error("S3 error while fetching metadata: {}", e.getMessage()); + throw new BizException(ImageErrorCode.S3_ERROR); + }
71-71: 불필요한 로깅 제거 권장해시 값을 단순히 로깅하는 것은 프로덕션 환경에서 불필요한 로그를 생성합니다. 더 의미 있는 컨텍스트와 함께 로깅하거나 제거하는 것이 좋습니다.
- log.info(hash); + log.debug("Processing image upload with hash: {}", hash);
158-158: 불필요한 공백 처리Line 158에서
return url;앞에 불필요한 공백이 있습니다.- return url; + return url;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
src/main/java/project/flipnote/auth/model/UserRegisterRequest.java(1 hunks)src/main/java/project/flipnote/common/config/SchedulerConfig.java(1 hunks)src/main/java/project/flipnote/common/model/request/UserCreateCommand.java(1 hunks)src/main/java/project/flipnote/group/controller/docs/GroupControllerDocs.java(2 hunks)src/main/java/project/flipnote/group/entity/Group.java(1 hunks)src/main/java/project/flipnote/group/model/GroupCreateRequest.java(1 hunks)src/main/java/project/flipnote/group/model/GroupDetailResponse.java(1 hunks)src/main/java/project/flipnote/group/model/GroupInfo.java(1 hunks)src/main/java/project/flipnote/group/model/GroupPutRequest.java(1 hunks)src/main/java/project/flipnote/group/repository/GroupRepositoryImpl.java(4 hunks)src/main/java/project/flipnote/group/service/GroupPolicyService.java(2 hunks)src/main/java/project/flipnote/group/service/GroupService.java(9 hunks)src/main/java/project/flipnote/image/controller/ImageUploadController.java(1 hunks)src/main/java/project/flipnote/image/entity/Image.java(1 hunks)src/main/java/project/flipnote/image/entity/ImageRef.java(1 hunks)src/main/java/project/flipnote/image/entity/ImageStatus.java(1 hunks)src/main/java/project/flipnote/image/entity/ReferenceType.java(1 hunks)src/main/java/project/flipnote/image/exception/ImageErrorCode.java(1 hunks)src/main/java/project/flipnote/image/model/ImageIdKey.java(1 hunks)src/main/java/project/flipnote/image/model/ImageUploadRequestDto.java(1 hunks)src/main/java/project/flipnote/image/model/ImageUploadResponseDto.java(1 hunks)src/main/java/project/flipnote/image/repository/ImageRefRepository.java(1 hunks)src/main/java/project/flipnote/image/repository/ImageRefRepositoryCustom.java(1 hunks)src/main/java/project/flipnote/image/repository/ImageRefRepositoryImpl.java(1 hunks)src/main/java/project/flipnote/image/repository/ImageRepository.java(1 hunks)src/main/java/project/flipnote/image/repository/ImageRepositoryCustom.java(1 hunks)src/main/java/project/flipnote/image/repository/ImageRepositoryImpl.java(1 hunks)src/main/java/project/flipnote/image/service/ImageCleanService.java(1 hunks)src/main/java/project/flipnote/image/service/ImageRefService.java(1 hunks)src/main/java/project/flipnote/image/service/ImageService.java(1 hunks)src/main/java/project/flipnote/image/service/ImageUploadService.java(0 hunks)src/main/java/project/flipnote/user/entity/UserProfile.java(3 hunks)src/main/java/project/flipnote/user/model/MyInfoResponse.java(3 hunks)src/main/java/project/flipnote/user/model/UserInfoResponse.java(1 hunks)src/main/java/project/flipnote/user/model/UserUpdateRequest.java(1 hunks)src/main/java/project/flipnote/user/model/UserUpdateResponse.java(1 hunks)src/main/java/project/flipnote/user/service/UserService.java(4 hunks)src/main/resources/application.yml(1 hunks)src/test/java/project/flipnote/group/service/GroupPolicyServiceTest.java(1 hunks)src/test/java/project/flipnote/group/service/GroupServiceTest.java(1 hunks)src/test/java/project/flipnote/image/service/ImageServiceTest.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/project/flipnote/image/service/ImageUploadService.java
🧰 Additional context used
🧬 Code graph analysis (9)
src/main/java/project/flipnote/image/service/ImageRefService.java (1)
src/main/java/project/flipnote/user/service/UserService.java (1)
RequiredArgsConstructor(31-153)
src/main/java/project/flipnote/image/repository/ImageRefRepositoryImpl.java (2)
src/main/java/project/flipnote/common/config/SchedulerConfig.java (1)
RequiredArgsConstructor(12-33)src/main/java/project/flipnote/image/repository/ImageRepositoryImpl.java (1)
RequiredArgsConstructor(20-67)
src/main/java/project/flipnote/image/repository/ImageRepositoryImpl.java (2)
src/main/java/project/flipnote/common/config/SchedulerConfig.java (1)
RequiredArgsConstructor(12-33)src/main/java/project/flipnote/image/repository/ImageRefRepositoryImpl.java (1)
RequiredArgsConstructor(13-32)
src/main/java/project/flipnote/image/controller/ImageUploadController.java (2)
src/main/java/project/flipnote/user/service/UserService.java (1)
RequiredArgsConstructor(31-153)src/main/java/project/flipnote/group/controller/GroupController.java (1)
RequiredArgsConstructor(32-109)
src/main/java/project/flipnote/image/service/ImageService.java (3)
src/main/java/project/flipnote/image/service/ImageRefService.java (1)
Service(14-52)src/main/java/project/flipnote/user/service/UserService.java (1)
RequiredArgsConstructor(31-153)src/main/java/project/flipnote/group/service/GroupService.java (1)
Slf4j(52-482)
src/main/java/project/flipnote/image/service/ImageCleanService.java (3)
src/main/java/project/flipnote/common/config/SchedulerConfig.java (1)
RequiredArgsConstructor(12-33)src/main/java/project/flipnote/image/repository/ImageRefRepositoryImpl.java (1)
RequiredArgsConstructor(13-32)src/main/java/project/flipnote/image/repository/ImageRepositoryImpl.java (1)
RequiredArgsConstructor(20-67)
src/main/java/project/flipnote/image/entity/ImageRef.java (1)
src/main/java/project/flipnote/image/entity/Image.java (1)
Getter(23-58)
src/main/java/project/flipnote/image/entity/Image.java (1)
src/main/java/project/flipnote/image/entity/ImageRef.java (1)
Getter(23-60)
src/main/java/project/flipnote/common/config/SchedulerConfig.java (1)
src/main/java/project/flipnote/common/config/ShedLockConfig.java (1)
EnableSchedulerLock(11-19)
⏰ 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 (16)
src/main/java/project/flipnote/image/entity/ReferenceType.java (1)
3-5: 확인됨 — ReferenceType 필드가 EnumType.STRING으로 매핑됩니다.
ImageRef 엔티티에 @Enumerated(EnumType.STRING)가 ReferenceType 필드 바로 위에 선언되어 있습니다 (src/main/java/project/flipnote/image/entity/ImageRef.java:32-33).src/main/java/project/flipnote/common/model/request/UserCreateCommand.java (1)
3-9: 승인 — 프로필 이미지 URL 제거 반영 확인레코드 축소가 의도와 일치하며, 호출부는 src/main/java/project/flipnote/auth/model/UserRegisterRequest.java의 toCommand() 하나뿐이고 new UserCreateCommand(...) 호출에 인자 변경이 반영되어 있습니다.
src/main/java/project/flipnote/image/entity/Image.java (1)
23-27: 소프트 삭제 정책 일관성 확인 요청동일 프로젝트의 Group 엔티티는 @SQLDelete/@SQLRestriction을 사용합니다. Image에도 소프트 삭제가 필요하다면 동일 정책 적용 또는 별도 클린업 로직으로 일관성 맞춰 주세요.
src/main/java/project/flipnote/image/exception/ImageErrorCode.java (1)
13-16: 에러 코드 확장 LGTMHTTP 상태/코드/메시지 매핑 일관적입니다. 상수 명명도 명확합니다.
src/main/java/project/flipnote/image/entity/ImageStatus.java (1)
3-5: 상태 enum 추가 LGTM단순·명확하며 ImageRef와의 일관성에 부합합니다.
src/main/java/project/flipnote/group/controller/docs/GroupControllerDocs.java (2)
62-62: 요청 바디의 imageRefId 전환 반영 👍생성 API 예시에 imageRefId 적용이 명확합니다.
114-115: 요청 바디의 imageRefId 전환 반영 👍수정 API 예시에 imageRefId 적용이 일관됩니다.
src/test/java/project/flipnote/image/service/ImageServiceTest.java (1)
16-35: 테스트 비활성화·presigner 모킹 누락 — 확인 필요
- 주요 테스트가 주석 처리/비활성화되어 회귀 감지 불가합니다. S3 SDK v2를 사용한다면 S3Presigner 기반 presigned URL 생성 로직을 모킹하고 성공/중복 케이스(각 최소 1개)를 복구하세요.
- 위치: src/test/java/project/flipnote/image/service/ImageServiceTest.java (라인 16–64). 저장소 검사 중 일부 파일 출력에 실패해 수동으로 내용·모킹 여부를 확인해 주세요.
src/main/java/project/flipnote/group/model/GroupDetailResponse.java (1)
30-39: 팩토리 시그니처 변경에 따른 호출부 점검 — 완료리포지터리 검색 결과 호출부는 src/main/java/project/flipnote/group/service/GroupService.java 356행의 GroupDetailResponse.from(group, imageRefId) 단일 호출뿐이며 imageRefId(Long)를 정상 전달하고 있습니다. 수정 불필요.
src/main/java/project/flipnote/group/service/GroupPolicyService.java (1)
41-43: 시그니처 확장(URL 전달) 방향 LGTM도메인 로직에서 URL을 명시적으로 주입하는게 imageRef 흐름과 잘 맞습니다. 그룹 변경은 트랜잭션 내 엔티티 변경으로 커밋 시 반영되어 추가 save 호출도 불필요합니다.
src/main/java/project/flipnote/user/service/UserService.java (2)
65-69: 회원 탈퇴 시 참조 정리 순서 LGTM이미지 참조를 먼저 정리 후 탈퇴 처리하는 순서 적절합니다.
81-84: 해결: changeImage(null) 동작은 '기존 참조 삭제 + 기본 URL 반환'으로 정의되어 있음
ImageService.changeImage(type, referenceId, imageRefId)는 imageRefId == null일 경우 기존 ImageRef가 있으면 삭제(imageRefService.delete(...))하고 getDefaultUrl(type)을 반환합니다 (src/main/java/project/flipnote/image/service/ImageService.java:184-193). UserService.update에서 추가 분기 불필요.src/main/java/project/flipnote/user/model/UserInfoResponse.java (1)
12-13: from(...) 시그니처 변경 — 호출부 전수 점검 완료
전역 검색 결과 호출은 한 곳(src/main/java/project/flipnote/user/service/UserService.java:106)뿐이며, 이미 두 인자 형태로 호출되고 있습니다.src/main/java/project/flipnote/group/model/GroupInfo.java (1)
10-14: API 계약(문서/스키마) 동기화 확인 — imageRefId 반영됨
GroupCreateRequest, GroupPutRequest, GroupInfo, GroupDetailResponse 및 src/main/java/project/flipnote/group/controller/docs/GroupControllerDocs.java 등에서 imageRefId가 이미 추가되어 있어 문서/스키마 누락으로 인한 역직렬화 오류 우려는 없습니다.src/main/java/project/flipnote/image/service/ImageRefService.java (1)
33-35: 예외 코드 의미 정합성 확인.ImageRef 미존재에
IMAGE_NOT_FOUND사용이 맞는지 확인하세요(별도 코드가 있다면 교체).src/main/java/project/flipnote/group/service/GroupService.java (1)
195-196: 확인 완료 — 메서드 존재 및 시그니처 일치
ImageService.changeUrlStatus(Long, ReferenceType, Long)가 존재하며 GroupService의 호출(req.imageRefId(), REFERENCE_TYPE, group.getId())과 시그니처가 일치합니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
src/main/java/project/flipnote/common/config/SchedulerConfig.java (1)
7-8: 중복 ShedLock 선언 제거 확인 — LGTM이전에 지적되었던 클래스 수준의
@EnableSchedulerLock중복 선언이 제거되었습니다. 중앙 설정(ShedLockConfig)만 유지하고, 메서드 단위@SchedulerLock으로 세분화한 방향 좋습니다.src/main/java/project/flipnote/user/service/UserService.java (1)
85-92: 응답의 imageRefId 신뢰성 문제 재발견업데이트 응답에서
imageRefService.findByTypeAndReferenceId로 실제 적용된 이미지 참조를 조회하는 코드가 추가되었으나, 이는 이미 과거 리뷰에서 지적된 사항입니다.src/main/java/project/flipnote/image/service/ImageService.java (4)
54-54: 파일 확장자가 없는 경우 처리 필요파일명에 확장자가 없거나 마지막 점(
.) 이후에 문자가 없는 경우StringIndexOutOfBoundsException이 발생합니다.
69-69: 해시 생성 시 확장자가 없는 파일명 처리 필요파일명에 점(
.)이 없는 경우ArrayIndexOutOfBoundsException이 발생할 수 있습니다. 또한 현재 구현은 파일명 자체를 해시로 사용하고 있어 실제 파일 내용의 무결성을 보장하지 못합니다.
66-118: 동시성 경합 처리 필요 — DB 유니크 제약은 존재함동일한 해시를 가진 파일이 동시에 업로드될 경우, 두 요청 모두 Line 74에서 이미지를 찾지 못하고 Line 109에서 중복된
Image엔티티 생성을 시도할 수 있습니다. DB의 유니크 제약으로 하나는 실패하지만 전체 트랜잭션이 롤백됩니다.
120-152: 트랜잭션 경계 누락이 메서드는 여러 데이터베이스 작업(
imageRefService.imageActivate,imageRepository.save)을 수행하지만@Transactional어노테이션이 없어 원자성이 보장되지 않습니다.
🧹 Nitpick comments (8)
src/main/java/project/flipnote/common/config/SchedulerConfig.java (1)
22-22: 주석으로 남긴 대체 크론 제거 또는 프로파일로 분리주석 코드는 장기적으로 부채가 됩니다. 로컬/스테이징 전용 크론은 프로파일 또는 프로퍼티로 관리하세요. 아래처럼 주석 줄은 삭제하는 것을 권장합니다.
- // @Scheduled(cron = "0 * * * * *", zone = "Asia/Seoul") @@ - // @Scheduled(cron = "30 */2 * * * *", zone = "Asia/Seoul")Also applies to: 31-31
src/main/java/project/flipnote/user/service/UserService.java (4)
100-100: 변수명 오타: imageRedId → imageRefId변수명에 오타가 있습니다.
imageRedId를imageRefId로 수정해야 일관성이 유지됩니다.- Long imageRedId = imageRef.isPresent() ? imageRef.get().getId() : null; + Long imageRefId = imageRef.isPresent() ? imageRef.get().getId() : null;Line 102에서도 이 변수를 사용하므로 함께 수정이 필요합니다:
- return MyInfoResponse.from(user, imageRedId); + return MyInfoResponse.from(user, imageRefId);
110-110: 변수명 오타: imageRedId → imageRefIdLine 100과 동일한 변수명 오타가 있습니다.
- Long imageRedId = imageRef.isPresent() ? imageRef.get().getId() : null; + Long imageRefId = imageRef.isPresent() ? imageRef.get().getId() : null;Line 113에서도 이 변수를 사용하므로 함께 수정이 필요합니다:
- return UserInfoResponse.from(user, imageRedId); + return UserInfoResponse.from(user, imageRefId);
98-102: Optional 처리 개선 가능
Optional의map메서드를 활용하면 더 간결하게 작성할 수 있습니다.- Optional<ImageRef> imageRef = imageRefService.findByTypeAndReferenceId(type, userId); - - Long imageRedId = imageRef.isPresent() ? imageRef.get().getId() : null; - - return MyInfoResponse.from(user, imageRedId); + Long imageRefId = imageRefService.findByTypeAndReferenceId(type, userId) + .map(ImageRef::getId) + .orElse(null); + + return MyInfoResponse.from(user, imageRefId);
108-113: Optional 처리 개선 가능
getUserInfo메서드에서도 동일하게Optional의map메서드를 활용할 수 있습니다.- Optional<ImageRef> imageRef = imageRefService.findByTypeAndReferenceId(type, userId); - - Long imageRedId = imageRef.isPresent() ? imageRef.get().getId() : null; - - - return UserInfoResponse.from(user, imageRedId); + Long imageRefId = imageRefService.findByTypeAndReferenceId(type, userId) + .map(ImageRef::getId) + .orElse(null); + + return UserInfoResponse.from(user, imageRefId);src/main/java/project/flipnote/image/service/ImageService.java (3)
189-189: Null 비교 시 코딩 스타일 개선
imageRefId==null대신imageRefId == null로 공백을 추가하면 가독성이 향상됩니다.- if(imageRefId==null) { + if(imageRefId == null) {
190-191: 공백 처리 일관성 개선
if문 뒤의 괄호 사이에 공백을 추가하면 코드 스타일이 일관됩니다.- if(imageRef.isPresent()) { + if (imageRef.isPresent()) {
196-196: 공백 처리 일관성 개선Line 190과 동일하게
if문 뒤의 괄호 사이에 공백을 추가해야 합니다.- if(imageRef.isPresent()) { + if (imageRef.isPresent()) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/project/flipnote/common/config/SchedulerConfig.java(1 hunks)src/main/java/project/flipnote/image/entity/Image.java(1 hunks)src/main/java/project/flipnote/image/entity/ImageRef.java(1 hunks)src/main/java/project/flipnote/image/service/ImageService.java(1 hunks)src/main/java/project/flipnote/user/entity/UserProfile.java(1 hunks)src/main/java/project/flipnote/user/service/UserService.java(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/project/flipnote/user/entity/UserProfile.java
- src/main/java/project/flipnote/image/entity/ImageRef.java
- src/main/java/project/flipnote/image/entity/Image.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/project/flipnote/image/service/ImageService.java (1)
src/main/java/project/flipnote/image/service/ImageRefService.java (1)
Service(14-52)
src/main/java/project/flipnote/common/config/SchedulerConfig.java (3)
src/main/java/project/flipnote/common/config/ShedLockConfig.java (1)
EnableSchedulerLock(11-19)src/main/java/project/flipnote/image/repository/ImageRefRepositoryImpl.java (1)
RequiredArgsConstructor(13-32)src/main/java/project/flipnote/image/repository/ImageRepositoryImpl.java (1)
RequiredArgsConstructor(20-67)
⏰ 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 (2)
src/main/java/project/flipnote/common/config/SchedulerConfig.java (2)
13-18: 생성자 주입(@requiredargsconstructor)과 final 필드 사용 — LGTM스케줄러가 의존하는
ImageCleanService를 불변 필드로 주입한 선택 적절합니다.
20-34: Cron/락 TTL 프로퍼티화 및 최소 락 추가 제안운영/테스트 환경별로 스케줄을 조정하기 쉽도록 크론/타임존/락 시간을 프로퍼티로 외부화하고, 재진입 방지를 위해
lockAtLeastFor를 추가하는 것을 권장합니다. 또한 2분 TTL이 실제 최대 실행 시간보다 짧으면 중복 실행 위험이 있습니다. 한 번의 배치 최악 시간에 맞춰 상향/프로퍼티화해 주세요.[ suggest_optional_refactor ][ request_verification ][ offer_operational_advice ]
적용 예시(diff):
- @Scheduled(cron = "0 0 23 * * *", zone = "Asia/Seoul") - // @Scheduled(cron = "0 * * * * *", zone = "Asia/Seoul") - @SchedulerLock(name = "image.cleanImageRef", lockAtMostFor = "PT2M") + @Scheduled(cron = "${jobs.image.clean-ref.cron:0 0 23 * * *}", zone = "${app.timezone:Asia/Seoul}") + @SchedulerLock( + name = "image.cleanImageRef", + lockAtMostFor = "${jobs.image.clean-ref.lockAtMostFor:PT5M}", + lockAtLeastFor = "${jobs.image.clean-ref.lockAtLeastFor:PT10S}" + ) @@ - @Scheduled(cron = "0 30 23 * * 0", zone = "Asia/Seoul") - @SchedulerLock(name = "image.cleanImage", lockAtMostFor = "PT2M") - // @Scheduled(cron = "30 */2 * * * *", zone = "Asia/Seoul") + @Scheduled(cron = "${jobs.image.clean.cron:0 30 23 * * 0}", zone = "${app.timezone:Asia/Seoul}") + @SchedulerLock( + name = "image.cleanImage", + lockAtMostFor = "${jobs.image.clean.lockAtMostFor:PT10M}", + lockAtLeastFor = "${jobs.image.clean.lockAtLeastFor:PT10S}" + )프로퍼티 샘플:
app.timezone: Asia/Seoul jobs: image: clean-ref: cron: 0 0 23 * * * lockAtMostFor: PT5M lockAtLeastFor: PT10S clean: cron: 0 30 23 * * 0 lockAtMostFor: PT10M lockAtLeastFor: PT10S운영 팁:
- 장시간 실행 가능성이 있으면
spring.task.scheduling.pool.size(예: 2~4)로 스케줄러 풀을 확장해 병행 실행 대기 시간을 줄이세요.
2 participants
📝 변경 내용
✅ 체크리스트
💬 기타 참고 사항
Summary by CodeRabbit
New Features
Documentation
Chores