[com8599-issue6] user 회원가입 기능 #22

com8599 · 2021-08-09T03:50:03Z

Issue: #6

작업 내용

유저 회원가입 기능 추가

생성/변경 로직

UserBean추가
스웨거추가
UserController 추가
에러 코드 추가(패킷결과)
result json 처리부분 추가
java secure key 관련 추가

개인 코멘트

~~ptmt 이슈 해결을 못했습니다. 이후 해결시 추가 푸쉬 하겠습니다.~~ map>>User객체 변경 후 해결완료되었습니다

- line coverage : 0.7 > 0.5 - method coverage : 1.0 > delete

feat: UserBean, UserController, Swagger, 등 기본 에셋 추가

jinyoungchoi95

바쁘셨을텐데 api 구현 진행하느라 고생하셨습니다 :)

첫 개발 세팅이라 많은 부분이 힘드셨을 텐데 몇가지 코멘트 남겼으니 확인부탁드려요~

jinyoungchoi95 · 2021-08-09T05:58:08Z

src/main/java/com/study/realworld/bean/UserBean.java

+import java.util.Map;
+
+@Component
+public class UserBean {


UserBean이 하는 역할이 정확하게 어떤 역할일까요?
db에 쿼리를 날리고 결과를 받아오는, UserDAO 혹은 UserRepository라는 네이밍에 더 걸맞은 클래스일 수 있지 않을까요?
@Component 대신에 @Repository 어노테이션에 대해서 알아보시는 것도 좋을 것 같아요 :)

jdbctemplate 객체를 선언, 초기화 하여, jdbctemplate의 객체로부터 쿼리를 날리는 dao 역할을 하고있습니다.
클래스 네임을 dao로 변경하는것이 좋을거같네요 피드백 감사합니다.

jinyoungchoi95 · 2021-08-09T06:02:10Z

src/main/java/com/study/realworld/bean/UserBean.java

+        return jdbcTemplate.update(query, new PSSetDao.PSSForTripleString(username, email, password));
+    }
+
+    public Map<String, Object> getUsers(String email) {     //getUsers


Map을 그대로 반환하는 것보다는 User라는 도메인 객체에 담아서 보내는 것이 좋지 않을까요?

모든 상황에서 객체를 반환하는 것이 좋다라는것은 단정할 수 없지만 Map을 반환하는 경우에는 항상 주석을 통해서 해당 Map이 어떤 값을 보내는지, 어떤 정보를 나타내는지를 표시하는 것이 중요해요. (다른 사람과의 협업을 위해서요)
그래서 간단하게 보내는 정보에 대해서는 Map을 반환하면서 주석을 통해서 정보를 나타내는 경우도 있지만 User는 자주 사용할 도메인이지 않을까요? 항상 주석을 통해서 남기는 것보다는 도메인 객체를 추가하는 방향에 대해서 고민해보는 것도 좋을 것 같아요 :)

User객체 추가해서 새로 커밋했습니다 피드백감사합니다!

jinyoungchoi95 · 2021-08-09T06:04:47Z

src/main/java/com/study/realworld/common/Errors.java

@@ -0,0 +1,31 @@
+package com.study.realworld.common;
+


코드가 어떻게 커밋되었는지는 모르겠지만 항상 코드를 작성하고 커밋하기 전에는 컨벤션 확인 부탁드려요 :)

mac: ⌥⌘L

windows: Ctrl+Alt+L

컨벤션에 대해 무지했었는데 알게 되어서 너무 신기합니다 감사합니다!!

jinyoungchoi95 · 2021-08-09T06:09:40Z

src/main/java/com/study/realworld/common/Func.java

