[H조] 백엔드파트 코드리뷰용 PR #75

pkl0912 · 2024-11-03T12:36:09Z

Resolves #{이슈-번호}

Issue Define

Summary of resolutions or improvements

Note

…_Server

…d-base-class Chore#1: 라이브러리 추가 및 기본 응답/에러 클래스 생성

…Server into dev

Fix#70: 완료된 할 일 desc -> asc

Refactor: 1차 기능 리팩토링 및 테스트코드 추가

Fix: Test용 redis 도입

Fix: test용 redis 포트번호 수정

Fix: backlog 리스트 조회 수정

Refactor: scheduler

…blocks Feat#79: 튜토리얼 데이터 추가

devxb

질문주신 내용은 코드리뷰로 작성해두었습니다.
간접참조와 직접참조를 구분하는것이 쉽지 않았을것 같은데, 수고하셨습니다 👍

devxb · 2024-11-10T01:20:16Z

.github/ISSUE_TEMPLATE/🥔poptato-server-issue-template.md

@@ -0,0 +1,31 @@
+---
+name: "\U0001F954Poptato Server Issue Template"


devxb · 2024-11-10T01:22:33Z

src/main/java/server/poptato/PoptatoApplication.java

+        System.out.println("Default Time Zone: " + java.util.TimeZone.getDefault().getID());
        System.out.println("Hello Poptato Server Team !!");


System print 보다는 logger를 사용하는것은 어떨까요?

devxb · 2024-11-10T01:23:15Z

src/main/java/server/poptato/auth/api/request/ReissueTokenRequestDto.java

