JuyeoungJun / cron-monitoring

for cron-monitoring
0 stars 0 forks source link

Resolve "크론 잡 기능 개발" #47 - [merged] #100

Closed JuyeoungJun closed 3 years ago

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 16, 2021, 15:13

Merges feat/47-cron-job -> develop

What?

크론 서비스 기능 개발

Issue Number

47

Why?

크론 기능 구현을 위해

How?

API 스펙 참조

Testing?

CRUD 기능의 계층별 구현 및 테스트 후 통합테스트 진행

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 16, 2021, 15:15

changed the description

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 17, 2021, 10:17

changed title from Draft: Resolve "크론 잡 기능 개발" to Draft: Resolve "크론 잡 기능 개발"{+ #47+}

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 17, 2021, 10:17

changed the description

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 21, 2021, 13:16

added 16 commits

Compare with previous version

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 21, 2021, 13:57

added 1 commit

Compare with previous version

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 21, 2021, 14:04

added 1 commit

Compare with previous version

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 21, 2021, 15:51

added 1 commit

Compare with previous version

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 09:42

added 1 commit

Compare with previous version

JuyeoungJun commented 3 years ago

In GitLab by @gm2202985 on Jun 22, 2021, 11:40

Commented on src/main/java/gabia/cronMonitoring/controller/CronJobController.java line 61

Delete에서만 ResponseEntity를 return 하는데 특별한 이유가 있으신가요?

JuyeoungJun commented 3 years ago

In GitLab by @gm2202985 on Jun 22, 2021, 11:40

Commented on src/main/java/gabia/cronMonitoring/repositoryImpl/CronJobRepositoryImpl.java line 28

여기서 em.persist와 같이 em.merge도 동시에 하는 것은 어떤가요? https://velog.io/@kobongkyu/JPA-Merge-vs-Persist persist와 merge의 차이점입니다.

JuyeoungJun commented 3 years ago

In GitLab by @gm2202985 on Jun 22, 2021, 11:40

Commented on src/main/java/gabia/cronMonitoring/service/CronJobService.java line 62

setter를 쓰지 않고 객체에서 메소드를 제공하는 방식은 어떤가요? https://velog.io/@aidenshin/%EB%82%B4%EA%B0%80-%EC%83%9D%EA%B0%81%ED%95%98%EB%8A%94-JPA-%EC%97%94%ED%8B%B0%ED%8B%B0-%EC%9E%91%EC%84%B1-%EC%9B%90%EC%B9%99

JuyeoungJun commented 3 years ago

In GitLab by @gm2202981 on Jun 22, 2021, 12:10

Commented on src/main/java/gabia/cronMonitoring/repositoryImpl/CronJobRepositoryImpl.java line 57

리턴 타입이 Spring Data JPA의 컨벤션인 void와 차이가 나는데, 이 부분은 derived query문을 이용하고 @Modifying 어노테이션을 달면 정수형으로 삭제된 row 개수를 반환받을 수 있습니다. 해당 코드를 후에 Spring Data JPA로 대체해야 하는 상황이 발생한다면 조정해야 할 것으로 보입니다.

참고자료

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 12:13

Commented on src/main/java/gabia/cronMonitoring/repositoryImpl/CronJobRepositoryImpl.java line 28

merge 함수를 일부러 사용하지 않았습니다. 이유는 다음과 같습니다.

  1. 요구된 기능 api에 put 메서드가 존재하지 않음.

  2. 사전 협의된 모든 수정은 patch 방식을 사용하기로 함.

혹시 모를 오류 상황으로 엔티티 정보에 null이 들어가는 put 방식의 merge 함수를 애초부터 사용하지 않아서

오류가 발생할 상황을 미리 막고자 합니다.

대신 서비스 계층에서 직접 set 하여 엔티티 수정 진행하도록 로직 작성했습니다.

JuyeoungJun commented 3 years ago

In GitLab by @gm2202981 on Jun 22, 2021, 12:15

Commented on src/test/java/gabia/cronMonitoring/controller/CronJobControllerTest.java line 213

테스트 코드에서 API 스펙에 따른 HTTP 상태도 확인하는 것이 좋지 않을까요?

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 12:20

Commented on src/main/java/gabia/cronMonitoring/service/CronJobService.java line 62

말씀해주신 엔티티의 생성자 혹은 create 메서드를 만드는 방법이 좋다고 생각하고 처음에 적용하려고 했으나 하지 않은 이유는 다음과 같습니다.

  1. 사전 협의 과정에서 개발 기간 동안은 setter를 이용하고 이후 리팩토링 과정을 통해 setter를 제거하기로 결정함.

  2. 크론 기능을 공동으로 개발하기에 공동으로 사용하는 엔티티에 [create 메서드 혹은 생성자] 를 만든다면, 혹은 setter를 없앤다면 머지 과정중 오류와 충돌이 예상됨.

따라서 개인적으로 만든 dto 객체들은 필요한 생성자를 자유롭게 선언/구현 해서 사용하고 있지만 엔티티 관련 부분은 setter 이외에 다른 방법을 사용하지 않았습니다.

이후 개발부터는 위 내용을 적용하겠습니다

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 12:21

Commented on src/main/java/gabia/cronMonitoring/repositoryImpl/CronJobRepositoryImpl.java line 57

넵 확인했습니다 감사합니다!

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 12:22

Commented on src/test/java/gabia/cronMonitoring/controller/CronJobControllerTest.java line 213

status().is4xxClientError()

함수를 이용해 큰 형태로 검사했는데 더 확실한 메서드를 사용해야 할까요? (ex, isSuccess(), isOk)

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 12:25

Commented on src/main/java/gabia/cronMonitoring/controller/CronJobController.java line 61

현재 api 스펙에는 삭제시 상태코드 이외에 다른 값을 return 하지 않도록 정해져 있습니다.

하지만 이는 추후에 변경될 수 있기에( 유지보수를 생각해)값을 편리하게 추가/삭제 할 수 있는 ResponseEntity를 이용했습니다.

JuyeoungJun commented 3 years ago

In GitLab by @gm2202981 on Jun 22, 2021, 13:05

Commented on src/test/java/gabia/cronMonitoring/controller/CronJobControllerTest.java line 213

아 그대로 가셔도 될듯하네요.

JuyeoungJun commented 3 years ago

In GitLab by @gm2202981 on Jun 22, 2021, 13:05

approved this merge request

JuyeoungJun commented 3 years ago

In GitLab by @gm2202981 on Jun 22, 2021, 13:20

Commented on src/main/java/gabia/cronMonitoring/util/CronMonitorUtil.java line 21

Jackson ObjectMapper는 싱글턴 빈으로 등록되어 있는 만큼 util로 분리하기 보다 다음과 같은 형식으로 그때그때 호출해서 쓰는게 낫지 않을까요?

@Autowired
ObjectMapper mapper
JuyeoungJun commented 3 years ago

In GitLab by @gm2202985 on Jun 22, 2021, 13:26

approved this merge request

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 13:30

Commented on src/main/java/gabia/cronMonitoring/util/CronMonitorUtil.java line 21

changed this line in version 6 of the diff

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 13:30

added 5 commits

Compare with previous version

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 13:31

marked this merge request as ready

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 13:31

enabled an automatic merge when the pipeline for 64c541c3da036ae453c3883dba9b881c5e8f780a succeeds

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 14:59

aborted the automatic merge because source branch was updated

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 14:59

added 1 commit

Compare with previous version

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 14:59

enabled an automatic merge when the pipeline for 7caf01ac96022f456f3333cdfb30e4325ff4d53d succeeds

JuyeoungJun commented 3 years ago

In GitLab by @gm2202983 on Jun 22, 2021, 15:01

mentioned in commit 39d67fe4265327ada8b4f3b2ab2e203503d85f0b