-
Notifications
You must be signed in to change notification settings - Fork 27
[이동훈_BackEnd] 11주차 과제 제출합니다. #4
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
base: main
Are you sure you want to change the base?
Conversation
Soundbar91
left a 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.
연휴 간 과제 하시느라 고생하셨습니다.
코멘트 확인 부탁드립니다 !
| try { | ||
| return ResponseEntity | ||
| .ok(articleService.getById(id)); | ||
| } catch(NoSuchElementException ex) { | ||
| return ResponseEntity.notFound().build(); | ||
| } |
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.
try-catch로 예외 처리를 해주셨네요 👍
| @RestController | ||
| @RequestMapping("/introduce") | ||
| public class IntroduceController { | ||
| @GetMapping | ||
| public String getIntroduce( | ||
| @RequestParam(name = "name", defaultValue="이동훈", required = false) String name | ||
| ) { | ||
| return "안녕하세요 제 이름은 " + name + "입니다!"; | ||
| } |
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.
html를 반환하고 있지 않는 것 같아요.
정적 자원, model, 타임 리프에 대해 학습 후 적용하면 좋을 것 같아요.
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.
쿼리 파라미터를 받았을 경우에는 텍스트를 반환하라고 나와있어서 제가 그것만 보고 구현을 했네요;;; 수정하겠습니다!
| return updatedAt; | ||
| } | ||
|
|
||
| public Article update(ArticleRequest request) { |
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.
model과 dto가 의존 관계를 가지고 있어요. model에서 dto와의 의존관계를 가져야할 지 생각해보면 좋을 것 같습니다.
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.
그러면 인자로 객체를 받아 의존 관계가 만들어지는 것 대신 인자에 수정하고자 하는 데이터(title, content)를 전달하는 형태로 하면 의존 관계가 사라질 것 같습니다
| import java.util.Map; | ||
| import java.util.NoSuchElementException; | ||
|
|
||
| @Repository |
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.
Repository 어노테이션은 보고서에서 따로 다루지 않았던 것 같은데, 설명 부탁드려도 될까요 ??
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.
Repository 어노테이션은 이 클래스가 데이터에 접근하는 계층임을 명시해주는 어노테이션입니다. 또한 제일 중요한 기능으로 Repository 어노테이션 내부에는 Component 라는 어노테이션이 포함되어 있는데 이것은 빈으로 등록하여 스프링 컨테이너에서 관리하겠다는 의미입니다. 저는 ArticleRepository의 객체를 빈으로 등록하는 것을 의도로 했는데 Component를 사용해도 되겠지만 처음에 말씀드린 데이터에 접근한다는 것을 명시해주기 위해 Repository 어노테이션을 사용했습니다.
- 추가적인 기능이 더 있나 찾아봤는데 데이터 접근 시 발생하는 예외를 DataAccessException로 변환해준다고 합니다.
https://www.inflearn.com/community/questions/1042796
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.
DataAccessException으로 반환해줘서 이전에 고생한 기억이..
| import com.example.bcsd.repository.ArticleRepository; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
| @Service |
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.
서비스도 동일합니다.
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.
Service는 비즈니스 로직을 수행하는 곳입니다. 비즈니스 로직이란 클라이언트의 요청에 따라 데이터를 처리 및 가공하는 것으로 Client(유저) <-> Controller <-> Service <-> Repository <-> DB(필요하다면) 와 같은 구조로 사용하는 이유는 각각 단일의 책임만을 지닐 수 있게 하기 위함입니다.
위와 마찬가지로 비즈니스 로직을 수행함을 명시하고 빈으로 등록하기 위해 Service 어노테이션을 달았습니다.
Soundbar91
left a 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.
DTO 패키지는 다른 프로젝트의 소스 코드를 보면 거의 다 Controller 패키지 안에 있던데 그렇게 지정하는 이유가 있나요??
제 생각으로는 요청과 응답을 클라이언트로부터 주고 받는 계층이 Controller 이므로 그런 것인지 궁금합니다!
레이어드 아키텍처로 봤을 때, 프레젠테이션 계층에 controller가 들어가는데 이때 요청값과 응답값을 사용하기 때문에 자연스럽게 들어가는 것 같습니다.
dto는 계층 간 데이터를 주고 받기 위해 설계됐기 때문에, 어떤 계층에 국한되지 않아 dto라는 패키지를 분리하는 케이스도 종종 있는 것 같아요. (저는 이렇게 작성하는 걸 선호합니다)
동훈님이 말씀해준 내용도 맞다고 생각하고, 이 부분도 부바부, 사바사라고 생각합니다.
대부분 dto를 사용하는 최종 계층에다 dto 패키지를 만들면 된다고 하는데, 그건 dto마다 다르므로 저도 따로 분할하는 것이 더 나아보인다고 생각합니다! |
Soundbar91
left a 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.
고생 많으셨습니다 !
10주차 과제는 잘 해주셔서 크게 코멘트 달 부분이 없는 거 같아요 !
Soundbar91
left a 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.
고생하셨습니다 ~
| @Column(length = 100) | ||
| @ColumnDefault("user") | ||
| private String role; |
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.
enum으로 관리하는 건 어떨까요 ?
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.
권한의 종류가 다양하다면 enum으로 관리하는 것이 적절한 것 같습니다!
우선은 user와 admin만 존재한다고 가정했기에 모든 권한의 기본 값으로 user를 지정했습니다. 만약 admin 권한을 가진 멤버가 필요하다면 DB 상으로 수정해주면 되기에 위와 같이 지정했습니다.
|
|
||
| @Column(length = 100) | ||
| @ColumnDefault("user") | ||
| @ColumnDefault("'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.
아마 예약어 때문에 안 먹혀서, 백틱 처리해야하는 걸로 알고 있어요
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.
아하...! 예약어로 뭐가 있는지 찾아봐야 할 것 같네요!!
| else if (uri.startsWith("/articles/")) { | ||
| ArticleResponse article = articleService.getArticleById(targetId); | ||
|
|
||
| if (!article.authorId().equals(loginUserId)) | ||
| throw new CustomException(ExceptionMessage.ACCESS_DENIED); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| else if (uri.startsWith("/members/")) { | ||
| MemberResponse member = memberService.getMemberById(targetId); | ||
|
|
||
| if (!member.id().equals(loginUserId)) | ||
| throw new CustomException(ExceptionMessage.ACCESS_DENIED); | ||
|
|
||
| return 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.
인증이 아닌 권한 체크 로직인거 같아요
인터셉터에서 권한 체크까지하기 보다는 서비스에 위임하는 게 어떨까요 ?
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.
자신의 게시글, 멤버 정보에 대해 수정하려는 지 체크하는 부분입니다.
서비스까지 도달했다는 것은 컨트롤러에서 요청이 수락되어 서비스로 넘어가고 비즈니스 로직을 실행하려는 차례인건데 서비스에서 체크하는 것보다는 인터셉터에서 미리 요청을 거부하는 것이 맞지 않나 하는 생각으로 인터셉터에 넣었습니다.
그러나 이를 구현하면서 들었던 생각이 인터셉터에서 서비스에게 get으로 자원을 조회하고 true가 반환되었다면 서비스에서도 다시 자원을 조회하는 부분이 있을건데, 이는 하나의 요청에서 같은 조회를 여러 번 하다보니 비효율적이라는 생각을 하긴했습니다.
인터셉터보다는 서비스에서 validation 클래스의 메소드를 호출해 체크하는 것이 맞는 것 같아보이네요!!!
DTO 패키지는 다른 프로젝트의 소스 코드를 보면 거의 다 Controller 패키지 안에 있던데 그렇게 지정하는 이유가 있나요??
제 생각으로는 요청과 응답을 클라이언트로부터 주고 받는 계층이 Controller 이므로 그런 것인지 궁금합니다!