Skip to content

[4기 김영주, 소재훈] JPA Mission 1 - 단일 Customer 엔티티를 이용한 CRUD #267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: 영주,재훈-mission
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package com.programmers.springbootjpa.controller;

import com.programmers.springbootjpa.dto.CustomerCreateRequest;
import com.programmers.springbootjpa.dto.CustomerResponse;
import com.programmers.springbootjpa.dto.CustomerUpdateRequest;
import com.programmers.springbootjpa.service.CustomerService;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequestMapping("/api/v1/customer")
@RequiredArgsConstructor
public class CustomerController {

private final CustomerService customerService;

@PostMapping
@ResponseStatus(HttpStatus.CREATED)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResponseEntity를 사용하지 않고 Status 설정으로 처리하는 것도 보기 좋네요 👍👍

public CustomerResponse createCustomer(@RequestBody CustomerCreateRequest request) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CustomerResponse대신 유저 id만을 반환하거나, 아무 것도 반환하지 않는 방식에 대해서는 어떻게 생각하시나요? 저는 구체적인 요구사항이 주어지지 않은 상황에서는 최소한의 정보만을 반환하고 있기에, CustomerResponse를 반환하신 이유가 궁금합니다.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀하신대로 구체적인 요구사항이 주어지지 않았기 때문에, 굳이 고객 정보 전체를 반환할 이유가 없다는 것에 동감합니다.

어떤 서비스에서는 고객 정보를 생성하고, 해당 고객의 프로필과 같은 고객 정보를 볼 수 있는 페이지로 바로 이동하는 경우도 있을 것 같은데요. 그런 경우에는 생성과 동시에 해당 고객 정보를 받아와 화면에 출력할 수도 있겠습니다. 만약 createCustomer가 void를 반환한다면 고객 프로필 화면을 띄워주기 위해 findCustomerById 요청을 다시 보내야 할 것입니다. 물론 클라이언트 단에서도 고객 정보를 가지고 있기 때문에 굳이 다시 조회를 하지 않아도 될 거라 생각할 수도 있지만, 백엔드의 어떤 비즈니스 로직에 의해 프론트에서는 가지고 있지 않는 정보가 DB에는 들어있을 수도 있을겁니다. (기간을 넣었는데 날짜로 변환해서 DB에 넣는다거나 등등)

결국 요청을 두 번 보내지 않고, create와 동시에 고객 정보를 반환해준다면 해당 서비스에서만큼은 이 방식이 더 효율적일 수 있겠습니다. 말씀해주신대로 요구사항이 구체적이지 않았기 때문에, 저희끼리 자유롭게 상황을 설정해서 반환했다고 보시면 좋을 것 같습니다! 감사합니다.

return customerService.createCustomer(request);
}

@GetMapping
public List<CustomerResponse> findAllCustomers() {
return customerService.findAllCustomers();
}

@PatchMapping("/{id}")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

엔티티 정보 중 일부만 수정하고 있기에 PatchMapping 해주신 점 좋네요!

public CustomerResponse updateCustomerById(@PathVariable Long id,
@RequestBody CustomerUpdateRequest request) {
return customerService.updateCustomerById(id, request);
}

