-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Setting : 스웨거 경로 수정 #214
Setting : 스웨거 경로 수정 #214
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review by ChatGPT
@@ -10,7 +10,7 @@ | |||
@EnableJpaAuditing | |||
@OpenAPIDefinition( | |||
servers = { | |||
@Server(description = "local Admin", url = "http://localhost/admin"), | |||
@Server(description = "local Port Admin", url = "http://localhost:8443/admin"), | |||
@Server(description = "server Admin", url = "https://api.devlens.work/admin") | |||
} | |||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치를 살펴보면 다음과 같은 점들이 있습니다.
-
변경 사항 이해:
@Server
의url
이http://localhost/admin
에서http://localhost:8443/admin
으로 변경되었습니다. 이 변경은 로컬 개발 환경에서의 포트 번호를 변경한 것으로 보입니다. 변경된 포트가 올바른지, 그리고 다른 설정이나 서비스에서 이 포트를 사용하고 있는지 확인이 필요합니다. -
일관성 유지: 만약 이와 같은 설정을 사용하는 다른 부분이 있다면, 해당 부분에서도 같은 포트 변경이 일관되게 적용되어야 합니다. 그렇지 않으면 혼란을 초래할 수 있습니다.
-
주석 추가: 변경 내용을 더 명확히 이해할 수 있도록 주석을 추가하는 것이 좋습니다. 예를 들어, 왜 포트를 변경했는지 설명하는 주석이 코드에 명시되어 있으면 좋을 것 같습니다.
-
HTTPS 사용 고려: 로컬 환경에서도 보안이 중요할 수 있으므로, 가능하다면 HTTPS를 사용하는 것이 좋습니다. 로컬에서도 보안 연결에 대한 테스트를 할 수 있는 환경을 구성하면 더 안전할 것입니다.
-
테스트: 변경이 이루어진 후에는 반드시 로컬 환경에서 새 URL이 잘 작동하는지 테스트해 보아야 합니다. 서버가 실행 중인지, 올바른 포트가 사용되고 있는지를 확인해야 합니다.
위의 사항들을 참고하여 코드가 무사히 운용되기를 바랍니다.
@@ -17,6 +17,8 @@ public void addCorsMappings(CorsRegistry registry) { | |||
"https://api.devlens.work", | |||
"https://local.devlens.work", | |||
"http://localhost/admin", | |||
"http://localhost/main", | |||
"http://localhost/admin", | |||
"http://localhost/main") | |||
.allowedMethods("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS") | |||
.allowedHeaders("*") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대한 간단한 코드 리뷰를 진행하겠습니다.
-
중복 URL:
"http://localhost/admin"
와"http://localhost/main"
가 중복되어 있습니다. 두 번째"http://localhost/admin"
는 불필요하므로 제거하는 것이 좋습니다. 중복된 항목은 혼란을 초래할 수 있습니다. -
CORS 정책: CORS 매핑을 추가하는 부분에서 허용되는 도메인이 너무 많이 열려 있는 것으로 보입니다.
allowedHeaders("*")
는 모든 헤더를 허용하도록 설정하지만, 이는 보안상 위험할 수 있습니다. 특정한 헤더만 허용하는 것이 더 좋습니다. -
다양한 도메인 과 환경: URL들이 localhost에 국한돼 있는데, 이는 개발 환경에서 유용할 수 있지만 배포 환경에서는 사용되지 않아야 합니다. 환경 변수나 프로파일을 사용하여 개발과 프로덕션 환경을 구분하는 방법을 고려해 보세요.
-
유지보수: CORS 설정이 많이 바뀌는 경우, 코드의 가독성과 관리성을 고려하여 설정을 외부 설정 파일로 분리할 수 있습니다.
이 코드 패치는 기본적인 CORS 설정을 잘 수행하고 있지만, 보안과 중복 항목 관리를 위한 개선이 필요합니다.
@@ -10,7 +10,7 @@ | |||
@EnableJpaAuditing | |||
@OpenAPIDefinition( | |||
servers = { | |||
@Server(description = "local Main", url = "http://localhost/main"), | |||
@Server(description = "local Port Main", url = "http://localhost:8444/main"), | |||
@Server(description = "server Main", url = "https://api.devlens.work/main") | |||
} | |||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대한 간단한 코드 리뷰를 진행하겠습니다.
-
변경 내용 이해:
- 기존의 서버 주소가
http://localhost/main
에서http://localhost:8444/main
으로 변경되었습니다. 이는 개발 환경에서 로컬 포트 번호를 8444로 수정한 것으로 보입니다.
- 기존의 서버 주소가
-
버그 리스크:
- 만약 애플리케이션이 다른 서비스나 의존성에 의해 기본적으로 8080 포트를 사용하고 있다면, 이 변경이 문제를 일으킬 수 있습니다. 이 경우, 이를 사용하는 클라이언트 또는 문서화된 API가 새로운 포트로 업데이트되지 않으면, 연결 실패가 발생할 수 있습니다.
-
개선 제안:
- 변경된 포트 번호에 대한 문서화나 주석 추가를 고려해보세요. 이는 다른 개발자들이 이 변경 사항을 쉽게 이해할 수 있도록 도와줍니다.
- 로컬 주소와 포트 수정을 환경 변수로 관리하여, 코드 내에서 하드코딩을 피하는 것이 좋습니다. 이를 통해 개발, 테스트, 프로덕션 환경에 맞춰 유연하게 설정할 수 있습니다.
@Server
배열에 다른 환경 (예: 테스트 환경, 프로덕션 환경)도 추가하는 것을 고려해보세요. 이를 통해 API 문서가 환경에 따라 더 명확하게 구성될 수 있습니다.
이 외에도 문제가 발생하지 않도록 전체적인 연동성 및 테스트를 충분히 진행하는 것이 좋습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review by ChatGPT
"http://localhost/main") | ||
"http://localhost/main", | ||
"http://localhost:8443/admin", | ||
"http://localhost:8444/main") | ||
.allowedMethods("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS") | ||
.allowedHeaders("*") | ||
.allowCredentials(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 패치에 대한 간단한 리뷰를 진행하겠습니다.
-
중복 제거: 코드에서
http://localhost/admin
과http://localhost/main
이 두 번 정의되어 있습니다. 첫 번째 삭제한 것과 두 번째로 수정된 부분은 중복이 아닌 변동 사항입니다. 중복이 아닌가 확인하고 필요없는 줄을 지우는 것이 좋습니다. -
포트 번호 변경:
http://localhost/admin
과http://localhost/main
의 포트를 각각8443
과8444
로 변경하여 접근하는 URL을 수정했습니다. 이 변경이 의도한 대로 이루어졌는지 확인할 필요가 있습니다. 실제로 해당 포트에서 서비스가 작동하는지 점검해야 합니다. -
CORS 설정 점검:
allowCredentials(true)
를 사용하는 경우,allowedOrigins
에서 특정 도메인을 등록하는 것이 좋습니다.*
을 사용하는 것은 보안상 위험할 수 있습니다. 필요한 도메인만 허용하는 것이 안전합니다. -
마법의 숫자: 포트 번호 또는 URL을 코드에서 하드코딩하지 않고, 상수로 정의하는 것이 좋습니다. 이를 통해 코드 유지보수가 용이하고 가독성이 향상됩니다.
-
주석 추가: 코드 변경의 의도를 설명하는 주석을 추가하면 다른 개발자들이 코드를 이해하는 데 도움이 될 수 있습니다. 특히 CORS와 관련하여 추가적인 문서나 설명을 붙일 수 있습니다.
이 외에는 전체적인 가독성과 구조는 나쁘지 않지만, 위의 사항들을 고려하면 코드의 안정성과 유지보수성이 더욱 향상될 것입니다.
No description provided.