[우아한테크코스] 레벨 2 후기 - 코드 리뷰 정리해보기
우아한테크코스 레벨2를 진행하면서 개인 블로그 미션, 팀 프로젝트 미션에 대한 리뷰 복습(?) 겸 정리를 해 보았다.
학습량이 꽤나 많아지면서 놓치거나 소화 하지 못한 부분도 많았다. 모두 다 소화하면 좋겠지만 그것보다는 최대한 놓치는 부분들을 최소화 해보려고 한다. 리뷰 받은 것들이나 공부 했던 것들을 다시 열어 보니까 완전히 잊고 있거나 새로운 것들이 보였다.
ID의 wrapper type 과 primitive type
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id; // long id ?id를 원시형과 래퍼타입 중 어느것을 써야할까?
원시형을 쓰게 되면 null이라는 것을 적절히 표현할 수 없다. 원시형에서 엔티티를 찾을 수 없다면 0을 써야하는게 최선 같지만 그럴 수 없다. 0이라는 값도 충분히 비지니스 로직에서 필요한 값이기 때문이다. 또한 0이라는 값으로 실수로 초기화 할 수 있는 문제도 존재한다. 그래서 래퍼타입을 쓰는 것이 좋다. 실제 Hibernate 문서에도 다음과 같이 권고하고 있다.
We recommend that you declare consistently-named identifier attributes on persistent classes and that you use a nullable (i.e., non-primitive) type.
Impl 네이밍의 부적절성
https://octoperf.com/blog/2016/10/27/impl-classes-are-evil/
해당 글은 Impl 네이밍의 부적절성에 대해 언급한다. 이 네이밍을 아주 신랄하게 비판하고 있다.
클래스 네이밍을 할 때 suffix에 impl 을 붙이면 유의어 반복이라고 한다. 단지 잡음일 뿐이다. 글에서는 Impl이라는 suffix를 붙이면 나는 네이밍을 할 줄 몰라요 라고 말하는 것이라고 한다. 그리고 자바의 List를 설명한다. List를 implements하고 있는 ArrayList나 LinkedList는 봤어도, ListImpl 이라는 클래스를 본적이 있냐고 되묻는다. 전혀 없다.
인터페이스 하나 당 구현체 하나를 1:1로 매핑하고 Impl을 붙이는 이상한 습관을 누군가가 가르쳐줘서 다들 그렇게 하는 줄 알았는데 절대로 아니었다. 인터페이스를 아주 잘못 사용하고 있는 예이며 고쳐야 할 관습이다.
JPA Auditing 적용
JPA Auditing을 사용하면 엔티티가 생성 혹은 수정될 때 자동으로 반영되도록 구현할 수 있다.
Web설정파일에
@EnableJpaAuditing을 선언해주고 엔티티에 대한 베이스를 정의해 보았다.
@MappedSuperclass
@EntityListeners(value = AuditingEntityListener.class)
public abstract class ContentsAudit {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
protected Long id;
@CreatedDate
private LocalDateTime createdDate;
@LastModifiedDate
private LocalDateTime lastModifiedDate;
// 중략...다음과 같이 엔티티가 생성된 시간과 수정된 시간을 자동으로 넣을 수 있다.
그리고 @CreatedBy와 @LastModifiedBy로 엔티티의 수정자 까지 넣을 수 있다. 이것을 넣으려면 현재 세션의 유저정보를 가져와서 넣어준다.
@Configuration
@EnableJpaAuditing(auditorAwareRef = "auditorProvider")
public class JpaAuditingConfiguration {
@Bean
public AuditorAware<String> auditorProvider() {
/*
스프링 시큐리티를 쓴다면 다음과 같이 가져온다.
SecurityContextHolder.getContext().getAuthentication().getName()
*/
return () -> "name"; // 임시 값
}
}Spring Security를 쓴다면 다음과 같이 현재 context에 있는 유저를 가져와서 넣어준다면 자동으로 auditing이 동작하게 된다. 미션 도중에는 쓰지 않았으므로 넘어갔다. Security 없이 이것을 구현 하려면 thread-safe한 map에 세션 정보를 담고 있다가 현재 session에서 user정보를 key로 해서 map에서 꺼내오면 되겠다. 미션 도중 진행해 보려고 했지만 시간 관계상 생략했다.
@Lob의 사용
데이터 크기를 결정하기 힘든 큰 크기의 데이터라면 @Lob을 쓰는 것도 좋다.
@Lob private String contents;DTO 매핑
DTO 매핑하는 것을 별도의 클래스의 위임할 수 있는 방법이 있다. Assembler를 따로 만들 수 있고 ModelMapper를 활용할 수 있다. 하지만 mapper를 쓸 때 코드 복잡도가 증가할 수 있고 성능 이슈도 있을 수 있다. 공부 차원에서 적용만 해보기로 했다.
1:1 대응(?)이 아닌 필드들은, 즉 가공이 필요한 필드들은 Converter를 사용해 가공할 수 있다. 나머지 필드들은 그냥 바로 대응된다.
public class CommentResponseMapper extends PropertyMap<Comment, CommentResponse> {
private Converter<LocalDateTime, Long> converter =
context -> context.getSource().until(LocalDateTime.now(), ChronoUnit.MILLIS);
@Override
protected void configure() {
using(converter).map(source.getCreatedDate()).setElapsedTime(null);
}
}private List<CommentResponse> findByArticle(Article article) {
return commentRepository.findByArticle(article).stream()
.map(comment -> commentMapper.map(comment, CommentResponse.class))
.collect(Collectors.toList());
}참고로 무조건 DTO가 꼭 필요하다고 얘기할 순 없다. Presentation Layer의 변경사항이 도메인 모델에 영향을 미치는 것을 줄이고자 작성한다는 관점에서 생각하라는 조언을 받았다.
테스트시 SpringBootTest를 무조건적으로 붙이는 행위
단순한 클래스를 테스트할 시 에는 spring context를 올릴 필요가 없기 때문에 테스트 환경도 신경을 써보자.
Rest api에 대한 네이밍
Rest에서는 복수형을 사용한다. 클린한 api를 설계하는데 도움이 되는 습관을 보자. Rest의 개념은 논리적으로 API 구조를 분리하기 위한 것이다.
https://blog.mwaysolutions.com/2014/06/05/10-best-practices-for-better-restful-api/
다음 글에서 중요한 내용들을 정리해 보았다. Restful 하다고 하면 hateoas까지 지켜야 만족하는 조건이지만, 실제로는 Rest의 Level을 몇 까지 지킬지 논의하고 생산성을 고려하여 사용한다고 한다.
동사가 아닌 명사를 사용하라
| Resource | GET | POST | PUT | DELETE |
| /cars | cars배열을 반환한다 | 새로운 car를 생성한다 | cars의 bulk 업데이트 | 모든 car 삭제 |
| /cars/711 | 특정 car 반환 | method not allowed(405) | 특정 car 업데이트 | 특정 car 삭제 |
다음과 같은 동사는 사용하지 않는다.
/getAllCars
/createNewCar
GET 메서드와 쿼리 파라미터는 상태를 바꾸면 안된다
상태를 바꾸려면 PUT, POST, DELETE를 사용하라
복수형 명사를 사용하라
/car 보다는 /cars 를 사용하라. 모든 자원에 복수형 명사를 사용하라.
관계표현을 위해 sub-resources 를 사용하라
GET /cars/711/drivers/ → car 711에 대한 모든 drivers 반환
GET /cars/711/drivers/4 → car 711에 대한 driver #4 반환
API 버젼을 명시하라
/blog/api/v1 가 같이 사용하라. 2.5와 같이 점을 찍는 것을 피하고 버전이 없는 api는 릴리즈하지 마라.
List의 Read-only
외부에서 값 변조를 막기 위해 List를 리턴할 시 다음과 같이 불변으로 리턴할 수 있다.
return Collections.unmodifiableList(list);각 Layer의 값 검증에 대한 고찰
DTO에서 어디 까지 검증을 할지 고민이 매우 많았다. 초기에는 제일 윗단 presentation layer에서 이메일이 중복 되는지 애노테이션을 커스텀하여 검사를 했었다. 결과적으로 이 방법은 DTO와 Repository의 연관관계가 형성이 되는 것이기 때문에 레이어를 파괴하는 행위 였다.
그래서 Presentatinon, Service, Domain Layer에서 각각의 유효성 검증은 다르다는 조언을 받았다.
내린 결론은 DTO에서는 형식을 테스트를 한다는 것이다. 정리하자면, presentation layer에서는 이메일, 이름 등 형식 검증 - 비어있는지, 글자 수가 초과하는 지 등을 검사를 하고 애플리케이션 내부에서는 예를들어, 중복된 이메일인지 등 데이터베이스를 뒤져봐야 알 수 있는 검증 등을 실행한다.
Service 간의 의존성
서비스가 다른 서비스를 가져다 쓰는 구조는 결론적으론 문제가 없지만 순환 참조 문제가 생길 수 있다. 이것은 서비스에 계층을 둬서 해결하면 된다는 조언을 받았다. 응용 서비스 레이어, 도메인 서비스 레이어를 나누어 해결하면 된다고 하는데 아직 이 부분을 이해하지 못했다. 마지막 레벨 까지 가야 이 궁금증은 해결 할 수 있을 듯 하다.
정답은 없다
나의 너무 정신없는 질문 공세에도 친절하게 답변 해주신 현구님. 많은 것들을 배웠고 배워야 할 것들을 주셨다. 정답이 있는 설계가 아닌 필요에 맞는 설계가 중요하다는 것을 깨달았다.
의존규칙
엔터프라이즈 규모의 애플리케이션이라면 내가 정리한 것처럼 의존 규칙을 갖는 것이 좋다. 하지만 작은 프로그램이라면 컨트롤러에서 도메인이 제공하는 기능을 쓸 수 있다고 생각한다고 하셨다. 단, 이 경우에도 도메인 기능을 실행하는 것이지 도메인 구현코드를 작성하는 것은 아니다.
서비스 레이어에 대한 고찰
서비스를 나눌 거면 의존적인 컨트롤러와 의존적인 부분들을 철저히 분리해야 한다. 내가 작성했던 코드에서 컨트롤러에서 서비스로 Session 정보가 UserSession 정보를 넘기는 부분이 있었다. 이렇게되면 서비스가 http에 의존적으로 될 수 있다고 조언을 받았다. DTO로 감싸서 전달하거나 도메인 객체로 변환해서 서비스레이어로 전달하는 것이 낫다는 의견을 주셨다.
양방향 관계
양방향 관계를 되도록 피하는 것이 좋다고 하셨다. 김영한님의 jpa책을 추천해주시면서 양방향 관계를 단뱡향 관계로 바꾸는 방법을 찾아보라고 말씀해주셨다. 아직 책을 다 읽지 못해서 이 부분도 학습 목록에 올려 두어야겠다.
gradle build시 test를 실행
gradle 설정파일에
test {
useJUnitPlatform()
}가 빠져있어서 build시 test가 수행되지 않았다.
db update
'좋아요'라는 버튼을 눌렀을 때 row가 계속 추가되거나 삭제 되면 부담이 갈 수 있기 때문에 상태값을 추가해서 y/n 혹은 on/off 방식으로 구현하는 방법도 있다.
팀 프로젝트
팀 프로젝트도 재밌게 진행했다. 프로젝트 하면서 힘든 부분도 많았지만 다 구현에 관련한 것들 이었다. 팀원 간의 갈등은 없었다. 물론 갈등이 없다 는게 좋은 것 만은 아니다. 나는 팀이 성장하려면 좋은 의미의 갈등이 있어야 한다고 생각한다. 하지만 우리 팀은 시작 전에 많은 규칙들을 정했고 설계에 관해서도 오랜 시간 회의를 했다. 그랬더니 1주차의 코딩량이 많지가 않아서 스타트는 다른 팀들보다 조금 느렸다. 하지만 중간 중간 갈등 요소를 미리 없앴기 때문에 2주차 부터 속도가 많이 났고 다른 팀들이 겪는 갈등들을 예방할 수 있었다. 협업의 묘미도 느껴 보았고 개인적으로 반성의 기회도 가질 수 있었던 좋은 경험이었다.