-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement Article CRUD and Comment CRUD #13
base: main
Are you sure you want to change the base?
Conversation
게시글 생성 기능 추가
Article Repository - findByTitle 메서드 추가 Test code - 모든 article 받아오기 (findAll) - article id 체크 (findById) - 제목으로 검색 (findByTitle)
findByTitleLikeOrContentLike() Title과 Content를 like로 검색
test code를 통해 article 데이터 수정 구현
test code를 통해 article 삭제 구현
commentService.create를 통해 댓글 저장 기능 추가
CommentService.getComment를 통해 댓글 내용 확인 기능 추가
id를 통해 comment를 받아서 바로 결과를 테스트합니다
나중에 로그인 인증 후 CRUD을 수행할 수 있게 추가하겠습니다
comment 저장 시 로그인된 유저를 저장
Article에 user 정보가 함께 저장됩니다
article의 controller와 service에서 실행되어야 할 기능 메서드 추가
로그인 권한을 확인하기 위해 Principal 객체 추가 Principal로 로그인 권한 확인 후 수정, 삭제 기능 수행
Article과 Comment에 Swagger 어노테이션 추가
Article controller에 UserService가 제대로 DI되지 않아 발생한 오류 해결
Article을 생성할 때 발생하는 sendError() 해결 - Entity에 @JsonBackReference 추가
article에 dto를 적용하여 DB에 저장
- DTO 생성 - @PathVariable에서 @parameter 제거 - 권한 ROLE_CLIENT 변경
댓글 생성과 읽기에 대한 기능 추가
댓글 수정, 삭제 기능 구현
@@ -5,9 +5,7 @@ | |||
|
|||
@SpringBootApplication | |||
public class BlogApplication { |
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.
이런 스타일 관련 변경점은 새로운 PR을 통해 작성하는게 좋아요!
다른 사람과 작업을 할 때, 이 부분을 예를 들자면, 어플리케이션 실행 전 로그를 남기는 등의 동작을 추가하게 된다면 Conflict가 나게 되겠죠?
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.
리뷰 한번 달아봤어요
//게시글 생성 | ||
Article article = Article.builder() | ||
.title(dto.getTitle()) | ||
.content(dto.getContent()) |
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.
저는 개인적으로 여기에서 Article을 생성하는 것보다... createArticle이 모든 책임을 가져가는 게 좋다고 봐요!
만약 Article 중 어떤 추가적인 내용이 제공되어야 한다고 했을 때, 지금 구조에서는 ArticleController와 ArticleService에서 Article 생성이라는 동일 책임을 분배하고 있기 때문에, 어느 부분에 로직이 추가되어야 하는지를 다시 고민하게 되는 비용이 발생하게 되겠죠.
그리고 createArticle 함수의 정확한 책임이 Article 생성보다는, 내용만 들어 있는 Article에 User를 적용하고 저장(듣기만 해도 복잡해 보이죠)하는 책임을 담당하고 있기 때문에, 이를 Article 객체를 생성하고 데이터를 입력
이라던지, 생성된 Article객체를 인자로 받아 DB에 저장
이라던지, 혹은 둘 다 진행 가능한 책임을 주던지, 결론적으로 이런 분배가 현재 구조에서는 그닥 유리해 보이지 않네요...
* @param dto 게시글 생성 정보 | ||
* @return 생성된 게시글 | ||
*/ | ||
@PostMapping("/create") //컨트롤러 메핑 |
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.
REST API를 기준으로 API를 작성하기로 했다면, create, delete, update 등의 동작을 표현하는 용어들은 HTTP Method를 통해 표현되어야 해요
GET
: 리스트 받아오기GET {id}
: 특정 데이터를 받아오기POST
: 생성하기PUT {id}
: 전체 수정/생성하기PATCH {id}
: 일부분 수정하기DELETE {id}
: 삭제하기
따라서 해당 API의 Path는 POST "/api/article"
이 적절하겠죠?
@PostMapping("/create") //컨트롤러 메핑 | |
@PostMapping //컨트롤러 메핑 |
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.
아래의 다른 RequestMapping 들도 마찬가지입니다!
@Operation(summary = "모든 게시글 조회") | ||
@PreAuthorize("hasRole('ROLE_ADMIN') or hasRole('ROLE_CLIENT')") | ||
public List<Article> getUserArticle( | ||
@Parameter(name = "HTTP 파싱 객체") HttpServletRequest req |
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.
이 HttpServletRequest
파라미터는 사용자가 입력하는 것이 아니라, Spring Boot 프레임워크에서 넘겨주는 것이기 떄문에, Swagger에서 표시되면 안 되는 친구에요!
따라서, @Parameter
어노테이션의 hidden
옵션을 달아 주는 게 좋을 거 같네요!
@Parameter(name = "HTTP 파싱 객체") HttpServletRequest req | |
@Parameter(hidden = true) HttpServletRequest req |
@PreAuthorize("hasRole('ROLE_ADMIN') or hasRole('ROLE_CLIENT')") | ||
@Operation(summary = "id로 게시글 조회") | ||
public Article getArticleById( | ||
@PathVariable("id") Long id, //PathVariable에는 @Parameter를 사용할 수 없음! |
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.
PathVariable에 사용 가능한 걸로 알고 있는데... value 대신에 description에 한번 설명을 넣어 보는 걸 추천드려요!
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 | ||
@RequiredArgsConstructor | ||
@Tag(name = "게시글 Service", description = "Article service 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.
Service는 사용자에게 노출되지 않는 레벨로, Swagger documentation이 필요하지 않은 레벨이에요! @Parameter
, @Operation
, @Tag
3개 모두 마찬가지입니다!
@Tag(name = "게시글 Service", description = "Article service 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.
위의 ArtileController에서 리뷰했던 부분들 중 상당수가 CommentController에서도 적용되기 때문에, 해당 리뷰 한번 쭉 읽어보신 후 CommentController에도 마찬가지로 적용시켜 주세요!
import org.springframework.data.jpa.repository.JpaRepository; | ||
|
||
public interface CommentRepository extends JpaRepository<Comment, Long>{ | ||
List<Comment> findByArticleIdx(Long articleIdx); |
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.
이 함수 작동하나요...? MethodQuery에서 생성 안 된다고 에러 걸릴 거 같은데...
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.
위의 ArticleService에서 리뷰했던 부분들 중 상당수가 CommentService에서도 적용되기 때문에, 해당 리뷰 한번 쭉 읽어보신 후 CommentService에도 마찬가지로 적용시켜 주세요!
// http.authorizeRequests() | ||
// .requestMatchers("/h2-console/**", "/swagger-ui/**", "/swagger-resources/**", | ||
// "/v3/api-docs/**", "/api/user/signin", "/api/user/signup") | ||
// .permitAll().anyRequest().authenticated(); |
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.
저 부분이 있으면 localhost:8080으로 접속했을 때 403 에러가 발생해서 임시로 주석처리 했습니다ㅠ
나중에 에러 해결하고 주석 해제할 예정입니다
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.
너무 테스트가 많이 들어가 있는 거 같은데...ArticleTest와 CommentTest로 분리되면 좋을 듯 합니다!
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.
테스트 코드에서 로그인을 유지하는 방법이 어려워서 처음에 구현한 코드 그대로 두었습니다!
(현 시점에서 테스트 코드 돌리면 에러 발생합니다..)
수정해서 다시 올리겠습니다
Article과 Comment의 Controller에 있는 mapping 경로를 HTTP Method에 맞게 수정
HttpServletRequest 파라미터를 Swagger에 노출되지 않게 hidden 옵션으로 수정
@PathVariable에 Swagger 파라미터 API인 @parameter(description) 추가
사용자에게 노출되지 않는 Service에 Swagger documention 제거
제목으로 게시글 검색 기능 추가 - url mapping 혼란을 막기 위해 /id/{id}, /title/{title}로 변경 - 검색 결과가 없을 경우를 대비하여 repository 메서드 리턴 타입을 Optional으로 수정
Optional 타입의 객체에서 if-else 대신 orElseThrow()로 사용
…ent object in Service 객체를 생성하는 로직을 전부 Service에서 처리합니다. Controller에서 수행하는 역할을 단순화 시켰습니다
- Base를 생성하고 Article과 Comment가 Base를 상속 - 중복으로 사용되는 속성을 제거 - @CreatedDate, @LastModifiedDate 추가 - Entity를 생성하거나 변경할 때 자동으로 시간 데이터를 추가
생성 및 수정 상황에서 에러가 발생하는 경우 여러 문제를 방지하기 위해 Transactional 어노테이션 추가
Transactional 어노테이션을 사용하기 때문에 변경사항을 자동으로 적용해줍니다
게시글과 댓글에 대한 CRUD 구현 완료했습니다.