웨이팅 뒷고기

[GIVEN53]

.

[GIVEN53]

reviewer:

현업에서는 auto 와 uuid(tsid) 를 주로 쓰고 있는 것 같아요.

어떤 서비스의 성격이냐에 따라서도 달라질 것 같은데요, 주로 클라이언트 와 식별자로 통신할 때 id 가 노출되는데
회원의 id 가 10000 이다 라고 노출이 되면 이 서비스의 회원이 최소 만명이네? 라고 유추할 수 있기 때문에 보안에 좋지 않아서
요새는 tsid 라는 것을 적용해서 사용하고 있어요. 이렇게 어플리케이션 레벨에서 id 를 주입해주면 null 이 없는 도메인을 핸들링 할 수 있기 때문에 비지니스 안정성도 올라가서 좋은 것 같아요,

그러나 그 외에 성격을 가진 서비스는 auto 가 주는 편리함이 압도적이라서 auto 도 자주 사용합니다!

link: woowacourse/spring-roomescape-waiting#7 (comment)

---

reviewer:
테스트에서는 각 객체마다 @Autowired 를 달아주어야 객체 주입이 되는 이유가 무엇일까요? ㅎㅎ
Junit Test Container 와 Spring Container 두개의 키워드로 공부해보시고 알려주시면 좋을 것 같아요!

reviewee:
테스트에 경우 Junit에 의해 객체가 주입하는데 @XxxTest어노테이션 내부에는 @ExtendWith(SpringExtension.class)라는 어노테이션이 등록되어 있습니다. SpringExtension은 ParameterResolver 를 구현한 구현체이고, supportParameter 메서드에서는 스프링 컨테이너에서 가져올 수 있는 빈을 @Autowired 어노테이션이 통해 판단하게 됩니다.
그래서 Junit 에서 @Autowired를 사용하지 않으면 SpringExtension은 해당 타입의 객체를 제공할 수 없다고 응답하게 됩니다. 이러한 이유 때문에 각 객체마다 @Autowired를 달아주어야 객체 주입이 됩니다.

link: woowacourse/spring-roomescape-waiting#7 (comment)

---

reviewee:

비밀번호 암호화를 Bcrypt 로 구현하신 이유가 궁금해요!

비밀번호는 원본 데이터를 다시 되돌릴 필요가 없기 때문에 단방향 암호화를 고려했습니다. 하지만 단방향 암호화의 단점으로는 동일한 digest를 갖는다는 것과 브루트 포스 공격의 위험이 있습니다.
이를 해결하기 위해 key stretching 방법과 salt를 더하는 방법으로 보완할 수 있으며, Bcrypt가 해당 장점들(단방향, key stretching, salt)를 가지고 있어 결정하게 되었습니다.

암호화/ 복호화되는 과정을 설명하실 수 있나요?

Bcrypt 암호화 과정은 랜덤한 salt 값을 생성하여 입력된 비밀번호와 salt 값을 결합합니다. 결합된 비밀번호와 salt 값을 반복적으로 해시 계산합니다. 최종 해시 값은 비밀번호와 salt 값의 반복된 해시 계산 결과로 digest로 암호화됩니다.

Bcrypt 검증 과정 사용자 입력 비밀번호를 해시화하고 저장된 해시 값과 비교하는 방식으로 검증합니다.

Bcrypt는 저장된 해시 값에서 salt 값을 추출하고, 입력된 비밀번호와 결합하여 동일한 해시 과정을 거칩니다. 입력된 비밀번호를 해시화한 결과와 저장된 해시 값을 비교합니다. 두 해시 값이 동일하면 비밀번호가 일치하는 것으로 간주하고, 로그인을 허용합니다. 그렇지 않으면 비밀번호가 틀린 것으로 처리합니다.

link: woowacourse/spring-roomescape-waiting#27 (comment)

---

reviewer:

좋은 단위테스트의 4대 요소

• 회귀 방지
• 리팩터링 내성
• 빠른 피드백
• 유지 보수성

예외 메세지를 이렇게 문자열로 관리하게 되면 리팩터링 내성이 떨어집니다.
비지니스 로직에서 "날짜 형식이 잘못되었습니다." -> "날짜"의" 형식이 잘못되었습니다." 로 변경했을 때
이 테스트는 2222 로 날짜 형식이 잘못되었기 때문에 예외가 발생하고 테스트가 성공해야하지만,
실제로는 메세지가 달라서 실패하게되는거죠. 이것을 거짓 양성이라고 표현하는데요~

리팩터링 내성

기존에 있는 테스트가 실패하지 않고, 어플리케이션 코드를 변경 할 수 있는지 여부이다.

• 리팩터링 이후
  • 어플리케이션 코드는 정상동작하는데, 테스트가 실패하는 경우 → 거짓 양성
  • 어플리케이션 코드는 실패하는데, 테스트가 성공하는 경우 → 거짓 음성
• 테스트가 실패하면 얻는 이점
  • 기존 기능이 고장 났을 때 조기 경고
  • 코드 변경이 회귀로 이어지지 않는 다는 확신을 제공
• 그런데 거짓 양성과 음성이 뜬다면?
  • 테스트에 신뢰성 하락
  • 허위 경보에 대한 인식이 생겨 더 큰 장애를 유발할 수 있음

따라서 메세지를 따로 관리해주거나 해당 클래스 내부에서 상수로 추출하고 protected 접근제한자를 사용하여
관련 있는 클래스에서만(테스트 포함) 접근 가능하도록 하는 것도 좋은 방법이 될 것 같습니다.