+    public static String getZipJson( String bin ) {
+        JsonObject json = new JsonObject();
+
+        json.addProperty("Z", bin);
+
+        return json.toString();
+    }
+
+    public static class RseBinJson {
+        JsonObject R;
+    }
+
+    public static String getBinJson(JsonObject result, Key key) {
+        RseBinJson rse = new RseBinJson();
+        rse.R = result;
+
+        JsonObject json = new JsonObject();
+
+        json.addProperty("B", SecurityFunc.encryptData(SecurityFunc.AES_ALGORITHM, key, rse));
+
+        return json.toString();
+    }


어디에 사용되는 메소드일까요?

음.. 미래에 필요할거같아서 남겨뒀었는데 관련 파일들 삭제하겠습니다.
감사합니다

jinyoungchoi95 · 2021-08-09T06:11:51Z

src/main/java/com/study/realworld/common/Func.java

+        if(objects != null && objects.length > 0) {
+            for(int i = 0; i < objects.length; i = i + 2) {
+                json.addProperty(String.valueOf(objects[i]), String.valueOf(objects[i + 1]));
+            }
+        }


Suggested change

if(objects != null && objects.length > 0) {

for(int i = 0; i < objects.length; i = i + 2) {

json.addProperty(String.valueOf(objects[i]), String.valueOf(objects[i + 1]));

}

}

if(objects == null && objects.length == 0) {

return json.toString();

}

for(int i = 0; i < objects.length; i = i + 2) {

json.addProperty(String.valueOf(objects[i]), String.valueOf(objects[i + 1]));

}

그냥 이렇게 바로 반환하는게 더 맞지 않을까요?

depth는 최대한 2 이하로 맞추는 걸 고려해보아요 :)

헙 감사합니다. 바로 바꾸겠습니다

jinyoungchoi95 · 2021-08-09T06:18:05Z

src/main/java/com/study/realworld/controller/UserController.java

+public class UserController implements ControllerBase{
+    public UserBean userBean;
+
+    @Autowired


Suggested change

@Autowired

생성자 주입의 경우 @Autowired를 제거해도 괜찮아요.

@Autowired 를 사용하지 않을 경우 dao가 null로 보여 객체를 쓸수없게 되어서 넣었는데 틀린 문법인가요?

jdbctemplate외에 다른
@Autowired를 모두 삭제하고 @repository를 붙였습니다.

null로 붙이는게 어떤말씀인지는 잘모르겠어요 :)
생성자 주입으로써

public final UserBean userBean; public UserController(UserBean userBean) { this.userBean = userBean; }

을 말씀드린 거였습니다
해결하셨다니 좋습니당 :)

jinyoungchoi95 · 2021-08-09T06:18:28Z

src/main/java/com/study/realworld/controller/UserController.java

+@RestController
+@RequestMapping("/api")
+public class UserController implements ControllerBase{
+    public UserBean userBean;


Suggested change

public UserBean userBean;

public final UserBean userBean;

static대신 final인자로 변경했습니다 감사합니다.

jinyoungchoi95 · 2021-08-09T06:23:40Z

src/main/java/com/study/realworld/controller/UserController.java

+        JsonObject user = new Gson().toJsonTree(param).getAsJsonObject();
+        if (userBean.checkSameName(getString((JsonObject) user.get("user"), "username"))) {  //닉네임 체크
+            return Func.getErrorJson(Errors.SAME_NICKNAME);
+        } else if (userBean.checkSameEmail(getString((JsonObject) user.get("user"), "email"))) { //email 체크
+            return Func.getErrorJson(Errors.SAME_EMAIL);
+        }
+
+        if ( userBean.registUser(getString((JsonObject) user.get("user"), "username"), getString((JsonObject) user.get("user"), "email"), getString((JsonObject) user.get("user"), "password")) < 0 ) {
+            //등록하다가 에러났을경우
+            return Func.getErrorJson(Errors.DB);
+        }
+
+        Map<String, Object> users = userBean.getUsers(getString((JsonObject) user.get("user"), "email"));
+        if ( users == null ) {
+            return Func.getErrorJson(Errors.INVALID_REQUEST);
+        }
+        JsonObject jsonObject = new Gson().toJsonTree(users).getAsJsonObject();
+        JsonObject result = new JsonObject();
+        result.add("user", jsonObject);
+
+        return Func.getResultJson(result);


컨트롤러 하나가 가지고 있는 역할이 너무 방대해요.

컨트롤러는 단순히 View와의 통신을 위한 다리라고 생각하고 분리한다면 어떨까요? db에 등록하는 역할이 view와 통신해야하는 컨트롤러가 해야하는 역할이 될 수 있을까요?
Service를 들어보셨을거라고 생각하는데 Service가 Controller의 역할을 조금 더 덜어갈 수 있지 않을까요?

서비스로 분리 하여 새로 커밋하였습니다 리뷰 감사합니다!

jinyoungchoi95 · 2021-08-09T06:24:45Z

src/main/java/com/study/realworld/secure/IGenerator.java

+	static final String _upper = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
+	static final String _upperHex = "0123456789ABCDEF";


Suggested change

static final String _upper = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";

static final String _upperHex = "0123456789ABCDEF";

static final String UPPER = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";

static final String UPPER_HEX = "0123456789ABCDEF";

상수에 대해서 자바 컨벤션은 항상 대문자 입니다 :)

