[pangnem] 5주차 과제 제출 #37

jxmen · 2021-09-07T14:25:09Z

5주차 과제 진행하겠습니다 ~

- UserController는 UserService에 의존 - UserControllerTest는 UserService를 mock으로 사용

jxmen · 2021-09-08T02:01:50Z

이번주는 테스트코드에서 의존관계가 있는 클래스는 구현되어 있지 않은 부분은 mock으로 처리하고, 구현이 다 완료된다면 mock을 제거하는 방식으로 해보려고 합니다.

UserService에서 repository 의존성 추가

UserService.updateUser

User.changeInfo

jxmen · 2021-09-08T10:55:34Z

app/src/main/java/com/codesoom/assignment/domain/User.java

+    public User changeInfo(User source) {
+        this.name = source.getName();
+        this.email = source.getEmail();
+        this.password = source.getPassword();
+
+        return this;
+    }


user.changeInfo(User.builder() .name("name") .email("email") .build()); // 패스워드 없음

source에서 특정 값이 null이라면 기존 값을 사용하게 하면 좋을 것 같습니다.
그렇다고 모든 값을 null 체크를 하기는 애매하네요.

this.name = Optional.ofNullable(source.getName()).orElse(this.name); this.email = Optional.ofNullable(source.getEmail()).orElse(this.name); this.password = Optional.ofNullable(source.getPassword()) .orElse(this.password);

이러한 방법을 써봤는데 별로네요... JavaScript라면 다음과 같이 작성할 수 있어요

this.name = source.getName() ?? this.name; this.email = source.getEmail() ?? this.email; this.password = source.getPassword() ?? this.password;

자바에도 이런 문법이 있나요?

js 테스트 코드에 있는 메서드와 경로로 수정합니다.

- 유저 -> 회원으로 이름 통일

hannut91 · 2021-09-08T16:31:55Z

app/src/main/java/com/codesoom/assignment/UserNotFoundException.java

+/**
+ * 회원을 찾지 못할 시 던집니다.
+ */


이 주석은 예외 자체에 대해 설명하는 주석이므로, 던진다라는 표현보다는 회원을 찾지 못했음을 나타내는 예외라는 설명이 더 좋을 것 같아요.

종립님의 javadoc 가이드를 보고 던진다라는 표현을 사용했습니다.

던진다 라는 표현의 주석은 예외 클래스 설명 보다는 실제 던지는 메서드가 더 적절할 것 같네요.

hannut91 · 2021-09-08T16:33:43Z

app/src/main/java/com/codesoom/assignment/domain/ProductRepository.java

+/**
+ * 상품 저장소.
+ */


Repository란 무엇인가요? 저장소를 나타내는 단어인가요?

네. Repository를 저장소를 나타내는 단어로 사용하였습니다. 저장소라는 표현이 적절하지 않은 걸까요?

제가 생각할 때 여기서 Repository는 데이터를 저장할 때 사용되는 Repository가 아니라고 생각해요. Repository pattern에 대해서 찾아보시면 좋을 것 같습니다

레포지토리는 데이터를 저장하고 불러오는 역할을 하며, 추상화된 객체를 전달합니다.
추상화되어 있기 때문에 이 객체는 DB 연결이나 SQL 작성 등을 고려하지 않아도 됩니다.

도메인 영역과 데이터를 매핑을 중재하는 역할을 하여, 마치 컬렉션처럼 다룰 수 있게 해줍니다.
이렇게 컬렉션으로 초첨을 두었을때는 상품 컬렉션이라는 단어도 어울리지 않을까요?

컬렉션이 더 나아 보이네요 ㅎㅎ 이 객체가 하는 일을 기술해주면 좋을 것 같습니다.

상품 데이터 접근에 대한 처리를 합니다. 같은 것을 생각해 보았어요

hannut91 · 2021-09-08T16:35:15Z

app/src/test/java/com/codesoom/assignment/application/UserServiceTest.java