link: woowacourse/spring-roomescape-waiting#94 (comment)

---

reviewer:
신경쓰이는 점은 어플리케이션 구동 시점에 데이터 세팅을 위한 repository 과의 의존성인데요~
저는 의존성이 계층별 단방향으로 이뤄져야 관리하기 수월하다고 생각합니다.

Spring Container( 관련 기본 세팅) -> 프레젠테이션 영역(controller) -> Applicaiton(Service) 영역 -> Respository -> DB
단방향으로 흐르는 상태에서 해당 세팅으로 인해서

Spring Container( 관련 기본 세팅) -> Respository -> DB -> 프레젠테이션 영역(controller) -> Applicaiton(Service) 영역 -> Respository -> DB

이런 식으로 중첩되어서 의존성이 발생하는 것들이 관리하기 어려워 지는 것 같아요. 그런데 컴파일 시점에 오류를 잡을 수 있는 것은 아주 큰 장점이라고 생각해요. 변경해달라기 보다는 이런 관점도 있다고 알아주시면 좋을 것 같습니다~!

추가로 SET foreign_key_checks = 0; 와 같이 외래키 제약 조건을 무시하는 조건을 사용할 수 있어요.

link: woowacourse/spring-roomescape-waiting#100 (comment)

---

reviewer:
Service 에서 Service 를 의존하는 것에 대해서 어떻게 생각하세요? 🤔

저는 Service 를 여러개 재조합 해야할 일이 있다면 ApplicationService 를 만들어서 조합하는 편이에요

이유는 Service 에서 또다른 Service 를 의존할 수 있다면 의도치 않은 순환 참조가 발생할 수 있어요.
그래서 계층별로 같은 역할을 담당하는 것 끼리는 독립적으로 존재하게 하고 그들을 조합할 일이 있으면 상위계층에서 진행하곤 합니다!

link: woowacourse/spring-roomescape-waiting#100 (comment)

[Kimprodp]

기본값은 EnumType.ORDINAL인데 STRING으로 변경한 이유가 있을까요?

@Enumerated의 기본값인 EnumType.ORDINAL의 문제점은 enum 순서의 인덱스로 db에 저장하기 때문에 만약 Status enum의 순서를 변경한다거나, 중간에 어떤 값이 추가되게 된다면 사이드 이펙트가 발생할 수 있습니다.
그래서 순서에 의존하지 않고 이름에 의존하는 EnumType.STRING을 사용하였습니다!

물론 EnumType.STRING도 enum의 이름(name()) 이 변경되게 된다면 사이드 이펙트가 발생할 수 있다만, 이름을 변경했을 때 문제가 발생하는것이 순서를 변경했을 때 문제가 발생하는 것보다 자연스럽게 느껴져 EnumType.STRING으로 변경해주었습니다!

woowacourse/spring-roomescape-waiting#26 (comment)

---

클래스명은 대문자인데, 소문자인 테이블과 매핑이 되네요.
다음 미션에서 확인해보면 어떨까요?
@table을 선언하면 어떤 이점이 있을까요?

말씀해 주신 대로 클래스명을 CamelCase로 선언했음에도 테이블명은 snake_case로 지정되는 것을 보고 조금 더 알아본 결과, 별도의 네이밍 전략이 존재한다는 것을 확인했습니다.

테이블명을 별도로 명시하지 않았을 경우에 JPA 기본은 ImplicitNamingStrategyJpaCompliantImpl 구현체를 활용하여 테이블명을 생성하고,
Spring 환경에서는 이를 상속받은 SpringImplicitNamingStrategy 를 사용하게 되는데, 이곳에서 snake_case로의 변환이 일어납니다.

public class SpringImplicitNamingStrategy extends ImplicitNamingStrategyJpaCompliantImpl {
    // ...
    public Identifier determineJoinTableName(ImplicitJoinTableNameSource source) {
        String name = source.getOwningPhysicalTableName() + "_" + source.getAssociationOwningAttributePath().getProperty();
        return this.toIdentifier(name, source.getBuildingContext());
    }
}

어노테이션을 생략했을 때 엔티티 클래스명과 테이블 명이 완전히 일치하지 않는(Camel vs snake)다면, 이렇게 프레임워크 안에서 '알아서' 처리하도록 두는 것 보다는 명시적으로 지정해 주는 것이 가독성 측면에서 유리할 것 같아 다음 단계에서는 @table 을 모두 명시하도록 변경해 두었습니다!

woowacourse/spring-roomescape-waiting#78 (comment)

---

다른 곳에서 MemberName, Email, Password를 사용한다면, 지속적으로 AttributeOverride를 사용해야겠네요.
이런 상황을 해결할 방법은 없을까요?

MemberName, Email, Password 와 같이 @Embaddable 로 사용할 VO 객체의 필드명에 @column 을 명시하는 방법이 있을 것 같은데, 말씀하신 방향이 이 방법이 맞을까요?

만약 맞다면, 각각 장단점이 존재할 것 같아요.

@entity 클래스에서 @AttributeOverride 로 @column 명시
장점: 엔티티 클래스만 보면 어떤 값들이 어떤 칼럼으로 저장되는지 쉽게 파악 가능
단점: @embeddable 객체를 다른 곳에서도 사용하려면 @AttributeOverride가 중복으로 작성됨
@embeddable 클래스에서 필드에 @column 명시
위와 반대
Email과 같이 depth가 하나인 경우에는 두 방법이 큰 차이가 없지만, Reservation -> Schedule -> ReservationDate 와 같이 깊이가 깊어질 경우 어떤 필드가 어떤 칼럼으로 저장되는지를 파악하려면 @embeddable 객체 내로 들어간 후 @Embedded 가 붙은 필드를 모두 확인하여 내부의 @column을 하나씩 확인해야 하므로 읽기 어려울 것 같았어요. 아직까지는 중복 @AttributeOverride가 발생하지 않았기도 했구요🤔

