refactor: redirectUrl 추상화 #679
Open
Conversation
This file contains 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
helenason
approved these changes
Dec 24, 2024
Comment on lines
63
to
69
| private static String toImage(CommentRoomStatus status, String resourceHost) { | ||
| return UriComponentsBuilder.newInstance() | ||
| .scheme("https") | ||
| .host(resourceHost) | ||
| .path(findViewMapper(status).image) | ||
| .build(false) | ||
| .toString(); |
There was a problem hiding this comment.
이 메서드에서 findViewMapper(status).image를 필드로 추출하는 것은 어떨까요? 처음 코드를 읽었을 때 이 메서드의 의도를 한번에 빠르게 파악하기는 어려웠던 것 같아요!
| .path(findViewMapper(status).image) | ||
| .build(false) | ||
| .toString(); | ||
| return "https://%s/common/%s.png".formatted(resourceHost, findViewMapper(status).image); |
There was a problem hiding this comment.
이 커밋에서 작업하신 개선 사항을 이전 커밋에서 필요해보여 길게 조심스럽게 제안해두었는데, 역시 포케도 저와 뜻이 같았군요 ㅎㅎ 훨씬 깔끔하고 좋네요! 이런 리팩터링 너무 좋습니다 👏
masonkimseoul
approved these changes
Dec 25, 2024
|
|
||
| @Value("${storage.path}") | ||
| private String storagePath; | ||
|
|
||
| LocalStorageService(String redirectUrl, String storagePath) { | ||
| this.redirectUrl = redirectUrl; | ||
| private LocalStorageService(String resourceHost, String storagePath) { |
There was a problem hiding this comment.
근데 우리는 왜 이 클래스에서 Lombok으로 생성자를 만들지 않았을까요? 잘 기억나지 않아서 여쭤봅니다.
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.
📌 관련 이슈
close #676
✨ 작업 내용
여러 객체 설정되어 있는
redirectUrl을 추상화 했습니다.마이그레이션 작업할 때
application.properties파일만 수정하면 이미지 스토리지 서버를 유연하게 옮길 수 있어 작업을 수행했습니다.resourceUrl의 명칭을 좀 더 명확한 이름으로 resourceHost로 변경했습니다.
📚 기타
PR 머지 이후 영향도를 고려하여 application.properties 시크릿 값 을 추가했습니다.