feat: 노션 이미지 url 저장방식 변경 - #202 #204
Merged
Conversation
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
📝 WalkthroughSummary by CodeRabbit릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings. Walkthrough노션 공개 URL에서 호스트를 추출해 이미지 프록시 URL을 생성하도록 저장 방식을 변경하고, 관련 URL 검증 및 신규 예외(NotionPublicUrlNotFoundException, NotionUrlMalformedException)와 에러코드(NOT_FOUND_NOTION_PUBLIC_ID)를 추가했습니다. Changes
Sequence DiagramsequenceDiagram
actor Client
participant NotionResponseMapper
participant NotionImageUrlUtil
participant DB as Media Storage
Client->>NotionResponseMapper: Notion 페이지 데이터 (publicUrl 포함)
rect rgb(230,240,255)
NotionResponseMapper->>NotionResponseMapper: publicUrl 존재 여부 확인
alt publicUrl 없음
NotionResponseMapper-->>Client: NotionPublicUrlNotFoundException
else publicUrl 있음
NotionResponseMapper->>NotionResponseMapper: URL 파싱 → host 추출
alt malformed
NotionResponseMapper-->>Client: NotionUrlMalformedException
end
end
end
rect rgb(220,255,230)
loop 각 미디어 파일
NotionResponseMapper->>NotionImageUrlUtil: buildProxyUrl(host, pageId, originalUrl)
NotionImageUrlUtil-->>NotionResponseMapper: proxyUrl
NotionResponseMapper->>DB: 미디어 엔티티 저장(proxyUrl)
DB-->>NotionResponseMapper: 저장 완료
end
end
NotionResponseMapper-->>Client: 처리 완료
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/main/java/com/permitseoul/permitserver/domain/admin/util/NotionImageUrlUtil.java (1)
17-30: null 반환 대신 예외 발생을 고려하세요.현재 유효하지 않은 입력에 대해
null을 반환하는데, 이는 호출자가 null 체크를 누락할 경우 잠재적 버그를 숨길 수 있습니다.명시적으로 예외를 발생시키는 것이 더 안전한 API 설계일 수 있습니다:
- 입력 검증 실패 시 즉시 감지 가능
- 호출자가 명시적으로 에러를 처리하도록 강제
- 디버깅이 용이함
다만 현재 호출 측에서 null 체크를 수행하고 있으므로, 기존 코드와의 일관성을 고려하여 판단하세요.
🔎 예외 기반 검증 예시
public static String buildProxyUrl(final String publishedHost, final String pageId, final String originalUrl) { - if (publishedHost == null || publishedHost.isBlank()) return null; - if (pageId == null || pageId.isBlank()) return null; - if (originalUrl == null || originalUrl.isBlank()) return null; + if (publishedHost == null || publishedHost.isBlank()) { + throw new IllegalArgumentException("publishedHost must not be null or blank"); + } + if (pageId == null || pageId.isBlank()) { + throw new IllegalArgumentException("pageId must not be null or blank"); + } + if (originalUrl == null || originalUrl.isBlank()) { + throw new IllegalArgumentException("originalUrl must not be null or blank"); + } // presigned query 제거하는 과정 final String baseUrl = originalUrl.split("\\?")[0]; return HTTPS + publishedHost + IMAGE_PATH + encodeURIComponent(baseUrl) + QUERY_PREFIX + pageId + CACHE_SUFFIX; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/api/service/AdminTimetableService.javasrc/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/AdminNotionException.javasrc/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionPublicUrlNotFoundException.javasrc/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionUrlMalformedException.javasrc/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/api/dto/NotionTimetableBlockUpdateWebhookRequest.javasrc/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/api/service/AdminNotionTimetableBlockService.javasrc/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/core/strategy/impl/NotionTimetableBlockMediaUpdateStrategyImpl.javasrc/main/java/com/permitseoul/permitserver/domain/admin/timetable/category/api/service/AdminNotionTimetableCategoryService.javasrc/main/java/com/permitseoul/permitserver/domain/admin/timetable/stage/api/service/AdminNotionTimetableStageService.javasrc/main/java/com/permitseoul/permitserver/domain/admin/util/NotionImageUrlUtil.javasrc/main/java/com/permitseoul/permitserver/domain/admin/util/NotionResponseMapper.javasrc/main/java/com/permitseoul/permitserver/domain/user/api/dto/UserInfoRequest.javasrc/main/java/com/permitseoul/permitserver/domain/user/api/service/UserService.javasrc/main/java/com/permitseoul/permitserver/domain/user/core/domain/entity/UserEntity.javasrc/main/java/com/permitseoul/permitserver/global/external/notion/dto/NotionTimetableDatasourceResponse.javasrc/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java
💤 Files with no reviewable changes (1)
- src/main/java/com/permitseoul/permitserver/domain/user/api/dto/UserInfoRequest.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sjk4618
Repo: PERMIT-SEOUL/permit-server PR: 48
File: src/main/java/com/permitseoul/permitserver/domain/guest/core/domain/entity/GuestEntity.java:31-37
Timestamp: 2025-07-15T09:37:32.765Z
Learning: sjk4618 prefers to implement factory methods or public constructors for entities when they are actually needed, rather than creating them proactively.
🧬 Code graph analysis (8)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionUrlMalformedException.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/base/AdminBaseException.java (1)
AdminBaseException(3-4)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/core/strategy/impl/NotionTimetableBlockMediaUpdateStrategyImpl.java (2)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionPublicUrlNotFoundException.java (1)
NotionPublicUrlNotFoundException(5-6)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionUrlMalformedException.java (1)
NotionUrlMalformedException(5-6)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/stage/api/service/AdminNotionTimetableStageService.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/AdminNotionException.java (1)
AdminNotionException(5-6)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/category/api/service/AdminNotionTimetableCategoryService.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/AdminNotionException.java (1)
AdminNotionException(5-6)
src/main/java/com/permitseoul/permitserver/domain/admin/util/NotionResponseMapper.java (3)
src/main/java/com/permitseoul/permitserver/domain/admin/base/AdminBaseException.java (1)
AdminBaseException(3-4)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionPublicUrlNotFoundException.java (1)
NotionPublicUrlNotFoundException(5-6)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionUrlMalformedException.java (1)
NotionUrlMalformedException(5-6)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/api/service/AdminNotionTimetableBlockService.java (3)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/AdminNotionException.java (1)
AdminNotionException(5-6)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionPublicUrlNotFoundException.java (1)
NotionPublicUrlNotFoundException(5-6)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionUrlMalformedException.java (1)
NotionUrlMalformedException(5-6)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/api/service/AdminTimetableService.java (2)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionPublicUrlNotFoundException.java (1)
NotionPublicUrlNotFoundException(5-6)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionUrlMalformedException.java (1)
NotionUrlMalformedException(5-6)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionPublicUrlNotFoundException.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/base/AdminBaseException.java (1)
AdminBaseException(3-4)
🔇 Additional comments (16)
src/main/java/com/permitseoul/permitserver/domain/user/core/domain/entity/UserEntity.java (1)
71-73: LGTM! 이메일 업데이트 로직이 안전하게 개선되었습니다.
nullable = false제약조건과도 충돌하지 않습니다.src/main/java/com/permitseoul/permitserver/domain/user/api/service/UserService.java (1)
47-49: LGTM! 이메일 중복 검증 로직이 효율적으로 개선되었습니다.
UserEntity.updateUserInfo의 조건부 업데이트 로직과 일관성을 유지합니다.src/main/java/com/permitseoul/permitserver/domain/admin/timetable/stage/api/service/AdminNotionTimetableStageService.java (1)
4-4: 예외 패키지 재구성 확인.
AdminNotionException을core.exception패키지로 이동한 것은 레이어 분리 측면에서 적절한 변경입니다.src/main/java/com/permitseoul/permitserver/domain/admin/timetable/category/api/service/AdminNotionTimetableCategoryService.java (1)
4-4: 예외 패키지 재구성 확인.다른 서비스들과 일관되게
AdminNotionException의 import 경로가 업데이트되었습니다.src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionPublicUrlNotFoundException.java (1)
5-6: 새 예외 클래스 추가 확인.Public URL을 찾을 수 없는 경우를 명시적으로 처리하기 위한 예외 클래스가 추가되었습니다. 현재는 마커 예외로 구현되어 있으며, 필요시 추후 상세한 에러 메시지나 컨텍스트 정보를 추가할 수 있습니다.
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/api/dto/NotionTimetableBlockUpdateWebhookRequest.java (1)
14-15: Public URL 필드 추가 확인.웹훅 요청 DTO에
publicUrl필드가 올바르게 추가되었습니다. Notion 웹훅에서 전달되는public_url데이터를 적절히 매핑하고 있습니다.src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionUrlMalformedException.java (1)
5-6: 새 예외 클래스 추가 확인.잘못된 형식의 Notion URL을 처리하기 위한 예외 클래스입니다.
NotionPublicUrlNotFoundException과 함께 URL 관련 오류를 세밀하게 구분할 수 있게 해줍니다.src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/api/service/AdminTimetableService.java (2)
8-9: 새 예외 타입 import 추가 확인.Notion URL 관련 예외 처리를 위한 import가 추가되었습니다.
88-89: Notion URL 예외 처리 구현 올바르게 확인됨.NotionResponseMapper와 NotionImageUrlUtil의 구현을 검증한 결과, 요청된 모든 사항이 올바르게 처리되고 있습니다:
publicUrlnull/공백 체크 시NotionPublicUrlNotFoundException발생 (NotionResponseMapper:152-153)- URL 파싱 실패 시
NotionUrlMalformedException발생 (NotionResponseMapper:157-161)- 프록시 URL 생성 로직이 무효한 입력값에 대해 null 반환하고, 호출부에서 적절히 처리됨 (NotionImageUrlUtil:20-22, NotionResponseMapper:175-176)
예외 처리가 견고하게 구현되어 있습니다.
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/api/service/AdminNotionTimetableBlockService.java (1)
4-7: LGTM!예외 클래스의 패키지 이동과 새로운 URL 관련 예외 import가 적절합니다.
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/core/strategy/impl/NotionTimetableBlockMediaUpdateStrategyImpl.java (2)
3-4: LGTM!새로운 URL 검증 및 프록시 URL 생성 로직을 위한 import가 적절합니다.
Also applies to: 10-10, 17-17
58-68: LGTM!미디어 파일 처리 로직이 견고합니다:
- 적절한 null 체크로 NPE 방지
- 프록시 URL 생성 및 검증 로직이 올바름
- sequence 관리가 정확함
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/AdminNotionException.java (1)
1-6: LGTM!예외 클래스의 패키지 이동이 적절합니다. 기능 변경 없이 구조적 개선입니다.
src/main/java/com/permitseoul/permitserver/domain/admin/util/NotionImageUrlUtil.java (1)
9-15: LGTM!유틸리티 클래스 구조가 적절합니다:
final클래스와 private 생성자로 인스턴스화 방지- 명확한 상수 정의
src/main/java/com/permitseoul/permitserver/domain/admin/util/NotionResponseMapper.java (2)
3-5: LGTM!URL 검증 및 프록시 URL 생성을 위한 import가 적절합니다.
Also applies to: 14-14, 20-21
163-183: LGTM!미디어 처리 로직이 견고하고 잘 구현되었습니다:
- 철저한 null 체크로 NPE 방지
- Line 174의 주석으로 프록시 URL 사용 이유를 명확히 설명
- 프록시 URL 검증 로직이 적절함
- sequence 관리가 정확함
이 변경으로 Notion URL의 1시간 유효기간 문제를 효과적으로 해결합니다.
.../permitserver/domain/admin/timetable/block/api/service/AdminNotionTimetableBlockService.java
Show resolved
Hide resolved
Comment on lines
+37
to
+44
| final String publicUrl = request.data().publicUrl(); | ||
|
|
||
| final String host; | ||
| try { | ||
| host = new java.net.URL(publicUrl).getHost(); | ||
| } catch (MalformedURLException e) { | ||
| throw new NotionUrlMalformedException(); | ||
| } |
There was a problem hiding this comment.
host 값 검증과 코드 중복을 확인하세요.
두 가지 개선이 필요합니다:
-
Host 검증 누락:
URL.getHost()는 일부 URL 형식에서null또는 빈 문자열을 반환할 수 있습니다.host값이 유효한지 검증이 필요합니다. -
코드 중복: 동일한 로직이
NotionResponseMapper.java(lines 156-161)에도 존재합니다. 공통 유틸리티 메서드로 추출하는 것을 고려하세요.
🔎 제안하는 개선 방안
방안 1: Host 검증 추가
final String host;
try {
host = new java.net.URL(publicUrl).getHost();
+ if (host == null || host.isBlank()) {
+ throw new NotionUrlMalformedException();
+ }
} catch (MalformedURLException e) {
throw new NotionUrlMalformedException();
}방안 2: 공통 메서드 추출 (선호)
NotionImageUrlUtil에 다음 메서드를 추가:
public static String extractHostFromPublicUrl(final String publicUrl) {
if (publicUrl == null || publicUrl.isBlank()) {
throw new NotionPublicUrlNotFoundException();
}
try {
final String host = new URL(publicUrl).getHost();
if (host == null || host.isBlank()) {
throw new NotionUrlMalformedException();
}
return host;
} catch (MalformedURLException e) {
throw new NotionUrlMalformedException();
}
}그리고 이 메서드를 NotionResponseMapper.java와 이 클래스 모두에서 사용하세요.
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/core/strategy/impl/NotionTimetableBlockMediaUpdateStrategyImpl.java
around lines 37-44: the current extraction of host from
request.data().publicUrl() lacks validation for null/blank host and duplicates
logic found in NotionResponseMapper.java (lines ~156-161); extract the logic
into a shared util (e.g., NotionImageUrlUtil.extractHostFromPublicUrl) that
checks publicUrl for null/blank (throw NotionPublicUrlNotFoundException), parses
the URL catching MalformedURLException (throw NotionUrlMalformedException),
verifies host is non-null/non-blank (throw NotionUrlMalformedException) and
returns the host, then replace the duplicated code in both
NotionTimetableBlockMediaUpdateStrategyImpl and NotionResponseMapper to call
this new utility.
src/main/java/com/permitseoul/permitserver/domain/admin/util/NotionImageUrlUtil.java
Show resolved
Hide resolved
Comment on lines
149
to
161
| final NotionTimetableDatasourceResponse.NotionPage notionPage = notionTimetableDatasourceResponse.results().get(i); | ||
| final String pageId = notionPage.id(); | ||
| final String publicUrl = notionPage.publicUrl(); | ||
| if (publicUrl == null || publicUrl.isBlank()) { | ||
| throw new NotionPublicUrlNotFoundException(); | ||
| } | ||
|
|
||
| final String host; | ||
| try { | ||
| host = new URL(publicUrl).getHost(); | ||
| } catch (MalformedURLException e) { | ||
| throw new NotionUrlMalformedException(); | ||
| } |
There was a problem hiding this comment.
코드 중복 및 host 검증 누락.
이 로직은 NotionTimetableBlockMediaUpdateStrategyImpl.java (lines 37-44)와 동일합니다.
앞서 해당 파일에서 언급한 것과 같이:
URL.getHost()가 반환하는host값에 대한 null/empty 검증이 필요합니다- 공통 유틸리티 메서드로 추출하여 중복을 제거하는 것을 권장합니다
NotionImageUrlUtil.extractHostFromPublicUrl(String publicUrl) 같은 메서드를 추가하여 두 곳에서 재사용하세요.
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/admin/util/NotionResponseMapper.java
around lines 149 to 161, this block duplicates logic in
NotionTimetableBlockMediaUpdateStrategyImpl (lines 37-44) and lacks validation
that URL.getHost() may be null/blank; extract the logic into a shared utility
method (e.g., NotionImageUrlUtil.extractHostFromPublicUrl(String publicUrl))
that: 1) validates publicUrl is not null/blank, 2) constructs a URL and catches
MalformedURLException to throw NotionUrlMalformedException, 3) verifies the
returned host is not null/blank and throws NotionUrlMalformedException (or a
suitable exception) if invalid; then replace the duplicated code in both files
with a call to this utility.
Comment on lines
+13
to
+15
| NotionProperties properties, | ||
| @JsonProperty("public_url") | ||
| String publicUrl |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -A5 -B5 "publicUrl|public_url" --type javaRepository: PERMIT-SEOUL/permit-server
Length of output: 10825
NotionTimetableBlockMediaUpdateStrategyImpl에서 publicUrl null 체크 누락
public_url 필드 매핑은 올바릅니다. 다만 NotionResponseMapper에서는 null 체크가 있으나, NotionTimetableBlockMediaUpdateStrategyImpl (37-44줄)에서는 publicUrl을 null 체크 없이 바로 new URL(publicUrl)에 전달하고 있습니다. Notion 웹훅 응답에서 public_url이 누락될 경우 NullPointerException이 발생할 수 있으므로, NotionResponseMapper와 동일하게 null/blank 체크를 추가하거나 exception 처리를 명시적으로 추가해야 합니다.
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/global/external/notion/dto/NotionTimetableDatasourceResponse.java
around lines 13-15, the public_url mapping is correct but the calling code
(NotionTimetableBlockMediaUpdateStrategyImpl lines ~37-44) passes publicUrl
directly into new URL(publicUrl) without null/blank checks; update the strategy
implementation to first validate publicUrl (e.g., check for null or blank using
Objects.isNull or StringUtils.isBlank) and either skip processing/return early
or throw a clear, handled exception; alternatively wrap new URL(publicUrl) in a
try/catch for MalformedURLException and handle null/blank the same way
NotionResponseMapper does to prevent NullPointerException.
| NOT_FOUND_NOTION_DATABASE_SOURCE(HttpStatus.NOT_FOUND, 40427, "Notion 데이터베이스 소스를 찾을 수 없습니다."), | ||
| NOT_FOUND_COUPON(HttpStatus.NOT_FOUND, 40428, "coupon을 찾을 수 없습니다."), | ||
| NOT_FOUND_SITE_MAP_IMAGE(HttpStatus.NOT_FOUND, 40429, "sitemap image를 찾을 수 없습니다."), | ||
| NOT_FOUND_NOTION_PUBLIC_ID(HttpStatus.NOT_FOUND, 40430, "public url을 찾을 수 없습니다."), |
There was a problem hiding this comment.
에러 메시지 수정 필요.
에러 코드는 NOT_FOUND_NOTION_PUBLIC_ID인데 메시지가 "public url을 찾을 수 있습니다"로 되어 있습니다. "찾을 수 없습니다"로 수정해야 합니다.
🔎 수정 제안
- NOT_FOUND_NOTION_PUBLIC_ID(HttpStatus.NOT_FOUND, 40430, "public url을 찾을 수 있습니다."),
+ NOT_FOUND_NOTION_PUBLIC_ID(HttpStatus.NOT_FOUND, 40430, "public url을 찾을 수 없습니다."),Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java
around line 91, the enum constant NOT_FOUND_NOTION_PUBLIC_ID has the wrong
Korean message (says "찾을 수 있습니다" instead of negative); update its message string
to "public url을 찾을 수 없습니다." so the error text matches the NOT_FOUND intent.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/java/com/permitseoul/permitserver/domain/admin/util/NotionResponseMapper.java (1)
180-196: 호스트 추출 로직 중복을 제거하세요.이 메서드는
NotionTimetableBlockMediaUpdateStrategyImpl.java(lines 38-46)와 동일한 로직을 포함하고 있습니다. 앞서 해당 파일에서 언급한 것과 같이,NotionImageUrlUtil에 공통 유틸리티 메서드(예:extractHostFromPublicUrl)를 추가하여 양쪽 모두에서 재사용하도록 리팩토링하세요.🔎 제안하는 개선 방안
NotionImageUrlUtil에 다음 메서드를 추가:public static String extractHostFromPublicUrl(final String publicUrl) { if (publicUrl == null || publicUrl.isBlank()) { throw new NotionPublicUrlNotFoundException(); } try { final String host = new URL(publicUrl).getHost(); if (host == null || host.isBlank()) { throw new NotionUrlMalformedException(); } return host; } catch (MalformedURLException e) { throw new NotionUrlMalformedException(); } }이 파일에서 다음과 같이 사용:
-private static String getHost(NotionTimetableDatasourceResponse.NotionPage notionPage) { - final String publicUrl = notionPage.publicUrl(); - if (publicUrl == null || publicUrl.isBlank()) { - throw new NotionPublicUrlNotFoundException(); - } - - final String host; - try { - host = new URL(publicUrl).getHost(); - } catch (MalformedURLException e) { - throw new NotionUrlMalformedException(); - } - if (host == null) { - throw new NotionUrlMalformedException(); - } - return host; -} +private static String getHost(NotionTimetableDatasourceResponse.NotionPage notionPage) { + return NotionImageUrlUtil.extractHostFromPublicUrl(notionPage.publicUrl()); +}src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/core/strategy/impl/NotionTimetableBlockMediaUpdateStrategyImpl.java (1)
36-46: 호스트 추출 로직 중복을 제거하세요.이 로직은
NotionResponseMapper.java의getHost()메서드 (lines 180-196)와 동일합니다. 이전 리뷰에서 제안된 대로,NotionImageUrlUtil에 공통 유틸리티 메서드를 추가하여 중복을 제거하는 것을 권장합니다.🔎 제안하는 개선 방안
NotionImageUrlUtil에 다음 메서드를 추가하세요:public static String extractHostFromPublicUrl(final String publicUrl) { if (publicUrl == null || publicUrl.isBlank()) { throw new NotionPublicUrlNotFoundException(); } try { final String host = new URL(publicUrl).getHost(); if (host == null || host.isBlank()) { throw new NotionUrlMalformedException(); } return host; } catch (MalformedURLException e) { throw new NotionUrlMalformedException(); } }그리고 이 클래스와
NotionResponseMapper.java모두에서 이 메서드를 사용하도록 리팩토링하세요:-final String publicUrl = request.data().publicUrl(); - -final String host; -try { - host = new java.net.URL(publicUrl).getHost(); -} catch (MalformedURLException e) { - throw new NotionUrlMalformedException(); -} -if (host == null) { - throw new NotionUrlMalformedException(); -} +final String host = NotionImageUrlUtil.extractHostFromPublicUrl(request.data().publicUrl());
🧹 Nitpick comments (1)
src/main/java/com/permitseoul/permitserver/domain/admin/util/NotionResponseMapper.java (1)
150-177: 미디어 URL 프록시 변환 로직이 적절하게 구현되었습니다.원본 미디어 URL을 프록시 URL로 변환하는 로직이 올바르게 작동하며, null 체크도 충분합니다.
참고: Line 167의 proxyUrl null 체크는
NotionTimetableBlockMediaUpdateStrategyImpl(line 67)과 달리isBlank()체크가 없지만,buildProxyUrl이 실패 시 null만 반환하므로 기능상 문제는 없습니다. 다만 일관성을 위해 동일한 검증 패턴을 사용하는 것을 고려해보세요.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/api/dto/NotionTimetableBlockUpdateWebhookRequest.javasrc/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/api/service/AdminNotionTimetableBlockService.javasrc/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/core/strategy/impl/NotionTimetableBlockMediaUpdateStrategyImpl.javasrc/main/java/com/permitseoul/permitserver/domain/admin/util/NotionImageUrlUtil.javasrc/main/java/com/permitseoul/permitserver/domain/admin/util/NotionResponseMapper.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sjk4618
Repo: PERMIT-SEOUL/permit-server PR: 48
File: src/main/java/com/permitseoul/permitserver/domain/guest/core/domain/entity/GuestEntity.java:31-37
Timestamp: 2025-07-15T09:37:32.765Z
Learning: sjk4618 prefers to implement factory methods or public constructors for entities when they are actually needed, rather than creating them proactively.
🧬 Code graph analysis (3)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/core/strategy/impl/NotionTimetableBlockMediaUpdateStrategyImpl.java (4)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionUrlMalformedException.java (1)
NotionUrlMalformedException(5-6)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/component/TimetableBlockMediaRetriever.java (1)
Component(10-19)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/blockmedia/core/component/AdminTimetableBlockMediaRemover.java (1)
Component(7-15)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/block/core/component/AdminTimetableBlockRetriever.java (1)
Component(13-51)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/api/service/AdminNotionTimetableBlockService.java (3)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/AdminNotionException.java (1)
AdminNotionException(5-6)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionPublicUrlNotFoundException.java (1)
NotionPublicUrlNotFoundException(5-6)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionUrlMalformedException.java (1)
NotionUrlMalformedException(5-6)
src/main/java/com/permitseoul/permitserver/domain/admin/util/NotionResponseMapper.java (3)
src/main/java/com/permitseoul/permitserver/domain/admin/base/AdminBaseException.java (1)
AdminBaseException(3-4)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionPublicUrlNotFoundException.java (1)
NotionPublicUrlNotFoundException(5-6)src/main/java/com/permitseoul/permitserver/domain/admin/timetable/base/core/exception/NotionUrlMalformedException.java (1)
NotionUrlMalformedException(5-6)
🔇 Additional comments (6)
src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/api/service/AdminNotionTimetableBlockService.java (2)
4-7: 예외 타입 import가 적절하게 추가되었습니다.노션 public URL 처리를 위한 새로운 예외 타입들이 추가되었고, AdminNotionException의 패키지 위치 변경도 반영되었습니다.
55-57: 이전 리뷰의 예외 처리 일관성 문제가 해결되었습니다.이제 다른 catch 블록들과 동일하게
AdminNotionException을 재발생시키므로 예외 처리가 일관성 있게 구현되었습니다.src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/api/dto/NotionTimetableBlockUpdateWebhookRequest.java (1)
15-17: publicUrl 필드 추가가 적절합니다.
@NotNull어노테이션으로 필수 값을 보장하고, Jackson 역직렬화를 위한@JsonProperty설정이 올바르게 적용되었습니다. 이후 호스트 추출 로직에서 이 값에 의존하므로 필수 필드 설정이 적절합니다.src/main/java/com/permitseoul/permitserver/domain/admin/timetable/block/core/strategy/impl/NotionTimetableBlockMediaUpdateStrategyImpl.java (1)
60-70: 미디어 URL 프록시 변환 로직이 적절합니다.원본 URL을 프록시 URL로 변환하는 로직이 올바르게 구현되었고, null 체크도 충분히 방어적으로 작성되었습니다.
참고: Line 67의
proxyUrl.isBlank()체크는NotionImageUrlUtil.buildProxyUrl이 실패 시 null을 반환하므로 불필요하지만, 방어적 코딩 측면에서 문제는 없습니다.src/main/java/com/permitseoul/permitserver/domain/admin/util/NotionImageUrlUtil.java (2)
17-30: 프록시 URL 생성 로직이 올바르게 구현되었습니다.입력값 검증, presigned 쿼리 파라미터 제거, URL 인코딩 및 프록시 URL 조합이 적절하게 처리되었습니다.
32-41: 이전 리뷰의 asterisk 처리 문제가 해결되었습니다.Line 39에
"%2A"를"*"로 변환하는 로직이 추가되어, JavaScript의encodeURIComponent와 동일하게 동작하도록 RFC 3986을 준수합니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항