그래서 모두 @entity 클래스에서 @AttributeOverride 로 @column을 명시하는 방법으로 통일해 두었습니다.

JPA가 익숙치 않아 컨벤션을 아직 잡지 못했는데, 미르는 어떤 방법을 선호하시나요?

저는 후자입니다. JPA는 ORM입니다. Object와 Relation을 매핑해주죠.
Object에 집중할 수 있도록 하는 편입니다 :)

woowacourse/spring-roomescape-waiting#78 (comment)

---

@query와 Query Method는 어떤 차이가 있을까요?

Query Method를 사용하면 특정한 컨벤션을 가지는 레포지토리 메서드들에 대해 메서드 이름을 기반으로 Spring Data JPA에서 JPQL 쿼리를 자동으로 생성해 줍니다.

반면 @query는 Query Method에서 기본적으로 지원하지 않는 상세한 쿼리들을 직접 어노테이션 안에 명시하여, 메서드 호출 시 해당 쿼리를 수행하도록 할 수 있는 기능입니다.

woowacourse/spring-roomescape-waiting#78 (comment)

---

@column을 생략하면 어떤 값이 기본으로 매핑이 될까요?

@column 를 명시하지 않으면 @column의 기본 옵션들이 적용돼요.

(1) 컬럼명 - 필드 이름
(2) 유니크 - false
(3) 널 가능 - true
(4) 길이 - 255

위 코드에서 Long 타입은 null이 기본 값이고 String도 마찬가지에요.
추가로 필드가 자바 원시 타입인 경우에, @column을 명시하지 않으면 테이블 생성시 not null 자동으로 추가돼요. 하지만 원시 타입에 @column을 명시하면 @column의 기본 설정인 nullable 이 적용되면서 not null이 추가되지 않아요. 원시 타입은 null을 받지 않기에 @column(nullable = false)를 명시해 주는 것이 좋을 것 같네요!

woowacourse/spring-roomescape-waiting#34 (comment)

[J-I-H-O]

Reviewer :

Reservation이 영속성 컨텍스트에 올라갔을때 가지고있는 모든 연관관계들이 반드시 객체의 형태로 존재해야하는지 고민해보시면 좋을것 같습니다.

Reservation 객체를 사용하기 위해서 모든 순간 Member의 객체 정보가 필요한가? 에 대해서 고민해보시면 좋을것 같습니다. 약한 연관관계로 Member가 아닌 member의 id를 멤버변수로 갖고 있도록 설계할 수 도 있기 때문입니다.

Reviewee :

id만 참조하는 방식을 사용하면 객체 사이의 의존관계를 끊어줄 수 있을 것 같습니다.
현재 Reservation도 Member 객체의 모든 데이터를 알 필요 없기 때문에 id만 참조하는 것도 방법일 것 같습니다.

그런데 의존관계를 끊었을 때 어떤 이점이 있을 지 아직 잘 모르겠습니다. Member의 객체가 정말 커졌을 때 의미있는 분리가 될 것 같습니다.

id참조를 사용하는 방법이 해당 객체의 전체 데이터가 필요하지 않을 때 외에 다른 경우가 있을까요?

Reviewer :

대표적으로 생명주기가 일치하지 않는 entity간의 참조는 id로 끊어서 가져가는 것이 좋다는 이야기가 많이 있습니다.

연관관계로 객체를 가지고 있는 경우와 연관관계 키를 가지고 있는 경우, 객체 그래프를 통한 탐색이 가능한가에 대한 차이가 존재합니다.
그 말을 Reservation 객체를 사용함에 있어 Member에 대한 객체 그래프 탐색이 항상 필요한가? 반드시 필요한가?에 대한 생각으로 이어질 수 있습니다.

객체 그래프를 통해 탐색이 가능하다는 것은 그만큼 의존성이 얽힌다는 것인데, 의존성이 얽히지 않으면 애플리케이션의 운영이 힘든 연관관계를 서로 가지고 있는가?에 대한 생각을 해볼 수 있습니다.

보통 이러한 관점에서 생명주기에 대한 이야기를 많이 합니다. 같은 생명주기를 가진 객체는 연관관계로 맵핑하지만 아니라면 연관관계 키를 이용하기도 합니다.

woowacourse/spring-roomescape-waiting#11 (comment)
woowacourse/spring-roomescape-waiting#24 (comment)
woowacourse/spring-roomescape-waiting#60 (comment)

---

Reviewee :

existsByThemeId와 existsByTheme 중 어떤 것을 선호하시나요?
저는 existsByThemeId를 사용하는 것은 Theme의 내부 필드를 노출하는 것이기 때문에 existsByTheme를 사용하는 것이 더 객체지향적이라고 생각합니다.

Reviewer :

저는 두개중 무엇이 더 낫다라고 말씀드리기는 어려울것 같고 상황에 따라 달라질 것 같습니다.

entity로 exist 조회를 하기 위해서는 entity의 조회가 먼저 선행되어야 할것 같습니다.
반면 id만을 사용한다면 조회가 선행되는 작업은 필요 없을 수 있습니다.

