Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jwt choi #12

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Jwt choi #12

wants to merge 11 commits into from

Conversation

JongHak19
Copy link
Collaborator

  • 앱 실행시 admin 계정 생성(UserService.java)
  • Article CRUD 구현

Copy link
Member

@cmsong111 cmsong111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드리뷰 완료했습니다!

부족한 부분도 보이지만, 인터넷에서 검색도 많이 하신게 눈에 느껴지네요..ㅎ
고생하셨습니다!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컨트룰러에서는 최대한 서비스 로직을 제거해주세요

단일 책임 원칙에 위반됩니다..!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PostMapping("/create")
@Operation(summary = "게시글 생성")
@PreAuthorize("hasRole('ROLE_ADMIN') or hasRole('ROLE_CLIENT')")
public Article create(@RequestBody CreateDto article) {

    Article article1 = Article.builder().subject(article.getSubject()).content(article.getContent()).build();

    //현재 인증된 사용자 가져오기
    Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
    String username = authentication.getName(); // 현재 인증된 사용자의 이름(아이디)를 가져온다.

    User user = userService.getUserByName(username); // DB에서 사용자를 조회합니다.
    return articleService.create(article1, user);
}

Entity를 그대로 반환하는 것은 좋지 않아요!
아래에 User Class를 보니 @JsonManagedReference가 있던데 이를 사용하는것 보다 DTO를 생성하는게 좋아요

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Article CreateDto와 UpdateDto가 코드가 일치하는것 같은데, 차라리 Article Form Dto 로 통일하는건 어떨까요?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DB(SQL)에 시간을 저장할 때는
java.time.LocalDateTime 대신
java.sql.timestamp 클래스를 사용해주세요

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public Article getArticle(Long id){
    List<Article> Allarticle = articleRepository.findAll();

    for(Article article: Allarticle){
        if(article.getIdx() == id){
            return article;
        }
    }
    throw new RuntimeException("No article");
}

위와 같은 코드는

Repository에 findbyUserIdx 메소드를 생성해주세요
Jpa Pallete를 활용하면 편하게 메소드를 생성할 수 있어요

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JsonManagedReference 제거 후 UserDTO도 생성해주세요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants