feature/4-update-1 #10

jiae5287 · 2022-02-17T13:05:15Z

<구현한 기능>

패스워드 변경
주소 저장하기
모든 주소 가져오기
주소 삭제하기
주소 수정하기

<구현할 기능>
session.

최근 주소 가져오기 기능은 추가할지 말지 좀 더 생각해볼 계획입니다.

f-lab-bright · 2022-02-22T11:32:03Z

src/main/java/com/happy/delivery/presentation/user/rest/UserController.java

+
+
+    @ResponseStatus(code = HttpStatus.OK)
+    @PatchMapping("/myAccount/password")


path에 대문자가 들어가는건 그다지 좋지 않습니다.
url에 들어가는 문자에 제한이 있는데, 한 번 찾아보시면 좋을 것 같네요 :)

확인했습니다. my-account로 바꿨습니다.

f-lab-bright · 2022-02-22T11:33:00Z

src/main/java/com/happy/delivery/infra/repository/user/HashMapUserRepository.java

+    public User changePassword(Long id, String encryptedPassword) {
+        User currentUser = this.findById(id);
+        User changedUser = new User(
+            currentUser.getId(),
+            currentUser.getEmail(),
+            encryptedPassword,
+            currentUser.getName(),
+            currentUser.getPhoneNumber()
+        );
+        hashmap.replace(id, changedUser);
+        return changedUser;
+    }


이게 repository에서 해야 할 일일까요?
Repository가 어떤건지 한 번 다시 찾아보시는게 필요할 것 같습니다.

삭제했습니다. repository에 대해 다시 찾아봤습니다.

f-lab-bright · 2022-02-22T11:33:34Z

src/main/java/com/happy/delivery/domain/user/repository/UserRepository.java

@@ -8,4 +8,6 @@
    User findById(Long id);

    User findByEmail(String email);
+
+    User changePassword(Long id, String encryptedPassword);


repository의 역할에 changePassword가 들어가는게 적합할까요?

삭제했습니다.

f-lab-bright · 2022-02-22T11:34:24Z

src/main/java/com/happy/delivery/application/user/UserServiceV1.java

+        // *** 아이디를 session에서 꺼내고 사용할 예정 *** session.getId();
+        // *** session을 이용한다면 일치하는 사용자가 있는지 확인하지 않아도 되겠지..? :: 67~8번째줄
+        //User user = userRepository.findById(session.getId());
+        User user = userRepository.findById(1L);


findById에는 command로부터 들어온 패스워드를 변경할 User의 id를 넣어줘야 되겠네요.

임시로 넣어뒀던거라ㅎㅎ 지금은 user의 id값으로 수정했습니다.

f-lab-bright · 2022-02-22T11:35:11Z

src/main/java/com/happy/delivery/application/user/UserServiceV1.java

@@ -54,4 +55,25 @@ public UserResult signin(SigninCommand signinCommand) {
        }
        return UserResult.fromUser(user);
    }
+
+    @Override
+    public UserResult updatePassword(PasswordUpdateCommand passwordUpdateCommand) {


PasswordUpdateCommand 객체에는 userId가 없는데, 어떤 유저의 비밀번호를 변경하는지 어떻게 알 수 있을까요?

updatePassword() 코드 전체 수정했습니다.

f-lab-bright · 2022-02-22T11:35:40Z

src/main/java/com/happy/delivery/application/user/UserServiceV1.java

+        // *** 아이디를 session에서 꺼내고 사용할 예정 *** session.getId();
+        // *** session을 이용한다면 일치하는 사용자가 있는지 확인하지 않아도 되겠지..? :: 67~8번째줄
+        //User user = userRepository.findById(session.getId());


세션을 Service에서 직접 사용하는게 바람직한 방법일까요?
session은 어떤 레이어에서 가져야 할 책임일까요?

service에서 사용하지는 않았고 controller에서 가져왔습니다. :)
session값을 가져와야 한다는 뜻이었습니다ㅎㅎ

…Address() 수정 / checkstyle 오류 수정

