[댓글 기능] 제이 미션 제출합니다. #107

JunHoPark93 · 2019-07-30T06:41:31Z

미션이 쉽지 않네요.
요청은 dto를 정의해서 받았고 응답은 댓글만 응답 dto를 서비스 레이어에서 정의했습니다. (댓글시간 계산 필드 때문에)
나머지 user나 article 도 전부 다 dto를 정의해보려다 너무 복잡해지는것 같아서 그냥 domain으로 뷰에 내렸는데 괜찮을까요?
항상 감사드립니다!

리다이렉트, 이름 잘못 수정된것 원복, header location 검증 추가

# Conflicts: # README.md # build.gradle # src/main/java/techcourse/myblog/domain/Article.java # src/main/java/techcourse/myblog/web/ArticleController.java # src/main/resources/application.properties # src/main/resources/templates/article.html # src/test/java/techcourse/myblog/web/ArticleControllerTests.java

@Valid

Squashed commit of the following: commit 3026e5c7eb726b65b992756d9ec17a73bec140d4 Author: JunHoPark <[email protected]> Date: Tue Jul 16 17:54:01 2019 +0900 feat: 유저 조회 기능 구현 commit c82f723cc643fd9aed7686d0231ed18cd35924b7 Author: JunHoPark <[email protected]> Date: Tue Jul 16 16:53:26 2019 +0900 fix: Valid 애노테이션 추가 Validation 에러 수정, @Valid가 없으면 검증이 발동되지 않는다

Squashed commit of the following: commit 3af160545dc0fcdce403342652fd1c9497e60982 Author: JunHoPark <[email protected]> Date: Wed Jul 17 15:57:01 2019 +0900 refactor: html 패키지화 원복 commit abaa172ae7507edcc0a8a21852291f5b216d967a Author: JunHoPark <[email protected]> Date: Wed Jul 17 15:53:36 2019 +0900 feat: 로그아웃 기능 구현 commit a523c371383247a05158711cdc7a1d1c6cd1c2ff Author: JunHoPark <[email protected]> Date: Wed Jul 17 15:26:37 2019 +0900 feat: 로그인 기능 구현 commit 010778b87f6995e0d38a29d15b3041529cb43b20 Author: JunHoPark <[email protected]> Date: Wed Jul 17 15:25:59 2019 +0900 chore: JPA 실행 쿼리 보기 설정 commit fcf46a795cfdb3dcdd14a683d93dd1c1ea0817b7 Author: JunHoPark <[email protected]> Date: Wed Jul 17 14:48:54 2019 +0900 feat: 세션 검증 Interceptor 구현 commit 261149e005613cc1fe6ab4ead0485842ff80780c Author: JunHoPark <[email protected]> Date: Wed Jul 17 13:12:06 2019 +0900 refactor: html 패키지 구조 변경

Squashed commit of the following: commit d36b93b9f56d5a631168ea59425740e3852b4c23 Author: JunHoPark <[email protected]> Date: Wed Jul 17 17:29:53 2019 +0900 feat: 회원 탈퇴 기능 구현 commit 0a0adfd710c88ca96f77b0a14b71812556c0643d Author: JunHoPark <[email protected]> Date: Wed Jul 17 17:07:59 2019 +0900 feat: 회원 수정 기능 구현

Login 관련 에러 처리 객체

woowahanCU · 2019-07-30T15:49:04Z

src/test/java/techcourse/myblog/web/ArticleControllerTests.java

+                .body(fromFormData("name", "Bob")
+                        .with("email", "[email protected]")
+                        .with("password", "PassWord1!")
+                        .with("reconfirmPassword", "PassWord1!"))


get, post 요청 등을 메서드 분리하거나 공통된 URI를 상수처리하여 중복을 없앨 수 있을거 같아요
body에 들어가는 부분도 메서드로 분리하면 재사용할 수 있지 않을까요

private <T> BodyInserters.FormInserter<String> mapBy(Class<T> classType, String... parameters) { BodyInserters.FormInserter<String> body = BodyInserters.fromFormData(Strings.EMPTY, Strings.EMPTY); for (int i = 1; i < classType.getDeclaredFields().length; i++) { body.with(classType.getDeclaredFields()[i].getName(), parameters[i - 1]); } return body; }

