멤버 뒷고기

[GIVEN53]

.

[le2sky]

티키 : 계층의 명확한 분리를 위해서는 Controller가 요청으로 받는/응답으로 뱉는 dto, Service에서 요청/응답 dto가 서로 나뉘어야 하지만, 두 Dto의 파라미터 집합이 서로 동일해 한 쪽으로 흡수되었다고 이해함

타카 :
data transfer object라는 명칭에서 알 수 있듯 엄밀히 따지면 Controller의 dto, Service의 dto 모두 분리하는 것이 맞다.
서로 동일해 흡수되었다는 것보다는 의존의 방향성을 생각해보면 좋을 것 같다.
상위 계층(Controller)는 하위 계층(Service)를 의존해도 되지만, 그 역순이 되면 의존 관계 자체가 꼬인다.
Service가 통합된 dto를 가지는 것이 차선책이 된다.

링크 : woowacourse/spring-roomescape-member#61 (comment)

---

상황 : 인증 객체를 Request 객체에 저장하는 상황

리뷰어 : 현재 어플리케이션 규모에서 ThreadLocal 혹은 @requestScope를 사용하지 않아도 된다는 것에는 동의하는데 HttpServletRequest를 컨트롤하는 것은 지양하는 편이 좋아요.

HttpServletRequest는 실제 request에서 들어오는 값들이 담긴 것으로, 여기서 뭔가 값을 추가하게되면 실제 http request에서 더 추가가 되는 것이기 때문에 이걸 만약에 쓸 수도 있는 다른 객체에서 request를 오인할 수 있어요. 그리고 HttpServletRequest와 같은 클래스들을 커스텀하다 에러 트러블슈팅할 일이 생기게 될 수 있는데 그때보면 HttpServletRequest에서 값을 읽거나 쓰는 동작이 왜 지양되는지 확인할 수 있을 것 같아요.

그래서 반영하지 않아도된다라는 뜻은 저 두가지가 아닌이상 사실 반복 로직을 타는게 낫다라는 생각입니다.

링크 : woowacourse/spring-roomescape-member#134 (comment)

[GIVEN53]

reviewee: GET은 body를 가질 수 없나요? 만약 그렇다면, 일반적으로 RequestParam을 사용하나요?

reviewer:
답변에 앞서서 RequestParam과 Requestbody 는 어떨 때 사용하면 좋을까요?

리소스를 구체적으로 url 에 명시하면

1. 캐싱을 할 수 있다. (조회 되는 데이터가 빈번한 변경이 없는 경우)
2. url 만으로 어떤 데이터가 올지 유추할 수 있다.
3. 요청 파라미터들이 URL로 인코딩되므로 즐겨찾기가 가능하다.

요청 데이터를 body에 숨기게 되면

1. 개인 정보를 url 에 노출시키지 않고 보호할 수 있다.
2. 파일을 업로드하는 것을 포함하여 큰 데이터를 송수신 할 수 있다.
3. form data의 길이 제한이 없다.

이런 차이가 있어요. 그래서 용도에 맞게 사용하는 것이 좋은데요.
Spring RestTemplate 에 경우 Get 메서드를 사용하는 경우 body 를 쓰지 않도록 설계 되어 있어요

reviewee:

• HTTP/1.1에 따르면 GET이 검색 이외의 동작을 수행하는 것을 권장하지 않음.
• HTTP/1.1에 따르면 GET 메서드는 Request-URI로 식별되는 정보를 검색하는 것을 의미.
• GET 요청은 캐시될 수 있는데, GET 요청을 간단하게 유지하여 캐시를 관리하기 쉽도록 해야 함.
• GET 요청은 안전 및 멱등해야 함. body를 허용하지 않음으로써 안전 & 멱등을 보장할 수 있음.

link: woowacourse/spring-roomescape-member#3 (comment)

---

reviewee: date, varchar 중 어떤 것을 사용할지에 대한 기준이 있으신가요?

reviewer:
아래와 같이 크게 4가지 이유로 인해서 날짜 타입은 date 타입으로 핸들링 하는 것이 좋을 것 같다고 생각합니다.