+    class Describe_updateUser {
+
+        @Nested
+        @DisplayName("존재하는 id인 경우")


존재하는 id라기보다 사용자를 찾을 수 있는 경우라고 하는 것은 어떨까요?

좋습니다. Context 클래스 이름도 canFindUser, 찾은 유저의 아이디 변수 이름은 findUserId 등으로 변경하였습니다.

hannut91 · 2021-09-08T16:36:15Z

app/src/test/java/com/codesoom/assignment/controllers/UserControllerTest.java

+    private String toJson(UpdateUserDto updateUserDto) {
+        return String.format(
+            "{\"name\": \"%s\","
+                + " \"email\": \"%s\","
+                + " \"password\": \"%s\"}",
+            updateUserDto.getName(),
+            updateUserDto.getEmail(),
+            updateUserDto.getPassword());
+    }


객체를 JSON문자열로 바꾸실려고 하시는 것 같아요.

객체 => JSON 문자열로 변경하는 다른 방법이 있을까요?

객체를 JSON으로 변경하는 것을 Serialization 이라고 표현하기도 합니다.
1주차때 사용했던 ObjectMapper를 사용하면 되네요.

private String toJson(Object value) throws JsonProcessingException { return new ObjectMapper().writeValueAsString(value); }

https://ramees.tistory.com/33

hannut91 · 2021-09-09T18:46:45Z

app/src/test/java/com/codesoom/assignment/application/UserServiceTest.java

+                User source = User.builder()
+                    .name("name2")
+                    .email("email2")
+                    .password("password2")
+                    .build();


테스트에 사용되는 데이터를 미리 만들어놓고 사용하는 것은 어떨까요? 테스트에서 계속 사용자를 새로 만드는게 눈에 띄네요.

좋습니다! Describe에서 미리 데이터를 만들었습니다.

hannut91 · 2021-09-09T18:51:11Z

app/src/test/java/com/codesoom/assignment/application/UserServiceTest.java

+                assertThat(userRepository.existsById(user.getId()))
+                    .isFalse();
+
+                notExistId = user.getId();


변수의 이름만 다르고 값은 동일한 것을 사용하는 것 같아요. 차라리 Describe에서 id를 똑같이 사용하고, Given이 달라지는 Mocking하는 부분을 달라지게 만들어서 표현하면 좋을 것 같아요.

private Long id; @BeforeEach void setUp() { id = user.getId(); } @Nested @DisplayName("존재하지 않는 id인 경우") class Context_notExistId { @BeforeEach void setUp() { assertThat(userRepository.existsById(id)).isTrue(); } // ... 생략 } @Nested @DisplayName("존재하지 않는 id인 경우") class Context_notExistId { @BeforeEach void setUp() { userRepository.deleteAll(); assertThat(userRepository.existsById(id)).isFalse(); } // ... 생략 }

hannut91 · 2021-09-09T18:53:29Z

app/src/test/java/com/codesoom/assignment/controllers/UserControllerTest.java

+    @Nested
+    @DisplayName("POST /users 요청은")
+    class Describe_postUsers {
+
+        @Nested
+        @DisplayName("유효한 유저 생성 DTO가 주어질 때")
+        class Context_validUser {
+
+            private CreateUserDto validCreateUserDto;
+
+            @BeforeEach
+            void setUp() {
+                validCreateUserDto = CreateUserDto.builder()
+                    .name("name")
+                    .email("email")
+                    .password("password")
+                    .build();
+            }
+
+            @Test
+            @DisplayName("생성한 유저를 리턴하고 201을 응답한다")
+            void it_returns_created_user_and_response_201() throws Exception {
+                mockMvc.perform(
+                    post("/users")
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .content(toJson(validCreateUserDto))
+                )
+                    .andExpect(status().isCreated())
+                    .andExpect(content().json(toJson(validCreateUserDto)));
+            }
+        }
+
+        @Nested
+        @DisplayName("유효하지 않은 유저 생성 DTO가 주어질 때")
+        class Context_invalidCreateUserDto {
+
+            private CreateUserDto invalidCreateUserDto;
+
+            @BeforeEach
+            void setUp() {
+                invalidCreateUserDto = CreateUserDto.builder()
+                    .name("name")
+                    .email("email")
+                    .build();
+            }
+
+            @Test
+            @DisplayName("에러를 던지고 400을 응답한다")
+            void it_throws_and_response_400() throws Exception {
+                mockMvc.perform(
+                    post("/users")
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .content(toJson(invalidCreateUserDto))
+                )
+                    .andExpect(status().isBadRequest());
+            }
+        }
+    }


@Nested @DisplayName("POST /users 요청은") class Describe_postUsers { private CreateUserDto userDto @Nested @DisplayName("유효한 유저 생성 DTO가 주어질 때") class Context_validUser { @BeforeEach void setUp() { userDto = CreateUserDto.builder() .name("name") .email("email") .password("password") .build(); } @Test @DisplayName("생성한 유저를 리턴하고 201을 응답한다") void it_returns_created_user_and_response_201() throws Exception { mockMvc.perform( post("/users") .contentType(MediaType.APPLICATION_JSON) .content(toJson(validCreateUserDto)) ) .andExpect(status().isCreated()) .andExpect(content().json(toJson(userDto))); } } @Nested @DisplayName("유효하지 않은 유저 생성 DTO가 주어질 때") class Context_invalidCreateUserDto { @BeforeEach void setUp() { userDto = CreateUserDto.builder() .name("name") .email("email") .build(); } @Test @DisplayName("에러를 던지고 400을 응답한다") void it_throws_and_response_400() throws Exception { mockMvc.perform( post("/users") .contentType(MediaType.APPLICATION_JSON) .content(toJson(userDto)) ) .andExpect(status().isBadRequest()); } } }

이런식으로 userDto란 변수 이름음 같지만, 주어진 값이 달라지는 경우로 표현하는 것은 어떨까요??

유효한 것과 유효하지 않은 것을 구분하기 위해 서로 다른 변수명 등을 사용했는데, 이 방법도 좋은 것 같습니다.

hannut91 · 2021-09-09T18:55:18Z

app/src/test/java/com/codesoom/assignment/domain/UserTest.java

+    @BeforeEach
+    void setUp() {
+        user = User.builder()
+            .name("name")
+            .email("email")
+            .password("password")
+            .build();
+    }
+
+    @Test
+    void createUser() {
+        assertThat(user.getName()).isEqualTo("name");
+        assertThat(user.getEmail()).isEqualTo("email");
+        assertThat(user.getPassword()).isEqualTo("password");
+    }


changeInfo에서 이미 객체를 생성하는 방법과, 변경하는 방법 그리고 객체의 정보를 얻는 방법 모두를 테스트하고 있으므로, 여기는 없어도 괜찮을 것 같아요

jxmen · 2021-09-10T09:02:59Z

오늘은 오랜만에 휴식을 좀 하려고 리뷰 내용 반영 외에는 큰 작업이 없어요. 미리 코멘트 남겨둡니다.

hannut91 · 2021-09-10T16:41:15Z

app/src/test/java/com/codesoom/assignment/controllers/UserControllerTest.java

+    @Nested
+    @DisplayName("DELETE /users/{id} 요청은")
+    class Describe_deleteUserWithId {
+
+        private User fixtureUser;
+
+        @BeforeEach
+        void setUp() {
+            fixtureUser = User.builder()
+                .name("name")
+                .email("email")
+                .password("password")
+                .build();
+        }
+
+        @Nested
+        @DisplayName("회원을 찾을 수 있는 경우")
+        class Context_findUser {
+
+            private Long findUserId;
+
+            @BeforeEach
+            void setUp() {
+                User user = userRepository.save(fixtureUser);
+
+                Long id = user.getId();
+
+                assertThat(userRepository.existsById(id))
+                    .isTrue();
+
+                findUserId = id;
+            }
+
+            @Test
+            @DisplayName("204를 응답한다")
+            void it_response_204() throws Exception {
+                mockMvc.perform(
+                    delete("/users/" + findUserId)
+                )
+                    .andExpect(status().isNoContent());
+            }
+        }
+
+        @Nested
+        @DisplayName("회원을 찾을 수 없는 경우")
+        class Context_notFoundUser {
+
+            private Long notFoundUserId;
+
+            @BeforeEach
+            void setUp() {
+                User user = userRepository.save(fixtureUser);
+
+                Long id = user.getId();
+
+                userRepository.deleteAll();
+
+                assertThat(userRepository.existsById(id))
+                    .isFalse();
+
+                notFoundUserId = id;
+            }
+
+            @Test
+            @DisplayName("404를 응답한다")
+            void it_response_404() throws Exception {
+                mockMvc.perform(
+                    delete("/users/" + notFoundUserId)
+                )
+                    .andExpect(status().isNotFound());
+            }
+        }
+    }


회원을 찾을 수 없는 경우를 테스트 할 때, 아이디를 얻기 위해 만든 후에 삭제 할 필요는 없을 것 같아요. 회원을 찾을 수 있는 경우에는 테스트를 하기전에 사용자를 만들고 그 id를 사용하고, 찾을 수 없는 경우에는 만들지 않으니까 그냥 값을 할당하더라도 없다고 인식할 수 있을 것 같아요.

@Nested @DisplayName("DELETE /users/{id} 요청은") class Describe_deleteUserWithId { private Long id; @BeforeEach void setUp() { userRepository.deleteAll(); } @Nested @DisplayName("회원을 찾을 수 있는 경우") class Context_findUser { @BeforeEach void setUp() { User fixtureUser = User.builder() .name("name") .email("email") .password("password") .build(); User user = userRepository.save(fixtureUser); id = user.getId(); } @Test @DisplayName("204를 응답한다") void it_response_204() throws Exception { mockMvc.perform( delete("/users/" + id) ) .andExpect(status().isNoContent()); } } @Nested @DisplayName("회원을 찾을 수 없는 경우") class Context_notFoundUser { @BeforeEach void setUp() { id = 9999L; } @Test @DisplayName("404를 응답한다") void it_response_404() throws Exception { mockMvc.perform( delete("/users/" + id) ) .andExpect(status().isNotFound()); } } }

지난주 리뷰에서 나왔던 내용으로, 테스트에서는 논리가 가장 중요하다 입니다.

저장하지 않아 id를 바로 할당해도 테스트가 실패하지는 않겠지만, 논리적인 근거는 조금 부족한 것 같아 이런 식으로 그 id가 없는지 검증하는 부분도 추가하였습니다.

Long id = 9999L; assertThat(userRepository.existsById(id)) .isFalse(); notFoundUserId = id;

다만 이렇게 하니 저장하지 않았기 때문에 아이디가 존재하지 않는다에 대한 설명은 조금 부족한 것 같습니다. 전에는 아이디가 존재하지 않는 것을 삭제하는 것으로 증명하였는데, 이 부분은 이대로 두어도 괜찮을까요?

(추가) 이부분을 못보았네요.. 🥲

@BeforeEach void setUp() { userRepository.deleteAll(); }

- 저장소 대신 컬렉션 표현 사용 - 기타 하는 일 상세 설명

단, 테스트 시에 응답 객체를 만들 경우 id가 null이 되므로 Json 문자열로 만들때는 null 값을 제외한다. reference: - https://junho85.pe.kr/1626

CreateUserDto -> CreateUserRequestDto UpdateUserDto -> UpdateUserRequestDto

hannut91 · 2021-09-11T17:49:09Z

app/src/test/java/com/codesoom/assignment/controllers/UserControllerTest.java

+                assertThat(userRepository.existsById(id))
+                    .isFalse();


이 것은 우리의 테스트가 통과하려면, 이 조건을 충족해야 합니다. 라고 말하고 있는거에요. 허용이라고도 부르는데 단정문과는 조금 의도가 다릅니다. 여기서는 크게 복잡하지 않지만, 오히려 너무 많은 단정문은 테스트의 의도를 파악하기 어렵게 할 수 있으니 주의해야 합니다.

테스트 주도 개발로 배우는 객체지향 설계와 실천에서는 이러한 내용들은 다루는데요. 이 책에서는 jMock을 사용하는데 그래서 이러한 의도를 드러내기 위해서는 일부러 allowing이라는 다른 것을 구분해서 사용합니다. 아마도 여기에 적용한다면 다음과 같은 그림이겠죠. Junit5에서 따로 지원하지 않는 것 같지만 만약에 지원한다면 대충 이런식으로 사용할 수도 있었을 것 같네요

allowing(userRepository.existsById(id)).then(false);

See also

24.3 정확한 예상구문 - 테스트 주도 개발로 배우는 객체지향 설계와 실천

hannut91 · 2021-09-13T15:01:17Z

Mock을 사용하는 것이 고민이시군요 ㅎㅎ 물론 Mock을 과도하게 사용하는 것은 단점이 있을 수 있죠. 엘레강트 오브젝트의 저자는 Mock대신에 Fake Object를 사용하는 것을 권하고 있긴 합니다.

이번 한 주도 고생 많으셨습니다.

jxmen added 2 commits September 7, 2021 21:30

docs: 상품 관련 클래스 javadoc 추가

a2df5f9

feat: User 도메인 생성

5055f04

jxmen changed the base branch from main to pangnem September 7, 2021 14:25

jxmen added 4 commits September 7, 2021 23:26

fix: User 생성자에서 id 제외

7df9d53

feat: CreateUserDto, UserController 추가

d8dbfa4

update: WebMvcTest로 변경

18f1ba7

feat: UserService 추가

9ee6628

- UserController는 UserService에 의존 - UserControllerTest는 UserService를 mock으로 사용

jxmen added 10 commits September 8, 2021 11:36

fix: /user -> /users 경로 수정

a9cb753

fix: 생정자 엑세스 protected로 변경

f4204b3

feat: UserRepository 추가

a22e417

UserService에서 repository 의존성 추가

test: 유저 수정 Web 테스트 추가

6e668dd

test: UserService 유저 업데이트 기능 테스트

b45b315

UserService.updateUser

feat: 유저 정보 수정 기능 추가

79413f3

User.changeInfo

feat: UserService 유저 수정 기능 추가

9f26c8b

refactor: 유저 서비스 유저 수정 map() 사용

963fe85

refactor: UserRepository mock 제거

61ab3a8

refactor: UserService mock 제거

7438d10

jxmen commented Sep 8, 2021

View reviewed changes

jxmen added 3 commits September 8, 2021 21:33

docs: 회원 HTTP 요청 수정

7664fd9

js 테스트 코드에 있는 메서드와 경로로 수정합니다.

update: 회원 정보 수정시 HTTP Method POST -> PATCH로 변경

acab5dc

docs: 회원 관련 클래스 javadoc 추가

e209121

- 유저 -> 회원으로 이름 통일

hannut91 reviewed Sep 8, 2021

View reviewed changes

jxmen added 7 commits September 9, 2021 10:52

docs: UserNotFoundException 문구 수정

be1a413

update: 사용자를 찾을 수 있는 경우로 문구 수정

437a55d

refactor: ObjectMapper로 Serialization 적용

59a8793

test: 회원 삭제 컨트롤러 테스트 작성

8c8158c

test: 회원 삭제 서비스 테스트 작성

48e4c50

feat: UserService 회원 삭제 기능 추가

456312a

feat: UserController 회원 삭제 기능 추가

e0a1e39

jxmen added 3 commits September 9, 2021 15:48

test: UserControllerTest 유효하지 않은 DTO 테스트 추가

f12ae05

refactor: UserControllerTest 삭제 요청시 fixtureUser로 분리

06aaf36

docs: 상품 관련 클래스 누락된 javadoc 추가

c3d21c7

hannut91 reviewed Sep 9, 2021

View reviewed changes

jxmen added 5 commits September 10, 2021 05:45

refactor: fixture 추출 - id, user

ba7059d

refactor: dto를 Describe에 미리 만들고 setup에서 선언하도록 변경

08ad801

remove: createUser 테스트 삭제

a8be41c

fix: JpaRepository에 누락된 메서드 추가

db8a77d

fix: 누락된 @Transactional 어노테이션 추가

180ede5

hannut91 reviewed Sep 10, 2021

View reviewed changes

jxmen added 10 commits September 11, 2021 04:40

refactor: id 필드로 추출, 저장 전 모든 회원 삭제

907eca7

fix: 회원 수정 요청 시 email 필수요소에서 제외

168c7ad

update: 잘못된 단어 수정

57c5bca

feat: 상품 CORS 정책 적용

54c73e9

docs: repository 설명 수정

a959635

- 저장소 대신 컬렉션 표현 사용 - 기타 하는 일 상세 설명

feat: 회원 생성 시 응답 DTO 추가

18022fb

update: 회원 생성 요청 응답 시 id도 추가

66e692b

단, 테스트 시에 응답 객체를 만들 경우 id가 null이 되므로 Json 문자열로 만들때는 null 값을 제외한다. reference: - https://junho85.pe.kr/1626

feat: 회원 정보 수정 시 응답 DTO 추가

b60ade0

docs: 회원 생성 컨트롤러 문구 수정

20ad779

rename: 기존 dto 이름에 request 추가

a131d49

CreateUserDto -> CreateUserRequestDto UpdateUserDto -> UpdateUserRequestDto

hannut91 reviewed Sep 11, 2021

View reviewed changes

hannut91 merged commit 62d09f6 into CodeSoom:pangnem Sep 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pangnem] 5주차 과제 제출 #37

[pangnem] 5주차 과제 제출 #37

jxmen commented Sep 7, 2021

jxmen commented Sep 8, 2021 •

edited

jxmen Sep 8, 2021

hannut91 Sep 8, 2021

hannut91 Sep 8, 2021

jxmen Sep 9, 2021 •

edited

hannut91 Sep 8, 2021

jxmen Sep 9, 2021

hannut91 Sep 9, 2021

jxmen Sep 10, 2021

hannut91 Sep 10, 2021

hannut91 Sep 8, 2021

jxmen Sep 9, 2021

hannut91 Sep 8, 2021

jxmen Sep 9, 2021 •

edited

hannut91 Sep 9, 2021

jxmen Sep 9, 2021

hannut91 Sep 9, 2021

hannut91 Sep 9, 2021

jxmen Sep 9, 2021

hannut91 Sep 9, 2021

jxmen commented Sep 10, 2021 •

edited

hannut91 Sep 10, 2021

jxmen Sep 10, 2021 •

edited

hannut91 Sep 11, 2021 •

edited

hannut91 commented Sep 13, 2021

[pangnem] 5주차 과제 제출 #37

[pangnem] 5주차 과제 제출 #37

Conversation

jxmen commented Sep 7, 2021

jxmen commented Sep 8, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxmen Sep 9, 2021 • edited

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

jxmen Sep 9, 2021 • edited

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

jxmen commented Sep 10, 2021 • edited

Choose a reason for hiding this comment

jxmen Sep 10, 2021 • edited

Choose a reason for hiding this comment

hannut91 Sep 11, 2021 • edited

Choose a reason for hiding this comment

hannut91 commented Sep 13, 2021

jxmen commented Sep 8, 2021 •

edited

jxmen Sep 9, 2021 •

edited

jxmen Sep 9, 2021 •

edited

jxmen commented Sep 10, 2021 •

edited

jxmen Sep 10, 2021 •

edited

hannut91 Sep 11, 2021 •

edited