+@NoArgsConstructor
+@AllArgsConstructor
+public class ReissueTokenRequestDto {
+    //TODO: 사용안하는 필드


@Deprecated 로 명시하면 사용하는 측에서도 더이상 사용하지 않는필드라고 인지할 수 있습니다.

devxb · 2024-11-10T01:26:14Z

src/main/java/server/poptato/auth/application/service/AuthService.java

+        Optional<User> user = userRepository.findByKakaoId(userInfo.socialId());
+        if (user.isEmpty()) {
+            LoginResponseDto response = createNewUserResponse(userInfo);
+            createTutorialData(response.userId());
+            return response;
+        }


Optional을 쓰면서 if문이 필요한 경우는 드뭅니다. (가독성을 위해 일부러 사용하는 경우를 제외하고는)
지금 상황에서도 if문을 없앨 수 있을거 같아요.

devxb · 2024-11-10T01:28:05Z

src/main/java/server/poptato/auth/application/service/AuthService.java

@@ -0,0 +1,127 @@
+package server.poptato.auth.application.service;


질문사항중에
public -> private으로 메소드를 정렬했다고 해주셨는데요, 이 클래스가 그렇게 되어있는 것 같아요.

읽기 좋은 코드는 어떻게 정렬되어 있어야 할까요? public -> private 순 이라기 보다는 사용하는 함수의 바로 아래에 위치한게 읽기 편한것 같습니다. (책 처럼 읽히는)
현재는 어떤 private 메소드를 사용하는지 추적하기위해 스크롤을 너무 많이 내려야 하는것 같아요.

관련해서 클린코드에도 이런 내용이 나오는데 읽어보셔도 좋을거 같습니다.

devxb · 2024-11-10T01:50:42Z

src/main/java/server/poptato/auth/application/service/AuthService.java

+import server.poptato.external.oauth.SocialPlatform;
+import server.poptato.external.oauth.SocialService;
+import server.poptato.external.oauth.SocialServiceProvider;
+import server.poptato.external.oauth.SocialUserInfo;


external레이어와 service레이어 중 도메인 로직이 있는 service 레이어가 더 중요한 레이어 같은데요.
이렇게, service -> external로 의존성이 가게 되면, external이 변경되었을때 service도 함께 변경되어야 하지 않을까요?

인터페이스 위치를 조정해서 external -> service에 의존하도록 만드는것은 어떨까요?

devxb · 2024-11-10T01:53:57Z

src/main/java/server/poptato/todo/application/TodoService.java

+
+

필드, 함수간 띄어쓰기 룰도 추가해서 적용해주시면 좋을거 같습니다!

devxb · 2024-11-10T02:01:12Z

src/main/java/server/poptato/todo/application/TodoService.java

+    public void updateDeadline(Long userId, Long todoId, DeadlineUpdateRequestDto deadlineUpdateRequestDto) {
+        userValidator.checkIsExistUser(userId);
+        Todo findTodo = validateAndReturnTodo(userId, todoId);
+        findTodo.updateDeadline(deadlineUpdateRequestDto.getDeadline());
+        //TODO: 여기도 왜 SAVE가 필수인지 몰겟담
+        todoRepository.save(findTodo);
+    }


어떻게 테스트 하셨나요??
저도 로직을 보니 변경감지가 적용되지 않을 이유가 없어보이는데요.

예상치못한 동작을 하는경우는 대표적으로

외래키로 걸려있는경우 연관관계 주인을 업데이트 해주지 않았을때 <- 지금은 해당되지 않음.

트랜잭션 중간에 다른 주솟값을 가진 Entity로 복사되었을때 영속성 컨텍스트에서 detach 되기 때문에 변경감지가 동작하지 않음 <- 마찬가지로 해당 X

DataJpaTest를 수행했을때 테스트 함수에 Transactional이 붙기 때문에 적용되지 않은것 처럼 느낄 수 있음.

devxb · 2024-11-10T02:03:16Z

src/main/java/server/poptato/todo/domain/entity/Todo.java

+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    private Long id;
+    @NotNull
+    private Long userId;


여기가 질문주신 간접참조를 한 곳 이군요 ㅎㅎ
구현하신것 처럼 다른 도메인의 경우 직접참조 대신 간접참조를 사용하곤 합니다. 👍
이유는, 소스코드가 커졌을때, 직접 참조를 하게 될 경우 Todo에서 User의 정보를 변경할수도 있고, DB join 관리와 코드가 복잡해지기 때문이에요.

fk 같은경우 상황에 따라 다른데, 저는 걸지 않고 애플리케이션 로직으로 정합성을 맞추는것을 선호합니다. fk를 걸게되면 운영할때 fk로 인해서 힘들어지는 경우가 많아서요 ㅎㅎ

devxb · 2024-11-10T02:04:02Z

src/main/java/server/poptato/todo/infra/repository/JpaTodoRepository.java

+import java.time.LocalDate;
+import java.util.List;
+
+public interface JpaTodoRepository extends TodoRepository, JpaRepository<Todo,Long> {


TodoRepository와 JpaRepository를 분리해서 어떤 이점이 있을까요?
JPA가 바뀔일이 흔할까요? 오히려 불필요한 코드가 늘어나는것 아닐까요?

devxb

질문주신 내용은 코드리뷰로 작성해두었습니다.
간접참조와 직접참조를 구분하는것이 쉽지 않았을것 같은데, 수고하셨습니다 👍

yeonjookang and others added 30 commits October 4, 2024 11:27

chore: readme 수정

e69d231

chore

9d61586

chore: readme 수정

5a4fb50

Merge branch 'main' of https://github.com/Kusitms-POPTATO-DEV/Poptato…

18f5b98

…_Server

chore: issue template 생성

5c9120a

chore: pr template 생성

6de6f73

chore: 자동 reviewer, assignee 설정

c05dbd9

chore: 자동 label 설정

4ac0f0e

feat: ci/cd 설정

88bb69d

chore: 외부 라이브러리 추가

78b63e4

feat: Base 응답 클래스, 에러 클래스 추가

90ec71f

fix: ci.yml 파일 수정

6005762

Merge branch 'dev' into chore/1-add-library-and-base-class

278640b

fix: ci 시 테스트 제외

3dbbc04

Merge pull request #2 from Kusitms-POPTATO-DEV/chore/1-add-library-an…

1d32f56

…d-base-class Chore#1: 라이브러리 추가 및 기본 응답/에러 클래스 생성

fix: cd 파일 수정

5e3e0a5

Merge branch 'dev' of https://github.com/Kusitms-POPTATO-DEV/Poptato_…

5932357

…Server into dev

fix: cd 파일 수정

6c9e908

fix: cd 파일 수정

015e7ba

fix: cd 파일 수정

05bee64

fix: cd 파일 수정

ed61ff5

fix: cd 파일 수정

5fa618b

fix: cd 파일 수정

3e79b46

chore: token record 추가

2572b87

chore: config 추가

b8c7df5

chore: auth dto 추가

f37f394

feat: Auth service, controller 추가

ceca9bd

feat: 카카오 관련 기능 추가 - external 패키지

3321ca9

chore: user 도메인 및 엔티디 추가 및 response 수정

1edacc5

fix: 회원가입, 로그인 시 응답 구분

3180770

yeonjookang and others added 16 commits October 29, 2024 19:23

refactor: Todo 도메인, Policy 도메인 API 리팩토링

64a035a

fix: 완료된 할 일 desc -> ascr

2b75ba7

Merge pull request #71 from Kusitms-POPTATO-DEV/fix/sort-today-finished

6f3cebf

Fix#70: 완료된 할 일 desc -> asc

test: 테스트코드 수정 및 추가

456edf4

merge: dev 브랜치 합병

f030917

fix: ci에서 redis image 받도록 ci.yml 수정

fc0ac5f

fix: ci에서 redis 실행하도록 ci.yml 수정

587d72c

fix: ci에서 redis 실행하도록 ci.yml 수정

18cfe40

fix: ci에서 redis 실행하도록 ci.yml 수정

092c703

Merge pull request #72 from Kusitms-POPTATO-DEV/refactor/mvp

0ad3913

Refactor: 1차 기능 리팩토링 및 테스트코드 추가

fix: @testcontainers를 사용하여 test용 redis 사용하도록 수정

e5b004a

Merge pull request #73 from Kusitms-POPTATO-DEV/fix/test-redis-docker

8f4309e

Fix: Test용 redis 도입

Chore: README 추가

23cb539

fix: test용 redis 포트번호 수정

fd6b0cf

Fix: Readme 이미지 수정

a117ee1

Merge pull request #74 from Kusitms-POPTATO-DEV/fix/test-redis-port

ae180e8

Fix: test용 redis 포트번호 수정

auto-assign bot requested a review from yeonjookang November 3, 2024 12:36

auto-assign bot assigned pkl0912 Nov 3, 2024

yeonjookang and others added 9 commits November 3, 2024 22:20

fix: backlog 리스트 조회 시 todayStatus가 null인 데이터도 조회하도록 수정

2af54c9

fix: findBacklogs 버그 수정

1d873fd

Merge pull request #76 from Kusitms-POPTATO-DEV/fix/backlog-list

852398d

Fix: backlog 리스트 조회 수정

refactor: 스케줄러 클래스 메소드 추출

227eb8b

test: 스케줄러 테스트코드 수정 및 추가

6887b4f

Merge pull request #77 from Kusitms-POPTATO-DEV/refactor/scheduler

8ebbe6d

Refactor: scheduler

feat: 튜토리얼 블럭 생성

fb16c91

chore: 튜토리얼 test 추가

bc761e9

Merge pull request #80 from Kusitms-POPTATO-DEV/feat/create-tutorial-…

dcac011

…blocks Feat#79: 튜토리얼 데이터 추가

devxb reviewed Nov 10, 2024

View reviewed changes

Update README.md

672f79a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[H조] 백엔드파트 코드리뷰용 PR #75

[H조] 백엔드파트 코드리뷰용 PR #75

pkl0912 commented Nov 3, 2024

devxb left a comment

devxb Nov 10, 2024

devxb Nov 10, 2024

devxb Nov 10, 2024

devxb Nov 10, 2024

devxb Nov 10, 2024

devxb Nov 10, 2024

devxb Nov 10, 2024

devxb Nov 10, 2024

devxb Nov 10, 2024

devxb Nov 10, 2024

devxb left a comment

		@@ -0,0 +1,31 @@
		---
		name: "\U0001F954Poptato Server Issue Template"

		System.out.println("Default Time Zone: " + java.util.TimeZone.getDefault().getID());
		System.out.println("Hello Poptato Server Team !!");

		@@ -0,0 +1,127 @@
		package server.poptato.auth.application.service;

[H조] 백엔드파트 코드리뷰용 PR #75

Are you sure you want to change the base?

[H조] 백엔드파트 코드리뷰용 PR #75

Conversation

pkl0912 commented Nov 3, 2024

Issue Define

Summary of resolutions or improvements

Note

devxb 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devxb left a comment

Choose a reason for hiding this comment