1. 회원가입 요구사항 Update by chimaek · Pull Request #1 · mrfourfour/maximus_spring_server

chimaek · 2021-05-11T06:51:00Z

post http메서드로 회원 data보내기 (정상처리 200)
중복 회원있을 시 예외처리 (badRequest 400)

aerain

더 많은 테스트 코드가 필요합니다.

aerain · 2021-05-11T06:54:18Z

src/main/java/com/maximus/spring_server/controller/AccountController.java

+    public Long save(@RequestBody AccountSaveDto accountSaveDto) {
+        return accountService.save(accountSaveDto);


표현층의 dto를 응용층에 내려주고 있는데, 이번 경우에는 간단한 요구사항이라 괜찮지만, 분리를 해야할지 고민해야할 구조입니다.

aerain · 2021-05-11T06:55:43Z

src/main/java/com/maximus/spring_server/service/AccountService.java

+
+    private final AccountRepository accountRepository;
+
+    @ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "잘못된 접근입니다.")


응용층에서 표현층의 책임을 지고있습니다. 응용층에서는 예외를 내려주되, 표현층에서 예외를 핸들링하여 요구사항에 맞는 상태 코드를 부과해야 합니다.

aerain · 2021-05-11T06:56:57Z

src/main/java/com/maximus/spring_server/service/AccountService.java

