-
Notifications
You must be signed in to change notification settings - Fork 26
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
[BE4] 최현산 #1
base: main
Are you sure you want to change the base?
[BE4] 최현산 #1
Conversation
private void validateAuthorPermission(Answer answer, String username, String errorMessage){ | ||
if (!answer.getAuthor().getUsername().equals(username)) { | ||
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, errorMessage); | ||
} | ||
} |
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 상태 코드에 따라 세세하게 나누면 더 좋을 것 같네요
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.
말씀 해주신 대로, 관련된 부분은 좀 더 세세하게 나눠서 생각해볼 부분이 많은 것 같습니다.
현재는 단순하게 BAD_REQUEST에 대해서만 체크하게 되어있어 제가 보기에도 부족하게 보여집니다.
http 상태 코드와 권한에 관한 지식이 아직 부족하다고 느껴져서, 관련된 공부 진행하면서 수정해보도록 하겠습니다!
@Controller | ||
@RequestMapping("/user") | ||
@RequiredArgsConstructor | ||
public class AuthorController { |
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.
네이밍 관련인데, Question이나 Answer도메인의 입장에서는 Author가 맞지만
백엔드 어플리케이션 전체로 봤을때는 Author보다는 Auth혹은 User가 적절하겠네요
Author서비스, Author리포, Author엔티티에도 같습니다.
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.
정확히 말씀해주신 것처럼 Question과 Answer 도메인을 바라보는 관점으로 Author라는 이름으로 짓고 사용했습니다.
이후에 추가 기능 구현을 하면서 저도 Author라는 관점 외의 기능을 제공하게 되는 상황에서 도메인 명이 적절치 않다는 생각이 들기 시작했던 것 같습니다.
이 부분도 수정하고 다음 REST API 버전으로 만들어 볼 때에도 참고하겠습니다.
피드백 감사합니다!
answer.content = content; | ||
answer.author = author; | ||
answer.createDate = LocalDateTime.now(); | ||
question.getAnswerList().add(answer); |
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.
question의 answerList에 answer를 추가 하는 것은 필요하지 않은 라인입니다.
연관관계의주인 이 글 확인해주시면 좋을 것 같네요
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.
두 엔티티 테이블이 조인을 이용해서 값을 가져오기 때문에 굳이 필요하지 않을 것으로 생각됩니다.
save되면서 answer가 영속화 되어서 이후에 조회 시에도 정상적으로 가져와 질 것으로 예상됩니다.
피드백 감사합니다!
Author author = new Author(); | ||
author.username = username; | ||
author.email = email; | ||
author.password = "SOCIAL_USER"; // 소셜 로그인 유저는 임의의 비밀번호 |
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.
소셜로그인 사용자의 비밀번호를 이후에 다시 초기화 하지 않는 것 같은데, 정적인 문자열의 경우에 보안의 문제점이 있습니다. uuid같은 난수를 생성하는걸 추천드립니다.
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.
처음 소셜 로그인을 구현하면서 임의로 지정해둔 비밀번호였는데, 구현 테스트 상 문제가 없어서 따로 수정을 하지 않은 것 같습니다.
이 부분은 보안과 큰 관련이 되는 부분인 만큼 앞으로 더 신경 써보겠습니다.
감사합니다!
private String email; | ||
|
||
@Column(unique = true) | ||
private String password; |
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.
패스워드를 unique로 설정시, 입력한 패스워드가 잘못되었을 경우, 누군가가 해당 패스워드를 쓰고 있음을 알 수 있으니 보안의 문제점이 있습니다.
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.
패스워드가 유니크로 됐을 때 발생할 수 있는 보안 상의 문제점을 짚어주셔서 감사합니다!
바로 수정해서 적용해보겠습니다.
|
||
private String content; | ||
|
||
@ManyToOne(fetch = FetchType.LAZY) |
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.
comment를 보여 줄 때에 작성자의 이름도 같이 보여주는 로직이 있는 것으로 아는데
FetchType.LAZY로 설정하면 N+1문제가 발생할 가능성이 높습니다.
FetchType.EAGER로 설정해도 ManyToOne이니, 쿼리를 보낼때 한번의 db hit로 작성자의 정보를 가져 올 수 있으니 참고바랍니다.
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.
LAZY로 패치하는 경우에 발생할 수 있는 부분에 대해 생각을 못해본 것 같습니다.
N + 1 발생하는 상황에 대해서 더 공부해보고 해당 부분을 수정해보겠습니다!
감사합니다.
comment.author = author; | ||
comment.content = content; | ||
comment.createDate = LocalDateTime.now(); | ||
answer.getCommentList().add(comment); |
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.
Answer.createAnswer와 똑같은 내용입니다.
private LocalDateTime createDate; | ||
private LocalDateTime modifyDate; | ||
|
||
@ManyToOne(fetch = FetchType.LAZY) |
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.
Comment의 Author와 같은 내용입니다.
spring.security.oauth2.client.registration.kakao.client-id=f43ad4748b49dfe40cdae535e3ec0591 | ||
spring.security.oauth2.client.registration.kakao.client-secret=J7TzJfJ1agO06I8YFfgAz6SirXulA3tQ | ||
spring.security.oauth2.client.registration.kakao.redirect-uri=http://localhost:8080/login/oauth2/code/kakao |
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.
공개된 깃허브 리포지터리에 암호화된 값이나 개인 정보가 들어있는 파일을 업로드 할 때엔 .gitignore에 등록하셔서 깃에 포함되지 않게 해야합니다.
지금 여기에 적힌 값들은 이미 깃에 올라와있으니, 카카오디벨로퍼즈에서 키를 재발급 받으시길 권장드립니다.
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.
일단 풀 리퀘스트는 그대로 두고, 포크한 저장소에서 바로 수정했습니다!
감사합니다
@ChoiHyunSan 너무 고생하셨습니다. 이거 마지막 강에 있는 예제를 혼자힘으로 구현할 수 있을 때 까지 연습하시죠 ㅎ 다 하셨으면 저한테 알려주세요 ^ ^ |
우선 내일까지 REST API 버전으로 만드는걸 목표로 하고 있는데, 올려주신 리액트 강좌를 10강? 정도까지 진행을 하다가 못했는데, 병행해서 보면서 진행해보겠습니다. 감사합니다! |
교재에 있는 3-14 기능 구현까지 완료해봤습니다.
코드가 정리가 안된 부분이 보이고, 웹페이지 상에서 테스트는 해봤지만 테스트 코드가 충분히 작성되지 않아서 해당 부분을 좀 더 다뤄볼 생각입니다.
잘 부탁드립니다!