[Spring Core] 김학인 과제 제출합니다. by IncleRepo · Pull Request #10 · BCSDLab-Edu/2024-BackEnd

IncleRepo · 2024-06-04T02:40:13Z

과제입니다.

songsunkook

과제를 깔끔하게 해주셨네요
예외처리는 애매한 부분이라고 생각합니다. 조금 더 고민해보면 좋을 것 같아요.
이번 주도 고생하셨습니다~

songsunkook · 2024-06-04T13:47:37Z

src/main/resources/application.yml

gitignore로 제외해주세요

songsunkook · 2024-06-04T13:49:58Z

src/main/java/com/example/demo/repository/ArticleRepositoryJdbc.java

+        } catch (Exception e) {
+            throw new RuntimeException("Article not found");
+        }


모든 익셉션이 반드시 "not found"라는 이유로 발생했다는 확신이 있을까요?

songsunkook · 2024-06-04T13:54:56Z

src/main/java/com/example/demo/service/ArticleService.java

+        } catch (RuntimeException e) {
+            throw new PostNotFoundException(e.getMessage());
+        }


이 클래스의 모든 메서드?에 동일한 예외처리를 하고자 한다면 AOP로 묶어버리는 방법도 있을 것 같아요

seongjae6751

고생 많으셨습니다! 몇가지 피드백 드립니다.
또 merge conflict도 떴으니 pull 당겨서 해결해주세요!

seongjae6751 · 2024-06-26T13:30:51Z

src/main/java/com/example/demo/AOPexception/ExceptHandler.java

+    @ExceptionHandler
+    public ResponseEntity<ErrorResponse> handleException(PutDuplicatedException e) {
+        return new ResponseEntity<>(new ErrorResponse(e.getMessage()),HttpStatus.CONFLICT);
+    }


얘는 httpstatus가 conflict가 적절한 것이 맞을까요?

seongjae6751 · 2024-06-26T13:32:40Z

src/main/java/com/example/demo/AOPexception/ExceptHandler.java

+    @ExceptionHandler
+    public ResponseEntity<ErrorResponse> handleException(PutNotFoundException e) {
+        return new ResponseEntity<>(new ErrorResponse(e.getMessage()),HttpStatus.BAD_REQUEST);
+    }
+    @ExceptionHandler
+    public ResponseEntity<ErrorResponse> handleException(PostIllegalArgumemtException e) {
+        return new ResponseEntity<>(new ErrorResponse(e.getMessage()),HttpStatus.BAD_REQUEST);
+    }
+    @ExceptionHandler
+    public ResponseEntity<ErrorResponse> handleException(PostNotFoundException e) {
+        return new ResponseEntity<>(new ErrorResponse(e.getMessage()),HttpStatus.BAD_REQUEST);
+    }
+    @ExceptionHandler
+    public ResponseEntity<ErrorResponse> handleException(DeleteExistedExcepton e) {
+        return new ResponseEntity<>(new ErrorResponse(e.getMessage()),HttpStatus.BAD_REQUEST);
+    }
+


이 친구들은 bad request라고 생각한 이유가 있나요?
왜 get에서 나오는 not found는 status가 not found이고 put이나 post할 때 나오는 not found는 bad request라고 생각하신 걸까요?

seongjae6751 · 2024-06-26T13:35:49Z

src/main/java/com/example/demo/controller/BoardController.java