f-lab-bright · 2022-02-27T01:03:14Z

src/main/java/com/happy/delivery/application/user/UserServiceV1.java

+      throw new PasswordIsNotMatchException("현재 패스워드가 일치하지 않습니다.");
+    }
+    User result = userRepository.save(
+        new User(


새 User 객체를 만들 필요가 있을까요?

사실 setter를 사용하고 싶었는데,
도메인일 경우 setter를 사용하기보단 새로운 객체를 생성하는 것이 더 안전하다는 글을 읽었습니다.
setter와 new를 사용하지 않고 안전하게 코드를 바꾸는 방법이 있을까요? 지금은 setter밖에 생각이 안 나서요ㅎㅎ;;

새 객체를 생성하는게 왜 더 안전한가요?
객체가 상태를 유지 관리 하는 객체라면 새 객체를 생성하면 상태가 사라져서 의도와 다른 동작을 하게 될 수 있습니다.

그렇군요!ㅎㅎ 그런 경우엔 새로운 객체를 생성하는 것보다 setter가 훨씬 좋겠네요 :)
저는 setter를 쓰면 안된다는 강박 같은게 있었는데 잘못 생각했네요ㅎㅎ

setXXX 대신 변경을 의미적으로 더 잘 드러낼 수 있는 메소드를 정의할 수 있다며 더 좋습니다.
setPassword -> changePassword 로 하고 내부에서 필요한 다른 검증이나 처리들을 더 해줄 수도 있겠죠 :)

변경했습니다. 비밀번호 암호화를 changePassword에서 하고 싶어서 encryptMapper를 매개변수로 넘겨줬습니다.
시간 되실 때 괜찮은 방법인지 한번 확인해주세요 :)

f-lab-bright · 2022-02-27T04:27:01Z

src/main/java/com/happy/delivery/domain/user/User.java

@@ -1,5 +1,7 @@
 package com.happy.delivery.domain.user;

+import com.happy.delivery.infra.encoder.EncryptMapper;


domain에서 infra의 의존성을 바로 가지는건 좋지 않을 것 같네요.
패스워드를 암호화 하는게 도메인의 역할이라 생각된다면 domain에서 패스워드를 암호화하는 인터페이스를 구현해두고, infra에서 구현한 뒤 주입받아서 사용하여야 될 것 같네요.

이 부분은 추후에 리펙토링 하면서 의존성 고민을 해보시면 좋겠네요

네 좀 더 고민해보겠습니다.

f-lab-bright

LGTM

f-lab-bright · 2022-02-27T23:53:49Z

src/main/java/com/happy/delivery/application/user/UserServiceV1.java

+      throw new PasswordIsNotMatchException("현재 패스워드가 일치하지 않습니다.");
+    }
+    user.changePassword(encryptMapper, passwordUpdateCommand.getChangedPassword());
+    User result = userRepository.save(user);


update에서는 다시 할당 받지 않아도 되긴 합니다.
다시 받더라도 결국 같은 주소의 인스턴스가 반환될거라 대부분은요 :)

아! 그렇네요ㅎㅎ 수정하겠습니다.

f-lab-bright · 2022-02-27T23:54:41Z

src/main/java/com/happy/delivery/domain/user/User.java

@@ -1,5 +1,7 @@
 package com.happy.delivery.domain.user;

+import com.happy.delivery.infra.encoder.EncryptMapper;


이 부분은 추후에 리펙토링 하면서 의존성 고민을 해보시면 좋겠네요

jiae5287 added 2 commits February 17, 2022 21:47

DeliveryApplication 필요 없는 import 삭제

87f2a98

updatePassword API 구현 중

1b20b54

jiae5287 requested review from takiki-web and f-lab-bright February 17, 2022 13:05

jiae5287 and others added 9 commits February 17, 2022 22:43

README.md 내용 추가

3137155

lint 구성 중 :: 아직 성공하지 못 함

59da798

비밀번호 변경 기능 구현 중

7bdf176

checkstyle 오류 수정

71f7112

주소 변경

562351a

주소변경

63e5d17