woowahanCU

GET /articles/dd HTTP/1.1" 400 7001

GetMapping("/articles/{articleId}" 요청시에
불필요한 요청이 날아가네요.

woowahanCU · 2019-07-30T16:10:59Z

src/test/java/techcourse/myblog/web/ArticleControllerTests.java


    @Test
-    void 게시글수정페이지() {
+    void 다른사용자가_게시글수정페이지접근_시도_게시글페이지로_이동() {


게시글 페이지가 아니라 메인 페이지로 이동되는 것이지요?

woowahanCU

안녕하세요! 리뷰어 CU입니다.
짧은 시간내에 전반적으로 깔끔하게 잘 작성하셨습니다! 👍
몇가지 피드백 남겨두었으니 확인해보시고 궁금한 점 있으면 DM 남겨주세요~ 고맙습니다.

woowahanCU · 2019-07-30T16:21:10Z

추가적으로 질문하신 부분에 대해서 답드리자면,
우선 요청과 응답의 시그니처는 맞춰주는것이 좋다고 생각합니다.
그리고 반드시 DTO를 작성해야 할 필요는 없습니다.
Presentation Layer의 변경사항이 도메인 모델에 영향을 미치는 것을 줄이고자 작성해준다는 관점으로 생각하심이 좋을거 같네요.

Article과 Comment는 User의 행위가 연관되어 있기 때문에 그 둘을 연관시켜 저장하는 auditing기능을 구현한다.

custom Converter를 구현한다.

수정자에 대한 업데이트가 이루어지도록 추가, 이 부분을 추가하려면 authenticated된 유저정보가 필요한데 security를 적용하지 않을 것이므로 static으로 유저정보들을 thread-safe하게 공유할 수 있는 contextholder를 직접 만들어본다.

미션 기한을 맞추기 위해서 추후 기능으로 한다

User 체크를 조금 더 명시적이게 변경

JunHoPark93 · 2019-08-02T06:10:45Z

리뷰 감사합니다!
지적해주신 부분들 반영해보았습니다.

1. JPA Auditing

보내주신 링크를 정독해 보았습니다. 링크안의 @createdby를 쓰게 된다면 수정자도 auditing으로 구현할 수 있다는 것이 매우 의미있다고 생각했습니다. 그런데 @createdby를 쓰려면 User 객체를 전역적으로 가지고 있다가 JPA 설정파일을 만들어 auditorAwareRef에 넣어줘야 했습니다.

문제상황:

저희 앱 자체가 context에 user정보를 가지고 있을 수가 없었습니다. 시큐리티를 썼다면 context에서 코드 한줄이면 현재 logged in 한 유저를 가져올 수 있었지만 지금은 쓸 필요가 없다고 생각해서 (써본적도없고) SessionContextHolder라는 thread-safe한 클래스를 만들어보려고 했습니다. 이 클래스는 제가만든 UserAuthentication 객체 (필드는 현재 한 클라이언트에서 부여받은 session id) 를 key로 user객체를 value로 가지고 있는 concurrentHashmap으로 구현했었습니다. 어떤 사용자가 로그인 시 앞단에서 검증을 통해 현재 서버의 sessionContext에 user객체가 존재한다면 (어디선가 로그인이 된 상태) 로그인을 막거나/그 사람을 튕기거나 둘 중에 하나를 구현하고 있었습니다. 이틀 동안 해본결과 오늘 아침 샤워하다가 초기 설계중 한 부분이 잘못되었다는 것을 뒤늦게 깨달았습니다. 이유는 Bean이 어떻게 관리되는지, thread가 어떻게 돌아가는지에 대한 명확한 이해가 없었기 때문에 로직이 어느정도 완성되었지만 치명적인 문제가 있었습니다.
마감 시간이 중요하다고 생각해서 했던 작업들은 전부 날리고, @createdby도 지우고 리뷰요청을 남깁니다. 이 부분은 미션이 끝나더라도 따로 작업을해서 꼭 완성해볼 예정입니다.

Dto 매핑 위임

ModelMapper를 활용해보라는 피드백을 받고 구현을 해보았습니다. CommentResponseMapper라는 매핑 클래스를 정의해보았습니다.

메서드 호출 의도

지적해주신 Comment의 검증부분이 말씀주신것처럼 의도가 잘 드러나지않은것 같았습니다. 조금 더 의도를 드러내기위해서 comment 객체에게 auther를 물어보는 형식으로 바꿔보았습니다.

test context제거

SaltEncrypt 클래스는 서버를 띄울 필요가 없는 테스트인데 아무생각없이 붙였습니다.. 바로 지웠습니다.

기타

url 매핑 하는 컨트롤러에서 공통 된 부분을 위로 뺐습니다. 테스트의 중복코드를 조금 더 줄여보았습니다.

JunHoPark93 · 2019-08-02T06:45:15Z

GET /articles/dd HTTP/1.1" 400 7001

GetMapping("/articles/{articleId}" 요청시에
불필요한 요청이 날아가네요.

라고 말씀해주신 부분 확인해보았는데 article의 coverUrl의 이미지 url을 찾지 못하였을 때 발생되는것으로 보입니다.

게시글을 대충 생성했을 때 coverUrl필드에 dd, df 그냥 이런 문자열을 저장한 이후 -> 프런트에서 image 태그에 그 url로 요청을보내서 resource를 가지고 오지 못하여 400대 에러가 발생합니다.

혹시 유효한 coverUrl인지 까지 Article객체를 생성할 때 검사를 하라는 것을 말씀하신건가용? 지금 생각나는 것은 article 객체를 저장할때 coverUrl을 restTemplate으로 찔러봐서 유효한 resource인지 검사하는 로직까지 넣는것을 의도하신건지 궁금합니다!

head 요청으로 리소스의 유무를 확인하여 없다면 article 저장이 되지 않게 함

JunHoPark93 · 2019-08-02T07:20:42Z

자답입니다! head 요청으로 resource 유뮤를 확인하여 없다면 게시글 저장이 되지 않게 하였습니다. 없는 url요청하는 불필요한 요청을 없앴습니다

woowahanCU · 2019-08-03T08:09:42Z

src/test/java/techcourse/myblog/web/WebTestHelper.java

+
+import static org.springframework.web.reactive.function.BodyInserters.fromFormData;
+
+public class WebTestHelper {


Helper Class로 중복 제거 한 부분 좋네요! 👍

woowahanCU · 2019-08-03T08:16:15Z

src/main/java/techcourse/myblog/domain/User.java

    private String name;
+
+    @Column(nullable = false)


java bean validator를 사용하여 email 등 포맷의 유효성 검증을 할 수 있어요

https://www.baeldung.com/javax-validation

woowahanCU · 2019-08-03T13:11:11Z

src/main/java/techcourse/myblog/support/config/WebConfig.java

+
+            @Override
+            public void addArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) {
+                resolvers.add(new SessionResolver());


woowahanCU · 2019-08-03T13:11:26Z

src/main/java/techcourse/myblog/support/config/WebConfig.java

+    @Bean
+    public ModelMapper modelMapper() {
+        ModelMapper modelMapper = new ModelMapper();
+        modelMapper.addMappings(new CommentResponseMapper());


woowahanCU · 2019-08-03T13:12:51Z

src/main/java/techcourse/myblog/web/ArticleController.java

-    @PostMapping("/articles")
-    public String saveArticle(@Valid ArticleRequest articleRequest, Model model) {
-        Article article = articleService.save(articleRequest);
+    @PostMapping()


()는 제거해주셔도 됩니다.

woowahanCU · 2019-08-03T13:14:22Z

src/main/java/techcourse/myblog/web/CommentController.java

+@RequestMapping("/comment")
+public class CommentController {
+    private CommentService commentService;
+    private ArticleService articleService;


CommentController가 ArticleService와 연관관계를 갖기보다는 CommentService와 ArticleService가 연관관계를 갖는 것은 어떨까요

제가 단방향 매핑으로 구현을 하였는데 CommentService내에 ArticleService를 맺게되면 ArticleService에도 CommentService를 맺어야 (왜냐하면 Article은 Comment를 모르기 때문에) 될것 같습니다. 혹시 둘의 장단점을 간단히 설명해주실수 있나요?
그리고 Service내에 다른 Service가 의존관계로 들어가도 괜찮은 구조 (같은 layer끼리) 인지 궁금합니다!

woowahanCU · 2019-08-03T13:16:34Z

src/main/java/techcourse/myblog/web/CommentController.java

+    }
+
+    @GetMapping("/{commentId}")
+    public String editCommentPage(@PathVariable("commentId") Long commentId, Model model, User user) {


매소드 이름이 edit보다는 show 등이 더 좋을거 같네요.

woowahanCU · 2019-08-03T13:21:03Z

src/test/java/techcourse/myblog/web/ArticleControllerTest.java

+
+@AutoConfigureWebTestClient
+@ExtendWith(SpringExtension.class)
+@DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD)


DirtiesContext를 사용하면 매번 ApplicationContext를 생성하니 테스트 시간이 오래 걸리지 않나요?

여러 테스트를 할 때 서로 영향을 주지 않게 하기 위해서 설정을 해보았는데 혹시 다른 방법이 있는지 궁금합니다!

woowahanCU · 2019-08-03T13:23:01Z

src/main/java/techcourse/myblog/service/UserService.java

@@ -35,7 +36,7 @@ private User createUser(UserRequest userRequest) {
        }
    }

-    public Iterable<? extends User> findAll() {
+    public List<User> findAll() {
        return userRepository.findAll();


Collections.unmodifiablelist()를 활용하여 read-only로 리턴하여 외부에서 값의 변조를 막을 수 있습니다.

https://www.geeksforgeeks.org/collections-unmodifiablelist-method-in-java-with-examples/

woowahanCU · 2019-08-03T14:55:30Z

src/main/java/techcourse/myblog/validator/EmailConstraintValidator.java

-        }
-
-        return true;
+        return !userRepository.existsByEmail(value);


아, 이런 로직이 있었군요.
Dto와 Repository가 연관관계가 형성되는 것이 좋은 구조인지는 의문이 드네요.
Presentation Layer에서의 값 검증과 Service Layer, Domain Layer에서 각각에서의 유효성 검증은 다르다고 생각해요.
Dto에서는 값의 형식 등을 테스트하는 것이 좋다고 생각합니다
https://stackoverflow.com/questions/21073878/validation-in-3-tier-architecture

woowahanCU

깔끔하게 잘 구현하셨네요!
SaltEncrypt를 작성하여 Password를 암호화한 부분, AuthInterceptor와 SessionResolver를 적용한 부분, 그리고 Helper 클래스를 작성하여 테스트코드의 중복을 제거한 부분 등이 상당히 인상적이었어요! 💯
앞으로 남은 미션들에서도 많은 것을 학습하고 성장하길 바래요~
고맙습니다. 😀

This reverts commit 827275b.

MrKwon and others added 30 commits July 9, 2019 15:47

docs: Readme 업데이트

dd87430

feat: 게시글 생성, 목록 조회 기능 구현

a2287b4

feat: 게시글 조회 구현

940525c

feat: 공통 Head 분리

5430d29

feat: html 중복제거

1f9ef73

feat: 게시글 수정

8dced8c

feat: 게시글 삭제 구현

9a3475c

feat: devtools 적용

8731d42

refactor: Js 분리

66751cd

refactor: 컨트롤러 메서드 이름 수정, autowired 삭제

8901bf4

refactor: 불필요한 메서드 제거

07ebbc1

feat: Article 제목 검증 추가

f2e700e

feat: error 페이지 추가

1961b76

refactor: 피드백 반영

f44d9ff

리다이렉트, 이름 잘못 수정된것 원복, header location 검증 추가

feat: Jpa repository 적용

8ef6ba5

docs: README 업데이트

087e5e0

feat: 회원 등록 기능

15e570a

refactor: html 중복제거 및 세션적용

e6f5579

fix: Interceptor에서 /users post요청은 통과

3e2bd3e

feat: Error 객체의 정의

63235c6

Login 관련 에러 처리 객체

test: 세션관련 인수테스트 작성

924cee2

test: 회원관련 인수테스트

7c83e30

refactor: 들여쓰기 및 사용하지 않는 import 제거

bb280ba

feat: 회원가입시 이메일 중복 오류 처리

e00061f

fix: 회원 가입시 email 중복 테스트 수정

614121c

refactor: UserService 분리

8d0188b

woowahanCU reviewed Jul 30, 2019

View reviewed changes

woowahanCU suggested changes Jul 30, 2019

View reviewed changes

JunHoPark93 added 6 commits July 31, 2019 16:58

feat: JPA auditing 적용

c915849

Article과 Comment는 User의 행위가 연관되어 있기 때문에 그 둘을 연관시켜 저장하는 auditing기능을 구현한다.

feat: Comment 응답 dto에 대한 Custom Mapper의 구현

fc5aa5c

custom Converter를 구현한다.

refactor: test 중복제거

ed0e739

fix: session holder 코드 원복

411388d

미션 기한을 맞추기 위해서 추후 기능으로 한다

refactor: Comment 검증 로직

6be9e90

User 체크를 조금 더 명시적이게 변경

JunHoPark93 added 2 commits August 2, 2019 16:11

feat: CoverUrl resource의 존재 확인

c1b252e

head 요청으로 리소스의 유무를 확인하여 없다면 article 저장이 되지 않게 함

test: coverUrl 경로 수정

8454227

woowahanCU reviewed Aug 3, 2019

View reviewed changes

woowahanCU approved these changes Aug 3, 2019

View reviewed changes

woowahanCU merged commit 827275b into woowacourse:JunHoPark93 Aug 3, 2019

green4469 mentioned this pull request Aug 4, 2019

[댓글 기능] 무민 미션 제출합니다. #106

Merged

JunHoPark93 added a commit to JunHoPark93/jwp-blog that referenced this pull request Aug 4, 2019

Revert "[댓글 기능] 제이 미션 제출합니다. (woowacourse#107)"

7bb4583

This reverts commit 827275b.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[댓글 기능] 제이 미션 제출합니다. #107

[댓글 기능] 제이 미션 제출합니다. #107

JunHoPark93 commented Jul 30, 2019

woowahanCU Jul 30, 2019

woowahanCU left a comment

woowahanCU Jul 30, 2019

woowahanCU left a comment

woowahanCU commented Jul 30, 2019

JunHoPark93 commented Aug 2, 2019

JunHoPark93 commented Aug 2, 2019

JunHoPark93 commented Aug 2, 2019

woowahanCU Aug 3, 2019

woowahanCU Aug 3, 2019

woowahanCU Aug 3, 2019

woowahanCU Aug 3, 2019

woowahanCU Aug 3, 2019

woowahanCU Aug 3, 2019

JunHoPark93 Aug 3, 2019

woowahanCU Aug 3, 2019 •

edited

Loading

woowahanCU Aug 3, 2019

JunHoPark93 Aug 3, 2019

woowahanCU Aug 3, 2019

woowahanCU Aug 3, 2019

woowahanCU left a comment


		import static org.springframework.web.reactive.function.BodyInserters.fromFormData;

		public class WebTestHelper {

[댓글 기능] 제이 미션 제출합니다. #107

[댓글 기능] 제이 미션 제출합니다. #107

Conversation

JunHoPark93 commented Jul 30, 2019

Choose a reason for hiding this comment

woowahanCU left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woowahanCU left a comment

Choose a reason for hiding this comment

woowahanCU commented Jul 30, 2019

JunHoPark93 commented Aug 2, 2019

1. JPA Auditing

문제상황:

Dto 매핑 위임

메서드 호출 의도

test context제거

기타

JunHoPark93 commented Aug 2, 2019

JunHoPark93 commented Aug 2, 2019

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

woowahanCU Aug 3, 2019 • 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

woowahanCU left a comment

Choose a reason for hiding this comment

woowahanCU Aug 3, 2019 •

edited

Loading