+    // 과제 수행함
    private final BoardService boardService;

    public BoardController(BoardService boardService) {


단일 정보를 가져오는 api들이 boards라는 이름을 갖고 있는 것이 적절할까요?

seongjae6751 · 2024-06-26T13:37:12Z

src/main/java/com/example/demo/domain/Article.java

+    public Article() {
+
+    }
+
+


이 친구는 엔티티마다 추가한 이유가 있을까요?
과연 여기서 필요한게 맞을까요?

seongjae6751 · 2024-06-26T13:47:46Z

src/main/java/com/example/demo/domain/Article.java

+    @Id
+    @GeneratedValue(strategy = GenerationType.IDENTITY)
    private Long id;
+    @Column(name = "author_id")
    private Long authorId;
+    @Column(name = "board_id")
    private Long boardId;
    private String title;
    private String content;
+    @Column(name = "created_date")
    private LocalDateTime createdAt;
+    @Column(name = "modified_date")
    private LocalDateTime modifiedAt;


현재 수준에서는 트집 잡기 일 수 있는데 보통은 가독성을 위해서 한칸씩 띄워서 써요~

seongjae6751 · 2024-06-26T13:58:21Z

src/main/java/com/example/demo/service/ArticleService.java

+        if (article.getAuthorId() == null || article.getBoardId() == null ||
+                article.getTitle() == null || article.getContent() == null) {


여기서 필드 검증을 수행하는 것도 좋지만 보통은 dto에서 검증 해요

seongjae6751 · 2024-06-26T13:58:42Z

src/main/java/com/example/demo/service/MemberService.java

+        if (request.name() == null || request.email() == null || request.password() == null) {
+            throw new PostIllegalArgumemtException("NULL field existed");
+        }


여기도 마찬가지!

피드백들을 이제야 확인했네요
천천히 읽어보고 연구해보겠습니다 감사합니다!

Choi-JJunho

고생하셨습니다~
코멘트 확인부탁드려요

Choi-JJunho · 2024-07-03T08:22:09Z

src/main/java/com/example/demo/service/ArticleService.java

+        try {
+            Member member = memberRepository.findById(article.getAuthorId())
+                    .orElseThrow(IllegalArgumentException::new);
+            ;


불필요한 세미콜론이 있네요~

Choi-JJunho · 2024-07-03T08:22:44Z

src/main/java/com/example/demo/service/BoardService.java

    public BoardResponse getBoardById(Long id) {
       try {
-           Board board = boardRepository.findById(id);
+           Board board = boardRepository.findById(id).orElseThrow(IllegalArgumentException::new);;


세미콜론이 여기저기 두번씩 찍혀있네요 ㅋㅋㅋㅋ;;

Choi-JJunho · 2024-07-03T08:23:09Z

src/main/java/com/example/demo/service/BoardService.java

+        Board board = boardRepository.findById(id).orElseThrow(IllegalArgumentException::new);;
        board.update(request.name());
-        Board updated = boardRepository.update(board);
+        Board updated = boardRepository.save(board);


save를 하지 않아도 될거같은데 그 이유는 무엇일까요

피드백들 감사합니다 :)
아무래도 update는 이미 persist된 객체를 수정하는거기때문에
이미 영속화가 되어있어서 객체만 수정해도 fflush()가 호출되는 시점에 db에 자동으로 동기화 될것이기 때문이라 생각합니다.

dirty checking이라는 키워드를 조사해보세요 :)

seongjae6751

고생 많으셨습니다!
솔직히 저도 세션 방식은 한번도 구현해본 적 없는데 보면서 공부가 되었어요
그리고 merge할 것은 아니지만 충돌 방치하면 안 될 것 같아요 해결해주세요

seongjae6751 · 2024-07-10T07:58:42Z

src/main/java/com/example/demo/controller/dto/request/MemberLoginRequest.java

+public record MemberLoginRequest(
+        String email,
+        String password


membercreate dto에서는 필드 검증을 하고 여기서는 필드 검증을 하지 않은 이유가 있나요?

seongjae6751 · 2024-07-10T08:16:05Z

src/main/java/com/example/demo/controller/MemberController.java

+        Member existingUser = (Member) session.getAttribute("loginUser");
+        if (existingUser != null) {
+            return ResponseEntity.status(HttpStatus.FOUND)
+                    .header(HttpHeaders.LOCATION, "/members/info")
+                    .build();
+        }


session은 컨트롤러에서 잘 처리하셨네요 👍👍

seongjae6751 · 2024-07-10T08:19:24Z

src/main/java/com/example/demo/controller/MemberController.java

+        Member user = memberService.login(request);
+        if (user == null) {
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED)
+                    .body("로그인 아이디 또는 비밀번호가 틀렸습니다.");
+        }


여기까지는 욕심일 수도 있는데 user가 없다면 service로직이나 repository 에러를 반환하고 exception handler에서 캐치하는 방식으로 하면 더 깔끔하게 구현 할 수 있었을 것 같아요

seongjae6751 · 2024-07-10T08:24:54Z

src/main/java/com/example/demo/service/MemberService.java

+    @Transactional
+    public Member getLoginUserByLoginId(String loginId) {


전체에 transactional(readonly = true)가 걸려 있어서 여기서 transactional 어노테이션은 필요 없을 것 같네요

seongjae6751 · 2024-07-10T08:27:36Z

src/main/java/com/example/demo/service/MemberService.java

    @Transactional
    public void delete(Long id) {
+        if (!articleRepository.findAllByAuthorId(id).isEmpty())
+            throw new DeleteExistedExcepton("one above articles existed written by this member");


다른 곳에서는 한글 메시지를 남겼던데 여기서는 메시지를 영어로 한 이유가 있나요?

seongjae6751 · 2024-07-10T08:33:05Z

src/main/java/com/example/demo/service/MemberService.java

+    @Transactional
+    public Member getLoginUserByLoginId(String loginId) {
+        if (loginId == null) return null;
+
+        Optional<Member> optionalUser = memberRepository.findByEmail(loginId);
+        return optionalUser.orElse(null);


이 친구는 쓰이는 곳이 보이지 않는 것 같은데 어디에서 쓰이나요?

Choi-JJunho and others added 3 commits May 29, 2024 00:37

feat: 과제 수행

75b1384

implements Exception Handling

8bf4897

fix bug

35b2d50

songsunkook reviewed Jun 4, 2024

View reviewed changes

implement Jpa

90d7114

seongjae6751 reviewed Jun 26, 2024

View reviewed changes

연관관계 매핑,의존성전이

064bf47

Choi-JJunho reviewed Jul 3, 2024

View reviewed changes

IncleRepo added 2 commits July 9, 2024 12:13

implement login

e96817e

simpify login

3996d4c

seongjae6751 reviewed Jul 10, 2024

View reviewed changes

Update README.md

fb4f666

		if (article.getAuthorId() == null \|\| article.getBoardId() == null \|\|
		article.getTitle() == null \|\| article.getContent() == null) {

		@Transactional
		public Member getLoginUserByLoginId(String loginId) {

Conversation

IncleRepo commented Jun 4, 2024

Uh oh!

songsunkook 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!

seongjae6751 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!

Choi-JJunho 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!

seongjae6751 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seongjae6751 Jul 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

seongjae6751 Jul 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seongjae6751 Jul 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

seongjae6751 left a comment •

edited

Loading

seongjae6751 Jul 10, 2024 •

edited

Loading

seongjae6751 Jul 10, 2024 •

edited

Loading

seongjae6751 Jul 10, 2024 •

edited

Loading