1. size
   varchar(8) 사용 시 언뜻 하이픈(-) 없이 데이터를 압축, 저장할 수 있을 것만 같지만 실제로는 date 타입에 비해 3배 큰 용량을 차지하게 됩니다.
   MySQL에서 varchar는 String 데이타 타입 중 하나로 가변(variable) 문자(character) 타입을 의미합니다. 고정된 용량을 사용하는 char와 달리 varchar는 실제 용량을 표현하기 위한 prefix를 필요로 합니다.
   따라서 varchar(255) 이하에서는 1 byte, 이상에서는 2 byte의 용량이 필요합니다. (1 byte = 8 bit, 즉 2^8=256, 0부터 255까지)
   UTF-8 인코딩방식에서는 하나의 문자 당 1바이트에서 최대 4바이트를 지원하지만 우리가 사용할 숫자로만 구성된 년월일의 경우 각 1바이트 * 8에 prefix인 1byte까지 총 9 byte를 필요로 합니다.
   반면 date 타입은 '1000-01-01' 부터 '9999-12-31' 까지의 값을 지원하며 3 byte의 용량을 필요로 합니다. 년, 월, 일을 나누어 인식하므로 최대 값('9999-12-31')도 3 byte로 충분히 표현할 수 있습니다.

2. validation
   값 입력 시 varchar(8) 문자열은 각 문자열을 년, 월, 일로 잘라 각각이 유효한 값인지 검사하여야 하지만 실제 저희 프로그램에서는 자리 수 체크 정도만 진행되고 있습니다. '88888888'도 유효한 날짜로 인식됩니다.
   date 타입은 날짜 타입인 만큼 월과 년,월에 따른 일에 대해 유효하지 않은 경우 예외가 발생합니다. 예를 들어 '1990-02-29' 또는 '2000-11-31' 같은 값은 입력할 수 없습니다.
   그러나 모든 유효하지 않은 날짜를 막는 것은 아닙니다(00월이나 00일은 허용). 또 비즈니스마다 유효한 연도의 범위도 필요하므로 이 이상의 validation은 어플리케이션 단에서 로직을 통해 통제하는 것이 합리적으로 보입니다.

3. 타 언어와 호환
   MySQL date 타입은 java.sql.Date로 리턴됩니다. 또 java.lang.String, java.sql.Date, java.sql.Timestamp와 같은 객체로 변환이 가능합니다.

4. 비교 및 연산
   MySQL에서는 날짜 연산을 위해 자체적으로 다양한 함수들을 제공합니다 MYSQL 날짜 및 시간 함수.
   date타입은 datetime, timestamp 등의 날짜 타입과 호환되므로 위 링크의 모든 함수 사용이 가능합니다.
   varchar(8)의 경우 String 타입이므로 바로 함수를 사용할 수 없습니다. DATE() 함수나 STR_TO_DATE() 함수로 가공 후 사용해야 합니다. 단순 문자열 비교는 가능하지만 더 구체적인 월 비교, 일 비교나 더 나아가 연산은 지원되지 않습니다.

link: woowacourse/spring-roomescape-member#81 (comment)

---

dependencies {
    implementation 'io.jsonwebtoken:jjwt-api:0.12.5'
    runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.12.5'
    runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.12.5'
}

reviewer:
해당 의존성을 3개 선언해주셨는데요~
각각 어떤 용도로 선언하게 되었는지 궁금합니다,
또 어떤건 임플리먼츠하였고, 어떤건 런타임 온리인데 이것두요!

reviewee:

• 'io.jsonwebtoken:jjwt-api:0.12.5': 명시적으로 설계된 jjwt api를 의미합니다.
• 'io.jsonwebtoken:jjwt-impl:0.12.5' : jjwt 기타 세부 구현 세부사항을 의미합니다.
• 'io.jsonwebtoken:jjwt-jackson:0.12.5' : Header 및 Claims Map을 JSON으로 직렬화하거나, JSON을 Header 및 Claims로 역직렬화 하기 위해 사용됩니다.

'io.jsonwebtoken:jjwt-api' 만 implemets로 선언하고 나머지 의존성은 runtimeOnly로 선언된 이유는 다음과 같습니다.
어플리케이션에서는 api에만 의존하도록 설계되었으며, 경고없이 변경될 수 있는 세부 사항은 runtimeOnly로 선언되어 변경에 보장하기 위해서 입니다.

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

---

reviewer:
단건으로 조회하여 합쳐주기에서 꼭 고려해야하는 점은
이렇게 조회된 데이터의 성질인데요~
여러 번의 쿼리를 날리는 도중 데이터 변경이 생길 경우 데이터 일관성을 유지하기 어렵습니다.

그래서 일관된 데이터가 필요할 경우 한꺼번에 조인을 통해서 가져오는 것이 더 적절한 상황이 있을수도 있을 것 같아요

예를 들면 방탈출 예약서비스 일 때 조회가능한 시간을 먼저 조회하고, 테마를 조회하고~.. 또 다른 걸 조회하고 있을 때
누군가가 예약을 해버려서 조회가능한 시간이 줄어들었다면,,?

link: woowacourse/spring-roomescape-member#113 (comment)

