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

feat: controller, service 수정 #14 #19

Merged
merged 21 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package HookKiller.server.notice.controller;

import HookKiller.server.common.type.LanguageType;
import HookKiller.server.notice.dto.NoticeArticleDto;
import HookKiller.server.notice.dto.NoticeContentDto;
import HookKiller.server.notice.entity.NoticeArticle;
import HookKiller.server.repository.NoticeArticleRepository;
import HookKiller.server.repository.NoticeContentRepository;
import HookKiller.server.service.NoticeService;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.web.bind.annotation.*;

import java.util.List;
import java.util.Optional;

@RestController
@RequiredArgsConstructor
@RequestMapping("/notice")
public class NoticeController {

private final NoticeService noticeService;
private final NoticeArticleRepository articleRepository;
private final NoticeContentRepository contentRepository;

/**
* 단건 조회
* @param noticeArticleId
* @param request
* @param languageType
* @return
*/
@GetMapping("{/noticeArticleId}")
public NoticeArticleDto getNoticeArticle(@PathVariable Long noticeArticleId, HttpServletRequest request, LanguageType languageType) {
Copy link
Member

Choose a reason for hiding this comment

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

Controller에서 LanguageType을 받을 수 있을 지가 확실하지않네요
아마 requestHeader에 넣어주고 받을 것 같습니다
이미 밑에서 해준 것 같으니 LanguageType을 빼도 될 것 같아요
requestHeader를 사용하기 위해서는 @RequestHeader 어노테이션에 대해서도 알아보시면 좋을 것 같네요

request.getHeader("languageType");
return noticeService.getNoticeArticleByArticleId(noticeArticleId, languageType);
}

/**
* 리스트 조회
* @param request
* @param languageType
* @param requestData
* @return
*/
@GetMapping
public List<NoticeArticleDto> getNoticeArticleList(HttpServletRequest request, LanguageType languageType, NoticeArticleDto requestData) {
Copy link
Member

Choose a reason for hiding this comment

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

마찬가지로 LanguageTyperequestHeader에서 뽑아올 수 있도록 구성할 것이므로 빼도 될 것 같습니다

request.getHeader("id");
Long id = requestData.getId();
List<NoticeArticleDto> articleDto = noticeService.getNoticeArticleList(id, languageType);
return articleDto;
}

/**
* 공지사항 등록
*/
@PostMapping
public void addNotice(@RequestBody @Valid NoticeArticleDto articleRequest, NoticeContentDto contentRequest) {
Copy link
Member

Choose a reason for hiding this comment

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

@Valid를 사용하려면 DTO 내에서 작업이 필요합니다
참고해보세요

noticeService.saveNotice(articleRequest, contentRequest);
}

/**
* 공지사항 수정
*/
@PutMapping("/{id}")
public void updateNotice(@PathVariable Long id,
@RequestBody @Valid NoticeArticleDto articleDto, NoticeContentDto contentDto) {
Copy link
Member

Choose a reason for hiding this comment

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

마찬가지로 @Valid를 사용하기 위해서는 따로 작업이 필요합니다
@RequestBody를 사용할 때는
@RequestBody @Valid UpdateNoticeRequest body와 같이 구성하고
request dto를 하나 만들어보시는게 나을 수도 있겠네요

noticeService.update(id, articleDto);
Optional<NoticeArticle> findNotice = articleRepository.findById(id);
}

/**
* 공지사항 삭제
*/
@DeleteMapping("/{noticeArticleId}")
public void deleteNotice(@PathVariable Long noticeArticleId) {
noticeService.deleteNotice(noticeArticleId);
}
Comment on lines +82 to +85
Copy link
Member

Choose a reason for hiding this comment

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

@DeleteMapping에 대해서는 추가적으로 찾아보셔야 할 것 같네요


}

35 changes: 35 additions & 0 deletions src/main/java/HookKiller/server/notice/dto/NoticeArticleDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package HookKiller.server.notice.dto;

import HookKiller.server.common.type.LanguageType;
import HookKiller.server.common.type.NoticeArticleStatus;
import HookKiller.server.notice.entity.NoticeArticle;
import HookKiller.server.notice.entity.NoticeContent;
import lombok.Builder;
import lombok.Getter;

import java.sql.Timestamp;

