[게시판 + DB 연동] 최정훈 제출합니다.#43
Conversation
Donghwan814
left a comment
There was a problem hiding this comment.
안녕하세요 정훈님!
우선 이번 주차 과제 제출하시느라 고생 많으셨습니다!
이번 과제는 저번 주 과제와 크게 다르지 않은 부분이 많았기에 이번에는 코드 자체의 수정 사항보다는 왜 이렇게 작성했는지, 어떤 의도로 사용했는지에 대한 질문 위주로 피드백을 남겼습니다.
이번 주차는 특히 DB 연동이 중요한 부분이라고 생각됩니다.
다만 제출해주신 이미지를 확인해보니 현재는 Postman으로 API 테스트한 결과만 첨부되어 있는 것 같습니다.
MySQL과 연동해서 작업해주신 만큼, 실제로 게시글 생성, 조회, 수정, 삭제가 MySQL DB에 정상적으로 반영되는지도 함께 확인할 수 있도록 DB 조회 결과 화면도 같이 첨부해주시면 더 좋을 것 같습니다!
There was a problem hiding this comment.
현재 설정 파일에 서버의 민감한 정보가 그대로 노출되어 있는 것 같습니다.
DB 비밀번호, API Key, JWT Secret 같은 값이 GitHub에 올라가면 단순 실수가 아니라 실제 보안 사고로 이어질 수 있습니다.
누군가 해당 정보를 가져가서 DB에 접근하거나, API를 무단 사용하거나, 토큰을 위조하는 등의 문제가 발생할 수 있기 때문에 반드시 주의해서 GitHub에 올리시길 바랍니다.
이런 민감 정보를 올릴 때는 코드에 직접 작성하지 말고 .env 파일에 따로 분리해서 관리하는 것이 좋습니다.
아래 링크를 통해 어떻게 활용하는 것인지 학습해 주실 것을 권장드리며, 보는 즉시 수정해 주시길 바랍니다.
.env 활용 방법
There was a problem hiding this comment.
말씀해주신 내용으로 민감 정보가 GitHub에 노출될 경우 보안 사고로 이어질 수 있다는 점을 인지했습니다.
DB 관련 정보 등을 .env파일로 분리하고 .gitinogre에 .env를 추가하여 GitHub에 올라가지 않도록 수정하겠습니다.
There was a problem hiding this comment.
현재는 파일 내용이 보이지 않아 어떤 내용을 작성해주셨는지 정확히 확인하기는 어렵습니다.
다만 이번 과제 범위에서는 checksums 폴더를 생성하고 관련 파일들을 사용할 필요성이 크지는 않아 보여서, 혹시 해당 파일들을 추가해주신 특별한 의도가 있으신지 궁금합니다.
만약 checksum을 활용해서 파일의 무결성 검사를 진행하려는 목적이었다면 좋은 시도라고 생각합니다. 다만 그 목적이 명확하지 않은 상태에서 생성된 파일이라면, 먼저 해당 파일이 어떤 역할을 하는지와 왜 필요한지를 이해한 뒤 적용하는 것이 더 좋을 것 같습니다.
불필요한 파일이 프로젝트에 포함되면 구조가 복잡해지고, 다른 팀원이 봤을 때 해당 파일의 역할을 파악하기 어려울 수 있기 때문에 한 번 확인해보시면 좋을 것 같습니다!
There was a problem hiding this comment.
checksums 폴더는 의도적으로 추가한 것은 아니고 git add . 과정에서 모두 추가된 것 같습니다. 해당 파일이 필요한 것 같진 않아서 제거하고 .gitignore에서도 GitHub에 올라가지 않도록 확인하겠습니다.
There was a problem hiding this comment.
executionHistory 폴더 외에도 fileChanges, fileHashes, buildOutputCleanup 등 여러 폴더와 파일들이 함께 포함되어 있는 것 같습니다.
정훈님께서 어떤 의도로 해당 파일들을 추가해주셨는지는 정확히 알 수 없지만, 제 생각에는 직접 작성한 코드라기보다는 빌드 과정이나 LLM을 사용해 작업하는 과정에서 자동으로 생성된 파일일 가능성도 있어 보입니다.
물론 제 판단이 틀렸을 수도 있기 때문에, 혹시 이 파일들이 어떤 이유로 생성되었고 프로젝트에 포함되어야 하는 특별한 이유가 있는지 정훈님의 의견을 한 번 들어보고 싶습니다.
만약 의도적으로 추가한 파일이 아니라면, 프로젝트 코드와 직접 관련 없는 자동 생성 파일들은 .gitignore에 등록해서 Git에 올라가지 않도록 관리하는 것이 더 깔끔할 것 같습니다!
There was a problem hiding this comment.
위와 마찬가지로 해당 파일들은 의도적으로 추가한 것은 아니고, git add . 과정에서 모두 추가된 것 같습니다. 마찬가지로 제거하고 .gitignore에서 GitHub에 올라가지 않도록 확인하겠습니다.
There was a problem hiding this comment.
예외 상황에 대해 IllegalArgumentException, Exception을 구분해서 상태코드를 반환해주신 점은 좋았습니다!
게시글이 없을 때는 404, 생성 성공 시에는 201, 삭제 성공 시에는 204를 사용해주신 부분도 REST API 응답 흐름을 잘 고려해주신 것 같습니다.
다만 현재는 각 Controller 메서드마다 try-catch가 반복되고 있어, Controller의 역할이 조금 많아진 느낌이 있습니다.
Controller는 보통 요청을 받고, Service를 호출한 뒤, 결과를 응답으로 반환하는 역할에 집중하는 것이 좋습니다.
반면 게시글이 존재하지 않는 경우처럼 의미 있는 예외를 발생시키는 책임은 Service에서 처리하고, 그 예외를 HTTP 상태코드로 변환하는 책임은 @RestControllerAdvice 기반의 전역 예외 핸들러로 분리하면 더 깔끔한 구조가 될 것 같습니다.
예를 들어 현재는 모든 메서드마다 아래와 같은 패턴이 반복되고 있습니다.
IllegalArgumentException → 404 Not Found
Exception → 500 Internal Server Error
이런 공통 예외 응답 처리를 전역 예외 핸들러로 분리하면, Controller는 본래 역할에만 집중할 수 있고 중복 코드도 줄일 수 있을 것 같습니다.
또한 e.printStackTrace()는 간단한 테스트 단계에서는 확인용으로 사용할 수 있지만, 실제 프로젝트에서는 로그 관리가 어렵기 때문에 SLF4J 같은 로깅 프레임워크를 사용하는 방식도 함께 학습해보시면 좋을 것 같습니다.
전체적으로 예외 상황을 고려해서 작성해주신 점은 좋았고, 다음 단계로는 Controller, Service, 전역 예외 처리의 역할을 분리하는 아키텍처 구조를 학습해보시면 코드가 더 깔끔해질 것 같습니다!
There was a problem hiding this comment.
말씀해주신대로 Controller마다 try-catch 문이 반복되면서 Controller의 역할이 많아지고 코드 중복이 발생한다는 것을 인지했습니다.
예외 발생은 Service 계층에서 처리하도록 하고, PostNotFoundException, DuplicatePostException과 같은 커스텀 예외를 추가하겠습니다.
또한 e.printStackTrace() 대신 SLF4J의 사용에 대해서도 알아보겠습니다.
There was a problem hiding this comment.
저번에 @Getter, @Setter 어노테이션에 대해서 설명드렸던 것 같은데, 현재 코드에서는 단순 getter/setter 메서드를 직접 하나씩 작성해주신 것으로 보입니다.
물론 직접 작성해도 기능상 문제는 없지만, 단순히 필드 값을 꺼내고 넣는 역할만 한다면 Lombok의 @Getter, @Setter를 사용하는 것이 코드 길이를 줄이고 가독성을 높이는 데 더 좋을 것 같습니다.
혹시 직접 getter/setter를 작성해주신 특별한 이유가 있으신지 궁금합니다!
There was a problem hiding this comment.
Post Entity에서는 @Getter, @Setter를 사용해주셨는데, PostResponse에서는 getter 메서드를 직접 작성해주신 부분이 조금 궁금했습니다!
또한 Entity에 기본 생성자 public Post() {}를 직접 작성해주셨는데, 이 기본 생성자가 왜 필요한지 한 번 학습해보시면 좋을 것 같습니다.
JPA Entity에서 기본 생성자가 필요한 이유가 있고, Lombok을 사용한다면 이를 어노테이션으로 처리하는 방법도 있습니다.
이미 Lombok을 사용하고 있는 상황이라면 어떤 경우에는 어노테이션을 쓰고, 어떤 경우에는 직접 메서드나 생성자를 작성하는지 기준을 잡아보면 좋을 것 같습니다.
혹시 Entity에서는 Lombok을 사용하고, DTO에서는 getter를 직접 작성해주신 특별한 이유가 있으실까요?
There was a problem hiding this comment.
Service 계층에서 게시글 생성, 조회, 수정, 삭제 로직을 역할별로 잘 분리해주신 점 좋았습니다!
Repository를 통해 게시글 존재 여부를 확인하고, 예외 상황까지 고려해서 작성해주신 부분도 인상 깊었습니다.
다만 Service 계층에서는 DB 조회뿐만 아니라 생성, 수정, 삭제 같은 데이터 변경 작업도 함께 일어나기 때문에 @Transactional이 어떤 역할을 하는지 한 번 학습해보시면 좋을 것 같습니다.
특히 수정이나 삭제처럼 DB 상태가 변경되는 로직에서 트랜잭션이 없을 때 어떤 문제가 생길 수 있는지 확인해보면 도움이 될 것 같습니다.
그리고 예외 처리 부분에서 궁금한 점이 있습니다.
현재 여러 메서드에서 IllegalArgumentException을 catch한 뒤 다시 throw e로 그대로 던지고 있는데, 제가 알기로는 이렇게 작성하면 예외를 잡지 않은 것과 동작상 큰 차이가 없는 것으로 알고 있습니다.
혹시 이렇게 try-catch 구조로 작성해주신 특별한 의도가 있으실까요?
또 하나 궁금한 부분은 수정 메서드입니다.
postData.setCreatedAt(LocalDateTime.now());메서드명과 필드명을 보면 createdAt은 보통 “생성 일시”를 의미하는 값으로 사용됩니다.
그런데 게시글 수정 시점에 createdAt을 다시 현재 시간으로 변경하면, 기존 생성 시간이 수정 시간으로 덮어써질 수 있을 것 같습니다.
보통 생성 일시와 수정 일시는 역할이 다르기 때문에, 수정 시간을 관리하고 싶다면 updatedAt 같은 필드를 따로 만들어 관리하는 방식이 더 자연스러워 보입니다.
혹시 수정 시 createdAt을 다시 변경하도록 구현해주신 이유가 있으실까요?
There was a problem hiding this comment.
IllegalArgumentException을 catch한 뒤 다시 throw e로 던지는 부분은 특별한 의도가 있었던 것은 아니고, 멋쟁이사자처럼의 강의를 참고하면서 그렇게 작성하게 된 것 같습니다. 말씀해주신 것 처럼 큰 차이가 없기 때문에 try-catch 문을 제거하겠습니다.
수정 메서드에서 createdAt을 다시 설정하는 부분은 처음에는 수정할 때 createdAt이 같이 수정되어도 상관없다고 생각했는데 말씀해주신대로 createdAt은 생성 시간을 의미하기에 게시글을 수정할 때 같이 수정되지 않고, 수정 시간을 따로 관리하는 필드를 따로 두는 것이 좋을 것 같습니다.
There was a problem hiding this comment.
환경변수로 분리해서 관리하려고 시도해주신 점은 좋았습니다!
다만 파일을 확인해보니 .gitignore 파일이 따로 보이지 않아서, 실제 환경변수 값이 어디에 저장되고 있는지 궁금했습니다.
보통 DB 비밀번호, API Key, JWT Secret 같은 민감한 값은 .env 파일에 따로 저장하고, .gitignore에 .env를 등록해서 GitHub에 올라가지 않도록 관리합니다.
그런데 현재 구조에서는 .env 파일로 관리한 흔적이 잘 보이지 않아, 혹시 환경변수 값을 프로젝트 내부 설정 파일에 직접 작성해두신 건지 궁금합니다.
환경변수 처리를 어떤 방식으로 구현해주셨는지 한 번 설명해주실 수 있을까요?
There was a problem hiding this comment.
.env 파일이 git add . 과정에서 포함되었으나 스테이징에서 제거하는 명령어를 사용하여 제거했습니다. 앞으로는 .gitignore에 .env를 추가하여 GitHub에 올라가지 않도록 관리하겠습니다.
|
안녕하세요 정훈님! 이번 주차는 지난 과제에 DB 연동이 추가된 형태인데요. 지난주 리뷰에서 함께 이야기 나누었던 코드 개선 포인트들을 어떻게 고민하고 적용해 보셨는지 중심적으로 여쭤보고자 합니다! 편하게 답변해 주세요. |
There was a problem hiding this comment.
포스트맨 캡처 화면을 보니 게시글 삭제 성공 시 반환값으로 1이 찍히는 것 같습니다! 물론 이렇게 처리해도 기능상 문제는 없지만, "삭제되었습니다"와 같은 명확한 메시지나 성공 상태를 함께 내려주면 클라이언트 입장에서 훨씬 직관적일 것 같아요. API 공통 응답 포맷을 어떻게 구성하면 더 보기 좋을지 한 번 고민해 보시고 적용해 보시면 좋겠습니다!
There was a problem hiding this comment.
삭제 성공 시 1을 반환하는 것은 단순히 삭제 여부를 확인하기 위한 것이었지만, 말씀주신대로 클라이언트의 입장에서는 단순히 숫자를 받는 것 보다 명확한 메시지가 표시되는 것이 더 직관적이라고 생각합니다.
삭제 요청은 204 No Content로 처리하여 상태 코드로 성공 여부를 표현하게 해보겠습니다.
| import org.springframework.data.domain.Page; | ||
| import org.springframework.data.domain.Pageable; | ||
|
|
||
| public interface PostService { |
There was a problem hiding this comment.
정훈님! 현재 PostService 인터페이스를 보면 JpaRepository가 기본으로 제공하는 save, findById 등의 메서드가 그대로 똑같이 선언되어 있습니다.
postRepisotry에 있는 코드들이 postService에 또 선언됐다고 볼 수 있는데요, postService 인터페이스를 만드신 의도가 궁금합니다!
There was a problem hiding this comment.
save, findById 같은 메서드가 JpaRepository에서 기본으로 제공된다는 점을 제대로 인지하지 못하고, Service에도 비슷하게 선언한 것 같습니다. 앞으로는 Repository가 제공하는 기본 기능과 Service에서 처리해야 하는 역할을 구분해서 작성하겠습니다.
| logging.level.org.springframework.web=DEBUG | ||
| logging.level.org.hibernate=DEBUG | ||
|
|
||
| spring.jpa.hibernate.ddl-auto=update |
There was a problem hiding this comment.
JPA 설정에서 ddl-auto 설정을 update로 지정해 주셨네요! 혹시 특별히 update 설정을 선택하신 이유나 의도가 있으신지 궁금합니다!
There was a problem hiding this comment.
ddl-auto=update는 코드 작성 중 테이블을 직접 수정하지 않고 자동으로 반영하기 위해서 사용했습니다.
과제명
게시판 + DB 연동
💡 작업 내용
🔗 참고 링크
🤔 느낀 점 / 어려웠던 점