따라서 entity가 이미 있는 상황에서는 id조회가 말씀하신 것처럼 멤버변수의 불필요한 조회로 이어질 수 있어보입니다.
하지만 그렇지 않은 경우에는 성능을 고려하여 그냥 id를 이용하여 조회할 수 있을것 같습니다.
다만 entity를 이용하여 별도의 작업이 필요한 경우 id로 조회할 필요는 없을 것 같습니다.

그런데, 제 경험상 보통 entity의 조회가 exist때문이 아니더라도 필요한 경우가 많았었습니다.
그래서 저는 아예 entity를 조회하고(findBy~~) find 결과가 있으면 throw하는 방식을 더 선호합니다.

woowacourse/spring-roomescape-waiting#24 (comment)

---

조회 시 Entity 대신 DTO로 받아오기

Reviewer :

조회를 위한 기능에서 영속성 컨텍스트가 반드시 필요한지 고민해보시면 좋을것 같습니다.

조회를 위해 Entity가 필요한가에 대한 질문이었습니다. 조회성 목적을 위한 모델만을 따로 만드는 것도 하나의 방법이 될 수 있기 때문입니다.
다시말해 조회후에 별도의 행동을 하지 않는데 영속성 컨텍스트에 올라가는 Entity를 조회할 필요는 없어보였기 때문입니다.

Reviewee :

영속성 컨텍스트에 올라가는 것은 메모리를 추가로 사용하는 작업이기 때문에 확식히 Entity를 조회할 필요가 없는 것 같습니다.
특히 예약 내역 조회의 경우 데이터가 많을 수 있으므로 영속성 컨텍스트에 올리지 않는 것도 좋은 것 같습니다.
나중에 많은 데이터를 다룰 때 해결할 수 있는 방법 중 하나겠군요!

woowacourse/spring-roomescape-waiting#24 (comment)

---

Reviewer :

이렇게(test_data.sql) 테스트 데이터를 모두 지워주는것도 좋은 방법입니다 👍
그러나 한가지 아쉬운 점은 truncate 대상을 관리해주어야 한다는 것인데요,
junit에서 제공하는 extension과 함께 이를 코드레벨로 자동 관리할 수 있는 방법도 있으니 시간이 괜찮으실때 한번 찾아보시면 좋을것 같습니다.

🤭 정답지 🤭 : https://github.com/woowacourse/spring-roomescape-waiting/pull/5/files/1aeb8395718d963474fa2c18fbe99d3dc8ff9da2#diff-79190fd3ff453bb2319447d49f05aa4237b8d0f81c2b70f19dc7740a793143db

• src/test/java/roomescape/support/extension/DatabaseClearExtension.java
• src/test/java/roomescape/support/extension/DatabaseCleaner.java
• src/test/java/roomescape/service/BaseServiceTest.java

woowacourse/spring-roomescape-waiting#60 (comment)
woowacourse/spring-roomescape-waiting#5 (comment)

---

예약의 동시성 문제

Reviewer :

3명이 같이 예약요청을 한다고 가정해보았을때, 2명은 동시에 요청을 진행하였고, 그보다 느린 1명이 이후 예약 요청을 진행하였다고 상황을 가정해보았을 때 예약과 대기의 순번이 어떻게 될 수 있을까요?

Reviewee :

동시성 문제를 해결하기 위해 @Lock(LockModeType.PESSIMISTIC_WRITE)와 @Transactional(isolation = Isolation.SERIALIZABLE) 까지 추가했는데 해결하지 못했습니다.

Reviewer :

여러가지 방법이 있을것 같습니다. 시도하신 것처럼 락을 잡는것도 하나의 방법이 될 수 있을 것 같고, 트랜잭션을 끊어서 가는것도 하나의 방법이 될 수 있을것 같습니다.

예를들어 요청이 오면 모두 waiting 으로 저장을 하고 이후 트랜잭션에서 waiting에 있는것중 가장 빠른 waiting을 reseravtion으로 격상하는 방식도 있을것 같습니다.

조금 더 자세하게 설명드리면

1. API 호출

• transaction 1
• waiting 저장
• transaction 1 end
• transaction 2
• waiting 중 가장빠른 waiting을 reservation으로 저장
• transaction 2 end

위와 같은 형태로 수행하면 요청의 누락은 없고 reservation을 하나만 생성할 수 있을것 같아보입니다.

+) SERIALIZABLE 로 고립레벨을 설정하면 동시성과 관련된 여러가지 문제점이 해결이 될 수 있지만, 성능에 대한 문제가 발생하게 됩니다. 가능하면 추가적인 데이터 구조 혹은 데이터 흐름을 변경하여 해결해보도록 노력해보시면 좋을것 같습니다.

+) unique constraint, synchronized

woowacourse/spring-roomescape-waiting#90 (comment)
woowacourse/spring-roomescape-waiting#90 (comment)
woowacourse/spring-roomescape-waiting#95 (comment)

---

도메인은 가능한 외부 라이브러리에 의존하지 않도록 작성

Reviewer :

저의 생각을 먼저 말씀 드리면 저는 가능한 연관관계를 맺지 않는 것을 좋아합니다.

jpa 라는 특별한 라이브러리에 도메인 기능이 종속적이게 되는 것에 불편한 경험이 많기 때문입니다.
애플리케이션의 요구사항은 예상치 못한 방향으로 변경되기도 하는데 기존 정책을 뒤집어야 하는 경우도 있습니다. 이러한 경우 jpa의 특정 영속성 컨텍스트를 이용하는 기능을 사용했다가 변경에 어려움을 겪은적이 있습니다.

그렇다면 절대 사용하지 않느냐? 라고 묻는다면 그렇지는 않습니다.
명백히 생명주기가 일치하는 케이스라면 사용하기도 합니다.

