🚀 "수정 전" 하향식 코드리뷰
@GetMapping("/write")
public PostWriteDTO write(@RequestParam Long id) {
return postService.getPostById(id);
}'/write' get방식으로 호출하면 위 화면이 나온다.
위 처럼 쿼리스트링에 아무것도 주지 않으면 새 글 작성을 위한 화면이,
id값을 주면 기존 글에 대한 수정 화면이 나온다.(/write?id=1)
@RequestParam에 의해 'id'와 같은 이름의
쿼리스트링의 파라미터의 값을(?id=1) 받아올 수 있다.수정의 경우 기존 저장된 글의 정보들을 위 화면에 뿌려주어야 하기 때문에
리턴타입으로 PostWriteDTO를 만들었다.@Data
public class PostWriteDTO {
private Long postId;
private String title;
private String introduce;
private String content;
private String access;
private Long seriesId;
private List<String> tags;
private ThumbnailResponseDTO thumbnail;
private TemporaryPostReadDTO temporary;
//생략
}id는 post방식으로 넘어갈 때 쓰이기 위해 필요하다.
(어떤 글을 수정할 것인지 지정해 주어야 하기 때문)
title, content, introduce, access(공개설정), tags는 화면에 내용 그대로 넣어주면 되겠다.
seriesId를 준 이유는 post방식으로 넘어갈 때 seriesId를 요구하기 때문이다.
thumbnail에는 내부에 fileName이라는 필드가 있다.
이 이름으로 /display를 호출하여 이미지를 불러올 수 있다.
temporary는 글 수정시 임시저장했던 이력이 있다면(null이 아니라면)
화면에 '불러오시겠습니까' 같은 화면을 위해 만들었다.(아래 참고)
임시저장된 제목과 내용만 있다.
return postService.getPostById(id);위의 DTO를 보내기 위하여 컨트롤러에서 서비스의 메서드를 호출한다.
postService는 final키워드와
컨트롤러의 @RequiredArgsConstructor에 의해
단일 생성자 묵시적 자동주입으로 의존성이 처리된다. public PostWriteDTO getPostById(Long id) {
Post post = postRepository.findById(id).orElse(new Post());
List<PostTag> postTags = postTagRepository.findByPost(post);
return new PostWriteDTO(post, postTags);
}Post post는 엔티티(VO)이다.
PostRepository는 리포지토리(DAO)이다.
리포지토리 역시 의존성이 위와 같은 방식으로 처리된다.
PostRepository는 JpaRepository를 상속 받기 때문에
기본적으로 제공하는 findById를 사용하였다.
(select * from post where id = ?)
postTagRepository.findByPost(post)에서 위 포스트에 달린 태그목록을 받아온다.
PostTagRepository역시 JpaRepository를 상속 받기 때문에
(기본적으로 제공하는..?이라고 하기 좀 그런) 쿼리메서드를 사용하였다.
(select * from post_tag where post_id = ?)
위에서 구한 두개의 엔티티를 받아 dto로 변환하는 작업을 생성자로 처리하였다.@Data
public class PostWriteDTO {
//생략
public PostWriteDTO(Post post, List<PostTag> postTags) {
this.postId = post.getId();
this.title = post.getTitle();
this.introduce = post.getIntroduce();
this.content = post.getContent();
if (post.getAccess() != null) {
this.access = post.getAccess().name();
}
if (post.getSeries() != null) {
this.seriesId = post.getSeries().getId();
}
this.tags = postTags.stream()
.map(postTag -> postTag.getTag().getName())
.collect(Collectors.toList());
this.thumbnail = makeThumbnail(post.getPostThumbnail());
if (post.getTemporaryPost() != null) {
this.temporary = new TemporaryPostReadDTO(post.getTemporaryPost());
}
}
private ThumbnailResponseDTO makeThumbnail(PostThumbnail postThumbnail) {
ThumbnailResponseDTO result = null;
if (postThumbnail != null) {
result = new ThumbnailResponseDTO(
new PostThumbnailDTO(postThumbnail)
.getSFileNameWithPath());
}
return result;
}
}생성자 블록 내부에 반 정도는 this.field = value; 식으로 되어있다.
글의 access 전체공개/비공개 둘중 하나이지만
임시저장글의 경우 글과 내용만 저장하기 때문에 비어있을 수 있기 때문에
null처리를 통하여 넣어주었다.
series는 글의 카테고리 같은 개념이다.
무소속일 수 있기 때문에 null처리를 해주었다.
PostTag[엔티티](post_tag[테이블])는 Post(post) Tag(tag)의
다대다 설계를 위한 교차테이블이다.
서비스 단의 메서드를 통해 postTags는
해당 포스트에 대한 태그달린 이력들을 가지고 있다.
화면에서는 태그의 이름만 필요하기 때문에 map으로 이름만 가져와 List로 저장하였다.
thumbnail과 temporary역시 널처리를 한다.🚀 기능상 문제될 만한 상황 찾기
@GetMapping("/write")
public PostWriteDTO write(@RequestParam Long id) {
return postService.getPostById(id);
}1. id를 안줬을 때 :
원래는 id가 없으면 새 글 작성인 것으로 하려고 했었는데
@RequestParam어노테이션으로 인해 error가 난다.(not present)
id가 없는 것도 유지해야하기 때문에 수정이 필요하다.
수정을 하고 나면 findById메서드에는 null이 들어갈 수 없기 때문에
illegalStateException(the given id must not be null)이 발생한다.
널처리가 필요하다.
2. 없는 글의 id를 주었을 때 :
모티브인 velog에서는 어떻게 하고 있는지 테스트 해보았다.
잘못된 id를 주었을 때 비어있는 화면이 뜬다.
어찌됐던 막는 것을 볼 수 있다.
현재 우리 코드대로면, 새 글 작성화면 처럼 빈 화면이 나온다.
이 id를 유지한 채로 post방식으로 가면 문제가 있을 수 있다고 생각했다.
유효하지 않은 id로 새 글 작성화면을 허용하게 되면,
그 사이에 다른 사용자가 작성한 글이 하필 그 id일 때(원랜 없었던)
기존 글의 수정이...!!
되진 않을 거다.
(다른 계정으로 로그인 되어있으면 막을 거니까, 현재 기준으로는 뚫리지만..)
새 글 작성은 id 없이 하는 게 원칙인데,
유효하지 않은 id로 새 글 작성하려는 것은
어차피 정상적인 사용자가 아닐 테니까;;
그 사람이 글을 못쓰게 하는게 전혀 나쁠 것 없다고 생각했다.
이래도 되고 저래도 되는 상황에서는 velog를 따라가기로 팀원들끼리 정했었다.
수정해야할 것 같다.
3. 해당 글을 작성한 회원이 아닌데 수정화면을 요구할 때 :
velog에서 내 글 수정화면 링크를 복사하여
(https://velog.io/write?id=cf58d823-5da9-432c-b4ff-f2e3879980ca)
로그아웃(또는 다른 계정으로 로그인)상태에서 불러와 보았다.
불러는 와지는데 submit(POST)을 했을 때 처리를 하는 것을 확인하였다.
그래서 그냥 그렇게 하려고 하다가,
그럼 비공개 게시글은 어떻게 되는거지? 하는 생각이들어 또 실험해보았다.
velog도 그냥 뚫린다.
어차피 작성할 때 막으면 되니 데이터가 바뀌는 문제는 없을 수 있지만,
비공개글이 보인다는 게 문제가 아닌가 하는 생각이 들었다.
4. 로그인 되지 않았을 때 :
velog에서는 위와 마찬가지로 불러오는 데에는 이상이 없으며
처리를 할 때 '이러시면 안됩니다' 한다.
public PostWriteDTO getPostById(Long id) {
Post post = postRepository.findById(id).orElse(new Post());
List<PostTag> postTags = postTagRepository.findByPost(post);
return new PostWriteDTO(post, postTags);
}1. id에 null이 들어올 때 :
findById에는 null이 들어오면 익셉션이 발생하기 때문에
널처리를 포함하게 수정하여야 한다.
2. 유효하지 않은 id가 들어올 때 :
velog와 동일하게 위 상황일 때 막으려면
orElse를 수정하여야 한다.
3. postTags를 위한 파라미터 post의 경우의 수는 다음과 같다 :
- id가 널인 경우
새 글 작성시에 컨트롤러 리턴값에 빈 dto라도 줬었는데,
그냥 null을 주는 게 리소스 낭비도 하지 않고 좋을 것 같다.
null일 경우 처음부터 dto를 생성하지 않게 수정하여야 겠다.
- 유효한 post -> 태그가 달려 있지 않은 경우만 신경쓰면 될 것 같다.
post가 태그가 달려있지 않을 경우에 findByPost의 리턴값이
빈 리스트이므로 dto변환 시 널포인터익셉션의 문제는 없다.
- new Post()가 들어오는 경우
new Post()가 들어오는 경우 저장되지 않은 엔티티가 들어와
익셉션이 발생한다.
velog상으로 유효하지 않은 id입력 시 애초부터 막으니,
orElseThrow()로 수정하여 유효하지 않으면 익셉션을 발생시키도록
수정하여야겠다.@Data
public class PostWriteDTO {
//...
public PostWriteDTO(Post post, List<PostTag> postTags) {
this.postId = post.getId();
this.title = post.getTitle();
this.introduce = post.getIntroduce();
this.content = post.getContent();
if (post.getAccess() != null) {
this.access = post.getAccess().name();
}
if (post.getSeries() != null) {
this.seriesId = post.getSeries().getId();
}
this.tags = postTags.stream()
.map(postTag -> postTag.getTag().getName())
.collect(Collectors.toList());
this.thumbnail = makeThumbnail(post.getPostThumbnail());
if (post.getTemporaryPost() != null) {
this.temporary = new TemporaryPostReadDTO(post.getTemporaryPost());
}
}
//...
}매개변수로 들어오는 post는 유효한 post이다.
(서비스단에서 id가 null이거나, 유효하지 않을 경우 여기로 오지 않기 때문)
유효한 post는 id, title, content가 무조건 있다.
introduce는 임시저장글의 경우 null일 수 있으나, null을 넣는 것은 문제가 되지 않는다.
(introduce.어떤method()할 일이 없다.)
매개변수 postTags는 null이 아니다.(최소 빈 리스트)
나머지 nullable인 것들은 null처리가 들어있다.seriesId를 주면 위 화면에서
속해있는 시리즈의 '이름'을 적어주기 위하여
1. id로 이름을 반환하는 url을 또 만들어야하거나,
2. 기존에 만들어 둔 /getSeriesList를 호출하여 거기서 찾아와야한다.
1번 방법은 요청이 추가되는 것이니 하지 않도록 하고
2번 방법은 이미 있는 요청이기도 해서 괜찮을까 싶었지만
시리즈 설정을 지금 적혀있는 대로 놔두고 싶으면
굳이 /getSeriesList를 호출하여 찾을 필요가 없다.
(바꾸고 싶을 때만 클릭해서 오른쪽에 화면을 띄우면 되기 때문)
이를 위해서 dto에 id만이 아니라 id와 name을 같이 주게 수정하여야 겠다.🚀 기능에 문제 없음을 우선적으로 코드 수정
@GetMapping("/write")
public PostWriteDTO write(@RequestParam(required = false) Long id) {
return id == null ? null : postService.getPostById(id);
}id가 null일 때 굳이 서비스를 들어갈 필요가 없어서
바로 null을 리턴하게 수정하였다. public PostWriteDTO getPostById(Long id) {
Post post = postRepository.findById(id).orElseThrow(
() -> new IllegalStateException("존재하지 않는 포스트입니다."));
List<PostTag> postTags = postTagRepository.findByPost(post);
return new PostWriteDTO(post, postTags);
}null인 경우는 존재하지도 않으며,
유효하지 않은 값이 들어왔을 경우 익셉션을 던지며 나와버린다.@Data
public class PostWriteDTO {
//..
//private Long seriesId;
private SeriesReadDTO series;
//...
public PostWriteDTO(Post post, List<PostTag> postTags) {
//...
if (post.getSeries() != null) {
//this.seriesId = post.getSeries().getId();
this.series = new SeriesReadDTO(post.getSeries());
}
//...
}
}기존의 id와 name만 갖고 있는 SeriesReadDTO를 재사용하였다.
위 두 속성을 함께 주도록 수정하였다.🚀 테스트 작성
//writeGET
@Test
void id에_null이_들어왔을_경우_새글작성이므로_null을_반환한다() throws Exception {
mockMvc.perform(get("/write"))
.andExpect(status().isOk())
.andExpect(content().string(""));
}
@Test
void 없는_게시글의_id가_들어왔을_경우_예외를_던진다() {
//가장 큰 id
Query query = em.createNativeQuery("select MAX(post_id) from post");
Long postId = ((BigInteger) query.getSingleResult()).longValue();
assertThatThrownBy(() ->
mockMvc.perform(get("/write")
.param("id", String.valueOf(postId + 1)))//가장 큰 id보다 1큰 id는 없는 게시물이다.
.andExpect(status().isOk())).isInstanceOf(Exception.class);
}//endwriteGet관련 코드를 수정을 하더라도 위 테스트만 통과한다면
리팩토링하더라도 괜찮다고 생각할 수 있겠다.
어제보다 나은 사람이 되자