[4기 박상민] Springboot-jpa weekly 미션 1차 PR입니다. #312

smartandhandsome · 2023-07-31T16:42:12Z

📌 과제 설명

단일 엔티티를 이용한 CRUD를 구현

seung-hun-h

리뷰 남겼습니다~

seung-hun-h · 2023-08-02T01:16:28Z

src/main/java/org/programmers/jpaweeklymission/customer/Customer.java

+import org.programmers.jpaweeklymission.global.BaseEntity;
+@Table(name = "customers")


Suggested change

import org.programmers.jpaweeklymission.global.BaseEntity;

@Table(name = "customers")

import org.programmers.jpaweeklymission.global.BaseEntity;

@Table(name = "customers")

seung-hun-h · 2023-08-02T01:17:46Z

src/main/java/org/programmers/jpaweeklymission/customer/Customer.java

+    private Long id;
+
+    @NotBlank
+    @Size(max = 20)


이렇게 되면 Empty String은 허용되는건가요?

Empty String 공백 허용하지 않는 걸로 정했습니다.

min 명시 해두었습니다.

Size가 있어서 NotBlank는 필요없는것 같아 제거했습니다.

d355a6b

seung-hun-h · 2023-08-02T01:19:04Z

src/main/java/org/programmers/jpaweeklymission/customer/presentation/CustomerController.java

+    }
+
+    @DeleteMapping("/{id}")
+    @ResponseStatus(HttpStatus.OK)


204 상태 코드에 대해서 알아보는 것도 좋겠네요~

같은 생각으로 해당 내용 작성했습니다.

prgrms-be-devcourse/springboot-board-jpa#225 (comment)

seung-hun-h · 2023-08-02T01:20:16Z

src/main/java/org/programmers/jpaweeklymission/global/GlobalExceptionHandler.java