조금 더 풀어서 말하면 생성되는 시기와 제거되는 시기가 언제나 동일하다면 연관관계로(OneToMany) 묶어서 사용하곤 합니다.
이러한 경우라면 cascade와 같은 속성을 사용해도 크게 이질감 없이 사용할 수 있기 때문입니다.

위 말씀을 토대로 다시 정리해보자면, 저는 jpa도 하나의 외부 라이브러리로 바라보고 있기 때문에
도메인이 해당 라이브러리에 의존하는것을 그다지 선호하는 편이 아닙니다.

도메인은 가능한 POJO하게 작성하는게 좋다고 생각하는 편이라 기회가 있다면 도메인용 객체와 영속성 컨텍스트용 객체 또한 분리하는 편입니다.

위와 같은 맥락에서 JPA의 특정 기능( 쿼리 조회시 조건절을 포함시키는 등(@Where))들에 의존하는것을 선호하지 않는 편입니다.

woowacourse/spring-roomescape-waiting#90 (comment)

---

Reviewer :

테스트 코드는 가능한 테스트 코드만 보고 무엇을 테스트하고자 하는지 파악할 수 있도록 작성하는 것이 좋습니다.
그래서 저는 Sql을 통해 setup data를 추가하는 행위를 지양하는 편입니다.

그 이유는 다음과 같습니다.

1. 해당 insert 쿼리는 애플리케이션을 통해 insert 될 수 없는 데이터가 될 수 있습니다. 이는 곧 올바른 데이터 모델의 조회가 아닐 수 있다는 것과 일맥상통해집니다.
2. 테스트 코드에서 검증하는 값들의 정확한 이유를 파악하기 위해 sql script를 확인해야합니다. 화면의 이동을 뜻하고 개발자의 인지과부하 요소로 작용할 수 있습니다.
3. sql script에 종속적인 테스트가 되어 여러곳에서 sql script를 활용하거나 sql script가 변경되면 테스트가 같이 변경되어야 합니다. 변경을 해야하는 상황이 발생했을때 sql script를 어떻게 바꾸어야 하는지 작성한 사람이 아니면 알기 힘든 경우가 많습니다.

이러한 불편함이 있지만 경험하신 것처럼 장점도 분명히 있습니다. 테스트 코드 본문 자체는 간결해집니다.

저는 이러한 불편함을 해소하기 위해 test fixture를 사용합니다. 해당 테스트 클래스 혹은 testFixture 패키지에 데이터를 생성할 수 있는 private 메서드를 미리 준비해두고 연속적으로 호출하여 필요한 데이터를 구성하는 방법입니다.

Reviewee :

1. 엄청 크게 초기 데이터를 만들어놓고 다른 여러 곳에서 사용하지는 않나요?
2. sql insert가 아닌 코드로 데이터를 넣어준다면, 어느 부분까지 검증된 코드를 넣나요? 예를 들어 도메인에 있는 검증만 거쳐서 된 값을 넣나요? 아니면 서비스에도 검증 로직이 있는데 그것까지 거친 값을 넣어주나요?
3. Fixture를 다른 폴더에 두고 사용할거면 sql이나 별반 다를 거 없지 않나요??

Reviewer :

엄청 크게 초기 데이터를 만들어놓고 다른 여러 곳에서 사용하지는 않나요?

제가 속한 곳에서는 지양하는 방법입니다. 초기 데이터에 영향을 너무 많이 받고 해당 테스트가 어떤 초기 데이터에 영향을 받는지 명시적으로 알기가 너무 힘든 경우가 많아지기 때문입니다.
따라서 테스트 데이터는 테스트 메서드 내에서 그때 그때 생성해주는 편입니다.

그리고 테스트를 위해서는 최소한의 데이터만 준비하는 것이 좋습니다. 큰 데이터는 큰 비용을 초래하기 때문에 테스트의 비용또한 올라갈 것 같습니다.

sql insert가 아닌 코드로 데이터를 넣어준다면, 어느 부분까지 검증된 코드를 넣나요? 예를 들어 도메인에 있는 검증만 거쳐서 된 값을 넣나요? 아니면 서비스에도 검증 로직이 있는데 그것까지 거친 값을 넣어주나요?

보통 최소한의 데이터 정합성을 위해서 도메인에 객체를 생성후 insert합니다. service를 통한 데이터 생성은 가능한 지양하는 편입니다.
해당 service가 안정적인 데이터 셋을 제공하는지 명시적으로 보장되지 않기 때문입니다.

Fixture를 다른 폴더에 두고 사용할거면 sql이나 별반 다를 거 없지 않나요??

fixture를 이용하면 테스트 메서드에서 생성되는 데이터를 명시적으로 표현할 수 있지만 sql을 이용하면 해당 스크립트로 이동해서 생성되는 데이터를 확인해보아야 합니다.

지금의 경우 waiting.sql에서 무엇을 생성하는지 확인해야하는것이 fixture를 이용하면 테스트 메서드 내에 몇줄로 명시적으로 드러낼 수 있게 되는 것이 큰 장점으로 보입니다.

참고 : https://blog.kingbbode.com/52

woowacourse/spring-roomescape-waiting#93 (comment)

---

데이터 영속성 모델과 도메인 모델은 1:1 일까요?

Reviewer :

데이터 영속성 모델과 도메인 모델을 1:1 관계로 바라보고 있지 않은지, 1:1로 바라보고 있다면 꼭 그래야만 하는지 고민해보시면 좋을것 같습니다.

데이터 영속성 모델이라 함은 JPA entity를 말하는 것이고, 도메인 모델이라함은 말 그대로 문제해결을 위한 객체를 말씀드린 것 입니다.

즉, Jpa Entity를 꼭 문제해결을 위한 도메인 모델로 써야하는 것처럼 생각하시지는 않을지 질문한 것 이었습니다.
영속성 모델은 저장이라는 문제해결을 위한 객체로 볼 수 있기 때문입니다.

woowacourse/spring-roomescape-waiting#93 (comment)

[alstn113]

리뷰어: 양방향 연관관계의 단점

woowacourse/spring-roomescape-waiting#128 (comment)

---

리뷰어: 역정규화도 선택할 수 있는 대안이다.

count쿼리는 무겁고, join을 할 경우 row가 많아질 수 있다.
조회에 대한 부하가 커질 경우 역정규화를 많이 활용한다.
update 상황에서는 불리하지만 대부분 웹 서비스는 조회가 갱신보다 빈번하게 일어난다.

woowacourse/spring-roomescape-waiting#105 (comment)

[nayonsoso]

🏃‍♂️🦆

🔸 JPA 가 자동으로 매핑해줌에도 @Table 과 @Column으로 이름을 명시해줘야 하는 이유

JPA가 자동으로 만들어주어서 불필요한 것처럼 보이지만, table은 한번 생성되면 수정하기가 매우매우 어려워요. 
그래서 이렇게 설정되면 필드명을 바꾸는 것이 까다로워질 수 있고, 처음부터 일관적으로 분리해서 관리하는 것이 낫다는 의견 드려요 :)
필드명과 테이블 명은 일반적으로 camelCase로 작성하니, column 명으로 전역검색할 때 불편하기도 하구요.
자칫하면 큰 장애로 이어질 수 있으니, 되도록 분리하여 관리하는 것이 좋다고 봐요.

🔸 Enbedded VO 를 사용할 때 주의할 점

• Embedded VO는 포함하고 있는 엔티티의 생명주기에 직접적으로 종속되어 독립적으로 존재할 수 없다.
• Embedded VO는 내부 필드의 변경만으로는 JPA가 엔티티의 상태가 변경되었다고 인식하지 못할 수 있다.
• Embedded VO가 다른 엔티티간에 공유되는 경우, 엔티티에서의 변경이 다른 엔티티에 영향을 미칠 수 있다.
• VO는 가능한 한 불변 객체(final)로 설계하는 것이 좋다. 객체가 한 번 생성되면 그 상태를 변경할 수 없도록 함으로써, 부작용을 줄이고 객체의 안전한 사용을 보장할 수 있다.

🔸 Embedded VO의 변경을 JPA가 자동으로 감지하지 못하는 경우 해결 방법

• JPA의 변경 감지(Dirty Checking) 기능은 영속성 컨텍스트 내에서 엔티티의 상태 변화를 자동으로 감지하고 데이터베이스에 반영한다. 하지만, JPA 구현체와 구성에 따라서 @Embedded 객체의 필드 변경이 즉각적으로 감지되지 않을 수도 있다.
• 따라서 데이터 무결성을 위해, 변경 후 명시적으로 save() 메서드를 사용하여 엔티티의 상태를 강제로 업데이트하는 것이 권장된다.

// 주소 업데이트 로직 (User가 Address Embedded VO를 갖고 있다고 가정)
public void updateAddress(User user, Address newAddress) {
  user.setAddress(newAddress);
  userRepository.save(user);
}

• JPA는 쓰기 지연 저장소 기능을 사용하여 트랜잭션 내에서 발생하는 여러 변경 사항을 모아서 트랜잭션이 커밋되는 시점에 한 번에 데이터베이스로 플러시한다.
• 따라서 save() 호출은 트랜잭션이 종료되는 시점에 수행되며, 즉시 데이터베이스에 쓰기 작업을 수행하지 않으므로 성능에 미치는 부담은 크지 않다.

🔸 lazy loading의 도입 시점을 고민해보자

lazy loading을 도입하는 경우에 eager로 가져오는 경우에 비해서 신경써야하는 것들이 많아져요. 
proxy 객체가 사이에 끼게 되니까요. 
그래서 일반적으로 실제 성능 이슈가 생기기 전까지 lazy loading 설정은 되도록 미루는 것을 권장해요. 
select 쿼리 몇개 더 나가는 게 보통 크게 문제되지 않는 경우가 많거든요. 
서비스 초반일수록 그 영향이 미미하고, lazy loading 을 도입하는 것이 오히려 과한 최적화 작업일 수 있어요.

이미 findAll 같은 경우에 fetch join을 해오고있어서 n+1과 같이 뻔하게 예상되는 성능 저하는 잘 피해가신 상황인데, 
별도로 lazy loading 하는 것이 필요한 최적화일지 한 번 생각해보셔도 좋겠네요 :)

woowacourse/spring-roomescape-waiting#38 (comment)

🔸 @NotNull vs @column(nullable = false)

둘 다 ddl 에 NOT NULL 을 달아주지만,
@NotNull 은 엔티티가 생성되는 시점에 필드를 검증해주고,
@column(nullable = false) 은 DB에 저장되는 시점에 검증이 들어가므로 더 안전하다고 할 수 있다.
cf. 참고로 @notempty 와 @notblank 는 하이버네이트에서 DDL을 만들 때 NOT NULL 키워드를 안넣어준다고 한다.

🔸 OSIV 를 꺼두는게 좋을까?

리뷰어가 OSIV에 대해 아래와 같은 이유로 끄는게 좋겠다고 말한 상황…

