[Feat] swagger config setup #8
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| public enum EnvironmentConstants { | ||
| PROD(PROD_ENV), | ||
| DEV(DEV_ENV), | ||
| LOCAL(LOCAL_ENV); | ||
|
|
||
| private final String value; | ||
|
|
||
| @NoArgsConstructor(access = AccessLevel.PRIVATE) | ||
| public static class Constants { | ||
| public static final String PROD_ENV = "prod"; | ||
| public static final String DEV_ENV = "dev"; | ||
| public static final String LOCAL_ENV = "local"; | ||
| public static final List<String> PROD_AND_DEV_ENV = List.of(PROD_ENV, DEV_ENV); | ||
| } | ||
| } |
There was a problem hiding this comment.
[컨벤션]
이런 얘기는 저희끼리 안 한 것 같긴 한데 다른 클래스들과 동일하게 첫줄 공백 주는 게 좋을 것 같습니다!
Constants와 UrlConstants 클래스도 확인 부탁드려요 !!
| private List<Server> swaggerServers() { | ||
| Server server = new Server().url(getServerUrl()).description(API_DESCRIPTION); | ||
| return List.of(server); | ||
| } | ||
|
|
||
| private String getServerUrl() { | ||
| return switch (springEnvironmentUtil.getCurrentProfile()) { | ||
| // TODO: prod, dev 연결 | ||
| // case "prod" -> UrlConstants.PROD_SERVER_URL.getValue(); | ||
| // case "dev" -> UrlConstants.DEV_SERVER_URL.getValue(); | ||
| default -> UrlConstants.LOCAL_SERVER_URL.getValue(); | ||
| }; | ||
| } | ||
|
|
||
| private Info swaggerInfo() { | ||
| License license = new License(); | ||
| license.setUrl(GITHUB_URL); | ||
| license.setName(SERVER_NAME); | ||
|
|
||
| return new Info() | ||
| .version("v" + version) | ||
| .title(API_TITLE) | ||
| .description(API_DESCRIPTION) | ||
| .license(license); | ||
| } |
There was a problem hiding this comment.
[컨벤션] 마찬가지로 여기 private 메서드도 사용하는 쪽 openAPI() 아래에 두면 가독성이 더욱 좋아질 것 같아요~~~!
+) 전체적인 파일 대상으로 EOL 확인 부탁드려용
There was a problem hiding this comment.
컨벤션 자꾸 안 지켜서 죄송합니다.!!!
| @NoArgsConstructor(access = AccessLevel.PRIVATE) | ||
| public static class Constants { | ||
| public static final String PROD_ENV = "prod"; | ||
| public static final String DEV_ENV = "dev"; | ||
| public static final String LOCAL_ENV = "local"; | ||
| public static final List<String> PROD_AND_DEV_ENV = List.of(PROD_ENV, DEV_ENV); | ||
| } |
There was a problem hiding this comment.
Constants 클래스를 static으로 선언하신 이유가 궁금해요! static이 붙은 클래스는 내부 클래스로 선언해야하니까 EnvironmentConstants enum 클래스도 생성된 것으로 보이는데, EnvironmentConstants 클래스의 추가적인 역할이 없는 것 같아서요! 만약 static 을 떼면 위 enum 클래스도 불필요해져 SpringEnvironmentUtil와 같은 외부에서 상수들을 사용할 때 depth가 적어져 좋아질 것 같다는 의견입니다!
아니면 제가 생각하지 못한 EnvironmentConstants 클래스의 다른 목적이 있는 것인지 궁금해요!!!
There was a problem hiding this comment.
내부 클래스에 static을 붙이는 이유는
static 없는 내부 클래스: 외부 클래스 참조(숨은 this$0 변수) → 메모리 누수 가능
static 있는 내부 클래스: 외부 클래스 참조 없음 → 메모리 관리 안전
으로 얘기할 수 있을거 같아요! 관련 레퍼런스
다만 채영님이 질문하신 내용이 EnvironmentConstants의 필요성에 대한 거라면 Enum으로 관리함으로써 환경 변수를 타입 세이프하게 관리할 수 있다는 장점에 더해 값을 하드 코딩하는 것이 아닌 내부 클래스를 각 환경에 맞는 변수를 설정하고자 함이었습니다!! 제가 잘못이해한 부분이 있다면 말씀해주세요!!
There was a problem hiding this comment.
다만 채영님이 질문하신 내용이 EnvironmentConstants의 필요성에 대한 거라면 Enum으로 관리함으로써 환경 변수를 타입 세이프하게 관리할 수 있다는 장점에 더해 값을 하드 코딩하는 것이 아닌 내부 클래스를 각 환경에 맞는 변수를 설정하고자 함이었습니다!! 제가 잘못이해한 부분이 있다면 말씀해주세요!!
맞습니다. EnvironmentConstants의 필요성에 관해 드렸던 질문이었어요!
저의 경우는 EnvironmentConstants 없이 Constants만(물론 다른 이름으로) 상위 클래스에 존재하여도 외부에서 Constanst.PROD_ENV와 같이 사용할 수 있기 때문에 타입안전과 하드코딩방지 측면 모두에서 만족시킬 수 있는 방법이라 생각하긴 하는데요! 경호님이 말씀하신 장점이 이 장점이 맞을까요?!?! 제가 잘못 이해했나, 혹은 이전 리뷰에서 잘 전달드리지 못했나 싶어 한번 더 여쭤봅니다!
There was a problem hiding this comment.
원래는 enum만으로 관리할 때 LOCAL.name() 처럼 상수에서 메서드를 굳이 호출하는게 좋지 않다고 생각해서 enum으로 관리하되, 외부에서는 상수를 호출하는 것처럼 관리하려했습니다!
채영님 말씀을 듣고 생각해보니 상수를 굳이 enum으로 관리할 필요는 없겠더라구요!! 그래서 수정했습니다!! 좋은 의견 감사해요!!!
아래는 enum만으로 관리할 때 LOCAL.name() 처럼 상수에서 메서드 호출 예시입니다! name 을 호출하는것도 문제지만 개인적으로 prod와 dev를 함께 확인하기 위한 메서드도 지금 굳이.?라는 생각이 들어서 위의 수정처럼 class에서 상수를 관리하고자합니다! 이 부분도 의견 있으시면 반영할게요!! 완전 필요한건 아니라고 생각해서 의견 많이많이 내주시면 좋습니다!
@Getter
@AllArgsConstructor
public enum EnvironmentConstants {
PROD("prod"),
DEV("dev"),
LOCAL("local"),
;
private final String value;
public static List<String> getProdAndDevEnv() {
return List.of(PROD.name(), DEV.name());
}
}@Component
@RequiredArgsConstructor
public class ProfileResolver {
private final Environment environment;
public String getCurrentProfile() {
return getActiveProfiles()
.filter(profile -> profile.equals(PROD.name()) || profile.equals(DEV.name()))
.findFirst()
.orElse(LOCAL.name());
}
public boolean isProdProfile() {
return getActiveProfiles().anyMatch(PROD.name()::equals);
}
public boolean isDevProfile() {
return getActiveProfiles().anyMatch(DEV.name()::equals);
}
public boolean isProdAndDevProfile() {
return getActiveProfiles().anyMatch(getProdAndDevEnv()::contains);
}
private Stream<String> getActiveProfiles() {
return Arrays.stream(environment.getActiveProfiles());
}
}There was a problem hiding this comment.
지금 방식 좋은 것 같습니다!! 의견 내기 조금 조심스러웠는데 잘 듣고 반영해주셔서 감사해요 🥹
There was a problem hiding this comment.
정말 아~~~~무 걱정말고 많이많이 피드백해주세요!! 감사합니다ㅎㅎㅎ
| public boolean isProdProfile() { | ||
| return getActiveProfiles().anyMatch(PROD_ENV::equals); | ||
| } | ||
|
|
||
| public boolean isDevProfile() { | ||
| return getActiveProfiles().anyMatch(DEV_ENV::equals); | ||
| } | ||
|
|
||
| public boolean isProdAndDevProfile() { | ||
| return getActiveProfiles().anyMatch(PROD_AND_DEV_ENV::contains); | ||
| } |
There was a problem hiding this comment.
위 메서드는 사용되지 않는 메서드로 보이는데, 어떤 목적으로 구현하셨는지 궁금합니다!
There was a problem hiding this comment.
이 부분은 저도 고민했던 부분인데요!! 아직은 사용하지 않을 메서드긴하지만,
추후에 인프라 세팅이 완료된 근시점에 필요한 메서드라고 생각해서 넣어두었습니다!! 아래는 그 예시입니다!
@Bean
@Order(1)
@ConditionalOnProfile({PROD, DEV, LOCAL})
public SecurityFilterChain swaggerFilterChain(HttpSecurity http) throws Exception {
defaultFilterChain(http);
http.securityMatcher(getSwaggerUrls()).httpBasic(withDefaults());
http.authorizeHttpRequests(
(springEnvironmentUtil.isDevProfile() || springEnvironmentUtil.isProdProfile())
? authorize -> authorize.anyRequest().authenticated()
: authorize -> authorize.anyRequest().permitAll());
return http.build();
이외에도 Cookie 관련된 세팅에 환경에 따라 다르게 설정(예: Samsite)하는 경우도 있다고 알고 있어서 이부분 추가 의견 부탁드려요!!
There was a problem hiding this comment.
추후 사용될 메서드라면 삭제하지 않아도 좋을 것 같아요! 설명 감사합니다!!
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class SpringEnvironmentUtil { |
There was a problem hiding this comment.
이 클래스가 profile을 관리하는 역할을 담당했다는 걸 한번에 인지하지 못했었는데요..! 클래스명이 조금은 추상적이라고 생각했습니다! 다른 이름으로 변경하는 것은 어떨지 조심스럽게 제안드려봅니다! (ProfileResolver, ActiveProfileResolver, ActiveProfileProvider 등...?)
There was a problem hiding this comment.
ProfileResolver가 더 적합해보여서 변경했습니다! 좋은 의견 감사해요!!
| spring: | ||
| config: | ||
| activate: | ||
| on-profile: dev |
There was a problem hiding this comment.
이렇게 dev, prod와 같은 profile의 이름이 yml 파일에 정의되어 있다면 @Value를 통해서 active profile 이름을 가져올 수 있을 것이라고 생각되는데요.
@Value("${spring.config.activate.on-profile}")
private String activeProfile;위와 같은 방식이 아닌, SpringEnvironmentUtil 클래스 내 Environment 클래스를 통해서 activeProfile을 가져오도록 하신 이유가 궁금합니다!!
There was a problem hiding this comment.
이 부분은 사실 스타일(?) 차이라고 생각하는데요! 개인적으로는 어느정도 공통으로 쓰이는 것은 util로 뒀던 편이라 이렇게 구현했습니다!
@value 방식이 더 적합해보인다면 찬기님 의견도 들어보고 바꿔도 좋을거 같아요!!
There was a problem hiding this comment.
문자열 값이 두 곳에서 함께 관리되어 휴먼에러 발생 측면에서 걱정이 돼 드렸던 리뷰였는데요! 스타일 차이이기도 하고 사실 dev, prod와 같은 값들은 휴먼에러 발생 가능성이 낮으니 유지해도 좋을 것 같아요!
There was a problem hiding this comment.
전체적으로 정말 고민해서 Swagger 세팅을 해주신게 눈에 띄네요...!! 정말 고생하셨습니다!!!
다만 구현 맥락은 어느정도 이해하였는데, 전체적으로 파편화가 많이 되어있다보니 현재 PR 에선 추가 설명이 더 필요할 것 같습니다..!
추가로 저희 JWT 를 활용할 예정이라면, Swagger 에 Authorization Bearer 헤더 넣을 수 있는 설정도 포함해주시면 좋을 것 같아요! (아래는 예시)
Components components = new Components().addSecuritySchemes(jwtSchemeName,
new SecurityScheme()
.type(SecurityScheme.Type.APIKEY)
.in(SecurityScheme.In.HEADER)
.name("Authorization")
);
(컨벤션)
SpringEnvironmentUtil -> ProfileResolver
오 이 부분도 고민했던 부분인데 집어주셔서 감사합니다! 추가했어용 |
관련 이슈가 있다면 적어주세요.
issue #2
📌 개발 이유
💻 수정 사항
1. Swagger 설정 추가 (
SwaggerConfig.java)swagger.version값을application.yml에서 주입받도록 설정.SpringEnvironmentUtil을 활용.Info객체에 정의 (제목, 설명, 버전, GitHub 링크 포함).2. URL 및 환경 상수 정의 (
UrlConstants.java,EnvironmentConstants.java)UrlConstants.java8080), 로컬 웹(3000) URL 상수 추가.EnvironmentConstants.javaprod,dev,local환경 상수 추가.3. 현재 활성화된 Spring Profile을 반환하는 유틸리티 (
SpringEnvironmentUtil.java)prod,dev,local중 하나로 반환.isProdProfile(),isDevProfile(),isProdAndDevProfile()등의 메서드 추가.4. Swagger 관련 설정 추가 (
appication-dev.yml,application-prod.yml)/swagger-ui로 설정.application/json;charset=UTF-8로 지정.5. 헬스 체크 API 문서화 (
HealthApi.java,HealthController.java)HealthApi.java: API 설명을 추가하는 Swagger 어노테이션 적용.HealthController.java:/api/health엔드포인트에서"ok"응답을 반환.🧪 테스트 방법
http://localhost:8080/swagger-ui/index.html/api/health엔드포인트 호출 후"ok"응답이 오는지 확인.⛳️ 고민한 점 || 궁금한 점
prod,dev서버 URL이 연결되지 않아, 향후 배포 서버 주소 확정 후 반영🎯 리뷰 포인트
EnvironmentConstants,UrlConstants와SpringEnvironmentUtil유틸리티 를 적용했는데, 그 필요성에 대해 논의되지 않은 상태라 이에 대해 의견 부탁드립니다!