바뀐 상수들에 대해서 런 이슈있었던 부분들 새로 수정하여 커밋했습니다!

jinyoungchoi95 · 2021-08-09T06:28:24Z

src/main/java/com/study/realworld/secure/GUIDGenerator.java

+	protected final int _length;
+	protected final Random _random;


Suggested change

protected final int _length;

protected final Random _random;

protected final int length;

protected final Random random;

변수명에 언더바를 쓰지 않는 것이 컨벤션입니다 :)

바뀐 상수들에 대해서 런 이슈있었던 부분들 새로 수정하여 커밋했습니다!

Co-authored-by: 최진영 <[email protected]>

상수에 대한 자바 컨벤션은 항상 대문자 스네이크 이다. Co-authored-by: 최진영 <[email protected]>

변수명에는 언더바를 쓰지않는것이 자바컨벤션입니다. Co-authored-by: 최진영 <[email protected]>

jinyoungchoi95 · 2021-08-09T08:49:21Z

src/main/java/com/study/realworld/bean/User.java

@@ -0,0 +1,41 @@
+package com.study.realworld.bean;
+
+public class User {


User domain 👍
bean 큰 의미보다는 domain이라는 패키지명이 더 어울릴 것 같아요 :)

아하 패키지명 변경하겠습니다! 감사합니다

jinyoungchoi95 · 2021-08-09T08:52:00Z

src/main/java/com/study/realworld/dao/UserDao.java

+@Component
+@Repository


Suggested change

@Component

@Repository

@Repository

@Repository 어노테이션을 확인해보시면 @Comonent 어노테이션이 포함되어있어요 :)

헉.. 헐.. 다른 어노테이션인줄알았는데 감사합니다 적용하겠습니다!

jinyoungchoi95 · 2021-08-09T08:52:18Z

src/main/java/com/study/realworld/dao/UserDao.java