[jcoding-play]

// Reservation 객체 안에서 BadRequestException 반환하는 상황
private void validateName(String name) {
    if (name == null || name.isBlank()) {
        throw new BadRequestException("예약자 이름은 비어있을 수 없습니다.");
    }    
}

reviewer : 도메인에서 이 예외가 "요청"이라는 걸 알 필요가 있을까요?

link: woowacourse/spring-roomescape-member#21 (comment)

---

reviewee: 잘못된 요청(bad request)에 대한 예외들 역시 로그로 남겨야 하는 이유가 무엇인가요 ?? 처음에는 서버 애플리케이션의 문제가 아니라면 예외에 대한 로그를 기록할 필요가 없다고 생각하여 남기지 않았는데 로그를 남기는 게 좋은 이유가 궁금합니다!

reviewer: 서비스를 운영하다보면 고객 cs가 들어올 때도 있고, 버그가 있는 경우 평소보다 에러가 늘기도 합니다. 이런 경우에 로그가 없다면 그 원인을 추적하기 쉽지 않겠죠.

link: woowacourse/spring-roomescape-member#21 (comment)

---

@Transactional
public void delete(Long id) {
    ReservationTime reservationTime = reservationTimeRepository.findById(id)
            .orElseThrow(() -> new NotFoundException("해당 ID의 예약 시간이 없습니다."));

reviewer: delete 동작의 핵심을 "지우는" 것이 아닌 "자원이 없는 상태"라고 생각한다면 에러를 던지지 않고 메서드를 종료할 수도 있겠네요.
rest 관점에서 봐도 예외를 던져주지 않아도 문제가 없습니다.

link: woowacourse/spring-roomescape-member#21 (comment)

---

reviewer: Spring validation을 사용한 것과 도메인에서 검증하는 것은 어떤 차이가 있을까요?

reviewee: 우선 Spring validation은 비즈니스 계층 전에 빠르게 검증할 수 있습니다. Spring validation을 거친 요청 데이터를 더 비즈니스 계층에서 전반적으로 안전하게 사용할 수 있습니다. 다만 요청이 아니더라도 도메인을 생성할 수 있기 때문에 항상 안전한 도메인을 생성하기 위해서는 도메인에서 검증해야 합니다.

link: woowacourse/spring-roomescape-member#92 (comment)

---

public class TokenExtractor {
    private static final String TOKEN_COOKIE_KEY = "token";

    public static String extract(HttpServletRequest servletRequest) {
        Cookie[] cookies = servletRequest.getCookies();
        if (cookies == null) {
            throw new AuthorizationException("토큰이 존재하지 않습니다.");
        }

reviewer: 이런 부분도 고민해볼만 할 것 같긴 하네요.

• TokenExtractor가 쿠키가 없다고 예외를 던지는 게 맞을지
• 예외를 던질거라면 AuthException을 던지는 게 맞을지

객체 입장에서 이런저런 고민을 해보면 좋을 것 같습니다.

reviewee: TokenExtractor가 토큰 추출의 역할을 한다고 한정하면 예외를 던지기 보다 Optional으로 토큰을 반환하는게 더 적절할 것 같습니다. '토큰이 있어야만 한다'는 인증에 대한 검증을하는 LoginMemberArgumentResolver나 AdminAuthorizationInterceptor에서 확인하고 예외를 던지는게 더 합리적이라고 생각해서 변경하였습니다!

link: woowacourse/spring-roomescape-member#92 (comment)

---

reviewer: Interceptor와 ArgumentResolver는 어떤 차이가 있을까요?

reviewee: Interceptor는 controller의 요청과 응답을 가로채어 동작하고 ArgumentResolver는 요청(HttpServletRequest)으로 controller의 파라미터를 조작할 수 있습니다. 따라서 Interceptor는 권한(관리자 권한 api 및 페이지)을 다룰 때 ArgumentResolver는 인증된 사용자(Member)를 주입받을 때 유용합니다.

link: woowacourse/spring-roomescape-member#92 (comment)

---

if (themeRepository.hasTheme(new Name(themeAddRequest.name()))) {
    throw new ClientException("이미 존재하는 테마 이름입니다.");
}

reviewer: 멀티 스레드 환경에서 같은 이름을 add하는 요청이 여러번 빠르게 올 경우 이 검증 로직을 통과할 수 있습니다.
이런 경우는 db에 unique 인덱스를 넣어줄 수 있을 것 같네요.

---

reviewer: 커스텀 예외가 아닌 일반 예외들의 메세지가 클라이언트에 노출되면 어떤 일이 일어날 수 있을까요?

reviewee: 커스텀 예외가 아닌 표준 예외는 외부 라이브러리와 같이 의도하지 않은 장소에서도 발생할 수 있습니다.
이때 현재 코드에서와 같이 예외 메세지를 응답으로 내려준다면 보안상 문제가 발생할 수 있을 것 같습니다 😢

먼저 애플리케이션의 내부 구조나 로직에 대한 정보를 노출할 수 있을 것이고,
또한 클라이언트가 이해하기 쉽게 처리해둔 예외 메세지와는 다른 형태의 기본 예외 메세지를 반환하므로, 클라이언트가 이해하기 어려운 내용이 포함될 수 있습니다.

link: woowacourse/spring-roomescape-member#56 (comment)

---

reviewer: 어노테이션에 기반한 argument resolve는 어떤 장단점이 있을까요?

reviewee: HandlerMethodArgumentResolver의 supportsParameter() 메서드를 오버라이딩 할 때, 어떤 파라미터에 값을 넣어줄지 결정하기 위해 파라미터 타입을 사용하는 방법과 어노테이션을 사용하는 방법 이 있습니다.

• 파라미터 타입을 사용하는 방법
  파라미터 타입을 사용하는 경우 어노테이션을 만들지 않아도 되고, 코드가 더 간결해진다는 장점이 있습니다.
  또한 같은 타입을 가지는 파라미터는 일관된 방식으로 처리됨이 보장됩니다.
  그러나 이는 파라미터 타입을 사용하는 방식의 단점이자, 어노테이션을 사용하는 방식의 장점으로도 볼 수 있습니다.
  파라미터 타입을 사용하는 경우, 같은 타입의 파라미터라도 다르게 처리하고 싶은 경우에는 유연성이 떨어질 수 있기 때문입니다.

• 어노테이션을 사용하는 방법
  반면 어노테이션을 사용하면 같은 타입의 파라미터라도 어노테이션의 유무에 따라 다르게 처리할 수 있습니다.
  또한 컨트롤러 코드에 어떤 파라미터가 특별히 처리되는지 명확하게 표시할 수 있습니다.
  그러나 어노테이션을 추가적으로 만들어야 하기 때문에 만약 특정 파라미터를 위한 어노테이션이 많아진다면 관리에 어려움이 있을 수 있습니다.

link: woowacourse/spring-roomescape-member#91 (comment)

---

reviewer: name과 role 등이 토큰에 있을 때의 장단점이 어떻게 될까요?

reviewee: (길어서 링크로 대체합니다.)

link: woowacourse/spring-roomescape-member#91 (comment)

---

reviwer: util이란 패키지명에는 어떤 클래스들이 위치하나요?

reviewee: 특정 도메인에 종속적이지 않으면서, 프로젝트 전반적으로 사용하는 클래스이 util 패키지에 모여 있어야 한다고 생각합니다.

물론 지금은 로그인/인증 부분에서만 CookieParser를 사용하고 있기는 하지만, '키 이름을 전달받아 해당하는 키의 쿠키 값을 파싱하는 기능' 은 도메인에 종속적이라기보다는 프로젝트 전반에서 사용해야 할 유틸리티로 판단했어요.

reviwer: common, util 등의 네이밍은 꽤나 위험합니다. 사람마다 이런 포괄적인 네이밍에 대한 생각이 조금씩 다르기 때문에, 또는 프로젝트 내부의 의존성 문제 때문에 이런 패키지들이 커지는 건 순식간이에요. 클래스나 패키지 등등 어떤 네이밍을 하든 구체적으로 이름을 짓는 습관을 가지면 좋을 것 같습니다.

존재하지 않는 요구사항에 맞춰 코드를 구현하기 보다는 현재 상황에 적절한 구현을 하면 좋을 것 같아요. 패키지를 추가하고 바꾸는 건 그리 어려운 일이 아닙니다. 공통적으로 사용하는 부분이 생기면 그 때 변경해도 될 것 같네요.

link: woowacourse/spring-roomescape-member#146 (comment)

[Kimprodp]

존재하지 않는다 -> Not Found로 404로 진행할 수 있을듯한데, 400으로 진행한 이유가 있을까요?

Not Found(404)는 찾는 자원(=URI)이 없을 때 응답하는 상태 코드라고 알고 있습니다. 위 경우는 URI가 아닌 DB의 데이터가 없는 경우라 클라이언트가 보낸 요청 파라미터가 잘못된 경우라고 판단했고, 그래서 Bad Request(400) 상태 코드를 응답하도록 설정하였습니다.

그렇게 알고 있었는데… 더 찾아 보니 404에서 말하는 자원이 꼭 경로만 의미하는 것이 아니고, DB의 자원을 의미한다고 하기도 하네요.😅 미르는 400, 404 코드를 각각 언제 사용하시는 편인가요?

자원이라는 내용은 광범휘하다고 생각해요 ㅎㅎ
저도 언제나 고민되는 내용이긴합니다.

400은 Client 요청 오류 (문법 오류)로 활용하고 있어요.
404는 URL 경로가 잘못된 경우 / DB 자원이 없는 경우로 활용하고 있습니다.

Http Status Code는 프론트와 협업을 진행할 때 중요하다고 생각해요.
프론트 개발자와 협의된다면 어떤 내용을 사용해도 괜찮을 듯 합니다.

가끔 422을 사용하는 팀도 있습니다.

woowacourse/spring-roomescape-member#104 (comment)

---

count / limit 1 등이 아닌 exists 함수를 사용한 이유가 있을까요?

id에 해당하는 row가 있는지 확인하는 로직을 작성하려고 하다 보니 자연스럽게 exists()를 떠올리게 되었는데요, 미르의 코멘트를 보고 exists(), count() 두 함수의 특징을 학습해 보았습니다.

exists(): 검색하려는 row가 존재할 경우 True를 반환하고 그렇지 않으면 False를 반환합니다. 조건에 일치하는 row를 찾으면 즉시 결과를 반환하기 때문에 특정 row의 존재 여부만 확인해야 할 경우 빠른 성능을 제공합니다.
count(): 특정 조건을 만족하는 row의 개수를 반환하는 메서드로, 전체 테이블을 스캔하는 방식으로 동작합니다. 따라서 데이터가 증가할수록 데이터베이스 성능이 저하됩니다.
두 함수의 특징을 따져 보니 exists를 사용하는 게 적절한 것 같아요. 만약 count를 사용해야 한다면, count(*) 대신 count(1)로 작성한다거나 미르가 말씀해 주신 대로 limit 1 조건을 추가하여 성능을 개선시킬 수 있을 것 같습니다.

woowacourse/spring-roomescape-member#17 (comment)

---

outer join과 inner join은 어떤 차이가 있을까요?

이미지 포함이라 URL로 대체 : woowacourse/spring-roomescape-member#17 (comment)

[J-I-H-O]

Reviewer :

(1) 전체가 동일하게 사용하는 테스트 데이터가 테스트 코드의 가독성을 저해하는 요소로 작용할 수 있으니 반드시 필요한 경우에만 사용하시는 것을 권장드립니다.

(2) 전체적으로 모든 테스트가 test.sql에서 insert된 데이터에 의존하는 것 같습니다.
테스트 코드에서 의존하는 데이터의 범위를 최소한으로 제한해주시면 좋을것 같습니다.

(3) 테스트 픽스쳐를 이용하는 것은 좋은것 같습니다.
제 의견을 말씀드리면 저는 지금의 사용법은 테스트의 가독성이 조금은 아쉬워지지 않나 싶습니다.
이미 말씀해주신것 처럼 id가 어디까지 발번되었는지 알고 코드를 작성해야하고 이러한 코드는 테스트 픽스쳐의 변경에 굉장히 취약한 구조를 갖게 됩니다.

저는 테스트 데이터는 최소한의 범위로 제공하는 것이 좋다고 생각하는 편입니다.
테스트 코드만 보고도 이 테스트의 상황에 대해 한눈에 파악할 수 있는 것이 좋기 때문입니다.
따라서 저는 여러가지 데이터를 매번 주입하고 사용하는 것보다 테스트에서 필요한 데이터만 최소한으로 사용할 수 있는 테스트 픽스쳐를 만들어 사용할 것 같습니다.

테스트 픽스쳐 세부사항에 영향을 받지 않도록 테스트를 개선해보시면 좋을것 같습니다.

woowacourse/spring-roomescape-member#13 (comment)
woowacourse/spring-roomescape-member#11 (comment)
woowacourse/spring-roomescape-member#22 (comment)

---

Reviewer :

하나의 단언문 lambda에 하나의 메서드(생성자 포함)만 수행되도록 해보시면 좋을것 같습니다.
만약 동일한 exception과 동일한 메세지를 발생시키는 두개의 메서드가 하나의 lambda 단언문에서 수행되면 거짓양성이 발생할 수 있습니다.

woowacourse/spring-roomescape-member#13 (comment)

---

Reviewer :

test에서 exception 타입만 검사하는 것으로는 안정적인 테스트가 힘들 수 있습니다.
거짓 양성, 거짓 음성에 대해 알아보시면 좋을것 같습니다.

exception만 검사했을때 검증하는 대상이 발생시키는 exception이 여러개인 경우, 그리고 여러개의 exception중 중복되는 exception이 있는 경우 정확히 검증하고자 하는 부분이 검증되었는지 확인이 어렵습니다.

woowacourse/spring-roomescape-member#31 (comment)
woowacourse/spring-roomescape-member#89 (comment)

---

Reviewer :
존재하는지에 대한 여부를 확인할때는 count 집계보다 limit 1 절을 이용하는 것이 더 효율적입니다.
count는 집계를 하기 위해 where절에 해당하는 모든 데이터를 검색해야하는 반면 limit은 1개의 집계결과가 탐색되면 거기서 탐색이 중지되기 때문입니다.

woowacourse/spring-roomescape-member#22 (comment)

---

테스트에서의 복잡한 의존성 설정

Reivewer :

의존성 설정을 하는 과정이 너무 힘들다면 의존성 관계를 간단하게 개선하여 애플리케이션 문제를 해결할 수 있는지 확인해보고 다음을 생각해보면 좋을것 같습니다.

위 고민이 끝난 상태에서 개선을 해야한다면 프레임워크가 의존성 관계에 따른 객체주입을 하여 운영 환경에서 제공을 하듯, 테스트 환경에서도 해당 행동을 대신해줄 수 있는 테스트 픽스쳐를 제공해보면 좋을것 같습니다.

woowacourse/spring-roomescape-member#31 (comment)

---

DDL 에서의 cascade 설정

Reviewer :

cascade를 활용하는 경우 reservation이 삭제되지 않아야하는데 삭제되는 경우 같이 삭제되면서 장애로 전파될 수 있습니다.

설령 reservation이 삭제되는 경우 reservation_list가 삭제되어야 하는 정책이 있다 하더라도, 그러한 정책을 인프라스트럭쳐 레이어에게 책임을 맡기는 것은 외부 구현 세부사항에 도메인이 의존하는 모양이 되는것 같습니다.

가능한 해당 설정은 지양하고 작성해보시면 좋을것 같습니다.

woowacourse/spring-roomescape-member#31 (comment)

---

JWT 검증 여부 및 방법

Reviewee :

jwt 구조에서 payload는 BASE64로 인코딩 되어 누구나 볼 수 있는 것으로 알고 있습니다. 클라이언트 측에서 토큰을 수정하거나 위조할 수 있기에, 페이로드의 내용을 검증함으로써 토큰이 변조되지 않았는지 확인할 수 있습니다.
해당 방식이 일반적인지 여쭤보고 싶습니다.

Reviewer :

일반적인지에 대한 답변은 조금 어려운것 같습니다.
팀마다 어떤 전략을 취하느냐에 따라 달라질 수 있는 부분으로 보이고, 애플리케이션을 운영하고 있는 시스템의 규모에 따라서도 달라질 수 있을것 같습니다.

아시다시피 Base64는 인코딩 기법이기 때문에 보안에 큰 이점은 없습니다. 따라서 payload를 서버에서 검증하는 것 또한 좋은 방법입니다.

저라면 특정 암호화 기법을 이용해 payload에 대한 암호화와 payload의 hash 값을 함께 검증하는 방식을 취할 것 같습니다. (제가 속한 팀은 이러한 방식으로 통신하는 경우가 많습니다.)

참고: https://curity.io/resources/learn/jwt-best-practices/

woowacourse/spring-roomescape-member#89 (comment)

[alstn113]

리뷰어: 응답은 Collection이 아닌 Object 형식으로 내려주는게 유연한 설계이다.

woowacourse/spring-roomescape-member#67 (comment)

---

리뷰어: IllegalArgumentException과 같은 표준 예외를 사용할 경우 의도치 않게 다른 라이브러리의 오류 메세지가 유저단까지 노출될 수 있다.
그렇기 때문에 커스텀예외를 활용하는 것이 좋다.

woowacourse/spring-roomescape-member#67 (comment)

[nayonsoso]

🏃‍♂️🦆

🔸 테스트 코드에서 @Transactional을 쓸 때 발생하는 문제

① 버그가 가려질 수 있다.
서비스에는 @Transacational을 붙이지 않고,
테스트 코드에만 @Transacational를 붙이는 상황에서 변경 메서드를 호출한다고 하자.
그럼 테스트는 통과하지만, 실제 운영 코드에서는 영속성 컨텍스트를 이용한 변경 감지가 되지 않아 flush가 되지 않아 데이터가 변경되지 않는다. 따라서 실제 운영 코드에서 문제가 일어나도 테스트 코드에서 이를 감지하지 못하는 상황이 발생할 수 있다.

② 인수테스트와 함께 사용 시 적용되지 않을 수 있다.
@Transacational 롤백과 RestAssured는 별도의 스레드를 사용해 하기 때문에 롤백이 되지 않는다.

🔸 중복을 외부로 분리해 읽기 편한 코드 vs 테스트 코드 안에 테스트와 관련된 모든 게 있는 코드

반복이 있더라도 가능하면 테스트 안에 있는 것이 읽기에 더 좋아요.
결국 테스트 안에 없으면 나중에 찾는 데 시간을 더 많이 쓰게 돼요.
편집 도구를 활용하면 반복되는 경우에도 쉽게 변경할 수 있기도 하구요.
물론 테스트가 점점 커져서 테스트 한번 작성하는데 지나치게 많은 boiler plate 코드들을 작성해야한다면 
Fixture를 관리하는 메서드를 따로 빼거나 할 수도 있겠죠.
하지만 지금 정도 수준에서는 다 작성하는 것이 훨씬 나아요.

🔸 모든 응답에 대해 dto를 만들어줘야 할까?

좋은 질문이네요.
정말 정석적으로 REST API 코드를 작성하자면 모든 응답 상황에 맞는 dto를 만들어줘야겠죠.
그런 부분이 REST API의 단점이고, 그래서 graphql같은 기술들이 나오게 된 거겠죠.
하지만 현실적으로 모든 API를 그렇게 각각의 목적에 fit하게 대응하기는 어려워요.
개발자의 리소스는 일반적으로 기계보다 훨씬 비싸니까요.
그래서 적당한 수준에서 잘 묶어 타협해야하죠.
'그 적당한 수준을 어떻게 결정할 것인가?' 를 고민하고 적절하게 구성하는 것이 무척 중요해요.
어떻게 묶었느냐에 따라 상황에 맞게 재사용 가능한 유연한 API가 되기도 하고,
그대로 제공해주기 애매해서 매번 새로 만들다가 어슷비슷한 n개가 무더기로 생겨버려서 관리하기 난감한 API 집합체가 될 수도 있죠.
결국 정답은 없고, 연관되어 함께 변경되는 것들이 무엇인지를 잘 고민해서 설계해야해요.

🔸 URI 를 어떻게 지어야 할까

함수나 변수의 이름을 짓는 것과 크게 다르지 않아요 :)
글쓰기나 다름없죠.
가져오려고 하는 대상이 무엇인지만 명확하게 알 수 있다면 뭐든 좋아요~

🔸엔티티에 equals 함수를 정의할 때 주의할 점

어떤 두 Theme가 '같다'는 것을 정의할 때 다양한 정의를 이야기해볼 수 있겠죠.
id가 같으면 같다고 할 수도 있겠고, 혹은 값까지 모두 같아야 같다고 이야기할 수도 있어요.
일반적인 value object와는 다르게 엔티티에 대해서는 equals에 대한 보편적인 합의를 이야기하기가 어렵다는 뜻이에요.
이렇게 equals 메서드를 구현해두면 어딘가에서 equals를 사용하였을 때 '같다는게 무슨 뜻이지?' 를 항상 체크해야해요. 
영속화된 객체에 '같은 객체다' 라고 볼 때에는 일반적으로 id를 비교하는데, 
이 메서드를 거치면 그게 아닐 수도 있으니 매번 헷갈리게 되는거죠.

모든 객체에 대해서 이렇게 무분별하게 equals를 구현해두는 것은 좋은 방법은 아닌것같아요. 
위와 같은 이유에서 entity에 대해서는 id를 꺼내 직접 비교하는 것을 권장해요. 
이렇게 equals 와 hashCode를 따로 구현해서 쓰는 것은 지양하는 것이 좋아요.

🔸 중복된 값을 저장하지 않기 위해 DB 에서 복합키를 갖게 하는 것에 대해

테이블에 복합키 조건이 생기는 것 자체가 테이블 관리를 상당히 피곤하게 만드는데
그 이유가 중복 검증을 위해서뿐이라면 약간 과한것같네요.
이러한 트레이드오프가 있는데도 코드 퀄리티가 비견될만큼 좋아진다고 보기도 어렵겠어요.
현업에서는 특별한 이유가 있는게 아니라면 복합키를 사용하지 않아요.

🔸 ArgumentResolver의 역할을 하는 커스텀 어노테이션을 만드는 것에 대해

리뷰어가 ‘개발자 입장에서 보다 명시적으로 의도를 드러내기 위해’ AuthenticationPrincipal이라는 어노테이션을 만든 상황…

단순히 마킹 용도로만 사용된다면 오히려 있어야하는지 없어야하는지 헷갈리게 될 수 있어요 :)
꼭 필요하게 만들어서 넣거나, 혹은 불필요하면 빼거나 둘 중 하나를 선택하는게 나을것같아요.

🔸 jwt와 관련된 부분이 서비스를 의존하게 하는 것이 맞는가?

리뷰어가 jwt 토큰을 생성하는 service 패키지 아래에 두고,
클래스 이름을 JwtService 라고 하고, MemberService 를 의존하게 한 상황…

지금 service 패키지는 예약과 관련된 다양한 비즈니스 로직들이 위치한 곳이에요.
인증과 관련된 내용은 비즈니스의 주된 관심사가 아니에요.
이 서비스가 다른 서비스들과 같은 패키지에 위치하는 것이 맞을지 좀더 고민해보시면 좋겠네요 :)

