[OT-104][FEAT]: 업로드 API 구현#64
Merged
arlen02-01 merged 19 commits intodevelopfrom Feb 25, 2026
Merged
Conversation
arlen02-01
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough모놀리식 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as BackOfficeController
participant Service as BackOfficeService
participant Repo as Repository
participant S3 as S3PresignService
Client->>Controller: POST /upload (UploadRequest)
Controller->>Service: create*Upload(request)
Service->>Repo: resolveUploader / resolveSeries/resolveContents
Service->>Repo: save Media (placeholder URLs)
Service->>Repo: save Contents/Series/ShortForm (placeholder URLs)
Service->>S3: createPutPresignedUrl(objectKey, contentType)
S3-->>Service: presigned PUT URL
Service->>S3: toObjectUrl(objectKey)
S3-->>Service: public object URL
Service->>Repo: update entities with object keys / URLs
Service-->>Controller: UploadResponse (IDs, objectKeys, presignUrls)
Controller-->>Client: 200 OK (SuccessResponse<UploadResponse>)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker-compose.yml (1)
88-96:⚠️ Potential issue | 🟠 Major
transcoder서비스에 AWS 환경 변수 누락트랜스코더 워커는 트랜스코딩 결과물(
master.m3u8등)을 S3에 저장하는 것이 핵심 역할입니다. 그러나transcoder서비스에는 AWS 관련 환경 변수(AWS_REGION,AWS_S3_BUCKET,AWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY)가 전혀 설정되어 있지 않아, 컨테이너 실행 시 S3 연동이 실패합니다.🔧 제안 수정
environment: SPRING_DATASOURCE_URL: ${SPRING_DATASOURCE_URL} SPRING_DATASOURCE_USERNAME: ${SPRING_DATASOURCE_USERNAME} SPRING_DATASOURCE_PASSWORD: ${SPRING_DATASOURCE_PASSWORD} + AWS_REGION: ${AWS_REGION} + AWS_S3_BUCKET: ${AWS_S3_BUCKET} + AWS_ACCESS_KEY_ID: ${AWS_ACCESS_KEY_ID} + AWS_SECRET_ACCESS_KEY: ${AWS_SECRET_ACCESS_KEY}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 88 - 96, The transcoder service is missing required AWS environment variables so S3 uploads will fail; update the docker-compose service definition for the transcoder service to add SPRING_DATASOURCE_* are already present but ensure you add AWS_REGION, AWS_S3_BUCKET, AWS_ACCESS_KEY_ID, and AWS_SECRET_ACCESS_KEY as environment entries (or reference existing env-file/compose variables like ${AWS_REGION}, ${AWS_S3_BUCKET}, ${AWS_ACCESS_KEY_ID}, ${AWS_SECRET_ACCESS_KEY}) so the transcoder container can authenticate and write outputs (look for the service named "transcoder" and modify its environment block to include these variable names).
🧹 Nitpick comments (11)
apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java (1)
9-31: 검증 메시지 및 필드 수준@Schema누락
SeriesUploadRequest와 동일하게, 기존 코드베이스 패턴과의 일관성을 위해@NotBlank/@NotNull에 메시지를 추가하고, 각 필드에@Schema(description = "...", example = "...")을 추가하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java` around lines 9 - 31, ShortFormUploadRequest is missing validation messages and field-level `@Schema` metadata; update the record component annotations to include message attributes on `@NotBlank/`@NotNull (e.g. give descriptive messages for title, description, publicStatus, posterFileName, thumbnailFileName, originFileName) and add `@Schema`(description="...", example="...") for each field (seriesId, contentsId, title, description, publicStatus, duration, videoSize, posterFileName, thumbnailFileName, originFileName) following the pattern used in SeriesUploadRequest; ensure you import and use io.swagger.v3.oas.annotations.media.Schema and include meaningful example values and descriptions consistent with existing DTOs.docker-compose.yml (1)
56-78:api-admin헬스체크 미정의현재는 어떤 서비스도
api-admin의 상태에 의존하지 않으므로 즉각적인 장애는 없지만, 향후 의존 서비스 추가 시 헬스체크가 없으면 준비되지 않은 상태에서 요청을 받을 수 있습니다.api-user와 동일한 패턴으로 추가를 권장합니다.🔧 제안 수정
depends_on: mysql: condition: service_healthy api-user: condition: service_healthy + healthcheck: + test: ["CMD-SHELL", "curl -f http://localhost:8081/actuator/health || exit 1"] + interval: 10s + timeout: 5s + retries: 12 + start_period: 30s🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 56 - 78, Add a healthcheck to the api-admin service so other services can safely depend on it; mirror the same pattern used by api-user by adding a healthcheck section under the api-admin service (use the same test command, interval, timeout, retries, and start_period values as api-user) and ensure depends_on in other services can reference api-admin with condition: service_healthy once added.apps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.java (1)
9-22: 검증 메시지 및 필드 수준@Schema누락기존 코드베이스 패턴과 일관성이 떨어집니다.
- 검증 메시지 누락:
ChangeRoleRequest(@NotNull(message = "변경할 역할은 필수입니다."))와 달리@NotBlank/@NotNull에message속성이 없어 클라이언트가 어떤 필드가 누락되었는지 파악하기 어렵습니다.- 필드 수준
@Schema누락:ContentsListResponse처럼 각 필드에@Schema(description = "...", example = "...")어노테이션이 없어 Swagger UI에서 필드 설명이 표시되지 않습니다.♻️ 제안 수정 예시
public record SeriesUploadRequest( - `@NotBlank` String title, - `@NotBlank` String description, - `@NotBlank` String actors, - `@NotNull` PublicStatus publicStatus, - `@NotBlank` String posterFileName, - `@NotBlank` String thumbnailFileName + `@NotBlank`(message = "시리즈 제목은 필수입니다.") + `@Schema`(description = "시리즈 제목", example = "Series A") + String title, + `@NotBlank`(message = "시리즈 설명은 필수입니다.") + `@Schema`(description = "시리즈 설명", example = "series desc") + String description, + `@NotBlank`(message = "출연진 정보는 필수입니다.") + `@Schema`(description = "출연진 문자열", example = "Actor1, Actor2") + String actors, + `@NotNull`(message = "공개 상태는 필수입니다.") + `@Schema`(description = "공개 상태", example = "PUBLIC") + PublicStatus publicStatus, + `@NotBlank`(message = "포스터 파일명은 필수입니다.") + `@Schema`(description = "포스터 원본 파일명", example = "poster.jpg") + String posterFileName, + `@NotBlank`(message = "썸네일 파일명은 필수입니다.") + `@Schema`(description = "썸네일 원본 파일명", example = "thumb.jpg") + String thumbnailFileName ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.java` around lines 9 - 22, Update the SeriesUploadRequest record to add validation messages and field-level Swagger metadata: for each field in SeriesUploadRequest (title, description, actors, publicStatus, posterFileName, thumbnailFileName) add a message on the existing `@NotBlank/`@NotNull annotations (e.g., message = "... is required") and annotate each parameter with `@Schema`(description = "...", example = "...") to match the project's pattern (provide meaningful descriptions and examples similar to ContentsListResponse); ensure the annotation imports are present and use the exact field names above so the validators and generated API docs display clear messages and field descriptions.apps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.java (3)
158-170:resolveContentType에서 지원하지 않는 확장자에 대한 에러 메시지가 구체적이지 않습니다.
INVALID_INPUT이라는 일반적인 에러 코드만 던지고 있어, 클라이언트가 어떤 확장자가 지원되는지 알기 어렵습니다. 에러 메시지에 지원 확장자 목록을 포함하면 디버깅과 사용자 경험이 개선됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.java` around lines 158 - 170, The resolveContentType method currently throws a generic BusinessException(ErrorCode.INVALID_INPUT) for unsupported file extensions; change it to include a clear message naming the supported extensions (e.g., ".jpg, .jpeg, .png, .webp") and optionally the provided fileName to aid debugging — update the throw in resolveContentType to BusinessException(ErrorCode.INVALID_INPUT, "Unsupported extension: " + fileName + ". Supported: .jpg, .jpeg, .png, .webp") or the project’s BusinessException constructor signature so clients receive a specific, actionable error.
110-152: 업로드 실패 시 고아 레코드(orphan record) 관리 방안을 고려하세요.트랜잭션이 성공적으로 커밋된 후 클라이언트가 presigned URL로 실제 파일을 업로드하지 않으면,
PENDINGURL을 가진 Media/Series 레코드가 DB에 남게 됩니다. 현재 구조에서는 이를 감지하거나 정리하는 메커니즘이 보이지 않습니다.향후 운영 안정성을 위해 다음을 검토하시면 좋겠습니다:
- 일정 시간 경과 후
PENDING상태 레코드를 정리하는 스케줄러- 또는 클라이언트의 업로드 완료 확인 콜백 API
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.java` around lines 110 - 152, createSeriesUpload currently persists Media/Series with poster/thumbnail set to "PENDING" and returns presigned URLs, which can leave orphan records if the client never uploads; add a remediation strategy: either (A) introduce a completion endpoint (e.g., markUploadComplete(seriesId, posterKey, thumbnailKey) called by client after successful PUT that updates Media.posterUrl/thumbnailUrl from "PENDING" to final URLs and validates owner via resolveUploader()), or (B) implement a scheduled cleaner that finds Media records with posterUrl/thumbnailUrl == "PENDING" older than a configurable threshold and deletes associated Series and Media rows (use seriesRepository and mediaRepository, and reference object keys created by buildObjectKey and s3PresignService.createPutPresignedUrl). Ensure transactions around the completion update or deletion so no partial state remains and log actions for audit.
193-213: 네 개의 헬퍼 메서드가 세 개의 서비스에서 중복됩니다.
resolveUploader(),sanitizeFileName(),resolveContentType(),buildObjectKey()가BackOfficeSeriesService,BackOfficeContentsService,BackOfficeShortFormService세 곳 모두에서 동일하게 정의되어 있습니다. 공통 유틸리티 클래스나 공유 서비스로 추출하면 DRY 원칙을 준수하고 향후 유지보수가 용이해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.java` around lines 193 - 213, Extract the four duplicated helpers into shared components and replace the copies in BackOfficeSeriesService, BackOfficeContentsService, and BackOfficeShortFormService: move resolveUploader() into a new bean/class (e.g., AuthenticatedMemberResolver) that injects MemberRepository and uses SecurityContextHolder to return Member or throw BusinessException; move sanitizeFileName(), resolveContentType(), and buildObjectKey() into a shared utility/service (e.g., FileNameUtils or StorageKeyGenerator) as either static helpers or a Spring bean depending on DI needs; update the three backoffice services to call AuthenticatedMemberResolver.resolveUploader() and FileNameUtils.sanitizeFileName()/resolveContentType()/buildObjectKey(), remove the duplicated method definitions, and add necessary imports/constructor injection where applicable.apps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsController.java (1)
15-15: 와일드카드 import가 다른 컨트롤러와 일관성이 없습니다.
BackOfficeSeriesController와BackOfficeShortFormController는 명시적 import를 사용하는 반면, 이 파일은org.springframework.web.bind.annotation.*와일드카드를 사용하고 있습니다.♻️ 명시적 import로 변경 제안
-import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsController.java` at line 15, BackOfficeContentsController currently uses a wildcard import (org.springframework.web.bind.annotation.*); change it to explicit imports to match the other controllers (e.g., import specific annotations used in this class such as RestController, RequestMapping, GetMapping, PostMapping, PutMapping, DeleteMapping, PathVariable, RequestBody, RequestParam, etc.) so the class BackOfficeContentsController has the same explicit import style as BackOfficeSeriesController and BackOfficeShortFormController; update the import statements at the top of the file to list only the annotations actually referenced by the class.modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java (2)
57-67:toObjectUrl의 URL 인코딩 로직이 정확합니다만,%2F복원 시 주의점이 있습니다.
URLEncoder.encode로 전체 키를 인코딩한 뒤%2F를/로 복원하는 패턴은 일반적으로 잘 동작합니다. 다만, 이 메서드가 이미 인코딩된 키를 받는 경우 이중 인코딩 문제가 발생할 수 있으므로, 호출부에서 항상 raw objectKey를 전달하는지 확인이 필요합니다.또한,
publicBaseUrl의 기본값이@Value에서 빈 문자열("")로 설정되어 있어 실제로null이 될 가능성은 낮지만,null체크를 포함한 방어적 코딩은 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java` around lines 57 - 67, The toObjectUrl method can double-encode an already-percent-encoded objectKey; update to normalize input in S3PresignService.toObjectUrl by detecting whether objectKey already contains percent-encoding (e.g., a '%' followed by two hex digits) and, if so, use it as-is (or first URL-decode then re-encode safely), otherwise perform the URLEncoder.encode(...) + replace("+","%20"). Keep the defensive null/blank handling for publicBaseUrl as-is but ensure publicBaseUrl trimming (replaceAll("/+$","")) remains applied; this prevents double-encoding while preserving current base URL fallback behavior.
23-35:expireSeconds의 유효성 검증이 없습니다.설정값이 0 이하인 경우
Duration.ofSeconds()가 의미 없는 결과를 만들 수 있습니다. 생성자에서 최소값 검증을 추가하면 설정 오류를 빠르게 감지할 수 있습니다.♻️ 생성자에서 유효성 검증 추가 제안
public S3PresignService( S3Presigner s3Presigner, `@Value`("${aws.region:ap-northeast-2}") String region, `@Value`("${aws.s3.bucket:local-bucket}") String bucket, `@Value`("${aws.s3.public-base-url:}") String publicBaseUrl, `@Value`("${aws.s3.presign-expire-seconds:600}") long expireSeconds ) { + if (expireSeconds <= 0) { + throw new IllegalArgumentException("presign-expire-seconds must be positive"); + } this.s3Presigner = s3Presigner; this.region = region; this.bucket = bucket; this.publicBaseUrl = publicBaseUrl; this.expireSeconds = expireSeconds; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java` around lines 23 - 35, 생성자 S3PresignService에서 주입된 expireSeconds 값에 대한 유효성 검증이 빠져 있어 0 이하일 경우 Duration.ofSeconds(...) 호출이 잘못된 동작을 할 수 있으므로 생성자 안에서 expireSeconds > 0 인지 확인하고 그렇지 않으면 IllegalArgumentException(또는 적절한 런타임 예외)을 던져 설정 오류를 조기에 노출하도록 변경하세요; 검증은 생성자 매개변수 expireSeconds 및 필드 this.expireSeconds 할당 전에 수행하고 예외 메시지에 실제 값과 기대 범위(>0)를 포함하세요.apps/api-admin/src/main/java/com/ott/api_admin/shortform/mapper/BackOfficeShortFormMapper.java (1)
64-84: 매퍼 메서드의 파라미터가 8개로 많습니다.현재 매퍼 메서드가 레코드 생성자를 단순히 위임하고 있습니다. 파라미터가 많아질 경우 가독성 저하가 우려되므로, 중간 빌더 객체나 파라미터 그룹핑을 고려해볼 수 있습니다. 다만 다른 매퍼와의 일관성을 위해 현재 패턴을 유지하는 것도 합리적입니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/mapper/BackOfficeShortFormMapper.java` around lines 64 - 84, The toShortFormUploadResponse method accepts eight parameters which hurts readability; create a small value object (e.g., ShortFormUploadData) or add a builder/static factory on ShortFormUploadResponse and update toShortFormUploadResponse to accept that single grouped object (or a builder) and pass its fields into the ShortFormUploadResponse constructor — modify the ShortFormUploadResponse type (or add ShortFormUploadData) and change the mapper method signature (toShortFormUploadResponse) to use the grouped parameter to keep call sites cleaner and maintain consistency with other mappers.apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java (1)
274-293: 파일명 정규화 정책의 일관성 확인 필요합니다.백오피스 내에서 파일 업로드 처리 시
BackOfficeShortFormService는 한글을 제거하는 반면([^0-9A-Za-z_-]),BackOfficeContentsService는 한글을 허용합니다([^0-9A-Za-z가-힣_-]). 의도된 차이가 아니라면 서비스 간 동작 불일치로 이어질 수 있으므로, 공통 유틸로 통일하거나 명시적으로 정책을 문서화하는 것이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java` around lines 274 - 293, sanitizeFileName in BackOfficeShortFormService currently strips Korean characters using the pattern "[^0-9A-Za-z_-]" which conflicts with BackOfficeContentsService that allows Korean ("[^0-9A-Za-z가-힣_-]"); make behavior consistent by either (a) updating sanitizeFileName to permit Korean characters (change regex to "[^0-9A-Za-z가-힣_-]" and adjust sanitizedExt accordingly) or (b) extract the normalization into a shared utility (e.g., FileNameSanitizer) and use it from both BackOfficeShortFormService and BackOfficeContentsService so both services use the same regex/policy; ensure BusinessException(ErrorCode.INVALID_INPUT) is still thrown when extension is empty and update unit tests accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.java`:
- Around line 172-191: sanitizeFileName currently strips all non-ASCII
characters (via [^0-9A-Za-z_-]) which turns names like "포스터" and "썸네일" into the
same "file" and risks collisions; modify sanitizeFileName to (1) allow Unicode
letters by changing the name-cleaning regex to include \p{L} and
digits/hyphen/underscore (e.g. replaceAll("[^\\p{L}0-9_-]", "") or equivalent)
so native-language names are preserved, (2) keep the ext sanitization but ensure
it remains ascii-safe, and (3) retain the existing fallback when
sanitizedName.isBlank() but append a short deterministic suffix (e.g. a 6-char
hash or timestamp derived from the original fileName) to the fallback "file" so
multiple blank-sanitized files don’t collide; reference sanitizeFileName and
consider how buildObjectKey composes keys to ensure uniqueness within the same
mediaType.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java`:
- Around line 9-31: ShortFormUploadRequest lacks cross-field validation
enforcing "exactly one of seriesId or contentsId is required"; add a
record-level constraint (either an `@AssertTrue` method or a custom class-level
annotation) to validate seriesId and contentsId are mutually exclusive and not
both null or both non-null; implement a boolean method like isSeriesXorContents
(or a validator class referenced by a new annotation) inside/for
ShortFormUploadRequest that returns true only when exactly one of seriesId or
contentsId is non-null and wire it into the javax/hibernate validation framework
so requests fail validation when both are null or both are present.
- Around line 22-23: The videoSize field in ShortFormUploadRequest is declared
as Integer which will overflow for files > ~2.1GB; change the field type to Long
(ShortFormUploadRequest.videoSize) and update the comment/Javadoc to explicitly
state the unit (e.g., bytes) you expect, and adjust any request examples or
validations that assume an Integer so they use Long-safe values and checks
(e.g., non-negative, max size) to prevent overflow and clarify intent.
In `@docker-compose.yml`:
- Around line 36-41: The api-user service in docker-compose.yml is receiving
unnecessary S3 credentials and presign settings; remove the four environment
variables AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY,
AWS_S3_PRESIGN_EXPIRE_SECONDS, and AWS_S3_PUBLIC_BASE_URL from the api-user
service block so only services that actually use infra-s3 (e.g., api-admin /
BackOfficeContentsService) get those secrets; update the api-user env list in
docker-compose.yml to exclude those keys and ensure no code in the api-user
expects them.
In `@modules/domain/src/main/java/com/ott/domain/contents/domain/Contents.java`:
- Around line 57-60: The updateStorageKeys method allows originUrl to be set to
null but originUrl is mapped with nullable = false; add a null defense in
Contents.updateStorageKeys by validating originUrl (e.g., Objects.requireNonNull
or explicit check) and throw an appropriate runtime exception
(IllegalArgumentException/NullPointerException) with a clear message; keep
masterPlaylistUrl assignment as-is but consider validating it similarly if
needed.
In `@modules/domain/src/main/java/com/ott/domain/media/domain/Media.java`:
- Around line 66-69: updateImageKeys currently assigns posterUrl directly but
posterUrl is mapped nullable = false; add a null guard in Media.updateImageKeys
to prevent DB constraint violations by validating posterUrl (e.g.,
Objects.requireNonNull(posterUrl, "posterUrl must not be null") or if (posterUrl
== null) throw new IllegalArgumentException("posterUrl must not be null")), then
assign this.posterUrl = posterUrl; keep thumbnailUrl assignment as-is (or allow
null if thumbnail is nullable). Reference: Media.updateImageKeys.
In
`@modules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java`:
- Around line 61-64: ShortForm.updateStorageKeys currently assigns originUrl
directly but the originUrl column is nullable = false, so add a null-defense at
the start of the method (e.g., Objects.requireNonNull(originUrl, "originUrl must
not be null") or explicit if (originUrl == null) throw new
IllegalArgumentException("originUrl must not be null")) and only proceed to set
this.originUrl and this.masterPlaylistUrl after the check to avoid persistence
failures; reference the ShortForm.updateStorageKeys method and the originUrl
field/column when making the change.
In `@modules/infra-s3/build.gradle`:
- Line 4: The S3 SDK dependency is pinned to an outdated, vulnerable version
("implementation 'software.amazon.awssdk:s3:2.21.0'"); either remove the
explicit version and adopt the AWS BOM in root dependencyManagement so the
module's build.gradle uses "software.amazon.awssdk:s3" without a version, or
update the dependency to a current safe release (e.g., 2.42.0) if you prefer a
quick fix; add the AWS BOM entry in the root dependencyManagement and remove the
version string in the modules/infra-s3 build.gradle (targeting the dependency
token software.amazon.awssdk:s3) so transitive Netty and other AWS modules are
managed centrally.
In
`@modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java`:
- Around line 37-51: The S3PresignService.createPutPresignedUrl method can
propagate AWS SdkException (via s3Presigner.presignPutObject) which may expose
sensitive info; catch SdkException around the presign call, log the full
exception details locally (using the class logger) and rethrow a safe
BusinessException with a generic message (e.g., "Failed to generate presigned
URL") so no AWS internals reach the client; ensure you reference
S3PresignService.createPutPresignedUrl, catch
software.amazon.awssdk.core.exception.SdkException, and throw your existing
BusinessException (or add one) instead of letting the exception bubble to the
global Exception handler.
---
Outside diff comments:
In `@docker-compose.yml`:
- Around line 88-96: The transcoder service is missing required AWS environment
variables so S3 uploads will fail; update the docker-compose service definition
for the transcoder service to add SPRING_DATASOURCE_* are already present but
ensure you add AWS_REGION, AWS_S3_BUCKET, AWS_ACCESS_KEY_ID, and
AWS_SECRET_ACCESS_KEY as environment entries (or reference existing
env-file/compose variables like ${AWS_REGION}, ${AWS_S3_BUCKET},
${AWS_ACCESS_KEY_ID}, ${AWS_SECRET_ACCESS_KEY}) so the transcoder container can
authenticate and write outputs (look for the service named "transcoder" and
modify its environment block to include these variable names).
---
Nitpick comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsController.java`:
- Line 15: BackOfficeContentsController currently uses a wildcard import
(org.springframework.web.bind.annotation.*); change it to explicit imports to
match the other controllers (e.g., import specific annotations used in this
class such as RestController, RequestMapping, GetMapping, PostMapping,
PutMapping, DeleteMapping, PathVariable, RequestBody, RequestParam, etc.) so the
class BackOfficeContentsController has the same explicit import style as
BackOfficeSeriesController and BackOfficeShortFormController; update the import
statements at the top of the file to list only the annotations actually
referenced by the class.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.java`:
- Around line 9-22: Update the SeriesUploadRequest record to add validation
messages and field-level Swagger metadata: for each field in SeriesUploadRequest
(title, description, actors, publicStatus, posterFileName, thumbnailFileName)
add a message on the existing `@NotBlank/`@NotNull annotations (e.g., message =
"... is required") and annotate each parameter with `@Schema`(description = "...",
example = "...") to match the project's pattern (provide meaningful descriptions
and examples similar to ContentsListResponse); ensure the annotation imports are
present and use the exact field names above so the validators and generated API
docs display clear messages and field descriptions.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.java`:
- Around line 158-170: The resolveContentType method currently throws a generic
BusinessException(ErrorCode.INVALID_INPUT) for unsupported file extensions;
change it to include a clear message naming the supported extensions (e.g.,
".jpg, .jpeg, .png, .webp") and optionally the provided fileName to aid
debugging — update the throw in resolveContentType to
BusinessException(ErrorCode.INVALID_INPUT, "Unsupported extension: " + fileName
+ ". Supported: .jpg, .jpeg, .png, .webp") or the project’s BusinessException
constructor signature so clients receive a specific, actionable error.
- Around line 110-152: createSeriesUpload currently persists Media/Series with
poster/thumbnail set to "PENDING" and returns presigned URLs, which can leave
orphan records if the client never uploads; add a remediation strategy: either
(A) introduce a completion endpoint (e.g., markUploadComplete(seriesId,
posterKey, thumbnailKey) called by client after successful PUT that updates
Media.posterUrl/thumbnailUrl from "PENDING" to final URLs and validates owner
via resolveUploader()), or (B) implement a scheduled cleaner that finds Media
records with posterUrl/thumbnailUrl == "PENDING" older than a configurable
threshold and deletes associated Series and Media rows (use seriesRepository and
mediaRepository, and reference object keys created by buildObjectKey and
s3PresignService.createPutPresignedUrl). Ensure transactions around the
completion update or deletion so no partial state remains and log actions for
audit.
- Around line 193-213: Extract the four duplicated helpers into shared
components and replace the copies in BackOfficeSeriesService,
BackOfficeContentsService, and BackOfficeShortFormService: move
resolveUploader() into a new bean/class (e.g., AuthenticatedMemberResolver) that
injects MemberRepository and uses SecurityContextHolder to return Member or
throw BusinessException; move sanitizeFileName(), resolveContentType(), and
buildObjectKey() into a shared utility/service (e.g., FileNameUtils or
StorageKeyGenerator) as either static helpers or a Spring bean depending on DI
needs; update the three backoffice services to call
AuthenticatedMemberResolver.resolveUploader() and
FileNameUtils.sanitizeFileName()/resolveContentType()/buildObjectKey(), remove
the duplicated method definitions, and add necessary imports/constructor
injection where applicable.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java`:
- Around line 9-31: ShortFormUploadRequest is missing validation messages and
field-level `@Schema` metadata; update the record component annotations to include
message attributes on `@NotBlank/`@NotNull (e.g. give descriptive messages for
title, description, publicStatus, posterFileName, thumbnailFileName,
originFileName) and add `@Schema`(description="...", example="...") for each field
(seriesId, contentsId, title, description, publicStatus, duration, videoSize,
posterFileName, thumbnailFileName, originFileName) following the pattern used in
SeriesUploadRequest; ensure you import and use
io.swagger.v3.oas.annotations.media.Schema and include meaningful example values
and descriptions consistent with existing DTOs.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/mapper/BackOfficeShortFormMapper.java`:
- Around line 64-84: The toShortFormUploadResponse method accepts eight
parameters which hurts readability; create a small value object (e.g.,
ShortFormUploadData) or add a builder/static factory on ShortFormUploadResponse
and update toShortFormUploadResponse to accept that single grouped object (or a
builder) and pass its fields into the ShortFormUploadResponse constructor —
modify the ShortFormUploadResponse type (or add ShortFormUploadData) and change
the mapper method signature (toShortFormUploadResponse) to use the grouped
parameter to keep call sites cleaner and maintain consistency with other
mappers.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java`:
- Around line 274-293: sanitizeFileName in BackOfficeShortFormService currently
strips Korean characters using the pattern "[^0-9A-Za-z_-]" which conflicts with
BackOfficeContentsService that allows Korean ("[^0-9A-Za-z가-힣_-]"); make
behavior consistent by either (a) updating sanitizeFileName to permit Korean
characters (change regex to "[^0-9A-Za-z가-힣_-]" and adjust sanitizedExt
accordingly) or (b) extract the normalization into a shared utility (e.g.,
FileNameSanitizer) and use it from both BackOfficeShortFormService and
BackOfficeContentsService so both services use the same regex/policy; ensure
BusinessException(ErrorCode.INVALID_INPUT) is still thrown when extension is
empty and update unit tests accordingly.
In `@docker-compose.yml`:
- Around line 56-78: Add a healthcheck to the api-admin service so other
services can safely depend on it; mirror the same pattern used by api-user by
adding a healthcheck section under the api-admin service (use the same test
command, interval, timeout, retries, and start_period values as api-user) and
ensure depends_on in other services can reference api-admin with condition:
service_healthy once added.
In
`@modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java`:
- Around line 57-67: The toObjectUrl method can double-encode an
already-percent-encoded objectKey; update to normalize input in
S3PresignService.toObjectUrl by detecting whether objectKey already contains
percent-encoding (e.g., a '%' followed by two hex digits) and, if so, use it
as-is (or first URL-decode then re-encode safely), otherwise perform the
URLEncoder.encode(...) + replace("+","%20"). Keep the defensive null/blank
handling for publicBaseUrl as-is but ensure publicBaseUrl trimming
(replaceAll("/+$","")) remains applied; this prevents double-encoding while
preserving current base URL fallback behavior.
- Around line 23-35: 생성자 S3PresignService에서 주입된 expireSeconds 값에 대한 유효성 검증이 빠져
있어 0 이하일 경우 Duration.ofSeconds(...) 호출이 잘못된 동작을 할 수 있으므로 생성자 안에서 expireSeconds >
0 인지 확인하고 그렇지 않으면 IllegalArgumentException(또는 적절한 런타임 예외)을 던져 설정 오류를 조기에 노출하도록
변경하세요; 검증은 생성자 매개변수 expireSeconds 및 필드 this.expireSeconds 할당 전에 수행하고 예외 메시지에 실제
값과 기대 범위(>0)를 포함하세요.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
apps/api-admin/build.gradleapps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.javaapps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsController.javaapps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/content/dto/response/ContentsUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/content/mapper/BackOfficeContentsMapper.javaapps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.javaapps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.javaapps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesController.javaapps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/series/dto/response/SeriesUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/series/mapper/BackOfficeSeriesMapper.javaapps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/controller/BackOfficeShortFormApi.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/controller/BackOfficeShortFormController.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/OriginMediaTitleListResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormDetailResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormListResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/mapper/BackOfficeShortFormMapper.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.javaapps/api-admin/src/main/java/com/ott/api_admin/upload/controller/.gitkeepapps/api-admin/src/main/java/com/ott/api_admin/upload/dto/.gitkeepapps/api-admin/src/main/java/com/ott/api_admin/upload/service/.gitkeepapps/api-user/build.gradleapps/transcoder/build.gradledocker-compose.ymlmodules/domain/src/main/java/com/ott/domain/contents/domain/Contents.javamodules/domain/src/main/java/com/ott/domain/media/domain/Media.javamodules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.javamodules/infra-db/build.gradlemodules/infra-db/src/main/resources/db/migration/V1__init.sqlmodules/infra-db/src/main/resources/db/migration/V2__media_table_inheritance.sqlmodules/infra-s3/build.gradlemodules/infra-s3/src/main/java/com/ott/infra/s3/config/S3PresignerConfig.javamodules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.javamodules/infra/src/main/java/com/ott/infra/db/.gitkeepmodules/infra/src/main/java/com/ott/infra/s3/.gitkeepsettings.gradle
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
|
이번 PR 리뷰에서 coderabbit/coderabbit-guidelines.md를 읽고 리뷰에도 반영해줘. 1개씩 댓글 달 수 있으면 그렇게 부탁해 |
🧩 Analysis chain🏁 Script executed: Length of output: 12253 🏁 Script executed: Length of output: 33613
네, 🚨 P1 (머지 전 보완 권장): 심각한 코드 중복1️⃣ 동일한 헬퍼 메서드가 3개 서비스에 중복위반 내용:
해당 파일:
가이드라인 위반:
수정 제안: 공통 유틸리티 클래스 또는 서비스로 추출하세요: // 옵션 1: 유틸리티 클래스로 추출
// apps/api-admin/src/main/java/com/ott/api_admin/common/util/UploadFileUtils.java
`@Component`
public class UploadFileUtils {
public String sanitizeFileName(String fileName) {
// 구현...
}
public String buildObjectKey(String root, Long id, String mediaType, String fileName) {
return root + "/" + id + "/" + mediaType + "/" + fileName;
}
public String resolveImageContentType(String fileName) {
// 구현...
}
public String resolveVideoContentType(String fileName) {
// 구현...
}
}
// 옵션 2: SecurityUtils로 분리
`@Component`
public class SecurityUtils {
private final MemberRepository memberRepository;
public Member resolveCurrentUploader() {
// 구현...
}
}그리고 각 서비스에서 주입받아 사용: `@Service`
`@RequiredArgsConstructor`
public class BackOfficeSeriesService {
private final UploadFileUtils uploadFileUtils;
private final SecurityUtils securityUtils;
`@Transactional`
public SeriesUploadResponse createSeriesUpload(SeriesUploadRequest request) {
Member uploader = securityUtils.resolveCurrentUploader();
String sanitizedPosterFileName = uploadFileUtils.sanitizeFileName(request.posterFileName());
String posterObjectKey = uploadFileUtils.buildObjectKey("series", seriesId, "poster", sanitizedPosterFileName);
// ...
}
}2️⃣ sanitizeFileName 로직 불일치위반 내용:
해당 파일:
문제점: 수정 제안: 만약 한글을 허용한다면: .replaceAll("[^0-9A-Za-z가-힣_-]", "")허용하지 않는다면: .replaceAll("[^0-9A-Za-z_-]", "")✅ 가이드라인 준수 확인다음 항목들은 가이드라인을 잘 따르고 있습니다:
📋 액션 아이템 요약
위 두 가지 이슈를 수정하시면 가이드라인을 완벽히 준수하는 깔끔한 구현이 될 것 같습니다! 💯 ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker-compose.yml (1)
80-96:⚠️ Potential issue | 🟠 Major
transcoder서비스에 AWS 자격증명 누락 및 설정 오류세 가지 문제가 있습니다:
(Major) AWS 자격증명 없음 — 트랜스코더는 S3에서 원본 영상을 읽고 변환 결과를 다시 S3에 저장해야 합니다.
AWS_REGION,AWS_S3_BUCKET,AWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY등이 없으면 S3 접근이 불가능합니다.(Minor)
healthcheck없음 — 다른 서비스가 트랜스코더의 준비 상태에 의존해야 하는 경우 감지할 수 없습니다.(Minor)
api-user의존 불합리 — 백그라운드 워커인 트랜스코더가api-user서비스의 헬스체크를 기다릴 이유가 없습니다.mysql의존만 유지하면 충분합니다.🔧 제안 수정
transcoder: build: context: . dockerfile: apps/transcoder/Dockerfile container_name: ott-transcoder ports: - "8082:8082" environment: SPRING_DATASOURCE_URL: ${SPRING_DATASOURCE_URL} SPRING_DATASOURCE_USERNAME: ${SPRING_DATASOURCE_USERNAME} SPRING_DATASOURCE_PASSWORD: ${SPRING_DATASOURCE_PASSWORD} + AWS_REGION: ${AWS_REGION} + AWS_S3_BUCKET: ${AWS_S3_BUCKET} + AWS_ACCESS_KEY_ID: ${AWS_ACCESS_KEY_ID} + AWS_SECRET_ACCESS_KEY: ${AWS_SECRET_ACCESS_KEY} depends_on: mysql: condition: service_healthy - api-user: - condition: service_healthy + healthcheck: + test: ["CMD-SHELL", "curl -f http://localhost:8082/actuator/health || exit 1"] + interval: 10s + timeout: 5s + retries: 12 + start_period: 30s🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 80 - 96, The transcoder service definition is missing required AWS S3 credentials and has incorrect dependencies and no healthcheck; update the "transcoder" service to include environment variables AWS_REGION, AWS_S3_BUCKET, AWS_ACCESS_KEY_ID, and AWS_SECRET_ACCESS_KEY (sourcing from env), remove the unnecessary dependency on "api-user" so only "mysql" with its health condition remains, and add a suitable healthcheck block (e.g., an HTTP/TCP or custom command that validates the worker is ready) to ensure other services can wait for transcoder readiness.
♻️ Duplicate comments (8)
modules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java (1)
61-64:originUrlnull 방어 — 이전 리뷰에서 이미 지적된 사항입니다.
origin_url은nullable = false컬럼이므로 null 전달 시 영속화 실패가 발생합니다. 아직 수정되지 않은 상태입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java` around lines 61 - 64, The updateStorageKeys method on ShortForm currently assigns originUrl directly which can set the non-nullable database column to null; add a null check for the originUrl parameter in ShortForm.updateStorageKeys and refuse or fail fast (e.g., throw IllegalArgumentException or use Objects.requireNonNull) before assigning fields (originUrl and masterPlaylistUrl) so that originUrl can never be set to null at runtime; update the method to validate originUrl and only then assign this.originUrl and this.masterPlaylistUrl.modules/domain/src/main/java/com/ott/domain/media/domain/Media.java (1)
66-69:posterUrlnull 방어 — 이전 리뷰에서 이미 지적된 사항입니다.
poster_url은nullable = false컬럼이므로 null 할당 시 DB 제약 위반이 발생합니다. 아직 수정되지 않은 상태입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/domain/src/main/java/com/ott/domain/media/domain/Media.java` around lines 66 - 69, updateImageKeys currently assigns posterUrl directly which can set the entity's non-nullable posterUrl column to null and violate DB constraints; add a null-check for posterUrl in the Media.updateImageKeys method (use Objects.requireNonNull or explicit if and throw IllegalArgumentException) so posterUrl cannot be set to null, and only assign thumbnailUrl after any required validation; reference method: updateImageKeys and fields: posterUrl, thumbnailUrl.modules/domain/src/main/java/com/ott/domain/contents/domain/Contents.java (1)
57-60:originUrlnull 방어 — 이전 리뷰에서 이미 지적된 사항입니다.
origin_url은nullable = false컬럼이므로 null 전달 시 영속화 오류가 발생합니다. 아직 수정되지 않은 상태입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/domain/src/main/java/com/ott/domain/contents/domain/Contents.java` around lines 57 - 60, updateStorageKeys에서 originUrl을 null로 설정하면 DB의 nullable=false 제약에 걸리므로 null 방어를 추가하세요: updateStorageKeys(String originUrl, String masterPlaylistUrl) 내부에서 originUrl이 null이면 IllegalArgumentException을 던지거나 기존 this.originUrl을 유지하도록 처리하고, masterPlaylistUrl도 필요시 동일한 검사(또는 허용 정책)를 적용하세요; 관련 식별자: method updateStorageKeys, fields originUrl, masterPlaylistUrl.modules/infra-s3/build.gradle (1)
4-4:software.amazon.awssdk:s3:2.21.0— 이전 리뷰에서 이미 지적된 사항입니다.취약한 버전 사용 문제(CVE-2025-67735 포함)가 아직 해결되지 않았습니다. AWS BOM 적용 또는 최신 버전으로 업그레이드가 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/infra-s3/build.gradle` at line 4, The dependency declaration implementation 'software.amazon.awssdk:s3:2.21.0' is using a known-vulnerable SDK version; replace the direct-version use with the AWS SDK BOM or upgrade to a patched SDK release: add the BOM via implementation platform("software.amazon.awssdk:bom:<secure-version>") (or update the coordinate to implementation 'software.amazon.awssdk:s3:<secure-version>') and remove the hardcoded 2.21.0 reference so the S3 client inherits a safe, vetted version; ensure the change is applied where the dependency string 'software.amazon.awssdk:s3:2.21.0' appears (build.gradle) and run a dependency refresh to verify the vulnerable version is gone.apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java (2)
22-23: 이전 리뷰 미반영:Integer videoSize오버플로 미수정이전 리뷰에서 지적된
Integer videoSize의 오버플로 이슈(~2.1GB 초과 시)가 여전히 수정되지 않았습니다.Long으로 변경이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java` around lines 22 - 23, The DTO field videoSize in ShortFormUploadRequest must be changed from Integer to Long to avoid overflow for files >~2.1GB; update the field declaration (videoSize), any getter/setter or Lombok annotations/constructor/builder usages in ShortFormUploadRequest, and any places that construct or read this DTO (e.g., controllers/services) to accept/return Long, and adjust related validation or JSON binding annotations if present to ensure correct serialization/deserialization.
9-31: 이전 리뷰 미반영:seriesId/contentsId상호 배타 검증 누락
seriesId와contentsId모두null또는 둘 다 값이 있는 케이스가 여전히 검증 없이 통과됩니다. 이전 리뷰에서 지적된 레코드 수준 교차 필드 검증이 적용되지 않았습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java` around lines 9 - 31, ShortFormUploadRequest lacks record-level cross-field validation: ensure seriesId and contentsId are mutually exclusive (not both null and not both non-null). Add a custom constraint annotation (e.g., `@EitherSeriesOrContents`) on the ShortFormUploadRequest record and implement a ConstraintValidator that checks the record's seriesId and contentsId to enforce exactly one non-null; register the validator and add a meaningful message when invalid so requests with both null or both set will fail validation.docker-compose.yml (1)
36-41: 이전 리뷰 미반영:api-user에 불필요한 AWS 자격증명 주입
api-user는infra-s3를 의존하지 않으며 S3 프리사인 기능을 사용하지 않습니다.AWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY,AWS_S3_PRESIGN_EXPIRE_SECONDS를api-user에 주입하는 것은 최소 권한 원칙 위반입니다. 이전 리뷰에서 지적된 사항이 미반영 상태입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 36 - 41, The api-user service is being injected with unnecessary AWS credentials and presign config; remove the environment variables AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and AWS_S3_PRESIGN_EXPIRE_SECONDS from the api-user service definition in docker-compose.yml so only services that depend on infra-s3 receive those secrets; keep AWS_REGION/AWS_S3_BUCKET/AWS_S3_PUBLIC_BASE_URL if needed but ensure api-user does not receive any AWS secret envs to follow least-privilege.modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java (1)
37-51: AWS SDK 예외가 클라이언트에 노출될 수 있습니다.
s3Presigner.presignPutObject()호출 시 발생하는SdkException이 글로벌 예외 핸들러를 통해 AWS 내부 정보(버킷명, 리전 등)가 그대로 클라이언트에 전달될 수 있습니다.SdkException을 catch하여BusinessException으로 래핑하는 것이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java` around lines 37 - 51, The createPutPresignedUrl method in S3PresignService should catch AWS SdkException thrown by s3Presigner.presignPutObject and wrap it in your domain BusinessException before propagating; update createPutPresignedUrl to try the presign call, catch SdkException (and optionally its subclasses), log or attach a safe message (no bucket/region details), and throw new BusinessException with a generic error code/message so AWS internal info is not leaked to clients.
🧹 Nitpick comments (7)
modules/infra-s3/src/main/java/com/ott/infra/s3/config/S3PresignerConfig.java (1)
14-19: 로컬 테스트용 엔드포인트 재정의 설정 부재현재 구현으로는 LocalStack 등 로컬 S3 모의 환경을 사용할 때 엔드포인트를 재정의할 방법이 없습니다.
S3Presigner.Builder는endpointOverride(URI)메서드를 제공하므로, 환경 프로퍼티로 선택적으로 주입할 수 있습니다.♻️ 엔드포인트 재정의 옵션 추가 제안
+import java.net.URI; +import org.springframework.lang.Nullable; `@Bean` public S3Presigner s3Presigner( - `@Value`("${aws.region:ap-northeast-2}") String region) { + `@Value`("${aws.region:ap-northeast-2}") String region, + `@Value`("${aws.s3.endpoint-override:#{null}}") `@Nullable` String endpointOverride) { S3Presigner.Builder builder = S3Presigner.builder() .region(Region.of(region)) .credentialsProvider(DefaultCredentialsProvider.create()); + if (endpointOverride != null) { + builder.endpointOverride(URI.create(endpointOverride)); + } return builder.build(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/infra-s3/src/main/java/com/ott/infra/s3/config/S3PresignerConfig.java` around lines 14 - 19, The s3Presigner bean currently never lets callers override the S3 endpoint for local testing; update the s3Presigner(String region) factory to accept an optional endpoint property (e.g. `@Value`("${aws.s3.endpoint:}") String endpoint) and, when endpoint is non-empty, call S3Presigner.builder().endpointOverride(URI.create(endpoint)) before build(); keep Region.of(region) and credentialsProvider as-is and only apply endpointOverride conditionally to support LocalStack or other local S3 mocks.apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormUploadResponse.java (1)
6-23: DTO 필드에@Schema어노테이션 누락각 record 컴포넌트에 주석 대신
@Schema어노테이션을 추가하면 OpenAPI 문서에 필드 설명이 자동으로 반영됩니다. 현재는 Java 주석(//)만 있어 Swagger UI에서 필드 설명이 표시되지 않습니다.♻️ 제안 예시
`@Schema`(description = "숏폼 업로드 응답") public record ShortFormUploadResponse( - // 생성된 숏폼 ID - Long shortFormId, - // 포스터 S3 object key - String posterObjectKey, + `@Schema`(description = "생성된 숏폼 ID") + Long shortFormId, + `@Schema`(description = "포스터 S3 object key") + String posterObjectKey, // ... 나머지 필드도 동일하게 적용🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormUploadResponse.java` around lines 6 - 23, The record ShortFormUploadResponse is using comments instead of OpenAPI metadata so add `@Schema` annotations to each record component (shortFormId, posterObjectKey, thumbnailObjectKey, originObjectKey, masterPlaylistObjectKey, posterUploadUrl, thumbnailUploadUrl, originUploadUrl) with appropriate description texts (e.g., "생성된 숏폼 ID", "포스터 S3 object key", etc.), import io.swagger.v3.oas.annotations.media.Schema, and place the annotations directly before each component declaration in the record header so Swagger/OpenAPI picks up the field descriptions.apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java (3)
295-315:resolveUploader()가SecurityContextHolder에 직접 접근하는 방식이 동일 클래스 내 다른 메서드와 불일치합니다.
getShortFormList,getShortFormDetail은 컨트롤러에서Authentication을 파라미터로 전달받지만,createShortFormUpload는SecurityContextHolder에서 직접 조회합니다. 테스트 시 SecurityContext 설정이 추가로 필요해지며 일관성이 떨어집니다. 컨트롤러에서Authentication을 주입받아 서비스로 전달하는 기존 패턴을 따르는 것을 권장합니다.♻️ 제안
컨트롤러에서
Authentication authentication파라미터를 받아 서비스로 전달:// Controller -public ResponseEntity<SuccessResponse<ShortFormUploadResponse>> createShortFormUpload( - `@Valid` `@RequestBody` ShortFormUploadRequest request -) { - return ResponseEntity.ok(SuccessResponse.of(backOfficeShortFormService.createShortFormUpload(request))); -} +public ResponseEntity<SuccessResponse<ShortFormUploadResponse>> createShortFormUpload( + `@Valid` `@RequestBody` ShortFormUploadRequest request, + Authentication authentication +) { + return ResponseEntity.ok(SuccessResponse.of(backOfficeShortFormService.createShortFormUpload(request, authentication))); +}서비스에서
Authentication으로부터 memberId를 추출:-private Member resolveUploader() { - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - ... - Long memberId; - try { - memberId = Long.valueOf(String.valueOf(principal)); - } catch (NumberFormatException ex) { - throw new BusinessException(ErrorCode.UNAUTHORIZED); - } +private Member resolveUploader(Authentication authentication) { + Long memberId = (Long) authentication.getPrincipal(); return memberRepository.findById(memberId) .orElseThrow(() -> new BusinessException(ErrorCode.UNAUTHORIZED)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java` around lines 295 - 315, resolveUploader() currently reads SecurityContextHolder directly which is inconsistent with other methods (getShortFormList, getShortFormDetail) and forces tests to set SecurityContext; change the API so the controller passes Authentication into the service (e.g. update createShortFormUpload to accept Authentication) and modify resolveUploader to accept an Authentication parameter (or add resolveUploader(Authentication authentication)) and extract principal -> memberId and lookup memberRepository.findById(memberId) with the same Unauthorized checks and exceptions; update callers to pass the Authentication from the controller instead of relying on SecurityContextHolder.
164-203:"PENDING"매직 문자열 사용
posterUrl,thumbnailUrl,originUrl,masterPlaylistUrl등을"PENDING"문자열로 초기화한 뒤 즉시 실제 URL로 갱신하고 있습니다. 트랜잭션 내에서 동작하므로 기능적 문제는 없지만, 만약 presigned URL 생성 전에 예외가 발생하면"PENDING"값이 롤백되어 문제는 없습니다.다만
"PENDING"대신 처음부터 실제 URL을 계산하여 엔티티 생성 시 전달하면 2단계 저장(save → update)을 1단계로 줄일 수 있습니다. 이를 위해서는 ID를 먼저 확보해야 하므로, 현재 구조에서는 save 후 ID를 얻어 key를 만드는 방식이 불가피할 수 있지만,@GeneratedValue전략에 따라 ID 사전 할당이 가능할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java` around lines 164 - 203, The code uses the magic string "PENDING" when creating Media and ShortForm and immediately updates them with presigned URLs (Media.builder(...).posterUrl("PENDING") / ShortForm.builder(...).originUrl("PENDING")), causing an unnecessary save→update; change the flow to compute and set the actual S3 object keys/URLs before persisting so you can create Media and ShortForm in one save: obtain the shortFormId needed for buildObjectKey either by switching the ID generation to a pre-assigned strategy (e.g., adjust `@GeneratedValue` strategy or allocate the ID via your ID generator) or by generating the object keys from a deterministically derived ID, then call s3PresignService.toObjectUrl(...) to set posterUrl/thumbnailUrl/originUrl/masterPlaylistUrl on the builders and persist with mediaRepository.save(...) and shortFormRepository.save(...) without using "PENDING"; refer to Media, ShortForm, buildObjectKey, s3PresignService, media.updateImageKeys and shortForm.updateStorageKeys to locate the changes.
239-272: 공통 업로드 유틸리티 클래스로 헬퍼 메서드 추출 권장검증 결과,
sanitizeFileName,resolveContentType,resolveOriginContentType,buildObjectKey메서드가 시리즈·콘텐츠·숏폼 서비스에 중복 정의되어 있습니다. 파일명 정규화, 확장자별 Content-Type 매핑, S3 object key 빌드 등의 공통 로직을FileUploadUtils또는MediaUploadHelper같은 공유 유틸리티 클래스로 추출하면 코드 중복을 제거하고 유지보수성을 향상시킬 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java` around lines 239 - 272, Extract the duplicated helpers (sanitizeFileName, resolveContentType, resolveOriginContentType, buildObjectKey) into a single shared utility class (e.g., FileUploadUtils or MediaUploadHelper) and update callers in series, content, and shortform services to use that utility; move the extension-to-content-type mappings and file name normalization logic into static methods on the new class, ensure the methods keep the same signatures/behavior (throw BusinessException(ErrorCode.INVALID_INPUT) for unknown extensions), and replace inline implementations in BackOfficeShortFormService with calls to the new utility to remove duplication and centralize maintenance.modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java (1)
57-66:objectKey가null일 때NullPointerException발생 가능
toObjectUrl에서objectKey가null이면URLEncoder.encode(null, ...)이 NPE를 발생시킵니다. 현재 호출부에서 null이 전달될 가능성은 낮지만, public API이므로 방어 코드를 추가하는 것이 좋습니다.🛡️ 방어 코드 제안
public String toObjectUrl(String objectKey) { + if (objectKey == null || objectKey.isBlank()) { + throw new IllegalArgumentException("objectKey must not be null or blank"); + } String encodedKey = URLEncoder.encode(objectKey, StandardCharsets.UTF_8)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java` around lines 57 - 66, The method toObjectUrl currently passes objectKey directly to URLEncoder.encode which will NPE if objectKey is null; add an explicit null check at the start of toObjectUrl (before calling URLEncoder.encode) and throw a clear IllegalArgumentException (e.g., "objectKey must not be null") or otherwise handle the null case per API contract; update callers/tests if you choose a different behavior. Ensure the check is placed in S3PresignService.toObjectUrl and protects the encodedKey computation and subsequent baseUrl concatenation.apps/api-admin/src/main/java/com/ott/api_admin/shortform/mapper/BackOfficeShortFormMapper.java (1)
64-84: 매퍼 메서드의 파라미터가 8개로 과다이 매퍼 메서드는 단순히 record 생성자로 위임하고 있어 실질적인 매핑 로직이 없습니다. 파라미터가 8개이면 호출부의 가독성이 떨어지므로, record를 직접 생성하거나 빌더 패턴/중간 DTO를 도입하는 것을 고려해볼 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/mapper/BackOfficeShortFormMapper.java` around lines 64 - 84, The toShortFormUploadResponse method in BackOfficeShortFormMapper currently takes eight separate parameters which hurts call-site readability; replace this with a single param object or use the record/builder for ShortFormUploadResponse: create a Small DTO/record (e.g., ShortFormUploadData) carrying posterObjectKey, thumbnailObjectKey, originObjectKey, masterPlaylistObjectKey, posterUploadUrl, thumbnailUploadUrl, originUploadUrl and change the mapper signature to toShortFormUploadResponse(ShortFormUploadData data) and return new ShortFormUploadResponse(data.get...()) or alternatively remove the mapper and construct ShortFormUploadResponse directly at the caller using its record/builder, updating all call sites that used BackOfficeShortFormMapper.toShortFormUploadResponse to pass the new DTO or the direct constructor/builder.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.java`:
- Around line 22-23: The videoSize field in ContentsUploadRequest uses Integer
which overflows for files >2GB; change the field type videoSize from Integer to
Long (e.g., Long videoSize) and update its comment to explicitly state the unit
(bytes) or use a clearly documented unit (e.g., "videoSize (bytes)") so callers
and validators use 64-bit sizes; also search for usages of
ContentsUploadRequest.videoSize and update any parsing/validation or
serialization logic to handle Long.
---
Outside diff comments:
In `@docker-compose.yml`:
- Around line 80-96: The transcoder service definition is missing required AWS
S3 credentials and has incorrect dependencies and no healthcheck; update the
"transcoder" service to include environment variables AWS_REGION, AWS_S3_BUCKET,
AWS_ACCESS_KEY_ID, and AWS_SECRET_ACCESS_KEY (sourcing from env), remove the
unnecessary dependency on "api-user" so only "mysql" with its health condition
remains, and add a suitable healthcheck block (e.g., an HTTP/TCP or custom
command that validates the worker is ready) to ensure other services can wait
for transcoder readiness.
---
Duplicate comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java`:
- Around line 22-23: The DTO field videoSize in ShortFormUploadRequest must be
changed from Integer to Long to avoid overflow for files >~2.1GB; update the
field declaration (videoSize), any getter/setter or Lombok
annotations/constructor/builder usages in ShortFormUploadRequest, and any places
that construct or read this DTO (e.g., controllers/services) to accept/return
Long, and adjust related validation or JSON binding annotations if present to
ensure correct serialization/deserialization.
- Around line 9-31: ShortFormUploadRequest lacks record-level cross-field
validation: ensure seriesId and contentsId are mutually exclusive (not both null
and not both non-null). Add a custom constraint annotation (e.g.,
`@EitherSeriesOrContents`) on the ShortFormUploadRequest record and implement a
ConstraintValidator that checks the record's seriesId and contentsId to enforce
exactly one non-null; register the validator and add a meaningful message when
invalid so requests with both null or both set will fail validation.
In `@docker-compose.yml`:
- Around line 36-41: The api-user service is being injected with unnecessary AWS
credentials and presign config; remove the environment variables
AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and AWS_S3_PRESIGN_EXPIRE_SECONDS from
the api-user service definition in docker-compose.yml so only services that
depend on infra-s3 receive those secrets; keep
AWS_REGION/AWS_S3_BUCKET/AWS_S3_PUBLIC_BASE_URL if needed but ensure api-user
does not receive any AWS secret envs to follow least-privilege.
In `@modules/domain/src/main/java/com/ott/domain/contents/domain/Contents.java`:
- Around line 57-60: updateStorageKeys에서 originUrl을 null로 설정하면 DB의
nullable=false 제약에 걸리므로 null 방어를 추가하세요: updateStorageKeys(String originUrl,
String masterPlaylistUrl) 내부에서 originUrl이 null이면 IllegalArgumentException을 던지거나
기존 this.originUrl을 유지하도록 처리하고, masterPlaylistUrl도 필요시 동일한 검사(또는 허용 정책)를 적용하세요;
관련 식별자: method updateStorageKeys, fields originUrl, masterPlaylistUrl.
In `@modules/domain/src/main/java/com/ott/domain/media/domain/Media.java`:
- Around line 66-69: updateImageKeys currently assigns posterUrl directly which
can set the entity's non-nullable posterUrl column to null and violate DB
constraints; add a null-check for posterUrl in the Media.updateImageKeys method
(use Objects.requireNonNull or explicit if and throw IllegalArgumentException)
so posterUrl cannot be set to null, and only assign thumbnailUrl after any
required validation; reference method: updateImageKeys and fields: posterUrl,
thumbnailUrl.
In
`@modules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java`:
- Around line 61-64: The updateStorageKeys method on ShortForm currently assigns
originUrl directly which can set the non-nullable database column to null; add a
null check for the originUrl parameter in ShortForm.updateStorageKeys and refuse
or fail fast (e.g., throw IllegalArgumentException or use
Objects.requireNonNull) before assigning fields (originUrl and
masterPlaylistUrl) so that originUrl can never be set to null at runtime; update
the method to validate originUrl and only then assign this.originUrl and
this.masterPlaylistUrl.
In `@modules/infra-s3/build.gradle`:
- Line 4: The dependency declaration implementation
'software.amazon.awssdk:s3:2.21.0' is using a known-vulnerable SDK version;
replace the direct-version use with the AWS SDK BOM or upgrade to a patched SDK
release: add the BOM via implementation
platform("software.amazon.awssdk:bom:<secure-version>") (or update the
coordinate to implementation 'software.amazon.awssdk:s3:<secure-version>') and
remove the hardcoded 2.21.0 reference so the S3 client inherits a safe, vetted
version; ensure the change is applied where the dependency string
'software.amazon.awssdk:s3:2.21.0' appears (build.gradle) and run a dependency
refresh to verify the vulnerable version is gone.
In
`@modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java`:
- Around line 37-51: The createPutPresignedUrl method in S3PresignService should
catch AWS SdkException thrown by s3Presigner.presignPutObject and wrap it in
your domain BusinessException before propagating; update createPutPresignedUrl
to try the presign call, catch SdkException (and optionally its subclasses), log
or attach a safe message (no bucket/region details), and throw new
BusinessException with a generic error code/message so AWS internal info is not
leaked to clients.
---
Nitpick comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormUploadResponse.java`:
- Around line 6-23: The record ShortFormUploadResponse is using comments instead
of OpenAPI metadata so add `@Schema` annotations to each record component
(shortFormId, posterObjectKey, thumbnailObjectKey, originObjectKey,
masterPlaylistObjectKey, posterUploadUrl, thumbnailUploadUrl, originUploadUrl)
with appropriate description texts (e.g., "생성된 숏폼 ID", "포스터 S3 object key",
etc.), import io.swagger.v3.oas.annotations.media.Schema, and place the
annotations directly before each component declaration in the record header so
Swagger/OpenAPI picks up the field descriptions.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/mapper/BackOfficeShortFormMapper.java`:
- Around line 64-84: The toShortFormUploadResponse method in
BackOfficeShortFormMapper currently takes eight separate parameters which hurts
call-site readability; replace this with a single param object or use the
record/builder for ShortFormUploadResponse: create a Small DTO/record (e.g.,
ShortFormUploadData) carrying posterObjectKey, thumbnailObjectKey,
originObjectKey, masterPlaylistObjectKey, posterUploadUrl, thumbnailUploadUrl,
originUploadUrl and change the mapper signature to
toShortFormUploadResponse(ShortFormUploadData data) and return new
ShortFormUploadResponse(data.get...()) or alternatively remove the mapper and
construct ShortFormUploadResponse directly at the caller using its
record/builder, updating all call sites that used
BackOfficeShortFormMapper.toShortFormUploadResponse to pass the new DTO or the
direct constructor/builder.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java`:
- Around line 295-315: resolveUploader() currently reads SecurityContextHolder
directly which is inconsistent with other methods (getShortFormList,
getShortFormDetail) and forces tests to set SecurityContext; change the API so
the controller passes Authentication into the service (e.g. update
createShortFormUpload to accept Authentication) and modify resolveUploader to
accept an Authentication parameter (or add resolveUploader(Authentication
authentication)) and extract principal -> memberId and lookup
memberRepository.findById(memberId) with the same Unauthorized checks and
exceptions; update callers to pass the Authentication from the controller
instead of relying on SecurityContextHolder.
- Around line 164-203: The code uses the magic string "PENDING" when creating
Media and ShortForm and immediately updates them with presigned URLs
(Media.builder(...).posterUrl("PENDING") /
ShortForm.builder(...).originUrl("PENDING")), causing an unnecessary
save→update; change the flow to compute and set the actual S3 object keys/URLs
before persisting so you can create Media and ShortForm in one save: obtain the
shortFormId needed for buildObjectKey either by switching the ID generation to a
pre-assigned strategy (e.g., adjust `@GeneratedValue` strategy or allocate the ID
via your ID generator) or by generating the object keys from a deterministically
derived ID, then call s3PresignService.toObjectUrl(...) to set
posterUrl/thumbnailUrl/originUrl/masterPlaylistUrl on the builders and persist
with mediaRepository.save(...) and shortFormRepository.save(...) without using
"PENDING"; refer to Media, ShortForm, buildObjectKey, s3PresignService,
media.updateImageKeys and shortForm.updateStorageKeys to locate the changes.
- Around line 239-272: Extract the duplicated helpers (sanitizeFileName,
resolveContentType, resolveOriginContentType, buildObjectKey) into a single
shared utility class (e.g., FileUploadUtils or MediaUploadHelper) and update
callers in series, content, and shortform services to use that utility; move the
extension-to-content-type mappings and file name normalization logic into static
methods on the new class, ensure the methods keep the same signatures/behavior
(throw BusinessException(ErrorCode.INVALID_INPUT) for unknown extensions), and
replace inline implementations in BackOfficeShortFormService with calls to the
new utility to remove duplication and centralize maintenance.
In
`@modules/infra-s3/src/main/java/com/ott/infra/s3/config/S3PresignerConfig.java`:
- Around line 14-19: The s3Presigner bean currently never lets callers override
the S3 endpoint for local testing; update the s3Presigner(String region) factory
to accept an optional endpoint property (e.g. `@Value`("${aws.s3.endpoint:}")
String endpoint) and, when endpoint is non-empty, call
S3Presigner.builder().endpointOverride(URI.create(endpoint)) before build();
keep Region.of(region) and credentialsProvider as-is and only apply
endpointOverride conditionally to support LocalStack or other local S3 mocks.
In
`@modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java`:
- Around line 57-66: The method toObjectUrl currently passes objectKey directly
to URLEncoder.encode which will NPE if objectKey is null; add an explicit null
check at the start of toObjectUrl (before calling URLEncoder.encode) and throw a
clear IllegalArgumentException (e.g., "objectKey must not be null") or otherwise
handle the null case per API contract; update callers/tests if you choose a
different behavior. Ensure the check is placed in S3PresignService.toObjectUrl
and protects the encodedKey computation and subsequent baseUrl concatenation.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
apps/api-admin/build.gradleapps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.javaapps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsController.javaapps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/content/dto/response/ContentsUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/content/mapper/BackOfficeContentsMapper.javaapps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.javaapps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.javaapps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesController.javaapps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/series/dto/response/SeriesUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/series/mapper/BackOfficeSeriesMapper.javaapps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/controller/BackOfficeShortFormApi.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/controller/BackOfficeShortFormController.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/OriginMediaTitleListResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormDetailResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormListResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/mapper/BackOfficeShortFormMapper.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.javaapps/api-admin/src/main/java/com/ott/api_admin/upload/controller/.gitkeepapps/api-admin/src/main/java/com/ott/api_admin/upload/dto/.gitkeepapps/api-admin/src/main/java/com/ott/api_admin/upload/service/.gitkeepapps/api-user/build.gradleapps/transcoder/build.gradledocker-compose.ymlmodules/domain/src/main/java/com/ott/domain/contents/domain/Contents.javamodules/domain/src/main/java/com/ott/domain/media/domain/Media.javamodules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.javamodules/infra-db/build.gradlemodules/infra-db/src/main/resources/db/migration/V1__init.sqlmodules/infra-db/src/main/resources/db/migration/V2__media_table_inheritance.sqlmodules/infra-s3/build.gradlemodules/infra-s3/src/main/java/com/ott/infra/s3/config/S3PresignerConfig.javamodules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.javamodules/infra/src/main/java/com/ott/infra/db/.gitkeepmodules/infra/src/main/java/com/ott/infra/s3/.gitkeepsettings.gradle
|
Tip For best results, initiate chat on the files or code changes.
네, 이미 이번 PR 리뷰에서 위의 제 이전 댓글(두 번째 응답)에서 가이드라인을 기반으로 상세 리뷰를 제공했습니다: 📋 이전 리뷰 요약발견한 주요 이슈:
가이드라인 준수 확인:
🔧 자동 읽기 설정 방법 (재안내)
reviews:
path_instructions:
- path: "**/*"
instructions: |
리뷰 시작 전에 반드시 `coderabbit/coderabbit-guidelines.md` 파일을 읽고 그 내용을 기반으로 코드를 리뷰하세요.혹시 추가로 검토가 필요한 특정 부분이 있으시면 말씀해 주세요! 🙏 |
There was a problem hiding this comment.
♻️ Duplicate comments (10)
docker-compose.yml (1)
31-41:⚠️ Potential issue | 🟠 Majorapi-user에 S3 자격증명 주입은 과도합니다.
이전 리뷰와 동일 이슈입니다. api-user가 S3 프리사인/업로드에 관여하지 않으면 최소 권한 원칙 위반이므로 AWS 관련 환경변수를 제거하는 게 안전합니다.
🔧 제안 수정
AWS_REGION: ${AWS_REGION} AWS_S3_BUCKET: ${AWS_S3_BUCKET} - AWS_S3_PUBLIC_BASE_URL: ${AWS_S3_PUBLIC_BASE_URL} - AWS_S3_PRESIGN_EXPIRE_SECONDS: ${AWS_S3_PRESIGN_EXPIRE_SECONDS} - AWS_ACCESS_KEY_ID: ${AWS_ACCESS_KEY_ID} - AWS_SECRET_ACCESS_KEY: ${AWS_SECRET_ACCESS_KEY}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 31 - 41, The api-user service is exposing unnecessary AWS credentials and S3 settings; remove AWS-related environment variables from the api-user service in docker-compose.yml to follow least-privilege. Specifically, in the environment block for the api-user service (the block containing SPRING_DATASOURCE_URL / JWT_SECRET_BASE64), delete AWS_REGION, AWS_S3_BUCKET, AWS_S3_PUBLIC_BASE_URL, AWS_S3_PRESIGN_EXPIRE_SECONDS, AWS_ACCESS_KEY_ID, and AWS_SECRET_ACCESS_KEY; keep only the variables required by api-user (e.g., SPRING_* and JWT_SECRET_BASE64) and verify any other services that actually need S3 creds get them instead.modules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java (1)
61-64:⚠️ Potential issue | 🟡 Minor
originUrlnull 방어 코드 미적용 (이전 리뷰 미반영)
originUrl컬럼은nullable = false(Line 55)이므로 null이 전달될 경우 영속화 실패가 발생합니다. 이전 리뷰에서 지적된 사항이 아직 반영되지 않았으며,Media.updateImageKeys와 동일한 패턴으로 보호가 필요합니다.🛡️ 수정 제안
public void updateStorageKeys(String originUrl, String masterPlaylistUrl) { + Objects.requireNonNull(originUrl, "originUrl must not be null"); this.originUrl = originUrl; this.masterPlaylistUrl = masterPlaylistUrl; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java` around lines 61 - 64, ShortForm.updateStorageKeys currently assigns originUrl directly but originUrl is mapped nullable = false; modify updateStorageKeys (same pattern as Media.updateImageKeys) to guard against null by only updating this.originUrl when originUrl parameter is non-null/non-blank (leave existing value otherwise) and still update this.masterPlaylistUrl as before; ensure the method uses the same null-check semantics used in Media.updateImageKeys to prevent persistence errors.modules/domain/src/main/java/com/ott/domain/media/domain/Media.java (1)
66-69:⚠️ Potential issue | 🟡 Minor
posterUrlnull 방어 코드 미적용 (이전 리뷰 미반영)
posterUrl컬럼은nullable = false(Line 46)이므로 null이 전달되면 JPA 영속화 시 DB 제약 위반이 발생합니다. 이전 리뷰에서 지적된 사항이 아직 반영되지 않았습니다.🛡️ 수정 제안
public void updateImageKeys(String posterUrl, String thumbnailUrl) { + Objects.requireNonNull(posterUrl, "posterUrl must not be null"); this.posterUrl = posterUrl; this.thumbnailUrl = thumbnailUrl; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/domain/src/main/java/com/ott/domain/media/domain/Media.java` around lines 66 - 69, updateImageKeys 메서드에 posterUrl이 null로 들어오면 JPA 제약 위반이 발생하므로 이 메서드에서 posterUrl null 방어로직을 추가하세요: Media.updateImageKeys(String posterUrl, String thumbnailUrl) 진입부에서 posterUrl이 null인지 검사하고 null이면 IllegalArgumentException(또는 적절한 런타임 예외)을 던져 호출자에게 명확히 알리거나(선호) null 입력을 무시하도록 처리한 뒤 posterUrl과 thumbnailUrl을 할당하도록 수정하세요; posterUrl 필드는 nullable=false 제약이 있으니 posterUrl 관련 검증은 반드시 구현해야 합니다.modules/infra-s3/build.gradle (1)
4-4:⚠️ Potential issue | 🟠 Major
software.amazon.awssdk:s3:2.21.0는 여전히 취약한 버전입니다 — 업그레이드 필요이전 리뷰에서 지적된 사항이 아직 미반영 상태입니다.
2.21.0에 포함된 Netty 4.1.94.Final은 CVE-2025-67735(CRLF injection을 통한 HTTP 요청 위조)에 취약합니다. 현재 최신 버전은2.42.0입니다. AWS BOM 방식을 권장합니다.📦 수정 제안
루트
build.gradle의dependencyManagement에 AWS BOM 추가:dependencyManagement { imports { mavenBom "org.springframework.boot:spring-boot-dependencies:..." + mavenBom "software.amazon.awssdk:bom:2.42.0" } }
modules/infra-s3/build.gradle에서 버전 제거:- implementation 'software.amazon.awssdk:s3:2.21.0' + implementation 'software.amazon.awssdk:s3'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/infra-s3/build.gradle` at line 4, Add the AWS SDK BOM to the root dependencyManagement and stop pinning the S3 client version in modules/infra-s3/build.gradle: in the root build.gradle add the AWS Java SDK BOM (aws-java-sdk-bom or aws-sdk-java-v2 BOM for v2, set to the current 2.42.0-managed platform) under dependencyManagement so versions are centralized, then in modules/infra-s3/build.gradle change the dependency declaration implementation 'software.amazon.awssdk:s3:2.21.0' to implementation 'software.amazon.awssdk:s3' (remove the explicit version) so the BOM controls the version and the project will pick up the non-vulnerable 2.42.0 artifact.apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java (2)
22-23:⚠️ Potential issue | 🟠 Major[미해결]
Integer videoSize— 2GB 초과 파일 오버플로이전 리뷰에서 지적된 내용이 아직 반영되지 않았습니다.
Integer최대값(≈ 2.1 GB)을 초과하는 영상 파일에서 오버플로가 발생하고, 주석의 단위 표현도 여전히 모호합니다.🔧 제안 수정
- // 영상 크기(바이트 또는 내부 단위) - Integer videoSize, + // 영상 크기(바이트) + Long videoSize,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java` around lines 22 - 23, The DTO field Integer videoSize in ShortFormUploadRequest can overflow for files >2GB; change its type to Long (or long) and rename or clarify the field/comment to explicitly state bytes (e.g., videoSizeBytes) so the unit is unambiguous; update the corresponding getter/setter/constructor usages and any mapping/validation logic that references videoSize (e.g., getVideoSize()/setVideoSize()) to use Long and ensure upstream/downstream code and API docs accept/return a 64-bit size value.
9-31: [재확인]seriesId/contentsId상호 배타 검증 — 서비스 계층 위임 여부 확인이전 리뷰에서 DTO 수준 cross-field 검증 부재가 지적되었고, 개발자가 "서비스 코드에서 검증"한다고 답변하였습니다. 서비스 내 검증은
@Valid바인딩 에러 처리 흐름과 분리되어 있으므로, 잘못된 요청이 서비스 레이어까지 도달한 후 처리됩니다. DTO 수준@AssertTrue검증이 여전히 권장되나, 팀 컨벤션에 따라 서비스 검증으로 유지할 경우 해당 예외를 전역 예외 핸들러에서 일관되게 처리하는지 확인이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java` around lines 9 - 31, ShortFormUploadRequest currently lacks cross-field validation to ensure seriesId and contentsId are mutually exclusive; add a DTO-level boolean validator method (e.g., validateSeriesAndContentsExclusive) annotated with `@AssertTrue` and a clear message to enforce that exactly one (or at most one, per team rule) of seriesId and contentsId is present, or if you choose to keep validation in the service layer instead, confirm and document that ShortFormService performs the check and that the resulting exception is mapped to a consistent client error response by the global exception handler so requests are rejected before/consistently with `@Valid` binding errors.modules/domain/src/main/java/com/ott/domain/contents/domain/Contents.java (1)
57-60:⚠️ Potential issue | 🟡 Minor[미해결]
originUrlnull 허용 방지 미적용이전 리뷰에서 지적된 내용이 아직 반영되지 않았습니다.
originUrl은nullable = false컬럼이지만,updateStorageKeys에 null이 전달될 경우 DB 영속화 시점에야 오류가 발생합니다. 도메인 불변식 보호를 위해 메서드 입구에서 검증해야 합니다.🛡️ 제안 수정
public void updateStorageKeys(String originUrl, String masterPlaylistUrl) { + Objects.requireNonNull(originUrl, "originUrl must not be null"); this.originUrl = originUrl; this.masterPlaylistUrl = masterPlaylistUrl; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/domain/src/main/java/com/ott/domain/contents/domain/Contents.java` around lines 57 - 60, The updateStorageKeys method in class Contents allows originUrl to be set to null even though the DB column is nullable = false; add an explicit null check at the start of Contents.updateStorageKeys (e.g., use Objects.requireNonNull or throw an IllegalArgumentException/NullPointerException with a clear message) to enforce the domain invariant and fail fast before persistence; optionally also validate masterPlaylistUrl if needed, but at minimum guard originUrl and include a descriptive error message referencing originUrl in the exception.apps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.java (1)
22-23:⚠️ Potential issue | 🟠 Major[미해결]
Integer videoSize— 2GB 초과 파일 오버플로이전 리뷰에서 지적된 내용이 아직 반영되지 않았습니다.
ContentsUploadRequest와 도메인 엔티티Contents의videoSize모두Integer로 선언되어 있어, DTO만Long으로 수정할 경우 서비스 레이어에서 데이터 손실이 발생할 수 있습니다. DTO · 엔티티 · DB 컬럼을 함께Long(BIGINT)으로 일괄 변경해야 합니다.🔧 제안 수정 (DTO)
- // 영상 크기(바이트 또는 내부 단위) - Integer videoSize, + // 영상 크기(바이트) + Long videoSize,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.java` around lines 22 - 23, ContentsUploadRequest.videoSize and the domain field Contents.videoSize are still Integer, causing overflow for files >2GB; change the DTO field (ContentsUploadRequest.videoSize), the entity field (Contents.videoSize), their getters/setters and any mapping code to use Long (Java Long), update the database column type to BIGINT via a migration, and adjust service/repository code and any JSON serialization/deserialization or validation that assumes Integer to handle Long so end-to-end uses 64-bit sizes without truncation.modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java (1)
37-50:⚠️ Potential issue | 🟠 MajorPresign 예외를 BusinessException으로 래핑 필요.
AWS SDK 예외가 그대로 전파되면 버킷/리전 정보 등 민감한 메시지가 노출될 수 있습니다.
SdkException을 잡아 로깅 후 안전한 메시지로 변환하는 처리가 필요합니다.#!/bin/bash # ErrorCode 및 글로벌 예외 처리 패턴 확인 rg -n "enum ErrorCode" -g "*.java" rg -n "@ExceptionHandler" --type=java -C2 rg -n "S3PresignService" --type=java -C2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java` around lines 37 - 50, Wrap AWS SDK failures in createPutPresignedUrl inside a try/catch that catches SdkException, log the caught exception using the service logger, and throw a BusinessException with a safe error message and suitable ErrorCode; specifically, modify S3PresignService.createPutPresignedUrl to catch SdkException (from s3Presigner.presignPutObject and related calls), call logger.error(...) with the exception, and then throw new BusinessException(ErrorCode.S3_PRESIGN_FAILED, "Unable to generate upload URL") so sensitive SDK details (bucket/region) are not leaked.apps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.java (1)
172-191:⚠️ Potential issue | 🟡 Minor파일명 정규화 규칙이 다른 업로드 타입과 불일치합니다.
콘텐츠 업로드는 한글을 허용하는데 여기서는 비ASCII 제거로 동작이 달라집니다. 정책을 통일해 예측 가능하게 맞추는 게 좋습니다.
🛠️ 규칙 통일 예시(콘텐츠 업로드와 동일하게 맞추는 경우)
- String sanitizedName = namePart - .replace("/", "") - .replace("\\", "") - .replaceAll("[^0-9A-Za-z_-]", ""); + String sanitizedName = namePart + .replace("/", "") + .replace("\\", "") + .replaceAll("[^0-9A-Za-z가-힣_-]", "");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.java` around lines 172 - 191, sanitizeFileName currently strips all non-ASCII characters (removing Korean) which is inconsistent with content upload rules; update the sanitization regexes in sanitizeFileName to allow Unicode letters and digits (e.g. use character classes like \p{L} and 0-9) instead of ASCII-only ranges so namePart.replaceAll("[^0-9A-Za-z_-]", "") becomes something like .replaceAll("[^\\p{L}0-9_-]", "") and extPart.replaceAll("[^0-9A-Za-z]", "") becomes .replaceAll("[^\\p{L}0-9]", "") (keep existing trimming, default "file" fallback, and BusinessException(ErrorCode.INVALID_INPUT) behavior for empty extension), and add/adjust unit tests to match the content-upload normalization examples.
🧹 Nitpick comments (3)
modules/infra-s3/src/main/java/com/ott/infra/s3/config/S3PresignerConfig.java (1)
14-19: 로컬 개발 환경(LocalStack)을 위한 엔드포인트 오버라이드 미지원현재 구성에서는
endpointOverride없이 AWS 공식 S3 엔드포인트만 사용합니다. LocalStack 등 로컬 모킹 환경에서 통합 테스트 시 presigner가 동작하지 않습니다. 프로퍼티로 선택적 엔드포인트 오버라이드를 지원하면 로컬 개발 편의성이 향상됩니다.참고로,
@Bean메서드에서 반환된S3Presigner는AutoCloseable을 구현하므로 Spring이 컨텍스트 종료 시 자동으로close()를 호출합니다. 별도@PreDestroy나destroyMethod지정은 불필요합니다.♻️ 제안 수정
+import java.net.URI; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; `@Bean` -public S3Presigner s3Presigner(`@Value`("${aws.region:ap-northeast-2}") String region) { - return S3Presigner.builder() - .region(Region.of(region)) - .credentialsProvider(DefaultCredentialsProvider.create()) - .build(); +public S3Presigner s3Presigner( + `@Value`("${aws.region:ap-northeast-2}") String region, + `@Value`("${aws.s3.endpoint-override:}") String endpointOverride +) { + S3Presigner.Builder builder = S3Presigner.builder() + .region(Region.of(region)) + .credentialsProvider(DefaultCredentialsProvider.create()); + if (!endpointOverride.isBlank()) { + builder.endpointOverride(URI.create(endpointOverride)); + } + return builder.build(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/infra-s3/src/main/java/com/ott/infra/s3/config/S3PresignerConfig.java` around lines 14 - 19, S3PresignerConfig's s3Presigner(...) currently always uses the AWS public endpoint; change s3Presigner to accept an optional endpoint property (e.g. `@Value`("${aws.s3.endpoint:}") String endpoint) and, when endpoint is non-empty, call S3Presigner.builder().endpointOverride(URI.create(endpoint)) in the builder chain before build(); keep Region.of(region) and DefaultCredentialsProvider.create() as-is and do not add explicit close/destroy logic since S3Presigner implements AutoCloseable and Spring will close it.apps/api-admin/src/main/java/com/ott/api_admin/series/dto/response/SeriesUploadResponse.java (1)
6-17: 각 필드에@Schema어노테이션 추가를 고려해 주세요.현재 클래스 레벨
@Schema만 적용되어 있어 Swagger/OpenAPI 문서에서 개별 필드 설명이 표시되지 않습니다. 인라인 주석을@Schema(description = "...")어노테이션으로 변환하면 API 문서 품질이 향상됩니다.♻️ 필드별 `@Schema` 어노테이션 적용 예시
`@Schema`(description = "시리즈 업로드 응답") public record SeriesUploadResponse( - // 생성된 시리즈 ID - Long seriesId, - // 포스터 S3 object key - String posterObjectKey, - // 썸네일 S3 object key - String thumbnailObjectKey, - // 포스터 업로드용 Presigned URL - String posterUploadUrl, - // 썸네일 업로드용 Presigned URL - String thumbnailUploadUrl + `@Schema`(description = "생성된 시리즈 ID") Long seriesId, + `@Schema`(description = "포스터 S3 object key") String posterObjectKey, + `@Schema`(description = "썸네일 S3 object key") String thumbnailObjectKey, + `@Schema`(description = "포스터 업로드용 Presigned URL") String posterUploadUrl, + `@Schema`(description = "썸네일 업로드용 Presigned URL") String thumbnailUploadUrl ) { }As per coding guidelines,
**/*.java: "record DTOs with@Schema" 규칙을 참고합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/series/dto/response/SeriesUploadResponse.java` around lines 6 - 17, Add per-field `@Schema` annotations to the SeriesUploadResponse record components so each field has an explicit OpenAPI description: annotate seriesId, posterObjectKey, thumbnailObjectKey, posterUploadUrl and thumbnailUploadUrl with `@Schema`(description = "...") using the same text as the existing inline comments, import io.swagger.v3.oas.annotations.media.Schema, and remove or replace the inline comments so Swagger shows the field-level descriptions for the SeriesUploadResponse record.apps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.java (1)
1-23: 요청 DTO 구현이 적절합니다.검증 어노테이션(
@NotBlank,@NotNull)이 모든 필수 필드에 올바르게 적용되어 있습니다. 응답 DTO와 마찬가지로 필드별@Schema어노테이션 추가를 고려해 주세요.한 가지 참고 사항: 파일명 필드(
posterFileName,thumbnailFileName)에@Size(max = ...)제약을 추가하면 S3 오브젝트 키 길이 제한(1024바이트) 초과를 사전에 방지할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.java` around lines 1 - 23, Add a max-length validation on the file-name fields to prevent S3 key length issues and add field-level OpenAPI descriptions: annotate posterFileName and thumbnailFileName in SeriesUploadRequest with `@Size`(max = 1024) (import jakarta.validation.constraints.Size) and add `@Schema`(...) annotations for the fields you want to document; update imports to include Size and ensure the existing `@NotBlank` remains on posterFileName and thumbnailFileName so both constraints apply.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.java`:
- Around line 22-23: ContentsUploadRequest.videoSize and the domain field
Contents.videoSize are still Integer, causing overflow for files >2GB; change
the DTO field (ContentsUploadRequest.videoSize), the entity field
(Contents.videoSize), their getters/setters and any mapping code to use Long
(Java Long), update the database column type to BIGINT via a migration, and
adjust service/repository code and any JSON serialization/deserialization or
validation that assumes Integer to handle Long so end-to-end uses 64-bit sizes
without truncation.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.java`:
- Around line 172-191: sanitizeFileName currently strips all non-ASCII
characters (removing Korean) which is inconsistent with content upload rules;
update the sanitization regexes in sanitizeFileName to allow Unicode letters and
digits (e.g. use character classes like \p{L} and 0-9) instead of ASCII-only
ranges so namePart.replaceAll("[^0-9A-Za-z_-]", "") becomes something like
.replaceAll("[^\\p{L}0-9_-]", "") and extPart.replaceAll("[^0-9A-Za-z]", "")
becomes .replaceAll("[^\\p{L}0-9]", "") (keep existing trimming, default "file"
fallback, and BusinessException(ErrorCode.INVALID_INPUT) behavior for empty
extension), and add/adjust unit tests to match the content-upload normalization
examples.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java`:
- Around line 22-23: The DTO field Integer videoSize in ShortFormUploadRequest
can overflow for files >2GB; change its type to Long (or long) and rename or
clarify the field/comment to explicitly state bytes (e.g., videoSizeBytes) so
the unit is unambiguous; update the corresponding getter/setter/constructor
usages and any mapping/validation logic that references videoSize (e.g.,
getVideoSize()/setVideoSize()) to use Long and ensure upstream/downstream code
and API docs accept/return a 64-bit size value.
- Around line 9-31: ShortFormUploadRequest currently lacks cross-field
validation to ensure seriesId and contentsId are mutually exclusive; add a
DTO-level boolean validator method (e.g., validateSeriesAndContentsExclusive)
annotated with `@AssertTrue` and a clear message to enforce that exactly one (or
at most one, per team rule) of seriesId and contentsId is present, or if you
choose to keep validation in the service layer instead, confirm and document
that ShortFormService performs the check and that the resulting exception is
mapped to a consistent client error response by the global exception handler so
requests are rejected before/consistently with `@Valid` binding errors.
In `@docker-compose.yml`:
- Around line 31-41: The api-user service is exposing unnecessary AWS
credentials and S3 settings; remove AWS-related environment variables from the
api-user service in docker-compose.yml to follow least-privilege. Specifically,
in the environment block for the api-user service (the block containing
SPRING_DATASOURCE_URL / JWT_SECRET_BASE64), delete AWS_REGION, AWS_S3_BUCKET,
AWS_S3_PUBLIC_BASE_URL, AWS_S3_PRESIGN_EXPIRE_SECONDS, AWS_ACCESS_KEY_ID, and
AWS_SECRET_ACCESS_KEY; keep only the variables required by api-user (e.g.,
SPRING_* and JWT_SECRET_BASE64) and verify any other services that actually need
S3 creds get them instead.
In `@modules/domain/src/main/java/com/ott/domain/contents/domain/Contents.java`:
- Around line 57-60: The updateStorageKeys method in class Contents allows
originUrl to be set to null even though the DB column is nullable = false; add
an explicit null check at the start of Contents.updateStorageKeys (e.g., use
Objects.requireNonNull or throw an IllegalArgumentException/NullPointerException
with a clear message) to enforce the domain invariant and fail fast before
persistence; optionally also validate masterPlaylistUrl if needed, but at
minimum guard originUrl and include a descriptive error message referencing
originUrl in the exception.
In `@modules/domain/src/main/java/com/ott/domain/media/domain/Media.java`:
- Around line 66-69: updateImageKeys 메서드에 posterUrl이 null로 들어오면 JPA 제약 위반이 발생하므로
이 메서드에서 posterUrl null 방어로직을 추가하세요: Media.updateImageKeys(String posterUrl,
String thumbnailUrl) 진입부에서 posterUrl이 null인지 검사하고 null이면
IllegalArgumentException(또는 적절한 런타임 예외)을 던져 호출자에게 명확히 알리거나(선호) null 입력을 무시하도록
처리한 뒤 posterUrl과 thumbnailUrl을 할당하도록 수정하세요; posterUrl 필드는 nullable=false 제약이 있으니
posterUrl 관련 검증은 반드시 구현해야 합니다.
In
`@modules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java`:
- Around line 61-64: ShortForm.updateStorageKeys currently assigns originUrl
directly but originUrl is mapped nullable = false; modify updateStorageKeys
(same pattern as Media.updateImageKeys) to guard against null by only updating
this.originUrl when originUrl parameter is non-null/non-blank (leave existing
value otherwise) and still update this.masterPlaylistUrl as before; ensure the
method uses the same null-check semantics used in Media.updateImageKeys to
prevent persistence errors.
In `@modules/infra-s3/build.gradle`:
- Line 4: Add the AWS SDK BOM to the root dependencyManagement and stop pinning
the S3 client version in modules/infra-s3/build.gradle: in the root build.gradle
add the AWS Java SDK BOM (aws-java-sdk-bom or aws-sdk-java-v2 BOM for v2, set to
the current 2.42.0-managed platform) under dependencyManagement so versions are
centralized, then in modules/infra-s3/build.gradle change the dependency
declaration implementation 'software.amazon.awssdk:s3:2.21.0' to implementation
'software.amazon.awssdk:s3' (remove the explicit version) so the BOM controls
the version and the project will pick up the non-vulnerable 2.42.0 artifact.
In
`@modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java`:
- Around line 37-50: Wrap AWS SDK failures in createPutPresignedUrl inside a
try/catch that catches SdkException, log the caught exception using the service
logger, and throw a BusinessException with a safe error message and suitable
ErrorCode; specifically, modify S3PresignService.createPutPresignedUrl to catch
SdkException (from s3Presigner.presignPutObject and related calls), call
logger.error(...) with the exception, and then throw new
BusinessException(ErrorCode.S3_PRESIGN_FAILED, "Unable to generate upload URL")
so sensitive SDK details (bucket/region) are not leaked.
---
Nitpick comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.java`:
- Around line 1-23: Add a max-length validation on the file-name fields to
prevent S3 key length issues and add field-level OpenAPI descriptions: annotate
posterFileName and thumbnailFileName in SeriesUploadRequest with `@Size`(max =
1024) (import jakarta.validation.constraints.Size) and add `@Schema`(...)
annotations for the fields you want to document; update imports to include Size
and ensure the existing `@NotBlank` remains on posterFileName and
thumbnailFileName so both constraints apply.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/series/dto/response/SeriesUploadResponse.java`:
- Around line 6-17: Add per-field `@Schema` annotations to the
SeriesUploadResponse record components so each field has an explicit OpenAPI
description: annotate seriesId, posterObjectKey, thumbnailObjectKey,
posterUploadUrl and thumbnailUploadUrl with `@Schema`(description = "...") using
the same text as the existing inline comments, import
io.swagger.v3.oas.annotations.media.Schema, and remove or replace the inline
comments so Swagger shows the field-level descriptions for the
SeriesUploadResponse record.
In
`@modules/infra-s3/src/main/java/com/ott/infra/s3/config/S3PresignerConfig.java`:
- Around line 14-19: S3PresignerConfig's s3Presigner(...) currently always uses
the AWS public endpoint; change s3Presigner to accept an optional endpoint
property (e.g. `@Value`("${aws.s3.endpoint:}") String endpoint) and, when endpoint
is non-empty, call S3Presigner.builder().endpointOverride(URI.create(endpoint))
in the builder chain before build(); keep Region.of(region) and
DefaultCredentialsProvider.create() as-is and do not add explicit close/destroy
logic since S3Presigner implements AutoCloseable and Spring will close it.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
apps/api-admin/build.gradleapps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.javaapps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsController.javaapps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/content/dto/response/ContentsUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/content/mapper/BackOfficeContentsMapper.javaapps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.javaapps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.javaapps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesController.javaapps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/series/dto/response/SeriesUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/series/mapper/BackOfficeSeriesMapper.javaapps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/controller/BackOfficeShortFormApi.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/controller/BackOfficeShortFormController.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/OriginMediaTitleListResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormDetailResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormListResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/mapper/BackOfficeShortFormMapper.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.javaapps/api-admin/src/main/java/com/ott/api_admin/upload/controller/.gitkeepapps/api-admin/src/main/java/com/ott/api_admin/upload/dto/.gitkeepapps/api-admin/src/main/java/com/ott/api_admin/upload/service/.gitkeepapps/api-user/build.gradleapps/transcoder/build.gradledocker-compose.ymlmodules/domain/src/main/java/com/ott/domain/contents/domain/Contents.javamodules/domain/src/main/java/com/ott/domain/media/domain/Media.javamodules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.javamodules/infra-db/build.gradlemodules/infra-db/src/main/resources/db/migration/V1__init.sqlmodules/infra-db/src/main/resources/db/migration/V2__media_table_inheritance.sqlmodules/infra-s3/build.gradlemodules/infra-s3/src/main/java/com/ott/infra/s3/config/S3PresignerConfig.javamodules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.javamodules/infra/src/main/java/com/ott/infra/db/.gitkeepmodules/infra/src/main/java/com/ott/infra/s3/.gitkeepsettings.gradle
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java (2)
169-252: 헬퍼 메서드 중복 구현 — 이전 리뷰에서 이미 제기된 P1 이슈
buildObjectKey,resolveContentType,resolveOriginContentType,sanitizeFileName,resolveUploader가BackOfficeSeriesService와 동일하게 구현되어 있습니다.BackOfficeShortFormService에도 동일한 패턴이 반복될 것으로 예상됩니다. PR 리뷰 토론에서 이미 P1로 지적된 사항입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java` around lines 169 - 252, These helper methods are duplicated across services; extract buildObjectKey, resolveContentType, resolveOriginContentType, sanitizeFileName, and resolveUploader into a shared utility/component (e.g., ContentUtils or BackOfficeHelper) and update BackOfficeContentsService (and BackOfficeSeriesService / BackOfficeShortFormService) to call the centralized methods instead; ensure resolveUploader depends on a single injectable memberRepository or a SecurityUtils wrapper that the services can reuse, keep method signatures identical, move existing validation/error behavior (BusinessException/ErrorCode) to the shared class, and update imports/DI where needed.
208-219:sanitizeFileName정규식이BackOfficeSeriesService와 불일치 — 이전 리뷰에서 이미 제기된 P1 이슈이 서비스의 정규식은
[^0-9A-Za-z가-힣_-](한글 허용)인 반면,BackOfficeSeriesService는[^0-9A-Za-z_-](한글 불허)를 사용합니다. 콘텐츠 타입에 따라 파일명 허용 범위가 달라지는 것은 의도치 않은 동작을 유발할 수 있습니다. PR 리뷰 토론에서 이미 P1로 지적된 사항입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java` around lines 208 - 219, The sanitizeFileName method uses a different regex (allows Korean: [^0-9A-Za-z가-힣_-]) than BackOfficeSeriesService ([^0-9A-Za-z_-]), causing inconsistent filename validation; fix by standardizing the policy—extract a shared constant (e.g., FILENAME_SANITIZE_REGEX / FILENAME_ALLOWED_CHARS) and use that constant in sanitizeFileName and BackOfficeSeriesService so both services use the exact same regex for namePart and extPart sanitization (or update both to the agreed policy of allowing or disallowing 한글 consistently).
🧹 Nitpick comments (4)
apps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.java (1)
8-30: 필드별@Schema어노테이션 미적용클래스 레벨
@Schema는 있으나 각 필드에description어노테이션이 없어 Swagger UI에서 파라미터 설명이 표시되지 않습니다. 코딩 가이드라인의 DTO 문서화 기준(DTOs as record +@Schema``)에 맞게 필드별@Schema(description = "...")추가를 고려해 주세요.As per coding guidelines,
DTOs as record +@Schema`` 규칙에 따라 필드 수준의@Schema적용이 권장됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.java` around lines 8 - 30, The record ContentsUploadRequest is missing field-level Swagger descriptions; add `@Schema`(description = "...") to each record component (seriesId, title, description, actors, publicStatus, duration, videoSize, posterFileName, thumbnailFileName, originFileName) so Swagger UI shows parameter descriptions, keeping existing validation annotations like `@NotBlank/`@NotNull intact and using concise descriptions matching the class-level Korean labels.apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java (1)
99-157: 파일 형식 유효성 검사가 DB 저장 이후에 실행됨
resolveContentType/resolveOriginContentType은 lines 154–156에서 호출되지만,Media와Contents레코드는 이미 lines 103–131에서 저장되었습니다. 지원하지 않는 확장자가 들어오면 예외로 트랜잭션이 롤백되어 데이터 오염은 없지만, 입력 검증은 DB 쓰기 전에 수행되어야 불필요한 INSERT를 방지할 수 있습니다.♻️ 제안 수정 — 유효성 검사를 저장 전으로 이동
String sanitizedPosterFileName = sanitizeFileName(request.posterFileName()); String sanitizedThumbnailFileName = sanitizeFileName(request.thumbnailFileName()); String sanitizedOriginFileName = sanitizeFileName(request.originFileName()); + // 지원하지 않는 확장자는 DB 저장 전에 차단합니다. + String posterContentType = resolveContentType(sanitizedPosterFileName); + String thumbContentType = resolveContentType(sanitizedThumbnailFileName); + String originContentType = resolveOriginContentType(sanitizedOriginFileName); + Media media = mediaRepository.save( ... ); Contents contents = contentsRepository.save( ... ); ... return backOfficeContentsMapper.toContentsUploadResponse( contentsId, posterObjectKey, thumbnailObjectKey, originObjectKey, masterPlaylistObjectKey, - s3PresignService.createPutPresignedUrl(posterObjectKey, resolveContentType(sanitizedPosterFileName)), - s3PresignService.createPutPresignedUrl(thumbnailObjectKey, resolveContentType(sanitizedThumbnailFileName)), - s3PresignService.createPutPresignedUrl(originObjectKey, resolveOriginContentType(sanitizedOriginFileName)) + s3PresignService.createPutPresignedUrl(posterObjectKey, posterContentType), + s3PresignService.createPutPresignedUrl(thumbnailObjectKey, thumbContentType), + s3PresignService.createPutPresignedUrl(originObjectKey, originContentType) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java` around lines 99 - 157, The content-type validation (calls to resolveContentType and resolveOriginContentType) must run before persisting Media and Contents to avoid needless DB writes; validate sanitizedPosterFileName, sanitizedThumbnailFileName, and sanitizedOriginFileName right after sanitizeFileName(...) and before calling mediaRepository.save(...) and contentsRepository.save(...), throwing a suitable exception on invalid types so the subsequent calls to Media.builder()/Contents.builder(), mediaRepository.save, and contentsRepository.save only occur when file types are valid; keep the later usage of these filenames/object keys (buildObjectKey, s3PresignService.createPutPresignedUrl, s3PresignService.toObjectUrl, media.updateImageKeys, contents.updateStorageKeys) unchanged.apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java (1)
9-30: [선택] 개별 필드에@Schema설명 추가 권장클래스 레벨
@Schema는 있지만 각 필드에description/example이 없어 Swagger UI에서 필드 의미 파악이 어렵습니다. 특히seriesId/contentsId의 상호 배타 조건,duration의 단위(초),videoSize의 단위(KB)는 문서화가 유용합니다.✏️ 예시
- // 연결할 시리즈 ID (선택) - Long seriesId, - // 연결할 콘텐츠 ID (선택) - Long contentsId, + `@Schema`(description = "연결할 시리즈 ID (seriesId/contentsId 중 하나만 필수)", example = "1") + Long seriesId, + `@Schema`(description = "연결할 콘텐츠 ID (seriesId/contentsId 중 하나만 필수)", example = "1") + Long contentsId, // 숏폼 제목 `@NotBlank` String title, ... - // 영상 길이(초) - Integer duration, - // 영상 크기(KB) - Integer videoSize, + `@Schema`(description = "영상 길이 (초)", example = "60") + Integer duration, + `@Schema`(description = "영상 크기 (KB)", example = "128") + Integer videoSize,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java` around lines 9 - 30, Add per-field Swagger `@Schema` annotations to the ShortFormUploadRequest record so each property has a clear description and example: annotate seriesId and contentsId with descriptions noting they are optional and mutually exclusive, annotate duration with "length in seconds" and an example, annotate videoSize with "size in KB" and an example, and add description/examples for title, description, posterFileName, thumbnailFileName, and originFileName; keep the existing class-level `@Schema` and place annotations on the record components in ShortFormUploadRequest to improve generated Swagger UI docs.apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java (1)
295-315: [선택]resolveUploader()의SecurityContextHolder직접 접근 방식 비일관성
getShortFormList,getShortFormDetail은Authentication을 파라미터로 받지만,createShortFormUpload는resolveUploader()내부에서SecurityContextHolder를 직접 호출합니다. 일관성을 위해 컨트롤러에서Authentication을 받아 전달하는 방식으로 통일하는 것을 권장합니다. 테스트 시에도 SecurityContext 설정 없이Member객체를 주입할 수 있어 테스트 가능성이 높아집니다.♻️ 제안 수정
- `@Transactional` - public ShortFormUploadResponse createShortFormUpload(ShortFormUploadRequest request) { + `@Transactional` + public ShortFormUploadResponse createShortFormUpload(ShortFormUploadRequest request, Authentication authentication) { validateExclusiveTarget(request.seriesId(), request.contentsId()); - Member uploader = resolveUploader(); + Long memberId = (Long) authentication.getPrincipal(); + Member uploader = memberRepository.findById(memberId) + .orElseThrow(() -> new BusinessException(ErrorCode.UNAUTHORIZED)); ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java` around lines 295 - 315, The resolveUploader() method currently reads Authentication from SecurityContextHolder directly; change it to accept an Authentication parameter (e.g., resolveUploader(Authentication authentication)) and move the existing null/anonymous/parse/lookup logic into that method, then update createShortFormUpload to receive an Authentication argument from the controller and pass it into resolveUploader; also update any other callers (or add an overloaded helper) so getShortFormList and getShortFormDetail remain consistent in using the controller-provided Authentication; reference symbols: resolveUploader(), createShortFormUpload, getShortFormList, getShortFormDetail, SecurityContextHolder, Authentication, Member, memberRepository.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.java`:
- Around line 21-23: ContentsUploadRequest currently declares Integer duration
and Integer videoSize without validation, allowing negative values; add
validation annotations (e.g., `@Min`(0) or `@PositiveOrZero`) to the duration and
videoSize fields in class ContentsUploadRequest so requests with negative values
are rejected, and add the corresponding import(s)
(javax.validation.constraints.Min or
javax.validation.constraints.PositiveOrZero) to the file; ensure the annotations
are placed on the fields (duration, videoSize) used by the request DTO and that
any validation framework is enabled in the controller pipeline.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java`:
- Around line 239-315: Several helper methods (sanitizeFileName,
resolveContentType, resolveOriginContentType, buildObjectKey, resolveUploader)
are duplicated across BackOfficeShortFormService, BackOfficeSeriesService, and
BackOfficeContentsService; extract them into a new Spring component (e.g.,
UploadSupportService) that has required deps (MemberRepository, S3PresignService
if needed), move the implementations for sanitizeFileName, resolveContentType,
resolveOriginContentType, buildObjectKey, and resolveUploader into that class,
then inject UploadSupportService into each BackOffice*Service and replace local
calls with calls to the new component (e.g.,
uploadSupportService.sanitizeFileName(...),
uploadSupportService.resolveUploader(), etc.), removing the duplicate private
methods from the BackOffice*Service classes.
---
Duplicate comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java`:
- Around line 169-252: These helper methods are duplicated across services;
extract buildObjectKey, resolveContentType, resolveOriginContentType,
sanitizeFileName, and resolveUploader into a shared utility/component (e.g.,
ContentUtils or BackOfficeHelper) and update BackOfficeContentsService (and
BackOfficeSeriesService / BackOfficeShortFormService) to call the centralized
methods instead; ensure resolveUploader depends on a single injectable
memberRepository or a SecurityUtils wrapper that the services can reuse, keep
method signatures identical, move existing validation/error behavior
(BusinessException/ErrorCode) to the shared class, and update imports/DI where
needed.
- Around line 208-219: The sanitizeFileName method uses a different regex
(allows Korean: [^0-9A-Za-z가-힣_-]) than BackOfficeSeriesService
([^0-9A-Za-z_-]), causing inconsistent filename validation; fix by standardizing
the policy—extract a shared constant (e.g., FILENAME_SANITIZE_REGEX /
FILENAME_ALLOWED_CHARS) and use that constant in sanitizeFileName and
BackOfficeSeriesService so both services use the exact same regex for namePart
and extPart sanitization (or update both to the agreed policy of allowing or
disallowing 한글 consistently).
---
Nitpick comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.java`:
- Around line 8-30: The record ContentsUploadRequest is missing field-level
Swagger descriptions; add `@Schema`(description = "...") to each record component
(seriesId, title, description, actors, publicStatus, duration, videoSize,
posterFileName, thumbnailFileName, originFileName) so Swagger UI shows parameter
descriptions, keeping existing validation annotations like `@NotBlank/`@NotNull
intact and using concise descriptions matching the class-level Korean labels.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java`:
- Around line 99-157: The content-type validation (calls to resolveContentType
and resolveOriginContentType) must run before persisting Media and Contents to
avoid needless DB writes; validate sanitizedPosterFileName,
sanitizedThumbnailFileName, and sanitizedOriginFileName right after
sanitizeFileName(...) and before calling mediaRepository.save(...) and
contentsRepository.save(...), throwing a suitable exception on invalid types so
the subsequent calls to Media.builder()/Contents.builder(),
mediaRepository.save, and contentsRepository.save only occur when file types are
valid; keep the later usage of these filenames/object keys (buildObjectKey,
s3PresignService.createPutPresignedUrl, s3PresignService.toObjectUrl,
media.updateImageKeys, contents.updateStorageKeys) unchanged.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java`:
- Around line 9-30: Add per-field Swagger `@Schema` annotations to the
ShortFormUploadRequest record so each property has a clear description and
example: annotate seriesId and contentsId with descriptions noting they are
optional and mutually exclusive, annotate duration with "length in seconds" and
an example, annotate videoSize with "size in KB" and an example, and add
description/examples for title, description, posterFileName, thumbnailFileName,
and originFileName; keep the existing class-level `@Schema` and place annotations
on the record components in ShortFormUploadRequest to improve generated Swagger
UI docs.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.java`:
- Around line 295-315: The resolveUploader() method currently reads
Authentication from SecurityContextHolder directly; change it to accept an
Authentication parameter (e.g., resolveUploader(Authentication authentication))
and move the existing null/anonymous/parse/lookup logic into that method, then
update createShortFormUpload to receive an Authentication argument from the
controller and pass it into resolveUploader; also update any other callers (or
add an overloaded helper) so getShortFormList and getShortFormDetail remain
consistent in using the controller-provided Authentication; reference symbols:
resolveUploader(), createShortFormUpload, getShortFormList, getShortFormDetail,
SecurityContextHolder, Authentication, Member, memberRepository.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.javaapps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.javamodules/domain/src/main/java/com/ott/domain/contents/domain/Contents.javamodules/domain/src/main/java/com/ott/domain/media/domain/Media.javamodules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.javamodules/infra-s3/build.gradlemodules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java
🚧 Files skipped from review as they are similar to previous changes (2)
- modules/infra-s3/build.gradle
- modules/infra-s3/src/main/java/com/ott/infra/s3/service/S3PresignService.java
phonil
reviewed
Feb 24, 2026
yubin012
reviewed
Feb 25, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
modules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java (1)
62-64:⚠️ Potential issue | 🟡 Minor
originUrl에 null 방어를 다시 넣어주세요.Line 63에서
nullable = false필드에 null이 들어올 수 있어 flush 시점에 DB 제약 오류로 늦게 실패합니다. 메서드 진입 시 즉시 검증하는 게 안전합니다.수정 예시
public void updateStorageKeys(String originUrl, String masterPlaylistUrl) { - this.originUrl = originUrl; + this.originUrl = Objects.requireNonNull(originUrl, "originUrl must not be null"); this.masterPlaylistUrl = masterPlaylistUrl; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java` around lines 62 - 64, In ShortForm.updateStorageKeys, reintroduce a null guard for the originUrl parameter (the field is non-nullable) by validating originUrl at method entry and throwing a clear runtime exception (e.g., IllegalArgumentException or NullPointerException with a descriptive message) if it's null, before assigning this.originUrl and this.masterPlaylistUrl; keep the existing assignment to masterPlaylistUrl intact and ensure the validation uses the method name updateStorageKeys to locate the change.modules/domain/src/main/java/com/ott/domain/contents/domain/Contents.java (1)
59-61:⚠️ Potential issue | 🟡 Minor
originUrl대입 전에 null 검증이 필요합니다.Line 60은
nullable = false컬럼에 대한 직접 대입이라 도메인 상태가 깨질 수 있습니다. null은 여기서 바로 차단해 주세요.수정 예시
public void updateStorageKeys(String originUrl, String masterPlaylistUrl) { - this.originUrl = originUrl; + this.originUrl = Objects.requireNonNull(originUrl, "originUrl must not be null"); this.masterPlaylistUrl = masterPlaylistUrl; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/domain/src/main/java/com/ott/domain/contents/domain/Contents.java` around lines 59 - 61, Ensure the domain invariant by validating originUrl before assignment in Contents.updateStorageKeys: check that originUrl is not null (and optionally not blank) and throw an unchecked exception (e.g., IllegalArgumentException or NullPointerException) with a clear message if it is null, then assign this.originUrl and this.masterPlaylistUrl; reference the Contents class, updateStorageKeys method, and the originUrl/masterPlaylistUrl fields to locate where to add the guard.modules/domain/src/main/java/com/ott/domain/media/domain/Media.java (1)
68-70:⚠️ Potential issue | 🟡 Minor
posterUrl도 null 방어를 복원해 주세요.Line 69에서
posterUrl을 그대로 대입하면nullable = false제약과 충돌할 수 있습니다. 도메인 메서드에서 선검증이 필요합니다.수정 예시
public void updateImageKeys(String posterUrl, String thumbnailUrl) { - this.posterUrl = posterUrl; + this.posterUrl = Objects.requireNonNull(posterUrl, "posterUrl must not be null"); this.thumbnailUrl = thumbnailUrl; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/domain/src/main/java/com/ott/domain/media/domain/Media.java` around lines 68 - 70, updateImageKeys에서 posterUrl을 그대로 대입하면 엔티티의 nullable = false 제약을 위반할 수 있으니 posterUrl에 대한 null 방어를 복원하세요: updateImageKeys(String posterUrl, String thumbnailUrl) 내부에서 posterUrl이 null이면 현재 객체 상태를 변경하지 않거나(선호 시) IllegalArgumentException을 던져 호출자에게 선검증 실패를 알리도록 처리하고, 유효한 posterUrl인 경우에만 this.posterUrl = posterUrl;을 수행하세요; thumbnailUrl는 기존 의도대로 처리하되 posterUrl 관련 유효성 검사 로직은 반드시 포함시켜 주세요.apps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.java (1)
87-88:⚠️ Potential issue | 🟡 MinorSwagger RequestBody 설명 문구를 스키마 기준으로 통일해주세요.
현재 설명이 내부 패키지 경로를 노출해서 API 문서 가독성이 떨어집니다. 스키마 기준 표현으로 맞추는 편이 좋습니다.
✏️ 제안 수정안
- `@Parameter`(description = "api_admin.content.dto.request.ContentsUploadRequest를 참고해주세요.", required = true) + `@Parameter`(description = "Schemas의 ContentsUploadRequest를 참고해주세요.", required = true) `@RequestBody` ContentsUploadRequest request🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.java` around lines 87 - 88, The `@Parameter` description on the ContentsUploadRequest parameter exposes the internal package path; update the annotation in BackOfficeContentsApi so the description uses a schema-based phrase (e.g., "ContentsUploadRequest 스키마를 참고해주세요." or "요청 바디는 ContentsUploadRequest 스키마를 따릅니다.") instead of the full package path, keeping the `@RequestBody` ContentsUploadRequest request and required = true unchanged; adjust the text in the `@Parameter`(...) on the method parameter to the new schema-focused wording.
🧹 Nitpick comments (3)
apps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.java (1)
111-114: 인터페이스 시그니처에도@Valid추가를 고려하세요.구현체
BackOfficeSeriesController에서 이미@Valid@RequestBody``로 검증이 실행되고 있으므로 현재 기능상 문제는 없습니다. 다만 프로젝트 내 일부 인터페이스(AdminAuthApi등)는 인터페이스 시그니처에 `@Valid`를 포함하고 있어, 일관성 차원에서 이 인터페이스에도 추가하면 API 계약을 더 명확히 할 수 있습니다.제안 diff
+import jakarta.validation.Valid; import org.springframework.web.bind.annotation.RequestBody; @@ ResponseEntity<SuccessResponse<SeriesUploadResponse>> createSeriesUpload( `@Parameter`(description = "api_admin.series.dto.request.SeriesUploadRequest를 참고해주세요.", required = true) - `@RequestBody` SeriesUploadRequest request + `@Valid` `@RequestBody` SeriesUploadRequest request );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.java` around lines 111 - 114, Add the `@Valid` annotation to the interface method signature for BackOfficeSeriesApi#createSeriesUpload so the contract explicitly enforces validation on the RequestBody type (SeriesUploadRequest); update the method parameter from `@RequestBody` SeriesUploadRequest request to `@Valid` `@RequestBody` SeriesUploadRequest request to match the implementation in BackOfficeSeriesController and maintain consistency with other APIs like AdminAuthApi.apps/api-admin/src/main/java/com/ott/api_admin/upload/support/ExtensionEnum.java (1)
3-4: 확장자 조회를 정적 맵으로 캐시하면 호출 비용을 줄일 수 있습니다.현재는 매번
values()순회 + stream 생성이 발생합니다. enum 고정 데이터이므로 정적 맵 조회로 단순화하는 편이 더 효율적입니다.리팩터 예시
import java.util.Arrays; import java.util.Locale; +import java.util.Map; +import java.util.stream.Collectors; @@ private final String extension; private final String contentType; private final Category category; + private static final Map<String, String> CONTENT_TYPE_BY_KEY = Arrays.stream(values()) + .collect(Collectors.toUnmodifiableMap( + candidate -> candidate.category.name() + ":" + candidate.extension, + candidate -> candidate.contentType + )); @@ private static String resolveContentType(String fileName, Category expectedCategory) { String extractedExtension = extractExtension(fileName); - - return Arrays.stream(values()) - .filter(candidate -> candidate.category == expectedCategory) - .filter(candidate -> candidate.extension.equals(extractedExtension)) - .findFirst() - .orElseThrow(() -> new IllegalArgumentException("Unsupported file extension: " + extractedExtension)) - .contentType; + String key = expectedCategory.name() + ":" + extractedExtension; + String contentType = CONTENT_TYPE_BY_KEY.get(key); + if (contentType == null) { + throw new IllegalArgumentException("Unsupported file extension: " + extractedExtension); + } + return contentType; }Also applies to: 34-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/upload/support/ExtensionEnum.java` around lines 3 - 4, Replace repeated values() iteration and stream creation in ExtensionEnum lookup with a static cached map: add a private static final Map<String, ExtensionEnum> (e.g., EXTENSION_LOOKUP) initialized once using Arrays.stream(ExtensionEnum.values()) and mapping lowercase names/extensions with Locale.ROOT to enum instances, and then change the existing lookup method (the method that currently iterates values()/uses streams to find an enum by string) to perform a single map get using key.toLowerCase(Locale.ROOT). Ensure the map covers all lookup keys you currently compute and keep behavior identical for misses (return null or Optional as the current method does).apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java (1)
132-136: object key prefix 하드코딩은 상수/헬퍼로 일원화해두는 것을 권장합니다.
"contents"문자열이 여러 위치에 반복되어 경로 정책 변경 시 누락 가능성이 있습니다.♻️ 제안 수정안
public class BackOfficeContentsService { + private static final String CONTENTS_OBJECT_PREFIX = "contents"; @@ - String posterObjectKey = uploadHelper.buildObjectKey("contents", contentsId, "poster", sanitizedPosterFileName); - String thumbnailObjectKey = uploadHelper.buildObjectKey("contents", contentsId, "thumbnail", sanitizedThumbnailFileName); - String originObjectKey = uploadHelper.buildObjectKey("contents", contentsId, "origin", sanitizedOriginFileName); - String masterPlaylistObjectKey = "contents/" + contentsId + "/transcoded/master.m3u8"; + String posterObjectKey = uploadHelper.buildObjectKey(CONTENTS_OBJECT_PREFIX, contentsId, "poster", sanitizedPosterFileName); + String thumbnailObjectKey = uploadHelper.buildObjectKey(CONTENTS_OBJECT_PREFIX, contentsId, "thumbnail", sanitizedThumbnailFileName); + String originObjectKey = uploadHelper.buildObjectKey(CONTENTS_OBJECT_PREFIX, contentsId, "origin", sanitizedOriginFileName); + String masterPlaylistObjectKey = CONTENTS_OBJECT_PREFIX + "/" + contentsId + "/transcoded/master.m3u8";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java` around lines 132 - 136, Replace the hardcoded "contents" prefix with a single constant or helper used across key construction: create/consume a CONTENTS_PREFIX constant (or add a method on uploadHelper) and update usages in the posterObjectKey, thumbnailObjectKey, originObjectKey assignments and the masterPlaylistObjectKey construction so they reference that constant/helper instead of the literal "contents" string; ensure uploadHelper.buildObjectKey calls and the manually-built "contents/" + contentsId + "/transcoded/master.m3u8" are changed to use the unified prefix to avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/upload/support/ExtensionEnum.java`:
- Around line 45-51: extractExtension currently calls fileName.trim() directly
which causes NullPointerException for null inputs and skips blank validation;
update extractExtension to first check if fileName is null and throw
IllegalArgumentException, then trim and validate that the trimmed string is not
empty before locating the '.'; keep the existing "File extension is missing"
IllegalArgumentException for missing delimiter or trailing dot; ensure public
callers (resolveImageContentType and resolveVideoContentType) rely on this
centralized validation in extractExtension rather than duplicating checks.
---
Duplicate comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.java`:
- Around line 87-88: The `@Parameter` description on the ContentsUploadRequest
parameter exposes the internal package path; update the annotation in
BackOfficeContentsApi so the description uses a schema-based phrase (e.g.,
"ContentsUploadRequest 스키마를 참고해주세요." or "요청 바디는 ContentsUploadRequest 스키마를
따릅니다.") instead of the full package path, keeping the `@RequestBody`
ContentsUploadRequest request and required = true unchanged; adjust the text in
the `@Parameter`(...) on the method parameter to the new schema-focused wording.
In `@modules/domain/src/main/java/com/ott/domain/contents/domain/Contents.java`:
- Around line 59-61: Ensure the domain invariant by validating originUrl before
assignment in Contents.updateStorageKeys: check that originUrl is not null (and
optionally not blank) and throw an unchecked exception (e.g.,
IllegalArgumentException or NullPointerException) with a clear message if it is
null, then assign this.originUrl and this.masterPlaylistUrl; reference the
Contents class, updateStorageKeys method, and the originUrl/masterPlaylistUrl
fields to locate where to add the guard.
In `@modules/domain/src/main/java/com/ott/domain/media/domain/Media.java`:
- Around line 68-70: updateImageKeys에서 posterUrl을 그대로 대입하면 엔티티의 nullable = false
제약을 위반할 수 있으니 posterUrl에 대한 null 방어를 복원하세요: updateImageKeys(String posterUrl,
String thumbnailUrl) 내부에서 posterUrl이 null이면 현재 객체 상태를 변경하지 않거나(선호 시)
IllegalArgumentException을 던져 호출자에게 선검증 실패를 알리도록 처리하고, 유효한 posterUrl인 경우에만
this.posterUrl = posterUrl;을 수행하세요; thumbnailUrl는 기존 의도대로 처리하되 posterUrl 관련 유효성
검사 로직은 반드시 포함시켜 주세요.
In
`@modules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java`:
- Around line 62-64: In ShortForm.updateStorageKeys, reintroduce a null guard
for the originUrl parameter (the field is non-nullable) by validating originUrl
at method entry and throwing a clear runtime exception (e.g.,
IllegalArgumentException or NullPointerException with a descriptive message) if
it's null, before assigning this.originUrl and this.masterPlaylistUrl; keep the
existing assignment to masterPlaylistUrl intact and ensure the validation uses
the method name updateStorageKeys to locate the change.
---
Nitpick comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.java`:
- Around line 132-136: Replace the hardcoded "contents" prefix with a single
constant or helper used across key construction: create/consume a
CONTENTS_PREFIX constant (or add a method on uploadHelper) and update usages in
the posterObjectKey, thumbnailObjectKey, originObjectKey assignments and the
masterPlaylistObjectKey construction so they reference that constant/helper
instead of the literal "contents" string; ensure uploadHelper.buildObjectKey
calls and the manually-built "contents/" + contentsId +
"/transcoded/master.m3u8" are changed to use the unified prefix to avoid
duplication.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.java`:
- Around line 111-114: Add the `@Valid` annotation to the interface method
signature for BackOfficeSeriesApi#createSeriesUpload so the contract explicitly
enforces validation on the RequestBody type (SeriesUploadRequest); update the
method parameter from `@RequestBody` SeriesUploadRequest request to `@Valid`
`@RequestBody` SeriesUploadRequest request to match the implementation in
BackOfficeSeriesController and maintain consistency with other APIs like
AdminAuthApi.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/upload/support/ExtensionEnum.java`:
- Around line 3-4: Replace repeated values() iteration and stream creation in
ExtensionEnum lookup with a static cached map: add a private static final
Map<String, ExtensionEnum> (e.g., EXTENSION_LOOKUP) initialized once using
Arrays.stream(ExtensionEnum.values()) and mapping lowercase names/extensions
with Locale.ROOT to enum instances, and then change the existing lookup method
(the method that currently iterates values()/uses streams to find an enum by
string) to perform a single map get using key.toLowerCase(Locale.ROOT). Ensure
the map covers all lookup keys you currently compute and keep behavior identical
for misses (return null or Optional as the current method does).
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/api-admin/src/main/java/com/ott/api_admin/config/SecurityConfig.javaapps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.javaapps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsController.javaapps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/content/dto/response/ContentsUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/content/service/BackOfficeContentsService.javaapps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.javaapps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesController.javaapps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/series/dto/response/SeriesUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/controller/BackOfficeShortFormApi.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/controller/BackOfficeShortFormController.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/response/ShortFormUploadResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/service/BackOfficeShortFormService.javaapps/api-admin/src/main/java/com/ott/api_admin/upload/support/ExtensionEnum.javaapps/api-admin/src/main/java/com/ott/api_admin/upload/support/UploadHelper.javamodules/domain/src/main/java/com/ott/domain/contents/domain/Contents.javamodules/domain/src/main/java/com/ott/domain/media/domain/Media.javamodules/domain/src/main/java/com/ott/domain/short_form/domain/ShortForm.java
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesController.java
- apps/api-admin/src/main/java/com/ott/api_admin/upload/support/UploadHelper.java
- apps/api-admin/src/main/java/com/ott/api_admin/content/dto/request/ContentsUploadRequest.java
- apps/api-admin/src/main/java/com/ott/api_admin/shortform/dto/request/ShortFormUploadRequest.java
- apps/api-admin/src/main/java/com/ott/api_admin/series/dto/request/SeriesUploadRequest.java
- apps/api-admin/src/main/java/com/ott/api_admin/series/service/BackOfficeSeriesService.java
- apps/api-admin/src/main/java/com/ott/api_admin/series/dto/response/SeriesUploadResponse.java
- apps/api-admin/src/main/java/com/ott/api_admin/content/dto/response/ContentsUploadResponse.java
phonil
reviewed
Feb 25, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.java (1)
87-88:@Parameter설명 문구가 이전 리뷰 요청과 다르게 적용됨phonil이 요청한 문구는
"Schemas의 ContentsUploadResponse를 참고해주세요."이나, 현재"ContentsUploadRequest를 참고해주세요."로 적용되어 있습니다. Swagger UI에서 응답 스키마를 확인하려면 응답 DTO(ContentsUploadResponse)를 안내하는 문구가 더 적합합니다.✏️ 수정 제안
- `@Parameter`(description = "ContentsUploadRequest를 참고해주세요.", required = true) + `@Parameter`(description = "Schemas의 ContentsUploadResponse를 참고해주세요.", required = true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.java` around lines 87 - 88, Update the `@Parameter` annotation on the request body in BackOfficeContentsApi so its description uses the response DTO text instead of the request DTO; replace the current description "ContentsUploadRequest를 참고해주세요." with "Schemas의 ContentsUploadResponse를 참고해주세요." on the `@Parameter` above the `@RequestBody` ContentsUploadRequest request to ensure Swagger shows the response schema guidance.apps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.java (1)
112-113: 파라미터 설명은 타입명 반복보다 의미 중심 문구가 더 명확합니다.현재 설명은 클래스명을 다시 말하는 형태라 Swagger 문서 가독성이 떨어집니다. 요청 목적/필수 조건 중심으로 짧게 바꾸는 것을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.java` around lines 112 - 113, 현재 `@Parameter`(description = "SeriesUploadRequest를 참고해주세요.") 설명은 타입명 반복이라 가독성이 떨어집니다; BackOfficeSeriesApi의 해당 메서드에서 `@Parameter` 애노테이션의 description을 요청 목적과 필수 조건 중심으로 간단히 바꿔주세요(예: "업로드할 시리즈 정보(필수: title, thumbnail 등)를 포함; 신규 시리즈 생성/수정용"). 변경 대상 식별자: the `@Parameter` annotation placed above the `@RequestBody` SeriesUploadRequest request in BackOfficeSeriesApi.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.java`:
- Line 71: The `@Operation` annotation for the createContentsUpload endpoint in
BackOfficeContentsApi is missing the required admin-note; update the description
on the `@Operation` for the createContentsUpload method (in class
BackOfficeContentsApi) to include " - ADMIN 권한 필요." so it matches the other
methods (e.g., getContents, getContentsDetail) and maintains consistency across
the interface.
- Around line 86-88: The createContentsUpload endpoint is missing the `@Valid`
annotation on its request body parameter, so Bean Validation on
ContentsUploadRequest (which uses `@NotBlank/`@NotNull/@PositiveOrZero) won't run;
update the method signature for createContentsUpload to annotate the parameter
as `@RequestBody` `@Valid` ContentsUploadRequest request so Spring will trigger
validation and return violations appropriately.
---
Duplicate comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.java`:
- Around line 87-88: Update the `@Parameter` annotation on the request body in
BackOfficeContentsApi so its description uses the response DTO text instead of
the request DTO; replace the current description "ContentsUploadRequest를
참고해주세요." with "Schemas의 ContentsUploadResponse를 참고해주세요." on the `@Parameter` above
the `@RequestBody` ContentsUploadRequest request to ensure Swagger shows the
response schema guidance.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.java`:
- Around line 112-113: 현재 `@Parameter`(description = "SeriesUploadRequest를
참고해주세요.") 설명은 타입명 반복이라 가독성이 떨어집니다; BackOfficeSeriesApi의 해당 메서드에서 `@Parameter`
애노테이션의 description을 요청 목적과 필수 조건 중심으로 간단히 바꿔주세요(예: "업로드할 시리즈 정보(필수: title,
thumbnail 등)를 포함; 신규 시리즈 생성/수정용"). 변경 대상 식별자: the `@Parameter` annotation placed
above the `@RequestBody` SeriesUploadRequest request in BackOfficeSeriesApi.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api-admin/src/main/java/com/ott/api_admin/config/SecurityConfig.javaapps/api-admin/src/main/java/com/ott/api_admin/content/controller/BackOfficeContentsApi.javaapps/api-admin/src/main/java/com/ott/api_admin/series/controller/BackOfficeSeriesApi.javaapps/api-admin/src/main/java/com/ott/api_admin/shortform/controller/BackOfficeShortFormApi.java
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api-admin/src/main/java/com/ott/api_admin/shortform/controller/BackOfficeShortFormApi.java
- apps/api-admin/src/main/java/com/ott/api_admin/config/SecurityConfig.java
marulog
approved these changes
Feb 25, 2026
This was referenced Feb 26, 2026
This was referenced Mar 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 작업 내용
📷 스크린샷
[시리즈 메타데이터 업로드]
[시리즈 업로드]
[콘텐츠 메타데이터 업로드]
[콘텐츠 업로드]
[숏폼 업로드]
[숏폼 업로드]
☑️ 체크 리스트
#️⃣ 연관된 이슈
#OT-104
💬 리뷰 요구사항
Summary by CodeRabbit
새로운 기능
개선사항