+public class UserDao {
+    private final JdbcTemplate jdbcTemplate;
+
+    @Autowired


Suggested change

@Autowired

제거하셔도 무방합니다~

헐 구글링했던 코드에는 그대로 되있어서 있어야 도는줄 알고있었네용... 없애고 한번 돌려보고 커밋하겠습니다 감사합니다!

이유를 추가적으로 설명드리자면, 생성자가 1개 있는 컴포넌트의 경우 스프링에서 빈으로 올릴 때 자동으로 삽입하기 때문에 @Autowired라는 키워드가 필요없는 것입니다.

jinyoungchoi95 · 2021-08-09T08:54:40Z

src/main/java/com/study/realworld/service/UserService.java

+@Service
+@Repository


Suggested change

@Service

@Repository

@Service

UserDao가 @Repository의 역할을 하고 있고 이를 주입받고 있으니 따로 Service에서는 처리하지 않으셔도됩니다~

아하 감사합니다 수정하겠습니다!

jinyoungchoi95 · 2021-08-09T09:03:50Z

src/main/java/com/study/realworld/service/UserService.java

+        if ( userDao.registUser(getString(user, "username"),
+                getString(user, "email"),
+                getString(user, "password")) < 0 ) {
+            //등록하다가 에러났을경우
+            return Func.getErrorJson(Errors.DB);
+        }
+
+        User users = userDao.getUsers(getString(user, "email"));
+        if ( users == null ) {
+            return Func.getErrorJson(Errors.INVALID_REQUEST);
+        }


입력받은 user를 저장하고 user를 또 반환을 하는 메소드네요.

저장을 한 user를 db에서 반환하는 이유가 있을까요? 실제로 db에 들어간 것을 또 가져온다는 의미에서 명시적으로 처리한다라고 말할 수 있겠지만 불필요한 행위가될 수 있을 것 같아요.
내가 만든 객체가 db에 저장이 된다면 결국 db에서 가져오는 객체는 내가 만든 객체랑 동일한 값을 가질테니까요.
id가 필요한 상황이라면 다음과 같이 Dao를 변경하는 것도 고려할 수 있을것 같네요 :)