[jimi567]

리뷰이 :
@Valid 기능을 알게된 이후로 어떤 계층에서 검증을 하는 것이 옳은지? 에 대한 갈등이 생겼습니다.(Dto 에서하느냐, 도메인 객체에서 검증하느냐?)

Fail-Fast 원칙에 따르면 Dto 단의 검증이 유의하다고 생각합니다.
그러나, 책 <오브젝트>에 나온 정보전문가 패턴(가장 많은 정보를 지닌 객체에게 정보를 할당하라)에 따르면 객체에게 검증 책임을 이전하는 것이 옳다고 생각합니다.

저는 이번 프로젝트에서 DTO와 객체에게 모두 검증 책임을 주었습니다.
그 이유는
첫째, DTO를 통해 Request가 들어오지 않는 협력이 존재할 수 있기에 객체는 DTO와 독립적으로 스스로 검증책임을 캡슐화할 의무가 있음
둘째, fail-fast 원칙에 따라 Controller 단에서 유의성 검증을 하는 것이 소프트웨어 안전성을 높여줌

그러나, 전체 코드를 기준으로 보았을 때 중복 검증코드란 생각도 들었습니다.

저는 DTO와 도메인 객체는 재사용성으로 고려해 최대한 독립적으로 모듈화된 객체들로 바라보고 싶습니다.
따라서, 검증 책임이 중복되었다고 생각하지 않습니다.

리뷰어 :

저도 비슷한 생각입니다.

domain은 web / controller 를 알지 못하는 상황이니 validation을 가지는게 당연하고,

web / controller는 자명한 사실에 대해 validation할 수 있겠죠.

중복으로 볼수도 있겠으나 이야기한대로 중복이 아닐수도 있겠죠.

ref : woowacourse/spring-roomescape-member#79 (comment)

---

리뷰어 : id가 같다고 하면 같은 entity일순 있겠지만, entity가 update될 가능성이 있다고 가정하면 id가 같다고 해서("동일"하다고 해서) "동등" 하지는 않는것 같아요.(id를 통해 동등성을 비교하는 메서드에 대해서 isSameReservation(Long id)

ref : woowacourse/spring-roomescape-member#26 (comment)

---

리뷰어 : .http엔 {{someValue}}로 manipulation이 가능합니다

리뷰이 : 환경변수 사용해서 host 지정하도록 수정해주었습니다. 추가로, session값은 http client가 자동으로 전달해주기에 기존의 Cookie: ~ 라인도 전부 지워주었습니다.

ref : woowacourse/spring-roomescape-member#144 (comment)

---

리뷰어 : 만약 검색 traffic이 거대하다면 (그럼 jpa같은걸 쓰던지 es같은걸 쓰겠지만..) maximum preparedStatement 를 오버해서 쿼리수행이 안될수 있습니다.(여러 조건절을 추가하는 쿼리에 대해서)
전면장애를 맞고나서 namedParameter를 사용하는 등 대책이 필요하게 됩니다

알아볼 것 : maximum preparedStatement

---

리뷰어 :
@component가 가변상태를 가지면 어떻게 될까요?(CookieProvider라는 컴포넌트가 Cookie를 멤버변수로 갖는 상황)
cookie를 들고 있지 말고 createCookie에서 바로 리턴하게 하면 어떤가요?

리뷰이 :
싱글톤 스코프의 빈으로 등록된 CookieProvider의 cookie 값이 예기치 못하게 변경될 수 있습니다.

이와 관련해 스프링 docs에선 원칙적으론 stateful bean은 프로토타입 스코프를 사용하고, stateless bean은 싱글톤 스코프를 사용해야 한다고 명시하고 있습니다. 지금 제 코드는 stateful bean을 싱글톤 스코프로 사용하고 있습니다. 이게 틀린 건 아니지만 분명 조심해야 합니다.

ref : woowacourse/spring-roomescape-member#144 (comment)

---