@Getter
@Builder
public class NoticeArticleDto {
Copy link
Member

Choose a reason for hiding this comment

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

dev을 머지하시고
AbstractTimestamp를 상속받아보세요


private Long id;
private LanguageType language;
private NoticeArticleStatus status;
private Timestamp createdAt;
private Long createdUser;
private Timestamp updatedAt;
private Long updatedUser;

public static NoticeArticleDto of(NoticeArticle noticeArticle, NoticeContent noticeContent) {
return NoticeArticleDto.builder()
.id(noticeArticle.getId())
.language(noticeArticle.getLanguage())
.status(noticeArticle.getStatus())
.createdAt(noticeArticle.getCreatedAt())
.createdUser(noticeArticle.getCreatedUser())
.updatedAt(noticeArticle.getUpdatedAt())
.updatedUser(noticeArticle.getUpdatedUser())
.build();
}
}
27 changes: 27 additions & 0 deletions src/main/java/HookKiller/server/notice/dto/NoticeContentDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package HookKiller.server.notice.dto;

import HookKiller.server.common.type.LanguageType;
import HookKiller.server.notice.entity.NoticeArticle;
import HookKiller.server.notice.entity.NoticeContent;
import lombok.Builder;
import lombok.Getter;

@Getter
@Builder
public class NoticeContentDto {

private Long id;
private LanguageType language;
private String title;
private String content;

public NoticeContentDto of(NoticeArticle noticeArticle, NoticeContent noticeContent) {

return NoticeContentDto.builder()
.id(noticeContent.getId())
.language(noticeContent.getLanguage())
.title(noticeContent.getTitle())
.content(noticeContent.getContent())
.build();
}
Copy link
Member

Choose a reason for hiding this comment

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

of 메소드로 build하기 좋습니다

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package HookKiller.server.notice.exception;

import HookKiller.server.common.exception.BaseException;

import static HookKiller.server.notice.exception.NoticeException.NOTICE_ARTICLE_NOT_FOUND_EXCEPTION;

public class NoticeArticleNotFoundException extends BaseException {
public static final BaseException EXCEPTION = new NoticeArticleNotFoundException();

private NoticeArticleNotFoundException() {
super(NOTICE_ARTICLE_NOT_FOUND_EXCEPTION);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package HookKiller.server.notice.exception;

import HookKiller.server.common.dto.ErrorDetail;
import HookKiller.server.common.exception.BaseErrorCode;
import lombok.AllArgsConstructor;
import lombok.Getter;
import org.springframework.http.HttpStatus;

import static org.springframework.http.HttpStatus.NOT_FOUND;

@Getter
@AllArgsConstructor
public enum NoticeException implements BaseErrorCode {
NOTICE_ARTICLE_NOT_FOUND_EXCEPTION(NOT_FOUND.value(), "404-1", "공지사항 게시물이 존재하지 않습니다.");
Copy link
Member

Choose a reason for hiding this comment

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

굿굿 좋습니다
앞으로 notice와 관련된 Exception들은 모두 여기에 모아놓으면 되겠네요


private final Integer statusCode;
private final String errorCode;
private final String reason;

@Override
public ErrorDetail getErrorDetail() {
return ErrorDetail.of(statusCode, errorCode, reason);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

@Repository
public interface NoticeArticleRepository extends JpaRepository<NoticeArticle, Long> {
NoticeArticle findAllById(NoticeArticle noticeArticle);
NoticeArticle findAll(NoticeArticle noticeArticle);
NoticeArticle getById(Long id);
Copy link
Member

Choose a reason for hiding this comment

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

레포지토리에서 getById는 쓰지 않는 것이 좋겠네요
아마 findById라는 메소드가 기본적으로 제공되는 것으로 알고 있습니다


}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import HookKiller.server.notice.entity.NoticeArticle;
import HookKiller.server.notice.entity.NoticeContent;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;

import java.util.Optional;

Expand Down
90 changes: 90 additions & 0 deletions src/main/java/HookKiller/server/service/NoticeService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package HookKiller.server.service;

import HookKiller.server.common.type.LanguageType;
import HookKiller.server.notice.dto.NoticeArticleDto;
import HookKiller.server.notice.dto.NoticeContentDto;
import HookKiller.server.notice.entity.NoticeArticle;
import HookKiller.server.notice.entity.NoticeContent;
import HookKiller.server.notice.exception.NoticeArticleNotFoundException;
import HookKiller.server.repository.NoticeArticleRepository;
import HookKiller.server.repository.NoticeContentRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

@Service
@RequiredArgsConstructor
public class NoticeService {

private final NoticeArticleRepository noticeArticleRepository;
private final NoticeContentRepository noticeContentRepository;

@Transactional
public void saveNotice(NoticeArticleDto articleDto, NoticeContentDto contentDto) {
NoticeArticle noticeArticle = NoticeArticle.builder()
.id(articleDto.getId())
.build();
NoticeContent noticeContent = NoticeContent.builder()
.id(contentDto.getId())
.build();
noticeArticleRepository.save(noticeArticle);
noticeContentRepository.save(noticeContent);
}

@Transactional(readOnly = true)
Copy link
Member

Choose a reason for hiding this comment

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

readonly 좋습니다

public List<NoticeArticleDto> getNoticeList() {

List<NoticeArticle> all = noticeArticleRepository.findAll();
List<NoticeArticleDto> noticeArticleDtoList = new ArrayList<>();

for (NoticeArticle noticeArticle : all) {
NoticeArticleDto articleDto = NoticeArticleDto.builder()
.id(noticeArticle.getId())
.language(noticeArticle.getLanguage())
.status(noticeArticle.getStatus())
.createdAt(noticeArticle.getCreatedAt())
.createdUser(noticeArticle.getCreatedUser())
.updatedAt(noticeArticle.getUpdatedAt())
.updatedUser(noticeArticle.getUpdatedUser())
.build();
noticeArticleDtoList.add(articleDto);
}
Copy link
Member

Choose a reason for hiding this comment

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

stream을 써보는 것은 어떨까요

return noticeArticleDtoList;
}

@Transactional
public NoticeContentDto getNotice(Long id) {
NoticeContent noticeContent = noticeContentRepository.findById(id).orElseThrow(() -> NoticeArticleNotFoundException.EXCEPTION);

return NoticeContentDto.builder()
.id(noticeContent.getId())
.language(noticeContent.getLanguage())
.title(noticeContent.getTitle())
.content(noticeContent.getContent())
.build();
}
Copy link
Member

Choose a reason for hiding this comment

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

이 코드는 나중에 또 쓸 수 있으니
NoticeContentDtofrom 메소드를 만들어보는 것은 어떨까요?

Suggested change
public NoticeContentDto getNotice(Long id) {
NoticeContent noticeContent = noticeContentRepository.findById(id).orElseThrow(() -> NoticeArticleNotFoundException.EXCEPTION);
return NoticeContentDto.builder()
.id(noticeContent.getId())
.language(noticeContent.getLanguage())
.title(noticeContent.getTitle())
.content(noticeContent.getContent())
.build();
}
public NoticeContentDto getNotice(Long id) {
NoticeContent noticeContent = noticeContentRepository.findById(id).orElseThrow(() -> NoticeArticleNotFoundException.EXCEPTION);
return NoticeContentDto.from(noticeContent);
}

처럼 나오도록 한번 만들어보세요!


@Transactional
public void deleteNotice(Long id) {
noticeArticleRepository.deleteById(id);
}

@Transactional
public void update(Long id, NoticeArticleDto articleDto) {
Optional<NoticeArticle> byId = noticeArticleRepository.findById(id);
// TODO : 수정 받고 다시 작업 예정
}
Copy link
Member

Choose a reason for hiding this comment

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

JPA에서 update는 어떻게 하는지 리서치해보시는것도 추천드립니다


public NoticeArticleDto getNoticeArticleByArticleId(Long noticeArticleId, LanguageType languageType) {
return null;
}

public List<NoticeArticleDto> getNoticeArticleList(Long noticeArticleId, LanguageType languageType) {
return null;
}
Copy link
Member

Choose a reason for hiding this comment

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

이런 것들을 서비스에서 따로 정의하는것도 괜찮지만,
JPA repository에서 바로 가져올 수도 있을 것 같네요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그러면 서비스에서 이것들은 지워버리고 컨트롤러에서 바로 가져오도록 하면 되나요?


}
Loading