정훈님 코드랑 합침

03d83af

Merge branch 'main' into feature/4-update-1

3efcd19

비밀번호 변경 기능 수정, 주소 저장

f24bcfa

f-lab-bright reviewed Feb 22, 2022

View reviewed changes

jiae5287 and others added 4 commits February 22, 2022 22:37

checkstyle 라인정리

9655bed

checkstyle 에러사항 수정

bb0dca1

UserRepository의 changePassword() 삭제 / UserAddressRepository 생성 / save…

3b3bc33

…Address() 수정 / checkstyle 오류 수정

Merge branch 'main' into feature/4-update-1

8dd1711

jiae5287 changed the title ~~비밀번호 수정 기능 구현 중~~ 비밀번호 수정 기능, 주소 받기 기능 만듦 Feb 24, 2022

jiae5287 changed the title ~~비밀번호 수정 기능, 주소 받기 기능 만듦~~ 비밀번호 수정 기능, 주소 받기 기능 구현 중 Feb 24, 2022

jiae5287 and others added 5 commits February 25, 2022 10:48

Merge branch 'main' into feature/4-update-1

c6af54a

모든 주소 가져오기 기능 구현

fe8cfb1

SessionUtils에 ADDRESS_ID 넣기

8d88caa

updateePassword() result값 수정

ac8588c

정규식 띄어쓰기 허용으로 수정

89e8bfa

jiae5287 changed the title ~~비밀번호 수정 기능, 주소 받기 기능 구현 중~~ feature/4-update-1 Feb 26, 2022

f-lab-bright reviewed Feb 27, 2022

View reviewed changes

jiae5287 added 3 commits February 27, 2022 11:32

주소 삭제 기능 추가

97574fa

checkcstyle 오류 수정

ba57b0c

updatePassword() 리팩토링

9645352

f-lab-bright reviewed Feb 27, 2022

View reviewed changes

jiae5287 added 3 commits February 27, 2022 15:43

updatePassword() 기능 구현

cb1e9c5

ApiResponse 약간 수정

42c95cc

UserService에 updateAddress 추가

b6b35e8

f-lab-bright approved these changes Feb 27, 2022

View reviewed changes

jiae5287 merged commit c691c07 into main Feb 28, 2022

jiae5287 linked an issue Feb 28, 2022 that may be closed by this pull request

[#1] 로그인 로그아웃 외 부가 정보 서비스 개발 #9

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature/4-update-1 #10

feature/4-update-1 #10

jiae5287 commented Feb 17, 2022 •

edited

Loading

f-lab-bright Feb 22, 2022

jiae5287 Feb 24, 2022

f-lab-bright Feb 22, 2022

jiae5287 Feb 24, 2022

f-lab-bright Feb 22, 2022

jiae5287 Feb 24, 2022

f-lab-bright Feb 22, 2022

jiae5287 Feb 24, 2022

f-lab-bright Feb 22, 2022

jiae5287 Feb 24, 2022

f-lab-bright Feb 22, 2022

jiae5287 Feb 24, 2022

f-lab-bright Feb 27, 2022

jiae5287 Feb 27, 2022

f-lab-bright Feb 27, 2022

jiae5287 Feb 27, 2022

f-lab-bright Feb 27, 2022

jiae5287 Feb 27, 2022

f-lab-bright Feb 27, 2022

f-lab-bright Feb 27, 2022

jiae5287 Feb 28, 2022

f-lab-bright left a comment

f-lab-bright Feb 27, 2022

jiae5287 Feb 28, 2022

f-lab-bright Feb 27, 2022



		@ResponseStatus(code = HttpStatus.OK)
		@PatchMapping("/myAccount/password")

		@@ -1,5 +1,7 @@
		package com.happy.delivery.domain.user;

		import com.happy.delivery.infra.encoder.EncryptMapper;

feature/4-update-1 #10

feature/4-update-1 #10

Conversation

jiae5287 commented Feb 17, 2022 • edited Loading

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

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

f-lab-bright 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

jiae5287 commented Feb 17, 2022 •

edited

Loading