+    @Transactional
+    public Long save(AccountSaveDto accountSaveDto) {
+        String username = accountSaveDto.getUsername();
+        boolean anyMatch = accountRepository.findAll()


하지말아야할 장애포인트입니다. findAll()은 시간, 공간 복잡도가 O(n)인 데다, 한번에 다량의 데이터를 애플리케이션 메모리에 적재하게 되므로, 애플리케이션 및 저장소 둘다 부담이 가게 됩니다.

aerain · 2021-05-11T06:58:20Z

src/test/java/com/maximus/spring_server/controller/AccountControllerTest.java

+        ResponseEntity<Long> responseEntity = testRestTemplate.postForEntity(url, accountSaveDto, Long.class);
+
+        assertThat(responseEntity.getStatusCode()).isEqualTo(HttpStatus.OK);
+
+        assertThat(responseEntity.getBody()).isGreaterThan(0L);
+
+        AccountSaveDto accountSaveDto2 = AccountSaveDto.builder()
+                .username(username)
+                .password(password)
+                .build();
+
+        ResponseEntity<Object> responseEntity2 = testRestTemplate.postForEntity(url, accountSaveDto2, Object.class);
+        assertThat(responseEntity2.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);


서로 다른 케이스에 대해서 테스트를 실시하고 있습니다. 한가지 테스트는 한가지 시나리오에만 집중해야 합니다.

aerain · 2021-05-11T07:02:07Z

src/test/java/com/maximus/spring_server/controller/AccountControllerTest.java

+import static org.junit.jupiter.api.Assertions.*;
+
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
+class AccountControllerTest {


요건 정확히 말하면 단위 테스트가 아니라, 모든 의존성이 맞물려 연결된 엔드 테스트입니다. 작성하신 각각의 코드들에 대해서 전부 테스트를 작성하셔야 합니다.

AccountService 단위 테스트

정상 작동 케이스

예외 발생 케이스

AccountController 단위 테스트

정상 작동 케이스

예외 발생 케이스

AccountController 엔드 테스트 <- 형이 지금 작성하신거.

aerain · 2021-06-01T13:41:43Z

src/main/java/com/maximus/spring_server/controller/existsUserException.java

+        if (httpStatus == HttpStatus.BAD_REQUEST){
+            System.out.println(s);
+        }


로깅할때는 표준 입출력에 직접 붙어서 사용하지마세요.

aerain · 2021-06-01T13:42:11Z

src/main/java/com/maximus/spring_server/domain/Account.java

+    @GeneratedValue(strategy = GenerationType.AUTO)
+    private Long userId;
+
+    @Id


?? 여기에 있어야할 이유를 모르겠네요

aerain · 2021-06-01T13:43:05Z

src/main/java/com/maximus/spring_server/domain/Account.java

+@Table(name = "account")
+public class Account {
+
+    private final AccountRepository accountRepository;


도메인 엔티티에는 자신의 속성을 표현하는것 이외에 필드를 담으면 안됩니다.

aerain · 2021-06-03T02:24:11Z

src/main/java/com/maximus/spring_server/controller/AccountController.java

@@ -0,0 +1,23 @@
+package com.maximus.spring_server.controller;
+
+import com.maximus.spring_server.domain.Account;


표현층에 도메인층 의존성이 노출되면 안됩니다. 변경에 취약한 구조입니다.

aerain · 2021-06-03T02:25:20Z

src/main/java/com/maximus/spring_server/service/AccountService.java

+        account.save();
+        return accountRepository.save(account);


두 save에는 무슨 차이가 있나요? 🤔

aerain · 2021-06-03T02:26:04Z

src/main/java/com/maximus/spring_server/domain/Account.java

+    public void save() {
+        checkUser(username);
+        checkPassword(password);
+    }


메소드명은 save인데 실상은 유저네임과 패스워드를 체크하는 구조네요? 🤔 메소드 명이 잘못 작성된것 같습니다.

aerain · 2021-06-03T02:27:22Z

src/test/java/com/maximus/spring_server/service/AccountServiceTest.java

+        Account account2 = accountService.saveUser(account);
+        assertThat(account.getUsername()).isEqualTo(username);
+        assertThat(account.getPassword()).isEqualTo(password);
+
+        assertThat(account2.getUsername()).isEqualTo(account.getUsername());
+        assertThat(account2.getPassword()).isEqualTo(account.getPassword());
+
+        assertThat(account2.getUsername()).isNotEmpty();
+        assertThat(account2.getPassword()).isNotEmpty();


반환된 객체는 어차피 given(..).willReturn(...) 로 직접 작성하신 구조인데, 테스트할 의미가 있나요?

그냥 accountRepository.save(..) 가 잘 동작했는지만 판단하면 될것 같습니다.

aerain · 2021-06-17T06:42:25Z

src/main/java/com/maximus/spring_server/controller/AccountController.java

-                e.printStackTrace();
-            }
-            return account;
+    public ResponseEntity<?> signUp(@RequestBody Account account) throws Exception {


throws 필요한가요?

aerain · 2021-06-17T06:42:56Z

src/main/java/com/maximus/spring_server/domain/Account.java

    private String password;

-    public void save() {
+    public void validationUser() {


Suggested change

public void validationUser() {

public void validateUser() {

서술형으로 작성 잊지맙시다

aerain · 2021-06-17T06:44:05Z

src/main/java/com/maximus/spring_server/service/AccountService.java

-        account.save();
-        return accountRepository.save(account);
+    public Account saveUser(Account account) throws Exception {
+        if (accountRepository.findByUsername(account.getUsername()) != null) {


accountRepository.existsByUsername 메소드를 추가하는건 어떨까요?

number1 commit

acb362c

aerain requested changes May 11, 2021

View reviewed changes

chimaek added 2 commits May 18, 2021 11:27

ddd modify

6778671

요구사항 수정 및 ddd적용중

9f70584

aerain requested changes Jun 1, 2021

View reviewed changes

요구사항 수정 및 ddd적용중

b05094a

aerain requested changes Jun 3, 2021

View reviewed changes

요구사항 수정

f7be906

aerain reviewed Jun 17, 2021

View reviewed changes

chimaek added 2 commits January 20, 2022 10:38

reset

a48dcd4

1번 구현소스 추가

9f99d14

		public Long save(@RequestBody AccountSaveDto accountSaveDto) {
		return accountService.save(accountSaveDto);


		private final AccountRepository accountRepository;

		@ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "잘못된 접근입니다.")

		@@ -0,0 +1,23 @@
		package com.maximus.spring_server.controller;

		import com.maximus.spring_server.domain.Account;

Conversation

chimaek commented May 11, 2021

Uh oh!

aerain left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants