Skip to content
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

feat: YSL-24 오늘의 다짐 CRUD 기능 추가 #16

Open
wants to merge 27 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
25f5485
feat: 오늘의 다짐 추가
jucheolkang Jan 10, 2024
7175a0e
refactor: 저장 기능 Dto 리팩토링
jucheolkang Jan 11, 2024
9cae178
refactor: 저장 기능 관련 리팩토링
jucheolkang Jan 11, 2024
b1f30d4
refactor: 저장 기능 관련 리팩토링
jucheolkang Jan 11, 2024
94f8fdd
feat: 오늘의 다짐 수정기능 추가
jucheolkang Jan 11, 2024
eebce38
feat: 오늘의 다짐 수행 여부 체크 기능 추가
jucheolkang Jan 11, 2024
cae20b6
feat: 오늘의 다짐 삭제 기능 추가
jucheolkang Jan 11, 2024
e134df8
refactor: Controller의 todoSave 수정
jucheolkang Jan 12, 2024
9b8ffde
feat: todoController 테스트 코드 추가
jucheolkang Jan 12, 2024
af6c019
refactor: todoController 테스트 코드 리팩토링
jucheolkang Jan 12, 2024
d42981e
refactor: 클래스명 시작 대문자로 수정
jucheolkang Jan 14, 2024
91fde0b
refactor: BusinessExceptionHandler클래스 이름 변경
jucheolkang Jan 14, 2024
3694065
refactor: TodoService클래스 리팩토링
jucheolkang Jan 14, 2024
ebcd5ac
refactor: TodoService클래스 리팩토링
jucheolkang Jan 14, 2024
1d65e19
refactor: todo 엔티티 리팩토링
jucheolkang Jan 14, 2024
0eeb7a9
refactor: TodoIdHandler 리팩토링
jucheolkang Jan 14, 2024
77093d7
refactor: yml 파일 리팩토링
jucheolkang Jan 16, 2024
367ce99
refactor: TodoControllerTest 리팩토링
jucheolkang Jan 16, 2024
649c403
refactor: yml 파일 리팩토링
jucheolkang Jan 16, 2024
b2dfa7d
refactor: rebase 문제 해결
jucheolkang Jan 16, 2024
05f8817
refactor: YesSlaveApplication에서 @EnableJpaAuditing 제거
jucheolkang Jan 20, 2024
6a29a5c
refactor: domain 이름 변경(todo -> Todo)
jucheolkang Jan 20, 2024
7f80143
refactor: TodoService 리팩토링
jucheolkang Jan 20, 2024
47e3cd0
refactor: Todo comment 수정
jucheolkang Jan 20, 2024
45fb635
refactor: Todo 와일드 카드 제거
jucheolkang Jan 20, 2024
f32b746
refactor: Todo에서 delete 메소드 내부 동작으로 변경
jucheolkang Jan 25, 2024
0127768
fix: TodoController에서 changeCheck메소드 boolean값 반환
jucheolkang Jan 26, 2024
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
2 changes: 2 additions & 0 deletions src/main/java/org/idiot/yesslave/YesSlaveApplication.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.data.jpa.repository.config.EnableJpaAuditing;

@EnableJpaAuditing
Copy link
Member

Choose a reason for hiding this comment

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

요거 JpaConfig에 작성 되어 있던 내용인데 앞으로 꺼내오신 이유가 있으실까요?

Copy link
Author

Choose a reason for hiding this comment

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

Auditing 사용하면서 오류가 발생해서 앞으로 꺼냈는데 다시 제거하고 확인해 보니까 정상적으로 동작하네요
코드를 작성하면서 뭔가 실수가 있었나 봐요. 지금은 지웠습니다

@SpringBootApplication
public class YesSlaveApplication {

Expand Down
15 changes: 15 additions & 0 deletions src/main/java/org/idiot/yesslave/global/exception/ErrorCode.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.idiot.yesslave.global.exception;

import lombok.Getter;

public enum ErrorCode {
ID_NOT_FOUND("해당 ID를 찾을 수 없습니다."),
ID_DELETE("삭제된 ID입니다.");

@Getter
private String message;

ErrorCode(String message) {
this.message = message;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.idiot.yesslave.global.exception;

import lombok.Getter;

public class TodoIdHandler extends RuntimeException {

@Getter
private final ErrorCode errorCode;

public TodoIdHandler(ErrorCode errorCode) {
super(errorCode.getMessage());
this.errorCode = errorCode;
}
}
59 changes: 59 additions & 0 deletions src/main/java/org/idiot/yesslave/todo/application/TodoService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package org.idiot.yesslave.todo.application;

import lombok.RequiredArgsConstructor;
import org.idiot.yesslave.global.exception.TodoIdHandler;
import org.idiot.yesslave.global.exception.ErrorCode;
import org.idiot.yesslave.todo.domain.todo;
import org.idiot.yesslave.todo.dto.SaveDto;
import org.idiot.yesslave.todo.dto.UpdateDto;
import org.idiot.yesslave.todo.repository.TodoRepository;
import org.springframework.stereotype.Service;

import javax.transaction.Transactional;

@Service
@RequiredArgsConstructor
public class TodoService {
private final TodoRepository todoRepository ;
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
Author

Choose a reason for hiding this comment

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

공백 제거했습니다
필요 없는 공백이 많네요. 코드를 다시 확인하는 습관을 기르겠습니다


@Transactional
public void save(SaveDto saveDto) {
todoRepository.save(todo.builder()
.todo(saveDto.getTodo())
.build());
}

@Transactional
public void update(Long id, UpdateDto updateDto) {
todo todo = existId(id);
checkDeleteStatus(todo);
todo.update(updateDto.getTodo());
}

@Transactional
public void changeCheck(Long id) {
todo todo = existId(id);
checkDeleteStatus(todo);
todo.changeCheck(!todo.isTodoCheck());
}

@Transactional
public void delete(Long id) {
todo todo = existId(id);
checkDeleteStatus(todo);
todo.delete(true);
}

private void checkDeleteStatus(todo todo) {
if (todo.isDelete()) {
throw new TodoIdHandler(ErrorCode.ID_DELETE);
}
}

public todo existId(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.

@transactional에 readOnly 라는 옵션이 있습니다.
요거 한 번 그냥 @transactional이랑 readOnly = true 주는거랑 무슨 차이가 있는지 한 번 봐보시면 좋을 것 같아요

todo todo = todoRepository.findById(id)
.orElseThrow(() -> new TodoIdHandler(ErrorCode.ID_NOT_FOUND));
return todo;
}

}
52 changes: 52 additions & 0 deletions src/main/java/org/idiot/yesslave/todo/domain/todo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package org.idiot.yesslave.todo.domain;

import lombok.*;
import org.hibernate.annotations.Comment;
import org.idiot.yesslave.global.jpa.AuditInformation;

import javax.persistence.*;
Copy link
Member

Choose a reason for hiding this comment

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

와일드 카드가 사용이 되었는데, 한 번 아래 참고글 봐보시면 좋을 것 같아용

https://blog.naver.com/d_d_o_l_/223232661158

Copy link
Author

Choose a reason for hiding this comment

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

와일드카드가 컴파일 속도를 느리게 한다는 것은 알고 있었는데 인텔리제이 설정에서 바꿀 수 있는 건 처음 알았습니다.
지금은 수정했습니다



@Entity
@Getter
@Table(name = "TODO")
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class todo extends AuditInformation {
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
Author

Choose a reason for hiding this comment

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

수정했습니다 😊


@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
@Column(name = "ID", nullable = false)
@Comment("id는 todo 테이블의 pk 값 입니다")
private Long id;

@Column(name = "TODO", nullable = false)
Copy link
Member

Choose a reason for hiding this comment

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

해당 칼럼에 Comment가 있다면 더 좋을 것 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

Comment 추가했습니다

Copy link
Member

Choose a reason for hiding this comment

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

훨씬 더 이해하기 좋은 코드가 된 것 같아요 👍🏻 감사합니다!

@Comment("todo는 오늘의 다짐을 적는 부분입니다.")
private String todo;

@Column(name = "TODO_CHECK", nullable = false)
@Comment("todoCheck는 실행 여부를 체크하는 부분입니다.")
private boolean todoCheck = false;

@Column(name = "DELETE", nullable = false)
@Comment("delete는 삭제 여부를 체크하는 부분입니다.")
Copy link
Member

Choose a reason for hiding this comment

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

실제 DB 설계하실 때에도 코멘트에 문장으로 작성하시는 편일까요..? 🤔

Copy link
Author

Choose a reason for hiding this comment

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

주석처럼 생각하고 문장으로 작성했습니다.
진주님 코드 보고 저도 erdcloud에 있는 코멘트로 바꿨습니다!

private boolean delete = false;


@Builder
public todo(String todo) {
this.todo = todo;
}


public void changeCheck(boolean todoCheck) {
this.todoCheck = todoCheck;
}

public void update(String todo) {
this.todo = todo;
}
public void delete (boolean delete) {
this.delete = delete;
Copy link
Member

Choose a reason for hiding this comment

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

delete 메서드는 "삭제"에 대한 상태를 "true"로만 바꿔주는 역할만을 할 것 같습니다.
외부에서 false를 받아 처리를 하는 경우가 있다면 [ delete ] 라는 메서드 명이 잘못 되었다고 판단이 드는데요.
만약 "true"로만 바꿔주는 역할이라함은, 굳이 [ boolean delete ] 값을 받을 필요는 없을 것 같아요.

외부에 의한 요인으로 내부 객체 상태가 바뀌는 경우가 있지만,
명확한 경우에는 내부에서 직접 객체 상태를 변경해주는게 더 좋지 않을까 싶습니다.

Copy link
Author

Choose a reason for hiding this comment

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

여기서 외부가 다른 클래스라는 의미일까요?

Copy link
Member

Choose a reason for hiding this comment

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

객체 상태(delete 여부)객체 자신(todo)이 바꿔야 된다고 생각을 하는 편입니다. 이래야 더 안전하고 좀 더 객체지향적인 코드를 작성을 한다고 저는 생각을 하고 있거든요 😂

저의 경우는 해당 코드를 아래와 같이 작성할 것 같아요

public void delete() {
    this.delete = true;
}

지금 주철님의 코드 경우에는 service계층에서 delete를 통해서 true를 넘겨주지만,
혹시 누가 몰래 false를 넣어주는 코드를 실수로 넣었다고 하면, 이를 또 하나씩 찾아야 하잖아요.
다행인건 해당 메서드는 boolean으로 넣어질 수 있는 값이 한정적이지만, String 같은걸로, 혹은 값이 다양한 enum 형식이였다면, 충분히 상태가 잘못될 수 있는 상황을 열어주는 것이라고 봅니다.

외부라는 건, 다른 클래스가 될 수도 있을 것 같네요.

혹시 질문에 답변이 되었을까요?

}

}
18 changes: 18 additions & 0 deletions src/main/java/org/idiot/yesslave/todo/dto/SaveDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.idiot.yesslave.todo.dto;

import lombok.AccessLevel;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
// cannot deserialize from Object value (no delegate- or property-based Creator)에러로 인해 기본 생성자 필요
public class SaveDto {
private String todo;
@Builder
public SaveDto(String todo) {
this.todo = todo;
}

}
15 changes: 15 additions & 0 deletions src/main/java/org/idiot/yesslave/todo/dto/UpdateDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.idiot.yesslave.todo.dto;

import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class UpdateDto {
private String todo;

public UpdateDto(String todo) {
this.todo = todo;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.idiot.yesslave.todo.repository;

import org.idiot.yesslave.todo.domain.todo;
import org.springframework.data.jpa.repository.JpaRepository;

public interface TodoRepository extends JpaRepository<todo,Long>{
Copy link
Member

Choose a reason for hiding this comment

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

JpaRepository<todo,Long>{

중괄호 시작 전에 공백이 하나 있으면 좋을 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

항상 이런 실수를 못 보고 지나가네요 수정하겠습니다

}
41 changes: 41 additions & 0 deletions src/main/java/org/idiot/yesslave/todo/ui/TodoController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package org.idiot.yesslave.todo.ui;

import lombok.RequiredArgsConstructor;
import org.idiot.yesslave.todo.application.TodoService;
import org.idiot.yesslave.todo.dto.SaveDto;
import org.idiot.yesslave.todo.dto.UpdateDto;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;

@RestController
@RequestMapping("/todo")
@RequiredArgsConstructor
public class TodoController {

private final TodoService todoService;

@PostMapping
public ResponseEntity todoSave(@RequestBody SaveDto saveDto) {
todoService.save(saveDto);
return ResponseEntity.ok("Todo save successfully");
}

@PatchMapping("/{id}")
Copy link
Member

Choose a reason for hiding this comment

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

저는 Patch형식보다는 Put을 주로 더 선호하는 편인데, 이유는 Patch의 경우 바꾸고자 하는 내용만 처리해줘야하는 "별도 로직"을 작성을 해야한다는 점 때문이에요.
주철님의 경우, UpdateDto 내부에 todo 밖에 없기 때문에 크게 문제는 되지 않을 것 같지만, 한 번 아래 글 읽어보시면 좋을 것 같아용

https://kimpaul.tistory.com/11

public ResponseEntity todoUpdate(@PathVariable Long id, @RequestBody UpdateDto updateDto) {
todoService.update(id, updateDto);
return ResponseEntity.ok("Todo updated successfully");
}

@PostMapping("/{id}")
public ResponseEntity changeCheck(@PathVariable Long id) {
todoService.changeCheck(id);
return ResponseEntity.ok("Todo change successfully");
}

@DeleteMapping("/{id}")
public ResponseEntity delete(@PathVariable Long id) {
todoService.delete(id);
return ResponseEntity.ok("Todo delete successfully");
}

}
4 changes: 3 additions & 1 deletion src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,6 @@ spring:
add-mappings: false
mvc:
throw-exception-if-no-handler-found: true

main:
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
Author

Choose a reason for hiding this comment

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

깃허브 빌드에서 발생하는 테스트 코드 에러를 없애려고 사용했는데 지금 보니까 제 테스트 자체에도 문제가 있어서 수정 중입니다.

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
Author

Choose a reason for hiding this comment

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

main:
allow-bean-definition-overriding: true
은 지금 삭제했습니다.
테스트 코드는 좀 더 수정하고 올리겠습니다!

allow-bean-definition-overriding: true

129 changes: 129 additions & 0 deletions src/test/java/org/idiot/yesslave/todo/ui/TodoControllerTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
package org.idiot.yesslave.todo.ui;

import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import com.fasterxml.jackson.databind.ObjectMapper;

import org.assertj.core.api.Assertions;
import org.idiot.yesslave.todo.domain.todo;
import org.idiot.yesslave.todo.dto.SaveDto;
import org.idiot.yesslave.todo.repository.TodoRepository;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.MockMvc;

import java.time.LocalDateTime;


@SpringBootTest
@AutoConfigureMockMvc
public class TodoControllerTest {
Copy link
Member

Choose a reason for hiding this comment

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

테스트 코드 작성 경험이 적으신 와중에 이렇게 짜주셔서 감사합니다!

제가 생각했을 때 테스트코드는 [ 성공 ] 보다는 [ 실패 ]에 대한 경우를 잡는게 더 중요하다고 생각하는데요.
주철님이 짜주신 테스트 코드는 전반적으로 [ 성공 ] 의 경우만 작성이 된 것 같아요.

아직 어색하시겠지만, 제가 곧 올리는 PR에서 "실패"의 경우는 어떻게 처리를 했는지 한 번 봐보시면 좋을 것 같습니다.

@Autowired
ObjectMapper mapper;

@Autowired
MockMvc mvc;

@Autowired
private TodoRepository todoRepository;

private static final String BASE_URL = "/todo";

@Test
@DisplayName("저장 테스트")
void save_test() throws Exception {
//given
String text = "Test schedule";

//when
String body = mapper.writeValueAsString(
SaveDto.builder()
.todo(text)
.build()
);

//then
mvc.perform(post(BASE_URL)
.content(body) //HTTP Body에 데이터를 담는다
.contentType(MediaType.APPLICATION_JSON) //보내는 데이터의 타입을 명시
)
.andExpect(status().isOk())
.andExpect(content().string("Todo save successfully"));
}

@Test
@DisplayName("수정 테스트")
void patch_test() throws Exception {

//given
String text = "Test schedule";
String change_text = "Change test schedule";

//when
todoRepository.save(todo.builder()
.todo(text)
.build());

String body = mapper.writeValueAsString(
SaveDto.builder()
.todo(change_text)
.build()
);

//then
mvc.perform(patch(BASE_URL + "/2")
.contentType(MediaType.APPLICATION_JSON)
.content(body)
)
.andExpect(status().isOk())
.andExpect(content().string("Todo updated successfully"));

}

@Test
@DisplayName("체크 변경 테스트")
void change_check_test() throws Exception {

//given
String text = "Test schedule";

//when
todoRepository.save(todo.builder()
.todo(text)
.build());

//then
mvc.perform(post(BASE_URL + "/2")
.contentType(MediaType.APPLICATION_JSON)
)
.andExpect(status().isOk())
.andExpect(content().string("Todo change successfully"));

}

@DisplayName("삭제 테스트")
@Test
public void delete_Test() throws Exception {

//given
String text = "Test schedule";

//when
todoRepository.save(todo.builder()
.todo(text)
.build());

//then
mvc.perform(delete(BASE_URL + "/1"))
.andExpect(status().isOk())
.andExpect(content().string("Todo delete successfully"));

Assertions.assertThat(todoRepository.findById(1L).isEmpty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;


@WebMvcTest
@WebMvcTest(WorkTimerController.class)
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻👍🏻👍🏻 이렇게 처리를 하면, 해댕 컨트롤러와, 관련 bean 들을 등록해서 스부테보다는 가볍게 테스트 환경을 꾸릴 수 있더라구요.

@ExtendWith(SpringExtension.class)
class WorkTimerControllerTest {

Expand Down
Loading