Skip to content

[4기 황창현, 이현호] JPA Mission 1 - 단일 Customer 엔티티를 이용한 CRUD #269

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 12 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
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ repositories {
dependencies {
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
implementation 'org.springframework.boot:spring-boot-starter-web'
implementation 'org.springframework.boot:spring-boot-starter-validation'
compileOnly 'org.projectlombok:lombok'
runtimeOnly 'com.h2database:h2'
annotationProcessor 'org.projectlombok:lombok'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package com.programmers.springbootjpa.controller;

import com.programmers.springbootjpa.dto.request.CustomerCreateRequest;
import com.programmers.springbootjpa.dto.request.CustomerUpdateRequest;
import com.programmers.springbootjpa.dto.response.CustomerResponse;
import com.programmers.springbootjpa.service.CustomerService;
import jakarta.validation.Valid;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
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.RestController;


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

private final CustomerService customerService;

@PostMapping
public ResponseEntity<Void> createCustomer(@Valid @RequestBody CustomerCreateRequest customerCreateRequest) {
customerService.createCustomer(customerCreateRequest);

return ResponseEntity.status(HttpStatus.CREATED).body(null);
Copy link
Member

Choose a reason for hiding this comment

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

@PostMapping
@ResponseStatus(HttpStatus.CREATED)
public void createCustomer(@Valid @RequestBody CustomerCreateRequest customerCreateRequest) {
    customerService.createCustomer(customerCreateRequest);
}

이런 방식으로 ResponseStatus 애너테이션을 이용해서 상태코드를 반환하는 건 어떨까요?
그리고 어차피 RestController 애너테이션이 있기 때문에, ResponseBody 애너테이션이 각 메서드마다 적용된 상태라서,
ResponseEntity 객체말고 커스텀 객체나 void로 반환하면 null 값을 사용하지 않아서 더 좋을 것 같아요!
이후 메서드 들도 마찬가지입니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

확실히 큰 의미가 없다면 ResponseEntity를 이용하는 것보다 실제 객체를 반환하고 @ResponseStatus를 이용하는게 좋아보이네요 ㅎㅎ 감사합니다! 👍👍 수정했어요!

적용 커밋: 358544f

}

@GetMapping("/{id}")
public ResponseEntity<CustomerResponse> readCustomer(@PathVariable Long id) {
Copy link
Member

Choose a reason for hiding this comment

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

readCustomerById 와 같이 id를 통해 customer를 조회한다는 이름으로 명시하면 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

말씀하신대로 어색한 느낌이어서 메소드명 수정해두었습니다!

적용 커밋: a1af086

CustomerResponse customerResponse = customerService.readCustomer(id);

return ResponseEntity.ok(customerResponse);
}

@GetMapping
public ResponseEntity<List<CustomerResponse>> readAllCustomer() {
List<CustomerResponse> customerResponses = customerService.readAllCustomer();

return ResponseEntity.ok(customerResponses);
}

@PatchMapping
Copy link
Member

Choose a reason for hiding this comment

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

CustomerUpdateRequest DTO에 어떤 customer를 수정할 것인지에 대한 ID를 받는 것 같더라구요.
그런데 REST API는 http method를 이용해 행위를 규정하고, uri를 이용해 자원을 나타내는 것을 지향한다고 알고 있습니다.
(관련 참고 : https://meetup.nhncloud.com/posts/92)
하지만 현재 update 메서드에서는 PATCH /api/customers 이므로 어떤 자원을 수정한다는 것이 명시되어 있지 않은 것 같습니다.
이것과 관련해서 PATCH /api/customers/{id} 이렇게 하면 REST에서 지향하는 바를 더 지킬 수 있지 않을까 하는데 어떠신가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

사실 이 부분에 대해서 굉장히 고민을 많이했었는데요. 피드백을 받기전에는 어차피 Post로 받을텐데 포함시켜서 받아도되지않을까? 라는 생각을 했던 것 같아요. 말씀하신대로 REST API를 따르려면 uri를 이용해 자원을 나타내는 것이 맞는 것 같아요! 수정해서 반영했습니다!

적용 커밋 : 737fa92

public ResponseEntity<Void> updateCustomer(@Valid @RequestBody CustomerUpdateRequest customerUpdateRequest) {
customerService.updateCustomer(customerUpdateRequest);

return ResponseEntity.ok(null);
}

@DeleteMapping("/{id}")
public ResponseEntity<Void> deleteCustomer(@PathVariable Long id) {
customerService.deleteCustomer(id);

return ResponseEntity.noContent().build();
}
}
26 changes: 26 additions & 0 deletions src/main/java/com/programmers/springbootjpa/domain/Address.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.programmers.springbootjpa.domain;

import jakarta.persistence.Column;
import jakarta.persistence.Embeddable;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Embeddable
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class Address {

@Column(name = "street_address", nullable = false)
private String streetAddress;
Copy link
Member

Choose a reason for hiding this comment

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

Column 애너테이션과 필드명 사이에 한줄 씩 띄어주시면 더 가독성에 좋을 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

말씀하신대로 가독성 측면에서 좋지 못한 것 같아서 전부 수정했습니다!

적용 커밋: f5c3dc7

@Column(name = "detailed_address", nullable = false)
private String detailedAddress;
Copy link
Member

Choose a reason for hiding this comment

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

해당 필드명은 AddressDetail이 더 괜찮을 것 같습니다! @colunm(name = "address_detail, nullable = false) 이렇게 변경할 수 있을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

말씀하신 필드명도 카멜케이스를 따를 수 있지만 네이밍시 조금 더 자연스러운 영어구조는 detailedAddress가 자연스러운 것 같습니다! 영어에서 일반적으로 형용사는 명사 앞에 위치해서 명사 + 형용사(addressDetail)보다 형용사 + 명사 형태의 구조인 detailedAddress가 더 어울리지 않을까요?

@Column(name = "zip_code", nullable = false)
private Integer zipCode;

public Address(String streetAddress, String detailedAddress, Integer zipCode) {
this.streetAddress = streetAddress;
this.detailedAddress = detailedAddress;
this.zipCode = zipCode;
}
}
50 changes: 50 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,50 @@
package com.programmers.springbootjpa.domain;

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

@Entity
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class Customer {

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

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

@Column(name = "age", nullable = false)
private Integer age;

@Column(name = "nick_name", nullable = false)
private String nickName;

@Embedded
private Address address;

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

public void changeNickName(String nickName) {
this.nickName = nickName;
}

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

import com.programmers.springbootjpa.domain.Address;
import com.programmers.springbootjpa.domain.Customer;
import jakarta.validation.constraints.Max;
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class CustomerCreateRequest {

@NotBlank(message = "이름은 공백이거나 값이 없으면 안됩니다.")
private String name;
Copy link
Member

Choose a reason for hiding this comment

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

여기도 한 칸 공백 줄을 두면 더 가독성에 좋을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

말씀하신대로 가독성 측면에서 좋지 못한 것 같아서 전부 수정했습니다!

적용 커밋: f5c3dc7

@Min(value = 1, message = "나이는 한살보다 많아야합니다.")
@Max(value = 100, message = "나이는 백살보다 적여야합니다.")
Copy link
Member

Choose a reason for hiding this comment

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

오타인 것 같습니다!
"적여야합니다." => "적어야합니다."

Copy link
Member Author

Choose a reason for hiding this comment

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

감사합니다! 적용 했습니다!

적용 커밋: 39c94fe

Copy link
Member

@jay-so jay-so Aug 2, 2023

Choose a reason for hiding this comment

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

@Min, @Max 어노테이션으로 범위를 지정해도 좋지만 @Size어노테이션으로 최소값, 최댓값에 대한 범위를 하는게 더 나아보입니다! @Size(min = 1, max = 100) 으로 바꿀 수 있을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

@Size 어노테이션은 문자열의 길이에 대한 제한으로 Integer에 대한 값의 범위를 지정할 수는 없는 것으로 알고있습니다!

@Max와 @Size의 차이

@NotNull(message = "나이는 값이 없으면 안됩니다.")
private Integer age;
@NotBlank(message = "닉네임은 공백이거나 값이 없으면 안됩니다.")
private String nickName;
@NotNull(message = "주소값은 없을 수 없습니다.")
private Address address;

public Customer of() {
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,22 @@
package com.programmers.springbootjpa.dto.request;

import com.programmers.springbootjpa.domain.Address;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.PositiveOrZero;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class CustomerUpdateRequest {

@PositiveOrZero(message = "id값은 0이상 이어야합니다.")
Copy link
Member

Choose a reason for hiding this comment

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

@PositiveOrZero 어노테이션은 null 값일 때 검증을 통과시킵니다!! @NotNull을 붙여주어서 null 값일때도 고려해주세요!!

Copy link
Member Author

Choose a reason for hiding this comment

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

REST API 형식에 맞지 않는 필드였어서 삭제처리했습니다~!

private Long id;
@NotBlank(message = "닉네임은 공백이거나 값이 없으면 안됩니다.")
private String nickName;
@NotNull(message = "주소값은 없을 수 없습니다.")
private Address address;

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package com.programmers.springbootjpa.dto.response;

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

@Getter
public class CustomerResponse {

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

public CustomerResponse(Customer customer) {
Copy link
Member

Choose a reason for hiding this comment

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

request DTO에서는 정적 팩토리 메서드를 통해 DTO 객체를 생성하고 있는데, response DTO에서는 그냥 생성자를 사용하신 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

현재 RequestDto에서는 DTO객체를 생성하지않고 도메인을 생성하고 있습니다! DTO에서 도메인을 생성해주는거와 도메인에서 DTO를 생성해주는 것이 다른 결이라고 생각해서 그렇게 했던 것 같아요! 이 부분은 다음 과제 때 적용해서 반영해보도록 하겠습니다!!! 아무래도 하나로 통일되게 작성하는 것이 옳다고 생각이 드네요!

this.id = customer.getId();
this.name = customer.getName();
this.age = customer.getAge();
this.nickName = customer.getNickName();
this.address = customer.getAddress();
}
}
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,64 @@
package com.programmers.springbootjpa.service;

import com.programmers.springbootjpa.domain.Customer;
import com.programmers.springbootjpa.dto.request.CustomerCreateRequest;
import com.programmers.springbootjpa.dto.request.CustomerUpdateRequest;
import com.programmers.springbootjpa.dto.response.CustomerResponse;
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
@Transactional(readOnly = true)
public class CustomerService {

private final CustomerRepository customerRepository;

@Transactional
public void createCustomer(CustomerCreateRequest customerCreateRequest) {
Customer customer = Customer.builder()
.name(customerCreateRequest.getName())
.age(customerCreateRequest.getAge())
.nickName(customerCreateRequest.getNickName())
.address(customerCreateRequest.getAddress())
.build();

customerRepository.save(customer);
}

public List<CustomerResponse> readAllCustomer() {
List<Customer> customers = customerRepository.findAll();

Copy link
Member

@jay-so jay-so Aug 2, 2023

Choose a reason for hiding this comment

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

해당 부분에서 바로 return문을 이용해서 다음과 같이 사용할 수 있을 것 같아요!

return customerRepository.findAll().stream(). ~~

Copy link
Member Author

Choose a reason for hiding this comment

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

저는 customerRespository.findAll()은 하나의 행위이기 때문에 stream과 이어서 붙여쓰면 오히려 가독성이 더 안좋다라고 느껴지는 것 같아요. customers.stream()을 쓰면 말씀하신 내용보다 명확하게 어떤 것에 대해서 stream()을 사용하는건지 알 수 있지 않을까요? 재훈님은 이 부분에 대해서 어떻게 생각하시나요?

return customers.stream()
.map(CustomerResponse::new)
.toList();
}

public CustomerResponse readCustomer(Long id) {
Copy link
Member

Choose a reason for hiding this comment

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

readCustomre 대신 readById로 바꿀 수 있을 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

말씀하신대로 Customer는 클래스명에 포함되기 때문에 삭제처리했습니다~!

적용 커밋: c64bb48

Customer customer = customerRepository.findById(id)
.orElseThrow(() -> new NoSuchElementException("찾는 사용자가 없습니다."));

return new CustomerResponse(customer);
}

@Transactional
public void updateCustomer(CustomerUpdateRequest customerUpdateRequest) {
Customer customer = customerRepository.findById(customerUpdateRequest.getId())
.orElseThrow(() -> new NoSuchElementException("업데이트 할 사용자가 없습니다."));

customer.changeNickName(customerUpdateRequest.getNickName());
customer.changeAddress(customerUpdateRequest.getAddress());
}

@Transactional
public void deleteCustomer(Long id) {
customerRepository.findById(id)
.orElseThrow(() -> new NoSuchElementException("삭제할 사용자가 없습니다."));

customerRepository.deleteById(id);
Copy link
Member

Choose a reason for hiding this comment

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

deleteCustomer 메소드에서 findById와 deleteById 2개의 쿼리가 발생됩니다! 여기서 다음과 같이 바꿀 수 있을 것 같아요!

Customer customer = customerRepository.findById(id).orElseThrow ~~~
customerRepository.delete(customer);

Copy link
Member Author

Choose a reason for hiding this comment

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

재훈님께서 말씀하신 내용이 어떤 의미인지 파악을 잘 못하겠습니다! 재훈님께서 작성해주신 코드도 쿼리는 2개의 쿼리가 나가지 않나요?(select와 delete)

우선 저희가 짠 로직은 삭제 전 customer에 대한 조회를 하여 있는 유저인지 파악하는 용도로 findById를 사용했기 때문에 굳이 Customer를 받을 필요가 없어서 위와 같은 로직으로 작성해둔 상태입니다!

}
}
1 change: 0 additions & 1 deletion src/main/resources/application.properties

This file was deleted.

21 changes: 21 additions & 0 deletions src/main/resources/application.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
spring:
datasource:
driver-class-name: org.h2.Driver
username: sa
password:
jpa:
hibernate:
ddl-auto: create-drop
database-platform: org.hibernate.dialect.H2Dialect
properties:
hibernate:
format_sql: true

logging:
level:
org:
hibernate:
SQL: debug
type:
descriptor:
sql: trace
Loading