• OSIV 가 true이면 영속성 컨텍스트가 request ~ response의 생명주기를 가짐.
• 그 과정동안 connection을 갖고있게 됨. connection이 필요하지 않은 부분에서도 불필요하게 connection을 갖고있는 문제.

언급해주신 것처럼 문제가 될 여지가 있는데다가, 만에 하나라도 의도치 않은 더티체킹을 유발할 수 있다는 문제가 있어요.
그래서 OSIV는 특별한 이유가 없는 한 꺼두고 모두 트렌젝션으로 처리하는 것이 대개는 더 안전해요.

[le2sky]

리뷰이 : 중복되는 코드를 줄이는 과정에서 AdminReservationService, ReservationTimeService, ThemeService가 ReservationService를 필드로 가지고 있게 되었습니다. 이런 구조가 괜찮다고 보시나요?

리뷰어 : 네 Service가 Service를 의존하지 말라라는 규칙은 없어요. 다만 Service가 Service를 의존하게 되는 경우 그 반대로도 의존을 추가해서 순환참조가 일어나지 않게끔만 고려하면 좋을 것 같아요.

리뷰이 : 저는 Response를 반환하는 메서드와 도메인을 반환하는 메서드가 함께 있는 것이 마음에 걸려 개선하고 싶은데 어떤 식으로 해야 좋을지 모르겠어요

리뷰어 : Response를 반환하는 것과 도메인을 반환하는 것의 차이가 어떤건지 고민해보고 Service는 어떻게 반환을 하는 것이 좋은가를 고민해보면 어떨까요? 정답은 없는 내용이지만 Service라는 "계층"에 대해서 도메인을 그냥 반환하는 것에 대해서 어떻게 받아들이느냐에 따라 다를 것 같아요. jpa를 사용하게 되면서 @entity를 반환하는 것에 대한 기능이 어떻게 동작하는지도 함께 고려해보세요.

추가로 domain을 단순히 find해서 반환하는 경우에 굳이 Service로 감싸지 않고 다른 Service가 Repository를 의존하지말라는 법은 없는 것 같네요.

링크 : woowacourse/spring-roomescape-waiting#8 (comment)

---

리뷰이 : 인기 테마를 조회하는 과정에서(findPopularThemes 메서드) stream을 많이 사용하게 되었습니다. 이때 stream을 사용하는 것과 @query로 sql문을 작성하는 것 중 어떤 방식이 좋을지 고민이 되었어요. 오리는 복잡한 쿼리가 필요한 경우 둘 중에 어떤 방식을 선호하시나요?

리뷰어 : 가능하면 복잡한 쿼리를 배제하려고 합니다. 복잡한 쿼리가 필요하면 필요할수록 설계단에서 도메인 중심적으로 짰는가, 모든걸 쿼리로 해결하려고 하고있지는 않은가를 고민해볼 필요가 있는 것 같아요. 쿼리로 모든걸 해결해야하는 상황이라면 Service, Domain들이 필요할까요?

링크 : woowacourse/spring-roomescape-waiting#8 (comment)

---

상황 : @column을 생략한 상황

리뷰어 : @column을 명시하지 않아도 동일한 네임의 컬럼일 경우 매핑이 되기는하지만, 테이블 정보를 @entity에 명시하기도 해요. 테이블을 굳이 한번 더 찾아가서 보지 않아도 되게끔 해준다라는 장점이 있는데, 데이터베이스 변경점이 있을때마다 옵션을 관리해주어야한다는 트레이드오프가 있습니다.

(반영하지 않으셔도 됩니다.) @column옵션에서 db와 비교하면서 볼만한 컬럼 옵션은 name, nullable, unique, length인 것 같아요. 그래서 이런 값들을 추가하여 db 스키마를 굳이 보지 않고 entity만 보고도 스키마 정보를 알 수 있도록 넣기도 해요.

링크 : woowacourse/spring-roomescape-waiting#8 (comment)
링크 : woowacourse/spring-roomescape-waiting#67 (comment)

---

리뷰이 : 기본 생성자의 private 설정이 불가한 이유는 프록시 설정 때문이라고 하는군요!

리뷰어 :
여기까지 학습하셨으니 나중에 학습할 수 있는 키워드를 드리면

private, default는 안되는데 public, protected는 된다.
public, protected는 가지고 있는 특성인데 private, default는 가지지 못하는 특성이 무엇인가?
그 특성을 프록시라는 것에서 활용할 것이다.
정도만 알고 넘어가시면 될 것 같아요.

링크 : woowacourse/spring-roomescape-waiting#14 (comment)

---

상황 : assertAll로 true false 검증하는 상황

리뷰어 : 테스트의 대상은 existsByTheme이 맞으나 테스트하려는 상황은 true, false로 분리되어있는데요. 테스트메소드가 제공하고자하는 것은 "테스트 대상"의 "각 상황"에 대한 테스트에요.

그래서 두 케이스는 분리하는 것이 맞습니다. 테스트도 하나의 문서라고 생각한다면 "예약이 존재하면 true를 반환한다", "예약이 존재하지 않으면 false"를 반환한다라는 하나의 문서가 되기도 하고요.

링크 : woowacourse/spring-roomescape-waiting#14 (comment)

---

상황 : 테스트에서 @transactional에 대한 티키타카

리뷰이 :

