feat: 게시글 생성,수정,상세조회,삭제 기능 추가#71
Conversation
SangkiHan
left a comment
SangkiHan
left a comment
There was a problem hiding this comment.
모든 객체 앞에 final을 붙히셨는데 특별한 이유가 있으실까요????
src/main/java/org/myteam/server/board/repository/BoardCountRepository.java
Show resolved
Hide resolved
| */ | ||
| @Transactional | ||
| public void deleteBoard(final Long boardId, final CustomUserDetails userDetails) { | ||
| final Member member = memberRepository.findByPublicId(userDetails.getPublicId()) |
There was a problem hiding this comment.
Member도 마찬가지로 MemberReadService로 만들어두고 재활용하면 좋을 것 같습니다.
해당 도메인 외의 다른 도메인의 Repository가 여러 서비스에 의존되어있으면 프로젝트 규모가 커졌을 때 어디서 사용이 되는지 확인하기가 힘들 것 같습니다ㅠ
There was a problem hiding this comment.
member와 category select부분들도 readService 분리 했습니다 :)
|
|
||
| return ip; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
이건 if문이 다 동일해보이는데 작업중이신건가요?
There was a problem hiding this comment.
if문 동일해보이지만, 클라이언트와 서버 사이에 다른 것들이 개입되면서 request.getRemoteAddr()의 내용이 변조될 수도 있다고 하더라구요.
대신 추가적인 header가 생기면서 원래 정보가 거기에 저장된다고 하길래
어떤 장비가 중간에 개입하든 정확한 IP주소를 얻기 위해 X-Forwared-For 헤더부터 검증한뒤 내려가는 코드를 쓴다고 해서 이렇게 작업했습니다!
There was a problem hiding this comment.
ip == null || ip.length() == 0 조건이 true면 저 3개의 로직이 다 적용될텐데 결국 ip = request.getRemoteAddr();가 최종으로 되고 위에꺼는 무용지물이 되는거 아닌가요...?
There was a problem hiding this comment.
이런것도 묶어봐도 좋을 것 같아요
public static String getRemoteIP(HttpServletRequest request) {
String ip = request.getHeader("X-FORWARDED-FOR");
ip = getValidIp(request, ip, "Proxy-Client-IP");
ip = getValidIp(request, ip, "WL-Proxy-Client-IP");
return (checkIp(ip)) ? request.getRemoteAddr() : ip;
}
private static String getValidIp(HttpServletRequest request, String currentIp, String headerName) {
return checkIp(currentIp) ? request.getHeader(headerName) : currentIp;
}
private static boolean checkIp(String currentIp) {
return currentIp == null || currentIp.isEmpty();
}There was a problem hiding this comment.
그리고 ip.length() == 0 또는 ip.size() == 0 보다는 isEmpty가 훨씬 가독성이 좋습니다!
위에 제가 짠 묶은 거는 이게 큰 기능은 아니라서 굳이 묶을 필요는 없을거 같고 isEmpty만 바꿔주시면 될듯합니다.
| @PostMapping | ||
| public ResponseEntity<ResponseDto<BoardResponse>> saveBoard( | ||
| @AuthenticationPrincipal final CustomUserDetails userDetails, | ||
| @Valid @RequestBody final BoardSaveRequest boardSaveRequest, |
There was a problem hiding this comment.
매 Controller마다 @AuthenticationPrincipal를 가져와서 Service요청값마다 넣어줘야하는게 좀 비효율적인거같은데 개선방안 없을까요..
There was a problem hiding this comment.
요건 저도 비효율적인 것 같은데 어떻게 해결하면 좋을지 잘 모르겠네요..ㅠㅠ 찾아보겠습니다 :)
| @OneToOne(fetch = FetchType.LAZY) | ||
| @MapsId | ||
| @JoinColumn(name = "board_id") | ||
| private Board board; |
There was a problem hiding this comment.
아하 그럼
private Long id가 더 자연스럽겠네요
|
혹시 카테고리 Entity는 어떤용도인가요? |
|
@SangkiHan 종욱님이 만드신 엔티티인데, 네비게이션 바에 들어가는 카테고리 입니다! |
4 participants
기능 설명
작업 내용
추가 작업 예정
테스트