public User registerUser(String username, String email, String password) { KeyHolder keyHolder = new GeneratedKeyHolder(); jdbcTemplate.update(conn -> { PreparedStatement ps = conn.prepareStatement("insert into USER(USERNAME, EMAIL, PASSWORD) values (?, ?, ?)", new String[]{"id"}); ps.setString(1, username; ps.setString(2, email); ps.setString(3, password); return ps; }, keyHolder); return null; }

헐 아 헐... update하면서 객체 리턴한다는 건 생각 못했네요!!....
일단은 update중에 db에러가 날 상황도 있을수있으니 해당 코드자체는 바로 머지는 힘들겠지만,
다시 db접근하여 이미 있는 데이터를 다시 불러오는방식은 지양하도록 하겠습니다 감사합니다!

jinyoungchoi95 · 2021-08-09T09:05:57Z

src/main/java/com/study/realworld/service/UserService.java

+        JsonObject result = new JsonObject();
+        ObjectMapper objectMapper = new ObjectMapper();
+        result.addProperty("user", objectMapper.writeValueAsString(users));


json으로 객체를 반환하는 로직이네요.
이전에 말씀드렸다시피 controller에서는 view와의 통신을 담당하는 역할로써 view에서 전달한 json을 객체로 매핑하는, 이를 service에 전달하는, service에서 받은 객체를 다시 view에게 전달하기 위해 객체를 json으로 전달하는 역할을 해요.

지금 로직을 controller에서 담당하도록 하고, Service는 로직이 수행된 객체만을 반환할 수 있는 방법을 생각해보는것이 어떨까요?

에러 로직을 처리하는부분들까지 같이 수정해서 수행된 객체만을 반환하고싶었으나, 그렇게 되었을 경우 에러 로직에 이슈가 있어 수정하지 못했습니다. 이후에 방법이 새로 생각나면 고치겠습니다 감사합니다!

@repository

@repository안에 @component가 있습니다! 주의주의! Co-authored-by: 최진영 <[email protected]>

chance0523

일단 1차 리뷰 드립니당 👍

chance0523 · 2021-08-09T09:19:27Z

src/main/java/com/study/realworld/RealworldApplication.java

+import org.springframework.boot.CommandLineRunner;
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Bean;
+
+import java.util.Arrays;


안 쓰는 import는 지우는 것이 좋습니다.
IntelliJ를 쓰신다면 커밋시에 optimize import에 체크하고 올리시면 됩니다.

피드백감사합니다 다음 커밋부터 유의하여 커밋하겠습니다!!

chance0523 · 2021-08-09T09:24:00Z

src/main/java/com/study/realworld/common/Errors.java

@@ -0,0 +1,40 @@
+package com.study.realworld.common;
+
+public enum Errors {


이건 단수인 Error가 더 좋지 않을까요?

단수가 맞는거같습니다. 예리한 지적감사합니다!

chance0523 · 2021-08-09T09:29:26Z

src/main/java/com/study/realworld/common/Errors.java

+
+    NOT_FOUND_SESSION_USER(10000, "유저 정보를 찾을 수 없습니다."),
+
+    SQLEXCEPTION(-2, "서버와 통신중 에러가 발생했습니다."),


위와 동일하게 SQL_EXCEPTION으로 쓰는 것이 좋아보이네요

trailing good 👍

감사합니다! 언더바 추가하겠습니다!

chance0523 · 2021-08-09T09:29:50Z

src/main/java/com/study/realworld/common/Func.java

+
+import java.security.Key;
+
+public class Func {


이 네이밍은 공식 문서 같은 곳에 있는건가요?

따로 공식 문서에 있어서 쓰진 않았습니다. 제가 쓰던 함수들을 그대로 들고 와서 쓴것입니다.
Func라는 클래스 이름이 조금 어색한거같아서 JsonFunc으로 변경하겠습니다!

chance0523 · 2021-08-09T09:32:16Z

src/main/java/com/study/realworld/service/UserService.java

+        }
+
+        User users = userDao.getUsers(getString(user, "email"));
+        if ( users == null ) {


Suggested change

if ( users == null ) {

if (users == null) {

로 하는게 좋겠네요. 29 라인두

넵 감사합니다!

chance0523 · 2021-08-09T09:32:50Z

src/main/java/com/study/realworld/service/UserService.java

+            return Func.getErrorJson(Errors.DB);
+        }
+
+        User users = userDao.getUsers(getString(user, "email"));


Suggested change

User users = userDao.getUsers(getString(user, "email"));

User user = userDao.getUser(getString(user, "email"));

단건만 가져오니 위에처럼 바꾸는 것이 좋아보이네요

해당 함수는 회원가입단계에서 필요하지 않다 판단하여 삭제하였습니다.
다음에 같은 기능을 넣게 된다면 고려하겠습니다 감사합니다

chance0523 · 2021-08-09T09:33:47Z

src/main/java/com/study/realworld/dao/UserDao.java

+        return (boolean) jdbcTemplate.query(query, new PSSetDao.PSSForString(email), new ResultDao.RSEForBoolean());
+    }
+
+    public int registUser(String username, String email, String password) { //신규 유저 삽입


registerUser 로 바꾸는 것이 좋겠네요
regist: 등록
register: 등록하다

다음부터 명사보다는 동사를 쓰도록 하겠습니다!
그리고 슬랙에서 피드백주신대로 register는 '회원가입하다'라는 뜻이 강해 db에 insert하는것이니 insert* 으로 이름짓기로 하였습니다! 감사합니다

chance0523 · 2021-08-09T09:36:48Z

src/main/java/com/study/realworld/dao/UserDao.java

+        this.jdbcTemplate = jdbcTemplate;
+    }
+
+    public boolean checkSameName(String username) { //같은 닉네임의 유저가 있는지 확인


checkName()

findByName 같은 식으로 그냥 이름으로 찾아서
boolean이 아니라 List 같은걸로 반환하는것은 어떨까요?
반환 받는 곳에서 어차피 에러 던지고 있으니까 거기서 다 판단하는것이 좋아보여요
아래 Email 도 마찬가지

서비스와 컨트롤러를 분리하여 커밋해보겠습니다. 감사합니다.

…sonFunc, follow Java's conventions, pssetdao make short, resultdao remove

DolphaGo

제가 모르겠는 코드들이 많네요 @_@
몇 가지만 리뷰 드렸습니다.

DolphaGo · 2021-08-11T09:49:18Z

build.gradle

    runtimeOnly 'com.h2database:h2'
    testImplementation 'org.springframework.boot:spring-boot-starter-test'
+    implementation group: 'com.google.code.gson', name: 'gson', version: '2.7'
+    implementation 'jakarta.xml.bind:jakarta.xml.bind-api:2.3.3'


자바 객체와 xml을 직렬화/역직렬화 하는 부분이 어디인가요?

DolphaGo · 2021-08-11T09:51:44Z

src/main/java/com/study/realworld/common/ErrorCode.java

+    EXCEPTION(-1, "통신중 에러가 발생했습니다."),
+    NONE(0, "이상없음"),
+    INVALID_REQUEST(1, "잘못된 요청입니다."),
+    DB(2, "데이터 베이스 오류입니다."),
+    SAME_NICKNAME(3, "동일 닉네임유저가 존재합니다."),
+    SAME_EMAIL(4, "동일 이메일유저가 존재합니다."),
+
+    NOT_FOUND_SESSION_USER(10000, "유저 정보를 찾을 수 없습니다."),
+
+    SQLEXCEPTION(-2, "서버와 통신중 에러가 발생했습니다."),
+    ;


EXCEPTION 이라는 단어만 보고 통신 중 에러가 발생했는지
SQLEXCEPTION이라는 단어만 보고 서버와 통신 중 에러가 발생했는지 추측하기가 어려운데요.

ENUM 명을 NOT_FOUND_SESSION_USER 처럼 구체적으로 작성하는 것이 좋을 것 같습니다.

DolphaGo · 2021-08-11T09:53:07Z

src/main/java/com/study/realworld/common/JsonFunc.java

+        for (int i = 0; i < objects.length; i = i + 2) {
+            json.addProperty(String.valueOf(objects[i]), String.valueOf(objects[i + 1]));
+        }
+        System.out.println(errorCode.name() + " - " + errorCode.getCode() + " - " + errorCode.getDesc());


logging이 더 좋을 것 같네요.

DolphaGo · 2021-08-11T09:53:50Z

src/main/java/com/study/realworld/controller/UserController.java

+        this.userService = userService;
+    }
+
+    @ApiOperation(value = "사용자 등록", notes = "사용자 등록")


동일한 내용을 표현하고 있는데, value와 notes를 구분하신 이유는 무엇인가요?

povia

고생 많으셨습니다. 추가적으로 필요한 부분 + 아시면 좋을 내용들로 리뷰를 좀 꾸려봤어요. 힘드시더라도 적용하시면서 어떠한 부분을 고려해야 할지, 더 나은 방향으로 운영/유지보수할 수 있을 지를 고민해보시는 것도 좋아보입니다.
그리고 꼭 테스트 코드 넣어주세요. 개발할 때에는 귀찮아도 테스트 코드가 있고 없고 차이는 어마무시합니다. 내가 개발을 하고 QA -> 릴리즈의 과정에서 나오는 많은 이슈들을 미리 없앨 수 있기 때문에 꼭 필요할 거에요.
다시 한번 너무 고생 많으셨습니다. 해당 부분 + jwt가 사실 이 프로젝트에서 가장 중요하고 힘든 부분이기 때문에 이 부분만 넘기신다면 나머지 부분은 그리 어렵지 않을 겁니다. 조금만 더 힘내봅시다!

povia · 2021-08-13T19:14:22Z

src/main/java/com/study/realworld/service/UserService.java

+        if (userDao.insertUser(user.getUsername(), user.getEmail(), user.getPassword()) < 0) {
+            //등록하다가 에러났을경우
+            return ErrorCode.DB;
+        }


삽입 시 에러가 날 때에는 throw exception을 하는게 좋지 않을까요?

povia · 2021-08-13T19:15:36Z

src/main/java/com/study/realworld/service/UserService.java

+        this.userDao = userDao;
+    }
+
+    public Object users(User user) {


아래의 ErrorCode를 반환하는 부분 때문에 Object type으로 반환토록 설정하신 것으로 보이는데 삽입 시 에러가 나면 exception을 던지도록 하고, 정상적인 경우에는 User를 반환하는 형식으로 구조를 잡으시는 게 좋아보입니다

povia · 2021-08-13T19:16:12Z

src/main/java/com/study/realworld/service/UserService.java

+    public UserService(UserDao userDao) {
+        this.userDao = userDao;
+    }


불변으로 레포 선언 + 생성자 주입 좋습니다 👍

povia · 2021-08-13T19:17:37Z

src/main/java/com/study/realworld/dao/UserDao.java

+public class UserDao {
+    private final JdbcTemplate jdbcTemplate;
+
+    @Autowired


이유를 추가적으로 설명드리자면, 생성자가 1개 있는 컴포넌트의 경우 스프링에서 빈으로 올릴 때 자동으로 삽입하기 때문에 @Autowired라는 키워드가 필요없는 것입니다.

povia · 2021-08-13T19:20:02Z

src/main/java/com/study/realworld/dao/UserDao.java

+
+    public Optional<User> getUserByName(String username) throws EmptyResultDataAccessException { //같은 닉네임의 유저가 있는지 확인
+        final String query = "select id, email, username, password, bio, image from USER where USERNAME=?";
+        User user = (User) jdbcTemplate.query(query, new PSSetDao.PSSForStrings(username), new ResultDao.RSEForUser());


파라메터에 Dao를 사용하기 보다는 username을 그대로 매핑하는 것이 좋아보입니다. 추가로 resultset에 대해서는 다음 페이지를 보시는 것도 좋아보여요.
Spring JdbcTemplate Querying Examples

povia · 2021-08-13T19:30:20Z

src/main/java/com/study/realworld/controller/UserController.java

+    @ApiOperation(value = "사용자 등록", notes = "사용자 등록")
+    @PostMapping("/users")   //post 등록 api
+    public String users(@JsonProperty("user") User user) throws JsonProcessingException {
+        Object result = userService.users(user);


변수의 타입에 Object를 사용하는 방식은 좋지 않아 보입니다 ㅜㅜ

povia · 2021-08-13T19:31:55Z

src/main/java/com/study/realworld/controller/UserController.java

+
+    @ApiOperation(value = "사용자 등록", notes = "사용자 등록")
+    @PostMapping("/users")   //post 등록 api
+    public String users(@JsonProperty("user") User user) throws JsonProcessingException {


Input Parameter로 User를 직접 사용하기 보다는, UserDto 등으로 사용하시는 것이 좋아보입니다. request와 response는 Controller 등의 표현 영역에서 받아 서비스 단에 넘기는 시점에 서비스에 필요한 타입으로 변환해서 넘기고, 받는 것이 좋아요.
나중에 DDD를 공부하시게 된다면 세세하게 아실 수 있을 듯 합니다!

povia · 2021-08-13T19:34:14Z

src/main/java/com/study/realworld/controller/UserController.java

+        if (result instanceof ErrorCode) {
+            return JsonFunc.getErrorJson((ErrorCode) result);
+        }


해당 방식으로 에러를 넘기는 것보다는 throw exception을 처리하도록 하시는게 좋아보여요.
Error Handling for REST with Spring
추천드리는 방식으로 개발을 하실 경우, exception을 따로 관리할 수 있기 때문에 운영, 유지보수 측면에서 이점이 상당히 높아집니다^^

povia · 2021-08-13T19:34:48Z

src/main/java/com/study/realworld/controller/UserController.java

+        if (result instanceof ErrorCode) {
+            return JsonFunc.getErrorJson((ErrorCode) result);
+        }
+        return JsonFunc.getResultJson(user);


반환 타입으로 String을 사용하시기 보다는 ResponseEntity를 사용하시는 것이 좋아보입니다.

povia · 2021-08-13T19:35:09Z

src/main/java/com/study/realworld/swagger/SwaggerConfig.java

+
+@Configuration
+@EnableSwagger2
+public class SwaggerConfig {


jinyoungchoi95 and others added 4 commits August 5, 2021 12:00

chore: test.gradle coverage change

9a6f361

- line coverage : 0.7 > 0.5 - method coverage : 1.0 > delete

chore: build.yml push, pr branch retouch

91cddf0

https://github.com/real-world-study/realworld/issues/6

94ef3b8

feat: UserBean, UserController, Swagger, 등 기본 에셋 추가

getUsers 주석 추가

c04e473

com8599 added the com8599 답이 없다잉의 이슈 label Aug 9, 2021

com8599 changed the title ~~[#6] user 회원가입 기능~~ [com8599-issue6] user 회원가입 기능 Aug 9, 2021

com8599 requested review from jinyoungchoi95, hoony-lab, chance0523, DolphaGo, kwj1270, povia and sujl95 August 9, 2021 06:15

jinyoungchoi95 reviewed Aug 9, 2021

View reviewed changes

jinyoungchoi95 assigned com8599 Aug 9, 2021

com8599 and others added 5 commits August 9, 2021 16:49

[#6] refactor:서비스 분리, map대신 객체로 불러오기

1f7101b

[#6] refactor: depth 2 이하로 조정

d2a8e48

Co-authored-by: 최진영 <[email protected]>

[#6] refactor: 상수-자바컨벤션

892214a

상수에 대한 자바 컨벤션은 항상 대문자 스네이크 이다. Co-authored-by: 최진영 <[email protected]>

[#6] refactor: 변수-자바컨벤션

c932065

변수명에는 언더바를 쓰지않는것이 자바컨벤션입니다. Co-authored-by: 최진영 <[email protected]>

[#6] refactor: 자바컨벤션 규칙에 맞춰서 전체 코드수정

803a1b8

com8599 requested a review from jinyoungchoi95 August 9, 2021 09:02

jinyoungchoi95 reviewed Aug 9, 2021

View reviewed changes

[#6] refactor: 어노테이션 중복 삭제

d96caed

@repository안에 @component가 있습니다! 주의주의! Co-authored-by: 최진영 <[email protected]>

chance0523 reviewed Aug 9, 2021

View reviewed changes

com8599 added 3 commits August 9, 2021 18:52

[#6] 패키지명 변경, 불필요한 어노테이션 삭제, 이미 있는 데이터 접근 지양적인 코드로 수정

071157e

[#6] unused import removing, Errors>>ErrorCode, underbar Add, Func>>J…

a19bb8b

…sonFunc, follow Java's conventions, pssetdao make short, resultdao remove

[#6] ptmt issue complete, return by User

7ee0063

DolphaGo reviewed Aug 11, 2021

View reviewed changes

povia requested changes Aug 13, 2021

View reviewed changes

		static final String _upper = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
		static final String _upperHex = "0123456789ABCDEF";

		@@ -0,0 +1,41 @@
		package com.study.realworld.bean;

		public class User {

		@@ -0,0 +1,40 @@
		package com.study.realworld.common;

		public enum Errors {


		NOT_FOUND_SESSION_USER(10000, "유저 정보를 찾을 수 없습니다."),

		SQLEXCEPTION(-2, "서버와 통신중 에러가 발생했습니다."),

	User users = userDao.getUsers(getString(user, "email"));
	User user = userDao.getUser(getString(user, "email"));

[com8599-issue6] user 회원가입 기능 #22

Are you sure you want to change the base?

[com8599-issue6] user 회원가입 기능 #22

Conversation

com8599 commented Aug 9, 2021 • edited Loading

Issue: #6

작업 내용

생성/변경 로직

개인 코멘트

jinyoungchoi95 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

jinyoungchoi95 Aug 9, 2021 • 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

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

chance0523 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

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

DolphaGo 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

povia 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

com8599 commented Aug 9, 2021 •

edited

Loading

jinyoungchoi95 Aug 9, 2021 •

edited

Loading