테스트 레이어에 적용된 트랜잭션 작업은 각 테스트가 끝날 때 트랜잭션을 롤백하는 것이 기본으로 되어 있습니다.(서비스 레이어에서는 커밋이 기본) JPA의 변경 감지는 트랜잭션을 커밋하는 경우에 변경 내용을 데이터베이스에 반영하기 때문에, 롤백이 기본인 테스트 코드에서는 insert나 update 쿼리로 데이터베이스에 반영하는 작업을 하지 못하고 1차 캐시의 내용이 그대로 휘발되어 버립니다. 따라서 엔티티 정보가 DB에 커밋되지 않고 1차 캐시에만 남아있을 때 DB 조회 테스트를 하면 깨지는 사례가 발생할 수 있습니다

리뷰어 :

커밋하는 경우 데이터베이스 반영은 맞기는 한데 조금 더 구체적으로 학습해볼 필요가 있을 것 같아요. 데이터베이스에 대한 쿼리 요청은 엄밀히 따지면 "commit"이 아니라 "flush"가 더 적절해요. 여기서 commit 시점에 flush가 날아가도록 jpa가 구성된 이유에 대해서 학습해보시면 좋을 것 같아요.
DB 조회 테스트를 하면 깨지지는 않을 수 있는 것이 @transactional을 클래스단에 선언되었기 때문에 하나의 테스트는 한 트랜잭션에 묶이게 되어요. 이때 커밋을 하지 않더라도 insert 혹은 update 메소드 동작에 대해서 1차캐시에 선반영이 되기때문에 테스트 자체는 성공합니다.

그래서 1차캐시에 반영이 되어있어 "실제 db에는 반영이 되지 않을 수 있다"가 조금 더 맞는 표현일 것 같아요. Service나 Repository를 통합테스트하려는 이유는 실제 db에 잘 반영되었는가를 테스트하는 것인데 1차캐시에 반영된걸로 테스트가 통과되어버리니까요. 부가적으로 insert임에도 불구하고 커밋을 하지 않아도 insert 쿼리가 날아갈 수도 있습니다. 현재의 Entity들에서는 모두 적용될 내용일 것 같은데요. 생성 전략이 IDENTITY일 때 commit을 직접하지 않아도 쿼리가 날아가는 것을 확인해보고 왜 쿼리가 날아가는지 한번 학습해보세요.

리뷰이 :

commit 시점에 flush가 날아가도록 jpa가 구성된 이유는 쓰기 지연을 지원하기 위함인 것 같아요. flush 기능으로 SQL을 모아서 데이터베이스에 한 번에 보냄으로써 네트워크 호출 비용을 줄일 수 있습니다.
(아래 주신 코멘트에 따라 테스트의 쿼리를 비교해보니 @transactional이 없는 경우 delete 트랜잭션 커밋 시점에 delete 쿼리가 발생하는데, @transactional이 있는 경우 findAll 트랜잭션 커밋 시점에 delete 쿼리가 발생하네요!)
IDENTITY 생성 전략은 엔티티를 데이터베이스에 저장해야 식별자를 구할 수 있으므로 em.persist()를 호출하는 즉시 INSERT SQL이 데이터베이스에 전달되어 쿼리가 발생합니다!

리뷰어 :

네 그래서 테스트에서 @transactional이 있는 경우 테스트 메소드 내의 모든 실행이 하나의 Transaction으로 묶이기 때문에 테스트를 위한 실행 메소드가 실제 메소드와 동일한 Transaction에 묶이는 문제가 발생한다 정도까지만 정리하고 넘어가셔도 좋을 것 같아요. 그 영향도로 jpa를 사용하면 테스트를 위한 메소드와 실제 메소드가 같은 영속성 컨텍스트를 공유하는 문제가 발생하고요.

링크 : woowacourse/spring-roomescape-waiting#130 (comment)

[jimi567]

리뷰어 : 근데 db에 접근하는 findAll은 어떻게 동작하고 있을까요? (@transactional(readOnly = true 을 사용하는 코드에 대한 질문)

리뷰이 : findAll은 기본적으로 @transactional(readOnly = true)을 설정하고 있어서 제가 명시적으로 어노테이션을 붙일 필요가 없네요... ㅎ... 수정하겠습니다.

알아볼 것 : findAll은 내부적으로 @transactional(readOnly = true을 사용하고 있다.

ref : woowacourse/spring-roomescape-waiting#83 (comment)

---

public class MemberReservationRequest {

    @NotNull(message = "테마 아이디는 반드시 입력되어야 합니다.")
    @Positive(message = "테마 아이디는 자연수여야 합니다. ${validatedValue}은 사용할 수 없습니다.")

(생략)

리뷰어 :
TermResolver에서 interpolate해주는군요 하나 알아갑니다

리뷰이 :
단순히 ${validatedValue} 뿐 만이 아닌 아래 코드 처럼 min 과 max 를 이렇게 사용할 수 있다.

@Size(
  min = 5,
  max = 14,
  message = "The author email '${validatedValue}' must be between {min} and {max} characters long"
)
private String authorEmail;

MessageInterpolator 인터페이스를 상속 받은 클래스인 MessageSourceMessageInterpolator 가 기본적으로 포맷팅을 해주는 것으로 확인됩니다!

알아볼 것 : https://www.baeldung.com/spring-validation-message-interpolation

ref : woowacourse/spring-roomescape-waiting#50 (comment)

---

리뷰어 :
이렇게 하면 변경가능한 부분과 아닌 부분이 나뉘고, update할 수 있는 요소를 명확히 할 수 있어요.

@DynamicUpdate
class SomeEntity {

  public void change(UpdateOption option) {
      ...
  }
}

record UpdateOption(...)

알아볼 것 : @DynamicUpdate
ref : woowacourse/spring-roomescape-waiting#160 (comment)

---