+    @ExceptionHandler({HttpMessageNotReadableException.class, MethodArgumentNotValidException.class})
+    public ResponseEntity<Map<String, String>> handleHttpMessageNotReadableException(HttpMessageNotReadableException e) {
+        log.warn(e.getMessage(), e);
+        return ResponseEntity.badRequest().body(Collections.singletonMap(MESSAGE, e.getMessage()));


예외 메시지를 그대로 클라이언트에 전댈해도 될까요?

Map을 사용하지말고 에러 응답 객체를 정의해도 좋을 것 같아요

변경했습니다.

cc9e51f

혹시 저 핸들러에 어떤어떤 에러를 잡으시나요?

저는

제가 내부에서 발생시킨 에러

요청을 날렸을 때 발생할 만한 에러

이런 기준으로 작성했는데, 추가할 에러가 또 있을까요?

seung-hun-h · 2023-08-02T01:22:04Z

src/test/java/org/programmers/jpaweeklymission/customer/Infra/CustomerJpaRepositoryTest.java

+        tem.flush();
+        tem.clear();


필요한 이유가 있나요?

Db에 대한 Test 라고 생각했고

1차 캐시를 이용하면 안된다고 생각했습니다.

또, 쿼리가 제대로 나가는지 확인하기 위해서 작성했습니다.

src/main/java/org/programmers/jpaweeklymission/customer/Customer.java

src/main/java/org/programmers/jpaweeklymission/customer/application/CustomerMapper.java

dojinyou · 2023-08-02T19:40:40Z

src/main/java/org/programmers/jpaweeklymission/customer/Customer.java

+    public void changeEntity(Customer customer) {
+        changeFirstName(customer.firstName);
+        changeLastName(customer.lastName);
+    }


Customer를 새로 만들어서 update하는 형태가 적절한 지 잘 모르겠어요!
이렇게 생각하신 이유가 있을까요?

일단 Update 요청으로 들어오는 Dto를 Domain Layer까지 끌고 오는 건 아니라고 생각했습니다.

Service Layer와 Domain Layer 사이에 Dto를 또 두는 건 이미 Service가 Domain을 알고 있고 사용하기 떄문에 굳이? 라고 생각했습니다. (작성하고 나니 Domain과 Entity를 분리하는 방법도 있구나라는 생각이 드는데 어떻게 생각하시나요?)

그리고 보통 사용자를 수정할 떄, 모든 항목의 내용을 다 받는 다는 걸 여러 사이트에서 참고 했습니다.

프론트에서 어떻게 처리되는지는 잘 모르겠지만 각각 한 요소 요소 마다 변경하는 것 보다는 보통 모든 요소를 다 변경하는구나 라고 파악해서 Customer를 넣는 방식으로 선택했습니다.

근데 뭐가 좋고 보통 어떻게 처리하는지 잘 모르겠습니다.

멘토님들은 어떻게 처리하시나요??

요구사항에 따라서 다르겠지만 개인적으로는 업데이트 되는 필드와 업데이트 되지 않은(혹은 사용자가 업데이트할 수 없는) 필드가 있을 수 있다고 생각해요. 그래서 업데이트 되는 필드를 한번에 모두 요청을 받아서 업데이트를 한다면 저는 해당 데이터 관리용 DTO를 도메인까지 넣어줄 것 같아요. 해당 dto를 외부까지 쓸 수도 있고 아닐 수도 있지만 이 부분도 고민해볼 수 있겠네요.

정리하면

변경 가능한 필드에 대해서 도메인에게 변경 요청을 하는 DTO를 만드는 것이 좋을 수 있다.

이 도메인 DTO를 외부 레이어 어디까지 쓸 것인지는 고민이 필요하다

src/main/java/org/programmers/jpaweeklymission/customer/application/CustomerService.java

src/main/java/org/programmers/jpaweeklymission/customer/presentation/CustomerController.java

src/main/java/org/programmers/jpaweeklymission/global/BaseEntity.java

src/test/java/org/programmers/jpaweeklymission/customer/CustomerTest.java

…ice로 변경

seung-hun-h

고생하셨습니다~

seung-hun-h · 2023-08-04T00:03:23Z

src/main/java/org/programmers/jpaweeklymission/global/exception/GlobalExceptionHandler.java

+
+@Slf4j
+@RestControllerAdvice
+public class GlobalExceptionHandler {  // TODO: 혹시 멘토님들은 여기서 어떤 어떤 에러를 잡으시나요?


GlobalExceptionHanlder이니 지금 작성하신 것 처럼 글로벌하게 처리할 예외들을 잡을 것 같습니다.

seung-hun-h · 2023-08-04T00:14:53Z

...java/org/programmers/jpaweeklymission/customer/presentation/dto/CustomerCreationRequest.java

+import jakarta.validation.constraints.Size;
+
+public record CustomerCreationRequest(
+        @Size(min = 1, max = 20, message = "First name must be at least 1 character and no more than 20 characters.")


기본 메시지는 어떻게 출력되던가요?

dojinyou

수고하셨습니다

smartandhandsome added 18 commits July 26, 2023 23:42

feat: initial commit

8885c72

test: 사용하지 않는 테스트 제거

085c2cf

feat: BaseEntity 생성

ab3d743

feat: Customer 객체 생성

d950c03

feat: Customer repository 생성

9bec218

feat: Response DTO 생성

f65284c

feat: Request DTO 생성

0c2bf94

feat: CustomerMapper 생성

926e69e

feat: Customer Service 구현

56150de

feat: customer controller 구현

692a8ac

feat: ControllerAdvice 구현

433d9d9

chore: logback 설정 및 gitignore 설정 추가

031f0d8

feat: Column 어노테이션 추가

fcc103b

refactor: package 구성 변경

14311b4

feat: 접근지시자 private 변경 및 Customer 객체를 받아 Customer 수정하는 메서드 생성

b546f4f

feat: CustomerUpdateRequest 객체를 Customer로 변환하는 메서드 생성

d9c8df4

feat: 새로 만든 changeCustomer 메서드 사용

2e223cd

feat: 400 500 상태코드에 body 추가

2911d3d

smartandhandsome self-assigned this Jul 31, 2023

smartandhandsome requested review from seung-hun-h and dojinyou July 31, 2023 16:42

smartandhandsome added 🔥 매운맛 Team-Seunghoon labels Jul 31, 2023

smartandhandsome changed the base branch from main to smart-sangmin July 31, 2023 16:44

smartandhandsome added 2 commits August 1, 2023 04:23

test: Customer 테스트 작성

2debcb0

test: Customer Repository 작성

c78e31b

seung-hun-h requested changes Aug 2, 2023

View reviewed changes

dojinyou reviewed Aug 2, 2023

View reviewed changes

smartandhandsome added 2 commits August 3, 2023 14:53

feat: @SiZe에 min 명시

ee9712b

test: BeforeEach로 데이터 생성 후 1차 캐시 비움

9b88d24

smartandhandsome added 9 commits August 3, 2023 15:07

refactor: import 시 asterisk 제거하고 import 항목 명시

d461cc5

refactor: class와 import 사이 개행 추가

d24d2a1

feat: 추상클래스로 변경

683d2fd

test: 각 field를 반환하여 비교한 것을 usingRecursiveComparison으로 변경

014b6ac

feat: ErrorResponse 생성 및 GlobalExceptionHandler 적용

cc9e51f

refactor: import 시 asterisk 제거 및 질문 삭제

2bf745d

feat: ErrorResponse에 Getter 추가, ControllerAdvice -> RestControllerAdv…

26d5308

…ice로 변경

feat: message 추가

d355a6b

refactor: Error Message 변경, ExceptionHandler 분리, 질문 추가

5a8ba05

seung-hun-h approved these changes Aug 4, 2023

View reviewed changes

dojinyou approved these changes Aug 4, 2023

View reviewed changes

smartandhandsome merged commit 0cdc711 into prgrms-be-devcourse:smart-sangmin Aug 4, 2023

smartandhandsome mentioned this pull request Aug 5, 2023

[4기 박상민] Springboot-jpa weekly 미션 3 PR입니다. #334

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4기 박상민] Springboot-jpa weekly 미션 1차 PR입니다. #312

[4기 박상민] Springboot-jpa weekly 미션 1차 PR입니다. #312

smartandhandsome commented Jul 31, 2023

seung-hun-h left a comment

seung-hun-h Aug 2, 2023

seung-hun-h Aug 2, 2023

smartandhandsome Aug 3, 2023

smartandhandsome Aug 3, 2023

seung-hun-h Aug 2, 2023

smartandhandsome Aug 3, 2023

seung-hun-h Aug 2, 2023

smartandhandsome Aug 3, 2023

smartandhandsome Aug 3, 2023 •

edited

Loading

seung-hun-h Aug 2, 2023

smartandhandsome Aug 3, 2023 •

edited

Loading

dojinyou Aug 2, 2023

smartandhandsome Aug 3, 2023 •

edited

Loading

smartandhandsome Aug 3, 2023

dojinyou Aug 4, 2023

seung-hun-h left a comment

seung-hun-h Aug 4, 2023

seung-hun-h Aug 4, 2023

dojinyou left a comment

		import org.programmers.jpaweeklymission.global.BaseEntity;
		@Table(name = "customers")

[4기 박상민] Springboot-jpa weekly 미션 1차 PR입니다. #312

[4기 박상민] Springboot-jpa weekly 미션 1차 PR입니다. #312

Conversation

smartandhandsome commented Jul 31, 2023

📌 과제 설명

seung-hun-h left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smartandhandsome Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smartandhandsome Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smartandhandsome Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seung-hun-h left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dojinyou left a comment

Choose a reason for hiding this comment

smartandhandsome Aug 3, 2023 •

edited

Loading

smartandhandsome Aug 3, 2023 •

edited

Loading

smartandhandsome Aug 3, 2023 •

edited

Loading