@DeleteMapping("/{id}")
@ResponseStatus(HttpStatus.NO_CONTENT)
public void deleteCustomerById(@PathVariable Long id) {
customerService.deleteCustomerById(id);
}
}
54 changes: 54 additions & 0 deletions src/main/java/com/programmers/springbootjpa/domain/Customer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package com.programmers.springbootjpa.domain;

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Entity
@Table(name = "customer")
@Getter
@NoArgsConstructor
public class Customer {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Column(name = "name", nullable = false, length = 30)
private String name;

@Column(name = "age")
private Integer age;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

age를 Primitive 타입이 아닌 Wrapper 클래스를 사용한 이유가 궁금합니다!

물론 @Id로 선언한 id의 경우 long으로 했을 때 값이 0이 있을 수 있어 실제 값이 없는건지 아니면 키가 0으로 설정되었는지 알 수 없어서 Long으로 선언하기도하지만 다른 필드의 경우 이러한 문제랑 상관이 없다고 생각이 들어서요!

그리고 int의 경우 4byte를 사용하지만 Integer의 경우 클래스 포인터, 플래그, 락, int 값까지 총 16byte가 사용된다고 합니다!
유저의 수가 증가하면 증가할 수록 부담이 될 수도 있지 않을까? 라는 생각이 듭니다!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 여러 이유가 있을 것 같은데요. 일단 Id가 Long 이기 때문에 Wrapper 클래스로 모두 통일하기 위함도 있었습니다.
  • 추가적으로 age 필드의 경우 nullable이 true로 되어 있어서 null이 될 수 있는데요. (나이가 입력되지 않은 경우)
  • 따라서 int보다는 Integer가 더 적합할 것이라 생각했습니다!


@Column(name = "nick_name", nullable = false, length = 30, unique = true)
private String nickName;

@Column(name = "address", nullable = false, length = 100)
private String address;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주소에는 '도로명 주소', '지번 주소', '우편 번호' 등이 정보가 포함될 수 있는데요. @Embedded, @embeddable 어노테이션을 경험해보기 위해 Address 객체를 따로 분리하여 3가지 필드를 포함시켜보는 것은 어떨까요? 참고

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 정말 좋은 기능이네요! 감사합니다. 덕분에 몰랐던 것을 알게된 계기가 된 것 같습니다. 다만 이번 미션에서는 최소 요구사항의 만족을 통한 기능 연습에 초점을 두고 있어서, 이번에는 따로 학습만 진행하고 실제 적용은 다음 기회에 한번 해보도록 하겠습니다!


@Builder
public Customer(String name, Integer age, String nickName, String address) {
this.name = name;
this.age = age;
this.nickName = nickName;
this.address = address;
}

public void updateName(String name) {
this.name = name;
}

public void updateAge(Integer age) {
this.age = age;
}

public void updateAddress(String address) {
this.address = address;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.programmers.springbootjpa.dto;

import com.programmers.springbootjpa.domain.Customer;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor
@AllArgsConstructor
public class CustomerCreateRequest {

private String name;
private Integer age;
private String nickName;
private String address;

public Customer toEntity() {
return Customer.builder()
.name(name)
.age(age)
.nickName(nickName)
.address(address)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.programmers.springbootjpa.dto;

import com.programmers.springbootjpa.domain.Customer;
import lombok.AllArgsConstructor;
import lombok.Getter;

@AllArgsConstructor
@Getter
public class CustomerResponse {

private Long id;
private String name;
private Integer age;
private String nickName;
private String address;

public static CustomerResponse fromEntity(Customer customer) {
return new CustomerResponse(
customer.getId(),
customer.getName(),
customer.getAge(),
customer.getNickName(),
customer.getAddress()
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.programmers.springbootjpa.dto;

import lombok.Getter;
import lombok.NoArgsConstructor;

@NoArgsConstructor
@Getter
public class CustomerUpdateRequest {

private String name;
private Integer age;
private String address;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.programmers.springbootjpa.repository;

import com.programmers.springbootjpa.domain.Customer;
import org.springframework.data.jpa.repository.JpaRepository;

public interface CustomerRepository extends JpaRepository<Customer, Long> {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package com.programmers.springbootjpa.service;

import com.programmers.springbootjpa.domain.Customer;
import com.programmers.springbootjpa.dto.CustomerCreateRequest;
import com.programmers.springbootjpa.dto.CustomerResponse;
import com.programmers.springbootjpa.dto.CustomerUpdateRequest;
import com.programmers.springbootjpa.repository.CustomerRepository;
import java.util.List;
import java.util.NoSuchElementException;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
public class CustomerService {

private final CustomerRepository customerRepository;

@Transactional
public CustomerResponse createCustomer(CustomerCreateRequest request) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Service레이어의 모든 기능 메서드에 'Customer' 라는 단어가 포함되고 있는데요. 정형화 된 CRUD 메서드만 존재하는 상황이고, 클래스명에 포함 된 단어이니 생략하는건 어떨까 싶습니다. 예를 들어 CustomerService.createCustomer() -> CustomerService.create()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 사실 해당 부분에 대해서는 많은 고민을 했었는데, 비슷한 고민을 하신 것 같아서 너무 재밌는 것 같습니다.
일단 저는 create 보다는 createCustomer가 더 낫지 않을까라는 의견을 가지고 있어요.

여러 상황을 생각해 볼 수 있는데, 만약 도메인이 조금 복잡해서 customer 말고 어떤 다른 것도 생성하는 기능도 추가된다면 어떨까요? 그렇다면 customerService.create()만으로는 정확히 고객을 생성하는 것인지, 아니면 고객과 관련된 다른 걸 생성하는 것인지 의미가 명확하지 않게 된다고 생각해요.

그리고 만약 의존성을 주입 받을 때, 권장되진 않지만 private CustomerService service;와 같은 형식으로 추가했다면 어떨까요?
그러면 service.create() 는 어떤 행위를 하는지 이것만 봐서는 절대 모를 것 같습니다. service.createCustomer() 는 그래도 대충 고객을 생성하는 기능이구나라는 걸 알 수 있지 않을까요? 물론 가장 중심적인 원인은 service 라고 모호한 변수명을 지었다는 것이지만, 이런 말도 안되는 상황까지 생각해봤을 때 createCustomer() 라고 메서드명을 짓는 것 자체가 다양한 상황에 대비할 수 있게 해준다고 생각합니다.

물론 현재 미션에서는 이런 상황까지 고려할 필요가 없을 정도로 단순하긴 해서 create()로 바꿔도 문제는 없을 것 같네요!

Customer savedCustomer = customerRepository.save(request.toEntity());

return CustomerResponse.fromEntity(savedCustomer);
}

public List<CustomerResponse> findAllCustomers() {
return customerRepository.findAll()
.stream()
.map(CustomerResponse::fromEntity)
.toList();
}

@Transactional
public CustomerResponse updateCustomerById(Long id, CustomerUpdateRequest request) {
Customer customer = customerRepository.findById(id)
.orElseThrow(() -> new NoSuchElementException("해당 ID에 해당하는 구매자가 없습니다."));

customer.updateName(request.getName());
customer.updateAge(request.getAge());
customer.updateAddress(request.getAddress());

return CustomerResponse.fromEntity(customer);
}

@Transactional
public void deleteCustomerById(Long id) {
if (!customerRepository.existsById(id)) {
throw new NoSuchElementException("해당 ID에 해당하는 구매자가 없습니다.");
}

customerRepository.deleteById(id);
}
}
1 change: 0 additions & 1 deletion src/main/resources/application.properties

This file was deleted.

10 changes: 10 additions & 0 deletions src/main/resources/application.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
spring:
datasource:
driver-class-name: org.h2.Driver
username: sa
password:
jpa:
hibernate:
ddl-auto: create-drop
database-platform: org.hibernate.dialect.H2Dialect
show-sql: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.programmers.springbootjpa.domain;

import static org.assertj.core.api.Assertions.assertThat;

import com.programmers.springbootjpa.dto.CustomerCreateRequest;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

class CustomerTest {

@Test
@DisplayName("Customer 생성 Dto에서 Customer Entity로 정상적으로 변환되는지 테스트한다.")
void customerDtoToEntityTest() {
CustomerCreateRequest request = new CustomerCreateRequest(
"Kim Jae Won",
28,
"hanjo",
"서울시 마포구"
);

Customer actualEntity = request.toEntity();

Customer expectedEntity = Customer.builder()
.name("Kim Jae Won")
.age(28)
.nickName("hanjo")
.address("서울시 마포구")
.build();

assertThat(actualEntity).usingRecursiveComparison().isEqualTo(expectedEntity);
}
}