yhxkit / prob_201808

prob_201808

prob_201808's Issues

게시판 소스 피드백 1차

prob_201808/08.pilot/test180924/src/main/java/com/home/test180924/config/WebConfig.java

Lines 30 to 37 in 5720c0e

	@Override
	public void addInterceptors(InterceptorRegistry registry) {
	registry.addInterceptor(loginCheckInterceptor).addPathPatterns("/myPage/**", "/admin/**", "/bbs/write", "/bbs/*/comment", "/logout", "/update", "/deleteAccount"); //해당 url은 로그인 되지 않았으면 접속 ㄴㄴ
	//rest로 해놔서 같은 url에 메서드만 다른건 어떻게 하지? 상세보기만 하게 하고 싶은데, 지금 상태로는 쓰기는 안되는데 수정/삭제까지 가능한 상태... 이거는 자기가 쓴 글만 되도록 하면 되나?
	registry.addInterceptor(adminCheckInterceptor).addPathPatterns("/admin/**");
	}

인터셉터를 적용할 경로를 지정해주는 방식을 사용했는데, 어노테이션을 붙여서 인터셉터 적용도 가능하니 참고해보세요(http://www.myjavarecipes.com/tag/spring-mvc-interceptor-annotation-example/)

prob_201808/08.pilot/test180924/src/main/java/com/home/test180924/controller/interceptor/AdminCheckInterceptor.java

Lines 24 to 32 in 5720c0e

	@Override
	public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception {
	String token =request.getHeader("token");
	log.debug("관리자 프리 핸들러");
	if(!jwt.parseToken(token).get("scope").equals(Auth.ADMIN.toString())){ // scope 가 ADMIN이 아니면
	log.debug("관리자 페이지에 접근하려는 유저 "+jwt.parseToken(token).get("subject"));
	response.sendError(402, "관리자 권한이 필요합니다");
	return false;
	}

들여쓰기가 안된걸로 보이는데, 깃헙에서 못맞춰주는건가요? 실제 코드상에서 들여쓰기가 저렇다면 들여쓰기 맞춰주는 습관을 들여보세요~

prob_201808/08.pilot/test180924/src/main/java/com/home/test180924/controller/responseUtil/EntityForResponse.java

Line 8 in 5720c0e

@Component //이거도 implements 해야 하는걸까..?

이 주석은 개인프로젝트라 상관없지만, 실무일땐 이렇게 자신이 메모해둔 주석은 안쓰는게 좋아요

prob_201808/08.pilot/test180924/src/main/java/com/home/test180924/controller/PostingController.java

Lines 89 to 93 in 5720c0e

	if(!writerCheckValidator.checkWriter(postWriterEmail, token)){
	resultMap.put("result", "fail");
	resultMap.put("message", "글쓴이가 아닙니다");
	return responseEntity.get(resultMap);
	}

PostingController에 Writer를 체크하는 로직이 중복되어 들어갔는데, 리팩토링해보세요(메소드 추출)

또한, 컨트롤러 코드에서 if절을 써서 유효성을 검사한뒤, 그에 맞는 에러메시지를 작성했는데 이것도 비지니스 로직 중에 하나라 validator쪽으로 옮겨가는게 좋을거 같아요(validator에서 유효성검사 실패시 그에맞는 에러를 설정하여 컨트롤러쪽으로 전달)
추후에 '글쓴이가 아닙니다' 이외에 다른 에러가 발생할 가능성이 생긴다면(관리자권한이나 블럭되는 유저같은 경우) 위 코드가 존재하는 컨트롤러를 전부 수정해야 할거에요

prob_201808/08.pilot/test180924/src/main/java/com/home/test180924/controller/PostingController.java

Lines 105 to 111 in 5720c0e

	Account writer = accountService.findByPostsPostIdx(postIdx);
	String postWriterEmail = writer.getEmail();
	if(!writerCheckValidator.checkWriter(postWriterEmail, token)){
	resultMap.put("result", "fail");
	resultMap.put("message", "글쓴이가 아닙니다");
	return responseEntity.get(resultMap);
	}

checkWriter메소드에서 Account을 받아 이메일을 검사하도록 수정하는게 좀 더 깔끔할거 같네요. 컨트롤러에서는 최대한 비지니스 로직은 안넣는게 좋아요

prob_201808/08.pilot/test180924/src/main/java/com/home/test180924/service/jwt/JWTImpl.java

Lines 75 to 85 in 5720c0e

	}catch (ExpiredJwtException e) {
	log.debug("ExpiredJwtException : "+token);
	}catch(UnsupportedJwtException e) {
	log.debug("UnsupportedJwtException : "+token);
	}catch(MalformedJwtException e) {
	log.debug("MalformedJwtException : "+token);
	}catch(SignatureException e) {
	log.debug("SignatureException : "+token);
	}catch(IllegalArgumentException e) {
	log.debug("IllegalArgumentException : "+token);
	}

예외가 발생하면 작성하신 .debug메소드로 출력하는 단순 한 라인만 나오지 않나요? 좀 더 자세한 에러 메시지가 나오는게 좋을거 같아요(e와 관련된..e.stacktrace나 e.getMessage 정도)

수정 고려해 볼 것

1. xml 설정을 java Configuration 으로 변경을 해보세요.
2. Entity 만들 때 Vo(Value Object)는 사용하지 않고 그냥 객체 명으로 만들어 주세요.

3. prob_201808/04.test180817/test18081702/src/main/java/com/test/prob/controller/ListController.java

   Line 12 in 92456ee

   import java.util.Date;

   
   이건 LocalDate 써서 해보는 것으로

4. prob_201808/04.test180817/test18081702/src/main/java/com/test/prob/controller/ListController.java

   Line 27 in 92456ee

   @RequestMapping(value = "/", method = RequestMethod.GET)

   
   이런건 GetMapping('...') 이런 식으로 줄여서

5. prob_201808/04.test180817/test18081702/src/main/java/com/test/prob/controller/ListController.java

   Line 50 in 92456ee

   System.out.println(test);

   
   이런건 Logger를 사용하세요. http://addio3305.tistory.com/43

6. prob_201808/04.test180817/test18081702/src/main/java/com/test/prob/controller/ListController.java

   Line 65 in 92456ee

   if(toDoBean.getDateFrom().equals("")||toDoBean.getDateFrom()==null){

   
   반대 일 것 같은데, null 인 경우 우선 에러 납니다. 그리고 이런 경우는 "".equal(...) 이런식으로 구현하는게 나을 것이고 더 낳은 방식은 Optional 쓰는 방식입니다.

7. prob_201808/04.test180817/test18081702/src/main/java/com/test/prob/controller/ListController.java

   Lines 65 to 67 in 92456ee

   	if(toDoBean.getDateFrom().equals("")||toDoBean.getDateFrom()==null){
   	toDoBean.setDateFrom(today);
   	}

   
   초기값 설정인 듯 한데 이런 건 Entity에서 하는게 맞을 것 같내요.

8. prob_201808/04.test180817/test18081702/src/main/java/com/test/prob/controller/ListController.java

   Lines 92 to 122 in 92456ee

   	@ResponseBody //
   	@RequestMapping(value="/{1}", method=RequestMethod.PUT, produces = MediaType.APPLICATION_JSON_UTF8_VALUE) //왜 인서트는 받아오면서 여기서는 못받아와서 이렇게 짜야하는지?
   	public void editOne(@PathVariable("1") int idx, @RequestBody Map<String, Object> params) throws Exception {
   	//json으로 보내서 @RequestBody를 적용한 MAP 으로만 받아지고, ToDoVo 커맨드 객체는 먹히지 않는다...
   	System.out.println("수정 "+idx);
   	System.out.println(params);
   	ToDoVo toDoBean = new ToDoVo();
   	toDoBean.setToDoIdx(idx);
   	toDoBean.setDateFrom((String)params.get("dateFrom"));
   	toDoBean.setDateTo((String)params.get("dateTo"));
   	if(toDoBean.getDateFrom().equals("")||toDoBean.getDateFrom()==null){
   	toDoBean.setDateFrom(today);
   	}
   	if(toDoBean.getDateTo().equals("")||toDoBean.getDateTo()==null){
   	toDoBean.setDateTo(today);
   	}
   	toDoBean.setTitle((String)params.get("title"));
   	toDoBean.setTags((String)params.get("tags"));
   	toDoBean.setStatus((boolean)params.get("status"));
   	System.out.println(toDoBean);
   	listDao.editOne(toDoBean);
   	}

   
   이건 아까 팀업으로 이야기해 준 것 처럼 https://www.baeldung.com/spring-httpmessageconverter-rest 이것 보고 한번 변경해주세요.

그리고 Lombok 이라는 것이 있는데요(https://projectlombok.org/) Entity 쪽을 좀 더 편하게 만들어주는 것 입니다. 한번 공부해서 적용해보세요.

코드 리뷰 피드백

prob_201808/04.test180817/test18081702/src/main/java/com/test/prob/controller/ListController.java

Line 94 in 92456ee

public void editOne(@PathVariable("1") int idx, @RequestBody Map<String, Object> params) throws Exception {

edit에 대한 성공/실패가 있을거라 결과를 리턴해주고, 유저에게 수정에 대한 결과를 알려주는게 좋을거 같아요(addOne, deleteOne도 마찬가지, 유저에게 최대한 친절하게)

prob_201808/04.test180817/test18081702/src/main/java/com/test/prob/controller/ListController.java

Lines 100 to 117 in 92456ee

	ToDoVo toDoBean = new ToDoVo();
	toDoBean.setToDoIdx(idx);
	toDoBean.setDateFrom((String)params.get("dateFrom"));
	toDoBean.setDateTo((String)params.get("dateTo"));
	if(toDoBean.getDateFrom().equals("")||toDoBean.getDateFrom()==null){
	toDoBean.setDateFrom(today);
	}
	if(toDoBean.getDateTo().equals("")||toDoBean.getDateTo()==null){
	toDoBean.setDateTo(today);
	}
	toDoBean.setTitle((String)params.get("title"));
	toDoBean.setTags((String)params.get("tags"));
	toDoBean.setStatus((boolean)params.get("status"));

ToDoVo 객체 설정은 ToDoVo 객체 안으로 옮기는게 좋을거 같아요(컨트롤러와 모델 결합도 최소화)

prob_201808/04.test180817/test18081702/src/main/java/com/test/prob/model/ListDaoImpl.java

Lines 28 to 42 in 92456ee

	public List<ToDoVo> selectAllWithTag(String tag) throws Exception{
	List<Integer> idxs = sqlSession.selectList(namespaceForTag+"selectIdxsForOneToDo", tag);
	// System.out.println(tag+" 태그로 찾은 리스트사이즈 "+idxs.size());
	// "." 은 검색안돼ㅠㅠ...
	List<ToDoVo> result = new ArrayList<>();
	for(int toDoIdx : idxs){
	ToDoVo toDoBean = sqlSession.selectOne(namespaceForToDo+"selectOne", toDoIdx);
	result.add(toDoBean);
	}
	return result;
	}

리스트로 idx 가져오고 idx로 건당 가져오는데, mybatis단에서 조인쿼리쓰면 쿼리 한번으로 해결할 수 있을 듯 한데..
뭔가 안되서 이렇게 하신듯한데..모르겠으면 물어보세용~

prob_201808/04.test180817/test18081702/src/main/java/com/test/prob/model/ListDaoImpl.java

Lines 91 to 135 in 92456ee

	public void editOne(ToDoVo toDoBean) throws Exception {
	// db접속 1, todo테이블 업데이트
	// idx를 이용해서
	// db 접속 2, tag 테이블 업데이트
	sqlSession.update(namespaceForTag+"updateWhere", toDoBean);
	int idx = toDoBean.getToDoIdx();
	ToDoVo oldBean = sqlSession.selectOne(namespaceForToDo+"selectOne", idx);
	List<TagVo> oldTags = sqlSession.selectList(namespaceForTag+"selectTagsForOneToDo", idx);
	String oldTagStr="";
	for(TagVo oldTag : oldTags){
	oldTagStr+=(oldTag.getTag()+" ");
	}
	oldBean.setTags(oldTagStr.trim());
	System.out.println("기존 데이터 "+oldBean);
	System.out.println("수정 데이터 "+toDoBean);
	if(toDoBean.getTags().trim().equals("")) {
	System.out.println("새 태그가 없어서 기존 태그 전부 삭제");
	sqlSession.delete(namespaceForTag+"deleteTag", idx);
	return;
	}
	if(oldBean.getTags()=="") {
	System.out.println("기존 태그가 없어서 새 태그만 추가");
	insertTags(toDoBean, idx);
	return;
	}
	if(!oldBean.getTags().trim().equals(toDoBean.getTags().trim())) { //태그는 수정됐을 때에만 일괄 삭제 후 다시 넣음;
	sqlSession.delete(namespaceForTag+"deleteTag", idx);
	insertTags(toDoBean, idx);
	}
	}

남이 봐도 이해하기 쉬운 코드가 좋은 코드입니당
메소드 추출(메소드명도 신경써서)을 써서 좀더 알아보기 쉽도록 수정하면 좋을거 같아요

prob_201808/04.test180817/test18081702/src/main/resources/spring/root-context.xml

Lines 8 to 12 in 92456ee

	<bean id="dataSource" class="org.springframework.jdbc.datasource.SimpleDriverDataSource">
	<property name="driverClass" value="com.mysql.jdbc.Driver"/>
	<property name="url" value="jdbc:mysql://127.0.0.1:3306/testdb?useUnicode=true&amp;characterEncoding=utf8"/>
	<property name="username" value="root"/>
	<property name="password" value="mysqlroot"/>

요기 DB 연결정보같은건 설정 프로퍼티로 빼는게 좋을거 같아요

게시판 소스 피드백 1차

prob_201808/08.pilot/test180924/src/main/java/com/home/test180924/controller/AdminController.java

Lines 44 to 47 in 5720c0e

	int page=0;
	if(request.getParameter("page")!=null){
	page = Integer.parseInt(request.getParameter("page"))-1;
	}

https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/bind/annotation/RequestParam.html

RequetParam을 사용하면 기본값과 이런 것 처리 가능

prob_201808/08.pilot/test180924/src/main/java/com/home/test180924/service/AccountServiceImpl.java

Line 48 in 5720c0e

public HashMap<String, ?> login(Account account){

Map 말고 객체로

prob_201808/08.pilot/test180924/src/main/java/com/home/test180924/service/AccountServiceImpl.java

Lines 94 to 98 in 5720c0e

	if(Status.ENABLE==account.getStatus()){
	account.setStatus(Status.DISABLE);
	}else if(Status.DISABLE==account.getStatus()){
	account.setStatus(Status.ENABLE);
	}

이런건 IF 문 말고 https://www.tutorialspoint.com/java/lang/enum_valueof.htm
enum의 valueOf를 사용해서 하시면 됩니다.

나머지는 보는대로 ..